From 79959d95813bc6e2b8c0a29a4dd0cbe0de32f746 Mon Sep 17 00:00:00 2001 From: kirrg001 Date: Wed, 13 Sep 2017 19:24:11 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20=20Fixed=20public=20api=20access?= =?UTF-8?q?=20on=20custom=20domain?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- core/server/api/app.js | 5 ---- core/server/api/middleware.js | 7 ++++- .../functional/routes/api/public_api_spec.js | 26 +++++++++++++++++++ 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/core/server/api/app.js b/core/server/api/app.js index 491658b489..74fad05efe 100644 --- a/core/server/api/app.js +++ b/core/server/api/app.js @@ -13,7 +13,6 @@ var debug = require('ghost-ignition').debug('api'), // Shared bodyParser = require('body-parser'), // global, shared cacheControl = require('../middleware/cache-control'), // global, shared - urlRedirects = require('../middleware/url-redirects'), maintenance = require('../middleware/maintenance'), // 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 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 // Therefore must come after themeHandler.ghostLocals, for now apiApp.use(versionMatch); diff --git a/core/server/api/middleware.js b/core/server/api/middleware.js index e49bea731a..c1024c86fc 100644 --- a/core/server/api/middleware.js +++ b/core/server/api/middleware.js @@ -1,13 +1,15 @@ var prettyURLs = require('../middleware/pretty-urls'), cors = require('../middleware/api/cors'), + urlRedirects = require('../middleware/url-redirects'), auth = require('../auth'); /** * Auth Middleware Packages * * 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 + * - 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 auth.authorize.requiresAuthorizedUserPublicAPI, cors, + urlRedirects, prettyURLs ]; @@ -30,6 +33,7 @@ module.exports.authenticatePrivate = [ auth.authenticate.authenticateUser, auth.authorize.requiresAuthorizedUser, cors, + urlRedirects, prettyURLs ]; @@ -42,6 +46,7 @@ module.exports.authenticateClient = function authenticateClient(client) { auth.authenticate.authenticateUser, auth.authorize.requiresAuthorizedClient(client), cors, + urlRedirects, prettyURLs ]; }; diff --git a/core/test/functional/routes/api/public_api_spec.js b/core/test/functional/routes/api/public_api_spec.js index 797bec881f..b4abb3b712 100644 --- a/core/test/functional/routes/api/public_api_spec.js +++ b/core/test/functional/routes/api/public_api_spec.js @@ -1,6 +1,7 @@ var should = require('should'), supertest = require('supertest'), testUtils = require('../../../utils'), + configUtils = require('../../../utils/configUtils'), _ = require('lodash'), config = require('../../../../../core/server/config'), ghost = testUtils.startGhost, @@ -29,6 +30,10 @@ describe('Public API', function () { }); }); + afterEach(function () { + configUtils.restore(); + }); + it('browse posts', function (done) { request.get(testUtils.API.getApiQuery('posts/?client_id=ghost-admin&client_secret=not_available')) .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) { request.get(testUtils.API.getApiQuery('posts/?client_id=ghost-admin&client_secret=not_available&staticPages=true')) .set('Origin', testUtils.API.getURL())