From c3c8e50b6a1ac8f2b311f49166ebddb96cc829f9 Mon Sep 17 00:00:00 2001 From: Daniel Kreuer Date: Mon, 9 Mar 2020 23:58:18 +0100 Subject: [PATCH 1/8] Fix returning possible null values The function may return null values but the return type annotation restricts to return strings only. This workaround prevents PHP from crashing by returning an empty string instead - which is fine because the result is sent to output. --- Command/ListClientsCommand.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Command/ListClientsCommand.php b/Command/ListClientsCommand.php index 2cbcdc66..06f79b62 100644 --- a/Command/ListClientsCommand.php +++ b/Command/ListClientsCommand.php @@ -115,7 +115,7 @@ private function getRows(array $clients, array $columns): array ]; return array_map(static function (string $column) use ($values): string { - return $values[$column]; + return $values[$column] ?? ''; }, $columns); }, $clients); } From 011131dfce88baab49aaf3964dd50f68f0f630bc Mon Sep 17 00:00:00 2001 From: Daniel Kreuer Date: Tue, 10 Mar 2020 00:10:47 +0100 Subject: [PATCH 2/8] Add test for clients without secret --- Tests/Acceptance/ListClientsCommandTest.php | 22 +++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/Tests/Acceptance/ListClientsCommandTest.php b/Tests/Acceptance/ListClientsCommandTest.php index 67b5782f..30effb63 100644 --- a/Tests/Acceptance/ListClientsCommandTest.php +++ b/Tests/Acceptance/ListClientsCommandTest.php @@ -35,6 +35,28 @@ public function testListClients(): void $this->assertEquals(trim($expected), trim($output)); } + public function testListClientsWithClientHavingNoSecret(): void + { + $client = $this->fakeAClient('foobar', null); + $this->getClientManager()->save($client); + + $command = $this->command(); + $commandTester = new CommandTester($command); + $commandTester->execute([ + 'command' => $command->getName(), + ]); + $output = $commandTester->getDisplay(); + $expected = <<assertEquals(trim($expected), trim($output)); + } + public function testListClientsEmpty(): void { $command = $this->command(); From bbbf5ed8569f33d864be8e2a1c4ccd4d6c1a560a Mon Sep 17 00:00:00 2001 From: Daniel Kreuer Date: Wed, 11 Mar 2020 19:07:27 +0100 Subject: [PATCH 3/8] Fix test --- Tests/Acceptance/ListClientsCommandTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Acceptance/ListClientsCommandTest.php b/Tests/Acceptance/ListClientsCommandTest.php index 30effb63..2c89566f 100644 --- a/Tests/Acceptance/ListClientsCommandTest.php +++ b/Tests/Acceptance/ListClientsCommandTest.php @@ -50,7 +50,7 @@ public function testListClientsWithClientHavingNoSecret(): void ------------ -------- ------- -------------- ------------ identifier secret scope redirect uri grant type ------------ -------- ------- -------------- ------------ - foobar + foobar ------------ -------- ------- -------------- ------------ TABLE; From d56dfb9fa37c4f37f6db70fb09c07e1f12682079 Mon Sep 17 00:00:00 2001 From: Toni Peric Date: Tue, 24 Mar 2020 15:36:31 +0100 Subject: [PATCH 4/8] Extract public API for finding clients --- Manager/ClientManagerInterface.php | 5 ++--- Resources/config/services.xml | 2 ++ Service/ClientFinderInterface.php | 15 +++++++++++++++ 3 files changed, 19 insertions(+), 3 deletions(-) create mode 100644 Service/ClientFinderInterface.php diff --git a/Manager/ClientManagerInterface.php b/Manager/ClientManagerInterface.php index 71e6d34a..35169386 100644 --- a/Manager/ClientManagerInterface.php +++ b/Manager/ClientManagerInterface.php @@ -5,11 +5,10 @@ namespace Trikoder\Bundle\OAuth2Bundle\Manager; use Trikoder\Bundle\OAuth2Bundle\Model\Client; +use Trikoder\Bundle\OAuth2Bundle\Service\ClientFinderInterface; -interface ClientManagerInterface +interface ClientManagerInterface extends ClientFinderInterface { - public function find(string $identifier): ?Client; - public function save(Client $client): void; public function remove(Client $client): void; diff --git a/Resources/config/services.xml b/Resources/config/services.xml index d2260595..0a137bf0 100644 --- a/Resources/config/services.xml +++ b/Resources/config/services.xml @@ -4,6 +4,8 @@ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd"> + + diff --git a/Service/ClientFinderInterface.php b/Service/ClientFinderInterface.php new file mode 100644 index 00000000..8e298cdf --- /dev/null +++ b/Service/ClientFinderInterface.php @@ -0,0 +1,15 @@ + Date: Tue, 24 Mar 2020 15:36:38 +0100 Subject: [PATCH 5/8] Add public API for revoking all credentials --- .../TrikoderOAuth2Extension.php | 6 + .../config/doctrine/model/AccessToken.orm.xml | 3 + .../doctrine/model/AuthorizationCode.orm.xml | 3 + Resources/config/storage/doctrine.xml | 5 + .../DoctrineCredentialsRevoker.php | 101 ++++++++++++++++ Service/CredentialsRevokerInterface.php | 21 ++++ .../DoctrineCredentialsRevokerTest.php | 112 ++++++++++++++++++ 7 files changed, 251 insertions(+) create mode 100644 Service/CredentialsRevoker/DoctrineCredentialsRevoker.php create mode 100644 Service/CredentialsRevokerInterface.php create mode 100644 Tests/Acceptance/DoctrineCredentialsRevokerTest.php diff --git a/DependencyInjection/TrikoderOAuth2Extension.php b/DependencyInjection/TrikoderOAuth2Extension.php index ed595dc7..9ca20cec 100644 --- a/DependencyInjection/TrikoderOAuth2Extension.php +++ b/DependencyInjection/TrikoderOAuth2Extension.php @@ -41,6 +41,7 @@ use Trikoder\Bundle\OAuth2Bundle\Manager\ScopeManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Model\Scope as ScopeModel; use Trikoder\Bundle\OAuth2Bundle\Security\Authentication\Token\OAuth2TokenFactory; +use Trikoder\Bundle\OAuth2Bundle\Service\CredentialsRevoker\DoctrineCredentialsRevoker; final class TrikoderOAuth2Extension extends Extension implements PrependExtensionInterface, CompilerPassInterface { @@ -268,6 +269,11 @@ private function configureDoctrinePersistence(ContainerBuilder $container, array ->replaceArgument('$entityManager', $entityManager) ; + $container + ->getDefinition(DoctrineCredentialsRevoker::class) + ->replaceArgument('$entityManager', $entityManager) + ; + $container->setParameter('trikoder.oauth2.persistence.doctrine.enabled', true); $container->setParameter('trikoder.oauth2.persistence.doctrine.manager', $entityManagerName); } diff --git a/Resources/config/doctrine/model/AccessToken.orm.xml b/Resources/config/doctrine/model/AccessToken.orm.xml index 192c6d9c..10886306 100644 --- a/Resources/config/doctrine/model/AccessToken.orm.xml +++ b/Resources/config/doctrine/model/AccessToken.orm.xml @@ -17,5 +17,8 @@ + + + diff --git a/Resources/config/doctrine/model/AuthorizationCode.orm.xml b/Resources/config/doctrine/model/AuthorizationCode.orm.xml index f3930596..58eafedd 100644 --- a/Resources/config/doctrine/model/AuthorizationCode.orm.xml +++ b/Resources/config/doctrine/model/AuthorizationCode.orm.xml @@ -17,5 +17,8 @@ + + + diff --git a/Resources/config/storage/doctrine.xml b/Resources/config/storage/doctrine.xml index 90a7f617..a3b2f732 100644 --- a/Resources/config/storage/doctrine.xml +++ b/Resources/config/storage/doctrine.xml @@ -12,6 +12,11 @@ + + + + + diff --git a/Service/CredentialsRevoker/DoctrineCredentialsRevoker.php b/Service/CredentialsRevoker/DoctrineCredentialsRevoker.php new file mode 100644 index 00000000..9158e706 --- /dev/null +++ b/Service/CredentialsRevoker/DoctrineCredentialsRevoker.php @@ -0,0 +1,101 @@ +entityManager = $entityManager; + } + + public function revokeCredentialsForUser(UserInterface $user): void + { + $userIdentifier = $user->getUsername(); + + $this->entityManager->createQueryBuilder() + ->update(AccessToken::class, 'at') + ->set('at.revoked', true) + ->where('at.userIdentifier = :userIdentifier') + ->setParameter('userIdentifier', $userIdentifier) + ->getQuery() + ->execute(); + + $queryBuilder = $this->entityManager->createQueryBuilder(); + $queryBuilder + ->update(RefreshToken::class, 'rt') + ->set('rt.revoked', true) + ->where($queryBuilder->expr()->in( + 'rt.accessToken', + $this->entityManager->createQueryBuilder() + ->select('at.identifier') + ->from(AccessToken::class, 'at') + ->where('at.userIdentifier = :userIdentifier') + ->getDQL() + )) + ->setParameter('userIdentifier', $userIdentifier) + ->getQuery() + ->execute(); + + $this->entityManager->createQueryBuilder() + ->update(AuthorizationCode::class, 'ac') + ->set('ac.revoked', true) + ->where('ac.userIdentifier = :userIdentifier') + ->setParameter('userIdentifier', $userIdentifier) + ->getQuery() + ->execute(); + } + + public function revokeCredentialsForClient(Client $client): void + { + $doctrineClient = $this->entityManager + ->getRepository(Client::class) + ->findOneBy(['identifier' => $client->getIdentifier()]); + + $this->entityManager->createQueryBuilder() + ->update(AccessToken::class, 'at') + ->set('at.revoked', true) + ->where('at.client = :client') + ->setParameter('client', $doctrineClient) + ->getQuery() + ->execute(); + + $queryBuilder = $this->entityManager->createQueryBuilder(); + $queryBuilder->update(RefreshToken::class, 'rt') + ->set('rt.revoked', true) + ->where($queryBuilder->expr()->in( + 'rt.accessToken', + $this->entityManager->createQueryBuilder() + ->select('at.identifier') + ->from(AccessToken::class, 'at') + ->where('at.client = :client') + ->getDQL() + )) + ->setParameter('client', $doctrineClient) + ->getQuery() + ->execute(); + + $this->entityManager->createQueryBuilder() + ->update(AuthorizationCode::class, 'ac') + ->set('ac.revoked', true) + ->where('ac.client = :client') + ->setParameter('client', $doctrineClient) + ->getQuery() + ->execute(); + } +} diff --git a/Service/CredentialsRevokerInterface.php b/Service/CredentialsRevokerInterface.php new file mode 100644 index 00000000..65cd7d9e --- /dev/null +++ b/Service/CredentialsRevokerInterface.php @@ -0,0 +1,21 @@ +client->getContainer()->get('doctrine.orm.entity_manager'); + + $em->persist($client = new Client('client', 'secret')); + + $authCode = $this->buildAuthCode('foo', '+1 minute', $client, $userIdentifier); + $accessToken = $this->buildAccessToken('bar', '+1 minute', $client, $userIdentifier); + $refreshToken = $this->buildRefreshToken('baz', '+1 minute', $accessToken); + + $em->persist($authCode); + $em->persist($accessToken); + $em->persist($refreshToken); + $em->flush(); + + $revoker = new DoctrineCredentialsRevoker($em); + + $revoker->revokeCredentialsForUser(FixtureFactory::createUser()); + + $em->refresh($authCode); + $em->refresh($accessToken); + $em->refresh($refreshToken); + + $this->assertTrue($authCode->isRevoked()); + $this->assertTrue($accessToken->isRevoked()); + $this->assertTrue($refreshToken->isRevoked()); + } + + public function testRevokesAllCredentialsForClient(): void + { + /** @var EntityManagerInterface $em */ + $em = $this->client->getContainer()->get('doctrine.orm.entity_manager'); + + $em->persist($client = new Client('acme', 'secret')); + + $authCode = $this->buildAuthCode('foo', '+1 minute', $client, 'john'); + $accessToken = $this->buildAccessToken('bar', '+1 minute', $client); + $refreshToken = $this->buildRefreshToken('baz', '+1 minute', $accessToken); + + $em->persist($authCode); + $em->persist($accessToken); + $em->persist($refreshToken); + $em->flush(); + + $revoker = new DoctrineCredentialsRevoker($em); + + $revoker->revokeCredentialsForClient($client); + + $em->refresh($authCode); + $em->refresh($accessToken); + $em->refresh($refreshToken); + + $this->assertTrue($authCode->isRevoked()); + $this->assertTrue($accessToken->isRevoked()); + $this->assertTrue($refreshToken->isRevoked()); + } + + private function buildRefreshToken(string $identifier, string $modify, AccessToken $accessToken): RefreshToken + { + return new RefreshToken( + $identifier, + new DateTimeImmutable($modify), + $accessToken + ); + } + + private function buildAccessToken(string $identifier, string $modify, Client $client, ?string $userIdentifier = null): AccessToken + { + return new AccessToken( + $identifier, + new DateTimeImmutable($modify), + $client, + $userIdentifier, + [] + ); + } + + private function buildAuthCode(string $identifier, string $modify, Client $client, ?string $userIdentifier = null): AuthorizationCode + { + return new AuthorizationCode( + $identifier, + new DateTimeImmutable($modify), + $client, + $userIdentifier, + [] + ); + } +} From 912c5418d856dfbd62b17c47160f42ec875440f1 Mon Sep 17 00:00:00 2001 From: Antonio Pauletich Date: Tue, 25 Feb 2020 22:38:11 +0100 Subject: [PATCH 6/8] Add support for registering custom grant types --- .../TrikoderOAuth2Extension.php | 4 ++ .../AuthorizationServer/GrantConfigurator.php | 27 +++++++++ .../GrantTypeInterface.php | 13 ++++ Model/Grant.php | 7 --- README.md | 1 + Resources/config/services.xml | 5 ++ Tests/Fixtures/FakeGrant.php | 30 ++++++++++ .../AuthorizationServerCustomGrantTest.php | 32 ++++++++++ Tests/TestKernel.php | 7 +++ docs/implementing-custom-grant-type.md | 59 +++++++++++++++++++ 10 files changed, 178 insertions(+), 7 deletions(-) create mode 100644 League/AuthorizationServer/GrantConfigurator.php create mode 100644 League/AuthorizationServer/GrantTypeInterface.php create mode 100644 Tests/Fixtures/FakeGrant.php create mode 100644 Tests/Integration/AuthorizationServerCustomGrantTest.php create mode 100644 docs/implementing-custom-grant-type.md diff --git a/DependencyInjection/TrikoderOAuth2Extension.php b/DependencyInjection/TrikoderOAuth2Extension.php index ed595dc7..0a2431f4 100644 --- a/DependencyInjection/TrikoderOAuth2Extension.php +++ b/DependencyInjection/TrikoderOAuth2Extension.php @@ -34,6 +34,7 @@ use Trikoder\Bundle\OAuth2Bundle\DBAL\Type\RedirectUri as RedirectUriType; use Trikoder\Bundle\OAuth2Bundle\DBAL\Type\Scope as ScopeType; use Trikoder\Bundle\OAuth2Bundle\EventListener\ConvertExceptionToResponseListener; +use Trikoder\Bundle\OAuth2Bundle\League\AuthorizationServer\GrantTypeInterface; use Trikoder\Bundle\OAuth2Bundle\Manager\Doctrine\AccessTokenManager; use Trikoder\Bundle\OAuth2Bundle\Manager\Doctrine\AuthorizationCodeManager; use Trikoder\Bundle\OAuth2Bundle\Manager\Doctrine\ClientManager; @@ -70,6 +71,9 @@ public function load(array $configs, ContainerBuilder $container) 'method' => 'onKernelException', 'priority' => $config['exception_event_listener_priority'], ]); + + $container->registerForAutoconfiguration(GrantTypeInterface::class) + ->addTag('trikoder.oauth2.authorization_server.grant'); } /** diff --git a/League/AuthorizationServer/GrantConfigurator.php b/League/AuthorizationServer/GrantConfigurator.php new file mode 100644 index 00000000..e0e9ea12 --- /dev/null +++ b/League/AuthorizationServer/GrantConfigurator.php @@ -0,0 +1,27 @@ +grants = $grants; + } + + public function __invoke(AuthorizationServer $authorizationServer): void + { + foreach ($this->grants as $grant) { + $authorizationServer->enableGrantType($grant, $grant->getAccessTokenTTL()); + } + } +} diff --git a/League/AuthorizationServer/GrantTypeInterface.php b/League/AuthorizationServer/GrantTypeInterface.php new file mode 100644 index 00000000..cb625747 --- /dev/null +++ b/League/AuthorizationServer/GrantTypeInterface.php @@ -0,0 +1,13 @@ +grant = $grant; } diff --git a/README.md b/README.md index 2a4b3668..d94c055e 100644 --- a/README.md +++ b/README.md @@ -151,6 +151,7 @@ security: * [Basic setup](docs/basic-setup.md) * [Controlling token scopes](docs/controlling-token-scopes.md) * [Password grant handling](docs/password-grant-handling.md) +* [Implementing custom grant type](docs/implementing-custom-grant-type.md) ## Development diff --git a/Resources/config/services.xml b/Resources/config/services.xml index d2260595..97579c65 100644 --- a/Resources/config/services.xml +++ b/Resources/config/services.xml @@ -70,6 +70,10 @@ + + + + @@ -77,6 +81,7 @@ + diff --git a/Tests/Fixtures/FakeGrant.php b/Tests/Fixtures/FakeGrant.php new file mode 100644 index 00000000..0eb992e9 --- /dev/null +++ b/Tests/Fixtures/FakeGrant.php @@ -0,0 +1,30 @@ +get(AuthorizationServer::class); + + $reflectionClass = new ReflectionClass(AuthorizationServer::class); + $reflectionProperty = $reflectionClass->getProperty('enabledGrantTypes'); + $reflectionProperty->setAccessible(true); + + $enabledGrantTypes = $reflectionProperty->getValue($authorizationServer); + + $this->assertArrayHasKey('fake_grant', $enabledGrantTypes); + $this->assertInstanceOf(FakeGrant::class, $enabledGrantTypes['fake_grant']); + $this->assertEquals(new DateInterval('PT5H'), $enabledGrantTypes['fake_grant']->getAccessTokenTTL()); + } +} diff --git a/Tests/TestKernel.php b/Tests/TestKernel.php index acf3bc28..bc46b86d 100644 --- a/Tests/TestKernel.php +++ b/Tests/TestKernel.php @@ -27,6 +27,7 @@ use Trikoder\Bundle\OAuth2Bundle\Manager\ClientManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Manager\RefreshTokenManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Manager\ScopeManagerInterface; +use Trikoder\Bundle\OAuth2Bundle\Tests\Fixtures\FakeGrant; use Trikoder\Bundle\OAuth2Bundle\Tests\Fixtures\FixtureFactory; use Trikoder\Bundle\OAuth2Bundle\Tests\Fixtures\SecurityTestController; use Trikoder\Bundle\OAuth2Bundle\Tests\Support\SqlitePlatform; @@ -196,6 +197,7 @@ protected function configureContainer(ContainerBuilder $container, LoaderInterfa $this->configureControllers($container); $this->configurePsrHttpFactory($container); $this->configureDatabaseServices($container); + $this->registerFakeGrant($container); } private function exposeManagerServices(ContainerBuilder $container): void @@ -298,6 +300,11 @@ private function configureDatabaseServices(ContainerBuilder $container): void ; } + private function registerFakeGrant(ContainerBuilder $container): void + { + $container->register(FakeGrant::class)->setAutoconfigured(true); + } + private function determinePsrHttpFactory(): void { $psrHttpProvider = getenv('PSR_HTTP_PROVIDER'); diff --git a/docs/implementing-custom-grant-type.md b/docs/implementing-custom-grant-type.md new file mode 100644 index 00000000..b387cf6b --- /dev/null +++ b/docs/implementing-custom-grant-type.md @@ -0,0 +1,59 @@ +# Implementing custom grant type + +1. Create a class that implements the `\Trikoder\Bundle\OAuth2Bundle\League\AuthorizationServer\GrantTypeInterface` interface. + + Example: + + ```php + foo = $foo; + } + + public function getIdentifier() + { + return 'fake_grant'; + } + + public function respondToAccessTokenRequest(ServerRequestInterface $request, ResponseTypeInterface $responseType, DateInterval $accessTokenTTL) + { + return new Response(); + } + + public function getAccessTokenTTL(): ?DateInterval + { + return new DateInterval('PT5H'); + } + } + ``` + +1. In order to enable the new grant type in the authorization server you must register the service in the container. +The service must be autoconfigured or you have to manually tag it with the `trikoder.oauth2.authorization_server.grant` tag: + + ```yaml + services: + _defaults: + autoconfigure: true + + App\Grant\FakeGrant: ~ + ``` From 475212957004c2240ffeb7e2da53c77f0e192947 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petar=20Obradovi=C4=87?= Date: Thu, 9 Apr 2020 17:11:26 +0200 Subject: [PATCH 7/8] Update changelog --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0484deff..48d23fa8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). +## [3.1.0] - 2020-04-09 +### Added +- Ability to revoke credentials (access tokens, authorization codes and refresh tokens) programmatically ([fee109d](https://github.com/trikoder/oauth2-bundle/commit/fee109da2d52d73dfc81501d0af3d66216f09de6)) +- Support for registering custom grant types ([6b37588](https://github.com/trikoder/oauth2-bundle/commit/6b3758807b7cca6835c00504f1632ea78de563a5)) + +### Fixed +- Console command `trikoder:oauth2:list-clients` not being able to list clients without a secret ([da38b7a](https://github.com/trikoder/oauth2-bundle/commit/da38b7ab77060b4d43aca405559d5ffbd7a34d8d)) + ## [3.0.0] - 2020-02-26 ### Added - Ability to restrict clients from using the `plain` challenge method during PKCE ([4562a1f](https://github.com/trikoder/oauth2-bundle/commit/4562a1ff306375fd651aa91c85d0d4fd6f4c1b13)) From 7eeb932ee34d431ef2078f33262667a961af8926 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petar=20Obradovi=C4=87?= Date: Thu, 9 Apr 2020 17:20:30 +0200 Subject: [PATCH 8/8] Add upgrade notes --- UPGRADE.md | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 7e4d99eb..a2667c71 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,7 +1,21 @@ # Upgrade -Here you will find upgrade steps between major releases. +Here you will find upgrade steps between releases. -## From 2.x to 3.x +## From 3.0 to 3.1 + +### SQL schema changes + +The bundle makes modifications to the existing schema. You will need to run the Doctrine schema update process to sync the changes: + +```sh +bin/console doctrine:schema:update +``` + +The schema changes include: + +* New `userIdentifier` index on the `oauth2_access_token` table + +## From 2.x to 3.0 ### Console command changes