From 2cba3c078149ef6f26b3610643fbf559aa46d737 Mon Sep 17 00:00:00 2001 From: Bobby Lawrence Date: Wed, 13 Sep 2017 17:36:01 -0400 Subject: [PATCH 01/31] updated \SAML2\Assertion to use \SAML2\XML\saml\Attribute objects instead of an array of string|int|DOMElement. This is a fairly major change, but the getAttributes and setAttributes methods have default params to handle backwards compatibility. This is a "fix" for issue https://github.com/simplesamlphp/simplesamlphp/issues/690 --- src/SAML2/Assertion.php | 303 ++++++++++++++++++++++---- src/SAML2/Binding.php | 11 +- src/SAML2/HTTPArtifact.php | 10 + src/SAML2/HTTPHoK.php | 42 ++++ src/SAML2/HTTPPost.php | 10 + src/SAML2/HTTPRedirect.php | 10 + src/SAML2/SOAP.php | 7 + src/SAML2/XML/saml/AttributeValue.php | 24 +- 8 files changed, 368 insertions(+), 49 deletions(-) create mode 100644 src/SAML2/HTTPHoK.php diff --git a/src/SAML2/Assertion.php b/src/SAML2/Assertion.php index 48f1b4cbc..2ed8c5eb5 100644 --- a/src/SAML2/Assertion.php +++ b/src/SAML2/Assertion.php @@ -70,9 +70,9 @@ class Assertion extends SignedElement /** * The encrypted Attributes. * - * If this is not an empty array, these Attributes need decryption before they can be used. + * If this is not null, these Attributes need decryption before they can be accessed. * - * @var \DOMElement[] + * @var \DOMElement[]|null */ private $encryptedAttributes; @@ -165,20 +165,10 @@ class Assertion extends SignedElement private $AuthenticatingAuthority = []; /** - * The attributes, as an associative array, indexed by attribute name + * The attributes, as an associative array. * - * To ease handling, all attribute values are represented as an array of values, also for values with a multiplicity - * of single. There are 5 possible variants of datatypes for the values: a string, an integer, an array, a - * DOMNodeList or a SAML2\XML\saml\NameID object. - * - * If the attribute is an eduPersonTargetedID, the values will be SAML2\XML\saml\NameID objects. - * If the attribute value has an type-definition (xsi:string or xsi:int), the values will be of that type. - * If the attribute value contains a nested XML structure, the values will be a DOMNodeList - * In all other cases the values are treated as strings - * - * **WARNING** a DOMNodeList cannot be serialized without data-loss and should be handled explicitly - * - * @var array multi-dimensional array of \DOMNodeList|\SAML2\XML\saml\NameID|string|int|array + * @var array array of attributes indexed by attribute name with each value an SAML2\XML\saml\Attribute + * or an SAML2\XML\saml\NameID object if the attribute is an eduPersonTargetedID */ private $attributes = []; @@ -200,6 +190,22 @@ class Assertion extends SignedElement */ private $attributesValueTypes = []; + /** + * The attributes NameFormats + * the variable is as an associative array, indexed by attribute name + * + * @var array + */ + private $attributeNameFormats; + + /** + * The attributes FriendlyNames + * the variable is as an associative array, indexed by attribute name + * + * @var array + */ + private $attributeFriendlyNames; + /** * The NameFormat used on all attributes. * @@ -257,6 +263,18 @@ public function __construct(DOMElement $xml = null) $this->issueInstant = Temporal::getTime(); $this->issuer = $issuer; $this->authnInstant = Temporal::getTime(); +<<<<<<< HEAD +======= + $this->attributes = array(); + $this->attributesValueTypes = array(); + $this->attributeNameFormats = array(); + $this->attributeFriendlyNames = array(); + $this->nameFormat = Constants::NAMEFORMAT_UNSPECIFIED; + $this->certificates = array(); + $this->AuthenticatingAuthority = array(); + $this->SubjectConfirmation = array(); + $this->requiredEncAttributes = false; +>>>>>>> updated \SAML2\Assertion to use \SAML2\XML\saml\Attribute objects instead of an array of string|int|DOMElement. if ($xml === null) { return; @@ -544,11 +562,14 @@ private function parseAttributes(DOMElement $xml) : void } } +<<<<<<< HEAD if (!array_key_exists($name, $this->attributes)) { $this->attributes[$name] = []; $this->attributesValueTypes[$name] = []; } +======= +>>>>>>> updated \SAML2\Assertion to use \SAML2\XML\saml\Attribute objects instead of an array of string|int|DOMElement. $this->parseAttributeValue($attribute, $name); } } @@ -559,10 +580,22 @@ private function parseAttributes(DOMElement $xml) : void * @param string $attributeName * @return void */ +<<<<<<< HEAD private function parseAttributeValue(DOMNode $attribute, string $attributeName) : void - { - /** @var \DOMElement[] $values */ +======= + private function parseAttributeValue(\DOMNode $attribute, $attributeName) +>>>>>>> updated \SAML2\Assertion to use \SAML2\XML\saml\Attribute objects instead of an array of string|int|DOMElement. + { + $this->attributes[$attributeName] = new XML\saml\Attribute($attribute); + + if (!array_key_exists($attributeName, $this->attributesValueTypes)) { + $this->attributesValueTypes[$attributeName] = array(); + } + $this->attributeNameFormats[$attributeName] = $attribute->getAttribute('NameFormat'); + $this->attributeFriendlyNames[$attributeName] = $attribute->getAttribute('FriendlyName'); + $values = Utils::xpQuery($attribute, './saml_assertion:AttributeValue'); +<<<<<<< HEAD if ($attributeName === Constants::EPTI_URN_MACE || $attributeName === Constants::EPTI_URN_OID) { foreach ($values as $index => $eptiAttributeValue) { @@ -579,27 +612,25 @@ private function parseAttributeValue(DOMNode $attribute, string $attributeName) $nameId = new NameID(); $nameId->setValue($eptiAttributeValue->textContent); $this->attributes[$attributeName][] = $nameId; +======= + foreach ($values as $index => $value) { + if ($attributeName === Constants::EPTI_URN_MACE || $attributeName === Constants::EPTI_URN_OID) { + $eptiNameId = Utils::xpQuery($value, './saml_assertion:NameID'); + if (count($eptiNameId) !== 1) { + throw new RuntimeException(sprintf( + 'A "%s" (EPTI) attribute value must be a NameID, none found for value no. "%d"', + $attributeName, + $index + )); +>>>>>>> updated \SAML2\Assertion to use \SAML2\XML\saml\Attribute objects instead of an array of string|int|DOMElement. } } - - return; - } - - foreach ($values as $value) { - $hasNonTextChildElements = false; - foreach ($value->childNodes as $childNode) { - /** @var \DOMNode $childNode */ - if ($childNode->nodeType !== XML_TEXT_NODE) { - $hasNonTextChildElements = true; - break; - } - } - $type = $value->getAttribute('xsi:type'); if ($type === '') { $type = null; } $this->attributesValueTypes[$attributeName][] = $type; +<<<<<<< HEAD if ($hasNonTextChildElements) { $this->attributes[$attributeName][] = $value->childNodes; @@ -611,6 +642,8 @@ private function parseAttributeValue(DOMNode $attribute, string $attributeName) } else { $this->attributes[$attributeName][] = trim($value->textContent); } +======= +>>>>>>> updated \SAML2\Assertion to use \SAML2\XML\saml\Attribute objects instead of an array of string|int|DOMElement. } } @@ -859,7 +892,7 @@ public function decryptNameId(XMLSecurityKey $key, array $blacklist = []) : void */ public function hasEncryptedAttributes() : bool { - return $this->encryptedAttributes !== []; + return $this->encryptedAttributes !== null && $this->encryptedAttributes !== []; } @@ -906,10 +939,13 @@ public function decryptAttributes(XMLSecurityKey $key, array $blacklist = []) : } } +<<<<<<< HEAD if (!array_key_exists($name, $this->attributes)) { $this->attributes[$name] = []; } +======= +>>>>>>> updated \SAML2\Assertion to use \SAML2\XML\saml\Attribute objects instead of an array of string|int|DOMElement. $this->parseAttributeValue($attribute, $name); } } @@ -1241,12 +1277,57 @@ public function setAuthenticatingAuthority(array $authenticatingAuthority) : voi /** * Retrieve all attributes. - * + * + * @param boolean Indicates that returned array should be actual attribute objects instead of strings * @return array All attributes, as an associative array. */ +<<<<<<< HEAD public function getAttributes() : array +======= + public function getAttributes($asObjects = false) +>>>>>>> updated \SAML2\Assertion to use \SAML2\XML\saml\Attribute objects instead of an array of string|int|DOMElement. { - return $this->attributes; + if ($asObjects) { + return $this->attributes; + } + $compatArray = array(); + foreach ($this->attributes as $attributeName => $attributeObj) { + $compatArray[$attributeName] = array(); + + if ($attributeObj instanceof \SAML2\XML\saml\NameID) { + $compatArray[$attributeName][] = $attributeObj; + continue; + } + + foreach ($attributeObj->AttributeValue as $attributeValue) { + if ($attributeObj->Name === Constants::EPTI_URN_MACE || $attributeObj->Name === Constants::EPTI_URN_OID) { + $eptiNameId = Utils::xpQuery($attributeValue->element, './saml_assertion:NameID'); + if (count($eptiNameId) === 1) { + $compatArray[$attributeName][] = new XML\saml\NameID($eptiNameId[0]); + continue; + } + } + + $hasNonTextChildElements = false; + foreach ($attributeValue->element->childNodes as $childNode) { + if ($childNode->nodeType !== XML_TEXT_NODE) { + $hasNonTextChildElements = true; + break; + } + } + if ($hasNonTextChildElements) { + $compatArray[$attributeName][] = $attributeValue->element->childNodes; + continue; + } + + if ($attributeValue->element->getAttribute('xsi:type') === 'xs:integer') { + $compatArray[$attributeName][] = (int)$attributeValue->getString(); + } else { + $compatArray[$attributeName][] = trim($attributeValue->getString()); + } + } + } + return $compatArray; } @@ -1258,7 +1339,54 @@ public function getAttributes() : array */ public function setAttributes(array $attributes) : void { - $this->attributes = $attributes; + if (!isset($attributes)) { + $this->attributes = null; + return; + } + $this->attributes = array(); + foreach ($attributes as $name => $value) { + if ($value instanceof \SAML2\XML\saml\Attribute || $value instanceof \SAML2\XML\saml\NameID) { + $this->attributes[$name] = $value; + continue; + } + $this->attributes[$name] = null; + if (is_array($value)) { + + $document = DOMDocumentFactory::create(); + $attrDomElement = $document->createElementNS(Constants::NS_SAML, 'saml:Attribute'); + $document->appendChild($attrDomElement); + $attrDomElement->setAttribute('Name', $name); + + if ($this->nameFormat !== null) { + $attrDomElement->setAttribute('NameFormat', $this->nameFormat); + } + if (array_key_exists($name, $this->attributeNameFormats)) { + $attrDomElement->setAttribute('NameFormat', $this->attributeNameFormats[$name]); + } + if (array_key_exists($name, $this->attributeFriendlyNames)) { + $attrDomElement->setAttribute('FriendlyName', $this->attributeFriendlyNames[$name]); + } + + $attributeObj = new \SAML2\XML\saml\Attribute($attrDomElement); + + foreach ($value as $vidx => $attributeValue) { + $attributeValueObj = new \SAML2\XML\saml\AttributeValue($attributeValue); + $type = null; + if (isset($this->attributesValueTypes[$name])) { + if (is_array($this->attributesValueTypes[$name])) { + $type = $this->attributesValueTypes[$name][$vidx]; + } else { + $type = $this->attributesValueTypes[$name]; + } + if ($type !== null) { + $attributeValueObj->element->setAttributeNS(Constants::NS_XSI, 'xsi:type', $type); + } + } + $attributeObj->AttributeValue[] = $attributeValueObj; + } + $this->attributes[$name] = $attributeObj; + } + } } /** @@ -1303,6 +1431,46 @@ public function setAttributesValueTypes(array $attributesValueTypes) : void } + /** + * Retrieve all attribute name formats. + * + * @return array All attribute name formats, as an associative array. + */ + public function getAttributeNameFormats() + { + return $this->attributeNameFormats; + } + + /** + * Replace all attribute name formats + * + * @param array $attributeNameFormats All new attribute name formats, as an associative array. + */ + public function setAttributeNameFormats(array $attributeNameFormats) + { + $this->attributeNameFormats = $attributeNameFormats; + } + + /** + * Retrieve all attribute friendly names. + * + * @return array All attribute friendly names, as an associative array. + */ + public function getAttributeFriendlyNames() + { + return $this->attributeFriendlyNames; + } + + /** + * Replace all attribute friendly names + * + * @param array $attributeFriendlyNames All new attribute friendly names, as an associative array. + */ + public function setAttributeFriendlyNames(array $attributeFriendlyNames) + { + $this->attributeFriendlyNames = $attributeFriendlyNames; + } + /** * Retrieve the NameFormat used on all attributes. * @@ -1654,6 +1822,7 @@ private function addAttributeStatement(DOMElement $root) : void $attributeStatement = $document->createElementNS(Constants::NS_SAML, 'saml:AttributeStatement'); $root->appendChild($attributeStatement); +<<<<<<< HEAD foreach ($this->attributes as $name => $values) { $attribute = $document->createElementNS(Constants::NS_SAML, 'saml:Attribute'); $attributeStatement->appendChild($attribute); @@ -1738,7 +1907,17 @@ private function addAttributeStatement(DOMElement $root) : void $value = strval($value); $attributeValue->appendChild($document->createTextNode($value)); } +======= + foreach ($this->attributes as $name => $attributeObj) { + + // possibly override the xsi type for the current attribute + if (is_array($this->attributesValueTypes) && + array_key_exists($attributeObj->Name, $this->attributesValueTypes)) { + $this->overrideAttributeType($attributeObj); +>>>>>>> updated \SAML2\Assertion to use \SAML2\XML\saml\Attribute objects instead of an array of string|int|DOMElement. } + + $attributeObj->toXML($attributeStatement); } } @@ -1759,18 +1938,22 @@ private function addEncryptedAttributeStatement(DOMElement $root) : void $document = $root->ownerDocument; $attributeStatement = $document->createElementNS(Constants::NS_SAML, 'saml:AttributeStatement'); - $root->appendChild($attributeStatement); - foreach ($this->attributes as $name => $values) { - $document2 = DOMDocumentFactory::create(); - $attribute = $document2->createElementNS(Constants::NS_SAML, 'saml:Attribute'); - $attribute->setAttribute('Name', $name); - $document2->appendChild($attribute); + foreach ($this->attributes as $name => $attributeObj) { +<<<<<<< HEAD if ($this->nameFormat !== Constants::NAMEFORMAT_UNSPECIFIED) { $attribute->setAttribute('NameFormat', $this->getAttributeNameFormat()); +======= + // possibly override the xsi type for the current attribute + if (is_array($this->attributesValueTypes) && + array_key_exists($attributeObj->Name, $this->attributesValueTypes)) { + $this->overrideAttributeType($attributeObj); +>>>>>>> updated \SAML2\Assertion to use \SAML2\XML\saml\Attribute objects instead of an array of string|int|DOMElement. } + $attributeElement = $attributeObj->toXML($attributeStatement); +<<<<<<< HEAD foreach ($values as $value) { if (is_string($value)) { $type = 'xs:string'; @@ -1797,8 +1980,11 @@ private function addEncryptedAttributeStatement(DOMElement $root) : void } } /*Once the attribute nodes are built, the are encrypted*/ +======= + /*Once the attribute nodes are built, they are encrypted*/ +>>>>>>> updated \SAML2\Assertion to use \SAML2\XML\saml\Attribute objects instead of an array of string|int|DOMElement. $EncAssert = new XMLSecEnc(); - $EncAssert->setNode($document2->documentElement); + $EncAssert->setNode($attributeElement); $EncAssert->type = 'http://www.w3.org/2001/04/xmlenc#Element'; /* * Attributes are encrypted with a session key and this one with @@ -1817,5 +2003,38 @@ private function addEncryptedAttributeStatement(DOMElement $root) : void $n = $document->importNode($EncrNode, true); $EncAttribute->appendChild($n); } + $root->appendChild($attributeStatement); + } + + /** + * Apply an attribute type override to the attribute's values. The xsi type of each attribute is inferred based on its datatype + * via the php gettype function inside XML\saml\AttributeValue, but not all xsi types can be inferred in this manner. + * Simple xsi types like decimal, double, date, dateTime, etc as well as custom types can only be set via overrides + * + * @param XML\saml\Attribute $attributeObj The actual attribute object to apply the override to. + */ + private function overrideAttributeType(XML\saml\Attribute &$attributeObj) + { + $valueTypes = $this->attributesValueTypes[$attributeObj->Name]; + if ($valueTypes === null) { + return; + } + if (is_array($valueTypes) && count($valueTypes) != count($attributeObj->AttributeValue)) { + throw new \Exception( + 'Array of value types and array of values have different size for attribute '. + var_export($attributeObj->Name, true) + ); + } + foreach ($attributeObj->AttributeValue as $vidx => &$attributeValue) { + $type = null; + if (is_array($valueTypes)) { + $type = $valueTypes[$vidx]; + } else { + $type = $valueTypes; + } + if ($type !== null) { + $attributeValue->element->setAttributeNS(Constants::NS_XSI, 'xsi:type', $type); + } + } } } diff --git a/src/SAML2/Binding.php b/src/SAML2/Binding.php index 8ec7bd3f4..7324f87c5 100644 --- a/src/SAML2/Binding.php +++ b/src/SAML2/Binding.php @@ -39,7 +39,9 @@ public static function getBinding(string $urn) : Binding case Constants::BINDING_HTTP_ARTIFACT: return new HTTPArtifact(); case Constants::BINDING_HOK_SSO: - return new HTTPPost(); + return new HTTPHoK(); + case Constants::BINDING_SOAP: + return new SOAP(); // ECP ACS is defined with the PAOS binding, but as the IdP, we // talk to the ECP using SOAP -- if support for ECP as an SP is // implemented, this logic may need to change @@ -51,6 +53,13 @@ public static function getBinding(string $urn) : Binding } + /** + * Retrieve the URN associated with the binding + * + * @return string The URN of the binding. + */ + abstract public function getURN(); + /** * Guess the current binding. * diff --git a/src/SAML2/HTTPArtifact.php b/src/SAML2/HTTPArtifact.php index b692fcda1..f60e34787 100644 --- a/src/SAML2/HTTPArtifact.php +++ b/src/SAML2/HTTPArtifact.php @@ -28,6 +28,16 @@ class HTTPArtifact extends Binding private $spMetadata; + /** + * Return the URN of this binding + * + * @return string The URN of the binding + */ + public function getURN() + { + return SAML2_Const::BINDING_HTTP_ARTIFACT; + } + /** * Create the redirect URL for a message. * diff --git a/src/SAML2/HTTPHoK.php b/src/SAML2/HTTPHoK.php new file mode 100644 index 000000000..bc75e0dd0 --- /dev/null +++ b/src/SAML2/HTTPHoK.php @@ -0,0 +1,42 @@ +getLogger(); + $logger->warning('Missing client presented certificate.'); + } + return parent::receive(); + } +} diff --git a/src/SAML2/HTTPPost.php b/src/SAML2/HTTPPost.php index 40e0a8815..7d92fe6fc 100644 --- a/src/SAML2/HTTPPost.php +++ b/src/SAML2/HTTPPost.php @@ -11,6 +11,16 @@ */ class HTTPPost extends Binding { + /** + * Return the URN of this binding + * + * @return string The URN of the binding + */ + public function getURN() + { + return SAML2_Const::BINDING_HTTP_POST; + } + /** * Send a SAML 2 message using the HTTP-POST binding. * diff --git a/src/SAML2/HTTPRedirect.php b/src/SAML2/HTTPRedirect.php index 8463c02a6..3973cc068 100644 --- a/src/SAML2/HTTPRedirect.php +++ b/src/SAML2/HTTPRedirect.php @@ -16,6 +16,16 @@ class HTTPRedirect extends Binding { const DEFLATE = 'urn:oasis:names:tc:SAML:2.0:bindings:URL-Encoding:DEFLATE'; + /** + * Return the URN of this binding + * + * @return string The URN of the binding + */ + public function getURN() + { + return SAML2_Const::BINDING_HTTP_REDIRECT; + } + /** * Create the redirect URL for a message. * diff --git a/src/SAML2/SOAP.php b/src/SAML2/SOAP.php index bb374cf67..f0b1a7072 100644 --- a/src/SAML2/SOAP.php +++ b/src/SAML2/SOAP.php @@ -16,10 +16,17 @@ class SOAP extends Binding { /** + * Return the URN of this binding + * * @param Message $message * @throws \Exception * @return string|false The XML or false on error */ + public function getURN() + { + return SAML2_Const::BINDING_SOAP; + } + public function getOutputToSend(Message $message) { $envelope = <<element = $doc->createElementNS(Constants::NS_SAML, 'saml:AttributeValue'); - $this->element->setAttributeNS(Constants::NS_XSI, 'xsi:type', 'xs:string'); - $this->element->appendChild($doc->createTextNode($value)); + if (is_null($value)) { + $this->element->setAttributeNS(Constants::NS_XSI, 'xsi:nil', 'true'); + } else { + $this->element->setAttributeNS(Constants::NS_XSI, 'xsi:type', 'xs:'.gettype($value)); + $this->element->appendChild($doc->createTextNode($value)); + } /* Make sure that the xs-namespace is available in the AttributeValue (for xs:string). */ $this->element->setAttributeNS(Constants::NS_XS, 'xs:tmp', 'tmp'); $this->element->removeAttributeNS(Constants::NS_XS, 'tmp'); return; } - + + if ($value instanceof \XML\saml\NameID) { + $this->element = $value->toXML(); + return; + } + if ($value->namespaceURI === Constants::NS_SAML && $value->localName === 'AttributeValue') { $this->element = Utils::copyElement($value); return; From af4c8d32736a2ae89d83ace4a4184963ddee2620 Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Tue, 22 May 2018 16:39:10 +0200 Subject: [PATCH 02/31] Remove code intended for other PR --- src/SAML2/Binding.php | 11 +--------- src/SAML2/HTTPArtifact.php | 10 --------- src/SAML2/HTTPHoK.php | 42 -------------------------------------- src/SAML2/HTTPPost.php | 10 --------- src/SAML2/HTTPRedirect.php | 10 --------- src/SAML2/SOAP.php | 2 ++ 6 files changed, 3 insertions(+), 82 deletions(-) delete mode 100644 src/SAML2/HTTPHoK.php diff --git a/src/SAML2/Binding.php b/src/SAML2/Binding.php index 7324f87c5..8ec7bd3f4 100644 --- a/src/SAML2/Binding.php +++ b/src/SAML2/Binding.php @@ -39,9 +39,7 @@ public static function getBinding(string $urn) : Binding case Constants::BINDING_HTTP_ARTIFACT: return new HTTPArtifact(); case Constants::BINDING_HOK_SSO: - return new HTTPHoK(); - case Constants::BINDING_SOAP: - return new SOAP(); + return new HTTPPost(); // ECP ACS is defined with the PAOS binding, but as the IdP, we // talk to the ECP using SOAP -- if support for ECP as an SP is // implemented, this logic may need to change @@ -53,13 +51,6 @@ public static function getBinding(string $urn) : Binding } - /** - * Retrieve the URN associated with the binding - * - * @return string The URN of the binding. - */ - abstract public function getURN(); - /** * Guess the current binding. * diff --git a/src/SAML2/HTTPArtifact.php b/src/SAML2/HTTPArtifact.php index f60e34787..b692fcda1 100644 --- a/src/SAML2/HTTPArtifact.php +++ b/src/SAML2/HTTPArtifact.php @@ -28,16 +28,6 @@ class HTTPArtifact extends Binding private $spMetadata; - /** - * Return the URN of this binding - * - * @return string The URN of the binding - */ - public function getURN() - { - return SAML2_Const::BINDING_HTTP_ARTIFACT; - } - /** * Create the redirect URL for a message. * diff --git a/src/SAML2/HTTPHoK.php b/src/SAML2/HTTPHoK.php deleted file mode 100644 index bc75e0dd0..000000000 --- a/src/SAML2/HTTPHoK.php +++ /dev/null @@ -1,42 +0,0 @@ -getLogger(); - $logger->warning('Missing client presented certificate.'); - } - return parent::receive(); - } -} diff --git a/src/SAML2/HTTPPost.php b/src/SAML2/HTTPPost.php index 7d92fe6fc..40e0a8815 100644 --- a/src/SAML2/HTTPPost.php +++ b/src/SAML2/HTTPPost.php @@ -11,16 +11,6 @@ */ class HTTPPost extends Binding { - /** - * Return the URN of this binding - * - * @return string The URN of the binding - */ - public function getURN() - { - return SAML2_Const::BINDING_HTTP_POST; - } - /** * Send a SAML 2 message using the HTTP-POST binding. * diff --git a/src/SAML2/HTTPRedirect.php b/src/SAML2/HTTPRedirect.php index 3973cc068..8463c02a6 100644 --- a/src/SAML2/HTTPRedirect.php +++ b/src/SAML2/HTTPRedirect.php @@ -16,16 +16,6 @@ class HTTPRedirect extends Binding { const DEFLATE = 'urn:oasis:names:tc:SAML:2.0:bindings:URL-Encoding:DEFLATE'; - /** - * Return the URN of this binding - * - * @return string The URN of the binding - */ - public function getURN() - { - return SAML2_Const::BINDING_HTTP_REDIRECT; - } - /** * Create the redirect URL for a message. * diff --git a/src/SAML2/SOAP.php b/src/SAML2/SOAP.php index f0b1a7072..1fa676232 100644 --- a/src/SAML2/SOAP.php +++ b/src/SAML2/SOAP.php @@ -27,6 +27,7 @@ public function getURN() return SAML2_Const::BINDING_SOAP; } + public function getOutputToSend(Message $message) { $envelope = << Date: Tue, 22 May 2018 19:52:50 +0200 Subject: [PATCH 03/31] Fix namespaces --- src/SAML2/XML/saml/AttributeValue.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/SAML2/XML/saml/AttributeValue.php b/src/SAML2/XML/saml/AttributeValue.php index 9a3875dbf..b63ca903b 100644 --- a/src/SAML2/XML/saml/AttributeValue.php +++ b/src/SAML2/XML/saml/AttributeValue.php @@ -33,7 +33,7 @@ class AttributeValue implements \Serializable * * @param mixed $value The value of this element. Can be one of: * - a scalar Create an attribute value with a simple value. - * - a \XML\saml\NameID Create an attribute value of the given NameID. + * - a NameID Create an attribute value of the given NameID. * - \DOMElement(AttributeValue) Create an attribute value of the given DOMElement. * - \DOMElement Create an attribute value with the given DOMElement as a child. */ @@ -57,7 +57,7 @@ public function __construct($value) return; } - if ($value instanceof \XML\saml\NameID) { + if ($value instanceof NameID) { $this->element = $value->toXML(); return; } From bc6b063e73080e13692192581529d5f57bd84aee Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Fri, 8 Mar 2019 20:28:03 +0100 Subject: [PATCH 04/31] Resolve some issues/conflichts --- src/SAML2/Assertion.php | 45 --------------------------- src/SAML2/XML/saml/AttributeValue.php | 1 - 2 files changed, 46 deletions(-) diff --git a/src/SAML2/Assertion.php b/src/SAML2/Assertion.php index 2ed8c5eb5..983214e75 100644 --- a/src/SAML2/Assertion.php +++ b/src/SAML2/Assertion.php @@ -263,18 +263,6 @@ public function __construct(DOMElement $xml = null) $this->issueInstant = Temporal::getTime(); $this->issuer = $issuer; $this->authnInstant = Temporal::getTime(); -<<<<<<< HEAD -======= - $this->attributes = array(); - $this->attributesValueTypes = array(); - $this->attributeNameFormats = array(); - $this->attributeFriendlyNames = array(); - $this->nameFormat = Constants::NAMEFORMAT_UNSPECIFIED; - $this->certificates = array(); - $this->AuthenticatingAuthority = array(); - $this->SubjectConfirmation = array(); - $this->requiredEncAttributes = false; ->>>>>>> updated \SAML2\Assertion to use \SAML2\XML\saml\Attribute objects instead of an array of string|int|DOMElement. if ($xml === null) { return; @@ -562,14 +550,11 @@ private function parseAttributes(DOMElement $xml) : void } } -<<<<<<< HEAD if (!array_key_exists($name, $this->attributes)) { $this->attributes[$name] = []; $this->attributesValueTypes[$name] = []; } -======= ->>>>>>> updated \SAML2\Assertion to use \SAML2\XML\saml\Attribute objects instead of an array of string|int|DOMElement. $this->parseAttributeValue($attribute, $name); } } @@ -580,11 +565,7 @@ private function parseAttributes(DOMElement $xml) : void * @param string $attributeName * @return void */ -<<<<<<< HEAD private function parseAttributeValue(DOMNode $attribute, string $attributeName) : void -======= - private function parseAttributeValue(\DOMNode $attribute, $attributeName) ->>>>>>> updated \SAML2\Assertion to use \SAML2\XML\saml\Attribute objects instead of an array of string|int|DOMElement. { $this->attributes[$attributeName] = new XML\saml\Attribute($attribute); @@ -595,7 +576,6 @@ private function parseAttributeValue(\DOMNode $attribute, $attributeName) $this->attributeFriendlyNames[$attributeName] = $attribute->getAttribute('FriendlyName'); $values = Utils::xpQuery($attribute, './saml_assertion:AttributeValue'); -<<<<<<< HEAD if ($attributeName === Constants::EPTI_URN_MACE || $attributeName === Constants::EPTI_URN_OID) { foreach ($values as $index => $eptiAttributeValue) { @@ -612,17 +592,6 @@ private function parseAttributeValue(\DOMNode $attribute, $attributeName) $nameId = new NameID(); $nameId->setValue($eptiAttributeValue->textContent); $this->attributes[$attributeName][] = $nameId; -======= - foreach ($values as $index => $value) { - if ($attributeName === Constants::EPTI_URN_MACE || $attributeName === Constants::EPTI_URN_OID) { - $eptiNameId = Utils::xpQuery($value, './saml_assertion:NameID'); - if (count($eptiNameId) !== 1) { - throw new RuntimeException(sprintf( - 'A "%s" (EPTI) attribute value must be a NameID, none found for value no. "%d"', - $attributeName, - $index - )); ->>>>>>> updated \SAML2\Assertion to use \SAML2\XML\saml\Attribute objects instead of an array of string|int|DOMElement. } } $type = $value->getAttribute('xsi:type'); @@ -630,7 +599,6 @@ private function parseAttributeValue(\DOMNode $attribute, $attributeName) $type = null; } $this->attributesValueTypes[$attributeName][] = $type; -<<<<<<< HEAD if ($hasNonTextChildElements) { $this->attributes[$attributeName][] = $value->childNodes; @@ -642,8 +610,6 @@ private function parseAttributeValue(\DOMNode $attribute, $attributeName) } else { $this->attributes[$attributeName][] = trim($value->textContent); } -======= ->>>>>>> updated \SAML2\Assertion to use \SAML2\XML\saml\Attribute objects instead of an array of string|int|DOMElement. } } @@ -939,13 +905,10 @@ public function decryptAttributes(XMLSecurityKey $key, array $blacklist = []) : } } -<<<<<<< HEAD if (!array_key_exists($name, $this->attributes)) { $this->attributes[$name] = []; } -======= ->>>>>>> updated \SAML2\Assertion to use \SAML2\XML\saml\Attribute objects instead of an array of string|int|DOMElement. $this->parseAttributeValue($attribute, $name); } } @@ -1281,11 +1244,7 @@ public function setAuthenticatingAuthority(array $authenticatingAuthority) : voi * @param boolean Indicates that returned array should be actual attribute objects instead of strings * @return array All attributes, as an associative array. */ -<<<<<<< HEAD public function getAttributes() : array -======= - public function getAttributes($asObjects = false) ->>>>>>> updated \SAML2\Assertion to use \SAML2\XML\saml\Attribute objects instead of an array of string|int|DOMElement. { if ($asObjects) { return $this->attributes; @@ -1953,7 +1912,6 @@ private function addEncryptedAttributeStatement(DOMElement $root) : void } $attributeElement = $attributeObj->toXML($attributeStatement); -<<<<<<< HEAD foreach ($values as $value) { if (is_string($value)) { $type = 'xs:string'; @@ -1979,10 +1937,7 @@ private function addEncryptedAttributeStatement(DOMElement $root) : void $attributeValue->appendChild($document2->createTextNode($value)); } } - /*Once the attribute nodes are built, the are encrypted*/ -======= /*Once the attribute nodes are built, they are encrypted*/ ->>>>>>> updated \SAML2\Assertion to use \SAML2\XML\saml\Attribute objects instead of an array of string|int|DOMElement. $EncAssert = new XMLSecEnc(); $EncAssert->setNode($attributeElement); $EncAssert->type = 'http://www.w3.org/2001/04/xmlenc#Element'; diff --git a/src/SAML2/XML/saml/AttributeValue.php b/src/SAML2/XML/saml/AttributeValue.php index b63ca903b..2b44ed648 100644 --- a/src/SAML2/XML/saml/AttributeValue.php +++ b/src/SAML2/XML/saml/AttributeValue.php @@ -1,4 +1,3 @@ - Date: Thu, 12 Dec 2019 15:33:38 +0100 Subject: [PATCH 05/31] Fix missing param --- src/SAML2/Assertion.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/SAML2/Assertion.php b/src/SAML2/Assertion.php index 9865cbe28..fd768cd49 100644 --- a/src/SAML2/Assertion.php +++ b/src/SAML2/Assertion.php @@ -1243,12 +1243,12 @@ public function setAuthenticatingAuthority(array $authenticatingAuthority): void * @param boolean Indicates that returned array should be actual attribute objects instead of strings * @return array All attributes, as an associative array. */ - public function getAttributes(): array + public function getAttributes(bool $asObjects = false): array { if ($asObjects) { return $this->attributes; } - $compatArray = array(); + $compatArray = []; foreach ($this->attributes as $attributeName => $attributeObj) { $compatArray[$attributeName] = array(); From fb57550d0b128d8fcb9624638dd273a8b14c36fd Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Fri, 13 Dec 2019 11:56:21 +0100 Subject: [PATCH 06/31] Resolve conflict --- src/SAML2/Assertion.php | 137 ++-------------------------------------- 1 file changed, 6 insertions(+), 131 deletions(-) diff --git a/src/SAML2/Assertion.php b/src/SAML2/Assertion.php index fd768cd49..80bb6ec36 100644 --- a/src/SAML2/Assertion.php +++ b/src/SAML2/Assertion.php @@ -598,17 +598,6 @@ private function parseAttributeValue(DOMNode $attribute, string $attributeName): $type = null; } $this->attributesValueTypes[$attributeName][] = $type; - - if ($hasNonTextChildElements) { - $this->attributes[$attributeName][] = $value->childNodes; - continue; - } - - if ($type === 'xs:integer') { - $this->attributes[$attributeName][] = (int) $value->textContent; - } else { - $this->attributes[$attributeName][] = trim($value->textContent); - } } } @@ -1781,99 +1770,12 @@ private function addAttributeStatement(DOMElement $root): void $attributeStatement = $document->createElementNS(Constants::NS_SAML, 'saml:AttributeStatement'); $root->appendChild($attributeStatement); -<<<<<<< HEAD - foreach ($this->attributes as $name => $values) { - $attribute = $document->createElementNS(Constants::NS_SAML, 'saml:Attribute'); - $attributeStatement->appendChild($attribute); - $attribute->setAttribute('Name', $name); - - if ($this->nameFormat !== Constants::NAMEFORMAT_UNSPECIFIED) { - $attribute->setAttribute('NameFormat', $this->nameFormat); - } - - // make sure eduPersonTargetedID can be handled properly as a NameID - if ($name === Constants::EPTI_URN_MACE || $name === Constants::EPTI_URN_OID) { - foreach ($values as $eptiValue) { - $attributeValue = $document->createElementNS(Constants::NS_SAML, 'saml:AttributeValue'); - $attribute->appendChild($attributeValue); - if ($eptiValue instanceof NameID) { - $eptiValue->toXML($attributeValue); - } elseif ($eptiValue instanceof DOMNodeList) { - /** @var \DOMElement $value */ - $value = $eptiValue->item(0); - $node = $root->ownerDocument->importNode($value, true); - $attributeValue->appendChild($node); - } else { - $attributeValue->textContent = $eptiValue; - } - } - - continue; - } - - // get value type(s) for the current attribute - if (array_key_exists($name, $this->attributesValueTypes)) { - $valueTypes = $this->attributesValueTypes[$name]; - if (is_array($valueTypes) && count($valueTypes) != count($values)) { - throw new \Exception('Array of value types and array of values have different size for attribute ' . - var_export($name, true)); - } - } else { - // if no type(s), default behaviour - $valueTypes = null; - } - - $vidx = -1; - foreach ($values as $value) { - $vidx++; - - // try to get type from current types - $type = null; - if (!is_null($valueTypes)) { - if (is_array($valueTypes)) { - $type = $valueTypes[$vidx]; - } else { - $type = $valueTypes; - } - } - - // if no type get from types, use default behaviour - if (is_null($type)) { - if (is_string($value)) { - $type = 'xs:string'; - } elseif (is_int($value)) { - $type = 'xs:integer'; - } else { - $type = null; - } - } - - $attributeValue = $document->createElementNS(Constants::NS_SAML, 'saml:AttributeValue'); - $attribute->appendChild($attributeValue); - if ($type !== null) { - $attributeValue->setAttributeNS(Constants::NS_XSI, 'xsi:type', $type); - } - if (is_null($value)) { - $attributeValue->setAttributeNS(Constants::NS_XSI, 'xsi:nil', 'true'); - } - - if ($value instanceof \DOMNodeList) { - foreach ($value as $v) { - $node = $document->importNode($v, true); - $attributeValue->appendChild($node); - } - } else { - $value = strval($value); - $attributeValue->appendChild($document->createTextNode($value)); - } -======= foreach ($this->attributes as $name => $attributeObj) { // possibly override the xsi type for the current attribute if (is_array($this->attributesValueTypes) && array_key_exists($attributeObj->Name, $this->attributesValueTypes)) { $this->overrideAttributeType($attributeObj); ->>>>>>> updated \SAML2\Assertion to use \SAML2\XML\saml\Attribute objects instead of an array of string|int|DOMElement. } $attributeObj->toXML($attributeStatement); @@ -1899,61 +1801,34 @@ private function addEncryptedAttributeStatement(DOMElement $root): void $attributeStatement = $document->createElementNS(Constants::NS_SAML, 'saml:AttributeStatement'); foreach ($this->attributes as $name => $attributeObj) { - -<<<<<<< HEAD - if ($this->nameFormat !== Constants::NAMEFORMAT_UNSPECIFIED) { - $attribute->setAttribute('NameFormat', $this->getAttributeNameFormat()); -======= // possibly override the xsi type for the current attribute if (is_array($this->attributesValueTypes) && array_key_exists($attributeObj->Name, $this->attributesValueTypes)) { $this->overrideAttributeType($attributeObj); ->>>>>>> updated \SAML2\Assertion to use \SAML2\XML\saml\Attribute objects instead of an array of string|int|DOMElement. } $attributeElement = $attributeObj->toXML($attributeStatement); - foreach ($values as $value) { - if (is_string($value)) { - $type = 'xs:string'; - } elseif (is_int($value)) { - $type = 'xs:integer'; - } else { - $type = null; - } - - $attributeValue = $document2->createElementNS(Constants::NS_SAML, 'saml:AttributeValue'); - $attribute->appendChild($attributeValue); - if ($type !== null) { - $attributeValue->setAttributeNS(Constants::NS_XSI, 'xsi:type', $type); - } - - if ($value instanceof DOMNodeList) { - foreach ($value as $v) { - $node = $document2->importNode($v, true); - $attributeValue->appendChild($node); - } - } else { - $value = strval($value); - $attributeValue->appendChild($document2->createTextNode($value)); - } - } - /*Once the attribute nodes are built, they are encrypted*/ + // Once the attribute nodes are built, they are encrypted $EncAssert = new XMLSecEnc(); $EncAssert->setNode($attributeElement); $EncAssert->type = 'http://www.w3.org/2001/04/xmlenc#Element'; - /* + + /** * Attributes are encrypted with a session key and this one with * $EncryptionKey */ $symmetricKey = new XMLSecurityKey(XMLSecurityKey::AES256_CBC); $symmetricKey->generateSessionKey(); + /** @psalm-suppress PossiblyNullArgument */ $EncAssert->encryptKey($this->encryptionKey, $symmetricKey); + /** @psalm-suppress UndefinedClass */ $EncrNode = $EncAssert->encryptNode($symmetricKey); $EncAttribute = $document->createElementNS(Constants::NS_SAML, 'saml:EncryptedAttribute'); $attributeStatement->appendChild($EncAttribute); + /** @psalm-suppress InvalidArgument */ $n = $document->importNode($EncrNode, true); $EncAttribute->appendChild($n); From 0a806a6c10fcddb4060478a37688159372718067 Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Fri, 13 Dec 2019 11:57:04 +0100 Subject: [PATCH 07/31] Shorthand array --- src/SAML2/Assertion.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/SAML2/Assertion.php b/src/SAML2/Assertion.php index 80bb6ec36..133cc7dfc 100644 --- a/src/SAML2/Assertion.php +++ b/src/SAML2/Assertion.php @@ -569,7 +569,7 @@ private function parseAttributeValue(DOMNode $attribute, string $attributeName): $this->attributes[$attributeName] = new XML\saml\Attribute($attribute); if (!array_key_exists($attributeName, $this->attributesValueTypes)) { - $this->attributesValueTypes[$attributeName] = array(); + $this->attributesValueTypes[$attributeName] = []; } $this->attributeNameFormats[$attributeName] = $attribute->getAttribute('NameFormat'); $this->attributeFriendlyNames[$attributeName] = $attribute->getAttribute('FriendlyName'); @@ -1239,7 +1239,7 @@ public function getAttributes(bool $asObjects = false): array } $compatArray = []; foreach ($this->attributes as $attributeName => $attributeObj) { - $compatArray[$attributeName] = array(); + $compatArray[$attributeName] = []; if ($attributeObj instanceof \SAML2\XML\saml\NameID) { $compatArray[$attributeName][] = $attributeObj; @@ -1290,7 +1290,7 @@ public function setAttributes(array $attributes): void $this->attributes = null; return; } - $this->attributes = array(); + $this->attributes = []; foreach ($attributes as $name => $value) { if ($value instanceof \SAML2\XML\saml\Attribute || $value instanceof \SAML2\XML\saml\NameID) { $this->attributes[$name] = $value; From f7dee5ad0856332ec58269e5d021de58bdcd5eb8 Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Fri, 13 Dec 2019 12:11:23 +0100 Subject: [PATCH 08/31] Fix unitialized vars --- src/SAML2/Assertion.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/SAML2/Assertion.php b/src/SAML2/Assertion.php index 133cc7dfc..6d789c5a9 100644 --- a/src/SAML2/Assertion.php +++ b/src/SAML2/Assertion.php @@ -195,7 +195,7 @@ class Assertion extends SignedElement * * @var array */ - private $attributeNameFormats; + private $attributeNameFormats = []; /** * The attributes FriendlyNames @@ -203,7 +203,7 @@ class Assertion extends SignedElement * * @var array */ - private $attributeFriendlyNames; + private $attributeFriendlyNames = []; /** * The NameFormat used on all attributes. @@ -550,7 +550,10 @@ private function parseAttributes(DOMElement $xml): void } if (!array_key_exists($name, $this->attributes)) { - $this->attributes[$name] = []; + $attr = new Attribute(); + $attr->setName($name); + + $this->attributes[$name] = $attr; $this->attributesValueTypes[$name] = []; } From ee5aa92363f63168a9bfb369e0615e219a91988d Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Fri, 13 Dec 2019 13:32:13 +0100 Subject: [PATCH 09/31] Fix implicit cast to string --- src/SAML2/Assertion.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SAML2/Assertion.php b/src/SAML2/Assertion.php index 6d789c5a9..899197305 100644 --- a/src/SAML2/Assertion.php +++ b/src/SAML2/Assertion.php @@ -1320,7 +1320,7 @@ public function setAttributes(array $attributes): void $attributeObj = new \SAML2\XML\saml\Attribute($attrDomElement); foreach ($value as $vidx => $attributeValue) { - $attributeValueObj = new \SAML2\XML\saml\AttributeValue($attributeValue); + $attributeValueObj = new \SAML2\XML\saml\AttributeValue(strval($attributeValue)); $type = null; if (isset($this->attributesValueTypes[$name])) { if (is_array($this->attributesValueTypes[$name])) { From 7a73a718282b1711e9ea7e3dd0b5e8e9d3de5d85 Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Fri, 13 Dec 2019 13:14:34 +0100 Subject: [PATCH 10/31] Fix accessing private properties --- src/SAML2/Assertion.php | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/SAML2/Assertion.php b/src/SAML2/Assertion.php index 899197305..2dfd169fb 100644 --- a/src/SAML2/Assertion.php +++ b/src/SAML2/Assertion.php @@ -1249,9 +1249,9 @@ public function getAttributes(bool $asObjects = false): array continue; } - foreach ($attributeObj->AttributeValue as $attributeValue) { - if ($attributeObj->Name === Constants::EPTI_URN_MACE || $attributeObj->Name === Constants::EPTI_URN_OID) { - $eptiNameId = Utils::xpQuery($attributeValue->element, './saml_assertion:NameID'); + foreach ($attributeObj->getAttributeValue() as $attributeValue) { + if ($attributeObj->getName() === Constants::EPTI_URN_MACE || $attributeObj->getName() === Constants::EPTI_URN_OID) { + $eptiNameId = Utils::xpQuery($attributeValue->getElement(), './saml_assertion:NameID'); if (count($eptiNameId) === 1) { $compatArray[$attributeName][] = new XML\saml\NameID($eptiNameId[0]); continue; @@ -1259,18 +1259,18 @@ public function getAttributes(bool $asObjects = false): array } $hasNonTextChildElements = false; - foreach ($attributeValue->element->childNodes as $childNode) { + foreach ($attributeValue->getElement()->childNodes as $childNode) { if ($childNode->nodeType !== XML_TEXT_NODE) { $hasNonTextChildElements = true; break; } } if ($hasNonTextChildElements) { - $compatArray[$attributeName][] = $attributeValue->element->childNodes; + $compatArray[$attributeName][] = $attributeValue->getElement()->childNodes; continue; } - if ($attributeValue->element->getAttribute('xsi:type') === 'xs:integer') { + if ($attributeValue->getElement()->getAttribute('xsi:type') === 'xs:integer') { $compatArray[$attributeName][] = (int)$attributeValue->getString(); } else { $compatArray[$attributeName][] = trim($attributeValue->getString()); @@ -1332,7 +1332,7 @@ public function setAttributes(array $attributes): void $attributeValueObj->element->setAttributeNS(Constants::NS_XSI, 'xsi:type', $type); } } - $attributeObj->AttributeValue[] = $attributeValueObj; + $attributeObj->addAttributeValue($attributeValueObj); } $this->attributes[$name] = $attributeObj; } @@ -1777,7 +1777,7 @@ private function addAttributeStatement(DOMElement $root): void // possibly override the xsi type for the current attribute if (is_array($this->attributesValueTypes) && - array_key_exists($attributeObj->Name, $this->attributesValueTypes)) { + array_key_exists($attributeObj->getName(), $this->attributesValueTypes)) { $this->overrideAttributeType($attributeObj); } @@ -1806,7 +1806,7 @@ private function addEncryptedAttributeStatement(DOMElement $root): void foreach ($this->attributes as $name => $attributeObj) { // possibly override the xsi type for the current attribute if (is_array($this->attributesValueTypes) && - array_key_exists($attributeObj->Name, $this->attributesValueTypes)) { + array_key_exists($attributeObj->getName(), $this->attributesValueTypes)) { $this->overrideAttributeType($attributeObj); } $attributeElement = $attributeObj->toXML($attributeStatement); @@ -1848,17 +1848,17 @@ private function addEncryptedAttributeStatement(DOMElement $root): void */ private function overrideAttributeType(XML\saml\Attribute &$attributeObj) { - $valueTypes = $this->attributesValueTypes[$attributeObj->Name]; + $valueTypes = $this->attributesValueTypes[$attributeObj->getName()]; if ($valueTypes === null) { return; } - if (is_array($valueTypes) && count($valueTypes) != count($attributeObj->AttributeValue)) { + if (is_array($valueTypes) && count($valueTypes) != count($attributeObj->getAttributeValue())) { throw new \Exception( 'Array of value types and array of values have different size for attribute '. - var_export($attributeObj->Name, true) + var_export($attributeObj->getName(), true) ); } - foreach ($attributeObj->AttributeValue as $vidx => &$attributeValue) { + foreach ($attributeObj->getAttributeValue() as $vidx => &$attributeValue) { $type = null; if (is_array($valueTypes)) { $type = $valueTypes[$vidx]; @@ -1866,7 +1866,7 @@ private function overrideAttributeType(XML\saml\Attribute &$attributeObj) $type = $valueTypes; } if ($type !== null) { - $attributeValue->element->setAttributeNS(Constants::NS_XSI, 'xsi:type', $type); + $attributeValue->getElement()->setAttributeNS(Constants::NS_XSI, 'xsi:type', $type); } } } From dde142b1ac04e418d3b9a64cd3e1601419b96fb3 Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Fri, 13 Dec 2019 16:19:10 +0100 Subject: [PATCH 11/31] Several fixes --- src/SAML2/Assertion.php | 75 ++++++++++++++++++++++++++++------------- 1 file changed, 51 insertions(+), 24 deletions(-) diff --git a/src/SAML2/Assertion.php b/src/SAML2/Assertion.php index 2dfd169fb..2e8a0acc2 100644 --- a/src/SAML2/Assertion.php +++ b/src/SAML2/Assertion.php @@ -12,6 +12,8 @@ use SAML2\Exception\RuntimeException; use SAML2\Utilities\Temporal; use SAML2\XML\Chunk; +use SAML2\XML\saml\Attribute; +use SAML2\XML\saml\AttributeValue; use SAML2\XML\saml\Issuer; use SAML2\XML\saml\NameID; use SAML2\XML\saml\SubjectConfirmation; @@ -569,14 +571,7 @@ private function parseAttributes(DOMElement $xml): void */ private function parseAttributeValue(DOMNode $attribute, string $attributeName): void { - $this->attributes[$attributeName] = new XML\saml\Attribute($attribute); - - if (!array_key_exists($attributeName, $this->attributesValueTypes)) { - $this->attributesValueTypes[$attributeName] = []; - } - $this->attributeNameFormats[$attributeName] = $attribute->getAttribute('NameFormat'); - $this->attributeFriendlyNames[$attributeName] = $attribute->getAttribute('FriendlyName'); - + /** @var \DOMElement[] $values */ $values = Utils::xpQuery($attribute, './saml_assertion:AttributeValue'); if ($attributeName === Constants::EPTI_URN_MACE || $attributeName === Constants::EPTI_URN_OID) { @@ -585,22 +580,50 @@ private function parseAttributeValue(DOMNode $attribute, string $attributeName): $eptiNameId = Utils::xpQuery($eptiAttributeValue, './saml_assertion:NameID'); if (count($eptiNameId) === 1) { - $this->attributes[$attributeName][] = new NameID($eptiNameId[0]); + $nameId = new NameID($eptiNameId[0]); + $this->attributes[$attributeName]->addAttributeValue( + new AttributeValue($nameId->toXML()->textContent) + ); } else { /* Fall back for legacy IdPs sending string value (e.g. SSP < 1.15) */ Utils::getContainer()->getLogger()->warning( sprintf("Attribute %s (EPTI) value %d is not an XML NameId", $attributeName, $index) ); - $nameId = new NameID(); - $nameId->setValue($eptiAttributeValue->textContent); - $this->attributes[$attributeName][] = $nameId; + $this->attributes[$attributeName]->addAttributeValue( + new AttributeValue($eptiAttributeValue->textContent) + ); } } + + return; + } + + foreach ($values as $value) { + $hasNonTextChildElements = false; + foreach ($value->childNodes as $childNode) { + /** @var \DOMNode $childNode */ + if ($childNode->nodeType !== XML_TEXT_NODE) { + $hasNonTextChildElements = true; + break; + } + } + $type = $value->getAttribute('xsi:type'); if ($type === '') { $type = null; } $this->attributesValueTypes[$attributeName][] = $type; + + if ($hasNonTextChildElements) { + $this->attributes[$attributeName]->addAttributeValue( + new AttributeValue($value->childNodes) + ); + continue; + } + + $this->attributes[$attributeName]->addAttributeValue( + new AttributeValue(trim($value->textContent)) + ); } } @@ -897,7 +920,10 @@ public function decryptAttributes(XMLSecurityKey $key, array $blacklist = []): v } if (!array_key_exists($name, $this->attributes)) { - $this->attributes[$name] = []; + $attr = new Attribute(); + $attr->setName($name); + + $this->attributes[$name] = $attr; } $this->parseAttributeValue($attribute, $name); @@ -1244,7 +1270,7 @@ public function getAttributes(bool $asObjects = false): array foreach ($this->attributes as $attributeName => $attributeObj) { $compatArray[$attributeName] = []; - if ($attributeObj instanceof \SAML2\XML\saml\NameID) { + if ($attributeObj instanceof NameID) { $compatArray[$attributeName][] = $attributeObj; continue; } @@ -1253,7 +1279,10 @@ public function getAttributes(bool $asObjects = false): array if ($attributeObj->getName() === Constants::EPTI_URN_MACE || $attributeObj->getName() === Constants::EPTI_URN_OID) { $eptiNameId = Utils::xpQuery($attributeValue->getElement(), './saml_assertion:NameID'); if (count($eptiNameId) === 1) { - $compatArray[$attributeName][] = new XML\saml\NameID($eptiNameId[0]); + $nameId = new NameID($eptiNameId[0]); + $compatArray[$attributeName]->addAttributeValue( + new AttributeValue($nameId->toXML()->textContent) + ); continue; } } @@ -1289,19 +1318,17 @@ public function getAttributes(bool $asObjects = false): array */ public function setAttributes(array $attributes): void { - if (!isset($attributes)) { - $this->attributes = null; + if (empty($attributes)) { return; } - $this->attributes = []; + foreach ($attributes as $name => $value) { - if ($value instanceof \SAML2\XML\saml\Attribute || $value instanceof \SAML2\XML\saml\NameID) { + if ($value instanceof Attribute || $value instanceof NameID) { $this->attributes[$name] = $value; continue; } $this->attributes[$name] = null; if (is_array($value)) { - $document = DOMDocumentFactory::create(); $attrDomElement = $document->createElementNS(Constants::NS_SAML, 'saml:Attribute'); $document->appendChild($attrDomElement); @@ -1317,10 +1344,10 @@ public function setAttributes(array $attributes): void $attrDomElement->setAttribute('FriendlyName', $this->attributeFriendlyNames[$name]); } - $attributeObj = new \SAML2\XML\saml\Attribute($attrDomElement); + $attributeObj = new Attribute($attrDomElement); foreach ($value as $vidx => $attributeValue) { - $attributeValueObj = new \SAML2\XML\saml\AttributeValue(strval($attributeValue)); + $attributeValueObj = new AttributeValue(strval($attributeValue)); $type = null; if (isset($this->attributesValueTypes[$name])) { if (is_array($this->attributesValueTypes[$name])) { @@ -1329,7 +1356,7 @@ public function setAttributes(array $attributes): void $type = $this->attributesValueTypes[$name]; } if ($type !== null) { - $attributeValueObj->element->setAttributeNS(Constants::NS_XSI, 'xsi:type', $type); + $attributeValueObj->getElement()->setAttributeNS(Constants::NS_XSI, 'xsi:type', $type); } } $attributeObj->addAttributeValue($attributeValueObj); @@ -1846,7 +1873,7 @@ private function addEncryptedAttributeStatement(DOMElement $root): void * * @param XML\saml\Attribute $attributeObj The actual attribute object to apply the override to. */ - private function overrideAttributeType(XML\saml\Attribute &$attributeObj) + private function overrideAttributeType(Attribute &$attributeObj) { $valueTypes = $this->attributesValueTypes[$attributeObj->getName()]; if ($valueTypes === null) { From ce3a4dee9b3e2c0fc726a53c4c234437828a6500 Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Fri, 13 Dec 2019 17:20:15 +0100 Subject: [PATCH 12/31] Fix tests --- tests/SAML2/AssertionTest.php | 42 ++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/tests/SAML2/AssertionTest.php b/tests/SAML2/AssertionTest.php index 301d6e820..a27547afb 100644 --- a/tests/SAML2/AssertionTest.php +++ b/tests/SAML2/AssertionTest.php @@ -9,6 +9,8 @@ use SAML2\DOMDocumentFactory; use SAML2\Utils; use SAML2\XML\Chunk; +use SAML2\XML\saml\Attribute; +use SAML2\XML\saml\AttributeValue; use SAML2\XML\saml\Issuer; use SAML2\XML\saml\NameID; use SAML2\XML\saml\SubjectConfirmation; @@ -140,11 +142,20 @@ public function testMarshallingUnmarshallingChristmas(): void $assertion->setAuthenticatingAuthority(["idp1", "idp2"]); - $assertion->setAttributes([ - "name1" => ["value1", "value2"], - "name2" => [2], - "name3" => [null] - ]); + $attr1 = new Attribute(); + $attr1->setName("name1"); + $attr1->addAttributeValue(new AttributeValue("value1")); + $attr1->addAttributeValue(new AttributeValue("value2")); + + $attr2 = new Attribute(); + $attr2->setName("name2"); + $attr2->addAttributeValue(new AttributeValue(2)); + + $attr3 = new Attribute(); + $attr3->setName("name3"); + $attr3->addAttributeValue(new AttributeValue(null)); + + $assertion->setAttributes([$attr1, $attr2, $attr3]); $assertion->setAttributeNameFormat("urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified"); $assertionElement = $assertion->toXML()->ownerDocument->saveXML(); @@ -195,11 +206,22 @@ public function testMarshallingUnmarshallingAttributeValTypes(): void $assertion->setAuthenticatingAuthority(["idp1", "idp2"]); - $assertion->setAttributes([ - "name1" => ["value1",123,"2017-31-12"], - "name2" => [2], - "name3" => [1234, "+2345"] - ]); + $attr1 = new Attribute(); + $attr1->setName("name1"); + $attr1->addAttributeValue(new AttributeValue("value1")); + $attr1->addAttributeValue(new AttributeValue(123)); + $attr1->addAttributeValue(new AttributeValue("2017-31-12")); + + $attr2 = new Attribute("name2"); + $attr2->setName("name2"); + $attr2->addAttributeValue(new AttributeValue(2)); + + $attr3 = new Attribute("name3"); + $attr3->setName("name3"); + $attr3->addAttributeValue(new AttributeValue(1234)); + $attr3->addAttributeValue(new AttributeValue("+2345")); + + $assertion->setAttributes([$attr1, $attr2, $attr3]); $assertion->setAttributeNameFormat(\SAML2\Constants::NAMEFORMAT_UNSPECIFIED); // set xs:type for first and third name1 values, and all name3 values. From cba702f730f82ffb66696c5e31022191eed0b293 Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Fri, 13 Dec 2019 18:18:48 +0100 Subject: [PATCH 13/31] Undo unrelated change; it should not have ended up in this PR --- src/SAML2/SOAP.php | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/SAML2/SOAP.php b/src/SAML2/SOAP.php index c1905e74b..6d05dbb3a 100644 --- a/src/SAML2/SOAP.php +++ b/src/SAML2/SOAP.php @@ -15,18 +15,10 @@ class SOAP extends Binding { /** - * Return the URN of this binding - * * @param Message $message * @throws \Exception * @return string|false The XML or false on error */ - public function getURN() - { - return SAML2_Const::BINDING_SOAP; - } - - public function getOutputToSend(Message $message) { $envelope = << Date: Fri, 13 Dec 2019 21:51:45 +0100 Subject: [PATCH 14/31] Add missing typehints --- src/SAML2/Assertion.php | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/SAML2/Assertion.php b/src/SAML2/Assertion.php index 2e8a0acc2..b99fc1976 100644 --- a/src/SAML2/Assertion.php +++ b/src/SAML2/Assertion.php @@ -1413,41 +1413,45 @@ public function setAttributesValueTypes(array $attributesValueTypes): void * * @return array All attribute name formats, as an associative array. */ - public function getAttributeNameFormats() + public function getAttributeNameFormats(): array { return $this->attributeNameFormats; } + /** * Replace all attribute name formats * * @param array $attributeNameFormats All new attribute name formats, as an associative array. */ - public function setAttributeNameFormats(array $attributeNameFormats) + public function setAttributeNameFormats(array $attributeNameFormats): array { $this->attributeNameFormats = $attributeNameFormats; } + /** * Retrieve all attribute friendly names. * * @return array All attribute friendly names, as an associative array. */ - public function getAttributeFriendlyNames() + public function getAttributeFriendlyNames(): array { return $this->attributeFriendlyNames; } + /** * Replace all attribute friendly names * * @param array $attributeFriendlyNames All new attribute friendly names, as an associative array. */ - public function setAttributeFriendlyNames(array $attributeFriendlyNames) + public function setAttributeFriendlyNames(array $attributeFriendlyNames): array { $this->attributeFriendlyNames = $attributeFriendlyNames; } + /** * Retrieve the NameFormat used on all attributes. * @@ -1872,8 +1876,9 @@ private function addEncryptedAttributeStatement(DOMElement $root): void * Simple xsi types like decimal, double, date, dateTime, etc as well as custom types can only be set via overrides * * @param XML\saml\Attribute $attributeObj The actual attribute object to apply the override to. + * @return void */ - private function overrideAttributeType(Attribute &$attributeObj) + private function overrideAttributeType(Attribute &$attributeObj): void { $valueTypes = $this->attributesValueTypes[$attributeObj->getName()]; if ($valueTypes === null) { From 8ad00a53cf87d9d3ab8bea9b6f5766998bcb116f Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Fri, 13 Dec 2019 22:10:21 +0100 Subject: [PATCH 15/31] RedundantConditionGivenDocblockType --- src/SAML2/Assertion.php | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/SAML2/Assertion.php b/src/SAML2/Assertion.php index b99fc1976..a7c84f8a9 100644 --- a/src/SAML2/Assertion.php +++ b/src/SAML2/Assertion.php @@ -1805,10 +1805,8 @@ private function addAttributeStatement(DOMElement $root): void $root->appendChild($attributeStatement); foreach ($this->attributes as $name => $attributeObj) { - // possibly override the xsi type for the current attribute - if (is_array($this->attributesValueTypes) && - array_key_exists($attributeObj->getName(), $this->attributesValueTypes)) { + if (array_key_exists($attributeObj->getName(), $this->attributesValueTypes)) { $this->overrideAttributeType($attributeObj); } @@ -1836,8 +1834,7 @@ private function addEncryptedAttributeStatement(DOMElement $root): void foreach ($this->attributes as $name => $attributeObj) { // possibly override the xsi type for the current attribute - if (is_array($this->attributesValueTypes) && - array_key_exists($attributeObj->getName(), $this->attributesValueTypes)) { + if (array_key_exists($attributeObj->getName(), $this->attributesValueTypes)) { $this->overrideAttributeType($attributeObj); } $attributeElement = $attributeObj->toXML($attributeStatement); From 21efc499878858738e44bc6c508b629b06827dfc Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Fri, 13 Dec 2019 22:13:06 +0100 Subject: [PATCH 16/31] Undo unrelated change --- src/SAML2/Assertion.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SAML2/Assertion.php b/src/SAML2/Assertion.php index a7c84f8a9..056527e4c 100644 --- a/src/SAML2/Assertion.php +++ b/src/SAML2/Assertion.php @@ -73,7 +73,7 @@ class Assertion extends SignedElement * * If this is not null, these Attributes need decryption before they can be accessed. * - * @var \DOMElement[]|null + * @var \DOMElement[] */ private $encryptedAttributes; From 25bd8d771480c3f0acc8802a88accf2fd1400fe9 Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Sun, 15 Dec 2019 16:46:12 +0100 Subject: [PATCH 17/31] Only Attribute-objects are allowed in the attributes-array --- src/SAML2/Assertion.php | 215 +--------------------------------------- 1 file changed, 5 insertions(+), 210 deletions(-) diff --git a/src/SAML2/Assertion.php b/src/SAML2/Assertion.php index 056527e4c..264512eb1 100644 --- a/src/SAML2/Assertion.php +++ b/src/SAML2/Assertion.php @@ -173,40 +173,6 @@ class Assertion extends SignedElement */ private $attributes = []; - /** - * The attributes values types as per http://www.w3.org/2001/XMLSchema definitions - * the variable is as an associative array, indexed by attribute name - * - * when parsing assertion, the variable will be: - * - => [|null, |null, ...] - * array will always have the same size of the array of vaules in $attributes for the same - * - * when generating assertion, the varuable can be: - * - null : backward compatibility - * - => : all values for the given attribute will have the same xs type - * - => [|null, |null, ...] : Nth value will have type of the - * Nth in the array - * - * @var array multi-dimensional array of array - */ - private $attributesValueTypes = []; - - /** - * The attributes NameFormats - * the variable is as an associative array, indexed by attribute name - * - * @var array - */ - private $attributeNameFormats = []; - - /** - * The attributes FriendlyNames - * the variable is as an associative array, indexed by attribute name - * - * @var array - */ - private $attributeFriendlyNames = []; - /** * The NameFormat used on all attributes. * @@ -556,7 +522,6 @@ private function parseAttributes(DOMElement $xml): void $attr->setName($name); $this->attributes[$name] = $attr; - $this->attributesValueTypes[$name] = []; } $this->parseAttributeValue($attribute, $name); @@ -608,12 +573,6 @@ private function parseAttributeValue(DOMNode $attribute, string $attributeName): } } - $type = $value->getAttribute('xsi:type'); - if ($type === '') { - $type = null; - } - $this->attributesValueTypes[$attributeName][] = $type; - if ($hasNonTextChildElements) { $this->attributes[$attributeName]->addAttributeValue( new AttributeValue($value->childNodes) @@ -1258,55 +1217,11 @@ public function setAuthenticatingAuthority(array $authenticatingAuthority): void /** * Retrieve all attributes. * - * @param boolean Indicates that returned array should be actual attribute objects instead of strings * @return array All attributes, as an associative array. */ - public function getAttributes(bool $asObjects = false): array + public function getAttributes(): array { - if ($asObjects) { - return $this->attributes; - } - $compatArray = []; - foreach ($this->attributes as $attributeName => $attributeObj) { - $compatArray[$attributeName] = []; - - if ($attributeObj instanceof NameID) { - $compatArray[$attributeName][] = $attributeObj; - continue; - } - - foreach ($attributeObj->getAttributeValue() as $attributeValue) { - if ($attributeObj->getName() === Constants::EPTI_URN_MACE || $attributeObj->getName() === Constants::EPTI_URN_OID) { - $eptiNameId = Utils::xpQuery($attributeValue->getElement(), './saml_assertion:NameID'); - if (count($eptiNameId) === 1) { - $nameId = new NameID($eptiNameId[0]); - $compatArray[$attributeName]->addAttributeValue( - new AttributeValue($nameId->toXML()->textContent) - ); - continue; - } - } - - $hasNonTextChildElements = false; - foreach ($attributeValue->getElement()->childNodes as $childNode) { - if ($childNode->nodeType !== XML_TEXT_NODE) { - $hasNonTextChildElements = true; - break; - } - } - if ($hasNonTextChildElements) { - $compatArray[$attributeName][] = $attributeValue->getElement()->childNodes; - continue; - } - - if ($attributeValue->getElement()->getAttribute('xsi:type') === 'xs:integer') { - $compatArray[$attributeName][] = (int)$attributeValue->getString(); - } else { - $compatArray[$attributeName][] = trim($attributeValue->getString()); - } - } - } - return $compatArray; + return $this->attributes; } @@ -1318,51 +1233,8 @@ public function getAttributes(bool $asObjects = false): array */ public function setAttributes(array $attributes): void { - if (empty($attributes)) { - return; - } - foreach ($attributes as $name => $value) { - if ($value instanceof Attribute || $value instanceof NameID) { - $this->attributes[$name] = $value; - continue; - } - $this->attributes[$name] = null; - if (is_array($value)) { - $document = DOMDocumentFactory::create(); - $attrDomElement = $document->createElementNS(Constants::NS_SAML, 'saml:Attribute'); - $document->appendChild($attrDomElement); - $attrDomElement->setAttribute('Name', $name); - - if ($this->nameFormat !== null) { - $attrDomElement->setAttribute('NameFormat', $this->nameFormat); - } - if (array_key_exists($name, $this->attributeNameFormats)) { - $attrDomElement->setAttribute('NameFormat', $this->attributeNameFormats[$name]); - } - if (array_key_exists($name, $this->attributeFriendlyNames)) { - $attrDomElement->setAttribute('FriendlyName', $this->attributeFriendlyNames[$name]); - } - - $attributeObj = new Attribute($attrDomElement); - - foreach ($value as $vidx => $attributeValue) { - $attributeValueObj = new AttributeValue(strval($attributeValue)); - $type = null; - if (isset($this->attributesValueTypes[$name])) { - if (is_array($this->attributesValueTypes[$name])) { - $type = $this->attributesValueTypes[$name][$vidx]; - } else { - $type = $this->attributesValueTypes[$name]; - } - if ($type !== null) { - $attributeValueObj->getElement()->setAttributeNS(Constants::NS_XSI, 'xsi:type', $type); - } - } - $attributeObj->addAttributeValue($attributeValueObj); - } - $this->attributes[$name] = $attributeObj; - } + $this->attributes[$name] = $value; } } @@ -1385,73 +1257,6 @@ public function setSignatureData(array $signatureData = null): void } - /** - * Retrieve all attributes value types. - * - * @return array All attributes value types, as an associative array. - */ - public function getAttributesValueTypes(): array - { - return $this->attributesValueTypes; - } - - - /** - * Replace all attributes value types.. - * - * @param array $attributesValueTypes All new attribute value types, as an associative array. - * @return void - */ - public function setAttributesValueTypes(array $attributesValueTypes): void - { - $this->attributesValueTypes = $attributesValueTypes; - } - - - /** - * Retrieve all attribute name formats. - * - * @return array All attribute name formats, as an associative array. - */ - public function getAttributeNameFormats(): array - { - return $this->attributeNameFormats; - } - - - /** - * Replace all attribute name formats - * - * @param array $attributeNameFormats All new attribute name formats, as an associative array. - */ - public function setAttributeNameFormats(array $attributeNameFormats): array - { - $this->attributeNameFormats = $attributeNameFormats; - } - - - /** - * Retrieve all attribute friendly names. - * - * @return array All attribute friendly names, as an associative array. - */ - public function getAttributeFriendlyNames(): array - { - return $this->attributeFriendlyNames; - } - - - /** - * Replace all attribute friendly names - * - * @param array $attributeFriendlyNames All new attribute friendly names, as an associative array. - */ - public function setAttributeFriendlyNames(array $attributeFriendlyNames): array - { - $this->attributeFriendlyNames = $attributeFriendlyNames; - } - - /** * Retrieve the NameFormat used on all attributes. * @@ -1806,7 +1611,7 @@ private function addAttributeStatement(DOMElement $root): void foreach ($this->attributes as $name => $attributeObj) { // possibly override the xsi type for the current attribute - if (array_key_exists($attributeObj->getName(), $this->attributesValueTypes)) { + if (array_key_exists($attributeObj->getName(), $this->attributes)) { $this->overrideAttributeType($attributeObj); } @@ -1834,7 +1639,7 @@ private function addEncryptedAttributeStatement(DOMElement $root): void foreach ($this->attributes as $name => $attributeObj) { // possibly override the xsi type for the current attribute - if (array_key_exists($attributeObj->getName(), $this->attributesValueTypes)) { + if (array_key_exists($attributeObj->getName(), $this->attributes)) { $this->overrideAttributeType($attributeObj); } $attributeElement = $attributeObj->toXML($attributeStatement); @@ -1877,16 +1682,6 @@ private function addEncryptedAttributeStatement(DOMElement $root): void */ private function overrideAttributeType(Attribute &$attributeObj): void { - $valueTypes = $this->attributesValueTypes[$attributeObj->getName()]; - if ($valueTypes === null) { - return; - } - if (is_array($valueTypes) && count($valueTypes) != count($attributeObj->getAttributeValue())) { - throw new \Exception( - 'Array of value types and array of values have different size for attribute '. - var_export($attributeObj->getName(), true) - ); - } foreach ($attributeObj->getAttributeValue() as $vidx => &$attributeValue) { $type = null; if (is_array($valueTypes)) { From 778968e387a30b5de0137c1da823cd9aaa340d75 Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Sun, 15 Dec 2019 17:04:25 +0100 Subject: [PATCH 18/31] Pass NameID's directly as values for AttributeValue --- src/SAML2/Assertion.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/SAML2/Assertion.php b/src/SAML2/Assertion.php index 264512eb1..6b76f9a52 100644 --- a/src/SAML2/Assertion.php +++ b/src/SAML2/Assertion.php @@ -545,9 +545,8 @@ private function parseAttributeValue(DOMNode $attribute, string $attributeName): $eptiNameId = Utils::xpQuery($eptiAttributeValue, './saml_assertion:NameID'); if (count($eptiNameId) === 1) { - $nameId = new NameID($eptiNameId[0]); $this->attributes[$attributeName]->addAttributeValue( - new AttributeValue($nameId->toXML()->textContent) + new AttributeValue(new NameID($eptiNameId[0])) ); } else { /* Fall back for legacy IdPs sending string value (e.g. SSP < 1.15) */ From e92f28d5a0f27c8169548d0e44b6bb95f393ff8c Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Sun, 15 Dec 2019 17:12:48 +0100 Subject: [PATCH 19/31] Explicitly cast value to string --- src/SAML2/XML/saml/AttributeValue.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SAML2/XML/saml/AttributeValue.php b/src/SAML2/XML/saml/AttributeValue.php index 183c0d9e3..b5c3c8582 100644 --- a/src/SAML2/XML/saml/AttributeValue.php +++ b/src/SAML2/XML/saml/AttributeValue.php @@ -45,7 +45,7 @@ public function __construct($value) $this->element->setAttributeNS(Constants::NS_XSI, 'xsi:nil', 'true'); } else { $this->element->setAttributeNS(Constants::NS_XSI, 'xsi:type', 'xs:'.gettype($value)); - $this->element->appendChild($doc->createTextNode($value)); + $this->element->appendChild($doc->createTextNode(strval($value))); } /* Make sure that the xs-namespace is available in the AttributeValue (for xs:string). */ From 15c89157ea4b5077b765384d70f9ae2218e9683f Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Sun, 15 Dec 2019 17:15:45 +0100 Subject: [PATCH 20/31] No need to override types? --- src/SAML2/Assertion.php | 32 -------------------------------- 1 file changed, 32 deletions(-) diff --git a/src/SAML2/Assertion.php b/src/SAML2/Assertion.php index 6b76f9a52..a8ca8ba9c 100644 --- a/src/SAML2/Assertion.php +++ b/src/SAML2/Assertion.php @@ -1609,11 +1609,6 @@ private function addAttributeStatement(DOMElement $root): void $root->appendChild($attributeStatement); foreach ($this->attributes as $name => $attributeObj) { - // possibly override the xsi type for the current attribute - if (array_key_exists($attributeObj->getName(), $this->attributes)) { - $this->overrideAttributeType($attributeObj); - } - $attributeObj->toXML($attributeStatement); } } @@ -1637,10 +1632,6 @@ private function addEncryptedAttributeStatement(DOMElement $root): void $attributeStatement = $document->createElementNS(Constants::NS_SAML, 'saml:AttributeStatement'); foreach ($this->attributes as $name => $attributeObj) { - // possibly override the xsi type for the current attribute - if (array_key_exists($attributeObj->getName(), $this->attributes)) { - $this->overrideAttributeType($attributeObj); - } $attributeElement = $attributeObj->toXML($attributeStatement); // Once the attribute nodes are built, they are encrypted @@ -1670,27 +1661,4 @@ private function addEncryptedAttributeStatement(DOMElement $root): void } $root->appendChild($attributeStatement); } - - /** - * Apply an attribute type override to the attribute's values. The xsi type of each attribute is inferred based on its datatype - * via the php gettype function inside XML\saml\AttributeValue, but not all xsi types can be inferred in this manner. - * Simple xsi types like decimal, double, date, dateTime, etc as well as custom types can only be set via overrides - * - * @param XML\saml\Attribute $attributeObj The actual attribute object to apply the override to. - * @return void - */ - private function overrideAttributeType(Attribute &$attributeObj): void - { - foreach ($attributeObj->getAttributeValue() as $vidx => &$attributeValue) { - $type = null; - if (is_array($valueTypes)) { - $type = $valueTypes[$vidx]; - } else { - $type = $valueTypes; - } - if ($type !== null) { - $attributeValue->getElement()->setAttributeNS(Constants::NS_XSI, 'xsi:type', $type); - } - } - } } From a0f14dd642067a281ca843aed8b9a7bc0a33bc56 Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Thu, 19 Dec 2019 18:00:52 +0100 Subject: [PATCH 21/31] Address comments --- src/SAML2/Assertion.php | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/SAML2/Assertion.php b/src/SAML2/Assertion.php index a8ca8ba9c..d87ce230c 100644 --- a/src/SAML2/Assertion.php +++ b/src/SAML2/Assertion.php @@ -166,10 +166,9 @@ class Assertion extends SignedElement private $AuthenticatingAuthority = []; /** - * The attributes, as an associative array. + * The attributes, as an array of Attribute-objects. * - * @var array array of attributes indexed by attribute name with each value an SAML2\XML\saml\Attribute - * or an SAML2\XML\saml\NameID object if the attribute is an eduPersonTargetedID + * @var \SAML2\XML\saml\Attribute[] array of attributes with each value an \SAML2\XML\saml\Attribute */ private $attributes = []; @@ -830,7 +829,7 @@ public function decryptNameId(XMLSecurityKey $key, array $blacklist = []): void */ public function hasEncryptedAttributes(): bool { - return $this->encryptedAttributes !== null && $this->encryptedAttributes !== []; + return !empty($this->encryptedAttributes); } @@ -1216,7 +1215,7 @@ public function setAuthenticatingAuthority(array $authenticatingAuthority): void /** * Retrieve all attributes. * - * @return array All attributes, as an associative array. + * @return \SAML2\XML\saml\Attribute[] All attributes, as an associative array. */ public function getAttributes(): array { @@ -1227,7 +1226,7 @@ public function getAttributes(): array /** * Replace all attributes. * - * @param array $attributes All new attributes, as an associative array. + * @param \SAML2\XML\saml\Attribute[] $attributes All new attributes, as an associative array. * @return void */ public function setAttributes(array $attributes): void From d43405f3fc165955a30e61bb48d26f155a5ecd96 Mon Sep 17 00:00:00 2001 From: Bobby Lawrence Date: Thu, 19 Dec 2019 18:07:11 -0500 Subject: [PATCH 22/31] allow assertion to use attribute objects and pass all tests with some degree of backwards compatibility all while maintaining the ability to keep the attribute FriendlyName and NameFormat --- src/SAML2/Assertion.php | 174 +++++++++++--------------- src/SAML2/XML/saml/Attribute.php | 63 ++++++++++ src/SAML2/XML/saml/AttributeValue.php | 72 +++++++++++ tests/SAML2/AssertionTest.php | 117 +++++++++-------- 4 files changed, 264 insertions(+), 162 deletions(-) diff --git a/src/SAML2/Assertion.php b/src/SAML2/Assertion.php index a8ca8ba9c..4ef61f87f 100644 --- a/src/SAML2/Assertion.php +++ b/src/SAML2/Assertion.php @@ -497,94 +497,45 @@ private function parseAttributes(DOMElement $xml): void /** @var \DOMElement[] $attributes */ $attributes = Utils::xpQuery($xml, './saml_assertion:AttributeStatement/saml_assertion:Attribute'); foreach ($attributes as $attribute) { - if (!$attribute->hasAttribute('Name')) { - throw new \Exception('Missing name on element.'); - } - $name = $attribute->getAttribute('Name'); - - if ($attribute->hasAttribute('NameFormat')) { - $nameFormat = $attribute->getAttribute('NameFormat'); - } else { - $nameFormat = Constants::NAMEFORMAT_UNSPECIFIED; - } - + $this->parseAttribute($attribute, $firstAttribute); if ($firstAttribute) { - $this->nameFormat = $nameFormat; $firstAttribute = false; - } else { - if ($this->nameFormat !== $nameFormat) { - $this->nameFormat = Constants::NAMEFORMAT_UNSPECIFIED; - } } - - if (!array_key_exists($name, $this->attributes)) { - $attr = new Attribute(); - $attr->setName($name); - - $this->attributes[$name] = $attr; - } - - $this->parseAttributeValue($attribute, $name); } } - /** - * @param \DOMNode $attribute - * @param string $attributeName + * Parse attribute statements in assertion. + * + * @param \DOMElement $xml The XML element with the Attribute. + * @param boolean $firstAttribute Whether this is the first attribute to parse. + * @throws \Exception * @return void */ - private function parseAttributeValue(DOMNode $attribute, string $attributeName): void - { - /** @var \DOMElement[] $values */ - $values = Utils::xpQuery($attribute, './saml_assertion:AttributeValue'); - - if ($attributeName === Constants::EPTI_URN_MACE || $attributeName === Constants::EPTI_URN_OID) { - foreach ($values as $index => $eptiAttributeValue) { - /** @var \DOMElement[] $eptiNameId */ - $eptiNameId = Utils::xpQuery($eptiAttributeValue, './saml_assertion:NameID'); - - if (count($eptiNameId) === 1) { - $this->attributes[$attributeName]->addAttributeValue( - new AttributeValue(new NameID($eptiNameId[0])) - ); - } else { - /* Fall back for legacy IdPs sending string value (e.g. SSP < 1.15) */ - Utils::getContainer()->getLogger()->warning( - sprintf("Attribute %s (EPTI) value %d is not an XML NameId", $attributeName, $index) - ); - $this->attributes[$attributeName]->addAttributeValue( - new AttributeValue($eptiAttributeValue->textContent) - ); - } - } - - return; + private function parseAttribute(DOMElement $xml, bool $firstAttribute = false): void + { + $attribute = new Attribute($xml); + + if ($attribute->getNameFormat() !== null) { + $nameFormat = $attribute->getNameFormat(); + } else { + $nameFormat = Constants::NAMEFORMAT_UNSPECIFIED; } - foreach ($values as $value) { - $hasNonTextChildElements = false; - foreach ($value->childNodes as $childNode) { - /** @var \DOMNode $childNode */ - if ($childNode->nodeType !== XML_TEXT_NODE) { - $hasNonTextChildElements = true; - break; - } + if ($firstAttribute) { + $this->nameFormat = $nameFormat; + } else { + if ($this->nameFormat !== $nameFormat) { + $this->nameFormat = Constants::NAMEFORMAT_UNSPECIFIED; } + } - if ($hasNonTextChildElements) { - $this->attributes[$attributeName]->addAttributeValue( - new AttributeValue($value->childNodes) - ); - continue; - } - - $this->attributes[$attributeName]->addAttributeValue( - new AttributeValue(trim($value->textContent)) - ); + $name = $attribute->getName(); + if (!array_key_exists($name, $this->attributes)) { + $this->attributes[$name] = $attribute; } } - + /** * Parse encrypted attribute statements in assertion. @@ -856,35 +807,10 @@ public function decryptAttributes(XMLSecurityKey $key, array $blacklist = []): v $key, $blacklist ); - - if (!$attribute->hasAttribute('Name')) { - throw new \Exception('Missing name on element.'); - } - $name = $attribute->getAttribute('Name'); - - if ($attribute->hasAttribute('NameFormat')) { - $nameFormat = $attribute->getAttribute('NameFormat'); - } else { - $nameFormat = Constants::NAMEFORMAT_UNSPECIFIED; - } - + $this->parseAttribute($attribute, $firstAttribute); if ($firstAttribute) { - $this->nameFormat = $nameFormat; $firstAttribute = false; - } else { - if ($this->nameFormat !== $nameFormat) { - $this->nameFormat = Constants::NAMEFORMAT_UNSPECIFIED; - } - } - - if (!array_key_exists($name, $this->attributes)) { - $attr = new Attribute(); - $attr->setName($name); - - $this->attributes[$name] = $attr; } - - $this->parseAttributeValue($attribute, $name); } } @@ -1216,14 +1142,60 @@ public function setAuthenticatingAuthority(array $authenticatingAuthority): void /** * Retrieve all attributes. * - * @return array All attributes, as an associative array. + * @return array of Attributes */ public function getAttributes(): array { return $this->attributes; } - + + /** + * Retrieve all attribute values as an associative array + * + * @return array of attribute values + */ + public function getAttributeValues(): array + { + $attributes = []; + + foreach ($this->attributes as $attributeObj){ + $attributeName = $attributeObj->getName(); + $attributes[$attributeName] = []; + + foreach ($attributeObj->getAttributeValue() as $attributeValue){ + $value = $attributeValue->getValue(); + + // need to determine if we should return a NameID + if ($attributeName === Constants::EPTI_URN_MACE || $attributeName === Constants::EPTI_URN_OID) { + $nameId = null; + if ($value instanceof DOMNodeList){ + foreach ($value as $node) { + /** @var \DOMNode $node */ + if ($node->nodeType !== XML_TEXT_NODE && $node->localName === 'NameID' && $node->namespaceURI === Constants::NS_SAML) { + $nameId = new NameID($node); + } + } + } + else { + /* Fall back for legacy IdPs sending string value (e.g. SSP < 1.15) */ + Utils::getContainer()->getLogger()->warning( + sprintf("Attribute %s (EPTI) value %d is not an XML NameId", $attributeName, $value) + ); + $nameId = new NameID(); + $nameId->setValue($value); + } + if ($nameId !== null){ + $value = $nameId; + } + } + $attributes[$attributeName][] = $value; + } + } + return $attributes; + } + + /** * Replace all attributes. * diff --git a/src/SAML2/XML/saml/Attribute.php b/src/SAML2/XML/saml/Attribute.php index a2dce5903..bf470c316 100644 --- a/src/SAML2/XML/saml/Attribute.php +++ b/src/SAML2/XML/saml/Attribute.php @@ -7,6 +7,7 @@ use DOMElement; use SAML2\Constants; use SAML2\Utils; +use SAML2\DOMDocumentFactory; /** * Class representing SAML 2 Attribute. @@ -226,4 +227,66 @@ public function toXML(DOMElement $parent): DOMElement { return $this->toXMLInternal($parent, Constants::NS_SAML, 'saml:Attribute'); } + + /** + * Convert an array of attributes with name = value(s) to an array of Attribute objects. + * + * @param \array $attributes The array of attributes we need to convert to objects + * @return \array Atrribute + */ + public static function fromArray($attributes = array(), $attributesValueTypes = array()): array + { + $attr_array = array(); + foreach ($attributes as $name => $value) { + if ($value instanceof Attribute || $value instanceof NameID) { + $attr_array[$name] = $value; + } + else { + $attr_array[$name] = null; + if (is_array($value)) { + + $document = DOMDocumentFactory::create(); + $attrDomElement = $document->createElementNS(Constants::NS_SAML, 'saml:Attribute'); + $document->appendChild($attrDomElement); + $attrDomElement->setAttribute('Name', $name); + + $attributeObj = new Attribute($attrDomElement); + + foreach ($value as $vidx => $attributeValue) { + $attributeValueObj = new AttributeValue($attributeValue); + $attributeObj->AttributeValue[] = $attributeValueObj; + } + $attr_array[$name] = $attributeObj; + } + } + } + + // set types + foreach ($attributesValueTypes as $name => $valueTypes){ + foreach ($attr_array as $attributeObj){ + if ($attributeObj->getName() === $name){ + if ($valueTypes !== null) { + if (is_array($valueTypes) && count($valueTypes) != count($attributeObj->getAttributeValue())) { + throw new \Exception( + 'Array of value types and array of values have different size for attribute '. + var_export($attributeObj->getName(), true) + ); + } + foreach ($attributeObj->getAttributeValue() as $vidx => &$attributeValue) { + $type = null; + if (is_array($valueTypes)) { + $type = $valueTypes[$vidx]; + } else { + $type = $valueTypes; + } + if ($type !== null) { + $attributeValue->getElement()->setAttributeNS(Constants::NS_XSI, 'xsi:type', $type); + } + } + } + } + } + } + return $attr_array; + } } diff --git a/src/SAML2/XML/saml/AttributeValue.php b/src/SAML2/XML/saml/AttributeValue.php index b5c3c8582..05ea1325c 100644 --- a/src/SAML2/XML/saml/AttributeValue.php +++ b/src/SAML2/XML/saml/AttributeValue.php @@ -118,7 +118,79 @@ public function getString(): string return $this->element->textContent; } + /** + * Returns the xsd type of the attribute value or null if its not defined. + * + * @return string + */ + public function getType(): ?string + { + $type = null; + if ($this->element->hasAttributeNS(Constants::NS_XSI, 'type')){ + $type = $this->element->getAttributeNS(Constants::NS_XSI, 'type'); + } + return $type; + } + /** + * Returns the actual value of the attribute value object's element. + * Since this function can return multiple types, we cannot declare the return type without running on php 8 + * + * @return string|boolean|int|float|DOMNodeList + */ + public function getValue() + { + $variable = null; + $xsi_type = $this->getType(); + if ($xsi_type !== null) + { + switch ($xsi_type) { + case 'xs:boolean': + $variable = $this->element->textContent === 'true'; + break; + case 'xs:int': + case 'xs:integer': + case 'xs:long': + case 'xs:negativeInteger': + case 'xs:nonNegativeInteger': + case 'xs:nonPositiveInteger': + case 'xs:positiveInteger': + case 'short': + case 'xs:unsignedLong': + case 'xs:unsignedInt': + case 'xs:unsignedShort': + case 'xs:unsignedByte': + $variable = intval($this->element->textContent); + break; + case 'xs:decimal': + case 'xs:double': + case 'xs:float': + $variable = floatval($this->element->textContent); + break; + default: + // what about date/time/dateTime, base64Binary/hexBinary or other xsd types? everything else is basically a string for now... + $variable = strval($this->element->textContent); + } + } + else { + $hasNonTextChildElements = false; + foreach ($this->element->childNodes as $childNode) { + /** @var \DOMNode $childNode */ + if ($childNode->nodeType !== XML_TEXT_NODE) { + $hasNonTextChildElements = true; + break; + } + } + if ($hasNonTextChildElements){ + $variable = $this->element->childNodes; + } + else { + $variable = strval($this->element->textContent); + } + } + return $variable; + } + /** * Convert this attribute value to a string. * diff --git a/tests/SAML2/AssertionTest.php b/tests/SAML2/AssertionTest.php index a27547afb..8e3f632a1 100644 --- a/tests/SAML2/AssertionTest.php +++ b/tests/SAML2/AssertionTest.php @@ -142,20 +142,12 @@ public function testMarshallingUnmarshallingChristmas(): void $assertion->setAuthenticatingAuthority(["idp1", "idp2"]); - $attr1 = new Attribute(); - $attr1->setName("name1"); - $attr1->addAttributeValue(new AttributeValue("value1")); - $attr1->addAttributeValue(new AttributeValue("value2")); - - $attr2 = new Attribute(); - $attr2->setName("name2"); - $attr2->addAttributeValue(new AttributeValue(2)); - - $attr3 = new Attribute(); - $attr3->setName("name3"); - $attr3->addAttributeValue(new AttributeValue(null)); + $assertion->setAttributes(Attribute::fromArray([ + "name1" => ["value1", "value2"], + "name2" => [2], + "name3" => [null] + ])); - $assertion->setAttributes([$attr1, $attr2, $attr3]); $assertion->setAttributeNameFormat("urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified"); $assertionElement = $assertion->toXML()->ownerDocument->saveXML(); @@ -176,7 +168,8 @@ public function testMarshallingUnmarshallingChristmas(): void $this->assertCount(2, $authauth); $this->assertEquals("idp2", $authauth[1]); - $attributes = $assertionToVerify->getAttributes(); + $attributes = $assertionToVerify->getAttributeValues(); + $this->assertCount(3, $attributes); $this->assertCount(2, $attributes['name1']); $this->assertEquals("value1", $attributes['name1'][0]); @@ -201,35 +194,22 @@ public function testMarshallingUnmarshallingAttributeValTypes(): void $assertion->setIssuer($issuer); $assertion->setValidAudiences(['audience1', 'audience2']); - $assertion->setAuthnContextClassRef('someAuthnContext'); - $assertion->setAuthenticatingAuthority(["idp1", "idp2"]); - $attr1 = new Attribute(); - $attr1->setName("name1"); - $attr1->addAttributeValue(new AttributeValue("value1")); - $attr1->addAttributeValue(new AttributeValue(123)); - $attr1->addAttributeValue(new AttributeValue("2017-31-12")); - - $attr2 = new Attribute("name2"); - $attr2->setName("name2"); - $attr2->addAttributeValue(new AttributeValue(2)); - - $attr3 = new Attribute("name3"); - $attr3->setName("name3"); - $attr3->addAttributeValue(new AttributeValue(1234)); - $attr3->addAttributeValue(new AttributeValue("+2345")); - - $assertion->setAttributes([$attr1, $attr2, $attr3]); - $assertion->setAttributeNameFormat(\SAML2\Constants::NAMEFORMAT_UNSPECIFIED); - // set xs:type for first and third name1 values, and all name3 values. // second name1 value and all name2 values will use default behaviour - $assertion->setAttributesValueTypes([ + $attributesValueTypes = [ "name1" => ["xs:string", null, "xs:date"], "name3" => "xs:decimal" - ]); + ]; + + $assertion->setAttributes(Attribute::fromArray([ + "name1" => ["value1",123,"2017-31-12"], + "name2" => [2], + "name3" => [1234, "+2345"] + ], $attributesValueTypes)); + $assertion->setAttributeNameFormat(\SAML2\Constants::NAMEFORMAT_UNSPECIFIED); $assertionElement = $assertion->toXML()->ownerDocument->saveXML(); @@ -239,7 +219,16 @@ public function testMarshallingUnmarshallingAttributeValTypes(): void $this->assertCount(2, $authauth); $this->assertEquals("idp2", $authauth[1]); - $attributes = $assertionToVerify->getAttributes(); + $attributes = []; + $attributesValueTypes = []; + foreach ($assertionToVerify->getAttributes() as $attributeObj){ + $attributes[$attributeObj->getName()] = []; + $attributesValueTypes[$attributeObj->getName()] = []; + foreach ($attributeObj->getAttributeValue() as $attributeValue){ + $attributes[$attributeObj->getName()][] = $attributeValue->getValue(); + $attributesValueTypes[$attributeObj->getName()][] = $attributeValue->getType(); + } + } $this->assertCount(3, $attributes); $this->assertCount(3, $attributes['name1']); $this->assertEquals("value1", $attributes['name1'][0]); @@ -251,7 +240,8 @@ public function testMarshallingUnmarshallingAttributeValTypes(): void $this->assertEquals("+2345", $attributes['name3'][1]); $this->assertEquals(\SAML2\Constants::NAMEFORMAT_UNSPECIFIED, $assertionToVerify->getAttributeNameFormat()); - $attributesValueTypes = $assertionToVerify->getAttributesValueTypes(); + + $this->assertCount(3, $attributesValueTypes); $this->assertCount(3, $attributesValueTypes['name1']); $this->assertEquals("xs:string", $attributesValueTypes['name1'][0]); @@ -280,28 +270,27 @@ public function testMarshallingWrongAttributeValTypes(): void $assertion->setIssuer($issuer); $assertion->setValidAudiences(['audience1', 'audience2']); - $assertion->setAuthnContextClassRef('someAuthnContext'); - $assertion->setAuthenticatingAuthority(["idp1", "idp2"]); - - $assertion->setAttributes([ - "name1" => ["value1", "2017-31-12"], - "name2" => [2], - "name3" => [1234, "+2345"] - ]); $assertion->setAttributeNameFormat(\SAML2\Constants::NAMEFORMAT_UNSPECIFIED); - + // set wrong number elements in name1 - $assertion->setAttributesValueTypes([ + $attributeValueTypes = [ "name1" => ["xs:string"], "name3" => "xs:decimal" - ]); - + ]; + $this->expectException( \Exception::class, "Array of value types and array of values have different size for attribute 'name1'" ); + + $assertion->setAttributes(Attribute::fromArray([ + "name1" => ["value1", "2017-31-12"], + "name2" => [2], + "name3" => [1234, "+2345"] + ], $attributeValueTypes)); + $assertionElement = $assertion->toXML()->ownerDocument->saveXML(); } @@ -805,7 +794,7 @@ public function testEptiAttributeValuesAreParsedCorrectly(): void $assertion = new Assertion(DOMDocumentFactory::fromString($xml)->firstChild); - $attributes = $assertion->getAttributes(); + $attributes = $assertion->getAttributeValues(); $maceValue = $attributes['urn:mace:dir:attribute-def:eduPersonTargetedID'][0]; $oidValue = $attributes['urn:oid:1.3.6.1.4.1.5923.1.1.1.10'][0]; @@ -873,7 +862,9 @@ public function testEptiLegacyAttributeValuesCanBeString(): void XML; $assertion = new Assertion(DOMDocumentFactory::fromString($xml)->firstChild); - $attributes = $assertion->getAttributes(); + + $attributes = $assertion->getAttributeValues(); + $maceValue = $attributes['urn:mace:dir:attribute-def:eduPersonTargetedID'][0]; $oidValue = $attributes['urn:oid:1.3.6.1.4.1.5923.1.1.1.10'][0]; @@ -921,7 +912,7 @@ public function testEptiAttributeParsingSupportsMultipleValues(): void $assertion = new Assertion(DOMDocumentFactory::fromString($xml)->firstChild); - $attributes = $assertion->getAttributes(); + $attributes = $assertion->getAttributeValues(); $maceFirstValue = $attributes['urn:mace:dir:attribute-def:eduPersonTargetedID'][0]; $maceSecondValue = $attributes['urn:mace:dir:attribute-def:eduPersonTargetedID'][1]; @@ -968,7 +959,8 @@ public function testAttributeValuesWithComplexTypesAreParsedCorrectly(): void $assertion = new Assertion(DOMDocumentFactory::fromString($xml)->firstChild); - $attributes = $assertion->getAttributes(); + $attributes = $assertion->getAttributeValues(); + $this->assertInstanceOf( \DOMNodeList::class, $attributes['urn:some:custom:outer:element'][0] @@ -1003,7 +995,8 @@ public function testTypedAttributeValuesAreParsedCorrectly(): void $assertion = new Assertion(DOMDocumentFactory::fromString($xml)->firstChild); - $attributes = $assertion->getAttributes(); + $attributes = $assertion->getAttributeValues(); + $this->assertIsInt($attributes['urn:some:integer'][0]); $this->assertIsString($attributes['urn:some:string'][0]); $this->assertXmlStringEqualsXmlString($xml, $assertion->toXML()->ownerDocument->saveXML()); @@ -1053,7 +1046,8 @@ public function testEncryptedAttributeValuesWithComplexTypeValuesAreParsedCorrec $assertionToVerify->decryptAttributes(CertificatesMock::getPrivateKey()); - $attributes = $assertionToVerify->getAttributes(); + $attributes = $assertionToVerify->getAttributeValues(); + $this->assertInstanceOf( \DOMNodeList::class, $attributes['urn:some:custom:outer:element'][0] @@ -1098,7 +1092,8 @@ public function testTypedEncryptedAttributeValuesAreParsedCorrectly(): void $this->assertTrue($assertionToVerify->hasEncryptedAttributes()); $assertionToVerify->decryptAttributes(CertificatesMock::getPrivateKey()); - $attributes = $assertionToVerify->getAttributes(); + + $attributes = $assertionToVerify->getAttributeValues(); $this->assertIsInt($attributes['urn:some:integer'][0]); $this->assertIsString($attributes['urn:some:string'][0]); @@ -1995,10 +1990,10 @@ public function testMarshallingElementOrdering(): void // Create an assertion $assertion = new Assertion(); $assertion->setIssuer($issuer); - $assertion->setAttributes([ + $assertion->setAttributes(Attribute::fromArray([ "name1" => ["value1","value2"], "name2" => ["value3"], - ]); + ])); $assertion->setAttributeNameFormat("urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified"); $assertion->setSignatureKey(CertificatesMock::getPrivateKey()); @@ -2040,11 +2035,11 @@ public function testAttributeValueEmptyStringAndNull(): void $issuer->setValue('testIssuer'); $assertion->setIssuer($issuer); - $assertion->setAttributes([ + $assertion->setAttributes(Attribute::fromArray([ "name1" => ["value1", "value2"], "name2" => ["value3", ""], "name3" => ["value1", null, "value5"], - ]); + ])); $assertionElement = $assertion->toXML(); $assertionElements = Utils::xpQuery( From 9a028f1bc00f67c1686e07dfeebf68324c636d4a Mon Sep 17 00:00:00 2001 From: Bobby Lawrence Date: Thu, 19 Dec 2019 22:26:54 -0500 Subject: [PATCH 23/31] ensure that Assertion can take attributes which aren't simple arrays and convert them to Attribute objects if needed. also ensure that if a null attribute value is stored, it is retrieved as null instead of an empty string --- src/SAML2/Assertion.php | 17 +++++- src/SAML2/XML/saml/AttributeValue.php | 24 +++++--- tests/SAML2/AssertionTest.php | 86 ++++++++++++++++++++++++++- 3 files changed, 116 insertions(+), 11 deletions(-) diff --git a/src/SAML2/Assertion.php b/src/SAML2/Assertion.php index 4ef61f87f..9cc35da54 100644 --- a/src/SAML2/Assertion.php +++ b/src/SAML2/Assertion.php @@ -1205,7 +1205,22 @@ public function getAttributeValues(): array public function setAttributes(array $attributes): void { foreach ($attributes as $name => $value) { - $this->attributes[$name] = $value; + if ($value instanceof Attribute){ + $this->attributes[$name] = $value; + } + else { + $attributeObj = new Attribute(); + $attributeObj->setName($name); + if (is_array($value)) { + foreach ($value as $vidx => $attributeValue) { + $attributeObj->addAttributeValue(new AttributeValue($attributeValue)); + } + } + else { + $attributeObj->addAttributeValue(new AttributeValue($value)); + } + $this->attributes[$name] = $attributeObj; + } } } diff --git a/src/SAML2/XML/saml/AttributeValue.php b/src/SAML2/XML/saml/AttributeValue.php index 05ea1325c..edcd520a0 100644 --- a/src/SAML2/XML/saml/AttributeValue.php +++ b/src/SAML2/XML/saml/AttributeValue.php @@ -54,19 +54,19 @@ public function __construct($value) return; } - if ($value instanceof NameID) { - $this->element = $value->toXML(); - return; - } - - if ($value->namespaceURI === Constants::NS_SAML && $value->localName === 'AttributeValue') { + if (($value instanceof DOMElement) && $value->namespaceURI === Constants::NS_SAML && $value->localName === 'AttributeValue') { $this->element = Utils::copyElement($value); return; } $doc = DOMDocumentFactory::create(); $this->element = $doc->createElementNS(Constants::NS_SAML, 'saml:AttributeValue'); - Utils::copyElement($value, $this->element); + if ($value instanceof NameID) { + $value->toXML($this->element); + } + else { + Utils::copyElement($value, $this->element); + } } @@ -141,7 +141,15 @@ public function getType(): ?string public function getValue() { $variable = null; - $xsi_type = $this->getType(); + + if ($this->element->hasAttributeNS(Constants::NS_XSI, 'nil')){ + $is_nil = $this->element->getAttributeNS(Constants::NS_XSI, 'nil'); + if ($is_nil === "true"){ + return $variable; + } + } + + $xsi_type = $this->getType(); if ($xsi_type !== null) { switch ($xsi_type) { diff --git a/tests/SAML2/AssertionTest.php b/tests/SAML2/AssertionTest.php index 8e3f632a1..adb08f85f 100644 --- a/tests/SAML2/AssertionTest.php +++ b/tests/SAML2/AssertionTest.php @@ -174,12 +174,94 @@ public function testMarshallingUnmarshallingChristmas(): void $this->assertCount(2, $attributes['name1']); $this->assertEquals("value1", $attributes['name1'][0]); $this->assertEquals(2, $attributes['name2'][0]); - // NOTE: nil attribute is currently parsed as string.. - //$this->assertNull($attributes["name3"][0]); + $this->assertNull($attributes["name3"][0]); $this->assertEquals(\SAML2\Constants::NAMEFORMAT_UNSPECIFIED, $assertionToVerify->getAttributeNameFormat()); } + /** + * Test an assertion with attributes of different types + */ + public function testMixedAttributes(): void + { + // Create an Issuer + $issuer = new Issuer(); + $issuer->setValue('testIssuer'); + + // Create an assertion + $assertion = new Assertion(); + + $assertion->setIssuer($issuer); + $assertion->setValidAudiences(['audience1', 'audience2']); + + $this->assertNull($assertion->getAuthnContextClassRef()); + + $assertion->setAuthnContextClassRef('someAuthnContext'); + $assertion->setAuthnContextDeclRef('/relative/path/to/document.xml'); + + $assertion->setID("_123abc"); + + $assertion->setIssueInstant(1234567890); + $assertion->setAuthnInstant(1234567890 - 1); + $assertion->setNotBefore(1234567890 - 10); + $assertion->setNotOnOrAfter(1234567890 + 100); + $assertion->setSessionNotOnOrAfter(1234568890 + 200); + + $assertion->setSessionIndex("idx1"); + + $assertion->setAuthenticatingAuthority(["idp1", "idp2"]); + + $name4 = new Attribute(); + $name4->setName("name4"); + $name4->addAttributeValue(new AttributeValue("testme")); + + $epti5 = new Attribute(); + $epti5->setName("urn:oid:1.3.6.1.4.1.5923.1.1.1.10"); + $epti5->addAttributeValue(new AttributeValue("myid")); + + $epti6 = new Attribute(); + $epti6->setName("urn:mace:dir:attribute-def:eduPersonTargetedID"); + $epti6NameId = new NameID(); + $epti6NameId->setValue("myid2"); + $epti6->addAttributeValue(new AttributeValue($epti6NameId)); + + $assertion->setAttributes([ + "name1" => ["value1", "value2"], + "name2" => [2], + "name3" => [null], + $name4->getName() => $name4, + $epti5->getName() => $epti5, + $epti6->getName() => $epti6 + ]); + + $assertion->setAttributeNameFormat("urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified"); + + $assertionElement = $assertion->toXML()->ownerDocument->saveXML(); + + $assertionToVerify = new Assertion(DOMDocumentFactory::fromString($assertionElement)->firstChild); + + + $attributes = $assertionToVerify->getAttributeValues(); + + $this->assertCount(6, $attributes); + $this->assertCount(2, $attributes['name1']); + $this->assertEquals("value1", $attributes['name1'][0]); + $this->assertEquals(2, $attributes['name2'][0]); + $this->assertNull($attributes["name3"][0]); + + $this->assertEquals("testme", $attributes['name4'][0]); + + $maceValue = $attributes['urn:mace:dir:attribute-def:eduPersonTargetedID'][0]; + $oidValue = $attributes['urn:oid:1.3.6.1.4.1.5923.1.1.1.10'][0]; + + $this->assertInstanceOf(NameID::class, $maceValue); + $this->assertInstanceOf(NameID::class, $oidValue); + + $this->assertEquals('myid2', $maceValue->getValue()); + $this->assertEquals('myid', $oidValue->getValue()); + } + + /** * Test an assertion attribute value types options */ From a809a7ef9e6265c71433df51cc29c1b7b2b7587c Mon Sep 17 00:00:00 2001 From: Bobby Lawrence Date: Thu, 19 Dec 2019 22:29:24 -0500 Subject: [PATCH 24/31] changed Assertion comment to resolve branch conflict in pull request --- src/SAML2/Assertion.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SAML2/Assertion.php b/src/SAML2/Assertion.php index 9cc35da54..eafa01384 100644 --- a/src/SAML2/Assertion.php +++ b/src/SAML2/Assertion.php @@ -1142,7 +1142,7 @@ public function setAuthenticatingAuthority(array $authenticatingAuthority): void /** * Retrieve all attributes. * - * @return array of Attributes + * @return \SAML2\XML\saml\Attribute[] All attributes, as an associative array. */ public function getAttributes(): array { From d82a87931b3abe8a50b5f9afd0a2c13a08ff963c Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Fri, 20 Dec 2019 11:12:14 +0100 Subject: [PATCH 25/31] Fix InvalidReturnStatement --- src/SAML2/XML/saml/AttributeValue.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SAML2/XML/saml/AttributeValue.php b/src/SAML2/XML/saml/AttributeValue.php index 487e49794..4ca1604ba 100644 --- a/src/SAML2/XML/saml/AttributeValue.php +++ b/src/SAML2/XML/saml/AttributeValue.php @@ -140,7 +140,7 @@ public function getType(): ?string * Returns the actual value of the attribute value object's element. * Since this function can return multiple types, we cannot declare the return type without running on php 8 * - * @return string|boolean|int|float|DOMNodeList + * @return string|boolean|int|float|\DOMNodeList */ public function getValue() { From 38ffe3f838a5fe826ed62f0c825beefc16094eeb Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Fri, 20 Dec 2019 11:20:20 +0100 Subject: [PATCH 26/31] Fix InvalidNullableReturnType|NullableReturnStatement --- src/SAML2/XML/saml/AttributeValue.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/SAML2/XML/saml/AttributeValue.php b/src/SAML2/XML/saml/AttributeValue.php index 4ca1604ba..3df304f67 100644 --- a/src/SAML2/XML/saml/AttributeValue.php +++ b/src/SAML2/XML/saml/AttributeValue.php @@ -125,7 +125,7 @@ public function getString(): string /** * Returns the xsd type of the attribute value or null if its not defined. * - * @return string + * @return string|null */ public function getType(): ?string { @@ -140,7 +140,7 @@ public function getType(): ?string * Returns the actual value of the attribute value object's element. * Since this function can return multiple types, we cannot declare the return type without running on php 8 * - * @return string|boolean|int|float|\DOMNodeList + * @return string|boolean|int|float|\DOMNodeList|null */ public function getValue() { @@ -203,6 +203,7 @@ public function getValue() return $variable; } + /** * Convert this attribute value to a string. * From 5839dfd386867b37a5be8deafd2220c8c07667af Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Fri, 20 Dec 2019 11:25:32 +0100 Subject: [PATCH 27/31] Fix MissingParamType --- src/SAML2/XML/saml/Attribute.php | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/SAML2/XML/saml/Attribute.php b/src/SAML2/XML/saml/Attribute.php index c87574e98..e0a672e5b 100644 --- a/src/SAML2/XML/saml/Attribute.php +++ b/src/SAML2/XML/saml/Attribute.php @@ -233,14 +233,15 @@ public function toXML(DOMElement $parent): DOMElement { return $this->toXMLInternal($parent, Constants::NS_SAML, 'saml:Attribute'); } - + + /** * Convert an array of attributes with name = value(s) to an array of Attribute objects. * - * @param \array $attributes The array of attributes we need to convert to objects - * @return \array Atrribute + * @param array $attributes The array of attributes we need to convert to objects + * @return array Atrribute */ - public static function fromArray($attributes = array(), $attributesValueTypes = array()): array + public static function fromArray(array $attributes = [], array $attributesValueTypes = []): array { $attr_array = array(); foreach ($attributes as $name => $value) { @@ -268,13 +269,13 @@ public static function fromArray($attributes = array(), $attributesValueTypes = } // set types - foreach ($attributesValueTypes as $name => $valueTypes){ + foreach ($attributesValueTypes as $name => $valueTypes) { foreach ($attr_array as $attributeObj){ if ($attributeObj->getName() === $name){ if ($valueTypes !== null) { if (is_array($valueTypes) && count($valueTypes) != count($attributeObj->getAttributeValue())) { throw new \Exception( - 'Array of value types and array of values have different size for attribute '. + 'Array of value types and array of values have different size for attribute ' . var_export($attributeObj->getName(), true) ); } From ad481893b674a09b40b81a76b5e1725d223f4714 Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Fri, 20 Dec 2019 11:28:14 +0100 Subject: [PATCH 28/31] PSR-12 --- src/SAML2/Assertion.php | 37 ++++++++++++------------- src/SAML2/XML/saml/Attribute.php | 10 +++---- src/SAML2/XML/saml/AttributeValue.php | 39 ++++++++++++++------------- 3 files changed, 44 insertions(+), 42 deletions(-) diff --git a/src/SAML2/Assertion.php b/src/SAML2/Assertion.php index 1bec375fb..eb8f32151 100644 --- a/src/SAML2/Assertion.php +++ b/src/SAML2/Assertion.php @@ -168,7 +168,7 @@ class Assertion extends SignedElement /** * The attributes, as an array of Attribute-objects. * - * @var \SAML2\XML\saml\Attribute[] array of attributes with each value an \SAML2\XML\saml\Attribute + * @var \SAML2\XML\saml\Attribute[] array of attributes with each value an \SAML2\XML\saml\Attribute */ private $attributes = []; @@ -1143,7 +1143,7 @@ public function setAuthenticatingAuthority(array $authenticatingAuthority): void /** * Retrieve all attributes. - * + * * @return \SAML2\XML\saml\Attribute[] All attributes, as an associative array. */ public function getAttributes(): array @@ -1161,36 +1161,39 @@ public function getAttributeValues(): array { $attributes = []; - foreach ($this->attributes as $attributeObj){ + foreach ($this->attributes as $attributeObj) { $attributeName = $attributeObj->getName(); $attributes[$attributeName] = []; - foreach ($attributeObj->getAttributeValue() as $attributeValue){ + foreach ($attributeObj->getAttributeValue() as $attributeValue) { $value = $attributeValue->getValue(); // need to determine if we should return a NameID if ($attributeName === Constants::EPTI_URN_MACE || $attributeName === Constants::EPTI_URN_OID) { $nameId = null; - if ($value instanceof DOMNodeList){ + if ($value instanceof DOMNodeList) { foreach ($value as $node) { /** @var \DOMNode $node */ - if ($node->nodeType !== XML_TEXT_NODE && $node->localName === 'NameID' && $node->namespaceURI === Constants::NS_SAML) { + if ( + $node->nodeType !== XML_TEXT_NODE + && $node->localName === 'NameID' + && $node->namespaceURI === Constants::NS_SAML + ) { $nameId = new NameID($node); } } - } - else { + } else { /* Fall back for legacy IdPs sending string value (e.g. SSP < 1.15) */ Utils::getContainer()->getLogger()->warning( sprintf("Attribute %s (EPTI) value %d is not an XML NameId", $attributeName, $value) ); $nameId = new NameID(); - $nameId->setValue($value); - } - if ($nameId !== null){ + $nameId->setValue($value); + } + if ($nameId !== null) { $value = $nameId; } - } + } $attributes[$attributeName][] = $value; } } @@ -1207,18 +1210,16 @@ public function getAttributeValues(): array public function setAttributes(array $attributes): void { foreach ($attributes as $name => $value) { - if ($value instanceof Attribute){ + if ($value instanceof Attribute) { $this->attributes[$name] = $value; - } - else { + } else { $attributeObj = new Attribute(); $attributeObj->setName($name); if (is_array($value)) { foreach ($value as $vidx => $attributeValue) { $attributeObj->addAttributeValue(new AttributeValue($attributeValue)); - } - } - else { + } + } else { $attributeObj->addAttributeValue(new AttributeValue($value)); } $this->attributes[$name] = $attributeObj; diff --git a/src/SAML2/XML/saml/Attribute.php b/src/SAML2/XML/saml/Attribute.php index e0a672e5b..5fd278958 100644 --- a/src/SAML2/XML/saml/Attribute.php +++ b/src/SAML2/XML/saml/Attribute.php @@ -243,15 +243,13 @@ public function toXML(DOMElement $parent): DOMElement */ public static function fromArray(array $attributes = [], array $attributesValueTypes = []): array { - $attr_array = array(); + $attr_array = []; foreach ($attributes as $name => $value) { if ($value instanceof Attribute || $value instanceof NameID) { $attr_array[$name] = $value; - } - else { + } else { $attr_array[$name] = null; if (is_array($value)) { - $document = DOMDocumentFactory::create(); $attrDomElement = $document->createElementNS(Constants::NS_SAML, 'saml:Attribute'); $document->appendChild($attrDomElement); @@ -270,8 +268,8 @@ public static function fromArray(array $attributes = [], array $attributesValueT // set types foreach ($attributesValueTypes as $name => $valueTypes) { - foreach ($attr_array as $attributeObj){ - if ($attributeObj->getName() === $name){ + foreach ($attr_array as $attributeObj) { + if ($attributeObj->getName() === $name) { if ($valueTypes !== null) { if (is_array($valueTypes) && count($valueTypes) != count($attributeObj->getAttributeValue())) { throw new \Exception( diff --git a/src/SAML2/XML/saml/AttributeValue.php b/src/SAML2/XML/saml/AttributeValue.php index 3df304f67..bc9abb6c9 100644 --- a/src/SAML2/XML/saml/AttributeValue.php +++ b/src/SAML2/XML/saml/AttributeValue.php @@ -46,7 +46,7 @@ public function __construct($value) if (is_null($value)) { $this->element->setAttributeNS(Constants::NS_XSI, 'xsi:nil', 'true'); } else { - $this->element->setAttributeNS(Constants::NS_XSI, 'xsi:type', 'xs:'.gettype($value)); + $this->element->setAttributeNS(Constants::NS_XSI, 'xsi:type', 'xs:' . gettype($value)); $this->element->appendChild($doc->createTextNode(strval($value))); } @@ -56,7 +56,11 @@ public function __construct($value) return; } - if (($value instanceof DOMElement) && $value->namespaceURI === Constants::NS_SAML && $value->localName === 'AttributeValue') { + if ( + ($value instanceof DOMElement) + && $value->namespaceURI === Constants::NS_SAML + && $value->localName === 'AttributeValue' + ) { $this->element = Utils::copyElement($value); return; } @@ -65,8 +69,7 @@ public function __construct($value) $this->element = $doc->createElementNS(Constants::NS_SAML, 'saml:AttributeValue'); if ($value instanceof NameID) { $value->toXML($this->element); - } - else { + } else { Utils::copyElement($value, $this->element); } } @@ -130,7 +133,7 @@ public function getString(): string public function getType(): ?string { $type = null; - if ($this->element->hasAttributeNS(Constants::NS_XSI, 'type')){ + if ($this->element->hasAttributeNS(Constants::NS_XSI, 'type')) { $type = $this->element->getAttributeNS(Constants::NS_XSI, 'type'); } return $type; @@ -138,7 +141,7 @@ public function getType(): ?string /** * Returns the actual value of the attribute value object's element. - * Since this function can return multiple types, we cannot declare the return type without running on php 8 + * Since this function can return multiple types, we cannot declare the return type without running on php 8 * * @return string|boolean|int|float|\DOMNodeList|null */ @@ -146,16 +149,15 @@ public function getValue() { $variable = null; - if ($this->element->hasAttributeNS(Constants::NS_XSI, 'nil')){ + if ($this->element->hasAttributeNS(Constants::NS_XSI, 'nil')) { $is_nil = $this->element->getAttributeNS(Constants::NS_XSI, 'nil'); - if ($is_nil === "true"){ + if ($is_nil === "true") { return $variable; } } - - $xsi_type = $this->getType(); - if ($xsi_type !== null) - { + + $xsi_type = $this->getType(); + if ($xsi_type !== null) { switch ($xsi_type) { case 'xs:boolean': $variable = $this->element->textContent === 'true'; @@ -180,11 +182,13 @@ public function getValue() $variable = floatval($this->element->textContent); break; default: - // what about date/time/dateTime, base64Binary/hexBinary or other xsd types? everything else is basically a string for now... + /** + * what about date/time/dateTime, base64Binary/hexBinary or other xsd types? + * everything else is basically a string for now... + */ $variable = strval($this->element->textContent); } - } - else { + } else { $hasNonTextChildElements = false; foreach ($this->element->childNodes as $childNode) { /** @var \DOMNode $childNode */ @@ -193,10 +197,9 @@ public function getValue() break; } } - if ($hasNonTextChildElements){ + if ($hasNonTextChildElements) { $variable = $this->element->childNodes; - } - else { + } else { $variable = strval($this->element->textContent); } } From b31d47b340222ce2b2f966326843ac89f6eec066 Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Fri, 20 Dec 2019 19:01:28 +0100 Subject: [PATCH 29/31] Remove BC --- src/SAML2/Assertion.php | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/SAML2/Assertion.php b/src/SAML2/Assertion.php index eb8f32151..b8d105f3b 100644 --- a/src/SAML2/Assertion.php +++ b/src/SAML2/Assertion.php @@ -1209,24 +1209,14 @@ public function getAttributeValues(): array */ public function setAttributes(array $attributes): void { + Assert::allIsInstanceOf($attributes, Attribute::class); + foreach ($attributes as $name => $value) { - if ($value instanceof Attribute) { - $this->attributes[$name] = $value; - } else { - $attributeObj = new Attribute(); - $attributeObj->setName($name); - if (is_array($value)) { - foreach ($value as $vidx => $attributeValue) { - $attributeObj->addAttributeValue(new AttributeValue($attributeValue)); - } - } else { - $attributeObj->addAttributeValue(new AttributeValue($value)); - } - $this->attributes[$name] = $attributeObj; - } + $this->attributes[$name] = $value; } } + /** * @return array|null */ From c4caed61d8bbe4edf566d94c44ec8a5074888bb1 Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Fri, 20 Dec 2019 19:12:48 +0100 Subject: [PATCH 30/31] Remove BC --- tests/SAML2/AssertionTest.php | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/tests/SAML2/AssertionTest.php b/tests/SAML2/AssertionTest.php index adb08f85f..666693300 100644 --- a/tests/SAML2/AssertionTest.php +++ b/tests/SAML2/AssertionTest.php @@ -226,9 +226,6 @@ public function testMixedAttributes(): void $epti6->addAttributeValue(new AttributeValue($epti6NameId)); $assertion->setAttributes([ - "name1" => ["value1", "value2"], - "name2" => [2], - "name3" => [null], $name4->getName() => $name4, $epti5->getName() => $epti5, $epti6->getName() => $epti6 @@ -243,11 +240,7 @@ public function testMixedAttributes(): void $attributes = $assertionToVerify->getAttributeValues(); - $this->assertCount(6, $attributes); - $this->assertCount(2, $attributes['name1']); - $this->assertEquals("value1", $attributes['name1'][0]); - $this->assertEquals(2, $attributes['name2'][0]); - $this->assertNull($attributes["name3"][0]); + $this->assertCount(3, $attributes); $this->assertEquals("testme", $attributes['name4'][0]); From 464f0b1b11fdf214f651cf3eb82f9d8648c4d0a8 Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Thu, 26 Dec 2019 12:39:45 +0100 Subject: [PATCH 31/31] Don't make any assumptions on NameFormat; it's perfectly legal to omit it or to have mixed formats --- src/SAML2/Assertion.php | 68 ++---------------- src/SAML2/AttributeQuery.php | 49 ------------- tests/SAML2/AssertionTest.php | 44 ------------ tests/SAML2/AttributeQueryTest.php | 106 +---------------------------- 4 files changed, 8 insertions(+), 259 deletions(-) diff --git a/src/SAML2/Assertion.php b/src/SAML2/Assertion.php index b8d105f3b..d78035d08 100644 --- a/src/SAML2/Assertion.php +++ b/src/SAML2/Assertion.php @@ -172,15 +172,6 @@ class Assertion extends SignedElement */ private $attributes = []; - /** - * The NameFormat used on all attributes. - * - * If more than one NameFormat is used, this will contain the unspecified nameformat. - * - * @var string - */ - private $nameFormat = Constants::NAMEFORMAT_UNSPECIFIED; - /** * The data needed to verify the signature. * @@ -492,14 +483,10 @@ private function parseAuthnContext(DOMElement $authnStatementEl): void */ private function parseAttributes(DOMElement $xml): void { - $firstAttribute = true; /** @var \DOMElement[] $attributes */ $attributes = Utils::xpQuery($xml, './saml_assertion:AttributeStatement/saml_assertion:Attribute'); foreach ($attributes as $attribute) { - $this->parseAttribute($attribute, $firstAttribute); - if ($firstAttribute) { - $firstAttribute = false; - } + $this->parseAttribute($attribute); } } @@ -507,28 +494,13 @@ private function parseAttributes(DOMElement $xml): void * Parse attribute statements in assertion. * * @param \DOMElement $xml The XML element with the Attribute. - * @param boolean $firstAttribute Whether this is the first attribute to parse. * @throws \Exception * @return void */ - private function parseAttribute(DOMElement $xml, bool $firstAttribute = false): void + private function parseAttribute(DOMElement $xml): void { $attribute = new Attribute($xml); - if ($attribute->getNameFormat() !== null) { - $nameFormat = $attribute->getNameFormat(); - } else { - $nameFormat = Constants::NAMEFORMAT_UNSPECIFIED; - } - - if ($firstAttribute) { - $this->nameFormat = $nameFormat; - } else { - if ($this->nameFormat !== $nameFormat) { - $this->nameFormat = Constants::NAMEFORMAT_UNSPECIFIED; - } - } - $name = $attribute->getName(); if (!array_key_exists($name, $this->attributes)) { $this->attributes[$name] = $attribute; @@ -800,7 +772,6 @@ public function decryptAttributes(XMLSecurityKey $key, array $blacklist = []): v if (!$this->hasEncryptedAttributes()) { return; } - $firstAttribute = true; $attributes = $this->getEncryptedAttributes(); foreach ($attributes as $attributeEnc) { /* Decrypt node */ @@ -809,10 +780,7 @@ public function decryptAttributes(XMLSecurityKey $key, array $blacklist = []): v $key, $blacklist ); - $this->parseAttribute($attribute, $firstAttribute); - if ($firstAttribute) { - $firstAttribute = false; - } + $this->parseAttribute($attribute); } } @@ -1173,16 +1141,16 @@ public function getAttributeValues(): array $nameId = null; if ($value instanceof DOMNodeList) { foreach ($value as $node) { - /** @var \DOMNode $node */ if ( $node->nodeType !== XML_TEXT_NODE && $node->localName === 'NameID' && $node->namespaceURI === Constants::NS_SAML ) { + /** @var \DOMElement $node */ $nameId = new NameID($node); } } - } else { + } elseif (is_string($value)) { /* Fall back for legacy IdPs sending string value (e.g. SSP < 1.15) */ Utils::getContainer()->getLogger()->warning( sprintf("Attribute %s (EPTI) value %d is not an XML NameId", $attributeName, $value) @@ -1236,32 +1204,6 @@ public function setSignatureData(array $signatureData = null): void } - /** - * Retrieve the NameFormat used on all attributes. - * - * If more than one NameFormat is used in the received attributes, this - * returns the unspecified NameFormat. - * - * @return string The NameFormat used on all attributes. - */ - public function getAttributeNameFormat(): string - { - return $this->nameFormat; - } - - - /** - * Set the NameFormat used on all attributes. - * - * @param string $nameFormat The NameFormat used on all attributes. - * @return void - */ - public function setAttributeNameFormat(string $nameFormat): void - { - $this->nameFormat = $nameFormat; - } - - /** * Retrieve the SubjectConfirmation elements we have in our Subject element. * diff --git a/src/SAML2/AttributeQuery.php b/src/SAML2/AttributeQuery.php index d2ef7a6dd..0f97bccd9 100644 --- a/src/SAML2/AttributeQuery.php +++ b/src/SAML2/AttributeQuery.php @@ -30,16 +30,6 @@ class AttributeQuery extends SubjectQuery */ private $attributes = []; - /** - * The NameFormat used on all attributes. - * - * If more than one NameFormat is used, this will contain - * the unspecified nameformat. - * - * @var string - */ - private $nameFormat = Constants::NAMEFORMAT_UNSPECIFIED; - /** * Constructor for SAML 2 attribute query messages. @@ -55,7 +45,6 @@ public function __construct(DOMElement $xml = null) return; } - $firstAttribute = true; /** @var \DOMElement[] $attributes */ $attributes = Utils::xpQuery($xml, './saml_assertion:Attribute'); foreach ($attributes as $attribute) { @@ -64,18 +53,10 @@ public function __construct(DOMElement $xml = null) } $name = $attribute->getAttribute('Name'); - $nameFormat = $this->nameFormat; if ($attribute->hasAttribute('NameFormat')) { $nameFormat = $attribute->getAttribute('NameFormat'); } - if ($firstAttribute) { - $this->nameFormat = $nameFormat; - $firstAttribute = false; - } elseif ($this->nameFormat !== $nameFormat) { - $this->nameFormat = Constants::NAMEFORMAT_UNSPECIFIED; - } - if (!array_key_exists($name, $this->attributes)) { $this->attributes[$name] = []; } @@ -111,32 +92,6 @@ public function setAttributes(array $attributes): void } - /** - * Retrieve the NameFormat used on all attributes. - * - * If more than one NameFormat is used in the received attributes, this - * returns the unspecified NameFormat. - * - * @return string The NameFormat used on all attributes. - */ - public function getAttributeNameFormat(): string - { - return $this->nameFormat; - } - - - /** - * Set the NameFormat used on all attributes. - * - * @param string $nameFormat The NameFormat used on all attributes. - * @return void - */ - public function setAttributeNameFormat(string $nameFormat): void - { - $this->nameFormat = $nameFormat; - } - - /** * Convert the attribute query message to an XML element. * @@ -151,10 +106,6 @@ public function toUnsignedXML(): DOMElement $root->appendChild($attribute); $attribute->setAttribute('Name', $name); - if ($this->nameFormat !== Constants::NAMEFORMAT_UNSPECIFIED) { - $attribute->setAttribute('NameFormat', $this->nameFormat); - } - foreach ($values as $value) { if (is_string($value)) { $type = 'xs:string'; diff --git a/tests/SAML2/AssertionTest.php b/tests/SAML2/AssertionTest.php index 666693300..3c0523f32 100644 --- a/tests/SAML2/AssertionTest.php +++ b/tests/SAML2/AssertionTest.php @@ -148,8 +148,6 @@ public function testMarshallingUnmarshallingChristmas(): void "name3" => [null] ])); - $assertion->setAttributeNameFormat("urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified"); - $assertionElement = $assertion->toXML()->ownerDocument->saveXML(); $assertionToVerify = new Assertion(DOMDocumentFactory::fromString($assertionElement)->firstChild); @@ -175,7 +173,6 @@ public function testMarshallingUnmarshallingChristmas(): void $this->assertEquals("value1", $attributes['name1'][0]); $this->assertEquals(2, $attributes['name2'][0]); $this->assertNull($attributes["name3"][0]); - $this->assertEquals(\SAML2\Constants::NAMEFORMAT_UNSPECIFIED, $assertionToVerify->getAttributeNameFormat()); } @@ -231,8 +228,6 @@ public function testMixedAttributes(): void $epti6->getName() => $epti6 ]); - $assertion->setAttributeNameFormat("urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified"); - $assertionElement = $assertion->toXML()->ownerDocument->saveXML(); $assertionToVerify = new Assertion(DOMDocumentFactory::fromString($assertionElement)->firstChild); @@ -284,7 +279,6 @@ public function testMarshallingUnmarshallingAttributeValTypes(): void "name2" => [2], "name3" => [1234, "+2345"] ], $attributesValueTypes)); - $assertion->setAttributeNameFormat(\SAML2\Constants::NAMEFORMAT_UNSPECIFIED); $assertionElement = $assertion->toXML()->ownerDocument->saveXML(); @@ -313,7 +307,6 @@ public function testMarshallingUnmarshallingAttributeValTypes(): void $this->assertCount(2, $attributes['name3']); $this->assertEquals("1234", $attributes['name3'][0]); $this->assertEquals("+2345", $attributes['name3'][1]); - $this->assertEquals(\SAML2\Constants::NAMEFORMAT_UNSPECIFIED, $assertionToVerify->getAttributeNameFormat()); @@ -347,7 +340,6 @@ public function testMarshallingWrongAttributeValTypes(): void $assertion->setValidAudiences(['audience1', 'audience2']); $assertion->setAuthnContextClassRef('someAuthnContext'); $assertion->setAuthenticatingAuthority(["idp1", "idp2"]); - $assertion->setAttributeNameFormat(\SAML2\Constants::NAMEFORMAT_UNSPECIFIED); // set wrong number elements in name1 $attributeValueTypes = [ @@ -1899,41 +1891,6 @@ public function testMissingNameOnAttribute(): void } - /** - * If this assertion mixes Attribute NameFormats, the AttributeNameFormat - * of this assertion will be set to unspecified. - */ - public function testMixedAttributeNameFormats(): void - { - $xml = << - Provider - - - - string - - - string - - - -XML; - - $assertion = new Assertion(DOMDocumentFactory::fromString($xml)->firstChild); - - $nameFormat = $assertion->getAttributeNameFormat(); - $this->assertEquals('urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified', $nameFormat); - } - - /** * Test basic NameID unmarshalling. */ @@ -2069,7 +2026,6 @@ public function testMarshallingElementOrdering(): void "name1" => ["value1","value2"], "name2" => ["value3"], ])); - $assertion->setAttributeNameFormat("urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified"); $assertion->setSignatureKey(CertificatesMock::getPrivateKey()); $nameId = new NameID(); diff --git a/tests/SAML2/AttributeQueryTest.php b/tests/SAML2/AttributeQueryTest.php index 320bf5f63..7eb2fc664 100644 --- a/tests/SAML2/AttributeQueryTest.php +++ b/tests/SAML2/AttributeQueryTest.php @@ -113,109 +113,9 @@ public function testUnmarshalling(): void $this->assertEquals('urn:oid:2.5.4.4', $attributes[1]); $this->assertEquals('urn:oid:2.16.840.1.113730.3.1.39', $attributes[2]); - $this->assertEquals('urn:oasis:names:tc:SAML:2.0:attrname-format:uri', $aq->getAttributeNameFormat()); - } - - - public function testAttributeNameFormat(): void - { - $fmt_uri = 'urn:oasis:names:tc:SAML:2.0:attrname-format:uri'; - - $attributeQuery = new AttributeQuery(); - $nameId = new NameID(); - $nameId->setValue('NameIDValue'); - $attributeQuery->setNameID($nameId); - $attributeQuery->setAttributes( - [ - 'test1' => [ - 'test1_attrv1', - 'test1_attrv2', - ], - 'test2' => [ - 'test2_attrv1', - 'test2_attrv2', - 'test2_attrv3', - ], - 'test3' => [], - ] - ); - $attributeQuery->setAttributeNameFormat($fmt_uri); - $attributeQueryElement = $attributeQuery->toUnsignedXML(); - - // Test Attribute Names - $attributes = Utils::xpQuery($attributeQueryElement, './saml_assertion:Attribute'); - $this->assertCount(3, $attributes); - $this->assertEquals('test1', $attributes[0]->getAttribute('Name')); - $this->assertEquals($fmt_uri, $attributes[0]->getAttribute('NameFormat')); - $this->assertEquals('test2', $attributes[1]->getAttribute('Name')); - $this->assertEquals($fmt_uri, $attributes[1]->getAttribute('NameFormat')); - $this->assertEquals('test3', $attributes[2]->getAttribute('Name')); - $this->assertEquals($fmt_uri, $attributes[2]->getAttribute('NameFormat')); - - // Sanity check: test if values are still ok - $av1 = Utils::xpQuery($attributes[0], './saml_assertion:AttributeValue'); - $this->assertCount(2, $av1); - $this->assertEquals('test1_attrv1', $av1[0]->textContent); - $this->assertEquals('test1_attrv2', $av1[1]->textContent); - } - - - public function testNoNameFormatDefaultsToUnspecified(): void - { - $xml = << - https://example.org/ - - urn:example:subject - - - - -XML; - $document = DOMDocumentFactory::fromString($xml); - $aq = new AttributeQuery($document->firstChild); - - // Sanity check - $this->assertEquals('https://example.org/', $aq->getIssuer()->getValue()); - - $this->assertEquals('urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified', $aq->getAttributeNameFormat()); - } - - - public function testMultiNameFormatDefaultsToUnspecified(): void - { - $xml = << - https://example.org/ - - urn:example:subject - - - - - - - - -XML; - $document = DOMDocumentFactory::fromString($xml); - $aq = new AttributeQuery($document->firstChild); - - // Sanity check - $this->assertEquals('https://example.org/', $aq->getIssuer()->getValue()); - - $this->assertEquals('urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified', $aq->getAttributeNameFormat()); + $this->assertEquals(Constants::NAMEFORMAT_URI, $attributes[0]->NameFormat()); + $this->assertEquals(Constants::NAMEFORMAT_URI, $attributes[1]->NameFormat()); + $this->assertEquals(Constants::NAMEFORMAT_URI, $attributes[2]->NameFormat()); }