Pull request #683: RED-6034 - Possible to assign a file to unauthorized users
Merge in RED/persistence-service from bugfix/RED-6034 to master * commit '4614ceb45bfa288e2dc50700807f12153270d883': RED-6034 - Possible to assign a file to unauthorized users - change back from knecon_release to iqser_release RED-6034 - Possible to assign a file to unauthorized users - update from iqser to knecon RED-6034 - Possible to assign a file to unauthorized users - rework user provider 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:
commit
13350e4702
@ -95,7 +95,7 @@ public class PlanSpec {
|
||||
.enabled(true),
|
||||
new InjectVariablesTask().description("Inject git Tag").path("git.tag").namespace("g").scope(InjectVariablesScope.LOCAL),
|
||||
new VcsTagTask().description("${bamboo.g.gitTag}").tagName("${bamboo.g.gitTag}").defaultRepository())
|
||||
.dockerConfiguration(new DockerConfiguration().image("nexus.iqser.com:5001/infra/maven:3.8.4-openjdk-17-slim")
|
||||
.dockerConfiguration(new DockerConfiguration().image("nexus.knecon.com:5001/infra/maven:3.8.4-openjdk-17-slim")
|
||||
.dockerRunArguments("--net=host")
|
||||
.volume("/etc/maven/settings.xml", "/usr/share/maven/ref/settings.xml")
|
||||
.volume("/var/run/docker.sock", "/var/run/docker.sock"))))
|
||||
@ -114,7 +114,7 @@ public class PlanSpec {
|
||||
.inlineBody("#!/bin/bash\n" + "set -e\n" + "rm -rf ./*"),
|
||||
new VcsCheckoutTask().description("Checkout Default Repository").cleanCheckout(true).checkoutItems(new CheckoutItem().defaultRepository()),
|
||||
new ScriptTask().description("Sonar").location(Location.FILE).fileFromPath("bamboo-specs/src/main/resources/scripts/sonar-java.sh").argument(SERVICE_NAME))
|
||||
.dockerConfiguration(new DockerConfiguration().image("nexus.iqser.com:5001/infra/maven:3.8.4-openjdk-17-slim")
|
||||
.dockerConfiguration(new DockerConfiguration().image("nexus.knecon.com:5001/infra/maven:3.8.4-openjdk-17-slim")
|
||||
.dockerRunArguments("--net=host")
|
||||
.volume("/etc/maven/settings.xml", "/usr/share/maven/conf/settings.xml")
|
||||
.volume("/var/run/docker.sock", "/var/run/docker.sock"))))
|
||||
|
||||
@ -48,7 +48,7 @@ mvn -f ${bamboo_build_working_directory}/$SERVICE_NAME-v1/pom.xml \
|
||||
-Dmaven.wagon.http.ssl.insecure=true \
|
||||
-Dmaven.wagon.http.ssl.allowall=true \
|
||||
-Dmaven.wagon.http.ssl.ignore.validity.dates=true \
|
||||
-DaltDeploymentRepository=iqser_release::default::https://nexus.iqser.com/repository/red-platform-releases
|
||||
-DaltDeploymentRepository=iqser_release::default::https://nexus.knecon.com/repository/red-platform-releases
|
||||
|
||||
mvn --no-transfer-progress \
|
||||
-f ${bamboo_build_working_directory}/$SERVICE_NAME-image-v1/pom.xml \
|
||||
|
||||
@ -22,7 +22,7 @@ then
|
||||
-f ${bamboo_build_working_directory}/$SERVICE_NAME-v1/pom.xml \
|
||||
sonar:sonar \
|
||||
-Dsonar.projectKey=RED_$SERVICE_NAME \
|
||||
-Dsonar.host.url=https://sonarqube.iqser.com \
|
||||
-Dsonar.host.url=https://sonarqube.knecon.com \
|
||||
-Dsonar.login=${bamboo_sonarqube_api_token_secret} \
|
||||
-Dsonar.branch.name=${bamboo_planRepository_1_branch} \
|
||||
-Dsonar.dependencyCheck.jsonReportPath=target/dependency-check-report.json \
|
||||
@ -34,7 +34,7 @@ else
|
||||
-f ${bamboo_build_working_directory}/$SERVICE_NAME-v1/pom.xml \
|
||||
sonar:sonar \
|
||||
-Dsonar.projectKey=RED_$SERVICE_NAME \
|
||||
-Dsonar.host.url=https://sonarqube.iqser.com \
|
||||
-Dsonar.host.url=https://sonarqube.knecon.com \
|
||||
-Dsonar.login=${bamboo_sonarqube_api_token_secret} \
|
||||
-Dsonar.pullrequest.key=${bamboo_repository_pr_key} \
|
||||
-Dsonar.pullrequest.branch=${bamboo_repository_pr_sourceBranch} \
|
||||
|
||||
@ -3,9 +3,9 @@
|
||||
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
|
||||
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
|
||||
<parent>
|
||||
<groupId>com.iqser.red</groupId>
|
||||
<groupId>com.knecon.fforesight</groupId>
|
||||
<artifactId>platform-docker-dependency</artifactId>
|
||||
<version>1.2.0</version>
|
||||
<version>0.1.0</version>
|
||||
<relativePath/>
|
||||
</parent>
|
||||
<modelVersion>4.0.0</modelVersion>
|
||||
|
||||
@ -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);
|
||||
}
|
||||
|
||||
|
||||
@ -349,13 +349,16 @@ public class UserService {
|
||||
|
||||
public Optional<User> getUserById(String userId) {
|
||||
|
||||
return userListingService.getAllUsers(TenantContext.getTenantId()).stream().filter(u -> u.getUserId().equalsIgnoreCase(userId)).findAny();
|
||||
return this.getAllUsers().stream().filter(u -> u.getUserId().equalsIgnoreCase(userId)).findAny();
|
||||
}
|
||||
|
||||
public Optional<User> getOptionalUserByUsername(String username) {
|
||||
return this.getAllUsers().stream().filter(u -> u.getUsername().contains(username)).findFirst();
|
||||
}
|
||||
|
||||
|
||||
public List<User> getUsersByIds(Collection<String> userIds) {
|
||||
|
||||
return userListingService.getAllUsers(TenantContext.getTenantId()).stream().filter(u -> userIds.contains(u.getUserId())).collect(Collectors.toList());
|
||||
return this.getAllUsers().stream().filter(u -> userIds.contains(u.getUserId())).collect(Collectors.toList());
|
||||
}
|
||||
|
||||
|
||||
|
||||
@ -12,20 +12,24 @@ public class UserProvider {
|
||||
private UserService userService;
|
||||
|
||||
public String getUserId(){
|
||||
var allUsers = userService.getAllUsers();
|
||||
var managerAdmin = allUsers.stream().filter(u -> u.getUsername().contains("manageradmin1")).findFirst();
|
||||
if(managerAdmin.isPresent()){
|
||||
return managerAdmin.get().getUserId();
|
||||
}
|
||||
throw new RuntimeException("user not created");
|
||||
return this.getUserIdByUsername("manageradmin1");
|
||||
}
|
||||
|
||||
public String getAltUserId(){
|
||||
var allUsers = userService.getAllUsers();
|
||||
var managerAdmin = allUsers.stream().filter(u -> u.getUsername().contains("manageradmin2")).findFirst();
|
||||
if(managerAdmin.isPresent()){
|
||||
return managerAdmin.get().getUserId();
|
||||
return this.getUserIdByUsername("manageradmin2");
|
||||
}
|
||||
|
||||
|
||||
public String getMemberUserId(){
|
||||
return this.getUserIdByUsername("user");
|
||||
}
|
||||
|
||||
private String getUserIdByUsername(String username) {
|
||||
var userOptional = userService.getOptionalUserByUsername(username);
|
||||
if(userOptional.isPresent()){
|
||||
return userOptional.get().getUserId();
|
||||
}
|
||||
throw new RuntimeException("user not created");
|
||||
throw new RuntimeException("user " + username + " not created");
|
||||
|
||||
}
|
||||
}
|
||||
|
||||
@ -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();
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user