From d6f823538c18ae093a0d669569f66420e19e7b40 Mon Sep 17 00:00:00 2001
From: Bas <bstrooband@ibuildings.nl>
Date: Tue, 28 Jan 2025 11:33:59 +0100
Subject: [PATCH] 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.
https://github.com/OpenConext/OpenConext-engineblock/issues/41
---
 library/EngineBlock/Corto/ProxyServer.php     |  2 +-
 .../Features/Context/MinkContext.php          | 32 +++++++++++++++++++
 .../Features/SessionIndex.feature             | 19 +++++++++++
 3 files changed, 52 insertions(+), 1 deletion(-)
 create mode 100644 src/OpenConext/EngineBlockFunctionalTestingBundle/Features/SessionIndex.feature

diff --git a/library/EngineBlock/Corto/ProxyServer.php b/library/EngineBlock/Corto/ProxyServer.php
index 1741a253e6..7bddc9f74f 100644
--- 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
index d3908179f7..97c7b70970 100644
--- 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
new file mode 100644
index 0000000000..4b3af8d3f7
--- /dev/null
+++ 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