Rework resource owners to use Symfony Http Client internally

hwi · Aug 6, 2021 · 545eafe · 545eafe
1 parent 0be50db
commit 545eafe
Show file tree

Hide file tree

Showing 95 changed files with 1,375 additions and 1,182 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,6 +16,9 @@ Changelog
 * BC Break: `ResourceOwnerMap` uses service locator instead of DI container,
 * BC Break: Removed abstract services: `hwi_oauth.abstract_resource_owner.generic`, `hwi_oauth.abstract_resource_owner.oauth1` & `hwi_oauth.abstract_resource_owner.oauth2`,
 * BC Break: Removed `setName()` method from `OAuth/ResourceOwnerInterface`,
+* 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
@@ -48,7 +48,6 @@ 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('resource_owners.xml');
         $loader->load('templating.xml');
@@ -58,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.');
@@ -156,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,24 +11,23 @@
 
 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.
- *
  * @author Geoffrey Bachelet <geoffrey.bachelet@gmail.com>
  * @author Alexander <iam.asm89@gmail.com>
  * @author Francisco Facioni <fran6co@gmail.com>
@@ -47,7 +46,7 @@ abstract class AbstractResourceOwner implements ResourceOwnerInterface
     protected $paths = [];
 
     /**
-     * @var HttpMethodsClient
+     * @var HttpClientInterface
      */
     protected $httpClient;
 
@@ -77,14 +76,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,
@@ -295,46 +291,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,9 +11,12 @@
 
 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 Symfony\Component\HttpClient\Exception\JsonException;
 use Symfony\Component\OptionsResolver\OptionsResolver;
+use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
 
 /**
  * DropboxResourceOwner.
@@ -44,8 +47,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'],
@@ -56,18 +58,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((string) $content->getBody());
-        $response->setResourceOwner($this);
-        $response->setOAuthToken(new OAuthToken($accessToken));
+        try {
+            $response = $this->getUserResponse();
+            $response->setData($content->toArray(false));
+            $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
@@ -86,7 +86,7 @@ public function getUserInformation(array $accessToken, array $extraParameters =
         }
 
         $response = $this->getUserResponse();
-        $response->setData((string) $content->getBody());
+        $response->setData($content->getContent());
         $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'])) {

diff --git a/OAuth/ResourceOwner/GenericOAuth1ResourceOwner.php b/OAuth/ResourceOwner/GenericOAuth1ResourceOwner.php
@@ -53,7 +53,7 @@ public function getUserInformation(array $accessToken, array $extraParameters =
         $content = $this->doGetUserInformationRequest($url, $parameters);
 
         $response = $this->getUserResponse();
-        $response->setData((string) $content->getBody());
+        $response->setData($content->getContent());
         $response->setResourceOwner($this);
         $response->setOAuthToken(new OAuthToken($accessToken));
 

diff --git a/OAuth/ResourceOwner/GenericOAuth2ResourceOwner.php b/OAuth/ResourceOwner/GenericOAuth2ResourceOwner.php
@@ -11,13 +11,16 @@
 
 namespace HWI\Bundle\OAuthBundle\OAuth\ResourceOwner;
 
+use HWI\Bundle\OAuthBundle\OAuth\Exception\HttpTransportException;
 use HWI\Bundle\OAuthBundle\Security\Core\Authentication\Token\OAuthToken;
 use HWI\Bundle\OAuthBundle\Security\Helper\NonceGenerator;
 use HWI\Bundle\OAuthBundle\Security\OAuthErrorHandler;
+use Symfony\Component\HttpClient\Exception\JsonException;
 use Symfony\Component\HttpFoundation\Request as HttpRequest;
 use Symfony\Component\OptionsResolver\Options;
 use Symfony\Component\OptionsResolver\OptionsResolver;
 use Symfony\Component\Security\Core\Exception\AuthenticationException;
+use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
 
 /**
  * @author Geoffrey Bachelet <geoffrey.bachelet@gmail.com>
@@ -45,12 +48,16 @@ public function getUserInformation(array $accessToken, array $extraParameters =
             );
         }
 
-        $response = $this->getUserResponse();
-        $response->setData((string) $content->getBody());
-        $response->setResourceOwner($this);
-        $response->setOAuthToken(new OAuthToken($accessToken));
-
-        return $response;
+        try {
+            $response = $this->getUserResponse();
+            $response->setData($content->toArray(false));
+            $response->setResourceOwner($this);
+            $response->setOAuthToken(new OAuthToken($accessToken));
+
+            return $response;
+        } catch (TransportExceptionInterface | JsonException $e) {
+            throw new HttpTransportException('Error while sending HTTP request', $this->getName(), $e->getCode(), $e);
+        }
     }
 
     /**