Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions static/app/views/authV2/oauth/queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import type {
DeviceConsent,
DeviceContext,
OAuthContext,
OAuthReauthentication,
OAuthRedirect,
} from './types';

Expand Down Expand Up @@ -60,10 +61,9 @@ export function authorizationConsentQueryOptions(
query: AuthorizationParameters,
sessionKey: string
) {
const options = apiOptions.as<AuthorizationConsent | OAuthRedirect>()(
'/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,
Expand Down
5 changes: 5 additions & 0 deletions static/app/views/authV2/oauth/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
46 changes: 46 additions & 0 deletions static/app/views/authV2/oauthAuthorize/index.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(<OAuthAuthorize />, {
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/',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>();
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<void>();
MockApiClient.addMockResponse({
Expand Down
31 changes: 31 additions & 0 deletions static/app/views/authV2/oauthAuthorize/useOAuthAuthorizeFlow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const authorizationParsers = {
state: parseAsString,
code_challenge: parseAsString,
code_challenge_method: parseAsString,
prompt: parseAsString,
};

export function useOAuthAuthorizeFlow() {
Expand Down Expand Up @@ -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<string | null>(null);
const {mutate: signOut} = session.switchAccount;

useEffect(() => {
if (!reauthenticate || reauthenticatedSession.current === session.sessionKey) {
return;
}

reauthenticatedSession.current = session.sessionKey;
signOut();
}, [reauthenticate, session.sessionKey, signOut]);

Comment thread
sentry-junior[bot] marked this conversation as resolved.
Resolved
// Consent belongs to the current account. Block decisions during account switching
// so a transaction prepared for the previous account cannot be submitted.
const actions = {
Expand All @@ -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();
},
Expand Down Expand Up @@ -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};
}
Expand Down
Loading