callback_handler.go: simplify stored ID token claims

Fosite is gonna set these fields for us.

Signed-off-by: Ryan Richard <richardry@vmware.com>
This commit is contained in:
Andrew Keesler 2020-11-20 15:36:51 -05:00 committed by Ryan Richard
parent 488d1b663a
commit 541019eb98
No known key found for this signature in database
GPG Key ID: 27CE0444346F9413
5 changed files with 26 additions and 34 deletions

View File

@ -125,7 +125,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
oauthStore := oidc.NullStorage{} oauthStore := oidc.NullStorage{}
hmacSecret := []byte("some secret - must have at least 32 bytes") hmacSecret := []byte("some secret - must have at least 32 bytes")
require.GreaterOrEqual(t, len(hmacSecret), 32, "fosite requires that hmac secrets have at least 32 bytes") require.GreaterOrEqual(t, len(hmacSecret), 32, "fosite requires that hmac secrets have at least 32 bytes")
oauthHelper := oidc.FositeOauth2Helper(oauthStore, hmacSecret) oauthHelper := oidc.FositeOauth2Helper(oauthStore, downstreamIssuer, hmacSecret)
happyCSRF := "test-csrf" happyCSRF := "test-csrf"
happyPKCE := "test-pkce" happyPKCE := "test-pkce"

View File

@ -36,13 +36,9 @@ const (
// downstreamGroupsClaim is what we will use to encode the groups in the downstream OIDC ID token // downstreamGroupsClaim is what we will use to encode the groups in the downstream OIDC ID token
// information. // information.
downstreamGroupsClaim = "groups" downstreamGroupsClaim = "groups"
// The lifetime of an issued downstream ID token.
downstreamIDTokenLifetime = time.Minute * 5
) )
func NewHandler( func NewHandler(
downstreamIssuer string,
idpListGetter oidc.IDPListGetter, idpListGetter oidc.IDPListGetter,
oauthHelper fosite.OAuth2Provider, oauthHelper fosite.OAuth2Provider,
stateDecoder, cookieDecoder oidc.Decoder, stateDecoder, cookieDecoder oidc.Decoder,
@ -97,13 +93,7 @@ func NewHandler(
return err return err
} }
openIDSession := makeDownstreamSession( openIDSession := makeDownstreamSession(username, groups)
downstreamIssuer,
downstreamAuthParams.Get("client_id"),
downstreamAuthParams.Get("nonce"),
username,
groups,
)
authorizeResponder, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, openIDSession) authorizeResponder, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, openIDSession)
if err != nil { if err != nil {
plog.WarningErr("error while generating and saving authcode", err, "upstreamName", upstreamIDPConfig.GetName()) plog.WarningErr("error while generating and saving authcode", err, "upstreamName", upstreamIDPConfig.GetName())
@ -297,16 +287,11 @@ func getGroupsFromUpstreamIDToken(
return groups, nil return groups, nil
} }
func makeDownstreamSession(issuer, clientID, nonce, username string, groups []string) *openid.DefaultSession { func makeDownstreamSession(username string, groups []string) *openid.DefaultSession {
now := time.Now() now := time.Now().UTC()
openIDSession := &openid.DefaultSession{ openIDSession := &openid.DefaultSession{
Claims: &jwt.IDTokenClaims{ Claims: &jwt.IDTokenClaims{
Issuer: issuer,
Subject: username, Subject: username,
Audience: []string{clientID},
Nonce: nonce,
ExpiresAt: now.Add(downstreamIDTokenLifetime),
IssuedAt: now,
RequestedAt: now, RequestedAt: now,
AuthTime: now, AuthTime: now,
}, },

View File

@ -430,10 +430,10 @@ func TestCallbackEndpoint(t *testing.T) {
} }
hmacSecret := []byte("some secret - must have at least 32 bytes") hmacSecret := []byte("some secret - must have at least 32 bytes")
require.GreaterOrEqual(t, len(hmacSecret), 32, "fosite requires that hmac secrets have at least 32 bytes") require.GreaterOrEqual(t, len(hmacSecret), 32, "fosite requires that hmac secrets have at least 32 bytes")
oauthHelper := oidc.FositeOauth2Helper(oauthStore, hmacSecret) oauthHelper := oidc.FositeOauth2Helper(oauthStore, downstreamIssuer, hmacSecret)
idpListGetter := oidctestutil.NewIDPListGetter(&test.idp) idpListGetter := oidctestutil.NewIDPListGetter(&test.idp)
subject := NewHandler(downstreamIssuer, idpListGetter, oauthHelper, happyStateCodec, happyCookieCodec) subject := NewHandler(idpListGetter, oauthHelper, happyStateCodec, happyCookieCodec)
req := httptest.NewRequest(test.method, test.path, nil) req := httptest.NewRequest(test.method, test.path, nil)
if test.csrfCookie != "" { if test.csrfCookie != "" {
req.Header.Set("Cookie", test.csrfCookie) req.Header.Set("Cookie", test.csrfCookie)
@ -480,7 +480,6 @@ func TestCallbackEndpoint(t *testing.T) {
test.wantDownstreamIDTokenSubject, test.wantDownstreamIDTokenSubject,
test.wantDownstreamIDTokenGroups, test.wantDownstreamIDTokenGroups,
test.wantDownstreamRequestedScopes, test.wantDownstreamRequestedScopes,
test.wantDownstreamNonce,
) )
validatePKCEStorage( validatePKCEStorage(
@ -688,7 +687,6 @@ func validateAuthcodeStorage(
wantDownstreamIDTokenSubject string, wantDownstreamIDTokenSubject string,
wantDownstreamIDTokenGroups []string, wantDownstreamIDTokenGroups []string,
wantDownstreamRequestedScopes []string, wantDownstreamRequestedScopes []string,
wantDownstreamNonce string,
) (*fosite.Request, *openid.DefaultSession) { ) (*fosite.Request, *openid.DefaultSession) {
t.Helper() t.Helper()
@ -740,14 +738,22 @@ func validateAuthcodeStorage(
require.NotContains(t, actualClaims.Extra, "groups") require.NotContains(t, actualClaims.Extra, "groups")
} }
// Check the rest of the downstream ID token's claims. // Check the rest of the downstream ID token's claims. Fosite wants us to set these (in UTC time).
require.Equal(t, downstreamIssuer, actualClaims.Issuer) testutil.RequireTimeInDelta(t, time.Now().UTC(), actualClaims.RequestedAt, timeComparisonFudgeFactor)
require.Equal(t, []string{downstreamClientID}, actualClaims.Audience) testutil.RequireTimeInDelta(t, time.Now().UTC(), actualClaims.AuthTime, timeComparisonFudgeFactor)
require.Equal(t, wantDownstreamNonce, actualClaims.Nonce) requestedAtZone, _ := actualClaims.RequestedAt.Zone()
testutil.RequireTimeInDelta(t, time.Now().Add(time.Minute*5), actualClaims.ExpiresAt, timeComparisonFudgeFactor) require.Equal(t, "UTC", requestedAtZone)
testutil.RequireTimeInDelta(t, time.Now(), actualClaims.IssuedAt, timeComparisonFudgeFactor) authTimeZone, _ := actualClaims.AuthTime.Zone()
testutil.RequireTimeInDelta(t, time.Now(), actualClaims.RequestedAt, timeComparisonFudgeFactor) require.Equal(t, "UTC", authTimeZone)
testutil.RequireTimeInDelta(t, time.Now(), actualClaims.AuthTime, timeComparisonFudgeFactor)
// Fosite will set these fields for us in the token endpoint based on the store session
// information. Therefore, we assert that they are empty because we want the library to do the
// lifting for us.
require.Empty(t, actualClaims.Issuer)
require.Nil(t, actualClaims.Audience)
require.Empty(t, actualClaims.Nonce)
require.Zero(t, actualClaims.ExpiresAt)
require.Zero(t, actualClaims.IssuedAt)
// These are not needed yet. // These are not needed yet.
require.Empty(t, actualClaims.JTI) require.Empty(t, actualClaims.JTI)

View File

@ -85,9 +85,10 @@ func PinnipedCLIOIDCClient() *fosite.DefaultOpenIDConnectClient {
} }
} }
func FositeOauth2Helper(oauthStore interface{}, hmacSecretOfLengthAtLeast32 []byte) fosite.OAuth2Provider { func FositeOauth2Helper(oauthStore interface{}, issuer string, hmacSecretOfLengthAtLeast32 []byte) fosite.OAuth2Provider {
oauthConfig := &compose.Config{ oauthConfig := &compose.Config{
EnforcePKCEForPublicClients: true, EnforcePKCEForPublicClients: true,
IDTokenIssuer: issuer,
} }
return compose.Compose( return compose.Compose(

View File

@ -71,7 +71,7 @@ func (m *Manager) SetProviders(oidcProviders ...*provider.OIDCProvider) {
// Use NullStorage for the authorize endpoint because we do not actually want to store anything until // Use NullStorage for the authorize endpoint because we do not actually want to store anything until
// the upstream callback endpoint is called later. // the upstream callback endpoint is called later.
oauthHelper := oidc.FositeOauth2Helper(oidc.NullStorage{}, []byte("some secret - must have at least 32 bytes")) // TODO replace this secret oauthHelper := oidc.FositeOauth2Helper(oidc.NullStorage{}, incomingProvider.Issuer(), []byte("some secret - must have at least 32 bytes")) // TODO replace this secret
// TODO use different codecs for the state and the cookie, because: // TODO use different codecs for the state and the cookie, because:
// 1. we would like to state to have an embedded expiration date while the cookie does not need that // 1. we would like to state to have an embedded expiration date while the cookie does not need that
@ -86,7 +86,7 @@ func (m *Manager) SetProviders(oidcProviders ...*provider.OIDCProvider) {
m.providerHandlers[authURL] = auth.NewHandler(incomingProvider.Issuer(), m.idpListGetter, oauthHelper, csrftoken.Generate, pkce.Generate, nonce.Generate, encoder, encoder) m.providerHandlers[authURL] = auth.NewHandler(incomingProvider.Issuer(), m.idpListGetter, oauthHelper, csrftoken.Generate, pkce.Generate, nonce.Generate, encoder, encoder)
callbackURL := strings.ToLower(incomingProvider.IssuerHost()) + "/" + incomingProvider.IssuerPath() + oidc.CallbackEndpointPath callbackURL := strings.ToLower(incomingProvider.IssuerHost()) + "/" + incomingProvider.IssuerPath() + oidc.CallbackEndpointPath
m.providerHandlers[callbackURL] = callback.NewHandler(incomingProvider.Issuer(), m.idpListGetter, oauthHelper, encoder, encoder) m.providerHandlers[callbackURL] = callback.NewHandler(m.idpListGetter, oauthHelper, encoder, encoder)
plog.Debug("oidc provider manager added or updated issuer", "issuer", incomingProvider.Issuer()) plog.Debug("oidc provider manager added or updated issuer", "issuer", incomingProvider.Issuer())
} }