From c18bc48af95b4a43a9a0bd22bd21b4f21556fb66 Mon Sep 17 00:00:00 2001 From: Thijs Kinkhorst Date: Tue, 23 Feb 2016 16:19:01 +0000 Subject: [PATCH 01/30] Add tests for shibmd:scope. --- tests/SAML2/XML/shibmd/ScopeTest.php | 68 ++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 tests/SAML2/XML/shibmd/ScopeTest.php diff --git a/tests/SAML2/XML/shibmd/ScopeTest.php b/tests/SAML2/XML/shibmd/ScopeTest.php new file mode 100644 index 000000000..b1c53fe42 --- /dev/null +++ b/tests/SAML2/XML/shibmd/ScopeTest.php @@ -0,0 +1,68 @@ +scope = "example.org"; + $scope->regexp = FALSE; + + $document = DOMDocumentFactory::fromString(''); + $scopeElement = $scope->toXML($document->firstChild); + + $scopeElements = Utils::xpQuery($scopeElement, '/root/shibmd:Scope'); + $this->assertCount(1, $scopeElements); + $scopeElement = $scopeElements[0]; + + $this->assertEquals('example.org', $scopeElement->nodeValue); + $this->assertEquals('urn:mace:shibboleth:metadata:1.0', $scopeElement->namespaceURI); + $this->assertEquals('false', $scopeElement->getAttribute('regexp')); + + $scope = new Scope(); + $scope->scope = "^(.*\.)?example\.edu$"; + $scope->regexp = TRUE; + + $document = DOMDocumentFactory::fromString(''); + $scopeElement = $scope->toXML($document->firstChild); + + $scopeElements = Utils::xpQuery($scopeElement, '/root/shibmd:Scope'); + $this->assertCount(1, $scopeElements); + $scopeElement = $scopeElements[0]; + + $this->assertEquals('^(.*\.)?example\.edu$', $scopeElement->nodeValue); + $this->assertEquals('urn:mace:shibboleth:metadata:1.0', $scopeElement->namespaceURI); + $this->assertEquals('true', $scopeElement->getAttribute('regexp')); + } + + public function testUnmarshalling() + { + $document = DOMDocumentFactory::fromString( +<<example.org +XML + ); + $scope = new Scope($document->firstChild); + + $this->assertEquals('example.org', $scope->scope); + $this->assertFalse($scope->regexp); + + $document = DOMDocumentFactory::fromString( +<<^(.*|)example.edu$ +XML + ); + $scope = new Scope($document->firstChild); + + $this->assertEquals('^(.*|)example.edu$', $scope->scope); + $this->assertTrue($scope->regexp); + } +} From 8b0601637db70f3a10d48d773dc0465219d74335 Mon Sep 17 00:00:00 2001 From: Thijs Kinkhorst Date: Tue, 23 Feb 2016 16:36:35 +0000 Subject: [PATCH 02/30] shibmd:scope regexp attribute should default to false and be always explicit The regexp attribute of a Scope used to default to null when not set explicitly. According to the spec, when omitted this should be interpreted as it being false. Therefore, I think the null default is not correct to have. Also, we should always output an explicit regexp attribute, as recommended by the spec for document signing reasons: https://wiki.shibboleth.net/confluence/display/SC/ShibMetaExt+V1.0 --- src/SAML2/XML/shibmd/Scope.php | 8 ++++---- tests/SAML2/XML/shibmd/ScopeTest.php | 27 +++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/SAML2/XML/shibmd/Scope.php b/src/SAML2/XML/shibmd/Scope.php index d3d1ca32c..60f509534 100644 --- a/src/SAML2/XML/shibmd/Scope.php +++ b/src/SAML2/XML/shibmd/Scope.php @@ -27,9 +27,9 @@ class Scope /** * Whether this is a regexp scope. * - * @var bool|null + * @var bool */ - public $regexp = null; + public $regexp = false; /** * Create a Scope. @@ -43,7 +43,7 @@ public function __construct(\DOMElement $xml = null) } $this->scope = $xml->textContent; - $this->regexp = Utils::parseBoolean($xml, 'regexp', null); + $this->regexp = Utils::parseBoolean($xml, 'regexp', false); } /** @@ -66,7 +66,7 @@ public function toXML(\DOMElement $parent) if ($this->regexp === true) { $e->setAttribute('regexp', 'true'); - } elseif ($this->regexp === false) { + } else { $e->setAttribute('regexp', 'false'); } diff --git a/tests/SAML2/XML/shibmd/ScopeTest.php b/tests/SAML2/XML/shibmd/ScopeTest.php index b1c53fe42..deb562446 100644 --- a/tests/SAML2/XML/shibmd/ScopeTest.php +++ b/tests/SAML2/XML/shibmd/ScopeTest.php @@ -12,6 +12,7 @@ class ScopeTest extends \PHPUnit_Framework_TestCase { public function testMarshalling() { + // In literal, non-regexp form $scope = new Scope(); $scope->scope = "example.org"; $scope->regexp = FALSE; @@ -27,6 +28,22 @@ public function testMarshalling() $this->assertEquals('urn:mace:shibboleth:metadata:1.0', $scopeElement->namespaceURI); $this->assertEquals('false', $scopeElement->getAttribute('regexp')); + // Without explicit regexp: should yield explicit 'false' + $scope = new Scope(); + $scope->scope = "example.org"; + + $document = DOMDocumentFactory::fromString(''); + $scopeElement = $scope->toXML($document->firstChild); + + $scopeElements = Utils::xpQuery($scopeElement, '/root/shibmd:Scope'); + $this->assertCount(1, $scopeElements); + $scopeElement = $scopeElements[0]; + + $this->assertEquals('example.org', $scopeElement->nodeValue); + $this->assertEquals('urn:mace:shibboleth:metadata:1.0', $scopeElement->namespaceURI); + $this->assertEquals('false', $scopeElement->getAttribute('regexp')); + + // In regexp form $scope = new Scope(); $scope->scope = "^(.*\.)?example\.edu$"; $scope->regexp = TRUE; @@ -57,6 +74,16 @@ public function testUnmarshalling() $document = DOMDocumentFactory::fromString( <<example.org +XML + ); + $scope = new Scope($document->firstChild); + + $this->assertEquals('example.org', $scope->scope); + $this->assertFalse($scope->regexp); + + $document = DOMDocumentFactory::fromString( +<<^(.*|)example.edu$ XML ); From 3d93cf9a84cdae4f949c3678e9ef401b9287b34d Mon Sep 17 00:00:00 2001 From: Thijs Kinkhorst Date: Wed, 24 Feb 2016 10:02:47 +0000 Subject: [PATCH 03/30] Fast finish build so we get feedback quickly. Especially relevant now that a HHVM bug causes long hangs while HHVM is an allowed failure. --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 921a88180..f3ae19304 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,6 +9,7 @@ php: - hhvm matrix: + fast_finish: true allow_failures: - php: hhvm From dded4f776ccf75bad3d278f50a9bb67c94386ae8 Mon Sep 17 00:00:00 2001 From: Thijs Kinkhorst Date: Wed, 24 Feb 2016 16:32:50 +0000 Subject: [PATCH 04/30] Complete coverage for XML/mdrpi/ --- tests/SAML2/XML/mdrpi/PublicationInfoTest.php | 14 +++ .../SAML2/XML/mdrpi/RegistrationInfoTest.php | 96 +++++++++++++++++++ 2 files changed, 110 insertions(+) create mode 100644 tests/SAML2/XML/mdrpi/RegistrationInfoTest.php diff --git a/tests/SAML2/XML/mdrpi/PublicationInfoTest.php b/tests/SAML2/XML/mdrpi/PublicationInfoTest.php index d58c83c67..830fa8200 100644 --- a/tests/SAML2/XML/mdrpi/PublicationInfoTest.php +++ b/tests/SAML2/XML/mdrpi/PublicationInfoTest.php @@ -69,4 +69,18 @@ public function testUnmarshalling() $this->assertEquals('http://TheEnglishUsagePolicy', $publicationInfo->UsagePolicy["en"]); $this->assertEquals('http://TheNorwegianUsagePolicy', $publicationInfo->UsagePolicy["no"]); } + + public function testMissingPublisherThrowsException() + { + $document = DOMDocumentFactory::fromString(<< + +XML + ); + + $this->setExpectedException('Exception', 'Missing required attribute "publisher"'); + $publicationInfo = new PublicationInfo($document->firstChild); + } } diff --git a/tests/SAML2/XML/mdrpi/RegistrationInfoTest.php b/tests/SAML2/XML/mdrpi/RegistrationInfoTest.php new file mode 100644 index 000000000..76d123813 --- /dev/null +++ b/tests/SAML2/XML/mdrpi/RegistrationInfoTest.php @@ -0,0 +1,96 @@ +registrationAuthority = 'https://ExampleAuthority'; + $registrationInfo->registrationInstant = 1234567890; + $registrationInfo->RegistrationPolicy = array( + 'en' => 'http://EnglishRegistrationPolicy', + 'nl' => 'https://DutchRegistratiebeleid', + ); + + $document = DOMDocumentFactory::fromString(''); + $xml = $registrationInfo->toXML($document->firstChild); + + $registrationInfoElements = Utils::xpQuery( + $xml, + '/root/*[local-name()=\'RegistrationInfo\' and namespace-uri()=\'urn:oasis:names:tc:SAML:metadata:rpi\']' + ); + $this->assertCount(1, $registrationInfoElements); + $registrationInfoElement = $registrationInfoElements[0]; + + $this->assertEquals('https://ExampleAuthority', $registrationInfoElement->getAttribute("registrationAuthority")); + $this->assertEquals('2009-02-13T23:31:30Z', $registrationInfoElement->getAttribute("registrationInstant")); + + $usagePolicyElements = Utils::xpQuery( + $registrationInfoElement, + './*[local-name()=\'RegistrationPolicy\' and namespace-uri()=\'urn:oasis:names:tc:SAML:metadata:rpi\']' + ); + $this->assertCount(2, $usagePolicyElements); + + $this->assertEquals('en', $usagePolicyElements[0]->getAttributeNS("http://www.w3.org/XML/1998/namespace", "lang")); + $this->assertEquals('http://EnglishRegistrationPolicy', $usagePolicyElements[0]->textContent); + $this->assertEquals('nl', $usagePolicyElements[1]->getAttributeNS("http://www.w3.org/XML/1998/namespace", "lang")); + $this->assertEquals('https://DutchRegistratiebeleid', $usagePolicyElements[1]->textContent); + } + + public function testUnmarshalling() + { + $document = DOMDocumentFactory::fromString(<< + + http://www.example.org/aai/metadata/en_registration.html + + + http://www.example.org/aai/metadata/de_registration.html + + +XML + ); + + $registrationInfo = new RegistrationInfo($document->firstChild); + + $this->assertEquals('urn:example:example.org', $registrationInfo->registrationAuthority); + $this->assertEquals(1148902467, $registrationInfo->registrationInstant); + $this->assertCount(2, $registrationInfo->RegistrationPolicy); + $this->assertEquals('http://www.example.org/aai/metadata/en_registration.html', $registrationInfo->RegistrationPolicy["en"]); + $this->assertEquals('http://www.example.org/aai/metadata/de_registration.html', $registrationInfo->RegistrationPolicy["de"]); + } + + public function testMissingPublisherThrowsException() + { + $document = DOMDocumentFactory::fromString(<< + +XML + ); + + $this->setExpectedException('Exception', 'Missing required attribute "registrationAuthority"'); + $registrationInfo = new RegistrationInfo($document->firstChild); + } + + public function testEmptyRegistrationAuthorityOutboundThrowsException() + { + $registrationInfo = new RegistrationInfo(); + $registrationInfo->registrationAuthority = ''; + + $document = DOMDocumentFactory::fromString(''); + + $this->setExpectedException('Exception', 'Missing required registration authority.'); + $xml = $registrationInfo->toXML($document->firstChild); + } +} From dee5f42e99afc73052ea9539cc5a6f026951edab Mon Sep 17 00:00:00 2001 From: Thijs Kinkhorst Date: Wed, 24 Feb 2016 17:20:13 +0000 Subject: [PATCH 05/30] Add coverage for mdattr --- .../SAML2/XML/mdattr/EntityAttributesTest.php | 105 ++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 tests/SAML2/XML/mdattr/EntityAttributesTest.php diff --git a/tests/SAML2/XML/mdattr/EntityAttributesTest.php b/tests/SAML2/XML/mdattr/EntityAttributesTest.php new file mode 100644 index 000000000..0fc326fd5 --- /dev/null +++ b/tests/SAML2/XML/mdattr/EntityAttributesTest.php @@ -0,0 +1,105 @@ +Name = 'urn:simplesamlphp:v1:simplesamlphp'; + $attribute1->NameFormat = 'urn:oasis:names:tc:SAML:2.0:attrname-format:uri'; + $attribute1->AttributeValue = array( + new AttributeValue('FirstValue'), + new AttributeValue('SecondValue'), + ); + $attribute2 = new Attribute(); + $attribute2->Name = 'foo'; + $attribute2->NameFormat = 'urn:simplesamlphp:v1'; + $attribute2->AttributeValue = array( + new AttributeValue('bar'), + ); + + $entityAttributes = new EntityAttributes(); + $entityAttributes->children[] = $attribute1; + $entityAttributes->children[] = $attribute2; + + $document = DOMDocumentFactory::fromString(''); + $xml = $entityAttributes->toXML($document->firstChild); + + $entityAttributesElements = Utils::xpQuery( + $xml, + '/root/*[local-name()=\'EntityAttributes\' and namespace-uri()=\'urn:oasis:names:tc:SAML:metadata:attribute\']' + ); + $this->assertCount(1, $entityAttributesElements); + $entityAttributesElement = $entityAttributesElements[0]; + + $attributeElements = Utils::xpQuery( + $entityAttributesElement, + './*[local-name()=\'Attribute\' and namespace-uri()=\'urn:oasis:names:tc:SAML:2.0:assertion\']' + ); + $this->assertCount(2, $attributeElements); + } + + public function testUnmarshalling() + { + $document = DOMDocumentFactory::fromString(<< + + + + + + +XML + ); + + $entityAttributes = new EntityAttributes($document->firstChild); + $this->assertCount(5, $entityAttributes->children); + + $this->assertInstanceOf('SAML2\XML\Chunk', $entityAttributes->children[0]); + $this->assertInstanceOf('SAML2\XML\saml\Attribute', $entityAttributes->children[1]); + $this->assertInstanceOf('SAML2\XML\Chunk', $entityAttributes->children[2]); + $this->assertInstanceOf('SAML2\XML\saml\Attribute', $entityAttributes->children[3]); + $this->assertInstanceOf('SAML2\XML\saml\Attribute', $entityAttributes->children[4]); + + $this->assertEquals('Assertion', $entityAttributes->children[0]->localName); + $this->assertEquals('1984-08-26T10:01:30.000Z', $entityAttributes->children[0]->xml->getAttribute('IssueInstant')); + $this->assertEquals('attrib2', $entityAttributes->children[3]->Name); + } + + public function testUnmarshallingAttributes() + { + $document = DOMDocumentFactory::fromString(<< + + is + really + cool + + + bar + + +XML + ); + + $entityAttributes = new EntityAttributes($document->firstChild); + $this->assertCount(2, $entityAttributes->children); + + $this->assertEquals('urn:simplesamlphp:v1:simplesamlphp', $entityAttributes->children[0]->Name); + $this->assertEquals('urn:oasis:names:tc:SAML:2.0:attrname-format:uri', $entityAttributes->children[0]->NameFormat); + $this->assertCount(3, $entityAttributes->children[0]->AttributeValue); + $this->assertEquals('foo', $entityAttributes->children[1]->Name); + $this->assertEquals('urn:simplesamlphp:v1', $entityAttributes->children[1]->NameFormat); + $this->assertCount(1, $entityAttributes->children[1]->AttributeValue); + } +} From 9236ce97be8a9bcc4c5ccbae57e971689412835c Mon Sep 17 00:00:00 2001 From: Thijs Kinkhorst Date: Wed, 24 Feb 2016 18:28:00 +0000 Subject: [PATCH 06/30] Complete coverage of SubjectConfirmation --- .../XML/saml/SubjectConfirmationTest.php | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/tests/SAML2/XML/saml/SubjectConfirmationTest.php b/tests/SAML2/XML/saml/SubjectConfirmationTest.php index 48c16898e..9a54a80bf 100644 --- a/tests/SAML2/XML/saml/SubjectConfirmationTest.php +++ b/tests/SAML2/XML/saml/SubjectConfirmationTest.php @@ -48,4 +48,54 @@ public function testUnmarshalling() $this->assertEquals('SomeNameIDValue', $subjectConfirmation->NameID->value); $this->assertTrue($subjectConfirmation->SubjectConfirmationData instanceof SubjectConfirmationData); } + + public function testMethodMissingThrowsException() + { + $samlNamespace = Constants::NS_SAML; + $document = DOMDocumentFactory::fromString( +<< + SomeNameIDValue + + +XML + ); + + $this->setExpectedException('Exception', 'SubjectConfirmation element without Method attribute'); + $subjectConfirmation = new SubjectConfirmation($document->firstChild); + } + + public function testManyNameIDThrowsException() + { + $samlNamespace = Constants::NS_SAML; + $document = DOMDocumentFactory::fromString( +<< + SomeNameIDValue + AnotherNameIDValue + + +XML + ); + + $this->setExpectedException('Exception', 'More than one NameID in a SubjectConfirmation element'); + $subjectConfirmation = new SubjectConfirmation($document->firstChild); + } + + public function testManySubjectConfirmationDataThrowsException() + { + $samlNamespace = Constants::NS_SAML; + $document = DOMDocumentFactory::fromString( +<< + SomeNameIDValue + + + +XML + ); + + $this->setExpectedException('Exception', 'More than one SubjectConfirmationData child in a SubjectConfirmation element'); + $subjectConfirmation = new SubjectConfirmation($document->firstChild); + } } From b30b2f19e3fba828e394d635d7ab6750ad03910d Mon Sep 17 00:00:00 2001 From: Thijs Kinkhorst Date: Fri, 26 Feb 2016 15:01:17 +0000 Subject: [PATCH 07/30] Doc fix: unserialize() does not return Chunk object --- src/SAML2/XML/Chunk.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/SAML2/XML/Chunk.php b/src/SAML2/XML/Chunk.php index 6f412eb62..2f25f4510 100644 --- a/src/SAML2/XML/Chunk.php +++ b/src/SAML2/XML/Chunk.php @@ -82,7 +82,6 @@ public function serialize() * Un-serialize this XML chunk. * * @param string $serialized The serialized chunk. - * @return \SAML2\XML\Chunk The chunk object represented by the serialized string. */ public function unserialize($serialized) { From ac72699ae79451bc3c07dd153ad5f1c6ed9b790a Mon Sep 17 00:00:00 2001 From: Thijs Kinkhorst Date: Fri, 26 Feb 2016 15:02:53 +0000 Subject: [PATCH 08/30] Complete coverage for Chunk and Extensions --- tests/SAML2/XML/ChunkTest.php | 63 ++++++++++++++++ tests/SAML2/XML/samlp/ExtensionsTest.php | 95 ++++++++++++++++++++++++ 2 files changed, 158 insertions(+) create mode 100644 tests/SAML2/XML/ChunkTest.php create mode 100644 tests/SAML2/XML/samlp/ExtensionsTest.php diff --git a/tests/SAML2/XML/ChunkTest.php b/tests/SAML2/XML/ChunkTest.php new file mode 100644 index 000000000..ac574f61f --- /dev/null +++ b/tests/SAML2/XML/ChunkTest.php @@ -0,0 +1,63 @@ +Name = 'TheName'; + $attribute->NameFormat = 'TheNameFormat'; + $attribute->FriendlyName = 'TheFriendlyName'; + $attribute->AttributeValue = array( + new AttributeValue('FirstValue'), + new AttributeValue('SecondValue'), + ); + + $document = DOMDocumentFactory::fromString(''); + $attributeElement = $attribute->toXML($document->firstChild); + + $this->chunk = new Chunk($attributeElement); + } + + /** + * Test the getXML() method + */ + public function testChunkGetXML() + { + $xml = $this->chunk->getXML(); + $this->assertInstanceOf('DOMElement', $xml); + $this->assertEquals('saml:Attribute', $xml->tagName); + } + + + /** + * Test serialization and unserialization + */ + public function testChunkSerializationLoop() + { + $ser = $this->chunk->serialize(); + $document = DOMDocumentFactory::fromString(''); + $newchunk = new Chunk($document->firstChild); + $newchunk->unserialize($ser); + + $this->assertEqualXMLStructure($this->chunk->getXML(), $newchunk->getXML()); + } + +} diff --git a/tests/SAML2/XML/samlp/ExtensionsTest.php b/tests/SAML2/XML/samlp/ExtensionsTest.php new file mode 100644 index 000000000..021994c96 --- /dev/null +++ b/tests/SAML2/XML/samlp/ExtensionsTest.php @@ -0,0 +1,95 @@ + + max.feide.no + + + + + + + + + + +XML + ); + $this->testElement = $document->documentElement; + } + + /** + * Test getList() method + */ + public function testExtensionsGet() + { + $list = Extensions::getList($this->testElement); + + $this->assertCount(2, $list); + $this->assertEquals("urn:mynamespace", $list[0]->namespaceURI); + $this->assertEquals("ExampleElement", $list[1]->localName); + } + + /** + * Adding empty list should leave exising extensions unchanged. + */ + public function testExtensionsAddEmpty() + { + Extensions::addList($this->testElement, array()); + + $list = Extensions::getList($this->testElement); + + $this->assertCount(2, $list); + $this->assertEquals("urn:mynamespace", $list[0]->namespaceURI); + $this->assertEquals("ExampleElement", $list[1]->localName); + } + + /** + * Test adding two random elements + */ + public function testExtensionsAddSome() + { + $attribute = new Attribute(); + $attribute->Name = 'TheName'; + $scope = new Scope(); + $scope->scope = "scope"; + + Extensions::addList($this->testElement, array($attribute, $scope)); + + $list = Extensions::getList($this->testElement); + + $this->assertCount(4, $list); + $this->assertEquals("urn:mynamespace", $list[0]->namespaceURI); + $this->assertEquals("ExampleElement", $list[1]->localName); + $this->assertEquals("Attribute", $list[2]->localName); + $this->assertEquals("urn:mace:shibboleth:metadata:1.0", $list[3]->namespaceURI); + } +} From 2cd087c5dc42ff243858ed4552039d2c12701b02 Mon Sep 17 00:00:00 2001 From: Thijs Kinkhorst Date: Fri, 26 Feb 2016 15:32:15 +0000 Subject: [PATCH 09/30] Basic test coverage for DiscoHints --- tests/SAML2/XML/mdrpi/PublicationInfoTest.php | 2 +- .../SAML2/XML/mdrpi/RegistrationInfoTest.php | 4 +- tests/SAML2/XML/mdui/DiscoHintsTest.php | 94 +++++++++++++++++++ 3 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 tests/SAML2/XML/mdui/DiscoHintsTest.php diff --git a/tests/SAML2/XML/mdrpi/PublicationInfoTest.php b/tests/SAML2/XML/mdrpi/PublicationInfoTest.php index 830fa8200..a6b9515ab 100644 --- a/tests/SAML2/XML/mdrpi/PublicationInfoTest.php +++ b/tests/SAML2/XML/mdrpi/PublicationInfoTest.php @@ -80,7 +80,7 @@ public function testMissingPublisherThrowsException() XML ); - $this->setExpectedException('Exception', 'Missing required attribute "publisher"'); + $this->setExpectedException('Exception', 'Missing required attribute "publisher"'); $publicationInfo = new PublicationInfo($document->firstChild); } } diff --git a/tests/SAML2/XML/mdrpi/RegistrationInfoTest.php b/tests/SAML2/XML/mdrpi/RegistrationInfoTest.php index 76d123813..981143e11 100644 --- a/tests/SAML2/XML/mdrpi/RegistrationInfoTest.php +++ b/tests/SAML2/XML/mdrpi/RegistrationInfoTest.php @@ -79,7 +79,7 @@ public function testMissingPublisherThrowsException() XML ); - $this->setExpectedException('Exception', 'Missing required attribute "registrationAuthority"'); + $this->setExpectedException('Exception', 'Missing required attribute "registrationAuthority"'); $registrationInfo = new RegistrationInfo($document->firstChild); } @@ -90,7 +90,7 @@ public function testEmptyRegistrationAuthorityOutboundThrowsException() $document = DOMDocumentFactory::fromString(''); - $this->setExpectedException('Exception', 'Missing required registration authority.'); + $this->setExpectedException('Exception', 'Missing required registration authority.'); $xml = $registrationInfo->toXML($document->firstChild); } } diff --git a/tests/SAML2/XML/mdui/DiscoHintsTest.php b/tests/SAML2/XML/mdui/DiscoHintsTest.php new file mode 100644 index 000000000..f8a7b5c12 --- /dev/null +++ b/tests/SAML2/XML/mdui/DiscoHintsTest.php @@ -0,0 +1,94 @@ +IPHint = array("192.168.6.0/24", "fd00:0123:aa:1001::/64"); + $discoHints->DomainHint = array("example.org", "student.example.org"); + $discoHints->GeolocationHint = array("geo:47.37328,8.531126", "geo:19.34343,12.342514"); + + $document = DOMDocumentFactory::fromString(''); + $xml = $discoHints->toXML($document->firstChild); + + $discoElements = Utils::xpQuery( + $xml, + '/root/*[local-name()=\'DiscoHints\' and namespace-uri()=\'urn:oasis:names:tc:SAML:metadata:ui\']' + ); + $this->assertCount(1, $discoElements); + $discoElement = $discoElements[0]; + + $ipHintElements = Utils::xpQuery( + $discoElement, + './*[local-name()=\'IPHint\' and namespace-uri()=\'urn:oasis:names:tc:SAML:metadata:ui\']' + ); + $this->assertCount(2, $ipHintElements); + $this->assertEquals("192.168.6.0/24", $ipHintElements[0]->textContent); + $this->assertEquals("fd00:0123:aa:1001::/64", $ipHintElements[1]->textContent); + + $domainHintElements = Utils::xpQuery( + $discoElement, + './*[local-name()=\'DomainHint\' and namespace-uri()=\'urn:oasis:names:tc:SAML:metadata:ui\']' + ); + $this->assertCount(2, $domainHintElements); + $this->assertEquals("example.org", $domainHintElements[0]->textContent); + $this->assertEquals("student.example.org", $domainHintElements[1]->textContent); + + $geoHintElements = Utils::xpQuery( + $discoElement, + './*[local-name()=\'GeolocationHint\' and namespace-uri()=\'urn:oasis:names:tc:SAML:metadata:ui\']' + ); + $this->assertCount(2, $geoHintElements); + $this->assertEquals("geo:47.37328,8.531126", $geoHintElements[0]->textContent); + $this->assertEquals("geo:19.34343,12.342514", $geoHintElements[1]->textContent); + } + + /** + * Create an empty discoHints element + */ + public function testMarshallingEmpty() + { + $discoHints = new DiscoHints(); + + $document = DOMDocumentFactory::fromString(''); + $xml = $discoHints->toXML($document->firstChild); + + $this->assertNull($xml); + } + + public function testUnmarshalling() + { + $document = DOMDocumentFactory::fromString(<< + 130.59.0.0/16 + 2001:620::0/96 + example.com + www.example.com + geo:47.37328,8.531126 + geo:19.34343,12.342514 + +XML + ); + + $disco = new DiscoHints($document->firstChild); + + $this->assertCount(2, $disco->IPHint); + $this->assertEquals('130.59.0.0/16', $disco->IPHint[0]); + $this->assertEquals('2001:620::0/96', $disco->IPHint[1]); + $this->assertCount(2, $disco->DomainHint); + $this->assertEquals('example.com', $disco->DomainHint[0]); + $this->assertEquals('www.example.com', $disco->DomainHint[1]); + $this->assertCount(2, $disco->GeolocationHint); + $this->assertEquals('geo:47.37328,8.531126', $disco->GeolocationHint[0]); + $this->assertEquals('geo:19.34343,12.342514', $disco->GeolocationHint[1]); + } +} From abac7d843b8285a95b42719aa5d84a3d83eb7b63 Mon Sep 17 00:00:00 2001 From: Thijs Kinkhorst Date: Wed, 9 Mar 2016 15:30:14 +0000 Subject: [PATCH 10/30] split large tests in more managable units --- tests/SAML2/XML/shibmd/ScopeTest.php | 39 ++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/tests/SAML2/XML/shibmd/ScopeTest.php b/tests/SAML2/XML/shibmd/ScopeTest.php index deb562446..92e2d4205 100644 --- a/tests/SAML2/XML/shibmd/ScopeTest.php +++ b/tests/SAML2/XML/shibmd/ScopeTest.php @@ -10,9 +10,11 @@ */ class ScopeTest extends \PHPUnit_Framework_TestCase { - public function testMarshalling() + /** + * Marshalling a scope in literal (non-regexp) form. + */ + public function testMarshallingLiteral() { - // In literal, non-regexp form $scope = new Scope(); $scope->scope = "example.org"; $scope->regexp = FALSE; @@ -27,8 +29,14 @@ public function testMarshalling() $this->assertEquals('example.org', $scopeElement->nodeValue); $this->assertEquals('urn:mace:shibboleth:metadata:1.0', $scopeElement->namespaceURI); $this->assertEquals('false', $scopeElement->getAttribute('regexp')); + } - // Without explicit regexp: should yield explicit 'false' + /** + * Marshalling a scope which does not specificy the value for + * regexp explicitly (expect it to default to 'false'). + */ + public function testMarshallingImplicitRegexpValue() + { $scope = new Scope(); $scope->scope = "example.org"; @@ -42,8 +50,13 @@ public function testMarshalling() $this->assertEquals('example.org', $scopeElement->nodeValue); $this->assertEquals('urn:mace:shibboleth:metadata:1.0', $scopeElement->namespaceURI); $this->assertEquals('false', $scopeElement->getAttribute('regexp')); + } - // In regexp form + /** + * Marshalling a scope which is in regexp form. + */ + public function testMarshallingRegexp() + { $scope = new Scope(); $scope->scope = "^(.*\.)?example\.edu$"; $scope->regexp = TRUE; @@ -60,7 +73,10 @@ public function testMarshalling() $this->assertEquals('true', $scopeElement->getAttribute('regexp')); } - public function testUnmarshalling() + /** + * Unmarshalling a scope in literal (non-regexp) form. + */ + public function testUnmarshallingLiteral() { $document = DOMDocumentFactory::fromString( <<assertEquals('example.org', $scope->scope); $this->assertFalse($scope->regexp); + } + /** + * Unmarshalling a scope that does not specify an explicit + * regexp value (assumed to be false). + */ + public function testUnmarshallingWithoutRegexpValue() + { $document = DOMDocumentFactory::fromString( <<example.org @@ -81,7 +104,13 @@ public function testUnmarshalling() $this->assertEquals('example.org', $scope->scope); $this->assertFalse($scope->regexp); + } + /** + * Unmarshalling a scope in regexp form. + */ + public function testUnmarshallingRegexp() + { $document = DOMDocumentFactory::fromString( <<^(.*|)example.edu$ From 1da26258155e4c6c5edacfe8ecbecfd46b755c3c Mon Sep 17 00:00:00 2001 From: Thijs Kinkhorst Date: Fri, 11 Mar 2016 15:45:28 +0000 Subject: [PATCH 11/30] Add tests for signed assertion verification --- tests/SAML2/AssertionTest.php | 92 ++++++++++++++++++++++++ tests/SAML2/CertificatesMock.php | 11 +++ tests/SAML2/signedassertion.xml | 5 ++ tests/SAML2/signedassertion_tampered.xml | 5 ++ 4 files changed, 113 insertions(+) create mode 100644 tests/SAML2/signedassertion.xml create mode 100644 tests/SAML2/signedassertion_tampered.xml diff --git a/tests/SAML2/AssertionTest.php b/tests/SAML2/AssertionTest.php index fb015b23a..35d7840a5 100644 --- a/tests/SAML2/AssertionTest.php +++ b/tests/SAML2/AssertionTest.php @@ -3,6 +3,7 @@ namespace SAML2; use SAML2\XML\Chunk; +use RobRichards\XMLSecLibs\XMLSecurityKey; /** * Class \SAML2\AssertionTest @@ -551,4 +552,95 @@ public function testTypedEncryptedAttributeValuesAreParsedCorrectly() $this->assertInternalType('string', $attributes['urn:some:string'][0]); $this->assertXmlStringEqualsXmlString($xml, $assertionToVerify->toXML()->ownerDocument->saveXML()); } + + /** + * Try to verify a signed assertion. + */ + public function testVerifySignedAssertion() + { + $doc = new \DOMDocument(); + $doc->load(__DIR__ . '/signedassertion.xml'); + + $publicKey = CertificatesMock::getPublicKey(); + + $assertion = new Assertion($doc->firstChild); + $result = $assertion->validate($publicKey); + + $this->assertTrue($result); + // Double-check that we can actually retrieve some basics. + $this->assertEquals("_d908a49b8b63665738430d1c5b655f297b91331864", $assertion->getId()); + $this->assertEquals("https://thki-sid.pt-48.utr.surfcloud.nl/ssp/saml2/idp/metadata.php", $assertion->getIssuer()); + $this->assertEquals("1457707995", $assertion->getIssueInstant()); + + $certs = $assertion->getCertificates(); + $this->assertCount(1, $certs); + $this->assertEquals(CertificatesMock::getPlainPublicKeyContents(), $certs[0]); + } + + /** + * Try to verify a signed assertion in which a byte was changed after signing. + * Must yield a validation exception. + */ + public function testVerifySignedAssertionChangedBody() + { + $doc = new \DOMDocument(); + $doc->load(__DIR__ . '/signedassertion_tampered.xml'); + + $publicKey = CertificatesMock::getPublicKey(); + + $this->setExpectedException('Exception', 'Reference validation failed'); + $assertion = new Assertion($doc->firstChild); + } + + /** + * Try to verify a signed assertion with the wrong key. + * Must yield a signature validation exception. + */ + public function testVerifySignedAssertionWrongKey() + { + $doc = new \DOMDocument(); + $doc->load(__DIR__ . '/signedassertion.xml'); + + $publicKey = CertificatesMock::getPublicKey2(); + + $assertion = new Assertion($doc->firstChild); + $this->setExpectedException('Exception', 'Unable to validate Signature'); + $assertion->validate($publicKey); + } + + /** + * Calling validate on an unsigned assertion must return + * false, not an exception. + */ + public function testVerifyUnsignedAssertion() + { + $xml = << + testIssuer + + + audience1 + audience2 + + + + + someAuthnContext + someIdP1 + someIdP2 + + + +XML; + $document = DOMDocumentFactory::fromString($xml); + $assertion = new Assertion($document->firstChild); + + $publicKey = CertificatesMock::getPublicKey(); + $result = $assertion->validate($publicKey); + $this->assertFalse($result); + } } diff --git a/tests/SAML2/CertificatesMock.php b/tests/SAML2/CertificatesMock.php index 276568037..9651e5c82 100644 --- a/tests/SAML2/CertificatesMock.php +++ b/tests/SAML2/CertificatesMock.php @@ -91,6 +91,17 @@ public static function getPrivateKey() return $privateKey; } + /** + * @return XMLSecurityKey + */ + public static function getPublicKey2() + { + $publicKey = new XMLSecurityKey(XMLSecurityKey::RSA_1_5, array('type'=>'public')); + $publicKey->loadKey(self::PUBLIC_KEY_2_PEM); + return $publicKey; + } + + /** * @return XMLSecurityKey */ diff --git a/tests/SAML2/signedassertion.xml b/tests/SAML2/signedassertion.xml new file mode 100644 index 000000000..76a4348cb --- /dev/null +++ b/tests/SAML2/signedassertion.xml @@ -0,0 +1,5 @@ +https://thki-sid.pt-48.utr.surfcloud.nl/ssp/saml2/idp/metadata.php + + + 3T1G7tVq5t3vYQEHerp8sRWakxs=pdcWOeAtYOnCAXSt/bTwtFHRM1Qk23NI85NMDXwUPpwC8RJ939fmleb3OL12LyO1lkOq7a8JRV/Qmav5CPapYMVMkNpN8Brz731rBcP9cPzcuXumP7h4m7HjxuAo4C+V6q6l981cuf/THsrX2PfG/+SFwwYzwAECb7JSHYFnoRc= +MIICgTCCAeoCCQCbOlrWDdX7FTANBgkqhkiG9w0BAQUFADCBhDELMAkGA1UEBhMCTk8xGDAWBgNVBAgTD0FuZHJlYXMgU29sYmVyZzEMMAoGA1UEBxMDRm9vMRAwDgYDVQQKEwdVTklORVRUMRgwFgYDVQQDEw9mZWlkZS5lcmxhbmcubm8xITAfBgkqhkiG9w0BCQEWEmFuZHJlYXNAdW5pbmV0dC5ubzAeFw0wNzA2MTUxMjAxMzVaFw0wNzA4MTQxMjAxMzVaMIGEMQswCQYDVQQGEwJOTzEYMBYGA1UECBMPQW5kcmVhcyBTb2xiZXJnMQwwCgYDVQQHEwNGb28xEDAOBgNVBAoTB1VOSU5FVFQxGDAWBgNVBAMTD2ZlaWRlLmVybGFuZy5ubzEhMB8GCSqGSIb3DQEJARYSYW5kcmVhc0B1bmluZXR0Lm5vMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDivbhR7P516x/S3BqKxupQe0LONoliupiBOesCO3SHbDrl3+q9IbfnfmE04rNuMcPsIxB161TdDpIesLCn7c8aPHISKOtPlAeTZSnb8QAu7aRjZq3+PbrP5uW3TcfCGPtKTytHOge/OlJbo078dVhXQ14d1EDwXJW1rRXuUt4C8QIDAQABMA0GCSqGSIb3DQEBBQUAA4GBACDVfp86HObqY+e8BUoWQ9+VMQx1ASDohBjwOsg2WykUqRXF+dLfcUH9dWR63CtZIKFDbStNomPnQz7nbK+onygwBspVEbnHuUihZq3ZUdmumQqCw4Uvs/1Uvq3orOo/WJVhTyvLgFVK2QarQ4/67OZfHd7R+POBXhophSMv1ZOo_1bbcf227253269d19a689c53cdd542fe2384a9538bhttps://engine.test.surfconext.nl/authentication/sp/metadataurn:oasis:names:tc:SAML:2.0:ac:classes:Passwordstudent2university.example.orgbbb.ccurn:schac:personalUniqueCode:nl:local:uvt.nl:memberid:524020urn:schac:personalUniqueCode:nl:local:surfnet.nl:studentid:12345memberstudent diff --git a/tests/SAML2/signedassertion_tampered.xml b/tests/SAML2/signedassertion_tampered.xml new file mode 100644 index 000000000..5e57b0591 --- /dev/null +++ b/tests/SAML2/signedassertion_tampered.xml @@ -0,0 +1,5 @@ +https://thki-sid.pt-48.utr.surfcloud.nl/ssp/saml2/idp/metadata.php + + + 3T1G7tVq5t3vYQEHerp8sRWakxs=pdcWOeAtYOnCAXSt/bTwtFHRM1Qk23NI85NMDXwUPpwC8RJ939fmleb3OL12LyO1lkOq7a8JRV/Qmav5CPapYMVMkNpN8Brz731rBcP9cPzcuXumP7h4m7HjxuAo4C+V6q6l981cuf/THsrX2PfG/+SFwwYzwAECb7JSHYFnoRc= +MIICgTCCAeoCCQCbOlrWDdX7FTANBgkqhkiG9w0BAQUFADCBhDELMAkGA1UEBhMCTk8xGDAWBgNVBAgTD0FuZHJlYXMgU29sYmVyZzEMMAoGA1UEBxMDRm9vMRAwDgYDVQQKEwdVTklORVRUMRgwFgYDVQQDEw9mZWlkZS5lcmxhbmcubm8xITAfBgkqhkiG9w0BCQEWEmFuZHJlYXNAdW5pbmV0dC5ubzAeFw0wNzA2MTUxMjAxMzVaFw0wNzA4MTQxMjAxMzVaMIGEMQswCQYDVQQGEwJOTzEYMBYGA1UECBMPQW5kcmVhcyBTb2xiZXJnMQwwCgYDVQQHEwNGb28xEDAOBgNVBAoTB1VOSU5FVFQxGDAWBgNVBAMTD2ZlaWRlLmVybGFuZy5ubzEhMB8GCSqGSIb3DQEJARYSYW5kcmVhc0B1bmluZXR0Lm5vMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDivbhR7P516x/S3BqKxupQe0LONoliupiBOesCO3SHbDrl3+q9IbfnfmE04rNuMcPsIxB161TdDpIesLCn7c8aPHISKOtPlAeTZSnb8QAu7aRjZq3+PbrP5uW3TcfCGPtKTytHOge/OlJbo078dVhXQ14d1EDwXJW1rRXuUt4C8QIDAQABMA0GCSqGSIb3DQEBBQUAA4GBACDVfp86HObqY+e8BUoWQ9+VMQx1ASDohBjwOsg2WykUqRXF+dLfcUH9dWR63CtZIKFDbStNomPnQz7nbK+onygwBspVEbnHuUihZq3ZUdmumQqCw4Uvs/1Uvq3orOo/WJVhTyvLgFVK2QarQ4/67OZfHd7R+POBXhophSMv1ZOo_1bbcf227253269d19a689c53cdd542fe2384a9538bhttps://engine.test.surfconext.nl/authentication/sp/metadataurn:oasis:names:tc:SAML:2.0:ac:classes:Passwordstudent3university.example.orgbbb.ccurn:schac:personalUniqueCode:nl:local:uvt.nl:memberid:524020urn:schac:personalUniqueCode:nl:local:surfnet.nl:studentid:12345memberstudent From aed2223e6ea8abc5ec470feb5aac40cecd7fb011 Mon Sep 17 00:00:00 2001 From: Thijs Kinkhorst Date: Fri, 11 Mar 2016 15:51:01 +0000 Subject: [PATCH 12/30] Remove spurious 'use' --- tests/SAML2/AssertionTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/SAML2/AssertionTest.php b/tests/SAML2/AssertionTest.php index 35d7840a5..3782091ac 100644 --- a/tests/SAML2/AssertionTest.php +++ b/tests/SAML2/AssertionTest.php @@ -3,7 +3,6 @@ namespace SAML2; use SAML2\XML\Chunk; -use RobRichards\XMLSecLibs\XMLSecurityKey; /** * Class \SAML2\AssertionTest From bc7740993d760056e760ab63132d627f93003154 Mon Sep 17 00:00:00 2001 From: Thijs Kinkhorst Date: Fri, 11 Mar 2016 15:59:24 +0000 Subject: [PATCH 13/30] Revert "Remove spurious 'use'" This reverts commit aed2223e6ea8abc5ec470feb5aac40cecd7fb011. --- tests/SAML2/AssertionTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/SAML2/AssertionTest.php b/tests/SAML2/AssertionTest.php index 3782091ac..35d7840a5 100644 --- a/tests/SAML2/AssertionTest.php +++ b/tests/SAML2/AssertionTest.php @@ -3,6 +3,7 @@ namespace SAML2; use SAML2\XML\Chunk; +use RobRichards\XMLSecLibs\XMLSecurityKey; /** * Class \SAML2\AssertionTest From 952e166e75babd12f5e706368407ed38adde8b11 Mon Sep 17 00:00:00 2001 From: Thijs Kinkhorst Date: Mon, 14 Mar 2016 12:16:18 +0000 Subject: [PATCH 14/30] Fix namespace in assertion test. Also update tests to use expected key format. Fixes #62 --- src/SAML2/Assertion.php | 2 +- tests/SAML2/AssertionTest.php | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/SAML2/Assertion.php b/src/SAML2/Assertion.php index 74a3c3f60..8dd1e5e1e 100644 --- a/src/SAML2/Assertion.php +++ b/src/SAML2/Assertion.php @@ -582,7 +582,7 @@ private function parseSignature(\DOMElement $xml) */ public function validate(XMLSecurityKey $key) { - assert('$key->type === XMLSecurityKey::RSA_SHA1'); + assert('$key->type === \RobRichards\XMLSecLibs\XMLSecurityKey::RSA_SHA1'); if ($this->signatureData === null) { return false; diff --git a/tests/SAML2/AssertionTest.php b/tests/SAML2/AssertionTest.php index 35d7840a5..fe04e6b20 100644 --- a/tests/SAML2/AssertionTest.php +++ b/tests/SAML2/AssertionTest.php @@ -3,7 +3,6 @@ namespace SAML2; use SAML2\XML\Chunk; -use RobRichards\XMLSecLibs\XMLSecurityKey; /** * Class \SAML2\AssertionTest @@ -561,7 +560,7 @@ public function testVerifySignedAssertion() $doc = new \DOMDocument(); $doc->load(__DIR__ . '/signedassertion.xml'); - $publicKey = CertificatesMock::getPublicKey(); + $publicKey = CertificatesMock::getPublicKeySha1(); $assertion = new Assertion($doc->firstChild); $result = $assertion->validate($publicKey); @@ -586,7 +585,7 @@ public function testVerifySignedAssertionChangedBody() $doc = new \DOMDocument(); $doc->load(__DIR__ . '/signedassertion_tampered.xml'); - $publicKey = CertificatesMock::getPublicKey(); + $publicKey = CertificatesMock::getPublicKeySha1(); $this->setExpectedException('Exception', 'Reference validation failed'); $assertion = new Assertion($doc->firstChild); @@ -601,7 +600,7 @@ public function testVerifySignedAssertionWrongKey() $doc = new \DOMDocument(); $doc->load(__DIR__ . '/signedassertion.xml'); - $publicKey = CertificatesMock::getPublicKey2(); + $publicKey = CertificatesMock::getPublicKey2Sha1(); $assertion = new Assertion($doc->firstChild); $this->setExpectedException('Exception', 'Unable to validate Signature'); @@ -639,7 +638,7 @@ public function testVerifyUnsignedAssertion() $document = DOMDocumentFactory::fromString($xml); $assertion = new Assertion($document->firstChild); - $publicKey = CertificatesMock::getPublicKey(); + $publicKey = CertificatesMock::getPublicKeySha1(); $result = $assertion->validate($publicKey); $this->assertFalse($result); } From 6ab32d5582bb08fcd1984ed06e98e012cd2a4e2f Mon Sep 17 00:00:00 2001 From: Thijs Kinkhorst Date: Mon, 14 Mar 2016 17:12:53 +0100 Subject: [PATCH 15/30] Test for a couple of error conditions --- tests/SAML2/AssertionTest.php | 106 +++++++++++++++++++++++++++++ tests/SAML2/StatusResponseTest.php | 33 ++++++++- 2 files changed, 138 insertions(+), 1 deletion(-) diff --git a/tests/SAML2/AssertionTest.php b/tests/SAML2/AssertionTest.php index fe04e6b20..b2ecf4d9b 100644 --- a/tests/SAML2/AssertionTest.php +++ b/tests/SAML2/AssertionTest.php @@ -9,6 +9,9 @@ */ class AssertionTest extends \PHPUnit_Framework_TestCase { + /** + * Test to build a basic assertion + */ public function testMarshalling() { // Create an assertion @@ -43,6 +46,9 @@ public function testMarshalling() $this->assertEquals('someAuthnContext', $authnContextElements[0]->textContent); } + /** + * Test to parse a basic assertion + */ public function testUnmarshalling() { // Unmarshall an assertion @@ -84,6 +90,9 @@ public function testUnmarshalling() $this->assertEquals('someIdP2', $assertionAuthenticatingAuthorities[1]); } + /** + * Test parsing AuthnContext elements Decl and ClassRef + */ public function testAuthnContextDeclAndClassRef() { $xml = <<assertEquals('someAuthnContext', $assertion->getAuthnContextClassRef()); } + /** + * Test parsing AuthnContext elements DeclRef and ClassRef + */ public function testAuthnContextDeclRefAndClassRef() { // Try with unmarshalling @@ -642,4 +654,98 @@ public function testVerifyUnsignedAssertion() $result = $assertion->validate($publicKey); $this->assertFalse($result); } + + /** + * An assertion must always be version "2.0". + */ + public function testAssertionVersionOtherThan20ThrowsException() + { + $xml = << + testIssuer + + + audience1 + audience2 + + + + + someAuthnContext + someIdP1 + someIdP2 + + + +XML; + $document = DOMDocumentFactory::fromString($xml); + $this->setExpectedException('Exception', 'Unsupported version: 1.3'); + $assertion = new Assertion($document->firstChild); + } + + /** + * An assertion without an ID must throw an exception + */ + public function testAssertionWithoutIDthrowsException() + { + $xml = << + testIssuer + + + audience1 + audience2 + + + + + someAuthnContext + someIdP1 + someIdP2 + + + +XML; + $document = DOMDocumentFactory::fromString($xml); + $this->setExpectedException('Exception', 'Missing ID attribute on SAML assertion'); + $assertion = new Assertion($document->firstChild); + } + + /** + * An assertion must always have an Issuer element. + */ + public function testAssertionWithoutIssuerThrowsException() + { + $xml = << + + + audience1 + audience2 + + + + + someAuthnContext + someIdP1 + someIdP2 + + + +XML; + $document = DOMDocumentFactory::fromString($xml); + $this->setExpectedException('Exception', 'Missing in assertion'); + $assertion = new Assertion($document->firstChild); + } } diff --git a/tests/SAML2/StatusResponseTest.php b/tests/SAML2/StatusResponseTest.php index 43761828f..fd3389303 100644 --- a/tests/SAML2/StatusResponseTest.php +++ b/tests/SAML2/StatusResponseTest.php @@ -162,12 +162,43 @@ public function testResponseTo() $this->assertEqualXMLStructure($expectedStructure, $responseElement); } + /** + * A response without any element throws exception + */ + public function testNoStatusElementThrowsException() + { + $this->setExpectedException('Exception', 'Missing status code on response'); + + $xml = << + max.example.org + + max.example.org + + +XML; + + $fixtureResponseDom = DOMDocumentFactory::fromString($xml); + $response = new Response($fixtureResponseDom->firstChild); + } + /** * StatusCode is required in a StatusResponse. */ public function testNoStatusCodeThrowsException() { - $this->setExpectedException('Exception', 'Missing status code'); + $this->setExpectedException('Exception', 'Missing status code in status element'); $xml = << Date: Mon, 14 Mar 2016 17:13:45 +0100 Subject: [PATCH 16/30] Fix typo in exception message --- 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 8dd1e5e1e..77f746080 100644 --- a/src/SAML2/Assertion.php +++ b/src/SAML2/Assertion.php @@ -388,7 +388,7 @@ private function parseAuthnStatement(\DOMElement $xml) return; } elseif (count($authnStatements) > 1) { - throw new \Exception('More that one in not supported.'); + throw new \Exception('More than one in not supported.'); } $authnStatement = $authnStatements[0]; From 8a83fc81aa74dd0ed085e397ffaf1b676994dc42 Mon Sep 17 00:00:00 2001 From: Jaime Perez Crespo Date: Wed, 16 Mar 2016 15:01:30 +0100 Subject: [PATCH 17/30] Allow the NameID / EncryptedID to be missing from a SAML subject. --- src/SAML2/Assertion.php | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/SAML2/Assertion.php b/src/SAML2/Assertion.php index 77f746080..1d7618fcb 100644 --- a/src/SAML2/Assertion.php +++ b/src/SAML2/Assertion.php @@ -287,21 +287,20 @@ private function parseSubject(\DOMElement $xml) $subject, './saml_assertion:NameID | ./saml_assertion:EncryptedID/xenc:EncryptedData' ); - if (empty($nameId)) { - throw new \Exception('Missing or in .'); - } elseif (count($nameId) > 1) { - throw new \Exception('More than one or in .'); - } - $nameId = $nameId[0]; - if ($nameId->localName === 'EncryptedData') { - /* The NameID element is encrypted. */ - $this->encryptedNameId = $nameId; - } else { - $this->nameId = Utils::parseNameId($nameId); + if (count($nameId) > 1) { + throw new \Exception('More than one or in .'); + } elseif (!empty($nameId)) { + $nameId = $nameId[0]; + if ($nameId->localName === 'EncryptedData') { + /* The NameID element is encrypted. */ + $this->encryptedNameId = $nameId; + } else { + $this->nameId = Utils::parseNameId($nameId); + } } $subjectConfirmation = Utils::xpQuery($subject, './saml_assertion:SubjectConfirmation'); - if (empty($subjectConfirmation)) { + if (empty($subjectConfirmation) && empty($nameId)) { throw new \Exception('Missing in .'); } From 85e1b97ff3e879a289c45c0ea92274990dd16bca Mon Sep 17 00:00:00 2001 From: Thijs Kinkhorst Date: Thu, 17 Mar 2016 06:56:59 -0700 Subject: [PATCH 18/30] More tests for Assertion --- tests/SAML2/AssertionTest.php | 728 ++++++++++++++++++++++++++++++++++ 1 file changed, 728 insertions(+) diff --git a/tests/SAML2/AssertionTest.php b/tests/SAML2/AssertionTest.php index b2ecf4d9b..d3f85c564 100644 --- a/tests/SAML2/AssertionTest.php +++ b/tests/SAML2/AssertionTest.php @@ -77,6 +77,9 @@ public function testUnmarshalling() $document = DOMDocumentFactory::fromString($xml); $assertion = new Assertion($document->firstChild); + // Was not signed + $this->assertFalse($assertion->getWasSignedAtConstruction()); + // Test for valid audiences $assertionValidAudiences = $assertion->getValidAudiences(); $this->assertCount(2, $assertionValidAudiences); @@ -90,6 +93,69 @@ public function testUnmarshalling() $this->assertEquals('someIdP2', $assertionAuthenticatingAuthorities[1]); } + /** + * Test an assertion with lots of options + */ + public function testMarshallingUnmarshallingChristmas() + { + // Create an assertion + $assertion = new Assertion(); + + $assertion->setIssuer('testIssuer'); + $assertion->setValidAudiences(array('audience1', 'audience2')); + + // deprecated function + $this->assertNull($assertion->getAuthnContext()); + + $assertion->setAuthnContext('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(array("idp1","idp2")); + + $assertion->setAttributes(array( + "name1" => array("value1","value2"), + "name2" => array(2), + "name3" => array(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); + + $this->assertEquals('/relative/path/to/document.xml', $assertionToVerify->getAuthnContextDeclRef()); + $this->assertEquals('_123abc', $assertionToVerify->getId()); + $this->assertEquals(1234567890, $assertionToVerify->getIssueInstant()); + $this->assertEquals(1234567889, $assertionToVerify->getAuthnInstant()); + $this->assertEquals(1234567880, $assertionToVerify->getNotBefore()); + $this->assertEquals(1234567990, $assertionToVerify->getNotOnOrAfter()); + $this->assertEquals(1234569090, $assertionToVerify->getSessionNotOnOrAfter()); + + $this->assertEquals('idx1', $assertionToVerify->getSessionIndex()); + + $authauth = $assertionToVerify->getAuthenticatingAuthority(); + $this->assertCount(2, $authauth); + $this->assertEquals("idp2", $authauth[1]); + + $attributes = $assertionToVerify->getAttributes(); + $this->assertCount(3, $attributes); + $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->assertEquals("urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified", $assertionToVerify->getAttributeNameFormat()); + } + /** * Test parsing AuthnContext elements Decl and ClassRef */ @@ -126,6 +192,9 @@ public function testAuthnContextDeclAndClassRef() $this->assertEquals('AuthenticationContextDeclaration', $childLocalName); $this->assertEquals('someAuthnContext', $assertion->getAuthnContextClassRef()); + + // deprecated function + $this->assertEquals('someAuthnContext', $assertion->getAuthnContext()); } /** @@ -158,6 +227,30 @@ public function testAuthnContextDeclRefAndClassRef() $this->assertEquals('someAuthnContext', $assertion->getAuthnContextClassRef()); } + /** + * Test setting an AuthnContextDecl chunk. + */ + public function testSetAuthnContextDecl() + { + $xml = << + +XML; + + $document = DOMDocumentFactory::fromString($xml); + $assertion = new Assertion(); + + $assertion->setAuthnContextDecl(new Chunk($document->documentElement)); + $documentParent = DOMDocumentFactory::fromString(""); + $assertionElement = $assertion->toXML($documentParent->firstChild); + + $acElements = Utils::xpQuery($assertionElement, './saml_assertion:AuthnStatement/saml_assertion:AuthnContext'); + $this->assertCount(1, $acElements); + $this->assertEquals('samlac:AuthenticationContextDeclaration', $acElements[0]->firstChild->tagName); + $this->assertEquals('urn:oasis:names:tc:SAML:2.0:ac', $acElements[0]->firstChild->namespaceURI); + } + + public function testAuthnContextDeclAndRefConstraint() { $xml = <<assertNotEmpty($e); + + // deprecated function + $this->assertEquals('/relative/path/to/document.xml', $assertion->getAuthnContext()); } public function testMustHaveClassRefOrDeclOrDeclRef() @@ -388,12 +484,17 @@ public function testCorrectSignatureMethodCanBeExtracted() $unsignedAssertion = new Assertion($document->firstChild); $unsignedAssertion->setSignatureKey($privateKey); $unsignedAssertion->setCertificates(array(CertificatesMock::PUBLIC_KEY_PEM)); + $this->assertFalse($unsignedAssertion->getWasSignedAtConstruction()); + $this->assertEquals($privateKey, $unsignedAssertion->getSignatureKey()); $signedAssertion = new Assertion($unsignedAssertion->toXML()); $signatureMethod = $signedAssertion->getSignatureMethod(); $this->assertEquals($privateKey->getAlgorith(), $signatureMethod); + + $this->assertTrue($signedAssertion->getWasSignedAtConstruction()); + } public function testAttributeValuesWithComplexTypeValuesAreParsedCorrectly() @@ -505,6 +606,8 @@ public function testEncryptedAttributeValuesWithComplexTypeValuesAreParsedCorrec $assertion = new Assertion(DOMDocumentFactory::fromString($xml)->firstChild); $assertion->setEncryptionKey($privateKey); $assertion->setEncryptedAttributes(true); + $this->assertEquals($privateKey, $assertion->getEncryptionKey()); + $encryptedAssertion = $assertion->toXML()->ownerDocument->saveXML(); $assertionToVerify = new Assertion(DOMDocumentFactory::fromString($encryptedAssertion)->firstChild); @@ -586,6 +689,9 @@ public function testVerifySignedAssertion() $certs = $assertion->getCertificates(); $this->assertCount(1, $certs); $this->assertEquals(CertificatesMock::getPlainPublicKeyContents(), $certs[0]); + + // Was signed + $this->assertTrue($assertion->getWasSignedAtConstruction()); } /** @@ -650,6 +756,9 @@ public function testVerifyUnsignedAssertion() $document = DOMDocumentFactory::fromString($xml); $assertion = new Assertion($document->firstChild); + // Was not signed + $this->assertFalse($assertion->getWasSignedAtConstruction()); + $publicKey = CertificatesMock::getPublicKeySha1(); $result = $assertion->validate($publicKey); $this->assertFalse($result); @@ -748,4 +857,623 @@ public function testAssertionWithoutIssuerThrowsException() $this->setExpectedException('Exception', 'Missing in assertion'); $assertion = new Assertion($document->firstChild); } + + /** + * More than one is not allowed in an Assertion. + */ + public function testMoreThanOneSubjectThrowsException() + { + $xml = << + testIssuer + + 5 + + + + someAuthnContext + /relative/path/to/document.xml + + + + aap + + +XML; + + $document = DOMDocumentFactory::fromString($xml); + + $this->setExpectedException('Exception', 'More than one in '); + $assertion = new Assertion($document->documentElement); + } + + /** + * No more than one NameID may be present in the Subject + */ + public function testMoreThanOneNameIDThrowsException() + { + $xml = << + testIssuer + + 5 + 6 + + + + someAuthnContext + /relative/path/to/document.xml + + + +XML; + + $document = DOMDocumentFactory::fromString($xml); + + $this->setExpectedException('Exception', 'More than one or in '); + $assertion = new Assertion($document->documentElement); + } + + /** + * A wtthout both NameID and SubjectConfirmation throws exception. + */ + public function testSubjectMustHaveNameIDorSubjectConfirmation() + { + $xml = << + testIssuer + + not a nameid or subject confirmation + + + + someAuthnContext + /relative/path/to/document.xml + + + +XML; + + $document = DOMDocumentFactory::fromString($xml); + + $this->setExpectedException('Exception', 'Missing in '); + $assertion = new Assertion($document->documentElement); + } + + /** + * An Assertion may not have more than one + */ + public function testTooManyConditionsThrowsException() + { + $xml = << + testIssuer + + + audience1 + audience2 + + + + + someAuthnContext + someIdP1 + someIdP2 + + + + not allowed + + +XML; + $document = DOMDocumentFactory::fromString($xml); + + $this->setExpectedException('Exception', 'More than one in '); + $assertion = new Assertion($document->firstChild); + } + + /** + * A Condition must be of namespace saml. + */ + public function testConditionWithUnknownNamespaceThrowsException() + { + $xml = << + testIssuer + + + audience1 + + this is not allowed + + + + someAuthnContext + someIdP1 + someIdP2 + + + +XML; + $document = DOMDocumentFactory::fromString($xml); + + $this->setExpectedException('Exception', 'Unknown namespace of condition:'); + $assertion = new Assertion($document->firstChild); + } + + /** + * Test various types of allowed Conditions. + * - AudienceRestriction: are ANDed together so should only be audience1 + * - OneTimeUse and ProxyRestrictions must be accepted but are + * currently a no-op. + */ + public function testConditionAllowedTypes() + { + $xml = << + testIssuer + + + audience1 + + + audience2 + audience1 + + + + + + + + + someAuthnContext + someIdP1 + someIdP2 + + + +XML; + $document = DOMDocumentFactory::fromString($xml); + + $assertion = new Assertion($document->firstChild); + + $audienceRestrictions = $assertion->getValidAudiences(); + $this->assertCount(1, $audienceRestrictions); + $this->assertEquals('audience1', $audienceRestrictions[0]); + } + + /** + * Any Condition other than AudienceRestirction, OneTimeUse and + * ProxyRestriction must throw an Exception. + */ + public function testUnkownThrowsException() + { + $xml = << + testIssuer + + + audience1 + + this is not allowed + + + + someAuthnContext + someIdP1 + someIdP2 + + + +XML; + $document = DOMDocumentFactory::fromString($xml); + + $this->setExpectedException('Exception', "Unknown condition: 'OtherCondition'"); + $assertion = new Assertion($document->firstChild); + } + + /** + * More than one AuthnStatement will throw Exception. + */ + public function testMoreThanOneAuthnStatementThrowsException() + { + $xml = << + testIssuer + + + someAuthnContext + someIdP1 + + + + + someAuthnContext + someIdP2 + + + +XML; + $document = DOMDocumentFactory::fromString($xml); + + $this->setExpectedException('Exception', "More than one in not supported"); + $assertion = new Assertion($document->firstChild); + } + + /** + * AuthnStatement must have AuthnInstant attribute, if missing + * throw Exception. + */ + public function testMissingAuthnInstantThrowsException() + { + $xml = << + testIssuer + + + someAuthnContext + someIdP1 + + + +XML; + $document = DOMDocumentFactory::fromString($xml); + + $this->setExpectedException('Exception', "Missing required AuthnInstant attribute on "); + $assertion = new Assertion($document->firstChild); + } + + /** + * More than one AuthnContext inside AuthnStatement will throw Exception. + */ + public function testMoreThanOneAuthnContextThrowsException() + { + $xml = << + testIssuer + + + someAuthnContext + someIdP1 + + + someAuthnContext + someIdP2 + + + +XML; + $document = DOMDocumentFactory::fromString($xml); + + $this->setExpectedException('Exception', "More than one in "); + $assertion = new Assertion($document->firstChild); + } + + /** + * No AuthnContext inside AuthnStatement will throw Exception. + */ + public function testMissingAuthnContextThrowsException() + { + $xml = << + testIssuer + + + +XML; + $document = DOMDocumentFactory::fromString($xml); + + $this->setExpectedException('Exception', "Missing required in "); + $assertion = new Assertion($document->firstChild); + } + + /** + * More than one AuthnContextDeclRef inside AuthnContext will throw Exception. + */ + public function testMoreThanOneAuthnContextDeclRefThrowsException() + { + $xml = << + testIssuer + + + /relative/path/to/document1.xml + someAuthnContext + /relative/path/to/document2.xml + + + +XML; + $document = DOMDocumentFactory::fromString($xml); + + $this->setExpectedException('Exception', "More than one found"); + $assertion = new Assertion($document->firstChild); + } + + /** + * More than one AuthnContextDecl inside AuthnContext will throw Exception. + */ + public function testMoreThanOneAuthnContextDeclThrowsException() + { + $xml = << + testIssuer + + + + + + + + + + + + + +XML; + $document = DOMDocumentFactory::fromString($xml); + + $this->setExpectedException('Exception', "More than one found?"); + $assertion = new Assertion($document->firstChild); + } + + /** + * More than one AuthnContextClassRef inside AuthnContext will throw Exception. + */ + public function testMoreThanOneAuthnContextClassRefThrowsException() + { + $xml = << + testIssuer + + + someAuthnContext + /relative/path/to/document.xml + someOtherAuthnContext + + + +XML; + $document = DOMDocumentFactory::fromString($xml); + + $this->setExpectedException('Exception', "More than one in "); + $assertion = new Assertion($document->firstChild); + } + + /** + * When an Attribute element has no name, exception is thrown. + */ + public function testMissingNameOnAttribute() + { + $document = new \DOMDocument(); + $document->loadXML(<< + Provider + + + 1 + + + 1 + + + +XML + ); + + $this->setExpectedException('Exception', "Missing name on element"); + $assertion = new Assertion($document->firstChild); + } + + /** + * If this assertion mixes Attribute NameFormats, the AttributeNameFormat + * of this assertion will be set to unspecified. + */ + public function testMixedAttributeNameFormats() + { + $xml = << + Provider + + + + string + + + string + + + +XML; + + $assertion = new Assertion(DOMDocumentFactory::fromString($xml)->firstChild); + + $namefmt = $assertion->getAttributeNameFormat(); + $this->assertEquals('urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified', $namefmt); + } + + /** + * Test basic NameID unmarshalling. + */ + public function testNameIDunmarshalling() + { + $xml = << + testIssuer + + b7de81420a19416 + + + + someAuthnContext + /relative/path/to/document.xml + + + +XML; + + $document = DOMDocumentFactory::fromString($xml); + + $assertion = new Assertion($document->documentElement); + + $nameID = $assertion->getNameID(); + $this->assertEquals('b7de81420a19416', $nameID['Value']); + $this->assertEquals('urn:oasis:names:tc:SAML:2.0:nameid-format:transient', $nameID['Format']); + $this->assertFalse($assertion->isNameIdEncrypted()); + + // Not encrypted, should be a no-op + $privateKey = CertificatesMock::getPrivateKey(); + $decrypted = $assertion->decryptNameId($privateKey); + $this->assertEquals('b7de81420a19416', $nameID['Value']); + $this->assertEquals('urn:oasis:names:tc:SAML:2.0:nameid-format:transient', $nameID['Format']); + $this->assertFalse($assertion->isNameIdEncrypted()); + } + + /** + * Test NameID Encryption and Decryption. + */ + public function testNameIdEncryption() + { + // Create an assertion + $assertion = new Assertion(); + $assertion->setIssuer('testIssuer'); + $assertion->setValidAudiences(array('audience1', 'audience2')); + $assertion->setAuthnContext('someAuthnContext'); + + $assertion->setNameId(array( + "Value" => "just_a_basic_identifier", + "Format" => "urn:oasis:names:tc:SAML:2.0:nameid-format:transient")); + $this->assertFalse($assertion->isNameIdEncrypted()); + + $publicKey = CertificatesMock::getPublicKey(); + $assertion->encryptNameId($publicKey); + $this->assertTrue($assertion->isNameIdEncrypted()); + + // Marshall it to a \DOMElement + $assertionElement = $assertion->toXML()->ownerDocument->saveXML(); + + $assertionToVerify = new Assertion(DOMDocumentFactory::fromString($assertionElement)->firstChild); + + $this->assertTrue($assertionToVerify->isNameIdEncrypted()); + $privateKey = CertificatesMock::getPrivateKey(); + $assertionToVerify->decryptNameId($privateKey); + $this->assertFalse($assertionToVerify->isNameIdEncrypted()); + $nameID = $assertionToVerify->getNameID(); + $this->assertEquals('just_a_basic_identifier', $nameID['Value']); + $this->assertEquals('urn:oasis:names:tc:SAML:2.0:nameid-format:transient', $nameID['Format']); + } + + /** + * Test Exception when trying to get encrypted NameId without + * decrypting it first. + */ + public function testRetrieveEncryptedNameIdException() + { + $xml = << + testIssuer + + + + + + + Y78/DDeSkI4qECUPXJM1cWUTaYVglxnqDRpjcqd6zdIR6yWMwIzUCd+fa9KhKutN4kN1i/koSMNmk+c6uOXSi0Xuohth61eU9oIwLl6mKZwThXEQiuphAtMVPXtooKfU1l58+xWiiO2IidYmtb1vCcVD0hZwnVv28kxrMQdQmzw= + + + + cfQoRV0xf+D5bOQs+8icVEkWX4MRNxl1MhImqO/GwYxjCwj0AH/9O4kr2v4WZ4MC3zHhUjcq4HO70/xrkzQVMN9pBsF2yv9sUuN2rEPd8k/Oj/OA3X4xGNywxoJILioh56OyNkFK/q4WRptvvSQV1vPc0G5y65MZBiR2fy+L+ukBJ8mnzxL7aIIEKRxNa0beKdrrZ2twWH3Uwn3UW5LcSefaY+VHcM/9I4Xb7U5QWxRXzBOEa6v/a3cZ/TmlXYkj + + + + + +XML; + $document = DOMDocumentFactory::fromString($xml); + + $assertion = new Assertion($document->documentElement); + $this->setExpectedException('Exception', "Attempted to retrieve encrypted NameID without decrypting it first"); + $assertion->getNameID(); + } } From 75adf101e62f3f6981745f85d06920b171e68a55 Mon Sep 17 00:00:00 2001 From: Thijs Kinkhorst Date: Thu, 17 Mar 2016 08:48:53 -0700 Subject: [PATCH 19/30] Fix documentation --- src/SAML2/XML/mdui/Keywords.php | 4 +++- src/SAML2/XML/mdui/Logo.php | 4 ++-- src/SAML2/XML/mdui/UIInfo.php | 10 +++++----- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/SAML2/XML/mdui/Keywords.php b/src/SAML2/XML/mdui/Keywords.php index 7e4cb3b83..310570051 100644 --- a/src/SAML2/XML/mdui/Keywords.php +++ b/src/SAML2/XML/mdui/Keywords.php @@ -13,7 +13,9 @@ class Keywords /** * The keywords of this item. * - * @var string + * Array of strings. + * + * @var string[] */ public $Keywords; diff --git a/src/SAML2/XML/mdui/Logo.php b/src/SAML2/XML/mdui/Logo.php index e31a5a12d..0f3f9bcc1 100644 --- a/src/SAML2/XML/mdui/Logo.php +++ b/src/SAML2/XML/mdui/Logo.php @@ -20,14 +20,14 @@ class Logo /** * The width of this logo. * - * @var string + * @var int */ public $width; /** * The height of this logo. * - * @var string + * @var int */ public $height; diff --git a/src/SAML2/XML/mdui/UIInfo.php b/src/SAML2/XML/mdui/UIInfo.php index aa52b9d3d..34c50b312 100644 --- a/src/SAML2/XML/mdui/UIInfo.php +++ b/src/SAML2/XML/mdui/UIInfo.php @@ -23,7 +23,7 @@ class UIInfo * * The elements can be any of the other \SAML2\XML\mdui\* elements. * - * @var array + * @var \SAML2\XML\Chunk[] */ public $children = array(); @@ -56,16 +56,16 @@ class UIInfo public $PrivacyStatementURL = array(); /** - * The Keywords, as an array of language => array of strings. + * The Keywords, as an array of Keywords objects * - * @var array + * @var \SAML2\XML\mdui\Keywords[] */ public $Keywords = array(); /** - * The Logo, as an array of associative arrays containing url, width, height, and optional lang. + * The Logo, as an array of Logo objects * - * @var array + * @var \SAML2\XML\mdui\Logo[] */ public $Logo = array(); From d9df067df9fb2f48bbcf0d8e95c9c9a04116a860 Mon Sep 17 00:00:00 2001 From: Thijs Kinkhorst Date: Thu, 17 Mar 2016 08:49:22 -0700 Subject: [PATCH 20/30] Complete tests for XML/mdui/ --- tests/SAML2/XML/mdui/DiscoHintsTest.php | 51 ++++++ tests/SAML2/XML/mdui/KeywordsTest.php | 95 +++++++++++ tests/SAML2/XML/mdui/LogoTest.php | 97 +++++++++++ tests/SAML2/XML/mdui/UIInfoTest.php | 210 ++++++++++++++++++++++++ 4 files changed, 453 insertions(+) create mode 100644 tests/SAML2/XML/mdui/KeywordsTest.php create mode 100644 tests/SAML2/XML/mdui/LogoTest.php create mode 100644 tests/SAML2/XML/mdui/UIInfoTest.php diff --git a/tests/SAML2/XML/mdui/DiscoHintsTest.php b/tests/SAML2/XML/mdui/DiscoHintsTest.php index f8a7b5c12..03f382e6c 100644 --- a/tests/SAML2/XML/mdui/DiscoHintsTest.php +++ b/tests/SAML2/XML/mdui/DiscoHintsTest.php @@ -10,6 +10,9 @@ */ class DiscoHintsTest extends \PHPUnit_Framework_TestCase { + /** + * Test marshalling a basic DiscoHints element + */ public function testMarshalling() { $discoHints = new DiscoHints(); @@ -65,6 +68,9 @@ public function testMarshallingEmpty() $this->assertNull($xml); } + /** + * Test unmarshalling a basic DiscoHints element + */ public function testUnmarshalling() { $document = DOMDocumentFactory::fromString(<<assertEquals('geo:47.37328,8.531126', $disco->GeolocationHint[0]); $this->assertEquals('geo:19.34343,12.342514', $disco->GeolocationHint[1]); } + + /** + * Add a Keywords element to the children attribute + */ + public function testMarshallingChildren() + { + $discoHints = new DiscoHints(); + $keywords = new Keywords(); + $keywords->lang = "nl"; + $keywords->Keywords = array("voorbeeld", "specimen"); + $discoHints->children = array($keywords); + + $document = DOMDocumentFactory::fromString(''); + $xml = $discoHints->toXML($document->firstChild); + + $discoElements = Utils::xpQuery( + $xml, + '/root/*[local-name()=\'DiscoHints\' and namespace-uri()=\'urn:oasis:names:tc:SAML:metadata:ui\']' + ); + $this->assertCount(1, $discoElements); + $discoElement = $discoElements[0]; + $this->assertEquals("mdui:Keywords", $discoElement->firstChild->nodeName); + $this->assertEquals("voorbeeld specimen", $discoElement->firstChild->textContent); + } + + /** + * Umarshal a DiscoHints attribute with extra children + */ + public function testUnmarshallingChildren() + { + $document = DOMDocumentFactory::fromString(<< + geo:47.37328,8.531126 + content of tag + +XML + ); + + $disco = new DiscoHints($document->firstChild); + + $this->assertCount(1, $disco->GeolocationHint); + $this->assertEquals('geo:47.37328,8.531126', $disco->GeolocationHint[0]); + $this->assertCount(1, $disco->children); + $this->assertEquals('content of tag', $disco->children[0]->xml->textContent); + } } diff --git a/tests/SAML2/XML/mdui/KeywordsTest.php b/tests/SAML2/XML/mdui/KeywordsTest.php new file mode 100644 index 000000000..4dc4e5dba --- /dev/null +++ b/tests/SAML2/XML/mdui/KeywordsTest.php @@ -0,0 +1,95 @@ +lang = "en"; + $keywords->Keywords = array("KLM", "royal", "Dutch", "air lines"); + + $document = DOMDocumentFactory::fromString(''); + $xml = $keywords->toXML($document->firstChild); + + $keywordElements = Utils::xpQuery( + $xml, + '/root/*[local-name()=\'Keywords\' and namespace-uri()=\'urn:oasis:names:tc:SAML:metadata:ui\']' + ); + $this->assertCount(1, $keywordElements); + $keywordElement = $keywordElements[0]; + $this->assertEquals("KLM royal Dutch air+lines", $keywordElement->textContent); + $this->assertEquals("en", $keywordElement->getAttribute('xml:lang')); + } + + /** + * Keyword may not contain a "+", Exception expected. + */ + public function testKeywordWithPlusSignThrowsException() + { + $keywords = new Keywords(); + $keywords->lang = "en"; + $keywords->Keywords = array("csharp", "pascal", "c++"); + + $document = DOMDocumentFactory::fromString(''); + + $this->setExpectedException('Exception', 'Keywords may not contain a "+" character'); + $xml = $keywords->toXML($document->firstChild); + } + + /** + * Unmarshalling of a keywords tag + */ + public function testUnmarshalling() + { + $document = DOMDocumentFactory::fromString(<<KLM koninklijke luchtvaart+maatschappij +XML + ); + + $keywords = new Keywords($document->firstChild); + $this->assertEquals("nl", $keywords->lang); + $this->assertCount(3, $keywords->Keywords); + $this->assertEquals("KLM", $keywords->Keywords[0]); + $this->assertEquals("koninklijke", $keywords->Keywords[1]); + $this->assertEquals("luchtvaart maatschappij", $keywords->Keywords[2]); + } + + /** + * Unmarshalling fails if lang attribute not present + */ + public function testUnmarshallingFailsMissingLanguage() + { + $document = DOMDocumentFactory::fromString(<<KLM koninklijke luchtvaart+maatschappij +XML + ); + + $this->setExpectedException('Exception', 'Missing lang on Keywords'); + $keywords = new Keywords($document->firstChild); + } + + /** + * Unmarshalling fails if attribute is empty + */ + public function testUnmarshallingFailsMissingKeywords() + { + $document = DOMDocumentFactory::fromString(<< +XML + ); + + $this->setExpectedException('Exception', 'Missing value for Keywords'); + $keywords = new Keywords($document->firstChild); + } +} diff --git a/tests/SAML2/XML/mdui/LogoTest.php b/tests/SAML2/XML/mdui/LogoTest.php new file mode 100644 index 000000000..4ea883331 --- /dev/null +++ b/tests/SAML2/XML/mdui/LogoTest.php @@ -0,0 +1,97 @@ +lang = "nl"; + $logo->width = 300; + $logo->height = 200; + $logo->url = "https://static.example.org/images/logos/logo300x200.png"; + + $document = DOMDocumentFactory::fromString(''); + $xml = $logo->toXML($document->firstChild); + + $logoElements = Utils::xpQuery( + $xml, + '/root/*[local-name()=\'Logo\' and namespace-uri()=\'urn:oasis:names:tc:SAML:metadata:ui\']' + ); + $this->assertCount(1, $logoElements); + $logoElement = $logoElements[0]; + $this->assertEquals("https://static.example.org/images/logos/logo300x200.png", $logoElement->textContent); + $this->assertEquals("nl", $logoElement->getAttribute("xml:lang")); + $this->assertEquals(300, $logoElement->getAttribute("width")); + $this->assertEquals(200, $logoElement->getAttribute("height")); + } + + /** + * Unmarshalling of a logo tag + */ + public function testUnmarshalling() + { + $document = DOMDocumentFactory::fromString(<<https://static.example.org/images/logos/logo300x200.png +XML + ); + + $logo = new Logo($document->firstChild); + $this->assertEquals("nl", $logo->lang); + $this->assertEquals(300, $logo->width); + $this->assertEquals(200, $logo->height); + $this->assertEquals("https://static.example.org/images/logos/logo300x200.png", $logo->url); + } + + /** + * Unmarshalling fails if url attribute not present + */ + public function testUnmarshallingFailsEmptyURL() + { + $document = DOMDocumentFactory::fromString(<< +XML + ); + + $this->setExpectedException('Exception', 'Missing url value for Logo'); + $logo = new Logo($document->firstChild); + } + + /** + * Unmarshalling fails if width attribute not present + */ + public function testUnmarshallingFailsMissingWidth() + { + $document = DOMDocumentFactory::fromString(<<https://static.example.org/images/logos/logo300x200.png +XML + ); + + $this->setExpectedException('Exception', 'Missing width of Logo'); + $logo = new Logo($document->firstChild); + } + + /** + * Unmarshalling fails if height attribute not present + */ + public function testUnmarshallingFailsMissingHeight() + { + $document = DOMDocumentFactory::fromString(<<https://static.example.org/images/logos/logo300x200.png +XML + ); + + $this->setExpectedException('Exception', 'Missing height of Logo'); + $logo = new Logo($document->firstChild); + } +} diff --git a/tests/SAML2/XML/mdui/UIInfoTest.php b/tests/SAML2/XML/mdui/UIInfoTest.php new file mode 100644 index 000000000..373d90e14 --- /dev/null +++ b/tests/SAML2/XML/mdui/UIInfoTest.php @@ -0,0 +1,210 @@ +DisplayName = array("nl" => "Voorbeeld", "en" => "Example"); + $uiinfo->Description = array("nl" => "Omschrijving", "en" => "Description"); + $uiinfo->InformationURL = array("nl" => "https://voorbeeld.nl/", "en" => "https://example.org"); + $uiinfo->PrivacyStatementURL = array("nl" => "https://voorbeeld.nl/privacy", "en" => "https://example.org/privacy"); + + $document = DOMDocumentFactory::fromString(''); + $xml = $uiinfo->toXML($document->firstChild); + + $infoElements = Utils::xpQuery( + $xml, + '/root/*[local-name()=\'UIInfo\' and namespace-uri()=\'urn:oasis:names:tc:SAML:metadata:ui\']' + ); + $this->assertCount(1, $infoElements); + $infoElement = $infoElements[0]; + + $displaynameElements = Utils::xpQuery( + $infoElement, + './*[local-name()=\'DisplayName\' and namespace-uri()=\'urn:oasis:names:tc:SAML:metadata:ui\']' + ); + $this->assertCount(2, $displaynameElements); + $this->assertEquals("Voorbeeld", $displaynameElements[0]->textContent); + $this->assertEquals("Example", $displaynameElements[1]->textContent); + $this->assertEquals("nl", $displaynameElements[0]->getAttribute("xml:lang")); + $this->assertEquals("en", $displaynameElements[1]->getAttribute("xml:lang")); + + $descriptionElements = Utils::xpQuery( + $infoElement, + './*[local-name()=\'Description\' and namespace-uri()=\'urn:oasis:names:tc:SAML:metadata:ui\']' + ); + $this->assertCount(2, $descriptionElements); + $this->assertEquals("Omschrijving", $descriptionElements[0]->textContent); + $this->assertEquals("Description", $descriptionElements[1]->textContent); + $this->assertEquals("nl", $descriptionElements[0]->getAttribute("xml:lang")); + $this->assertEquals("en", $descriptionElements[1]->getAttribute("xml:lang")); + + $infourlElements = Utils::xpQuery( + $infoElement, + './*[local-name()=\'InformationURL\' and namespace-uri()=\'urn:oasis:names:tc:SAML:metadata:ui\']' + ); + $this->assertCount(2, $infourlElements); + $this->assertEquals("https://voorbeeld.nl/", $infourlElements[0]->textContent); + $this->assertEquals("https://example.org", $infourlElements[1]->textContent); + $this->assertEquals("nl", $infourlElements[0]->getAttribute("xml:lang")); + $this->assertEquals("en", $infourlElements[1]->getAttribute("xml:lang")); + + $privurlElements = Utils::xpQuery( + $infoElement, + './*[local-name()=\'PrivacyStatementURL\' and namespace-uri()=\'urn:oasis:names:tc:SAML:metadata:ui\']' + ); + $this->assertCount(2, $privurlElements); + $this->assertEquals("https://voorbeeld.nl/privacy", $privurlElements[0]->textContent); + $this->assertEquals("https://example.org/privacy", $privurlElements[1]->textContent); + $this->assertEquals("nl", $privurlElements[0]->getAttribute("xml:lang")); + $this->assertEquals("en", $privurlElements[1]->getAttribute("xml:lang")); + } + + /** + * Test creating an UIinfo element with XML children + */ + public function testMarshallingChildren() + { + $keywords = new Keywords(); + $keywords->lang = "nl"; + $keywords->Keywords = array("voorbeeld", "specimen"); + $logo = new Logo(); + $logo->lang = "nl"; + $logo->width = 30; + $logo->height = 20; + $logo->url = "https://example.edu/logo.png"; + $discohints = new DiscoHints(); + $discohints->IPHint = array("192.168.6.0/24", "fd00:0123:aa:1001::/64"); + // keywords appears twice, direcyly under UIinfo and as child of DiscoHints + $discohints->children = array($keywords); + + $uiinfo = new UIInfo(); + $uiinfo->Logo = array($logo); + $uiinfo->Keywords = array($keywords); + $uiinfo->children = array($discohints); + + $document = DOMDocumentFactory::fromString(''); + $xml = $uiinfo->toXML($document->firstChild); + + $infoElements = Utils::xpQuery( + $xml, + '/root/*[local-name()=\'UIInfo\' and namespace-uri()=\'urn:oasis:names:tc:SAML:metadata:ui\']' + ); + $this->assertCount(1, $infoElements); + $infoElement = $infoElements[0]; + + $logoElements = Utils::xpQuery( + $infoElement, + './*[local-name()=\'Logo\' and namespace-uri()=\'urn:oasis:names:tc:SAML:metadata:ui\']' + ); + $this->assertCount(1, $logoElements); + $this->assertEquals("https://example.edu/logo.png", $logoElements[0]->textContent); + + $keywordElements = Utils::xpQuery( + $infoElement, + './*[local-name()=\'Keywords\' and namespace-uri()=\'urn:oasis:names:tc:SAML:metadata:ui\']' + ); + $this->assertCount(1, $keywordElements); + $this->assertEquals("voorbeeld specimen", $keywordElements[0]->textContent); + $this->assertEquals("nl", $keywordElements[0]->getAttribute("xml:lang")); + + $discoElements = Utils::xpQuery( + $infoElement, + './*[local-name()=\'DiscoHints\' and namespace-uri()=\'urn:oasis:names:tc:SAML:metadata:ui\']' + ); + $this->assertCount(1, $discoElements); + $discoElement = $discoElements[0]; + + $iphintElements = Utils::xpQuery( + $discoElement, + './*[local-name()=\'IPHint\' and namespace-uri()=\'urn:oasis:names:tc:SAML:metadata:ui\']' + ); + $this->assertCount(2, $iphintElements); + $this->assertEquals("192.168.6.0/24", $iphintElements[0]->textContent); + $this->assertEquals("fd00:0123:aa:1001::/64", $iphintElements[1]->textContent); + + $keywordElements = Utils::xpQuery( + $discoElement, + './*[local-name()=\'Keywords\' and namespace-uri()=\'urn:oasis:names:tc:SAML:metadata:ui\']' + ); + $this->assertCount(1, $keywordElements); + $this->assertEquals("voorbeeld specimen", $keywordElements[0]->textContent); + $this->assertEquals("nl", $keywordElements[0]->getAttribute("xml:lang")); + } + + /** + * Test unmarshalling a basic UIInfo element + */ + public function testUnmarshalling() + { + $document = DOMDocumentFactory::fromString(<< + University of Examples + Univërsitä øf Exåmpleß + http://www.example.edu/en/ + http://www.example.edu/ + Just an example + https://example.org/privacy + +XML + ); + + $uiinfo = new UIInfo($document->firstChild); + + $this->assertCount(2, $uiinfo->DisplayName); + $this->assertEquals('University of Examples', $uiinfo->DisplayName['en']); + $this->assertEquals('Univërsitä øf Exåmpleß', $uiinfo->DisplayName['el']); + $this->assertCount(2, $uiinfo->InformationURL); + $this->assertEquals('http://www.example.edu/en/', $uiinfo->InformationURL['en']); + $this->assertEquals('http://www.example.edu/', $uiinfo->InformationURL['el']); + $this->assertCount(1, $uiinfo->PrivacyStatementURL); + $this->assertEquals('https://example.org/privacy', $uiinfo->PrivacyStatementURL['en']); + $this->assertCount(1, $uiinfo->Description); + $this->assertEquals('Just an example', $uiinfo->Description['en']); + } + + /** + * Test unmarshalling wuth Logo, Keywords child elements + */ + public function testUnmarshallingChildren() + { + $document = DOMDocumentFactory::fromString(<< + University of Examples + https://example.org/idp/images/logo_87x88.png + University Fictional + Université Fictif + + + +XML + ); + + $uiinfo = new UIInfo($document->firstChild); + + $this->assertCount(1, $uiinfo->DisplayName); + $this->assertEquals('University of Examples', $uiinfo->DisplayName['en']); + $this->assertCount(1, $uiinfo->Logo); + $this->assertEquals('https://example.org/idp/images/logo_87x88.png', $uiinfo->Logo[0]->url); + $this->assertEquals(87, $uiinfo->Logo[0]->width); + $this->assertEquals(88, $uiinfo->Logo[0]->height); + $this->assertEquals("fy", $uiinfo->Logo[0]->lang); + $this->assertCount(2, $uiinfo->Keywords); + $this->assertEquals('Fictional', $uiinfo->Keywords[0]->Keywords[1]); + $this->assertEquals('fr', $uiinfo->Keywords[1]->lang); + $this->assertCount(2, $uiinfo->children); + $this->assertEquals('child2', $uiinfo->children[1]->localName); + } +} From 41076cd823dbae0db022e74d30c01de26a46a310 Mon Sep 17 00:00:00 2001 From: Charles Bell Date: Sun, 27 Mar 2016 22:43:27 +0200 Subject: [PATCH 21/30] Fix KeyLoader::loadKeys --- src/SAML2/Certificate/KeyLoader.php | 2 +- tests/SAML2/Certificate/KeyLoaderTest.php | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/SAML2/Certificate/KeyLoader.php b/src/SAML2/Certificate/KeyLoader.php index ebf8dd5ca..2345f7768 100644 --- a/src/SAML2/Certificate/KeyLoader.php +++ b/src/SAML2/Certificate/KeyLoader.php @@ -90,7 +90,7 @@ public function loadKeysFromConfiguration( public function loadKeys(array $configuredKeys, $usage) { foreach ($configuredKeys as $keyData) { - if (isset($key['X509Certificate'])) { + if (isset($keyData['X509Certificate'])) { $key = new X509($keyData); } else { $key = new Key($keyData); diff --git a/tests/SAML2/Certificate/KeyLoaderTest.php b/tests/SAML2/Certificate/KeyLoaderTest.php index 18eb319c7..a28540e57 100644 --- a/tests/SAML2/Certificate/KeyLoaderTest.php +++ b/tests/SAML2/Certificate/KeyLoaderTest.php @@ -48,6 +48,24 @@ public function load_keys_checks_for_usage_of_key() $this->assertTrue($loadedKeys->get(0)->canBeUsedFor(Key::USAGE_SIGNING)); } + /** + * @group certificate + * + * @test + */ + public function load_keys_constructs_x509_certificate() + { + $keys = array(array( + 'X509Certificate' => $this->certificate + )); + + $this->keyLoader->loadKeys($keys, null); + $loadedKeys = $this->keyLoader->getKeys(); + + $this->assertCount(1, $loadedKeys); + $this->assertInstanceOf('SAML2\Certificate\X509', $loadedKeys->get(0)); + } + /** * @group certificate * From c31766dd3b194af7b544a49a28762b137e03cfa7 Mon Sep 17 00:00:00 2001 From: Jaime Perez Crespo Date: Tue, 29 Mar 2016 22:46:11 +0200 Subject: [PATCH 22/30] Allow setting "AllowCreate" in a NameIDPolicy to something other than "true". This resolves #64. Related to simplesamlphp/simplesamlphp#346 and simplesamlphp/simplesamlphp#347. --- src/SAML2/AuthnRequest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/SAML2/AuthnRequest.php b/src/SAML2/AuthnRequest.php index bf083adc2..060c1d563 100644 --- a/src/SAML2/AuthnRequest.php +++ b/src/SAML2/AuthnRequest.php @@ -714,8 +714,8 @@ public function toUnsignedXML() if (array_key_exists('SPNameQualifier', $this->nameIdPolicy)) { $nameIdPolicy->setAttribute('SPNameQualifier', $this->nameIdPolicy['SPNameQualifier']); } - if (array_key_exists('AllowCreate', $this->nameIdPolicy) && $this->nameIdPolicy['AllowCreate']) { - $nameIdPolicy->setAttribute('AllowCreate', 'true'); + if (array_key_exists('AllowCreate', $this->nameIdPolicy) && is_bool($this->nameIdPolicy['AllowCreate'])) { + $nameIdPolicy->setAttribute('AllowCreate', ($this->nameIdPolicy['AllowCreate']) ? 'true' : 'false'); } $root->appendChild($nameIdPolicy); } From 00e38f85b417be1e10a2d738dd2f5ea82edb472c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaime=20Pe=CC=81rez?= Date: Tue, 26 Jul 2016 15:28:46 +0200 Subject: [PATCH 23/30] Update the compat logger to the latest version of SSP. SimpleSAML_Logger has been refactored to use namespaces. Replace all calls to the old class with the new. --- src/SAML2/Compat/Ssp/Logger.php | 49 ++++++++++++++++----------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/src/SAML2/Compat/Ssp/Logger.php b/src/SAML2/Compat/Ssp/Logger.php index 877c4cb4f..6a9f90449 100644 --- a/src/SAML2/Compat/Ssp/Logger.php +++ b/src/SAML2/Compat/Ssp/Logger.php @@ -3,7 +3,6 @@ namespace SAML2\Compat\Ssp; use Psr\Log\LoggerInterface; -use SimpleSAML_Logger; class Logger implements LoggerInterface { @@ -16,7 +15,7 @@ class Logger implements LoggerInterface */ public function emergency($message, array $context = array()) { - SimpleSAML_Logger::emergency($message . var_export($context, true)); + \SimpleSAML\Logger::emergency($message . var_export($context, true)); } /** @@ -31,7 +30,7 @@ public function emergency($message, array $context = array()) */ public function alert($message, array $context = array()) { - SimpleSAML_Logger::alert($message . var_export($context, true)); + \SimpleSAML\Logger::alert($message . var_export($context, true)); } /** @@ -45,7 +44,7 @@ public function alert($message, array $context = array()) */ public function critical($message, array $context = array()) { - SimpleSAML_Logger::critical($message . var_export($context, true)); + \SimpleSAML\Logger::critical($message . var_export($context, true)); } /** @@ -58,7 +57,7 @@ public function critical($message, array $context = array()) */ public function error($message, array $context = array()) { - SimpleSAML_Logger::error($message . var_export($context, true)); + \SimpleSAML\Logger::error($message . var_export($context, true)); } /** @@ -73,7 +72,7 @@ public function error($message, array $context = array()) */ public function warning($message, array $context = array()) { - SimpleSAML_Logger::warning($message . var_export($context, true)); + \SimpleSAML\Logger::warning($message . var_export($context, true)); } /** @@ -85,7 +84,7 @@ public function warning($message, array $context = array()) */ public function notice($message, array $context = array()) { - SimpleSAML_Logger::notice($message . var_export($context, true)); + \SimpleSAML\Logger::notice($message . var_export($context, true)); } /** @@ -99,7 +98,7 @@ public function notice($message, array $context = array()) */ public function info($message, array $context = array()) { - SimpleSAML_Logger::info($message . var_export($context, true)); + \SimpleSAML\Logger::info($message . var_export($context, true)); } /** @@ -111,7 +110,7 @@ public function info($message, array $context = array()) */ public function debug($message, array $context = array()) { - SimpleSAML_Logger::debug($message . var_export($context, true)); + \SimpleSAML\Logger::debug($message . var_export($context, true)); } /** @@ -125,29 +124,29 @@ public function debug($message, array $context = array()) public function log($level, $message, array $context = array()) { switch ($level) { - case SimpleSAML_Logger::ALERT: - SimpleSAML_Logger::alert($message); + case \SimpleSAML\Logger::ALERT: + \SimpleSAML\Logger::alert($message); break; - case SimpleSAML_Logger::CRIT: - SimpleSAML_Logger::critical($message); + case \SimpleSAML\Logger::CRIT: + \SimpleSAML\Logger::critical($message); break; - case SimpleSAML_Logger::DEBUG: - SimpleSAML_Logger::debug($message); + case \SimpleSAML\Logger::DEBUG: + \SimpleSAML\Logger::debug($message); break; - case SimpleSAML_Logger::EMERG: - SimpleSAML_Logger::emergency($message); + case \SimpleSAML\Logger::EMERG: + \SimpleSAML\Logger::emergency($message); break; - case SimpleSAML_Logger::ERR: - SimpleSAML_Logger::error($message); + case \SimpleSAML\Logger::ERR: + \SimpleSAML\Logger::error($message); break; - case SimpleSAML_Logger::INFO: - SimpleSAML_Logger::info($message); + case \SimpleSAML\Logger::INFO: + \SimpleSAML\Logger::info($message); break; - case SimpleSAML_Logger::NOTICE: - SimpleSAML_Logger::notice($message); + case \SimpleSAML\Logger::NOTICE: + \SimpleSAML\Logger::notice($message); break; - case SimpleSAML_Logger::WARNING: - SimpleSAML_Logger::warning($message); + case \SimpleSAML\Logger::WARNING: + \SimpleSAML\Logger::warning($message); } } } From d1680d3e924793dcc6bffeff5c3dfbd62ed05eab Mon Sep 17 00:00:00 2001 From: Thijs Kinkhorst Date: Fri, 5 Aug 2016 12:00:25 +0000 Subject: [PATCH 24/30] Add deprecation tags to classes and methods that work with certificate fingerprints. See #68 --- src/SAML2/Certificate/Fingerprint.php | 4 ++++ src/SAML2/Certificate/FingerprintCollection.php | 5 +++++ src/SAML2/Certificate/FingerprintLoader.php | 7 +++++++ src/SAML2/Certificate/X509.php | 2 ++ src/SAML2/Configuration/CertificateProvider.php | 2 ++ src/SAML2/Configuration/IdentityProvider.php | 3 +++ src/SAML2/Configuration/ServiceProvider.php | 3 +++ src/SAML2/Signature/FingerprintValidator.php | 5 +++++ src/SAML2/SignedElementHelper.php | 2 +- 9 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/SAML2/Certificate/Fingerprint.php b/src/SAML2/Certificate/Fingerprint.php index 7cd9e5430..f42e4dc5d 100644 --- a/src/SAML2/Certificate/Fingerprint.php +++ b/src/SAML2/Certificate/Fingerprint.php @@ -6,6 +6,8 @@ /** * Simple representation of the fingerprint of a certificate + * + * @deprecated Please use full certificates instead. */ class Fingerprint { @@ -16,6 +18,8 @@ class Fingerprint /** * @param string $fingerPrint + * + * @deprecated Please use full certificates instead. */ public function __construct($fingerPrint) { diff --git a/src/SAML2/Certificate/FingerprintCollection.php b/src/SAML2/Certificate/FingerprintCollection.php index cf0e0e253..1d80afcd8 100644 --- a/src/SAML2/Certificate/FingerprintCollection.php +++ b/src/SAML2/Certificate/FingerprintCollection.php @@ -7,6 +7,7 @@ /** * Simple collection object for transporting keys + * @deprecated Please load full certificates instead. */ class FingerprintCollection extends ArrayCollection { @@ -14,6 +15,8 @@ class FingerprintCollection extends ArrayCollection * Add a key to the collection * * @param \SAML2\Certificate\Fingerprint $fingerprint + * + * @deprecated */ public function add($fingerprint) { @@ -31,6 +34,8 @@ public function add($fingerprint) * @param \SAML2\Certificate\Fingerprint $otherFingerprint * * @return bool + * + * @deprecated */ public function contains(Fingerprint $otherFingerprint) { diff --git a/src/SAML2/Certificate/FingerprintLoader.php b/src/SAML2/Certificate/FingerprintLoader.php index 1e4ed2793..26a63e1f0 100644 --- a/src/SAML2/Certificate/FingerprintLoader.php +++ b/src/SAML2/Certificate/FingerprintLoader.php @@ -5,6 +5,9 @@ use SAML2\Configuration\CertificateProvider; use SAML2\Exception\InvalidArgumentException; +/** + * @deprecated Please load full certificates instead. + */ class FingerprintLoader { /** @@ -13,6 +16,8 @@ class FingerprintLoader * @param \SAML2\Configuration\CertificateProvider $configuration * * @return \SAML2\Certificate\FingerprintCollection + * + * @deprecated */ public static function loadFromConfiguration(CertificateProvider $configuration) { @@ -27,6 +32,8 @@ public static function loadFromConfiguration(CertificateProvider $configuration) * @param \SAML2\Configuration\CertificateProvider $configuration * * @return \SAML2\Certificate\FingerprintCollection + * + * @deprecated */ public function loadFingerprints(CertificateProvider $configuration) { diff --git a/src/SAML2/Certificate/X509.php b/src/SAML2/Certificate/X509.php index 904ab0f83..92ee24c9e 100644 --- a/src/SAML2/Certificate/X509.php +++ b/src/SAML2/Certificate/X509.php @@ -50,6 +50,8 @@ public function getCertificate() /** * @return \SAML2\Certificate\Fingerprint + * + * @deprecated Please use full certificates instead. */ public function getFingerprint() { diff --git a/src/SAML2/Configuration/CertificateProvider.php b/src/SAML2/Configuration/CertificateProvider.php index 849e2fcea..902d320c7 100644 --- a/src/SAML2/Configuration/CertificateProvider.php +++ b/src/SAML2/Configuration/CertificateProvider.php @@ -36,6 +36,8 @@ public function getCertificateFile(); * fingerprint is a string containing the certificate fingerprint. * * @return null|array|\Traversable + * + * @deprecated Please use getCertifiateFile() or getCertificateData() */ public function getCertificateFingerprints(); } diff --git a/src/SAML2/Configuration/IdentityProvider.php b/src/SAML2/Configuration/IdentityProvider.php index 4c0d56e2a..6c32283b7 100644 --- a/src/SAML2/Configuration/IdentityProvider.php +++ b/src/SAML2/Configuration/IdentityProvider.php @@ -25,6 +25,9 @@ public function getCertificateFile() return $this->get('certificateFile'); } + /** + * @deprecated Please use getCertifiateFile() or getCertificateData() + */ public function getCertificateFingerprints() { return $this->get('certificateFingerprints'); diff --git a/src/SAML2/Configuration/ServiceProvider.php b/src/SAML2/Configuration/ServiceProvider.php index 59b88a0ae..e71d0166f 100644 --- a/src/SAML2/Configuration/ServiceProvider.php +++ b/src/SAML2/Configuration/ServiceProvider.php @@ -27,6 +27,9 @@ public function getCertificateFile() return $this->get('certificateFile'); } + /** + * @deprecated Please use getCertificateData() or getCertificateFile(). + */ public function getCertificateFingerprints() { return $this->get('certificateFingerprints'); diff --git a/src/SAML2/Signature/FingerprintValidator.php b/src/SAML2/Signature/FingerprintValidator.php index 1bb8b3185..828ea5a73 100644 --- a/src/SAML2/Signature/FingerprintValidator.php +++ b/src/SAML2/Signature/FingerprintValidator.php @@ -10,6 +10,8 @@ /** * Validates the signature based on the fingerprint of the certificate + * + * @deprecated Please use full certificates instead. */ class FingerprintValidator extends AbstractChainedValidator { @@ -23,6 +25,9 @@ class FingerprintValidator extends AbstractChainedValidator */ private $fingerprintLoader; + /** + * @deprecated Please use full certificates instead. + */ public function __construct( LoggerInterface $logger, FingerprintLoader $fingerprintLoader diff --git a/src/SAML2/SignedElementHelper.php b/src/SAML2/SignedElementHelper.php index 5ba069890..50de0cfe2 100644 --- a/src/SAML2/SignedElementHelper.php +++ b/src/SAML2/SignedElementHelper.php @@ -175,7 +175,7 @@ public function getValidatingCertificates() $ret = array(); foreach ($this->certificates as $cert) { - /* We have found a matching fingerprint. */ + /* Construct a PEM formatted certificate */ $pemCert = "-----BEGIN CERTIFICATE-----\n" . chunk_split($cert, 64) . "-----END CERTIFICATE-----\n"; From e6195826bf368da67ec118f41512a979d1819032 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaime=20Pe=CC=81rez?= Date: Fri, 12 Aug 2016 16:02:15 +0200 Subject: [PATCH 25/30] bugfix: PrivateKeyConfiguration alias confused with PrivateKey. This resolves #71. --- src/SAML2/Certificate/PrivateKeyLoader.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/SAML2/Certificate/PrivateKeyLoader.php b/src/SAML2/Certificate/PrivateKeyLoader.php index 82a335587..d9232ded6 100644 --- a/src/SAML2/Certificate/PrivateKeyLoader.php +++ b/src/SAML2/Certificate/PrivateKeyLoader.php @@ -46,13 +46,13 @@ public function loadDecryptionKeys( return $decryptionKeys; } - $newPrivateKey = $serviceProvider->getPrivateKey(PrivateKey::NAME_NEW); + $newPrivateKey = $serviceProvider->getPrivateKey(PrivateKeyConfiguration::NAME_NEW); if ($newPrivateKey instanceof PrivateKey) { $loadedKey = $this->loadPrivateKey($newPrivateKey); $decryptionKeys->add($this->convertPrivateKeyToRsaKey($loadedKey)); } - $privateKey = $serviceProvider->getPrivateKey(PrivateKey::NAME_DEFAULT, true); + $privateKey = $serviceProvider->getPrivateKey(PrivateKeyConfiguration::NAME_DEFAULT, true); $loadedKey = $this->loadPrivateKey($privateKey); $decryptionKeys->add($this->convertPrivateKeyToRsaKey($loadedKey)); From 5f68f863e40ed935b1227bfe65958bc2a76922f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaime=20Pe=CC=81rez?= Date: Fri, 12 Aug 2016 16:26:39 +0200 Subject: [PATCH 26/30] bugfix: Set logger in SAML2\Assertion\Transformer\NameIDDecryptionTransformer. This resolves #72. --- src/SAML2/Assertion/Transformer/NameIdDecryptionTransformer.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/SAML2/Assertion/Transformer/NameIdDecryptionTransformer.php b/src/SAML2/Assertion/Transformer/NameIdDecryptionTransformer.php index 95c6c94b0..644058161 100644 --- a/src/SAML2/Assertion/Transformer/NameIdDecryptionTransformer.php +++ b/src/SAML2/Assertion/Transformer/NameIdDecryptionTransformer.php @@ -40,6 +40,7 @@ public function __construct( LoggerInterface $logger, PrivateKeyLoader $privateKeyLoader ) { + $this->logger = $logger; $this->privateKeyLoader = $privateKeyLoader; } From e25ed1a7fbedd5a9cea76b494641f325a0351937 Mon Sep 17 00:00:00 2001 From: Thijs Kinkhorst Date: Mon, 15 Aug 2016 19:09:14 +0200 Subject: [PATCH 27/30] Fix typo in error message. --- src/SAML2/Response/Processor.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SAML2/Response/Processor.php b/src/SAML2/Response/Processor.php index 03ad14d93..89275f50f 100644 --- a/src/SAML2/Response/Processor.php +++ b/src/SAML2/Response/Processor.php @@ -148,7 +148,7 @@ private function processAssertions(Response $response) foreach ($assertions as $assertion) { if (!$assertion->getWasSignedAtConstruction()) { throw new UnsignedResponseException( - 'Both the response and the assertion it containes are not signed.' + 'Both the response and the assertion it contains are not signed.' ); } } From a5328a8879fa8fa58bf675f0dd49eb282460bff4 Mon Sep 17 00:00:00 2001 From: DRvanR Date: Wed, 7 Sep 2016 13:33:52 +0200 Subject: [PATCH 28/30] EPTI urn mace and urn oid are available --- src/SAML2/Constants.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/SAML2/Constants.php b/src/SAML2/Constants.php index 4fa5bc933..7407bfa96 100644 --- a/src/SAML2/Constants.php +++ b/src/SAML2/Constants.php @@ -53,7 +53,7 @@ class Constants * Holder-of-Key subject confirmation method. */ const CM_HOK = 'urn:oasis:names:tc:SAML:2.0:cm:holder-of-key'; - + /** * Vouches subject confirmation method. */ @@ -126,6 +126,10 @@ class Constants */ const CONSENT_INAPPLICABLE = 'urn:oasis:names:tc:SAML:2.0:consent:inapplicable'; + const EPTI_URN_MACE = 'urn:mace:dir:attribute-def:eduPersonTargetedID'; + + const EPTI_URN_OID = 'urn:oid:1.3.6.1.4.1.5923.1.1.1.10'; + /** * The interpretation of the attribute name is left to individual implementations. */ From da07159082bae9b6febfc26f689ccb4ebcb8cd22 Mon Sep 17 00:00:00 2001 From: DRvanR Date: Wed, 7 Sep 2016 13:35:08 +0200 Subject: [PATCH 29/30] EPTI attribute parsing adheres to specified structure --- src/SAML2/Assertion.php | 31 ++++++ tests/SAML2/AssertionTest.php | 180 +++++++++++++++++++++++++++++++--- 2 files changed, 198 insertions(+), 13 deletions(-) diff --git a/src/SAML2/Assertion.php b/src/SAML2/Assertion.php index 1d7618fcb..e49ecacd0 100644 --- a/src/SAML2/Assertion.php +++ b/src/SAML2/Assertion.php @@ -4,6 +4,7 @@ use RobRichards\XMLSecLibs\XMLSecEnc; use RobRichards\XMLSecLibs\XMLSecurityKey; +use SAML2\Exception\RuntimeException; use SAML2\Utilities\Temporal; use SAML2\XML\Chunk; use SAML2\XML\saml\SubjectConfirmation; @@ -511,7 +512,27 @@ private function parseAttributes(\DOMElement $xml) */ private function parseAttributeValue($attribute, $attributeName) { + /** @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) { + $eptiNameId = Utils::xpQuery($eptiAttributeValue, './saml: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 + )); + } + + $this->attributes[$attributeName][] = Utils::parseNameId($eptiNameId[0]); + } + + return; + } + foreach ($values as $value) { $hasNonTextChildElements = false; foreach ($value->childNodes as $childNode) { @@ -1468,6 +1489,16 @@ private function addAttributeStatement(\DOMElement $root) $attribute->setAttribute('NameFormat', $this->nameFormat); } + 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); + Utils::addNameId($attributeValue, $eptiValue); + } + + continue; + } + foreach ($values as $value) { if (is_string($value)) { $type = 'xs:string'; diff --git a/tests/SAML2/AssertionTest.php b/tests/SAML2/AssertionTest.php index d3f85c564..dbf8e599b 100644 --- a/tests/SAML2/AssertionTest.php +++ b/tests/SAML2/AssertionTest.php @@ -497,7 +497,7 @@ public function testCorrectSignatureMethodCanBeExtracted() } - public function testAttributeValuesWithComplexTypeValuesAreParsedCorrectly() + public function testEptiAttributeValuesAreParsedCorrectly() { $xml = <<firstChild); + $attributes = $assertion->getAttributes(); + + $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->assertEquals( + array( + 'Value' => 'abcd-some-value-xyz', + 'Format' => 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent' + ), + $maceValue, + 'Parsing the EPTI attribute named with urn:mace did not result in the correct value' + ); + $this->assertEquals( + array( + 'Value' => 'abcd-some-value-xyz', + 'Format' => 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent' + ), + $oidValue, + 'Parsing the EPTI attribute named with urn:oid did not result in the correct value' + ); + + $this->assertXmlStringEqualsXmlString($xml, $assertion->toXML()->ownerDocument->saveXML()); + } + + public function testEptiAttributeValuesMustBeANameID() + { + $xml = << + Provider + + + + + abcd-some-value-xyz + + + + + + abcd-some-value-xyz + + + + + string + + + +XML; + + $this->setExpectedException( + 'SAML2\Exception\RuntimeException', + 'A "urn:mace:dir:attribute-def:eduPersonTargetedID" (EPTI) attribute value must be a NameID, ' + . 'none found for value no. "0"' + ); + new Assertion(DOMDocumentFactory::fromString($xml)->firstChild); + } + + /** + * as per http://software.internet2.edu/eduperson/internet2-mace-dir-eduperson-201310.html#eduPersonTargetedID + * it is multivalued + */ + public function testEptiAttributeParsingSupportsMultipleValues() + { + $xml + = << + Provider + + + + + abcd-some-value-xyz + + + xyz-some-value-abcd + + + + string + + + +XML; + + $assertion = new Assertion(DOMDocumentFactory::fromString($xml)->firstChild); + + $attributes = $assertion->getAttributes(); + + $maceFirstValue = $attributes['urn:mace:dir:attribute-def:eduPersonTargetedID'][0]; + $maceSecondValue = $attributes['urn:mace:dir:attribute-def:eduPersonTargetedID'][1]; + + $this->assertEquals( + array( + 'Value' => 'abcd-some-value-xyz', + 'Format' => 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent' + ), + $maceFirstValue, + 'Parsing the EPTI attribute with multiple values resulted in an incorrect first value' + ); + $this->assertEquals( + array( + 'Value' => 'xyz-some-value-abcd', + 'Format' => 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent' + ), + $maceSecondValue, + 'Parsing the EPTI attribute with multiple values resulted in an incorrect second value' + ); + + $this->assertXmlStringEqualsXmlString($xml, $assertion->toXML()->ownerDocument->saveXML()); + } + + public function testAttributeValuesWithComplexTypesAreParsedCorrectly() + { + $xml = << + Provider + + + + + + abcd-some-value-xyz + + + + + string + + + +XML; + + $assertion = new Assertion(DOMDocumentFactory::fromString($xml)->firstChild); + $attributes = $assertion->getAttributes(); $this->assertInstanceOf( '\DOMNodeList', - $attributes['urn:mace:dir:attribute-def:eduPersonTargetedID'][0] + $attributes['urn:some:custom:outer:element'][0] ); $this->assertXmlStringEqualsXmlString($xml, $assertion->toXML()->ownerDocument->saveXML()); } @@ -584,14 +741,11 @@ public function testEncryptedAttributeValuesWithComplexTypeValuesAreParsedCorrec Provider - - - abcd-some-value-xyz - - - + - abcd-some-value-xyz + + abcd-some-value-xyz + @@ -619,7 +773,7 @@ public function testEncryptedAttributeValuesWithComplexTypeValuesAreParsedCorrec $attributes = $assertionToVerify->getAttributes(); $this->assertInstanceOf( '\DOMNodeList', - $attributes['urn:mace:dir:attribute-def:eduPersonTargetedID'][0] + $attributes['urn:some:custom:outer:element'][0] ); $this->assertXmlStringEqualsXmlString($xml, $assertionToVerify->toXML()->ownerDocument->saveXML()); } @@ -1347,7 +1501,7 @@ public function testMixedAttributeNameFormats() Provider - + string @@ -1359,8 +1513,8 @@ public function testMixedAttributeNameFormats() $assertion = new Assertion(DOMDocumentFactory::fromString($xml)->firstChild); - $namefmt = $assertion->getAttributeNameFormat(); - $this->assertEquals('urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified', $namefmt); + $nameFormat = $assertion->getAttributeNameFormat(); + $this->assertEquals('urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified', $nameFormat); } /** From 09e41571b53e209c3a96b3f213efdc2da6239d12 Mon Sep 17 00:00:00 2001 From: DRvanR Date: Wed, 7 Sep 2016 14:59:15 +0200 Subject: [PATCH 30/30] Documentation of attribute value representation is complete --- src/SAML2/Assertion.php | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/SAML2/Assertion.php b/src/SAML2/Assertion.php index e49ecacd0..ff85a56ba 100644 --- a/src/SAML2/Assertion.php +++ b/src/SAML2/Assertion.php @@ -153,10 +153,21 @@ class Assertion implements SignedElement private $AuthenticatingAuthority; /** - * The attributes, as an associative array. + * The attributes, as an associative array, indexed by attribute name * - * @var array multi-dimensional array, indexed by attribute name with each value representing the attribute value - * of that attribute. This value is an array of \DOMNodeList|string|int + * To ease handling, all attribute values are represented as an array of values, also for values with a multiplicity + * of single. There are 4 possible variants of datatypes for the values: a string, an integer, an array or + * a DOMNodeList + * + * If the attribute is an eduPersonTargetedID, the values will be arrays that are created by @see Utils::parseNameId + * and compatible with @see Utils::addNameID + * 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|string|int|array */ private $attributes;