Improve pollForExpiredAuth and reauthenticate UX

web/app/components/notification.hbs

@@ -2,6 +2,7 @@
   {{#each this.flashMessages.queue as |flash|}}
     <FlashMessage
       data-test-flash-notification
+      data-test-flash-notification-type={{flash.type}}
       @flash={{flash}}
       class="notification"
       as |component flash close|

web/app/controllers/authenticate.ts

@@ -1,7 +1,7 @@
 import Controller from "@ember/controller";
 import { inject as service } from "@ember/service";
-import { action } from "@ember/object";
 import SessionService from "hermes/services/session";
+import { dropTask } from "ember-concurrency";
 
 export default class AuthenticateController extends Controller {
   @service declare session: SessionService;
@@ -10,11 +10,13 @@ export default class AuthenticateController extends Controller {
     return new Date().getFullYear();
   }
 
-  @action protected authenticate(): void {
-    this.session.authenticate("authenticator:torii", "google-oauth2-bearer");
-  }
+  protected authenticate = dropTask(async () => {
+    await this.session.authenticate(
+      "authenticator:torii",
+      "google-oauth2-bearer"
+    );
+  });
 }
-
 declare module "@ember/controller" {
   interface Registry {
     authenticate: AuthenticateController;

web/app/services/authenticated-user.ts

@@ -62,9 +62,9 @@ export default class AuthenticatedUserService extends Service {
 
   /**
    * Loads the user's info from the Google API.
-   * Called by the `authenticated` route on load.
-   * Ensures `authenticatedUser.info` is always defined.
-   * On error, will bubble up to the application route.
+   * Called by `session.handleAuthentication` and `authenticated.afterModel`.
+   * Ensures `authenticatedUser.info` is always defined and up-to-date
+   * in any route that needs it. On error, bubbles up to the application route.
    */
   loadInfo = task(async () => {
     try {

web/app/services/session.ts

@@ -2,13 +2,15 @@ import { inject as service } from "@ember/service";
 import RouterService from "@ember/routing/router-service";
 import EmberSimpleAuthSessionService from "ember-simple-auth/services/session";
 import window from "ember-window-mock";
-import { keepLatestTask } from "ember-concurrency";
+import { dropTask, keepLatestTask, timeout } from "ember-concurrency";
 import FlashMessageService from "ember-cli-flash/services/flash-messages";
 import Ember from "ember";
 import { tracked } from "@glimmer/tracking";
 import simpleTimeout from "hermes/utils/simple-timeout";
 import ConfigService from "hermes/services/config";
 import FetchService from "./fetch";
+import AuthenticatedUserService from "./authenticated-user";
+import { capitalize } from "@ember/string";
 
 export const REDIRECT_STORAGE_KEY = "hermes.redirectTarget";
 
@@ -27,6 +29,7 @@ export default class SessionService extends EmberSimpleAuthSessionService {
   @service declare fetch: FetchService;
   @service declare session: SessionService;
   @service declare flashMessages: FlashMessageService;
+  @service declare authenticatedUser: AuthenticatedUserService;
 
   /**
    * Whether the current session is valid.
@@ -35,6 +38,14 @@ export default class SessionService extends EmberSimpleAuthSessionService {
    */
   @tracked tokenIsValid = true;
 
+  /**
+   * Whether the reauthentication message is shown.
+   * Dictates if we poll the back end for a 401.
+   * Set true when the user's session has expired.
+   * Set false when the user successfully reauthenticates.
+   */
+  @tracked reauthenticationMessageIsShown = false;
+
   /**
    * Whether the service should show a reauthentication message.
    * True when the user has dismissed a previous re-auth message.
@@ -55,16 +66,18 @@ export default class SessionService extends EmberSimpleAuthSessionService {
   pollForExpiredAuth = keepLatestTask(async () => {
     await simpleTimeout(Ember.testing ? 100 : 10000);
 
-    this.fetch.fetch(
-      "/api/v1/me",
-      {
-        method: "HEAD",
-      },
-      true
-    );
+    // If the reauth message is shown, do nothing but restart the counter.
+    if (this.reauthenticationMessageIsShown) {
+      this.pollForExpiredAuth.perform();
+      return;
+    }
+
+    // Make a HEAD request to the back end.
+    // On 401, the fetch service will set `this.pollResponseIs401` true.
+    await this.fetch.fetch("/api/v1/me", { method: "HEAD" }, true);
 
     if (!this.configSvc.config.skip_google_auth) {
-      let isLoggedIn = await this.requireAuthentication(null, () => {});
+      let isLoggedIn = this.requireAuthentication(null, () => {});
 
       if (this.pollResponseIs401 || !isLoggedIn) {
         this.tokenIsValid = false;
@@ -76,54 +89,113 @@ export default class SessionService extends EmberSimpleAuthSessionService {
     if (this.tokenIsValid) {
       this.preventReauthenticationMessage = false;
     } else if (!this.preventReauthenticationMessage) {
-      if (!this.configSvc.config.skip_google_auth) {
-        this.flashMessages.add({
-          title: "Login token expired",
-          message: "Please reauthenticate to keep using Hermes.",
-          type: "warning",
-          sticky: true,
-          destroyOnClick: false,
-          preventDuplicates: true,
-          buttonText: "Authenticate with Google",
-          buttonIcon: "google",
-          buttonAction: () => {
-            this.authenticate("authenticator:torii", "google-oauth2-bearer");
-            this.flashMessages.clearMessages();
-          },
-          onDestroy: () => {
-            this.preventReauthenticationMessage = true;
-          },
-        });
-      } else {
-        this.flashMessages.add({
-          title: "Session expired",
-          message: "Please reauthenticate to keep using Hermes.",
-          type: "warning",
-          sticky: true,
-          destroyOnClick: false,
-          preventDuplicates: true,
-          buttonText: "Authenticate with Okta",
-          buttonIcon: "okta",
-          buttonAction: () => {
-            // Reload to redirect to Okta login.
-            window.location.reload();
-            this.flashMessages.clearMessages();
-          },
-          onDestroy: () => {
-            this.preventReauthenticationMessage = true;
-          },
-        });
-      }
+      this.showReauthMessage(
+        "Session expired",
+        "Please reauthenticate to keep using Hermes.",
+        "warning",
+        () => {
+          this.preventReauthenticationMessage = true;
+          this.reauthenticationMessageIsShown = false;
+        }
+      );
+
+      // Set this true to prevent additional HEAD requests.
+      // On successful reauth, this will be reset false.
+      this.reauthenticationMessageIsShown = true;
     }
 
+    // Restart this very task.
     this.pollForExpiredAuth.perform();
   });
 
+  /**
+   * Triggers a flash message with a button to reauthenticate.
+   * Used when the user's session has expired, or when the user
+   * unsuccessfully attempts to reauthenticate.
+   * Functions in accordance with the `skip_google_auth` config.
+   */
+  private showReauthMessage(
+    title: string,
+    message: string,
+    type: "warning" | "critical",
+    onDestroy?: () => void
+  ) {
+    const buttonIcon = this.configSvc.config.skip_google_auth
+      ? "okta"
+      : "google";
+
+    const buttonText = `Authenticate with ${capitalize(buttonIcon)}`;
+
+    this.flashMessages.add({
+      title,
+      message,
+      type,
+      sticky: true,
+      destroyOnClick: false,
+      preventDuplicates: true,
+      buttonText,
+      buttonIcon,
+      buttonAction: async () => await this.reauthenticate.perform(),
+      onDestroy,
+    });
+  }
+
+  /**
+   * Makes an attempt to reauthenticate the user. Triggered by the button in the
+   * "session expired" flash message. On re-auth, shows a success message
+   * and resets the locally tracked parameters. On failure, shows a "critical"
+   * error message with a button to retry.
+   */
+  protected reauthenticate = dropTask(async () => {
+    try {
+      if (this.configSvc.config.skip_google_auth) {
+        // Reload to redirect to Okta login.
+        window.location.reload();
+      } else {
+        await this.authenticate("authenticator:torii", "google-oauth2-bearer");
+      }
+
+      this.flashMessages.clearMessages();
+
+      // Wait a bit to show the success message.
+      await timeout(Ember.testing ? 0 : 1500);
+
+      this.flashMessages.add({
+        title: "Login successful",
+        message: `Welcome back${
+          this.authenticatedUser.info.name
+            ? `, ${this.authenticatedUser.info.name}`
+            : ""
+        }!`,
+        type: "success",
+        timeout: 3000,
+        destroyOnClick: true,
+      });
+
+      // Reset the local parameters.
+      this.preventReauthenticationMessage = false;
+      this.reauthenticationMessageIsShown = false;
+      this.pollForExpiredAuth.perform();
+    } catch (error: unknown) {
+      this.flashMessages.clearMessages();
+      this.showReauthMessage("Login failed", error as string, "critical");
+    }
+  });
+
   // ember-simple-auth only uses a cookie to track redirect target if you're using fastboot, otherwise it keeps track of the redirect target as a parameter on the session service. See the source here: https://github.com/mainmatter/ember-simple-auth/blob/a7e583cf4d04d6ebc96b198a8fa6dde7445abf0e/packages/ember-simple-auth/addon/-internals/routing.js#L33-L50
   //
   // Because we redirect as part of the authentication flow, the parameter storing the transition gets reset. Instead, we keep track of the redirectTarget in browser sessionStorage and override the handleAuthentication method as recommended by ember-simple-auth.
 
   handleAuthentication(routeAfterAuthentication: string) {
+    if (this.authenticatedUser.info) {
+      /**
+       * This will be true when reauthenticating via the "token expired" message.
+       * Since we already have cached userInfo, we don't need to await it.
+       */
+      void this.authenticatedUser.loadInfo.perform();
+      void this.pollForExpiredAuth.perform();
+    }
+
     let redirectStorageValue =
       window.sessionStorage.getItem(REDIRECT_STORAGE_KEY) ||
       window.localStorage.getItem(REDIRECT_STORAGE_KEY);

web/app/templates/authenticate.hbs

@@ -21,7 +21,7 @@
       @text="Authenticate with Google"
       @size="large"
       @icon="google"
-      {{on "click" this.authenticate}}
+      {{on "click" (perform this.authenticate)}}
     />
 
   </Hds::Card::Container>

web/tests/acceptance/application-test.ts

@@ -1,7 +1,10 @@
 import { click, teardownContext, visit, waitFor } from "@ember/test-helpers";
 import { setupApplicationTest } from "ember-qunit";
 import { module, test } from "qunit";
-import { authenticateSession } from "ember-simple-auth/test-support";
+import {
+  authenticateSession,
+  invalidateSession,
+} from "ember-simple-auth/test-support";
 import { MirageTestContext, setupMirage } from "ember-cli-mirage/test-support";
 import SessionService from "hermes/services/session";
 
@@ -36,7 +39,7 @@ module("Acceptance | application", function (hooks) {
 
     assert
       .dom("[data-test-flash-notification-title]")
-      .hasText("Login token expired");
+      .hasText("Session expired");
 
     assert
       .dom("[data-test-flash-notification-description]")
@@ -45,6 +48,7 @@ module("Acceptance | application", function (hooks) {
     assert
       .dom("[data-test-flash-notification-button]")
       .hasText("Authenticate with Google");
+
     /**
      * FIXME: Investigate unresolved promises
      *
@@ -84,23 +88,41 @@ module("Acceptance | application", function (hooks) {
     teardownContext(this);
   });
 
-  test("the authorize button works as expected", async function (this: ApplicationTestContext, assert) {
+  test("the reauthenticate button works as expected (success)", async function (this: ApplicationTestContext, assert) {
     let authCount = 0;
 
     await authenticateSession({});
 
-    this.session.authenticate = () => {
+    this.session.authenticate = async () => {
       authCount++;
     };
 
+    const warningSelector = "[data-test-flash-notification-type='warning']";
+    const successSelector = "[data-test-flash-notification-type='success']";
+
     await visit("/");
-    await this.session.invalidate();
-    await waitFor("[data-test-flash-notification]");
+    await invalidateSession();
+
+    await waitFor(warningSelector);
+
     await click("[data-test-flash-notification-button]");
+    await waitFor(successSelector);
 
     assert
-      .dom("[data-test-flash-notification]")
-      .doesNotExist("flash notification is dismissed on buttonClick");
+      .dom(warningSelector)
+      .doesNotExist("flash notification is dismissed on reauth buttonClick");
+
+    assert
+      .dom(successSelector)
+      .exists("flash notification is shown on successful re-auth");
+
+    assert
+      .dom(`${successSelector} [data-test-flash-notification-title]`)
+      .hasText("Login successful");
+
+    assert
+      .dom(`${successSelector} [data-test-flash-notification-description]`)
+      .hasText("Welcome back, Test User!");
 
     assert.equal(authCount, 1, "session.authenticate() was called");
 
@@ -114,4 +136,42 @@ module("Acceptance | application", function (hooks) {
      */
     teardownContext(this);
   });
+
+  test("the reauthenticate button works as expected (failure)", async function (this: ApplicationTestContext, assert) {
+    await authenticateSession({});
+
+    this.session.authenticate = async () => {
+      throw new Error("Authentication failed");
+    };
+
+    const warningSelector = "[data-test-flash-notification-type='warning']";
+    const criticalSelector = "[data-test-flash-notification-type='critical']";
+
+    await visit("/");
+    await this.session.invalidate();
+
+    await waitFor(warningSelector);
+
+    assert
+      .dom(warningSelector)
+      .exists("flash notification is shown on session invalidation");
+
+    await click("[data-test-flash-notification-button]");
+
+    await waitFor(criticalSelector);
+
+    assert
+      .dom(criticalSelector)
+      .exists("flash notification is shown on re-auth failure");
+
+    /**
+     * FIXME: Investigate unresolved promises
+     *
+     * For reasons not yet clear, this test has unresolved promises
+     * that prevent it from completing naturally. Because of this,
+     * we handle teardown manually.
+     *
+     */
+    teardownContext(this);
+  });
 });