libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases 1 Packages Activity Actions

KEYCLOAK-16082 save attributes when role is created (with REST POST request)

Browse source 
- add missing mapping code to RoleContainerResource#createRole
- extend ClientRolesTest and RealmRolesTest to check that now the attributes are saved when a role is created
- remove no longer needed code which updated roles because attributes were not saved on creation
This commit is contained in:
Daniel Fesenmeyer 2021-03-25 16:41:33 +01:00 committed by Bruno Oliveira da Silva
parent e0d660d815
commit a48d04bfe0
4 changed files with 92 additions and 105 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
services/src/main/java/org/keycloak/services/resources/admin/RoleContainerResource.java
View file

@ -53,6 +53,7 @@ import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.UriInfo;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.function.Function;
import java.util.stream.Stream;
@ -131,6 +132,13 @@ public class RoleContainerResource extends RoleResource {
RoleModel role = roleContainer.addRole(rep.getName());
role.setDescription(rep.getDescription());
Map<String, List<String>> attributes = rep.getAttributes();
if (attributes != null) {
for (Map.Entry<String, List<String>> attr : attributes.entrySet()) {
role.setAttribute(attr.getKey(), attr.getValue());
}
}
rep.setId(role.getId());
if (role.isClientRole()) {

2
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java
View file

@ -2046,7 +2046,6 @@ public class UserTest extends AbstractAdminTest {
realm.roles().create(RoleBuilder.create().name("realm-role").build());
realm.roles().create(realmCompositeRole);
realm.roles().get("realm-composite").update(realmCompositeRole);
realm.roles().create(RoleBuilder.create().name("realm-child").build());
realm.roles().get("realm-composite").addComposites(Collections.singletonList(realm.roles().get("realm-child").toRepresentation()));
@ -2061,7 +2060,6 @@ public class UserTest extends AbstractAdminTest {
realm.clients().get(clientUuid).roles().create(RoleBuilder.create().name("client-role").build());
realm.clients().get(clientUuid).roles().create(RoleBuilder.create().name("client-role2").build());
realm.clients().get(clientUuid).roles().create(clientCompositeRole);
realm.clients().get(clientUuid).roles().get("client-composite").update(clientCompositeRole);
realm.clients().get(clientUuid).roles().create(RoleBuilder.create().name("client-child").build());
realm.clients().get(clientUuid).roles().get("client-composite").addComposites(Collections.singletonList(realm.clients().get(clientUuid).roles().get("client-child").toRepresentation()));

26
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/ClientRolesTest.java
View file

@ -85,9 +85,15 @@ public class ClientRolesTest extends AbstractClientTest {
@Test
public void testAddRole() {
RoleRepresentation role1 = makeRole("role1");
role1.setDescription("role1-description");
role1.setAttributes(Collections.singletonMap("role1-attr-key", Collections.singletonList("role1-attr-val")));
rolesRsc.create(role1);
assertAdminEvents.assertEvent(getRealmId(), OperationType.CREATE, AdminEventPaths.clientRoleResourcePath(clientDbId, "role1"), role1, ResourceType.CLIENT_ROLE);
assertTrue(hasRole(rolesRsc, "role1"));
RoleRepresentation addedRole = rolesRsc.get(role1.getName()).toRepresentation();
assertEquals(role1.getName(), addedRole.getName());
assertEquals(role1.getDescription(), addedRole.getDescription());
assertEquals(role1.getAttributes(), addedRole.getAttributes());
}
@Test(expected = ClientErrorException.class)
@ -279,14 +285,6 @@ public class ClientRolesTest extends AbstractClientTest {
rolesRsc.create(role);
assertAdminEvents.assertEvent(getRealmId(), OperationType.CREATE, AdminEventPaths.clientRoleResourcePath(clientDbId,roleName), role, ResourceType.CLIENT_ROLE);
// we have to update the role to set the attributes because
// the add role endpoint only care about name and description
RoleResource roleToUpdate = rolesRsc.get(roleName);
role.setId(roleToUpdate.toRepresentation().getId());
roleToUpdate.update(role);
assertAdminEvents.assertEvent(getRealmId(), OperationType.UPDATE, AdminEventPaths.clientRoleResourcePath(clientDbId,roleName), role, ResourceType.CLIENT_ROLE);
}
List<RoleRepresentation> roles = rolesRsc.list(false);
@ -304,15 +302,7 @@ public class ClientRolesTest extends AbstractClientTest {
role.setAttributes(attributes);
rolesRsc.create(role);
assertAdminEvents.assertEvent(getRealmId(), OperationType.CREATE, AdminEventPaths.clientRoleResourcePath(clientDbId,roleName), role, ResourceType.CLIENT_ROLE);
// we have to update the role to set the attributes because
// the add role endpoint only care about name and description
RoleResource roleToUpdate = rolesRsc.get(roleName);
role.setId(roleToUpdate.toRepresentation().getId());
roleToUpdate.update(role);
assertAdminEvents.assertEvent(getRealmId(), OperationType.UPDATE, AdminEventPaths.clientRoleResourcePath(clientDbId,roleName), role, ResourceType.CLIENT_ROLE);
assertAdminEvents.assertEvent(getRealmId(), OperationType.CREATE, AdminEventPaths.clientRoleResourcePath(clientDbId,roleName), role, ResourceType.CLIENT_ROLE);
}
List<RoleRepresentation> roles = rolesRsc.list();

161
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/realm/RealmRolesTest.java
View file

@ -70,6 +70,9 @@ import org.keycloak.models.Constants;
*/
public class RealmRolesTest extends AbstractAdminTest {
private static final Map<String, List<String>> ROLE_A_ATTRIBUTES =
Collections.singletonMap("role-a-attr-key1", Collections.singletonList("role-a-attr-val1"));
private RolesResource resource;
private Map<String, String> ids = new HashMap<>();
@ -77,7 +80,7 @@ public class RealmRolesTest extends AbstractAdminTest {
@Before
public void before() {
RoleRepresentation roleA = RoleBuilder.create().name("role-a").description("Role A").build();
RoleRepresentation roleA = RoleBuilder.create().name("role-a").description("Role A").attributes(ROLE_A_ATTRIBUTES).build();
RoleRepresentation roleB = RoleBuilder.create().name("role-b").description("Role B").build();
//KEYCLOAK-2035
RoleRepresentation roleWithUsers = RoleBuilder.create().name("role-with-users").description("Role with users").build();
@ -87,7 +90,7 @@ public class RealmRolesTest extends AbstractAdminTest {
adminClient.realm(REALM_NAME).roles().create(roleWithUsers);
adminClient.realm(REALM_NAME).roles().create(roleWithoutUsers);
ClientRepresentation clientRep = ClientBuilder.create().clientId("client-a").build();
try (Response response = adminClient.realm(REALM_NAME).clients().create(clientRep)) {
clientUuid = ApiUtil.getCreatedId(response);
@ -104,12 +107,12 @@ public class RealmRolesTest extends AbstractAdminTest {
for (RoleRepresentation r : adminClient.realm(REALM_NAME).clients().get(clientUuid).roles().list()) {
ids.put(r.getName(), r.getId());
}
UserRepresentation userRep = new UserRepresentation();
userRep.setUsername("test-role-member");
userRep.setEmail("test-role-member@test-role-member.com");
userRep.setRequiredActions(Collections.<String>emptyList());
userRep.setEnabled(true);
userRep.setEnabled(true);
adminClient.realm(REALM_NAME).users().create(userRep);
getCleanup().addRoleId(ids.get("role-a"));
@ -118,13 +121,13 @@ public class RealmRolesTest extends AbstractAdminTest {
getCleanup().addRoleId(ids.get("role-with-users"));
getCleanup().addRoleId(ids.get("role-without-users"));
getCleanup().addUserId(adminClient.realm(REALM_NAME).users().search(userRep.getUsername()).get(0).getId());
GroupRepresentation groupRep = new GroupRepresentation();
groupRep.setName("test-role-group");
groupRep.setPath("/test-role-group");
adminClient.realm(REALM_NAME).groups().add(groupRep);
getCleanup().addGroupId(adminClient.realm(REALM_NAME).groups().groups().get(0).getId());
resource = adminClient.realm(REALM_NAME).roles();
assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.roleResourcePath("role-a"), roleA, ResourceType.REALM_ROLE);
@ -134,10 +137,10 @@ public class RealmRolesTest extends AbstractAdminTest {
assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.clientResourcePath(clientUuid), clientRep, ResourceType.CLIENT);
assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.clientRoleResourcePath(clientUuid, "role-c"), roleC, ResourceType.CLIENT_ROLE);
assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.userResourcePath(adminClient.realm(REALM_NAME).users().search(userRep.getUsername()).get(0).getId()), userRep, ResourceType.USER);
assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.groupPath(adminClient.realm(REALM_NAME).groups().groups().get(0).getId()), groupRep, ResourceType.GROUP);
}
private RoleRepresentation makeRole(String name) {
@ -152,6 +155,7 @@ public class RealmRolesTest extends AbstractAdminTest {
assertNotNull(role);
assertEquals("role-a", role.getName());
assertEquals("Role A", role.getDescription());
assertEquals(ROLE_A_ATTRIBUTES, role.getAttributes());
assertFalse(role.isComposite());
}
@ -166,6 +170,8 @@ public class RealmRolesTest extends AbstractAdminTest {
role.setName("role-a-new");
role.setDescription("Role A New");
Map<String, List<String>> newAttributes = Collections.singletonMap("attrKeyNew", Collections.singletonList("attrValueNew"));
role.setAttributes(newAttributes);
resource.get("role-a").update(role);
assertAdminEvents.assertEvent(realmId, OperationType.UPDATE, AdminEventPaths.roleResourcePath("role-a"), role, ResourceType.REALM_ROLE);
@ -175,6 +181,7 @@ public class RealmRolesTest extends AbstractAdminTest {
assertNotNull(role);
assertEquals("role-a-new", role.getName());
assertEquals("Role A New", role.getDescription());
assertEquals(newAttributes, role.getAttributes());
assertFalse(role.isComposite());
}
@ -226,7 +233,7 @@ public class RealmRolesTest extends AbstractAdminTest {
* KEYCLOAK-2035 Verifies that Users assigned to Role are being properly retrieved as members in API endpoint for role membership
*/
@Test
public void testUsersInRole() {
public void testUsersInRole() {
RoleResource role = resource.get("role-with-users");
List<UserRepresentation> users = adminClient.realm(REALM_NAME).users().search("test-role-member", null, null, null, null, null);
@ -234,63 +241,63 @@ public class RealmRolesTest extends AbstractAdminTest {
UserResource user = adminClient.realm(REALM_NAME).users().get(users.get(0).getId());
UserRepresentation userRep = user.toRepresentation();
RoleResource roleResource = adminClient.realm(REALM_NAME).roles().get(role.toRepresentation().getName());
RoleResource roleResource = adminClient.realm(REALM_NAME).roles().get(role.toRepresentation().getName());
List<RoleRepresentation> rolesToAdd = new LinkedList<>();
rolesToAdd.add(roleResource.toRepresentation());
adminClient.realm(REALM_NAME).users().get(userRep.getId()).roles().realmLevel().add(rolesToAdd);
roleResource = adminClient.realm(REALM_NAME).roles().get(role.toRepresentation().getName());
roleResource = adminClient.realm(REALM_NAME).roles().get(role.toRepresentation().getName());
roleResource.getRoleUserMembers();
//roleResource.getRoleUserMembers().stream().forEach((member) -> log.infof("Found user {}", member.getUsername()));
assertEquals(1, roleResource.getRoleUserMembers().size());
}
/**
* KEYCLOAK-2035 Verifies that Role with no users assigned is being properly retrieved without members in API endpoint for role membership
*/
@Test
public void testUsersNotInRole() {
RoleResource role = resource.get("role-without-users");
RoleResource role = resource.get("role-without-users");
role = adminClient.realm(REALM_NAME).roles().get(role.toRepresentation().getName());
role.getRoleUserMembers();
assertEquals(0, role.getRoleUserMembers().size());
}
/**
* KEYCLOAK-4978 Verifies that Groups assigned to Role are being properly retrieved as members in API endpoint for role membership
*/
@Test
public void testGroupsInRole() {
public void testGroupsInRole() {
RoleResource role = resource.get("role-with-users");
List<GroupRepresentation> groups = adminClient.realm(REALM_NAME).groups().groups();
GroupRepresentation groupRep = groups.stream().filter(g -> g.getPath().equals("/test-role-group")).findFirst().get();
RoleResource roleResource = adminClient.realm(REALM_NAME).roles().get(role.toRepresentation().getName());
RoleResource roleResource = adminClient.realm(REALM_NAME).roles().get(role.toRepresentation().getName());
List<RoleRepresentation> rolesToAdd = new LinkedList<>();
rolesToAdd.add(roleResource.toRepresentation());
adminClient.realm(REALM_NAME).groups().group(groupRep.getId()).roles().realmLevel().add(rolesToAdd);
roleResource = adminClient.realm(REALM_NAME).roles().get(role.toRepresentation().getName());
roleResource = adminClient.realm(REALM_NAME).roles().get(role.toRepresentation().getName());
Set<GroupRepresentation> groupsInRole = roleResource.getRoleGroupMembers();
assertTrue(groupsInRole.stream().filter(g -> g.getPath().equals("/test-role-group")).findFirst().isPresent());
}
/**
* KEYCLOAK-4978 Verifies that Role with no users assigned is being properly retrieved without groups in API endpoint for role membership
*/
@Test
public void testGroupsNotInRole() {
RoleResource role = resource.get("role-without-users");
RoleResource role = resource.get("role-without-users");
role = adminClient.realm(REALM_NAME).roles().get(role.toRepresentation().getName());
Set<GroupRepresentation> groupsInRole = role.getRoleGroupMembers();
assertTrue(groupsInRole.isEmpty());
}
@ -299,7 +306,7 @@ public class RealmRolesTest extends AbstractAdminTest {
* KEYCLOAK-2035 Verifies that Role Membership is ok after user removal
*/
@Test
public void roleMembershipAfterUserRemoval() {
public void roleMembershipAfterUserRemoval() {
RoleResource role = resource.get("role-with-users");
List<UserRepresentation> users = adminClient.realm(REALM_NAME).users().search("test-role-member", null, null, null, null, null);
@ -307,12 +314,12 @@ public class RealmRolesTest extends AbstractAdminTest {
UserResource user = adminClient.realm(REALM_NAME).users().get(users.get(0).getId());
UserRepresentation userRep = user.toRepresentation();
RoleResource roleResource = adminClient.realm(REALM_NAME).roles().get(role.toRepresentation().getName());
RoleResource roleResource = adminClient.realm(REALM_NAME).roles().get(role.toRepresentation().getName());
List<RoleRepresentation> rolesToAdd = new LinkedList<>();
rolesToAdd.add(roleResource.toRepresentation());
adminClient.realm(REALM_NAME).users().get(userRep.getId()).roles().realmLevel().add(rolesToAdd);
roleResource = adminClient.realm(REALM_NAME).roles().get(role.toRepresentation().getName());
roleResource = adminClient.realm(REALM_NAME).roles().get(role.toRepresentation().getName());
roleResource.getRoleUserMembers();
assertEquals(1, roleResource.getRoleUserMembers().size());
@ -362,80 +369,80 @@ public class RealmRolesTest extends AbstractAdminTest {
assertThat(expectedMembers, containsInAnyOrder("test-role-member", "test-role-member2"));
}
@Test
public void testSearchForRoles() {
for(int i = 0; i<15; i++) {
String roleName = "testrole"+i;
RoleRepresentation role = makeRole(roleName);
resource.create(role);
assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.roleResourcePath(roleName), role, ResourceType.REALM_ROLE);
}
assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.roleResourcePath(roleName), role, ResourceType.REALM_ROLE);
}
String roleNameA = "abcdefg";
RoleRepresentation roleA = makeRole(roleNameA);
resource.create(roleA);
assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.roleResourcePath(roleNameA), roleA, ResourceType.REALM_ROLE);
assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.roleResourcePath(roleNameA), roleA, ResourceType.REALM_ROLE);
String roleNameB = "defghij";
RoleRepresentation roleB = makeRole(roleNameB);
resource.create(roleB);
assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.roleResourcePath(roleNameB), roleB, ResourceType.REALM_ROLE);
assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.roleResourcePath(roleNameB), roleB, ResourceType.REALM_ROLE);
List<RoleRepresentation> resultSearch = resource.list("defg", -1, -1);
assertEquals(2,resultSearch.size());
List<RoleRepresentation> resultSearch2 = resource.list("testrole", -1, -1);
assertEquals(15,resultSearch2.size());
List<RoleRepresentation> resultSearchPagination = resource.list("testrole", 1, 5);
assertEquals(5,resultSearchPagination.size());
}
@Test
public void testPaginationRoles() {
for(int i = 0; i<15; i++) {
String roleName = "role"+i;
RoleRepresentation role = makeRole(roleName);
resource.create(role);
assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.roleResourcePath(roleName), role, ResourceType.REALM_ROLE);
}
assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.roleResourcePath(roleName), role, ResourceType.REALM_ROLE);
}
List<RoleRepresentation> resultSearchPagination = resource.list(1, 5);
assertEquals(5,resultSearchPagination.size());
List<RoleRepresentation> resultSearchPagination2 = resource.list(5, 5);
assertEquals(5,resultSearchPagination2.size());
List<RoleRepresentation> resultSearchPagination3 = resource.list(1, 5);
assertEquals(5,resultSearchPagination3.size());
List<RoleRepresentation> resultSearchPaginationIncoherentParams = resource.list(1, null);
assertTrue(resultSearchPaginationIncoherentParams.size() > 15);
}
@Test
public void testPaginationRolesCache() {
for(int i = 0; i<5; i++) {
String roleName = "paginaterole"+i;
RoleRepresentation role = makeRole(roleName);
resource.create(role);
assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.roleResourcePath(roleName), role, ResourceType.REALM_ROLE);
}
List<RoleRepresentation> resultBeforeAddingRoleToTestCache = resource.list(1, 1000);
assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.roleResourcePath(roleName), role, ResourceType.REALM_ROLE);
}
List<RoleRepresentation> resultBeforeAddingRoleToTestCache = resource.list(1, 1000);
// after a first call which init the cache, we add a new role to see if the result change
RoleRepresentation role = makeRole("anewrole");
resource.create(role);
assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.roleResourcePath("anewrole"), role, ResourceType.REALM_ROLE);
List<RoleRepresentation> resultafterAddingRoleToTestCache = resource.list(1, 1000);
assertEquals(resultBeforeAddingRoleToTestCache.size()+1, resultafterAddingRoleToTestCache.size());
}
@ -444,23 +451,15 @@ public class RealmRolesTest extends AbstractAdminTest {
for(int i = 0; i<5; i++) {
String roleName = "attributesrole"+i;
RoleRepresentation role = makeRole(roleName);
Map<String, List<String>> attributes = new HashMap<String, List<String>>();
Map<String, List<String>> attributes = new HashMap<>();
attributes.put("attribute1", Arrays.asList("value1","value2"));
role.setAttributes(attributes);
resource.create(role);
assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.roleResourcePath(roleName), role, ResourceType.REALM_ROLE);
// we have to update the role to set the attributes because
// the add role endpoint only care about name and description
RoleResource roleToUpdate = resource.get(roleName);
role.setId(roleToUpdate.toRepresentation().getId());
roleToUpdate.update(role);
assertAdminEvents.assertEvent(realmId, OperationType.UPDATE, AdminEventPaths.roleResourcePath(roleName), role, ResourceType.REALM_ROLE);
assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.roleResourcePath(roleName), role, ResourceType.REALM_ROLE);
}
List<RoleRepresentation> roles = resource.list("attributesrole", false);
assertTrue(roles.get(0).getAttributes().containsKey("attribute1"));
}
@ -470,23 +469,15 @@ public class RealmRolesTest extends AbstractAdminTest {
for(int i = 0; i<5; i++) {
String roleName = "attributesrolebrief"+i;
RoleRepresentation role = makeRole(roleName);
Map<String, List<String>> attributes = new HashMap<String, List<String>>();
Map<String, List<String>> attributes = new HashMap<>();
attributes.put("attribute1", Arrays.asList("value1","value2"));
role.setAttributes(attributes);
resource.create(role);
assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.roleResourcePath(roleName), role, ResourceType.REALM_ROLE);
// we have to update the role to set the attributes because
// the add role endpoint only care about name and description
RoleResource roleToUpdate = resource.get(roleName);
role.setId(roleToUpdate.toRepresentation().getId());
roleToUpdate.update(role);
assertAdminEvents.assertEvent(realmId, OperationType.UPDATE, AdminEventPaths.roleResourcePath(roleName), role, ResourceType.REALM_ROLE);
}
List<RoleRepresentation> roles = resource.list("attributesrolebrief", true);
assertNull(roles.get(0).getAttributes());
}
@ -527,7 +518,7 @@ public class RealmRolesTest extends AbstractAdminTest {
userResource = adminClient.realm(REALM_NAME).users().get(user.getId());
assertThat(userResource.roles().clientLevel(clientUuid).listAll(), empty());
assertThat(convertRolesToNames(userResource.roles().clientLevel(clientUuid).listEffective()),
assertThat(convertRolesToNames(userResource.roles().clientLevel(clientUuid).listEffective()),
hasItem("role-c")
);
}