From d9b589d8eb874f0409e039602f31454dbf963522 Mon Sep 17 00:00:00 2001 From: devplant Date: Thu, 27 Apr 2023 13:33:24 +0300 Subject: [PATCH] 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 --- .../api/impl/controller/StatusController.java | 45 +++++++++------- .../integration/service/UserProvider.java | 10 ++++ .../v1/server/integration/tests/FileTest.java | 54 +++++++++++++++++-- 3 files changed, 85 insertions(+), 24 deletions(-) diff --git a/persistence-service-v1/persistence-service-external-api-impl-v1/src/main/java/com/iqser/red/persistence/service/v1/external/api/impl/controller/StatusController.java b/persistence-service-v1/persistence-service-external-api-impl-v1/src/main/java/com/iqser/red/persistence/service/v1/external/api/impl/controller/StatusController.java index e8149cbbb..93648210a 100644 --- a/persistence-service-v1/persistence-service-external-api-impl-v1/src/main/java/com/iqser/red/persistence/service/v1/external/api/impl/controller/StatusController.java +++ b/persistence-service-v1/persistence-service-external-api-impl-v1/src/main/java/com/iqser/red/persistence/service/v1/external/api/impl/controller/StatusController.java @@ -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); } diff --git a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/service/UserProvider.java b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/service/UserProvider.java index 0420e2d86..f045e0649 100644 --- a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/service/UserProvider.java +++ b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/service/UserProvider.java @@ -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"); + } } diff --git a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/FileTest.java b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/FileTest.java index a463bd8e2..a9c57a057 100644 --- a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/FileTest.java +++ b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/FileTest.java @@ -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(); + } }