From e927e717ce47e13d24031f193038534d28cf7450 Mon Sep 17 00:00:00 2001 From: empassaro <113031808+empassaro@users.noreply.github.com> Date: Thu, 19 Sep 2024 15:42:30 +0200 Subject: [PATCH] [SELC-5466] feat: added filename handling to persist signed contract with correct extension (#486) --- .../onboarding/constants/CustomError.java | 1 + .../controller/OnboardingController.java | 21 +++--- .../selfcare/onboarding/model/FormItem.java | 13 ++++ .../onboarding/service/OnboardingService.java | 9 ++- .../service/OnboardingServiceDefault.java | 69 +++++++++++++------ .../selfcare/onboarding/util/Utils.java | 28 ++++++++ .../service/OnboardingServiceDefaultTest.java | 26 ++++--- 7 files changed, 119 insertions(+), 48 deletions(-) create mode 100644 apps/onboarding-ms/src/main/java/it/pagopa/selfcare/onboarding/model/FormItem.java create mode 100644 apps/onboarding-ms/src/main/java/it/pagopa/selfcare/onboarding/util/Utils.java diff --git a/apps/onboarding-ms/src/main/java/it/pagopa/selfcare/onboarding/constants/CustomError.java b/apps/onboarding-ms/src/main/java/it/pagopa/selfcare/onboarding/constants/CustomError.java index 44b8559c8..0f6891cf9 100644 --- a/apps/onboarding-ms/src/main/java/it/pagopa/selfcare/onboarding/constants/CustomError.java +++ b/apps/onboarding-ms/src/main/java/it/pagopa/selfcare/onboarding/constants/CustomError.java @@ -15,6 +15,7 @@ public enum CustomError { DENIED_NO_BILLING("0040","Recipient code linked to an institution with invoicing service not active"), DENIED_NO_ASSOCIATION("0041","Recipient code not linked to any institution"), + TOO_MANY_CONTRACTS("0043","Too many contracts provided"), ; diff --git a/apps/onboarding-ms/src/main/java/it/pagopa/selfcare/onboarding/controller/OnboardingController.java b/apps/onboarding-ms/src/main/java/it/pagopa/selfcare/onboarding/controller/OnboardingController.java index c669e106d..152740164 100644 --- a/apps/onboarding-ms/src/main/java/it/pagopa/selfcare/onboarding/controller/OnboardingController.java +++ b/apps/onboarding-ms/src/main/java/it/pagopa/selfcare/onboarding/controller/OnboardingController.java @@ -20,20 +20,20 @@ import jakarta.validation.Valid; import jakarta.validation.constraints.NotNull; import jakarta.ws.rs.*; -import jakarta.ws.rs.core.Context; -import jakarta.ws.rs.core.MediaType; -import jakarta.ws.rs.core.Response; -import jakarta.ws.rs.core.SecurityContext; +import jakarta.ws.rs.core.*; import lombok.AllArgsConstructor; import org.apache.http.HttpStatus; import org.eclipse.microprofile.openapi.annotations.Operation; import org.eclipse.microprofile.openapi.annotations.tags.Tag; import org.jboss.resteasy.reactive.RestForm; +import org.jboss.resteasy.reactive.server.core.ResteasyReactiveRequestContext; import java.io.File; import java.util.List; import java.util.Objects; +import static it.pagopa.selfcare.onboarding.util.Utils.retrieveContractFromFormData; + @Authenticated @Path("/v1/onboarding") @Tag(name = "Onboarding Controller") @@ -164,7 +164,6 @@ public Uni onboardingPgCompletion(@Valid OnboardingPgRequest } private Uni readUserIdFromToken(SecurityContext ctx) { - return currentIdentityAssociation.getDeferredIdentity() .onItem().transformToUni(identity -> { if (ctx.getUserPrincipal() == null || !ctx.getUserPrincipal().getName().equals(identity.getPrincipal().getName())) { @@ -189,8 +188,8 @@ private Uni readUserIdFromToken(SecurityContext ctx) { @Path("/{onboardingId}/complete") @Tag(name = "internal-v1") @Consumes(MediaType.MULTIPART_FORM_DATA) - public Uni complete(@PathParam(value = "onboardingId") String onboardingId, @NotNull @RestForm("contract") File file) { - return onboardingService.complete(onboardingId, file) + public Uni complete(@PathParam(value = "onboardingId") String onboardingId, @NotNull @RestForm("contract") File file, @Context ResteasyReactiveRequestContext ctx) { + return onboardingService.complete(onboardingId, retrieveContractFromFormData(ctx.getFormData(), file)) .map(ignore -> Response .status(HttpStatus.SC_NO_CONTENT) .build()); @@ -204,8 +203,8 @@ public Uni complete(@PathParam(value = "onboardingId") String onboardi @PUT @Path("/{onboardingId}/completeOnboardingUsers") @Consumes(MediaType.MULTIPART_FORM_DATA) - public Uni completeOnboardingUser(@PathParam(value = "onboardingId") String onboardingId, @NotNull @RestForm("contract") File file) { - return onboardingService.completeOnboardingUsers(onboardingId, file) + public Uni completeOnboardingUser(@PathParam(value = "onboardingId") String onboardingId, @NotNull @RestForm("contract") File file, @Context ResteasyReactiveRequestContext ctx) { + return onboardingService.completeOnboardingUsers(onboardingId, retrieveContractFromFormData(ctx.getFormData(), file)) .map(ignore -> Response .status(HttpStatus.SC_NO_CONTENT) .build()); @@ -218,8 +217,8 @@ public Uni completeOnboardingUser(@PathParam(value = "onboardingId") S @Tag(name = "internal-v1") @Tag(name = "Onboarding") @Consumes(MediaType.MULTIPART_FORM_DATA) - public Uni consume(@PathParam(value = "onboardingId") String onboardingId, @NotNull @RestForm("contract") File file) { - return onboardingService.completeWithoutSignatureVerification(onboardingId, file) + public Uni consume(@PathParam(value = "onboardingId") String onboardingId, @NotNull @RestForm("contract") File file, @Context ResteasyReactiveRequestContext ctx) { + return onboardingService.completeWithoutSignatureVerification(onboardingId, retrieveContractFromFormData(ctx.getFormData(), file)) .map(ignore -> Response .status(HttpStatus.SC_NO_CONTENT) .build()); diff --git a/apps/onboarding-ms/src/main/java/it/pagopa/selfcare/onboarding/model/FormItem.java b/apps/onboarding-ms/src/main/java/it/pagopa/selfcare/onboarding/model/FormItem.java new file mode 100644 index 000000000..bcc644f53 --- /dev/null +++ b/apps/onboarding-ms/src/main/java/it/pagopa/selfcare/onboarding/model/FormItem.java @@ -0,0 +1,13 @@ +package it.pagopa.selfcare.onboarding.model; + +import lombok.Builder; +import lombok.Getter; + +import java.io.File; + +@Builder +@Getter +public class FormItem { + private File file; + private String fileName; +} diff --git a/apps/onboarding-ms/src/main/java/it/pagopa/selfcare/onboarding/service/OnboardingService.java b/apps/onboarding-ms/src/main/java/it/pagopa/selfcare/onboarding/service/OnboardingService.java index b709e4078..a310cd2e7 100644 --- a/apps/onboarding-ms/src/main/java/it/pagopa/selfcare/onboarding/service/OnboardingService.java +++ b/apps/onboarding-ms/src/main/java/it/pagopa/selfcare/onboarding/service/OnboardingService.java @@ -11,9 +11,8 @@ import it.pagopa.selfcare.onboarding.controller.response.OnboardingGetResponse; import it.pagopa.selfcare.onboarding.controller.response.OnboardingResponse; import it.pagopa.selfcare.onboarding.entity.Onboarding; +import it.pagopa.selfcare.onboarding.model.FormItem; import it.pagopa.selfcare.onboarding.model.OnboardingGetFilters; - -import java.io.File; import java.util.List; public interface OnboardingService { @@ -30,11 +29,11 @@ public interface OnboardingService { Uni approve(String onboardingId); - Uni complete(String tokenId, File contract); + Uni complete(String tokenId, FormItem formItem); - Uni completeOnboardingUsers(String tokenId, File contract); + Uni completeOnboardingUsers(String tokenId, FormItem formItem); - Uni completeWithoutSignatureVerification(String tokenId, File contract); + Uni completeWithoutSignatureVerification(String tokenId, FormItem formItem); Uni onboardingGet(OnboardingGetFilters filters); diff --git a/apps/onboarding-ms/src/main/java/it/pagopa/selfcare/onboarding/service/OnboardingServiceDefault.java b/apps/onboarding-ms/src/main/java/it/pagopa/selfcare/onboarding/service/OnboardingServiceDefault.java index 9376ba45b..630a16f20 100644 --- a/apps/onboarding-ms/src/main/java/it/pagopa/selfcare/onboarding/service/OnboardingServiceDefault.java +++ b/apps/onboarding-ms/src/main/java/it/pagopa/selfcare/onboarding/service/OnboardingServiceDefault.java @@ -27,6 +27,7 @@ import it.pagopa.selfcare.onboarding.mapper.InstitutionMapper; import it.pagopa.selfcare.onboarding.mapper.OnboardingMapper; import it.pagopa.selfcare.onboarding.mapper.UserMapper; +import it.pagopa.selfcare.onboarding.model.FormItem; import it.pagopa.selfcare.onboarding.model.OnboardingGetFilters; import it.pagopa.selfcare.onboarding.service.strategy.OnboardingValidationStrategy; import it.pagopa.selfcare.onboarding.service.util.OnboardingUtils; @@ -57,7 +58,6 @@ import org.openapi.quarkus.user_registry_json.api.UserApi; import org.openapi.quarkus.user_registry_json.model.*; -import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.time.LocalDateTime; @@ -729,7 +729,7 @@ public Uni approve(String onboardingId) { } @Override - public Uni complete(String onboardingId, File contract) { + public Uni complete(String onboardingId, FormItem formItem) { if (Boolean.TRUE.equals(isVerifyEnabled)) { //Retrieve as Tuple: managers fiscal-code from user registry and contract digest @@ -739,23 +739,21 @@ public Uni complete(String onboardingId, File contract) { .asTuple() .onItem().transformToUni(inputSignatureVerification -> Uni.createFrom().item(() -> { - signatureService.verifySignature(contract, + signatureService.verifySignature(formItem.getFile(), inputSignatureVerification.getItem2(), inputSignatureVerification.getItem1()); return onboarding; }) .runSubscriptionOn(Infrastructure.getDefaultWorkerPool()) ); - - return complete(onboardingId, contract, verification); + return complete(onboardingId, formItem, verification); } else { - return completeWithoutSignatureVerification(onboardingId, contract); + return completeWithoutSignatureVerification(onboardingId, formItem); } } @Override - public Uni completeOnboardingUsers(String onboardingId, File contract) { - + public Uni completeOnboardingUsers(String onboardingId, FormItem formItem) { if (Boolean.TRUE.equals(isVerifyEnabled)) { //Retrieve as Tuple: managers fiscal-code from user registry and contract digest //At least, verify contract signature using both @@ -764,7 +762,7 @@ public Uni completeOnboardingUsers(String onboardingId, File contrac .asTuple() .onItem().transformToUni(inputSignatureVerification -> Uni.createFrom().item(() -> { - signatureService.verifySignature(contract, + signatureService.verifySignature(formItem.getFile(), inputSignatureVerification.getItem2(), inputSignatureVerification.getItem1()); return onboarding; @@ -772,24 +770,24 @@ public Uni completeOnboardingUsers(String onboardingId, File contrac .runSubscriptionOn(Infrastructure.getDefaultWorkerPool()) ); - return completeOnboardingUsers(onboardingId, contract, verification); + return completeOnboardingUsers(onboardingId, formItem, verification); } else { - return completeOnboardingUsersWithoutSignatureVerification(onboardingId, contract); + return completeOnboardingUsersWithoutSignatureVerification(onboardingId, formItem); } } - public Uni completeOnboardingUsersWithoutSignatureVerification(String onboardingId, File contract) { + public Uni completeOnboardingUsersWithoutSignatureVerification(String onboardingId, FormItem formItem) { Function> verification = ignored -> Uni.createFrom().item(ignored); - return completeOnboardingUsers(onboardingId, contract, verification); + return completeOnboardingUsers(onboardingId, formItem, verification); } @Override - public Uni completeWithoutSignatureVerification(String onboardingId, File contract) { + public Uni completeWithoutSignatureVerification(String onboardingId, FormItem formItem) { Function> verification = ignored -> Uni.createFrom().item(ignored); - return complete(onboardingId, contract, verification); + return complete(onboardingId, formItem, verification); } - private Uni complete(String onboardingId, File contract, Function> verificationContractSignature) { + private Uni complete(String onboardingId, FormItem formItem, Function> verificationContractSignature) { return retrieveOnboardingAndCheckIfExpired(onboardingId) .onItem().transformToUni(verificationContractSignature::apply) @@ -801,7 +799,7 @@ private Uni complete(String onboardingId, File contract, Function uploadSignedContractAndUpdateToken(onboardingId, contract) + .onItem().transformToUni(onboarding -> uploadSignedContractAndUpdateToken(onboardingId, formItem) .map(ignore -> onboarding)) // Start async activity if onboardingOrchestrationEnabled is true .onItem().transformToUni(onboarding -> onboardingOrchestrationEnabled @@ -810,7 +808,7 @@ private Uni complete(String onboardingId, File contract, Function completeOnboardingUsers(String onboardingId, File contract, Function> verificationContractSignature) { + private Uni completeOnboardingUsers(String onboardingId, FormItem formItem, Function> verificationContractSignature) { return retrieveOnboardingAndCheckIfExpired(onboardingId) .onItem().transformToUni(verificationContractSignature::apply) @@ -822,7 +820,7 @@ private Uni completeOnboardingUsers(String onboardingId, File contra .replaceWith(onboarding) ) //Upload contract on storage - .onItem().transformToUni(onboarding -> uploadSignedContractAndUpdateToken(onboardingId, contract) + .onItem().transformToUni(onboarding -> uploadSignedContractAndUpdateToken(onboardingId, formItem) .map(ignore -> onboarding)) // Start async activity if onboardingOrchestrationEnabled is true .onItem().transformToUni(onboarding -> onboardingOrchestrationEnabled @@ -831,14 +829,17 @@ private Uni completeOnboardingUsers(String onboardingId, File contra : Uni.createFrom().item(onboarding)); } - private Uni uploadSignedContractAndUpdateToken(String onboardingId, File contract) { + private Uni uploadSignedContractAndUpdateToken(String onboardingId, FormItem formItem) { return retrieveToken(onboardingId) .onItem().transformToUni(token -> Uni.createFrom().item(Unchecked.supplier(() -> { final String path = String.format("%s%s", pathContracts, onboardingId); - final String filename = String.format("signed_%s", Optional.ofNullable(token.getContractFilename()).orElse(onboardingId)); + final String signedContractExtension = getFileExtension(formItem.getFileName()); + final String persistedContractFileName = Optional.ofNullable(token.getContractFilename()).orElse(onboardingId); + final String signedContractFileName = replaceFileExtension(persistedContractFileName, signedContractExtension); + final String filename = String.format("signed_%s", signedContractFileName); try { - return azureBlobClient.uploadFile(path, filename, Files.readAllBytes(contract.toPath())); + return azureBlobClient.uploadFile(path, filename, Files.readAllBytes(formItem.getFile().toPath())); } catch (IOException e) { throw new OnboardingNotAllowedException(GENERIC_ERROR.getCode(), "Error on upload contract for onboarding with id " + onboardingId); @@ -851,6 +852,30 @@ private Uni uploadSignedContractAndUpdateToken(String onboardingId, File ); } + private String getFileExtension(String name) { + String[] parts = name.split("\\."); + String ext = ""; + + if (parts.length == 2) { + return parts[1]; + } + + if(parts.length > 2) { + // join all parts except the first one + ext = String.join(".", Arrays.copyOfRange(parts, 1, parts.length)); + } + + return ext; + } + + private String replaceFileExtension(String originalFilename, String newExtension) { + int lastIndexOf = originalFilename.lastIndexOf("."); + if (lastIndexOf == -1) { + return originalFilename + newExtension; + } else { + return originalFilename.substring(0, lastIndexOf) + "." + newExtension; + } + } private Uni retrieveOnboardingAndCheckIfExpired(String onboardingId) { //Retrieve Onboarding if exists diff --git a/apps/onboarding-ms/src/main/java/it/pagopa/selfcare/onboarding/util/Utils.java b/apps/onboarding-ms/src/main/java/it/pagopa/selfcare/onboarding/util/Utils.java new file mode 100644 index 000000000..a9dde5595 --- /dev/null +++ b/apps/onboarding-ms/src/main/java/it/pagopa/selfcare/onboarding/util/Utils.java @@ -0,0 +1,28 @@ +package it.pagopa.selfcare.onboarding.util; + +import it.pagopa.selfcare.onboarding.constants.CustomError; +import it.pagopa.selfcare.onboarding.exception.InvalidRequestException; +import it.pagopa.selfcare.onboarding.model.FormItem; +import org.jboss.resteasy.reactive.server.core.multipart.FormData; +import org.jboss.resteasy.reactive.server.multipart.FormValue; + +import java.io.File; +import java.util.Deque; + +public class Utils { + private static final String DEFAULT_CONTRACT_FORM_DATA_NAME = "contract"; + private Utils() { + } + + public static FormItem retrieveContractFromFormData(FormData formData, File file) { + Deque deck = formData.get(DEFAULT_CONTRACT_FORM_DATA_NAME); + if(deck.size() > 1) { + throw new InvalidRequestException(CustomError.TOO_MANY_CONTRACTS.getMessage(), CustomError.TOO_MANY_CONTRACTS.getCode()); + } + + return FormItem.builder() + .file(file) + .fileName(deck.getFirst().getFileName()) + .build(); + } +} diff --git a/apps/onboarding-ms/src/test/java/it/pagopa/selfcare/onboarding/service/OnboardingServiceDefaultTest.java b/apps/onboarding-ms/src/test/java/it/pagopa/selfcare/onboarding/service/OnboardingServiceDefaultTest.java index 69dbcf151..8d9dbb2a7 100644 --- a/apps/onboarding-ms/src/test/java/it/pagopa/selfcare/onboarding/service/OnboardingServiceDefaultTest.java +++ b/apps/onboarding-ms/src/test/java/it/pagopa/selfcare/onboarding/service/OnboardingServiceDefaultTest.java @@ -31,6 +31,7 @@ import it.pagopa.selfcare.onboarding.exception.ResourceNotFoundException; import it.pagopa.selfcare.onboarding.mapper.OnboardingMapper; import it.pagopa.selfcare.onboarding.mapper.OnboardingMapperImpl; +import it.pagopa.selfcare.onboarding.model.FormItem; import it.pagopa.selfcare.onboarding.model.OnboardingGetFilters; import it.pagopa.selfcare.onboarding.service.profile.OnboardingTestProfile; import it.pagopa.selfcare.onboarding.service.strategy.OnboardingValidationStrategy; @@ -150,6 +151,11 @@ class OnboardingServiceDefaultTest { static final File testFile = new File("src/test/resources/application.properties"); + static final FormItem TEST_FORM_ITEM = FormItem.builder() + .fileName("testFile") + .file(testFile) + .build(); + static { managerResource = new UserResource(); managerResource.setId(UUID.randomUUID()); @@ -1203,7 +1209,7 @@ void completeWithoutSignatureVerification(UniAsserter asserter) { when(azureBlobClient.uploadFile(any(), any(), any())).thenReturn(filepath); mockUpdateToken(asserter, filepath); - asserter.assertThat(() -> onboardingService.completeWithoutSignatureVerification(onboarding.getId(), testFile), + asserter.assertThat(() -> onboardingService.completeWithoutSignatureVerification(onboarding.getId(), TEST_FORM_ITEM), Assertions::assertNotNull); } @@ -1238,7 +1244,7 @@ void completeWithoutSignatureValidation(UniAsserter asserter) { when(azureBlobClient.uploadFile(any(), any(), any())).thenReturn(filepath); mockUpdateToken(asserter, filepath); - asserter.assertThat(() -> onboardingService.complete(onboarding.getId(), testFile), + asserter.assertThat(() -> onboardingService.complete(onboarding.getId(), TEST_FORM_ITEM), Assertions::assertNotNull); } @@ -1273,7 +1279,7 @@ void completeOnboardingUsersWithoutSignatureValidation(UniAsserter asserter) { when(azureBlobClient.uploadFile(any(), any(), any())).thenReturn(filepath); mockUpdateToken(asserter, filepath); - asserter.assertThat(() -> onboardingService.completeOnboardingUsers(onboarding.getId(), testFile), + asserter.assertThat(() -> onboardingService.completeOnboardingUsers(onboarding.getId(), TEST_FORM_ITEM), Assertions::assertNotNull); } @@ -1290,7 +1296,7 @@ void completeOnboardingUsers_throwProductNotOnboardedInReferenceOnboarding(UniAs mockSimpleProductValidAssert(onboarding.getProductId(), false, asserter); mockVerifyAllowedMap(onboarding.getInstitution().getTaxCode(), onboarding.getProductId(), asserter); - asserter.assertFailedWith(() -> onboardingService.completeOnboardingUsers(onboarding.getId(), testFile), + asserter.assertFailedWith(() -> onboardingService.completeOnboardingUsers(onboarding.getId(), TEST_FORM_ITEM), InvalidRequestException.class); } @@ -1308,7 +1314,7 @@ void completeOnboardingUsers_throwOnboardingNotAllowedException(UniAsserter asse asserter.execute(() -> when(onboardingValidationStrategy.validate(onboarding.getProductId(), onboarding.getInstitution().getTaxCode())) .thenReturn(false)); - asserter.assertFailedWith(() -> onboardingService.completeOnboardingUsers(onboarding.getId(), testFile), + asserter.assertFailedWith(() -> onboardingService.completeOnboardingUsers(onboarding.getId(), TEST_FORM_ITEM), OnboardingNotAllowedException.class); } @@ -1324,7 +1330,7 @@ void completeOnboardingUsers_throwInvalidRequestException(UniAsserter asserter) mockSimpleProductValidAssert(onboarding.getProductId(), false, asserter); mockVerifyAllowedMap(onboarding.getInstitution().getTaxCode(), onboarding.getProductId(), asserter); - asserter.assertFailedWith(() -> onboardingService.completeOnboardingUsers(onboarding.getId(), testFile), + asserter.assertFailedWith(() -> onboardingService.completeOnboardingUsers(onboarding.getId(), TEST_FORM_ITEM), InvalidRequestException.class); } @@ -1952,7 +1958,7 @@ void complete_shouldThrowExceptionWhenSignatureFail(UniAsserter asserter) { .when(signatureService) .verifySignature(any(), any(), any())); - asserter.assertFailedWith(() -> onboardingService.complete(onboarding.getId(), testFile), + asserter.assertFailedWith(() -> onboardingService.complete(onboarding.getId(), TEST_FORM_ITEM), InvalidRequestException.class); } // can't be tested @@ -1978,7 +1984,7 @@ void completeOnboardingUsers_shouldThrowExceptionWhenSignatureFail(UniAsserter a .when(signatureService) .verifySignature(any(), any(), any())); - asserter.assertFailedWith(() -> onboardingService.completeOnboardingUsers(onboarding.getId(), testFile), + asserter.assertFailedWith(() -> onboardingService.completeOnboardingUsers(onboarding.getId(), TEST_FORM_ITEM), InvalidRequestException.class); } @@ -2013,7 +2019,7 @@ void completeOnboardingUsers(UniAsserter asserter) { when(azureBlobClient.uploadFile(any(), any(), any())).thenReturn(filepath); mockUpdateToken(asserter, filepath); - asserter.assertThat(() -> onboardingService.completeOnboardingUsers(onboarding.getId(), testFile), + asserter.assertThat(() -> onboardingService.completeOnboardingUsers(onboarding.getId(), TEST_FORM_ITEM), Assertions::assertNotNull); } @@ -2047,7 +2053,7 @@ void complete(UniAsserter asserter) { when(azureBlobClient.uploadFile(any(), any(), any())).thenReturn(filepath); mockUpdateToken(asserter, filepath); - asserter.assertThat(() -> onboardingService.complete(onboarding.getId(), testFile), + asserter.assertThat(() -> onboardingService.complete(onboarding.getId(), TEST_FORM_ITEM), Assertions::assertNotNull); } }