-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Support token revocation and introspection #1473
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
base: master
Are you sure you want to change the base?
Support token revocation and introspection #1473
Conversation
|
@Sephster , as mentioned #1453 (comment), it would be great to have more maintainers for this repository! @hafezdivandari is an excellent professional with extensive knowledge of OAuth2. Having more people to support the project would certainly help its growth and maintenance! 🚀 |
| use EmitterAwarePolyfill; | ||
| use CryptTrait; | ||
|
|
||
| protected ClientRepositoryInterface $clientRepository; | ||
|
|
||
| protected AccessTokenRepositoryInterface $accessTokenRepository; | ||
|
|
||
| protected RefreshTokenRepositoryInterface $refreshTokenRepository; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From AbstractGrant class.
| public function setClientRepository(ClientRepositoryInterface $clientRepository): void | ||
| { | ||
| $this->clientRepository = $clientRepository; | ||
| } | ||
|
|
||
| public function setAccessTokenRepository(AccessTokenRepositoryInterface $accessTokenRepository): void | ||
| { | ||
| $this->accessTokenRepository = $accessTokenRepository; | ||
| } | ||
|
|
||
| public function setRefreshTokenRepository(RefreshTokenRepositoryInterface $refreshTokenRepository): void | ||
| { | ||
| $this->refreshTokenRepository = $refreshTokenRepository; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From AbstractGrant class.
| /** | ||
| * Validate the client. | ||
| * | ||
| * @throws OAuthServerException | ||
| */ | ||
| protected function validateClient(ServerRequestInterface $request): ClientEntityInterface | ||
| { | ||
| [$clientId, $clientSecret] = $this->getClientCredentials($request); | ||
|
|
||
| $client = $this->getClientEntityOrFail($clientId, $request); | ||
|
|
||
| if ($client->isConfidential()) { | ||
| if ($clientSecret === '') { | ||
| throw OAuthServerException::invalidRequest('client_secret'); | ||
| } | ||
|
|
||
| if ( | ||
| $this->clientRepository->validateClient( | ||
| $clientId, | ||
| $clientSecret, | ||
| $this instanceof GrantTypeInterface ? $this->getIdentifier() : null | ||
| ) === false | ||
| ) { | ||
| $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request)); | ||
|
|
||
| throw OAuthServerException::invalidClient($request); | ||
| } | ||
| } | ||
|
|
||
| return $client; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved from AbstractGrant class.
| $this->clientRepository->validateClient( | ||
| $clientId, | ||
| $clientSecret, | ||
| $this instanceof GrantTypeInterface ? $this->getIdentifier() : null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is the only change on this method, since getIdentifier() method is defined on GrantTypeInterface only.
| /** | ||
| * Wrapper around ClientRepository::getClientEntity() that ensures we emit | ||
| * an event and throw an exception if the repo doesn't return a client | ||
| * entity. | ||
| * | ||
| * This is a bit of defensive coding because the interface contract | ||
| * doesn't actually enforce non-null returns/exception-on-no-client so | ||
| * getClientEntity might return null. By contrast, this method will | ||
| * always either return a ClientEntityInterface or throw. | ||
| * | ||
| * @throws OAuthServerException | ||
| */ | ||
| protected function getClientEntityOrFail(string $clientId, ServerRequestInterface $request): ClientEntityInterface | ||
| { | ||
| $client = $this->clientRepository->getClientEntity($clientId); | ||
|
|
||
| if ($client instanceof ClientEntityInterface === false) { | ||
| $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request)); | ||
| throw OAuthServerException::invalidClient($request); | ||
| } | ||
|
|
||
| return $client; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved from AbstractGrant class.
| use Psr\Http\Message\ResponseInterface; | ||
| use Psr\Http\Message\ServerRequestInterface; | ||
|
|
||
| class TokenIntrospectionHandler extends AbstractTokenHandler |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class implements "Token Introspection RFC7662". This class also allows customizing the introspection response by injecting a IntrospectionResponseTypeInterface instance.
| use Psr\Http\Message\ResponseInterface; | ||
| use Psr\Http\Message\ServerRequestInterface; | ||
|
|
||
| class TokenRevocationHandler extends AbstractTokenHandler |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class implements "Token Revocation RFC7009".
| use DateTimeInterface; | ||
| use Psr\Http\Message\ResponseInterface; | ||
|
|
||
| class IntrospectionResponse implements IntrospectionResponseTypeInterface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class implements Introspection Response. RFC7662 section 2.2
|
|
||
| use Psr\Http\Message\ResponseInterface; | ||
|
|
||
| interface IntrospectionResponseTypeInterface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Defines an interface for introspection response type.
| use Psr\Http\Message\ResponseInterface; | ||
| use Psr\Http\Message\ServerRequestInterface; | ||
|
|
||
| class TokenServer implements EmitterAwareInterface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class defines 2 methods to respond to token revocation and token introspection requests. Also allows injecting customized token revocation/introspection handlers.
This class is inspired by AuthorizationServer and ResourceServer classes.
|
Hi @Sephster, by any chance, did you have the time to check this PR? I'd appreciate any comments or feedback. |
|
Sorry I hadn't realised it was ready for review. I'll get to it as soon as I can but am going to clear some outstanding issues prior. Thanks for your work on this |
|
@Sephster thanks so much for all your work maintaining this library! I was just wondering whether you had even a rough idea of when you might get to looking at this PR? It would be super helpful for the problem I'm currently trying to solve! 😅 |
|
Hey @Sephster, I feel the OAuth Server is a bit incomplete without this. When you have a chance, could you take a look? The PR is large, but I did my best to make it easy to review. It’s also difficult to keep it open for too long since other merged PRs may cause conflicts. Thanks. |
|
I'll be picking this up next. Thanks |
|
Thanks @Sephster! I have the implementation PR ready for Laravel Passport if you’d like to see it in action: laravel/passport#1858 |
|
I'm on holiday with the family for a few days but still reviewing and hoping to get a first response to you soon. Thanks for the patience |
| public static function unsupportedTokenType(?string $hint = null): static | ||
| { | ||
| return new static( | ||
| 'The authorization server does not support the revocation of the presented token type.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method name implies the error isn't specifically related to token revocation, but the message itself does. Should the message be reworded to make it more generic, or should the method name be updated to imply it's specifically for token revocation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review. Like other existing exception methods, the method name is based on the error code and message is based on its description according to the revocation RFC.
This error code has not been mentioned on the introspection RFC.
Although this new exception is added on this PR, it is not been used because at the current implementation, we support revocation of both access and refresh tokens. But I decided to add it anyway as it is defined on the RFC and we may have a config property to disable revocation of access token type in the future.
|
Hey @Sephster Any updates on this one? |
|
Was working on this last night. There are quite a few aspects to this so it is taking a while. I have two RFCs I need to check for compliance, significant architecture changes, and tagged issues I need to ensure are satisfied. It is a big review but am quietly plugging away on it. I'm going to try dedicate an evening to it soon |
Fixes #806
Fixes #1350
Closes #1255
Closes #1135
Closes #995
Closes #925
It's much easier to implement Token Revocation RFC7009 and Token Introspection RFC7662 at the same time! These 2 RFCs have most of the logic in common:
"application/x-www-form-urlencoded" data:
tokenis required, accepts either an access token or a refresh token.token_type_hintis optional to help the server optimize the token lookup.access_tokenorrefresh_token.Authorization: Basic {client_credentials}headerclient_idandclient_secretparameters.400:invalid_requestthe requiredtokenparameter was not sent.401:invalid_clientthe request is not authorized. Client credentials were not sent or invalid.200: The token is revoked, expired, doesn't exists, or was not issued to the client making the request:activefield set tofalse.{ "active": false }200: The token is valid, active, and was issued to the client making the request:activefield set totrue.{ "active": true, ... }Implementation
Summary
Summary of file changes in order of appearance.
New
AbstractHandlerclassAbstractGrantandAbstractTokenHandlerclasses.clientRepository,accessTokenRepository, andrefreshTokenRepositoryproperties have been moved to this class from childAbstractGrantclass.setClientRepository,setAccessTokenRepository,setRefreshTokenRepository,getClientCredentials,parseParam,getRequestParameter,getBasicAuthCredentials,getQueryStringParameter,getCookieParameter,getServerParameter, andvalidateEncryptedRefreshTokenmethods have been moved to this class from childAbstractGrantclass without any change.validateClientmethod from childAbstractGranthas been moved to this class with a minor change: The call togetIdentifier()isGrantTypeInterfacespecific.getClientEntityOrFailmethod from childAbstractGranthas been moved to this class. This method is also overridden on childAbstractGrant: Call tosupportsGrantTypemethod isAbstracGrantspecific.validateEncryptedRefreshTokenis extracted fromRefreshTokenGrant::validateOldRefreshTokenmethod.Change
AuthorizationValidators\BearerTokenValidatorclassJwtValidatorInterfaceinterface.validateAuthorizationmethod into a newvalidateJwtmethod.validateJwtmethod also accepts optional$clientIdargument. If a$clientIdis provided, it verifies whether the token was issued to the given client.New
AuthorizationValidators\JwtValidatorInterfaceinterfaceBearerTokenValidatorclass.validateJwtmethod.AbstractTokenHandler::setJwtValidator()method.Change
Exception\OAuthServerExceptionclassunsupportedTokenTypemethod.Change
Grant\AbstractGrantclassAbstractHandlerclass.AbstractHandlerclass.getClientEntityOrFailmethod.Change
Grant\RefreshTokenGrantclassvalidateOldRefreshTokenmethod has been extracted into a new parent AbstractHandler::validateEncryptedRefreshToken method.New
Handlers\AbstractTokenHandlerclassAbstractHandlerclass.TokenHandlerInterfaceinterface.setPublicKeyandsetJwtValidatormethods that to allow injecting a customized JWT validator.validateToken,validateAccessTokenandvalidateRefreshTokenmethods that have been used byTokenIntrospectionHandlerandTokenRevocationHandlerclasses.New
Handlers\TokenHandlerInterfaceinterfaceAbstractTokenHandlerclass.setTokenRevocationHandlerandsetTokenIntrospectionHandlermethods of theTokenServerclass.New
Handlers\TokenIntrospectionHandlerclassAbstractTokenHandlerclass.respondToRequestmethod that responds to "Token Introspection RFC7662" request.setResponseTypemethod that allows injecting a customized response type.New
Handlers\TokenRevocationHandlerclassAbstractTokenHandlerclass.respondToRequestmethod that responds to "Token Revocation RFC7009" request.New
ResponseTypes\IntrospectionResponseclassIntrospectionResponseTypeInterfaceinterface.generateHttpResponsemethod to generate "Token Revocation RFC7009" response.New
ResponseTypes\IntrospectionResponseTypeInterfaceclassIntrospectionResponseclass.TokenIntrospectionHandler::setResponseTypemethod that allows injecting a customized response type.New
TokenServerclassrespondToTokenRevocationRequestandrespondToTokenIntrospectionRequestmethods to respond to the respective request.setTokenRevocationHandlerandsetTokenIntrospectionHandlermethods that allows injecting customized handlers.