Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: validating elements and attributes using Xpath #108

Open
ctrlaltca opened this issue Feb 17, 2020 · 1 comment
Open

Proposal: validating elements and attributes using Xpath #108

ctrlaltca opened this issue Feb 17, 2020 · 1 comment
Labels
enhancement New feature or request

Comments

@ctrlaltca
Copy link

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
Copy link
Collaborator

simevo commented Feb 19, 2020

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 !)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants