From a47617cad07d43462afed231b3770b40971022c5 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Thu, 19 Nov 2020 08:53:53 -0800 Subject: [PATCH] callback_handler.go: Add JWT Audience claim to storage --- internal/oidc/callback/callback_handler.go | 10 ++++++--- .../oidc/callback/callback_handler_test.go | 21 ++++++++++++------- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/internal/oidc/callback/callback_handler.go b/internal/oidc/callback/callback_handler.go index 009fa737a..a2521488a 100644 --- a/internal/oidc/callback/callback_handler.go +++ b/internal/oidc/callback/callback_handler.go @@ -72,7 +72,7 @@ func NewHandler( _, idTokenClaims, err := upstreamIDPConfig.ExchangeAuthcodeAndValidateTokens( r.Context(), - r.URL.Query().Get("code"), // TODO: do we need to validate this? + authcode(r), state.PKCECode, state.Nonce, ) @@ -113,7 +113,7 @@ func NewHandler( Claims: &jwt.IDTokenClaims{ Issuer: downstreamIssuer, Subject: username, - Audience: []string{"my-client"}, // TODO use the right value here + Audience: []string{downstreamAuthParams.Get("client_id")}, ExpiresAt: now.Add(time.Minute * 30), // TODO use the right value here IssuedAt: now, // TODO test this RequestedAt: now, // TODO test this @@ -133,6 +133,10 @@ func NewHandler( }) } +func authcode(r *http.Request) string { + return r.FormValue("code") +} + func validateRequest(r *http.Request, stateDecoder, cookieDecoder oidc.Decoder) (*oidc.UpstreamStateParamData, error) { if r.Method != http.MethodGet { return nil, httperr.Newf(http.StatusMethodNotAllowed, "%s (try GET)", r.Method) @@ -144,7 +148,7 @@ func validateRequest(r *http.Request, stateDecoder, cookieDecoder oidc.Decoder) return nil, err } - if r.FormValue("code") == "" { + if authcode(r) == "" { plog.Info("code param not found") return nil, httperr.New(http.StatusBadRequest, "code param not found") } diff --git a/internal/oidc/callback/callback_handler_test.go b/internal/oidc/callback/callback_handler_test.go index 07fc7b18b..a37133718 100644 --- a/internal/oidc/callback/callback_handler_test.go +++ b/internal/oidc/callback/callback_handler_test.go @@ -36,6 +36,12 @@ func TestCallbackEndpoint(t *testing.T) { downstreamIssuer = "https://my-downstream-issuer.com/path" downstreamRedirectURI = "http://127.0.0.1/callback" happyUpstreamAuthcode = "upstream-auth-code" + upstreamUsername = "test-pinniped-username" + downstreamClientID = "pinniped-cli" + ) + + var ( + upstreamGroupMembership = []string{"test-pinniped-group-0", "test-pinniped-group-1"} ) upstreamOIDCIdentityProvider := testutil.TestUpstreamOIDCIdentityProvider{ @@ -47,8 +53,8 @@ func TestCallbackEndpoint(t *testing.T) { ExchangeAuthcodeAndValidateTokensFunc: func(ctx context.Context, authcode string, pkceCodeVerifier pkce.Code, expectedIDTokenNonce nonce.Nonce) (oidcclient.Token, map[string]interface{}, error) { return oidcclient.Token{}, map[string]interface{}{ - "the-user-claim": "test-pinniped-username", - "the-groups-claim": []string{"test-pinniped-group-0", "test-pinniped-group-1"}, + "the-user-claim": upstreamUsername, + "the-groups-claim": upstreamGroupMembership, "other-claim": "should be ignored", }, nil @@ -62,8 +68,8 @@ func TestCallbackEndpoint(t *testing.T) { ExchangeAuthcodeAndValidateTokensFunc: func(ctx context.Context, authcode string, pkceCodeVerifier pkce.Code, expectedIDTokenNonce nonce.Nonce) (oidcclient.Token, map[string]interface{}, error) { return oidcclient.Token{}, map[string]interface{}{ - "sub": "test-pinniped-username", - "groups": []string{"test-pinniped-group-0", "test-pinniped-group-1"}, + "sub": upstreamUsername, + "groups": upstreamGroupMembership, "other-claim": "should be ignored", }, nil @@ -104,7 +110,7 @@ func TestCallbackEndpoint(t *testing.T) { happyOriginalRequestParamsQuery := url.Values{ "response_type": []string{"code"}, "scope": []string{"openid profile email"}, - "client_id": []string{"pinniped-cli"}, + "client_id": []string{downstreamClientID}, "state": []string{happyDownstreamState}, "nonce": []string{"some-nonce-value"}, "code_challenge": []string{"some-challenge"}, @@ -451,8 +457,9 @@ func TestCallbackEndpoint(t *testing.T) { require.NotContains(t, storedRequest.GetGrantedScopes(), "openid") } require.Equal(t, downstreamIssuer, storedSession.Claims.Issuer) - require.Equal(t, "test-pinniped-username", storedSession.Claims.Subject) - require.Equal(t, []string{"test-pinniped-group-0", "test-pinniped-group-1"}, storedSession.Claims.Extra["oidc.pinniped.dev/groups"]) + require.Equal(t, upstreamUsername, storedSession.Claims.Subject) + require.Equal(t, []string{downstreamClientID}, storedSession.Claims.Audience) + require.Equal(t, upstreamGroupMembership, storedSession.Claims.Extra["oidc.pinniped.dev/groups"]) } else { require.Empty(t, rsp.Header().Values("Location")) }