Merge pull request #13 from ioigoume/symfony-http-client

Replace legacy HTTP/Utils fetch with Symfony HTTP Client

Author: ioigoume

composer.json

@@ -44,8 +44,10 @@
         "simplesamlphp/simplesamlphp-module-ldap": "~1.2",
         "simplesamlphp/xml-cas-module-slate": "~1.1.0",
         "simplesamlphp/xml-cas": "^v2.2.0",
-        "simplesamlphp/xml-common": "~2.4.0",
-        "symfony/http-foundation": "~7.4.0"
+        "simplesamlphp/xml-common": "~2.4",
+        "symfony/http-foundation": "~7.4",
+        "symfony/http-client": "~7.4",
+        "symfony/http-client-contracts": "^3.5"
     },
     "require-dev": {
         "simplesamlphp/simplesamlphp-test-framework": "^1.10",

src/Auth/Source/CAS.php

@@ -21,6 +21,8 @@
 use SimpleSAML\Utils;
 use SimpleSAML\XML\Chunk;
 use SimpleSAML\XML\DOMDocumentFactory;
+use Symfony\Component\HttpClient\HttpClient;
+use Symfony\Contracts\HttpClient\HttpClientInterface;
 
 use function array_merge_recursive;
 use function preg_split;
@@ -76,11 +78,15 @@ class CAS extends Auth\Source
     private bool $useSlate;
 
     /**
-     * HTTP utility class for making requests and handling redirects.
-     * @var \SimpleSAML\Utils\HTTP
+     * HTTP utilities instance for handling redirects and URLs.
      */
     private Utils\HTTP $httpUtils;
 
+    /**
+     * Symfony HTTP client for CAS requests.
+     */
+    private HttpClientInterface $httpClient;
+
 
     /**
      * Constructor for this authentication source.
@@ -116,11 +122,30 @@ public function __construct(array $info, array $config)
     }
 
 
+    /**
+     * Initialize HttpClient instance
+     *
+     * @param \Symfony\Contracts\HttpClient\HttpClientInterface|null $httpClient Optional HTTP client instance to use
+     */
+    protected function initHttpClient(?HttpClientInterface $httpClient = null): void
+    {
+        if ($httpClient !== null) {
+            $this->httpClient = $httpClient;
+        } else {
+            $this->httpClient = $this->httpClient ?? HttpClient::create();
+        }
+    }
+
+
     /**
      * Initialize HTTP utilities instance
      *
      * @param \SimpleSAML\Utils\HTTP|null $httpUtils Optional HTTP utilities instance to use
      * @return void
+     * @deprecated This helper is kept only for the legacy authenticate(array &$state): void
+     *             flow. Once the Request-based authenticate(Request, array &$state): ?Response
+     *             API is active in SimpleSAMLphp, this method will be removed and HTTP
+     *             handling should be done via Symfony responses instead.
      */
     protected function initHttpUtils(?Utils\HTTP $httpUtils = null): void
     {
@@ -142,14 +167,16 @@ protected function initHttpUtils(?Utils\HTTP $httpUtils = null): void
      */
     private function casValidate(string $ticket, string $service): array
     {
-        $this->initHttpUtils();
-        $url = $this->httpUtils->addURLParameters($this->casConfig['validate'], [
-            'ticket' => $ticket,
-            'service' => $service,
+        $this->initHttpClient();
+
+        $response = $this->httpClient->request('GET', $this->casConfig['validate'], [
+            'query' => [
+                'ticket'  => $ticket,
+                'service' => $service,
+            ],
         ]);
 
-        /** @var string $result */
-        $result = $this->httpUtils->fetch($url);
+        $result = $response->getContent();
 
         /** @var list<string> $res */
         $res = preg_split("/\r?\n/", $result) ?: [];
@@ -172,19 +199,24 @@ private function casValidate(string $ticket, string $service): array
      */
     private function casServiceValidate(string $ticket, string $service): array
     {
-        $this->initHttpUtils();
-        $url = $this->httpUtils->addURLParameters(
-            $this->casConfig['serviceValidate'],
-            [
-                'ticket' => $ticket,
+        $this->initHttpClient();
+
+        $response = $this->httpClient->request('GET', $this->casConfig['serviceValidate'], [
+            'query' => [
+                'ticket'  => $ticket,
                 'service' => $service,
             ],
-        );
-        $result = $this->httpUtils->fetch($url);
+        ]);
+
+        $result = $response->getContent();
 
         /** @var string $result */
         $dom = DOMDocumentFactory::fromString($result);
 
+        // In practice that `if (...) return [];` branch is unreachable with the current behavior.
+        // `DOMDocumentFactory::fromString()`
+        // PHPStan still flags / cares about it because it only sees
+        // and has no way to know `null` won’t actually occur here. `DOMElement|null`
         if ($dom->documentElement === null) {
             return [];
         }
@@ -228,7 +260,7 @@ private function casServiceValidate(string $ticket, string $service): array
 
 
     /**
-     * Main validation method, redirects to correct method
+     * Main validation method, redirects to the correct method
      * (keeps finalStep clean)
      *
      * @param string $ticket