diff --git a/internal/controller/supervisorconfig/jwks_observer.go b/internal/controller/supervisorconfig/jwks_observer.go index 0d6df5b6..5c01c5b4 100644 --- a/internal/controller/supervisorconfig/jwks_observer.go +++ b/internal/controller/supervisorconfig/jwks_observer.go @@ -24,7 +24,10 @@ type jwksObserverController struct { } type IssuerToJWKSMapSetter interface { - SetIssuerToJWKSMap(issuerToJWKSMap map[string]*jose.JSONWebKeySet) + SetIssuerToJWKSMap( + issuerToJWKSMap map[string]*jose.JSONWebKeySet, + issuerToActiveJWKMap map[string]*jose.JSONWebKey, + ) } // Returns a controller which watches all of the OIDCProviders and their corresponding Secrets @@ -70,6 +73,7 @@ func (c *jwksObserverController) Sync(ctx controllerlib.Context) error { // Rebuild the whole map on any change to any Secret or OIDCProvider, because either can have changes that // can cause the map to need to be updated. issuerToJWKSMap := map[string]*jose.JSONWebKeySet{} + issuerToActiveJWKMap := map[string]*jose.JSONWebKey{} for _, provider := range allProviders { secretRef := provider.Status.JWKSSecret @@ -78,17 +82,33 @@ func (c *jwksObserverController) Sync(ctx controllerlib.Context) error { plog.Debug("jwksObserverController Sync could not find JWKS secret", "namespace", ns, "secretName", secretRef.Name) continue } - jwkFromSecret := jose.JSONWebKeySet{} - err = json.Unmarshal(jwksSecret.Data[jwksKey], &jwkFromSecret) + + jwksFromSecret := jose.JSONWebKeySet{} + err = json.Unmarshal(jwksSecret.Data[jwksKey], &jwksFromSecret) if err != nil { plog.Debug("jwksObserverController Sync found a JWKS secret with Data in an unexpected format", "namespace", ns, "secretName", secretRef.Name) continue } - issuerToJWKSMap[provider.Spec.Issuer] = &jwkFromSecret + + activeJWKFromSecret := jose.JSONWebKey{} + err = json.Unmarshal(jwksSecret.Data[activeJWKKey], &activeJWKFromSecret) + if err != nil { + plog.Debug("jwksObserverController Sync found an active JWK secret with Data in an unexpected format", "namespace", ns, "secretName", secretRef.Name) + continue + } + + issuerToJWKSMap[provider.Spec.Issuer] = &jwksFromSecret + issuerToActiveJWKMap[provider.Spec.Issuer] = &activeJWKFromSecret } - plog.Debug("jwksObserverController Sync updated the JWKS cache", "issuerCount", len(issuerToJWKSMap)) - c.issuerToJWKSSetter.SetIssuerToJWKSMap(issuerToJWKSMap) + plog.Debug( + "jwksObserverController Sync updated the JWKS cache", + "issuerJWKSCount", + len(issuerToJWKSMap), + "issuerActiveJWKCount", + len(issuerToActiveJWKMap), + ) + c.issuerToJWKSSetter.SetIssuerToJWKSMap(issuerToJWKSMap, issuerToActiveJWKMap) return nil } diff --git a/internal/controller/supervisorconfig/jwks_observer_test.go b/internal/controller/supervisorconfig/jwks_observer_test.go index 8bf920ef..ad7323b8 100644 --- a/internal/controller/supervisorconfig/jwks_observer_test.go +++ b/internal/controller/supervisorconfig/jwks_observer_test.go @@ -96,13 +96,18 @@ func TestJWKSObserverControllerInformerFilters(t *testing.T) { } type fakeIssuerToJWKSMapSetter struct { - setIssuerToJWKSMapWasCalled bool - issuerToJWKSMapReceived map[string]*jose.JSONWebKeySet + setIssuerToJWKSMapWasCalled bool + issuerToJWKSMapReceived map[string]*jose.JSONWebKeySet + issuerToActiveJWKMapReceived map[string]*jose.JSONWebKey } -func (f *fakeIssuerToJWKSMapSetter) SetIssuerToJWKSMap(issuerToJWKSMap map[string]*jose.JSONWebKeySet) { +func (f *fakeIssuerToJWKSMapSetter) SetIssuerToJWKSMap( + issuerToJWKSMap map[string]*jose.JSONWebKeySet, + issuerToActiveJWKMap map[string]*jose.JSONWebKey, +) { f.setIssuerToJWKSMapWasCalled = true f.issuerToJWKSMapReceived = issuerToJWKSMap + f.issuerToActiveJWKMapReceived = issuerToActiveJWKMap } func TestJWKSObserverControllerSync(t *testing.T) { @@ -181,6 +186,7 @@ func TestJWKSObserverControllerSync(t *testing.T) { r.True(issuerToJWKSSetter.setIssuerToJWKSMapWasCalled) r.Empty(issuerToJWKSSetter.issuerToJWKSMapReceived) + r.Empty(issuerToJWKSSetter.issuerToActiveJWKMapReceived) }) }) @@ -212,10 +218,30 @@ func TestJWKSObserverControllerSync(t *testing.T) { Namespace: installedInNamespace, }, Spec: v1alpha1.OIDCProviderSpec{Issuer: "https://bad-secret-issuer.com"}, + Status: v1alpha1.OIDCProviderStatus{ + JWKSSecret: corev1.LocalObjectReference{Name: "bad-secret-name"}, + }, + } + oidcProviderWithBadJWKSSecret := &v1alpha1.OIDCProvider{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bad-jwks-secret-oidcprovider", + Namespace: installedInNamespace, + }, + Spec: v1alpha1.OIDCProviderSpec{Issuer: "https://bad-jwks-secret-issuer.com"}, Status: v1alpha1.OIDCProviderStatus{ JWKSSecret: corev1.LocalObjectReference{Name: "bad-jwks-secret-name"}, }, } + oidcProviderWithBadActiveJWKSecret := &v1alpha1.OIDCProvider{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bad-active-jwk-secret-oidcprovider", + Namespace: installedInNamespace, + }, + Spec: v1alpha1.OIDCProviderSpec{Issuer: "https://bad-active-jwk-secret-issuer.com"}, + Status: v1alpha1.OIDCProviderStatus{ + JWKSSecret: corev1.LocalObjectReference{Name: "bad-active-jwk-secret-name"}, + }, + } oidcProviderWithGoodSecret1 := &v1alpha1.OIDCProvider{ ObjectMeta: metav1.ObjectMeta{ Name: "good-secret-oidcprovider1", @@ -260,24 +286,48 @@ func TestJWKSObserverControllerSync(t *testing.T) { "jwks": []byte(`{"keys": [` + expectedJWK2 + `]}`), }, } + badSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bad-secret-name", + Namespace: installedInNamespace, + }, + Data: map[string][]byte{"junk": nil}, + } badJWKSSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "bad-jwks-secret-name", Namespace: installedInNamespace, }, - Data: map[string][]byte{"junk": nil}, + Data: map[string][]byte{ + "activeJWK": []byte(expectedJWK2), + "jwks": []byte("bad"), + }, + } + badActiveJWKSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bad-active-jwk-secret-name", + Namespace: installedInNamespace, + }, + Data: map[string][]byte{ + "activeJWK": []byte("bad"), + "jwks": []byte(`{"keys": [` + expectedJWK2 + `]}`), + }, } r.NoError(pinnipedInformerClient.Tracker().Add(oidcProviderWithoutSecret1)) r.NoError(pinnipedInformerClient.Tracker().Add(oidcProviderWithoutSecret2)) r.NoError(pinnipedInformerClient.Tracker().Add(oidcProviderWithBadSecret)) + r.NoError(pinnipedInformerClient.Tracker().Add(oidcProviderWithBadJWKSSecret)) + r.NoError(pinnipedInformerClient.Tracker().Add(oidcProviderWithBadActiveJWKSecret)) r.NoError(pinnipedInformerClient.Tracker().Add(oidcProviderWithGoodSecret1)) r.NoError(pinnipedInformerClient.Tracker().Add(oidcProviderWithGoodSecret2)) r.NoError(kubeInformerClient.Tracker().Add(goodJWKSSecret1)) r.NoError(kubeInformerClient.Tracker().Add(goodJWKSSecret2)) + r.NoError(kubeInformerClient.Tracker().Add(badSecret)) r.NoError(kubeInformerClient.Tracker().Add(badJWKSSecret)) + r.NoError(kubeInformerClient.Tracker().Add(badActiveJWKSecret)) }) - requireJWKJSON := func(expectedJWKJSON string, actualJWKS *jose.JSONWebKeySet) { + requireJWKSJSON := func(expectedJWKJSON string, actualJWKS *jose.JSONWebKeySet) { r.NotNil(actualJWKS) r.Len(actualJWKS.Keys, 1) actualJWK := actualJWKS.Keys[0] @@ -286,16 +336,26 @@ func TestJWKSObserverControllerSync(t *testing.T) { r.JSONEq(expectedJWKJSON, string(actualJWKJSON)) } + requireJWKJSON := func(expectedJWKJSON string, actualJWK *jose.JSONWebKey) { + r.NotNil(actualJWK) + actualJWKJSON, err := json.Marshal(actualJWK) + r.NoError(err) + r.JSONEq(expectedJWKJSON, string(actualJWKJSON)) + } + it("updates the issuerToJWKSSetter's map to include only the issuers that had valid JWKS", func() { startInformersAndController() r.NoError(controllerlib.TestSync(t, subject, *syncContext)) r.True(issuerToJWKSSetter.setIssuerToJWKSMapWasCalled) r.Len(issuerToJWKSSetter.issuerToJWKSMapReceived, 2) + r.Len(issuerToJWKSSetter.issuerToActiveJWKMapReceived, 2) // the actual JWK should match the one from the test fixture that was put into the secret - requireJWKJSON(expectedJWK1, issuerToJWKSSetter.issuerToJWKSMapReceived["https://issuer-with-good-secret1.com"]) - requireJWKJSON(expectedJWK2, issuerToJWKSSetter.issuerToJWKSMapReceived["https://issuer-with-good-secret2.com"]) + requireJWKSJSON(expectedJWK1, issuerToJWKSSetter.issuerToJWKSMapReceived["https://issuer-with-good-secret1.com"]) + requireJWKJSON(expectedJWK1, issuerToJWKSSetter.issuerToActiveJWKMapReceived["https://issuer-with-good-secret1.com"]) + requireJWKSJSON(expectedJWK2, issuerToJWKSSetter.issuerToJWKSMapReceived["https://issuer-with-good-secret2.com"]) + requireJWKJSON(expectedJWK2, issuerToJWKSSetter.issuerToActiveJWKMapReceived["https://issuer-with-good-secret2.com"]) }) }) }, spec.Parallel(), spec.Report(report.Terminal{})) diff --git a/internal/oidc/dynamic_open_id_connect_ecdsa_strategy.go b/internal/oidc/dynamic_open_id_connect_ecdsa_strategy.go new file mode 100644 index 00000000..6d7d837f --- /dev/null +++ b/internal/oidc/dynamic_open_id_connect_ecdsa_strategy.go @@ -0,0 +1,42 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package oidc + +import ( + "context" + + "github.com/ory/fosite" + "github.com/ory/fosite/compose" + "github.com/ory/fosite/handler/openid" + + "go.pinniped.dev/internal/oidc/jwks" +) + +// TODO: doc me. +type dynamicOpenIDConnectECDSAStrategy struct { + issuer string + fositeConfig *compose.Config + jwksProvider jwks.DynamicJWKSProvider +} + +var _ openid.OpenIDConnectTokenStrategy = &dynamicOpenIDConnectECDSAStrategy{} + +func newDynamicOpenIDConnectECDSAStrategy( + issuer string, + fositeConfig *compose.Config, + jwksProvider jwks.DynamicJWKSProvider, +) *dynamicOpenIDConnectECDSAStrategy { + return &dynamicOpenIDConnectECDSAStrategy{ + issuer: issuer, + fositeConfig: fositeConfig, + jwksProvider: jwksProvider, + } +} + +func (s *dynamicOpenIDConnectECDSAStrategy) GenerateIDToken( + ctx context.Context, + requester fosite.Requester, +) (string, error) { + return "", nil +} diff --git a/internal/oidc/dynamic_open_id_connect_ecdsa_strategy_test.go b/internal/oidc/dynamic_open_id_connect_ecdsa_strategy_test.go new file mode 100644 index 00000000..93ba46cc --- /dev/null +++ b/internal/oidc/dynamic_open_id_connect_ecdsa_strategy_test.go @@ -0,0 +1,136 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package oidc + +import ( + "context" + "crypto" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/rsa" + "fmt" + "testing" + + coreosoidc "github.com/coreos/go-oidc" + "github.com/ory/fosite" + "github.com/ory/fosite/compose" + "github.com/stretchr/testify/require" + "go.pinniped.dev/internal/oidc/jwks" + "gopkg.in/square/go-jose.v2" +) + +func TestDynamicOpenIDConnectECDSAStrategy(t *testing.T) { + const ( + goodIssuer = "https://some-good-issuer.com" + clientID = "some-client-id" + ) + + ecPrivateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + + rsaPrivateKey, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + tests := []struct { + name string + issuer string + jwksProvider func(jwks.DynamicJWKSProvider) + wantError string + wantSigningJWK *jose.JSONWebKey + }{ + { + name: "jwks provider does contain signing key for issuer", + issuer: goodIssuer, + jwksProvider: func(provider jwks.DynamicJWKSProvider) { + provider.SetIssuerToJWKSMap( + nil, + map[string]*jose.JSONWebKey{ + goodIssuer: { + Key: ecPrivateKey, + }, + }, + ) + }, + wantSigningJWK: &jose.JSONWebKey{ + Key: ecPrivateKey, + }, + }, + { + name: "jwks provider does not contain signing key for issuer", + issuer: goodIssuer, + wantError: "some unkonwn key error", + }, + { + name: "jwks provider contains signing key of wrong type for issuer", + issuer: goodIssuer, + jwksProvider: func(provider jwks.DynamicJWKSProvider) { + provider.SetIssuerToJWKSMap( + nil, + map[string]*jose.JSONWebKey{ + goodIssuer: { + Key: rsaPrivateKey, + }, + }, + ) + }, + wantError: "some invalid key type error", + }, + } + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + jwksProvider := jwks.NewDynamicJWKSProvider() + if test.jwksProvider != nil { + test.jwksProvider(jwksProvider) + } + s := newDynamicOpenIDConnectECDSAStrategy(test.issuer, &compose.Config{}, jwksProvider) + + requester := &fosite.Request{ + Client: &fosite.DefaultClient{ + ID: clientID, + }, + // Session: fositeopenid.DefaultSession{}, + } + idToken, err := s.GenerateIDToken(context.Background(), requester) + if test.wantError != "" { + require.EqualError(t, err, test.wantError) + } 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) + } + }) + } +} + +// 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/jwks/dynamic_jwks_provider.go b/internal/oidc/jwks/dynamic_jwks_provider.go index 8672fd2f..fa156e3c 100644 --- a/internal/oidc/jwks/dynamic_jwks_provider.go +++ b/internal/oidc/jwks/dynamic_jwks_provider.go @@ -10,29 +10,38 @@ import ( ) type DynamicJWKSProvider interface { - SetIssuerToJWKSMap(issuerToJWKSMap map[string]*jose.JSONWebKeySet) - GetJWKS(issuerName string) *jose.JSONWebKeySet + SetIssuerToJWKSMap( + issuerToJWKSMap map[string]*jose.JSONWebKeySet, + issuerToActiveJWKMap map[string]*jose.JSONWebKey, + ) + GetJWKS(issuerName string) (jwks *jose.JSONWebKeySet, activeJWK *jose.JSONWebKey) } type dynamicJWKSProvider struct { - issuerToJWKSMap map[string]*jose.JSONWebKeySet - mutex sync.RWMutex + issuerToJWKSMap map[string]*jose.JSONWebKeySet + issuerToActiveJWKMap map[string]*jose.JSONWebKey + mutex sync.RWMutex } func NewDynamicJWKSProvider() DynamicJWKSProvider { return &dynamicJWKSProvider{ - issuerToJWKSMap: map[string]*jose.JSONWebKeySet{}, + issuerToJWKSMap: map[string]*jose.JSONWebKeySet{}, + issuerToActiveJWKMap: map[string]*jose.JSONWebKey{}, } } -func (p *dynamicJWKSProvider) SetIssuerToJWKSMap(issuerToJWKSMap map[string]*jose.JSONWebKeySet) { +func (p *dynamicJWKSProvider) SetIssuerToJWKSMap( + issuerToJWKSMap map[string]*jose.JSONWebKeySet, + issuerToActiveJWKMap map[string]*jose.JSONWebKey, +) { p.mutex.Lock() // acquire a write lock defer p.mutex.Unlock() p.issuerToJWKSMap = issuerToJWKSMap + p.issuerToActiveJWKMap = issuerToActiveJWKMap } -func (p *dynamicJWKSProvider) GetJWKS(issuerName string) *jose.JSONWebKeySet { +func (p *dynamicJWKSProvider) GetJWKS(issuerName string) (*jose.JSONWebKeySet, *jose.JSONWebKey) { p.mutex.RLock() // acquire a read lock defer p.mutex.RUnlock() - return p.issuerToJWKSMap[issuerName] + return p.issuerToJWKSMap[issuerName], p.issuerToActiveJWKMap[issuerName] } diff --git a/internal/oidc/jwks/jwks_handler.go b/internal/oidc/jwks/jwks_handler.go index bdb036e6..2c975e95 100644 --- a/internal/oidc/jwks/jwks_handler.go +++ b/internal/oidc/jwks/jwks_handler.go @@ -19,7 +19,7 @@ func NewHandler(issuerName string, provider DynamicJWKSProvider) http.Handler { return } - jwks := provider.GetJWKS(issuerName) + jwks, _ := provider.GetJWKS(issuerName) if jwks == nil { http.Error(w, "JWKS not found for requested issuer", http.StatusNotFound) diff --git a/internal/oidc/jwks/jwks_handler_test.go b/internal/oidc/jwks/jwks_handler_test.go index d80e66d5..37f53e8c 100644 --- a/internal/oidc/jwks/jwks_handler_test.go +++ b/internal/oidc/jwks/jwks_handler_test.go @@ -105,8 +105,12 @@ func newDynamicJWKSProvider(t *testing.T, issuer string, jwksJSON string) Dynami var keySet jose.JSONWebKeySet err := json.Unmarshal([]byte(jwksJSON), &keySet) require.NoError(t, err) - jwksProvider.SetIssuerToJWKSMap(map[string]*jose.JSONWebKeySet{ + issuerToJWKSMap := map[string]*jose.JSONWebKeySet{ issuer: &keySet, - }) + } + issuerToActiveJWKMap := map[string]*jose.JSONWebKey{ + issuer: &keySet.Keys[0], + } + jwksProvider.SetIssuerToJWKSMap(issuerToJWKSMap, issuerToActiveJWKMap) return jwksProvider } diff --git a/internal/oidc/oidc.go b/internal/oidc/oidc.go index 14f9a725..c5e6abf0 100644 --- a/internal/oidc/oidc.go +++ b/internal/oidc/oidc.go @@ -5,13 +5,13 @@ package oidc import ( - "crypto/ecdsa" "time" "github.com/ory/fosite" "github.com/ory/fosite/compose" "go.pinniped.dev/internal/oidc/csrftoken" + "go.pinniped.dev/internal/oidc/jwks" "go.pinniped.dev/internal/oidc/provider" "go.pinniped.dev/pkg/oidcclient/nonce" "go.pinniped.dev/pkg/oidcclient/pkce" @@ -94,7 +94,7 @@ func FositeOauth2Helper( oauthStore interface{}, issuer string, hmacSecretOfLengthAtLeast32 []byte, - jwtSigningKey *ecdsa.PrivateKey, + jwksProvider jwks.DynamicJWKSProvider, ) fosite.OAuth2Provider { oauthConfig := &compose.Config{ AuthorizeCodeLifespan: 3 * time.Minute, // seems more than long enough to exchange a code @@ -122,7 +122,8 @@ func FositeOauth2Helper( &compose.CommonStrategy{ // Note that Fosite requires the HMAC secret to be at least 32 bytes. CoreStrategy: compose.NewOAuth2HMACStrategy(oauthConfig, hmacSecretOfLengthAtLeast32, nil), - OpenIDConnectTokenStrategy: compose.NewOpenIDConnectECDSAStrategy(oauthConfig, jwtSigningKey), + OpenIDConnectTokenStrategy: newDynamicOpenIDConnectECDSAStrategy(issuer, oauthConfig, jwksProvider), + // OpenIDConnectTokenStrategy: compose.NewOpenIDConnectECDSAStrategy(oauthConfig, jwtSigningKey), }, nil, // hasher, defaults to using BCrypt when nil. Used for hashing client secrets. compose.OAuth2AuthorizeExplicitFactory, diff --git a/internal/oidc/provider/manager/manager.go b/internal/oidc/provider/manager/manager.go index 1d80a0fc..b1d9d6b5 100644 --- a/internal/oidc/provider/manager/manager.go +++ b/internal/oidc/provider/manager/manager.go @@ -18,6 +18,7 @@ import ( "go.pinniped.dev/internal/oidc/discovery" "go.pinniped.dev/internal/oidc/jwks" "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/oidc/token" "go.pinniped.dev/internal/plog" "go.pinniped.dev/pkg/oidcclient/nonce" "go.pinniped.dev/pkg/oidcclient/pkce" @@ -115,6 +116,10 @@ func (m *Manager) SetProviders(oidcProviders ...*provider.OIDCProvider) { issuer+oidc.CallbackEndpointPath, ) + m.providerHandlers[(issuerHostWithPath + oidc.TokenEndpointPath)] = token.NewHandler( + oauthHelperWithKubeStorage, + ) + plog.Debug("oidc provider manager added or updated issuer", "issuer", issuer) } } diff --git a/internal/oidc/provider/manager/manager_test.go b/internal/oidc/provider/manager/manager_test.go index a3f8090d..d3cf1cc9 100644 --- a/internal/oidc/provider/manager/manager_test.go +++ b/internal/oidc/provider/manager/manager_test.go @@ -5,7 +5,12 @@ package manager import ( "context" + "crypto" + "crypto/ecdsa" + "crypto/sha256" + "encoding/base64" "encoding/json" + "fmt" "io/ioutil" "net/http" "net/http/httptest" @@ -13,6 +18,7 @@ import ( "strings" "testing" + coreosoidc "github.com/coreos/go-oidc" "github.com/sclevine/spec" "github.com/stretchr/testify/require" "gopkg.in/square/go-jose.v2" @@ -48,13 +54,22 @@ func TestManager(t *testing.T) { issuer2DifferentCaseHostname = "https://exAmPlE.Com/some/path/more/deeply/nested/path" issuer2KeyID = "issuer2-key" upstreamIDPAuthorizationURL = "https://test-upstream.com/auth" + downstreamClientID = "pinniped-cli" downstreamRedirectURL = "http://127.0.0.1:12345/callback" + + downstreamPKCECodeVerifier = "some-pkce-verifier-that-must-be-at-least-43-characters-to-meet-entropy-requirements" ) newGetRequest := func(url string) *http.Request { return httptest.NewRequest(http.MethodGet, url, nil) } + newPostRequest := func(url, body string) *http.Request { + req := httptest.NewRequest(http.MethodPost, url, strings.NewReader(body)) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + return req + } + requireDiscoveryRequestToBeHandled := func(requestIssuer, requestURLSuffix, expectedIssuerInResponse string) { recorder := httptest.NewRecorder() @@ -106,7 +121,7 @@ func TestManager(t *testing.T) { return csrfCookieValue, redirectStateParam } - requireCallbackRequestToBeHandled := func(requestIssuer, requestURLSuffix, csrfCookieValue string) { + requireCallbackRequestToBeHandled := func(requestIssuer, requestURLSuffix, csrfCookieValue string) string { recorder := httptest.NewRecorder() numberOfKubeActionsBeforeThisRequest := len(kubeClient.Actions()) @@ -139,9 +154,56 @@ func TestManager(t *testing.T) { // Make sure that we wired up the callback endpoint to use kube storage for fosite sessions. r.Equal(len(kubeClient.Actions()), numberOfKubeActionsBeforeThisRequest+3, "did not perform any kube actions during the callback request, but should have") + + // Return the important parts of the response so we can use them in our next request to the token endpoint. + return actualLocationQueryParams.Get("code") } - requireJWKSRequestToBeHandled := func(requestIssuer, requestURLSuffix, expectedJWKKeyID string) { + requireTokenRequestToBeHandled := func(requestIssuer, authCode string, jwks *jose.JSONWebKeySet) { + recorder := httptest.NewRecorder() + + numberOfKubeActionsBeforeThisRequest := len(kubeClient.Actions()) + + tokenRequestBody := url.Values{ + "code": []string{authCode}, + "client_id": []string{downstreamClientID}, + "redirect_uri": []string{downstreamRedirectURL}, + "code_verifier": []string{downstreamPKCECodeVerifier}, + "grant_type": []string{"authorization_code"}, + }.Encode() + subject.ServeHTTP(recorder, newPostRequest(requestIssuer+oidc.TokenEndpointPath, tokenRequestBody)) + + r.False(fallbackHandlerWasCalled) + + // Minimal check to ensure that the right endpoint was called + var body map[string]interface{} + r.Equal(http.StatusOK, recorder.Code) + r.NoError(json.Unmarshal(recorder.Body.Bytes(), &body)) + r.Contains(body, "id_token") + r.Contains(body, "access_token") + + // 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"]) + + r.GreaterOrEqual(len(jwks.Keys), 1) + 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(requestIssuer, keySet, &verifyConfig) + _, err := verifier.Verify(context.Background(), idToken) + r.NoError(err) + + // Make sure that we wired up the callback endpoint to use kube storage for fosite sessions. + r.Equal(len(kubeClient.Actions()), numberOfKubeActionsBeforeThisRequest+7, + "did not perform any kube actions during the callback request, but should have") + } + + requireJWKSRequestToBeHandled := func(requestIssuer, requestURLSuffix, expectedJWKKeyID string) *jose.JSONWebKeySet { recorder := httptest.NewRecorder() subject.ServeHTTP(recorder, newGetRequest(requestIssuer+oidc.JWKSEndpointPath+requestURLSuffix)) @@ -156,6 +218,8 @@ func TestManager(t *testing.T) { err = json.Unmarshal(responseBody, &parsedJWKSResult) r.NoError(err) r.Equal(expectedJWKKeyID, parsedJWKSResult.Keys[0].KeyID) + + return &parsedJWKSResult } it.Before(func() { @@ -198,7 +262,7 @@ func TestManager(t *testing.T) { }) }) - newTestJWK := func(keyID string) jose.JSONWebKey { + newTestJWK := func(keyID string) *jose.JSONWebKey { testJWKSJSONString := here.Docf(` { "use": "sig", @@ -206,13 +270,14 @@ func TestManager(t *testing.T) { "kid": "%s", "crv": "P-256", "alg": "ES256", - "x": "awmmj6CIMhSoJyfsqH7sekbTeY72GGPLEy16tPWVz2U", - "y": "FcMh06uXLaq9b2MOixlLVidUkycO1u7IHOkrTi7N0aw" + "x": "9c_oMKjd_ruVIy4pA5y9quT1E-Fampx0w270OtPx5Ng", + "y": "-Y-9nfrtJdFPl-9kzXv55-Fq9Oo2AWDg5zZBs9P-Vds", + "d": "LXdNChAEtGKETBzYXiL_G0wESXceBuajE_EBQbcRuGg" } `, keyID) k := jose.JSONWebKey{} r.NoError(json.Unmarshal([]byte(testJWKSJSONString), &k)) - return k + return &k } requireRoutesMatchingRequestsToAppropriateProvider := func() { @@ -225,8 +290,8 @@ func TestManager(t *testing.T) { requireDiscoveryRequestToBeHandled(issuer2DifferentCaseHostname, "", issuer2) requireDiscoveryRequestToBeHandled(issuer2DifferentCaseHostname, "?some=query", issuer2) - requireJWKSRequestToBeHandled(issuer1, "", issuer1KeyID) - requireJWKSRequestToBeHandled(issuer2, "", issuer2KeyID) + issuer1JWKS := requireJWKSRequestToBeHandled(issuer1, "", issuer1KeyID) + issuer2JWKS := requireJWKSRequestToBeHandled(issuer2, "", issuer2KeyID) requireJWKSRequestToBeHandled(issuer2, "?some=query", issuer2KeyID) // Hostnames are case-insensitive, so test that we can handle that. @@ -237,10 +302,10 @@ func TestManager(t *testing.T) { authRequestParams := "?" + url.Values{ "response_type": []string{"code"}, "scope": []string{"openid profile email"}, - "client_id": []string{"pinniped-cli"}, + "client_id": []string{downstreamClientID}, "state": []string{"some-state-value-that-is-32-byte"}, - "nonce": []string{"some-nonce-value"}, - "code_challenge": []string{"some-challenge"}, + "nonce": []string{"some-nonce-value-that-is-at-least-32-bytes"}, + "code_challenge": []string{doSHA256(downstreamPKCECodeVerifier)}, "code_challenge_method": []string{"S256"}, "redirect_uri": []string{downstreamRedirectURL}, }.Encode() @@ -258,12 +323,19 @@ func TestManager(t *testing.T) { "state": []string{upstreamStateParam}, }.Encode() - requireCallbackRequestToBeHandled(issuer1, callbackRequestParams, csrfCookieValue) - requireCallbackRequestToBeHandled(issuer2, callbackRequestParams, csrfCookieValue) + downstreamAuthCode1 := requireCallbackRequestToBeHandled(issuer1, callbackRequestParams, csrfCookieValue) + downstreamAuthCode2 := requireCallbackRequestToBeHandled(issuer2, callbackRequestParams, csrfCookieValue) // // Hostnames are case-insensitive, so test that we can handle that. - requireCallbackRequestToBeHandled(issuer1DifferentCaseHostname, callbackRequestParams, csrfCookieValue) - requireCallbackRequestToBeHandled(issuer2DifferentCaseHostname, callbackRequestParams, csrfCookieValue) + downstreamAuthCode3 := requireCallbackRequestToBeHandled(issuer1DifferentCaseHostname, callbackRequestParams, csrfCookieValue) + downstreamAuthCode4 := requireCallbackRequestToBeHandled(issuer2DifferentCaseHostname, callbackRequestParams, csrfCookieValue) + + requireTokenRequestToBeHandled(issuer1, downstreamAuthCode1, issuer1JWKS) + requireTokenRequestToBeHandled(issuer2, downstreamAuthCode2, issuer2JWKS) + + // Hostnames are case-insensitive, so test that we can handle that. + requireTokenRequestToBeHandled(issuer1DifferentCaseHostname, downstreamAuthCode3, issuer1JWKS) + requireTokenRequestToBeHandled(issuer2DifferentCaseHostname, downstreamAuthCode4, issuer2JWKS) } when("given some valid providers via SetProviders()", func() { @@ -274,10 +346,15 @@ func TestManager(t *testing.T) { r.NoError(err) subject.SetProviders(p1, p2) - dynamicJWKSProvider.SetIssuerToJWKSMap(map[string]*jose.JSONWebKeySet{ - issuer1: {Keys: []jose.JSONWebKey{newTestJWK(issuer1KeyID)}}, - issuer2: {Keys: []jose.JSONWebKey{newTestJWK(issuer2KeyID)}}, - }) + jwks := map[string]*jose.JSONWebKeySet{ + issuer1: {Keys: []jose.JSONWebKey{*newTestJWK(issuer1KeyID)}}, + issuer2: {Keys: []jose.JSONWebKey{*newTestJWK(issuer2KeyID)}}, + } + activeJWK := map[string]*jose.JSONWebKey{ + issuer1: newTestJWK(issuer1KeyID), + issuer2: newTestJWK(issuer2KeyID), + } + dynamicJWKSProvider.SetIssuerToJWKSMap(jwks, activeJWK) }) it("sends all non-matching host requests to the nextHandler", func() { @@ -312,10 +389,15 @@ func TestManager(t *testing.T) { r.NoError(err) subject.SetProviders(p2, p1) - dynamicJWKSProvider.SetIssuerToJWKSMap(map[string]*jose.JSONWebKeySet{ - issuer1: {Keys: []jose.JSONWebKey{newTestJWK(issuer1KeyID)}}, - issuer2: {Keys: []jose.JSONWebKey{newTestJWK(issuer2KeyID)}}, - }) + jwks := map[string]*jose.JSONWebKeySet{ + issuer1: {Keys: []jose.JSONWebKey{*newTestJWK(issuer1KeyID)}}, + issuer2: {Keys: []jose.JSONWebKey{*newTestJWK(issuer2KeyID)}}, + } + activeJWK := map[string]*jose.JSONWebKey{ + issuer1: newTestJWK(issuer1KeyID), + issuer2: newTestJWK(issuer2KeyID), + } + dynamicJWKSProvider.SetIssuerToJWKSMap(jwks, activeJWK) }) it("still routes matching requests to the appropriate provider", func() { @@ -324,3 +406,24 @@ 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_test.go b/internal/oidc/token/token_handler_test.go index fd52004e..ea64cc1b 100644 --- a/internal/oidc/token/token_handler_test.go +++ b/internal/oidc/token/token_handler_test.go @@ -591,6 +591,7 @@ 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[:]) @@ -856,6 +857,7 @@ func requireValidIDToken(t *testing.T, body map[string]interface{}, jwtSigningKe requireTimeInDelta(t, goodAuthTime, authTime, timeComparisonFudgeSeconds*time.Second) } +// TODO: de-dup me (manager test). func newStaticKeySet(publicKey crypto.PublicKey) coreosoidc.KeySet { return &staticKeySet{publicKey} }