Skip to content

Commit

Permalink
馃悰 fix cors middleware (#8066)
Browse files Browse the repository at this point in the history
no issue
- previously we have reordered the cors middleware, see 0414b9a
- but cors middleware MUST happen after client authentication
  • Loading branch information
kirrg001 authored and kevinansfield committed Mar 1, 2017
1 parent a109bfb commit 8b3e30f
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 12 deletions.
20 changes: 11 additions & 9 deletions core/server/routes/api.js
Expand Up @@ -3,31 +3,33 @@ var express = require('express'),
api = require('../api'),
apiRoutes;

/**
* IMPORTANT
* - cors middleware MUST happen before pretty urls, because otherwise cors header can get lost
* - cors middleware MUST happen after authenticateClient, because authenticateClient reads the trusted domains
*/
apiRoutes = function apiRoutes(middleware) {
var router = express.Router(),
// Authentication for public endpoints
authenticatePublic = [
middleware.api.authenticateClient,
middleware.api.authenticateUser,
middleware.api.requiresAuthorizedUserPublicAPI
middleware.api.requiresAuthorizedUserPublicAPI,
middleware.api.cors,
middleware.api.prettyUrls
],
// Require user for private endpoints
authenticatePrivate = [
middleware.api.authenticateClient,
middleware.api.authenticateUser,
middleware.api.requiresAuthorizedUser
middleware.api.requiresAuthorizedUser,
middleware.api.cors,
middleware.api.prettyUrls
];

// alias delete with del
router.del = router.delete;

/**
* cors middleware need's to be registered before pretty urls
* otherwise the cors header information get's lost when a redirect happens
*/
router.use(middleware.api.cors);
router.use(middleware.api.prettyUrls);

// send 503 json response in case of maintenance
router.use(middleware.api.maintenance);

Expand Down
32 changes: 31 additions & 1 deletion core/test/functional/routes/api/public_api_spec.js
Expand Up @@ -20,7 +20,7 @@ describe('Public API', function () {
ghost().then(function (ghostServer) {
request = supertest.agent(ghostServer.rootApp);
}).then(function () {
return testUtils.doAuth(request, 'posts', 'tags');
return testUtils.doAuth(request, 'posts', 'tags', 'trusted_domains');
}).then(function (token) {
// enable public API
request.put(testUtils.API.getApiQuery('settings/'))
Expand Down Expand Up @@ -55,7 +55,37 @@ describe('Public API', function () {
return done(err);
}

res.headers.vary.should.eql('Origin, Accept-Encoding');
should.exist(res.headers['access-control-allow-origin']);
should.not.exist(res.headers['x-cache-invalidate']);

var jsonResponse = res.body;
should.exist(jsonResponse.posts);
testUtils.API.checkResponse(jsonResponse, 'posts');
jsonResponse.posts.should.have.length(5);
testUtils.API.checkResponse(jsonResponse.posts[0], 'post');
testUtils.API.checkResponse(jsonResponse.meta.pagination, 'pagination');
_.isBoolean(jsonResponse.posts[0].featured).should.eql(true);
_.isBoolean(jsonResponse.posts[0].page).should.eql(true);
done();
});
});

it('browse posts from different origin', function (done) {
request.get(testUtils.API.getApiQuery('posts/?client_id=ghost-admin&client_secret=not_available'))
.set('Origin', 'https://example.com')
.expect('Content-Type', /json/)
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(200)
.end(function (err, res) {
if (err) {
return done(err);
}

res.headers.vary.should.eql('Origin, Accept-Encoding');
should.exist(res.headers['access-control-allow-origin']);
should.not.exist(res.headers['x-cache-invalidate']);

var jsonResponse = res.body;
should.exist(jsonResponse.posts);
testUtils.API.checkResponse(jsonResponse, 'posts');
Expand Down
12 changes: 11 additions & 1 deletion core/test/utils/fixtures/data-generator.js
Expand Up @@ -265,7 +265,8 @@ DataGenerator.forKnex = (function () {
roles,
users,
roles_users,
clients;
clients,
trustedDomains;

function createBasic(overrides) {
var newObj = _.cloneDeep(overrides);
Expand Down Expand Up @@ -431,6 +432,14 @@ DataGenerator.forKnex = (function () {
createClient({name: 'Ghost Scheduler', slug: 'ghost-scheduler', type: 'web'})
];

trustedDomains = [
{
uuid: '117084d0-d660-11e6-8b46-f7d235f120db',
client_id: 1,
trusted_domain: 'https://example.com'
}
];

roles_users = [
{user_id: 1, role_id: 4},
{user_id: 2, role_id: 1},
Expand Down Expand Up @@ -475,6 +484,7 @@ DataGenerator.forKnex = (function () {
createToken: createToken,
createSubscriber: createBasic,

trustedDomains: trustedDomains,
posts: posts,
tags: tags,
posts_tags: posts_tags,
Expand Down
9 changes: 8 additions & 1 deletion core/test/utils/index.js
Expand Up @@ -379,6 +379,10 @@ fixtures = {
});
},

insertTrustedDomains: function insertTrustedDomains() {
return db.knex('client_trusted_domains').insert(DataGenerator.forKnex.trustedDomains);
},

insertClients: function insertClients() {
return db.knex('clients').insert(DataGenerator.forKnex.clients);
},
Expand Down Expand Up @@ -436,7 +440,10 @@ toDoList = {
return function permissionsForObj() { return fixtures.permissionsFor(obj); };
},
clients: function insertClients() { return fixtures.insertClients(); },
filter: function createFilterParamFixtures() { return filterData(DataGenerator); }
filter: function createFilterParamFixtures() { return filterData(DataGenerator); },
trusted_domains: function trustedDomains() {
return fixtures.insertTrustedDomains();
}
};

/**
Expand Down

0 comments on commit 8b3e30f

Please sign in to comment.