Rework resource owners to use Symfony Http Client internally

hwi · Jan 5, 2021 · 601a256 · 601a256
1 parent fc6192d
commit 601a256
Show file tree

Hide file tree

Showing 76 changed files with 1,348 additions and 1,152 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,9 @@
 Changelog
 =========
+## 2.0.0 (2021-xx-xx)
+* BC Break: replaced `php-http/httplug-bundle` with `symfony/http-client`
+* BC Break: removed `hwi_oauth.http` configuration
+
 ## 1.3.0 (2021-01-03)
 * BC Break: dropped support for Symfony `<4.4`,
 * BC Break: dropped support for Doctrine Bundle `<2.0`,

diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php
@@ -185,7 +185,6 @@ public function getConfigTreeBuilder()
             ->end()
         ;
 
-        $this->addHttpClientConfiguration($rootNode);
         $this->addConnectConfiguration($rootNode);
         $this->addFosubConfiguration($rootNode);
         $this->addResourceOwnersConfiguration($rootNode);
@@ -437,21 +436,6 @@ private function addResourceOwnersConfiguration(ArrayNodeDefinition $node)
         ;
     }
 
-    private function addHttpClientConfiguration(ArrayNodeDefinition $node)
-    {
-        $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)
     {
         $node

diff --git a/DependencyInjection/HWIOAuthExtension.php b/DependencyInjection/HWIOAuthExtension.php
@@ -44,7 +44,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('templating.xml');
         $loader->load('twig.xml');
@@ -53,8 +52,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.');
@@ -150,27 +147,6 @@ public function getAlias()
         return 'hwi_oauth';
     }
 
-    /**
-     * @param ContainerBuilder $container
-     * @param array            $config
-     */
-    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,22 @@
 
 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\HttpClient\HttpClient;
 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 +49,7 @@ abstract class AbstractResourceOwner implements ResourceOwnerInterface
     protected $paths = [];
 
     /**
-     * @var HttpMethodsClient
+     * @var HttpClient
      */
     protected $httpClient;
 
@@ -77,20 +79,20 @@ 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 HttpClientInterface|null    $httpClient
      */
     public function __construct(
-        HttpMethodsClient $httpClient,
         HttpUtils $httpUtils,
         array $options,
         string $name,
-        RequestDataStorageInterface $storage
+        RequestDataStorageInterface $storage,
+        HttpClientInterface $httpClient = null
     ) {
-        $this->httpClient = $httpClient;
+        $this->httpClient = $httpClient ?: HttpClient::create();
         $this->httpUtils = $httpUtils;
         $this->name = $name;
         $this->storage = $storage;
@@ -304,48 +306,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.
-     *
-     * @param ResponseInterface $rawResponse
-     *
-     * @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 json_decode($rawResponse->getContent(false), true) ?: [];
+        }
     }
 
     /**

diff --git a/OAuth/ResourceOwner/AppleResourceOwner.php b/OAuth/ResourceOwner/AppleResourceOwner.php
@@ -50,10 +50,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'])) {
@@ -103,9 +103,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));
     }
@@ -128,7 +129,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,
@@ -138,10 +138,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.
@@ -46,8 +49,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'],
@@ -58,18 +60,23 @@ 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 (JsonException $e) {
+            throw new HttpTransportException('Error while sending HTTP request', $this->getName(), $e->getCode(), $e);
+        } catch (TransportExceptionInterface $e) {
+            throw new HttpTransportException('Error while sending HTTP request', $this->getName(), $e->getCode(), $e);
+        }
     }
 
     /**

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.
@@ -37,14 +37,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
@@ -15,10 +15,10 @@
 use HWI\Bundle\OAuthBundle\Security\Helper\NonceGenerator;
 use HWI\Bundle\OAuthBundle\Security\OAuthErrorHandler;
 use HWI\Bundle\OAuthBundle\Security\OAuthUtils;
-use Psr\Http\Message\ResponseInterface;
 use Symfony\Component\HttpFoundation\Request as HttpRequest;
 use Symfony\Component\OptionsResolver\OptionsResolver;
 use Symfony\Component\Security\Core\Exception\AuthenticationException;
+use Symfony\Contracts\HttpClient\ResponseInterface;
 
 /**
  * GenericOAuth1ResourceOwner.
@@ -54,7 +54,7 @@ public function getUserInformation(array $accessToken, array $extraParameters =
         $content = $this->doGetUserInformationRequest($url, $parameters);
 
         $response = $this->getUserResponse();
-        $response->setData($content instanceof ResponseInterface ? (string) $content->getBody() : $content);
+        $response->setData($content instanceof ResponseInterface ? $content->getContent() : $content);
         $response->setResourceOwner($this);
         $response->setOAuthToken(new OAuthToken($accessToken));