From 1d7f77c6d5c62d4b56e8621651e3ca97b707b234 Mon Sep 17 00:00:00 2001 From: Joshua Casey Date: Thu, 2 May 2024 21:05:43 -0500 Subject: [PATCH] Additional WIP that does resolve some TODOs but adds more and needs a lot more testing --- cmd/pinniped/cmd/login_oidc.go | 112 +----- cmd/pinniped/cmd/login_oidc_test.go | 313 ++------------- cmd/pinniped/cmd/oidc_client_options.go | 9 +- .../mockoidcclientoptions.go | 8 +- pkg/oidcclient/login.go | 124 +++++- pkg/oidcclient/login_test.go | 359 ++++++++++++++++-- 6 files changed, 498 insertions(+), 427 deletions(-) diff --git a/cmd/pinniped/cmd/login_oidc.go b/cmd/pinniped/cmd/login_oidc.go index a90dca5e7..192684b9c 100644 --- a/cmd/pinniped/cmd/login_oidc.go +++ b/cmd/pinniped/cmd/login_oidc.go @@ -12,7 +12,6 @@ import ( "net/http" "os" "path/filepath" - "strings" "time" "github.com/spf13/cobra" @@ -201,43 +200,13 @@ func runOIDCLogin(cmd *cobra.Command, deps oidcLoginCommandDeps, flags oidcLogin flags.upstreamIdentityProviderName, flags.upstreamIdentityProviderType)) } - // TODO This is the caller, but note that OIDC discovery has not happened yet at this point. :( - // It hasn't even constructed the http client for it yet. Nor has it checked to see if - // it already has a cached cluster-specific credential that it can return without ever talking to a server. - // This is too early to discover if the requested IDP type is supported by the Supervisor, and also - // too early to discover if that IDP type supports the requested flow or not. - // So this code could be replaced by code which simply passes through (as a new Option) the value of the - // flow type flag, possibly overridden by the value of the env var, or possibly nothing when the flow type - // flag and env var are both blank. - flowOpts, err := flowOptions( - idpdiscoveryv1alpha1.IDPType(flags.upstreamIdentityProviderType), - idpdiscoveryv1alpha1.IDPFlow(flags.upstreamIdentityProviderFlow), - deps, - ) - if err != nil { - return err + requestedFlow, flowSource := idpdiscoveryv1alpha1.IDPFlow(flags.upstreamIdentityProviderFlow), "--upstream-identity-provider-flow" + if flowOverride, hasFlowOverride := deps.lookupEnv(upstreamIdentityProviderFlowEnvVarName); hasFlowOverride { + requestedFlow, flowSource = idpdiscoveryv1alpha1.IDPFlow(flowOverride), upstreamIdentityProviderFlowEnvVarName + } + if requestedFlow != "" { + opts = append(opts, deps.optionsFactory.WithLoginFlow(requestedFlow, flowSource)) } - opts = append(opts, flowOpts...) - - // TODO: something like this - //requestedFlow := idpdiscoveryv1alpha1.IDPFlow(flags.upstreamIdentityProviderFlow) - //flowOverride, hasFlowOverride := deps.lookupEnv(upstreamIdentityProviderFlowEnvVarName) - //if hasFlowOverride { - // requestedFlow = idpdiscoveryv1alpha1.IDPFlow(flowOverride) - //} - //if requestedFlow != "" { - // switch requestedFlow { - // case idpdiscoveryv1alpha1.IDPFlowCLIPassword, - // idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode: - // default: - // return fmt.Errorf( - // "WithLoginFlow parameter must be %s or %s", - // idpdiscoveryv1alpha1.IDPFlowCLIPassword, - // idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode, - // ) - // } - // opts = append(opts, deps.optionsFactory.WithLoginFlow(requestedFlow)) - //} var concierge *conciergeclient.Client if flags.conciergeEnabled { @@ -327,75 +296,6 @@ func runOIDCLogin(cmd *cobra.Command, deps oidcLoginCommandDeps, flags oidcLogin return json.NewEncoder(cmd.OutOrStdout()).Encode(cred) } -// flowOptions takes in the requested IDP type, the requested flow type (which can be blank), and the env var which -// can override the requested flow type. -// It determines whether the CLI should prompt for the user's username/password (known as the "CLI flow"), which is -// returned as an oidcclient.Option. Returning nil means that the CLI should not prompt for username/password. -// It also tries to make nice error messages when the requested IDP type or requested flow values are not understood. -func flowOptions( - requestedIDPType idpdiscoveryv1alpha1.IDPType, - requestedFlow idpdiscoveryv1alpha1.IDPFlow, - deps oidcLoginCommandDeps, -) ([]oidcclient.Option, error) { - useCLIFlow := []oidcclient.Option{oidcclient.WithCLISendingCredentials()} - - // If the env var is set to override the --upstream-identity-provider-type flag, then override it. - flowOverride, hasFlowOverride := deps.lookupEnv(upstreamIdentityProviderFlowEnvVarName) - flowSource := "--upstream-identity-provider-flow" - if hasFlowOverride { - requestedFlow = idpdiscoveryv1alpha1.IDPFlow(flowOverride) - flowSource = upstreamIdentityProviderFlowEnvVarName - } - - // TODO This switch statement means that we have to change the CLI every time we add a new IDP type to the Supervisor server. :( - // If we added a new flow type, then CLI changes would be expected. But adding a new IDP type should ideally - // not require any CLI functionality changes (except maybe some command-line flag help text messages). - // Rather than hardcode knowledge of which IDP types are supported, and which flow types are supported by each IDP, - // and which is the default flow type for each IDP type, all that knowledge should come from a discovery request. - // Default flow type for each IDP type can be determined by the ordering of the flow types in the discovery response. - // (See https://github.com/vmware-tanzu/pinniped/blob/v0.29.0/internal/federationdomain/endpoints/idpdiscovery/idp_discovery_handler_test.go#L56-L57.) - // Keeping the nice error messages would only be possible if the CLI used discovery to check what the Supervisor supports. - // We would need to be careful to also support old Supervisors that don't advertise which IDP types are supported - // by retaining the old CLI behavior for that case (assume that the old Supervisor supports only OIDC, LDAP, and AD). - switch requestedIDPType { - case idpdiscoveryv1alpha1.IDPTypeOIDC: - switch requestedFlow { - case idpdiscoveryv1alpha1.IDPFlowCLIPassword: - return useCLIFlow, nil - case idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode, "": - return nil, nil // browser authcode flow is the default Option, so don't need to return an Option here - default: - return nil, fmt.Errorf( - "%s value not recognized for identity provider type %q: %s (supported values: %s)", - flowSource, requestedIDPType, requestedFlow, - strings.Join([]string{idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode.String(), idpdiscoveryv1alpha1.IDPFlowCLIPassword.String()}, ", ")) - } - case idpdiscoveryv1alpha1.IDPTypeLDAP, idpdiscoveryv1alpha1.IDPTypeActiveDirectory: - switch requestedFlow { - case idpdiscoveryv1alpha1.IDPFlowCLIPassword, "": - return useCLIFlow, nil - case idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode: - return nil, nil // browser authcode flow is the default Option, so don't need to return an Option here - default: - return nil, fmt.Errorf( - "%s value not recognized for identity provider type %q: %s (supported values: %s)", - flowSource, requestedIDPType, requestedFlow, - strings.Join([]string{idpdiscoveryv1alpha1.IDPFlowCLIPassword.String(), idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode.String()}, ", ")) - } - default: - // Surprisingly cobra does not support this kind of flag validation. See https://github.com/spf13/pflag/issues/236 - return nil, fmt.Errorf( - "--upstream-identity-provider-type value not recognized: %s (supported values: %s)", - requestedIDPType, - strings.Join([]string{ - idpdiscoveryv1alpha1.IDPTypeOIDC.String(), - idpdiscoveryv1alpha1.IDPTypeLDAP.String(), - idpdiscoveryv1alpha1.IDPTypeActiveDirectory.String(), - }, ", "), - ) - } -} - func makeClient(caBundlePaths []string, caBundleData []string) (*http.Client, error) { pool := x509.NewCertPool() for _, p := range caBundlePaths { diff --git a/cmd/pinniped/cmd/login_oidc_test.go b/cmd/pinniped/cmd/login_oidc_test.go index b90469f35..9cbc0b642 100644 --- a/cmd/pinniped/cmd/login_oidc_test.go +++ b/cmd/pinniped/cmd/login_oidc_test.go @@ -8,22 +8,22 @@ import ( "context" "encoding/base64" "fmt" - idpdiscoveryv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idpdiscovery/v1alpha1" - oidcapi "go.pinniped.dev/generated/latest/apis/supervisor/oidc" - "go.pinniped.dev/internal/mocks/mockoidcclientoptions" - "go.uber.org/mock/gomock" "os" "path/filepath" "testing" "time" "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clientauthv1beta1 "k8s.io/client-go/pkg/apis/clientauthentication/v1beta1" clocktesting "k8s.io/utils/clock/testing" + idpdiscoveryv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idpdiscovery/v1alpha1" + oidcapi "go.pinniped.dev/generated/latest/apis/supervisor/oidc" "go.pinniped.dev/internal/certauthority" "go.pinniped.dev/internal/here" + "go.pinniped.dev/internal/mocks/mockoidcclientoptions" "go.pinniped.dev/internal/plog" "go.pinniped.dev/internal/testutil" "go.pinniped.dev/pkg/conciergeclient" @@ -169,31 +169,6 @@ func TestLoginOIDCCommand(t *testing.T) { Error: invalid Concierge parameters: invalid API group suffix: a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*') `), }, - { - name: "invalid upstream type is an error", - args: []string{ - "--issuer", "test-issuer", - "--upstream-identity-provider-type", "invalid", - }, - wantOptions: defaultWantedOptions, - wantError: true, - wantStderr: here.Doc(` - Error: --upstream-identity-provider-type value not recognized: invalid (supported values: oidc, ldap, activedirectory) - `), - }, - { - name: "invalid upstream type when flow override env var is used is still an error", - args: []string{ - "--issuer", "test-issuer", - "--upstream-identity-provider-type", "invalid", - }, - env: map[string]string{"PINNIPED_UPSTREAM_IDENTITY_PROVIDER_FLOW": "browser_authcode"}, - wantOptions: defaultWantedOptions, - wantError: true, - wantStderr: here.Doc(` - Error: --upstream-identity-provider-type value not recognized: invalid (supported values: oidc, ldap, activedirectory) - `), - }, { name: "oidc upstream type with default flow is allowed", args: []string{ @@ -223,268 +198,36 @@ func TestLoginOIDCCommand(t *testing.T) { wantStdout: `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{"interactive":false},"status":{"expirationTimestamp":"3020-10-12T13:14:15Z","token":"test-id-token"}}` + "\n", }, { - name: "oidc upstream type with CLI flow is allowed", + name: "--upstream-identity-provider-flow adds an option", args: []string{ "--issuer", "test-issuer", "--client-id", "test-client-id", - "--upstream-identity-provider-type", "oidc", "--upstream-identity-provider-flow", "cli_password", "--credential-cache", "", // must specify --credential-cache or else the cache file on disk causes test pollution }, wantOptions: func(f *mockoidcclientoptions.MockOIDCClientOptions) { defaultWantedOptions(f) - f.EXPECT().WithLoginFlow(idpdiscoveryv1alpha1.IDPFlowCLIPassword) - }, - wantOptionsCount: 5, - wantStdout: `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{"interactive":false},"status":{"expirationTimestamp":"3020-10-12T13:14:15Z","token":"test-id-token"}}` + "\n", - }, - { - name: "oidc upstream type with browser flow is allowed", - args: []string{ - "--issuer", "test-issuer", - "--client-id", "test-client-id", - "--upstream-identity-provider-type", "oidc", - "--upstream-identity-provider-flow", "browser_authcode", - "--credential-cache", "", // must specify --credential-cache or else the cache file on disk causes test pollution - }, - wantOptionsCount: 4, - wantStdout: `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{"interactive":false},"status":{"expirationTimestamp":"3020-10-12T13:14:15Z","token":"test-id-token"}}` + "\n", - }, - { - name: "oidc upstream type with CLI flow in flow override env var is allowed", - args: []string{ - "--issuer", "test-issuer", - "--client-id", "test-client-id", - "--upstream-identity-provider-type", "oidc", - "--upstream-identity-provider-flow", "browser_authcode", - "--credential-cache", "", // must specify --credential-cache or else the cache file on disk causes test pollution - }, - env: map[string]string{"PINNIPED_UPSTREAM_IDENTITY_PROVIDER_FLOW": "cli_password"}, - wantOptionsCount: 5, - wantStdout: `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{"interactive":false},"status":{"expirationTimestamp":"3020-10-12T13:14:15Z","token":"test-id-token"}}` + "\n", - }, - { - name: "oidc upstream type with with browser flow in flow override env var is allowed", - args: []string{ - "--issuer", "test-issuer", - "--client-id", "test-client-id", - "--upstream-identity-provider-type", "oidc", - "--upstream-identity-provider-flow", "cli_password", - "--credential-cache", "", // must specify --credential-cache or else the cache file on disk causes test pollution - }, - env: map[string]string{"PINNIPED_UPSTREAM_IDENTITY_PROVIDER_FLOW": "browser_authcode"}, - wantOptionsCount: 4, - wantStdout: `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{"interactive":false},"status":{"expirationTimestamp":"3020-10-12T13:14:15Z","token":"test-id-token"}}` + "\n", - }, - { - name: "oidc upstream type with unsupported flow is an error", - args: []string{ - "--issuer", "test-issuer", - "--client-id", "test-client-id", - "--upstream-identity-provider-type", "oidc", - "--upstream-identity-provider-flow", "foobar", - "--credential-cache", "", // must specify --credential-cache or else the cache file on disk causes test pollution - }, - wantError: true, - wantStderr: here.Doc(` - Error: --upstream-identity-provider-flow value not recognized for identity provider type "oidc": foobar (supported values: browser_authcode, cli_password) - `), - }, - { - name: "oidc upstream type with unsupported flow in flow override env var is an error", - args: []string{ - "--issuer", "test-issuer", - "--client-id", "test-client-id", - "--upstream-identity-provider-type", "oidc", - "--upstream-identity-provider-flow", "browser_authcode", - "--credential-cache", "", // must specify --credential-cache or else the cache file on disk causes test pollution - }, - env: map[string]string{"PINNIPED_UPSTREAM_IDENTITY_PROVIDER_FLOW": "foo"}, - wantError: true, - wantStderr: here.Doc(` - Error: PINNIPED_UPSTREAM_IDENTITY_PROVIDER_FLOW value not recognized for identity provider type "oidc": foo (supported values: browser_authcode, cli_password) - `), - }, - { - name: "ldap upstream type with default flow is allowed", - args: []string{ - "--issuer", "test-issuer", - "--client-id", "test-client-id", - "--upstream-identity-provider-type", "ldap", - "--credential-cache", "", // must specify --credential-cache or else the cache file on disk causes test pollution + f.EXPECT().WithLoginFlow(idpdiscoveryv1alpha1.IDPFlowCLIPassword, "--upstream-identity-provider-flow") }, wantOptionsCount: 5, wantStdout: `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{"interactive":false},"status":{"expirationTimestamp":"3020-10-12T13:14:15Z","token":"test-id-token"}}` + "\n", }, { - name: "activedirectory upstream type with default flow is allowed", + name: "PINNIPED_UPSTREAM_IDENTITY_PROVIDER_FLOW adds an option that overrides --upstream-identity-provider-flow", args: []string{ "--issuer", "test-issuer", "--client-id", "test-client-id", - "--upstream-identity-provider-type", "activedirectory", + "--upstream-identity-provider-flow", "ignored-value-from-param", "--credential-cache", "", // must specify --credential-cache or else the cache file on disk causes test pollution }, - wantOptionsCount: 5, - wantStdout: `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{"interactive":false},"status":{"expirationTimestamp":"3020-10-12T13:14:15Z","token":"test-id-token"}}` + "\n", - }, - { - name: "ldap upstream type with CLI flow is allowed", - args: []string{ - "--issuer", "test-issuer", - "--client-id", "test-client-id", - "--upstream-identity-provider-type", "ldap", - "--upstream-identity-provider-flow", "cli_password", - "--credential-cache", "", // must specify --credential-cache or else the cache file on disk causes test pollution - }, - wantOptionsCount: 5, - wantStdout: `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{"interactive":false},"status":{"expirationTimestamp":"3020-10-12T13:14:15Z","token":"test-id-token"}}` + "\n", - }, - { - name: "ldap upstream type with browser_authcode flow is allowed", - args: []string{ - "--issuer", "test-issuer", - "--client-id", "test-client-id", - "--upstream-identity-provider-type", "ldap", - "--upstream-identity-provider-flow", "browser_authcode", - "--credential-cache", "", // must specify --credential-cache or else the cache file on disk causes test pollution - }, - wantOptionsCount: 4, - wantStdout: `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{"interactive":false},"status":{"expirationTimestamp":"3020-10-12T13:14:15Z","token":"test-id-token"}}` + "\n", - }, - { - name: "ldap upstream type with CLI flow in flow override env var is allowed", - args: []string{ - "--issuer", "test-issuer", - "--client-id", "test-client-id", - "--upstream-identity-provider-type", "ldap", - "--upstream-identity-provider-flow", "browser_authcode", - "--credential-cache", "", // must specify --credential-cache or else the cache file on disk causes test pollution - }, - env: map[string]string{"PINNIPED_UPSTREAM_IDENTITY_PROVIDER_FLOW": "cli_password"}, - wantOptionsCount: 5, - wantStdout: `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{"interactive":false},"status":{"expirationTimestamp":"3020-10-12T13:14:15Z","token":"test-id-token"}}` + "\n", - }, - { - name: "ldap upstream type with browser_authcode flow in flow override env var is allowed", - args: []string{ - "--issuer", "test-issuer", - "--client-id", "test-client-id", - "--upstream-identity-provider-type", "ldap", - "--upstream-identity-provider-flow", "cli_password", - "--credential-cache", "", // must specify --credential-cache or else the cache file on disk causes test pollution - }, - env: map[string]string{"PINNIPED_UPSTREAM_IDENTITY_PROVIDER_FLOW": "browser_authcode"}, - wantOptionsCount: 4, - wantStdout: `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{"interactive":false},"status":{"expirationTimestamp":"3020-10-12T13:14:15Z","token":"test-id-token"}}` + "\n", - }, - { - name: "ldap upstream type with unsupported flow is an error", - args: []string{ - "--issuer", "test-issuer", - "--client-id", "test-client-id", - "--upstream-identity-provider-type", "ldap", - "--upstream-identity-provider-flow", "foo", - "--credential-cache", "", // must specify --credential-cache or else the cache file on disk causes test pollution - }, - wantError: true, - wantStderr: here.Doc(` - Error: --upstream-identity-provider-flow value not recognized for identity provider type "ldap": foo (supported values: cli_password, browser_authcode) - `), - }, - { - name: "ldap upstream type with unsupported flow in flow override env var is an error", - args: []string{ - "--issuer", "test-issuer", - "--client-id", "test-client-id", - "--upstream-identity-provider-type", "ldap", - "--upstream-identity-provider-flow", "browser_authcode", - "--credential-cache", "", // must specify --credential-cache or else the cache file on disk causes test pollution - }, - env: map[string]string{"PINNIPED_UPSTREAM_IDENTITY_PROVIDER_FLOW": "foo"}, - wantError: true, - wantStderr: here.Doc(` - Error: PINNIPED_UPSTREAM_IDENTITY_PROVIDER_FLOW value not recognized for identity provider type "ldap": foo (supported values: cli_password, browser_authcode) - `), - }, - { - name: "active directory upstream type with CLI flow is allowed", - args: []string{ - "--issuer", "test-issuer", - "--client-id", "test-client-id", - "--upstream-identity-provider-type", "activedirectory", - "--upstream-identity-provider-flow", "cli_password", - "--credential-cache", "", // must specify --credential-cache or else the cache file on disk causes test pollution - }, - wantOptionsCount: 5, - wantStdout: `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{"interactive":false},"status":{"expirationTimestamp":"3020-10-12T13:14:15Z","token":"test-id-token"}}` + "\n", - }, - { - name: "active directory upstream type with browser_authcode is allowed", - args: []string{ - "--issuer", "test-issuer", - "--client-id", "test-client-id", - "--upstream-identity-provider-type", "activedirectory", - "--upstream-identity-provider-flow", "browser_authcode", - "--credential-cache", "", // must specify --credential-cache or else the cache file on disk causes test pollution - }, - wantOptionsCount: 4, - wantStdout: `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{"interactive":false},"status":{"expirationTimestamp":"3020-10-12T13:14:15Z","token":"test-id-token"}}` + "\n", - }, - { - name: "active directory upstream type with CLI flow in flow override env var is allowed", - args: []string{ - "--issuer", "test-issuer", - "--client-id", "test-client-id", - "--upstream-identity-provider-type", "activedirectory", - "--upstream-identity-provider-flow", "browser_authcode", - "--credential-cache", "", // must specify --credential-cache or else the cache file on disk causes test pollution + env: map[string]string{"PINNIPED_UPSTREAM_IDENTITY_PROVIDER_FLOW": "actual-value-from-env"}, + wantOptions: func(f *mockoidcclientoptions.MockOIDCClientOptions) { + defaultWantedOptions(f) + f.EXPECT().WithLoginFlow(idpdiscoveryv1alpha1.IDPFlow("actual-value-from-env"), "PINNIPED_UPSTREAM_IDENTITY_PROVIDER_FLOW") }, - env: map[string]string{"PINNIPED_UPSTREAM_IDENTITY_PROVIDER_FLOW": "cli_password"}, wantOptionsCount: 5, wantStdout: `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{"interactive":false},"status":{"expirationTimestamp":"3020-10-12T13:14:15Z","token":"test-id-token"}}` + "\n", }, - { - name: "active directory upstream type with browser_authcode in flow override env var is allowed", - args: []string{ - "--issuer", "test-issuer", - "--client-id", "test-client-id", - "--upstream-identity-provider-type", "activedirectory", - "--upstream-identity-provider-flow", "cli_password", - "--credential-cache", "", // must specify --credential-cache or else the cache file on disk causes test pollution - }, - env: map[string]string{"PINNIPED_UPSTREAM_IDENTITY_PROVIDER_FLOW": "browser_authcode"}, - wantOptionsCount: 4, - wantStdout: `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{"interactive":false},"status":{"expirationTimestamp":"3020-10-12T13:14:15Z","token":"test-id-token"}}` + "\n", - }, - { - name: "active directory upstream type with unsupported flow is an error", - args: []string{ - "--issuer", "test-issuer", - "--client-id", "test-client-id", - "--upstream-identity-provider-type", "activedirectory", - "--upstream-identity-provider-flow", "foo", - "--credential-cache", "", // must specify --credential-cache or else the cache file on disk causes test pollution - }, - wantError: true, - wantStderr: here.Doc(` - Error: --upstream-identity-provider-flow value not recognized for identity provider type "activedirectory": foo (supported values: cli_password, browser_authcode) - `), - }, - { - name: "active directory upstream type with unsupported flow in flow override env var is an error", - args: []string{ - "--issuer", "test-issuer", - "--client-id", "test-client-id", - "--upstream-identity-provider-type", "activedirectory", - "--upstream-identity-provider-flow", "browser_authcode", - "--credential-cache", "", // must specify --credential-cache or else the cache file on disk causes test pollution - }, - env: map[string]string{"PINNIPED_UPSTREAM_IDENTITY_PROVIDER_FLOW": "foo"}, - wantError: true, - wantStderr: here.Doc(` - Error: PINNIPED_UPSTREAM_IDENTITY_PROVIDER_FLOW value not recognized for identity provider type "activedirectory": foo (supported values: cli_password, browser_authcode) - `), - }, { name: "login error", args: []string{ @@ -493,6 +236,7 @@ func TestLoginOIDCCommand(t *testing.T) { "--credential-cache", "", // must specify --credential-cache or else the cache file on disk causes test pollution }, loginErr: fmt.Errorf("some login error"), + wantOptions: defaultWantedOptions, wantOptionsCount: 4, wantError: true, wantStderr: here.Doc(` @@ -511,6 +255,7 @@ func TestLoginOIDCCommand(t *testing.T) { "--credential-cache", "", // must specify --credential-cache or else the cache file on disk causes test pollution }, conciergeErr: fmt.Errorf("some concierge error"), + wantOptions: defaultWantedOptions, wantOptionsCount: 4, wantError: true, wantStderr: here.Doc(` @@ -525,11 +270,12 @@ func TestLoginOIDCCommand(t *testing.T) { "--credential-cache", "", // must specify --credential-cache or else the cache file on disk causes test pollution }, env: map[string]string{"PINNIPED_DEBUG": "true"}, + wantOptions: defaultWantedOptions, wantOptionsCount: 4, wantStdout: `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{"interactive":false},"status":{"expirationTimestamp":"3020-10-12T13:14:15Z","token":"test-id-token"}}` + "\n", wantLogs: []string{ - nowStr + ` pinniped-login cmd/login_oidc.go:260 Performing OIDC login {"issuer": "test-issuer", "client id": "test-client-id"}`, - nowStr + ` pinniped-login cmd/login_oidc.go:280 No concierge configured, skipping token credential exchange`, + nowStr + ` pinniped-login cmd/login_oidc.go:259 Performing OIDC login {"issuer": "test-issuer", "client id": "test-client-id"}`, + nowStr + ` pinniped-login cmd/login_oidc.go:288 No concierge configured, skipping token credential exchange`, }, }, { @@ -553,15 +299,30 @@ func TestLoginOIDCCommand(t *testing.T) { "--credential-cache", t.TempDir() + "/credentials.yaml", // must specify --credential-cache or else the cache file on disk causes test pollution "--upstream-identity-provider-name", "some-upstream-name", "--upstream-identity-provider-type", "ldap", + "--upstream-identity-provider-flow", "some-flow-type", + }, + env: map[string]string{"PINNIPED_DEBUG": "true", "PINNIPED_SKIP_PRINT_LOGIN_URL": "true"}, + wantOptions: func(f *mockoidcclientoptions.MockOIDCClientOptions) { + f.EXPECT().WithContext(gomock.Any()) + f.EXPECT().WithLogger(gomock.Any()) + f.EXPECT().WithScopes([]string{oidcapi.ScopeOfflineAccess, oidcapi.ScopeOpenID, oidcapi.ScopeRequestAudience, oidcapi.ScopeUsername, oidcapi.ScopeGroups}) + f.EXPECT().WithSessionCache(gomock.Any()) + f.EXPECT().WithListenPort(uint16(1234)) + f.EXPECT().WithSkipBrowserOpen() + f.EXPECT().WithSkipListen() + f.EXPECT().WithSkipPrintLoginURL() + f.EXPECT().WithClient(gomock.Any()) + f.EXPECT().WithRequestAudience("cluster-1234") + f.EXPECT().WithLoginFlow(idpdiscoveryv1alpha1.IDPFlow("some-flow-type"), "--upstream-identity-provider-flow") + f.EXPECT().WithUpstreamIdentityProvider("some-upstream-name", "ldap") }, - env: map[string]string{"PINNIPED_DEBUG": "true", "PINNIPED_SKIP_PRINT_LOGIN_URL": "true"}, wantOptionsCount: 12, wantStdout: `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{"interactive":false},"status":{"token":"exchanged-token"}}` + "\n", wantLogs: []string{ - nowStr + ` pinniped-login cmd/login_oidc.go:260 Performing OIDC login {"issuer": "test-issuer", "client id": "test-client-id"}`, - nowStr + ` pinniped-login cmd/login_oidc.go:270 Exchanging token for cluster credential {"endpoint": "https://127.0.0.1:1234/", "authenticator type": "webhook", "authenticator name": "test-authenticator"}`, - nowStr + ` pinniped-login cmd/login_oidc.go:278 Successfully exchanged token for cluster credential.`, - nowStr + ` pinniped-login cmd/login_oidc.go:285 caching cluster credential for future use.`, + nowStr + ` pinniped-login cmd/login_oidc.go:259 Performing OIDC login {"issuer": "test-issuer", "client id": "test-client-id"}`, + nowStr + ` pinniped-login cmd/login_oidc.go:278 Exchanging token for cluster credential {"endpoint": "https://127.0.0.1:1234/", "authenticator type": "webhook", "authenticator name": "test-authenticator"}`, + nowStr + ` pinniped-login cmd/login_oidc.go:286 Successfully exchanged token for cluster credential.`, + nowStr + ` pinniped-login cmd/login_oidc.go:293 caching cluster credential for future use.`, }, }, } diff --git a/cmd/pinniped/cmd/oidc_client_options.go b/cmd/pinniped/cmd/oidc_client_options.go index ba3c82379..7b5f098c9 100644 --- a/cmd/pinniped/cmd/oidc_client_options.go +++ b/cmd/pinniped/cmd/oidc_client_options.go @@ -1,3 +1,6 @@ +// Copyright 2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + package cmd import ( @@ -24,7 +27,7 @@ type OIDCClientOptions interface { WithClient(httpClient *http.Client) oidcclient.Option WithScopes(scopes []string) oidcclient.Option WithRequestAudience(audience string) oidcclient.Option - WithLoginFlow(loginFlow v1alpha1.IDPFlow) oidcclient.Option + WithLoginFlow(loginFlow v1alpha1.IDPFlow, flowSource string) oidcclient.Option WithUpstreamIdentityProvider(upstreamName, upstreamType string) oidcclient.Option } @@ -73,8 +76,8 @@ func (o *clientOptions) WithRequestAudience(audience string) oidcclient.Option { return oidcclient.WithRequestAudience(audience) } -func (o *clientOptions) WithLoginFlow(loginFlow v1alpha1.IDPFlow) oidcclient.Option { - return oidcclient.WithLoginFlow(loginFlow) +func (o *clientOptions) WithLoginFlow(loginFlow v1alpha1.IDPFlow, flowSource string) oidcclient.Option { + return oidcclient.WithLoginFlow(loginFlow, flowSource) } func (o *clientOptions) WithUpstreamIdentityProvider(upstreamName, upstreamType string) oidcclient.Option { diff --git a/internal/mocks/mockoidcclientoptions/mockoidcclientoptions.go b/internal/mocks/mockoidcclientoptions/mockoidcclientoptions.go index aec2ee8b5..231d3f897 100644 --- a/internal/mocks/mockoidcclientoptions/mockoidcclientoptions.go +++ b/internal/mocks/mockoidcclientoptions/mockoidcclientoptions.go @@ -104,17 +104,17 @@ func (mr *MockOIDCClientOptionsMockRecorder) WithLogger(arg0 any) *gomock.Call { } // WithLoginFlow mocks base method. -func (m *MockOIDCClientOptions) WithLoginFlow(arg0 v1alpha1.IDPFlow) oidcclient.Option { +func (m *MockOIDCClientOptions) WithLoginFlow(arg0 v1alpha1.IDPFlow, arg1 string) oidcclient.Option { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "WithLoginFlow", arg0) + ret := m.ctrl.Call(m, "WithLoginFlow", arg0, arg1) ret0, _ := ret[0].(oidcclient.Option) return ret0 } // WithLoginFlow indicates an expected call of WithLoginFlow. -func (mr *MockOIDCClientOptionsMockRecorder) WithLoginFlow(arg0 any) *gomock.Call { +func (mr *MockOIDCClientOptionsMockRecorder) WithLoginFlow(arg0, arg1 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WithLoginFlow", reflect.TypeOf((*MockOIDCClientOptions)(nil).WithLoginFlow), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WithLoginFlow", reflect.TypeOf((*MockOIDCClientOptions)(nil).WithLoginFlow), arg0, arg1) } // WithRequestAudience mocks base method. diff --git a/pkg/oidcclient/login.go b/pkg/oidcclient/login.go index 1454447af..dd2e4029e 100644 --- a/pkg/oidcclient/login.go +++ b/pkg/oidcclient/login.go @@ -16,6 +16,7 @@ import ( "net/http" "net/url" "os" + "slices" "sort" "strings" "sync" @@ -27,7 +28,6 @@ import ( "golang.org/x/oauth2" "golang.org/x/term" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/strings/slices" idpdiscoveryv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idpdiscovery/v1alpha1" oidcapi "go.pinniped.dev/generated/latest/apis/supervisor/oidc" @@ -89,7 +89,7 @@ type handlerState struct { // Tracking the usage of some other functional options. upstreamIdentityProviderName string - upstreamIdentityProviderType string + upstreamIdentityProviderType idpdiscoveryv1alpha1.IDPType cliToSendCredentials bool loginFlow idpdiscoveryv1alpha1.IDPFlow skipBrowser bool @@ -103,6 +103,7 @@ type handlerState struct { // Generated parameters of a login flow. provider *coreosoidc.Provider + idpDiscovery *idpdiscoveryv1alpha1.IDPDiscoveryResponse oauth2Config *oauth2.Config useFormPost bool state state.State @@ -269,14 +270,16 @@ func WithCLISendingCredentials() Option { // and perform the OIDC authcode flow. // When not used, the default when the issuer is a Pinniped Supervisor will be determined automatically, // and the default for non-Supervisor issuers will be the browser authcode flow. -func WithLoginFlow(loginFlow idpdiscoveryv1alpha1.IDPFlow) Option { +func WithLoginFlow(loginFlow idpdiscoveryv1alpha1.IDPFlow, flowSource string) Option { return func(h *handlerState) error { switch loginFlow { case idpdiscoveryv1alpha1.IDPFlowCLIPassword, idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode: default: return fmt.Errorf( - "WithLoginFlow parameter must be %s or %s", + "WithLoginFlow error: loginFlow '%s' from '%s' must be '%s' or '%s'", + loginFlow, + flowSource, idpdiscoveryv1alpha1.IDPFlowCLIPassword, idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode, ) @@ -293,7 +296,10 @@ func WithLoginFlow(loginFlow idpdiscoveryv1alpha1.IDPFlow) Option { func WithUpstreamIdentityProvider(upstreamName, upstreamType string) Option { return func(h *handlerState) error { h.upstreamIdentityProviderName = upstreamName - h.upstreamIdentityProviderType = upstreamType + + // Do not perform validation on this cast. + // If possible, dynamic validation against a Pinniped Supervisor's supported IDP types will be performed. + h.upstreamIdentityProviderType = idpdiscoveryv1alpha1.IDPType(upstreamType) return nil } } @@ -340,8 +346,11 @@ func Login(issuer string, clientID string, opts ...Option) (*oidctypes.Token, er } } - if h.loginFlow != "" && h.cliToSendCredentials { - return nil, fmt.Errorf("do not use deprecated option WithCLISendingCredentials when using option WithLoginFlow") + if h.cliToSendCredentials { + if h.loginFlow != "" { + return nil, fmt.Errorf("do not use deprecated option WithCLISendingCredentials when using option WithLoginFlow") + } + h.loginFlow = idpdiscoveryv1alpha1.IDPFlowCLIPassword } // Copy the configured HTTP client to set a request timeout (the Go default client has no timeout configured). @@ -472,7 +481,7 @@ func (h *handlerState) baseLogin() (*oidctypes.Token, error) { ) // 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, h.upstreamIdentityProviderType), + oauth2.SetAuthURLParam(oidcapi.AuthorizeUpstreamIDPTypeParamName, string(h.upstreamIdentityProviderType)), ) } @@ -490,13 +499,42 @@ 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, + h.upstreamIdentityProviderType, + ) + } + } + + // 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] + } + } + } + + // Preserve the legacy behavior where browser-based auth is preferred + authFunc := h.webBrowserBasedAuth + // Choose the appropriate authorization and authcode exchange strategy. - var authFunc = h.webBrowserBasedAuth - // TODO Note that this is the only place where the flow setting is currently used in this file. - // So in the new approach we would need to validate the requested flow type and make a final determination - // of flow type before this line of code. Luckily, OIDC discovery has already happened by now. - if h.loginFlow == idpdiscoveryv1alpha1.IDPFlowCLIPassword || h.cliToSendCredentials { + // 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. @@ -817,7 +855,7 @@ func promptForSecret(promptLabel string, out io.Writer) (string, error) { } func (h *handlerState) initOIDCDiscovery() error { - // Make this method idempotent so it can be called in multiple cases with no extra network requests. + // Make this method idempotent, so it can be called in multiple cases with no extra network requests. if h.provider != nil { return nil } @@ -861,6 +899,64 @@ func (h *handlerState) initOIDCDiscovery() error { return fmt.Errorf("could not decode response_modes_supported in OIDC discovery from %q: %w", h.issuer, err) } h.useFormPost = slices.Contains(discoveryClaims.ResponseModesSupported, "form_post") + + return h.maybePerformPinnipedSupervisorIDPDiscovery() +} + +func (h *handlerState) maybePerformPinnipedSupervisorIDPDiscovery() error { + // If this OIDC IDP is a Pinniped Supervisor, it will have a reference to the IDP discovery document. + // Go to that document and retrieve the IDPs. + // 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 + }() + + 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) + } + + 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) + } + + 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 8321b8141..1d347daf3 100644 --- a/pkg/oidcclient/login_test.go +++ b/pkg/oidcclient/login_test.go @@ -10,7 +10,6 @@ import ( "encoding/json" "errors" "fmt" - idpdiscoveryv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idpdiscovery/v1alpha1" "io" "net" "net/http" @@ -30,6 +29,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/klog/v2" + idpdiscoveryv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idpdiscovery/v1alpha1" + federationdomainoidc "go.pinniped.dev/internal/federationdomain/oidc" "go.pinniped.dev/internal/federationdomain/upstreamprovider" "go.pinniped.dev/internal/httputil/httperr" "go.pinniped.dev/internal/httputil/roundtripper" @@ -190,6 +191,70 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }) }) + // Start a test server that returns IDP discovery at some other location + emptyIDPDiscoveryMux := http.NewServeMux() + emptyIDPDiscoveryServer, emptyIDPDiscoveryServerCA := tlsserver.TestServerIPv4(t, emptyIDPDiscoveryMux, nil) + emptyIDPDiscoveryMux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + http.Error(w, "unexpected method", http.StatusMethodNotAllowed) + return + } + w.Header().Set("content-type", "application/json") + _ = json.NewEncoder(w).Encode(&struct { + Issuer string `json:"issuer"` + AuthURL string `json:"authorization_endpoint"` + TokenURL string `json:"token_endpoint"` + JWKSURL string `json:"jwks_uri"` + ResponseModesSupported []string `json:"response_modes_supported,omitempty"` + SupervisorDiscovery idpdiscoveryv1alpha1.OIDCDiscoveryResponseIDPEndpoint `json:"discovery.supervisor.pinniped.dev/v1alpha1"` + }{ + Issuer: emptyIDPDiscoveryServer.URL, + AuthURL: emptyIDPDiscoveryServer.URL + "/authorize", + TokenURL: emptyIDPDiscoveryServer.URL + "/token", + JWKSURL: emptyIDPDiscoveryServer.URL + "/keys", + ResponseModesSupported: []string{}, + SupervisorDiscovery: idpdiscoveryv1alpha1.OIDCDiscoveryResponseIDPEndpoint{ + PinnipedIDPsEndpoint: "https://example.com" + federationdomainoidc.PinnipedIDPsPathV1Alpha1, + }, + }) + }) + + // Start a test server that has invalid IDP discovery + invalidIDPDiscoveryMux := http.NewServeMux() + invalidIDPDiscoveryServer, invalidIDPDiscoveryServerCA := tlsserver.TestServerIPv4(t, invalidIDPDiscoveryMux, nil) + invalidIDPDiscoveryMux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + http.Error(w, "unexpected method", http.StatusMethodNotAllowed) + return + } + w.Header().Set("content-type", "application/json") + _ = json.NewEncoder(w).Encode(&struct { + Issuer string `json:"issuer"` + AuthURL string `json:"authorization_endpoint"` + TokenURL string `json:"token_endpoint"` + JWKSURL string `json:"jwks_uri"` + ResponseModesSupported []string `json:"response_modes_supported,omitempty"` + SupervisorDiscovery idpdiscoveryv1alpha1.OIDCDiscoveryResponseIDPEndpoint `json:"discovery.supervisor.pinniped.dev/v1alpha1"` + }{ + Issuer: invalidIDPDiscoveryServer.URL, + AuthURL: invalidIDPDiscoveryServer.URL + "/authorize", + TokenURL: invalidIDPDiscoveryServer.URL + "/token", + JWKSURL: invalidIDPDiscoveryServer.URL + "/keys", + ResponseModesSupported: []string{}, + SupervisorDiscovery: idpdiscoveryv1alpha1.OIDCDiscoveryResponseIDPEndpoint{ + PinnipedIDPsEndpoint: invalidIDPDiscoveryServer.URL + federationdomainoidc.PinnipedIDPsPathV1Alpha1, + }, + }) + }) + invalidIDPDiscoveryMux.HandleFunc(federationdomainoidc.PinnipedIDPsPathV1Alpha1, func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + http.Error(w, "unexpected method", http.StatusMethodNotAllowed) + return + } + w.Header().Set("content-type", "application/json") + _, _ = fmt.Fprint(w, "not real json") + }) + discoveryHandler := func(server *httptest.Server, responseModes []string) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodGet { @@ -198,17 +263,55 @@ func TestLogin(t *testing.T) { //nolint:gocyclo } w.Header().Set("content-type", "application/json") _ = json.NewEncoder(w).Encode(&struct { - Issuer string `json:"issuer"` - AuthURL string `json:"authorization_endpoint"` - TokenURL string `json:"token_endpoint"` - JWKSURL string `json:"jwks_uri"` - ResponseModesSupported []string `json:"response_modes_supported,omitempty"` + Issuer string `json:"issuer"` + AuthURL string `json:"authorization_endpoint"` + TokenURL string `json:"token_endpoint"` + JWKSURL string `json:"jwks_uri"` + ResponseModesSupported []string `json:"response_modes_supported,omitempty"` + SupervisorDiscovery idpdiscoveryv1alpha1.OIDCDiscoveryResponseIDPEndpoint `json:"discovery.supervisor.pinniped.dev/v1alpha1"` }{ Issuer: server.URL, AuthURL: server.URL + "/authorize", TokenURL: server.URL + "/token", JWKSURL: server.URL + "/keys", ResponseModesSupported: responseModes, + SupervisorDiscovery: idpdiscoveryv1alpha1.OIDCDiscoveryResponseIDPEndpoint{ + PinnipedIDPsEndpoint: server.URL + federationdomainoidc.PinnipedIDPsPathV1Alpha1, + }, + }) + } + } + + idpDiscoveryHandler := func(server *httptest.Server) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + http.Error(w, "unexpected method", http.StatusMethodNotAllowed) + return + } + w.Header().Set("content-type", "application/json") + _ = json.NewEncoder(w).Encode(idpdiscoveryv1alpha1.IDPDiscoveryResponse{ + PinnipedIDPs: []idpdiscoveryv1alpha1.PinnipedIDP{ + { + Name: "some-upstream-name", + Type: idpdiscoveryv1alpha1.IDPTypeLDAP, + Flows: []idpdiscoveryv1alpha1.IDPFlow{ + idpdiscoveryv1alpha1.IDPFlowCLIPassword, + idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode, + }, + }, + { + Name: "some-oidc-upstream-name", + Type: idpdiscoveryv1alpha1.IDPTypeOIDC, + Flows: []idpdiscoveryv1alpha1.IDPFlow{ + idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode, + idpdiscoveryv1alpha1.IDPFlowCLIPassword, + }, + }, + }, + PinnipedSupportedIDPTypes: []idpdiscoveryv1alpha1.PinnipedSupportedIDPType{ + {Type: idpdiscoveryv1alpha1.IDPTypeOIDC}, + {Type: idpdiscoveryv1alpha1.IDPTypeLDAP}, + }, }) } } @@ -300,14 +403,18 @@ func TestLogin(t *testing.T) { //nolint:gocyclo providerMux := http.NewServeMux() successServer, successServerCA := tlsserver.TestServerIPv4(t, providerMux, nil) providerMux.HandleFunc("/.well-known/openid-configuration", discoveryHandler(successServer, nil)) + providerMux.HandleFunc(federationdomainoidc.PinnipedIDPsPathV1Alpha1, idpDiscoveryHandler(successServer)) providerMux.HandleFunc("/token", tokenHandler) // Start a test server that returns a real discovery document and answers refresh requests, _and_ supports form_mode=post. formPostProviderMux := http.NewServeMux() formPostSuccessServer, formPostSuccessServerCA := tlsserver.TestServerIPv4(t, formPostProviderMux, nil) formPostProviderMux.HandleFunc("/.well-known/openid-configuration", discoveryHandler(formPostSuccessServer, []string{"query", "form_post"})) + formPostProviderMux.HandleFunc(federationdomainoidc.PinnipedIDPsPathV1Alpha1, idpDiscoveryHandler(formPostSuccessServer)) formPostProviderMux.HandleFunc("/token", tokenHandler) + // TODO: Use a test server without federationdomainoidc.PinnipedIDPsPathV1Alpha1 (e.g. a non-Supervisor server) + defaultDiscoveryResponse := func(req *http.Request) (*http.Response, error) { // Call the handler function from the test server to calculate the response. handler, _ := providerMux.Handler(req) @@ -337,7 +444,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo require.Equal(t, []SessionCacheKey{cacheKey}, cache.sawGetKeys) }) require.NoError(t, WithSessionCache(cache)(h)) - require.NoError(t, WithLoginFlow(idpdiscoveryv1alpha1.IDPFlowCLIPassword)(h)) + require.NoError(t, WithLoginFlow(idpdiscoveryv1alpha1.IDPFlowCLIPassword, "foo")(h)) require.NoError(t, WithUpstreamIdentityProvider("some-upstream-name", "ldap")(h)) require.NoError(t, WithClient(buildHTTPClientForPEM(successServerCA))(h)) @@ -346,6 +453,8 @@ func TestLogin(t *testing.T) { //nolint:gocyclo switch req.URL.Scheme + "://" + req.URL.Host + req.URL.Path { case "https://" + successServer.Listener.Addr().String() + "/.well-known/openid-configuration": return defaultDiscoveryResponse(req) + case "https://" + successServer.Listener.Addr().String() + federationdomainoidc.PinnipedIDPsPathV1Alpha1: + return defaultDiscoveryResponse(req) case "https://" + successServer.Listener.Addr().String() + "/authorize": return authResponse, authError default: @@ -380,8 +489,8 @@ func TestLogin(t *testing.T) { //nolint:gocyclo name: "WithLoginFlow option and deprecated WithCLISendingCredentials option cannot be used together (with CLI flow selected)", opt: func(t *testing.T) Option { return func(h *handlerState) error { - require.NoError(t, WithCLISendingCredentials()(h)) - require.NoError(t, WithLoginFlow(idpdiscoveryv1alpha1.IDPFlowCLIPassword)(h)) + require.NoError(t, WithCLISendingCredentials()(h)) // This is meant to call a deprecated function + require.NoError(t, WithLoginFlow(idpdiscoveryv1alpha1.IDPFlowCLIPassword, "foo")(h)) return nil } }, @@ -391,26 +500,26 @@ func TestLogin(t *testing.T) { //nolint:gocyclo name: "WithLoginFlow option and deprecated WithCLISendingCredentials option cannot be used together (with browser flow selected)", opt: func(t *testing.T) Option { return func(h *handlerState) error { - require.NoError(t, WithCLISendingCredentials()(h)) - require.NoError(t, WithLoginFlow(idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode)(h)) + require.NoError(t, WithCLISendingCredentials()(h)) // This is meant to call a deprecated function + require.NoError(t, WithLoginFlow(idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode, "foo")(h)) return nil } }, wantErr: "do not use deprecated option WithCLISendingCredentials when using option WithLoginFlow", }, { - name: "WithLoginFlow option will only accept the actual enum values", + name: "WithLoginFlow option rejects a non-enum value", opt: func(t *testing.T) Option { - return WithLoginFlow("this is not one of the enum values") + return WithLoginFlow("this is not one of the enum values", "some-flow-source") }, - wantErr: "WithLoginFlow parameter must be cli_password or browser_authcode", + wantErr: "WithLoginFlow error: loginFlow 'this is not one of the enum values' from 'some-flow-source' must be 'cli_password' or 'browser_authcode'", }, { name: "WithLoginFlow option will not accept empty string either", opt: func(t *testing.T) Option { - return WithLoginFlow("") + return WithLoginFlow("", "other-flow-source") }, - wantErr: "WithLoginFlow parameter must be cli_password or browser_authcode", + wantErr: "WithLoginFlow error: loginFlow '' from 'other-flow-source' must be 'cli_password' or 'browser_authcode'", }, { name: "error generating state", @@ -718,6 +827,30 @@ func TestLogin(t *testing.T) { //nolint:gocyclo wantLogs: []string{`"level"=4 "msg"="Pinniped: Performing OIDC discovery" "issuer"="` + insecureAuthURLServer.URL + `"`}, wantErr: `discovered authorize URL from issuer must be an https URL, but had scheme "http" instead`, }, + { + name: "issuer has Pinniped Supervisor's IDP discovery, but from another location", + opt: func(t *testing.T) Option { + return func(h *handlerState) error { + require.NoError(t, WithClient(buildHTTPClientForPEM(emptyIDPDiscoveryServerCA))(h)) + return nil + } + }, + 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), + }, + { + name: "issuer has Pinniped Supervisor's IDP discovery, but it cannot be unmarshaled", + opt: func(t *testing.T) Option { + return func(h *handlerState) error { + require.NoError(t, WithClient(buildHTTPClientForPEM(invalidIDPDiscoveryServerCA))(h)) + return nil + } + }, + issuer: invalidIDPDiscoveryServer.URL, + wantLogs: []string{`"level"=4 "msg"="Pinniped: Performing OIDC discovery" "issuer"="` + invalidIDPDiscoveryServer.URL + `"`}, + wantErr: "unable to fetch the Pinniped IDP discovery document: could not parse response JSON: invalid character 'o' in literal null (expecting 'u')", + }, { name: "listen failure and non-tty stdin", opt: func(t *testing.T) Option { @@ -1284,7 +1417,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: "some-oidc-upstream-name", } t.Cleanup(func() { require.Equal(t, []SessionCacheKey{cacheKey}, cache.sawGetKeys) @@ -1292,7 +1425,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", "oidc")(h)) + require.NoError(t, WithUpstreamIdentityProvider("some-oidc-upstream-name", "oidc")(h)) client := buildHTTPClientForPEM(successServerCA) client.Timeout = 10 * time.Second @@ -1315,7 +1448,7 @@ 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-upstream-name"}, + "pinniped_idp_name": []string{"some-oidc-upstream-name"}, "pinniped_idp_type": []string{"oidc"}, }, actualParams) @@ -1337,7 +1470,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-upstream-name&pinniped_idp_type=oidc"+ + "&code_challenge_method=S256&nonce=test-nonce&pinniped_idp_name=some-oidc-upstream-name&pinniped_idp_type=oidc"+ "&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") + @@ -1593,7 +1726,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)(h)) + require.NoError(t, WithLoginFlow(idpdiscoveryv1alpha1.IDPFlowCLIPassword, "foo")(h)) require.NoError(t, WithUpstreamIdentityProvider("some-upstream-name", "ldap")(h)) discoveryRequestWasMade := false @@ -1609,6 +1742,8 @@ func TestLogin(t *testing.T) { //nolint:gocyclo case "https://" + successServer.Listener.Addr().String() + "/.well-known/openid-configuration": discoveryRequestWasMade = true return defaultDiscoveryResponse(req) + case "https://" + successServer.Listener.Addr().String() + federationdomainoidc.PinnipedIDPsPathV1Alpha1: + return defaultDiscoveryResponse(req) case "https://" + successServer.Listener.Addr().String() + "/authorize": authorizeRequestWasMade = true require.Equal(t, "some-upstream-username", req.Header.Get("Pinniped-Username")) @@ -1647,6 +1782,73 @@ func TestLogin(t *testing.T) { //nolint:gocyclo wantStdErr: "^\nLog in to some-upstream-name\n\n$", wantToken: &testToken, }, + { + name: "unable to login with unknown IDP, when Supervisor provides its supported IDP types", + clientID: "test-client-id", + opt: func(t *testing.T) Option { + return func(h *handlerState) error { + h.generateState = func() (state.State, error) { return "test-state", nil } + h.generatePKCE = func() (pkce.Code, error) { return "test-pkce", nil } + h.generateNonce = func() (nonce.Nonce, error) { return "test-nonce", nil } + h.getEnv = func(_ string) string { + return "" // asking for any env var returns empty as if it were unset + } + h.promptForValue = func(_ context.Context, promptLabel string, _ io.Writer) (string, error) { + require.Equal(t, "Username: ", promptLabel) + return "some-upstream-username", nil + } + h.promptForSecret = func(promptLabel string, _ io.Writer) (string, error) { + require.Equal(t, "Password: ", promptLabel) + return "some-upstream-password", nil + } + + cache := &mockSessionCache{t: t, getReturnsToken: nil} + cacheKey := SessionCacheKey{ + Issuer: successServer.URL, + ClientID: "test-client-id", + Scopes: []string{"test-scope"}, + RedirectURI: "http://localhost:0/callback", + UpstreamProviderName: "some-upstream-name", + } + t.Cleanup(func() { + require.Equal(t, []SessionCacheKey{cacheKey}, cache.sawGetKeys) + require.Equal(t, []SessionCacheKey(nil), cache.sawPutKeys) + 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)) + + discoveryRequestWasMade := false + idpDiscoveryRequestWasMade := false + t.Cleanup(func() { + require.True(t, discoveryRequestWasMade, "should have made an discovery request") + require.True(t, idpDiscoveryRequestWasMade, "should have made an discovery request") + }) + + client := buildHTTPClientForPEM(successServerCA) + client.Transport = roundtripper.Func(func(req *http.Request) (*http.Response, error) { + switch req.URL.Scheme + "://" + req.URL.Host + req.URL.Path { + case "https://" + successServer.Listener.Addr().String() + "/.well-known/openid-configuration": + discoveryRequestWasMade = true + return defaultDiscoveryResponse(req) + case "https://" + successServer.Listener.Addr().String() + federationdomainoidc.PinnipedIDPsPathV1Alpha1: + idpDiscoveryRequestWasMade = true + return defaultDiscoveryResponse(req) + default: + // Note that "/token" requests should not be made. They are mocked by mocking calls to ExchangeAuthcodeAndValidateTokens(). + require.FailNow(t, fmt.Sprintf("saw unexpected http call from the CLI: %s", req.URL.String())) + return nil, nil + } + }) + require.NoError(t, WithClient(client)(h)) + return nil + } + }, + 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'", + }, { name: "successful ldap login with prompts for username and password, using deprecated WithCLISendingCredentials option", clientID: "test-client-id", @@ -1692,7 +1894,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, WithCLISendingCredentials()(h)) + require.NoError(t, WithCLISendingCredentials()(h)) // This is meant to call a deprecated function require.NoError(t, WithUpstreamIdentityProvider("some-upstream-name", "ldap")(h)) discoveryRequestWasMade := false @@ -1708,6 +1910,111 @@ func TestLogin(t *testing.T) { //nolint:gocyclo case "https://" + successServer.Listener.Addr().String() + "/.well-known/openid-configuration": discoveryRequestWasMade = true return defaultDiscoveryResponse(req) + case "https://" + successServer.Listener.Addr().String() + federationdomainoidc.PinnipedIDPsPathV1Alpha1: + return defaultDiscoveryResponse(req) + case "https://" + successServer.Listener.Addr().String() + "/authorize": + authorizeRequestWasMade = true + require.Equal(t, "some-upstream-username", req.Header.Get("Pinniped-Username")) + require.Equal(t, "some-upstream-password", req.Header.Get("Pinniped-Password")) + require.Equal(t, url.Values{ + "code_challenge": []string{testCodeChallenge}, + "code_challenge_method": []string{"S256"}, + "response_type": []string{"code"}, + "scope": []string{"test-scope"}, + "nonce": []string{"test-nonce"}, + "state": []string{"test-state"}, + "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"}, + }, req.URL.Query()) + return &http.Response{ + StatusCode: http.StatusFound, + Header: http.Header{"Location": []string{ + fmt.Sprintf("http://127.0.0.1:0/callback?code=%s&state=test-state", fakeAuthCode), + }}, + }, nil + default: + // Note that "/token" requests should not be made. They are mocked by mocking calls to ExchangeAuthcodeAndValidateTokens(). + require.FailNow(t, fmt.Sprintf("saw unexpected http call from the CLI: %s", req.URL.String())) + return nil, nil + } + }) + require.NoError(t, WithClient(client)(h)) + return nil + } + }, + issuer: successServer.URL, + wantLogs: []string{`"level"=4 "msg"="Pinniped: Performing OIDC discovery" "issuer"="` + successServer.URL + `"`}, + wantStdErr: "^\nLog in to some-upstream-name\n\n$", + wantToken: &testToken, + }, + { + name: "successful ldap login with prompts for username and password, infers flow when not specified", + clientID: "test-client-id", + opt: func(t *testing.T) Option { + return func(h *handlerState) error { + fakeAuthCode := "test-authcode-value" + + h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { + mock := mockUpstream(t) + mock.EXPECT(). + ExchangeAuthcodeAndValidateTokens( + gomock.Any(), fakeAuthCode, pkce.Code("test-pkce"), nonce.Nonce("test-nonce"), "http://127.0.0.1:0/callback"). + Return(&testToken, nil) + return mock + } + + h.generateState = func() (state.State, error) { return "test-state", nil } + h.generatePKCE = func() (pkce.Code, error) { return "test-pkce", nil } + h.generateNonce = func() (nonce.Nonce, error) { return "test-nonce", nil } + h.getEnv = func(_ string) string { + return "" // asking for any env var returns empty as if it were unset + } + h.promptForValue = func(_ context.Context, promptLabel string, _ io.Writer) (string, error) { + require.Equal(t, "Username: ", promptLabel) + return "some-upstream-username", nil + } + h.promptForSecret = func(promptLabel string, _ io.Writer) (string, error) { + require.Equal(t, "Password: ", promptLabel) + return "some-upstream-password", nil + } + + cache := &mockSessionCache{t: t, getReturnsToken: nil} + cacheKey := SessionCacheKey{ + Issuer: successServer.URL, + ClientID: "test-client-id", + Scopes: []string{"test-scope"}, + RedirectURI: "http://localhost:0/callback", + UpstreamProviderName: "some-upstream-name", + } + t.Cleanup(func() { + require.Equal(t, []SessionCacheKey{cacheKey}, cache.sawGetKeys) + require.Equal(t, []SessionCacheKey{cacheKey}, cache.sawPutKeys) + require.Equal(t, []*oidctypes.Token{&testToken}, cache.sawPutTokens) + }) + require.NoError(t, WithSessionCache(cache)(h)) + require.NoError(t, WithUpstreamIdentityProvider("some-upstream-name", "ldap")(h)) + + discoveryRequestWasMade := false + idpDiscoveryRequestWasMade := false + authorizeRequestWasMade := false + t.Cleanup(func() { + require.True(t, discoveryRequestWasMade, "should have made an discovery request") + require.True(t, idpDiscoveryRequestWasMade, "should have made an IDP discovery request") + require.True(t, authorizeRequestWasMade, "should have made an authorize request") + }) + + client := buildHTTPClientForPEM(successServerCA) + client.Transport = roundtripper.Func(func(req *http.Request) (*http.Response, error) { + switch req.URL.Scheme + "://" + req.URL.Host + req.URL.Path { + case "https://" + successServer.Listener.Addr().String() + "/.well-known/openid-configuration": + discoveryRequestWasMade = true + return defaultDiscoveryResponse(req) + case "https://" + successServer.Listener.Addr().String() + federationdomainoidc.PinnipedIDPsPathV1Alpha1: + idpDiscoveryRequestWasMade = true + return defaultDiscoveryResponse(req) case "https://" + successServer.Listener.Addr().String() + "/authorize": authorizeRequestWasMade = true require.Equal(t, "some-upstream-username", req.Header.Get("Pinniped-Username")) @@ -1797,7 +2104,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)(h)) + require.NoError(t, WithLoginFlow(idpdiscoveryv1alpha1.IDPFlowCLIPassword, "foo")(h)) discoveryRequestWasMade := false authorizeRequestWasMade := false @@ -1812,6 +2119,8 @@ func TestLogin(t *testing.T) { //nolint:gocyclo case "https://" + successServer.Listener.Addr().String() + "/.well-known/openid-configuration": discoveryRequestWasMade = true return defaultDiscoveryResponse(req) + case "https://" + successServer.Listener.Addr().String() + federationdomainoidc.PinnipedIDPsPathV1Alpha1: + return defaultDiscoveryResponse(req) case "https://" + successServer.Listener.Addr().String() + "/authorize": authorizeRequestWasMade = true require.Equal(t, "some-upstream-username", req.Header.Get("Pinniped-Username")) @@ -1903,7 +2212,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)(h)) + require.NoError(t, WithLoginFlow(idpdiscoveryv1alpha1.IDPFlowCLIPassword, "foo")(h)) require.NoError(t, WithUpstreamIdentityProvider("some-upstream-name", "ldap")(h)) discoveryRequestWasMade := false @@ -1919,6 +2228,8 @@ func TestLogin(t *testing.T) { //nolint:gocyclo case "https://" + successServer.Listener.Addr().String() + "/.well-known/openid-configuration": discoveryRequestWasMade = true return defaultDiscoveryResponse(req) + case "https://" + successServer.Listener.Addr().String() + federationdomainoidc.PinnipedIDPsPathV1Alpha1: + return defaultDiscoveryResponse(req) case "https://" + successServer.Listener.Addr().String() + "/authorize": authorizeRequestWasMade = true require.Equal(t, "some-upstream-username", req.Header.Get("Pinniped-Username"))