Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow additional strategies to authenticate if no token is found #104. #105

Merged
merged 2 commits into from
Oct 9, 2015
Merged

Allow additional strategies to authenticate if no token is found #104. #105

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1839d75
Allow additional strategies to authenticate if no token is found #104.
martinj Sep 15, 2015
a31854d
Add option for specifying token type (Auth scheme) in Authorisation h…
martinj Oct 9, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions lib/extract.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,18 @@ var Boom = require('boom'); // error handling https://github.com/hapijs/boom
*/

module.exports = function (request, options) {

// The key holding token value in url or cookie defaults to token
var urlKey = typeof options.urlKey === 'string' ? options.urlKey : 'token';
var cookieKey = typeof options.cookieKey === 'string' ? options.cookieKey : 'token';
var auth;

if(request.query[urlKey]) { // tokens via url: https://github.com/dwyl/hapi-auth-jwt2/issues/19
auth = request.query[urlKey];
} // JWT tokens in cookie: https://github.com/dwyl/hapi-auth-jwt2/issues/55
else if (request.headers.authorization) {
auth = request.headers.authorization;
var token = request.headers.authorization.match(/Bearer\s+([^$]+)/i);
return token === null ? null : token[1];
}
else if (request.headers.cookie) {
auth = Cookie.parse(request.headers.cookie)[cookieKey];
Expand Down
8 changes: 4 additions & 4 deletions lib/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,15 @@ internals.implementation = function (server, options) {
if (!token && request.auth.mode === 'optional') {
return reply.continue({ credentials: {} });
}

if (!token) {
return reply(Boom.unauthorized('Missing auth token'));
return reply(Boom.unauthorized(null, 'Token'));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First parameter of Boom.unauthorized is the error message
https://github.com/hapijs/boom#boomunauthorizedmessage-scheme-attributes
We could do:

return reply(Boom.unauthorized('Missing JWT Auth token', 'Token'));

Is there a reason you want to return null as the first argument?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what enables additional strategies to be attempted.
From hapi documentation http://hapijs.com/api#serverauthschemename-scheme:

If the err returned by the reply() method includes a message, no additional strategies will be attempted. If the err does not include a message but does include a scheme name (e.g. Boom.unauthorized(null, 'Custom')), additional strategies will be attempted in order of preference.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but have you tried using the strategy with try mode and checking if that prevents other strategies from being attempted?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I havn't.
For this plugin to be useful for me it needs to support different strategies with required mode.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. I haven't seen two auth strategies required for the same request/route but I'm all ears. Can you describe the scenario in a bit more detail so we understand the use-case? we'd love to help you solve this because others will probably have similar needs in the future. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For our API we use JWT for user authenticated requests, but for services consuming the API we use an apiKey because we want to handle them differently.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't your API Key be a JWT?
(or do you already have an existing/legacy format for API Keys)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be possible, but thats not what we want to do and it's outside the point here.
Hapi offers this functionality for authentication plugins and its was supported by the library i want to replace hapi-auth-jwt thats why I submitted this PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

understood. I was just wondering if you could simplify your own development by using JTWs everywhere.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello! I'm also interested in seeing this particular line merged.

I have two auth strategies in place for several routes, although the second strategy does little more than change the value of request.auth.credentials at this point :)

In any case, I did test the difference between try and required and did not see a difference in behavior. If the first argument to Boom.unauthorized is not null, then Hapi doesn't try other authentication strategies, regardless of mode selected.

Thanks!

}

if (!extract.isValid(token)) {
return reply(Boom.unauthorized('Invalid token format', 'Token'));
}

var keyFunc = (internals.isFunction(options.key)) ? options.key : function (decoded, callback) { callback(null, options.key); };

var decoded;
Expand Down
2 changes: 1 addition & 1 deletion test/validate-func-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ test('Should respond with 500 series error when validateFunc errs', function (t)
var options = {
method: 'POST',
url: '/privado',
headers: {Authorization: JWT.sign({id: 138, name: 'Test'}, secret)}
headers: {Authorization: 'Bearer ' + JWT.sign({id: 138, name: 'Test'}, secret)}
};

server.inject(options, function (response) {
Expand Down