From 77f016fb64d91446aafcae57d9e6bd3db3d40ff5 Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Wed, 27 Apr 2022 08:53:53 -0700 Subject: [PATCH] Allow browser_authcode flow for pinniped login command Signed-off-by: Margo Crawford --- cmd/pinniped/cmd/login_oidc.go | 6 +++--- cmd/pinniped/cmd/login_oidc_test.go | 32 +++++++++++++++++++++++++---- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/cmd/pinniped/cmd/login_oidc.go b/cmd/pinniped/cmd/login_oidc.go index c8b2b0cc..bf35a6ba 100644 --- a/cmd/pinniped/cmd/login_oidc.go +++ b/cmd/pinniped/cmd/login_oidc.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package cmd @@ -271,11 +271,11 @@ func flowOptions(requestedIDPType idpdiscoveryv1alpha1.IDPType, requestedFlow id case idpdiscoveryv1alpha1.IDPFlowCLIPassword, "": return useCLIFlow, nil case idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode: - fallthrough // not supported for LDAP providers, so fallthrough to error case + return nil, nil default: return nil, fmt.Errorf( "--upstream-identity-provider-flow value not recognized for identity provider type %q: %s (supported values: %s)", - requestedIDPType, requestedFlow, []string{idpdiscoveryv1alpha1.IDPFlowCLIPassword.String()}) + 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 4a384c76..da0cfcb7 100644 --- a/cmd/pinniped/cmd/login_oidc_test.go +++ b/cmd/pinniped/cmd/login_oidc_test.go @@ -235,18 +235,30 @@ func TestLoginOIDCCommand(t *testing.T) { 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 unsupported flow is an error", args: []string{ "--issuer", "test-issuer", "--client-id", "test-client-id", "--upstream-identity-provider-type", "ldap", - "--upstream-identity-provider-flow", "browser_authcode", // "browser_authcode" is only supported for OIDC upstreams + "--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": browser_authcode (supported values: [cli_password]) + Error: --upstream-identity-provider-flow value not recognized for identity provider type "ldap": foo (supported values: cli_password, browser_authcode) `), }, { @@ -261,18 +273,30 @@ func TestLoginOIDCCommand(t *testing.T) { 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 unsupported flow is an error", args: []string{ "--issuer", "test-issuer", "--client-id", "test-client-id", "--upstream-identity-provider-type", "activedirectory", - "--upstream-identity-provider-flow", "browser_authcode", // "browser_authcode" is only supported for OIDC upstreams + "--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": browser_authcode (supported values: [cli_password]) + Error: --upstream-identity-provider-flow value not recognized for identity provider type "activedirectory": foo (supported values: cli_password, browser_authcode) `), }, {