diff --git a/packages/fxa-settings/src/lib/oauth/hooks.tsx b/packages/fxa-settings/src/lib/oauth/hooks.tsx index b678c051640..9fa53e694e2 100644 --- a/packages/fxa-settings/src/lib/oauth/hooks.tsx +++ b/packages/fxa-settings/src/lib/oauth/hooks.tsx @@ -9,6 +9,7 @@ import { OAuthIntegration, isOAuthNativeIntegrationSync, isOAuthIntegration, + isSyncDesktopV3Integration, } from '../../models'; import { createEncryptedBundle } from '../crypto/scoped-keys'; import { Constants } from '../constants'; @@ -283,3 +284,31 @@ export function useFinishOAuthFlowHandler( } return { oAuthDataError: null, finishOAuthFlowHandler }; } + +/** + * If we don't have `keyFetchToken` or `unwrapBKey` and it's required, the user needs to + * restart the flow. We only store these values in memory, so they don't persist across page + * refreshes or browser session restarts. We can't redirect to /signin and reprompt for + * password in a browser session restart/crash because the browser stores the flow state in + * memory and the query params won't match after a browser session is restored. + * + * TODO: Can we check session storage for if the user just refreshed so we can redirect them + * to /signin instead of showing an error component? FXA-10707 + */ +export function useOAuthKeysCheck( + integration: Pick, + keyFetchToken?: hexstring, + unwrapBKey?: hexstring +) { + if ( + (isOAuthIntegration(integration) || + isSyncDesktopV3Integration(integration)) && + integration.wantsKeys() && + (!keyFetchToken || !unwrapBKey) + ) { + return { + oAuthKeysCheckError: OAUTH_ERRORS.TRY_AGAIN, + }; + } + return { oAuthKeysCheckError: null }; +} diff --git a/packages/fxa-settings/src/lib/sensitive-data-client.ts b/packages/fxa-settings/src/lib/sensitive-data-client.ts index 308c8135a5f..519787f6a66 100644 --- a/packages/fxa-settings/src/lib/sensitive-data-client.ts +++ b/packages/fxa-settings/src/lib/sensitive-data-client.ts @@ -6,6 +6,20 @@ export const AUTH_DATA_KEY = 'auth'; +export type SensitiveDataClientData = { + [AUTH_DATA_KEY]: { + emailForAuth?: string; + authPW?: string; + keyFetchToken?: hexstring; + unwrapBKey?: hexstring; + }; +}; + +export type SensitiveDataClientAuthKeys = Pick< + SensitiveDataClientData[typeof AUTH_DATA_KEY], + 'keyFetchToken' | 'unwrapBKey' +>; + /** * Class representing a client for handling sensitive data. * diff --git a/packages/fxa-settings/src/models/integrations/oauth-native-integration.ts b/packages/fxa-settings/src/models/integrations/oauth-native-integration.ts index 2ec9ffa0a2a..198095ee530 100644 --- a/packages/fxa-settings/src/models/integrations/oauth-native-integration.ts +++ b/packages/fxa-settings/src/models/integrations/oauth-native-integration.ts @@ -20,6 +20,9 @@ export function isOAuthNativeIntegration(integration: { export type OAuthIntegration = OAuthWebIntegration | OAuthNativeIntegration; +/** + * Check if the integration is OAuthWeb or OAuthNative + */ export function isOAuthIntegration(integration: { type: IntegrationType; }): integration is OAuthIntegration { diff --git a/packages/fxa-settings/src/pages/InlineRecoveryKeySetup/container.test.tsx b/packages/fxa-settings/src/pages/InlineRecoveryKeySetup/container.test.tsx index 81a6acd9202..118f82c61c4 100644 --- a/packages/fxa-settings/src/pages/InlineRecoveryKeySetup/container.test.tsx +++ b/packages/fxa-settings/src/pages/InlineRecoveryKeySetup/container.test.tsx @@ -55,6 +55,7 @@ function mockModelsModule() { mockSensitiveDataClient.getData = jest.fn().mockReturnValue({ emailForAuth: 'bloop@gmail.com', authPW: MOCK_AUTH_PW, + unwrapBKey: MOCK_UNWRAP_BKEY, }); } diff --git a/packages/fxa-settings/src/pages/InlineRecoveryKeySetup/container.tsx b/packages/fxa-settings/src/pages/InlineRecoveryKeySetup/container.tsx index c2eb579767a..5d46875be60 100644 --- a/packages/fxa-settings/src/pages/InlineRecoveryKeySetup/container.tsx +++ b/packages/fxa-settings/src/pages/InlineRecoveryKeySetup/container.tsx @@ -28,13 +28,16 @@ export const InlineRecoveryKeySetupContainer = (_: RouteComponentProps) => { const location = useLocation() as ReturnType & { state?: SigninLocationState; }; - const { email, uid, sessionToken, unwrapBKey } = location.state || {}; + const { email, uid, sessionToken } = location.state || {}; const sensitiveDataClient = useSensitiveDataClient(); const sensitiveData = sensitiveDataClient.getData(AUTH_DATA_KEY); - const { authPW, emailForAuth } = - (sensitiveData as unknown as { emailForAuth: string; authPW: string }) || - {}; + const { authPW, emailForAuth, unwrapBKey } = + (sensitiveData as { + emailForAuth?: string; + authPW?: string; + unwrapBKey?: hexstring; + }) || {}; const navigateForward = useCallback(() => { setCurrentStep(currentStep + 1); @@ -44,7 +47,9 @@ export const InlineRecoveryKeySetupContainer = (_: RouteComponentProps) => { ( uid: string, sessionToken: string, - unwrapBKey: string + unwrapBKey: string, + emailForAuth: string, + authPW: string ): (() => Promise) => async () => { try { @@ -93,7 +98,7 @@ export const InlineRecoveryKeySetupContainer = (_: RouteComponentProps) => { }; } }, - [authClient, emailForAuth, authPW, navigateForward, ftlMsgResolver] + [authClient, navigateForward, ftlMsgResolver] ); const updateRecoveryHint = useCallback( @@ -124,7 +129,9 @@ export const InlineRecoveryKeySetupContainer = (_: RouteComponentProps) => { const createRecoveryKeyHandler = createRecoveryKey( uid, sessionToken, - unwrapBKey + unwrapBKey, + emailForAuth, + authPW ); const updateRecoveryHintHandler = updateRecoveryHint(sessionToken); diff --git a/packages/fxa-settings/src/pages/InlineRecoverySetup/container.test.tsx b/packages/fxa-settings/src/pages/InlineRecoverySetup/container.test.tsx index bb6c9edd810..3ac8e3606fd 100644 --- a/packages/fxa-settings/src/pages/InlineRecoverySetup/container.test.tsx +++ b/packages/fxa-settings/src/pages/InlineRecoverySetup/container.test.tsx @@ -11,7 +11,8 @@ import { LocationProvider } from '@reach/router'; import { renderWithLocalizationProvider } from 'fxa-react/lib/test-utils/localizationProvider'; import { AuthUiError, AuthUiErrors } from '../../lib/auth-errors/auth-errors'; import { MozServices } from '../../lib/types'; -import { OAuthIntegration } from '../../models'; +import { OAuthIntegration, useSensitiveDataClient } from '../../models'; +import { mockSensitiveDataClient as createMockSensitiveDataClient } from '../../models/mocks'; import { MOCK_QUERY_PARAMS, MOCK_SIGNIN_LOCATION_STATE, @@ -27,7 +28,11 @@ import { MOCK_OAUTH_FLOW_HANDLER_RESPONSE, MOCK_TOTP_STATUS_VERIFIED, } from '../Signin/mocks'; -import { useFinishOAuthFlowHandler } from '../../lib/oauth/hooks'; +import { + useFinishOAuthFlowHandler, + useOAuthKeysCheck, +} from '../../lib/oauth/hooks'; +import { AUTH_DATA_KEY } from '../../lib/sensitive-data-client'; let mockLocationState = {}; const mockSearch = '?' + new URLSearchParams(MOCK_QUERY_PARAMS); @@ -52,9 +57,12 @@ jest.mock('../../lib/oauth/hooks.tsx', () => { return { __esModule: true, useFinishOAuthFlowHandler: jest.fn(), + useOAuthKeysCheck: jest.fn(), }; }); +const mockSensitiveDataClient = createMockSensitiveDataClient(); +mockSensitiveDataClient.getData = jest.fn(); const mockAuthClient = new AuthClient('http://localhost:9000', { keyStretchVersion: 1, }); @@ -64,6 +72,7 @@ jest.mock('../../models', () => { ...jest.requireActual('../../models'), useSession: jest.fn(() => mockSessionHook()), useAuthClient: jest.fn(() => mockAuthClient), + useSensitiveDataClient: jest.fn(), }; }); @@ -119,6 +128,12 @@ function setMocks() { .mockReturnValueOnce(MOCK_OAUTH_FLOW_HANDLER_RESPONSE), oAuthDataError: null, })); + (useSensitiveDataClient as jest.Mock).mockImplementation( + () => mockSensitiveDataClient + ); + (useOAuthKeysCheck as jest.Mock).mockImplementation(() => ({ + oAuthKeysCheckError: null, + })); } const defaultProps = { @@ -201,6 +216,13 @@ describe('InlineRecoverySetupContainer', () => { }); }); + it('reads data from sensitive data client', () => { + render(); + expect(mockSensitiveDataClient.getData).toHaveBeenCalledWith( + AUTH_DATA_KEY + ); + }); + describe('callbacks', () => { describe('cancelSetupHandler', () => { it('redirects when returnOnError is true', async () => { diff --git a/packages/fxa-settings/src/pages/InlineRecoverySetup/container.tsx b/packages/fxa-settings/src/pages/InlineRecoverySetup/container.tsx index b49475d3e78..166b96941cb 100644 --- a/packages/fxa-settings/src/pages/InlineRecoverySetup/container.tsx +++ b/packages/fxa-settings/src/pages/InlineRecoverySetup/container.tsx @@ -11,10 +11,15 @@ import { AuthUiErrors } from '../../lib/auth-errors/auth-errors'; import { FinishOAuthFlowHandlerResult, useFinishOAuthFlowHandler, + useOAuthKeysCheck, } from '../../lib/oauth/hooks'; import { getCode } from '../../lib/totp'; import { MozServices } from '../../lib/types'; -import { OAuthIntegration, useAuthClient } from '../../models'; +import { + OAuthIntegration, + useAuthClient, + useSensitiveDataClient, +} from '../../models'; import { VERIFY_TOTP_MUTATION } from './gql'; import InlineRecoverySetup from './index'; import { hardNavigate } from 'fxa-react/lib/utils'; @@ -23,6 +28,10 @@ import { TotpStatusResponse } from '../Signin/SigninTokenCode/interfaces'; import { GET_TOTP_STATUS } from '../../components/App/gql'; import OAuthDataError from '../../components/OAuthDataError'; import { isFirefoxService } from '../../models/integrations/utils'; +import { + AUTH_DATA_KEY, + SensitiveDataClientAuthKeys, +} from '../../lib/sensitive-data-client'; export const InlineRecoverySetupContainer = ({ isSignedIn, @@ -46,6 +55,16 @@ export const InlineRecoverySetupContainer = ({ }; const signinRecoveryLocationState = location.state; const { totp, ...signinLocationState } = signinRecoveryLocationState || {}; + const sensitiveDataClient = useSensitiveDataClient(); + const sensitiveData = sensitiveDataClient.getData(AUTH_DATA_KEY); + const { keyFetchToken, unwrapBKey } = + (sensitiveData as SensitiveDataClientAuthKeys) || {}; + + const { oAuthKeysCheckError } = useOAuthKeysCheck( + integration, + keyFetchToken, + unwrapBKey + ); const [recoveryCodes, setRecoveryCodes] = useState(); const [verifyTotp] = useMutation<{ verifyTotp: { success: boolean } }>( @@ -78,15 +97,20 @@ export const InlineRecoverySetupContainer = ({ const { redirect, error } = await finishOAuthFlowHandler( signinRecoveryLocationState!.uid, signinRecoveryLocationState!.sessionToken, - signinRecoveryLocationState!.keyFetchToken, - signinRecoveryLocationState!.unwrapBKey + keyFetchToken, + unwrapBKey ); if (error) { setOAuthError(error); return; } hardNavigate(redirect); - }, [signinRecoveryLocationState, finishOAuthFlowHandler]); + }, [ + signinRecoveryLocationState, + finishOAuthFlowHandler, + keyFetchToken, + unwrapBKey, + ]); const cancelSetupHandler = useCallback(() => { const error = AuthUiErrors.TOTP_REQUIRED; @@ -125,6 +149,11 @@ export const InlineRecoverySetupContainer = ({ if (oAuthDataError) { return ; } + // Note that we don't currently need this check on this page right now since AMO is the only + // RP requiring 2FA and it doesn't require keys. However it's here for consistency. + if (oAuthKeysCheckError) { + return ; + } return ( { sendLoginPushRequest: mockSendLoginPushRequest, }; }, + useSensitiveDataClient: jest.fn(), }; }); @@ -96,6 +106,16 @@ function mockCurrentAccount(storedAccount = { uid: '123' }) { jest.spyOn(CacheModule, 'currentAccount').mockReturnValue(storedAccount); } +function resetMockSensitiveDataClient() { + (useSensitiveDataClient as jest.Mock).mockImplementation( + () => mockSensitiveDataClient + ); + mockSensitiveDataClient.getData = jest.fn().mockReturnValue({ + keyFetchToken: MOCK_KEY_FETCH_TOKEN, + unwrapBKey: MOCK_UNWRAP_BKEY, + }); +} + async function render() { renderWithLocalizationProvider( diff --git a/packages/fxa-settings/src/pages/Signin/SigninPushCode/container.tsx b/packages/fxa-settings/src/pages/Signin/SigninPushCode/container.tsx index 1eb1be33594..2ce5a7207e5 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninPushCode/container.tsx +++ b/packages/fxa-settings/src/pages/Signin/SigninPushCode/container.tsx @@ -7,7 +7,12 @@ import SigninPushCode from '.'; import { MozServices } from '../../../lib/types'; import { getSigninState, handleNavigation } from '../utils'; import { SigninLocationState } from '../interfaces'; -import { Integration, isWebIntegration, useAuthClient } from '../../../models'; +import { + Integration, + isWebIntegration, + useAuthClient, + useSensitiveDataClient, +} from '../../../models'; import { useFinishOAuthFlowHandler } from '../../../lib/oauth/hooks'; import { hardNavigate } from 'fxa-react/lib/utils'; import LoadingSpinner from 'fxa-react/components/LoadingSpinner'; @@ -15,6 +20,10 @@ import OAuthDataError from '../../../components/OAuthDataError'; import { useWebRedirect } from '../../../lib/hooks/useWebRedirect'; import { useEffect, useState } from 'react'; import { useNavigateWithQuery as useNavigate } from '../../../lib/hooks/useNavigateWithQuery'; +import { + AUTH_DATA_KEY, + SensitiveDataClientAuthKeys, +} from '../../../lib/sensitive-data-client'; export type SigninPushCodeContainerProps = { integration: Integration; @@ -35,8 +44,10 @@ export const SigninPushCodeContainer = ({ const location = useLocation() as ReturnType & { state: SigninLocationState; }; - const signinState = getSigninState(location.state); + const sensitiveDataClient = useSensitiveDataClient(); + const sensitiveData = sensitiveDataClient.getData(AUTH_DATA_KEY); + const { unwrapBKey } = (sensitiveData as SensitiveDataClientAuthKeys) || {}; const webRedirectCheck = useWebRedirect(integration.data.redirectTo); @@ -82,7 +93,7 @@ export const SigninPushCodeContainer = ({ const navigationOptions = { email: signinState.email, signinData: { ...signinState, verified: true }, - unwrapBKey: signinState.unwrapBKey, + unwrapBKey, integration, finishOAuthFlowHandler, queryParams: location.search, diff --git a/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/container.test.tsx b/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/container.test.tsx index ea8335b9a10..9d30556d09f 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/container.test.tsx +++ b/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/container.test.tsx @@ -14,7 +14,8 @@ import { renderWithLocalizationProvider } from 'fxa-react/lib/test-utils/localiz import SigninRecoveryCodeContainer from './container'; import { createMockWebIntegration } from '../../../lib/integrations/mocks'; import { MozServices } from '../../../lib/types'; -import { Integration } from '../../../models'; +import { Integration, useSensitiveDataClient } from '../../../models'; +import { mockSensitiveDataClient as createMockSensitiveDataClient } from '../../../models/mocks'; import { MOCK_STORED_ACCOUNT, MOCK_RECOVERY_CODE, @@ -25,6 +26,7 @@ import { mockGqlError, mockSigninLocationState } from '../mocks'; import { mockConsumeRecoveryCodeUseMutation } from './mocks'; import { waitFor } from '@testing-library/react'; import { AuthUiErrors } from '../../../lib/auth-errors/auth-errors'; +import { AUTH_DATA_KEY } from '../../../lib/sensitive-data-client'; let integration: Integration; function mockWebIntegration() { @@ -47,10 +49,13 @@ jest.mock('../../../models', () => { return { ...jest.requireActual('../../../models'), useAuthClient: jest.fn(), + useSensitiveDataClient: jest.fn(), }; }); let currentSigninRecoveryCodeProps: SigninRecoveryCodeProps | undefined; +const mockSensitiveDataClient = createMockSensitiveDataClient(); +mockSensitiveDataClient.getData = jest.fn(); function mockSigninRecoveryCodeModule() { currentSigninRecoveryCodeProps = undefined; jest @@ -96,16 +101,22 @@ function mockReachRouter( .mockImplementation(() => mockLocation(pathname, mockLocationState)); } +function resetMockSensitiveDataClient() { + (useSensitiveDataClient as jest.Mock).mockImplementation( + () => mockSensitiveDataClient + ); +} + function applyDefaultMocks() { jest.resetAllMocks(); jest.restoreAllMocks(); mockSigninRecoveryCodeModule(); mockLoadingSpinnerModule(); - // mockUseValidateModule(); mockReactUtilsModule(); mockCache(); mockReachRouter(undefined, 'signin_recovery_code', mockSigninLocationState); mockWebIntegration(); + resetMockSensitiveDataClient(); } function render(mocks: Array) { @@ -151,6 +162,13 @@ describe('SigninRecoveryCode container', () => { await render([]); expect(ReactUtils.hardNavigate).not.toBeCalledWith('/', {}, true); }); + + it('reads data from sensitive data client', () => { + render([]); + expect(mockSensitiveDataClient.getData).toHaveBeenCalledWith( + AUTH_DATA_KEY + ); + }); }); describe('submitRecoveryCode', () => { diff --git a/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/container.tsx b/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/container.tsx index 84da6a202ac..e250ec26d70 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/container.tsx +++ b/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/container.tsx @@ -6,17 +6,28 @@ import { RouteComponentProps, useLocation } from '@reach/router'; import { hardNavigate } from 'fxa-react/lib/utils'; import { MozServices } from '../../../lib/types'; import SigninRecoveryCode from '.'; -import { Integration, useAuthClient } from '../../../models'; +import { + Integration, + useAuthClient, + useSensitiveDataClient, +} from '../../../models'; import LoadingSpinner from 'fxa-react/components/LoadingSpinner'; import { useMutation } from '@apollo/client'; import { CONSUME_RECOVERY_CODE_MUTATION } from './gql'; import { useCallback } from 'react'; import { getSigninState } from '../utils'; import { SigninLocationState } from '../interfaces'; -import { useFinishOAuthFlowHandler } from '../../../lib/oauth/hooks'; +import { + useFinishOAuthFlowHandler, + useOAuthKeysCheck, +} from '../../../lib/oauth/hooks'; import { ConsumeRecoveryCodeResponse, SubmitRecoveryCode } from './interfaces'; import OAuthDataError from '../../../components/OAuthDataError'; import { getHandledError } from '../../../lib/error-utils'; +import { + AUTH_DATA_KEY, + SensitiveDataClientAuthKeys, +} from '../../../lib/sensitive-data-client'; export type SigninRecoveryCodeContainerProps = { integration: Integration; @@ -36,8 +47,17 @@ export const SigninRecoveryCodeContainer = ({ const location = useLocation() as ReturnType & { state: SigninLocationState; }; - const signinState = getSigninState(location.state); + const sensitiveDataClient = useSensitiveDataClient(); + const sensitiveData = sensitiveDataClient.getData(AUTH_DATA_KEY); + const { keyFetchToken, unwrapBKey } = + (sensitiveData as SensitiveDataClientAuthKeys) || {}; + + const { oAuthKeysCheckError } = useOAuthKeysCheck( + integration, + keyFetchToken, + unwrapBKey + ); const [consumeRecoveryCode] = useMutation( CONSUME_RECOVERY_CODE_MUTATION @@ -64,6 +84,9 @@ export const SigninRecoveryCodeContainer = ({ if (oAuthDataError) { return ; } + if (oAuthKeysCheckError) { + return ; + } if (!signinState) { hardNavigate('/', {}, true); @@ -78,6 +101,8 @@ export const SigninRecoveryCodeContainer = ({ serviceName, signinState, submitRecoveryCode, + keyFetchToken, + unwrapBKey, }} /> ); diff --git a/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/index.tsx b/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/index.tsx index 6f3742da3f6..f0def550cc0 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/index.tsx +++ b/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/index.tsx @@ -32,6 +32,8 @@ const SigninRecoveryCode = ({ serviceName, signinState, submitRecoveryCode, + keyFetchToken, + unwrapBKey, }: SigninRecoveryCodeProps & RouteComponentProps) => { useEffect(() => { GleanMetrics.loginBackupCode.view(); @@ -63,15 +65,8 @@ const SigninRecoveryCode = ({ submitButtonText: 'Confirm', }; - const { - email, - sessionToken, - uid, - verificationMethod, - verificationReason, - keyFetchToken, - unwrapBKey, - } = signinState; + const { email, sessionToken, uid, verificationMethod, verificationReason } = + signinState; const onSuccessNavigate = useCallback(async () => { const navigationOptions = { diff --git a/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/interfaces.ts b/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/interfaces.ts index 551be248b45..5659080be5a 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/interfaces.ts +++ b/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/interfaces.ts @@ -4,6 +4,7 @@ import { BeginSigninError } from '../../../lib/error-utils'; import { FinishOAuthFlowHandler } from '../../../lib/oauth/hooks'; +import { SensitiveDataClientAuthKeys } from '../../../lib/sensitive-data-client'; import { MozServices } from '../../../lib/types'; import { SigninIntegration, SigninLocationState } from '../interfaces'; @@ -13,7 +14,7 @@ export type SigninRecoveryCodeProps = { serviceName?: MozServices; signinState: SigninLocationState; submitRecoveryCode: SubmitRecoveryCode; -}; +} & SensitiveDataClientAuthKeys; export type SubmitRecoveryCode = ( code: string diff --git a/packages/fxa-settings/src/pages/Signin/SigninTokenCode/container.test.tsx b/packages/fxa-settings/src/pages/Signin/SigninTokenCode/container.test.tsx index 6e0a41c9881..34c34f4b596 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninTokenCode/container.test.tsx +++ b/packages/fxa-settings/src/pages/Signin/SigninTokenCode/container.test.tsx @@ -7,16 +7,23 @@ import * as ReactUtils from 'fxa-react/lib/utils'; import * as CacheModule from '../../../lib/cache'; import { SigninTokenCodeProps } from './interfaces'; -import { Integration } from '../../../models'; +import { Integration, useSensitiveDataClient } from '../../../models'; import { renderWithLocalizationProvider } from 'fxa-react/lib/test-utils/localizationProvider'; import { LocationProvider } from '@reach/router'; import SigninTokenCodeContainer from './container'; import { screen, waitFor } from '@testing-library/react'; -import { MOCK_EMAIL, MOCK_STORED_ACCOUNT } from '../../mocks'; +import { + MOCK_EMAIL, + MOCK_KEY_FETCH_TOKEN, + MOCK_STORED_ACCOUNT, + MOCK_UNWRAP_BKEY, +} from '../../mocks'; import { createMockWebIntegration } from '../../../lib/integrations/mocks'; import { createMockSigninLocationState } from './mocks'; +import { mockSensitiveDataClient as createMockSensitiveDataClient } from '../../../models/mocks'; let integration: Integration; +const mockSensitiveDataClient = createMockSensitiveDataClient(); function mockWebIntegration() { integration = createMockWebIntegration() as Integration; @@ -31,6 +38,7 @@ function applyDefaultMocks() { mockSigninTokenCodeModule(); mockCurrentAccount(); + resetMockSensitiveDataClient(); } let mockHasTotpAuthClient = false; @@ -44,6 +52,7 @@ jest.mock('../../../models', () => { .mockResolvedValue({ verified: mockHasTotpAuthClient }), }; }, + useSensitiveDataClient: jest.fn(), }; }); @@ -85,6 +94,16 @@ function mockCurrentAccount(storedAccount = { uid: '123' }) { jest.spyOn(CacheModule, 'currentAccount').mockReturnValue(storedAccount); } +function resetMockSensitiveDataClient() { + (useSensitiveDataClient as jest.Mock).mockImplementation( + () => mockSensitiveDataClient + ); + mockSensitiveDataClient.getData = jest.fn().mockReturnValue({ + keyFetchToken: MOCK_KEY_FETCH_TOKEN, + unwrapBKey: MOCK_UNWRAP_BKEY, + }); +} + async function render() { renderWithLocalizationProvider( diff --git a/packages/fxa-settings/src/pages/Signin/SigninTokenCode/container.tsx b/packages/fxa-settings/src/pages/Signin/SigninTokenCode/container.tsx index 83325c129fd..34ab76ecf61 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninTokenCode/container.tsx +++ b/packages/fxa-settings/src/pages/Signin/SigninTokenCode/container.tsx @@ -7,12 +7,23 @@ import { useNavigateWithQuery as useNavigate } from '../../../lib/hooks/useNavig import SigninTokenCode from '.'; import LoadingSpinner from 'fxa-react/components/LoadingSpinner'; import { hardNavigate } from 'fxa-react/lib/utils'; -import { Integration, useAuthClient } from '../../../models'; -import { useFinishOAuthFlowHandler } from '../../../lib/oauth/hooks'; +import { + Integration, + useAuthClient, + useSensitiveDataClient, +} from '../../../models'; +import { + useFinishOAuthFlowHandler, + useOAuthKeysCheck, +} from '../../../lib/oauth/hooks'; import { SigninLocationState } from '../interfaces'; import { getSigninState } from '../utils'; import OAuthDataError from '../../../components/OAuthDataError'; import { useEffect, useState } from 'react'; +import { + AUTH_DATA_KEY, + SensitiveDataClientAuthKeys, +} from '../../../lib/sensitive-data-client'; // The email with token code (verifyLoginCodeEmail) is sent on `/signin` // submission if conditions are met. @@ -28,12 +39,21 @@ const SigninTokenCodeContainer = ({ }; const signinState = getSigninState(location.state); + const sensitiveDataClient = useSensitiveDataClient(); + const sensitiveData = sensitiveDataClient.getData(AUTH_DATA_KEY); + const { keyFetchToken, unwrapBKey } = + (sensitiveData as SensitiveDataClientAuthKeys) || {}; const authClient = useAuthClient(); const { finishOAuthFlowHandler, oAuthDataError } = useFinishOAuthFlowHandler( authClient, integration ); + const { oAuthKeysCheckError } = useOAuthKeysCheck( + integration, + keyFetchToken, + unwrapBKey + ); const [totpVerified, setTotpVerified] = useState(false); useEffect(() => { @@ -67,6 +87,9 @@ const SigninTokenCodeContainer = ({ if (oAuthDataError) { return ; } + if (oAuthKeysCheckError) { + return ; + } return ( ); diff --git a/packages/fxa-settings/src/pages/Signin/SigninTokenCode/index.tsx b/packages/fxa-settings/src/pages/Signin/SigninTokenCode/index.tsx index 794bd7fe0cc..d2279b6b12d 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninTokenCode/index.tsx +++ b/packages/fxa-settings/src/pages/Signin/SigninTokenCode/index.tsx @@ -34,6 +34,8 @@ const SigninTokenCode = ({ finishOAuthFlowHandler, integration, signinState, + keyFetchToken, + unwrapBKey, }: SigninTokenCodeProps & RouteComponentProps) => { usePageViewEvent(viewName, REACT_ENTRYPOINT); const session = useSession(); @@ -44,8 +46,6 @@ const SigninTokenCode = ({ uid, sessionToken, verificationReason, - keyFetchToken, - unwrapBKey, showInlineRecoveryKeySetup, } = signinState; diff --git a/packages/fxa-settings/src/pages/Signin/SigninTokenCode/interfaces.ts b/packages/fxa-settings/src/pages/Signin/SigninTokenCode/interfaces.ts index 46647f0b4db..10380b48063 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninTokenCode/interfaces.ts +++ b/packages/fxa-settings/src/pages/Signin/SigninTokenCode/interfaces.ts @@ -5,12 +5,13 @@ import { AccountTotp } from '../../../lib/interfaces'; import { FinishOAuthFlowHandler } from '../../../lib/oauth/hooks'; import { SigninIntegration, SigninLocationState } from '../interfaces'; +import { SensitiveDataClientAuthKeys } from './../../../lib/sensitive-data-client'; -export interface SigninTokenCodeProps { +export type SigninTokenCodeProps = { finishOAuthFlowHandler: FinishOAuthFlowHandler; integration: SigninIntegration; signinState: SigninLocationState; -} +} & SensitiveDataClientAuthKeys; export interface TotpStatusResponse { account: { diff --git a/packages/fxa-settings/src/pages/Signin/SigninTotpCode/container.test.tsx b/packages/fxa-settings/src/pages/Signin/SigninTotpCode/container.test.tsx index e8f884225c8..c5cd7b69883 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninTotpCode/container.test.tsx +++ b/packages/fxa-settings/src/pages/Signin/SigninTotpCode/container.test.tsx @@ -20,7 +20,9 @@ import { renderWithLocalizationProvider } from 'fxa-react/lib/test-utils/localiz import SigninTotpCodeContainer from './container'; import { MozServices } from '../../../lib/types'; import { createMockWebIntegration } from '../SigninTokenCode/mocks'; -import { Integration } from '../../../models'; +import { Integration, useSensitiveDataClient } from '../../../models'; +import { mockSensitiveDataClient as createMockSensitiveDataClient } from '../../../models/mocks'; + import { MOCK_NON_TOTP_LOCATION_STATE, MOCK_TOTP_LOCATION_STATE, @@ -60,6 +62,7 @@ jest.mock('../../../models', () => { return { ...jest.requireActual('../../../models'), useAuthClient: jest.fn(), + useSensitiveDataClient: jest.fn(), }; }); @@ -119,6 +122,13 @@ function mockVerifyTotp(success: boolean = true, errorOut: boolean = false) { }, ]); } +const mockSensitiveDataClient = createMockSensitiveDataClient(); +mockSensitiveDataClient.getData = jest.fn(); +function restMockSensitiveDataClient() { + (useSensitiveDataClient as jest.Mock).mockImplementation( + () => mockSensitiveDataClient + ); +} function applyDefaultMocks() { jest.resetAllMocks(); @@ -131,6 +141,7 @@ function applyDefaultMocks() { mockReachRouter(MOCK_TOTP_LOCATION_STATE); mockVerifyTotp(); mockWebIntegration(); + restMockSensitiveDataClient(); } describe('signin totp code container', () => { diff --git a/packages/fxa-settings/src/pages/Signin/SigninTotpCode/container.tsx b/packages/fxa-settings/src/pages/Signin/SigninTotpCode/container.tsx index e04bf5010b7..599e170100f 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninTotpCode/container.tsx +++ b/packages/fxa-settings/src/pages/Signin/SigninTotpCode/container.tsx @@ -12,13 +12,25 @@ import VerificationMethods from '../../../constants/verification-methods'; import { VERIFY_TOTP_CODE_MUTATION } from './gql'; import { getSigninState } from '../utils'; import { SigninLocationState } from '../interfaces'; -import { Integration, isWebIntegration, useAuthClient } from '../../../models'; -import { useFinishOAuthFlowHandler } from '../../../lib/oauth/hooks'; +import { + Integration, + isWebIntegration, + useAuthClient, + useSensitiveDataClient, +} from '../../../models'; +import { + useFinishOAuthFlowHandler, + useOAuthKeysCheck, +} from '../../../lib/oauth/hooks'; import { hardNavigate } from 'fxa-react/lib/utils'; import LoadingSpinner from 'fxa-react/components/LoadingSpinner'; import OAuthDataError from '../../../components/OAuthDataError'; import { getHandledError } from '../../../lib/error-utils'; import { useWebRedirect } from '../../../lib/hooks/useWebRedirect'; +import { + AUTH_DATA_KEY, + SensitiveDataClientAuthKeys, +} from '../../../lib/sensitive-data-client'; export type SigninTotpCodeContainerProps = { integration: Integration; @@ -38,14 +50,23 @@ export const SigninTotpCodeContainer = ({ const location = useLocation() as ReturnType & { state: SigninLocationState; }; - const signinState = getSigninState(location.state); + const sensitiveDataClient = useSensitiveDataClient(); + const sensitiveData = sensitiveDataClient.getData(AUTH_DATA_KEY); + const { keyFetchToken, unwrapBKey } = + (sensitiveData as SensitiveDataClientAuthKeys) || {}; const { queryParamModel } = useValidatedQueryParams(SigninQueryParams); const { service } = queryParamModel; const webRedirectCheck = useWebRedirect(integration.data.redirectTo); + const { oAuthKeysCheckError } = useOAuthKeysCheck( + integration, + keyFetchToken, + unwrapBKey + ); + const redirectTo = isWebIntegration(integration) && webRedirectCheck.isValid() ? integration.data.redirectTo @@ -78,6 +99,9 @@ export const SigninTotpCodeContainer = ({ if (oAuthDataError) { return ; } + if (oAuthKeysCheckError) { + return ; + } if ( !signinState || @@ -97,6 +121,8 @@ export const SigninTotpCodeContainer = ({ signinState, submitTotpCode, serviceName, + keyFetchToken, + unwrapBKey, }} /> ); diff --git a/packages/fxa-settings/src/pages/Signin/SigninTotpCode/index.tsx b/packages/fxa-settings/src/pages/Signin/SigninTotpCode/index.tsx index 2244f2aea2d..51d8ca2a869 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninTotpCode/index.tsx +++ b/packages/fxa-settings/src/pages/Signin/SigninTotpCode/index.tsx @@ -24,6 +24,7 @@ import { } from '../../../lib/error-utils'; import protectionShieldIcon from '@fxa/shared/assets/images/protection-shield.svg'; import Banner from '../../../components/Banner'; +import { SensitiveDataClientAuthKeys } from '../../../lib/sensitive-data-client'; // TODO: show a banner success message if a user is coming from reset password // in FXA-6491. This differs from content-server where currently, users only @@ -39,7 +40,7 @@ export type SigninTotpCodeProps = { totpCode: string ) => Promise<{ error?: BeginSigninError; status: boolean }>; serviceName?: MozServices; -}; +} & SensitiveDataClientAuthKeys; export const viewName = 'signin-totp-code'; @@ -49,7 +50,8 @@ export const SigninTotpCode = ({ redirectTo, signinState, submitTotpCode, - serviceName, + keyFetchToken, + unwrapBKey, }: SigninTotpCodeProps & RouteComponentProps) => { const location = useLocation(); @@ -63,8 +65,6 @@ export const SigninTotpCode = ({ sessionToken, verificationMethod, verificationReason, - keyFetchToken, - unwrapBKey, showInlineRecoveryKeySetup, } = signinState; diff --git a/packages/fxa-settings/src/pages/Signin/container.test.tsx b/packages/fxa-settings/src/pages/Signin/container.test.tsx index 51dadc1fad6..ce7a044ca1a 100644 --- a/packages/fxa-settings/src/pages/Signin/container.test.tsx +++ b/packages/fxa-settings/src/pages/Signin/container.test.tsx @@ -46,6 +46,7 @@ import { mockGqlPasswordChangeStartMutation, MOCK_FLOW_ID, MOCK_CLIENT_ID, + MOCK_KEY_FETCH_TOKEN, } from './mocks'; import AuthClient from 'fxa-auth-client/browser'; import VerificationMethods from '../../constants/verification-methods'; @@ -177,6 +178,7 @@ function mockModelsModule() { }, })); } + // Call this when testing local storage function mockCurrentAccount(storedAccount = { uid: '123' }) { jest.spyOn(CacheModule, 'currentAccount').mockReturnValue(storedAccount); @@ -519,6 +521,8 @@ describe('signin container', () => { expect(mockSensitiveDataClient.setData).toBeCalledWith(AUTH_DATA_KEY, { authPW: MOCK_AUTH_PW, emailForAuth: MOCK_EMAIL, + unwrapBKey: MOCK_UNWRAP_BKEY, + keyFetchToken: MOCK_KEY_FETCH_TOKEN, }); expect(mockAuthClient.recoveryKeyExists).toBeCalledWith( handlerResult?.data?.signIn.sessionToken, diff --git a/packages/fxa-settings/src/pages/Signin/container.tsx b/packages/fxa-settings/src/pages/Signin/container.tsx index 4aa06bdf249..c11f3ecb992 100644 --- a/packages/fxa-settings/src/pages/Signin/container.tsx +++ b/packages/fxa-settings/src/pages/Signin/container.tsx @@ -706,11 +706,13 @@ export async function trySignIn( ? v2Credentials.unwrapBKey : v1Credentials.unwrapBKey; - // Store for inline recovery key flow sensitiveDataClient.setData(AUTH_DATA_KEY, { + // Store for inline recovery key flow authPW, // Store this in case the email was corrected emailForAuth: email, + unwrapBKey, + keyFetchToken: data.signIn.keyFetchToken, }); return { diff --git a/packages/fxa-settings/src/pages/Signin/index.test.tsx b/packages/fxa-settings/src/pages/Signin/index.test.tsx index af5c522d7e5..f58d2f6e1fb 100644 --- a/packages/fxa-settings/src/pages/Signin/index.test.tsx +++ b/packages/fxa-settings/src/pages/Signin/index.test.tsx @@ -39,6 +39,7 @@ import { import firefox from '../../lib/channels/firefox'; import { navigate } from '@reach/router'; import { IntegrationType } from '../../models'; +import { AUTH_DATA_KEY } from '../../lib/sensitive-data-client'; // import { getFtlBundle, testAllL10n } from 'fxa-react/lib/test-utils'; // import { FluentBundle } from '@fluent/bundle'; @@ -173,7 +174,7 @@ function differentAccountLinkRendered() { screen.getByRole('link', { name: 'Use a different account' }); } -describe('Signin', () => { +describe('Signin component', () => { // let bundle: FluentBundle; // beforeAll(async () => { // bundle = await getFtlBundle('settings'); @@ -380,8 +381,6 @@ describe('Signin', () => { uid: MOCK_UID, sessionToken: MOCK_SESSION_TOKEN, verified: true, - keyFetchToken: MOCK_KEY_FETCH_TOKEN, - unwrapBKey: MOCK_UNWRAP_BKEY, showInlineRecoveryKeySetup: true, verificationMethod: 'email-otp', verificationReason: VerificationReasons.SIGN_IN, @@ -502,8 +501,6 @@ describe('Signin', () => { verified: false, verificationReason: 'signup', verificationMethod: 'email-otp', - keyFetchToken: MOCK_KEY_FETCH_TOKEN, - unwrapBKey: MOCK_UNWRAP_BKEY, }, }); }); @@ -672,7 +669,7 @@ describe('Signin', () => { enterPasswordAndSubmit(); await waitFor(() => { expect(sendUnblockEmailHandler).toHaveBeenCalled(); - expect(mockSetData).toHaveBeenCalledWith('auth', { + expect(mockSetData).toHaveBeenCalledWith(AUTH_DATA_KEY, { password: MOCK_PASSWORD, }); expect(mockNavigate).toHaveBeenCalledWith('/signin_unblock', { diff --git a/packages/fxa-settings/src/pages/Signin/index.tsx b/packages/fxa-settings/src/pages/Signin/index.tsx index 6dce1cbb68a..af1fdb3f3ff 100644 --- a/packages/fxa-settings/src/pages/Signin/index.tsx +++ b/packages/fxa-settings/src/pages/Signin/index.tsx @@ -36,6 +36,7 @@ import { handleNavigation } from './utils'; import { useWebRedirect } from '../../lib/hooks/useWebRedirect'; import { getLocalizedErrorMessage } from '../../lib/error-utils'; import Banner from '../../components/Banner'; +import { AUTH_DATA_KEY } from '../../lib/sensitive-data-client'; export const viewName = 'signin'; @@ -254,7 +255,7 @@ const Signin = ({ } // Store password to be used in another component - sensitiveDataClient.setData('auth', { + sensitiveDataClient.setData(AUTH_DATA_KEY, { password, }); // navigate only if sending the unblock code email is successful diff --git a/packages/fxa-settings/src/pages/Signin/interfaces.ts b/packages/fxa-settings/src/pages/Signin/interfaces.ts index 78aff2e2a12..c3813ff8a39 100644 --- a/packages/fxa-settings/src/pages/Signin/interfaces.ts +++ b/packages/fxa-settings/src/pages/Signin/interfaces.ts @@ -213,8 +213,6 @@ export interface SigninLocationState { verified: boolean; verificationMethod?: VerificationMethods; verificationReason?: VerificationReasons; - keyFetchToken?: hexstring; - unwrapBKey?: hexstring; showInlineRecoveryKeySetup?: boolean; } diff --git a/packages/fxa-settings/src/pages/Signin/utils.ts b/packages/fxa-settings/src/pages/Signin/utils.ts index 64729570beb..b5e187c3a7a 100644 --- a/packages/fxa-settings/src/pages/Signin/utils.ts +++ b/packages/fxa-settings/src/pages/Signin/utils.ts @@ -149,9 +149,7 @@ const createSigninLocationState = ( verified, verificationMethod, verificationReason, - keyFetchToken, }, - unwrapBKey, showInlineRecoveryKeySetup, } = navigationOptions; return { @@ -161,8 +159,6 @@ const createSigninLocationState = ( verified, verificationMethod, verificationReason, - keyFetchToken, - unwrapBKey, showInlineRecoveryKeySetup, }; }; @@ -178,8 +174,8 @@ function sendFxaLogin(navigationOptions: NavigationOptions) { // sending these values can cause intermittent sync disconnect issues in oauth desktop. ...(!isOAuth && { // keyFetchToken and unwrapBKey should always exist if Sync integration - keyFetchToken: navigationOptions.signinData.keyFetchToken!, - unwrapBKey: navigationOptions.unwrapBKey!, + keyFetchToken: navigationOptions.signinData.keyFetchToken, + unwrapBKey: navigationOptions.unwrapBKey, }), services: navigationOptions.integration.isDesktopRelay() ? { relay: {} } diff --git a/packages/fxa-settings/src/pages/Signup/ConfirmSignupCode/container.test.tsx b/packages/fxa-settings/src/pages/Signup/ConfirmSignupCode/container.test.tsx index fbff3b80cf6..c2f6c5ddf8f 100644 --- a/packages/fxa-settings/src/pages/Signup/ConfirmSignupCode/container.test.tsx +++ b/packages/fxa-settings/src/pages/Signup/ConfirmSignupCode/container.test.tsx @@ -18,6 +18,8 @@ import { StoredAccountData } from '../../../lib/storage-utils'; import { renderWithLocalizationProvider } from 'fxa-react/lib/test-utils/localizationProvider'; import SignupConfirmCodeContainer from './container'; import { Integration } from '../../../models'; +import { mockSensitiveDataClient as createMockSensitiveDataClient } from '../../../models/mocks'; + import { MOCK_EMAIL, MOCK_FLOW_ID, @@ -26,6 +28,7 @@ import { MOCK_UID, MOCK_UNWRAP_BKEY, } from '../../mocks'; +import { OAUTH_ERRORS } from '../../../lib/oauth'; // Setup mocks @@ -34,6 +37,7 @@ jest.mock('../../../models', () => { return { ...jest.requireActual('../../../models'), useAuthClient: jest.fn(), + useSensitiveDataClient: jest.fn(), }; }); @@ -48,10 +52,10 @@ let integration: Integration; let mockAuthClient = new AuthClient('localhost:9000', { keyStretchVersion: 1 }); let currentProps: any | undefined; let mockEmailBounceStatusQuery = jest.fn(); +const mockSensitiveDataClient = createMockSensitiveDataClient(); function mockLocation( originIsSignup: boolean = true, - withIntegrationProps: boolean = true, withAccountInfo: boolean = true ) { jest.spyOn(ReachRouterModule, 'useLocation').mockImplementation(() => { @@ -62,8 +66,6 @@ function mockLocation( sessionToken: withAccountInfo ? MOCK_SESSION_TOKEN : undefined, origin: originIsSignup ? 'signup' : null, selectedNewsletterSlugs: 'slugs', - keyFetchToken: withIntegrationProps ? MOCK_KEY_FETCH_TOKEN : null, - unwrapBKey: withIntegrationProps ? MOCK_UNWRAP_BKEY : null, }, } as ReturnType; }); @@ -92,6 +94,7 @@ function applyMocks() { integration = { type: ModelsModule.IntegrationType.OAuthWeb, + wantsKeys: () => false, } as Integration; jest .spyOn(ConfirmSignupCodeModule, 'default') @@ -106,6 +109,13 @@ function applyMocks() { (ModelsModule.useAuthClient as jest.Mock).mockImplementation( () => mockAuthClient ); + (ModelsModule.useSensitiveDataClient as jest.Mock).mockImplementation( + () => mockSensitiveDataClient + ); + mockSensitiveDataClient.getData = jest.fn().mockReturnValue({ + keyFetchToken: MOCK_KEY_FETCH_TOKEN, + unwrapBKey: MOCK_UNWRAP_BKEY, + }); jest .spyOn(HooksModule, 'useFinishOAuthFlowHandler') .mockImplementation(() => { @@ -164,7 +174,7 @@ describe('confirm-signup-container', () => { }); it('renders as expected with account info in local storage', async () => { - mockLocation(true, true, false); + mockLocation(true, false); jest.spyOn(CacheModule, 'currentAccount').mockImplementationOnce(() => { return { uid: MOCK_UID, @@ -231,7 +241,7 @@ describe('confirm-signup-container', () => { describe('renders-spinner', () => { it('has no account in location state or local storage', async () => { - mockLocation(false, false, false); + mockLocation(false, false); jest.spyOn(CacheModule, 'currentAccount').mockImplementationOnce(() => { return {} as StoredAccountData; }); @@ -244,15 +254,6 @@ describe('confirm-signup-container', () => { expect.stringMatching('/') ); }); - - it('has missing integration', async () => { - // Testing type safety violation - integration = undefined as any as Integration; - render(); - await waitFor(() => - expect(screen.getByText('loading spinner mock')).toBeInTheDocument() - ); - }); }); describe('handles oAuthDataError', () => { @@ -282,4 +283,21 @@ describe('confirm-signup-container', () => { expect(screen.getByText('Unexpected error')).toBeInTheDocument(); }); }); + describe('useOAuthKeysCheck', () => { + it('renders error component when value is undefined', () => { + integration = { + type: ModelsModule.IntegrationType.OAuthNative, + wantsKeys: () => true, + } as Integration; + mockSensitiveDataClient.getData = jest.fn().mockReturnValue({ + keyFetchToken: undefined, + unwrapBKey: undefined, + }); + render(); + screen.getByText('Bad Request'); + screen.getByText( + 'Something went wrong. Please close this tab and try again.' + ); + }); + }); }); diff --git a/packages/fxa-settings/src/pages/Signup/ConfirmSignupCode/container.tsx b/packages/fxa-settings/src/pages/Signup/ConfirmSignupCode/container.tsx index 55d8f9d2cf6..b816781e3b2 100644 --- a/packages/fxa-settings/src/pages/Signup/ConfirmSignupCode/container.tsx +++ b/packages/fxa-settings/src/pages/Signup/ConfirmSignupCode/container.tsx @@ -6,8 +6,15 @@ import React, { useCallback, useEffect } from 'react'; import { RouteComponentProps, useLocation } from '@reach/router'; import { useNavigateWithQuery as useNavigate } from '../../../lib/hooks/useNavigateWithQuery'; import { currentAccount } from '../../../lib/cache'; -import { useFinishOAuthFlowHandler } from '../../../lib/oauth/hooks'; -import { Integration, useAuthClient } from '../../../models'; +import { + useFinishOAuthFlowHandler, + useOAuthKeysCheck, +} from '../../../lib/oauth/hooks'; +import { + Integration, + useAuthClient, + useSensitiveDataClient, +} from '../../../models'; import ConfirmSignupCode from '.'; import { hardNavigate } from 'fxa-react/lib/utils'; import LoadingSpinner from 'fxa-react/components/LoadingSpinner'; @@ -16,6 +23,10 @@ import { useQuery } from '@apollo/client'; import { EMAIL_BOUNCE_STATUS_QUERY } from './gql'; import OAuthDataError from '../../../components/OAuthDataError'; import { QueryParams } from '../../..'; +import { + AUTH_DATA_KEY, + SensitiveDataClientAuthKeys, +} from '../../../lib/sensitive-data-client'; export const POLL_INTERVAL = 5000; @@ -45,6 +56,17 @@ const SignupConfirmCodeContainer = ({ flowQueryParams: QueryParams; } & RouteComponentProps) => { const authClient = useAuthClient(); + const sensitiveDataClient = useSensitiveDataClient(); + const sensitiveData = sensitiveDataClient.getData(AUTH_DATA_KEY); + const { keyFetchToken, unwrapBKey } = + (sensitiveData as SensitiveDataClientAuthKeys) || {}; + + const { oAuthKeysCheckError } = useOAuthKeysCheck( + integration, + keyFetchToken, + unwrapBKey + ); + const location = useLocation() as ReturnType & { state: LocationState; }; @@ -53,8 +75,6 @@ const SignupConfirmCodeContainer = ({ selectedNewsletterSlugs: newsletterSlugs, offeredSyncEngines, declinedSyncEngines, - keyFetchToken, - unwrapBKey, sessionToken: sessionTokenFromLocationState, email: emailFromLocationState, uid: uidFromLocationState, @@ -137,6 +157,9 @@ const SignupConfirmCodeContainer = ({ if (oAuthDataError) { return ; } + if (oAuthKeysCheckError) { + return ; + } return ( & +} & SensitiveDataClientAuthKeys & RouteComponentProps; export interface ConfirmSignupCodeFormData { diff --git a/packages/fxa-settings/src/pages/Signup/index.test.tsx b/packages/fxa-settings/src/pages/Signup/index.test.tsx index 030781d8b6d..05e40b6770f 100644 --- a/packages/fxa-settings/src/pages/Signup/index.test.tsx +++ b/packages/fxa-settings/src/pages/Signup/index.test.tsx @@ -24,12 +24,7 @@ import { createMockSignupOAuthNativeIntegration, createMockSignupSyncDesktopV3Integration, } from './mocks'; -import { - MOCK_EMAIL, - MOCK_KEY_FETCH_TOKEN, - MOCK_PASSWORD, - MOCK_UNWRAP_BKEY, -} from '../mocks'; +import { MOCK_EMAIL, MOCK_PASSWORD } from '../mocks'; import { newsletters } from '../../components/ChooseNewsletters/newsletters'; import firefox from '../../lib/channels/firefox'; import GleanMetrics from '../../lib/glean'; @@ -40,6 +35,9 @@ import { syncEngineConfigs, } from '../../components/ChooseWhatToSync/sync-engines'; import { AuthUiErrors } from '../../lib/auth-errors/auth-errors'; +import { AUTH_DATA_KEY } from '../../lib/sensitive-data-client'; +import { mockSensitiveDataClient as createMockSensitiveDataClient } from '../../models/mocks'; +import { useSensitiveDataClient } from '../../models'; jest.mock('../../lib/metrics', () => ({ usePageViewEvent: jest.fn(), @@ -83,6 +81,16 @@ jest.mock('../../lib/glean', () => ({ }, })); +jest.mock('../../models', () => { + return { + ...jest.requireActual('../../models'), + useSensitiveDataClient: jest.fn(), + }; +}); + +const mockSensitiveDataClient = createMockSensitiveDataClient(); +mockSensitiveDataClient.setData = jest.fn(); + const oauthCommonFxaLoginOptions = { email: MOCK_EMAIL, sessionToken: BEGIN_SIGNUP_HANDLER_RESPONSE.data.signUp.sessionToken, @@ -100,6 +108,10 @@ describe('Signup page', () => { beforeEach(() => { jest.resetAllMocks(); cleanup(); + + (useSensitiveDataClient as jest.Mock).mockImplementation( + () => mockSensitiveDataClient + ); }); // TODO: enable l10n tests when they've been updated to handle embedded tags in ftl strings // TODO: in FXA-6461 @@ -332,6 +344,27 @@ describe('Signup page', () => { fireEvent.click(screen.getByRole('button', { name: 'Create account' })); } + it('sets data on sensitive data client', async () => { + const mockBeginSignupHandler = jest + .fn() + .mockResolvedValue(BEGIN_SIGNUP_HANDLER_RESPONSE); + renderWithLocalizationProvider( + + ); + await fillOutForm(); + submit(); + await waitFor(() => { + expect(mockSensitiveDataClient.setData).toHaveBeenCalledWith( + AUTH_DATA_KEY, + { + keyFetchToken: + BEGIN_SIGNUP_HANDLER_RESPONSE.data.signUp.keyFetchToken, + unwrapBKey: BEGIN_SIGNUP_HANDLER_RESPONSE.data.unwrapBKey, + } + ); + }); + }); + describe('cookies', () => { // Clean up cookies to start with a clean slate and avoid polluting other tests. const originalCookie = document.cookie; @@ -491,7 +524,6 @@ describe('Signup page', () => { `/confirm_signup_code${mockLocation().search}`, { state: { - keyFetchToken: MOCK_KEY_FETCH_TOKEN, origin: 'signup', // we expect three newsletter options, but 4 slugs should be passed // because the first newsletter checkbox subscribes the user to 2 newsletters @@ -501,7 +533,6 @@ describe('Signup page', () => { 'mozilla-foundation', 'test-pilot', ], - unwrapBKey: MOCK_UNWRAP_BKEY, }, replace: true, } @@ -551,10 +582,8 @@ describe('Signup page', () => { `/confirm_signup_code${mockLocation().search}`, { state: { - keyFetchToken: MOCK_KEY_FETCH_TOKEN, origin: 'signup', selectedNewsletterSlugs: [], - unwrapBKey: MOCK_UNWRAP_BKEY, }, replace: true, } diff --git a/packages/fxa-settings/src/pages/Signup/index.tsx b/packages/fxa-settings/src/pages/Signup/index.tsx index ab2b2c1570a..ac786a40ffc 100644 --- a/packages/fxa-settings/src/pages/Signup/index.tsx +++ b/packages/fxa-settings/src/pages/Signup/index.tsx @@ -43,6 +43,7 @@ import { isOAuthNativeIntegrationSync, isSyncDesktopV3Integration, useFtlMsgResolver, + useSensitiveDataClient, } from '../../models'; import { isClientMonitor, @@ -50,6 +51,7 @@ import { } from '../../models/integrations/client-matching'; import { SignupFormData, SignupProps } from './interfaces'; import Banner from '../../components/Banner'; +import { AUTH_DATA_KEY } from '../../lib/sensitive-data-client'; export const viewName = 'signup'; @@ -59,6 +61,7 @@ export const Signup = ({ beginSignupHandler, webChannelEngines, }: SignupProps) => { + const sensitiveDataClient = useSensitiveDataClient(); usePageViewEvent(viewName, REACT_ENTRYPOINT); useEffect(() => { @@ -244,6 +247,12 @@ export const Signup = ({ // this allows the recent account to be used for /signin storeAccountData(accountData); + // Set these for use in ConfirmSignupCode + sensitiveDataClient.setData(AUTH_DATA_KEY, { + keyFetchToken: data.signUp.keyFetchToken, + unwrapBKey: data.unwrapBKey, + }); + const getOfferedSyncEngines = () => getSyncEngineIds(offeredSyncEngineConfigs || []); @@ -303,8 +312,6 @@ export const Signup = ({ state: { origin: 'signup', selectedNewsletterSlugs, - keyFetchToken: data.signUp.keyFetchToken, - unwrapBKey: data.unwrapBKey, // Sync desktop v3 sends a web channel message up on Signup // while OAuth Sync does on confirm signup ...(isSyncOAuth && { @@ -338,6 +345,7 @@ export const Signup = ({ localizedValidAgeError, isDesktopRelay, isOAuth, + sensitiveDataClient, ] ); diff --git a/packages/fxa-settings/src/pages/Signup/interfaces.ts b/packages/fxa-settings/src/pages/Signup/interfaces.ts index 591b2283dc7..955df92f394 100644 --- a/packages/fxa-settings/src/pages/Signup/interfaces.ts +++ b/packages/fxa-settings/src/pages/Signup/interfaces.ts @@ -61,7 +61,12 @@ export type SignupOAuthIntegration = Pick< export type SignupBaseIntegration = Pick< BaseIntegration, - 'type' | 'isSync' | 'getService' | 'isDesktopRelay' | 'wantsKeys' | 'getClientId' + | 'type' + | 'isSync' + | 'getService' + | 'isDesktopRelay' + | 'wantsKeys' + | 'getClientId' >; export interface SignupFormData {