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.

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

'Authorization' header not sent when isAuthenticated() evaluates to false #152

Open
don-bluelinegrid opened this issue Sep 2, 2016 · 0 comments

Comments

@don-bluelinegrid
Copy link

don-bluelinegrid commented Sep 2, 2016

@paulvanbladel

Paul,

I've discovered what may be a bug, or at the least something that causes a problem supporting the OAuth use case of using 'refresh tokens'.

As background, we are using an OAuth-based authentication/authorization system, and the expectation is that we should provide an access-token in the Auth header (which aurelia-auth does for us), and we should also rely on the server to respond with an appropriate error (401 or 403) to a request that uses an invalid/expired token.

I'm in the process of implementing interceptor logic to handle this use case, and am running into an issue because of the code in aurelia-auth's tokenInterceptor() function. In the function, which is responsible for adding the Auth header to the request, the header is only added when the token is detected as being unexpired:

if (auth.isAuthenticated() && config.httpInterceptor) {

With this approach, this causes an AJAX/REST request to be sent with no Auth header at all - this is obviously invalid and will result in a server error. But, the server will not respond with a meaningful error, because the error cause is a missing Auth header - it cannot therefore respond with an error indicating the token expiration. So, the server will return a 400 Bad Request error status, which is not very indicative of the true error state.

I believe that most REST service implementations for authentication will be implemented with the expectation that the service will have the responsibility/concern for managing authentication and the associated tokens, and for detecting a state of invalidity of the token.

I understand why you may have implemented the aurelia-auth function this way, to allow the client to take control of this concern. However, I think this is confusing the concerns, and breaking the single point of responsibility in the service. This means that every client of a REST auth service would have to right interceptor code to handle the case of rejected requests and then figure out whether the cause of rejection is due to token expiration; whereas we could just let the REST service tell us that the specific cause of the rejection was token expiration. This also causes complications in the possible case where a token may have been programmatically deleted or expired prematurely through some server process - which would not necessarily be detected by the aurelia-auth code.

With the way the function is implemented currently, I would have to implement code that assumes all of the token expiration detection will be done on the client, and then check every request failure to see if it is due to token expiration.

I'd like to get your thought on this, and propose a possible solution/enhancement.

My suggestion would be to add a configuration parameter like:

responseTokenForce: false,

which would either keep the current behavior of omitting the responseToken header when the aurelia-auth module detects the token is expired (config value false); or would always include the responseToken header without checking for expiration (config value true). This would allow developers to control the behavior, and choose whether their REST service should be in control of this concern, or to allow the aurelia-auth plugin to manage the expiration detection. My opinion would be that the default value for such a new config option should be 'true' so that the responseToken header is always sent consistent with OAuth expectations, unless a developer explicitly chooses to filter it out.

Regards,
Don

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant