UI/fix safari oidc login (#11884)

* use window.postMessage instead of localStorage on oidc callback
This commit is contained in:
Chelsea Shaw 2021-06-17 15:56:04 -05:00 committed by GitHub
parent e47be738b2
commit 565871f63c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 84 additions and 51 deletions

3
changelog/11884.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:bug
ui: fix oidc login with Safari
```

View File

@ -83,15 +83,19 @@ export default Component.extend({
}, },
prepareForOIDC: task(function*(oidcWindow) { prepareForOIDC: task(function*(oidcWindow) {
const thisWindow = this.getWindow();
// show the loading animation in the parent // show the loading animation in the parent
this.onLoading(true); this.onLoading(true);
// start watching the popup window and the current one // start watching the popup window and the current one
this.watchPopup.perform(oidcWindow); this.watchPopup.perform(oidcWindow);
this.watchCurrent.perform(oidcWindow); this.watchCurrent.perform(oidcWindow);
// and then wait for storage event to be fired from the popup // wait for message posted from popup
// window setting a value in localStorage when the callback route is loaded const event = yield waitForEvent(thisWindow, 'message');
let storageEvent = yield waitForEvent(this.getWindow(), 'storage'); if (event.origin === thisWindow.origin && event.isTrusted) {
this.exchangeOIDC.perform(storageEvent, oidcWindow); this.exchangeOIDC.perform(event.data, oidcWindow);
} else {
this.handleOIDCError();
}
}), }),
watchPopup: task(function*(oidcWindow) { watchPopup: task(function*(oidcWindow) {
@ -104,6 +108,7 @@ export default Component.extend({
}), }),
watchCurrent: task(function*(oidcWindow) { watchCurrent: task(function*(oidcWindow) {
// when user is about to change pages, close the popup window
yield waitForEvent(this.getWindow(), 'beforeunload'); yield waitForEvent(this.getWindow(), 'beforeunload');
oidcWindow.close(); oidcWindow.close();
}), }),
@ -114,20 +119,13 @@ export default Component.extend({
oidcWindow.close(); oidcWindow.close();
}, },
exchangeOIDC: task(function*(event, oidcWindow) { exchangeOIDC: task(function*(oidcState, 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'));
if (oidcState === null || oidcState === undefined) { if (oidcState === null || oidcState === undefined) {
return; return;
} }
this.onLoading(true); 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 } = oidcState;
let { namespace, path, state, code } = JSON.parse(oidcState);
this.getWindow().localStorage.removeItem('oidcState');
// The namespace can be either be passed as a query paramter, or be embedded // 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 // in the state param in the format `<state_id>,ns=<namespace>`. So if

View File

@ -10,7 +10,7 @@ export default Route.extend({
let { namespaceQueryParam: namespace } = this.paramsFor('vault.cluster'); let { namespaceQueryParam: namespace } = this.paramsFor('vault.cluster');
path = window.decodeURIComponent(path); path = window.decodeURIComponent(path);
let queryParams = { namespace, path, code, state }; let queryParams = { namespace, path, code, state };
window.localStorage.setItem('oidcState', JSON.stringify(queryParams)); window.opener.postMessage(queryParams, window.origin);
}, },
renderTemplate() { renderTemplate() {
this.render(this.templateName, { this.render(this.templateName, {

View File

@ -55,7 +55,7 @@
@roleName={{this.roleName}} @roleName={{this.roleName}}
@selectedAuthType={{this.selectedAuthBackend.type}} @selectedAuthType={{this.selectedAuthBackend.type}}
@selectedAuthPath={{or this.customPath this.selectedAuthBackend.id}} @selectedAuthPath={{or this.customPath this.selectedAuthBackend.id}}
@disabled={{authenticate.isRunning}} @disabled={{or authenticate.isRunning this.isLoading}}
> >
<AuthFormOptions <AuthFormOptions
@customPath={{this.customPath}} @customPath={{this.customPath}}
@ -110,7 +110,7 @@
<AlertInline <AlertInline
@paddingTop=true @paddingTop=true
@sizeSmall=true @sizeSmall=true
@type="info" @type="info"
@message="If login takes longer than usual, you may need to check your device for an MFA notification, or contact your administrator if login times out." @message="If login takes longer than usual, you may need to check your device for an MFA notification, or contact your administrator if login times out."
data-test-auth-message="push" data-test-auth-message="push"
/> />

View File

@ -15,6 +15,12 @@ import { ERROR_WINDOW_CLOSED, ERROR_MISSING_PARAMS, ERROR_JWT_LOGIN } from 'vaul
const component = create(form); const component = create(form);
const windows = []; const windows = [];
const buildMessage = opts => ({
isTrusted: true,
origin: 'https://my-vault.com',
data: {},
...opts,
});
const fakeWindow = EmberObject.extend(Evented, { const fakeWindow = EmberObject.extend(Evented, {
init() { init() {
this._super(...arguments); this._super(...arguments);
@ -29,12 +35,7 @@ const fakeWindow = EmberObject.extend(Evented, {
width: 500, width: 500,
}; };
}), }),
localStorage: computed(function() { origin: 'https://my-vault.com',
return {
getItem: sinon.stub(),
removeItem: sinon.stub(),
};
}),
closed: false, closed: false,
}); });
@ -70,7 +71,6 @@ const renderIt = async (context, path = 'jwt') => {
context.set('handler', sinon.spy(handler)); context.set('handler', sinon.spy(handler));
context.set('roleName', ''); context.set('roleName', '');
context.set('selectedAuthPath', path); context.set('selectedAuthPath', path);
await render(hbs` await render(hbs`
<AuthJwt <AuthJwt
@window={{window}} @window={{window}}
@ -135,7 +135,7 @@ module('Integration | Component | auth jwt', function(hooks) {
assert.equal(component.yieldContent, 'Hello!', 'yields properly'); 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 renderIt(this);
await settled(); await settled();
assert.ok(component.jwtPresent, 'renders jwt field'); assert.ok(component.jwtPresent, 'renders jwt field');
@ -203,7 +203,7 @@ module('Integration | Component | auth jwt', function(hooks) {
assert.equal(this.error, ERROR_WINDOW_CLOSED, 'calls onError with error string'); 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); await renderIt(this);
this.set('selectedAuthPath', 'foo'); this.set('selectedAuthPath', 'foo');
await component.role('test'); await component.role('test');
@ -211,26 +211,8 @@ module('Integration | Component | auth jwt', function(hooks) {
await waitUntil(() => { await waitUntil(() => {
return this.openSpy.calledOnce; return this.openSpy.calledOnce;
}); });
this.window.localStorage.getItem.returns(null); this.window.trigger('message', buildMessage({ data: { state: 'state', foo: 'bar' } }));
this.window.trigger('storage', { storageArea: this.window.localStorage });
run.cancelTimers(); run.cancelTimers();
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 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(JSON.stringify({}));
this.window.trigger('storage', { storageArea: this.window.localStorage });
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'); assert.equal(this.error, ERROR_MISSING_PARAMS, 'calls onError with params missing error');
}); });
@ -242,17 +224,67 @@ module('Integration | Component | auth jwt', function(hooks) {
await waitUntil(() => { await waitUntil(() => {
return this.openSpy.calledOnce; return this.openSpy.calledOnce;
}); });
this.window.localStorage.getItem.returns( this.window.trigger(
JSON.stringify({ 'message',
path: 'foo', buildMessage({
state: 'state', data: {
code: 'code', path: 'foo',
state: 'state',
code: 'code',
},
}) })
); );
this.window.trigger('storage', { storageArea: this.window.localStorage });
await settled(); await settled();
assert.equal(this.selectedAuth, 'token', 'calls onSelectedAuth with token'); assert.equal(this.selectedAuth, 'token', 'calls onSelectedAuth with token');
assert.equal(this.token, 'token', 'calls onToken with token'); assert.equal(this.token, 'token', 'calls onToken with token');
assert.ok(this.handler.calledOnce, 'calls the onSubmit handler'); 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();
await settled();
assert.notOk(this.handler.called, 'should not call the submit handler');
});
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.trigger(
'message',
buildMessage({
isTrusted: false,
data: {
path: 'foo',
state: 'state',
code: 'code',
},
})
);
run.cancelTimers();
await settled();
assert.notOk(this.handler.called, 'should not call the submit handler');
});
}); });