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

RED-8491: Hide all KNECON_* roles for any possible access in all endpoints && RED-9346: User without roles not displayed in GET endpoint

Browse Source
This commit is contained in:
Maverick Studer 2024-06-14 11:51:51 +02:00
parent 8ed4e8660f
commit c303d46d8e
3 changed files with 84 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/main/java/com/knecon/fforesight/tenantusermanagement/api/external/UserResource.java vendored
View File

@ -61,7 +61,7 @@ public interface UserResource {
@ResponseBody
@ResponseStatus(value = HttpStatus.OK)
@Operation(summary = "Update a user profile", description = "None")
@ApiResponses(value = {@ApiResponse(responseCode = "204", description = "OK"), @ApiResponse(responseCode = "400", description = "Failed to update profile, e-mail cannot be empty")})
@ApiResponses(value = {@ApiResponse(responseCode = "204", description = "OK"), @ApiResponse(responseCode = "400", description = "Failed to update profile, e-mail invalid")})
@PostMapping(value = UPDATE_USER_PROFILE_PATH + USER_ID_PATH_VARIABLE, consumes = MediaType.APPLICATION_JSON_VALUE, produces = MediaType.APPLICATION_JSON_VALUE)
User updateProfile(@PathVariable(USER_ID) String userId, @RequestBody UpdateProfileRequest updateProfileRequest);
@ -69,7 +69,7 @@ public interface UserResource {
@ResponseBody
@ResponseStatus(value = HttpStatus.OK)
@Operation(summary = "Update your user profile", description = "None")
@ApiResponses(value = {@ApiResponse(responseCode = "204", description = "OK"), @ApiResponse(responseCode = "400", description = "Failed to update profile, e-mail cannot be empty")})
@ApiResponses(value = {@ApiResponse(responseCode = "204", description = "OK"), @ApiResponse(responseCode = "400", description = "Failed to update profile, e-mail invalid")})
@PostMapping(value = UPDATE_MY_USER_PROFILE_PATH, consumes = MediaType.APPLICATION_JSON_VALUE, produces = MediaType.APPLICATION_JSON_VALUE)
User updateMyProfile(@RequestBody UpdateMyProfileRequest updateProfileRequest);
@ -92,7 +92,7 @@ public interface UserResource {
@ResponseBody
@Operation(summary = "Create a new user", description = "None")
@ApiResponses(value = {@ApiResponse(responseCode = "200", description = "OK"), @ApiResponse(responseCode = "400", description = "Invalid Data."), @ApiResponse(responseCode = "409", description = "User already exists.")})
@ApiResponses(value = {@ApiResponse(responseCode = "200", description = "OK"), @ApiResponse(responseCode = "400", description = "Invalid Data.")})
@PostMapping(value = USER_REST_PATH, consumes = MediaType.APPLICATION_JSON_VALUE, produces = MediaType.APPLICATION_JSON_VALUE)
User createUser(@RequestBody CreateUserRequest user);

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

@ -83,7 +83,7 @@ public class UserService {
}
if (!EmailValidator.getInstance().isValid(user.getEmail())) {
throw new ResponseStatusException(HttpStatus.CONFLICT, "Email address format is not valid");
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Email address format is not valid");
}
tenantUserManagementProperties.getKcRoleMapping().validateRoles(user.getRoles());
@ -101,7 +101,7 @@ public class UserService {
if (response.getStatusInfo().getFamily() != Response.Status.Family.SUCCESSFUL) {
if (response.getStatusInfo().getStatusCode() == 409) {
throw new ResponseStatusException(HttpStatus.CONFLICT, response.getStatusInfo().getReasonPhrase());
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, response.getStatusInfo().getReasonPhrase());
}
if (response.getStatusInfo().getStatusCode() == 400) {
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, response.getStatusInfo().getReasonPhrase());
@ -282,7 +282,7 @@ public class UserService {
user.update(userRepresentation);
} catch (ClientErrorException e) {
if (e.getResponse().getStatus() == 409) {
throw new ResponseStatusException(HttpStatus.CONFLICT, "E-mail already in use");
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "E-mail is not available");
}
throw e;
}
@ -454,7 +454,7 @@ public class UserService {
}
if (!EmailValidator.getInstance().isValid(updateProfileRequest.getEmail())) {
throw new ResponseStatusException(HttpStatus.CONFLICT, "Email address format is not valid");
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Email address format is not valid");
}
userRepresentation.setFirstName(updateProfileRequest.getFirstName());

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

@ -57,8 +57,10 @@ public class UserTest extends AbstractTenantUserManagementIntegrationTest {
assertThat(allUsers).isNotEmpty();
assertThat(testUserFound).isTrue();
var optionalUser = allUsers.stream().filter(user -> user.getUsername().equalsIgnoreCase("test@fforesight.com")).findFirst();
assert(optionalUser.isPresent());
var optionalUser = allUsers.stream()
.filter(user -> user.getUsername().equalsIgnoreCase("test@fforesight.com"))
.findFirst();
assert (optionalUser.isPresent());
var testUser = userClient.getUserById(optionalUser.get().getUserId());
assertThat(testUser).isNotNull();
@ -70,8 +72,8 @@ public class UserTest extends AbstractTenantUserManagementIntegrationTest {
assertThat(testUser.getLastName()).isEqualTo("updateTestLastName");
assertThat(testUser.getFirstName()).isEqualTo("updateTestFirstName");
Set<String> allButKneconRoles = tenantUserManagementProperties.getKcRoleMapping().getAllRoles().stream()
Set<String> allButKneconRoles = tenantUserManagementProperties.getKcRoleMapping().getAllRoles()
.stream()
.filter(ApplicationRoles::isNoKneconRole)
.collect(Collectors.toSet());
@ -102,8 +104,7 @@ public class UserTest extends AbstractTenantUserManagementIntegrationTest {
.lastName("update test")
.roles(allButKneconRoles)
.build());
assertThat(createdUser.getRoles()).containsExactly(allButKneconRoles
.toArray(new String[0]));
assertThat(createdUser.getRoles()).containsExactly(allButKneconRoles.toArray(new String[0]));
userClient.deleteUsers(List.of(createdUser.getUserId()));
@ -300,7 +301,8 @@ public class UserTest extends AbstractTenantUserManagementIntegrationTest {
public void testCreateUserWithExistingUser() {
TenantContext.setTenantId(AbstractTenantUserManagementIntegrationTest.TEST_TENANT_ID);
Set<String> allButKneconRoles = tenantUserManagementProperties.getKcRoleMapping().getAllRoles().stream()
Set<String> allButKneconRoles = tenantUserManagementProperties.getKcRoleMapping().getAllRoles()
.stream()
.filter(ApplicationRoles::isNoKneconRole)
.collect(Collectors.toSet());
@ -406,14 +408,19 @@ public class UserTest extends AbstractTenantUserManagementIntegrationTest {
var allUsers = userClient.getAllUsers(true);
// should not contain the knecon user but the others
assertTrue(allUsers.stream().anyMatch(u -> u.getUserId().equals(user.getUserId())));
assertTrue(allUsers.stream().anyMatch(u -> u.getUserId().equals(noKneconUser.getUserId())));
assertFalse(allUsers.stream().anyMatch(u -> u.getUserId().equals(onlyKneconUser.getUserId())));
assertTrue(allUsers.stream()
.anyMatch(u -> u.getUserId().equals(user.getUserId())));
assertTrue(allUsers.stream()
.anyMatch(u -> u.getUserId().equals(noKneconUser.getUserId())));
assertFalse(allUsers.stream()
.anyMatch(u -> u.getUserId().equals(onlyKneconUser.getUserId())));
var foundUser = allUsers.stream()
.filter(u -> u.getUserId().equals(user.getUserId()))
.findFirst();
assertTrue(foundUser.isPresent());
assertFalse(foundUser.get().getRoles().stream().anyMatch(ApplicationRoles::isKneconRole));
assertFalse(foundUser.get().getRoles()
.stream()
.anyMatch(ApplicationRoles::isKneconRole));
// should not be found
var e = assertThrows(FeignException.class, () -> userClient.getUserById(onlyKneconUser.getUserId()));
@ -421,22 +428,33 @@ public class UserTest extends AbstractTenantUserManagementIntegrationTest {
// should not show any knecon roles
User userById = userClient.getUserById(user.getUserId());
assertTrue(userById.getRoles().stream().noneMatch(ApplicationRoles::isKneconRole));
assertTrue(userById.getRoles()
.stream()
.noneMatch(ApplicationRoles::isKneconRole));
List<User> applicationSpecificUsers = userClient.getApplicationSpecificUsers(true);
// should not contain the knecon user but the others
assertTrue(applicationSpecificUsers.stream().anyMatch(u -> u.getUserId().equals(user.getUserId())));
assertTrue(applicationSpecificUsers.stream().anyMatch(u -> u.getUserId().equals(noKneconUser.getUserId())));
assertFalse(applicationSpecificUsers.stream().anyMatch(u -> u.getUserId().equals(onlyKneconUser.getUserId())));
assertTrue(applicationSpecificUsers.stream()
.anyMatch(u -> u.getUserId().equals(user.getUserId())));
assertTrue(applicationSpecificUsers.stream()
.anyMatch(u -> u.getUserId().equals(noKneconUser.getUserId())));
assertFalse(applicationSpecificUsers.stream()
.anyMatch(u -> u.getUserId().equals(onlyKneconUser.getUserId())));
foundUser = applicationSpecificUsers.stream()
.filter(u -> u.getUserId().equals(user.getUserId()))
.findFirst();
assertTrue(foundUser.isPresent());
assertFalse(foundUser.get().getRoles().stream().anyMatch(ApplicationRoles::isKneconRole));
assertFalse(foundUser.get().getRoles()
.stream()
.anyMatch(ApplicationRoles::isKneconRole));
// no knecon roles are visible
assertEquals(allUsers.stream().filter(u -> u.getRoles().stream().anyMatch(ApplicationRoles::isKneconRole)).count(), 0);
assertEquals(allUsers.stream()
.filter(u -> u.getRoles()
.stream()
.anyMatch(ApplicationRoles::isKneconRole))
.count(), 0);
// we can get this user because of other roles being present
userClient.getUserById(noKneconUser.getUserId());
@ -520,7 +538,8 @@ public class UserTest extends AbstractTenantUserManagementIntegrationTest {
userClient.deleteUsers(List.of(user4.getUserId()));
allUsers = userClient.getAllUsers(true);
assertTrue(allUsers.stream().anyMatch(u -> u.getUserId().equals(user4.getUserId())));
assertTrue(allUsers.stream()
.anyMatch(u -> u.getUserId().equals(user4.getUserId())));
// and should not have changed
var user5 = userClient.getUserById(user4.getUserId());
assertEquals(user4, user5);
@ -528,7 +547,6 @@ public class UserTest extends AbstractTenantUserManagementIntegrationTest {
}
@Test
public void testShowUserWithoutRoles() {
@ -559,11 +577,49 @@ public class UserTest extends AbstractTenantUserManagementIntegrationTest {
User userById = userClient.getUserById(noRolesUser.getUserId());
List<User> allUsers = userClient.getAllUsers(true);
assertTrue(allUsers.stream().anyMatch(u -> u.getUserId().equals(noRolesUser.getUserId())));
assertTrue(allUsers.stream()
.anyMatch(u -> u.getUserId().equals(noRolesUser.getUserId())));
List<User> applicationSpecificUsers = userClient.getApplicationSpecificUsers(true);
assertFalse(applicationSpecificUsers.stream().anyMatch(u -> u.getUserId().equals(noRolesUser.getUserId())));
assertFalse(applicationSpecificUsers.stream()
.anyMatch(u -> u.getUserId().equals(noRolesUser.getUserId())));
}
@Test
public void testCreateUserWithInvalidEmailFormat() {
// set context and user
TenantContext.setTenantId(AbstractTenantUserManagementIntegrationTest.TEST_TENANT_ID);
tokenService.setUser("admin@knecon.com", "secret");
// create several users with different roles for testing
var createUserRequest = new CreateUserRequest();
createUserRequest.setFirstName("My nice");
createUserRequest.setLastName("User");
createUserRequest.setUsername("MyNiceUser");
createUserRequest.setRoles(new HashSet<>());
createUserRequest.setEmail(null);
var e = assertThrows(FeignException.class, () -> userClient.createUser(createUserRequest));
assertEquals(400, e.status());
createUserRequest.setEmail("");
e = assertThrows(FeignException.class, () -> userClient.createUser(createUserRequest));
assertEquals(400, e.status());
createUserRequest.setEmail("myniceuser@notknecon");
e = assertThrows(FeignException.class, () -> userClient.createUser(createUserRequest));
assertEquals(400, e.status());
createUserRequest.setEmail("myniceuser.com");
e = assertThrows(FeignException.class, () -> userClient.createUser(createUserRequest));
assertEquals(400, e.status());
createUserRequest.setEmail("myniceuser@notknecon@at.com");
e = assertThrows(FeignException.class, () -> userClient.createUser(createUserRequest));
assertEquals(400, e.status());
}