From 26dbd355ac6408c5bf1c4663ea38417ba0b3b0bd Mon Sep 17 00:00:00 2001 From: Luka Skukan Date: Mon, 19 Dec 2016 13:51:59 +0100 Subject: [PATCH 1/4] Default to produces[0] when Accept header is not set --- lib/helpers/handle-accept-request.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/helpers/handle-accept-request.js b/lib/helpers/handle-accept-request.js index f734561a..b6db527e 100644 --- a/lib/helpers/handle-accept-request.js +++ b/lib/helpers/handle-accept-request.js @@ -26,11 +26,14 @@ function handleAcceptRequest(req, res, handlers, fallbackHandler) { // Accepted is an ordered list of preferred types, as specified by the request. var accepted = req.accepts(); var produces = config.web.produces; + var defaultContentType = produces && produces.length >= 1 + ? produces[0] + : 'text/html'; - // Our default response is HTML, if the client does not specify something more - // specific. As such, map the wildcard type to html. + // Our default response is the first entry in the `produces` array, if the + // request does not specify otherwise via the `Accepts` header. accepted = accepted.map(function (contentType) { - return contentType === '*/*' ? 'text/html' : contentType; + return contentType === '*/*' ? defaultContentType : contentType; }); // Of the accepted types, find the ones that are allowed by the configuration. From 45b58220c7e8a80932a2d1b1b79ad504a39447d1 Mon Sep 17 00:00:00 2001 From: Luka Skukan Date: Mon, 19 Dec 2016 13:52:12 +0100 Subject: [PATCH 2/4] Add tests for defaulting response content type --- test/helpers/test-handle-accept-request.js | 108 +++++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 test/helpers/test-handle-accept-request.js diff --git a/test/helpers/test-handle-accept-request.js b/test/helpers/test-handle-accept-request.js new file mode 100644 index 00000000..908e5753 --- /dev/null +++ b/test/helpers/test-handle-accept-request.js @@ -0,0 +1,108 @@ +'use strict'; + +var assert = require('assert'); +var sinon = require('sinon'); + +var handleAcceptRequest = require('../../lib/helpers').handleAcceptRequest; + +var defaultsAccepts = ['*/*']; +var mockReq = { + app: { + get: function get(name) { + return mockReq.app['_' + name]; + }, + _stormpathConfig: { + web: { + produces: ['application/json', 'text/html'], + spa: { + enabled: false + } + } + } + }, + accepts: function accepts() { + return mockReq._accepts; + }, + _accepts: defaultsAccepts +}; + +describe('handleAcceptRequest helper', function () { + describe('default response content type', function () { + var sandbox; + var jsonSpy; + var handlers; + var defaultHandler; + + before(function () { + sandbox = sinon.sandbox.create(); + jsonSpy = sandbox.spy(); + handlers = { + 'application/json': jsonSpy + }; + defaultHandler = sandbox.spy(); + }); + + after(function () { + sandbox.restore(); + }); + + it('should default to the first element of `config.web.produces`', function () { + handleAcceptRequest(mockReq, null, handlers, defaultHandler); + assert(jsonSpy.calledOnce); + assert.equal(defaultHandler.callCount, 0); + }); + }); + + describe('content type set via Accept', function () { + var oldAccepts; + var sandbox; + var htmlSpy; + var handlers; + var defaultHandler; + + beforeEach(function () { + sandbox = sinon.sandbox.create(); + oldAccepts = mockReq._accepts; + mockReq._accepts = ['text/html']; + htmlSpy = sandbox.spy(); + defaultHandler = sandbox.spy(); + handlers = { + 'text/html': htmlSpy + }; + }); + + afterEach(function () { + sandbox.restore(); + mockReq._accepts = oldAccepts; + }); + + describe('when provided in produces', function () { + it('should call the correct handler', function () { + handleAcceptRequest(mockReq, null, handlers, defaultHandler); + + assert(htmlSpy.calledOnce); + assert.equal(defaultHandler.callCount, 0); + }); + }); + + describe('when not provided in produces', function () { + var oldProduces; + + before(function () { + oldProduces = mockReq.app._stormpathConfig.web.produces.slice(0); + mockReq.app._stormpathConfig.web.produces = ['application/json']; + }); + + after(function () { + mockReq.app._stormpathConfig.web.produces = oldProduces.slice(0); + }); + + it('should call the default handler', function () { + handleAcceptRequest(mockReq, null, handlers, defaultHandler); + + assert.equal(htmlSpy.callCount, 0); + assert(defaultHandler.calledOnce); + }); + }); + }); +}); \ No newline at end of file From 552039026e7d6602f1ad081178d8d2d63906b252 Mon Sep 17 00:00:00 2001 From: Luka Skukan Date: Mon, 19 Dec 2016 15:07:49 +0100 Subject: [PATCH 3/4] Fix failing tests --- test/controllers/test-email-verification.js | 8 ++++--- test/controllers/test-forgot-password.js | 18 ++++++++++------ test/controllers/test-id-site.js | 3 ++- test/controllers/test-login.js | 11 +++++++--- test/controllers/test-logout.js | 3 ++- test/controllers/test-register.js | 21 ++++++++++++------- test/controllers/test-reset-password.js | 8 +++++-- test/fixtures/spa-root-fixture.js | 4 +++- .../test-api-authentication-required.js | 3 ++- test/middlewares/test-get-user.js | 6 ++++-- test/middlewares/test-groups-required.js | 3 ++- 11 files changed, 60 insertions(+), 28 deletions(-) diff --git a/test/controllers/test-email-verification.js b/test/controllers/test-email-verification.js index 022c635b..383c3009 100644 --- a/test/controllers/test-email-verification.js +++ b/test/controllers/test-email-verification.js @@ -54,7 +54,8 @@ describe('email verification', function () { web: { register: { enabled: true - } + }, + produces: ['text/html', 'application/json'] } }); expressApp.on('stormpath.ready', done); @@ -113,8 +114,8 @@ describe('email verification', function () { var config = expressApp.get('stormpathConfig'); request(expressApp) .post(config.web.verifyEmail.uri) - .send({ login: uuid() }) .set('Accept', 'application/json') + .send({ login: uuid() }) .expect(200, '', done); }); }); @@ -206,7 +207,8 @@ describe('email verification', function () { web:{ verifyEmail:{ uri: '/' + uuid() - } + }, + produces: ['text/html'] } }); diff --git a/test/controllers/test-forgot-password.js b/test/controllers/test-forgot-password.js index 344c2509..bed2ad8e 100644 --- a/test/controllers/test-forgot-password.js +++ b/test/controllers/test-forgot-password.js @@ -70,7 +70,8 @@ describe('forgotPassword', function () { web: { forgotPassword: { enabled: true - } + }, + produces: ['text/html', 'application/json'] } }); @@ -101,7 +102,8 @@ describe('forgotPassword', function () { web: { forgotPassword: { enabled: true - } + }, + produces: ['text/html', 'application/json'] } }); @@ -130,7 +132,8 @@ describe('forgotPassword', function () { web: { forgotPassword: { enabled: true - } + }, + produces: ['text/html', 'application/json'] } }); @@ -153,7 +156,8 @@ describe('forgotPassword', function () { web: { forgotPassword: { enabled: true - } + }, + produces: ['text/html', 'application/json'] } }); @@ -177,7 +181,8 @@ describe('forgotPassword', function () { web: { forgotPassword: { enabled: true - } + }, + produces: ['text/html', 'application/json'] } }); @@ -204,7 +209,8 @@ describe('forgotPassword', function () { web: { forgotPassword: { enabled: true - } + }, + produces: ['text/html', 'application/json'] } }); spaRootFixture.before(done); diff --git a/test/controllers/test-id-site.js b/test/controllers/test-id-site.js index 46a6f3fc..0132be00 100644 --- a/test/controllers/test-id-site.js +++ b/test/controllers/test-id-site.js @@ -149,7 +149,8 @@ describe('id site', function () { }, idSite: { enabled: true - } + }, + produces: ['text/html', 'application/json'] } }); diff --git a/test/controllers/test-login.js b/test/controllers/test-login.js index cf53f16e..a482c286 100644 --- a/test/controllers/test-login.js +++ b/test/controllers/test-login.js @@ -33,14 +33,18 @@ describe('login', function () { stormpathApplication = app; defaultExpressApp = helpers.createStormpathExpressApp({ - application: stormpathApplication + application: stormpathApplication, + web: { + produces: ['text/html', 'application/json'] + } }); alternateUrlExpressApp = helpers.createStormpathExpressApp({ application: stormpathApplication, web: { login: { uri: '/newlogin' - } + }, + produces: ['text/html', 'application/json'] } }); app.createAccount(accountData, done); @@ -240,7 +244,8 @@ describe('login', function () { web: { login: { enabled: true - } + }, + produces: ['text/html', 'application/json'] } }); spaRootFixture.before(done); diff --git a/test/controllers/test-logout.js b/test/controllers/test-logout.js index b46bf48d..86b29763 100644 --- a/test/controllers/test-logout.js +++ b/test/controllers/test-logout.js @@ -35,7 +35,8 @@ describe('logout', function () { }, accessTokenCookie: { domain: 'example.com' - } + }, + produces: ['text/html', 'application/json'] }, postLogoutHandler: postLogoutHandlerSpy }); diff --git a/test/controllers/test-register.js b/test/controllers/test-register.js index 86a1ff36..4510c432 100644 --- a/test/controllers/test-register.js +++ b/test/controllers/test-register.js @@ -34,7 +34,8 @@ function DefaultRegistrationFixture(stormpathApplication) { web: { register: { enabled: true - } + }, + produces: ['text/html', 'application/json'] } }); return this; @@ -128,7 +129,8 @@ function NamesOptionalRegistrationFixture(stormpathApplication) { } } } - } + }, + produces: ['text/html', 'application/json'] } }); return this; @@ -191,7 +193,8 @@ function NamesDisabledRegistrationFixture(stormpathApplication) { } } } - } + }, + produces: ['text/html', 'application/json'] } }); return this; @@ -247,7 +250,8 @@ function CustomFieldRegistrationFixture(stormpathApplication) { }, fieldOrder: ['givenName', 'surname', 'color', 'music', 'email', 'password'] } - } + }, + produces: ['text/html', 'application/json'] } }); return this; @@ -405,7 +409,8 @@ describe('register', function () { register: { enabled: true, uri: '/customRegistrationUri' - } + }, + produces: ['text/html', 'application/json'] } }); app.on('stormpath.ready', done); @@ -687,7 +692,8 @@ describe('register', function () { web: { register: { enabled: true - } + }, + produces: ['text/html', 'application/json'] } }); spaRootFixture.before(done); @@ -956,7 +962,8 @@ describe('register', function () { autoLogin: true, enabled: true, nextUri: '/woot' - } + }, + produces: ['text/html', 'application/json'] } }); app.on('stormpath.ready', done); diff --git a/test/controllers/test-reset-password.js b/test/controllers/test-reset-password.js index ef38eb17..4712920e 100644 --- a/test/controllers/test-reset-password.js +++ b/test/controllers/test-reset-password.js @@ -75,7 +75,10 @@ describe('resetPassword', function () { passwordResetToken = tokenResource.href.match(/\/([^\/]+)$/)[1]; defaultExpressApp = helpers.createStormpathExpressApp({ - application: stormpathApplication + application: stormpathApplication, + web: { + produces: ['text/html', 'application/json'] + } }); defaultExpressApp.on('stormpath.ready', done); @@ -320,7 +323,8 @@ describe('resetPassword', function () { web: { changePassword: { enabled: true - } + }, + produces: ['text/html', 'application/json'] } }); spaRootFixture.before(done); diff --git a/test/fixtures/spa-root-fixture.js b/test/fixtures/spa-root-fixture.js index 20f2f102..7598a5bf 100644 --- a/test/fixtures/spa-root-fixture.js +++ b/test/fixtures/spa-root-fixture.js @@ -21,7 +21,9 @@ function SpaRootFixture(stormpathConfig) { this.filename = '/tmp/' + uuid.v4(); if (!stormpathConfig.web) { - stormpathConfig.web = {}; + stormpathConfig.web = { + produces: ['text/html', 'application/json'] + }; } stormpathConfig.web.spa = { diff --git a/test/middlewares/test-api-authentication-required.js b/test/middlewares/test-api-authentication-required.js index 87b07b50..8f85262b 100644 --- a/test/middlewares/test-api-authentication-required.js +++ b/test/middlewares/test-api-authentication-required.js @@ -98,7 +98,8 @@ describe('apiAuthenticationRequired', function () { password: { validationStrategy: 'stormpath' } - } + }, + produces: ['text/html', 'application/json'] } }); diff --git a/test/middlewares/test-get-user.js b/test/middlewares/test-get-user.js index 93c2a494..57d96d50 100644 --- a/test/middlewares/test-get-user.js +++ b/test/middlewares/test-get-user.js @@ -69,7 +69,8 @@ describe('getUser', function () { web: { login: { enabled: true - } + }, + produces: ['text/html', 'application/json'] } }); @@ -463,7 +464,8 @@ describe('getUser', function () { password: { validationStrategy: 'local' } - } + }, + produces: ['text/html', 'application/json'] } }); diff --git a/test/middlewares/test-groups-required.js b/test/middlewares/test-groups-required.js index 07b30860..7e206d72 100644 --- a/test/middlewares/test-groups-required.js +++ b/test/middlewares/test-groups-required.js @@ -15,7 +15,8 @@ function createExpressTestApp(applicationHref, groupsToAssert, assertAll) { web: { login: { enabled: true - } + }, + produces: ['text/html', 'application/json'] } }); From 005df40255897232e2721f04788f3577d762fd4f Mon Sep 17 00:00:00 2001 From: Luka Skukan Date: Mon, 19 Dec 2016 15:45:43 +0100 Subject: [PATCH 4/4] Further test fixes --- test/handlers/test-post-registration-handler.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/handlers/test-post-registration-handler.js b/test/handlers/test-post-registration-handler.js index 3e2e415c..cf28c873 100644 --- a/test/handlers/test-post-registration-handler.js +++ b/test/handlers/test-post-registration-handler.js @@ -26,7 +26,8 @@ function preparePostRegistrationExpansionTestFixture(stormpathApplication, cb) { } } } - } + }, + produces: ['text/html', 'application/json'] }, postRegistrationHandler: function (account, req, res) { // Simply return the user object, so that we can @@ -54,6 +55,9 @@ function preparePostRegistrationPassThroughTestFixture(stormpathApplication, cb) fixture.expressApp = helpers.createStormpathExpressApp({ application: stormpathApplication, + web: { + produces: ['text/html', 'application/json'] + }, postRegistrationHandler: function (account, req, res, next) { fixture.sideEffect = fixture.sideEffectData; next(); @@ -78,7 +82,8 @@ function preparePostRegistrationAutoLoginTestFixture(stormpathApplication, cb) { register: { enabled: true, autoLogin: true - } + }, + produces: ['text/html', 'application/json'] }, postRegistrationHandler: function (account, req, res, next) { fixture.sideEffect = fixture.sideEffectData;