From aab052536fa950c12870f60c8ee47ba9ec985a6a Mon Sep 17 00:00:00 2001 From: Tim van Dijen <tvdijen@gmail.com> Date: Sat, 5 Feb 2022 23:27:17 +0100 Subject: [PATCH] More fixes --- lib/SimpleSAML/Auth/Simple.php | 6 +- lib/SimpleSAML/Auth/Source.php | 2 +- lib/SimpleSAML/Configuration.php | 204 ++++++++++++------- lib/SimpleSAML/Error/Error.php | 2 +- lib/SimpleSAML/Error/Exception.php | 2 +- lib/SimpleSAML/Locale/Language.php | 13 +- lib/SimpleSAML/Metadata/SAMLBuilder.php | 6 +- lib/SimpleSAML/Module.php | 2 +- lib/SimpleSAML/Session.php | 4 +- lib/SimpleSAML/Utils/EMail.php | 2 +- lib/SimpleSAML/Utils/HTTP.php | 9 +- modules/admin/lib/Controller/Config.php | 4 +- modules/admin/lib/Controller/Federation.php | 4 +- modules/cron/hooks/hook_cron.php | 2 +- modules/cron/lib/Controller/Cron.php | 8 +- modules/saml/lib/Auth/Source/SP.php | 8 +- modules/saml/lib/IdP/SAML2.php | 2 +- modules/saml/www/sp/saml2-acs.php | 6 +- tests/lib/SimpleSAML/ConfigurationTest.php | 7 +- tests/modules/saml/lib/IdP/SQLNameIDTest.php | 8 +- www/index.php | 2 +- 21 files changed, 182 insertions(+), 121 deletions(-) diff --git a/lib/SimpleSAML/Auth/Simple.php b/lib/SimpleSAML/Auth/Simple.php index 0909d9790..350901fbe 100644 --- a/lib/SimpleSAML/Auth/Simple.php +++ b/lib/SimpleSAML/Auth/Simple.php @@ -26,8 +26,8 @@ class Simple */ protected string $authSource; - /** @var \SimpleSAML\Configuration|null */ - protected ?Configuration $app_config; + /** @var \SimpleSAML\Configuration */ + protected Configuration $app_config; /** @var \SimpleSAML\Session */ protected Session $session; @@ -46,7 +46,7 @@ class Simple $config = Configuration::getInstance(); } $this->authSource = $authSource; - $this->app_config = $config->getOptionalConfigItem('application', null); + $this->app_config = $config->getOptionalConfigItem('application', []); if ($session === null) { $session = Session::getSessionFromRequest(); diff --git a/lib/SimpleSAML/Auth/Source.php b/lib/SimpleSAML/Auth/Source.php index 79fec9e02..1b54babb9 100644 --- a/lib/SimpleSAML/Auth/Source.php +++ b/lib/SimpleSAML/Auth/Source.php @@ -344,7 +344,7 @@ abstract class Source // for now - load and parse config file $config = Configuration::getConfig('authsources.php'); - $authConfig = $config->getArray($authId, null); + $authConfig = $config->getOptionalArray($authId, null); if ($authConfig === null) { if ($type !== null) { throw new Error\Exception( diff --git a/lib/SimpleSAML/Configuration.php b/lib/SimpleSAML/Configuration.php index 71c4f0ea5..54a6d9edb 100644 --- a/lib/SimpleSAML/Configuration.php +++ b/lib/SimpleSAML/Configuration.php @@ -6,6 +6,7 @@ namespace SimpleSAML; use SAML2\Constants; use SimpleSAML\Assert\Assert; +use SimpleSAML\Assert\AssertionFailedException; use SimpleSAML\Error; use SimpleSAML\Utils; @@ -382,7 +383,7 @@ class Configuration implements Utils\ClearableState return $default; } - return $this->configuration[$name]; + return $this->configuration[$name] ?? $default; } @@ -589,17 +590,24 @@ class Configuration implements Utils\ClearableState * The default value can be null or a boolean. * * @return bool|null The option with the given name, or $default. + * @psalm-return ($default is null ? null : bool) * * @throws \SimpleSAML\Assert\AssertionFailedException If the option is not boolean. */ public function getOptionalBoolean(string $name, ?bool $default): ?bool { - if (!$this->hasValue($name)) { - // the option wasn't found, or it matches the default value. In any case, return this value - return $default; - } + $ret = $this->getOptionalValue($name, $default); - return $this->getBoolean($name); + Assert::nullOrBoolean( + $ret, + sprintf( + '%s: The option %s is not a valid boolean value or null.', + $this->location, + var_export($name, true) + ), + ); + + return $ret; } @@ -636,17 +644,24 @@ class Configuration implements Utils\ClearableState * The default value can be null or a string. * * @return string|null The option with the given name, or $default if the option isn't found. + * @psalm-return ($default is null ? null : string) * * @throws \SimpleSAML\Assert\AssertionFailedException If the option is not a string. */ public function getOptionalString(string $name, ?string $default): ?string { - if (!$this->hasValue($name)) { - // the option wasn't found, or it matches the default value. In any case, return this value - return $default; - } + $ret = $this->getOptionalValue($name, $default); + + Assert::nullOrString( + $ret, + sprintf( + '%s: The option %s is not a valid string value or null.', + $this->location, + var_export($name, true) + ), + ); - return $this->getString($name); + return $ret; } @@ -683,17 +698,24 @@ class Configuration implements Utils\ClearableState * The default value can be null or an integer. * * @return int|null The option with the given name, or $default if the option isn't found. + * @psalm-return ($default is null ? null : int) * * @throws \SimpleSAML\Assert\AssertionFailedException If the option is not an integer. */ public function getOptionalInteger(string $name, ?int $default): ?int { - if (!$this->hasValue($name)) { - // the option wasn't found, or it matches the default value. In any case, return this value - return $default; - } + $ret = $this->getOptionalValue($name, $default); + + Assert::nullOrInteger( + $ret, + sprintf( + '%s: The option %s is not a valid integer value or null.', + $this->location, + var_export($name, true) + ), + ); - return $this->getInteger($name); + return $ret; } @@ -747,17 +769,26 @@ class Configuration implements Utils\ClearableState * * @return int|null The option with the given name, or $default if the option isn't found and $default is * specified. + * @psalm-return ($default is null ? null : int) * * @throws \SimpleSAML\Assert\AssertionFailedException If the option is not in the range specified. */ public function getOptionalIntegerRange(string $name, int $minimum, int $maximum, ?int $default): ?int { - if (!$this->hasValue($name)) { - // the option wasn't found, or it matches the default value. In any case, return this value - return $default; - } + $ret = $this->getOptionalInteger($name, $default); + + Assert::nullOrRange( + $ret, + $minimum, + $maximum, + sprintf( + '%s: Value of option %s is out of range. Value is %%s, allowed range is [%%2$s - %%3$s] or null.', + $this->location, + var_export($name, true), + ), + ); - return $this->getInteger($name, $minimum, $maximum); + return $ret; } @@ -784,7 +815,7 @@ class Configuration implements Utils\ClearableState $ret, $allowedValues, sprintf( - '%s: Invalid value given for option %s. It should have one of: %2$s; but got: %%s.', + '%s: Invalid value given for option %s. It should have one of: %%2$s; but got: %%s.', $this->location, var_export($name, true), ), @@ -813,12 +844,19 @@ class Configuration implements Utils\ClearableState */ public function getOptionalValueValidate(string $name, array $allowedValues, $default) { - if (!$this->hasValue($name)) { - // the option wasn't found, or it matches the default value. In any case, return this value - return $default; - } + $ret = $this->getOptionalValue($name, $default); + + Assert::nullOrOneOf( + $ret, + $allowedValues, + sprintf( + '%s: Invalid value given for option %s. It should have one of: %%2$s or null; but got: %%s.', + $this->location, + var_export($name, true), + ), + ); - return $this->getValueValidate($name, $allowedValues); + return $ret; } @@ -850,23 +888,26 @@ class Configuration implements Utils\ClearableState * * An exception will be thrown if this option isn't an array, or if this option isn't found. * - * @param string $name The name of the option. - * @param array|null $default A default value which will be returned if the option isn't found. - * The default value can be null or an array. + * @param string $name The name of the option. + * @param array|null $default A default value which will be returned if the option isn't found. + * The default value can be null or an array. * * @return array|null The option with the given name, or $default if the option isn't found and $default is * specified. + * @psalm-return ($default is null ? null : array) * * @throws \SimpleSAML\Assert\AssertionFailedException If the option is not an array. */ public function getOptionalArray(string $name, ?array $default): ?array { - if (!$this->hasValue($name)) { - // the option wasn't found, or it matches the default value. In any case, return this value - return $default; - } + $ret = $this->getOptionalValue($name, $default); + + Assert::nullOrIsArray( + $ret, + sprintf('%s: The option %s is not an array or null.', $this->location, var_export($name, true)), + ); - return $this->getArray($name); + return $ret; } @@ -892,7 +933,7 @@ class Configuration implements Utils\ClearableState /** - * This function retrieves an array configuration option. + * This function retrieves an optional array configuration option. * * If the configuration option isn't an array, it will be converted to an array. * @@ -901,15 +942,12 @@ class Configuration implements Utils\ClearableState * The default value can be null or an array. * * @return array|null The option with the given name. + * @psalm-return ($default is null ? null : array) */ public function getOptionalArrayize(string $name, $default): ?array { - if (!$this->hasValue($name)) { - // the option wasn't found, or it matches the default value. In any case, return this value - return $default; - } + $ret = $this->getOptionalValue($name, $default); - $ret = $this->getValue($name); if (!is_array($ret)) { $ret = [$ret]; } @@ -926,7 +964,7 @@ class Configuration implements Utils\ClearableState * @param string $name The name of the option. * @return string[] The option with the given name. * - * @throws \SimpleSAML\Assert\AssertFailedException If the option is not a string or an array of strings. + * @throws \SimpleSAML\Assert\AssertionFailedException If the option is not a string or an array of strings. */ public function getArrayizeString(string $name): array { @@ -955,17 +993,24 @@ class Configuration implements Utils\ClearableState * The default value can be null or an array of strings. * * @return string[]|null The option with the given name, or $default if the option isn't found and $default is specified. + * @psalm-return ($default is null ? null : array) * * @throws \SimpleSAML\Assert\AssertionFailedException If the option is not a string or an array of strings. */ public function getOptionalArrayizeString(string $name, ?array $default): ?array { - if (!$this->hasValue($name)) { - // the option wasn't found, or it matches the default value. In any case, return this value - return $default; - } + $ret = $this->getOptionalArrayize($name, $default); - return $this->getArrayizeString($name, $allowedValues); + Assert::nullOrAllString( + $ret, + sprintf( + '%s: The option %s must be null, a string or an array of strings.', + $this->location, + var_export($name, true), + ), + ); + + return $ret; } @@ -1006,17 +1051,15 @@ class Configuration implements Utils\ClearableState * * @return \SimpleSAML\Configuration|null The option with the given name, * or $default, converted into a Configuration object. + * @psalm-return ($default is null ? null : \SimpleSAML\Configuration) * - * @throws \SimpleSAML\Assert\AssertionFailed\Exception If the option is not an array. + * @throws \SimpleSAML\Assert\AssertionFailedException If the option is not an array. */ public function getOptionalConfigItem(string $name, ?array $default): ?Configuration { - if (!$this->hasValue($name)) { - // the option wasn't found, or it matches the default value. In any case, return this value - return $default; - } + $ret = $this->getOptionalArray($name, $default); - return $this->getConfigItem($name); + return ($ret === null) ? null : self::loadFromArray($ret, $this->location . '[' . var_export($name, true) . ']'); } @@ -1223,44 +1266,57 @@ class Configuration implements Utils\ClearableState * The default language returned is always 'en'. * * @param string $name The name of the option. - * @param mixed $default The default value. If no default is given, and the option isn't found, an exception will - * be thrown. + * @param array $default The default value. * - * @return mixed Associative array with language => string pairs, or the provided default value. + * @return array Associative array with language => string pairs. * - * @throws \Exception If the translation is not an array or a string, or its index or value are not strings. + * @throws \SimpleSAML\Assert\AssertionFailedException + * If the translation is not an array or a string, or its index or value are not strings. */ - public function getLocalizedString(string $name, $default = self::REQUIRED_OPTION) + public function getLocalizedString(string $name): array { - $ret = $this->getValue($name, $default); - if ($ret === $default) { - // the option wasn't found, or it matches the default value. In any case, return this value - return $ret; - } - - $loc = $this->location . '[' . var_export($name, true) . ']'; + $ret = $this->getValue($name); if (is_string($ret)) { $ret = ['en' => $ret]; } - if (!is_array($ret)) { - throw new \Exception($loc . ': Must be an array or a string.'); - } + Assert::isArray($ret, sprintf('%s: Must be an array or a string.', $this->location)); foreach ($ret as $k => $v) { - if (!is_string($k)) { - throw new \Exception($loc . ': Invalid language code: ' . var_export($k, true)); - } - if (!is_string($v)) { - throw new \Exception($loc . '[' . var_export($v, true) . ']: Must be a string.'); - } + Assert::string($k, sprintf('%s: Invalid language code: %s', $this->location, var_export($k, true))); + Assert::string($v, sprintf('%s[%s]: Must be a string.', $this->location, var_export($v, true))); } return $ret; } + /** + * Retrieve an optional string which may be localized into many languages. + * + * The default language returned is always 'en'. + * + * @param string $name The name of the option. + * @param mixed $default The default value. + * + * @return array|null Associative array with language => string pairs, or the provided default value. + * @psalm-return ($default is null ? null : array) + * + * @throws \SimpleSAML\Assert\AssertionFailedException + * If the translation is not an array or a string, or its index or value are not strings. + */ + public function getOptionalLocalizedString(string $name, ?array $default): ?array + { + if (!$this->hasValue($name)) { + // the option wasn't found, or it matches the default value. In any case, return this value + return $default; + } + + return $this->getLocalizedString($name); + } + + /** * Get public key from metadata. * diff --git a/lib/SimpleSAML/Error/Error.php b/lib/SimpleSAML/Error/Error.php index 755bcb3c6..bce003aa7 100644 --- a/lib/SimpleSAML/Error/Error.php +++ b/lib/SimpleSAML/Error/Error.php @@ -262,7 +262,7 @@ class Error extends Exception } } - $show_function = $config->getArray('errors.show_function', null); + $show_function = $config->getOptionalArray('errors.show_function', null); if (isset($show_function)) { Assert::isCallable($show_function); $this->setHTTPCode(); diff --git a/lib/SimpleSAML/Error/Exception.php b/lib/SimpleSAML/Error/Exception.php index 40c556994..e75fbbab3 100644 --- a/lib/SimpleSAML/Error/Exception.php +++ b/lib/SimpleSAML/Error/Exception.php @@ -200,7 +200,7 @@ class Exception extends \Exception protected function logBacktrace(int $level = Logger::DEBUG): void { // Do nothing if backtraces have been disabled in config. - $debug = Configuration::getInstance()->getArray('debug', ['backtraces' => true]); + $debug = Configuration::getInstance()->getOptionalArray('debug', ['backtraces' => true]); if (array_key_exists('backtraces', $debug) && $debug['backtraces'] === false) { return; } diff --git a/lib/SimpleSAML/Locale/Language.php b/lib/SimpleSAML/Locale/Language.php index ba8b826cf..da982f670 100644 --- a/lib/SimpleSAML/Locale/Language.php +++ b/lib/SimpleSAML/Locale/Language.php @@ -155,8 +155,8 @@ class Language $this->availableLanguages = $this->getInstalledLanguages(); $this->defaultLanguage = $this->configuration->getOptionalString('language.default', self::FALLBACKLANGUAGE); $this->languageParameterName = $this->configuration->getOptionalString('language.parameter.name', 'language'); - $this->customFunction = $this->configuration->getArray('language.get_language_function', null); - $this->rtlLanguages = $this->configuration->getArray('language.rtl', []); + $this->customFunction = $this->configuration->getOptionalArray('language.get_language_function', null); + $this->rtlLanguages = $this->configuration->getOptionalArray('language.rtl', []); if (isset($_GET[$this->languageParameterName])) { $this->setLanguage( $_GET[$this->languageParameterName], @@ -173,7 +173,10 @@ class Language */ private function getInstalledLanguages(): array { - $configuredAvailableLanguages = $this->configuration->getArray('language.available', [self::FALLBACKLANGUAGE]); + $configuredAvailableLanguages = $this->configuration->getOptionalArray( + 'language.available', + [self::FALLBACKLANGUAGE] + ); $availableLanguages = []; foreach ($configuredAvailableLanguages as $code) { if (array_key_exists($code, self::$language_names) && isset(self::$language_names[$code])) { @@ -403,7 +406,7 @@ class Language public static function getLanguageCookie(): ?string { $config = Configuration::getInstance(); - $availableLanguages = $config->getArray('language.available', [self::FALLBACKLANGUAGE]); + $availableLanguages = $config->getOptionalArray('language.available', [self::FALLBACKLANGUAGE]); $name = $config->getOptionalString('language.cookie.name', 'language'); if (isset($_COOKIE[$name])) { @@ -426,7 +429,7 @@ class Language { $language = strtolower($language); $config = Configuration::getInstance(); - $availableLanguages = $config->getArray('language.available', [self::FALLBACKLANGUAGE]); + $availableLanguages = $config->getOptionalArray('language.available', [self::FALLBACKLANGUAGE]); if (!in_array($language, $availableLanguages, true) || headers_sent()) { return; diff --git a/lib/SimpleSAML/Metadata/SAMLBuilder.php b/lib/SimpleSAML/Metadata/SAMLBuilder.php index d37f65e9f..58269de85 100644 --- a/lib/SimpleSAML/Metadata/SAMLBuilder.php +++ b/lib/SimpleSAML/Metadata/SAMLBuilder.php @@ -417,7 +417,7 @@ class SAMLBuilder Configuration $metadata ): void { $attributes = $metadata->getOptionalArray('attributes', []); - $name = $metadata->getLocalizedString('name', null); + $name = $metadata->getOptionalLocalizedString('name', null); if ($name === null || count($attributes) == 0) { // we cannot add an AttributeConsumingService without name and attributes @@ -434,11 +434,11 @@ class SAMLBuilder $attributeconsumer->setIndex($metadata->getOptionalInteger('attributes.index', 0)); if ($metadata->hasValue('attributes.isDefault')) { - $attributeconsumer->setIsDefault($metadata->getBoolean('attributes.isDefault', false)); + $attributeconsumer->setIsDefault($metadata->getOptionalBoolean('attributes.isDefault', false)); } $attributeconsumer->setServiceName($name); - $attributeconsumer->setServiceDescription($metadata->getLocalizedString('description', [])); + $attributeconsumer->setServiceDescription($metadata->getOptionalLocalizedString('description', [])); $nameFormat = $metadata->getOptionalString('attributes.NameFormat', Constants::NAMEFORMAT_URI); foreach ($attributes as $friendlyName => $attribute) { diff --git a/lib/SimpleSAML/Module.php b/lib/SimpleSAML/Module.php index d956eaeb0..bd59e4494 100644 --- a/lib/SimpleSAML/Module.php +++ b/lib/SimpleSAML/Module.php @@ -301,7 +301,7 @@ class Module /** @psalm-var \SimpleSAML\Configuration $assetConfig */ $assetConfig = $config->getOptionalConfigItem('assets', null); /** @psalm-var \SimpleSAML\Configuration $cacheConfig */ - $cacheConfig = $assetConfig ?: $assetOptionalConfig->getConfigItem('caching', null); + $cacheConfig = $assetConfig ?: $assetConfig->getOptionalConfigItem('caching', null); $response = new BinaryFileResponse($path); $response->setCache([ // "public" allows response caching even if the request was authenticated, diff --git a/lib/SimpleSAML/Session.php b/lib/SimpleSAML/Session.php index 2e25d0ebf..0b317b1a5 100644 --- a/lib/SimpleSAML/Session.php +++ b/lib/SimpleSAML/Session.php @@ -180,7 +180,7 @@ class Session implements Utils\ClearableState $this->markDirty(); // initialize data for session check function if defined - $checkFunction = self::$config->getValue('session.check_function', null); + $checkFunction = self::$config->getOptionalValue('session.check_function', null); if (is_callable($checkFunction)) { call_user_func($checkFunction, $this, true); } @@ -371,7 +371,7 @@ class Session implements Utils\ClearableState } // run session check function if defined - $checkFunction = $globalConfig->getValue('session.check_function', null); + $checkFunction = $globalConfig->getOptionalValue('session.check_function', null); if (is_callable($checkFunction)) { $check = call_user_func($checkFunction, $session); if ($check !== true) { diff --git a/lib/SimpleSAML/Utils/EMail.php b/lib/SimpleSAML/Utils/EMail.php index edd75d8f3..60f98b852 100644 --- a/lib/SimpleSAML/Utils/EMail.php +++ b/lib/SimpleSAML/Utils/EMail.php @@ -229,7 +229,7 @@ class EMail $config = Configuration::getInstance(); $EMail->setTransportMethod( $config->getOptionalString('mail.transport.method', 'mail'), - $config->getArrayize('mail.transport.options', []) + $config->getOptionalArrayize('mail.transport.options', []) ); return $EMail; diff --git a/lib/SimpleSAML/Utils/HTTP.php b/lib/SimpleSAML/Utils/HTTP.php index 25317d1c4..e264a4e25 100644 --- a/lib/SimpleSAML/Utils/HTTP.php +++ b/lib/SimpleSAML/Utils/HTTP.php @@ -406,10 +406,10 @@ class HTTP $self_host = $this->getSelfHostWithNonStandardPort(); - $trustedRegex = Configuration::getInstance()->getOptionalString('trusted.url.regex', null); + $trustedRegex = Configuration::getInstance()->getOptionalValue('trusted.url.regex', null); $trusted = false; - if ($trustedRegex !== null) { + if (!in_array($trustedRegex, [null, false])) { // add self host to the white list $trustedSites[] = preg_quote($self_host); foreach ($trustedSites as $regex) { @@ -802,10 +802,9 @@ class HTTP * current URI, so we need to build it back from the PHP environment, unless we have a base URL specified * for this case in the configuration. First, check if that's the case. */ - - /** @var \SimpleSAML\Configuration $appcfg */ $appcfg = $cfg->getOptionalConfigItem('application', null); - $appurl = $appcfg ?: $appcfg->getOptionalString('baseURL', null); + $appurl = ($appcfg !== null) ? $appcfg->getOptionalString('baseURL', null) : null; + if (!empty($appurl)) { $protocol = parse_url($appurl, PHP_URL_SCHEME); $hostname = parse_url($appurl, PHP_URL_HOST); diff --git a/modules/admin/lib/Controller/Config.php b/modules/admin/lib/Controller/Config.php index 1a70d345e..aeba9b7e9 100644 --- a/modules/admin/lib/Controller/Config.php +++ b/modules/admin/lib/Controller/Config.php @@ -442,7 +442,7 @@ class Config } // make sure we have a secret salt set - if ($this->config->getValue('secretsalt') === 'defaultsecretsalt') { + if ($this->config->getString('secretsalt') === 'defaultsecretsalt') { $warnings[] = Translate::noop( '<strong>The configuration uses the default secret salt</strong>. Make sure to modify the <code>' . 'secretsalt</code> option in the SimpleSAMLphp configuration in production environments. <a ' . @@ -469,7 +469,7 @@ class Config curl_setopt($ch, CURLOPT_USERAGENT, 'SimpleSAMLphp'); curl_setopt($ch, CURLOPT_TIMEOUT, 2); curl_setopt($ch, CURLOPT_PROXY, $this->config->getOptionalString('proxy', null)); - curl_setopt($ch, CURLOPT_PROXYUSERPWD, $this->config->getValue('proxy.auth', null)); + curl_setopt($ch, CURLOPT_PROXYUSERPWD, $this->config->getOptionalValue('proxy.auth', null)); $response = curl_exec($ch); if (curl_getinfo($ch, CURLINFO_RESPONSE_CODE) === 200) { diff --git a/modules/admin/lib/Controller/Federation.php b/modules/admin/lib/Controller/Federation.php index 8387c6057..6969ca17d 100644 --- a/modules/admin/lib/Controller/Federation.php +++ b/modules/admin/lib/Controller/Federation.php @@ -329,9 +329,9 @@ class Federation } // get the name - $name = $source->getMetadata()->getLocalizedString( + $name = $source->getMetadata()->getOptionalLocalizedString( 'name', - $source->getMetadata()->getLocalizedString('OrganizationDisplayName', $source->getAuthId()) + $source->getMetadata()->getOptionalLocalizedString('OrganizationDisplayName', ['en' => $source->getAuthId()]) ); $builder = new SAMLBuilder($source->getEntityId()); diff --git a/modules/cron/hooks/hook_cron.php b/modules/cron/hooks/hook_cron.php index 843d4dd5d..c21b3015d 100644 --- a/modules/cron/hooks/hook_cron.php +++ b/modules/cron/hooks/hook_cron.php @@ -15,7 +15,7 @@ function cron_hook_cron(array &$croninfo): void $cronconfig = Configuration::getConfig('module_cron.php'); - if ($cronconfig->getValue('debug_message', true)) { + if ($cronconfig->getOptionalBoolean('debug_message', true)) { $croninfo['summary'][] = 'Cron did run tag [' . $croninfo['tag'] . '] at ' . date(DATE_RFC822); } } diff --git a/modules/cron/lib/Controller/Cron.php b/modules/cron/lib/Controller/Cron.php index 42e1c6227..80346cd12 100644 --- a/modules/cron/lib/Controller/Cron.php +++ b/modules/cron/lib/Controller/Cron.php @@ -85,8 +85,8 @@ class Cron { $this->authUtils->requireAdmin(); - $key = $this->cronconfig->getValue('key', 'secret'); - $tags = $this->cronconfig->getValue('allowed_tags', []); + $key = $this->cronconfig->getOptionalString('key', 'secret'); + $tags = $this->cronconfig->getOptionalArray('allowed_tags', []); $def = [ 'weekly' => "22 0 * * 0", @@ -127,7 +127,7 @@ class Cron */ public function run(string $tag, string $key, string $output = 'xhtml'): Response { - $configKey = $this->cronconfig->getValue('key', 'secret'); + $configKey = $this->cronconfig->getOptionalString('key', 'secret'); if ($key !== $configKey) { Logger::error('Cron - Wrong key provided. Cron will not run.'); exit; @@ -146,7 +146,7 @@ class Cron $croninfo = $cron->runTag($tag); $summary = $croninfo['summary']; - if ($this->cronconfig->getValue('sendemail', true) && count($summary) > 0) { + if ($this->cronconfig->getOptionalBoolean('sendemail', true) && count($summary) > 0) { $mail = new Utils\EMail('SimpleSAMLphp cron report'); $mail->setData(['url' => $url, 'tag' => $croninfo['tag'], 'summary' => $croninfo['summary']]); try { diff --git a/modules/saml/lib/Auth/Source/SP.php b/modules/saml/lib/Auth/Source/SP.php index 0c49cae5f..e17726f5a 100644 --- a/modules/saml/lib/Auth/Source/SP.php +++ b/modules/saml/lib/Auth/Source/SP.php @@ -151,7 +151,7 @@ class SP extends \SimpleSAML\Auth\Source } // add attributes - $name = $this->metadata->getLocalizedString('name', null); + $name = $this->metadata->getOptionalLocalizedString('name', null); $attributes = $this->metadata->getOptionalArray('attributes', []); if ($name !== null) { if (!empty($attributes)) { @@ -176,11 +176,11 @@ class SP extends \SimpleSAML\Auth\Source } // add organization info - $org = $this->metadata->getLocalizedString('OrganizationName', null); + $org = $this->metadata->getOptionalLocalizedString('OrganizationName', null); if ($org !== null) { $metadata['OrganizationName'] = $org; - $metadata['OrganizationDisplayName'] = $this->metadata->getLocalizedString('OrganizationDisplayName', $org); - $metadata['OrganizationURL'] = $this->metadata->getLocalizedString('OrganizationURL', null); + $metadata['OrganizationDisplayName'] = $this->metadata->getOptionalLocalizedString('OrganizationDisplayName', $org); + $metadata['OrganizationURL'] = $this->metadata->getOptionalLocalizedString('OrganizationURL', null); if ($metadata['OrganizationURL'] === null) { throw new Error\Exception( 'If OrganizationName is set, OrganizationURL must also be set.' diff --git a/modules/saml/lib/IdP/SAML2.php b/modules/saml/lib/IdP/SAML2.php index 620d7893b..da86d9c1c 100644 --- a/modules/saml/lib/IdP/SAML2.php +++ b/modules/saml/lib/IdP/SAML2.php @@ -876,7 +876,7 @@ class SAML2 // add organization information if ($config->hasValue('OrganizationName')) { $metadata['OrganizationName'] = $config->getLocalizedString('OrganizationName'); - $metadata['OrganizationDisplayName'] = $config->getLocalizedString( + $metadata['OrganizationDisplayName'] = $config->getOptionalLocalizedString( 'OrganizationDisplayName', $metadata['OrganizationName'] ); diff --git a/modules/saml/www/sp/saml2-acs.php b/modules/saml/www/sp/saml2-acs.php index fd670e8b5..091a65781 100644 --- a/modules/saml/www/sp/saml2-acs.php +++ b/modules/saml/www/sp/saml2-acs.php @@ -101,7 +101,7 @@ if (!empty($stateId)) { } } -$enableUnsolicited = $spMetadata->getBoolean('enable_unsolicited', true); +$enableUnsolicited = $spMetadata->getOptionalBoolean('enable_unsolicited', true); if ($state === null && $enableUnsolicited === false) { throw new Error\BadRequest('Unsolicited responses are denied by configuration.'); } @@ -119,7 +119,7 @@ if ($state) { Assert::keyExists($state, 'ExpectedIssuer'); if ($state['ExpectedIssuer'] !== $issuer) { $idpMetadata = $source->getIdPMetadata($issuer); - $idplist = $idpMetadata->getArrayize('IDPList', []); + $idplist = $idpMetadata->getOptionalArrayize('IDPList', []); if (!in_array($state['ExpectedIssuer'], $idplist, true)) { Logger::warning( 'The issuer of the response not match to the identity provider we sent the request to.' @@ -159,7 +159,7 @@ $attributes = []; $foundAuthnStatement = false; $config = Configuration::getInstance(); -$storeType = $config->getString('store.type', 'phpsession'); +$storeType = $config->getOptionalString('store.type', 'phpsession'); $store = StoreFactory::getInstance($storeType); diff --git a/tests/lib/SimpleSAML/ConfigurationTest.php b/tests/lib/SimpleSAML/ConfigurationTest.php index b13e07823..8509797ca 100644 --- a/tests/lib/SimpleSAML/ConfigurationTest.php +++ b/tests/lib/SimpleSAML/ConfigurationTest.php @@ -494,11 +494,12 @@ class ConfigurationTest extends ClearStateTestCase $this->assertEquals($c->getOptionalValueValidate('opt', ['a', 'b', 'c'], 'f'), 'b'); // Missing option - $this->assertEquals($c->getOptionalValueValidate('missing_opt', ['a', 'b', 'c'], 'f'), 'f'); + $this->assertEquals($c->getOptionalValueValidate('missing_opt', ['a', 'b', 'c'], 'b'), 'b'); // Value not allowed $this->expectException(AssertionFailedException::class); $c->getOptionalValueValidate('opt', ['d', 'e', 'f'], 'c'); + $c->getOptionalValueValidate('missing_opt', ['d', 'e', 'f'], 'c'); } @@ -1011,9 +1012,11 @@ class ConfigurationTest extends ClearStateTestCase 'no' => 'Hei Verden!', ], ]); - $this->assertEquals($c->getLocalizedString('missing_opt', '--missing--'), '--missing--'); $this->assertEquals($c->getLocalizedString('str_opt'), ['en' => 'Hello World!']); $this->assertEquals($c->getLocalizedString('str_array'), ['en' => 'Hello World!', 'no' => 'Hei Verden!']); + + $this->expectException(AssertionFailedException::class); + $c->getLocalizedString('missing_opt'); } diff --git a/tests/modules/saml/lib/IdP/SQLNameIDTest.php b/tests/modules/saml/lib/IdP/SQLNameIDTest.php index 91e8a2be5..e12629ae7 100644 --- a/tests/modules/saml/lib/IdP/SQLNameIDTest.php +++ b/tests/modules/saml/lib/IdP/SQLNameIDTest.php @@ -86,15 +86,15 @@ class SQLNameIDTest extends TestCase { $config = [ 'database.dsn' => 'sqlite::memory:', - 'database.username' => '', - 'database.password' => '', + 'database.username' => null, + 'database.password' => null, 'database.prefix' => 'phpunit_', 'database.persistent' => true, 'database.secondaries' => [ [ 'dsn' => 'sqlite::memory:', - 'username' => '', - 'password' => '', + 'username' => null, + 'password' => null, ], ], ]; diff --git a/www/index.php b/www/index.php index 2e366c1d2..a01831ec1 100644 --- a/www/index.php +++ b/www/index.php @@ -5,5 +5,5 @@ require_once('_include.php'); $config = \SimpleSAML\Configuration::getInstance(); $httpUtils = new \SimpleSAML\Utils\HTTP(); -$redirect = $config->getString('frontpage.redirect', SimpleSAML\Module::getModuleURL('core/welcome')); +$redirect = $config->getOptionalString('frontpage.redirect', SimpleSAML\Module::getModuleURL('core/welcome')); $httpUtils->redirectTrustedURL($redirect); -- GitLab