🐛 Fixed public api access on custom domain

no issue

- if you blog runs on a custom domain, but your admin panel is configured using a different domain
  -> Ghost losts the origin header
- we had this situation once with pretty urls (your request get's redirected from /posts to /posts/, see https://github.com/TryGhost/Ghost/pull/8094)
- we've moved all our redirect logic to Ghost and ran into the same situation
- i've added proper test to ensure it won't happen again
This commit is contained in:
kirrg001 2017-09-13 19:24:11 +02:00 committed by Aileen Nowak
parent 85f8498bd6
commit 79959d9581
3 changed files with 32 additions and 6 deletions

View File

@ -13,7 +13,6 @@ var debug = require('ghost-ignition').debug('api'),
// Shared // Shared
bodyParser = require('body-parser'), // global, shared bodyParser = require('body-parser'), // global, shared
cacheControl = require('../middleware/cache-control'), // global, shared cacheControl = require('../middleware/cache-control'), // global, shared
urlRedirects = require('../middleware/url-redirects'),
maintenance = require('../middleware/maintenance'), // global, shared maintenance = require('../middleware/maintenance'), // global, shared
errorHandler = require('../middleware/error-handler'); // global, shared errorHandler = require('../middleware/error-handler'); // global, shared
@ -37,10 +36,6 @@ module.exports = function setupApiApp() {
// send 503 json response in case of maintenance // send 503 json response in case of maintenance
apiApp.use(maintenance); apiApp.use(maintenance);
// Force SSL if required
// must happen AFTER asset loading and BEFORE routing
apiApp.use(urlRedirects);
// Check version matches for API requests, depends on res.locals.safeVersion being set // Check version matches for API requests, depends on res.locals.safeVersion being set
// Therefore must come after themeHandler.ghostLocals, for now // Therefore must come after themeHandler.ghostLocals, for now
apiApp.use(versionMatch); apiApp.use(versionMatch);

View File

@ -1,13 +1,15 @@
var prettyURLs = require('../middleware/pretty-urls'), var prettyURLs = require('../middleware/pretty-urls'),
cors = require('../middleware/api/cors'), cors = require('../middleware/api/cors'),
urlRedirects = require('../middleware/url-redirects'),
auth = require('../auth'); auth = require('../auth');
/** /**
* Auth Middleware Packages * Auth Middleware Packages
* *
* IMPORTANT * IMPORTANT
* - cors middleware MUST happen before pretty urls, because otherwise cors header can get lost * - cors middleware MUST happen before pretty urls, because otherwise cors header can get lost on redirect
* - cors middleware MUST happen after authenticateClient, because authenticateClient reads the trusted domains * - cors middleware MUST happen after authenticateClient, because authenticateClient reads the trusted domains
* - url redirects MUST happen after cors, otherwise cors header can get lost on redirect
*/ */
/** /**
@ -19,6 +21,7 @@ module.exports.authenticatePublic = [
// This is a labs-enabled middleware // This is a labs-enabled middleware
auth.authorize.requiresAuthorizedUserPublicAPI, auth.authorize.requiresAuthorizedUserPublicAPI,
cors, cors,
urlRedirects,
prettyURLs prettyURLs
]; ];
@ -30,6 +33,7 @@ module.exports.authenticatePrivate = [
auth.authenticate.authenticateUser, auth.authenticate.authenticateUser,
auth.authorize.requiresAuthorizedUser, auth.authorize.requiresAuthorizedUser,
cors, cors,
urlRedirects,
prettyURLs prettyURLs
]; ];
@ -42,6 +46,7 @@ module.exports.authenticateClient = function authenticateClient(client) {
auth.authenticate.authenticateUser, auth.authenticate.authenticateUser,
auth.authorize.requiresAuthorizedClient(client), auth.authorize.requiresAuthorizedClient(client),
cors, cors,
urlRedirects,
prettyURLs prettyURLs
]; ];
}; };

View File

@ -1,6 +1,7 @@
var should = require('should'), var should = require('should'),
supertest = require('supertest'), supertest = require('supertest'),
testUtils = require('../../../utils'), testUtils = require('../../../utils'),
configUtils = require('../../../utils/configUtils'),
_ = require('lodash'), _ = require('lodash'),
config = require('../../../../../core/server/config'), config = require('../../../../../core/server/config'),
ghost = testUtils.startGhost, ghost = testUtils.startGhost,
@ -29,6 +30,10 @@ describe('Public API', function () {
}); });
}); });
afterEach(function () {
configUtils.restore();
});
it('browse posts', function (done) { it('browse posts', function (done) {
request.get(testUtils.API.getApiQuery('posts/?client_id=ghost-admin&client_secret=not_available')) request.get(testUtils.API.getApiQuery('posts/?client_id=ghost-admin&client_secret=not_available'))
.set('Origin', testUtils.API.getURL()) .set('Origin', testUtils.API.getURL())
@ -83,6 +88,27 @@ describe('Public API', function () {
}); });
}); });
it('ensure origin header on redirect is not getting lost', function (done) {
// NOTE: force a redirect to the admin url
configUtils.set('admin:url', 'http://localhost:9999');
request.get(testUtils.API.getApiQuery('posts?client_id=ghost-test&client_secret=not_available'))
.set('Origin', 'https://example.com')
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(301)
.end(function (err, res) {
if (err) {
return done(err);
}
res.headers.vary.should.eql('Origin, Accept, Accept-Encoding');
res.headers.location.should.eql('http://localhost:9999/ghost/api/v0.1/posts/?client_id=ghost-test&client_secret=not_available');
should.exist(res.headers['access-control-allow-origin']);
should.not.exist(res.headers['x-cache-invalidate']);
done();
});
});
it('browse posts, ignores staticPages', function (done) { it('browse posts, ignores staticPages', function (done) {
request.get(testUtils.API.getApiQuery('posts/?client_id=ghost-admin&client_secret=not_available&staticPages=true')) request.get(testUtils.API.getApiQuery('posts/?client_id=ghost-admin&client_secret=not_available&staticPages=true'))
.set('Origin', testUtils.API.getURL()) .set('Origin', testUtils.API.getURL())