8000 Reject resourceType on user-managed permissions by pedroigor · Pull Request #54967 · keycloak/keycloak · GitHub
[go: up one dir, main page]
Skip to content

Reject resourceType on user-managed permissions #54967

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
Nov 12, 2026
Merged

Reject resourceType on user-managed permissions #54967

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
8 changes: 7 additions & 1 deletion ...te/src/main/java/org/keycloak/authorization/policy/evaluation/DefaultPolicyEvaluator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,13 @@ protected void evaluateResourceTypePolicies(ResourcePermission permission, Autho
PolicyStore policyStore = storeFactory.getPolicyStore();
ResourceServer resourceServer = permission.getResourceServer();

policyStore.findByResourceType(resourceServer, resource.getType(), policyConsumer);
// resource-type policies are only considered when they are not bound to a specific owner (e.g. not a
// user-managed permission), so that an owner cannot use a resource type to reach resources they don't own
policyStore.findByResourceType(resourceServer, resource.getType(), policy -> {
if (policy.getOwner() == null) {
policyConsumer.accept(policy);
}
});

if (!resource.getOwner().equals(resourceServer.getClientId())) {
ResourceStore resourceStore = storeFactory.getResourceStore();
Expand Down
4 changes: 4 additions & 0 deletions .../main/java/org/keycloak/authorization/protection/policy/UserManagedPermissionService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,10 @@ private void checkRequest(String resourceId, UmaPermissionRepresentation represe
}

if (representation != null) {
if (representation.getResourceType() != null) {
throw new ErrorResponseException(OAuthErrorException.INVALID_REQUEST, "Resource type cannot be set on a user-managed permission", Response.Status.BAD_REQUEST);
}

Set<String> resourceScopes = resource.getScopes().stream().map(scope -> scope.getName()).collect(Collectors.toSet());
Set<String> scopes = representation.getScopes();

Expand Down
112 changes: 112 additions & 0 deletions ...sts/base/src/test/java/org/keycloak/testsuite/authz/UserManagedPermissionServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.keycloak.authorization.model.Policy;
import org.keycloak.authorization.model.Resource;
import org.keycloak.authorization.model.ResourceServer;
import org.keycloak.authorization.model.Scope;
import org.keycloak.authorization.store.PolicyStore;
import org.keycloak.common.Profile.Feature;
import org.keycloak.events.admin.OperationType;
Expand All @@ -46,6 +47,7 @@
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
import org.keycloak.models.UserModel;
import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.representations.AccessToken;
import org.keycloak.representations.idm.AdminEventRepresentation;
import org.keycloak.representations.idm.RealmEventsConfigRepresentation;
Expand All @@ -62,6 +64,7 @@
import org.keycloak.representations.idm.authorization.ResourceRepresentation;
import org.keycloak.representations.idm.authorization.ResourceServerRepresentation;
import org.keycloak.representations.idm.authorization.UmaPermissionRepresentation;
import org.keycloak.representations.idm.authorization.UserPolicyRepresentation;
import org.keycloak.testframework.realm.ClientBuilder;
import org.keycloak.testframework.realm.GroupBuilder;
import org.keycloak.testframework.realm.RealmBuilder;
Expand Down Expand Up @@ -175,6 +178,19 @@ private void testCreate() {

newPermission.getResources().remove(resourceB.getId());

newPermission.setResourceType("patient-data");

try {
protection.policy(resource.getId()).create(newPermission);
fail("Should fail, not allowed to set a resource type on a user-managed permission");
} catch (RuntimeException ignore) {
Throwable cause = ignore.getCause();
assertTrue(cause instanceof HttpResponseException);
assertEquals(Status.BAD_REQUEST.getStatusCode(), ((HttpResponseException) cause).getStatusCode());
}

newPermission.setResourceType(null);

UmaPermissionRepresentation permission = protection.policy(resource.getId()).create(newPermission);

assertEquals(newPermission.getName(), permission.getName());
Expand Down Expand Up @@ -451,6 +467,39 @@ public void testUpdateResources() {
}
}

@Test
public void testUpdateResourceType() {
ResourceRepresentation resource = new ResourceRepresentation();

resource.setName("Resource A");
resource.setType("patient-data");
resource.setOwnerManagedAccess(true);
resource.setOwner("marta");
resource.addScope("Scope A", "Scope B", "Scope C");

resource = getAuthzClient().protection("marta", "password").resource().create(resource);

UmaPermissionRepresentation permission = new UmaPermissionRepresentation();

permission.setName("my-policy");
permission.addUser("kolo");

ProtectionResource protection = getAuthzClient().protection("marta", "password");

permission = protection.policy(resource.getId()).create(permission);

permission.setResourceType("patient-data");

try {
protection.policy(resource.getId()).update(permission);
fail("Updates do not allow setting a resource type on a user-managed permission");
} catch (RuntimeException ignore) {
Throwable cause = ignore.getCause();
assertTrue(cause instanceof HttpResponseException);
assertEquals(Status.BAD_REQUEST.getStatusCode(), ((HttpResponseException) cause).getStatusCode());
}
}

@Test
@UncaughtServerErrorExpected
public void testUploadScriptDisabled() {
Expand Down Expand Up @@ -1190,6 +1239,69 @@ public void testResourceTypeAnyDoesNotLeakOwnerManagedResources() {
}
}

@Test
public void testResourceTypePolicyWithOwnerDoesNotGrantCrossOwnerAccess() {
// alice owns a resource of type "patient-data" that she never shares with kolo
ResourceRepresentation aliceResource = new ResourceRepresentation();
aliceResource.setName("alice-medical-records");
aliceResource.setType("patient-data");
aliceResource.setOwnerManagedAccess(true);
aliceResource.addScope("read");
aliceResource = getAuthzClient().protection("alice", "password").resource().create(aliceResource);

// marta owns a resource of the same type and has a (pre-existing / legacy) owner-bound policy that
// matches by resource type and grants kolo access; this simulates data created before this fix, since
// the Protection API no longer allows setting a resource type on a user-managed permission
ResourceRepresentation martaResource = new ResourceRepresentation();
martaResource.setName("marta-medical-records");
martaResource.setType("patient-data");
martaResource.setOwner("marta");
martaResource.setOwnerManagedAccess(true);
martaResource.addScope("read");
getAuthzClient().protection("marta", "password").resource().create(martaResource);

getTestingClient().server().run((RunOnServer) UserManagedPermissionServiceTest::createOwnerBoundResourceTypePolicy);

AuthorizationRequest request = new AuthorizationRequest();
request.addPermission(aliceResource.getId(), "read");

try {
getAuthzClient().authorization("kolo", "password").authorize(request);
fail("kolo should not gain access to alice's resource through marta's owner-bound resource-type policy");
} catch (Exception e) {
assertTrue(e instanceof AuthorizationDeniedException);
}
}

private static void createOwnerBoundResourceTypePolicy(KeycloakSession session) {
RealmModel realm = session.realms().getRealmByName("authz-test");
session.getContext().setRealm(realm);
ClientModel client = realm.getClientByClientId("resource-server-test");
AuthorizationProvider provider = session.getProvider(AuthorizationProvider.class);
ResourceServer resourceServer = provider.getStoreFactory().getResourceServerStore().findByClient(client);
UserModel marta = session.users().getUserByUsername(realm, "marta");

UserPolicyRepresentation userPolicyRep = new UserPolicyRepresentation();
userPolicyRep.setName(KeycloakModelUtils.generateId());
userPolicyRep.addUser("kolo");

PolicyStore policyStore = provider.getStoreFactory().getPolicyStore();
Policy userPolicy = policyStore.create(resourceServer, userPolicyRep);
userPolicy.setOwner(marta.getId());

PolicyRepresentation policyRep = new PolicyRepresentation();
policyRep.setName(KeycloakModelUtils.generateId());
policyRep.setType("uma");
policyRep.addPolicy(userPolicy.getId());

Policy policy = policyStore.create(resourceServer, policyRep);
policy.setOwner(marta.getId());
policy.setResourceType("patient-data");

Scope scope = provider.getStoreFactory().getScopeStore().findByName(resourceServer, "read");
policy.addScope(scope);
}

private List<PolicyRepresentation> getAssociatedPolicies(UmaPermissionRepresentation permission) {
return getClient(getRealm()).authorization().policies().policy(permission.getId()).associatedPolicies();
}
Expand Down
Loading
0