mirror of
https://github.com/TryGhost/Ghost.git
synced 2025-02-24 23:48:13 -05:00
🐛 prevent session sync issues with multiple tabs/refreshes (#772)
refs https://github.com/TryGhost/Ghost/issues/8616 - only refresh tokens on boot if we last refreshed more than 24hrs ago - this should prevent rapidly changing access/refresh tokens when opening new admin tabs or refreshing whilst other tabs are open - fix token refresh test which was testing it's own behaviour instead of the applications 🙈 This may not be the full solution to the session issues but it closes one potential culprit and should at least reduce token churn which can only help track down the real cause.
This commit is contained in:
parent
034f74a560
commit
0853b57244
5 changed files with 74 additions and 23 deletions
|
@ -6,6 +6,7 @@ import Route from 'ember-route';
|
||||||
import ShortcutsRoute from 'ghost-admin/mixins/shortcuts-route';
|
import ShortcutsRoute from 'ghost-admin/mixins/shortcuts-route';
|
||||||
import ctrlOrCmd from 'ghost-admin/utils/ctrl-or-cmd';
|
import ctrlOrCmd from 'ghost-admin/utils/ctrl-or-cmd';
|
||||||
import injectService from 'ember-service/inject';
|
import injectService from 'ember-service/inject';
|
||||||
|
import moment from 'moment';
|
||||||
import observer from 'ember-metal/observer';
|
import observer from 'ember-metal/observer';
|
||||||
import run from 'ember-runloop';
|
import run from 'ember-runloop';
|
||||||
import windowProxy from 'ghost-admin/utils/window-proxy';
|
import windowProxy from 'ghost-admin/utils/window-proxy';
|
||||||
|
@ -48,14 +49,21 @@ export default Route.extend(ApplicationRouteMixin, ShortcutsRoute, {
|
||||||
transition.send('loadServerNotifications');
|
transition.send('loadServerNotifications');
|
||||||
transition.send('checkForOutdatedDesktopApp');
|
transition.send('checkForOutdatedDesktopApp');
|
||||||
|
|
||||||
// trigger a background refresh of the access token to enable
|
// trigger a background token refresh to enable "infinite" sessions
|
||||||
// "infinite" sessions. We also trigger a logout if the refresh
|
// NOTE: we only do this if the last refresh was > 1 day ago to avoid
|
||||||
// token is invalid to prevent attackers with only the access token
|
// potential issues with multiple tabs and concurrent admin loads/refreshes.
|
||||||
// from loading the admin
|
// see https://github.com/TryGhost/Ghost/issues/8616
|
||||||
let session = this.get('session.session');
|
let session = this.get('session.session');
|
||||||
let authenticator = session._lookupAuthenticator(session.authenticator);
|
let expiresIn = session.get('authenticated.expires_in') * 1000;
|
||||||
if (authenticator && authenticator.onOnline) {
|
let expiresAt = session.get('authenticated.expires_at');
|
||||||
authenticator.onOnline();
|
let lastRefresh = moment(expiresAt - expiresIn);
|
||||||
|
let oneDayAgo = moment().subtract(1, 'day');
|
||||||
|
|
||||||
|
if (lastRefresh.isBefore(oneDayAgo)) {
|
||||||
|
let authenticator = session._lookupAuthenticator(session.authenticator);
|
||||||
|
if (authenticator && authenticator.onOnline) {
|
||||||
|
authenticator.onOnline();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
let featurePromise = this.get('feature').fetch().then(() => {
|
let featurePromise = this.get('feature').fetch().then(() => {
|
||||||
|
|
|
@ -15,16 +15,16 @@ export default function mockAuthentication(server) {
|
||||||
}
|
}
|
||||||
|
|
||||||
return {
|
return {
|
||||||
access_token: '5JhTdKI7PpoZv4ROsFoERc6wCHALKFH5jxozwOOAErmUzWrFNARuH1q01TYTKeZkPW7FmV5MJ2fU00pg9sm4jtH3Z1LjCf8D6nNqLYCfFb2YEKyuvG7zHj4jZqSYVodN2YTCkcHv6k8oJ54QXzNTLIDMlCevkOebm5OjxGiJpafMxncm043q9u1QhdU9eee3zouGRMVVp8zkKVoo5zlGMi3zvS2XDpx7xsfk8hKHpUgd7EDDQxmMueifWv7hv6n',
|
access_token: 'MirageAccessToken',
|
||||||
expires_in: 3600,
|
expires_in: 172800,
|
||||||
refresh_token: 'XP13eDjwV5mxOcrq1jkIY9idhdvN3R1Br5vxYpYIub2P5Hdc8pdWMOGmwFyoUshiEB62JWHTl8H1kACJR18Z8aMXbnk5orG28br2kmVgtVZKqOSoiiWrQoeKTqrRV0t7ua8uY5HdDUaKpnYKyOdpagsSPn3WEj8op4vHctGL3svOWOjZhq6F2XeVPMR7YsbiwBE8fjT3VhTB3KRlBtWZd1rE0Qo2EtSplWyjGKv1liAEiL0ndQoLeeSOCH4rTP7'
|
refresh_token: 'MirageRefreshToken'
|
||||||
};
|
};
|
||||||
} else {
|
} else {
|
||||||
// Password sign-in
|
// Password sign-in
|
||||||
return {
|
return {
|
||||||
access_token: '5JhTdKI7PpoZv4ROsFoERc6wCHALKFH5jxozwOOAErmUzWrFNARuH1q01TYTKeZkPW7FmV5MJ2fU00pg9sm4jtH3Z1LjCf8D6nNqLYCfFb2YEKyuvG7zHj4jZqSYVodN2YTCkcHv6k8oJ54QXzNTLIDMlCevkOebm5OjxGiJpafMxncm043q9u1QhdU9eee3zouGRMVVp8zkKVoo5zlGMi3zvS2XDpx7xsfk8hKHpUgd7EDDQxmMueifWv7hv6n',
|
access_token: 'MirageAccessToken',
|
||||||
expires_in: 3600,
|
expires_in: 172800,
|
||||||
refresh_token: 'XP13eDjwV5mxOcrq1jkIY9idhdvN3R1Br5vxYpYIub2P5Hdc8pdWMOGmwFyoUshiEB62JWHTl8H1kACJR18Z8aMXbnk5orG28br2kmVgtVZKqOSoiiWrQoeKTqrRV0t7ua8uY5HdDUaKpnYKyOdpagsSPn3WEj8op4vHctGL3svOWOjZhq6F2XeVPMR7YsbiwBE8fjT3VhTB3KRlBtWZd1rE0Qo2EtSplWyjGKv1liAEiL0ndQoLeeSOCH4rTP7',
|
refresh_token: 'MirageRefreshToken',
|
||||||
token_type: 'Bearer'
|
token_type: 'Bearer'
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
|
@ -48,12 +48,26 @@ describe('Acceptance: Authentication', function () {
|
||||||
server.create('user', {roles: [role], slug: 'test-user'});
|
server.create('user', {roles: [role], slug: 'test-user'});
|
||||||
});
|
});
|
||||||
|
|
||||||
it('refreshes app tokens on boot', async function () {
|
it('refreshes tokens on boot if last refreshed > 24hrs ago', async function () {
|
||||||
/* eslint-disable camelcase */
|
/* eslint-disable camelcase */
|
||||||
authenticateSession(application, {
|
// the tokens here don't matter, we're using the actual oauth
|
||||||
access_token: 'testAccessToken',
|
// authenticator so we get the tokens back from the mirage endpoint
|
||||||
refresh_token: 'refreshAccessToken'
|
await authenticateSession(application, {
|
||||||
|
access_token: 'access_token',
|
||||||
|
refresh_token: 'refresh_token'
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// authenticating the session above will trigger a token refresh
|
||||||
|
// request so we need to clear it to ensure we aren't testing the
|
||||||
|
// test behaviour instead of application behaviour
|
||||||
|
server.pretender.handledRequests = [];
|
||||||
|
|
||||||
|
// fake a longer session so it appears that we last refreshed > 24hrs ago
|
||||||
|
let {__container__: container} = application;
|
||||||
|
let {session} = container.lookup('service:session');
|
||||||
|
let newSession = session.get('content');
|
||||||
|
newSession.authenticated.expires_in = 172800 * 2;
|
||||||
|
session.get('store').persist(newSession);
|
||||||
/* eslint-enable camelcase */
|
/* eslint-enable camelcase */
|
||||||
|
|
||||||
await visit('/');
|
await visit('/');
|
||||||
|
@ -61,13 +75,36 @@ describe('Acceptance: Authentication', function () {
|
||||||
let requests = server.pretender.handledRequests;
|
let requests = server.pretender.handledRequests;
|
||||||
let refreshRequest = requests.findBy('url', '/ghost/api/v0.1/authentication/token');
|
let refreshRequest = requests.findBy('url', '/ghost/api/v0.1/authentication/token');
|
||||||
|
|
||||||
expect(refreshRequest).to.exist;
|
expect(refreshRequest, 'token refresh request').to.exist;
|
||||||
expect(refreshRequest.method, 'method').to.equal('POST');
|
expect(refreshRequest.method, 'method').to.equal('POST');
|
||||||
|
|
||||||
let requestBody = $.deparam(refreshRequest.requestBody);
|
let requestBody = $.deparam(refreshRequest.requestBody);
|
||||||
expect(requestBody.grant_type, 'grant_type').to.equal('password');
|
expect(requestBody.grant_type, 'grant_type').to.equal('refresh_token');
|
||||||
expect(requestBody.username.access_token, 'access_token').to.equal('testAccessToken');
|
expect(requestBody.refresh_token, 'refresh_token').to.equal('MirageRefreshToken');
|
||||||
expect(requestBody.username.refresh_token, 'refresh_token').to.equal('refreshAccessToken');
|
});
|
||||||
|
|
||||||
|
it('doesn\'t refresh tokens on boot if last refreshed < 24hrs ago', async function () {
|
||||||
|
/* eslint-disable camelcase */
|
||||||
|
// the tokens here don't matter, we're using the actual oauth
|
||||||
|
// authenticator so we get the tokens back from the mirage endpoint
|
||||||
|
await authenticateSession(application, {
|
||||||
|
access_token: 'access_token',
|
||||||
|
refresh_token: 'refresh_token'
|
||||||
|
});
|
||||||
|
/* eslint-enable camelcase */
|
||||||
|
|
||||||
|
// authenticating the session above will trigger a token refresh
|
||||||
|
// request so we need to clear it to ensure we aren't testing the
|
||||||
|
// test behaviour instead of application behaviour
|
||||||
|
server.pretender.handledRequests = [];
|
||||||
|
|
||||||
|
// we've only just refreshed tokens above so we should always be < 24hrs
|
||||||
|
await visit('/');
|
||||||
|
|
||||||
|
let requests = server.pretender.handledRequests;
|
||||||
|
let refreshRequest = requests.findBy('url', '/ghost/api/v0.1/authentication/token');
|
||||||
|
|
||||||
|
expect(refreshRequest, 'refresh request').to.not.exist;
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
|
@ -10,6 +10,7 @@ import {afterEach, beforeEach, describe, it} from 'mocha';
|
||||||
import {authenticateSession, invalidateSession} from 'ghost-admin/tests/helpers/ember-simple-auth';
|
import {authenticateSession, invalidateSession} from 'ghost-admin/tests/helpers/ember-simple-auth';
|
||||||
import {errorOverride, errorReset} from 'ghost-admin/tests/helpers/adapter-error';
|
import {errorOverride, errorReset} from 'ghost-admin/tests/helpers/adapter-error';
|
||||||
import {expect} from 'chai';
|
import {expect} from 'chai';
|
||||||
|
import {timeout} from 'ember-concurrency';
|
||||||
|
|
||||||
// Grabbed from keymaster's testing code because Ember's `keyEvent` helper
|
// Grabbed from keymaster's testing code because Ember's `keyEvent` helper
|
||||||
// is for some reason not triggering the events in a way that keymaster detects:
|
// is for some reason not triggering the events in a way that keymaster detects:
|
||||||
|
@ -183,6 +184,11 @@ describe('Acceptance: Settings - Tags', function () {
|
||||||
await fillIn('.tag-settings-pane input[name="name"]', 'New Tag');
|
await fillIn('.tag-settings-pane input[name="name"]', 'New Tag');
|
||||||
await triggerEvent('.tag-settings-pane input[name="name"]', 'blur');
|
await triggerEvent('.tag-settings-pane input[name="name"]', 'blur');
|
||||||
|
|
||||||
|
// extra timeout needed for FF on Linux - sometimes it doesn't update
|
||||||
|
// quick enough, especially on Travis, and an extra wait() call
|
||||||
|
// doesn't help
|
||||||
|
await timeout(100);
|
||||||
|
|
||||||
// it redirects to the new tag's URL
|
// it redirects to the new tag's URL
|
||||||
expect(currentURL(), 'URL after tag creation').to.equal('/settings/tags/new-tag');
|
expect(currentURL(), 'URL after tag creation').to.equal('/settings/tags/new-tag');
|
||||||
|
|
||||||
|
|
|
@ -58,9 +58,9 @@ describe('Acceptance: Signin', function() {
|
||||||
|
|
||||||
if (password === 'testpass') {
|
if (password === 'testpass') {
|
||||||
return {
|
return {
|
||||||
access_token: '5JhTdKI7PpoZv4ROsFoERc6wCHALKFH5jxozwOOAErmUzWrFNARuH1q01TYTKeZkPW7FmV5MJ2fU00pg9sm4jtH3Z1LjCf8D6nNqLYCfFb2YEKyuvG7zHj4jZqSYVodN2YTCkcHv6k8oJ54QXzNTLIDMlCevkOebm5OjxGiJpafMxncm043q9u1QhdU9eee3zouGRMVVp8zkKVoo5zlGMi3zvS2XDpx7xsfk8hKHpUgd7EDDQxmMueifWv7hv6n',
|
access_token: 'MirageAccessToken',
|
||||||
expires_in: 3600,
|
expires_in: 3600,
|
||||||
refresh_token: 'XP13eDjwV5mxOcrq1jkIY9idhdvN3R1Br5vxYpYIub2P5Hdc8pdWMOGmwFyoUshiEB62JWHTl8H1kACJR18Z8aMXbnk5orG28br2kmVgtVZKqOSoiiWrQoeKTqrRV0t7ua8uY5HdDUaKpnYKyOdpagsSPn3WEj8op4vHctGL3svOWOjZhq6F2XeVPMR7YsbiwBE8fjT3VhTB3KRlBtWZd1rE0Qo2EtSplWyjGKv1liAEiL0ndQoLeeSOCH4rTP7',
|
refresh_token: 'MirageRefreshToken',
|
||||||
token_type: 'Bearer'
|
token_type: 'Bearer'
|
||||||
};
|
};
|
||||||
} else {
|
} else {
|
||||||
|
|
Loading…
Add table
Reference in a new issue