Merge pull request #4863 from pedroigor/KEYCLOAK-5877
[KEYCLOAK-5877] - Allow save permissions without policies
This commit is contained in:
commit
a66fe003d3
7 changed files with 106 additions and 38 deletions
|
@ -72,7 +72,9 @@ public class ResourcePermissionForm extends Form {
|
|||
resourceSelect.update(expected.getResources());
|
||||
}
|
||||
|
||||
policySelect.update(expected.getPolicies());
|
||||
if (expected.getPolicies() != null) {
|
||||
policySelect.update(expected.getPolicies());
|
||||
}
|
||||
|
||||
save();
|
||||
}
|
||||
|
|
|
@ -75,7 +75,9 @@ public class ScopePermissionForm extends Form {
|
|||
scopeSelect.update(expected.getScopes());
|
||||
}
|
||||
|
||||
policySelect.update(expected.getPolicies());
|
||||
if (expected.getPolicies() != null) {
|
||||
policySelect.update(expected.getPolicies());
|
||||
}
|
||||
|
||||
save();
|
||||
}
|
||||
|
|
|
@ -18,6 +18,7 @@ package org.keycloak.testsuite.console.authorization;
|
|||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertNull;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
|
||||
import java.util.stream.Collectors;
|
||||
|
||||
|
@ -30,7 +31,6 @@ import org.keycloak.admin.client.resource.RolePoliciesResource;
|
|||
import org.keycloak.admin.client.resource.RolesResource;
|
||||
import org.keycloak.representations.idm.RoleRepresentation;
|
||||
import org.keycloak.representations.idm.authorization.DecisionStrategy;
|
||||
import org.keycloak.representations.idm.authorization.Logic;
|
||||
import org.keycloak.representations.idm.authorization.ResourcePermissionRepresentation;
|
||||
import org.keycloak.representations.idm.authorization.ResourceRepresentation;
|
||||
import org.keycloak.representations.idm.authorization.RolePolicyRepresentation;
|
||||
|
@ -80,12 +80,28 @@ public class ResourcePermissionManagementTest extends AbstractAuthorizationSetti
|
|||
resources.create(new ResourceRepresentation("Resource B"));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testCreateWithoutPolicies() throws InterruptedException {
|
||||
authorizationPage.navigateTo();
|
||||
ResourcePermissionRepresentation expected = new ResourcePermissionRepresentation();
|
||||
|
||||
expected.setName("testCreateWithoutPolicies Permission");
|
||||
expected.setDescription("description");
|
||||
expected.addResource("Resource A");
|
||||
|
||||
expected = createPermission(expected);
|
||||
|
||||
authorizationPage.navigateTo();
|
||||
ResourcePermission actual = authorizationPage.authorizationTabs().permissions().name(expected.getName());
|
||||
assertPolicy(expected, actual);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testUpdateResource() throws InterruptedException {
|
||||
authorizationPage.navigateTo();
|
||||
ResourcePermissionRepresentation expected = new ResourcePermissionRepresentation();
|
||||
|
||||
expected.setName("Test Resource A Permission");
|
||||
expected.setName("testUpdateResource Permission");
|
||||
expected.setDescription("description");
|
||||
expected.addResource("Resource A");
|
||||
expected.addPolicy("Policy A");
|
||||
|
@ -96,7 +112,7 @@ public class ResourcePermissionManagementTest extends AbstractAuthorizationSetti
|
|||
|
||||
String previousName = expected.getName();
|
||||
|
||||
expected.setName("Changed Test Resource A Permission");
|
||||
expected.setName(expected.getName() + " Changed");
|
||||
expected.setDescription("Changed description");
|
||||
expected.setDecisionStrategy(DecisionStrategy.CONSENSUS);
|
||||
expected.getResources().clear();
|
||||
|
@ -110,7 +126,16 @@ public class ResourcePermissionManagementTest extends AbstractAuthorizationSetti
|
|||
|
||||
authorizationPage.navigateTo();
|
||||
ResourcePermission actual = authorizationPage.authorizationTabs().permissions().name(expected.getName());
|
||||
assertPolicy(expected, actual);
|
||||
|
||||
expected.getPolicies().clear();
|
||||
|
||||
authorizationPage.navigateTo();
|
||||
authorizationPage.authorizationTabs().permissions().update(expected.getName(), expected);
|
||||
assertAlertSuccess();
|
||||
|
||||
authorizationPage.navigateTo();
|
||||
actual = authorizationPage.authorizationTabs().permissions().name(expected.getName());
|
||||
assertPolicy(expected, actual);
|
||||
}
|
||||
|
||||
|
@ -119,7 +144,7 @@ public class ResourcePermissionManagementTest extends AbstractAuthorizationSetti
|
|||
authorizationPage.navigateTo();
|
||||
ResourcePermissionRepresentation expected = new ResourcePermissionRepresentation();
|
||||
|
||||
expected.setName("Test Resource B Type Permission");
|
||||
expected.setName("testUpdateResourceType Permission");
|
||||
expected.setDescription("description");
|
||||
expected.setResourceType("test-resource-type");
|
||||
expected.addPolicy("Policy A");
|
||||
|
@ -130,7 +155,7 @@ public class ResourcePermissionManagementTest extends AbstractAuthorizationSetti
|
|||
|
||||
String previousName = expected.getName();
|
||||
|
||||
expected.setName("Changed Test Resource B Type Permission");
|
||||
expected.setName(expected.getName() + " Changed");
|
||||
expected.setDescription("Changed description");
|
||||
expected.setDecisionStrategy(DecisionStrategy.CONSENSUS);
|
||||
|
||||
|
@ -152,7 +177,7 @@ public class ResourcePermissionManagementTest extends AbstractAuthorizationSetti
|
|||
authorizationPage.navigateTo();
|
||||
ResourcePermissionRepresentation expected = new ResourcePermissionRepresentation();
|
||||
|
||||
expected.setName("Test Delete Resource Permission");
|
||||
expected.setName("testDelete Permission");
|
||||
expected.setDescription("description");
|
||||
expected.addResource("Resource B");
|
||||
expected.addPolicy("Policy C");
|
||||
|
@ -170,7 +195,7 @@ public class ResourcePermissionManagementTest extends AbstractAuthorizationSetti
|
|||
authorizationPage.navigateTo();
|
||||
ResourcePermissionRepresentation expected = new ResourcePermissionRepresentation();
|
||||
|
||||
expected.setName("Test Delete Resource Permission");
|
||||
expected.setName("testDeleteFromList Permission");
|
||||
expected.setDescription("description");
|
||||
expected.addResource("Resource B");
|
||||
expected.addPolicy("Policy C");
|
||||
|
@ -195,6 +220,11 @@ public class ResourcePermissionManagementTest extends AbstractAuthorizationSetti
|
|||
assertEquals(expected.getDescription(), actual.getDescription());
|
||||
assertEquals(expected.getDecisionStrategy(), actual.getDecisionStrategy());
|
||||
assertEquals(expected.getResourceType(), actual.getResourceType());
|
||||
if (expected.getPolicies() == null) {
|
||||
assertTrue(actual.getPolicies() == null || actual.getPolicies().isEmpty());
|
||||
} else {
|
||||
assertEquals(expected.getPolicies().size(), actual.getPolicies().size());
|
||||
}
|
||||
assertEquals(0, actual.getPolicies().stream().filter(actualPolicy -> !expected.getPolicies().stream()
|
||||
.filter(expectedPolicy -> actualPolicy.equals(expectedPolicy))
|
||||
.findFirst().isPresent())
|
||||
|
|
|
@ -18,6 +18,7 @@ package org.keycloak.testsuite.console.authorization;
|
|||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertNull;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
|
||||
import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
|
@ -33,7 +34,6 @@ import org.keycloak.representations.idm.authorization.RolePolicyRepresentation;
|
|||
import org.keycloak.representations.idm.authorization.ScopePermissionRepresentation;
|
||||
import org.keycloak.representations.idm.authorization.ScopeRepresentation;
|
||||
import org.keycloak.representations.idm.authorization.UserPolicyRepresentation;
|
||||
import org.keycloak.testsuite.console.page.clients.authorization.permission.ResourcePermission;
|
||||
import org.keycloak.testsuite.console.page.clients.authorization.permission.ScopePermission;
|
||||
|
||||
/**
|
||||
|
@ -84,31 +84,17 @@ public class ScopePermissionManagementTest extends AbstractAuthorizationSettings
|
|||
}
|
||||
|
||||
@Test
|
||||
public void testUpdateScopeOnly() throws InterruptedException {
|
||||
public void testCreateWithoutPolicies() throws InterruptedException {
|
||||
authorizationPage.navigateTo();
|
||||
ScopePermissionRepresentation expected = new ScopePermissionRepresentation();
|
||||
|
||||
expected.setName("Test Scope Only Permission");
|
||||
expected.setName("testCreateWithoutPolicies Permission");
|
||||
expected.setDescription("description");
|
||||
expected.addScope("Scope C", "Scope A", "Scope B");
|
||||
expected.addPolicy("Policy C", "Policy A", "Policy B");
|
||||
expected.addResource("Resource A");
|
||||
expected.addScope("Scope A");
|
||||
|
||||
expected = createPermission(expected);
|
||||
|
||||
String previousName = expected.getName();
|
||||
|
||||
expected.setName(previousName + "Changed");
|
||||
expected.setDescription("changed");
|
||||
expected.setDecisionStrategy(DecisionStrategy.CONSENSUS);
|
||||
expected.getScopes().clear();
|
||||
expected.addScope("Scope B");
|
||||
expected.getPolicies().clear();
|
||||
expected.addPolicy("Policy C");
|
||||
|
||||
authorizationPage.navigateTo();
|
||||
authorizationPage.authorizationTabs().permissions().update(previousName, expected);
|
||||
assertAlertSuccess();
|
||||
|
||||
authorizationPage.navigateTo();
|
||||
ScopePermission actual = authorizationPage.authorizationTabs().permissions().name(expected.getName());
|
||||
assertPolicy(expected, actual);
|
||||
|
@ -119,7 +105,7 @@ public class ScopePermissionManagementTest extends AbstractAuthorizationSettings
|
|||
authorizationPage.navigateTo();
|
||||
ScopePermissionRepresentation expected = new ScopePermissionRepresentation();
|
||||
|
||||
expected.setName("Test Resource Scope Permission");
|
||||
expected.setName("testUpdateResourceScope Permission");
|
||||
expected.setDescription("description");
|
||||
expected.addResource("Resource A");
|
||||
expected.addScope("Scope A");
|
||||
|
@ -146,6 +132,47 @@ public class ScopePermissionManagementTest extends AbstractAuthorizationSettings
|
|||
authorizationPage.navigateTo();
|
||||
ScopePermission actual = authorizationPage.authorizationTabs().permissions().name(expected.getName());
|
||||
assertPolicy(expected, actual);
|
||||
|
||||
expected.getPolicies().clear();
|
||||
|
||||
authorizationPage.navigateTo();
|
||||
authorizationPage.authorizationTabs().permissions().update(expected.getName(), expected);
|
||||
assertAlertSuccess();
|
||||
|
||||
authorizationPage.navigateTo();
|
||||
actual = authorizationPage.authorizationTabs().permissions().name(expected.getName());
|
||||
assertPolicy(expected, actual);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testUpdateScopeOnly() throws InterruptedException {
|
||||
authorizationPage.navigateTo();
|
||||
ScopePermissionRepresentation expected = new ScopePermissionRepresentation();
|
||||
|
||||
expected.setName("testUpdateScopeOnly Permission");
|
||||
expected.setDescription("description");
|
||||
expected.addScope("Scope C", "Scope A", "Scope B");
|
||||
expected.addPolicy("Policy C", "Policy A", "Policy B");
|
||||
|
||||
expected = createPermission(expected);
|
||||
|
||||
String previousName = expected.getName();
|
||||
|
||||
expected.setName(previousName + "Changed");
|
||||
expected.setDescription("changed");
|
||||
expected.setDecisionStrategy(DecisionStrategy.CONSENSUS);
|
||||
expected.getScopes().clear();
|
||||
expected.addScope("Scope B");
|
||||
expected.getPolicies().clear();
|
||||
expected.addPolicy("Policy C");
|
||||
|
||||
authorizationPage.navigateTo();
|
||||
authorizationPage.authorizationTabs().permissions().update(previousName, expected);
|
||||
assertAlertSuccess();
|
||||
|
||||
authorizationPage.navigateTo();
|
||||
ScopePermission actual = authorizationPage.authorizationTabs().permissions().name(expected.getName());
|
||||
assertPolicy(expected, actual);
|
||||
}
|
||||
|
||||
@Test
|
||||
|
@ -153,7 +180,7 @@ public class ScopePermissionManagementTest extends AbstractAuthorizationSettings
|
|||
authorizationPage.navigateTo();
|
||||
ScopePermissionRepresentation expected = new ScopePermissionRepresentation();
|
||||
|
||||
expected.setName("Test Delete Scope Permission");
|
||||
expected.setName("testDelete Permission");
|
||||
expected.setDescription("description");
|
||||
expected.addScope("Scope C");
|
||||
expected.addPolicy("Policy C");
|
||||
|
@ -171,7 +198,7 @@ public class ScopePermissionManagementTest extends AbstractAuthorizationSettings
|
|||
authorizationPage.navigateTo();
|
||||
ScopePermissionRepresentation expected = new ScopePermissionRepresentation();
|
||||
|
||||
expected.setName("Test Delete Scope Permission");
|
||||
expected.setName("testDeleteFromList Permission");
|
||||
expected.setDescription("description");
|
||||
expected.addScope("Scope C");
|
||||
expected.addPolicy("Policy C");
|
||||
|
@ -196,7 +223,12 @@ public class ScopePermissionManagementTest extends AbstractAuthorizationSettings
|
|||
assertEquals(expected.getDescription(), actual.getDescription());
|
||||
assertEquals(expected.getDecisionStrategy(), actual.getDecisionStrategy());
|
||||
|
||||
assertEquals(expected.getPolicies().size(), actual.getPolicies().size());
|
||||
if (expected.getPolicies() == null) {
|
||||
assertTrue(actual.getPolicies() == null || actual.getPolicies().isEmpty());
|
||||
} else {
|
||||
assertEquals(expected.getPolicies().size(), actual.getPolicies().size());
|
||||
}
|
||||
|
||||
assertEquals(0, actual.getPolicies().stream().filter(actualPolicy -> !expected.getPolicies().stream()
|
||||
.filter(expectedPolicy -> actualPolicy.equals(expectedPolicy))
|
||||
.findFirst().isPresent())
|
||||
|
|
|
@ -1050,8 +1050,10 @@ module.controller('ResourceServerPolicyResourceDetailCtrl', function($scope, $ro
|
|||
|
||||
var policies = [];
|
||||
|
||||
for (i = 0; i < $scope.selectedPolicies.length; i++) {
|
||||
policies.push($scope.selectedPolicies[i].id);
|
||||
if ($scope.selectedPolicies) {
|
||||
for (i = 0; i < $scope.selectedPolicies.length; i++) {
|
||||
policies.push($scope.selectedPolicies[i].id);
|
||||
}
|
||||
}
|
||||
|
||||
$scope.policy.policies = policies;
|
||||
|
|
|
@ -56,7 +56,7 @@
|
|||
<label class="col-md-2 control-label" for="policies">{{:: 'authz-policy-apply-policy' | translate}} <span class="required">*</span></label>
|
||||
|
||||
<div class="col-md-6">
|
||||
<input type="hidden" ui-select2="policiesUiSelect" id="policies" data-ng-model="selectedPolicies" data-placeholder="{{:: 'authz-select-a-policy' | translate}}..." multiple required />
|
||||
<input type="hidden" ui-select2="policiesUiSelect" id="policies" data-ng-model="selectedPolicies" data-placeholder="{{:: 'authz-select-a-policy' | translate}}..." multiple />
|
||||
</div>
|
||||
|
||||
<kc-tooltip>{{:: 'authz-policy-apply-policy.tooltip' | translate}}</kc-tooltip>
|
||||
|
@ -81,7 +81,7 @@
|
|||
|
||||
<div class="form-group" data-ng-show="access.manageAuthorization">
|
||||
<div class="col-md-10 col-md-offset-2">
|
||||
<button kc-save data-ng-disabled="!changed || (selectedPolicies == null || selectedPolicies.length == 0)">{{:: 'save' | translate}}</button>
|
||||
<button kc-save data-ng-disabled="!changed">{{:: 'save' | translate}}</button>
|
||||
<button kc-reset data-ng-disabled="!changed">{{:: 'cancel' | translate}}</button>
|
||||
</div>
|
||||
</div>
|
||||
|
|
|
@ -60,7 +60,7 @@
|
|||
<label class="col-md-2 control-label" for="policies">{{:: 'authz-policy-apply-policy' | translate}} <span class="required">*</span></label>
|
||||
|
||||
<div class="col-md-6">
|
||||
<input type="hidden" ui-select2="policiesUiSelect" id="policies" data-ng-model="selectedPolicies" data-placeholder="{{:: 'authz-select-a-policy' | translate}}..." multiple required />
|
||||
<input type="hidden" ui-select2="policiesUiSelect" id="policies" data-ng-model="selectedPolicies" data-placeholder="{{:: 'authz-select-a-policy' | translate}}..." multiple />
|
||||
</div>
|
||||
|
||||
<kc-tooltip>{{:: 'authz-policy-apply-policy.tooltip' | translate}}</kc-tooltip>
|
||||
|
@ -84,7 +84,7 @@
|
|||
</fieldset>
|
||||
<div class="form-group" data-ng-show="access.manageAuthorization">
|
||||
<div class="col-md-10 col-md-offset-2">
|
||||
<button kc-save data-ng-disabled="!changed || ((selectedPolicies == null || selectedPolicies.length == 0) || (selectedScopes == null || selectedScopes.length == 0))">{{:: 'save' | translate}}</button>
|
||||
<button kc-save data-ng-disabled="!changed">{{:: 'save' | translate}}</button>
|
||||
<button kc-reset data-ng-disabled="!changed">{{:: 'cancel' | translate}}</button>
|
||||
</div>
|
||||
</div>
|
||||
|
|
Loading…
Reference in a new issue