Skip to content
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

Add token revocation support #995

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

jacobweber
Copy link

My attempt to implement a fix for #806.

  • Moved some of the validation methods from AbstractGrant into a RequestValidatorTrait trait, so they can be used by non-grant classes. This trait includes some abstract methods, to get the client repository and the grant identifier. Those could be refactored into arguments to validateClient, if that's preferable.
  • Added RevokeTokenHandler class to handle revocation. The constructor requires the refresh token repository and the public key as arguments. Uses existing repository methods for revocation.
  • Added enableRevokeTokenHandler method to AuthorizationServer to be used during setup.
  • Added respondToRevokeTokenRequest method to AuthorizationServer, to be used in a POST request, similar to respondToAccessTokenRequest. CORS support is up to the application.
  • Example setup (based on this):
// Init our repositories
$clientRepository = new ClientRepository();
$accessTokenRepository = new AccessTokenRepository();
$scopeRepository = new ScopeRepository();
$refreshTokenRepository = new RefreshTokenRepository();

// Path to public and private keys
$privateKey = 'file://path/to/private.key';
//$privateKey = new CryptKey('file://path/to/private.key', 'passphrase'); // if private key has a pass phrase
$encryptionKey = 'lxZFUEsBCJ2Yb14IF2ygAHI5N4+ZAUXXaSeeJm6+twsUmIen'; // generate using base64_encode(random_bytes(32))
$publicKey = 'file://path/to/public.key';

// Setup the authorization server
$server = new \League\OAuth2\Server\AuthorizationServer(
    $clientRepository,
    $accessTokenRepository,
    $scopeRepository,
    $privateKey,
    $encryptionKey
);

$handler = new \League\OAuth2\Server\RevokeTokenHandler($refreshTokenRepository, $publicKey);

// Enable the revoke token handler on the server
$server->enableRevokeTokenHandler($handler);
  • Example implementation:
$app->post('/revoke', function (ServerRequestInterface $request, ResponseInterface $response) use ($app) {

    /* @var \League\OAuth2\Server\AuthorizationServer $server */
    $server = $app->getContainer()->get(AuthorizationServer::class);

    // Try to respond to the request
    try {
        return $server->respondToRevokeTokenRequest($request, $response);

    } catch (\League\OAuth2\Server\Exception\OAuthServerException $exception) {
        return $exception->generateHttpResponse($response);

    } catch (\Exception $exception) {
        $body = new Stream('php://temp', 'r+');
        $body->write($exception->getMessage());
        return $response->withStatus(500)->withBody($body);
    }
});
  • You can set $canRevokeAccessTokens in the RevokeTokenHandler constructor if you want to allow access tokens to be revoked, since the spec describes this as optional.
  • If you revoke a refresh token, and the above setting is true, it will also revoke the associated access token.
  • If you revoke an access token, it will not revoke the associated refresh token, since I don't think this is possible without adding a new repository method.

@jacobweber
Copy link
Author

BTW, I couldn't figure out how to get it to pass both tests, if I have a function that sometimes returns null. The style checker wants me to change "return null" to "return", but if I do that, I get a CI error.

@Sephster
Copy link
Member

Is there any alternative approach you can take? I am not a fan of blank returns and would prefer the code is more explicit.

For example, if token verification fails, could we use exceptions to issue an error rather than aborting the execution?

@jacobweber
Copy link
Author

Sure, I refactored it to avoid returning null, and just made it revoke the token right away.

@neodc
Copy link

neodc commented Oct 2, 2019

Is there any reason this PR is not merged? I'd like to use the revoke in one of my project

@simonhamp
Copy link

@neodc right now it looks like there are merge conflicts

@jacobweber
Copy link
Author

I think this may need some re-working – it was written for 7.x, and 8.0.0 has come out since then. Not sure I'll have time right now to do it, but if you guys are still interested in merging it, I can try.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants