Skip to content

Refactor OIDC profileURL call to EnrichSessionState #883

Closed
@NickMeves

Description

@NickMeves

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:

// BearerToken case, allow empty UserID

This is because EnrichSessionState is only called in the Callback flow after the Redeem method.

Activity

JoelSpeed

JoelSpeed commented on Nov 4, 2020

@JoelSpeed
Member

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

NickMeves commented on Nov 4, 2020

@NickMeves
ContributorAuthor

If any claims are missing from the IDToken (email & potentiall groups in the future) -- OIDC provider attempts to get it from the profileURL here:

profileURL := p.ProfileURL.String()

We are migrating this logic in the other providers to EnrichSessionState (a lot of them previously had it in GetEmailAddress). 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

NickMeves commented on Nov 4, 2020

@NickMeves
ContributorAuthor

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

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 initial Redeem method during initial sign in.

anannaya

anannaya commented on Nov 9, 2020

@anannaya

Hopefully #850 (comment) changes will be part of this issue.

github-actions

github-actions commented on Jan 9, 2021

@github-actions
Contributor

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

NickMeves commented on Jan 9, 2021

@NickMeves
ContributorAuthor

This is done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      Refactor OIDC `profileURL` call to `EnrichSessionState` · Issue #883 · oauth2-proxy/oauth2-proxy