From 3826d6138e85850b6f38ca6dcad5f752ab85a46b Mon Sep 17 00:00:00 2001 From: Thor Galle Date: Thu, 26 Jan 2023 14:57:22 +0200 Subject: [PATCH] fix: improve verification handling between independent windows Should fix #298. Mitigates #297 somewhat, in the sense that this adds proactive one-time checks/state reloads when the user tries to access a resource that is verification-depedent client-side. No verification state streaming yet (only possible via Firestore), but this should already reduce the UX friction that exists today when verifying on a different device/window, especially in combination with the ReloadSuggestion component. --- src/lib/api/auth.ts | 26 ++++++++++-- src/lib/components/Garden/Form.svelte | 2 + src/lib/components/ReloadSuggestion.svelte | 35 ++++++++++++++++ src/lib/routes.js | 1 + src/locales/en.json | 12 ++++-- src/locales/fr.json | 12 ++++-- src/locales/nl.json | 12 ++++-- src/routes/+layout.svelte | 2 + src/routes/account/+page.svelte | 4 +- src/routes/auth/action/+page.svelte | 48 ++++++++++++++-------- src/routes/chat/+layout.svelte | 10 ++--- src/routes/garden/manage/+page.svelte | 16 ++++++-- 12 files changed, 140 insertions(+), 40 deletions(-) create mode 100644 src/lib/components/ReloadSuggestion.svelte diff --git a/src/lib/api/auth.ts b/src/lib/api/auth.ts index feb5bb62..7d45ce3f 100644 --- a/src/lib/api/auth.ts +++ b/src/lib/api/auth.ts @@ -52,6 +52,10 @@ export const createAuthObserver = (): Unsubscribe => { let routeTo: string | null = null; if (firebaseUser) { + if (oldStoredUser && oldStoredUser.uid !== firebaseUser.uid) { + console.info('The Firebase account was changed.'); + } + // If already logged in const firebaseUserEmailVerified: boolean = firebaseUser.emailVerified; // This is undocumented, but it gets set, and it is used to check whether the email was verified in @@ -97,10 +101,8 @@ export const createAuthObserver = (): Unsubscribe => { console.log('Email verification full sync success - storing new user'); isVerifying = false; newStoredUser = await storeNewUser(); - if (!isActiveContains(get(page), routes.ACCOUNT)) { - // - demo-test auth will already be on /account - // - staging/production auth will be on auth/action, - // if it has not already been redirected by the continueUrl redirector. + if (isActiveContains(get(page), routes.AUTH_ACTION)) { + // If we're not on a useful page, redirect to /account routeTo = routes.ACCOUNT; } } else { @@ -144,6 +146,22 @@ export const createAuthObserver = (): Unsubscribe => { }; }; +export const checkAndHandleUnverified = async (message?: string, timeout = 8000) => { + if (!get(user)?.emailVerified) { + // Migitates https://github.com/WelcometoMyGarden/welcometomygarden/issues/297 + await auth().currentUser?.reload(); + // Note: we check the currentUser.emailVerified prop here, because that is the only + // one that is atomically set after the .reload() + // It's enough signal to not show the warning, but for functionality, + // $user.emailVerified should still be checked before rendering content + // or taking actions that require full token verification. + if (!auth().currentUser?.emailVerified) { + notify.warning(message ?? get(_)('auth.verification.unverified'), timeout); + return goto(routes.ACCOUNT); + } + } +}; + export const isEmailVerifiedAndTokenSynced = async () => { const currentUser = auth().currentUser; const firebaseUserEmailVerified: boolean = currentUser?.emailVerified ?? false; diff --git a/src/lib/components/Garden/Form.svelte b/src/lib/components/Garden/Form.svelte index 01196d1f..9144a053 100644 --- a/src/lib/components/Garden/Form.svelte +++ b/src/lib/components/Garden/Form.svelte @@ -19,6 +19,7 @@ waterIcon, tentIcon } from '$lib/images/icons'; + import ReloadSuggestion from '../ReloadSuggestion.svelte'; const dispatch = createEventDispatcher(); @@ -172,6 +173,7 @@ )}` } })} +

{:else}

diff --git a/src/lib/components/ReloadSuggestion.svelte b/src/lib/components/ReloadSuggestion.svelte new file mode 100644 index 00000000..2a6628e7 --- /dev/null +++ b/src/lib/components/ReloadSuggestion.svelte @@ -0,0 +1,35 @@ + + +

+ {@html $_('account.verify.reload-suggestion', { + values: { + reloading: `${$_('account.verify.reloading-text')}` + } + })} +

+ + diff --git a/src/lib/routes.js b/src/lib/routes.js index b19bf784..eb49711b 100644 --- a/src/lib/routes.js +++ b/src/lib/routes.js @@ -3,6 +3,7 @@ export default { ADD_GARDEN: '/garden/add', ABOUT_SUPERFAN: '/about-superfan', ABOUT_US: '/about-us', + AUTH_ACTION: '/auth/action', BECOME_SUPERFAN: '/become-superfan', CHAT: '/chat', COOKIE_POLICY: '/terms/cookies', diff --git a/src/locales/en.json b/src/locales/en.json index e017669b..96d958d8 100644 --- a/src/locales/en.json +++ b/src/locales/en.json @@ -622,7 +622,8 @@ "title": "Add your garden" }, "manage": { - "title": "Manage garden" + "title": "Manage garden", + "add-first": "Please add a garden first." }, "form": { "title": "Add your garden to the map", @@ -954,7 +955,9 @@ "title": "Verify your email", "text": "You need to verify your email address if you want to chat or add a garden.", "button": "Resend email", - "sent": "Email sent!" + "sent": "Email sent!", + "reload-suggestion": "Did you already verify? Try {reloading} the page.", + "reloading-text": "reloading" }, "preferences": { "title": "Email preferences", @@ -993,12 +996,15 @@ "password": { "expired": "This password reset link has expired. Please request a new one" }, + "unsigned": "Please sign in first.", "verification": { "verifying": "Verifying your email address... Don't close this page, this could take a few seconds.", + "reloading": "Reloading your account...", "success": "Your email address was verified successfully!", "refresh": "Your email has already been verified.", "expired": "This verification link has expired. Please request a new one.", - "unsigned": "Please sign in first" + "sign-in": "Sign in to continue.", + "unverified": "Please verify your email first." }, "logged-out": "You have been logged out elsewhere. Please log in again." }, diff --git a/src/locales/fr.json b/src/locales/fr.json index cfe81e31..2a718a0f 100644 --- a/src/locales/fr.json +++ b/src/locales/fr.json @@ -677,7 +677,8 @@ "title": "Ajoutez votre jardin" }, "manage": { - "title": "Gérer le jardin" + "title": "Gérer le jardin", + "add-first": "Veuillez d'abord ajouter votre jardin" } }, "sign-in": { @@ -976,7 +977,9 @@ "title": "Confirmez votre adresse électronique", "sent": "Courriel envoyé !", "button": "Renvoyer le courriel", - "text": "Vous devez vérifier votre adresse électronique si vous voulez discuter ou ajouter un jardin." + "text": "Vous devez vérifier votre adresse électronique si vous voulez discuter ou ajouter un jardin.", + "reload-suggestion": "Avez vous déjà verifié? Essaye de {reloading} la page.", + "reloading-text": "recharger" }, "preferences": { "text": "M'envoyer des courriels quand :", @@ -987,12 +990,15 @@ "title": "Compte" }, "auth": { + "unsigned": "Veuillez vous inscrire d'abord.", "verification": { "verifying": "Vérification de votre email en cours... Merci de ne pas fermer cette page, cela peut prendre quelques secondes.", + "reloading": "Rechargement de votre compte en cours...", "refresh": "Votre adresse électronique a déjà été vérifié.", "success": "Votre adresse électronique a été vérifiée avec succès !", "expired": "Ce lien de vérification a expiré. Veuillez en demander un nouveau.", - "unsigned": "Veuillez vous inscrire d'abord" + "sign-in": "Veuillez vous inscrire pour continuer.", + "unverified": "Veuillez vérifier voter adresse électronique d'abord." }, "password": { "expired": "Ce lien de réinitialisation du mot de passe a expiré. Veuillez en demander un nouveau" diff --git a/src/locales/nl.json b/src/locales/nl.json index cc5653da..e3ecf466 100644 --- a/src/locales/nl.json +++ b/src/locales/nl.json @@ -185,7 +185,8 @@ "title": "Voeg jouw tuin toe" }, "manage": { - "title": "Beheer jouw tuin" + "title": "Beheer jouw tuin", + "add-first": "Voeg eerst jouw tuin toe." }, "form": { "title": "Voeg jouw tuin toe aan de kaart", @@ -517,7 +518,9 @@ "title": "Bevestig jouw e-mail", "text": "Je moet jouw e-mailadres controleren als je wilt chatten of een tuin wilt toevoegen.", "button": "E-mail opnieuw versturen", - "sent": "E-mail verzonden!" + "sent": "E-mail verzonden!", + "reload-suggestion": "Heb je al geverifieerd? Probeer de pagina te {reloading}.", + "reloading-text": "herladen" }, "preferences": { "title": "E-mail voorkeuren", @@ -556,12 +559,15 @@ "password": { "expired": "De link om jouw wachtwoord te herstellen is verlopen, probeer nog eens." }, + "unsigned": "Gelieve eerst in te loggen", "verification": { "verifying": "Je e-mailadres wordt geverifieerd... Gelieve deze pagina niet te sluiten, dit kan enkele seconden duren.", + "reloading": "Je account wordt herladen...", "success": "Jouw e-mailadres is met succes geverifieerd!", "refresh": "Jouw e-mail is al geverifieerd.", "expired": "Deze verificatielink is vervallen. Vraag een nieuwe aan.", - "unsigned": "Gelieve eerst in te loggen" + "sign-in": "Log in om verder te gaan.", + "unverified": "Gelieve eerst je email te verifiëren." } }, "register": { diff --git a/src/routes/+layout.svelte b/src/routes/+layout.svelte index 3be79373..e072b515 100644 --- a/src/routes/+layout.svelte +++ b/src/routes/+layout.svelte @@ -39,10 +39,12 @@ let vh = `0px`; + // Manage chat observers user.subscribe(async (tempUser) => { if (!unsubscribeFromChatObserver && tempUser && tempUser.emailVerified) unsubscribeFromChatObserver = await createChatObserver(tempUser.uid); + // Unsubscribe if the user logged out if (unsubscribeFromChatObserver && !tempUser) unsubscribeFromChatObserver(); if (tempUser && !tempUser.communicationLanguage && $locale) diff --git a/src/routes/account/+page.svelte b/src/routes/account/+page.svelte index fcb1d0b0..f17458fc 100644 --- a/src/routes/account/+page.svelte +++ b/src/routes/account/+page.svelte @@ -5,7 +5,7 @@ import { updateMailPreferences } from '@/lib/api/user'; import { resendAccountVerification } from '@/lib/api/auth'; import { changeListedStatus } from '$lib/api/garden'; - import { getUser, user } from '@/lib/stores/auth'; + import { user } from '@/lib/stores/auth'; import { updatingMailPreferences } from '$lib/stores/user'; import { Avatar, Icon, Button, LabeledCheckbox } from '$lib/components/UI'; import { flagIcon, emailIcon } from '$lib/images/icons'; @@ -13,6 +13,7 @@ import routes from '$lib/routes'; import { SUPPORT_EMAIL } from '$lib/constants'; import { createCustomerPortalSession } from '@/lib/api/functions'; + import ReloadSuggestion from '@/lib/components/ReloadSuggestion.svelte'; if (!$user) { goto(routes.SIGN_IN); @@ -121,6 +122,7 @@ {:else}

{$_('account.verify.sent')}

{/if} + {/if} diff --git a/src/routes/auth/action/+page.svelte b/src/routes/auth/action/+page.svelte index 8ede4a15..1216a9fb 100644 --- a/src/routes/auth/action/+page.svelte +++ b/src/routes/auth/action/+page.svelte @@ -2,12 +2,15 @@ // This page is set up according to the instructions here: // https://firebase.google.com/docs/auth/custom-email-handler#link_to_your_custom_handler_in_your_email_templates // - // It does not yet seem to be possible to set a custom handler URL for the local auth emulator: that one handles verifications internally with URLs of the form: + // It does NOT yet seem to be possible to set a custom handler URL for the local auth emulator: that one handles verifications internally with URLs of the form: // http://127.0.0.1:9099/emulator/action?mode=verifyEmail&lang=en&oobCode=EkpfDbT1x6s2Odi2hwAzJtW6dmoWX9BXjRWE96VWUrxXxgmbW7H-CW&apiKey=fake-api-key&continueUrl=http%3A%2F%2Flocalhost%3A5173%2Faccount // At the time of writing, someone submitted a PR for this though: https://github.com/firebase/firebase-tools/pull/5360 + // It IS possible to test this page locally by taking local auth email verification URLs of the form + // pasting the query string ?... bit of the above URL after http://localhost:5173/auth/action manually. // // In the demo, when the page /account continueUrl is loaded: - // The onIdTokenChanged in auth.ts will get notified that the email address was verified. + // The onIdTokenChanged in auth.ts will get notified that the email address was verified, but only when it is + // in a tab on the same window (https://github.com/WelcometoMyGarden/welcometomygarden/issues/297). // It will detect that email_verified is not yet true as a token claim, and will force-reset the token. // The force-reset triggers idTokenChanged everywhere. import { _ } from 'svelte-i18n'; @@ -35,7 +38,7 @@ // It must be on the same host as this page. // So far, this is not used, but this adds support for when we want to use it. const continueUrl = $page.url.searchParams.get('continueUrl'); - const continueUrlPathMatch = /https?:\/\/[^\/]+(\/.*$)/.exec(continueUrl ?? ''); + const continueUrlPathMatch = /https?:\/\/[^/]+(\/.*$)/.exec(continueUrl ?? ''); let gotoPath: string | null = null; if (continueUrlPathMatch) { gotoPath = continueUrlPathMatch[1]; @@ -68,28 +71,39 @@ try { // Try verifying the email console.log('Verification: applying oobCode'); - // TODO: localize loadingState = $_('auth.verification.verifying'); // applyActionCode DOES NOT trigger idTokenChanged await applyActionCode(oobCode); - // The .reload() call triggers an idTokenChanged event. - // We don't need to force-reload the token here, idTokenChanged - // will take care of that. console.log('Verification: Reloading user to trigger idTokenChanged'); - await auth().currentUser?.reload(); + if ($user) { + loadingState = $_('auth.verification.reloading'); + // TODO: an edge case we're not handling here: + // if you're logged in into a different account from the + // account that you are verifying. - // Success handling is done by idTokenChanged in auth.ts + // If we're logged in, reload the user. + // + // The .reload() call triggers an idTokenChanged event. + // We don't need to force-reload the token here, idTokenChanged + // will take care of that. + await auth().currentUser?.reload(); - // Note: it is not guaranteed here that the verification was fully synced to the token yet. - // In that case, we consider the account email as unverified locally. - // Thus, to prevent confusion with the "Resend Verification" button, - // we don't immediately redirect to /account here - // idTokenChanged will do the redirection. - if (gotoPath && gotoPath !== routes.ACCOUNT) { - console.log('Verification: Redirecting to continueUrl'); - goto(gotoPath); + // Success handling is done by idTokenChanged in auth.ts + + // Note: it is not guaranteed here that the verification was fully synced to the token yet. + // In that case, we consider the account email as unverified locally. + // Thus, to prevent confusion with the "Resend Verification" button, + // we don't immediately redirect to /account here + // idTokenChanged will do the redirection. + if (gotoPath && gotoPath !== routes.ACCOUNT) { + console.log('Verification: Redirecting to continueUrl'); + goto(gotoPath); + } + } else { + notify.success(`${$_('auth.verification.success')} ${$_('auth.verification.sign-in')}`); + goto(routes.SIGN_IN); } } catch (ex) { if ($user && (await isEmailVerifiedAndTokenSynced())) { diff --git a/src/routes/chat/+layout.svelte b/src/routes/chat/+layout.svelte index b7f652db..1ea8617f 100644 --- a/src/routes/chat/+layout.svelte +++ b/src/routes/chat/+layout.svelte @@ -13,6 +13,7 @@ import { Progress } from '$lib/components/UI'; import { removeDiacritics } from '$lib/util'; import { onMount } from 'svelte'; + import { checkAndHandleUnverified } from '@/lib/api/auth'; let localPage = $page; // Subscribe to page is necessary to render the chat page of the selected chat (when the url changes) for mobile @@ -43,13 +44,12 @@ let isMobile = false; $: outerWidth <= 700 ? (isMobile = true) : (isMobile = false); - onMount(() => { + onMount(async () => { if (!$user) { + notify.info($_('auth.unsigned'), 8000); return goto(routes.SIGN_IN); - } else if (!$user.emailVerified) { - notify.warning($_('chat.notify.unverified'), 10000); - return goto(routes.ACCOUNT); } + await checkAndHandleUnverified($_('chat.notify.unverified')); if (localPage.url.searchParams.get('with')) { startChattingWith(localPage.url.searchParams.get('with')); @@ -95,7 +95,7 @@ -{#if !localPage.url.searchParams.get('with') && $hasInitialized} +{#if !localPage.url.searchParams.get('with') && $hasInitialized && $user && $user.emailVerified}
{#if !isMobile || (isMobile && isOverview)}
diff --git a/src/routes/garden/manage/+page.svelte b/src/routes/garden/manage/+page.svelte index 653f19dd..b24e6008 100644 --- a/src/routes/garden/manage/+page.svelte +++ b/src/routes/garden/manage/+page.svelte @@ -8,10 +8,16 @@ import { updateGarden } from '$lib/api/garden'; import Form from '$lib/components/Garden/Form.svelte'; import routes from '$lib/routes'; + import { checkAndHandleUnverified } from '@/lib/api/auth'; - if (!$user || !$user.emailVerified) { - notify.warning('Please verify your email first.', 8000); - goto(routes.ACCOUNT); + if (!$user) { + notify.info($_('auth.unsigned'), 8000); + goto(routes.SIGN_IN); + } else if (!$user.emailVerified) { + checkAndHandleUnverified($_('auth.verification.unverified'), 8000); + } else if (!$user.garden) { + notify.warning($_('garden.manage.add-first')); + goto(routes.ADD_GARDEN); } let updatingGarden = false; @@ -45,4 +51,6 @@ -
+{#if $user && $user.garden} + +{/if}