From 4544580604dade605f909b5a497f8f7373b856d4 Mon Sep 17 00:00:00 2001
From: Tim van Dijen <tim.dijen@minbzk.nl>
Date: Thu, 6 May 2021 21:56:45 +0200
Subject: [PATCH] Identifiers fix #2 (#1460)

* Improve docs & handle scopeAttribute in a similar matter as we do for ScopeAttribute

* Add tests for the scoped scopeAttribute scenario
---
 modules/core/docs/authproc_pairwiseid.md      |  4 ++++
 modules/core/docs/authproc_subjectid.md       |  4 ++++
 modules/core/lib/Auth/Process/PairwiseID.php  |  3 +++
 modules/core/lib/Auth/Process/SubjectID.php   | 10 ++++++++
 .../core/lib/Auth/Process/PairwiseIDTest.php  | 24 +++++++++++++++++++
 .../core/lib/Auth/Process/SubjectIDTest.php   | 20 ++++++++++++++++
 6 files changed, 65 insertions(+)

diff --git a/modules/core/docs/authproc_pairwiseid.md b/modules/core/docs/authproc_pairwiseid.md
index 0fe15914c..75b20209f 100644
--- a/modules/core/docs/authproc_pairwiseid.md
+++ b/modules/core/docs/authproc_pairwiseid.md
@@ -7,6 +7,10 @@ http://docs.oasis-open.org/security/saml-subject-id-attr/v1.0/saml-subject-id-at
 This filter will take an attribute and a scope as input and transforms this into a anonymized and scoped
 identifier that is globally unique for a given user & service provider combination.
 
+Note:
+Since the subject-id is specified as single-value attribute, only the first value of `identifyingAttribute`
+ and `scopeAttribute` are considered.
+
 Examples
 --------
 
diff --git a/modules/core/docs/authproc_subjectid.md b/modules/core/docs/authproc_subjectid.md
index b29446f3b..85347c2ca 100644
--- a/modules/core/docs/authproc_subjectid.md
+++ b/modules/core/docs/authproc_subjectid.md
@@ -10,6 +10,10 @@ Note:
 -----
 If privacy is of your concern, you may want to use the PairwiseID-filter instead.
 
+Note:
+Since the subject-id is specified as single-value attribute, only the first value of `identifyingAttribute`
+ and `scopeAttribute` are considered.
+
 Examples
 --------
 
diff --git a/modules/core/lib/Auth/Process/PairwiseID.php b/modules/core/lib/Auth/Process/PairwiseID.php
index caf281e9c..b404491f6 100644
--- a/modules/core/lib/Auth/Process/PairwiseID.php
+++ b/modules/core/lib/Auth/Process/PairwiseID.php
@@ -20,6 +20,9 @@ use SimpleSAML\Utils;
  * This is generated from the attribute configured in 'identifyingAttribute' in the
  * authproc-configuration.
  *
+ * NOTE: since the subject-id is specified as single-value attribute, only the first value of `identifyingAttribute`
+ *       and `scopeAttribute` are considered.
+ *
  * Example - generate from attribute:
  * <code>
  * 'authproc' => [
diff --git a/modules/core/lib/Auth/Process/SubjectID.php b/modules/core/lib/Auth/Process/SubjectID.php
index 45233a096..5bd3b1cca 100644
--- a/modules/core/lib/Auth/Process/SubjectID.php
+++ b/modules/core/lib/Auth/Process/SubjectID.php
@@ -21,6 +21,9 @@ use SimpleSAML\Logger;
  * This is generated from the attribute configured in 'identifyingAttribute' in the
  * authproc-configuration.
  *
+ * NOTE: since the subject-id is specified as single-value attribute, only the first value of `identifyingAttribute`
+ *       and `scopeAttribute` are considered.
+ *
  * Example - generate from attribute:
  * <code>
  * 'authproc' => [
@@ -167,6 +170,13 @@ class SubjectID extends Auth\ProcessingFilter
 
         $scope = $state['Attributes'][$this->scopeAttribute][0];
         Assert::stringNotEmpty($scope, 'core' . static::NAME . ': \'scopeAttribute\' cannot be an empty string.');
+
+        // If the value is scoped, extract the scope from it
+        if (strpos($scope, '@') !== false) {
+            $scope = explode('@', $scope, 2);
+            $scope = $scope[1];
+        }
+
         Assert::regex(
             $scope,
             self::SCOPE_PATTERN,
diff --git a/tests/modules/core/lib/Auth/Process/PairwiseIDTest.php b/tests/modules/core/lib/Auth/Process/PairwiseIDTest.php
index 0ba291941..daa32fc4b 100644
--- a/tests/modules/core/lib/Auth/Process/PairwiseIDTest.php
+++ b/tests/modules/core/lib/Auth/Process/PairwiseIDTest.php
@@ -97,6 +97,30 @@ class PairwiseIDTest extends TestCase
     }
 
 
+    /**
+     * Test the most basic functionality, but with a scoped scope-attribute
+     */
+    public function testBasicScopedScope(): void
+    {
+        $config = ['identifyingAttribute' => 'uid', 'scopeAttribute' => 'scope'];
+        $request = [
+            'Attributes' => ['uid' => ['u=se-r2'], 'scope' => ['u=se-r2@ex-ample.org']],
+            'core:SP' => 'urn:sp',
+        ];
+        $result = self::processFilter($config, $request);
+        $attributes = $result['Attributes'];
+        $this->assertArrayHasKey(Constants::ATTR_PAIRWISE_ID, $attributes);
+        $this->assertMatchesRegularExpression(
+            PairwiseID::SPEC_PATTERN,
+            $attributes[Constants::ATTR_PAIRWISE_ID][0]
+        );
+        $this->assertEquals(
+            '53d4f7fe57fb597ada481e81e0f15048bc610774cbb5614ea38f08ea918ba199@ex-ample.org',
+            $attributes[Constants::ATTR_PAIRWISE_ID][0]
+        );
+    }
+
+
     /**
      * Test the most basic functionality on proxied request
      */
diff --git a/tests/modules/core/lib/Auth/Process/SubjectIDTest.php b/tests/modules/core/lib/Auth/Process/SubjectIDTest.php
index 62769234d..f090ffcc2 100644
--- a/tests/modules/core/lib/Auth/Process/SubjectIDTest.php
+++ b/tests/modules/core/lib/Auth/Process/SubjectIDTest.php
@@ -81,6 +81,26 @@ class SubjectIDTest extends TestCase
     }
 
 
+    /**
+     * Test the most basic functionality, but with a scoped scope-attribute
+     */
+    public function testScopedScope(): void
+    {
+        $config = ['identifyingAttribute' => 'uid', 'scopeAttribute' => 'scope'];
+        $request = [
+            'Attributes' => ['uid' => ['u=se-r2'], 'scope' => ['u=se-r2@ex-ample.org']],
+        ];
+        $result = self::processFilter($config, $request);
+        $attributes = $result['Attributes'];
+        $this->assertArrayHasKey(Constants::ATTR_SUBJECT_ID, $attributes);
+        $this->assertMatchesRegularExpression(
+            SubjectID::SPEC_PATTERN,
+            $attributes[Constants::ATTR_SUBJECT_ID][0]
+        );
+        $this->assertEquals('u=se-r2@ex-ample.org', $attributes[Constants::ATTR_SUBJECT_ID][0]);
+    }
+
+
     /**
      * Test that illegal characters in userID throws an exception.
      */
-- 
GitLab