From 23c162796a8d2e608d4b6edc2ce4389ab72c3e00 Mon Sep 17 00:00:00 2001 From: Jason Williams Date: Sat, 26 Mar 2016 13:26:57 -0500 Subject: [PATCH] Relax origin checking in auth middleware Refs #6642 - Do not send CORS headers on an invalid "origin" header, but otherwise allow the response to proceed normally. This enforces CORS for the browser but does not blow up non-CORS requests. --- core/server/middleware/auth.js | 22 +++++------------- .../functional/routes/api/public_api_spec.js | 10 ++++---- .../unit/middleware/authentication_spec.js | 23 ------------------- 3 files changed, 10 insertions(+), 45 deletions(-) diff --git a/core/server/middleware/auth.js b/core/server/middleware/auth.js index effd76a8f5..ff9dd85355 100644 --- a/core/server/middleware/auth.js +++ b/core/server/middleware/auth.js @@ -94,8 +94,8 @@ auth = { return passport.authenticate(['oauth2-client-password'], {session: false, failWithError: false}, function authenticate(err, client) { - var origin = null, - error; + var origin = null; + if (err) { return next(err); // will generate a 500 error } @@ -119,22 +119,12 @@ auth = { if (!origin && client && client.type === 'ua') { res.header('Access-Control-Allow-Origin', config.url); - req.client = client; - return next(null, client); + } else if (isValidOrigin(origin, client)) { + res.header('Access-Control-Allow-Origin', req.headers.origin); } - if (isValidOrigin(origin, client)) { - res.header('Access-Control-Allow-Origin', req.headers.origin); - req.client = client; - return next(null, client); - } else { - error = new errors.UnauthorizedError(i18n.t('errors.middleware.auth.accessDeniedFromUrl', {origin: origin})); - errors.logError(error, - i18n.t('errors.middleware.auth.attemptedToAccessAdmin'), - i18n.t('errors.middleware.auth.forInformationRead', {url: 'http://support.ghost.org/config/#url'}) - ); - return errors.handleAPIError(error, req, res, next); - } + req.client = client; + return next(null, client); } )(req, res, next); }, diff --git a/core/test/functional/routes/api/public_api_spec.js b/core/test/functional/routes/api/public_api_spec.js index 1f893f3302..49bf3fc13e 100644 --- a/core/test/functional/routes/api/public_api_spec.js +++ b/core/test/functional/routes/api/public_api_spec.js @@ -198,22 +198,20 @@ describe('Public API', function () { }); }); - it('denies access from invalid origin', function (done) { + it('does not send CORS headers on an invalid origin', function (done) { request.get(testUtils.API.getApiQuery('posts/?client_id=ghost-admin&client_secret=not_available')) .set('Origin', 'http://invalid-origin') .expect('Content-Type', /json/) .expect('Cache-Control', testUtils.cacheRules.private) - .expect(401) + .expect(200) .end(function (err, res) { if (err) { return done(err); } should.not.exist(res.headers['x-cache-invalidate']); - var jsonResponse = res.body; - should.exist(jsonResponse); - should.exist(jsonResponse.errors); - testUtils.API.checkResponseValue(jsonResponse.errors[0], ['message', 'errorType']); + should.not.exist(res.headers['access-control-allow-origin']); + done(); }); }); diff --git a/core/test/unit/middleware/authentication_spec.js b/core/test/unit/middleware/authentication_spec.js index ae3635e33b..f8c28961ac 100644 --- a/core/test/unit/middleware/authentication_spec.js +++ b/core/test/unit/middleware/authentication_spec.js @@ -369,29 +369,6 @@ describe('Auth', function () { done(); }); - it('shouldn\'t authenticate client with invalid origin', function (done) { - req.body = {}; - req.body.client_id = testClient; - req.body.client_secret = testSecret; - req.headers = {}; - req.headers.origin = 'http://invalid.origin.com'; - res.status = {}; - - sandbox.stub(res, 'status', function (statusCode) { - statusCode.should.eql(401); - return { - json: function (err) { - err.errors[0].errorType.should.eql('UnauthorizedError'); - } - }; - }); - - registerSuccessfulClientPasswordStrategy(); - auth.authenticateClient(req, res, next); - next.called.should.be.false(); - done(); - }); - it('should authenticate valid/known client', function (done) { req.body = {}; req.body.client_id = testClient;