From 829bcf5eafc9ed584e2c4716abdd9b9433c83ac2 Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Tue, 23 May 2017 17:50:06 -0300 Subject: [PATCH] Fix to evaluation tool --- .../PolicyEvaluationResponse.java | 12 ++-- .../admin/PermissionService.java | 12 ++++ .../authorization/admin/PolicyService.java | 6 +- .../admin/PolicyTypeService.java | 8 +++ .../PolicyEvaluationResponseBuilder.java | 58 ++++--------------- .../resources/js/authz/authz-controller.js | 23 ++++++-- ...esource-server-policy-evaluate-result.html | 2 +- .../resource-server-policy-evaluate.html | 2 +- 8 files changed, 61 insertions(+), 62 deletions(-) diff --git a/core/src/main/java/org/keycloak/representations/idm/authorization/PolicyEvaluationResponse.java b/core/src/main/java/org/keycloak/representations/idm/authorization/PolicyEvaluationResponse.java index 45657713a4..bfa4c30d11 100644 --- a/core/src/main/java/org/keycloak/representations/idm/authorization/PolicyEvaluationResponse.java +++ b/core/src/main/java/org/keycloak/representations/idm/authorization/PolicyEvaluationResponse.java @@ -19,13 +19,11 @@ package org.keycloak.representations.idm.authorization; import org.keycloak.representations.AccessToken; -import org.keycloak.representations.idm.authorization.DecisionEffect; -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.HashSet; import java.util.List; +import java.util.Set; /** * @author Pedro Igor @@ -123,7 +121,7 @@ public class PolicyEvaluationResponse { private PolicyRepresentation policy; private DecisionEffect status; private List associatedPolicies; - private List scopes = new ArrayList<>(); + private Set scopes = new HashSet<>(); public PolicyRepresentation getPolicy() { return policy; @@ -162,11 +160,11 @@ public class PolicyEvaluationResponse { return this.policy.equals(policy.getPolicy()); } - public void setScopes(List scopes) { + public void setScopes(Set scopes) { this.scopes = scopes; } - public List getScopes() { + public Set getScopes() { return scopes; } } diff --git a/services/src/main/java/org/keycloak/authorization/admin/PermissionService.java b/services/src/main/java/org/keycloak/authorization/admin/PermissionService.java index 4ada87d316..ba4fae255d 100644 --- a/services/src/main/java/org/keycloak/authorization/admin/PermissionService.java +++ b/services/src/main/java/org/keycloak/authorization/admin/PermissionService.java @@ -38,6 +38,18 @@ public class PermissionService extends PolicyService { return new PolicyTypeResourceService(policy, resourceServer, authorization, auth); } + @Override + protected PolicyTypeService doCreatePolicyTypeResource(String type) { + return new PolicyTypeService(type, resourceServer, authorization, auth) { + @Override + protected List doSearch(Integer firstResult, Integer maxResult, Map filters) { + filters.put("permission", new String[] {Boolean.TRUE.toString()}); + filters.put("type", new String[] {type}); + return super.doSearch(firstResult, maxResult, filters); + } + }; + } + @Override protected List doSearch(Integer firstResult, Integer maxResult, Map filters) { filters.put("permission", new String[] {Boolean.TRUE.toString()}); diff --git a/services/src/main/java/org/keycloak/authorization/admin/PolicyService.java b/services/src/main/java/org/keycloak/authorization/admin/PolicyService.java index 011aa2d3a1..e6700422a6 100644 --- a/services/src/main/java/org/keycloak/authorization/admin/PolicyService.java +++ b/services/src/main/java/org/keycloak/authorization/admin/PolicyService.java @@ -75,7 +75,7 @@ public class PolicyService { PolicyProviderFactory providerFactory = getPolicyProviderFactory(type); if (providerFactory != null) { - return new PolicyTypeService(type, resourceServer, authorization, auth); + return doCreatePolicyTypeResource(type); } Policy policy = authorization.getStoreFactory().getPolicyStore().findById(type, resourceServer.getId()); @@ -83,6 +83,10 @@ public class PolicyService { return doCreatePolicyResource(policy); } + protected PolicyTypeService doCreatePolicyTypeResource(String type) { + return new PolicyTypeService(type, resourceServer, authorization, auth); + } + protected Object doCreatePolicyResource(Policy policy) { return new PolicyResourceService(policy, resourceServer, authorization, auth); } diff --git a/services/src/main/java/org/keycloak/authorization/admin/PolicyTypeService.java b/services/src/main/java/org/keycloak/authorization/admin/PolicyTypeService.java index c2e4db5c55..5f03dad3f4 100644 --- a/services/src/main/java/org/keycloak/authorization/admin/PolicyTypeService.java +++ b/services/src/main/java/org/keycloak/authorization/admin/PolicyTypeService.java @@ -17,6 +17,8 @@ package org.keycloak.authorization.admin; import java.io.IOException; +import java.util.List; +import java.util.Map; import javax.ws.rs.Path; @@ -88,4 +90,10 @@ public class PolicyTypeService extends PolicyService { PolicyProviderFactory providerFactory = authorization.getProviderFactory(policy.getType()); return ModelToRepresentation.toRepresentation(policy, providerFactory.getRepresentationType(), authorization); } + + @Override + protected List doSearch(Integer firstResult, Integer maxResult, Map filters) { + filters.put("type", new String[] {type}); + return super.doSearch(firstResult, maxResult, filters); + } } diff --git a/services/src/main/java/org/keycloak/authorization/admin/representation/PolicyEvaluationResponseBuilder.java b/services/src/main/java/org/keycloak/authorization/admin/representation/PolicyEvaluationResponseBuilder.java index c42472549c..4b59450f5a 100644 --- a/services/src/main/java/org/keycloak/authorization/admin/representation/PolicyEvaluationResponseBuilder.java +++ b/services/src/main/java/org/keycloak/authorization/admin/representation/PolicyEvaluationResponseBuilder.java @@ -19,12 +19,10 @@ package org.keycloak.authorization.admin.representation; import org.keycloak.authorization.AuthorizationProvider; import org.keycloak.authorization.Decision; import org.keycloak.authorization.common.KeycloakIdentity; -import org.keycloak.authorization.model.Policy; import org.keycloak.authorization.model.ResourceServer; import org.keycloak.authorization.model.Scope; import org.keycloak.authorization.policy.evaluation.Result; import org.keycloak.authorization.util.Permissions; -import org.keycloak.models.utils.ModelToRepresentation; import org.keycloak.representations.AccessToken; import org.keycloak.representations.idm.authorization.DecisionEffect; import org.keycloak.representations.idm.authorization.PolicyEvaluationResponse; @@ -38,7 +36,7 @@ import java.util.Comparator; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.function.Consumer; +import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -128,37 +126,8 @@ public class PolicyEvaluationResponseBuilder { List scopes = result.getScopes(); - if (scopes == null) { - scopes = new ArrayList<>(); - result.setScopes(scopes); - } - - List currentScopes = evaluationResultRepresentation.getScopes(); - - if (currentScopes != null) { - List allowedScopes = result.getAllowedScopes(); - for (ScopeRepresentation scope : currentScopes) { - if (!scopes.contains(scope)) { - scopes.add(scope); - } - if (evaluationResultRepresentation.getStatus().equals(Decision.Effect.PERMIT)) { - if (!allowedScopes.contains(scope)) { - allowedScopes.add(scope); - } - } else { - evaluationResultRepresentation.getPolicies().forEach(new Consumer() { - @Override - public void accept(PolicyEvaluationResponse.PolicyResultRepresentation policyResultRepresentation) { - if (policyResultRepresentation.getStatus().equals(Decision.Effect.PERMIT)) { - if (!allowedScopes.contains(scope)) { - allowedScopes.add(scope); - } - } - } - }); - } - } - result.setAllowedScopes(allowedScopes); + if (DecisionEffect.PERMIT.equals(result.getStatus())) { + result.setAllowedScopes(scopes); } if (resource.getId() != null) { @@ -176,19 +145,7 @@ public class PolicyEvaluationResponseBuilder { for (PolicyEvaluationResponse.PolicyResultRepresentation policy : new ArrayList<>(evaluationResultRepresentation.getPolicies())) { if (!policies.contains(policy)) { policies.add(policy); - } else { - policy = policies.get(policies.indexOf(policy)); } - - if (policy.getStatus().equals(Decision.Effect.DENY)) { - Policy policyModel = authorization.getStoreFactory().getPolicyStore().findById(policy.getPolicy().getId(), resourceServer.getId()); - for (ScopeRepresentation scope : policyModel.getScopes().stream().map(scopeModel -> ModelToRepresentation.toRepresentation(scopeModel, authorization)).collect(Collectors.toList())) { - if (!policy.getScopes().contains(scope) && policyModel.getScopes().stream().filter(policyScope -> policyScope.getId().equals(scope.getId())).findFirst().isPresent()) { - result.getAllowedScopes().remove(scope); - policy.getScopes().add(scope); - } - } - } else {} } }); @@ -207,12 +164,19 @@ public class PolicyEvaluationResponseBuilder { representation.setType(policy.getPolicy().getType()); representation.setDecisionStrategy(policy.getPolicy().getDecisionStrategy()); + representation.setResources(policy.getPolicy().getResources().stream().map(resource -> resource.getName()).collect(Collectors.toSet())); + + Set scopeNames = policy.getPolicy().getScopes().stream().map(scope -> scope.getName()).collect(Collectors.toSet()); + + representation.setScopes(scopeNames); + policyResultRep.setPolicy(representation); + if (policy.getStatus() == Decision.Effect.DENY) { policyResultRep.setStatus(DecisionEffect.DENY); + policyResultRep.setScopes(representation.getScopes()); } else { policyResultRep.setStatus(DecisionEffect.PERMIT); - } policyResultRep.setAssociatedPolicies(policy.getAssociatedPolicies().stream().map(result -> toRepresentation(result, authorization)).collect(Collectors.toList())); diff --git a/themes/src/main/resources/theme/base/admin/resources/js/authz/authz-controller.js b/themes/src/main/resources/theme/base/admin/resources/js/authz/authz-controller.js index 700d7f7747..2cce1381b6 100644 --- a/themes/src/main/resources/theme/base/admin/resources/js/authz/authz-controller.js +++ b/themes/src/main/resources/theme/base/admin/resources/js/authz/authz-controller.js @@ -2082,13 +2082,21 @@ module.controller('PolicyEvaluateCtrl', function($scope, $http, $route, $locatio $scope.client = client; $scope.clients = clients; $scope.roles = roles; - $scope.authzRequest = {}; - $scope.authzRequest.resources = []; - $scope.authzRequest.context = {}; - $scope.authzRequest.context.attributes = {}; - $scope.authzRequest.roleIds = []; + authzRequest = {}; + authzRequest.resources = []; + authzRequest.context = {}; + authzRequest.context.attributes = {}; + authzRequest.roleIds = []; $scope.resultUrl = resourceUrl + '/partials/authz/policy/resource-server-policy-evaluate-result.html'; + $scope.authzRequest = angular.copy(authzRequest); + + $scope.$watch('authzRequest', function() { + if (!angular.equals($scope.authzRequest, authzRequest)) { + $scope.changed = true; + } + }, true); + $scope.addContextAttribute = function() { if (!$scope.newContextAttribute.value || $scope.newContextAttribute.value == '') { Notifications.error("You must provide a value to a context attribute."); @@ -2396,4 +2404,9 @@ module.controller('PolicyEvaluateCtrl', function($scope, $http, $route, $locatio $scope.authzRequest.userId = user.id; } + + $scope.reset = function() { + $scope.authzRequest = angular.copy(authzRequest); + $scope.changed = false; + } }); \ No newline at end of file diff --git a/themes/src/main/resources/theme/base/admin/resources/partials/authz/policy/resource-server-policy-evaluate-result.html b/themes/src/main/resources/theme/base/admin/resources/partials/authz/policy/resource-server-policy-evaluate-result.html index 8e9061c4e9..cd8007f3a3 100644 --- a/themes/src/main/resources/theme/base/admin/resources/partials/authz/policy/resource-server-policy-evaluate-result.html +++ b/themes/src/main/resources/theme/base/admin/resources/partials/authz/policy/resource-server-policy-evaluate-result.html @@ -45,7 +45,7 @@ href="#/realms/{{realm.realm}}/clients/{{client.id}}/authz/resource-server/permission/{{policyResult.policy.type}}/{{policyResult.policy.id}}">{{policyResult.policy.name}} decision was {{policyResult.status}} {{policyResult.status}} - by {{policyResult.policy.decisionStrategy}} decision. {{policyResult.scopes.length > 0 ? 'Denied Scopes:' : ''}} {{scope.name}}{{$last ? '' : ', '}}{{policyResult.scopes.length > 0 ? '.' : ''}} + by {{policyResult.policy.decisionStrategy}} decision. {{policyResult.policy.scopes.length > 0 ? (policyResult.status == 'DENY' ? 'Denied Scopes:' : 'Granted Scopes:') : ''}} {{scope}}{{$last ? '' : ', '}}{{policyResult.policy.scopes.length > 0 ? '.' : ''}}