diff --git a/hack/prepare-for-integration-tests.sh b/hack/prepare-for-integration-tests.sh index ce683a4d..21e99aa4 100755 --- a/hack/prepare-for-integration-tests.sh +++ b/hack/prepare-for-integration-tests.sh @@ -305,11 +305,15 @@ export PINNIPED_TEST_CLI_OIDC_USERNAME=pinny@example.com export PINNIPED_TEST_CLI_OIDC_PASSWORD=password export PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_ISSUER=https://dex.dex.svc.cluster.local/dex export PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_ISSUER_CA_BUNDLE="${test_ca_bundle_pem}" +export PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_ADDITIONAL_SCOPES=email +export PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_USERNAME_CLAIM=email +export PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_GROUPS_CLAIM=groups export PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_CLIENT_ID=pinniped-supervisor export PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_CLIENT_SECRET=pinniped-supervisor-secret export PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_CALLBACK_URL=https://pinniped-supervisor-clusterip.supervisor.svc.cluster.local/some/path/callback export PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_USERNAME=pinny@example.com export PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_PASSWORD=password +export PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_EXPECTED_GROUPS= # Dex's local user store does not let us configure groups. read -r -d '' PINNIPED_TEST_CLUSTER_CAPABILITY_YAML << PINNIPED_TEST_CLUSTER_CAPABILITY_YAML_EOF || true ${pinniped_cluster_capability_file_content} diff --git a/internal/oidc/callback/callback_handler.go b/internal/oidc/callback/callback_handler.go index 66e7f3b1..417f1d2c 100644 --- a/internal/oidc/callback/callback_handler.go +++ b/internal/oidc/callback/callback_handler.go @@ -269,7 +269,7 @@ func getGroupsFromUpstreamIDToken( "configuredGroupsClaim", upstreamIDPConfig.GetGroupsClaim(), "groupsClaim", groupsClaim, ) - return nil, httperr.New(http.StatusUnprocessableEntity, "no groups claim in upstream ID token") + return nil, nil // the upstream IDP may have omitted the claim if the user has no groups } groupsAsArray, okAsArray := groupsAsInterface.([]string) diff --git a/internal/oidc/callback/callback_handler_test.go b/internal/oidc/callback/callback_handler_test.go index d7914cab..3b5659f4 100644 --- a/internal/oidc/callback/callback_handler_test.go +++ b/internal/oidc/callback/callback_handler_test.go @@ -430,8 +430,16 @@ func TestCallbackEndpoint(t *testing.T) { method: http.MethodGet, path: newRequestPath().WithState(happyState).String(), csrfCookie: happyCSRFCookie, - wantStatus: http.StatusUnprocessableEntity, - wantBody: "Unprocessable Entity: no groups claim in upstream ID token\n", + wantStatus: http.StatusFound, + wantRedirectLocationRegexp: happyDownstreamRedirectLocationRegexp, + wantBody: "", + wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + upstreamSubject, + wantDownstreamIDTokenUsername: upstreamUsername, + wantDownstreamRequestedScopes: happyDownstreamScopesRequested, + wantDownstreamGrantedScopes: happyDownstreamScopesGranted, + wantDownstreamNonce: downstreamNonce, + wantDownstreamPKCEChallenge: downstreamPKCEChallenge, + wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs, }, { diff --git a/test/integration/e2e_test.go b/test/integration/e2e_test.go index 90800096..844e3e35 100644 --- a/test/integration/e2e_test.go +++ b/test/integration/e2e_test.go @@ -16,20 +16,24 @@ import ( "os/exec" "path/filepath" "regexp" + "sort" "strings" "testing" "time" - v1 "k8s.io/api/core/v1" - + coreosoidc "github.com/coreos/go-oidc" "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" authv1alpha "go.pinniped.dev/generated/1.20/apis/concierge/authentication/v1alpha1" configv1alpha1 "go.pinniped.dev/generated/1.20/apis/supervisor/config/v1alpha1" idpv1alpha1 "go.pinniped.dev/generated/1.20/apis/supervisor/idp/v1alpha1" "go.pinniped.dev/internal/certauthority" + "go.pinniped.dev/internal/oidc" "go.pinniped.dev/internal/testutil" + "go.pinniped.dev/pkg/oidcclient" + "go.pinniped.dev/pkg/oidcclient/filesession" "go.pinniped.dev/test/library" "go.pinniped.dev/test/library/browsertest" ) @@ -86,7 +90,7 @@ func TestE2EFullIntegration(t *testing.T) { certSecret := library.CreateTestSecret(t, env.SupervisorNamespace, "oidc-provider-tls", - v1.SecretTypeTLS, + corev1.SecretTypeTLS, map[string]string{"tls.crt": string(certPEM), "tls.key": string(keyPEM)}, ) @@ -104,10 +108,11 @@ func TestE2EFullIntegration(t *testing.T) { CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.SupervisorTestUpstream.CABundle)), }, AuthorizationConfig: idpv1alpha1.OIDCAuthorizationConfig{ - AdditionalScopes: []string{"email"}, + AdditionalScopes: env.SupervisorTestUpstream.AdditionalScopes, }, Claims: idpv1alpha1.OIDCClaims{ - Username: "email", + Username: env.SupervisorTestUpstream.UsernameClaim, + Groups: env.SupervisorTestUpstream.GroupsClaim, }, Client: idpv1alpha1.OIDCClient{ SecretName: library.CreateClientCredsSecret(t, env.SupervisorTestUpstream.ClientID, env.SupervisorTestUpstream.ClientSecret).Name, @@ -270,4 +275,31 @@ func TestE2EFullIntegration(t *testing.T) { require.NoError(t, err) require.Greaterf(t, len(bytes.Split(kubectlOutput2, []byte("\n"))), 2, "expected some namespaces to be returned again") t.Logf("second kubectl command took %s", time.Since(start).String()) + + // probe our cache for the current ID token as a proxy for a whoami API + cache := filesession.New(sessionCachePath, filesession.WithErrorReporter(func(err error) { + require.NoError(t, err) + })) + + downstreamScopes := []string{coreosoidc.ScopeOfflineAccess, coreosoidc.ScopeOpenID, "pinniped:request-audience"} + sort.Strings(downstreamScopes) + token := cache.GetToken(oidcclient.SessionCacheKey{ + Issuer: downstream.Spec.Issuer, + ClientID: "pinniped-cli", + Scopes: downstreamScopes, + RedirectURI: "http://localhost:0/callback", + }) + require.NotNil(t, token) + + idTokenClaims := token.IDToken.Claims + username := idTokenClaims[oidc.DownstreamUsernameClaim].(string) + groups, _ := idTokenClaims[oidc.DownstreamGroupsClaim].([]string) + + require.Equal(t, env.SupervisorTestUpstream.Username, username) + if len(env.SupervisorTestUpstream.ExpectedGroups) == 0 { + // We only put a groups claim in our downstream ID token if we got groups from the upstream. + require.Nil(t, groups) + } else { + require.Equal(t, env.SupervisorTestUpstream.ExpectedGroups, groups) + } } diff --git a/test/library/env.go b/test/library/env.go index ec8aecaa..0fd61789 100644 --- a/test/library/env.go +++ b/test/library/env.go @@ -50,13 +50,17 @@ type TestEnv struct { } type TestOIDCUpstream struct { - Issuer string `json:"issuer"` - CABundle string `json:"caBundle" ` - ClientID string `json:"clientID"` - ClientSecret string `json:"clientSecret"` - CallbackURL string `json:"callback"` - Username string `json:"username"` - Password string `json:"password"` + Issuer string `json:"issuer"` + CABundle string `json:"caBundle"` + AdditionalScopes []string `json:"additionalScopes"` + UsernameClaim string `json:"usernameClaim"` + GroupsClaim string `json:"groupsClaim"` + ClientID string `json:"clientID"` + ClientSecret string `json:"clientSecret"` + CallbackURL string `json:"callback"` + Username string `json:"username"` + Password string `json:"password"` + ExpectedGroups []string `json:"expectedGroups"` } // ProxyEnv returns a set of environment variable strings (e.g., to combine with os.Environ()) which set up the configured test HTTP proxy. @@ -102,6 +106,16 @@ func needEnv(t *testing.T, key string) string { return value } +func filterEmpty(ss []string) []string { + filtered := []string{} + for _, s := range ss { + if len(s) != 0 { + filtered = append(filtered, s) + } + } + return filtered +} + func loadEnvVars(t *testing.T, result *TestEnv) { t.Helper() @@ -151,13 +165,17 @@ func loadEnvVars(t *testing.T, result *TestEnv) { } result.SupervisorTestUpstream = TestOIDCUpstream{ - Issuer: needEnv(t, "PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_ISSUER"), - CABundle: os.Getenv("PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_ISSUER_CA_BUNDLE"), - ClientID: needEnv(t, "PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_CLIENT_ID"), - ClientSecret: needEnv(t, "PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_CLIENT_SECRET"), - CallbackURL: needEnv(t, "PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_CALLBACK_URL"), - Username: needEnv(t, "PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_USERNAME"), - Password: needEnv(t, "PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_PASSWORD"), + Issuer: needEnv(t, "PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_ISSUER"), + CABundle: os.Getenv("PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_ISSUER_CA_BUNDLE"), + AdditionalScopes: strings.Fields(os.Getenv("PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_ADDITIONAL_SCOPES")), + UsernameClaim: os.Getenv("PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_USERNAME_CLAIM"), + GroupsClaim: os.Getenv("PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_GROUPS_CLAIM"), + ClientID: needEnv(t, "PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_CLIENT_ID"), + ClientSecret: needEnv(t, "PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_CLIENT_SECRET"), + CallbackURL: needEnv(t, "PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_CALLBACK_URL"), + Username: needEnv(t, "PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_USERNAME"), + Password: needEnv(t, "PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_PASSWORD"), + ExpectedGroups: filterEmpty(strings.Split(strings.ReplaceAll(os.Getenv("PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_EXPECTED_GROUPS"), " ", ""), ",")), } }