Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-04-08 02:52:39 -05:00
Code Issues Activity

Changed context.api_key_id to an object containing key type information

Browse source 
refs #9865

- Changed id passed for api_key to an object to be able to differenciate between admin and content api requests
- Added integration id to frame context
- Small refactoring of frame context initialization
This commit is contained in:
Nazar Gargol 2019-01-24 14:19:34 +00:00 • committed by Naz Gargol
parent 8ba3a91387
commit 6318b65cab
12 changed files with 121 additions and 50 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

21
core/server/api/shared/http.js
View file

@ -6,6 +6,22 @@ const http = (apiImpl) => {
return (req, res, next) => {
debug('request');
let apiKey = null;
let integration = null;
let user = null;
if (req.api_key) {
apiKey = {
id: req.api_key.get('id'),
type: req.api_key.get('type')
};
integration = req.api_key.get('integration_id');
}
if ((req.user && req.user.id) || (req.user && models.User.isExternalUser(req.user.id))) {
user = req.user.id;
}
const frame = new shared.Frame({
body: req.body,
file: req.file,
@ -14,8 +30,9 @@ const http = (apiImpl) => {
params: req.params,
user: req.user,
context: {
api_key_id: (req.api_key && req.api_key.id) ? req.api_key.id : null,
user: ((req.user && req.user.id) || (req.user && models.User.isExternalUser(req.user.id))) ? req.user.id : null,
api_key: apiKey,
user: user,
integration: integration,
member: (req.member || null)
}
});

2
core/server/api/v2/utils/index.js
View file

@ -17,7 +17,7 @@ module.exports = {
*/
isContentAPI: (frame) => {
return !!(Object.keys(frame.options.context).length === 0 ||
(!frame.options.context.user && frame.options.context.api_key_id) ||
(!frame.options.context.user && frame.options.context.api_key && (frame.options.context.api_key.type === 'content')) ||
frame.options.context.public
);
}

6
core/server/api/v2/utils/serializers/input/posts.js
View file

@ -37,11 +37,10 @@ module.exports = {
browse(apiConfig, frame) {
debug('browse');
// @TODO: `api_key_id` does not work long term, because it can be either a content or an admin api key?
/**
* ## current cases:
* - context object is empty (functional call, content api access)
* - api_key_id exists? content api access
* - api_key.type == 'content' ? content api access
* - user exists? admin api access
*/
if (utils.isContentAPI(frame)) {
@ -76,11 +75,10 @@ module.exports = {
read(apiConfig, frame) {
debug('read');
// @TODO: `api_key_id` does not work long term, because it can be either a content or an admin api key?
/**
* ## current cases:
* - context object is empty (functional call, content api access)
* - api_key_id exists? content api access
* - api_key.type == 'content' ? content api access
* - user exists? admin api access
*/
if (utils.isContentAPI(frame)) {

2
core/server/models/base/index.js
View file

@ -423,7 +423,7 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
* use ID '1'. This logic exists for a LONG while now. The owner ID only changes from '1' to something else,
* if you transfer ownership.
*
* @TODO: Update this code section as soon as we have decided between `context.api_key_id` and `context.integration`
* @TODO: Update this code section as soon as we have decided between `context.api_key` and `context.integration`
*/
return ghostBookshelf.Model.internalUser;
} else if (options.context.internal) {

8
core/server/services/permissions/can-this.js
View file

@ -124,7 +124,7 @@ CanThisResult.prototype.beginCheck = function (context) {
appPermissionLoad,
permissionsLoad;
// Get context.user, context.api_key_id and context.app
// Get context.user, context.api_key and context.app
context = parseContext(context);
if (actionsMap.empty()) {
@ -140,10 +140,10 @@ CanThisResult.prototype.beginCheck = function (context) {
}
// Kick off loading of api key permissions if necessary
if (context.api_key_id) {
apiKeyPermissionLoad = providers.apiKey(context.api_key_id);
if (context.api_key) {
apiKeyPermissionLoad = providers.apiKey(context.api_key.id);
} else {
// Resolve null if no context.api_key_id
// Resolve null if no context.api_key
apiKeyPermissionLoad = Promise.resolve(null);
}

12
core/server/services/permissions/parse-context.js
View file

@ -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, api_key_id: ObjectId|null}}
* @return {{internal: boolean, external: boolean, user: integer|null, app: integer|null, public: boolean, api_key: Object|null}}
*/
module.exports = function parseContext(context) {
// Parse what's passed to canThis.beginCheck for standard user and app scopes
@ -11,7 +11,7 @@ module.exports = function parseContext(context) {
internal: false,
external: false,
user: null,
api_key_id: null,
api_key: null,
app: null,
public: true
};
@ -32,11 +32,9 @@ module.exports = function parseContext(context) {
parsed.public = false;
}
// @TODO: content & admin use `parseContext`, there is no differentiation between them.
// e.g. context.admin_api_key and context.content_api_key or context.type = admin | content
if (context && context.api_key_id) {
parsed.api_key_id = context.api_key_id;
// parsed.public = false;
if (context && context.api_key) {
parsed.api_key = context.api_key;
parsed.public = (context.api_key.type === 'content');
}
if (context && context.app) {

2
core/test/unit/api/shared/http_spec.js
View file

@ -44,7 +44,7 @@ describe('Unit: api/shared/http', function () {
apiImpl.args[0][0].data.should.eql({a: 'a'});
apiImpl.args[0][0].options.should.eql({
context: {
api_key_id: null,
api_key: null,
user: null,
member: null
}

6
core/test/unit/api/v0.1/utils_spec.js
View file

@ -606,7 +606,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, api_key_id: null}});
options.should.eql({context: {app: null, external: false, internal: false, public: true, user: null, api_key: null}});
done();
}).catch(done);
});
@ -615,7 +615,7 @@ describe('API Utils', function () {
var aPPStub = sinon.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, api_key_id: null}});
options.should.eql({context: {app: null, external: false, internal: false, public: true, user: null, api_key: null}});
done();
}).catch(done);
});
@ -631,7 +631,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, api_key_id: null}});
options.should.eql({context: {app: null, external: false, internal: false, public: false, user: 1, api_key: null}});
done();
}).catch(done);
});

14
core/test/unit/api/v2/index_spec.js
View file

@ -7,23 +7,29 @@ describe('Unit: v2/utils/index', function () {
sinon.restore();
});
describe('isContentAPI', function () {
it('is truthy when having api key and no user', function () {
it('is truthy when having api key of content type', function () {
const frame = {
options: {
context: {
api_key_id: 'api_key'
api_key: {
id: 'keyId',
type: 'content'
}
}
}
};
should(utils.isContentAPI(frame)).equal(true);
});
it('is falsy when having api key and a user', function () {
it.only('is falsy when having api key and a user', function () {
const frame = {
options: {
context: {
user: {},
api_key_id: 'api_key'
api_key: {
id: 'keyId',
type: 'content'
}
}
}
};

45
core/test/unit/api/v2/utils/serializers/input/posts_spec.js
View file

@ -10,7 +10,10 @@ describe('Unit: v2/utils/serializers/input/posts', function () {
options: {
context: {
user: 0,
api_key_id: 1
api_key: {
id: 1,
type: 'content'
},
}
}
};
@ -39,7 +42,10 @@ describe('Unit: v2/utils/serializers/input/posts', function () {
options: {
context: {
user: 0,
api_key_id: 1
api_key: {
id: 1,
type: 'content'
},
},
filter: 'status:published+tag:eins'
}
@ -55,7 +61,10 @@ describe('Unit: v2/utils/serializers/input/posts', function () {
options: {
context: {
user: 0,
api_key_id: 1
api_key: {
id: 1,
type: 'content'
},
},
filter: 'page:true+tag:eins'
}
@ -71,7 +80,10 @@ describe('Unit: v2/utils/serializers/input/posts', function () {
options: {
context: {
user: 0,
api_key_id: 1
api_key: {
id: 1,
type: 'content'
},
},
filter: 'page:true'
}
@ -87,7 +99,10 @@ describe('Unit: v2/utils/serializers/input/posts', function () {
options: {
context: {
user: 0,
api_key_id: 1
api_key: {
id: 1,
type: 'content'
},
},
filter: '(page:true,page:false)'
}
@ -120,7 +135,10 @@ describe('Unit: v2/utils/serializers/input/posts', function () {
options: {
context: {
user: 0,
api_key_id: 1
api_key: {
id: 1,
type: 'content'
},
}
},
data: {}
@ -136,7 +154,10 @@ describe('Unit: v2/utils/serializers/input/posts', function () {
options: {
context: {
user: 0,
api_key_id: 1
api_key: {
id: 1,
type: 'content'
},
}
},
data: {
@ -214,7 +235,10 @@ describe('Unit: v2/utils/serializers/input/posts', function () {
options: {
context: {
user: 0,
api_key_id: 1
api_key: {
id: 1,
type: 'content'
},
},
withRelated: ['tags', 'authors']
},
@ -254,7 +278,10 @@ describe('Unit: v2/utils/serializers/input/posts', function () {
options: {
context: {
user: 0,
api_key_id: 1
api_key: {
id: 1,
type: 'content'
},
},
withRelated: ['tags', 'authors']
},

4
core/test/unit/services/permissions/can-this_spec.js
View file

@ -435,7 +435,9 @@ describe('Permissions', function () {
roles: [testUtils.DataGenerator.Content.roles[5]]
});
});
permissions.canThis({api_key_id: 123}) // api key context
permissions.canThis({api_key: {
id: 123
}}) // api key context
.edit
.tag({id: 1}) // tag id in model syntax
.then(function (res) {

49
core/test/unit/services/permissions/parse-context_spec.js
View file

@ -8,7 +8,7 @@ describe('Permissions', function () {
internal: false,
external: false,
user: null,
api_key_id: null,
api_key: null,
app: null,
public: true
});
@ -16,7 +16,7 @@ describe('Permissions', function () {
internal: false,
external: false,
user: null,
api_key_id: null,
api_key: null,
app: null,
public: true
});
@ -27,7 +27,7 @@ describe('Permissions', function () {
internal: false,
external: false,
user: null,
api_key_id: null,
api_key: null,
app: null,
public: true
});
@ -35,7 +35,7 @@ describe('Permissions', function () {
internal: false,
external: false,
user: null,
api_key_id: null,
api_key: null,
app: null,
public: true
});
@ -46,29 +46,52 @@ describe('Permissions', function () {
internal: false,
external: false,
user: 1,
api_key_id: null,
api_key: null,
app: null,
public: false
});
});
it('should return api_key_id if api_key_id provided', function () {
parseContext({api_key_id: 1}).should.eql({
it('should return api_key and public context if content api_key provided', function () {
parseContext({api_key: {
id: 1,
type: 'content'
}}).should.eql({
internal: false,
external: false,
user: null,
api_key_id: 1,
api_key: {
id: 1,
type: 'content'
},
app: null,
public: true
});
});
it('should return api_key and non public context if admin api_key provided', function () {
parseContext({api_key: {
id: 1,
type: 'admin'
}}).should.eql({
internal: false,
external: false,
user: null,
api_key: {
id: 1,
type: 'admin'
},
app: null,
public: false
});
});
it('should return app if app populated', function () {
parseContext({app: 5}).should.eql({
internal: false,
external: false,
user: null,
api_key_id: null,
api_key: null,
app: 5,
public: false
});
@ -79,7 +102,7 @@ describe('Permissions', function () {
internal: true,
external: false,
user: null,
api_key_id: null,
api_key: null,
app: null,
public: false
});
@ -88,7 +111,7 @@ describe('Permissions', function () {
internal: true,
external: false,
user: null,
api_key_id: null,
api_key: null,
app: null,
public: false
});
@ -99,7 +122,7 @@ describe('Permissions', function () {
internal: false,
external: true,
user: null,
api_key_id: null,
api_key: null,
app: null,
public: false
});
@ -108,7 +131,7 @@ describe('Permissions', function () {
internal: false,
external: true,
user: null,
api_key_id: null,
api_key: null,
app: null,
public: false
});