From 417a769e13dd7e484ca7b247b62b715366131363 Mon Sep 17 00:00:00 2001 From: ThumulaPerera <42399179+ThumulaPerera@users.noreply.github.com> Date: Thu, 6 Aug 2026 10:53:54 +0530 Subject: [PATCH] Fix OAuth error propagation --- .../executeEmbeddedSignInFlow.test.ts | 281 ++++++++++++++++++ .../src/api/executeEmbeddedSignInFlow.ts | 108 +++++-- .../src/models/embedded-signin-flow.ts | 6 + .../presentation/auth/SignIn/SignIn.tsx | 10 + 4 files changed, 377 insertions(+), 28 deletions(-) diff --git a/packages/javascript/src/api/__tests__/executeEmbeddedSignInFlow.test.ts b/packages/javascript/src/api/__tests__/executeEmbeddedSignInFlow.test.ts index 25d19472..cd317864 100644 --- a/packages/javascript/src/api/__tests__/executeEmbeddedSignInFlow.test.ts +++ b/packages/javascript/src/api/__tests__/executeEmbeddedSignInFlow.test.ts @@ -18,6 +18,7 @@ import {beforeEach, describe, expect, it, vi} from 'vitest'; import {EmbeddedSignInFlowResponse, EmbeddedSignInFlowStatus} from '../../models/embedded-signin-flow'; +import logger from '../../utils/logger'; import executeEmbeddedSignInFlow from '../executeEmbeddedSignInFlow'; const URL = 'https://localhost:8090/flow/execute'; @@ -171,6 +172,286 @@ describe('executeEmbeddedSignInFlow', (): void => { }); }); + describe('failure relay to the OAuth2 callback', (): void => { + const BASE_URL = 'https://localhost:8090'; + + const mockFlowThenCallback = (flowResponse: unknown, callbackResult: unknown, flowOk = true): void => { + global.fetch = vi + .fn() + .mockResolvedValueOnce({ + json: () => Promise.resolve(flowResponse), + ok: flowOk, + status: flowOk ? 200 : 500, + statusText: flowOk ? 'OK' : 'Internal Server Error', + text: () => Promise.resolve(JSON.stringify(flowResponse)), + }) + .mockResolvedValueOnce({ + json: () => Promise.resolve(callbackResult), + ok: true, + }); + }; + + /** An in-band flow failure whose relay is then rejected by the callback. */ + const mockFlowThenCallbackRejection = (callbackErrorText: string): void => { + global.fetch = vi + .fn() + .mockResolvedValueOnce({ + json: () => + Promise.resolve({errorAssertion: 'signed-error-assertion', flowStatus: EmbeddedSignInFlowStatus.Error}), + ok: true, + }) + .mockResolvedValueOnce({ + ok: false, + status: 400, + statusText: 'Bad Request', + text: () => Promise.resolve(callbackErrorText), + }); + }; + + it('relays an in-band errorAssertion and returns the client redirect', async (): Promise => { + mockFlowThenCallback( + {errorAssertion: 'signed-error-assertion', flowStatus: EmbeddedSignInFlowStatus.Error}, + {redirect_uri: 'https://client.example.com/cb?error=access_denied'}, + ); + + const response = await executeEmbeddedSignInFlow({ + authId: 'auth-1', + baseUrl: BASE_URL, + payload: {action: 'submit', executionId: 'exec-abc'}, + }); + + expect(fetch).toHaveBeenCalledTimes(2); + expect((fetch as ReturnType).mock.calls[1][0]).toBe(`${BASE_URL}/oauth2/auth/callback`); + // The error assertion is relayed in the same field a success assertion uses. + expect(captureRequestBody()).toEqual({assertion: 'signed-error-assertion', authId: 'auth-1'}); + expect(response.flowStatus).toBe(EmbeddedSignInFlowStatus.Error); + expect((response as {redirectUrl?: string}).redirectUrl).toBe( + 'https://client.example.com/cb?error=access_denied', + ); + }); + + it('preserves the original error details when the callback returns no redirect', async (): Promise => { + mockFlowThenCallback( + { + error: {code: 'FET-1066'}, + errorAssertion: 'signed-error-assertion', + executionId: 'exec-abc', + flowStatus: EmbeddedSignInFlowStatus.Error, + }, + {status: 'OK'}, + ); + + const response = await executeEmbeddedSignInFlow({ + authId: 'auth-1', + baseUrl: BASE_URL, + payload: {action: 'submit', executionId: 'exec-abc'}, + }); + + // CIBA callbacks return no redirect, so the caller still needs the flow error to display. + expect((response as {redirectUrl?: string}).redirectUrl).toBeUndefined(); + expect((response as {executionId?: string}).executionId).toBe('exec-abc'); + expect((response as {error?: {code?: string}}).error?.code).toBe('FET-1066'); + }); + + it('relays the errorAssertion carried in a non-OK flow response body', async (): Promise => { + mockFlowThenCallback( + {code: 'FES-1013', errorAssertion: 'signed-error-assertion'}, + {redirect_uri: 'https://client.example.com/cb?error=server_error'}, + false, + ); + + const response = await executeEmbeddedSignInFlow({ + authId: 'auth-1', + baseUrl: BASE_URL, + payload: {action: 'submit', executionId: 'exec-abc'}, + }); + + // The error assertion is relayed in the same field a success assertion uses. + expect(captureRequestBody()).toEqual({assertion: 'signed-error-assertion', authId: 'auth-1'}); + expect((response as {redirectUrl?: string}).redirectUrl).toBe('https://client.example.com/cb?error=server_error'); + }); + + it('throws a generic error when the relay itself fails', async (): Promise => { + global.fetch = vi + .fn() + .mockResolvedValueOnce({ + ok: false, + status: 500, + statusText: 'Internal Server Error', + text: () => Promise.resolve(JSON.stringify({code: 'FES-1013', errorAssertion: 'signed-error-assertion'})), + }) + .mockResolvedValueOnce({ + ok: false, + status: 400, + statusText: 'Bad Request', + text: () => Promise.resolve('callback rejected'), + }); + + // A failed relay leaves the authorization request to expire, so it must surface rather than be + // swallowed. The message stays generic because it is rendered to the end user; the callback's + // own response is logged instead. + await expect( + executeEmbeddedSignInFlow({ + authId: 'auth-1', + baseUrl: BASE_URL, + payload: {action: 'submit', executionId: 'exec-abc'}, + }), + ).rejects.toThrow(/OAuth2 authorization failed/); + }); + + it('does not relay an in-band failure without an authId', async (): Promise => { + global.fetch = vi.fn().mockResolvedValue({ + json: () => + Promise.resolve({errorAssertion: 'signed-error-assertion', flowStatus: EmbeddedSignInFlowStatus.Error}), + ok: true, + }); + + const response = await executeEmbeddedSignInFlow({ + baseUrl: BASE_URL, + payload: {action: 'submit', executionId: 'exec-abc'}, + }); + + expect(fetch).toHaveBeenCalledTimes(1); + expect(response.flowStatus).toBe(EmbeddedSignInFlowStatus.Error); + expect((response as {redirectUrl?: string}).redirectUrl).toBeUndefined(); + }); + + it('does not relay an in-band failure without an errorAssertion', async (): Promise => { + global.fetch = vi.fn().mockResolvedValue({ + json: () => Promise.resolve({flowStatus: EmbeddedSignInFlowStatus.Error}), + ok: true, + }); + + const response = await executeEmbeddedSignInFlow({ + authId: 'auth-1', + baseUrl: BASE_URL, + payload: {action: 'submit', executionId: 'exec-abc'}, + }); + + expect(fetch).toHaveBeenCalledTimes(1); + expect(response.flowStatus).toBe(EmbeddedSignInFlowStatus.Error); + }); + + it('throws as before when a non-OK response carries no errorAssertion', async (): Promise => { + global.fetch = vi.fn().mockResolvedValue({ + ok: false, + status: 400, + statusText: 'Bad Request', + text: () => Promise.resolve('plain failure'), + }); + + await expect( + executeEmbeddedSignInFlow({ + authId: 'auth-1', + baseUrl: BASE_URL, + payload: {action: 'submit', executionId: 'exec-abc'}, + }), + ).rejects.toThrow(/plain failure/); + }); + + // The body of a real 4xx is structured JSON that simply has no assertion, e.g. an expired flow + // context. That reaches readErrorAssertion's non-throwing branch, unlike the plain-text case above. + it('throws the flow error when a JSON body carries no errorAssertion', async (): Promise => { + global.fetch = vi.fn().mockResolvedValue({ + ok: false, + status: 400, + statusText: 'Bad Request', + text: () => Promise.resolve(JSON.stringify({code: 'FES-1004', message: 'Invalid execution id'})), + }); + + await expect( + executeEmbeddedSignInFlow({ + authId: 'auth-1', + baseUrl: BASE_URL, + payload: {action: 'submit', executionId: 'exec-abc'}, + }), + ).rejects.toThrow(/FES-1004/); + // Only the flow request was made; there was nothing to relay. + expect(fetch).toHaveBeenCalledTimes(1); + }); + + // The callback's own response is deliberately kept out of the thrown error because that message is + // rendered to the end user, which makes this log the only surviving record of why a relay failed. + it('logs the callback response when the relay fails', async (): Promise => { + const errorSpy = vi.spyOn(logger, 'error').mockImplementation((): void => {}); + + mockFlowThenCallbackRejection('callback rejected the assertion'); + + await expect( + executeEmbeddedSignInFlow({ + authId: 'auth-1', + baseUrl: BASE_URL, + payload: {action: 'submit', executionId: 'exec-abc'}, + }), + ).rejects.toThrow(/OAuth2 authorization failed/); + + expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining('400'), 'callback rejected the assertion'); + + errorSpy.mockRestore(); + }); + }); + + describe('success relay to the OAuth2 callback', (): void => { + const BASE_URL = 'https://localhost:8090'; + + it('returns the client redirect when the callback accepts the assertion', async (): Promise => { + global.fetch = vi + .fn() + .mockResolvedValueOnce({ + json: () => Promise.resolve({assertion: 'signed-assertion', flowStatus: EmbeddedSignInFlowStatus.Complete}), + ok: true, + }) + .mockResolvedValueOnce({ + json: () => Promise.resolve({redirect_uri: 'https://client.example.com/cb?code=xyz'}), + ok: true, + }); + + const response = await executeEmbeddedSignInFlow({ + authId: 'auth-1', + baseUrl: BASE_URL, + payload: {action: 'submit', executionId: 'exec-abc'}, + }); + + expect(captureRequestBody()).toEqual({assertion: 'signed-assertion', authId: 'auth-1'}); + expect(response.flowStatus).toBe(EmbeddedSignInFlowStatus.Complete); + expect((response as {redirectUrl?: string}).redirectUrl).toBe('https://client.example.com/cb?code=xyz'); + }); + + // Success and failure relays share postAuthCallback, so a rejected callback surfaces the same + // generic message either way, and the callback's own response is never put in front of the user. + it('throws a generic error without the callback body when the callback rejects', async (): Promise => { + const errorSpy = vi.spyOn(logger, 'error').mockImplementation((): void => {}); + + global.fetch = vi + .fn() + .mockResolvedValueOnce({ + json: () => Promise.resolve({assertion: 'signed-assertion', flowStatus: EmbeddedSignInFlowStatus.Complete}), + ok: true, + }) + .mockResolvedValueOnce({ + ok: false, + status: 400, + statusText: 'Bad Request', + text: () => Promise.resolve('assertion not bound to the authorization request'), + }); + + const error: Error = await executeEmbeddedSignInFlow({ + authId: 'auth-1', + baseUrl: BASE_URL, + payload: {action: 'submit', executionId: 'exec-abc'}, + }).catch((err: Error) => err); + + expect(error.message).toBe('OAuth2 authorization failed'); + expect(error.message).not.toContain('not bound'); + expect(errorSpy).toHaveBeenCalledWith( + expect.stringContaining('400'), + 'assertion not bound to the authorization request', + ); + + errorSpy.mockRestore(); + }); + }); + it('throws when payload is missing', async (): Promise => { await expect(executeEmbeddedSignInFlow({url: URL})).rejects.toThrow('Authorization payload is required'); }); diff --git a/packages/javascript/src/api/executeEmbeddedSignInFlow.ts b/packages/javascript/src/api/executeEmbeddedSignInFlow.ts index 6eeb80e3..8d8c2bc7 100644 --- a/packages/javascript/src/api/executeEmbeddedSignInFlow.ts +++ b/packages/javascript/src/api/executeEmbeddedSignInFlow.ts @@ -20,6 +20,71 @@ import ThunderIDAPIError from '../errors/ThunderIDAPIError'; import {EmbeddedFlowExecuteRequestConfig} from '../models/embedded-flow'; import {EmbeddedSignInFlowResponse, EmbeddedSignInFlowStatus} from '../models/embedded-signin-flow'; import injectRequestedPermissions from '../utils/injectRequestedPermissions'; +import logger from '../utils/logger'; + +const readErrorAssertion = (body: string): string | undefined => { + try { + return (JSON.parse(body) as {errorAssertion?: string}).errorAssertion; + } catch { + return undefined; + } +}; + +const postAuthCallback = async ( + baseUrl: string | undefined, + body: Record, + headers: HeadersInit | undefined, +): Promise> => { + const oauth2Response: Response = await fetch(`${baseUrl}/oauth2/auth/callback`, { + body: JSON.stringify(body), + credentials: 'include', + headers: { + Accept: 'application/json', + 'Content-Type': 'application/json', + ...headers, + } as HeadersInit, + method: 'POST', + }); + + if (!oauth2Response.ok) { + const oauth2ErrorText: string = await oauth2Response.text(); + + logger.error( + `OAuth2 authorization failed. The callback responded ` + `${oauth2Response.status} ${oauth2Response.statusText}:`, + oauth2ErrorText, + ); + + throw new ThunderIDAPIError( + 'OAuth2 authorization failed', + 'executeEmbeddedSignInFlow-OAuth2Error-002', + 'javascript', + oauth2Response.status, + oauth2Response.statusText, + ); + } + + return oauth2Response.json(); +}; + +const relayFailure = async ( + baseUrl: string | undefined, + authId: string, + errorAssertion: string, + headers?: HeadersInit, + flowResponse?: EmbeddedSignInFlowResponse, +): Promise => { + const oauth2Result: Record = await postAuthCallback( + baseUrl, + {assertion: errorAssertion, authId}, + headers, + ); + + return { + ...flowResponse, + flowStatus: EmbeddedSignInFlowStatus.Error, + redirectUrl: oauth2Result['redirect_uri'], + } as any; +}; const executeEmbeddedSignInFlow = async ({ url, @@ -86,6 +151,12 @@ const executeEmbeddedSignInFlow = async ({ if (!response.ok) { const errorText: string = await response.text(); + const errorAssertion: string | undefined = readErrorAssertion(errorText); + + if (authId && errorAssertion) { + return await relayFailure(baseUrl, authId, errorAssertion, requestConfig.headers); + } + throw new ThunderIDAPIError( errorText, 'executeEmbeddedSignInFlow-ResponseError-001', @@ -102,34 +173,11 @@ const executeEmbeddedSignInFlow = async ({ // Check if the flow is complete and has an assertion and authId is provided, then call OAuth2 auth callback. if (flowResponse.flowStatus === EmbeddedSignInFlowStatus.Complete && flowResponse.assertion && authId) { try { - const oauth2Response: Response = await fetch(`${baseUrl}/oauth2/auth/callback`, { - body: JSON.stringify({ - assertion: flowResponse.assertion, - authId, - }), - credentials: 'include', - headers: { - Accept: 'application/json', - 'Content-Type': 'application/json', - ...requestConfig.headers, - } as HeadersInit, - method: 'POST', - }); - - if (!oauth2Response.ok) { - const oauth2ErrorText: string = await oauth2Response.text(); - - throw new ThunderIDAPIError( - oauth2ErrorText, - 'executeEmbeddedSignInFlow-OAuth2Error-002', - 'javascript', - oauth2Response.status, - oauth2Response.statusText, - 'OAuth2 authorization failed', - ); - } - - const oauth2Result: Record = await oauth2Response.json(); + const oauth2Result: Record = await postAuthCallback( + baseUrl, + {assertion: flowResponse.assertion, authId}, + requestConfig.headers, + ); return { flowStatus: flowResponse.flowStatus, @@ -151,6 +199,10 @@ const executeEmbeddedSignInFlow = async ({ } } + if (flowResponse.flowStatus === EmbeddedSignInFlowStatus.Error && flowResponse.errorAssertion && authId) { + return relayFailure(baseUrl, authId, flowResponse.errorAssertion, requestConfig.headers, flowResponse); + } + return flowResponse; }; diff --git a/packages/javascript/src/models/embedded-signin-flow.ts b/packages/javascript/src/models/embedded-signin-flow.ts index ea119511..84e184fe 100644 --- a/packages/javascript/src/models/embedded-signin-flow.ts +++ b/packages/javascript/src/models/embedded-signin-flow.ts @@ -173,6 +173,12 @@ export interface EmbeddedSignInFlowResponse extends ExtendedEmbeddedSignInFlowRe */ assertion?: string; + /** + * JWT error assertion returned when the flow terminates in failure on the V2 platform. + * Relayed to the OAuth2 auth callback so the waiting authorization request is failed. + */ + errorAssertion?: string; + /** * Per-step challenge token for replay protection. * Must be included in the next request to continue this flow. diff --git a/packages/react/src/components/presentation/auth/SignIn/SignIn.tsx b/packages/react/src/components/presentation/auth/SignIn/SignIn.tsx index aa23b812..ee9868c7 100644 --- a/packages/react/src/components/presentation/auth/SignIn/SignIn.tsx +++ b/packages/react/src/components/presentation/auth/SignIn/SignIn.tsx @@ -450,6 +450,16 @@ const SignIn: FC = ({ */ const handleTerminalResponse = async (response: EmbeddedSignInFlowResponse): Promise => { if (response.flowStatus === EmbeddedSignInFlowStatus.Error) { + // Propagate OAuth error to the client + if (response.redirectUrl && window?.location) { + setIsSubmitting(false); + await clearFlowState(); + cleanupOAuthUrlParams(true); + window.location.href = response.redirectUrl; + + return true; + } + if (response.executionId) { // Recoverable: session still alive. Show inline error without firing onError. setExecutionId(response.executionId);