From aff8bfb0d3f389667fae9c985ab876388fd1caca Mon Sep 17 00:00:00 2001 From: Simon Van Accoleyen Date: Tue, 25 Mar 2025 19:15:10 +0100 Subject: [PATCH 1/3] feat: Add support for device code grant --- config/routes.php | 4 + config/services.php | 40 ++++ config/storage/doctrine.php | 9 + config/storage/in_memory.php | 9 + docs/basic-setup.md | 5 +- docs/device-code-grant.md | 76 +++++++ docs/index.md | 11 +- src/Command/ClearExpiredTokensCommand.php | 32 ++- src/Controller/DeviceCodeController.php | 96 +++++++++ src/DependencyInjection/Configuration.php | 22 ++ .../LeagueOAuth2ServerExtension.php | 26 +++ src/Entity/DeviceCode.php | 15 ++ src/Manager/DeviceCodeManagerInterface.php | 21 ++ src/Manager/Doctrine/DeviceCodeManager.php | 60 ++++++ src/Manager/InMemory/DeviceCodeManager.php | 50 +++++ src/Model/DeviceCode.php | 193 ++++++++++++++++++ src/Model/DeviceCodeInterface.php | 48 +++++ src/Persistence/Mapping/Driver.php | 28 +++ src/Repository/ClientRepository.php | 2 +- src/Repository/DeviceCodeRepository.php | 182 +++++++++++++++++ .../DoctrineCredentialsRevoker.php | 19 ++ .../Acceptance/AuthorizationEndpointTest.php | 4 +- .../ClearExpiredTokensCommandTest.php | 50 ++++- .../CustomPersistenceManagerTest.php | 25 +++ tests/Acceptance/DeviceCodeEndpointTest.php | 68 ++++++ .../DoctrineCredentialsRevokerTest.php | 26 +++ .../DoctrineDeviceCodeManagerTest.php | 83 ++++++++ tests/Acceptance/SecurityLayerTest.php | 4 +- tests/Acceptance/TokenEndpointTest.php | 110 +++++++++- tests/Fixtures/FakeDeviceCodeManager.php | 30 +++ tests/Fixtures/FixtureFactory.php | 78 +++++++ tests/Integration/AbstractIntegrationTest.php | 15 +- tests/Integration/AuthorizationServerTest.php | 3 +- .../Integration/DeviceCodeRepositoryTest.php | 46 +++++ tests/Integration/ResourceServerTest.php | 3 +- tests/TestKernel.php | 12 ++ tests/Unit/InMemoryDeviceCodeManagerTest.php | 71 +++++++ 37 files changed, 1565 insertions(+), 11 deletions(-) create mode 100644 docs/device-code-grant.md create mode 100644 src/Controller/DeviceCodeController.php create mode 100644 src/Entity/DeviceCode.php create mode 100644 src/Manager/DeviceCodeManagerInterface.php create mode 100644 src/Manager/Doctrine/DeviceCodeManager.php create mode 100644 src/Manager/InMemory/DeviceCodeManager.php create mode 100644 src/Model/DeviceCode.php create mode 100644 src/Model/DeviceCodeInterface.php create mode 100644 src/Repository/DeviceCodeRepository.php create mode 100644 tests/Acceptance/DeviceCodeEndpointTest.php create mode 100644 tests/Acceptance/DoctrineDeviceCodeManagerTest.php create mode 100644 tests/Fixtures/FakeDeviceCodeManager.php create mode 100644 tests/Integration/DeviceCodeRepositoryTest.php create mode 100644 tests/Unit/InMemoryDeviceCodeManagerTest.php diff --git a/config/routes.php b/config/routes.php index ddce95d1..5ecab2b6 100644 --- a/config/routes.php +++ b/config/routes.php @@ -9,6 +9,10 @@ ->add('oauth2_authorize', '/authorize') ->controller(['league.oauth2_server.controller.authorization', 'indexAction']) + ->add('oauth2_device_code', '/device-code') + ->controller(['league.oauth2_server.controller.device_code', 'indexAction']) + ->methods(['POST']) + ->add('oauth2_token', '/token') ->controller(['league.oauth2_server.controller.token', 'indexAction']) ->methods(['POST']) diff --git a/config/services.php b/config/services.php index be416c4f..c68a1d01 100644 --- a/config/services.php +++ b/config/services.php @@ -2,6 +2,11 @@ declare(strict_types=1); +use League\Bundle\OAuth2ServerBundle\Controller\DeviceCodeController; +use League\Bundle\OAuth2ServerBundle\Manager\DeviceCodeManagerInterface; +use League\Bundle\OAuth2ServerBundle\Repository\DeviceCodeRepository; +use League\OAuth2\Server\Grant\DeviceCodeGrant; +use League\OAuth2\Server\Repositories\DeviceCodeRepositoryInterface; use function Symfony\Component\DependencyInjection\Loader\Configurator\abstract_arg; use function Symfony\Component\DependencyInjection\Loader\Configurator\param; use function Symfony\Component\DependencyInjection\Loader\Configurator\service; @@ -79,6 +84,16 @@ ->alias(RefreshTokenRepositoryInterface::class, 'league.oauth2_server.repository.refresh_token') ->alias(RefreshTokenRepository::class, 'league.oauth2_server.repository.refresh_token') + ->set('league.oauth2_server.repository.device_code', DeviceCodeRepository::class) + ->args([ + service(DeviceCodeManagerInterface::class), + service(ClientManagerInterface::class), + service(ScopeConverterInterface::class), + service(ClientRepositoryInterface::class), + ]) + ->alias(DeviceCodeRepositoryInterface::class, 'league.oauth2_server.repository.device_code') + ->alias(DeviceCodeRepository::class, 'league.oauth2_server.repository.device_code') + ->set('league.oauth2_server.repository.scope', ScopeRepository::class) ->args([ service(ScopeManagerInterface::class), @@ -195,6 +210,16 @@ ]) ->alias(AuthCodeGrant::class, 'league.oauth2_server.grant.auth_code') + ->set('league.oauth2_server.grant.device_code', DeviceCodeGrant::class) + ->args([ + service(DeviceCodeRepositoryInterface::class), + service(RefreshTokenRepositoryInterface::class), + null, + null, + null + ]) + ->alias(DeviceCodeGrant::class, 'league.oauth2_server.grant.device_code') + ->set('league.oauth2_server.grant.implicit', ImplicitGrant::class) ->args([ null, @@ -216,6 +241,20 @@ ->tag('controller.service_arguments') ->alias(AuthorizationController::class, 'league.oauth2_server.controller.authorization') + ->set('league.oauth2_server.controller.device_code', DeviceCodeController::class) + ->args([ + service(AuthorizationServer::class), + service(EventDispatcherInterface::class), + service(AuthorizationRequestResolveEventFactory::class), + service(UserConverterInterface::class), + service(ClientManagerInterface::class), + service('league.oauth2_server.factory.psr_http'), + service('league.oauth2_server.factory.http_foundation'), + service('league.oauth2_server.factory.psr17'), + ]) + ->tag('controller.service_arguments') + ->alias(DeviceCodeController::class, 'league.oauth2_server.controller.device_code') + // Token controller ->set('league.oauth2_server.controller.token', TokenController::class) ->args([ @@ -263,6 +302,7 @@ service(AccessTokenManagerInterface::class), service(RefreshTokenManagerInterface::class), service(AuthorizationCodeManagerInterface::class), + service(DeviceCodeManagerInterface::class), ]) ->tag('console.command', ['command' => 'league:oauth2-server:clear-expired-tokens']) ->alias(ClearExpiredTokensCommand::class, 'league.oauth2_server.command.clear_expired_tokens') diff --git a/config/storage/doctrine.php b/config/storage/doctrine.php index 61c25f9d..68dfb107 100644 --- a/config/storage/doctrine.php +++ b/config/storage/doctrine.php @@ -2,6 +2,8 @@ declare(strict_types=1); +use League\Bundle\OAuth2ServerBundle\Manager\DeviceCodeManagerInterface; +use League\Bundle\OAuth2ServerBundle\Manager\Doctrine\DeviceCodeManager; use function Symfony\Component\DependencyInjection\Loader\Configurator\service; use League\Bundle\OAuth2ServerBundle\Manager\AccessTokenManagerInterface; @@ -53,6 +55,13 @@ ->alias(RefreshTokenManagerInterface::class, 'league.oauth2_server.manager.doctrine.refresh_token') ->alias(RefreshTokenManager::class, 'league.oauth2_server.manager.doctrine.refresh_token') + ->set('league.oauth2_server.manager.doctrine.device_code', DeviceCodeManager::class) + ->args([ + null, + ]) + ->alias(DeviceCodeManagerInterface::class, 'league.oauth2_server.manager.doctrine.device_code') + ->alias(DeviceCodeManager::class, 'league.oauth2_server.manager.doctrine.device_code') + ->set('league.oauth2_server.manager.doctrine.authorization_code', AuthorizationCodeManager::class) ->args([ null, diff --git a/config/storage/in_memory.php b/config/storage/in_memory.php index e42d484b..5565c652 100644 --- a/config/storage/in_memory.php +++ b/config/storage/in_memory.php @@ -2,6 +2,8 @@ declare(strict_types=1); +use League\Bundle\OAuth2ServerBundle\Manager\DeviceCodeManagerInterface; +use League\Bundle\OAuth2ServerBundle\Manager\InMemory\DeviceCodeManager; use function Symfony\Component\DependencyInjection\Loader\Configurator\service; use League\Bundle\OAuth2ServerBundle\Manager\AccessTokenManagerInterface; @@ -39,6 +41,13 @@ ->alias(RefreshTokenManagerInterface::class, 'league.oauth2_server.manager.in_memory.refresh_token') ->alias(RefreshTokenManager::class, 'league.oauth2_server.manager.in_memory.refresh_token') + ->set('league.oauth2_server.manager.in_memory.device_code', DeviceCodeManager::class) + ->args([ + null, + ]) + ->alias(DeviceCodeManagerInterface::class, 'league.oauth2_server.manager.in_memory.device_code') + ->alias(DeviceCodeManager::class, 'league.oauth2_server.manager.in_memory.device_code') + ->set('league.oauth2_server.manager.in_memory.authorization_code', AuthorizationCodeManager::class) ->args([ null, diff --git a/docs/basic-setup.md b/docs/basic-setup.md index 63a4cb08..8f818dd2 100644 --- a/docs/basic-setup.md +++ b/docs/basic-setup.md @@ -97,6 +97,9 @@ security: api_token: pattern: ^/token$ security: false + api_device_code: + pattern: ^/device-code$ + security: false api: pattern: ^/api security: true @@ -104,7 +107,7 @@ security: oauth2: true ``` -* The `api_token` firewall will ensure that anyone can access the `/api/token` endpoint in order to be able to retrieve their access tokens. +* The `api_token` and `api_device_code` firewall will ensure that anyone can access the `/token` and `/device-code` endpoint respectively in order to be able to retrieve their access tokens or device codes. * The `api` firewall will protect all routes prefixed with `/api` and clients will require a valid access token in order to access them. Basically, any firewall which sets the `oauth2` parameter to `true` will make any routes that match the selected pattern go through our OAuth 2.0 security layer. diff --git a/docs/device-code-grant.md b/docs/device-code-grant.md new file mode 100644 index 00000000..1f474be2 --- /dev/null +++ b/docs/device-code-grant.md @@ -0,0 +1,76 @@ +# Password grant handling + +The device code grant type is designed for devices without a browser or with limited input capabilities. In this flow, the user authenticates on another device—like a smartphone or computer—and receives a code to enter on the original device. + +Initially, the device sends a request to /device-code with its client ID and scope. The server then returns a device code, a user code, and a verification URL. The user takes the code to a secondary device, opens the verification URL in a browser, and enters the user code. + +Meanwhile, the original device continuously polls the /token endpoint with the device code. Once the user approves the request on the secondary device, the token endpoint returns the access token to the polling device. + +## Requirements + +You need to implement the verification URL yourself and handle the user code input : this bundle does not provide a route or UI for this. + +## Example + +### Controller + +This is a sample Symfony 7 controller to handle the user code input + +```php +createFormBuilder() + ->add('userCode', TextType::class, [ + 'required' => true, + ]) + ->getForm() + ->handleRequest($request); + + if ($form->isSubmitted() && $form->isValid()) { + try { + $this->deviceCodeRepository->approveDeviceCode($form->get('userCode')->getData(), $this->getUser()->getId()); + // Device code approved, show success message to user + } catch (OAuthServerException $e) { + // Handle exception (invalid code or missing user ID) + } + } + + return $this->render( + 'verify_device.html.twig', + ['form' => $form] + ); + } + +} +``` + +### Configuration + +```yaml +league_oauth2_server: + authorization_server: + device_code_verification_uri: 'https://your-domain.com/verify-device' +``` diff --git a/docs/index.md b/docs/index.md index 366776f7..8938348d 100644 --- a/docs/index.md +++ b/docs/index.md @@ -6,7 +6,7 @@ For implementation into Symfony projects, please see [bundle documentation](basi ## Features -* API endpoint for client authorization and token issuing +* API endpoint for client authorization, device code and token issuing * Configurable client and token persistance (includes [Doctrine](https://www.doctrine-project.org/) support) * Integration with Symfony's [Security](https://symfony.com/doc/current/security.html) layer @@ -78,6 +78,15 @@ For implementation into Symfony projects, please see [bundle documentation](basi # Whether to revoke refresh tokens after they were used for all grant types (default to true) revoke_refresh_tokens: true + # Whether to enable the device code grant + enable_device_code_grant: true + + # The full URI the user will need to visit to enter the user code + device_code_verification_uri: '' + + # How soon (in seconds) can the device code be used to poll for the access token without being throttled + device_code_polling_interval: 5 + resource_server: # Required # Full path to the public key file diff --git a/src/Command/ClearExpiredTokensCommand.php b/src/Command/ClearExpiredTokensCommand.php index 3946a9db..ee161c0a 100644 --- a/src/Command/ClearExpiredTokensCommand.php +++ b/src/Command/ClearExpiredTokensCommand.php @@ -6,6 +6,7 @@ use League\Bundle\OAuth2ServerBundle\Manager\AccessTokenManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\AuthorizationCodeManagerInterface; +use League\Bundle\OAuth2ServerBundle\Manager\DeviceCodeManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\RefreshTokenManagerInterface; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; @@ -32,16 +33,23 @@ final class ClearExpiredTokensCommand extends Command */ private $authorizationCodeManager; + /** + * @var DeviceCodeManagerInterface + */ + private $deviceCodeManager; + public function __construct( AccessTokenManagerInterface $accessTokenManager, RefreshTokenManagerInterface $refreshTokenManager, AuthorizationCodeManagerInterface $authorizationCodeManager, + DeviceCodeManagerInterface $deviceCodeManager, ) { parent::__construct(); $this->accessTokenManager = $accessTokenManager; $this->refreshTokenManager = $refreshTokenManager; $this->authorizationCodeManager = $authorizationCodeManager; + $this->deviceCodeManager = $deviceCodeManager; } protected function configure(): void @@ -66,6 +74,12 @@ protected function configure(): void InputOption::VALUE_NONE, 'Clear expired auth codes.' ) + ->addOption( + 'device-codes', + 'dc', + InputOption::VALUE_NONE, + 'Clear expired device codes.' + ) ; } @@ -76,11 +90,13 @@ protected function execute(InputInterface $input, OutputInterface $output): int $clearExpiredAccessTokens = $input->getOption('access-tokens'); $clearExpiredRefreshTokens = $input->getOption('refresh-tokens'); $clearExpiredAuthCodes = $input->getOption('auth-codes'); + $clearExpiredDeviceCodes = $input->getOption('device-codes'); - if (!$clearExpiredAccessTokens && !$clearExpiredRefreshTokens && !$clearExpiredAuthCodes) { + if (!$clearExpiredAccessTokens && !$clearExpiredRefreshTokens && !$clearExpiredAuthCodes && !$clearExpiredDeviceCodes) { $this->clearExpiredAccessTokens($io); $this->clearExpiredRefreshTokens($io); $this->clearExpiredAuthCodes($io); + $this->clearExpiredDeviceCodes($io); return 0; } @@ -97,6 +113,10 @@ protected function execute(InputInterface $input, OutputInterface $output): int $this->clearExpiredAuthCodes($io); } + if ($clearExpiredDeviceCodes) { + $this->clearExpiredDeviceCodes($io); + } + return 0; } @@ -129,4 +149,14 @@ private function clearExpiredAuthCodes(SymfonyStyle $io): void 1 === $numOfClearedAuthCodes ? '' : 's' )); } + + private function clearExpiredDeviceCodes(SymfonyStyle $io): void + { + $numberOfClearedDeviceCodes = $this->deviceCodeManager->clearExpired(); + $io->success(\sprintf( + 'Cleared %d expired device code%s.', + $numberOfClearedDeviceCodes, + 1 === $numberOfClearedDeviceCodes ? '' : 's' + )); + } } diff --git a/src/Controller/DeviceCodeController.php b/src/Controller/DeviceCodeController.php new file mode 100644 index 00000000..7b3e57c1 --- /dev/null +++ b/src/Controller/DeviceCodeController.php @@ -0,0 +1,96 @@ +server = $server; + $this->eventDispatcher = $eventDispatcher; + $this->eventFactory = $eventFactory; + $this->userConverter = $userConverter; + $this->clientManager = $clientManager; + $this->httpMessageFactory = $httpMessageFactory; + $this->httpFoundationFactory = $httpFoundationFactory; + $this->responseFactory = $responseFactory; + } + + public function indexAction(Request $request): Response + { + $serverRequest = $this->httpMessageFactory->createRequest($request); + $serverResponse = $this->responseFactory->createResponse(); + + try { + $response = $this->server->respondToDeviceAuthorizationRequest($serverRequest, $serverResponse); + } catch (OAuthServerException $e) { + $response = $e->generateHttpResponse($serverResponse); + } + + return $this->httpFoundationFactory->createResponse($response); + } +} diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index bbf3b03d..57d0997c 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -79,6 +79,11 @@ private function createAuthorizationServerNode(): NodeDefinition ->cannotBeEmpty() ->defaultValue('PT10M') ->end() + ->scalarNode('device_code_ttl') + ->info("How long the issued device code should be valid for.\nThe value should be a valid interval: http://php.net/manual/en/dateinterval.construct.php#refsect1-dateinterval.construct-parameters") + ->cannotBeEmpty() + ->defaultValue('PT10M') + ->end() ->booleanNode('enable_client_credentials_grant') ->info('Whether to enable the client credentials grant') ->defaultTrue() @@ -115,6 +120,18 @@ private function createAuthorizationServerNode(): NodeDefinition ->info('Whether to revoke refresh tokens after they were used for all grant types') ->defaultTrue() ->end() + ->booleanNode('enable_device_code_grant') + ->info('Whether to enable the device code grant') + ->defaultTrue() + ->end() + ->scalarNode('device_code_verification_uri') + ->info('The full URI the user will need to visit to enter the user code') + ->defaultValue('') + ->end() + ->scalarNode('device_code_polling_interval') + ->info('How soon (in seconds) can the device code be used to poll for the access token without being throttled') + ->defaultValue(5) + ->end() ->end() ; @@ -227,6 +244,11 @@ private function createPersistenceNode(): NodeDefinition ->cannotBeEmpty() ->isRequired() ->end() + ->scalarNode('device_code_manager') + ->info('Service id of the custom device code manager') + ->cannotBeEmpty() + ->isRequired() + ->end() ->scalarNode('credentials_revoker') ->info('Service id of the custom credentials revoker') ->cannotBeEmpty() diff --git a/src/DependencyInjection/LeagueOAuth2ServerExtension.php b/src/DependencyInjection/LeagueOAuth2ServerExtension.php index 03750dd8..3f6b7475 100644 --- a/src/DependencyInjection/LeagueOAuth2ServerExtension.php +++ b/src/DependencyInjection/LeagueOAuth2ServerExtension.php @@ -14,9 +14,11 @@ use League\Bundle\OAuth2ServerBundle\Manager\AccessTokenManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\AuthorizationCodeManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface; +use League\Bundle\OAuth2ServerBundle\Manager\DeviceCodeManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\Doctrine\AccessTokenManager; use League\Bundle\OAuth2ServerBundle\Manager\Doctrine\AuthorizationCodeManager; use League\Bundle\OAuth2ServerBundle\Manager\Doctrine\ClientManager; +use League\Bundle\OAuth2ServerBundle\Manager\Doctrine\DeviceCodeManager; use League\Bundle\OAuth2ServerBundle\Manager\Doctrine\RefreshTokenManager; use League\Bundle\OAuth2ServerBundle\Manager\InMemory\AccessTokenManager as InMemoryAccessTokenManager; use League\Bundle\OAuth2ServerBundle\Manager\RefreshTokenManagerInterface; @@ -31,6 +33,7 @@ use League\OAuth2\Server\CryptKey; use League\OAuth2\Server\Grant\AuthCodeGrant; use League\OAuth2\Server\Grant\ClientCredentialsGrant; +use League\OAuth2\Server\Grant\DeviceCodeGrant; use League\OAuth2\Server\Grant\ImplicitGrant; use League\OAuth2\Server\Grant\PasswordGrant; use League\OAuth2\Server\Grant\RefreshTokenGrant; @@ -205,6 +208,13 @@ private function configureAuthorizationServer(ContainerBuilder $container, array ]); } + if ($config['enable_device_code_grant']) { + $authorizationServer->addMethodCall('enableGrantType', [ + new Reference(DeviceCodeGrant::class), + new Definition(\DateInterval::class, [$config['access_token_ttl']]), + ]); + } + if ($config['enable_implicit_grant']) { $authorizationServer->addMethodCall('enableGrantType', [ new Reference(ImplicitGrant::class), @@ -241,6 +251,16 @@ private function configureGrants(ContainerBuilder $container, array $config): vo ]) ; + $deviceCodeGrantDefinition = $container->findDefinition(DeviceCodeGrant::class); + $deviceCodeGrantDefinition + ->replaceArgument(2, new Definition(\DateInterval::class, [$config['device_code_ttl']])) + ->replaceArgument(3, $config['device_code_verification_uri']) + ->replaceArgument(4, $config['device_code_polling_interval']) + ->addMethodCall('setRefreshTokenTTL', [ + new Definition(\DateInterval::class, [$config['refresh_token_ttl']]), + ]) + ; + if (false === $config['require_code_challenge_for_public_clients']) { $authCodeGrantDefinition->addMethodCall('disableRequireCodeChallengeForPublicClients'); } @@ -336,6 +356,11 @@ private function configureDoctrinePersistence(ContainerBuilder $container, array ->replaceArgument(0, $entityManager) ; + $container + ->findDefinition(DeviceCodeManager::class) + ->replaceArgument(0, $entityManager) + ; + $container ->findDefinition(DoctrineCredentialsRevoker::class) ->replaceArgument(0, $entityManager) @@ -372,6 +397,7 @@ private function configureCustomPersistence(ContainerBuilder $container, array $ $container->setAlias(ClientManagerInterface::class, $persistenceConfig['client_manager']); $container->setAlias(AccessTokenManagerInterface::class, $persistenceConfig['access_token_manager']); $container->setAlias(RefreshTokenManagerInterface::class, $persistenceConfig['refresh_token_manager']); + $container->setAlias(DeviceCodeManagerInterface::class, $persistenceConfig['device_code_manager']); $container->setAlias(AuthorizationCodeManagerInterface::class, $persistenceConfig['authorization_code_manager']); $container->setAlias(CredentialsRevokerInterface::class, $persistenceConfig['credentials_revoker']); diff --git a/src/Entity/DeviceCode.php b/src/Entity/DeviceCode.php new file mode 100644 index 00000000..4dd474ed --- /dev/null +++ b/src/Entity/DeviceCode.php @@ -0,0 +1,15 @@ +entityManager = $entityManager; + } + + public function find(string $identifier): ?DeviceCodeInterface + { + return $this->entityManager->find(DeviceCode::class, $identifier); + } + + public function findByUserCode(string $code): ?DeviceCodeInterface + { + /** @var DeviceCodeInterface */ + return $this->entityManager->createQueryBuilder() + ->select('dc') + ->from(DeviceCode::class, 'dc') + ->where('dc.userCode = :code') + ->setParameter('code', $code) + ->getQuery() + ->getOneOrNullResult(); + } + + public function save(DeviceCodeInterface $deviceCode, bool $persist = true): void + { + if ($persist) { + $this->entityManager->persist($deviceCode); + } + $this->entityManager->flush(); + } + + public function clearExpired(): int + { + /** @var int */ + return $this->entityManager->createQueryBuilder() + ->delete(DeviceCode::class, 'at') + ->where('at.expiry < :expiry') + ->setParameter('expiry', new \DateTimeImmutable(), 'datetime_immutable') + ->getQuery() + ->execute(); + } + +} diff --git a/src/Manager/InMemory/DeviceCodeManager.php b/src/Manager/InMemory/DeviceCodeManager.php new file mode 100644 index 00000000..e2f47f5d --- /dev/null +++ b/src/Manager/InMemory/DeviceCodeManager.php @@ -0,0 +1,50 @@ + + */ + private $deviceCodes = []; + + public function find(string $identifier): ?DeviceCodeInterface + { + return $this->deviceCodes[$identifier] ?? null; + } + + public function findByUserCode(string $code): ?DeviceCodeInterface + { + foreach ($this->deviceCodes as $deviceCode) { + if ($deviceCode->getUserCode() === $code) { + return $deviceCode; + } + } + + return null; + } + + public function save(DeviceCodeInterface $deviceCode, bool $persist = true): void + { + $this->deviceCodes[$deviceCode->getIdentifier()] = $deviceCode; + } + + public function clearExpired(): int + { + $count = \count($this->deviceCodes); + + $now = new \DateTimeImmutable(); + $this->deviceCodes = array_filter($this->deviceCodes, static function (DeviceCodeInterface $accessToken) use ($now): bool { + return $accessToken->getExpiry() >= $now; + }); + + return $count - \count($this->deviceCodes); + } + +} diff --git a/src/Model/DeviceCode.php b/src/Model/DeviceCode.php new file mode 100644 index 00000000..d16d3b49 --- /dev/null +++ b/src/Model/DeviceCode.php @@ -0,0 +1,193 @@ + + */ + private $scopes; + + /** + * @var bool + */ + private $revoked = false; + + /** + * @var string + */ + private $userCode; + + /** + * @var bool + */ + private $userApproved; + + /** + * @var bool + */ + private $includeVerificationUriComplete; + + /** + * @var string + */ + private $verificationUri; + + /** + * @var \DateTimeInterface|null + */ + private $lastPolledAt; + + /** + * @var int + */ + private $interval; + + /** + * @param list $scopes + */ + public function __construct( + string $identifier, + \DateTimeImmutable $expiry, + ClientInterface $client, + ?string $userIdentifier, + array $scopes, + string $userCode, + bool $userApproved, + bool $includeVerificationUriComplete, + string $verificationUri, + ?\DateTimeImmutable $lastPolledAt, + int $interval + ) { + $this->identifier = $identifier; + $this->expiry = $expiry; + $this->client = $client; + $this->userIdentifier = $userIdentifier; + $this->scopes = $scopes; + $this->userCode = $userCode; + $this->userApproved = $userApproved; + $this->includeVerificationUriComplete = $includeVerificationUriComplete; + $this->verificationUri = $verificationUri; + $this->lastPolledAt = $lastPolledAt; + $this->interval = $interval; + } + + public function __toString(): string + { + return $this->getIdentifier(); + } + + public function getIdentifier(): string + { + return $this->identifier; + } + + public function getExpiry(): \DateTimeImmutable + { + return $this->expiry; + } + + public function getUserIdentifier(): ?string + { + return $this->userIdentifier; + } + + public function setUserIdentifier($userIdentifier): DeviceCodeInterface + { + $this->userIdentifier = $userIdentifier; + + return $this; + } + + public function getClient(): ClientInterface + { + return $this->client; + } + + public function getScopes(): array + { + return $this->scopes; + } + + public function isRevoked(): bool + { + return $this->revoked; + } + + public function revoke(): DeviceCodeInterface + { + $this->revoked = true; + + return $this; + } + + public function getUserCode(): string + { + return $this->userCode; + } + + public function getUserApproved(): bool + { + return $this->userApproved; + } + + public function setUserApproved(bool $userApproved): DeviceCodeInterface + { + $this->userApproved = $userApproved; + + return $this; + } + + public function getIncludeVerificationUriComplete(): bool + { + return $this->includeVerificationUriComplete; + } + + public function getVerificationUri(): string + { + return $this->verificationUri; + } + + public function getLastPolledAt(): ?\DateTimeImmutable + { + return $this->lastPolledAt; + } + + public function setLastPolledAt(\DateTimeImmutable $lastPolledAt): DeviceCodeInterface + { + $this->lastPolledAt = $lastPolledAt; + + return $this; + } + + public function getInterval(): int + { + return $this->interval; + } + +} diff --git a/src/Model/DeviceCodeInterface.php b/src/Model/DeviceCodeInterface.php new file mode 100644 index 00000000..d87ed47f --- /dev/null +++ b/src/Model/DeviceCodeInterface.php @@ -0,0 +1,48 @@ + + */ + public function getScopes(): array; + + public function isRevoked(): bool; + + public function revoke(): self; + + public function getUserCode(): string; + + public function getUserApproved(): bool; + + public function setUserApproved(bool $userApproved): self; + + public function getIncludeVerificationUriComplete(): bool; + + public function getVerificationUri(): string; + + public function getLastPolledAt(): ?\DateTimeImmutable; + + public function setLastPolledAt(\DateTimeImmutable $lastPolledAt): self; + + public function getInterval(): int; + +} diff --git a/src/Persistence/Mapping/Driver.php b/src/Persistence/Mapping/Driver.php index e9c85ae5..bd740afb 100644 --- a/src/Persistence/Mapping/Driver.php +++ b/src/Persistence/Mapping/Driver.php @@ -12,6 +12,7 @@ use League\Bundle\OAuth2ServerBundle\Model\AccessToken; use League\Bundle\OAuth2ServerBundle\Model\AuthorizationCode; use League\Bundle\OAuth2ServerBundle\Model\Client; +use League\Bundle\OAuth2ServerBundle\Model\DeviceCode; use League\Bundle\OAuth2ServerBundle\Model\RefreshToken; /** @@ -53,6 +54,10 @@ public function loadMetadataForClass($className, ClassMetadata $metadata): void case AccessToken::class: $this->buildAccessTokenMetadata($metadata); + break; + case DeviceCode::class: + $this->buildDeviceCodeMetadata($metadata); + break; case AuthorizationCode::class: $this->buildAuthorizationCodeMetadata($metadata); @@ -76,6 +81,7 @@ public function getAllClassNames(): array return array_merge( [ AbstractClient::class, + DeviceCode::class, AuthorizationCode::class, RefreshToken::class, ], @@ -122,6 +128,28 @@ private function buildAccessTokenMetadata(ORMClassMetadata $metadata): void ; } + /** + * @param ORMClassMetadata $metadata + */ + private function buildDeviceCodeMetadata(ORMClassMetadata $metadata): void + { + (new ClassMetadataBuilder($metadata)) + ->setTable($this->tablePrefix . 'device_code') + ->createField('identifier', 'string')->makePrimaryKey()->length(80)->option('fixed', true)->build() + ->addField('expiry', 'datetime_immutable') + ->createField('userIdentifier', 'string')->length(128)->nullable(true)->build() + ->createField('scopes', 'oauth2_scope')->nullable(true)->build() + ->addField('revoked', 'boolean') + ->createField('userCode', 'string')->length(255)->nullable(true)->build() + ->addField('userApproved', 'boolean') + ->addField('includeVerificationUriComplete', 'boolean') + ->createField('verificationUri', 'string')->length(255)->nullable(true)->build() + ->createField('lastPolledAt', 'datetime_immutable')->nullable(true)->build() + ->addField('interval', 'integer') + ->createManyToOne('client', $this->clientClass)->addJoinColumn('client', 'identifier', false, false, 'CASCADE')->build() + ; + } + /** * @param ORMClassMetadata $metadata */ diff --git a/src/Repository/ClientRepository.php b/src/Repository/ClientRepository.php index af84b730..da99f806 100644 --- a/src/Repository/ClientRepository.php +++ b/src/Repository/ClientRepository.php @@ -56,7 +56,7 @@ public function validateClient(string $clientIdentifier, ?string $clientSecret, return false; } - private function buildClientEntity(ClientInterface $client): ClientEntity + public function buildClientEntity(ClientInterface $client): ClientEntity { $clientEntity = new ClientEntity(); $clientEntity->setName($client->getName()); diff --git a/src/Repository/DeviceCodeRepository.php b/src/Repository/DeviceCodeRepository.php new file mode 100644 index 00000000..527b5e25 --- /dev/null +++ b/src/Repository/DeviceCodeRepository.php @@ -0,0 +1,182 @@ +deviceCodeManager = $deviceCodeManager; + $this->clientManager = $clientManager; + $this->scopeConverter = $scopeConverter; + $this->clientRepository = $clientRepository; + } + + public function getNewDeviceCode(): DeviceCodeEntityInterface + { + return new DeviceCodeEntity(); + } + + public function persistDeviceCode(DeviceCodeEntityInterface $deviceCodeEntity): void + { + $deviceCode = $this->deviceCodeManager->find($deviceCodeEntity->getIdentifier()); + $newDeviceCode = false; + + if ($deviceCode) { + if ($deviceCodeEntity->getLastPolledAt()) { + $deviceCode->setLastPolledAt($deviceCodeEntity->getLastPolledAt()); + } + } else { + $newDeviceCode = true; + $deviceCode = $this->buildDeviceCodeModel($deviceCodeEntity); + } + + $this->deviceCodeManager->save($deviceCode, $newDeviceCode); + } + + public function approveDeviceCode(string $userCode, string $userId): void + { + $deviceCode = $this->deviceCodeManager->findByUserCode($userCode); + + if ($deviceCode instanceof DeviceCodeInterface === false) { + throw OAuthServerException::invalidRequest('device_code', 'Device code does not exist'); + } + + if ($deviceCode->isRevoked()) { + throw OAuthServerException::invalidRequest('device_code', 'Device code has been revoked'); + } + + if ($userId === '') { + throw OAuthServerException::invalidRequest('user_id', 'User ID is required'); + } + + $deviceCode->setUserIdentifier($userId); + $deviceCode->setUserApproved(true); + + $this->deviceCodeManager->save($deviceCode, false); + } + + public function getDeviceCodeEntityByDeviceCode(string $deviceCodeEntity): ?DeviceCodeEntityInterface + { + $deviceCode = $this->deviceCodeManager->find($deviceCodeEntity); + + if (null === $deviceCode) { + return null; + } + + return $this->buildDeviceCodeEntity($deviceCode); + } + + public function revokeDeviceCode(string $codeId): void + { + $deviceCode = $this->deviceCodeManager->find($codeId); + + if (null === $deviceCode) { + return; + } + + $deviceCode->revoke(); + + $this->deviceCodeManager->save($deviceCode, false); + } + + public function isDeviceCodeRevoked(string $codeId): bool + { + $deviceCode = $this->deviceCodeManager->find($codeId); + + if (null === $deviceCode) { + return true; + } + + return $deviceCode->isRevoked(); + } + + private function buildDeviceCodeEntity(DeviceCodeInterface $deviceCode): DeviceCodeEntity + { + $deviceCodeEntity = new DeviceCodeEntity(); + $deviceCodeEntity->setIdentifier($deviceCode->getIdentifier()); + $deviceCodeEntity->setExpiryDateTime($deviceCode->getExpiry()); + $deviceCodeEntity->setClient( + $this->clientRepository->buildClientEntity($deviceCode->getClient()) + ); + if ($deviceCode->getUserIdentifier()) { + $deviceCodeEntity->setUserIdentifier($deviceCode->getUserIdentifier()); + } + $deviceCodeEntity->setUserCode($deviceCode->getUserCode()); + $deviceCodeEntity->setUserApproved($deviceCode->getUserApproved()); + $deviceCodeEntity->setVerificationUriCompleteInAuthResponse($deviceCode->getIncludeVerificationUriComplete()); + $deviceCodeEntity->setVerificationUri($deviceCode->getVerificationUri()); + if ($deviceCode->getLastPolledAt()) { + $deviceCodeEntity->setLastPolledAt($deviceCode->getLastPolledAt()); + } + $deviceCodeEntity->setInterval($deviceCode->getInterval()); + + foreach ($deviceCode->getScopes() as $scope) { + $deviceCodeEntity->addScope($this->scopeConverter->toLeague($scope)); + } + + return $deviceCodeEntity; + } + + + private function buildDeviceCodeModel(DeviceCodeEntityInterface $deviceCodeEntity): DeviceCodeModel + { + /** @var AbstractClient $client */ + $client = $this->clientManager->find($deviceCodeEntity->getClient()->getIdentifier()); + + $userIdentifier = $deviceCodeEntity->getUserIdentifier(); + + return new DeviceCodeModel( + $deviceCodeEntity->getIdentifier(), + $deviceCodeEntity->getExpiryDateTime(), + $client, + $userIdentifier, + $this->scopeConverter->toDomainArray(array_values($deviceCodeEntity->getScopes())), + $deviceCodeEntity->getUserCode(), + $deviceCodeEntity->getUserApproved(), + $deviceCodeEntity->getVerificationUriCompleteInAuthResponse(), + $deviceCodeEntity->getVerificationUri(), + $deviceCodeEntity->getLastPolledAt(), + $deviceCodeEntity->getInterval() + ); + } + +} diff --git a/src/Service/CredentialsRevoker/DoctrineCredentialsRevoker.php b/src/Service/CredentialsRevoker/DoctrineCredentialsRevoker.php index 6679a84a..bcb6a222 100644 --- a/src/Service/CredentialsRevoker/DoctrineCredentialsRevoker.php +++ b/src/Service/CredentialsRevoker/DoctrineCredentialsRevoker.php @@ -9,6 +9,7 @@ use League\Bundle\OAuth2ServerBundle\Model\AbstractClient; use League\Bundle\OAuth2ServerBundle\Model\AccessToken; use League\Bundle\OAuth2ServerBundle\Model\AuthorizationCode; +use League\Bundle\OAuth2ServerBundle\Model\DeviceCode; use League\Bundle\OAuth2ServerBundle\Model\RefreshToken; use League\Bundle\OAuth2ServerBundle\Service\CredentialsRevokerInterface; use Symfony\Component\Security\Core\User\UserInterface; @@ -69,6 +70,15 @@ public function revokeCredentialsForUser(UserInterface $user): void ->setParameter('userIdentifier', $userIdentifier) ->getQuery() ->execute(); + + $this->entityManager->createQueryBuilder() + ->update(DeviceCode::class, 'dc') + ->set('dc.revoked', ':revoked') + ->where('dc.userIdentifier = :userIdentifier') + ->setParameter('revoked', true) + ->setParameter('userIdentifier', $userIdentifier) + ->getQuery() + ->execute(); } public function revokeCredentialsForClient(AbstractClient $client): void @@ -109,5 +119,14 @@ public function revokeCredentialsForClient(AbstractClient $client): void ->setParameter('revoked', true) ->getQuery() ->execute(); + + $this->entityManager->createQueryBuilder() + ->update(DeviceCode::class, 'dc') + ->set('dc.revoked', ':revoked') + ->where('dc.client = :client') + ->setParameter('client', $doctrineClient->getIdentifier(), 'string') + ->setParameter('revoked', true) + ->getQuery() + ->execute(); } } diff --git a/tests/Acceptance/AuthorizationEndpointTest.php b/tests/Acceptance/AuthorizationEndpointTest.php index 7b82ad4c..a5a8f161 100644 --- a/tests/Acceptance/AuthorizationEndpointTest.php +++ b/tests/Acceptance/AuthorizationEndpointTest.php @@ -8,6 +8,7 @@ use League\Bundle\OAuth2ServerBundle\Manager\AccessTokenManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\AuthorizationCodeManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface; +use League\Bundle\OAuth2ServerBundle\Manager\DeviceCodeManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\RefreshTokenManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\ScopeManagerInterface; use League\Bundle\OAuth2ServerBundle\Model\AuthorizationCode; @@ -27,7 +28,8 @@ protected function setUp(): void $this->client->getContainer()->get(ClientManagerInterface::class), $this->client->getContainer()->get(AccessTokenManagerInterface::class), $this->client->getContainer()->get(RefreshTokenManagerInterface::class), - $this->client->getContainer()->get(AuthorizationCodeManagerInterface::class) + $this->client->getContainer()->get(AuthorizationCodeManagerInterface::class), + $this->client->getContainer()->get(DeviceCodeManagerInterface::class) ); } diff --git a/tests/Acceptance/ClearExpiredTokensCommandTest.php b/tests/Acceptance/ClearExpiredTokensCommandTest.php index 48e4d9fa..046e70c7 100644 --- a/tests/Acceptance/ClearExpiredTokensCommandTest.php +++ b/tests/Acceptance/ClearExpiredTokensCommandTest.php @@ -8,6 +8,7 @@ use League\Bundle\OAuth2ServerBundle\Manager\AccessTokenManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\AuthorizationCodeManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface; +use League\Bundle\OAuth2ServerBundle\Manager\DeviceCodeManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\RefreshTokenManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\ScopeManagerInterface; use League\Bundle\OAuth2ServerBundle\Model\AccessToken; @@ -28,7 +29,8 @@ protected function setUp(): void $this->client->getContainer()->get(ClientManagerInterface::class), $this->client->getContainer()->get(AccessTokenManagerInterface::class), $this->client->getContainer()->get(RefreshTokenManagerInterface::class), - $this->client->getContainer()->get(AuthorizationCodeManagerInterface::class) + $this->client->getContainer()->get(AuthorizationCodeManagerInterface::class), + $this->client->getContainer()->get(DeviceCodeManagerInterface::class) ); } @@ -52,6 +54,7 @@ public function testClearExpiredAccessAndRefreshTokensAndAuthCodes(): void $this->assertStringContainsString('Cleared 1 expired access token.', $output); $this->assertStringContainsString('Cleared 1 expired refresh token.', $output); $this->assertStringContainsString('Cleared 1 expired auth code.', $output); + $this->assertStringContainsString('Cleared 1 expired device code.', $output); /** @var EntityManagerInterface $em */ $em = $this->client->getContainer()->get('doctrine.orm.entity_manager'); @@ -72,6 +75,11 @@ public function testClearExpiredAccessAndRefreshTokensAndAuthCodes(): void FixtureFactory::FIXTURE_AUTH_CODE_EXPIRED ) ); + $this->assertNull( + $this->client->getContainer()->get(DeviceCodeManagerInterface::class)->find( + FixtureFactory::FIXTURE_DEVICE_CODE_EXPIRED + ) + ); } public function testClearExpiredAccessTokens(): void @@ -194,6 +202,46 @@ public function testClearExpiredAuthCodes(): void ); } + public function testClearExpiredDeviceCodes(): void + { + $command = $this->command(); + $commandTester = new CommandTester($command); + + $exitCode = $commandTester->execute([ + 'command' => $command->getName(), + '--device-codes' => true, + ]); + + $this->assertSame(0, $exitCode); + + $output = $commandTester->getDisplay(); + $this->assertStringNotContainsString('Cleared 1 expired access token.', $output); + $this->assertStringNotContainsString('Cleared 1 expired refresh token.', $output); + $this->assertStringContainsString('Cleared 1 expired device code.', $output); + + /** @var EntityManagerInterface $em */ + $em = $this->client->getContainer()->get('doctrine.orm.entity_manager'); + $em->clear(); + + $this->assertInstanceOf( + AccessToken::class, + $this->client->getContainer()->get(AccessTokenManagerInterface::class)->find( + FixtureFactory::FIXTURE_ACCESS_TOKEN_EXPIRED + ) + ); + $this->assertInstanceOf( + RefreshToken::class, + $this->client->getContainer()->get(RefreshTokenManagerInterface::class)->find( + FixtureFactory::FIXTURE_REFRESH_TOKEN_EXPIRED + ) + ); + $this->assertNull( + $this->client->getContainer()->get(DeviceCodeManagerInterface::class)->find( + FixtureFactory::FIXTURE_DEVICE_CODE_EXPIRED + ) + ); + } + private function command(): Command { return $this->application->find('league:oauth2-server:clear-expired-tokens'); diff --git a/tests/Acceptance/CustomPersistenceManagerTest.php b/tests/Acceptance/CustomPersistenceManagerTest.php index 4ceb71bb..aa2da8a8 100644 --- a/tests/Acceptance/CustomPersistenceManagerTest.php +++ b/tests/Acceptance/CustomPersistenceManagerTest.php @@ -8,6 +8,7 @@ use League\Bundle\OAuth2ServerBundle\Manager\AccessTokenManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\AuthorizationCodeManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface; +use League\Bundle\OAuth2ServerBundle\Manager\DeviceCodeManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\RefreshTokenManagerInterface; use League\Bundle\OAuth2ServerBundle\Model\AccessToken; use League\Bundle\OAuth2ServerBundle\Model\AuthorizationCode; @@ -19,6 +20,7 @@ use League\Bundle\OAuth2ServerBundle\Tests\Fixtures\FakeAuthorizationCodeManager; use League\Bundle\OAuth2ServerBundle\Tests\Fixtures\FakeClientManager; use League\Bundle\OAuth2ServerBundle\Tests\Fixtures\FakeCredentialsRevoker; +use League\Bundle\OAuth2ServerBundle\Tests\Fixtures\FakeDeviceCodeManager; use League\Bundle\OAuth2ServerBundle\Tests\Fixtures\FakeRefreshTokenManager; use League\Bundle\OAuth2ServerBundle\Tests\Fixtures\FixtureFactory; use League\Bundle\OAuth2ServerBundle\Tests\TestHelper; @@ -34,6 +36,7 @@ class CustomPersistenceManagerTest extends AbstractAcceptanceTest private ClientManagerInterface&MockObject $clientManager; private RefreshTokenManagerInterface&MockObject $refreshTokenManager; private AuthorizationCodeManagerInterface&MockObject $authCodeManager; + private DeviceCodeManagerInterface&MockObject $deviceCodeManager; protected function setUp(): void { @@ -42,6 +45,7 @@ protected function setUp(): void $this->clientManager = $this->createMock(ClientManagerInterface::class); $this->refreshTokenManager = $this->createMock(RefreshTokenManagerInterface::class); $this->authCodeManager = $this->createMock(AuthorizationCodeManagerInterface::class); + $this->deviceCodeManager = $this->createMock(DeviceCodeManagerInterface::class); $this->application = new Application($this->client->getKernel()); } @@ -52,6 +56,7 @@ public function testRegisteredServices(): void static::assertInstanceOf(FakeClientManager::class, $this->client->getContainer()->get(ClientManagerInterface::class)); static::assertInstanceOf(FakeRefreshTokenManager::class, $this->client->getContainer()->get(RefreshTokenManagerInterface::class)); static::assertInstanceOf(FakeCredentialsRevoker::class, $this->client->getContainer()->get(CredentialsRevokerInterface::class)); + static::assertInstanceOf(FakeDeviceCodeManager::class, $this->client->getContainer()->get(DeviceCodeManagerInterface::class)); } public function testSuccessfulClientCredentialsRequest(): void @@ -154,6 +159,25 @@ public function testSuccessfulAuthorizationCodeRequest(): void static::assertResponseIsSuccessful(); } + public function testSuccessfullDeviceCodeRequest(): void + { + $client = new Client('name', 'foo', 'secret'); + + $this->deviceCodeManager->expects(self::atLeastOnce())->method('find')->willReturn(null); + $this->deviceCodeManager->expects(self::atLeastOnce())->method('save'); + $this->client->getContainer()->set('test.device_code_manager', $this->deviceCodeManager); + + $this->clientManager->expects(self::atLeastOnce())->method('find')->with('foo')->willReturn($client); + $this->client->getContainer()->set('test.client_manager', $this->clientManager); + + $this->client->request('POST', '/device-code', [ + 'client_id' => $client->getIdentifier(), + ]); + + $this->client->getResponse(); + static::assertResponseIsSuccessful(); + } + protected static function createKernel(array $options = []): KernelInterface { return new TestKernel( @@ -167,6 +191,7 @@ protected static function createKernel(array $options = []): KernelInterface 'client_manager' => 'test.client_manager', 'refresh_token_manager' => 'test.refresh_token_manager', 'credentials_revoker' => 'test.credentials_revoker', + 'device_code_manager' => 'test.device_code_manager', ], ] ); diff --git a/tests/Acceptance/DeviceCodeEndpointTest.php b/tests/Acceptance/DeviceCodeEndpointTest.php new file mode 100644 index 00000000..d83e3294 --- /dev/null +++ b/tests/Acceptance/DeviceCodeEndpointTest.php @@ -0,0 +1,68 @@ +client->getContainer()->get(ScopeManagerInterface::class), + $this->client->getContainer()->get(ClientManagerInterface::class), + $this->client->getContainer()->get(AccessTokenManagerInterface::class), + $this->client->getContainer()->get(RefreshTokenManagerInterface::class), + $this->client->getContainer()->get(AuthorizationCodeManagerInterface::class), + $this->client->getContainer()->get(DeviceCodeManagerInterface::class) + ); + } + + public function testSuccessfulCodeRequest(): void + { + $this->client->request('POST', '/device-code', [ + 'client_id' => FixtureFactory::FIXTURE_PUBLIC_CLIENT, + ]); + + $response = $this->client->getResponse(); + + $this->assertSame(200, $response->getStatusCode()); + $this->assertSame('application/json; charset=UTF-8', $response->headers->get('Content-Type')); + + $jsonResponse = json_decode($response->getContent(), true); + + $this->assertNotEmpty($jsonResponse['device_code']); + $this->assertNotEmpty($jsonResponse['user_code']); + $this->assertSame('', $jsonResponse['verification_uri']); + $this->assertLessThanOrEqual(3600, $jsonResponse['expires_in']); + } + + public function testFailedWithUnkownClientRequest(): void + { + $this->client->request('POST', '/device-code', [ + 'client_id' => 'unknown_client', + ]); + + $response = $this->client->getResponse(); + + $this->assertSame(401, $response->getStatusCode()); + $this->assertSame('application/json', $response->headers->get('Content-Type')); + + $jsonResponse = json_decode($response->getContent(), true); + + $this->assertNotEmpty($jsonResponse['error']); + $this->assertNotEmpty($jsonResponse['error_description']); + $this->assertSame('invalid_client', $jsonResponse['error']); + } + +} diff --git a/tests/Acceptance/DoctrineCredentialsRevokerTest.php b/tests/Acceptance/DoctrineCredentialsRevokerTest.php index 4de2ebf1..7c7ef216 100644 --- a/tests/Acceptance/DoctrineCredentialsRevokerTest.php +++ b/tests/Acceptance/DoctrineCredentialsRevokerTest.php @@ -9,6 +9,7 @@ use League\Bundle\OAuth2ServerBundle\Model\AccessToken; use League\Bundle\OAuth2ServerBundle\Model\AuthorizationCode; use League\Bundle\OAuth2ServerBundle\Model\Client; +use League\Bundle\OAuth2ServerBundle\Model\DeviceCode; use League\Bundle\OAuth2ServerBundle\Model\RefreshToken; use League\Bundle\OAuth2ServerBundle\Service\CredentialsRevoker\DoctrineCredentialsRevoker; use League\Bundle\OAuth2ServerBundle\Tests\Fixtures\FixtureFactory; @@ -33,10 +34,12 @@ public function testRevokesAllCredentialsForUser(): void $authCode = $this->buildAuthCode('foo', '+1 minute', $client, $userIdentifier); $accessToken = $this->buildAccessToken('bar', '+1 minute', $client, $userIdentifier); $refreshToken = $this->buildRefreshToken('baz', '+1 minute', $accessToken); + $deviceCode = $this->buildDeviceCode('baz', '+1 minute', $client, $userIdentifier); $em->persist($authCode); $em->persist($accessToken); $em->persist($refreshToken); + $em->persist($deviceCode); $em->flush(); $revoker = new DoctrineCredentialsRevoker($em, new ClientManager($em, self::getContainer()->get(EventDispatcherInterface::class), Client::class)); @@ -46,10 +49,12 @@ public function testRevokesAllCredentialsForUser(): void $em->refresh($authCode); $em->refresh($accessToken); $em->refresh($refreshToken); + $em->refresh($deviceCode); $this->assertTrue($authCode->isRevoked()); $this->assertTrue($accessToken->isRevoked()); $this->assertTrue($refreshToken->isRevoked()); + $this->assertTrue($deviceCode->isRevoked()); } public function testRevokesAllCredentialsForClient(): void @@ -62,10 +67,12 @@ public function testRevokesAllCredentialsForClient(): void $authCode = $this->buildAuthCode('foo', '+1 minute', $client, 'john'); $accessToken = $this->buildAccessToken('bar', '+1 minute', $client); $refreshToken = $this->buildRefreshToken('baz', '+1 minute', $accessToken); + $deviceCode = $this->buildDeviceCode('baz', '+1 minute', $client, 'john'); $em->persist($authCode); $em->persist($accessToken); $em->persist($refreshToken); + $em->persist($deviceCode); $em->flush(); $revoker = new DoctrineCredentialsRevoker($em, new ClientManager($em, self::getContainer()->get(EventDispatcherInterface::class), Client::class)); @@ -75,10 +82,12 @@ public function testRevokesAllCredentialsForClient(): void $em->refresh($authCode); $em->refresh($accessToken); $em->refresh($refreshToken); + $em->refresh($deviceCode); $this->assertTrue($authCode->isRevoked()); $this->assertTrue($accessToken->isRevoked()); $this->assertTrue($refreshToken->isRevoked()); + $this->assertTrue($deviceCode->isRevoked()); } private function buildRefreshToken(string $identifier, string $modify, AccessToken $accessToken): RefreshToken @@ -111,4 +120,21 @@ private function buildAuthCode(string $identifier, string $modify, Client $clien [] ); } + + private function buildDeviceCode(string $identifier, string $modify, Client $client, ?string $userIdentifier = null): DeviceCode + { + return new DeviceCode( + $identifier, + new \DateTimeImmutable($modify), + $client, + $userIdentifier, + [], + '', + false, + false, + '', + null, + 5 + ); + } } diff --git a/tests/Acceptance/DoctrineDeviceCodeManagerTest.php b/tests/Acceptance/DoctrineDeviceCodeManagerTest.php new file mode 100644 index 00000000..2602acb6 --- /dev/null +++ b/tests/Acceptance/DoctrineDeviceCodeManagerTest.php @@ -0,0 +1,83 @@ +client->getContainer()->get('doctrine.orm.entity_manager'); + + $doctrineDeviceCodeManager = new DoctrineDeviceCodeManager($em); + + $client = new Client('client', 'client', 'secret'); + $em->persist($client); + + $testData = $this->buildClearExpiredTestData($client); + + /** @var DeviceCode $authCode */ + foreach ($testData['input'] as $authCode) { + $doctrineDeviceCodeManager->save($authCode); + } + + $em->flush(); + + $this->assertSame(3, $doctrineDeviceCodeManager->clearExpired()); + + $this->assertSame( + array_values($testData['output']), + $em->getRepository(DeviceCode::class)->findBy([], ['identifier' => 'ASC']) + ); + } + + private function buildClearExpiredTestData($client): array + { + $validDeviceCodes = [ + '1111' => $this->buildDeviceCode('1111', '+1 day', $client), + '2222' => $this->buildDeviceCode('2222', '+1 hour', $client), + '3333' => $this->buildDeviceCode('3333', '+5 seconds', $client), + ]; + + $expiredDeviceCodes = [ + '5555' => $this->buildDeviceCode('5555', '-1 day', $client), + '6666' => $this->buildDeviceCode('6666', '-1 hour', $client), + '7777' => $this->buildDeviceCode('7777', '-1 second', $client), + ]; + + return [ + 'output' => $validDeviceCodes, + 'input' => $validDeviceCodes + $expiredDeviceCodes, + ]; + } + + private function buildDeviceCode(string $identifier, string $modify, $client): DeviceCode + { + return new DeviceCode( + $identifier, + new \DateTimeImmutable($modify), + $client, + null, + [], + '', + false, + false, + '', + null, + 5 + ); + } + +} diff --git a/tests/Acceptance/SecurityLayerTest.php b/tests/Acceptance/SecurityLayerTest.php index 6e13e816..8365ea8a 100644 --- a/tests/Acceptance/SecurityLayerTest.php +++ b/tests/Acceptance/SecurityLayerTest.php @@ -7,6 +7,7 @@ use League\Bundle\OAuth2ServerBundle\Manager\AccessTokenManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\AuthorizationCodeManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface; +use League\Bundle\OAuth2ServerBundle\Manager\DeviceCodeManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\RefreshTokenManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\ScopeManagerInterface; use League\Bundle\OAuth2ServerBundle\Tests\Fixtures\FixtureFactory; @@ -23,7 +24,8 @@ protected function setUp(): void $this->client->getContainer()->get(ClientManagerInterface::class), $this->client->getContainer()->get(AccessTokenManagerInterface::class), $this->client->getContainer()->get(RefreshTokenManagerInterface::class), - $this->client->getContainer()->get(AuthorizationCodeManagerInterface::class) + $this->client->getContainer()->get(AuthorizationCodeManagerInterface::class), + $this->client->getContainer()->get(DeviceCodeManagerInterface::class) ); } diff --git a/tests/Acceptance/TokenEndpointTest.php b/tests/Acceptance/TokenEndpointTest.php index 5f586714..8ad5eef0 100644 --- a/tests/Acceptance/TokenEndpointTest.php +++ b/tests/Acceptance/TokenEndpointTest.php @@ -9,6 +9,7 @@ use League\Bundle\OAuth2ServerBundle\Manager\AccessTokenManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\AuthorizationCodeManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface; +use League\Bundle\OAuth2ServerBundle\Manager\DeviceCodeManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\RefreshTokenManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\ScopeManagerInterface; use League\Bundle\OAuth2ServerBundle\OAuth2Events; @@ -29,7 +30,8 @@ protected function setUp(): void $this->client->getContainer()->get(ClientManagerInterface::class), $this->client->getContainer()->get(AccessTokenManagerInterface::class), $this->client->getContainer()->get(RefreshTokenManagerInterface::class), - $this->client->getContainer()->get(AuthorizationCodeManagerInterface::class) + $this->client->getContainer()->get(AuthorizationCodeManagerInterface::class), + $this->client->getContainer()->get(DeviceCodeManagerInterface::class) ); } @@ -340,4 +342,110 @@ public function testFailedClientCredentialsTokenRequest(): void $this->assertTrue($wasClientAuthenticationEventDispatched); } + + public function testSuccessfulDeviceCodeRequestWithPublicClient(): void + { + $deviceCode = $this->client + ->getContainer() + ->get(DeviceCodeManagerInterface::class) + ->find(FixtureFactory::FIXTURE_DEVICE_CODE_APPROVED); + + $eventDispatcher = $this->client->getContainer()->get('event_dispatcher'); + + $eventDispatcher->addListener(OAuth2Events::TOKEN_REQUEST_RESOLVE, static function (TokenRequestResolveEvent $event): void { + $event->getResponse()->headers->set('foo', 'bar'); + }); + + $wasRequestAccessTokenEventDispatched = false; + $wasRequestRefreshTokenEventDispatched = false; + $accessToken = null; + $refreshToken = null; + + $eventDispatcher->addListener(RequestEvent::ACCESS_TOKEN_ISSUED, static function (RequestAccessTokenEvent $event) use (&$wasRequestAccessTokenEventDispatched, &$accessToken): void { + $wasRequestAccessTokenEventDispatched = true; + $accessToken = $event->getAccessToken(); + }); + + $eventDispatcher->addListener(RequestEvent::REFRESH_TOKEN_ISSUED, static function (RequestRefreshTokenEvent $event) use (&$wasRequestRefreshTokenEventDispatched, &$refreshToken): void { + $wasRequestRefreshTokenEventDispatched = true; + $refreshToken = $event->getRefreshToken(); + }); + + $this->client->request('POST', '/token', [ + 'client_id' => $deviceCode->getClient()->getIdentifier(), + 'client_secret' => 'top_secret', + 'grant_type' => 'urn:ietf:params:oauth:grant-type:device_code', + 'device_code' => $deviceCode->getIdentifier(), + ]); + + $response = $this->client->getResponse(); + + $this->assertSame(200, $response->getStatusCode()); + $this->assertSame('application/json; charset=UTF-8', $response->headers->get('Content-Type')); + + $jsonResponse = json_decode($response->getContent(), true); + + $this->assertSame('Bearer', $jsonResponse['token_type']); + $this->assertLessThanOrEqual(3600, $jsonResponse['expires_in']); + $this->assertGreaterThan(0, $jsonResponse['expires_in']); + $this->assertNotEmpty($jsonResponse['access_token']); + $this->assertNotEmpty($jsonResponse['refresh_token']); + + $this->assertTrue($wasRequestAccessTokenEventDispatched); + $this->assertTrue($wasRequestRefreshTokenEventDispatched); + + $this->assertSame($deviceCode->getClient()->getIdentifier(), $accessToken->getClient()->getIdentifier()); + $this->assertSame($deviceCode->getUserIdentifier(), $accessToken->getUserIdentifier()); + $this->assertSame($accessToken->getIdentifier(), $refreshToken->getAccessToken()->getIdentifier()); + } + + public function testFailedDeviceCodeRequestWithExpiredCode(): void + { + $deviceCode = $this->client + ->getContainer() + ->get(DeviceCodeManagerInterface::class) + ->find(FixtureFactory::FIXTURE_DEVICE_CODE_EXPIRED); + + $this->client->request('POST', '/token', [ + 'client_id' => $deviceCode->getClient()->getIdentifier(), + 'client_secret' => 'secret', + 'grant_type' => 'urn:ietf:params:oauth:grant-type:device_code', + 'device_code' => $deviceCode->getIdentifier(), + ]); + + $response = $this->client->getResponse(); + + $this->assertSame(400, $response->getStatusCode()); + $this->assertSame('application/json', $response->headers->get('Content-Type')); + + $jsonResponse = json_decode($response->getContent(), true); + + $this->assertSame('expired_token', $jsonResponse['error']); + $this->assertSame('device_code', $jsonResponse['hint']); + } + + public function testFailedDeviceCodeWithPendingCode(): void + { + $deviceCode = $this->client + ->getContainer() + ->get(DeviceCodeManagerInterface::class) + ->find(FixtureFactory::FIXTURE_DEVICE_CODE_PUBLIC_CLIENT); + + $this->client->request('POST', '/token', [ + 'client_id' => $deviceCode->getClient()->getIdentifier(), + 'grant_type' => 'urn:ietf:params:oauth:grant-type:device_code', + 'device_code' => $deviceCode->getIdentifier(), + ]); + + $response = $this->client->getResponse(); + + $this->assertSame(400, $response->getStatusCode()); + $this->assertSame('application/json', $response->headers->get('Content-Type')); + + $jsonResponse = json_decode($response->getContent(), true); + + $this->assertSame('authorization_pending', $jsonResponse['error']); + $this->assertSame('', $jsonResponse['hint']); + } + } diff --git a/tests/Fixtures/FakeDeviceCodeManager.php b/tests/Fixtures/FakeDeviceCodeManager.php new file mode 100644 index 00000000..bf2bdfcf --- /dev/null +++ b/tests/Fixtures/FakeDeviceCodeManager.php @@ -0,0 +1,30 @@ +save($scope); @@ -97,6 +105,10 @@ public static function initializeFixtures( foreach (self::createAuthorizationCodes($clientManager) as $authorizationCode) { $authCodeManager->save($authorizationCode); } + + foreach (self::createDeviceCodes($clientManager) as $deviceCode) { + $deviceCodeManager->save($deviceCode); + } } /** @@ -249,6 +261,72 @@ public static function createAuthorizationCodes(ClientManagerInterface $clientMa return $authorizationCodes; } + /** + * @return DeviceCode[] + */ + public static function createDeviceCodes(ClientManagerInterface $clientManager): array + { + $deviceCodes = []; + + $deviceCodes[] = new DeviceCode( + self::FIXTURE_DEVICE_CODE, + new \DateTimeImmutable('+10 minute'), + $clientManager->find(self::FIXTURE_CLIENT_FIRST), + null, + [], + 'XQMWNGSP', + false, + false, + '', + null, + 5 + ); + + $deviceCodes[] = new DeviceCode( + self::FIXTURE_DEVICE_CODE_PUBLIC_CLIENT, + new \DateTimeImmutable('+10 minute'), + $clientManager->find(self::FIXTURE_PUBLIC_CLIENT), + null, + [], + 'XQMWNGSP', + false, + false, + '', + null, + 5 + ); + + $deviceCodes[] = new DeviceCode( + self::FIXTURE_DEVICE_CODE_APPROVED, + new \DateTimeImmutable('+10 minute'), + $clientManager->find(self::FIXTURE_CLIENT_SECOND), + self::FIXTURE_USER, + [], + 'XQMWNGSP', + true, + false, + '', + null, + 5 + ); + + $deviceCodes[] = new DeviceCode( + self::FIXTURE_DEVICE_CODE_EXPIRED, + new \DateTimeImmutable('-30 minute'), + $clientManager->find(self::FIXTURE_CLIENT_FIRST), + null, + [], + 'XQMWNGSP', + false, + false, + '', + null, + 5 + ); + + return $deviceCodes; + } + /** * @return Client[] */ diff --git a/tests/Integration/AbstractIntegrationTest.php b/tests/Integration/AbstractIntegrationTest.php index ef91fb65..5fe915a7 100644 --- a/tests/Integration/AbstractIntegrationTest.php +++ b/tests/Integration/AbstractIntegrationTest.php @@ -15,6 +15,7 @@ use League\Bundle\OAuth2ServerBundle\Manager\InMemory\AccessTokenManager; use League\Bundle\OAuth2ServerBundle\Manager\InMemory\AuthorizationCodeManager; use League\Bundle\OAuth2ServerBundle\Manager\InMemory\ClientManager; +use League\Bundle\OAuth2ServerBundle\Manager\InMemory\DeviceCodeManager; use League\Bundle\OAuth2ServerBundle\Manager\InMemory\RefreshTokenManager; use League\Bundle\OAuth2ServerBundle\Manager\InMemory\ScopeManager; use League\Bundle\OAuth2ServerBundle\Manager\RefreshTokenManagerInterface; @@ -24,6 +25,7 @@ use League\Bundle\OAuth2ServerBundle\Repository\AccessTokenRepository; use League\Bundle\OAuth2ServerBundle\Repository\AuthCodeRepository; use League\Bundle\OAuth2ServerBundle\Repository\ClientRepository; +use League\Bundle\OAuth2ServerBundle\Repository\DeviceCodeRepository; use League\Bundle\OAuth2ServerBundle\Repository\RefreshTokenRepository; use League\Bundle\OAuth2ServerBundle\Repository\ScopeRepository; use League\Bundle\OAuth2ServerBundle\Repository\UserRepository; @@ -33,6 +35,7 @@ use League\OAuth2\Server\Exception\OAuthServerException; use League\OAuth2\Server\Grant\AuthCodeGrant; use League\OAuth2\Server\Grant\ClientCredentialsGrant; +use League\OAuth2\Server\Grant\DeviceCodeGrant; use League\OAuth2\Server\Grant\ImplicitGrant; use League\OAuth2\Server\Grant\PasswordGrant; use League\OAuth2\Server\Grant\RefreshTokenGrant; @@ -72,6 +75,11 @@ abstract class AbstractIntegrationTest extends TestCase */ protected $authCodeManager; + /** + * @var DeviceCodeManager + */ + protected $deviceCodeManager; + /** * @var RefreshTokenManagerInterface */ @@ -110,6 +118,7 @@ protected function setUp(): void $this->accessTokenManager = new AccessTokenManager(true); $this->refreshTokenManager = new RefreshTokenManager(); $this->authCodeManager = new AuthorizationCodeManager(); + $this->deviceCodeManager = new DeviceCodeManager(); $scopeConverter = new ScopeConverter(); $scopeRepository = new ScopeRepository($this->scopeManager, $this->clientManager, $scopeConverter, $this->eventDispatcher); @@ -119,6 +128,7 @@ protected function setUp(): void $userConverter = new UserConverter(); $userRepository = new UserRepository($this->clientManager, $this->eventDispatcher, $userConverter); $authCodeRepository = new AuthCodeRepository($this->authCodeManager, $this->clientManager, $scopeConverter); + $deviceCodeRepository = new DeviceCodeRepository($this->deviceCodeManager, $this->clientManager, $scopeConverter, $clientRepository); $this->authorizationServer = $this->createAuthorizationServer( $scopeRepository, @@ -126,7 +136,8 @@ protected function setUp(): void $accessTokenRepository, $refreshTokenRepository, $userRepository, - $authCodeRepository + $authCodeRepository, + $deviceCodeRepository ); $this->resourceServer = $this->createResourceServer($accessTokenRepository); @@ -269,6 +280,7 @@ private function createAuthorizationServer( RefreshTokenRepositoryInterface $refreshTokenRepository, UserRepositoryInterface $userRepository, AuthCodeRepositoryInterface $authCodeRepository, + DeviceCodeRepository $deviceCodeRepository ): AuthorizationServer { $authorizationServer = new AuthorizationServer( $clientRepository, @@ -289,6 +301,7 @@ private function createAuthorizationServer( $authorizationServer->enableGrantType(new PasswordGrant($userRepository, $refreshTokenRepository)); $authorizationServer->enableGrantType($authCodeGrant); $authorizationServer->enableGrantType(new ImplicitGrant(new \DateInterval('PT10M'))); + $authorizationServer->enableGrantType(new DeviceCodeGrant($deviceCodeRepository, $refreshTokenRepository, new \DateInterval('PT10M'), 'http://localhost/verify-url', 5)); return $authorizationServer; } diff --git a/tests/Integration/AuthorizationServerTest.php b/tests/Integration/AuthorizationServerTest.php index 88bae9be..cb0e9988 100644 --- a/tests/Integration/AuthorizationServerTest.php +++ b/tests/Integration/AuthorizationServerTest.php @@ -25,7 +25,8 @@ protected function setUp(): void $this->clientManager, $this->accessTokenManager, $this->refreshTokenManager, - $this->authCodeManager + $this->authCodeManager, + $this->deviceCodeManager ); } diff --git a/tests/Integration/DeviceCodeRepositoryTest.php b/tests/Integration/DeviceCodeRepositoryTest.php new file mode 100644 index 00000000..5b876836 --- /dev/null +++ b/tests/Integration/DeviceCodeRepositoryTest.php @@ -0,0 +1,46 @@ +deviceCodeManager->save($deviceCode); + + $this->assertSame($deviceCode, $this->deviceCodeManager->find($identifier)); + + $deviceCodeRepository = new DeviceCodeRepository( + $this->deviceCodeManager, $this->clientManager, new ScopeConverter(), new ClientRepository($this->clientManager) + ); + + $deviceCodeRepository->revokeDeviceCode($identifier); + + $this->assertTrue($deviceCode->isRevoked()); + $this->assertSame($deviceCode, $this->deviceCodeManager->find($identifier)); + } +} diff --git a/tests/Integration/ResourceServerTest.php b/tests/Integration/ResourceServerTest.php index b52f9b37..3385c40e 100644 --- a/tests/Integration/ResourceServerTest.php +++ b/tests/Integration/ResourceServerTest.php @@ -18,7 +18,8 @@ protected function setUp(): void $this->clientManager, $this->accessTokenManager, $this->refreshTokenManager, - $this->authCodeManager + $this->authCodeManager, + $this->deviceCodeManager ); } diff --git a/tests/TestKernel.php b/tests/TestKernel.php index c25870f3..e19649a8 100644 --- a/tests/TestKernel.php +++ b/tests/TestKernel.php @@ -9,12 +9,14 @@ use League\Bundle\OAuth2ServerBundle\Manager\AccessTokenManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\AuthorizationCodeManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface; +use League\Bundle\OAuth2ServerBundle\Manager\DeviceCodeManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\RefreshTokenManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\ScopeManagerInterface; use League\Bundle\OAuth2ServerBundle\Tests\Fixtures\FakeAccessTokenManager; use League\Bundle\OAuth2ServerBundle\Tests\Fixtures\FakeAuthorizationCodeManager; use League\Bundle\OAuth2ServerBundle\Tests\Fixtures\FakeClientManager; use League\Bundle\OAuth2ServerBundle\Tests\Fixtures\FakeCredentialsRevoker; +use League\Bundle\OAuth2ServerBundle\Tests\Fixtures\FakeDeviceCodeManager; use League\Bundle\OAuth2ServerBundle\Tests\Fixtures\FakeGrant; use League\Bundle\OAuth2ServerBundle\Tests\Fixtures\FakeRefreshTokenManager; use League\Bundle\OAuth2ServerBundle\Tests\Fixtures\FixtureFactory; @@ -159,6 +161,10 @@ public function registerContainerConfiguration(LoaderInterface $loader): void 'path' => '^/authorize', 'roles' => 'IS_AUTHENTICATED', ], + [ + 'path' => '^/device-code', + 'roles' => 'IS_AUTHENTICATED', + ], ], ]; @@ -215,6 +221,11 @@ private function exposeManagerServices(ContainerBuilder $container): void ->getAlias(AuthorizationCodeManagerInterface::class) ->setPublic(true) ; + + $container + ->getAlias(DeviceCodeManagerInterface::class) + ->setPublic(true) + ; } private function configureControllers(ContainerBuilder $container): void @@ -242,6 +253,7 @@ private function configureCustomPersistenceServices(ContainerBuilder $container) $container->register('test.client_manager', FakeClientManager::class)->setPublic(true); $container->register('test.refresh_token_manager', FakeRefreshTokenManager::class)->setPublic(true); $container->register('test.credentials_revoker', FakeCredentialsRevoker::class)->setPublic(true); + $container->register('test.device_code_manager', FakeDeviceCodeManager::class)->setPublic(true); } private function registerFakeGrant(ContainerBuilder $container): void diff --git a/tests/Unit/InMemoryDeviceCodeManagerTest.php b/tests/Unit/InMemoryDeviceCodeManagerTest.php new file mode 100644 index 00000000..640f69f9 --- /dev/null +++ b/tests/Unit/InMemoryDeviceCodeManagerTest.php @@ -0,0 +1,71 @@ +buildClearExpiredTestData(); + + foreach ($testData['input'] as $token) { + $inMemoryDeviceCodeManager->save($token); + } + + $this->assertSame(3, $inMemoryDeviceCodeManager->clearExpired()); + + $reflectionProperty = new \ReflectionProperty(InMemoryDeviceCodeManager::class, 'deviceCodes'); + $reflectionProperty->setAccessible(true); + + $this->assertSame($testData['output'], $reflectionProperty->getValue($inMemoryDeviceCodeManager)); + } + + private function buildClearExpiredTestData(): array + { + $validDeviceCodes = [ + '1111' => $this->buildDeviceCode('1111', '+1 day'), + '2222' => $this->buildDeviceCode('2222', '+1 hour'), + '3333' => $this->buildDeviceCode('3333', '+5 seconds'), + ]; + + $expiredDeviceCodes = [ + '5555' => $this->buildDeviceCode('5555', '-1 day'), + '6666' => $this->buildDeviceCode('6666', '-1 hour'), + '7777' => $this->buildDeviceCode('7777', '-1 second'), + ]; + + return [ + 'input' => $validDeviceCodes + $expiredDeviceCodes, + 'output' => $validDeviceCodes, + ]; + } + + private function buildDeviceCode(string $identifier, string $modify): DeviceCode + { + return new DeviceCode( + $identifier, + new \DateTimeImmutable($modify), + new Client('name', 'identifier', 'secret'), + null, + [], + '', + false, + false, + '', + null, + 5 + ); + } +} From 515c9425a74bf70e855e8069808880b22d25f258 Mon Sep 17 00:00:00 2001 From: Simon Van Accoleyen Date: Tue, 25 Mar 2025 21:24:06 +0100 Subject: [PATCH 2/3] fix: lint and static analysis --- config/services.php | 16 ++++----- config/storage/doctrine.php | 4 +-- config/storage/in_memory.php | 4 +-- src/Controller/DeviceCodeController.php | 34 ------------------- src/Entity/DeviceCode.php | 2 ++ src/Manager/Doctrine/DeviceCodeManager.php | 4 +-- src/Manager/InMemory/DeviceCodeManager.php | 1 - src/Model/DeviceCode.php | 16 ++++----- src/Model/DeviceCodeInterface.php | 4 ++- src/Repository/DeviceCodeRepository.php | 22 ++++++------ tests/Acceptance/DeviceCodeEndpointTest.php | 1 - .../DoctrineCredentialsRevokerTest.php | 1 - .../DoctrineDeviceCodeManagerTest.php | 4 +-- tests/Acceptance/TokenEndpointTest.php | 1 - tests/Fixtures/FixtureFactory.php | 6 +--- tests/Integration/AbstractIntegrationTest.php | 2 +- .../Integration/DeviceCodeRepositoryTest.php | 1 - tests/Unit/InMemoryDeviceCodeManagerTest.php | 5 ++- 18 files changed, 39 insertions(+), 89 deletions(-) diff --git a/config/services.php b/config/services.php index c68a1d01..aa52723d 100644 --- a/config/services.php +++ b/config/services.php @@ -2,11 +2,6 @@ declare(strict_types=1); -use League\Bundle\OAuth2ServerBundle\Controller\DeviceCodeController; -use League\Bundle\OAuth2ServerBundle\Manager\DeviceCodeManagerInterface; -use League\Bundle\OAuth2ServerBundle\Repository\DeviceCodeRepository; -use League\OAuth2\Server\Grant\DeviceCodeGrant; -use League\OAuth2\Server\Repositories\DeviceCodeRepositoryInterface; use function Symfony\Component\DependencyInjection\Loader\Configurator\abstract_arg; use function Symfony\Component\DependencyInjection\Loader\Configurator\param; use function Symfony\Component\DependencyInjection\Loader\Configurator\service; @@ -20,6 +15,7 @@ use League\Bundle\OAuth2ServerBundle\Command\ListClientsCommand; use League\Bundle\OAuth2ServerBundle\Command\UpdateClientCommand; use League\Bundle\OAuth2ServerBundle\Controller\AuthorizationController; +use League\Bundle\OAuth2ServerBundle\Controller\DeviceCodeController; use League\Bundle\OAuth2ServerBundle\Controller\TokenController; use League\Bundle\OAuth2ServerBundle\Converter\ScopeConverter; use League\Bundle\OAuth2ServerBundle\Converter\ScopeConverterInterface; @@ -30,12 +26,14 @@ use League\Bundle\OAuth2ServerBundle\Manager\AccessTokenManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\AuthorizationCodeManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface; +use League\Bundle\OAuth2ServerBundle\Manager\DeviceCodeManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\InMemory\ScopeManager; use League\Bundle\OAuth2ServerBundle\Manager\RefreshTokenManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\ScopeManagerInterface; use League\Bundle\OAuth2ServerBundle\OAuth2Events; use League\Bundle\OAuth2ServerBundle\Repository\AuthCodeRepository; use League\Bundle\OAuth2ServerBundle\Repository\ClientRepository; +use League\Bundle\OAuth2ServerBundle\Repository\DeviceCodeRepository; use League\Bundle\OAuth2ServerBundle\Repository\RefreshTokenRepository; use League\Bundle\OAuth2ServerBundle\Repository\ScopeRepository; use League\Bundle\OAuth2ServerBundle\Repository\UserRepository; @@ -47,12 +45,14 @@ use League\OAuth2\Server\EventEmitting\EventEmitter; use League\OAuth2\Server\Grant\AuthCodeGrant; use League\OAuth2\Server\Grant\ClientCredentialsGrant; +use League\OAuth2\Server\Grant\DeviceCodeGrant; use League\OAuth2\Server\Grant\ImplicitGrant; use League\OAuth2\Server\Grant\PasswordGrant; use League\OAuth2\Server\Grant\RefreshTokenGrant; use League\OAuth2\Server\Repositories\AccessTokenRepositoryInterface; use League\OAuth2\Server\Repositories\AuthCodeRepositoryInterface; use League\OAuth2\Server\Repositories\ClientRepositoryInterface; +use League\OAuth2\Server\Repositories\DeviceCodeRepositoryInterface; use League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface; use League\OAuth2\Server\Repositories\ScopeRepositoryInterface; use League\OAuth2\Server\Repositories\UserRepositoryInterface; @@ -216,7 +216,7 @@ service(RefreshTokenRepositoryInterface::class), null, null, - null + null, ]) ->alias(DeviceCodeGrant::class, 'league.oauth2_server.grant.device_code') @@ -244,10 +244,6 @@ ->set('league.oauth2_server.controller.device_code', DeviceCodeController::class) ->args([ service(AuthorizationServer::class), - service(EventDispatcherInterface::class), - service(AuthorizationRequestResolveEventFactory::class), - service(UserConverterInterface::class), - service(ClientManagerInterface::class), service('league.oauth2_server.factory.psr_http'), service('league.oauth2_server.factory.http_foundation'), service('league.oauth2_server.factory.psr17'), diff --git a/config/storage/doctrine.php b/config/storage/doctrine.php index 68dfb107..bbb49c6b 100644 --- a/config/storage/doctrine.php +++ b/config/storage/doctrine.php @@ -2,16 +2,16 @@ declare(strict_types=1); -use League\Bundle\OAuth2ServerBundle\Manager\DeviceCodeManagerInterface; -use League\Bundle\OAuth2ServerBundle\Manager\Doctrine\DeviceCodeManager; use function Symfony\Component\DependencyInjection\Loader\Configurator\service; use League\Bundle\OAuth2ServerBundle\Manager\AccessTokenManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\AuthorizationCodeManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface; +use League\Bundle\OAuth2ServerBundle\Manager\DeviceCodeManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\Doctrine\AccessTokenManager; use League\Bundle\OAuth2ServerBundle\Manager\Doctrine\AuthorizationCodeManager; use League\Bundle\OAuth2ServerBundle\Manager\Doctrine\ClientManager; +use League\Bundle\OAuth2ServerBundle\Manager\Doctrine\DeviceCodeManager; use League\Bundle\OAuth2ServerBundle\Manager\Doctrine\RefreshTokenManager; use League\Bundle\OAuth2ServerBundle\Manager\RefreshTokenManagerInterface; use League\Bundle\OAuth2ServerBundle\Persistence\Mapping\Driver; diff --git a/config/storage/in_memory.php b/config/storage/in_memory.php index 5565c652..d72df917 100644 --- a/config/storage/in_memory.php +++ b/config/storage/in_memory.php @@ -2,16 +2,16 @@ declare(strict_types=1); -use League\Bundle\OAuth2ServerBundle\Manager\DeviceCodeManagerInterface; -use League\Bundle\OAuth2ServerBundle\Manager\InMemory\DeviceCodeManager; use function Symfony\Component\DependencyInjection\Loader\Configurator\service; use League\Bundle\OAuth2ServerBundle\Manager\AccessTokenManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\AuthorizationCodeManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface; +use League\Bundle\OAuth2ServerBundle\Manager\DeviceCodeManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\InMemory\AccessTokenManager; use League\Bundle\OAuth2ServerBundle\Manager\InMemory\AuthorizationCodeManager; use League\Bundle\OAuth2ServerBundle\Manager\InMemory\ClientManager; +use League\Bundle\OAuth2ServerBundle\Manager\InMemory\DeviceCodeManager; use League\Bundle\OAuth2ServerBundle\Manager\InMemory\RefreshTokenManager; use League\Bundle\OAuth2ServerBundle\Manager\RefreshTokenManagerInterface; use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; diff --git a/src/Controller/DeviceCodeController.php b/src/Controller/DeviceCodeController.php index 7b3e57c1..74394b51 100644 --- a/src/Controller/DeviceCodeController.php +++ b/src/Controller/DeviceCodeController.php @@ -4,11 +4,6 @@ namespace League\Bundle\OAuth2ServerBundle\Controller; -use League\Bundle\OAuth2ServerBundle\Converter\UserConverterInterface; -use League\Bundle\OAuth2ServerBundle\Event\AuthorizationRequestResolveEventFactory; -use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface; -use League\Bundle\OAuth2ServerBundle\Model\AbstractClient; -use League\Bundle\OAuth2ServerBundle\OAuth2Events; use League\OAuth2\Server\AuthorizationServer; use League\OAuth2\Server\Exception\OAuthServerException; use Psr\Http\Message\ResponseFactoryInterface; @@ -16,7 +11,6 @@ use Symfony\Bridge\PsrHttpMessage\HttpMessageFactoryInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; -use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; final class DeviceCodeController { @@ -25,26 +19,6 @@ final class DeviceCodeController */ private $server; - /** - * @var EventDispatcherInterface - */ - private $eventDispatcher; - - /** - * @var AuthorizationRequestResolveEventFactory - */ - private $eventFactory; - - /** - * @var UserConverterInterface - */ - private $userConverter; - - /** - * @var ClientManagerInterface - */ - private $clientManager; - /** * @var HttpMessageFactoryInterface */ @@ -62,19 +36,11 @@ final class DeviceCodeController public function __construct( AuthorizationServer $server, - EventDispatcherInterface $eventDispatcher, - AuthorizationRequestResolveEventFactory $eventFactory, - UserConverterInterface $userConverter, - ClientManagerInterface $clientManager, HttpMessageFactoryInterface $httpMessageFactory, HttpFoundationFactoryInterface $httpFoundationFactory, ResponseFactoryInterface $responseFactory, ) { $this->server = $server; - $this->eventDispatcher = $eventDispatcher; - $this->eventFactory = $eventFactory; - $this->userConverter = $userConverter; - $this->clientManager = $clientManager; $this->httpMessageFactory = $httpMessageFactory; $this->httpFoundationFactory = $httpFoundationFactory; $this->responseFactory = $responseFactory; diff --git a/src/Entity/DeviceCode.php b/src/Entity/DeviceCode.php index 4dd474ed..64308adb 100644 --- a/src/Entity/DeviceCode.php +++ b/src/Entity/DeviceCode.php @@ -1,5 +1,7 @@ entityManager->createQueryBuilder() ->select('dc') ->from(DeviceCode::class, 'dc') @@ -43,6 +43,7 @@ public function save(DeviceCodeInterface $deviceCode, bool $persist = true): voi if ($persist) { $this->entityManager->persist($deviceCode); } + $this->entityManager->flush(); } @@ -56,5 +57,4 @@ public function clearExpired(): int ->getQuery() ->execute(); } - } diff --git a/src/Manager/InMemory/DeviceCodeManager.php b/src/Manager/InMemory/DeviceCodeManager.php index e2f47f5d..51a19e67 100644 --- a/src/Manager/InMemory/DeviceCodeManager.php +++ b/src/Manager/InMemory/DeviceCodeManager.php @@ -46,5 +46,4 @@ public function clearExpired(): int return $count - \count($this->deviceCodes); } - } diff --git a/src/Model/DeviceCode.php b/src/Model/DeviceCode.php index d16d3b49..60a4709b 100644 --- a/src/Model/DeviceCode.php +++ b/src/Model/DeviceCode.php @@ -9,12 +9,12 @@ class DeviceCode implements DeviceCodeInterface { /** - * @var string + * @var non-empty-string */ private $identifier; /** - * @var \DateTimeInterface + * @var \DateTimeImmutable */ private $expiry; @@ -51,7 +51,7 @@ class DeviceCode implements DeviceCodeInterface /** * @var bool */ - private $includeVerificationUriComplete; + private $includeVerificationUriComplete = false; /** * @var string @@ -59,7 +59,7 @@ class DeviceCode implements DeviceCodeInterface private $verificationUri; /** - * @var \DateTimeInterface|null + * @var \DateTimeImmutable|null */ private $lastPolledAt; @@ -69,6 +69,7 @@ class DeviceCode implements DeviceCodeInterface private $interval; /** + * @param non-empty-string $identifier * @param list $scopes */ public function __construct( @@ -79,10 +80,9 @@ public function __construct( array $scopes, string $userCode, bool $userApproved, - bool $includeVerificationUriComplete, string $verificationUri, ?\DateTimeImmutable $lastPolledAt, - int $interval + int $interval, ) { $this->identifier = $identifier; $this->expiry = $expiry; @@ -91,7 +91,6 @@ public function __construct( $this->scopes = $scopes; $this->userCode = $userCode; $this->userApproved = $userApproved; - $this->includeVerificationUriComplete = $includeVerificationUriComplete; $this->verificationUri = $verificationUri; $this->lastPolledAt = $lastPolledAt; $this->interval = $interval; @@ -117,7 +116,7 @@ public function getUserIdentifier(): ?string return $this->userIdentifier; } - public function setUserIdentifier($userIdentifier): DeviceCodeInterface + public function setUserIdentifier(?string $userIdentifier): DeviceCodeInterface { $this->userIdentifier = $userIdentifier; @@ -189,5 +188,4 @@ public function getInterval(): int { return $this->interval; } - } diff --git a/src/Model/DeviceCodeInterface.php b/src/Model/DeviceCodeInterface.php index d87ed47f..6bf2327a 100644 --- a/src/Model/DeviceCodeInterface.php +++ b/src/Model/DeviceCodeInterface.php @@ -10,6 +10,9 @@ interface DeviceCodeInterface { public function __toString(): string; + /** + * @return non-empty-string + */ public function getIdentifier(): string; public function getExpiry(): \DateTimeImmutable; @@ -44,5 +47,4 @@ public function getLastPolledAt(): ?\DateTimeImmutable; public function setLastPolledAt(\DateTimeImmutable $lastPolledAt): self; public function getInterval(): int; - } diff --git a/src/Repository/DeviceCodeRepository.php b/src/Repository/DeviceCodeRepository.php index 527b5e25..5de6f1d0 100644 --- a/src/Repository/DeviceCodeRepository.php +++ b/src/Repository/DeviceCodeRepository.php @@ -42,12 +42,12 @@ public function __construct( DeviceCodeManagerInterface $deviceCodeManager, ClientManagerInterface $clientManager, ScopeConverterInterface $scopeConverter, - ClientRepositoryInterface $clientRepository + ClientRepositoryInterface $clientRepository, ) { $this->deviceCodeManager = $deviceCodeManager; - $this->clientManager = $clientManager; - $this->scopeConverter = $scopeConverter; - $this->clientRepository = $clientRepository; + $this->clientManager = $clientManager; + $this->scopeConverter = $scopeConverter; + $this->clientRepository = $clientRepository; } public function getNewDeviceCode(): DeviceCodeEntityInterface @@ -76,7 +76,7 @@ public function approveDeviceCode(string $userCode, string $userId): void { $deviceCode = $this->deviceCodeManager->findByUserCode($userCode); - if ($deviceCode instanceof DeviceCodeInterface === false) { + if (false === $deviceCode instanceof DeviceCodeInterface) { throw OAuthServerException::invalidRequest('device_code', 'Device code does not exist'); } @@ -84,7 +84,7 @@ public function approveDeviceCode(string $userCode, string $userId): void throw OAuthServerException::invalidRequest('device_code', 'Device code has been revoked'); } - if ($userId === '') { + if ('' === $userId) { throw OAuthServerException::invalidRequest('user_id', 'User ID is required'); } @@ -134,9 +134,10 @@ private function buildDeviceCodeEntity(DeviceCodeInterface $deviceCode): DeviceC $deviceCodeEntity = new DeviceCodeEntity(); $deviceCodeEntity->setIdentifier($deviceCode->getIdentifier()); $deviceCodeEntity->setExpiryDateTime($deviceCode->getExpiry()); - $deviceCodeEntity->setClient( - $this->clientRepository->buildClientEntity($deviceCode->getClient()) - ); + $client = $this->clientRepository->getClientEntity($deviceCode->getClient()->getIdentifier()); + if ($client) { + $deviceCodeEntity->setClient($client); + } if ($deviceCode->getUserIdentifier()) { $deviceCodeEntity->setUserIdentifier($deviceCode->getUserIdentifier()); } @@ -156,7 +157,6 @@ private function buildDeviceCodeEntity(DeviceCodeInterface $deviceCode): DeviceC return $deviceCodeEntity; } - private function buildDeviceCodeModel(DeviceCodeEntityInterface $deviceCodeEntity): DeviceCodeModel { /** @var AbstractClient $client */ @@ -172,11 +172,9 @@ private function buildDeviceCodeModel(DeviceCodeEntityInterface $deviceCodeEntit $this->scopeConverter->toDomainArray(array_values($deviceCodeEntity->getScopes())), $deviceCodeEntity->getUserCode(), $deviceCodeEntity->getUserApproved(), - $deviceCodeEntity->getVerificationUriCompleteInAuthResponse(), $deviceCodeEntity->getVerificationUri(), $deviceCodeEntity->getLastPolledAt(), $deviceCodeEntity->getInterval() ); } - } diff --git a/tests/Acceptance/DeviceCodeEndpointTest.php b/tests/Acceptance/DeviceCodeEndpointTest.php index d83e3294..ea1e207a 100644 --- a/tests/Acceptance/DeviceCodeEndpointTest.php +++ b/tests/Acceptance/DeviceCodeEndpointTest.php @@ -64,5 +64,4 @@ public function testFailedWithUnkownClientRequest(): void $this->assertNotEmpty($jsonResponse['error_description']); $this->assertSame('invalid_client', $jsonResponse['error']); } - } diff --git a/tests/Acceptance/DoctrineCredentialsRevokerTest.php b/tests/Acceptance/DoctrineCredentialsRevokerTest.php index 7c7ef216..e1a1270a 100644 --- a/tests/Acceptance/DoctrineCredentialsRevokerTest.php +++ b/tests/Acceptance/DoctrineCredentialsRevokerTest.php @@ -131,7 +131,6 @@ private function buildDeviceCode(string $identifier, string $modify, Client $cli [], '', false, - false, '', null, 5 diff --git a/tests/Acceptance/DoctrineDeviceCodeManagerTest.php b/tests/Acceptance/DoctrineDeviceCodeManagerTest.php index 2602acb6..c112765e 100644 --- a/tests/Acceptance/DoctrineDeviceCodeManagerTest.php +++ b/tests/Acceptance/DoctrineDeviceCodeManagerTest.php @@ -6,8 +6,8 @@ use Doctrine\ORM\EntityManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\Doctrine\DeviceCodeManager as DoctrineDeviceCodeManager; -use League\Bundle\OAuth2ServerBundle\Model\DeviceCode; use League\Bundle\OAuth2ServerBundle\Model\Client; +use League\Bundle\OAuth2ServerBundle\Model\DeviceCode; /** * @TODO This should be in the Integration tests folder but the current tests infrastructure would need improvements first. @@ -73,11 +73,9 @@ private function buildDeviceCode(string $identifier, string $modify, $client): D [], '', false, - false, '', null, 5 ); } - } diff --git a/tests/Acceptance/TokenEndpointTest.php b/tests/Acceptance/TokenEndpointTest.php index 8ad5eef0..4c3add87 100644 --- a/tests/Acceptance/TokenEndpointTest.php +++ b/tests/Acceptance/TokenEndpointTest.php @@ -447,5 +447,4 @@ public function testFailedDeviceCodeWithPendingCode(): void $this->assertSame('authorization_pending', $jsonResponse['error']); $this->assertSame('', $jsonResponse['hint']); } - } diff --git a/tests/Fixtures/FixtureFactory.php b/tests/Fixtures/FixtureFactory.php index b5ac713f..ef5ebdbe 100644 --- a/tests/Fixtures/FixtureFactory.php +++ b/tests/Fixtures/FixtureFactory.php @@ -84,7 +84,7 @@ public static function initializeFixtures( AccessTokenManagerInterface $accessTokenManager, RefreshTokenManagerInterface $refreshTokenManager, AuthorizationCodeManagerInterface $authCodeManager, - DeviceCodeManagerInterface $deviceCodeManager + DeviceCodeManagerInterface $deviceCodeManager, ): void { foreach (self::createScopes() as $scope) { $scopeManager->save($scope); @@ -276,7 +276,6 @@ public static function createDeviceCodes(ClientManagerInterface $clientManager): [], 'XQMWNGSP', false, - false, '', null, 5 @@ -290,7 +289,6 @@ public static function createDeviceCodes(ClientManagerInterface $clientManager): [], 'XQMWNGSP', false, - false, '', null, 5 @@ -304,7 +302,6 @@ public static function createDeviceCodes(ClientManagerInterface $clientManager): [], 'XQMWNGSP', true, - false, '', null, 5 @@ -318,7 +315,6 @@ public static function createDeviceCodes(ClientManagerInterface $clientManager): [], 'XQMWNGSP', false, - false, '', null, 5 diff --git a/tests/Integration/AbstractIntegrationTest.php b/tests/Integration/AbstractIntegrationTest.php index 5fe915a7..39f3db94 100644 --- a/tests/Integration/AbstractIntegrationTest.php +++ b/tests/Integration/AbstractIntegrationTest.php @@ -280,7 +280,7 @@ private function createAuthorizationServer( RefreshTokenRepositoryInterface $refreshTokenRepository, UserRepositoryInterface $userRepository, AuthCodeRepositoryInterface $authCodeRepository, - DeviceCodeRepository $deviceCodeRepository + DeviceCodeRepository $deviceCodeRepository, ): AuthorizationServer { $authorizationServer = new AuthorizationServer( $clientRepository, diff --git a/tests/Integration/DeviceCodeRepositoryTest.php b/tests/Integration/DeviceCodeRepositoryTest.php index 5b876836..7581ecb4 100644 --- a/tests/Integration/DeviceCodeRepositoryTest.php +++ b/tests/Integration/DeviceCodeRepositoryTest.php @@ -24,7 +24,6 @@ public function testDeviceCodeRevoking(): void [], '', false, - false, '', null, 5 diff --git a/tests/Unit/InMemoryDeviceCodeManagerTest.php b/tests/Unit/InMemoryDeviceCodeManagerTest.php index 640f69f9..64f5d12d 100644 --- a/tests/Unit/InMemoryDeviceCodeManagerTest.php +++ b/tests/Unit/InMemoryDeviceCodeManagerTest.php @@ -5,8 +5,8 @@ namespace League\Bundle\OAuth2ServerBundle\Tests\Unit; use League\Bundle\OAuth2ServerBundle\Manager\InMemory\DeviceCodeManager as InMemoryDeviceCodeManager; -use League\Bundle\OAuth2ServerBundle\Model\DeviceCode; use League\Bundle\OAuth2ServerBundle\Model\Client; +use League\Bundle\OAuth2ServerBundle\Model\DeviceCode; use PHPUnit\Framework\TestCase; /** @@ -16,7 +16,7 @@ final class InMemoryDeviceCodeManagerTest extends TestCase { public function testClearExpired(): void { - $inMemoryDeviceCodeManager = new InMemoryDeviceCodeManager; + $inMemoryDeviceCodeManager = new InMemoryDeviceCodeManager(); $testData = $this->buildClearExpiredTestData(); @@ -62,7 +62,6 @@ private function buildDeviceCode(string $identifier, string $modify): DeviceCode [], '', false, - false, '', null, 5 From 3d3ceaa8d2cff8abf105aeedb96aed466ec6aa3d Mon Sep 17 00:00:00 2001 From: Simon Van Accoleyen Date: Sat, 26 Apr 2025 10:24:49 +0200 Subject: [PATCH 3/3] fix: feedback from PR #229 --- docs/device-code-grant.md | 56 +++++++++------------- src/Manager/Doctrine/DeviceCodeManager.php | 12 ++--- 2 files changed, 29 insertions(+), 39 deletions(-) diff --git a/docs/device-code-grant.md b/docs/device-code-grant.md index 1f474be2..7900a743 100644 --- a/docs/device-code-grant.md +++ b/docs/device-code-grant.md @@ -1,4 +1,4 @@ -# Password grant handling +# Device grant handling The device code grant type is designed for devices without a browser or with limited input capabilities. In this flow, the user authenticates on another device—like a smartphone or computer—and receives a code to enter on the original device. @@ -23,47 +23,37 @@ namespace App\Controller; use League\Bundle\OAuth2ServerBundle\Repository\DeviceCodeRepository; use League\OAuth2\Server\Exception\OAuthServerException; -use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; -use Symfony\Component\Form\Extension\Core\Type\SubmitType; use Symfony\Component\Form\Extension\Core\Type\TextType; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Routing\Attribute\Route; -class DeviceCodeController extends AbstractController -{ - - public function __construct( - private readonly DeviceCodeRepository $deviceCodeRepository - ) { - } +public function __construct( + private readonly DeviceCodeRepository $deviceCodeRepository +) { +} - #[Route(path: '/verify-device', name: 'app_verify_device', methods: ['GET', 'POST'])] - public function verifyDevice( - Request $request - ): Response { - $form = $this->createFormBuilder() - ->add('userCode', TextType::class, [ - 'required' => true, - ]) - ->getForm() - ->handleRequest($request); - - if ($form->isSubmitted() && $form->isValid()) { - try { - $this->deviceCodeRepository->approveDeviceCode($form->get('userCode')->getData(), $this->getUser()->getId()); - // Device code approved, show success message to user - } catch (OAuthServerException $e) { - // Handle exception (invalid code or missing user ID) - } +#[Route(path: '/verify-device', name: 'app_verify_device', methods: ['GET', 'POST'])] +public function verifyDevice( + Request $request +): Response { + $form = $this->createFormBuilder() + ->add('userCode', TextType::class, [ + 'required' => true, + ]) + ->getForm() + ->handleRequest($request); + + if ($form->isSubmitted() && $form->isValid()) { + try { + $this->deviceCodeRepository->approveDeviceCode($form->get('userCode')->getData(), $this->getUser()->getId()); + // Device code approved, show success message to user + } catch (OAuthServerException $e) { + // Handle exception (invalid code or missing user ID) } - - return $this->render( - 'verify_device.html.twig', - ['form' => $form] - ); } + // Render the form to the user } ``` diff --git a/src/Manager/Doctrine/DeviceCodeManager.php b/src/Manager/Doctrine/DeviceCodeManager.php index 0e04ecbd..8cc76449 100644 --- a/src/Manager/Doctrine/DeviceCodeManager.php +++ b/src/Manager/Doctrine/DeviceCodeManager.php @@ -30,12 +30,12 @@ public function findByUserCode(string $code): ?DeviceCodeInterface { /** @var ?DeviceCodeInterface */ return $this->entityManager->createQueryBuilder() - ->select('dc') - ->from(DeviceCode::class, 'dc') - ->where('dc.userCode = :code') - ->setParameter('code', $code) - ->getQuery() - ->getOneOrNullResult(); + ->select('dc') + ->from(DeviceCode::class, 'dc') + ->where('dc.userCode = :code') + ->setParameter('code', $code) + ->getQuery() + ->getOneOrNullResult(); } public function save(DeviceCodeInterface $deviceCode, bool $persist = true): void