diff --git a/.golangci.yaml b/.golangci.yaml index 1caff55d6..25c44ba65 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,8 +1,7 @@ -# https://github.com/golangci/golangci-lint#config-file +# https://golangci-lint.run/usage/configuration/ run: - deadline: 1m - skip-dirs: - - generated + timeout: 1m + linters: disable-all: true @@ -47,6 +46,8 @@ linters: - whitespace issues: + exclude-dirs: + - generated exclude-rules: # exclude tests from some rules for things that are useful in a testing context. - path: _test\.go diff --git a/pkg/oidcclient/login.go b/pkg/oidcclient/login.go index dd2e4029e..e8424a500 100644 --- a/pkg/oidcclient/login.go +++ b/pkg/oidcclient/login.go @@ -475,14 +475,41 @@ func (h *handlerState) baseLogin() (*oidctypes.Token, error) { h.pkce.Challenge(), h.pkce.Method(), } - if h.upstreamIdentityProviderName != "" { - authorizeOptions = append(authorizeOptions, - oauth2.SetAuthURLParam(oidcapi.AuthorizeUpstreamIDPNameParamName, h.upstreamIdentityProviderName), - ) - // TODO Note that this is the only place where the requested IDP type is used in this file. It is sent as a custom param on the auth request. - authorizeOptions = append(authorizeOptions, - oauth2.SetAuthURLParam(oidcapi.AuthorizeUpstreamIDPTypeParamName, string(h.upstreamIdentityProviderType)), - ) + + pinnipedSupervisorOptions, err := h.maybePerformPinnipedSupervisorValidations() + if err != nil { + return nil, err + } + authorizeOptions = append(authorizeOptions, pinnipedSupervisorOptions...) + + // Preserve the legacy behavior where browser-based auth is preferred + authFunc := h.webBrowserBasedAuth + + // Choose the appropriate authorization and authcode exchange strategy. + // Use a switch so that lint will make sure we have full coverage. + switch h.loginFlow { + case idpdiscoveryv1alpha1.IDPFlowCLIPassword: + authFunc = h.cliBasedAuth + case idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode: + // NOOP + } + + // Perform the authorize request and authcode exchange to get back OIDC tokens. + token, err := authFunc(&authorizeOptions) + + // If we got tokens, put them in the cache. + if err == nil { + h.cache.PutToken(cacheKey, token) + } + + return token, err +} + +// maybePerformPinnipedSupervisorValidations will add some authorization options and perform additional validations if +// the issuer is a Pinniped Supervisor and exposes IDP discovery. +func (h *handlerState) maybePerformPinnipedSupervisorValidations() ([]oauth2.AuthCodeOption, error) { + if h.upstreamIdentityProviderName == "" { + return nil, nil } // TODO Right here is where we have already performed discovery and we are about to need a final choice of flow type. @@ -499,53 +526,72 @@ func (h *handlerState) baseLogin() (*oidctypes.Token, error) { // and you may or may not be talking to a Supervisor in that case. We should continue to allow this to work // by omitting the name and type from the auth request and choosing the web browser flow. - if h.upstreamIdentityProviderName != "" && h.idpDiscovery != nil { - found := slices.ContainsFunc(h.idpDiscovery.PinnipedIDPs, func(idp idpdiscoveryv1alpha1.PinnipedIDP) bool { - return idp.Name == h.upstreamIdentityProviderName && - idp.Type == h.upstreamIdentityProviderType - }) - if !found { - return nil, fmt.Errorf( - "unable to login to IDP with name '%s' and type '%s'", - h.upstreamIdentityProviderName, + if h.idpDiscovery == nil { + return nil, fmt.Errorf("this Pinniped Supervisor does not have IDP discovery, please contact the system administrator to request an upgrade to the Pinniped Supervisor") + } + + // Legacy Pinniped Supervisors do not provide this information + if len(h.idpDiscovery.PinnipedSupportedIDPTypes) > 0 { + var supportedIDPTypes []idpdiscoveryv1alpha1.IDPType + for _, idpType := range h.idpDiscovery.PinnipedSupportedIDPTypes { + supportedIDPTypes = append(supportedIDPTypes, idpType.Type) + } + + // Sort by name for repeatability + slices.Sort(supportedIDPTypes) + + if !slices.Contains(supportedIDPTypes, h.upstreamIdentityProviderType) { + return nil, fmt.Errorf("unable to login to IDP with type %q, this Pinniped Supervisor supports IDP types [%s]", h.upstreamIdentityProviderType, - ) + strings.Join(func() []string { + var temp []string + for _, idpType := range supportedIDPTypes { + temp = append(temp, fmt.Sprintf("%q", idpType)) + } + return temp + }(), ", ")) } } - // If the caller has not requested a specific flow, but has requested a specific IDP, and we're talking to a Pinniped Supervisor, - // infer the authentication flow. - if h.loginFlow == "" && h.upstreamIdentityProviderName != "" && h.idpDiscovery != nil { - for _, idp := range h.idpDiscovery.PinnipedIDPs { - if idp.Name == h.upstreamIdentityProviderName && - idp.Type == h.upstreamIdentityProviderType && // TODO: not sure if this is explicitly tested - len(idp.Flows) > 0 { // TODO: we should test this even if the Supervisor always populates something here - h.loginFlow = idp.Flows[0] - } + foundIDPIndex := slices.IndexFunc(h.idpDiscovery.PinnipedIDPs, func(idp idpdiscoveryv1alpha1.PinnipedIDP) bool { + return idp.Name == h.upstreamIdentityProviderName && + idp.Type == h.upstreamIdentityProviderType && + (h.loginFlow == "" || slices.Contains(idp.Flows, h.loginFlow)) + }) + + if foundIDPIndex < 0 { + if h.loginFlow == "" { + return nil, fmt.Errorf( + "unable to login to IDP with name %q and type %q", + h.upstreamIdentityProviderName, + h.upstreamIdentityProviderType, + ) } + return nil, fmt.Errorf( + "unable to login to IDP with name %q and type %q and flow %q", + h.upstreamIdentityProviderName, + h.upstreamIdentityProviderType, + h.loginFlow, + ) } - // Preserve the legacy behavior where browser-based auth is preferred - authFunc := h.webBrowserBasedAuth + foundIDP := h.idpDiscovery.PinnipedIDPs[foundIDPIndex] - // Choose the appropriate authorization and authcode exchange strategy. - // Use a switch so that lint will make sure we have full coverage. - switch h.loginFlow { - case idpdiscoveryv1alpha1.IDPFlowCLIPassword: - authFunc = h.cliBasedAuth - case idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode: - // NOOP + // If the caller has not requested a specific flow, but has requested a specific IDP, infer the authentication flow. + if h.loginFlow == "" { + h.loginFlow = foundIDP.Flows[0] } - // Perform the authorize request and authcode exchange to get back OIDC tokens. - token, err := authFunc(&authorizeOptions) + var authorizeOptions []oauth2.AuthCodeOption - // If we got tokens, put them in the cache. - if err == nil { - h.cache.PutToken(cacheKey, token) - } + authorizeOptions = append(authorizeOptions, + oauth2.SetAuthURLParam(oidcapi.AuthorizeUpstreamIDPNameParamName, h.upstreamIdentityProviderName), + ) + authorizeOptions = append(authorizeOptions, + oauth2.SetAuthURLParam(oidcapi.AuthorizeUpstreamIDPTypeParamName, string(h.upstreamIdentityProviderType)), + ) - return token, err + return authorizeOptions, nil } // Make a direct call to the authorize endpoint, including the user's username and password on custom http headers, @@ -909,54 +955,52 @@ func (h *handlerState) maybePerformPinnipedSupervisorIDPDiscovery() error { // Note that this library can be used with OIDC IDPs other than Pinniped Supervisor, so var pinnipedSupervisorClaims idpdiscoveryv1alpha1.OIDCDiscoveryResponse if err := h.provider.Claims(&pinnipedSupervisorClaims); err != nil { - // TODO: Test this branch? return fmt.Errorf("could not decode the Pinniped IDP discovery document URL in OIDC discovery from %q: %w", h.issuer, err) } - if pinnipedSupervisorClaims.SupervisorDiscovery.PinnipedIDPsEndpoint != "" { - // This check confirms that the issuer is hosting the IDP discovery document, which would always be the case for - // Pinniped Supervisor. Since there are checks above to confirm that the issuer uses HTTPS, IDP discovery will - // always use HTTPS. - if !strings.HasPrefix(pinnipedSupervisorClaims.SupervisorDiscovery.PinnipedIDPsEndpoint, h.issuer) { - return fmt.Errorf("the Pinniped IDP discovery document must always be hosted by the issuer %q", h.issuer) - } - - idpDiscoveryCtx, idpDiscoveryCtxCancelFunc := context.WithTimeout(h.ctx, httpRequestTimeout) - defer idpDiscoveryCtxCancelFunc() - idpDiscoveryReq, err := http.NewRequestWithContext(idpDiscoveryCtx, http.MethodGet, pinnipedSupervisorClaims.SupervisorDiscovery.PinnipedIDPsEndpoint, nil) - if err != nil { - // TODO: Test this branch? - return fmt.Errorf("could not build IDP Discovery request: %w", err) - } - idpDiscoveryRes, err := h.httpClient.Do(idpDiscoveryReq) - if err != nil { - // TODO: Test this branch? - return fmt.Errorf("IDP Discovery response error: %w", err) - } - defer func() { - _ = idpDiscoveryRes.Body.Close() // We can't do anything if this fails to close - }() + // This is not an error - it just means that this issuer is not a Pinniped Supervisor + if pinnipedSupervisorClaims.SupervisorDiscovery.PinnipedIDPsEndpoint == "" { + return nil + } + // This check confirms that the issuer is hosting the IDP discovery document, which would always be the case for + // Pinniped Supervisor. Since there are checks above to confirm that the issuer uses HTTPS, IDP discovery will + // always use HTTPS. + if !strings.HasPrefix(pinnipedSupervisorClaims.SupervisorDiscovery.PinnipedIDPsEndpoint, h.issuer) { + return fmt.Errorf("the Pinniped IDP discovery document must always be hosted by the issuer: %q", h.issuer) + } - if idpDiscoveryRes.StatusCode != http.StatusOK { - // TODO: Test this branch? - return fmt.Errorf("unable to fetch IDP discovery data from issuer: unexpected http response status: %s", idpDiscoveryRes.Status) - } + idpDiscoveryCtx, idpDiscoveryCtxCancelFunc := context.WithTimeout(h.ctx, httpRequestTimeout) + defer idpDiscoveryCtxCancelFunc() + idpDiscoveryReq, err := http.NewRequestWithContext(idpDiscoveryCtx, http.MethodGet, pinnipedSupervisorClaims.SupervisorDiscovery.PinnipedIDPsEndpoint, nil) + if err != nil { // untested + return fmt.Errorf("could not build IDP Discovery request: %w", err) + } + idpDiscoveryRes, err := h.httpClient.Do(idpDiscoveryReq) + if err != nil { + // TODO: Test this branch? + return fmt.Errorf("IDP Discovery response error: %w", err) + } + defer func() { + _ = idpDiscoveryRes.Body.Close() // We can't do anything if this fails to close + }() - rawBody, err := io.ReadAll(idpDiscoveryRes.Body) - if err != nil { - // TODO: Test this branch? - return fmt.Errorf("unable to fetch IDP discovery data from issuer: could not read response body: %w", err) - } + if idpDiscoveryRes.StatusCode != http.StatusOK { + // TODO: Test this branch? + return fmt.Errorf("unable to fetch IDP discovery data from issuer: unexpected http response status: %s", idpDiscoveryRes.Status) + } - var body idpdiscoveryv1alpha1.IDPDiscoveryResponse - err = json.Unmarshal(rawBody, &body) - if err != nil { - return fmt.Errorf("unable to fetch the Pinniped IDP discovery document: could not parse response JSON: %w", err) - } + rawBody, err := io.ReadAll(idpDiscoveryRes.Body) + if err != nil { // untested + return fmt.Errorf("unable to fetch IDP discovery data from issuer: could not read response body: %w", err) + } - h.idpDiscovery = &body + var body idpdiscoveryv1alpha1.IDPDiscoveryResponse + err = json.Unmarshal(rawBody, &body) + if err != nil { + return fmt.Errorf("unable to fetch the Pinniped IDP discovery document: could not parse response JSON: %w", err) } + h.idpDiscovery = &body return nil } diff --git a/pkg/oidcclient/login_test.go b/pkg/oidcclient/login_test.go index 1d347daf3..7ceff8161 100644 --- a/pkg/oidcclient/login_test.go +++ b/pkg/oidcclient/login_test.go @@ -21,7 +21,7 @@ import ( "testing" "time" - "github.com/coreos/go-oidc/v3/oidc" + coreosoidc "github.com/coreos/go-oidc/v3/oidc" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" @@ -30,6 +30,7 @@ import ( "k8s.io/klog/v2" idpdiscoveryv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idpdiscovery/v1alpha1" + oidcapi "go.pinniped.dev/generated/latest/apis/supervisor/oidc" federationdomainoidc "go.pinniped.dev/internal/federationdomain/oidc" "go.pinniped.dev/internal/federationdomain/upstreamprovider" "go.pinniped.dev/internal/httputil/httperr" @@ -71,7 +72,9 @@ func (m *mockSessionCache) PutToken(key SessionCacheKey, token *oidctypes.Token) func buildHTTPClientForPEM(pemData []byte) *http.Client { pool := x509.NewCertPool() pool.AppendCertsFromPEM(pemData) - return phttp.Default(pool) + client := phttp.Default(pool) + client.Timeout = 2 * time.Second // these are all local requests + return client } func TestLogin(t *testing.T) { //nolint:gocyclo @@ -292,16 +295,16 @@ func TestLogin(t *testing.T) { //nolint:gocyclo _ = json.NewEncoder(w).Encode(idpdiscoveryv1alpha1.IDPDiscoveryResponse{ PinnipedIDPs: []idpdiscoveryv1alpha1.PinnipedIDP{ { - Name: "some-upstream-name", - Type: idpdiscoveryv1alpha1.IDPTypeLDAP, + Name: "upstream-idp-name-with-cli-password-flow-first", + Type: "upstream-idp-type-with-cli-password-flow-first", Flows: []idpdiscoveryv1alpha1.IDPFlow{ idpdiscoveryv1alpha1.IDPFlowCLIPassword, idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode, }, }, { - Name: "some-oidc-upstream-name", - Type: idpdiscoveryv1alpha1.IDPTypeOIDC, + Name: "upstream-idp-name-with-browser-authcode-flow-first", + Type: "upstream-idp-type-with-browser-authcode-flow-first", Flows: []idpdiscoveryv1alpha1.IDPFlow{ idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode, idpdiscoveryv1alpha1.IDPFlowCLIPassword, @@ -309,8 +312,8 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }, }, PinnipedSupportedIDPTypes: []idpdiscoveryv1alpha1.PinnipedSupportedIDPType{ - {Type: idpdiscoveryv1alpha1.IDPTypeOIDC}, - {Type: idpdiscoveryv1alpha1.IDPTypeLDAP}, + {Type: "upstream-idp-type-with-cli-password-flow-first"}, + {Type: "upstream-idp-type-with-browser-authcode-flow-first"}, }, }) } @@ -438,14 +441,14 @@ func TestLogin(t *testing.T) { //nolint:gocyclo ClientID: "test-client-id", Scopes: []string{"test-scope"}, RedirectURI: "http://localhost:0/callback", - UpstreamProviderName: "some-upstream-name", + UpstreamProviderName: "upstream-idp-name-with-cli-password-flow-first", } t.Cleanup(func() { require.Equal(t, []SessionCacheKey{cacheKey}, cache.sawGetKeys) }) require.NoError(t, WithSessionCache(cache)(h)) - require.NoError(t, WithLoginFlow(idpdiscoveryv1alpha1.IDPFlowCLIPassword, "foo")(h)) - require.NoError(t, WithUpstreamIdentityProvider("some-upstream-name", "ldap")(h)) + require.NoError(t, WithLoginFlow(idpdiscoveryv1alpha1.IDPFlowCLIPassword, "flowSource")(h)) + require.NoError(t, WithUpstreamIdentityProvider("upstream-idp-name-with-cli-password-flow-first", "upstream-idp-type-with-cli-password-flow-first")(h)) require.NoError(t, WithClient(buildHTTPClientForPEM(successServerCA))(h)) require.NoError(t, WithClient(&http.Client{ @@ -490,7 +493,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo opt: func(t *testing.T) Option { return func(h *handlerState) error { require.NoError(t, WithCLISendingCredentials()(h)) // This is meant to call a deprecated function - require.NoError(t, WithLoginFlow(idpdiscoveryv1alpha1.IDPFlowCLIPassword, "foo")(h)) + require.NoError(t, WithLoginFlow(idpdiscoveryv1alpha1.IDPFlowCLIPassword, "flowSource")(h)) return nil } }, @@ -501,7 +504,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo opt: func(t *testing.T) Option { return func(h *handlerState) error { require.NoError(t, WithCLISendingCredentials()(h)) // This is meant to call a deprecated function - require.NoError(t, WithLoginFlow(idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode, "foo")(h)) + require.NoError(t, WithLoginFlow(idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode, "flowSource")(h)) return nil } }, @@ -658,7 +661,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo return func(h *handlerState) error { require.NoError(t, WithClient(buildHTTPClientForPEM(successServerCA))(h)) - h.getProvider = func(config *oauth2.Config, provider *oidc.Provider, client *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(config *oauth2.Config, provider *coreosoidc.Provider, client *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ValidateTokenAndMergeWithUserInfo(gomock.Any(), HasAccessToken(testToken.AccessToken.Token), nonce.Nonce(""), true, false). @@ -709,7 +712,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo return func(h *handlerState) error { require.NoError(t, WithClient(buildHTTPClientForPEM(successServerCA))(h)) - h.getProvider = func(config *oauth2.Config, provider *oidc.Provider, client *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(config *oauth2.Config, provider *coreosoidc.Provider, client *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ValidateTokenAndMergeWithUserInfo(gomock.Any(), HasAccessToken(testToken.AccessToken.Token), nonce.Nonce(""), true, false). @@ -837,7 +840,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }, issuer: emptyIDPDiscoveryServer.URL, wantLogs: []string{`"level"=4 "msg"="Pinniped: Performing OIDC discovery" "issuer"="` + emptyIDPDiscoveryServer.URL + `"`}, - wantErr: fmt.Sprintf(`the Pinniped IDP discovery document must always be hosted by the issuer %q`, emptyIDPDiscoveryServer.URL), + wantErr: fmt.Sprintf(`the Pinniped IDP discovery document must always be hosted by the issuer: %q`, emptyIDPDiscoveryServer.URL), }, { name: "issuer has Pinniped Supervisor's IDP discovery, but it cannot be unmarshaled", @@ -1401,6 +1404,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo wantToken: &testToken, }, { + // TODO: This test name says that "upstream type" is included in the session cache key but I don't see that below. name: "upstream name and type are included in authorize request and session cache key if upstream name is provided", clientID: "test-client-id", opt: func(t *testing.T) Option { @@ -1417,7 +1421,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo ClientID: "test-client-id", Scopes: []string{"test-scope"}, RedirectURI: "http://localhost:0/callback", - UpstreamProviderName: "some-oidc-upstream-name", + UpstreamProviderName: "upstream-idp-name-with-browser-authcode-flow-first", } t.Cleanup(func() { require.Equal(t, []SessionCacheKey{cacheKey}, cache.sawGetKeys) @@ -1425,7 +1429,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo require.Equal(t, []*oidctypes.Token{&testToken}, cache.sawPutTokens) }) require.NoError(t, WithSessionCache(cache)(h)) - require.NoError(t, WithUpstreamIdentityProvider("some-oidc-upstream-name", "oidc")(h)) + require.NoError(t, WithUpstreamIdentityProvider("upstream-idp-name-with-browser-authcode-flow-first", "upstream-idp-type-with-browser-authcode-flow-first")(h)) client := buildHTTPClientForPEM(successServerCA) client.Timeout = 10 * time.Second @@ -1448,8 +1452,8 @@ func TestLogin(t *testing.T) { //nolint:gocyclo "state": []string{"test-state"}, "access_type": []string{"offline"}, "client_id": []string{"test-client-id"}, - "pinniped_idp_name": []string{"some-oidc-upstream-name"}, - "pinniped_idp_type": []string{"oidc"}, + "pinniped_idp_name": []string{"upstream-idp-name-with-browser-authcode-flow-first"}, + "pinniped_idp_type": []string{"upstream-idp-type-with-browser-authcode-flow-first"}, }, actualParams) parsedActualURL.RawQuery = "" @@ -1470,7 +1474,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo regexp.QuoteMeta(" https://127.0.0.1:") + "[0-9]+" + // random port regexp.QuoteMeta("/authorize?access_type=offline&client_id=test-client-id&code_challenge="+testCodeChallenge+ - "&code_challenge_method=S256&nonce=test-nonce&pinniped_idp_name=some-oidc-upstream-name&pinniped_idp_type=oidc"+ + "&code_challenge_method=S256&nonce=test-nonce&pinniped_idp_name=upstream-idp-name-with-browser-authcode-flow-first&pinniped_idp_type=upstream-idp-type-with-browser-authcode-flow-first"+ "&redirect_uri=http%3A%2F%2F127.0.0.1%3A") + "[0-9]+" + // random port regexp.QuoteMeta("%2Fcallback&response_type=code&scope=test-scope&state=test-state") + @@ -1493,7 +1497,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }, issuer: successServer.URL, wantLogs: []string{`"level"=4 "msg"="Pinniped: Performing OIDC discovery" "issuer"="` + successServer.URL + `"`}, - wantStdErr: "^\nLog in to some-upstream-name\n\n$", + wantStdErr: "^\nLog in to upstream-idp-name-with-cli-password-flow-first\n\n$", wantErr: "error prompting for username: some prompt error", }, { @@ -1508,7 +1512,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }, issuer: successServer.URL, wantLogs: []string{`"level"=4 "msg"="Pinniped: Performing OIDC discovery" "issuer"="` + successServer.URL + `"`}, - wantStdErr: "^\nLog in to some-upstream-name\n\n$", + wantStdErr: "^\nLog in to upstream-idp-name-with-cli-password-flow-first\n\n$", wantErr: "error prompting for password: some prompt error", }, { @@ -1563,11 +1567,11 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }, issuer: successServer.URL, wantLogs: []string{`"level"=4 "msg"="Pinniped: Performing OIDC discovery" "issuer"="` + successServer.URL + `"`}, - wantStdErr: "^\nLog in to some-upstream-name\n\n$", + wantStdErr: "^\nLog in to upstream-idp-name-with-cli-password-flow-first\n\n$", wantErr: `authorization response error: Get "https://` + successServer.Listener.Addr().String() + `/authorize?access_type=offline&client_id=test-client-id&code_challenge=` + testCodeChallenge + - `&code_challenge_method=S256&nonce=test-nonce&pinniped_idp_name=some-upstream-name&` + - `pinniped_idp_type=ldap&redirect_uri=http%3A%2F%2F127.0.0.1%3A0%2Fcallback&response_type=code` + + `&code_challenge_method=S256&nonce=test-nonce&pinniped_idp_name=upstream-idp-name-with-cli-password-flow-first&` + + `pinniped_idp_type=upstream-idp-type-with-cli-password-flow-first&redirect_uri=http%3A%2F%2F127.0.0.1%3A0%2Fcallback&response_type=code` + `&scope=test-scope&state=test-state": some error fetching authorize endpoint`, }, { @@ -1580,7 +1584,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }, issuer: successServer.URL, wantLogs: []string{`"level"=4 "msg"="Pinniped: Performing OIDC discovery" "issuer"="` + successServer.URL + `"`}, - wantStdErr: "^\nLog in to some-upstream-name\n\n$", + wantStdErr: "^\nLog in to upstream-idp-name-with-cli-password-flow-first\n\n$", wantErr: `error getting authorization: expected to be redirected, but response status was 502 Bad Gateway`, }, { @@ -1598,7 +1602,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }, issuer: successServer.URL, wantLogs: []string{`"level"=4 "msg"="Pinniped: Performing OIDC discovery" "issuer"="` + successServer.URL + `"`}, - wantStdErr: "^\nLog in to some-upstream-name\n\n$", + wantStdErr: "^\nLog in to upstream-idp-name-with-cli-password-flow-first\n\n$", wantErr: `login failed with code "access_denied": optional-error-description`, }, { @@ -1616,7 +1620,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }, issuer: successServer.URL, wantLogs: []string{`"level"=4 "msg"="Pinniped: Performing OIDC discovery" "issuer"="` + successServer.URL + `"`}, - wantStdErr: "^\nLog in to some-upstream-name\n\n$", + wantStdErr: "^\nLog in to upstream-idp-name-with-cli-password-flow-first\n\n$", wantErr: `error getting authorization: redirected to the wrong location: http://other-server.example.com/callback?code=foo&state=test-state`, }, { @@ -1634,7 +1638,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }, issuer: successServer.URL, wantLogs: []string{`"level"=4 "msg"="Pinniped: Performing OIDC discovery" "issuer"="` + successServer.URL + `"`}, - wantStdErr: "^\nLog in to some-upstream-name\n\n$", + wantStdErr: "^\nLog in to upstream-idp-name-with-cli-password-flow-first\n\n$", wantErr: `login failed with code "access_denied"`, }, { @@ -1650,7 +1654,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }, issuer: successServer.URL, wantLogs: []string{`"level"=4 "msg"="Pinniped: Performing OIDC discovery" "issuer"="` + successServer.URL + `"`}, - wantStdErr: "^\nLog in to some-upstream-name\n\n$", + wantStdErr: "^\nLog in to upstream-idp-name-with-cli-password-flow-first\n\n$", wantErr: `missing or invalid state parameter in authorization response: http://127.0.0.1:0/callback?code=foo&state=wrong-state`, }, { @@ -1665,7 +1669,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo fmt.Sprintf("http://127.0.0.1:0/callback?code=%s&state=test-state", fakeAuthCode), }}, }, nil) - h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(_ *oauth2.Config, _ *coreosoidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ExchangeAuthcodeAndValidateTokens( @@ -1678,7 +1682,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }, issuer: successServer.URL, wantLogs: []string{`"level"=4 "msg"="Pinniped: Performing OIDC discovery" "issuer"="` + successServer.URL + `"`}, - wantStdErr: "^\nLog in to some-upstream-name\n\n$", + wantStdErr: "^\nLog in to upstream-idp-name-with-cli-password-flow-first\n\n$", wantErr: "could not complete authorization code exchange: some authcode exchange or token validation error", }, { @@ -1688,7 +1692,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo return func(h *handlerState) error { fakeAuthCode := "test-authcode-value" - h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(_ *oauth2.Config, _ *coreosoidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ExchangeAuthcodeAndValidateTokens( @@ -1718,7 +1722,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo ClientID: "test-client-id", Scopes: []string{"test-scope"}, RedirectURI: "http://localhost:0/callback", - UpstreamProviderName: "some-upstream-name", + UpstreamProviderName: "upstream-idp-name-with-cli-password-flow-first", } t.Cleanup(func() { require.Equal(t, []SessionCacheKey{cacheKey}, cache.sawGetKeys) @@ -1726,8 +1730,8 @@ func TestLogin(t *testing.T) { //nolint:gocyclo require.Equal(t, []*oidctypes.Token{&testToken}, cache.sawPutTokens) }) require.NoError(t, WithSessionCache(cache)(h)) - require.NoError(t, WithLoginFlow(idpdiscoveryv1alpha1.IDPFlowCLIPassword, "foo")(h)) - require.NoError(t, WithUpstreamIdentityProvider("some-upstream-name", "ldap")(h)) + require.NoError(t, WithLoginFlow(idpdiscoveryv1alpha1.IDPFlowCLIPassword, "flowSource")(h)) + require.NoError(t, WithUpstreamIdentityProvider("upstream-idp-name-with-cli-password-flow-first", "upstream-idp-type-with-cli-password-flow-first")(h)) discoveryRequestWasMade := false authorizeRequestWasMade := false @@ -1758,8 +1762,8 @@ func TestLogin(t *testing.T) { //nolint:gocyclo "access_type": []string{"offline"}, "client_id": []string{"test-client-id"}, "redirect_uri": []string{"http://127.0.0.1:0/callback"}, - "pinniped_idp_name": []string{"some-upstream-name"}, - "pinniped_idp_type": []string{"ldap"}, + "pinniped_idp_name": []string{"upstream-idp-name-with-cli-password-flow-first"}, + "pinniped_idp_type": []string{"upstream-idp-type-with-cli-password-flow-first"}, }, req.URL.Query()) return &http.Response{ StatusCode: http.StatusFound, @@ -1779,7 +1783,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }, issuer: successServer.URL, wantLogs: []string{`"level"=4 "msg"="Pinniped: Performing OIDC discovery" "issuer"="` + successServer.URL + `"`}, - wantStdErr: "^\nLog in to some-upstream-name\n\n$", + wantStdErr: "^\nLog in to upstream-idp-name-with-cli-password-flow-first\n\n$", wantToken: &testToken, }, { @@ -1808,7 +1812,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo ClientID: "test-client-id", Scopes: []string{"test-scope"}, RedirectURI: "http://localhost:0/callback", - UpstreamProviderName: "some-upstream-name", + UpstreamProviderName: "upstream-idp-name-with-cli-password-flow-first", } t.Cleanup(func() { require.Equal(t, []SessionCacheKey{cacheKey}, cache.sawGetKeys) @@ -1816,8 +1820,8 @@ func TestLogin(t *testing.T) { //nolint:gocyclo require.Equal(t, []*oidctypes.Token(nil), cache.sawPutTokens) }) require.NoError(t, WithSessionCache(cache)(h)) - require.NoError(t, WithLoginFlow(idpdiscoveryv1alpha1.IDPFlowCLIPassword, "foo")(h)) - require.NoError(t, WithUpstreamIdentityProvider("some-upstream-name", "INVALID UPSTREAM TYPE")(h)) + require.NoError(t, WithLoginFlow(idpdiscoveryv1alpha1.IDPFlowCLIPassword, "flowSource")(h)) + require.NoError(t, WithUpstreamIdentityProvider("upstream-idp-name-with-cli-password-flow-first", "INVALID UPSTREAM TYPE")(h)) discoveryRequestWasMade := false idpDiscoveryRequestWasMade := false @@ -1847,7 +1851,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }, issuer: successServer.URL, wantLogs: []string{`"level"=4 "msg"="Pinniped: Performing OIDC discovery" "issuer"="` + successServer.URL + `"`}, - wantErr: "unable to login to IDP with name 'some-upstream-name' and type 'INVALID UPSTREAM TYPE'", + wantErr: `unable to login to IDP with type "INVALID UPSTREAM TYPE", this Pinniped Supervisor supports IDP types ["upstream-idp-type-with-browser-authcode-flow-first", "upstream-idp-type-with-cli-password-flow-first"]`, }, { name: "successful ldap login with prompts for username and password, using deprecated WithCLISendingCredentials option", @@ -1856,7 +1860,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo return func(h *handlerState) error { fakeAuthCode := "test-authcode-value" - h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(_ *oauth2.Config, _ *coreosoidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ExchangeAuthcodeAndValidateTokens( @@ -1886,7 +1890,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo ClientID: "test-client-id", Scopes: []string{"test-scope"}, RedirectURI: "http://localhost:0/callback", - UpstreamProviderName: "some-upstream-name", + UpstreamProviderName: "upstream-idp-name-with-cli-password-flow-first", } t.Cleanup(func() { require.Equal(t, []SessionCacheKey{cacheKey}, cache.sawGetKeys) @@ -1895,7 +1899,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }) require.NoError(t, WithSessionCache(cache)(h)) require.NoError(t, WithCLISendingCredentials()(h)) // This is meant to call a deprecated function - require.NoError(t, WithUpstreamIdentityProvider("some-upstream-name", "ldap")(h)) + require.NoError(t, WithUpstreamIdentityProvider("upstream-idp-name-with-cli-password-flow-first", "upstream-idp-type-with-cli-password-flow-first")(h)) discoveryRequestWasMade := false authorizeRequestWasMade := false @@ -1926,8 +1930,8 @@ func TestLogin(t *testing.T) { //nolint:gocyclo "access_type": []string{"offline"}, "client_id": []string{"test-client-id"}, "redirect_uri": []string{"http://127.0.0.1:0/callback"}, - "pinniped_idp_name": []string{"some-upstream-name"}, - "pinniped_idp_type": []string{"ldap"}, + "pinniped_idp_name": []string{"upstream-idp-name-with-cli-password-flow-first"}, + "pinniped_idp_type": []string{"upstream-idp-type-with-cli-password-flow-first"}, }, req.URL.Query()) return &http.Response{ StatusCode: http.StatusFound, @@ -1947,7 +1951,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }, issuer: successServer.URL, wantLogs: []string{`"level"=4 "msg"="Pinniped: Performing OIDC discovery" "issuer"="` + successServer.URL + `"`}, - wantStdErr: "^\nLog in to some-upstream-name\n\n$", + wantStdErr: "^\nLog in to upstream-idp-name-with-cli-password-flow-first\n\n$", wantToken: &testToken, }, { @@ -1957,7 +1961,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo return func(h *handlerState) error { fakeAuthCode := "test-authcode-value" - h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(_ *oauth2.Config, _ *coreosoidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ExchangeAuthcodeAndValidateTokens( @@ -1987,7 +1991,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo ClientID: "test-client-id", Scopes: []string{"test-scope"}, RedirectURI: "http://localhost:0/callback", - UpstreamProviderName: "some-upstream-name", + UpstreamProviderName: "upstream-idp-name-with-cli-password-flow-first", } t.Cleanup(func() { require.Equal(t, []SessionCacheKey{cacheKey}, cache.sawGetKeys) @@ -1995,7 +1999,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo require.Equal(t, []*oidctypes.Token{&testToken}, cache.sawPutTokens) }) require.NoError(t, WithSessionCache(cache)(h)) - require.NoError(t, WithUpstreamIdentityProvider("some-upstream-name", "ldap")(h)) + require.NoError(t, WithUpstreamIdentityProvider("upstream-idp-name-with-cli-password-flow-first", "upstream-idp-type-with-cli-password-flow-first")(h)) discoveryRequestWasMade := false idpDiscoveryRequestWasMade := false @@ -2029,8 +2033,8 @@ func TestLogin(t *testing.T) { //nolint:gocyclo "access_type": []string{"offline"}, "client_id": []string{"test-client-id"}, "redirect_uri": []string{"http://127.0.0.1:0/callback"}, - "pinniped_idp_name": []string{"some-upstream-name"}, - "pinniped_idp_type": []string{"ldap"}, + "pinniped_idp_name": []string{"upstream-idp-name-with-cli-password-flow-first"}, + "pinniped_idp_type": []string{"upstream-idp-type-with-cli-password-flow-first"}, }, req.URL.Query()) return &http.Response{ StatusCode: http.StatusFound, @@ -2050,7 +2054,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }, issuer: successServer.URL, wantLogs: []string{`"level"=4 "msg"="Pinniped: Performing OIDC discovery" "issuer"="` + successServer.URL + `"`}, - wantStdErr: "^\nLog in to some-upstream-name\n\n$", + wantStdErr: "^\nLog in to upstream-idp-name-with-cli-password-flow-first\n\n$", wantToken: &testToken, }, { @@ -2060,7 +2064,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo return func(h *handlerState) error { fakeAuthCode := "test-authcode-value" - h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(_ *oauth2.Config, _ *coreosoidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ExchangeAuthcodeAndValidateTokens( @@ -2104,7 +2108,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo require.Equal(t, []*oidctypes.Token{&testToken}, cache.sawPutTokens) }) require.NoError(t, WithSessionCache(cache)(h)) - require.NoError(t, WithLoginFlow(idpdiscoveryv1alpha1.IDPFlowCLIPassword, "foo")(h)) + require.NoError(t, WithLoginFlow(idpdiscoveryv1alpha1.IDPFlowCLIPassword, "flowSource")(h)) discoveryRequestWasMade := false authorizeRequestWasMade := false @@ -2167,7 +2171,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo return func(h *handlerState) error { fakeAuthCode := "test-authcode-value" - h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(_ *oauth2.Config, _ *coreosoidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ExchangeAuthcodeAndValidateTokens( @@ -2204,7 +2208,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo ClientID: "test-client-id", Scopes: []string{"test-scope"}, RedirectURI: "http://localhost:0/callback", - UpstreamProviderName: "some-upstream-name", + UpstreamProviderName: "upstream-idp-name-with-cli-password-flow-first", } t.Cleanup(func() { require.Equal(t, []SessionCacheKey{cacheKey}, cache.sawGetKeys) @@ -2212,8 +2216,8 @@ func TestLogin(t *testing.T) { //nolint:gocyclo require.Equal(t, []*oidctypes.Token{&testToken}, cache.sawPutTokens) }) require.NoError(t, WithSessionCache(cache)(h)) - require.NoError(t, WithLoginFlow(idpdiscoveryv1alpha1.IDPFlowCLIPassword, "foo")(h)) - require.NoError(t, WithUpstreamIdentityProvider("some-upstream-name", "ldap")(h)) + require.NoError(t, WithLoginFlow(idpdiscoveryv1alpha1.IDPFlowCLIPassword, "flowSource")(h)) + require.NoError(t, WithUpstreamIdentityProvider("upstream-idp-name-with-cli-password-flow-first", "upstream-idp-type-with-cli-password-flow-first")(h)) discoveryRequestWasMade := false authorizeRequestWasMade := false @@ -2244,8 +2248,8 @@ func TestLogin(t *testing.T) { //nolint:gocyclo "access_type": []string{"offline"}, "client_id": []string{"test-client-id"}, "redirect_uri": []string{"http://127.0.0.1:0/callback"}, - "pinniped_idp_name": []string{"some-upstream-name"}, - "pinniped_idp_type": []string{"ldap"}, + "pinniped_idp_name": []string{"upstream-idp-name-with-cli-password-flow-first"}, + "pinniped_idp_type": []string{"upstream-idp-type-with-cli-password-flow-first"}, }, req.URL.Query()) return &http.Response{ StatusCode: http.StatusSeeOther, @@ -2269,7 +2273,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo `"level"=4 "msg"="Pinniped: Read username from environment variable" "name"="PINNIPED_USERNAME"`, `"level"=4 "msg"="Pinniped: Read password from environment variable" "name"="PINNIPED_PASSWORD"`, }, - wantStdErr: "^\nLog in to some-upstream-name\n\n$", + wantStdErr: "^\nLog in to upstream-idp-name-with-cli-password-flow-first\n\n$", wantToken: &testToken, }, { @@ -2611,10 +2615,10 @@ func TestLogin(t *testing.T) { //nolint:gocyclo require.NoError(t, WithSessionCache(cache)(h)) require.NoError(t, WithRequestAudience("test-audience")(h)) - h.validateIDToken = func(ctx context.Context, provider *oidc.Provider, audience string, token string) (*oidc.IDToken, error) { + h.validateIDToken = func(ctx context.Context, provider *coreosoidc.Provider, audience string, token string) (*coreosoidc.IDToken, error) { require.Equal(t, "test-audience", audience) require.Equal(t, "test-id-token-with-requested-audience", token) - return &oidc.IDToken{Expiry: testExchangedToken.IDToken.Expiry.Time}, nil + return &coreosoidc.IDToken{Expiry: testExchangedToken.IDToken.Expiry.Time}, nil } return nil } @@ -2654,7 +2658,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo require.NoError(t, WithSessionCache(cache)(h)) require.NoError(t, WithRequestAudience("request-this-test-audience")(h)) - h.validateIDToken = func(ctx context.Context, provider *oidc.Provider, audience string, token string) (*oidc.IDToken, error) { + h.validateIDToken = func(ctx context.Context, provider *coreosoidc.Provider, audience string, token string) (*coreosoidc.IDToken, error) { require.FailNow(t, "should not have performed a token exchange because the cached ID token already had the requested audience") return nil, nil } @@ -2662,7 +2666,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo } }, wantLogs: []string{`"level"=4 "msg"="Pinniped: Found unexpired cached token." "type"="id_token"`}, - wantToken: &oidctypes.Token{ // the same tokens that were pulled from from the cache + wantToken: &oidctypes.Token{ // the same tokens that were pulled from the cache AccessToken: testToken.AccessToken, IDToken: &oidctypes.IDToken{ Token: testToken.IDToken.Token, @@ -2706,7 +2710,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo require.NoError(t, WithSessionCache(cache)(h)) require.NoError(t, WithRequestAudience("test-custom-request-audience")(h)) - h.getProvider = func(config *oauth2.Config, provider *oidc.Provider, client *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(config *oauth2.Config, provider *coreosoidc.Provider, client *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ValidateTokenAndMergeWithUserInfo(gomock.Any(), HasAccessToken(testToken.AccessToken.Token), nonce.Nonce(""), true, false). @@ -2768,10 +2772,10 @@ func TestLogin(t *testing.T) { //nolint:gocyclo require.NoError(t, WithSessionCache(cache)(h)) require.NoError(t, WithRequestAudience("request-this-test-audience")(h)) - h.validateIDToken = func(ctx context.Context, provider *oidc.Provider, audience string, token string) (*oidc.IDToken, error) { + h.validateIDToken = func(ctx context.Context, provider *coreosoidc.Provider, audience string, token string) (*coreosoidc.IDToken, error) { require.Equal(t, "request-this-test-audience", audience) require.Equal(t, "test-id-token-with-requested-audience", token) - return &oidc.IDToken{Expiry: testExchangedToken.IDToken.Expiry.Time}, nil + return &coreosoidc.IDToken{Expiry: testExchangedToken.IDToken.Expiry.Time}, nil } return nil } @@ -2816,7 +2820,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }) h.cache = cache - h.getProvider = func(config *oauth2.Config, provider *oidc.Provider, client *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(config *oauth2.Config, provider *coreosoidc.Provider, client *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ValidateTokenAndMergeWithUserInfo(gomock.Any(), HasAccessToken(testToken.AccessToken.Token), nonce.Nonce(""), true, false). @@ -2832,10 +2836,10 @@ func TestLogin(t *testing.T) { //nolint:gocyclo require.NoError(t, WithRequestAudience("test-audience")(h)) - h.validateIDToken = func(ctx context.Context, provider *oidc.Provider, audience string, token string) (*oidc.IDToken, error) { + h.validateIDToken = func(ctx context.Context, provider *coreosoidc.Provider, audience string, token string) (*coreosoidc.IDToken, error) { require.Equal(t, "test-audience", audience) require.Equal(t, "test-id-token-with-requested-audience", token) - return &oidc.IDToken{Expiry: testExchangedToken.IDToken.Expiry.Time}, nil + return &coreosoidc.IDToken{Expiry: testExchangedToken.IDToken.Expiry.Time}, nil } return nil } @@ -2993,7 +2997,7 @@ func TestHandlePasteCallback(t *testing.T) { return "invalid", nil } h.oauth2Config = &oauth2.Config{RedirectURL: testRedirectURI} - h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(_ *oauth2.Config, _ *coreosoidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ExchangeAuthcodeAndValidateTokens(gomock.Any(), "invalid", pkce.Code("test-pkce"), nonce.Nonce("test-nonce"), testRedirectURI). @@ -3020,7 +3024,7 @@ func TestHandlePasteCallback(t *testing.T) { return "valid", nil } h.oauth2Config = &oauth2.Config{RedirectURL: testRedirectURI} - h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(_ *oauth2.Config, _ *coreosoidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ExchangeAuthcodeAndValidateTokens(gomock.Any(), "valid", pkce.Code("test-pkce"), nonce.Nonce("test-nonce"), testRedirectURI). @@ -3047,7 +3051,7 @@ func TestHandlePasteCallback(t *testing.T) { return "valid", nil } h.oauth2Config = &oauth2.Config{RedirectURL: testRedirectURI} - h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(_ *oauth2.Config, _ *coreosoidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ExchangeAuthcodeAndValidateTokens(gomock.Any(), "valid", pkce.Code("test-pkce"), nonce.Nonce("test-nonce"), testRedirectURI). @@ -3286,7 +3290,7 @@ func TestHandleAuthCodeCallback(t *testing.T) { opt: func(t *testing.T) Option { return func(h *handlerState) error { h.oauth2Config = &oauth2.Config{RedirectURL: testRedirectURI} - h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(_ *oauth2.Config, _ *coreosoidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ExchangeAuthcodeAndValidateTokens(gomock.Any(), "invalid", pkce.Code("test-pkce"), nonce.Nonce("test-nonce"), testRedirectURI). @@ -3310,7 +3314,7 @@ func TestHandleAuthCodeCallback(t *testing.T) { opt: func(t *testing.T) Option { return func(h *handlerState) error { h.oauth2Config = &oauth2.Config{RedirectURL: testRedirectURI} - h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(_ *oauth2.Config, _ *coreosoidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ExchangeAuthcodeAndValidateTokens(gomock.Any(), "valid", pkce.Code("test-pkce"), nonce.Nonce("test-nonce"), testRedirectURI). @@ -3335,7 +3339,7 @@ func TestHandleAuthCodeCallback(t *testing.T) { opt: func(t *testing.T) Option { return func(h *handlerState) error { h.oauth2Config = &oauth2.Config{RedirectURL: testRedirectURI} - h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(_ *oauth2.Config, _ *coreosoidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ExchangeAuthcodeAndValidateTokens(gomock.Any(), "valid", pkce.Code("test-pkce"), nonce.Nonce("test-nonce"), testRedirectURI). @@ -3363,7 +3367,7 @@ func TestHandleAuthCodeCallback(t *testing.T) { opt: func(t *testing.T) Option { return func(h *handlerState) error { h.oauth2Config = &oauth2.Config{RedirectURL: testRedirectURI} - h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { + h.getProvider = func(_ *oauth2.Config, _ *coreosoidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). ExchangeAuthcodeAndValidateTokens(gomock.Any(), "valid", pkce.Code("test-pkce"), nonce.Nonce("test-nonce"), testRedirectURI). @@ -3483,3 +3487,216 @@ func (m hasAccessTokenMatcher) String() string { func HasAccessToken(expected string) gomock.Matcher { return hasAccessTokenMatcher{expected: expected} } + +func TestMaybePerformPinnipedSupervisorIDPDiscovery(t *testing.T) { + withProvider := func(t *testing.T, issuerURL string) Option { + return func(h *handlerState) error { + t.Helper() + + cancelCtx, cancel := context.WithCancel(context.Background()) + t.Cleanup(func() { + cancel() + }) + + cancelCtx = coreosoidc.ClientContext(cancelCtx, h.httpClient) + provider, err := coreosoidc.NewProvider(cancelCtx, issuerURL) + require.NoError(t, err) + h.provider = provider + return nil + } + } + + for _, tt := range []struct { + name string + options []Option + pinnipedDiscovery string + wantErr string + }{ + { + name: "not a Supervisor returns nothing", + }, + { + name: "Supervisor returns empty discovery information returns nothing", + pinnipedDiscovery: `{"pinniped_identity_providers_endpoint":""}`, + }, + { + name: "Supervisor returns invalid discovery information", + pinnipedDiscovery: `"not-valid-discovery-claim"`, + wantErr: `could not decode the Pinniped IDP discovery document URL in OIDC discovery from "FAKE-ISSUER": json: cannot unmarshal string into Go struct field OIDCDiscoveryResponse.discovery.supervisor.pinniped.dev/v1alpha1 of type v1alpha1.OIDCDiscoveryResponseIDPEndpoint`, + }, + { + name: "Supervisor has invalid pinniped_identity_providers_endpoint", + pinnipedDiscovery: `{"pinniped_identity_providers_endpoint":"asdf"}`, + wantErr: `the Pinniped IDP discovery document must always be hosted by the issuer: "FAKE-ISSUER"`, + }, + } { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + issuerMux := http.NewServeMux() + issuerServer, issuerServerCA := tlsserver.TestServerIPv4(t, issuerMux, nil) + + issuerMux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("content-type", "application/json") + _, _ = fmt.Fprintf(w, `{"issuer": %q`, issuerServer.URL) + if len(tt.pinnipedDiscovery) > 0 { + _, _ = fmt.Fprintf(w, `, "discovery.supervisor.pinniped.dev/v1alpha1": %s`, tt.pinnipedDiscovery) + } + _, _ = fmt.Fprint(w, `}`) + }) + + var h handlerState + h.issuer = "FAKE-ISSUER" + require.NoError(t, WithClient(buildHTTPClientForPEM(issuerServerCA))(&h)) + require.NoError(t, withProvider(t, issuerServer.URL)(&h)) + for _, option := range tt.options { + require.NoError(t, option(&h)) + } + + actualError := h.maybePerformPinnipedSupervisorIDPDiscovery() + + if tt.wantErr != "" { + require.EqualError(t, actualError, tt.wantErr) + return + } + + require.NoError(t, actualError) + }) + } +} + +func TestMaybePerformPinnipedSupervisorValidations(t *testing.T) { + withIDPDiscovery := func(idpDiscovery idpdiscoveryv1alpha1.IDPDiscoveryResponse) Option { + return func(h *handlerState) error { + h.idpDiscovery = &idpDiscovery + return nil + } + } + + someIDPDiscoveryResponse := idpdiscoveryv1alpha1.IDPDiscoveryResponse{ + PinnipedIDPs: []idpdiscoveryv1alpha1.PinnipedIDP{ + { + Name: "some-upstream-name", + Type: "some-upstream-type", + Flows: []idpdiscoveryv1alpha1.IDPFlow{ + idpdiscoveryv1alpha1.IDPFlowCLIPassword, + idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode, + }, + }, + { + Name: "idp-name-with-cli-password-only-flow", + Type: "idp-type-with-cli-password-only-flow", + Flows: []idpdiscoveryv1alpha1.IDPFlow{ + idpdiscoveryv1alpha1.IDPFlowCLIPassword, + }, + }, + { + Name: "idp-name-with-no-flows", + Type: "idp-type-with-no-flows", + }, + }, + PinnipedSupportedIDPTypes: []idpdiscoveryv1alpha1.PinnipedSupportedIDPType{ + {Type: "some-upstream-type"}, + {Type: "idp-type-with-cli-password-only-flow"}, + {Type: "idp-type-with-no-flows"}, + {Type: "other-supported-type-with-no-idp"}, + }, + } + + for _, tt := range []struct { + name string + options []Option + wantAuthCodeOptions []oauth2.AuthCodeOption + wantLoginFlow idpdiscoveryv1alpha1.IDPFlow + wantErr string + }{ + { + name: "not a Supervisor returns nothing", + }, + { + name: "with IDP name and IDP type, returns the right AuthCodeOptions and infers the loginFlow", + options: []Option{ + WithUpstreamIdentityProvider("some-upstream-name", "some-upstream-type"), + withIDPDiscovery(someIDPDiscoveryResponse), + }, + wantAuthCodeOptions: []oauth2.AuthCodeOption{ + oauth2.SetAuthURLParam(oidcapi.AuthorizeUpstreamIDPNameParamName, "some-upstream-name"), + oauth2.SetAuthURLParam(oidcapi.AuthorizeUpstreamIDPTypeParamName, "some-upstream-type"), + }, + wantLoginFlow: idpdiscoveryv1alpha1.IDPFlowCLIPassword, + }, + { + name: "when the Supervisor does not support the given IDP type, return a specific error message", + options: []Option{ + WithUpstreamIdentityProvider("INVALID-upstream-name", "some-upstream-type"), + withIDPDiscovery(someIDPDiscoveryResponse), + }, + wantErr: `unable to login to IDP with name "INVALID-upstream-name" and type "some-upstream-type"`, + }, + { + name: "when the Supervisor lists pinniped_supported_identity_provider_types and the given upstreamType is not found, return a specific error", + options: []Option{ + WithUpstreamIdentityProvider("some-upstream-name", "NOT_A_DISCOVERED_TYPE"), + withIDPDiscovery(someIDPDiscoveryResponse), + }, + wantErr: `unable to login to IDP with type "NOT_A_DISCOVERED_TYPE", this Pinniped Supervisor supports IDP types ["idp-type-with-cli-password-only-flow", "idp-type-with-no-flows", "other-supported-type-with-no-idp", "some-upstream-type"]`, + }, + { + name: "when the Supervisor does not list pinniped_supported_identity_provider_types (legacy behavior) and the given upstreamType is not found, return a generic error", + options: []Option{ + WithUpstreamIdentityProvider("some-upstream-name", "NOT_A_DISCOVERED_TYPE"), + withIDPDiscovery(func() idpdiscoveryv1alpha1.IDPDiscoveryResponse { + temp := someIDPDiscoveryResponse + temp.PinnipedSupportedIDPTypes = nil + return temp + }()), + }, + wantErr: `unable to login to IDP with name "some-upstream-name" and type "NOT_A_DISCOVERED_TYPE"`, + }, + { + name: "when the Supervisor does not have an IDP that matches by type, return an error", + options: []Option{ + WithUpstreamIdentityProvider("some-upstream-name", "other-supported-type-with-no-idp"), + withIDPDiscovery(someIDPDiscoveryResponse), + }, + wantErr: `unable to login to IDP with name "some-upstream-name" and type "other-supported-type-with-no-idp"`, + }, + { + name: "when the Supervisor does not have an IDP that matches by flow, return an error", + options: []Option{ + WithUpstreamIdentityProvider("idp-name-with-no-flows", "idp-type-with-no-flows"), + WithLoginFlow(idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode, "flowSource"), + withIDPDiscovery(someIDPDiscoveryResponse), + }, + wantErr: `unable to login to IDP with name "idp-name-with-no-flows" and type "idp-type-with-no-flows" and flow "browser_authcode"`, + }, + { + name: "when Supervisor does not have IDP discovery, return error", + options: []Option{ + WithUpstreamIdentityProvider("some-upstream-name", "some-upstream-type"), + }, + wantErr: "this Pinniped Supervisor does not have IDP discovery, please contact the system administrator to request an upgrade to the Pinniped Supervisor", + }, + } { + tt := tt + t.Run(tt.name, func(t *testing.T) { + var h handlerState + + for _, option := range tt.options { + require.NoError(t, option(&h)) + } + + actualAuthCodeOptions, actualError := h.maybePerformPinnipedSupervisorValidations() + + if tt.wantErr != "" { + require.EqualError(t, actualError, tt.wantErr) + return + } + + require.NoError(t, actualError) + require.Equal(t, tt.wantAuthCodeOptions, actualAuthCodeOptions) + require.Equal(t, tt.wantLoginFlow, h.loginFlow) + }) + } +}