Skip to content

Commit

Permalink
Add dynamic client_id/client_secret
Browse files Browse the repository at this point in the history
no issue
- added ghost-admin client_id to admin
- added ghost-admin client_secret to admin
- added client.read() api endpoint
- added random generation of client_secret to migration
- removed addClientSecret method
- updated tests
  • Loading branch information
sebgie authored and ErisDS committed Sep 2, 2015
1 parent 6926e20 commit f22796f
Show file tree
Hide file tree
Showing 12 changed files with 93 additions and 68 deletions.
5 changes: 4 additions & 1 deletion core/client/app/authenticators/oauth2.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import Ember from 'ember';
import Authenticator from 'simple-auth-oauth2/authenticators/oauth2';

export default Authenticator.extend({
config: Ember.inject.service(),
makeRequest: function (url, data) {
data.client_id = 'ghost-admin';
data.client_id = this.get('config.clientId');
data.client_secret = this.get('config.clientSecret');
return this._super(url, data);
}
});
60 changes: 60 additions & 0 deletions core/server/api/clients.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// # Client API
// RESTful API for the Client resource
var Promise = require('bluebird'),
_ = require('lodash'),
dataProvider = require('../models'),
errors = require('../errors'),
utils = require('./utils'),
pipeline = require('../utils/pipeline'),

docName = 'clients',
clients;

/**
* ### Clients API Methods
*
* **See:** [API Methods](index.js.html#api%20methods)
*/
clients = {

/**
* ## Read
* @param {{id}} options
* @return {Promise<Client>} Client
*/
read: function read(options) {
var attrs = ['id', 'slug'],
tasks;

/**
* ### Model Query
* Make the call to the Model layer
* @param {Object} options
* @returns {Object} options
*/
function doQuery(options) {
// only User Agent (type = `ua`) clients are available at the moment.
options.data = _.extend(options.data, {type: 'ua'});
return dataProvider.Client.findOne(options.data, _.omit(options, ['data']));
}

// Push all of our tasks into a `tasks` array in the correct order
tasks = [
utils.validate(docName, {attrs: attrs}),
// TODO: add permissions
// utils.handlePublicPermissions(docName, 'read'),
doQuery
];

// Pipeline calls each task passing the result of one to be the arguments for the next
return pipeline(tasks, options).then(function formatResponse(result) {
if (result) {
return {clients: [result.toJSON(options)]};
}

return Promise.reject(new errors.NotFoundError('Client not found.'));
});
}
};

module.exports = clients;
2 changes: 2 additions & 0 deletions core/server/api/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ var _ = require('lodash'),
roles = require('./roles'),
settings = require('./settings'),
tags = require('./tags'),
clients = require('./clients'),
themes = require('./themes'),
users = require('./users'),
slugs = require('./slugs'),
Expand Down Expand Up @@ -233,6 +234,7 @@ module.exports = {
roles: roles,
settings: settings,
tags: tags,
clients: clients,
themes: themes,
users: users,
slugs: slugs,
Expand Down
2 changes: 1 addition & 1 deletion core/server/api/tags.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ var Promise = require('bluebird'),
dataProvider = require('../models'),
errors = require('../errors'),
utils = require('./utils'),
pipeline = require('../utils/pipeline'),
pipeline = require('../utils/pipeline'),

docName = 'tags',
allowedIncludes = ['post_count'],
Expand Down
10 changes: 9 additions & 1 deletion core/server/controllers/admin.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,16 @@ adminControllers = {
/*jslint unparam:true*/

function renderIndex() {
var configuration;
return api.configuration.browse().then(function then(data) {
var apiConfig = _.omit(data.configuration, function omit(value) {
configuration = data.configuration;
}).then(function getAPIClient() {
return api.clients.read({slug: 'ghost-admin'});
}).then(function renderIndex(adminClient) {
configuration.push({key: 'clientId', value: adminClient.clients[0].slug});
configuration.push({key: 'clientSecret', value: adminClient.clients[0].secret});

var apiConfig = _.omit(configuration, function omit(value) {
return _.contains(['environment', 'database', 'mail', 'version'], value.key);
});

Expand Down
9 changes: 7 additions & 2 deletions core/server/data/fixtures/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
// making them.

var Promise = require('bluebird'),
crypto = require('crypto'),
sequence = require('../../utils/sequence'),
_ = require('lodash'),
errors = require('../../errors'),
Expand Down Expand Up @@ -224,7 +225,9 @@ to004 = function to004() {
upgradeOp = models.Client.findOne({slug: fixtures.clients[0].slug}).then(function (client) {
if (client) {
logInfo('Update ghost-admin client fixture');
return models.Client.edit(fixtures.clients[0], _.extend({}, options, {id: client.id}));
var adminClient = fixtures.clients[0];
adminClient.secret = crypto.randomBytes(6).toString('hex');
return models.Client.edit(adminClient, _.extend({}, options, {id: client.id}));
}
return Promise.resolve();
});
Expand All @@ -234,7 +237,9 @@ to004 = function to004() {
upgradeOp = models.Client.findOne({slug: fixtures.clients[1].slug}).then(function (client) {
if (!client) {
logInfo('Add ghost-frontend client fixture');
return models.Client.add(fixtures.clients[1], options);
var frontendClient = fixtures.clients[1];
frontendClient.secret = crypto.randomBytes(6).toString('hex');
return models.Client.add(frontendClient, options);
}
return Promise.resolve();
});
Expand Down
10 changes: 0 additions & 10 deletions core/server/middleware/client-auth.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
var passport = require('passport'),
_ = require('lodash'),
oauthServer,

clientAuth;
Expand All @@ -9,15 +8,6 @@ function cacheOauthServer(server) {
}

clientAuth = {
// work around to handle missing client_secret
// oauth2orize needs it, but untrusted clients don't have it
addClientSecret: function addClientSecret(req, res, next) {
if (_.isEmpty(req.body.client_secret)) {
req.body.client_secret = 'not_available';
}
next();
},

// ### Authenticate Client Middleware
// authenticate client that is asking for an access token
authenticateClient: function authenticateClient(req, res, next) {
Expand Down
1 change: 0 additions & 1 deletion core/server/middleware/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ middleware = {
spamPrevention: spamPrevention,
privateBlogging: privateBlogging,
api: {
addClientSecret: clientAuth.addClientSecret,
cacheOauthServer: clientAuth.cacheOauthServer,
authenticateClient: clientAuth.authenticateClient,
generateAccessToken: clientAuth.generateAccessToken,
Expand Down
4 changes: 3 additions & 1 deletion core/server/routes/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ apiRoutes = function apiRoutes(middleware) {
// ## Roles
router.get('/roles/', api.http(api.roles.browse));

// ## Clients
router.get('/clients/slug/:slug', api.http(api.clients.read));

// ## Slugs
router.get('/slugs/:type/:name', api.http(api.slugs.generate));

Expand Down Expand Up @@ -81,7 +84,6 @@ apiRoutes = function apiRoutes(middleware) {
router.get('/authentication/setup', api.http(api.authentication.isSetup));
router.post('/authentication/token',
middleware.spamPrevention.signin,
middleware.api.addClientSecret,
middleware.api.authenticateClient,
middleware.api.generateAccessToken
);
Expand Down
12 changes: 6 additions & 6 deletions core/test/functional/routes/api/authentication_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe('Authentication API', function () {

it('can authenticate', function (done) {
request.post(testUtils.API.getApiQuery('authentication/token'))
.send({grant_type: 'password', username: user.email, password: user.password, client_id: 'ghost-admin'})
.send({grant_type: 'password', username: user.email, password: user.password, client_id: 'ghost-admin', client_secret: 'not_available'})
.expect('Content-Type', /json/)
// TODO: make it possible to override oauth2orize's header so that this is consistent
.expect('Cache-Control', 'no-store')
Expand All @@ -52,7 +52,7 @@ describe('Authentication API', function () {

it('can\'t authenticate unknown user', function (done) {
request.post(testUtils.API.getApiQuery('authentication/token'))
.send({grant_type: 'password', username: '[email protected]', password: user.password, client_id: 'ghost-admin'})
.send({grant_type: 'password', username: '[email protected]', password: user.password, client_id: 'ghost-admin', client_secret: 'not_available'})
.expect('Content-Type', /json/)
.expect('Cache-Control', testUtils.cacheRules['private'])
.expect(404)
Expand All @@ -69,7 +69,7 @@ describe('Authentication API', function () {

it('can\'t authenticate invalid password user', function (done) {
request.post(testUtils.API.getApiQuery('authentication/token'))
.send({grant_type: 'password', username: user.email, password: 'invalid', client_id: 'ghost-admin'})
.send({grant_type: 'password', username: user.email, password: 'invalid', client_id: 'ghost-admin', client_secret: 'not_available'})
.expect('Content-Type', /json/)
.expect('Cache-Control', testUtils.cacheRules['private'])
.expect(401)
Expand All @@ -86,7 +86,7 @@ describe('Authentication API', function () {

it('can request new access token', function (done) {
request.post(testUtils.API.getApiQuery('authentication/token'))
.send({grant_type: 'password', username: user.email, password: user.password, client_id: 'ghost-admin'})
.send({grant_type: 'password', username: user.email, password: user.password, client_id: 'ghost-admin', client_secret: 'not_available'})
.expect('Content-Type', /json/)
// TODO: make it possible to override oauth2orize's header so that this is consistent
.expect('Cache-Control', 'no-store')
Expand All @@ -97,7 +97,7 @@ describe('Authentication API', function () {
}
var refreshToken = res.body.refresh_token;
request.post(testUtils.API.getApiQuery('authentication/token'))
.send({grant_type: 'refresh_token', refresh_token: refreshToken, client_id: 'ghost-admin'})
.send({grant_type: 'refresh_token', refresh_token: refreshToken, client_id: 'ghost-admin', client_secret: 'not_available'})
.expect('Content-Type', /json/)
// TODO: make it possible to override oauth2orize's header so that this is consistent
.expect('Cache-Control', 'no-store')
Expand All @@ -116,7 +116,7 @@ describe('Authentication API', function () {

it('can\'t request new access token with invalid refresh token', function (done) {
request.post(testUtils.API.getApiQuery('authentication/token'))
.send({grant_type: 'refresh_token', refresh_token: 'invalid', client_id: 'ghost-admin'})
.send({grant_type: 'refresh_token', refresh_token: 'invalid', client_id: 'ghost-admin', client_secret: 'not_available'})
.expect('Content-Type', /json/)
.expect('Cache-Control', testUtils.cacheRules['private'])
.expect(403)
Expand Down
44 changes: 0 additions & 44 deletions core/test/unit/middleware/client-auth_spec.js

This file was deleted.

2 changes: 1 addition & 1 deletion core/test/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ login = function login(request) {

return new Promise(function (resolve, reject) {
request.post('/ghost/api/v0.1/authentication/token/')
.send({grant_type: 'password', username: user.email, password: user.password, client_id: 'ghost-admin'})
.send({grant_type: 'password', username: user.email, password: user.password, client_id: 'ghost-admin', client_secret: 'not_available'})
.end(function (err, res) {
if (err) {
return reject(err);
Expand Down

0 comments on commit f22796f

Please sign in to comment.