diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index 707b2266..8c16b06d 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -79,48 +79,9 @@ private function createAuthorizationServerNode(): NodeDefinition ->cannotBeEmpty() ->defaultValue('P1M') ->end() - - // @TODO Remove in v4 start - - ->scalarNode('auth_code_ttl') - ->info("How long the issued authorization code should be valid for.\nThe value should be a valid interval: http://php.net/manual/en/dateinterval.construct.php#refsect1-dateinterval.construct-parameters") - ->cannotBeEmpty() - ->setDeprecated('"%path%.%node%" is deprecated, use "%path%.grant_types.authorization_code.auth_code_ttl" instead.') - ->beforeNormalization() - ->ifNull() - ->thenUnset() - ->end() - ->end() - ->booleanNode('require_code_challenge_for_public_clients') - ->info('Whether to require code challenge for public clients for the authorization code grant.') - ->setDeprecated('"%path%.%node%" is deprecated, use "%path%.grant_types.authorization_code.require_code_challenge_for_public_clients" instead.') - ->beforeNormalization() - ->ifNull() - ->thenUnset() - ->end() - ->end() ->end() ; - foreach (OAuth2Grants::ALL as $grantType => $grantTypeName) { - $oldGrantType = 'authorization_code' === $grantType ? 'auth_code' : $grantType; - - $node - ->children() - ->booleanNode(sprintf('enable_%s_grant', $oldGrantType)) - ->info(sprintf('Whether to enable the %s grant.', $grantTypeName)) - ->setDeprecated(sprintf('"%%path%%.%%node%%" is deprecated, use "%%path%%.grant_types.%s.enable" instead.', $grantType)) - ->beforeNormalization() - ->ifNull() - ->thenUnset() - ->end() - ->end() - ->end() - ; - } - - // @TODO Remove in v4 end - $node->append($this->createAuthorizationServerGrantTypesNode()); $node @@ -134,33 +95,9 @@ private function createAuthorizationServerNode(): NodeDefinition if (isset($grantTypesWithRefreshToken[$grantType])) { $grantTypeConfig['refresh_token_ttl'] = $grantTypeConfig['refresh_token_ttl'] ?? $v['refresh_token_ttl']; } - - // @TODO Remove in v4 start - $oldGrantType = 'authorization_code' === $grantType ? 'auth_code' : $grantType; - - $grantTypeConfig['enable'] = $v[sprintf('enable_%s_grant', $oldGrantType)] ?? $grantTypeConfig['enable']; - - if ('authorization_code' === $grantType) { - $grantTypeConfig['auth_code_ttl'] = $v['auth_code_ttl'] ?? $grantTypeConfig['auth_code_ttl']; - $grantTypeConfig['require_code_challenge_for_public_clients'] = $v['require_code_challenge_for_public_clients'] - ?? $grantTypeConfig['require_code_challenge_for_public_clients']; - } - // @TODO Remove in v4 end } - unset( - $v['access_token_ttl'], - $v['refresh_token_ttl'], - // @TODO Remove in v4 start - $v['enable_auth_code_grant'], - $v['enable_client_credentials_grant'], - $v['enable_implicit_grant'], - $v['enable_password_grant'], - $v['enable_refresh_token_grant'], - $v['auth_code_ttl'], - $v['require_code_challenge_for_public_clients'] - // @TODO Remove in v4 end - ); + unset($v['access_token_ttl'], $v['refresh_token_ttl']); return $v; }) diff --git a/Event/AuthenticationFailureEvent.php b/Event/AuthenticationFailureEvent.php new file mode 100644 index 00000000..3efab875 --- /dev/null +++ b/Event/AuthenticationFailureEvent.php @@ -0,0 +1,52 @@ + + */ +class AuthenticationFailureEvent extends Event { + /** + * @var AuthenticationException + */ + protected $exception; + + /** + * @var Response + */ + protected $response; + + /** + * @param AuthenticationException $exception + * @param Response $response + */ + public function __construct(AuthenticationException $exception, Response $response) { + $this->exception = $exception; + $this->response = $response; + } + + /** + * @return AuthenticationException + */ + public function getException(): AuthenticationException { + return $this->exception; + } + + /** + * @return Response + */ + public function getResponse(): Response { + return $this->response; + } + + /** + * @param Response $response + */ + public function setResponse(Response $response): void { + $this->response = $response; + } +} diff --git a/EventListener/ConvertExceptionToResponseListener.php b/EventListener/ConvertExceptionToResponseListener.php index b29fdffe..535eee16 100644 --- a/EventListener/ConvertExceptionToResponseListener.php +++ b/EventListener/ConvertExceptionToResponseListener.php @@ -7,7 +7,7 @@ use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Event\ExceptionEvent; use Trikoder\Bundle\OAuth2Bundle\Security\Exception\InsufficientScopesException; -use Trikoder\Bundle\OAuth2Bundle\Security\Exception\Oauth2AuthenticationFailedException; +use Trikoder\Bundle\OAuth2Bundle\Security\Exception\OAuth2AuthenticationFailedException; /** * @author Tobias Nyholm @@ -17,7 +17,7 @@ final class ConvertExceptionToResponseListener public function onKernelException(ExceptionEvent $event): void { $exception = $event->getThrowable(); - if ($exception instanceof InsufficientScopesException || $exception instanceof Oauth2AuthenticationFailedException) { + if ($exception instanceof InsufficientScopesException || $exception instanceof OAuth2AuthenticationFailedException) { $event->setResponse(new Response($exception->getMessage(), $exception->getCode())); } } diff --git a/OAuth2Events.php b/OAuth2Events.php index 8894eb9d..836ffe33 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,18 @@ 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 AUTHENTICATION_FAILURE event occurs when the oauth token wasn't found + * + * You can set a custom error message in the response body + */ + public const AUTHENTICATION_FAILURE = 'trikoder.oauth2.autentication_failure'; } diff --git a/OAuth2Grants.php b/OAuth2Grants.php index d823f17e..025a617e 100644 --- a/OAuth2Grants.php +++ b/OAuth2Grants.php @@ -54,14 +54,4 @@ final class OAuth2Grants self::PASSWORD => 'password', self::REFRESH_TOKEN => 'refresh token', ]; - - /** - * @deprecated Will be removed in v4, use {@see OAuth2Grants::ALL} instead - * - * @TODO Remove in v4. - */ - public static function has(string $grant): bool - { - return isset(self::ALL[$grant]); - } } diff --git a/README.md b/README.md index 553a1c15..e18868d3 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # Trikoder OAuth 2 Bundle -[![Build Status](https://github.com/trikoder/oauth2-bundle/workflows/Tests/badge.svg?branch=v3.x)](https://github.com/trikoder/oauth2-bundle/actions) +[![Build Status](https://github.com/trikoder/oauth2-bundle/workflows/Tests/badge.svg?branch=master)](https://github.com/trikoder/oauth2-bundle/actions) [![Latest Stable Version](https://poser.pugx.org/trikoder/oauth2-bundle/v/stable)](https://packagist.org/packages/trikoder/oauth2-bundle) [![License](https://poser.pugx.org/trikoder/oauth2-bundle/license)](https://packagist.org/packages/trikoder/oauth2-bundle) [![Code coverage](https://codecov.io/gh/trikoder/oauth2-bundle/branch/master/graph/badge.svg)](https://codecov.io/gh/trikoder/oauth2-bundle) @@ -68,28 +68,6 @@ This package is currently in the active development. # The value should be a valid interval: http://php.net/manual/en/dateinterval.construct.php#refsect1-dateinterval.construct-parameters refresh_token_ttl: P1M - # How long the issued authorization code should be valid for. - # The value should be a valid interval: http://php.net/manual/en/dateinterval.construct.php#refsect1-dateinterval.construct-parameters - auth_code_ttl: ~ # Deprecated ("trikoder_oauth2.authorization_server.auth_code_ttl" is deprecated, use "trikoder_oauth2.authorization_server.grant_types.authorization_code.auth_code_ttl" instead.) - - # Whether to require code challenge for public clients for the authorization code grant. - require_code_challenge_for_public_clients: ~ # Deprecated ("trikoder_oauth2.authorization_server.require_code_challenge_for_public_clients" is deprecated, use "trikoder_oauth2.authorization_server.grant_types.authorization_code.require_code_challenge_for_public_clients" instead.) - - # Whether to enable the authorization code grant. - enable_auth_code_grant: ~ # Deprecated ("trikoder_oauth2.authorization_server.enable_auth_code_grant" is deprecated, use "trikoder_oauth2.authorization_server.grant_types.authorization_code.enable" instead.) - - # Whether to enable the client credentials grant. - enable_client_credentials_grant: ~ # Deprecated ("trikoder_oauth2.authorization_server.enable_client_credentials_grant" is deprecated, use "trikoder_oauth2.authorization_server.grant_types.client_credentials.enable" instead.) - - # Whether to enable the implicit grant. - enable_implicit_grant: ~ # Deprecated ("trikoder_oauth2.authorization_server.enable_implicit_grant" is deprecated, use "trikoder_oauth2.authorization_server.grant_types.implicit.enable" instead.) - - # Whether to enable the password grant. - enable_password_grant: ~ # Deprecated ("trikoder_oauth2.authorization_server.enable_password_grant" is deprecated, use "trikoder_oauth2.authorization_server.grant_types.password.enable" instead.) - - # Whether to enable the refresh token grant. - enable_refresh_token_grant: ~ # Deprecated ("trikoder_oauth2.authorization_server.enable_refresh_token_grant" is deprecated, use "trikoder_oauth2.authorization_server.grant_types.refresh_token.enable" instead.) - # Enable and configure grant types. grant_types: authorization_code: diff --git a/Resources/config/services.xml b/Resources/config/services.xml index 73734a27..dfc77ecf 100644 --- a/Resources/config/services.xml +++ b/Resources/config/services.xml @@ -84,6 +84,7 @@ + diff --git a/Security/Exception/OAuth2AuthenticationFailedException.php b/Security/Exception/OAuth2AuthenticationFailedException.php new file mode 100644 index 00000000..00580998 --- /dev/null +++ b/Security/Exception/OAuth2AuthenticationFailedException.php @@ -0,0 +1,19 @@ + + * @author Benoit VIGNAL + */ +class OAuth2AuthenticationFailedException extends AuthenticationException +{ + /** + * {@inheritdoc} + */ + public function getMessageKey(): string { + return "OAuth Token not found"; + } +} diff --git a/Security/Exception/Oauth2AuthenticationFailedException.php b/Security/Exception/Oauth2AuthenticationFailedException.php deleted file mode 100644 index f42097ed..00000000 --- a/Security/Exception/Oauth2AuthenticationFailedException.php +++ /dev/null @@ -1,18 +0,0 @@ - - */ -class Oauth2AuthenticationFailedException extends AuthenticationException -{ - public static function create(string $message): self - { - return new self($message, 401); - } -} diff --git a/Security/Firewall/OAuth2Listener.php b/Security/Firewall/OAuth2Listener.php index 36330fc7..e617138b 100644 --- a/Security/Firewall/OAuth2Listener.php +++ b/Security/Firewall/OAuth2Listener.php @@ -5,15 +5,19 @@ namespace Trikoder\Bundle\OAuth2Bundle\Security\Firewall; use Symfony\Bridge\PsrHttpMessage\HttpMessageFactoryInterface; +use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Response; 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 Trikoder\Bundle\OAuth2Bundle\Event\AuthenticationFailureEvent; +use Trikoder\Bundle\OAuth2Bundle\OAuth2Events; 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; +use Trikoder\Bundle\OAuth2Bundle\Security\Exception\OAuth2AuthenticationFailedException; final class OAuth2Listener { @@ -37,6 +41,11 @@ final class OAuth2Listener */ private $oauth2TokenFactory; + /** + * @var EventDispatcherInterface + */ + private $eventDispatcher; + /** * @var string */ @@ -46,12 +55,14 @@ public function __construct( TokenStorageInterface $tokenStorage, AuthenticationManagerInterface $authenticationManager, HttpMessageFactoryInterface $httpMessageFactory, + EventDispatcherInterface $eventDispatcher, OAuth2TokenFactory $oauth2TokenFactory, string $providerKey ) { $this->tokenStorage = $tokenStorage; $this->authenticationManager = $authenticationManager; $this->httpMessageFactory = $httpMessageFactory; + $this->eventDispatcher = $eventDispatcher; $this->oauth2TokenFactory = $oauth2TokenFactory; $this->providerKey = $providerKey; } @@ -68,7 +79,16 @@ public function __invoke(RequestEvent $event) /** @var OAuth2Token $authenticatedToken */ $authenticatedToken = $this->authenticationManager->authenticate($this->oauth2TokenFactory->createOAuth2Token($request, null, $this->providerKey)); } catch (AuthenticationException $e) { - throw new Oauth2AuthenticationFailedException($e->getMessage(), 401, $e); + $exception = new OAuth2AuthenticationFailedException("OAuth Token not found", 0, $e); + $response = new Response($exception->getMessageKey(), Response::HTTP_UNAUTHORIZED); + + $authenticationFailureEvent = new AuthenticationFailureEvent($exception, $response); + $this->eventDispatcher->dispatch($authenticationFailureEvent, OAuth2Events::AUTHENTICATION_FAILURE); + + if ($response = $authenticationFailureEvent->getResponse()) { + $event->setResponse($response); + } + return; } if (!$this->isAccessToRouteGranted($event->getRequest(), $authenticatedToken)) { diff --git a/Tests/Unit/ExtensionTest.php b/Tests/Unit/ExtensionTest.php index 146ae35f..3ed16159 100644 --- a/Tests/Unit/ExtensionTest.php +++ b/Tests/Unit/ExtensionTest.php @@ -265,44 +265,12 @@ public function grantsProvider(): iterable yield 'Refresh token grant can be disabled' => [ RefreshTokenGrant::class, 'refresh_token.enable', false, ]; - - yield 'Legacy authorization code grant can be enabled' => [ - AuthCodeGrant::class, 'enable_auth_code_grant', true, - ]; - yield 'Legacy authorization code grant can be disabled' => [ - AuthCodeGrant::class, 'enable_auth_code_grant', false, - ]; - yield 'Legacy client credentials grant can be enabled' => [ - ClientCredentialsGrant::class, 'enable_client_credentials_grant', true, - ]; - yield 'Legacy client credentials grant can be disabled' => [ - ClientCredentialsGrant::class, 'enable_client_credentials_grant', false, - ]; - yield 'Legacy implicit grant can be enabled' => [ - ImplicitGrant::class, 'enable_implicit_grant', true, - ]; - yield 'Legacy implicit grant can be disabled' => [ - ImplicitGrant::class, 'enable_implicit_grant', false, - ]; - yield 'Legacy password grant can be enabled' => [ - PasswordGrant::class, 'enable_password_grant', true, - ]; - yield 'Legacy password grant can be disabled' => [ - PasswordGrant::class, 'enable_password_grant', false, - ]; - yield 'Legacy refresh token grant can be enabled' => [ - RefreshTokenGrant::class, 'enable_refresh_token_grant', true, - ]; - yield 'Legacy refresh token grant can be disabled' => [ - RefreshTokenGrant::class, 'enable_refresh_token_grant', false, - ]; } /** * @dataProvider requireCodeChallengeForPublicClientsProvider */ public function testAuthCodeGrantDisableRequireCodeChallengeForPublicClientsConfig( - string $configKey, ?bool $requireCodeChallengeForPublicClients, bool $shouldTheRequirementBeDisabled ): void { @@ -313,7 +281,7 @@ public function testAuthCodeGrantDisableRequireCodeChallengeForPublicClientsConf $extension = new TrikoderOAuth2Extension(); $configuration = $this->getValidConfiguration([ - $configKey => $requireCodeChallengeForPublicClients, + 'authorization_code.require_code_challenge_for_public_clients' => $requireCodeChallengeForPublicClients, ]); $extension->load($configuration, $container); @@ -336,23 +304,13 @@ public function testAuthCodeGrantDisableRequireCodeChallengeForPublicClientsConf public function requireCodeChallengeForPublicClientsProvider(): iterable { yield 'When not requiring code challenge for public clients the requirement should be disabled' => [ - 'authorization_code.require_code_challenge_for_public_clients', false, true, + false, true, ]; yield 'When code challenge for public clients is required the requirement should not be disabled' => [ - 'authorization_code.require_code_challenge_for_public_clients', true, false, + true, false, ]; yield 'With the default value the requirement should not be disabled' => [ - 'authorization_code.require_code_challenge_for_public_clients', null, false, - ]; - - yield 'Legacy when not requiring code challenge for public clients the requirement should be disabled' => [ - 'require_code_challenge_for_public_clients', false, true, - ]; - yield 'Legacy when code challenge for public clients is required the requirement should not be disabled' => [ - 'require_code_challenge_for_public_clients', true, false, - ]; - yield 'Legacy with the default value the requirement should not be disabled' => [ - 'require_code_challenge_for_public_clients', null, false, + null, false, ]; } @@ -360,7 +318,6 @@ public function requireCodeChallengeForPublicClientsProvider(): iterable * @dataProvider authCodeTTLProvider */ public function testAuthCodeTTLConfig( - string $configKey, ?string $authCodeTTL, string $expectedAuthCodeTTL ): void { @@ -371,7 +328,7 @@ public function testAuthCodeTTLConfig( $extension = new TrikoderOAuth2Extension(); $configuration = $this->getValidConfiguration([ - $configKey => $authCodeTTL, + 'authorization_code.auth_code_ttl' => $authCodeTTL, ]); $extension->load($configuration, $container); @@ -384,17 +341,10 @@ public function testAuthCodeTTLConfig( public function authCodeTTLProvider(): iterable { yield 'Authorization code TTL can be set' => [ - 'authorization_code.auth_code_ttl', 'PT20M', 'PT20M', + 'PT20M', 'PT20M', ]; yield 'When no authorization code TTL is set, the default is used' => [ - 'authorization_code.auth_code_ttl', null, 'PT10M', - ]; - - yield 'Legacy authorization code TTL can be set' => [ - 'auth_code_ttl', 'PT20M', 'PT20M', - ]; - yield 'Legacy when no authorization code TTL is set, the default is used' => [ - 'auth_code_ttl', null, 'PT10M', + null, 'PT10M', ]; } @@ -405,15 +355,8 @@ private function getValidConfiguration(array $options = []): array 'authorization_server' => [ 'private_key' => 'foo', 'encryption_key' => 'foo', - 'enable_auth_code_grant' => $options['enable_auth_code_grant'] ?? null, 'access_token_ttl' => $options['access_token_ttl'] ?? 'PT1H', 'refresh_token_ttl' => $options['refresh_token_ttl'] ?? 'P1M', - 'enable_client_credentials_grant' => $options['enable_client_credentials_grant'] ?? null, - 'enable_implicit_grant' => $options['enable_implicit_grant'] ?? null, - 'enable_password_grant' => $options['enable_password_grant'] ?? null, - 'enable_refresh_token_grant' => $options['enable_refresh_token_grant'] ?? null, - 'require_code_challenge_for_public_clients' => $options['require_code_challenge_for_public_clients'] ?? null, - 'auth_code_ttl' => $options['auth_code_ttl'] ?? null, 'grant_types' => [ 'authorization_code' => [ 'enable' => $options['authorization_code.enable'] ?? true, diff --git a/composer.json b/composer.json index a24a3133..5d176798 100644 --- a/composer.json +++ b/composer.json @@ -59,7 +59,7 @@ }, "extra": { "branch-alias": { - "dev-master": "3.x-dev" + "dev-master": "4.x-dev" } } }