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

if validation fails, please make credentials object to be optional #152

Closed manju4ever closed 6 years ago

manju4ever commented 6 years ago

Lets assume in the validate function the validity is proven to be wrong. The plugin still expects a credentials object to be passed even if isValid:false. Even though the function is called with the response toolkit, using something like return h.response().code(401) does not work as expected.

johnbrett commented 6 years ago

Have you a code sample to explain what issue you're encoutering @manju4ever ? Apologies for delay in getting back to you on this.

robinvdvleuten commented 6 years ago

I ran into the same issue; when the token is invalid according to my business logic I do something like the following.

if (!accessToken) {
  return { isValid: false }
}

But unfortunately, the hapi-auth-bearer-token throws me the following error;

Authentication data missing credentials information

As Hapi expects a valid credentials object to be passed IF a data object is passed as optional argument (https://hapijs.com/api#-hunauthenticatederror-data). This should easily be fixable by checking if a credentials object is returned by the validateFunc.

johnbrett commented 6 years ago

Why can't you return a credentials object with invalid false, if you don't want to return the actual credentials object? e.g.

if (!accessToken) {
  return { isValid: false, credentials: {} }
}
johnbrett commented 6 years ago

If this doesn't work, if you can submit a PR with a failing test I will take a look.

robinvdvleuten commented 6 years ago

@johnbrett that's the way I have it now and that works, but it feels inconsistent with Hapi's own authentication strategies like hapi-auth-cookie.

johnbrett commented 6 years ago

This is how hapi-auth-basic also works, so I'm not sure how it's inconsistent: https://github.com/hapijs/hapi-auth-basic/blob/master/lib/index.js#L74-L78

johnbrett commented 6 years ago

I'm going to close this due to inactivity, happy to continue the conversation, but I don't believe this to be an issue. Open to being persuaded otherwise though.