From 494f4204d6133a0a208960ece17f5829fddabdf0 Mon Sep 17 00:00:00 2001
From: Tim van Dijen <tvdijen@gmail.com>
Date: Wed, 19 Feb 2020 22:01:34 +0100
Subject: [PATCH] Fix a handful of Psalm-issues
---
lib/SimpleSAML/Configuration.php | 7 ++++---
modules/saml/lib/IdP/SAML2.php | 1 +
modules/saml/lib/SP/LogoutStore.php | 1 +
modules/saml/www/sp/saml2-acs.php | 1 +
modules/saml/www/sp/saml2-logout.php | 4 ++--
psalm.xml | 10 +++++++---
6 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/lib/SimpleSAML/Configuration.php b/lib/SimpleSAML/Configuration.php
index be7ad8955..0a3639738 100644
--- a/lib/SimpleSAML/Configuration.php
+++ b/lib/SimpleSAML/Configuration.php
@@ -22,7 +22,7 @@ class Configuration implements Utils\ClearableState
*
* @var string
*/
- const REQUIRED_OPTION = '___REQUIRED_OPTION___';
+ public const REQUIRED_OPTION = '___REQUIRED_OPTION___';
/**
* Associative array with mappings from instance-names to configuration objects.
@@ -110,6 +110,7 @@ class Configuration implements Utils\ClearableState
}
if (file_exists($filename)) {
+ /** @psalm-var mixed $config */
$config = 'UNINITIALIZED';
// the file initializes a variable named '$config'
@@ -1019,7 +1020,7 @@ class Configuration implements Utils\ClearableState
* @param mixed $default The default value to return if no matching endpoint is found. If no default is provided,
* an exception will be thrown.
*
- * @return mixed|null The default endpoint, or null if no acceptable endpoints are used.
+ * @return mixed|null The default endpoint.
*
* @throws \Exception If no supported endpoint is found.
*/
@@ -1027,7 +1028,7 @@ class Configuration implements Utils\ClearableState
string $endpointType,
array $bindings,
$default = self::REQUIRED_OPTION
- ): ?array {
+ ): array {
$endpoints = $this->getEndpoints($endpointType);
foreach ($bindings as $binding) {
diff --git a/modules/saml/lib/IdP/SAML2.php b/modules/saml/lib/IdP/SAML2.php
index 89b035128..613c9e707 100644
--- a/modules/saml/lib/IdP/SAML2.php
+++ b/modules/saml/lib/IdP/SAML2.php
@@ -1035,6 +1035,7 @@ class SAML2
case 'raw':
if (is_string($value)) {
$doc = DOMDocumentFactory::fromString('<root>' . $value . '</root>');
+ /** @psalm-suppress PossiblyNullPropertyFetch */
$value = $doc->firstChild->childNodes;
}
Assert::isInstanceOfAny($value, [\DOMNodeList::class, \SAML2\XML\saml\NameID::class]);
diff --git a/modules/saml/lib/SP/LogoutStore.php b/modules/saml/lib/SP/LogoutStore.php
index a229d43bd..96114e344 100644
--- a/modules/saml/lib/SP/LogoutStore.php
+++ b/modules/saml/lib/SP/LogoutStore.php
@@ -282,6 +282,7 @@ class LogoutStore
$res[$row['_sessionindex']] = $row['_sessionid'];
}
+ /** @var array $res */
return $res;
}
diff --git a/modules/saml/www/sp/saml2-acs.php b/modules/saml/www/sp/saml2-acs.php
index 1f7e60313..1ca32b5c3 100644
--- a/modules/saml/www/sp/saml2-acs.php
+++ b/modules/saml/www/sp/saml2-acs.php
@@ -139,6 +139,7 @@ try {
// the status of the response wasn't "success"
$e = $e->toException();
\SimpleSAML\Auth\State::throwException($state, $e);
+ return;
}
$authenticatingAuthority = null;
diff --git a/modules/saml/www/sp/saml2-logout.php b/modules/saml/www/sp/saml2-logout.php
index 76661b3eb..aaf269e04 100644
--- a/modules/saml/www/sp/saml2-logout.php
+++ b/modules/saml/www/sp/saml2-logout.php
@@ -125,7 +125,6 @@ if ($message instanceof \SAML2\LogoutResponse) {
\SimpleSAML\Logger::warning('Logged out of ' . $numLoggedOut . ' of ' . count($sessionIndexes) . ' sessions.');
}
- /** @var array $dst */
$dst = $idpMetadata->getEndpointPrioritizedByBinding(
'SingleLogoutService',
[
@@ -142,8 +141,9 @@ if ($message instanceof \SAML2\LogoutResponse) {
$dst = $dst['Location'];
}
$binding->setDestination($dst);
+ } else {
+ $lr->setDestination($dst['Location']);
}
- $lr->setDestination($dst);
$binding->send($lr);
} else {
diff --git a/psalm.xml b/psalm.xml
index 065e7d06b..b3e2b2026 100644
--- a/psalm.xml
+++ b/psalm.xml
@@ -9,9 +9,6 @@
<projectFiles>
<directory name="lib/SimpleSAML" />
- <!-- Replaces all modules/... with this one-liner for 2.0
- <directory name="modules" />
- -->
<directory name="modules/admin" />
<directory name="modules/core" />
<directory name="modules/cron" />
@@ -78,6 +75,13 @@
<directory name="tests" />
</errorLevel>
</PropertyNotSetInConstructor>
+
+ <!-- Suppress PHPunit-issue -->
+ <InternalMethod>
+ <errorLevel type="suppress">
+ <directory name="tests" />
+ </errorLevel>
+ </InternalMethod>
</issueHandlers>
<stubs>
--
GitLab