Skip to content

Commit

Permalink
🐛 Fixed public api access on custom domain
Browse files Browse the repository at this point in the history
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 #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
  • Loading branch information
kirrg001 authored and aileen committed Sep 14, 2017
1 parent 85f8498 commit 79959d9
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 6 deletions.
5 changes: 0 additions & 5 deletions core/server/api/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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);
Expand Down
7 changes: 6 additions & 1 deletion core/server/api/middleware.js
Original file line number Diff line number Diff line change
@@ -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
*/

/**
Expand All @@ -19,6 +21,7 @@ module.exports.authenticatePublic = [
// This is a labs-enabled middleware
auth.authorize.requiresAuthorizedUserPublicAPI,
cors,
urlRedirects,
prettyURLs
];

Expand All @@ -30,6 +33,7 @@ module.exports.authenticatePrivate = [
auth.authenticate.authenticateUser,
auth.authorize.requiresAuthorizedUser,
cors,
urlRedirects,
prettyURLs
];

Expand All @@ -42,6 +46,7 @@ module.exports.authenticateClient = function authenticateClient(client) {
auth.authenticate.authenticateUser,
auth.authorize.requiresAuthorizedClient(client),
cors,
urlRedirects,
prettyURLs
];
};
26 changes: 26 additions & 0 deletions core/test/functional/routes/api/public_api_spec.js
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down

0 comments on commit 79959d9

Please sign in to comment.