added parameter to irgnore url token

[bitcloud]

added allowUrlToken to be able to disable the URL Token for security reasons. defaults to true, to avoid breaking change.

[codecov-io]

Current coverage is 100.00%

Merging #146 into master will not affect coverage as of 477826b

@@            master    #146   diff @@
======================================
  Files            2       2       
  Stmts           76      76       
  Branches         0       0       
  Methods          0       0       
======================================
  Hit             76      76       
  Partial          0       0       
  Missed           0       0       

Review entire Coverage Diff as of 477826b

Powered by Codecov. Updated on successful CI builds.

[iteles]

@bitcloud Thanks for this contribution!
Does this relate to an issue where the use cases were discussed and agreed or is this still in the suggestion phase?

Thanks again 😊

[bitcloud]

I think this is still in the suggestion phase. Didn't know that there is a process ;-)
Wanted to have it for our implementation, thats why I made it.

[vdeturckheim]

What about when allowUrlToken has a falsy value and request.query[urlKey] a truthly one ? If this case is not possible, please add a comment to state that.

[bitcloud]

That was the whole idea to be able to turn the url key off. And I wanted it to default to true, so that it's not a breaking change. Another way would probably be to set the urlKey explicit to false.

[nelsonic]

@bitcloud there is already an easy way of achieving this result.
Simply set the urlKey to something impossible to guess when you are defining your auth strategy:

server.auth.strategy('jwt', 'jwt',{ 
  key: 'YourJWTSecret', 
  validateFunc: validate,      // your validate function
  urlKey: require('crypto').randomBytes(256).toString('base64'), // the NSA cannot crack this!
  verifyOptions: { algorithms: [ 'HS256' ] } 
});
}

Try running the following command in your terminal:

node -e "console.log(require('crypto').randomBytes(256).toString('base64'));"

It would take all the computing power in the world the rest of time, to crack that key.

Watch: https://www.youtube.com/watch?v=koJQQWHI-ZA
and read: https://www.reddit.com/r/theydidthemath/comments/1x50xl/time_and_energy_required_to_bruteforce_a_aes256/

The person trying to "hack" your Hapi website/app would literally be better off
spending the resources mining bitcoin ...

[bitcloud]

seriously? So you suggest to request another lib to generate enough noise so that the key couldn't be guessed?!? I would have accepted something like setting it to an empty string because this is kind of achieve the same result, but this is really bad.

In my opinion it should be the default behaviour that urlKeys are not allowed, because this just adds an unnecessary attack vector. But its your call.

[bitcloud]

ok, I just checked it with an empty string. It is really possible to pass a query string with no parameter name ... but still. Setting it to boolean false and check on that would be better (as I mentioned in the inline comment).

[ghost]

My 2¢ is that it would be much nicer to simply have a feature to disable accepting the token from the URL. The workaround is easy enough but results in more complicated source for projects using this module.

[nelsonic]

@wprl your 2¢ are always welcome. 👍
Our decision to not include this as an explicit feature is that there was a work-able way to (effectively) prevent people passing in tokens via the URL ...

@bitcloud is _totally right_ setting the urlKey to '' (empty string) will achieve the same desired result without requiring crypto; that's a much better idea! 💡 😄

We could/should add that to the Docs so others know how to disable tokens via URL.

Also, @bitcloud I don't understand how allowing JWTs via the URL creates an _attack vector_ any more than accepting JWTs in the header ... requests with an invalid URL JWT will still be rejected (and just as quickly as if the invalid JWT was passed in via Header...) ❓ (please elaborate...)

We could set urlKey to '' (empty string) by default but this would require a major version release with upgrade/release notes for the people who currently expect the feature to be available by default...
I prefer to only release new major versions if there is a new functionality added or reported security vulnerability.

[bitcloud]

@nelsonic concerning the security, it is perhaps a little bit constructed but:
Lets say you get a hold on an valid JWT token, but the server that accepts that token is behind a firewall. Then you could construct an URL the attacked person just has to click on to do some kind of reflected attack. This URL could be posted anywhere then. Do you know what I mean?
How you are getting your hands on the valid token is another story, thats true.

The whole reason I introduced the additional parameter was because of not having a breaking change ;-)

[nelsonic]

Yeah, that does make an assumption that you are able to intercept a valid JWT.

A much easier way of the JWT leaking is if your server sends an email (containing a url with a valid JWT token e.g. "Click link to verify your account") to an inbox on an un-encrypted email service provider ... but this is no longer the security "fault" of JWTs but of the email service provider ... 💔

I really like your empty string solution to blocking JWTs in URLs. 👍

[bitcloud]

And the users for using an unencrypted service. ;-)
True and sadly even I know enough people who don't care enough .... but thats for something completely different.

The server doesn't even need to be firewalled, it is sufficient if the client IP is encoded in the JWT token for his session. And I think the biggest part of leaking a session is probably stupid and simple "look at that" copy&pasting URL's in a chat.

But thats the reason why JWT shouldn't be used in URLs anyway besides that they are logged to the server.log as well...
Thats why I would default it to disable in a further release so that hopefully people think twice before enabling and using it.

It all boils down to "you should know what you do" and If someone whats to use JWT in an unsecure whey - well he can and there is nothing we can do about it :-/

Ok, back to topic now ... I could try to update my pull request in the next days to the empty string solution. Is mostly docs and testcases anyway. Probably even allow a boolean false to make it more explicit. At least a false shouldn't default to 'token' ;-)

[nelsonic]

Good plan. yes, please do update the PR. 👍

[bitcloud]

hmm, I updated the branch, but it doesn't appear here. Probably because you already closed it. Do you want to reopen it?

[nelsonic]

@bitcloud re-opened. 👍

[ghost]

From a systems hardening perspective, any feature is part of the attack surface, because it is another "moving part" that could be broken in some unforeseen way to gain unauthorized access to a system. It's a paranoid way of thinking, but also effective :)

Glad to see this getting some polish. Really happy with how easy this plugin is to drop in place!

[bitcloud]

ok, finally some time to resolve the conflicts ;-)
I added the functionality to disable not to all options to bi consistent.

[nelsonic]

@bitcloud looks good. will merge and publish to NPM when back desk. thanks! 👍

[nelsonic]

Merging.
thanks again! 👍

[bitcloud]

damn it, I missed something in the docs. There is still your example of setting it to 'something impossible to guess'. Probably you can clean that up on the next readme update? I don't know if I can update the pull request anymore now :)

[nelsonic]

Ah, I did not spot it either... ok. lets fix that on the next PR. 👍