zalando-stups / play-zhewbacca

Play! framework library to protect REST endpoint by OAuth2 token verification - THIS PROJECT IS NOT LONGER ACTIVELY MAINTAINED
MIT License
22 stars 13 forks source link

HTTP Specification is not respected in regard to return codes #53

Closed slavaschmidt closed 6 years ago

slavaschmidt commented 6 years ago

The HTTP specification is very clear in regard to the return codes which are expected to be returned by the server in the case of absent/invalid/outdated credentials.

Current version of the library does not respect the RFC in at least in two cases:

  1. DenyAllRule always returns 403 Forbidden error code, even in the case there were no credentials provided. As per specification (https://tools.ietf.org/html/rfc7235#section-3.1), an error code 401 Unauthorised and a challenge header should be returned in this case.
  2. Another case is IAMClient which returns 403 Forbidden error code in any case if a token string can't be validated by the remote endpoint. There are multiple reasons why validation can fail including, but not limited to the remote endpoint availability, malformed token and expired token. It is arguable that in these cases the response codes should be 500, 401 and 401 respectively to comply with the RFC.

Our API uses play-zhewbacca as a security filter and described behaviour breaks expectations of the client-side software. For instance, android devices do not refresh expired tokens because 403 error code returned by the server implies that the token is not expired yet.

I'd suggest to introduce one more AuthResult value, for example AuthTokenInsufficient or something which would encode the case of a valid token but insufficient rights/scopes. This type would be converted to the appropriate 403 Forbidden error code as per https://tools.ietf.org/html/rfc7231#section-6.5.3.

In any other case an error code 401 Unauthorised should be returned.

slavaschmidt commented 6 years ago

A good summary of erroneous conditions with appropriate error codes: https://tools.ietf.org/html/rfc6750#section-3.1

dmitrykrivaltsevich commented 6 years ago

@slavaschmidt , thank you for the reporting. Will try to solve this ASAP