From 16364671d035b04b7d87e1cb37b182bd71858e28 Mon Sep 17 00:00:00 2001 From: Maverick Studer Date: Tue, 7 Jan 2025 14:23:03 +0100 Subject: [PATCH] RED-10669: Unassigned user can alter component values inside file viewer --- .../controller/FileManagementController.java | 18 +----------------- .../impl/controller/ComponentControllerV2.java | 6 ++++++ .../service/AccessControlService.java | 18 ++++++++++++++++++ .../tests/ComponentControllerV2Test.java | 3 +++ .../tests/ComponentOverrideTest.java | 17 +++++++++++++---- 5 files changed, 41 insertions(+), 21 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/FileManagementController.java b/persistence-service-v1/persistence-service-external-api-impl-v1/src/main/java/com/iqser/red/persistence/service/v1/external/api/impl/controller/FileManagementController.java index b1123932e..3e4579052 100644 --- a/persistence-service-v1/persistence-service-external-api-impl-v1/src/main/java/com/iqser/red/persistence/service/v1/external/api/impl/controller/FileManagementController.java +++ b/persistence-service-v1/persistence-service-external-api-impl-v1/src/main/java/com/iqser/red/persistence/service/v1/external/api/impl/controller/FileManagementController.java @@ -232,7 +232,7 @@ public class FileManagementController implements FileManagementResource { public void restoreFiles(@PathVariable(DOSSIER_ID) String dossierId, @RequestBody Set fileIds) { accessControlService.checkAccessPermissionsToDossier(dossierId); - verifyUserIsDossierOwnerOrApproverOrAssignedReviewer(dossierId, fileIds); + accessControlService.verifyUserIsDossierOwnerOrApproverOrAssignedReviewer(dossierId, fileIds); fileService.undeleteFiles(dossierId, fileIds); auditPersistenceService.audit(AuditRequest.builder() .userId(KeycloakSecurity.getUserId()) @@ -289,20 +289,4 @@ public class FileManagementController implements FileManagementResource { } } - - private void verifyUserIsDossierOwnerOrApproverOrAssignedReviewer(String dossierId, Set fileIds) { - - try { - accessControlService.verifyUserIsDossierOwnerOrApprover(dossierId); - } catch (AccessDeniedException e1) { - try { - for (String fileId : fileIds) { - accessControlService.verifyUserIsReviewer(dossierId, fileId); - } - } catch (NotAllowedException e2) { - throw new NotAllowedException("User must be dossier owner, approver or assigned reviewer of the file."); - } - } - } - } diff --git a/persistence-service-v1/persistence-service-external-api-impl-v2/src/main/java/com/iqser/red/persistence/service/v2/external/api/impl/controller/ComponentControllerV2.java b/persistence-service-v1/persistence-service-external-api-impl-v2/src/main/java/com/iqser/red/persistence/service/v2/external/api/impl/controller/ComponentControllerV2.java index f16507e2e..6cd764d62 100644 --- a/persistence-service-v1/persistence-service-external-api-impl-v2/src/main/java/com/iqser/red/persistence/service/v2/external/api/impl/controller/ComponentControllerV2.java +++ b/persistence-service-v1/persistence-service-external-api-impl-v2/src/main/java/com/iqser/red/persistence/service/v2/external/api/impl/controller/ComponentControllerV2.java @@ -7,9 +7,11 @@ import static com.iqser.red.service.persistence.service.v2.api.external.resource import java.util.ArrayList; import java.util.Optional; +import java.util.Set; import java.util.stream.Collectors; import org.springframework.beans.factory.annotation.Value; +import org.springframework.security.access.AccessDeniedException; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RequestBody; @@ -22,6 +24,7 @@ import com.iqser.red.persistence.service.v2.external.api.impl.mapper.ComponentMa import com.iqser.red.service.persistence.management.v1.processor.exception.BadRequestException; import com.iqser.red.service.persistence.management.v1.processor.exception.NotAllowedException; import com.iqser.red.service.persistence.management.v1.processor.roles.ApplicationRoles; +import com.iqser.red.service.persistence.management.v1.processor.service.AccessControlService; import com.iqser.red.service.persistence.management.v1.processor.service.ComponentLogService; import com.iqser.red.service.persistence.management.v1.processor.service.CurrentApplicationTypeProvider; import com.iqser.red.service.persistence.management.v1.processor.service.FileStatusService; @@ -51,6 +54,7 @@ import lombok.experimental.FieldDefaults; @Tag(name = "4. Component endpoints", description = "Provides operations related to components") public class ComponentControllerV2 implements ComponentResource { + private final AccessControlService accessControlService; private final ComponentLogService componentLogService; private final UserService userService; private final StatusController statusController; @@ -140,6 +144,7 @@ public class ComponentControllerV2 implements ComponentResource { @RequestBody Component override) { checkApplicationType(); + accessControlService.verifyUserIsReviewer(dossierId, fileId); dossierTemplatePersistenceService.checkDossierTemplateExistsOrElseThrow404(dossierTemplateId); componentLogService.overrideComponent(dossierId, fileId, componentMapper.toComponentLogEntry(override)); @@ -180,6 +185,7 @@ public class ComponentControllerV2 implements ComponentResource { @RequestBody RevertOverrideRequest revertOverrideRequest) { checkApplicationType(); + accessControlService.verifyUserIsReviewer(dossierId, fileId); dossierTemplatePersistenceService.checkDossierTemplateExistsOrElseThrow404(dossierTemplateId); componentLogService.revertOverrides(dossierId, fileId, revertOverrideRequest); diff --git a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/AccessControlService.java b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/AccessControlService.java index 36def5563..8adaf9b87 100644 --- a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/AccessControlService.java +++ b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/AccessControlService.java @@ -1,6 +1,9 @@ package com.iqser.red.service.persistence.management.v1.processor.service; +import java.util.Set; + import org.springframework.http.HttpStatus; +import org.springframework.security.access.AccessDeniedException; import org.springframework.security.access.prepost.PostAuthorize; import org.springframework.security.acls.AclPermissionEvaluator; import org.springframework.security.core.context.SecurityContextHolder; @@ -203,4 +206,19 @@ public class AccessControlService { } } + public void verifyUserIsDossierOwnerOrApproverOrAssignedReviewer(String dossierId, Set fileIds) { + + try { + verifyUserIsDossierOwnerOrApprover(dossierId); + } catch (AccessDeniedException e1) { + try { + for (String fileId : fileIds) { + verifyUserIsReviewer(dossierId, fileId); + } + } catch (NotAllowedException e2) { + throw new NotAllowedException("User must be dossier owner, approver or assigned reviewer of the file."); + } + } + } + } diff --git a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/ComponentControllerV2Test.java b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/ComponentControllerV2Test.java index bc7010fa1..9e4b83ab5 100644 --- a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/ComponentControllerV2Test.java +++ b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/ComponentControllerV2Test.java @@ -19,6 +19,7 @@ import org.mockito.MockedStatic; import com.iqser.red.persistence.service.v1.external.api.impl.controller.StatusController; import com.iqser.red.persistence.service.v2.external.api.impl.controller.ComponentControllerV2; import com.iqser.red.service.persistence.management.v1.processor.exception.NotAllowedException; +import com.iqser.red.service.persistence.management.v1.processor.service.AccessControlService; import com.iqser.red.service.persistence.management.v1.processor.service.ComponentLogService; import com.iqser.red.service.persistence.management.v1.processor.service.CurrentApplicationTypeProvider; import com.iqser.red.service.persistence.management.v1.processor.service.FileStatusService; @@ -59,6 +60,8 @@ public class ComponentControllerV2Test { private ComponentLog mockComponentLog; @Mock private CurrentApplicationTypeProvider currentApplicationTypeProvider; + @Mock + private AccessControlService accessControlService; @BeforeEach diff --git a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/ComponentOverrideTest.java b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/ComponentOverrideTest.java index b07a269b2..18ceab6e9 100644 --- a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/ComponentOverrideTest.java +++ b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/ComponentOverrideTest.java @@ -9,17 +9,17 @@ import java.io.IOException; import java.util.List; import java.util.Set; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.core.io.ClassPathResource; import com.fasterxml.jackson.databind.ObjectMapper; -import com.iqser.red.persistence.service.v2.external.api.impl.controller.ComponentControllerV2; import com.iqser.red.service.peristence.v1.server.integration.client.ComponentClient; import com.iqser.red.service.peristence.v1.server.integration.client.DossierTemplateClient; +import com.iqser.red.service.peristence.v1.server.integration.client.FileClient; import com.iqser.red.service.peristence.v1.server.integration.service.DossierTesterAndProvider; import com.iqser.red.service.peristence.v1.server.integration.service.FileTesterAndProvider; +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.service.FileService; import com.iqser.red.service.persistence.service.v1.api.shared.model.analysislog.componentlog.ComponentLog; @@ -27,6 +27,7 @@ import com.iqser.red.service.persistence.service.v1.api.shared.model.component.R import com.iqser.red.service.persistence.service.v2.api.external.model.Component; import com.iqser.red.service.persistence.service.v2.api.external.model.ComponentValue; import com.iqser.red.service.persistence.service.v2.api.external.model.EntityReference; +import com.knecon.fforesight.keycloakcommons.security.KeycloakSecurity; import com.knecon.fforesight.tenantcommons.TenantApplicationType; import feign.FeignException; @@ -53,7 +54,10 @@ public class ComponentOverrideTest extends AbstractPersistenceServerServiceTest private FileService fileService; @Autowired - private ComponentControllerV2 componentControllerV2; + private FileClient fileClient; + + @Autowired + private UserProvider userProvider; @Override @@ -63,7 +67,6 @@ public class ComponentOverrideTest extends AbstractPersistenceServerServiceTest } - @Test public void testOverrides() throws IOException { @@ -73,6 +76,8 @@ public class ComponentOverrideTest extends AbstractPersistenceServerServiceTest var file = fileTesterAndProvider.testAndProvideFile(dossier, "filename1"); + fileClient.setStatusUnderReview(dossier.getId(), file.getId(), userProvider.getUserId()); + System.out.println("DOSSIER TEMPLATE ID: " + dossierTemplate.getId()); System.out.println("DOSSIER ID: " + dossier.getId()); System.out.println("FILE ID: " + file.getId()); @@ -194,6 +199,8 @@ public class ComponentOverrideTest extends AbstractPersistenceServerServiceTest var file = fileTesterAndProvider.testAndProvideFile(dossier, "filename2"); + fileClient.setStatusUnderReview(dossier.getId(), file.getId(), userProvider.getUserId()); + System.out.println("DOSSIER TEMPLATE ID: " + dossierTemplate.getId()); System.out.println("DOSSIER ID: " + dossier.getId()); System.out.println("FILE ID: " + file.getId()); @@ -256,6 +263,8 @@ public class ComponentOverrideTest extends AbstractPersistenceServerServiceTest // case 2: re-upload file that was deleted before overrides should also not be returned anymore fileTesterAndProvider.testAndProvideFile(dossier, "filename2"); + fileClient.setStatusUnderReview(dossier.getId(), file.getId(), userProvider.getUserId()); + var e = assertThrows(FeignException.class, () -> componentClient.getOverrides(dossierTemplate.getId(), dossier.getId(), file.getId())); assertEquals(e.status(), 404); fileManagementStorageService.saveComponentLog(dossier.getId(), file.getId(), componentLog);