Improved password reset and session invalidation for "locked" users (#11790)

- Fixed session invalidation for "locked" user
  - Currently Ghost API was returning 404 for users having status set to "locked". This lead the user to be stuck in Ghost-Admin with "Rousource Not Found" error message.
  - By returning 401 for non-"active" users it allows for the Ghost-Admin to redirect the user to "signin" screen where they would be instructed to reset their password

- Fixed error message returned by session API
  - Instead of returning generic 'access' denied message when error happens during `User.check` we want to return more specific error thrown inside of the method, e.g.: 'accountLocked' or 'accountSuspended'
  - Fixed messaging for 'accountLocked' i18n, which not corresponds to the
actual UI available to the end user

- Added automatic password reset email to locked users on sign-in
  - uses alternative email for required password reset so it's clear that this is a security related reset and not a user-requested reset

- Backported the auto sending of required password reset email to v2 sign-in route
  - used by 3rd party clients where the email is necessary for users to know why login is failing

Co-authored-by: Kevin Ansfield <kevin@lookingsideways.co.uk>

core/server/api/canary/authentication.js

@@ -108,7 +108,11 @@ module.exports = {
                     return auth.passwordreset.generateToken(frame.data.passwordreset[0].email, api.settings);
                 })
                 .then((token) => {
-                    return auth.passwordreset.sendResetNotification(token, api.mail);
+                    if (frame.data.required) {
+                        return auth.passwordreset.sendRequiredResetNotification(token, api.mail);
+                    } else {
+                        return auth.passwordreset.sendResetNotification(token, api.mail);
+                    }
                 });
         }
     },

core/server/api/canary/session.js

@@ -2,6 +2,7 @@ const Promise = require('bluebird');
 const common = require('../../lib/common');
 const models = require('../../models');
 const auth = require('../../services/auth');
+const api = require('./index');
 
 const session = {
     read(frame) {
@@ -35,11 +36,24 @@ const session = {
                     auth.session.createSession(req, res, next);
                 });
             });
-        }).catch((err) => {
-            throw new common.errors.UnauthorizedError({
-                message: common.i18n.t('errors.middleware.auth.accessDenied'),
-                err
-            });
+        }).catch(async (err) => {
+            if (!common.errors.utils.isIgnitionError(err)) {
+                throw new common.errors.UnauthorizedError({
+                    message: common.i18n.t('errors.middleware.auth.accessDenied'),
+                    err
+                });
+            }
+
+            if (err.errorType === 'PasswordResetRequiredError') {
+                await api.authentication.generateResetToken({
+                    passwordreset: [{
+                        email: object.username
+                    }],
+                    required: true
+                }, frame.options.context);
+            }
+
+            throw err;
         });
     },
     delete() {

core/server/api/v2/authentication.js

@@ -108,7 +108,11 @@ module.exports = {
                     return auth.passwordreset.generateToken(frame.data.passwordreset[0].email, api.settings);
                 })
                 .then((token) => {
-                    return auth.passwordreset.sendResetNotification(token, api.mail);
+                    if (frame.data.required) {
+                        return auth.passwordreset.sendRequiredResetNotification(token, api.mail);
+                    } else {
+                        return auth.passwordreset.sendResetNotification(token, api.mail);
+                    }
                 });
         }
     },

core/server/api/v2/session.js

@@ -2,6 +2,7 @@ const Promise = require('bluebird');
 const common = require('../../lib/common');
 const models = require('../../models');
 const auth = require('../../services/auth');
+const api = require('./index');
 
 const session = {
     read(frame) {
@@ -35,11 +36,24 @@ const session = {
                     auth.session.createSession(req, res, next);
                 });
             });
-        }).catch((err) => {
-            throw new common.errors.UnauthorizedError({
-                message: common.i18n.t('errors.middleware.auth.accessDenied'),
-                err
-            });
+        }).catch(async (err) => {
+            if (!common.errors.utils.isIgnitionError(err)) {
+                throw new common.errors.UnauthorizedError({
+                    message: common.i18n.t('errors.middleware.auth.accessDenied'),
+                    err
+                });
+            }
+
+            if (err.errorType === 'PasswordResetRequiredError') {
+                await api.authentication.generateResetToken({
+                    passwordreset: [{
+                        email: object.username
+                    }],
+                    required: true
+                }, frame.options.context);
+            }
+
+            throw err;
         });
     },
     delete() {

core/server/lib/common/errors.js

@@ -67,6 +67,13 @@ const ghostErrors = {
             errorType: 'HelperWarning',
             hideStack: true
         }, options));
+    },
+    PasswordResetRequiredError: function PasswordResetRequiredError(options) {
+        GhostError.call(this, merge({
+            errorType: 'PasswordResetRequiredError',
+            statusCode: 401,
+            message: 'For security, you need to create a new password. An email has been sent to you with instructions!'
+        }, options));
     }
 };
 

core/server/models/user.js

@@ -805,9 +805,7 @@ User = ghostBookshelf.Model.extend({
                 }
 
                 if (user.isLocked()) {
-                    throw new common.errors.NoPermissionError({
-                        message: common.i18n.t('errors.models.user.accountLocked')
-                    });
+                    throw new common.errors.PasswordResetRequiredError();
                 }
 
                 if (user.isInactive()) {

core/server/services/auth/passwordreset.js

@@ -145,10 +145,37 @@ async function sendResetNotification(data, mailAPI) {
     return mailAPI.send(payload, {context: {internal: true}});
 }
 
+async function sendRequiredResetNotification(data, mailAPI) {
+    const adminUrl = urlUtils.urlFor('admin', true);
+    const resetUrl = urlUtils.urlJoin(adminUrl, 'reset', security.url.encodeBase64(data.resetToken), '/');
+
+    const content = await mail.utils.generateContent({
+        data: {
+            resetUrl: resetUrl
+        },
+        template: 'reset-password-required'
+    });
+
+    const payload = {
+        mail: [{
+            message: {
+                to: data.email,
+                subject: i18n.t('common.api.authentication.mail.resetPasswordRequired'),
+                html: content.html,
+                text: content.text
+            },
+            options: {}
+        }]
+    };
+
+    return mailAPI.send(payload, {context: {internal: true}});
+}
+
 module.exports = {
-    generateToken: generateToken,
-    extractTokenParts: extractTokenParts,
-    protectBruteForce: protectBruteForce,
-    doReset: doReset,
-    sendResetNotification: sendResetNotification
+    generateToken,
+    extractTokenParts,
+    protectBruteForce,
+    doReset,
+    sendResetNotification,
+    sendRequiredResetNotification
 };

core/server/services/mail/templates/reset-password-required.html

@@ -0,0 +1,54 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN       " "http://www.w3.org/TR/html4/loose.dtd">
+
+<html lang="en">
+<head>
+    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
+
+</head>
+<body bgcolor="#ffffff" topmargin="0" leftmargin="0" marginheight="0" marginwidth="0" style="-webkit-font-smoothing: antialiased; -webkit-text-size-adjust: none; background: #ffffff; color: #808284; font-family: sans-serif; font-size: 15px; line-height: 1.5; margin: 0; width: 100%;">
+
+<table width="100%" cellpadding="0" cellspacing="0" border="0" bgcolor="#ffffff">
+    <tr>
+        <td bgcolor="#ffffff" width="100%">
+
+            <table class="main-wrapper" width="600" cellpadding="0" cellspacing="0" border="0" align="center" bgcolor="#ffffff">
+                <tr>
+                    <td class="cell" width="100%">
+
+                        <div class="wrapper" style="-moz-border-radius: 3px; -webkit-border-radius: 3px; border: #e5e3d8 1px solid; border-radius: 3px; margin: 2%; padding: 5% 8%;">
+                            <table class="content" width="100%" cellpadding="0" cellspacing="0" border="0">
+                                <tr>
+                                    <td class="content-cell" width="100%">
+
+                                        <!-- START OF EMAIL CONTENT -->
+                                        <p style="color: #808284; font-family: sans-serif; font-size: 15px; font-weight: normal; line-height: 1.5em; margin: 0; padding: 0 0 1.5em 0;"><strong>Hello!</strong></p>
+                                        <p style="color: #808284; font-family: sans-serif; font-size: 15px; font-weight: normal; line-height: 1.5em; margin: 0; padding: 0 0 1.5em 0;">For security, it's necessary to reset your password on <a href="{{ siteUrl }}" style="color: #5ba4e5;">{{ siteUrl }}</a>.</p>
+                                        <p style="color: #808284; font-family: sans-serif; font-size: 15px; font-weight: normal; line-height: 1.5em; margin: 0; padding: 0 0 1.5em 0;">Please follow the link below to complete the process:<br><br>  <a href="{{ resetUrl }}" style="color: #5ba4e5;">Click here to reset your password</a></p>
+                                        <p style="color: #808284; font-family: sans-serif; font-size: 15px; font-weight: normal; line-height: 1.5em; margin: 0; padding: 0 0 1.5em 0;"><i>Alternatively please visit your site's admin area and follow the forgot password process.</i></p>
+                                        <p style="color: #808284; font-family: sans-serif; font-size: 15px; font-weight: normal; line-height: 1.5em; margin: 0; padding: 0 0 1.5em 0;">Ghost</p>
+                                        <!-- END OF EMAIL CONTENT -->
+
+                                    </td>
+                                </tr>
+                            </table>
+                        </div>
+                        <div class="container" style="padding: 0 4%;">
+                            <table class="footer" width="100%" border="0" cellspacing="0" cellpadding="0">
+                                <tr>
+                                    <td class="footer-cell" align="right" style="color: #888888; font-family: sans-serif; font-size: 11px; line-height: 1.3; padding: 0 0 20px 0;">
+                                        Sent by <a href="{{siteUrl}}" style="color: #5ba4e5;">{{siteUrl}}</a>
+                                    </td>
+                                </tr>
+                            </table>
+                        </div>
+
+                    </td>
+                </tr>
+            </table>
+
+        </td>
+    </tr>
+</table>
+
+</body>
+</html>

core/server/services/permissions/providers.js

@@ -6,7 +6,7 @@ const {i18n} = require('../../lib/common');
 
 module.exports = {
     user: function (id) {
-        return models.User.findOne({id: id, status: 'active'}, {withRelated: ['permissions', 'roles', 'roles.permissions']})
+        return models.User.findOne({id: id}, {withRelated: ['permissions', 'roles', 'roles.permissions']})
             .then(function (foundUser) {
             // CASE: {context: {user: id}} where the id is not in our database
                 if (!foundUser) {
@@ -15,6 +15,10 @@ module.exports = {
                     }));
                 }
 
+                if (foundUser.get('status') !== 'active') {
+                    return Promise.reject(new errors.UnauthorizedError());
+                }
+
                 const seenPerms = {};
 
                 const rolePerms = _.map(foundUser.related('roles').models, function (role) {

core/server/translations/en.json

@@ -12,6 +12,7 @@
                 "sampleBlogDescription": "Thoughts, stories and ideas.",
                 "mail": {
                     "resetPassword": "Reset Password",
+                    "resetPasswordRequired": "Reset Password",
                     "checkEmailForInstructions": "Check your email for further instructions.",
                     "passwordChanged": "Password changed successfully.",
                     "invitationAccepted": "Invitation accepted.",
@@ -227,7 +228,6 @@
                     "help": "Visit and save your profile after logging in to check for problems."
                 },
                 "incorrectPassword": "Your password is incorrect.",
-                "accountLocked": "Your account is locked. Please reset your password to log in again by clicking the \"Forgotten password?\" link!",
                 "accountSuspended": "Your account was suspended.",
                 "newPasswordsDoNotMatch": "Your new passwords do not match",
                 "passwordRequiredForOperation": "Password is required for this operation",

test/unit/models/user_spec.js

@@ -135,6 +135,20 @@ describe('Unit: models/user', function () {
                     (err instanceof common.errors.ValidationError).should.eql(true);
                 });
         });
+
+        it('status is locked', function () {
+            const user = models.User.forge(testUtils.DataGenerator.forKnex.createUser({
+                status: 'locked',
+                email: 'test@ghost.de'
+            }));
+
+            sinon.stub(models.User, 'getByEmail').resolves(user);
+
+            return models.User.check({email: user.get('email'), password: 'test'})
+                .catch(function (err) {
+                    (err instanceof common.errors.PasswordResetRequiredError).should.eql(true);
+                });
+        });
     });
 
     describe('permissible', function () {

test/unit/services/permissions/providers_spec.js

@@ -36,6 +36,7 @@ describe('Permission Providers', function () {
             const findUserSpy = sinon.stub(models.User, 'findOne').callsFake(function () {
                 // Create a fake model
                 const fakeUser = models.User.forge(testUtils.DataGenerator.Content.users[0]);
+                fakeUser.set('status', 'active');
 
                 // Roles & Permissions need to be collections
                 const fakeAdminRole = models.Roles.forge(testUtils.DataGenerator.Content.roles[0]);
@@ -83,6 +84,7 @@ describe('Permission Providers', function () {
             const findUserSpy = sinon.stub(models.User, 'findOne').callsFake(function () {
                 // Create a fake model
                 const fakeUser = models.User.forge(testUtils.DataGenerator.Content.users[0]);
+                fakeUser.set('status', 'active');
 
                 // Roles & Permissions need to be collections
                 const fakeAdminRole = models.Roles.forge(testUtils.DataGenerator.Content.roles[0]);
@@ -132,6 +134,7 @@ describe('Permission Providers', function () {
             const findUserSpy = sinon.stub(models.User, 'findOne').callsFake(function () {
                 // Create a fake model
                 const fakeUser = models.User.forge(testUtils.DataGenerator.Content.users[0]);
+                fakeUser.set('status', 'active');
 
                 // Roles & Permissions need to be collections
                 const fakeAdminRole = models.Roles.forge(testUtils.DataGenerator.Content.roles[0]);
@@ -176,6 +179,28 @@ describe('Permission Providers', function () {
                 })
                 .catch(done);
         });
+
+        it('throws when user with non-active status is loaded', function (done) {
+            // This test requires quite a lot of unique setup work
+            const findUserSpy = sinon.stub(models.User, 'findOne').callsFake(function () {
+                // Create a fake model
+                const fakeUser = models.User.forge(testUtils.DataGenerator.Content.users[0]);
+                fakeUser.set('status', 'locked');
+
+                return Promise.resolve(fakeUser);
+            });
+
+            // Get permissions for the user
+            providers.user(1)
+                .then(function (res) {
+                    done(new Error('Locked user should should throw an error'));
+                })
+                .catch((err) => {
+                    err.errorType.should.equal('UnauthorizedError');
+                    findUserSpy.callCount.should.eql(1);
+                    done();
+                });
+        });
     });
 
     describe('API Key', function () {