Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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,112 @@ 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,
})
);
});

it('blocks decisions for the previous account while switching and after logout', async () => {
const delay = Promise.withResolvers<void>();
MockApiClient.addMockResponse({
Expand Down
29 changes: 29 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 Down Expand Up @@ -187,6 +203,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