Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Make username key configurable for SAML auth providers#60603

Merged
pjlast merged 7 commits into
mainfrom
pjlast/59877-inconsistent-usernames-saml-oidc
Mar 13, 2024
Merged

Make username key configurable for SAML auth providers#60603
pjlast merged 7 commits into
mainfrom
pjlast/59877-inconsistent-usernames-saml-oidc

Conversation

@pjlast

@pjlast pjlast commented Feb 19, 2024

Copy link
Copy Markdown
Contributor

Closes #59877

Docs change: sourcegraph/docs#105

Instead of changing the ordering of how the SAML auth provider selects a username (which could cause backwards compatibility issues for existing setups), I decided to add a usernameAttributeNames field to the SAML auth provider config. This way an admin can specify their own preferred set of SAML username keys and configure it based on their preference.

Test plan

Existing tests still passing, new unit test added.

@cla-bot cla-bot Bot added the cla-signed label Feb 19, 2024
@github-actions github-actions Bot added the team/source Tickets under the purview of Source - the one Source to graph it all label Feb 19, 2024
@pjlast pjlast changed the title Refactor OIDC and SAML profile creations to be a bit more readable Make username key configurable for SAML auth providers Feb 19, 2024
@pjlast pjlast marked this pull request as ready for review February 19, 2024 11:49
@pjlast pjlast requested a review from a team February 19, 2024 11:50

@eseliger eseliger left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From that issue, it sounds like that the two providers use different methods to derive the key to uniquely identify a user - do I get that right?

I think I agree with that just changing this will be dangerous, as some customers might rely on this.

Can we make sure to reach out to Chris who filed the ticket to inform the customer about this change and that they should give it a try?

@cconcannon

Copy link
Copy Markdown
Contributor

I think that this change will provide a satisfactory workaround. Thanks!

@pjlast pjlast merged commit 15ef80e into main Mar 13, 2024
@pjlast pjlast deleted the pjlast/59877-inconsistent-usernames-saml-oidc branch March 13, 2024 09:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla-signed team/source Tickets under the purview of Source - the one Source to graph it all

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SAML and OIDC behaviors for forming username are inconsistent

3 participants