From 67dca688d748c62397e0c366a0b757179aa3cb1e Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Thu, 13 May 2021 10:05:56 -0700 Subject: [PATCH] Add an API version to the Supervisor IDP discovery endpoint Also rename one of the new functional opts in login.go to more accurately reflect the intention of the opt. --- cmd/pinniped/cmd/kubeconfig.go | 14 +- cmd/pinniped/cmd/kubeconfig_test.go | 153 ++++++++++++------ cmd/pinniped/cmd/login_oidc.go | 2 +- internal/oidc/discovery/discovery_handler.go | 8 +- .../oidc/discovery/discovery_handler_test.go | 12 +- internal/oidc/oidc.go | 2 +- internal/oidc/provider/manager/manager.go | 2 +- .../oidc/provider/manager/manager_test.go | 4 +- pkg/oidcclient/login.go | 19 +-- pkg/oidcclient/login_test.go | 4 +- 10 files changed, 147 insertions(+), 73 deletions(-) diff --git a/cmd/pinniped/cmd/kubeconfig.go b/cmd/pinniped/cmd/kubeconfig.go index 832b1365..88f2b86d 100644 --- a/cmd/pinniped/cmd/kubeconfig.go +++ b/cmd/pinniped/cmd/kubeconfig.go @@ -95,11 +95,15 @@ type getKubeconfigParams struct { credentialCachePathSet bool } -type supervisorOIDCDiscoveryResponse struct { +type supervisorOIDCDiscoveryResponseWithV1Alpha1 struct { + SupervisorDiscovery SupervisorDiscoveryResponseV1Alpha1 `json:"discovery.supervisor.pinniped.dev/v1alpha1"` +} + +type SupervisorDiscoveryResponseV1Alpha1 struct { PinnipedIDPsEndpoint string `json:"pinniped_identity_providers_endpoint"` } -type supervisorIDPsDiscoveryResponse struct { +type supervisorIDPsDiscoveryResponseV1Alpha1 struct { PinnipedIDPs []pinnipedIDPResponse `json:"pinniped_identity_providers"` } @@ -800,13 +804,13 @@ func discoverIDPsDiscoveryEndpointURL(ctx context.Context, issuer string, httpCl return "", fmt.Errorf("unable to fetch OIDC discovery data from issuer: could not read response body: %w", err) } - var body supervisorOIDCDiscoveryResponse + var body supervisorOIDCDiscoveryResponseWithV1Alpha1 err = json.Unmarshal(rawBody, &body) if err != nil { return "", fmt.Errorf("unable to fetch OIDC discovery data from issuer: could not parse response JSON: %w", err) } - return body.PinnipedIDPsEndpoint, nil + return body.SupervisorDiscovery.PinnipedIDPsEndpoint, nil } func discoverAllAvailableSupervisorUpstreamIDPs(ctx context.Context, pinnipedIDPsEndpoint string, httpClient *http.Client) ([]pinnipedIDPResponse, error) { @@ -831,7 +835,7 @@ func discoverAllAvailableSupervisorUpstreamIDPs(ctx context.Context, pinnipedIDP return nil, fmt.Errorf("unable to fetch IDP discovery data from issuer: could not read response body: %w", err) } - var body supervisorIDPsDiscoveryResponse + var body supervisorIDPsDiscoveryResponseV1Alpha1 err = json.Unmarshal(rawBody, &body) if err != nil { return nil, fmt.Errorf("unable to fetch IDP discovery data from issuer: could not parse response JSON: %w", err) diff --git a/cmd/pinniped/cmd/kubeconfig_test.go b/cmd/pinniped/cmd/kubeconfig_test.go index c2223350..8fd00f66 100644 --- a/cmd/pinniped/cmd/kubeconfig_test.go +++ b/cmd/pinniped/cmd/kubeconfig_test.go @@ -74,6 +74,16 @@ func TestGetKubeconfig(t *testing.T) { } } + happyOIDCDIscoveryResponse := func(issuerURL string) string { + return here.Docf(`{ + "other-key": "other-value", + "discovery.supervisor.pinniped.dev/v1alpha1": { + "pinniped_identity_providers_endpoint": "%s/v1alpha1/pinniped_identity_providers" + }, + "another-key": "another-value" + }`, issuerURL) + } + tests := []struct { name string args func(string, string) []string @@ -737,9 +747,7 @@ func TestGetKubeconfig(t *testing.T) { jwtAuthenticator(issuerCABundle, issuerURL), } }, - oidcDiscoveryResponse: func(issuerURL string) string { - return fmt.Sprintf(`{"pinniped_identity_providers_endpoint": "%s/pinniped_identity_providers"}`, issuerURL) - }, + oidcDiscoveryResponse: happyOIDCDIscoveryResponse, idpsDiscoveryStatusCode: http.StatusBadRequest, wantLogs: func(issuerCABundle string, issuerURL string) []string { return []string{ @@ -772,9 +780,7 @@ func TestGetKubeconfig(t *testing.T) { jwtAuthenticator(issuerCABundle, issuerURL), } }, - oidcDiscoveryResponse: func(issuerURL string) string { - return fmt.Sprintf(`{"pinniped_identity_providers_endpoint": "%s/pinniped_identity_providers"}`, issuerURL) - }, + oidcDiscoveryResponse: happyOIDCDIscoveryResponse, idpsDiscoveryResponse: here.Docf(`{ "pinniped_identity_providers": [ {"name": "some-ldap-idp", "type": "ldap"}, @@ -848,9 +854,7 @@ func TestGetKubeconfig(t *testing.T) { jwtAuthenticator(issuerCABundle, issuerURL), } }, - oidcDiscoveryResponse: func(issuerURL string) string { - return fmt.Sprintf(`{"pinniped_identity_providers_endpoint": "%s/pinniped_identity_providers"}`, issuerURL) - }, + oidcDiscoveryResponse: happyOIDCDIscoveryResponse, idpsDiscoveryResponse: "this is not valid JSON", wantLogs: func(issuerCABundle string, issuerURL string) []string { return []string{ @@ -958,7 +962,11 @@ func TestGetKubeconfig(t *testing.T) { } }, oidcDiscoveryResponse: func(issuerURL string) string { - return `{"pinniped_identity_providers_endpoint": "https%://illegal_url"}` + return here.Doc(`{ + "discovery.supervisor.pinniped.dev/v1alpha1": { + "pinniped_identity_providers_endpoint": "https%://illegal_url" + } + }`) }, wantLogs: func(issuerCABundle string, issuerURL string) []string { return []string{ @@ -990,9 +998,7 @@ func TestGetKubeconfig(t *testing.T) { "--upstream-identity-provider-type", "ldap", } }, - oidcDiscoveryResponse: func(issuerURL string) string { - return fmt.Sprintf(`{"pinniped_identity_providers_endpoint": "%s/pinniped_identity_providers"}`, issuerURL) - }, + oidcDiscoveryResponse: happyOIDCDIscoveryResponse, idpsDiscoveryResponse: here.Docf(`{ "pinniped_identity_providers": [ {"name": "some-ldap-idp", "type": "ldap"}, @@ -1021,9 +1027,7 @@ func TestGetKubeconfig(t *testing.T) { "--upstream-identity-provider-name", "my-idp", } }, - oidcDiscoveryResponse: func(issuerURL string) string { - return fmt.Sprintf(`{"pinniped_identity_providers_endpoint": "%s/pinniped_identity_providers"}`, issuerURL) - }, + oidcDiscoveryResponse: happyOIDCDIscoveryResponse, idpsDiscoveryResponse: here.Docf(`{ "pinniped_identity_providers": [ {"name": "my-idp", "type": "ldap"}, @@ -1051,9 +1055,7 @@ func TestGetKubeconfig(t *testing.T) { "--upstream-identity-provider-type", "ldap", } }, - oidcDiscoveryResponse: func(issuerURL string) string { - return fmt.Sprintf(`{"pinniped_identity_providers_endpoint": "%s/pinniped_identity_providers"}`, issuerURL) - }, + oidcDiscoveryResponse: happyOIDCDIscoveryResponse, idpsDiscoveryResponse: here.Docf(`{ "pinniped_identity_providers": [ {"name": "some-oidc-idp", "type": "oidc"}, @@ -1079,9 +1081,7 @@ func TestGetKubeconfig(t *testing.T) { "--upstream-identity-provider-name", "my-nonexistent-idp", } }, - oidcDiscoveryResponse: func(issuerURL string) string { - return fmt.Sprintf(`{"pinniped_identity_providers_endpoint": "%s/pinniped_identity_providers"}`, issuerURL) - }, + oidcDiscoveryResponse: happyOIDCDIscoveryResponse, idpsDiscoveryResponse: here.Docf(`{ "pinniped_identity_providers": [ {"name": "some-oidc-idp", "type": "oidc"}, @@ -1598,9 +1598,7 @@ func TestGetKubeconfig(t *testing.T) { jwtAuthenticator(issuerCABundle, issuerURL), } }, - oidcDiscoveryResponse: func(issuerURL string) string { - return fmt.Sprintf(`{"pinniped_identity_providers_endpoint": "%s/pinniped_identity_providers"}`, issuerURL) - }, + oidcDiscoveryResponse: happyOIDCDIscoveryResponse, idpsDiscoveryResponse: here.Docf(`{ "pinniped_identity_providers": [ {"name": "some-ldap-idp", "type": "ldap"} @@ -1677,9 +1675,7 @@ func TestGetKubeconfig(t *testing.T) { jwtAuthenticator(issuerCABundle, issuerURL), } }, - oidcDiscoveryResponse: func(issuerURL string) string { - return fmt.Sprintf(`{"pinniped_identity_providers_endpoint": "%s/pinniped_identity_providers"}`, issuerURL) - }, + oidcDiscoveryResponse: happyOIDCDIscoveryResponse, idpsDiscoveryResponse: here.Docf(`{ "pinniped_identity_providers": [ {"name": "some-oidc-idp", "type": "oidc"} @@ -1756,9 +1752,7 @@ func TestGetKubeconfig(t *testing.T) { jwtAuthenticator(issuerCABundle, issuerURL), } }, - oidcDiscoveryResponse: func(issuerURL string) string { - return fmt.Sprintf(`{"pinniped_identity_providers_endpoint": "%s/pinniped_identity_providers"}`, issuerURL) - }, + oidcDiscoveryResponse: happyOIDCDIscoveryResponse, idpsDiscoveryResponse: here.Docf(`{ "pinniped_identity_providers": [] }`), @@ -1818,7 +1812,7 @@ func TestGetKubeconfig(t *testing.T) { }, }, { - name: "IDP discovery endpoint is not listed in OIDC discovery document", + name: "Supervisor discovery section is not listed in OIDC discovery document", args: func(issuerCABundle string, issuerURL string) []string { return []string{ "--kubeconfig", "./testdata/kubeconfig.yaml", @@ -1890,6 +1884,83 @@ func TestGetKubeconfig(t *testing.T) { base64.StdEncoding.EncodeToString([]byte(issuerCABundle))) }, }, + { + name: "IDP discovery endpoint is not listed in OIDC discovery document within the Supervisor discovery section", + args: func(issuerCABundle string, issuerURL string) []string { + return []string{ + "--kubeconfig", "./testdata/kubeconfig.yaml", + "--skip-validation", + } + }, + conciergeObjects: func(issuerCABundle string, issuerURL string) []runtime.Object { + return []runtime.Object{ + credentialIssuer(), + jwtAuthenticator(issuerCABundle, issuerURL), + } + }, + oidcDiscoveryResponse: func(issuerURL string) string { + return here.Doc(`{ + "discovery.supervisor.pinniped.dev/v1alpha1": { + "wrong-key": "some-value" + } + }`) + }, + idpsDiscoveryStatusCode: http.StatusBadRequest, // IDPs endpoint shouldn't be called by this test + wantLogs: func(issuerCABundle string, issuerURL string) []string { + return []string{ + `"level"=0 "msg"="discovered CredentialIssuer" "name"="test-credential-issuer"`, + `"level"=0 "msg"="discovered Concierge operating in TokenCredentialRequest API mode"`, + `"level"=0 "msg"="discovered Concierge endpoint" "endpoint"="https://fake-server-url-value"`, + `"level"=0 "msg"="discovered Concierge certificate authority bundle" "roots"=0`, + `"level"=0 "msg"="discovered JWTAuthenticator" "name"="test-authenticator"`, + fmt.Sprintf(`"level"=0 "msg"="discovered OIDC issuer" "issuer"="%s"`, issuerURL), + `"level"=0 "msg"="discovered OIDC audience" "audience"="test-audience"`, + `"level"=0 "msg"="discovered OIDC CA bundle" "roots"=1`, + } + }, + wantStdout: func(issuerCABundle string, issuerURL string) string { + return here.Docf(` + apiVersion: v1 + clusters: + - cluster: + certificate-authority-data: ZmFrZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YS12YWx1ZQ== + server: https://fake-server-url-value + name: kind-cluster-pinniped + contexts: + - context: + cluster: kind-cluster-pinniped + user: kind-user-pinniped + name: kind-context-pinniped + current-context: kind-context-pinniped + kind: Config + preferences: {} + users: + - name: kind-user-pinniped + user: + exec: + apiVersion: client.authentication.k8s.io/v1beta1 + args: + - login + - oidc + - --enable-concierge + - --concierge-api-group-suffix=pinniped.dev + - --concierge-authenticator-name=test-authenticator + - --concierge-authenticator-type=jwt + - --concierge-endpoint=https://fake-server-url-value + - --concierge-ca-bundle-data=ZmFrZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YS12YWx1ZQ== + - --issuer=%s + - --client-id=pinniped-cli + - --scopes=offline_access,openid,pinniped:request-audience + - --ca-bundle-data=%s + - --request-audience=test-audience + command: '.../path/to/pinniped' + env: [] + provideClusterInfo: true + `, + issuerURL, + base64.StdEncoding.EncodeToString([]byte(issuerCABundle))) + }, + }, { name: "when OIDC discovery document 404s, dont set idp related flags", args: func(issuerCABundle string, issuerURL string) []string { @@ -2050,9 +2121,7 @@ func TestGetKubeconfig(t *testing.T) { jwtAuthenticator(issuerCABundle, issuerURL), } }, - oidcDiscoveryResponse: func(issuerURL string) string { - return fmt.Sprintf(`{"pinniped_identity_providers_endpoint": "%s/pinniped_identity_providers"}`, issuerURL) - }, + oidcDiscoveryResponse: happyOIDCDIscoveryResponse, idpsDiscoveryResponse: here.Docf(`{ "pinniped_identity_providers": [ {"name": "some-other-ldap-idp", "type": "ldap"} @@ -2127,9 +2196,7 @@ func TestGetKubeconfig(t *testing.T) { "--oidc-ca-bundle", f.Name(), } }, - oidcDiscoveryResponse: func(issuerURL string) string { - return fmt.Sprintf(`{"pinniped_identity_providers_endpoint": "%s/pinniped_identity_providers"}`, issuerURL) - }, + oidcDiscoveryResponse: happyOIDCDIscoveryResponse, idpsDiscoveryResponse: here.Docf(`{ "pinniped_identity_providers": [ {"name": "some-ldap-idp", "type": "ldap"} @@ -2186,9 +2253,7 @@ func TestGetKubeconfig(t *testing.T) { "--upstream-identity-provider-type", "ldap", } }, - oidcDiscoveryResponse: func(issuerURL string) string { - return fmt.Sprintf(`{"pinniped_identity_providers_endpoint": "%s/pinniped_identity_providers"}`, issuerURL) - }, + oidcDiscoveryResponse: happyOIDCDIscoveryResponse, idpsDiscoveryResponse: here.Docf(`{ "pinniped_identity_providers": [ {"name": "some-ldap-idp", "type": "ldap"}, @@ -2247,9 +2312,7 @@ func TestGetKubeconfig(t *testing.T) { "--upstream-identity-provider-name", "some-ldap-idp", } }, - oidcDiscoveryResponse: func(issuerURL string) string { - return fmt.Sprintf(`{"pinniped_identity_providers_endpoint": "%s/pinniped_identity_providers"}`, issuerURL) - }, + oidcDiscoveryResponse: happyOIDCDIscoveryResponse, idpsDiscoveryResponse: here.Docf(`{ "pinniped_identity_providers": [ {"name": "some-ldap-idp", "type": "ldap"}, @@ -2313,7 +2376,7 @@ func TestGetKubeconfig(t *testing.T) { w.WriteHeader(tt.oidcDiscoveryStatusCode) _, err = w.Write([]byte(jsonResponseBody)) require.NoError(t, err) - case "/pinniped_identity_providers": + case "/v1alpha1/pinniped_identity_providers": jsonResponseBody := tt.idpsDiscoveryResponse if tt.idpsDiscoveryResponse == "" { jsonResponseBody = "{}" diff --git a/cmd/pinniped/cmd/login_oidc.go b/cmd/pinniped/cmd/login_oidc.go index 8674573f..83542c01 100644 --- a/cmd/pinniped/cmd/login_oidc.go +++ b/cmd/pinniped/cmd/login_oidc.go @@ -160,7 +160,7 @@ func runOIDCLogin(cmd *cobra.Command, deps oidcLoginCommandDeps, flags oidcLogin case "oidc": // this is the default, so don't need to do anything case "ldap": - opts = append(opts, oidcclient.WithLDAPUpstreamIdentityProvider()) + opts = append(opts, oidcclient.WithCLISendingCredentials()) default: // Surprisingly cobra does not support this kind of flag validation. See https://github.com/spf13/pflag/issues/236 return fmt.Errorf( diff --git a/internal/oidc/discovery/discovery_handler.go b/internal/oidc/discovery/discovery_handler.go index 542ef729..e472c012 100644 --- a/internal/oidc/discovery/discovery_handler.go +++ b/internal/oidc/discovery/discovery_handler.go @@ -40,11 +40,15 @@ type Metadata struct { // vvv Custom vvv - PinnipedIDPsEndpoint string `json:"pinniped_identity_providers_endpoint"` + SupervisorDiscovery SupervisorDiscoveryMetadataV1Alpha1 `json:"discovery.supervisor.pinniped.dev/v1alpha1"` // ^^^ Custom ^^^ } +type SupervisorDiscoveryMetadataV1Alpha1 struct { + PinnipedIDPsEndpoint string `json:"pinniped_identity_providers_endpoint"` +} + type IdentityProviderMetadata struct { Name string `json:"name"` Type string `json:"type"` @@ -57,7 +61,7 @@ func NewHandler(issuerURL string) http.Handler { AuthorizationEndpoint: issuerURL + oidc.AuthorizationEndpointPath, TokenEndpoint: issuerURL + oidc.TokenEndpointPath, JWKSURI: issuerURL + oidc.JWKSEndpointPath, - PinnipedIDPsEndpoint: issuerURL + oidc.PinnipedIDPsPath, + SupervisorDiscovery: SupervisorDiscoveryMetadataV1Alpha1{PinnipedIDPsEndpoint: issuerURL + oidc.PinnipedIDPsPathV1Alpha1}, ResponseTypesSupported: []string{"code"}, SubjectTypesSupported: []string{"public"}, IDTokenSigningAlgValuesSupported: []string{"ES256"}, diff --git a/internal/oidc/discovery/discovery_handler_test.go b/internal/oidc/discovery/discovery_handler_test.go index fd37341b..b3c70b35 100644 --- a/internal/oidc/discovery/discovery_handler_test.go +++ b/internal/oidc/discovery/discovery_handler_test.go @@ -35,11 +35,13 @@ func TestDiscovery(t *testing.T) { wantStatus: http.StatusOK, wantContentType: "application/json", wantBodyJSON: &Metadata{ - Issuer: "https://some-issuer.com/some/path", - AuthorizationEndpoint: "https://some-issuer.com/some/path/oauth2/authorize", - TokenEndpoint: "https://some-issuer.com/some/path/oauth2/token", - JWKSURI: "https://some-issuer.com/some/path/jwks.json", - PinnipedIDPsEndpoint: "https://some-issuer.com/some/path/pinniped_identity_providers", + Issuer: "https://some-issuer.com/some/path", + AuthorizationEndpoint: "https://some-issuer.com/some/path/oauth2/authorize", + TokenEndpoint: "https://some-issuer.com/some/path/oauth2/token", + JWKSURI: "https://some-issuer.com/some/path/jwks.json", + SupervisorDiscovery: SupervisorDiscoveryMetadataV1Alpha1{ + PinnipedIDPsEndpoint: "https://some-issuer.com/some/path/v1alpha1/pinniped_identity_providers", + }, ResponseTypesSupported: []string{"code"}, SubjectTypesSupported: []string{"public"}, IDTokenSigningAlgValuesSupported: []string{"ES256"}, diff --git a/internal/oidc/oidc.go b/internal/oidc/oidc.go index 3be60dcb..78319329 100644 --- a/internal/oidc/oidc.go +++ b/internal/oidc/oidc.go @@ -24,7 +24,7 @@ const ( TokenEndpointPath = "/oauth2/token" //nolint:gosec // ignore lint warning that this is a credential CallbackEndpointPath = "/callback" JWKSEndpointPath = "/jwks.json" - PinnipedIDPsPath = "/pinniped_identity_providers" + PinnipedIDPsPathV1Alpha1 = "/v1alpha1/pinniped_identity_providers" ) const ( diff --git a/internal/oidc/provider/manager/manager.go b/internal/oidc/provider/manager/manager.go index 1d41e4ef..ea1d2d62 100644 --- a/internal/oidc/provider/manager/manager.go +++ b/internal/oidc/provider/manager/manager.go @@ -107,7 +107,7 @@ func (m *Manager) SetProviders(federationDomains ...*provider.FederationDomainIs m.providerHandlers[(issuerHostWithPath + oidc.JWKSEndpointPath)] = jwks.NewHandler(issuer, m.dynamicJWKSProvider) - m.providerHandlers[(issuerHostWithPath + oidc.PinnipedIDPsPath)] = idpdiscovery.NewHandler(m.upstreamIDPs) + m.providerHandlers[(issuerHostWithPath + oidc.PinnipedIDPsPathV1Alpha1)] = idpdiscovery.NewHandler(m.upstreamIDPs) m.providerHandlers[(issuerHostWithPath + oidc.AuthorizationEndpointPath)] = auth.NewHandler( issuer, diff --git a/internal/oidc/provider/manager/manager_test.go b/internal/oidc/provider/manager/manager_test.go index c99fadff..469a085d 100644 --- a/internal/oidc/provider/manager/manager_test.go +++ b/internal/oidc/provider/manager/manager_test.go @@ -86,13 +86,13 @@ func TestManager(t *testing.T) { err = json.Unmarshal(responseBody, &parsedDiscoveryResult) r.NoError(err) r.Equal(expectedIssuer, parsedDiscoveryResult.Issuer) - r.Equal(parsedDiscoveryResult.PinnipedIDPsEndpoint, expectedIssuer+oidc.PinnipedIDPsPath) + r.Equal(parsedDiscoveryResult.SupervisorDiscovery.PinnipedIDPsEndpoint, expectedIssuer+oidc.PinnipedIDPsPathV1Alpha1) } requirePinnipedIDPsDiscoveryRequestToBeHandled := func(requestIssuer, requestURLSuffix, expectedIDPName, expectedIDPType string) { recorder := httptest.NewRecorder() - subject.ServeHTTP(recorder, newGetRequest(requestIssuer+oidc.PinnipedIDPsPath+requestURLSuffix)) + subject.ServeHTTP(recorder, newGetRequest(requestIssuer+oidc.PinnipedIDPsPathV1Alpha1+requestURLSuffix)) r.False(fallbackHandlerWasCalled) diff --git a/pkg/oidcclient/login.go b/pkg/oidcclient/login.go index e2d0e2bf..40739eb8 100644 --- a/pkg/oidcclient/login.go +++ b/pkg/oidcclient/login.go @@ -74,7 +74,7 @@ type handlerState struct { upstreamIdentityProviderName string upstreamIdentityProviderType string - ldapUpstreamIdentityProvider bool + cliToSendCredentials bool requestedAudience string @@ -200,14 +200,15 @@ func WithRequestAudience(audience string) Option { } } -// WithLDAPUpstreamIdentityProvider causes the login flow to use CLI prompts for username and password and causes the +// WithCLISendingCredentials causes the login flow to use CLI-based prompts for username and password and causes the // call to the Issuer's authorize endpoint to be made directly (no web browser) with the username and password on custom // HTTP headers. This is only intended to be used when the issuer is a Pinniped Supervisor and the upstream identity -// provider is an LDAP provider. It should never be used with non-Supervisor issuers because it will send the user's -// password as a custom header, which would be ignored but could potentially get logged somewhere by the issuer. -func WithLDAPUpstreamIdentityProvider() Option { +// provider type supports this style of authentication. Currently this is supported by LDAPIdentityProviders. +// This should never be used with non-Supervisor issuers because it will send the user's password to the authorization +// endpoint as a custom header, which would be ignored but could potentially get logged somewhere by the issuer. +func WithCLISendingCredentials() Option { return func(h *handlerState) error { - h.ldapUpstreamIdentityProvider = true + h.cliToSendCredentials = true return nil } } @@ -356,7 +357,7 @@ func (h *handlerState) baseLogin() (*oidctypes.Token, error) { // Choose the appropriate authorization and authcode exchange strategy. var authFunc = h.webBrowserBasedAuth - if h.ldapUpstreamIdentityProvider { + if h.cliToSendCredentials { authFunc = h.cliBasedAuth } @@ -371,8 +372,8 @@ func (h *handlerState) baseLogin() (*oidctypes.Token, error) { return token, err } -// Make a direct call to the authorize endpoint and parse the authcode from the response. -// Exchange the authcode for tokens. Return the tokens or an error. +// Make a direct call to the authorize endpoint, including the user's username and password on custom http headers, +// and parse the authcode from the response. Exchange the authcode for tokens. Return the tokens or an error. func (h *handlerState) cliBasedAuth(authorizeOptions *[]oauth2.AuthCodeOption) (*oidctypes.Token, error) { // Ask the user for their username and password. username, err := h.promptForValue(defaultLDAPUsernamePrompt) diff --git a/pkg/oidcclient/login_test.go b/pkg/oidcclient/login_test.go index 4cc23f93..d85c01ac 100644 --- a/pkg/oidcclient/login_test.go +++ b/pkg/oidcclient/login_test.go @@ -232,7 +232,7 @@ func TestLogin(t *testing.T) { // nolint:gocyclo require.Equal(t, []SessionCacheKey{cacheKey}, cache.sawGetKeys) }) require.NoError(t, WithSessionCache(cache)(h)) - require.NoError(t, WithLDAPUpstreamIdentityProvider()(h)) + require.NoError(t, WithCLISendingCredentials()(h)) require.NoError(t, WithUpstreamIdentityProvider("some-upstream-name", "ldap")(h)) require.NoError(t, WithClient(&http.Client{ @@ -875,7 +875,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, WithLDAPUpstreamIdentityProvider()(h)) + require.NoError(t, WithCLISendingCredentials()(h)) require.NoError(t, WithUpstreamIdentityProvider("some-upstream-name", "ldap")(h)) discoveryRequestWasMade := false