From 2ac9b1ce9001b4e232a18aea8d1be5f1c4c4a8a5 Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Tue, 17 Feb 2026 08:51:51 -0300 Subject: [PATCH] Check resource server when managing their resources Closes #45650 Signed-off-by: Pedro Igor --- .../StoreFactoryCacheSession.java | 12 ++++++ .../jpa/store/JPAPermissionTicketStore.java | 5 ++- .../jpa/store/JPAPolicyStore.java | 2 +- .../jpa/store/JPAResourceStore.java | 6 ++- .../jpa/store/JPAScopeStore.java | 5 ++- .../admin/PolicyResourceService.java | 4 ++ .../admin/ResourceSetService.java | 39 ++++++++----------- .../permission/PermissionTicketService.java | 27 +++++++------ .../admin/AbstractAuthorizationTest.java | 7 ++++ .../authz/admin/ResourceManagementTest.java | 22 ++++++++++- 10 files changed, 88 insertions(+), 41 deletions(-) diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/StoreFactoryCacheSession.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/StoreFactoryCacheSession.java index b11fdf647aa7..bfe5632f6c08 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/StoreFactoryCacheSession.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/StoreFactoryCacheSession.java @@ -532,6 +532,9 @@ public Scope findById(ResourceServer resourceServer, String id) { } else if (managedScopes.containsKey(id)) { return managedScopes.get(id); } + if (resourceServer != null && !resourceServer.getId().equals(cached.getResourceServerId())) { + return null; + } ScopeAdapter adapter = new ScopeAdapter(cached, StoreFactoryCacheSession.this); managedScopes.put(id, adapter); return adapter; @@ -625,6 +628,9 @@ public Resource findById(ResourceServer resourceServer, String id) { } else if (managedResources.containsKey(id)) { return managedResources.get(id); } + if (resourceServer != null && ! resourceServer.getId().equals(cached.getResourceServerId())) { + return null; + } ResourceAdapter adapter = new ResourceAdapter(cached, StoreFactoryCacheSession.this); managedResources.put(id, adapter); return adapter; @@ -875,6 +881,9 @@ public Policy findById(ResourceServer resourceServer, String id) { } else if (managedPolicies.containsKey(id)) { return managedPolicies.get(id); } + if (resourceServer != null && !cached.getResourceServerId().equals(resourceServer.getId())) { + return null; + } PolicyAdapter adapter = new PolicyAdapter(cached, StoreFactoryCacheSession.this); managedPolicies.put(id, adapter); return adapter; @@ -1124,6 +1133,9 @@ public PermissionTicket findById(ResourceServer resourceServer, String id) { } else if (managedPermissionTickets.containsKey(id)) { return managedPermissionTickets.get(id); } + if (resourceServer != null && !cached.getResourceServerId().equals(resourceServer.getId())) { + return null; + } PermissionTicketAdapter adapter = new PermissionTicketAdapter(cached, StoreFactoryCacheSession.this); managedPermissionTickets.put(id, adapter); return adapter; diff --git a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAPermissionTicketStore.java b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAPermissionTicketStore.java index e54b58815bf2..9a68fe8c7e91 100644 --- a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAPermissionTicketStore.java +++ b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAPermissionTicketStore.java @@ -166,7 +166,10 @@ public PermissionTicket findById(ResourceServer resourceServer, String id) { } PermissionTicketEntity entity = entityManager.find(PermissionTicketEntity.class, id); - if (entity == null) return null; + + if (entity == null || !resourceServer.getId().equals(entity.getResourceServer().getId())) { + return null; + } return new PermissionTicketAdapter(entity, entityManager, provider.getStoreFactory()); } diff --git a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAPolicyStore.java b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAPolicyStore.java index 2190a8d58fe2..13d06fb69b1f 100644 --- a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAPolicyStore.java +++ b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAPolicyStore.java @@ -104,7 +104,7 @@ public Policy findById(ResourceServer resourceServer, String id) { PolicyEntity policyEntity = entityManager.find(PolicyEntity.class, id); - if (policyEntity == null) { + if (policyEntity == null || (resourceServer != null && !policyEntity.getResourceServer().getId().equals(resourceServer.getId()))) { return null; } diff --git a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAResourceStore.java b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAResourceStore.java index 66044dcee0fe..a8092c329fd5 100644 --- a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAResourceStore.java +++ b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAResourceStore.java @@ -95,7 +95,11 @@ public Resource findById(ResourceServer resourceServer, String id) { } ResourceEntity entity = entityManager.find(ResourceEntity.class, id); - if (entity == null) return null; + + if (entity == null || (resourceServer != null && !resourceServer.getId().equals(entity.getResourceServer()))) { + return null; + } + return new ResourceAdapter(entity, entityManager, provider.getStoreFactory()); } diff --git a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAScopeStore.java b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAScopeStore.java index d6795705f190..fc72d940ab4b 100644 --- a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAScopeStore.java +++ b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAScopeStore.java @@ -93,7 +93,10 @@ public Scope findById(ResourceServer resourceServer, String id) { } ScopeEntity entity = entityManager.find(ScopeEntity.class, id); - if (entity == null) return null; + + if (entity == null || !entity.getResourceServer().getId().equals(resourceServer.getId())) { + return null; + } return new ScopeAdapter(entity, entityManager, provider.getStoreFactory()); } diff --git a/services/src/main/java/org/keycloak/authorization/admin/PolicyResourceService.java b/services/src/main/java/org/keycloak/authorization/admin/PolicyResourceService.java index ea79a11ecc4f..c57c0ee8ea7e 100644 --- a/services/src/main/java/org/keycloak/authorization/admin/PolicyResourceService.java +++ b/services/src/main/java/org/keycloak/authorization/admin/PolicyResourceService.java @@ -23,6 +23,7 @@ import jakarta.ws.rs.Consumes; import jakarta.ws.rs.DELETE; import jakarta.ws.rs.GET; +import jakarta.ws.rs.NotFoundException; import jakarta.ws.rs.PUT; import jakarta.ws.rs.Path; import jakarta.ws.rs.Produces; @@ -64,6 +65,9 @@ public class PolicyResourceService { private final AdminEventBuilder adminEvent; public PolicyResourceService(Policy policy, ResourceServer resourceServer, AuthorizationProvider authorization, AdminPermissionEvaluator auth, AdminEventBuilder adminEvent) { + if (policy == null || !policy.getResourceServer().equals(resourceServer)) { + throw new NotFoundException(); + } this.policy = policy; this.resourceServer = resourceServer; this.authorization = authorization; diff --git a/services/src/main/java/org/keycloak/authorization/admin/ResourceSetService.java b/services/src/main/java/org/keycloak/authorization/admin/ResourceSetService.java index 6b03f61aa279..fc8a8cf88224 100644 --- a/services/src/main/java/org/keycloak/authorization/admin/ResourceSetService.java +++ b/services/src/main/java/org/keycloak/authorization/admin/ResourceSetService.java @@ -32,6 +32,7 @@ import jakarta.ws.rs.Consumes; import jakarta.ws.rs.DELETE; import jakarta.ws.rs.GET; +import jakarta.ws.rs.NotFoundException; import jakarta.ws.rs.POST; import jakarta.ws.rs.PUT; import jakarta.ws.rs.Path; @@ -164,14 +165,7 @@ public Response update(@PathParam("resource-id") String id, ResourceRepresentati AdminPermissionsSchema.SCHEMA.throwExceptionIfAdminPermissionClient(session, resourceServer.getId()); requireManage(); resource.setId(id); - StoreFactory storeFactory = this.authorization.getStoreFactory(); - ResourceStore resourceStore = storeFactory.getResourceStore(); - Resource model = resourceStore.findById(resourceServer, resource.getId()); - - if (model == null) { - return Response.status(Status.NOT_FOUND).build(); - } - + getResource(id); toModel(resource, resourceServer, authorization); audit(resource, OperationType.UPDATE); @@ -179,6 +173,17 @@ public Response update(@PathParam("resource-id") String id, ResourceRepresentati return Response.noContent().build(); } + private Resource getResource(String id) { + ResourceStore resourceStore = authorization.getStoreFactory().getResourceStore(); + Resource model = resourceStore.findById(resourceServer, id); + + if (model == null || !model.getResourceServer().equals(resourceServer)) { + throw new NotFoundException(); + } + + return model; + } + @Path("{resource-id}") @DELETE @APIResponses(value = { @@ -188,15 +193,10 @@ public Response update(@PathParam("resource-id") String id, ResourceRepresentati public Response delete(@PathParam("resource-id") String id) { AdminPermissionsSchema.SCHEMA.throwExceptionIfAdminPermissionClient(session, resourceServer.getId()); requireManage(); - StoreFactory storeFactory = authorization.getStoreFactory(); - Resource resource = storeFactory.getResourceStore().findById(resourceServer, id); - - if (resource == null) { - return Response.status(Status.NOT_FOUND).build(); - } - + Resource resource = getResource(id); //to be able to access all lazy loaded fields it's needed to create representation before it's deleted ResourceRepresentation resourceRep = toRepresentation(resource, resourceServer, authorization); + StoreFactory storeFactory = authorization.getStoreFactory(); storeFactory.getResourceStore().delete(id); @@ -222,14 +222,7 @@ public Response findById(@PathParam("resource-id") String id) { public Response findById(String id, Function toRepresentation) { requireView(); - StoreFactory storeFactory = authorization.getStoreFactory(); - Resource model = storeFactory.getResourceStore().findById(resourceServer, id); - - if (model == null) { - return Response.status(Status.NOT_FOUND).build(); - } - - return Response.ok(toRepresentation.apply(model)).build(); + return Response.ok(toRepresentation.apply(getResource(id))).build(); } @Path("{resource-id}/scopes") diff --git a/services/src/main/java/org/keycloak/authorization/protection/permission/PermissionTicketService.java b/services/src/main/java/org/keycloak/authorization/protection/permission/PermissionTicketService.java index 5e780dfaf872..46418c6e7b16 100644 --- a/services/src/main/java/org/keycloak/authorization/protection/permission/PermissionTicketService.java +++ b/services/src/main/java/org/keycloak/authorization/protection/permission/PermissionTicketService.java @@ -140,13 +140,8 @@ public Response update(PermissionTicketRepresentation representation) { throw new ErrorResponseException(OAuthErrorException.INVALID_REQUEST, "invalid_ticket", Response.Status.BAD_REQUEST); } - PermissionTicketStore ticketStore = authorization.getStoreFactory().getPermissionTicketStore(); - PermissionTicket ticket = ticketStore.findById(resourceServer, representation.getId()); + PermissionTicket ticket = getPermissionTicket(representation.getId()); - if (ticket == null) { - throw new ErrorResponseException(OAuthErrorException.INVALID_REQUEST, "invalid_ticket", Response.Status.BAD_REQUEST); - } - if (!ticket.getOwner().equals(this.identity.getId()) && !this.identity.isResourceServer()) throw new ErrorResponseException("not_authorised", "permissions for [" + representation.getResource() + "] can be updated only by the owner or by the resource server", Response.Status.FORBIDDEN); @@ -164,21 +159,29 @@ public Response delete(@PathParam("id") String id) { throw new ErrorResponseException(OAuthErrorException.INVALID_REQUEST, "invalid_ticket", Response.Status.BAD_REQUEST); } - PermissionTicketStore ticketStore = authorization.getStoreFactory().getPermissionTicketStore(); - PermissionTicket ticket = ticketStore.findById(resourceServer, id); + PermissionTicket ticket = getPermissionTicket(id); - if (ticket == null) { - throw new ErrorResponseException(OAuthErrorException.INVALID_REQUEST, "invalid_ticket", Response.Status.BAD_REQUEST); - } - if (!ticket.getOwner().equals(this.identity.getId()) && !this.identity.isResourceServer() && !ticket.getRequester().equals(this.identity.getId())) throw new ErrorResponseException("not_authorised", "permissions for [" + ticket.getResource() + "] can be deleted only by the owner, the requester, or the resource server", Response.Status.FORBIDDEN); + PermissionTicketStore ticketStore = authorization.getStoreFactory().getPermissionTicketStore(); + ticketStore.delete(id); return Response.noContent().build(); } + private PermissionTicket getPermissionTicket(String id) { + PermissionTicketStore ticketStore = authorization.getStoreFactory().getPermissionTicketStore(); + PermissionTicket ticket = ticketStore.findById(resourceServer, id); + + if (ticket == null || !ticket.getResourceServer().equals(resourceServer)) { + throw new ErrorResponseException(OAuthErrorException.INVALID_REQUEST, "invalid_ticket", Response.Status.BAD_REQUEST); + } + + return ticket; + } + @GET @Produces("application/json") public Response find(@QueryParam("scopeId") String scopeId, diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/admin/AbstractAuthorizationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/admin/AbstractAuthorizationTest.java index b0319fc38149..1ad9357fda85 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/admin/AbstractAuthorizationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/admin/AbstractAuthorizationTest.java @@ -134,6 +134,13 @@ private RealmBuilder createTestRealm() { .authorizationServicesEnabled(true) .redirectUris("http://localhost/" + RESOURCE_SERVER_CLIENT_ID) .defaultRoles("uma_protection") + .directAccessGrants()) + .client(ClientBuilder.create().clientId("another-resource-server-other") + .name("another-resource-server-other") + .secret("secret") + .authorizationServicesEnabled(true) + .redirectUris("http://localhost/" + "another-resource-server-other") + .defaultRoles("uma_protection") .directAccessGrants()); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/admin/ResourceManagementTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/admin/ResourceManagementTest.java index 70dfee8d7dc3..7974033e52a5 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/admin/ResourceManagementTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/admin/ResourceManagementTest.java @@ -342,6 +342,22 @@ public void testUpdateScopes() { assertEquals(0, updated.getScopes().size()); } + @Test + public void testFindResourceById() { + ResourceRepresentation resource = createResourceWithDefaultScopes(); + resource.setId(null); + resource.setName("Another Resource"); + resource.setOwner((String) null); + resource.setScopes(Set.of()); + ResourceRepresentation anotherResource = doCreateResource(resource, findClientResource("another-resource-server-other").authorization().resources()); + + try { + getClientResource().authorization().resources().resource(anotherResource.getId()).toRepresentation(); + fail("Should not find resource from another resource server"); + } catch (NotFoundException ignore) { + } + } + private ResourceRepresentation createResourceWithDefaultScopes() { ResourceRepresentation resource = createResource(); @@ -396,8 +412,10 @@ private ResourceRepresentation createResource(String name, String owner, String } protected ResourceRepresentation doCreateResource(ResourceRepresentation newResource) { - ResourcesResource resources = getClientResource().authorization().resources(); + return doCreateResource(newResource, getClientResource().authorization().resources()); + } + private ResourceRepresentation doCreateResource(ResourceRepresentation newResource, ResourcesResource resources) { try (Response response = resources.create(newResource)) { int status = response.getStatus(); @@ -425,4 +443,4 @@ protected void doRemoveResource(ResourceRepresentation resource) { ResourcesResource resources = getClientResource().authorization().resources(); resources.resource(resource.getId()).remove(); } -} \ No newline at end of file +}