Set WWW-Authenticate header for invalid requests

[FStefanni]

Summary

This adds the WWW-Authenticate header for InvalidRequestError, InvalidTokenError,
and InsufficientScopeError, as specified in RFC 6750, Section 3
The original pr is oauthjs/node-oauth2-server#646

Linked issue(s)

Fixes issue #89, point 18.

Added tests?

Yes

OAuth2 standard

RFC 6750, Section 3

[jorenvandeweyer]

Looks fine for me. I can not find that the header "MUST be" included like the original pull request claimed. Personally I do not see any harm in including this context.

[jankapunkt]

Please set target to development, not master

[jankapunkt]

please set target to development, not master

[FStefanni]

Hi,

done

Regards

[jankapunkt]

Two things to mention here regarding RFC6750/section3:

If the request lacks any authentication information (e.g., the client was unaware that authentication is necessary or attempted using an unsupported authentication method), the resource server SHOULD NOT include an error code or other error information.

This should be taken into consideration.

And in response to a protected resource request with an authentication attempt using an expired access token:

HTTP/1.1 401 Unauthorized
WWW-Authenticate: Bearer realm="example",
                  error="invalid_token",
                  error_description="The access token expired"

The example also shows, that the error_description is included. I think we should add the description as well but only considering the above mentioned issue that the client is aware of the authentication)

[FStefanni]

Hi,

so, if I understand correctly what you mean and what the standard says:

1. The proposed pr is fine, since it does not regards the UnauthorizedRequestError case
2. You suggest to add a error_description instead of only the error (even if the error_description is optional according with the standard)

I think that adding the description could be useful, but not so straightforward, since it should report the specific failure reason IMHO. So I propose:

• If this pr is fine, just accept it
• About the optional description, let's open an issue as a reminder for a possible improvement, and in case let's propose a specific pr

Regards

[jankapunkt]

@FStefanni the optional description can remain optional that's fine to me but I wanted to ask you as the author of the PR if you are sure, that you don't send error information in case the request contains no authentication method or info. From what I see there is no check about this but maybe this happens already at some other place?

[FStefanni]

Hi,

to me, it seems fine: please see getTokenFromRequest() at line 117.
There, there are the checks about authorization info, and if it arrives to the end, it means that no info was provided.
Also corner cases seems fine to me: the checking methods are called only if the corresponding info is found.

But a second check by someone else is welcome.

Regards.

[jankapunkt]

From my end this seems to be okay but I'd like to have a second check on the auth-checking issue (see prior discussion)

[jankapunkt]

From my point this is resolved and approved. Can someone else second-check, please?

[jankapunkt]

@jorenvandeweyer sorry for being late this is now finally merged