diff --git a/core/server/services/permissions/can-this.js b/core/server/services/permissions/can-this.js index 5bd9d0d70a..e0a94f890b 100644 --- a/core/server/services/permissions/can-this.js +++ b/core/server/services/permissions/can-this.js @@ -50,8 +50,10 @@ CanThisResult.prototype.buildObjectTypeHandlers = function (objTypes, actType, c return permissionLoad.then(function (loadedPermissions) { // Iterate through the user permissions looking for an affirmation var userPermissions = loadedPermissions.user ? loadedPermissions.user.permissions : null, + apiKeyPermissions = loadedPermissions.apiKey ? loadedPermissions.apiKey.permissions : null, appPermissions = loadedPermissions.app ? loadedPermissions.app.permissions : null, hasUserPermission, + hasApiKeyPermission, hasAppPermission, checkPermission = function (perm) { var permObjId; @@ -82,6 +84,14 @@ CanThisResult.prototype.buildObjectTypeHandlers = function (objTypes, actType, c hasUserPermission = _.some(userPermissions, checkPermission); } + // Check api key permissions if they were passed + hasApiKeyPermission = true; + if (!_.isNull(apiKeyPermissions)) { + // api key request have no user, but we want the user permissions checks to pass + hasUserPermission = true; + hasApiKeyPermission = _.some(apiKeyPermissions, checkPermission); + } + // Check app permissions if they were passed hasAppPermission = true; if (!_.isNull(appPermissions)) { @@ -91,11 +101,11 @@ CanThisResult.prototype.buildObjectTypeHandlers = function (objTypes, actType, c // Offer a chance for the TargetModel to override the results if (TargetModel && _.isFunction(TargetModel.permissible)) { return TargetModel.permissible( - modelId, actType, context, unsafeAttrs, loadedPermissions, hasUserPermission, hasAppPermission + modelId, actType, context, unsafeAttrs, loadedPermissions, hasUserPermission, hasAppPermission, hasApiKeyPermission ); } - if (hasUserPermission && hasAppPermission) { + if (hasUserPermission && hasApiKeyPermission && hasAppPermission) { return; } @@ -110,10 +120,11 @@ CanThisResult.prototype.buildObjectTypeHandlers = function (objTypes, actType, c CanThisResult.prototype.beginCheck = function (context) { var self = this, userPermissionLoad, + apiKeyPermissionLoad, appPermissionLoad, permissionsLoad; - // Get context.user and context.app + // Get context.user, context.api_key_id and context.app context = parseContext(context); if (actionsMap.empty()) { @@ -128,6 +139,14 @@ CanThisResult.prototype.beginCheck = function (context) { userPermissionLoad = Promise.resolve(null); } + // Kick off loading of api key permissions if necessary + if (context.api_key_id) { + apiKeyPermissionLoad = providers.apiKey(context.api_key_id); + } else { + // Resolve null if no context.api_key_id + apiKeyPermissionLoad = Promise.resolve(null); + } + // Kick off loading of app permissions if necessary if (context.app) { appPermissionLoad = providers.app(context.app); @@ -137,10 +156,11 @@ CanThisResult.prototype.beginCheck = function (context) { } // Wait for both user and app permissions to load - permissionsLoad = Promise.all([userPermissionLoad, appPermissionLoad]).then(function (result) { + permissionsLoad = Promise.all([userPermissionLoad, apiKeyPermissionLoad, appPermissionLoad]).then(function (result) { return { user: result[0], - app: result[1] + apiKey: result[1], + app: result[2] }; }); diff --git a/core/server/services/permissions/parse-context.js b/core/server/services/permissions/parse-context.js index acc1a9cd4a..27ac1c5829 100644 --- a/core/server/services/permissions/parse-context.js +++ b/core/server/services/permissions/parse-context.js @@ -3,7 +3,7 @@ * * Utility function, to expand strings out into objects. * @param {Object|String} context - * @return {{internal: boolean, external: boolean, user: integer|null, app: integer|null, public: boolean}} + * @return {{internal: boolean, external: boolean, user: integer|null, app: integer|null, public: boolean, api_key_id: ObjectId|null}} */ module.exports = function parseContext(context) { // Parse what's passed to canThis.beginCheck for standard user and app scopes @@ -11,6 +11,7 @@ module.exports = function parseContext(context) { internal: false, external: false, user: null, + api_key_id: null, app: null, public: true }; @@ -31,6 +32,11 @@ module.exports = function parseContext(context) { parsed.public = false; } + if (context && context.api_key_id) { + parsed.api_key_id = context.api_key_id; + parsed.public = false; + } + if (context && context.app) { parsed.app = context.app; parsed.public = false; diff --git a/core/server/services/permissions/providers.js b/core/server/services/permissions/providers.js index 3b05b7a2d6..bede6238ee 100644 --- a/core/server/services/permissions/providers.js +++ b/core/server/services/permissions/providers.js @@ -53,5 +53,23 @@ module.exports = { return {permissions: foundApp.related('permissions').models}; }); + }, + + apiKey(id) { + return models.ApiKey.findOne({id}, {withRelated: ['role', 'role.permissions']}) + .then((foundApiKey) => { + if (!foundApiKey) { + throw new common.errors.NotFoundError({ + message: common.i18n.t('errors.models.api_key.apiKeyNotFound') + }); + } + + // api keys have a belongs_to relationship to a role and no individual permissions + // so there's no need for permission deduplication + const permissions = foundApiKey.related('role').related('permissions').models; + const roles = [foundApiKey.toJSON().role]; + + return {permissions, roles}; + }); } }; diff --git a/core/server/translations/en.json b/core/server/translations/en.json index cfe0005ac5..2b3d0a22ed 100644 --- a/core/server/translations/en.json +++ b/core/server/translations/en.json @@ -270,6 +270,9 @@ "onlyOwnerCanTransferOwnerRole": "Only owners are able to transfer the owner role.", "onlyAdmCanBeAssignedOwnerRole": "Only administrators can be assigned the owner role." }, + "api_key": { + "apiKeyNotFound": "API Key not found" + }, "base": { "index": { "missingContext": "missing context" diff --git a/core/test/unit/api/v0.1/utils_spec.js b/core/test/unit/api/v0.1/utils_spec.js index c755322378..cdbe295849 100644 --- a/core/test/unit/api/v0.1/utils_spec.js +++ b/core/test/unit/api/v0.1/utils_spec.js @@ -608,7 +608,7 @@ describe('API Utils', function () { describe('handlePublicPermissions', function () { it('should return empty options if passed empty options', function (done) { apiUtils.handlePublicPermissions('tests', 'test')({}).then(function (options) { - options.should.eql({context: {app: null, external: false, internal: false, public: true, user: null}}); + options.should.eql({context: {app: null, external: false, internal: false, public: true, user: null, api_key_id: null}}); done(); }).catch(done); }); @@ -617,7 +617,7 @@ describe('API Utils', function () { var aPPStub = sandbox.stub(apiUtils, 'applyPublicPermissions').returns(Promise.resolve({})); apiUtils.handlePublicPermissions('tests', 'test')({}).then(function (options) { aPPStub.calledOnce.should.eql(true); - options.should.eql({context: {app: null, external: false, internal: false, public: true, user: null}}); + options.should.eql({context: {app: null, external: false, internal: false, public: true, user: null, api_key_id: null}}); done(); }).catch(done); }); @@ -633,7 +633,7 @@ describe('API Utils', function () { apiUtils.handlePublicPermissions('tests', 'test')({context: {user: 1}}).then(function (options) { cTStub.calledOnce.should.eql(true); cTMethodStub.test.test.calledOnce.should.eql(true); - options.should.eql({context: {app: null, external: false, internal: false, public: false, user: 1}}); + options.should.eql({context: {app: null, external: false, internal: false, public: false, user: 1, api_key_id: null}}); done(); }).catch(done); }); diff --git a/core/test/unit/services/permissions/can-this_spec.js b/core/test/unit/services/permissions/can-this_spec.js index 82d7292a1c..bf508fbea8 100644 --- a/core/test/unit/services/permissions/can-this_spec.js +++ b/core/test/unit/services/permissions/can-this_spec.js @@ -423,6 +423,32 @@ describe('Permissions', function () { }); }); + describe('API Key-based permissions', function () { + // TODO change to using fake models in tests! + // Permissions need to be NOT fundamentally baked into Ghost, but a separate module, at some point + // It can depend on bookshelf, but should NOT use hard coded model knowledge. + // We use the tag model here because it doesn't have permissible, once that changes, these tests must also change + it('With permissions: can edit non-specific tag (no permissible function on model)', function (done) { + const apiKeyProviderStub = sandbox.stub(providers, 'apiKey').callsFake(() => { + // Fake the response from providers.user, which contains permissions and roles + return Promise.resolve({ + permissions: models.Permissions.forge(testUtils.DataGenerator.Content.permissions).models, + // This should be JSON, so no need to run it through the model layer. 5 === admin api key + roles: [testUtils.DataGenerator.Content.roles[5]] + }); + }); + permissions.canThis({api_key_id: 123}) // api key context + .edit + .tag({id: 1}) // tag id in model syntax + .then(function (res) { + apiKeyProviderStub.callCount.should.eql(1); + should.not.exist(res); + done(); + }) + .catch(done); + }); + }); + describe('App-based permissions (requires user as well)', function () { // @TODO: revisit this - do we really need to have USER permissions AND app permissions? it('No permissions: cannot edit tag with app only (no permissible function on model)', function (done) { @@ -553,7 +579,7 @@ describe('Permissions', function () { }) .catch(function (err) { permissibleStub.callCount.should.eql(1); - permissibleStub.firstCall.args.should.have.lengthOf(7); + permissibleStub.firstCall.args.should.have.lengthOf(8); permissibleStub.firstCall.args[0].should.eql(1); permissibleStub.firstCall.args[1].should.eql('edit'); @@ -562,6 +588,7 @@ describe('Permissions', function () { permissibleStub.firstCall.args[4].should.be.an.Object(); permissibleStub.firstCall.args[5].should.be.true(); permissibleStub.firstCall.args[6].should.be.true(); + permissibleStub.firstCall.args[7].should.be.true(); userProviderStub.callCount.should.eql(1); err.message.should.eql('Hello World!'); @@ -587,7 +614,7 @@ describe('Permissions', function () { .post({id: 1}) // tag id in model syntax .then(function (res) { permissibleStub.callCount.should.eql(1); - permissibleStub.firstCall.args.should.have.lengthOf(7); + permissibleStub.firstCall.args.should.have.lengthOf(8); permissibleStub.firstCall.args[0].should.eql(1); permissibleStub.firstCall.args[1].should.eql('edit'); permissibleStub.firstCall.args[2].should.be.an.Object(); @@ -595,6 +622,7 @@ describe('Permissions', function () { permissibleStub.firstCall.args[4].should.be.an.Object(); permissibleStub.firstCall.args[5].should.be.true(); permissibleStub.firstCall.args[6].should.be.true(); + permissibleStub.firstCall.args[7].should.be.true(); userProviderStub.callCount.should.eql(1); should.not.exist(res); diff --git a/core/test/unit/services/permissions/parse-context_spec.js b/core/test/unit/services/permissions/parse-context_spec.js index 93e43d325a..dc095fc5a0 100644 --- a/core/test/unit/services/permissions/parse-context_spec.js +++ b/core/test/unit/services/permissions/parse-context_spec.js @@ -8,6 +8,7 @@ describe('Permissions', function () { internal: false, external: false, user: null, + api_key_id: null, app: null, public: true }); @@ -15,6 +16,7 @@ describe('Permissions', function () { internal: false, external: false, user: null, + api_key_id: null, app: null, public: true }); @@ -25,6 +27,7 @@ describe('Permissions', function () { internal: false, external: false, user: null, + api_key_id: null, app: null, public: true }); @@ -32,6 +35,7 @@ describe('Permissions', function () { internal: false, external: false, user: null, + api_key_id: null, app: null, public: true }); @@ -42,6 +46,18 @@ describe('Permissions', function () { internal: false, external: false, user: 1, + api_key_id: null, + app: null, + public: false + }); + }); + + it('should return api_key_id if api_key_id provided', function () { + parseContext({api_key_id: 1}).should.eql({ + internal: false, + external: false, + user: null, + api_key_id: 1, app: null, public: false }); @@ -52,6 +68,7 @@ describe('Permissions', function () { internal: false, external: false, user: null, + api_key_id: null, app: 5, public: false }); @@ -62,6 +79,7 @@ describe('Permissions', function () { internal: true, external: false, user: null, + api_key_id: null, app: null, public: false }); @@ -70,6 +88,7 @@ describe('Permissions', function () { internal: true, external: false, user: null, + api_key_id: null, app: null, public: false }); @@ -80,6 +99,7 @@ describe('Permissions', function () { internal: false, external: true, user: null, + api_key_id: null, app: null, public: false }); @@ -88,6 +108,7 @@ describe('Permissions', function () { internal: false, external: true, user: null, + api_key_id: null, app: null, public: false }); diff --git a/core/test/unit/services/permissions/providers_spec.js b/core/test/unit/services/permissions/providers_spec.js index f2d64b557c..3e59aca58e 100644 --- a/core/test/unit/services/permissions/providers_spec.js +++ b/core/test/unit/services/permissions/providers_spec.js @@ -174,6 +174,47 @@ describe('Permission Providers', function () { }); }); + describe('API Key', function () { + it('errors if api_key cannot be found', function (done) { + let findApiKeySpy = sandbox.stub(models.ApiKey, 'findOne'); + findApiKeySpy.returns(new Promise.resolve()); + providers.apiKey(1) + .then(() => { + done(new Error('Should have thrown an api key not found error')); + }) + .catch((err) => { + findApiKeySpy.callCount.should.eql(1); + err.errorType.should.eql('NotFoundError'); + done(); + }); + }); + it('can load api_key with role, and role.permissions', function (done) { + const findApiKeySpy = sandbox.stub(models.ApiKey, 'findOne').callsFake(function () { + const fakeApiKey = models.ApiKey.forge(testUtils.DataGenerator.Content.api_keys[0]); + const fakeAdminRole = models.Role.forge(testUtils.DataGenerator.Content.roles[0]); + const fakeAdminRolePermissions = models.Permissions.forge(testUtils.DataGenerator.Content.permissions); + fakeAdminRole.relations = { + permissions: fakeAdminRolePermissions + }; + fakeApiKey.relations = { + role: fakeAdminRole + }; + fakeApiKey.withRelated = ['role', 'role.permissions']; + return Promise.resolve(fakeApiKey); + }); + providers.apiKey(1).then((res) => { + findApiKeySpy.callCount.should.eql(1); + res.should.be.an.Object().with.properties('permissions', 'roles'); + res.roles.should.be.an.Array().with.lengthOf(1); + res.permissions[0].should.be.an.Object().with.properties('attributes', 'id'); + res.roles[0].should.be.an.Object().with.properties('id', 'name', 'description'); + res.permissions[0].should.be.instanceOf(models.Base.Model); + res.roles[0].should.not.be.instanceOf(models.Base.Model); + done(); + }).catch(done); + }); + }); + describe('App', function () { // @TODO make this consistent or sane or something! // Why is this an empty array, when the success is an object?