diff --git a/cmd/pinniped/cmd/kubeconfig.go b/cmd/pinniped/cmd/kubeconfig.go index f10fcd43..74280ec5 100644 --- a/cmd/pinniped/cmd/kubeconfig.go +++ b/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,7 @@ 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: "+discoveredIDPFlows[0].String(), "idpName", selectedIDPName, "idpType", selectedIDPType) + return discoveredIDPFlows[0], nil } } diff --git a/cmd/pinniped/cmd/kubeconfig_test.go b/cmd/pinniped/cmd/kubeconfig_test.go index e5c760aa..e5c27797 100644 --- a/cmd/pinniped/cmd/kubeconfig_test.go +++ b/cmd/pinniped/cmd/kubeconfig_test.go @@ -1261,13 +1261,51 @@ 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: cli_password\" \"idpName\"=\"some-ldap-idp\" \"idpType\"=\"ldap\""} }, }, { diff --git a/internal/oidc/idpdiscovery/idp_discovery_handler.go b/internal/oidc/idpdiscovery/idp_discovery_handler.go index 8949502c..66a974c9 100644 --- a/internal/oidc/idpdiscovery/idp_discovery_handler.go +++ b/internal/oidc/idpdiscovery/idp_discovery_handler.go @@ -1,4 +1,4 @@ -// Copyright 2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package idpdiscovery provides a handler for the upstream IDP discovery endpoint. @@ -44,14 +44,14 @@ func responseAsJSON(upstreamIDPs oidc.UpstreamIdentityProvidersLister) ([]byte, r.PinnipedIDPs = append(r.PinnipedIDPs, v1alpha1.PinnipedIDP{ Name: provider.GetName(), Type: v1alpha1.IDPTypeLDAP, - Flows: []v1alpha1.IDPFlow{v1alpha1.IDPFlowCLIPassword}, + Flows: []v1alpha1.IDPFlow{v1alpha1.IDPFlowCLIPassword, v1alpha1.IDPFlowBrowserAuthcode}, }) } for _, provider := range upstreamIDPs.GetActiveDirectoryIdentityProviders() { r.PinnipedIDPs = append(r.PinnipedIDPs, v1alpha1.PinnipedIDP{ Name: provider.GetName(), Type: v1alpha1.IDPTypeActiveDirectory, - Flows: []v1alpha1.IDPFlow{v1alpha1.IDPFlowCLIPassword}, + Flows: []v1alpha1.IDPFlow{v1alpha1.IDPFlowCLIPassword, v1alpha1.IDPFlowBrowserAuthcode}, }) } for _, provider := range upstreamIDPs.GetOIDCIdentityProviders() { diff --git a/internal/oidc/idpdiscovery/idp_discovery_handler_test.go b/internal/oidc/idpdiscovery/idp_discovery_handler_test.go index f5e601bd..b33ab2d8 100644 --- a/internal/oidc/idpdiscovery/idp_discovery_handler_test.go +++ b/internal/oidc/idpdiscovery/idp_discovery_handler_test.go @@ -1,4 +1,4 @@ -// Copyright 2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package idpdiscovery @@ -37,22 +37,22 @@ func TestIDPDiscovery(t *testing.T) { wantContentType: "application/json", wantFirstResponseBodyJSON: here.Doc(`{ "pinniped_identity_providers": [ - {"name": "a-some-ldap-idp", "type": "ldap", "flows": ["cli_password"]}, + {"name": "a-some-ldap-idp", "type": "ldap", "flows": ["cli_password", "browser_authcode"]}, {"name": "a-some-oidc-idp", "type": "oidc", "flows": ["browser_authcode"]}, - {"name": "x-some-idp", "type": "ldap", "flows": ["cli_password"]}, + {"name": "x-some-idp", "type": "ldap", "flows": ["cli_password", "browser_authcode"]}, {"name": "x-some-idp", "type": "oidc", "flows": ["browser_authcode"]}, - {"name": "y-some-ad-idp", "type": "activedirectory", "flows": ["cli_password"]}, - {"name": "z-some-ad-idp", "type": "activedirectory", "flows": ["cli_password"]}, - {"name": "z-some-ldap-idp", "type": "ldap", "flows": ["cli_password"]}, + {"name": "y-some-ad-idp", "type": "activedirectory", "flows": ["cli_password", "browser_authcode"]}, + {"name": "z-some-ad-idp", "type": "activedirectory", "flows": ["cli_password", "browser_authcode"]}, + {"name": "z-some-ldap-idp", "type": "ldap", "flows": ["cli_password", "browser_authcode"]}, {"name": "z-some-oidc-idp", "type": "oidc", "flows": ["browser_authcode", "cli_password"]} ] }`), wantSecondResponseBodyJSON: here.Doc(`{ "pinniped_identity_providers": [ - {"name": "some-other-ad-idp-1", "type": "activedirectory", "flows": ["cli_password"]}, - {"name": "some-other-ad-idp-2", "type": "activedirectory", "flows": ["cli_password"]}, - {"name": "some-other-ldap-idp-1", "type": "ldap", "flows": ["cli_password"]}, - {"name": "some-other-ldap-idp-2", "type": "ldap", "flows": ["cli_password"]}, + {"name": "some-other-ad-idp-1", "type": "activedirectory", "flows": ["cli_password", "browser_authcode"]}, + {"name": "some-other-ad-idp-2", "type": "activedirectory", "flows": ["cli_password", "browser_authcode"]}, + {"name": "some-other-ldap-idp-1", "type": "ldap", "flows": ["cli_password", "browser_authcode"]}, + {"name": "some-other-ldap-idp-2", "type": "ldap", "flows": ["cli_password", "browser_authcode"]}, {"name": "some-other-oidc-idp-1", "type": "oidc", "flows": ["browser_authcode", "cli_password"]}, {"name": "some-other-oidc-idp-2", "type": "oidc", "flows": ["browser_authcode"]} ]