Skip to content

Commit

Permalink
fix(oauth-desktop): Don't send up some options to fxaLogin when OAuth…
Browse files Browse the repository at this point in the history
… sync

Because:
* This is causing intermittent issues where desktop thinks it should fetch keys

This commit:
* Does not send keyFetchToken/unwrapBKey if isOAuth is true

fixes FXA-10617
  • Loading branch information
LZoog authored and vpomerleau committed Oct 24, 2024
1 parent c7f37c1 commit 7b70f04
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 15 deletions.
18 changes: 11 additions & 7 deletions packages/fxa-settings/src/pages/Signup/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -83,15 +83,19 @@ jest.mock('../../lib/glean', () => ({
},
}));

const commonFxaLoginOptions = {
const oauthCommonFxaLoginOptions = {
email: MOCK_EMAIL,
keyFetchToken: BEGIN_SIGNUP_HANDLER_RESPONSE.data.signUp.keyFetchToken,
sessionToken: BEGIN_SIGNUP_HANDLER_RESPONSE.data.signUp.sessionToken,
uid: BEGIN_SIGNUP_HANDLER_RESPONSE.data.signUp.uid,
unwrapBKey: BEGIN_SIGNUP_HANDLER_RESPONSE.data.unwrapBKey,
verified: false,
};

const commonFxaLoginOptions = {
...oauthCommonFxaLoginOptions,
keyFetchToken: BEGIN_SIGNUP_HANDLER_RESPONSE.data.signUp.keyFetchToken,
unwrapBKey: BEGIN_SIGNUP_HANDLER_RESPONSE.data.unwrapBKey,
};

describe('Signup page', () => {
beforeEach(() => {
jest.resetAllMocks();
Expand Down Expand Up @@ -636,7 +640,7 @@ describe('Signup page', () => {
});

expect(fxaLoginSpy).toBeCalledWith({
...commonFxaLoginOptions,
...oauthCommonFxaLoginOptions,
services: {
sync: {
declinedEngines: [],
Expand Down Expand Up @@ -667,7 +671,7 @@ describe('Signup page', () => {
});

expect(fxaLoginSpy).toBeCalledWith({
...commonFxaLoginOptions,
...oauthCommonFxaLoginOptions,
services: {
sync: {
declinedEngines: ['prefs', 'bookmarks'],
Expand Down Expand Up @@ -711,7 +715,7 @@ describe('Signup page', () => {
});

expect(fxaLoginSpy).toBeCalledWith({
...commonFxaLoginOptions,
...oauthCommonFxaLoginOptions,
services: {
sync: {
declinedEngines: offeredEngines,
Expand Down Expand Up @@ -787,7 +791,7 @@ describe('Signup page', () => {
await waitFor(() => {
// Does not send services: { sync: {...} }
expect(fxaLoginSpy).toBeCalledWith({
...commonFxaLoginOptions,
...oauthCommonFxaLoginOptions,
});
});

Expand Down
19 changes: 11 additions & 8 deletions packages/fxa-settings/src/pages/Signup/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export const Signup = ({
GleanMetrics.registration.view();
}, []);

const isOAuth = isOAuthIntegration(integration);
const isSyncOAuth = isOAuthNativeIntegrationSync(integration);
const isSyncDesktopV3 = isSyncDesktopV3Integration(integration);
const isSync = integration.isSync();
Expand Down Expand Up @@ -95,7 +96,7 @@ export const Signup = ({
const [hasAgeInputFocused, setHasAgeInputFocused] = useState<boolean>(false);

useEffect(() => {
if (isOAuthIntegration(integration)) {
if (isOAuth) {
const clientId = integration.getClientId();
if (isClientPocket(clientId)) {
setClient(MozServices.Pocket);
Expand All @@ -105,7 +106,7 @@ export const Signup = ({
setClient(MozServices.Monitor);
}
}
}, [integration]);
}, [integration, isOAuth]);

const [offeredSyncEngineConfigs, setOfferedSyncEngineConfigs] = useState<
typeof syncEngineConfigs | undefined
Expand Down Expand Up @@ -261,9 +262,13 @@ export const Signup = ({
GleanMetrics.registration.cwts({ sync: { cwts: syncOptions } });
firefox.fxaLogin({
email,
// keyFetchToken and unwrapBKey should always exist if Sync integration
keyFetchToken: data.signUp.keyFetchToken!,
unwrapBKey: data.unwrapBKey!,
// Do not send these values if OAuth. Mobile doesn't care about this message, and
// sending these values can cause intermittent sync disconnect issues in oauth desktop.
...(!isOAuth && {
// keyFetchToken and unwrapBKey should always exist if Sync integration
keyFetchToken: data.signUp.keyFetchToken!,
unwrapBKey: data.unwrapBKey!,
}),
sessionToken: data.signUp.sessionToken,
uid: data.signUp.uid,
verified: false,
Expand All @@ -274,9 +279,6 @@ export const Signup = ({
} else if (isDesktopRelay) {
firefox.fxaLogin({
email,
// keyFetchToken and unwrapBKey should always exist if integration wants keys
keyFetchToken: data.signUp.keyFetchToken!,
unwrapBKey: data.unwrapBKey!,
sessionToken: data.signUp.sessionToken,
uid: data.signUp.uid,
verified: false,
Expand Down Expand Up @@ -333,6 +335,7 @@ export const Signup = ({
isSyncOAuth,
localizedValidAgeError,
isDesktopRelay,
isOAuth,
]
);

Expand Down

0 comments on commit 7b70f04

Please sign in to comment.