From 6e461821d6e14d15b6a2d3c6bf40157e7cfe3f14 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Thu, 2 Jun 2022 10:30:03 -0700 Subject: [PATCH] Allow PINNIPED_UPSTREAM_IDENTITY_PROVIDER_FLOW env var to override flow Env var may be used with CLI to override the flow selected by the --upstream-identity-provider-flow CLI flag. --- cmd/pinniped/cmd/login_oidc.go | 36 +++++-- cmd/pinniped/cmd/login_oidc_test.go | 149 ++++++++++++++++++++++++++-- 2 files changed, 172 insertions(+), 13 deletions(-) diff --git a/cmd/pinniped/cmd/login_oidc.go b/cmd/pinniped/cmd/login_oidc.go index 8f9378f5..b31f8dd6 100644 --- a/cmd/pinniped/cmd/login_oidc.go +++ b/cmd/pinniped/cmd/login_oidc.go @@ -32,6 +32,15 @@ import ( "go.pinniped.dev/pkg/oidcclient/oidctypes" ) +const ( + // The user may override the flow selection made by `--upstream-identity-provider-flow` using an env var. + // This allows the user to override their default flow selected inside their Pinniped-compatible kubeconfig file. + // A user might want to use this env var, for example, to choose the "browser_authcode" flow when using a kubeconfig + // which specifies "cli_password" when using an IDE plugin where there is no interactive CLI available. This allows + // the user to use one kubeconfig file for both flows. + upstreamIdentityProviderFlowEnvVarName = "PINNIPED_UPSTREAM_IDENTITY_PROVIDER_FLOW" +) + // nolint: gochecknoinits func init() { loginCmd.AddCommand(oidcLoginCommand(oidcLoginCommandRealDeps())) @@ -165,6 +174,7 @@ func runOIDCLogin(cmd *cobra.Command, deps oidcLoginCommandDeps, flags oidcLogin flowOpts, err := flowOptions( idpdiscoveryv1alpha1.IDPType(flags.upstreamIdentityProviderType), idpdiscoveryv1alpha1.IDPFlow(flags.upstreamIdentityProviderFlow), + deps, ) if err != nil { return err @@ -250,9 +260,21 @@ func runOIDCLogin(cmd *cobra.Command, deps oidcLoginCommandDeps, flags oidcLogin return json.NewEncoder(cmd.OutOrStdout()).Encode(cred) } -func flowOptions(requestedIDPType idpdiscoveryv1alpha1.IDPType, requestedFlow idpdiscoveryv1alpha1.IDPFlow) ([]oidcclient.Option, error) { +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 + } + switch requestedIDPType { case idpdiscoveryv1alpha1.IDPTypeOIDC: switch requestedFlow { @@ -262,19 +284,21 @@ func flowOptions(requestedIDPType idpdiscoveryv1alpha1.IDPType, requestedFlow id return nil, nil // browser authcode flow is the default Option, so don't need to return an Option here default: return nil, fmt.Errorf( - "--upstream-identity-provider-flow value not recognized for identity provider type %q: %s (supported values: %s)", - requestedIDPType, requestedFlow, strings.Join([]string{idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode.String(), idpdiscoveryv1alpha1.IDPFlowCLIPassword.String()}, ", ")) + "%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 + return nil, nil // browser authcode flow is the default Option, so don't need to return an Option here default: return nil, fmt.Errorf( - "--upstream-identity-provider-flow value not recognized for identity provider type %q: %s (supported values: %s)", - requestedIDPType, requestedFlow, strings.Join([]string{idpdiscoveryv1alpha1.IDPFlowCLIPassword.String(), idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode.String()}, ", ")) + "%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 diff --git a/cmd/pinniped/cmd/login_oidc_test.go b/cmd/pinniped/cmd/login_oidc_test.go index 492891e2..2e4fbd45 100644 --- a/cmd/pinniped/cmd/login_oidc_test.go +++ b/cmd/pinniped/cmd/login_oidc_test.go @@ -148,7 +148,7 @@ func TestLoginOIDCCommand(t *testing.T) { `), }, { - name: "invalid upstream type", + name: "invalid upstream type is an error", args: []string{ "--issuer", "test-issuer", "--upstream-identity-provider-type", "invalid", @@ -158,6 +158,18 @@ func TestLoginOIDCCommand(t *testing.T) { 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"}, + 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{ @@ -193,6 +205,32 @@ func TestLoginOIDCCommand(t *testing.T) { 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{ @@ -207,6 +245,21 @@ func TestLoginOIDCCommand(t *testing.T) { 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{ @@ -253,6 +306,32 @@ func TestLoginOIDCCommand(t *testing.T) { 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{ @@ -267,6 +346,21 @@ func TestLoginOIDCCommand(t *testing.T) { 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{ @@ -291,6 +385,32 @@ func TestLoginOIDCCommand(t *testing.T) { 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": "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{ @@ -305,6 +425,21 @@ func TestLoginOIDCCommand(t *testing.T) { 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{ @@ -348,8 +483,8 @@ func TestLoginOIDCCommand(t *testing.T) { 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:222 Performing OIDC login {"issuer": "test-issuer", "client id": "test-client-id"}`, - nowStr + ` pinniped-login cmd/login_oidc.go:242 No concierge configured, skipping token credential exchange`, + nowStr + ` pinniped-login cmd/login_oidc.go:232 Performing OIDC login {"issuer": "test-issuer", "client id": "test-client-id"}`, + nowStr + ` pinniped-login cmd/login_oidc.go:252 No concierge configured, skipping token credential exchange`, }, }, { @@ -378,10 +513,10 @@ func TestLoginOIDCCommand(t *testing.T) { wantOptionsCount: 11, 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:222 Performing OIDC login {"issuer": "test-issuer", "client id": "test-client-id"}`, - nowStr + ` pinniped-login cmd/login_oidc.go:232 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:240 Successfully exchanged token for cluster credential.`, - nowStr + ` pinniped-login cmd/login_oidc.go:247 caching cluster credential for future use.`, + nowStr + ` pinniped-login cmd/login_oidc.go:232 Performing OIDC login {"issuer": "test-issuer", "client id": "test-client-id"}`, + nowStr + ` pinniped-login cmd/login_oidc.go:242 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:250 Successfully exchanged token for cluster credential.`, + nowStr + ` pinniped-login cmd/login_oidc.go:257 caching cluster credential for future use.`, }, }, }