From 0777e280f2349f8abe7427bf160cab8af405c11f Mon Sep 17 00:00:00 2001
From: Marko Ivancic <mivanci@srce.hr>
Date: Mon, 9 Jun 2025 12:19:35 +0200
Subject: [PATCH] Make sub claim value single value only

---
 src/Utils/ClaimTranslatorExtractor.php        | 30 +++++-
 .../Utils/ClaimTranslatorExtractorTest.php    | 94 +++++++++++++++++++
 2 files changed, 123 insertions(+), 1 deletion(-)

diff --git a/src/Utils/ClaimTranslatorExtractor.php b/src/Utils/ClaimTranslatorExtractor.php
index 599a7b04..9d536136 100644
--- a/src/Utils/ClaimTranslatorExtractor.php
+++ b/src/Utils/ClaimTranslatorExtractor.php
@@ -125,6 +125,33 @@ class ClaimTranslatorExtractor
         'sub_jwk',
     ];
 
+    /**
+     * As per https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims
+     */
+    final public const MANDATORY_SINGLE_VALUE_CLAIMS = [
+        'sub',
+        // TODO mivanci v7 Uncomment the rest of the claims, as this was a potential breaking change in v6.
+//        'name',
+//        'given_name',
+//        'family_name',
+//        'middle_name',
+//        'nickname',
+//        'preferred_username',
+//        'profile',
+//        'picture',
+//        'website',
+//        'email',
+//        'email_verified',
+//        'gender',
+//        'birthdate',
+//        'zoneinfo',
+//        'locale',
+//        'phone_number',
+//        'phone_number_verified',
+//        'address',
+//        'updated_at',
+    ];
+
     /**
      * ClaimTranslatorExtractor constructor.
      *
@@ -248,7 +275,8 @@ private function translateSamlAttributesToClaims(array $translationTable, array
             foreach ($attributes as $samlMatch) {
                 if (array_key_exists($samlMatch, $samlAttributes)) {
                     /** @psalm-suppress MixedAssignment, MixedArgument */
-                    $values = in_array($claim, $this->allowedMultiValueClaims, true) ?
+                    $values =  (!in_array($claim, self::MANDATORY_SINGLE_VALUE_CLAIMS, true)) &&
+                    in_array($claim, $this->allowedMultiValueClaims, true) ?
                     $samlAttributes[$samlMatch] :
                     current($samlAttributes[$samlMatch]);
                     /** @psalm-suppress MixedAssignment */
diff --git a/tests/unit/src/Utils/ClaimTranslatorExtractorTest.php b/tests/unit/src/Utils/ClaimTranslatorExtractorTest.php
index f5d4cba5..03e4beae 100644
--- a/tests/unit/src/Utils/ClaimTranslatorExtractorTest.php
+++ b/tests/unit/src/Utils/ClaimTranslatorExtractorTest.php
@@ -292,4 +292,98 @@ public function testCanUnsetClaimWhichIsSupportedByDefault(): void
         $translate = ['nickname' => []];
         $this->assertFalse(in_array('nickname', $this->mock([], $translate)->getSupportedClaims(), true));
     }
+
+    public function testCanReleaseMultiValueClaims(): void
+    {
+        $claimSet = new ClaimSetEntity(
+            'multiValueClaimsScope',
+            ['multiValueClaim'],
+        );
+
+        $translate = [
+            'multiValueClaim' => [
+                'multiValueAttribute',
+            ],
+        ];
+
+        $userAttributes = [
+            'multiValueAttribute' => ['1', '2', '3'],
+        ];
+
+
+        $claimTranslator = $this->mock([$claimSet], $translate, ['multiValueClaim']);
+
+        $releasedClaims = $claimTranslator->extract(
+            ['multiValueClaimsScope'],
+            $userAttributes,
+        );
+
+        $expectedClaims = [
+            'multiValueClaim' => ['1', '2', '3'],
+        ];
+
+        $this->assertSame($expectedClaims, $releasedClaims);
+    }
+
+    public function testWillReleaseSingleValueClaimsIfMultiValueNotAllowed(): void
+    {
+        $claimSet = new ClaimSetEntity(
+            'multiValueClaimsScope',
+            ['multiValueClaim'],
+        );
+
+
+        $translate = [
+            'multiValueClaim' => [
+                'multiValueAttribute',
+            ],
+        ];
+
+        $userAttributes = [
+            'multiValueAttribute' => ['1', '2', '3'],
+        ];
+
+        $claimTranslator = $this->mock([$claimSet], $translate, []);
+
+        $releasedClaims = $claimTranslator->extract(
+            ['multiValueClaimsScope'],
+            $userAttributes,
+        );
+
+        $expectedClaims = ['multiValueClaim' => '1'];
+
+        $this->assertSame($expectedClaims, $releasedClaims);
+    }
+
+    public function testWillReleaseSingleValueClaimsForMandatorySingleValueClaims(): void
+    {
+
+        // TODO mivanci v7 Test for mandatory single value claims in other scopes, as per
+        // \SimpleSAML\Module\oidc\Utils\ClaimTranslatorExtractor::MANDATORY_SINGLE_VALUE_CLAIMS
+        $claimSet = new ClaimSetEntity(
+            'customScopeWithSubClaim',
+            ['sub'],
+        );
+
+        $translate = [
+            'sub' => [
+                'subAttribute',
+            ],
+        ];
+
+        $userAttributes = [
+            'subAttribute' => ['1', '2', '3'],
+        ];
+
+        $claimTranslator = $this->mock([$claimSet], $translate, ['sub']);
+
+        $releasedClaims = $claimTranslator->extract(
+            ['openid'],
+            $userAttributes,
+        );
+
+        $expectedClaims = ['sub' => '1'];
+
+        $this->assertSame($expectedClaims, $releasedClaims);
+    }
 }