From 23bca1459fa9f0187ed1eb2067de4acb5c5ba62c Mon Sep 17 00:00:00 2001 From: Vijay Budhram Date: Wed, 10 Jun 2026 13:42:14 -0400 Subject: [PATCH] feat(settings): redesign app error dialog with Sentry error ID Because: * The app error dialog needed to surface a Sentry error ID and present a clearer, redesigned UI for unexpected failures. This commit: * Redesigns AppErrorDialog to display the captured Sentry event ID. * Adds a functional test covering the app error view for an invalid query parameter and folds the 404 test into a shared errorViews spec. * Adds supporting model hook and OAuth web integration unit tests. --- .../tests/misc/errorViews.spec.ts | 40 ++++++++ .../tests/misc/fourOhFour.spec.ts | 19 ---- .../components/AppErrorBoundary/index.tsx | 17 +++- .../components/AppErrorDialog/en.ftl | 8 +- .../components/AppErrorDialog/index.test.tsx | 71 ++++++++++++++- .../components/AppErrorDialog/index.tsx | 74 +++++++++++---- .../components/ErrorBoundaries/index.test.tsx | 73 +++++++++++++++ .../src/components/ErrorBoundaries/index.tsx | 36 ++++++-- .../src/components/LegalWithMarkdown/en.ftl | 2 + .../components/OAuthDataError/index.test.tsx | 70 ++++++++++++++ .../src/components/OAuthDataError/index.tsx | 37 +++++++- .../src/lib/oauth/oauth-errors.ts | 3 + .../fxa-settings/src/models/hooks.test.ts | 40 ++++++++ .../oauth-web-integration.test.ts | 91 +++++++++++++++++++ .../integrations/oauth-web-integration.ts | 2 +- 15 files changed, 524 insertions(+), 59 deletions(-) create mode 100644 packages/functional-tests/tests/misc/errorViews.spec.ts delete mode 100644 packages/functional-tests/tests/misc/fourOhFour.spec.ts create mode 100644 packages/fxa-settings/src/components/ErrorBoundaries/index.test.tsx create mode 100644 packages/fxa-settings/src/components/OAuthDataError/index.test.tsx diff --git a/packages/functional-tests/tests/misc/errorViews.spec.ts b/packages/functional-tests/tests/misc/errorViews.spec.ts new file mode 100644 index 00000000000..29775ae3f8a --- /dev/null +++ b/packages/functional-tests/tests/misc/errorViews.spec.ts @@ -0,0 +1,40 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +import { expect, test } from '../../lib/fixtures/standard'; + +test.describe('error views', () => { + test('404 view links back to signin', async ({ + page, + pages: { signin, fourOhFour }, + }) => { + await fourOhFour.goto('load'); + await expect(fourOhFour.header).toBeVisible(); + await expect(fourOhFour.homeLink).toBeVisible(); + await fourOhFour.homeLink.click(); + await page.waitForLoadState(); + await expect(signin.emailFirstHeading).toBeVisible(); + }); + + test('app error view renders for an invalid query parameter', async ({ + target, + page, + pages: { signin }, + }) => { + // A malformed redirect_to fails query-parameter validation, which the app + // surfaces through the redesigned AppErrorDialog instead of the signin form. + const response = await page.goto( + `${target.contentServerUrl}/?redirect_to=javascript:alert(1)` + ); + + // The WAF may block the request (Fastly returns 406) before it reaches the + // app; that already prevents the bad parameter, so there is no view to check. + if (response && response.status() === 406) { + return; + } + + await expect(signin.badRequestHeading).toBeVisible(); + await expect(signin.emailFirstHeading).toBeHidden(); + }); +}); diff --git a/packages/functional-tests/tests/misc/fourOhFour.spec.ts b/packages/functional-tests/tests/misc/fourOhFour.spec.ts deleted file mode 100644 index c9526f00eaa..00000000000 --- a/packages/functional-tests/tests/misc/fourOhFour.spec.ts +++ /dev/null @@ -1,19 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -import { expect, test } from '../../lib/fixtures/standard'; - -test.describe('404', () => { - test('visit an invalid page', async ({ - page, - pages: { signin, fourOhFour }, - }) => { - await fourOhFour.goto('load'); - await expect(fourOhFour.header).toBeVisible(); - await expect(fourOhFour.homeLink).toBeVisible(); - await fourOhFour.homeLink.click(); - await page.waitForLoadState(); - await expect(signin.emailFirstHeading).toBeVisible(); - }); -}); diff --git a/packages/fxa-react/components/AppErrorBoundary/index.tsx b/packages/fxa-react/components/AppErrorBoundary/index.tsx index f0690a2e05b..74e7f5a4f53 100644 --- a/packages/fxa-react/components/AppErrorBoundary/index.tsx +++ b/packages/fxa-react/components/AppErrorBoundary/index.tsx @@ -12,6 +12,7 @@ type AppErrorBoundaryProps = { type AppErrorBoundaryState = { error: Error | undefined; + sentryEventId: string | undefined; }; class AppErrorBoundary extends React.Component< @@ -20,11 +21,12 @@ class AppErrorBoundary extends React.Component< > { state: { error: undefined | Error; + sentryEventId: undefined | string; }; constructor(props: AppErrorBoundaryProps) { super(props); - this.state = { error: undefined }; + this.state = { error: undefined, sentryEventId: undefined }; } static getDerivedStateFromError(error: Error) { @@ -33,12 +35,19 @@ class AppErrorBoundary extends React.Component< componentDidCatch(error: Error) { console.error('AppError', error); - Sentry.captureException(error, { tags: { source: 'AppErrorBoundary' } }); + const sentryEventId = Sentry.captureException(error, { + tags: { source: 'AppErrorBoundary' }, + }); + this.setState({ sentryEventId }); } render() { - const { error } = this.state; - return error ? : this.props.children; + const { error, sentryEventId } = this.state; + return error ? ( + + ) : ( + this.props.children + ); } } diff --git a/packages/fxa-react/components/AppErrorDialog/en.ftl b/packages/fxa-react/components/AppErrorDialog/en.ftl index 99fdaf7e756..41af999ffed 100644 --- a/packages/fxa-react/components/AppErrorDialog/en.ftl +++ b/packages/fxa-react/components/AppErrorDialog/en.ftl @@ -1,7 +1,11 @@ ## FxA React - Strings shared between multiple FxA products for application error dialog -app-general-err-heading = General application error -app-general-err-message = Something went wrong. Please try again later. +app-something-went-wrong-heading = Something went wrong +app-something-went-wrong-message = We’ve been notified of the issue. Refresh the page to try again. +# $errorId (String) - Unique identifier for the error report, used to look it up in our monitoring system +app-error-id = Error ID: { $errorId } +# Expandable toggle that reveals technical details about the error +app-error-details-summary = Error details # Specific handling for issues when bad or missing query parameters are detected app-query-parameter-err-heading = Bad Request: Invalid Query Parameters diff --git a/packages/fxa-react/components/AppErrorDialog/index.test.tsx b/packages/fxa-react/components/AppErrorDialog/index.test.tsx index 9efecf99f06..77cb418c32b 100644 --- a/packages/fxa-react/components/AppErrorDialog/index.test.tsx +++ b/packages/fxa-react/components/AppErrorDialog/index.test.tsx @@ -3,15 +3,78 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ import React from 'react'; +import { screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import AppErrorDialog from '.'; import { renderWithLocalizationProvider } from '../../lib/test-utils/localizationProvider'; +const MOCK_SENTRY_EVENT_ID = '0a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d'; + describe('AppErrorDialog', () => { - it('renders a general error dialog', () => { - const { queryByTestId } = renderWithLocalizationProvider( - + it('renders the "Something went wrong" heading for the general error type', () => { + renderWithLocalizationProvider(); + + expect( + screen.getByRole('heading', { name: 'Something went wrong' }) + ).toBeInTheDocument(); + }); + + it('renders the invalid query parameters heading for the query-parameter-violation error type', () => { + renderWithLocalizationProvider( + + ); + + expect( + screen.getByRole('heading', { + name: 'Bad Request: Invalid Query Parameters', + }) + ).toBeInTheDocument(); + }); + + it('omits the "we\'ve been notified" message for the query-parameter-violation type', () => { + renderWithLocalizationProvider( + + ); + + expect( + screen.queryByText(/we’ve been notified of the issue/i) + ).not.toBeInTheDocument(); + }); + + it('shows the Sentry error ID when provided', () => { + renderWithLocalizationProvider( + + ); + + expect( + screen.getByText(`Error ID: ${MOCK_SENTRY_EVENT_ID}`) + ).toBeInTheDocument(); + }); + + it('reveals the error message when the error details toggle is expanded', async () => { + const user = userEvent.setup(); + renderWithLocalizationProvider( + ); - expect(queryByTestId('error-loading-app')).toBeInTheDocument(); + expect(screen.getByText('Invalid redirect parameter')).not.toBeVisible(); + + await user.click(screen.getByText('Error details')); + + expect(screen.getByText('Invalid redirect parameter')).toBeVisible(); + }); + + it('includes errno and code in the details when the error provides them', async () => { + const user = userEvent.setup(); + const error = Object.assign(new Error('Invalid redirect parameter'), { + errno: 102, + code: 400, + }); + renderWithLocalizationProvider(); + + await user.click(screen.getByText('Error details')); + + expect(screen.getByText(/errno: 102/)).toBeVisible(); + expect(screen.getByText(/code: 400/)).toBeVisible(); }); }); diff --git a/packages/fxa-react/components/AppErrorDialog/index.tsx b/packages/fxa-react/components/AppErrorDialog/index.tsx index 713ef206ebf..062e27c27d8 100644 --- a/packages/fxa-react/components/AppErrorDialog/index.tsx +++ b/packages/fxa-react/components/AppErrorDialog/index.tsx @@ -7,28 +7,45 @@ import { Localized } from '@fluent/react'; export type ErrorType = 'general' | 'query-parameter-violation'; -const AppErrorDialog = ({ errorType }: { errorType?: ErrorType }) => { - if (errorType == null) { - errorType = 'general'; - } +type AppErrorDialogProps = { + errorType?: ErrorType; + /** The caught error; its message shows under the details toggle. */ + error?: Error; + /** Sentry event ID from captureException; search Sentry for id:. */ + sentryEventId?: string; +}; + +const AppErrorDialog = ({ + errorType = 'general', + error, + sentryEventId, +}: AppErrorDialogProps) => { + // Auth UI errors carry errno/code; include them since some share a message. + const { errno, code } = (error ?? {}) as { errno?: number; code?: number }; + const errorDetails = [ + error?.message, + errno != null && `errno: ${errno}`, + code != null && `code: ${code}`, + ] + .filter(Boolean) + .join('\n'); return ( -
-
- {errorType === 'general' && ( - +
+
+ {errorType === 'general' ? ( +

- General application error + Something went wrong

- )} - {errorType === 'query-parameter-violation' && ( + ) : (

Bad Request: Invalid Query Parameters @@ -37,14 +54,35 @@ const AppErrorDialog = ({ errorType }: { errorType?: ErrorType }) => { )} {errorType === 'general' && ( - -

- Something went wrong. Please try again later. + +

+ We’ve been notified of the issue. Refresh the page to try again. +

+ + )} + + {sentryEventId && ( + +

+ {`Error ID: ${sentryEventId}`}

)} -

-
+ + {errorDetails && ( +
+ + + Error details + + + 
+              {errorDetails}
+
+
+ )} + + ); }; diff --git a/packages/fxa-settings/src/components/ErrorBoundaries/index.test.tsx b/packages/fxa-settings/src/components/ErrorBoundaries/index.test.tsx new file mode 100644 index 00000000000..3fc6949ed1c --- /dev/null +++ b/packages/fxa-settings/src/components/ErrorBoundaries/index.test.tsx @@ -0,0 +1,73 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +import React from 'react'; +import { screen } from '@testing-library/react'; +import * as Sentry from '@sentry/browser'; +import { renderWithLocalizationProvider } from 'fxa-react/lib/test-utils/localizationProvider'; +import { AppErrorBoundary } from '.'; +import { ModelValidationErrors } from '../../lib/model-data'; + +jest.mock('@sentry/browser', () => ({ + captureException: jest.fn(), +})); + +const MOCK_SENTRY_EVENT_ID = '0a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d'; + +// Error boundaries log the thrown error; keep test output clean. +window.console.error = jest.fn(); + +const BadComponent = ({ error }: { error: Error }) => { + throw error; +}; + +describe('AppErrorBoundary', () => { + beforeEach(() => { + jest.clearAllMocks(); + (Sentry.captureException as jest.Mock).mockReturnValue( + MOCK_SENTRY_EVENT_ID + ); + }); + + it('renders children when no error occurs', () => { + renderWithLocalizationProvider( + +

all good

+ + ); + + expect(screen.getByText('all good')).toBeInTheDocument(); + }); + + it('catches a thrown error and displays the Sentry event ID', () => { + renderWithLocalizationProvider( + + + + ); + + expect( + screen.getByRole('heading', { name: 'Something went wrong' }) + ).toBeInTheDocument(); + expect( + screen.getByText(`Error ID: ${MOCK_SENTRY_EVENT_ID}`) + ).toBeInTheDocument(); + }); + + it('renders the invalid query parameters heading for ModelValidationErrors', () => { + renderWithLocalizationProvider( + + + + ); + + expect( + screen.getByRole('heading', { + name: 'Bad Request: Invalid Query Parameters', + }) + ).toBeInTheDocument(); + }); +}); diff --git a/packages/fxa-settings/src/components/ErrorBoundaries/index.tsx b/packages/fxa-settings/src/components/ErrorBoundaries/index.tsx index bec52d8cf56..1889cc84884 100644 --- a/packages/fxa-settings/src/components/ErrorBoundaries/index.tsx +++ b/packages/fxa-settings/src/components/ErrorBoundaries/index.tsx @@ -13,11 +13,12 @@ import AppErrorDialog from 'fxa-react/components/AppErrorDialog'; export class AppErrorBoundary extends React.Component<{ children: ReactNode }> { state: { error?: Error; + sentryEventId?: string; }; constructor(props: { children: React.ReactNode }) { super(props); - this.state = { error: undefined }; + this.state = { error: undefined, sentryEventId: undefined }; } static getDerivedStateFromError(error: Error) { @@ -25,32 +26,51 @@ export class AppErrorBoundary extends React.Component<{ children: ReactNode }> { } componentDidCatch(error: Error) { + let sentryEventId: string; if (error instanceof ModelValidationErrors) { console.error('Model Validation errors encountered', error); - Sentry.captureException(error, { + sentryEventId = Sentry.captureException(error, { tags: { source: 'AppErrorBoundary', condition: error.condition }, }); } else { console.error('AppError', error); - Sentry.captureException(error, { tags: { source: 'AppErrorBoundary' } }); + sentryEventId = Sentry.captureException(error, { + tags: { source: 'AppErrorBoundary' }, + }); } - this.setState({ error }); + this.setState({ error, sentryEventId }); } render() { if (this.state.error) { - return ; + return ( + + ); } return this.props.children; } } -export const AppError = ({ error }: { error?: Error }) => { +export const AppError = ({ + error, + sentryEventId, +}: { + error?: Error; + sentryEventId?: string; +}) => { // Special handling for validation errors if (error instanceof ModelValidationErrors) { - return ; + return ( + + ); } - return ; + return ; }; diff --git a/packages/fxa-settings/src/components/LegalWithMarkdown/en.ftl b/packages/fxa-settings/src/components/LegalWithMarkdown/en.ftl index 0c314171d4b..124ccfc3827 100644 --- a/packages/fxa-settings/src/components/LegalWithMarkdown/en.ftl +++ b/packages/fxa-settings/src/components/LegalWithMarkdown/en.ftl @@ -1,2 +1,4 @@ # Back button on legal/terms or legal/privacy that takes users to the previous page legal-back-button = Back +# Generic error shown when the legal document fails to load +app-general-err-message = Something went wrong. Please try again later. diff --git a/packages/fxa-settings/src/components/OAuthDataError/index.test.tsx b/packages/fxa-settings/src/components/OAuthDataError/index.test.tsx new file mode 100644 index 00000000000..7cc5d76600c --- /dev/null +++ b/packages/fxa-settings/src/components/OAuthDataError/index.test.tsx @@ -0,0 +1,70 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +import React from 'react'; +import { screen } from '@testing-library/react'; +import * as Sentry from '@sentry/browser'; +import { renderWithLocalizationProvider } from 'fxa-react/lib/test-utils/localizationProvider'; +import OAuthDataError from '.'; + +jest.mock('@sentry/browser', () => ({ + captureException: jest.fn(), +})); + +jest.mock('../../models', () => ({ + useFtlMsgResolver: () => ({ + getMsg: (_id: string, fallback: string) => fallback, + }), +})); + +jest.mock('../../models/hooks', () => ({ + useConfig: jest.fn(() => ({ featureFlags: {} })), + useFtlMsgResolver: () => ({ + getMsg: (_id: string, fallback: string) => fallback, + }), +})); + +const MOCK_AUTH_ERROR = { message: 'Unexpected error', errno: 999 }; +const MOCK_SENTRY_EVENT_ID = '0a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d'; + +describe('OAuthDataError', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('renders the error page and displays the captured Sentry event ID', () => { + (Sentry.captureException as jest.Mock).mockReturnValue( + MOCK_SENTRY_EVENT_ID + ); + + // Local copy: the component stamps sentryEventId onto the error. + renderWithLocalizationProvider( + + ); + + expect( + screen.getByRole('heading', { name: 'Bad Request' }) + ).toBeInTheDocument(); + expect(screen.getByText('Unexpected error')).toBeInTheDocument(); + expect(Sentry.captureException).toHaveBeenCalledWith(expect.any(Error), { + tags: { source: 'OAuthDataError', errno: 999 }, + }); + expect( + screen.getByText(`Error ID: ${MOCK_SENTRY_EVENT_ID}`) + ).toBeInTheDocument(); + }); + + it('reuses the event ID from an error already captured at its throw site', () => { + renderWithLocalizationProvider( + + ); + + expect(Sentry.captureException).not.toHaveBeenCalled(); + expect( + screen.getByText(`Error ID: ${MOCK_SENTRY_EVENT_ID}`) + ).toBeInTheDocument(); + }); +}); diff --git a/packages/fxa-settings/src/components/OAuthDataError/index.tsx b/packages/fxa-settings/src/components/OAuthDataError/index.tsx index b5dabc98b61..924e98bb75a 100644 --- a/packages/fxa-settings/src/components/OAuthDataError/index.tsx +++ b/packages/fxa-settings/src/components/OAuthDataError/index.tsx @@ -2,7 +2,9 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -import React, { useEffect } from 'react'; +import React, { useEffect, useState } from 'react'; +import * as Sentry from '@sentry/browser'; +import { FtlMsg } from 'fxa-react/lib/utils'; import { getLocalizedErrorMessage } from '../../lib/error-utils'; import { AuthError } from '../../lib/oauth'; import { useFtlMsgResolver } from '../../models'; @@ -10,13 +12,35 @@ import AppLayout from '../AppLayout'; import Banner from '../Banner'; import CardHeader from '../CardHeader'; -const OAuthDataError = ({ error, gleanMetric }: { error: AuthError, gleanMetric?: (data: { event: { reason: string } }) => void }) => { +const OAuthDataError = ({ + error, + gleanMetric, +}: { + error: AuthError; + gleanMetric?: (data: { event: { reason: string } }) => void; +}) => { const ftlMsgResolver = useFtlMsgResolver(); + // Prefer the ID from the original capture site. + const [sentryEventId, setSentryEventId] = useState(error.sentryEventId); + + useEffect(() => { + // Capture only errors not already reported at their throw site, so one + // error never emits two events. Stamp the id onto the error so a remount + // (e.g. StrictMode) reuses it instead of capturing again. + if (error.sentryEventId) { + return; + } + const eventId = Sentry.captureException(new Error(error.message), { + tags: { source: 'OAuthDataError', errno: error.errno }, + }); + error.sentryEventId = eventId; + setSentryEventId(eventId); + }, [error]); useEffect(() => { if (gleanMetric) { - gleanMetric({ event: { reason: error.errno.toString() }}); + gleanMetric({ event: { reason: error.errno.toString() } }); } }, [gleanMetric, error.errno]); @@ -37,6 +61,13 @@ const OAuthDataError = ({ error, gleanMetric }: { error: AuthError, gleanMetric? : error.message, }} /> + {sentryEventId && ( + +

+ {`Error ID: ${sentryEventId}`} +

+ + )} ); }; diff --git a/packages/fxa-settings/src/lib/oauth/oauth-errors.ts b/packages/fxa-settings/src/lib/oauth/oauth-errors.ts index b59cc5412b3..1bd2a3cb837 100644 --- a/packages/fxa-settings/src/lib/oauth/oauth-errors.ts +++ b/packages/fxa-settings/src/lib/oauth/oauth-errors.ts @@ -11,6 +11,8 @@ export type AuthError = { response_error_code?: string; interpolate?: boolean; version?: number; + /** Sentry event ID when the error was already captured at its throw site. */ + sentryEventId?: string; }; export const UNEXPECTED_ERROR = 'Unexpected error'; @@ -141,6 +143,7 @@ export const OAUTH_ERRORS: Record = { export class OAuthError extends Error { public errno: number; public response_error_code?: string; + public sentryEventId?: string; constructor( public readonly error: string | number, diff --git a/packages/fxa-settings/src/models/hooks.test.ts b/packages/fxa-settings/src/models/hooks.test.ts index a2bc89cdc3c..753be8d5a01 100644 --- a/packages/fxa-settings/src/models/hooks.test.ts +++ b/packages/fxa-settings/src/models/hooks.test.ts @@ -670,6 +670,46 @@ describe('useClientInfoState', () => { expect(Sentry.captureException).toHaveBeenCalledTimes(1); }); + it('reports loading on the very first render when a fetch will run (race regression)', async () => { + mockUrlQueryData.get.mockImplementation((key: string) => + key === 'client_id' ? '1234567890abcdef' : null + ); + + let resolveFetch!: (value: unknown) => void; + mockFetch.mockReturnValue( + new Promise((resolve) => { + resolveFetch = resolve; + }) + ); + + // A settled-empty render (no loading/data/error) is the race that fed + // useIntegration an empty clientInfo before the fetch ran. + const settledEmptyRenders: boolean[] = []; + const { result } = renderHook( + () => { + const state = useClientInfoState(); + settledEmptyRenders.push(!state.loading && !state.data && !state.error); + return state; + }, + { wrapper: MockAppProvider } + ); + + resolveFetch({ + ok: true, + json: jest.fn().mockResolvedValue({ + id: '1234567890abcdef', + name: 'Test RP', + redirect_uri: 'https://example.test/redirect', + image_uri: '', + trusted: true, + }), + }); + + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.data?.clientInfo.trusted).toBe(true); + expect(settledEmptyRenders).not.toContain(true); + }); + it('surfaces an Invalid clientId error and does not fetch when client_id is missing', async () => { mockUrlQueryData.get.mockReturnValue(null); const { isHexadecimal, length } = require('class-validator'); diff --git a/packages/fxa-settings/src/models/integrations/oauth-web-integration.test.ts b/packages/fxa-settings/src/models/integrations/oauth-web-integration.test.ts index ae3b0dbb881..ed8ca91957b 100644 --- a/packages/fxa-settings/src/models/integrations/oauth-web-integration.test.ts +++ b/packages/fxa-settings/src/models/integrations/oauth-web-integration.test.ts @@ -256,6 +256,97 @@ describe('models/integrations/oauth-relier', function () { }); }); + describe('clientInfo never loaded (race or skipped fetch)', () => { + // What IntegrationFactory.initClientInfo produces when useClientInfoState + // had no data: only redirectUri is set, defaulting to ''. + const huskClientInfo = { + clientId: undefined, + imageUri: undefined, + serviceName: undefined, + redirectUri: '', + trusted: undefined, + }; + + const loadedClientInfo = { + clientId: 'a2270f727f45f648', + imageUri: undefined, + serviceName: 'Firefox', + redirectUri: 'urn:ietf:wg:oauth:2.0:oob:oauth-redirect-webchannel', + trusted: true, + }; + + // The scopes Fenix IP Protection (VPN) requests on first sign-in. + const VPN_AUTH_SCOPE = + 'profile https://identity.mozilla.com/apps/vpn https://identity.mozilla.com/apps/oldsync'; + + function getIntegration( + clientInfo: typeof huskClientInfo | typeof loadedClientInfo, + dataOverrides: Record = {} + ) { + const integration = new OAuthWebIntegration( + new GenericData({ scope: VPN_AUTH_SCOPE, ...dataOverrides }), + new GenericData({}), + { + scopedKeysEnabled: true, + scopedKeysValidation: { + 'https://identity.mozilla.com/apps/oldsync': { + redirectUris: [ + 'urn:ietf:wg:oauth:2.0:oob:oauth-redirect-webchannel', + ], + }, + }, + isPromptNoneEnabled: true, + isPromptNoneEnabledClientIds: [], + } + ); + integration.clientInfo = clientInfo; + return integration; + } + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('throws invalid-scope for a valid VPN/sync request when clientInfo is the empty husk', () => { + const integration = getIntegration(huskClientInfo); + + // trusted=undefined reads as untrusted and strips every requested scope. + expect(() => integration.getServiceName()).toThrow(OAuthError); + + const capturedError = (Sentry.captureException as jest.Mock).mock + .calls[0][0]; + expect(capturedError.errno).toBe(OAUTH_ERRORS.INVALID_PARAMETER.errno); + }); + + it('does not throw for the same request once clientInfo is loaded', () => { + const integration = getIntegration(loadedClientInfo); + + expect(integration.getServiceName()).toBe('Firefox Sync'); + expect(Sentry.captureException).not.toHaveBeenCalled(); + }); + + it('throws "Invalid redirect parameter" for a keys request when clientInfo is the empty husk', () => { + const integration = getIntegration(huskClientInfo, { + keys_jwk: 'fakeJwk', + }); + + // The husk's '' redirectUri never matches the scoped-keys allowlist. + expect(() => integration.wantsKeys()).toThrow( + 'Invalid redirect parameter' + ); + expect(Sentry.captureMessage).toHaveBeenCalled(); + }); + + it('wants keys for the same request once clientInfo is loaded', () => { + const integration = getIntegration(loadedClientInfo, { + keys_jwk: 'fakeJwk', + }); + + expect(integration.wantsKeys()).toBe(true); + expect(Sentry.captureMessage).not.toHaveBeenCalled(); + }); + }); + describe('trusted reliers that do not ask for consent', () => { function getIntegration(scope: string) { const integration = getIntegrationWithScope(scope); diff --git a/packages/fxa-settings/src/models/integrations/oauth-web-integration.ts b/packages/fxa-settings/src/models/integrations/oauth-web-integration.ts index 94401584606..1ae5a408003 100644 --- a/packages/fxa-settings/src/models/integrations/oauth-web-integration.ts +++ b/packages/fxa-settings/src/models/integrations/oauth-web-integration.ts @@ -240,7 +240,7 @@ export class OAuthWebIntegration extends GenericIntegration< service: this.data.service || 'undefined', clientInfo: this.clientInfo || 'undefined', }; - Sentry.captureException(err, { + err.sentryEventId = Sentry.captureException(err, { tags: { area: 'OAuthWebIntegration.getPermissions', clientId: this.data.clientId,