Proposal: validating elements and attributes using Xpath

[ctrlaltca]

Currently validation on the received xml is made using simple DOM lookups like DOMDocument::getElementsByTagName. While this technically works good enough to fool the SPID validator into thinking the xml is being validated and passing the tests, it's not really checking every aspect of the xml.

Take by example the first test for an element in a response object at Response.php#L54:

if ($xml->getElementsByTagName('Issuer')->length == 0) {
...

This is checking if an <Issuer> tag is present in the xml, but not validating if it's nested in the correct place in the xml.

So how do we correctly validate an xml? Usually by using a XSD (XML Schema Definition) file, supported by php with the DOMDocument::schemaValidate method. Unfortunately this method outputs some cryptic validation messages that are extremely precise but hard to interpret for users. Imho a possible compromose would be to use xpath queries to check for elements. First, a DOMXpath object is created on the DOMDocument object:

  $xpath = new \DOMXpath($doc);
  $xpath->registerNamespace('samlp', 'urn:oasis:names:tc:SAML:2.0:protocol');
  $xpath->registerNamespace('saml', 'urn:oasis:names:tc:SAML:2.0:assertion');

Then, this new object can be used to perform queries on the document similarly to how css selectors work

  $issuer = $xpath->query('/samlp:Response/saml:Issuer')

You can check for elements, for their specific path in the DOM tree, for specific attributes, for specific values, and so on.

[simevo]

I have created a feature branch to work on this: https://github.com/italia/spid-php-lib/tree/feature/issue-108-xpath, in there I have created a test case to validate a static Response; run just that test with:

./vendor/bin/phpunit tests/ValidTest.php

now tweak the test like this:

diff --git a/tests/ValidTest.php b/tests/ValidTest.php
index a31b16a..6def3b1 100644
--- a/tests/ValidTest.php
+++ b/tests/ValidTest.php
@@ -48,8 +48,6 @@ final class ValidTest extends PHPUnit\Framework\TestCase
     InResponseTo="_4d38c302617b5bf98951e65b4cf304711e2166df20" 
     IssueInstant="2015-01-29T10:01:03Z" 
     Destination="http://spid-sp.it">
-  <saml:Issuer NameQualifier="https://spidIdp.spidIdpProvider.it"
-      Format="urn:oasis:names:tc:SAML:2.0:nameid-format:entity">spididp.it</saml:Issuer>
   <ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
     ............. 
   </ds:Signature>

then run it again and get:

Exception: Invalid Issuer attribute, expected spididp.it but received WrongIssuer

what is happening is that it is picking up the Issuer element inside Assertion, where I set an incorrect Issuer (side note: we should add a test on that as well !)