0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-02-24 23:48:13 -05:00

Fixed multiple 401s caused by token revocation requests

no issue
- disabled Ember Simple Auth's default token revocation
    - we trigger session invalidation on a 401 which means our token isn't valid so the revoke requests will also fail
- renamed application route's `invalidateSession` to `logout` in order to distinguish it from any ESA methods
    - added the token revocation requests to this action, we can be fairly sure at this point that the current tokens will be valid so the requests will succeed
- added check to `ajax.handleResponse` so that we don't invalidate the session for requests to external services
- removed pointless assertion from the ajax integration test
This commit is contained in:
Kevin Ansfield 2018-04-16 17:55:21 +01:00
parent f5fb674804
commit ae759d39ea
5 changed files with 47 additions and 20 deletions

View file

@ -26,9 +26,10 @@ export default Authenticator.extend({
return `${this.get('ghostPaths.apiRoot')}/authentication/token`; return `${this.get('ghostPaths.apiRoot')}/authentication/token`;
}), }),
serverTokenRevocationEndpoint: computed('ghostPaths.apiRoot', function () { // disable general token revocation because the requests will always 401
return `${this.get('ghostPaths.apiRoot')}/authentication/revoke`; // (revocation is triggered by invalid access token so it's already invalid)
}), // we have a separate logout procedure that sends revocation requests
serverTokenRevocationEndpoint: null,
makeRequest(url, data) { makeRequest(url, data) {
/* eslint-disable camelcase */ /* eslint-disable camelcase */

View file

@ -30,8 +30,10 @@ shortcuts.esc = {action: 'closeMenus', scope: 'default'};
shortcuts[`${ctrlOrCmd}+s`] = {action: 'save', scope: 'all'}; shortcuts[`${ctrlOrCmd}+s`] = {action: 'save', scope: 'all'};
export default Route.extend(ApplicationRouteMixin, ShortcutsRoute, { export default Route.extend(ApplicationRouteMixin, ShortcutsRoute, {
ajax: service(),
config: service(), config: service(),
feature: service(), feature: service(),
ghostPaths: service(),
notifications: service(), notifications: service(),
settings: service(), settings: service(),
tour: service(), tour: service(),
@ -106,10 +108,33 @@ export default Route.extend(ApplicationRouteMixin, ShortcutsRoute, {
this.send('loadServerNotifications', true); this.send('loadServerNotifications', true);
}, },
invalidateSession() { // this is only called by the `signout` route at present.
this.get('session').invalidate().catch((error) => { // it's separate to the normal ESA session invalidadition because it will
this.get('notifications').showAlert(error.message, {type: 'error', key: 'session.invalidate.failed'}); // actually send the token revocation requests whereas we have to avoid
// those most of the time because they will fail if we have invalid tokens
logout() {
let session = this.get('session');
// revoke keys on the server
if (session.get('isAuthenticated')) {
let auth = session.get('data.authenticated');
let revokeEndpoint = `${this.get('ghostPaths.apiRoot')}/authentication/revoke`;
let authenticator = session.get('session')._lookupAuthenticator(session.get('session.authenticator'));
let requests = [];
['refresh_token', 'access_token'].forEach((tokenType) => {
let data = {
token_type_hint: tokenType,
token: auth[tokenType]
};
authenticator.makeRequest(revokeEndpoint, data);
}); });
RSVP.all(requests).finally(() => {
// remove local keys and refresh
session.invalidate();
});
} else {
// remove local keys and refresh
session.invalidate();
}
}, },
authorizationFailed() { authorizationFailed() {

View file

@ -16,9 +16,9 @@ export default AuthenticatedRoute.extend(styleBody, {
afterModel(model, transition) { afterModel(model, transition) {
this.get('notifications').clearAll(); this.get('notifications').clearAll();
if (canInvoke(transition, 'send')) { if (canInvoke(transition, 'send')) {
transition.send('invalidateSession'); transition.send('logout');
} else { } else {
this.send('invalidateSession'); this.send('logout');
} }
} }
}); });

View file

@ -7,13 +7,15 @@ import {isArray as isEmberArray} from '@ember/array';
import {isNone} from '@ember/utils'; import {isNone} from '@ember/utils';
import {inject as service} from '@ember/service'; import {inject as service} from '@ember/service';
const JSONContentType = 'application/json'; const JSON_CONTENT_TYPE = 'application/json';
const GHOST_REQUEST = /\/ghost\/api\//;
const TOKEN_REQUEST = /authentication\/(?:token|ghost|revoke)/;
function isJSONContentType(header) { function isJSONContentType(header) {
if (!header || isNone(header)) { if (!header || isNone(header)) {
return false; return false;
} }
return header.indexOf(JSONContentType) === 0; return header.indexOf(JSON_CONTENT_TYPE) === 0;
} }
/* Version mismatch error */ /* Version mismatch error */
@ -135,8 +137,8 @@ let ajaxService = AjaxService.extend({
// and formats appropriately, we want to handle `application/json` the same // and formats appropriately, we want to handle `application/json` the same
_makeRequest(hash) { _makeRequest(hash) {
let isAuthenticated = this.get('session.isAuthenticated'); let isAuthenticated = this.get('session.isAuthenticated');
let isGhostRequest = hash.url.indexOf('/ghost/api/') !== -1; let isGhostRequest = GHOST_REQUEST.test(hash.url);
let isTokenRequest = isGhostRequest && hash.url.match(/authentication\/(?:token|ghost)/); let isTokenRequest = isGhostRequest && TOKEN_REQUEST.test(hash.url);
let tokenExpiry = this.get('session.authenticated.expires_at'); let tokenExpiry = this.get('session.authenticated.expires_at');
let isTokenExpired = tokenExpiry < (new Date()).getTime(); let isTokenExpired = tokenExpiry < (new Date()).getTime();
@ -167,7 +169,7 @@ let ajaxService = AjaxService.extend({
return this._super(...arguments); return this._super(...arguments);
}, },
handleResponse(status, headers, payload) { handleResponse(status, headers, payload, request) {
if (this.isVersionMismatchError(status, headers, payload)) { if (this.isVersionMismatchError(status, headers, payload)) {
return new VersionMismatchError(payload); return new VersionMismatchError(payload);
} else if (this.isServerUnreachableError(status, headers, payload)) { } else if (this.isServerUnreachableError(status, headers, payload)) {
@ -182,9 +184,11 @@ let ajaxService = AjaxService.extend({
return new ThemeValidationError(payload); return new ThemeValidationError(payload);
} }
// TODO: we may want to check that we are hitting our own API before let isGhostRequest = GHOST_REQUEST.test(request.url);
// logging the user out due to a 401 response let isAuthenticated = this.get('session.isAuthenticated');
if (this.isUnauthorizedError(status, headers, payload) && this.get('session.isAuthenticated')) { let isUnauthorized = this.isUnauthorizedError(status, headers, payload);
if (isAuthenticated && isGhostRequest && isUnauthorized) {
this.get('session').invalidate(); this.get('session').invalidate();
} }

View file

@ -177,8 +177,6 @@ describe('Integration: Service: ajax', function () {
/* eslint-disable camelcase */ /* eslint-disable camelcase */
describe('session handling', function () { describe('session handling', function () {
let successfulRequest = false;
let sessionStub = Service.extend({ let sessionStub = Service.extend({
isAuthenticated: true, isAuthenticated: true,
restoreCalled: false, restoreCalled: false,
@ -261,7 +259,6 @@ describe('Integration: Service: ajax', function () {
// TODO: fix the error return when a session restore fails // TODO: fix the error return when a session restore fails
// expect(isUnauthorizedError(error)).to.be.true; // expect(isUnauthorizedError(error)).to.be.true;
expect(ajax.get('session.restoreCalled'), 'restoreCalled').to.be.true; expect(ajax.get('session.restoreCalled'), 'restoreCalled').to.be.true;
expect(successfulRequest, 'successfulRequest').to.be.false;
expect(invalidateCalled, 'invalidateCalled').to.be.true; expect(invalidateCalled, 'invalidateCalled').to.be.true;
done(); done();
}); });