🐛 Fix last seen for users (#10141)

* Added updateLastSeen method to user model

refs #10138

* Refactor codebase to use user.updateLastSeen

refs #10138

This is to ensure all updates go via the same method, meaning any
specific logic can be handled in one place, it also helps with grepping
the codebase to find where this occurs

* Created updateUserLastSeen middleware for v2 admin

refs #10138

This is intended to be used with the v2 admin api and _possibly_ the
content api, to give us an accruate report on thelast time a user access
a ghost instance.

* Wired updateUserLastSeen up to v2 Admin API

closes #10138

* Fixed broken test for v2 admin api

no-issue

This test was broken because it was incorrectly testing for a method to
be called exactly once - this was irrelevant to the functionality being
tested for.

* Updated user check method to set status to active

no-issue

* Debounced the updateUserLastSeen middlware an hour

no-issue

* Resolved some PR comments

core/server/models/base/listeners.js

@@ -5,10 +5,14 @@ var moment = require('moment-timezone'),
     sequence = require('../../lib/promise/sequence');
 
 /**
+ * @TODO REMOVE WHEN v0.1 IS DROPPED
  * WHEN access token is created we will update last_seen for user.
  */
 common.events.on('token.added', function (tokenModel) {
-    models.User.edit({last_seen: moment().toDate()}, {id: tokenModel.get('user_id')})
+    models.User.findOne({id: tokenModel.get('user_id')})
+        .then(function (user) {
+            return user.updateLastSeen();
+        })
         .catch(function (err) {
             common.logging.error(new common.errors.GhostError({err: err, level: 'critical'}));
         });

core/server/models/user.js

@@ -259,6 +259,11 @@ User = ghostBookshelf.Model.extend({
         });
     },
 
+    updateLastSeen: function updateLastSeen() {
+        this.set({last_seen: new Date()});
+        return this.save();
+    },
+
     enforcedFilters: function enforcedFilters(options) {
         if (options.context && options.context.internal) {
             return null;
@@ -756,7 +761,7 @@ User = ghostBookshelf.Model.extend({
         var self = this;
 
         return this.getByEmail(object.email)
-            .then(function then(user) {
+            .then((user) => {
                 if (!user) {
                     throw new common.errors.NotFoundError({
                         message: common.i18n.t('errors.models.user.noUserWithEnteredEmailAddr')
@@ -776,12 +781,15 @@ User = ghostBookshelf.Model.extend({
                 }
 
                 return self.isPasswordCorrect({plainPassword: object.password, hashedPassword: user.get('password')})
-                    .then(function then() {
-                        user.set({status: 'active', last_seen: new Date()});
+                    .then(() => {
+                        return user.updateLastSeen();
+                    })
+                    .then(() => {
+                        user.set({status: 'active'});
                         return user.save();
                     });
             })
-            .catch(function (err) {
+            .catch((err) => {
                 if (err.message === 'NotFound' || err.message === 'EmptyResponse') {
                     throw new common.errors.NotFoundError({
                         message: common.i18n.t('errors.models.user.noUserWithEnteredEmailAddr')

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

@@ -7,6 +7,7 @@ const shared = require('../../../shared');
 module.exports.authAdminAPI = [
     auth.authenticate.authenticateAdminAPI,
     auth.authorize.authorizeAdminAPI,
+    shared.middlewares.updateUserLastSeen,
     shared.middlewares.api.cors,
     shared.middlewares.urlRedirects.adminRedirect,
     shared.middlewares.prettyUrls

core/server/web/shared/middlewares/index.js

@@ -75,6 +75,10 @@ module.exports = {
         return require('./url-redirects');
     },
 
+    get updateUserLastSeen() {
+        return require('./update-user-last-seen');
+    },
+
     get emitEvents() {
         return require('./emit-events');
     }

core/server/web/shared/middlewares/update-user-last-seen.js

@@ -0,0 +1,15 @@
+const constants = require('../../../lib/constants');
+
+module.exports = function updateUserLastSeenMiddleware(req, res, next) {
+    if (!req.user) {
+        return next();
+    }
+
+    if (Date.now() - req.user.get('last_seen') < constants.ONE_HOUR_MS) {
+        return next();
+    }
+
+    req.user.updateLastSeen().then(() => {
+        next();
+    }, next);
+};

core/test/functional/api/v2/admin/slack_spec.js

@@ -43,7 +43,6 @@ describe('Slack API', function () {
                 should.not.exist(res.headers['x-cache-invalidate']);
                 const jsonResponse = res.body;
                 should.exist(jsonResponse);
-                eventSpy.calledOnce.should.be.true();
                 eventSpy.calledWith('slack.test').should.be.true();
                 done();
             });

core/test/unit/models/base/listeners_spec.js

@@ -8,6 +8,7 @@ var should = require('should'),
 
 describe('Models: listeners', function () {
     var eventsToRemember = {};
+    const emit = (event, data) => eventsToRemember[event](data);
 
     before(function () {
         sandbox.stub(common.events, 'on').callsFake(function (name, callback) {
@@ -23,22 +24,20 @@ describe('Models: listeners', function () {
     });
 
     describe('on token added', function () {
-        it('calls User edit when event is emitted', function (done) {
-            var userModelSpy = sandbox.spy(Models.User, 'edit');
-
-            eventsToRemember['token.added']({
-                get: function () {
-                    return 1;
-                }
+        it('calls updateLastSeen on the user when the token.added event is emited', function (done) {
+            const userId = 1;
+            const user = Models.User.forge({id: 1});
+            sandbox.stub(Models.User, 'findOne').withArgs({id: userId}).resolves(user);
+            const updateLastSeenSpy = sandbox.stub(user, 'updateLastSeen').callsFake(function () {
+                updateLastSeenSpy.calledOnce.should.be.true();
+                done();
             });
 
-            userModelSpy.calledOnce.should.be.true();
-            userModelSpy.calledWith(
-                sinon.match.has('last_seen'),
-                sinon.match.has('id')
-            );
+            const fakeToken = {
+                get: sandbox.stub().withArgs('user_id').returns(userId)
+            };
 
-            done();
+            emit('token.added', fakeToken);
         });
     });
 });

core/test/unit/models/user_spec.js

@@ -24,6 +24,32 @@ describe('Unit: models/user', function () {
         sandbox.restore();
     });
 
+    describe('updateLastSeen method', function () {
+        it('exists', function () {
+            should.equal(typeof models.User.prototype.updateLastSeen, 'function');
+        });
+
+        it('sets the last_seen property to new Date and returns a call to save', function () {
+            const instance = {
+                set: sandbox.spy(),
+                save: sandbox.stub().resolves()
+            };
+
+            const now = new Date();
+            const clock = sinon.useFakeTimers(now.getTime());
+
+            const returnVal = models.User.prototype.updateLastSeen.call(instance);
+
+            should.deepEqual(instance.set.args[0][0], {
+                last_seen: now
+            });
+
+            should.equal(returnVal, instance.save.returnValues[0]);
+
+            clock.restore();
+        });
+    });
+
     describe('validation', function () {
         beforeEach(function () {
             sandbox.stub(security.password, 'hash').resolves('$2a$10$we16f8rpbrFZ34xWj0/ZC.LTPUux8ler7bcdTs5qIleN6srRHhilG');

core/test/unit/web/middleware/update-user-last-seen_spec.js

@@ -0,0 +1,63 @@
+const should = require('should');
+const sinon = require('sinon');
+const constants = require('../../../../server/lib/constants');
+const updateUserLastSeenMiddleware = require('../../../../server/web/shared/middlewares').updateUserLastSeen;
+
+describe('updateUserLastSeenMiddleware', function () {
+    let sandbox;
+
+    before(function () {
+        sandbox = sinon.sandbox.create();
+    });
+
+    afterEach(function () {
+        sandbox.restore();
+    });
+
+    it('calls next with no error if there is no user on the request', function (done) {
+        updateUserLastSeenMiddleware({}, {}, function next(err) {
+            should.equal(err, undefined);
+            done();
+        });
+    });
+
+    it('calls next with no error if the current last_seen is less than an hour before now', function (done) {
+        const fakeLastSeen = new Date();
+        const fakeUser = {
+            get: sandbox.stub().withArgs('last_seen').returns(fakeLastSeen)
+        };
+        updateUserLastSeenMiddleware({user: fakeUser}, {}, function next(err) {
+            should.equal(err, undefined);
+            done();
+        });
+    });
+
+    describe('when the last_seen is longer than an hour ago', function () {
+        it('calls updateLastSeen on the req.user, calling next with nothing if success', function (done) {
+            const fakeLastSeen = new Date(Date.now() - constants.ONE_HOURS_MS);
+            const fakeUser = {
+                get: sandbox.stub().withArgs('last_seen').returns(fakeLastSeen),
+                updateLastSeen: sandbox.stub().resolves()
+            };
+            updateUserLastSeenMiddleware({user: fakeUser}, {}, function next(err) {
+                should.equal(err, undefined);
+                should.equal(fakeUser.updateLastSeen.callCount, 1);
+                done();
+            });
+        });
+
+        it('calls updateLastSeen on the req.user, calling next with err if error', function (done) {
+            const fakeLastSeen = new Date(Date.now() - constants.ONE_HOURS_MS);
+            const fakeError = new Error('gonna need a bigger boat');
+            const fakeUser = {
+                get: sandbox.stub().withArgs('last_seen').returns(fakeLastSeen),
+                updateLastSeen: sandbox.stub().rejects(fakeError)
+            };
+            updateUserLastSeenMiddleware({user: fakeUser}, {}, function next(err) {
+                should.equal(err, fakeError);
+                should.equal(fakeUser.updateLastSeen.callCount, 1);
+                done();
+            });
+        });
+    });
+});