Upgrade xml-libraries to their 2.x versions (using typed values) (#62)

* Bump dependencies

* Migrate calls to xml-libraries to use the new interface

* Use local version of phpcs

* Replace dataProvider annotation with attribute

* Fix sniffer-issues

* Fix phpstan-issues

Author: tvdijen

.github/workflows/php.yml

@@ -162,7 +162,7 @@ jobs:
         with:
           # Should be the higest supported version, so we can use the newest tools
           php-version: '8.5'
-          tools: composer, composer-require-checker, composer-unused, phpcs
+          tools: composer, composer-require-checker, composer-unused
           extensions: ctype, date, dom, fileinfo, filter, hash, intl, mbstring, openssl, pcre, spl, xml
 
       - name: Setup problem matchers for PHP

composer.json

@@ -37,18 +37,19 @@
         "ext-SimpleXML": "*",
         "ext-session": "*",
 
-        "simplesamlphp/assert": "^1.8",
-        "simplesamlphp/composer-module-installer": "~1.5",
-        "simplesamlphp/simplesamlphp": "~2.5@RC",
-        "simplesamlphp/xml-cas": "~1.3",
-        "simplesamlphp/xml-common": "~1.17",
-        "simplesamlphp/xml-soap": "~1.5",
-        "symfony/http-foundation": "^7.4",
-        "symfony/http-kernel": "^7.4",
-        "simplesamlphp/saml11": "~1.3"
+        "beste/clock": "~3.0",
+        "simplesamlphp/assert": "~1.9",
+        "simplesamlphp/composer-module-installer": "~1.6",
+        "simplesamlphp/simplesamlphp": "~2.5@dev",
+        "simplesamlphp/xml-cas": "~2.0",
+        "simplesamlphp/xml-common": "~2.0",
+        "simplesamlphp/xml-soap": "~2.0",
+        "symfony/http-foundation": "~7.4",
+        "symfony/http-kernel": "~7.4",
+        "simplesamlphp/saml11": "~2.0"
     },
     "require-dev": {
-        "simplesamlphp/simplesamlphp-test-framework": "^1.11"
+        "simplesamlphp/simplesamlphp-test-framework": "~1.11"
     },
     "support": {
         "issues": "https://github.com/simplesamlphp/simplesamlphp-module-casserver/issues",

phpstan-baseline.neon

@@ -1,36 +1,43 @@
 parameters:
 	ignoreErrors:
 		-
-			message: "#^Property SimpleSAML\\\\Module\\\\casserver\\\\Tests\\\\Controller\\\\Cas10ControllerTest\\:\\:\\$sessionMock is never read, only written\\.$#"
+			message: '#^Property SimpleSAML\\Module\\casserver\\Tests\\Controller\\Cas10ControllerTest\:\:\$sessionMock is never read, only written\.$#'
+			identifier: property.onlyWritten
 			count: 1
 			path: tests/src/Controller/Cas10ControllerTest.php
 
 		-
-			message: "#^Property SimpleSAML\\\\Module\\\\casserver\\\\Tests\\\\Controller\\\\Cas20ControllerTest\\:\\:\\$sessionMock is never read, only written\\.$#"
+			message: '#^Property SimpleSAML\\Module\\casserver\\Tests\\Controller\\Cas20ControllerTest\:\:\$sessionMock is never read, only written\.$#'
+			identifier: property.onlyWritten
 			count: 1
 			path: tests/src/Controller/Cas20ControllerTest.php
 
 		-
-			message: "#^Property SimpleSAML\\\\Module\\\\casserver\\\\Tests\\\\Controller\\\\Cas20ControllerTest\\:\\:\\$ticketValidatorMock is never read, only written\\.$#"
+			message: '#^Property SimpleSAML\\Module\\casserver\\Tests\\Controller\\Cas20ControllerTest\:\:\$ticketValidatorMock is never read, only written\.$#'
+			identifier: property.onlyWritten
 			count: 1
 			path: tests/src/Controller/Cas20ControllerTest.php
 
 		-
-			message: "#^Call to an undefined method SimpleSAML\\\\Module\\\\casserver\\\\Cas\\\\TicketValidator\\:\\:expects\\(\\)\\.$#"
+			message: '#^Call to an undefined method SimpleSAML\\Module\\casserver\\Cas\\TicketValidator\:\:expects\(\)\.$#'
+			identifier: method.notFound
 			count: 2
 			path: tests/src/Controller/Cas30ControllerTest.php
 
 		-
-			message: "#^Property SimpleSAML\\\\Module\\\\casserver\\\\Tests\\\\Controller\\\\Cas30ControllerTest\\:\\:\\$sessionMock is never read, only written\\.$#"
+			message: '#^Property SimpleSAML\\Module\\casserver\\Tests\\Controller\\Cas30ControllerTest\:\:\$sessionMock is never read, only written\.$#'
+			identifier: property.onlyWritten
 			count: 1
 			path: tests/src/Controller/Cas30ControllerTest.php
 
 		-
-			message: "#^Parameter \\#2 \\$renew of method SimpleSAML\\\\Module\\\\casserver\\\\Controller\\\\LoginController\\:\\:login\\(\\) expects bool, string given\\.$#"
+			message: '#^Parameter \#2 \$renew of method SimpleSAML\\Module\\casserver\\Controller\\LoginController\:\:login\(\) expects bool, string given\.$#'
+			identifier: argument.type
 			count: 1
 			path: tests/src/Controller/LoginControllerTest.php
 
 		-
-			message: "#^Property SimpleSAML\\\\Module\\\\casserver\\\\Tests\\\\Controller\\\\LoginControllerTest\\:\\:\\$sspContainer is never read, only written\\.$#"
+			message: '#^Property SimpleSAML\\Module\\casserver\\Tests\\Controller\\LoginControllerTest\:\:\$sspContainer is never read, only written\.$#'
+			identifier: property.onlyWritten
 			count: 1
 			path: tests/src/Controller/LoginControllerTest.php

phpstan-dev-baseline.neon

@@ -16,29 +16,23 @@
  */
 class AttributeExtractor
 {
-    /** @var Configuration */
-    private Configuration $casconfig;
-
-    /** @var ProcessingChainFactory  */
-    private ProcessingChainFactory $processingChainFactory;
-
-    /** @var State */
+    /** @var \SimpleSAML\Auth\State */
     private State $authState;
 
     /**
      * ID of the Authentication Source used during authn.
      */
     private ?string $authSourceId = null;
 
+
     public function __construct(
-        Configuration $casconfig,
-        ProcessingChainFactory $processingChainFactory,
+        protected Configuration $casconfig,
+        protected ProcessingChainFactory $processingChainFactory,
     ) {
-        $this->casconfig = $casconfig;
-        $this->processingChainFactory = $processingChainFactory;
         $this->authState = new State();
     }
 
+
     /**
      * Determine the user and any CAS attributes based on the attributes from the
      * authsource and the CAS configuration.
@@ -97,6 +91,7 @@ public function extractUserAndAttributes(?array $state): array
         ];
     }
 
+
     /**
      * Run authproc filters with the processing chain
      * Creating the ProcessingChain require metadata.
@@ -129,14 +124,15 @@ protected function runAuthProcs(array &$state): void
         $this->processingChainFactory->build($state)->processState($state);
     }
 
+
     /**
      * This is a wrapper around Auth/State::loadState that facilitates testing by
      * hiding the static method
      *
      * @param   string  $stateId
      *
      * @return array|null
-     * @throws NoState
+     * @throws \SimpleSAML\Error\NoState
      */
     public function manageState(string $stateId): ?array
     {
@@ -154,6 +150,7 @@ public function manageState(string $stateId): ?array
         return $state;
     }
 
+
     /**
      * @param   string  $id
      * @param   string  $stage

src/Cas/AttributeExtractor.php

@@ -12,20 +12,20 @@
  */
 class CasException extends Exception
 {
-    /** @var string */
-    private string $casCode;
-
     /**
      * CasException constructor.
+     *
      * @param string $casCode
      * @param string $message
      */
-    public function __construct(string $casCode, string $message)
-    {
+    public function __construct(
+        protected string $casCode,
+        string $message,
+    ) {
         parent::__construct($message);
-        $this->casCode = $casCode;
     }
 
+
     /**
      * @return string
      */

src/Cas/CasException.php

@@ -16,15 +16,12 @@
 
 class ProcessingChainFactory
 {
-    /** @var Configuration */
-    private Configuration $casconfig;
-
     public function __construct(
-        Configuration $casconfig,
+        protected Configuration $casconfig,
     ) {
-        $this->casconfig = $casconfig;
     }
 
+
     /**
      * @codeCoverageIgnore
      * @throws \Exception

src/Cas/Factories/ProcessingChainFactory.php

@@ -25,27 +25,33 @@
 
 namespace SimpleSAML\Module\casserver\Cas\Protocol;
 
-use DateTimeImmutable;
-use SimpleSAML\CAS\XML\cas\Attributes;
-use SimpleSAML\CAS\XML\cas\AuthenticationDate;
-use SimpleSAML\CAS\XML\cas\AuthenticationFailure;
-use SimpleSAML\CAS\XML\cas\AuthenticationSuccess;
-use SimpleSAML\CAS\XML\cas\IsFromNewLogin;
-use SimpleSAML\CAS\XML\cas\LongTermAuthenticationRequestTokenUsed;
-use SimpleSAML\CAS\XML\cas\ProxyFailure;
-use SimpleSAML\CAS\XML\cas\ProxyGrantingTicket;
-use SimpleSAML\CAS\XML\cas\ProxySuccess;
-use SimpleSAML\CAS\XML\cas\ProxyTicket;
-use SimpleSAML\CAS\XML\cas\ServiceResponse;
-use SimpleSAML\CAS\XML\cas\User;
+use Beste\Clock\LocalizedClock;
+use DateTimeZone;
+use SimpleSAML\Assert\AssertionFailedException;
+use SimpleSAML\CAS\Type\CodeValue;
+use SimpleSAML\CAS\XML\Attributes;
+use SimpleSAML\CAS\XML\AuthenticationDate;
+use SimpleSAML\CAS\XML\AuthenticationFailure;
+use SimpleSAML\CAS\XML\AuthenticationSuccess;
+use SimpleSAML\CAS\XML\IsFromNewLogin;
+use SimpleSAML\CAS\XML\LongTermAuthenticationRequestTokenUsed;
+use SimpleSAML\CAS\XML\ProxyFailure;
+use SimpleSAML\CAS\XML\ProxyGrantingTicket;
+use SimpleSAML\CAS\XML\ProxySuccess;
+use SimpleSAML\CAS\XML\ProxyTicket;
+use SimpleSAML\CAS\XML\ServiceResponse;
+use SimpleSAML\CAS\XML\User;
 use SimpleSAML\Configuration;
 use SimpleSAML\Logger;
+use SimpleSAML\XML\Assert\Assert;
 use SimpleSAML\XML\Chunk;
 use SimpleSAML\XML\DOMDocumentFactory;
+use SimpleSAML\XMLSchema\Type\BooleanValue;
+use SimpleSAML\XMLSchema\Type\DateTimeValue;
+use SimpleSAML\XMLSchema\Type\StringValue;
 
 use function base64_encode;
 use function count;
-use function filter_var;
 use function is_null;
 use function is_string;
 use function str_replace;
@@ -117,27 +123,28 @@ public function getProxyGrantingTicketIOU(): ?string
 
     /**
      * @param string $username
-     * @return \SimpleSAML\CAS\XML\cas\ServiceResponse
+     * @return \SimpleSAML\CAS\XML\ServiceResponse
      */
     public function getValidateSuccessResponse(string $username): ServiceResponse
     {
-        $user = new User($username);
+        $user = new User(StringValue::fromString($username));
 
         $proxyGrantingTicket = null;
         if (is_string($this->proxyGrantingTicketIOU)) {
-            $proxyGrantingTicket = new ProxyGrantingTicket($this->proxyGrantingTicketIOU);
+            $proxyGrantingTicket = new ProxyGrantingTicket(StringValue::fromString($this->proxyGrantingTicketIOU));
         }
 
         $attr = [];
         if ($this->sendAttributes && count($this->attributes) > 0) {
             foreach ($this->attributes as $name => $values) {
                 // Fix the most common cause of invalid XML elements
                 $_name = str_replace(':', '_', $name);
-                if ($this->isValidXmlName($_name) === true) {
+                try {
+                    Assert::validNCName($_name);
                     foreach ($values as $value) {
                         $attr[] = $this->generateCas20Attribute($_name, $value);
                     }
-                } else {
+                } catch (AssertionFailedException) {
                     Logger::warning("DOMException creating attribute '$_name'. Continuing without attribute'");
                 }
             }
@@ -150,10 +157,11 @@ public function getValidateSuccessResponse(string $username): ServiceResponse
             }
         }
 
+        $systemClock = LocalizedClock::in(new DateTimeZone('Z'));
         $attributes = new Attributes(
-            new AuthenticationDate(new DateTimeImmutable('now')),
-            new LongTermAuthenticationRequestTokenUsed('true'),
-            new IsFromNewLogin('true'),
+            new AuthenticationDate(DateTimeValue::now($systemClock)),
+            new LongTermAuthenticationRequestTokenUsed(BooleanValue::fromBoolean(true)),
+            new IsFromNewLogin(BooleanValue::fromBoolean(true)),
             $attr,
         );
 
@@ -167,11 +175,14 @@ public function getValidateSuccessResponse(string $username): ServiceResponse
     /**
      * @param string $errorCode
      * @param string $explanation
-     * @return \SimpleSAML\CAS\XML\cas\ServiceResponse
+     * @return \SimpleSAML\CAS\XML\ServiceResponse
      */
     public function getValidateFailureResponse(string $errorCode, string $explanation): ServiceResponse
     {
-        $authenticationFailure = new AuthenticationFailure($explanation, $errorCode);
+        $authenticationFailure = new AuthenticationFailure(
+            StringValue::fromString($explanation),
+            CodeValue::fromString($errorCode),
+        );
         $serviceResponse = new ServiceResponse($authenticationFailure);
 
         return $serviceResponse;
@@ -180,11 +191,11 @@ public function getValidateFailureResponse(string $errorCode, string $explanatio
 
     /**
      * @param string $proxyTicketId
-     * @return \SimpleSAML\CAS\XML\cas\ServiceResponse
+     * @return \SimpleSAML\CAS\XML\ServiceResponse
      */
     public function getProxySuccessResponse(string $proxyTicketId): ServiceResponse
     {
-        $proxyTicket = new ProxyTicket($proxyTicketId);
+        $proxyTicket = new ProxyTicket(StringValue::fromString($proxyTicketId));
         $proxySuccess = new ProxySuccess($proxyTicket);
         $serviceResponse = new ServiceResponse($proxySuccess);
 
@@ -195,11 +206,14 @@ public function getProxySuccessResponse(string $proxyTicketId): ServiceResponse
     /**
      * @param string $errorCode
      * @param string $explanation
-     * @return \SimpleSAML\CAS\XML\cas\ServiceResponse
+     * @return \SimpleSAML\CAS\XML\ServiceResponse
      */
     public function getProxyFailureResponse(string $errorCode, string $explanation): ServiceResponse
     {
-        $proxyFailure = new ProxyFailure($explanation, $errorCode);
+        $proxyFailure = new ProxyFailure(
+            StringValue::fromString($explanation),
+            CodeValue::fromString($errorCode),
+        );
         $serviceResponse = new ServiceResponse($proxyFailure);
 
         return $serviceResponse;
@@ -222,26 +236,4 @@ private function generateCas20Attribute(
 
         return new Chunk($attributeElement);
     }
-
-
-    /**
-     * XML element names have a lot of rules and not every SAML attribute name can be converted.
-     * Ref: https://www.w3.org/TR/REC-xml/#NT-NameChar
-     * https://stackoverflow.com/q/2519845/54396
-     * must only start with letter or underscore
-     * cannot start with 'xml' (or maybe it can - stackoverflow commenters don't agree)
-     * cannot contain a ':' since those are for namespaces
-     * cannot contains space
-     * can only  contain letters, digits, hyphens, underscores, and periods
-     * @param string $name The attribute name to be used as an element
-     * @return bool true if $name would make a valid xml element.
-     */
-    private function isValidXmlName(string $name): bool
-    {
-        return filter_var(
-            $name,
-            FILTER_VALIDATE_REGEXP,
-            ['options' => ['regexp' => '/^[a-zA-Z_][\w.-]*$/']],
-        ) !== false;
-    }
 }