-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Try to get email from UserInfo before giving up #56
Comments
There might be situations where email is not provided at all (it is not mandatory), in that case it would be useful to default to the value of "sub" in the ID Token. |
Email is not mandatory field, Subject is mandatory and expected to be unique. Might want to take a look at UserInfo first, however. Issue: oauth2-proxy#56
This would be a sensible improvement to make but is probably a fairly substantial change, gonna leave this as |
Email is not mandatory field, Subject is mandatory and expected to be unique. Might want to take a look at UserInfo first, however. Issue: oauth2-proxy#56
So this appears to be working in the current version of oauth2_proxy. I did run into an odd issue, in that the "email" field was being sent as "eMail" for my local OpenID system. Would it be possible (and I'll open a new issue if needed, didn't open one now to avoid issue clutter) to make that match case insensitive? |
Can you tell me which provider you use?
I don't think we should be implementing case insensitivity when reading from IDTokens. OIDC tokens and userinfo endpoints encode data in JSON which is case sensitive. The OIDC Spec also lists the fields and their standard names that any provider should be using. I'd prefer not to have special logic for individual providers who are mis-implementing OIDC in the main codebase if possible. |
@JoelSpeed It's an "internal" supposedly Open ID compliant system. I asked the people responsible for the setup, and there wasn't necessarily a reason that they set it up that way, they just did (and apparently didn't understand the whole Open ID spec and weren't thinking)... I had them add the I wouldn't be asking for anything from the IDTokens to be changed, I agree wholeheartedly that nothing should be touched there. Email from the userinfo endpoint was the only field I was even suggesting be case insensitive, and I totally understand why you wouldn't want to do that. That said, a method to override a keyname like |
I'm trying to use jetbrains hub and I'm getting the I'm willing to do the work to get this fixed, I'm just not sure exactly what you all would be looking for here so some pointers would be cool. |
Actually I found a work around by providing the |
This issue has been inactive for 60 days. If the issue is still relevant please comment to re-activate the issue. If no action is taken within 7 days, the issue will be marked closed. |
Some OIDC providers do not include email in the default IDToken information. In such cases email would need to be requested separately, either via UserInfo interface or via Remote Claims.
Expected Behavior
To get email in providers/oidc.go:createSessionState check ID token first, if email is not there, request UserInfo and check there, if not there then try resolving a claim for "email"
Current Behavior
If email field is not included in the ID token, then authentication just stops with "id_token did not contain an email"
The text was updated successfully, but these errors were encountered: