Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ test.describe('severity-1 #smoke', () => {
await expect(settings.alertBar).toHaveText('Recovery phone added');

await settings.totp.removeRecoveryPhoneButton.click();

await page.waitForURL(/recovery_phone\/remove/);
await page.getByRole('button', { name: 'Remove phone number' }).click();

Expand Down
38 changes: 24 additions & 14 deletions packages/fxa-auth-client/lib/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1531,14 +1531,16 @@ export default class AuthClient {
} = {},
headers?: Headers
): Promise<SignedInAccountData> {

const oldCredentials = await this.sessionReauth(sessionToken, options.reauthEmail || email, oldPassword, {
keys: true
}, headers);
const oldCredentialsAuth = await crypto.getCredentials(
email,
oldPassword
const oldCredentials = await this.sessionReauth(
sessionToken,
options.reauthEmail || email,
oldPassword,
{
keys: true,
},
headers
);
const oldCredentialsAuth = await crypto.getCredentials(email, oldPassword);
const oldAuthPW = oldCredentialsAuth.authPW;

const keys = await this.accountKeys(
Expand All @@ -1547,10 +1549,7 @@ export default class AuthClient {
headers
);

const newCredentials = await crypto.getCredentials(
email,
newPassword
);
const newCredentials = await crypto.getCredentials(email, newPassword);

const wrapKb = crypto.unwrapKB(keys.kB, newCredentials.unwrapBKey);
const authPW = newCredentials.authPW;
Expand Down Expand Up @@ -1600,8 +1599,8 @@ export default class AuthClient {
if (
error &&
error.email &&
error.errno === ERRORS.INCORRECT_EMAIL_CASE
&& !options.skipCaseError
error.errno === ERRORS.INCORRECT_EMAIL_CASE &&
!options.skipCaseError
) {
options.skipCaseError = true;
options.reauthEmail = email;
Expand Down Expand Up @@ -2979,7 +2978,7 @@ export default class AuthClient {
}

/**
* Removes a recovery phone from the user's account
* @deprecated Use recoveryPhoneDeleteWithJwt instead
*
* @param sessionToken The user's current session token
* @param headers
Expand All @@ -2988,6 +2987,17 @@ export default class AuthClient {
return this.sessionDelete('/recovery_phone', sessionToken, {}, headers);
}

/**
* Disables 2FA Protection on the account.
*
* @param jwt - required, must be a verified session token
* @param headers - Optional additional headers for the request
* @returns A promise that resolves when the 2FA has been removed
*/
async recoveryPhoneDeleteWithJwt(jwt: string, headers?: Headers) {
return this.jwtDelete('/mfa/recovery_phone', jwt, {}, headers);
}

/**
* Gets status of the recovery phone on the users account.
* @param sessionToken The user's current session token
Expand Down
20 changes: 20 additions & 0 deletions packages/fxa-auth-server/lib/routes/recovery-phone.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1193,6 +1193,26 @@ export const recoveryPhoneRoutes = (
return recoveryPhoneHandler.destroy(request);
},
},
{
method: 'DELETE',
path: '/mfa/recovery_phone',
options: {
auth: {
strategy: 'mfa',
scope: ['mfa:2fa'],
payload: false,
},
response: {},
},
handler: async function (request: AuthRequest) {
return routes
.find(
(route) =>
route.path === '/v1/recovery_phone' && route.method === 'DELETE'
)
?.handler(request);
},
},
{
method: 'GET',
path: '/recovery_phone',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import React from 'react';
import { Meta } from '@storybook/react';
import { withLocalization } from 'fxa-react/lib/storybooks';
import PageRecoveryPhoneRemove from '.';
import { PageRecoveryPhoneRemove } from '.';
import { LocationProvider } from '@reach/router';
import { mockAppContext, MOCK_ACCOUNT } from '../../../models/mocks';
import { Account, AppContext } from '../../../models';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import React from 'react';
import PageRecoveryPhoneRemove from '.';
import { PageRecoveryPhoneRemove } from '.';
import { act, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { Account, AppContext } from '../../../models';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import FlowContainer from '../FlowContainer';
import { getLocalizedErrorMessage } from '../../../lib/error-utils';
import GleanMetrics from '../../../lib/glean';
import { useNavigateWithQuery } from '../../../lib/hooks/useNavigateWithQuery';
import { MfaGuard } from '../MfaGuard';

// TODO, update this link with #section-heading once the SUMO article is updated (FXA-10918)
const sumoTwoStepLink = (
Expand All @@ -26,7 +27,7 @@ const sumoTwoStepLink = (
</LinkExternal>
);

const PageRecoveryPhoneRemove = (props: RouteComponentProps) => {
export const PageRecoveryPhoneRemove = (props: RouteComponentProps) => {
const navigateWithQuery = useNavigateWithQuery();
const account = useAccount();
const alertBar = useAlertBar();
Expand Down Expand Up @@ -140,4 +141,10 @@ const PageRecoveryPhoneRemove = (props: RouteComponentProps) => {
);
};

export default PageRecoveryPhoneRemove;
export const PageMfaGuardRecoveryPhoneRemove = (props: RouteComponentProps) => {
return (
<MfaGuard requiredScope="2fa">
<PageRecoveryPhoneRemove {...props} />
</MfaGuard>
);
};
11 changes: 8 additions & 3 deletions packages/fxa-settings/src/components/Settings/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,9 @@ describe('Settings App', () => {
});
it('routes to PageChangePassword', async () => {
// Suppress MFA guard for this test
mockMfaGuard.mockImplementationOnce(({ children }: { children: ReactNode }) => (
<>{children}</>
));
mockMfaGuard.mockImplementationOnce(
({ children }: { children: ReactNode }) => <>{children}</>
);

// Mock the JWT token cache to suppress MFA guard
const mockJwtTokenCache = {
Expand Down Expand Up @@ -373,6 +373,11 @@ describe('Settings App', () => {
route: '/change_password',
hasPassword: true,
},
{
pageName: 'PageRecoveryPhoneRemove',
route: '/recovery_phone/remove',
hasPassword: true,
},
];

it.each(guardedRoutes)(
Expand Down
4 changes: 2 additions & 2 deletions packages/fxa-settings/src/components/Settings/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import { currentAccount } from '../../lib/cache';
import { hasAccount, setCurrentAccount } from '../../lib/storage-utils';
import GleanMetrics from '../../lib/glean';
import Head from 'fxa-react/components/Head';
import PageRecoveryPhoneRemove from './PageRecoveryPhoneRemove';
import { PageMfaGuardRecoveryPhoneRemove } from './PageRecoveryPhoneRemove';
import { SettingsIntegration } from './interfaces';
import { useNavigateWithQuery } from '../../lib/hooks/useNavigateWithQuery';
import MfaGuardPage2faChange from './Page2faChange';
Expand Down Expand Up @@ -197,7 +197,7 @@ export const Settings = ({
<Redirect from="/avatar/change" to="/settings/avatar/" noThrow />

<MfaGuardPageRecoveryPhoneSetup path="/recovery_phone/setup" />
<PageRecoveryPhoneRemove path="/recovery_phone/remove" />
<PageMfaGuardRecoveryPhoneRemove path="/recovery_phone/remove" />

<PageMfaGuardTestWithAuthClient path="/mfa_guard/test/auth_client" />
<PageMfaGuardTestWithGql path="/mfa_guard/test/gql" />
Expand Down
3 changes: 2 additions & 1 deletion packages/fxa-settings/src/models/Account.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1566,8 +1566,9 @@ export class Account implements AccountData {
}

async removeRecoveryPhone() {
const jwt = this.getCachedJwtByScope('2fa');
const result = await this.withLoadingStatus(
this.authClient.recoveryPhoneDelete(sessionToken()!)
this.authClient.recoveryPhoneDeleteWithJwt(jwt)
);
return result;
}
Expand Down