-
Notifications
You must be signed in to change notification settings - Fork 30
feat: Add Google One Tap #14335
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
feat: Add Google One Tap #14335
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
920cc9f
feat: Add Google One Tap proof of concept
AshCorr 93276de
feat: Use built-in FedCM API instead of Google GSI
AshCorr 04dbeba
chore: Only initialize FedCM if user is in test
AshCorr 39c4ffa
chore: Move Google One Tap to FrontPage component where the other scr…
AshCorr f53fd83
chore: Only load FedCM on supported browsers and improve documentation
AshCorr d249893
feat: Redirect to Gateway after FedCM auth and check for Consent befo…
AshCorr 2ba186e
chore: add missing useConsent hook
AshCorr c5b4146
chore: Update Google One Tap gateway urls
AshCorr 7733a82
chore: Add tests for Google One Tap behaviour
AshCorr 8a807ab
chore: also test isInTest and isSignedIn behaviour in google one tap
AshCorr 4c93744
docs: add todo for adding google one tap to other pages
AshCorr ec2df7d
fix: failing google one tap tests
AshCorr f449da6
chore: Move to server side tests for Google One Tap to avoid loading …
AshCorr 29e905b
Merge branch 'main' into ash/add-google-one-ta
AshCorr 8a50d3d
Merge branch 'main' into ash/add-google-one-ta
arelra File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
201 changes: 201 additions & 0 deletions
201
dotcom-rendering/src/components/GoogleOneTap.importable.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,201 @@ | ||
import { log } from '@guardian/libs'; | ||
import { useIsSignedIn } from '../lib/useAuthStatus'; | ||
import { useConsent } from '../lib/useConsent'; | ||
import { useOnce } from '../lib/useOnce'; | ||
import type { ServerSideTests, StageType } from '../types/config'; | ||
|
||
type IdentityProviderConfig = { | ||
configURL: string; | ||
clientId: string; | ||
}; | ||
|
||
type CredentialsProvider = { | ||
get: (options: { | ||
mediation: 'required'; | ||
identity: { | ||
context: 'continue'; | ||
providers: IdentityProviderConfig[]; | ||
}; | ||
}) => Promise<{ token: string }>; | ||
}; | ||
|
||
export const isInGoogleOneTapTest = (tests: ServerSideTests): boolean => | ||
tests['googleOneTapVariant'] === 'variant'; | ||
|
||
/** | ||
* Detect the current stage of the application based on the hostname. | ||
* | ||
* We do have a `window.guardian.config.stage` field, but it is based on the | ||
* environment of the article that DCR is rendering, which may not be the same as environment | ||
* DCR is running in. | ||
* | ||
* For example, running DCR locally and loading `https://r.thegulocal.com/Front/https://www.theguardian.com/international` will | ||
* give a stage of `PROD` as the article is in PROD, but DCR is running in `DEV`. | ||
*/ | ||
const getStage = (hostname: string): StageType => { | ||
if (window.location.hostname === 'm.code.dev-theguardian.com') { | ||
return 'CODE'; | ||
} else if (['r.thegulocal.com', 'localhost'].includes(hostname)) { | ||
return 'DEV'; | ||
} | ||
|
||
return 'PROD'; | ||
}; | ||
|
||
export const getRedirectUrl = ({ | ||
stage, | ||
token, | ||
currentLocation, | ||
}: { | ||
stage: StageType; | ||
token: string; | ||
currentLocation: string; | ||
}): string => { | ||
const profileDomain = { | ||
PROD: 'https://profile.theguardian.com', | ||
CODE: 'https://profile.code.dev-theguardian.com', | ||
DEV: 'https://profile.thegulocal.com', | ||
}[stage]; | ||
const queryParams = new URLSearchParams({ | ||
token, | ||
returnUrl: currentLocation, | ||
}); | ||
|
||
return `${profileDomain}/signin/google?${queryParams.toString()}`; | ||
}; | ||
|
||
// TODO: Do we want to use different Google Client IDs for One Tap than we use for social sign in? | ||
const getProviders = (stage: StageType): IdentityProviderConfig[] => { | ||
switch (stage) { | ||
case 'PROD': | ||
return [ | ||
{ | ||
configURL: 'https://accounts.google.com/gsi/fedcm.json', | ||
clientId: '774465807556.apps.googleusercontent.com', | ||
}, | ||
]; | ||
case 'CODE': | ||
case 'DEV': | ||
return [ | ||
{ | ||
configURL: 'https://accounts.google.com/gsi/fedcm.json', | ||
// TODO: m.code.dev-theguardian.com is not a supported origin for this Client ID | ||
clientId: | ||
'774465807556-pkevncqpfs9486ms0bo5q1f2g9vhpior.apps.googleusercontent.com', | ||
}, | ||
]; | ||
} | ||
}; | ||
|
||
export const initializeFedCM = async ({ | ||
isSignedIn, | ||
}: { | ||
isSignedIn?: boolean; | ||
isInTest?: boolean; | ||
}): Promise<void> => { | ||
if (isSignedIn) return; | ||
|
||
/** | ||
* Firefox does not support the FedCM API at the time of writting, | ||
* and it seems like it will not support it in the near future. | ||
* | ||
* Instead they're focusing on an alternative API called "Lightweight FedCM" | ||
* which may not support Google One Tap. | ||
* | ||
* See: https://bugzilla.mozilla.org/show_bug.cgi?id=1803629 | ||
*/ | ||
if (!('IdentityCredential' in window)) { | ||
// TODO: Track Ophan "FedCM" unsupported event here. | ||
log('identity', 'FedCM API not supported in this browser'); | ||
return; | ||
} | ||
|
||
const stage = getStage(window.location.hostname); | ||
|
||
/** | ||
* Typescripts built-in DOM types do not include the full `CredentialsProvider` | ||
* interface, so we need to cast `window.navigator.credentials` to our own | ||
* `CredentialsProvider` type which includes the FedCM API. | ||
* | ||
* Related issue: https://github.com/microsoft/TypeScript/issues/60641 | ||
*/ | ||
const credentialsProvider = window.navigator | ||
.credentials as unknown as CredentialsProvider; | ||
|
||
const credentials = await credentialsProvider | ||
.get({ | ||
/**. | ||
* Default `mediation` is "optional" which auto-authenticates the user if they have already interacted with FedCM | ||
* prompt on a previous page view. | ||
* | ||
* In practice this shouldn't happen as we won't trigger the prompt if the user is already signed in. But just in | ||
* case, we set `mediation` to "required" to ensure the user isn't put in a login loop where they are signed in, | ||
* FedCM auto-authenticates them, and they're sent to Gateway to get a new Okta token. | ||
*/ | ||
mediation: 'required', | ||
identity: { | ||
context: 'continue', | ||
providers: getProviders(stage), | ||
}, | ||
}) | ||
.catch((error) => { | ||
/** | ||
* The fedcm API hides issues with the user's federated login state | ||
* behind a generic NetworkError. This error is thrown up to 60 | ||
* seconds after the prompt is triggered to avoid timing attacks. | ||
* | ||
* This allows the browser to avoid leaking sensitive information | ||
* about the user's login state to the website. | ||
* | ||
* Unfortunately for us it means we can't differentiate between | ||
* a genuine network error and a user declining the FedCM prompt. | ||
*/ | ||
if (error instanceof Error && error.name === 'NetworkError') { | ||
log( | ||
'identity', | ||
'FedCM prompt failed, potentially due to user declining', | ||
); | ||
} else { | ||
throw error; | ||
} | ||
}); | ||
|
||
if (credentials) { | ||
// TODO: Track Ophan "FedCM" success event here. | ||
log('identity', 'FedCM credentials received', { | ||
credentials, | ||
}); | ||
|
||
window.location.replace( | ||
getRedirectUrl({ | ||
stage, | ||
token: credentials.token, | ||
currentLocation: window.location.href, | ||
}), | ||
); | ||
} else { | ||
// TODO: Track Ophan "FedCM" skip event here. | ||
log('identity', 'No FedCM credentials received'); | ||
} | ||
}; | ||
|
||
// TODO: GoogleOneTap is currently only used on the front page, but we do probably want to use it on other pages in the future. | ||
export const GoogleOneTap = () => { | ||
// We don't care what consent we get, we just want to make sure Google One Tap is not shown above the consent banner. | ||
// TODO: FedCM doesn't require cookies? Do we need to check consent? | ||
const consent = useConsent(); | ||
const isSignedIn = useIsSignedIn(); | ||
// useIsSignedIn returns 'Pending' until the auth status is known. | ||
// We don't want to initialize FedCM until we know the auth status, so we pass `undefined` to `useOnce` if it is 'Pending' | ||
// to stop it from initializing. | ||
const isSignedInWithoutPending = | ||
isSignedIn !== 'Pending' ? isSignedIn : undefined; | ||
|
||
useOnce(() => { | ||
void initializeFedCM({ | ||
isSignedIn: isSignedInWithoutPending, | ||
}); | ||
}, [isSignedInWithoutPending, consent]); | ||
|
||
return <></>; | ||
}; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies if I'm pointing out the obvious here but this is fine for the 0% test as we only want to initiate if the user is in the variant. However, if this does become a sampled test > 0% you'd want to initialise if the user is in the control or variant of the test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah okay, so would this mean that if we roll this out to users we'd basically have 3 scenarios?
A.
googleOneTapVariant === 'variant'
-> User is in test and we should load and initialize Google One TapB.
googleOneTapVariant === 'control'
-> User is in test and we should load but not initialize Google One TapC.
googleOneTapVariant === undefined
-> User isn't in test, don't load or hydrate the island at all?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good q, actually if I can revise my comment, I think it depends on what you want from your test.
If we want D&I to analyse any statistical difference between control and variant wrt core web vitals, click-through rates, bounce-rate etc, then we need to decide on the control and variant behaviour we want to compare.
If we want to compare the complete absence of GOT (i.e. not loading GOT at all including the island) vs loading and running GOT then the control shouldn't initialise GOT.
If we want to measure a closer simulation to what a production release might look like, where I assume the decision to initialise GOT is made client-side then the island will be included by default and the test would measure the difference between loading the island but not running GOT (control) vs running GOT.
Those could be variants of the same test or two separate tests depending on how far this work goes.
Just thought I would raise now in case we get to a sampled test.
For a 0% test the current setup is fine.