Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] API Key auth #9869

Closed
wants to merge 38 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
9770439
integrations & api_keys tables, and 'Admin API Client' role migration
kevinansfield Sep 13, 2018
cf26130
bump secret column length for hex-formatted 512bit secrets
kevinansfield Sep 13, 2018
437e895
fix integrity spec
kevinansfield Sep 14, 2018
96479d2
add empty integration and api-key models
kevinansfield Sep 14, 2018
25aa68c
`role_id` should be `nullable` for Content API keys
kevinansfield Sep 14, 2018
7de9115
`secret` min-length should be 128 chars
kevinansfield Sep 14, 2018
7c2b2d8
generate secret by default when adding an API Key
kevinansfield Sep 14, 2018
4078318
match column lengths to match #7932
kevinansfield Sep 14, 2018
7f05458
enforce Administrator/null roles for admin/content api keys
kevinansfield Sep 14, 2018
bbe8ec0
add jsonwebtoken dependency
kevinansfield Sep 14, 2018
34fa0e4
cleanup unused requires in unit/models/api_key_spec
kevinansfield Sep 14, 2018
02b450b
WIP: API key authentication middleware
kevinansfield Sep 14, 2018
63651e0
default to `Admin API Client` role for admin keys not `Administrator`
kevinansfield Sep 14, 2018
8069fb4
fixed unreachable error mismatched API Key type
kevinansfield Sep 14, 2018
3274771
explicit error messages for admin auth
kevinansfield Sep 17, 2018
20d092a
stub api-key auth middleware tests
kevinansfield Sep 17, 2018
625af71
remove accidentally committed file
kevinansfield Sep 18, 2018
33091f6
fixed require paths in unit tests
kevinansfield Sep 18, 2018
e6cdccd
remove eslint TODO
kevinansfield Sep 18, 2018
2de7df4
get api_key id from decoded token, use buffer for secret, verify audi…
kevinansfield Sep 18, 2018
58918e1
admin key auth specs
kevinansfield Sep 18, 2018
009c39b
use valid hex value in tests
kevinansfield Sep 18, 2018
4f33d8e
remove key ID from content auth query param
kevinansfield Sep 18, 2018
f1c8272
start wiring up client auth for new v2 admin endpoints
kevinansfield Sep 18, 2018
81a869c
WIP: adapt posts route tests for v2 Admin API w/client auth
kevinansfield Sep 18, 2018
84b21aa
finish setup for running route tests
kevinansfield Sep 19, 2018
78d8f4a
add apiKey to permissions providers
kevinansfield Sep 19, 2018
b03eddb
add some todos for dealing with api key requests vs user requests
kevinansfield Sep 19, 2018
79f0665
wire up can-this and permissible functions for context.api_key
kevinansfield Sep 19, 2018
b08fb1f
return 422 BadRequestError for invalid JWT instead of 500
kevinansfield Sep 20, 2018
ad6467e
stick to common.errors.* rather than destructuring
kevinansfield Sep 20, 2018
4f60c22
fixed incorrect response code in invalid accesstoken request
kevinansfield Sep 20, 2018
aa52860
API Key requests must specify post authors
kevinansfield Sep 20, 2018
5621483
replace API with Api in method names for consistency
kevinansfield Sep 20, 2018
14007d0
fix post.permissible tests
kevinansfield Sep 20, 2018
6036f09
fix handlePublicPermissions tests
kevinansfield Sep 20, 2018
66f16f8
fix user.permissible tests
kevinansfield Sep 20, 2018
f63963d
send invites as owner if added via an admin api key
kevinansfield Sep 21, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions core/server/api/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ http = (apiMethod) => {
context: {
// @TODO: forward the client and user obj (options.context.user.id)
user: ((req.user && req.user.id) || (req.user && models.User.isExternalUser(req.user.id))) ? req.user.id : null,
api_key: (req.api_key && req.api_key.id) ? req.api_key.id : null,
client: (req.client && req.client.slug) ? req.client.slug : null,
client_id: (req.client && req.client.id) ? req.client.id : null
}
Expand Down
29 changes: 25 additions & 4 deletions core/server/api/invites.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ const invites = {
return invite.destroy(options);
})
.then(() => {
return options;
return options;
});
}

Expand Down Expand Up @@ -193,7 +193,10 @@ const invites = {
const loggedInUserRole = loggedInUser.related('roles').models[0].get('name');
let allowed = [];

if (loggedInUserRole === 'Owner' || loggedInUserRole === 'Administrator') {
let userHasAdminRole = options.context.user && (loggedInUserRole === 'Owner' || loggedInUserRole === 'Administrator');

// admin api keys have an equivalent of the Adminstrator role
if (options.context.api_key || userHasAdminRole) {
allowed = ['Administrator', 'Editor', 'Author', 'Contributor'];
} else if (loggedInUserRole === 'Editor') {
allowed = ['Author', 'Contributor'];
Expand All @@ -205,7 +208,7 @@ const invites = {
}));
}
}).then(() => {
return options;
return options;
});
}

Expand Down Expand Up @@ -234,11 +237,29 @@ const invites = {
});
}

function fetchOwner(options) {
return models.User.getOwnerUser(merge({}, omit(options, 'data'), {withRelated: ['roles']}))
.then((owner) => {
loggedInUser = owner;
return options;
});
}

// API Key requests are not tied to a user so send the invite from the
// owner user instead
function fetchLoggedInUserOrOwner(options) {
if (options.context.api_key && !options.context.user) {
return fetchOwner(options);
}

return fetchLoggedInUser(options);
}

tasks = [
localUtils.validate(docName, {opts: ['email']}),
localUtils.convertOptions(allowedIncludes),
localUtils.handlePermissions(docName, 'add'),
fetchLoggedInUser,
fetchLoggedInUserOrOwner,
validation,
checkIfUserExists,
destroyOldInvite,
Expand Down
1 change: 1 addition & 0 deletions core/server/api/mail.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ const apiMail = {
* @param {Object} options required property 'to' which contains the recipient address
* @returns {Promise}
*/
// TODO: ensure this can't be triggered by API Key requests
sendTest(options) {
let tasks;

Expand Down
3 changes: 2 additions & 1 deletion core/server/api/posts.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ posts = {

// Workaround to remove static pages from results
// TODO: rework after https://github.com/TryGhost/Ghost/issues/5151
if (options && options.context && (options.context.user || options.context.internal)) {
// TODO: always allow staticPages for v2 Admin API requests?
if (options && options.context && (options.context.user || options.context.api_key || options.context.internal)) {
extraOptions.push('staticPages');
}
permittedOptions = localUtils.browseDefaultOptions.concat(extraOptions);
Expand Down
1 change: 1 addition & 0 deletions core/server/api/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ users = {
* @param {Object} options
* @returns {Object} options
*/
// TODO: update for API Key requests which won't have options.context.user
function handlePermissions(options) {
if (options.id === 'me' && options.context && options.context.user) {
options.id = options.context.user;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
const commands = require('../../../schema').commands;
const logging = require('../../../../lib/common/logging');
const merge = require('lodash/merge');
const models = require('../../../../models');
const utils = require('../../../schema/fixtures/utils');
const _private = {};

_private.createTable = function createTable(table, message, options) {
return options.connection.schema.hasTable(table)
.then((exists) => {
if (exists) {
logging.warn(message);
return Promise.resolve();
}

logging.info(message);
return commands.createTable(table, options.connection);
});
};

_private.addIntegrationsTable = function addIntegrationsTable(options) {
let table = 'integrations';
let message = 'Adding "Integrations" table';

return _private.createTable(table, message, options);
};

_private.addApiKeysTable = function addApiKeysTable(options) {
let table = 'api_keys';
let message = 'Adding "API Keys" table';

return _private.createTable(table, message, options);
};

_private.addApiKeyRole = function addApiKeyRole(options) {
let message = 'Adding "Admin API Client" role to roles table';
let apiKeyRole = utils.findModelFixtureEntry('Role', {name: 'Admin API Client'});

return models.Role.findOne({name: apiKeyRole.name}, options)
.then((role) => {
if (!role) {
logging.info(message);
return utils.addFixturesForModel({
name: 'Role',
entries: [apiKeyRole]
}, options);
}

logging.warn(message);
});
};

_private.addApiKeyPermissions = function addApiKeyPermissions(options) {
let message = 'Adding permissions_roles fixtures for the admin_api_key role';
let relations = utils.findRelationFixture('Role', 'Permission');

return utils.addFixturesForRelation({
from: relations.from,
to: relations.to,
entries: {
'Admin API Client': relations.entries['Admin API Client']
}
}, options).then((result) => {
if (result.done === result.expected) {
logging.info(message);
}

logging.warn(`(${result.done}/${result.expected}) ${message}`);
});
};

module.exports.up = function setupIntegrations(options) {
let localOptions = merge({
context: {internal: true}
}, options);

return _private.addIntegrationsTable(localOptions)
.then(() => _private.addApiKeysTable(localOptions))
.then(() => _private.addApiKeyRole(localOptions))
.then(() => _private.addApiKeyPermissions(localOptions));
};
92 changes: 82 additions & 10 deletions core/server/data/schema/fixtures/fixtures.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,24 +44,28 @@
"name": "Role",
"entries": [
{
"name": "Administrator",
"description": "Administrators"
"name": "Administrator",
"description": "Administrators"
},
{
"name": "Editor",
"description": "Editors"
"name": "Editor",
"description": "Editors"
},
{
"name": "Author",
"description": "Authors"
"name": "Author",
"description": "Authors"
},
{
"name": "Contributor",
"description": "Contributors"
"name": "Contributor",
"description": "Contributors"
},
{
"name": "Owner",
"description": "Blog Owner"
"name": "Owner",
"description": "Blog Owner"
},
{
"name": "Admin API Client",
"description": "External Apps"
}
]
},
Expand Down Expand Up @@ -332,6 +336,56 @@
"name": "Delete webhooks",
"action_type": "destroy",
"object_type": "webhook"
},
{
"name": "Browse integrations",
"action_type": "browse",
"object_type": "integration"
},
{
"name": "Read integrations",
"action_type": "read",
"object_type": "integration"
},
{
"name": "Edit integrations",
"action_type": "edit",
"object_type": "integration"
},
{
"name": "Add integrations",
"action_type": "add",
"object_type": "integration"
},
{
"name": "Delete integrations",
"action_type": "destroy",
"object_type": "integration"
},
{
"name": "Browse API keys",
"action_type": "browse",
"object_type": "api_key"
},
{
"name": "Read API keys",
"action_type": "read",
"object_type": "api_key"
},
{
"name": "Edit API keys",
"action_type": "edit",
"object_type": "api_key"
},
{
"name": "Add API keys",
"action_type": "add",
"object_type": "api_key"
},
{
"name": "Delete API keys",
"action_type": "destroy",
"object_type": "api_key"
}
]
},
Expand Down Expand Up @@ -485,6 +539,24 @@
"entries": {
"Administrator": {
"db": "all",
"mail": "all",
"notification": "all",
"post": "all",
"setting": "all",
"slug": "all",
"tag": "all",
"theme": "all",
"user": "all",
"role": "all",
"client": "all",
"subscriber": "all",
"invite": "all",
"redirect": "all",
"webhook": "all",
"integration": "all",
"api_key": "all"
},
"Admin API Client": {
"mail": "all",
"notification": "all",
"post": "all",
Expand Down
29 changes: 29 additions & 0 deletions core/server/data/schema/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -300,5 +300,34 @@ module.exports = {
created_by: {type: 'string', maxlength: 24, nullable: false},
updated_at: {type: 'dateTime', nullable: true},
updated_by: {type: 'string', maxlength: 24, nullable: true}
},
integrations: {
id: {type: 'string', maxlength: 24, nullable: false, primary: true},
name: {type: 'string', maxlength: 191, nullable: false},
slug: {type: 'string', maxlength: 191, nullable: false, unique: true},
icon: {type: 'string', maxlength: 2000, nullable: true},
description: {type: 'string', maxlength: 2000, nullable: true},
created_at: {type: 'dateTime', nullable: false},
created_by: {type: 'string', maxlength: 24, nullable: false},
updated_at: {type: 'dateTime', nullable: true},
updated_by: {type: 'string', maxlength: 24, nullable: true}
},
api_keys: {
allouis marked this conversation as resolved.
Show resolved Hide resolved
id: {type: 'string', maxlength: 24, nullable: false, primary: true},
type: {
type: 'string',
maxlength: 50,
kevinansfield marked this conversation as resolved.
Show resolved Hide resolved
nullable: false,
validations: {isIn: [['content', 'admin']]}
},
secret: {type: 'string', maxlength: 191, nullable: false, unique: true, validations: {isLength: {min: 128}}},
role_id: {type: 'string', maxlength: 24, nullable: true},
integration_id: {type: 'string', maxlength: 24, nullable: true},
last_seen_at: {type: 'dateTime', nullable: true},
last_seen_version: {type: 'string', maxlength: 50, nullable: true},
created_at: {type: 'dateTime', nullable: false},
created_by: {type: 'string', maxlength: 24, nullable: false},
updated_at: {type: 'dateTime', nullable: true},
updated_by: {type: 'string', maxlength: 24, nullable: true}
}
};
63 changes: 63 additions & 0 deletions core/server/models/api-key.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
const crypto = require('crypto');
const ghostBookshelf = require('./base');
const {Role} = require('./role');

const ApiKey = ghostBookshelf.Model.extend({
tableName: 'api_keys',

defaults() {
// 512bit key for HS256 JWT signing
let secret = crypto.randomBytes(64).toString('hex');

return {
secret
};
},

role() {
return this.belongsTo('Role');
},

integration() {
return this.belongsTo('Integration');
},

onSaving(/* model, attrs, options */) {
let tasks = [];

ghostBookshelf.Model.prototype.onSaving.apply(this, arguments);

// enforce roles which are currently hardcoded
// - admin key = Adminstrator role
// - content key = no role
if (this.hasChanged('type')) {
if (this.get('type') === 'admin') {
tasks.setAdminRole = Role.findOne({name: 'Admin API Client'}, {columns: ['id']})
.then((role) => {
this.set('role_id', role.get('id'));
});
}

if (this.get('type') === 'content') {
this.set('role_id', null);
}
}

return Promise.props(tasks);
}
}, {
add(data, unfilteredOptions) {
const options = ApiKey.filterOptions(unfilteredOptions, 'add');

return ghostBookshelf.Model.add.call(this, data, options);
}
});

const ApiKeys = ghostBookshelf.Collection.extend({
model: ApiKey
});

module.exports = {
ApiKey: ghostBookshelf.model('ApiKey', ApiKey),
ApiKeys: ghostBookshelf.collection('ApiKeys', ApiKeys)
};
Loading