Skip to content

Commit 417a769

Browse files
committed
Fix OAuth error propagation
1 parent 5d3f8da commit 417a769

4 files changed

Lines changed: 377 additions & 28 deletions

File tree

packages/javascript/src/api/__tests__/executeEmbeddedSignInFlow.test.ts

Lines changed: 281 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import {beforeEach, describe, expect, it, vi} from 'vitest';
2020
import {EmbeddedSignInFlowResponse, EmbeddedSignInFlowStatus} from '../../models/embedded-signin-flow';
21+
import logger from '../../utils/logger';
2122
import executeEmbeddedSignInFlow from '../executeEmbeddedSignInFlow';
2223

2324
const URL = 'https://localhost:8090/flow/execute';
@@ -171,6 +172,286 @@ describe('executeEmbeddedSignInFlow', (): void => {
171172
});
172173
});
173174

175+
describe('failure relay to the OAuth2 callback', (): void => {
176+
const BASE_URL = 'https://localhost:8090';
177+
178+
const mockFlowThenCallback = (flowResponse: unknown, callbackResult: unknown, flowOk = true): void => {
179+
global.fetch = vi
180+
.fn()
181+
.mockResolvedValueOnce({
182+
json: () => Promise.resolve(flowResponse),
183+
ok: flowOk,
184+
status: flowOk ? 200 : 500,
185+
statusText: flowOk ? 'OK' : 'Internal Server Error',
186+
text: () => Promise.resolve(JSON.stringify(flowResponse)),
187+
})
188+
.mockResolvedValueOnce({
189+
json: () => Promise.resolve(callbackResult),
190+
ok: true,
191+
});
192+
};
193+
194+
/** An in-band flow failure whose relay is then rejected by the callback. */
195+
const mockFlowThenCallbackRejection = (callbackErrorText: string): void => {
196+
global.fetch = vi
197+
.fn()
198+
.mockResolvedValueOnce({
199+
json: () =>
200+
Promise.resolve({errorAssertion: 'signed-error-assertion', flowStatus: EmbeddedSignInFlowStatus.Error}),
201+
ok: true,
202+
})
203+
.mockResolvedValueOnce({
204+
ok: false,
205+
status: 400,
206+
statusText: 'Bad Request',
207+
text: () => Promise.resolve(callbackErrorText),
208+
});
209+
};
210+
211+
it('relays an in-band errorAssertion and returns the client redirect', async (): Promise<void> => {
212+
mockFlowThenCallback(
213+
{errorAssertion: 'signed-error-assertion', flowStatus: EmbeddedSignInFlowStatus.Error},
214+
{redirect_uri: 'https://client.example.com/cb?error=access_denied'},
215+
);
216+
217+
const response = await executeEmbeddedSignInFlow({
218+
authId: 'auth-1',
219+
baseUrl: BASE_URL,
220+
payload: {action: 'submit', executionId: 'exec-abc'},
221+
});
222+
223+
expect(fetch).toHaveBeenCalledTimes(2);
224+
expect((fetch as ReturnType<typeof vi.fn>).mock.calls[1][0]).toBe(`${BASE_URL}/oauth2/auth/callback`);
225+
// The error assertion is relayed in the same field a success assertion uses.
226+
expect(captureRequestBody()).toEqual({assertion: 'signed-error-assertion', authId: 'auth-1'});
227+
expect(response.flowStatus).toBe(EmbeddedSignInFlowStatus.Error);
228+
expect((response as {redirectUrl?: string}).redirectUrl).toBe(
229+
'https://client.example.com/cb?error=access_denied',
230+
);
231+
});
232+
233+
it('preserves the original error details when the callback returns no redirect', async (): Promise<void> => {
234+
mockFlowThenCallback(
235+
{
236+
error: {code: 'FET-1066'},
237+
errorAssertion: 'signed-error-assertion',
238+
executionId: 'exec-abc',
239+
flowStatus: EmbeddedSignInFlowStatus.Error,
240+
},
241+
{status: 'OK'},
242+
);
243+
244+
const response = await executeEmbeddedSignInFlow({
245+
authId: 'auth-1',
246+
baseUrl: BASE_URL,
247+
payload: {action: 'submit', executionId: 'exec-abc'},
248+
});
249+
250+
// CIBA callbacks return no redirect, so the caller still needs the flow error to display.
251+
expect((response as {redirectUrl?: string}).redirectUrl).toBeUndefined();
252+
expect((response as {executionId?: string}).executionId).toBe('exec-abc');
253+
expect((response as {error?: {code?: string}}).error?.code).toBe('FET-1066');
254+
});
255+
256+
it('relays the errorAssertion carried in a non-OK flow response body', async (): Promise<void> => {
257+
mockFlowThenCallback(
258+
{code: 'FES-1013', errorAssertion: 'signed-error-assertion'},
259+
{redirect_uri: 'https://client.example.com/cb?error=server_error'},
260+
false,
261+
);
262+
263+
const response = await executeEmbeddedSignInFlow({
264+
authId: 'auth-1',
265+
baseUrl: BASE_URL,
266+
payload: {action: 'submit', executionId: 'exec-abc'},
267+
});
268+
269+
// The error assertion is relayed in the same field a success assertion uses.
270+
expect(captureRequestBody()).toEqual({assertion: 'signed-error-assertion', authId: 'auth-1'});
271+
expect((response as {redirectUrl?: string}).redirectUrl).toBe('https://client.example.com/cb?error=server_error');
272+
});
273+
274+
it('throws a generic error when the relay itself fails', async (): Promise<void> => {
275+
global.fetch = vi
276+
.fn()
277+
.mockResolvedValueOnce({
278+
ok: false,
279+
status: 500,
280+
statusText: 'Internal Server Error',
281+
text: () => Promise.resolve(JSON.stringify({code: 'FES-1013', errorAssertion: 'signed-error-assertion'})),
282+
})
283+
.mockResolvedValueOnce({
284+
ok: false,
285+
status: 400,
286+
statusText: 'Bad Request',
287+
text: () => Promise.resolve('callback rejected'),
288+
});
289+
290+
// A failed relay leaves the authorization request to expire, so it must surface rather than be
291+
// swallowed. The message stays generic because it is rendered to the end user; the callback's
292+
// own response is logged instead.
293+
await expect(
294+
executeEmbeddedSignInFlow({
295+
authId: 'auth-1',
296+
baseUrl: BASE_URL,
297+
payload: {action: 'submit', executionId: 'exec-abc'},
298+
}),
299+
).rejects.toThrow(/OAuth2 authorization failed/);
300+
});
301+
302+
it('does not relay an in-band failure without an authId', async (): Promise<void> => {
303+
global.fetch = vi.fn().mockResolvedValue({
304+
json: () =>
305+
Promise.resolve({errorAssertion: 'signed-error-assertion', flowStatus: EmbeddedSignInFlowStatus.Error}),
306+
ok: true,
307+
});
308+
309+
const response = await executeEmbeddedSignInFlow({
310+
baseUrl: BASE_URL,
311+
payload: {action: 'submit', executionId: 'exec-abc'},
312+
});
313+
314+
expect(fetch).toHaveBeenCalledTimes(1);
315+
expect(response.flowStatus).toBe(EmbeddedSignInFlowStatus.Error);
316+
expect((response as {redirectUrl?: string}).redirectUrl).toBeUndefined();
317+
});
318+
319+
it('does not relay an in-band failure without an errorAssertion', async (): Promise<void> => {
320+
global.fetch = vi.fn().mockResolvedValue({
321+
json: () => Promise.resolve({flowStatus: EmbeddedSignInFlowStatus.Error}),
322+
ok: true,
323+
});
324+
325+
const response = await executeEmbeddedSignInFlow({
326+
authId: 'auth-1',
327+
baseUrl: BASE_URL,
328+
payload: {action: 'submit', executionId: 'exec-abc'},
329+
});
330+
331+
expect(fetch).toHaveBeenCalledTimes(1);
332+
expect(response.flowStatus).toBe(EmbeddedSignInFlowStatus.Error);
333+
});
334+
335+
it('throws as before when a non-OK response carries no errorAssertion', async (): Promise<void> => {
336+
global.fetch = vi.fn().mockResolvedValue({
337+
ok: false,
338+
status: 400,
339+
statusText: 'Bad Request',
340+
text: () => Promise.resolve('plain failure'),
341+
});
342+
343+
await expect(
344+
executeEmbeddedSignInFlow({
345+
authId: 'auth-1',
346+
baseUrl: BASE_URL,
347+
payload: {action: 'submit', executionId: 'exec-abc'},
348+
}),
349+
).rejects.toThrow(/plain failure/);
350+
});
351+
352+
// The body of a real 4xx is structured JSON that simply has no assertion, e.g. an expired flow
353+
// context. That reaches readErrorAssertion's non-throwing branch, unlike the plain-text case above.
354+
it('throws the flow error when a JSON body carries no errorAssertion', async (): Promise<void> => {
355+
global.fetch = vi.fn().mockResolvedValue({
356+
ok: false,
357+
status: 400,
358+
statusText: 'Bad Request',
359+
text: () => Promise.resolve(JSON.stringify({code: 'FES-1004', message: 'Invalid execution id'})),
360+
});
361+
362+
await expect(
363+
executeEmbeddedSignInFlow({
364+
authId: 'auth-1',
365+
baseUrl: BASE_URL,
366+
payload: {action: 'submit', executionId: 'exec-abc'},
367+
}),
368+
).rejects.toThrow(/FES-1004/);
369+
// Only the flow request was made; there was nothing to relay.
370+
expect(fetch).toHaveBeenCalledTimes(1);
371+
});
372+
373+
// The callback's own response is deliberately kept out of the thrown error because that message is
374+
// rendered to the end user, which makes this log the only surviving record of why a relay failed.
375+
it('logs the callback response when the relay fails', async (): Promise<void> => {
376+
const errorSpy = vi.spyOn(logger, 'error').mockImplementation((): void => {});
377+
378+
mockFlowThenCallbackRejection('callback rejected the assertion');
379+
380+
await expect(
381+
executeEmbeddedSignInFlow({
382+
authId: 'auth-1',
383+
baseUrl: BASE_URL,
384+
payload: {action: 'submit', executionId: 'exec-abc'},
385+
}),
386+
).rejects.toThrow(/OAuth2 authorization failed/);
387+
388+
expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining('400'), 'callback rejected the assertion');
389+
390+
errorSpy.mockRestore();
391+
});
392+
});
393+
394+
describe('success relay to the OAuth2 callback', (): void => {
395+
const BASE_URL = 'https://localhost:8090';
396+
397+
it('returns the client redirect when the callback accepts the assertion', async (): Promise<void> => {
398+
global.fetch = vi
399+
.fn()
400+
.mockResolvedValueOnce({
401+
json: () => Promise.resolve({assertion: 'signed-assertion', flowStatus: EmbeddedSignInFlowStatus.Complete}),
402+
ok: true,
403+
})
404+
.mockResolvedValueOnce({
405+
json: () => Promise.resolve({redirect_uri: 'https://client.example.com/cb?code=xyz'}),
406+
ok: true,
407+
});
408+
409+
const response = await executeEmbeddedSignInFlow({
410+
authId: 'auth-1',
411+
baseUrl: BASE_URL,
412+
payload: {action: 'submit', executionId: 'exec-abc'},
413+
});
414+
415+
expect(captureRequestBody()).toEqual({assertion: 'signed-assertion', authId: 'auth-1'});
416+
expect(response.flowStatus).toBe(EmbeddedSignInFlowStatus.Complete);
417+
expect((response as {redirectUrl?: string}).redirectUrl).toBe('https://client.example.com/cb?code=xyz');
418+
});
419+
420+
// Success and failure relays share postAuthCallback, so a rejected callback surfaces the same
421+
// generic message either way, and the callback's own response is never put in front of the user.
422+
it('throws a generic error without the callback body when the callback rejects', async (): Promise<void> => {
423+
const errorSpy = vi.spyOn(logger, 'error').mockImplementation((): void => {});
424+
425+
global.fetch = vi
426+
.fn()
427+
.mockResolvedValueOnce({
428+
json: () => Promise.resolve({assertion: 'signed-assertion', flowStatus: EmbeddedSignInFlowStatus.Complete}),
429+
ok: true,
430+
})
431+
.mockResolvedValueOnce({
432+
ok: false,
433+
status: 400,
434+
statusText: 'Bad Request',
435+
text: () => Promise.resolve('assertion not bound to the authorization request'),
436+
});
437+
438+
const error: Error = await executeEmbeddedSignInFlow({
439+
authId: 'auth-1',
440+
baseUrl: BASE_URL,
441+
payload: {action: 'submit', executionId: 'exec-abc'},
442+
}).catch((err: Error) => err);
443+
444+
expect(error.message).toBe('OAuth2 authorization failed');
445+
expect(error.message).not.toContain('not bound');
446+
expect(errorSpy).toHaveBeenCalledWith(
447+
expect.stringContaining('400'),
448+
'assertion not bound to the authorization request',
449+
);
450+
451+
errorSpy.mockRestore();
452+
});
453+
});
454+
174455
it('throws when payload is missing', async (): Promise<void> => {
175456
await expect(executeEmbeddedSignInFlow({url: URL})).rejects.toThrow('Authorization payload is required');
176457
});

0 commit comments

Comments
 (0)