From d659b90e19990b77fd28000203a11d9b03ebc118 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Wed, 29 Mar 2023 13:55:00 -0700 Subject: [PATCH] `pinniped get kubeconfig` discovers support for username/groups scopes --- cmd/pinniped/cmd/kubeconfig.go | 93 ++++- cmd/pinniped/cmd/kubeconfig_test.go | 520 +++++++++++++++++++++------- pkg/oidcclient/login.go | 14 +- 3 files changed, 474 insertions(+), 153 deletions(-) diff --git a/cmd/pinniped/cmd/kubeconfig.go b/cmd/pinniped/cmd/kubeconfig.go index e4e73f61..9e4a53b5 100644 --- a/cmd/pinniped/cmd/kubeconfig.go +++ b/cmd/pinniped/cmd/kubeconfig.go @@ -23,6 +23,7 @@ import ( _ "k8s.io/client-go/plugin/pkg/client/auth" // Adds handlers for various dynamic auth plugins in client-go "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" + "k8s.io/utils/strings/slices" conciergev1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/authentication/v1alpha1" configv1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/config/v1alpha1" @@ -97,6 +98,11 @@ type getKubeconfigParams struct { installHint string } +type discoveryResponseScopesSupported struct { + // Same as ScopesSupported in the Supervisor's discovery handler's struct. + ScopesSupported []string `json:"scopes_supported"` +} + func kubeconfigCommand(deps kubeconfigDeps) *cobra.Command { var ( cmd = &cobra.Command{ @@ -232,11 +238,9 @@ func runGetKubeconfig(ctx context.Context, out io.Writer, deps kubeconfigDeps, f cluster.CertificateAuthorityData = flags.concierge.caBundle } - // 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, deps.log); err != nil { + if len(flags.oidc.issuer) > 0 { + err = pinnipedSupervisorDiscovery(ctx, &flags, deps.log) + if err != nil { return err } } @@ -733,21 +737,75 @@ func hasPendingStrategy(credentialIssuer *configv1alpha1.CredentialIssuer) bool return false } -func discoverSupervisorUpstreamIDP(ctx context.Context, flags *getKubeconfigParams, log plog.MinLogger) error { - httpClient, err := newDiscoveryHTTPClient(flags.oidc.caBundle) +func pinnipedSupervisorDiscovery(ctx context.Context, flags *getKubeconfigParams, log plog.MinLogger) error { + // Make a client suitable for calling the provider, which may or may not be a Pinniped Supervisor. + oidcProviderHTTPClient, err := newDiscoveryHTTPClient(flags.oidc.caBundle) if err != nil { return err } - pinnipedIDPsEndpoint, err := discoverIDPsDiscoveryEndpointURL(ctx, flags.oidc.issuer, httpClient) + // Call the provider's discovery endpoint, but don't parse the results yet. + discoveredProvider, err := discoverOIDCProvider(ctx, flags.oidc.issuer, oidcProviderHTTPClient) + if err != nil { + return err + } + + // Parse the discovery response to find the Supervisor IDP discovery endpoint. + pinnipedIDPsEndpoint, err := discoverIDPsDiscoveryEndpointURL(discoveredProvider) if err != nil { return err } if pinnipedIDPsEndpoint == "" { // The issuer is not advertising itself as a Pinniped Supervisor which supports upstream IDP discovery. + // Since this field is not present, then assume that the provider is not a Pinniped Supervisor. This field + // was added to the discovery response in v0.9.0, which is so long ago that we can assume there are no such + // old Supervisors in the wild which need to work with this CLI command anymore. Since the issuer is not a + // Supervisor, then there is no need to do the rest of the Supervisor-specific business logic below related + // to username/groups scopes or IDP types/names/flows. return nil } + // Now that we know that the provider is a Supervisor, perform an additional check based on its response. + // The username and groups scopes were added to the Supervisor in v0.20.0, and were also added to the + // "scopes_supported" field in the discovery response in that same version. If this CLI command is talking + // to an older Supervisor, then remove the username and groups scopes from the list of requested scopes + // since they will certainly cause an error from the old Supervisor during authentication. + supervisorSupportsBothUsernameAndGroupsScopes, err := discoverScopesSupportedIncludesBothUsernameAndGroups(discoveredProvider) + if err != nil { + return err + } + if !supervisorSupportsBothUsernameAndGroupsScopes { + flags.oidc.scopes = slices.Filter(nil, flags.oidc.scopes, func(scope string) bool { + if scope == oidcapi.ScopeUsername || scope == oidcapi.ScopeGroups { + log.Info("removed scope from --oidc-scopes list because it is not supported by this Supervisor", "scope", scope) + return false // Remove username and groups scopes if there were present in the flags. + } + return true // Keep any other scopes in the flag list. + }) + } + + // 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 flags.oidc.upstreamIDPType == "" || flags.oidc.upstreamIDPName == "" || flags.oidc.upstreamIDPFlow == "" { + if err := discoverSupervisorUpstreamIDP(ctx, pinnipedIDPsEndpoint, oidcProviderHTTPClient, flags, log); err != nil { + return err + } + } + + return nil +} + +func discoverOIDCProvider(ctx context.Context, issuer string, httpClient *http.Client) (*coreosoidc.Provider, error) { + discoveredProvider, err := coreosoidc.NewProvider(coreosoidc.ClientContext(ctx, httpClient), issuer) + if err != nil { + return nil, fmt.Errorf("while fetching OIDC discovery data from issuer: %w", err) + } + return discoveredProvider, nil +} + +func discoverSupervisorUpstreamIDP(ctx context.Context, pinnipedIDPsEndpoint string, httpClient *http.Client, flags *getKubeconfigParams, log plog.MinLogger) error { discoveredUpstreamIDPs, err := discoverAllAvailableSupervisorUpstreamIDPs(ctx, pinnipedIDPsEndpoint, httpClient) if err != nil { return err @@ -787,21 +845,24 @@ func newDiscoveryHTTPClient(caBundleFlag caBundleFlag) (*http.Client, error) { return phttp.Default(rootCAs), nil } -func discoverIDPsDiscoveryEndpointURL(ctx context.Context, issuer string, httpClient *http.Client) (string, error) { - discoveredProvider, err := coreosoidc.NewProvider(coreosoidc.ClientContext(ctx, httpClient), issuer) - if err != nil { - return "", fmt.Errorf("while fetching OIDC discovery data from issuer: %w", err) - } - +func discoverIDPsDiscoveryEndpointURL(discoveredProvider *coreosoidc.Provider) (string, error) { var body idpdiscoveryv1alpha1.OIDCDiscoveryResponse - err = discoveredProvider.Claims(&body) + err := discoveredProvider.Claims(&body) if err != nil { return "", fmt.Errorf("while fetching OIDC discovery data from issuer: %w", err) } - return body.SupervisorDiscovery.PinnipedIDPsEndpoint, nil } +func discoverScopesSupportedIncludesBothUsernameAndGroups(discoveredProvider *coreosoidc.Provider) (bool, error) { + var body discoveryResponseScopesSupported + err := discoveredProvider.Claims(&body) + if err != nil { + return false, fmt.Errorf("while fetching OIDC discovery data from issuer: %w", err) + } + return slices.Contains(body.ScopesSupported, oidcapi.ScopeUsername) && slices.Contains(body.ScopesSupported, oidcapi.ScopeGroups), nil +} + 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 { diff --git a/cmd/pinniped/cmd/kubeconfig_test.go b/cmd/pinniped/cmd/kubeconfig_test.go index eb629849..ca8b762c 100644 --- a/cmd/pinniped/cmd/kubeconfig_test.go +++ b/cmd/pinniped/cmd/kubeconfig_test.go @@ -81,6 +81,7 @@ func TestGetKubeconfig(t *testing.T) { "discovery.supervisor.pinniped.dev/v1alpha1": { "pinniped_identity_providers_endpoint": "%s/v1alpha1/pinniped_identity_providers" }, + "scopes_supported": ["openid", "offline_access", "pinniped:request-audience", "username", "groups"], "another-key": "another-value" }`, issuerURL, issuerURL) } @@ -1086,7 +1087,8 @@ func TestGetKubeconfig(t *testing.T) { "issuer": "%s", "discovery.supervisor.pinniped.dev/v1alpha1": { "pinniped_identity_providers_endpoint": "https%%://illegal_url" - } + }, + "scopes_supported": ["openid", "offline_access", "pinniped:request-audience", "username", "groups"] }`, issuerURL) }, wantLogs: func(issuerCABundle string, issuerURL string) []string { @@ -1369,7 +1371,7 @@ func TestGetKubeconfig(t *testing.T) { command: '.../path/to/pinniped' env: [] installHint: The pinniped CLI does not appear to be installed. See https://get.pinniped.dev/cli - for more details + for more details provideClusterInfo: true `, issuerURL, @@ -1406,41 +1408,41 @@ func TestGetKubeconfig(t *testing.T) { }, wantStdout: func(issuerCABundle string, issuerURL string) string { return here.Doc(` - 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 - - static - - --enable-concierge - - --concierge-api-group-suffix=pinniped.dev - - --concierge-authenticator-name=test-authenticator - - --concierge-authenticator-type=webhook - - --concierge-endpoint=https://fake-server-url-value - - --concierge-ca-bundle-data=ZmFrZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YS12YWx1ZQ== - - --token=test-token - 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 - `) + 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 + - static + - --enable-concierge + - --concierge-api-group-suffix=pinniped.dev + - --concierge-authenticator-name=test-authenticator + - --concierge-authenticator-type=webhook + - --concierge-endpoint=https://fake-server-url-value + - --concierge-ca-bundle-data=ZmFrZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YS12YWx1ZQ== + - --token=test-token + 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 + `) }, }, { @@ -1470,42 +1472,42 @@ func TestGetKubeconfig(t *testing.T) { }, wantStdout: func(issuerCABundle string, issuerURL string) string { return here.Doc(` - 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 - - static - - --enable-concierge - - --concierge-api-group-suffix=pinniped.dev - - --concierge-authenticator-name=test-authenticator - - --concierge-authenticator-type=webhook - - --concierge-endpoint=https://fake-server-url-value - - --concierge-ca-bundle-data=ZmFrZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YS12YWx1ZQ== - - --credential-cache= - - --token-env=TEST_TOKEN - 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 - `) + 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 + - static + - --enable-concierge + - --concierge-api-group-suffix=pinniped.dev + - --concierge-authenticator-name=test-authenticator + - --concierge-authenticator-type=webhook + - --concierge-endpoint=https://fake-server-url-value + - --concierge-ca-bundle-data=ZmFrZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YS12YWx1ZQ== + - --credential-cache= + - --token-env=TEST_TOKEN + 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 + `) }, }, { @@ -1573,7 +1575,7 @@ func TestGetKubeconfig(t *testing.T) { command: '.../path/to/pinniped' env: [] installHint: The pinniped CLI does not appear to be installed. See https://get.pinniped.dev/cli - for more details + for more details provideClusterInfo: true `, issuerURL, @@ -1659,7 +1661,7 @@ func TestGetKubeconfig(t *testing.T) { command: '.../path/to/pinniped' env: [] installHint: The pinniped CLI does not appear to be installed. See https://get.pinniped.dev/cli - for more details + for more details provideClusterInfo: true `, base64.StdEncoding.EncodeToString(testConciergeCA.Bundle()), @@ -1772,7 +1774,7 @@ func TestGetKubeconfig(t *testing.T) { command: '.../path/to/pinniped' env: [] installHint: The pinniped CLI does not appear to be installed. See https://get.pinniped.dev/cli - for more details + for more details provideClusterInfo: true `, base64.StdEncoding.EncodeToString(testConciergeCA.Bundle()), @@ -1881,7 +1883,7 @@ func TestGetKubeconfig(t *testing.T) { command: '.../path/to/pinniped' env: [] installHint: The pinniped CLI does not appear to be installed. See https://get.pinniped.dev/cli - for more details + for more details provideClusterInfo: true `, issuerURL, @@ -1960,7 +1962,7 @@ func TestGetKubeconfig(t *testing.T) { command: '.../path/to/pinniped' env: [] installHint: The pinniped CLI does not appear to be installed. See https://get.pinniped.dev/cli - for more details + for more details provideClusterInfo: true `, issuerURL, @@ -2039,7 +2041,7 @@ func TestGetKubeconfig(t *testing.T) { command: '.../path/to/pinniped' env: [] installHint: The pinniped CLI does not appear to be installed. See https://get.pinniped.dev/cli - for more details + for more details provideClusterInfo: true `, issuerURL, @@ -2114,7 +2116,7 @@ func TestGetKubeconfig(t *testing.T) { command: '.../path/to/pinniped' env: [] installHint: The pinniped CLI does not appear to be installed. See https://get.pinniped.dev/cli - for more details + for more details provideClusterInfo: true `, issuerURL, @@ -2187,7 +2189,7 @@ func TestGetKubeconfig(t *testing.T) { command: '.../path/to/pinniped' env: [] installHint: The pinniped CLI does not appear to be installed. See https://get.pinniped.dev/cli - for more details + for more details provideClusterInfo: true `, issuerURL, @@ -2267,7 +2269,272 @@ func TestGetKubeconfig(t *testing.T) { command: '.../path/to/pinniped' env: [] installHint: The pinniped CLI does not appear to be installed. See https://get.pinniped.dev/cli - for more details + for more details + provideClusterInfo: true + `, + issuerURL, + base64.StdEncoding.EncodeToString([]byte(issuerCABundle))) + }, + }, + { + name: "IDP discovery endpoint is listed in OIDC discovery document but scopes_supported does not include username or groups, so do not request username or groups in kubeconfig's --scopes", + args: func(issuerCABundle string, issuerURL string) []string { + return []string{ + "--kubeconfig", "./testdata/kubeconfig.yaml", + "--skip-validation", + } + }, + conciergeObjects: func(issuerCABundle string, issuerURL string) []runtime.Object { + return []runtime.Object{ + credentialIssuer(), + jwtAuthenticator(issuerCABundle, issuerURL), + } + }, + oidcDiscoveryResponse: func(issuerURL string) string { + return here.Docf(`{ + "issuer": "%s", + "discovery.supervisor.pinniped.dev/v1alpha1": { + "pinniped_identity_providers_endpoint": "%s/v1alpha1/pinniped_identity_providers" + }, + "scopes_supported": ["openid", "offline_access", "pinniped:request-audience"] + }`, issuerURL, issuerURL) + }, + idpsDiscoveryResponse: here.Docf(`{ + "pinniped_identity_providers": [ + {"name": "some-oidc-idp", "type": "oidc"} + ] + }`), + wantLogs: func(issuerCABundle string, issuerURL string) []string { + return []string{ + `"level"=0 "msg"="discovered CredentialIssuer" "name"="test-credential-issuer"`, + `"level"=0 "msg"="discovered Concierge operating in TokenCredentialRequest API mode"`, + `"level"=0 "msg"="discovered Concierge endpoint" "endpoint"="https://fake-server-url-value"`, + `"level"=0 "msg"="discovered Concierge certificate authority bundle" "roots"=0`, + `"level"=0 "msg"="discovered JWTAuthenticator" "name"="test-authenticator"`, + fmt.Sprintf(`"level"=0 "msg"="discovered OIDC issuer" "issuer"="%s"`, issuerURL), + `"level"=0 "msg"="discovered OIDC audience" "audience"="test-audience"`, + `"level"=0 "msg"="discovered OIDC CA bundle" "roots"=1`, + `"level"=0 "msg"="removed scope from --oidc-scopes list because it is not supported by this Supervisor" "scope"="username"`, + `"level"=0 "msg"="removed scope from --oidc-scopes list because it is not supported by this Supervisor" "scope"="groups"`, + } + }, + 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 + - --enable-concierge + - --concierge-api-group-suffix=pinniped.dev + - --concierge-authenticator-name=test-authenticator + - --concierge-authenticator-type=jwt + - --concierge-endpoint=https://fake-server-url-value + - --concierge-ca-bundle-data=ZmFrZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YS12YWx1ZQ== + - --issuer=%s + - --client-id=pinniped-cli + - --scopes=offline_access,openid,pinniped:request-audience + - --ca-bundle-data=%s + - --request-audience=test-audience + - --upstream-identity-provider-name=some-oidc-idp + - --upstream-identity-provider-type=oidc + 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))) + }, + }, + { + name: "IDP discovery endpoint is listed in OIDC discovery document but scopes_supported is not listed (which shouldn't really happen), so do not request username or groups in kubeconfig's --scopes", + args: func(issuerCABundle string, issuerURL string) []string { + return []string{ + "--kubeconfig", "./testdata/kubeconfig.yaml", + "--skip-validation", + } + }, + conciergeObjects: func(issuerCABundle string, issuerURL string) []runtime.Object { + return []runtime.Object{ + credentialIssuer(), + jwtAuthenticator(issuerCABundle, issuerURL), + } + }, + oidcDiscoveryResponse: func(issuerURL string) string { + return here.Docf(`{ + "issuer": "%s", + "discovery.supervisor.pinniped.dev/v1alpha1": { + "pinniped_identity_providers_endpoint": "%s/v1alpha1/pinniped_identity_providers" + } + }`, issuerURL, issuerURL) + }, + idpsDiscoveryResponse: here.Docf(`{ + "pinniped_identity_providers": [ + {"name": "some-oidc-idp", "type": "oidc"} + ] + }`), + wantLogs: func(issuerCABundle string, issuerURL string) []string { + return []string{ + `"level"=0 "msg"="discovered CredentialIssuer" "name"="test-credential-issuer"`, + `"level"=0 "msg"="discovered Concierge operating in TokenCredentialRequest API mode"`, + `"level"=0 "msg"="discovered Concierge endpoint" "endpoint"="https://fake-server-url-value"`, + `"level"=0 "msg"="discovered Concierge certificate authority bundle" "roots"=0`, + `"level"=0 "msg"="discovered JWTAuthenticator" "name"="test-authenticator"`, + fmt.Sprintf(`"level"=0 "msg"="discovered OIDC issuer" "issuer"="%s"`, issuerURL), + `"level"=0 "msg"="discovered OIDC audience" "audience"="test-audience"`, + `"level"=0 "msg"="discovered OIDC CA bundle" "roots"=1`, + `"level"=0 "msg"="removed scope from --oidc-scopes list because it is not supported by this Supervisor" "scope"="username"`, + `"level"=0 "msg"="removed scope from --oidc-scopes list because it is not supported by this Supervisor" "scope"="groups"`, + } + }, + 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 + - --enable-concierge + - --concierge-api-group-suffix=pinniped.dev + - --concierge-authenticator-name=test-authenticator + - --concierge-authenticator-type=jwt + - --concierge-endpoint=https://fake-server-url-value + - --concierge-ca-bundle-data=ZmFrZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YS12YWx1ZQ== + - --issuer=%s + - --client-id=pinniped-cli + - --scopes=offline_access,openid,pinniped:request-audience + - --ca-bundle-data=%s + - --request-audience=test-audience + - --upstream-identity-provider-name=some-oidc-idp + - --upstream-identity-provider-type=oidc + 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))) + }, + }, + { + name: "IDP discovery endpoint is listed in OIDC discovery document but scopes_supported does not include username or groups, and scopes username and groups were also not requested by flags", + args: func(issuerCABundle string, issuerURL string) []string { + return []string{ + "--kubeconfig", "./testdata/kubeconfig.yaml", + "--skip-validation", + "--oidc-scopes", "foo,bar,baz", + } + }, + conciergeObjects: func(issuerCABundle string, issuerURL string) []runtime.Object { + return []runtime.Object{ + credentialIssuer(), + jwtAuthenticator(issuerCABundle, issuerURL), + } + }, + oidcDiscoveryResponse: func(issuerURL string) string { + return here.Docf(`{ + "issuer": "%s", + "discovery.supervisor.pinniped.dev/v1alpha1": { + "pinniped_identity_providers_endpoint": "%s/v1alpha1/pinniped_identity_providers" + }, + "scopes_supported": ["openid", "offline_access", "pinniped:request-audience"] + }`, issuerURL, issuerURL) + }, + idpsDiscoveryResponse: here.Docf(`{ + "pinniped_identity_providers": [ + {"name": "some-oidc-idp", "type": "oidc"} + ] + }`), + wantLogs: func(issuerCABundle string, issuerURL string) []string { + return []string{ + `"level"=0 "msg"="discovered CredentialIssuer" "name"="test-credential-issuer"`, + `"level"=0 "msg"="discovered Concierge operating in TokenCredentialRequest API mode"`, + `"level"=0 "msg"="discovered Concierge endpoint" "endpoint"="https://fake-server-url-value"`, + `"level"=0 "msg"="discovered Concierge certificate authority bundle" "roots"=0`, + `"level"=0 "msg"="discovered JWTAuthenticator" "name"="test-authenticator"`, + fmt.Sprintf(`"level"=0 "msg"="discovered OIDC issuer" "issuer"="%s"`, issuerURL), + `"level"=0 "msg"="discovered OIDC audience" "audience"="test-audience"`, + `"level"=0 "msg"="discovered OIDC CA bundle" "roots"=1`, + } + }, + 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 + - --enable-concierge + - --concierge-api-group-suffix=pinniped.dev + - --concierge-authenticator-name=test-authenticator + - --concierge-authenticator-type=jwt + - --concierge-endpoint=https://fake-server-url-value + - --concierge-ca-bundle-data=ZmFrZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YS12YWx1ZQ== + - --issuer=%s + - --client-id=pinniped-cli + - --scopes=foo,bar,baz + - --ca-bundle-data=%s + - --request-audience=test-audience + - --upstream-identity-provider-name=some-oidc-idp + - --upstream-identity-provider-type=oidc + 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, @@ -2291,7 +2558,8 @@ func TestGetKubeconfig(t *testing.T) { jwtAuthenticator(issuerCABundle, issuerURL), } }, - oidcDiscoveryStatusCode: http.StatusNotFound, // should not get called by the client in this case + oidcDiscoveryResponse: happyOIDCDiscoveryResponse, // still called to check for support of username and groups scopes + idpsDiscoveryStatusCode: 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"`, @@ -2345,7 +2613,7 @@ func TestGetKubeconfig(t *testing.T) { command: '.../path/to/pinniped' env: [] installHint: The pinniped CLI does not appear to be installed. See https://get.pinniped.dev/cli - for more details + for more details provideClusterInfo: true `, issuerURL, @@ -2428,7 +2696,7 @@ func TestGetKubeconfig(t *testing.T) { command: '.../path/to/pinniped' env: [] installHint: The pinniped CLI does not appear to be installed. See https://get.pinniped.dev/cli - for more details + for more details provideClusterInfo: true `, issuerURL, @@ -2486,7 +2754,7 @@ func TestGetKubeconfig(t *testing.T) { command: '.../path/to/pinniped' env: [] installHint: The pinniped CLI does not appear to be installed. See https://get.pinniped.dev/cli - for more details + for more details provideClusterInfo: true `, issuerURL, @@ -2547,7 +2815,7 @@ func TestGetKubeconfig(t *testing.T) { command: '.../path/to/pinniped' env: [] installHint: The pinniped CLI does not appear to be installed. See https://get.pinniped.dev/cli - for more details + for more details provideClusterInfo: true `, issuerURL, @@ -2608,7 +2876,7 @@ func TestGetKubeconfig(t *testing.T) { command: '.../path/to/pinniped' env: [] installHint: The pinniped CLI does not appear to be installed. See https://get.pinniped.dev/cli - for more details + for more details provideClusterInfo: true `, issuerURL, @@ -2670,7 +2938,7 @@ func TestGetKubeconfig(t *testing.T) { command: '.../path/to/pinniped' env: [] installHint: The pinniped CLI does not appear to be installed. See https://get.pinniped.dev/cli - for more details + for more details provideClusterInfo: true `, issuerURL, @@ -2733,7 +3001,7 @@ func TestGetKubeconfig(t *testing.T) { command: '.../path/to/pinniped' env: [] installHint: The pinniped CLI does not appear to be installed. See https://get.pinniped.dev/cli - for more details + for more details provideClusterInfo: true `, issuerURL, @@ -2794,7 +3062,7 @@ func TestGetKubeconfig(t *testing.T) { command: '.../path/to/pinniped' env: [] installHint: The pinniped CLI does not appear to be installed. See https://get.pinniped.dev/cli - for more details + for more details provideClusterInfo: true `, issuerURL, @@ -2854,7 +3122,7 @@ func TestGetKubeconfig(t *testing.T) { command: '.../path/to/pinniped' env: [] installHint: The pinniped CLI does not appear to be installed. See https://get.pinniped.dev/cli - for more details + for more details provideClusterInfo: true `, issuerURL, @@ -2888,40 +3156,40 @@ func TestGetKubeconfig(t *testing.T) { }, wantStdout: func(issuerCABundle string, issuerURL string) string { return here.Doc(` - 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 - - static - - --enable-concierge - - --concierge-api-group-suffix=pinniped.dev - - --concierge-authenticator-name=test-authenticator - - --concierge-authenticator-type=webhook - - --concierge-endpoint=https://fake-server-url-value - - --concierge-ca-bundle-data=ZmFrZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YS12YWx1ZQ== - - --token=test-token - command: '.../path/to/pinniped' - env: [] - installHint: Test installHint message - provideClusterInfo: true - `) + 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 + - static + - --enable-concierge + - --concierge-api-group-suffix=pinniped.dev + - --concierge-authenticator-name=test-authenticator + - --concierge-authenticator-type=webhook + - --concierge-endpoint=https://fake-server-url-value + - --concierge-ca-bundle-data=ZmFrZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YS12YWx1ZQ== + - --token=test-token + command: '.../path/to/pinniped' + env: [] + installHint: Test installHint message + provideClusterInfo: true + `) }, }, } diff --git a/pkg/oidcclient/login.go b/pkg/oidcclient/login.go index 69ddd33f..b75f743e 100644 --- a/pkg/oidcclient/login.go +++ b/pkg/oidcclient/login.go @@ -1,4 +1,4 @@ -// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package oidcclient implements a CLI OIDC login flow. @@ -27,6 +27,7 @@ import ( "golang.org/x/oauth2" "golang.org/x/term" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/strings/slices" oidcapi "go.pinniped.dev/generated/latest/apis/supervisor/oidc" "go.pinniped.dev/internal/httputil/httperr" @@ -741,7 +742,7 @@ func (h *handlerState) initOIDCDiscovery() error { if err := h.provider.Claims(&discoveryClaims); err != nil { return fmt.Errorf("could not decode response_modes_supported in OIDC discovery from %q: %w", h.issuer, err) } - h.useFormPost = stringSliceContains(discoveryClaims.ResponseModesSupported, "form_post") + h.useFormPost = slices.Contains(discoveryClaims.ResponseModesSupported, "form_post") return nil } @@ -756,15 +757,6 @@ func validateURLUsesHTTPS(uri string, uriName string) error { return nil } -func stringSliceContains(slice []string, s string) bool { - for _, item := range slice { - if item == s { - return true - } - } - return false -} - func (h *handlerState) tokenExchangeRFC8693(baseToken *oidctypes.Token) (*oidctypes.Token, error) { h.logger.V(plog.KlogLevelDebug).Info("Pinniped: Performing RFC8693 token exchange", "requestedAudience", h.requestedAudience) // Perform OIDC discovery. This may have already been performed if there was not a cached base token.