diff --git a/core/server/api/authentication.js b/core/server/api/authentication.js index cc85eec2d1d8..dcda4186f511 100644 --- a/core/server/api/authentication.js +++ b/core/server/api/authentication.js @@ -132,38 +132,6 @@ function setupTasks(setupData) { * **See:** [API Methods](index.js.html#api%20methods) */ authentication = { - /** - * Generate a pair of tokens - */ - createTokens: function createTokens(data, options) { - var localAccessToken = globalUtils.uid(191), - localRefreshToken = globalUtils.uid(191), - accessExpires = Date.now() + globalUtils.ONE_MONTH_MS, - refreshExpires = Date.now() + globalUtils.SIX_MONTH_MS, - client = options.context.client_id, - user = options.context.user; - - return models.Accesstoken.add({ - token: localAccessToken, - user_id: user, - client_id: client, - expires: accessExpires - }).then(function () { - return models.Refreshtoken.add({ - token: localRefreshToken, - user_id: user, - client_id: client, - expires: refreshExpires - }); - }).then(function () { - return { - access_token: localAccessToken, - refresh_token: localRefreshToken, - expires_in: globalUtils.ONE_MONTH_S - }; - }); - }, - /** * @description generate a reset token for a given email address * @param {Object} object diff --git a/core/server/auth/authenticate.js b/core/server/auth/authenticate.js index 3a8b7f0f301d..1f76e944009a 100644 --- a/core/server/auth/authenticate.js +++ b/core/server/auth/authenticate.js @@ -1,38 +1,16 @@ var passport = require('passport'), + authUtils = require('./utils'), errors = require('../errors'), models = require('../models'), events = require('../events'), i18n = require('../i18n'), authenticate; -function isBearerAutorizationHeader(req) { - var parts, - scheme, - credentials; - - if (req.headers && req.headers.authorization) { - parts = req.headers.authorization.split(' '); - } else if (req.query && req.query.access_token) { - return true; - } else { - return false; - } - - if (parts.length === 2) { - scheme = parts[0]; - credentials = parts[1]; - if (/^Bearer$/i.test(scheme)) { - return true; - } - } - return false; -} - authenticate = { // ### Authenticate Client Middleware authenticateClient: function authenticateClient(req, res, next) { // skip client authentication if bearer token is present - if (isBearerAutorizationHeader(req)) { + if (authUtils.getBearerAutorizationToken(req)) { return next(); } @@ -92,7 +70,7 @@ authenticate = { events.emit('user.authenticated', user); return next(null, user, info); - } else if (isBearerAutorizationHeader(req)) { + } else if (authUtils.getBearerAutorizationToken(req)) { return next(new errors.UnauthorizedError({ message: i18n.t('errors.middleware.auth.accessDenied') })); diff --git a/core/server/auth/oauth.js b/core/server/auth/oauth.js index 7588283c0f0a..044475ffee45 100644 --- a/core/server/auth/oauth.js +++ b/core/server/auth/oauth.js @@ -1,9 +1,8 @@ var oauth2orize = require('oauth2orize'), passport = require('passport'), models = require('../models'), - utils = require('../utils'), errors = require('../errors'), - authenticationAPI = require('../api/authentication'), + authUtils = require('./utils'), spamPrevention = require('../middleware/api/spam-prevention'), i18n = require('../i18n'), oauthServer, @@ -15,23 +14,18 @@ function exchangeRefreshToken(client, refreshToken, scope, body, authInfo, done) if (!model) { return done(new errors.NoPermissionError({message: i18n.t('errors.middleware.oauth.invalidRefreshToken')}), false); } else { - var token = model.toJSON(), - accessToken = utils.uid(191), - accessExpires = Date.now() + utils.ONE_MONTH_MS, - refreshExpires = Date.now() + utils.SIX_MONTH_MS; + var token = model.toJSON(); if (token.expires > Date.now()) { spamPrevention.userLogin.reset(authInfo.ip, body.refresh_token + 'login'); - models.Accesstoken.add({ - token: accessToken, - user_id: token.user_id, - client_id: token.client_id, - expires: accessExpires - }).then(function then() { - return models.Refreshtoken.edit({expires: refreshExpires}, {id: token.id}); - }).then(function then() { - return done(null, accessToken, {expires_in: utils.ONE_MONTH_S}); + authUtils.createTokens({ + clientId: token.client_id, + userId: token.user_id, + oldAccessToken: authInfo.accessToken, + oldRefreshToken: refreshToken + }).then(function (response) { + return done(null, response.access_token, {expires_in: response.expires_in}); }).catch(function handleError(error) { return done(error, false); }); @@ -54,7 +48,10 @@ function exchangePassword(client, username, password, scope, body, authInfo, don // Validate the user return models.User.check({email: username, password: password}) .then(function then(user) { - return authenticationAPI.createTokens({}, {context: {client_id: client.id, user: user.id}}); + return authUtils.createTokens({ + clientId: client.id, + userId: user.id + }); }) .then(function then(response) { spamPrevention.userLogin.reset(authInfo.ip, username + 'login'); @@ -72,6 +69,7 @@ function exchangeAuthorizationCode(req, res, next) { message: i18n.t('errors.middleware.auth.accessDenied') })); } + req.query.code = req.body.authorizationCode; passport.authenticate('ghost', {session: false, failWithError: false}, function authenticate(err, user) { @@ -89,17 +87,18 @@ function exchangeAuthorizationCode(req, res, next) { spamPrevention.userLogin.reset(req.authInfo.ip, req.body.authorizationCode + 'login'); - authenticationAPI.createTokens({}, {context: {client_id: req.client.id, user: user.id}}) - .then(function then(response) { - res.json({ - access_token: response.access_token, - refresh_token: response.refresh_token, - expires_in: response.expires_in - }); - }) - .catch(function (err) { - next(err); + authUtils.createTokens({ + clientId: req.client.id, + userId: user.id + }).then(function then(response) { + res.json({ + access_token: response.access_token, + refresh_token: response.refresh_token, + expires_in: response.expires_in }); + }).catch(function (err) { + next(err); + }); })(req, res, next); } @@ -156,7 +155,8 @@ oauth = { * Important: only used for resetting the brute count (access to req.ip) */ req.authInfo = { - ip: req.ip + ip: req.ip, + accessToken: authUtils.getBearerAutorizationToken(req) }; return oauthServer.token()(req, res, next); diff --git a/core/server/auth/utils.js b/core/server/auth/utils.js new file mode 100644 index 000000000000..8d227a46e241 --- /dev/null +++ b/core/server/auth/utils.js @@ -0,0 +1,116 @@ +var Promise = require('bluebird'), + _ = require('lodash'), + debug = require('ghost-ignition').debug('auth:utils'), + models = require('../models'), + globalUtils = require('../utils'), + knex = require('../data/db').knex, + _private = {}; + +/** + * The initial idea was to delete all old tokens connected to a user and a client. + * But if multiple browsers/apps are using the same client, we would log out them out. + * So the idea is to always decrease the expiry of the old access token if available. + * This access token auto expires and get's cleaned up on bootstrap (see oauth.js). + */ +_private.decreaseOldAccessTokenExpiry = function decreaseOldAccessTokenExpiry(data, options) { + debug('decreaseOldAccessTokenExpiry', data, options); + + if (!data.token) { + return Promise.resolve(); + } + + return models.Accesstoken.findOne(data, options) + .then(function (oldAccessToken) { + if (!oldAccessToken) { + return Promise.resolve(); + } + + return models.Accesstoken.edit({ + expires: Date.now() + globalUtils.FIVE_MINUTES_MS + }, _.merge({id: oldAccessToken.id}, options)); + }); +}; + +_private.destroyOldRefreshToken = function destroyOldRefreshToken(options) { + debug('destroyOldRefreshToken', options); + + if (!options.token) { + return Promise.resolve(); + } + + return models.Refreshtoken.destroyByToken(options); +}; + +/** + * A user can have one token per client at a time. + * If the user requests a new pair of tokens, we decrease the expiry of the old access token + * and re-add the refresh token (this happens because this function is used for 3 different cases). + * If the operation fails in between, the user can still use e.g. the refresh token and try again. + */ +module.exports.createTokens = function createTokens(options) { + options = options || {}; + debug('createTokens'); + + var oldAccessToken = options.oldAccessToken, + oldRefreshToken = options.oldRefreshToken, + newAccessToken = globalUtils.uid(191), + newRefreshToken = oldRefreshToken || globalUtils.uid(191), + accessExpires = Date.now() + globalUtils.ONE_MONTH_MS, + refreshExpires = Date.now() + globalUtils.SIX_MONTH_MS, + clientId = options.clientId, + userId = options.userId, + modelOptions; + + return knex.transaction(function (transaction) { + modelOptions = {transacting: transaction}; + + return _private.decreaseOldAccessTokenExpiry({token: oldAccessToken}, modelOptions) + .then(function () { + return _private.destroyOldRefreshToken(_.merge({ + token: oldRefreshToken + }, modelOptions)); + }) + .then(function () { + return models.Accesstoken.add({ + token: newAccessToken, + user_id: userId, + client_id: clientId, + expires: accessExpires + }, modelOptions); + }) + .then(function () { + return models.Refreshtoken.add({ + token: newRefreshToken, + user_id: userId, + client_id: clientId, + expires: refreshExpires + }, modelOptions); + }) + .then(function () { + return { + access_token: newAccessToken, + refresh_token: newRefreshToken, + expires_in: globalUtils.ONE_MONTH_S + }; + }); + }); +}; + +module.exports.getBearerAutorizationToken = function (req) { + var parts, + scheme, + token; + + if (req.headers && req.headers.authorization) { + parts = req.headers.authorization.split(' '); + scheme = parts[0]; + + if (/^Bearer$/i.test(scheme)) { + token = parts[1]; + } + } else if (req.query && req.query.access_token) { + token = req.query.access_token; + } + + return token; +}; diff --git a/core/server/models/base/token.js b/core/server/models/base/token.js index 338082f29616..c779159a59c5 100644 --- a/core/server/models/base/token.js +++ b/core/server/models/base/token.js @@ -32,6 +32,7 @@ Basetoken = ghostBookshelf.Model.extend({ return collection.invokeThen('destroy', options); }); }, + /** * ### destroyByUser * @param {[type]} options has context and id. Context is the user doing the destroy, id is the user to destroy diff --git a/core/server/utils/index.js b/core/server/utils/index.js index 0d83df01c903..ccf3964f1a83 100644 --- a/core/server/utils/index.js +++ b/core/server/utils/index.js @@ -24,6 +24,7 @@ utils = { ONE_MONTH_S: 2628000, SIX_MONTH_S: 15768000, ONE_YEAR_S: 31536000, + FIVE_MINUTES_MS: 300000, ONE_HOUR_MS: 3600000, ONE_DAY_MS: 86400000, ONE_WEEK_MS: 604800000, diff --git a/core/test/functional/routes/api/authentication_spec.js b/core/test/functional/routes/api/authentication_spec.js index 81adc646c13c..8338d1cecd65 100644 --- a/core/test/functional/routes/api/authentication_spec.js +++ b/core/test/functional/routes/api/authentication_spec.js @@ -1,5 +1,6 @@ var supertest = require('supertest'), should = require('should'), + moment = require('moment'), testUtils = require('../../../utils'), user = testUtils.DataGenerator.forModel.users[0], userForKnex = testUtils.DataGenerator.forKnex.users[0], @@ -126,7 +127,8 @@ describe('Authentication API', function () { password: user.password, client_id: 'ghost-admin', client_secret: 'not_available' - }).expect('Content-Type', /json/) + }) + .expect('Content-Type', /json/) // TODO: make it possible to override oauth2orize's header so that this is consistent .expect('Cache-Control', 'no-store') .expect(200) @@ -137,26 +139,41 @@ describe('Authentication API', function () { var refreshToken = res.body.refresh_token; - request.post(testUtils.API.getApiQuery('authentication/token')) - .set('Origin', config.get('url')) - .send({ - grant_type: 'refresh_token', - refresh_token: refreshToken, - client_id: 'ghost-admin', - client_secret: 'not_available' - }).expect('Content-Type', /json/) - // TODO: make it possible to override oauth2orize's header so that this is consistent - .expect('Cache-Control', 'no-store') - .expect(200) - .end(function (err, res) { - if (err) { - return done(err); - } - var jsonResponse = res.body; - should.exist(jsonResponse.access_token); - should.exist(jsonResponse.expires_in); - done(); - }); + models.Accesstoken.findOne({ + token: accesstoken + }).then(function (oldAccessToken) { + moment(oldAccessToken.get('expires')).diff(moment(), 'minutes').should.be.above(6); + + request.post(testUtils.API.getApiQuery('authentication/token')) + .set('Origin', config.get('url')) + .set('Authorization', 'Bearer ' + accesstoken) + .send({ + grant_type: 'refresh_token', + refresh_token: refreshToken, + client_id: 'ghost-admin', + client_secret: 'not_available' + }) + .expect('Content-Type', /json/) + // TODO: make it possible to override oauth2orize's header so that this is consistent + .expect('Cache-Control', 'no-store') + .expect(200) + .end(function (err, res) { + if (err) { + return done(err); + } + + var jsonResponse = res.body; + should.exist(jsonResponse.access_token); + should.exist(jsonResponse.expires_in); + + models.Accesstoken.findOne({ + token: accesstoken + }).then(function (oldAccessToken) { + moment(oldAccessToken.get('expires')).diff(moment(), 'minutes').should.be.below(6); + done(); + }); + }); + }); }); }); diff --git a/core/test/unit/auth/oauth_spec.js b/core/test/unit/auth/oauth_spec.js index aec0a7554b33..9c198c3efbb3 100644 --- a/core/test/unit/auth/oauth_spec.js +++ b/core/test/unit/auth/oauth_spec.js @@ -4,8 +4,8 @@ var sinon = require('sinon'), passport = require('passport'), testUtils = require('../../utils'), oAuth = require('../../../server/auth/oauth'), + authUtils = require('../../../server/auth/utils'), spamPrevention = require('../../../server/middleware/api/spam-prevention'), - api = require('../../../server/api'), errors = require('../../../server/errors'), models = require('../../../server/models'); @@ -62,11 +62,12 @@ describe('OAuth', function () { id: 1 })); - sandbox.stub(models.Accesstoken, 'add') - .returns(new Promise.resolve()); - - sandbox.stub(models.Refreshtoken, 'add') - .returns(new Promise.resolve()); + sandbox.stub(authUtils, 'createTokens') + .returns(new Promise.resolve({ + access_token: 'AT', + refresh_token: 'RT', + expires_in: Date.now() + 1000 + })); sandbox.stub(res, 'setHeader', function () {}); @@ -134,7 +135,7 @@ describe('OAuth', function () { id: 1 })); - sandbox.stub(models.Accesstoken, 'add') + sandbox.stub(authUtils, 'createTokens') .returns(new Promise.reject({ message: 'DB error' })); @@ -177,11 +178,12 @@ describe('OAuth', function () { } })); - sandbox.stub(models.Accesstoken, 'add') - .returns(new Promise.resolve()); - - sandbox.stub(models.Refreshtoken, 'edit') - .returns(new Promise.resolve()); + sandbox.stub(authUtils, 'createTokens') + .returns(new Promise.resolve({ + access_token: 'AT', + refresh_token: 'RT', + expires_in: Date.now() + 1000 + })); sandbox.stub(res, 'setHeader', function () {}); @@ -271,7 +273,7 @@ describe('OAuth', function () { } })); - sandbox.stub(models.Accesstoken, 'add') + sandbox.stub(authUtils, 'createTokens') .returns(new Promise.reject({ message: 'DB error' })); @@ -314,11 +316,12 @@ describe('OAuth', function () { done(); }; - sandbox.stub(api.authentication, 'createTokens').returns(Promise.resolve({ - access_token: 'access-token', - refresh_token: 'refresh-token', - expires_in: 10 - })); + sandbox.stub(authUtils, 'createTokens') + .returns(new Promise.resolve({ + access_token: 'access-token', + refresh_token: 'refresh-token', + expires_in: 10 + })); sandbox.stub(passport, 'authenticate', function (name, options, onSuccess) { return function () {