Skip to content

Commit

Permalink
Merge pull request #16869 from mozilla/FXA-7890
Browse files Browse the repository at this point in the history
feat(reset-password): Use OTP instead of link to start reset (front-end)
  • Loading branch information
vpomerleau authored May 22, 2024
2 parents abc3eb1 + f9af7b1 commit 931d425
Show file tree
Hide file tree
Showing 68 changed files with 2,321 additions and 156 deletions.
2 changes: 2 additions & 0 deletions packages/functional-tests/lib/email.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export enum EmailType {
newDeviceLogin,
passwordChanged,
passwordChangeRequired,
passwordForgotOtp,
passwordReset,
passwordResetAccountRecovery,
passwordResetRequired,
Expand Down Expand Up @@ -71,6 +72,7 @@ export enum EmailHeader {
link = 'x-link',
templateName = 'x-template-name',
templateVersion = 'x-template-version',
resetPasswordCode = 'x-password-forgot-otp',
}

export class EmailClient {
Expand Down
33 changes: 26 additions & 7 deletions packages/functional-tests/pages/resetPasswordReact.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,44 @@ export class ResetPasswordReactPage extends BaseLayout {
readonly path = '';

get resetPasswordHeading() {
return this.page.getByRole('heading', { name: /^Reset password/ });
return (
this.page
.getByRole('heading', { name: /^Reset password/ })
// for password reset redesign, with resetPasswordWithCode flag
.or(this.page.getByRole('heading', { name: /^Password reset/ }))
);
}

get emailTextbox() {
return this.page.getByRole('textbox', { name: 'email' });
}

get beginResetButton() {
return this.page.getByRole('button', { name: 'Begin reset' });
return (
this.page
.getByRole('button', { name: 'Begin reset' })
// for password reset redesign, with resetPasswordWithCode flag
.or(
this.page.getByRole('button', { name: 'Send me reset instructions' })
)
);
}

get resetEmailSentHeading() {
return this.page.getByRole('heading', { name: 'Reset email sent' });
get confirmResetPasswordHeading() {
return (
this.page
.getByRole('heading', { name: 'Reset email sent' })
// for password reset redesign, with resetPasswordWithCode flag
.or(this.page.getByRole('heading', { name: 'Enter confirmation code' }))
);
}

get resendButton() {
return this.page.getByRole('button', {
name: 'Not in inbox or spam folder? Resend',
});
return this.page
.getByRole('button', {
name: 'Not in inbox or spam folder? Resend',
}) // for password reset redesign, with resetPasswordWithCode flag
.or(this.page.getByRole('button', { name: 'Resend code' }));
}

get statusBar() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ test.describe('severity-2 #smoke', () => {
page,
target,
pages: {
configPage,
signinReact,
signupReact,
settings,
Expand All @@ -60,6 +61,11 @@ test.describe('severity-2 #smoke', () => {
},
testAccountTracker,
}) => {
const config = await configPage.getConfig();
test.fixme(
config.featureFlags.resetPasswordWithCode === true,
'see FXA-9612'
);
const { email, password } = testAccountTracker.generateAccountDetails();
await page.goto(
`${target.contentServerUrl}/?showReactApp=true&forceExperiment=generalizedReactApp&forceExperimentGroup=react&${signup.query}`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,20 @@ test.describe('severity-2 #smoke', () => {
test(`signs up as v${signup.version} resets password as v${reset.version} and signs in as v${signin.version}`, async ({
page,
target,
pages: { signinReact, signupReact, settings, resetPasswordReact },
pages: {
configPage,
signinReact,
signupReact,
settings,
resetPasswordReact,
},
testAccountTracker,
}) => {
const config = await configPage.getConfig();
test.fixme(
config.featureFlags.resetPasswordWithCode === true,
'see FXA-9612'
);
const { email, password } = testAccountTracker.generateAccountDetails();
await page.goto(
`${target.contentServerUrl}/?showReactApp=true&forceExperiment=generalizedReactApp&forceExperimentGroup=react&${signup.query}`
Expand Down
2 changes: 2 additions & 0 deletions packages/functional-tests/tests/misc/keyStretchingV2.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { SignupReactPage } from '../../pages/signupReact';
import { TotpPage } from '../../pages/settings/totp';
import { getCode } from 'fxa-settings/src/lib/totp';
import { DeleteAccountPage } from '../../pages/settings/deleteAccount';
import { ConfigPage } from '../../pages/config';

// Disable this check for these tests. We are holding assertion in shared functions due
// to how test permutations work, and these setup falsely trips this rule.
Expand Down Expand Up @@ -47,6 +48,7 @@ test.describe('key-stretching-v2', () => {
recoveryKey: RecoveryKeyPage;
totp: TotpPage;
deleteAccount: DeleteAccountPage;
configPage: ConfigPage;
};

// Helpers
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ test.describe('severity-1 #smoke', () => {
test.beforeEach(async ({ pages: { configPage } }) => {
const config = await configPage.getConfig();
test.skip(config.showReactApp.resetPasswordRoutes !== true);
test.fixme(
config.featureFlags.resetPasswordWithCode === true,
'see FXA-9612'
);
test.slow();
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ test.describe('severity-1 #smoke', () => {
// Ensure that the react reset password route feature flag is enabled
const config = await configPage.getConfig();
test.skip(config.showReactApp.resetPasswordRoutes !== true);
test.fixme(
config.featureFlags.resetPasswordWithCode === true,
'see FXA-9612'
);
test.slow();
});

Expand Down Expand Up @@ -64,7 +68,9 @@ test.describe('severity-1 #smoke', () => {
await resetPasswordReact.goto();

await resetPasswordReact.fillOutEmailForm(credentials.email);
await expect(resetPasswordReact.resetEmailSentHeading).toBeVisible();
await expect(
resetPasswordReact.confirmResetPasswordHeading
).toBeVisible();

const link = await target.emailClient.waitForEmail(
credentials.email,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,12 @@ import { ResetPasswordReactPage } from '../../pages/resetPasswordReact';
test.describe('severity-1 #smoke', () => {
test.describe('reset password react', () => {
test.beforeEach(async ({ pages: { configPage } }) => {
// Ensure that the feature flag is enabled
const config = await configPage.getConfig();
test.skip(config.showReactApp.resetPasswordRoutes !== true);
test.fixme(
config.featureFlags.resetPasswordWithCode === true,
'see FXA-9612'
);
test.slow();
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,13 @@ import { getReactFeatureFlagUrl } from '../../lib/react-flag';
test.describe('severity-1 #smoke', () => {
test.describe('Firefox Desktop Sync v3 reset password react', () => {
test.beforeEach(async ({ pages: { configPage } }) => {
test.slow();
// Ensure that the feature flag is enabled
const config = await configPage.getConfig();
test.skip(config.showReactApp.resetPasswordRoutes !== true);
test.fixme(
config.featureFlags.resetPasswordWithCode === true,
'see FXA-9612'
);
test.slow();
});

test('reset pw for sync user', async ({
Expand Down
14 changes: 13 additions & 1 deletion packages/functional-tests/tests/settings/changePassword.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,21 @@ test.describe('severity-1 #smoke', () => {

test('reset password via settings works', async ({
target,
pages: { page, login, settings, changePassword, resetPasswordReact },
pages: {
configPage,
page,
login,
settings,
changePassword,
resetPasswordReact,
},
testAccountTracker,
}) => {
const config = await configPage.getConfig();
test.fixme(
config.featureFlags.resetPasswordWithCode === true,
'see FXA-9612'
);
test.slow();

await signInAccount(target, page, login, testAccountTracker);
Expand Down
8 changes: 7 additions & 1 deletion packages/functional-tests/tests/signin/signIn.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,15 @@ test.describe('severity-2 #smoke', () => {
test('signin verified with incorrect password, click `forgot password?`', async ({
target,
page,
pages: { login, resetPassword },
pages: { configPage, login, resetPassword },
testAccountTracker,
}) => {
const config = await configPage.getConfig();
test.fixme(
config.featureFlags.resetPasswordWithCode === true,
'see FXA-9612'
);

const credentials = await testAccountTracker.signUp();

await page.goto(target.contentServerUrl);
Expand Down
3 changes: 3 additions & 0 deletions packages/fxa-auth-server/config/dev.json
Original file line number Diff line number Diff line change
Expand Up @@ -467,5 +467,8 @@
"subscriptionAccountReminders": {
"firstInterval": "5s",
"secondInterval": "10s"
},
"passwordForgotOtp": {
"enabled": true
}
}
4 changes: 3 additions & 1 deletion packages/fxa-auth-server/lib/routes/password.ts
Original file line number Diff line number Diff line change
Expand Up @@ -700,8 +700,10 @@ module.exports = function (
statsd.increment('otp.passwordForgot.verified');

return {
token: passwordForgotToken.data,
code: passwordForgotToken.passCode,
emailToHashWith: passwordForgotToken.email,
token: passwordForgotToken.data,
uid: passwordForgotToken.uid,
};
},
},
Expand Down
12 changes: 2 additions & 10 deletions packages/fxa-auth-server/lib/senders/email.js
Original file line number Diff line number Diff line change
Expand Up @@ -1129,20 +1129,14 @@ module.exports = function (log, config, bounces) {

const templateName = 'passwordForgotOtp';
const code = message.code;
const links = this._generateLinks(
this.initiatePasswordChangeUrl,
message,
{},
templateName
);
const links = this._generateLinks(undefined, message, {}, templateName);
const [time, date] = this._constructLocalTimeString(
message.timeZone,
message.acceptLanguage
);

const headers = {
'X-Password-Forgot-OTP': code,
'X-Link': links.passwordChangeLink,
'X-Password-Forgot-Otp': code,
};

return this.send({
Expand All @@ -1154,8 +1148,6 @@ module.exports = function (log, config, bounces) {
date,
device: this._formatUserAgentInfo(message),
email: message.email,
passwordChangeLink: links.passwordChangeLink,
passwordChangeLinkAttributes: links.passwordChangeLinkAttributes,
privacyUrl: links.privacyUrl,
supportLinkAttributes: links.supportLinkAttributes,
supportUrl: links.supportUrl,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ can obtain one at http://mozilla.org/MPL/2.0/. %>
<mj-section>
<mj-column>
<mj-text css-class="text-body-subtext">
<%- include('/partials/automatedEmailChangePassword/index.mjml') %>
<%- include('/partials/automatedEmailNoAction/index.mjml') %>
</mj-text>
</mj-column>
</mj-section>
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ const createStory = storyWithProps(
{
...MOCK_USER_INFO,
code: '96318398',
passwordChangeLink: 'http://localhost:3030/settings/change_password',
}
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ password-forgot-otp-code = "If yes, here is your confirmation code to proceed:"

password-forgot-otp-expiry-notice = "This code expires in 10 minutes."

<%- include('/partials/automatedEmailChangePassword/index.txt') %>
<%- include('/partials/automatedEmailNoAction/index.txt') %>
4 changes: 1 addition & 3 deletions packages/fxa-auth-server/test/local/senders/emails.ts
Original file line number Diff line number Diff line change
Expand Up @@ -898,7 +898,7 @@ const TESTS: [string, any, Record<string, any>?][] = [
['passwordForgotOtpEmail', new Map<string, Test | any>([
['subject', { test: 'equal', expected: 'Forgot your password?' }],
['headers', new Map([
['X-Password-Forgot-OTP', { test: 'equal', expected: MESSAGE.code}],
['X-Password-Forgot-Otp', { test: 'equal', expected: MESSAGE.code}],
['X-SES-MESSAGE-TAGS', { test: 'equal', expected: sesMessageTagsHeaderValue('passwordForgotOtp') }],
['X-Template-Name', { test: 'equal', expected: 'passwordForgotOtp' }],
['X-Template-Version', { test: 'equal', expected: TEMPLATE_VERSIONS.passwordForgotOtp }],
Expand All @@ -908,14 +908,12 @@ const TESTS: [string, any, Record<string, any>?][] = [
{ test: 'include', expected: 'We received a request for a password change on your Mozilla account from:' },
{ test: 'include', expected: MESSAGE.code },
{ test: 'include', expected: `${MESSAGE.device.uaBrowser} on ${MESSAGE.device.uaOS} ${MESSAGE.device.uaOSVersion}` },
{ test: 'include', expected: 'change your password' },
]],
['text', [
{ test: 'include', expected: 'Forgot your password?' },
{ test: 'include', expected: 'We received a request for a password change on your Mozilla account from:' },
{ test: 'include', expected: MESSAGE.code },
{ test: 'include', expected: `${MESSAGE.device.uaBrowser} on ${MESSAGE.device.uaOS} ${MESSAGE.device.uaOSVersion}` },
{ test: 'include', expected: 'change your password' },
]],
])],

Expand Down
3 changes: 3 additions & 0 deletions packages/fxa-auth-server/test/mail_helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ module.exports = (printLogs) => {
const vc = mail.headers['x-verify-code'];
const vsc = mail.headers['x-verify-short-code'];
const sc = mail.headers['x-signin-verify-code'];
const rpc = mail.headers['x-password-forgot-otp'];
const template = mail.headers['x-template-name'];

// Workaround because the email service wraps this header in `< >`.
Expand All @@ -62,6 +63,8 @@ module.exports = (printLogs) => {
} else if (uc) {
console.log('\x1B[36mUnblock code:', uc, '\x1B[39m');
console.log('\x1B[36mReport link:', rul, '\x1B[39m');
} else if (rpc) {
console.log('\x1B[36mReset password Otp:', rpc, '\x1B[39m');
} else if (TEMPLATES_WITH_NO_CODE.has(template)) {
console.log(`Notification email: ${template}`);
} else {
Expand Down
2 changes: 1 addition & 1 deletion packages/fxa-content-server/server/config/local.json-dist
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,6 @@
},
"featureFlags": {
"sendFxAStatusOnSettings": true,
"resetPasswordWithCode": false
"resetPasswordWithCode": true
}
}
4 changes: 4 additions & 0 deletions packages/fxa-content-server/server/lib/routes/get-index.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ module.exports = function (config) {
const FEATURE_FLAGS_FXA_STATUS_ON_SETTINGS = config.get(
'featureFlags.sendFxAStatusOnSettings'
);
const FEATURE_FLAGS_RESET_PWD_WITH_CODE = config.get(
'featureFlags.resetPasswordWithCode'
);
const GLEAN_ENABLED = config.get('glean.enabled');
const GLEAN_APPLICATION_ID = config.get('glean.applicationId');
const GLEAN_UPLOAD_ENABLED = config.get('glean.uploadEnabled');
Expand Down Expand Up @@ -110,6 +113,7 @@ module.exports = function (config) {
brandMessagingMode: BRAND_MESSAGING_MODE,
featureFlags: {
sendFxAStatusOnSettings: FEATURE_FLAGS_FXA_STATUS_ON_SETTINGS,
resetPasswordWithCode: FEATURE_FLAGS_RESET_PWD_WITH_CODE,
},
glean: {
// feature toggle
Expand Down
Loading

0 comments on commit 931d425

Please sign in to comment.