From 06c80dd91136ef41c3a913f46d92cfa1c3ac92eb Mon Sep 17 00:00:00 2001
From: Tim van Dijen <tim.dijen@minbzk.nl>
Date: Mon, 8 Jul 2019 21:25:13 +0200
Subject: [PATCH] Fix Psalm-issues batch #1 (#1157)
---
composer.json | 2 +-
composer.lock | 31 ++++++++--------
lib/SimpleSAML/Auth/ProcessingChain.php | 4 ++-
lib/SimpleSAML/Auth/Source.php | 5 +++
lib/SimpleSAML/Auth/State.php | 1 +
lib/SimpleSAML/Bindings/Shib13/Artifact.php | 1 +
lib/SimpleSAML/Bindings/Shib13/HTTPPost.php | 8 ++---
lib/SimpleSAML/Configuration.php | 4 +--
.../Error/UnserializableException.php | 2 +-
lib/SimpleSAML/HTTP/Router.php | 25 ++++++++-----
lib/SimpleSAML/IdP/IFrameLogoutHandler.php | 2 +-
lib/SimpleSAML/Locale/Localization.php | 8 +++--
lib/SimpleSAML/Locale/Translate.php | 12 ++++---
.../MetaDataStorageHandlerFlatFile.php | 9 +++--
.../MetaDataStorageHandlerSerialize.php | 12 +++++--
.../Metadata/MetaDataStorageSource.php | 2 ++
lib/SimpleSAML/Metadata/SAMLParser.php | 28 +++++++--------
lib/SimpleSAML/Metadata/Signer.php | 1 +
lib/SimpleSAML/Metadata/Sources/MDQ.php | 11 ++++++
lib/SimpleSAML/Utils/EMail.php | 2 +-
lib/SimpleSAML/XHTML/Template.php | 36 +++++++++++--------
lib/SimpleSAML/XHTML/TemplateLoader.php | 6 ++++
lib/SimpleSAML/XML/Parser.php | 15 +++++---
lib/SimpleSAML/XML/Shib13/AuthnRequest.php | 6 ++--
lib/SimpleSAML/XML/Shib13/AuthnResponse.php | 10 ++++--
lib/SimpleSAML/XML/Signer.php | 9 +++--
lib/SimpleSAML/XML/Validator.php | 24 +++++++------
psalm.xml | 30 ++++++++++++++++
28 files changed, 203 insertions(+), 103 deletions(-)
diff --git a/composer.json b/composer.json
index 007e580d3..08f7cd886 100644
--- a/composer.json
+++ b/composer.json
@@ -43,7 +43,7 @@
"ext-json": "*",
"ext-mbstring": "*",
"gettext/gettext": "^4.6",
- "jaimeperez/twig-configurable-i18n": "^2.0",
+ "jaimeperez/twig-configurable-i18n": "^2.1",
"phpmailer/phpmailer": "^6.0",
"robrichards/xmlseclibs": "^3.0",
"simplesamlphp/saml2": "^3.3",
diff --git a/composer.lock b/composer.lock
index 01c6c2c08..7eb7a5cc6 100644
--- a/composer.lock
+++ b/composer.lock
@@ -4,7 +4,7 @@
"Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies",
"This file is @generated automatically"
],
- "content-hash": "b809e29b2bfe75e501cfdc5efe9a7c9e",
+ "content-hash": "11faaddab3dbdd175af8c5a20e8f5d0f",
"packages": [
{
"name": "gettext/gettext",
@@ -131,20 +131,24 @@
},
{
"name": "jaimeperez/twig-configurable-i18n",
- "version": "v2.0",
+ "version": "v2.1",
"source": {
"type": "git",
"url": "https://github.com/jaimeperez/twig-configurable-i18n.git",
- "reference": "4ccf150ba9f28d2e31d0622ecc9b81cdc6a2638b"
+ "reference": "38a22aaa6b31efdc0d76d58f5934dea3ebac8556"
},
"dist": {
"type": "zip",
- "url": "https://api.github.com/repos/jaimeperez/twig-configurable-i18n/zipball/4ccf150ba9f28d2e31d0622ecc9b81cdc6a2638b",
- "reference": "4ccf150ba9f28d2e31d0622ecc9b81cdc6a2638b",
+ "url": "https://api.github.com/repos/jaimeperez/twig-configurable-i18n/zipball/38a22aaa6b31efdc0d76d58f5934dea3ebac8556",
+ "reference": "38a22aaa6b31efdc0d76d58f5934dea3ebac8556",
"shasum": ""
},
"require": {
- "twig/extensions": "^1.3"
+ "twig/extensions": "^1.5"
+ },
+ "require-dev": {
+ "phpunit/phpunit": "~4.8.36",
+ "twig/twig": "^1.37 || ^2.7"
},
"type": "project",
"autoload": {
@@ -171,7 +175,7 @@
"translation",
"twig"
],
- "time": "2018-10-10T09:12:46+00:00"
+ "time": "2019-06-07T11:03:28+00:00"
},
{
"name": "paragonie/random_compat",
@@ -2714,16 +2718,16 @@
},
{
"name": "twig/twig",
- "version": "v1.42.0",
+ "version": "v1.42.2",
"source": {
"type": "git",
"url": "https://github.com/twigphp/Twig.git",
- "reference": "2983fcf2e20a4afe95f07e61d89a7590b3c8df2e"
+ "reference": "21707d6ebd05476854805e4f91b836531941bcd4"
},
"dist": {
"type": "zip",
- "url": "https://api.github.com/repos/twigphp/Twig/zipball/2983fcf2e20a4afe95f07e61d89a7590b3c8df2e",
- "reference": "2983fcf2e20a4afe95f07e61d89a7590b3c8df2e",
+ "url": "https://api.github.com/repos/twigphp/Twig/zipball/21707d6ebd05476854805e4f91b836531941bcd4",
+ "reference": "21707d6ebd05476854805e4f91b836531941bcd4",
"shasum": ""
},
"require": {
@@ -2776,7 +2780,7 @@
"keywords": [
"templating"
],
- "time": "2019-05-31T06:20:05+00:00"
+ "time": "2019-06-18T15:35:16+00:00"
},
{
"name": "webmozart/assert",
@@ -4153,8 +4157,5 @@
},
"platform-dev": {
"ext-curl": "*"
- },
- "platform-overrides": {
- "php": "5.6.40"
}
}
diff --git a/lib/SimpleSAML/Auth/ProcessingChain.php b/lib/SimpleSAML/Auth/ProcessingChain.php
index e07fc2534..3ff70888c 100644
--- a/lib/SimpleSAML/Auth/ProcessingChain.php
+++ b/lib/SimpleSAML/Auth/ProcessingChain.php
@@ -149,7 +149,7 @@ class ProcessingChain
* @param array $config Array with the authentication processing filter configuration.
* @param int $priority The priority of the current filter, (not included in the filter
* definition.)
- * @return ProcessingFilter The parsed filter.
+ * @return \SimpleSAML\Auth\ProcessingFilter The parsed filter.
*/
private static function parseFilter($config, $priority)
{
@@ -166,6 +166,8 @@ class ProcessingChain
);
$config['%priority'] = $priority;
unset($config['class']);
+
+ /** @var \SimpleSAML\Auth\ProcessingFilter */
return new $className($config, null);
}
diff --git a/lib/SimpleSAML/Auth/Source.php b/lib/SimpleSAML/Auth/Source.php
index 3327103d6..71469744c 100644
--- a/lib/SimpleSAML/Auth/Source.php
+++ b/lib/SimpleSAML/Auth/Source.php
@@ -126,6 +126,10 @@ abstract class Source
// the default implementation just copies over the previous authentication data
$session = Session::getSessionFromRequest();
$data = $session->getAuthState($this->authId);
+ if ($data === null) {
+ throw new Error\NoState();
+ }
+
foreach ($data as $k => $v) {
$state[$k] = $v;
}
@@ -333,6 +337,7 @@ abstract class Source
$authSource = new $className($info, $config);
}
+ /** @var \SimpleSAML\Auth\Source */
return $authSource;
}
diff --git a/lib/SimpleSAML/Auth/State.php b/lib/SimpleSAML/Auth/State.php
index 8e5030ddf..09daaa9a1 100644
--- a/lib/SimpleSAML/Auth/State.php
+++ b/lib/SimpleSAML/Auth/State.php
@@ -400,6 +400,7 @@ class State
$id = $_REQUEST[self::EXCEPTION_PARAM];
}
+ /** @var array $state */
$state = self::loadState($id, self::EXCEPTION_STAGE);
assert(array_key_exists(self::EXCEPTION_DATA, $state));
diff --git a/lib/SimpleSAML/Bindings/Shib13/Artifact.php b/lib/SimpleSAML/Bindings/Shib13/Artifact.php
index 702c59f82..839271f62 100644
--- a/lib/SimpleSAML/Bindings/Shib13/Artifact.php
+++ b/lib/SimpleSAML/Bindings/Shib13/Artifact.php
@@ -136,6 +136,7 @@ class Artifact
Utils\XML::debugSAMLMessage($request, 'out');
+ /** @var array $url */
$url = $idpMetadata->getDefaultEndpoint(
'ArtifactResolutionService',
['urn:oasis:names:tc:SAML:1.0:bindings:SOAP-binding']
diff --git a/lib/SimpleSAML/Bindings/Shib13/HTTPPost.php b/lib/SimpleSAML/Bindings/Shib13/HTTPPost.php
index 89c3640df..b98f34c3e 100644
--- a/lib/SimpleSAML/Bindings/Shib13/HTTPPost.php
+++ b/lib/SimpleSAML/Bindings/Shib13/HTTPPost.php
@@ -20,14 +20,14 @@ use SimpleSAML\XML\Signer;
class HTTPPost
{
/**
- * @var \SimpleSAML\Configuration|null
+ * @var \SimpleSAML\Configuration
*/
- private $configuration = null;
+ private $configuration;
/**
- * @var \SimpleSAML\Metadata\MetaDataStorageHandler|null
+ * @var \SimpleSAML\Metadata\MetaDataStorageHandler
*/
- private $metadata = null;
+ private $metadata;
/**
diff --git a/lib/SimpleSAML/Configuration.php b/lib/SimpleSAML/Configuration.php
index 95077aed9..dbc37ebc2 100644
--- a/lib/SimpleSAML/Configuration.php
+++ b/lib/SimpleSAML/Configuration.php
@@ -1243,9 +1243,9 @@ 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 array|null The default endpoint, or null if no acceptable endpoints are used.
+ * @return mixed The default endpoint, or the $default parameter if no acceptable endpoints are used.
*
- * @throws \Exception If no supported endpoint is found.
+ * @throws \Exception If no supported endpoint is found and no $default parameter is specified.
*/
public function getDefaultEndpoint($endpointType, array $bindings = null, $default = self::REQUIRED_OPTION)
{
diff --git a/lib/SimpleSAML/Error/UnserializableException.php b/lib/SimpleSAML/Error/UnserializableException.php
index 54f4eb34f..6ac5b39ff 100644
--- a/lib/SimpleSAML/Error/UnserializableException.php
+++ b/lib/SimpleSAML/Error/UnserializableException.php
@@ -37,7 +37,7 @@ class UnserializableException extends Exception
$msg = $original->getMessage();
$code = $original->getCode();
- if (!is_int($code)) {
+ if ($original instanceof \PDOException) {
// PDOException uses a string as the code. Filter it out here.
$code = -1;
}
diff --git a/lib/SimpleSAML/HTTP/Router.php b/lib/SimpleSAML/HTTP/Router.php
index cb49e8057..7adb8ed80 100644
--- a/lib/SimpleSAML/HTTP/Router.php
+++ b/lib/SimpleSAML/HTTP/Router.php
@@ -21,19 +21,19 @@ use Symfony\Component\Routing\RequestContext;
*/
class Router
{
- /** @var ArgumentResolver */
+ /** @var \Symfony\Component\HttpKernel\Controller\ArgumentResolver */
protected $arguments;
/** @var \SimpleSAML\Configuration|null */
protected $config = null;
- /** @var RequestContext */
+ /** @var \Symfony\Component\Routing\RequestContext */
protected $context;
- /** @var EventDispatcher */
+ /** @var \Symfony\Component\EventDispatcher\EventDispatcher */
protected $dispatcher;
- /** @var Request|null */
+ /** @var \Symfony\Component\HttpFoundation\Request|null */
protected $request = null;
/** @var \SimpleSAML\Module\ControllerResolver */
@@ -42,7 +42,7 @@ class Router
/** @var \SimpleSAML\Session|null */
protected $session = null;
- /** @var RequestStack|null */
+ /** @var \Symfony\Component\HttpFoundation\RequestStack|null */
protected $stack = null;
@@ -65,9 +65,10 @@ class Router
*
* If no specific arguments are given, the default instances will be used (configuration, session, etc).
*
- * @param Request|null $request The request to process. Defaults to the current one.
+ * @param \Symfony\Component\HttpFoundation\Request|null $request
+ * The request to process. Defaults to the current one.
*
- * @return Response A response suitable for the given request.
+ * @return \Symfony\Component\HttpFoundation\Response A response suitable for the given request.
*
* @throws \Exception If an error occurs.
*/
@@ -79,10 +80,13 @@ class Router
if ($this->session === null) {
$this->setSession(Session::getSessionFromRequest());
}
- $this->request = $request;
+
if ($request === null) {
$this->request = Request::createFromGlobals();
+ } else {
+ $this->request = $request;
}
+
$stack = new RequestStack();
$stack->push($this->request);
$this->context->fromRequest($this->request);
@@ -94,11 +98,14 @@ class Router
/**
* Send a given response to the browser.
*
- * @param Response $response The response to send.
+ * @param \Symfony\Component\HttpFoundation\Response $response The response to send.
* @return void
*/
public function send(Response $response)
{
+ if ($this->request === null) {
+ throw new \Exception("No request found to respond to");
+ }
$response->prepare($this->request);
$response->send();
}
diff --git a/lib/SimpleSAML/IdP/IFrameLogoutHandler.php b/lib/SimpleSAML/IdP/IFrameLogoutHandler.php
index 6a6aa90d5..58374cf69 100644
--- a/lib/SimpleSAML/IdP/IFrameLogoutHandler.php
+++ b/lib/SimpleSAML/IdP/IFrameLogoutHandler.php
@@ -88,7 +88,7 @@ class IFrameLogoutHandler implements LogoutHandlerInterface
* This function will never return.
*
* @param string $assocId The association that is terminated.
- * @param string $relayState The RelayState from the start of the logout.
+ * @param string|null $relayState The RelayState from the start of the logout.
* @param \SimpleSAML\Error\Exception|null $error The error that occurred during session termination (if any).
* @return void
*/
diff --git a/lib/SimpleSAML/Locale/Localization.php b/lib/SimpleSAML/Locale/Localization.php
index dd25eb131..8a9346aac 100644
--- a/lib/SimpleSAML/Locale/Localization.php
+++ b/lib/SimpleSAML/Locale/Localization.php
@@ -95,7 +95,9 @@ class Localization
public function __construct(Configuration $configuration)
{
$this->configuration = $configuration;
- $this->localeDir = $this->configuration->resolvePath('locales');
+ /** @var string $locales */
+ $locales = $this->configuration->resolvePath('locales');
+ $this->localeDir = $locales;
$this->language = new Language($configuration);
$this->langcode = $this->language->getPosixLanguage($this->language->getLanguage());
$this->i18nBackend = ($this->configuration->getBoolean('usenewui', false) ? self::GETTEXT_I18N_BACKEND : self::SSP_I18N_BACKEND);
@@ -123,7 +125,9 @@ class Localization
*/
public function getDomainLocaleDir($domain)
{
- $localeDir = $this->configuration->resolvePath('modules').'/'.$domain.'/locales';
+ /** @var string $base */
+ $base = $this->configuration->resolvePath('modules');
+ $localeDir = $base.'/'.$domain.'/locales';
return $localeDir;
}
diff --git a/lib/SimpleSAML/Locale/Translate.php b/lib/SimpleSAML/Locale/Translate.php
index 7d7dfd177..a92ca7ee8 100644
--- a/lib/SimpleSAML/Locale/Translate.php
+++ b/lib/SimpleSAML/Locale/Translate.php
@@ -108,7 +108,7 @@ class Translate
$fileName = substr($name, $sepPos + 1);
$dictDir = Module::getModuleDir($module).'/dictionaries/';
} else {
- $dictDir = $this->configuration->getPathValue('dictionarydir', 'dictionaries/');
+ $dictDir = $this->configuration->getPathValue('dictionarydir', 'dictionaries/') ?: 'dictionaries/';
$fileName = $name;
}
@@ -261,7 +261,7 @@ class Translate
* @param bool $striptags
* @deprecated Not used in twig, gettext
*
- * @return string The translated tag, or a placeholder value if the tag wasn't found.
+ * @return string|null The translated tag, or a placeholder value if the tag wasn't found.
*/
public function t(
$tag,
@@ -292,6 +292,7 @@ class Translate
);
// for backwards compatibility
+ /** @psalm-suppress PossiblyInvalidArgument */
if (!$replacements && ($this->getTag($tag) === null)) {
Logger::warning(
'Code which uses $fallbackdefault === FALSE should be updated to use the getTag() method instead.'
@@ -387,6 +388,7 @@ class Translate
} else {
$filebase = $this->configuration->getPathValue('dictionarydir', 'dictionaries/');
}
+ $filebase = $filebase ?: 'dictionaries/';
$lang = $this->readDictionaryFile($filebase.$file);
Logger::debug('Translate: Merging language array. Loading ['.$file.']');
@@ -520,9 +522,9 @@ class Translate
/**
* Pick a translation from a given array of translations for the current language.
*
- * @param array $context An array of options. The current language must be specified as an ISO 639 code accessible
+ * @param array|null $context An array of options. The current language must be specified as an ISO 639 code accessible
* with the key "currentLanguage" in the array.
- * @param array $translations An array of translations. Each translation has an ISO 639 code as its key, identifying
+ * @param array|null $translations An array of translations. Each translation has an ISO 639 code as its key, identifying
* the language it corresponds to.
*
* @return null|string The translation appropriate for the current language, or null if none found. If the
@@ -530,7 +532,7 @@ class Translate
*/
public static function translateFromArray($context, $translations)
{
- if (!is_array($translations) || $translations === null) {
+ if (!is_array($translations)) {
return null;
}
diff --git a/lib/SimpleSAML/Metadata/MetaDataStorageHandlerFlatFile.php b/lib/SimpleSAML/Metadata/MetaDataStorageHandlerFlatFile.php
index f18ac585a..03c7c1fc9 100644
--- a/lib/SimpleSAML/Metadata/MetaDataStorageHandlerFlatFile.php
+++ b/lib/SimpleSAML/Metadata/MetaDataStorageHandlerFlatFile.php
@@ -21,7 +21,7 @@ class MetaDataStorageHandlerFlatFile extends MetaDataStorageSource
*
* @var string
*/
- private $directory;
+ private $directory = '/';
/**
@@ -50,7 +50,7 @@ class MetaDataStorageHandlerFlatFile extends MetaDataStorageSource
// find the path to the directory we should search for metadata in
if (array_key_exists('directory', $config)) {
- $this->directory = $config['directory'];
+ $this->directory = $config['directory'] ?: 'metadata/';
} else {
$this->directory = $globalConfig->getString('metadatadir', 'metadata/');
}
@@ -58,7 +58,10 @@ class MetaDataStorageHandlerFlatFile extends MetaDataStorageSource
/* Resolve this directory relative to the SimpleSAMLphp directory (unless it is
* an absolute path).
*/
- $this->directory = $globalConfig->resolvePath($this->directory).'/';
+
+ /** @var string $base */
+ $base = $globalConfig->resolvePath($this->directory);
+ $this->directory = $base.'/';
}
diff --git a/lib/SimpleSAML/Metadata/MetaDataStorageHandlerSerialize.php b/lib/SimpleSAML/Metadata/MetaDataStorageHandlerSerialize.php
index a41663fcc..db1b086be 100644
--- a/lib/SimpleSAML/Metadata/MetaDataStorageHandlerSerialize.php
+++ b/lib/SimpleSAML/Metadata/MetaDataStorageHandlerSerialize.php
@@ -4,6 +4,7 @@ namespace SimpleSAML\Metadata;
use SimpleSAML\Configuration;
use SimpleSAML\Logger;
+use SimpleSAML\Utils;
/**
* Class for handling metadata files in serialized format.
@@ -24,9 +25,9 @@ class MetaDataStorageHandlerSerialize extends MetaDataStorageSource
/**
* The base directory where metadata is stored.
*
- * @var string|null
+ * @var string
*/
- private $directory;
+ private $directory = '/';
/**
@@ -49,7 +50,7 @@ class MetaDataStorageHandlerSerialize extends MetaDataStorageSource
/* Resolve this directory relative to the SimpleSAMLphp directory (unless it is
* an absolute path).
*/
- $this->directory = $globalConfig->resolvePath($this->directory);
+ $this->directory = Utils\System::resolvePath($this->directory, $globalConfig->getBaseDir());
}
@@ -187,6 +188,7 @@ class MetaDataStorageHandlerSerialize extends MetaDataStorageSource
$data = @file_get_contents($filePath);
if ($data === false) {
+ /** @var array $error */
$error = error_get_last();
Logger::warning(
'Error reading file '.$filePath.': '.$error['message']
@@ -231,6 +233,7 @@ class MetaDataStorageHandlerSerialize extends MetaDataStorageSource
Logger::info('Creating directory: '.$dir);
$res = @mkdir($dir, 0777, true);
if ($res === false) {
+ /** @var array $error */
$error = error_get_last();
Logger::error('Failed to create directory '.$dir.': '.$error['message']);
return false;
@@ -243,6 +246,7 @@ class MetaDataStorageHandlerSerialize extends MetaDataStorageSource
$res = file_put_contents($newPath, $data);
if ($res === false) {
+ /** @var array $error */
$error = error_get_last();
Logger::error('Error saving file '.$newPath.': '.$error['message']);
return false;
@@ -250,6 +254,7 @@ class MetaDataStorageHandlerSerialize extends MetaDataStorageSource
$res = rename($newPath, $filePath);
if ($res === false) {
+ /** @var array $error */
$error = error_get_last();
Logger::error('Error renaming '.$newPath.' to '.$filePath.': '.$error['message']);
return false;
@@ -283,6 +288,7 @@ class MetaDataStorageHandlerSerialize extends MetaDataStorageSource
$res = unlink($filePath);
if ($res === false) {
+ /** @var array $error */
$error = error_get_last();
Logger::error(
'Failed to delete file '.$filePath.
diff --git a/lib/SimpleSAML/Metadata/MetaDataStorageSource.php b/lib/SimpleSAML/Metadata/MetaDataStorageSource.php
index 3ec6c0b55..45fac75df 100644
--- a/lib/SimpleSAML/Metadata/MetaDataStorageSource.php
+++ b/lib/SimpleSAML/Metadata/MetaDataStorageSource.php
@@ -97,6 +97,8 @@ abstract class MetaDataStorageSource
null
);
}
+
+ /** @var \SimpleSAML\Metadata\MetaDataStorageSource */
return new $className($sourceConfig);
}
}
diff --git a/lib/SimpleSAML/Metadata/SAMLParser.php b/lib/SimpleSAML/Metadata/SAMLParser.php
index d18c6d176..6c9f7fed0 100644
--- a/lib/SimpleSAML/Metadata/SAMLParser.php
+++ b/lib/SimpleSAML/Metadata/SAMLParser.php
@@ -220,8 +220,9 @@ class SAMLParser
}
}
- if ($entityElement->getOrganization() !== null) {
- $this->processOrganization($entityElement->getOrganization());
+ $organization = $entityElement->getOrganization();
+ if ($organization !== null) {
+ $this->processOrganization($organization);
}
if ($entityElement->getContactPerson() !== []) {
@@ -333,10 +334,6 @@ class SAMLParser
throw new \Exception('Failed to read XML from file: '.$file);
}
- if ($doc->documentElement === null) {
- throw new \Exception('Opened file is not an XML document: '.$file);
- }
-
return self::parseDescriptorsElement($doc->documentElement);
}
@@ -1118,13 +1115,12 @@ class SAMLParser
$ret['UIInfo']['PrivacyStatementURL'] = $e->getPrivacyStatementURL();
foreach ($e->getKeywords() as $uiItem) {
- if (!($uiItem instanceof \SAML2\XML\mdui\Keywords)
- || ($uiItem->getKeywords() === [])
- || ($uiItem->getLanguage() === null)
- ) {
+ $keywords = $uiItem->getKeywords();
+ $language = $uiItem->getLanguage();
+ if (($keywords === []) || ($language === null)) {
continue;
}
- $ret['UIInfo']['Keywords'][$uiItem->getLanguage()] = $uiItem->getKeywords();
+ $ret['UIInfo']['Keywords'][$language] = $keywords;
}
foreach ($e->getLogo() as $uiItem) {
if (!($uiItem instanceof Logo)
@@ -1444,10 +1440,6 @@ class SAMLParser
// find the EntityDescriptor DOMElement. This should be the first (and only) child of the DOMDocument
$ed = $doc->documentElement;
- if ($ed === null) {
- throw new \Exception('Failed to load SAML metadata from empty XML document.');
- }
-
if (Utils\XML::isDOMNodeOfType($ed, 'EntityDescriptor', '@md') === false) {
throw new \Exception('Expected first element in the metadata document to be an EntityDescriptor element.');
}
@@ -1494,6 +1486,12 @@ class SAMLParser
}
+ /**
+ * @param string $algorithm
+ * @param string $data
+ * @throws \UnexpectedValueException
+ * @return string
+ */
private function computeFingerprint($algorithm, $data)
{
switch ($algorithm) {
diff --git a/lib/SimpleSAML/Metadata/Signer.php b/lib/SimpleSAML/Metadata/Signer.php
index b149fdd9e..28824c059 100644
--- a/lib/SimpleSAML/Metadata/Signer.php
+++ b/lib/SimpleSAML/Metadata/Signer.php
@@ -265,6 +265,7 @@ class Signer
$objKey->loadKey($keyData, false);
// get the EntityDescriptor node we should sign
+ /** @var \DOMElement $rootNode */
$rootNode = $xml->firstChild;
$rootNode->setAttribute('ID', '_'.hash('sha256', $metadataString));
diff --git a/lib/SimpleSAML/Metadata/Sources/MDQ.php b/lib/SimpleSAML/Metadata/Sources/MDQ.php
index 399975d5d..60d4c01f2 100644
--- a/lib/SimpleSAML/Metadata/Sources/MDQ.php
+++ b/lib/SimpleSAML/Metadata/Sources/MDQ.php
@@ -4,6 +4,7 @@ namespace SimpleSAML\Metadata\Sources;
use RobRichards\XMLSecLibs\XMLSecurityDSig;
use SimpleSAML\Configuration;
+use SimpleSAML\Error;
use SimpleSAML\Logger;
use SimpleSAML\Metadata\SAMLParser;
use SimpleSAML\Utils;
@@ -34,6 +35,11 @@ class MDQ extends \SimpleSAML\Metadata\MetaDataStorageSource
*/
private $validateFingerprint;
+ /**
+ * @var string
+ */
+ private $validateFingerprintAlgorithm;
+
/**
* The cache directory, or null if no cache directory is configured.
*
@@ -129,6 +135,10 @@ class MDQ extends \SimpleSAML\Metadata\MetaDataStorageSource
assert(is_string($set));
assert(is_string($entityId));
+ if ($this->cacheDir === null) {
+ throw new Error\ConfigurationError("Missing cache directory configuration.");
+ }
+
$cachekey = sha1($entityId);
return $this->cacheDir.'/'.$set.'-'.$cachekey.'.cached.xml';
}
@@ -174,6 +184,7 @@ class MDQ extends \SimpleSAML\Metadata\MetaDataStorageSource
$rawData = file_get_contents($cachefilename);
if (empty($rawData)) {
+ /** @var array $error */
$error = error_get_last();
throw new \Exception(
__CLASS__.': error reading metadata from cache file "'.$cachefilename.'": '.$error['message']
diff --git a/lib/SimpleSAML/Utils/EMail.php b/lib/SimpleSAML/Utils/EMail.php
index add2e6503..5e41d79c7 100644
--- a/lib/SimpleSAML/Utils/EMail.php
+++ b/lib/SimpleSAML/Utils/EMail.php
@@ -260,7 +260,7 @@ class EMail
]);
$t = new Template($config, $template);
$twig = $t->getTwig();
- if (is_bool($twig)) {
+ if (!isset($twig)) {
throw new \Exception('Even though we explicitly configure that we want Twig, the Template class does not give us Twig. This is a bug.');
}
$result = $twig->render($template, [
diff --git a/lib/SimpleSAML/XHTML/Template.php b/lib/SimpleSAML/XHTML/Template.php
index b1c3cca00..9ba61d9e6 100644
--- a/lib/SimpleSAML/XHTML/Template.php
+++ b/lib/SimpleSAML/XHTML/Template.php
@@ -65,9 +65,9 @@ class Template extends Response
/**
* The twig environment.
*
- * @var false|\JaimePerez\TwigConfigurableI18n\Twig\Environment
+ * @var \Twig\Environment
*/
- private $twig = false;
+ private $twig;
/**
* The template name.
@@ -78,6 +78,8 @@ class Template extends Response
/**
* Current module, if any.
+ *
+ * @var string
*/
private $module;
@@ -108,9 +110,9 @@ class Template extends Response
* of the module and the name of the theme, respectively. If we are using the default theme, the variable has
* the 'default' string in the "name" key, and 'null' in the "module" key.
*
- * @var array|null
+ * @var array
*/
- private $theme = null;
+ private $theme = ['module' => null, 'name' => 'default'];
/**
* Constructor
@@ -147,6 +149,7 @@ class Template extends Response
if ($controller && class_exists($controller) &&
in_array(TemplateControllerInterface::class, class_implements($controller))
) {
+ /** @var \SimpleSAML\XHTML\TemplateControllerInterface $this->controller */
$this->controller = new $controller();
}
@@ -173,7 +176,7 @@ class Template extends Response
$tag = $this->configuration->getVersion();
if ($tag === 'master') {
- $tag = filemtime($file);
+ $tag = strval(filemtime($file));
}
$tag = substr(hash('md5', $tag), 0, 5);
return $this->configuration->getBasePath().'assets/'.$asset.'?tag='.$tag;
@@ -261,17 +264,20 @@ class Template extends Response
/**
* Setup twig.
- * @return Twig_Environment|false
+ * @return \Twig\Environment
+ * @throws \Exception if the template does not exist
*/
private function setupTwig()
{
$auto_reload = $this->configuration->getBoolean('template.auto_reload', true);
$cache = $this->configuration->getString('template.cache', false);
+
// set up template paths
$loader = $this->setupTwigTemplatepaths();
+
// abort if twig template does not exist
if (!$loader->exists($this->twig_template)) {
- return false;
+ throw new \Exception('Template-file \"'.$this->template.'\" does not exist.');
}
// load extra i18n domains
@@ -337,7 +343,7 @@ class Template extends Response
*/
private function findThemeTemplateDirs()
{
- if ($this->theme['module'] === null) {
+ if (!isset($this->theme['module'])) {
// no module involved
return [];
}
@@ -517,7 +523,7 @@ class Template extends Response
*/
public function show()
{
- if ($this->twig !== false) {
+ if ($this->useNewUI) {
echo $this->getContents();
} else {
require($this->findTemplatePath($this->template));
@@ -573,7 +579,8 @@ class Template extends Response
$filename = Module::getModuleDir($templateModule).'/templates/'.$templateName;
} else {
// .../templates/<theme>/<templateName>
- $filename = $this->configuration->getPathValue('templatedir', 'templates/').$templateName;
+ $base = $this->configuration->getPathValue('templatedir', 'templates/') ?: 'templates/';
+ $filename = $base.$templateName;
}
if (file_exists($filename)) {
@@ -592,7 +599,8 @@ class Template extends Response
$filename = Module::getModuleDir($templateModule).'/templates/'.$templateName;
} else {
// .../templates/<templateName>
- $filename = $this->configuration->getPathValue('templatedir', 'templates/').'/'.$templateName;
+ $base = $this->configuration->getPathValue('templatedir', 'templates/') ?: 'templates/';
+ $filename = $base.'/'.$templateName;
}
if (file_exists($filename)) {
@@ -638,7 +646,7 @@ class Template extends Response
/**
* Get the current instance of Twig in use.
*
- * @return false|Twig_Environment The Twig instance in use, or false if Twig is not used.
+ * @return \Twig\Environment The Twig instance in use, or null if Twig is not used.
*/
public function getTwig()
{
@@ -727,7 +735,7 @@ class Template extends Response
/**
* @param string $tag
*
- * @return array
+ * @return array|null
* @deprecated This method will be removed in SSP 2.0. Please use \SimpleSAML\Locale\Translate::getTag() instead.
*/
public function getTag($tag)
@@ -856,7 +864,7 @@ class Template extends Response
* @param bool $fallbackdefault
* @param array $oldreplacements
* @param bool $striptags
- * @return string
+ * @return string|null
*/
public function t(
$tag,
diff --git a/lib/SimpleSAML/XHTML/TemplateLoader.php b/lib/SimpleSAML/XHTML/TemplateLoader.php
index a2c657147..14231bb9f 100644
--- a/lib/SimpleSAML/XHTML/TemplateLoader.php
+++ b/lib/SimpleSAML/XHTML/TemplateLoader.php
@@ -9,6 +9,8 @@ use SimpleSAML\Module;
* when the main template is not part of a module (or the same one).
*
* @package simplesamlphp/simplesamlphp
+ *
+ * @psalm-suppress DeprecatedInterface This suppress may be removed when Twig 3.0 becomes the default
*/
class TemplateLoader extends \Twig\Loader\FilesystemLoader
{
@@ -16,6 +18,10 @@ class TemplateLoader extends \Twig\Loader\FilesystemLoader
* This method adds a namespace dynamically so that we can load templates from modules whenever we want.
*
* {@inheritdoc}
+ *
+ * @param string $name
+ * @param bool $throw
+ * @return string|false|null
*/
protected function findTemplate($name, $throw = true)
{
diff --git a/lib/SimpleSAML/XML/Parser.php b/lib/SimpleSAML/XML/Parser.php
index 234aaf3ca..2e5713d15 100644
--- a/lib/SimpleSAML/XML/Parser.php
+++ b/lib/SimpleSAML/XML/Parser.php
@@ -11,8 +11,8 @@ namespace SimpleSAML\XML;
class Parser
{
- /** @var \SimpleXMLElement|null */
- public $simplexml = null;
+ /** @var \SimpleXMLElement */
+ public $simplexml;
/**
* @param string $xml
@@ -29,6 +29,7 @@ class Parser
/**
* @param \SimpleXMLElement $element
* @return \SimpleSAML\XML\Parser
+ * @psalm-return \SimpleSAML\XML\Parser
*/
public static function fromSimpleXMLElement(\SimpleXMLElement $element)
{
@@ -41,8 +42,11 @@ class Parser
/* Create a new parser with the xml document where the namespace definitions
* are added.
*/
- $parser = new Parser($element->asXML());
- return $parser;
+ $xml = $element->asXML();
+ if ($xml === false) {
+ throw new \Exception('Error converting SimpleXMLElement to well-formed XML string.');
+ }
+ return new Parser($xml);
}
@@ -55,6 +59,7 @@ class Parser
public function getValueDefault($xpath, $defvalue)
{
try {
+ /** @var string */
return $this->getValue($xpath, true);
} catch (\Exception $e) {
return $defvalue;
@@ -66,7 +71,7 @@ class Parser
* @param string $xpath
* @param bool $required
* @throws \Exception
- * @return string
+ * @return string|null
*/
public function getValue($xpath, $required = false)
{
diff --git a/lib/SimpleSAML/XML/Shib13/AuthnRequest.php b/lib/SimpleSAML/XML/Shib13/AuthnRequest.php
index 8277e51bb..942dabb38 100644
--- a/lib/SimpleSAML/XML/Shib13/AuthnRequest.php
+++ b/lib/SimpleSAML/XML/Shib13/AuthnRequest.php
@@ -76,9 +76,11 @@ class AuthnRequest
$desturl = $desturl['Location'];
$target = $this->getRelayState();
-
+ $issuer = $this->getIssuer();
+ assert($issuer !== null);
+
$url = $desturl.'?'.
- 'providerId='.urlencode($this->getIssuer()).
+ 'providerId='.urlencode($issuer).
'&shire='.urlencode($shire).
(isset($target) ? '&target='.urlencode($target) : '');
return $url;
diff --git a/lib/SimpleSAML/XML/Shib13/AuthnResponse.php b/lib/SimpleSAML/XML/Shib13/AuthnResponse.php
index d04983b31..9148c3869 100644
--- a/lib/SimpleSAML/XML/Shib13/AuthnResponse.php
+++ b/lib/SimpleSAML/XML/Shib13/AuthnResponse.php
@@ -269,10 +269,14 @@ class AuthnResponse
'shib:AttributeStatement/shib:Attribute/shib:AttributeValue',
$assertion
);
- /** @var \DOMElement $attribute */
+
foreach ($attribute_nodes as $attribute) {
+ /** @var \DOMElement $attribute */
+
$value = $attribute->textContent;
- $name = $attribute->parentNode->getAttribute('AttributeName');
+ /** @var \DOMElement $parentNode */
+ $parentNode = $attribute->parentNode;
+ $name = $parentNode->getAttribute('AttributeName');
if ($attribute->hasAttribute('Scope')) {
$scopePart = '@'.$attribute->getAttribute('Scope');
@@ -280,7 +284,7 @@ class AuthnResponse
$scopePart = '';
}
- if (!is_string($name)) {
+ if (empty($name)) {
throw new \Exception('Shib13 Attribute node without an AttributeName.');
}
diff --git a/lib/SimpleSAML/XML/Signer.php b/lib/SimpleSAML/XML/Signer.php
index 235335b64..09144e196 100644
--- a/lib/SimpleSAML/XML/Signer.php
+++ b/lib/SimpleSAML/XML/Signer.php
@@ -26,7 +26,7 @@ class Signer
private $idAttrName = '';
/**
- * @var XMLSecurityKey|bool The private key (as an XMLSecurityKey).
+ * @var XMLSecurityKey|false The private key (as an XMLSecurityKey).
*/
private $privateKey = false;
@@ -283,7 +283,8 @@ class Signer
assert($insertBefore === null || $insertBefore instanceof DOMElement ||
$insertBefore instanceof DOMComment || $insertBefore instanceof DOMText);
- if ($this->privateKey === false) {
+ $privateKey = $this->privateKey;
+ if ($privateKey === false) {
throw new \Exception('Private key not set.');
}
@@ -303,9 +304,7 @@ class Signer
$options
);
- /** @var \RobRichards\XMLSecLibs\XMLSecurityKey $this->privateKey */
- $objXMLSecDSig->sign($this->privateKey);
-
+ $objXMLSecDSig->sign($privateKey);
// Add the certificate to the signature
$objXMLSecDSig->add509Cert($this->certificate, true);
diff --git a/lib/SimpleSAML/XML/Validator.php b/lib/SimpleSAML/XML/Validator.php
index 848445ac0..2d1184525 100644
--- a/lib/SimpleSAML/XML/Validator.php
+++ b/lib/SimpleSAML/XML/Validator.php
@@ -16,7 +16,7 @@ use SimpleSAML\Logger;
class Validator
{
/**
- * @var string This variable contains the X509 certificate the XML document
+ * @var string|null This variable contains the X509 certificate the XML document
* was signed with, or NULL if it wasn't signed with an X509 certificate.
*/
private $x509Certificate = null;
@@ -37,7 +37,7 @@ class Validator
* - A string: Assumed to be a PEM-encoded certificate / public key.
* - An array: Assumed to be an array returned by \SimpleSAML\Utils\Crypto::loadPublicKey.
*
- * @param \DOMNode $xmlNode The XML node which contains the Signature element.
+ * @param \DOMDocument $xmlNode The XML node which contains the Signature element.
* @param string|array $idAttribute The ID attribute which is used in node references. If
* this attribute is NULL (the default), then we will use whatever is the default
* ID. Can be eigther a string with one value, or an array with multiple ID
@@ -47,7 +47,7 @@ class Validator
*/
public function __construct($xmlNode, $idAttribute = null, $publickey = false)
{
- assert($xmlNode instanceof \DOMNode);
+ assert($xmlNode instanceof \DOMDocument);
if ($publickey === null) {
$publickey = false;
@@ -143,7 +143,7 @@ class Validator
* This function will return the certificate as a PEM-encoded string. If the XML
* wasn't signed by an X509 certificate, NULL will be returned.
*
- * @return string The certificate as a PEM-encoded string, or NULL if not signed with an X509 certificate.
+ * @return string|null The certificate as a PEM-encoded string, or NULL if not signed with an X509 certificate.
*/
public function getX509Certificate()
{
@@ -277,12 +277,14 @@ class Validator
{
assert($node instanceof \DOMNode);
- while ($node !== null) {
- if (in_array($node, $this->validNodes, true)) {
- return true;
- }
+ if ($this->validNodes !== null) {
+ while ($node !== null) {
+ if (in_array($node, $this->validNodes, true)) {
+ return true;
+ }
- $node = $node->parentNode;
+ $node = $node->parentNode;
+ }
}
/* Neither this node nor any of the parent nodes could be found in the list of
@@ -438,8 +440,8 @@ class Validator
if ($resExternal !== true) {
Logger::debug('Failed to validate with external function: '.var_export($resExternal, true));
throw new \Exception('Could not verify certificate against CA file "'.
- $caFile.'". Internal result:'.$resBuiltin.
- ' External result:'.$resExternal);
+ $caFile.'". Internal result:'.var_export($resBuiltin, true).
+ ' External result:'.var_export($resExternal, true));
}
}
diff --git a/psalm.xml b/psalm.xml
index 5e1a94690..c80d81a44 100644
--- a/psalm.xml
+++ b/psalm.xml
@@ -3,15 +3,36 @@
name="SimpleSAMLphp"
useDocblockTypes="true"
totallyTyped="false"
+ hideExternalErrors="true"
>
<projectFiles>
+ <directory name="lib/SimpleSAML/Auth" />
+ <directory name="lib/SimpleSAML/Bindings" />
+ <directory name="lib/SimpleSAML/Error" />
+ <directory name="lib/SimpleSAML/HTTP" />
+ <directory name="lib/SimpleSAML/IdP" />
+ <directory name="lib/SimpleSAML/Locale" />
+ <directory name="lib/SimpleSAML/Logger" />
+ <directory name="lib/SimpleSAML/Metadata" />
+ <directory name="lib/SimpleSAML/Module" />
+ <directory name="lib/SimpleSAML/Stats" />
+ <directory name="lib/SimpleSAML/Store" />
<directory name="lib/SimpleSAML/Utils" />
+ <directory name="lib/SimpleSAML/XHTML" />
+ <directory name="lib/SimpleSAML/XML" />
+
+ <!-- Ignore deprecated classes -->
+ <ignoreFiles>
+ <file name="lib/SimpleSAML/Auth/DefaultAuth.php" />
+ <file name="lib/SimpleSAML/Utilities.php" />
+ </ignoreFiles>
</projectFiles>
<issueHandlers>
<LessSpecificReturnType errorLevel="info" />
<!-- level 3 issues - slightly lazy code writing, but probably low false-negatives -->
+ <DeprecatedClass errorLevel="info" />
<DeprecatedMethod errorLevel="info" />
<MissingClosureReturnType errorLevel="info" />
@@ -30,6 +51,15 @@
<!-- Ignore these errors until we are fully typehinted -->
<DocblockTypeContradiction errorLevel="suppress" />
<RedundantConditionGivenDocblockType errorLevel="suppress" />
+
+ <!-- Suppress false-positive -->
+ <UnresolvableInclude errorLevel="suppress" />
+ <!-- See #1141 -->
+ <MissingDependency>
+ <errorLevel type="suppress">
+ <file name="lib/SimpleSAML/HTTP/Router.php" />
+ </errorLevel>
+ </MissingDependency>
</issueHandlers>
<stubs>
--
GitLab