diff --git a/packages/fxa-settings/src/components/App/index.test.tsx b/packages/fxa-settings/src/components/App/index.test.tsx index 7e4e15044cc..2c844bed175 100644 --- a/packages/fxa-settings/src/components/App/index.test.tsx +++ b/packages/fxa-settings/src/components/App/index.test.tsx @@ -35,7 +35,7 @@ import { MozServices } from '../../lib/types'; import mockUseFxAStatus from '../../lib/hooks/useFxAStatus/mocks'; import useFxAStatus from '../../lib/hooks/useFxAStatus'; import sentryMetrics from 'fxa-shared/sentry/browser'; -import { OAuthError } from '../../lib/oauth'; +import { OAuthError, OAUTH_ERRORS } from '../../lib/oauth'; jest.mock('../../lib/hooks/useFxAStatus', () => ({ __esModule: true, @@ -698,6 +698,47 @@ describe('Integration serviceName error handling', () => { expect(screen.getByText(mockError.message)).toBeInTheDocument(); }); + it('shows OAuthDataError immediately when OAuth client info load failed, before metrics/sign-in resolve', async () => { + // App init not yet complete: metrics still loading and sign-in undetermined. + // The fail-fast path must surface the error without waiting on these. + (useInitialMetricsQueryState as jest.Mock).mockReturnValue({ + loading: true, + }); + (useLocalSignedInQueryState as jest.Mock).mockReturnValue({ + data: undefined, + }); + + const mockError = new OAuthError(OAUTH_ERRORS.SERVICE_UNAVAILABLE.errno); + const mockOAuthIntegration = { + type: IntegrationType.OAuthWeb, + clientInfoLoadFailed: true, + isSync: jest.fn().mockReturnValue(false), + isFirefoxClientServiceRelay: jest.fn().mockReturnValue(false), + checkClientInfo: jest.fn().mockImplementation(() => { + throw mockError; + }), + getClientId: jest.fn(), + data: {}, + getCmsInfo: jest.fn(), + getLegalTerms: jest.fn(), + }; + + (useIntegration as jest.Mock).mockReturnValue(mockOAuthIntegration); + + await act(async () => { + renderWithLocalizationProvider( + + + + ); + }); + + expect(screen.getByText('Bad Request')).toBeInTheDocument(); + expect(screen.getByText(mockError.message)).toBeInTheDocument(); + }); + it('throws error when non-OAuth integration', async () => { const mockError = new Error('Non-OAuth integration error'); const mockNonOAuthIntegration = { diff --git a/packages/fxa-settings/src/components/App/index.tsx b/packages/fxa-settings/src/components/App/index.tsx index f12aa04436d..71cf03d0087 100644 --- a/packages/fxa-settings/src/components/App/index.tsx +++ b/packages/fxa-settings/src/components/App/index.tsx @@ -383,6 +383,25 @@ export const App = ({ metricsEnabled, ]); + // Fail fast: if the OAuth client-info fetch in useClientInfoState exhausted its + // retries, surface the user-facing error immediately rather than waiting downstream + // failures to occur. + if ( + integration && + isOAuthIntegration(integration) && + integration.clientInfoLoadFailed + ) { + try { + integration.checkClientInfo(); + } catch (err: any) { + return ( + }> + + + ); + } + } + // Wait until app initialization is complete if ( metricsLoading || diff --git a/packages/fxa-settings/src/models/hooks.test.ts b/packages/fxa-settings/src/models/hooks.test.ts index ea881f8f230..a2bc89cdc3c 100644 --- a/packages/fxa-settings/src/models/hooks.test.ts +++ b/packages/fxa-settings/src/models/hooks.test.ts @@ -670,7 +670,7 @@ describe('useClientInfoState', () => { expect(Sentry.captureException).toHaveBeenCalledTimes(1); }); - it('does not fetch when client_id is missing', async () => { + 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'); isHexadecimal.mockReturnValue(false); @@ -682,7 +682,11 @@ describe('useClientInfoState', () => { expect(mockFetch).not.toHaveBeenCalled(); expect(result.current.loading).toBe(false); - expect(result.current.error).toBeUndefined(); + // Fail fast: a missing/invalid clientId is surfaced as an error so the + // App-level guard can render OAuthDataError rather than proceeding with + // half-populated client info. This is a breadcrumb, not a Sentry capture. + expect(result.current.error).toBeInstanceOf(Error); + expect(result.current.error?.message).toBe('Invalid clientId'); expect(Sentry.captureException).not.toHaveBeenCalled(); }); }); diff --git a/packages/fxa-settings/src/models/hooks.ts b/packages/fxa-settings/src/models/hooks.ts index d6d678a7d04..a000c3c4b75 100644 --- a/packages/fxa-settings/src/models/hooks.ts +++ b/packages/fxa-settings/src/models/hooks.ts @@ -287,13 +287,20 @@ export function useClientInfoState() { useEffect(() => { let mounted = true; - if (!isValidClientId || !config) { + if (!config) { Sentry.addBreadcrumb({ - message: `OAuth Client - Invalid state for fetching clientInfo`, + message: `OAuth Client - Missing config`, category: 'useClientInfoState.fetch', - data: { isValidClientId, hasConfig:!!config } }); - setState((prev) => ({ ...prev, loading: false })); + setState((prev) => ({ ...prev, loading: false, error: new Error('Missing config') })); + return; + } + if (!isValidClientId) { + Sentry.addBreadcrumb({ + message: `OAuth Client - Invalid clientId`, + category: 'useClientInfoState.fetch', + }); + setState((prev) => ({ ...prev, loading: false, error: new Error('Invalid clientId') })); return; } 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 32eb26fd27f..ae3b0dbb881 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 @@ -298,6 +298,69 @@ describe('models/integrations/oauth-relier', function () { }); }); + describe('checkClientInfo', () => { + // 16-byte hex clientId, matching the shape real OAuth clients use. + const MOCK_CLIENT_ID = 'a1b2c3d4e5f6789012345678901234567890abcd'; + + function getIntegration() { + return new OAuthWebIntegration( + new GenericData({ scope: 'profile' }), + new GenericData({}), + { + scopedKeysEnabled: true, + scopedKeysValidation: {}, + isPromptNoneEnabled: true, + isPromptNoneEnabledClientIds: [], + } + ); + } + + it('throws SERVICE_UNAVAILABLE when clientInfoLoadFailed is true', () => { + const integration = getIntegration(); + integration.clientInfoLoadFailed = true; + + let caught: OAuthError | undefined; + try { + integration.checkClientInfo(); + } catch (err) { + caught = err as OAuthError; + } + + expect(caught).toBeInstanceOf(OAuthError); + expect(caught?.errno).toBe(OAUTH_ERRORS.SERVICE_UNAVAILABLE.errno); + }); + + it('throws UNKNOWN_CLIENT when the client info has no clientId', () => { + const integration = getIntegration(); + integration.clientInfoLoadFailed = false; + // clientInfo is left undefined, so `clientInfo?.clientId` is falsy. + + let caught: OAuthError | undefined; + try { + integration.checkClientInfo(); + } catch (err) { + caught = err as OAuthError; + } + + expect(caught).toBeInstanceOf(OAuthError); + expect(caught?.errno).toBe(OAUTH_ERRORS.UNKNOWN_CLIENT.errno); + }); + + it('does not throw when client info has a clientId', () => { + const integration = getIntegration(); + integration.clientInfoLoadFailed = false; + integration.clientInfo = { + clientId: MOCK_CLIENT_ID, + imageUri: 'https://example.com/icon.png', + serviceName: 'Test Service', + redirectUri: 'https://example.com/redirect', + trusted: true, + }; + + expect(() => integration.checkClientInfo()).not.toThrow(); + }); + }); + describe('replaceItemInArray', () => { it('handles empty array', () => { expect(replaceItemInArray([], 'foo', ['bar'])).toEqual([]); 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 d9613da9cbe..94401584606 100644 --- a/packages/fxa-settings/src/models/integrations/oauth-web-integration.ts +++ b/packages/fxa-settings/src/models/integrations/oauth-web-integration.ts @@ -159,6 +159,15 @@ export class OAuthWebIntegration extends GenericIntegration< return this.clientInfo; } + checkClientInfo() { + if (this.clientInfoLoadFailed) { + throw new OAuthError(OAUTH_ERRORS.SERVICE_UNAVAILABLE.errno); + } + if (!this.clientInfo?.clientId) { + throw new OAuthError(OAUTH_ERRORS.UNKNOWN_CLIENT.errno); + } + } + isTrusted() { return this.clientInfo?.trusted === true; }