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 +}