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.
WelcometoMyGarden · Jan 26, 2023 · 3826d61 · 3826d61
1 parent 84f5edc
commit 3826d61
Show file tree

Hide file tree

Showing 12 changed files with 140 additions and 40 deletions.
diff --git 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
@@ -19,6 +19,7 @@
     waterIcon,
     tentIcon
   } from '$lib/images/icons';
+  import ReloadSuggestion from '../ReloadSuggestion.svelte';
 
   const dispatch = createEventDispatcher();
 
@@ -172,6 +173,7 @@
                 )}</a>`
               }
             })}
+            <ReloadSuggestion />
           </p>
         {:else}
           <p>

diff --git a/src/lib/components/ReloadSuggestion.svelte b/src/lib/components/ReloadSuggestion.svelte
@@ -0,0 +1,35 @@
+<script>
+  import { _ } from 'svelte-i18n';
+  import { auth } from '../api/firebase';
+
+  // Migitates https://github.com/WelcometoMyGarden/welcometomygarden/issues/297
+  // NOTE: Might perform an unnecessary double reload when being redirected by
+  // checkAndHandleUnverified
+  auth().currentUser?.reload();
+</script>
+
+<p class="reload-suggestion">
+  {@html $_('account.verify.reload-suggestion', {
+    values: {
+      reloading: `<a onclick="window.location.reload()">${$_('account.verify.reloading-text')}</a>`
+    }
+  })}
+</p>
+
+<style>
+  .reload-suggestion {
+    margin-top: 1rem;
+    font-style: italic;
+  }
+  .reload-suggestion :global(a),
+  .reload-suggestion :global(a:link),
+  .reload-suggestion :global(a:visited),
+  .reload-suggestion :global(a:active) {
+    cursor: pointer;
+    color: var(--color-orange);
+    text-decoration: underline;
+  }
+  .reload-suggestion :global(a:hover) {
+    text-decoration: none;
+  }
+</style>
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -5,14 +5,15 @@
   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';
   import { countries } from '$lib/util';
   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}
               <p class="resend-verification">{$_('account.verify.sent')}</p>
             {/if}
+            <ReloadSuggestion />
           </div>
         </section>
       {/if}

diff --git 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
@@ -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 @@
 <svelte:window bind:outerWidth />
 <Progress active={!$hasInitialized || $creatingNewChat} />
 
-{#if !localPage.url.searchParams.get('with') && $hasInitialized}
+{#if !localPage.url.searchParams.get('with') && $hasInitialized && $user && $user.emailVerified}
   <div class="container">
     {#if !isMobile || (isMobile && isOverview)}
       <section class="conversations" in:fly={{ x: -outerWidth, duration: 400 }}>