johnbrett / hapi-auth-bearer-token

Simple Bearer authentication scheme plugin for hapi, accepts token by Header, Cookie or Query parameter.
MIT License
218 stars 46 forks source link

Why delete access token from request.query #6

Closed adrianblynch closed 10 years ago

adrianblynch commented 10 years ago

Hi John, out of interest, why do you delete the access token field from request.query in lib/index.js:26?

delete request.query[settings.accessTokenName];

johnbrett commented 10 years ago

Hi Adrian,

Good question :)

If the access token wasn't deleted, and you were using route validation, you would need to explicitly add a validation rule for every route created to allow the access_token to be passed through, which shouldn't be a requirement. It allows for a separation of concerns where this module handles your auth and your routing config and validation is just for that.

Does this pose issues for you?

adrianblynch commented 10 years ago

OK, that makes sense!

We're using HABT on only one route, /login so we haven't bumped into that.

The reason I ask is that I'm now creating a /logout route and I need to implement the same logic you have in lib/index.js that attempts to extract the access token first from the auth header, then the URL.

I was thinking to pull out that logic into extra functions so I could call HABT.getAccessToken().

It doesn't feel 100% right as it's more of a utility function.

Will ponder on it some more...

johnbrett commented 10 years ago

Is the project you're working on github? I'd be curious to take a look if so.

Am I right in assuming on the logout you're hoping to retrieve the auth token to invalidate it on the logout route or something along those lines?

adrianblynch commented 10 years ago

It's not on GH unfortunately.

Yup, that's what I was looking to do.

Am I missing a trick in getting access to the access token?

johnbrett commented 10 years ago

Well I'm actually just wondering what you're doing with auth in the mean time, for the /already_logged_in routes.

If you're storing authentication in the cookie from then on, you could also store the auth token. When I have a user authenticated, I store the auth token among the user's credentials by passing it back in the strategy callback:

callback(null, true, { token: token }) // this line in the example on the readme

so it is then accessible from request.auth.credentials.token.

I think I'd need to understand your full use case in order to be able to offer any more advice though.

adrianblynch commented 10 years ago

All routes are using auth barer except for /login with is using auth basic (This is likely to change).

We're leaving it up to the client to choose whether they send the barer token in the URL or as a header.

I'd like the route /logout to not be any different to all the other routes in that I can call /logout?accessToken=123 or /logout (Barer base64 rep. of 123).

So I need to do the same logic as in your plugin, first look in the headers, then the URL.

Does that seem like an OK plan?

johnbrett commented 10 years ago

Ah I misunderstood, I thought only /login was auth bearer.

That all seems perfectly ok in that case. So my advice:

  1. Store the auth token in the credentials object from the authentication strategy, so you have access to it throughout the app like in the example above.
  2. Also use HABT for the /logout route, so when they call /logout?accessToken=123, you can access the token from request.auth.credentials.token for you to mark invalid etc.

Let me know if this poses any problems.

johnbrett commented 10 years ago

Hi Adrian, going to close this issue, but feel free to comment or follow up if the above approach doesn't work for you.