diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index 6947568d5..5ab67d5af 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -56,6 +56,11 @@ func TestSupervisorLogin_Browser(t *testing.T) { testlib.SkipTestWhenLDAPIsUnavailable(t, env) } + skipGitHubTests := func(t *testing.T) { + t.Helper() + testlib.SkipTestWhenGitHubIsUnavailable(t) + } + skipActiveDirectoryTests := func(t *testing.T) { t.Helper() testlib.SkipTestWhenActiveDirectoryIsUnavailable(t, env) @@ -73,6 +78,19 @@ func TestSupervisorLogin_Browser(t *testing.T) { } } + basicGitHubIdentityProviderSpec := func() idpv1alpha1.GitHubIdentityProviderSpec { + return idpv1alpha1.GitHubIdentityProviderSpec{ + AllowAuthentication: idpv1alpha1.GitHubAllowAuthenticationSpec{ + Organizations: idpv1alpha1.GitHubOrganizationsSpec{ + Policy: ptr.To(idpv1alpha1.GitHubAllowedAuthOrganizationsPolicyAllGitHubUsers), + }, + }, + Client: idpv1alpha1.GitHubClientSpec{ + SecretName: testlib.CreateGitHubClientCredentialsSecret(t, env.SupervisorUpstreamGithub.GithubAppClientID, env.SupervisorUpstreamGithub.GithubAppClientSecret).Name, + }, + } + } + createActiveDirectoryIdentityProvider := func(t *testing.T, edit func(spec *idpv1alpha1.ActiveDirectoryIdentityProviderSpec)) (*idpv1alpha1.ActiveDirectoryIdentityProvider, *corev1.Secret) { t.Helper() @@ -169,6 +187,14 @@ func TestSupervisorLogin_Browser(t *testing.T) { regexp.QuoteMeta("&sub=") + ".+" + "$" + // The downstream ID token Subject should include the upstream user ID after the upstream issuer name + // and IDP display name. + expectedIDTokenSubjectRegexForUpstreamGitHub := "^" + + regexp.QuoteMeta("https://api.github.com?idpName=test-upstream-github-idp-") + `[\w]+` + + regexp.QuoteMeta("&login=") + regexp.QuoteMeta(env.SupervisorUpstreamGithub.TestUserUsername) + + regexp.QuoteMeta("&id=") + regexp.QuoteMeta(env.SupervisorUpstreamGithub.TestUserID) + + "$" + // The downstream ID token Subject should be the Host URL, plus the user search base, plus the IDP display name, // plus value pulled from the requested UserSearch.Attributes.UID attribute. expectedIDTokenSubjectRegexForUpstreamLDAP := "^" + @@ -484,6 +510,98 @@ func TestSupervisorLogin_Browser(t *testing.T) { "upstream_username": env.SupervisorUpstreamOIDC.Username, }, "upstream_groups", env.SupervisorUpstreamOIDC.ExpectedGroups), }, + { + name: "github with all orgs allowed and default claim settings", + maybeSkip: skipGitHubTests, + createIDP: func(t *testing.T) string { + return testlib.CreateTestGitHubIdentityProvider(t, basicGitHubIdentityProviderSpec(), idpv1alpha1.GitHubPhaseReady).Name + }, + requestAuthorization: requestAuthorizationUsingBrowserAuthcodeFlowGitHub, + wantDownstreamIDTokenSubjectToMatch: expectedIDTokenSubjectRegexForUpstreamGitHub, + wantDownstreamIDTokenUsernameToMatch: func(_ string) string { + return "^" + regexp.QuoteMeta(env.SupervisorUpstreamGithub.TestUserUsername+":"+env.SupervisorUpstreamGithub.TestUserID) + "$" + }, + wantDownstreamIDTokenGroups: env.SupervisorUpstreamGithub.TestUserExpectedTeamSlugs, + }, + { + name: "github with list of allowed orgs, username as login, and groups as names", + maybeSkip: skipGitHubTests, + createIDP: func(t *testing.T) string { + spec := basicGitHubIdentityProviderSpec() + spec.AllowAuthentication = idpv1alpha1.GitHubAllowAuthenticationSpec{ + Organizations: idpv1alpha1.GitHubOrganizationsSpec{ + Policy: ptr.To(idpv1alpha1.GitHubAllowedAuthOrganizationsPolicyOnlyUsersFromAllowedOrganizations), + Allowed: []string{env.SupervisorUpstreamGithub.TestUserOrganization, "some-unrelated-org"}, + }, + } + spec.Claims = idpv1alpha1.GitHubClaims{ + Username: ptr.To(idpv1alpha1.GitHubUsernameLogin), + Groups: ptr.To(idpv1alpha1.GitHubUseTeamNameForGroupName), + } + return testlib.CreateTestGitHubIdentityProvider(t, spec, idpv1alpha1.GitHubPhaseReady).Name + }, + requestAuthorization: requestAuthorizationUsingBrowserAuthcodeFlowGitHub, + wantDownstreamIDTokenSubjectToMatch: expectedIDTokenSubjectRegexForUpstreamGitHub, + wantDownstreamIDTokenUsernameToMatch: func(_ string) string { + return "^" + regexp.QuoteMeta(env.SupervisorUpstreamGithub.TestUserUsername) + "$" + }, + wantDownstreamIDTokenGroups: env.SupervisorUpstreamGithub.TestUserExpectedTeamNames, + }, + { + name: "github with list of allowed orgs differently cased, username as id, and groups as names", + maybeSkip: skipGitHubTests, + createIDP: func(t *testing.T) string { + spec := basicGitHubIdentityProviderSpec() + spec.AllowAuthentication = idpv1alpha1.GitHubAllowAuthenticationSpec{ + Organizations: idpv1alpha1.GitHubOrganizationsSpec{ + Policy: ptr.To(idpv1alpha1.GitHubAllowedAuthOrganizationsPolicyOnlyUsersFromAllowedOrganizations), + Allowed: []string{strings.ToUpper(env.SupervisorUpstreamGithub.TestUserOrganization), "some-unrelated-org"}, + }, + } + spec.Claims = idpv1alpha1.GitHubClaims{ + Username: ptr.To(idpv1alpha1.GitHubUsernameID), + Groups: ptr.To(idpv1alpha1.GitHubUseTeamNameForGroupName), + } + return testlib.CreateTestGitHubIdentityProvider(t, spec, idpv1alpha1.GitHubPhaseReady).Name + }, + requestAuthorization: requestAuthorizationUsingBrowserAuthcodeFlowGitHub, + wantDownstreamIDTokenSubjectToMatch: expectedIDTokenSubjectRegexForUpstreamGitHub, + wantDownstreamIDTokenUsernameToMatch: func(_ string) string { + return "^" + regexp.QuoteMeta(env.SupervisorUpstreamGithub.TestUserID) + "$" + }, + wantDownstreamIDTokenGroups: env.SupervisorUpstreamGithub.TestUserExpectedTeamNames, + }, + { + name: "github when user does not belong to any of the allowed orgs, should fail to exchange downstream authcode", + maybeSkip: skipGitHubTests, + createIDP: func(t *testing.T) string { + spec := basicGitHubIdentityProviderSpec() + spec.AllowAuthentication = idpv1alpha1.GitHubAllowAuthenticationSpec{ + Organizations: idpv1alpha1.GitHubOrganizationsSpec{ + Policy: ptr.To(idpv1alpha1.GitHubAllowedAuthOrganizationsPolicyOnlyUsersFromAllowedOrganizations), + Allowed: []string{"some-unrelated-org"}, + }, + } + return testlib.CreateTestGitHubIdentityProvider(t, spec, idpv1alpha1.GitHubPhaseReady).Name + }, + requestAuthorization: func(t *testing.T, _, downstreamAuthorizeURL, downstreamCallbackURL, _, _ string, httpClient *http.Client) { + t.Helper() + browser := openBrowserAndNavigateToAuthorizeURL(t, downstreamAuthorizeURL, httpClient) + // Expect to be redirected to the upstream provider and log in. + browsertest.LoginToUpstreamGitHub(t, browser, env.SupervisorUpstreamGithub) + // Wait for the login to happen and us be redirected back to the Supervisor callback with an error showing. + t.Logf("waiting for redirect to Supervisor callback endpoint, which should be showing an error") + callbackURLPattern := regexp.MustCompile(`\A` + regexp.QuoteMeta(env.SupervisorUpstreamOIDC.CallbackURL) + `\?.+\z`) + browser.WaitForURL(t, callbackURLPattern) + // Get the text of the preformatted error message showing on the page. + textOfPreTag := browser.TextOfFirstMatch(t, "pre") + require.Equal(t, + "Unprocessable Entity: failed to get user info from GitHub API: "+ + "user is not allowed to log in due to organization membership policy\n", + textOfPreTag) + }, + wantLocalhostCallbackToNeverHappen: true, + }, { name: "ldap with email as username and groups names as DNs and using an LDAP provider which supports TLS", maybeSkip: skipLDAPTests, @@ -2427,7 +2545,7 @@ func testSupervisorLogin( } // Create the downstream FederationDomain and expect it to go into the appropriate status condition. - downstream := testlib.CreateTestFederationDomain(ctx, t, + federationDomain := testlib.CreateTestFederationDomain(ctx, t, configv1alpha1.FederationDomainSpec{ Issuer: issuerURL.String(), TLS: &configv1alpha1.FederationDomainTLSSpec{SecretName: certSecret.Name}, @@ -2476,7 +2594,7 @@ func testSupervisorLogin( var discovery *coreosoidc.Provider testlib.RequireEventually(t, func(requireEventually *require.Assertions) { var err error - discovery, err = coreosoidc.NewProvider(oidcHTTPClientContext, downstream.Spec.Issuer) + discovery, err = coreosoidc.NewProvider(oidcHTTPClientContext, federationDomain.Spec.Issuer) requireEventually.NoError(err) }, 30*time.Second, 200*time.Millisecond) @@ -2527,7 +2645,7 @@ func testSupervisorLogin( downstreamAuthorizeURL := downstreamOAuth2Config.AuthCodeURL(stateParam.String(), authorizeRequestParams...) // Perform parameterized auth code acquisition. - requestAuthorization(t, downstream.Spec.Issuer, downstreamAuthorizeURL, localCallbackServer.URL, username, password, httpClient) + requestAuthorization(t, federationDomain.Spec.Issuer, downstreamAuthorizeURL, localCallbackServer.URL, username, password, httpClient) // Expect that our callback handler was invoked. callback, err := localCallbackServer.waitForCallback(10 * time.Second) @@ -2884,6 +3002,19 @@ func loginToUpstreamOIDCAndWaitForCallback(t *testing.T, b *browsertest.Browser, b.WaitForURL(t, callbackURLPattern) } +func loginToUpstreamGitHubAndWaitForCallback(t *testing.T, b *browsertest.Browser, downstreamCallbackURL string) { + t.Helper() + env := testlib.IntegrationEnv(t) + + // Expect to be redirected to the upstream provider and log in. + browsertest.LoginToUpstreamGitHub(t, b, env.SupervisorUpstreamGithub) + + // Wait for the login to happen and us be redirected back to a localhost callback. + t.Logf("waiting for redirect to callback") + callbackURLPattern := regexp.MustCompile(`\A` + regexp.QuoteMeta(downstreamCallbackURL) + `\?.+\z`) + b.WaitForURL(t, callbackURLPattern) +} + func requestAuthorizationUsingBrowserAuthcodeFlowOIDC(t *testing.T, _, downstreamAuthorizeURL, downstreamCallbackURL, _, _ string, httpClient *http.Client) { t.Helper() @@ -2892,6 +3023,14 @@ func requestAuthorizationUsingBrowserAuthcodeFlowOIDC(t *testing.T, _, downstrea loginToUpstreamOIDCAndWaitForCallback(t, browser, downstreamCallbackURL) } +func requestAuthorizationUsingBrowserAuthcodeFlowGitHub(t *testing.T, _, downstreamAuthorizeURL, downstreamCallbackURL, _, _ string, httpClient *http.Client) { + t.Helper() + + browser := openBrowserAndNavigateToAuthorizeURL(t, downstreamAuthorizeURL, httpClient) + + loginToUpstreamGitHubAndWaitForCallback(t, browser, downstreamCallbackURL) +} + func requestAuthorizationUsingBrowserAuthcodeFlowOIDCWithIDPChooserPage(t *testing.T, downstreamIssuer, downstreamAuthorizeURL, downstreamCallbackURL, _, _ string, httpClient *http.Client) { t.Helper() diff --git a/test/testlib/browsertest/browsertest.go b/test/testlib/browsertest/browsertest.go index 2cf06a991..33b0415c1 100644 --- a/test/testlib/browsertest/browsertest.go +++ b/test/testlib/browsertest/browsertest.go @@ -380,6 +380,15 @@ func LoginToUpstreamGitHub(t *testing.T, b *Browser, upstream testlib.TestGithub b.SendKeysToFirstMatch(t, passwordSelector, upstream.TestUserPassword) b.ClickFirstMatch(t, loginButtonSelector) + handleGithubOTPLoginPage(t, b, upstream) + + // Keep looping until we get to a page that we do not know how to handle. Then return to allow the test to move on. + for handleOccasionalGithubLoginPage(t, b, upstream) { + continue + } +} + +func handleGithubOTPLoginPage(t *testing.T, b *Browser, upstream testlib.TestGithubUpstream) { // Next, GitHub should go to a new page and prompt for the six digit MFA/OTP code. otpSelector := "input#app_totp" @@ -397,16 +406,11 @@ func LoginToUpstreamGitHub(t *testing.T, b *Browser, upstream testlib.TestGithub // Fill in the OTP code. We do not need to click "verify" because entering the code automatically submits the page. t.Logf("entering GitHub OTP code") b.SendKeysToFirstMatch(t, otpSelector, code) - - // Keep looping until we get to a page that we do not know how to handle. Then return to allow the test to move on. - for handleOccasionalGithubLoginPage(t, b) { - continue - } } // handleOccasionalGithubLoginPage handles the interstitial pages which GitHub might show during a login flow. // None of these will always happen. -func handleOccasionalGithubLoginPage(t *testing.T, b *Browser) bool { +func handleOccasionalGithubLoginPage(t *testing.T, b *Browser, upstream testlib.TestGithubUpstream) bool { t.Helper() t.Log("sleeping for 2 seconds before looking at page title") @@ -456,11 +460,19 @@ func handleOccasionalGithubLoginPage(t *testing.T, b *Browser) bool { b.ClickFirstMatch(t, dontAskAgainLinkSelector) return true + case strings.HasPrefix(lowercaseTitle, "two-factor authentication"): + // Sometimes this happens after the OTP page when we try to use the same OTP code again too quickly. + // GitHub stays on the same page and shows an error banner saying that we used the same code again. + t.Log("sleeping before trying to generate and use a new GitHub OTP code") + time.Sleep(5 * time.Second) // 5 seconds may not be enough time, but if we get the error again then we can try again + handleGithubOTPLoginPage(t, b, upstream) + return true + case strings.HasPrefix(lowercaseTitle, "server error"): // Sometimes this happens after the OTP page. Not sure why. The page has a cute cartoon, but no helpful information. // The URL bar shows https://github.com/sessions/trusted-device for this error page, which is the URL that usually // asks if you want to configure passwordless authentication (aka passkey). - t.Fatal("Got GitHub server error page during login flow. This is not expected, but is unfortunately unrecoverable.") + t.Fatal("Got GitHub server internal error page during login flow. This is not expected, but is unfortunately unrecoverable.") return false // we recognized the title, but we don't know how to handle this page because it has no buttons or other way forward default: