Skip to content

Commit bba005d

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

4 files changed

Lines changed: 382 additions & 28 deletions

File tree

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

Lines changed: 285 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,290 @@ 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(
272+
'https://client.example.com/cb?error=server_error',
273+
);
274+
});
275+
276+
it('throws a generic error when the relay itself fails', async (): Promise<void> => {
277+
global.fetch = vi
278+
.fn()
279+
.mockResolvedValueOnce({
280+
ok: false,
281+
status: 500,
282+
statusText: 'Internal Server Error',
283+
text: () => Promise.resolve(JSON.stringify({code: 'FES-1013', errorAssertion: 'signed-error-assertion'})),
284+
})
285+
.mockResolvedValueOnce({
286+
ok: false,
287+
status: 400,
288+
statusText: 'Bad Request',
289+
text: () => Promise.resolve('callback rejected'),
290+
});
291+
292+
// A failed relay leaves the authorization request to expire, so it must surface rather than be
293+
// swallowed. The message stays generic because it is rendered to the end user; the callback's
294+
// own response is logged instead.
295+
await expect(
296+
executeEmbeddedSignInFlow({
297+
authId: 'auth-1',
298+
baseUrl: BASE_URL,
299+
payload: {action: 'submit', executionId: 'exec-abc'},
300+
}),
301+
).rejects.toThrow(/OAuth2 authorization failed/);
302+
});
303+
304+
it('does not relay an in-band failure without an authId', async (): Promise<void> => {
305+
global.fetch = vi.fn().mockResolvedValue({
306+
json: () =>
307+
Promise.resolve({errorAssertion: 'signed-error-assertion', flowStatus: EmbeddedSignInFlowStatus.Error}),
308+
ok: true,
309+
});
310+
311+
const response = await executeEmbeddedSignInFlow({
312+
baseUrl: BASE_URL,
313+
payload: {action: 'submit', executionId: 'exec-abc'},
314+
});
315+
316+
expect(fetch).toHaveBeenCalledTimes(1);
317+
expect(response.flowStatus).toBe(EmbeddedSignInFlowStatus.Error);
318+
expect((response as {redirectUrl?: string}).redirectUrl).toBeUndefined();
319+
});
320+
321+
it('does not relay an in-band failure without an errorAssertion', async (): Promise<void> => {
322+
global.fetch = vi.fn().mockResolvedValue({
323+
json: () => Promise.resolve({flowStatus: EmbeddedSignInFlowStatus.Error}),
324+
ok: true,
325+
});
326+
327+
const response = await executeEmbeddedSignInFlow({
328+
authId: 'auth-1',
329+
baseUrl: BASE_URL,
330+
payload: {action: 'submit', executionId: 'exec-abc'},
331+
});
332+
333+
expect(fetch).toHaveBeenCalledTimes(1);
334+
expect(response.flowStatus).toBe(EmbeddedSignInFlowStatus.Error);
335+
});
336+
337+
it('throws as before when a non-OK response carries no errorAssertion', async (): Promise<void> => {
338+
global.fetch = vi.fn().mockResolvedValue({
339+
ok: false,
340+
status: 400,
341+
statusText: 'Bad Request',
342+
text: () => Promise.resolve('plain failure'),
343+
});
344+
345+
await expect(
346+
executeEmbeddedSignInFlow({
347+
authId: 'auth-1',
348+
baseUrl: BASE_URL,
349+
payload: {action: 'submit', executionId: 'exec-abc'},
350+
}),
351+
).rejects.toThrow(/plain failure/);
352+
});
353+
354+
// The body of a real 4xx is structured JSON that simply has no assertion, e.g. an expired flow
355+
// context. That reaches readErrorAssertion's non-throwing branch, unlike the plain-text case above.
356+
it('throws the flow error when a JSON body carries no errorAssertion', async (): Promise<void> => {
357+
global.fetch = vi.fn().mockResolvedValue({
358+
ok: false,
359+
status: 400,
360+
statusText: 'Bad Request',
361+
text: () => Promise.resolve(JSON.stringify({code: 'FES-1004', message: 'Invalid execution id'})),
362+
});
363+
364+
await expect(
365+
executeEmbeddedSignInFlow({
366+
authId: 'auth-1',
367+
baseUrl: BASE_URL,
368+
payload: {action: 'submit', executionId: 'exec-abc'},
369+
}),
370+
).rejects.toThrow(/FES-1004/);
371+
// Only the flow request was made; there was nothing to relay.
372+
expect(fetch).toHaveBeenCalledTimes(1);
373+
});
374+
375+
// The callback's own response is deliberately kept out of the thrown error because that message is
376+
// rendered to the end user, which makes this log the only surviving record of why a relay failed.
377+
it('logs the callback response when the relay fails', async (): Promise<void> => {
378+
const errorSpy = vi.spyOn(logger, 'error').mockImplementation((): void => {});
379+
380+
mockFlowThenCallbackRejection('callback rejected the assertion');
381+
382+
await expect(
383+
executeEmbeddedSignInFlow({
384+
authId: 'auth-1',
385+
baseUrl: BASE_URL,
386+
payload: {action: 'submit', executionId: 'exec-abc'},
387+
}),
388+
).rejects.toThrow(/OAuth2 authorization failed/);
389+
390+
expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining('400'), 'callback rejected the assertion');
391+
392+
errorSpy.mockRestore();
393+
});
394+
});
395+
396+
describe('success relay to the OAuth2 callback', (): void => {
397+
const BASE_URL = 'https://localhost:8090';
398+
399+
it('returns the client redirect when the callback accepts the assertion', async (): Promise<void> => {
400+
global.fetch = vi
401+
.fn()
402+
.mockResolvedValueOnce({
403+
json: () =>
404+
Promise.resolve({assertion: 'signed-assertion', flowStatus: EmbeddedSignInFlowStatus.Complete}),
405+
ok: true,
406+
})
407+
.mockResolvedValueOnce({
408+
json: () => Promise.resolve({redirect_uri: 'https://client.example.com/cb?code=xyz'}),
409+
ok: true,
410+
});
411+
412+
const response = await executeEmbeddedSignInFlow({
413+
authId: 'auth-1',
414+
baseUrl: BASE_URL,
415+
payload: {action: 'submit', executionId: 'exec-abc'},
416+
});
417+
418+
expect(captureRequestBody()).toEqual({assertion: 'signed-assertion', authId: 'auth-1'});
419+
expect(response.flowStatus).toBe(EmbeddedSignInFlowStatus.Complete);
420+
expect((response as {redirectUrl?: string}).redirectUrl).toBe('https://client.example.com/cb?code=xyz');
421+
});
422+
423+
// Success and failure relays share postAuthCallback, so a rejected callback surfaces the same
424+
// generic message either way, and the callback's own response is never put in front of the user.
425+
it('throws a generic error without the callback body when the callback rejects', async (): Promise<void> => {
426+
const errorSpy = vi.spyOn(logger, 'error').mockImplementation((): void => {});
427+
428+
global.fetch = vi
429+
.fn()
430+
.mockResolvedValueOnce({
431+
json: () =>
432+
Promise.resolve({assertion: 'signed-assertion', flowStatus: EmbeddedSignInFlowStatus.Complete}),
433+
ok: true,
434+
})
435+
.mockResolvedValueOnce({
436+
ok: false,
437+
status: 400,
438+
statusText: 'Bad Request',
439+
text: () => Promise.resolve('assertion not bound to the authorization request'),
440+
});
441+
442+
const error: Error = await executeEmbeddedSignInFlow({
443+
authId: 'auth-1',
444+
baseUrl: BASE_URL,
445+
payload: {action: 'submit', executionId: 'exec-abc'},
446+
}).catch((err: Error) => err);
447+
448+
expect(error.message).toBe('OAuth2 authorization failed');
449+
expect(error.message).not.toContain('not bound');
450+
expect(errorSpy).toHaveBeenCalledWith(
451+
expect.stringContaining('400'),
452+
'assertion not bound to the authorization request',
453+
);
454+
455+
errorSpy.mockRestore();
456+
});
457+
});
458+
174459
it('throws when payload is missing', async (): Promise<void> => {
175460
await expect(executeEmbeddedSignInFlow({url: URL})).rejects.toThrow('Authorization payload is required');
176461
});

0 commit comments

Comments
 (0)