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

Distributed claims utilities #171

Open
ericchiang opened this issue May 24, 2018 · 4 comments
Open

Distributed claims utilities #171

ericchiang opened this issue May 24, 2018 · 4 comments

Comments

@ericchiang
Copy link
Collaborator

ericchiang commented May 24, 2018

http://openid.net/specs/openid-connect-core-1_0.html#AggregatedDistributedClaims

ref kubernetes/kubernetes#63213

Consider adding some sort of distributed claim resolver. Maybe another field on the IDToken?

type IDToken struct { }

// Stays the same, doesn't do any resolution
func (t *IDToken) Claims(into interface{}) error {}

// If the named claim is a distributed claim, this will be resolved and unmarshalled into "v"
func (t *IDToken) Claim(name string, v interface{}) error {}

Claim expansion could be done when calling IDToken.Claim or could be built into IDTokenVerifier.Verify

cc @filmil for any thoughts here

@ericchiang
Copy link
Collaborator Author

cc @sean-q-sun

@filmil
Copy link

filmil commented May 29, 2018

Hi folks. I was away from keyboard for a few days, hence the late response.

This certainly seems like an useful addition to have out of the box.

A few questions come to mind:

  • What is the roll-out strategy for the distributed claims? Should they become available to the users out of the box? (I mean, what is the incentive to start using it if it requires a code change?)
  • Would we also want to support aggregated claims on the library level? If yes, is there a benefit to keeping the structure of the claim somehow visible (e.g. if there are conflicting claims, do we expose all of them and how)
  • I think that the OIDC spec is somewhat unclear about the structure of the distributed claim response. E.g. it seems that while the response to the claim must be a JWT, it says nothing about whether it may be wrapped into something.

Hope this helps, even if belated, as you've already started working on it.

Cheers,
F

@ericchiang
Copy link
Collaborator Author

ericchiang commented May 29, 2018

Hi folks. I was away from keyboard for a few days, hence the late response.

Hope you enjoyed your time off!

What is the roll-out strategy for the distributed claims? Should they become available to the users out of the box? (I mean, what is the incentive to start using it if it requires a code change?)

The proposed change is to keep all the existing behavior the same, but introduce a new method Claim to the IDToken struct:

// Claim unmarshals the value of the named claim into v.
//
//    var email string
//    if err := idToken.Claim("email", &email); err != nil {
//        // handle error
//    }
//
// If the named claims is a distributed claim, Claim queries the remote endpoint to
// resolve the claim value.
//
// See: https://openid.net/specs/openid-connect-core-1_0.html#DistributedExample
func (i *IDToken) Claim(name string, v interface{}) error

We'd also add something to the verifier config:

type Config struct {
    // Existing fields...

    // GetVerifier, if not nil, is called when attempting to verify JWTs returned by a
    // distributed claim. If GetVerifier returns an error, the JWT is rejected.
    //
    // If nil, the verifier will be initialized using HTTP discovery and passed the same
    // context and config as the existing verifier.
    GetVerifier func(issuerURL string) (*IDTokenVerifier, error)
}

Would we also want to support aggregated claims on the library level? If yes, is there a benefit to keeping the structure of the claim somehow visible (e.g. if there are conflicting claims, do we expose all of them and how)

I'd like to hold off on aggregated claims until we have a use case. The existing Claims method will also continue to unmarshal the unmodified claims payload, so users still have the flexibility to do custom stuff.

I think that the OIDC spec is somewhat unclear about the structure of the distributed claim response. E.g. it seems that while the response to the claim must be a JWT, it says nothing about whether it may be wrapped into something.

Yeah... I guess we'll start with the strictest verification then loosen our requirements as someone asks for it.

@universam1
Copy link

universam1 commented Feb 18, 2022

Yeah... I guess we'll start with the strictest verification then loosen our requirements as someone asks for it.

Hi - we are facing the distributed claim with Azure AD -> zalando/skipper#1955

The map distributedClaims would be very handy for further Graph calls, however it is not exported. Any other access available?
Thanks!

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

No branches or pull requests

3 participants