Skip to content

Commit 4882bcb

Browse files
authored
Merge pull request #20178 from mozilla/fxa-13227
fix(password): Passwordless feature flag gates signup only and add more functional tests
2 parents 0a86624 + 16230e1 commit 4882bcb

19 files changed

Lines changed: 977 additions & 249 deletions

File tree

packages/functional-tests/tests/cms/cms-2fa.spec.ts

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -207,9 +207,6 @@ test.describe('severity-1 #smoke', () => {
207207

208208
await assertCmsCustomization(page, {
209209
headline: 'Enter two-step authentication code',
210-
description: '',
211-
logoUrl:
212-
'https://accounts-cdn.stage.mozaws.net/other/123Done-blue-logo.svg',
213210
buttonColor: '#4845D2',
214211
buttonText: 'Confirm',
215212
});
@@ -349,9 +346,6 @@ test.describe('severity-1 #smoke', () => {
349346

350347
await assertCmsCustomization(page, {
351348
headline: 'Enter two-step authentication code',
352-
description: '',
353-
logoUrl:
354-
'https://accounts-cdn.stage.mozaws.net/other/123Done-blue-logo.svg',
355349
buttonColor: '#4845D2',
356350
buttonText: 'Confirm',
357351
});
@@ -364,8 +358,6 @@ test.describe('severity-1 #smoke', () => {
364358

365359
await assertCmsCustomization(page, {
366360
headline: 'Enter recovery code',
367-
logoUrl:
368-
'https://accounts-cdn.stage.mozaws.net/other/123Done-blue-logo.svg',
369361
buttonColor: '#4845D2',
370362
buttonText: 'Confirm',
371363
});
@@ -487,9 +479,6 @@ test.describe('severity-1 #smoke', () => {
487479

488480
await assertCmsCustomization(page, {
489481
headline: 'Enter two-step authentication code',
490-
description: '',
491-
logoUrl:
492-
'https://accounts-cdn.stage.mozaws.net/other/123Done-blue-logo.svg',
493482
buttonColor: '#4845D2',
494483
buttonText: 'Confirm',
495484
});
@@ -502,9 +491,6 @@ test.describe('severity-1 #smoke', () => {
502491

503492
await assertCmsCustomization(page, {
504493
headline: 'Enter backup authentication code',
505-
description: '',
506-
logoUrl:
507-
'https://accounts-cdn.stage.mozaws.net/other/123Done-blue-logo.svg',
508494
buttonColor: '#4845D2',
509495
buttonText: 'Confirm',
510496
});
@@ -630,9 +616,6 @@ test.describe('severity-1 #smoke', () => {
630616

631617
await assertCmsCustomization(page, {
632618
headline: 'Enter two-step authentication code',
633-
logoUrl:
634-
'https://accounts-cdn.stage.mozaws.net/other/firefox-browser-logo.svg',
635-
logoAlt: 'Firefox logo',
636619
buttonColor: '#FF630B',
637620
buttonText: 'Confirm',
638621
});

packages/functional-tests/tests/passwordless/signinPasswordless.spec.ts

Lines changed: 227 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,9 @@ test.describe('severity-1 #smoke', () => {
4545
// Clear any cached sessions
4646
await signin.clearCache();
4747

48-
// Now sign in via UI
49-
await relier.goto('force_passwordless=true');
48+
// Sign in via UI WITHOUT force_passwordless — existing passwordless
49+
// accounts are always redirected to OTP regardless of feature flag
50+
await relier.goto();
5051
await relier.clickEmailFirst();
5152
await signin.fillOutEmailFirstForm(email);
5253

@@ -669,8 +670,9 @@ test.describe('severity-1 #smoke', () => {
669670
// Clear browser cache
670671
await signin.clearCache();
671672

672-
// Now try passwordless flow via UI
673-
await relier.goto('force_passwordless=true');
673+
// Sign in via UI WITHOUT force_passwordless — existing passwordless
674+
// accounts (even with TOTP) are always redirected to OTP
675+
await relier.goto();
674676
await relier.clickEmailFirst();
675677
await signin.fillOutEmailFirstForm(email);
676678

@@ -727,6 +729,144 @@ test.describe('severity-1 #smoke', () => {
727729
});
728730

729731
test.describe('severity-2', () => {
732+
test.describe('Passwordless authentication - Cached login', () => {
733+
test('passwordless account navigating to content server redirects to OTP (not cached login)', async ({
734+
target,
735+
pages: { page, signin, relier, signinPasswordlessCode, settings },
736+
testAccountTracker,
737+
}) => {
738+
// Create passwordless account via 123done
739+
const { email } =
740+
testAccountTracker.generatePasswordlessAccountDetails();
741+
742+
await relier.goto('force_passwordless=true');
743+
await relier.clickEmailFirst();
744+
await signin.fillOutEmailFirstForm(email);
745+
746+
// Should redirect to passwordless code page
747+
await expect(page).toHaveURL(/signin_passwordless_code/);
748+
749+
// Get OTP code from email
750+
const code = await target.emailClient.getPasswordlessSignupCode(email);
751+
await signinPasswordlessCode.fillOutCodeForm(code);
752+
753+
// Should complete OAuth and redirect to RP
754+
expect(await relier.isLoggedIn()).toBe(true);
755+
756+
// Navigate to content server — passwordless accounts are always
757+
// redirected to OTP verification (container.tsx checks hasPassword=false
758+
// && passwordlessSupported=true and redirects before cached UI renders)
759+
await page.goto(target.contentServerUrl);
760+
761+
await expect(page).toHaveURL(/signin_passwordless_code/);
762+
await expect(signinPasswordlessCode.heading).toBeVisible();
763+
764+
// Complete OTP flow to reach settings
765+
const signinCode = await target.emailClient.getPasswordlessSigninCode(email);
766+
await signinPasswordlessCode.fillOutCodeForm(signinCode);
767+
768+
await expect(settings.settingsHeading).toBeVisible();
769+
});
770+
});
771+
772+
test.describe('Passwordless authentication - Edge cases', () => {
773+
test('direct /signin URL with email param redirects passwordless account to OTP', async ({
774+
target,
775+
pages: { page, signin, signinPasswordlessCode, settings },
776+
testAccountTracker,
777+
}) => {
778+
// Existing passwordless account accessed via direct /signin?email=...
779+
// should redirect to OTP page without needing force_passwordless flag.
780+
// The auth server bypasses the client allowlist for existing passwordless
781+
// accounts, so this works even with the Settings clientId.
782+
const { email } = await testAccountTracker.signUpPasswordless();
783+
784+
await signin.clearCache();
785+
786+
// Direct /signin URL with email query param (no force_passwordless)
787+
await page.goto(
788+
`${target.contentServerUrl}/signin?email=${encodeURIComponent(email)}`
789+
);
790+
await expect(page).toHaveURL(/signin_passwordless_code/);
791+
await expect(signinPasswordlessCode.heading).toBeVisible();
792+
793+
// Complete OTP flow — non-OAuth path should land on settings
794+
const code = await target.emailClient.getPasswordlessSigninCode(email);
795+
await signinPasswordlessCode.fillOutCodeForm(code);
796+
await expect(settings.settingsHeading).toBeVisible();
797+
});
798+
799+
test('passwordless account with invalidated cached session redirects to passwordless code (not password form)', async ({
800+
target,
801+
pages: { page, signin, relier, signinPasswordlessCode },
802+
testAccountTracker,
803+
}) => {
804+
const { email, sessionToken } =
805+
await testAccountTracker.signUpPasswordless();
806+
807+
// Destroy session server-side to simulate expiration/revocation
808+
await target.authClient.sessionDestroy(sessionToken);
809+
810+
// Navigate WITHOUT force_passwordless — existing passwordless accounts
811+
// are always redirected to OTP regardless of feature flag
812+
await relier.goto();
813+
await relier.clickEmailFirst();
814+
await signin.fillOutEmailFirstForm(email);
815+
816+
// Should go to passwordless code page, NOT a password form
817+
await expect(page).toHaveURL(/signin_passwordless_code/);
818+
await expect(signinPasswordlessCode.heading).toBeVisible();
819+
820+
const code = await target.emailClient.getPasswordlessSigninCode(email);
821+
await signinPasswordlessCode.fillOutCodeForm(code);
822+
expect(await relier.isLoggedIn()).toBe(true);
823+
});
824+
825+
test('password creation switches account to password flow; other passwordless accounts unaffected', async ({
826+
target,
827+
pages: { page, signin, relier, signinPasswordlessCode },
828+
testAccountTracker,
829+
}) => {
830+
// Create two passwordless accounts — one will get a password, the other stays passwordless
831+
const { email } = await testAccountTracker.signUpPasswordless();
832+
const account: any = testAccountTracker.accounts.find(
833+
(a) => a.email === email
834+
);
835+
const password = account?.password || '';
836+
const { email: otherPasswordlessEmail } =
837+
await testAccountTracker.signUpPasswordless();
838+
839+
// Create a password on the first account via API
840+
await target.authClient.passwordlessSendCode(email, {
841+
clientId: 'dcdb5ae7add825d2',
842+
});
843+
const otpCode =
844+
await target.emailClient.getPasswordlessSigninCode(email);
845+
const result = await target.authClient.passwordlessConfirmCode(
846+
email,
847+
otpCode,
848+
{ clientId: 'dcdb5ae7add825d2' }
849+
);
850+
await target.authClient.createPassword(result.sessionToken, email, password);
851+
account.isPasswordless = false;
852+
853+
// First account now has a password — should show password form
854+
await signin.clearCache();
855+
await relier.goto('force_passwordless=true');
856+
await relier.clickEmailFirst();
857+
await signin.fillOutEmailFirstForm(email);
858+
await expect(signin.passwordFormHeading).toBeVisible();
859+
await expect(page).not.toHaveURL(/signin_passwordless_code/);
860+
861+
// Second account is still passwordless — should get passwordless flow
862+
await relier.goto('force_passwordless=true');
863+
await relier.clickEmailFirst();
864+
await signin.fillOutEmailFirstForm(otherPasswordlessEmail);
865+
await expect(page).toHaveURL(/signin_passwordless_code/);
866+
await expect(signinPasswordlessCode.heading).toBeVisible();
867+
});
868+
});
869+
730870
test.describe('Passwordless authentication - Sync flows', () => {
731871
// Note: New Sync users are excluded from passwordless flow.
732872
// They go through traditional password-first signup.
@@ -738,17 +878,16 @@ test.describe('severity-2', () => {
738878
page,
739879
signin,
740880
signinPasswordlessCode,
741-
connectAnotherDevice,
742881
},
743882
testAccountTracker,
744883
}) => {
745884
// Create passwordless account via API first (no password)
746885
const { email } = await testAccountTracker.signUpPasswordless();
747886
const password = (testAccountTracker.accounts[0] as any).password;
748887

749-
// Navigate to Sync OAuth signin with passwordless enabled
888+
// Navigate to Sync OAuth signin — existing passwordless accounts
889+
// are redirected to OTP without needing force_passwordless flag
750890
const params = new URLSearchParams(syncDesktopOAuthQueryParams);
751-
params.set('force_passwordless', 'true');
752891
await signin.goto('/authorization', params);
753892
await signin.fillOutEmailFirstForm(email);
754893

@@ -770,15 +909,89 @@ test.describe('severity-2', () => {
770909
await page.getByLabel('Repeat password').fill(password);
771910
await page.getByRole('button', { name: 'Start syncing' }).click();
772911

773-
// Should show Sync connected page or pair page
912+
// Should show Sync confirmed page with success banner
913+
await expect(page).toHaveURL(/signup_confirmed_sync/);
774914
await expect(
775-
connectAnotherDevice.fxaConnected.or(
776-
page.getByText(/connected|syncing|pair/i)
777-
)
778-
).toBeVisible({
779-
timeout: 30000,
780-
});
915+
page.getByRole('heading', { name: 'Sync is turned on' })
916+
).toBeVisible();
917+
});
918+
919+
test('passwordless signin - Sync with TOTP and set password', async ({
920+
target,
921+
syncOAuthBrowserPages: {
922+
page,
923+
signin,
924+
signinPasswordlessCode,
925+
signinTotpCode,
926+
},
927+
testAccountTracker,
928+
}) => {
929+
// Flow: Sync login → passwordless OTP → TOTP → set password → sync enabled
930+
// TOTP must come before set_password because /password/create requires
931+
// a verifiedSessionToken.
932+
933+
// Create passwordless account and set up TOTP
934+
const { email, sessionToken } =
935+
await testAccountTracker.signUpPasswordless();
936+
const account: any = testAccountTracker.accounts.find(
937+
(a) => a.email === email
938+
);
939+
const password = account?.password || '';
940+
941+
const { secret } = await target.authClient.createTotpToken(
942+
sessionToken,
943+
{}
944+
);
945+
const totpCode = await getTotpCode(secret);
946+
await target.authClient.verifyTotpSetupCode(sessionToken, totpCode);
947+
await target.authClient.completeTotpSetup(sessionToken);
948+
949+
if (account) {
950+
account.secret = secret;
951+
account.sessionToken = sessionToken;
952+
}
953+
954+
await signin.clearCache();
955+
956+
// Navigate to Sync OAuth signin
957+
const params = new URLSearchParams(syncDesktopOAuthQueryParams);
958+
await signin.goto('/authorization', params);
959+
await signin.fillOutEmailFirstForm(email);
960+
961+
// Should redirect to passwordless code page
962+
await expect(page).toHaveURL(/signin_passwordless_code/);
963+
964+
const code = await target.emailClient.getPasswordlessSigninCode(email);
965+
await signinPasswordlessCode.fillOutCodeForm(code);
966+
967+
// TOTP verification comes before set_password
968+
await expect(page).toHaveURL(/signin_totp_code/);
969+
970+
const newTotpCode = await getTotpCode(secret);
971+
await signinTotpCode.fillOutCodeForm(newTotpCode);
972+
973+
// After TOTP, redirect to set password for Sync key derivation
974+
await expect(page).toHaveURL(/set_password/);
975+
await expect(
976+
page.getByRole('heading', { name: 'Create password to sync' })
977+
).toBeVisible();
978+
979+
// Complete password creation for Sync
980+
await page.getByLabel('Password', { exact: true }).fill(password);
981+
await page.getByLabel('Repeat password').fill(password);
982+
await page.getByRole('button', { name: 'Start syncing' }).click();
983+
984+
// Should show Sync confirmed page with success banner
985+
await expect(page).toHaveURL(/signup_confirmed_sync/);
986+
await expect(
987+
page.getByRole('heading', { name: 'Sync is turned on' })
988+
).toBeVisible();
989+
990+
// TODO: FXA-XXXX - Verify re-login uses password flow (not OTP) after
991+
// password creation. Requires fresh browser context for second Sync OAuth
992+
// flow since webchannel state from first login is stale.
781993
});
782994
});
783995
});
784996

997+

packages/fxa-auth-server/config/dev.json

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,5 +469,13 @@
469469
"enabled": true,
470470
"rpId": "localhost",
471471
"allowedOrigins": ["http://localhost:3030"]
472+
},
473+
"passwordlessOtp": {
474+
"enabled": true,
475+
"allowedClientIds": [
476+
"98e6508e88680e1a",
477+
"5882386c6d801776",
478+
"dcdb5ae7add825d2"
479+
]
472480
}
473481
}

packages/fxa-auth-server/lib/routes/account.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1632,17 +1632,23 @@ export class AccountHandler {
16321632
result.exists = true;
16331633
result.hasLinkedAccount = (account.linkedAccounts?.length || 0) > 0;
16341634
result.hasPassword = account.verifierSetAt > 0;
1635-
// Passwordless is supported if account is eligible AND clientId is allowed
1636-
result.passwordlessSupported =
1637-
isPasswordlessEligible(
1635+
// Existing passwordless accounts are always supported — the user must
1636+
// be able to sign in via OTP regardless of feature flag or RP allowlist.
1637+
// The flag + allowlist only gate *new* passwordless signups.
1638+
// Third-party auth accounts also have verifierSetAt === 0, but they
1639+
// should use their linked provider (Google/Apple), not passwordless OTP.
1640+
const hasLinkedAccount = (account.linkedAccounts?.length || 0) > 0;
1641+
const isExistingPasswordless = account.verifierSetAt === 0 && !hasLinkedAccount;
1642+
result.passwordlessSupported = isExistingPasswordless ||
1643+
(isPasswordlessEligible(
16381644
account,
16391645
email,
16401646
this.config.passwordlessOtp.enabled
16411647
) &&
16421648
isClientAllowedForPasswordless(
16431649
this.config.passwordlessOtp.allowedClientIds as string[],
16441650
clientId
1645-
);
1651+
));
16461652
} else {
16471653
const exist = await this.db.accountExists(email);
16481654
if (!exist) {

0 commit comments

Comments
 (0)