diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index c8be83c..4eed08c 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -162,9 +162,8 @@ jobs: with: # Should be the higest supported version, so we can use the newest tools php-version: '8.4' - tools: composer, composer-require-checker, composer-unused, phpcs, psalm - # optional performance gain for psalm: opcache - extensions: ctype, date, dom, fileinfo, filter, hash, intl, mbstring, opcache, openssl, pcre, spl, xml + tools: composer, composer-require-checker, composer-unused, phpcs + extensions: ctype, date, dom, fileinfo, filter, hash, intl, mbstring, openssl, pcre, spl, xml - name: Setup problem matchers for PHP run: echo "::add-matcher::${{ runner.tool_cache }}/php.json" @@ -196,27 +195,13 @@ jobs: - name: PHP Code Sniffer run: phpcs - - name: Psalm - continue-on-error: true - run: | - psalm -c psalm.xml \ - --show-info=true \ - --shepherd \ - --php-version=${{ steps.setup-php.outputs.php-version }} - - - name: Psalm (testsuite) + - name: PHPStan run: | - psalm -c psalm-dev.xml \ - --show-info=true \ - --shepherd \ - --php-version=${{ steps.setup-php.outputs.php-version }} + vendor/bin/phpstan analyze -c phpstan.neon - - name: Psalter + - name: PHPStan (testsuite) run: | - psalm --alter \ - --issues=UnnecessaryVarAnnotation \ - --dry-run \ - --php-version=${{ steps.setup-php.outputs.php-version }} + vendor/bin/phpstan analyze -c phpstan-dev.neon security: name: Security checks diff --git a/composer.json b/composer.json index 7675981..b9817c8 100644 --- a/composer.json +++ b/composer.json @@ -19,7 +19,8 @@ "composer/package-versions-deprecated": true, "simplesamlphp/composer-module-installer": true, "dealerdirect/phpcodesniffer-composer-installer": true, - "phpstan/extension-installer": true + "phpstan/extension-installer": true, + "simplesamlphp/composer-xmlprovider-installer": true } }, "autoload": { @@ -35,7 +36,7 @@ "require": { "php": "^8.1", "simplesamlphp/composer-module-installer": "^1.3.4", - "simplesamlphp/simplesamlphp": "^3@dev", + "simplesamlphp/simplesamlphp": "~2.4.0", "simplesamlphp/simplesamlphp-module-ldap": "~1.2", "symfony/http-foundation": "^6.4" }, diff --git a/phpstan-dev.neon b/phpstan-dev.neon new file mode 100644 index 0000000..116dfc6 --- /dev/null +++ b/phpstan-dev.neon @@ -0,0 +1,4 @@ +parameters: + level: 8 + paths: + - tests diff --git a/phpstan.neon b/phpstan.neon new file mode 100644 index 0000000..d5341b1 --- /dev/null +++ b/phpstan.neon @@ -0,0 +1,4 @@ +parameters: + level: 7 + paths: + - src diff --git a/psalm-dev.xml b/psalm-dev.xml deleted file mode 100644 index 6116331..0000000 --- a/psalm-dev.xml +++ /dev/null @@ -1,27 +0,0 @@ - - - - - - - - - - - - - - - - - - - - diff --git a/psalm.xml b/psalm.xml deleted file mode 100644 index 9feb67a..0000000 --- a/psalm.xml +++ /dev/null @@ -1,30 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/src/Auth/Source/CAS.php b/src/Auth/Source/CAS.php index af55991..30ad6b6 100644 --- a/src/Auth/Source/CAS.php +++ b/src/Auth/Source/CAS.php @@ -41,12 +41,12 @@ 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; @@ -64,8 +64,8 @@ class CAS extends Auth\Source /** * Constructor for this authentication source. * - * @param array $info Information about this authentication source. - * @param array $config Configuration. + * @param array $info Information about this authentication source. + * @param array $config Configuration. */ public function __construct(array $info, array $config) { @@ -105,7 +105,7 @@ public function __construct(array $info, array $config) * @param string $ticket * @param string $service * - * @return array username and attributes + * @return array username and attributes */ private function casValidate(string $ticket, string $service): array { @@ -114,9 +114,11 @@ private function casValidate(string $ticket, string $service): array 'ticket' => $ticket, 'service' => $service, ]); - $result = $httpUtils->fetch($url); /** @var string $result */ + $result = $httpUtils->fetch($url); + + /** @var string $res */ $res = preg_split("/\r?\n/", $result); if (strcmp($res[0], "yes") == 0) { @@ -133,7 +135,7 @@ private function casValidate(string $ticket, string $service): array * @param string $ticket * @param string $service * - * @return array username and attributes + * @return array username and attributes */ private function casServiceValidate(string $ticket, string $service): array { @@ -151,8 +153,9 @@ private function casServiceValidate(string $ticket, string $service): array $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->length == 0) { + 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 { @@ -160,6 +163,7 @@ private function casServiceValidate(string $ticket, string $service): array 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; @@ -184,7 +188,7 @@ private function casServiceValidate(string $ticket, string $service): array * * @param string $ticket * @param string $service - * @return array username and attributes + * @return array username and attributes */ protected function casValidation(string $ticket, string $service): array { @@ -201,7 +205,7 @@ protected function casValidation(string $ticket, string $service): array /** * Called by linkback, to finish validate/ finish logging in. - * @param array $state + * @param array $state */ public function finalStep(array &$state): void { @@ -237,7 +241,7 @@ public function finalStep(array &$state): void /** * Log-in using cas * - * @param array &$state Information about the current authentication. + * @param array &$state Information about the current authentication. */ public function authenticate(array &$state): void { @@ -264,7 +268,7 @@ public function authenticate(array &$state): void * should be called with the state. If this operation can be completed without * showing the user a page, or redirecting, this function should return. * - * @param array &$state Information about the current logout operation. + * @param array &$state Information about the current logout operation. */ public function logout(array &$state): void { diff --git a/src/Controller/CAS.php b/src/Controller/CAS.php index b949fe3..d3bac27 100644 --- a/src/Controller/CAS.php +++ b/src/Controller/CAS.php @@ -84,14 +84,14 @@ public function linkback(Request $request): RunnableResponse throw new Error\BadRequest('Missing StateId parameter.'); } - $stateId = $request->query->get('stateId'); + $stateId = $request->query->getString('stateId'); $state = $this->authState::loadState($stateId, CASSource::STAGE_INIT); if (!$request->query->has('ticket')) { throw new Error\BadRequest('Missing ticket parameter.'); } - $ticket = $request->query->get('ticket'); + $ticket = $request->query->getString('ticket'); $state['cas:ticket'] = $ticket; // Find authentication source diff --git a/tests/src/Controller/CASTest.php b/tests/src/Controller/CASTest.php index f3856e8..65b4430 100644 --- a/tests/src/Controller/CASTest.php +++ b/tests/src/Controller/CASTest.php @@ -19,7 +19,7 @@ * * @package SimpleSAML\Test */ -class CASTest extends TestCase +final class CASTest extends TestCase { /** @var \SimpleSAML\Configuration */ protected Configuration $config; @@ -112,6 +112,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 { return []; @@ -141,6 +142,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 { return [CAS::AUTHID => 'somethingElse']; @@ -169,6 +171,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 { return [CAS::AUTHID => 'something']; @@ -180,7 +183,10 @@ public function __construct() //dummy } - public function authenticate(Request $request, array &$state): Response + /** + * @param array $state + */ + public function authenticate(array &$state): void { //dummy } @@ -192,6 +198,8 @@ public function __construct() { //dummy } + + /** @param array $state */ public function finalStep(array &$state): void { //dummy