Also enable for id_token token response type

Author: cicnavi

docs/6-oidc-upgrade.md

@@ -77,16 +77,19 @@ 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, in the authorization code flow the user's (scope-derived)
+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 increases the ID
-Token size. For security reasons, it can only be set by an administrator (via the
-admin UI / API) and is deliberately never accepted from client-supplied dynamic /
-OpenID Federation registration metadata.
+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 / API) 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

src/Server/Grants/ImplicitGrant.php

@@ -12,6 +12,7 @@
 use Psr\Http\Message\ServerRequestInterface;
 use RuntimeException;
 use SimpleSAML\Module\oidc\Entities\AccessTokenEntity;
+use SimpleSAML\Module\oidc\Entities\ClientEntity;
 use SimpleSAML\Module\oidc\Entities\Interfaces\EntityStringRepresentationInterface;
 use SimpleSAML\Module\oidc\Entities\UserEntity;
 use SimpleSAML\Module\oidc\Factories\Entities\AccessTokenEntityFactory;
@@ -275,10 +276,19 @@ private function completeOidcAuthorizationRequest(AuthorizationRequest $authoriz
             $responseParams['expires_in'] = $accessToken->getExpiryDateTime()->getTimestamp() - time();
         }
 
+        // Decide whether the user's (scope-derived) claims go into the ID Token. The response-type-driven decision
+        // (AddClaimsToIdTokenRule) already requests them for response_type=id_token, where there is no access token
+        // to call the UserInfo endpoint with. For id_token token the claims would otherwise be available only at
+        // UserInfo, so we additionally honor the per-client, administrator-only `add_claims_to_id_token` option,
+        // matching the authorization code flow (see TokenResponse::prepareIdTokenExtraParam()).
+        $client = $authorizationRequest->getClient();
+        $addClaimsToIdToken = $authorizationRequest->getAddClaimsToIdToken()
+        || ($client instanceof ClientEntity && $client->getAddClaimsToIdToken());
+
         $idToken = $this->idTokenBuilder->buildFor(
             $user,
             $accessToken,
-            $authorizationRequest->getAddClaimsToIdToken(),
+            $addClaimsToIdToken,
             $addAccessTokenHashToIdToken,
             $authorizationRequest->getNonce(),
             $authorizationRequest->getAuthTime(),

tests/unit/src/Server/Grants/ImplicitGrantTest.php

@@ -23,6 +23,7 @@
 use SimpleSAML\Module\oidc\Services\IdTokenBuilder;
 use SimpleSAML\Module\oidc\Services\LoggerService;
 use SimpleSAML\Module\oidc\Utils\RequestParamsResolver;
+use SimpleSAML\OpenID\Core\IdToken;
 
 #[CoversClass(ImplicitGrant::class)]
 class ImplicitGrantTest extends TestCase
@@ -168,6 +169,76 @@ public function testCanCompleteAuthorizationRequest(): void
         );
     }
 
+    /**
+     * For id_token token the AddClaimsToIdTokenRule does not request the claims (the rule only matches the exact
+     * "id_token" response type), but a client configured with the administrator-only `add_claims_to_id_token`
+     * option still gets the user's claims released in the ID Token.
+     */
+    public function testReleasesUserClaimsInIdTokenWhenClientConfiguredTo(): void
+    {
+        $this->authorizationRequestMock->method('getUser')->willReturn($this->userEntityMock);
+        $this->authorizationRequestMock->method('getRedirectUri')->willReturn('redirectUri');
+        $this->authorizationRequestMock->method('isAuthorizationApproved')->willReturn(true);
+        $this->authorizationRequestMock->method('getScopes')->willReturn([$this->scopeEntityMock]);
+        $this->authorizationRequestMock->method('getClient')->willReturn($this->clientEntityMock);
+        // Response-type rule did NOT request the claims (e.g. id_token token), but the client opts in.
+        $this->authorizationRequestMock->method('getAddClaimsToIdToken')->willReturn(false);
+        $this->clientEntityMock->method('getAddClaimsToIdToken')->willReturn(true);
+        $this->scopeRepositoryMock->method('finalizeScopes')->willReturn([$this->scopeEntityMock]);
+
+        $idTokenMock = $this->createMock(IdToken::class);
+        $idTokenMock->method('getToken')->willReturn('token');
+        $this->idTokenBuilderMock->expects($this->once())
+            ->method('buildFor')
+            ->with(
+                $this->anything(),
+                $this->anything(),
+                true, // $addClaimsFromScopes
+                $this->anything(),
+                $this->anything(),
+                $this->anything(),
+                $this->anything(),
+                $this->anything(),
+            )
+            ->willReturn($idTokenMock);
+
+        $this->sut()->completeAuthorizationRequest($this->authorizationRequestMock);
+    }
+
+    /**
+     * When neither the response type nor the client requests it, the user's claims are not released in the ID
+     * Token (they remain available at the UserInfo endpoint via the issued access token).
+     */
+    public function testDoesNotReleaseUserClaimsInIdTokenByDefault(): void
+    {
+        $this->authorizationRequestMock->method('getUser')->willReturn($this->userEntityMock);
+        $this->authorizationRequestMock->method('getRedirectUri')->willReturn('redirectUri');
+        $this->authorizationRequestMock->method('isAuthorizationApproved')->willReturn(true);
+        $this->authorizationRequestMock->method('getScopes')->willReturn([$this->scopeEntityMock]);
+        $this->authorizationRequestMock->method('getClient')->willReturn($this->clientEntityMock);
+        $this->authorizationRequestMock->method('getAddClaimsToIdToken')->willReturn(false);
+        $this->clientEntityMock->method('getAddClaimsToIdToken')->willReturn(false);
+        $this->scopeRepositoryMock->method('finalizeScopes')->willReturn([$this->scopeEntityMock]);
+
+        $idTokenMock = $this->createMock(IdToken::class);
+        $idTokenMock->method('getToken')->willReturn('token');
+        $this->idTokenBuilderMock->expects($this->once())
+            ->method('buildFor')
+            ->with(
+                $this->anything(),
+                $this->anything(),
+                false, // $addClaimsFromScopes
+                $this->anything(),
+                $this->anything(),
+                $this->anything(),
+                $this->anything(),
+                $this->anything(),
+            )
+            ->willReturn($idTokenMock);
+
+        $this->sut()->completeAuthorizationRequest($this->authorizationRequestMock);
+    }
+
     public function testCanValidateAuthorizationRequestWithRequestRules(): void
     {
         $this->markTestIncomplete('RequestRulesManager needs to be refactored so it can be strongly typed.');