From 1c4c5f64d79f86718e7e9333dacd37fe578328ba Mon Sep 17 00:00:00 2001 From: Tim van Dijen <tvdijen@gmail.com> Date: Sat, 15 Feb 2020 21:19:42 +0100 Subject: [PATCH] Add typehints for modules/* --- lib/SimpleSAML/Command/RouterDebugCommand.php | 4 +- lib/SimpleSAML/Configuration.php | 11 ++-- lib/SimpleSAML/Error/AuthSource.php | 5 +- lib/SimpleSAML/Logger.php | 1 - lib/SimpleSAML/Memcache.php | 1 - .../Metadata/MetaDataStorageHandler.php | 3 - lib/SimpleSAML/Metadata/SAMLBuilder.php | 4 -- lib/SimpleSAML/Module.php | 2 +- lib/SimpleSAML/Session.php | 2 - lib/SimpleSAML/Utils/Crypto.php | 4 +- lib/SimpleSAML/Utils/System.php | 3 +- modules/admin/lib/Controller/Config.php | 10 +-- modules/admin/lib/Controller/Federation.php | 8 +-- modules/admin/lib/Controller/Menu.php | 4 +- modules/admin/lib/Controller/Test.php | 2 +- modules/core/hooks/hook_frontpage.php | 3 +- modules/core/hooks/hook_sanitycheck.php | 3 +- modules/core/lib/ACL.php | 11 +++- .../core/lib/Auth/Process/AttributeMap.php | 1 + .../core/lib/Auth/Source/AdminPassword.php | 6 -- modules/core/lib/Auth/UserPassBase.php | 17 ++--- modules/core/lib/Auth/UserPassOrgBase.php | 7 +-- modules/core/lib/Controller/Login.php | 2 +- modules/core/lib/Stats/Output/File.php | 2 +- .../core/lib/Storage/SQLPermanentStorage.php | 57 +++++++++-------- modules/cron/hooks/hook_configpage.php | 2 +- modules/cron/hooks/hook_cron.php | 3 +- modules/cron/hooks/hook_frontpage.php | 3 +- modules/cron/lib/Controller/Cron.php | 8 +-- modules/cron/lib/Cron.php | 4 +- .../exampleauth/lib/Auth/Source/External.php | 9 +-- .../lib/Auth/Source/StaticSource.php | 6 +- .../exampleauth/lib/Auth/Source/UserPass.php | 10 +-- .../multiauth/lib/Auth/Source/MultiAuth.php | 20 ++---- modules/portal/hooks/hook_htmlinject.php | 3 +- modules/portal/lib/Portal.php | 10 +-- modules/saml/hooks/hook_metadata_hosted.php | 4 +- .../Process/ExpectedAuthnContextClassRef.php | 2 +- modules/saml/lib/Auth/Source/SP.php | 62 +++++++------------ modules/saml/lib/BaseNameIDGenerator.php | 2 +- modules/saml/lib/Error.php | 37 +++-------- modules/saml/lib/Error/NoAuthnContext.php | 2 +- modules/saml/lib/Error/NoAvailableIDP.php | 2 +- modules/saml/lib/Error/NoPassive.php | 2 +- modules/saml/lib/Error/NoSupportedIDP.php | 2 +- modules/saml/lib/Error/ProxyCountExceeded.php | 2 +- modules/saml/lib/IdP/SAML2.php | 35 ++++------- modules/saml/lib/IdP/SQLNameID.php | 44 +++++++------ modules/saml/lib/Message.php | 27 ++++---- modules/saml/lib/SP/LogoutStore.php | 16 ++--- tests/Utils/SpTester.php | 6 +- tests/lib/SimpleSAML/DatabaseTest.php | 1 - 52 files changed, 194 insertions(+), 303 deletions(-) diff --git a/lib/SimpleSAML/Command/RouterDebugCommand.php b/lib/SimpleSAML/Command/RouterDebugCommand.php index d1bfab227..05edf2fe6 100644 --- a/lib/SimpleSAML/Command/RouterDebugCommand.php +++ b/lib/SimpleSAML/Command/RouterDebugCommand.php @@ -26,7 +26,7 @@ class RouterDebugCommand extends Command /** - * @param Symfony\Component\Routing\RouterInterface + * @param \Symfony\Component\Routing\RouterInterface * * @throws \Symfony\Component\Console\Exception\LogicException When the command name is empty */ @@ -58,7 +58,7 @@ EOF /** * @param \Symfony\Component\Console\Input\InputInterface $input * @param \Symfony\Component\Console\Output\OutputInterface $output - * @return int 0 if everything went fine, or an exit code + * @return void */ protected function execute(InputInterface $input, OutputInterface $output) { diff --git a/lib/SimpleSAML/Configuration.php b/lib/SimpleSAML/Configuration.php index 5059487a9..be7ad8955 100644 --- a/lib/SimpleSAML/Configuration.php +++ b/lib/SimpleSAML/Configuration.php @@ -468,8 +468,6 @@ class Configuration implements Utils\ClearableState return null; } - Assert::string($path); - return Utils\System::resolvePath($path, $this->getBaseDir()); } @@ -790,8 +788,6 @@ class Configuration implements Utils\ClearableState */ public function getArrayize(string $name, $default = self::REQUIRED_OPTION) { - Assert::string($name); - $ret = $this->getValue($name, $default); if ($ret === $default) { @@ -817,7 +813,7 @@ class Configuration implements Utils\ClearableState * required if this parameter isn't given. The default value can be any value, including * null. * - * @return array The option with the given name, or $default if the option isn't found and $default is specified. + * @return mixed The option with the given name, or $default if the option isn't found and $default is specified. * * @throws \Exception If the option is not a string or an array of strings. */ @@ -858,7 +854,8 @@ class Configuration implements Utils\ClearableState * This function will only return null if $default is set to null and the option * doesn't exist. * - * @return \SimpleSAML\Configuration|null The option with the given name, or $default if the option isn't found and $default is specified. + * @return \SimpleSAML\Configuration|null The option with the given name, + * or $default if the option isn't found and $default is specified. * * @throws \Exception If the option is not an array. */ @@ -1022,7 +1019,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 array|null The default endpoint, or null if no acceptable endpoints are used. + * @return mixed|null The default endpoint, or null if no acceptable endpoints are used. * * @throws \Exception If no supported endpoint is found. */ diff --git a/lib/SimpleSAML/Error/AuthSource.php b/lib/SimpleSAML/Error/AuthSource.php index b836b93b8..b42ab25d0 100644 --- a/lib/SimpleSAML/Error/AuthSource.php +++ b/lib/SimpleSAML/Error/AuthSource.php @@ -4,13 +4,12 @@ declare(strict_types=1); namespace SimpleSAML\Error; -use Exception; use Webmozart\Assert\Assert; /** * Baseclass for auth source exceptions. * - * @package SimpleSAMLphp_base + * @package SimpleSAMLphp * */ @@ -36,7 +35,7 @@ class AuthSource extends Error * @param string $reason Description of the error. * @param \Exception|null $cause */ - public function __construct(string $authsource, string $reason, Exception $cause = null) + public function __construct(string $authsource, string $reason, \Exception $cause = null) { $this->authsource = $authsource; $this->reason = $reason; diff --git a/lib/SimpleSAML/Logger.php b/lib/SimpleSAML/Logger.php index 0805dd328..be1742dcd 100644 --- a/lib/SimpleSAML/Logger.php +++ b/lib/SimpleSAML/Logger.php @@ -426,7 +426,6 @@ class Logger // get the configuration $config = Configuration::getInstance(); - Assert::isInstanceOf($config, Configuration::class); // setting minimum log_level self::$logLevel = $config->getInteger('logging.level', self::INFO); diff --git a/lib/SimpleSAML/Memcache.php b/lib/SimpleSAML/Memcache.php index 7eff621f7..07302815e 100644 --- a/lib/SimpleSAML/Memcache.php +++ b/lib/SimpleSAML/Memcache.php @@ -408,7 +408,6 @@ class Memcache { // get the configuration instance $config = Configuration::getInstance(); - Assert::isInstanceOf($config, Configuration::class); // get the expire-value from the configuration $expire = $config->getInteger('memcache_store.expires', 0); diff --git a/lib/SimpleSAML/Metadata/MetaDataStorageHandler.php b/lib/SimpleSAML/Metadata/MetaDataStorageHandler.php index ac8407ce3..e4a88e189 100644 --- a/lib/SimpleSAML/Metadata/MetaDataStorageHandler.php +++ b/lib/SimpleSAML/Metadata/MetaDataStorageHandler.php @@ -108,7 +108,6 @@ class MetaDataStorageHandler implements \SimpleSAML\Utils\ClearableState // get the configuration $config = Configuration::getInstance(); - Assert::isInstanceOf($config, Configuration::class); $baseurl = Utils\HTTP::getSelfURLHost() . $config->getBasePath(); @@ -311,8 +310,6 @@ class MetaDataStorageHandler implements \SimpleSAML\Utils\ClearableState $index = $this->getMetaDataCurrentEntityID($set, 'metaindex'); } - Assert::string($index); - foreach ($this->sources as $source) { $metadata = $source->getMetaData($index, $set); diff --git a/lib/SimpleSAML/Metadata/SAMLBuilder.php b/lib/SimpleSAML/Metadata/SAMLBuilder.php index f598e7e02..d7ae63676 100644 --- a/lib/SimpleSAML/Metadata/SAMLBuilder.php +++ b/lib/SimpleSAML/Metadata/SAMLBuilder.php @@ -76,8 +76,6 @@ class SAMLBuilder */ public function __construct(string $entityId, int $maxCache = null, int $maxDuration = null) { - Assert::string($entityId); - $this->maxCache = $maxCache; $this->maxDuration = $maxDuration; @@ -132,8 +130,6 @@ class SAMLBuilder */ public function getEntityDescriptorText(bool $formatted = true): string { - Assert::boolean($formatted); - $xml = $this->getEntityDescriptor(); if ($formatted) { Utils\XML::formatDOMElement($xml); diff --git a/lib/SimpleSAML/Module.php b/lib/SimpleSAML/Module.php index e59418bdc..285882f7c 100644 --- a/lib/SimpleSAML/Module.php +++ b/lib/SimpleSAML/Module.php @@ -583,7 +583,7 @@ class Module * * This method removes the trailing slash and redirects to the resulting URL. * - * @param Symfony\Component\HttpFoundation\Request $request The request to process by this controller method. + * @param \Symfony\Component\HttpFoundation\Request $request The request to process by this controller method. * * @return \Symfony\Component\HttpFoundation\RedirectResponse * A redirection to the URI specified in the request, but without the trailing slash. diff --git a/lib/SimpleSAML/Session.php b/lib/SimpleSAML/Session.php index e0a16dd3e..61428d3e9 100644 --- a/lib/SimpleSAML/Session.php +++ b/lib/SimpleSAML/Session.php @@ -352,8 +352,6 @@ class Session implements \Serializable, Utils\ClearableState return null; } - Assert::isInstanceOf($session, self::class); - if ($checkToken) { $globalConfig = Configuration::getInstance(); diff --git a/lib/SimpleSAML/Utils/Crypto.php b/lib/SimpleSAML/Utils/Crypto.php index 44752f4e5..f37c31803 100644 --- a/lib/SimpleSAML/Utils/Crypto.php +++ b/lib/SimpleSAML/Utils/Crypto.php @@ -328,9 +328,7 @@ class Crypto */ public static function pwHash(string $password): string { - if (!is_string($password)) { - throw new \InvalidArgumentException('Invalid input parameter.'); - } elseif (!is_string($hash = password_hash($password, PASSWORD_DEFAULT))) { + if (!is_string($hash = password_hash($password, PASSWORD_DEFAULT))) { throw new \InvalidArgumentException('Error while hashing password.'); } return $hash; diff --git a/lib/SimpleSAML/Utils/System.php b/lib/SimpleSAML/Utils/System.php index 2c2b4d181..a61b6784c 100644 --- a/lib/SimpleSAML/Utils/System.php +++ b/lib/SimpleSAML/Utils/System.php @@ -28,7 +28,8 @@ class System /** * This function returns the Operating System we are running on. * - * @return string|false A predefined constant identifying the OS we are running on. False if we are unable to determine it. + * @return int|false A predefined constant identifying the OS we are running on. + * False if we are unable to determine it. * * @author Jaime Perez, UNINETT AS <jaime.perez@uninett.no> */ diff --git a/modules/admin/lib/Controller/Config.php b/modules/admin/lib/Controller/Config.php index ad94e0df7..423adafb0 100644 --- a/modules/admin/lib/Controller/Config.php +++ b/modules/admin/lib/Controller/Config.php @@ -57,7 +57,7 @@ class Config * * @return \SimpleSAML\XHTML\Template */ - public function diagnostics(Request $request) + public function diagnostics(Request $request): Template { Utils\Auth::requireAdmin(); @@ -90,7 +90,7 @@ class Config * * @return \SimpleSAML\XHTML\Template */ - public function main() + public function main(): Template { Utils\Auth::requireAdmin(); @@ -127,7 +127,7 @@ class Config * * @return RunnableResponse */ - public function phpinfo() + public function phpinfo(): RunnableResponse { Utils\Auth::requireAdmin(); @@ -148,7 +148,7 @@ class Config * * @return array */ - protected function getPrerequisiteChecks() + protected function getPrerequisiteChecks(): array { $matrix = [ [ @@ -333,7 +333,7 @@ class Config * * @return array */ - protected function getWarnings() + protected function getWarnings(): array { $warnings = []; diff --git a/modules/admin/lib/Controller/Federation.php b/modules/admin/lib/Controller/Federation.php index 6b6e0ca37..7ba872b7d 100644 --- a/modules/admin/lib/Controller/Federation.php +++ b/modules/admin/lib/Controller/Federation.php @@ -62,7 +62,7 @@ class Federation * @throws \SimpleSAML\Error\Exception * @throws \SimpleSAML\Error\Exception */ - public function main() + public function main(): Template { Utils\Auth::requireAdmin(); @@ -344,7 +344,7 @@ class Federation * * @return \SimpleSAML\XHTML\Template */ - public function metadataConverter(Request $request) + public function metadataConverter(Request $request): Template { Utils\Auth::requireAdmin(); if ($xmlfile = $request->files->get('xmlfile')) { @@ -411,7 +411,7 @@ class Federation * * @return Response PEM-encoded certificate. */ - public function downloadCert(Request $request) + public function downloadCert(Request $request): Response { Utils\Auth::requireAdmin(); @@ -454,7 +454,7 @@ class Federation * * @return Response */ - public function showRemoteEntity(Request $request) + public function showRemoteEntity(Request $request): Response { Utils\Auth::requireAdmin(); diff --git a/modules/admin/lib/Controller/Menu.php b/modules/admin/lib/Controller/Menu.php index 6887fab1a..bd1b2f430 100644 --- a/modules/admin/lib/Controller/Menu.php +++ b/modules/admin/lib/Controller/Menu.php @@ -55,7 +55,7 @@ final class Menu * @param string $name The name of the option for display purposes. * @return void */ - public function addOption($id, $url, $name) + public function addOption(string $id, string $url, string $name): void { $this->options[$id] = [ 'url' => $url, @@ -84,7 +84,7 @@ final class Menu * * @return \SimpleSAML\XHTML\Template The template with the added menu. */ - public function insert(Template $template) + public function insert(Template $template): Template { $template->data['menu'] = $this->options; Module::callHooks('adminmenu', $template); diff --git a/modules/admin/lib/Controller/Test.php b/modules/admin/lib/Controller/Test.php index 06b4baa0f..5a9dd1db2 100644 --- a/modules/admin/lib/Controller/Test.php +++ b/modules/admin/lib/Controller/Test.php @@ -58,7 +58,7 @@ class Test * @param string|null $as * @return \SimpleSAML\XHTML\Template */ - public function main(Request $request, $as) + public function main(Request $request, string $as = null) { Utils\Auth::requireAdmin(); if (is_null($as)) { diff --git a/modules/core/hooks/hook_frontpage.php b/modules/core/hooks/hook_frontpage.php index f6ff146a3..8eb6d6416 100644 --- a/modules/core/hooks/hook_frontpage.php +++ b/modules/core/hooks/hook_frontpage.php @@ -10,9 +10,8 @@ use Webmozart\Assert\Assert; * @param array &$links The links on the frontpage, split into sections. * @return void */ -function core_hook_frontpage(&$links) +function core_hook_frontpage(array &$links): void { - Assert::isArray($links); Assert::keyExists($links, 'links'); $links['links']['frontpage_welcome'] = [ diff --git a/modules/core/hooks/hook_sanitycheck.php b/modules/core/hooks/hook_sanitycheck.php index c786872bb..d37d6c62b 100644 --- a/modules/core/hooks/hook_sanitycheck.php +++ b/modules/core/hooks/hook_sanitycheck.php @@ -10,9 +10,8 @@ use Webmozart\Assert\Assert; * @param array &$hookinfo hookinfo * @return void */ -function core_hook_sanitycheck(&$hookinfo) +function core_hook_sanitycheck(array &$hookinfo): void { - Assert::isArray($hookinfo); Assert::keyExists($hookinfo, 'errors'); Assert::keyExists($hookinfo, 'info'); diff --git a/modules/core/lib/ACL.php b/modules/core/lib/ACL.php index 6ab1baa79..0840cd12b 100644 --- a/modules/core/lib/ACL.php +++ b/modules/core/lib/ACL.php @@ -23,6 +23,7 @@ class ACL */ private $acl; + /** * Initializer for this access control list. * @@ -70,13 +71,14 @@ class ACL return $config->getArray($id); } + /** * Match the attributes against the access control list. * * @param array $attributes The attributes of an user. * @return boolean TRUE if the user is allowed to access the resource, FALSE if not. */ - public function allows(array $attributes) + public function allows(array $attributes): bool { foreach ($this->acl as $rule) { $action = array_shift($rule); @@ -94,6 +96,7 @@ class ACL return false; } + /** * Match the attributes against the given rule. * @@ -129,6 +132,7 @@ class ACL } } + /** * 'and' match operator. * @@ -148,6 +152,7 @@ class ACL return true; } + /** * 'equals' match operator. * @@ -188,6 +193,7 @@ class ACL return true; } + /** * 'equals-preg' match operator. * @@ -229,6 +235,7 @@ class ACL return true; } + /** * 'has' match operator. * @@ -256,6 +263,7 @@ class ACL return true; } + /** * 'has-preg' match operator. * @@ -284,6 +292,7 @@ class ACL return true; } + /** * 'or' match operator. * diff --git a/modules/core/lib/Auth/Process/AttributeMap.php b/modules/core/lib/Auth/Process/AttributeMap.php index 63c81aad0..5a79368b9 100644 --- a/modules/core/lib/Auth/Process/AttributeMap.php +++ b/modules/core/lib/Auth/Process/AttributeMap.php @@ -101,6 +101,7 @@ class AttributeMap extends \SimpleSAML\Auth\ProcessingFilter throw new \Exception('Could not find attribute map file: ' . $filePath); } + /** @psalm-var mixed|null $attributemap */ $attributemap = null; include($filePath); if (!is_array($attributemap)) { diff --git a/modules/core/lib/Auth/Source/AdminPassword.php b/modules/core/lib/Auth/Source/AdminPassword.php index 5f2b8f7b5..943337255 100644 --- a/modules/core/lib/Auth/Source/AdminPassword.php +++ b/modules/core/lib/Auth/Source/AdminPassword.php @@ -25,9 +25,6 @@ class AdminPassword extends \SimpleSAML\Module\core\Auth\UserPassBase */ public function __construct(array $info, array $config) { - Assert::isArray($info); - Assert::isArray($config); - // Call the parent constructor first, as required by the interface parent::__construct($info, $config); @@ -50,9 +47,6 @@ class AdminPassword extends \SimpleSAML\Module\core\Auth\UserPassBase */ protected function login(string $username, string $password): array { - Assert::string($username); - Assert::string($password); - $config = Configuration::getInstance(); $adminPassword = $config->getString('auth.adminpassword', '123'); if ($adminPassword === '123') { diff --git a/modules/core/lib/Auth/UserPassBase.php b/modules/core/lib/Auth/UserPassBase.php index 4b2ac3452..14890d8de 100644 --- a/modules/core/lib/Auth/UserPassBase.php +++ b/modules/core/lib/Auth/UserPassBase.php @@ -100,11 +100,8 @@ abstract class UserPassBase extends \SimpleSAML\Auth\Source * @param array $info Information about this authentication source. * @param array &$config Configuration for this authentication source. */ - public function __construct($info, &$config) + public function __construct(array $info, array &$config) { - Assert::isArray($info); - Assert::isArray($config); - if (isset($config['core:loginpage_links'])) { $this->loginLinks = $config['core:loginpage_links']; } @@ -135,7 +132,7 @@ abstract class UserPassBase extends \SimpleSAML\Auth\Source * @param string|null $forcedUsername The forced username. * @return void */ - public function setForcedUsername($forcedUsername): void + public function setForcedUsername(?string $forcedUsername): void { Assert::nullOrString($forcedUsername); $this->forcedUsername = $forcedUsername; @@ -143,7 +140,7 @@ abstract class UserPassBase extends \SimpleSAML\Auth\Source /** * Return login links from configuration - * @return array + * @return string[] */ public function getLoginLinks(): array { @@ -238,7 +235,6 @@ abstract class UserPassBase extends \SimpleSAML\Auth\Source } $attributes = $this->login($username, $password); - Assert::isArray($attributes); $state['Attributes'] = $attributes; return; @@ -271,9 +267,9 @@ abstract class UserPassBase extends \SimpleSAML\Auth\Source * * @param string $username The username the user wrote. * @param string $password The password the user wrote. - * @return array Associative array with the user's attributes. + * @return array Associative array with the user's attributes. */ - abstract protected function login($username, $password): array; + abstract protected function login(string $username, string $password): array; /** @@ -288,7 +284,7 @@ abstract class UserPassBase extends \SimpleSAML\Auth\Source * @param string $password The password the user wrote. * @return void */ - public static function handleLogin(string $authStateId, string $username, string $password) + public static function handleLogin(string $authStateId, string $username, string $password): void { // Here we retrieve the state array we saved in the authenticate-function. /** @var array $state */ @@ -319,7 +315,6 @@ abstract class UserPassBase extends \SimpleSAML\Auth\Source Logger::stats('User \'' . $username . '\' successfully authenticated from ' . $_SERVER['REMOTE_ADDR']); // Save the attributes we received from the login-function in the $state-array - Assert::isArray($attributes); $state['Attributes'] = $attributes; // Return control to SimpleSAMLphp after successful authentication. diff --git a/modules/core/lib/Auth/UserPassOrgBase.php b/modules/core/lib/Auth/UserPassOrgBase.php index f8ba52fd0..b854a240d 100644 --- a/modules/core/lib/Auth/UserPassOrgBase.php +++ b/modules/core/lib/Auth/UserPassOrgBase.php @@ -99,11 +99,8 @@ abstract class UserPassOrgBase extends \SimpleSAML\Auth\Source * @param array $info Information about this authentication source. * @param array &$config Configuration for this authentication source. */ - public function __construct($info, &$config) + public function __construct(array $info, array &$config) { - Assert::isArray($info); - Assert::isArray($config); - // Call the parent constructor first, as required by the interface parent::__construct($info, $config); @@ -143,7 +140,7 @@ abstract class UserPassOrgBase extends \SimpleSAML\Auth\Source * @param string $usernameOrgMethod The method which should be used. * @return void */ - protected function setUsernameOrgMethod($usernameOrgMethod): void + protected function setUsernameOrgMethod(string $usernameOrgMethod): void { Assert::oneOf($usernameOrgMethod, ['none', 'allow', 'force']); diff --git a/modules/core/lib/Controller/Login.php b/modules/core/lib/Controller/Login.php index 99b13664e..e117cdc2c 100644 --- a/modules/core/lib/Controller/Login.php +++ b/modules/core/lib/Controller/Login.php @@ -199,7 +199,7 @@ class Login * @param Request $request The request that lead to this login operation. * @return void */ - public function cleardiscochoices(Request $request) + public function cleardiscochoices(Request $request): void { // The base path for cookies. This should be the installation directory for SimpleSAMLphp. $cookiePath = $this->config->getBasePath(); diff --git a/modules/core/lib/Stats/Output/File.php b/modules/core/lib/Stats/Output/File.php index a6562637a..df8307692 100644 --- a/modules/core/lib/Stats/Output/File.php +++ b/modules/core/lib/Stats/Output/File.php @@ -58,7 +58,7 @@ class File extends \SimpleSAML\Stats\Output * @param string $date The date for the log file. * @return void */ - private function openLog(string $date) + private function openLog(string $date): void { if ($this->file !== null && $this->file !== false) { fclose($this->file); diff --git a/modules/core/lib/Storage/SQLPermanentStorage.php b/modules/core/lib/Storage/SQLPermanentStorage.php index 541bb7117..dda3fa35c 100644 --- a/modules/core/lib/Storage/SQLPermanentStorage.php +++ b/modules/core/lib/Storage/SQLPermanentStorage.php @@ -28,7 +28,7 @@ class SQLPermanentStorage * @param \SimpleSAML\Configuration|null $config * @throws \Exception */ - public function __construct($name, $config = null) + public function __construct(string $name, Configuration $config = null) { if (is_null($config)) { $config = Configuration::getInstance(); @@ -78,7 +78,7 @@ class SQLPermanentStorage * @param int|null $duration * @return void */ - public function set($type, $key1, $key2, $value, $duration = null) + public function set(string $type, string $key1, string $key2, string $value, int $duration = null): void { if ($this->exists($type, $key1, $key2)) { $this->update($type, $key1, $key2, $value, $duration); @@ -140,12 +140,12 @@ class SQLPermanentStorage /** - * @param string $type - * @param string $key1 - * @param string $key2 + * @param string|null $type + * @param string|null $key1 + * @param string|null $key2 * @return array|null */ - public function get($type = null, $key1 = null, $key2 = null) + public function get(string $type = null, string $key1 = null, string $key2 = null): ?array { $conditions = $this->getCondition($type, $key1, $key2); $query = 'SELECT * FROM data WHERE ' . $conditions; @@ -165,12 +165,12 @@ class SQLPermanentStorage /** * Return the value directly (not in a container) * - * @param string $type - * @param string $key1 - * @param string $key2 - * @return array|null + * @param string|null $type + * @param string|null $key1 + * @param string|null $key2 + * @return string|null */ - public function getValue($type = null, $key1 = null, $key2 = null) + public function getValue(string $type = null, string $key1 = null, string $key2 = null): ?string { $res = $this->get($type, $key1, $key2); if ($res === null) { @@ -186,7 +186,7 @@ class SQLPermanentStorage * @param string $key2 * @return bool */ - public function exists($type, $key1, $key2) + public function exists(string $type, string $key1, string $key2): bool { $query = 'SELECT * FROM data WHERE type = :type AND key1 = :key1 AND key2 = :key2 LIMIT 1'; $prepared = $this->db->prepare($query); @@ -198,12 +198,12 @@ class SQLPermanentStorage /** - * @param string $type - * @param string $key1 - * @param string $key2 + * @param string|null $type + * @param string|null $key1 + * @param string|null $key2 * @return array|false */ - public function getList($type = null, $key1 = null, $key2 = null) + public function getList(string $type = null, string $key1 = null, string $key2 = null) { $conditions = $this->getCondition($type, $key1, $key2); $query = 'SELECT * FROM data WHERE ' . $conditions; @@ -223,15 +223,19 @@ class SQLPermanentStorage /** - * @param string $type - * @param string $key1 - * @param string $key2 + * @param string|null $type + * @param string|null $key1 + * @param string|null $key2 * @param string $whichKey * @throws \Exception * @return array|null */ - public function getKeys($type = null, $key1 = null, $key2 = null, $whichKey = 'type') - { + public function getKeys( + string $type = null, + string $key1 = null, + string $key2 = null, + string $whichKey = 'type' + ): ?array { if (!in_array($whichKey, ['key1', 'key2', 'type'], true)) { throw new \Exception('Invalid key type'); } @@ -254,13 +258,14 @@ class SQLPermanentStorage return $resarray; } + /** * @param string $type * @param string $key1 * @param string $key2 * @return bool */ - public function remove($type, $key1, $key2) + public function remove(string $type, string $key1, string $key2): bool { $query = 'DELETE FROM data WHERE type = :type AND key1 = :key1 AND key2 = :key2'; $prepared = $this->db->prepare($query); @@ -274,7 +279,7 @@ class SQLPermanentStorage /** * @return int */ - public function removeExpired() + public function removeExpired(): int { $query = "DELETE FROM data WHERE expire IS NOT NULL AND expire < :expire"; $prepared = $this->db->prepare($query); @@ -286,9 +291,9 @@ class SQLPermanentStorage /** * Create a SQL condition statement based on parameters * - * @param string $type - * @param string $key1 - * @param string $key2 + * @param string|null $type + * @param string|null $key1 + * @param string|null $key2 * @return string */ private function getCondition(string $type = null, string $key1 = null, string $key2 = null): string diff --git a/modules/cron/hooks/hook_configpage.php b/modules/cron/hooks/hook_configpage.php index 136888a00..cb4358299 100644 --- a/modules/cron/hooks/hook_configpage.php +++ b/modules/cron/hooks/hook_configpage.php @@ -6,7 +6,7 @@ * @param \SimpleSAML\XHTML\Template &$template The template that we should alter in this hook. * @return void */ -function cron_hook_configpage(\SimpleSAML\XHTML\Template &$template) +function cron_hook_configpage(\SimpleSAML\XHTML\Template &$template): void { $template->data['links']['cron'] = [ 'href' => SimpleSAML\Module::getModuleURL('cron/croninfo.php'), diff --git a/modules/cron/hooks/hook_cron.php b/modules/cron/hooks/hook_cron.php index 18b18847b..2211d7a40 100644 --- a/modules/cron/hooks/hook_cron.php +++ b/modules/cron/hooks/hook_cron.php @@ -8,9 +8,8 @@ use Webmozart\Assert\Assert; * @param array &$croninfo Output * @return void */ -function cron_hook_cron(&$croninfo) +function cron_hook_cron(array &$croninfo): void { - Assert::isArray($croninfo); Assert::keyExists($croninfo, 'summary'); Assert::keyExists($croninfo, 'tag'); diff --git a/modules/cron/hooks/hook_frontpage.php b/modules/cron/hooks/hook_frontpage.php index 3ca490313..4e34f505d 100644 --- a/modules/cron/hooks/hook_frontpage.php +++ b/modules/cron/hooks/hook_frontpage.php @@ -8,9 +8,8 @@ use Webmozart\Assert\Assert; * @param array &$links The links on the frontpage, split into sections. * @return void */ -function cron_hook_frontpage(&$links) +function cron_hook_frontpage(array &$links): void { - Assert::isArray($links); Assert::keyExists($links, 'links'); $links['config'][] = [ diff --git a/modules/cron/lib/Controller/Cron.php b/modules/cron/lib/Controller/Cron.php index 7654533f3..e3d1a0176 100644 --- a/modules/cron/lib/Controller/Cron.php +++ b/modules/cron/lib/Controller/Cron.php @@ -63,7 +63,7 @@ class Cron * @return \SimpleSAML\XHTML\Template * An HTML template or a redirection if we are not authenticated. */ - public function info() + public function info(): Template { Utils\Auth::requireAdmin(); @@ -100,14 +100,14 @@ class Cron * * @param string $tag The tag * @param string $key The secret key - * @param string|null $output The output format, defaulting to xhtml + * @param string $output The output format, defaulting to xhtml * * @return \SimpleSAML\XHTML\Template|\Symfony\Component\HttpFoundation\Response * An HTML template, a redirect or a "runnable" response. * * @throws \SimpleSAML\Error\Exception */ - public function run($tag, $key, $output) + public function run(string $tag, string $key, string $output = 'xhtml'): Response { $configKey = $this->cronconfig->getValue('key', 'secret'); if ($key !== $configKey) { @@ -116,7 +116,7 @@ class Cron } $cron = new \SimpleSAML\Module\cron\Cron(); - if ($tag === null || !$cron->isValidTag($tag)) { + if (!$cron->isValidTag($tag)) { Logger::error('Cron - Illegal tag [' . $tag . '].'); exit; } diff --git a/modules/cron/lib/Cron.php b/modules/cron/lib/Cron.php index 81b3641b6..128ddc8bc 100644 --- a/modules/cron/lib/Cron.php +++ b/modules/cron/lib/Cron.php @@ -38,7 +38,7 @@ class Cron * @return array the tag, and summary information from the run. * @throws \Exception If an invalid tag specified */ - public function runTag($tag) + public function runTag(string $tag): array { if (!$this->isValidTag($tag)) { throw new \Exception("Invalid cron tag '$tag''"); @@ -63,7 +63,7 @@ class Cron * @param string $tag * @return bool */ - public function isValidTag($tag) + public function isValidTag(string $tag): bool { if (!is_null($this->cronconfig->getValue('allowed_tags'))) { return in_array($tag, $this->cronconfig->getArray('allowed_tags'), true); diff --git a/modules/exampleauth/lib/Auth/Source/External.php b/modules/exampleauth/lib/Auth/Source/External.php index 966ab4b57..606479543 100644 --- a/modules/exampleauth/lib/Auth/Source/External.php +++ b/modules/exampleauth/lib/Auth/Source/External.php @@ -44,11 +44,8 @@ class External extends \SimpleSAML\Auth\Source * @param array $info Information about this authentication source. * @param array $config Configuration. */ - public function __construct($info, $config) + public function __construct(array $info, array $config) { - Assert::isArray($info); - Assert::isArray($config); - // Call the parent constructor first, as required by the interface parent::__construct($info, $config); @@ -109,8 +106,6 @@ class External extends \SimpleSAML\Auth\Source */ public function authenticate(array &$state): void { - Assert::isArray($state); - $attributes = $this->getUser(); if ($attributes !== null) { /* @@ -273,8 +268,6 @@ class External extends \SimpleSAML\Auth\Source */ public function logout(array &$state): void { - Assert::isArray($state); - if (!session_id()) { // session_start not called before. Do it here session_start(); diff --git a/modules/exampleauth/lib/Auth/Source/StaticSource.php b/modules/exampleauth/lib/Auth/Source/StaticSource.php index 21c86c697..75e6a57da 100644 --- a/modules/exampleauth/lib/Auth/Source/StaticSource.php +++ b/modules/exampleauth/lib/Auth/Source/StaticSource.php @@ -31,11 +31,8 @@ class StaticSource extends \SimpleSAML\Auth\Source * @param array $info Information about this authentication source. * @param array $config Configuration. */ - public function __construct($info, $config) + public function __construct(array $info, array $config) { - Assert::isArray($info); - Assert::isArray($config); - // Call the parent constructor first, as required by the interface parent::__construct($info, $config); @@ -57,7 +54,6 @@ class StaticSource extends \SimpleSAML\Auth\Source */ public function authenticate(array &$state): void { - Assert::isArray($state); $state['Attributes'] = $this->attributes; } } diff --git a/modules/exampleauth/lib/Auth/Source/UserPass.php b/modules/exampleauth/lib/Auth/Source/UserPass.php index 6d6849f23..e4e6b2188 100644 --- a/modules/exampleauth/lib/Auth/Source/UserPass.php +++ b/modules/exampleauth/lib/Auth/Source/UserPass.php @@ -35,11 +35,8 @@ class UserPass extends \SimpleSAML\Module\core\Auth\UserPassBase * @param array $info Information about this authentication source. * @param array $config Configuration. */ - public function __construct($info, $config) + public function __construct(array $info, array $config) { - Assert::isArray($info); - Assert::isArray($config); - // Call the parent constructor first, as required by the interface parent::__construct($info, $config); @@ -86,11 +83,8 @@ class UserPass extends \SimpleSAML\Module\core\Auth\UserPassBase * @param string $password The password the user wrote. * @return array Associative array with the users attributes. */ - protected function login($username, $password) + protected function login(string $username, string $password): array { - Assert::string($username); - Assert::string($password); - $userpass = $username . ':' . $password; if (!array_key_exists($userpass, $this->users)) { throw new Error\Error('WRONGUSERPASS'); diff --git a/modules/multiauth/lib/Auth/Source/MultiAuth.php b/modules/multiauth/lib/Auth/Source/MultiAuth.php index 5bad36250..009b9fb8c 100644 --- a/modules/multiauth/lib/Auth/Source/MultiAuth.php +++ b/modules/multiauth/lib/Auth/Source/MultiAuth.php @@ -59,11 +59,8 @@ class MultiAuth extends \SimpleSAML\Auth\Source * @param array $info Information about this authentication source. * @param array $config Configuration. */ - public function __construct($info, $config) + public function __construct(array $info, array $config) { - Assert::isArray($info); - Assert::isArray($config); - // Call the parent constructor first, as required by the interface parent::__construct($info, $config); @@ -141,8 +138,6 @@ class MultiAuth extends \SimpleSAML\Auth\Source */ public function authenticate(array &$state): void { - Assert::isArray($state); - $state[self::AUTHID] = $this->authId; $state[self::SOURCESID] = $this->sources; @@ -184,11 +179,8 @@ class MultiAuth extends \SimpleSAML\Auth\Source * @return void * @throws \Exception */ - public static function delegateAuthentication($authId, $state) + public static function delegateAuthentication(string $authId, array $state): void { - assert::string($authId); - Assert::isArray($state); - $as = Auth\Source::getById($authId); $valid_sources = array_map( /** @@ -236,8 +228,6 @@ class MultiAuth extends \SimpleSAML\Auth\Source */ public function logout(array &$state): void { - Assert::isArray($state); - // Get the source that was used to authenticate $session = Session::getSessionFromRequest(); $authId = $session->getData(self::SESSION_SOURCE, $this->authId); @@ -260,10 +250,8 @@ class MultiAuth extends \SimpleSAML\Auth\Source * @param string $source Name of the authentication source the user selected. * @return void */ - public function setPreviousSource($source) + public function setPreviousSource(string $source): void { - Assert::string($source); - $cookieName = 'multiauth_source_' . $this->authId; $config = Configuration::getInstance(); @@ -286,7 +274,7 @@ class MultiAuth extends \SimpleSAML\Auth\Source * last time or NULL if this is the first time or remembering is disabled. * @return string|null */ - public function getPreviousSource() + public function getPreviousSource(): ?string { $cookieName = 'multiauth_source_' . $this->authId; if (array_key_exists($cookieName, $_COOKIE)) { diff --git a/modules/portal/hooks/hook_htmlinject.php b/modules/portal/hooks/hook_htmlinject.php index d446a6ed6..acd88405c 100644 --- a/modules/portal/hooks/hook_htmlinject.php +++ b/modules/portal/hooks/hook_htmlinject.php @@ -8,9 +8,8 @@ use Webmozart\Assert\Assert; * @param array &$hookinfo hookinfo * @return void */ -function portal_hook_htmlinject(&$hookinfo) +function portal_hook_htmlinject(array &$hookinfo) { - Assert::isArray($hookinfo); Assert::keyExists($hookinfo, 'pre'); Assert::keyExists($hookinfo, 'post'); Assert::keyExists($hookinfo, 'page'); diff --git a/modules/portal/lib/Portal.php b/modules/portal/lib/Portal.php index e116c6e5a..f397953fe 100644 --- a/modules/portal/lib/Portal.php +++ b/modules/portal/lib/Portal.php @@ -21,7 +21,7 @@ class Portal * @param array $pages * @param array|null $config */ - public function __construct($pages, $config = null) + public function __construct(array $pages, array $config = null) { $this->pages = $pages; $this->config = $config; @@ -32,7 +32,7 @@ class Portal * @param string $thispage * @return array|null */ - public function getTabset($thispage) + public function getTabset(string $thispage): ?array { if (!isset($this->config)) { return null; @@ -50,7 +50,7 @@ class Portal * @param string $thispage * @return bool */ - public function isPortalized($thispage) + public function isPortalized(string $thispage): bool { if (!isset($this->config)) { return false; @@ -69,7 +69,7 @@ class Portal * @param string $thispage * @return string */ - public function getLoginInfo($translator, $thispage) + public function getLoginInfo(Translate $translator, string $thispage): string { $info = ['info' => '', 'translator' => $translator, 'thispage' => $thispage]; Module::callHooks('portalLoginInfo', $info); @@ -81,7 +81,7 @@ class Portal * @param string $thispage * @return string */ - public function getMenu($thispage) + public function getMenu(string $thispage): string { $config = Configuration::getInstance(); $t = new Translate($config); diff --git a/modules/saml/hooks/hook_metadata_hosted.php b/modules/saml/hooks/hook_metadata_hosted.php index 75ee581b2..38f35ccfe 100644 --- a/modules/saml/hooks/hook_metadata_hosted.php +++ b/modules/saml/hooks/hook_metadata_hosted.php @@ -8,10 +8,8 @@ use Webmozart\Assert\Assert; * @param array &$metadataHosted The metadata links for hosted metadata on the frontpage. * @return void */ -function saml_hook_metadata_hosted(&$metadataHosted) +function saml_hook_metadata_hosted(array &$metadataHosted) { - Assert::isArray($metadataHosted); - $sources = \SimpleSAML\Auth\Source::getSourcesOfType('saml:SP'); foreach ($sources as $source) { diff --git a/modules/saml/lib/Auth/Process/ExpectedAuthnContextClassRef.php b/modules/saml/lib/Auth/Process/ExpectedAuthnContextClassRef.php index cd64ac6ad..d33f6bad7 100644 --- a/modules/saml/lib/Auth/Process/ExpectedAuthnContextClassRef.php +++ b/modules/saml/lib/Auth/Process/ExpectedAuthnContextClassRef.php @@ -97,7 +97,7 @@ class ExpectedAuthnContextClassRef extends \SimpleSAML\Auth\ProcessingFilter * @param array $request * @return void */ - protected function unauthorized(&$request): void + protected function unauthorized(array &$request): void { Logger::error( 'ExpectedAuthnContextClassRef: Invalid authentication context: ' . strval($this->AuthnContextClassRef) . diff --git a/modules/saml/lib/Auth/Source/SP.php b/modules/saml/lib/Auth/Source/SP.php index 5dedebf51..eb9536602 100644 --- a/modules/saml/lib/Auth/Source/SP.php +++ b/modules/saml/lib/Auth/Source/SP.php @@ -71,11 +71,8 @@ class SP extends \SimpleSAML\Auth\Source * @param array $info Information about this authentication source. * @param array $config Configuration. */ - public function __construct($info, $config) + public function __construct(array $info, array $config) { - Assert::isArray($info); - Assert::isArray($config); - // Call the parent constructor first, as required by the interface parent::__construct($info, $config); @@ -107,7 +104,7 @@ class SP extends \SimpleSAML\Auth\Source * * @return string The metadata URL. */ - public function getMetadataURL() + public function getMetadataURL(): string { return Module::getModuleURL('saml/sp/metadata.php/' . urlencode($this->authId)); } @@ -118,7 +115,7 @@ class SP extends \SimpleSAML\Auth\Source * * @return string The entity id of this SP. */ - public function getEntityId() + public function getEntityId(): string { return $this->entityId; } @@ -129,7 +126,7 @@ class SP extends \SimpleSAML\Auth\Source * * @return array The metadata array for its use by a remote IdP. */ - public function getHostedMetadata() + public function getHostedMetadata(): array { $entityid = $this->getEntityId(); $metadata = [ @@ -286,10 +283,8 @@ class SP extends \SimpleSAML\Auth\Source * @param string $entityId The entity id of the IdP. * @return \SimpleSAML\Configuration The metadata of the IdP. */ - public function getIdPMetadata($entityId) + public function getIdPMetadata(string $entityId): Configuration { - Assert::string($entityId); - if ($this->idp !== null && $this->idp !== $entityId) { throw new Error\Exception('Cannot retrieve metadata for IdP ' . var_export($entityId, true) . ' because it isn\'t a valid IdP for this SP.'); @@ -316,7 +311,7 @@ class SP extends \SimpleSAML\Auth\Source * * @return \SimpleSAML\Configuration The metadata of this SP. */ - public function getMetadata() + public function getMetadata(): Configuration { return $this->metadata; } @@ -325,9 +320,9 @@ class SP extends \SimpleSAML\Auth\Source /** * Get a list with the protocols supported by this SP. * - * @return array + * @return string[] */ - public function getSupportedProtocols() + public function getSupportedProtocols(): array { return $this->protocols; } @@ -455,7 +450,7 @@ class SP extends \SimpleSAML\Auth\Source * @param array $state The state array for the current authentication. * @return void */ - private function startSSO2(Configuration $idpMetadata, array $state) + private function startSSO2(Configuration $idpMetadata, array $state): void { if (isset($state['saml:ProxyCount']) && $state['saml:ProxyCount'] < 0) { Auth\State::throwException( @@ -654,7 +649,7 @@ class SP extends \SimpleSAML\Auth\Source * @param \SAML2\AuthnRequest $ar The authentication request. * @return void */ - public function sendSAML2AuthnRequest(array &$state, Binding $binding, AuthnRequest $ar) + public function sendSAML2AuthnRequest(array &$state, Binding $binding, AuthnRequest $ar): void { $binding->send($ar); Assert::true(false); @@ -668,10 +663,8 @@ class SP extends \SimpleSAML\Auth\Source * @param array $state The state array for the current authentication. * @return void */ - public function startSSO($idp, array $state) + public function startSSO(string $idp, array $state): void { - Assert::string($idp); - $idpMetadata = $this->getIdPMetadata($idp); $type = $idpMetadata->getString('metadata-set'); @@ -692,7 +685,7 @@ class SP extends \SimpleSAML\Auth\Source * @param array $state The state array. * @return void */ - private function startDisco(array $state) + private function startDisco(array $state): void { $id = Auth\State::saveState($state, 'saml:sp:sso'); @@ -732,8 +725,6 @@ class SP extends \SimpleSAML\Auth\Source */ public function authenticate(array &$state): void { - Assert::isArray($state); - // We are going to need the authId in order to retrieve this authentication source later $state['saml:sp:AuthId'] = $this->authId; @@ -877,7 +868,7 @@ class SP extends \SimpleSAML\Auth\Source * @return void * @throws \SimpleSAML\Module\saml\Error\NoPassive In case the authentication request was passive. */ - public static function askForIdPChange(array &$state) + public static function askForIdPChange(array &$state): void { Assert::keyExists($state, 'saml:sp:IdPMetadata'); Assert::keyExists($state, 'saml:sp:AuthId'); @@ -908,7 +899,7 @@ class SP extends \SimpleSAML\Auth\Source * @param array $state The state array. * @return void */ - public static function reauthLogout(array $state) + public static function reauthLogout(array $state): void { Logger::debug('Proxy: logging the user out before re-authentication.'); @@ -929,7 +920,7 @@ class SP extends \SimpleSAML\Auth\Source * @param array $state The authentication state. * @return void */ - public static function reauthPostLogin(array $state) + public static function reauthPostLogin(array $state): void { Assert::keyExists($state, 'ReturnCallback'); @@ -953,7 +944,7 @@ class SP extends \SimpleSAML\Auth\Source * @param array &$state The state array with the state during logout. * @return void */ - public static function reauthPostLogout(IdP $idp, array $state) + public static function reauthPostLogout(IdP $idp, array $state): void { Assert::keyExists($state, 'saml:sp:AuthId'); @@ -978,9 +969,8 @@ class SP extends \SimpleSAML\Auth\Source * @param array $state The logout state. * @return void */ - public function startSLO2(&$state) + public function startSLO2(array &$state): void { - Assert::isArray($state); Assert::keyExists($state, 'saml:logout:IdP'); Assert::keyExists($state, 'saml:logout:NameID'); Assert::keyExists($state, 'saml:logout:SessionIndex'); @@ -1036,14 +1026,10 @@ class SP extends \SimpleSAML\Auth\Source */ public function logout(array &$state): void { - Assert::isArray($state); Assert::keyExists($state, 'saml:logout:Type'); $logoutType = $state['saml:logout:Type']; switch ($logoutType) { - case 'saml1': - // Nothing to do - return; case 'saml2': $this->startSLO2($state); return; @@ -1062,9 +1048,8 @@ class SP extends \SimpleSAML\Auth\Source * @param array $attributes The attributes. * @return void */ - public function handleResponse(array $state, $idp, array $attributes) + public function handleResponse(array $state, string $idp, array $attributes): void { - Assert::string($idp); Assert::keyExists($state, 'LogoutState'); Assert::keyExists($state, 'saml:logout:Type'); @@ -1107,10 +1092,8 @@ class SP extends \SimpleSAML\Auth\Source * @param string $idpEntityId The entity ID of the IdP. * @return void */ - public function handleLogout($idpEntityId) + public function handleLogout(string $idpEntityId): void { - Assert::string($idpEntityId); - /* Call the logout callback we registered in onProcessingCompleted(). */ $this->callLogoutCallback($idpEntityId); } @@ -1131,11 +1114,8 @@ class SP extends \SimpleSAML\Auth\Source * configuration directive for more information about allowing (or disallowing) URLs. * @return void */ - public static function handleUnsolicitedAuth($authId, array $state, $redirectTo) + public static function handleUnsolicitedAuth(string $authId, array $state, string $redirectTo): void { - Assert::string($authId); - Assert::string($redirectTo); - $session = Session::getSessionFromRequest(); $session->doLogin($authId, Auth\State::getPersistentAuthData($state)); @@ -1149,7 +1129,7 @@ class SP extends \SimpleSAML\Auth\Source * @param array $authProcState The processing chain state. * @return void */ - public static function onProcessingCompleted(array $authProcState) + public static function onProcessingCompleted(array $authProcState): void { Assert::keyExists($authProcState, 'saml:sp:IdP'); Assert::keyExists($authProcState, 'saml:sp:State'); diff --git a/modules/saml/lib/BaseNameIDGenerator.php b/modules/saml/lib/BaseNameIDGenerator.php index 0e7dc5c9c..15bb39770 100644 --- a/modules/saml/lib/BaseNameIDGenerator.php +++ b/modules/saml/lib/BaseNameIDGenerator.php @@ -78,7 +78,7 @@ abstract class BaseNameIDGenerator extends \SimpleSAML\Auth\ProcessingFilter * * @return string|null The NameID value. */ - abstract protected function getValue(array &$state); + abstract protected function getValue(array &$state): ?string; /** diff --git a/modules/saml/lib/Error.php b/modules/saml/lib/Error.php index e299117b2..9c28eab9e 100644 --- a/modules/saml/lib/Error.php +++ b/modules/saml/lib/Error.php @@ -47,12 +47,12 @@ class Error extends \SimpleSAML\Error\Exception * Can be NULL, in which case there is no status message. * @param \Exception|null $cause The cause of this exception. Can be NULL. */ - public function __construct($status, $subStatus = null, $statusMessage = null, \Exception $cause = null) - { - Assert::string($status); - Assert::nullOrString($subStatus); - Assert::nullOrString($statusMessage); - + public function __construct( + string $status, + string $subStatus = null, + string $statusMessage = null, + \Exception $cause = null + ) { $st = self::shortStatus($status); if ($subStatus !== null) { $st .= '/' . self::shortStatus($subStatus); @@ -73,7 +73,7 @@ class Error extends \SimpleSAML\Error\Exception * * @return string The top-level status code. */ - public function getStatus() + public function getStatus(): string { return $this->status; } @@ -84,7 +84,7 @@ class Error extends \SimpleSAML\Error\Exception * * @return string|null The second-level status code or NULL if no second-level status code is present. */ - public function getSubStatus() + public function getSubStatus(): ?string { return $this->subStatus; } @@ -95,7 +95,7 @@ class Error extends \SimpleSAML\Error\Exception * * @return string|null The status message or NULL if no status message is present. */ - public function getStatusMessage() + public function getStatusMessage(): ?string { return $this->statusMessage; } @@ -115,23 +115,6 @@ class Error extends \SimpleSAML\Error\Exception if ($exception instanceof \SimpleSAML\Module\saml\Error) { // Return the original exception unchanged return $exception; - - // TODO: remove this branch in 2.0 - } elseif ($exception instanceof \SimpleSAML\Error\NoPassive) { - $e = new self( - Constants::STATUS_RESPONDER, - Constants::STATUS_NO_PASSIVE, - $exception->getMessage(), - $exception - ); - // TODO: remove this branch in 2.0 - } elseif ($exception instanceof \SimpleSAML\Error\ProxyCountExceeded) { - $e = new self( - Constants::STATUS_RESPONDER, - Constants::STATUS_PROXY_COUNT_EXCEEDED, - $exception->getMessage(), - $exception - ); } else { $e = new self( \SAML2\Constants::STATUS_RESPONDER, @@ -156,7 +139,7 @@ class Error extends \SimpleSAML\Error\Exception * * @return \SimpleSAML\Error\Exception An exception representing this error. */ - public function toException() + public function toException(): \SimpleSAML\Error\Exception { $e = null; diff --git a/modules/saml/lib/Error/NoAuthnContext.php b/modules/saml/lib/Error/NoAuthnContext.php index 508da8e0c..f9a3a40b1 100644 --- a/modules/saml/lib/Error/NoAuthnContext.php +++ b/modules/saml/lib/Error/NoAuthnContext.php @@ -23,7 +23,7 @@ class NoAuthnContext extends \SimpleSAML\Module\saml\Error * @param string|null $message A short message explaining why this error happened. * @param \Exception|null $cause An exception that caused this error. */ - public function __construct($responsible, $message = null, \Exception $cause = null) + public function __construct(string $responsible, string $message = null, \Exception $cause = null) { parent::__construct($responsible, Constants::STATUS_NO_AUTHN_CONTEXT, $message, $cause); } diff --git a/modules/saml/lib/Error/NoAvailableIDP.php b/modules/saml/lib/Error/NoAvailableIDP.php index f1c66ab4b..f740aedc6 100644 --- a/modules/saml/lib/Error/NoAvailableIDP.php +++ b/modules/saml/lib/Error/NoAvailableIDP.php @@ -23,7 +23,7 @@ class NoAvailableIDP extends \SimpleSAML\Module\saml\Error * @param string|null $message A short message explaining why this error happened. * @param \Exception|null $cause An exception that caused this error. */ - public function __construct($responsible, $message = null, \Exception $cause = null) + public function __construct(string $responsible, string $message = null, \Exception $cause = null) { parent::__construct($responsible, Constants::STATUS_NO_AVAILABLE_IDP, $message, $cause); } diff --git a/modules/saml/lib/Error/NoPassive.php b/modules/saml/lib/Error/NoPassive.php index 3fc03e049..46ed47a3b 100644 --- a/modules/saml/lib/Error/NoPassive.php +++ b/modules/saml/lib/Error/NoPassive.php @@ -23,7 +23,7 @@ class NoPassive extends \SimpleSAML\Module\saml\Error * @param string|null $message A short message explaining why this error happened. * @param \Exception|null $cause An exception that caused this error. */ - public function __construct($responsible, $message = null, \Exception $cause = null) + public function __construct(string $responsible, string $message = null, \Exception $cause = null) { parent::__construct($responsible, Constants::STATUS_NO_PASSIVE, $message, $cause); } diff --git a/modules/saml/lib/Error/NoSupportedIDP.php b/modules/saml/lib/Error/NoSupportedIDP.php index 7806fb379..8e2a31dcd 100644 --- a/modules/saml/lib/Error/NoSupportedIDP.php +++ b/modules/saml/lib/Error/NoSupportedIDP.php @@ -23,7 +23,7 @@ class NoSupportedIDP extends \SimpleSAML\Module\saml\Error * @param string|null $message A short message explaining why this error happened. * @param \Exception|null $cause An exception that caused this error. */ - public function __construct($responsible, $message = null, \Exception $cause = null) + public function __construct(string $responsible, string $message = null, \Exception $cause = null) { parent::__construct($responsible, Constants::STATUS_NO_SUPPORTED_IDP, $message, $cause); } diff --git a/modules/saml/lib/Error/ProxyCountExceeded.php b/modules/saml/lib/Error/ProxyCountExceeded.php index 044aded1f..cfdbfb1e0 100644 --- a/modules/saml/lib/Error/ProxyCountExceeded.php +++ b/modules/saml/lib/Error/ProxyCountExceeded.php @@ -23,7 +23,7 @@ class ProxyCountExceeded extends \SimpleSAML\Module\saml\Error * @param string|null $message A short message explaining why this error happened. * @param \Exception|null $cause An exception that caused this error. */ - public function __construct($responsible, $message = null, \Exception $cause = null) + public function __construct(string $responsible, string $message = null, \Exception $cause = null) { parent::__construct($responsible, Constants::STATUS_PROXY_COUNT_EXCEEDED, $message, $cause); } diff --git a/modules/saml/lib/IdP/SAML2.php b/modules/saml/lib/IdP/SAML2.php index da474da24..89b035128 100644 --- a/modules/saml/lib/IdP/SAML2.php +++ b/modules/saml/lib/IdP/SAML2.php @@ -49,7 +49,7 @@ class SAML2 * @param array $state The authentication state. * @return void */ - public static function sendResponse(array $state) + public static function sendResponse(array $state): void { Assert::keyExists($state, 'saml:RequestId'); // Can be NULL Assert::keyExists($state, 'saml:RelayState'); // Can be NULL. @@ -127,7 +127,7 @@ class SAML2 * @param array $state The error state. * @return void */ - public static function handleAuthError(\SimpleSAML\Error\Exception $exception, array $state) + public static function handleAuthError(\SimpleSAML\Error\Exception $exception, array $state): void { Assert::keyExists($state, 'saml:RequestId'); // Can be NULL. Assert::keyExists($state, 'saml:RelayState'); // Can be NULL. @@ -200,7 +200,7 @@ class SAML2 string $AssertionConsumerServiceURL = null, string $ProtocolBinding = null, int $AssertionConsumerServiceIndex = null - ) { + ): ?array { /* We want to pick the best matching endpoint in the case where for example * only the ProtocolBinding is given. We therefore pick endpoints with the * following priority: @@ -277,7 +277,7 @@ class SAML2 * @return void * @throws \SimpleSAML\Error\BadRequest In case an error occurs when trying to receive the request. */ - public static function receiveAuthnRequest(\SimpleSAML\IdP $idp) + public static function receiveAuthnRequest(IdP $idp): void { $metadata = MetaDataStorageHandler::getMetadataHandler(); $idpMetadata = $idp->getConfig(); @@ -494,10 +494,8 @@ class SAML2 * @param string|null $relayState An id that should be carried across the logout. * @return void */ - public static function sendLogoutRequest(IdP $idp, array $association, $relayState) + public static function sendLogoutRequest(IdP $idp, array $association, string $relayState = null): void { - Assert::nullOrString($relayState); - Logger::info('Sending SAML 2.0 LogoutRequest to: ' . var_export($association['saml:entityID'], true)); $metadata = MetaDataStorageHandler::getMetadataHandler(); @@ -532,7 +530,7 @@ class SAML2 * @param array &$state The logout state array. * @return void */ - public static function sendLogoutResponse(IdP $idp, array $state) + public static function sendLogoutResponse(IdP $idp, array $state): void { Assert::keyExists($state, 'saml:RelayState'); // Can be NULL. Assert::notNull($state['saml:SPEntityId']); @@ -593,7 +591,7 @@ class SAML2 * @return void * @throws \SimpleSAML\Error\BadRequest In case an error occurs while trying to receive the logout message. */ - public static function receiveLogoutMessage(IdP $idp) + public static function receiveLogoutMessage(IdP $idp): void { $binding = Binding::getCurrentBinding(); $message = $binding->receive(); @@ -602,15 +600,8 @@ class SAML2 if ($issuer === null) { /* Without an issuer we have no way to respond to the message. */ throw new Error\BadRequest('Received message on logout endpoint without issuer.'); - } elseif ($issuer instanceof Issuer) { - /** @psalm-var string|null $spEntityId */ - $spEntityId = $issuer->getValue(); - if ($spEntityId === null) { - /* Without an issuer we have no way to respond to the message. */ - throw new Error\BadRequest('Received message on logout endpoint without issuer.'); - } } else { - $spEntityId = $issuer; + $spEntityId = $issuer->getValue(); } $metadata = MetaDataStorageHandler::getMetadataHandler(); @@ -676,10 +667,8 @@ class SAML2 * * @return string The logout URL. */ - public static function getLogoutURL(IdP $idp, array $association, $relayState) + public static function getLogoutURL(IdP $idp, array $association, string $relayState = null): string { - Assert::nullOrString($relayState); - Logger::info('Sending SAML 2.0 LogoutRequest to: ' . var_export($association['saml:entityID'], true)); $metadata = MetaDataStorageHandler::getMetadataHandler(); @@ -718,7 +707,7 @@ class SAML2 * * @return \SimpleSAML\Configuration Configuration object for the SP metadata. */ - public static function getAssociationConfig(IdP $idp, array $association) + public static function getAssociationConfig(IdP $idp, array $association): Configuration { $metadata = MetaDataStorageHandler::getMetadataHandler(); try { @@ -739,7 +728,7 @@ class SAML2 * @throws \SimpleSAML\Error\Exception * @throws \SimpleSAML\Error\MetadataNotFound */ - public static function getHostedMetadata($entityid) + public static function getHostedMetadata(string $entityid): array { $handler = MetaDataStorageHandler::getMetadataHandler(); $config = $handler->getMetaDataConfig($entityid, 'saml20-idp-hosted'); @@ -942,7 +931,7 @@ class SAML2 Configuration $idpMetadata, Configuration $spMetadata, array &$state - ) { + ): ?string { $attribute = $spMetadata->getString('simplesaml.nameidattribute', null); if ($attribute === null) { $attribute = $idpMetadata->getString('simplesaml.nameidattribute', null); diff --git a/modules/saml/lib/IdP/SQLNameID.php b/modules/saml/lib/IdP/SQLNameID.php index 37a3a2899..1d810040b 100644 --- a/modules/saml/lib/IdP/SQLNameID.php +++ b/modules/saml/lib/IdP/SQLNameID.php @@ -178,12 +178,13 @@ class SQLNameID * @param array $config * @return void */ - public static function add($idpEntityId, $spEntityId, $user, $value, array $config = []) - { - Assert::string($idpEntityId); - Assert::string($spEntityId); - Assert::string($user); - Assert::string($value); + public static function add( + string $idpEntityId, + string $spEntityId, + string $user, + string $value, + array $config = [] + ): void { $params = [ '_idp' => $idpEntityId, @@ -207,12 +208,12 @@ class SQLNameID * @param array $config * @return string|null $value The NameID value, or NULL of no NameID value was found. */ - public static function get($idpEntityId, $spEntityId, $user, array $config = []) - { - Assert::string($idpEntityId); - Assert::string($spEntityId); - Assert::string($user); - + public static function get( + string $idpEntityId, + string $spEntityId, + string $user, + array $config = [] + ): ?string { $params = [ '_idp' => $idpEntityId, '_sp' => $spEntityId, @@ -229,7 +230,7 @@ class SQLNameID return null; } - return $row['_value']; + return strval($row['_value']); } @@ -242,12 +243,12 @@ class SQLNameID * @param array $config * @return void */ - public static function delete($idpEntityId, $spEntityId, $user, array $config = []) - { - Assert::string($idpEntityId); - Assert::string($spEntityId); - assert::string($user); - + public static function delete( + string $idpEntityId, + string $spEntityId, + string $user, + array $config = [] + ): void { $params = [ '_idp' => $idpEntityId, '_sp' => $spEntityId, @@ -268,11 +269,8 @@ class SQLNameID * @param array $config * @return array Array of userid => NameID. */ - public static function getIdentities($idpEntityId, $spEntityId, array $config = []) + public static function getIdentities(string $idpEntityId, string $spEntityId, array $config = []): array { - Assert::string($idpEntityId); - assert::string($spEntityId); - $params = [ '_idp' => $idpEntityId, '_sp' => $spEntityId, diff --git a/modules/saml/lib/Message.php b/modules/saml/lib/Message.php index 7027e4f7e..c5194aeca 100644 --- a/modules/saml/lib/Message.php +++ b/modules/saml/lib/Message.php @@ -43,7 +43,7 @@ class Message Configuration $srcMetadata, Configuration $dstMetadata, SignedElement $element - ) { + ): void { $dstPrivateKey = $dstMetadata->getString('signature.privatekey', null); if ($dstPrivateKey !== null) { @@ -95,8 +95,7 @@ class Message Configuration $srcMetadata, Configuration $dstMetadata, \SAML2\Message $message - ) { - + ): void { $signingEnabled = null; if ($message instanceof LogoutRequest || $message instanceof LogoutResponse) { $signingEnabled = $srcMetadata->getBoolean('sign.logout', null); @@ -171,7 +170,7 @@ class Message * @throws \SimpleSAML\Error\Exception if there is not certificate in the metadata for the entity. * @throws \Exception if the signature validation fails with an exception. */ - public static function checkSign(Configuration $srcMetadata, SignedElement $element) + public static function checkSign(Configuration $srcMetadata, SignedElement $element): bool { // find the public key that should verify signatures by this entity $keys = $srcMetadata->getPublicKeys('signing'); @@ -239,7 +238,7 @@ class Message Configuration $srcMetadata, Configuration $dstMetadata, \SAML2\Message $message - ) { + ): void { $enabled = null; if ($message instanceof LogoutRequest || $message instanceof LogoutResponse) { $enabled = $srcMetadata->getBoolean('validate.logout', null); @@ -283,7 +282,7 @@ class Message public static function getDecryptionKeys( Configuration $srcMetadata, Configuration $dstMetadata - ) { + ): array { $sharedKey = $srcMetadata->getString('sharedkey', null); if ($sharedKey !== null) { $key = new XMLSecurityKey(XMLSecurityKey::AES128_CBC); @@ -338,7 +337,7 @@ class Message public static function getBlacklistedAlgorithms( Configuration $srcMetadata, Configuration $dstMetadata - ) { + ): array { $blacklist = $srcMetadata->getArray('encryption.blacklisted-algorithms', null); if ($blacklist === null) { $blacklist = $dstMetadata->getArray('encryption.blacklisted-algorithms', [XMLSecurityKey::RSA_1_5]); @@ -424,7 +423,7 @@ class Message Configuration $srcMetadata, Configuration $dstMetadata, Assertion &$assertion - ) { + ): void { if (!$assertion->hasEncryptedAttributes()) { return; } @@ -461,7 +460,7 @@ class Message * * @return \SimpleSAML\Module\saml\Error The error. */ - public static function getResponseError(StatusResponse $response) + public static function getResponseError(StatusResponse $response): \SimpleSAML\Module\saml\Error { $status = $response->getStatus(); return new \SimpleSAML\Module\saml\Error($status['Code'], $status['SubCode'], $status['Message']); @@ -478,7 +477,7 @@ class Message public static function buildAuthnRequest( Configuration $spMetadata, Configuration $idpMetadata - ) { + ): AuthnRequest { $ar = new AuthnRequest(); // get the NameIDPolicy to apply. IdP metadata has precedence. @@ -540,7 +539,7 @@ class Message public static function buildLogoutRequest( Configuration $srcMetadata, Configuration $dstMetadata - ) { + ): LogoutRequest { $lr = new LogoutRequest(); $issuer = new Issuer(); $issuer->setValue($srcMetadata->getString('entityid')); @@ -563,7 +562,7 @@ class Message public static function buildLogoutResponse( Configuration $srcMetadata, Configuration $dstMetadata - ) { + ): LogoutResponse { $lr = new LogoutResponse(); $issuer = new Issuer(); $issuer->setValue($srcMetadata->getString('entityid')); @@ -594,7 +593,7 @@ class Message Configuration $spMetadata, Configuration $idpMetadata, Response $response - ) { + ): array { if (!$response->isSuccess()) { throw self::getResponseError($response); } @@ -891,7 +890,7 @@ class Message * * @throws \SimpleSAML\Error\Exception if there is no supported encryption key in the metadata of this entity. */ - public static function getEncryptionKey(Configuration $metadata) + public static function getEncryptionKey(Configuration $metadata): XMLSecurityKey { $sharedKey = $metadata->getString('sharedkey', null); if ($sharedKey !== null) { diff --git a/modules/saml/lib/SP/LogoutStore.php b/modules/saml/lib/SP/LogoutStore.php index 8b941acd9..a229d43bd 100644 --- a/modules/saml/lib/SP/LogoutStore.php +++ b/modules/saml/lib/SP/LogoutStore.php @@ -26,7 +26,7 @@ class LogoutStore * @param \SimpleSAML\Store\SQL $store The datastore. * @return void */ - private static function createLogoutTable(Store\SQL $store) + private static function createLogoutTable(Store\SQL $store): void { $tableVer = $store->getTableVersion('saml_LogoutStore'); if ($tableVer === 4) { @@ -201,7 +201,7 @@ class LogoutStore * @param \SimpleSAML\Store\SQL $store The datastore. * @return void */ - private static function cleanLogoutStore(Store\SQL $store) + private static function cleanLogoutStore(Store\SQL $store): void { Logger::debug('saml.LogoutStore: Cleaning logout store.'); @@ -231,7 +231,7 @@ class LogoutStore string $sessionIndex, int $expire, string $sessionId - ) { + ): void { self::createLogoutTable($store); if (rand(0, 1000) < 10) { @@ -330,12 +330,8 @@ class LogoutStore * @param int $expire * @return void */ - public static function addSession($authId, $nameId, $sessionIndex, $expire) + public static function addSession(string $authId, NameID $nameId, ?string $sessionIndex, int $expire): void { - Assert::string($authId); - Assert::nullorString($sessionIndex); - Assert::integer($expire); - $session = Session::getSessionFromRequest(); if ($session->isTransient()) { // transient sessions are useless for this purpose, nothing to do @@ -385,10 +381,8 @@ class LogoutStore * @param array $sessionIndexes The SessionIndexes we should log out of. Logs out of all if this is empty. * @return int|false Number of sessions logged out, or FALSE if not supported. */ - public static function logoutSessions($authId, $nameId, array $sessionIndexes) + public static function logoutSessions(string $authId, NameID $nameId, array $sessionIndexes) { - Assert::string($authId); - $store = Store::getInstance(); if ($store === false) { // We don't have a datastore diff --git a/tests/Utils/SpTester.php b/tests/Utils/SpTester.php index 9580819c5..bca15ff61 100644 --- a/tests/Utils/SpTester.php +++ b/tests/Utils/SpTester.php @@ -21,7 +21,7 @@ class SpTester extends \SimpleSAML\Module\saml\Auth\Source\SP * @param array $config * @return void */ - public function __construct($info, $config) + public function __construct(array $info, array $config) { parent::__construct($info, $config); } @@ -30,7 +30,7 @@ class SpTester extends \SimpleSAML\Module\saml\Auth\Source\SP /** * @return void */ - public function startSSO2Test(Configuration $idpMetadata, array $state) + public function startSSO2Test(Configuration $idpMetadata, array $state): void { $reflector = new ReflectionObject($this); $method = $reflector->getMethod('startSSO2'); @@ -43,7 +43,7 @@ class SpTester extends \SimpleSAML\Module\saml\Auth\Source\SP * override the method that sends the request to avoid sending anything * @return void */ - public function sendSAML2AuthnRequest(array &$state, Binding $binding, AuthnRequest $ar) + public function sendSAML2AuthnRequest(array &$state, Binding $binding, AuthnRequest $ar): void { // Exit test. Continuing would mean running into a assert(FALSE) throw new ExitTestException( diff --git a/tests/lib/SimpleSAML/DatabaseTest.php b/tests/lib/SimpleSAML/DatabaseTest.php index eea953043..95b4671e2 100644 --- a/tests/lib/SimpleSAML/DatabaseTest.php +++ b/tests/lib/SimpleSAML/DatabaseTest.php @@ -72,7 +72,6 @@ class DatabaseTest extends TestCase $this->config = new Configuration($config, "test/SimpleSAML/DatabaseTest.php"); // Ensure that we have a functional configuration class - $this->assertInstanceOf(Configuration::class, $this->config); $this->assertEquals($config['database.dsn'], $this->config->getString('database.dsn')); $this->db = Database::getInstance($this->config); -- GitLab