Skip to content

Commit 6e66667

Browse files
committed
fix(signin): Render cached signin for passwordless users in Sync flow
Because: * We recently made some changes to the Signin page to hide the third party auth buttons in favor of showing a cached sign in view for passwordless users signing into Sync. There is a bug where we render the password input for passwordless users trying to sign into Sync This commit: * Updates the isPasswordNeeded logic to always exclude users that do not have a password * Navigates these users to set password instead and does not send web channel messages until password creation is complete closes FXA-13251
1 parent b343151 commit 6e66667

4 files changed

Lines changed: 101 additions & 13 deletions

File tree

packages/fxa-settings/src/pages/PostVerify/SetPassword/container.tsx

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import { RouteComponentProps, useLocation } from '@reach/router';
66
import SetPassword from '.';
77
import { currentAccount } from '../../../lib/cache';
8+
import { persistAccount } from '../../../lib/storage-utils';
89
import { useNavigateWithQuery } from '../../../lib/hooks/useNavigateWithQuery';
910
import { Integration, useAuthClient } from '../../../models';
1011
import { useCallback, useEffect, useRef, useState } from 'react';
@@ -45,7 +46,6 @@ const SetPasswordContainer = ({
4546
const sessionToken = storedLocalAccount?.sessionToken;
4647
const uid = storedLocalAccount?.uid;
4748

48-
4949
const location = useLocation() as ReturnType<typeof useLocation> & {
5050
state?: SetPasswordLocationState;
5151
};
@@ -122,14 +122,18 @@ const SetPasswordContainer = ({
122122
(uid: string, email: string, sessionToken: string): CreatePasswordHandler =>
123123
async (newPassword: string) => {
124124
try {
125-
const { authPW, unwrapBKey } =
126-
await authClient.createPassword(sessionToken, email, newPassword);
125+
const { authPW, unwrapBKey } = await authClient.createPassword(
126+
sessionToken,
127+
email,
128+
newPassword
129+
);
127130

128131
const keyFetchToken = await getKeyFetchToken(
129132
authPW,
130133
email,
131134
sessionToken
132135
);
136+
persistAccount({ uid, hasPassword: true });
133137

134138
GleanMetrics.thirdPartyAuthSetPassword.success();
135139

packages/fxa-settings/src/pages/Signin/index.stories.tsx

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,17 @@ export const CachedSyncBrowserService = storyWithProps(
130130
sessionToken: MOCK_SESSION_TOKEN,
131131
integration: createMockSigninOAuthNativeSyncIntegration(),
132132
},
133-
'Cached > Sync browser service'
133+
'Cached > Sync browser service > Account has password'
134+
);
135+
export const CachedSyncBrowserServicePasswordlessAccount = storyWithProps(
136+
{
137+
sessionToken: MOCK_SESSION_TOKEN,
138+
serviceName: MozServices.FirefoxSync,
139+
hasLinkedAccount: true,
140+
hasPassword: false,
141+
integration: createMockSigninOAuthNativeSyncIntegration(),
142+
},
143+
'Cached > Sync browser service > Passwordless account (user will be taken to Set Password page)'
134144
);
135145
export const CachedNonSyncBrowserServiceWithoutPasswordlessCapability =
136146
storyWithProps(

packages/fxa-settings/src/pages/Signin/index.test.tsx

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1136,12 +1136,55 @@ describe('Signin component', () => {
11361136
hardNavigateSpy.mockRestore();
11371137
});
11381138

1139-
it('always renders password input when integration wants keys', () => {
1139+
it('renders password input when integration wants keys and user has a password', () => {
11401140
const integration = createMockSigninOAuthNativeSyncIntegration();
11411141
render({ integration, sessionToken: MOCK_SESSION_TOKEN });
11421142
passwordInputRendered();
11431143
});
11441144

1145+
it('does not render password input when integration wants keys but user has no password', () => {
1146+
const integration = createMockSigninOAuthNativeSyncIntegration();
1147+
render({
1148+
integration,
1149+
sessionToken: MOCK_SESSION_TOKEN,
1150+
hasPassword: false,
1151+
hasLinkedAccount: true,
1152+
});
1153+
passwordInputNotRendered();
1154+
// Should show cached sign-in button instead
1155+
screen.getByRole('button', { name: 'Sign in' });
1156+
});
1157+
1158+
it('passes isSignInWithThirdPartyAuth for cached Sync passwordless user', async () => {
1159+
const handleNavigationSpy = jest.spyOn(
1160+
SigninUtils,
1161+
'handleNavigation'
1162+
);
1163+
const cachedSigninHandler = jest
1164+
.fn()
1165+
.mockReturnValueOnce(CACHED_SIGNIN_HANDLER_RESPONSE);
1166+
const integration = createMockSigninOAuthNativeSyncIntegration();
1167+
render({
1168+
integration,
1169+
sessionToken: MOCK_SESSION_TOKEN,
1170+
hasPassword: false,
1171+
hasLinkedAccount: true,
1172+
cachedSigninHandler,
1173+
});
1174+
1175+
await submit();
1176+
await waitFor(() => {
1177+
expect(handleNavigationSpy).toHaveBeenCalledWith(
1178+
expect.objectContaining({
1179+
isSignInWithThirdPartyAuth: true,
1180+
handleFxaLogin: false,
1181+
handleFxaOAuthLogin: false,
1182+
})
1183+
);
1184+
});
1185+
handleNavigationSpy.mockRestore();
1186+
});
1187+
11451188
it('navigates to OAuth redirect', async () => {
11461189
const cachedSigninHandler = jest
11471190
.fn()
@@ -1402,6 +1445,28 @@ describe('Signin component', () => {
14021445
});
14031446
});
14041447

1448+
it('shows third party auth instead of password when session expires for passwordless user', async () => {
1449+
const cachedSigninHandler = jest
1450+
.fn()
1451+
.mockReturnValueOnce(createCachedSigninResponseError());
1452+
renderWithLocalizationProvider(
1453+
<Subject
1454+
sessionToken={MOCK_SESSION_TOKEN}
1455+
hasPassword={false}
1456+
hasLinkedAccount={true}
1457+
{...{ cachedSigninHandler }}
1458+
/>
1459+
);
1460+
1461+
await submit();
1462+
await waitFor(() => {
1463+
expect(cachedSigninHandler).toHaveBeenCalledWith(MOCK_SESSION_TOKEN);
1464+
screen.getByText('Session expired. Sign in to continue.');
1465+
passwordInputNotRendered();
1466+
thirdPartyAuthRendered();
1467+
});
1468+
});
1469+
14051470
it('displays other errors', async () => {
14061471
const unexpectedError = AuthUiErrors.UNEXPECTED_ERROR;
14071472
const cachedSigninHandler = jest.fn().mockReturnValueOnce(

packages/fxa-settings/src/pages/Signin/index.tsx

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ const Signin = ({
8484

8585
const isOAuth = isOAuthIntegration(integration);
8686
const isOAuthNative = isOAuthNativeIntegration(integration);
87+
const isSync = integration.isSync();
8788
const clientId = integration.getClientId();
8889
const hasLinkedAccountAndNoPassword = hasLinkedAccount && !hasPassword;
8990

@@ -95,17 +96,22 @@ const Signin = ({
9596
const [hasCachedAccount, setHasCachedAccount] =
9697
useState<boolean>(!!sessionToken);
9798

98-
// A password is needed if:
99-
// * There is no session token in local storage and the user has a password, OR
99+
// A password is needed and password input rendered if the user has a password AND:
100+
// * There is no session token in local storage, OR
100101
// * The integration wants keys (e.g. Sync always wants keys, and non-sync browser
101102
// services like Smart Window also request keys if there's no cached sign-in
102103
// to prevent a redirect to FxA to turn Sync on), OR
103104
// * The integration is OAuth and wants login (prompt=login)
104105
// UNLESS the user has a cached account AND they are in an OAuth Native flow AND
105106
// the browser supports the "keys optional" capability for non-Sync browser signins.
106107
// These users will be redirected to FxA later to enter a password to turn Sync on.
108+
//
109+
// If the user has no password (e.g. third-party auth only), we show the cached
110+
// sign-in view instead. After session verification, Sync users will be redirected
111+
// to set a password via post_verify/third_party_auth/set_password.
107112
const isPasswordNeeded =
108-
((!hasCachedAccount && hasPassword) ||
113+
hasPassword &&
114+
(!hasCachedAccount ||
109115
integration.wantsKeys() ||
110116
(isOAuth && integration.wantsLogin())) &&
111117
!(hasCachedAccount && supportsKeysOptionalLogin);
@@ -136,9 +142,7 @@ const Signin = ({
136142
// Show for all other cases.
137143
const hideThirdPartyAuth =
138144
hasCachedAccount ||
139-
(integration.isSync()
140-
? hasPassword
141-
: isOAuthNative && !supportsKeysOptionalLogin);
145+
(isSync ? hasPassword : isOAuthNative && !supportsKeysOptionalLogin);
142146

143147
useEffect(() => {
144148
if (!isPasswordNeeded) {
@@ -181,8 +185,12 @@ const Signin = ({
181185
queryParams: location.search,
182186
performNavigation: !integration.isFirefoxMobileClient(),
183187
isServiceWithEmailVerification,
184-
handleFxaLogin: true,
185-
handleFxaOAuthLogin: true,
188+
// Sync users in the cached path are passwordless (third-party auth);
189+
// defer web channel messages until after password creation.
190+
handleFxaLogin: !isSync,
191+
handleFxaOAuthLogin: !isSync,
192+
// Redirect passwordless Sync users to set_password after session verification.
193+
isSignInWithThirdPartyAuth: isSync,
186194
};
187195

188196
const { error: navError } = await handleNavigation(navigationOptions);
@@ -209,6 +217,7 @@ const Signin = ({
209217
setLocalizedBannerError,
210218
integration,
211219
finishOAuthFlowHandler,
220+
isSync,
212221
location.search,
213222
webRedirectCheck,
214223
isServiceWithEmailVerification,

0 commit comments

Comments
 (0)