Marmelator/tenant-user-management-service
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 202 Wiki Activity

Merge branch 'RED-10169-fp2' into 'main'

Browse Source 
RED-10619: Multiple bugs when creating or editing users

See merge request fforesight/tenant-user-management-service!149
This commit is contained in:
Maverick Studer 2024-12-10 15:05:13 +01:00
commit 100b1c4cc1
2 changed files with 172 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

59
src/main/java/com/knecon/fforesight/tenantusermanagement/service/UserService.java
View File

@ -129,7 +129,7 @@ public class UserService {
return getUserByUsername(username);
}
}
public void checkRankOrderForAssigningRole(Set<String> newRoles, Set<String> currentUserRoles) {
@ -239,33 +239,67 @@ public class UserService {
public void validateSufficientRoles(String userId, Set<String> userRoles, Set<String> newRoles, Set<String> currentUserRoles) {
var roleMapping = tenantApplicationTypeService.getCurrentProperties().getKcRoleMapping();
var maxRank = currentUserRoles.stream()
int maxCurrentUserRank = currentUserRoles.stream()
.map(r -> roleMapping.getRole(r).getRank())
.max(Integer::compare)
.orElse(-1);
var newRolesRank = newRoles.stream()
.map(r -> roleMapping.getRole(r).getRank())
.toList();
var maxNewRolesRank = newRolesRank.stream()
.max(Integer::compare)
.orElse(-1);
var untouchableRoles = userRoles.stream()
Set<String> untouchableRoles = userRoles.stream()
.filter(roleMapping::isValidRole)
.map(roleMapping::getRole)
.filter(r -> r.getRank() > maxRank || ApplicationRoles.isKneconRole(r.getName()))
.filter(r -> r.getRank() > maxCurrentUserRank && !ApplicationRoles.isKneconRole(r.getName()))
.map(KCRole::getName)
.collect(Collectors.toSet());
if (maxNewRolesRank > maxRank) {
Set<String> kneconRoles = userRoles.stream()
.filter(roleMapping::isValidRole)
.map(roleMapping::getRole)
.map(KCRole::getName)
.filter(ApplicationRoles::isKneconRole)
.collect(Collectors.toSet());
int maxNewRolesRank = newRoles.stream()
.map(r -> roleMapping.getRole(r).getRank())
.max(Integer::compare)
.orElse(-1);
newRoles.addAll(kneconRoles);
int maxNewRolesRankIncludingKnecon = newRoles.stream()
.map(r -> roleMapping.getRole(r).getRank())
.max(Integer::compare)
.orElse(-1);
ensureNoHigherRankAssigned(maxCurrentUserRank, maxNewRolesRank);
ensureUntouchableRolesPreserved(untouchableRoles, newRoles);
ensureHighestRankNotRemovedFromSelf(userId, maxCurrentUserRank, maxNewRolesRankIncludingKnecon, roleMapping.getMaxRank());
}
private void ensureNoHigherRankAssigned(int maxCurrentUserRank, int maxNewRolesRank) {
if (maxNewRolesRank > maxCurrentUserRank) {
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Cannot assign this role to that user. Insufficient rights");
}
}
private void ensureUntouchableRolesPreserved(Set<String> untouchableRoles, Set<String> newRoles) {
if (!newRoles.containsAll(untouchableRoles)) {
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Cannot modify some roles for this user. Insufficient rights");
}
}
if (userId.equalsIgnoreCase(KeycloakSecurity.getUserId()) && maxRank.equals(roleMapping.getMaxRank()) && !maxNewRolesRank.equals(maxRank)) {
private void ensureHighestRankNotRemovedFromSelf(String userId, int maxCurrentUserRank, int maxNewRolesRankIncludingKnecon, int overallMaxRank) {
boolean isSelf = userId.equalsIgnoreCase(KeycloakSecurity.getUserId());
boolean isUserHighestRank = maxCurrentUserRank == overallMaxRank;
boolean highestRankRemoved = !Integer.valueOf(maxNewRolesRankIncludingKnecon).equals(maxCurrentUserRank);
if (isSelf && isUserHighestRank && highestRankRemoved) {
throw new ResponseStatusException(HttpStatus.CONFLICT, "Cannot remove highest ranking role from self.");
}
}
@ -655,6 +689,7 @@ public class UserService {
.max(Integer::compare)
.orElse(-1);
var targetRank = userRoles.stream()
.filter(ApplicationRoles::isNoKneconRole)
.map(r -> roleMapping.getRole(r).getRank())
.max(Integer::compare)
.orElse(-1);

130
src/test/java/com/knecon/fforesight/tenantusermanagement/tests/UserTest.java
View File

@ -30,8 +30,10 @@ import com.knecon.fforesight.tenantusermanagement.permissions.ApplicationRoles;
import com.knecon.fforesight.tenantusermanagement.properties.TenantUserManagementProperties;
import com.knecon.fforesight.tenantusermanagement.service.RealmService;
import com.knecon.fforesight.tenantusermanagement.service.TenantApplicationTypeService;
import com.knecon.fforesight.tenantusermanagement.service.UserService;
import feign.FeignException;
import lombok.NonNull;
import lombok.SneakyThrows;
public class UserTest extends AbstractTenantUserManagementIntegrationTest {
@ -48,6 +50,9 @@ public class UserTest extends AbstractTenantUserManagementIntegrationTest {
@Autowired
private TenantApplicationTypeService tenantApplicationTypeService;
@Autowired
private UserService userService;
@Test
public void testUsers() {
@ -505,11 +510,11 @@ public class UserTest extends AbstractTenantUserManagementIntegrationTest {
tokenService.setUser("admin@knecon.com", "secret");
// this should still not be possible
e = assertThrows(FeignException.class, () -> userClient.setRoles(onlyKneconUser.getUserId(), allRoles));
e = assertThrows(FeignException.class, () -> userClient.setRoles(onlyKneconUser.getUserId(), allRoles));
assertEquals(404, e.status());
// and also not this
e = assertThrows(FeignException.class, () -> userClient.setRoles(user.getUserId(), allRoles));
e = assertThrows(FeignException.class, () -> userClient.setRoles(user.getUserId(), allRoles));
assertEquals(400, e.status());
// we can also poll the user
@ -588,12 +593,16 @@ public class UserTest extends AbstractTenantUserManagementIntegrationTest {
addRoles(user4.getUserId(), allButKneconRoles);
allUsers = userClient.getAllUsers(true);
var user4AfterShenanigansOpt = allUsers.stream().filter(u -> u.getUserId().equals(user4.getUserId())).findFirst();
var user4AfterShenanigansOpt = allUsers.stream()
.filter(u -> u.getUserId().equals(user4.getUserId()))
.findFirst();
assertTrue(user4AfterShenanigansOpt.isPresent());
user4AfterShenanigansOpt.get().setRoles(new HashSet<>());
assertEquals(user4AfterShenanigansOpt.get(), user4);
var stillOnlyKneconUserOpt = allUsers.stream().filter(u -> u.getUserId().equals(onlyKneconUser.getUserId())).findFirst();
var stillOnlyKneconUserOpt = allUsers.stream()
.filter(u -> u.getUserId().equals(onlyKneconUser.getUserId()))
.findFirst();
assertTrue(stillOnlyKneconUserOpt.isPresent());
stillOnlyKneconUserOpt.get().setRoles(new HashSet<>());
assertEquals(stillOnlyKneconUserOpt.get(), onlyKneconUser);
@ -708,6 +717,113 @@ public class UserTest extends AbstractTenantUserManagementIntegrationTest {
}
@Test
public void testUpdateProfileForUserWithAllRoles() {
TenantContext.setTenantId(AbstractTenantUserManagementIntegrationTest.TEST_TENANT_ID);
tokenService.setUser("admin@knecon.com", "secret");
var allRoles = tenantApplicationTypeService.getCurrentProperties().getKcRoleMapping().getAllRoles();
Set<String> allButKneconRoles = allRoles.stream()
.filter(ApplicationRoles::isNoKneconRole)
.collect(Collectors.toSet());
CreateUserRequest createUserRequest = new CreateUserRequest();
createUserRequest.setEmail("all.roles.user@knecon.com");
createUserRequest.setUsername("all.roles.user@knecon.com");
createUserRequest.setFirstName("All");
createUserRequest.setLastName("Roles");
var allRolesuser = userClient.createUser(createUserRequest);
addRoles(allRolesuser.getUserId(), allRoles);
assertThat(allRolesuser).isNotNull();
UpdateProfileRequest updateProfileRequest = UpdateProfileRequest.builder()
.email("all.roles.user@knecon.com")
.firstName("All")
.lastName("NewLastName")
.roles(allButKneconRoles)
.build();
var updatedUser = userClient.updateProfile(allRolesuser.getUserId(), updateProfileRequest);
assertThat(updatedUser).isNotNull();
assertThat(updatedUser.getLastName()).isEqualTo("NewLastName");
tokenService.setUser("test@fforesight.com", "secret");
updateProfileRequest.setLastName("AnotherNewLastName");
updatedUser = userClient.updateProfile(allRolesuser.getUserId(), updateProfileRequest);
assertThat(updatedUser).isNotNull();
assertThat(updatedUser.getLastName()).isEqualTo("AnotherNewLastName");
createUserRequest.setEmail("less.super.user.1@knecon.com");
createUserRequest.setUsername(createUserRequest.getEmail());
createUserRequest.setRoles(Set.of("LESS_SUPER_USER"));
var lessSuperUser = userClient.createUser(createUserRequest);
userClient.resetPassword(lessSuperUser.getUserId(), ResetPasswordRequest.builder().password("Secret@secured!23").build());
tokenService.setUser("less.super.user.1@knecon.com", "Secret@secured!23");
FeignException feignException = assertThrows(FeignException.class, () -> userClient.updateProfile(allRolesuser.getUserId(), updateProfileRequest));
assertEquals(400, feignException.status());
assertTrue(feignException.getMessage().contains("Cannot assign this role to that user. Insufficient rights"));
tokenService.setUser("admin@knecon.com", "secret");
userClient.deleteUser(lessSuperUser.getUserId());
userClient.deleteUser(allRolesuser.getUserId());
}
@Test
public void testDeleteKneconRolesUserAsNormalAdmin() {
TenantContext.setTenantId(AbstractTenantUserManagementIntegrationTest.TEST_TENANT_ID);
tokenService.setUser("admin@knecon.com", "secret");
var allRoles = tenantApplicationTypeService.getCurrentProperties().getKcRoleMapping().getAllRoles();
Set<String> allButKneconRoles = allRoles.stream()
.filter(ApplicationRoles::isNoKneconRole)
.collect(Collectors.toSet());
CreateUserRequest createUserRequest = new CreateUserRequest();
createUserRequest.setEmail("normalAdmin@knecon.com");
createUserRequest.setUsername("normalAdmin@knecon.com");
createUserRequest.setFirstName("Mister");
createUserRequest.setLastName("Admin");
var adminUser = userClient.createUser(createUserRequest);
addRoles(adminUser.getUserId(), allButKneconRoles);
assertThat(adminUser).isNotNull();
createUserRequest = new CreateUserRequest();
createUserRequest.setEmail("kneconAdmin@knecon.com");
createUserRequest.setUsername("kneconAdmin@knecon.com");
createUserRequest.setFirstName("Knecon");
createUserRequest.setLastName("Admin");
var kneconAdminuser = userClient.createUser(createUserRequest);
addRoles(kneconAdminuser.getUserId(), allRoles);
assertThat(kneconAdminuser).isNotNull();
userClient.resetPassword(adminUser.getUserId(), ResetPasswordRequest.builder().password("Secret@secured!23").build());
tokenService.setUser("normalAdmin@knecon.com", "Secret@secured!23");
userClient.deleteUser(kneconAdminuser.getUserId());
List<User> allUsers = userClient.getAllUsers(true);
assertTrue(allUsers.stream()
.noneMatch(u -> u.getUserId().equals(kneconAdminuser.getUserId())));
List<User> unfilteredUsers = userService.getAllUsers();
assertTrue(unfilteredUsers.stream()
.anyMatch(u -> u.getUserId().equals(kneconAdminuser.getUserId())));
tokenService.setUser("admin@knecon.com", "secret");
userClient.deleteUser(adminUser.getUserId());
}
private UsersResource getTenantUsersResource() {
return realmService.realm(TenantContext.getTenantId()).users();
@ -730,7 +846,11 @@ public class UserTest extends AbstractTenantUserManagementIntegrationTest {
private void addRoles(String userId, Set<String> roles) {
getUserResource(userId).roles().realmLevel().add(roles.stream().map(this::getRoleRepresentation).toList());
getUserResource(userId).roles()
.realmLevel()
.add(roles.stream()
.map(this::getRoleRepresentation)
.toList());
}
}