Skip to content

Commit 066cc05

Browse files
AshCorrAshCorr
committed
feat: Use POST instead of GET to log user in with Google One Tap
1 parent eb87859 commit 066cc05

File tree

2 files changed

+100
-66
lines changed

2 files changed

+100
-66
lines changed

‎dotcom-rendering/src/components/GoogleOneTap.importable.tsx

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,9 @@ const getStage = (hostname: string): StageType => {
4444

4545
export const getRedirectUrl = ({
4646
stage,
47-
token,
4847
currentLocation,
4948
}: {
5049
stage: StageType;
51-
token: string;
5250
currentLocation: string;
5351
}): string => {
5452
const profileDomain = {
@@ -57,11 +55,10 @@ export const getRedirectUrl = ({
5755
DEV: 'https://profile.thegulocal.com',
5856
}[stage];
5957
const queryParams = new URLSearchParams({
60-
token,
6158
returnUrl: currentLocation,
6259
});
6360

64-
return `${profileDomain}/signin/google?${queryParams.toString()}`;
61+
return `${profileDomain}/signin/google-one-tap?${queryParams.toString()}`;
6562
};
6663

6764
// TODO: Do we want to use different Google Client IDs for One Tap than we use for social sign in?
@@ -87,6 +84,32 @@ const getProviders = (stage: StageType): IdentityProviderConfig[] => {
8784
}
8885
};
8986

87+
/**
88+
* Submits the Google One Tap token to the Gateway.
89+
*
90+
* This is done via a form submission in order to redirect the user to Gateway
91+
* in a POST request. This way we avoid including the token in the URL,
92+
* which could be logged or cached and potentially expose
93+
* the user's Google One Tap token.
94+
*
95+
* @param {string} url - The URL to submit the form to.
96+
* @param {string} token - The Google One Tap token to submit.
97+
*/
98+
const submitGoogleOneTapToken = (url: string, token: string) => {
99+
const form = document.createElement('form');
100+
form.method = 'POST';
101+
form.action = url;
102+
103+
const tokenInput = document.createElement('input');
104+
tokenInput.type = 'hidden';
105+
tokenInput.name = 'token';
106+
tokenInput.value = token;
107+
form.appendChild(tokenInput);
108+
document.body.appendChild(form);
109+
110+
form.submit();
111+
};
112+
90113
export const initializeFedCM = async ({
91114
isSignedIn,
92115
}: {
@@ -166,12 +189,9 @@ export const initializeFedCM = async ({
166189
credentials,
167190
});
168191

169-
window.location.replace(
170-
getRedirectUrl({
171-
stage,
172-
token: credentials.token,
173-
currentLocation: window.location.href,
174-
}),
192+
submitGoogleOneTapToken(
193+
getRedirectUrl({ stage, currentLocation: window.location.href }),
194+
credentials.token,
175195
);
176196
} else {
177197
// TODO: Track Ophan "FedCM" skip event here.

‎dotcom-rendering/src/components/GoogleOneTap.test.tsx

Lines changed: 70 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,102 +1,124 @@
11
import { getRedirectUrl, initializeFedCM } from './GoogleOneTap.importable';
22

33
const mockWindow = ({
4-
replace,
54
get,
65
enableFedCM = true,
76
}: {
8-
replace: jest.Mock;
97
get: jest.Mock;
108
enableFedCM?: boolean;
11-
}) =>
12-
Object.defineProperty(globalThis, 'window', {
13-
value: {
14-
location: {
15-
href: 'https://www.theguardian.com/uk',
16-
hostname: 'm.code.theguardian.com',
17-
replace,
18-
},
19-
navigator: { credentials: { get } },
20-
...(enableFedCM ? { IdentityCredential: 'mock value' } : {}),
9+
}) => {
10+
const form = {
11+
submit: jest.fn(),
12+
appendChild: jest.fn(),
13+
method: undefined,
14+
action: undefined,
15+
};
16+
const input = {
17+
submit: jest.fn(),
18+
type: undefined,
19+
name: undefined,
20+
value: undefined,
21+
};
22+
23+
const window = {
24+
location: {
25+
href: 'https://www.theguardian.com/uk',
26+
hostname: 'm.code.theguardian.com',
27+
},
28+
navigator: { credentials: { get } },
29+
...(enableFedCM ? { IdentityCredential: 'mock value' } : {}),
30+
};
31+
32+
const document = {
33+
createElement: jest.fn((element) => {
34+
if (element === 'form') {
35+
return form;
36+
} else if (element === 'input') {
37+
return input;
38+
} else {
39+
throw new Error(`Unsupported element: ${element}`);
40+
}
41+
}),
42+
addEventListener: jest.fn(),
43+
body: {
44+
appendChild: jest.fn(),
2145
},
46+
};
47+
48+
Object.defineProperty(globalThis, 'window', {
49+
value: window,
50+
writable: true,
51+
});
52+
Object.defineProperty(globalThis, 'document', {
53+
value: document,
2254
writable: true,
2355
});
2456

57+
return { form, input };
58+
};
59+
2560
describe('GoogleOneTap', () => {
2661
it('should return the correct signin URL after constructing it with the provided stage and token', () => {
2762
expect(
2863
getRedirectUrl({
2964
stage: 'PROD',
30-
token: 'test-token',
3165
currentLocation: 'https://www.theguardian.com/uk',
3266
}),
3367
).toEqual(
34-
'https://profile.theguardian.com/signin/google?token=test-token&returnUrl=https%3A%2F%2Fwww.theguardian.com%2Fuk',
68+
'https://profile.theguardian.com/signin/google-one-tap?returnUrl=https%3A%2F%2Fwww.theguardian.com%2Fuk',
3569
);
3670

3771
expect(
3872
getRedirectUrl({
3973
stage: 'CODE',
40-
token: 'test-token',
4174
currentLocation: 'https://m.code.dev-theguardian.com/uk',
4275
}),
4376
).toEqual(
44-
'https://profile.code.dev-theguardian.com/signin/google?token=test-token&returnUrl=https%3A%2F%2Fm.code.dev-theguardian.com%2Fuk',
77+
'https://profile.code.dev-theguardian.com/signin/google-one-tap?returnUrl=https%3A%2F%2Fm.code.dev-theguardian.com%2Fuk',
4578
);
4679

4780
expect(
4881
getRedirectUrl({
4982
stage: 'DEV',
50-
token: 'test-token',
5183
currentLocation:
5284
'http://localhost/Front/https://m.code.dev-theguardian.com/uk',
5385
}),
5486
).toEqual(
55-
'https://profile.thegulocal.com/signin/google?token=test-token&returnUrl=http%3A%2F%2Flocalhost%2FFront%2Fhttps%3A%2F%2Fm.code.dev-theguardian.com%2Fuk',
87+
'https://profile.thegulocal.com/signin/google-one-tap?returnUrl=http%3A%2F%2Flocalhost%2FFront%2Fhttps%3A%2F%2Fm.code.dev-theguardian.com%2Fuk',
5688
);
5789
});
5890

5991
it('should initializeFedCM and redirect to Gateway with token on success', async () => {
6092
const navigatorGet = jest.fn(() =>
6193
Promise.resolve({ token: 'test-token' }),
6294
);
63-
const locationReplace = jest.fn();
6495

65-
mockWindow({
96+
const { form, input } = mockWindow({
6697
get: navigatorGet,
67-
replace: locationReplace,
6898
});
6999

70100
await initializeFedCM({ isSignedIn: false });
71101

72-
expect(navigatorGet).toHaveBeenCalledWith({
73-
identity: {
74-
context: 'continue',
75-
providers: [
76-
{
77-
clientId: '774465807556.apps.googleusercontent.com',
78-
configURL: 'https://accounts.google.com/gsi/fedcm.json',
79-
},
80-
],
81-
},
82-
mediation: 'required',
83-
});
84-
85-
expect(locationReplace).toHaveBeenCalledWith(
86-
'https://profile.theguardian.com/signin/google?token=test-token&returnUrl=https%3A%2F%2Fwww.theguardian.com%2Fuk',
102+
expect(form.action).toBe(
103+
'https://profile.theguardian.com/signin/google-one-tap?returnUrl=https%3A%2F%2Fwww.theguardian.com%2Fuk',
87104
);
105+
expect(form.method).toBe('POST');
106+
107+
expect(input.type).toBe('hidden');
108+
expect(input.name).toBe('token');
109+
expect(input.value).toBe('test-token');
110+
111+
expect(form.submit).toHaveBeenCalled();
88112
});
89113

90114
it('should initializeFedCM and not redirect to Gateway with token on failure', async () => {
91115
const error = new Error('Network Error');
92116
error.name = 'NetworkError';
93117

94118
const navigatorGet = jest.fn(() => Promise.reject(error));
95-
const locationReplace = jest.fn();
96119

97-
mockWindow({
120+
const { form } = mockWindow({
98121
get: navigatorGet,
99-
replace: locationReplace,
100122
});
101123

102124
await initializeFedCM({ isSignedIn: false });
@@ -115,19 +137,17 @@ describe('GoogleOneTap', () => {
115137
});
116138

117139
// Don't redirect whenever there is a "NetworkError" (aka user declined prompt)
118-
expect(locationReplace).not.toHaveBeenCalled();
140+
expect(form.submit).not.toHaveBeenCalled();
119141
});
120142

121143
it('should initializeFedCM and throw error when unexpected', async () => {
122144
const error = new Error('window.navigator.credentials.get failed');
123145
error.name = 'DOMException';
124146

125147
const navigatorGet = jest.fn(() => Promise.reject(error));
126-
const locationReplace = jest.fn();
127148

128-
mockWindow({
149+
const { form } = mockWindow({
129150
get: navigatorGet,
130-
replace: locationReplace,
131151
});
132152

133153
await expect(initializeFedCM({ isSignedIn: false })).rejects.toThrow(
@@ -148,52 +168,46 @@ describe('GoogleOneTap', () => {
148168
});
149169

150170
// Don't redirect whenever there is an unexpected error
151-
expect(locationReplace).not.toHaveBeenCalled();
171+
expect(form.submit).not.toHaveBeenCalled();
152172
});
153173

154174
it('should not initializeFedCM when FedCM is unsupported', async () => {
155175
const navigatorGet = jest.fn();
156-
const locationReplace = jest.fn();
157176

158-
mockWindow({
177+
const { form } = mockWindow({
159178
get: navigatorGet,
160-
replace: locationReplace,
161179
enableFedCM: false,
162180
});
163181

164182
await initializeFedCM({ isSignedIn: false });
165183

166184
expect(navigatorGet).not.toHaveBeenCalled();
167-
expect(locationReplace).not.toHaveBeenCalled();
185+
expect(form.submit).not.toHaveBeenCalled();
168186
});
169187

170188
it('should not initializeFedCM when user is signed in', async () => {
171189
const navigatorGet = jest.fn();
172-
const locationReplace = jest.fn();
173190

174-
mockWindow({
191+
const { form } = mockWindow({
175192
get: navigatorGet,
176-
replace: locationReplace,
177193
});
178194

179195
await initializeFedCM({ isSignedIn: true });
180196

181197
expect(navigatorGet).not.toHaveBeenCalled();
182-
expect(locationReplace).not.toHaveBeenCalled();
198+
expect(form.submit).not.toHaveBeenCalled();
183199
});
184200

185201
it('should not initializeFedCM when user is not in test', async () => {
186202
const navigatorGet = jest.fn();
187-
const locationReplace = jest.fn();
188203

189-
mockWindow({
204+
const { form } = mockWindow({
190205
get: navigatorGet,
191-
replace: locationReplace,
192206
});
193207

194208
await initializeFedCM({ isSignedIn: true });
195209

196210
expect(navigatorGet).not.toHaveBeenCalled();
197-
expect(locationReplace).not.toHaveBeenCalled();
211+
expect(form.submit).not.toHaveBeenCalled();
198212
});
199213
});

0 commit comments

Comments
 (0)