diff --git a/lib/SimpleSAML/Configuration.php b/lib/SimpleSAML/Configuration.php index d06328a2c00b0d18418aa104da09fe8bff800323..60513d29cf4a3be959cae2927d89dbfd6b4a0c0e 100644 --- a/lib/SimpleSAML/Configuration.php +++ b/lib/SimpleSAML/Configuration.php @@ -767,44 +767,58 @@ class Configuration implements Utils\ClearableState * This will check that the configuration option matches one of the given values. The match will use * strict comparison. An exception will be thrown if it does not match. * - * The option can be mandatory or optional. If no default value is given, it will be considered to be - * mandatory, and an exception will be thrown if it isn't provided. If a default value is given, it - * is considered to be optional, and the default value is returned. The default value is automatically - * included in the list of allowed values. + * The option is mandatory and an exception will be thrown if it isn't provided. * - * @param string $name The name of the option. - * @param array $allowedValues The values the option is allowed to take, as an array. - * @param mixed $default The default value which will be returned if the option isn't found. If this parameter - * isn't given, the option will be considered to be mandatory. The default value can be - * any value, including null. + * @param string $name The name of the option. + * @param array $allowedValues The values the option is allowed to take, as an array. * - * @return mixed The option with the given name, or $default if the option isn't found and $default is given. + * @return mixed The option with the given name. * - * @throws \Exception If the option does not have any of the allowed values. + * @throws \SimpleSAML\Assert\AssertionFailedException If the option does not have any of the allowed values. */ - public function getValueValidate(string $name, array $allowedValues, $default = self::REQUIRED_OPTION) + public function getValueValidate(string $name, array $allowedValues) { - $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; - } + $ret = $this->getValue($name); - if (!in_array($ret, $allowedValues, true)) { - $strValues = []; - foreach ($allowedValues as $av) { - $strValues[] = var_export($av, true); - } - $strValues = implode(', ', $strValues); + Assert::oneOf( + $ret, + $allowedValues, + sprintf( + '%s: Invalid value given for option %s. It should have one of: %2$s; but got: %%s.', + $this->location, + var_export($name, true), + ), + ); - throw new \Exception( - $this->location . ': Invalid value given for the option ' . - var_export($name, true) . '. It should have one of the following values: ' . - $strValues . '; but it had the following value: ' . var_export($ret, true) - ); + return $ret; + } + + + /** + * Retrieve an optional configuration option with one of the given values. + * + * This will check that the configuration option matches one of the given values. The match will use + * strict comparison. An exception will be thrown if it does not match. + * + * The option is optional. The default value is automatically included in the list of allowed values. + * + * @param string $name The name of the option. + * @param array $allowedValues The values the option is allowed to take, as an array. + * @param mixed $default The default value which will be returned if the option isn't found. + * The default value can be any value, including null. + * + * @return mixed The option with the given name, or $default if the option isn't found and $default is given. + * + * @throws \SimpleSAML\Assert\AssertionFailedException If the option does not have any of the allowed values. + */ + 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; } - return $ret; + return $this->getValueValidate($name, $allowedValues); } diff --git a/modules/saml/lib/Message.php b/modules/saml/lib/Message.php index 8eb1c842856b97c573dc81bde955dbd1a2fa6931..32108fc3eaed8088b9c3b3c79ae915efdf521054 100644 --- a/modules/saml/lib/Message.php +++ b/modules/saml/lib/Message.php @@ -483,7 +483,7 @@ class Message $ar->setForceAuthn($spMetadata->getOptionalBoolean('ForceAuthn', false)); $ar->setIsPassive($spMetadata->getOptionalBoolean('IsPassive', false)); - $protbind = $spMetadata->getValueValidate('ProtocolBinding', [ + $protbind = $spMetadata->getOptionalValueValidate('ProtocolBinding', [ Constants::BINDING_HTTP_POST, Constants::BINDING_HOK_SSO, Constants::BINDING_HTTP_ARTIFACT, @@ -504,7 +504,7 @@ class Message if ($spMetadata->hasValue('AuthnContextClassRef')) { $accr = $spMetadata->getArrayizeString('AuthnContextClassRef'); - $comp = $spMetadata->getValueValidate('AuthnContextComparison', [ + $comp = $spMetadata->getOptionalValueValidate('AuthnContextComparison', [ Constants::COMPARISON_EXACT, Constants::COMPARISON_MINIMUM, Constants::COMPARISON_MAXIMUM, diff --git a/tests/lib/SimpleSAML/ConfigurationTest.php b/tests/lib/SimpleSAML/ConfigurationTest.php index ba7a5dd5ca69737cf48d23069354b38ccec5ac08..578ac02c94f8908e87d0ff8f6f55833f6ac900fa 100644 --- a/tests/lib/SimpleSAML/ConfigurationTest.php +++ b/tests/lib/SimpleSAML/ConfigurationTest.php @@ -467,21 +467,38 @@ class ConfigurationTest extends ClearStateTestCase $c = Configuration::loadFromArray([ 'opt' => 'b', ]); - $this->assertEquals($c->getValueValidate('missing_opt', ['a', 'b', 'c'], '--missing--'), '--missing--'); + + // Normal use $this->assertEquals($c->getValueValidate('opt', ['a', 'b', 'c']), 'b'); + + // Value not allowed + $this->expectException(AssertionFailedException::class); + $c->getValueValidate('opt', ['d', 'e', 'f']); + + // Missing option + $this->expectException(AssertionFailedException::class); + $c->getValueValidate('missing_opt', ['a', 'b', 'c']); } /** - * Test \SimpleSAML\Configuration::getValueValidate() wrong option + * Test \SimpleSAML\Configuration::getOptionalValueValidate() */ - public function testGetValueValidateWrong(): void + public function testGetOptionalValueValidate(): void { - $this->expectException(Exception::class); $c = Configuration::loadFromArray([ - 'opt' => 'd', + 'opt' => 'b', ]); - $c->getValueValidate('opt', ['a', 'b', 'c']); + + // Normal use + $this->assertEquals($c->getOptionalValueValidate('opt', ['a', 'b', 'c'], 'f'), 'b'); + + // Missing option + $this->assertEquals($c->getOptionalValueValidate('missing_opt', ['a', 'b', 'c'], 'f'), 'f'); + + // Value not allowed + $this->expectException(AssertionFailedException::class); + $c->getOptionalValueValidate('opt', ['d', 'e', 'f'], 'c'); }