diff --git a/static/app/views/authV2/oauth/queries.ts b/static/app/views/authV2/oauth/queries.ts index a500200bdb99..cd797fddefb3 100644 --- a/static/app/views/authV2/oauth/queries.ts +++ b/static/app/views/authV2/oauth/queries.ts @@ -8,6 +8,7 @@ import type { DeviceConsent, DeviceContext, OAuthContext, + OAuthReauthentication, OAuthRedirect, } from './types'; @@ -60,10 +61,9 @@ export function authorizationConsentQueryOptions( query: AuthorizationParameters, sessionKey: string ) { - const options = apiOptions.as()( - '/auth/oauth/authorize/', - {query, staleTime: 0, allowAuthError: true} - ); + const options = apiOptions.as< + AuthorizationConsent | OAuthReauthentication | OAuthRedirect + >()('/auth/oauth/authorize/', {query, staleTime: 0, allowAuthError: true}); const queryKey: ApiQueryKey = options.queryKey; return { ...options, diff --git a/static/app/views/authV2/oauth/types.ts b/static/app/views/authV2/oauth/types.ts index 012ffd448850..d1b83b08f1f3 100644 --- a/static/app/views/authV2/oauth/types.ts +++ b/static/app/views/authV2/oauth/types.ts @@ -27,6 +27,11 @@ export interface OAuthRedirect { stage: 'redirect'; } +/** prompt=login: the session must sign in again before consent is prepared. */ +export interface OAuthReauthentication { + stage: 'reauthenticate'; +} + export interface OAuthOrganization { id: string; name: string; diff --git a/static/app/views/authV2/oauthAuthorize/index.spec.tsx b/static/app/views/authV2/oauthAuthorize/index.spec.tsx index e844543ef265..dbdbe6ab8498 100644 --- a/static/app/views/authV2/oauthAuthorize/index.spec.tsx +++ b/static/app/views/authV2/oauthAuthorize/index.spec.tsx @@ -126,6 +126,52 @@ describe('OAuthAuthorize', () => { } ); + it('asks for a fresh sign-in instead of the existing session for prompt=login', async () => { + MockApiClient.addMockResponse({ + url: '/auth/oauth/authorize/', + body: {stage: 'reauthenticate'}, + }); + const logout = MockApiClient.addMockResponse({ + url: '/auth/', + method: 'DELETE', + body: {}, + }); + MockApiClient.addMockResponse({url: '/auth-v2/csrf/', body: {}}); + MockApiClient.addMockResponse({ + url: '/auth/config/', + body: { + pendingMfa: null, + serverHostname: 'sentry.example.com', + canRegister: false, + hasNewsletter: false, + }, + }); + const approve = MockApiClient.addMockResponse({ + url: '/auth/oauth/authorize/', + method: 'POST', + body: {}, + }); + render(, { + initialRouterConfig: { + location: { + pathname: location.pathname, + query: { + ...Object.fromEntries(new URLSearchParams(location.search)), + prompt: 'login', + }, + }, + }, + }); + + expect( + await screen.findByText('Sign in to authorize this application…') + ).toBeVisible(); + expect(screen.getByRole('textbox', {name: 'Email'})).toBeVisible(); + expect(screen.queryByRole('button', {name: 'Authorize'})).not.toBeInTheDocument(); + expect(logout).toHaveBeenCalledTimes(1); + expect(approve).not.toHaveBeenCalled(); + }); + it('posts denial and follows the server callback', async () => { const deny = MockApiClient.addMockResponse({ url: '/auth/oauth/authorize/', diff --git a/static/app/views/authV2/oauthAuthorize/useOAuthAuthorizeFlow.spec.tsx b/static/app/views/authV2/oauthAuthorize/useOAuthAuthorizeFlow.spec.tsx index 4be892cc20d7..0e01fa545dab 100644 --- a/static/app/views/authV2/oauthAuthorize/useOAuthAuthorizeFlow.spec.tsx +++ b/static/app/views/authV2/oauthAuthorize/useOAuthAuthorizeFlow.spec.tsx @@ -199,6 +199,138 @@ describe('useOAuthAuthorizeFlow', () => { } ); + it('forwards the OIDC prompt to context and consent preparation', async () => { + const context = MockApiClient.addMockResponse({ + url: '/auth/oauth/authorize/context/', + body: OAuthContextFixture(), + match: [MockApiClient.matchQuery({prompt: 'select_account consent'})], + }); + const consent = MockApiClient.addMockResponse({ + url: '/auth/oauth/authorize/', + body: OAuthConsentFixture(), + match: [MockApiClient.matchQuery({prompt: 'select_account consent'})], + }); + const {result} = renderHookWithProviders(useOAuthAuthorizeFlow, { + initialRouterConfig: { + location: { + ...location, + query: {...location.query, prompt: 'select_account consent'}, + }, + }, + }); + + await waitFor(() => expect(result.current.state.status).toBe('consent')); + expect(context).toHaveBeenCalledTimes(1); + expect(consent).toHaveBeenCalledTimes(1); + }); + + it('signs out and prepares consent again after prompt=login reauthentication', async () => { + const logout = MockApiClient.addMockResponse({ + url: '/auth/', + method: 'DELETE', + body: {}, + }); + MockApiClient.addMockResponse({url: '/auth-v2/csrf/', body: {}}); + MockApiClient.addMockResponse({url: '/auth/config/', body: loginConfig}); + MockApiClient.addMockResponse({ + url: '/auth/oauth/authorize/', + body: {stage: 'reauthenticate'}, + }); + const decide = MockApiClient.addMockResponse({ + url: '/auth/oauth/authorize/', + method: 'POST', + body: {}, + }); + const {result} = renderHookWithProviders(useOAuthAuthorizeFlow, { + initialRouterConfig: { + location: {...location, query: {...location.query, prompt: 'login'}}, + }, + }); + + await waitFor(() => + expect(result.current.state).toMatchObject({ + status: 'authentication', + authConfig: loginConfig, + }) + ); + expect(logout).toHaveBeenCalledTimes(1); + expect(decide).not.toHaveBeenCalled(); + + const consent = OAuthConsentFixture({ + transactionId: 'reauthenticated-transaction', + hasExistingConsent: true, + }); + MockApiClient.addMockResponse({url: '/auth/oauth/authorize/', body: consent}); + const user = UserFixture({id: '2'}); + + act(() => { + result.current.actions.onAuthenticated({user, nextUri: '/home/'}, loginConfig); + }); + + await waitFor(() => + expect(decide).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + data: {transactionId: 'reauthenticated-transaction', op: 'approve'}, + }) + ) + ); + expect(logout).toHaveBeenCalledTimes(1); + }); + + it('offers a retry when prompt=login cannot sign out', async () => { + MockApiClient.addMockResponse({ + url: '/auth/', + method: 'DELETE', + statusCode: 503, + body: {detail: 'Sign out unavailable.'}, + }); + MockApiClient.addMockResponse({ + url: '/auth/oauth/authorize/', + body: {stage: 'reauthenticate'}, + }); + const {result} = renderHookWithProviders(useOAuthAuthorizeFlow, { + initialRouterConfig: { + location: {...location, query: {...location.query, prompt: 'login'}}, + }, + }); + + await waitFor(() => + expect(result.current.state).toMatchObject({ + status: 'error', + message: 'Sign out unavailable.', + canRetry: true, + switchError: null, + }) + ); + + const consentDelay = Promise.withResolvers(); + MockApiClient.addMockResponse({ + url: '/auth/oauth/authorize/', + body: {stage: 'reauthenticate'}, + asyncDelay: consentDelay.promise, + }); + const logout = MockApiClient.addMockResponse({ + url: '/auth/', + method: 'DELETE', + body: {}, + }); + MockApiClient.addMockResponse({url: '/auth-v2/csrf/', body: {}}); + MockApiClient.addMockResponse({url: '/auth/config/', body: loginConfig}); + + act(() => result.current.actions.retry()); + + expect(result.current.state).toMatchObject({status: 'loading', switchError: null}); + + await act(() => { + consentDelay.resolve(); + return consentDelay.promise; + }); + + await waitFor(() => expect(result.current.state.status).toBe('authentication')); + expect(logout).toHaveBeenCalledTimes(1); + }); + it('blocks decisions for the previous account while switching and after logout', async () => { const delay = Promise.withResolvers(); MockApiClient.addMockResponse({ diff --git a/static/app/views/authV2/oauthAuthorize/useOAuthAuthorizeFlow.ts b/static/app/views/authV2/oauthAuthorize/useOAuthAuthorizeFlow.ts index 62d5ba0db294..ecf808bae27a 100644 --- a/static/app/views/authV2/oauthAuthorize/useOAuthAuthorizeFlow.ts +++ b/static/app/views/authV2/oauthAuthorize/useOAuthAuthorizeFlow.ts @@ -28,6 +28,7 @@ const authorizationParsers = { state: parseAsString, code_challenge: parseAsString, code_challenge_method: parseAsString, + prompt: parseAsString, }; export function useOAuthAuthorizeFlow() { @@ -97,6 +98,21 @@ export function useOAuthAuthorizeFlow() { mutate({transactionId: consent.transactionId, op: 'approve'}); }, [autoApprove, consent, mutate, session.switchAccount.isPending]); + // prompt=login: sign out so the existing session cannot satisfy reauthentication. + // Signing in starts a new session, which prepares consent again. + const reauthenticate = consent?.stage === 'reauthenticate'; + const reauthenticatedSession = useRef(null); + const {mutate: signOut} = session.switchAccount; + + useEffect(() => { + if (!reauthenticate || reauthenticatedSession.current === session.sessionKey) { + return; + } + + reauthenticatedSession.current = session.sessionKey; + signOut(); + }, [reauthenticate, session.sessionKey, signOut]); + // Consent belongs to the current account. Block decisions during account switching // so a transaction prepared for the previous account cannot be submitted. const actions = { @@ -119,6 +135,8 @@ export function useOAuthAuthorizeFlow() { retry: () => { decision.reset(); + // A failed prompt=login sign-out must not resurface before the next attempt. + session.switchAccount.reset(); session.refresh(); void contextQuery.refetch(); }, @@ -187,6 +205,19 @@ export function useOAuthAuthorizeFlow() { }; } + if (reauthenticate && session.switchAccount.isError) { + return { + state: { + ...shared, + switchError: null, + status: 'error', + message: getRequestErrorUserMessage(session.switchAccount.error), + canRetry: true, + } as const, + actions, + }; + } + if (consent?.stage !== 'consent') { return {state: {...shared, status: 'loading'} as const, actions}; }