-
-
Notifications
You must be signed in to change notification settings - Fork 568
OIDC: use info in id_token if not in response #1259
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
base: master
Are you sure you want to change the base?
OIDC: use info in id_token if not in response #1259
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1259 +/- ##
=======================================
Coverage 78.04% 78.05%
=======================================
Files 354 354
Lines 10915 10918 +3
Branches 481 483 +2
=======================================
+ Hits 8519 8522 +3
Misses 2224 2224
Partials 172 172
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@@ -337,12 +337,19 @@ def user_data(self, access_token, *args, **kwargs): | |||
|
|||
def get_user_details(self, response): | |||
username_key = self.setting("USERNAME_KEY", self.USERNAME_KEY) | |||
# populate response with id_token if needed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# populate response with id_token if needed |
return { | ||
"username": response.get(username_key), | ||
"email": response.get("email"), | ||
"fullname": response.get("name"), | ||
"first_name": response.get("given_name"), | ||
"last_name": response.get("family_name"), | ||
"username": user_details.get(username_key), | ||
"email": user_details.get("email"), | ||
"fullname": user_details.get("name"), | ||
"first_name": user_details.get("given_name"), | ||
"last_name": user_details.get("family_name"), | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't just return user_details
work the same? It has all the keys now.
Dear developpers,
When using the OpenIdConnectAuth class with a ADFS IdP I got an issue because the email and
USERNAME_KEY
was not part of the response but part of the id_token.The changes I made propose to use the info from the id_token if the key is not present in the response.
I don't know if this makes sense or if this should be a new Class.
I don't know how to make a test for this.