From bcf58305f497b19368b15954bc2b520dfa9e0987 Mon Sep 17 00:00:00 2001 From: Kasama Date: Fri, 22 Nov 2019 19:10:36 -0300 Subject: [PATCH 1/2] 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`. --- ui/app/components/auth-jwt.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ui/app/components/auth-jwt.js b/ui/app/components/auth-jwt.js index acaeb98ae011..f349c5cf4060 100644 --- 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 From 577b57ad7ca27df998bf1c026dfa7336b37a66e0 Mon Sep 17 00:00:00 2001 From: Kasama Date: Fri, 22 Nov 2019 21:51:36 -0300 Subject: [PATCH 2/2] Fix tests for auth-jwt --- .../integration/components/auth-jwt-test.js | 29 +++++++++++-------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/ui/tests/integration/components/auth-jwt-test.js b/ui/tests/integration/components/auth-jwt-test.js index 9c2bd9f015f9..f4d1a56c0592 100644 --- 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,7 +203,7 @@ 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'); @@ -210,12 +211,14 @@ module('Integration | Component | auth jwt', function(hooks) { 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'); @@ -223,13 +226,15 @@ module('Integration | Component | auth jwt', function(hooks) { 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'); @@ -237,14 +242,14 @@ module('Integration | Component | auth jwt', function(hooks) { 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');