djpbessems/ContainerImage.Pinniped
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #1180 from vmware-tanzu/cli_flow_env_var

Browse Source 
Allow `PINNIPED_UPSTREAM_IDENTITY_PROVIDER_FLOW` env var to override `--upstream-identity-provider-flow` CLI flag
This commit is contained in:
Margo Crawford 2022-06-06 11:49:00 -07:00 committed by GitHub
commit d6442ed53d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 246 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
cmd/pinniped/cmd/login_oidc.go
View File

@ -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

149
cmd/pinniped/cmd/login_oidc_test.go
View File

@ -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.`,
},
},
}

5
site/content/docs/howto/login.md
View File

@ -125,6 +125,11 @@ will depend on which type of identity provider was configured.
Unlike the optional flow for OIDC providers described above, this optional flow does not need to be configured in
the LDAPIdentityProvider or ActiveDirectoryIdentityProvider resource, so it is always available for end-users.
The flow selected by the `--upstream-identity-provider-flow` CLI flag may be overridden by using the
`PINNIPED_UPSTREAM_IDENTITY_PROVIDER_FLOW` environment variable for the CLI at runtime. This environment variable
may be set to the same values as the CLI flag (`browser_authcode` or `cli_password`). This allows a user to switch
flows based on their needs without editing their kubeconfig file.
Once the user completes authentication, the `kubectl` command will automatically continue and complete the user's requested command.
For the example above, `kubectl` would list the cluster's namespaces.

69
test/integration/e2e_test.go
View File

@ -53,6 +53,15 @@ import (
func TestE2EFullIntegration_Browser(t *testing.T) {
env := testlib.IntegrationEnv(t)
// Avoid allowing PINNIPED_UPSTREAM_IDENTITY_PROVIDER_FLOW to interfere with these tests.
originalFlowEnvVarValue, flowOverrideEnvVarSet := os.LookupEnv("PINNIPED_UPSTREAM_IDENTITY_PROVIDER_FLOW")
if flowOverrideEnvVarSet {
require.NoError(t, os.Unsetenv("PINNIPED_UPSTREAM_IDENTITY_PROVIDER_FLOW"))
t.Cleanup(func() {
require.NoError(t, os.Setenv("PINNIPED_UPSTREAM_IDENTITY_PROVIDER_FLOW", originalFlowEnvVarValue))
})
}
topSetupCtx, cancelFunc := context.WithTimeout(context.Background(), 5*time.Minute)
defer cancelFunc()
@ -1015,6 +1024,66 @@ func TestE2EFullIntegration_Browser(t *testing.T) {
expectedGroups,
)
})
// Add an LDAP upstream IDP and try using it to authenticate during kubectl commands, using the env var to choose the browser flow.
t.Run("with Supervisor LDAP upstream IDP and browser flow selected by env var override with with form_post automatic authcode delivery to CLI", func(t *testing.T) {
testCtx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
t.Cleanup(cancel)
tempDir := testutil.TempDir(t) // per-test tmp dir to avoid sharing files between tests
// Start a fresh browser driver because we don't want to share cookies between the various tests in this file.
page := browsertest.Open(t)
expectedUsername := env.SupervisorUpstreamLDAP.TestUserMailAttributeValue
expectedGroups := env.SupervisorUpstreamLDAP.TestUserDirectGroupsDNs
setupClusterForEndToEndLDAPTest(t, expectedUsername, env)
// Use a specific session cache for this test.
sessionCachePath := tempDir + "/test-sessions.yaml"
kubeconfigPath := runPinnipedGetKubeconfig(t, env, pinnipedExe, tempDir, []string{
"get", "kubeconfig",
"--concierge-api-group-suffix", env.APIGroupSuffix,
"--concierge-authenticator-type", "jwt",
"--concierge-authenticator-name", authenticator.Name,
"--oidc-skip-browser",
"--oidc-ca-bundle", testCABundlePath,
"--upstream-identity-provider-flow", "cli_password", // put cli_password in the kubeconfig, so we can override it with the env var
"--oidc-session-cache", sessionCachePath,
})
// Override the --upstream-identity-provider-flow flag from the kubeconfig using the env var.
require.NoError(t, os.Setenv("PINNIPED_UPSTREAM_IDENTITY_PROVIDER_FLOW", "browser_authcode"))
t.Cleanup(func() {
require.NoError(t, os.Unsetenv("PINNIPED_UPSTREAM_IDENTITY_PROVIDER_FLOW"))
})
// Run "kubectl get namespaces" which should trigger a browser login via the plugin.
kubectlCmd := exec.CommandContext(testCtx, "kubectl", "get", "namespace", "--kubeconfig", kubeconfigPath, "-v", "6")
kubectlCmd.Env = append(os.Environ(), env.ProxyEnv()...)
// Run the kubectl command, wait for the Pinniped CLI to print the authorization URL, and open it in the browser.
kubectlOutputChan := startKubectlAndOpenAuthorizationURLInBrowser(testCtx, t, kubectlCmd, page)
// Confirm that we got to the Supervisor's login page, fill out the form, and submit the form.
browsertest.LoginToUpstreamLDAP(t, page, downstream.Spec.Issuer,
expectedUsername, env.SupervisorUpstreamLDAP.TestUserPassword)
formpostExpectSuccessState(t, page)
requireKubectlGetNamespaceOutput(t, env, waitForKubectlOutput(t, kubectlOutputChan))
requireUserCanUseKubectlWithoutAuthenticatingAgain(testCtx, t, env,
downstream,
kubeconfigPath,
sessionCachePath,
pinnipedExe,
expectedUsername,
expectedGroups,
)
})
}
func startKubectlAndOpenAuthorizationURLInBrowser(testCtx context.Context, t *testing.T, kubectlCmd *exec.Cmd, page *agouti.Page) chan string {