fix: session data shape bug (#1264)

* fix: session data shape bug * add a test * add more tests * wait to setup session until after we check if it is authenticated
eclipse-pass · May 7, 2024 · 1bf82fe · 1bf82fe
1 parent 21d5ed3
commit 1bf82fe
Show file tree

Hide file tree

Showing 7 changed files with 180 additions and 64 deletions.
diff --git a/app/authenticators/http-only.js b/app/authenticators/http-only.js
@@ -1,4 +1,3 @@
-/* eslint-disable ember/no-computed-properties-in-native-classes */
 import Base from 'ember-simple-auth/authenticators/base';
 import RSVP from 'rsvp';
 import { inject as service } from '@ember/service';
@@ -16,16 +15,34 @@ export default class HttpOnly extends Base {
    @return {Ember.RSVP.Promise} A promise that when it resolves results in the session becoming or remaining authenticated
    @public
    */
-  restore(data) {
+  async restore(data) {
+    const normalizedData = this.normalizeSessionData(data);
+    const dataIsValid = await this._validateData(normalizedData);
+
     return new RSVP.Promise((resolve, reject) => {
-      if (!this._validateData(data)) {
+      if (dataIsValid) {
+        return resolve(normalizedData);
+      } else {
         return reject('Could not restore session.');
       }
-
-      return resolve(data);
     });
   }
 
+  /**
+   * Normalizes session data so that we maintain a consistent shape.
+   * @param {*} data
+   * @returns data
+   */
+  normalizeSessionData(data) {
+    if (!data?.id && data?.user?.id) {
+      return {
+        id: data.user.id,
+        ...data,
+      };
+    }
+    return data;
+  }
+
   /**
    * @method authenticate
    * @return {Ember.RSVP.Promise} A promise that when it resolves results in the session becoming authenticated
@@ -37,25 +54,18 @@ export default class HttpOnly extends Base {
     let response = await fetch(url);
 
     if (response.ok) {
-      return response.json();
+      const data = await response.json();
+      const normalizedData = this.normalizeSessionData(data);
+
+      return new RSVP.Promise((resolve, _reject) => {
+        return resolve(normalizedData);
+      });
     } else {
       let error = await response.text();
       throw new Error(error);
     }
   }
 
-  /**
-   This method simply returns a resolving promise.
-   @method invalidate
-   @return {Ember.RSVP.Promise} A promise that when it resolves results in the session being invalidated
-   @public
-   */
-  invalidate() {
-    return new RSVP.Promise((resolve /*, reject*/) => {
-      resolve();
-    });
-  }
-
   async _validateData(data) {
     // see https://tools.ietf.org/html/rfc6749#section-4.2.2
     if (isEmpty(data) || isEmpty(data.id)) return false;
@@ -65,7 +75,7 @@ export default class HttpOnly extends Base {
     let response = await fetch(url);
 
     if (response.ok) {
-      const refreshedData = await response.json();
+      const refreshedData = this.normalizeSessionData(await response.json());
 
       return data.id === refreshedData.id;
     } else {

diff --git a/app/routes/check-session-route.js b/app/routes/check-session-route.js
@@ -1,8 +1,6 @@
-/* eslint-disable ember/no-jquery */
 import { inject as service } from '@ember/service';
 import Route from '@ember/routing/route';
 import { action } from '@ember/object';
-import ENV from 'pass-ui/config/environment';
 
 export default class CheckSessionRouteRoute extends Route {
   @service session;
@@ -13,6 +11,8 @@ export default class CheckSessionRouteRoute extends Route {
 
   async beforeModel() {
     if (!this.session.isAuthenticated) {
+      await this.session.setup();
+
       await this.session.authenticate('authenticator:http-only');
     }
 

diff --git a/mirage/config.js b/mirage/config.js
@@ -43,9 +43,8 @@ export default function (config) {
       });
 
       /** User Service */
-      this.get('/pass-user-service/whoami', (schema, request) => {
-        const userId = request.queryParams.userToken;
-        return schema.find('user', userId);
+      this.get('/user/whoami', (_schema, _request) => {
+        return { user: { id: '0' } };
       });
 
       /** Download service */

diff --git a/package.json b/package.json
@@ -81,7 +81,7 @@
     "ember-qunit": "^6.2.0",
     "ember-radio-buttons": "^5.0.0",
     "ember-resolver": "^10.0.0",
-    "ember-simple-auth": "^4.2.2",
+    "ember-simple-auth": "^6.0.0",
     "ember-sinon-qunit": "^7.1.4",
     "ember-source": "~4.8.0",
     "ember-template-lint": "^4.16.1",

diff --git a/tests/acceptance/session-test.js b/tests/acceptance/session-test.js
@@ -0,0 +1,24 @@
+import { module, test } from 'qunit';
+import { visit } from '@ember/test-helpers';
+import { setupApplicationTest } from 'ember-qunit';
+import { currentSession, authenticateSession } from 'ember-simple-auth/test-support';
+import sharedScenario from '../../mirage/scenarios/shared';
+import setupMirage from 'ember-cli-mirage/test-support/setup-mirage';
+
+module('Acceptance | session', function (hooks) {
+  setupApplicationTest(hooks);
+  setupMirage(hooks);
+
+  test('authenticating with the correct session data shape results in valid session data', async function (assert) {
+    await authenticateSession({
+      id: '0',
+    });
+
+    sharedScenario(this.server);
+
+    await visit('/app');
+
+    let sessionData = currentSession().get('data.authenticated');
+    assert.equal(sessionData.id, '0');
+  });
+});
diff --git a/tests/unit/authenticators/http-only-test.js b/tests/unit/authenticators/http-only-test.js
@@ -0,0 +1,71 @@
+import HttpOnly from 'pass-ui/authenticators/http-only';
+import { module, test } from 'qunit';
+import { setupTest } from 'ember-qunit';
+import { setupMirage } from 'ember-cli-mirage/test-support';
+
+module('HttpOnlyAuthenticator', function (hooks) {
+  setupTest(hooks);
+  setupMirage(hooks);
+
+  let authenticator;
+
+  hooks.beforeEach(function () {
+    authenticator = HttpOnly.create();
+  });
+
+  module('#restore', function () {
+    test('returns a resolving promise', async function (assert) {
+      server.get('/user/whoami', (_schema, _request) => {
+        return {
+          id: '0',
+        };
+      });
+
+      const response = await authenticator.restore({ id: '0' });
+      const expectedResponse = { id: '0' };
+
+      assert.strictEqual(response.id, expectedResponse.id);
+    });
+
+    test('returns a resolving promise even when whoami response is not normalized', async function (assert) {
+      server.get('/user/whoami', (_schema, _request) => {
+        return {
+          user: { id: '0' },
+        };
+      });
+
+      const response = await authenticator.restore({ id: '0' });
+      const expectedResponse = { id: '0' };
+
+      assert.strictEqual(response.id, expectedResponse.id);
+    });
+  });
+
+  module('#authenticate', function () {
+    test('returns a resolving promise', async function (assert) {
+      server.get('/user/whoami', (_schema, _request) => {
+        return {
+          id: '0',
+        };
+      });
+
+      const response = await authenticator.authenticate();
+      const expectedResponse = { id: '0' };
+
+      assert.strictEqual(response.id, expectedResponse.id);
+    });
+
+    test('returns a resolving promise even when whoami response is not normalized', async function (assert) {
+      server.get('/user/whoami', (_schema, _request) => {
+        return {
+          user: { id: '0' },
+        };
+      });
+
+      const response = await authenticator.authenticate();
+      const expectedResponse = { id: '0' };
+
+      assert.strictEqual(response.id, expectedResponse.id);
+    });
+  });
+});
diff --git a/yarn.lock b/yarn.lock
@@ -1303,6 +1303,16 @@
     ember-cli-htmlbars "^6.1.1"
     ember-destroyable-polyfill "^2.0.3"
 
+"@ember/test-waiters@^3":
+  version "3.1.0"
+  resolved "https://registry.yarnpkg.com/@ember/test-waiters/-/test-waiters-3.1.0.tgz#61399919cbf793978da0b8bfdfdb7bca0cb80e9e"
+  integrity sha512-bb9h95ktG2wKY9+ja1sdsFBdOms2lB19VWs8wmNpzgHv1NCetonBoV5jHBV4DHt0uS1tg9z66cZqhUVlYs96KQ==
+  dependencies:
+    calculate-cache-key-for-tree "^2.0.0"
+    ember-cli-babel "^7.26.6"
+    ember-cli-version-checker "^5.1.2"
+    semver "^7.3.5"
+
 "@ember/test-waiters@^3.0.0":
   version "3.0.2"
   resolved "https://registry.yarnpkg.com/@ember/test-waiters/-/test-waiters-3.0.2.tgz#5b950c580a1891ed1d4ee64f9c6bacf49a15ea6f"
@@ -1313,6 +1323,15 @@
     ember-cli-version-checker "^5.1.2"
     semver "^7.3.5"
 
+"@embroider/addon-shim@^1.0.0", "@embroider/addon-shim@^1.7.1":
+  version "1.8.7"
+  resolved "https://registry.yarnpkg.com/@embroider/addon-shim/-/addon-shim-1.8.7.tgz#ba2dcb0647ed2cb0c500c835326266b89ceec595"
+  integrity sha512-JGOQNRj3UR0NdWEg8MsM2eqPLncEwSB1IX+rwntIj22TEKj8biqx7GDgSbeH+ZedijmCh354Hf2c5rthrdzUAw==
+  dependencies:
+    "@embroider/shared-internals" "^2.5.1"
+    broccoli-funnel "^3.0.8"
+    semver "^7.3.8"
+
 "@embroider/addon-shim@^1.5.0", "@embroider/addon-shim@^1.8.4":
   version "1.8.4"
   resolved "https://registry.yarnpkg.com/@embroider/addon-shim/-/addon-shim-1.8.4.tgz#0e7f32c5506bf0f3eb0840506e31c36c7053763c"
@@ -1364,6 +1383,22 @@
     semver "^7.3.5"
     typescript-memoize "^1.0.1"
 
+"@embroider/shared-internals@^2.5.1":
+  version "2.6.0"
+  resolved "https://registry.yarnpkg.com/@embroider/shared-internals/-/shared-internals-2.6.0.tgz#851fd8d051fd4f7f93b2b7130e2ae5cdd537c5d6"
+  integrity sha512-A2BYQkhotdKOXuTaxvo9dqOIMbk+2LqFyqvfaaePkZcFJvtCkvTaD31/sSzqvRF6rdeBHjdMwU9Z2baPZ55fEQ==
+  dependencies:
+    babel-import-util "^2.0.0"
+    debug "^4.3.2"
+    ember-rfc176-data "^0.3.17"
+    fs-extra "^9.1.0"
+    js-string-escape "^1.0.1"
+    lodash "^4.17.21"
+    minimatch "^3.0.4"
+    resolve-package-path "^4.0.1"
+    semver "^7.3.5"
+    typescript-memoize "^1.0.1"
+
 "@embroider/[email protected]":
   version "1.8.3"
   resolved "https://registry.yarnpkg.com/@embroider/util/-/util-1.8.3.tgz#7267a2b6fcbf3e56712711441159ab373f9bee7a"
@@ -3102,11 +3137,6 @@ balanced-match@^1.0.0:
   resolved "https://registry.yarnpkg.com/balanced-match/-/balanced-match-1.0.2.tgz#e83e3a7e3f300b34cb9d87f615fa0cbf357690ee"
   integrity sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw==
 
-base-64@^0.1.0:
-  version "0.1.0"
-  resolved "https://registry.yarnpkg.com/base-64/-/base-64-0.1.0.tgz#780a99c84e7d600260361511c4877613bf24f6bb"
-  integrity sha512-Y5gU45svrR5tI2Vt/X9GPd3L0HNIKzGu202EjxrXMpuc2V2CiKgemAbUUsqYmZJvPtCXoUKjNZwBJzsNScUbXA==
-
 base64-js@^1.0.2, base64-js@^1.3.1:
   version "1.5.1"
   resolved "https://registry.yarnpkg.com/base64-js/-/base64-js-1.5.1.tgz#1b1b440160a5bf7ad40b650f095963481903930a"
@@ -3501,7 +3531,7 @@ [email protected]:
     symlink-or-copy "^1.0.0"
     walk-sync "^0.3.1"
 
-"broccoli-funnel@^1.2.0 || ^2.0.0", broccoli-funnel@^2.0.0, broccoli-funnel@^2.0.1, broccoli-funnel@^2.0.2:
+broccoli-funnel@^2.0.0, broccoli-funnel@^2.0.1, broccoli-funnel@^2.0.2:
   version "2.0.2"
   resolved "https://registry.yarnpkg.com/broccoli-funnel/-/broccoli-funnel-2.0.2.tgz#0edf629569bc10bd02cc525f74b9a38e71366a75"
   integrity sha512-/vDTqtv7ipjEZQOVqO4vGDVAOZyuYzQ/EgGoyewfOgh1M7IQAToBKZI0oAQPgMBeFPPlIbfMuAngk+ohPBuaHQ==
@@ -3557,7 +3587,7 @@ broccoli-merge-trees@^3.0.0, broccoli-merge-trees@^3.0.1, broccoli-merge-trees@^
     broccoli-plugin "^1.3.0"
     merge-trees "^2.0.0"
 
-broccoli-merge-trees@^4.0.0, broccoli-merge-trees@^4.2.0:
+broccoli-merge-trees@^4.2.0:
   version "4.2.0"
   resolved "https://registry.yarnpkg.com/broccoli-merge-trees/-/broccoli-merge-trees-4.2.0.tgz#692d3c163ecea08c5714a9434d664e628919f47c"
   integrity sha512-nTrQe5AQtCrW4enLRvbD/vTLHqyW2tz+vsLXQe4IEaUhepuMGVKJJr+I8n34Vu6fPjmPLwTjzNC8izMIDMtHPw==
@@ -5271,7 +5301,7 @@ ember-cli-babel-plugin-helpers@^1.0.0, ember-cli-babel-plugin-helpers@^1.1.0, em
   resolved "https://registry.yarnpkg.com/ember-cli-babel-plugin-helpers/-/ember-cli-babel-plugin-helpers-1.1.1.tgz#5016b80cdef37036c4282eef2d863e1d73576879"
   integrity sha512-sKvOiPNHr5F/60NLd7SFzMpYPte/nnGkq/tMIfXejfKHIhaiIkYFqX8Z9UFTKWLLn+V7NOaby6niNPZUdvKCRw==
 
-ember-cli-babel@^7.0.0, ember-cli-babel@^7.1.0, ember-cli-babel@^7.1.3, ember-cli-babel@^7.10.0, ember-cli-babel@^7.11.1, ember-cli-babel@^7.13.0, ember-cli-babel@^7.13.2, ember-cli-babel@^7.19.0, ember-cli-babel@^7.20.5, ember-cli-babel@^7.22.1, ember-cli-babel@^7.23.0, ember-cli-babel@^7.23.1, ember-cli-babel@^7.26.0, ember-cli-babel@^7.26.10, ember-cli-babel@^7.26.11, ember-cli-babel@^7.26.3, ember-cli-babel@^7.26.4, ember-cli-babel@^7.26.5, ember-cli-babel@^7.26.6, ember-cli-babel@^7.26.8, ember-cli-babel@^7.5.0, ember-cli-babel@^7.7.3, ember-cli-babel@^7.9.0:
+ember-cli-babel@^7.0.0, ember-cli-babel@^7.1.3, ember-cli-babel@^7.10.0, ember-cli-babel@^7.11.1, ember-cli-babel@^7.13.0, ember-cli-babel@^7.13.2, ember-cli-babel@^7.19.0, ember-cli-babel@^7.22.1, ember-cli-babel@^7.23.0, ember-cli-babel@^7.23.1, ember-cli-babel@^7.26.0, ember-cli-babel@^7.26.10, ember-cli-babel@^7.26.11, ember-cli-babel@^7.26.3, ember-cli-babel@^7.26.4, ember-cli-babel@^7.26.5, ember-cli-babel@^7.26.6, ember-cli-babel@^7.26.8, ember-cli-babel@^7.5.0, ember-cli-babel@^7.7.3, ember-cli-babel@^7.9.0:
   version "7.26.11"
   resolved "https://registry.yarnpkg.com/ember-cli-babel/-/ember-cli-babel-7.26.11.tgz#50da0fe4dcd99aada499843940fec75076249a9f"
   integrity sha512-JJYeYjiz/JTn34q7F5DSOjkkZqy8qwFOOxXfE6pe9yEJqWGu4qErKxlz8I22JoVEQ/aBUO+OcKTpmctvykM9YA==
@@ -5791,13 +5821,12 @@ ember-concurrency@^2.0.0, ember-concurrency@^2.3.4:
     ember-compatibility-helpers "^1.2.0"
     ember-destroyable-polyfill "^2.0.2"
 
-ember-cookies@^0.5.0:
-  version "0.5.2"
-  resolved "https://registry.yarnpkg.com/ember-cookies/-/ember-cookies-0.5.2.tgz#06e33463f2f83254fefaf224cc944dec3fb9d3ba"
-  integrity sha512-nZ7oG97kBcO9UHjO95ryABpnVx62Bhxo7lIsBJNmWvFXleILm9DGueiAynzXxuYWWPuKIeeSbYakrS1869tNTw==
+ember-cookies@^1.0.0:
+  version "1.1.2"
+  resolved "https://registry.yarnpkg.com/ember-cookies/-/ember-cookies-1.1.2.tgz#1e7b2abdffc1364633efe7cc8f4c0d0439a3459f"
+  integrity sha512-6GaN0eEDZT9SEUSZBxWzZMlvxjcGKXFTJNjv30LVXTTOxozE5IBmIxiDAEq0udi0UpWUGHLYQBgnANn4jdll7w==
   dependencies:
-    ember-cli-babel "^7.1.0"
-    ember-getowner-polyfill "^1.1.0 || ^2.0.0"
+    "@embroider/addon-shim" "^1.7.1"
 
 ember-css-modules@^2.0.1:
   version "2.0.1"
@@ -5868,13 +5897,6 @@ ember-element-helper@^0.6.1:
     ember-cli-babel "^7.26.11"
     ember-cli-htmlbars "^6.0.1"
 
-ember-factory-for-polyfill@^1.3.1:
-  version "1.3.1"
-  resolved "https://registry.yarnpkg.com/ember-factory-for-polyfill/-/ember-factory-for-polyfill-1.3.1.tgz#b446ed64916d293c847a4955240eb2c993b86eae"
-  integrity sha512-y3iG2iCzH96lZMTWQw6LWNLAfOmDC4pXKbZP6FxG8lt7GGaNFkZjwsf+Z5GAe7kxfD7UG4lVkF7x37K82rySGA==
-  dependencies:
-    ember-cli-version-checker "^2.1.0"
-
 ember-fetch@^8.1.2:
   version "8.1.2"
   resolved "https://registry.yarnpkg.com/ember-fetch/-/ember-fetch-8.1.2.tgz#651839780519319309127054786bf35cd4b84543"
@@ -5927,14 +5949,6 @@ ember-get-config@^2.0.0, ember-get-config@^2.1.1:
     "@embroider/macros" "^0.50.0 || ^1.0.0"
     ember-cli-babel "^7.26.6"
 
-"ember-getowner-polyfill@^1.1.0 || ^2.0.0":
-  version "2.2.0"
-  resolved "https://registry.yarnpkg.com/ember-getowner-polyfill/-/ember-getowner-polyfill-2.2.0.tgz#38e7dccbcac69d5ec694000329ec0b2be651d2b2"
-  integrity sha512-rwGMJgbGzxIAiWYjdpAh04Abvt0s3HuS/VjHzUFhVyVg2pzAuz45B9AzOxYXzkp88vFC7FPaiA4kE8NxNk4A4Q==
-  dependencies:
-    ember-cli-version-checker "^2.1.0"
-    ember-factory-for-polyfill "^1.3.1"
-
 "ember-inflector@^2.0.0 || ^3.0.0 || ^4.0.2", ember-inflector@^4.0.2:
   version "4.0.2"
   resolved "https://registry.yarnpkg.com/ember-inflector/-/ember-inflector-4.0.2.tgz#4494f1a5f61c1aca7702d59d54024cc92211d8ec"
@@ -6088,18 +6102,16 @@ ember-router-generator@^2.0.0:
     "@babel/traverse" "^7.4.5"
     recast "^0.18.1"
 
-ember-simple-auth@^4.2.2:
-  version "4.2.2"
-  resolved "https://registry.yarnpkg.com/ember-simple-auth/-/ember-simple-auth-4.2.2.tgz#5c1de1bb13a75e519e512fb82cba7cc1e277b07c"
-  integrity sha512-D7W6OREUvf5OzeB0ePptSNBilccchRYukH4f7mkbL6WT+z6VEqRRAIaQuBZdFM6lrcSFGmzctINLZJwsIpI3wg==
+ember-simple-auth@^6.0.0:
+  version "6.0.0"
+  resolved "https://registry.yarnpkg.com/ember-simple-auth/-/ember-simple-auth-6.0.0.tgz#7c3e4d0885de3f58c0568056eba03a6434f34084"
+  integrity sha512-9SzSFApxZ74CD4UxIeTV+poIPeXcRLXWM60cMvC1SwTYjoc/p9DeQF0pVm6m1XV6uA3kPUzEsEn4/GeHc2YX1w==
   dependencies:
-    base-64 "^0.1.0"
-    broccoli-file-creator "^2.1.1"
-    broccoli-funnel "^1.2.0 || ^2.0.0"
-    broccoli-merge-trees "^4.0.0"
-    ember-cli-babel "^7.20.5"
+    "@ember/test-waiters" "^3"
+    "@embroider/addon-shim" "^1.0.0"
+    "@embroider/macros" "^1.0.0"
     ember-cli-is-package-missing "^1.0.0"
-    ember-cookies "^0.5.0"
+    ember-cookies "^1.0.0"
     silent-error "^1.0.0"
 
 ember-sinon-qunit@^7.1.4: