Skip to content

Commit c9ed020

Browse files
vbudhramvbudhram
authored
Merge pull request #18911 from mozilla/fxa-11732
fix(auth): Correctly store MetricsContext in third party auth component
2 parents 9a18a4e + d755d83 commit c9ed020

13 files changed

Lines changed: 61 additions & 30 deletions

File tree

‎packages/fxa-settings/src/components/App/index.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -343,8 +343,8 @@ const AuthAndAccountSetupRoutes = ({
343343
return (
344344
<Router>
345345
{/* Index */}
346-
<IndexContainer path="/" {...{ integration, serviceName }} />
347-
<IndexContainer path="/oauth" {...{ integration, serviceName }} />
346+
<IndexContainer path="/" {...{ integration, serviceName, flowQueryParams }} />
347+
<IndexContainer path="/oauth" {...{ integration, serviceName, flowQueryParams }} />
348348

349349
{/* Legal */}
350350
<Legal path="/legal/*" />

‎packages/fxa-settings/src/components/ThirdPartyAuth/index.test.tsx‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,9 @@ describe('ThirdPartyAuthComponent', () => {
7575
it('renders', async () => {
7676
renderWith({
7777
enabled: true,
78+
flowQueryParams: {
79+
flowId: '123'
80+
}
7881
});
7982

8083
await screen.findByText('Continue with Google');
@@ -85,13 +88,13 @@ describe('ThirdPartyAuthComponent', () => {
8588
(await screen.findByTestId('google-signin-form-state')).getAttribute(
8689
'value'
8790
)
88-
).toEqual('http%3A%2F%2Flocalhost%2F%3F');
91+
).toEqual('http%3A%2F%2Flocalhost%2F%3FflowId%3D123');
8992

9093
expect(
9194
(await screen.findByTestId('apple-signin-form-state')).getAttribute(
9295
'value'
9396
)
94-
).toEqual('http%3A%2F%2Flocalhost%2F%3F');
97+
).toEqual('http%3A%2F%2Flocalhost%2F%3FflowId%3D123');
9598
});
9699

97100
it('submits apple form', async () => {

‎packages/fxa-settings/src/components/ThirdPartyAuth/index.tsx‎

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,15 @@ import { useConfig } from '../../models';
1212
import { ReactElement } from 'react-markdown/lib/react-markdown';
1313
import { useMetrics } from '../../lib/metrics';
1414
import GleanMetrics from '../../lib/glean';
15+
import { QueryParams } from '../..';
1516

1617
export type ThirdPartyAuthProps = {
1718
onContinueWithGoogle?: FormEventHandler<HTMLFormElement>;
1819
onContinueWithApple?: FormEventHandler<HTMLFormElement>;
1920
showSeparator?: boolean;
2021
viewName?: string;
2122
deeplink?: string;
23+
flowQueryParams?: QueryParams;
2224
};
2325

2426
/**
@@ -31,7 +33,8 @@ const ThirdPartyAuth = ({
3133
onContinueWithApple,
3234
showSeparator = true,
3335
viewName = 'unknown',
34-
deeplink,
36+
deeplink,
37+
flowQueryParams
3538
}: ThirdPartyAuthProps) => {
3639
const config = useConfig();
3740

@@ -63,7 +66,7 @@ const ThirdPartyAuth = ({
6366
{...{
6467
party: 'google',
6568
...config.googleAuthConfig,
66-
state: getState(),
69+
state: getState(flowQueryParams),
6770
scope: 'openid email profile',
6871
responseType: 'code',
6972
accessType: 'offline',
@@ -85,7 +88,7 @@ const ThirdPartyAuth = ({
8588
{...{
8689
party: 'apple',
8790
...config.appleAuthConfig,
88-
state: getState(),
91+
state: getState(flowQueryParams),
8992
scope: 'email',
9093
responseType: 'code id_token',
9194
accessType: 'offline',
@@ -129,7 +132,8 @@ const ThirdPartySignInForm = ({
129132
buttonText,
130133
onSubmit,
131134
viewName,
132-
deeplink
135+
deeplink,
136+
flowQueryParams
133137
}: {
134138
party: 'google' | 'apple';
135139
authorizationEndpoint: string;
@@ -145,6 +149,7 @@ const ThirdPartySignInForm = ({
145149
onSubmit?: FormEventHandler<HTMLFormElement>;
146150
viewName?: string;
147151
deeplink?: string;
152+
flowQueryParams?: QueryParams;
148153
}) => {
149154
const { logViewEventOnce } = useMetrics();
150155
const stateRef = useRef<HTMLInputElement>(null);
@@ -153,7 +158,6 @@ const ThirdPartySignInForm = ({
153158

154159
const onClick = useCallback(async () => {
155160
logViewEventOnce(`flow.${party}`, 'oauth-start');
156-
157161
switch (`${party}-${viewName}`) {
158162
case 'google-index':
159163
GleanMetrics.emailFirst.googleOauthStart();
@@ -185,9 +189,9 @@ const ThirdPartySignInForm = ({
185189
await GleanMetrics.isDone();
186190

187191
if (stateRef.current) {
188-
stateRef.current.value = getState();
192+
stateRef.current.value = getState(flowQueryParams);
189193
}
190-
}, [party, stateRef, viewName, logViewEventOnce]);
194+
}, [party, stateRef, viewName, logViewEventOnce, flowQueryParams]);
191195

192196

193197
if (onSubmit === undefined) {
@@ -242,23 +246,33 @@ function deleteParams(searchParams: URLSearchParams, paramsToDelete: string[]) {
242246
return searchParams;
243247
}
244248

245-
function getState() {
249+
function getState(flowQueryParams: QueryParams | undefined) {
246250
// We stash the originating location in the state oauth param
247251
// because we will need it to use it to reconstruct the redirect URL for RP
248252
const params = new URLSearchParams(window.location.search);
249-
// we won't need these params that are used for internal backbone/react navigation
250-
const modifiedParams = deleteParams(params, [
253+
254+
// Combine flowQueryParams and paramsObject, ensuring all values are strings
255+
const paramsObject = Object.fromEntries(params.entries());
256+
const combinedParams = {
257+
...paramsObject,
258+
...Object.fromEntries(
259+
Object.entries(flowQueryParams || {}).map(([key, value]) => [key, String(value)])
260+
),
261+
};
262+
263+
// Remove unwanted keys
264+
const filteredParams = deleteParams(new URLSearchParams(combinedParams), [
251265
'deeplink',
252266
'email',
253267
'emailStatusChecked',
254268
'forceExperiment',
255269
'forceExperimentGroup',
256270
'showReactApp',
257271
]);
272+
// we won't need these params that are used for internal backbone/react navigation
258273
return encodeURIComponent(
259-
`${window.location.origin}${
260-
window.location.pathname
261-
}?${modifiedParams.toString()}`
274+
`${window.location.origin}${window.location.pathname
275+
}?${filteredParams.toString()}`
262276
);
263277
}
264278

‎packages/fxa-settings/src/pages/Index/container.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import LoadingSpinner from 'fxa-react/components/LoadingSpinner';
3535
export const IndexContainer = ({
3636
integration,
3737
serviceName,
38+
flowQueryParams
3839
}: IndexContainerProps & RouteComponentProps) => {
3940
const authClient = useAuthClient();
4041
const ftlMsgResolver = useFtlMsgResolver();
@@ -268,7 +269,8 @@ export const IndexContainer = ({
268269
errorBannerMessage,
269270
successBannerMessage,
270271
tooltipErrorMessage,
271-
deeplink
272+
deeplink,
273+
flowQueryParams
272274
}}
273275
prefillEmail={initialPrefill}
274276
/>

‎packages/fxa-settings/src/pages/Index/index.tsx‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ export const Index = ({
3131
setErrorBannerMessage,
3232
setSuccessBannerMessage,
3333
setTooltipErrorMessage,
34-
deeplink
34+
deeplink,
35+
flowQueryParams
3536
}: IndexProps) => {
3637
const clientId = integration.getClientId();
3738
const isSync = integration.isSync();
@@ -78,7 +79,7 @@ export const Index = ({
7879

7980
if (isDeeplinking) {
8081
// To avoid flickering, we just render third party auth when deeplinking
81-
return <ThirdPartyAuth showSeparator={false} viewName="deeplink" deeplink={deeplink} />
82+
return <ThirdPartyAuth showSeparator={false} viewName="deeplink" deeplink={deeplink} flowQueryParams={flowQueryParams}/>
8283
}
8384

8485
return (
@@ -164,7 +165,7 @@ export const Index = ({
164165
</FtlMsg>
165166
</p>
166167
) : (
167-
!isDesktopRelay && <ThirdPartyAuth showSeparator viewName="index" />
168+
!isDesktopRelay && <ThirdPartyAuth showSeparator viewName="index" flowQueryParams={flowQueryParams}/>
168169
)}
169170
<TermsPrivacyAgreement
170171
{...{ isPocketClient, isMonitorClient, isDesktopRelay, isRelayClient }}

‎packages/fxa-settings/src/pages/Index/interfaces.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import { MozServices } from '../../lib/types';
66
import { Integration } from '../../models';
7+
import { QueryParams } from '../../index'
78

89
export type IndexIntegration = Pick<
910
Integration,
@@ -13,6 +14,7 @@ export type IndexIntegration = Pick<
1314
export interface IndexContainerProps {
1415
integration: IndexIntegration;
1516
serviceName: MozServices;
17+
flowQueryParams?: QueryParams;
1618
}
1719

1820
export interface LocationState {
@@ -32,6 +34,7 @@ export interface IndexProps extends LocationState {
3234
successBannerMessage?: string;
3335
tooltipErrorMessage?: string;
3436
deeplink?: string
37+
flowQueryParams?: QueryParams;
3538
}
3639

3740
export interface IndexFormData {

‎packages/fxa-settings/src/pages/Signin/container.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,8 @@ const SigninContainer = ({
495495
finishOAuthFlowHandler,
496496
localizedSuccessBannerHeading,
497497
localizedSuccessBannerDescription,
498-
deeplink
498+
deeplink,
499+
flowQueryParams
499500
}}
500501
/>
501502
);

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ const Signin = ({
6161
finishOAuthFlowHandler,
6262
localizedSuccessBannerHeading,
6363
localizedSuccessBannerDescription,
64-
deeplink
64+
deeplink,
65+
flowQueryParams
6566
}: SigninProps & RouteComponentProps) => {
6667
usePageViewEvent(viewName, REACT_ENTRYPOINT);
6768
const location = useLocation();
@@ -325,7 +326,7 @@ const Signin = ({
325326

326327
if (isDeeplinking) {
327328
// To avoid flickering, we only render third party auth and navigate
328-
return <ThirdPartyAuth showSeparator={false} viewName="deeplink" deeplink={deeplink} />
329+
return <ThirdPartyAuth showSeparator={false} viewName="deeplink" deeplink={deeplink} flowQueryParams={flowQueryParams}/>
329330
}
330331

331332
return (
@@ -445,7 +446,7 @@ const Signin = ({
445446
{!hideThirdPartyAuth && (
446447
<ThirdPartyAuth
447448
showSeparator={!hasLinkedAccountAndNoPassword}
448-
{...{ viewName }}
449+
{...{ viewName, flowQueryParams }}
449450
/>
450451
)}
451452

‎packages/fxa-settings/src/pages/Signin/interfaces.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { AccountAvatar } from '../../lib/interfaces';
1010
import { FinishOAuthFlowHandler } from '../../lib/oauth/hooks';
1111
import { MozServices } from '../../lib/types';
1212
import { Account, Integration } from '../../models';
13+
import { QueryParams } from '../..';
1314

1415
export interface AvatarResponse {
1516
account: {
@@ -88,6 +89,7 @@ export interface SigninProps {
8889
localizedSuccessBannerHeading?: string;
8990
localizedSuccessBannerDescription?: string;
9091
deeplink?: string;
92+
flowQueryParams?: QueryParams;
9193
}
9294

9395
export type BeginSigninHandler = (

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,8 @@ const SignupContainer = ({
221221
email,
222222
beginSignupHandler,
223223
useSyncEnginesResult,
224-
deeplink
224+
deeplink,
225+
flowQueryParams
225226
}}
226227
/>
227228
);

0 commit comments

Comments
 (0)