Skip to content

Commit

Permalink
UI/fix safari oidc login (#11884) (#11905)
Browse files Browse the repository at this point in the history
* use window.postMessage instead of localStorage on oidc callback
  • Loading branch information
hashishaw authored Jun 22, 2021
1 parent 04db388 commit f8ba333
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 45 deletions.
3 changes: 3 additions & 0 deletions changelog/11884.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
ui: fix oidc login with Safari
```
26 changes: 12 additions & 14 deletions ui/app/components/auth-jwt.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,19 @@ export default Component.extend({
},

prepareForOIDC: task(function*(oidcWindow) {
const thisWindow = this.getWindow();
// show the loading animation in the parent
this.onLoading(true);
// start watching the popup window and the current one
this.watchPopup.perform(oidcWindow);
this.watchCurrent.perform(oidcWindow);
// and then wait for storage event to be fired from the popup
// window setting a value in localStorage when the callback route is loaded
let storageEvent = yield waitForEvent(this.getWindow(), 'storage');
this.exchangeOIDC.perform(storageEvent, 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();
}
}),

watchPopup: task(function*(oidcWindow) {
Expand All @@ -103,6 +107,7 @@ export default Component.extend({
}),

watchCurrent: task(function*(oidcWindow) {
// when user is about to change pages, close the popup window
yield waitForEvent(this.getWindow(), 'beforeunload');
oidcWindow.close();
}),
Expand All @@ -113,20 +118,13 @@ export default Component.extend({
oidcWindow.close();
},

exchangeOIDC: task(function*(event, oidcWindow) {
// in non-incognito mode we need to use a timeout because it takes time before oidcState is written to local storage.
let oidcState = Ember.testing
? event.storageArea.getItem('oidcState')
: yield timeout(1000).then(() => event.storageArea.getItem('oidcState'));

exchangeOIDC: task(function*(oidcState, oidcWindow) {
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(oidcState);
this.getWindow().localStorage.removeItem('oidcState');

let { namespace, path, state, code } = oidcState;

// The namespace can be either be passed as a query paramter, or be embedded
// in the state param in the format `<state_id>,ns=<namespace>`. So if
Expand Down
2 changes: 1 addition & 1 deletion ui/app/routes/vault/cluster/oidc-callback.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export default Route.extend({
let { namespaceQueryParam: namespace } = this.paramsFor('vault.cluster');
path = window.decodeURIComponent(path);
let queryParams = { namespace, path, code, state };
window.localStorage.setItem('oidcState', JSON.stringify(queryParams));
window.opener.postMessage(queryParams, window.origin);
},
renderTemplate() {
this.render(this.templateName, {
Expand Down
2 changes: 1 addition & 1 deletion ui/app/templates/components/auth-form.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
@roleName={{this.roleName}}
@selectedAuthType={{this.selectedAuthBackend.type}}
@selectedAuthPath={{or this.customPath this.selectedAuthBackend.id}}
@disabled={{authenticate.isRunning}}
@disabled={{or authenticate.isRunning this.isLoading}}
>
<AuthFormOptions
@customPath={{this.customPath}}
Expand Down
90 changes: 61 additions & 29 deletions ui/tests/integration/components/auth-jwt-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ import { ERROR_WINDOW_CLOSED, ERROR_MISSING_PARAMS, ERROR_JWT_LOGIN } from 'vaul

const component = create(form);
const windows = [];
const buildMessage = opts => ({
isTrusted: true,
origin: 'https://my-vault.com',
data: {},
...opts,
});
const fakeWindow = EmberObject.extend(Evented, {
init() {
this._super(...arguments);
Expand All @@ -29,12 +35,7 @@ const fakeWindow = EmberObject.extend(Evented, {
width: 500,
};
}),
localStorage: computed(function() {
return {
getItem: sinon.stub(),
removeItem: sinon.stub(),
};
}),
origin: 'https://my-vault.com',
closed: false,
});

Expand Down Expand Up @@ -70,7 +71,6 @@ const renderIt = async (context, path = 'jwt') => {
context.set('handler', sinon.spy(handler));
context.set('roleName', '');
context.set('selectedAuthPath', path);

await render(hbs`
<AuthJwt
@window={{window}}
Expand Down Expand Up @@ -135,7 +135,7 @@ module('Integration | Component | auth jwt', function(hooks) {
assert.equal(component.yieldContent, 'Hello!', 'yields properly');
});

test('jwt: it renders', async function(assert) {
test('jwt: it renders and makes auth_url requests', async function(assert) {
await renderIt(this);
await settled();
assert.ok(component.jwtPresent, 'renders jwt field');
Expand Down Expand Up @@ -203,56 +203,88 @@ module('Integration | Component | auth jwt', function(hooks) {
assert.equal(this.error, ERROR_WINDOW_CLOSED, 'calls onError with error string');
});

test('oidc: storage event fires without state key', async function(assert) {
test('oidc: shows error when message posted 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.localStorage.getItem.returns(null);
this.window.trigger('storage', { storageArea: this.window.localStorage });
this.window.trigger('message', buildMessage({ data: { state: 'state', foo: 'bar' } }));
run.cancelTimers();
assert.ok(this.window.localStorage.getItem.calledOnce, 'calls getItem');
assert.notOk(this.window.localStorage.removeItem.called, 'never calls removeItem');
assert.equal(this.error, ERROR_MISSING_PARAMS, 'calls onError with params missing error');
});

test('oidc: storage event fires with state key, wrong 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.localStorage.getItem.returns(JSON.stringify({}));
this.window.trigger('storage', { storageArea: this.window.localStorage });
this.window.trigger(
'message',
buildMessage({
data: {
path: 'foo',
state: 'state',
code: 'code',
},
})
);
await settled();
assert.equal(this.selectedAuth, 'token', 'calls onSelectedAuth with token');
assert.equal(this.token, 'token', 'calls onToken with token');
assert.ok(this.handler.calledOnce, 'calls the onSubmit handler');
});

test('oidc: fails silently when event origin does not match window origin', 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(
'message',
buildMessage({
origin: 'http://hackerz.com',
data: {
path: 'foo',
state: 'state',
code: 'code',
},
})
);
run.cancelTimers();
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');
await settled();
assert.notOk(this.handler.called, 'should not call the submit handler');
});

test('oidc: storage event fires with state key, correct params', async function(assert) {
test('oidc: fails silently when event is not trusted', async function(assert) {
await renderIt(this);
this.set('selectedAuthPath', 'foo');
await component.role('test');
component.login();
await waitUntil(() => {
return this.openSpy.calledOnce;
});
this.window.localStorage.getItem.returns(
JSON.stringify({
path: 'foo',
state: 'state',
code: 'code',
this.window.trigger(
'message',
buildMessage({
isTrusted: false,
data: {
path: 'foo',
state: 'state',
code: 'code',
},
})
);
this.window.trigger('storage', { storageArea: this.window.localStorage });
run.cancelTimers();
await settled();
assert.equal(this.selectedAuth, 'token', 'calls onSelectedAuth with token');
assert.equal(this.token, 'token', 'calls onToken with token');
assert.ok(this.handler.calledOnce, 'calls the onSubmit handler');
assert.notOk(this.handler.called, 'should not call the submit handler');
});
});

0 comments on commit f8ba333

Please sign in to comment.