From d8dc69f97b2437890760eef87b5703675953326a Mon Sep 17 00:00:00 2001 From: Tim van Dijen <tvdijen@gmail.com> Date: Sat, 15 Feb 2020 19:19:32 +0100 Subject: [PATCH] Misses & mistakes --- lib/SimpleSAML/Auth/AuthenticationFactory.php | 4 + lib/SimpleSAML/Command/RouterDebugCommand.php | 10 ++- lib/SimpleSAML/Configuration.php | 17 ++--- lib/SimpleSAML/Database.php | 2 +- lib/SimpleSAML/Error/AuthSource.php | 1 - lib/SimpleSAML/Error/Error.php | 1 + lib/SimpleSAML/Error/Exception.php | 7 +- lib/SimpleSAML/Error/NotFound.php | 2 +- lib/SimpleSAML/IdP.php | 1 + lib/SimpleSAML/Kernel.php | 22 +++--- lib/SimpleSAML/Locale/Language.php | 2 +- lib/SimpleSAML/Locale/Localization.php | 8 +- lib/SimpleSAML/Locale/Translate.php | 17 +---- lib/SimpleSAML/Logger.php | 3 +- lib/SimpleSAML/Memcache.php | 4 +- .../Metadata/MetaDataStorageHandler.php | 2 +- .../Metadata/MetaDataStorageSource.php | 4 +- lib/SimpleSAML/Metadata/SAMLBuilder.php | 73 +------------------ lib/SimpleSAML/Metadata/SAMLParser.php | 14 ++-- lib/SimpleSAML/Metadata/Signer.php | 10 +-- lib/SimpleSAML/Metadata/Sources/MDQ.php | 2 +- lib/SimpleSAML/Module.php | 28 ++++--- lib/SimpleSAML/Session.php | 18 ++--- lib/SimpleSAML/Store/Redis.php | 1 + lib/SimpleSAML/Store/SQL.php | 5 +- lib/SimpleSAML/Utils/Arrays.php | 2 +- lib/SimpleSAML/Utils/EMail.php | 59 ++++----------- lib/SimpleSAML/Utils/HTTP.php | 2 +- lib/SimpleSAML/Utils/System.php | 2 +- lib/SimpleSAML/Utils/XML.php | 14 +--- lib/SimpleSAML/XHTML/IdPDisco.php | 4 +- lib/SimpleSAML/XHTML/Template.php | 53 ++++---------- .../core/lib/Auth/Source/AdminPassword.php | 4 +- tests/Utils/TestAuthSource.php | 3 +- tests/Utils/TestAuthSourceFactory.php | 3 +- .../SimpleSAML/Locale/LocalizationTest.php | 10 +-- .../modules/core/lib/Controller/LoginTest.php | 1 - 37 files changed, 118 insertions(+), 297 deletions(-) diff --git a/lib/SimpleSAML/Auth/AuthenticationFactory.php b/lib/SimpleSAML/Auth/AuthenticationFactory.php index 79a22c06e..e59bac61b 100644 --- a/lib/SimpleSAML/Auth/AuthenticationFactory.php +++ b/lib/SimpleSAML/Auth/AuthenticationFactory.php @@ -19,6 +19,10 @@ class AuthenticationFactory protected $session; + /** + * @param \SimpleSAML\Configuration $config + * @param \SimpleSAML\Session $session + */ public function __construct(Configuration $config, Session $session) { $this->config = $config; diff --git a/lib/SimpleSAML/Command/RouterDebugCommand.php b/lib/SimpleSAML/Command/RouterDebugCommand.php index 2fba9f6a3..d1bfab227 100644 --- a/lib/SimpleSAML/Command/RouterDebugCommand.php +++ b/lib/SimpleSAML/Command/RouterDebugCommand.php @@ -26,7 +26,9 @@ class RouterDebugCommand extends Command /** - * {@inheritdoc} + * @param Symfony\Component\Routing\RouterInterface + * + * @throws \Symfony\Component\Console\Exception\LogicException When the command name is empty */ public function __construct(RouterInterface $router) { @@ -36,7 +38,6 @@ class RouterDebugCommand extends Command /** - * {@inheritDoc} * @return void */ protected function configure() @@ -55,8 +56,9 @@ EOF /** - * {@inheritdoc} - * @psalm-suppress InvalidReturnType + * @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 */ protected function execute(InputInterface $input, OutputInterface $output) { diff --git a/lib/SimpleSAML/Configuration.php b/lib/SimpleSAML/Configuration.php index 7cf763f94..5059487a9 100644 --- a/lib/SimpleSAML/Configuration.php +++ b/lib/SimpleSAML/Configuration.php @@ -24,7 +24,6 @@ class Configuration implements Utils\ClearableState */ const REQUIRED_OPTION = '___REQUIRED_OPTION___'; - /** * Associative array with mappings from instance-names to configuration objects. * @@ -32,7 +31,6 @@ class Configuration implements Utils\ClearableState */ private static $instance = []; - /** * Configuration directories. * @@ -43,7 +41,6 @@ class Configuration implements Utils\ClearableState */ private static $configDirs = []; - /** * Cache of loaded configuration files. * @@ -53,7 +50,6 @@ class Configuration implements Utils\ClearableState */ private static $loadedConfigs = []; - /** * The configuration array. * @@ -61,7 +57,6 @@ class Configuration implements Utils\ClearableState */ private $configuration; - /** * The location which will be given when an error occurs. * @@ -69,7 +64,6 @@ class Configuration implements Utils\ClearableState */ private $location; - /** * The file this configuration was loaded from. * @@ -77,7 +71,6 @@ class Configuration implements Utils\ClearableState */ private $filename = null; - /** * Temporary property that tells if the deprecated getBaseURL() method has been called or not. * @@ -865,11 +858,11 @@ class Configuration implements Utils\ClearableState * This function will only return null if $default is set to null and the option * doesn't exist. * - * @return mixed 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. */ - public function getConfigItem(string $name, $default = []) + public function getConfigItem(string $name, $default = []): ?Configuration { $ret = $this->getValue($name, $default); @@ -896,7 +889,7 @@ class Configuration implements Utils\ClearableState * This function returns the name of all options which are defined in this * configuration file, as an array of strings. * - * @return array Name of all options defined in this configuration file. + * @return string[] Name of all options defined in this configuration file. */ public function getOptions(): array { @@ -1025,7 +1018,7 @@ class Configuration implements Utils\ClearableState * Find an endpoint of the given type, using a list of supported bindings as a way to prioritize. * * @param string $endpointType The endpoint type. - * @param array $bindings Sorted array of acceptable bindings. + * @param string[] $bindings Sorted array of acceptable bindings. * @param mixed $default The default value to return if no matching endpoint is found. If no default is provided, * an exception will be thrown. * @@ -1061,7 +1054,7 @@ class Configuration implements Utils\ClearableState * Find the default endpoint of the given type. * * @param string $endpointType The endpoint type. - * @param array $bindings Array with acceptable bindings. Can be null if any binding is allowed. + * @param string[]|null $bindings Array with acceptable bindings. Can be null if any binding is allowed. * @param mixed $default The default value to return if no matching endpoint is found. If no default is provided, * an exception will be thrown. * diff --git a/lib/SimpleSAML/Database.php b/lib/SimpleSAML/Database.php index c0a5dcf3d..e5604ca5e 100644 --- a/lib/SimpleSAML/Database.php +++ b/lib/SimpleSAML/Database.php @@ -271,7 +271,7 @@ class Database * * @return \PDOStatement object */ - public function read(string $stmt, array $params = []) + public function read(string $stmt, array $params = []): PDOStatement { $db = $this->getSlave(); diff --git a/lib/SimpleSAML/Error/AuthSource.php b/lib/SimpleSAML/Error/AuthSource.php index 3e8d589be..b836b93b8 100644 --- a/lib/SimpleSAML/Error/AuthSource.php +++ b/lib/SimpleSAML/Error/AuthSource.php @@ -22,7 +22,6 @@ class AuthSource extends Error */ private $authsource; - /** * Reason why this request was invalid. * @var string diff --git a/lib/SimpleSAML/Error/Error.php b/lib/SimpleSAML/Error/Error.php index 6dee40ac0..ab9bd0258 100644 --- a/lib/SimpleSAML/Error/Error.php +++ b/lib/SimpleSAML/Error/Error.php @@ -69,6 +69,7 @@ class Error extends Exception */ protected $includeTemplate = null; + /** * Constructor for this error. * diff --git a/lib/SimpleSAML/Error/Exception.php b/lib/SimpleSAML/Error/Exception.php index 1a2cfc37b..516643ff8 100644 --- a/lib/SimpleSAML/Error/Exception.php +++ b/lib/SimpleSAML/Error/Exception.php @@ -29,7 +29,6 @@ class Exception extends \Exception */ private $backtrace = []; - /** * The cause of this exception. * @@ -111,7 +110,7 @@ class Exception extends \Exception /** * Retrieve the backtrace. * - * @return array An array where each function call is a single item. + * @return string[] An array where each function call is a single item. */ public function getBacktrace(): array { @@ -148,7 +147,7 @@ class Exception extends \Exception * * @param boolean $anonymize Whether the resulting messages should be anonymized or not. * - * @return array Log lines that should be written out. + * @return string[] Log lines that should be written out. */ public function format(bool $anonymize = false): array { @@ -166,7 +165,7 @@ class Exception extends \Exception * * @param boolean $anonymize Whether the resulting messages should be anonymized or not. * - * @return array All lines of the backtrace, properly formatted. + * @return string[] All lines of the backtrace, properly formatted. */ public function formatBacktrace(bool $anonymize = false): array { diff --git a/lib/SimpleSAML/Error/NotFound.php b/lib/SimpleSAML/Error/NotFound.php index f7a37dfc5..216f007b9 100644 --- a/lib/SimpleSAML/Error/NotFound.php +++ b/lib/SimpleSAML/Error/NotFound.php @@ -64,7 +64,7 @@ class NotFound extends Error * * @param bool $anonymize Whether to anonymize the trace or not. * - * @return array + * @return string[] */ public function format(bool $anonymize = false): array { diff --git a/lib/SimpleSAML/IdP.php b/lib/SimpleSAML/IdP.php index 7eebc6ecb..430eb37b3 100644 --- a/lib/SimpleSAML/IdP.php +++ b/lib/SimpleSAML/IdP.php @@ -63,6 +63,7 @@ class IdP */ private $authSource; + /** * Initialize an IdP. * diff --git a/lib/SimpleSAML/Kernel.php b/lib/SimpleSAML/Kernel.php index 2127d870d..db6cd3ede 100644 --- a/lib/SimpleSAML/Kernel.php +++ b/lib/SimpleSAML/Kernel.php @@ -24,16 +24,14 @@ class Kernel extends BaseKernel const CONFIG_EXTS = '.{php,xml,yaml,yml}'; - /** - * @var string - */ + /** @var string */ private $module; /** * @param string $module */ - public function __construct($module) + public function __construct(string $module) { $this->module = $module; @@ -46,7 +44,7 @@ class Kernel extends BaseKernel /** * @return string */ - public function getCacheDir() + public function getCacheDir(): string { $configuration = Configuration::getInstance(); $cachePath = $configuration->getString('tempdir') . '/cache/' . $this->module; @@ -62,7 +60,7 @@ class Kernel extends BaseKernel /** * @return string */ - public function getLogDir() + public function getLogDir(): string { $configuration = Configuration::getInstance(); $loggingPath = $configuration->getString('loggingdir'); @@ -76,9 +74,9 @@ class Kernel extends BaseKernel /** - * {@inheritdoc} + * @return array */ - public function registerBundles() + public function registerBundles(): array { return [ new FrameworkBundle(), @@ -91,7 +89,7 @@ class Kernel extends BaseKernel * * @return string */ - public function getModule() + public function getModule(): string { return $this->module; } @@ -104,7 +102,7 @@ class Kernel extends BaseKernel * @param LoaderInterface $loader * @return void */ - protected function configureContainer(ContainerBuilder $container, LoaderInterface $loader) + protected function configureContainer(ContainerBuilder $container, LoaderInterface $loader): void { $configuration = Configuration::getInstance(); $baseDir = $configuration->getBaseDir(); @@ -128,7 +126,7 @@ class Kernel extends BaseKernel * @param RouteCollectionBuilder $routes * @return void */ - protected function configureRoutes(RouteCollectionBuilder $routes) + protected function configureRoutes(RouteCollectionBuilder $routes): void { $configuration = Configuration::getInstance(); $baseDir = $configuration->getBaseDir(); @@ -144,7 +142,7 @@ class Kernel extends BaseKernel * @param ContainerBuilder $container * @return void */ - private function registerModuleControllers(ContainerBuilder $container) + private function registerModuleControllers(ContainerBuilder $container): void { try { $definition = new Definition(); diff --git a/lib/SimpleSAML/Locale/Language.php b/lib/SimpleSAML/Locale/Language.php index aa8133e65..c457378dd 100644 --- a/lib/SimpleSAML/Locale/Language.php +++ b/lib/SimpleSAML/Locale/Language.php @@ -163,7 +163,7 @@ class Language /** * Filter configured (available) languages against installed languages. * - * @return array The set of languages both in 'language.available' and self::$language_names. + * @return string[] The set of languages both in 'language.available' and self::$language_names. */ private function getInstalledLanguages(): array { diff --git a/lib/SimpleSAML/Locale/Localization.php b/lib/SimpleSAML/Locale/Localization.php index 411116441..b00b85f54 100644 --- a/lib/SimpleSAML/Locale/Localization.php +++ b/lib/SimpleSAML/Locale/Localization.php @@ -89,6 +89,7 @@ class Localization */ public $i18nBackend; + /** * Constructor * @@ -102,11 +103,7 @@ class Localization $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 - ); + $this->i18nBackend = self::GETTEXT_I18N_BACKEND; $this->setupL10N(); } @@ -301,6 +298,7 @@ class Localization $this->addDomain($this->localeDir, self::DEFAULT_DOMAIN); } + /** * Show which domains are registered * diff --git a/lib/SimpleSAML/Locale/Translate.php b/lib/SimpleSAML/Locale/Translate.php index 1a03e2772..e2d2fe7d2 100644 --- a/lib/SimpleSAML/Locale/Translate.php +++ b/lib/SimpleSAML/Locale/Translate.php @@ -55,6 +55,7 @@ class Translate */ private $language; + /** * Constructor * @@ -65,21 +66,7 @@ class Translate { $this->configuration = $configuration; $this->language = new Language($configuration); - - if ($defaultDictionary !== null && substr($defaultDictionary, -4) === '.php') { - // TODO: drop this entire if clause for 2.0 - // for backwards compatibility - print warning - $backtrace = debug_backtrace(); - $where = $backtrace[0]['file'] . ':' . $backtrace[0]['line']; - Logger::warning( - 'Deprecated use of new SimpleSAML\Locale\Translate(...) at ' . $where . - '. The last parameter is now a dictionary name, which should not end in ".php".' - ); - - $this->defaultDictionary = substr($defaultDictionary, 0, -4); - } else { - $this->defaultDictionary = $defaultDictionary; - } + $this->defaultDictionary = $defaultDictionary; } diff --git a/lib/SimpleSAML/Logger.php b/lib/SimpleSAML/Logger.php index 0142a21f1..0805dd328 100644 --- a/lib/SimpleSAML/Logger.php +++ b/lib/SimpleSAML/Logger.php @@ -72,7 +72,6 @@ class Logger */ private static $logMask = 0; - /** * This constant defines the string we set the track ID to while we are fetching the track ID from the session * class. This is used to prevent infinite recursion. @@ -307,7 +306,7 @@ class Logger * * @return void */ - public static function flush() + public static function flush(): void { foreach (self::$earlyLog as $msg) { self::log($msg['level'], $msg['string'], $msg['statsLog']); diff --git a/lib/SimpleSAML/Memcache.php b/lib/SimpleSAML/Memcache.php index ac5085e54..7eff621f7 100644 --- a/lib/SimpleSAML/Memcache.php +++ b/lib/SimpleSAML/Memcache.php @@ -33,7 +33,6 @@ class Memcache */ private static $serverGroups = null; - /** * The flavor of memcache PHP extension we are using. * @@ -41,7 +40,6 @@ class Memcache */ private static $extension = ''; - /** * Find data stored with a given key. * @@ -220,7 +218,7 @@ class Memcache * * @throws \Exception If any configuration option for the server is invalid. */ - private static function addMemcacheServer($memcache, array $server): void + private static function addMemcacheServer(\Memcached $memcache, array $server): void { // the hostname option is required if (!array_key_exists('hostname', $server)) { diff --git a/lib/SimpleSAML/Metadata/MetaDataStorageHandler.php b/lib/SimpleSAML/Metadata/MetaDataStorageHandler.php index 6d3e4b1c9..ac8407ce3 100644 --- a/lib/SimpleSAML/Metadata/MetaDataStorageHandler.php +++ b/lib/SimpleSAML/Metadata/MetaDataStorageHandler.php @@ -262,7 +262,7 @@ class MetaDataStorageHandler implements \SimpleSAML\Utils\ClearableState /** * This function loads the metadata for entity IDs in $entityIds. It is returned as an associative array * where the key is the entity id. An empty array may be returned if no matching entities were found - * @param array $entityIds The entity ids to load + * @param string[] $entityIds The entity ids to load * @param string $set The set we want to get metadata from. * @return array An associative array with the metadata for the requested entities, if found. */ diff --git a/lib/SimpleSAML/Metadata/MetaDataStorageSource.php b/lib/SimpleSAML/Metadata/MetaDataStorageSource.php index a36807ca4..875a488cc 100644 --- a/lib/SimpleSAML/Metadata/MetaDataStorageSource.php +++ b/lib/SimpleSAML/Metadata/MetaDataStorageSource.php @@ -244,7 +244,7 @@ abstract class MetaDataStorageSource * where the key is the entity id. An empty array may be returned if no matching entities were found. * Subclasses should override if their getMetadataSet returns nothing or is slow. Subclasses may want to * delegate to getMetaDataForEntitiesIndividually if loading entities one at a time is faster. - * @param array $entityIds The entity ids to load + * @param string[] $entityIds The entity ids to load * @param string $set The set we want to get metadata from. * @return array An associative array with the metadata for the requested entities, if found. */ @@ -262,7 +262,7 @@ abstract class MetaDataStorageSource * Loads metadata entities one at a time, rather than the default implementation of loading all entities * and filtering. * @see MetaDataStorageSource::getMetaDataForEntities() - * @param array $entityIds The entity ids to load + * @param string[] $entityIds The entity ids to load * @param string $set The set we want to get metadata from. * @return array An associative array with the metadata for the requested entities, if found. */ diff --git a/lib/SimpleSAML/Metadata/SAMLBuilder.php b/lib/SimpleSAML/Metadata/SAMLBuilder.php index 988371bb1..f598e7e02 100644 --- a/lib/SimpleSAML/Metadata/SAMLBuilder.php +++ b/lib/SimpleSAML/Metadata/SAMLBuilder.php @@ -501,7 +501,7 @@ class SAMLBuilder * Add SAML 2.0 SP metadata. * * @param array $metadata The metadata. - * @param array $protocols The protocols supported. Defaults to \SAML2\Constants::NS_SAMLP. + * @param string[] $protocols The protocols supported. Defaults to \SAML2\Constants::NS_SAMLP. * @return void */ public function addMetadataSP20(array $metadata, array $protocols = [Constants::NS_SAMLP]): void @@ -602,77 +602,6 @@ class SAMLBuilder } - /** - * Add metadata of a SAML 1.1 service provider. - * - * @param array $metadata The metadata. - * @return void - */ - public function addMetadataSP11(array $metadata): void - { - Assert::notNull($metadata['entityid']); - Assert::notNull($metadata['metadata-set']); - - $metadata = Configuration::loadFromArray($metadata, $metadata['entityid']); - - $e = new SPSSODescriptor(); - $e->setProtocolSupportEnumeration( - array_merge( - $e->getProtocolSupportEnumeration(), - ['urn:oasis:names:tc:SAML:1.1:protocol'] - ) - ); - - $this->addCertificate($e, $metadata); - - $e->setNameIDFormat($metadata->getArrayizeString('NameIDFormat', [])); - - $endpoints = $metadata->getEndpoints('AssertionConsumerService'); - foreach ($metadata->getArrayizeString('AssertionConsumerService.artifact', []) as $acs) { - $endpoints[] = [ - 'Binding' => 'urn:oasis:names:tc:SAML:1.0:profiles:artifact-01', - 'Location' => $acs, - ]; - } - $e->setAssertionConsumerService(self::createEndpoints($endpoints, true)); - - $this->addAttributeConsumingService($e, $metadata); - - $this->entityDescriptor->addRoleDescriptor($e); - } - - - /** - * Add metadata of a SAML 1.1 identity provider. - * - * @param array $metadata The metadata. - * @return void - */ - public function addMetadataIdP11(array $metadata): void - { - Assert::notNull($metadata['entityid']); - Assert::notNull($metadata['metadata-set']); - - $metadata = Configuration::loadFromArray($metadata, $metadata['entityid']); - - $e = new IDPSSODescriptor(); - $e->setProtocolSupportEnumeration( - array_merge($e->getProtocolSupportEnumeration(), [ - 'urn:oasis:names:tc:SAML:1.1:protocol', - 'urn:mace:shibboleth:1.0' - ]) - ); - - $this->addCertificate($e, $metadata); - - $e->setNameIDFormat($metadata->getArrayizeString('NameIDFormat', [])); - - $e->setSingleSignOnService(self::createEndpoints($metadata->getEndpoints('SingleSignOnService'), false)); - - $this->entityDescriptor->addRoleDescriptor($e); - } - - /** * Add metadata of a SAML attribute authority. * diff --git a/lib/SimpleSAML/Metadata/SAMLParser.php b/lib/SimpleSAML/Metadata/SAMLParser.php index 71ff4dc5e..bf036e0ed 100644 --- a/lib/SimpleSAML/Metadata/SAMLParser.php +++ b/lib/SimpleSAML/Metadata/SAMLParser.php @@ -278,8 +278,6 @@ class SAMLParser */ public static function parseDocument(DOMDocument $document): SAMLParser { - Assert::isInstanceOf($document, DOMDocument::class); - $entityElement = self::findEntityDescriptor($document); return self::parseElement($entityElement); @@ -294,10 +292,8 @@ class SAMLParser * * @return SAMLParser An instance of this class with the metadata loaded. */ - public static function parseElement(\SAML2\XML\md\EntityDescriptor $entityElement): SAMLParser + public static function parseElement(EntityDescriptor $entityElement): SAMLParser { - Assert::isInstanceOf($entityElement, EntityDescriptor::class); - return new SAMLParser($entityElement, null, []); } @@ -393,7 +389,7 @@ class SAMLParser */ private static function processDescriptorsElement( SignedElementHelper $element, - ?int $maxExpireTime, + ?int $maxExpireTime = null, array $validators = [], array $parentExtensions = [] ): array { @@ -426,7 +422,7 @@ class SAMLParser * This function looks for the 'validUntil' attribute to determine * how long a given XML-element is valid. It returns this as a unix timestamp. * - * @param mixed $element The element we should determine the expiry time of. + * @param mixed $element The element we should determine the expiry time of. * @param int|null $maxExpireTime The maximum expiration time. * * @return int|null The unix timestamp for when the element should expire. Will be NULL if no @@ -705,7 +701,7 @@ class SAMLParser /** * Retrieve AttributeAuthorities from the metadata. * - * @return array Array of AttributeAuthorityDescriptor entries. + * @return \SAML2\XML\md\AttributeAuthorityDescriptor[] Array of AttributeAuthorityDescriptor entries. */ public function getAttributeAuthorities(): array { @@ -1256,7 +1252,7 @@ class SAMLParser /** * This function finds IdP descriptors which supports one of the given protocols. * - * @param array $protocols Array with the protocols we accept. + * @param string[] $protocols Array with the protocols we accept. * * @return array with IdP descriptors which supports one of the given protocols. */ diff --git a/lib/SimpleSAML/Metadata/Signer.php b/lib/SimpleSAML/Metadata/Signer.php index f496f3cd8..ca358c124 100644 --- a/lib/SimpleSAML/Metadata/Signer.php +++ b/lib/SimpleSAML/Metadata/Signer.php @@ -26,8 +26,7 @@ class Signer * * @param \SimpleSAML\Configuration $config Our \SimpleSAML\Configuration instance. * @param array $entityMetadata The metadata of the entity. - * @param string $type A string which describes the type entity this is, e.g. 'SAML 2 IdP' or - * 'Shib 1.3 SP'. + * @param string $type A string which describes the type entity this is, e.g. 'SAML 2 IdP' * * @return array An associative array with the keys 'privatekey', 'certificate', and optionally 'privatekey_pass'. * @throws \Exception If the key and certificate used to sign is unknown. @@ -126,8 +125,7 @@ class Signer * * @param \SimpleSAML\Configuration $config Our \SimpleSAML\Configuration instance. * @param array $entityMetadata The metadata of the entity. - * @param string $type A string which describes the type entity this is, e.g. 'SAML 2 IdP' or - * 'Shib 1.3 SP'. + * @param string $type A string which describes the type entity this is, e.g. 'SAML 2 IdP' * * @return boolean True if metadata signing is enabled, false otherwise. * @throws \Exception If the value of the 'metadata.sign.enable' option is not a boolean. @@ -147,9 +145,7 @@ class Signer return $entityMetadata['metadata.sign.enable']; } - $enabled = $config->getBoolean('metadata.sign.enable', false); - - return $enabled; + return $config->getBoolean('metadata.sign.enable', false); } diff --git a/lib/SimpleSAML/Metadata/Sources/MDQ.php b/lib/SimpleSAML/Metadata/Sources/MDQ.php index 1b388c3ac..00750c95b 100644 --- a/lib/SimpleSAML/Metadata/Sources/MDQ.php +++ b/lib/SimpleSAML/Metadata/Sources/MDQ.php @@ -309,7 +309,7 @@ class MDQ extends \SimpleSAML\Metadata\MetaDataStorageSource /** * This function loads the metadata for entity IDs in $entityIds. It is returned as an associative array * where the key is the entity id. An empty array may be returned if no matching entities were found - * @param array $entityIds The entity ids to load + * @param string[] $entityIds The entity ids to load * @param string $set The set we want to get metadata from. * @return array An associative array with the metadata for the requested entities, if found. */ diff --git a/lib/SimpleSAML/Module.php b/lib/SimpleSAML/Module.php index a2cbf2fe4..e59418bdc 100644 --- a/lib/SimpleSAML/Module.php +++ b/lib/SimpleSAML/Module.php @@ -126,7 +126,7 @@ class Module * @throws Error\BadRequest In case the request URI is malformed. * @throws Error\NotFound In case the request URI is invalid or the resource it points to cannot be found. */ - public static function process(Request $request = null) + public static function process(Request $request = null): Response { if ($request === null) { $request = Request::createFromGlobals(); @@ -193,18 +193,16 @@ class Module $request->getContent() ); - if ($config->getBoolean('usenewui', false) === true) { - try { - $kernel = new Kernel($module); - $response = $kernel->handle($request); - $kernel->terminate($request, $response); - - return $response; - } catch (FileLocatorFileNotFoundException $e) { - // no routes configured for this module, fall back to the old system - } catch (NotFoundHttpException $e) { - // this module has been migrated, but the route wasn't found - } + try { + $kernel = new Kernel($module); + $response = $kernel->handle($request); + $kernel->terminate($request, $response); + + return $response; + } catch (FileLocatorFileNotFoundException $e) { + // no routes configured for this module, fall back to the old system + } catch (NotFoundHttpException $e) { + // this module has been migrated, but the route wasn't found } $moduleDir = self::getModuleDir($module) . '/www/'; @@ -370,7 +368,7 @@ class Module /** * Get available modules. * - * @return array One string for each module. + * @return string[] One string for each module. * * @throws \Exception If we cannot open the module's directory. */ @@ -573,7 +571,7 @@ class Module * * @return RedirectResponse A redirection to the URI specified in the request, but with a trailing slash. */ - public static function addTrailingSlash(Request $request) + public static function addTrailingSlash(Request $request): RedirectResponse { // Must be of form /{module} - append a slash return new RedirectResponse($request->getRequestUri() . '/', 308); diff --git a/lib/SimpleSAML/Session.php b/lib/SimpleSAML/Session.php index 00d57606f..e0a16dd3e 100644 --- a/lib/SimpleSAML/Session.php +++ b/lib/SimpleSAML/Session.php @@ -221,7 +221,7 @@ class Session implements \Serializable, Utils\ClearableState * * Cannot typehint param as string due to upstream restrictions */ - public function unserialize($serialized): void + public function unserialize($serialized) { $session = unserialize($serialized); if (is_array($session)) { @@ -599,12 +599,12 @@ class Session implements \Serializable, Utils\ClearableState * If the user already has logged in, the user will be logged out first. * * @param string $authority The authority the user logged in with. - * @param array|null $data The authentication data for this authority. + * @param array $data The authentication data for this authority. * @return void * * @throws Error\CannotSetCookie If the authentication token cannot be set for some reason. */ - public function doLogin(string $authority, array $data = null): void + public function doLogin(string $authority, array $data = []): void { Logger::debug('Session: doLogin("' . $authority . '")'); @@ -615,10 +615,6 @@ class Session implements \Serializable, Utils\ClearableState $this->doLogout($authority); } - if ($data === null) { - $data = []; - } - $data['Authority'] = $authority; if (!isset($data['AuthnInstant'])) { @@ -791,7 +787,7 @@ class Session implements \Serializable, Utils\ClearableState * @param array $params The parameters for the cookies. * @return void */ - public function updateSessionCookies(array $params = null): void + public function updateSessionCookies(array $params = []): void { $sessionHandler = SessionHandler::getSessionHandler(); @@ -799,7 +795,7 @@ class Session implements \Serializable, Utils\ClearableState $sessionHandler->setCookie($sessionHandler->getSessionCookieName(), $this->sessionId, $params); } - $params = array_merge($sessionHandler->getCookieParams(), is_array($params) ? $params : []); + $params = array_merge($sessionHandler->getCookieParams(), $params); if ($this->authToken !== null) { Utils\HTTP::setCookie( @@ -1155,9 +1151,9 @@ class Session implements \Serializable, Utils\ClearableState * Retrieve a list of authorities (authentication sources) that are currently valid within * this session. * - * @return mixed An array containing every authority currently valid. Empty if none available. + * @return string[] An array containing every authority currently valid. Empty if none available. */ - public function getAuthorities() + public function getAuthorities(): array { $authorities = []; foreach (array_keys($this->authData) as $authority) { diff --git a/lib/SimpleSAML/Store/Redis.php b/lib/SimpleSAML/Store/Redis.php index 85ee3562a..cb242e3a7 100644 --- a/lib/SimpleSAML/Store/Redis.php +++ b/lib/SimpleSAML/Store/Redis.php @@ -20,6 +20,7 @@ class Redis extends Store /** @var \Predis\Client */ public $redis; + /** * Initialize the Redis data store. * @param \Predis\Client|null $redis diff --git a/lib/SimpleSAML/Store/SQL.php b/lib/SimpleSAML/Store/SQL.php index e9e8418bb..8b3b52ed3 100644 --- a/lib/SimpleSAML/Store/SQL.php +++ b/lib/SimpleSAML/Store/SQL.php @@ -25,7 +25,6 @@ class SQL extends Store */ public $pdo; - /** * Our database driver. * @@ -33,7 +32,6 @@ class SQL extends Store */ public $driver; - /** * The prefix we should use for our tables. * @@ -41,7 +39,6 @@ class SQL extends Store */ public $prefix; - /** * Associative array of table versions. * @@ -222,7 +219,7 @@ class SQL extends Store * Since various databases implement different methods for doing this, we abstract it away here. * * @param string $table The table we should update. - * @param array $keys The key columns. + * @param string[] $keys The key columns. * @param array $data Associative array with columns. * @return void */ diff --git a/lib/SimpleSAML/Utils/Arrays.php b/lib/SimpleSAML/Utils/Arrays.php index f6b1fcf2d..0f9bd90ad 100644 --- a/lib/SimpleSAML/Utils/Arrays.php +++ b/lib/SimpleSAML/Utils/Arrays.php @@ -34,7 +34,7 @@ class Arrays * * @param array $array The two-dimensional array to transpose. * - * @return mixed The transposed array, or false if $array is not a valid two-dimensional array. + * @return array|false The transposed array, or false if $array is not a valid two-dimensional array. * * @author Andreas Solberg, UNINETT AS <andreas.solberg@uninett.no> */ diff --git a/lib/SimpleSAML/Utils/EMail.php b/lib/SimpleSAML/Utils/EMail.php index 48fb3fac4..d485b6707 100644 --- a/lib/SimpleSAML/Utils/EMail.php +++ b/lib/SimpleSAML/Utils/EMail.php @@ -252,53 +252,20 @@ class EMail public function generateBody(string $template): string { $config = Configuration::getInstance(); - $newui = $config->getBoolean('usenewui', false); - - if ($newui === false) { - $result = '<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" - "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"> -<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en"> -<head> - <meta http-equiv="content-type" content="text/html; charset=utf-8" /> - <title>SimpleSAMLphp Email report</title> - <style type="text/css"> -pre, div.box { - margin: .4em 2em .4em 1em; - padding: 4px; -} -pre { - background: #eee; - border: 1px solid #aaa; -} - </style> -</head> -<body> -<h1>' . htmlspecialchars($this->mail->Subject) . '</h1> -<div class="container" style="background: #fafafa; border: 1px solid #eee; margin: 2em; padding: .6em;"> -<blockquote>"' . htmlspecialchars($this->text) . '"</blockquote> -</div>'; - foreach ($this->data as $name => $values) { - $result .= '<h2>' . htmlspecialchars($name) . '</h2><ul>'; - foreach ($values as $value) { - $result .= '<li><pre>' . htmlspecialchars($value) . '</pre></li>'; - } - $result .= '</ul>'; - } - } else { - $t = new Template($config, $template); - $twig = $t->getTwig(); - 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, [ - 'subject' => $this->mail->Subject, - 'text' => $this->text, - 'data' => $this->data - ]); + + $t = new Template($config, $template); + $twig = $t->getTwig(); + 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, [ + 'subject' => $this->mail->Subject, + 'text' => $this->text, + 'data' => $this->data + ]); return $result; } } diff --git a/lib/SimpleSAML/Utils/HTTP.php b/lib/SimpleSAML/Utils/HTTP.php index 6ae9f2349..a0ab8ecd2 100644 --- a/lib/SimpleSAML/Utils/HTTP.php +++ b/lib/SimpleSAML/Utils/HTTP.php @@ -333,7 +333,7 @@ class HTTP * Check if a URL is valid and is in our list of allowed URLs. * * @param string $url The URL to check. - * @param array $trustedSites An optional white list of domains. If none specified, the 'trusted.url.domains' + * @param string[] $trustedSites An optional white list of domains. If none specified, the 'trusted.url.domains' * configuration directive will be used. * * @return string The normalized URL itself if it is allowed. An empty string if the $url parameter is empty as diff --git a/lib/SimpleSAML/Utils/System.php b/lib/SimpleSAML/Utils/System.php index b2a1c3155..2c2b4d181 100644 --- a/lib/SimpleSAML/Utils/System.php +++ b/lib/SimpleSAML/Utils/System.php @@ -28,7 +28,7 @@ class System /** * This function returns the Operating System we are running on. * - * @return mixed A predefined constant identifying the OS we are running on. False if we are unable to determine it. + * @return string|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/lib/SimpleSAML/Utils/XML.php b/lib/SimpleSAML/Utils/XML.php index 46e2f3f22..750340c8d 100644 --- a/lib/SimpleSAML/Utils/XML.php +++ b/lib/SimpleSAML/Utils/XML.php @@ -31,7 +31,6 @@ class XML * @param string $message The SAML document we want to check. * @param string $type The type of document. Can be one of: * - 'saml20' - * - 'saml11' * - 'saml-meta' * * @throws \InvalidArgumentException If $message is not a string or $type is not a string containing one of the @@ -45,7 +44,7 @@ class XML */ public static function checkSAMLMessage(string $message, string $type): void { - $allowed_types = ['saml20', 'saml11', 'saml-meta']; + $allowed_types = ['saml20', 'saml-meta']; if (!in_array($type, $allowed_types, true)) { throw new \InvalidArgumentException('Invalid input parameters.'); } @@ -73,9 +72,6 @@ class XML $result = true; switch ($type) { - case 'saml11': - $result = self::isValid($message, 'oasis-sstc-saml-schema-protocol-1.1.xsd'); - break; case 'saml20': $result = self::isValid($message, 'saml-schema-protocol-2.0.xsd'); break; @@ -344,9 +340,7 @@ class XML * We also define the following shortcuts for namespaces: * - '@ds': 'http://www.w3.org/2000/09/xmldsig#' * - '@md': 'urn:oasis:names:tc:SAML:2.0:metadata' - * - '@saml1': 'urn:oasis:names:tc:SAML:1.0:assertion' * - '@saml1md': 'urn:oasis:names:tc:SAML:profiles:v1metadata' - * - '@saml1p': 'urn:oasis:names:tc:SAML:1.0:protocol' * - '@saml2': 'urn:oasis:names:tc:SAML:2.0:assertion' * - '@saml2p': 'urn:oasis:names:tc:SAML:2.0:protocol' * @@ -373,12 +367,8 @@ class XML $shortcuts = [ '@ds' => 'http://www.w3.org/2000/09/xmldsig#', '@md' => 'urn:oasis:names:tc:SAML:2.0:metadata', - '@saml1' => 'urn:oasis:names:tc:SAML:1.0:assertion', - '@saml1md' => 'urn:oasis:names:tc:SAML:profiles:v1metadata', - '@saml1p' => 'urn:oasis:names:tc:SAML:1.0:protocol', '@saml2' => 'urn:oasis:names:tc:SAML:2.0:assertion', - '@saml2p' => 'urn:oasis:names:tc:SAML:2.0:protocol', - '@shibmd' => 'urn:mace:shibboleth:metadata:1.0', + '@saml2p' => 'urn:oasis:names:tc:SAML:2.0:protocol' ]; // check if it is a valid shortcut diff --git a/lib/SimpleSAML/XHTML/IdPDisco.php b/lib/SimpleSAML/XHTML/IdPDisco.php index 32cb60a97..7b71ec053 100644 --- a/lib/SimpleSAML/XHTML/IdPDisco.php +++ b/lib/SimpleSAML/XHTML/IdPDisco.php @@ -113,7 +113,7 @@ class IdPDisco * * The constructor does the parsing of the request. If this is an invalid request, it will throw an exception. * - * @param array $metadataSets Array with metadata sets we find remote entities in. + * @param string[] $metadataSets Array with metadata sets we find remote entities in. * @param string $instance The name of this instance of the discovery service. * * @throws \Exception If the request is invalid. @@ -479,7 +479,7 @@ class IdPDisco /** * Return the list of scoped idp * - * @return array An array of IdP entities + * @return string[] An array of IdP entities */ protected function getScopedIDPList(): array { diff --git a/lib/SimpleSAML/XHTML/Template.php b/lib/SimpleSAML/XHTML/Template.php index 521325380..bc30e11df 100644 --- a/lib/SimpleSAML/XHTML/Template.php +++ b/lib/SimpleSAML/XHTML/Template.php @@ -85,13 +85,6 @@ class Template extends Response */ private $module; - /** - * Whether to use the new user interface or not. - * - * @var bool - */ - private $useNewUI = false; - /** * A template controller, if any. * @@ -141,24 +134,19 @@ class Template extends Response $this->translator = new Translate($configuration, $defaultDictionary); $this->localization = new Localization($configuration); - // check if we are supposed to use the new UI - $this->useNewUI = $this->configuration->getBoolean('usenewui', false); - - if ($this->useNewUI) { - // check if we need to attach a theme controller - $controller = $this->configuration->getString('theme.controller', false); - if ( - $controller - && class_exists($controller) - && in_array(TemplateControllerInterface::class, class_implements($controller)) - ) { - /** @var \SimpleSAML\XHTML\TemplateControllerInterface $this->controller */ - $this->controller = new $controller(); - } - - $this->twig = $this->setupTwig(); + // check if we need to attach a theme controller + $controller = $this->configuration->getString('theme.controller', false); + if ( + $controller + && class_exists($controller) + && in_array(TemplateControllerInterface::class, class_implements($controller)) + ) { + /** @var \SimpleSAML\XHTML\TemplateControllerInterface $this->controller */ + $this->controller = new $controller(); } + $this->twig = $this->setupTwig(); + $this->charset = 'UTF-8'; parent::__construct(); } @@ -220,19 +208,8 @@ class Template extends Response if (strripos($templateName, '.twig')) { return $templateName; } - $phppos = strripos($templateName, '.php'); - if ($phppos) { - $templateName = substr($templateName, 0, $phppos); - } - $tplpos = strripos($templateName, '.tpl'); - if ($tplpos) { - $templateName = substr($templateName, 0, $tplpos); - } - if ($this->useNewUI || ($this->theme['module'] !== null)) { - return $templateName . '.twig'; - } - return $templateName; + return $templateName . '.twig'; } @@ -657,9 +634,9 @@ class Template extends Response /** * Get the current instance of Twig in use. * - * @return \Twig\Environment The Twig instance in use, or null if Twig is not used. + * @return \Twig\Environment|null The Twig instance in use, or null if Twig is not used. */ - public function getTwig(): \Twig\Environment + public function getTwig(): ?\Twig\Environment { return $this->twig; } @@ -668,7 +645,7 @@ class Template extends Response /** * Wraps Language->getLanguageList * - * @return array + * @return string[] */ private function getLanguageList(): array { diff --git a/modules/core/lib/Auth/Source/AdminPassword.php b/modules/core/lib/Auth/Source/AdminPassword.php index 5fc1b7b5e..5f2b8f7b5 100644 --- a/modules/core/lib/Auth/Source/AdminPassword.php +++ b/modules/core/lib/Auth/Source/AdminPassword.php @@ -23,7 +23,7 @@ class AdminPassword 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); @@ -48,7 +48,7 @@ class AdminPassword 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); diff --git a/tests/Utils/TestAuthSource.php b/tests/Utils/TestAuthSource.php index de395181d..2388bfb25 100644 --- a/tests/Utils/TestAuthSource.php +++ b/tests/Utils/TestAuthSource.php @@ -9,9 +9,10 @@ use SimpleSAML\Auth\Source; class TestAuthSource extends Source { /** + * @param array &$state * @return void */ - public function authenticate(&$state) + public function authenticate(array &$state): void { } } diff --git a/tests/Utils/TestAuthSourceFactory.php b/tests/Utils/TestAuthSourceFactory.php index 6c42abcd2..6849f6e93 100644 --- a/tests/Utils/TestAuthSourceFactory.php +++ b/tests/Utils/TestAuthSourceFactory.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace SimpleSAML\Test\Utils; +use SimpleSAML\Auth\Source; use SimpleSAML\Auth\SourceFactory; class TestAuthSourceFactory implements SourceFactory @@ -11,7 +12,7 @@ class TestAuthSourceFactory implements SourceFactory /** * @return \SimpleSAML\Test\Utils\TestAuthSource */ - public function create(array $info, array $config) + public function create(array $info, array $config): Source { return new TestAuthSource($info, $config); } diff --git a/tests/lib/SimpleSAML/Locale/LocalizationTest.php b/tests/lib/SimpleSAML/Locale/LocalizationTest.php index 914dbc248..7a138d6bf 100644 --- a/tests/lib/SimpleSAML/Locale/LocalizationTest.php +++ b/tests/lib/SimpleSAML/Locale/LocalizationTest.php @@ -26,12 +26,8 @@ class LocalizationTest extends TestCase */ public function testLocalization() { - // The constructor should activate the default domain - $c = Configuration::loadFromArray( - ['usenewui' => false] - ); + $c = Configuration::loadFromArray([]); $l = new Localization($c); - $this->assertTrue($l->isI18NBackendDefault()); $this->assertEquals(Localization::DEFAULT_DOMAIN, 'messages'); } @@ -42,9 +38,7 @@ class LocalizationTest extends TestCase */ public function testAddDomain() { - $c = Configuration::loadFromArray( - ['usenewui' => true] - ); + $c = Configuration::loadFromArray([]); $l = new Localization($c); $newDomain = 'test'; $newDomainLocaleDir = $l->getLocaleDir(); diff --git a/tests/modules/core/lib/Controller/LoginTest.php b/tests/modules/core/lib/Controller/LoginTest.php index 9cfafbc0b..95d30addf 100644 --- a/tests/modules/core/lib/Controller/LoginTest.php +++ b/tests/modules/core/lib/Controller/LoginTest.php @@ -58,7 +58,6 @@ class LoginTest extends ClearStateTestCase [ 'baseurlpath' => 'https://example.org/simplesaml', 'module.enable' => ['exampleauth' => true], - 'usenewui' => true, ], '[ARRAY]', 'simplesaml' -- GitLab