Skip to content

Commit 74ffb25

Browse files
Merge pull request #19452 from mozilla/FXA-12226
feat(mfa): wrap change primary email action with MFA guard
2 parents 6a97921 + 4e3cc09 commit 74ffb25

10 files changed

Lines changed: 312 additions & 92 deletions

File tree

packages/functional-tests/tests/settings/changeEmail.spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ test.describe('severity-1 #smoke', () => {
124124

125125
// Change back the primary email again
126126
await settings.secondaryEmail.makePrimaryButton.click();
127+
await settings.confirmMfaGuard(secondEmail);
127128
await settings.signOut();
128129

129130
// Login with primary email and new password

packages/functional-tests/tests/settings/changeEmailBlocked.spec.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,13 @@ test.describe('severity-1 #smoke', () => {
2828
const invalidPassword = testAccountTracker.generatePassword();
2929

3030
await settings.goto();
31-
await changePrimaryEmail(target, settings, secondaryEmail, blockedEmail, credentials.email);
31+
await changePrimaryEmail(
32+
target,
33+
settings,
34+
secondaryEmail,
35+
blockedEmail,
36+
credentials.email
37+
);
3238
await settings.signOut();
3339
await signin.fillOutEmailFirstForm(blockedEmail);
3440
await signin.fillOutPasswordForm(invalidPassword);
@@ -47,6 +53,7 @@ test.describe('severity-1 #smoke', () => {
4753
await expect(settings.settingsHeading).toBeVisible();
4854
// reset primary email to non-blocked email for account cleanup
4955
await settings.secondaryEmail.makePrimaryButton.click();
56+
await settings.confirmMfaGuard(blockedEmail);
5057
await expect(settings.alertBar).toHaveText(
5158
new RegExp(`${credentials.email}.*is now your primary email`)
5259
);
@@ -67,7 +74,13 @@ test.describe('severity-1 #smoke', () => {
6774
const blockedEmail = testAccountTracker.generateBlockedEmail();
6875

6976
await settings.goto();
70-
await changePrimaryEmail(target, settings, secondaryEmail, blockedEmail, credentials.email);
77+
await changePrimaryEmail(
78+
target,
79+
settings,
80+
secondaryEmail,
81+
blockedEmail,
82+
credentials.email
83+
);
7184
await settings.signOut();
7285

7386
await signin.fillOutEmailFirstForm(blockedEmail);
@@ -81,6 +94,7 @@ test.describe('severity-1 #smoke', () => {
8194

8295
// reset primary email to non-blocked email for account cleanup
8396
await settings.secondaryEmail.makePrimaryButton.click();
97+
await settings.confirmMfaGuard(blockedEmail);
8498
await expect(settings.alertBar).toHaveText(
8599
new RegExp(`${credentials.email}.*is now your primary email`)
86100
);
@@ -111,7 +125,7 @@ async function changePrimaryEmail(
111125
settings: SettingsPage,
112126
secondaryEmail: SecondaryEmailPage,
113127
email: string,
114-
primaryEmail: string,
128+
primaryEmail: string
115129
): Promise<void> {
116130
await settings.secondaryEmail.addButton.click();
117131
await settings.confirmMfaGuard(primaryEmail);

packages/fxa-auth-client/lib/client.ts

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1713,6 +1713,9 @@ export default class AuthClient {
17131713
return this.jwtPost('/mfa/recovery_email/destroy', jwt, { email }, headers);
17141714
}
17151715

1716+
/**
1717+
* @deprecated Use recoveryEmailSetPrimaryEmailWithJwt instead
1718+
*/
17161719
async recoveryEmailSetPrimaryEmail(
17171720
sessionToken: hexstring,
17181721
email: string,
@@ -1721,9 +1724,27 @@ export default class AuthClient {
17211724
return this.sessionPost(
17221725
'/recovery_email/set_primary',
17231726
sessionToken,
1724-
{
1725-
email,
1726-
},
1727+
{ email },
1728+
headers
1729+
);
1730+
}
1731+
1732+
/**
1733+
* Sets a secondary email as the primary email for the account.
1734+
* @param jwt Required scope 'mfa:email'
1735+
* @param email a secondary email to set as primary
1736+
* @param headers
1737+
* @returns Response
1738+
*/
1739+
async recoveryEmailSetPrimaryEmailWithJwt(
1740+
jwt: string,
1741+
email: string,
1742+
headers?: Headers
1743+
) {
1744+
return this.jwtPost(
1745+
'/mfa/recovery_email/set_primary',
1746+
jwt,
1747+
{ email },
17271748
headers
17281749
);
17291750
}

packages/fxa-auth-server/docs/swagger/emails-api.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,38 @@ const RECOVERY_EMAIL_SET_PRIMARY_POST = {
212212
},
213213
};
214214

215+
const MFA_RECOVERY_EMAIL_SET_PRIMARY_POST = {
216+
...TAGS_EMAILS,
217+
description: '/mfa/recovery_email/set_primary',
218+
notes: [
219+
dedent`
220+
🔒 Authenticated with session MFA JWT (scope: mfa:email)
221+
222+
This endpoint changes a user's primary email address. This email address must belong to the user and be verified.
223+
`,
224+
],
225+
plugins: {
226+
'hapi-swagger': {
227+
responses: {
228+
400: {
229+
description: dedent`
230+
Failing requests may be caused by the following errors (this is not an exhaustive list):
231+
- \`errno: 138\` - Unverified session
232+
- \`errno: 147\` - Can not change primary email to an unverified email
233+
- \`errno: 148\` - Can not change primary email to an email that does not belong to this account
234+
`,
235+
},
236+
401: {
237+
description: dedent`
238+
Failing requests may be caused by the following errors (this is not an exhaustive list):
239+
- \`errno: 110\` - Invalid authentication token in request signature
240+
`,
241+
},
242+
},
243+
},
244+
},
245+
};
246+
215247
const RECOVERY_EMAIL_SECONDARY_RESEND_CODE_POST = {
216248
...TAGS_EMAILS,
217249
description: '/recovery_email/secondary/resend_code',
@@ -271,6 +303,7 @@ const API_DOCS = {
271303
RECOVERY_EMAIL_SECONDARY_RESEND_CODE_POST,
272304
RECOVERY_EMAIL_SECONDARY_VERIFY_CODE_POST,
273305
RECOVERY_EMAIL_SET_PRIMARY_POST,
306+
MFA_RECOVERY_EMAIL_SET_PRIMARY_POST,
274307
RECOVERY_EMAIL_STATUS_GET,
275308
RECOVERY_EMAIL_VERIFY_CODE_POST,
276309
RECOVERY_EMAILS_GET,

packages/fxa-auth-server/lib/routes/emails.js

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1008,6 +1008,35 @@ module.exports = (
10081008
return {};
10091009
},
10101010
},
1011+
{
1012+
method: 'POST',
1013+
path: '/mfa/recovery_email/set_primary',
1014+
options: {
1015+
...EMAILS_DOCS.MFA_RECOVERY_EMAIL_SET_PRIMARY_POST,
1016+
auth: {
1017+
strategy: 'mfa',
1018+
scope: ['mfa:email'],
1019+
payload: false,
1020+
},
1021+
validate: {
1022+
payload: isA.object({
1023+
email: validators
1024+
.email()
1025+
.required()
1026+
.description(DESCRIPTION.emailNewPrimary),
1027+
}),
1028+
},
1029+
response: {},
1030+
},
1031+
handler: async function (request) {
1032+
return routes
1033+
.find(
1034+
(r) =>
1035+
r.path === '/v1/recovery_email/set_primary' && r.method === 'POST'
1036+
)
1037+
.handler(request);
1038+
},
1039+
},
10111040
{
10121041
method: 'POST',
10131042
path: '/recovery_email/secondary/resend_code',

packages/fxa-graphql-api/src/gql/account.resolver.spec.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -450,15 +450,17 @@ describe('#integration - AccountResolver', () => {
450450

451451
describe('updatePrimaryEmail', () => {
452452
it('succeeds', async () => {
453-
authClient.recoveryEmailSetPrimaryEmail = jest
453+
authClient.recoveryEmailSetPrimaryEmailWithJwt = jest
454454
.fn()
455455
.mockResolvedValue(true);
456-
const result = await resolver.updatePrimaryEmail('token', headers, {
456+
const result = await resolver.updatePrimaryEmail(headers, {
457457
jwt: 'jwtToken',
458458
clientMutationId: 'testid',
459459
460460
});
461-
expect(authClient.recoveryEmailSetPrimaryEmail).toBeCalledTimes(1);
461+
expect(authClient.recoveryEmailSetPrimaryEmailWithJwt).toBeCalledTimes(
462+
1
463+
);
462464
expect(result).toStrictEqual({
463465
clientMutationId: 'testid',
464466
});

packages/fxa-graphql-api/src/gql/account.resolver.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -376,12 +376,11 @@ export class AccountResolver {
376376
@UseGuards(GqlAuthGuard)
377377
@CatchGatewayError
378378
public async updatePrimaryEmail(
379-
@GqlSessionToken() token: string,
380379
@GqlXHeaders() headers: Headers,
381380
@Args('input', { type: () => EmailInput }) input: EmailInput
382381
): Promise<BasicPayload> {
383-
await this.authAPI.recoveryEmailSetPrimaryEmail(
384-
token,
382+
await this.authAPI.recoveryEmailSetPrimaryEmailWithJwt(
383+
input.jwt,
385384
input.email,
386385
headers
387386
);

0 commit comments

Comments
 (0)