Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Fix #3855 #3868

Closed
wants to merge 1 commit into from
Closed

WIP: Fix #3855 #3868

wants to merge 1 commit into from

Conversation

cappyzawa
Copy link
Member

@cappyzawa cappyzawa commented May 15, 2019

Signed-off-by: cappyzawa <cappyzawa@yahoo.ne.jp>
@cappyzawa
Copy link
Member Author

@vito dexidp/dex#1448 is merged 🎉

What should I do to merge this change into concourse/dex ?

@cirocosta cirocosta requested a review from jwntrs May 24, 2019 12:50
@cirocosta
Copy link
Member

Hi @cappyzawa , thanks for the PR!

cc @pivotal-jwinters

@jwntrs
Copy link
Contributor

jwntrs commented May 24, 2019

I'm actually a little hesitant to make this change (even though dex accepted the PR).

OIDC is a standard, that says the sub is the unique identifier for the user. The reason we made the user_id configurable in the OAuth connector is because there is no standard claim to identify the user.

In addition, what you are actually trying to achieve is a better way to identity the user, when doing set-team, so that you don't have to use some guid, is that correct? You can already do this by specifying the username, which already is configurable.

@cappyzawa
Copy link
Member Author

@pivotal-jwinters Oh... I misunderstood that user name key is only related to the display of UI 🤦

Now, I try to login with CONCOURSE_OIDC_USER_NAME_KEY, then succeed.
Thanks for your useful information 🙇

@cirocosta Thanks!!

@cappyzawa cappyzawa closed this May 24, 2019
@cappyzawa cappyzawa deleted the issues/3855 branch May 24, 2019 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

It's difficult to set oidc users
3 participants