diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index fcc5212..39303b3 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.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.9.2 + uses: simplesamlphp/simplesamlphp-test-framework/.github/workflows/reusable_linter.yml@v1.10.6 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 @@ -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 @@ -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 @@ -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, zip + 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 @@ -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 b9817c8..75ecb26 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 } }, @@ -34,17 +34,47 @@ } }, "require": { - "php": "^8.1", - "simplesamlphp/composer-module-installer": "^1.3.4", - "simplesamlphp/simplesamlphp": "~2.4.0", + "php": "^8.2", + "ext-pcre": "*", + "ext-dom": "*", + + "simplesamlphp/assert": "^1.9", + "simplesamlphp/composer-module-installer": "^1.4", + "simplesamlphp/simplesamlphp": "dev-simplesamlphp-2.5 as v2.5.x-dev", "simplesamlphp/simplesamlphp-module-ldap": "~1.2", - "symfony/http-foundation": "^6.4" + "simplesamlphp/xml-cas-module-slate": "~1.1.0", + "simplesamlphp/xml-cas": "^v2.2.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.9.2" + "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", "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/docs/cas.md b/docs/cas.md index 266a188..fdb280f 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,35 @@ 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 @@ -76,10 +105,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,11 +116,11 @@ 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: -set `ldap` to a `null`: +set `ldap` to `null`: ```php 'example-cas' => [ 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..a7c64bd 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,4 +1,4 @@ parameters: - level: 7 + level: 8 paths: - src 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 30ad6b6..7477246 100644 --- a/src/Auth/Source/CAS.php +++ b/src/Auth/Source/CAS.php @@ -4,20 +4,30 @@ namespace SimpleSAML\Module\cas\Auth\Source; -use DOMXpath; +use DOMDocument; +use DOMElement; use Exception; -use SAML2\DOMDocumentFactory; use SimpleSAML\Auth; +use SimpleSAML\CAS\Utils\XPath; +use SimpleSAML\CAS\XML\AuthenticationFailure; +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 Symfony\Component\HttpClient\HttpClient; +use Symfony\Contracts\HttpClient\HttpClientInterface; -use function array_key_exists; use function array_merge_recursive; -use function is_null; use function preg_split; use function strcmp; +use function strval; use function var_export; /** @@ -40,13 +50,14 @@ class CAS extends Auth\Source */ public const AUTHID = '\SimpleSAML\Module\cas\Auth\Source\CAS.AuthId'; + /** - * @var array with ldap configuration + * @var array with ldap configuration */ private array $ldapConfig; /** - * @var array cas configuration + * @var array cas configuration */ private array $casConfig; @@ -61,6 +72,22 @@ class CAS extends Auth\Source */ private string $loginMethod; + /** + * @var bool flag indicating if slate XML format should be used + */ + private bool $useSlate; + + /** + * 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. * @@ -72,16 +99,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]'); - } + $authsources = Configuration::loadFromArray($config); - if (!array_key_exists('ldap', $config)) { - throw new Exception('ldap authentication source is not properly configured: missing [ldap]'); - } - - $this->casConfig = $config['cas']; - $this->ldapConfig = $config['ldap']; + $this->casConfig = (array)$authsources->getValue('cas'); + $this->ldapConfig = (array)$authsources->getValue('ldap'); if (isset($this->casConfig['serviceValidate'])) { $this->validationMethod = 'serviceValidate'; @@ -96,6 +117,43 @@ public function __construct(array $info, array $config) } else { throw new Exception("cas login URL not specified"); } + + $this->useSlate = $this->casConfig['slate.enabled'] ?? false; + } + + + /** + * 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 + { + if ($httpUtils !== null) { + $this->httpUtils = $httpUtils; + } else { + $this->httpUtils = $this->httpUtils ?? new Utils\HTTP(); + } } @@ -109,17 +167,19 @@ 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'], [ - 'ticket' => $ticket, - 'service' => $service, + $this->initHttpClient(); + + $response = $this->httpClient->request('GET', $this->casConfig['validate'], [ + 'query' => [ + 'ticket' => $ticket, + 'service' => $service, + ], ]); - /** @var string $result */ - $result = $httpUtils->fetch($url); + $result = $response->getContent(); - /** @var 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], []]; @@ -139,51 +199,68 @@ private function casValidate(string $ticket, string $service): array */ private function casServiceValidate(string $ticket, string $service): array { - $httpUtils = new Utils\HTTP(); - $url = $httpUtils->addURLParameters( - $this->casConfig['serviceValidate'], - [ - 'ticket' => $ticket, + $this->initHttpClient(); + + $response = $this->httpClient->request('GET', $this->casConfig['serviceValidate'], [ + 'query' => [ + 'ticket' => $ticket, 'service' => $service, ], - ); - $result = $httpUtils->fetch($url); + ]); + + $result = $response->getContent(); /** @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); + // 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 []; + } + + if ($this->useSlate) { + $serviceResponse = SlateServiceResponse::fromXML($dom->documentElement); } else { - $attributes = []; - if ($casattributes = $this->casConfig['attributes']) { - // Some has 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); - foreach ($attrs as $attrvalue) { - $attributes[$name][] = $attrvalue->textContent; - } - } - } + $serviceResponse = CasServiceResponse::fromXML($dom->documentElement); + } - $item = $success->item(0); - if (is_null($item)) { - throw new Exception("Error parsing serviceResponse."); + $message = $serviceResponse->getResponse(); + if ($message instanceof AuthenticationFailure) { + throw new Exception(sprintf( + "Error when validating CAS service ticket: %s (%s)", + strval($message->getContent()), + strval($message->getCode()), + )); + } 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 + // array is empty or not set then an empty array will be returned. + $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) { + // 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; + } } - $casusername = $item->textContent; - return [$casusername, $attributes]; + return [$user, $attributes]; } + + throw new Exception("Error parsing serviceResponse."); } /** - * Main validation method, redirects to correct method + * Main validation method, redirects to the correct method * (keeps finalStep clean) * * @param string $ticket @@ -212,8 +289,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 +305,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; } @@ -252,8 +330,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]); } @@ -277,7 +355,181 @@ 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); + } + + + /** + * 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\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(CasAuthnSuccess|SlateAuthnSuccess $message): array + { + /** @var array> $result */ + $result = []; + + // user -> content + $user = $message->getUser()->getContent(); + + // 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) { + continue; // skip malformed entries + } + + // Key selection rule + $key = ($prefix === '' || $prefix === 'cas') + ? $localName + : ($prefix . ':' . $localName); + + $value = trim($xmlElement->textContent ?? ''); + + // Collect values (single or multi) + $result[$key] ??= []; + $result[$key][] = $value; + } + + // (DOMElement instances under cas:authenticationSuccess, outside cas:attributes) + $this->parseAuthenticationSuccessMetadata($message, $result); + + return [$user, $result]; + } + + + /** + * Parse metadata elements from AuthenticationSuccess message and add them to attributes array + * + * @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( + 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 Chunk) { + continue; + } + + $localName = $element->getLocalName(); + $prefix = $element->getPrefix(); + + if ($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->getXML()->textContent ?? ''); + + $attributes[$key] ??= []; + $attributes[$key][] = $value; + } + } + + + /** + * 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, true); + + $metadata = []; + $casattributes = $this->casConfig['attributes'] ?? null; + if (!is_array($casattributes)) { + 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) { + $marker = 'cas:authenticationSuccess/'; + + if (isset($query[0]) && $query[0] === '/') { + // Absolute XPath + if (strpos($query, $marker) !== false && $authn instanceof \DOMElement) { + $originalQuery = $query; + $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); + } + + foreach ($nodes as $n) { + $metadata[$name][] = trim($n->textContent); + } + + Logger::debug(sprintf( + 'CAS client: parsed metadata %s => %s', + $name, + json_encode($metadata[$name] ?? []), + )); + } + + return $metadata; } } diff --git a/src/Controller/CAS.php b/src/Controller/CAS.php index d3bac27..e52af83 100644 --- a/src/Controller/CAS.php +++ b/src/Controller/CAS.php @@ -81,10 +81,11 @@ 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.'); + throw new Error\BadRequest('Missing stateId parameter.'); } $stateId = $request->query->getString('stateId'); + /** @var array $state */ $state = $this->authState::loadState($stateId, CASSource::STAGE_INIT); if (!$request->query->has('ticket')) { 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' => [ + '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: + 'person_top' => 'slate:person', + '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' => [ + '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: + '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', + // phpcs:ignore Generic.Files.LineLength.TooLong + 'eduPersonPrincipalName' => '/cas:serviceResponse/cas:authenticationSuccess/cas:attributes/cas:eduPersonPrincipalName', + ], + ], + 'ldap' => [], + ], + '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', + '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..0626647 --- /dev/null +++ b/tests/response/cas-success-service-response-slate.xml @@ -0,0 +1,15 @@ + + + + example-user@technolutions.com + 345d2e1b-65de-419c-96ce-e1866d4c57cd + Regular Decision + 774482874 + + Example + User + example-user@technolutions.com + + + \ 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..797e678 --- /dev/null +++ b/tests/response/cas-success-service-response.xml @@ -0,0 +1,20 @@ + + + 12345_top + 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 65b4430..c5eb80e 100644 --- a/tests/src/Controller/CASTest.php +++ b/tests/src/Controller/CASTest.php @@ -5,14 +5,22 @@ namespace SimpleSAML\Test\Module\cas\Controller; use Exception; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; 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 Symfony\Component\HttpFoundation\{Request, Response}; +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; /** * Set of tests for the controllers in the "cas" module. @@ -35,6 +43,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' => [ @@ -47,22 +60,130 @@ 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'); } /** - * Test that request without StateId results in a BadRequest-error + * 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 */ public function testNoStateId(): void { @@ -74,8 +195,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); } @@ -88,7 +212,7 @@ public function testNoState(): void $request = Request::create( '/linkback', 'GET', - ['StateId' => 'abc123'], + ['stateId' => 'abc123'], ); $c = new Controller\CAS($this->config); @@ -107,20 +231,24 @@ public function testNoTicket(): void $request = Request::create( '/linkback', 'GET', - ['StateId' => 'abc123'], + ['stateId' => 'abc123'], ); $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 + /** @return array */ + public static function loadState(string $id, string $stage, bool $allowMissing = false): array { return []; } }); $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); } @@ -135,15 +263,15 @@ public function testUnknownAuthSource(): void '/linkback', 'GET', [ - 'StateId' => 'abc123', + 'stateId' => 'abc123', 'ticket' => 'abc123', ], ); $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 + /** @return array */ + public static function loadState(string $id, string $stage, bool $allowMissing = false): array { return [CAS::AUTHID => 'somethingElse']; } @@ -164,15 +292,15 @@ public function testNormalOperation(): void '/linkback', 'GET', [ - 'StateId' => 'abc123', + 'stateId' => 'abc123', 'ticket' => 'abc123', ], ); $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 + /** @return array */ + public static function loadState(string $id, string $stage, bool $allowMissing = false): array { return [CAS::AUTHID => 'something']; } @@ -183,6 +311,7 @@ public function __construct() //dummy } + /** * @param array $state */ @@ -191,7 +320,8 @@ public function authenticate(array &$state): void //dummy } - 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() @@ -199,6 +329,7 @@ public function __construct() //dummy } + /** @param array $state */ public function finalStep(array &$state): void { @@ -209,6 +340,193 @@ 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). + * + * @return array + */ + public static function casConfigsProvider(): array + { + return [ + "casserver short attribute mapping" => ['casserver'], + "casserver legacy/long attribute mapping" => ['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::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'"); + + // 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( + \SimpleSAML\CAS\XML\AuthenticationSuccess::class, + $message, + 'Expected AuthenticationSuccess message', + ); + + // Instantiate the CAS source with the selected configuration + $cas = new Cas(['AuthId' => 'unit-cas'], $sourceConfig); + + // 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 + self::assertSame('jdoe', $user, "$sourceKey: user mismatch"); + //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::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'"); + + // 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'); + } + + $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( + $isSlateEnabled ? SlateAuthenticationSuccess::class : AuthenticationSuccess::class, + $message, + 'Expected SlateAuthenticationSuccess 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); + + [$user, $modelAttrs] = $userAndModelAttrs; + + self::assertInstanceOf(ValueTypeInterface::class, $user); + $modelAttrs['user'] = [$user->getValue()]; + + // Assert same keys + $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", + ); + } + } } 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" ] }