From d93068f7b9cce823d9fb5e7fdd3da14fd8c057cb Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Tue, 14 Mar 2023 18:55:43 +0100 Subject: [PATCH 01/27] Migrate to cas-lib --- composer.json | 4 ++++ src/Auth/Source/CAS.php | 51 ++++++++++++++++++++++------------------- 2 files changed, 32 insertions(+), 23 deletions(-) diff --git a/composer.json b/composer.json index b9817c8..2de892c 100644 --- a/composer.json +++ b/composer.json @@ -35,6 +35,10 @@ }, "require": { "php": "^8.1", + "ext-pcre": "*", + + "simplesamlphp/assert": "^0.8 || ^1.0", + "simplesamlphp/cas": "^1.0", "simplesamlphp/composer-module-installer": "^1.3.4", "simplesamlphp/simplesamlphp": "~2.4.0", "simplesamlphp/simplesamlphp-module-ldap": "~1.2", diff --git a/src/Auth/Source/CAS.php b/src/Auth/Source/CAS.php index 30ad6b6..6adc525 100644 --- a/src/Auth/Source/CAS.php +++ b/src/Auth/Source/CAS.php @@ -6,12 +6,16 @@ use DOMXpath; use Exception; -use SAML2\DOMDocumentFactory; use SimpleSAML\Auth; +use SimpleSAML\CAS\XML\cas\AuthenticationFailure; +use SimpleSAML\CAS\XML\cas\AuthenticationSuccess; +use SimpleSAML\CAS\XML\cas\ServiceResponse; +use SimpleSAML\CAS\Utils\XPath; use SimpleSAML\Configuration; use SimpleSAML\Module; use SimpleSAML\Module\ldap\Auth\Ldap; use SimpleSAML\Utils; +use SimpleSAML\XML\DOMDocumentFactory; use function array_key_exists; use function array_merge_recursive; @@ -151,34 +155,34 @@ private function casServiceValidate(string $ticket, string $service): array /** @var string $result */ $dom = DOMDocumentFactory::fromString($result); - $xPath = new DOMXpath($dom); - $xPath->registerNamespace("cas", 'http://www.yale.edu/tp/cas'); - $success = $xPath->query("/cas:serviceResponse/cas:authenticationSuccess/cas:user"); - if ($success === false || $success->length === 0) { - $failure = $xPath->evaluate("/cas:serviceResponse/cas:authenticationFailure"); - throw new Exception("Error when validating CAS service ticket: " . $failure->item(0)->textContent); - } else { + $serviceResponse = ServiceResponse::fromXML($dom->documentElement); + $message = $serviceResponse->getResponse(); + if ($message instanceof AuthenticationFailure) { + throw new Exception(sprintf( + "Error when validating CAS service ticket: %s (%s)", + $message->getContent(), + $message->getCode(), + )); + } elseif ($message instanceof AuthenticationSuccess) { + $user = $message->getUser()->getContent(); + $xPath = XPath::getXPath(); + $attributes = []; if ($casattributes = $this->casConfig['attributes']) { - // Some has attributes in the xml - attributes is a list of XPath expressions to get them + // Some have attributes in the xml - attributes is a list of XPath expressions to get them foreach ($casattributes as $name => $query) { - /** @var \DOMNodeList<\DOMNode> $attrs */ - $attrs = $xPath->query($query); + $attrs = $xPath->xpQuery($query, $xPath); foreach ($attrs as $attrvalue) { $attributes[$name][] = $attrvalue->textContent; } } } - $item = $success->item(0); - if (is_null($item)) { - throw new Exception("Error parsing serviceResponse."); - } - $casusername = $item->textContent; - - return [$casusername, $attributes]; + return [$user, $attributes]; } + + throw new Exception("Error parsing serviceResponse."); } @@ -212,8 +216,8 @@ public function finalStep(array &$state): void $ticket = $state['cas:ticket']; $stateId = Auth\State::saveState($state, self::STAGE_INIT); $service = Module::getModuleURL('cas/linkback.php', ['stateId' => $stateId]); - list($username, $casattributes) = $this->casValidation($ticket, $service); - $ldapattributes = []; + list($username, $casAttributes) = $this->casValidation($ticket, $service); + $ldapAttributes = []; $config = Configuration::loadFromArray( $this->ldapConfig, @@ -228,12 +232,13 @@ public function finalStep(array &$state): void $config->getOptionalInteger('port', 389), $config->getOptionalBoolean('referrals', true), ); - $ldapattributes = $ldap->validate($this->ldapConfig, $username); - if ($ldapattributes === false) { + + $ldapAttributes = $ldap->validate($this->ldapConfig, $username); + if ($ldapAttributes === false) { throw new Exception("Failed to authenticate against LDAP-server."); } } - $attributes = array_merge_recursive($casattributes, $ldapattributes); + $attributes = array_merge_recursive($casAttributes, $ldapAttributes); $state['Attributes'] = $attributes; } From 5885adb388e5dd18c5be455aa204a1195406dab8 Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Mon, 14 Apr 2025 22:33:20 +0200 Subject: [PATCH 02/27] Use SSP 2.3 --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 2de892c..11c38b1 100644 --- a/composer.json +++ b/composer.json @@ -37,7 +37,7 @@ "php": "^8.1", "ext-pcre": "*", - "simplesamlphp/assert": "^0.8 || ^1.0", + "simplesamlphp/assert": "^1.1", "simplesamlphp/cas": "^1.0", "simplesamlphp/composer-module-installer": "^1.3.4", "simplesamlphp/simplesamlphp": "~2.4.0", From 80bde34162161410eb9f5a31a22b848d7af8cdae Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Mon, 14 Apr 2025 23:04:53 +0200 Subject: [PATCH 03/27] Allow xmlprovider-plugin --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 11c38b1..a4799dd 100644 --- a/composer.json +++ b/composer.json @@ -17,9 +17,9 @@ }, "allow-plugins": { "composer/package-versions-deprecated": true, - "simplesamlphp/composer-module-installer": true, "dealerdirect/phpcodesniffer-composer-installer": true, "phpstan/extension-installer": true, + "simplesamlphp/composer-module-installer": true, "simplesamlphp/composer-xmlprovider-installer": true } }, From c2fc59371ff136b37b857283c5b42bd251ee46b8 Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Mon, 14 Apr 2025 23:19:24 +0200 Subject: [PATCH 04/27] Fix unit-test --- .github/workflows/php.yml | 4 ++-- composer.json | 3 ++- src/Controller/CAS.php | 2 +- tools/composer-require-checker.json | 1 + 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index 4eed08c..3f1ee11 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -53,7 +53,7 @@ jobs: uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php-versions }} - extensions: ctype, date, dom, fileinfo, filter, hash, intl, mbstring, openssl, pcre, spl, xml + extensions: ctype, date, dom, fileinfo, filter, hash, intl, ldap, mbstring, openssl, pcre, spl, xml tools: composer ini-values: error_reporting=E_ALL coverage: pcov @@ -115,7 +115,7 @@ jobs: uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php-versions }} - extensions: ctype, date, dom, fileinfo, filter, hash, intl, mbstring, openssl, pcre, spl, xml + extensions: ctype, date, dom, fileinfo, filter, hash, intl, ldap, mbstring, openssl, pcre, spl, xml tools: composer ini-values: error_reporting=E_ALL coverage: none diff --git a/composer.json b/composer.json index a4799dd..156a418 100644 --- a/composer.json +++ b/composer.json @@ -38,10 +38,11 @@ "ext-pcre": "*", "simplesamlphp/assert": "^1.1", - "simplesamlphp/cas": "^1.0", "simplesamlphp/composer-module-installer": "^1.3.4", "simplesamlphp/simplesamlphp": "~2.4.0", "simplesamlphp/simplesamlphp-module-ldap": "~1.2", + "simplesamlphp/xml-cas": "^1.0", + "simplesamlphp/xml-common": "^1.8", "symfony/http-foundation": "^6.4" }, "require-dev": { diff --git a/src/Controller/CAS.php b/src/Controller/CAS.php index d3bac27..10ce8e0 100644 --- a/src/Controller/CAS.php +++ b/src/Controller/CAS.php @@ -80,7 +80,7 @@ public function setAuthSource(Auth\Source $authSource): void */ public function linkback(Request $request): RunnableResponse { - if (!$request->query->has('stateId')) { + if (!$request->query->has('StateId')) { throw new Error\BadRequest('Missing StateId parameter.'); } diff --git a/tools/composer-require-checker.json b/tools/composer-require-checker.json index eed71aa..e0b6af2 100644 --- a/tools/composer-require-checker.json +++ b/tools/composer-require-checker.json @@ -1,4 +1,5 @@ { "symbol-whitelist": [ + "SimpleSAML\\Module\\ldap\\Auth\\Ldap" ] } From 88f0586c4eb7450d4c6b69e5842853b319628e2a Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Mon, 14 Apr 2025 23:28:00 +0200 Subject: [PATCH 05/27] Fix sniffer-issues --- src/Auth/Source/CAS.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Auth/Source/CAS.php b/src/Auth/Source/CAS.php index 6adc525..509acae 100644 --- a/src/Auth/Source/CAS.php +++ b/src/Auth/Source/CAS.php @@ -4,13 +4,12 @@ namespace SimpleSAML\Module\cas\Auth\Source; -use DOMXpath; use Exception; use SimpleSAML\Auth; +use SimpleSAML\CAS\Utils\XPath; use SimpleSAML\CAS\XML\cas\AuthenticationFailure; use SimpleSAML\CAS\XML\cas\AuthenticationSuccess; use SimpleSAML\CAS\XML\cas\ServiceResponse; -use SimpleSAML\CAS\Utils\XPath; use SimpleSAML\Configuration; use SimpleSAML\Module; use SimpleSAML\Module\ldap\Auth\Ldap; @@ -19,7 +18,6 @@ use function array_key_exists; use function array_merge_recursive; -use function is_null; use function preg_split; use function strcmp; use function var_export; From d20d255d93581cb7b45c6bf92da7a4104253f7c5 Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Mon, 7 Jul 2025 23:54:46 +0200 Subject: [PATCH 06/27] Fix phpstan-issues --- phpstan-dev.neon | 2 +- phpstan.neon | 2 +- src/Auth/Source/CAS.php | 9 +++++---- src/Controller/CAS.php | 1 + 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/phpstan-dev.neon b/phpstan-dev.neon index 116dfc6..4d29b8b 100644 --- a/phpstan-dev.neon +++ b/phpstan-dev.neon @@ -1,4 +1,4 @@ parameters: - level: 8 + level: 9 paths: - tests diff --git a/phpstan.neon b/phpstan.neon index d5341b1..db37782 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,4 +1,4 @@ parameters: - level: 7 + level: 6 paths: - src diff --git a/src/Auth/Source/CAS.php b/src/Auth/Source/CAS.php index 509acae..b9bac3a 100644 --- a/src/Auth/Source/CAS.php +++ b/src/Auth/Source/CAS.php @@ -20,6 +20,7 @@ use function array_merge_recursive; use function preg_split; use function strcmp; +use function strval; use function var_export; /** @@ -159,18 +160,18 @@ private function casServiceValidate(string $ticket, string $service): array if ($message instanceof AuthenticationFailure) { throw new Exception(sprintf( "Error when validating CAS service ticket: %s (%s)", - $message->getContent(), - $message->getCode(), + strval($message->getContent()), + strval($message->getCode()), )); } elseif ($message instanceof AuthenticationSuccess) { $user = $message->getUser()->getContent(); - $xPath = XPath::getXPath(); + $xPath = XPath::getXPath($message->toXML()); $attributes = []; if ($casattributes = $this->casConfig['attributes']) { // Some have attributes in the xml - attributes is a list of XPath expressions to get them foreach ($casattributes as $name => $query) { - $attrs = $xPath->xpQuery($query, $xPath); + $attrs = XPath::xpQuery($message->toXML(), $query, $xPath); foreach ($attrs as $attrvalue) { $attributes[$name][] = $attrvalue->textContent; } diff --git a/src/Controller/CAS.php b/src/Controller/CAS.php index 10ce8e0..0b69c2f 100644 --- a/src/Controller/CAS.php +++ b/src/Controller/CAS.php @@ -85,6 +85,7 @@ public function linkback(Request $request): RunnableResponse } $stateId = $request->query->getString('stateId'); + /** @var array $state */ $state = $this->authState::loadState($stateId, CASSource::STAGE_INIT); if (!$request->query->has('ticket')) { From 6c9b58dd0cd0e492d17464156f1f4d590affae62 Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Wed, 29 Oct 2025 17:28:56 +0100 Subject: [PATCH 07/27] Fix case --- .github/workflows/php.yml | 2 +- src/Controller/CAS.php | 4 ++-- tests/src/Controller/CASTest.php | 12 ++++++------ 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index 3f1ee11..5940f4b 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -115,7 +115,7 @@ jobs: uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php-versions }} - extensions: ctype, date, dom, fileinfo, filter, hash, intl, ldap, mbstring, openssl, pcre, spl, xml + extensions: ctype, date, dom, fileinfo, filter, hash, intl, ldap, mbstring, openssl, pcre, spl, xml, zip tools: composer ini-values: error_reporting=E_ALL coverage: none diff --git a/src/Controller/CAS.php b/src/Controller/CAS.php index 0b69c2f..e52af83 100644 --- a/src/Controller/CAS.php +++ b/src/Controller/CAS.php @@ -80,8 +80,8 @@ public function setAuthSource(Auth\Source $authSource): void */ public function linkback(Request $request): RunnableResponse { - if (!$request->query->has('StateId')) { - throw new Error\BadRequest('Missing StateId parameter.'); + if (!$request->query->has('stateId')) { + throw new Error\BadRequest('Missing stateId parameter.'); } $stateId = $request->query->getString('stateId'); diff --git a/tests/src/Controller/CASTest.php b/tests/src/Controller/CASTest.php index 65b4430..ce8646d 100644 --- a/tests/src/Controller/CASTest.php +++ b/tests/src/Controller/CASTest.php @@ -62,7 +62,7 @@ protected function setUp(): void /** - * Test that request without StateId results in a BadRequest-error + * Test that request without stateId results in a BadRequest-error */ public function testNoStateId(): void { @@ -74,7 +74,7 @@ public function testNoStateId(): void $c = new Controller\CAS($this->config); $this->expectException(Error\BadRequest::class); - $this->expectExceptionMessage("BADREQUEST('%REASON%' => 'Missing StateId parameter.')"); + $this->expectExceptionMessage("BADREQUEST('%REASON%' => 'Missing stateId parameter.')"); $c->linkback($request); } @@ -88,7 +88,7 @@ public function testNoState(): void $request = Request::create( '/linkback', 'GET', - ['StateId' => 'abc123'], + ['stateId' => 'abc123'], ); $c = new Controller\CAS($this->config); @@ -107,7 +107,7 @@ public function testNoTicket(): void $request = Request::create( '/linkback', 'GET', - ['StateId' => 'abc123'], + ['stateId' => 'abc123'], ); $c = new Controller\CAS($this->config); @@ -135,7 +135,7 @@ public function testUnknownAuthSource(): void '/linkback', 'GET', [ - 'StateId' => 'abc123', + 'stateId' => 'abc123', 'ticket' => 'abc123', ], ); @@ -164,7 +164,7 @@ public function testNormalOperation(): void '/linkback', 'GET', [ - 'StateId' => 'abc123', + 'stateId' => 'abc123', 'ticket' => 'abc123', ], ); From 1572c07d95bfa92ac1c450d2a61712615e2ea11f Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Wed, 29 Oct 2025 17:56:22 +0100 Subject: [PATCH 08/27] Use local phpcs, bump minimum PHP-version and start testing on PHP 8.5 --- .github/workflows/php.yml | 22 +++++++++++----------- composer.json | 2 +- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index 5940f4b..5d8f11f 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -19,9 +19,9 @@ jobs: strategy: fail-fast: false matrix: - php-version: ['8.1', '8.2', '8.3', '8.4'] + php-version: ['8.2', '8.3', '8.4', '8.5'] - uses: simplesamlphp/simplesamlphp-test-framework/.github/workflows/reusable_phplinter.yml@v1.9.2 + uses: simplesamlphp/simplesamlphp-test-framework/.github/workflows/reusable_phplinter.yml@v1.10.3 with: php-version: ${{ matrix.php-version }} @@ -30,7 +30,7 @@ jobs: strategy: fail-fast: false - uses: simplesamlphp/simplesamlphp-test-framework/.github/workflows/reusable_linter.yml@v1.9.2 + uses: simplesamlphp/simplesamlphp-test-framework/.github/workflows/reusable_linter.yml@v1.10.3 with: enable_eslinter: false enable_jsonlinter: true @@ -45,7 +45,7 @@ jobs: fail-fast: false matrix: operating-system: [ubuntu-latest] - php-versions: ['8.1', '8.2', '8.3', '8.4'] + php-versions: ['8.2', '8.3', '8.4', '8.5'] steps: - name: Setup PHP, with composer and extensions @@ -85,15 +85,15 @@ jobs: run: composer install --no-progress --prefer-dist --optimize-autoloader - name: Run unit tests with coverage - if: ${{ matrix.php-versions == '8.4' }} + if: ${{ matrix.php-versions == '8.5' }} run: vendor/bin/phpunit - name: Run unit tests (no coverage) - if: ${{ matrix.php-versions != '8.4' }} + if: ${{ matrix.php-versions != '8.5' }} run: vendor/bin/phpunit --no-coverage - name: Save coverage data - if: ${{ matrix.php-versions == '8.4' }} + if: ${{ matrix.php-versions == '8.5' }} uses: actions/upload-artifact@v4 with: name: coverage-data @@ -107,7 +107,7 @@ jobs: fail-fast: true matrix: operating-system: [windows-latest] - php-versions: ['8.1', '8.2', '8.3', '8.4'] + php-versions: ['8.2', '8.3', '8.4', '8.5'] steps: - name: Setup PHP, with composer and extensions @@ -161,7 +161,7 @@ jobs: uses: shivammathur/setup-php@v2 with: # Should be the higest supported version, so we can use the newest tools - php-version: '8.4' + php-version: '8.5' tools: composer, composer-require-checker, composer-unused, phpcs extensions: ctype, date, dom, fileinfo, filter, hash, intl, mbstring, openssl, pcre, spl, xml @@ -193,7 +193,7 @@ jobs: run: composer-unused - name: PHP Code Sniffer - run: phpcs + run: vendor/bin/phpcs - name: PHPStan run: | @@ -214,7 +214,7 @@ jobs: uses: shivammathur/setup-php@v2 with: # Should be the lowest supported version - php-version: '8.1' + php-version: '8.2' extensions: ctype, date, dom, fileinfo, filter, hash, intl, mbstring, openssl, pcre, spl, xml tools: composer coverage: none diff --git a/composer.json b/composer.json index 156a418..dfd33f6 100644 --- a/composer.json +++ b/composer.json @@ -34,7 +34,7 @@ } }, "require": { - "php": "^8.1", + "php": "^8.2", "ext-pcre": "*", "simplesamlphp/assert": "^1.1", From f73a49238582385148368d818fd0f5bf56f33f88 Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Wed, 29 Oct 2025 18:04:15 +0100 Subject: [PATCH 09/27] Adopt SSP coding-style --- src/Auth/Source/CAS.php | 2 ++ tests/src/Controller/CASTest.php | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Auth/Source/CAS.php b/src/Auth/Source/CAS.php index b9bac3a..ab179ac 100644 --- a/src/Auth/Source/CAS.php +++ b/src/Auth/Source/CAS.php @@ -43,6 +43,7 @@ class CAS extends Auth\Source */ public const AUTHID = '\SimpleSAML\Module\cas\Auth\Source\CAS.AuthId'; + /** * @var array with ldap configuration */ @@ -64,6 +65,7 @@ class CAS extends Auth\Source */ private string $loginMethod; + /** * Constructor for this authentication source. * diff --git a/tests/src/Controller/CASTest.php b/tests/src/Controller/CASTest.php index ce8646d..d4d0bf9 100644 --- a/tests/src/Controller/CASTest.php +++ b/tests/src/Controller/CASTest.php @@ -12,7 +12,7 @@ use SimpleSAML\HTTP\RunnableResponse; use SimpleSAML\Module\cas\Auth\Source\CAS; use SimpleSAML\Module\cas\Controller; -use Symfony\Component\HttpFoundation\{Request, Response}; +use Symfony\Component\HttpFoundation\Request; /** * Set of tests for the controllers in the "cas" module. @@ -183,6 +183,7 @@ public function __construct() //dummy } + /** * @param array $state */ @@ -191,6 +192,7 @@ public function authenticate(array &$state): void //dummy } + public static function getById(string $authId, ?string $type = null): ?Auth\Source { return new class () extends CAS { @@ -199,6 +201,7 @@ public function __construct() //dummy } + /** @param array $state */ public function finalStep(array &$state): void { From f6d45431622f8a7d8077d175e2de3f444e0fa281 Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Wed, 29 Oct 2025 18:19:20 +0100 Subject: [PATCH 10/27] Fix phpstan --- src/Auth/Source/CAS.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Auth/Source/CAS.php b/src/Auth/Source/CAS.php index ab179ac..4b7c04c 100644 --- a/src/Auth/Source/CAS.php +++ b/src/Auth/Source/CAS.php @@ -123,7 +123,7 @@ private function casValidate(string $ticket, string $service): array /** @var string $result */ $result = $httpUtils->fetch($url); - /** @var string $res */ + /** @var list}|string> $res */ $res = preg_split("/\r?\n/", $result); if (strcmp($res[0], "yes") == 0) { From d8f06fb76cefdde93cd173d783bdf7eb451cf84c Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Wed, 29 Oct 2025 18:19:20 +0100 Subject: [PATCH 11/27] Fix phpstan --- tests/src/Controller/CASTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/src/Controller/CASTest.php b/tests/src/Controller/CASTest.php index d4d0bf9..3745464 100644 --- a/tests/src/Controller/CASTest.php +++ b/tests/src/Controller/CASTest.php @@ -113,7 +113,7 @@ public function testNoTicket(): void $c = new Controller\CAS($this->config); $c->setAuthState(new class () extends Auth\State { /** @return array|null */ - public static function loadState(string $id, string $stage, bool $allowMissing = false): ?array + public static function loadState(string $id, string $stage, bool $allowMissing = false): array { return []; } @@ -143,7 +143,7 @@ public function testUnknownAuthSource(): void $c = new Controller\CAS($this->config); $c->setAuthState(new class () extends Auth\State { /** @return array|null */ - public static function loadState(string $id, string $stage, bool $allowMissing = false): ?array + public static function loadState(string $id, string $stage, bool $allowMissing = false): array { return [CAS::AUTHID => 'somethingElse']; } @@ -172,7 +172,7 @@ public function testNormalOperation(): void $c = new Controller\CAS($this->config); $c->setAuthState(new class () extends Auth\State { /** @return array|null */ - public static function loadState(string $id, string $stage, bool $allowMissing = false): ?array + public static function loadState(string $id, string $stage, bool $allowMissing = false): array { return [CAS::AUTHID => 'something']; } @@ -193,7 +193,7 @@ public function authenticate(array &$state): void } - public static function getById(string $authId, ?string $type = null): ?Auth\Source + public static function getById(string $authId, ?string $type = null): Auth\Source { return new class () extends CAS { public function __construct() From 68ed87f8b76dad5c4d2af6e03a812c7b18c4a0ba Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Fri, 7 Nov 2025 11:26:42 +0200 Subject: [PATCH 12/27] composer.json add dev dependencies and pre-commit script.fix phpstan repoprted issues. --- composer.json | 23 ++++++++++++++++++++++- tests/src/Controller/CASTest.php | 11 ++++++++--- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/composer.json b/composer.json index dfd33f6..09e0267 100644 --- a/composer.json +++ b/composer.json @@ -46,10 +46,31 @@ "symfony/http-foundation": "^6.4" }, "require-dev": { - "simplesamlphp/simplesamlphp-test-framework": "^1.9.2" + "simplesamlphp/simplesamlphp-test-framework": "^1.9.2", + "phpunit/phpunit": "^10", + "icanhazstring/composer-unused": "^0.9.5", + "squizlabs/php_codesniffer": "^4.0.0", + "phpstan/phpstan": "^1.11", + "maglnet/composer-require-checker": "^4" }, "support": { "issues": "https://github.com/simplesamlphp/simplesamlphp-module-cas/issues", "source": "https://github.com/simplesamlphp/simplesamlphp-module-cas" + }, + "scripts": { + "pre-commit": [ + "vendor/bin/phpcs -p", + "vendor/bin/composer-require-checker check --config-file=tools/composer-require-checker.json composer.json", + "vendor/bin/phpstan analyze -c phpstan.neon", + "vendor/bin/phpstan analyze -c phpstan-dev.neon", + "vendor/bin/composer-unused", + "vendor/bin/phpunit --no-coverage --testdox" + ], + "tests": [ + "vendor/bin/phpunit --no-coverage" + ], + "propose-fix": [ + "vendor/bin/phpcs --report=diff" + ] } } diff --git a/tests/src/Controller/CASTest.php b/tests/src/Controller/CASTest.php index 3745464..b7c1114 100644 --- a/tests/src/Controller/CASTest.php +++ b/tests/src/Controller/CASTest.php @@ -35,6 +35,11 @@ protected function setUp(): void { parent::setUp(); + // Minimal server globals needed by SimpleSAML internals + $_SERVER['REQUEST_URI'] = '/linkback'; + $_SERVER['HTTP_HOST'] = 'localhost'; + $_SERVER['HTTPS'] = 'off'; + $this->config = Configuration::loadFromArray( [ 'module.enable' => [ @@ -112,7 +117,7 @@ public function testNoTicket(): void $c = new Controller\CAS($this->config); $c->setAuthState(new class () extends Auth\State { - /** @return array|null */ + /** @return array */ public static function loadState(string $id, string $stage, bool $allowMissing = false): array { return []; @@ -142,7 +147,7 @@ public function testUnknownAuthSource(): void $c = new Controller\CAS($this->config); $c->setAuthState(new class () extends Auth\State { - /** @return array|null */ + /** @return array */ public static function loadState(string $id, string $stage, bool $allowMissing = false): array { return [CAS::AUTHID => 'somethingElse']; @@ -171,7 +176,7 @@ public function testNormalOperation(): void $c = new Controller\CAS($this->config); $c->setAuthState(new class () extends Auth\State { - /** @return array|null */ + /** @return array */ public static function loadState(string $id, string $stage, bool $allowMissing = false): array { return [CAS::AUTHID => 'something']; From bce21f7951b3e863b4fc8e76575cf440f0c1dd3a Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Fri, 7 Nov 2025 13:56:59 +0200 Subject: [PATCH 13/27] Fix XML element parsing issue --- src/Auth/Source/CAS.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Auth/Source/CAS.php b/src/Auth/Source/CAS.php index 4b7c04c..37c2cc7 100644 --- a/src/Auth/Source/CAS.php +++ b/src/Auth/Source/CAS.php @@ -167,13 +167,16 @@ private function casServiceValidate(string $ticket, string $service): array )); } elseif ($message instanceof AuthenticationSuccess) { $user = $message->getUser()->getContent(); - $xPath = XPath::getXPath($message->toXML()); + + // Build XPath from the same document/node we will query and reuse it. + $element = $message->toXML(); + $xPath = XPath::getXPath($element); $attributes = []; if ($casattributes = $this->casConfig['attributes']) { // Some have attributes in the xml - attributes is a list of XPath expressions to get them foreach ($casattributes as $name => $query) { - $attrs = XPath::xpQuery($message->toXML(), $query, $xPath); + $attrs = XPath::xpQuery($element, $query, $xPath); foreach ($attrs as $attrvalue) { $attributes[$name][] = $attrvalue->textContent; } From f4a058b0bedfd18d58d0d7c67eb4e12b9d618f14 Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Fri, 7 Nov 2025 14:17:57 +0200 Subject: [PATCH 14/27] Add useful debug messages --- src/Auth/Source/CAS.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/Auth/Source/CAS.php b/src/Auth/Source/CAS.php index 37c2cc7..2bcd4f1 100644 --- a/src/Auth/Source/CAS.php +++ b/src/Auth/Source/CAS.php @@ -170,6 +170,9 @@ private function casServiceValidate(string $ticket, string $service): array // Build XPath from the same document/node we will query and reuse it. $element = $message->toXML(); + // Dump the XML we received from the CAS server + Logger::debug('CAS client: serviceValidate XML: ' . $element->ownerDocument?->saveXML()); + $xPath = XPath::getXPath($element); $attributes = []; @@ -180,6 +183,10 @@ private function casServiceValidate(string $ticket, string $service): array foreach ($attrs as $attrvalue) { $attributes[$name][] = $attrvalue->textContent; } + // Log what we parsed for this attribute name + Logger::debug( + sprintf('CAS client: parsed attribute %s => %s', $name, json_encode($attributes[$name] ?? [])), + ); } } From 1d4111b2127cb50f1ca46552b46173421dc0811a Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Sat, 8 Nov 2025 11:16:48 +0200 Subject: [PATCH 15/27] Fix attributes configuration behavioral change. Add tests. Fix cas.md documentation. --- docs/cas.md | 10 +-- phpunit.xml | 6 +- src/Auth/Source/CAS.php | 85 ++++++++++++------ tests/bootstrap.php | 30 ++++++- tests/config/authsources.php | 51 +++++++++++ .../response/cas-success-service-response.xml | 17 ++++ tests/src/Controller/CASTest.php | 88 ++++++++++++++++--- 7 files changed, 245 insertions(+), 42 deletions(-) create mode 100644 tests/config/authsources.php create mode 100644 tests/response/cas-success-service-response.xml diff --git a/docs/cas.md b/docs/cas.md index 266a188..fa3293d 100644 --- a/docs/cas.md +++ b/docs/cas.md @@ -76,10 +76,10 @@ for each value: ```php 'cas' => [ 'attributes' => [ - 'uid' => '/cas:serviceResponse/cas:authenticationSuccess/cas:user', - 'sn' => '/cas:serviceResponse/cas:authenticationSuccess/cas:attributes/cas:sn', - 'givenName' => '/cas:serviceResponse/cas:authenticationSuccess/cas:attributes/cas:firstname', - 'mail' => '/cas:serviceResponse/cas:authenticationSuccess/cas:attributes/cas:mail', + 'uid' => 'cas:user', + 'sn' => 'cas:attributes/cas:sn', + 'givenName' => 'cas:attributes/cas:firstname', + 'mail' => 'cas:attributes/cas:mail', ], ], ``` @@ -87,7 +87,7 @@ for each value: and even some custom attributes if they're set: ```php -'customabc' => '/cas:serviceResponse/cas:authenticationSuccess/custom:abc', +'customabc' => 'custom:abc', ``` You'll probably want to avoid querying LDAP for attributes: diff --git a/phpunit.xml b/phpunit.xml index a285b98..d9e908b 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -1,5 +1,9 @@ - + diff --git a/src/Auth/Source/CAS.php b/src/Auth/Source/CAS.php index 2bcd4f1..13064d8 100644 --- a/src/Auth/Source/CAS.php +++ b/src/Auth/Source/CAS.php @@ -11,6 +11,7 @@ use SimpleSAML\CAS\XML\cas\AuthenticationSuccess; use SimpleSAML\CAS\XML\cas\ServiceResponse; use SimpleSAML\Configuration; +use SimpleSAML\Logger; use SimpleSAML\Module; use SimpleSAML\Module\ldap\Auth\Ldap; use SimpleSAML\Utils; @@ -166,31 +167,7 @@ private function casServiceValidate(string $ticket, string $service): array strval($message->getCode()), )); } elseif ($message instanceof AuthenticationSuccess) { - $user = $message->getUser()->getContent(); - - // Build XPath from the same document/node we will query and reuse it. - $element = $message->toXML(); - // Dump the XML we received from the CAS server - Logger::debug('CAS client: serviceValidate XML: ' . $element->ownerDocument?->saveXML()); - - $xPath = XPath::getXPath($element); - - $attributes = []; - if ($casattributes = $this->casConfig['attributes']) { - // Some have attributes in the xml - attributes is a list of XPath expressions to get them - foreach ($casattributes as $name => $query) { - $attrs = XPath::xpQuery($element, $query, $xPath); - foreach ($attrs as $attrvalue) { - $attributes[$name][] = $attrvalue->textContent; - } - // Log what we parsed for this attribute name - Logger::debug( - sprintf('CAS client: parsed attribute %s => %s', $name, json_encode($attributes[$name] ?? [])), - ); - } - } - - return [$user, $attributes]; + return $this->parseUserAndAttributes($message); } throw new Exception("Error parsing serviceResponse."); @@ -296,4 +273,62 @@ public function logout(array &$state): void $httpUtils = new Utils\HTTP(); $httpUtils->redirectTrustedURL($logoutUrl); } + + /** + * Extract the CAS user and attributes from an AuthenticationSuccess message. + * + * @param \SimpleSAML\CAS\XML\cas\AuthenticationSuccess $message + * @return array{0: string, 1: array} + */ + private function parseUserAndAttributes(AuthenticationSuccess $message): array + { + $user = $message->getUser()->getContent(); + + // Build XPath from the same document/node we will query and reuse it. + $element = $message->toXML(); + // Dump the XML we received from the CAS server + Logger::debug('CAS client: serviceValidate XML: ' . $element->ownerDocument?->saveXML()); + + $xPath = XPath::getXPath($element); + + $attributes = []; + if ($casattributes = $this->casConfig['attributes']) { + // Some have attributes in the xml - attributes is a list of XPath expressions to get them + foreach ($casattributes as $name => $query) { + // If query is an absolute CAS path starting at /cas:serviceResponse/.../cas:authenticationSuccess/, + // rewrite it to be relative to the authenticationSuccess element. + // Example: + // /cas:serviceResponse/cas:authenticationSuccess/cas:attributes/cas:firstname + // /cas:serviceResponse/cas:authenticationSuccess/cas:user + // becomes: + // cas:attributes/cas:firstname + // cas:user + $marker = 'cas:authenticationSuccess/'; + if (isset($query[0]) && $query[0] === '/') { + $pos = strpos($query, $marker); + if ($pos !== false) { + $originalQuery = $query; + $query = substr($query, $pos + strlen($marker)); + Logger::info(sprintf( + 'CAS client: rewriting absolute CAS XPath for "%s" from "%s" to relative "%s"', + $name, + $originalQuery, + $query, + )); + } + } + + $attrs = XPath::xpQuery($element, $query, $xPath); + foreach ($attrs as $attrvalue) { + $attributes[$name][] = $attrvalue->textContent; + } + // log what we parsed for this attribute name + Logger::debug( + sprintf('CAS client: parsed attribute %s => %s', $name, json_encode($attributes[$name] ?? [])), + ); + } + } + + return [$user, $attributes]; + } } diff --git a/tests/bootstrap.php b/tests/bootstrap.php index fd78890..81b7fd0 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -1,8 +1,10 @@ [ + 'core:AdminPassword', + ], + 'something' => [ + 'cas:CAS', + 'cas' => [ + 'login' => 'https://example.org/login', + 'validate' => 'https://example.org/validate', + ], + 'ldap' => [], + ], + 'casserver' => [ + 'cas:CAS', + 'cas' => [ + 'login' => 'https://ugrad.apply.example.edu/account/cas/login', + 'serviceValidate' => 'https://ugrad.apply.example.edu/account/cas/serviceValidate', + 'logout' => 'https://ugrad.apply.example.edu/account/cas/logout', + 'attributes' => [ + 'uid' => 'cas:user', + 'sn' => 'cas:attributes/cas:sn', + 'givenName' => 'cas:attributes/cas:firstname', + 'mail' => 'cas:attributes/cas:mail', + 'eduPersonPrincipalName' => 'cas:attributes/cas:eduPersonPrincipalName', + ], + ], + 'ldap' => [], + ], + 'casserver_legacy' => [ + 'cas:CAS', + 'cas' => [ + 'login' => 'https://ugrad.apply.example.edu/account/cas/login', + 'serviceValidate' => 'https://ugrad.apply.example.edu/account/cas/serviceValidate', + 'logout' => 'https://ugrad.apply.example.edu/account/cas/logout', + 'attributes' => [ + 'uid' => '/cas:serviceResponse/cas:authenticationSuccess/cas:user', + 'sn' => '/cas:serviceResponse/cas:authenticationSuccess/cas:attributes/cas:sn', + 'givenName' => '/cas:serviceResponse/cas:authenticationSuccess/cas:attributes/cas:firstname', + 'mail' => '/cas:serviceResponse/cas:authenticationSuccess/cas:attributes/cas:mail', + 'eduPersonPrincipalName' => '/cas:serviceResponse/cas:authenticationSuccess/cas:attributes/cas:eduPersonPrincipalName', + ], + ], + 'ldap' => [], + ], +]; +// phpcs:enable \ No newline at end of file diff --git a/tests/response/cas-success-service-response.xml b/tests/response/cas-success-service-response.xml new file mode 100644 index 0000000..4878f26 --- /dev/null +++ b/tests/response/cas-success-service-response.xml @@ -0,0 +1,17 @@ + + + jdoe + + 2025-11-07T22:00:24+02:00 + true + true + Doe + John + jdoe@example.edu + jdoe@example.edu + + 12345 + Fall-2025 + ABC-123 + + \ No newline at end of file diff --git a/tests/src/Controller/CASTest.php b/tests/src/Controller/CASTest.php index b7c1114..35a25cb 100644 --- a/tests/src/Controller/CASTest.php +++ b/tests/src/Controller/CASTest.php @@ -5,13 +5,17 @@ namespace SimpleSAML\Test\Module\cas\Controller; use Exception; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use SimpleSAML\Auth; +use SimpleSAML\CAS\XML\cas\AuthenticationSuccess; +use SimpleSAML\CAS\XML\cas\ServiceResponse; use SimpleSAML\Configuration; use SimpleSAML\Error; use SimpleSAML\HTTP\RunnableResponse; use SimpleSAML\Module\cas\Auth\Source\CAS; use SimpleSAML\Module\cas\Controller; +use SimpleSAML\XML\DOMDocumentFactory; use Symfony\Component\HttpFoundation\Request; /** @@ -52,16 +56,7 @@ protected function setUp(): void ); Configuration::setPreLoadedConfig($this->config, 'config.php'); - $this->sourceConfig = Configuration::loadFromArray([ - 'something' => [ - 'cas:CAS', - 'cas' => [ - 'login' => 'https://example.org/login', - 'validate' => 'https://example.org/validate', - ], - 'ldap' => [], - ], - ]); + $this->sourceConfig = Configuration::getConfig('authsources.php'); Configuration::setPreLoadedConfig($this->sourceConfig, 'authsources.php'); } @@ -219,4 +214,77 @@ public function finalStep(array &$state): void $result = $c->linkback($request); $this->assertInstanceOf(RunnableResponse::class, $result); } + + /** + * Provide both CAS configs: relative (casserver) and absolute (casserver_legacy). + * + * @return array + */ + public static function casConfigsProvider(): array + { + return [ + ['casserver'], + ['casserver_legacy'], + ]; + } + + /** + * Run the same extraction assertions for both configurations. + * + * @param string $sourceKey The key of the CAS configuration to test ('casserver' or 'casserver_legacy') + * @throws \ReflectionException + */ + #[DataProvider('casConfigsProvider')] + public function testCasConfigAbsoluteXPathsReturnValues(string $sourceKey): void + { + $authsources = Configuration::getConfig('authsources.php'); + $config = $authsources->toArray(); + + self::assertIsArray($config, 'authsources.php did not return expected $config array'); + self::assertArrayHasKey($sourceKey, $config, "Missing source '$sourceKey' in authsources.php"); + $sourceConfig = $config[$sourceKey]; + self::assertArrayHasKey('cas', $sourceConfig, "Missing 'cas' config for '$sourceKey'"); + self::assertArrayHasKey('ldap', $sourceConfig, "Missing 'ldap' config for '$sourceKey'"); + + // Load the CAS success message XML and build an AuthenticationSuccess message + $successXmlFile = dirname(__DIR__, 1) . '/../response/cas-success-service-response.xml'; + self::assertFileExists($successXmlFile, 'CAS success XML not found at expected path'); + + $dom = DOMDocumentFactory::fromFile($successXmlFile); + // Ensure documentElement is a DOMElement before passing to fromXML() + $root = $dom->documentElement; + if (!$root instanceof \DOMElement) { + self::fail('Loaded XML does not have a document element'); + } + $serviceResponse = ServiceResponse::fromXML($root); + $message = $serviceResponse->getResponse(); + self::assertInstanceOf( + AuthenticationSuccess::class, + $message, + 'Expected AuthenticationSuccess message', + ); + + // Instantiate the CAS source with the selected configuration + $cas = new Cas(['AuthId' => 'unit-cas'], $sourceConfig); + + // Invoke the private parseUserAndAttributes() method via reflection + $refMethod = new \ReflectionMethod(Cas::class, 'parseUserAndAttributes'); + $refMethod->setAccessible(true); + /** @var array{0:string,1:array>} $result */ + $result = $refMethod->invoke($cas, $message); + + // Assert user and attributes are identical for both configurations + [$user, $attributes] = $result; + + self::assertSame('jdoe', $user, "$sourceKey: user mismatch"); + self::assertSame(['jdoe'], $attributes['uid'] ?? [], "$sourceKey: uid not extracted"); + self::assertSame(['Doe'], $attributes['sn'] ?? [], "$sourceKey: sn not extracted"); + self::assertSame(['John'], $attributes['givenName'] ?? [], "$sourceKey: givenName not extracted"); + self::assertSame(['jdoe@example.edu'], $attributes['mail'] ?? [], "$sourceKey: mail not extracted"); + self::assertSame( + ['jdoe@example.edu'], + $attributes['eduPersonPrincipalName'] ?? [], + "$sourceKey: ePPN not extracted", + ); + } } From 62a2de031c3d6e151e259423b98e3ef89c831b8b Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Mon, 10 Nov 2025 11:47:33 +0200 Subject: [PATCH 16/27] improve test doc.replace phpcs disable with php line max length handling. --- tests/config/authsources.php | 3 +-- tests/src/Controller/CASTest.php | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/config/authsources.php b/tests/config/authsources.php index 10ca235..3f0f726 100644 --- a/tests/config/authsources.php +++ b/tests/config/authsources.php @@ -2,7 +2,6 @@ declare(strict_types=1); -// phpcs:disable $config = [ 'admin' => [ 'core:AdminPassword', @@ -42,10 +41,10 @@ 'sn' => '/cas:serviceResponse/cas:authenticationSuccess/cas:attributes/cas:sn', 'givenName' => '/cas:serviceResponse/cas:authenticationSuccess/cas:attributes/cas:firstname', 'mail' => '/cas:serviceResponse/cas:authenticationSuccess/cas:attributes/cas:mail', + // phpcs:ignore Generic.Files.LineLength.TooLong 'eduPersonPrincipalName' => '/cas:serviceResponse/cas:authenticationSuccess/cas:attributes/cas:eduPersonPrincipalName', ], ], 'ldap' => [], ], ]; -// phpcs:enable \ No newline at end of file diff --git a/tests/src/Controller/CASTest.php b/tests/src/Controller/CASTest.php index 35a25cb..3ee7a8a 100644 --- a/tests/src/Controller/CASTest.php +++ b/tests/src/Controller/CASTest.php @@ -223,8 +223,8 @@ public function finalStep(array &$state): void public static function casConfigsProvider(): array { return [ - ['casserver'], - ['casserver_legacy'], + "casserver short attribute mapping" => ['casserver'], + "casserver legacy/long attribute mapping" => ['casserver_legacy'], ]; } From 55b298543484ef9f1b05c384758d03ff09119a07 Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Fri, 14 Nov 2025 18:43:19 +0100 Subject: [PATCH 17/27] Bump dependencies --- composer.json | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/composer.json b/composer.json index dfd33f6..d4defec 100644 --- a/composer.json +++ b/composer.json @@ -37,16 +37,16 @@ "php": "^8.2", "ext-pcre": "*", - "simplesamlphp/assert": "^1.1", - "simplesamlphp/composer-module-installer": "^1.3.4", - "simplesamlphp/simplesamlphp": "~2.4.0", + "simplesamlphp/assert": "^1.9", + "simplesamlphp/composer-module-installer": "^1.4", + "simplesamlphp/simplesamlphp": "~2.4", "simplesamlphp/simplesamlphp-module-ldap": "~1.2", - "simplesamlphp/xml-cas": "^1.0", - "simplesamlphp/xml-common": "^1.8", - "symfony/http-foundation": "^6.4" + "simplesamlphp/xml-cas": "^2.0", + "simplesamlphp/xml-common": "^2.0", + "symfony/http-foundation": "^6.4 || ^7.3" }, "require-dev": { - "simplesamlphp/simplesamlphp-test-framework": "^1.9.2" + "simplesamlphp/simplesamlphp-test-framework": "^1.10" }, "support": { "issues": "https://github.com/simplesamlphp/simplesamlphp-module-cas/issues", From 7daedad354ad9278adc9169767cffcd1fa26e59b Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Sun, 23 Nov 2025 08:51:43 +0200 Subject: [PATCH 18/27] Upgrade dependencies. --- composer.json | 9 +- src/Auth/Source/CAS.php | 186 ++++++++++++------ tests/config/authsources.php | 17 ++ .../response/cas-success-service-response.xml | 11 +- tests/src/Controller/CASTest.php | 117 ++++++++++- 5 files changed, 271 insertions(+), 69 deletions(-) diff --git a/composer.json b/composer.json index 09e0267..5131c09 100644 --- a/composer.json +++ b/composer.json @@ -36,14 +36,15 @@ "require": { "php": "^8.2", "ext-pcre": "*", + "ext-dom": "*", "simplesamlphp/assert": "^1.1", "simplesamlphp/composer-module-installer": "^1.3.4", - "simplesamlphp/simplesamlphp": "~2.4.0", + "simplesamlphp/simplesamlphp": "dev-simplesamlphp-2.5 as v2.5.x-dev", "simplesamlphp/simplesamlphp-module-ldap": "~1.2", - "simplesamlphp/xml-cas": "^1.0", - "simplesamlphp/xml-common": "^1.8", - "symfony/http-foundation": "^6.4" + "simplesamlphp/xml-cas": "~v2.1.0", + "simplesamlphp/xml-common": "~2.4.0", + "symfony/http-foundation": "~7.3.0" }, "require-dev": { "simplesamlphp/simplesamlphp-test-framework": "^1.9.2", diff --git a/src/Auth/Source/CAS.php b/src/Auth/Source/CAS.php index 13064d8..b891c75 100644 --- a/src/Auth/Source/CAS.php +++ b/src/Auth/Source/CAS.php @@ -4,12 +4,14 @@ namespace SimpleSAML\Module\cas\Auth\Source; +use DOMDocument; +use DOMElement; use Exception; use SimpleSAML\Auth; use SimpleSAML\CAS\Utils\XPath; -use SimpleSAML\CAS\XML\cas\AuthenticationFailure; -use SimpleSAML\CAS\XML\cas\AuthenticationSuccess; -use SimpleSAML\CAS\XML\cas\ServiceResponse; +use SimpleSAML\CAS\XML\AuthenticationFailure; +use SimpleSAML\CAS\XML\AuthenticationSuccess; +use SimpleSAML\CAS\XML\ServiceResponse; use SimpleSAML\Configuration; use SimpleSAML\Logger; use SimpleSAML\Module; @@ -17,7 +19,6 @@ use SimpleSAML\Utils; use SimpleSAML\XML\DOMDocumentFactory; -use function array_key_exists; use function array_merge_recursive; use function preg_split; use function strcmp; @@ -66,6 +67,13 @@ class CAS extends Auth\Source */ private string $loginMethod; + /** + * If enabled, convert all CAS attributes found in the XML: + * - cas:NAME => NAME + * - OTHERPREFIX:NAME => OTHERPREFIX:NAME + * - collect multi-valued elements into arrays of strings + */ +// private bool $convertAllAttributes; /** * Constructor for this authentication source. @@ -78,16 +86,10 @@ public function __construct(array $info, array $config) // Call the parent constructor first, as required by the interface parent::__construct($info, $config); - if (!array_key_exists('cas', $config)) { - throw new Exception('cas authentication source is not properly configured: missing [cas]'); - } - - if (!array_key_exists('ldap', $config)) { - throw new Exception('ldap authentication source is not properly configured: missing [ldap]'); - } + $authsources = Configuration::loadFromArray($config); - $this->casConfig = $config['cas']; - $this->ldapConfig = $config['ldap']; + $this->casConfig = $authsources->getValue('cas'); + $this->ldapConfig = $authsources->getValue('ldap'); if (isset($this->casConfig['serviceValidate'])) { $this->validationMethod = 'serviceValidate'; @@ -167,7 +169,16 @@ private function casServiceValidate(string $ticket, string $service): array strval($message->getCode()), )); } elseif ($message instanceof AuthenticationSuccess) { - return $this->parseUserAndAttributes($message); + [$user, $attributes] = $this->parseAuthenticationSuccess($message); + + // This will only be parsed if i hae an attribute query. If the configuration + // array is empty or not set then an empty array will be returned. + $metadata = $this->parseQueryAttributes($dom); + if (!empty($metadata)) { + $attributes = array_merge_recursive($attributes, $metadata); + } + + return [$user, $attributes]; } throw new Exception("Error parsing serviceResponse."); @@ -275,60 +286,117 @@ public function logout(array &$state): void } /** - * Extract the CAS user and attributes from an AuthenticationSuccess message. + * Parse a CAS AuthenticationSuccess into a flat associative array. + * + * Rules: + * - 'user' => content + * - For each attribute element (Chunk): + * - If prefix is 'cas' or empty => key is localName + * - Else => key is "prefix:localName" + * - Value is the element's textContent + * - If multiple values for the same key, collect into array * - * @param \SimpleSAML\CAS\XML\cas\AuthenticationSuccess $message - * @return array{0: string, 1: array} + * @return array{ + * 0: \SimpleSAML\XMLSchema\Type\Interface\ValueTypeInterface, + * 1: array> + * } */ - private function parseUserAndAttributes(AuthenticationSuccess $message): array + private function parseAuthenticationSuccess(AuthenticationSuccess $message): array { + /** @var array> $result */ + $result = []; + + // user -> content $user = $message->getUser()->getContent(); - // Build XPath from the same document/node we will query and reuse it. - $element = $message->toXML(); - // Dump the XML we received from the CAS server - Logger::debug('CAS client: serviceValidate XML: ' . $element->ownerDocument?->saveXML()); - - $xPath = XPath::getXPath($element); - - $attributes = []; - if ($casattributes = $this->casConfig['attributes']) { - // Some have attributes in the xml - attributes is a list of XPath expressions to get them - foreach ($casattributes as $name => $query) { - // If query is an absolute CAS path starting at /cas:serviceResponse/.../cas:authenticationSuccess/, - // rewrite it to be relative to the authenticationSuccess element. - // Example: - // /cas:serviceResponse/cas:authenticationSuccess/cas:attributes/cas:firstname - // /cas:serviceResponse/cas:authenticationSuccess/cas:user - // becomes: - // cas:attributes/cas:firstname - // cas:user - $marker = 'cas:authenticationSuccess/'; - if (isset($query[0]) && $query[0] === '/') { - $pos = strpos($query, $marker); - if ($pos !== false) { - $originalQuery = $query; - $query = substr($query, $pos + strlen($marker)); - Logger::info(sprintf( - 'CAS client: rewriting absolute CAS XPath for "%s" from "%s" to relative "%s"', - $name, - $originalQuery, - $query, - )); - } - } + // attributes -> elements (array of SimpleSAML\XML\Chunk) + $attributes = $message->getAttributes(); + /** @var list<\SimpleSAML\XML\Chunk> $elements */ + $elements = $attributes->getElements(); + + foreach ($elements as $chunk) { + // Safely extract localName, prefix, and DOMElement from the Chunk + $localName = $chunk->getLocalName(); + $prefix = $chunk->getPrefix(); + // DOMElement carrying the actual text content + $xmlElement = $chunk->getXML(); + + if (!$localName || !($xmlElement instanceof DOMElement)) { + continue; // skip malformed entries + } + + // Key selection rule + $key = ($prefix === '' || $prefix === 'cas') + ? $localName + : ($prefix . ':' . $localName); - $attrs = XPath::xpQuery($element, $query, $xPath); - foreach ($attrs as $attrvalue) { - $attributes[$name][] = $attrvalue->textContent; + $value = trim($xmlElement->textContent ?? ''); + + // Collect values (single or multi) + $result[$key] ??= []; + $result[$key][] = $value; + } + + return [$user, $result]; + } + + + /** + * Parse metadata attributes from CAS response XML using configured XPath queries + * + * @param DOMDocument $dom The XML document containing CAS response + * @return array> Array of metadata attribute names and values + */ + private function parseQueryAttributes(DOMDocument $dom): array + { + $root = $dom->documentElement; + if (!$root instanceof DOMElement) { + return []; + } + + $xPath = XPath::getXPath($root); + + $metadata = []; + $casattributes = $this->casConfig['attributes'] ?? null; + if (!is_array($casattributes)) { + return $metadata; + } + + // Some have attributes in the xml - attributes is a list of XPath expressions to get them + foreach ($casattributes as $name => $query) { + // If query is an absolute CAS path starting at /cas:serviceResponse/.../cas:authenticationSuccess/, + // rewrite it to be relative to the authenticationSuccess element. + // Example: + // /cas:serviceResponse/cas:authenticationSuccess/cas:attributes/cas:firstname + // /cas:serviceResponse/cas:authenticationSuccess/cas:user + // becomes: + // cas:attributes/cas:firstname + // cas:user + $marker = 'cas:authenticationSuccess/'; + if (isset($query[0]) && $query[0] === '/') { + $pos = strpos($query, $marker); + if ($pos !== false) { + $originalQuery = $query; + $query = substr($query, $pos + strlen($marker)); + Logger::info(sprintf( + 'CAS client: rewriting absolute CAS XPath for "%s" from "%s" to relative "%s"', + $name, + $originalQuery, + $query, + )); } - // log what we parsed for this attribute name - Logger::debug( - sprintf('CAS client: parsed attribute %s => %s', $name, json_encode($attributes[$name] ?? [])), - ); } + + $attrs = XPath::xpQuery($root, $query, $xPath); + foreach ($attrs as $attrvalue) { + $metadata[$name][] = $attrvalue->textContent; + } + // log what we parsed for this metadata name + Logger::debug( + sprintf('CAS client: parsed metadata %s => %s', $name, json_encode($metadata[$name] ?? [])), + ); } - return [$user, $attributes]; + return $metadata; } } diff --git a/tests/config/authsources.php b/tests/config/authsources.php index 3f0f726..23d07a9 100644 --- a/tests/config/authsources.php +++ b/tests/config/authsources.php @@ -22,6 +22,10 @@ 'logout' => 'https://ugrad.apply.example.edu/account/cas/logout', 'attributes' => [ 'uid' => 'cas:user', + // target the intended person under cas:attributes + 'person' => 'cas:attributes/slate:person', + // if you still want to capture the top-level one, keep a separate key: + 'person_top' => 'slate:person', 'sn' => 'cas:attributes/cas:sn', 'givenName' => 'cas:attributes/cas:firstname', 'mail' => 'cas:attributes/cas:mail', @@ -38,6 +42,10 @@ 'logout' => 'https://ugrad.apply.example.edu/account/cas/logout', 'attributes' => [ 'uid' => '/cas:serviceResponse/cas:authenticationSuccess/cas:user', + // target the intended person under cas:attributes + 'person' => '/cas:serviceResponse/cas:authenticationSuccess/cas:attributes/slate:person', + // if you still want to capture the top-level one, keep a separate key: + 'person_top' => '/cas:serviceResponse/cas:authenticationSuccess/slate:person', 'sn' => '/cas:serviceResponse/cas:authenticationSuccess/cas:attributes/cas:sn', 'givenName' => '/cas:serviceResponse/cas:authenticationSuccess/cas:attributes/cas:firstname', 'mail' => '/cas:serviceResponse/cas:authenticationSuccess/cas:attributes/cas:mail', @@ -47,4 +55,13 @@ ], 'ldap' => [], ], + 'casserver_auto_map' => [ + 'cas:CAS', + 'cas' => [ + 'login' => 'https://ugrad.apply.example.edu/account/cas/login', + 'serviceValidate' => 'https://ugrad.apply.example.edu/account/cas/serviceValidate', + 'logout' => 'https://ugrad.apply.example.edu/account/cas/logout', + ], + 'ldap' => [], + ], ]; diff --git a/tests/response/cas-success-service-response.xml b/tests/response/cas-success-service-response.xml index 4878f26..797e678 100644 --- a/tests/response/cas-success-service-response.xml +++ b/tests/response/cas-success-service-response.xml @@ -1,5 +1,7 @@ - + + 12345_top jdoe 2025-11-07T22:00:24+02:00 @@ -9,9 +11,10 @@ John jdoe@example.edu jdoe@example.edu + + 12345 + Fall-2025 + ABC-123 - 12345 - Fall-2025 - ABC-123 \ No newline at end of file diff --git a/tests/src/Controller/CASTest.php b/tests/src/Controller/CASTest.php index 3ee7a8a..eeef9ee 100644 --- a/tests/src/Controller/CASTest.php +++ b/tests/src/Controller/CASTest.php @@ -8,8 +8,8 @@ use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use SimpleSAML\Auth; -use SimpleSAML\CAS\XML\cas\AuthenticationSuccess; -use SimpleSAML\CAS\XML\cas\ServiceResponse; +use SimpleSAML\CAS\XML\AuthenticationSuccess; +use SimpleSAML\CAS\XML\ServiceResponse; use SimpleSAML\Configuration; use SimpleSAML\Error; use SimpleSAML\HTTP\RunnableResponse; @@ -60,6 +60,117 @@ protected function setUp(): void Configuration::setPreLoadedConfig($this->sourceConfig, 'authsources.php'); } + /** + * Verify constructor picks serviceValidate and login from the 'casserver' config + * (serviceValidate preferred when present). + * + * @throws \ReflectionException + */ + public function testConstructorUsesServiceValidateWhenPresent(): void + { + $authsources = Configuration::getConfig('authsources.php')->toArray(); + self::assertArrayHasKey('casserver', $authsources); + $sourceConfig = $authsources['casserver']; + + $cas = new CAS(['AuthId' => 'unit-cas'], $sourceConfig); + + $ref = new \ReflectionClass($cas); + $validationMethod = $ref->getProperty('validationMethod'); + $validationMethod->setAccessible(true); + $loginMethod = $ref->getProperty('loginMethod'); + $loginMethod->setAccessible(true); + + self::assertSame('serviceValidate', $validationMethod->getValue($cas)); + self::assertSame( + 'https://ugrad.apply.example.edu/account/cas/login', + $loginMethod->getValue($cas), + ); + } + + /** + * Verify constructor falls back to validate when serviceValidate is absent + * using the 'something' authsource. + * + * @throws \ReflectionException + */ + public function testConstructorUsesValidateWhenServiceValidateMissing(): void + { + $authsources = Configuration::getConfig('authsources.php')->toArray(); + self::assertArrayHasKey('something', $authsources); + $sourceConfig = $authsources['something']; + + $cas = new CAS(['AuthId' => 'unit-cas'], $sourceConfig); + + $ref = new \ReflectionClass($cas); + $validationMethod = $ref->getProperty('validationMethod'); + $validationMethod->setAccessible(true); + $loginMethod = $ref->getProperty('loginMethod'); + $loginMethod->setAccessible(true); + + self::assertSame('validate', $validationMethod->getValue($cas)); + self::assertSame('https://example.org/login', $loginMethod->getValue($cas)); + } + + /** + * When both serviceValidate and validate are present, serviceValidate is preferred. + * + * @throws \ReflectionException + */ + public function testConstructorPrefersServiceValidateIfBothPresent(): void + { + $config = [ + 'cas' => [ + 'login' => 'https://example.org/login', + 'serviceValidate' => 'https://example.org/sv', + 'validate' => 'https://example.org/v', + ], + 'ldap' => [], + ]; + + $cas = new CAS(['AuthId' => 'unit-cas'], $config); + + $ref = new \ReflectionClass($cas); + $validationMethod = $ref->getProperty('validationMethod'); + $validationMethod->setAccessible(true); + + self::assertSame('serviceValidate', $validationMethod->getValue($cas)); + } + + /** + * Missing both serviceValidate and validate should throw. + */ + public function testConstructorThrowsIfNoValidationMethodConfigured(): void + { + $config = [ + 'cas' => [ + 'login' => 'https://example.org/login', + // no serviceValidate / validate + ], + 'ldap' => [], + ]; + + $this->expectException(Exception::class); + $this->expectExceptionMessage('validate or serviceValidate not specified'); + new CAS(['AuthId' => 'unit-cas'], $config); + } + + /** + * Missing login should throw. + */ + public function testConstructorThrowsIfNoLoginConfigured(): void + { + $config = [ + 'cas' => [ + 'serviceValidate' => 'https://example.org/sv', + // no login + ], + 'ldap' => [], + ]; + + $this->expectException(Exception::class); + $this->expectExceptionMessage('cas login URL not specified'); + new CAS(['AuthId' => 'unit-cas'], $config); + } /** * Test that request without stateId results in a BadRequest-error @@ -278,6 +389,8 @@ public function testCasConfigAbsoluteXPathsReturnValues(string $sourceKey): void self::assertSame('jdoe', $user, "$sourceKey: user mismatch"); self::assertSame(['jdoe'], $attributes['uid'] ?? [], "$sourceKey: uid not extracted"); + self::assertSame(['12345'], $attributes['person'] ?? [], "$sourceKey: person not extracted"); + self::assertSame(['12345_top'], $attributes['person_top'] ?? [], "$sourceKey: person top not extracted"); self::assertSame(['Doe'], $attributes['sn'] ?? [], "$sourceKey: sn not extracted"); self::assertSame(['John'], $attributes['givenName'] ?? [], "$sourceKey: givenName not extracted"); self::assertSame(['jdoe@example.edu'], $attributes['mail'] ?? [], "$sourceKey: mail not extracted"); From 35f052e3d8e48539b8e647f61720a6ab8ae1d521 Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Mon, 24 Nov 2025 18:26:04 +0200 Subject: [PATCH 19/27] add tests. parse both attributes and metadata. --- src/Auth/Source/CAS.php | 145 +++++++++++++--- tests/config/authsources.php | 18 +- .../cas-success-service-response-slate.xml | 18 ++ tests/src/Controller/CASTest.php | 163 ++++++++++++++++-- 4 files changed, 300 insertions(+), 44 deletions(-) create mode 100644 tests/response/cas-success-service-response-slate.xml diff --git a/src/Auth/Source/CAS.php b/src/Auth/Source/CAS.php index b891c75..4d1045b 100644 --- a/src/Auth/Source/CAS.php +++ b/src/Auth/Source/CAS.php @@ -171,11 +171,23 @@ private function casServiceValidate(string $ticket, string $service): array } elseif ($message instanceof AuthenticationSuccess) { [$user, $attributes] = $this->parseAuthenticationSuccess($message); - // This will only be parsed if i hae an attribute query. If the configuration + // This will only be parsed if i have an attribute query. If the configuration // array is empty or not set then an empty array will be returned. - $metadata = $this->parseQueryAttributes($dom); - if (!empty($metadata)) { - $attributes = array_merge_recursive($attributes, $metadata); + $attributesFromQueryConfiguration = $this->parseQueryAttributes($dom); + if (!empty($attributesFromQueryConfiguration)) { + // Overwrite attributes from parseAuthenticationSuccess with configured + // XPath-based attributes, instead of combining them. + foreach ($attributesFromQueryConfiguration as $name => $values) { + if (!is_array($values)) { + $values = [$values]; + } + + // Ensure a clean, unique list of string values + $values = array_values(array_unique(array_map('strval', $values))); + + // Configuration wins: replace any existing attribute with the same name + $attributes[$name] = $values; + } } return [$user, $attributes]; @@ -337,9 +349,89 @@ private function parseAuthenticationSuccess(AuthenticationSuccess $message): arr $result[$key][] = $value; } + // 2) Metadata children from AuthenticationSuccess::getAuthenticationSuccessMetadata() + // (DOMElement instances under cas:authenticationSuccess, outside cas:attributes) + // + /** @var list $metaElements */ + $metaElements = $message->getAuthenticationSuccessMetadata(); + + foreach ($metaElements as $element) { + if (!$element instanceof DOMElement) { + continue; + } + + $localName = $element->localName; + $prefix = $element->prefix ?? ''; + + if ($localName === null || $localName === '') { + continue; + } + + // For metadata elements we do NOT special-case 'cas': + // we always use "prefix:localName" when there is a prefix, + // and just localName when there is none. + $key = ($prefix === '') + ? $localName + : ($prefix . ':' . $localName); + + $value = trim($element->textContent ?? ''); + + $result[$key] ??= []; + $result[$key][] = $value; + } + return [$user, $result]; } + /** + * Parse AuthenticationSuccess "metadata" children into a flat associative array. + * + * Source: AuthenticationSuccess::getAuthenticationSuccessMetadata() + * (an array of DOMElement instances under cas:authenticationSuccess, + * outside cas:attributes). + * + * Rules: + * - If element prefix is 'cas' or empty => key is localName + * - Else => key is "prefix:localName" + * - Value is element's textContent (trimmed) + * - If multiple elements resolve to the same key, collect all values in an array + * + * @return array> + */ + private function parseAuthenticationSuccessMetadata(AuthenticationSuccess $message): array + { + /** @var array> $result */ + $result = []; + + /** @var list $metaElements */ + $metaElements = $message->getAuthenticationSuccessMetadata(); + + foreach ($metaElements as $element) { + if (!$element instanceof DOMElement) { + continue; + } + + $localName = $element->localName; + $prefix = $element->prefix ?? ''; + + if ($localName === null || $localName === '') { + continue; + } + + // Same keying rule as for attribute Chunks + $key = ($prefix === '' || $prefix === 'cas') + ? $localName + : ($prefix . ':' . $localName); + + $value = trim($element->textContent ?? ''); + + $result[$key] ??= []; + $result[$key][] = $value; + } + + return $result; + } + /** * Parse metadata attributes from CAS response XML using configured XPath queries @@ -354,7 +446,7 @@ private function parseQueryAttributes(DOMDocument $dom): array return []; } - $xPath = XPath::getXPath($root); + $xPath = XPath::getXPath($root, true); $metadata = []; $casattributes = $this->casConfig['attributes'] ?? null; @@ -362,39 +454,46 @@ private function parseQueryAttributes(DOMDocument $dom): array return $metadata; } + /** @var list<\DOMElement> $authnNodes */ + $authnNodes = XPath::xpQuery($root, 'cas:authenticationSuccess', $xPath); + /** @var \DOMElement|null $authn */ + $authn = $authnNodes[0] ?? null; + // Some have attributes in the xml - attributes is a list of XPath expressions to get them foreach ($casattributes as $name => $query) { - // If query is an absolute CAS path starting at /cas:serviceResponse/.../cas:authenticationSuccess/, - // rewrite it to be relative to the authenticationSuccess element. - // Example: - // /cas:serviceResponse/cas:authenticationSuccess/cas:attributes/cas:firstname - // /cas:serviceResponse/cas:authenticationSuccess/cas:user - // becomes: - // cas:attributes/cas:firstname - // cas:user $marker = 'cas:authenticationSuccess/'; + if (isset($query[0]) && $query[0] === '/') { - $pos = strpos($query, $marker); - if ($pos !== false) { + // Absolute XPath + if (strpos($query, $marker) !== false && $authn instanceof \DOMElement) { $originalQuery = $query; - $query = substr($query, $pos + strlen($marker)); + $query = substr($query, strpos($query, $marker) + strlen($marker)); Logger::info(sprintf( 'CAS client: rewriting absolute CAS XPath for "%s" from "%s" to relative "%s"', $name, $originalQuery, $query, )); + $nodes = XPath::xpQuery($authn, $query, $xPath); + } else { + // Keep absolute; evaluate from document root + $nodes = XPath::xpQuery($root, $query, $xPath); } + } else { + // Relative XPath; prefer evaluating under authenticationSuccess if available + $context = $authn instanceof \DOMElement ? $authn : $root; + $nodes = XPath::xpQuery($context, $query, $xPath); } - $attrs = XPath::xpQuery($root, $query, $xPath); - foreach ($attrs as $attrvalue) { - $metadata[$name][] = $attrvalue->textContent; + foreach ($nodes as $n) { + $metadata[$name][] = trim($n->textContent); } - // log what we parsed for this metadata name - Logger::debug( - sprintf('CAS client: parsed metadata %s => %s', $name, json_encode($metadata[$name] ?? [])), - ); + + Logger::debug(sprintf( + 'CAS client: parsed metadata %s => %s', + $name, + json_encode($metadata[$name] ?? []), + )); } return $metadata; diff --git a/tests/config/authsources.php b/tests/config/authsources.php index 23d07a9..6ca3a8e 100644 --- a/tests/config/authsources.php +++ b/tests/config/authsources.php @@ -21,7 +21,7 @@ 'serviceValidate' => 'https://ugrad.apply.example.edu/account/cas/serviceValidate', 'logout' => 'https://ugrad.apply.example.edu/account/cas/logout', 'attributes' => [ - 'uid' => 'cas:user', + 'user' => 'cas:user', // target the intended person under cas:attributes 'person' => 'cas:attributes/slate:person', // if you still want to capture the top-level one, keep a separate key: @@ -41,7 +41,7 @@ 'serviceValidate' => 'https://ugrad.apply.example.edu/account/cas/serviceValidate', 'logout' => 'https://ugrad.apply.example.edu/account/cas/logout', 'attributes' => [ - 'uid' => '/cas:serviceResponse/cas:authenticationSuccess/cas:user', + 'user' => '/cas:serviceResponse/cas:authenticationSuccess/cas:user', // target the intended person under cas:attributes 'person' => '/cas:serviceResponse/cas:authenticationSuccess/cas:attributes/slate:person', // if you still want to capture the top-level one, keep a separate key: @@ -61,6 +61,20 @@ 'login' => 'https://ugrad.apply.example.edu/account/cas/login', 'serviceValidate' => 'https://ugrad.apply.example.edu/account/cas/serviceValidate', 'logout' => 'https://ugrad.apply.example.edu/account/cas/logout', + 'attributes' => [ + // CAS core identifier + 'user' => 'cas:user', + + // Top-level slate elements under cas:authenticationSuccess + 'slate:person'=> 'slate:person', + 'slate:round' => 'slate:round', + 'slate:ref' => 'slate:ref', + + // Attributes inside … + 'firstname' => 'cas:attributes/cas:firstname', + 'lastname' => 'cas:attributes/cas:lastname', + 'email' => 'cas:attributes/cas:email', + ], ], 'ldap' => [], ], diff --git a/tests/response/cas-success-service-response-slate.xml b/tests/response/cas-success-service-response-slate.xml new file mode 100644 index 0000000..6219d99 --- /dev/null +++ b/tests/response/cas-success-service-response-slate.xml @@ -0,0 +1,18 @@ + + + + example-user@technolutions.com + 345d2e1b-65de-419c-96ce-e1866d4c57cd + Regular Decision + 774482874 + + 2025-11-07T22:00:24+02:00 + true + true + Example + User + example-user@technolutions.com + + + \ No newline at end of file diff --git a/tests/src/Controller/CASTest.php b/tests/src/Controller/CASTest.php index eeef9ee..b3b8933 100644 --- a/tests/src/Controller/CASTest.php +++ b/tests/src/Controller/CASTest.php @@ -185,8 +185,11 @@ public function testNoStateId(): void $c = new Controller\CAS($this->config); $this->expectException(Error\BadRequest::class); - $this->expectExceptionMessage("BADREQUEST('%REASON%' => 'Missing stateId parameter.')"); - + $errorResponse = [ + 'errorCode' => 'BADREQUEST', + '%REASON%' => 'Missing stateId parameter.', + ]; + $this->expectExceptionMessage(json_encode($errorResponse, JSON_THROW_ON_ERROR)); $c->linkback($request); } @@ -231,7 +234,11 @@ public static function loadState(string $id, string $stage, bool $allowMissing = }); $this->expectException(Error\BadRequest::class); - $this->expectExceptionMessage("BADREQUEST('%REASON%' => 'Missing ticket parameter.')"); + $errorResponse = [ + 'errorCode' => 'BADREQUEST', + '%REASON%' => 'Missing ticket parameter.', + ]; + $this->expectExceptionMessage(json_encode($errorResponse, JSON_THROW_ON_ERROR)); $c->linkback($request); } @@ -378,26 +385,144 @@ public function testCasConfigAbsoluteXPathsReturnValues(string $sourceKey): void // Instantiate the CAS source with the selected configuration $cas = new Cas(['AuthId' => 'unit-cas'], $sourceConfig); - // Invoke the private parseUserAndAttributes() method via reflection - $refMethod = new \ReflectionMethod(Cas::class, 'parseUserAndAttributes'); - $refMethod->setAccessible(true); - /** @var array{0:string,1:array>} $result */ - $result = $refMethod->invoke($cas, $message); + // Invoke the new private methods via reflection + $ref = new \ReflectionClass(Cas::class); + + $parseAuthSuccess = $ref->getMethod('parseAuthenticationSuccess'); + $parseAuthSuccess->setAccessible(true); + /** @var array{0:string,1:array>} $userAndAttrs */ + $userAndAttrs = $parseAuthSuccess->invoke($cas, $message); + + $parseQueryAttrs = $ref->getMethod('parseQueryAttributes'); + $parseQueryAttrs->setAccessible(true); + /** @var array> $queryAttrs */ + $queryAttrs = $parseQueryAttrs->invoke($cas, $dom); + + // Merge attribute arrays (values are lists) + [$user, $elementAttrs] = $userAndAttrs; + // Normalize user to a plain string (may be a StringValue-like object) + $user = strval($user); + + /** @var array> $attributes */ + $attributes = $elementAttrs; + foreach ($queryAttrs as $k => $vals) { + if (!isset($attributes[$k])) { + $attributes[$k] = []; + } + // Append preserving order + foreach ($vals as $v) { + $attributes[$k][] = $v; + } + } // Assert user and attributes are identical for both configurations - [$user, $attributes] = $result; - self::assertSame('jdoe', $user, "$sourceKey: user mismatch"); - self::assertSame(['jdoe'], $attributes['uid'] ?? [], "$sourceKey: uid not extracted"); - self::assertSame(['12345'], $attributes['person'] ?? [], "$sourceKey: person not extracted"); - self::assertSame(['12345_top'], $attributes['person_top'] ?? [], "$sourceKey: person top not extracted"); - self::assertSame(['Doe'], $attributes['sn'] ?? [], "$sourceKey: sn not extracted"); - self::assertSame(['John'], $attributes['givenName'] ?? [], "$sourceKey: givenName not extracted"); - self::assertSame(['jdoe@example.edu'], $attributes['mail'] ?? [], "$sourceKey: mail not extracted"); + //phpcs:ignore Generic.Files.LineLength.TooLong + self::assertSame('12345', array_pop($attributes['person']) ?? '', "$sourceKey: person not extracted"); + //phpcs:ignore Generic.Files.LineLength.TooLong + self::assertSame('12345_top', array_pop($attributes['person_top']) ?? '', "$sourceKey: person top not extracted"); + //phpcs:ignore Generic.Files.LineLength.TooLong + self::assertSame('Doe', array_pop($attributes['sn']) ?? '', "$sourceKey: sn not extracted"); + //phpcs:ignore Generic.Files.LineLength.TooLong + self::assertSame('John', array_pop($attributes['givenName']) ?? '', "$sourceKey: givenName not extracted"); + //phpcs:ignore Generic.Files.LineLength.TooLong + self::assertSame('jdoe@example.edu', array_pop($attributes['mail']) ?? '', "$sourceKey: mail not extracted"); + //phpcs:ignore Generic.Files.LineLength.TooLong + self::assertSame('jdoe@example.edu', array_pop($attributes['eduPersonPrincipalName']) ?? '', "$sourceKey: ePPN not extracted",); + } + + /** + * Ensure that for casserver attributes configuration and the slate CAS response, + * the attributes built from the AuthenticationSuccess model match + * exactly those extracted via XPath configuration: same keys, + * same values (per key), and same total count. + */ + public function testCasserverAutoMapAttributesMatchBetweenModelAndXPath(): void + { + // Load authsources and retrieve casserver_auto_map configuration + $authsources = Configuration::getConfig('authsources.php'); + $config = $authsources->toArray(); + + self::assertIsArray($config, 'authsources.php did not return expected $config array'); + self::assertArrayHasKey( + 'casserver_auto_map', + $config, + "Missing source 'casserver_auto_map' in authsources.php", + ); + $sourceConfig = $config['casserver_auto_map']; + self::assertArrayHasKey('cas', $sourceConfig, "Missing 'cas' config for 'casserver_auto_map'"); + self::assertArrayHasKey('ldap', $sourceConfig, "Missing 'ldap' config for 'casserver_auto_map'"); + + // Load the CAS success message XML (slate variant) + $successXmlFile = dirname(__DIR__, 1) . '/../response/cas-success-service-response-slate.xml'; + self::assertFileExists($successXmlFile, 'Slate CAS success XML not found at expected path'); + + $dom = DOMDocumentFactory::fromFile($successXmlFile); + $root = $dom->documentElement; + if (!$root instanceof \DOMElement) { + self::fail('Loaded slate XML does not have a document element'); + } + + // Build AuthenticationSuccess message from XML + $serviceResponse = ServiceResponse::fromXML($root); + $message = $serviceResponse->getResponse(); + self::assertInstanceOf( + AuthenticationSuccess::class, + $message, + 'Expected AuthenticationSuccess message for slate XML', + ); + + // Instantiate the CAS source with casserver_auto_map configuration + $cas = new Cas(['AuthId' => 'unit-cas'], $sourceConfig); + + // Use reflection to access the private parsers + $ref = new \ReflectionClass(Cas::class); + + $parseAuthSuccess = $ref->getMethod('parseAuthenticationSuccess'); + $parseAuthSuccess->setAccessible(true); + /** @var array{0:mixed,1:array>} $userAndModelAttrs */ + $userAndModelAttrs = $parseAuthSuccess->invoke($cas, $message); + + $parseQueryAttrs = $ref->getMethod('parseQueryAttributes'); + $parseQueryAttrs->setAccessible(true); + /** @var array> $xpathAttrs */ + $xpathAttrs = $parseQueryAttrs->invoke($cas, $dom); + + [, $modelAttrs] = $userAndModelAttrs; + + // Normalize both maps: sort keys and sort values within each key + ksort($modelAttrs); + ksort($xpathAttrs); + + foreach ($modelAttrs as $k => &$vals) { + sort($vals); + } + unset($vals); + + foreach ($xpathAttrs as $k => &$vals) { + sort($vals); + } + unset($vals); + + // Assert same number of attributes + self::assertCount( + count($modelAttrs), + $xpathAttrs, + 'Attribute count mismatch between model and XPath extraction', + ); + + // Assert same keys + self::assertSame( + array_keys($modelAttrs), + array_keys($xpathAttrs), + 'Attribute keys mismatch between model and XPath extraction', + ); + + // Assert same values per key self::assertSame( - ['jdoe@example.edu'], - $attributes['eduPersonPrincipalName'] ?? [], - "$sourceKey: ePPN not extracted", + $modelAttrs, + $xpathAttrs, + 'Attribute values mismatch between model and XPath extraction', ); } } From 0cc392b5bab093309165e5a7a39bb20e2e64d300 Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Mon, 24 Nov 2025 18:47:27 +0200 Subject: [PATCH 20/27] Sync changes. Fix quality issues. --- src/Auth/Source/CAS.php | 69 ++++++++------------------------ tests/config/authsources.php | 2 +- tests/src/Controller/CASTest.php | 51 +++++++++-------------- 3 files changed, 36 insertions(+), 86 deletions(-) diff --git a/src/Auth/Source/CAS.php b/src/Auth/Source/CAS.php index 4d1045b..21ab525 100644 --- a/src/Auth/Source/CAS.php +++ b/src/Auth/Source/CAS.php @@ -308,6 +308,7 @@ public function logout(array &$state): void * - Value is the element's textContent * - If multiple values for the same key, collect into array * + * @param \SimpleSAML\CAS\XML\AuthenticationSuccess $message The authentication success message to parse * @return array{ * 0: \SimpleSAML\XMLSchema\Type\Interface\ValueTypeInterface, * 1: array> @@ -349,61 +350,23 @@ private function parseAuthenticationSuccess(AuthenticationSuccess $message): arr $result[$key][] = $value; } - // 2) Metadata children from AuthenticationSuccess::getAuthenticationSuccessMetadata() - // (DOMElement instances under cas:authenticationSuccess, outside cas:attributes) - // - /** @var list $metaElements */ - $metaElements = $message->getAuthenticationSuccessMetadata(); - - foreach ($metaElements as $element) { - if (!$element instanceof DOMElement) { - continue; - } - - $localName = $element->localName; - $prefix = $element->prefix ?? ''; - - if ($localName === null || $localName === '') { - continue; - } - - // For metadata elements we do NOT special-case 'cas': - // we always use "prefix:localName" when there is a prefix, - // and just localName when there is none. - $key = ($prefix === '') - ? $localName - : ($prefix . ':' . $localName); - - $value = trim($element->textContent ?? ''); - - $result[$key] ??= []; - $result[$key][] = $value; - } + // Metadata children from AuthenticationSuccess::getAuthenticationSuccessMetadata() + // (DOMElement instances under cas:authenticationSuccess, outside cas:attributes) + $this->parseAuthenticationSuccessMetadata($message, $result); return [$user, $result]; } + /** - * Parse AuthenticationSuccess "metadata" children into a flat associative array. - * - * Source: AuthenticationSuccess::getAuthenticationSuccessMetadata() - * (an array of DOMElement instances under cas:authenticationSuccess, - * outside cas:attributes). + * Parse metadata elements from AuthenticationSuccess message and add them to attributes array * - * Rules: - * - If element prefix is 'cas' or empty => key is localName - * - Else => key is "prefix:localName" - * - Value is element's textContent (trimmed) - * - If multiple elements resolve to the same key, collect all values in an array - * - * @return array> + * @param \SimpleSAML\CAS\XML\AuthenticationSuccess $message The authentication success message + * @param array> &$attributes Reference to attributes array to update + * @return void */ - private function parseAuthenticationSuccessMetadata(AuthenticationSuccess $message): array + private function parseAuthenticationSuccessMetadata(AuthenticationSuccess $message, array &$attributes): void { - /** @var array> $result */ - $result = []; - - /** @var list $metaElements */ $metaElements = $message->getAuthenticationSuccessMetadata(); foreach ($metaElements as $element) { @@ -418,18 +381,18 @@ private function parseAuthenticationSuccessMetadata(AuthenticationSuccess $messa continue; } - // Same keying rule as for attribute Chunks - $key = ($prefix === '' || $prefix === 'cas') + // For metadata elements we do NOT special-case 'cas': + // we always use "prefix:localName" when there is a prefix, + // and just localName when there is none. + $key = ($prefix === '') ? $localName : ($prefix . ':' . $localName); $value = trim($element->textContent ?? ''); - $result[$key] ??= []; - $result[$key][] = $value; + $attributes[$key] ??= []; + $attributes[$key][] = $value; } - - return $result; } diff --git a/tests/config/authsources.php b/tests/config/authsources.php index 6ca3a8e..2e5f23f 100644 --- a/tests/config/authsources.php +++ b/tests/config/authsources.php @@ -66,7 +66,7 @@ 'user' => 'cas:user', // Top-level slate elements under cas:authenticationSuccess - 'slate:person'=> 'slate:person', + 'slate:person' => 'slate:person', 'slate:round' => 'slate:round', 'slate:ref' => 'slate:ref', diff --git a/tests/src/Controller/CASTest.php b/tests/src/Controller/CASTest.php index b3b8933..4f0bdd6 100644 --- a/tests/src/Controller/CASTest.php +++ b/tests/src/Controller/CASTest.php @@ -488,41 +488,28 @@ public function testCasserverAutoMapAttributesMatchBetweenModelAndXPath(): void /** @var array> $xpathAttrs */ $xpathAttrs = $parseQueryAttrs->invoke($cas, $dom); - [, $modelAttrs] = $userAndModelAttrs; + [$user, $modelAttrs] = $userAndModelAttrs; - // Normalize both maps: sort keys and sort values within each key - ksort($modelAttrs); - ksort($xpathAttrs); + // I have to add this here because the query attributes has it. + // I have to add this here because the query attributes has it. + self::assertInstanceOf(ValueTypeInterface::class, $user); + $modelAttrs['user'] = [$user->getValue()]; - foreach ($modelAttrs as $k => &$vals) { - sort($vals); - } - unset($vals); - - foreach ($xpathAttrs as $k => &$vals) { - sort($vals); - } - unset($vals); - - // Assert same number of attributes - self::assertCount( - count($modelAttrs), - $xpathAttrs, - 'Attribute count mismatch between model and XPath extraction', - ); // Assert same keys - self::assertSame( - array_keys($modelAttrs), - array_keys($xpathAttrs), - 'Attribute keys mismatch between model and XPath extraction', - ); - - // Assert same values per key - self::assertSame( - $modelAttrs, - $xpathAttrs, - 'Attribute values mismatch between model and XPath extraction', - ); + $modelKeys = array_keys($modelAttrs); + $xpathKeys = array_keys($xpathAttrs); + sort($modelKeys); + sort($xpathKeys); + + self::assertSame($modelKeys, $xpathKeys, 'Attribute keys mismatch between model and XPath extraction'); + + foreach ($modelAttrs as $key => $values) { + $this->assertTrue(isset($xpathAttrs[$key]), "Missing attribute '$key' in XPath extraction"); + $this->assertTrue( + in_array($values[0], $xpathAttrs[$key], true), + "Attribute '$key' values mismatch", + ); + } } } From bfd094facdc9a875eb6da2b32bdd4c41b98fa107 Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Mon, 15 Dec 2025 18:28:34 +0200 Subject: [PATCH 21/27] fix depedency list.Use new xml-cas-module-slate. --- composer.json | 12 +++- docs/cas.md | 36 +++++++++- src/Auth/Source/CAS.php | 67 +++++++++++-------- tests/config/authsources.php | 1 + .../cas-success-service-response-slate.xml | 3 - tests/src/Controller/CASTest.php | 41 ++++++++---- 6 files changed, 112 insertions(+), 48 deletions(-) diff --git a/composer.json b/composer.json index 1ff8e9f..4fe9061 100644 --- a/composer.json +++ b/composer.json @@ -42,12 +42,18 @@ "simplesamlphp/composer-module-installer": "^1.4", "simplesamlphp/simplesamlphp": "dev-simplesamlphp-2.5 as v2.5.x-dev", "simplesamlphp/simplesamlphp-module-ldap": "~1.2", - "simplesamlphp/xml-cas": "~2.1.0", + "simplesamlphp/xml-cas-module-slate": "~1.1.0", + "simplesamlphp/xml-cas": "^v2.2.0", "simplesamlphp/xml-common": "~2.4.0", - "symfony/http-foundation": "~7.3.0" + "symfony/http-foundation": "~7.4.0" }, "require-dev": { - "simplesamlphp/simplesamlphp-test-framework": "^1.10" + "simplesamlphp/simplesamlphp-test-framework": "^1.10", + "phpunit/phpunit": "^11", + "icanhazstring/composer-unused": "^0.9.5", + "squizlabs/php_codesniffer": "^4.0.0", + "phpstan/phpstan": "^2.1.33", + "maglnet/composer-require-checker": "^4" }, "support": { "issues": "https://github.com/simplesamlphp/simplesamlphp-module-cas/issues", diff --git a/docs/cas.md b/docs/cas.md index fa3293d..54adb32 100644 --- a/docs/cas.md +++ b/docs/cas.md @@ -5,7 +5,7 @@ the only difference is this is authentication module and not a script. ## Setting up the CAS authentication module -Adding a authentication source +Adding an authentication source Example authsource.php: @@ -31,7 +31,7 @@ Example authsource.php: ## Querying Attributes -CAS V3 (since 2017) supports querying attributes. Those have to be published +CAS v3 (since 2017) supports querying attributes. Those have to be published for the service you're calling. Here the service publishes `sn`, `firstName` and `mail`. @@ -51,6 +51,36 @@ Or you might have to call serviceValidate for Protocol 3 via **/p3/**: ] ``` + +### Optional: Enabling Slate extensions + +Some deployments include vendor‑specific fields (for example `slate:*`) in CAS responses. +You can opt in to Slate support: + +```php +'cas' => [ + // ... + 'serviceValidate' => 'https://cas.example.com/p3/serviceValidate', + // Enable Slate support (optional) + 'slate.enabled' => true, + + // Optional XPath-based attribute mappings + 'attributes' => [ + // Standard CAS attributes + 'uid' => 'cas:user', + 'mail' => 'cas:attributes/cas:mail', + + // Slate namespaced attributes inside cas:attributes + 'slate_person' => 'cas:attributes/slate:person', + 'slate_round' => 'cas:attributes/slate:round', + 'slate_ref' => 'cas:attributes/slate:ref', + + // Some deployments also place vendor elements at the top level + 'slate_person_top' => '/cas:serviceResponse/cas:authenticationSuccess/slate:person', + ], +], +``` + which would return something like ```xml @@ -91,7 +121,7 @@ and even some custom attributes if they're set: ``` You'll probably want to avoid querying LDAP for attributes: -set `ldap` to a `null`: +set `ldap` to `null`: ```php 'example-cas' => [ diff --git a/src/Auth/Source/CAS.php b/src/Auth/Source/CAS.php index 21ab525..4c9f596 100644 --- a/src/Auth/Source/CAS.php +++ b/src/Auth/Source/CAS.php @@ -10,13 +10,16 @@ use SimpleSAML\Auth; use SimpleSAML\CAS\Utils\XPath; use SimpleSAML\CAS\XML\AuthenticationFailure; -use SimpleSAML\CAS\XML\AuthenticationSuccess; -use SimpleSAML\CAS\XML\ServiceResponse; +use SimpleSAML\CAS\XML\AuthenticationSuccess as CasAuthnSuccess; +use SimpleSAML\CAS\XML\ServiceResponse as CasServiceResponse; use SimpleSAML\Configuration; use SimpleSAML\Logger; use SimpleSAML\Module; use SimpleSAML\Module\ldap\Auth\Ldap; +use SimpleSAML\Slate\XML\AuthenticationSuccess as SlateAuthnSuccess; +use SimpleSAML\Slate\XML\ServiceResponse as SlateServiceResponse; use SimpleSAML\Utils; +use SimpleSAML\XML\Chunk; use SimpleSAML\XML\DOMDocumentFactory; use function array_merge_recursive; @@ -68,12 +71,10 @@ class CAS extends Auth\Source private string $loginMethod; /** - * If enabled, convert all CAS attributes found in the XML: - * - cas:NAME => NAME - * - OTHERPREFIX:NAME => OTHERPREFIX:NAME - * - collect multi-valued elements into arrays of strings + * @var bool flag indicating if slate XML format should be used */ -// private bool $convertAllAttributes; + private bool $useSlate; + /** * Constructor for this authentication source. @@ -104,6 +105,8 @@ public function __construct(array $info, array $config) } else { throw new Exception("cas login URL not specified"); } + + $this->useSlate = $this->casConfig['slate.enabled'] ?? false; } @@ -160,7 +163,12 @@ private function casServiceValidate(string $ticket, string $service): array /** @var string $result */ $dom = DOMDocumentFactory::fromString($result); - $serviceResponse = ServiceResponse::fromXML($dom->documentElement); + if ($this->useSlate) { + $serviceResponse = SlateServiceResponse::fromXML($dom->documentElement); + } else { + $serviceResponse = CasServiceResponse::fromXML($dom->documentElement); + } + $message = $serviceResponse->getResponse(); if ($message instanceof AuthenticationFailure) { throw new Exception(sprintf( @@ -168,7 +176,7 @@ private function casServiceValidate(string $ticket, string $service): array strval($message->getContent()), strval($message->getCode()), )); - } elseif ($message instanceof AuthenticationSuccess) { + } elseif ($message instanceof CasAuthnSuccess || $message instanceof SlateAuthnSuccess) { [$user, $attributes] = $this->parseAuthenticationSuccess($message); // This will only be parsed if i have an attribute query. If the configuration @@ -178,10 +186,6 @@ private function casServiceValidate(string $ticket, string $service): array // Overwrite attributes from parseAuthenticationSuccess with configured // XPath-based attributes, instead of combining them. foreach ($attributesFromQueryConfiguration as $name => $values) { - if (!is_array($values)) { - $values = [$values]; - } - // Ensure a clean, unique list of string values $values = array_values(array_unique(array_map('strval', $values))); @@ -297,6 +301,7 @@ public function logout(array &$state): void $httpUtils->redirectTrustedURL($logoutUrl); } + /** * Parse a CAS AuthenticationSuccess into a flat associative array. * @@ -308,13 +313,14 @@ public function logout(array &$state): void * - Value is the element's textContent * - If multiple values for the same key, collect into array * - * @param \SimpleSAML\CAS\XML\AuthenticationSuccess $message The authentication success message to parse + * @param \SimpleSAML\CAS\XML\AuthenticationSuccess|\SimpleSAML\Slate\XML\AuthenticationSuccess $message + * The authentication success message to parse * @return array{ * 0: \SimpleSAML\XMLSchema\Type\Interface\ValueTypeInterface, * 1: array> * } */ - private function parseAuthenticationSuccess(AuthenticationSuccess $message): array + private function parseAuthenticationSuccess(CasAuthnSuccess|SlateAuthnSuccess $message): array { /** @var array> $result */ $result = []; @@ -334,7 +340,7 @@ private function parseAuthenticationSuccess(AuthenticationSuccess $message): arr // DOMElement carrying the actual text content $xmlElement = $chunk->getXML(); - if (!$localName || !($xmlElement instanceof DOMElement)) { + if (!$localName) { continue; // skip malformed entries } @@ -350,7 +356,6 @@ private function parseAuthenticationSuccess(AuthenticationSuccess $message): arr $result[$key][] = $value; } - // Metadata children from AuthenticationSuccess::getAuthenticationSuccessMetadata() // (DOMElement instances under cas:authenticationSuccess, outside cas:attributes) $this->parseAuthenticationSuccessMetadata($message, $result); @@ -361,23 +366,31 @@ private function parseAuthenticationSuccess(AuthenticationSuccess $message): arr /** * Parse metadata elements from AuthenticationSuccess message and add them to attributes array * - * @param \SimpleSAML\CAS\XML\AuthenticationSuccess $message The authentication success message + * @param \SimpleSAML\CAS\XML\AuthenticationSuccess|\SimpleSAML\Slate\XML\AuthenticationSuccess $message + * The authentication success message * @param array> &$attributes Reference to attributes array to update * @return void */ - private function parseAuthenticationSuccessMetadata(AuthenticationSuccess $message, array &$attributes): void - { - $metaElements = $message->getAuthenticationSuccessMetadata(); + private function parseAuthenticationSuccessMetadata( + CasAuthnSuccess|SlateAuthnSuccess $message, + array &$attributes, + ): void { + if (!method_exists($message, 'getElements')) { + // Either bail out or use a fallback + return; + } + + $metaElements = $message->getElements(); foreach ($metaElements as $element) { - if (!$element instanceof DOMElement) { + if (!$element instanceof Chunk) { continue; } - $localName = $element->localName; - $prefix = $element->prefix ?? ''; + $localName = $element->getLocalName(); + $prefix = $element->getPrefix(); - if ($localName === null || $localName === '') { + if ($localName === '') { continue; } @@ -388,7 +401,7 @@ private function parseAuthenticationSuccessMetadata(AuthenticationSuccess $messa ? $localName : ($prefix . ':' . $localName); - $value = trim($element->textContent ?? ''); + $value = trim($element->getXML()->textContent ?? ''); $attributes[$key] ??= []; $attributes[$key][] = $value; @@ -399,7 +412,7 @@ private function parseAuthenticationSuccessMetadata(AuthenticationSuccess $messa /** * Parse metadata attributes from CAS response XML using configured XPath queries * - * @param DOMDocument $dom The XML document containing CAS response + * @param \DOMDocument $dom The XML document containing CAS response * @return array> Array of metadata attribute names and values */ private function parseQueryAttributes(DOMDocument $dom): array diff --git a/tests/config/authsources.php b/tests/config/authsources.php index 2e5f23f..3866ca2 100644 --- a/tests/config/authsources.php +++ b/tests/config/authsources.php @@ -58,6 +58,7 @@ 'casserver_auto_map' => [ 'cas:CAS', 'cas' => [ + 'slate.enabled' => true, 'login' => 'https://ugrad.apply.example.edu/account/cas/login', 'serviceValidate' => 'https://ugrad.apply.example.edu/account/cas/serviceValidate', 'logout' => 'https://ugrad.apply.example.edu/account/cas/logout', diff --git a/tests/response/cas-success-service-response-slate.xml b/tests/response/cas-success-service-response-slate.xml index 6219d99..0626647 100644 --- a/tests/response/cas-success-service-response-slate.xml +++ b/tests/response/cas-success-service-response-slate.xml @@ -7,9 +7,6 @@ Regular Decision 774482874 - 2025-11-07T22:00:24+02:00 - true - true Example User example-user@technolutions.com diff --git a/tests/src/Controller/CASTest.php b/tests/src/Controller/CASTest.php index 4f0bdd6..c5eb80e 100644 --- a/tests/src/Controller/CASTest.php +++ b/tests/src/Controller/CASTest.php @@ -10,12 +10,16 @@ use SimpleSAML\Auth; use SimpleSAML\CAS\XML\AuthenticationSuccess; use SimpleSAML\CAS\XML\ServiceResponse; +use SimpleSAML\CAS\XML\ServiceResponse as CasServiceResponse; use SimpleSAML\Configuration; use SimpleSAML\Error; use SimpleSAML\HTTP\RunnableResponse; use SimpleSAML\Module\cas\Auth\Source\CAS; use SimpleSAML\Module\cas\Controller; +use SimpleSAML\Slate\XML\AuthenticationSuccess as SlateAuthenticationSuccess; +use SimpleSAML\Slate\XML\ServiceResponse as SlateServiceResponse; use SimpleSAML\XML\DOMDocumentFactory; +use SimpleSAML\XMLSchema\Type\Interface\ValueTypeInterface; use Symfony\Component\HttpFoundation\Request; /** @@ -60,6 +64,7 @@ protected function setUp(): void Configuration::setPreLoadedConfig($this->sourceConfig, 'authsources.php'); } + /** * Verify constructor picks serviceValidate and login from the 'casserver' config * (serviceValidate preferred when present). @@ -87,6 +92,7 @@ public function testConstructorUsesServiceValidateWhenPresent(): void ); } + /** * Verify constructor falls back to validate when serviceValidate is absent * using the 'something' authsource. @@ -111,6 +117,7 @@ public function testConstructorUsesValidateWhenServiceValidateMissing(): void self::assertSame('https://example.org/login', $loginMethod->getValue($cas)); } + /** * When both serviceValidate and validate are present, serviceValidate is preferred. * @@ -136,6 +143,7 @@ public function testConstructorPrefersServiceValidateIfBothPresent(): void self::assertSame('serviceValidate', $validationMethod->getValue($cas)); } + /** * Missing both serviceValidate and validate should throw. */ @@ -154,6 +162,7 @@ public function testConstructorThrowsIfNoValidationMethodConfigured(): void new CAS(['AuthId' => 'unit-cas'], $config); } + /** * Missing login should throw. */ @@ -172,6 +181,7 @@ public function testConstructorThrowsIfNoLoginConfigured(): void new CAS(['AuthId' => 'unit-cas'], $config); } + /** * Test that request without stateId results in a BadRequest-error */ @@ -330,9 +340,14 @@ public function finalStep(array &$state): void }); $result = $c->linkback($request); + /* + * @var mixed $result + * @phpstan-ignore method.alreadyNarrowedType + */ $this->assertInstanceOf(RunnableResponse::class, $result); } + /** * Provide both CAS configs: relative (casserver) and absolute (casserver_legacy). * @@ -358,9 +373,9 @@ public function testCasConfigAbsoluteXPathsReturnValues(string $sourceKey): void $authsources = Configuration::getConfig('authsources.php'); $config = $authsources->toArray(); - self::assertIsArray($config, 'authsources.php did not return expected $config array'); self::assertArrayHasKey($sourceKey, $config, "Missing source '$sourceKey' in authsources.php"); $sourceConfig = $config[$sourceKey]; + /** @var array $sourceConfig */ self::assertArrayHasKey('cas', $sourceConfig, "Missing 'cas' config for '$sourceKey'"); self::assertArrayHasKey('ldap', $sourceConfig, "Missing 'ldap' config for '$sourceKey'"); @@ -377,7 +392,7 @@ public function testCasConfigAbsoluteXPathsReturnValues(string $sourceKey): void $serviceResponse = ServiceResponse::fromXML($root); $message = $serviceResponse->getResponse(); self::assertInstanceOf( - AuthenticationSuccess::class, + \SimpleSAML\CAS\XML\AuthenticationSuccess::class, $message, 'Expected AuthenticationSuccess message', ); @@ -431,6 +446,7 @@ public function testCasConfigAbsoluteXPathsReturnValues(string $sourceKey): void self::assertSame('jdoe@example.edu', array_pop($attributes['eduPersonPrincipalName']) ?? '', "$sourceKey: ePPN not extracted",); } + /** * Ensure that for casserver attributes configuration and the slate CAS response, * the attributes built from the AuthenticationSuccess model match @@ -443,13 +459,14 @@ public function testCasserverAutoMapAttributesMatchBetweenModelAndXPath(): void $authsources = Configuration::getConfig('authsources.php'); $config = $authsources->toArray(); - self::assertIsArray($config, 'authsources.php did not return expected $config array'); self::assertArrayHasKey( 'casserver_auto_map', $config, "Missing source 'casserver_auto_map' in authsources.php", ); $sourceConfig = $config['casserver_auto_map']; + /** @var array $sourceConfig */ + self::assertArrayHasKey('cas', $sourceConfig, "Missing 'cas' config for 'casserver_auto_map'"); self::assertArrayHasKey('ldap', $sourceConfig, "Missing 'ldap' config for 'casserver_auto_map'"); @@ -463,20 +480,23 @@ public function testCasserverAutoMapAttributesMatchBetweenModelAndXPath(): void self::fail('Loaded slate XML does not have a document element'); } - // Build AuthenticationSuccess message from XML - $serviceResponse = ServiceResponse::fromXML($root); + $isSlateEnabled = $sourceConfig['cas']['slate.enabled'] ?? false; + // Build AuthenticationSuccess message from XML. + // With xml-cas-module-slate installed, this will be a SlateAuthenticationSuccess instance. + $serviceResponse = $isSlateEnabled ? SlateServiceResponse::fromXML($root) : CasServiceResponse::fromXML($root); + $message = $serviceResponse->getResponse(); self::assertInstanceOf( - AuthenticationSuccess::class, + $isSlateEnabled ? SlateAuthenticationSuccess::class : AuthenticationSuccess::class, $message, - 'Expected AuthenticationSuccess message for slate XML', + 'Expected SlateAuthenticationSuccess message for slate XML', ); // Instantiate the CAS source with casserver_auto_map configuration - $cas = new Cas(['AuthId' => 'unit-cas'], $sourceConfig); + $cas = new CAS(['AuthId' => 'unit-cas'], $sourceConfig); // Use reflection to access the private parsers - $ref = new \ReflectionClass(Cas::class); + $ref = new \ReflectionClass(CAS::class); $parseAuthSuccess = $ref->getMethod('parseAuthenticationSuccess'); $parseAuthSuccess->setAccessible(true); @@ -490,12 +510,9 @@ public function testCasserverAutoMapAttributesMatchBetweenModelAndXPath(): void [$user, $modelAttrs] = $userAndModelAttrs; - // I have to add this here because the query attributes has it. - // I have to add this here because the query attributes has it. self::assertInstanceOf(ValueTypeInterface::class, $user); $modelAttrs['user'] = [$user->getValue()]; - // Assert same keys $modelKeys = array_keys($modelAttrs); $xpathKeys = array_keys($xpathAttrs); From 38478aa601d274c3bd0ee82dd0ad036db2c063f5 Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Mon, 15 Dec 2025 17:44:08 +0100 Subject: [PATCH 22/27] Fix failing PHP-linters --- .github/workflows/php.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index 5d8f11f..39303b3 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -21,7 +21,7 @@ jobs: matrix: php-version: ['8.2', '8.3', '8.4', '8.5'] - uses: simplesamlphp/simplesamlphp-test-framework/.github/workflows/reusable_phplinter.yml@v1.10.3 + uses: simplesamlphp/simplesamlphp-test-framework/.github/workflows/reusable_phplinter.yml@v1.10.6 with: php-version: ${{ matrix.php-version }} @@ -30,7 +30,7 @@ jobs: strategy: fail-fast: false - uses: simplesamlphp/simplesamlphp-test-framework/.github/workflows/reusable_linter.yml@v1.10.3 + uses: simplesamlphp/simplesamlphp-test-framework/.github/workflows/reusable_linter.yml@v1.10.6 with: enable_eslinter: false enable_jsonlinter: true From 8477cf94708dc2c0a70f8920386ad7148725d482 Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Mon, 15 Dec 2025 17:45:41 +0100 Subject: [PATCH 23/27] Fix markdown --- docs/cas.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/cas.md b/docs/cas.md index 54adb32..fdb280f 100644 --- a/docs/cas.md +++ b/docs/cas.md @@ -51,13 +51,12 @@ Or you might have to call serviceValidate for Protocol 3 via **/p3/**: ] ``` - ### Optional: Enabling Slate extensions Some deployments include vendor‑specific fields (for example `slate:*`) in CAS responses. You can opt in to Slate support: -```php +```php 'cas' => [ // ... 'serviceValidate' => 'https://cas.example.com/p3/serviceValidate', From c70caf6cb019dfff4f44cda5d9b6f4ed57a3c87e Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Mon, 15 Dec 2025 18:53:27 +0200 Subject: [PATCH 24/27] move linter workflow to 1.10.6 --- .github/workflows/php.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index 5d8f11f..13f5e75 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -21,7 +21,7 @@ jobs: matrix: php-version: ['8.2', '8.3', '8.4', '8.5'] - uses: simplesamlphp/simplesamlphp-test-framework/.github/workflows/reusable_phplinter.yml@v1.10.3 + uses: simplesamlphp/simplesamlphp-test-framework/.github/workflows/reusable_phplinter.yml@v1.10.6 with: php-version: ${{ matrix.php-version }} From c6c42d223de9106a197d9ce64ada910ae63c77e8 Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Mon, 15 Dec 2025 20:07:09 +0200 Subject: [PATCH 25/27] Increase phpstan src level --- phpstan.neon | 2 +- src/Auth/Source/CAS.php | 60 +++++++++++++++++++++++++++++------------ 2 files changed, 44 insertions(+), 18 deletions(-) diff --git a/phpstan.neon b/phpstan.neon index db37782..a7c64bd 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,4 +1,4 @@ parameters: - level: 6 + level: 8 paths: - src diff --git a/src/Auth/Source/CAS.php b/src/Auth/Source/CAS.php index 4c9f596..27ea3f4 100644 --- a/src/Auth/Source/CAS.php +++ b/src/Auth/Source/CAS.php @@ -50,12 +50,12 @@ class CAS extends Auth\Source /** - * @var array with ldap configuration + * @var array with ldap configuration */ private array $ldapConfig; /** - * @var array cas configuration + * @var array cas configuration */ private array $casConfig; @@ -75,6 +75,12 @@ class CAS extends Auth\Source */ private bool $useSlate; + /** + * HTTP utility class for making requests and handling redirects. + * @var \SimpleSAML\Utils\HTTP + */ + private Utils\HTTP $httpUtils; + /** * Constructor for this authentication source. @@ -89,8 +95,8 @@ public function __construct(array $info, array $config) $authsources = Configuration::loadFromArray($config); - $this->casConfig = $authsources->getValue('cas'); - $this->ldapConfig = $authsources->getValue('ldap'); + $this->casConfig = (array)$authsources->getValue('cas'); + $this->ldapConfig = (array)$authsources->getValue('ldap'); if (isset($this->casConfig['serviceValidate'])) { $this->validationMethod = 'serviceValidate'; @@ -110,6 +116,22 @@ public function __construct(array $info, array $config) } + /** + * Initialize HTTP utilities instance + * + * @param \SimpleSAML\Utils\HTTP|null $httpUtils Optional HTTP utilities instance to use + * @return void + */ + protected function initHttpUtils(Utils\HTTP $httpUtils = null): void + { + if ($httpUtils !== null) { + $this->httpUtils = $httpUtils; + } else { + $this->httpUtils = $this->httpUtils ?? new Utils\HTTP(); + } + } + + /** * This the most simple version of validating, this provides only authentication validation * @@ -120,17 +142,17 @@ public function __construct(array $info, array $config) */ private function casValidate(string $ticket, string $service): array { - $httpUtils = new Utils\HTTP(); - $url = $httpUtils->addURLParameters($this->casConfig['validate'], [ + $this->initHttpUtils(); + $url = $this->httpUtils->addURLParameters($this->casConfig['validate'], [ 'ticket' => $ticket, 'service' => $service, ]); /** @var string $result */ - $result = $httpUtils->fetch($url); + $result = $this->httpUtils->fetch($url); - /** @var list}|string> $res */ - $res = preg_split("/\r?\n/", $result); + /** @var list $res */ + $res = preg_split("/\r?\n/", $result) ?: []; if (strcmp($res[0], "yes") == 0) { return [$res[1], []]; @@ -150,19 +172,23 @@ private function casValidate(string $ticket, string $service): array */ private function casServiceValidate(string $ticket, string $service): array { - $httpUtils = new Utils\HTTP(); - $url = $httpUtils->addURLParameters( + $this->initHttpUtils(); + $url = $this->httpUtils->addURLParameters( $this->casConfig['serviceValidate'], [ 'ticket' => $ticket, 'service' => $service, ], ); - $result = $httpUtils->fetch($url); + $result = $this->httpUtils->fetch($url); /** @var string $result */ $dom = DOMDocumentFactory::fromString($result); + if ($dom->documentElement === null) { + return []; + } + if ($this->useSlate) { $serviceResponse = SlateServiceResponse::fromXML($dom->documentElement); } else { @@ -272,8 +298,8 @@ public function authenticate(array &$state): void $serviceUrl = Module::getModuleURL('cas/linkback.php', ['stateId' => $stateId]); - $httpUtils = new Utils\HTTP(); - $httpUtils->redirectTrustedURL($this->loginMethod, ['service' => $serviceUrl]); + $this->initHttpUtils(); + $this->httpUtils->redirectTrustedURL($this->loginMethod, ['service' => $serviceUrl]); } @@ -297,8 +323,8 @@ public function logout(array &$state): void Auth\State::deleteState($state); // we want cas to log us out - $httpUtils = new Utils\HTTP(); - $httpUtils->redirectTrustedURL($logoutUrl); + $this->initHttpUtils(); + $this->httpUtils->redirectTrustedURL($logoutUrl); } @@ -457,7 +483,7 @@ private function parseQueryAttributes(DOMDocument $dom): array } } else { // Relative XPath; prefer evaluating under authenticationSuccess if available - $context = $authn instanceof \DOMElement ? $authn : $root; + $context = $authn instanceof DOMElement ? $authn : $root; $nodes = XPath::xpQuery($context, $query, $xPath); } From dd29ff13cec24249794e86277ac5130974e02320 Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Mon, 15 Dec 2025 20:19:09 +0200 Subject: [PATCH 26/27] In PHP 8.4, if a parameter has a default value of `null`, its type must be explicitly nullable --- src/Auth/Source/CAS.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Auth/Source/CAS.php b/src/Auth/Source/CAS.php index 27ea3f4..40c43b3 100644 --- a/src/Auth/Source/CAS.php +++ b/src/Auth/Source/CAS.php @@ -122,7 +122,7 @@ public function __construct(array $info, array $config) * @param \SimpleSAML\Utils\HTTP|null $httpUtils Optional HTTP utilities instance to use * @return void */ - protected function initHttpUtils(Utils\HTTP $httpUtils = null): void + protected function initHttpUtils(?Utils\HTTP $httpUtils = null): void { if ($httpUtils !== null) { $this->httpUtils = $httpUtils; From afaa39e9b9f59f1693bd60be99df9d6051201a1e Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Tue, 16 Dec 2025 13:41:55 +0200 Subject: [PATCH 27/27] Replace legacey HTTP/Utils fetch with Symfony HTTP Client --- composer.json | 6 ++-- src/Auth/Source/CAS.php | 64 ++++++++++++++++++++++++++++++----------- 2 files changed, 52 insertions(+), 18 deletions(-) diff --git a/composer.json b/composer.json index 4fe9061..75ecb26 100644 --- a/composer.json +++ b/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", diff --git a/src/Auth/Source/CAS.php b/src/Auth/Source/CAS.php index 40c43b3..7477246 100644 --- a/src/Auth/Source/CAS.php +++ b/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 $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