Session based auth #9867

allouis · 2018-09-13T08:54:16Z

This is the session side of #9865

This adds a "session auth" service, which exposes some middleware to be used by the admin api.

getSession - populates req.session
cookieCsrfProtection - makes sure request is made from same origin session was created
safeGetSession - the above two mixed together
getUser - populates req.user based on req.session.user_id
ensureUser - responds with 401 if the user is missing
createSession - accepts user credentials and creates a user session (login)
destroySession - deletes the current session (logout)

package.json

@@ -1,6 +1,6 @@
 {
  "name": "ghost",
-  "version": "2.1.1",
+  "version": "2.2.0",


core/test/unit/services/auth/session/index_spec.js

core/server/services/auth/session/index.js

core/server/services/auth/authorize.js

+    },
+
+    authorizeAdminAPI: [function (req, res, next) {
+        next();


core/server/services/auth/session/index.js

+            req.user = user;
+            next();
+        }).catch(() => {
+            next(new UnauthorizedError('No user found'));


.gitignore

@@ -76,3 +76,5 @@ CHANGELOG.md

 # Coverage reports
 coverage.html
+
+cookies.txt


core/server/web/api/routes.js

@@ -168,6 +168,12 @@ module.exports = function apiRoutes() {
    apiRouter.put('/authentication/setup', mw.authenticatePrivate, api.http(api.authentication.updateSetup));
    apiRouter.get('/authentication/setup', api.http(api.authentication.isSetup));

+    apiRouter.post('/session', auth.session.getSession, auth.session.createSession);


delete-session.sh

@@ -0,0 +1,3 @@
+#!/bin/sh


get-session.sh

@@ -0,0 +1,5 @@
+#!/bin/sh


post-session.sh

@@ -0,0 +1,5 @@
+#!/bin/sh


allouis · 2018-09-18T06:24:51Z

Added comments to files that are only there for allowing CI tests to pass or for local testing to happen. They'll be deleted before squashing when this gets merged

refs #9867

kirrg001 · 2018-09-18T17:46:39Z

@allouis Is this PR revieable? 🤔 Doesn't use the WIP keyword.

allouis · 2018-09-19T06:16:41Z

@kirrg001 Yup. Future changes will happen in separate PR's

core/server/data/migrations/init/3-create-session-secret.js

+    deletedMessage = 'Deleted Settings Key `session-secret`.';
+
+module.exports.up = () => {
+    return models.Settings.findOne({key: 'session-secret'})


core/server/data/schema/schema.js

@@ -300,5 +300,12 @@ module.exports = {
        created_by: {type: 'string', maxlength: 24, nullable: false},
        updated_at: {type: 'dateTime', nullable: true},
        updated_by: {type: 'string', maxlength: 24, nullable: true}
+    },
+    sessions: {
+        id: {type: 'string', maxlength: 32, nullable: false, primary: true},


core/server/data/schema/schema.js

+    sessions: {
+        id: {type: 'string', maxlength: 32, nullable: false, primary: true},
+        user_id: {type: 'string', maxlength: 24, nullable: false},
+        session_data: {type: 'text', maxlength: 2000, nullable: false},


allouis

Lots of style issues and some comments.

core/server/data/migrations/versions/2.2/1-add-sessions-table.js

@@ -0,0 +1,35 @@
+var common = require('../../../../lib/common'),


core/server/data/migrations/versions/2.2/1-add-sessions-table.js

+    message2 = 'Dropping table: ' + table;
+
+module.exports.up = function addSessionsTable(options) {
+    let connection = options.connection;


core/server/data/migrations/versions/2.2/1-add-sessions-table.js

+};
+
+module.exports.down = function removeSessionsTable(options) {
+    let connection = options.connection;


core/test/unit/data/schema/integrity_spec.js

@@ -19,7 +19,7 @@ var should = require('should'),
 */
 describe('DB version integrity', function () {
    // Only these variables should need updating
-    var currentSchemaHash = '22d24b1de23d118b90e9547fefae5ad7',
+    var currentSchemaHash = 'd40f0e1f36c3d2f73852f60323cfa09c',


core/server/models/session.js

@@ -0,0 +1,39 @@
+const ghostBookshelf = require('./base');
+
+let Session,


core/test/unit/services/auth/session/store_spec.js

@@ -0,0 +1,255 @@
+const SessionStore = require('../../../../../server/services/auth/session/store'),


core/server/services/auth/authenticate.js

@@ -2,6 +2,7 @@ var passport = require('passport'),
    authUtils = require('./utils'),
    models = require('../../models'),
    common = require('../../lib/common'),
+    session = require('./session'),


core/server/services/auth/authorize.js

@@ -1,4 +1,5 @@
 var labs = require('../labs'),
+    session = require('./session'),


core/server/services/auth/auth.js

@@ -0,0 +1,5 @@
+const authenticate = require('./authenticate'),


core/server/web/api/v2/admin/routes.js

+    // ## Sessions
+    // Only need to `getSession` here because this is only route they don't
+    // have to be authenticated for.
+    router.post('/session', auth.session.getSession, auth.session.createSession);


core/server/services/auth/auth.js

+    authorize = require('./authorize');
+
+module.exports.contentAPI = [authenticate.authenticateContentAPI, authorize.authorizeContentAPI];
+module.exports.adminAPI = [authenticate.authenticateAdminAPI, authorize.authorizeAdminAPI];


core/server/services/auth/index.js

    oauth = require('./oauth');

+/* TODO: Remove when v0.1 is dropped */


core/test/unit/services/auth/session/store_spec.js

@@ -0,0 +1,255 @@
+const SessionStore = require('../../../../../server/services/auth/session/store'),


This reverts commit 1bd8a10.

refs #9865 This is to ensure that if a controller returns a function, it will always get called regardless of method. Also cleaned up top level const usage

refs #9865 This is so we can use the service inside of a controller, and handle as much as possible in the controller, leaving only session specific stuff up to this service.

refs #9865 Note that this controller is the singular, that's because we plan to make a session resource controller to be used with /sessions, wheras this is on /session

This reverts commit b171248.

This reverts commit 4bd5151.

refs #9865 Updates the function signature to look like other db methods for consistency

refs #9865

refs #9865 this keeps model logic in tha model

core/server/services/auth/session/middleware.js

+let UNO_SESSIONIONA;
+const getSession = (req, res, next) => {
+    if (!UNO_SESSIONIONA) {
+        UNO_SESSIONIONA = session({


allouis · 2018-09-26T07:08:23Z

closing this in favour of smaller PRs 👍

kevinansfield reviewed Sep 13, 2018

View reviewed changes

package.json Outdated

@@ -1,6 +1,6 @@

{

"name": "ghost",

"version": "2.1.1",

"version": "2.2.0",

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

allouis commented Sep 13, 2018

View reviewed changes

core/test/unit/services/auth/session/index_spec.js Outdated Show resolved Hide resolved

kirrg001 assigned kevinansfield and kirrg001 Sep 14, 2018

kevinansfield reviewed Sep 14, 2018

View reviewed changes

core/server/services/auth/session/index.js Outdated Show resolved Hide resolved

kirrg001 self-requested a review September 14, 2018 15:32