Skip to content

Conversation

battermann
Copy link
Contributor

@battermann battermann commented Jul 14, 2022

https://wearezeta.atlassian.net/browse/SQSERVICES-1533

handle is an optional query parameter on the POST and PUT requests. Can't easily add it to the body because the content-type is XML. Refactoring this will be a breaking change for the clients.

When a handle is not given, we will generate one like "IdP 1..n" and take the first one that is not used already.

If an IdP already exists, the handle that is returned is generated from the uuid of the IdP, taking the first 6 characters, e.g.

IdP 63621f

Since IdP handles are only alias, the uniqueness requirements for the handle can be a bit softer. Also, team admins can simply change the handles in case of a clash.

There was a discussion in the BE channel about a potential data migration. But the current implementation doesn’t need one, if it is acceptable from the user’s point of view to have the handles generated from the IdP Id as described above. There is also no need to update the database on GET requests, as discussed elsewhere.

Checklist

  • The PR Title explains the impact of the change.
  • The PR description provides context as to why the change should occur and what the code contributes to that effect. This could also be a link to a JIRA ticket or a Github issue, if there is one.
  • If a cassandra schema migration has been added, I ran make git-add-cassandra-schema to update the cassandra schema documentation.
  • changelog.d contains the following bits of information (details):
    • A file with the changelog entry in one or more suitable sub-sections. The sub-sections are marked by directories inside changelog.d.

@battermann battermann force-pushed the SQSERVICES-1533-be-give-saml-id-ps-human-readable-names-in-tm branch from 414d10f to c50e7bd Compare July 15, 2022 08:31
@battermann battermann temporarily deployed to cachix July 15, 2022 08:31 Inactive
@battermann battermann temporarily deployed to cachix July 15, 2022 09:13 Inactive
@battermann battermann temporarily deployed to cachix July 15, 2022 10:46 Inactive
@battermann battermann marked this pull request as ready for review July 15, 2022 10:51
@battermann battermann requested a review from elland July 15, 2022 10:51
@elland elland force-pushed the SQSERVICES-1533-be-give-saml-id-ps-human-readable-names-in-tm branch from 08156ff to f306ec0 Compare August 8, 2022 08:16
@elland elland temporarily deployed to cachix August 8, 2022 08:16 Inactive
@elland elland temporarily deployed to cachix August 8, 2022 08:16 Inactive
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Aug 8, 2022
@elland elland force-pushed the SQSERVICES-1533-be-give-saml-id-ps-human-readable-names-in-tm branch from f306ec0 to 1dc264d Compare August 8, 2022 08:56
@elland elland temporarily deployed to cachix August 8, 2022 08:56 Inactive
@elland elland temporarily deployed to cachix August 8, 2022 08:56 Inactive
@battermann battermann merged commit cfaf918 into develop Aug 8, 2022
@battermann battermann deleted the SQSERVICES-1533-be-give-saml-id-ps-human-readable-names-in-tm branch August 8, 2022 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants