Move to AddClaimsToIdTokenRule for ImplicitGrant

Author: cicnavi

src/Server/Grants/ImplicitGrant.php

@@ -12,7 +12,6 @@
 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;
@@ -276,19 +275,13 @@ 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());
-
+        // 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,
-            $addClaimsToIdToken,
+            $authorizationRequest->getAddClaimsToIdToken(),
             $addAccessTokenHashToIdToken,
             $authorizationRequest->getNonce(),
             $authorizationRequest->getAuthTime(),

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);
     }
 }

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

@@ -170,20 +170,18 @@ 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.
+     * The grant forwards the "add claims to ID Token" decision (made by AddClaimsToIdTokenRule and carried on the
+     * authorization request) to the ID Token builder. When it is true, the user's claims are released in the ID
+     * Token.
      */
-    public function testReleasesUserClaimsInIdTokenWhenClientConfiguredTo(): void
+    public function testReleasesUserClaimsInIdTokenWhenRequested(): 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->authorizationRequestMock->method('getAddClaimsToIdToken')->willReturn(true);
         $this->scopeRepositoryMock->method('finalizeScopes')->willReturn([$this->scopeEntityMock]);
 
         $idTokenMock = $this->createMock(IdToken::class);
@@ -206,18 +204,17 @@ public function testReleasesUserClaimsInIdTokenWhenClientConfiguredTo(): void
     }
 
     /**
-     * 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).
+     * When the decision is false, 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
+    public function testDoesNotReleaseUserClaimsInIdTokenWhenNotRequested(): 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);

tests/unit/src/Server/RequestRules/Rules/AddClaimsToIdTokenRuleTest.php

@@ -8,10 +8,12 @@
 use PHPUnit\Framework\MockObject\Stub;
 use PHPUnit\Framework\TestCase;
 use Psr\Http\Message\ServerRequestInterface;
+use SimpleSAML\Module\oidc\Entities\Interfaces\ClientEntityInterface;
 use SimpleSAML\Module\oidc\Helpers;
 use SimpleSAML\Module\oidc\Server\RequestRules\Result;
 use SimpleSAML\Module\oidc\Server\RequestRules\ResultBag;
 use SimpleSAML\Module\oidc\Server\RequestRules\Rules\AddClaimsToIdTokenRule;
+use SimpleSAML\Module\oidc\Server\RequestRules\Rules\ClientRule;
 use SimpleSAML\Module\oidc\Server\RequestRules\Rules\ResponseTypeRule;
 use SimpleSAML\Module\oidc\Server\ResponseModes\ResponseModeInterface;
 use SimpleSAML\Module\oidc\Services\LoggerService;
@@ -136,6 +138,57 @@ public static function invalidResponseTypeProvider(): array
         ];
     }
 
+    /**
+     * A client configured with the administrator-only `add_claims_to_id_token` option gets the claims released
+     * in the ID Token even for a response type that would not otherwise trigger it (e.g. `id_token token`).
+     *
+     * @throws \Throwable
+     */
+    public function testAddsClaimsWhenClientConfiguredEvenForNonIdTokenResponseType(): void
+    {
+        $this->resultBag->add(new Result(ResponseTypeRule::class, 'id_token token'));
+
+        $clientStub = $this->createStub(ClientEntityInterface::class);
+        $clientStub->method('getAddClaimsToIdToken')->willReturn(true);
+        $this->resultBag->add(new Result(ClientRule::class, $clientStub));
+
+        $result = $this->sut()->checkRule(
+            $this->requestStub,
+            $this->resultBag,
+            $this->loggerServiceStub,
+            [],
+            $this->responseModeStub,
+        ) ??
+        new Result(AddClaimsToIdTokenRule::class, null);
+
+        $this->assertTrue($result->getValue());
+    }
+
+    /**
+     * When neither the response type nor the client requests it, claims are not released in the ID Token.
+     *
+     * @throws \Throwable
+     */
+    public function testDoesNotAddClaimsWhenNeitherResponseTypeNorClientRequestIt(): void
+    {
+        $this->resultBag->add(new Result(ResponseTypeRule::class, 'id_token token'));
+
+        $clientStub = $this->createStub(ClientEntityInterface::class);
+        $clientStub->method('getAddClaimsToIdToken')->willReturn(false);
+        $this->resultBag->add(new Result(ClientRule::class, $clientStub));
+
+        $result = $this->sut()->checkRule(
+            $this->requestStub,
+            $this->resultBag,
+            $this->loggerServiceStub,
+            [],
+            $this->responseModeStub,
+        ) ??
+        new Result(AddClaimsToIdTokenRule::class, null);
+
+        $this->assertFalse($result->getValue());
+    }
+
     /**
      * @throws \Throwable
      * @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException