Enable releasing user claims in ID Token per-client

docs/6-oidc-upgrade.md

@@ -76,6 +76,24 @@ registration metadata (a filter names a PHP class executed on the OP, so
 honoring it from registration would be a remote code execution vector).
   - Clients can now be configured with a new property related to the above:
     - Authentication Processing Filters (`authproc`)
+- Clients can now be configured to release the user's claims directly in the ID
+Token. By default, whenever an access token is also issued (the authorization
+code flow, and the `id_token token` implicit flow) the user's (scope-derived)
+claims are only available from the UserInfo endpoint and are not included in the
+ID Token. Some clients, however, never call the UserInfo endpoint and rely solely
+on the ID Token to obtain user attributes. For such clients, this new per-client
+property makes the OP include the user's claims (resolved from the granted
+scopes) in the ID Token as well. It is disabled by default, so existing clients
+are unaffected; enable it only for clients that need it, as it opens some
+privacy challenges (for example, ID token ending up in access logs), and as it
+increases the ID Token size. (Note: for the bare `id_token` implicit response
+type there is no access token to call UserInfo with, so the claims are already
+included in the ID Token regardless of this property.) For security reasons,
+it can only be set by an administrator (via the admin UI) and is
+deliberately never accepted from client-supplied dynamic / OpenID Federation
+registration metadata.
+  - Clients can now be configured with a new property related to the above:
+    - Release user claims in ID Token (`add_claims_to_id_token`)
 - The encryption key (used to encrypt / decrypt artifacts like authorization
 codes and refresh tokens) can now optionally be set to a strong, pre-generated
 `\Defuse\Crypto\Key`, instead of always deriving it from the SimpleSAMLphp

docs/9-oidc-dcr-client-metadata.md

@@ -104,9 +104,10 @@ request omits is reset to its OP default (or removed), so the client must send t
 complete intended metadata set on every update. Server-managed and admin-only
 properties are preserved across the update — the client identifier and secret,
 `created_at`, the registration type, the registration access token, and in
-particular any administrator-set `authproc` (which a registering client can never
-set). This applies to Dynamic (DCR) registrations; manual (admin UI) and OpenID
-Federation registrations are unaffected.
+particular any administrator-set admin-only extra metadata, i.e. `authproc` and
+`add_claims_to_id_token` (which a registering client can never set). This applies
+to Dynamic (DCR) registrations; manual (admin UI) and OpenID Federation
+registrations are unaffected.
 
 ## `redirect_uris` constraints by `application_type`
 

src/Controllers/Admin/ClientController.php

@@ -429,6 +429,12 @@ protected function buildClientEntityFromFormData(
         $extraMetadata[ClientEntity::KEY_AUTH_PROC_FILTERS] = is_array($rawAuthProcFilters) ?
         $rawAuthProcFilters : [];
 
+        // Per-client "release user claims in ID Token" toggle. Like authproc, this is administrator-only
+        // (settable here, via the admin UI) and is never accepted from client-supplied registration metadata.
+        // See ClientEntity::ADMIN_ONLY_METADATA_KEYS.
+        $extraMetadata[ClientEntity::KEY_ADD_CLAIMS_TO_ID_TOKEN] =
+        (bool)($data[ClientEntity::KEY_ADD_CLAIMS_TO_ID_TOKEN] ?? false);
+
         return $this->clientEntityFactory->fromData(
             $identifier,
             $secret,

src/Entities/ClientEntity.php

@@ -66,6 +66,12 @@ class ClientEntity implements ClientEntityInterface
      * the extra metadata JSON blob.
      */
     public const string KEY_AUTH_PROC_FILTERS = 'authproc';
+    /**
+     * Whether all of the user's (scope-derived) claims should be released in the
+     * ID Token issued to this client, in addition to being available at the
+     * UserInfo endpoint. Stored as an entry inside the extra metadata JSON blob.
+     */
+    public const string KEY_ADD_CLAIMS_TO_ID_TOKEN = 'add_claims_to_id_token';
 
     /**
      * Client properties (metadata keys) which are "administrator-only":
@@ -80,6 +86,7 @@ class ClientEntity implements ClientEntityInterface
      */
     public const array ADMIN_ONLY_METADATA_KEYS = [
         self::KEY_AUTH_PROC_FILTERS,
+        self::KEY_ADD_CLAIMS_TO_ID_TOKEN,
     ];
 
 
@@ -285,6 +292,7 @@ public function toArray(): array
             ClaimsEnum::ApplicationType->value => $this->getApplicationType(),
             ClaimsEnum::Contacts->value => $this->getContacts(),
             self::KEY_AUTH_PROC_FILTERS => $this->getAuthProcFilters(),
+            self::KEY_ADD_CLAIMS_TO_ID_TOKEN => $this->getAddClaimsToIdToken(),
             self::KEY_REGISTRATION_ACCESS_TOKEN => $this->registrationAccessToken,
         ];
     }
@@ -513,6 +521,25 @@ public function getAuthProcFilters(): array
         return is_array($authProcFilters) ? $authProcFilters : [];
     }
 
+    /**
+     * Whether all of the user's (scope-derived) claims should be released in the
+     * ID Token issued to this client. By default (false) such claims are only
+     * available from the UserInfo endpoint in the authorization code flow; some
+     * clients never call UserInfo and rely solely on the ID Token, which is what
+     * this per-client, administrator-only option accommodates.
+     */
+    public function getAddClaimsToIdToken(): bool
+    {
+        if (!is_array($this->extraMetadata)) {
+            return false;
+        }
+
+        return filter_var(
+            $this->extraMetadata[self::KEY_ADD_CLAIMS_TO_ID_TOKEN] ?? false,
+            FILTER_VALIDATE_BOOLEAN,
+        );
+    }
+
     /**
      * @return string[]
      */

src/Entities/Interfaces/ClientEntityInterface.php

@@ -138,4 +138,9 @@ public function getContacts(): array;
      * @return array
      */
     public function getAuthProcFilters(): array;
+
+    /**
+     * Whether the user's (scope-derived) claims should be released in the ID Token issued to this client.
+     */
+    public function getAddClaimsToIdToken(): bool;
 }

src/Forms/ClientForm.php

@@ -448,6 +448,9 @@ public function getValues(string|object|bool|null $returnType = null, ?array $co
 
         $values[ClaimsEnum::RequireAuthTime->value] = (bool)($values[ClaimsEnum::RequireAuthTime->value] ?? false);
 
+        $values[ClientEntity::KEY_ADD_CLAIMS_TO_ID_TOKEN] =
+        (bool)($values[ClientEntity::KEY_ADD_CLAIMS_TO_ID_TOKEN] ?? false);
+
         /** @var mixed $defaultAcrValues */
         $defaultAcrValues = $values[ClaimsEnum::DefaultAcrValues->value] ?? null;
         $defaultAcrValues = is_array($defaultAcrValues) ? $defaultAcrValues : [];
@@ -598,6 +601,9 @@ public function setDefaults(object|array $values, bool $erase = false): static
 
         $values[ClaimsEnum::RequireAuthTime->value] = (bool)($values[ClaimsEnum::RequireAuthTime->value] ?? false);
 
+        $values[ClientEntity::KEY_ADD_CLAIMS_TO_ID_TOKEN] =
+        (bool)($values[ClientEntity::KEY_ADD_CLAIMS_TO_ID_TOKEN] ?? false);
+
         /** @var mixed $defaultAcrValues */
         $defaultAcrValues = $values[ClaimsEnum::DefaultAcrValues->value] ?? null;
         $defaultAcrValues = is_array($defaultAcrValues) ? $defaultAcrValues : [];
@@ -758,6 +764,11 @@ protected function buildForm(): void
 
         $this->addCheckbox(ClaimsEnum::RequireAuthTime->value, Translate::noop('Require auth_time in ID Token'));
 
+        $this->addCheckbox(
+            ClientEntity::KEY_ADD_CLAIMS_TO_ID_TOKEN,
+            Translate::noop('Release User Claims in ID Token'),
+        );
+
         // Bound to the OP's supported ACRs (acr_values_supported). When the OP advertises no ACRs, this has no
         // items and the field is hidden in the template (a per-client default ACR cannot do anything in that case).
         $this->addMultiSelect(

src/Server/Grants/ImplicitGrant.php

@@ -275,6 +275,9 @@ private function completeOidcAuthorizationRequest(AuthorizationRequest $authoriz
             $responseParams['expires_in'] = $accessToken->getExpiryDateTime()->getTimestamp() - time();
         }
 
+        // Whether to release the user's claims in the ID Token is decided by AddClaimsToIdTokenRule (based on the
+        // response type and the per-client `add_claims_to_id_token` option) and carried on the authorization
+        // request; see validateAuthorizationRequestWithRequestRules().
         $idToken = $this->idTokenBuilder->buildFor(
             $user,
             $accessToken,

src/Server/RequestRules/Rules/AddClaimsToIdTokenRule.php

@@ -5,6 +5,7 @@
 namespace SimpleSAML\Module\oidc\Server\RequestRules\Rules;
 
 use Psr\Http\Message\ServerRequestInterface;
+use SimpleSAML\Module\oidc\Entities\Interfaces\ClientEntityInterface;
 use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultBagInterface;
 use SimpleSAML\Module\oidc\Server\RequestRules\Result;
 use SimpleSAML\Module\oidc\Server\ResponseModes\QueryResponseMode;
@@ -13,6 +14,14 @@
 use SimpleSAML\OpenID\Codebooks\HttpMethodsEnum;
 
 /**
+ * Decides whether the user's (scope-derived) claims should be released in the ID Token.
+ *
+ * This is true when either:
+ *  - the response type is exactly `id_token` (there is no access token, so the client cannot obtain the claims
+ *    from the UserInfo endpoint and they must go in the ID Token); or
+ *  - the client is configured with the administrator-only `add_claims_to_id_token` option (the client wants its
+ *    claims in the ID Token regardless, e.g. because it never calls the UserInfo endpoint).
+ *
  * @extends AbstractRule<bool>
  */
 class AddClaimsToIdTokenRule extends AbstractRule
@@ -34,6 +43,15 @@ public function checkRule(
     ): ?Result {
         $responseType = $currentResultBag->getOrFail(ResponseTypeRule::class)->getValue();
 
-        return new Result($this->getKey(), $responseType === "id_token");
+        $addClaimsToIdToken = $responseType === "id_token";
+
+        // Honor the per-client option. The client is resolved by ClientRule, which is predefined in the result bag
+        // before this rule runs; if it is not available for some reason, fall back to the response-type decision.
+        $client = $currentResultBag->get(ClientRule::class)?->getValue();
+        if ($client instanceof ClientEntityInterface && $client->getAddClaimsToIdToken()) {
+            $addClaimsToIdToken = true;
+        }
+
+        return new Result($this->getKey(), $addClaimsToIdToken);
     }
 }

src/Server/ResponseTypes/TokenResponse.php

@@ -21,6 +21,7 @@
 use League\OAuth2\Server\ResponseTypes\BearerTokenResponse;
 use RuntimeException;
 use SimpleSAML\Module\oidc\Entities\AccessTokenEntity;
+use SimpleSAML\Module\oidc\Entities\ClientEntity;
 use SimpleSAML\Module\oidc\Repositories\Interfaces\IdentityProviderInterface;
 use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException;
 use SimpleSAML\Module\oidc\Server\ResponseTypes\Interfaces\AcrResponseTypeInterface;
@@ -116,11 +117,17 @@ protected function prepareIdTokenExtraParam(AccessTokenEntity $accessToken): arr
             throw OidcServerException::accessDenied('No user available for provided user identifier.');
         }
 
+        // Release the user's (scope-derived) claims in the ID Token when the client is configured to (the
+        // administrator-only `add_claims_to_id_token` property). Otherwise such claims remain available only at
+        // the UserInfo endpoint in the authorization code flow.
+        $client = $accessToken->getClient();
+        $addClaimsToIdToken = $client instanceof ClientEntity && $client->getAddClaimsToIdToken();
+
         //$token = $this->idTokenBuilder->build(
         $token = $this->idTokenBuilder->buildFor(
             $userEntity,
             $accessToken,
-            false,
+            $addClaimsToIdToken,
             true,
             $this->getNonce(),
             $this->getAuthTime(),

templates/clients/includes/form.twig

@@ -223,6 +223,12 @@
             {% trans %}When enabled, the auth_time claim is always included in ID Tokens issued to this Client.{% endtrans %}
         </span>
 
+        <label for="">{{ 'Release User Claims in ID Token'|trans }}</label>
+        {{ form.add_claims_to_id_token.control | raw }}
+        <span class="pure-form-message">
+            {% trans %}When enabled, the user's claims (resolved from the granted scopes) are included directly in the ID Token issued to this Client, in addition to being available from the UserInfo endpoint. This is useful for clients that do not call the UserInfo endpoint and rely solely on the ID Token to obtain user attributes. Leave it disabled (the default) unless a client needs it, as it opens some privacy challenges (for example, ID token ending up in access logs), and as it increases the ID Token size. For security reasons, this can only be set here (by an administrator) and is never accepted from dynamic / federation client registration.{% endtrans %}
+        </span>
+
         {% if form.hasConfiguredAcrValues() %}
         <label for="frm-default_acr_values">{{ 'Default ACR Values'|trans }}</label>
         {{ form.default_acr_values.control | raw }}

templates/clients/show.twig

@@ -305,6 +305,14 @@
                     {{ client.requireAuthTime ? 'Yes' : 'No' }}
                 </td>
             </tr>
+            <tr>
+                <td class="client-col col-property">
+                    {{ 'Release User Claims in ID Token'|trans }}
+                </td>
+                <td>
+                    {{ client.addClaimsToIdToken ? 'Yes' : 'No' }}
+                </td>
+            </tr>
             <tr>
                 <td class="client-col col-property">
                     {{ 'Default ACR Values'|trans }}

tests/unit/src/Entities/ClientEntityTest.php

@@ -246,6 +246,7 @@ public function testCanExportAsArray(): void
                 'application_type' => null,
                 'contacts' => [],
                 'authproc' => [],
+                'add_claims_to_id_token' => false,
                 'registration_access_token' => null,
             ],
         );
@@ -295,6 +296,46 @@ public function testCanGetAuthProcFilters(): void
         $this->assertSame($authProcFilters, $clientEntity->toArray()[ClientEntity::KEY_AUTH_PROC_FILTERS]);
     }
 
+    /**
+     * @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException
+     * @throws \JsonException
+     */
+    public function testCanGetAddClaimsToIdToken(): void
+    {
+        // No extra metadata -> disabled by default.
+        $this->assertFalse($this->mock()->getAddClaimsToIdToken());
+
+        $clientEntity = new ClientEntity(
+            $this->id,
+            $this->secret,
+            $this->name,
+            $this->description,
+            $this->redirectUri,
+            $this->scopes,
+            $this->isEnabled,
+            $this->isConfidential,
+            $this->authSource,
+            $this->owner,
+            $this->postLogoutRedirectUri,
+            $this->backChannelLogoutUri,
+            $this->entityIdentifier,
+            $this->clientRegistrationTypes,
+            $this->federationJwks,
+            $this->jwks,
+            $this->jwksUri,
+            $this->signedJwksUri,
+            $this->registrationType,
+            $this->updatedAt,
+            $this->createdAt,
+            $this->expiresAt,
+            $this->isGeneric,
+            [ClientEntity::KEY_ADD_CLAIMS_TO_ID_TOKEN => true],
+        );
+
+        $this->assertTrue($clientEntity->getAddClaimsToIdToken());
+        $this->assertTrue($clientEntity->toArray()[ClientEntity::KEY_ADD_CLAIMS_TO_ID_TOKEN]);
+    }
+
     public function testEnforcementGettersReturnRawRegisteredValues(): void
     {
         // v7 transition: when not registered, these getters return the raw "unset" value (empty / null) rather

tests/unit/src/Factories/Entities/ClientEntityFactoryTest.php

@@ -237,6 +237,55 @@ public function testFromRegistrationDataPreservesAdminSetAuthProcFiltersAndIgnor
         $this->assertSame($adminSetFilters, $client->getAuthProcFilters());
     }
 
+    /**
+     * The administrator-only "release user claims in ID Token" property must NEVER be honored when supplied
+     * through client registration metadata; an untrusted client must not be able to force its own claims into
+     * the ID Token. It can only be set by an administrator via the admin UI / API.
+     *
+     * @throws \SimpleSAML\Error\ConfigurationError
+     * @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException
+     */
+    public function testFromRegistrationDataIgnoresAdminOnlyAddClaimsToIdToken(): void
+    {
+        $client = $this->sut()->fromRegistrationData(
+            [
+                ClaimsEnum::RedirectUris->value => ['https://example.org/cb'],
+                // Client tries to opt itself into ID Token claim release.
+                ClientEntity::KEY_ADD_CLAIMS_TO_ID_TOKEN => true,
+            ],
+            RegistrationTypeEnum::FederatedAutomatic,
+        );
+
+        $this->assertFalse($client->getAddClaimsToIdToken());
+    }
+
+    /**
+     * An administrator-set "release user claims in ID Token" value on an existing client must be preserved
+     * across re-registration, and must not be overridable by the (untrusted) registration metadata.
+     *
+     * @throws \SimpleSAML\Error\ConfigurationError
+     * @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException
+     */
+    public function testFromRegistrationDataPreservesAdminSetAddClaimsToIdTokenAndIgnoresSuppliedOnes(): void
+    {
+        $existingClient = $this->createMock(ClientEntity::class);
+        $existingClient->method('getExtraMetadata')->willReturn(
+            [ClientEntity::KEY_ADD_CLAIMS_TO_ID_TOKEN => true],
+        );
+
+        $client = $this->sut()->fromRegistrationData(
+            [
+                ClaimsEnum::RedirectUris->value => ['https://example.org/cb'],
+                // Attempt to turn the admin-set value off via registration metadata.
+                ClientEntity::KEY_ADD_CLAIMS_TO_ID_TOKEN => false,
+            ],
+            RegistrationTypeEnum::FederatedAutomatic,
+            existingClient: $existingClient,
+        );
+
+        $this->assertTrue($client->getAddClaimsToIdToken());
+    }
+
     /**
      * The behavioral default metadata (default_max_age, require_auth_time, default_acr_values) and informational
      * metadata (initiate_login_uri, software_id, software_version) are persisted from a registration request.

tests/unit/src/Forms/ClientFormTest.php

@@ -434,4 +434,21 @@ public function testSetDefaultsAndGetValuesRoundTripAuthProcFilters(): void
             $sut->getValues()[ClientEntity::KEY_AUTH_PROC_FILTERS],
         );
     }
+
+    public function testAddClaimsToIdTokenDefaultsToFalse(): void
+    {
+        $this->assertFalse($this->sut()->getValues()[ClientEntity::KEY_ADD_CLAIMS_TO_ID_TOKEN]);
+    }
+
+    public function testSetDefaultsAndGetValuesRoundTripAddClaimsToIdToken(): void
+    {
+        $this->sspBridgeAuthSourceMock->method('getSources')->willReturn(['default-sp']);
+
+        $data = $this->clientDataSample;
+        $data[ClientEntity::KEY_ADD_CLAIMS_TO_ID_TOKEN] = true;
+
+        $sut = $this->sut()->setDefaults($data);
+
+        $this->assertTrue($sut->getValues()[ClientEntity::KEY_ADD_CLAIMS_TO_ID_TOKEN]);
+    }
 }