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

Ensure OAuth falls back to OpenID userinfo if id_token is not sufficient #5939

Merged
merged 2 commits into from
Nov 29, 2023

Conversation

philippjfr
Copy link
Member

@philippjfr philippjfr commented Nov 29, 2023

Before refactoring the OAuth handlers we would always either obtain our user info via the id_token OR via the OpenID userinfo endpoint. After the refactor we ended up skipping the request to the userinfo endpoint if a valid id_token was returned by the token endpoint. However in many cases that id_token does not return all the info needed to verify the user. Therefore we now check whether the id_token contains the required user key and if it doesn't we fall back to making the request to the userinfo endpoint.

Fixes #5897

panel/auth.py Outdated Show resolved Hide resolved
Copy link

codecov bot commented Nov 29, 2023

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (39ec575) 82.41% compared to head (bba5fb9) 84.24%.
Report is 2 commits behind head on main.

Files Patch % Lines
panel/auth.py 44.44% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5939      +/-   ##
==========================================
+ Coverage   82.41%   84.24%   +1.83%     
==========================================
  Files         291      291              
  Lines       43055    43072      +17     
==========================================
+ Hits        35485    36288     +803     
+ Misses       7570     6784     -786     
Flag Coverage Δ
ui-tests 41.06% <44.44%> (+2.45%) ⬆️
unitexamples-tests 72.23% <0.00%> (+0.28%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@philippjfr philippjfr merged commit cc9400a into main Nov 29, 2023
13 checks passed
@philippjfr philippjfr deleted the id_token_fallback branch November 29, 2023 18:10
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.

Logging in with Generic Provider in V1.3+
1 participant