From 03806629b80038cb7cbbf82c6404dc1bf347081c Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Fri, 4 Dec 2020 10:06:55 -0500 Subject: [PATCH] Cleanup code via TODOs accumulated during token endpoint work We opened https://github.com/vmware-tanzu/pinniped/issues/254 for the TODO in dynamicOpenIDConnectECDSAStrategy.GenerateToken(). This commit also ensures that linting and unit tests are passing again. Signed-off-by: Andrew Keesler --- internal/oidc/auth/auth_handler.go | 16 +-- internal/oidc/auth/auth_handler_test.go | 26 +--- .../oidc/callback/callback_handler_test.go | 4 +- .../dynamic_open_id_connect_ecdsa_strategy.go | 10 +- ...mic_open_id_connect_ecdsa_strategy_test.go | 43 ++----- internal/oidc/oidc.go | 21 ++++ internal/oidc/oidctestutil/oidc.go | 45 +++++++ .../oidc/provider/manager/manager_test.go | 36 +----- internal/oidc/token/token_handler.go | 18 +-- internal/oidc/token/token_handler_test.go | 117 +++++------------- internal/testutil/assertions.go | 17 +++ internal/testutil/crypto.go | 15 +++ 12 files changed, 168 insertions(+), 200 deletions(-) create mode 100644 internal/testutil/crypto.go diff --git a/internal/oidc/auth/auth_handler.go b/internal/oidc/auth/auth_handler.go index 231cf8ee..966bfb54 100644 --- a/internal/oidc/auth/auth_handler.go +++ b/internal/oidc/auth/auth_handler.go @@ -45,7 +45,7 @@ func NewHandler( authorizeRequester, err := oauthHelper.NewAuthorizeRequest(r.Context(), r) if err != nil { - plog.Info("authorize request error", fositeErrorForLog(err)...) + plog.Info("authorize request error", oidc.FositeErrorForLog(err)...) oauthHelper.WriteAuthorizeError(w, authorizeRequester, err) return nil } @@ -69,7 +69,7 @@ func NewHandler( }, }) if err != nil { - plog.Info("authorize response error", fositeErrorForLog(err)...) + plog.Info("authorize response error", oidc.FositeErrorForLog(err)...) oauthHelper.WriteAuthorizeError(w, authorizeRequester, err) return nil } @@ -232,15 +232,3 @@ func addCSRFSetCookieHeader(w http.ResponseWriter, csrfValue csrftoken.CSRFToken return nil } - -func fositeErrorForLog(err error) []interface{} { - rfc6749Error := fosite.ErrorToRFC6749Error(err) - keysAndValues := make([]interface{}, 0) - keysAndValues = append(keysAndValues, "name") - keysAndValues = append(keysAndValues, rfc6749Error.Name) - keysAndValues = append(keysAndValues, "status") - keysAndValues = append(keysAndValues, rfc6749Error.Status()) - keysAndValues = append(keysAndValues, "description") - keysAndValues = append(keysAndValues, rfc6749Error.Description) - return keysAndValues -} diff --git a/internal/oidc/auth/auth_handler_test.go b/internal/oidc/auth/auth_handler_test.go index 79f7b7eb..30a8dc5d 100644 --- a/internal/oidc/auth/auth_handler_test.go +++ b/internal/oidc/auth/auth_handler_test.go @@ -4,10 +4,8 @@ package auth import ( - "crypto/ecdsa" "fmt" "html" - "mime" "net/http" "net/http/httptest" "net/url" @@ -21,8 +19,10 @@ import ( "go.pinniped.dev/internal/here" "go.pinniped.dev/internal/oidc" "go.pinniped.dev/internal/oidc/csrftoken" + "go.pinniped.dev/internal/oidc/jwks" "go.pinniped.dev/internal/oidc/oidctestutil" "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/testutil" "go.pinniped.dev/pkg/oidcclient/nonce" "go.pinniped.dev/pkg/oidcclient/pkce" ) @@ -125,9 +125,9 @@ func TestAuthorizationEndpoint(t *testing.T) { // Configure fosite the same way that the production code would, using NullStorage to turn off storage. oauthStore := oidc.NullStorage{} hmacSecret := []byte("some secret - must have at least 32 bytes") - var signingKeyIsUnused *ecdsa.PrivateKey require.GreaterOrEqual(t, len(hmacSecret), 32, "fosite requires that hmac secrets have at least 32 bytes") - oauthHelper := oidc.FositeOauth2Helper(oauthStore, downstreamIssuer, hmacSecret, signingKeyIsUnused) + jwksProviderIsUnused := jwks.NewDynamicJWKSProvider() + oauthHelper := oidc.FositeOauth2Helper(oauthStore, downstreamIssuer, hmacSecret, jwksProviderIsUnused) happyCSRF := "test-csrf" happyPKCE := "test-pkce" @@ -725,7 +725,7 @@ func TestAuthorizationEndpoint(t *testing.T) { t.Logf("response body: %q", rsp.Body.String()) require.Equal(t, test.wantStatus, rsp.Code) - requireEqualContentType(t, rsp.Header().Get("Content-Type"), test.wantContentType) + testutil.RequireEqualContentType(t, rsp.Header().Get("Content-Type"), test.wantContentType) actualLocation := rsp.Header().Get("Location") if test.wantLocationHeader != "" { @@ -826,22 +826,6 @@ func (*errorReturningEncoder) Encode(_ string, _ interface{}) (string, error) { return "", fmt.Errorf("some encoding error") } -func requireEqualContentType(t *testing.T, actual string, expected string) { - t.Helper() - - if expected == "" { - require.Empty(t, actual) - return - } - - actualContentType, actualContentTypeParams, err := mime.ParseMediaType(expected) - require.NoError(t, err) - expectedContentType, expectedContentTypeParams, err := mime.ParseMediaType(expected) - require.NoError(t, err) - require.Equal(t, actualContentType, expectedContentType) - require.Equal(t, actualContentTypeParams, expectedContentTypeParams) -} - func requireEqualDecodedStateParams(t *testing.T, actualURL string, expectedURL string, stateParamDecoder oidc.Codec) { t.Helper() actualLocationURL, err := url.Parse(actualURL) diff --git a/internal/oidc/callback/callback_handler_test.go b/internal/oidc/callback/callback_handler_test.go index ead11693..d73fb097 100644 --- a/internal/oidc/callback/callback_handler_test.go +++ b/internal/oidc/callback/callback_handler_test.go @@ -24,6 +24,7 @@ import ( kubetesting "k8s.io/client-go/testing" "go.pinniped.dev/internal/oidc" + "go.pinniped.dev/internal/oidc/jwks" "go.pinniped.dev/internal/oidc/oidctestutil" "go.pinniped.dev/internal/testutil" "go.pinniped.dev/pkg/oidcclient/nonce" @@ -433,7 +434,8 @@ func TestCallbackEndpoint(t *testing.T) { oauthStore := oidc.NewKubeStorage(secrets) 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") - oauthHelper := oidc.FositeOauth2Helper(oauthStore, downstreamIssuer, hmacSecret) + jwksProviderIsUnused := jwks.NewDynamicJWKSProvider() + oauthHelper := oidc.FositeOauth2Helper(oauthStore, downstreamIssuer, hmacSecret, jwksProviderIsUnused) idpListGetter := oidctestutil.NewIDPListGetter(&test.idp) subject := NewHandler(idpListGetter, oauthHelper, happyStateCodec, happyCookieCodec, happyUpstreamRedirectURI) diff --git a/internal/oidc/dynamic_open_id_connect_ecdsa_strategy.go b/internal/oidc/dynamic_open_id_connect_ecdsa_strategy.go index 83472a0f..582b9cc6 100644 --- a/internal/oidc/dynamic_open_id_connect_ecdsa_strategy.go +++ b/internal/oidc/dynamic_open_id_connect_ecdsa_strategy.go @@ -18,7 +18,14 @@ import ( "go.pinniped.dev/internal/oidc/jwks" ) -// TODO: doc me. +// dynamicOpenIDConnectECDSAStrategy is an openid.OpenIDConnectTokenStrategy that can dynamically +// load a signing key to issue ID tokens. We want this dynamic capability since our controllers for +// loading OIDCProvider's and signing keys run in parallel, and thus the signing key might not be +// ready when an OIDCProvider is otherwise ready. +// +// If we ever update OIDCProvider's to hold their signing key, we might not need this type, since we +// could have an invariant that routes to an OIDCProvider's endpoints are only wired up if an +// OIDCProvider has a valid signing key. type dynamicOpenIDConnectECDSAStrategy struct { fositeConfig *compose.Config jwksProvider jwks.DynamicJWKSProvider @@ -61,6 +68,5 @@ func (s *dynamicOpenIDConnectECDSAStrategy) GenerateIDToken( return "", constable.Error("JWK must be of type ecdsa") } - // todo write story/issue about caching this strategy return compose.NewOpenIDConnectECDSAStrategy(s.fositeConfig, key).GenerateIDToken(ctx, requester) } diff --git a/internal/oidc/dynamic_open_id_connect_ecdsa_strategy_test.go b/internal/oidc/dynamic_open_id_connect_ecdsa_strategy_test.go index 38cb35de..1b48f471 100644 --- a/internal/oidc/dynamic_open_id_connect_ecdsa_strategy_test.go +++ b/internal/oidc/dynamic_open_id_connect_ecdsa_strategy_test.go @@ -5,15 +5,13 @@ package oidc import ( "context" - "crypto" "crypto/ecdsa" "crypto/elliptic" "crypto/rand" "crypto/rsa" - "fmt" + "net/url" "testing" - coreosoidc "github.com/coreos/go-oidc" "github.com/ory/fosite" "github.com/ory/fosite/compose" "github.com/ory/fosite/handler/openid" @@ -22,6 +20,7 @@ import ( "gopkg.in/square/go-jose.v2" "go.pinniped.dev/internal/oidc/jwks" + "go.pinniped.dev/internal/oidc/oidctestutil" ) func TestDynamicOpenIDConnectECDSAStrategy(t *testing.T) { @@ -30,6 +29,7 @@ func TestDynamicOpenIDConnectECDSAStrategy(t *testing.T) { clientID = "some-client-id" goodSubject = "some-subject" goodUsername = "some-username" + goodNonce = "some-nonce-that-is-at-least-32-characters-to-meet-entropy-requirements" ) ecPrivateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) @@ -106,6 +106,9 @@ func TestDynamicOpenIDConnectECDSAStrategy(t *testing.T) { Subject: goodSubject, Username: goodUsername, }, + Form: url.Values{ + "nonce": {goodNonce}, + }, } idToken, err := s.GenerateIDToken(context.Background(), requester) if test.wantError != "" { @@ -113,38 +116,16 @@ func TestDynamicOpenIDConnectECDSAStrategy(t *testing.T) { } else { require.NoError(t, err) - // TODO: common-ize this code with token endpoint test. - // TODO: make more assertions about ID token - privateKey, ok := test.wantSigningJWK.Key.(*ecdsa.PrivateKey) require.True(t, ok, "wanted private key to be *ecdsa.PrivateKey, but was %T", test.wantSigningJWK) - keySet := newStaticKeySet(privateKey.Public()) - verifyConfig := coreosoidc.Config{ - ClientID: clientID, - SupportedSigningAlgs: []string{coreosoidc.ES256}, - } - verifier := coreosoidc.NewVerifier(test.issuer, keySet, &verifyConfig) - _, err := verifier.Verify(context.Background(), idToken) - require.NoError(t, err) + // Perform a light validation on the token to make sure 1) we passed through the correct + // signing key and 2) we forwarded the fosite.Requester correctly. Token generation is + // tested more expansively in the token endpoint. + token := oidctestutil.VerifyECDSAIDToken(t, goodIssuer, clientID, privateKey, idToken) + require.Equal(t, goodSubject, token.Subject) + require.Equal(t, goodNonce, token.Nonce) } }) } } - -// TODO: de-dep me. -func newStaticKeySet(publicKey crypto.PublicKey) coreosoidc.KeySet { - return &staticKeySet{publicKey} -} - -type staticKeySet struct { - publicKey crypto.PublicKey -} - -func (s *staticKeySet) VerifySignature(ctx context.Context, jwt string) ([]byte, error) { - jws, err := jose.ParseSigned(jwt) - if err != nil { - return nil, fmt.Errorf("oidc: malformed jwt: %v", err) - } - return jws.Verify(s.publicKey) -} diff --git a/internal/oidc/oidc.go b/internal/oidc/oidc.go index d38a6495..7865def8 100644 --- a/internal/oidc/oidc.go +++ b/internal/oidc/oidc.go @@ -134,6 +134,27 @@ func FositeOauth2Helper( ) } +// FositeErrorForLog generates a list of information about the provided Fosite error that can be +// passed to a plog function (e.g., plog.Info()). +// +// Sample usage: +// err := someFositeLibraryFunction() +// if err != nil { +// plog.Info("some error", FositeErrorForLog(err)...) +// ... +// } +func FositeErrorForLog(err error) []interface{} { + rfc6749Error := fosite.ErrorToRFC6749Error(err) + keysAndValues := make([]interface{}, 0) + keysAndValues = append(keysAndValues, "name") + keysAndValues = append(keysAndValues, rfc6749Error.Name) + keysAndValues = append(keysAndValues, "status") + keysAndValues = append(keysAndValues, rfc6749Error.Status()) + keysAndValues = append(keysAndValues, "description") + keysAndValues = append(keysAndValues, rfc6749Error.Description) + return keysAndValues +} + type IDPListGetter interface { GetIDPList() []provider.UpstreamOIDCIdentityProviderI } diff --git a/internal/oidc/oidctestutil/oidc.go b/internal/oidc/oidctestutil/oidc.go index 5b214e5c..19aa7c07 100644 --- a/internal/oidc/oidctestutil/oidc.go +++ b/internal/oidc/oidctestutil/oidc.go @@ -5,9 +5,16 @@ package oidctestutil import ( "context" + "crypto" + "crypto/ecdsa" + "fmt" "net/url" + "testing" + coreosoidc "github.com/coreos/go-oidc" + "github.com/stretchr/testify/require" "golang.org/x/oauth2" + "gopkg.in/square/go-jose.v2" "go.pinniped.dev/internal/oidc/provider" "go.pinniped.dev/pkg/oidcclient/nonce" @@ -127,3 +134,41 @@ type ExpectedUpstreamStateParamFormat struct { K string `json:"k"` V string `json:"v"` } + +type staticKeySet struct { + publicKey crypto.PublicKey +} + +func newStaticKeySet(publicKey crypto.PublicKey) coreosoidc.KeySet { + return &staticKeySet{publicKey} +} + +func (s *staticKeySet) VerifySignature(ctx context.Context, jwt string) ([]byte, error) { + jws, err := jose.ParseSigned(jwt) + if err != nil { + return nil, fmt.Errorf("oidc: malformed jwt: %w", err) + } + return jws.Verify(s.publicKey) +} + +// VerifyECDSAIDToken verifies that the provided idToken was issued via the provided jwtSigningKey. +// It also performs some light validation on the claims, i.e., it makes sure the provided idToken +// has the provided issuer and clientID. +// +// Further validation can be done via callers via the returned coreosoidc.IDToken. +func VerifyECDSAIDToken( + t *testing.T, + issuer, clientID string, + jwtSigningKey *ecdsa.PrivateKey, + idToken string, +) *coreosoidc.IDToken { + t.Helper() + + keySet := newStaticKeySet(jwtSigningKey.Public()) + verifyConfig := coreosoidc.Config{ClientID: clientID, SupportedSigningAlgs: []string{coreosoidc.ES256}} + verifier := coreosoidc.NewVerifier(issuer, keySet, &verifyConfig) + token, err := verifier.Verify(context.Background(), idToken) + require.NoError(t, err) + + return token +} diff --git a/internal/oidc/provider/manager/manager_test.go b/internal/oidc/provider/manager/manager_test.go index 465b3c10..bf017cfe 100644 --- a/internal/oidc/provider/manager/manager_test.go +++ b/internal/oidc/provider/manager/manager_test.go @@ -5,12 +5,8 @@ package manager import ( "context" - "crypto" "crypto/ecdsa" - "crypto/sha256" - "encoding/base64" "encoding/json" - "fmt" "io/ioutil" "net/http" "net/http/httptest" @@ -18,7 +14,6 @@ import ( "strings" "testing" - coreosoidc "github.com/coreos/go-oidc" "github.com/sclevine/spec" "github.com/stretchr/testify/require" "gopkg.in/square/go-jose.v2" @@ -30,6 +25,7 @@ import ( "go.pinniped.dev/internal/oidc/jwks" "go.pinniped.dev/internal/oidc/oidctestutil" "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/testutil" "go.pinniped.dev/pkg/oidcclient/nonce" "go.pinniped.dev/pkg/oidcclient/oidctypes" "go.pinniped.dev/pkg/oidcclient/pkce" @@ -184,7 +180,6 @@ func TestManager(t *testing.T) { // Validate ID token is signed by the correct JWK to make sure we wired the token endpoint // signing key correctly. - // TODO: common-ize this code with token endpoint test. idToken, ok := body["id_token"].(string) r.True(ok, "wanted id_token type to be string, but was %T", body["id_token"]) @@ -192,11 +187,7 @@ func TestManager(t *testing.T) { privateKey, ok := jwks.Keys[0].Key.(*ecdsa.PrivateKey) r.True(ok, "wanted private key to be *ecdsa.PrivateKey, but was %T", jwks.Keys[0].Key) - keySet := newStaticKeySet(privateKey.Public()) - verifyConfig := coreosoidc.Config{ClientID: downstreamClientID, SupportedSigningAlgs: []string{coreosoidc.ES256}} - verifier := coreosoidc.NewVerifier(jwkIssuer, keySet, &verifyConfig) - _, err := verifier.Verify(context.Background(), idToken) - r.NoError(err) + oidctestutil.VerifyECDSAIDToken(t, jwkIssuer, downstreamClientID, privateKey, idToken) // Make sure that we wired up the callback endpoint to use kube storage for fosite sessions. r.Equal(len(kubeClient.Actions()), numberOfKubeActionsBeforeThisRequest+7, @@ -305,7 +296,7 @@ func TestManager(t *testing.T) { "client_id": []string{downstreamClientID}, "state": []string{"some-state-value-that-is-32-byte"}, "nonce": []string{"some-nonce-value-that-is-at-least-32-bytes"}, - "code_challenge": []string{doSHA256(downstreamPKCECodeVerifier)}, + "code_challenge": []string{testutil.SHA256(downstreamPKCECodeVerifier)}, "code_challenge_method": []string{"S256"}, "redirect_uri": []string{downstreamRedirectURL}, }.Encode() @@ -406,24 +397,3 @@ func TestManager(t *testing.T) { }) }) } - -func doSHA256(s string) string { - b := sha256.Sum256([]byte(s)) - return base64.RawURLEncoding.EncodeToString(b[:]) -} - -func newStaticKeySet(publicKey crypto.PublicKey) coreosoidc.KeySet { - return &staticKeySet{publicKey} -} - -type staticKeySet struct { - publicKey crypto.PublicKey -} - -func (s *staticKeySet) VerifySignature(ctx context.Context, jwt string) ([]byte, error) { - jws, err := jose.ParseSigned(jwt) - if err != nil { - return nil, fmt.Errorf("oidc: malformed jwt: %v", err) - } - return jws.Verify(s.publicKey) -} diff --git a/internal/oidc/token/token_handler.go b/internal/oidc/token/token_handler.go index b8747636..c481168b 100644 --- a/internal/oidc/token/token_handler.go +++ b/internal/oidc/token/token_handler.go @@ -11,6 +11,7 @@ import ( "github.com/ory/fosite/handler/openid" "go.pinniped.dev/internal/httputil/httperr" + "go.pinniped.dev/internal/oidc" "go.pinniped.dev/internal/plog" ) @@ -21,14 +22,14 @@ func NewHandler( var session openid.DefaultSession accessRequest, err := oauthHelper.NewAccessRequest(r.Context(), r, &session) if err != nil { - plog.Info("token request error", fositeErrorForLog(err)...) + plog.Info("token request error", oidc.FositeErrorForLog(err)...) oauthHelper.WriteAccessError(w, accessRequest, err) return nil } accessResponse, err := oauthHelper.NewAccessResponse(r.Context(), accessRequest) if err != nil { - plog.Info("token response error", fositeErrorForLog(err)...) + plog.Info("token response error", oidc.FositeErrorForLog(err)...) oauthHelper.WriteAccessError(w, accessRequest, err) return nil } @@ -38,16 +39,3 @@ func NewHandler( return nil }) } - -// TODO: de-dup me. -func fositeErrorForLog(err error) []interface{} { - rfc6749Error := fosite.ErrorToRFC6749Error(err) - keysAndValues := make([]interface{}, 0) - keysAndValues = append(keysAndValues, "name") - keysAndValues = append(keysAndValues, rfc6749Error.Name) - keysAndValues = append(keysAndValues, "status") - keysAndValues = append(keysAndValues, rfc6749Error.Status()) - keysAndValues = append(keysAndValues, "description") - keysAndValues = append(keysAndValues, rfc6749Error.Description) - return keysAndValues -} diff --git a/internal/oidc/token/token_handler_test.go b/internal/oidc/token/token_handler_test.go index ea64cc1b..9e8ccd11 100644 --- a/internal/oidc/token/token_handler_test.go +++ b/internal/oidc/token/token_handler_test.go @@ -5,17 +5,14 @@ package token import ( "context" - "crypto" "crypto/ecdsa" "crypto/elliptic" "crypto/rand" "crypto/sha256" "encoding/base64" "encoding/json" - "fmt" "io" "io/ioutil" - "mime" "net/http" "net/http/httptest" "net/url" @@ -23,7 +20,6 @@ import ( "testing" "time" - coreosoidc "github.com/coreos/go-oidc" "github.com/ory/fosite" "github.com/ory/fosite/handler/oauth2" "github.com/ory/fosite/handler/openid" @@ -35,6 +31,9 @@ import ( "go.pinniped.dev/internal/here" "go.pinniped.dev/internal/oidc" + "go.pinniped.dev/internal/oidc/jwks" + "go.pinniped.dev/internal/oidc/oidctestutil" + "go.pinniped.dev/internal/testutil" ) const ( @@ -188,7 +187,7 @@ func TestTokenEndpoint(t *testing.T) { "client_id": {goodClient}, "state": {"some-state-value-that-is-32-byte"}, "nonce": {goodNonce}, - "code_challenge": {doSHA256(goodPKCECodeVerifier)}, + "code_challenge": {testutil.SHA256(goodPKCECodeVerifier)}, "code_challenge_method": {"S256"}, "redirect_uri": {goodRedirectURI}, }, @@ -406,7 +405,7 @@ func TestTokenEndpoint(t *testing.T) { t.Logf("response body: %q", rsp.Body.String()) require.Equal(t, test.wantStatus, rsp.Code) - requireEqualContentType(t, rsp.Header().Get("Content-Type"), "application/json") + testutil.RequireEqualContentType(t, rsp.Header().Get("Content-Type"), "application/json") if test.wantBodyFields != nil { var m map[string]interface{} require.NoError(t, json.Unmarshal(rsp.Body.Bytes(), &m)) @@ -444,7 +443,7 @@ func TestTokenEndpoint(t *testing.T) { subject.ServeHTTP(rsp0, req) t.Logf("response 0: %#v", rsp0) t.Logf("response 0 body: %q", rsp0.Body.String()) - requireEqualContentType(t, rsp0.Header().Get("Content-Type"), "application/json") + testutil.RequireEqualContentType(t, rsp0.Header().Get("Content-Type"), "application/json") require.Equal(t, http.StatusOK, rsp0.Code) var m map[string]interface{} @@ -470,7 +469,7 @@ func TestTokenEndpoint(t *testing.T) { t.Logf("response 1: %#v", rsp1) t.Logf("response 1 body: %q", rsp1.Body.String()) require.Equal(t, http.StatusBadRequest, rsp1.Code) - requireEqualContentType(t, rsp1.Header().Get("Content-Type"), "application/json") + testutil.RequireEqualContentType(t, rsp1.Header().Get("Content-Type"), "application/json") require.JSONEq(t, fositeReusedAuthCodeErrorBody, rsp1.Body.String()) requireInvalidAuthCodeStorage(t, code, oauthStore) @@ -547,8 +546,8 @@ func makeHappyOauthHelper( ) (fosite.OAuth2Provider, string, *ecdsa.PrivateKey) { t.Helper() - jwtSigningKey := generateJWTSigningKey(t) - oauthHelper := oidc.FositeOauth2Helper(store, goodIssuer, []byte(hmacSecret), jwtSigningKey) + jwtSigningKey, jwkProvider := generateJWTSigningKeyAndJWKSProvider(t, goodIssuer) + oauthHelper := oidc.FositeOauth2Helper(store, goodIssuer, []byte(hmacSecret), jwkProvider) // Simulate the auth endpoint running so Fosite code will fill the store with realistic values. // @@ -574,11 +573,21 @@ func makeHappyOauthHelper( return oauthHelper, authResponder.GetCode(), jwtSigningKey } -func generateJWTSigningKey(t *testing.T) *ecdsa.PrivateKey { +func generateJWTSigningKeyAndJWKSProvider(t *testing.T, issuer string) (*ecdsa.PrivateKey, jwks.DynamicJWKSProvider) { t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) require.NoError(t, err) - return key + + jwksProvider := jwks.NewDynamicJWKSProvider() + jwksProvider.SetIssuerToJWKSMap( + nil, // public JWKS unused + map[string]*jose.JSONWebKey{ + issuer: {Key: key}, + }, + ) + + return key, jwksProvider } func hashAccessToken(accessToken string) string { @@ -591,12 +600,6 @@ func hashAccessToken(accessToken string) string { return base64.RawURLEncoding.EncodeToString(b[:len(b)/2]) } -// TODO: de-dup me (manager test). -func doSHA256(s string) string { - b := sha256.Sum256([]byte(s)) - return base64.RawURLEncoding.EncodeToString(b[:]) -} - func requireInvalidAuthCodeStorage( t *testing.T, code string, @@ -736,7 +739,7 @@ func requireValidAuthRequest( wantGrantedScopes = append([]string{"openid"}, wantGrantedScopes...) } require.NotEmpty(t, authRequest.GetID()) - requireTimeInDelta(t, authRequest.GetRequestedAt(), time.Now().UTC(), timeComparisonFudgeSeconds*time.Second) + testutil.RequireTimeInDelta(t, authRequest.GetRequestedAt(), time.Now().UTC(), timeComparisonFudgeSeconds*time.Second) require.Equal(t, goodClient, authRequest.GetClient().GetID()) require.Equal(t, fosite.Arguments(wantRequestedScopes), authRequest.GetRequestedScopes()) require.Equal(t, fosite.Arguments(wantGrantedScopes), authRequest.GetGrantedScopes()) @@ -756,13 +759,13 @@ func requireValidAuthRequest( require.Equal(t, goodSubject, claims.Subject) require.Equal(t, []string{goodClient}, claims.Audience) require.Equal(t, goodNonce, claims.Nonce) - requireTimeInDelta( + testutil.RequireTimeInDelta( t, time.Now().UTC().Add(idTokenExpirationSeconds*time.Second), claims.ExpiresAt, timeComparisonFudgeSeconds*time.Second, ) - requireTimeInDelta(t, time.Now().UTC(), claims.IssuedAt, timeComparisonFudgeSeconds*time.Second) + testutil.RequireTimeInDelta(t, time.Now().UTC(), claims.IssuedAt, timeComparisonFudgeSeconds*time.Second) require.Equal(t, wantAccessTokenHash, claims.AccessTokenHash) // We are in charge of setting these fields. For the purpose of testing, we ensure that the @@ -784,7 +787,7 @@ func requireValidAuthRequest( // Assert that the token expirations are what we think they should be. authCodeExpiresAt, ok := session.ExpiresAt[fosite.AuthorizeCode] require.True(t, ok, "expected session to hold expiration time for auth code") - requireTimeInDelta( + testutil.RequireTimeInDelta( t, time.Now().UTC().Add(authCodeExpirationSeconds*time.Second), authCodeExpiresAt, @@ -792,7 +795,7 @@ func requireValidAuthRequest( ) accessTokenExpiresAt, ok := session.ExpiresAt[fosite.AccessToken] require.True(t, ok, "expected session to hold expiration time for access token") - requireTimeInDelta( + testutil.RequireTimeInDelta( t, time.Now().UTC().Add(accessTokenExpirationSeconds*time.Second), accessTokenExpiresAt, @@ -805,17 +808,15 @@ func requireValidAuthRequest( } func requireValidIDToken(t *testing.T, body map[string]interface{}, jwtSigningKey *ecdsa.PrivateKey) { + t.Helper() + idToken, ok := body["id_token"] require.Truef(t, ok, "body did not contain 'id_token': %s", body) idTokenString, ok := idToken.(string) require.Truef(t, ok, "wanted id_token to be a string, but got %T", idToken) // The go-oidc library will validate the signature and the client claim in the ID token. - keySet := newStaticKeySet(jwtSigningKey.Public()) - verifyConfig := coreosoidc.Config{ClientID: goodClient, SupportedSigningAlgs: []string{coreosoidc.ES256}} - verifier := coreosoidc.NewVerifier(goodIssuer, keySet, &verifyConfig) - token, err := verifier.Verify(context.Background(), idTokenString) - require.NoError(t, err) + token := oidctestutil.VerifyECDSAIDToken(t, goodIssuer, goodClient, jwtSigningKey, idTokenString) var claims struct { Subject string `json:"sub"` @@ -837,7 +838,7 @@ func requireValidIDToken(t *testing.T, body map[string]interface{}, jwtSigningKe require.ElementsMatch(t, idTokenFields, getMapKeys(m)) // verify each of the claims - err = token.Claims(&claims) + err := token.Claims(&claims) require.NoError(t, err) require.Equal(t, goodSubject, claims.Subject) require.Len(t, claims.Audience, 1) @@ -851,60 +852,10 @@ func requireValidIDToken(t *testing.T, body map[string]interface{}, jwtSigningKe issuedAt := time.Unix(claims.IssuedAt, 0) requestedAt := time.Unix(claims.RequestedAt, 0) authTime := time.Unix(claims.AuthTime, 0) - requireTimeInDelta(t, time.Now().UTC().Add(idTokenExpirationSeconds*time.Second), expiresAt, timeComparisonFudgeSeconds*time.Second) - requireTimeInDelta(t, time.Now().UTC(), issuedAt, timeComparisonFudgeSeconds*time.Second) - requireTimeInDelta(t, goodRequestedAtTime, requestedAt, timeComparisonFudgeSeconds*time.Second) - requireTimeInDelta(t, goodAuthTime, authTime, timeComparisonFudgeSeconds*time.Second) -} - -// TODO: de-dup me (manager test). -func newStaticKeySet(publicKey crypto.PublicKey) coreosoidc.KeySet { - return &staticKeySet{publicKey} -} - -type staticKeySet struct { - publicKey crypto.PublicKey -} - -func (s *staticKeySet) VerifySignature(ctx context.Context, jwt string) ([]byte, error) { - jws, err := jose.ParseSigned(jwt) - if err != nil { - return nil, fmt.Errorf("oidc: malformed jwt: %v", err) - } - return jws.Verify(s.publicKey) -} - -// TODO: de-dup me. -func requireEqualContentType(t *testing.T, actual string, expected string) { - t.Helper() - - if expected == "" { - require.Empty(t, actual) - return - } - - actualContentType, actualContentTypeParams, err := mime.ParseMediaType(expected) - require.NoError(t, err) - expectedContentType, expectedContentTypeParams, err := mime.ParseMediaType(expected) - require.NoError(t, err) - require.Equal(t, actualContentType, expectedContentType) - require.Equal(t, actualContentTypeParams, expectedContentTypeParams) -} - -// TODO: use actual testutil function. -//nolint:unparam -func requireTimeInDelta(t *testing.T, t1 time.Time, t2 time.Time, delta time.Duration) { - t.Helper() - require.InDeltaf(t, - float64(t1.UnixNano()), - float64(t2.UnixNano()), - float64(delta.Nanoseconds()), - "expected %s and %s to be < %s apart, but they are %s apart", - t1.Format(time.RFC3339Nano), - t2.Format(time.RFC3339Nano), - delta.String(), - t1.Sub(t2).String(), - ) + testutil.RequireTimeInDelta(t, time.Now().UTC().Add(idTokenExpirationSeconds*time.Second), expiresAt, timeComparisonFudgeSeconds*time.Second) + testutil.RequireTimeInDelta(t, time.Now().UTC(), issuedAt, timeComparisonFudgeSeconds*time.Second) + testutil.RequireTimeInDelta(t, goodRequestedAtTime, requestedAt, timeComparisonFudgeSeconds*time.Second) + testutil.RequireTimeInDelta(t, goodAuthTime, authTime, timeComparisonFudgeSeconds*time.Second) } func deepCopyRequestForm(r *http.Request) *http.Request { diff --git a/internal/testutil/assertions.go b/internal/testutil/assertions.go index 77247602..e49233d5 100644 --- a/internal/testutil/assertions.go +++ b/internal/testutil/assertions.go @@ -4,6 +4,7 @@ package testutil import ( + "mime" "testing" "time" @@ -22,3 +23,19 @@ func RequireTimeInDelta(t *testing.T, t1 time.Time, t2 time.Time, delta time.Dur t1.Sub(t2).String(), ) } + +func RequireEqualContentType(t *testing.T, actual string, expected string) { + t.Helper() + + if expected == "" { + require.Empty(t, actual) + return + } + + actualContentType, actualContentTypeParams, err := mime.ParseMediaType(expected) + require.NoError(t, err) + expectedContentType, expectedContentTypeParams, err := mime.ParseMediaType(expected) + require.NoError(t, err) + require.Equal(t, actualContentType, expectedContentType) + require.Equal(t, actualContentTypeParams, expectedContentTypeParams) +} diff --git a/internal/testutil/crypto.go b/internal/testutil/crypto.go new file mode 100644 index 00000000..fdc83e84 --- /dev/null +++ b/internal/testutil/crypto.go @@ -0,0 +1,15 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package testutil + +import ( + "crypto/sha256" + "encoding/base64" +) + +// SHA256 returns the base64 URL encoding of the SHA256 sum of the provided string. +func SHA256(s string) string { + b := sha256.Sum256([]byte(s)) + return base64.RawURLEncoding.EncodeToString(b[:]) +}