Skip to content

Commit

Permalink
OIDC Auth Bug (#13133) (#13218)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
zofskeez authored Nov 18, 2021
1 parent 0079130 commit 0d87606
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 8 deletions.
3 changes: 3 additions & 0 deletions changelog/13133.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
ui: Fixes issue with OIDC auth workflow when using MetaMask Chrome extension
```
18 changes: 12 additions & 6 deletions ui/app/components/auth-jwt.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}),

Expand Down
3 changes: 2 additions & 1 deletion ui/app/routes/vault/cluster/oidc-callback.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
8 changes: 7 additions & 1 deletion ui/tests/integration/components/auth-jwt-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
Expand All @@ -228,6 +231,7 @@ module('Integration | Component | auth jwt', function(hooks) {
'message',
buildMessage({
data: {
source: 'oidc-callback',
path: 'foo',
state: 'state',
code: 'code',
Expand All @@ -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',
Expand All @@ -277,6 +282,7 @@ module('Integration | Component | auth jwt', function(hooks) {
buildMessage({
isTrusted: false,
data: {
source: 'oidc-callback',
path: 'foo',
state: 'state',
code: 'code',
Expand Down
2 changes: 2 additions & 0 deletions ui/tests/pages/components/console/ui-panel.js
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -45,6 +46,7 @@ export default {
for (let command of toExecute) {
await this.consoleInput(command);
await this.enter();
await settled();
}
},
};
65 changes: 65 additions & 0 deletions ui/tests/unit/components/auth-jwt-test.js
Original file line number Diff line number Diff line change
@@ -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();
});
});

0 comments on commit 0d87606

Please sign in to comment.