Rework resource owners to use Symfony Http Client internally

hwi · Aug 2, 2021 · 390e765 · 390e765
1 parent 4b4ec05
commit 390e765
Show file tree

Hide file tree

Showing 76 changed files with 1,336 additions and 1,135 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,9 @@ Changelog
 * BC Break: removed configuration option `fosub` from `oauth_user_provider`,
 * BC Break: removed configuration options `hwi_oauth.fosub`, & all related DI parameters,
 * BC Break: removed DI parameter `hwi_oauth.registration.form.factory` in favour of declaring form class name as DI parameter: `hwi_oauth.connect.registration_form`,
+* BC Break: changed `__construct()` argument for `OAuth/ResourceOwner/AbstractResourceOwner`, from `HttpMethodsClient $httpClient` to `HttpClientInterface $httpClient`,
+* BC Break: replaced `php-http/httplug-bundle` with `symfony/http-client`
+* BC Break: removed `hwi_oauth.http` configuration
 
 ## 1.4.1 (2021-07-28)
 * Bugfix: Define missing `hwi_oauth.connect.confirmation` parameter,

diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php
@@ -180,7 +180,6 @@ public function getConfigTreeBuilder()
             ->end()
         ;
 
-        $this->addHttpClientConfiguration($rootNode);
         $this->addConnectConfiguration($rootNode);
         $this->addResourceOwnersConfiguration($rootNode);
 
@@ -431,21 +430,6 @@ private function addResourceOwnersConfiguration(ArrayNodeDefinition $node): void
         ;
     }
 
-    private function addHttpClientConfiguration(ArrayNodeDefinition $node): void
-    {
-        $node
-            ->children()
-                ->arrayNode('http')
-                    ->addDefaultsIfNotSet()
-                    ->children()
-                        ->scalarNode('client')->defaultValue('httplug.client.default')->end()
-                        ->scalarNode('message_factory')->defaultValue('httplug.message_factory.default')->end()
-                    ->end()
-                ->end()
-            ->end()
-        ;
-    }
-
     private function addConnectConfiguration(ArrayNodeDefinition $node): void
     {
         $node

diff --git a/DependencyInjection/HWIOAuthExtension.php b/DependencyInjection/HWIOAuthExtension.php
@@ -20,9 +20,12 @@
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Exception\BadMethodCallException;
 use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
+use Symfony\Component\DependencyInjection\Exception\OutOfBoundsException;
+use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException;
 use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
 use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\HttpKernel\DependencyInjection\Extension;
+use Symfony\Contracts\HttpClient\HttpClientInterface;
 
 /**
  * @author Geoffrey Bachelet <geoffrey.bachelet@gmail.com>
@@ -39,14 +42,13 @@ final class HWIOAuthExtension extends Extension
      * @throws InvalidConfigurationException
      * @throws BadMethodCallException
      * @throws InvalidArgumentException
-     * @throws \Symfony\Component\DependencyInjection\Exception\OutOfBoundsException
-     * @throws \Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException
+     * @throws OutOfBoundsException
+     * @throws ServiceNotFoundException
      */
     public function load(array $configs, ContainerBuilder $container)
     {
         $loader = new XmlFileLoader($container, new FileLocator(__DIR__.'/../Resources/config/'));
         $loader->load('controller.xml');
-        $loader->load('http_client.xml');
         $loader->load('oauth.xml');
         $loader->load('templating.xml');
         $loader->load('twig.xml');
@@ -55,8 +57,6 @@ public function load(array $configs, ContainerBuilder $container)
         $processor = new Processor();
         $config = $processor->processConfiguration(new Configuration(), $configs);
 
-        $this->createHttplugClient($container, $config);
-
         // set current firewall
         if (empty($config['firewall_names'])) {
             throw new InvalidConfigurationException('The child node "firewall_names" at path "hwi_oauth" must be configured.');
@@ -110,7 +110,7 @@ public function load(array $configs, ContainerBuilder $container)
      * @throws BadMethodCallException
      * @throws InvalidArgumentException
      */
-    public function createResourceOwnerService(ContainerBuilder $container, $name, array $options)
+    public function createResourceOwnerService(ContainerBuilder $container, string $name, array $options): void
     {
         // alias services
         if (isset($options['service'])) {
@@ -137,8 +137,9 @@ public function createResourceOwnerService(ContainerBuilder $container, $name, a
             $definition->setClass("%hwi_oauth.resource_owner.$type.class%");
         }
 
-        $definition->replaceArgument(2, $options);
-        $definition->replaceArgument(3, $name);
+        $definition->replaceArgument('$httpClient', new Reference(HttpClientInterface::class));
+        $definition->replaceArgument('$options', $options);
+        $definition->replaceArgument('$name', $name);
         $definition->setPublic(true);
 
         $container->setDefinition('hwi_oauth.resource_owner.'.$name, $definition);
@@ -152,23 +153,6 @@ public function getAlias()
         return 'hwi_oauth';
     }
 
-    protected function createHttplugClient(ContainerBuilder $container, array $config)
-    {
-        $httpClientId = $config['http']['client'];
-        $httpMessageFactoryId = $config['http']['message_factory'];
-        $bundles = $container->getParameter('kernel.bundles');
-
-        if ('httplug.client.default' === $httpClientId && !isset($bundles['HttplugBundle'])) {
-            throw new InvalidConfigurationException('You must setup php-http/httplug-bundle to use the default http client service.');
-        }
-        if ('httplug.message_factory.default' === $httpMessageFactoryId && !isset($bundles['HttplugBundle'])) {
-            throw new InvalidConfigurationException('You must setup php-http/httplug-bundle to use the default http message factory service.');
-        }
-
-        $container->setAlias('hwi_oauth.http.client', new Alias($config['http']['client'], true));
-        $container->setAlias('hwi_oauth.http.message_factory', new Alias($config['http']['message_factory'], true));
-    }
-
     /**
      * Check of the connect controllers etc should be enabled.
      *

diff --git a/OAuth/ResourceOwner/AbstractResourceOwner.php b/OAuth/ResourceOwner/AbstractResourceOwner.php
@@ -11,20 +11,21 @@
 
 namespace HWI\Bundle\OAuthBundle\OAuth\ResourceOwner;
 
-use Http\Client\Common\HttpMethodsClient;
-use Http\Client\Exception;
 use HWI\Bundle\OAuthBundle\OAuth\Exception\HttpTransportException;
 use HWI\Bundle\OAuthBundle\OAuth\RequestDataStorageInterface;
 use HWI\Bundle\OAuthBundle\OAuth\ResourceOwnerInterface;
 use HWI\Bundle\OAuthBundle\OAuth\Response\PathUserResponse;
 use HWI\Bundle\OAuthBundle\OAuth\Response\UserResponseInterface;
 use HWI\Bundle\OAuthBundle\OAuth\State\State;
 use HWI\Bundle\OAuthBundle\OAuth\StateInterface;
-use Psr\Http\Message\ResponseInterface;
+use Symfony\Component\HttpClient\Exception\JsonException;
 use Symfony\Component\HttpFoundation\Request as HttpRequest;
 use Symfony\Component\OptionsResolver\OptionsResolver;
 use Symfony\Component\Security\Core\Exception\AuthenticationException;
 use Symfony\Component\Security\Http\HttpUtils;
+use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
+use Symfony\Contracts\HttpClient\HttpClientInterface;
+use Symfony\Contracts\HttpClient\ResponseInterface;
 
 /**
  * AbstractResourceOwner.
@@ -47,7 +48,7 @@ abstract class AbstractResourceOwner implements ResourceOwnerInterface
     protected $paths = [];
 
     /**
-     * @var HttpMethodsClient
+     * @var HttpClientInterface
      */
     protected $httpClient;
 
@@ -77,14 +78,11 @@ abstract class AbstractResourceOwner implements ResourceOwnerInterface
     private $stateLoaded = false;
 
     /**
-     * @param HttpMethodsClient           $httpClient Httplug client
-     * @param HttpUtils                   $httpUtils  Http utils
-     * @param array                       $options    Options for the resource owner
-     * @param string                      $name       Name for the resource owner
-     * @param RequestDataStorageInterface $storage    Request token storage
+     * @param array  $options Options for the resource owner
+     * @param string $name    Name for the resource owner
      */
     public function __construct(
-        HttpMethodsClient $httpClient,
+        HttpClientInterface $httpClient,
         HttpUtils $httpUtils,
         array $options,
         string $name,
@@ -303,46 +301,41 @@ protected function httpRequest($url, $content = null, array $headers = [], $meth
             $method = null === $content || '' === $content ? 'GET' : 'POST';
         }
 
-        $headers += ['User-Agent' => 'HWIOAuthBundle (https://github.com/hwi/HWIOAuthBundle)'];
+        $options = ['headers' => $headers];
+        $options['headers'] += ['User-Agent' => 'HWIOAuthBundle (https://github.com/hwi/HWIOAuthBundle)'];
         if (\is_string($content)) {
-            if (!isset($headers['Content-Length'])) {
-                $headers += ['Content-Length' => (string) \strlen($content)];
+            if (!isset($options['headers']['Content-Length'])) {
+                $options['headers'] += ['Content-Length' => (string) \strlen($content)];
             }
         } elseif (\is_array($content)) {
-            $content = http_build_query($content, '', '&');
+            $options['body'] = $content;
         }
 
         try {
-            return $this->httpClient->send(
+            return $this->httpClient->request(
                 $method,
                 $url,
-                $headers,
-                $content
+                $options
             );
-        } catch (Exception $e) {
+        } catch (TransportExceptionInterface $e) {
             throw new HttpTransportException('Error while sending HTTP request', $this->getName(), $e->getCode(), $e);
         }
     }
 
-    /**
-     * Get the 'parsed' content based on the response headers.
-     *
-     * @return array
-     */
-    protected function getResponseContent(ResponseInterface $rawResponse)
+    protected function getResponseContent(ResponseInterface $rawResponse): array
     {
-        // First check that content in response exists, due too bug: https://bugs.php.net/bug.php?id=54484
-        $content = (string) $rawResponse->getBody();
-        if (!$content) {
-            return [];
-        }
+        $contentTypes = $rawResponse->getHeaders(false)['content-type'] ?? [];
+        if (\in_array('text/plain', $contentTypes, true)) {
+            parse_str($rawResponse->getContent(false), $response);
 
-        $response = json_decode($content, true);
-        if (\JSON_ERROR_NONE !== json_last_error()) {
-            parse_str($content, $response);
+            return $response;
         }
 
-        return $response;
+        try {
+            return $rawResponse->toArray(false);
+        } catch (JsonException $e) {
+            return [];
+        }
     }
 
     /**

diff --git a/OAuth/ResourceOwner/AppleResourceOwner.php b/OAuth/ResourceOwner/AppleResourceOwner.php
@@ -52,10 +52,10 @@ public function getAuthorizationUrl($redirectUri, array $extraParameters = [])
     public function getUserInformation(array $accessToken, array $extraParameters = [])
     {
         if (!isset($accessToken['id_token'])) {
-            throw new \Exception('Undefined index id_token');
+            throw new \InvalidArgumentException('Undefined index id_token');
         }
 
-        $jwt = self::jwt_decode($accessToken['id_token']);
+        $jwt = self::jwtDecode($accessToken['id_token']);
         $data = json_decode(base64_decode($jwt), true);
 
         if (isset($accessToken['firstName'], $accessToken['lastName'])) {
@@ -105,9 +105,10 @@ public function getAccessToken(Request $request, $redirectUri, array $extraParam
      */
     public function refreshAccessToken($refreshToken, array $extraParameters = [])
     {
-        $parameters = [];
-        $parameters['client_id'] = $this->options['client_id'];
-        $parameters['client_secret'] = $this->options['client_secret'];
+        $parameters = [
+            'client_id' => $this->options['client_id'],
+            'client_secret' => $this->options['client_secret'],
+        ];
 
         return parent::refreshAccessToken($refreshToken, array_merge($parameters, $extraParameters));
     }
@@ -130,7 +131,6 @@ protected function configureOptions(OptionsResolver $resolver)
         $resolver->setDefaults([
             'authorization_url' => 'https://appleid.apple.com/auth/authorize',
             'access_token_url' => 'https://appleid.apple.com/auth/token',
-            'revoke_token_url' => '',
             'infos_url' => '',
             'use_commas_in_scope' => false,
             'display' => null,
@@ -140,10 +140,10 @@ protected function configureOptions(OptionsResolver $resolver)
         ]);
     }
 
-    private static function jwt_decode($id_token)
+    private static function jwtDecode(string $idToken)
     {
         //// from http://stackoverflow.com/a/28748285/624544
-        [, $jwt] = explode('.', $id_token, 3);
+        [, $jwt] = explode('.', $idToken, 3);
 
         // if the token was urlencoded, do some fixes to ensure that it is valid base64 encoded
         $jwt = str_replace(['-', '_'], ['+', '/'], $jwt);

diff --git a/OAuth/ResourceOwner/DropboxResourceOwner.php b/OAuth/ResourceOwner/DropboxResourceOwner.php
@@ -11,10 +11,13 @@
 
 namespace HWI\Bundle\OAuthBundle\OAuth\ResourceOwner;
 
+use HWI\Bundle\OAuthBundle\OAuth\Exception\HttpTransportException;
 use HWI\Bundle\OAuthBundle\OAuth\Response\UserResponseInterface;
 use HWI\Bundle\OAuthBundle\Security\Core\Authentication\Token\OAuthToken;
-use Psr\Http\Message\ResponseInterface;
+use Symfony\Component\HttpClient\Exception\JsonException;
 use Symfony\Component\OptionsResolver\OptionsResolver;
+use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
+use Symfony\Contracts\HttpClient\ResponseInterface;
 
 /**
  * DropboxResourceOwner.
@@ -45,8 +48,7 @@ public function getUserInformation(array $accessToken,
     ) {
         if ($this->options['use_bearer_authorization']) {
             $content = $this->httpRequest(
-                $this->normalizeUrl($this->options['infos_url'],
-                    $extraParameters),
+                $this->normalizeUrl($this->options['infos_url'], $extraParameters),
                 'null',
                 [
                     'Authorization' => 'Bearer'.' '.$accessToken['access_token'],
@@ -57,18 +59,21 @@ public function getUserInformation(array $accessToken,
             $content = $this->doGetUserInformationRequest(
                 $this->normalizeUrl(
                     $this->options['infos_url'],
-                    array_merge([$this->options['attr_name'] => $accessToken['access_token']],
-                        $extraParameters)
+                    array_merge([$this->options['attr_name'] => $accessToken['access_token']], $extraParameters)
                 )
             );
         }
 
-        $response = $this->getUserResponse();
-        $response->setData($content instanceof ResponseInterface ? (string) $content->getBody() : $content);
-        $response->setResourceOwner($this);
-        $response->setOAuthToken(new OAuthToken($accessToken));
+        try {
+            $response = $this->getUserResponse();
+            $response->setData($content instanceof ResponseInterface ? $content->toArray(false) : $content);
+            $response->setResourceOwner($this);
+            $response->setOAuthToken(new OAuthToken($accessToken));
 
-        return $response;
+            return $response;
+        } catch (TransportExceptionInterface | JsonException $e) {
+            throw new HttpTransportException('Error while sending HTTP request', $this->getName(), $e->getCode(), $e);
+        }
     }
 
     /**

diff --git a/OAuth/ResourceOwner/FiwareResourceOwner.php b/OAuth/ResourceOwner/FiwareResourceOwner.php
@@ -15,6 +15,7 @@
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\OptionsResolver\Options;
 use Symfony\Component\OptionsResolver\OptionsResolver;
+use Symfony\Contracts\HttpClient\ResponseInterface;
 
 /**
  * FiwareResourceOwner.
@@ -86,7 +87,7 @@ public function getUserInformation(array $accessToken, array $extraParameters =
         }
 
         $response = $this->getUserResponse();
-        $response->setData((string) $content->getBody());
+        $response->setData($content instanceof ResponseInterface ? $content->getContent() : $content);
         $response->setResourceOwner($this);
         $response->setOAuthToken(new OAuthToken($accessToken));
 

diff --git a/OAuth/ResourceOwner/FoursquareResourceOwner.php b/OAuth/ResourceOwner/FoursquareResourceOwner.php
@@ -11,8 +11,8 @@
 
 namespace HWI\Bundle\OAuthBundle\OAuth\ResourceOwner;
 
-use Psr\Http\Message\ResponseInterface;
 use Symfony\Component\OptionsResolver\OptionsResolver;
+use Symfony\Contracts\HttpClient\ResponseInterface;
 
 /**
  * FoursquareResourceOwner.
@@ -39,14 +39,14 @@ class FoursquareResourceOwner extends GenericOAuth2ResourceOwner
     /**
      * {@inheritdoc}
      */
-    protected function getResponseContent(ResponseInterface $rawResponse)
+    protected function getResponseContent(ResponseInterface $rawResponse): array
     {
         $response = parent::getResponseContent($rawResponse);
 
         // Foursquare use quite custom response structure in case of error
         if (isset($response['meta']['errorType'])) {
             // Prevent to mark deprecated calls as errors
-            if (200 == $response['meta']['code']) {
+            if (200 === (int) $response['meta']['code']) {
                 $response['error'] = $response['meta']['errorType'];
                 // Try to add some details of error if available
                 if (isset($response['meta']['errorMessage'])) {