From ff121c310f5bdee3a2764e282f507a695a6aa3e0 Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Wed, 8 Feb 2023 11:53:49 -0600 Subject: [PATCH 1/3] Remove later runloop from auth-jwt --- ui/app/components/auth-jwt.js | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/ui/app/components/auth-jwt.js b/ui/app/components/auth-jwt.js index 0e5b53a1d3ec..41110ebbf259 100644 --- a/ui/app/components/auth-jwt.js +++ b/ui/app/components/auth-jwt.js @@ -2,7 +2,6 @@ import Ember from 'ember'; import { inject as service } from '@ember/service'; // ARG NOTE: Once you remove outer-html after glimmerizing you can remove the outer-html component import Component from './outer-html'; -import { later } from '@ember/runloop'; import { task, timeout, waitForEvent } from 'ember-concurrency'; import { computed } from '@ember/object'; import { waitFor } from '@ember/test-waiters'; @@ -76,6 +75,17 @@ export default Component.extend({ }) ).restartable(), + cancelLogin(oidcWindow, errorMessage) { + this.closeWindow(oidcWindow); + this.handleOIDCError(errorMessage); + }, + + closeWindow(oidcWindow) { + this.watchPopup.cancelAll(); + this.watchCurrent.cancelAll(); + oidcWindow.close(); + }, + handleOIDCError(err) { this.onLoading(false); this.prepareForOIDC.cancelAll(); @@ -94,10 +104,7 @@ export default Component.extend({ // 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') { + if (event.data.source === 'oidc-callback' && event.isTrusted && event.origin === thisWindow.origin) { return this.exchangeOIDC.perform(event.data, oidcWindow); } // continue to wait for the correct message @@ -119,12 +126,6 @@ export default Component.extend({ oidcWindow.close(); }), - closeWindow(oidcWindow) { - this.watchPopup.cancelAll(); - this.watchCurrent.cancelAll(); - oidcWindow.close(); - }, - exchangeOIDC: task(function* (oidcState, oidcWindow) { if (oidcState === null || oidcState === undefined) { return; @@ -145,12 +146,8 @@ export default Component.extend({ } } - // defer closing of the window, but continue executing the task - later(() => { - this.closeWindow(oidcWindow); - }, WAIT_TIME); if (!path || !state || !code) { - return this.handleOIDCError(ERROR_MISSING_PARAMS); + return this.cancelLogin(oidcWindow, ERROR_MISSING_PARAMS); } const adapter = this.store.adapterFor('auth-method'); this.onNamespace(namespace); @@ -159,8 +156,11 @@ export default Component.extend({ // and submit auth form try { resp = yield adapter.exchangeOIDC(path, state, code); + this.closeWindow(oidcWindow); } catch (e) { - return this.handleOIDCError(e); + // If there was an error on Vault's end, close the popup + // and show the error on the login screen + return this.cancelLogin(oidcWindow, e); } yield this.onSubmit(null, null, resp.auth.client_token); }), From 7f28b9b3ffd20a5dfc74bca2221754819cc70e55 Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Wed, 8 Feb 2023 12:02:44 -0600 Subject: [PATCH 2/3] Add changelog --- changelog/19071.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/19071.txt diff --git a/changelog/19071.txt b/changelog/19071.txt new file mode 100644 index 000000000000..ca988dbebbce --- /dev/null +++ b/changelog/19071.txt @@ -0,0 +1,3 @@ +```release-note:bug +ui: Fix bug where logging in via OIDC fails if browser is in fullscreen mode +``` From cec1ff784f646efa1249aa41630d7b84002eeeb9 Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Wed, 8 Feb 2023 13:46:01 -0600 Subject: [PATCH 3/3] Update auth-jwt tests --- ui/tests/unit/components/auth-jwt-test.js | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/ui/tests/unit/components/auth-jwt-test.js b/ui/tests/unit/components/auth-jwt-test.js index e6cc5e73c874..410fdce5a675 100644 --- a/ui/tests/unit/components/auth-jwt-test.js +++ b/ui/tests/unit/components/auth-jwt-test.js @@ -7,6 +7,7 @@ import { _cancelTimers as cancelTimers } from '@ember/runloop'; const mockWindow = EmberObject.extend(Evented, { origin: 'http://localhost:4200', + close: () => {}, }); module('Unit | Component | auth-jwt', function (hooks) { @@ -18,21 +19,25 @@ module('Unit | Component | auth-jwt', function (hooks) { 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); + test('it should ignore messages from cross origin windows while waiting for oidc callback', async function (assert) { + assert.expect(2); 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'); + + assert.ok(this.errorSpy.notCalled, 'Error handler not triggered while waiting for oidc callback message'); + assert.strictEqual(this.component.exchangeOIDC.performCount, 0, 'exchangeOIDC method not fired'); cancelTimers(); }); - test('it should handle error for untrusted messages while waiting for oidc callback', async function (assert) { - assert.expect(1); + test('it should ignore untrusted messages while waiting for oidc callback', async function (assert) { + assert.expect(2); 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'); + assert.ok(this.errorSpy.notCalled, 'Error handler not triggered while waiting for oidc callback message'); + assert.strictEqual(this.component.exchangeOIDC.performCount, 0, 'exchangeOIDC method not fired'); 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);