From 0e9c82541b6ee620c83f00c53f56993537d966b9 Mon Sep 17 00:00:00 2001 From: Seth Holladay Date: Wed, 10 Jan 2018 00:57:48 -0500 Subject: [PATCH 1/4] Remove unnecessary file permissions --- .travis.yml | 0 LICENSE | 0 README.md | 0 3 files changed, 0 insertions(+), 0 deletions(-) mode change 100755 => 100644 .travis.yml mode change 100755 => 100644 LICENSE mode change 100755 => 100644 README.md diff --git a/.travis.yml b/.travis.yml old mode 100755 new mode 100644 diff --git a/LICENSE b/LICENSE old mode 100755 new mode 100644 diff --git a/README.md b/README.md old mode 100755 new mode 100644 From ef4de9cb6be37caab43a7d68e7da251d7cda6e22 Mon Sep 17 00:00:00 2001 From: Seth Holladay Date: Wed, 10 Jan 2018 01:33:41 -0500 Subject: [PATCH 2/4] Improve redirectTo and remove redirectOnTry --- README.md | 9 +++------ lib/index.js | 9 +-------- test/index.js | 32 -------------------------------- 3 files changed, 4 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index 05065f4..fbc700b 100644 --- a/README.md +++ b/README.md @@ -33,16 +33,13 @@ The `'cookie`' scheme takes the following options: - `isSecure` - if `false`, the cookie is allowed to be transmitted over insecure connections which exposes it to attacks. Defaults to `true`. - `isHttpOnly` - if `false`, the cookie will not include the 'HttpOnly' flag. Defaults to `true`. -- `redirectTo` - optional login URI to redirect unauthenticated requests to. Note that using - `redirectTo` with authentication mode `'try'` will cause the protected endpoint to always - redirect, voiding `'try'` mode. To set an individual route to use or disable redirections, use - the route `plugins` config (`{ options: { plugins: { 'hapi-auth-cookie': { redirectTo: false } } } }`). +- `redirectTo` - optional login URI to redirect unauthenticated requests to. Note that it will only + trigger when the authentication mode is `'required'`. To enable or disable redirections for a specific route, + set the route `plugins` config (`{ options: { plugins: { 'hapi-auth-cookie': { redirectTo: false } } } }`). Defaults to no redirection. - `appendNext` - if `true` and `redirectTo` is `true`, appends the current request path to the query component of the `redirectTo` URI using the parameter name `'next'`. Set to a string to use a different parameter name. Defaults to `false`. -- `redirectOnTry` - if `false` and route authentication mode is `'try'`, authentication errors will - not trigger a redirection. Defaults to `true`; - `async validateFunc` - an optional session validation function used to validate the content of the session cookie on each request. Used to verify that the internal session state is still valid (e.g. user account still exists). The function has the signature `function(request, session)` diff --git a/lib/index.js b/lib/index.js index ce58c54..77e4b80 100755 --- a/lib/index.js +++ b/lib/index.js @@ -32,7 +32,6 @@ internals.schema = Joi.object({ isHttpOnly: Joi.boolean().default(true), redirectTo: Joi.alternatives(Joi.string(), Joi.func()).allow(false), appendNext: Joi.alternatives(Joi.string(), Joi.boolean()).default(false), - redirectOnTry: Joi.boolean().default(true), validateFunc: Joi.func(), requestDecoratorName: Joi.string().default('cookieAuth'), ignoreIfDecorated: Joi.boolean().default(true) @@ -196,12 +195,6 @@ internals.implementation = (server, options) => { const unauthenticated = (err, result) => { - if (settings.redirectOnTry === false && // Defaults to true - request.auth.mode === 'try') { - - return h.unauthenticated(err); - } - let redirectTo = settings.redirectTo; if (request.route.settings.plugins['hapi-auth-cookie'] && request.route.settings.plugins['hapi-auth-cookie'].redirectTo !== undefined) { @@ -209,7 +202,7 @@ internals.implementation = (server, options) => { redirectTo = request.route.settings.plugins['hapi-auth-cookie'].redirectTo; } - if (!redirectTo) { + if (!redirectTo || request.auth.mode !== 'required') { return h.unauthenticated(err); } diff --git a/test/index.js b/test/index.js index bf094f3..fee17ce 100755 --- a/test/index.js +++ b/test/index.js @@ -1140,38 +1140,6 @@ describe('scheme', () => { expect(res.statusCode).to.equal(401); }); - it('skips when redirectOnTry is false in try mode', async () => { - - const server = Hapi.server(); - await server.register(require('../')); - - server.auth.strategy('default', 'cookie', { - password: 'password-should-be-32-characters', - ttl: 60 * 1000, - redirectOnTry: false, - redirectTo: 'http://example.com/login', - appendNext: true - }); - server.auth.default({ - mode: 'try', - strategy: 'default' - }); - - server.route({ - method: 'GET', - path: '/', - handler: function (request, h) { - - return h.response(request.auth.isAuthenticated); - } - }); - - const res = await server.inject('/'); - - expect(res.statusCode).to.equal(200); - expect(res.result).to.equal(false); - }); - it('sends to login page (uri with query)', async () => { const server = Hapi.server(); From 34f8de29e9c22ae997d3ff5441a6ff1efcb611a2 Mon Sep 17 00:00:00 2001 From: Seth Holladay Date: Wed, 10 Jan 2018 01:49:32 -0500 Subject: [PATCH 3/4] Add tests for improved redirectTo --- test/index.js | 55 +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 53 insertions(+), 2 deletions(-) diff --git a/test/index.js b/test/index.js index fee17ce..fea5fe6 100755 --- a/test/index.js +++ b/test/index.js @@ -1218,7 +1218,33 @@ describe('scheme', () => { expect(res.headers.location).to.equal('http://example.com/login?mode=1&done=%2F'); }); - it('redirect on try', async () => { + it('redirect for required mode', async () => { + + const server = Hapi.server(); + await server.register(require('../')); + + server.auth.strategy('default', 'cookie', { + password: 'password-should-be-32-characters', + ttl: 60 * 1000, + redirectTo: 'http://example.com/login', + appendNext: true + }); + server.auth.default('default'); + + server.route({ + method: 'GET', path: '/', config: { auth: { mode: 'required' } }, handler: function (request, h) { + + return h.response('required'); + } + }); + + const res = await server.inject('/'); + + expect(res.statusCode).to.equal(302); + expect(res.headers.location).to.equal('http://example.com/login?next=%2F'); + }); + + it('skips redirect for try mode', async () => { const server = Hapi.server(); await server.register(require('../')); @@ -1240,7 +1266,32 @@ describe('scheme', () => { const res = await server.inject('/'); - expect(res.statusCode).to.equal(302); + expect(res.statusCode).to.equal(200); + }); + + it('skips redirect for optional mode', async () => { + + const server = Hapi.server(); + await server.register(require('../')); + + server.auth.strategy('default', 'cookie', { + password: 'password-should-be-32-characters', + ttl: 60 * 1000, + redirectTo: 'http://example.com/login', + appendNext: true + }); + server.auth.default('default'); + + server.route({ + method: 'GET', path: '/', config: { auth: { mode: 'optional' } }, handler: function (request, h) { + + return h.response('optional'); + } + }); + + const res = await server.inject('/'); + + expect(res.statusCode).to.equal(200); }); }); From c9749a47f0b0568cd3659d6904b0a38b49e63279 Mon Sep 17 00:00:00 2001 From: Seth Holladay Date: Sat, 24 Mar 2018 21:21:06 -0400 Subject: [PATCH 4/4] Update old variable names --- test/index.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/index.js b/test/index.js index 27bd72f..a446603 100755 --- a/test/index.js +++ b/test/index.js @@ -403,9 +403,9 @@ describe('scheme', () => { const schema = () => { return { - authenticate: (request, reply) => { + authenticate: (request, h) => { - return reply.authenticated({ credentials: { user: 'bogus-user' } }); + return h.authenticated({ credentials: { user: 'bogus-user' } }); } }; }; @@ -431,10 +431,10 @@ describe('scheme', () => { server.route({ method: 'GET', path: '/login/{user}', config: { - handler: function (request, reply) { + handler: function (request, h) { request.cookieAuth.set({ user: request.params.user }); - return reply.response(request.params.user); + return h.response(request.params.user); } } }); @@ -443,9 +443,9 @@ describe('scheme', () => { method: 'GET', path: '/resource', config: { auth: { mode: 'required', strategies: ['first', 'second'] }, - handler: function (request, reply) { + handler: function (request, h) { - return reply.response('valid-resource'); + return h.response('valid-resource'); } } }); @@ -1255,9 +1255,9 @@ describe('scheme', () => { server.auth.default('default'); server.route({ - method: 'GET', path: '/', handler: function (request, reply) { + method: 'GET', path: '/', handler: function () { - return reply('never'); + return 'never'; } });