Bypass 2FA for accounts which haven't yet logged in

ref ENG-1681
TryGhost · Nov 26, 2024 · 28dba53 · 28dba53
1 parent 781bfdd
commit 28dba53
Show file tree

Hide file tree

Showing 6 changed files with 97 additions and 5 deletions.
diff --git a/ghost/core/core/server/api/endpoints/session.js b/ghost/core/core/server/api/endpoints/session.js
@@ -28,16 +28,26 @@ const controller = {
             }));
         }
 
-        return models.User.check({
-            email: object.username,
-            password: object.password
+        let skipVerification = false;
+
+        return models.User.getByEmail(object.username).then((user) => {
+            if (!user.hasLoggedIn()) {
+                skipVerification = true;
+            }
+
+            return models.User.check({
+                email: object.username,
+                password: object.password
+            });
         }).then((user) => {
             return Promise.resolve(function sessionMiddleware(req, res, next) {
                 req.brute.reset(function (err) {
                     if (err) {
                         return next(err);
                     }
                     req.user = user;
+                    req.skipVerification = skipVerification;
+
                     auth.session.createSession(req, res, next);
                 });
             });

diff --git a/ghost/core/core/server/data/schema/schema.js b/ghost/core/core/server/data/schema/schema.js
@@ -161,6 +161,7 @@ module.exports = {
         meta_title: {type: 'string', maxlength: 2000, nullable: true, validations: {isLength: {max: 300}}},
         meta_description: {type: 'string', maxlength: 2000, nullable: true, validations: {isLength: {max: 500}}},
         tour: {type: 'text', maxlength: 65535, nullable: true},
+        // NOTE: Used to determine whether a user has logged in previously
         last_seen: {type: 'dateTime', nullable: true},
         comment_notifications: {type: 'boolean', nullable: false, defaultTo: true},
         free_member_signup_notification: {type: 'boolean', nullable: false, defaultTo: true},

diff --git a/ghost/core/core/server/models/user.js b/ghost/core/core/server/models/user.js
@@ -329,6 +329,10 @@ User = ghostBookshelf.Model.extend({
         return this.save();
     },
 
+    hasLoggedIn: function hasLoggedIn() {
+        return Boolean(this.get('last_seen'));
+    },
+
     enforcedFilters: function enforcedFilters(options) {
         if (options.context && options.context.internal) {
             return null;

diff --git a/ghost/core/core/server/services/auth/session/middleware.js b/ghost/core/core/server/services/auth/session/middleware.js
@@ -3,7 +3,11 @@ const errors = require('@tryghost/errors');
 function SessionMiddleware({sessionService}) {
     async function createSession(req, res, next) {
         try {
-            await sessionService.createSessionForUser(req, res, req.user);
+            if (req.skipVerification) {
+                await sessionService.createVerifiedSessionForUser(req, res, req.user);
+            } else {
+                await sessionService.createSessionForUser(req, res, req.user);
+            }
 
             const isVerified = await sessionService.isVerifiedSession(req, res);
             if (isVerified) {

diff --git a/ghost/core/test/unit/api/canary/session.test.js b/ghost/core/test/unit/api/canary/session.test.js
@@ -57,6 +57,8 @@ describe('Session controller', function () {
             const fakeUser = models.User.forge({});
             sinon.stub(models.User, 'check')
                 .resolves(fakeUser);
+            sinon.stub(models.User, 'getByEmail')
+                .resolves(fakeUser);
 
             const createSessionStub = sinon.stub(sessionServiceMiddleware, 'createSession');
 
@@ -88,6 +90,8 @@ describe('Session controller', function () {
             const fakeUser = models.User.forge({});
             sinon.stub(models.User, 'check')
                 .resolves(fakeUser);
+            sinon.stub(models.User, 'getByEmail')
+                .resolves(fakeUser);
 
             sinon.stub(sessionServiceMiddleware, 'createSession');
 
@@ -102,6 +106,74 @@ describe('Session controller', function () {
                 should.equal(fakeNext.args[0][0], resetError);
             });
         });
+
+        it('it creates a verified session when the user has not logged in before', function () {
+            const fakeReq = {
+                brute: {
+                    reset: sinon.stub().callsArg(0)
+                }
+            };
+            const fakeRes = {};
+            const fakeNext = () => {};
+            const fakeUser = models.User.forge({});
+            sinon.stub(models.User, 'check')
+                .resolves(fakeUser);
+            sinon.stub(models.User, 'getByEmail')
+                .resolves(fakeUser);
+
+            const createSessionStub = sinon.stub(sessionServiceMiddleware, 'createSession');
+
+            return sessionController.add({data: {
+                username: '[email protected]',
+                password: 'qu33nRul35'
+            }}).then((fn) => {
+                fn(fakeReq, fakeRes, fakeNext);
+            }).then(function () {
+                should.equal(fakeReq.brute.reset.callCount, 1);
+
+                const createSessionStubCall = createSessionStub.getCall(0);
+                should.equal(fakeReq.user, fakeUser);
+                should.equal(createSessionStubCall.args[0], fakeReq);
+                should.equal(createSessionStubCall.args[1], fakeRes);
+                should.equal(createSessionStubCall.args[2], fakeNext);
+
+                should.equal(fakeReq.skipVerification, true);
+            });
+        });
+
+        it('it creates a non-verified session when the user has logged in before', function () {
+            const fakeReq = {
+                brute: {
+                    reset: sinon.stub().callsArg(0)
+                }
+            };
+            const fakeRes = {};
+            const fakeNext = () => {};
+            const fakeUser = models.User.forge({last_seen: new Date()});
+            sinon.stub(models.User, 'check')
+                .resolves(fakeUser);
+            sinon.stub(models.User, 'getByEmail')
+                .resolves(fakeUser);
+
+            const createSessionStub = sinon.stub(sessionServiceMiddleware, 'createSession');
+
+            return sessionController.add({data: {
+                username: '[email protected]',
+                password: 'qu33nRul35'
+            }}).then((fn) => {
+                fn(fakeReq, fakeRes, fakeNext);
+            }).then(function () {
+                should.equal(fakeReq.brute.reset.callCount, 1);
+
+                const createSessionStubCall = createSessionStub.getCall(0);
+                should.equal(fakeReq.user, fakeUser);
+                should.equal(createSessionStubCall.args[0], fakeReq);
+                should.equal(createSessionStubCall.args[1], fakeRes);
+                should.equal(createSessionStubCall.args[2], fakeNext);
+
+                should.equal(fakeReq.skipVerification, false);
+            });
+        });
     });
 
     describe('#delete', function () {

diff --git a/ghost/core/test/utils/fixtures/data-generator.js b/ghost/core/test/utils/fixtures/data-generator.js
@@ -134,7 +134,8 @@ DataGenerator.Content = {
             email: '[email protected]',
             password: 'Sl1m3rson99',
             profile_image: 'https://example.com/super_photo.jpg',
-            paid_subscription_canceled_notification: true
+            paid_subscription_canceled_notification: true,
+            last_seen: moment().subtract(1, 'hour').toDate()
         },
         {
             // admin