From a3862bcf974442799f1b6a3eefab5000bc0029bf Mon Sep 17 00:00:00 2001 From: Jordan Reimer Date: Mon, 15 Nov 2021 08:48:11 -0700 Subject: [PATCH] OIDC Auth Bug (#13133) * fixes issue with oidc auth method when MetaMask chrome extenstion is used * adds changelog entry * updates auth-jwt integration tests * fixes race condition in runCommands ui-panel helper method where running multiple commands would not always result in the same output order --- changelog/13133.txt | 3 + ui/app/components/auth-jwt.js | 18 +++-- ui/app/routes/vault/cluster/oidc-callback.js | 3 +- .../integration/components/auth-jwt-test.js | 8 ++- ui/tests/pages/components/console/ui-panel.js | 2 + ui/tests/unit/components/auth-jwt-test.js | 65 +++++++++++++++++++ 6 files changed, 91 insertions(+), 8 deletions(-) create mode 100644 changelog/13133.txt create mode 100644 ui/tests/unit/components/auth-jwt-test.js diff --git a/changelog/13133.txt b/changelog/13133.txt new file mode 100644 index 000000000..ae39a1c92 --- /dev/null +++ b/changelog/13133.txt @@ -0,0 +1,3 @@ +```release-note:bug +ui: Fixes issue with OIDC auth workflow when using MetaMask Chrome extension +``` \ No newline at end of file diff --git a/ui/app/components/auth-jwt.js b/ui/app/components/auth-jwt.js index 80de616b5..2417754ed 100644 --- a/ui/app/components/auth-jwt.js +++ b/ui/app/components/auth-jwt.js @@ -89,12 +89,18 @@ export default Component.extend({ // start watching the popup window and the current one this.watchPopup.perform(oidcWindow); this.watchCurrent.perform(oidcWindow); - // wait for message posted from popup - const event = yield waitForEvent(thisWindow, 'message'); - if (event.origin === thisWindow.origin && event.isTrusted) { - this.exchangeOIDC.perform(event.data, oidcWindow); - } else { - this.handleOIDCError(); + // wait for message posted from oidc callback + // see issue https://github.com/hashicorp/vault/issues/12436 + // ensure that postMessage event is from expected source + while (true) { + const event = yield waitForEvent(thisWindow, 'message'); + if (event.origin !== thisWindow.origin || !event.isTrusted) { + return this.handleOIDCError(); + } + if (event.data.source === 'oidc-callback') { + return this.exchangeOIDC.perform(event.data, oidcWindow); + } + // continue to wait for the correct message } }), diff --git a/ui/app/routes/vault/cluster/oidc-callback.js b/ui/app/routes/vault/cluster/oidc-callback.js index 6c32ea7f7..964d595a7 100644 --- a/ui/app/routes/vault/cluster/oidc-callback.js +++ b/ui/app/routes/vault/cluster/oidc-callback.js @@ -9,7 +9,8 @@ export default Route.extend({ let { auth_path: path, code, state } = this.paramsFor(this.routeName); let { namespaceQueryParam: namespace } = this.paramsFor('vault.cluster'); path = window.decodeURIComponent(path); - let queryParams = { namespace, path, code, state }; + const source = 'oidc-callback'; // required by event listener in auth-jwt component + let queryParams = { source, namespace, path, code, state }; window.opener.postMessage(queryParams, window.origin); }, renderTemplate() { diff --git a/ui/tests/integration/components/auth-jwt-test.js b/ui/tests/integration/components/auth-jwt-test.js index 23d92d77f..bcf49e797 100644 --- a/ui/tests/integration/components/auth-jwt-test.js +++ b/ui/tests/integration/components/auth-jwt-test.js @@ -211,7 +211,10 @@ module('Integration | Component | auth jwt', function(hooks) { await waitUntil(() => { return this.openSpy.calledOnce; }); - this.window.trigger('message', buildMessage({ data: { state: 'state', foo: 'bar' } })); + this.window.trigger( + 'message', + buildMessage({ data: { source: 'oidc-callback', state: 'state', foo: 'bar' } }) + ); run.cancelTimers(); assert.equal(this.error, ERROR_MISSING_PARAMS, 'calls onError with params missing error'); }); @@ -228,6 +231,7 @@ module('Integration | Component | auth jwt', function(hooks) { 'message', buildMessage({ data: { + source: 'oidc-callback', path: 'foo', state: 'state', code: 'code', @@ -253,6 +257,7 @@ module('Integration | Component | auth jwt', function(hooks) { buildMessage({ origin: 'http://hackerz.com', data: { + source: 'oidc-callback', path: 'foo', state: 'state', code: 'code', @@ -277,6 +282,7 @@ module('Integration | Component | auth jwt', function(hooks) { buildMessage({ isTrusted: false, data: { + source: 'oidc-callback', path: 'foo', state: 'state', code: 'code', diff --git a/ui/tests/pages/components/console/ui-panel.js b/ui/tests/pages/components/console/ui-panel.js index 3623e2188..bc6ee1f02 100644 --- a/ui/tests/pages/components/console/ui-panel.js +++ b/ui/tests/pages/components/console/ui-panel.js @@ -1,5 +1,6 @@ import { text, triggerable, clickable, collection, fillable, value, isPresent } from 'ember-cli-page-object'; import { getter } from 'ember-cli-page-object/macros'; +import { settled } from '@ember/test-helpers'; import keys from 'vault/lib/keycodes'; @@ -45,6 +46,7 @@ export default { for (let command of toExecute) { await this.consoleInput(command); await this.enter(); + await settled(); } }, }; diff --git a/ui/tests/unit/components/auth-jwt-test.js b/ui/tests/unit/components/auth-jwt-test.js new file mode 100644 index 000000000..9bfa50b04 --- /dev/null +++ b/ui/tests/unit/components/auth-jwt-test.js @@ -0,0 +1,65 @@ +import { module, test } from 'qunit'; +import { setupTest } from 'ember-qunit'; +import EmberObject from '@ember/object'; +import Evented from '@ember/object/evented'; +import sinon from 'sinon'; +import { run } from '@ember/runloop'; + +const mockWindow = EmberObject.extend(Evented, { + origin: 'http://localhost:4200', +}); + +module('Unit | Component | auth-jwt', function(hooks) { + setupTest(hooks); + + hooks.beforeEach(function() { + this.component = this.owner.lookup('component:auth-jwt'); + this.component.set('window', mockWindow.create()); + this.errorSpy = sinon.spy(this.component, 'handleOIDCError'); + }); + + test('it should handle error for cross origin messages while waiting for oidc callback', async function(assert) { + assert.expect(1); + this.component.prepareForOIDC.perform(mockWindow.create()); + this.component.window.trigger('message', { origin: 'http://anotherdomain.com', isTrusted: true }); + assert.ok(this.errorSpy.calledOnce, 'Error handled from cross origin window message event'); + run.cancelTimers(); + }); + + test('it should handle error for untrusted messages while waiting for oidc callback', async function(assert) { + assert.expect(1); + this.component.prepareForOIDC.perform(mockWindow.create()); + this.component.window.trigger('message', { origin: 'http://localhost:4200', isTrusted: false }); + assert.ok(this.errorSpy.calledOnce, 'Error handled from untrusted window message event'); + run.cancelTimers(); + }); + // test case for https://github.com/hashicorp/vault/issues/12436 + test('it should ignore messages sent from outside the app while waiting for oidc callback', async function(assert) { + assert.expect(2); + this.component.prepareForOIDC.perform(mockWindow.create()); + const message = { + origin: 'http://localhost:4200', + isTrusted: true, + data: { + namespace: 'foobar', + path: '/foo/bar', + state: 'authorized', + code: 204, + }, + }; + + this.component.window.trigger('message', message); + message.data.source = 'foo-bar'; + this.component.window.trigger('message', message); + message.data.source = 'oidc-callback'; + this.component.window.trigger('message', message); + + assert.ok(this.errorSpy.notCalled, 'Error handler not triggered while waiting for oidc callback message'); + assert.equal( + this.component.exchangeOIDC.performCount, + 1, + 'exchangeOIDC method fires when oidc callback message is received' + ); + run.cancelTimers(); + }); +});