0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-01-20 22:42:53 -05:00

Added support for token session to /ghost (#11709)

no-issue

* Added default for getting origin of request

This function is used to attach the origin of the request to the
session, and later check that requests using the session are coming from
the same origin. This protects us against CSRF attacks as requests in
the browser MUST originate from the same origin on which the user
logged in.

Previously, when we could not determine the origin we would return
null, as a "safety" net.

This updates the function to use a secure and sensible default - which
is the origin of the Ghost-Admin application, and if that's not set -
the origin of the Ghost application.

This will make dealing with magic links simpler as you can not always
guaruntee the existence of these headers when visiting via a hyperlink

* Removed init fns and getters from session service

This simplifies the code here, making it easier to read and maintain

* Moved express-session initialisation to own file

This is complex enough that it deserves its own module

* Added createSessionFromToken to session service

* Wired up the createSessionFromToken middleware
This commit is contained in:
Fabien O'Carroll 2020-04-06 11:49:14 +02:00 committed by GitHub
parent 022a433e56
commit a701ee7023
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 75 additions and 122 deletions

View file

@ -0,0 +1,37 @@
const session = require('express-session');
const constants = require('../../../lib/constants');
const config = require('../../../config');
const settingsCache = require('../../settings/cache');
const models = require('../../../models');
const urlUtils = require('../../../lib/url-utils');
const SessionStore = require('./store');
const expressSessionMiddleware = session({
store: new SessionStore(models.Session),
secret: settingsCache.get('session_secret'),
resave: false,
saveUninitialized: false,
name: 'ghost-admin-api-session',
cookie: {
maxAge: constants.SIX_MONTH_MS,
httpOnly: true,
path: urlUtils.getSubdir() + '/ghost',
sameSite: 'lax',
secure: urlUtils.isSSL(config.get('url'))
}
});
exports.getSession = async function getSession(req, res) {
if (req.session) {
return req.session;
}
return new Promise((resolve, reject) => {
expressSessionMiddleware(req, res, function (err) {
if (err) {
return reject(err);
}
resolve(req.session);
});
});
};

View file

@ -1,18 +1,17 @@
const session = require('express-session'); const adapterManager = require('../../adapter-manager');
const constants = require('../../../lib/constants'); const createSessionService = require('@tryghost/session-service');
const config = require('../../../config'); const sessionFromToken = require('@tryghost/mw-session-from-token');
const settingsCache = require('../../settings/cache'); const createSessionMiddleware = require('./middleware');
const expressSession = require('./express-session');
const models = require('../../../models'); const models = require('../../../models');
const urlUtils = require('../../../lib/url-utils'); const urlUtils = require('../../../lib/url-utils');
const url = require('url'); const url = require('url');
const SessionService = require('@tryghost/session-service');
const SessionMiddleware = require('./middleware');
const SessionStore = require('./store');
function getOriginOfRequest(req) { function getOriginOfRequest(req) {
const origin = req.get('origin'); const origin = req.get('origin');
const referrer = req.get('referrer'); const referrer = req.get('referrer') || urlUtils.getAdminUrl() || urlUtils.getSiteUrl();
if (!origin && !referrer) { if (!origin && !referrer) {
return null; return null;
@ -29,95 +28,22 @@ function getOriginOfRequest(req) {
return null; return null;
} }
async function getSession(req, res) { const sessionService = createSessionService({
if (req.session) { getOriginOfRequest,
return req.session; getSession: expressSession.getSession,
findUserById({id}) {
return models.User.findOne({id});
} }
return new Promise((resolve, reject) => { });
expressSessionMiddleware(req, res, function (err) {
if (err) {
return reject(err);
}
resolve(req.session);
});
});
}
function findUserById({id}) { module.exports = createSessionMiddleware({sessionService});
return models.User.findOne({id});
}
let expressSessionMiddleware; const ssoAdapter = adapterManager.getAdapter('sso');
function initExpressSessionMiddleware() { // Looks funky but this is a "custom" piece of middleware
if (!expressSessionMiddleware) { module.exports.createSessionFromToken = sessionFromToken({
expressSessionMiddleware = session({ callNextWithError: false,
store: new SessionStore(models.Session), createSession: sessionService.createSessionForUser,
secret: settingsCache.get('session_secret'), findUserByLookup: ssoAdapter.getUserForIdentity,
resave: false, getLookupFromToken: ssoAdapter.getIdentityFromCredentials,
saveUninitialized: false, getTokenFromRequest: ssoAdapter.getRequestCredentials
name: 'ghost-admin-api-session', });
cookie: {
maxAge: constants.SIX_MONTH_MS,
httpOnly: true,
path: urlUtils.getSubdir() + '/ghost',
sameSite: 'lax',
secure: urlUtils.isSSL(config.get('url'))
}
});
}
}
let sessionService;
function initSessionService() {
if (!sessionService) {
if (!expressSessionMiddleware) {
initExpressSessionMiddleware();
}
sessionService = SessionService({
getOriginOfRequest,
getSession,
findUserById
});
}
}
let sessionMiddleware;
function initSessionMiddleware() {
if (!sessionMiddleware) {
if (!sessionService) {
initSessionService();
}
sessionMiddleware = SessionMiddleware({
sessionService
});
}
}
module.exports = {
get createSession() {
return this.middleware.createSession;
},
get destroySession() {
return this.middleware.destroySession;
},
get authenticate() {
return this.middleware.authenticate;
},
get service() {
if (!sessionService) {
initSessionService();
}
return sessionService;
},
get middleware() {
if (!sessionMiddleware) {
initSessionMiddleware();
}
return sessionMiddleware;
}
};

View file

@ -59,7 +59,7 @@ module.exports = function setupParentApp(options = {}) {
adminApp.enable('trust proxy'); // required to respect x-forwarded-proto in admin requests adminApp.enable('trust proxy'); // required to respect x-forwarded-proto in admin requests
adminApp.use('/ghost/api', require('./api')()); adminApp.use('/ghost/api', require('./api')());
adminApp.use('/ghost/.well-known', require('./well-known')()); adminApp.use('/ghost/.well-known', require('./well-known')());
adminApp.use('/ghost', require('./admin')()); adminApp.use('/ghost', require('../services/auth/session').createSessionFromToken, require('./admin')());
// TODO: remove {admin url}/content/* once we're sure the API is not returning relative asset URLs anywhere // TODO: remove {admin url}/content/* once we're sure the API is not returning relative asset URLs anywhere
// only register this route if the admin is separate so we're not overriding the {site}/content/* route // only register this route if the admin is separate so we're not overriding the {site}/content/* route

View file

@ -50,6 +50,7 @@
"@tryghost/kg-markdown-html-renderer": "1.0.2", "@tryghost/kg-markdown-html-renderer": "1.0.2",
"@tryghost/members-api": "0.18.0", "@tryghost/members-api": "0.18.0",
"@tryghost/members-ssr": "0.7.4", "@tryghost/members-ssr": "0.7.4",
"@tryghost/mw-session-from-token": "^0.1.0",
"@tryghost/session-service": "^0.1.0", "@tryghost/session-service": "^0.1.0",
"@tryghost/social-urls": "0.1.8", "@tryghost/social-urls": "0.1.8",
"@tryghost/string": "0.1.8", "@tryghost/string": "0.1.8",

View file

@ -5,7 +5,7 @@ const models = require('../../../../core/server/models');
const {UnauthorizedError} = require('../../../../core/server/lib/common/errors'); const {UnauthorizedError} = require('../../../../core/server/lib/common/errors');
const sessionController = require('../../../../core/server/api/canary/session'); const sessionController = require('../../../../core/server/api/canary/session');
const sessionServiceMiddleware = require('../../../../core/server/services/auth/session').middleware; const sessionServiceMiddleware = require('../../../../core/server/services/auth/session');
describe('Session controller', function () { describe('Session controller', function () {
before(function () { before(function () {

View file

@ -5,7 +5,7 @@ const models = require('../../../../core/server/models');
const {UnauthorizedError} = require('../../../../core/server/lib/common/errors'); const {UnauthorizedError} = require('../../../../core/server/lib/common/errors');
const sessionController = require('../../../../core/server/api/v2/session'); const sessionController = require('../../../../core/server/api/v2/session');
const sessionServiceMiddleware = require('../../../../core/server/services/auth/session').middleware; const sessionServiceMiddleware = require('../../../../core/server/services/auth/session');
describe('v2 Session controller', function () { describe('v2 Session controller', function () {
before(function () { before(function () {

View file

@ -5,7 +5,7 @@ const models = require('../../../../core/server/models');
const {UnauthorizedError} = require('../../../../core/server/lib/common/errors'); const {UnauthorizedError} = require('../../../../core/server/lib/common/errors');
const sessionController = require('../../../../core/server/api/canary/session'); const sessionController = require('../../../../core/server/api/canary/session');
const sessionServiceMiddleware = require('../../../../core/server/services/auth/session').middleware; const sessionServiceMiddleware = require('../../../../core/server/services/auth/session');
describe('v3 Session controller', function () { describe('v3 Session controller', function () {
before(function () { before(function () {

View file

@ -2,11 +2,6 @@ const sessionMiddleware = require('../../../../../core/server/services/auth').se
const models = require('../../../../../core/server/models'); const models = require('../../../../../core/server/models');
const sinon = require('sinon'); const sinon = require('sinon');
const should = require('should'); const should = require('should');
const {
BadRequestError,
UnauthorizedError,
InternalServerError
} = require('../../../../../core/server/lib/common/errors');
describe('Session Service', function () { describe('Session Service', function () {
before(function () { before(function () {
@ -22,6 +17,7 @@ describe('Session Service', function () {
session: { session: {
destroy() {} destroy() {}
}, },
user: null,
body: {}, body: {},
get() {} get() {}
}; };
@ -34,18 +30,6 @@ describe('Session Service', function () {
}; };
describe('createSession', function () { describe('createSession', function () {
it('calls next with a BadRequestError if there is no Origin or Refferer', function (done) {
const req = fakeReq();
sinon.stub(req, 'get')
.withArgs('origin').returns('')
.withArgs('referrer').returns('');
sessionMiddleware.createSession(req, fakeRes(), function next(err) {
should.equal(err instanceof BadRequestError, true);
done();
});
});
it('sets req.session.origin from the Referer header', function (done) { it('sets req.session.origin from the Referer header', function (done) {
const req = fakeReq(); const req = fakeReq();
const res = fakeRes(); const res = fakeRes();
@ -59,7 +43,7 @@ describe('Session Service', function () {
req.user = models.User.forge({id: 23}); req.user = models.User.forge({id: 23});
sinon.stub(res, 'sendStatus') sinon.stub(res, 'sendStatus')
.callsFake(function (statusCode) { .callsFake(function () {
should.equal(req.session.origin, 'http://ghost.org'); should.equal(req.session.origin, 'http://ghost.org');
done(); done();
}); });
@ -102,7 +86,7 @@ describe('Session Service', function () {
}); });
sinon.stub(res, 'sendStatus') sinon.stub(res, 'sendStatus')
.callsFake(function (statusCode) { .callsFake(function () {
should.equal(destroyStub.callCount, 1); should.equal(destroyStub.callCount, 1);
done(); done();
}); });
@ -119,7 +103,7 @@ describe('Session Service', function () {
}); });
sessionMiddleware.destroySession(req, res, function next(err) { sessionMiddleware.destroySession(req, res, function next(err) {
should.equal(err instanceof InternalServerError, true); should.equal(err.errorType, 'InternalServerError');
done(); done();
}); });
}); });

View file

@ -435,6 +435,11 @@
mobiledoc-dom-renderer "0.7.0" mobiledoc-dom-renderer "0.7.0"
mobiledoc-text-renderer "0.4.0" mobiledoc-text-renderer "0.4.0"
"@tryghost/mw-session-from-token@^0.1.0":
version "0.1.0"
resolved "https://registry.yarnpkg.com/@tryghost/mw-session-from-token/-/mw-session-from-token-0.1.0.tgz#ad73da936d3cc3abdfc7753a5098e76ae5dc271e"
integrity sha512-t6a9YMMHaQ2Uypl+WaGLc4mFKjwhfyfA+l7V9H5DwFeAFIYWcRssy/D3e+gTT3rbLC39W0vqkmZFNE2JQZuuEQ==
"@tryghost/pretty-cli@1.2.3": "@tryghost/pretty-cli@1.2.3":
version "1.2.3" version "1.2.3"
resolved "https://registry.yarnpkg.com/@tryghost/pretty-cli/-/pretty-cli-1.2.3.tgz#06fc84e4659ffde7166ca2a9ebe17c3951438a6c" resolved "https://registry.yarnpkg.com/@tryghost/pretty-cli/-/pretty-cli-1.2.3.tgz#06fc84e4659ffde7166ca2a9ebe17c3951438a6c"