diff --git a/docs/6-oidc-upgrade.md b/docs/6-oidc-upgrade.md index 63296787..82a02e4f 100644 --- a/docs/6-oidc-upgrade.md +++ b/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 diff --git a/docs/9-oidc-dcr-client-metadata.md b/docs/9-oidc-dcr-client-metadata.md index a2898b30..d91b099b 100644 --- a/docs/9-oidc-dcr-client-metadata.md +++ b/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` diff --git a/src/Controllers/Admin/ClientController.php b/src/Controllers/Admin/ClientController.php index 09510adc..a5bddcab 100644 --- a/src/Controllers/Admin/ClientController.php +++ b/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, diff --git a/src/Entities/ClientEntity.php b/src/Entities/ClientEntity.php index f7772452..d2ae79d4 100644 --- a/src/Entities/ClientEntity.php +++ b/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[] */ diff --git a/src/Entities/Interfaces/ClientEntityInterface.php b/src/Entities/Interfaces/ClientEntityInterface.php index 4410c468..8e8a6989 100644 --- a/src/Entities/Interfaces/ClientEntityInterface.php +++ b/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; } diff --git a/src/Forms/ClientForm.php b/src/Forms/ClientForm.php index 45381f21..401b43c4 100644 --- a/src/Forms/ClientForm.php +++ b/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( diff --git a/src/Server/Grants/ImplicitGrant.php b/src/Server/Grants/ImplicitGrant.php index 7690c99c..533a8076 100644 --- a/src/Server/Grants/ImplicitGrant.php +++ b/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, diff --git a/src/Server/RequestRules/Rules/AddClaimsToIdTokenRule.php b/src/Server/RequestRules/Rules/AddClaimsToIdTokenRule.php index 5fd6473a..eb22cc98 100644 --- a/src/Server/RequestRules/Rules/AddClaimsToIdTokenRule.php +++ b/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 */ 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); } } diff --git a/src/Server/ResponseTypes/TokenResponse.php b/src/Server/ResponseTypes/TokenResponse.php index 000331ac..a8a5dedf 100644 --- a/src/Server/ResponseTypes/TokenResponse.php +++ b/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(), diff --git a/templates/clients/includes/form.twig b/templates/clients/includes/form.twig index 3d55e6c0..f43165e2 100644 --- a/templates/clients/includes/form.twig +++ b/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 %} + + {{ form.add_claims_to_id_token.control | raw }} + + {% 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 %} + + {% if form.hasConfiguredAcrValues() %} {{ form.default_acr_values.control | raw }} diff --git a/templates/clients/show.twig b/templates/clients/show.twig index 418428ed..80e979a9 100644 --- a/templates/clients/show.twig +++ b/templates/clients/show.twig @@ -305,6 +305,14 @@ {{ client.requireAuthTime ? 'Yes' : 'No' }} + + + {{ 'Release User Claims in ID Token'|trans }} + + + {{ client.addClaimsToIdToken ? 'Yes' : 'No' }} + + {{ 'Default ACR Values'|trans }} diff --git a/tests/unit/src/Entities/ClientEntityTest.php b/tests/unit/src/Entities/ClientEntityTest.php index b4c9ade8..dd4134d1 100644 --- a/tests/unit/src/Entities/ClientEntityTest.php +++ b/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 diff --git a/tests/unit/src/Factories/Entities/ClientEntityFactoryTest.php b/tests/unit/src/Factories/Entities/ClientEntityFactoryTest.php index 1d425daf..5ed1936b 100644 --- a/tests/unit/src/Factories/Entities/ClientEntityFactoryTest.php +++ b/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. diff --git a/tests/unit/src/Forms/ClientFormTest.php b/tests/unit/src/Forms/ClientFormTest.php index abea9800..d740c95c 100644 --- a/tests/unit/src/Forms/ClientFormTest.php +++ b/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]); + } } diff --git a/tests/unit/src/Server/Grants/ImplicitGrantTest.php b/tests/unit/src/Server/Grants/ImplicitGrantTest.php index 316bb60c..2142c5c8 100644 --- a/tests/unit/src/Server/Grants/ImplicitGrantTest.php +++ b/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,73 @@ public function testCanCompleteAuthorizationRequest(): void ); } + /** + * 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 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); + $this->authorizationRequestMock->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 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 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->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.'); diff --git a/tests/unit/src/Server/RequestRules/Rules/AddClaimsToIdTokenRuleTest.php b/tests/unit/src/Server/RequestRules/Rules/AddClaimsToIdTokenRuleTest.php index 121dc7e2..0103bf94 100644 --- a/tests/unit/src/Server/RequestRules/Rules/AddClaimsToIdTokenRuleTest.php +++ b/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 diff --git a/tests/unit/src/Server/ResponseTypes/TokenResponseTest.php b/tests/unit/src/Server/ResponseTypes/TokenResponseTest.php index 79058fca..9bcd7c07 100644 --- a/tests/unit/src/Server/ResponseTypes/TokenResponseTest.php +++ b/tests/unit/src/Server/ResponseTypes/TokenResponseTest.php @@ -133,11 +133,13 @@ protected function setUp(): void $this->idTokenMock = $this->createMock(IdToken::class); } - protected function prepareMockedInstance(): TokenResponse + protected function prepareMockedInstance(?IdTokenBuilder $idTokenBuilder = null): TokenResponse { + $idTokenBuilder ??= $this->idTokenBuilder; + $tokenResponse = new TokenResponse( $this->identityProviderMock, - $this->idTokenBuilder, + $idTokenBuilder, $this->privateKey, $this->loggerMock, ); @@ -217,6 +219,73 @@ public function testItCanGenerateResponseWithIndividualRequestedClaims(): void $this->assertTrue($this->shouldHaveValidIdToken($body, ['name' => 'Homer Simpson'])); } + /** + * When the client is configured to release user claims in the ID Token (admin-only + * `add_claims_to_id_token`), the ID Token is built with $addClaimsFromScopes = true, so the scope-derived + * user claims end up in the ID Token (in addition to the UserInfo endpoint). + * + * @throws \Exception + */ + public function testReleasesUserClaimsInIdTokenWhenClientConfiguredTo(): void + { + $this->clientEntityMock->method('getAddClaimsToIdToken')->willReturn(true); + $this->accessTokenEntityMock->method('getRequestedClaims')->willReturn([]); + $this->accessTokenEntityMock->method('getScopes')->willReturn($this->scopes); + + $idTokenBuilderMock = $this->createMock(IdTokenBuilder::class); + $idTokenBuilderMock->expects($this->once()) + ->method('buildFor') + ->with( + $this->anything(), + $this->anything(), + true, // $addClaimsFromScopes + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + ) + ->willReturn($this->idTokenMock); + $this->idTokenMock->method('getToken')->willReturn('token'); + + $idTokenResponse = $this->prepareMockedInstance($idTokenBuilderMock); + $idTokenResponse->setAccessToken($this->accessTokenEntityMock); + $idTokenResponse->generateHttpResponse(new Response()); + } + + /** + * By default (client not configured to release claims in the ID Token), the ID Token is built with + * $addClaimsFromScopes = false, so scope-derived user claims remain available only at the UserInfo endpoint. + * + * @throws \Exception + */ + public function testDoesNotReleaseUserClaimsInIdTokenByDefault(): void + { + $this->clientEntityMock->method('getAddClaimsToIdToken')->willReturn(false); + $this->accessTokenEntityMock->method('getRequestedClaims')->willReturn([]); + $this->accessTokenEntityMock->method('getScopes')->willReturn($this->scopes); + + $idTokenBuilderMock = $this->createMock(IdTokenBuilder::class); + $idTokenBuilderMock->expects($this->once()) + ->method('buildFor') + ->with( + $this->anything(), + $this->anything(), + false, // $addClaimsFromScopes + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + ) + ->willReturn($this->idTokenMock); + $this->idTokenMock->method('getToken')->willReturn('token'); + + $idTokenResponse = $this->prepareMockedInstance($idTokenBuilderMock); + $idTokenResponse->setAccessToken($this->accessTokenEntityMock); + $idTokenResponse->generateHttpResponse(new Response()); + } + public function testNoExtraParamsForNonOidcRequest(): void { $this->accessTokenEntityMock->method('getRequestedClaims')->willReturn([]);