From f6ded84f07d467ab0b0dabe20dc32fb9ae5caa96 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Thu, 8 Apr 2021 17:28:01 -0700 Subject: [PATCH] Implement upstream LDAP support in auth_handler.go - When the upstream IDP is an LDAP IDP and the user's LDAP username and password are received as new custom headers, then authenticate the user and, if authentication was successful, return a redirect with an authcode. Handle errors according to the OAuth/OIDC specs. - Still does not support having multiple upstream IDPs defined at the same time, which was an existing limitation of this endpoint. - Does not yet include the actual LDAP authentication, which is hidden behind an interface from the point of view of auth_handler.go - Move the oidctestutil package to the testutil directory. - Add an interface for Fosite storage to avoid a cyclical test dependency. - Add GetURL() to the UpstreamLDAPIdentityProviderI interface. - Extract test helpers to be shared between callback_handler_test.go and auth_handler_test.go because the authcode and fosite storage assertions should be identical. - Backfill Content-Type assertions in callback_handler_test.go. Signed-off-by: Andrew Keesler --- .../upstreamwatcher/upstreamwatcher_test.go | 2 +- .../fosite_sotrage_interface.go | 21 + internal/ldap/ldap.go | 15 + internal/oidc/auth/auth_handler.go | 317 ++++++---- internal/oidc/auth/auth_handler_test.go | 573 +++++++++++++++--- .../oidc/callback/callback_handler_test.go | 405 ++++--------- .../dynamic_open_id_connect_ecdsa_strategy.go | 7 +- ...mic_open_id_connect_ecdsa_strategy_test.go | 4 +- internal/oidc/kube_storage.go | 5 +- internal/oidc/nullstorage.go | 5 +- internal/oidc/oidctestutil/oidc.go | 187 ------ .../provider/dynamic_upstream_idp_provider.go | 5 + .../oidc/provider/manager/manager_test.go | 2 +- internal/oidc/token/token_handler_test.go | 45 +- .../testutil/oidctestutil/oidctestutil.go | 469 ++++++++++++++ 15 files changed, 1360 insertions(+), 702 deletions(-) create mode 100644 internal/fositestoragei/fosite_sotrage_interface.go delete mode 100644 internal/oidc/oidctestutil/oidc.go create mode 100644 internal/testutil/oidctestutil/oidctestutil.go diff --git a/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher_test.go b/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher_test.go index 0b199a11..bb0cf454 100644 --- a/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher_test.go +++ b/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher_test.go @@ -24,9 +24,9 @@ import ( pinnipedfake "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/fake" pinnipedinformers "go.pinniped.dev/generated/latest/client/supervisor/informers/externalversions" "go.pinniped.dev/internal/controllerlib" - "go.pinniped.dev/internal/oidc/oidctestutil" "go.pinniped.dev/internal/oidc/provider" "go.pinniped.dev/internal/testutil" + "go.pinniped.dev/internal/testutil/oidctestutil" "go.pinniped.dev/internal/testutil/testlogger" "go.pinniped.dev/internal/upstreamoidc" ) diff --git a/internal/fositestoragei/fosite_sotrage_interface.go b/internal/fositestoragei/fosite_sotrage_interface.go new file mode 100644 index 00000000..408dfb28 --- /dev/null +++ b/internal/fositestoragei/fosite_sotrage_interface.go @@ -0,0 +1,21 @@ +// Copyright 2021 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package fositestoragei + +import ( + "github.com/ory/fosite" + "github.com/ory/fosite/handler/oauth2" + "github.com/ory/fosite/handler/openid" + "github.com/ory/fosite/handler/pkce" +) + +// This interface seems to be missing from Fosite. +// Not having this interface makes it a pain to avoid cyclical test dependencies, so we'll define it. +type AllFositeStorage interface { + fosite.ClientManager + oauth2.CoreStorage + oauth2.TokenRevocationStorage + openid.OpenIDConnectRequestStorage + pkce.PKCERequestStorage +} diff --git a/internal/ldap/ldap.go b/internal/ldap/ldap.go index 182971d4..a5899be2 100644 --- a/internal/ldap/ldap.go +++ b/internal/ldap/ldap.go @@ -13,6 +13,21 @@ import ( // This interface is similar to the k8s token authenticator, but works with username/passwords instead // of a single token string. // +// The return values should be as follows. +// 1. For a successful authentication: +// - A response which includes the username, uid, and groups in the userInfo. The username and uid must not be blank. +// - true +// - nil error +// 2. For an unsuccessful authentication, e.g. bad username or password: +// - nil response +// - false +// - nil error +// 3. For an unexpected error, e.g. a network problem: +// - nil response +// - false +// - an error +// Other combinations of return values must be avoided. +// // See k8s.io/apiserver/pkg/authentication/authenticator/interfaces.go for the token authenticator // interface, as well as the Response type. type UserAuthenticator interface { diff --git a/internal/oidc/auth/auth_handler.go b/internal/oidc/auth/auth_handler.go index 7b007863..701d64da 100644 --- a/internal/oidc/auth/auth_handler.go +++ b/internal/oidc/auth/auth_handler.go @@ -13,6 +13,7 @@ import ( "github.com/ory/fosite" "github.com/ory/fosite/handler/openid" "github.com/ory/fosite/token/jwt" + "github.com/pkg/errors" "golang.org/x/oauth2" "go.pinniped.dev/internal/httputil/httperr" @@ -25,11 +26,16 @@ import ( "go.pinniped.dev/pkg/oidcclient/pkce" ) +const ( + CustomUsernameHeaderName = "X-Pinniped-Upstream-Username" + CustomPasswordHeaderName = "X-Pinniped-Upstream-Password" //nolint:gosec // this is not a credential +) + func NewHandler( downstreamIssuer string, idpLister oidc.UpstreamIdentityProvidersLister, - oauthHelperWithNullStorage fosite.OAuth2Provider, - oauthHelperWithRealStorage fosite.OAuth2Provider, + oauthHelperWithoutStorage fosite.OAuth2Provider, + oauthHelperWithStorage fosite.OAuth2Provider, generateCSRF func() (csrftoken.CSRFToken, error), generatePKCE func() (pkce.Code, error), generateNonce func() (nonce.Nonce, error), @@ -44,109 +50,203 @@ func NewHandler( return httperr.Newf(http.StatusMethodNotAllowed, "%s (try GET or POST)", r.Method) } - csrfFromCookie := readCSRFCookie(r, cookieCodec) - - authorizeRequester, err := oauthHelperWithNullStorage.NewAuthorizeRequest(r.Context(), r) - if err != nil { - plog.Info("authorize request error", oidc.FositeErrorForLog(err)...) - oauthHelperWithNullStorage.WriteAuthorizeError(w, authorizeRequester, err) - return nil - } - - upstreamIDP, err := chooseUpstreamIDP(idpLister) + oidcUpstream, ldapUpstream, err := chooseUpstreamIDP(idpLister) if err != nil { plog.WarningErr("authorize upstream config", err) return err } - // Grant the openid scope (for now) if they asked for it so that `NewAuthorizeResponse` will perform its OIDC validations. - oidc.GrantScopeIfRequested(authorizeRequester, coreosoidc.ScopeOpenID) - // There don't seem to be any validations inside `NewAuthorizeResponse` related to the offline_access scope - // at this time, however we will temporarily grant the scope just in case that changes in a future release of fosite. - oidc.GrantScopeIfRequested(authorizeRequester, coreosoidc.ScopeOfflineAccess) - - // Grant the pinniped:request-audience scope if requested. - oidc.GrantScopeIfRequested(authorizeRequester, "pinniped:request-audience") - - now := time.Now() - _, err = oauthHelperWithNullStorage.NewAuthorizeResponse(r.Context(), authorizeRequester, &openid.DefaultSession{ - Claims: &jwt.IDTokenClaims{ - // Temporary claim values to allow `NewAuthorizeResponse` to perform other OIDC validations. - Subject: "none", - AuthTime: now, - RequestedAt: now, - }, - }) - if err != nil { - plog.Info("authorize response error", oidc.FositeErrorForLog(err)...) - oauthHelperWithNullStorage.WriteAuthorizeError(w, authorizeRequester, err) - return nil + if oidcUpstream != nil { + return handleAuthRequestForOIDCUpstream(r, w, + oauthHelperWithoutStorage, + generateCSRF, generateNonce, generatePKCE, + oidcUpstream, + downstreamIssuer, + upstreamStateEncoder, + cookieCodec, + ) } - - csrfValue, nonceValue, pkceValue, err := generateValues(generateCSRF, generateNonce, generatePKCE) - if err != nil { - plog.Error("authorize generate error", err) - return err - } - if csrfFromCookie != "" { - csrfValue = csrfFromCookie - } - - upstreamOAuthConfig := oauth2.Config{ - ClientID: upstreamIDP.GetClientID(), - Endpoint: oauth2.Endpoint{ - AuthURL: upstreamIDP.GetAuthorizationURL().String(), - }, - RedirectURL: fmt.Sprintf("%s/callback", downstreamIssuer), - Scopes: upstreamIDP.GetScopes(), - } - - encodedStateParamValue, err := upstreamStateParam( - authorizeRequester, - upstreamIDP.GetName(), - nonceValue, - csrfValue, - pkceValue, - upstreamStateEncoder, + return handleAuthRequestForLDAPUpstream(r, w, + oauthHelperWithStorage, + ldapUpstream, ) - if err != nil { - plog.Error("authorize upstream state param error", err) - return err - } - - if csrfFromCookie == "" { - // We did not receive an incoming CSRF cookie, so write a new one. - err := addCSRFSetCookieHeader(w, csrfValue, cookieCodec) - if err != nil { - plog.Error("error setting CSRF cookie", err) - return err - } - } - - authCodeOptions := []oauth2.AuthCodeOption{ - oauth2.AccessTypeOffline, - nonceValue.Param(), - pkceValue.Challenge(), - pkceValue.Method(), - } - - promptParam := r.Form.Get("prompt") - if promptParam != "" && oidc.ScopeWasRequested(authorizeRequester, coreosoidc.ScopeOpenID) { - authCodeOptions = append(authCodeOptions, oauth2.SetAuthURLParam("prompt", promptParam)) - } - - http.Redirect(w, r, - upstreamOAuthConfig.AuthCodeURL( - encodedStateParamValue, - authCodeOptions..., - ), - 302, - ) - - return nil })) } +func handleAuthRequestForLDAPUpstream( + r *http.Request, + w http.ResponseWriter, + oauthHelper fosite.OAuth2Provider, + ldapUpstream provider.UpstreamLDAPIdentityProviderI, +) error { + authorizeRequester, created := newAuthorizeRequest(r, w, oauthHelper) + if !created { + return nil + } + + username := r.Header.Get(CustomUsernameHeaderName) + password := r.Header.Get(CustomPasswordHeaderName) + if username == "" || password == "" { + // Return an error according to OIDC spec 3.1.2.6 (second paragraph). + err := errors.WithStack(fosite.ErrAccessDenied.WithHintf("Missing or blank username or password.")) + oauthHelper.WriteAuthorizeError(w, authorizeRequester, err) + return nil + } + + authenticateResponse, authenticated, err := ldapUpstream.AuthenticateUser(r.Context(), username, password) + if err != nil { + plog.WarningErr("unexpected error during upstream authentication", err, "upstreamName", ldapUpstream.GetName()) + return httperr.New(http.StatusBadGateway, "unexpected error during upstream authentication") + } + if !authenticated { + // Return an error according to OIDC spec 3.1.2.6 (second paragraph). + err = errors.WithStack(fosite.ErrAccessDenied.WithHintf("Username/password not accepted by LDAP provider.")) + oauthHelper.WriteAuthorizeError(w, authorizeRequester, err) + return nil + } + + subject := fmt.Sprintf("%s?%s=%s", ldapUpstream.GetURL(), oidc.IDTokenSubjectClaim, authenticateResponse.User.GetUID()) + now := time.Now().UTC() + openIDSession := &openid.DefaultSession{ + Claims: &jwt.IDTokenClaims{ + Subject: subject, + RequestedAt: now, + AuthTime: now, + }, + } + openIDSession.Claims.Extra = map[string]interface{}{ + oidc.DownstreamUsernameClaim: authenticateResponse.User.GetName(), + oidc.DownstreamGroupsClaim: authenticateResponse.User.GetGroups(), + } + + authorizeResponder, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, openIDSession) + if err != nil { + plog.Info("authorize response error", oidc.FositeErrorForLog(err)...) + oauthHelper.WriteAuthorizeError(w, authorizeRequester, err) + return nil + } + + oauthHelper.WriteAuthorizeResponse(w, authorizeRequester, authorizeResponder) + return nil +} + +func handleAuthRequestForOIDCUpstream( + r *http.Request, + w http.ResponseWriter, + oauthHelper fosite.OAuth2Provider, + generateCSRF func() (csrftoken.CSRFToken, error), + generateNonce func() (nonce.Nonce, error), + generatePKCE func() (pkce.Code, error), + oidcUpstream provider.UpstreamOIDCIdentityProviderI, + downstreamIssuer string, + upstreamStateEncoder oidc.Encoder, + cookieCodec oidc.Codec, +) error { + authorizeRequester, created := newAuthorizeRequest(r, w, oauthHelper) + if !created { + return nil + } + + now := time.Now() + _, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, &openid.DefaultSession{ + Claims: &jwt.IDTokenClaims{ + // Temporary claim values to allow `NewAuthorizeResponse` to perform other OIDC validations. + Subject: "none", + AuthTime: now, + RequestedAt: now, + }, + }) + if err != nil { + plog.Info("authorize response error", oidc.FositeErrorForLog(err)...) + oauthHelper.WriteAuthorizeError(w, authorizeRequester, err) + return nil + } + + csrfValue, nonceValue, pkceValue, err := generateValues(generateCSRF, generateNonce, generatePKCE) + if err != nil { + plog.Error("authorize generate error", err) + return err + } + csrfFromCookie := readCSRFCookie(r, cookieCodec) + if csrfFromCookie != "" { + csrfValue = csrfFromCookie + } + + upstreamOAuthConfig := oauth2.Config{ + ClientID: oidcUpstream.GetClientID(), + Endpoint: oauth2.Endpoint{ + AuthURL: oidcUpstream.GetAuthorizationURL().String(), + }, + RedirectURL: fmt.Sprintf("%s/callback", downstreamIssuer), + Scopes: oidcUpstream.GetScopes(), + } + + encodedStateParamValue, err := upstreamStateParam( + authorizeRequester, + oidcUpstream.GetName(), + nonceValue, + csrfValue, + pkceValue, + upstreamStateEncoder, + ) + if err != nil { + plog.Error("authorize upstream state param error", err) + return err + } + + if csrfFromCookie == "" { + // We did not receive an incoming CSRF cookie, so write a new one. + err := addCSRFSetCookieHeader(w, csrfValue, cookieCodec) + if err != nil { + plog.Error("error setting CSRF cookie", err) + return err + } + } + + authCodeOptions := []oauth2.AuthCodeOption{ + oauth2.AccessTypeOffline, + nonceValue.Param(), + pkceValue.Challenge(), + pkceValue.Method(), + } + + promptParam := r.Form.Get("prompt") + if promptParam != "" && oidc.ScopeWasRequested(authorizeRequester, coreosoidc.ScopeOpenID) { + authCodeOptions = append(authCodeOptions, oauth2.SetAuthURLParam("prompt", promptParam)) + } + + http.Redirect(w, r, + upstreamOAuthConfig.AuthCodeURL( + encodedStateParamValue, + authCodeOptions..., + ), + 302, + ) + + return nil +} + +func newAuthorizeRequest(r *http.Request, w http.ResponseWriter, oauthHelper fosite.OAuth2Provider) (fosite.AuthorizeRequester, bool) { + authorizeRequester, err := oauthHelper.NewAuthorizeRequest(r.Context(), r) + if err != nil { + plog.Info("authorize request error", oidc.FositeErrorForLog(err)...) + oauthHelper.WriteAuthorizeError(w, authorizeRequester, err) + return nil, false + } + grantScopes(authorizeRequester) + return authorizeRequester, true +} + +func grantScopes(authorizeRequester fosite.AuthorizeRequester) { + // Grant the openid scope (for now) if they asked for it so that `NewAuthorizeResponse` will perform its OIDC validations. + oidc.GrantScopeIfRequested(authorizeRequester, coreosoidc.ScopeOpenID) + // There don't seem to be any validations inside `NewAuthorizeResponse` related to the offline_access scope + // at this time, however we will temporarily grant the scope just in case that changes in a future release of fosite. + oidc.GrantScopeIfRequested(authorizeRequester, coreosoidc.ScopeOfflineAccess) + // Grant the pinniped:request-audience scope if requested. + oidc.GrantScopeIfRequested(authorizeRequester, "pinniped:request-audience") +} + func readCSRFCookie(r *http.Request, codec oidc.Decoder) csrftoken.CSRFToken { receivedCSRFCookie, err := r.Cookie(oidc.CSRFCookieName) if err != nil { @@ -166,27 +266,34 @@ func readCSRFCookie(r *http.Request, codec oidc.Decoder) csrftoken.CSRFToken { return csrfFromCookie } -func chooseUpstreamIDP(idpLister oidc.UpstreamOIDCIdentityProvidersLister) (provider.UpstreamOIDCIdentityProviderI, error) { - allUpstreamIDPs := idpLister.GetOIDCIdentityProviders() - if len(allUpstreamIDPs) == 0 { - return nil, httperr.New( +// Select either an OIDC or an LDAP IDP, or return an error. +func chooseUpstreamIDP(idpLister oidc.UpstreamIdentityProvidersLister) (provider.UpstreamOIDCIdentityProviderI, provider.UpstreamLDAPIdentityProviderI, error) { + oidcUpstreams := idpLister.GetOIDCIdentityProviders() + ldapUpstreams := idpLister.GetLDAPIdentityProviders() + switch { + case len(oidcUpstreams)+len(ldapUpstreams) == 0: + return nil, nil, httperr.New( http.StatusUnprocessableEntity, "No upstream providers are configured", ) - } else if len(allUpstreamIDPs) > 1 { + case len(oidcUpstreams)+len(ldapUpstreams) > 1: var upstreamIDPNames []string - for _, idp := range allUpstreamIDPs { + for _, idp := range oidcUpstreams { + upstreamIDPNames = append(upstreamIDPNames, idp.GetName()) + } + for _, idp := range ldapUpstreams { upstreamIDPNames = append(upstreamIDPNames, idp.GetName()) } - plog.Warning("Too many upstream providers are configured (found: %s)", upstreamIDPNames) - - return nil, httperr.New( + return nil, nil, httperr.New( http.StatusUnprocessableEntity, "Too many upstream providers are configured (support for multiple upstreams is not yet implemented)", ) + case len(oidcUpstreams) == 1: + return oidcUpstreams[0], nil, nil + default: + return nil, ldapUpstreams[0], nil } - return allUpstreamIDPs[0], nil } func generateValues( diff --git a/internal/oidc/auth/auth_handler_test.go b/internal/oidc/auth/auth_handler_test.go index 0be99023..2a597881 100644 --- a/internal/oidc/auth/auth_handler_test.go +++ b/internal/oidc/auth/auth_handler_test.go @@ -4,6 +4,7 @@ package auth import ( + "context" "fmt" "html" "net/http" @@ -13,18 +14,21 @@ import ( "strings" "testing" - "k8s.io/client-go/kubernetes/fake" - "github.com/gorilla/securecookie" + "github.com/ory/fosite" "github.com/stretchr/testify/require" + "k8s.io/apiserver/pkg/authentication/authenticator" + "k8s.io/apiserver/pkg/authentication/user" + "k8s.io/client-go/kubernetes/fake" + v1 "k8s.io/client-go/kubernetes/typed/core/v1" "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/internal/testutil/oidctestutil" "go.pinniped.dev/pkg/oidcclient/nonce" "go.pinniped.dev/pkg/oidcclient/pkce" ) @@ -34,7 +38,13 @@ func TestAuthorizationEndpoint(t *testing.T) { downstreamIssuer = "https://my-downstream-issuer.com/some-path" downstreamRedirectURI = "http://127.0.0.1/callback" downstreamRedirectURIWithDifferentPort = "http://127.0.0.1:42/callback" + downstreamNonce = "some-nonce-value" + downstreamPKCEChallenge = "some-challenge" + downstreamPKCEChallengeMethod = "S256" happyState = "8b-state" + downstreamClientID = "pinniped-cli" + upstreamLDAPURL = "ldaps://some-ldap-host:123" + htmlContentType = "text/html; charset=utf-8" ) require.Len(t, happyState, 8, "we expect fosite to allow 8 byte state params, so we want to test that boundary case") @@ -101,20 +111,31 @@ func TestAuthorizationEndpoint(t *testing.T) { "error_description": "The authorization server does not support obtaining a token using this method. `The request is missing the 'response_type' parameter.", "state": happyState, } - ) - kubeClient := fake.NewSimpleClientset() - secretsClient := kubeClient.CoreV1().Secrets("some-namespace") + fositeAccessDeniedWithBadUsernamePasswordHintErrorQuery = map[string]string{ + "error": "access_denied", + "error_description": "The resource owner or authorization server denied the request. Username/password not accepted by LDAP provider.", + "state": happyState, + } + + fositeAccessDeniedWithMissingUsernamePasswordHintErrorQuery = map[string]string{ + "error": "access_denied", + "error_description": "The resource owner or authorization server denied the request. Missing or blank username or password.", + "state": happyState, + } + ) hmacSecretFunc := func() []byte { return []byte("some secret - must have at least 32 bytes") } require.GreaterOrEqual(t, len(hmacSecretFunc()), 32, "fosite requires that hmac secrets have at least 32 bytes") jwksProviderIsUnused := jwks.NewDynamicJWKSProvider() - - // Configure fosite the same way that the production code would when using Kube storage. - // Inject this into our test subject at the last second so we get a fresh storage for every test. timeoutsConfiguration := oidc.DefaultOIDCTimeoutsConfiguration() - kubeOauthStore := oidc.NewKubeStorage(secretsClient, timeoutsConfiguration) - oauthHelperWithRealStorage := oidc.FositeOauth2Helper(kubeOauthStore, downstreamIssuer, hmacSecretFunc, jwksProviderIsUnused, timeoutsConfiguration) + + createOauthHelperWithRealStorage := func(secretsClient v1.SecretInterface) (fosite.OAuth2Provider, *oidc.KubeStorage) { + // Configure fosite the same way that the production code would when using Kube storage. + // Inject this into our test subject at the last second so we get a fresh storage for every test. + kubeOauthStore := oidc.NewKubeStorage(secretsClient, timeoutsConfiguration) + return oidc.FositeOauth2Helper(kubeOauthStore, downstreamIssuer, hmacSecretFunc, jwksProviderIsUnused, timeoutsConfiguration), kubeOauthStore + } // Configure fosite the same way that the production code would, using NullStorage to turn off storage. nullOauthStore := oidc.NullStorage{} @@ -124,12 +145,45 @@ func TestAuthorizationEndpoint(t *testing.T) { require.NoError(t, err) upstreamOIDCIdentityProvider := oidctestutil.TestUpstreamOIDCIdentityProvider{ - Name: "some-idp", + Name: "some-oidc-idp", ClientID: "some-client-id", AuthorizationURL: *upstreamAuthURL, Scopes: []string{"scope1", "scope2"}, // the scopes to request when starting the upstream authorization flow } + happyLDAPUsername := "some-ldap-user" + happyLDAPUsernameFromAuthenticator := "some-mapped-ldap-username" + happyLDAPPassword := "some-ldap-password" //nolint:gosec + happyLDAPUID := "some-ldap-uid" + happyLDAPGroups := []string{"group1", "group2", "group3"} + + upstreamLDAPIdentityProvider := oidctestutil.TestUpstreamLDAPIdentityProvider{ + Name: "some-ldap-idp", + URL: upstreamLDAPURL, + AuthenticateFunc: func(ctx context.Context, username, password string) (*authenticator.Response, bool, error) { + if username == "" || password == "" { + return nil, false, fmt.Errorf("should not have passed empty username or password to the authenticator") + } + if username == happyLDAPUsername && password == happyLDAPPassword { + return &authenticator.Response{ + User: &user.DefaultInfo{ + Name: happyLDAPUsernameFromAuthenticator, + UID: happyLDAPUID, + Groups: happyLDAPGroups, + }, + }, true, nil + } + return nil, false, nil + }, + } + + erroringUpstreamLDAPIdentityProvider := oidctestutil.TestUpstreamLDAPIdentityProvider{ + Name: "some-ldap-idp", + AuthenticateFunc: func(ctx context.Context, username, password string) (*authenticator.Response, bool, error) { + return nil, false, fmt.Errorf("some ldap upstream auth error") + }, + } + happyCSRF := "test-csrf" happyPKCE := "test-pkce" happyNonce := "test-nonce" @@ -177,14 +231,17 @@ func TestAuthorizationEndpoint(t *testing.T) { return urlToReturn } + happyDownstreamScopesRequested := []string{"openid", "profile", "email"} + happyDownstreamScopesGranted := []string{"openid"} + happyGetRequestQueryMap := map[string]string{ "response_type": "code", - "scope": "openid profile email", - "client_id": "pinniped-cli", + "scope": strings.Join(happyDownstreamScopesRequested, " "), + "client_id": downstreamClientID, "state": happyState, - "nonce": "some-nonce-value", - "code_challenge": "some-challenge", - "code_challenge_method": "S256", + "nonce": downstreamNonce, + "code_challenge": downstreamPKCEChallenge, + "code_challenge_method": downstreamPKCEChallengeMethod, "redirect_uri": downstreamRedirectURI, } @@ -242,7 +299,7 @@ func TestAuthorizationEndpoint(t *testing.T) { "state": expectedUpstreamState, "nonce": happyNonce, "code_challenge": expectedUpstreamCodeChallenge, - "code_challenge_method": "S256", + "code_challenge_method": downstreamPKCEChallengeMethod, "redirect_uri": downstreamIssuer + "/callback", } if expectedPrompt != "" { @@ -251,6 +308,9 @@ func TestAuthorizationEndpoint(t *testing.T) { return urlWithQuery(upstreamAuthURL.String(), query) } + // Note that fosite puts the granted scopes as a param in the redirect URI even though the spec doesn't seem to require it + happyAuthcodeDownstreamRedirectLocationRegexp := downstreamRedirectURI + `\?code=([^&]+)&scope=openid&state=` + happyState + incomingCookieCSRFValue := "csrf-value-from-cookie" encodedIncomingCookieCSRFValue, err := happyCookieEncoder.Encode("csrf", incomingCookieCSRFValue) require.NoError(t, err) @@ -258,27 +318,41 @@ func TestAuthorizationEndpoint(t *testing.T) { type testCase struct { name string - idpLister provider.DynamicUpstreamIDPProvider - generateCSRF func() (csrftoken.CSRFToken, error) - generatePKCE func() (pkce.Code, error) - generateNonce func() (nonce.Nonce, error) - stateEncoder oidc.Codec - cookieEncoder oidc.Codec - method string - path string - contentType string - body string - csrfCookie string + idpLister provider.DynamicUpstreamIDPProvider + generateCSRF func() (csrftoken.CSRFToken, error) + generatePKCE func() (pkce.Code, error) + generateNonce func() (nonce.Nonce, error) + stateEncoder oidc.Codec + cookieEncoder oidc.Codec + method string + path string + contentType string + body string + csrfCookie string + customUsernameHeader *string // nil means do not send header, empty means send header with empty value + customPasswordHeader *string // nil means do not send header, empty means send header with empty value - wantStatus int - wantContentType string - wantBodyString string - wantBodyJSON string - wantLocationHeader string - wantCSRFValueInCookieHeader string - - wantUpstreamStateParamInLocationHeader bool + wantStatus int + wantContentType string + wantBodyString string + wantBodyJSON string + wantCSRFValueInCookieHeader string wantBodyStringWithLocationInHref bool + wantLocationHeader string + wantUpstreamStateParamInLocationHeader bool + + // For when the request was authenticated by an upstream LDAP provider and an authcode is being returned. + wantRedirectLocationRegexp string + wantDownstreamRedirectURI string + wantDownstreamGrantedScopes []string + wantDownstreamIDTokenSubject string + wantDownstreamIDTokenUsername string + wantDownstreamIDTokenGroups []string + wantDownstreamRequestedScopes []string + wantDownstreamPKCEChallenge string + wantDownstreamPKCEChallengeMethod string + wantDownstreamNonce string + wantUnnecessaryStoredRecords int } tests := []testCase{ { @@ -292,12 +366,33 @@ func TestAuthorizationEndpoint(t *testing.T) { method: http.MethodGet, path: happyGetRequestPath, wantStatus: http.StatusFound, - wantContentType: "text/html; charset=utf-8", + wantContentType: htmlContentType, wantCSRFValueInCookieHeader: happyCSRF, wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(nil, "", ""), ""), wantUpstreamStateParamInLocationHeader: true, wantBodyStringWithLocationInHref: true, }, + { + name: "LDAP upstream happy path using GET", + idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(), + method: http.MethodGet, + path: happyGetRequestPath, + customUsernameHeader: stringPtr(happyLDAPUsername), + customPasswordHeader: stringPtr(happyLDAPPassword), + wantStatus: http.StatusFound, + wantContentType: htmlContentType, + wantRedirectLocationRegexp: happyAuthcodeDownstreamRedirectLocationRegexp, + wantBodyStringWithLocationInHref: false, + wantDownstreamIDTokenSubject: upstreamLDAPURL + "?sub=" + happyLDAPUID, + wantDownstreamIDTokenUsername: happyLDAPUsernameFromAuthenticator, + wantDownstreamIDTokenGroups: happyLDAPGroups, + wantDownstreamRequestedScopes: happyDownstreamScopesRequested, + wantDownstreamRedirectURI: downstreamRedirectURI, + wantDownstreamGrantedScopes: happyDownstreamScopesGranted, + wantDownstreamNonce: downstreamNonce, + wantDownstreamPKCEChallenge: downstreamPKCEChallenge, + wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + }, { name: "OIDC upstream happy path using GET with a CSRF cookie", idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(), @@ -310,7 +405,7 @@ func TestAuthorizationEndpoint(t *testing.T) { path: happyGetRequestPath, csrfCookie: "__Host-pinniped-csrf=" + encodedIncomingCookieCSRFValue + " ", wantStatus: http.StatusFound, - wantContentType: "text/html; charset=utf-8", + wantContentType: htmlContentType, wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(nil, incomingCookieCSRFValue, ""), ""), wantUpstreamStateParamInLocationHeader: true, wantBodyStringWithLocationInHref: true, @@ -334,6 +429,29 @@ func TestAuthorizationEndpoint(t *testing.T) { wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(nil, "", ""), ""), wantUpstreamStateParamInLocationHeader: true, }, + { + name: "LDAP upstream happy path using POST", + idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(), + method: http.MethodPost, + path: "/some/path", + contentType: "application/x-www-form-urlencoded", + body: encodeQuery(happyGetRequestQueryMap), + customUsernameHeader: stringPtr(happyLDAPUsername), + customPasswordHeader: stringPtr(happyLDAPPassword), + wantStatus: http.StatusFound, + wantContentType: htmlContentType, + wantRedirectLocationRegexp: happyAuthcodeDownstreamRedirectLocationRegexp, + wantBodyStringWithLocationInHref: false, + wantDownstreamIDTokenSubject: upstreamLDAPURL + "?sub=" + happyLDAPUID, + wantDownstreamIDTokenUsername: happyLDAPUsernameFromAuthenticator, + wantDownstreamIDTokenGroups: happyLDAPGroups, + wantDownstreamRequestedScopes: happyDownstreamScopesRequested, + wantDownstreamRedirectURI: downstreamRedirectURI, + wantDownstreamGrantedScopes: happyDownstreamScopesGranted, + wantDownstreamNonce: downstreamNonce, + wantDownstreamPKCEChallenge: downstreamPKCEChallenge, + wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + }, { name: "OIDC upstream happy path with prompt param login passed through to redirect uri", idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(), @@ -347,14 +465,14 @@ func TestAuthorizationEndpoint(t *testing.T) { contentType: "application/x-www-form-urlencoded", body: encodeQuery(happyGetRequestQueryMap), wantStatus: http.StatusFound, - wantContentType: "text/html; charset=utf-8", + wantContentType: htmlContentType, wantBodyStringWithLocationInHref: true, wantCSRFValueInCookieHeader: happyCSRF, wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(map[string]string{"prompt": "login"}, "", ""), "login"), wantUpstreamStateParamInLocationHeader: true, }, { - name: "error while decoding CSRF cookie just generates a new cookie and succeeds as usual", + name: "OIDC upstream with error while decoding CSRF cookie just generates a new cookie and succeeds as usual", idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, @@ -365,7 +483,7 @@ func TestAuthorizationEndpoint(t *testing.T) { path: happyGetRequestPath, csrfCookie: "__Host-pinniped-csrf=this-value-was-not-signed-by-pinniped", wantStatus: http.StatusFound, - wantContentType: "text/html; charset=utf-8", + wantContentType: htmlContentType, // Generated a new CSRF cookie and set it in the response. wantCSRFValueInCookieHeader: happyCSRF, wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(nil, "", ""), ""), @@ -385,7 +503,7 @@ func TestAuthorizationEndpoint(t *testing.T) { "redirect_uri": downstreamRedirectURIWithDifferentPort, // not the same port number that is registered for the client }), wantStatus: http.StatusFound, - wantContentType: "text/html; charset=utf-8", + wantContentType: htmlContentType, wantCSRFValueInCookieHeader: happyCSRF, wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(map[string]string{ "redirect_uri": downstreamRedirectURIWithDifferentPort, // not the same port number that is registered for the client @@ -393,6 +511,29 @@ func TestAuthorizationEndpoint(t *testing.T) { wantUpstreamStateParamInLocationHeader: true, wantBodyStringWithLocationInHref: true, }, + { + name: "LDAP upstream happy path when downstream redirect uri matches what is configured for client except for the port number", + idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(), + method: http.MethodGet, + path: modifiedHappyGetRequestPath(map[string]string{ + "redirect_uri": downstreamRedirectURIWithDifferentPort, // not the same port number that is registered for the client + }), + customUsernameHeader: stringPtr(happyLDAPUsername), + customPasswordHeader: stringPtr(happyLDAPPassword), + wantStatus: http.StatusFound, + wantContentType: htmlContentType, + wantRedirectLocationRegexp: downstreamRedirectURIWithDifferentPort + `\?code=([^&]+)&scope=openid&state=` + happyState, + wantBodyStringWithLocationInHref: false, + wantDownstreamIDTokenSubject: upstreamLDAPURL + "?sub=" + happyLDAPUID, + wantDownstreamIDTokenUsername: happyLDAPUsernameFromAuthenticator, + wantDownstreamIDTokenGroups: happyLDAPGroups, + wantDownstreamRequestedScopes: happyDownstreamScopesRequested, + wantDownstreamRedirectURI: downstreamRedirectURIWithDifferentPort, + wantDownstreamGrantedScopes: happyDownstreamScopesGranted, + wantDownstreamNonce: downstreamNonce, + wantDownstreamPKCEChallenge: downstreamPKCEChallenge, + wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + }, { name: "OIDC upstream happy path when downstream requested scopes include offline_access", idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(), @@ -404,7 +545,7 @@ func TestAuthorizationEndpoint(t *testing.T) { method: http.MethodGet, path: modifiedHappyGetRequestPath(map[string]string{"scope": "openid offline_access"}), wantStatus: http.StatusFound, - wantContentType: "text/html; charset=utf-8", + wantContentType: htmlContentType, wantCSRFValueInCookieHeader: happyCSRF, wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(map[string]string{ "scope": "openid offline_access", @@ -413,7 +554,66 @@ func TestAuthorizationEndpoint(t *testing.T) { wantBodyStringWithLocationInHref: true, }, { - name: "downstream redirect uri does not match what is configured for client", + name: "error during upstream LDAP authentication", + idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&erroringUpstreamLDAPIdentityProvider).Build(), + method: http.MethodGet, + path: happyGetRequestPath, + customUsernameHeader: stringPtr(happyLDAPUsername), + customPasswordHeader: stringPtr(happyLDAPPassword), + wantStatus: http.StatusBadGateway, + wantContentType: htmlContentType, + wantBodyString: "Bad Gateway: unexpected error during upstream authentication\n", + }, + { + name: "wrong upstream password for LDAP authentication", + idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(), + method: http.MethodGet, + path: happyGetRequestPath, + customUsernameHeader: stringPtr(happyLDAPUsername), + customPasswordHeader: stringPtr("wrong-password"), + wantStatus: http.StatusFound, + wantContentType: "application/json; charset=utf-8", + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithBadUsernamePasswordHintErrorQuery), + wantBodyString: "", + }, + { + name: "wrong upstream username for LDAP authentication", + idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(), + method: http.MethodGet, + path: happyGetRequestPath, + customUsernameHeader: stringPtr("wrong-username"), + customPasswordHeader: stringPtr(happyLDAPPassword), + wantStatus: http.StatusFound, + wantContentType: "application/json; charset=utf-8", + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithBadUsernamePasswordHintErrorQuery), + wantBodyString: "", + }, + { + name: "missing upstream username on request for LDAP authentication", + idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(), + method: http.MethodGet, + path: happyGetRequestPath, + customUsernameHeader: nil, // do not send header + customPasswordHeader: stringPtr(happyLDAPPassword), + wantStatus: http.StatusFound, + wantContentType: "application/json; charset=utf-8", + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingUsernamePasswordHintErrorQuery), + wantBodyString: "", + }, + { + name: "missing upstream password on request for LDAP authentication", + idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(), + method: http.MethodGet, + path: happyGetRequestPath, + customUsernameHeader: stringPtr(happyLDAPUsername), + customPasswordHeader: nil, // do not send header + wantStatus: http.StatusFound, + wantContentType: "application/json; charset=utf-8", + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingUsernamePasswordHintErrorQuery), + wantBodyString: "", + }, + { + name: "downstream redirect uri does not match what is configured for client when using OIDC upstream", idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, @@ -429,7 +629,20 @@ func TestAuthorizationEndpoint(t *testing.T) { wantBodyJSON: fositeInvalidRedirectURIErrorBody, }, { - name: "downstream client does not exist", + name: "downstream redirect uri does not match what is configured for client when using LDAP upstream", + idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(), + method: http.MethodGet, + path: modifiedHappyGetRequestPath(map[string]string{ + "redirect_uri": "http://127.0.0.1/does-not-match-what-is-configured-for-pinniped-cli-client", + }), + customUsernameHeader: stringPtr(happyLDAPUsername), + customPasswordHeader: stringPtr(happyLDAPPassword), + wantStatus: http.StatusBadRequest, + wantContentType: "application/json; charset=utf-8", + wantBodyJSON: fositeInvalidRedirectURIErrorBody, + }, + { + name: "downstream client does not exist when using OIDC upstream", idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, @@ -443,7 +656,16 @@ func TestAuthorizationEndpoint(t *testing.T) { wantBodyJSON: fositeInvalidClientErrorBody, }, { - name: "response type is unsupported", + name: "downstream client does not exist when using LDAP upstream", + idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(), + method: http.MethodGet, + path: modifiedHappyGetRequestPath(map[string]string{"client_id": "invalid-client"}), + wantStatus: http.StatusUnauthorized, + wantContentType: "application/json; charset=utf-8", + wantBodyJSON: fositeInvalidClientErrorBody, + }, + { + name: "response type is unsupported when using OIDC upstream", idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, @@ -458,7 +680,17 @@ func TestAuthorizationEndpoint(t *testing.T) { wantBodyString: "", }, { - name: "downstream scopes do not match what is configured for client", + name: "response type is unsupported when using LDAP upstream", + idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(), + method: http.MethodGet, + path: modifiedHappyGetRequestPath(map[string]string{"response_type": "unsupported"}), + wantStatus: http.StatusFound, + wantContentType: "application/json; charset=utf-8", + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeUnsupportedResponseTypeErrorQuery), + wantBodyString: "", + }, + { + name: "downstream scopes do not match what is configured for client using OIDC upstream", idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, @@ -473,7 +705,19 @@ func TestAuthorizationEndpoint(t *testing.T) { wantBodyString: "", }, { - name: "missing response type in request", + name: "downstream scopes do not match what is configured for client using LDAP upstream", + idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(), + method: http.MethodGet, + path: modifiedHappyGetRequestPath(map[string]string{"scope": "openid tuna"}), + customUsernameHeader: stringPtr(happyLDAPUsername), + customPasswordHeader: stringPtr(happyLDAPPassword), + wantStatus: http.StatusFound, + wantContentType: "application/json; charset=utf-8", + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeInvalidScopeErrorQuery), + wantBodyString: "", + }, + { + name: "missing response type in request using OIDC upstream", idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, @@ -488,7 +732,17 @@ func TestAuthorizationEndpoint(t *testing.T) { wantBodyString: "", }, { - name: "missing client id in request", + name: "missing response type in request using LDAP upstream", + idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(), + method: http.MethodGet, + path: modifiedHappyGetRequestPath(map[string]string{"response_type": ""}), + wantStatus: http.StatusFound, + wantContentType: "application/json; charset=utf-8", + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeMissingResponseTypeErrorQuery), + wantBodyString: "", + }, + { + name: "missing client id in request using OIDC upstream", idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, @@ -502,7 +756,16 @@ func TestAuthorizationEndpoint(t *testing.T) { wantBodyJSON: fositeInvalidClientErrorBody, }, { - name: "missing PKCE code_challenge in request", // See https://tools.ietf.org/html/rfc7636#section-4.4.1 + name: "missing client id in request using LDAP upstream", + idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(), + method: http.MethodGet, + path: modifiedHappyGetRequestPath(map[string]string{"client_id": ""}), + wantStatus: http.StatusUnauthorized, + wantContentType: "application/json; charset=utf-8", + wantBodyJSON: fositeInvalidClientErrorBody, + }, + { + name: "missing PKCE code_challenge in request using OIDC upstream", // See https://tools.ietf.org/html/rfc7636#section-4.4.1 idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, @@ -517,7 +780,20 @@ func TestAuthorizationEndpoint(t *testing.T) { wantBodyString: "", }, { - name: "invalid value for PKCE code_challenge_method in request", // https://tools.ietf.org/html/rfc7636#section-4.3 + name: "missing PKCE code_challenge in request using LDAP upstream", // See https://tools.ietf.org/html/rfc7636#section-4.4.1 + idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(), + method: http.MethodGet, + path: modifiedHappyGetRequestPath(map[string]string{"code_challenge": ""}), + customUsernameHeader: stringPtr(happyLDAPUsername), + customPasswordHeader: stringPtr(happyLDAPPassword), + wantStatus: http.StatusFound, + wantContentType: "application/json; charset=utf-8", + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeErrorQuery), + wantBodyString: "", + wantUnnecessaryStoredRecords: 2, // fosite already stored the authcode and oidc session before it noticed the error + }, + { + name: "invalid value for PKCE code_challenge_method in request using OIDC upstream", // https://tools.ietf.org/html/rfc7636#section-4.3 idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, @@ -532,7 +808,20 @@ func TestAuthorizationEndpoint(t *testing.T) { wantBodyString: "", }, { - name: "when PKCE code_challenge_method in request is `plain`", // https://tools.ietf.org/html/rfc7636#section-4.3 + name: "invalid value for PKCE code_challenge_method in request using LDAP upstream", // https://tools.ietf.org/html/rfc7636#section-4.3 + idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(), + method: http.MethodGet, + path: modifiedHappyGetRequestPath(map[string]string{"code_challenge_method": "this-is-not-a-valid-pkce-alg"}), + customUsernameHeader: stringPtr(happyLDAPUsername), + customPasswordHeader: stringPtr(happyLDAPPassword), + wantStatus: http.StatusFound, + wantContentType: "application/json; charset=utf-8", + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeInvalidCodeChallengeErrorQuery), + wantBodyString: "", + wantUnnecessaryStoredRecords: 2, // fosite already stored the authcode and oidc session before it noticed the error + }, + { + name: "when PKCE code_challenge_method in request is `plain` using OIDC upstream", // https://tools.ietf.org/html/rfc7636#section-4.3 idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, @@ -547,7 +836,20 @@ func TestAuthorizationEndpoint(t *testing.T) { wantBodyString: "", }, { - name: "missing PKCE code_challenge_method in request", // See https://tools.ietf.org/html/rfc7636#section-4.4.1 + name: "when PKCE code_challenge_method in request is `plain` using LDAP upstream", // https://tools.ietf.org/html/rfc7636#section-4.3 + idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(), + method: http.MethodGet, + path: modifiedHappyGetRequestPath(map[string]string{"code_challenge_method": "plain"}), + customUsernameHeader: stringPtr(happyLDAPUsername), + customPasswordHeader: stringPtr(happyLDAPPassword), + wantStatus: http.StatusFound, + wantContentType: "application/json; charset=utf-8", + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeMethodErrorQuery), + wantBodyString: "", + wantUnnecessaryStoredRecords: 2, // fosite already stored the authcode and oidc session before it noticed the error + }, + { + name: "missing PKCE code_challenge_method in request using OIDC upstream", // See https://tools.ietf.org/html/rfc7636#section-4.4.1 idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, @@ -561,10 +863,23 @@ func TestAuthorizationEndpoint(t *testing.T) { wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeMethodErrorQuery), wantBodyString: "", }, + { + name: "missing PKCE code_challenge_method in request using LDAP upstream", // See https://tools.ietf.org/html/rfc7636#section-4.4.1 + idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(), + method: http.MethodGet, + path: modifiedHappyGetRequestPath(map[string]string{"code_challenge_method": ""}), + customUsernameHeader: stringPtr(happyLDAPUsername), + customPasswordHeader: stringPtr(happyLDAPPassword), + wantStatus: http.StatusFound, + wantContentType: "application/json; charset=utf-8", + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeMethodErrorQuery), + wantBodyString: "", + wantUnnecessaryStoredRecords: 2, // fosite already stored the authcode and oidc session before it noticed the error + }, { // This is just one of the many OIDC validations run by fosite. This test is to ensure that we are running - // through that part of the fosite library. - name: "prompt param is not allowed to have none and another legal value at the same time", + // through that part of the fosite library when using an OIDC upstream. + name: "prompt param is not allowed to have none and another legal value at the same time using OIDC upstream", idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, @@ -579,7 +894,22 @@ func TestAuthorizationEndpoint(t *testing.T) { wantBodyString: "", }, { - name: "OIDC validations are skipped when the openid scope was not requested", + // This is just one of the many OIDC validations run by fosite. This test is to ensure that we are running + // through that part of the fosite library when using an LDAP upstream. + name: "prompt param is not allowed to have none and another legal value at the same time using LDAP upstream", + idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(), + method: http.MethodGet, + path: modifiedHappyGetRequestPath(map[string]string{"prompt": "none login"}), + customUsernameHeader: stringPtr(happyLDAPUsername), + customPasswordHeader: stringPtr(happyLDAPPassword), + wantStatus: http.StatusFound, + wantContentType: "application/json; charset=utf-8", + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositePromptHasNoneAndOtherValueErrorQuery), + wantBodyString: "", + wantUnnecessaryStoredRecords: 1, // fosite already stored the authcode before it noticed the error + }, + { + name: "happy path: downstream OIDC validations are skipped when the openid scope was not requested using OIDC upstream", idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, @@ -590,7 +920,7 @@ func TestAuthorizationEndpoint(t *testing.T) { // The following prompt value is illegal when openid is requested, but note that openid is not requested. path: modifiedHappyGetRequestPath(map[string]string{"prompt": "none login", "scope": "email"}), wantStatus: http.StatusFound, - wantContentType: "text/html; charset=utf-8", + wantContentType: htmlContentType, wantCSRFValueInCookieHeader: happyCSRF, wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam( map[string]string{"prompt": "none login", "scope": "email"}, "", "", @@ -599,7 +929,29 @@ func TestAuthorizationEndpoint(t *testing.T) { wantBodyStringWithLocationInHref: true, }, { - name: "state does not have enough entropy", + name: "happy path: downstream OIDC validations are skipped when the openid scope was not requested using LDAP upstream", + idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(), + method: http.MethodGet, + // The following prompt value is illegal when openid is requested, but note that openid is not requested. + path: modifiedHappyGetRequestPath(map[string]string{"prompt": "none login", "scope": "email"}), + customUsernameHeader: stringPtr(happyLDAPUsername), + customPasswordHeader: stringPtr(happyLDAPPassword), + wantStatus: http.StatusFound, + wantContentType: htmlContentType, + wantRedirectLocationRegexp: downstreamRedirectURI + `\?code=([^&]+)&scope=&state=` + happyState, // no scopes granted + wantBodyStringWithLocationInHref: false, + wantDownstreamIDTokenSubject: upstreamLDAPURL + "?sub=" + happyLDAPUID, + wantDownstreamIDTokenUsername: happyLDAPUsernameFromAuthenticator, + wantDownstreamIDTokenGroups: happyLDAPGroups, + wantDownstreamRequestedScopes: []string{"email"}, // only email was requested + wantDownstreamRedirectURI: downstreamRedirectURI, + wantDownstreamGrantedScopes: []string{}, // no scopes granted + wantDownstreamNonce: downstreamNonce, + wantDownstreamPKCEChallenge: downstreamPKCEChallenge, + wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + }, + { + name: "downstream state does not have enough entropy using OIDC upstream", idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, @@ -614,7 +966,19 @@ func TestAuthorizationEndpoint(t *testing.T) { wantBodyString: "", }, { - name: "error while encoding upstream state param", + name: "downstream state does not have enough entropy using LDAP upstream", + idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider).Build(), + method: http.MethodGet, + path: modifiedHappyGetRequestPath(map[string]string{"state": "short"}), + customUsernameHeader: stringPtr(happyLDAPUsername), + customPasswordHeader: stringPtr(happyLDAPPassword), + wantStatus: http.StatusFound, + wantContentType: "application/json; charset=utf-8", + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeInvalidStateErrorQuery), + wantBodyString: "", + }, + { + name: "error while encoding upstream state param using OIDC upstream", idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, @@ -628,7 +992,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantBodyString: "Internal Server Error: error encoding upstream state param\n", }, { - name: "error while encoding CSRF cookie value for new cookie", + name: "error while encoding CSRF cookie value for new cookie using OIDC upstream", idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, @@ -642,7 +1006,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantBodyString: "Internal Server Error: error encoding CSRF cookie\n", }, { - name: "error while generating CSRF token", + name: "error while generating CSRF token using OIDC upstream", idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(), generateCSRF: sadCSRFGenerator, generatePKCE: happyPKCEGenerator, @@ -656,7 +1020,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantBodyString: "Internal Server Error: error generating CSRF token\n", }, { - name: "error while generating nonce", + name: "error while generating nonce using OIDC upstream", idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(), generateCSRF: happyCSRFGenerator, generatePKCE: happyPKCEGenerator, @@ -670,7 +1034,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantBodyString: "Internal Server Error: error generating nonce param\n", }, { - name: "error while generating PKCE", + name: "error while generating PKCE using OIDC upstream", idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(), generateCSRF: happyCSRFGenerator, generatePKCE: sadPKCEGenerator, @@ -693,7 +1057,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantBodyString: "Unprocessable Entity: No upstream providers are configured\n", }, { - name: "too many upstream providers are configured", + name: "too many upstream providers are configured: multiple OIDC", idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider, &upstreamOIDCIdentityProvider).Build(), // more than one not allowed method: http.MethodGet, path: happyGetRequestPath, @@ -701,6 +1065,24 @@ func TestAuthorizationEndpoint(t *testing.T) { wantContentType: "text/plain; charset=utf-8", wantBodyString: "Unprocessable Entity: Too many upstream providers are configured (support for multiple upstreams is not yet implemented)\n", }, + { + name: "too many upstream providers are configured: multiple LDAP", + idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&upstreamLDAPIdentityProvider, &upstreamLDAPIdentityProvider).Build(), // more than one not allowed + method: http.MethodGet, + path: happyGetRequestPath, + wantStatus: http.StatusUnprocessableEntity, + wantContentType: "text/plain; charset=utf-8", + wantBodyString: "Unprocessable Entity: Too many upstream providers are configured (support for multiple upstreams is not yet implemented)\n", + }, + { + name: "too many upstream providers are configured: both OIDC and LDAP", + idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).WithLDAP(&upstreamLDAPIdentityProvider).Build(), // more than one not allowed + method: http.MethodGet, + path: happyGetRequestPath, + wantStatus: http.StatusUnprocessableEntity, + wantContentType: "text/plain; charset=utf-8", + wantBodyString: "Unprocessable Entity: Too many upstream providers are configured (support for multiple upstreams is not yet implemented)\n", + }, { name: "PUT is a bad method", idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(), @@ -730,12 +1112,18 @@ func TestAuthorizationEndpoint(t *testing.T) { }, } - runOneTestCase := func(t *testing.T, test testCase, subject http.Handler) { + runOneTestCase := func(t *testing.T, test testCase, subject http.Handler, kubeOauthStore *oidc.KubeStorage, kubeClient *fake.Clientset, secretsClient v1.SecretInterface) { req := httptest.NewRequest(test.method, test.path, strings.NewReader(test.body)) req.Header.Set("Content-Type", test.contentType) if test.csrfCookie != "" { req.Header.Set("Cookie", test.csrfCookie) } + if test.customUsernameHeader != nil { + req.Header.Set("X-Pinniped-Upstream-Username", *test.customUsernameHeader) + } + if test.customPasswordHeader != nil { + req.Header.Set("X-Pinniped-Upstream-Password", *test.customPasswordHeader) + } rsp := httptest.NewRecorder() subject.ServeHTTP(rsp, req) t.Logf("response: %#v", rsp) @@ -746,7 +1134,8 @@ func TestAuthorizationEndpoint(t *testing.T) { testutil.RequireSecurityHeaders(t, rsp) actualLocation := rsp.Header().Get("Location") - if test.wantLocationHeader != "" { + switch { + case test.wantLocationHeader != "": if test.wantUpstreamStateParamInLocationHeader { requireEqualDecodedStateParams(t, actualLocation, test.wantLocationHeader, test.stateEncoder) } @@ -754,7 +1143,34 @@ func TestAuthorizationEndpoint(t *testing.T) { // compare those states since they may be different, but we do want to compare the downstream // state param that should be exactly the same. requireEqualURLs(t, actualLocation, test.wantLocationHeader, test.wantUpstreamStateParamInLocationHeader) - } else { + + // Authorization requests for either a successful OIDC upstream or for an error with any upstream + // should never use Kube storage. There is only one exception to this rule, which is that certain + // OIDC validations are checked in fosite after the OAuth authcode (and sometimes the OIDC session) + // is stored, so it is possible with an LDAP upstream to store objects and then return an error to + // the client anyway (which makes the stored objects useless, but oh well). + require.Len(t, kubeClient.Actions(), test.wantUnnecessaryStoredRecords) + case test.wantRedirectLocationRegexp != "": + require.Len(t, rsp.Header().Values("Location"), 1) + oidctestutil.RequireAuthcodeRedirectLocation( + t, + rsp.Header().Get("Location"), + test.wantRedirectLocationRegexp, + kubeClient, + secretsClient, + kubeOauthStore, + test.wantDownstreamGrantedScopes, + test.wantDownstreamIDTokenSubject, + test.wantDownstreamIDTokenUsername, + test.wantDownstreamIDTokenGroups, + test.wantDownstreamRequestedScopes, + test.wantDownstreamPKCEChallenge, + test.wantDownstreamPKCEChallengeMethod, + test.wantDownstreamNonce, + downstreamClientID, + test.wantDownstreamRedirectURI, + ) + default: require.Empty(t, rsp.Header().Values("Location")) } @@ -782,14 +1198,14 @@ func TestAuthorizationEndpoint(t *testing.T) { } else { require.Empty(t, rsp.Header().Values("Set-Cookie")) } - - // Authorization requests for an OIDC upstream should never use Kube storage. - require.Len(t, kubeClient.Actions(), 0) } for _, test := range tests { test := test t.Run(test.name, func(t *testing.T) { + kubeClient := fake.NewSimpleClientset() + secretsClient := kubeClient.CoreV1().Secrets("some-namespace") + oauthHelperWithRealStorage, kubeOauthStore := createOauthHelperWithRealStorage(secretsClient) subject := NewHandler( downstreamIssuer, test.idpLister, @@ -797,7 +1213,7 @@ func TestAuthorizationEndpoint(t *testing.T) { test.generateCSRF, test.generatePKCE, test.generateNonce, test.stateEncoder, test.cookieEncoder, ) - runOneTestCase(t, test, subject) + runOneTestCase(t, test, subject, kubeOauthStore, kubeClient, secretsClient) }) } @@ -805,6 +1221,9 @@ func TestAuthorizationEndpoint(t *testing.T) { test := tests[0] require.Equal(t, "OIDC upstream happy path using GET without a CSRF cookie", test.name) // re-use the happy path test case + kubeClient := fake.NewSimpleClientset() + secretsClient := kubeClient.CoreV1().Secrets("some-namespace") + oauthHelperWithRealStorage, kubeOauthStore := createOauthHelperWithRealStorage(secretsClient) subject := NewHandler( downstreamIssuer, test.idpLister, @@ -813,7 +1232,7 @@ func TestAuthorizationEndpoint(t *testing.T) { test.stateEncoder, test.cookieEncoder, ) - runOneTestCase(t, test, subject) + runOneTestCase(t, test, subject, kubeOauthStore, kubeClient, secretsClient) // Call the setter to change the upstream IDP settings. newProviderSettings := oidctestutil.TestUpstreamOIDCIdentityProvider{ @@ -834,7 +1253,7 @@ func TestAuthorizationEndpoint(t *testing.T) { "state": expectedUpstreamStateParam(nil, "", newProviderSettings.Name), "nonce": happyNonce, "code_challenge": expectedUpstreamCodeChallenge, - "code_challenge_method": "S256", + "code_challenge_method": downstreamPKCEChallengeMethod, "redirect_uri": downstreamIssuer + "/callback", }, ) @@ -847,7 +1266,7 @@ func TestAuthorizationEndpoint(t *testing.T) { // modified expectations. This should ensure that the implementation is using the in-memory cache // of upstream IDP settings appropriately in terms of always getting the values from the cache // on every request. - runOneTestCase(t, test, subject) + runOneTestCase(t, test, subject, kubeOauthStore, kubeClient, secretsClient) }) } @@ -912,3 +1331,7 @@ func requireEqualURLs(t *testing.T, actualURL string, expectedURL string, ignore } require.Equal(t, expectedLocationQuery, actualLocationQuery) } + +func stringPtr(s string) *string { + return &s +} diff --git a/internal/oidc/callback/callback_handler_test.go b/internal/oidc/callback/callback_handler_test.go index ad82928b..c999cba4 100644 --- a/internal/oidc/callback/callback_handler_test.go +++ b/internal/oidc/callback/callback_handler_test.go @@ -9,26 +9,17 @@ import ( "net/http" "net/http/httptest" "net/url" - "regexp" "strings" "testing" - "time" "github.com/gorilla/securecookie" - "github.com/ory/fosite" - "github.com/ory/fosite/handler/openid" "github.com/stretchr/testify/require" - "k8s.io/apimachinery/pkg/labels" "k8s.io/client-go/kubernetes/fake" - "go.pinniped.dev/internal/crud" - "go.pinniped.dev/internal/fositestorage/authorizationcode" - "go.pinniped.dev/internal/fositestorage/openidconnect" - "go.pinniped.dev/internal/fositestorage/pkce" "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/internal/testutil/oidctestutil" "go.pinniped.dev/pkg/oidcclient/nonce" "go.pinniped.dev/pkg/oidcclient/oidctypes" oidcpkce "go.pinniped.dev/pkg/oidcclient/pkce" @@ -44,8 +35,7 @@ const ( upstreamUsernameClaim = "the-user-claim" upstreamGroupsClaim = "the-groups-claim" - happyUpstreamAuthcode = "upstream-auth-code" - + happyUpstreamAuthcode = "upstream-auth-code" happyUpstreamRedirectURI = "https://example.com/callback" happyDownstreamState = "8b-state" @@ -61,8 +51,7 @@ const ( downstreamPKCEChallenge = "some-challenge" downstreamPKCEChallengeMethod = "S256" - authCodeExpirationSeconds = 10 * 60 // Current, we set our auth code expiration to 10 minutes - timeComparisonFudgeFactor = time.Second * 15 + htmlContentType = "text/html; charset=utf-8" ) var ( @@ -129,6 +118,7 @@ func TestCallbackEndpoint(t *testing.T) { csrfCookie string wantStatus int + wantContentType string wantBody string wantRedirectLocationRegexp string wantDownstreamGrantedScopes []string @@ -252,6 +242,7 @@ func TestCallbackEndpoint(t *testing.T) { path: newRequestPath().WithState(happyState).String(), csrfCookie: happyCSRFCookie, wantStatus: http.StatusUnprocessableEntity, + wantContentType: htmlContentType, wantBody: "Unprocessable Entity: email_verified claim in upstream ID token has invalid format\n", wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, }, @@ -264,6 +255,7 @@ func TestCallbackEndpoint(t *testing.T) { path: newRequestPath().WithState(happyState).String(), csrfCookie: happyCSRFCookie, wantStatus: http.StatusUnprocessableEntity, + wantContentType: htmlContentType, wantBody: "Unprocessable Entity: email_verified claim in upstream ID token has false value\n", wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, }, @@ -327,57 +319,64 @@ func TestCallbackEndpoint(t *testing.T) { // Pre-upstream-exchange verification { - name: "PUT method is invalid", - method: http.MethodPut, - path: newRequestPath().String(), - wantStatus: http.StatusMethodNotAllowed, - wantBody: "Method Not Allowed: PUT (try GET)\n", + name: "PUT method is invalid", + method: http.MethodPut, + path: newRequestPath().String(), + wantStatus: http.StatusMethodNotAllowed, + wantContentType: htmlContentType, + wantBody: "Method Not Allowed: PUT (try GET)\n", }, { - name: "POST method is invalid", - method: http.MethodPost, - path: newRequestPath().String(), - wantStatus: http.StatusMethodNotAllowed, - wantBody: "Method Not Allowed: POST (try GET)\n", + name: "POST method is invalid", + method: http.MethodPost, + path: newRequestPath().String(), + wantStatus: http.StatusMethodNotAllowed, + wantContentType: htmlContentType, + wantBody: "Method Not Allowed: POST (try GET)\n", }, { - name: "PATCH method is invalid", - method: http.MethodPatch, - path: newRequestPath().String(), - wantStatus: http.StatusMethodNotAllowed, - wantBody: "Method Not Allowed: PATCH (try GET)\n", + name: "PATCH method is invalid", + method: http.MethodPatch, + path: newRequestPath().String(), + wantStatus: http.StatusMethodNotAllowed, + wantContentType: htmlContentType, + wantBody: "Method Not Allowed: PATCH (try GET)\n", }, { - name: "DELETE method is invalid", - method: http.MethodDelete, - path: newRequestPath().String(), - wantStatus: http.StatusMethodNotAllowed, - wantBody: "Method Not Allowed: DELETE (try GET)\n", + name: "DELETE method is invalid", + method: http.MethodDelete, + path: newRequestPath().String(), + wantStatus: http.StatusMethodNotAllowed, + wantContentType: htmlContentType, + wantBody: "Method Not Allowed: DELETE (try GET)\n", }, { - name: "code param was not included on request", - method: http.MethodGet, - path: newRequestPath().WithState(happyState).WithoutCode().String(), - csrfCookie: happyCSRFCookie, - wantStatus: http.StatusBadRequest, - wantBody: "Bad Request: code param not found\n", + name: "code param was not included on request", + method: http.MethodGet, + path: newRequestPath().WithState(happyState).WithoutCode().String(), + csrfCookie: happyCSRFCookie, + wantStatus: http.StatusBadRequest, + wantContentType: htmlContentType, + wantBody: "Bad Request: code param not found\n", }, { - name: "state param was not included on request", - method: http.MethodGet, - path: newRequestPath().WithoutState().String(), - csrfCookie: happyCSRFCookie, - wantStatus: http.StatusBadRequest, - wantBody: "Bad Request: state param not found\n", + name: "state param was not included on request", + method: http.MethodGet, + path: newRequestPath().WithoutState().String(), + csrfCookie: happyCSRFCookie, + wantStatus: http.StatusBadRequest, + wantContentType: htmlContentType, + wantBody: "Bad Request: state param not found\n", }, { - name: "state param was not signed correctly, has expired, or otherwise cannot be decoded for any reason", - idp: happyUpstream().Build(), - method: http.MethodGet, - path: newRequestPath().WithState("this-will-not-decode").String(), - csrfCookie: happyCSRFCookie, - wantStatus: http.StatusBadRequest, - wantBody: "Bad Request: error reading state\n", + name: "state param was not signed correctly, has expired, or otherwise cannot be decoded for any reason", + idp: happyUpstream().Build(), + method: http.MethodGet, + path: newRequestPath().WithState("this-will-not-decode").String(), + csrfCookie: happyCSRFCookie, + wantStatus: http.StatusBadRequest, + wantContentType: htmlContentType, + wantBody: "Bad Request: error reading state\n", }, { // This shouldn't happen in practice because the authorize endpoint should have already run the same @@ -393,16 +392,18 @@ func TestCallbackEndpoint(t *testing.T) { csrfCookie: happyCSRFCookie, wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, wantStatus: http.StatusInternalServerError, + wantContentType: htmlContentType, wantBody: "Internal Server Error: error while generating and saving authcode\n", }, { - name: "state's internal version does not match what we want", - idp: happyUpstream().Build(), - method: http.MethodGet, - path: newRequestPath().WithState(happyUpstreamStateParam().WithStateVersion("wrong-state-version").Build(t, happyStateCodec)).String(), - csrfCookie: happyCSRFCookie, - wantStatus: http.StatusUnprocessableEntity, - wantBody: "Unprocessable Entity: state format version is invalid\n", + name: "state's internal version does not match what we want", + idp: happyUpstream().Build(), + method: http.MethodGet, + path: newRequestPath().WithState(happyUpstreamStateParam().WithStateVersion("wrong-state-version").Build(t, happyStateCodec)).String(), + csrfCookie: happyCSRFCookie, + wantStatus: http.StatusUnprocessableEntity, + wantContentType: htmlContentType, + wantBody: "Unprocessable Entity: state format version is invalid\n", }, { name: "state's downstream auth params element is invalid", @@ -411,9 +412,10 @@ func TestCallbackEndpoint(t *testing.T) { path: newRequestPath().WithState(happyUpstreamStateParam(). WithAuthorizeRequestParams("the following is an invalid url encoding token, and therefore this is an invalid param: %z"). Build(t, happyStateCodec)).String(), - csrfCookie: happyCSRFCookie, - wantStatus: http.StatusBadRequest, - wantBody: "Bad Request: error reading state downstream auth params\n", + csrfCookie: happyCSRFCookie, + wantStatus: http.StatusBadRequest, + wantContentType: htmlContentType, + wantBody: "Bad Request: error reading state downstream auth params\n", }, { name: "state's downstream auth params are missing required value (e.g., client_id)", @@ -424,9 +426,10 @@ func TestCallbackEndpoint(t *testing.T) { WithAuthorizeRequestParams(shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"client_id": ""}).Encode()). Build(t, happyStateCodec), ).String(), - csrfCookie: happyCSRFCookie, - wantStatus: http.StatusBadRequest, - wantBody: "Bad Request: error using state downstream auth params\n", + csrfCookie: happyCSRFCookie, + wantStatus: http.StatusBadRequest, + wantContentType: htmlContentType, + wantBody: "Bad Request: error using state downstream auth params\n", }, { name: "state's downstream auth params does not contain openid scope", @@ -474,39 +477,43 @@ func TestCallbackEndpoint(t *testing.T) { wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, }, { - name: "the OIDCIdentityProvider CRD has been deleted", - idp: otherUpstreamOIDCIdentityProvider, - method: http.MethodGet, - path: newRequestPath().WithState(happyState).String(), - csrfCookie: happyCSRFCookie, - wantStatus: http.StatusUnprocessableEntity, - wantBody: "Unprocessable Entity: upstream provider not found\n", + name: "the OIDCIdentityProvider CRD has been deleted", + idp: otherUpstreamOIDCIdentityProvider, + method: http.MethodGet, + path: newRequestPath().WithState(happyState).String(), + csrfCookie: happyCSRFCookie, + wantStatus: http.StatusUnprocessableEntity, + wantContentType: htmlContentType, + wantBody: "Unprocessable Entity: upstream provider not found\n", }, { - name: "the CSRF cookie does not exist on request", - idp: happyUpstream().Build(), - method: http.MethodGet, - path: newRequestPath().WithState(happyState).String(), - wantStatus: http.StatusForbidden, - wantBody: "Forbidden: CSRF cookie is missing\n", + name: "the CSRF cookie does not exist on request", + idp: happyUpstream().Build(), + method: http.MethodGet, + path: newRequestPath().WithState(happyState).String(), + wantStatus: http.StatusForbidden, + wantContentType: htmlContentType, + wantBody: "Forbidden: CSRF cookie is missing\n", }, { - name: "cookie was not signed correctly, has expired, or otherwise cannot be decoded for any reason", - idp: happyUpstream().Build(), - method: http.MethodGet, - path: newRequestPath().WithState(happyState).String(), - csrfCookie: "__Host-pinniped-csrf=this-value-was-not-signed-by-pinniped", - wantStatus: http.StatusForbidden, - wantBody: "Forbidden: error reading CSRF cookie\n", + name: "cookie was not signed correctly, has expired, or otherwise cannot be decoded for any reason", + idp: happyUpstream().Build(), + method: http.MethodGet, + path: newRequestPath().WithState(happyState).String(), + csrfCookie: "__Host-pinniped-csrf=this-value-was-not-signed-by-pinniped", + wantStatus: http.StatusForbidden, + wantContentType: htmlContentType, + wantBody: "Forbidden: error reading CSRF cookie\n", }, { - name: "cookie csrf value does not match state csrf value", - idp: happyUpstream().Build(), - method: http.MethodGet, - path: newRequestPath().WithState(happyUpstreamStateParam().WithCSRF("wrong-csrf-value").Build(t, happyStateCodec)).String(), - csrfCookie: happyCSRFCookie, - wantStatus: http.StatusForbidden, - wantBody: "Forbidden: CSRF value does not match\n", + name: "cookie csrf value does not match state csrf value", + idp: happyUpstream().Build(), + method: http.MethodGet, + path: newRequestPath().WithState(happyUpstreamStateParam().WithCSRF("wrong-csrf-value").Build(t, happyStateCodec)).String(), + csrfCookie: happyCSRFCookie, + wantStatus: http.StatusForbidden, + wantContentType: htmlContentType, + wantBody: "Forbidden: CSRF value does not match\n", }, // Upstream exchange @@ -518,6 +525,7 @@ func TestCallbackEndpoint(t *testing.T) { csrfCookie: happyCSRFCookie, wantStatus: http.StatusBadGateway, wantBody: "Bad Gateway: error exchanging and validating upstream tokens\n", + wantContentType: htmlContentType, wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, }, { @@ -528,6 +536,7 @@ func TestCallbackEndpoint(t *testing.T) { csrfCookie: happyCSRFCookie, wantStatus: http.StatusUnprocessableEntity, wantBody: "Unprocessable Entity: no username claim in upstream ID token\n", + wantContentType: htmlContentType, wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, }, { @@ -556,6 +565,7 @@ func TestCallbackEndpoint(t *testing.T) { path: newRequestPath().WithState(happyState).String(), csrfCookie: happyCSRFCookie, wantStatus: http.StatusUnprocessableEntity, + wantContentType: htmlContentType, wantBody: "Unprocessable Entity: username claim in upstream ID token has invalid format\n", wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, }, @@ -566,6 +576,7 @@ func TestCallbackEndpoint(t *testing.T) { path: newRequestPath().WithState(happyState).String(), csrfCookie: happyCSRFCookie, wantStatus: http.StatusUnprocessableEntity, + wantContentType: htmlContentType, wantBody: "Unprocessable Entity: issuer claim in upstream ID token missing\n", wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, }, @@ -576,6 +587,7 @@ func TestCallbackEndpoint(t *testing.T) { path: newRequestPath().WithState(happyState).String(), csrfCookie: happyCSRFCookie, wantStatus: http.StatusUnprocessableEntity, + wantContentType: htmlContentType, wantBody: "Unprocessable Entity: issuer claim in upstream ID token has invalid format\n", wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, }, @@ -586,6 +598,7 @@ func TestCallbackEndpoint(t *testing.T) { path: newRequestPath().WithState(happyState).String(), csrfCookie: happyCSRFCookie, wantStatus: http.StatusUnprocessableEntity, + wantContentType: htmlContentType, wantBody: "Unprocessable Entity: groups claim in upstream ID token has invalid format\n", wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, }, @@ -596,6 +609,7 @@ func TestCallbackEndpoint(t *testing.T) { path: newRequestPath().WithState(happyState).String(), csrfCookie: happyCSRFCookie, wantStatus: http.StatusUnprocessableEntity, + wantContentType: htmlContentType, wantBody: "Unprocessable Entity: groups claim in upstream ID token has invalid format\n", wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, }, @@ -606,6 +620,7 @@ func TestCallbackEndpoint(t *testing.T) { path: newRequestPath().WithState(happyState).String(), csrfCookie: happyCSRFCookie, wantStatus: http.StatusUnprocessableEntity, + wantContentType: htmlContentType, wantBody: "Unprocessable Entity: groups claim in upstream ID token has invalid format\n", wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, }, @@ -648,6 +663,7 @@ func TestCallbackEndpoint(t *testing.T) { } require.Equal(t, test.wantStatus, rsp.Code) + testutil.RequireEqualContentType(t, rsp.Header().Get("Content-Type"), test.wantContentType) if test.wantBody != "" { require.Equal(t, test.wantBody, rsp.Body.String()) @@ -656,79 +672,30 @@ func TestCallbackEndpoint(t *testing.T) { } if test.wantRedirectLocationRegexp != "" { //nolint:nestif // don't mind have several sequential if statements in this test - // Assert that Location header matches regular expression. require.Len(t, rsp.Header().Values("Location"), 1) - actualLocation := rsp.Header().Get("Location") - regex := regexp.MustCompile(test.wantRedirectLocationRegexp) - submatches := regex.FindStringSubmatch(actualLocation) - require.Lenf(t, submatches, 2, "no regexp match in actualLocation: %q", actualLocation) - capturedAuthCode := submatches[1] - - // fosite authcodes are in the format `data.signature`, so grab the signature part, which is the lookup key in the storage interface - authcodeDataAndSignature := strings.Split(capturedAuthCode, ".") - require.Len(t, authcodeDataAndSignature, 2) - - // Several Secrets should have been created - expectedNumberOfCreatedSecrets := 2 - if includesOpenIDScope(test.wantDownstreamGrantedScopes) { - expectedNumberOfCreatedSecrets++ - } - require.Len(t, client.Actions(), expectedNumberOfCreatedSecrets) - - // One authcode should have been stored. - testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: authorizationcode.TypeLabelValue}, 1) - - storedRequestFromAuthcode, storedSessionFromAuthcode := validateAuthcodeStorage( + oidctestutil.RequireAuthcodeRedirectLocation( t, + rsp.Header().Get("Location"), + test.wantRedirectLocationRegexp, + client, + secrets, oauthStore, - authcodeDataAndSignature[1], // Authcode store key is authcode signature test.wantDownstreamGrantedScopes, test.wantDownstreamIDTokenSubject, test.wantDownstreamIDTokenUsername, test.wantDownstreamIDTokenGroups, test.wantDownstreamRequestedScopes, - ) - - // One PKCE should have been stored. - testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: pkce.TypeLabelValue}, 1) - - validatePKCEStorage( - t, - oauthStore, - authcodeDataAndSignature[1], // PKCE store key is authcode signature - storedRequestFromAuthcode, - storedSessionFromAuthcode, test.wantDownstreamPKCEChallenge, test.wantDownstreamPKCEChallengeMethod, + test.wantDownstreamNonce, + downstreamClientID, + downstreamRedirectURI, ) - - // One IDSession should have been stored, if the downstream actually requested the "openid" scope - if includesOpenIDScope(test.wantDownstreamGrantedScopes) { - testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: openidconnect.TypeLabelValue}, 1) - - validateIDSessionStorage( - t, - oauthStore, - capturedAuthCode, // IDSession store key is full authcode - storedRequestFromAuthcode, - storedSessionFromAuthcode, - test.wantDownstreamNonce, - ) - } } }) } } -func includesOpenIDScope(scopes []string) bool { - for _, scope := range scopes { - if scope == "openid" { - return true - } - } - return false -} - type requestPath struct { code, state *string } @@ -898,141 +865,3 @@ func shallowCopyAndModifyQuery(query url.Values, modifications map[string]string } return copied } - -func validateAuthcodeStorage( - t *testing.T, - oauthStore *oidc.KubeStorage, - storeKey string, - wantDownstreamGrantedScopes []string, - wantDownstreamIDTokenSubject string, - wantDownstreamIDTokenUsername string, - wantDownstreamIDTokenGroups []string, - wantDownstreamRequestedScopes []string, -) (*fosite.Request, *openid.DefaultSession) { - t.Helper() - - // Get the authcode session back from storage so we can require that it was stored correctly. - storedAuthorizeRequestFromAuthcode, err := oauthStore.GetAuthorizeCodeSession(context.Background(), storeKey, nil) - require.NoError(t, err) - - // Check that storage returned the expected concrete data types. - storedRequestFromAuthcode, storedSessionFromAuthcode := castStoredAuthorizeRequest(t, storedAuthorizeRequestFromAuthcode) - - // Check which scopes were granted. - require.ElementsMatch(t, wantDownstreamGrantedScopes, storedRequestFromAuthcode.GetGrantedScopes()) - - // Check all the other fields of the stored request. - require.NotEmpty(t, storedRequestFromAuthcode.ID) - require.Equal(t, downstreamClientID, storedRequestFromAuthcode.Client.GetID()) - require.ElementsMatch(t, wantDownstreamRequestedScopes, storedRequestFromAuthcode.RequestedScope) - require.Nil(t, storedRequestFromAuthcode.RequestedAudience) - require.Empty(t, storedRequestFromAuthcode.GrantedAudience) - require.Equal(t, url.Values{"redirect_uri": []string{downstreamRedirectURI}}, storedRequestFromAuthcode.Form) - testutil.RequireTimeInDelta(t, time.Now(), storedRequestFromAuthcode.RequestedAt, timeComparisonFudgeFactor) - - // We're not using these fields yet, so confirm that we did not set them (for now). - require.Empty(t, storedSessionFromAuthcode.Subject) - require.Empty(t, storedSessionFromAuthcode.Username) - require.Empty(t, storedSessionFromAuthcode.Headers) - - // The authcode that we are issuing should be good for the length of time that we declare in the fosite config. - testutil.RequireTimeInDelta(t, time.Now().Add(authCodeExpirationSeconds*time.Second), storedSessionFromAuthcode.ExpiresAt[fosite.AuthorizeCode], timeComparisonFudgeFactor) - require.Len(t, storedSessionFromAuthcode.ExpiresAt, 1) - - // Now confirm the ID token claims. - actualClaims := storedSessionFromAuthcode.Claims - - // Check the user's identity, which are put into the downstream ID token's subject, username and groups claims. - require.Equal(t, wantDownstreamIDTokenSubject, actualClaims.Subject) - require.Equal(t, wantDownstreamIDTokenUsername, actualClaims.Extra["username"]) - require.Len(t, actualClaims.Extra, 2) - actualDownstreamIDTokenGroups := actualClaims.Extra["groups"] - require.NotNil(t, actualDownstreamIDTokenGroups) - require.ElementsMatch(t, wantDownstreamIDTokenGroups, actualDownstreamIDTokenGroups) - - // Check the rest of the downstream ID token's claims. Fosite wants us to set these (in UTC time). - testutil.RequireTimeInDelta(t, time.Now().UTC(), actualClaims.RequestedAt, timeComparisonFudgeFactor) - testutil.RequireTimeInDelta(t, time.Now().UTC(), actualClaims.AuthTime, timeComparisonFudgeFactor) - requestedAtZone, _ := actualClaims.RequestedAt.Zone() - require.Equal(t, "UTC", requestedAtZone) - authTimeZone, _ := actualClaims.AuthTime.Zone() - require.Equal(t, "UTC", authTimeZone) - - // 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. - require.Empty(t, actualClaims.JTI) - require.Empty(t, actualClaims.CodeHash) - require.Empty(t, actualClaims.AccessTokenHash) - require.Empty(t, actualClaims.AuthenticationContextClassReference) - require.Empty(t, actualClaims.AuthenticationMethodsReference) - - return storedRequestFromAuthcode, storedSessionFromAuthcode -} - -func validatePKCEStorage( - t *testing.T, - oauthStore *oidc.KubeStorage, - storeKey string, - storedRequestFromAuthcode *fosite.Request, - storedSessionFromAuthcode *openid.DefaultSession, - wantDownstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod string, -) { - t.Helper() - - storedAuthorizeRequestFromPKCE, err := oauthStore.GetPKCERequestSession(context.Background(), storeKey, nil) - require.NoError(t, err) - - // Check that storage returned the expected concrete data types. - storedRequestFromPKCE, storedSessionFromPKCE := castStoredAuthorizeRequest(t, storedAuthorizeRequestFromPKCE) - - // The stored PKCE request should be the same as the stored authcode request. - require.Equal(t, storedRequestFromAuthcode.ID, storedRequestFromPKCE.ID) - require.Equal(t, storedSessionFromAuthcode, storedSessionFromPKCE) - - // The stored PKCE request should also contain the PKCE challenge that the downstream sent us. - require.Equal(t, wantDownstreamPKCEChallenge, storedRequestFromPKCE.Form.Get("code_challenge")) - require.Equal(t, wantDownstreamPKCEChallengeMethod, storedRequestFromPKCE.Form.Get("code_challenge_method")) -} - -func validateIDSessionStorage( - t *testing.T, - oauthStore *oidc.KubeStorage, - storeKey string, - storedRequestFromAuthcode *fosite.Request, - storedSessionFromAuthcode *openid.DefaultSession, - wantDownstreamNonce string, -) { - t.Helper() - - storedAuthorizeRequestFromIDSession, err := oauthStore.GetOpenIDConnectSession(context.Background(), storeKey, nil) - require.NoError(t, err) - - // Check that storage returned the expected concrete data types. - storedRequestFromIDSession, storedSessionFromIDSession := castStoredAuthorizeRequest(t, storedAuthorizeRequestFromIDSession) - - // The stored IDSession request should be the same as the stored authcode request. - require.Equal(t, storedRequestFromAuthcode.ID, storedRequestFromIDSession.ID) - require.Equal(t, storedSessionFromAuthcode, storedSessionFromIDSession) - - // The stored IDSession request should also contain the nonce that the downstream sent us. - require.Equal(t, wantDownstreamNonce, storedRequestFromIDSession.Form.Get("nonce")) -} - -func castStoredAuthorizeRequest(t *testing.T, storedAuthorizeRequest fosite.Requester) (*fosite.Request, *openid.DefaultSession) { - t.Helper() - - storedRequest, ok := storedAuthorizeRequest.(*fosite.Request) - require.Truef(t, ok, "could not cast %T to %T", storedAuthorizeRequest, &fosite.Request{}) - storedSession, ok := storedAuthorizeRequest.GetSession().(*openid.DefaultSession) - require.Truef(t, ok, "could not cast %T to %T", storedAuthorizeRequest.GetSession(), &openid.DefaultSession{}) - - return storedRequest, storedSession -} diff --git a/internal/oidc/dynamic_open_id_connect_ecdsa_strategy.go b/internal/oidc/dynamic_open_id_connect_ecdsa_strategy.go index 6df5e5bc..5c32c798 100644 --- a/internal/oidc/dynamic_open_id_connect_ecdsa_strategy.go +++ b/internal/oidc/dynamic_open_id_connect_ecdsa_strategy.go @@ -1,4 +1,4 @@ -// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package oidc @@ -8,14 +8,13 @@ import ( "crypto/ecdsa" "reflect" - "go.pinniped.dev/internal/constable" - "go.pinniped.dev/internal/plog" - "github.com/ory/fosite" "github.com/ory/fosite/compose" "github.com/ory/fosite/handler/openid" + "go.pinniped.dev/internal/constable" "go.pinniped.dev/internal/oidc/jwks" + "go.pinniped.dev/internal/plog" ) // dynamicOpenIDConnectECDSAStrategy is an openid.OpenIDConnectTokenStrategy that can dynamically 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 f0250e22..c8e036c1 100644 --- a/internal/oidc/dynamic_open_id_connect_ecdsa_strategy_test.go +++ b/internal/oidc/dynamic_open_id_connect_ecdsa_strategy_test.go @@ -1,4 +1,4 @@ -// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package oidc @@ -21,7 +21,7 @@ import ( "gopkg.in/square/go-jose.v2" "go.pinniped.dev/internal/oidc/jwks" - "go.pinniped.dev/internal/oidc/oidctestutil" + "go.pinniped.dev/internal/testutil/oidctestutil" ) func TestDynamicOpenIDConnectECDSAStrategy(t *testing.T) { diff --git a/internal/oidc/kube_storage.go b/internal/oidc/kube_storage.go index 46f9c947..f775c22b 100644 --- a/internal/oidc/kube_storage.go +++ b/internal/oidc/kube_storage.go @@ -1,4 +1,4 @@ -// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package oidc @@ -19,6 +19,7 @@ import ( "go.pinniped.dev/internal/fositestorage/openidconnect" "go.pinniped.dev/internal/fositestorage/pkce" "go.pinniped.dev/internal/fositestorage/refreshtoken" + "go.pinniped.dev/internal/fositestoragei" ) const errKubeStorageNotImplemented = constable.Error("KubeStorage does not implement this method. It should not have been called.") @@ -31,6 +32,8 @@ type KubeStorage struct { refreshTokenStorage refreshtoken.RevocationStorage } +var _ fositestoragei.AllFositeStorage = &KubeStorage{} + func NewKubeStorage(secrets corev1client.SecretInterface, timeoutsConfiguration TimeoutsConfiguration) *KubeStorage { nowFunc := time.Now return &KubeStorage{ diff --git a/internal/oidc/nullstorage.go b/internal/oidc/nullstorage.go index 3dcd7a06..121e3c3a 100644 --- a/internal/oidc/nullstorage.go +++ b/internal/oidc/nullstorage.go @@ -1,4 +1,4 @@ -// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package oidc @@ -10,12 +10,15 @@ import ( "github.com/ory/fosite" "go.pinniped.dev/internal/constable" + "go.pinniped.dev/internal/fositestoragei" ) const errNullStorageNotImplemented = constable.Error("NullStorage does not implement this method. It should not have been called.") type NullStorage struct{} +var _ fositestoragei.AllFositeStorage = &NullStorage{} + func (NullStorage) RevokeRefreshToken(_ context.Context, _ string) error { return errNullStorageNotImplemented } diff --git a/internal/oidc/oidctestutil/oidc.go b/internal/oidc/oidctestutil/oidc.go deleted file mode 100644 index 34938139..00000000 --- a/internal/oidc/oidctestutil/oidc.go +++ /dev/null @@ -1,187 +0,0 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -package oidctestutil - -import ( - "context" - "crypto" - "crypto/ecdsa" - "fmt" - "net/url" - "testing" - - coreosoidc "github.com/coreos/go-oidc/v3/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" - "go.pinniped.dev/pkg/oidcclient/oidctypes" - "go.pinniped.dev/pkg/oidcclient/pkce" -) - -// Test helpers for the OIDC package. - -// ExchangeAuthcodeAndValidateTokenArgs is a POGO (plain old go object?) used to spy on calls to -// TestUpstreamOIDCIdentityProvider.ExchangeAuthcodeAndValidateTokensFunc(). -type ExchangeAuthcodeAndValidateTokenArgs struct { - Ctx context.Context - Authcode string - PKCECodeVerifier pkce.Code - ExpectedIDTokenNonce nonce.Nonce - RedirectURI string -} - -type TestUpstreamOIDCIdentityProvider struct { - Name string - ClientID string - AuthorizationURL url.URL - UsernameClaim string - GroupsClaim string - Scopes []string - ExchangeAuthcodeAndValidateTokensFunc func( - ctx context.Context, - authcode string, - pkceCodeVerifier pkce.Code, - expectedIDTokenNonce nonce.Nonce, - ) (*oidctypes.Token, error) - - exchangeAuthcodeAndValidateTokensCallCount int - exchangeAuthcodeAndValidateTokensArgs []*ExchangeAuthcodeAndValidateTokenArgs -} - -func (u *TestUpstreamOIDCIdentityProvider) GetName() string { - return u.Name -} - -func (u *TestUpstreamOIDCIdentityProvider) GetClientID() string { - return u.ClientID -} - -func (u *TestUpstreamOIDCIdentityProvider) GetAuthorizationURL() *url.URL { - return &u.AuthorizationURL -} - -func (u *TestUpstreamOIDCIdentityProvider) GetScopes() []string { - return u.Scopes -} - -func (u *TestUpstreamOIDCIdentityProvider) GetUsernameClaim() string { - return u.UsernameClaim -} - -func (u *TestUpstreamOIDCIdentityProvider) GetGroupsClaim() string { - return u.GroupsClaim -} - -func (u *TestUpstreamOIDCIdentityProvider) ExchangeAuthcodeAndValidateTokens( - ctx context.Context, - authcode string, - pkceCodeVerifier pkce.Code, - expectedIDTokenNonce nonce.Nonce, - redirectURI string, -) (*oidctypes.Token, error) { - if u.exchangeAuthcodeAndValidateTokensArgs == nil { - u.exchangeAuthcodeAndValidateTokensArgs = make([]*ExchangeAuthcodeAndValidateTokenArgs, 0) - } - u.exchangeAuthcodeAndValidateTokensCallCount++ - u.exchangeAuthcodeAndValidateTokensArgs = append(u.exchangeAuthcodeAndValidateTokensArgs, &ExchangeAuthcodeAndValidateTokenArgs{ - Ctx: ctx, - Authcode: authcode, - PKCECodeVerifier: pkceCodeVerifier, - ExpectedIDTokenNonce: expectedIDTokenNonce, - RedirectURI: redirectURI, - }) - return u.ExchangeAuthcodeAndValidateTokensFunc(ctx, authcode, pkceCodeVerifier, expectedIDTokenNonce) -} - -func (u *TestUpstreamOIDCIdentityProvider) ExchangeAuthcodeAndValidateTokensCallCount() int { - return u.exchangeAuthcodeAndValidateTokensCallCount -} - -func (u *TestUpstreamOIDCIdentityProvider) ExchangeAuthcodeAndValidateTokensArgs(call int) *ExchangeAuthcodeAndValidateTokenArgs { - if u.exchangeAuthcodeAndValidateTokensArgs == nil { - u.exchangeAuthcodeAndValidateTokensArgs = make([]*ExchangeAuthcodeAndValidateTokenArgs, 0) - } - return u.exchangeAuthcodeAndValidateTokensArgs[call] -} - -func (u *TestUpstreamOIDCIdentityProvider) ValidateToken(_ context.Context, _ *oauth2.Token, _ nonce.Nonce) (*oidctypes.Token, error) { - panic("implement me") -} - -type UpstreamIDPListerBuilder struct { - upstreamOIDCIdentityProviders []*TestUpstreamOIDCIdentityProvider -} - -func (b *UpstreamIDPListerBuilder) WithOIDC(upstreamOIDCIdentityProviders ...*TestUpstreamOIDCIdentityProvider) *UpstreamIDPListerBuilder { - b.upstreamOIDCIdentityProviders = append(b.upstreamOIDCIdentityProviders, upstreamOIDCIdentityProviders...) - return b -} - -func (b *UpstreamIDPListerBuilder) Build() provider.DynamicUpstreamIDPProvider { - idpProvider := provider.NewDynamicUpstreamIDPProvider() - upstreams := make([]provider.UpstreamOIDCIdentityProviderI, len(b.upstreamOIDCIdentityProviders)) - for i := range b.upstreamOIDCIdentityProviders { - upstreams[i] = provider.UpstreamOIDCIdentityProviderI(b.upstreamOIDCIdentityProviders[i]) - } - idpProvider.SetOIDCIdentityProviders(upstreams) - return idpProvider -} - -func NewUpstreamIDPListerBuilder() *UpstreamIDPListerBuilder { - return &UpstreamIDPListerBuilder{} -} - -// Declare a separate type from the production code to ensure that the state param's contents was serialized -// in the format that we expect, with the json keys that we expect, etc. This also ensure that the order of -// the serialized fields is the same, which doesn't really matter expect that we can make simpler equality -// assertions about the redirect URL in this test. -type ExpectedUpstreamStateParamFormat struct { - P string `json:"p"` - U string `json:"u"` - N string `json:"n"` - C string `json:"c"` - 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/dynamic_upstream_idp_provider.go b/internal/oidc/provider/dynamic_upstream_idp_provider.go index 1893352b..59175fac 100644 --- a/internal/oidc/provider/dynamic_upstream_idp_provider.go +++ b/internal/oidc/provider/dynamic_upstream_idp_provider.go @@ -53,6 +53,11 @@ type UpstreamLDAPIdentityProviderI interface { // A name for this upstream provider. GetName() string + // Return a URL which uniquely identifies this LDAP provider, e.g. "ldaps://host.example.com:1234". + // This URL is not used for connecting to the provider, but rather is used for creating a globally unique user + // identifier by being combined with the user's UID, since user UIDs are only unique within one provider. + GetURL() string + // A method for performing user authentication against the upstream LDAP provider. ldap.UserAuthenticator } diff --git a/internal/oidc/provider/manager/manager_test.go b/internal/oidc/provider/manager/manager_test.go index a5d79df9..04f30fa0 100644 --- a/internal/oidc/provider/manager/manager_test.go +++ b/internal/oidc/provider/manager/manager_test.go @@ -25,9 +25,9 @@ import ( "go.pinniped.dev/internal/oidc" "go.pinniped.dev/internal/oidc/discovery" "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/internal/testutil/oidctestutil" "go.pinniped.dev/pkg/oidcclient/nonce" "go.pinniped.dev/pkg/oidcclient/oidctypes" "go.pinniped.dev/pkg/oidcclient/pkce" diff --git a/internal/oidc/token/token_handler_test.go b/internal/oidc/token/token_handler_test.go index 6bc1ad63..dee77cd8 100644 --- a/internal/oidc/token/token_handler_test.go +++ b/internal/oidc/token/token_handler_test.go @@ -1,4 +1,4 @@ -// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package token @@ -40,11 +40,12 @@ import ( "go.pinniped.dev/internal/fositestorage/openidconnect" storagepkce "go.pinniped.dev/internal/fositestorage/pkce" "go.pinniped.dev/internal/fositestorage/refreshtoken" + "go.pinniped.dev/internal/fositestoragei" "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" + "go.pinniped.dev/internal/testutil/oidctestutil" ) const ( @@ -214,25 +215,13 @@ type authcodeExchangeInputs struct { modifyTokenRequest func(tokenRequest *http.Request, authCode string) modifyStorage func( t *testing.T, - s interface { - oauth2.TokenRevocationStorage - oauth2.CoreStorage - openid.OpenIDConnectRequestStorage - pkce.PKCERequestStorage - fosite.ClientManager - }, + s fositestoragei.AllFositeStorage, authCode string, ) makeOathHelper func( t *testing.T, authRequest *http.Request, - store interface { - oauth2.TokenRevocationStorage - oauth2.CoreStorage - openid.OpenIDConnectRequestStorage - pkce.PKCERequestStorage - fosite.ClientManager - }, + store fositestoragei.AllFositeStorage, ) (fosite.OAuth2Provider, string, *ecdsa.PrivateKey) want tokenEndpointResponseExpectedValues @@ -1315,13 +1304,7 @@ func getFositeDataSignature(t *testing.T, data string) string { func makeHappyOauthHelper( t *testing.T, authRequest *http.Request, - store interface { - oauth2.TokenRevocationStorage - oauth2.CoreStorage - openid.OpenIDConnectRequestStorage - pkce.PKCERequestStorage - fosite.ClientManager - }, + store fositestoragei.AllFositeStorage, ) (fosite.OAuth2Provider, string, *ecdsa.PrivateKey) { t.Helper() @@ -1347,13 +1330,7 @@ func (s *singleUseJWKProvider) GetJWKS(issuerName string) (jwks *jose.JSONWebKey func makeOauthHelperWithJWTKeyThatWorksOnlyOnce( t *testing.T, authRequest *http.Request, - store interface { - oauth2.TokenRevocationStorage - oauth2.CoreStorage - openid.OpenIDConnectRequestStorage - pkce.PKCERequestStorage - fosite.ClientManager - }, + store fositestoragei.AllFositeStorage, ) (fosite.OAuth2Provider, string, *ecdsa.PrivateKey) { t.Helper() @@ -1366,13 +1343,7 @@ func makeOauthHelperWithJWTKeyThatWorksOnlyOnce( func makeOauthHelperWithNilPrivateJWTSigningKey( t *testing.T, authRequest *http.Request, - store interface { - oauth2.TokenRevocationStorage - oauth2.CoreStorage - openid.OpenIDConnectRequestStorage - pkce.PKCERequestStorage - fosite.ClientManager - }, + store fositestoragei.AllFositeStorage, ) (fosite.OAuth2Provider, string, *ecdsa.PrivateKey) { t.Helper() diff --git a/internal/testutil/oidctestutil/oidctestutil.go b/internal/testutil/oidctestutil/oidctestutil.go new file mode 100644 index 00000000..fdf998d1 --- /dev/null +++ b/internal/testutil/oidctestutil/oidctestutil.go @@ -0,0 +1,469 @@ +// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package oidctestutil + +import ( + "context" + "crypto" + "crypto/ecdsa" + "fmt" + "net/url" + "regexp" + "strings" + "testing" + "time" + + coreosoidc "github.com/coreos/go-oidc/v3/oidc" + "github.com/ory/fosite" + "github.com/ory/fosite/handler/openid" + "github.com/stretchr/testify/require" + "golang.org/x/oauth2" + "gopkg.in/square/go-jose.v2" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apiserver/pkg/authentication/authenticator" + "k8s.io/client-go/kubernetes/fake" + v1 "k8s.io/client-go/kubernetes/typed/core/v1" + + "go.pinniped.dev/internal/crud" + "go.pinniped.dev/internal/fositestorage/authorizationcode" + "go.pinniped.dev/internal/fositestorage/openidconnect" + pkce2 "go.pinniped.dev/internal/fositestorage/pkce" + "go.pinniped.dev/internal/fositestoragei" + "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" +) + +// Test helpers for the OIDC package. + +// ExchangeAuthcodeAndValidateTokenArgs is a POGO (plain old go object?) used to spy on calls to +// TestUpstreamOIDCIdentityProvider.ExchangeAuthcodeAndValidateTokensFunc(). +type ExchangeAuthcodeAndValidateTokenArgs struct { + Ctx context.Context + Authcode string + PKCECodeVerifier pkce.Code + ExpectedIDTokenNonce nonce.Nonce + RedirectURI string +} + +type TestUpstreamLDAPIdentityProvider struct { + Name string + URL string + AuthenticateFunc func(ctx context.Context, username, password string) (*authenticator.Response, bool, error) +} + +func (u *TestUpstreamLDAPIdentityProvider) GetName() string { + return u.Name +} + +func (u *TestUpstreamLDAPIdentityProvider) AuthenticateUser(ctx context.Context, username, password string) (*authenticator.Response, bool, error) { + return u.AuthenticateFunc(ctx, username, password) +} + +func (u *TestUpstreamLDAPIdentityProvider) GetURL() string { + return u.URL +} + +type TestUpstreamOIDCIdentityProvider struct { + Name string + ClientID string + AuthorizationURL url.URL + UsernameClaim string + GroupsClaim string + Scopes []string + ExchangeAuthcodeAndValidateTokensFunc func( + ctx context.Context, + authcode string, + pkceCodeVerifier pkce.Code, + expectedIDTokenNonce nonce.Nonce, + ) (*oidctypes.Token, error) + + exchangeAuthcodeAndValidateTokensCallCount int + exchangeAuthcodeAndValidateTokensArgs []*ExchangeAuthcodeAndValidateTokenArgs +} + +func (u *TestUpstreamOIDCIdentityProvider) GetName() string { + return u.Name +} + +func (u *TestUpstreamOIDCIdentityProvider) GetClientID() string { + return u.ClientID +} + +func (u *TestUpstreamOIDCIdentityProvider) GetAuthorizationURL() *url.URL { + return &u.AuthorizationURL +} + +func (u *TestUpstreamOIDCIdentityProvider) GetScopes() []string { + return u.Scopes +} + +func (u *TestUpstreamOIDCIdentityProvider) GetUsernameClaim() string { + return u.UsernameClaim +} + +func (u *TestUpstreamOIDCIdentityProvider) GetGroupsClaim() string { + return u.GroupsClaim +} + +func (u *TestUpstreamOIDCIdentityProvider) ExchangeAuthcodeAndValidateTokens( + ctx context.Context, + authcode string, + pkceCodeVerifier pkce.Code, + expectedIDTokenNonce nonce.Nonce, + redirectURI string, +) (*oidctypes.Token, error) { + if u.exchangeAuthcodeAndValidateTokensArgs == nil { + u.exchangeAuthcodeAndValidateTokensArgs = make([]*ExchangeAuthcodeAndValidateTokenArgs, 0) + } + u.exchangeAuthcodeAndValidateTokensCallCount++ + u.exchangeAuthcodeAndValidateTokensArgs = append(u.exchangeAuthcodeAndValidateTokensArgs, &ExchangeAuthcodeAndValidateTokenArgs{ + Ctx: ctx, + Authcode: authcode, + PKCECodeVerifier: pkceCodeVerifier, + ExpectedIDTokenNonce: expectedIDTokenNonce, + RedirectURI: redirectURI, + }) + return u.ExchangeAuthcodeAndValidateTokensFunc(ctx, authcode, pkceCodeVerifier, expectedIDTokenNonce) +} + +func (u *TestUpstreamOIDCIdentityProvider) ExchangeAuthcodeAndValidateTokensCallCount() int { + return u.exchangeAuthcodeAndValidateTokensCallCount +} + +func (u *TestUpstreamOIDCIdentityProvider) ExchangeAuthcodeAndValidateTokensArgs(call int) *ExchangeAuthcodeAndValidateTokenArgs { + if u.exchangeAuthcodeAndValidateTokensArgs == nil { + u.exchangeAuthcodeAndValidateTokensArgs = make([]*ExchangeAuthcodeAndValidateTokenArgs, 0) + } + return u.exchangeAuthcodeAndValidateTokensArgs[call] +} + +func (u *TestUpstreamOIDCIdentityProvider) ValidateToken(_ context.Context, _ *oauth2.Token, _ nonce.Nonce) (*oidctypes.Token, error) { + panic("implement me") +} + +type UpstreamIDPListerBuilder struct { + upstreamOIDCIdentityProviders []*TestUpstreamOIDCIdentityProvider + upstreamLDAPIdentityProviders []*TestUpstreamLDAPIdentityProvider +} + +func (b *UpstreamIDPListerBuilder) WithOIDC(upstreamOIDCIdentityProviders ...*TestUpstreamOIDCIdentityProvider) *UpstreamIDPListerBuilder { + b.upstreamOIDCIdentityProviders = append(b.upstreamOIDCIdentityProviders, upstreamOIDCIdentityProviders...) + return b +} + +func (b *UpstreamIDPListerBuilder) WithLDAP(upstreamLDAPIdentityProviders ...*TestUpstreamLDAPIdentityProvider) *UpstreamIDPListerBuilder { + b.upstreamLDAPIdentityProviders = append(b.upstreamLDAPIdentityProviders, upstreamLDAPIdentityProviders...) + return b +} + +func (b *UpstreamIDPListerBuilder) Build() provider.DynamicUpstreamIDPProvider { + idpProvider := provider.NewDynamicUpstreamIDPProvider() + + oidcUpstreams := make([]provider.UpstreamOIDCIdentityProviderI, len(b.upstreamOIDCIdentityProviders)) + for i := range b.upstreamOIDCIdentityProviders { + oidcUpstreams[i] = provider.UpstreamOIDCIdentityProviderI(b.upstreamOIDCIdentityProviders[i]) + } + idpProvider.SetOIDCIdentityProviders(oidcUpstreams) + + ldapUpstreams := make([]provider.UpstreamLDAPIdentityProviderI, len(b.upstreamLDAPIdentityProviders)) + for i := range b.upstreamLDAPIdentityProviders { + ldapUpstreams[i] = provider.UpstreamLDAPIdentityProviderI(b.upstreamLDAPIdentityProviders[i]) + } + idpProvider.SetLDAPIdentityProviders(ldapUpstreams) + + return idpProvider +} + +func NewUpstreamIDPListerBuilder() *UpstreamIDPListerBuilder { + return &UpstreamIDPListerBuilder{} +} + +// Declare a separate type from the production code to ensure that the state param's contents was serialized +// in the format that we expect, with the json keys that we expect, etc. This also ensure that the order of +// the serialized fields is the same, which doesn't really matter expect that we can make simpler equality +// assertions about the redirect URL in this test. +type ExpectedUpstreamStateParamFormat struct { + P string `json:"p"` + U string `json:"u"` + N string `json:"n"` + C string `json:"c"` + 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(_ 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 +} + +func RequireAuthcodeRedirectLocation( + t *testing.T, + actualRedirectLocation string, + wantRedirectLocationRegexp string, + kubeClient *fake.Clientset, + secretsClient v1.SecretInterface, + oauthStore fositestoragei.AllFositeStorage, + wantDownstreamGrantedScopes []string, + wantDownstreamIDTokenSubject string, + wantDownstreamIDTokenUsername string, + wantDownstreamIDTokenGroups []string, + wantDownstreamRequestedScopes []string, + wantDownstreamPKCEChallenge string, + wantDownstreamPKCEChallengeMethod string, + wantDownstreamNonce string, + wantDownstreamClientID string, + wantDownstreamRedirectURI string, +) { + t.Helper() + + // Assert that Location header matches regular expression. + regex := regexp.MustCompile(wantRedirectLocationRegexp) + submatches := regex.FindStringSubmatch(actualRedirectLocation) + require.Lenf(t, submatches, 2, "no regexp match in actualRedirectLocation: %q", actualRedirectLocation) + capturedAuthCode := submatches[1] + + // fosite authcodes are in the format `data.signature`, so grab the signature part, which is the lookup key in the storage interface + authcodeDataAndSignature := strings.Split(capturedAuthCode, ".") + require.Len(t, authcodeDataAndSignature, 2) + + // Several Secrets should have been created + expectedNumberOfCreatedSecrets := 2 + if includesOpenIDScope(wantDownstreamGrantedScopes) { + expectedNumberOfCreatedSecrets++ + } + require.Len(t, kubeClient.Actions(), expectedNumberOfCreatedSecrets) + + // One authcode should have been stored. + testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secretsClient, labels.Set{crud.SecretLabelKey: authorizationcode.TypeLabelValue}, 1) + + storedRequestFromAuthcode, storedSessionFromAuthcode := validateAuthcodeStorage( + t, + oauthStore, + authcodeDataAndSignature[1], // Authcode store key is authcode signature + wantDownstreamGrantedScopes, + wantDownstreamIDTokenSubject, + wantDownstreamIDTokenUsername, + wantDownstreamIDTokenGroups, + wantDownstreamRequestedScopes, + wantDownstreamClientID, + wantDownstreamRedirectURI, + ) + + // One PKCE should have been stored. + testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secretsClient, labels.Set{crud.SecretLabelKey: pkce2.TypeLabelValue}, 1) + + validatePKCEStorage( + t, + oauthStore, + authcodeDataAndSignature[1], // PKCE store key is authcode signature + storedRequestFromAuthcode, + storedSessionFromAuthcode, + wantDownstreamPKCEChallenge, + wantDownstreamPKCEChallengeMethod, + ) + + // One IDSession should have been stored, if the downstream actually requested the "openid" scope + if includesOpenIDScope(wantDownstreamGrantedScopes) { + testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secretsClient, labels.Set{crud.SecretLabelKey: openidconnect.TypeLabelValue}, 1) + + validateIDSessionStorage( + t, + oauthStore, + capturedAuthCode, // IDSession store key is full authcode + storedRequestFromAuthcode, + storedSessionFromAuthcode, + wantDownstreamNonce, + ) + } +} + +func includesOpenIDScope(scopes []string) bool { + for _, scope := range scopes { + if scope == "openid" { + return true + } + } + return false +} + +func validateAuthcodeStorage( + t *testing.T, + oauthStore fositestoragei.AllFositeStorage, + storeKey string, + wantDownstreamGrantedScopes []string, + wantDownstreamIDTokenSubject string, + wantDownstreamIDTokenUsername string, + wantDownstreamIDTokenGroups []string, + wantDownstreamRequestedScopes []string, + wantDownstreamClientID string, + wantDownstreamRedirectURI string, +) (*fosite.Request, *openid.DefaultSession) { + t.Helper() + + const ( + authCodeExpirationSeconds = 10 * 60 // Currently, we set our auth code expiration to 10 minutes + timeComparisonFudgeFactor = time.Second * 15 + ) + + // Get the authcode session back from storage so we can require that it was stored correctly. + storedAuthorizeRequestFromAuthcode, err := oauthStore.GetAuthorizeCodeSession(context.Background(), storeKey, nil) + require.NoError(t, err) + + // Check that storage returned the expected concrete data types. + storedRequestFromAuthcode, storedSessionFromAuthcode := castStoredAuthorizeRequest(t, storedAuthorizeRequestFromAuthcode) + + // Check which scopes were granted. + require.ElementsMatch(t, wantDownstreamGrantedScopes, storedRequestFromAuthcode.GetGrantedScopes()) + + // Check all the other fields of the stored request. + require.NotEmpty(t, storedRequestFromAuthcode.ID) + require.Equal(t, wantDownstreamClientID, storedRequestFromAuthcode.Client.GetID()) + require.ElementsMatch(t, wantDownstreamRequestedScopes, storedRequestFromAuthcode.RequestedScope) + require.Nil(t, storedRequestFromAuthcode.RequestedAudience) + require.Empty(t, storedRequestFromAuthcode.GrantedAudience) + require.Equal(t, url.Values{"redirect_uri": []string{wantDownstreamRedirectURI}}, storedRequestFromAuthcode.Form) + testutil.RequireTimeInDelta(t, time.Now(), storedRequestFromAuthcode.RequestedAt, timeComparisonFudgeFactor) + + // We're not using these fields yet, so confirm that we did not set them (for now). + require.Empty(t, storedSessionFromAuthcode.Subject) + require.Empty(t, storedSessionFromAuthcode.Username) + require.Empty(t, storedSessionFromAuthcode.Headers) + + // The authcode that we are issuing should be good for the length of time that we declare in the fosite config. + testutil.RequireTimeInDelta(t, time.Now().Add(authCodeExpirationSeconds*time.Second), storedSessionFromAuthcode.ExpiresAt[fosite.AuthorizeCode], timeComparisonFudgeFactor) + require.Len(t, storedSessionFromAuthcode.ExpiresAt, 1) + + // Now confirm the ID token claims. + actualClaims := storedSessionFromAuthcode.Claims + + // Check the user's identity, which are put into the downstream ID token's subject, username and groups claims. + require.Equal(t, wantDownstreamIDTokenSubject, actualClaims.Subject) + require.Equal(t, wantDownstreamIDTokenUsername, actualClaims.Extra["username"]) + require.Len(t, actualClaims.Extra, 2) + actualDownstreamIDTokenGroups := actualClaims.Extra["groups"] + require.NotNil(t, actualDownstreamIDTokenGroups) + require.ElementsMatch(t, wantDownstreamIDTokenGroups, actualDownstreamIDTokenGroups) + + // Check the rest of the downstream ID token's claims. Fosite wants us to set these (in UTC time). + testutil.RequireTimeInDelta(t, time.Now().UTC(), actualClaims.RequestedAt, timeComparisonFudgeFactor) + testutil.RequireTimeInDelta(t, time.Now().UTC(), actualClaims.AuthTime, timeComparisonFudgeFactor) + requestedAtZone, _ := actualClaims.RequestedAt.Zone() + require.Equal(t, "UTC", requestedAtZone) + authTimeZone, _ := actualClaims.AuthTime.Zone() + require.Equal(t, "UTC", authTimeZone) + + // 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. + require.Empty(t, actualClaims.JTI) + require.Empty(t, actualClaims.CodeHash) + require.Empty(t, actualClaims.AccessTokenHash) + require.Empty(t, actualClaims.AuthenticationContextClassReference) + require.Empty(t, actualClaims.AuthenticationMethodsReference) + + return storedRequestFromAuthcode, storedSessionFromAuthcode +} + +func validatePKCEStorage( + t *testing.T, + oauthStore fositestoragei.AllFositeStorage, + storeKey string, + storedRequestFromAuthcode *fosite.Request, + storedSessionFromAuthcode *openid.DefaultSession, + wantDownstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod string, +) { + t.Helper() + + storedAuthorizeRequestFromPKCE, err := oauthStore.GetPKCERequestSession(context.Background(), storeKey, nil) + require.NoError(t, err) + + // Check that storage returned the expected concrete data types. + storedRequestFromPKCE, storedSessionFromPKCE := castStoredAuthorizeRequest(t, storedAuthorizeRequestFromPKCE) + + // The stored PKCE request should be the same as the stored authcode request. + require.Equal(t, storedRequestFromAuthcode.ID, storedRequestFromPKCE.ID) + require.Equal(t, storedSessionFromAuthcode, storedSessionFromPKCE) + + // The stored PKCE request should also contain the PKCE challenge that the downstream sent us. + require.Equal(t, wantDownstreamPKCEChallenge, storedRequestFromPKCE.Form.Get("code_challenge")) + require.Equal(t, wantDownstreamPKCEChallengeMethod, storedRequestFromPKCE.Form.Get("code_challenge_method")) +} + +func validateIDSessionStorage( + t *testing.T, + oauthStore fositestoragei.AllFositeStorage, + storeKey string, + storedRequestFromAuthcode *fosite.Request, + storedSessionFromAuthcode *openid.DefaultSession, + wantDownstreamNonce string, +) { + t.Helper() + + storedAuthorizeRequestFromIDSession, err := oauthStore.GetOpenIDConnectSession(context.Background(), storeKey, nil) + require.NoError(t, err) + + // Check that storage returned the expected concrete data types. + storedRequestFromIDSession, storedSessionFromIDSession := castStoredAuthorizeRequest(t, storedAuthorizeRequestFromIDSession) + + // The stored IDSession request should be the same as the stored authcode request. + require.Equal(t, storedRequestFromAuthcode.ID, storedRequestFromIDSession.ID) + require.Equal(t, storedSessionFromAuthcode, storedSessionFromIDSession) + + // The stored IDSession request should also contain the nonce that the downstream sent us. + require.Equal(t, wantDownstreamNonce, storedRequestFromIDSession.Form.Get("nonce")) +} + +func castStoredAuthorizeRequest(t *testing.T, storedAuthorizeRequest fosite.Requester) (*fosite.Request, *openid.DefaultSession) { + t.Helper() + + storedRequest, ok := storedAuthorizeRequest.(*fosite.Request) + require.Truef(t, ok, "could not cast %T to %T", storedAuthorizeRequest, &fosite.Request{}) + storedSession, ok := storedAuthorizeRequest.GetSession().(*openid.DefaultSession) + require.Truef(t, ok, "could not cast %T to %T", storedAuthorizeRequest.GetSession(), &openid.DefaultSession{}) + + return storedRequest, storedSession +}