Enable showing supported claims in OP discovery (#299)

Author: cicnavi

UPGRADE.md

@@ -35,7 +35,8 @@ https://openid.net/specs/openid-connect-core-1_0.html#RequestObject
 https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication
 
 ## New configuration options
-
+- (from v6.1) Show `claims_supported` claim in OP Discovery endpoint - you can now choose to show supported claims,
+as is recommended by OpenID Connect Discovery specification https://openid.net/specs/openid-connect-discovery-1_0.html.
 - (optional) Issuer - you can now override the issuer (OP identifier). If not set, it falls back to current scheme, host
 and optionally a port (as in all previous module versions).
 - (optional) Protocol caching adapter and its arguments

config/module_oidc.php.dist

@@ -78,7 +78,8 @@ $config = [
     // to use this attribute as the 'sub' claim since it designates unique identifier for the user).
     ModuleConfig::OPTION_AUTH_USER_IDENTIFIER_ATTRIBUTE => 'uid',
 
-    // The default translate table from SAML attributes to OIDC claims.
+    // The default translate table from SAML attributes to OIDC claims. If you don't want to support specific default
+    // claim, set it to an empty array.
     ModuleConfig::OPTION_AUTH_SAML_TO_OIDC_TRANSLATE_TABLE => [
         /*
          * The basic format is
@@ -243,6 +244,11 @@ $config = [
 //     ModuleConfig::OPTION_AUTH_FORCED_ACR_VALUE_FOR_COOKIE_AUTHENTICATION => '0',
     ModuleConfig::OPTION_AUTH_FORCED_ACR_VALUE_FOR_COOKIE_AUTHENTICATION => null,
 
+    // Choose if OP discovery document will include 'claims_supported' claim, which is recommended per OpenID Connect
+    // Discovery specification https://openid.net/specs/openid-connect-discovery-1_0.html. The list will include all
+    // claims for which "SAML attribute to OIDC claim translation" has been defined above.
+    ModuleConfig::OPTION_PROTOCOL_DISCOVERY_SHOW_CLAIMS_SUPPORTED => false,
+
     // Settings regarding Authentication Processing Filters.
     // Note: OIDC authN state array will not contain all the keys which are available during SAML authN,
     // like Service Provider metadata, etc.

src/ModuleConfig.php

@@ -81,6 +81,7 @@ class ModuleConfig
     final public const OPTION_PROTOCOL_CACHE_ADAPTER_ARGUMENTS = 'protocol_cache_adapter_arguments';
     final public const OPTION_PROTOCOL_USER_ENTITY_CACHE_DURATION = 'protocol_user_entity_cache_duration';
     final public const OPTION_PROTOCOL_CLIENT_ENTITY_CACHE_DURATION = 'protocol_client_entity_cache_duration';
+    final public const OPTION_PROTOCOL_DISCOVERY_SHOW_CLAIMS_SUPPORTED = 'protocol_discover_show_claims_supported';
     final public const OPTION_FEDERATION_PARTICIPATION_LIMIT_BY_TRUST_MARKS =
     'federation_participation_limit_by_trust_marks';
 
@@ -504,6 +505,14 @@ public function getProtocolClientEntityCacheDuration(): DateInterval
         );
     }
 
+    public function getProtocolDiscoveryShowClaimsSupported(): bool
+    {
+        return $this->config()->getOptionalBoolean(
+            self::OPTION_PROTOCOL_DISCOVERY_SHOW_CLAIMS_SUPPORTED,
+            false,
+        );
+    }
+
 
     /*****************************************************************************************************************
      * OpenID Federation related config.

src/Services/Container.php

@@ -189,12 +189,6 @@ public function __construct()
         );
         $this->services[TemplateFactory::class] = $templateFactory;
 
-        $opMetadataService = new OpMetadataService($moduleConfig);
-        $this->services[OpMetadataService::class] = $opMetadataService;
-
-        $metadataStorageHandler = MetaDataStorageHandler::getMetadataHandler();
-        $this->services[MetaDataStorageHandler::class] = $metadataStorageHandler;
-
         $claimSetEntityFactory = new ClaimSetEntityFactory();
         $this->services[ClaimSetEntityFactory::class] = $claimSetEntityFactory;
 
@@ -204,6 +198,12 @@ public function __construct()
         ))->build();
         $this->services[ClaimTranslatorExtractor::class] = $claimTranslatorExtractor;
 
+        $opMetadataService = new OpMetadataService($moduleConfig, $claimTranslatorExtractor);
+        $this->services[OpMetadataService::class] = $opMetadataService;
+
+        $metadataStorageHandler = MetaDataStorageHandler::getMetadataHandler();
+        $this->services[MetaDataStorageHandler::class] = $metadataStorageHandler;
+
         $processingChainFactory = new ProcessingChainFactory($moduleConfig);
         $this->services[ProcessingChainFactory::class] = $processingChainFactory;
 

src/Services/OpMetadataService.php

@@ -6,6 +6,7 @@
 
 use SimpleSAML\Module\oidc\Codebooks\RoutesEnum;
 use SimpleSAML\Module\oidc\ModuleConfig;
+use SimpleSAML\Module\oidc\Utils\ClaimTranslatorExtractor;
 use SimpleSAML\OpenID\Codebooks\ClaimsEnum;
 use SimpleSAML\OpenID\Codebooks\TokenEndpointAuthMethodsEnum;
 
@@ -24,6 +25,7 @@ class OpMetadataService
      */
     public function __construct(
         private readonly ModuleConfig $moduleConfig,
+        private readonly ClaimTranslatorExtractor $claimTranslatorExtractor,
     ) {
         $this->initMetadata();
     }
@@ -75,6 +77,11 @@ private function initMetadata(): void
         }
         $this->metadata[ClaimsEnum::BackChannelLogoutSupported->value] = true;
         $this->metadata[ClaimsEnum::BackChannelLogoutSessionSupported->value] = true;
+
+        if ($this->moduleConfig->getProtocolDiscoveryShowClaimsSupported()) {
+            $claimsSupported = $this->claimTranslatorExtractor->getSupportedClaims();
+            $this->metadata[ClaimsEnum::ClaimsSupported->value] = $claimsSupported;
+        }
     }
 
     /**

src/Utils/ClaimTranslatorExtractor.php

@@ -35,7 +35,7 @@ class ClaimTranslatorExtractor
     protected array $claimSets = [];
 
     /** @var string[] */
-    protected array $protectedClaims = ['openid', 'profile', 'email', 'address', 'phone'];
+    protected array $protectedScopes = ['openid', 'profile', 'email', 'address', 'phone'];
 
     protected array $translationTable = [
         'sub' => [
@@ -85,13 +85,13 @@ class ClaimTranslatorExtractor
             'preferredLanguage',
         ],
         'updated_at' => [
-            'type' => 'int',
+//            'type' => 'int',
         ],
         'email' => [
             'mail',
         ],
         'email_verified' => [
-            'type' => 'bool',
+//            'type' => 'bool',
         ],
         'address' => [
             'type' => 'json',
@@ -105,7 +105,7 @@ class ClaimTranslatorExtractor
             'homePhone',
         ],
         'phone_number_verified' => [
-            'type' => 'bool',
+//            'type' => 'bool',
             // Empty
         ],
     ];
@@ -198,7 +198,7 @@ public function addClaimSet(ClaimSetEntityInterface $claimSet): self
     {
         $scope = $claimSet->getScope();
 
-        if (in_array($scope, $this->protectedClaims, true) && isset($this->claimSets[$scope])) {
+        if (in_array($scope, $this->protectedScopes, true) && isset($this->claimSets[$scope])) {
             throw OidcServerException::serverError(
                 sprintf("%s is a protected scope and is pre-defined by the OpenID Connect specification.", $scope),
             );
@@ -352,4 +352,13 @@ private function extractAdditionalClaims(array $requestedClaims, array $claims):
             ARRAY_FILTER_USE_KEY,
         );
     }
+
+    /**
+     * Get supported claims for this OP. This will return all the claims for which the "SAML attribute to OIDC claim
+     * translation" has been defined in module config, meaning it is expected for OP to release those claims.
+     */
+    public function getSupportedClaims(): array
+    {
+        return array_keys(array_filter($this->translationTable));
+    }
 }

tests/unit/src/ModuleConfigTest.php

@@ -372,4 +372,15 @@ public function testCanGetProtocolCacheConfiguration(): void
         $this->assertInstanceOf(DateInterval::class, $this->sut()->getProtocolUserEntityCacheDuration());
         $this->assertInstanceOf(DateInterval::class, $this->sut()->getProtocolClientEntityCacheDuration());
     }
+
+    public function testCanGetProtocolDiscoveryShowClaimsSupported(): void
+    {
+        $this->assertFalse($this->sut()->getProtocolDiscoveryShowClaimsSupported());
+        $this->assertTrue(
+            $this->sut(
+                null,
+                [ModuleConfig::OPTION_PROTOCOL_DISCOVERY_SHOW_CLAIMS_SUPPORTED => true],
+            )->getProtocolDiscoveryShowClaimsSupported(),
+        );
+    }
 }

tests/unit/src/Services/OpMetadataServiceTest.php

@@ -10,13 +10,16 @@
 use SimpleSAML\Module\oidc\Codebooks\RoutesEnum;
 use SimpleSAML\Module\oidc\ModuleConfig;
 use SimpleSAML\Module\oidc\Services\OpMetadataService;
+use SimpleSAML\Module\oidc\Utils\ClaimTranslatorExtractor;
+use SimpleSAML\OpenID\Codebooks\ClaimsEnum;
 
 /**
  * @covers \SimpleSAML\Module\oidc\Services\OpMetadataService
  */
 class OpMetadataServiceTest extends TestCase
 {
     protected MockObject $moduleConfigMock;
+    protected MockObject $claimTranslatorExtractorMock;
 
     /**
      * @throws \Exception
@@ -46,14 +49,24 @@ public function setUp(): void
         $signer = $this->createMock(Rsa::class);
         $signer->method('algorithmId')->willReturn('RS256');
         $this->moduleConfigMock->method('getProtocolSigner')->willReturn($signer);
+
+        $this->claimTranslatorExtractorMock = $this->createMock(ClaimTranslatorExtractor::class);
     }
 
     /**
      * @throws \Exception
      */
-    protected function prepareMockedInstance(): OpMetadataService
-    {
-        return new OpMetadataService($this->moduleConfigMock);
+    protected function sut(
+        ?ModuleConfig $moduleConfig = null,
+        ?ClaimTranslatorExtractor $claimTranslatorExtractor = null,
+    ): OpMetadataService {
+        $moduleConfig = $moduleConfig ?? $this->moduleConfigMock;
+        $claimTranslatorExtractor = $claimTranslatorExtractor ?? $this->claimTranslatorExtractorMock;
+
+        return new OpMetadataService(
+            $moduleConfig,
+            $claimTranslatorExtractor,
+        );
     }
 
     /**
@@ -63,7 +76,7 @@ public function testItIsInitializable(): void
     {
         $this->assertInstanceOf(
             OpMetadataService::class,
-            $this->prepareMockedInstance(),
+            $this->sut(),
         );
     }
 
@@ -102,7 +115,23 @@ public function testItReturnsExpectedMetadata(): void
                 'backchannel_logout_supported' => true,
                 'backchannel_logout_session_supported' => true,
             ],
-            $this->prepareMockedInstance()->getMetadata(),
+            $this->sut()->getMetadata(),
+        );
+    }
+
+    public function testCanShowClaimsSupportedClaim(): void
+    {
+        $this->moduleConfigMock->method('getProtocolDiscoveryShowClaimsSupported')->willReturn(true);
+        $this->claimTranslatorExtractorMock->method('getSupportedClaims')->willReturn(['sample']);
+
+        $sut = $this->sut();
+        $this->assertArrayHasKey(
+            ClaimsEnum::ClaimsSupported->value,
+            $sut->getMetadata(),
+        );
+
+        $this->assertTrue(
+            in_array('sample', $sut->getMetadata()[ClaimsEnum::ClaimsSupported->value], true),
         );
     }
 }

tests/unit/src/Utils/ClaimTranslatorExtractorTest.php

@@ -272,4 +272,24 @@ public function testExtractRequestClaimsIdToken(): void
         $claims = $claimTranslator->extractAdditionalIdTokenClaims($requestClaims, ['displayName' => ['bob']]);
         $this->assertEquals(['name' => 'bob'], $claims);
     }
+
+    public function testCanGetSupportedClaims(): void
+    {
+        $translate = [
+            'custom' => [
+                'type' => 'int',
+                'custom_attr',
+            ],
+        ];
+
+        $this->assertTrue(in_array('custom', $this->mock([], $translate)->getSupportedClaims(), true));
+    }
+
+    public function testCanUnsetClaimWhichIsSupportedByDefault(): void
+    {
+        $this->assertTrue(in_array('nickname', $this->mock()->getSupportedClaims(), true));
+
+        $translate = ['nickname' => []];
+        $this->assertFalse(in_array('nickname', $this->mock([], $translate)->getSupportedClaims(), true));
+    }
 }