diff --git a/docs/simplesamlphp-changelog.md b/docs/simplesamlphp-changelog.md index 2ad95b068674abe74049244ddb8b902f423748d5..9ef80f9a064abb73956cc83701e14f91de357411 100644 --- a/docs/simplesamlphp-changelog.md +++ b/docs/simplesamlphp-changelog.md @@ -32,6 +32,9 @@ Released TBD * Add initial support for SAML Subject Id Attributes. * Allow to specify multiple supported NameIdFormats in IdP hosted and SP remote metadata. + * Allow to specifiy NameIDPolicy Format and AllowCreate in hosted SP + and remote IdP configurtion, and restore possibility to omit it + from AuthnRequests entirely. ## Version 1.16.2 diff --git a/docs/simplesamlphp-reference-idp-remote.md b/docs/simplesamlphp-reference-idp-remote.md index 9d46826b0303e86e0198907e9497b98705b3edc4..0859495d56af5809a89ddede4c4e5f59f03a3638 100644 --- a/docs/simplesamlphp-reference-idp-remote.md +++ b/docs/simplesamlphp-reference-idp-remote.md @@ -140,6 +140,15 @@ The following SAML 2.0 options are available: entry in the IdP-remote metadata overrides the option in the [SP configuration](./saml:sp). +`NameIDPolicy` +: The format of the NameID we request from this IdP: an array in the form of + `[ 'Format' => the format, 'AllowCreate' => true or false ]`. + Set to `false` instead of an array to omit sending any specific NameIDPolicy + in the AuthnRequest. + +: For compatibility purposes, `null` is equivalent to Transient and a format + can be defined as a string instead of an array. These variants are deprecated. + `sign.authnrequest` : Whether to sign authentication requests sent to this IdP. diff --git a/docs/simplesamlphp-upgrade-notes-1.17.md b/docs/simplesamlphp-upgrade-notes-1.17.md index 32a2dae1c7931343013fa49a1a6c4580bd71cbe6..376f081c6d2b1144adc26ce6ac1b061ddc993605 100644 --- a/docs/simplesamlphp-upgrade-notes-1.17.md +++ b/docs/simplesamlphp-upgrade-notes-1.17.md @@ -8,6 +8,12 @@ from the legacy names so calling code should remain working. Custom code (e.g. modules) that test for class names, e.g. when catching specific exceptions, may need to be changed. +The possibility has been reintroduced to omit the NameIdPolicy from SP +AuthnRequests by setting NameIDPolicy to `false`. The prefered way is +to configure it as an array `[ 'Format' => format, 'AllowCreate' => true/false ]`, +which is now also the format used in the `saml:NameIDPolicy` variable +in `$state`. + The code, config and documentation have switched to using the modern PHP array syntax. This should not have an impact as both will remain working equally, but the code examples and config templates look slightly different. diff --git a/lib/SimpleSAML/Utils/Config/Metadata.php b/lib/SimpleSAML/Utils/Config/Metadata.php index b306d200c1e8b332b622bbfab2bd66782a1ce571..7c66f290cd1aec631b3e06ac496a2cd251a2ccb6 100644 --- a/lib/SimpleSAML/Utils/Config/Metadata.php +++ b/lib/SimpleSAML/Utils/Config/Metadata.php @@ -278,4 +278,38 @@ class Metadata \SimpleSAML\Logger::popErrorMask(); return $hidden === true; } + + + /** + * This method parses the different possible values of the NameIDPolicy metadata configuration. + * + * @param mixed $nameIdPolicy + * + * @return null|array + */ + public static function parseNameIdPolicy($nameIdPolicy) + { + $policy = null; + + if (is_string($nameIdPolicy)) { + // handle old configurations where 'NameIDPolicy' was used to specify just the format + $policy = ['Format' => $nameIdPolicy]; + } elseif (is_array($nameIdPolicy)) { + // handle current configurations specifying an array in the NameIDPolicy config option + $nameIdPolicy_cf = \SimpleSAML\Configuration::loadFromArray($nameIdPolicy); + $policy = [ + 'Format' => $nameIdPolicy_cf->getString('Format', \SAML2\Constants::NAMEID_TRANSIENT), + 'AllowCreate' => $nameIdPolicy_cf->getBoolean('AllowCreate', true), + ]; + $spNameQualifier = $nameIdPolicy_cf->getString('SPNameQualifier', false); + if ($spNameQualifier !== false) { + $policy['SPNameQualifier'] = $spNameQualifier; + } + } elseif ($nameIdPolicy === null) { + // when NameIDPolicy is unset or set to null, default to transient as before + $policy = ['Format' => \SAML2\Constants::NAMEID_TRANSIENT]; + } + + return $policy; + } } diff --git a/modules/core/docs/authproc_php.md b/modules/core/docs/authproc_php.md index b913d8fe6d242d613b82afe2d1728ac409c5b174..3f8125bd1b95386f6b7cf8c089dde3c4e7cb3efb 100644 --- a/modules/core/docs/authproc_php.md +++ b/modules/core/docs/authproc_php.md @@ -54,5 +54,5 @@ Force a specific NameIdFormat. Useful if an SP misbehaves and requests (or publi 90 => array( 'class' => 'core:PHP', - 'code' => '$state["saml:NameIDFormat"] = "urn:oasis:names:tc:SAML:2.0:nameid-format:transient";' - ), \ No newline at end of file + 'code' => '$state["saml:NameIDFormat"] = ["Format" => "urn:oasis:names:tc:SAML:2.0:nameid-format:transient", "AllowCreate" => true];' + ), diff --git a/modules/saml/docs/sp.md b/modules/saml/docs/sp.md index 96f8fd510a4506773ab78184f1e19d443c33227c..dde2a0d3617fca530534728404e7536d90180ebb 100644 --- a/modules/saml/docs/sp.md +++ b/modules/saml/docs/sp.md @@ -17,7 +17,7 @@ See the documentation for those extensions for more details: Parameters -------- +---------- These are parameters that can be used at runtime to control the authentication. All these parameters override the equivalent option from the configuration. @@ -52,12 +52,6 @@ All these parameters override the equivalent option from the configuration. : *Note*: SAML 2 specific. -`saml:NameIDPolicy` -: The format of the NameID we request from the IdP. - Defaults to the transient format if unspecified. - -: *Note*: SAML 2 specific. - `saml:Extensions` : The samlp:Extensions that will be sent in the login request. @@ -69,6 +63,17 @@ All these parameters override the equivalent option from the configuration. : *Note*: SAML 2 specific. +`saml:NameIDPolicy` +: The format of the NameID we request from the IdP: an array in the form of + `[ 'Format' => the format, 'allowcreate' => true or false ]`. + Set to `false` instead of an array to omit sending any specific NameIDPolicy + in the AuthnRequest. + +: For compatibility purposes, `null` is equivalent to transient and a format + can be defined as a string instead of an array. These variants are deprecated. + +: *Note*: SAML 2 specific. + Authentication data ------------------- @@ -260,12 +265,15 @@ Options : *Note*: SAML 2 specific. `NameIDPolicy` -: The format of the NameID we request from the IdP. - Defaults to the `transient` format if unspecified. +: The format of the NameID we request from the idp: an array in the form of + `[ 'Format' => the format, 'AllowCreate' => true or false ]`. + Set to `false` instead of an array to omit sending any specific NameIDPolicy + in the AuthnRequest. -: If this option is set, its value will be added to the metadata generated for this SP, in the NameIDFormat element. +: For compatibility purposes, `null` is equivalent to transient and a format + can be defined as a string instead of an array. These variants are deprecated. -: *Note 1*: SAML 2 specific. +: *Note*: SAML 2 specific. `OrganizationName` : The name of the organization responsible for this SP. diff --git a/modules/saml/lib/Auth/Process/SQLPersistentNameID.php b/modules/saml/lib/Auth/Process/SQLPersistentNameID.php index 38b6d0c98229a0e6c24d0a439758aedeabedd1c6..172aace1c243957996a2c5677b49f49d7a1aff1b 100644 --- a/modules/saml/lib/Auth/Process/SQLPersistentNameID.php +++ b/modules/saml/lib/Auth/Process/SQLPersistentNameID.php @@ -94,7 +94,6 @@ class SQLPersistentNameID extends \SimpleSAML\Module\saml\BaseNameIDGenerator $validNameIdFormats = @array_filter([ $state['saml:NameIDFormat'], - $state['SPMetadata']['NameIDPolicy'], $state['SPMetadata']['NameIDFormat'] ]); if (count($validNameIdFormats) && !in_array($this->format, $validNameIdFormats, true) && diff --git a/modules/saml/lib/Auth/Source/SP.php b/modules/saml/lib/Auth/Source/SP.php index e1bb494241922b5e8838a894c6adfae73902b83f..d2cab6d8c3d01bbb6146d6dce0780ddae96368d5 100644 --- a/modules/saml/lib/Auth/Source/SP.php +++ b/modules/saml/lib/Auth/Source/SP.php @@ -236,6 +236,7 @@ class SP extends Source } if (isset($state['saml:NameIDPolicy'])) { + $policy = null; if (is_string($state['saml:NameIDPolicy'])) { $policy = [ 'Format' => (string) $state['saml:NameIDPolicy'], @@ -243,10 +244,12 @@ class SP extends Source ]; } elseif (is_array($state['saml:NameIDPolicy'])) { $policy = $state['saml:NameIDPolicy']; - } else { - throw new \SimpleSAML\Error\Exception('Invalid value of $state[\'saml:NameIDPolicy\'].'); + } elseif ($state['saml:NameIDPolicy'] === null) { + $policy = ['Format' => \SAML2\Constants::NAMEID_TRANSIENT]; + } + if ($policy !== null) { + $ar->setNameIdPolicy($policy); } - $ar->setNameIdPolicy($policy); } $IDPList = []; diff --git a/modules/saml/lib/Message.php b/modules/saml/lib/Message.php index 1af7164e83deb500bace45ee125e7fb1349324c2..7c1d5be68589aaf83009a59701b3c30d45bad3ec 100644 --- a/modules/saml/lib/Message.php +++ b/modules/saml/lib/Message.php @@ -471,29 +471,19 @@ class Message $ar = new \SAML2\AuthnRequest(); // get the NameIDPolicy to apply. IdP metadata has precedence. - $nameIdPolicy = []; + $nameIdPolicy = null; if ($idpMetadata->hasValue('NameIDPolicy')) { $nameIdPolicy = $idpMetadata->getValue('NameIDPolicy'); } elseif ($spMetadata->hasValue('NameIDPolicy')) { $nameIdPolicy = $spMetadata->getValue('NameIDPolicy'); } - if (!is_array($nameIdPolicy)) { - // handle old configurations where 'NameIDPolicy' was used to specify just the format - $nameIdPolicy = ['Format' => $nameIdPolicy]; + $policy = \SimpleSAML\Utils\Config\Metadata::parseNameIdPolicy($nameIdPolicy); + if ($policy !== null) { + // either we have a policy set, or we used the transient default + $ar->setNameIdPolicy($policy); } - $nameIdPolicy_cf = \SimpleSAML\Configuration::loadFromArray($nameIdPolicy); - $policy = [ - 'Format' => $nameIdPolicy_cf->getString('Format', \SAML2\Constants::NAMEID_TRANSIENT), - 'AllowCreate' => $nameIdPolicy_cf->getBoolean('AllowCreate', true), - ]; - $spNameQualifier = $nameIdPolicy_cf->getString('SPNameQualifier', false); - if ($spNameQualifier !== false) { - $policy['SPNameQualifier'] = $spNameQualifier; - } - $ar->setNameIdPolicy($policy); - $ar->setForceAuthn($spMetadata->getBoolean('ForceAuthn', false)); $ar->setIsPassive($spMetadata->getBoolean('IsPassive', false)); diff --git a/modules/saml/www/sp/metadata.php b/modules/saml/www/sp/metadata.php index 355218d0820c791696b16f6ff4ae839c3ce0d82b..1289d31e9880e28f0a9b54b0e3fd4a0ff7ab5575 100644 --- a/modules/saml/www/sp/metadata.php +++ b/modules/saml/www/sp/metadata.php @@ -139,9 +139,16 @@ if ($certInfo !== null && array_key_exists('certData', $certInfo)) { $certData = null; } -$format = $spconfig->getString('NameIDPolicy', null); +$format = $spconfig->getValue('NameIDPolicy', null); if ($format !== null) { - $metaArray20['NameIDFormat'] = $format; + if (is_array($format)) { + $metaArray20['NameIDFormat'] = \SimpleSAML\Configuration::loadFromArray($format)->getString( + 'Format', + \SAML2\Constants::NAMEID_TRANSIENT + ); + } elseif (is_string($format)) { + $metaArray20['NameIDFormat'] = $format; + } } $name = $spconfig->getLocalizedString('name', null); diff --git a/tests/lib/SimpleSAML/Utils/Config/MetadataTest.php b/tests/lib/SimpleSAML/Utils/Config/MetadataTest.php index 71002024435d92608bbd1ab3e8d92bca5b206063..21f56b65e8365c1444d3fce167f7464619625804 100644 --- a/tests/lib/SimpleSAML/Utils/Config/MetadataTest.php +++ b/tests/lib/SimpleSAML/Utils/Config/MetadataTest.php @@ -259,4 +259,44 @@ class MetadataTest extends TestCase ], ])); } + + + /** + * Test \SimpleSAML\Utils\Config\Metadata::parseNameIdPolicy(). + */ + public function testParseNameIdPolicy() + { + // Test null or unset + $nameIdPolicy = null; + $this->assertEquals(['Format' => \SAML2\Constants::NAMEID_TRANSIENT], Metadata::parseNameIdPolicy($nameIdPolicy)); + + // Test false + $nameIdPolicy = false; + $this->assertEquals(null, Metadata::parseNameIdPolicy($nameIdPolicy)); + + // Test string + $nameIdPolicy = 'urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress'; + $this->assertEquals(['Format' => 'urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress'], Metadata::parseNameIdPolicy($nameIdPolicy)); + + // Test array + $nameIdPolicy = [ + 'Format' => 'urn:oasis:names:tc:SAML:1.1:nameid-format:persistent', + 'AllowCreate' => false + ]; + $this->assertEquals([ + 'Format' => 'urn:oasis:names:tc:SAML:1.1:nameid-format:persistent', + 'AllowCreate' => false + ], Metadata::parseNameIdPolicy($nameIdPolicy)); + + $nameIdPolicy = [ + 'Format' => 'urn:oasis:names:tc:SAML:1.1:nameid-format:persistent', + 'AllowCreate' => false, + 'SPNameQualifier' => 'TEST' + ]; + $this->assertEquals([ + 'Format' => 'urn:oasis:names:tc:SAML:1.1:nameid-format:persistent', + 'AllowCreate' => false, + 'SPNameQualifier' => 'TEST' + ], Metadata::parseNameIdPolicy($nameIdPolicy)); + } } diff --git a/tests/modules/consent/lib/Auth/Process/ConsentTest.php b/tests/modules/consent/lib/Auth/Process/ConsentTest.php index 1e162314720ab0cdeda95d15f02456c15b9c8a94..e9008f3d6584d21861525038bf0853668fa004d8 100644 --- a/tests/modules/consent/lib/Auth/Process/ConsentTest.php +++ b/tests/modules/consent/lib/Auth/Process/ConsentTest.php @@ -16,7 +16,7 @@ class ConsentTest extends TestCase public function setUp() { $this->config = Configuration::loadFromArray(['module.enable' => ['consent' => true]], '[ARRAY]', 'simplesaml'); - \SimpleSAML_Configuration::setPreLoadedConfig($this->config, 'config.php'); + Configuration::setPreLoadedConfig($this->config, 'config.php'); } /**