Always add the `azp` claim to ID tokens to show the original client ID

When the token exchange grant type is used to get a cluster-scoped
ID token, the returned token has a new audience value. The client ID
of the client which performed the authorization was lost. This didn't
matter before, since the only client was `pinniped-cli`, but now that
dynamic clients can be registered, the information would be lost in the
cluster-scoped ID token. It could be useful for logging, tracing, or
auditing, so preserve the information by putting the client ID into the
`azp` claim in every ID token (authcode exchange, clsuter-scoped, and
refreshed ID tokens).
This commit is contained in:
Ryan Richard 2022-08-09 16:07:23 -07:00
parent 8a5db99abf
commit 0bb2c7beb7
7 changed files with 50 additions and 16 deletions

View File

@ -149,7 +149,8 @@ func handleAuthRequestForLDAPUpstreamCLIFlow(
username = authenticateResponse.User.GetName() username = authenticateResponse.User.GetName()
groups := authenticateResponse.User.GetGroups() groups := authenticateResponse.User.GetGroups()
customSessionData := downstreamsession.MakeDownstreamLDAPOrADCustomSessionData(ldapUpstream, idpType, authenticateResponse, username) customSessionData := downstreamsession.MakeDownstreamLDAPOrADCustomSessionData(ldapUpstream, idpType, authenticateResponse, username)
openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups, authorizeRequester.GetGrantedScopes(), customSessionData) openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups,
authorizeRequester.GetGrantedScopes(), authorizeRequester.GetClient().GetID(), customSessionData)
oidc.PerformAuthcodeRedirect(r, w, oauthHelper, authorizeRequester, openIDSession, true) oidc.PerformAuthcodeRedirect(r, w, oauthHelper, authorizeRequester, openIDSession, true)
return nil return nil
@ -250,7 +251,8 @@ func handleAuthRequestForOIDCUpstreamPasswordGrant(
return nil return nil
} }
openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups, authorizeRequester.GetGrantedScopes(), customSessionData) openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups,
authorizeRequester.GetGrantedScopes(), authorizeRequester.GetClient().GetID(), customSessionData)
oidc.PerformAuthcodeRedirect(r, w, oauthHelper, authorizeRequester, openIDSession, true) oidc.PerformAuthcodeRedirect(r, w, oauthHelper, authorizeRequester, openIDSession, true)

View File

@ -79,7 +79,8 @@ func NewHandler(
return httperr.Wrap(http.StatusUnprocessableEntity, err.Error(), err) return httperr.Wrap(http.StatusUnprocessableEntity, err.Error(), err)
} }
openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups, authorizeRequester.GetGrantedScopes(), customSessionData) openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups,
authorizeRequester.GetGrantedScopes(), authorizeRequester.GetClient().GetID(), customSessionData)
authorizeResponder, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, openIDSession) authorizeResponder, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, openIDSession)
if err != nil { if err != nil {

View File

@ -41,7 +41,14 @@ const (
) )
// MakeDownstreamSession creates a downstream OIDC session. // MakeDownstreamSession creates a downstream OIDC session.
func MakeDownstreamSession(subject string, username string, groups []string, grantedScopes []string, custom *psession.CustomSessionData) *psession.PinnipedSession { func MakeDownstreamSession(
subject string,
username string,
groups []string,
grantedScopes []string,
clientID string,
custom *psession.CustomSessionData,
) *psession.PinnipedSession {
now := time.Now().UTC() now := time.Now().UTC()
openIDSession := &psession.PinnipedSession{ openIDSession := &psession.PinnipedSession{
Fosite: &openid.DefaultSession{ Fosite: &openid.DefaultSession{
@ -56,13 +63,17 @@ func MakeDownstreamSession(subject string, username string, groups []string, gra
if groups == nil { if groups == nil {
groups = []string{} groups = []string{}
} }
openIDSession.IDTokenClaims().Extra = map[string]interface{}{}
extras := map[string]interface{}{}
extras[oidcapi.IDTokenClaimAuthorizedParty] = clientID
if slices.Contains(grantedScopes, oidcapi.ScopeUsername) { if slices.Contains(grantedScopes, oidcapi.ScopeUsername) {
openIDSession.IDTokenClaims().Extra[oidcapi.IDTokenClaimUsername] = username extras[oidcapi.IDTokenClaimUsername] = username
} }
if slices.Contains(grantedScopes, oidcapi.ScopeGroups) { if slices.Contains(grantedScopes, oidcapi.ScopeGroups) {
openIDSession.IDTokenClaims().Extra[oidcapi.IDTokenClaimGroups] = groups extras[oidcapi.IDTokenClaimGroups] = groups
} }
openIDSession.IDTokenClaims().Extra = extras
return openIDSession return openIDSession
} }

View File

@ -83,7 +83,8 @@ func NewPostHandler(issuerURL string, upstreamIDPs oidc.UpstreamIdentityProvider
username = authenticateResponse.User.GetName() username = authenticateResponse.User.GetName()
groups := authenticateResponse.User.GetGroups() groups := authenticateResponse.User.GetGroups()
customSessionData := downstreamsession.MakeDownstreamLDAPOrADCustomSessionData(ldapUpstream, idpType, authenticateResponse, username) customSessionData := downstreamsession.MakeDownstreamLDAPOrADCustomSessionData(ldapUpstream, idpType, authenticateResponse, username)
openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups, authorizeRequester.GetGrantedScopes(), customSessionData) openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups,
authorizeRequester.GetGrantedScopes(), authorizeRequester.GetClient().GetID(), customSessionData)
oidc.PerformAuthcodeRedirect(r, w, oauthHelper, authorizeRequester, openIDSession, false) oidc.PerformAuthcodeRedirect(r, w, oauthHelper, authorizeRequester, openIDSession, false)
return nil return nil

View File

@ -1398,7 +1398,7 @@ func TestTokenEndpointTokenExchange(t *testing.T) { // tests for grant_type "urn
require.NoError(t, json.Unmarshal(parsedJWT.UnsafePayloadWithoutVerification(), &tokenClaims)) require.NoError(t, json.Unmarshal(parsedJWT.UnsafePayloadWithoutVerification(), &tokenClaims))
// Make sure that these are the only fields in the token. // Make sure that these are the only fields in the token.
idTokenFields := []string{"sub", "aud", "iss", "jti", "auth_time", "exp", "iat", "rat", "username"} idTokenFields := []string{"sub", "aud", "iss", "jti", "auth_time", "exp", "iat", "rat", "username", "azp"}
if test.authcodeExchange.want.wantGroups != nil { if test.authcodeExchange.want.wantGroups != nil {
idTokenFields = append(idTokenFields, "groups") idTokenFields = append(idTokenFields, "groups")
} }
@ -1412,6 +1412,7 @@ func TestTokenEndpointTokenExchange(t *testing.T) { // tests for grant_type "urn
require.NotEmpty(t, tokenClaims["rat"]) require.NotEmpty(t, tokenClaims["rat"])
require.Len(t, tokenClaims["aud"], 1) require.Len(t, tokenClaims["aud"], 1)
require.Contains(t, tokenClaims["aud"], test.requestedAudience) require.Contains(t, tokenClaims["aud"], test.requestedAudience)
require.Equal(t, test.authcodeExchange.want.wantClientID, tokenClaims["azp"])
require.Equal(t, goodSubject, tokenClaims["sub"]) require.Equal(t, goodSubject, tokenClaims["sub"])
require.Equal(t, goodIssuer, tokenClaims["iss"]) require.Equal(t, goodIssuer, tokenClaims["iss"])
if test.authcodeExchange.want.wantUsername != "" { if test.authcodeExchange.want.wantUsername != "" {
@ -4027,6 +4028,9 @@ func simulateAuthEndpointHavingAlreadyRun(
session.Fosite.Claims.Extra["groups"] = goodGroups session.Fosite.Claims.Extra["groups"] = goodGroups
} }
// The authorization endpoint sets the authorized party to the client ID of the original requester.
session.Fosite.Claims.Extra["azp"] = authRequester.GetClient().GetID()
authResponder, err := oauthHelper.NewAuthorizeResponse(ctx, authRequester, session) authResponder, err := oauthHelper.NewAuthorizeResponse(ctx, authRequester, session)
require.NoError(t, err) require.NoError(t, err)
return authResponder return authResponder
@ -4291,6 +4295,7 @@ func requireValidStoredRequest(
if wantGroups != nil { if wantGroups != nil {
expectedExtra["groups"] = toSliceOfInterface(wantGroups) expectedExtra["groups"] = toSliceOfInterface(wantGroups)
} }
expectedExtra["azp"] = wantClientID
require.Equal(t, expectedExtra, claims.Extra) require.Equal(t, expectedExtra, claims.Extra)
// We are in charge of setting these fields. For the purpose of testing, we ensure that the // We are in charge of setting these fields. For the purpose of testing, we ensure that the
@ -4412,7 +4417,7 @@ func requireValidIDToken(
// Note that there is a bug in fosite which prevents the `at_hash` claim from appearing in this ID token // Note that there is a bug in fosite which prevents the `at_hash` claim from appearing in this ID token
// during the initial authcode exchange, but does not prevent `at_hash` from appearing in the refreshed ID token. // during the initial authcode exchange, but does not prevent `at_hash` from appearing in the refreshed ID token.
// We can add a workaround for this later. // We can add a workaround for this later.
idTokenFields := []string{"sub", "aud", "iss", "jti", "auth_time", "exp", "iat", "rat"} idTokenFields := []string{"sub", "aud", "iss", "jti", "auth_time", "exp", "iat", "rat", "azp"}
if wantAtHashClaimInIDToken { if wantAtHashClaimInIDToken {
idTokenFields = append(idTokenFields, "at_hash") idTokenFields = append(idTokenFields, "at_hash")
} }
@ -4439,6 +4444,7 @@ func requireValidIDToken(
require.Equal(t, wantGroupsInIDToken, claims.Groups) require.Equal(t, wantGroupsInIDToken, claims.Groups)
require.Len(t, claims.Audience, 1) require.Len(t, claims.Audience, 1)
require.Equal(t, wantClientID, claims.Audience[0]) require.Equal(t, wantClientID, claims.Audience[0])
require.Equal(t, wantClientID, m["azp"])
require.Equal(t, goodIssuer, claims.Issuer) require.Equal(t, goodIssuer, claims.Issuer)
require.NotEmpty(t, claims.JTI) require.NotEmpty(t, claims.JTI)

View File

@ -1062,27 +1062,31 @@ func validateAuthcodeStorage(
// Now confirm the ID token claims. // Now confirm the ID token claims.
actualClaims := storedSessionFromAuthcode.Fosite.Claims actualClaims := storedSessionFromAuthcode.Fosite.Claims
// Should always have an azp claim.
require.Equal(t, wantDownstreamClientID, actualClaims.Extra["azp"])
wantDownstreamIDTokenExtraClaimsCount := 1 // should always have azp claim
// Check the user's identity, which are put into the downstream ID token's subject, username and groups claims. // Check the user's identity, which are put into the downstream ID token's subject, username and groups claims.
require.Equal(t, wantDownstreamIDTokenSubject, actualClaims.Subject) require.Equal(t, wantDownstreamIDTokenSubject, actualClaims.Subject)
wantDownstreamIDTokenUsernameClaimToExist := 1
if wantDownstreamIDTokenUsername == "" { if wantDownstreamIDTokenUsername == "" {
wantDownstreamIDTokenUsernameClaimToExist = 0
require.NotContains(t, actualClaims.Extra, "username") require.NotContains(t, actualClaims.Extra, "username")
} else { } else {
wantDownstreamIDTokenExtraClaimsCount++ // should also have username claim
require.Equal(t, wantDownstreamIDTokenUsername, actualClaims.Extra["username"]) require.Equal(t, wantDownstreamIDTokenUsername, actualClaims.Extra["username"])
} }
if slices.Contains(wantDownstreamGrantedScopes, "groups") { if slices.Contains(wantDownstreamGrantedScopes, "groups") {
require.Len(t, actualClaims.Extra, wantDownstreamIDTokenUsernameClaimToExist+1) wantDownstreamIDTokenExtraClaimsCount++ // should also have groups claim
actualDownstreamIDTokenGroups := actualClaims.Extra["groups"] actualDownstreamIDTokenGroups := actualClaims.Extra["groups"]
require.NotNil(t, actualDownstreamIDTokenGroups) require.NotNil(t, actualDownstreamIDTokenGroups)
require.ElementsMatch(t, wantDownstreamIDTokenGroups, actualDownstreamIDTokenGroups) require.ElementsMatch(t, wantDownstreamIDTokenGroups, actualDownstreamIDTokenGroups)
} else { } else {
require.Emptyf(t, wantDownstreamIDTokenGroups, "test case did not want the groups scope to be granted, "+ require.Emptyf(t, wantDownstreamIDTokenGroups, "test case did not want the groups scope to be granted, "+
"but wanted something in the groups claim, which doesn't make sense. please review the test case's expectations.") "but wanted something in the groups claim, which doesn't make sense. please review the test case's expectations.")
require.Len(t, actualClaims.Extra, wantDownstreamIDTokenUsernameClaimToExist)
actualDownstreamIDTokenGroups := actualClaims.Extra["groups"] actualDownstreamIDTokenGroups := actualClaims.Extra["groups"]
require.Nil(t, actualDownstreamIDTokenGroups) require.Nil(t, actualDownstreamIDTokenGroups)
} }
// Make sure that we asserted on every extra claim.
require.Len(t, actualClaims.Extra, wantDownstreamIDTokenExtraClaimsCount)
// Check the rest of the downstream ID token's claims. Fosite wants us to set these (in UTC time). // Check the rest of the downstream ID token's claims. Fosite wants us to set these (in UTC time).
testutil.RequireTimeInDelta(t, time.Now().UTC(), actualClaims.RequestedAt, timeComparisonFudgeFactor) testutil.RequireTimeInDelta(t, time.Now().UTC(), actualClaims.RequestedAt, timeComparisonFudgeFactor)

View File

@ -1996,7 +1996,7 @@ func testSupervisorLogin(
} }
require.NoError(t, err) require.NoError(t, err)
expectedIDTokenClaims := []string{"iss", "exp", "sub", "aud", "auth_time", "iat", "jti", "nonce", "rat"} expectedIDTokenClaims := []string{"iss", "exp", "sub", "aud", "auth_time", "iat", "jti", "nonce", "rat", "azp"}
if slices.Contains(wantDownstreamScopes, "username") { if slices.Contains(wantDownstreamScopes, "username") {
// If the test wants the username scope to have been granted, then also expect the claim in the ID token. // If the test wants the username scope to have been granted, then also expect the claim in the ID token.
expectedIDTokenClaims = append(expectedIDTokenClaims, "username") expectedIDTokenClaims = append(expectedIDTokenClaims, "username")
@ -2044,7 +2044,7 @@ func testSupervisorLogin(
require.NoError(t, err) require.NoError(t, err)
// When refreshing, expect to get an "at_hash" claim, but no "nonce" claim. // When refreshing, expect to get an "at_hash" claim, but no "nonce" claim.
expectRefreshedIDTokenClaims := []string{"iss", "exp", "sub", "aud", "auth_time", "iat", "jti", "rat", "at_hash"} expectRefreshedIDTokenClaims := []string{"iss", "exp", "sub", "aud", "auth_time", "iat", "jti", "rat", "azp", "at_hash"}
if slices.Contains(wantDownstreamScopes, "username") { if slices.Contains(wantDownstreamScopes, "username") {
// If the test wants the username scope to have been granted, then also expect the claim in the refreshed ID token. // If the test wants the username scope to have been granted, then also expect the claim in the refreshed ID token.
expectRefreshedIDTokenClaims = append(expectRefreshedIDTokenClaims, "username") expectRefreshedIDTokenClaims = append(expectRefreshedIDTokenClaims, "username")
@ -2148,6 +2148,10 @@ func verifyTokenResponse(
} }
require.ElementsMatch(t, expectedIDTokenClaims, idTokenClaimNames) require.ElementsMatch(t, expectedIDTokenClaims, idTokenClaimNames)
// There should always be an "azp" claim, and the value should be the client ID of the client which made
// the authorization request.
require.Equal(t, downstreamOAuth2Config.ClientID, idTokenClaims["azp"])
// Check username claim of the ID token, if one is expected. Asserting on the lack of a username claim is // Check username claim of the ID token, if one is expected. Asserting on the lack of a username claim is
// handled above where the full list of claims are asserted. // handled above where the full list of claims are asserted.
if wantDownstreamIDTokenUsernameToMatch != "" { if wantDownstreamIDTokenUsernameToMatch != "" {
@ -2423,6 +2427,11 @@ func doTokenExchange(
indentedClaims, err := json.MarshalIndent(claims, " ", " ") indentedClaims, err := json.MarshalIndent(claims, " ", " ")
require.NoError(t, err) require.NoError(t, err)
t.Logf("exchanged token claims:\n%s", string(indentedClaims)) t.Logf("exchanged token claims:\n%s", string(indentedClaims))
// The original client ID should be preserved in the azp claim, therefore preserving this information
// about the original source of the authorization for tracing/auditing purposes, since the "aud" claim
// has been updated to have a new value.
require.Equal(t, config.ClientID, claims["azp"])
} }
func expectSecurityHeaders(t *testing.T, response *http.Response, expectFositeToOverrideSome bool) { func expectSecurityHeaders(t *testing.T, response *http.Response, expectFositeToOverrideSome bool) {