RED-6034 - Possible to assign a file to unauthorized users

- validate the assignee before triggering the under review state for the given file
- update junit tests
This commit is contained in:
devplant 2023-04-27 13:33:24 +03:00
parent 00d6230783
commit d9b589d8eb
3 changed files with 85 additions and 24 deletions

View File

@ -159,24 +159,7 @@ public class StatusController implements StatusResource {
log.debug("Requested [setFileReviewer] for dossier: {} / file: {} / reviewer: {}", dossierId, fileId, assigneeId);
if (assigneeId != null) {
var user = userService.getUserById(assigneeId);
if (user.isEmpty()) {
userService.removeDeletedUsers(Collections.singleton(assigneeId));
throw new BadRequestException("Unknown user=" + assigneeId);
}
// check he has a manager role, thus he can be the owner
if (user.get().getRoles().stream().noneMatch(VALID_MEMBER_ROLES::contains)) {
throw new BadRequestException("Make sure each provided member id has the permission to be a member of a dossier.");
}
// check assignee is a member
accessControlService.verifyUserIsMemberOrApprover(dossierId, assigneeId);
} else {
// check if file is already unassigned (cannot unassign an unassigned file)
if (fileStatusManagementService.getFileStatus(fileId).getAssignee() == null) {
throw new BadRequestException("File is already unassigned!");
}
}
validateAssignee(dossierId, fileId, assigneeId);
var fileStatus = fileStatusManagementService.getFileStatus(fileId);
@ -222,6 +205,30 @@ public class StatusController implements StatusResource {
}
private void validateAssignee(String dossierId, String fileId, String assigneeId) {
if (assigneeId != null) {
var user = userService.getUserById(assigneeId);
if (user.isEmpty()) {
userService.removeDeletedUsers(Collections.singleton(assigneeId));
throw new BadRequestException("Unknown user=" + assigneeId);
}
// check he has a manager role, thus he can be the owner
if (user.get().getRoles().stream().noneMatch(VALID_MEMBER_ROLES::contains)) {
throw new BadRequestException("Make sure each provided member id has the permission to be a member of a dossier.");
}
// check assignee is a member
accessControlService.verifyUserIsMemberOrApprover(dossierId, assigneeId);
} else {
// check if file is already unassigned (cannot unassign an unassigned file)
if (fileStatusManagementService.getFileStatus(fileId).getAssignee() == null) {
throw new BadRequestException("File is already unassigned!");
}
}
}
@PreAuthorize("hasAuthority('" + SET_REVIEWER + "')")
public void setStatusUnderReview(@PathVariable(DOSSIER_ID) String dossierId,
@PathVariable(FILE_ID) String fileId,
@ -330,7 +337,7 @@ public class StatusController implements StatusResource {
private void setStatusUnderReviewForFile(String dossierId, String fileId, String assigneeId) {
accessControlService.verifyUserIsMemberOrApprover(dossierId);
validateAssignee(dossierId, fileId, assigneeId);
fileStatusManagementService.setStatusUnderReview(dossierId, fileId, assigneeId);
}

View File

@ -28,4 +28,14 @@ public class UserProvider {
}
throw new RuntimeException("user not created");
}
public String getMemberUserId(){
var allUsers = userService.getAllUsers();
var managerAdmin = allUsers.stream().filter(u -> u.getUsername().contains("user")).findFirst();
if(managerAdmin.isPresent()){
return managerAdmin.get().getUserId();
}
throw new RuntimeException("user not created");
}
}

View File

@ -9,6 +9,7 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.springframework.beans.BeanUtils;
import org.springframework.beans.factory.annotation.Autowired;
@ -31,6 +32,7 @@ import com.iqser.red.service.peristence.v1.server.integration.service.FileTester
import com.iqser.red.service.peristence.v1.server.integration.service.TypeProvider;
import com.iqser.red.service.peristence.v1.server.integration.service.UserProvider;
import com.iqser.red.service.peristence.v1.server.integration.utils.AbstractPersistenceServerServiceTest;
import com.iqser.red.service.persistence.management.v1.processor.exception.BadRequestException;
import com.iqser.red.service.persistence.service.v1.api.shared.model.FileAttributes;
import com.iqser.red.service.persistence.service.v1.api.shared.model.FileAttributesConfig;
import com.iqser.red.service.persistence.service.v1.api.shared.model.PageExclusionRequest;
@ -47,6 +49,8 @@ import com.iqser.red.service.persistence.service.v1.api.shared.model.manual.Imag
import com.iqser.red.service.persistence.service.v1.api.shared.model.manual.LegalBasisChangeRequest;
import com.iqser.red.service.persistence.service.v1.api.shared.model.manual.RemoveRedactionRequest;
import feign.FeignException;
public class FileTest extends AbstractPersistenceServerServiceTest {
@Autowired
@ -480,11 +484,6 @@ public class FileTest extends AbstractPersistenceServerServiceTest {
var userId = userProvider.getUserId();
var altUserId = userProvider.getAltUserId();
// update dossier - add new member
CreateOrUpdateDossierRequest cru = new CreateOrUpdateDossierRequest();
BeanUtils.copyProperties(dossier, cru);
cru.getMemberIds().add(altUserId);
var loadedFile = fileClient.getFileStatus(dossier.getId(), file.getId());
assertThat(loadedFile.getAssignee()).isNull();
assertThat(loadedFile.getWorkflowStatus()).isEqualTo(WorkflowStatus.NEW);
@ -521,4 +520,49 @@ public class FileTest extends AbstractPersistenceServerServiceTest {
}
@Test
public void testFile6034SetUnderReview() {
var start = OffsetDateTime.now();
var dossier = dossierTesterAndProvider.provideTestDossier();
var file = fileTesterAndProvider.testAndProvideFile(dossier);
var userId = userProvider.getUserId();
var altUserId = userProvider.getAltUserId();
var user2 = userProvider.getMemberUserId();
assertThat(dossier.getMemberIds().contains(user2)).isFalse();
var loadedFile = fileClient.getFileStatus(dossier.getId(), file.getId());
assertThat(loadedFile.getAssignee()).isNull();
assertThat(loadedFile.getWorkflowStatus()).isEqualTo(WorkflowStatus.NEW);
assertThat(loadedFile.getLastReviewer()).isNull();
assertThat(loadedFile.getLastApprover()).isNull();
Exception exception = Assertions.assertThrows(FeignException.BadRequest.class, () -> {
fileClient.setStatusUnderReview(dossier.getId(), file.getId(), null);
});
String expectedMessage = "File is already unassigned!";
String actualMessage = exception.getMessage();
assertThat(actualMessage).contains(expectedMessage);
exception = Assertions.assertThrows(FeignException.Forbidden.class, () -> {
fileClient.setStatusUnderReview(dossier.getId(), file.getId(), user2);
});
expectedMessage = "User must be dossier member";
actualMessage = exception.getMessage();
assertThat(actualMessage).contains(expectedMessage);
fileClient.setStatusUnderReview(dossier.getId(), file.getId(), altUserId);
loadedFile = fileClient.getFileStatus(dossier.getId(), file.getId());
assertThat(loadedFile.getWorkflowStatus()).isEqualTo(WorkflowStatus.UNDER_REVIEW);
assertThat(loadedFile.getAssignee()).isEqualTo(altUserId);
assertThat(loadedFile.getLastReviewer()).isNull();
assertThat(loadedFile.getLastApprover()).isNull();
}
}