RED-10669: Unassigned user can alter component values inside file viewer

This commit is contained in:
Maverick Studer 2025-01-07 14:23:03 +01:00
parent 8054806cd3
commit 16364671d0
5 changed files with 41 additions and 21 deletions

View File

@ -232,7 +232,7 @@ public class FileManagementController implements FileManagementResource {
public void restoreFiles(@PathVariable(DOSSIER_ID) String dossierId, @RequestBody Set<String> 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<String> 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.");
}
}
}
}

View File

@ -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);

View File

@ -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<String> 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.");
}
}
}
}

View File

@ -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

View File

@ -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);