From 05330482e62500e4250aaab8f62d4020f76cfcf3 Mon Sep 17 00:00:00 2001
From: Fabien O'Carroll <fabien@allou.is>
Date: Fri, 12 Oct 2018 15:38:57 +0700
Subject: [PATCH] Stopped api key from assigning the 'Owner' role (#9971)

* Stopped api key from assigning the 'Owner' role

refs #9865

We do not want api keys to be able to assign the Owner role to any other
key or user.

* Cleaned up Role model permissible method

no-issue
---
 core/server/models/role.js         | 29 +++++++++++++++++------------
 core/test/unit/models/role_spec.js | 20 ++++++++++++++++++++
 2 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/core/server/models/role.js b/core/server/models/role.js
index 9bf7d857f2..07f719420b 100644
--- a/core/server/models/role.js
+++ b/core/server/models/role.js
@@ -51,33 +51,29 @@ Role = ghostBookshelf.Model.extend({
     },
 
     permissible: function permissible(roleModelOrId, action, context, unsafeAttrs, loadedPermissions, hasUserPermission, hasAppPermission) {
-        var self = this,
-            checkAgainst = [],
-            origArgs;
-
         // If we passed in an id instead of a model, get the model
         // then check the permissions
         if (_.isNumber(roleModelOrId) || _.isString(roleModelOrId)) {
-            // Grab the original args without the first one
-            origArgs = _.toArray(arguments).slice(1);
-
             // Get the actual role model
             return this.findOne({id: roleModelOrId, status: 'all'})
-                .then(function then(foundRoleModel) {
+                .then((foundRoleModel) => {
                     if (!foundRoleModel) {
                         throw new common.errors.NotFoundError({
                             message: common.i18n.t('errors.models.role.roleNotFound')
                         });
                     }
 
-                    // Build up the original args but substitute with actual model
-                    var newArgs = [foundRoleModel].concat(origArgs);
+                    // Grab the original args without the first one
+                    const origArgs = _.toArray(arguments).slice(1);
 
-                    return self.permissible.apply(self, newArgs);
+                    return this.permissible(foundRoleModel, ...origArgs);
                 });
         }
 
+        const roleModel = roleModelOrId;
+
         if (action === 'assign' && loadedPermissions.user) {
+            let checkAgainst;
             if (_.some(loadedPermissions.user.roles, {name: 'Owner'})) {
                 checkAgainst = ['Owner', 'Administrator', 'Editor', 'Author', 'Contributor'];
             } else if (_.some(loadedPermissions.user.roles, {name: 'Administrator'})) {
@@ -87,7 +83,16 @@ Role = ghostBookshelf.Model.extend({
             }
 
             // Role in the list of permissible roles
-            hasUserPermission = roleModelOrId && _.includes(checkAgainst, roleModelOrId.get('name'));
+            hasUserPermission = roleModelOrId && _.includes(checkAgainst, roleModel.get('name'));
+        }
+
+        if (action === 'assign' && loadedPermissions.apiKey) {
+            // apiKey cannot 'assign' the 'Owner' role
+            if (roleModel.get('name') === 'Owner') {
+                return Promise.reject(new common.errors.NoPermissionError({
+                    message: common.i18n.t('errors.models.role.notEnoughPermission')
+                }));
+            }
         }
 
         if (hasUserPermission && hasAppPermission) {
diff --git a/core/test/unit/models/role_spec.js b/core/test/unit/models/role_spec.js
index 12c3ab3b2e..6ad54302a4 100644
--- a/core/test/unit/models/role_spec.js
+++ b/core/test/unit/models/role_spec.js
@@ -1,4 +1,5 @@
 const models = require('../../../server/models');
+const {NoPermissionError} = require('../../../server/lib/common/errors');
 const ghostBookshelf = require('../../../server/models/base');
 const testUtils = require('../../utils');
 const should = require('should');
@@ -25,4 +26,23 @@ describe('Unit: models/role', function () {
                 .then(() => checkRolePermissionsCount(0));
         });
     });
+
+    describe('permissible', function () {
+        it('does not let api key assign the owner role', function () {
+            return models.Role.permissible(
+                models.Role.forge({name: 'Owner'}), // Owner role
+                'assign',                           // assign action
+                {},
+                {},
+                {apiKey: {}},                       // apiKey loaded permissions
+                true,
+                true,
+                true
+            ).then(() => {
+                throw new Error('models.Role.permissible should have thrown!');
+            }, (err) => {
+                should.equal(err instanceof NoPermissionError, true);
+            });
+        });
+    });
 });