Closed
Description
Currently a call to the profileURL
is embedded inside of the findClaimsFromIDToken
as a fallback in case the claims are missing.
This logic doesn't belong there -- it should be implemented in EnrichSessionState
to be more uniform with other providers in how they do secondary calls with an Access Token to enrich the session with more data.
This will additionally remove the need for this corner case logic for bearer sessions (since bearer sessions don't have an Access Token to use) here:
oauth2-proxy/providers/oidc.go
Line 247 in 3fa42ed
This is because EnrichSessionState
is only called in the Callback
flow after the Redeem
method.
Activity
JoelSpeed commentedon Nov 4, 2020
What do we use the
profileURL
for at the moment? IIRC it's used to try and find a claim if the ID Token doesn't contain the claim right?Is there some way we can make a generic claim extraction that tries to get a claim and if not falls back to the profile URL? But do so in a way that the profile URL is called at most once 🤔 This is something I was thinking about for when we support any additional claim users want us to try and look up, we will need some logic like this I would imagine
NickMeves commentedon Nov 4, 2020
If any claims are missing from the IDToken (email & potentiall groups in the future) -- OIDC provider attempts to get it from the
profileURL
here:oauth2-proxy/providers/oidc.go
Line 257 in 65016c8
We are migrating this logic in the other providers to
EnrichSessionState
(a lot of them previously had it inGetEmailAddress
). Makes sense to do the same in OIDC.Also helps reduce the complexity of the logic where we try to figure out how
findClaimsFromIDToken
is called (since in the Bearer auth case the function is also used, we do not want call the profileURL, we take what we get)NickMeves commentedon Nov 4, 2020
Forgot to address this part: This is the benefit of moving the logic to
EnrichSessionState
, it is only called at the moment immediately after the initialRedeem
method during initial sign in.anannaya commentedon Nov 9, 2020
Hopefully #850 (comment) changes will be part of this issue.
github-actions commentedon Jan 9, 2021
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.
NickMeves commentedon Jan 9, 2021
This is done.
feat: higress global configmap support config route timeout (oauth2-p…