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/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 63a53d83..1fc84135 100644 --- a/README.md +++ b/README.md @@ -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/Security/Exception/OAuth2AuthenticationFailedException.php b/Security/Exception/OAuth2AuthenticationFailedException.php index b21d4d09..35d8fd20 100644 --- a/Security/Exception/OAuth2AuthenticationFailedException.php +++ b/Security/Exception/OAuth2AuthenticationFailedException.php @@ -5,14 +5,15 @@ namespace Trikoder\Bundle\OAuth2Bundle\Security\Exception; use Symfony\Component\Security\Core\Exception\AuthenticationException; +use Throwable; /** * @author Tobias Nyholm */ class OAuth2AuthenticationFailedException extends AuthenticationException { - public static function create(string $message): self + public static function create(string $message, ?Throwable $previous = null): self { - return new self($message, 401); + return new self($message, 401, $previous); } } diff --git a/Security/Firewall/OAuth2Listener.php b/Security/Firewall/OAuth2Listener.php index 817c3a43..8f1ae8a6 100644 --- a/Security/Firewall/OAuth2Listener.php +++ b/Security/Firewall/OAuth2Listener.php @@ -68,7 +68,7 @@ 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); + throw OAuth2AuthenticationFailedException::create($e->getMessage(), $e); } 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,