diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 00000000..7809bfb4 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,9 @@ +/Tests export-ignore +/dev export-ignore +/.editorconfig export-ignore +/.gitattributes export-ignore +/.gitignore export-ignore +/.php_cs.dist export-ignore +/.travis.yml export-ignore +/docker-compose.yml export-ignore +/phpunit.xml.dist export-ignore diff --git a/.php_cs.dist b/.php_cs.dist index 2d146f50..d3c2e277 100644 --- a/.php_cs.dist +++ b/.php_cs.dist @@ -34,6 +34,7 @@ return PhpCsFixer\Config::create() 'yoda_style' => true, 'compact_nullable_typehint' => true, 'visibility_required' => true, + 'nullable_type_declaration_for_default_null_value' => true, ]) ->setRiskyAllowed(true) ->setFinder($finder) diff --git a/.travis.yml b/.travis.yml index fe633763..d8183811 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,3 +1,5 @@ +dist: bionic + sudo: required language: bash @@ -5,38 +7,34 @@ language: bash services: - docker -addons: - apt: - packages: - - docker-ce - env: # PHP 7.2 - - PHP_VERSION=7.2 PSR_HTTP_PROVIDER=nyholm SYMFONY_VERSION=3.4.* - - PHP_VERSION=7.2 PSR_HTTP_PROVIDER=nyholm SYMFONY_VERSION=4.2.* - - PHP_VERSION=7.2 PSR_HTTP_PROVIDER=nyholm SYMFONY_VERSION=4.3.* - - PHP_VERSION=7.2 PSR_HTTP_PROVIDER=zendframework SYMFONY_VERSION=3.4.* - - PHP_VERSION=7.2 PSR_HTTP_PROVIDER=zendframework SYMFONY_VERSION=4.2.* - - PHP_VERSION=7.2 PSR_HTTP_PROVIDER=zendframework SYMFONY_VERSION=4.3.* + - PHP_VERSION=7.2 PSR_HTTP_PROVIDER=nyholm SYMFONY_REQUIRE=4.4.* + - PHP_VERSION=7.2 PSR_HTTP_PROVIDER=zendframework SYMFONY_REQUIRE=4.4.* + - PHP_VERSION=7.2 PSR_HTTP_PROVIDER=nyholm SYMFONY_REQUIRE=5.0.* + - PHP_VERSION=7.2 PSR_HTTP_PROVIDER=zendframework SYMFONY_REQUIRE=5.0.* # PHP 7.3 - - PHP_VERSION=7.3 PSR_HTTP_PROVIDER=nyholm SYMFONY_VERSION=3.4.* - - PHP_VERSION=7.3 PSR_HTTP_PROVIDER=nyholm SYMFONY_VERSION=4.2.* - - PHP_VERSION=7.3 PSR_HTTP_PROVIDER=nyholm SYMFONY_VERSION=4.3.* - - PHP_VERSION=7.3 PSR_HTTP_PROVIDER=zendframework SYMFONY_VERSION=3.4.* - - PHP_VERSION=7.3 PSR_HTTP_PROVIDER=zendframework SYMFONY_VERSION=4.2.* - - PHP_VERSION=7.3 PSR_HTTP_PROVIDER=zendframework SYMFONY_VERSION=4.3.* + - PHP_VERSION=7.3 PSR_HTTP_PROVIDER=nyholm SYMFONY_REQUIRE=4.4.* + - PHP_VERSION=7.3 PSR_HTTP_PROVIDER=zendframework SYMFONY_REQUIRE=4.4.* + - PHP_VERSION=7.3 PSR_HTTP_PROVIDER=nyholm SYMFONY_REQUIRE=5.0.* + - PHP_VERSION=7.3 PSR_HTTP_PROVIDER=zendframework SYMFONY_REQUIRE=5.0.* + + # PHP 7.4 + - PHP_VERSION=7.4 PSR_HTTP_PROVIDER=nyholm SYMFONY_REQUIRE=4.4.* + - PHP_VERSION=7.4 PSR_HTTP_PROVIDER=zendframework SYMFONY_REQUIRE=4.4.* + - PHP_VERSION=7.4 PSR_HTTP_PROVIDER=nyholm SYMFONY_REQUIRE=5.0.* + - PHP_VERSION=7.4 PSR_HTTP_PROVIDER=zendframework SYMFONY_REQUIRE=5.0.* install: - dev/bin/docker-compose build --build-arg PHP_VERSION=${PHP_VERSION} php before_script: - # Our docker image has symfony/flex installed to make sure SYMFONY_VERSION is working - - dev/bin/php composer config extra.symfony.require "${SYMFONY_VERSION}" + # Our docker image has symfony/flex installed to make sure SYMFONY_REQUIRE is working - dev/bin/php composer update --ansi --prefer-dist script: - - dev/bin/php composer test -- --colors=always --coverage-clover=coverage.xml --debug + - dev/bin/php-test composer test -- --colors=always --coverage-clover=coverage.xml --debug - dev/bin/php composer lint -- --ansi --diff --dry-run --using-cache=no --verbose after_script: diff --git a/CHANGELOG.md b/CHANGELOG.md index 15a9a3f2..0484deff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,44 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). +## [3.0.0] - 2020-02-26 +### Added +- Ability to restrict clients from using the `plain` challenge method during PKCE ([4562a1f](https://github.com/trikoder/oauth2-bundle/commit/4562a1ff306375fd651aa91c85d0d4fd6f4c1b13)) +- Ability to clear expired authorization codes ([91b6447](https://github.com/trikoder/oauth2-bundle/commit/91b6447257419d8e961c4f5b0abd187f1b735856)) +- Support for defining public (non-confidential) clients ([8a71f55](https://github.com/trikoder/oauth2-bundle/commit/8a71f55aa1482d00cee66684141cc9ef81d31f31)) +- The bundle is now compatible with Symfony 5.x ([3f36977](https://github.com/trikoder/oauth2-bundle/commit/3f369771385c0b90855da712b9cb31faa4c651dc)) + +### Changed +- [PSR-7 Bridge](https://github.com/symfony/psr-http-message-bridge) version constraint to `^2.0` ([3c741ca](https://github.com/trikoder/oauth2-bundle/commit/3c741ca1e394886e8936ad018c28cd1ddd3dff02)) +- The bundle now relies on `8.x` versions of [league/oauth2-server](https://github.com/thephpleague/oauth2-server) for base functionality ([8becc18](https://github.com/trikoder/oauth2-bundle/commit/8becc18255052a73d0f76a030be9de0fe9868928)) + +### Removed +- Support for Symfony 3.4, 4.2 and 4.3 ([3f36977](https://github.com/trikoder/oauth2-bundle/commit/3f369771385c0b90855da712b9cb31faa4c651dc)) + +## [2.1.1] - 2020-02-25 +### Added +- The bundle is now additionally tested against PHP 7.4 ([2b29be3](https://github.com/trikoder/oauth2-bundle/commit/2b29be3629877a648f4a199b96185b40d625f6aa)) + +### Fixed +- Authentication provider not being aware of the current firewall context ([d349329](https://github.com/trikoder/oauth2-bundle/commit/d349329056c219969e097ae6bd3eb724968f9812)) +- Faulty logic when revoking authorization codes ([24ad882](https://github.com/trikoder/oauth2-bundle/commit/24ad88211cefddf97170f5c1cc8ba1e5cf285e42)) + +## [2.1.0] - 2019-12-09 +### Added +- Ability to change the scope role prefix using the `role_prefix` configuration option ([b2ee617](https://github.com/trikoder/oauth2-bundle/commit/b2ee6179832cc142d95e3b13d9af09d6cb6831d5)) +- Interfaces for converter type service classes ([d2caf69](https://github.com/trikoder/oauth2-bundle/commit/d2caf690839523a2c84d967a6f99787898d4c654)) +- New testing target in Travis CI for Symfony 4.4 ([8a44fd4](https://github.com/trikoder/oauth2-bundle/commit/8a44fd4d7673467cc4f69988424cdfc677767aab)) +- The bundle is now fully compatible with [Symfony Flex](https://github.com/symfony/flex) ([a4ccea1](https://github.com/trikoder/oauth2-bundle/commit/a4ccea1dfaaba6d95daf3e1f1a84952cafb65d01)) + +### Changed +- [DoctrineBundle](https://github.com/doctrine/DoctrineBundle) version constraint to allow `2.x` derived versions ([885e398](https://github.com/trikoder/oauth2-bundle/commit/885e39811331e89bae99bca71f1a783497d26d12)) +- Explicitly list [league/oauth2-server](https://github.com/thephpleague/oauth2-server) version requirements in the documentation ([9dce66a](https://github.com/trikoder/oauth2-bundle/commit/9dce66a089c33c224fe5cb58bdfd6285350a607b)) +- Reduce distributed package size by excluding files that are used only for development ([80b9e41](https://github.com/trikoder/oauth2-bundle/commit/80b9e41155e7a94c3b1a4602c8daa25cc6d246b2)) +- Simplify `AuthorizationRequestResolveEvent` class creation ([32908c1](https://github.com/trikoder/oauth2-bundle/commit/32908c1a4a89fd89d5835d4de931d237de223b50)) + +### Fixed +- Not being able to delete clients that have access/refresh tokens assigned to them ([424b770](https://github.com/trikoder/oauth2-bundle/commit/424b770dbd99e4651777a3fa26186a756b4e93c4)) + ## [2.0.1] - 2019-08-13 ### Removed - PSR-7/17 alias check during the container compile process ([0847ea3](https://github.com/trikoder/oauth2-bundle/commit/0847ea3034cc433c9c8f92ec46fedbdace259e3d)) diff --git a/Command/ClearExpiredTokensCommand.php b/Command/ClearExpiredTokensCommand.php index 6116699e..c01c4269 100644 --- a/Command/ClearExpiredTokensCommand.php +++ b/Command/ClearExpiredTokensCommand.php @@ -10,6 +10,7 @@ use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; use Trikoder\Bundle\OAuth2Bundle\Manager\AccessTokenManagerInterface; +use Trikoder\Bundle\OAuth2Bundle\Manager\AuthorizationCodeManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Manager\RefreshTokenManagerInterface; final class ClearExpiredTokensCommand extends Command @@ -26,31 +27,44 @@ final class ClearExpiredTokensCommand extends Command */ private $refreshTokenManager; + /** + * @var AuthorizationCodeManagerInterface + */ + private $authorizationCodeManager; + public function __construct( AccessTokenManagerInterface $accessTokenManager, - RefreshTokenManagerInterface $refreshTokenManager + RefreshTokenManagerInterface $refreshTokenManager, + AuthorizationCodeManagerInterface $authorizationCodeManager ) { parent::__construct(); $this->accessTokenManager = $accessTokenManager; $this->refreshTokenManager = $refreshTokenManager; + $this->authorizationCodeManager = $authorizationCodeManager; } protected function configure(): void { $this - ->setDescription('Clears all expired access and/or refresh tokens') + ->setDescription('Clears all expired access and/or refresh tokens and/or auth codes') ->addOption( - 'access-tokens-only', + 'access-tokens', 'a', InputOption::VALUE_NONE, - 'Clear only access tokens.' + 'Clear expired access tokens.' ) ->addOption( - 'refresh-tokens-only', + 'refresh-tokens', 'r', InputOption::VALUE_NONE, - 'Clear only refresh tokens.' + 'Clear expired refresh tokens.' + ) + ->addOption( + 'auth-codes', + 'c', + InputOption::VALUE_NONE, + 'Clear expired auth codes.' ) ; } @@ -59,33 +73,60 @@ protected function execute(InputInterface $input, OutputInterface $output): int { $io = new SymfonyStyle($input, $output); - $clearExpiredAccessTokens = !$input->getOption('refresh-tokens-only'); - $clearExpiredRefreshTokens = !$input->getOption('access-tokens-only'); + $clearExpiredAccessTokens = $input->getOption('access-tokens'); + $clearExpiredRefreshTokens = $input->getOption('refresh-tokens'); + $clearExpiredAuthCodes = $input->getOption('auth-codes'); - if (!$clearExpiredAccessTokens && !$clearExpiredRefreshTokens) { - $io->error('Please choose only one of the following options: "access-tokens-only", "refresh-tokens-only".'); + if (!$clearExpiredAccessTokens && !$clearExpiredRefreshTokens && !$clearExpiredAuthCodes) { + $this->clearExpiredAccessTokens($io); + $this->clearExpiredRefreshTokens($io); + $this->clearExpiredAuthCodes($io); - return 1; + return 0; } if (true === $clearExpiredAccessTokens) { - $numOfClearedAccessTokens = $this->accessTokenManager->clearExpired(); - $io->success(sprintf( - 'Cleared %d expired access token%s.', - $numOfClearedAccessTokens, - 1 === $numOfClearedAccessTokens ? '' : 's' - )); + $this->clearExpiredAccessTokens($io); } if (true === $clearExpiredRefreshTokens) { - $numOfClearedRefreshTokens = $this->refreshTokenManager->clearExpired(); - $io->success(sprintf( - 'Cleared %d expired refresh token%s.', - $numOfClearedRefreshTokens, - 1 === $numOfClearedRefreshTokens ? '' : 's' - )); + $this->clearExpiredRefreshTokens($io); + } + + if (true === $clearExpiredAuthCodes) { + $this->clearExpiredAuthCodes($io); } return 0; } + + private function clearExpiredAccessTokens(SymfonyStyle $io): void + { + $numOfClearedAccessTokens = $this->accessTokenManager->clearExpired(); + $io->success(sprintf( + 'Cleared %d expired access token%s.', + $numOfClearedAccessTokens, + 1 === $numOfClearedAccessTokens ? '' : 's' + )); + } + + private function clearExpiredRefreshTokens(SymfonyStyle $io): void + { + $numOfClearedRefreshTokens = $this->refreshTokenManager->clearExpired(); + $io->success(sprintf( + 'Cleared %d expired refresh token%s.', + $numOfClearedRefreshTokens, + 1 === $numOfClearedRefreshTokens ? '' : 's' + )); + } + + private function clearExpiredAuthCodes(SymfonyStyle $io): void + { + $numOfClearedAuthCodes = $this->authorizationCodeManager->clearExpired(); + $io->success(sprintf( + 'Cleared %d expired auth code%s.', + $numOfClearedAuthCodes, + 1 === $numOfClearedAuthCodes ? '' : 's' + )); + } } diff --git a/Command/CreateClientCommand.php b/Command/CreateClientCommand.php index ac8dbba9..8c5bd381 100644 --- a/Command/CreateClientCommand.php +++ b/Command/CreateClientCommand.php @@ -4,6 +4,7 @@ namespace Trikoder\Bundle\OAuth2Bundle\Command; +use InvalidArgumentException; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; @@ -67,13 +68,33 @@ protected function configure(): void InputArgument::OPTIONAL, 'The client secret' ) + ->addOption( + 'public', + null, + InputOption::VALUE_NONE, + 'Create a public client.' + ) + ->addOption( + 'allow-plain-text-pkce', + null, + InputOption::VALUE_NONE, + 'Create a client who is allowed to use plain challenge method for PKCE.' + ) ; } protected function execute(InputInterface $input, OutputInterface $output): int { $io = new SymfonyStyle($input, $output); - $client = $this->buildClientFromInput($input); + + try { + $client = $this->buildClientFromInput($input); + } catch (InvalidArgumentException $exception) { + $io->error($exception->getMessage()); + + return 1; + } + $this->clientManager->save($client); $io->success('New oAuth2 client created successfully.'); @@ -89,25 +110,33 @@ protected function execute(InputInterface $input, OutputInterface $output): int private function buildClientFromInput(InputInterface $input): Client { $identifier = $input->getArgument('identifier') ?? hash('md5', random_bytes(16)); - $secret = $input->getArgument('secret') ?? hash('sha512', random_bytes(32)); + + $isPublic = $input->getOption('public'); + + if (null !== $input->getArgument('secret') && $isPublic) { + throw new InvalidArgumentException('The client cannot have a secret and be public.'); + } + + $secret = $isPublic ? null : $input->getArgument('secret') ?? hash('sha512', random_bytes(32)); $client = new Client($identifier, $secret); $client->setActive(true); + $client->setAllowPlainTextPkce($input->getOption('allow-plain-text-pkce')); $redirectUris = array_map( - function (string $redirectUri): RedirectUri { return new RedirectUri($redirectUri); }, + static function (string $redirectUri): RedirectUri { return new RedirectUri($redirectUri); }, $input->getOption('redirect-uri') ); $client->setRedirectUris(...$redirectUris); $grants = array_map( - function (string $grant): Grant { return new Grant($grant); }, + static function (string $grant): Grant { return new Grant($grant); }, $input->getOption('grant-type') ); $client->setGrants(...$grants); $scopes = array_map( - function (string $scope): Scope { return new Scope($scope); }, + static function (string $scope): Scope { return new Scope($scope); }, $input->getOption('scope') ); $client->setScopes(...$scopes); diff --git a/Command/ListClientsCommand.php b/Command/ListClientsCommand.php index abfe1cbe..2cbcdc66 100644 --- a/Command/ListClientsCommand.php +++ b/Command/ListClientsCommand.php @@ -30,6 +30,7 @@ final class ListClientsCommand extends Command public function __construct(ClientManagerInterface $clientManager) { parent::__construct(); + $this->clientManager = $clientManager; } @@ -82,13 +83,13 @@ private function getFindByCriteria(InputInterface $input): ClientFilter return ClientFilter ::create() - ->addGrantCriteria(...array_map(function (string $grant): Grant { + ->addGrantCriteria(...array_map(static function (string $grant): Grant { return new Grant($grant); }, $input->getOption('grant-type'))) - ->addRedirectUriCriteria(...array_map(function (string $redirectUri): RedirectUri { + ->addRedirectUriCriteria(...array_map(static function (string $redirectUri): RedirectUri { return new RedirectUri($redirectUri); }, $input->getOption('redirect-uri'))) - ->addScopeCriteria(...array_map(function (string $scope): Scope { + ->addScopeCriteria(...array_map(static function (string $scope): Scope { return new Scope($scope); }, $input->getOption('scope'))) ; @@ -104,7 +105,7 @@ private function drawTable(InputInterface $input, OutputInterface $output, array private function getRows(array $clients, array $columns): array { - return array_map(function (Client $client) use ($columns): array { + return array_map(static function (Client $client) use ($columns): array { $values = [ 'identifier' => $client->getIdentifier(), 'secret' => $client->getSecret(), @@ -113,7 +114,7 @@ private function getRows(array $clients, array $columns): array 'grant type' => implode(', ', $client->getGrants()), ]; - return array_map(function (string $column) use ($values): string { + return array_map(static function (string $column) use ($values): string { return $values[$column]; }, $columns); }, $clients); @@ -122,7 +123,7 @@ private function getRows(array $clients, array $columns): array private function getColumns(InputInterface $input): array { $requestedColumns = $input->getOption('columns'); - $requestedColumns = array_map(function (string $column): string { + $requestedColumns = array_map(static function (string $column): string { return strtolower(trim($column)); }, $requestedColumns); diff --git a/Command/UpdateClientCommand.php b/Command/UpdateClientCommand.php index ccc9a72b..624c7be1 100644 --- a/Command/UpdateClientCommand.php +++ b/Command/UpdateClientCommand.php @@ -93,19 +93,19 @@ private function updateClientFromInput(Client $client, InputInterface $input): C $client->setActive(!$input->getOption('deactivated')); $redirectUris = array_map( - function (string $redirectUri): RedirectUri { return new RedirectUri($redirectUri); }, + static function (string $redirectUri): RedirectUri { return new RedirectUri($redirectUri); }, $input->getOption('redirect-uri') ); $client->setRedirectUris(...$redirectUris); $grants = array_map( - function (string $grant): Grant { return new Grant($grant); }, + static function (string $grant): Grant { return new Grant($grant); }, $input->getOption('grant-type') ); $client->setGrants(...$grants); $scopes = array_map( - function (string $scope): Scope { return new Scope($scope); }, + static function (string $scope): Scope { return new Scope($scope); }, $input->getOption('scope') ); $client->setScopes(...$scopes); diff --git a/Controller/AuthorizationController.php b/Controller/AuthorizationController.php index d809586f..68253057 100644 --- a/Controller/AuthorizationController.php +++ b/Controller/AuthorizationController.php @@ -9,10 +9,11 @@ use Psr\Http\Message\ResponseFactoryInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; -use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; +use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Trikoder\Bundle\OAuth2Bundle\Converter\UserConverterInterface; use Trikoder\Bundle\OAuth2Bundle\Event\AuthorizationRequestResolveEvent; use Trikoder\Bundle\OAuth2Bundle\Event\AuthorizationRequestResolveEventFactory; +use Trikoder\Bundle\OAuth2Bundle\Manager\ClientManagerInterface; use Trikoder\Bundle\OAuth2Bundle\OAuth2Events; final class AuthorizationController @@ -37,16 +38,23 @@ final class AuthorizationController */ private $userConverter; + /** + * @var ClientManagerInterface + */ + private $clientManager; + public function __construct( AuthorizationServer $server, EventDispatcherInterface $eventDispatcher, AuthorizationRequestResolveEventFactory $eventFactory, - UserConverterInterface $userConverter + UserConverterInterface $userConverter, + ClientManagerInterface $clientManager ) { $this->server = $server; $this->eventDispatcher = $eventDispatcher; $this->eventFactory = $eventFactory; $this->userConverter = $userConverter; + $this->clientManager = $clientManager; } public function indexAction(ServerRequestInterface $serverRequest, ResponseFactoryInterface $responseFactory): ResponseInterface @@ -56,6 +64,16 @@ public function indexAction(ServerRequestInterface $serverRequest, ResponseFacto try { $authRequest = $this->server->validateAuthorizationRequest($serverRequest); + if ('plain' === $authRequest->getCodeChallengeMethod()) { + $client = $this->clientManager->find($authRequest->getClient()->getIdentifier()); + if (!$client->isPlainTextPkceAllowed()) { + return OAuthServerException::invalidRequest( + 'code_challenge_method', + 'Plain code challenge method is not allowed for this client' + )->generateHttpResponse($serverResponse); + } + } + /** @var AuthorizationRequestResolveEvent $event */ $event = $this->eventDispatcher->dispatch( $this->eventFactory->fromAuthorizationRequest($authRequest), diff --git a/Controller/UserInfoController.php b/Controller/UserInfoController.php new file mode 100644 index 00000000..f4a79fed --- /dev/null +++ b/Controller/UserInfoController.php @@ -0,0 +1,62 @@ +server = $server; + $this->identityProvider = $identityProvider; + $this->claimExtractor = $claimExtractor; + } + + public function indexAction(ServerRequestInterface $serverRequest, ResponseFactoryInterface $responseFactory) + { + $request = $this->serverRequestWithBearerToken($serverRequest); + + try { + $validatedRequest = $this->server->validateAuthenticatedRequest($request); + } catch (OAuthServerException $e) { + return $e->generateHttpResponse($responseFactory->createResponse()); + } + + $userEntity = $this->identityProvider->getUserEntityByIdentifier($validatedRequest->getAttribute('oauth_user_id')); + $claims = $this->claimExtractor->extract($validatedRequest->getAttribute('oauth_scopes', []), $userEntity->getClaims()); + + return new JsonResponse(['sub' => $userEntity->getIdentifier()] + $claims); + } + + private function serverRequestWithBearerToken(ServerRequestInterface $serverRequest): ServerRequestInterface + { + if ($serverRequest->hasHeader('Authorization')) { + return $serverRequest; + } + + if ('POST' !== $serverRequest->getMethod()) { + return $serverRequest; + } + + if (!\is_array($serverRequest->getParsedBody())) { + return $serverRequest; + } + + if (!isset($serverRequest->getParsedBody()['access_token'])) { + return $serverRequest; + } + + return $serverRequest->withHeader('Authorization', sprintf('Bearer %s', (string) $serverRequest->getParsedBody()['access_token'])); + } +} diff --git a/DBAL/Type/ImplodedArray.php b/DBAL/Type/ImplodedArray.php index 725a2156..78548c41 100644 --- a/DBAL/Type/ImplodedArray.php +++ b/DBAL/Type/ImplodedArray.php @@ -82,9 +82,7 @@ private function assertValueCanBeImploded($value): void return; } - throw new InvalidArgumentException( - sprintf('The value of \'%s\' type cannot be imploded.', \gettype($value)) - ); + throw new InvalidArgumentException(sprintf('The value of \'%s\' type cannot be imploded.', \gettype($value))); } abstract protected function convertDatabaseValues(array $values): array; diff --git a/DependencyInjection/CompilerPass/EventDispatcherCompilerPass.php b/DependencyInjection/CompilerPass/EventDispatcherCompilerPass.php deleted file mode 100644 index 863cf7f6..00000000 --- a/DependencyInjection/CompilerPass/EventDispatcherCompilerPass.php +++ /dev/null @@ -1,38 +0,0 @@ -has(EventDispatcherInterface::class)) { - return; - } - - // Register a new service - $definition = new Definition(BCEventDispatcher::class); - $definition->addArgument(new Reference(\Symfony\Component\EventDispatcher\EventDispatcherInterface::class)); - $container->setDefinition(BCEventDispatcher::class, $definition); - - // Use our new service - $container->getDefinition(ScopeRepository::class) - ->replaceArgument(3, new Reference(BCEventDispatcher::class)); - $container->getDefinition(UserRepository::class) - ->replaceArgument(1, new Reference(BCEventDispatcher::class)); - $container->getDefinition(AuthorizationController::class) - ->replaceArgument(1, new Reference(BCEventDispatcher::class)); - } -} diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index 111b6c79..9c24ff03 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -8,21 +8,23 @@ use Symfony\Component\Config\Definition\Builder\NodeDefinition; use Symfony\Component\Config\Definition\Builder\TreeBuilder; use Symfony\Component\Config\Definition\ConfigurationInterface; +use Trikoder\Bundle\OAuth2Bundle\Model\AuthorizationDecision\UserConsentDecisionStrategy; final class Configuration implements ConfigurationInterface { /** * {@inheritdoc} */ - public function getConfigTreeBuilder() + public function getConfigTreeBuilder(): TreeBuilder { - $treeBuilder = $this->getWrappedTreeBuilder('trikoder_oauth2'); + $treeBuilder = new TreeBuilder('trikoder_oauth2'); $rootNode = $treeBuilder->getRootNode(); $rootNode->append($this->createAuthorizationServerNode()); $rootNode->append($this->createResourceServerNode()); $rootNode->append($this->createScopesNode()); $rootNode->append($this->createPersistenceNode()); + $rootNode->append($this->createOpenIDConnectNode()); $rootNode ->children() @@ -30,6 +32,11 @@ public function getConfigTreeBuilder() ->info('The priority of the event listener that converts an Exception to a Response') ->defaultValue(10) ->end() + ->scalarNode('role_prefix') + ->info('Set a custom prefix that replaces the default \'ROLE_OAUTH2_\' role prefix') + ->defaultValue('ROLE_OAUTH2_') + ->cannotBeEmpty() + ->end() ->end(); return $treeBuilder; @@ -37,7 +44,7 @@ public function getConfigTreeBuilder() private function createAuthorizationServerNode(): NodeDefinition { - $treeBuilder = $this->getWrappedTreeBuilder('authorization_server'); + $treeBuilder = new TreeBuilder('authorization_server'); $node = $treeBuilder->getRootNode(); $node @@ -94,10 +101,24 @@ private function createAuthorizationServerNode(): NodeDefinition ->info('Whether to enable the authorization code grant') ->defaultTrue() ->end() + ->booleanNode('require_code_challenge_for_public_clients') + ->info('Whether to require code challenge for public clients for the auth code grant') + ->defaultTrue() + ->end() ->booleanNode('enable_implicit_grant') ->info('Whether to enable the implicit grant') ->defaultTrue() ->end() + ->scalarNode('authorization_strategy') + ->isRequired() + ->info("What strategy should be used to authorize user.\nService must implement AuthorizationDecisionStrategy interface") + ->defaultValue(UserConsentDecisionStrategy::class) + ->end() + ->scalarNode('consent_route') + ->isRequired() + ->info('The route to redirect the user to when the user consent is required for authorization') + ->defaultValue('oauth2_consent') + ->end() ->end() ; @@ -106,7 +127,7 @@ private function createAuthorizationServerNode(): NodeDefinition private function createResourceServerNode(): NodeDefinition { - $treeBuilder = $this->getWrappedTreeBuilder('resource_server'); + $treeBuilder = new TreeBuilder('resource_server'); $node = $treeBuilder->getRootNode(); $node @@ -126,7 +147,7 @@ private function createResourceServerNode(): NodeDefinition private function createScopesNode(): NodeDefinition { - $treeBuilder = $this->getWrappedTreeBuilder('scopes'); + $treeBuilder = new TreeBuilder('scopes'); $node = $treeBuilder->getRootNode(); $node @@ -140,7 +161,7 @@ private function createScopesNode(): NodeDefinition private function createPersistenceNode(): NodeDefinition { - $treeBuilder = $this->getWrappedTreeBuilder('persistence'); + $treeBuilder = new TreeBuilder('persistence'); $node = $treeBuilder->getRootNode(); $node @@ -167,26 +188,27 @@ private function createPersistenceNode(): NodeDefinition return $node; } - private function getWrappedTreeBuilder(string $name): object + private function createOpenIDConnectNode(): NodeDefinition { - return new class($name) extends TreeBuilder { - public function __construct(string $name) - { - // Compatibility path for Symfony 3.4 - if (!method_exists(TreeBuilder::class, 'getRootNode')) { - $this->root($name); - } - - // Compatibility path for Symfony 4.2+ - if (method_exists(TreeBuilder::class, '__construct')) { - parent::__construct($name); - } - } - - public function getRootNode(): NodeDefinition - { - return $this->root; - } - }; + $treeBuilder = new TreeBuilder('openid_connect'); + $node = $treeBuilder->getRootNode(); + + $node + ->info('Adds OpenID Connect Provider capabilities.') + ->treatFalseLike(['enabled' => false]) + ->treatTrueLike(['enabled' => true]) + ->treatNullLike(['enabled' => false]) + ->addDefaultsIfNotSet() + ->children() + ->booleanNode('enabled')->defaultNull()->end() + ->scalarNode('login_route') + ->isRequired() + ->info('Login route to redirect to unauthenticated users') + ->defaultValue('app_login') + ->end() + ->end() + ; + + return $node; } } diff --git a/DependencyInjection/Security/OAuth2Factory.php b/DependencyInjection/Security/OAuth2Factory.php index dc4404bb..30c2fa77 100644 --- a/DependencyInjection/Security/OAuth2Factory.php +++ b/DependencyInjection/Security/OAuth2Factory.php @@ -23,11 +23,13 @@ public function create(ContainerBuilder $container, $id, $config, $userProvider, $providerId = 'security.authentication.provider.oauth2.' . $id; $container ->setDefinition($providerId, new ChildDefinition(OAuth2Provider::class)) - ->replaceArgument('$userProvider', new Reference($userProvider)); + ->replaceArgument('$userProvider', new Reference($userProvider)) + ->replaceArgument('$providerKey', $id); $listenerId = 'security.authentication.listener.oauth2.' . $id; $container - ->setDefinition($listenerId, new ChildDefinition(OAuth2Listener::class)); + ->setDefinition($listenerId, new ChildDefinition(OAuth2Listener::class)) + ->replaceArgument('$providerKey', $id); return [$providerId, $listenerId, OAuth2EntryPoint::class]; } diff --git a/DependencyInjection/TrikoderOAuth2Extension.php b/DependencyInjection/TrikoderOAuth2Extension.php index f46dafe3..25ee1221 100644 --- a/DependencyInjection/TrikoderOAuth2Extension.php +++ b/DependencyInjection/TrikoderOAuth2Extension.php @@ -7,6 +7,7 @@ use DateInterval; use Defuse\Crypto\Key; use Doctrine\Bundle\DoctrineBundle\DoctrineBundle; +use Exception; use League\OAuth2\Server\AuthorizationServer; use League\OAuth2\Server\CryptKey; use League\OAuth2\Server\Grant\AuthCodeGrant; @@ -16,6 +17,7 @@ use League\OAuth2\Server\Grant\RefreshTokenGrant; use League\OAuth2\Server\ResourceServer; use LogicException; +use RuntimeException; use Sensio\Bundle\FrameworkExtraBundle\SensioFrameworkExtraBundle; use Symfony\Bundle\SecurityBundle\SecurityBundle; use Symfony\Component\Config\FileLocator; @@ -31,6 +33,8 @@ use Trikoder\Bundle\OAuth2Bundle\DBAL\Type\Grant as GrantType; use Trikoder\Bundle\OAuth2Bundle\DBAL\Type\RedirectUri as RedirectUriType; use Trikoder\Bundle\OAuth2Bundle\DBAL\Type\Scope as ScopeType; +use Trikoder\Bundle\OAuth2Bundle\EventListener\AuthorizationRequestAuthenticationResolvingListener; +use Trikoder\Bundle\OAuth2Bundle\EventListener\AuthorizationRequestDecisionResolvingListener; use Trikoder\Bundle\OAuth2Bundle\EventListener\ConvertExceptionToResponseListener; use Trikoder\Bundle\OAuth2Bundle\Manager\Doctrine\AccessTokenManager; use Trikoder\Bundle\OAuth2Bundle\Manager\Doctrine\AuthorizationCodeManager; @@ -38,11 +42,15 @@ use Trikoder\Bundle\OAuth2Bundle\Manager\Doctrine\RefreshTokenManager; use Trikoder\Bundle\OAuth2Bundle\Manager\ScopeManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Model\Scope as ScopeModel; +use Trikoder\Bundle\OAuth2Bundle\Security\Authentication\Token\OAuth2TokenFactory; +use Trikoder\Bundle\OAuth2Bundle\OpenIDConnect\IdTokenResponse; final class TrikoderOAuth2Extension extends Extension implements PrependExtensionInterface, CompilerPassInterface { /** * {@inheritdoc} + * + * @throws Exception */ public function load(array $configs, ContainerBuilder $container) { @@ -55,6 +63,10 @@ public function load(array $configs, ContainerBuilder $container) $this->configureAuthorizationServer($container, $config['authorization_server']); $this->configureResourceServer($container, $config['resource_server']); $this->configureScopes($container, $config['scopes']); + $this->configureOpenIDConnect($container, $config['openid_connect']); + + $container->getDefinition(OAuth2TokenFactory::class) + ->setArgument(0, $config['role_prefix']); $container->getDefinition(ConvertExceptionToResponseListener::class) ->addTag('kernel.event_listener', [ @@ -107,12 +119,7 @@ private function assertRequiredBundlesAreEnabled(ContainerBuilder $container): v foreach ($requiredBundles as $bundleAlias => $requiredBundle) { if (!$container->hasExtension($bundleAlias)) { - throw new LogicException( - sprintf( - 'Bundle \'%s\' needs to be enabled in your application kernel.', - $requiredBundle - ) - ); + throw new LogicException(sprintf('Bundle \'%s\' needs to be enabled in your application kernel.', $requiredBundle)); } } } @@ -131,7 +138,7 @@ private function configureAuthorizationServer(ContainerBuilder $container, array $authorizationServer->replaceArgument('$encryptionKey', $config['encryption_key']); } elseif ('defuse' === $config['encryption_key_type']) { if (!class_exists(Key::class)) { - throw new \RuntimeException('You must install the "defuse/php-encryption" package to use "encryption_key_type: defuse".'); + throw new RuntimeException('You must install the "defuse/php-encryption" package to use "encryption_key_type: defuse".'); } $keyDefinition = (new Definition(Key::class)) @@ -178,6 +185,7 @@ private function configureAuthorizationServer(ContainerBuilder $container, array } $this->configureGrants($container, $config); + $this->configureAuthorizationStrategy($container, $config['authorization_strategy'], $config['consent_route']); } private function configureGrants(ContainerBuilder $container, array $config): void @@ -196,20 +204,26 @@ private function configureGrants(ContainerBuilder $container, array $config): vo ]) ; - $container - ->getDefinition(AuthCodeGrant::class) - ->replaceArgument('$authCodeTTL', new Definition(DateInterval::class, [$config['auth_code_ttl']])) + $authCodeGrantDefinition = $container->getDefinition(AuthCodeGrant::class); + $authCodeGrantDefinition->replaceArgument('$authCodeTTL', new Definition(DateInterval::class, [$config['auth_code_ttl']])) ->addMethodCall('setRefreshTokenTTL', [ new Definition(DateInterval::class, [$config['refresh_token_ttl']]), ]) ; + if (false === $config['require_code_challenge_for_public_clients']) { + $authCodeGrantDefinition->addMethodCall('disableRequireCodeChallengeForPublicClients'); + } + $container ->getDefinition(ImplicitGrant::class) ->replaceArgument('$accessTokenTTL', new Definition(DateInterval::class, [$config['access_token_ttl']])) ; } + /** + * @throws Exception + */ private function configurePersistence(LoaderInterface $loader, ContainerBuilder $container, array $config): void { if (\count($config) > 1) { @@ -284,7 +298,7 @@ private function configureScopes(ContainerBuilder $container, array $scopes): vo { $scopeManager = $container ->getDefinition( - $container->getAlias(ScopeManagerInterface::class) + (string) $container->getAlias(ScopeManagerInterface::class) ) ; @@ -294,4 +308,26 @@ private function configureScopes(ContainerBuilder $container, array $scopes): vo ]); } } + + private function configureAuthorizationStrategy(ContainerBuilder $container, string $authorizationStrategy, string $consentRoute) + { + $container->getDefinition('trikoder.oauth2.authorization_decision_strategy.user_consent')->replaceArgument(3, $consentRoute); + $container + ->getDefinition(AuthorizationRequestDecisionResolvingListener::class) + ->replaceArgument(0, new Reference($authorizationStrategy)); + } + + private function configureOpenIDConnect(ContainerBuilder $container, array $openid_connect): void + { + if (isset($openid_connect['enabled']) && $openid_connect['enabled']) { + $container + ->getDefinition(AuthorizationServer::class) + ->setArgument(5, new Reference(IdTokenResponse::class)) + ; + $container + ->getDefinition(AuthorizationRequestAuthenticationResolvingListener::class) + ->setArgument(5, $openid_connect['login_route']) + ; + } + } } diff --git a/Event/AuthorizationRequestResolveEvent.php b/Event/AuthorizationRequestResolveEvent.php index 06189f81..640ee6d3 100644 --- a/Event/AuthorizationRequestResolveEvent.php +++ b/Event/AuthorizationRequestResolveEvent.php @@ -7,11 +7,8 @@ use League\OAuth2\Server\RequestTypes\AuthorizationRequest; use LogicException; use Psr\Http\Message\ResponseInterface; -use RuntimeException; -use Symfony\Component\EventDispatcher\Event; use Symfony\Component\Security\Core\User\UserInterface; -use Trikoder\Bundle\OAuth2Bundle\Converter\ScopeConverterInterface; -use Trikoder\Bundle\OAuth2Bundle\Manager\ClientManagerInterface; +use Symfony\Contracts\EventDispatcher\Event; use Trikoder\Bundle\OAuth2Bundle\Model\Client; use Trikoder\Bundle\OAuth2Bundle\Model\Scope; @@ -26,14 +23,14 @@ final class AuthorizationRequestResolveEvent extends Event private $authorizationRequest; /** - * @var ScopeConverterInterface + * @var Scope[] */ - private $scopeConverter; + private $scopes; /** - * @var ClientManagerInterface + * @var Client */ - private $clientManager; + private $client; /** * @var bool @@ -50,11 +47,14 @@ final class AuthorizationRequestResolveEvent extends Event */ private $user; - public function __construct(AuthorizationRequest $authorizationRequest, ScopeConverterInterface $scopeConverter, ClientManagerInterface $clientManager) + /** + * @param Scope[] $scopes + */ + public function __construct(AuthorizationRequest $authorizationRequest, array $scopes, Client $client) { $this->authorizationRequest = $authorizationRequest; - $this->scopeConverter = $scopeConverter; - $this->clientManager = $clientManager; + $this->scopes = $scopes; + $this->client = $client; } public function getAuthorizationResolution(): bool @@ -100,14 +100,7 @@ public function getGrantTypeId(): string public function getClient(): Client { - $identifier = $this->authorizationRequest->getClient()->getIdentifier(); - $client = $this->clientManager->find($identifier); - - if (null === $client) { - throw new RuntimeException(sprintf('No client found for the given identifier "%s".', $identifier)); - } - - return $client; + return $this->client; } public function getUser(): ?UserInterface @@ -127,9 +120,7 @@ public function setUser(?UserInterface $user): self */ public function getScopes(): array { - return $this->scopeConverter->toDomainArray( - $this->authorizationRequest->getScopes() - ); + return $this->scopes; } public function isAuthorizationApproved(): bool diff --git a/Event/AuthorizationRequestResolveEventFactory.php b/Event/AuthorizationRequestResolveEventFactory.php index 317bbf97..f213d4a1 100644 --- a/Event/AuthorizationRequestResolveEventFactory.php +++ b/Event/AuthorizationRequestResolveEventFactory.php @@ -5,6 +5,7 @@ namespace Trikoder\Bundle\OAuth2Bundle\Event; use League\OAuth2\Server\RequestTypes\AuthorizationRequest; +use RuntimeException; use Trikoder\Bundle\OAuth2Bundle\Converter\ScopeConverterInterface; use Trikoder\Bundle\OAuth2Bundle\Manager\ClientManagerInterface; @@ -28,6 +29,14 @@ public function __construct(ScopeConverterInterface $scopeConverter, ClientManag public function fromAuthorizationRequest(AuthorizationRequest $authorizationRequest): AuthorizationRequestResolveEvent { - return new AuthorizationRequestResolveEvent($authorizationRequest, $this->scopeConverter, $this->clientManager); + $scopes = $this->scopeConverter->toDomainArray($authorizationRequest->getScopes()); + + $client = $this->clientManager->find($authorizationRequest->getClient()->getIdentifier()); + + if (null === $client) { + throw new RuntimeException(sprintf('No client found for the given identifier \'%s\'.', $authorizationRequest->getClient()->getIdentifier())); + } + + return new AuthorizationRequestResolveEvent($authorizationRequest, $scopes, $client); } } diff --git a/Event/ClaimsResolveEvent.php b/Event/ClaimsResolveEvent.php new file mode 100644 index 00000000..37c2625b --- /dev/null +++ b/Event/ClaimsResolveEvent.php @@ -0,0 +1,43 @@ +identifier = $identifier; + } + + public function getIdentifier(): string + { + return $this->identifier; + } + + public function getClaims(): array + { + return $this->claims; + } + + /** + * @param string[] $claims + */ + public function setClaims(array $claims): self + { + $this->claims = $claims; + + return $this; + } +} diff --git a/Event/ScopeResolveEvent.php b/Event/ScopeResolveEvent.php index 4ff2c640..6f45c0a6 100644 --- a/Event/ScopeResolveEvent.php +++ b/Event/ScopeResolveEvent.php @@ -4,7 +4,7 @@ namespace Trikoder\Bundle\OAuth2Bundle\Event; -use Symfony\Component\EventDispatcher\Event; +use Symfony\Contracts\EventDispatcher\Event; use Trikoder\Bundle\OAuth2Bundle\Model\Client; use Trikoder\Bundle\OAuth2Bundle\Model\Grant; use Trikoder\Bundle\OAuth2Bundle\Model\Scope; diff --git a/Event/UserResolveEvent.php b/Event/UserResolveEvent.php index 5526cfbe..b9a3db07 100644 --- a/Event/UserResolveEvent.php +++ b/Event/UserResolveEvent.php @@ -4,8 +4,8 @@ namespace Trikoder\Bundle\OAuth2Bundle\Event; -use Symfony\Component\EventDispatcher\Event; use Symfony\Component\Security\Core\User\UserInterface; +use Symfony\Contracts\EventDispatcher\Event; use Trikoder\Bundle\OAuth2Bundle\Model\Client; use Trikoder\Bundle\OAuth2Bundle\Model\Grant; diff --git a/EventListener/AuthorizationRequestAuthenticationResolvingListener.php b/EventListener/AuthorizationRequestAuthenticationResolvingListener.php new file mode 100644 index 00000000..c74b8ae8 --- /dev/null +++ b/EventListener/AuthorizationRequestAuthenticationResolvingListener.php @@ -0,0 +1,88 @@ +authorizationChecker = $authorizationChecker; + $this->session = $session; + $this->requestStack = $requestStack; + $this->urlGenerator = $urlGenerator; + $this->firewallMap = $firewallMap; + $this->loginRoute = $loginRoute; + } + + public function onAuthorizationRequest(AuthorizationRequestResolveEvent $event): void + { + if (null === $request = $this->requestStack->getMasterRequest()) { + throw new \RuntimeException('Authentication listener depends on the request context'); + } + + if (!$this->authorizationChecker->isGranted('IS_AUTHENTICATED_REMEMBERED')) { + $firewallConfig = $this->firewallMap->getFirewallConfig($request); + $this->saveTargetPath($this->session, $firewallConfig->getProvider(), $request->getUri()); + $this->setResponse($event); + } + } + + protected function setResponse(AuthorizationRequestResolveEvent $event): void + { + $loginUrl = $this->urlGenerator->generate($this->loginRoute); + $event->setResponse(new RedirectResponse($loginUrl)); + } +} diff --git a/EventListener/AuthorizationRequestDecisionResolvingListener.php b/EventListener/AuthorizationRequestDecisionResolvingListener.php new file mode 100644 index 00000000..11545bd4 --- /dev/null +++ b/EventListener/AuthorizationRequestDecisionResolvingListener.php @@ -0,0 +1,39 @@ +authorizationDecisionStrategy = $authorizationDecisionStrategy; + } + + public function onAuthorizationRequest(AuthorizationRequestResolveEvent $event): void + { + // if request is already approved by other listener, there is nothing left to do + if ($event->isAuthorizationApproved()) { + return; + } + + // delegate to configured authorization strategy + $this->authorizationDecisionStrategy->decide($event); + } +} diff --git a/EventListener/ConvertExceptionToResponseListener.php b/EventListener/ConvertExceptionToResponseListener.php index a3fcaeaf..b29fdffe 100644 --- a/EventListener/ConvertExceptionToResponseListener.php +++ b/EventListener/ConvertExceptionToResponseListener.php @@ -5,7 +5,7 @@ namespace Trikoder\Bundle\OAuth2Bundle\EventListener; use Symfony\Component\HttpFoundation\Response; -use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; +use Symfony\Component\HttpKernel\Event\ExceptionEvent; use Trikoder\Bundle\OAuth2Bundle\Security\Exception\InsufficientScopesException; use Trikoder\Bundle\OAuth2Bundle\Security\Exception\Oauth2AuthenticationFailedException; @@ -14,9 +14,9 @@ */ final class ConvertExceptionToResponseListener { - public function onKernelException(GetResponseForExceptionEvent $event): void + public function onKernelException(ExceptionEvent $event): void { - $exception = $event->getException(); + $exception = $event->getThrowable(); if ($exception instanceof InsufficientScopesException || $exception instanceof Oauth2AuthenticationFailedException) { $event->setResponse(new Response($exception->getMessage(), $exception->getCode())); } diff --git a/Grant/AuthCodeGrant.php b/Grant/AuthCodeGrant.php new file mode 100644 index 00000000..b56b9ed4 --- /dev/null +++ b/Grant/AuthCodeGrant.php @@ -0,0 +1,55 @@ +nonce = $this->getQueryStringParameter('nonce', $request, null); + + return $authorizationRequest; + } + + protected function issueAuthCode(DateInterval $authCodeTTL, ClientEntityInterface $client, $userIdentifier, $redirectUri, array $scopes = []) + { + $autCode = parent::issueAuthCode($authCodeTTL, $client, $userIdentifier, $redirectUri, $scopes); + + if ($this->nonce !== null) { + $this->authCodeRepository->updateWithNonce($autCode, $this->nonce); + } + + return $autCode; + } + + public function respondToAccessTokenRequest(ServerRequestInterface $request, ResponseTypeInterface $responseType, DateInterval $accessTokenTTL) + { + $response = parent::respondToAccessTokenRequest($request, $responseType, $accessTokenTTL); + + if ($response instanceof IdTokenResponse) { + $encryptedAuthCode = $this->getRequestParameter('code', $request, null); + $authCodePayload = json_decode($this->decrypt($encryptedAuthCode)); + + $nonce = $this->authCodeRepository->getNonce($authCodePayload->auth_code_id); + $response->setNonce($nonce); + } + + return $response; + } +} diff --git a/League/Entity/Client.php b/League/Entity/Client.php index 4c3d2091..bff49c73 100644 --- a/League/Entity/Client.php +++ b/League/Entity/Client.php @@ -13,6 +13,11 @@ final class Client implements ClientEntityInterface use EntityTrait; use ClientTrait; + /** + * @var bool + */ + private $allowPlainTextPkce = false; + /** * {@inheritdoc} */ @@ -28,4 +33,19 @@ public function setRedirectUri(array $redirectUri): void { $this->redirectUri = $redirectUri; } + + public function setConfidential(bool $isConfidential): void + { + $this->isConfidential = $isConfidential; + } + + public function isPlainTextPkceAllowed(): bool + { + return $this->allowPlainTextPkce; + } + + public function setAllowPlainTextPkce(bool $allowPlainTextPkce): void + { + $this->allowPlainTextPkce = $allowPlainTextPkce; + } } diff --git a/League/Repository/AccessTokenRepository.php b/League/Repository/AccessTokenRepository.php index c293aa75..4ff70cd0 100644 --- a/League/Repository/AccessTokenRepository.php +++ b/League/Repository/AccessTokenRepository.php @@ -46,7 +46,15 @@ public function __construct( */ public function getNewToken(ClientEntityInterface $clientEntity, array $scopes, $userIdentifier = null) { - return new AccessTokenEntity(); + $accessToken = new AccessTokenEntity(); + $accessToken->setClient($clientEntity); + $accessToken->setUserIdentifier($userIdentifier); + + foreach ($scopes as $scope) { + $accessToken->addScope($scope); + } + + return $accessToken; } /** @@ -99,14 +107,12 @@ private function buildAccessTokenModel(AccessTokenEntityInterface $accessTokenEn { $client = $this->clientManager->find($accessTokenEntity->getClient()->getIdentifier()); - $accessToken = new AccessTokenModel( + return new AccessTokenModel( $accessTokenEntity->getIdentifier(), $accessTokenEntity->getExpiryDateTime(), $client, $accessTokenEntity->getUserIdentifier(), $this->scopeConverter->toDomainArray($accessTokenEntity->getScopes()) ); - - return $accessToken; } } diff --git a/League/Repository/AuthCodeRepository.php b/League/Repository/AuthCodeRepository.php index 4b9638be..c1ce8e29 100644 --- a/League/Repository/AuthCodeRepository.php +++ b/League/Repository/AuthCodeRepository.php @@ -64,6 +64,26 @@ public function persistNewAuthCode(AuthCodeEntityInterface $authCode) $this->authorizationCodeManager->save($authorizationCode); } + public function updateWithNonce(AuthCodeEntityInterface $authCode, string $nonce) + { + /** @var AuthorizationCode $authorizationCode */ + $authorizationCode = $this->authorizationCodeManager->find($authCode->getIdentifier()); + + if (null === $authorizationCode) { + throw new \LogicException('You cant update code that wasnt\'t persisted'); + } + + $authorizationCode->setNonce($nonce); + + $this->authorizationCodeManager->save($authorizationCode); + } + + public function getNonce(string $authCodeIdentifier) + { + $authCode = $this->authorizationCodeManager->find($authCodeIdentifier); + return $authCode->getNonce(); + } + /** * {@inheritdoc} */ @@ -71,7 +91,7 @@ public function revokeAuthCode($codeId) { $authorizationCode = $this->authorizationCodeManager->find($codeId); - if (null === $codeId) { + if (null === $authorizationCode) { return; } @@ -98,14 +118,12 @@ private function buildAuthorizationCode(AuthCode $authCode): AuthorizationCode { $client = $this->clientManager->find($authCode->getClient()->getIdentifier()); - $authorizationCode = new AuthorizationCode( + return new AuthorizationCode( $authCode->getIdentifier(), $authCode->getExpiryDateTime(), $client, $authCode->getUserIdentifier(), $this->scopeConverter->toDomainArray($authCode->getScopes()) ); - - return $authorizationCode; } } diff --git a/League/Repository/ClientRepository.php b/League/Repository/ClientRepository.php index 58ae6e55..2707ba3e 100644 --- a/League/Repository/ClientRepository.php +++ b/League/Repository/ClientRepository.php @@ -24,35 +24,41 @@ public function __construct(ClientManagerInterface $clientManager) /** * {@inheritdoc} */ - public function getClientEntity( - $clientIdentifier, - $grantType = null, - $clientSecret = null, - $mustValidateSecret = true - ) { + public function getClientEntity($clientIdentifier) + { $client = $this->clientManager->find($clientIdentifier); if (null === $client) { return null; } - if (!$client->isActive()) { - return null; + return $this->buildClientEntity($client); + } + + /** + * {@inheritdoc} + */ + public function validateClient($clientIdentifier, $clientSecret, $grantType) + { + $client = $this->clientManager->find($clientIdentifier); + + if (null === $client) { + return false; } - if (!$this->isGrantSupported($client, $grantType)) { - return null; + if (!$client->isActive()) { + return false; } - if (!$mustValidateSecret) { - return $this->buildClientEntity($client); + if (!$this->isGrantSupported($client, $grantType)) { + return false; } - if (!hash_equals($client->getSecret(), (string) $clientSecret)) { - return null; + if (!$client->isConfidential() || hash_equals($client->getSecret(), (string) $clientSecret)) { + return true; } - return $this->buildClientEntity($client); + return false; } private function buildClientEntity(ClientModel $client): ClientEntity @@ -60,6 +66,8 @@ private function buildClientEntity(ClientModel $client): ClientEntity $clientEntity = new ClientEntity(); $clientEntity->setIdentifier($client->getIdentifier()); $clientEntity->setRedirectUri(array_map('strval', $client->getRedirectUris())); + $clientEntity->setConfidential($client->isConfidential()); + $clientEntity->setAllowPlainTextPkce($client->isPlainTextPkceAllowed()); return $clientEntity; } diff --git a/League/Repository/RefreshTokenRepository.php b/League/Repository/RefreshTokenRepository.php index fc62dd1c..92b5ade7 100644 --- a/League/Repository/RefreshTokenRepository.php +++ b/League/Repository/RefreshTokenRepository.php @@ -90,12 +90,10 @@ private function buildRefreshTokenModel(RefreshTokenEntityInterface $refreshToke { $accessToken = $this->accessTokenManager->find($refreshTokenEntity->getAccessToken()->getIdentifier()); - $refreshToken = new RefreshTokenModel( + return new RefreshTokenModel( $refreshTokenEntity->getIdentifier(), $refreshTokenEntity->getExpiryDateTime(), $accessToken ); - - return $refreshToken; } } diff --git a/League/Repository/ScopeRepository.php b/League/Repository/ScopeRepository.php index 94c9cb62..f8936430 100644 --- a/League/Repository/ScopeRepository.php +++ b/League/Repository/ScopeRepository.php @@ -7,7 +7,7 @@ use League\OAuth2\Server\Entities\ClientEntityInterface; use League\OAuth2\Server\Exception\OAuthServerException; use League\OAuth2\Server\Repositories\ScopeRepositoryInterface; -use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; +use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Trikoder\Bundle\OAuth2Bundle\Converter\ScopeConverterInterface; use Trikoder\Bundle\OAuth2Bundle\Event\ScopeResolveEvent; use Trikoder\Bundle\OAuth2Bundle\Manager\ClientManagerInterface; @@ -79,7 +79,12 @@ public function finalizeScopes( $scopes = $this->setupScopes($client, $this->scopeConverter->toDomainArray($scopes)); $event = $this->eventDispatcher->dispatch( - new ScopeResolveEvent($scopes, new GrantModel($grantType), $client, $userIdentifier), + new ScopeResolveEvent( + $scopes, + new GrantModel($grantType), + $client, + $userIdentifier + ), OAuth2Events::SCOPE_RESOLVE ); diff --git a/League/Repository/UserRepository.php b/League/Repository/UserRepository.php index e765fdf2..bdacb26c 100644 --- a/League/Repository/UserRepository.php +++ b/League/Repository/UserRepository.php @@ -6,7 +6,7 @@ use League\OAuth2\Server\Entities\ClientEntityInterface; use League\OAuth2\Server\Repositories\UserRepositoryInterface; -use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; +use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Trikoder\Bundle\OAuth2Bundle\Converter\UserConverterInterface; use Trikoder\Bundle\OAuth2Bundle\Event\UserResolveEvent; use Trikoder\Bundle\OAuth2Bundle\Manager\ClientManagerInterface; @@ -52,7 +52,12 @@ public function getUserEntityByUserCredentials( $client = $this->clientManager->find($clientEntity->getIdentifier()); $event = $this->eventDispatcher->dispatch( - new UserResolveEvent($username, $password, new GrantModel($grantType), $client), + new UserResolveEvent( + $username, + $password, + new GrantModel($grantType), + $client + ), OAuth2Events::USER_RESOLVE ); diff --git a/Manager/AuthorizationCodeManagerInterface.php b/Manager/AuthorizationCodeManagerInterface.php index 4775c9d9..9454d595 100644 --- a/Manager/AuthorizationCodeManagerInterface.php +++ b/Manager/AuthorizationCodeManagerInterface.php @@ -11,4 +11,6 @@ interface AuthorizationCodeManagerInterface public function find(string $identifier): ?AuthorizationCode; public function save(AuthorizationCode $authCode): void; + + public function clearExpired(): int; } diff --git a/Manager/Doctrine/AccessTokenManager.php b/Manager/Doctrine/AccessTokenManager.php index 1192266e..41921af8 100644 --- a/Manager/Doctrine/AccessTokenManager.php +++ b/Manager/Doctrine/AccessTokenManager.php @@ -4,7 +4,7 @@ namespace Trikoder\Bundle\OAuth2Bundle\Manager\Doctrine; -use DateTime; +use DateTimeImmutable; use Doctrine\ORM\EntityManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Manager\AccessTokenManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Model\AccessToken; @@ -43,7 +43,7 @@ public function clearExpired(): int return $this->entityManager->createQueryBuilder() ->delete(AccessToken::class, 'at') ->where('at.expiry < :expiry') - ->setParameter('expiry', new DateTime()) + ->setParameter('expiry', new DateTimeImmutable()) ->getQuery() ->execute(); } diff --git a/Manager/Doctrine/AuthorizationCodeManager.php b/Manager/Doctrine/AuthorizationCodeManager.php index a24124c8..b0aa7c3b 100644 --- a/Manager/Doctrine/AuthorizationCodeManager.php +++ b/Manager/Doctrine/AuthorizationCodeManager.php @@ -4,6 +4,7 @@ namespace Trikoder\Bundle\OAuth2Bundle\Manager\Doctrine; +use DateTimeImmutable; use Doctrine\ORM\EntityManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Manager\AuthorizationCodeManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Model\AuthorizationCode; @@ -36,4 +37,14 @@ public function save(AuthorizationCode $authorizationCode): void $this->entityManager->persist($authorizationCode); $this->entityManager->flush(); } + + public function clearExpired(): int + { + return $this->entityManager->createQueryBuilder() + ->delete(AuthorizationCode::class, 'ac') + ->where('ac.expiry < :expiry') + ->setParameter('expiry', new DateTimeImmutable()) + ->getQuery() + ->execute(); + } } diff --git a/Manager/Doctrine/RefreshTokenManager.php b/Manager/Doctrine/RefreshTokenManager.php index 96b92777..2f8172f2 100644 --- a/Manager/Doctrine/RefreshTokenManager.php +++ b/Manager/Doctrine/RefreshTokenManager.php @@ -4,7 +4,7 @@ namespace Trikoder\Bundle\OAuth2Bundle\Manager\Doctrine; -use DateTime; +use DateTimeImmutable; use Doctrine\ORM\EntityManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Manager\RefreshTokenManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Model\RefreshToken; @@ -43,7 +43,7 @@ public function clearExpired(): int return $this->entityManager->createQueryBuilder() ->delete(RefreshToken::class, 'rt') ->where('rt.expiry < :expiry') - ->setParameter('expiry', new DateTime()) + ->setParameter('expiry', new DateTimeImmutable()) ->getQuery() ->execute(); } diff --git a/Manager/InMemory/AccessTokenManager.php b/Manager/InMemory/AccessTokenManager.php index 13a99eab..9fd7ff63 100644 --- a/Manager/InMemory/AccessTokenManager.php +++ b/Manager/InMemory/AccessTokenManager.php @@ -4,7 +4,7 @@ namespace Trikoder\Bundle\OAuth2Bundle\Manager\InMemory; -use DateTime; +use DateTimeImmutable; use Trikoder\Bundle\OAuth2Bundle\Manager\AccessTokenManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Model\AccessToken; @@ -35,8 +35,8 @@ public function clearExpired(): int { $count = \count($this->accessTokens); - $now = new DateTime(); - $this->accessTokens = array_filter($this->accessTokens, function (AccessToken $accessToken) use ($now): bool { + $now = new DateTimeImmutable(); + $this->accessTokens = array_filter($this->accessTokens, static function (AccessToken $accessToken) use ($now): bool { return $accessToken->getExpiry() >= $now; }); diff --git a/Manager/InMemory/AuthorizationCodeManager.php b/Manager/InMemory/AuthorizationCodeManager.php index a51f36d1..f8d42b2b 100644 --- a/Manager/InMemory/AuthorizationCodeManager.php +++ b/Manager/InMemory/AuthorizationCodeManager.php @@ -4,6 +4,7 @@ namespace Trikoder\Bundle\OAuth2Bundle\Manager\InMemory; +use DateTimeImmutable; use Trikoder\Bundle\OAuth2Bundle\Manager\AuthorizationCodeManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Model\AuthorizationCode; @@ -23,4 +24,16 @@ public function save(AuthorizationCode $authorizationCode): void { $this->authorizationCodes[$authorizationCode->getIdentifier()] = $authorizationCode; } + + public function clearExpired(): int + { + $count = \count($this->authorizationCodes); + + $now = new DateTimeImmutable(); + $this->authorizationCodes = array_filter($this->authorizationCodes, static function (AuthorizationCode $authorizationCode) use ($now): bool { + return $authorizationCode->getExpiryDateTime() >= $now; + }); + + return $count - \count($this->authorizationCodes); + } } diff --git a/Manager/InMemory/ClientManager.php b/Manager/InMemory/ClientManager.php index c5d57ab2..10a5fbd9 100644 --- a/Manager/InMemory/ClientManager.php +++ b/Manager/InMemory/ClientManager.php @@ -48,7 +48,7 @@ public function list(?ClientFilter $clientFilter): array return $this->clients; } - return array_filter($this->clients, function (Client $client) use ($clientFilter): bool { + return array_filter($this->clients, static function (Client $client) use ($clientFilter): bool { $grantsPassed = self::passesFilter($client->getGrants(), $clientFilter->getGrants()); $scopesPassed = self::passesFilter($client->getScopes(), $clientFilter->getScopes()); $redirectUrisPassed = self::passesFilter($client->getRedirectUris(), $clientFilter->getRedirectUris()); diff --git a/Manager/InMemory/RefreshTokenManager.php b/Manager/InMemory/RefreshTokenManager.php index 02970601..be410b9b 100644 --- a/Manager/InMemory/RefreshTokenManager.php +++ b/Manager/InMemory/RefreshTokenManager.php @@ -4,7 +4,7 @@ namespace Trikoder\Bundle\OAuth2Bundle\Manager\InMemory; -use DateTime; +use DateTimeImmutable; use Trikoder\Bundle\OAuth2Bundle\Manager\RefreshTokenManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Model\RefreshToken; @@ -35,8 +35,8 @@ public function clearExpired(): int { $count = \count($this->refreshTokens); - $now = new DateTime(); - $this->refreshTokens = array_filter($this->refreshTokens, function (RefreshToken $refreshToken) use ($now): bool { + $now = new DateTimeImmutable(); + $this->refreshTokens = array_filter($this->refreshTokens, static function (RefreshToken $refreshToken) use ($now): bool { return $refreshToken->getExpiry() >= $now; }); diff --git a/Model/AuthorizationCode.php b/Model/AuthorizationCode.php index 06675997..2e496e01 100644 --- a/Model/AuthorizationCode.php +++ b/Model/AuthorizationCode.php @@ -38,6 +38,11 @@ class AuthorizationCode */ private $revoked = false; + /** + * @var string|null + */ + private $nonce; + public function __construct( string $identifier, DateTimeInterface $expiry, @@ -96,4 +101,18 @@ public function revoke(): self return $this; } + + public function getNonce(): ?string + { + return $this->nonce; + } + + public function setNonce(string $nonce): self + { + if ($this->nonce === null) { + $this->nonce = $nonce; + } + + return $this; + } } diff --git a/Model/AuthorizationDecision/AlwaysAllowDecisionStrategy.php b/Model/AuthorizationDecision/AlwaysAllowDecisionStrategy.php new file mode 100644 index 00000000..022f4c32 --- /dev/null +++ b/Model/AuthorizationDecision/AlwaysAllowDecisionStrategy.php @@ -0,0 +1,13 @@ +resolveAuthorization(true); + } +} diff --git a/Model/AuthorizationDecision/AuthorizationDecisionStrategy.php b/Model/AuthorizationDecision/AuthorizationDecisionStrategy.php new file mode 100644 index 00000000..7bd987c3 --- /dev/null +++ b/Model/AuthorizationDecision/AuthorizationDecisionStrategy.php @@ -0,0 +1,9 @@ +consentApprovalRoute = $consentApprovalRoute; + $this->uriSigner = $uriSigner; + $this->requestStack = $requestStack; + $this->urlGenerator = $urlGenerator; + } + + /** + * @param AuthorizationRequestResolveEvent $event + */ + public function decide(AuthorizationRequestResolveEvent $event): void + { + if (null === $request = $this->requestStack->getMasterRequest()) { + throw new \RuntimeException('Consent decision strategy depends on the request context'); + } + + // if the request carries approval result + if ($this->canResolveAuthorizationRequest($event, $request)) { + if ($this->isAuthorizationAllowed($request)) { + $event->resolveAuthorization(true); + } + + // disapproved consent is handled by League component + return; + } + + $event->setResponse($this->createRedirectToConsentResponse($event)); + } + + private function canResolveAuthorizationRequest(AuthorizationRequestResolveEvent $event, Request $request) + { + if (!$request->query->has(self::ATTRIBUTE_DECISION)) { + return false; + } + + $currentUri = $request->getRequestUri(); + if (!$this->uriSigner->check($currentUri)) { + return false; + } + + if ($request->query->get('client_id') !== $event->getClient()->getIdentifier()) { + return false; + } + if ($request->query->get('response_type') !== $this->getResponseType($event)) { + return false; + } + if ($request->query->get('redirect_uri') !== $event->getRedirectUri()) { + return false; + } + if ($request->query->get('scope') !== $this->getScope($event)) { + return false; + } + + return true; + } + + private function createRedirectToConsentResponse(AuthorizationRequestResolveEvent $event): Response + { + $params = [ + 'client_id' => $event->getClient()->getIdentifier(), + 'response_type' => $this->getResponseType($event), + ]; + if (null !== $redirectUri = $event->getRedirectUri()) { + $params['redirect_uri'] = $redirectUri; + } + if (null !== $state = $event->getState()) { + $params['state'] = $state; + } + $scope = $this->getScope($event); + if (null !== $scope) { + $params['scope'] = $scope; + } + + $redirectUri = $this->urlGenerator->generate($this->consentApprovalRoute, $params); + return new Response\RedirectResponse($redirectUri); + } + + private function getResponseType(AuthorizationRequestResolveEvent $event): string + { + switch ($event->getGrantTypeId()) { + case OAuth2Grants::AUTHORIZATION_CODE: + return 'code'; + case OAuth2Grants::IMPLICIT: + return 'token'; + default: + return $event->getGrantTypeId(); + } + } + + private function getScope(AuthorizationRequestResolveEvent $event): ?string + { + $scopes = array_map(function (Scope $scope) { + return $scope->getIdentifier(); + }, $event->getScopes()); + + if (empty($scopes)) { + return null; + } + + return implode(' ', $scopes); + } + + private function isAuthorizationAllowed(Request $request): bool + { + return $request->get(self::ATTRIBUTE_DECISION) === self::ATTRIBUTE_DECISION_ALLOW; + } +} diff --git a/Model/Client.php b/Model/Client.php index 9cc742f0..06a4c55c 100644 --- a/Model/Client.php +++ b/Model/Client.php @@ -12,7 +12,7 @@ class Client private $identifier; /** - * @var string + * @var string|null */ private $secret; @@ -36,7 +36,12 @@ class Client */ private $active = true; - public function __construct(string $identifier, string $secret) + /** + * @var bool + */ + private $allowPlainTextPkce = false; + + public function __construct(string $identifier, ?string $secret) { $this->identifier = $identifier; $this->secret = $secret; @@ -52,7 +57,7 @@ public function getIdentifier(): string return $this->identifier; } - public function getSecret(): string + public function getSecret(): ?string { return $this->secret; } @@ -113,4 +118,21 @@ public function setActive(bool $active): self return $this; } + + public function isConfidential(): bool + { + return !empty($this->secret); + } + + public function isPlainTextPkceAllowed(): bool + { + return $this->allowPlainTextPkce; + } + + public function setAllowPlainTextPkce(bool $allowPlainTextPkce): self + { + $this->allowPlainTextPkce = $allowPlainTextPkce; + + return $this; + } } diff --git a/Model/Grant.php b/Model/Grant.php index 9203d22c..60aad4e6 100644 --- a/Model/Grant.php +++ b/Model/Grant.php @@ -17,9 +17,7 @@ class Grant public function __construct(string $grant) { if (!OAuth2Grants::has($grant)) { - throw new RuntimeException( - sprintf('The \'%s\' grant is not supported.', $grant) - ); + throw new RuntimeException(sprintf('The \'%s\' grant is not supported.', $grant)); } $this->grant = $grant; diff --git a/Model/RedirectUri.php b/Model/RedirectUri.php index 866690ec..707138de 100644 --- a/Model/RedirectUri.php +++ b/Model/RedirectUri.php @@ -16,9 +16,7 @@ class RedirectUri public function __construct(string $redirectUri) { if (!filter_var($redirectUri, FILTER_VALIDATE_URL)) { - throw new RuntimeException( - sprintf('The \'%s\' string is not a valid URI.', $redirectUri) - ); + throw new RuntimeException(sprintf('The \'%s\' string is not a valid URI.', $redirectUri)); } $this->redirectUri = $redirectUri; diff --git a/OAuth2Events.php b/OAuth2Events.php index 8894eb9d..7b49a83f 100644 --- a/OAuth2Events.php +++ b/OAuth2Events.php @@ -7,7 +7,7 @@ final class OAuth2Events { /** - * The USER_RESOLVE event occurrs when the client requests a "password" + * The USER_RESOLVE event occurs when the client requests a "password" * grant type from the authorization server. * * You should set a valid user here if applicable. @@ -15,7 +15,7 @@ final class OAuth2Events public const USER_RESOLVE = 'trikoder.oauth2.user_resolve'; /** - * The SCOPE_RESOLVE event occurrs right before the user obtains their + * The SCOPE_RESOLVE event occurs right before the user obtains their * valid access token. * * You could alter the access token's scope here. @@ -23,11 +23,19 @@ final class OAuth2Events public const SCOPE_RESOLVE = 'trikoder.oauth2.scope_resolve'; /** - * The AUTHORIZATION_REQUEST_RESOLVE event occurrs right before the system + * The AUTHORIZATION_REQUEST_RESOLVE event occurs right before the system * complete the authorization request. * * You could approve or deny the authorization request, or set the uri where * must be redirected to resolve the authorization request. */ public const AUTHORIZATION_REQUEST_RESOLVE = 'trikoder.oauth2.authorization_request_resolve'; + + /** + * The AUTHORIZATION_CLAIMS_RESOLVE event occurs when the user requests + * an id token from the OpenID Connect Provider + * + * You should set the user claims here if applicable. + */ + public const AUTHORIZATION_CLAIMS_RESOLVE = 'trikoder.oauth2.claims_resolve'; } diff --git a/OpenIDConnect/Entity/Identity.php b/OpenIDConnect/Entity/Identity.php new file mode 100644 index 00000000..b3211c84 --- /dev/null +++ b/OpenIDConnect/Entity/Identity.php @@ -0,0 +1,24 @@ +claims; + } + + public function setClaims(array $claims) + { + $this->claims = $claims; + } +} diff --git a/OpenIDConnect/IdTokenResponse.php b/OpenIDConnect/IdTokenResponse.php new file mode 100644 index 00000000..3f73e1ac --- /dev/null +++ b/OpenIDConnect/IdTokenResponse.php @@ -0,0 +1,29 @@ +nonce = $nonce; + } + + protected function getBuilder(AccessTokenEntityInterface $accessToken, UserEntityInterface $userEntity) + { + $builder = parent::getBuilder($accessToken, $userEntity); + + if (null !== $this->nonce) { + $builder->set('nonce', $this->nonce); + } + + return $builder; + } +} diff --git a/OpenIDConnect/Repository/IdentityProvider.php b/OpenIDConnect/Repository/IdentityProvider.php new file mode 100644 index 00000000..b58710d5 --- /dev/null +++ b/OpenIDConnect/Repository/IdentityProvider.php @@ -0,0 +1,37 @@ +eventDispatcher = $eventDispatcher; + } + + public function getUserEntityByIdentifier($identifier) + { + $user = new Identity(); + $user->setIdentifier($identifier); + + $event = $this->eventDispatcher->dispatch( + new ClaimsResolveEvent($identifier), + OAuth2Events::AUTHORIZATION_CLAIMS_RESOLVE + ); + + $user->setClaims($event->getClaims()); + + return $user; + } +} diff --git a/README.md b/README.md index b9022337..2a4b3668 100644 --- a/README.md +++ b/README.md @@ -26,8 +26,7 @@ This package is currently in the active development. ## Requirements * [PHP 7.2](http://php.net/releases/7_2_0.php) or greater -* [Symfony 4.2](https://symfony.com/roadmap/4.2) or [Symfony 3.4](https://symfony.com/roadmap/3.4) -* [league/oauth2-server (versions >=7.2.0 <8.0)](https://packagist.org/packages/league/oauth2-server) +* [Symfony 4.4](https://symfony.com/roadmap/4.4) or [Symfony 5.x](https://symfony.com/roadmap/5.0) ## Installation @@ -85,6 +84,9 @@ This package is currently in the active development. # Whether to enable the authorization code grant enable_auth_code_grant: true + # Whether to require code challenge for public clients for the auth code grant + require_code_challenge_for_public_clients: true + # Whether to enable the implicit grant enable_implicit_grant: true resource_server: # Required @@ -108,6 +110,9 @@ This package is currently in the active development. # The priority of the event listener that converts an Exception to a Response exception_event_listener_priority: 10 + + # Set a custom prefix that replaces the default 'ROLE_OAUTH2_' role prefix + role_prefix: ROLE_OAUTH2_ ``` 1. Enable the bundle in `config/bundles.php` by adding it to the array: @@ -172,7 +177,7 @@ dev/bin/php composer install You can run the test suite using the following command: ```sh -dev/bin/php composer test +dev/bin/php-test composer test ``` ### Debugging diff --git a/Resources/config/doctrine/model/AccessToken.orm.xml b/Resources/config/doctrine/model/AccessToken.orm.xml index 247c68fe..192c6d9c 100644 --- a/Resources/config/doctrine/model/AccessToken.orm.xml +++ b/Resources/config/doctrine/model/AccessToken.orm.xml @@ -1,3 +1,5 @@ + + true - + - + diff --git a/Resources/config/doctrine/model/AuthorizationCode.orm.xml b/Resources/config/doctrine/model/AuthorizationCode.orm.xml index 1beaa55b..a5acc9c2 100644 --- a/Resources/config/doctrine/model/AuthorizationCode.orm.xml +++ b/Resources/config/doctrine/model/AuthorizationCode.orm.xml @@ -1,3 +1,5 @@ + + true - + + diff --git a/Resources/config/doctrine/model/Client.orm.xml b/Resources/config/doctrine/model/Client.orm.xml index 804dffca..1c90b76a 100644 --- a/Resources/config/doctrine/model/Client.orm.xml +++ b/Resources/config/doctrine/model/Client.orm.xml @@ -1,13 +1,20 @@ + + - + + + + + + diff --git a/Resources/config/doctrine/model/RefreshToken.orm.xml b/Resources/config/doctrine/model/RefreshToken.orm.xml index 94e48f20..bcd9cefc 100644 --- a/Resources/config/doctrine/model/RefreshToken.orm.xml +++ b/Resources/config/doctrine/model/RefreshToken.orm.xml @@ -1,3 +1,5 @@ + + true - + diff --git a/Resources/config/routes.xml b/Resources/config/routes.xml index 797fe8d9..091f1a90 100644 --- a/Resources/config/routes.xml +++ b/Resources/config/routes.xml @@ -1,3 +1,5 @@ + + + diff --git a/Resources/config/services.xml b/Resources/config/services.xml index 83e56715..59104dc5 100644 --- a/Resources/config/services.xml +++ b/Resources/config/services.xml @@ -1,3 +1,5 @@ + + @@ -28,14 +30,14 @@ - + - + @@ -53,6 +55,7 @@ + @@ -63,6 +66,7 @@ + @@ -113,9 +117,10 @@ - + + @@ -130,6 +135,33 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -137,6 +169,24 @@ + + + + + + + + + + + + + + + + + + @@ -165,6 +215,7 @@ + @@ -183,5 +234,9 @@ + + + + diff --git a/Resources/config/storage/doctrine.xml b/Resources/config/storage/doctrine.xml index 9e5214b9..90a7f617 100644 --- a/Resources/config/storage/doctrine.xml +++ b/Resources/config/storage/doctrine.xml @@ -1,3 +1,5 @@ + + diff --git a/Resources/config/storage/in_memory.xml b/Resources/config/storage/in_memory.xml index 10ef3f05..396b9fcb 100644 --- a/Resources/config/storage/in_memory.xml +++ b/Resources/config/storage/in_memory.xml @@ -1,3 +1,5 @@ + + diff --git a/Security/Authentication/Provider/OAuth2Provider.php b/Security/Authentication/Provider/OAuth2Provider.php index 2338bf3e..3528b2e7 100644 --- a/Security/Authentication/Provider/OAuth2Provider.php +++ b/Security/Authentication/Provider/OAuth2Provider.php @@ -13,6 +13,7 @@ use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Component\Security\Core\User\UserProviderInterface; use Trikoder\Bundle\OAuth2Bundle\Security\Authentication\Token\OAuth2Token; +use Trikoder\Bundle\OAuth2Bundle\Security\Authentication\Token\OAuth2TokenFactory; final class OAuth2Provider implements AuthenticationProviderInterface { @@ -26,10 +27,26 @@ final class OAuth2Provider implements AuthenticationProviderInterface */ private $resourceServer; - public function __construct(UserProviderInterface $userProvider, ResourceServer $resourceServer) - { + /** + * @var OAuth2TokenFactory + */ + private $oauth2TokenFactory; + + /** + * @var string + */ + private $providerKey; + + public function __construct( + UserProviderInterface $userProvider, + ResourceServer $resourceServer, + OAuth2TokenFactory $oauth2TokenFactory, + string $providerKey + ) { $this->userProvider = $userProvider; $this->resourceServer = $resourceServer; + $this->oauth2TokenFactory = $oauth2TokenFactory; + $this->providerKey = $providerKey; } /** @@ -38,12 +55,7 @@ public function __construct(UserProviderInterface $userProvider, ResourceServer public function authenticate(TokenInterface $token) { if (!$this->supports($token)) { - throw new RuntimeException( - sprintf( - 'This authentication provider can only handle tokes of type \'%s\'.', - OAuth2Token::class - ) - ); + throw new RuntimeException(sprintf('This authentication provider can only handle tokes of type \'%s\'.', OAuth2Token::class)); } try { @@ -58,7 +70,7 @@ public function authenticate(TokenInterface $token) $request->getAttribute('oauth_user_id') ); - $token = new OAuth2Token($request, $user); + $token = $this->oauth2TokenFactory->createOAuth2Token($request, $user, $this->providerKey); $token->setAuthenticated(true); return $token; @@ -69,7 +81,7 @@ public function authenticate(TokenInterface $token) */ public function supports(TokenInterface $token) { - return $token instanceof OAuth2Token; + return $token instanceof OAuth2Token && $this->providerKey === $token->getProviderKey(); } private function getAuthenticatedUser(string $userIdentifier): ?UserInterface diff --git a/Security/Authentication/Token/OAuth2Token.php b/Security/Authentication/Token/OAuth2Token.php index 6b873128..d368e933 100644 --- a/Security/Authentication/Token/OAuth2Token.php +++ b/Security/Authentication/Token/OAuth2Token.php @@ -10,9 +10,19 @@ final class OAuth2Token extends AbstractToken { - public function __construct(ServerRequestInterface $serverRequest, ?UserInterface $user) - { + /** + * @var string + */ + private $providerKey; + + public function __construct( + ServerRequestInterface $serverRequest, + ?UserInterface $user, + string $rolePrefix, + string $providerKey + ) { $this->setAttribute('server_request', $serverRequest); + $this->setAttribute('role_prefix', $rolePrefix); $roles = $this->buildRolesFromScopes(); @@ -24,6 +34,8 @@ public function __construct(ServerRequestInterface $serverRequest, ?UserInterfac } parent::__construct(array_unique($roles)); + + $this->providerKey = $providerKey; } /** @@ -34,12 +46,29 @@ public function getCredentials() return $this->getAttribute('server_request')->getAttribute('oauth_access_token_id'); } + public function getProviderKey(): string + { + return $this->providerKey; + } + + public function __serialize(): array + { + return [$this->providerKey, parent::__serialize()]; + } + + public function __unserialize(array $data): void + { + [$this->providerKey, $parentData] = $data; + parent::__unserialize($parentData); + } + private function buildRolesFromScopes(): array { + $prefix = $this->getAttribute('role_prefix'); $roles = []; foreach ($this->getAttribute('server_request')->getAttribute('oauth_scopes', []) as $scope) { - $roles[] = sprintf('ROLE_OAUTH2_%s', trim(strtoupper($scope))); + $roles[] = strtoupper(trim($prefix . $scope)); } return $roles; diff --git a/Security/Authentication/Token/OAuth2TokenFactory.php b/Security/Authentication/Token/OAuth2TokenFactory.php new file mode 100644 index 00000000..61419706 --- /dev/null +++ b/Security/Authentication/Token/OAuth2TokenFactory.php @@ -0,0 +1,26 @@ +rolePrefix = $rolePrefix; + } + + public function createOAuth2Token(ServerRequestInterface $serverRequest, ?UserInterface $user, string $providerKey): OAuth2Token + { + return new OAuth2Token($serverRequest, $user, $this->rolePrefix, $providerKey); + } +} diff --git a/Security/EntryPoint/OAuth2EntryPoint.php b/Security/EntryPoint/OAuth2EntryPoint.php index 3186ea87..366cae1a 100644 --- a/Security/EntryPoint/OAuth2EntryPoint.php +++ b/Security/EntryPoint/OAuth2EntryPoint.php @@ -15,7 +15,7 @@ final class OAuth2EntryPoint implements AuthenticationEntryPointInterface /** * {@inheritdoc} */ - public function start(Request $request, AuthenticationException $authException = null) + public function start(Request $request, ?AuthenticationException $authException = null) { $exception = new UnauthorizedHttpException('Bearer'); diff --git a/Security/Firewall/OAuth2Listener.php b/Security/Firewall/OAuth2Listener.php index 94ef9639..618cdc51 100644 --- a/Security/Firewall/OAuth2Listener.php +++ b/Security/Firewall/OAuth2Listener.php @@ -6,16 +6,16 @@ use Symfony\Bridge\PsrHttpMessage\HttpMessageFactoryInterface; use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\HttpKernel\Event\GetResponseEvent; +use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; use Symfony\Component\Security\Core\Exception\AuthenticationException; -use Symfony\Component\Security\Http\Firewall\ListenerInterface; use Trikoder\Bundle\OAuth2Bundle\Security\Authentication\Token\OAuth2Token; +use Trikoder\Bundle\OAuth2Bundle\Security\Authentication\Token\OAuth2TokenFactory; use Trikoder\Bundle\OAuth2Bundle\Security\Exception\InsufficientScopesException; use Trikoder\Bundle\OAuth2Bundle\Security\Exception\Oauth2AuthenticationFailedException; -final class OAuth2Listener implements ListenerInterface +final class OAuth2Listener { /** * @var TokenStorageInterface @@ -32,25 +32,31 @@ final class OAuth2Listener implements ListenerInterface */ private $httpMessageFactory; + /** + * @var OAuth2TokenFactory + */ + private $oauth2TokenFactory; + + /** + * @var string + */ + private $providerKey; + public function __construct( TokenStorageInterface $tokenStorage, AuthenticationManagerInterface $authenticationManager, - HttpMessageFactoryInterface $httpMessageFactory + HttpMessageFactoryInterface $httpMessageFactory, + OAuth2TokenFactory $oauth2TokenFactory, + string $providerKey ) { $this->tokenStorage = $tokenStorage; $this->authenticationManager = $authenticationManager; $this->httpMessageFactory = $httpMessageFactory; + $this->oauth2TokenFactory = $oauth2TokenFactory; + $this->providerKey = $providerKey; } - /** - * BC layer for Symfony < 4.3 - */ - public function handle(GetResponseEvent $event) - { - $this->__invoke($event); - } - - public function __invoke(GetResponseEvent $event) + public function __invoke(RequestEvent $event) { $request = $this->httpMessageFactory->createRequest($event->getRequest()); @@ -60,7 +66,7 @@ public function __invoke(GetResponseEvent $event) try { /** @var OAuth2Token $authenticatedToken */ - $authenticatedToken = $this->authenticationManager->authenticate(new OAuth2Token($request, null)); + $authenticatedToken = $this->authenticationManager->authenticate($this->oauth2TokenFactory->createOAuth2Token($request, null, $this->providerKey)); } catch (AuthenticationException $e) { throw Oauth2AuthenticationFailedException::create($e->getMessage()); } diff --git a/Service/BCEventDispatcher.php b/Service/BCEventDispatcher.php deleted file mode 100644 index c70bbca4..00000000 --- a/Service/BCEventDispatcher.php +++ /dev/null @@ -1,39 +0,0 @@ - - */ -class BCEventDispatcher implements ContractsEventDispatcher -{ - private $eventDispatcher; - - public function __construct(LegacyEventDispatcher $eventDispatcher) - { - $this->eventDispatcher = $eventDispatcher; - } - - /** - * This method is only used in this bundle. We will always call dispatch(object, string) - */ - public function dispatch($event/*, string $eventName = null*/) - { - $eventName = 1 < \func_num_args() ? func_get_arg(1) : null; - $eventName = $eventName ?? \get_class($event); - - return $this->eventDispatcher->dispatch($eventName, $event); - } - - public function addListener($eventName, $listener, $priority = 0) - { - return $this->eventDispatcher->addListener($eventName, $listener, $priority); - } -} diff --git a/Tests/Acceptance/AbstractAcceptanceTest.php b/Tests/Acceptance/AbstractAcceptanceTest.php index bdbdac06..3d5d0bc5 100644 --- a/Tests/Acceptance/AbstractAcceptanceTest.php +++ b/Tests/Acceptance/AbstractAcceptanceTest.php @@ -4,8 +4,8 @@ namespace Trikoder\Bundle\OAuth2Bundle\Tests\Acceptance; -use Symfony\Bundle\FrameworkBundle\Client; use Symfony\Bundle\FrameworkBundle\Console\Application; +use Symfony\Bundle\FrameworkBundle\KernelBrowser; use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; use Trikoder\Bundle\OAuth2Bundle\Tests\TestHelper; @@ -17,12 +17,13 @@ abstract class AbstractAcceptanceTest extends WebTestCase protected $application; /** - * @var Client + * @var KernelBrowser */ protected $client; protected function setUp(): void { + ini_set('memory_limit', '256M'); $this->client = self::createClient(); $this->application = new Application($this->client->getKernel()); diff --git a/Tests/Acceptance/AuthorizationEndpointTest.php b/Tests/Acceptance/AuthorizationEndpointTest.php index ed4198ce..304bac7a 100644 --- a/Tests/Acceptance/AuthorizationEndpointTest.php +++ b/Tests/Acceptance/AuthorizationEndpointTest.php @@ -4,16 +4,18 @@ namespace Trikoder\Bundle\OAuth2Bundle\Tests\Acceptance; -use DateTime; +use DateTimeImmutable; +use Nyholm\Psr7\Response; use Trikoder\Bundle\OAuth2Bundle\Event\AuthorizationRequestResolveEvent; use Trikoder\Bundle\OAuth2Bundle\Manager\AccessTokenManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Manager\AuthorizationCodeManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Manager\ClientManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Manager\RefreshTokenManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Manager\ScopeManagerInterface; +use Trikoder\Bundle\OAuth2Bundle\Model\AuthorizationCode; use Trikoder\Bundle\OAuth2Bundle\OAuth2Events; use Trikoder\Bundle\OAuth2Bundle\Tests\Fixtures\FixtureFactory; -use Zend\Diactoros\Response; +use Trikoder\Bundle\OAuth2Bundle\Tests\TestHelper; final class AuthorizationEndpointTest extends AbstractAcceptanceTest { @@ -35,11 +37,11 @@ public function testSuccessfulCodeRequest(): void $this->client ->getContainer() ->get('event_dispatcher') - ->addListener(OAuth2Events::AUTHORIZATION_REQUEST_RESOLVE, function (AuthorizationRequestResolveEvent $event): void { + ->addListener(OAuth2Events::AUTHORIZATION_REQUEST_RESOLVE, static function (AuthorizationRequestResolveEvent $event): void { $event->resolveAuthorization(AuthorizationRequestResolveEvent::AUTHORIZATION_APPROVED); }); - timecop_freeze(new DateTime()); + timecop_freeze(new DateTimeImmutable()); try { $this->client->request( @@ -68,16 +70,240 @@ public function testSuccessfulCodeRequest(): void $this->assertEquals('foobar', $query['state']); } - public function testSuccessfulTokenRequest(): void + public function testSuccessfulPKCEAuthCodeRequest(): void + { + $state = bin2hex(random_bytes(20)); + $codeVerifier = bin2hex(random_bytes(64)); + $codeChallengeMethod = 'S256'; + + $codeChallenge = strtr( + rtrim(base64_encode(hash('sha256', $codeVerifier, true)), '='), + '+/', + '-_' + ); + + $this->client + ->getContainer() + ->get('event_dispatcher') + ->addListener(OAuth2Events::AUTHORIZATION_REQUEST_RESOLVE, function (AuthorizationRequestResolveEvent $event) use ($state, $codeChallenge, $codeChallengeMethod): void { + $this->assertSame($state, $event->getState()); + $this->assertSame($codeChallenge, $event->getCodeChallenge()); + $this->assertSame($codeChallengeMethod, $event->getCodeChallengeMethod()); + + $event->resolveAuthorization(AuthorizationRequestResolveEvent::AUTHORIZATION_APPROVED); + }); + + timecop_freeze(new DateTimeImmutable()); + + try { + $this->client->request( + 'GET', + '/authorize', + [ + 'client_id' => FixtureFactory::FIXTURE_PUBLIC_CLIENT, + 'response_type' => 'code', + 'scope' => '', + 'state' => $state, + 'code_challenge' => $codeChallenge, + 'code_challenge_method' => $codeChallengeMethod, + ] + ); + } finally { + timecop_return(); + } + + $response = $this->client->getResponse(); + + $this->assertSame(302, $response->getStatusCode()); + $redirectUri = $response->headers->get('Location'); + + $this->assertStringStartsWith(FixtureFactory::FIXTURE_CLIENT_FIRST_REDIRECT_URI, $redirectUri); + $query = []; + parse_str(parse_url($redirectUri, PHP_URL_QUERY), $query); + $this->assertArrayHasKey('state', $query); + $this->assertSame($state, $query['state']); + + $this->assertArrayHasKey('code', $query); + $payload = json_decode(TestHelper::decryptPayload($query['code']), true); + + $this->assertArrayHasKey('code_challenge', $payload); + $this->assertArrayHasKey('code_challenge_method', $payload); + $this->assertSame($codeChallenge, $payload['code_challenge']); + $this->assertSame($codeChallengeMethod, $payload['code_challenge_method']); + + /** @var AuthorizationCode|null $authCode */ + $authCode = $this->client + ->getContainer() + ->get('doctrine.orm.entity_manager') + ->getRepository(AuthorizationCode::class) + ->findOneBy(['identifier' => $payload['auth_code_id']]); + + $this->assertInstanceOf(AuthorizationCode::class, $authCode); + $this->assertSame(FixtureFactory::FIXTURE_PUBLIC_CLIENT, $authCode->getClient()->getIdentifier()); + } + + public function testAuthCodeRequestWithPublicClientWithoutCodeChallengeWhenTheChallengeIsRequiredForPublicClients(): void { $this->client ->getContainer() ->get('event_dispatcher') ->addListener(OAuth2Events::AUTHORIZATION_REQUEST_RESOLVE, function (AuthorizationRequestResolveEvent $event): void { + $this->fail('This event should not have been dispatched.'); + }); + + timecop_freeze(new DateTimeImmutable()); + + try { + $this->client->request( + 'GET', + '/authorize', + [ + 'client_id' => FixtureFactory::FIXTURE_PUBLIC_CLIENT, + 'response_type' => 'code', + 'scope' => '', + 'state' => bin2hex(random_bytes(20)), + ] + ); + } finally { + timecop_return(); + } + + $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('invalid_request', $jsonResponse['error']); + $this->assertSame('The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed.', $jsonResponse['message']); + $this->assertSame('Code challenge must be provided for public clients', $jsonResponse['hint']); + } + + public function testAuthCodeRequestWithClientWhoIsNotAllowedToMakeARequestWithPlainCodeChallengeMethod(): void + { + $state = bin2hex(random_bytes(20)); + $codeVerifier = bin2hex(random_bytes(32)); + $codeChallengeMethod = 'plain'; + $codeChallenge = strtr(rtrim(base64_encode($codeVerifier), '='), '+/', '-_'); + + $this->client + ->getContainer() + ->get('event_dispatcher') + ->addListener(OAuth2Events::AUTHORIZATION_REQUEST_RESOLVE, function (AuthorizationRequestResolveEvent $event): void { + $this->fail('This event should not have been dispatched.'); + }); + + timecop_freeze(new DateTimeImmutable()); + + try { + $this->client->request( + 'GET', + '/authorize', + [ + 'client_id' => FixtureFactory::FIXTURE_PUBLIC_CLIENT, + 'response_type' => 'code', + 'scope' => '', + 'state' => $state, + 'code_challenge' => $codeChallenge, + 'code_challenge_method' => $codeChallengeMethod, + ] + ); + } finally { + timecop_return(); + } + + $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('invalid_request', $jsonResponse['error']); + $this->assertSame('The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed.', $jsonResponse['message']); + $this->assertSame('Plain code challenge method is not allowed for this client', $jsonResponse['hint']); + } + + public function testAuthCodeRequestWithClientWhoIsAllowedToMakeARequestWithPlainCodeChallengeMethod(): void + { + $state = bin2hex(random_bytes(20)); + $codeVerifier = bin2hex(random_bytes(32)); + $codeChallengeMethod = 'plain'; + $codeChallenge = strtr(rtrim(base64_encode($codeVerifier), '='), '+/', '-_'); + + $this->client + ->getContainer() + ->get('event_dispatcher') + ->addListener(OAuth2Events::AUTHORIZATION_REQUEST_RESOLVE, function (AuthorizationRequestResolveEvent $event) use ($state, $codeChallenge, $codeChallengeMethod): void { + $this->assertSame($state, $event->getState()); + $this->assertSame($codeChallenge, $event->getCodeChallenge()); + $this->assertSame($codeChallengeMethod, $event->getCodeChallengeMethod()); + $event->resolveAuthorization(AuthorizationRequestResolveEvent::AUTHORIZATION_APPROVED); }); - timecop_freeze(new DateTime()); + timecop_freeze(new DateTimeImmutable()); + + try { + $this->client->request( + 'GET', + '/authorize', + [ + 'client_id' => FixtureFactory::FIXTURE_PUBLIC_CLIENT_ALLOWED_TO_USE_PLAIN_CHALLENGE_METHOD, + 'response_type' => 'code', + 'scope' => '', + 'state' => $state, + 'code_challenge' => $codeChallenge, + 'code_challenge_method' => $codeChallengeMethod, + ] + ); + } finally { + timecop_return(); + } + + $response = $this->client->getResponse(); + + $this->assertSame(302, $response->getStatusCode()); + $redirectUri = $response->headers->get('Location'); + + $this->assertStringStartsWith(FixtureFactory::FIXTURE_PUBLIC_CLIENT_ALLOWED_TO_USE_PLAIN_CHALLENGE_METHOD_REDIRECT_URI, $redirectUri); + $query = []; + parse_str(parse_url($redirectUri, PHP_URL_QUERY), $query); + $this->assertArrayHasKey('state', $query); + $this->assertSame($state, $query['state']); + + $this->assertArrayHasKey('code', $query); + $payload = json_decode(TestHelper::decryptPayload($query['code']), true); + + $this->assertArrayHasKey('code_challenge', $payload); + $this->assertArrayHasKey('code_challenge_method', $payload); + $this->assertSame($codeChallenge, $payload['code_challenge']); + $this->assertSame($codeChallengeMethod, $payload['code_challenge_method']); + + /** @var AuthorizationCode|null $authCode */ + $authCode = $this->client + ->getContainer() + ->get('doctrine.orm.entity_manager') + ->getRepository(AuthorizationCode::class) + ->findOneBy(['identifier' => $payload['auth_code_id']]); + + $this->assertInstanceOf(AuthorizationCode::class, $authCode); + $this->assertSame(FixtureFactory::FIXTURE_PUBLIC_CLIENT_ALLOWED_TO_USE_PLAIN_CHALLENGE_METHOD, $authCode->getClient()->getIdentifier()); + } + + public function testSuccessfulTokenRequest(): void + { + $this->client + ->getContainer() + ->get('event_dispatcher') + ->addListener(OAuth2Events::AUTHORIZATION_REQUEST_RESOLVE, static function (AuthorizationRequestResolveEvent $event): void { + $event->resolveAuthorization(AuthorizationRequestResolveEvent::AUTHORIZATION_APPROVED); + }); + + timecop_freeze(new DateTimeImmutable()); try { $this->client->request( @@ -113,12 +339,12 @@ public function testCodeRequestRedirectToResolutionUri(): void $this->client ->getContainer() ->get('event_dispatcher') - ->addListener(OAuth2Events::AUTHORIZATION_REQUEST_RESOLVE, function (AuthorizationRequestResolveEvent $event): void { + ->addListener(OAuth2Events::AUTHORIZATION_REQUEST_RESOLVE, static function (AuthorizationRequestResolveEvent $event): void { $response = (new Response())->withStatus(302)->withHeader('Location', '/authorize/consent'); $event->setResponse($response); }); - timecop_freeze(new DateTime()); + timecop_freeze(new DateTimeImmutable()); try { $this->client->request( @@ -148,15 +374,15 @@ public function testAuthorizationRequestEventIsStoppedAfterSettingAResponse(): v $eventDispatcher = $this->client ->getContainer() ->get('event_dispatcher'); - $eventDispatcher->addListener(OAuth2Events::AUTHORIZATION_REQUEST_RESOLVE, function (AuthorizationRequestResolveEvent $event): void { + $eventDispatcher->addListener(OAuth2Events::AUTHORIZATION_REQUEST_RESOLVE, static function (AuthorizationRequestResolveEvent $event): void { $event->resolveAuthorization(AuthorizationRequestResolveEvent::AUTHORIZATION_APPROVED); }, 100); - $eventDispatcher->addListener(OAuth2Events::AUTHORIZATION_REQUEST_RESOLVE, function (AuthorizationRequestResolveEvent $event): void { + $eventDispatcher->addListener(OAuth2Events::AUTHORIZATION_REQUEST_RESOLVE, static function (AuthorizationRequestResolveEvent $event): void { $response = (new Response())->withStatus(302)->withHeader('Location', '/authorize/consent'); $event->setResponse($response); }, 200); - timecop_freeze(new DateTime()); + timecop_freeze(new DateTimeImmutable()); try { $this->client->request( @@ -184,15 +410,15 @@ public function testAuthorizationRequestEventIsStoppedAfterResolution(): void $eventDispatcher = $this->client ->getContainer() ->get('event_dispatcher'); - $eventDispatcher->addListener(OAuth2Events::AUTHORIZATION_REQUEST_RESOLVE, function (AuthorizationRequestResolveEvent $event): void { + $eventDispatcher->addListener(OAuth2Events::AUTHORIZATION_REQUEST_RESOLVE, static function (AuthorizationRequestResolveEvent $event): void { $event->resolveAuthorization(AuthorizationRequestResolveEvent::AUTHORIZATION_APPROVED); }, 200); - $eventDispatcher->addListener(OAuth2Events::AUTHORIZATION_REQUEST_RESOLVE, function (AuthorizationRequestResolveEvent $event): void { + $eventDispatcher->addListener(OAuth2Events::AUTHORIZATION_REQUEST_RESOLVE, static function (AuthorizationRequestResolveEvent $event): void { $response = (new Response())->withStatus(302)->withHeader('Location', '/authorize/consent'); $event->setResponse($response); }, 100); - timecop_freeze(new DateTime()); + timecop_freeze(new DateTimeImmutable()); try { $this->client->request( @@ -226,11 +452,11 @@ public function testFailedCodeRequestRedirectWithFakedRedirectUri(): void $this->client ->getContainer() ->get('event_dispatcher') - ->addListener(OAuth2Events::AUTHORIZATION_REQUEST_RESOLVE, function (AuthorizationRequestResolveEvent $event): void { + ->addListener(OAuth2Events::AUTHORIZATION_REQUEST_RESOLVE, static function (AuthorizationRequestResolveEvent $event): void { $event->resolveAuthorization(AuthorizationRequestResolveEvent::AUTHORIZATION_APPROVED); }); - timecop_freeze(new DateTime()); + timecop_freeze(new DateTimeImmutable()); try { $this->client->request( diff --git a/Tests/Acceptance/ClearExpiredTokensCommandTest.php b/Tests/Acceptance/ClearExpiredTokensCommandTest.php index abdd198a..5d0f02fb 100644 --- a/Tests/Acceptance/ClearExpiredTokensCommandTest.php +++ b/Tests/Acceptance/ClearExpiredTokensCommandTest.php @@ -4,7 +4,8 @@ namespace Trikoder\Bundle\OAuth2Bundle\Tests\Acceptance; -use DateTime; +use DateTimeImmutable; +use Doctrine\ORM\EntityManagerInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Tester\CommandTester; use Trikoder\Bundle\OAuth2Bundle\Manager\AccessTokenManagerInterface; @@ -12,6 +13,9 @@ use Trikoder\Bundle\OAuth2Bundle\Manager\ClientManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Manager\RefreshTokenManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Manager\ScopeManagerInterface; +use Trikoder\Bundle\OAuth2Bundle\Model\AccessToken; +use Trikoder\Bundle\OAuth2Bundle\Model\AuthorizationCode; +use Trikoder\Bundle\OAuth2Bundle\Model\RefreshToken; use Trikoder\Bundle\OAuth2Bundle\Tests\Fixtures\FixtureFactory; final class ClearExpiredTokensCommandTest extends AbstractAcceptanceTest @@ -20,7 +24,7 @@ protected function setUp(): void { parent::setUp(); - timecop_freeze(new DateTime()); + timecop_freeze(new DateTimeImmutable()); FixtureFactory::initializeFixtures( $this->client->getContainer()->get(ScopeManagerInterface::class), @@ -38,7 +42,7 @@ protected function tearDown(): void parent::tearDown(); } - public function testClearExpiredAccessAndRefreshTokens(): void + public function testClearExpiredAccessAndRefreshTokensAndAuthCodes(): void { $command = $this->command(); $commandTester = new CommandTester($command); @@ -52,6 +56,27 @@ public function testClearExpiredAccessAndRefreshTokens(): void $output = $commandTester->getDisplay(); $this->assertStringContainsString('Cleared 1 expired access token.', $output); $this->assertStringContainsString('Cleared 1 expired refresh token.', $output); + $this->assertStringContainsString('Cleared 1 expired auth code.', $output); + + /** @var EntityManagerInterface $em */ + $em = $this->client->getContainer()->get('doctrine.orm.entity_manager'); + $em->clear(); + + $this->assertNull( + $this->client->getContainer()->get(AccessTokenManagerInterface::class)->find( + FixtureFactory::FIXTURE_ACCESS_TOKEN_EXPIRED + ) + ); + $this->assertNull( + $this->client->getContainer()->get(RefreshTokenManagerInterface::class)->find( + FixtureFactory::FIXTURE_REFRESH_TOKEN_EXPIRED + ) + ); + $this->assertNull( + $this->client->getContainer()->get(AuthorizationCodeManagerInterface::class)->find( + FixtureFactory::FIXTURE_AUTH_CODE_EXPIRED + ) + ); } public function testClearExpiredAccessTokens(): void @@ -61,7 +86,7 @@ public function testClearExpiredAccessTokens(): void $exitCode = $commandTester->execute([ 'command' => $command->getName(), - '--access-tokens-only' => true, + '--access-tokens' => true, ]); $this->assertSame(0, $exitCode); @@ -69,6 +94,29 @@ public function testClearExpiredAccessTokens(): void $output = $commandTester->getDisplay(); $this->assertStringContainsString('Cleared 1 expired access token.', $output); $this->assertStringNotContainsString('Cleared 1 expired refresh token.', $output); + $this->assertStringNotContainsString('Cleared 1 expired auth code.', $output); + + /** @var EntityManagerInterface $em */ + $em = $this->client->getContainer()->get('doctrine.orm.entity_manager'); + $em->clear(); + + $this->assertNull( + $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->assertInstanceOf( + AuthorizationCode::class, + $this->client->getContainer()->get(AuthorizationCodeManagerInterface::class)->find( + FixtureFactory::FIXTURE_AUTH_CODE_EXPIRED + ) + ); } public function testClearExpiredRefreshTokens(): void @@ -78,7 +126,7 @@ public function testClearExpiredRefreshTokens(): void $exitCode = $commandTester->execute([ 'command' => $command->getName(), - '--refresh-tokens-only' => true, + '--refresh-tokens' => true, ]); $this->assertSame(0, $exitCode); @@ -86,23 +134,69 @@ public function testClearExpiredRefreshTokens(): void $output = $commandTester->getDisplay(); $this->assertStringNotContainsString('Cleared 1 expired access token.', $output); $this->assertStringContainsString('Cleared 1 expired refresh token.', $output); + $this->assertStringNotContainsString('Cleared 1 expired auth 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->assertNull( + $this->client->getContainer()->get(RefreshTokenManagerInterface::class)->find( + FixtureFactory::FIXTURE_REFRESH_TOKEN_EXPIRED + ) + ); + $this->assertInstanceOf( + AuthorizationCode::class, + $this->client->getContainer()->get(AuthorizationCodeManagerInterface::class)->find( + FixtureFactory::FIXTURE_AUTH_CODE_EXPIRED + ) + ); } - public function testErrorWhenBothOptionsAreUsed(): void + public function testClearExpiredAuthCodes(): void { $command = $this->command(); $commandTester = new CommandTester($command); $exitCode = $commandTester->execute([ 'command' => $command->getName(), - '--access-tokens-only' => true, - '--refresh-tokens-only' => true, + '--auth-codes' => true, ]); - $this->assertSame(1, $exitCode); + $this->assertSame(0, $exitCode); $output = $commandTester->getDisplay(); - $this->assertStringContainsString('Please choose only one of the following options:', $output); + $this->assertStringNotContainsString('Cleared 1 expired access token.', $output); + $this->assertStringNotContainsString('Cleared 1 expired refresh token.', $output); + $this->assertStringContainsString('Cleared 1 expired auth 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(AuthorizationCodeManagerInterface::class)->find( + FixtureFactory::FIXTURE_AUTH_CODE_EXPIRED + ) + ); } private function command(): Command diff --git a/Tests/Acceptance/CreateClientCommandTest.php b/Tests/Acceptance/CreateClientCommandTest.php index de3a9da4..166a8480 100644 --- a/Tests/Acceptance/CreateClientCommandTest.php +++ b/Tests/Acceptance/CreateClientCommandTest.php @@ -35,11 +35,68 @@ public function testCreateClientWithIdentifier(): void $this->assertStringContainsString('New oAuth2 client created successfully', $output); $this->assertStringContainsString('foobar', $output); + /** @var Client $client */ $client = $this->client ->getContainer() ->get(ClientManagerInterface::class) ->find('foobar'); $this->assertInstanceOf(Client::class, $client); + $this->assertTrue($client->isConfidential()); + $this->assertNotEmpty($client->getSecret()); + $this->assertFalse($client->isPlainTextPkceAllowed()); + } + + public function testCreatePublicClientWithIdentifier(): void + { + $clientIdentifier = 'foobar test'; + $command = $this->application->find('trikoder:oauth2:create-client'); + $commandTester = new CommandTester($command); + $commandTester->execute([ + 'command' => $command->getName(), + 'identifier' => $clientIdentifier, + '--public' => true, + ]); + + $this->assertSame(0, $commandTester->getStatusCode()); + + $output = $commandTester->getDisplay(); + $this->assertStringContainsString('New oAuth2 client created successfully', $output); + $this->assertStringContainsString($clientIdentifier, $output); + + /** @var Client $client */ + $client = $this->client + ->getContainer() + ->get(ClientManagerInterface::class) + ->find($clientIdentifier); + $this->assertInstanceOf(Client::class, $client); + $this->assertFalse($client->isConfidential()); + $this->assertNull($client->getSecret()); + $this->assertFalse($client->isPlainTextPkceAllowed()); + } + + public function testCannotCreatePublicClientWithSecret(): void + { + $clientIdentifier = 'foobar test'; + $command = $this->application->find('trikoder:oauth2:create-client'); + $commandTester = new CommandTester($command); + $commandTester->execute([ + 'command' => $command->getName(), + 'identifier' => $clientIdentifier, + 'secret' => 'foo', + '--public' => true, + ]); + + $this->assertSame(1, $commandTester->getStatusCode()); + + $output = $commandTester->getDisplay(); + $this->assertStringContainsString('The client cannot have a secret and be public.', $output); + $this->assertStringNotContainsString($clientIdentifier, $output); + + $client = $this->client + ->getContainer() + ->get(ClientManagerInterface::class) + ->find($clientIdentifier); + $this->assertNull($client); } public function testCreateClientWithSecret(): void @@ -54,12 +111,38 @@ public function testCreateClientWithSecret(): void $output = $commandTester->getDisplay(); $this->assertStringContainsString('New oAuth2 client created successfully', $output); + + /** @var Client $client */ $client = $this->client ->getContainer() ->get(ClientManagerInterface::class) ->find('foobar'); $this->assertInstanceOf(Client::class, $client); $this->assertSame('quzbaz', $client->getSecret()); + $this->assertTrue($client->isConfidential()); + $this->assertFalse($client->isPlainTextPkceAllowed()); + } + + public function testCreateClientWhoIsAllowedToUsePlainPkceChallengeMethod(): void + { + $command = $this->application->find('trikoder:oauth2:create-client'); + $commandTester = new CommandTester($command); + $commandTester->execute([ + 'command' => $command->getName(), + 'identifier' => 'foobar-123', + '--allow-plain-text-pkce' => true, + ]); + + $output = $commandTester->getDisplay(); + $this->assertStringContainsString('New oAuth2 client created successfully', $output); + + /** @var Client $client */ + $client = $this->client + ->getContainer() + ->get(ClientManagerInterface::class) + ->find('foobar-123'); + $this->assertInstanceOf(Client::class, $client); + $this->assertTrue($client->isPlainTextPkceAllowed()); } public function testCreateClientWithRedirectUris(): void diff --git a/Tests/Acceptance/DeleteClientCommandTest.php b/Tests/Acceptance/DeleteClientCommandTest.php index de132f19..eda37db7 100644 --- a/Tests/Acceptance/DeleteClientCommandTest.php +++ b/Tests/Acceptance/DeleteClientCommandTest.php @@ -9,6 +9,9 @@ use Trikoder\Bundle\OAuth2Bundle\Manager\ClientManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Model\Client; +/** + * @covers \Trikoder\Bundle\OAuth2Bundle\Command\DeleteClientCommand + */ final class DeleteClientCommandTest extends AbstractAcceptanceTest { public function testDeleteClient(): void @@ -42,27 +45,27 @@ public function testDeleteNonExistentClient(): void $this->assertStringContainsString(sprintf('oAuth2 client identified as "%s" does not exist', $identifierName), $output); } - private function findClient($identifier): ?Client + private function findClient(string $identifier): ?Client { return $this - ->client - ->getContainer() - ->get(ClientManagerInterface::class) + ->getClientManager() ->find($identifier) ; } - private function fakeAClient($identifier): Client + private function fakeAClient(string $identifier): Client { return new Client($identifier, 'quzbaz'); } private function getClientManager(): ClientManagerInterface { - return $this->client - ->getContainer() - ->get(ClientManagerInterface::class) + return + $this + ->client + ->getContainer() + ->get(ClientManagerInterface::class) ; } diff --git a/Tests/Acceptance/DoctrineAccessTokenManagerTest.php b/Tests/Acceptance/DoctrineAccessTokenManagerTest.php index b0194a93..c961d1c2 100644 --- a/Tests/Acceptance/DoctrineAccessTokenManagerTest.php +++ b/Tests/Acceptance/DoctrineAccessTokenManagerTest.php @@ -2,22 +2,24 @@ declare(strict_types=1); -namespace Trikoder\Bundle\OAuth2Bundle\Tests\Unit; +namespace Trikoder\Bundle\OAuth2Bundle\Tests\Acceptance; -use DateTime; +use DateTimeImmutable; +use Doctrine\ORM\EntityManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Manager\Doctrine\AccessTokenManager as DoctrineAccessTokenManager; use Trikoder\Bundle\OAuth2Bundle\Model\AccessToken; use Trikoder\Bundle\OAuth2Bundle\Model\Client; use Trikoder\Bundle\OAuth2Bundle\Model\RefreshToken; -use Trikoder\Bundle\OAuth2Bundle\Tests\Acceptance\AbstractAcceptanceTest; /** - * @TODO This should be in the Integration tests folder but the current tests infrastructure would need improvements first. + * @TODO This should be in the Integration tests folder but the current tests infrastructure would need improvements first. + * @covers \Trikoder\Bundle\OAuth2Bundle\Manager\Doctrine\AccessTokenManager */ final class DoctrineAccessTokenManagerTest extends AbstractAcceptanceTest { public function testClearExpired(): void { + /** @var EntityManagerInterface $em */ $em = $this->client->getContainer()->get('doctrine.orm.entity_manager'); $doctrineAccessTokenManager = new DoctrineAccessTokenManager($em); @@ -26,7 +28,7 @@ public function testClearExpired(): void $em->persist($client); $em->flush(); - timecop_freeze(new DateTime()); + timecop_freeze(new DateTimeImmutable()); try { $testData = $this->buildClearExpiredTestData($client); @@ -72,7 +74,7 @@ private function buildAccessToken(string $identifier, string $modify, Client $cl { return new AccessToken( $identifier, - (new DateTime())->modify($modify), + new DateTimeImmutable($modify), $client, null, [] @@ -81,15 +83,15 @@ private function buildAccessToken(string $identifier, string $modify, Client $cl public function testClearExpiredWithRefreshToken(): void { + /** @var EntityManagerInterface $em */ $em = $this->client->getContainer()->get('doctrine.orm.entity_manager'); - $doctrineAccessTokenManager = new DoctrineAccessTokenManager($em); $client = new Client('client', 'secret'); $em->persist($client); $em->flush(); - timecop_freeze(new DateTime()); + timecop_freeze(new DateTimeImmutable()); try { $testData = $this->buildClearExpiredTestDataWithRefreshToken($client); @@ -138,10 +140,10 @@ private function buildRefreshToken(string $identifier, string $modify, Client $c { return new RefreshToken( $identifier, - (new DateTime('+1 day')), + new DateTimeImmutable('+1 day'), new AccessToken( $identifier, - (new DateTime())->modify($modify), + new DateTimeImmutable($modify), $client, null, [] diff --git a/Tests/Acceptance/DoctrineAuthCodeManagerTest.php b/Tests/Acceptance/DoctrineAuthCodeManagerTest.php new file mode 100644 index 00000000..bda19680 --- /dev/null +++ b/Tests/Acceptance/DoctrineAuthCodeManagerTest.php @@ -0,0 +1,83 @@ +client->getContainer()->get('doctrine.orm.entity_manager'); + + $doctrineAuthCodeManager = new DoctrineAuthCodeManager($em); + + $client = new Client('client', 'secret'); + $em->persist($client); + + timecop_freeze(new DateTimeImmutable()); + + try { + $testData = $this->buildClearExpiredTestData($client); + + /** @var AuthorizationCode $authCode */ + foreach ($testData['input'] as $authCode) { + $doctrineAuthCodeManager->save($authCode); + } + + $em->flush(); + + $this->assertSame(3, $doctrineAuthCodeManager->clearExpired()); + } finally { + timecop_return(); + } + + $this->assertSame( + $testData['output'], + $em->getRepository(AuthorizationCode::class)->findBy([], ['identifier' => 'ASC']) + ); + } + + private function buildClearExpiredTestData(Client $client): array + { + $validAuthCodes = [ + $this->buildAuthCode('1111', '+1 day', $client), + $this->buildAuthCode('2222', '+1 hour', $client), + $this->buildAuthCode('3333', '+1 second', $client), + $this->buildAuthCode('4444', 'now', $client), + ]; + + $expiredAuthCodes = [ + $this->buildAuthCode('5555', '-1 day', $client), + $this->buildAuthCode('6666', '-1 hour', $client), + $this->buildAuthCode('7777', '-1 second', $client), + ]; + + return [ + 'input' => array_merge($validAuthCodes, $expiredAuthCodes), + 'output' => $validAuthCodes, + ]; + } + + private function buildAuthCode(string $identifier, string $modify, Client $client): AuthorizationCode + { + return new AuthorizationCode( + $identifier, + new DateTimeImmutable($modify), + $client, + null, + [] + ); + } +} diff --git a/Tests/Acceptance/DoctrineClientManagerTest.php b/Tests/Acceptance/DoctrineClientManagerTest.php new file mode 100644 index 00000000..4c1eda4d --- /dev/null +++ b/Tests/Acceptance/DoctrineClientManagerTest.php @@ -0,0 +1,116 @@ +client->getContainer()->get('doctrine.orm.entity_manager'); + $doctrineClientManager = new DoctrineClientManager($em); + + $client = new Client('client', 'secret'); + $em->persist($client); + $em->flush(); + + $doctrineClientManager->remove($client); + + $this->assertNull( + $em + ->getRepository(Client::class) + ->find($client->getIdentifier()) + ); + } + + public function testClientDeleteCascadesToAccessTokens(): void + { + /** @var $em EntityManagerInterface */ + $em = $this->client->getContainer()->get('doctrine.orm.entity_manager'); + $doctrineClientManager = new DoctrineClientManager($em); + + $client = new Client('client', 'secret'); + $em->persist($client); + $em->flush(); + + $accessToken = new AccessToken('access token', new DateTimeImmutable('+1 day'), $client, $client->getIdentifier(), []); + $em->persist($accessToken); + $em->flush(); + + $doctrineClientManager->remove($client); + + $this->assertNull( + $em + ->getRepository(Client::class) + ->find($client->getIdentifier()) + ); + + // The entity manager has to be cleared manually + // because it doesn't process deep integrity constraints + $em->clear(); + + $this->assertNull( + $em + ->getRepository(AccessToken::class) + ->find($accessToken->getIdentifier()) + ); + } + + public function testClientDeleteCascadesToAccessTokensAndRefreshTokens(): void + { + /** @var $em EntityManagerInterface */ + $em = $this->client->getContainer()->get('doctrine.orm.entity_manager'); + $doctrineClientManager = new DoctrineClientManager($em); + + $client = new Client('client', 'secret'); + $em->persist($client); + $em->flush(); + + $accessToken = new AccessToken('access token', new DateTimeImmutable('+1 day'), $client, $client->getIdentifier(), []); + $em->persist($accessToken); + $em->flush(); + + $refreshToken = new RefreshToken('refresh token', new DateTimeImmutable('+1 day'), $accessToken); + $em->persist($refreshToken); + $em->flush(); + + $doctrineClientManager->remove($client); + + $this->assertNull( + $em + ->getRepository(Client::class) + ->find($client->getIdentifier()) + ); + + // The entity manager has to be cleared manually + // because it doesn't process deep integrity constraints + $em->clear(); + + $this->assertNull( + $em + ->getRepository(AccessToken::class) + ->find($accessToken->getIdentifier()) + ); + + /** @var $refreshToken RefreshToken */ + $refreshToken = $em + ->getRepository(RefreshToken::class) + ->find($refreshToken->getIdentifier()) + ; + $this->assertNotNull($refreshToken); + $this->assertNull($refreshToken->getAccessToken()); + } +} diff --git a/Tests/Acceptance/DoctrineRefreshTokenManagerTest.php b/Tests/Acceptance/DoctrineRefreshTokenManagerTest.php index 7213678e..a3359d87 100644 --- a/Tests/Acceptance/DoctrineRefreshTokenManagerTest.php +++ b/Tests/Acceptance/DoctrineRefreshTokenManagerTest.php @@ -2,22 +2,24 @@ declare(strict_types=1); -namespace Trikoder\Bundle\OAuth2Bundle\Tests\Unit; +namespace Trikoder\Bundle\OAuth2Bundle\Tests\Acceptance; -use DateTime; +use DateTimeImmutable; +use Doctrine\ORM\EntityManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Manager\Doctrine\RefreshTokenManager as DoctrineRefreshTokenManager; use Trikoder\Bundle\OAuth2Bundle\Model\AccessToken; use Trikoder\Bundle\OAuth2Bundle\Model\Client; use Trikoder\Bundle\OAuth2Bundle\Model\RefreshToken; -use Trikoder\Bundle\OAuth2Bundle\Tests\Acceptance\AbstractAcceptanceTest; /** - * @TODO This should be in the Integration tests folder but the current tests infrastructure would need improvements first. + * @TODO This should be in the Integration tests folder but the current tests infrastructure would need improvements first. + * @covers \Trikoder\Bundle\OAuth2Bundle\Manager\Doctrine\RefreshTokenManager */ final class DoctrineRefreshTokenManagerTest extends AbstractAcceptanceTest { public function testClearExpired(): void { + /** @var EntityManagerInterface $em */ $em = $this->client->getContainer()->get('doctrine.orm.entity_manager'); $doctrineRefreshTokenManager = new DoctrineRefreshTokenManager($em); @@ -26,7 +28,7 @@ public function testClearExpired(): void $em->persist($client); $em->flush(); - timecop_freeze(new DateTime()); + timecop_freeze(new DateTimeImmutable()); try { $testData = $this->buildClearExpiredTestData($client); @@ -75,10 +77,10 @@ private function buildRefreshToken(string $identifier, string $modify, Client $c { return new RefreshToken( $identifier, - (new DateTime())->modify($modify), + new DateTimeImmutable($modify), new AccessToken( $identifier, - (new DateTime('+1 day')), + new DateTimeImmutable('+1 day'), $client, null, [] diff --git a/Tests/Acceptance/TokenEndpointTest.php b/Tests/Acceptance/TokenEndpointTest.php index b23d1d95..5af4b8b2 100644 --- a/Tests/Acceptance/TokenEndpointTest.php +++ b/Tests/Acceptance/TokenEndpointTest.php @@ -4,7 +4,7 @@ namespace Trikoder\Bundle\OAuth2Bundle\Tests\Acceptance; -use DateTime; +use DateTimeImmutable; use Trikoder\Bundle\OAuth2Bundle\Event\UserResolveEvent; use Trikoder\Bundle\OAuth2Bundle\Manager\AccessTokenManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Manager\AuthorizationCodeManagerInterface; @@ -31,7 +31,7 @@ protected function setUp(): void public function testSuccessfulClientCredentialsRequest(): void { - timecop_freeze(new DateTime()); + timecop_freeze(new DateTimeImmutable()); try { $this->client->request('POST', '/token', [ @@ -60,11 +60,11 @@ public function testSuccessfulPasswordRequest(): void $this->client ->getContainer() ->get('event_dispatcher') - ->addListener('trikoder.oauth2.user_resolve', function (UserResolveEvent $event): void { + ->addListener('trikoder.oauth2.user_resolve', static function (UserResolveEvent $event): void { $event->setUser(FixtureFactory::createUser()); }); - timecop_freeze(new DateTime()); + timecop_freeze(new DateTimeImmutable()); try { $this->client->request('POST', '/token', [ @@ -98,7 +98,7 @@ public function testSuccessfulRefreshTokenRequest(): void ->get(RefreshTokenManagerInterface::class) ->find(FixtureFactory::FIXTURE_REFRESH_TOKEN); - timecop_freeze(new DateTime()); + timecop_freeze(new DateTimeImmutable()); try { $this->client->request('POST', '/token', [ @@ -131,14 +131,46 @@ public function testSuccessfulAuthorizationCodeRequest(): void ->get(AuthorizationCodeManagerInterface::class) ->find(FixtureFactory::FIXTURE_AUTH_CODE); - timecop_freeze(new DateTime()); + timecop_freeze(new DateTimeImmutable()); try { $this->client->request('POST', '/token', [ - 'client_id' => 'foo', + 'client_id' => FixtureFactory::FIXTURE_CLIENT_FIRST, 'client_secret' => 'secret', 'grant_type' => 'authorization_code', - 'redirect_uri' => 'https://example.org/oauth2/redirect-uri', + 'redirect_uri' => FixtureFactory::FIXTURE_CLIENT_FIRST_REDIRECT_URI, + 'code' => TestHelper::generateEncryptedAuthCodePayload($authCode), + ]); + } finally { + timecop_return(); + } + + $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->assertSame(3600, $jsonResponse['expires_in']); + $this->assertNotEmpty($jsonResponse['access_token']); + } + + public function testSuccessfulAuthorizationCodeRequestWithPublicClient(): void + { + $authCode = $this->client + ->getContainer() + ->get(AuthorizationCodeManagerInterface::class) + ->find(FixtureFactory::FIXTURE_AUTH_CODE_PUBLIC_CLIENT); + + timecop_freeze(new DateTimeImmutable()); + + try { + $this->client->request('POST', '/token', [ + 'client_id' => FixtureFactory::FIXTURE_PUBLIC_CLIENT, + 'grant_type' => 'authorization_code', + 'redirect_uri' => FixtureFactory::FIXTURE_PUBLIC_CLIENT_REDIRECT_URI, 'code' => TestHelper::generateEncryptedAuthCodePayload($authCode), ]); } finally { @@ -157,7 +189,41 @@ public function testSuccessfulAuthorizationCodeRequest(): void $this->assertNotEmpty($jsonResponse['access_token']); } - public function testFailedTokenRequest(): void + public function testSuccessfulAuthorizationCodeOpenIDRequest() + { + $authCode = $this->client + ->getContainer() + ->get(AuthorizationCodeManagerInterface::class) + ->find(FixtureFactory::FIXTURE_AUTH_CODE_OPENID); + + timecop_freeze(new DateTimeImmutable()); + + try { + $this->client->request('POST', '/token', [ + 'client_id' => FixtureFactory::FIXTURE_CLIENT_FIRST, + 'client_secret' => 'secret', + 'grant_type' => 'authorization_code', + 'redirect_uri' => FixtureFactory::FIXTURE_CLIENT_FIRST_REDIRECT_URI, + 'code' => TestHelper::generateEncryptedAuthCodePayload($authCode, false), + ]); + } finally { + timecop_return(); + } + + $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->assertSame(3600, $jsonResponse['expires_in']); + $this->assertNotEmpty($jsonResponse['access_token']); + $this->assertNotEmpty($jsonResponse['id_token']); + } + + public function testFailedTokenRequest() { $this->client->request('POST', '/token'); diff --git a/Tests/Acceptance/UserInfoEndpointTest.php b/Tests/Acceptance/UserInfoEndpointTest.php new file mode 100644 index 00000000..c41ea5b2 --- /dev/null +++ b/Tests/Acceptance/UserInfoEndpointTest.php @@ -0,0 +1,91 @@ +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) + ); + } + + public function testSuccessfulGetUserInfoRequest() + { + $accessToken = $this->client->getContainer()->get(AccessTokenManagerInterface::class) + ->find(FixtureFactory::FIXTURE_ACCESS_TOKEN_USER_BOUND); + + $this->client->request('GET', '/userinfo', [], [], [ + 'HTTP_AUTHORIZATION' => TestHelper::generateJwtToken($accessToken), + ]); + + $response = $this->client->getResponse(); + + $this->assertSame(200, $response->getStatusCode()); + $this->assertStringStartsWith('application/json', $response->headers->get('Content-Type')); + + $jsonResponse = json_decode($response->getContent(), true); + $this->assertEquals('user', $jsonResponse['sub']); + } + + public function testSuccessfulPostUserInfoRequest() + { + $accessToken = $this->client->getContainer()->get(AccessTokenManagerInterface::class) + ->find(FixtureFactory::FIXTURE_ACCESS_TOKEN_USER_BOUND); + + $this->client->request('POST', '/userinfo', [ + 'access_token' => TestHelper::generateJwtToken($accessToken), + ]); + + $response = $this->client->getResponse(); + + $this->assertSame(200, $response->getStatusCode()); + $this->assertStringStartsWith('application/json', $response->headers->get('Content-Type')); + + $jsonResponse = json_decode($response->getContent(), true); + $this->assertEquals('user', $jsonResponse['sub']); + } + + public function testUnauthorizedGetUserInfoRequest() + { + $accessToken = $this->client->getContainer()->get(AccessTokenManagerInterface::class) + ->find(FixtureFactory::FIXTURE_ACCESS_TOKEN_EXPIRED); + + $this->client->request('GET', '/userinfo', [], [], [ + 'HTTP_AUTHORIZATION' => TestHelper::generateJwtToken($accessToken), + ]); + + $response = $this->client->getResponse(); + + $this->assertSame(401, $response->getStatusCode()); + } + + public function testUnauthorizedPostUserInfoRequest() + { + $accessToken = $this->client->getContainer()->get(AccessTokenManagerInterface::class) + ->find(FixtureFactory::FIXTURE_ACCESS_TOKEN_REVOKED); + + $this->client->request('POST', '/userinfo', [ + 'access_token' => TestHelper::generateJwtToken($accessToken), + ]); + + $response = $this->client->getResponse(); + + $this->assertSame(401, $response->getStatusCode()); + } +} diff --git a/Tests/Fixtures/FixtureFactory.php b/Tests/Fixtures/FixtureFactory.php index 877efa1a..34d2acde 100644 --- a/Tests/Fixtures/FixtureFactory.php +++ b/Tests/Fixtures/FixtureFactory.php @@ -4,7 +4,7 @@ namespace Trikoder\Bundle\OAuth2Bundle\Tests\Fixtures; -use DateTime; +use DateTimeImmutable; use Trikoder\Bundle\OAuth2Bundle\Manager\AccessTokenManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Manager\AuthorizationCodeManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Manager\ClientManagerInterface; @@ -41,19 +41,26 @@ final class FixtureFactory public const FIXTURE_REFRESH_TOKEN_WITH_SCOPES = 'e47d593ed661840b3633e4577c3261ef57ba225be193b190deb69ee9afefdc19f54f890fbdda59f5'; public const FIXTURE_AUTH_CODE = '0aa70e8152259988b3c8e9e8cff604019bb986eb226bd126da189829b95a2be631e2506042064e12'; + public const FIXTURE_AUTH_CODE_PUBLIC_CLIENT = 'xaa70e8152259988b3c8e9e8cff604019bb986eb226bd126da189829b95a2be631e2506042064e12'; public const FIXTURE_AUTH_CODE_DIFFERENT_CLIENT = 'e8fe264053cb346f4437af05c8cc9036931cfec3a0d5b54bdae349304ca4a83fd2f4590afd51e559'; public const FIXTURE_AUTH_CODE_EXPIRED = 'a7bdbeb26c9f095d842f5e5b8e313b24318d6b26728d1c543136727bbe9525f7a7930305a09b7401'; + public const FIXTURE_AUTH_CODE_OPENID = '86adfc23d7b07ba70b9a501c03ff9fafb967efb2b4b14099feced03a14430c3c00a69b3282f769a8'; public const FIXTURE_CLIENT_FIRST = 'foo'; public const FIXTURE_CLIENT_SECOND = 'bar'; public const FIXTURE_CLIENT_INACTIVE = 'baz_inactive'; public const FIXTURE_CLIENT_RESTRICTED_GRANTS = 'qux_restricted_grants'; public const FIXTURE_CLIENT_RESTRICTED_SCOPES = 'quux_restricted_scopes'; + public const FIXTURE_PUBLIC_CLIENT = 'foo_public'; + public const FIXTURE_PUBLIC_CLIENT_ALLOWED_TO_USE_PLAIN_CHALLENGE_METHOD = 'bar_public'; public const FIXTURE_CLIENT_FIRST_REDIRECT_URI = 'https://example.org/oauth2/redirect-uri'; + public const FIXTURE_PUBLIC_CLIENT_REDIRECT_URI = 'https://example.org/oauth2/redirect-uri-foo-test'; + public const FIXTURE_PUBLIC_CLIENT_ALLOWED_TO_USE_PLAIN_CHALLENGE_METHOD_REDIRECT_URI = 'https://example.org/oauth2/redirect-uri-bar-test'; public const FIXTURE_SCOPE_FIRST = 'fancy'; public const FIXTURE_SCOPE_SECOND = 'rock'; + public const FIXTURE_SCOPE_OPENID = 'openid'; public const FIXTURE_USER = 'user'; public const FIXTURE_PASSWORD = 'password'; @@ -103,7 +110,7 @@ private static function createAccessTokens(ScopeManagerInterface $scopeManager, $accessTokens[] = (new AccessToken( self::FIXTURE_ACCESS_TOKEN_USER_BOUND, - new DateTime('+1 hour'), + new DateTimeImmutable('+1 hour'), $clientManager->find(self::FIXTURE_CLIENT_FIRST), self::FIXTURE_USER, [] @@ -111,7 +118,7 @@ private static function createAccessTokens(ScopeManagerInterface $scopeManager, $accessTokens[] = (new AccessToken( self::FIXTURE_ACCESS_TOKEN_DIFFERENT_CLIENT, - new DateTime('+1 hour'), + new DateTimeImmutable('+1 hour'), $clientManager->find(self::FIXTURE_CLIENT_SECOND), self::FIXTURE_USER, [] @@ -119,7 +126,7 @@ private static function createAccessTokens(ScopeManagerInterface $scopeManager, $accessTokens[] = (new AccessToken( self::FIXTURE_ACCESS_TOKEN_EXPIRED, - new DateTime('-1 hour'), + new DateTimeImmutable('-1 hour'), $clientManager->find(self::FIXTURE_CLIENT_FIRST), self::FIXTURE_USER, [] @@ -127,7 +134,7 @@ private static function createAccessTokens(ScopeManagerInterface $scopeManager, $accessTokens[] = (new AccessToken( self::FIXTURE_ACCESS_TOKEN_REVOKED, - new DateTime('+1 hour'), + new DateTimeImmutable('+1 hour'), $clientManager->find(self::FIXTURE_CLIENT_FIRST), self::FIXTURE_USER, [] @@ -136,7 +143,7 @@ private static function createAccessTokens(ScopeManagerInterface $scopeManager, $accessTokens[] = new AccessToken( self::FIXTURE_ACCESS_TOKEN_PUBLIC, - new DateTime('+1 hour'), + new DateTimeImmutable('+1 hour'), $clientManager->find(self::FIXTURE_CLIENT_FIRST), null, [] @@ -144,7 +151,7 @@ private static function createAccessTokens(ScopeManagerInterface $scopeManager, $accessTokens[] = (new AccessToken( self::FIXTURE_ACCESS_TOKEN_WITH_SCOPES, - new DateTime('+1 hour'), + new DateTimeImmutable('+1 hour'), $clientManager->find(self::FIXTURE_CLIENT_FIRST), null, [$scopeManager->find(self::FIXTURE_SCOPE_FIRST)] @@ -152,7 +159,7 @@ private static function createAccessTokens(ScopeManagerInterface $scopeManager, $accessTokens[] = (new AccessToken( self::FIXTURE_ACCESS_TOKEN_USER_BOUND_WITH_SCOPES, - new DateTime('+1 hour'), + new DateTimeImmutable('+1 hour'), $clientManager->find(self::FIXTURE_CLIENT_FIRST), self::FIXTURE_USER, [$scopeManager->find(self::FIXTURE_SCOPE_FIRST)] @@ -170,32 +177,32 @@ private static function createRefreshTokens(AccessTokenManagerInterface $accessT $refreshTokens[] = new RefreshToken( self::FIXTURE_REFRESH_TOKEN, - new DateTime('+1 month'), + new DateTimeImmutable('+1 month'), $accessTokenManager->find(self::FIXTURE_ACCESS_TOKEN_USER_BOUND) ); $refreshTokens[] = new RefreshToken( self::FIXTURE_REFRESH_TOKEN_DIFFERENT_CLIENT, - new DateTime('+1 month'), + new DateTimeImmutable('+1 month'), $accessTokenManager->find(self::FIXTURE_ACCESS_TOKEN_DIFFERENT_CLIENT) ); $refreshTokens[] = new RefreshToken( self::FIXTURE_REFRESH_TOKEN_EXPIRED, - new DateTime('-1 month'), + new DateTimeImmutable('-1 month'), $accessTokenManager->find(self::FIXTURE_ACCESS_TOKEN_EXPIRED) ); $refreshTokens[] = (new RefreshToken( self::FIXTURE_REFRESH_TOKEN_REVOKED, - new DateTime('+1 month'), + new DateTimeImmutable('+1 month'), $accessTokenManager->find(self::FIXTURE_ACCESS_TOKEN_REVOKED) )) ->revoke(); $refreshTokens[] = new RefreshToken( self::FIXTURE_REFRESH_TOKEN_WITH_SCOPES, - new DateTime('+1 month'), + new DateTimeImmutable('+1 month'), $accessTokenManager->find(self::FIXTURE_ACCESS_TOKEN_USER_BOUND_WITH_SCOPES) ); @@ -211,15 +218,23 @@ public static function createAuthorizationCodes(ClientManagerInterface $clientMa $authorizationCodes[] = new AuthorizationCode( self::FIXTURE_AUTH_CODE, - new DateTime('+2 minute'), + new DateTimeImmutable('+2 minute'), $clientManager->find(self::FIXTURE_CLIENT_FIRST), self::FIXTURE_USER, [] ); + $authorizationCodes[] = new AuthorizationCode( + self::FIXTURE_AUTH_CODE_PUBLIC_CLIENT, + new DateTimeImmutable('+2 minute'), + $clientManager->find(self::FIXTURE_PUBLIC_CLIENT), + self::FIXTURE_USER, + [] + ); + $authorizationCodes[] = new AuthorizationCode( self::FIXTURE_AUTH_CODE_DIFFERENT_CLIENT, - new DateTime('+2 minute'), + new DateTimeImmutable('+2 minute'), $clientManager->find(self::FIXTURE_CLIENT_SECOND), self::FIXTURE_USER, [] @@ -227,12 +242,21 @@ public static function createAuthorizationCodes(ClientManagerInterface $clientMa $authorizationCodes[] = new AuthorizationCode( self::FIXTURE_AUTH_CODE_EXPIRED, - new DateTime('-30 minute'), + new DateTimeImmutable('-30 minute'), $clientManager->find(self::FIXTURE_CLIENT_FIRST), self::FIXTURE_USER, [] + + ); + $authorizationCodes[] = new AuthorizationCode( + self::FIXTURE_AUTH_CODE_OPENID, + new DateTimeImmutable('+2 minute'), + $clientManager->find(self::FIXTURE_CLIENT_FIRST), + self::FIXTURE_USER, + [new Scope(self::FIXTURE_SCOPE_OPENID)] ); + return $authorizationCodes; } @@ -257,6 +281,13 @@ private static function createClients(): array $clients[] = (new Client(self::FIXTURE_CLIENT_RESTRICTED_SCOPES, 'beer')) ->setScopes(new Scope(self::FIXTURE_SCOPE_SECOND)); + $clients[] = (new Client(self::FIXTURE_PUBLIC_CLIENT, null)) + ->setRedirectUris(new RedirectUri(self::FIXTURE_PUBLIC_CLIENT_REDIRECT_URI)); + + $clients[] = (new Client(self::FIXTURE_PUBLIC_CLIENT_ALLOWED_TO_USE_PLAIN_CHALLENGE_METHOD, null)) + ->setAllowPlainTextPkce(true) + ->setRedirectUris(new RedirectUri(self::FIXTURE_PUBLIC_CLIENT_ALLOWED_TO_USE_PLAIN_CHALLENGE_METHOD_REDIRECT_URI)); + return $clients; } @@ -269,6 +300,7 @@ private static function createScopes(): array $scopes[] = new Scope(self::FIXTURE_SCOPE_FIRST); $scopes[] = new Scope(self::FIXTURE_SCOPE_SECOND); + $scopes[] = new Scope(self::FIXTURE_SCOPE_OPENID); return $scopes; } diff --git a/Tests/Fixtures/SecurityTestController.php b/Tests/Fixtures/SecurityTestController.php index 4a533598..2384bbdc 100644 --- a/Tests/Fixtures/SecurityTestController.php +++ b/Tests/Fixtures/SecurityTestController.php @@ -6,11 +6,21 @@ use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\HttpFoundation\Response; -use Symfony\Component\Security\Core\Role\Role; +use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; use Symfony\Component\Security\Core\User\UserInterface; final class SecurityTestController extends AbstractController { + /** + * @var TokenStorageInterface + */ + private $tokenStorage; + + public function __construct(TokenStorageInterface $tokenStorage) + { + $this->tokenStorage = $tokenStorage; + } + public function helloAction(): Response { /** @var UserInterface $user */ @@ -28,11 +38,7 @@ public function scopeAction(): Response public function rolesAction(): Response { - $roles = $this->get('security.token_storage')->getToken()->getRoles(); - - $roles = array_map(function (Role $role): string { - return $role->getRole(); - }, $roles); + $roles = $this->tokenStorage->getToken()->getRoleNames(); return new Response( sprintf( diff --git a/Tests/Integration/AbstractIntegrationTest.php b/Tests/Integration/AbstractIntegrationTest.php index bba1fbd5..0b295daf 100644 --- a/Tests/Integration/AbstractIntegrationTest.php +++ b/Tests/Integration/AbstractIntegrationTest.php @@ -7,6 +7,8 @@ use DateInterval; use Defuse\Crypto\Crypto; use Defuse\Crypto\Exception\CryptoException; +use Lcobucci\JWT\Parser; +use Lcobucci\JWT\Token; use League\OAuth2\Server\AuthorizationServer; use League\OAuth2\Server\CryptKey; use League\OAuth2\Server\Exception\OAuthServerException; @@ -21,13 +23,16 @@ use League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface; use League\OAuth2\Server\Repositories\ScopeRepositoryInterface; use League\OAuth2\Server\Repositories\UserRepositoryInterface; +use OpenIDConnectServer\ClaimExtractor; +use OpenIDConnectServer\IdTokenResponse; +use OpenIDConnectServer\Repositories\IdentityProviderInterface; use League\OAuth2\Server\ResourceServer; use Nyholm\Psr7\Factory\Psr17Factory; +use PHPUnit\Framework\TestCase; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; -use Symfony\Bundle\FrameworkBundle\Tests\TestCase; use Symfony\Component\EventDispatcher\EventDispatcher; -use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; +use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Trikoder\Bundle\OAuth2Bundle\Converter\ScopeConverter; use Trikoder\Bundle\OAuth2Bundle\Converter\UserConverter; use Trikoder\Bundle\OAuth2Bundle\League\Entity\User; @@ -47,9 +52,10 @@ use Trikoder\Bundle\OAuth2Bundle\Manager\InMemory\ScopeManager; use Trikoder\Bundle\OAuth2Bundle\Manager\RefreshTokenManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Manager\ScopeManagerInterface; +use Trikoder\Bundle\OAuth2Bundle\OpenIDConnect\Repository\IdentityProvider; +use Trikoder\Bundle\OAuth2Bundle\Tests\Fixtures\FixtureFactory; use Trikoder\Bundle\OAuth2Bundle\Model\AccessToken; use Trikoder\Bundle\OAuth2Bundle\Model\RefreshToken; -use Trikoder\Bundle\OAuth2Bundle\Service\BCEventDispatcher; use Trikoder\Bundle\OAuth2Bundle\Tests\TestHelper; abstract class AbstractIntegrationTest extends TestCase @@ -99,6 +105,11 @@ abstract class AbstractIntegrationTest extends TestCase */ private $psrFactory; + /** + * @var bool + */ + private $requireCodeChallengeForPublicClients = true; + /** * {@inheritdoc} */ @@ -109,13 +120,14 @@ protected function setUp(): void $this->accessTokenManager = new AccessTokenManager(); $this->refreshTokenManager = new RefreshTokenManager(); $this->authCodeManager = new AuthorizationCodeManager(); - $this->eventDispatcher = new BCEventDispatcher(new EventDispatcher()); + $this->eventDispatcher = new EventDispatcher(); $scopeConverter = new ScopeConverter(); $scopeRepository = new ScopeRepository($this->scopeManager, $this->clientManager, $scopeConverter, $this->eventDispatcher); $clientRepository = new ClientRepository($this->clientManager); $accessTokenRepository = new AccessTokenRepository($this->accessTokenManager, $this->clientManager, $scopeConverter); $refreshTokenRepository = new RefreshTokenRepository($this->refreshTokenManager, $this->accessTokenManager); + $identityRepository = new IdentityProvider($this->eventDispatcher); $userConverter = new UserConverter(); $userRepository = new UserRepository($this->clientManager, $this->eventDispatcher, $userConverter); $authCodeRepository = new AuthCodeRepository($this->authCodeManager, $this->clientManager, $scopeConverter); @@ -126,7 +138,8 @@ protected function setUp(): void $accessTokenRepository, $refreshTokenRepository, $userRepository, - $authCodeRepository + $authCodeRepository, + $identityRepository ); $this->resourceServer = $this->createResourceServer($accessTokenRepository); @@ -156,14 +169,17 @@ protected function getRefreshToken(string $encryptedPayload): ?RefreshToken } catch (CryptoException $e) { return null; } - $payload = json_decode($payload, true); - return $this->refreshTokenManager->find( $payload['refresh_token_id'] ); } + protected function getIdToken(string $jwtToken): Token + { + return (new Parser())->parse($jwtToken); + } + protected function createAuthorizationRequest(?string $credentials, array $body = []): ServerRequestInterface { $request = $this @@ -252,26 +268,44 @@ protected function extractQueryDataFromUri(string $uri): array return $data; } + protected function enableRequireCodeChallengeForPublicClients(): void + { + $this->requireCodeChallengeForPublicClients = true; + } + + protected function disableRequireCodeChallengeForPublicClients(): void + { + $this->requireCodeChallengeForPublicClients = false; + } + private function createAuthorizationServer( ScopeRepositoryInterface $scopeRepository, ClientRepositoryInterface $clientRepository, AccessTokenRepositoryInterface $accessTokenRepository, RefreshTokenRepositoryInterface $refreshTokenRepository, UserRepositoryInterface $userRepository, - AuthCodeRepositoryInterface $authCodeRepository + AuthCodeRepositoryInterface $authCodeRepository, + IdentityProviderInterface $identityRepository ): AuthorizationServer { $authorizationServer = new AuthorizationServer( $clientRepository, $accessTokenRepository, $scopeRepository, new CryptKey(TestHelper::PRIVATE_KEY_PATH, null, false), - TestHelper::ENCRYPTION_KEY + TestHelper::ENCRYPTION_KEY, + new IdTokenResponse($identityRepository, new ClaimExtractor()) ); + $authCodeGrant = new AuthCodeGrant($authCodeRepository, $refreshTokenRepository, new DateInterval('PT10M')); + + if (!$this->requireCodeChallengeForPublicClients) { + $authCodeGrant->disableRequireCodeChallengeForPublicClients(); + } + $authorizationServer->enableGrantType(new ClientCredentialsGrant()); $authorizationServer->enableGrantType(new RefreshTokenGrant($refreshTokenRepository)); $authorizationServer->enableGrantType(new PasswordGrant($userRepository, $refreshTokenRepository)); - $authorizationServer->enableGrantType(new AuthCodeGrant($authCodeRepository, $refreshTokenRepository, new DateInterval('PT10M'))); + $authorizationServer->enableGrantType($authCodeGrant); $authorizationServer->enableGrantType(new ImplicitGrant(new DateInterval('PT10M'))); return $authorizationServer; diff --git a/Tests/Integration/AuthCodeRepositoryTest.php b/Tests/Integration/AuthCodeRepositoryTest.php new file mode 100644 index 00000000..bd090d3c --- /dev/null +++ b/Tests/Integration/AuthCodeRepositoryTest.php @@ -0,0 +1,38 @@ +authCodeManager->save($authCode); + + $this->assertSame($authCode, $this->authCodeManager->find($identifier)); + + $authCodeRepository = new AuthCodeRepository($this->authCodeManager, $this->clientManager, new ScopeConverter()); + + $authCodeRepository->revokeAuthCode($identifier); + + $this->assertTrue($authCode->isRevoked()); + $this->assertSame($authCode, $this->authCodeManager->find($identifier)); + } +} diff --git a/Tests/Integration/AuthorizationServerTest.php b/Tests/Integration/AuthorizationServerTest.php index 65934beb..cc1e9f40 100644 --- a/Tests/Integration/AuthorizationServerTest.php +++ b/Tests/Integration/AuthorizationServerTest.php @@ -4,7 +4,7 @@ namespace Trikoder\Bundle\OAuth2Bundle\Tests\Integration; -use DateTime; +use DateTimeImmutable; use Trikoder\Bundle\OAuth2Bundle\Event\UserResolveEvent; use Trikoder\Bundle\OAuth2Bundle\Model\AccessToken; use Trikoder\Bundle\OAuth2Bundle\Model\RefreshToken; @@ -168,7 +168,7 @@ public function testValidClientCredentialsGrant(): void 'grant_type' => 'client_credentials', ]); - timecop_freeze(new DateTime()); + timecop_freeze(new DateTimeImmutable()); try { $response = $this->handleTokenRequest($request); @@ -194,7 +194,7 @@ public function testValidClientCredentialsGrantWithScope(): void 'scope' => 'fancy', ]); - timecop_freeze(new DateTime()); + timecop_freeze(new DateTimeImmutable()); try { $response = $this->handleTokenRequest($request); @@ -227,7 +227,7 @@ public function testValidClientCredentialsGrantWithInheritedScope(): void 'grant_type' => 'client_credentials', ]); - timecop_freeze(new DateTime()); + timecop_freeze(new DateTimeImmutable()); try { $response = $this->handleTokenRequest($request); @@ -261,7 +261,7 @@ public function testValidClientCredentialsGrantWithRequestedScope(): void 'scope' => 'rock', ]); - timecop_freeze(new DateTime()); + timecop_freeze(new DateTimeImmutable()); try { $response = $this->handleTokenRequest($request); @@ -290,7 +290,7 @@ public function testValidClientCredentialsGrantWithRequestedScope(): void public function testValidPasswordGrant(): void { - $this->eventDispatcher->addListener('trikoder.oauth2.user_resolve', function (UserResolveEvent $event): void { + $this->eventDispatcher->addListener('trikoder.oauth2.user_resolve', static function (UserResolveEvent $event): void { $event->setUser(FixtureFactory::createUser()); }); @@ -300,7 +300,7 @@ public function testValidPasswordGrant(): void 'password' => 'pass', ]); - timecop_freeze(new DateTime()); + timecop_freeze(new DateTimeImmutable()); try { $response = $this->handleTokenRequest($request); @@ -326,7 +326,7 @@ public function testValidPasswordGrant(): void public function testInvalidCredentialsPasswordGrant(): void { - $this->eventDispatcher->addListener('trikoder.oauth2.user_resolve', function (UserResolveEvent $event): void { + $this->eventDispatcher->addListener('trikoder.oauth2.user_resolve', static function (UserResolveEvent $event): void { $event->setUser(null); }); @@ -339,8 +339,8 @@ public function testInvalidCredentialsPasswordGrant(): void $response = $this->handleTokenRequest($request); // Response assertions. - $this->assertSame('invalid_credentials', $response['error']); - $this->assertSame('The user credentials were incorrect.', $response['message']); + $this->assertSame('invalid_grant', $response['error']); + $this->assertSame('The provided authorization grant (e.g., authorization code, resource owner credentials) or refresh token is invalid, expired, revoked, does not match the redirection URI used in the authorization request, or was issued to another client.', $response['message']); } public function testMissingUsernameFieldPasswordGrant(): void @@ -383,7 +383,7 @@ public function testValidRefreshGrant(): void 'refresh_token' => TestHelper::generateEncryptedPayload($existingRefreshToken), ]); - timecop_freeze(new DateTime()); + timecop_freeze(new DateTimeImmutable()); try { $response = $this->handleTokenRequest($request); @@ -675,7 +675,7 @@ public function testSuccessfulAuthorizationWithCode(): void 'redirect_uri' => 'https://example.org/oauth2/redirect-uri', ]); - timecop_freeze(new DateTime()); + timecop_freeze(new DateTimeImmutable()); try { $response = $this->handleTokenRequest($request); @@ -751,7 +751,7 @@ public function testSuccessfulImplicitRequest(): void 'client_id' => 'foo', ]); - timecop_freeze(new DateTime()); + timecop_freeze(new DateTimeImmutable()); try { $response = $this->handleAuthorizationRequest($request); @@ -779,7 +779,7 @@ public function testSuccessfulImplicitRequestWithState(): void 'state' => 'quzbaz', ]); - timecop_freeze(new DateTime()); + timecop_freeze(new DateTimeImmutable()); try { $response = $this->handleAuthorizationRequest($request); @@ -808,7 +808,7 @@ public function testSuccessfulImplicitRequestRedirectUri(): void 'redirect_uri' => 'https://example.org/oauth2/redirect-uri', ]); - timecop_freeze(new DateTime()); + timecop_freeze(new DateTimeImmutable()); try { $response = $this->handleAuthorizationRequest($request); diff --git a/Tests/Integration/OpenIDProviderTest.php b/Tests/Integration/OpenIDProviderTest.php new file mode 100644 index 00000000..0d43758d --- /dev/null +++ b/Tests/Integration/OpenIDProviderTest.php @@ -0,0 +1,48 @@ +scopeManager, + $this->clientManager, + $this->accessTokenManager, + $this->refreshTokenManager, + $this->authCodeManager + ); + } + + public function testSuccessfulIDTokenRequest(): void + { + $openIdAuthCode = $this->authCodeManager->find(FixtureFactory::FIXTURE_AUTH_CODE_OPENID); + + $request = $this->createAuthorizationRequest('foo:secret', [ + 'grant_type' => 'authorization_code', + 'code' => TestHelper::generateEncryptedAuthCodePayload($openIdAuthCode, false), + 'redirect_uri' => 'https://example.org/oauth2/redirect-uri', + ]); + + timecop_freeze(new DateTime()); + $response = $this->handleAuthorizationRequest($request); + $issuedAtTimestamp = time(); + $expirationTimestamp = strtotime('+3600 sec'); + timecop_return(); + + $this->assertArrayHasKey('id_token', $response); + $idToken = $this->getIdToken($response['id_token']); + $this->assertSame('http://', $idToken->getClaim('iss')); + $this->assertSame('user', $idToken->getClaim('sub')); + $this->assertSame('foo', $idToken->getClaim('aud')); + $this->assertEquals($expirationTimestamp, $idToken->getClaim('exp')); + $this->assertEquals($issuedAtTimestamp, $idToken->getClaim('iat')); + } +} diff --git a/Tests/Support/SqlitePlatform.php b/Tests/Support/SqlitePlatform.php new file mode 100644 index 00000000..37372108 --- /dev/null +++ b/Tests/Support/SqlitePlatform.php @@ -0,0 +1,38 @@ + $authCode->getClient()->getIdentifier(), 'redirect_uri' => (string) $authCode->getClient()->getRedirectUris()[0], 'auth_code_id' => $authCode->getIdentifier(), - 'scopes' => (new ScopeConverter())->toDomainArray($authCode->getScopes()), + 'scopes' => $convertScopes ? (new ScopeConverter())->toDomainArray($authCode->getScopes()) : $authCode->getScopes(), 'user_id' => $authCode->getUserIdentifier(), 'expire_time' => $authCode->getExpiryDateTime()->getTimestamp(), 'code_challenge' => null, @@ -62,6 +63,15 @@ public static function generateEncryptedAuthCodePayload(AuthorizationCodeModel $ } } + public static function decryptPayload(string $payload): ?string + { + try { + return Crypto::decryptWithPassword($payload, self::ENCRYPTION_KEY); + } catch (CryptoException $e) { + return null; + } + } + public static function generateJwtToken(AccessTokenModel $accessToken): string { $clientEntity = new ClientEntity(); @@ -69,6 +79,7 @@ public static function generateJwtToken(AccessTokenModel $accessToken): string $clientEntity->setRedirectUri(array_map('strval', $accessToken->getClient()->getRedirectUris())); $accessTokenEntity = new AccessTokenEntity(); + $accessTokenEntity->setPrivateKey(new CryptKey(self::PRIVATE_KEY_PATH, null, false)); $accessTokenEntity->setIdentifier($accessToken->getIdentifier()); $accessTokenEntity->setExpiryDateTime($accessToken->getExpiry()); $accessTokenEntity->setClient($clientEntity); @@ -81,11 +92,12 @@ public static function generateJwtToken(AccessTokenModel $accessToken): string $accessTokenEntity->addScope($scopeEntity); } - return (string) $accessTokenEntity->convertToJWT( - new CryptKey(self::PRIVATE_KEY_PATH, null, false) - ); + return (string) $accessTokenEntity; } + /** + * @throws Exception + */ public static function initializeDoctrineSchema(Application $application, array $arguments = []): bool { $statusCode = $application diff --git a/Tests/TestKernel.php b/Tests/TestKernel.php index 392970cf..2dbf713e 100644 --- a/Tests/TestKernel.php +++ b/Tests/TestKernel.php @@ -4,6 +4,10 @@ namespace Trikoder\Bundle\OAuth2Bundle\Tests; +use Laminas\Diactoros\ResponseFactory; +use Laminas\Diactoros\ServerRequestFactory; +use Laminas\Diactoros\StreamFactory; +use Laminas\Diactoros\UploadedFileFactory; use LogicException; use Nyholm\Psr7\Factory as Nyholm; use Psr\Http\Message\ResponseFactoryInterface; @@ -11,6 +15,7 @@ use Psr\Http\Message\StreamFactoryInterface; use Psr\Http\Message\UploadedFileFactoryInterface; use Symfony\Bundle\FrameworkBundle\Kernel\MicroKernelTrait; +use Symfony\Component\Config\Exception\LoaderLoadException; use Symfony\Component\Config\Loader\LoaderInterface; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -22,9 +27,10 @@ use Trikoder\Bundle\OAuth2Bundle\Manager\ClientManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Manager\RefreshTokenManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Manager\ScopeManagerInterface; +use Trikoder\Bundle\OAuth2Bundle\Model\AuthorizationDecision\AlwaysAllowDecisionStrategy; use Trikoder\Bundle\OAuth2Bundle\Tests\Fixtures\FixtureFactory; use Trikoder\Bundle\OAuth2Bundle\Tests\Fixtures\SecurityTestController; -use Zend\Diactoros as ZendFramework; +use Trikoder\Bundle\OAuth2Bundle\Tests\Support\SqlitePlatform; final class TestKernel extends Kernel implements CompilerPassInterface { @@ -97,6 +103,8 @@ protected function getContainerClass() /** * {@inheritdoc} + * + * @throws LoaderLoadException */ protected function configureRoutes(RouteCollectionBuilder $routes) { @@ -131,6 +139,7 @@ protected function configureContainer(ContainerBuilder $container, LoaderInterfa 'charset' => 'utf8mb4', 'utf8mb4_unicode_ci' => 'utf8mb4_unicode_ci', ], + 'platform_service' => SqlitePlatform::class, ], 'orm' => null, ]); @@ -138,6 +147,9 @@ protected function configureContainer(ContainerBuilder $container, LoaderInterfa $container->loadFromExtension('framework', [ 'secret' => 'nope', 'test' => null, + 'session' => [ + 'storage_id' => 'session.storage.mock_file' + ] ]); $container->loadFromExtension('security', [ @@ -171,6 +183,8 @@ protected function configureContainer(ContainerBuilder $container, LoaderInterfa 'authorization_server' => [ 'private_key' => '%env(PRIVATE_KEY_PATH)%', 'encryption_key' => '%env(ENCRYPTION_KEY)%', + 'authorization_strategy' => AlwaysAllowDecisionStrategy::class, + 'consent_route' => 'default' ], 'resource_server' => [ 'public_key' => '%env(PUBLIC_KEY_PATH)%', @@ -183,17 +197,22 @@ protected function configureContainer(ContainerBuilder $container, LoaderInterfa 'entity_manager' => 'default', ], ], + 'openid_connect' => [ + 'enabled' => false, + 'login_route' => 'default' + ], ]); $this->configureControllers($container); $this->configurePsrHttpFactory($container); + $this->configureDatabaseServices($container); } private function exposeManagerServices(ContainerBuilder $container): void { $container ->getDefinition( - $container + (string) $container ->getAlias(ScopeManagerInterface::class) ->setPublic(true) ) @@ -202,7 +221,7 @@ private function exposeManagerServices(ContainerBuilder $container): void $container ->getDefinition( - $container + (string) $container ->getAlias(ClientManagerInterface::class) ->setPublic(true) ) @@ -211,7 +230,7 @@ private function exposeManagerServices(ContainerBuilder $container): void $container ->getDefinition( - $container + (string) $container ->getAlias(AccessTokenManagerInterface::class) ->setPublic(true) ) @@ -220,7 +239,7 @@ private function exposeManagerServices(ContainerBuilder $container): void $container ->getDefinition( - $container + (string) $container ->getAlias(RefreshTokenManagerInterface::class) ->setPublic(true) ) @@ -229,7 +248,7 @@ private function exposeManagerServices(ContainerBuilder $container): void $container ->getDefinition( - $container + (string) $container ->getAlias(AuthorizationCodeManagerInterface::class) ->setPublic(true) ) @@ -241,10 +260,10 @@ private function configurePsrHttpFactory(ContainerBuilder $container): void { switch ($this->psrHttpProvider) { case self::PSR_HTTP_PROVIDER_ZENDFRAMEWORK: - $serverRequestFactory = ZendFramework\ServerRequestFactory::class; - $streamFactory = ZendFramework\StreamFactory::class; - $uploadedFileFactory = ZendFramework\UploadedFileFactory::class; - $responseFactory = ZendFramework\ResponseFactory::class; + $serverRequestFactory = ServerRequestFactory::class; + $streamFactory = StreamFactory::class; + $uploadedFileFactory = UploadedFileFactory::class; + $responseFactory = ResponseFactory::class; break; case self::PSR_HTTP_PROVIDER_NYHOLM: $serverRequestFactory = Nyholm\Psr17Factory::class; @@ -253,9 +272,7 @@ private function configurePsrHttpFactory(ContainerBuilder $container): void $responseFactory = Nyholm\Psr17Factory::class; break; default: - throw new LogicException( - sprintf('PSR HTTP factory provider \'%s\' is not supported.', $this->psrHttpProvider) - ); + throw new LogicException(sprintf('PSR HTTP factory provider \'%s\' is not supported.', $this->psrHttpProvider)); } $container->addDefinitions([ @@ -282,6 +299,15 @@ private function configureControllers(ContainerBuilder $container): void ; } + private function configureDatabaseServices(ContainerBuilder $container): void + { + $container + ->register(SqlitePlatform::class) + ->setAutoconfigured(true) + ->setAutowired(true) + ; + } + private function determinePsrHttpFactory(): void { $psrHttpProvider = getenv('PSR_HTTP_PROVIDER'); @@ -294,9 +320,7 @@ private function determinePsrHttpFactory(): void $this->psrHttpProvider = self::PSR_HTTP_PROVIDER_NYHOLM; break; default: - throw new LogicException( - sprintf('PSR HTTP factory provider \'%s\' is not supported.', $psrHttpProvider) - ); + throw new LogicException(sprintf('PSR HTTP factory provider \'%s\' is not supported.', $psrHttpProvider)); } } diff --git a/Tests/Unit/ClientEntityTest.php b/Tests/Unit/ClientEntityTest.php new file mode 100644 index 00000000..aced381c --- /dev/null +++ b/Tests/Unit/ClientEntityTest.php @@ -0,0 +1,30 @@ +assertSame($isConfidential, $client->isConfidential()); + } + + public function confidentialDataProvider(): iterable + { + return [ + 'Client with null secret is not confidential' => [null, false], + 'Client with empty secret is not confidential' => ['', false], + 'Client with non empty secret is confidential' => ['f', true], + ]; + } +} diff --git a/Tests/Unit/ExtensionTest.php b/Tests/Unit/ExtensionTest.php index e2e4c7d7..bba4b082 100644 --- a/Tests/Unit/ExtensionTest.php +++ b/Tests/Unit/ExtensionTest.php @@ -5,6 +5,7 @@ namespace Trikoder\Bundle\OAuth2Bundle\Tests\Unit; use League\OAuth2\Server\AuthorizationServer; +use League\OAuth2\Server\Grant\AuthCodeGrant; use League\OAuth2\Server\Grant\ClientCredentialsGrant; use League\OAuth2\Server\Grant\PasswordGrant; use League\OAuth2\Server\Grant\RefreshTokenGrant; @@ -12,7 +13,7 @@ use Symfony\Component\DependencyInjection\ContainerBuilder; use Trikoder\Bundle\OAuth2Bundle\DependencyInjection\TrikoderOAuth2Extension; use Trikoder\Bundle\OAuth2Bundle\Manager\InMemory\ScopeManager; -use Trikoder\Bundle\OAuth2Bundle\Manager\ScopeManagerInterface; +use Trikoder\Bundle\OAuth2Bundle\Model\AuthorizationDecision\AlwaysAllowDecisionStrategy; final class ExtensionTest extends TestCase { @@ -65,7 +66,53 @@ public function grantsProvider(): iterable ]; } - private function getValidConfiguration(array $options): array + /** + * @dataProvider requireCodeChallengeForPublicClientsProvider + */ + public function testAuthCodeGrantDisableRequireCodeChallengeForPublicClientsConfig( + ?bool $requireCodeChallengeForPublicClients, + bool $shouldTheRequirementBeDisabled + ): void { + $container = new ContainerBuilder(); + + $this->setupContainer($container); + + $extension = new TrikoderOAuth2Extension(); + + $configuration = $this->getValidConfiguration(); + $configuration[0]['authorization_server']['require_code_challenge_for_public_clients'] = $requireCodeChallengeForPublicClients; + + $extension->load($configuration, $container); + + $authorizationServer = $container->getDefinition(AuthCodeGrant::class); + $methodCalls = $authorizationServer->getMethodCalls(); + + $isRequireCodeChallengeForPublicClientsDisabled = false; + + foreach ($methodCalls as $methodCall) { + if ('disableRequireCodeChallengeForPublicClients' === $methodCall[0]) { + $isRequireCodeChallengeForPublicClientsDisabled = true; + break; + } + } + + $this->assertSame($shouldTheRequirementBeDisabled, $isRequireCodeChallengeForPublicClientsDisabled); + } + + public function requireCodeChallengeForPublicClientsProvider(): iterable + { + yield 'when not requiring code challenge for public clients the requirement should be disabled' => [ + false, true, + ]; + yield 'when code challenge for public clients is required the requirement should not be disabled' => [ + true, false, + ]; + yield 'with the default value the requirement should not be disabled' => [ + null, false, + ]; + } + + private function getValidConfiguration(array $options = []): array { return [ [ @@ -75,11 +122,15 @@ private function getValidConfiguration(array $options): array 'enable_client_credentials_grant' => $options['enable_client_credentials_grant'] ?? true, 'enable_password_grant' => $options['enable_password_grant'] ?? true, 'enable_refresh_token_grant' => $options['enable_refresh_token_grant'] ?? true, + 'authorization_strategy' => AlwaysAllowDecisionStrategy::class, + 'consent_route' => 'oauth2_consent' ], 'resource_server' => [ 'public_key' => 'foo', ], - 'persistence' => [], + //Pick one for valid config: + //'persistence' => ['doctrine' => []] + 'persistence' => ['in_memory' => 1], ], ]; } @@ -87,6 +138,5 @@ private function getValidConfiguration(array $options): array private function setupContainer(ContainerBuilder $container): void { $container->register(ScopeManager::class); - $container->setAlias(ScopeManagerInterface::class, ScopeManager::class); } } diff --git a/Tests/Unit/InMemoryAccessTokenManagerTest.php b/Tests/Unit/InMemoryAccessTokenManagerTest.php index 542e031e..c7986538 100644 --- a/Tests/Unit/InMemoryAccessTokenManagerTest.php +++ b/Tests/Unit/InMemoryAccessTokenManagerTest.php @@ -4,7 +4,7 @@ namespace Trikoder\Bundle\OAuth2Bundle\Tests\Unit; -use DateTime; +use DateTimeImmutable; use PHPUnit\Framework\TestCase; use ReflectionProperty; use Trikoder\Bundle\OAuth2Bundle\Manager\InMemory\AccessTokenManager as InMemoryAccessTokenManager; @@ -17,7 +17,7 @@ public function testClearExpired(): void { $inMemoryAccessTokenManager = new InMemoryAccessTokenManager(); - timecop_freeze(new DateTime()); + timecop_freeze(new DateTimeImmutable()); try { $testData = $this->buildClearExpiredTestData(); @@ -62,7 +62,7 @@ private function buildAccessToken(string $identifier, string $modify): AccessTok { return new AccessToken( $identifier, - (new DateTime())->modify($modify), + new DateTimeImmutable($modify), new Client('client', 'secret'), null, [] diff --git a/Tests/Unit/InMemoryAuthCodeManagerTest.php b/Tests/Unit/InMemoryAuthCodeManagerTest.php new file mode 100644 index 00000000..df7ad6ba --- /dev/null +++ b/Tests/Unit/InMemoryAuthCodeManagerTest.php @@ -0,0 +1,72 @@ +buildClearExpiredTestData(); + + /** @var AuthorizationCode $authCode */ + foreach ($testData['input'] as $authCode) { + $inMemoryAuthCodeManager->save($authCode); + } + + $this->assertSame(3, $inMemoryAuthCodeManager->clearExpired()); + } finally { + timecop_return(); + } + + $reflectionProperty = new ReflectionProperty(InMemoryAuthCodeManager::class, 'authorizationCodes'); + $reflectionProperty->setAccessible(true); + + $this->assertSame($testData['output'], $reflectionProperty->getValue($inMemoryAuthCodeManager)); + } + + private function buildClearExpiredTestData(): array + { + $validAuthCodes = [ + '1111' => $this->buildAuthCode('1111', '+1 day'), + '2222' => $this->buildAuthCode('2222', '+1 hour'), + '3333' => $this->buildAuthCode('3333', '+1 second'), + '4444' => $this->buildAuthCode('4444', 'now'), + ]; + + $expiredAuthCodes = [ + '5555' => $this->buildAuthCode('5555', '-1 day'), + '6666' => $this->buildAuthCode('6666', '-1 hour'), + '7777' => $this->buildAuthCode('7777', '-1 second'), + ]; + + return [ + 'input' => $validAuthCodes + $expiredAuthCodes, + 'output' => $validAuthCodes, + ]; + } + + private function buildAuthCode(string $identifier, string $modify): AuthorizationCode + { + return new AuthorizationCode( + $identifier, + new DateTimeImmutable($modify), + new Client('client', 'secret'), + null, + [] + ); + } +} diff --git a/Tests/Unit/InMemoryRefreshTokenManagerTest.php b/Tests/Unit/InMemoryRefreshTokenManagerTest.php index 73a3282d..62e958d9 100644 --- a/Tests/Unit/InMemoryRefreshTokenManagerTest.php +++ b/Tests/Unit/InMemoryRefreshTokenManagerTest.php @@ -4,7 +4,7 @@ namespace Trikoder\Bundle\OAuth2Bundle\Tests\Unit; -use DateTime; +use DateTimeImmutable; use PHPUnit\Framework\TestCase; use ReflectionProperty; use Trikoder\Bundle\OAuth2Bundle\Manager\InMemory\RefreshTokenManager as InMemoryRefreshTokenManager; @@ -18,7 +18,7 @@ public function testClearExpired(): void { $inMemoryRefreshTokenManager = new InMemoryRefreshTokenManager(); - timecop_freeze(new DateTime()); + timecop_freeze(new DateTimeImmutable()); try { $testData = $this->buildClearExpiredTestData(); @@ -63,10 +63,10 @@ private function buildRefreshToken(string $identifier, string $modify): RefreshT { return new RefreshToken( $identifier, - (new DateTime())->modify($modify), + new DateTimeImmutable($modify), new AccessToken( $identifier, - (new DateTime('+1 day')), + new DateTimeImmutable('+1 day'), new Client('client', 'secret'), null, [] diff --git a/Tests/Unit/OAuth2ProviderTest.php b/Tests/Unit/OAuth2ProviderTest.php new file mode 100644 index 00000000..f1227f2d --- /dev/null +++ b/Tests/Unit/OAuth2ProviderTest.php @@ -0,0 +1,49 @@ +createMock(UserProviderInterface::class), + $this->createMock(ResourceServer::class), + $tokenFactory, + $providerKey + ); + + $this->assertTrue($provider->supports($this->createToken($tokenFactory, $providerKey))); + $this->assertFalse($provider->supports($this->createToken($tokenFactory, $providerKey . 'bar'))); + } + + private function createToken(OAuth2TokenFactory $tokenFactory, string $providerKey): OAuth2Token + { + $scopes = [FixtureFactory::FIXTURE_SCOPE_FIRST]; + $serverRequest = $this->createMock(ServerRequestInterface::class); + $serverRequest->expects($this->once()) + ->method('getAttribute') + ->with('oauth_scopes', []) + ->willReturn($scopes); + + $user = new User(); + + return $tokenFactory->createOAuth2Token($serverRequest, $user, $providerKey); + } +} diff --git a/Tests/Unit/OAuth2TokenFactoryTest.php b/Tests/Unit/OAuth2TokenFactoryTest.php new file mode 100644 index 00000000..3a29c559 --- /dev/null +++ b/Tests/Unit/OAuth2TokenFactoryTest.php @@ -0,0 +1,43 @@ +createMock(ServerRequestInterface::class); + $serverRequest->expects($this->once()) + ->method('getAttribute') + ->with('oauth_scopes', []) + ->willReturn($scopes); + + $user = new User(); + $providerKey = 'main'; + + $token = $factory->createOAuth2Token($serverRequest, $user, $providerKey); + + $this->assertInstanceOf(OAuth2Token::class, $token); + + $roles = $token->getRoleNames(); + $this->assertCount(1, $roles); + $this->assertSame($rolePrefix . strtoupper($scopes[0]), $roles[0]); + + $this->assertFalse($token->isAuthenticated()); + $this->assertSame($user, $token->getUser()); + $this->assertSame($providerKey, $token->getProviderKey()); + } +} diff --git a/Tests/Unit/OAuth2TokenTest.php b/Tests/Unit/OAuth2TokenTest.php new file mode 100644 index 00000000..81b0255b --- /dev/null +++ b/Tests/Unit/OAuth2TokenTest.php @@ -0,0 +1,40 @@ +createMock(ServerRequestInterface::class); + $serverRequest->expects($this->once()) + ->method('getAttribute') + ->with('oauth_scopes', []) + ->willReturn($scopes); + + $user = new User(); + $rolePrefix = 'ROLE_OAUTH2_'; + $providerKey = 'main'; + $token = new OAuth2Token($serverRequest, $user, $rolePrefix, $providerKey); + + /** @var OAuth2Token $unserializedToken */ + $unserializedToken = unserialize(serialize($token)); + + $this->assertSame($providerKey, $unserializedToken->getProviderKey()); + + $expectedRole = $rolePrefix . strtoupper($scopes[0]); + $this->assertSame([$expectedRole], $token->getRoleNames()); + + $this->assertSame($user->getUsername(), $unserializedToken->getUser()->getUsername()); + $this->assertFalse($unserializedToken->isAuthenticated()); + } +} diff --git a/TrikoderOAuth2Bundle.php b/TrikoderOAuth2Bundle.php index 07e19a54..14e2d58c 100644 --- a/TrikoderOAuth2Bundle.php +++ b/TrikoderOAuth2Bundle.php @@ -8,7 +8,6 @@ use Symfony\Bundle\SecurityBundle\DependencyInjection\SecurityExtension; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\HttpKernel\Bundle\Bundle; -use Trikoder\Bundle\OAuth2Bundle\DependencyInjection\CompilerPass\EventDispatcherCompilerPass; use Trikoder\Bundle\OAuth2Bundle\DependencyInjection\Security\OAuth2Factory; use Trikoder\Bundle\OAuth2Bundle\DependencyInjection\TrikoderOAuth2Extension; @@ -42,7 +41,6 @@ private function configureSecurityExtension(ContainerBuilder $container): void private function configureDoctrineExtension(ContainerBuilder $container): void { - $container->addCompilerPass(new EventDispatcherCompilerPass()); $container->addCompilerPass( DoctrineOrmMappingsPass::createXmlMappingDriver( [ diff --git a/UPGRADE.md b/UPGRADE.md index fd05a454..7e4d99eb 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,6 +1,50 @@ # Upgrade Here you will find upgrade steps between major releases. +## From 2.x to 3.x + +### Console command changes + +#### `trikoder:oauth2:clear-expired-tokens` + +The following options have been renamed: + +* `access-tokens-only` has been renamed to `access-tokens` +* `refresh-tokens-only` has been renamed to `refresh-tokens` + +### SQL schema changes + +The bundle makes modifications to the existing schema. You will need to run the Doctrine schema update process to sync the changes: + +```sh +bin/console doctrine:schema:update +``` + +The schema changes include: + +* New `allow_plain_text_pkce` field on the `oauth2_client` table +* `secret` field on the `oauth2_client` table is now nullable + +### Interface changes + +The following interfaces have been changed: + +#### `Trikoder\Bundle\OAuth2Bundle\Manager\AuthorizationCodeManagerInterface` + +- [Added the clearExpired() method](https://github.com/trikoder/oauth2-bundle/blob/v3.0.0/Manager/AuthorizationCodeManagerInterface.php#L15) + +### Method signature changes + +The following method signatures have been changed: + +#### `Trikoder\Bundle\OAuth2Bundle\Model\Client` + +- [Return type for getSecret() is now nullable](https://github.com/trikoder/oauth2-bundle/blob/v3.0.0/Model/Client.php#L60) + +--- + +> **NOTE:** The underlying [league/oauth2-server](https://github.com/thephpleague/oauth2-server) library has been upgraded from version `7.x` to `8.x`. Please check your code if you are directly implementing their interfaces or extending existing non-final classes. + ## From 1.x to 2.x ### PSR-7/17 HTTP transport implementation diff --git a/composer.json b/composer.json index 46780bdb..a0398236 100644 --- a/composer.json +++ b/composer.json @@ -3,9 +3,17 @@ "type": "symfony-bundle", "description": "Symfony bundle which provides OAuth 2.0 authorization/resource server capabilities.", "keywords": ["oauth2"], - "homepage": "http://www.trikoder.net/", + "homepage": "https://www.trikoder.net/", "license": "MIT", "authors": [ + { + "name": "Antonio Pauletich", + "email": "antonio.pauletich@trikoder.net" + }, + { + "name": "Berislav Balogović", + "email": "berislav.balogovic@trikoder.net" + }, { "name": "Petar Obradović", "email": "petar.obradovic@trikoder.net" @@ -13,26 +21,25 @@ ], "require": { "php": ">=7.2", - "doctrine/doctrine-bundle": "^1.8", - "doctrine/orm": "^2.6", - "league/oauth2-server": "^7.2", - "psr/event-dispatcher": "^1.0", + "doctrine/doctrine-bundle": "^1.8|^2.0", + "doctrine/orm": "^2.7", + "league/oauth2-server": "^8.0", "psr/http-factory": "^1.0", - "sensio/framework-extra-bundle": "^5.3", - "symfony/event-dispatcher-contracts": "^1.1", - "symfony/framework-bundle": "^3.4|^4.2", - "symfony/psr-http-message-bridge": "^1.2", - "symfony/security-bundle": "^3.4|^4.2" + "sensio/framework-extra-bundle": "^5.5", + "symfony/event-dispatcher-contracts": "^1.1|^2.0", + "symfony/framework-bundle": "^4.4|^5.0", + "symfony/psr-http-message-bridge": "^2.0", + "symfony/security-bundle": "^4.4|^5.0", + "steverhoades/oauth2-openid-connect-server": "^1.0" }, "require-dev": { "ext-timecop": "*", "ext-xdebug": "*", - "friendsofphp/php-cs-fixer": "^2.15", - "nyholm/psr7": "^1.1", - "phpunit/phpunit": "~8.2.0", - "symfony/browser-kit": "^3.4|^4.2", - "symfony/phpunit-bridge": "^3.4|^4.3", - "zendframework/zend-diactoros": "^2.1" + "laminas/laminas-diactoros": "^2.2", + "nyholm/psr7": "^1.2", + "phpunit/phpunit": "^8.5", + "symfony/browser-kit": "^4.4|^5.0", + "symfony/phpunit-bridge": "^5.0" }, "autoload": { "psr-4": { "Trikoder\\Bundle\\OAuth2Bundle\\": "" }, @@ -47,6 +54,7 @@ "suggest": { "nelmio/cors-bundle": "For handling CORS requests", "nyholm/psr7": "For a super lightweight PSR-7/17 implementation", + "steverhoades/oauth2-openid-connect-server": "For OpenID Connect Provider capabilities", "defuse/php-encryption": "For better performance when doing encryption" }, "config": { @@ -54,7 +62,7 @@ }, "extra": { "branch-alias": { - "dev-master": "3.x-dev" + "dev-master": "4.x-dev" } } } diff --git a/docs/basic-setup.md b/docs/basic-setup.md index bd3507b2..2262b5ad 100644 --- a/docs/basic-setup.md +++ b/docs/basic-setup.md @@ -23,6 +23,8 @@ Options: --redirect-uri[=REDIRECT-URI] Sets redirect uri for client. Use this option multiple times to set multiple redirect URIs. (multiple values allowed) --grant-type[=GRANT-TYPE] Sets allowed grant type for client. Use this option multiple times to set multiple grant types. (multiple values allowed) --scope[=SCOPE] Sets allowed scope for client. Use this option multiple times to set multiple scopes. (multiple values allowed) + --public Creates a public client (a client which does not have a secret) + --allow-plain-text-pkce Creates a client which is allowed to create an authorization code grant PKCE request with the "plain" code challenge method ``` @@ -129,7 +131,8 @@ oauth2_restricted: ## Security roles -Once the user gets past the `oauth2` firewall, they will be granted additional roles based on their granted [token scopes](controlling-token-scopes.md). The roles are named in the following format: +Once the user gets past the `oauth2` firewall, they will be granted additional roles based on their granted [token scopes](controlling-token-scopes.md). +By default, the roles are named in the following format: ``` ROLE_OAUTH2_ @@ -147,30 +150,36 @@ public function indexAction() } ``` +> **NOTE:** You can change the `ROLE_OAUTH2_` prefix via the `role_prefix` configuration option described in [Installation section](../README.md#installation) + ## Auth There are two possible reasons for the authentication server to reject a request: - Provided token is expired or invalid (HTTP response 401 `Unauthorized`) - Provided token is valid but scopes are insufficient (HTTP response 403 `Forbidden`) -## Clearing expired access & refresh tokens +## Clearing expired access, refresh tokens and auth codes -To clear expired access & refresh tokens you can use the `trikoder:oauth2:clear-expired-tokens` command. +To clear expired access and refresh tokens and auth codes you can use the `trikoder:oauth2:clear-expired-tokens` command. The command removes all tokens whose expiry time is lesser than the current. ```sh Description: - Clears all expired access and/or refresh tokens + Clears all expired access and/or refresh tokens and/or auth codes Usage: trikoder:oauth2:clear-expired-tokens [options] Options: - -a, --access-tokens-only Clear only access tokens. - -r, --refresh-tokens-only Clear only refresh tokens. + -a, --access-tokens Clear expired access tokens. + -r, --refresh-tokens Clear expired refresh tokens. + -c, --auth-codes Clear expired auth codes. ``` +Not passing any option means that both expired access and refresh tokens as well as expired auth codes +will be cleared. + ## CORS requests For CORS handling, use [NelmioCorsBundle](https://github.com/nelmio/NelmioCorsBundle)