Fix oidc callback to check entire storage (#7929)

* Fix oidc callback to check entire storage In some cases, extensions or other unexpected javascript can be executed on the oidc callback. Sometimes changing the localstorage and generating a `storage` event before the callback component does. In such cases, the authorization component will hang indefinitely. A simple fix is to check for the expected `oidcState` key in the whole localstorage instead of just the event `key`. * Fix tests for auth-jwt Co-authored-by: Angel Garbarino <[email protected]>
hashicorp · Jan 15, 2020 · aea4d89 · aea4d89
1 parent d38b368
commit aea4d89
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 14 deletions.
diff --git a/ui/app/components/auth-jwt.js b/ui/app/components/auth-jwt.js
@@ -109,13 +109,14 @@ export default Component.extend({
   },
 
   exchangeOIDC: task(function*(event, oidcWindow) {
-    if (event.key !== 'oidcState') {
+    let oidcState = event.storageArea.getItem('oidcState');
+    if (oidcState === null || oidcState === undefined) {
       return;
     }
     this.onLoading(true);
     // get the info from the event fired by the other window and
     // then remove it from localStorage
-    let { namespace, path, state, code } = JSON.parse(event.newValue);
+    let { namespace, path, state, code } = JSON.parse(oidcState);
     this.getWindow().localStorage.removeItem('oidcState');
 
     // defer closing of the window, but continue executing the task

diff --git a/ui/tests/integration/components/auth-jwt-test.js b/ui/tests/integration/components/auth-jwt-test.js
@@ -31,6 +31,7 @@ const fakeWindow = EmberObject.extend(Evented, {
   }),
   localStorage: computed(function() {
     return {
+      getItem: sinon.stub(),
       removeItem: sinon.stub(),
     };
   }),
@@ -202,49 +203,53 @@ module('Integration | Component | auth jwt', function(hooks) {
     assert.equal(this.error, ERROR_WINDOW_CLOSED, 'calls onError with error string');
   });
 
-  test('oidc: storage event fires with wrong key', async function(assert) {
+  test('oidc: storage event fires without state key', async function(assert) {
     await renderIt(this);
     this.set('selectedAuthPath', 'foo');
     await component.role('test');
     component.login();
     await waitUntil(() => {
       return this.openSpy.calledOnce;
     });
-    this.window.trigger('storage', { key: 'wrongThing' });
+    this.window.localStorage.getItem.returns(null);
+    this.window.trigger('storage', { storageArea: this.window.localStorage });
     run.cancelTimers();
-    assert.equal(this.window.localStorage.removeItem.callCount, 0, 'never calls removeItem');
+    assert.ok(this.window.localStorage.getItem.calledOnce, 'calls getItem');
+    assert.notOk(this.window.localStorage.removeItem.called, 'never calls removeItem');
   });
 
-  test('oidc: storage event fires with correct key, wrong params', async function(assert) {
+  test('oidc: storage event fires with state key, wrong params', async function(assert) {
     await renderIt(this);
     this.set('selectedAuthPath', 'foo');
     await component.role('test');
     component.login();
     await waitUntil(() => {
       return this.openSpy.calledOnce;
     });
-    this.window.trigger('storage', { key: 'oidcState', newValue: JSON.stringify({}) });
+    this.window.localStorage.getItem.returns(JSON.stringify({}));
+    this.window.trigger('storage', { storageArea: this.window.localStorage });
     run.cancelTimers();
-    assert.equal(this.window.localStorage.removeItem.callCount, 1, 'calls removeItem');
+    assert.ok(this.window.localStorage.getItem.calledOnce, 'calls getItem');
+    assert.ok(this.window.localStorage.removeItem.calledOnce, 'calls removeItem');
     assert.equal(this.error, ERROR_MISSING_PARAMS, 'calls onError with params missing error');
   });
 
-  test('oidc: storage event fires with correct key, correct params', async function(assert) {
+  test('oidc: storage event fires with state key, correct params', async function(assert) {
     await renderIt(this);
     this.set('selectedAuthPath', 'foo');
     await component.role('test');
     component.login();
     await waitUntil(() => {
       return this.openSpy.calledOnce;
     });
-    this.window.trigger('storage', {
-      key: 'oidcState',
-      newValue: JSON.stringify({
+    this.window.localStorage.getItem.returns(
+      JSON.stringify({
         path: 'foo',
         state: 'state',
         code: 'code',
-      }),
-    });
+      })
+    );
+    this.window.trigger('storage', { storageArea: this.window.localStorage });
     await settled();
     assert.equal(this.selectedAuth, 'token', 'calls onSelectedAuth with token');
     assert.equal(this.token, 'token', 'calls onToken with token');