Error messages/codes should be more helpful & match spec.

Here's a basic pass at resolving this issue. The underlying OAuth library does have decent error messages (and varying status codes) for the various cases when an access or refresh token is not valid. Instead of eating the exception and only logging to watchdog, this rethrows as a symphony HttpException which allows the underlying error to be passed all the way back to the response.

Reapplies patch against 8.x-3.x branch and fixes formatting issues.

IMHO, I think this is still applicable--I still use this patch anywhere I'm using Simple OAuth.

I want for the client application(s) of our service to be able to understand why a request was rejected. Without this patch, the server responds with a generic access denied status code. With this patch, the response body will indicate if the request was rejected due to an access token or refresh token being expired or revoked.

Adjusting the patch to avoid sending the exception headers--it seems that has an unintended side effect of changing the response format.

To put a bit of a finer point on it, certain client implementations (e.g., https://github.com/shoutem/fetch-token-intercept) use the HTTP status code to more directly map the error to the spec and its recovery/refresh strategy.

Adding related issue that pertains to 401 responses prompting a retry on the client-side and a Drupal core bug poisoning the route matching cache.

Found another spot where this needed to be applied; also updated the previous patch to pass along the previous exception and break up the lines to under 80 chars.

Actually... the test failures point to an interesting angle, which should have been more clear to me earlier on: this is a change in behaviour in so far as the authentication provider throws an error when the provided Bearer token is invalid; the tests rely on the previous behaviour, where the provider returned NULL, meaning no user was found and thus the anonymous user would be loaded. (More on why this is bad, below.)

For the reasons already explored above, this is undesirable in the context of OAuth. I don't think the current behaviour is strictly inconsistent with RFC 6750, but it also makes a SHOULD recommendation regarding invalid access tokens:

invalid_token
         The access token provided is expired, revoked, malformed, or
         invalid for other reasons.  The resource SHOULD respond with
         the HTTP 401 (Unauthorized) status code.  The client MAY
         request a new access token and retry the protected resource
         request.

Sending a 401/other compliant response helps clients choose what to do next; a 200 response with the anonymous user's access reflected in the content is indecipherable from a failed authentication via Bearer token.

Updated patch enclosed with the tests updated to reflect this; as it's a change in behaviour, I think it also requires a change record. I'm happy to draft one up.

Telling Guzzle not to throw an exception on errors.