callback_handler.go: assert nonce is stored correctly

I think we want to do this here since we are storing all of the
other ID token claims?

Signed-off-by: Andrew Keesler <akeesler@vmware.com>

internal/oidc/callback/callback_handler.go

@@ -97,7 +97,13 @@ func NewHandler(
 			return err
 		}
 
-		openIDSession := makeDownstreamSession(downstreamIssuer, downstreamAuthParams.Get("client_id"), username, groups)
+		openIDSession := makeDownstreamSession(
+			downstreamIssuer,
+			downstreamAuthParams.Get("client_id"),
+			downstreamAuthParams.Get("nonce"),
+			username,
+			groups,
+		)
 		authorizeResponder, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, openIDSession)
 		if err != nil {
 			plog.WarningErr("error while generating and saving authcode", err, "upstreamName", upstreamIDPConfig.GetName())
@@ -291,7 +297,7 @@ func getGroupsFromUpstreamIDToken(
 	return groups, nil
 }
 
-func makeDownstreamSession(issuer, clientID, username string, groups []string) *openid.DefaultSession {
+func makeDownstreamSession(issuer, clientID, nonce, username string, groups []string) *openid.DefaultSession {
 	now := time.Now()
 	openIDSession := &openid.DefaultSession{
 		Claims: &jwt.IDTokenClaims{
@@ -302,6 +308,7 @@ func makeDownstreamSession(issuer, clientID, username string, groups []string) *
 			IssuedAt:    now,
 			RequestedAt: now,
 			AuthTime:    now,
+			Nonce:       nonce,
 		},
 	}
 	if groups != nil {

internal/oidc/callback/callback_handler_test.go

@@ -49,6 +49,7 @@ const (
 	happyUpstreamAuthcode = "upstream-auth-code"
 	downstreamRedirectURI = "http://127.0.0.1/callback"
 	downstreamClientID    = "pinniped-cli"
+	downstreamNonce       = "some-nonce-value"
 
 	timeComparisonFudgeFactor = time.Second * 15
 )
@@ -62,7 +63,7 @@ var (
 		"scope":                 []string{strings.Join(happyDownstreamScopesRequested, " ")},
 		"client_id":             []string{downstreamClientID},
 		"state":                 []string{happyDownstreamState},
-		"nonce":                 []string{"some-nonce-value"},
+		"nonce":                 []string{downstreamNonce},
 		"code_challenge":        []string{"some-challenge"},
 		"code_challenge_method": []string{"S256"},
 		"redirect_uri":          []string{downstreamRedirectURI},
@@ -119,6 +120,7 @@ func TestCallbackEndpoint(t *testing.T) {
 		wantDownstreamIDTokenSubject  string
 		wantDownstreamIDTokenGroups   []string
 		wantDownstreamRequestedScopes []string
+		wantDownstreamNonce           string
 
 		wantExchangeAndValidateTokensCall *oidctestutil.ExchangeAuthcodeAndValidateTokenArgs
 	}{
@@ -135,6 +137,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantDownstreamIDTokenSubject:      upstreamUsername,
 			wantDownstreamIDTokenGroups:       upstreamGroupMembership,
 			wantDownstreamRequestedScopes:     happyDownstreamScopesRequested,
+			wantDownstreamNonce:               downstreamNonce,
 			wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs,
 		},
 		{
@@ -150,6 +153,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantDownstreamIDTokenSubject:      upstreamIssuer + "?sub=" + upstreamSubject,
 			wantDownstreamIDTokenGroups:       nil,
 			wantDownstreamRequestedScopes:     happyDownstreamScopesRequested,
+			wantDownstreamNonce:               downstreamNonce,
 			wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs,
 		},
 		{
@@ -165,6 +169,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantDownstreamIDTokenSubject:      upstreamSubject,
 			wantDownstreamIDTokenGroups:       upstreamGroupMembership,
 			wantDownstreamRequestedScopes:     happyDownstreamScopesRequested,
+			wantDownstreamNonce:               downstreamNonce,
 			wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs,
 		},
 
@@ -287,6 +292,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantDownstreamIDTokenSubject:      upstreamUsername,
 			wantDownstreamRequestedScopes:     []string{"profile", "email"},
 			wantDownstreamIDTokenGroups:       upstreamGroupMembership,
+			wantDownstreamNonce:               downstreamNonce,
 			wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs,
 		},
 		{
@@ -507,6 +513,7 @@ func TestCallbackEndpoint(t *testing.T) {
 				// Check the rest of the downstream ID token's claims.
 				require.Equal(t, downstreamIssuer, actualClaims.Issuer)
 				require.Equal(t, []string{downstreamClientID}, actualClaims.Audience)
+				require.Equal(t, test.wantDownstreamNonce, actualClaims.Nonce)
 				testutil.RequireTimeInDelta(t, time.Now().Add(time.Minute*5), actualClaims.ExpiresAt, timeComparisonFudgeFactor)
 				testutil.RequireTimeInDelta(t, time.Now(), actualClaims.IssuedAt, timeComparisonFudgeFactor)
 				testutil.RequireTimeInDelta(t, time.Now(), actualClaims.RequestedAt, timeComparisonFudgeFactor)
@@ -519,9 +526,6 @@ func TestCallbackEndpoint(t *testing.T) {
 				require.Empty(t, actualClaims.AuthenticationContextClassReference)
 				require.Empty(t, actualClaims.AuthenticationMethodsReference)
 
-				// TODO we should put the downstream request's nonce into the ID token, but maybe the token endpoint is responsible for that?
-				require.Empty(t, actualClaims.Nonce)
-
 				// TODO add thorough tests about what should be stored for PKCES and IDSessions
 			} else {
 				require.Empty(t, rsp.Header().Values("Location"))