-
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 12 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
204 changes: 204 additions & 0 deletions
204
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,204 @@ | ||
import { log } from '@guardian/libs'; | ||
import { useAB } from '../lib/useAB'; | ||
import { useIsSignedIn } from '../lib/useAuthStatus'; | ||
import { useConsent } from '../lib/useConsent'; | ||
import { useOnce } from '../lib/useOnce'; | ||
import type { StageType } from '../types/config'; | ||
|
||
type IdentityProviderConfig = { | ||
configURL: string; | ||
clientId: string; | ||
}; | ||
|
||
type CredentialsProvider = { | ||
get: (options: { | ||
mediation: 'required'; | ||
identity: { | ||
context: 'continue'; | ||
providers: IdentityProviderConfig[]; | ||
}; | ||
}) => Promise<{ token: string }>; | ||
}; | ||
|
||
/** | ||
* 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, | ||
isInTest, | ||
}: { | ||
isSignedIn?: boolean; | ||
isInTest?: boolean; | ||
}): Promise<void> => { | ||
// Only initialize Google One Tap if the user is in the AB test. Currently 0% of users are in the test. | ||
if (!isInTest) return; | ||
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; | ||
const isInTest = useAB()?.api.isUserInVariant('GoogleOneTap', 'variant'); | ||
|
||
useOnce(() => { | ||
void initializeFedCM({ | ||
isSignedIn: isSignedInWithoutPending, | ||
isInTest, | ||
}); | ||
}, [isSignedInWithoutPending, isInTest, 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.
I assume this is a server side test, in which case could we wrap the island with a check to see if the test is active? It would mean the island wouldn't get async imported or hydrated unless actually required.
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 good point! At the moment its a client side test, but I could make it a server side one. I'd have to use one of the limited 0% experiment slots though, is that alright?
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.
Thats fine there's still a couple available. First come, first served!
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.
Great, I've switched it to a served side experiment in guardian/frontend#28160 and pushed a commit in this PR to use it. Thanks for the suggestion!