Skip to content

Commit

Permalink
Check if access requests are enabled before redirecting to auth provi…
Browse files Browse the repository at this point in the history
…der (#62376)
  • Loading branch information
pjlast committed May 2, 2024
1 parent 269e301 commit a217a6a
Show file tree
Hide file tree
Showing 4 changed files with 144 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ All notable changes to Sourcegraph are documented in this file.
- Pinned code intel popovers and popovers opened via the keyboard are properly shown again. [#61966](https://github.com/sourcegraph/sourcegraph/pull/61966)
- Syntax highlighting works correctly for JSX files. [#62027](https://github.com/sourcegraph/sourcegraph/pull/62027)
- Changesets with a skipped CI check are now incorrectly displayed in the Batch Changes UI. [#62204](https://github.com/sourcegraph/sourcegraph/pull/62204)
- Fixed the Sourcegraph login page auto-redirecting to the single auth provider when request access is enabled. [#62376](https://github.com/sourcegraph/sourcegraph/pull/62376)

## 5.3.12303

Expand Down
41 changes: 41 additions & 0 deletions client/web/src/auth/SignInPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ describe('SignInPage', () => {
sourcegraphDotComMode?: SourcegraphContext['sourcegraphDotComMode']
allowSignup?: SourcegraphContext['allowSignup']
primaryLoginProvidersCount?: SourcegraphContext['primaryLoginProvidersCount']
authAccessRequest?: SourcegraphContext['authAccessRequest']
}
) =>
renderWithBrandedContext(
Expand All @@ -78,6 +79,7 @@ describe('SignInPage', () => {
resetPasswordEnabled: true,
xhrHeaders: {},
primaryLoginProvidersCount: props.primaryLoginProvidersCount ?? 5,
authAccessRequest: props.authAccessRequest,
}}
telemetryRecorder={noOpTelemetryRecorder}
/>
Expand Down Expand Up @@ -228,6 +230,45 @@ describe('SignInPage', () => {
expect(render('/sign-in', { authenticatedUser: mockUser }).asFragment()).toMatchSnapshot()
})

it('renders redirect when there is only 1 auth provider', () => {
const withGitHubProvider: SourcegraphContext['authProviders'] = [
{
serviceType: 'github',
displayName: 'GitHub',
isBuiltin: false,
authenticationURL: 'http://localhost/.auth/gitlab/login?pc=f00bar&returnTo=%2Fsearch',
serviceID: 'https://github.com',
clientID: '1234',
noSignIn: false,
},
]

expect(render('/sign-in', { authProviders: withGitHubProvider }).asFragment()).toMatchSnapshot()
})

it('does not render redirect when there is only 1 auth provider with request access enabled', () => {
const withGitHubProvider: SourcegraphContext['authProviders'] = [
{
serviceType: 'github',
displayName: 'GitHub',
isBuiltin: false,
authenticationURL: 'http://localhost/.auth/gitlab/login?pc=f00bar&returnTo=%2Fsearch',
serviceID: 'https://github.com',
clientID: '1234',
noSignIn: false,
},
]

expect(
render('/sign-in', {
authProviders: withGitHubProvider,
authAccessRequest: { enabled: true },
allowSignup: false,
sourcegraphDotComMode: false,
}).asFragment()
).toMatchSnapshot()
})

it('renders different prefix on provider buttons', () => {
const providers = authProviders.map(provider => ({ ...provider, displayPrefix: 'Just login through' }))

Expand Down
4 changes: 2 additions & 2 deletions client/web/src/auth/SignInPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ export const SignInPage: React.FunctionComponent<React.PropsWithChildren<SignInP
}

const thirdPartyAuthProviders = nonBuiltinAuthProviders.filter(provider => shouldShowProvider(provider))
// If there is only one auth provider that is going to be displayed, we want to redirect to it directly.
if (thirdPartyAuthProviders.length === 1 && !builtInAuthProvider) {
// If there is only one auth provider that is going to be displayed, and request access is disabled, we want to redirect to the auth provider directly.
if (thirdPartyAuthProviders.length === 1 && !builtInAuthProvider && !isRequestAccessAllowed) {
// Add '?returnTo=' + encodeURIComponent(returnTo) to thirdPartyAuthProviders[0].authenticationURL in a safe way.
const redirectUrl = new URL(thirdPartyAuthProviders[0].authenticationURL, window.location.href)
if (returnTo) {
Expand Down
100 changes: 100 additions & 0 deletions client/web/src/auth/__snapshots__/SignInPage.test.tsx.snap

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit a217a6a

Please sign in to comment.