Merge pull request #3075 from mainmatter/redirectTarget-to-use-sessionStorage

feat(ember-simple-auth): redirectTarget should be stored in sessionStorage when available

Author: BobrImperator

packages/ember-simple-auth/src/-internals/routing.js

@@ -6,7 +6,6 @@ export function requireAuthentication(owner, transition, extraArgs) {
   let sessionService = owner.lookup('service:session');
   let isAuthenticated = sessionService.get('isAuthenticated');
   if (!isAuthenticated) {
-    const internalSession = sessionService.session;
     let redirectTarget = extraArgs?.redirectTarget;
 
     if (transition) {
@@ -18,7 +17,7 @@ export function requireAuthentication(owner, transition, extraArgs) {
     }
 
     if (redirectTarget) {
-      internalSession.setRedirectTarget(redirectTarget);
+      sessionService.setRedirectTarget(redirectTarget);
     }
   }
   return isAuthenticated;
@@ -37,8 +36,7 @@ export function prohibitAuthentication(owner, routeIfAlreadyAuthenticated) {
 export function handleSessionAuthenticated(owner, routeAfterAuthentication) {
   let sessionService = owner.lookup('service:session');
   let attemptedTransition = sessionService.get('attemptedTransition');
-  const internalSession = sessionService.session;
-  const redirectTarget = internalSession.getRedirectTarget();
+  const redirectTarget = sessionService.getRedirectTarget();
 
   let routerService = owner.lookup('service:router');
 
@@ -47,7 +45,7 @@ export function handleSessionAuthenticated(owner, routeAfterAuthentication) {
     sessionService.set('attemptedTransition', null);
   } else if (redirectTarget) {
     routerService.transitionTo(redirectTarget);
-    internalSession.clearRedirectTarget();
+    sessionService.clearRedirectTarget();
   } else {
     routerService.transitionTo(routeAfterAuthentication);
   }

packages/ember-simple-auth/src/internal-session.js

@@ -288,14 +288,14 @@ export default ObjectProxy.extend({
   },
 
   setRedirectTarget(url) {
-    this.store.setRedirectTarget(url);
+    this.store.setRedirectTarget?.(url);
   },
 
   getRedirectTarget() {
-    return this.store.getRedirectTarget();
+    return this.store.getRedirectTarget?.();
   },
 
   clearRedirectTarget() {
-    return this.store.clearRedirectTarget();
+    return this.store.clearRedirectTarget?.();
   },
 });

packages/ember-simple-auth/src/services/session.ts

@@ -135,6 +135,9 @@ export default class SessionService<Data = DefaultDataShape> extends Service {
     {@linkplain SessionService.requireAuthentication}
     If an attempted transition is present it will be retried.
 
+    This is an `in-memory` property, see {@linkplain SessionService.setRedirectTarget}, {@linkplain SessionService.getRedirectTarget} for a persistent redirect mechanism.
+    `attemptedTransition` is used _first_ if set.
+
     @memberof SessionService
     @property attemptedTransition
     @type Transition
@@ -144,6 +147,15 @@ export default class SessionService<Data = DefaultDataShape> extends Service {
   @alias('session.attemptedTransition')
   attemptedTransition: null | Transition = null;
 
+  get redirectTargetKey(): string | null {
+    const store = this.store as { key?: string; cookieName?: string };
+    const key = store.key || store.cookieName;
+    if (key) {
+      return `${key}-redirectTarget`;
+    }
+    return null;
+  }
+
   set(key: any, value: any) {
     const setsSessionData = SESSION_DATA_KEY_PREFIX.test(key);
     if (setsSessionData) {
@@ -238,6 +250,13 @@ export default class SessionService<Data = DefaultDataShape> extends Service {
     will be saved in a `ember_simple_auth-redirectTarget` cookie for use by the
     browser after authentication is complete.
 
+    Accepts an optional object with `redirectTarget` property. Related to {@linkplain SessionService.setRedirectTarget}, {@linkplain SessionService.getRedirectTarget}
+
+    @example
+      // your-route.js
+      this.session.requireAuthentication(transition, 'login', { redirectTarget: '/alternative-to-transition.intent.url' })
+
+
     @memberof SessionService
     @method requireAuthentication
     @param {Transition} transition A transition that triggered the authentication requirement or null if the requirement originated independently of a transition
@@ -354,4 +373,57 @@ export default class SessionService<Data = DefaultDataShape> extends Service {
       // If it raises an error then it means that restore didn't find any restorable state.
     });
   }
+
+  /**
+    Stores the `redirectTarget` in both `globalThis.sessionStorage` and the configured `session-store`.
+    Key is computed based on the `session-store:application` `key` or `cookieName` property.
+
+    This method is internally called by {@linkplain SessionService.requireAuthentication}.
+
+    @memberof SessionService
+    @method setRedirectTarget
+    @public
+  */
+  setRedirectTarget(url: string) {
+    this.session.setRedirectTarget(url);
+    if (this.redirectTargetKey) {
+      globalThis.sessionStorage?.setItem(this.redirectTargetKey, url);
+    }
+  }
+
+  /**
+    Retrieves the `redirectTarget` from `globalThis.sessionStorage` first,
+    falls back to `session-store:application` when nothing's found.
+
+    This method is internally called by {@linkplain SessionService.handleAuthentication}.
+
+    @memberof SessionService
+    @method getRedirectTarget
+    @public
+  */
+  getRedirectTarget() {
+    let redirectTarget: string | null = this.session.getRedirectTarget();
+
+    if (this.redirectTargetKey) {
+      return globalThis.sessionStorage?.getItem(this.redirectTargetKey) || redirectTarget;
+    } else {
+      return redirectTarget;
+    }
+  }
+
+  /**
+    Clears the `redirectTarget` from `globalThis.sessionStorage` and the `session-store:application`.
+
+    This method is internally called by {@linkplain SessionService.handleAuthentication}.
+
+    @memberof SessionService
+    @method clearRedirectTarget
+    @public
+  */
+  clearRedirectTarget() {
+    this.session.clearRedirectTarget();
+    if (this.redirectTargetKey) {
+      globalThis.sessionStorage?.removeItem(this.redirectTargetKey);
+    }
+  }
 }

packages/ember-simple-auth/src/session-stores/base.ts

@@ -73,8 +73,52 @@ export default abstract class EsaBaseSessionStore extends EmberObject {
   */
   abstract clear(): Promise<unknown>;
 
+  /**
+    Called by {@linkplain SessionService.setRedirectTarget}.
+
+    This method is used to persist a `redirectTarget` which is an alternative to the non-persistent {@linkplain SessionService.attemptedTransition}.
+    `redirectTarget` is meant to replace {@linkplain SessionService.attemptedTransition} that is discarded the moment a browser tab is refreshed.
+
+    This method is meant to be always implemented but can be `undefined` for backwards compatibility.
+    Additionally can be assigned `null` to opt out of using this mechanism.
+
+    @example
+      setRedirectTarget = null;
+
+    @memberof BaseStore
+    @method setRedirectTarget
+    @public
+  */
   abstract setRedirectTarget(urL: string): void;
+
+  /**
+    Called by {@linkplain SessionService.getRedirectTarget}.
+
+    This method is retrieve a persisted `redirectTarget` from a child class' store mechanism.
+    Additionally can be assigned `null` to opt out of using this mechanism.
+
+    @example
+      getRedirectTarget = null;
+
+    @memberof BaseStore
+    @method getRedirectTarget
+    @public
+  */
   abstract getRedirectTarget(): string | null;
+
+  /**
+    Called by {@linkplain SessionService.clearRedirectTarget}.
+
+    This method clears a `redirectTarget` from a child class' store mechanism.
+    Additionally can be assigned `null` to opt out of using this mechanism.
+
+    @example
+      clearRedirectTarget = null;
+
+    @memberof BaseStore
+    @method clearRedirectTarget
+    @public
+  */
   abstract clearRedirectTarget(): void;
 
   on<Event extends keyof SessionEvents>(event: Event, cb: EventListener<SessionEvents, Event>) {

packages/playwright-tests/tests/test-app.spec.ts

@@ -111,5 +111,34 @@ STORAGE_SCENARIOS.forEach(scenario => {
       await loginWithPassword(page);
       await confirmLoggedIn(page, { expectedUrl: '/protected' });
     });
+
+    test('user is redirected to the last visited route for a given browser tab session', async ({
+      page,
+      context,
+    }) => {
+      test.skip(
+        process.env.FASTBOOT_DISABLED !== 'true',
+        'This feature relies on SessionStorage which is unavailable in fastboot.'
+      );
+      await specifyTestAppStorageAdapter(page, scenario);
+      await page.goto('/protected');
+
+      await page.getByTestId('route-login').click();
+      await expect(page).toHaveURL('/login#');
+
+      const anotherPage = await context.newPage();
+      await specifyTestAppStorageAdapter(anotherPage, scenario);
+      await anotherPage.goto('/another-protected');
+
+      // Make sure to verify persistence
+      await anotherPage.reload();
+      await page.reload();
+
+      await expect(anotherPage).toHaveURL('/login');
+
+      await loginWithPassword(page);
+      await confirmLoggedIn(page, { expectedUrl: '/protected' });
+      await confirmLoggedIn(anotherPage, { expectedUrl: '/another-protected' });
+    });
   });
 });

packages/test-app/app/router.js

@@ -9,6 +9,7 @@ class Router extends EmberRouter {
 Router.map(function () {
   this.route('login');
   this.route('protected');
+  this.route('another-protected');
   this.route('auth-error');
   this.route('callback');
   this.mount('my-engine', { as: 'engine', path: '/engine' });

packages/test-app/app/routes/another-protected.js

@@ -0,0 +1,15 @@
+import Route from '@ember/routing/route';
+import { service } from '@ember/service';
+
+export default class AnotherProtectedRoute extends Route {
+  @service session;
+  @service store;
+
+  beforeModel(transition) {
+    this.session.requireAuthentication(transition, 'login');
+  }
+
+  model() {
+    return this.store.findAll('post');
+  }
+}

packages/test-app/app/templates/another-protected.hbs

@@ -0,0 +1,13 @@
+<h1>Another Protected Page</h1>
+<div class="alert alert-warning">
+  This is an "another" protected page only visible to authenticated users!
+</div>
+<h2>Posts</h2>
+<ul>
+  {{#each this.model as |post|}}
+    <li>
+      <h3>{{post.title}}</h3>
+      <p>{{post.body}}</p>
+    </li>
+  {{/each}}
+</ul>

packages/test-esa/tests/helpers/mocked-session-storage.js

@@ -0,0 +1,25 @@
+export class MockSessionStorage {
+  constructor() {
+    this.store = new Map();
+  }
+
+  get length() {
+    return this.store.size;
+  }
+
+  clear() {
+    this.store.clear();
+  }
+
+  getItem(key) {
+    return this.store.get(key) ?? null;
+  }
+
+  removeItem(key) {
+    this.store.delete(key);
+  }
+
+  setItem(key, value) {
+    this.store.set(key, String(value));
+  }
+}

packages/test-esa/tests/unit/services/session-test.js

@@ -3,6 +3,7 @@ import { setupTest } from 'ember-qunit';
 import Service from '@ember/service';
 import EmberObject, { set } from '@ember/object';
 import sinonjs from 'sinon';
+import { MockSessionStorage } from '../../helpers/mocked-session-storage';
 
 module('SessionService', function (hooks) {
   setupTest(hooks);
@@ -13,6 +14,7 @@ module('SessionService', function (hooks) {
 
   hooks.beforeEach(function () {
     sinon = sinonjs.createSandbox();
+    sinon.stub(window, 'sessionStorage').value(new MockSessionStorage());
     this.owner.register(
       'authorizer:custom',
       EmberObject.extend({
@@ -281,6 +283,15 @@ module('SessionService', function (hooks) {
           session.getRedirectTarget();
           assert.ok(readCookieStub.calledWith(cookieName));
         });
+
+        test("doesn't throw when session-store doesn't implement redirectTarget methods", function (assert) {
+          assert.expect(0);
+          session.store.setRedirectTarget = null;
+          session.store.getRedirectTarget = null;
+          session.store.clearRedirectTarget = null;
+
+          sessionService.requireAuthentication(transition, 'login', { redirectTarget });
+        });
       });
 
       module('if no transition is passed', function () {