diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/DecisionPermissionCollector.java b/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/DecisionPermissionCollector.java index 61d6a26753..e9260bc5f1 100644 --- a/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/DecisionPermissionCollector.java +++ b/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/DecisionPermissionCollector.java @@ -91,7 +91,7 @@ public class DecisionPermissionCollector extends AbstractDecisionCollector { userManagedPermissions.add(policyResult); } if (!resourceGranted) { - resourceGranted = containsResource; + resourceGranted = isGrantingAccessToResource(resource, policy) && containsResource; } } else { if (isResourcePermission(policy)) { @@ -150,6 +150,25 @@ public class DecisionPermissionCollector extends AbstractDecisionCollector { } } + /** + * Checks if the given {@code policy} is eligible to grant access to a resource. Resources are only granted if policy is + * not a scope-permission or, if so, the resource is a user-owned resource so that permissions can be overridden when + * inheriting policies from a typed/parent resource. + * + * @param resource the resource + * @param policy the policy that grants access to the resources + * @return {@code true} if the resource should be granted + */ + private boolean isGrantingAccessToResource(Resource resource, Policy policy) { + boolean scopePermission = isScopePermission(policy); + + if (!scopePermission) { + return true; + } + + return resource != null && !resource.getOwner().equals(resourceServer.getId()); + } + public Collection results() { return permissions; } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/EntitlementAPITest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/EntitlementAPITest.java index 8d46a9a953..ad0ba87cb7 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/EntitlementAPITest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/EntitlementAPITest.java @@ -16,6 +16,7 @@ */ package org.keycloak.testsuite.authz; +import static org.hamcrest.CoreMatchers.hasItem; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -45,7 +46,9 @@ import org.jboss.arquillian.test.api.ArquillianResource; import org.jetbrains.annotations.NotNull; import org.junit.After; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.keycloak.OAuth2Constants; import org.keycloak.admin.client.resource.AuthorizationResource; import org.keycloak.admin.client.resource.ClientResource; @@ -58,7 +61,6 @@ import org.keycloak.authorization.client.AuthzClient; import org.keycloak.authorization.client.Configuration; import org.keycloak.authorization.client.representation.TokenIntrospectionResponse; import org.keycloak.authorization.client.util.HttpResponseException; -import org.keycloak.authorization.permission.ResourcePermission; import org.keycloak.common.util.Base64Url; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.protocol.oidc.OIDCLoginProtocol; @@ -115,6 +117,9 @@ public class EntitlementAPITest extends AbstractAuthzTest { @ArquillianResource protected ContainerController controller; + @Rule + public ExpectedException expectedException = ExpectedException.none(); + @Override public void addTestRealms(List testRealms) { testRealms.add(RealmBuilder.create().name("authz-test") @@ -233,13 +238,12 @@ public class EntitlementAPITest extends AbstractAuthzTest { obj.put("claim-a", "claim-a"); request.setClaimToken(Base64Url.encode(JsonSerialization.writeValueAsBytes(obj))); + this.expectedException.expect(AuthorizationDeniedException.class); + this.expectedException.expectCause(Matchers.allOf(Matchers.instanceOf(HttpResponseException.class), Matchers.hasProperty("statusCode", Matchers.is(403)))); + this.expectedException.expectMessage("Public clients are not allowed to send claims"); + this.expectedException.reportMissingExceptionWithMessage("Should fail, public clients not allowed"); - try { - getAuthzClient(AUTHZ_CLIENT_CONFIG).authorization(response.getAccessToken()).authorize(request); - } catch (AuthorizationDeniedException expected) { - assertEquals(403, HttpResponseException.class.cast(expected.getCause()).getStatusCode()); - assertTrue(HttpResponseException.class.cast(expected.getCause()).toString().contains("Public clients are not allowed to send claims")); - } + getAuthzClient(AUTHZ_CLIENT_CONFIG).authorization(response.getAccessToken()).authorize(request); } @Test @@ -1925,6 +1929,62 @@ public class EntitlementAPITest extends AbstractAuthzTest { } } + @Test + public void testDenyScopeNotManagedByScopePolicy() throws Exception { + ClientResource client = getClient(getRealm(), RESOURCE_SERVER_TEST); + AuthorizationResource authorization = client.authorization(); + + JSPolicyRepresentation policy = new JSPolicyRepresentation(); + + policy.setName(KeycloakModelUtils.generateId()); + policy.setCode("$evaluation.grant();"); + + authorization.policies().js().create(policy).close(); + + ResourceRepresentation resource = new ResourceRepresentation(); + + resource.setName(KeycloakModelUtils.generateId()); + resource.addScope("sensors:view", "sensors:update", "sensors:delete"); + + try (Response response = authorization.resources().create(resource)) { + resource = response.readEntity(ResourceRepresentation.class); + } + + ScopePermissionRepresentation permission = new ScopePermissionRepresentation(); + + permission.setName(KeycloakModelUtils.generateId()); + permission.addResource(resource.getId()); + permission.addScope("sensors:view"); + permission.addPolicy(policy.getName()); + + authorization.permissions().scope().create(permission).close(); + + String accessToken = new OAuthClient().realm("authz-test").clientId(RESOURCE_SERVER_TEST).doGrantAccessTokenRequest("secret", "kolo", "password").getAccessToken(); + AuthzClient authzClient = getAuthzClient(AUTHZ_CLIENT_CONFIG); + AuthorizationRequest request = new AuthorizationRequest(); + + request.addPermission(resource.getId(), "sensors:view"); + + AuthorizationResponse response = authzClient.authorization(accessToken).authorize(request); + assertNotNull(response.getToken()); + Collection permissions = toAccessToken(response.getToken()).getAuthorization().getPermissions(); + assertEquals(1, permissions.size()); + + for (Permission grantedPermission : permissions) { + assertEquals(resource.getId(), grantedPermission.getResourceId()); + assertEquals(1, grantedPermission.getScopes().size()); + assertThat(grantedPermission.getScopes(), hasItem("sensors:view")); + } + + request = new AuthorizationRequest(); + request.addPermission(resource.getId(), "sensors:update"); + this.expectedException.expect(AuthorizationDeniedException.class); + this.expectedException.expectCause(Matchers.allOf(Matchers.instanceOf(HttpResponseException.class), Matchers.hasProperty("statusCode", Matchers.is(403)))); + this.expectedException.reportMissingExceptionWithMessage("should fail, session invalidated"); + + authzClient.authorization().authorize(request); + } + private void testRptRequestWithResourceName(String configFile) { Metadata metadata = new Metadata();