upstream/keycloak
1
0
Fork 0
mirror of https://github.com/keycloak/keycloak.git synced 2026-02-18 18:37:54 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions 17

Make sure updates do not allow updating the resource associated with the uma policy (#46154)

Browse source 
Closes #46147

Signed-off-by: Pedro Igor <pigor.craveiro@gmail.com>
This commit is contained in:
Pedro Igor 2026-02-10 13:42:27 -03:00 committed by GitHub
parent 8d70b91ead
commit 295945773e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 97 additions and 20 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
services/src/main/java/org/keycloak/authorization/protection/policy/UserManagedPermissionService.java
View file

@ -17,6 +17,7 @@
package org.keycloak.authorization.protection.policy;
import java.io.IOException;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
@ -96,9 +97,16 @@ public class UserManagedPermissionService {
throw new ErrorResponseException(OAuthErrorException.INVALID_REQUEST, "Failed to parse representation", Status.BAD_REQUEST);
}
checkRequest(getAssociatedResourceId(policyId), representation);
String resourceId = getAssociatedResourceId(policyId);
Set<String> resources = Optional.ofNullable(representation.getResources()).orElse(Set.of());
return PolicyTypeResourceService.class.cast(delegate.getResource(policyId)).update(payload);
if (resources.isEmpty() || (resources.size() == 1 && resources.contains(resourceId))) {
checkRequest(resourceId, representation);
return PolicyTypeResourceService.class.cast(delegate.getResource(policyId)).update(payload);
}
throw new ErrorResponseException(OAuthErrorException.INVALID_REQUEST, "Uma permission resource cannot be changed", Response.Status.BAD_REQUEST);
}
@Path("{policyId}")

105
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UserManagedPermissionServiceTest.java
View file

@ -22,6 +22,7 @@ import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.UUID;
import jakarta.ws.rs.NotFoundException;
@ -66,6 +67,8 @@ import org.keycloak.testsuite.util.UserBuilder;
import org.junit.Test;
import static java.util.Collections.singletonList;
import static org.keycloak.authorization.model.Policy.FilterOption.OWNER;
import static org.junit.Assert.assertEquals;
@ -92,7 +95,7 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest
.realmRole(RoleBuilder.create().name("role_d").build())
)
.group(GroupBuilder.create().name("group_a")
.subGroups(Arrays.asList(GroupBuilder.create().name("group_b").build()))
.subGroups(singletonList(GroupBuilder.create().name("group_b").build()))
.build())
.group(GroupBuilder.create().name("group_c").build())
.group(GroupBuilder.create().name("group_remove").build())
@ -357,6 +360,72 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest
testUpdate();
}
@Test
public void testUpdateResources() {
ResourceRepresentation resource = new ResourceRepresentation();
resource.setName("Resource A");
resource.setOwnerManagedAccess(true);
resource.setOwner("marta");
resource.addScope("Scope A", "Scope B", "Scope C");
resource = getAuthzClient().protection().resource().create(resource);
ResourceRepresentation resourceB = new ResourceRepresentation();
resourceB.setName("Resource B");
resourceB.setOwnerManagedAccess(true);
resourceB.setOwner("kolo");
resourceB.addScope("Scope A", "Scope B", "Scope C");
resourceB = getAuthzClient().protection().resource().create(resourceB);
UmaPermissionRepresentation permission = new UmaPermissionRepresentation();
permission.setName("my-policy");
permission.addResource(resource.getId());
ProtectionResource protection = getAuthzClient().protection("marta", "password");
permission = protection.policy(resource.getId()).create(permission);
permission.addResource(resourceB.getId());
try {
protection.policy(resource.getId()).update(permission);
fail("Updates do not allow changing resources");
} catch (RuntimeException ignore) {
}
permission.setResources(Set.of(resource.getId()));
protection.policy(resource.getId()).update(permission);
permission.setResources(null);
protection.policy(resource.getId()).update(permission);
AuthorizationResource authorization = getAuthzClient().authorization("marta", "password");
AuthorizationRequest request = new AuthorizationRequest();
request.addPermission(resource.getId());
AuthorizationResponse authzResponse = authorization.authorize(request);
assertNotNull(authzResponse);
authorization = getAuthzClient().authorization("marta", "password");
request = new AuthorizationRequest();
request.addPermission(resourceB.getId());
try {
authorization.authorize(request);
fail("Updates do not allow changing resources");
} catch (RuntimeException denied) {
assertTrue(denied.getMessage().contains("403"));
}
}
@Test
@UncaughtServerErrorExpected
public void testUploadScriptDisabled() {
@ -437,14 +506,14 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest
authorization.authorize(request);
fail("User should not have permission");
} catch (Exception e) {
assertTrue(AuthorizationDeniedException.class.isInstance(e));
assertTrue(e instanceof AuthorizationDeniedException);
}
try {
getAuthzClient().authorization("alice", "password").authorize(request);
fail("User should not have permission");
} catch (Exception e) {
assertTrue(AuthorizationDeniedException.class.isInstance(e));
assertTrue(e instanceof AuthorizationDeniedException);
}
permission.addRole("role_a");
@ -461,14 +530,14 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest
authorization.authorize(request);
fail("User should not have permission");
} catch (Exception e) {
assertTrue(AuthorizationDeniedException.class.isInstance(e));
assertTrue(e instanceof AuthorizationDeniedException);
}
try {
getAuthzClient().protection("marta", "password").policy(resource.getId()).findById(permission.getId());
fail("Permission must not exist");
} catch (Exception e) {
assertEquals(404, HttpResponseException.class.cast(e.getCause()).getStatusCode());
assertEquals(404, ((HttpResponseException) e.getCause()).getStatusCode());
}
// create a user based permission, where only selected users are allowed access to the resource.
@ -488,7 +557,7 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest
authorization.authorize(request);
fail("User should not have permission to access the protected resource");
} catch(Exception e) {
assertTrue(AuthorizationDeniedException.class.isInstance(e));
assertTrue(e instanceof AuthorizationDeniedException);
}
}
@ -526,7 +595,7 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest
UsersResource users = realmsResouce().realm(REALM_NAME).users();
UserRepresentation marta = users.search("marta").get(0);
users.delete(marta.getId());
users.delete(marta.getId()).close();
getTestingClient().server().run((RunOnServer) UserManagedPermissionServiceTest::testRemovePolicyWhenOwnerDeleted);
}
@ -571,7 +640,7 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest
getAuthzClient().authorization("kolo", "password").authorize(request);
fail("User should not have permission");
} catch (Exception e) {
assertTrue(AuthorizationDeniedException.class.isInstance(e));
assertTrue(e instanceof AuthorizationDeniedException);
assertTrue(e.getMessage().contains("request_submitted"));
}
@ -620,7 +689,7 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest
getAuthzClient().authorization("kolo", "password").authorize(request);
fail("User should not have permission");
} catch (Exception e) {
assertTrue(AuthorizationDeniedException.class.isInstance(e));
assertTrue(e instanceof AuthorizationDeniedException);
}
request = new AuthorizationRequest();
@ -631,7 +700,7 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest
getAuthzClient().authorization("kolo", "password").authorize(request);
fail("User should not have permission");
} catch (Exception e) {
assertTrue(AuthorizationDeniedException.class.isInstance(e));
assertTrue(e instanceof AuthorizationDeniedException);
}
getAuthzClient().protection("marta", "password").policy(resource.getId()).delete(permission.getId());
@ -640,7 +709,7 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest
getAuthzClient().authorization("kolo", "password").authorize(request);
fail("User should not have permission");
} catch (Exception e) {
assertTrue(AuthorizationDeniedException.class.isInstance(e));
assertTrue(e instanceof AuthorizationDeniedException);
}
}
@ -699,7 +768,7 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest
getAuthzClient().protection("alice", "password").policy(resource.getId()).create(new UmaPermissionRepresentation());
fail("Error expected");
} catch (Exception e) {
assertTrue(HttpResponseException.class.cast(e.getCause()).toString().contains("Only resource owner can access policies for resource"));
assertTrue(e.getCause().toString().contains("Only resource owner can access policies for resource"));
}
}
@ -719,7 +788,7 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest
getAuthzClient().protection("marta", "password").policy(resource.getId()).create(new UmaPermissionRepresentation());
fail("Error expected");
} catch (Exception e) {
assertTrue(HttpResponseException.class.cast(e.getCause()).toString().contains("Only resources with owner managed accessed can have policies"));
assertTrue(e.getCause().toString().contains("Only resources with owner managed accessed can have policies"));
}
}
@ -746,7 +815,7 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest
rep = getAuthzClient().protection("marta", "password").policy(resource.getId()).create(rep);
} catch (Exception e) {
assertTrue(HttpResponseException.class.cast(e.getCause()).toString().contains("Only resources with owner managed accessed can have policies"));
assertTrue(e.getCause().toString().contains("Only resources with owner managed accessed can have policies"));
}
AuthorizationResource authorization = getAuthzClient().authorization("marta", "password");
@ -763,7 +832,7 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest
getAuthzClient().authorization("kolo", "password").authorize(request);
fail("User should not have permission");
} catch (Exception e) {
assertTrue(AuthorizationDeniedException.class.isInstance(e));
assertTrue(e instanceof AuthorizationDeniedException);
}
rep.addRole("role_a");
@ -851,7 +920,7 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest
getAuthzClient().authorization("kolo", "password").authorize(request);
fail("User should not have permission");
} catch (Exception e) {
assertTrue(AuthorizationDeniedException.class.isInstance(e));
assertTrue(e instanceof AuthorizationDeniedException);
}
request = new AuthorizationRequest();
@ -862,7 +931,7 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest
getAuthzClient().authorization("kolo", "password").authorize(request);
fail("User should not have permission");
} catch (Exception e) {
assertTrue(AuthorizationDeniedException.class.isInstance(e));
assertTrue(e instanceof AuthorizationDeniedException);
}
request = new AuthorizationRequest();
@ -922,7 +991,7 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest
getAuthzClient().authorization("kolo", "password").authorize();
fail("User should not have permission");
} catch (Exception e) {
assertTrue(AuthorizationDeniedException.class.isInstance(e));
assertTrue(e instanceof AuthorizationDeniedException);
}
}