8000 Enforce owner checks when calling the resource set service by pedroigor · Pull Request #49121 · keycloak/keycloak · GitHub
[go: up one dir, main page]
Skip to content

Enforce owner checks when calling the resource set service #49121

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
stianst merged 1 commit into from
May 20, 2026
Merged

Enforce owner checks when calling the resource set service #49121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion services/src/main/java/org/keycloak/authorization/protection/ProtectionService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public Object resource() {
KeycloakIdentity identity = createIdentity(true);
ResourceServer resourceServer = getResourceServer(identity);
ResourceSetService resourceManager = new ResourceSetService(this.session, resourceServer, this.authorization, null, createAdminEventBuilder(identity, resourceServer));
return new ResourceService(this.session, resourceServer, identity, resourceManager);
return new ResourceService(authorization, resourceServer, identity, resourceManager);
}

private AdminEventBuilder createAdminEventBuilder(KeycloakIdentity identity, ResourceServer resourceServer) {
Expand Down
52 changes: 32 additions & 20 deletions services/src/main/java/org/keycloak/authorization/protection/resource/ResourceService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.core.Response.Status;

import org.keycloak.authorization.AuthorizationProvider;
import org.keycloak.authorization.admin.ResourceSetService;
import org.keycloak.authorization.identity.Identity;
import org.keycloak.authorization.common.KeycloakIdentity;
import org.keycloak.authorization.model.Resource;
import org.keycloak.authorization.model.ResourceServer;
import org.keycloak.events.admin.OperationType;
import org.keycloak.models.KeycloakSession;
import org.keycloak.representations.idm.authorization.ResourceOwnerRepresentation;
import org.keycloak.representations.idm.authorization.ResourceRepresentation;
import org.keycloak.services.ErrorResponseException;
Expand All @@ -50,11 +50,11 @@ public class ResourceService {

private final ResourceServer resourceServer;
private final ResourceSetService resourceManager;
private final KeycloakSession session;
private final Identity identity;
private final AuthorizationProvider authorization;
private final KeycloakIdentity identity;

public ResourceService(KeycloakSession session, ResourceServer resourceServer, Identity identity, ResourceSetService resourceManager) {
this.session = session;
public ResourceService(AuthorizationProvider authorization, ResourceServer resourceServer, KeycloakIdentity identity, ResourceSetService resourceManager) {
this.authorization = authorization;
this.identity = identity;
this.resourceServer = resourceServer;
this.resourceManager = resourceManager;
Expand All @@ -69,20 +69,9 @@ public Response create(UmaResourceRepresentation resource) {
return Response.status(Status.BAD_REQUEST).build();
}

ResourceOwnerRepresentation owner = resource.getOwner();

if (owner == null) {
owner = new ResourceOwnerRepresentation();
resource.setOwner(owner);
}

String ownerId = owner.getId();

if (ownerId == null) {
ownerId = this.identity.getId();
}

owner.setId(ownerId);
ResourceOwnerRepresentation owner = new ResourceOwnerRepresentation();
owner.setId(identity.getId());
resource.setOwner(owner);

ResourceRepresentation newResource = resourceManager.create(resource);

Expand All @@ -96,19 +85,22 @@ public Response create(UmaResourceRepresentation resource) {
@Consumes("application/json")
@Produces("application/json")
public Response update(@PathParam("id") String id, ResourceRepresentation resource) {
checkOwner(id);
return this.resourceManager.update(id, resource);
}

@Path("/{id}")
@DELETE
public Response delete(@PathParam("id") String id) {
checkOwner(id);
return this.resourceManager.delete(id);
}

@Path("/{id}")
@GET
@Produces("application/json")
public Response findById(@PathParam("id") String id) {
checkOwner(id);
return this.resourceManager.findById(id, UmaResourceRepresentation::new);
}

Expand All @@ -126,6 +118,10 @@ public Response find(@QueryParam("_id") String id,
@QueryParam("deep") Boolean deep,
@QueryParam("first") Integer firstResult,
@QueryParam("max") Integer maxResult) {
if (!identity.isResourceServer()) {
owner = identity.getId();
}

if(deep != null && deep) {
return resourceManager.find(id, name, uri, owner, type, scope, matchingUri, exactName, deep, firstResult, maxResult);
} else {
Expand All @@ -138,4 +134,20 @@ private void checkResourceServerSettings() {
throw new ErrorResponseException("not_supported", "Remote management is disabled.", Status.BAD_REQUEST);
}
}

private void checkOwner(String resourceId) {
if (identity.isResourceServer()) {
return;
}

Resource resource = authorization.getStoreFactory().getResourceStore().findById(resourceServer, resourceId);

if (resource == null) {
throw new ErrorResponseException("not_found", "Resource not found", Status.NOT_FOUND);
}

if (!resource.getOwner().equals(identity.getId())) {
throw new ErrorResponseException("forbidden", "You don't have permission to manage this resource", Status.FORBIDDEN);
}
}
}
55 changes: 44 additions & 11 deletions ...uillian/tests/base/src/test/java/org/keycloak/testsuite/authz/ProtectionResourceTest.java
View file
10BC8 Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@
import org.junit.Before;
import org.junit.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

public class ProtectionResourceTest extends AbstractResourceServerTest {

Expand All @@ -38,17 +39,47 @@ public void testAllowRemoteManagement() throws Exception {
authorization.update(settings);
});

assertBadRequest(() -> {
String expectedMessage = "Remote management is disabled";
assertResponse(() -> {
ResourceRepresentation resource = new ResourceRepresentation();
resource.setName(KeycloakModelUtils.generateId());
protection.resource().create(resource);
});
assertBadRequest(() -> protection.resource().update(existing));
assertBadRequest(() -> protection.resource().delete(existing.getId()));
assertBadRequest(() -> protection.resource().findAll());
}, Status.BAD_REQUEST, expectedMessage);
assertResponse(() -> protection.resource().update(existing), Status.BAD_REQUEST, expectedMessage);
assertResponse(() -> protection.resource().delete(existing.getId()), Status.BAD_REQUEST, expectedMessage);
assertResponse(() -> protection.resource().findAll(), Status.BAD_REQUEST, expectedMessage);
}

@Test
public void testOnlyOwnerAndServerCanManageResources() throws Exception {
ResourceRepresentation resource = addResource(KeycloakModelUtils.generateId(), "marta", true);
ResourceRepresentation existing = assertCanManage(protection, resource);

ProtectionResource koloClient = getAuthzClient().protection("kolo", "password");
assertResponse(() -> koloClient.resource().update(existing), Status.FORBIDDEN);
assertResponse(() -> koloClient.resource().findById(existing.getId()), Status.FORBIDDEN);
assertEquals(0, koloClient.resource().findAll().length);
assertResponse(() -> koloClient.resource().delete(existing.getId()), Status.FORBIDDEN);

ProtectionResource resourceServerClient = getAuthzClient().protection();
assertCanManage(resourceServerClient, existing);
}

private void assertBadRequest(Runnable runnable) {
private ResourceRepresentation assertCanManage(ProtectionResource protection, ResourceRepresentation resource) throws Exception {
protection.resource().update(resource);
resource = protection.resource().findById(resource.getId());
assertNotNull(resource);
assertEquals(1, protection.resource().findAll().length);
protection.resource().delete(resource.getId());
assertEquals(0, protection.resource().findAll().length);
return addResource(KeycloakModelUtils.generateId(), "marta", true);
}

private void assertResponse(Runnable runnable, Status status) {
assertResponse(runnable, status, null);
}

private void assertResponse(Runnable runnable, Status status, String message) {
try {
runnable.run();
fail("Server should fail the request");
Expand All @@ -58,8 +89,10 @@ private void assertBadRequest(Runnable runnable) {
if (!(error instanceof HttpResponseException)) {
error = error.getCause();
}
assertEquals(Status.BAD_REQUEST.getStatusCode(), ((HttpResponseException) error).getStatusCode());
assertTrue(new String(((HttpResponseException) error).getBytes()).contains("Remote management is disabled"));
assertEquals(status.getStatusCode(), ((HttpResponseException) error).getStatusCode());
if (message != null) {
assertTrue(new String(((HttpResponseException) error).getBytes()).contains(message));
}
}
}
}
Loading
Loading
0