Merge branch 'main' of github.com:vmware-tanzu/pinniped into active-directory-identity-provider

cmd/pinniped/cmd/kubeconfig.go

@@ -32,6 +32,7 @@ import (
 
 	conciergev1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/authentication/v1alpha1"
 	configv1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/config/v1alpha1"
+	idpdiscoveryv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idpdiscovery/v1alpha1"
 	conciergeclientset "go.pinniped.dev/generated/latest/client/concierge/clientset/versioned"
 	"go.pinniped.dev/internal/groupsuffix"
 )
@@ -68,6 +69,7 @@ type getKubeconfigOIDCParams struct {
 	requestAudience   string
 	upstreamIDPName   string
 	upstreamIDPType   string
+	upstreamIDPFlow   string
 }
 
 type getKubeconfigConciergeParams struct {
@@ -97,23 +99,6 @@ type getKubeconfigParams struct {
 	credentialCachePathSet    bool
 }
 
-type supervisorOIDCDiscoveryResponseWithV1Alpha1 struct {
-	SupervisorDiscovery SupervisorDiscoveryResponseV1Alpha1 `json:"discovery.supervisor.pinniped.dev/v1alpha1"`
-}
-
-type SupervisorDiscoveryResponseV1Alpha1 struct {
-	PinnipedIDPsEndpoint string `json:"pinniped_identity_providers_endpoint"`
-}
-
-type supervisorIDPsDiscoveryResponseV1Alpha1 struct {
-	PinnipedIDPs []pinnipedIDPResponse `json:"pinniped_identity_providers"`
-}
-
-type pinnipedIDPResponse struct {
-	Name string `json:"name"`
-	Type string `json:"type"`
-}
-
 func kubeconfigCommand(deps kubeconfigDeps) *cobra.Command {
 	var (
 		cmd = &cobra.Command{
@@ -153,7 +138,8 @@ func kubeconfigCommand(deps kubeconfigDeps) *cobra.Command {
 	f.BoolVar(&flags.oidc.debugSessionCache, "oidc-debug-session-cache", false, "Print debug logs related to the OpenID Connect session cache")
 	f.StringVar(&flags.oidc.requestAudience, "oidc-request-audience", "", "Request a token with an alternate audience using RFC8693 token exchange")
 	f.StringVar(&flags.oidc.upstreamIDPName, "upstream-identity-provider-name", "", "The name of the upstream identity provider used during login with a Supervisor")
-	f.StringVar(&flags.oidc.upstreamIDPType, "upstream-identity-provider-type", "", "The type of the upstream identity provider used during login with a Supervisor (e.g. 'oidc', 'ldap', 'activedirectory')")
+	f.StringVar(&flags.oidc.upstreamIDPType, "upstream-identity-provider-type", "", fmt.Sprintf("The type of the upstream identity provider used during login with a Supervisor (e.g. '%s', '%s', '%s')", idpdiscoveryv1alpha1.IDPTypeOIDC, idpdiscoveryv1alpha1.IDPTypeLDAP, idpdiscoveryv1alpha1.IDPTypeActiveDirectory))
+	f.StringVar(&flags.oidc.upstreamIDPFlow, "upstream-identity-provider-flow", "", fmt.Sprintf("The type of client flow to use with the upstream identity provider during login with a Supervisor (e.g. '%s', '%s')", idpdiscoveryv1alpha1.IDPFlowCLIPassword, idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode))
 	f.StringVar(&flags.kubeconfigPath, "kubeconfig", os.Getenv("KUBECONFIG"), "Path to kubeconfig file")
 	f.StringVar(&flags.kubeconfigContextOverride, "kubeconfig-context", "", "Kubeconfig context name (default: current active context)")
 	f.BoolVar(&flags.skipValidate, "skip-validation", false, "Skip final validation of the kubeconfig (default: false)")
@@ -243,8 +229,10 @@ func runGetKubeconfig(ctx context.Context, out io.Writer, deps kubeconfigDeps, f
 		cluster.CertificateAuthorityData = flags.concierge.caBundle
 	}
 
-	// If there is an issuer, and if both upstream flags are not already set, then try to discover Supervisor upstream IDP.
-	if len(flags.oidc.issuer) > 0 && (flags.oidc.upstreamIDPType == "" || flags.oidc.upstreamIDPName == "") {
+	// If there is an issuer, and if any upstream IDP flags are not already set, then try to discover Supervisor upstream IDP details.
+	// 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 {
 			return err
 		}
@@ -346,6 +334,9 @@ func newExecConfig(deps kubeconfigDeps, flags getKubeconfigParams) (*clientcmdap
 	if flags.oidc.upstreamIDPType != "" {
 		execConfig.Args = append(execConfig.Args, "--upstream-identity-provider-type="+flags.oidc.upstreamIDPType)
 	}
+	if flags.oidc.upstreamIDPFlow != "" {
+		execConfig.Args = append(execConfig.Args, "--upstream-identity-provider-flow="+flags.oidc.upstreamIDPFlow)
+	}
 
 	return execConfig, nil
 }
@@ -758,21 +749,31 @@ func discoverSupervisorUpstreamIDP(ctx context.Context, flags *getKubeconfigPara
 		return nil
 	}
 
-	upstreamIDPs, err := discoverAllAvailableSupervisorUpstreamIDPs(ctx, pinnipedIDPsEndpoint, httpClient)
+	discoveredUpstreamIDPs, err := discoverAllAvailableSupervisorUpstreamIDPs(ctx, pinnipedIDPsEndpoint, httpClient)
 	if err != nil {
 		return err
 	}
-	if len(upstreamIDPs) == 1 {
-		flags.oidc.upstreamIDPName = upstreamIDPs[0].Name
-		flags.oidc.upstreamIDPType = upstreamIDPs[0].Type
-	} else if len(upstreamIDPs) > 1 {
-		idpName, idpType, err := selectUpstreamIDP(upstreamIDPs, flags.oidc.upstreamIDPName, flags.oidc.upstreamIDPType)
-		if err != nil {
-			return err
-		}
-		flags.oidc.upstreamIDPName = idpName
-		flags.oidc.upstreamIDPType = idpType
+
+	if len(discoveredUpstreamIDPs) == 0 {
+		// Discovered that the Supervisor does not have any upstream IDPs defined. Continue without putting one into the
+		// kubeconfig. This kubeconfig will only work if the user defines one (and only one) OIDC IDP in the Supervisor
+		// later and wants to use the default client flow for OIDC (browser-based auth).
+		return nil
 	}
+
+	selectedIDPName, selectedIDPType, discoveredIDPFlows, err := selectUpstreamIDPNameAndType(discoveredUpstreamIDPs, flags.oidc.upstreamIDPName, flags.oidc.upstreamIDPType)
+	if err != nil {
+		return err
+	}
+
+	selectedIDPFlow, err := selectUpstreamIDPFlow(discoveredIDPFlows, selectedIDPName, selectedIDPType, flags.oidc.upstreamIDPFlow)
+	if err != nil {
+		return err
+	}
+
+	flags.oidc.upstreamIDPName = selectedIDPName
+	flags.oidc.upstreamIDPType = selectedIDPType.String()
+	flags.oidc.upstreamIDPFlow = selectedIDPFlow.String()
 	return nil
 }
 
@@ -800,7 +801,7 @@ func discoverIDPsDiscoveryEndpointURL(ctx context.Context, issuer string, httpCl
 		return "", fmt.Errorf("while fetching OIDC discovery data from issuer: %w", err)
 	}
 
-	var body supervisorOIDCDiscoveryResponseWithV1Alpha1
+	var body idpdiscoveryv1alpha1.OIDCDiscoveryResponse
 	err = discoveredProvider.Claims(&body)
 	if err != nil {
 		return "", fmt.Errorf("while fetching OIDC discovery data from issuer: %w", err)
@@ -809,7 +810,7 @@ func discoverIDPsDiscoveryEndpointURL(ctx context.Context, issuer string, httpCl
 	return body.SupervisorDiscovery.PinnipedIDPsEndpoint, nil
 }
 
-func discoverAllAvailableSupervisorUpstreamIDPs(ctx context.Context, pinnipedIDPsEndpoint string, httpClient *http.Client) ([]pinnipedIDPResponse, error) {
+func discoverAllAvailableSupervisorUpstreamIDPs(ctx context.Context, pinnipedIDPsEndpoint string, httpClient *http.Client) ([]idpdiscoveryv1alpha1.PinnipedIDP, error) {
 	request, err := http.NewRequestWithContext(ctx, http.MethodGet, pinnipedIDPsEndpoint, nil)
 	if err != nil {
 		return nil, fmt.Errorf("while forming request to IDP discovery URL: %w", err)
@@ -831,7 +832,7 @@ func discoverAllAvailableSupervisorUpstreamIDPs(ctx context.Context, pinnipedIDP
 		return nil, fmt.Errorf("unable to fetch IDP discovery data from issuer: could not read response body: %w", err)
 	}
 
-	var body supervisorIDPsDiscoveryResponseV1Alpha1
+	var body idpdiscoveryv1alpha1.IDPDiscoveryResponse
 	err = json.Unmarshal(rawBody, &body)
 	if err != nil {
 		return nil, fmt.Errorf("unable to fetch IDP discovery data from issuer: could not parse response JSON: %w", err)
@@ -840,53 +841,106 @@ func discoverAllAvailableSupervisorUpstreamIDPs(ctx context.Context, pinnipedIDP
 	return body.PinnipedIDPs, nil
 }
 
-func selectUpstreamIDP(pinnipedIDPs []pinnipedIDPResponse, idpName, idpType string) (string, string, error) {
+func selectUpstreamIDPNameAndType(pinnipedIDPs []idpdiscoveryv1alpha1.PinnipedIDP, specifiedIDPName, specifiedIDPType string) (string, idpdiscoveryv1alpha1.IDPType, []idpdiscoveryv1alpha1.IDPFlow, error) {
 	pinnipedIDPsString, _ := json.Marshal(pinnipedIDPs)
+	var discoveredFlows []idpdiscoveryv1alpha1.IDPFlow
 	switch {
-	case idpType != "":
-		discoveredName := ""
+	case specifiedIDPName != "" && specifiedIDPType != "":
+		// The user specified both name and type, so check to see if there exists an exact match.
 		for _, idp := range pinnipedIDPs {
-			if idp.Type == idpType {
+			if idp.Name == specifiedIDPName && idp.Type.Equals(specifiedIDPType) {
+				return specifiedIDPName, idp.Type, idp.Flows, nil
+			}
+		}
+		return "", "", nil, fmt.Errorf(
+			"no Supervisor upstream identity providers with name %q of type %q were found. "+
+				"Found these upstreams: %s", specifiedIDPName, specifiedIDPType, pinnipedIDPsString)
+	case specifiedIDPType != "":
+		// The user specified only a type, so check if there is only one of that type found.
+		discoveredName := ""
+		var discoveredType idpdiscoveryv1alpha1.IDPType
+		for _, idp := range pinnipedIDPs {
+			if idp.Type.Equals(specifiedIDPType) {
 				if discoveredName != "" {
-					return "", "", fmt.Errorf(
-						"multiple Supervisor upstream identity providers of type \"%s\" were found,"+
-							" so the --upstream-identity-provider-name flag must be specified. "+
+					return "", "", nil, fmt.Errorf(
+						"multiple Supervisor upstream identity providers of type %q were found, "+
+							"so the --upstream-identity-provider-name flag must be specified. "+
 							"Found these upstreams: %s",
-						idpType, pinnipedIDPsString)
+						specifiedIDPType, pinnipedIDPsString)
 				}
 				discoveredName = idp.Name
+				discoveredType = idp.Type
+				discoveredFlows = idp.Flows
 			}
 		}
 		if discoveredName == "" {
-			return "", "", fmt.Errorf(
-				"no Supervisor upstream identity providers of type \"%s\" were found."+
-					" Found these upstreams: %s", idpType, pinnipedIDPsString)
+			return "", "", nil, fmt.Errorf(
+				"no Supervisor upstream identity providers of type %q were found. "+
+					"Found these upstreams: %s", specifiedIDPType, pinnipedIDPsString)
 		}
-		return discoveredName, idpType, nil
-	case idpName != "":
-		discoveredType := ""
+		return discoveredName, discoveredType, discoveredFlows, nil
+	case specifiedIDPName != "":
+		// The user specified only a name, so check if there is only one of that name found.
+		var discoveredType idpdiscoveryv1alpha1.IDPType
 		for _, idp := range pinnipedIDPs {
-			if idp.Name == idpName {
+			if idp.Name == specifiedIDPName {
 				if discoveredType != "" {
-					return "", "", fmt.Errorf(
-						"multiple Supervisor upstream identity providers with name \"%s\" were found,"+
-							" so the --upstream-identity-provider-type flag must be specified. Found these upstreams: %s",
-						idpName, pinnipedIDPsString)
+					return "", "", nil, fmt.Errorf(
+						"multiple Supervisor upstream identity providers with name %q were found, "+
+							"so the --upstream-identity-provider-type flag must be specified. Found these upstreams: %s",
+						specifiedIDPName, pinnipedIDPsString)
 				}
 				discoveredType = idp.Type
+				discoveredFlows = idp.Flows
 			}
 		}
 		if discoveredType == "" {
-			return "", "", fmt.Errorf(
-				"no Supervisor upstream identity providers with name \"%s\" were found."+
-					" Found these upstreams: %s", idpName, pinnipedIDPsString)
+			return "", "", nil, fmt.Errorf(
+				"no Supervisor upstream identity providers with name %q were found. "+
+					"Found these upstreams: %s", specifiedIDPName, pinnipedIDPsString)
 		}
-		return idpName, discoveredType, nil
+		return specifiedIDPName, discoveredType, discoveredFlows, nil
+	case len(pinnipedIDPs) == 1:
+		// The user did not specify any name or type, but there is only one found, so select it.
+		return pinnipedIDPs[0].Name, pinnipedIDPs[0].Type, pinnipedIDPs[0].Flows, nil
 	default:
-		return "", "", fmt.Errorf(
-			"multiple Supervisor upstream identity providers were found,"+
-				" so the --upstream-identity-provider-name/--upstream-identity-provider-type flags must be specified."+
-				" Found these upstreams: %s",
+		// The user did not specify any name or type, and there is more than one found.
+		return "", "", nil, fmt.Errorf(
+			"multiple Supervisor upstream identity providers were found, "+
+				"so the --upstream-identity-provider-name/--upstream-identity-provider-type flags must be specified. "+
+				"Found these upstreams: %s",
 			pinnipedIDPsString)
 	}
 }
+
+func selectUpstreamIDPFlow(discoveredIDPFlows []idpdiscoveryv1alpha1.IDPFlow, selectedIDPName string, selectedIDPType idpdiscoveryv1alpha1.IDPType, specifiedFlow string) (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.
+		// If the user specified a flow on the CLI flag then use it without validation, otherwise skip flow selection
+		// and return empty string.
+		return idpdiscoveryv1alpha1.IDPFlow(specifiedFlow), nil
+	case specifiedFlow != "":
+		// The user specified a flow, so validate that it is available for the selected IDP.
+		for _, flow := range discoveredIDPFlows {
+			if flow.Equals(specifiedFlow) {
+				// Found it, so use it as specified by the user.
+				return flow, nil
+			}
+		}
+		return "", fmt.Errorf(
+			"no client flow %q for Supervisor upstream identity provider %q of type %q were found. "+
+				"Found these flows: %v",
+			specifiedFlow, selectedIDPName, selectedIDPType, discoveredIDPFlows)
+	case len(discoveredIDPFlows) == 1:
+		// The user did not specify a flow, but there is only one found, so select it.
+		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)
+	}
+}

cmd/pinniped/cmd/kubeconfig_test.go

@@ -149,6 +149,7 @@ func TestGetKubeconfig(t *testing.T) {
 				      --static-token string                      Instead of doing an OIDC-based login, specify a static token
 				      --static-token-env string                  Instead of doing an OIDC-based login, read a static token from the environment
 				      --timeout duration                         Timeout for autodiscovery and validation (default 10m0s)
+				      --upstream-identity-provider-flow string   The type of client flow to use with the upstream identity provider during login with a Supervisor (e.g. 'cli_password', 'browser_authcode')
 				      --upstream-identity-provider-name string   The name of the upstream identity provider used during login with a Supervisor
 				      --upstream-identity-provider-type string   The type of the upstream identity provider used during login with a Supervisor (e.g. 'oidc', 'ldap', 'activedirectory')
 			`)
@@ -814,7 +815,7 @@ func TestGetKubeconfig(t *testing.T) {
 			},
 		},
 		{
-			name: "when IDP discovery document contains multiple pinniped_idps and no name or type flags are given",
+			name: "when IDP discovery document contains multiple IDPs and no name or type flags are given",
 			args: func(issuerCABundle string, issuerURL string) []string {
 				return []string{
 					"--kubeconfig", "./testdata/kubeconfig.yaml",
@@ -1033,6 +1034,33 @@ func TestGetKubeconfig(t *testing.T) {
 				return `Error: while forming request to IDP discovery URL: parse "https%://illegal_url": first path segment in URL cannot contain colon` + "\n"
 			},
 		},
+		{
+			name: "supervisor upstream IDP discovery does not find matching IDP when name and type are both specified",
+			args: func(issuerCABundle string, issuerURL string) []string {
+				f := testutil.WriteStringToTempFile(t, "testca-*.pem", issuerCABundle)
+				return []string{
+					"--kubeconfig", "./testdata/kubeconfig.yaml",
+					"--skip-validation",
+					"--no-concierge",
+					"--oidc-issuer", issuerURL,
+					"--oidc-ca-bundle", f.Name(),
+					"--upstream-identity-provider-name", "does-not-exist-idp",
+					"--upstream-identity-provider-type", "ldap",
+				}
+			},
+			oidcDiscoveryResponse: happyOIDCDiscoveryResponse,
+			idpsDiscoveryResponse: here.Docf(`{
+				"pinniped_identity_providers": [
+					{"name": "some-ldap-idp", "type": "ldap"},
+					{"name": "some-other-ldap-idp", "type": "ldap"}
+				]
+			}`),
+			wantError: true,
+			wantStderr: func(issuerCABundle string, issuerURL string) string {
+				return `Error: no Supervisor upstream identity providers with name "does-not-exist-idp" of type "ldap" were found.` +
+					` Found these upstreams: [{"name":"some-ldap-idp","type":"ldap"},{"name":"some-other-ldap-idp","type":"ldap"}]` + "\n"
+			},
+		},
 		{
 			name: "supervisor upstream IDP discovery fails to resolve ambiguity when type is specified but name is not",
 			args: func(issuerCABundle string, issuerURL string) []string {
@@ -1091,7 +1119,7 @@ func TestGetKubeconfig(t *testing.T) {
 			},
 		},
 		{
-			name: "supervisor upstream IDP discovery fails to find any matching idps when type is specified but name is not",
+			name: "supervisor upstream IDP discovery fails to find any matching IDPs when type is specified but name is not",
 			args: func(issuerCABundle string, issuerURL string) []string {
 				f := testutil.WriteStringToTempFile(t, "testca-*.pem", issuerCABundle)
 				return []string{
@@ -1117,7 +1145,32 @@ func TestGetKubeconfig(t *testing.T) {
 			},
 		},
 		{
-			name: "supervisor upstream IDP discovery fails to find any matching idps when name is specified but type is not",
+			name: "supervisor upstream IDP discovery fails to find any matching IDPs when type is specified but name is not and there is only one IDP found",
+			args: func(issuerCABundle string, issuerURL string) []string {
+				f := testutil.WriteStringToTempFile(t, "testca-*.pem", issuerCABundle)
+				return []string{
+					"--kubeconfig", "./testdata/kubeconfig.yaml",
+					"--skip-validation",
+					"--no-concierge",
+					"--oidc-issuer", issuerURL,
+					"--oidc-ca-bundle", f.Name(),
+					"--upstream-identity-provider-type", "ldap",
+				}
+			},
+			oidcDiscoveryResponse: happyOIDCDiscoveryResponse,
+			idpsDiscoveryResponse: here.Docf(`{
+				"pinniped_identity_providers": [
+					{"name": "some-oidc-idp", "type": "oidc"}
+				]
+			}`),
+			wantError: true,
+			wantStderr: func(issuerCABundle string, issuerURL string) string {
+				return `Error: no Supervisor upstream identity providers of type "ldap" were found.` +
+					` Found these upstreams: [{"name":"some-oidc-idp","type":"oidc"}]` + "\n"
+			},
+		},
+		{
+			name: "supervisor upstream IDP discovery fails to find any matching IDPs when name is specified but type is not",
 			args: func(issuerCABundle string, issuerURL string) []string {
 				f := testutil.WriteStringToTempFile(t, "testca-*.pem", issuerCABundle)
 				return []string{
@@ -1142,6 +1195,80 @@ func TestGetKubeconfig(t *testing.T) {
 					` Found these upstreams: [{"name":"some-oidc-idp","type":"oidc"},{"name":"some-other-oidc-idp","type":"oidc"}]` + "\n"
 			},
 		},
+		{
+			name: "supervisor upstream IDP discovery fails to find any matching IDPs when name is specified but type is not and there is only one IDP found",
+			args: func(issuerCABundle string, issuerURL string) []string {
+				f := testutil.WriteStringToTempFile(t, "testca-*.pem", issuerCABundle)
+				return []string{
+					"--kubeconfig", "./testdata/kubeconfig.yaml",
+					"--skip-validation",
+					"--no-concierge",
+					"--oidc-issuer", issuerURL,
+					"--oidc-ca-bundle", f.Name(),
+					"--upstream-identity-provider-name", "my-nonexistent-idp",
+				}
+			},
+			oidcDiscoveryResponse: happyOIDCDiscoveryResponse,
+			idpsDiscoveryResponse: here.Docf(`{
+				"pinniped_identity_providers": [
+					{"name": "some-oidc-idp", "type": "oidc"}
+				]
+			}`),
+			wantError: true,
+			wantStderr: func(issuerCABundle string, issuerURL string) string {
+				return `Error: no Supervisor upstream identity providers with name "my-nonexistent-idp" were found.` +
+					` Found these upstreams: [{"name":"some-oidc-idp","type":"oidc"}]` + "\n"
+			},
+		},
+		{
+			name: "supervisor upstream IDP discovery when flow is specified but it does not match any flow returned by discovery",
+			args: func(issuerCABundle string, issuerURL string) []string {
+				f := testutil.WriteStringToTempFile(t, "testca-*.pem", issuerCABundle)
+				return []string{
+					"--kubeconfig", "./testdata/kubeconfig.yaml",
+					"--skip-validation",
+					"--no-concierge",
+					"--oidc-issuer", issuerURL,
+					"--oidc-ca-bundle", f.Name(),
+					"--upstream-identity-provider-flow", "my-nonexistent-flow",
+				}
+			},
+			oidcDiscoveryResponse: happyOIDCDiscoveryResponse,
+			idpsDiscoveryResponse: here.Docf(`{
+				"pinniped_identity_providers": [
+					{"name": "some-oidc-idp", "type": "oidc", "flows": ["non-matching-flow-1", "non-matching-flow-2"]}
+				]
+			}`),
+			wantError: true,
+			wantStderr: func(issuerCABundle string, issuerURL string) string {
+				return `Error: no client flow "my-nonexistent-flow" for Supervisor upstream identity provider "some-oidc-idp" of type "oidc" were found.` +
+					` Found these flows: [non-matching-flow-1 non-matching-flow-2]` + "\n"
+			},
+		},
+		{
+			name: "supervisor upstream IDP discovery when no flow is specified and more than one flow is returned by discovery",
+			args: func(issuerCABundle string, issuerURL string) []string {
+				f := testutil.WriteStringToTempFile(t, "testca-*.pem", issuerCABundle)
+				return []string{
+					"--kubeconfig", "./testdata/kubeconfig.yaml",
+					"--skip-validation",
+					"--no-concierge",
+					"--oidc-issuer", issuerURL,
+					"--oidc-ca-bundle", f.Name(),
+				}
+			},
+			oidcDiscoveryResponse: happyOIDCDiscoveryResponse,
+			idpsDiscoveryResponse: here.Docf(`{
+				"pinniped_identity_providers": [
+					{"name": "some-oidc-idp", "type": "oidc", "flows": ["flow1", "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"
+			},
+		},
 		{
 			name: "valid static token",
 			args: func(issuerCABundle string, issuerURL string) []string {
@@ -1535,7 +1662,7 @@ func TestGetKubeconfig(t *testing.T) {
 			},
 		},
 		{
-			name: "autodetect impersonation proxy with autodiscovered JWT authenticator",
+			name: "autodetect impersonation proxy with auto-discovered JWT authenticator",
 			args: func(issuerCABundle string, issuerURL string) []string {
 				return []string{
 					"--kubeconfig", "./testdata/kubeconfig.yaml",
@@ -1958,7 +2085,7 @@ func TestGetKubeconfig(t *testing.T) {
 					}
 				}`, issuerURL)
 			},
-			idpsDiscoveryStatusCode: http.StatusBadRequest, // IDPs endpoint shouldn't be called by this test
+			idpsDiscoveryStatusCode: http.StatusBadRequest, // IDP discovery endpoint shouldn't be called by this test
 			wantLogs: func(issuerCABundle string, issuerURL string) []string {
 				return []string{
 					`"level"=0 "msg"="discovered CredentialIssuer"  "name"="test-credential-issuer"`,
@@ -2015,13 +2142,14 @@ func TestGetKubeconfig(t *testing.T) {
 			},
 		},
 		{
-			name: "when upstream idp related flags are sent, pass them through",
+			name: "when all upstream IDP related flags are sent, pass them through without performing IDP discovery",
 			args: func(issuerCABundle string, issuerURL string) []string {
 				return []string{
 					"--kubeconfig", "./testdata/kubeconfig.yaml",
 					"--skip-validation",
 					"--upstream-identity-provider-name=some-oidc-idp",
 					"--upstream-identity-provider-type=oidc",
+					"--upstream-identity-provider-flow=foobar",
 				}
 			},
 			conciergeObjects: func(issuerCABundle string, issuerURL string) []runtime.Object {
@@ -2030,7 +2158,7 @@ func TestGetKubeconfig(t *testing.T) {
 					jwtAuthenticator(issuerCABundle, issuerURL),
 				}
 			},
-			oidcDiscoveryStatusCode: http.StatusNotFound,
+			oidcDiscoveryStatusCode: http.StatusNotFound, // should not get called by the client in this case
 			wantLogs: func(issuerCABundle string, issuerURL string) []string {
 				return []string{
 					`"level"=0 "msg"="discovered CredentialIssuer"  "name"="test-credential-issuer"`,
@@ -2080,6 +2208,7 @@ func TestGetKubeconfig(t *testing.T) {
 						  - --request-audience=test-audience
 						  - --upstream-identity-provider-name=some-oidc-idp
 						  - --upstream-identity-provider-type=oidc
+						  - --upstream-identity-provider-flow=foobar
 						  command: '.../path/to/pinniped'
 						  env: []
 						  provideClusterInfo: true
@@ -2089,13 +2218,14 @@ func TestGetKubeconfig(t *testing.T) {
 			},
 		},
 		{
-			name: "when upstream IDP related flags are sent, pass them through even when IDP discovery shows a different IDP",
+			name: "when all upstream IDP related flags are sent, pass them through even when IDP discovery shows a different IDP",
 			args: func(issuerCABundle string, issuerURL string) []string {
 				return []string{
 					"--kubeconfig", "./testdata/kubeconfig.yaml",
 					"--skip-validation",
 					"--upstream-identity-provider-name=some-oidc-idp",
 					"--upstream-identity-provider-type=oidc",
+					"--upstream-identity-provider-flow=foobar",
 				}
 			},
 			conciergeObjects: func(issuerCABundle string, issuerURL string) []runtime.Object {
@@ -2159,6 +2289,7 @@ func TestGetKubeconfig(t *testing.T) {
 						  - --request-audience=test-audience
 						  - --upstream-identity-provider-name=some-oidc-idp
 						  - --upstream-identity-provider-type=oidc
+						  - --upstream-identity-provider-flow=foobar
 						  command: '.../path/to/pinniped'
 						  env: []
 						  provideClusterInfo: true
@@ -2340,6 +2471,244 @@ func TestGetKubeconfig(t *testing.T) {
 					issuerURL,
 					base64.StdEncoding.EncodeToString([]byte(issuerCABundle)))
 			},
+		}, // TODO make sure there are active directory tests for various flows
+		{
+			name: "supervisor upstream IDP discovery when both name and type are specified but flow is not and a matching IDP is found",
+			args: func(issuerCABundle string, issuerURL string) []string {
+				f := testutil.WriteStringToTempFile(t, "testca-*.pem", issuerCABundle)
+				return []string{
+					"--kubeconfig", "./testdata/kubeconfig.yaml",
+					"--skip-validation",
+					"--no-concierge",
+					"--oidc-issuer", issuerURL,
+					"--oidc-ca-bundle", f.Name(),
+					"--upstream-identity-provider-name", "some-ldap-idp",
+					"--upstream-identity-provider-type", "ldap",
+				}
+			},
+			oidcDiscoveryResponse: happyOIDCDiscoveryResponse,
+			idpsDiscoveryResponse: here.Docf(`{
+				"pinniped_identity_providers": [
+					{"name": "some-ldap-idp", "type": "ldap"},
+					{"name": "some-oidc-idp", "type": "oidc"},
+					{"name": "some-other-oidc-idp", "type": "oidc"}
+				]
+			}`),
+			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
+						  command: '.../path/to/pinniped'
+						  env: []
+						  provideClusterInfo: true
+					`,
+					issuerURL,
+					base64.StdEncoding.EncodeToString([]byte(issuerCABundle)))
+			},
+		},
+		{
+			name: "supervisor upstream IDP discovery when flow is specified and no flows were returned by discovery uses the specified flow",
+			args: func(issuerCABundle string, issuerURL string) []string {
+				f := testutil.WriteStringToTempFile(t, "testca-*.pem", issuerCABundle)
+				return []string{
+					"--kubeconfig", "./testdata/kubeconfig.yaml",
+					"--skip-validation",
+					"--no-concierge",
+					"--oidc-issuer", issuerURL,
+					"--oidc-ca-bundle", f.Name(),
+					"--upstream-identity-provider-flow", "foobar",
+					"--upstream-identity-provider-type", "ldap",
+				}
+			},
+			oidcDiscoveryResponse: happyOIDCDiscoveryResponse,
+			idpsDiscoveryResponse: here.Docf(`{
+				"pinniped_identity_providers": [
+					{"name": "some-ldap-idp", "type": "ldap"},
+					{"name": "some-oidc-idp", "type": "oidc"},
+					{"name": "some-other-oidc-idp", "type": "oidc"}
+				]
+			}`),
+			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=foobar
+						  command: '.../path/to/pinniped'
+						  env: []
+						  provideClusterInfo: true
+					`,
+					issuerURL,
+					base64.StdEncoding.EncodeToString([]byte(issuerCABundle)))
+			},
+		},
+		{
+			name: "supervisor upstream IDP discovery when flow is specified and it matches a flow returned by discovery uses the specified flow",
+			args: func(issuerCABundle string, issuerURL string) []string {
+				f := testutil.WriteStringToTempFile(t, "testca-*.pem", issuerCABundle)
+				return []string{
+					"--kubeconfig", "./testdata/kubeconfig.yaml",
+					"--skip-validation",
+					"--no-concierge",
+					"--oidc-issuer", issuerURL,
+					"--oidc-ca-bundle", f.Name(),
+					"--upstream-identity-provider-flow", "cli_password",
+					"--upstream-identity-provider-type", "ldap",
+				}
+			},
+			oidcDiscoveryResponse: happyOIDCDiscoveryResponse,
+			idpsDiscoveryResponse: here.Docf(`{
+				"pinniped_identity_providers": [
+					{"name": "some-ldap-idp", "type": "ldap", "flows": ["some_flow", "cli_password", "some_other_flow"]}
+				]
+			}`),
+			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: []
+						  provideClusterInfo: true
+					`,
+					issuerURL,
+					base64.StdEncoding.EncodeToString([]byte(issuerCABundle)))
+			},
+		},
+		{
+			name: "supervisor upstream IDP discovery when no flow is specified but there is only one flow returned by discovery uses the discovered flow",
+			args: func(issuerCABundle string, issuerURL string) []string {
+				f := testutil.WriteStringToTempFile(t, "testca-*.pem", issuerCABundle)
+				return []string{
+					"--kubeconfig", "./testdata/kubeconfig.yaml",
+					"--skip-validation",
+					"--no-concierge",
+					"--oidc-issuer", issuerURL,
+					"--oidc-ca-bundle", f.Name(),
+					"--upstream-identity-provider-type", "ldap",
+				}
+			},
+			oidcDiscoveryResponse: happyOIDCDiscoveryResponse,
+			idpsDiscoveryResponse: here.Docf(`{
+				"pinniped_identity_providers": [
+					{"name": "some-ldap-idp", "type": "ldap", "flows": ["cli_password"]}
+				]
+			}`),
+			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: []
+						  provideClusterInfo: true
+					`,
+					issuerURL,
+					base64.StdEncoding.EncodeToString([]byte(issuerCABundle)))
+			},
 		},
 	}
 	for _, tt := range tests {

cmd/pinniped/cmd/login_oidc.go

@@ -14,6 +14,7 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
+	"strings"
 	"time"
 
 	"github.com/coreos/go-oidc/v3/oidc"
@@ -23,6 +24,7 @@ import (
 	"k8s.io/client-go/transport"
 	"k8s.io/klog/v2/klogr"
 
+	idpdiscoveryv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idpdiscovery/v1alpha1"
 	"go.pinniped.dev/internal/execcredcache"
 	"go.pinniped.dev/internal/groupsuffix"
 	"go.pinniped.dev/internal/plog"
@@ -74,6 +76,7 @@ type oidcLoginFlags struct {
 	credentialCachePath          string
 	upstreamIdentityProviderName string
 	upstreamIdentityProviderType string
+	upstreamIdentityProviderFlow string
 }
 
 func oidcLoginCommand(deps oidcLoginCommandDeps) *cobra.Command {
@@ -107,7 +110,8 @@ func oidcLoginCommand(deps oidcLoginCommandDeps) *cobra.Command {
 	cmd.Flags().StringVar(&flags.conciergeAPIGroupSuffix, "concierge-api-group-suffix", groupsuffix.PinnipedDefaultSuffix, "Concierge API group suffix")
 	cmd.Flags().StringVar(&flags.credentialCachePath, "credential-cache", filepath.Join(mustGetConfigDir(), "credentials.yaml"), "Path to cluster-specific credentials cache (\"\" disables the cache)")
 	cmd.Flags().StringVar(&flags.upstreamIdentityProviderName, "upstream-identity-provider-name", "", "The name of the upstream identity provider used during login with a Supervisor")
-	cmd.Flags().StringVar(&flags.upstreamIdentityProviderType, "upstream-identity-provider-type", "oidc", "The type of the upstream identity provider used during login with a Supervisor (e.g. 'oidc', 'ldap', 'activedirectory')")
+	cmd.Flags().StringVar(&flags.upstreamIdentityProviderType, "upstream-identity-provider-type", idpdiscoveryv1alpha1.IDPTypeOIDC.String(), fmt.Sprintf("The type of the upstream identity provider used during login with a Supervisor (e.g. '%s', '%s', '%s')", idpdiscoveryv1alpha1.IDPTypeOIDC, idpdiscoveryv1alpha1.IDPTypeLDAP, idpdiscoveryv1alpha1.IDPTypeActiveDirectory))
+	cmd.Flags().StringVar(&flags.upstreamIdentityProviderFlow, "upstream-identity-provider-flow", "", fmt.Sprintf("The type of client flow to use with the upstream identity provider during login with a Supervisor (e.g. '%s', '%s')", idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode, idpdiscoveryv1alpha1.IDPFlowCLIPassword))
 
 	// --skip-listen is mainly needed for testing. We'll leave it hidden until we have a non-testing use case.
 	mustMarkHidden(cmd, "skip-listen")
@@ -160,19 +164,14 @@ func runOIDCLogin(cmd *cobra.Command, deps oidcLoginCommandDeps, flags oidcLogin
 			flags.upstreamIdentityProviderName, flags.upstreamIdentityProviderType))
 	}
 
-	switch flags.upstreamIdentityProviderType {
-	case "oidc":
-		// this is the default, so don't need to do anything
-	case "ldap":
-		opts = append(opts, oidcclient.WithCLISendingCredentials())
-	case "activedirectory":
-		opts = append(opts, oidcclient.WithCLISendingCredentials())
-	default:
-		// Surprisingly cobra does not support this kind of flag validation. See https://github.com/spf13/pflag/issues/236
-		return fmt.Errorf(
-			"--upstream-identity-provider-type value not recognized: %s (supported values: oidc, ldap, activedirectory)",
-			flags.upstreamIdentityProviderType)
+	flowOpts, err := flowOptions(
+		idpdiscoveryv1alpha1.IDPType(flags.upstreamIdentityProviderType),
+		idpdiscoveryv1alpha1.IDPFlow(flags.upstreamIdentityProviderFlow),
+	)
+	if err != nil {
+		return err
 	}
+	opts = append(opts, flowOpts...)
 
 	var concierge *conciergeclient.Client
 	if flags.conciergeEnabled {
@@ -253,6 +252,46 @@ 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) {
+	useCLIFlow := []oidcclient.Option{oidcclient.WithCLISendingCredentials()}
+
+	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(
+				"--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()}, ", "))
+		}
+	case idpdiscoveryv1alpha1.IDPTypeLDAP, idpdiscoveryv1alpha1.IDPTypeActiveDirectory:
+		switch requestedFlow {
+		case idpdiscoveryv1alpha1.IDPFlowCLIPassword, "":
+			return useCLIFlow, nil
+		case idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode:
+			fallthrough // not supported for LDAP providers, so fallthrough to error case
+		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()})
+		}
+	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 {

cmd/pinniped/cmd/login_oidc_test.go

@@ -77,6 +77,7 @@ func TestLoginOIDCCommand(t *testing.T) {
 				      --scopes strings                           OIDC scopes to request during login (default [offline_access,openid,pinniped:request-audience])
 				      --session-cache string                     Path to session cache file (default "` + cfgDir + `/sessions.yaml")
 				      --skip-browser                             Skip opening the browser (just print the URL)
+					  --upstream-identity-provider-flow string   The type of client flow to use with the upstream identity provider during login with a Supervisor (e.g. 'browser_authcode', 'cli_password')
 					  --upstream-identity-provider-name string   The name of the upstream identity provider used during login with a Supervisor
 					  --upstream-identity-provider-type string   The type of the upstream identity provider used during login with a Supervisor (e.g. 'oidc', 'ldap', 'activedirectory') (default "oidc")
 			`),
@@ -152,7 +153,7 @@ func TestLoginOIDCCommand(t *testing.T) {
 			`),
 		},
 		{
-			name: "oidc upstream type is allowed",
+			name: "oidc upstream type with default flow is allowed",
 			args: []string{
 				"--issuer", "test-issuer",
 				"--client-id", "test-client-id",
@@ -163,7 +164,45 @@ 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: "ldap upstream type is allowed",
+			name: "oidc upstream type with CLI flow 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
+			},
+			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 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: "ldap upstream type with default flow is allowed",
 			args: []string{
 				"--issuer", "test-issuer",
 				"--client-id", "test-client-id",
@@ -174,7 +213,7 @@ 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: "activedirectory upstream type is allowed",
+			name: "activedirectory upstream type with default flow is allowed",
 			args: []string{
 				"--issuer", "test-issuer",
 				"--client-id", "test-client-id",
@@ -184,6 +223,58 @@ 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 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 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
+				"--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])
+			`),
+		},
+		{
+			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 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
+				"--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])
+			`),
+		},
 		{
 			name: "login error",
 			args: []string{