Support a browser-based login flow for LDAP and Active Directory providers

cmd/pinniped/cmd/kubeconfig.go

@@ -233,7 +233,7 @@ func runGetKubeconfig(ctx context.Context, out io.Writer, deps kubeconfigDeps, f
 	// When all the upstream IDP flags are set by the user, then skip discovery and don't validate their input. Maybe they know something
 	// that we can't know, like the name of an IDP that they are going to define in the future.
 	if len(flags.oidc.issuer) > 0 && (flags.oidc.upstreamIDPType == "" || flags.oidc.upstreamIDPName == "" || flags.oidc.upstreamIDPFlow == "") {
-		if err := discoverSupervisorUpstreamIDP(ctx, &flags); err != nil {
+		if err := discoverSupervisorUpstreamIDP(ctx, &flags, deps.log); err != nil {
 			return err
 		}
 	}
@@ -726,7 +726,7 @@ func hasPendingStrategy(credentialIssuer *configv1alpha1.CredentialIssuer) bool
 	return false
 }
 
-func discoverSupervisorUpstreamIDP(ctx context.Context, flags *getKubeconfigParams) error {
+func discoverSupervisorUpstreamIDP(ctx context.Context, flags *getKubeconfigParams, log logr.Logger) error {
 	httpClient, err := newDiscoveryHTTPClient(flags.oidc.caBundle)
 	if err != nil {
 		return err
@@ -758,7 +758,7 @@ func discoverSupervisorUpstreamIDP(ctx context.Context, flags *getKubeconfigPara
 		return err
 	}
 
-	selectedIDPFlow, err := selectUpstreamIDPFlow(discoveredIDPFlows, selectedIDPName, selectedIDPType, flags.oidc.upstreamIDPFlow)
+	selectedIDPFlow, err := selectUpstreamIDPFlow(discoveredIDPFlows, selectedIDPName, selectedIDPType, flags.oidc.upstreamIDPFlow, log)
 	if err != nil {
 		return err
 	}
@@ -898,7 +898,7 @@ func selectUpstreamIDPNameAndType(pinnipedIDPs []idpdiscoveryv1alpha1.PinnipedID
 	}
 }
 
-func selectUpstreamIDPFlow(discoveredIDPFlows []idpdiscoveryv1alpha1.IDPFlow, selectedIDPName string, selectedIDPType idpdiscoveryv1alpha1.IDPType, specifiedFlow string) (idpdiscoveryv1alpha1.IDPFlow, error) {
+func selectUpstreamIDPFlow(discoveredIDPFlows []idpdiscoveryv1alpha1.IDPFlow, selectedIDPName string, selectedIDPType idpdiscoveryv1alpha1.IDPType, specifiedFlow string, log logr.Logger) (idpdiscoveryv1alpha1.IDPFlow, error) {
 	switch {
 	case len(discoveredIDPFlows) == 0:
 		// No flows listed by discovery means that we are talking to an old Supervisor from before this feature existed.
@@ -922,10 +922,9 @@ func selectUpstreamIDPFlow(discoveredIDPFlows []idpdiscoveryv1alpha1.IDPFlow, se
 		return discoveredIDPFlows[0], nil
 	default:
 		// The user did not specify a flow, and more than one was found.
-		return "", fmt.Errorf(
-			"multiple client flows for Supervisor upstream identity provider %q of type %q were found, "+
-				"so the --upstream-identity-provider-flow flag must be specified. "+
-				"Found these flows: %v",
-			selectedIDPName, selectedIDPType, discoveredIDPFlows)
+		log.Info("multiple client flows found, selecting first value as default",
+			"idpName", selectedIDPName, "idpType", selectedIDPType,
+			"selectedFlow", discoveredIDPFlows[0].String(), "availableFlows", discoveredIDPFlows)
+		return discoveredIDPFlows[0], nil
 	}
 }

cmd/pinniped/cmd/kubeconfig_test.go

@@ -1261,13 +1261,52 @@ func TestGetKubeconfig(t *testing.T) {
 			oidcDiscoveryResponse: happyOIDCDiscoveryResponse,
 			idpsDiscoveryResponse: here.Docf(`{
 				"pinniped_identity_providers": [
-					{"name": "some-oidc-idp", "type": "oidc", "flows": ["flow1", "flow2"]}
+					{"name": "some-ldap-idp", "type": "ldap", "flows": ["cli_password", "flow2"]}
 				]
 			}`),
-			wantError: true,
-			wantStderr: func(issuerCABundle string, issuerURL string) string {
-				return `Error: multiple client flows for Supervisor upstream identity provider "some-oidc-idp" of type "oidc" were found, so the --upstream-identity-provider-flow flag must be specified.` +
-					` Found these flows: [flow1 flow2]` + "\n"
+			wantStdout: func(issuerCABundle string, issuerURL string) string {
+				return here.Docf(`
+					apiVersion: v1
+					clusters:
+					- cluster:
+						certificate-authority-data: ZmFrZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YS12YWx1ZQ==
+						server: https://fake-server-url-value
+					  name: kind-cluster-pinniped
+					contexts:
+					- context:
+						cluster: kind-cluster-pinniped
+						user: kind-user-pinniped
+					  name: kind-context-pinniped
+					current-context: kind-context-pinniped
+					kind: Config
+					preferences: {}
+					users:
+					- name: kind-user-pinniped
+					  user:
+						exec:
+						  apiVersion: client.authentication.k8s.io/v1beta1
+						  args:
+						  - login
+						  - oidc
+						  - --issuer=%s
+						  - --client-id=pinniped-cli
+						  - --scopes=offline_access,openid,pinniped:request-audience
+						  - --ca-bundle-data=%s
+						  - --upstream-identity-provider-name=some-ldap-idp
+						  - --upstream-identity-provider-type=ldap
+						  - --upstream-identity-provider-flow=cli_password
+						  command: '.../path/to/pinniped'
+						  env: []
+						  installHint: The pinniped CLI does not appear to be installed.  See https://get.pinniped.dev/cli
+             for more details
+						  provideClusterInfo: true
+					`,
+					issuerURL,
+					base64.StdEncoding.EncodeToString([]byte(issuerCABundle)))
+			},
+			wantLogs: func(_ string, _ string) []string {
+				return []string{`"level"=0 "msg"="multiple client flows found, selecting first value as default"  ` +
+					`"availableFlows"=["cli_password","flow2"] "idpName"="some-ldap-idp" "idpType"="ldap" "selectedFlow"="cli_password"`}
 			},
 		},
 		{

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

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)
 			`),
 		},
 		{

hack/prepare-supervisor-on-kind.sh

@@ -25,11 +25,36 @@ set -euo pipefail
 ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
 cd "$ROOT"
 
+function log_error() {
+  RED='\033[0;31m'
+  NC='\033[0m'
+  if [[ ${COLORTERM:-unknown} =~ ^(truecolor|24bit)$ ]]; then
+    echo -e "🙁${RED} Error: $* ${NC}"
+  else
+    echo ":( Error: $*"
+  fi
+}
+
 use_oidc_upstream=no
 use_ldap_upstream=no
 use_ad_upstream=no
+use_flow=""
 while (("$#")); do
   case "$1" in
+  --flow)
+    shift
+    # If there are no more command line arguments, or there is another command line argument but it starts with a dash, then error
+    if [[ "$#" == "0" || "$1" == -* ]]; then
+      log_error "--flow requires a flow name to be specified (e.g. cli_password or browser_authcode"
+      exit 1
+    fi
+    if [[ "$1" != "browser_authcode" && "$1" != "cli_password" ]]; then
+      log_error "--flow must be cli_password or browser_authcode"
+      exit 1
+    fi
+    use_flow=$1
+    shift
+    ;;
   --ldap)
     use_ldap_upstream=yes
     shift
@@ -56,7 +81,7 @@ while (("$#")); do
 done
 
 if [[ "$use_oidc_upstream" == "no" && "$use_ldap_upstream" == "no" && "$use_ad_upstream" == "no" ]]; then
-  echo "Error: Please use --oidc, --ldap, or --ad to specify which type of upstream identity provider(s) you would like"
+  log_error "Error: Please use --oidc, --ldap, or --ad to specify which type of upstream identity provider(s) you would like"
   exit 1
 fi
 
@@ -127,6 +152,7 @@ spec:
     certificateAuthorityData: "$PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_ISSUER_CA_BUNDLE"
   authorizationConfig:
     additionalScopes: [ ${PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_ADDITIONAL_SCOPES} ]
+    allowPasswordGrant: true
   claims:
     username: "$PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_USERNAME_CLAIM"
     groups: "$PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_GROUPS_CLAIM"
@@ -196,7 +222,7 @@ EOF
 fi
 
 if [[ "$use_ad_upstream" == "yes" ]]; then
-  # Make an ActiveDirectoryIdentityProvider.
+  # Make an ActiveDirectoryIdentityProvider. Needs to be pointed to a real AD server by env vars.
   cat <<EOF | kubectl apply --namespace "$PINNIPED_TEST_SUPERVISOR_NAMESPACE" -f -
 apiVersion: idp.supervisor.pinniped.dev/v1alpha1
 kind: ActiveDirectoryIdentityProvider
@@ -256,7 +282,11 @@ while [[ -z "$(kubectl get credentialissuer pinniped-concierge-config -o=jsonpat
 done
 
 # Use the CLI to get the kubeconfig. Tell it that you don't want the browser to automatically open for logins.
-https_proxy="$PINNIPED_TEST_PROXY" no_proxy="127.0.0.1" ./pinniped get kubeconfig --oidc-skip-browser >kubeconfig
+flow_arg=""
+if [[ -n "$use_flow" ]]; then
+  flow_arg="--upstream-identity-provider-flow $use_flow"
+fi
+https_proxy="$PINNIPED_TEST_PROXY" no_proxy="127.0.0.1" ./pinniped get kubeconfig --oidc-skip-browser $flow_arg >kubeconfig
 
 # Clear the local CLI cache to ensure that the kubectl command below will need to perform a fresh login.
 rm -f "$HOME/.config/pinniped/sessions.yaml"
@@ -265,25 +295,27 @@ rm -f "$HOME/.config/pinniped/credentials.yaml"
 echo
 echo "Ready! 🚀"
 
-if [[ "$use_oidc_upstream" == "yes" ]]; then
+if [[ "$use_oidc_upstream" == "yes" || "$use_flow" == "browser_authcode" ]]; then
   echo
   echo "To be able to access the login URL shown below, start Chrome like this:"
   echo "    open -a \"Google Chrome\" --args --proxy-server=\"$PINNIPED_TEST_PROXY\""
-  echo "Then use these credentials at the Dex login page:"
+  echo "Note that Chrome must be fully quit before being started with --proxy-server."
+  echo "Then open the login URL shown below in that new Chrome window."
+  echo
+  echo "When prompted for username and password, use these values:"
+fi
+
+if [[ "$use_oidc_upstream" == "yes" ]]; then
   echo "    Username: $PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_USERNAME"
   echo "    Password: $PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_PASSWORD"
 fi
 
 if [[ "$use_ldap_upstream" == "yes" ]]; then
-  echo
-  echo "When prompted for username and password by the CLI, use these values:"
   echo "    Username: $PINNIPED_TEST_LDAP_USER_CN"
   echo "    Password: $PINNIPED_TEST_LDAP_USER_PASSWORD"
 fi
 
 if [[ "$use_ad_upstream" == "yes" ]]; then
-  echo
-  echo "When prompted for username and password by the CLI, use these values:"
   echo "    Username: $PINNIPED_TEST_AD_USER_USER_PRINCIPAL_NAME"
   echo "    Password: $PINNIPED_TEST_AD_USER_PASSWORD"
 fi