Skip to content

Commit 86136e9

Browse files
committed
fix(settings): fix signup email redirection loop and add functional tests
Because: - Navigating directly to /signup?email=... caused a redirect loop - The useEffect in SignupContainer had no dependency array, re-running every render - After account creation, accountStatusByEmail returned exists: true and redirected to /signin instead of allowing navigation to /confirm_signup_code This commit: - Adds a useRef guard and empty dependency array in SignupContainer to ensure the email status check runs only once on mount - Passes service param to accountStatusByEmail for correct passwordless eligibility - Adds functional tests for password signup via Sync OAuth RP navigating directly to /signup?email=... and passwordless redirect behavior Fixes FXA-13309
1 parent eaae7f6 commit 86136e9

3 files changed

Lines changed: 79 additions & 9 deletions

File tree

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -994,6 +994,32 @@ test.describe('severity-2', () => {
994994
await page.waitForURL(/signin_passwordless_code/);
995995
await expect(signinPasswordlessCode.heading).toBeVisible();
996996
});
997+
998+
test('direct /signup URL with email param redirects to passwordless code when enabled', async ({
999+
target,
1000+
page,
1001+
pages: { settings, signinPasswordlessCode },
1002+
testAccountTracker,
1003+
}) => {
1004+
const { email } = testAccountTracker.generatePasswordlessAccountDetails();
1005+
1006+
// Navigate directly to /signup with email, without force_passwordless=false
1007+
// so the passwordlessEnabled config flag takes effect
1008+
await page.goto(
1009+
`${target.contentServerUrl}/signup?showReactApp=true&email=${encodeURIComponent(email)}&forceExperiment=generalizedReactApp&forceExperimentGroup=react`
1010+
);
1011+
1012+
// Should redirect to passwordless code page
1013+
await page.waitForURL(/signin_passwordless_code/);
1014+
await expect(signinPasswordlessCode.heading).toBeVisible();
1015+
1016+
const code = await target.emailClient.getPasswordlessSignupCode(email);
1017+
await signinPasswordlessCode.fillOutCodeForm(code);
1018+
1019+
await page.waitForURL(/settings/);
1020+
1021+
await settings.signOut();
1022+
});
9971023
});
9981024

9991025
test.describe('Passwordless authentication - Sync flows', () => {

packages/functional-tests/tests/react-conversion/signup.spec.ts

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@
44

55
import { FirefoxCommand, LinkAccountResponse } from '../../lib/channels';
66
import { expect, test } from '../../lib/fixtures/standard';
7-
import { syncDesktopV3QueryParams } from '../../lib/query-params';
7+
import {
8+
syncDesktopOAuthQueryParams,
9+
syncDesktopV3QueryParams,
10+
} from '../../lib/query-params';
811

912
const eventDetailLinkAccount: LinkAccountResponse = {
1013
id: 'account_updates',
@@ -41,6 +44,38 @@ test.describe('severity-1 #smoke', () => {
4144
await settings.signOut();
4245
});
4346

47+
test('signup with email query param navigates to confirm page', async ({
48+
target,
49+
syncOAuthBrowserPages: {
50+
confirmSignupCode,
51+
page,
52+
signup,
53+
signupConfirmedSync,
54+
},
55+
testAccountTracker,
56+
}) => {
57+
const { email, password } =
58+
testAccountTracker.generateSignupAccountDetails();
59+
60+
// Navigate directly to /signup with email in query params using Sync OAuth RP,
61+
// bypassing the email-first page (no emailStatusChecked flag).
62+
// Sync requires keys, so passwordless never applies.
63+
const params = new URLSearchParams(syncDesktopOAuthQueryParams);
64+
params.set('email', email);
65+
await signup.goto('/signup', params);
66+
67+
await signup.fillOutSyncSignupForm(password);
68+
69+
// Should navigate to confirm_signup_code, not redirect to signin
70+
await page.waitForURL(/confirm_signup_code/);
71+
72+
const code = await target.emailClient.getVerifyShortCode(email);
73+
await confirmSignupCode.fillOutCodeForm(code);
74+
75+
await page.waitForURL(/signup_confirmed_sync/);
76+
await expect(signupConfirmedSync.bannerConfirmed).toBeVisible();
77+
});
78+
4479
test('signup sync desktop v3, verify account', async ({
4580
target,
4681
syncBrowserPages: {

packages/fxa-settings/src/pages/Signup/container.tsx

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { Signup } from '.';
99
import { useValidatedQueryParams } from '../../lib/hooks/useValidate';
1010
import { SignupQueryParams } from '../../models/pages/signup';
1111
import { BeginSignupHandler, SignupIntegration } from './interfaces';
12-
import { useCallback, useEffect } from 'react';
12+
import { useCallback, useEffect, useRef } from 'react';
1313
import { handleAuthClientError } from './utils';
1414
import {
1515
getCredentials,
@@ -87,13 +87,21 @@ const SignupContainer = ({
8787

8888
const wantsKeys = integration.wantsKeys();
8989

90+
const attemptedEmailStatusCheck = useRef(false);
91+
9092
useEffect(() => {
9193
(async () => {
92-
if (!validationError && !emailStatusChecked) {
94+
if (
95+
!validationError &&
96+
!emailStatusChecked &&
97+
!attemptedEmailStatusCheck.current
98+
) {
99+
attemptedEmailStatusCheck.current = true;
93100
const { exists, hasLinkedAccount, hasPassword, passwordlessSupported } =
94101
await authClient.accountStatusByEmail(queryParamModel.email, {
95102
thirdPartyAuthStatus: true,
96103
clientId: integration.getClientId(),
104+
service: integration.getService(),
97105
});
98106
if (exists) {
99107
const signInPath = location.pathname.startsWith('/oauth')
@@ -120,7 +128,8 @@ const SignupContainer = ({
120128
}
121129
}
122130
})();
123-
});
131+
// eslint-disable-next-line react-hooks/exhaustive-deps
132+
}, []);
124133

125134
const beginSignupHandler: BeginSignupHandler = useCallback(
126135
async (email, password) => {
@@ -145,11 +154,11 @@ const SignupContainer = ({
145154
let credentialsV2 = undefined;
146155
let v2Payload:
147156
| {
148-
wrapKb: string;
149-
authPWVersion2: string;
150-
wrapKbVersion2: string;
151-
clientSalt: string;
152-
}
157+
wrapKb: string;
158+
authPWVersion2: string;
159+
wrapKbVersion2: string;
160+
clientSalt: string;
161+
}
153162
| {} = {};
154163

155164
if (keyStretchExp.queryParamModel.isV2(config)) {

0 commit comments

Comments
 (0)