Use assertion id for session index

This was a privacy issue because this allows different SP to correlate users, defeating persistent and transient NameID mechanisms. #41
OpenConext · Jan 28, 2025 · d6f8235 · d6f8235
1 parent c72e17a
commit d6f8235
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 1 deletion.
diff --git a/library/EngineBlock/Corto/ProxyServer.php b/library/EngineBlock/Corto/ProxyServer.php
@@ -666,7 +666,7 @@ public function createEnhancedResponse(
 
         // Copy over the Authentication information because the IdP did the authentication, not us.
         $newAssertion->setAuthnInstant($sourceAssertion->getAuthnInstant());
-        $newAssertion->setSessionIndex($sourceAssertion->getSessionIndex());
+        $newAssertion->setSessionIndex($newAssertion->getId());
 
         $newAssertion->setAuthnContextClassRef($sourceAssertion->getAuthnContextClassRef());
         $newAssertion->setAuthnContextDeclRef($sourceAssertion->getAuthnContextDeclRef());

diff --git a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Context/MinkContext.php b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Context/MinkContext.php
@@ -159,6 +159,38 @@ public function theCollabPersonIdIsNotPresent()
         }
     }
 
+    /**
+     * @Then /^the SessionIndex should match the Assertion ID$/
+     */
+    public function theSessionIndexShouldMatchTheAssertionID()
+    {
+        $document = new DOMDocument();
+        $document->loadXML($this->getSession()->getPage()->getContent());
+        $xpathObj = new DOMXPath($document);
+        $xpathObj->registerNamespace('ds', XMLSecurityDSig::XMLDSIGNS);
+        $xpathObj->registerNamespace('mdui', Common::NS);
+        $xpathObj->registerNamespace('shibmd', Scope::NS);
+        $nodeListAssertion = $xpathObj->query('/samlp:Response/saml:Assertion[@ID]');
+        $nodeListAuthStatement = $xpathObj->query('/samlp:Response/saml:Assertion/saml:AuthnStatement[@SessionIndex]');
+
+        if ($nodeListAssertion->count() == 0) {
+            throw new ExpectationException('The assertion ID was not found', $this->getSession());
+        }
+
+        if ($nodeListAuthStatement->count() == 0) {
+            throw new ExpectationException('The SessionIndex wasnot found', $this->getSession());
+        }
+
+        $assertionID = $nodeListAssertion->item(0)->attributes->getNamedItem('ID')->value;
+        $sessionIndex = $nodeListAuthStatement->item(0)->attributes->getNamedItem('SessionIndex')->value;
+        if ($sessionIndex == "") {
+            throw new ExpectationException('The SessionIndex was empty', $this->getSession());
+        }
+        if ($assertionID !== $sessionIndex) {
+            throw new ExpectationException('The SessionIndex was not the same as the assertion ID', $this->getSession());
+        }
+    }
+
     /**
      * @Then /^the response should not match xpath \'([^\']*)\'$/
      */

diff --git a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/SessionIndex.feature b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/SessionIndex.feature
@@ -0,0 +1,19 @@
+Feature:
+  In order to have a privacy safe session index in the assertion
+  As EngineBlock
+  I need to set the assertion id as session index
+
+  Background:
+    Given an EngineBlock instance on "dev.openconext.local"
+    And no registered SPs
+    And no registered Idps
+    And an Identity Provider named "IP"
+    And a Service Provider named "SP"
+
+  Scenario: User logs in to SP, in that case the session index should be the assertion id
+    And SP "SP" does not require consent
+    When I log in at "SP"
+    And I pass through EngineBlock
+    And I pass through the IdP
+    And I pass through EngineBlock
+    And the SessionIndex should match the Assertion ID