From bc700d58ae6ea7840975ddfb029907ef2346be94 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Thu, 19 Nov 2020 15:05:31 -0600 Subject: [PATCH] Split test environment variables so there's a specific supervisor upstream client. Prior to this we re-used the CLI testing client to test the authorize flow of the supervisor, but they really need to be separate upstream clients. For example, the supervisor client should be a non-public client with a client secret and a different callback endpoint. Signed-off-by: Matt Moyer --- hack/prepare-for-integration-tests.sh | 9 +++- test/deploy/dex/dex.yaml | 6 +++ test/integration/cli_test.go | 56 +++++++++++++------- test/integration/supervisor_login_test.go | 14 +++-- test/integration/supervisor_upstream_test.go | 4 +- test/library/env.go | 45 ++++++++++------ 6 files changed, 88 insertions(+), 46 deletions(-) diff --git a/hack/prepare-for-integration-tests.sh b/hack/prepare-for-integration-tests.sh index 09155dbb..11e1fbf8 100755 --- a/hack/prepare-for-integration-tests.sh +++ b/hack/prepare-for-integration-tests.sh @@ -295,9 +295,16 @@ export PINNIPED_TEST_PROXY=http://127.0.0.1:12346 export PINNIPED_TEST_CLI_OIDC_ISSUER=https://dex.dex.svc.cluster.local/dex export PINNIPED_TEST_CLI_OIDC_ISSUER_CA_BUNDLE="${test_ca_bundle_pem}" export PINNIPED_TEST_CLI_OIDC_CLIENT_ID=pinniped-cli -export PINNIPED_TEST_CLI_OIDC_LOCALHOST_PORT=48095 +export PINNIPED_TEST_CLI_OIDC_CALLBACK_URL=http://127.0.0.1:48095/callback 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_CLIENT_ID=pinniped-supervisor +export PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_CLIENT_SECRET=pinniped-supervisor-secret +export PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_CALLBACK_URL=https://127.0.0.1:12345/some/path/callback +export PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_USERNAME=pinny@example.com +export PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_PASSWORD=password read -r -d '' PINNIPED_TEST_CLUSTER_CAPABILITY_YAML << PINNIPED_TEST_CLUSTER_CAPABILITY_YAML_EOF || true ${pinniped_cluster_capability_file_content} diff --git a/test/deploy/dex/dex.yaml b/test/deploy/dex/dex.yaml index 274fdf27..bd078f24 100644 --- a/test/deploy/dex/dex.yaml +++ b/test/deploy/dex/dex.yaml @@ -24,6 +24,12 @@ staticClients: redirectURIs: - #@ "http://127.0.0.1:" + str(data.values.ports.cli) + "/callback" - #@ "http://[::1]:" + str(data.values.ports.cli) + "/callback" +- id: pinniped-supervisor + name: 'Pinniped Supervisor' + secret: pinniped-supervisor-secret + redirectURIs: + - #@ "http://127.0.0.1:" + str(data.values.ports.cli) + "/callback" + - #@ "http://[::1]:" + str(data.values.ports.cli) + "/callback" enablePasswordDB: true staticPasswords: - username: "pinny" diff --git a/test/integration/cli_test.go b/test/integration/cli_test.go index 48c65833..48965f76 100644 --- a/test/integration/cli_test.go +++ b/test/integration/cli_test.go @@ -11,11 +11,11 @@ import ( "fmt" "io" "io/ioutil" + "net/url" "os" "os/exec" "path/filepath" "regexp" - "strconv" "strings" "testing" "time" @@ -118,7 +118,7 @@ type loginProviderPatterns struct { func getLoginProvider(t *testing.T) *loginProviderPatterns { t.Helper() - issuer := library.IntegrationEnv(t).OIDCUpstream.Issuer + issuer := library.IntegrationEnv(t).CLITestUpstream.Issuer for _, p := range []loginProviderPatterns{ { Name: "Okta", @@ -270,13 +270,13 @@ func TestCLILoginOIDC(t *testing.T) { // Fill in the username and password and click "submit". t.Logf("logging into %s", loginProvider.Name) - require.NoError(t, page.First(loginProvider.UsernameSelector).Fill(env.OIDCUpstream.Username)) - require.NoError(t, page.First(loginProvider.PasswordSelector).Fill(env.OIDCUpstream.Password)) + require.NoError(t, page.First(loginProvider.UsernameSelector).Fill(env.CLITestUpstream.Username)) + require.NoError(t, page.First(loginProvider.PasswordSelector).Fill(env.CLITestUpstream.Password)) require.NoError(t, page.First(loginProvider.LoginButtonSelector).Click()) // Wait for the login to happen and us be redirected back to a localhost callback. t.Logf("waiting for redirect to localhost callback") - callbackURLPattern := regexp.MustCompile(`\Ahttp://127.0.0.1:` + strconv.Itoa(env.OIDCUpstream.LocalhostPort) + `/.+\z`) + callbackURLPattern := regexp.MustCompile(`\A` + regexp.QuoteMeta(env.CLITestUpstream.CallbackURL) + `\?.+\z`) waitForURL(t, page, callbackURLPattern) // Wait for the "pre" element that gets rendered for a `text/plain` page, and @@ -313,9 +313,9 @@ func TestCLILoginOIDC(t *testing.T) { require.NoError(t, err) claims := map[string]interface{}{} require.NoError(t, json.Unmarshal(jws.UnsafePayloadWithoutVerification(), &claims)) - require.Equal(t, env.OIDCUpstream.Issuer, claims["iss"]) - require.Equal(t, env.OIDCUpstream.ClientID, claims["aud"]) - require.Equal(t, env.OIDCUpstream.Username, claims["email"]) + require.Equal(t, env.CLITestUpstream.Issuer, claims["iss"]) + require.Equal(t, env.CLITestUpstream.ClientID, claims["aud"]) + require.Equal(t, env.CLITestUpstream.Username, claims["email"]) require.NotEmpty(t, claims["nonce"]) // Run the CLI again with the same session cache and login parameters. @@ -334,10 +334,10 @@ func TestCLILoginOIDC(t *testing.T) { t.Logf("overwriting cache to remove valid ID token") cache := filesession.New(sessionCachePath) cacheKey := oidcclient.SessionCacheKey{ - Issuer: env.OIDCUpstream.Issuer, - ClientID: env.OIDCUpstream.ClientID, + Issuer: env.CLITestUpstream.Issuer, + ClientID: env.CLITestUpstream.ClientID, Scopes: []string{"email", "offline_access", "openid", "profile"}, - RedirectURI: fmt.Sprintf("http://localhost:%d/callback", env.OIDCUpstream.LocalhostPort), + RedirectURI: strings.ReplaceAll(env.CLITestUpstream.CallbackURL, "127.0.0.1", "localhost"), } cached := cache.GetToken(cacheKey) require.NotNil(t, cached) @@ -378,10 +378,24 @@ func waitForVisibleElements(t *testing.T, page *agouti.Page, selectors ...string } func waitForURL(t *testing.T, page *agouti.Page, pat *regexp.Regexp) { - require.Eventually(t, func() bool { - url, err := page.URL() - return err == nil && pat.MatchString(url) - }, 10*time.Second, 100*time.Millisecond) + var lastURL string + require.Eventuallyf(t, + func() bool { + url, err := page.URL() + if err == nil && pat.MatchString(url) { + return true + } + if url != lastURL { + t.Logf("saw URL %s", url) + lastURL = url + } + return false + }, + 10*time.Second, + 100*time.Millisecond, + "expected to browse to %s, but never got there", + pat, + ) } func readAndExpectEmpty(r io.Reader) (err error) { @@ -407,18 +421,20 @@ func spawnTestGoroutine(t *testing.T, f func() error) { func oidcLoginCommand(ctx context.Context, t *testing.T, pinnipedExe string, sessionCachePath string) *exec.Cmd { env := library.IntegrationEnv(t) + callbackURL, err := url.Parse(env.CLITestUpstream.CallbackURL) + require.NoError(t, err) cmd := exec.CommandContext(ctx, pinnipedExe, "login", "oidc", - "--issuer", env.OIDCUpstream.Issuer, - "--client-id", env.OIDCUpstream.ClientID, - "--listen-port", strconv.Itoa(env.OIDCUpstream.LocalhostPort), + "--issuer", env.CLITestUpstream.Issuer, + "--client-id", env.CLITestUpstream.ClientID, + "--listen-port", callbackURL.Port(), "--session-cache", sessionCachePath, "--skip-browser", ) // If there is a custom CA bundle, pass it via --ca-bundle and a temporary file. - if env.OIDCUpstream.CABundle != "" { + if env.CLITestUpstream.CABundle != "" { path := filepath.Join(t.TempDir(), "test-ca.pem") - require.NoError(t, ioutil.WriteFile(path, []byte(env.OIDCUpstream.CABundle), 0600)) + require.NoError(t, ioutil.WriteFile(path, []byte(env.CLITestUpstream.CABundle), 0600)) cmd.Args = append(cmd.Args, "--ca-bundle", path) } diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index be403a3e..ac7c3d02 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -68,15 +68,13 @@ func TestSupervisorLogin(t *testing.T) { require.Equal(t, http.StatusUnprocessableEntity, rsp.StatusCode) // Create upstream OIDC provider. - testClientID := "test-client-id" - testClientSecret := "test-client-secret" spec := idpv1alpha1.UpstreamOIDCProviderSpec{ - Issuer: env.OIDCUpstream.Issuer, + Issuer: env.SupervisorTestUpstream.Issuer, TLS: &idpv1alpha1.TLSSpec{ - CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.OIDCUpstream.CABundle)), + CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.SupervisorTestUpstream.CABundle)), }, Client: idpv1alpha1.OIDCClient{ - SecretName: makeTestClientCredsSecret(t, testClientID, testClientSecret).Name, + SecretName: makeTestClientCredsSecret(t, env.SupervisorTestUpstream.ClientID, env.SupervisorTestUpstream.ClientSecret).Name, }, } upstream := makeTestUpstream(t, spec, idpv1alpha1.PhaseReady) @@ -94,7 +92,7 @@ func TestSupervisorLogin(t *testing.T) { ctx, t, upstream.Spec.Issuer, - testClientID, + env.SupervisorTestUpstream.ClientID, upstreamRedirectURI, rsp.Header.Get("Location"), ) @@ -147,9 +145,9 @@ func requireValidRedirectLocation( return url.Parse(env.Proxy) } } - if env.OIDCUpstream.CABundle != "" { + if env.SupervisorTestUpstream.CABundle != "" { transport.TLSClientConfig = &tls.Config{RootCAs: x509.NewCertPool()} - transport.TLSClientConfig.RootCAs.AppendCertsFromPEM([]byte(env.OIDCUpstream.CABundle)) + transport.TLSClientConfig.RootCAs.AppendCertsFromPEM([]byte(env.SupervisorTestUpstream.CABundle)) } ctx = oidc.ClientContext(ctx, &http.Client{Transport: &transport}) diff --git a/test/integration/supervisor_upstream_test.go b/test/integration/supervisor_upstream_test.go index ffda54b8..e38f2b17 100644 --- a/test/integration/supervisor_upstream_test.go +++ b/test/integration/supervisor_upstream_test.go @@ -48,9 +48,9 @@ func TestSupervisorUpstreamOIDCDiscovery(t *testing.T) { t.Run("valid", func(t *testing.T) { t.Parallel() spec := v1alpha1.UpstreamOIDCProviderSpec{ - Issuer: env.OIDCUpstream.Issuer, + Issuer: env.SupervisorTestUpstream.Issuer, TLS: &v1alpha1.TLSSpec{ - CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.OIDCUpstream.CABundle)), + CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.SupervisorTestUpstream.CABundle)), }, AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{ AdditionalScopes: []string{"email", "profile"}, diff --git a/test/library/env.go b/test/library/env.go index ce7e5f94..e6f3c1da 100644 --- a/test/library/env.go +++ b/test/library/env.go @@ -6,7 +6,6 @@ package library import ( "io/ioutil" "os" - "strconv" "strings" "testing" @@ -46,14 +45,18 @@ type TestEnv struct { ExpectedGroups []string `json:"expectedGroups"` } `json:"testUser"` - OIDCUpstream struct { - Issuer string `json:"issuer"` - CABundle string `json:"caBundle" ` - ClientID string `json:"clientID"` - LocalhostPort int `json:"localhostPort"` - Username string `json:"username"` - Password string `json:"password"` - } `json:"oidcUpstream"` + CLITestUpstream TestOIDCUpstream `json:"cliOIDCUpstream"` + SupervisorTestUpstream TestOIDCUpstream `json:"supervisorOIDCUpstream"` +} + +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"` } // IntegrationEnv gets the integration test environment from OS environment variables. This @@ -130,12 +133,24 @@ func loadEnvVars(t *testing.T, result *TestEnv) { require.NotEmpty(t, result.SupervisorCustomLabels, "PINNIPED_TEST_SUPERVISOR_CUSTOM_LABELS cannot be empty") result.Proxy = os.Getenv("PINNIPED_TEST_PROXY") - result.OIDCUpstream.Issuer = needEnv(t, "PINNIPED_TEST_CLI_OIDC_ISSUER") - result.OIDCUpstream.CABundle = os.Getenv("PINNIPED_TEST_CLI_OIDC_ISSUER_CA_BUNDLE") - result.OIDCUpstream.ClientID = needEnv(t, "PINNIPED_TEST_CLI_OIDC_CLIENT_ID") - result.OIDCUpstream.LocalhostPort, _ = strconv.Atoi(needEnv(t, "PINNIPED_TEST_CLI_OIDC_LOCALHOST_PORT")) - result.OIDCUpstream.Username = needEnv(t, "PINNIPED_TEST_CLI_OIDC_USERNAME") - result.OIDCUpstream.Password = needEnv(t, "PINNIPED_TEST_CLI_OIDC_PASSWORD") + result.CLITestUpstream = TestOIDCUpstream{ + Issuer: needEnv(t, "PINNIPED_TEST_CLI_OIDC_ISSUER"), + CABundle: os.Getenv("PINNIPED_TEST_CLI_OIDC_ISSUER_CA_BUNDLE"), + ClientID: needEnv(t, "PINNIPED_TEST_CLI_OIDC_CLIENT_ID"), + CallbackURL: needEnv(t, "PINNIPED_TEST_CLI_OIDC_CALLBACK_URL"), + Username: needEnv(t, "PINNIPED_TEST_CLI_OIDC_USERNAME"), + Password: needEnv(t, "PINNIPED_TEST_CLI_OIDC_PASSWORD"), + } + + 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"), + } } func (e *TestEnv) HasCapability(cap Capability) bool {