diff --git a/docs/documentation/upgrading/topics/changes/changes-26_6_3.adoc b/docs/documentation/upgrading/topics/changes/changes-26_6_3.adoc index 90e52347517..b7292a682da 100644 --- a/docs/documentation/upgrading/topics/changes/changes-26_6_3.adoc +++ b/docs/documentation/upgrading/topics/changes/changes-26_6_3.adoc @@ -9,6 +9,14 @@ The wildcard comparison for valid redirect URIs does not affect the hostname any Note that OAuth 2.0 recommends exact string matching in the link:https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics#name-protecting-redirect-based-f[Security Best Current Practice] and draft for OAuth 2.1 enforces it. {project_name} recommends to not use any wildcard valid redirect URI for clients. See link:{adminguide_link}#unspecific-redirect-uris_server_administration_guide[Unspecific redirect URIs] in the {adminguide_name} for more information. +=== Client scope evaluation now enforces access to the user when generating tokens + +In previous versions of {project_name}, client scope evaluation allow generating tokens without necessarily having +the necessary admin roles or permissions to access the user. + +In this release, client scope evaluation now requires at the very least the `view-users` admin role granted to the +realm administrator or any permission that grants the `view` scope on the user. + == Notable changes Notable changes may include internal behavior changes that prevent common misconfigurations, bugs that are fixed, or changes to simplify running {project_name}. diff --git a/services/src/main/java/org/keycloak/services/resources/admin/ClientScopeEvaluateResource.java b/services/src/main/java/org/keycloak/services/resources/admin/ClientScopeEvaluateResource.java index 75584e1762e..459915ae16d 100644 --- a/services/src/main/java/org/keycloak/services/resources/admin/ClientScopeEvaluateResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/ClientScopeEvaluateResource.java @@ -25,6 +25,7 @@ import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; +import jakarta.ws.rs.ForbiddenException; import jakarta.ws.rs.GET; import jakarta.ws.rs.NotFoundException; import jakarta.ws.rs.Path; @@ -340,9 +341,17 @@ public class ClientScopeEvaluateResource { } UserModel user = session.users().getUserById(realm, userId); + + try { + auth.users().requireView(user); + } catch (ForbiddenException e) { + throw new ForbiddenException("You have no access to this user"); + } + if (user == null) { throw new NotFoundException("No user found"); } + return user; } diff --git a/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/ClientResourceTypeEvaluationTest.java b/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/ClientResourceTypeEvaluationTest.java index f42240c016c..45848d4123c 100644 --- a/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/ClientResourceTypeEvaluationTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/ClientResourceTypeEvaluationTest.java @@ -34,6 +34,7 @@ import org.keycloak.authorization.fgap.AdminPermissionsSchema; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.ClientScopeRepresentation; +import org.keycloak.representations.idm.OAuth2ErrorRepresentation; import org.keycloak.representations.idm.ProtocolMapperRepresentation; import org.keycloak.representations.idm.RoleRepresentation; import org.keycloak.representations.idm.UserRepresentation; @@ -54,6 +55,8 @@ import org.keycloak.testframework.annotations.InjectAdminClient; import org.keycloak.testframework.annotations.InjectClient; import org.keycloak.testframework.annotations.KeycloakIntegrationTest; import org.keycloak.testframework.realm.ManagedClient; +import org.keycloak.testframework.realm.UserBuilder; +import org.keycloak.testframework.util.ApiUtil; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -69,6 +72,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.instanceOf; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; @KeycloakIntegrationTest @@ -222,6 +226,41 @@ public class ClientResourceTypeEvaluationTest extends AbstractPermissionTest { assertThat(found, not(empty())); } + @Test + public void testClientScopeEvaluation() { + UserRepresentation myadmin = realm.admin().users().search("myadmin").get(0); + + ClientRepresentation newClient = new ClientRepresentation(); + newClient.setClientId("newClient"); + newClient.setProtocol("openid-connect"); + + UserPolicyRepresentation onlyMyAdminUserPolicy = createUserPolicy(realm, client, "Only My Admin User Policy", myadmin.getId()); + createAllPermission(client, clientsType, onlyMyAdminUserPolicy, Set.of(VIEW, MANAGE)); + + realmAdminClient.realm(realm.getName()).clients().create(newClient).close(); + List found = realmAdminClient.realm(realm.getName()).clients().findByClientId("newClient"); + assertThat(found, hasSize(1)); + + UserRepresentation user = UserBuilder.create() + .username(KeycloakModelUtils.generateId()) + .build(); + try (Response response = realm.admin().users().create(user)) { + user.setId(ApiUtil.getCreatedId(response)); + } + + ClientResource clientApi = realmAdminClient.realm(realm.getName()).clients().get(found.get(0).getId()); + + try { + clientApi.clientScopesEvaluate().generateAccessToken("openid", user.getId(), null); + fail("no permissions to view the user."); + } catch (ForbiddenException e) { + assertEquals("You have no access to this user", e.getResponse().readEntity(OAuth2ErrorRepresentation.class).getError()); + } + + createPermission(client, user.getId(), AdminPermissionsSchema.USERS_RESOURCE_TYPE, Set.of(VIEW), onlyMyAdminUserPolicy); + clientApi.clientScopesEvaluate().generateAccessToken("openid", user.getId(), null); + } + @Test public void testViewAllClients() { ClientRepresentation myclient = realm.admin().clients().findByClientId("myclient").get(0);