libre.sh/keycloak-scim
2
0
Fork 0
Code Issues 14 Pull requests Releases Packages Activity Actions 6

[KEYCLOAK-4555] - Fixes and improvements to evaluation code

Browse source
This commit is contained in:
Pedro Igor 2017-03-10 17:34:05 -03:00
parent 45caea4dbd
commit e7e6314146
12 changed files with 161 additions and 93 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
model/infinispan/src/main/java/org/keycloak/models/authorization/infinispan/CachedPolicyStore.java
View file

@ -285,7 +285,7 @@ public class CachedPolicyStore implements PolicyStore {
@Override
public void removeScope(Scope scope) {
getDelegateForUpdate().removeScope(scope);
getDelegateForUpdate().removeScope(getStoreFactory().getScopeStore().findById(scope.getId(), cached.getResourceServerId()));
cached.removeScope(scope);
}

14
model/infinispan/src/main/java/org/keycloak/models/authorization/infinispan/CachedScopeStore.java
View file

@ -22,6 +22,7 @@ import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import org.infinispan.Cache;
import org.keycloak.authorization.model.ResourceServer;
@ -202,6 +203,19 @@ public class CachedScopeStore implements ScopeStore {
return this.updated;
}
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || !Scope.class.isInstance(o)) return false;
Scope that = (Scope) o;
return Objects.equals(getId(), that.getId());
}
@Override
public int hashCode() {
return Objects.hash(getId());
}
};
}

14
model/infinispan/src/main/java/org/keycloak/models/authorization/infinispan/entities/CachedScope.java
View file

@ -22,6 +22,7 @@ import org.keycloak.authorization.model.ResourceServer;
import org.keycloak.authorization.model.Scope;
import java.io.Serializable;
import java.util.Objects;
/**
* @author <a href="mailto:psilva@redhat.com">Pedro Igor</a>
@ -79,4 +80,17 @@ public class CachedScope implements Scope, Serializable {
public String getResourceServerId() {
return this.resourceServerId;
}
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || !Scope.class.isInstance(o)) return false;
Scope that = (Scope) o;
return Objects.equals(id, that.getId());
}
@Override
public int hashCode() {
return Objects.hash(id);
}
}

6
model/jpa/src/main/java/org/keycloak/authorization/jpa/entities/ScopeEntity.java
View file

@ -114,9 +114,9 @@ public class ScopeEntity implements Scope {
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
ScopeEntity that = (ScopeEntity) o;
return Objects.equals(id, that.id);
if (o == null || !Scope.class.isInstance(o)) return false;
Scope that = (Scope) o;
return Objects.equals(id, that.getId());
}
@Override

2
server-spi-private/src/main/java/org/keycloak/authorization/permission/ResourcePermission.java
View file

@ -57,7 +57,7 @@ public class ResourcePermission {
* @return a lit of permitted scopes
*/
public List<Scope> getScopes() {
return Collections.unmodifiableList(this.scopes);
return this.scopes;
}
/**

20
server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/DecisionResultCollector.java
View file

@ -18,16 +18,16 @@
package org.keycloak.authorization.policy.evaluation;
import org.keycloak.authorization.Decision;
import org.keycloak.authorization.model.Policy;
import org.keycloak.authorization.permission.ResourcePermission;
import org.keycloak.representations.idm.authorization.DecisionStrategy;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import org.keycloak.authorization.Decision;
import org.keycloak.authorization.model.Policy;
import org.keycloak.authorization.permission.ResourcePermission;
import org.keycloak.representations.idm.authorization.DecisionStrategy;
/**
* @author <a href="mailto:psilva@redhat.com">Pedro Igor</a>
*/
@ -49,19 +49,21 @@ public abstract class DecisionResultCollector implements Decision<DefaultEvaluat
@Override
public void onComplete() {
for (Result result : results.values()) {
int deniedCount = result.getResults().size();
for (Result.PolicyResult policyResult : result.getResults()) {
if (isGranted(policyResult)) {
policyResult.setStatus(Effect.PERMIT);
deniedCount--;
} else {
policyResult.setStatus(Effect.DENY);
}
}
if (result.getResults().stream()
.filter(policyResult -> Effect.DENY.equals(policyResult.getStatus())).count() > 0) {
result.setStatus(Effect.DENY);
} else {
if (deniedCount == 0) {
result.setStatus(Effect.PERMIT);
} else {
result.setStatus(Effect.DENY);
}
}

2
server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/DefaultPolicyEvaluator.java
View file

@ -85,7 +85,7 @@ public class DefaultPolicyEvaluator implements PolicyEvaluator {
evaluatePolicies(() -> policyStore.findByScopeIds(scopes.stream().map(Scope::getId).collect(Collectors.toList()), resourceServer.getId()), consumer);
}
if (PolicyEnforcementMode.PERMISSIVE.equals(enforcementMode) && verified.get()) {
if (PolicyEnforcementMode.PERMISSIVE.equals(enforcementMode) && !verified.get()) {
createEvaluation(permission, executionContext, decision, null, null).grant();
}
}

24
services/src/main/java/org/keycloak/authorization/admin/representation/PolicyEvaluationResponse.java
View file

@ -18,6 +18,17 @@
package org.keycloak.authorization.admin.representation;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Comparator;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.keycloak.authorization.AuthorizationProvider;
import org.keycloak.authorization.Decision.Effect;
import org.keycloak.authorization.common.KeycloakIdentity;
@ -33,17 +44,6 @@ import org.keycloak.representations.idm.authorization.PolicyRepresentation;
import org.keycloak.representations.idm.authorization.ResourceRepresentation;
import org.keycloak.representations.idm.authorization.ScopeRepresentation;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Comparator;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Stream;
/**
* @author <a href="mailto:psilva@redhat.com">Pedro Igor</a>
*/
@ -64,7 +64,7 @@ public class PolicyEvaluationResponse {
AccessToken accessToken = identity.getAccessToken();
AccessToken.Authorization authorizationData = new AccessToken.Authorization();
authorizationData.setPermissions(Permissions.allPermits(results, authorization, resourceServer));
authorizationData.setPermissions(Permissions.permits(results, authorization, resourceServer.getId()));
accessToken.setAuthorization(authorizationData);
response.rpt = accessToken;

49
services/src/main/java/org/keycloak/authorization/entitlement/EntitlementService.java
View file

@ -17,6 +17,30 @@
*/
package org.keycloak.authorization.entitlement;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import javax.ws.rs.Consumes;
import javax.ws.rs.GET;
import javax.ws.rs.OPTIONS;
import javax.ws.rs.POST;
import javax.ws.rs.Path;
import javax.ws.rs.PathParam;
import javax.ws.rs.Produces;
import javax.ws.rs.container.AsyncResponse;
import javax.ws.rs.container.Suspended;
import javax.ws.rs.core.Context;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.Response.Status;
import org.jboss.resteasy.spi.HttpRequest;
import org.keycloak.OAuth2Constants;
import org.keycloak.OAuthErrorException;
@ -50,29 +74,6 @@ import org.keycloak.services.ErrorResponse;
import org.keycloak.services.ErrorResponseException;
import org.keycloak.services.resources.Cors;
import javax.ws.rs.Consumes;
import javax.ws.rs.GET;
import javax.ws.rs.OPTIONS;
import javax.ws.rs.POST;
import javax.ws.rs.Path;
import javax.ws.rs.PathParam;
import javax.ws.rs.Produces;
import javax.ws.rs.container.AsyncResponse;
import javax.ws.rs.container.Suspended;
import javax.ws.rs.core.Context;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.Response.Status;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Stream;
/**
* @author <a href="mailto:psilva@redhat.com">Pedro Igor</a>
*/
@ -126,7 +127,7 @@ public class EntitlementService {
@Override
protected void onComplete(List<Result> results) {
List<Permission> entitlements = Permissions.allPermits(results, authorization, resourceServer);
List<Permission> entitlements = Permissions.permits(results, authorization, resourceServer.getId());
if (entitlements.isEmpty()) {
HashMap<Object, Object> error = new HashMap<>();

96
services/src/main/java/org/keycloak/authorization/util/Permissions.java
View file

@ -134,54 +134,82 @@ public final class Permissions {
return permissions;
}
public static List<Permission> allPermits(List<Result> evaluation, AuthorizationProvider authorizationProvider, ResourceServer resourceServer) {
public static List<Permission> permits(List<Result> evaluation, AuthorizationProvider authorizationProvider, String resourceServerId) {
Map<String, Permission> permissions = new HashMap<>();
for (Result evaluationResult : evaluation) {
ResourcePermission permission = evaluationResult.getPermission();
for (Result result : evaluation) {
Set<Scope> deniedScopes = new HashSet<>();
Set<Scope> grantedScopes = new HashSet<>();
boolean resourceDenied = false;
ResourcePermission permission = result.getPermission();
List<Result.PolicyResult> results = result.getResults();
int deniedCount = results.size();
// if overall decision was a DENY, check for scope-based policies for a PERMIT
if (evaluationResult.getEffect().equals(Effect.DENY)) {
for (Result.PolicyResult result : evaluationResult.getResults()) {
Policy policy = result.getPolicy();
for (Result.PolicyResult policyResult : results) {
Policy policy = policyResult.getPolicy();
Set<Scope> policyScopes = policy.getScopes();
if ("scope".equals(policy.getType())) {
Set<Resource> resources = policy.getResources();
if (Effect.PERMIT.equals(result.getStatus())) {
List<Scope> scopes = policy.getScopes().stream().collect(Collectors.toList());
if (!resources.isEmpty()) {
resources.forEach(resource -> grantPermission(authorizationProvider, permissions, new ResourcePermission(resource, scopes, policy.getResourceServer()), resourceServer.getId()));
} else {
grantPermission(authorizationProvider, permissions, new ResourcePermission(permission.getResource(), scopes, policy.getResourceServer()), resourceServer.getId());
}
}
if (Effect.PERMIT.equals(policyResult.getStatus())) {
if (isScopePermission(policy)) {
// try to grant any scope from a scope-based permission
grantedScopes.addAll(policyScopes);
} else if (isResourcePermission(policy)) {
// we assume that all requested scopes should be granted given that we are processing a resource-based permission.
// Later they will be filtered based on any denied scope, if any.
// TODO: we could probably provide a configuration option to let users decide whether or not a resource-based permission should grant all scopes associated with the resource.
grantedScopes.addAll(permission.getScopes());
}
deniedCount--;
} else {
if (isScopePermission(policy)) {
// store all scopes associated with the scope-based permission
deniedScopes.addAll(policyScopes);
} else if (isResourcePermission(policy)) {
// we should not grant anything
resourceDenied = true;
break;
}
}
continue;
}
grantPermission(authorizationProvider, permissions, permission, resourceServer.getId());
if (!resourceDenied) {
// remove any scope denied from the list of granted scopes
if (!deniedScopes.isEmpty()) {
grantedScopes.removeAll(deniedScopes);
}
// if there are no policy results is because the permission didn't match any policy.
// In this case, if results is empty is because we are in permissive mode.
if (!results.isEmpty()) {
// update the current permission with the granted scopes
permission.getScopes().clear();
permission.getScopes().addAll(grantedScopes);
}
if (deniedCount == 0) {
result.setStatus(Effect.PERMIT);
grantPermission(authorizationProvider, permissions, permission, resourceServerId);
} else {
// if a full deny or resource denied or the requested scopes were denied
if (deniedCount == results.size() || resourceDenied || (!deniedScopes.isEmpty() && grantedScopes.isEmpty())) {
result.setStatus(Effect.DENY);
} else {
result.setStatus(Effect.PERMIT);
grantPermission(authorizationProvider, permissions, permission, resourceServerId);
}
}
}
}
return permissions.values().stream().collect(Collectors.toList());
}
public static List<Permission> permits(List<Result> evaluation, AuthorizationProvider authorizationProvider, String resourceServer) {
Map<String, Permission> permissions = new HashMap<>();
private static boolean isResourcePermission(Policy policy) {
return "resource".equals(policy.getType());
}
for (Result evaluationResult : evaluation) {
ResourcePermission permission = evaluationResult.getPermission();
if (evaluationResult.getEffect().equals(Effect.DENY)) {
continue;
}
grantPermission(authorizationProvider, permissions, permission, resourceServer);
}
return permissions.values().stream().collect(Collectors.toList());
private static boolean isScopePermission(Policy policy) {
return "scope".equals(policy.getType());
}
private static void grantPermission(AuthorizationProvider authorizationProvider, Map<String, Permission> permissions, ResourcePermission permission, String resourceServer) {

14
testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/adapter/page/PhotozClientAuthzTestApp.java
View file

@ -32,6 +32,7 @@ import java.net.URL;
import static org.keycloak.testsuite.util.WaitUtils.pause;
import static org.keycloak.testsuite.util.WaitUtils.waitForPageToLoad;
import static org.keycloak.testsuite.util.WaitUtils.waitUntilElement;
/**
* @author <a href="mailto:psilva@redhat.com">Pedro Igor</a>
@ -99,7 +100,8 @@ public class PhotozClientAuthzTestApp extends AbstractPageWithInjectedUrl {
}
public void logOut() {
signOutButton.click(); // Sometimes doesn't work in PhantomJS!
waitUntilElement(signOutButton); // Sometimes doesn't work in PhantomJS!
signOutButton.click();
pause(WAIT_AFTER_OPERATION);
}
@ -113,7 +115,15 @@ public class PhotozClientAuthzTestApp extends AbstractPageWithInjectedUrl {
pause(WAIT_AFTER_OPERATION);
}
public void login(String username, String password, String... scopes) {
public void login(String username, String password, String... scopes) throws InterruptedException {
if (this.driver.getCurrentUrl().startsWith(getInjectedUrl().toString())) {
Thread.sleep(2000);
logOut();
navigateTo();
}
Thread.sleep(2000);
if (scopes.length > 0) {
StringBuilder scopesValue = new StringBuilder();

11
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/example/authorization/AbstractPhotozExampleAdapterTest.java
View file

@ -143,7 +143,7 @@ public abstract class AbstractPhotozExampleAdapterTest extends AbstractExampleAd
}
@Test
public void createAlbumWithInvalidUser() {
public void createAlbumWithInvalidUser() throws Exception {
try {
this.deployer.deploy(RESOURCE_SERVER_ID);
@ -470,6 +470,7 @@ public abstract class AbstractPhotozExampleAdapterTest extends AbstractExampleAd
this.clientPage.createAlbum(resourceName);
this.clientPage.logOut();
loginToClientPage("admin", "admin");
this.clientPage.navigateToAdminAlbum();
@ -629,7 +630,7 @@ public abstract class AbstractPhotozExampleAdapterTest extends AbstractExampleAd
//KEYCLOAK-3777
@Test
public void testEntitlementRequest() {
public void testEntitlementRequest() throws Exception {
try {
this.deployer.deploy(RESOURCE_SERVER_ID);
@ -663,13 +664,11 @@ public abstract class AbstractPhotozExampleAdapterTest extends AbstractExampleAd
}
private void deleteAllCookiesForClientPage() {
clientPage.navigateTo();
driver.manage().deleteAllCookies();
}
private void loginToClientPage(String username, String password, String... scopes) {
private void loginToClientPage(String username, String password, String... scopes) throws InterruptedException {
// We need to log out by deleting cookies because the log out button sometimes doesn't work in PhantomJS
deleteAllCookiesForClientPage();
deleteAllCookiesForTestRealm();
clientPage.navigateTo();
clientPage.login(username, password, scopes);