diff --git a/phpcs.xml b/phpcs.xml index 82471a2e1..468182e25 100644 --- a/phpcs.xml +++ b/phpcs.xml @@ -73,6 +73,7 @@ tests/SAML2/XML/saml/AuthnContextTest.php tests/SAML2/XML/saml/IssuerXMLShowAllTest.php tests/SAML2/XML/saml/NameIDTest.php + tests/SAML2/XML/samlp/RequestedAuthnContextTest.php tests/SAML2/XML/samlp/StatusDetailTest.php tests/SAML2/XML/shibmd/KeyAuthorityTest.php tests/SAML2/Assertion/Validation/ConstraintValidator/SubjectConfirmationResponseToMatchesTest.php diff --git a/src/SAML2/AuthnRequest.php b/src/SAML2/AuthnRequest.php index 44a6702ce..a3b23d9d5 100644 --- a/src/SAML2/AuthnRequest.php +++ b/src/SAML2/AuthnRequest.php @@ -18,6 +18,7 @@ use SimpleSAML\SAML2\XML\samlp\IDPEntry; use SimpleSAML\SAML2\XML\samlp\IDPList; use SimpleSAML\SAML2\XML\samlp\NameIDPolicy; +use SimpleSAML\SAML2\XML\samlp\RequestedAuthnContext; use SimpleSAML\SAML2\XML\samlp\RequesterID; use SimpleSAML\SAML2\XML\samlp\Scoping; use SimpleSAML\XML\Exception\MissingElementException; @@ -105,13 +106,9 @@ class AuthnRequest extends Request /** * What authentication context was requested. * - * Array with the following elements. - * - AuthnContextClassRef (required) - * - Comparison (optional) - * - * @var array + * @var \SimpleSAML\SAML2\XML\samlp\RequestedAuthnContext|null */ - private array $requestedAuthnContext = []; + private ?RequestedAuthnContext $requestedAuthnContext = null; /** * Audiences to send in the request. @@ -178,7 +175,9 @@ public function __construct(DOMElement $xml = null) $nameIdPolicy = NameIDPolicy::getChildrenOfClass($xml); $this->nameIdPolicy = array_pop($nameIdPolicy); - $this->parseRequestedAuthnContext($xml); + $requestedAuthnContext = RequestedAuthnContext::getChildrenOfClass($xml); + $this->requestedAuthnContext = array_pop($requestedAuthnContext); + $this->parseConditions($xml); $scoping = Scoping::getChildrenOfClass($xml); @@ -233,42 +232,6 @@ private function parseSubject(DOMElement $xml): void } - /** - * @param \DOMElement $xml - * @return void - */ - protected function parseRequestedAuthnContext(DOMElement $xml): void - { - $xpCache = XPath::getXPath($xml); - - /** @var \DOMElement[] $requestedAuthnContext */ - $requestedAuthnContext = XPath::xpQuery($xml, './saml_protocol:RequestedAuthnContext', $xpCache); - if (empty($requestedAuthnContext)) { - return; - } - - $requestedAuthnContext = $requestedAuthnContext[0]; - - $rac = [ - 'AuthnContextClassRef' => [], - 'Comparison' => Constants::COMPARISON_EXACT, - ]; - - $xpCache = XPath::getXPath($requestedAuthnContext); - /** @var \DOMElement[] $accr */ - $accr = XPath::xpQuery($requestedAuthnContext, './saml_assertion:AuthnContextClassRef', $xpCache); - foreach ($accr as $i) { - $rac['AuthnContextClassRef'][] = trim($i->textContent); - } - - if ($requestedAuthnContext->hasAttribute('Comparison')) { - $rac['Comparison'] = $requestedAuthnContext->getAttribute('Comparison'); - } - - $this->requestedAuthnContext = $rac; - } - - /** * @param \DOMElement $xml * @return void @@ -532,9 +495,9 @@ public function setAssertionConsumerServiceIndex(int $assertionConsumerServiceIn /** * Retrieve the RequestedAuthnContext. * - * @return array|null The RequestedAuthnContext. + * @return \SimpleSAML\SAML2\XML\samlp\RequestedAuthnContext|null The RequestedAuthnContext. */ - public function getRequestedAuthnContext(): ?array + public function getRequestedAuthnContext(): ?RequestedAuthnContext { return $this->requestedAuthnContext; } @@ -543,10 +506,10 @@ public function getRequestedAuthnContext(): ?array /** * Set the RequestedAuthnContext. * - * @param array|null $requestedAuthnContext The RequestedAuthnContext. + * @param \SimpleSAML\SAML2\XML\samlp\RequestedAuthnContext|null $requestedAuthnContext The RequestedAuthnContext. * @return void */ - public function setRequestedAuthnContext(array $requestedAuthnContext = null): void + public function setRequestedAuthnContext(?RequestedAuthnContext $requestedAuthnContext = null): void { $this->requestedAuthnContext = $requestedAuthnContext; } @@ -709,17 +672,7 @@ public function toUnsignedXML(): DOMElement $this->addConditions($root); - $rac = $this->requestedAuthnContext; - if (!empty($rac) && !empty($rac['AuthnContextClassRef'])) { - $e = $this->document->createElementNS(Constants::NS_SAMLP, 'RequestedAuthnContext'); - $root->appendChild($e); - if (isset($rac['Comparison']) && $rac['Comparison'] !== Constants::COMPARISON_EXACT) { - $e->setAttribute('Comparison', $rac['Comparison']); - } - foreach ($rac['AuthnContextClassRef'] as $accr) { - XMLUtils::addString($e, Constants::NS_SAML, 'AuthnContextClassRef', $accr); - } - } + $this->requestedAuthnContext?->toXML($root); if ($this->scoping !== null) { $this->scoping->toXML($root); diff --git a/src/SAML2/XML/samlp/RequestedAuthnContext.php b/src/SAML2/XML/samlp/RequestedAuthnContext.php new file mode 100644 index 000000000..33740fb2c --- /dev/null +++ b/src/SAML2/XML/samlp/RequestedAuthnContext.php @@ -0,0 +1,125 @@ +requestedAuthnContexts; + } + + + /** + * Collect the value of the Comparison-property + * + * @return string|null + */ + public function getComparison(): ?string + { + return $this->Comparison; + } + + + /** + * Convert XML into a RequestedAuthnContext + * + * @param \DOMElement $xml The XML element we should load + * @return static + * + * @throws \SimpleSAML\XML\Exception\InvalidDOMElementException + * if the qualified name of the supplied element is wrong + */ + public static function fromXML(DOMElement $xml): static + { + Assert::same($xml->localName, 'RequestedAuthnContext', InvalidDOMElementException::class); + Assert::same($xml->namespaceURI, RequestedAuthnContext::NS, InvalidDOMElementException::class); + + return new static( + array_merge( + AuthnContextClassRef::getChildrenOfClass($xml), + AuthnContextDeclRef::getChildrenOfClass($xml), + ), + self::getOptionalAttribute($xml, 'Comparison', null), + ); + } + + + /** + * Convert this RequestedAuthnContext to XML. + * + * @param \DOMElement|null $parent The element we should append this RequestedAuthnContext to. + * @return \DOMElement + */ + public function toXML(DOMElement $parent = null): DOMElement + { + /** @psalm-var \DOMDocument $e->ownerDocument */ + $e = $this->instantiateParentElement($parent); + + foreach ($this->getRequestedAuthnContexts() as $context) { + $context->toXML($e); + } + + if ($this->getComparison() !== null) { + $e->setAttribute('Comparison', $this->getComparison()); + } + + return $e; + } +} diff --git a/tests/SAML2/AuthnRequestTest.php b/tests/SAML2/AuthnRequestTest.php index aa1832661..906667917 100644 --- a/tests/SAML2/AuthnRequestTest.php +++ b/tests/SAML2/AuthnRequestTest.php @@ -11,11 +11,13 @@ use SimpleSAML\SAML2\AuthnRequest; use SimpleSAML\SAML2\Constants as C; use SimpleSAML\SAML2\XML\saml\Audience; +use SimpleSAML\SAML2\XML\saml\AuthnContextClassRef; use SimpleSAML\SAML2\XML\saml\Issuer; use SimpleSAML\SAML2\XML\saml\NameID; use SimpleSAML\SAML2\XML\samlp\IDPEntry; use SimpleSAML\SAML2\XML\samlp\IDPList; use SimpleSAML\SAML2\XML\samlp\NameIDPolicy; +use SimpleSAML\SAML2\XML\samlp\RequestedAuthnContext; use SimpleSAML\SAML2\XML\samlp\RequesterID; use SimpleSAML\SAML2\XML\samlp\Scoping; use SimpleSAML\SAML2\Utils\XPath; @@ -32,13 +34,15 @@ class AuthnRequestTest extends TestCase public function testUnmarshalling(): void { $authnRequest = new AuthnRequest(); - $authnRequest->setRequestedAuthnContext([ - 'AuthnContextClassRef' => [ - 'accr1', - 'accr2', - ], - 'Comparison' => 'better', - ]); + $authnRequest->setRequestedAuthnContext( + new RequestedAuthnContext( + [ + new AuthnContextClassRef('accr1'), + new AuthnContextClassRef('accr2'), + ], + 'better', + ), + ); $authnRequestElement = $authnRequest->toUnsignedXML(); diff --git a/tests/SAML2/XML/samlp/RequestedAuthnContextTest.php b/tests/SAML2/XML/samlp/RequestedAuthnContextTest.php new file mode 100644 index 000000000..6c1eef716 --- /dev/null +++ b/tests/SAML2/XML/samlp/RequestedAuthnContextTest.php @@ -0,0 +1,133 @@ +assertEquals( + self::$xmlRepresentation->saveXML(self::$xmlRepresentation->documentElement), + strval($requestedAuthnContext), + ); + } + + + /** + */ + public function testMarshallingWithMixedContextsFails(): void + { + $authnContextDeclRef = new AuthnContextDeclRef('https://example.org/relative/path/to/document.xml'); + $authnContextClassRef = new AuthnContextClassRef(C::AC_PASSWORD_PROTECTED_TRANSPORT); + + $this->expectException(AssertionFailedException::class); + $this->expectExceptionMessage('You need either AuthnContextClassRef or AuthnContextDeclRef, not both.'); + + new RequestedAuthnContext([$authnContextClassRef, $authnContextDeclRef], 'exact'); + } + + + /** + */ + public function testMarshallingWithInvalidContentFails(): void + { + $authnContextDeclRef = new AuthnContextDeclRef('https://example.org/relative/path/to/document.xml'); + + $this->expectException(AssertionFailedException::class); + $this->expectExceptionMessage( + 'Expected an instance of any of "' . AuthnContextClassRef::class . '", "' . AuthnContextDeclRef::class . + '". Got: DOMDocument' + ); + + /** @psalm-suppress InvalidArgument */ + new RequestedAuthnContext( + [ + DOMDocumentFactory::fromString(''), + $authnContextDeclRef, + ], + 'exact', + ); + } + + + /** + */ + public function testUnmarshalling(): void + { + $requestedAuthnContext = RequestedAuthnContext::fromXML(self::$xmlRepresentation->documentElement); + + $this->assertEquals( + self::$xmlRepresentation->saveXML(self::$xmlRepresentation->documentElement), + strval($requestedAuthnContext), + ); + } + + + /** + */ + public function testUnmarshallingWithMixedContextsFails(): void + { + $samlNamespace = C::NS_SAML; + $samlpNamespace = C::NS_SAMLP; + + $document = DOMDocumentFactory::fromString(<< + urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport + https://example.org/relative/path/to/document.xml + +XML + ); + + $this->expectException(AssertionFailedException::class); + $this->expectExceptionMessage('You need either AuthnContextClassRef or AuthnContextDeclRef, not both.'); + + RequestedAuthnContext::fromXML($document->documentElement); + } +}