diff --git a/composer.json b/composer.json index 4b961f4..9a16e76 100644 --- a/composer.json +++ b/composer.json @@ -51,9 +51,8 @@ "scripts": { "pre-commit": [ "vendor/bin/phpcs -p", - "vendor/bin/composer-require-checker check composer.json", - "vendor/bin/phpstan analyze -c phpstan.neon", - "vendor/bin/phpstan analyze -c phpstan-dev.neon", + "vendor/bin/phpstan analyze -c phpstan.neon --memory-limit=1024M", + "vendor/bin/phpstan analyze -c phpstan-dev.neon --memory-limit=1024M", "vendor/bin/composer-unused", "vendor/bin/phpunit --no-coverage --testdox" ] diff --git a/src/XML/fed/AbstractWebServiceDescriptorType.php b/src/XML/fed/AbstractWebServiceDescriptorType.php index 011c3ef..b054d76 100644 --- a/src/XML/fed/AbstractWebServiceDescriptorType.php +++ b/src/XML/fed/AbstractWebServiceDescriptorType.php @@ -12,6 +12,8 @@ use SimpleSAML\SAML2\XML\md\AbstractRoleDescriptor; use SimpleSAML\SAML2\XML\md\Extensions; use SimpleSAML\SAML2\XML\md\Organization; +use SimpleSAML\XML\Attribute as XMLAttribute; +use SimpleSAML\XMLSchema\Constants as C; use SimpleSAML\XMLSchema\Type\DurationValue; use SimpleSAML\XMLSchema\Type\IDValue; use SimpleSAML\XMLSchema\Type\QNameValue; @@ -23,6 +25,24 @@ */ abstract class AbstractWebServiceDescriptorType extends AbstractRoleDescriptor { + /** + * The element is md:RoleDescriptor, but its content model lives in the WS-Federation schema — that is + * what the xsi:type points at. Validating against the inherited metadata schema alone can never resolve + * it, because md:RoleDescriptorType is abstract. ws-federation.xsd imports the metadata namespace. + */ + public const string SCHEMA = 'resources/schemas/ws-federation.xsd'; + + /** + * The exclusions for the xs:anyAttribute element + * + * xsi:type is modeled by AbstractRoleDescriptor as $type and returned by getXsiType(); without this + * exclusion it would also be swept into the extendable-attributes bucket and written a second time. + */ + public const array XS_ANY_ATTR_EXCLUSIONS = [ + [C::NS_XSI, 'type'], + ]; + + /** * WebServiceDescriptorType constructor. * @@ -198,6 +218,21 @@ public function toUnsignedXML(?DOMElement $parent = null): DOMElement { $e = parent::toUnsignedXML($parent); + // md:RoleDescriptor requires an xsi:type. AbstractRoleDescriptor stores it and demands it back in + // fromXML(), but nothing ever writes it, so the element cannot round-trip its own output. + // + // Re-express it with this type's own prefix rather than the caller's: a QName is identified by its + // {namespace, local name}, so the prefix is lexical only and the value keeps its meaning — and this + // is the one prefix AbstractSignedMdElement::toXML() is guaranteed to declare for us. A caller's + // prefix, or none at all, would otherwise be left unbound and the QName unresolvable. + $xsiType = QNameValue::fromParts( + $this->getXsiType()->getLocalName(), + $this->getXsiType()->getNamespaceURI(), + static::getXsiTypePrefix(), + ); + + (new XMLAttribute(C::NS_XSI, 'xsi', 'type', $xsiType))->toXML($e); + $this->getLogicalServiceNamesOffered()?->toXML($e); $this->getTokenTypesOffered()?->toXML($e); $this->getClaimDialectsOffered()?->toXML($e); diff --git a/tests/Federation/XML/fed/SecurityTokenServiceTypeTest.php b/tests/Federation/XML/fed/SecurityTokenServiceTypeTest.php new file mode 100644 index 0000000..2e7f875 --- /dev/null +++ b/tests/Federation/XML/fed/SecurityTokenServiceTypeTest.php @@ -0,0 +1,207 @@ +assertEquals( + self::$xmlRepresentation->saveXML(self::$xmlRepresentation->documentElement), + strval(self::buildSecurityTokenServiceType()), + ); + } + + + /** + * md:RoleDescriptor carries its element type in xsi:type, and AbstractRoleDescriptor::fromXML() + * rejects the element outright when it is absent. Serialising has to emit it. + */ + public function testMarshallingWritesXsiType(): void + { + $element = self::buildSecurityTokenServiceType()->toXML(); + + $this->assertEquals( + 'fed:SecurityTokenServiceType', + $element->getAttributeNS(C_XSI::NS_XSI, 'type'), + ); + + // The prefix used inside the attribute value only counts if it is bound on the element. + $this->assertEquals(C::NS_FED, $element->lookupNamespaceURI('fed')); + } + + + /** + * The element must be able to read back what it writes. Before the xsi:type was emitted this threw + * a SchemaViolationException on the object's own output. + */ + public function testRoundTrip(): void + { + $original = self::buildSecurityTokenServiceType(); + + $this->assertEquals( + strval($original), + strval(SecurityTokenServiceType::fromXML($original->toXML())), + ); + } + + + /** + * An xsi:type may legally be unprefixed, resolving through the document's default namespace. The + * element must not lose that namespace on the way out — a default namespace cannot be re-declared + * here without capturing unqualified descendants, so the type is re-expressed with its own prefix. + */ + public function testUnprefixedXsiTypeKeepsItsNamespace(): void + { + $xml = << + + + https://idp.example.org/adfs/services/trust + + + + XML; + + $parsed = SecurityTokenServiceType::fromXML( + DOMDocumentFactory::fromString($xml)->documentElement, + ); + $this->assertNull($parsed->getXsiType()->getNamespacePrefix()); + + $element = $parsed->toXML(); + $this->assertEquals( + 'fed:SecurityTokenServiceType', + $element->getAttributeNS(C_XSI::NS_XSI, 'type'), + ); + + // No default namespace may be introduced, or unqualified descendants would change meaning. + $this->assertNull($element->lookupNamespaceURI(null)); + + // What matters is that the QName still denotes the same {namespace, local name}. + $this->assertEquals( + C::NS_FED, + SecurityTokenServiceType::fromXML($element)->getXsiType()->getNamespaceURI()->getValue(), + ); + } + + + // test unmarshalling + + + /** + * Test creating a SecurityTokenServiceType object from XML. + */ + public function testUnmarshalling(): void + { + $securityTokenServiceType = SecurityTokenServiceType::fromXML( + self::$xmlRepresentation->documentElement, + ); + + $this->assertEquals( + 'fed:SecurityTokenServiceType', + strval($securityTokenServiceType->getXsiType()), + ); + + // xsi:type is modelled as the element's own type, not as one of its extendable attributes. + $this->assertEmpty($securityTokenServiceType->getAttributesNS()); + $this->assertCount(1, $securityTokenServiceType->getSecurityTokenServiceEndpoint()); + $this->assertCount(1, $securityTokenServiceType->getPassiveRequestorEndpoint()); + $this->assertEquals( + 'SimpleSAMLphp ADFS IdP', + strval($securityTokenServiceType->getServiceDisplayName()), + ); + } +} diff --git a/tests/resources/xml/fed/SecurityTokenServiceType.xml b/tests/resources/xml/fed/SecurityTokenServiceType.xml new file mode 100644 index 0000000..e63ad78 --- /dev/null +++ b/tests/resources/xml/fed/SecurityTokenServiceType.xml @@ -0,0 +1,13 @@ + + + + + https://idp.example.org/adfs/services/trust + + + + + https://idp.example.org/adfs/ls/ + + +