Merge pull request #231 from trikoder/remove-deprecated-code

Remove deprecated code
trikoder · Oct 5, 2020 · 5d0d1ba · 5d0d1ba
2 parents 1c32d8c + 9701bd5
commit 5d0d1ba
Show file tree

Hide file tree

Showing 6 changed files with 12 additions and 163 deletions.
diff --git 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
@@ -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
@@ -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
@@ -5,14 +5,15 @@
 namespace Trikoder\Bundle\OAuth2Bundle\Security\Exception;
 
 use Symfony\Component\Security\Core\Exception\AuthenticationException;
+use Throwable;
 
 /**
  * @author Tobias Nyholm <tobias.nyholm@gmail.com>
  */
 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
@@ -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
@@ -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,31 +304,20 @@ 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,
         ];
     }
 
     /**
      * @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,