zf-fr / zfr-oauth2-server

PHP library for creating an OAuth 2 server (currently proof of concept)
BSD 3-Clause "New" or "Revised" License
36 stars 13 forks source link

Q: Inconsistencies around InvalidAccessTokenException and OAuth2Exception #70

Closed basz closed 7 years ago

basz commented 7 years ago

There are OAuth2 error's defined. We have those in OAuth2Exception.

A special case is for the 'Access token has expired or has been deleted'. It is thrown as an InvalidAccessTokenException and catched by the ResourceServerMiddleware which converts it into a JsonResponse.

It seems to me that that is an 'access_denied' error from the spec.

So my question is; I've noted that the ResourceServerMiddleware catches and creates a response, but the AuthorisationService catches error and creates responses internally.

I don't like this inconsistency? Why do you think this InvalidAccessTokenException deserves it's own class? It doesn't even extend OAuth2Exception but extends \InvalidArgumentException.

Additionally there are more errors defined in OAuth2Exception then there are in the spec and not all of them are used.

bakura10 commented 7 years ago

I think that I did that because the "InvalidAccessToken" is not an error defined per spec (while in the Authorization Server those exception are thrown due to some conditions that are defined in the spec, then converted to a response).

Also the AuthorizationServer is meant to be used standalone: it accepts a request and returns an answer, so it makes sense to me that it catches the exception and transform it to a response: all the rules defined in the spec are handled by this service and converted to a standardized response.

How would you prefer to tackle that?

basz commented 7 years ago

i wont, it ok