From 0bb5345beabb3ffa4cf4b807fde99a43298537d4 Mon Sep 17 00:00:00 2001 From: mscarsel Date: Mon, 13 Jan 2025 17:17:41 +0100 Subject: [PATCH 01/12] [P4ADEV-1830] added logics to save the ingestionFlowFile --- openapi/p4pa-fileshare.openapi.yaml | 1 + .../exception/FileshareExceptionHandler.java | 6 ++ .../exception/custom/FileUploadException.java | 7 ++ .../pu/fileshare/service/FileService.java | 51 ++++++++++++ .../IngestionFlowFileServiceImpl.java | 7 +- src/main/resources/application.yml | 5 ++ .../FileshareExceptionHandlerTest.java | 14 ++++ .../pu/fileshare/service/FileServiceTest.java | 78 ++++++++++++++++++- .../IngestionFlowFileServiceImplTest.java | 4 +- 9 files changed, 170 insertions(+), 3 deletions(-) create mode 100644 src/main/java/it/gov/pagopa/pu/fileshare/exception/custom/FileUploadException.java diff --git a/openapi/p4pa-fileshare.openapi.yaml b/openapi/p4pa-fileshare.openapi.yaml index 721074f..1c44608 100644 --- a/openapi/p4pa-fileshare.openapi.yaml +++ b/openapi/p4pa-fileshare.openapi.yaml @@ -70,5 +70,6 @@ components: type: string enum: - INVALID_FILE + - FILE_UPLOAD_ERROR message: type: string diff --git a/src/main/java/it/gov/pagopa/pu/fileshare/exception/FileshareExceptionHandler.java b/src/main/java/it/gov/pagopa/pu/fileshare/exception/FileshareExceptionHandler.java index 27f4c80..f98c315 100644 --- a/src/main/java/it/gov/pagopa/pu/fileshare/exception/FileshareExceptionHandler.java +++ b/src/main/java/it/gov/pagopa/pu/fileshare/exception/FileshareExceptionHandler.java @@ -2,6 +2,7 @@ import it.gov.pagopa.pu.fileshare.dto.generated.FileshareErrorDTO; import it.gov.pagopa.pu.fileshare.dto.generated.FileshareErrorDTO.CodeEnum; +import it.gov.pagopa.pu.fileshare.exception.custom.FileUploadException; import it.gov.pagopa.pu.fileshare.exception.custom.InvalidFileException; import jakarta.servlet.http.HttpServletRequest; import lombok.extern.slf4j.Slf4j; @@ -26,6 +27,11 @@ public ResponseEntity handleInvalidFileError(RuntimeException return handleFileshareErrorException(ex, request, HttpStatus.BAD_REQUEST, CodeEnum.INVALID_FILE); } + @ExceptionHandler({FileUploadException.class}) + public ResponseEntity handleFileStorageError(RuntimeException ex, HttpServletRequest request){ + return handleFileshareErrorException(ex, request, HttpStatus.INTERNAL_SERVER_ERROR, CodeEnum.FILE_UPLOAD_ERROR); + } + static ResponseEntity handleFileshareErrorException(RuntimeException ex, HttpServletRequest request, HttpStatus httpStatus, FileshareErrorDTO.CodeEnum errorEnum) { String message = logException(ex, request, httpStatus); diff --git a/src/main/java/it/gov/pagopa/pu/fileshare/exception/custom/FileUploadException.java b/src/main/java/it/gov/pagopa/pu/fileshare/exception/custom/FileUploadException.java new file mode 100644 index 0000000..67b2bdb --- /dev/null +++ b/src/main/java/it/gov/pagopa/pu/fileshare/exception/custom/FileUploadException.java @@ -0,0 +1,7 @@ +package it.gov.pagopa.pu.fileshare.exception.custom; + +public class FileUploadException extends RuntimeException { + public FileUploadException(String message) { + super(message); + } +} diff --git a/src/main/java/it/gov/pagopa/pu/fileshare/service/FileService.java b/src/main/java/it/gov/pagopa/pu/fileshare/service/FileService.java index a8b3412..9025f6e 100644 --- a/src/main/java/it/gov/pagopa/pu/fileshare/service/FileService.java +++ b/src/main/java/it/gov/pagopa/pu/fileshare/service/FileService.java @@ -1,18 +1,69 @@ package it.gov.pagopa.pu.fileshare.service; +import it.gov.pagopa.pu.fileshare.exception.custom.FileUploadException; import it.gov.pagopa.pu.fileshare.exception.custom.InvalidFileException; +import it.gov.pagopa.pu.fileshare.util.AESUtils; +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.StandardCopyOption; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.StringUtils; +import org.springframework.beans.factory.annotation.Value; import org.springframework.stereotype.Service; import org.springframework.web.multipart.MultipartFile; @Slf4j @Service public class FileService { + private final String sharedFolderRootPath; + private final String fileEncryptPassword; + + public FileService(@Value("${folders.shared}") String sharedFolderRootPath, + @Value(("${app.fileEncryptPassword}")) String fileEncryptPassword) { + this.sharedFolderRootPath = sharedFolderRootPath; + this.fileEncryptPassword = fileEncryptPassword; + } + public void validateFile(MultipartFile ingestionFlowFile, String validFileExt) { if( ingestionFlowFile == null || !StringUtils.defaultString(ingestionFlowFile.getOriginalFilename()).endsWith(validFileExt)){ log.debug("Invalid ingestion flow file extension"); throw new InvalidFileException("Invalid file extension"); } } + + public void saveToSharedFolder(MultipartFile file, String relativePath){ + if(file==null){ + log.debug("File is mandatory"); + throw new FileUploadException("File is mandatory"); + } + + String filename = org.springframework.util.StringUtils.cleanPath(StringUtils.defaultString(file.getOriginalFilename())); + Path fileLocation = Paths.get(sharedFolderRootPath, relativePath, filename); + //create missing parent folder, if any + try { + if (!fileLocation.toAbsolutePath().getParent().toFile().exists()) + Files.createDirectories(fileLocation.toAbsolutePath().getParent()); + encryptAndSaveFile(file, fileLocation); + }catch (Exception e) { + log.debug( + "Error uploading file to folder %s%s".formatted(sharedFolderRootPath, + relativePath), e); + throw new FileUploadException( + "Error uploading file to folder %s%s [%s]".formatted( + sharedFolderRootPath, relativePath, e.getMessage())); + } + log.debug("File upload to folder %s%s completed".formatted(sharedFolderRootPath, + relativePath)); + } + + private void encryptAndSaveFile(MultipartFile file, Path fileLocation) + throws IOException { + try(InputStream is = file.getInputStream(); + InputStream cipherIs = AESUtils.encrypt(fileEncryptPassword, is)){ + Files.copy(cipherIs, fileLocation, StandardCopyOption.REPLACE_EXISTING); + } + } } diff --git a/src/main/java/it/gov/pagopa/pu/fileshare/service/ingestion/IngestionFlowFileServiceImpl.java b/src/main/java/it/gov/pagopa/pu/fileshare/service/ingestion/IngestionFlowFileServiceImpl.java index f14baf6..9c88898 100644 --- a/src/main/java/it/gov/pagopa/pu/fileshare/service/ingestion/IngestionFlowFileServiceImpl.java +++ b/src/main/java/it/gov/pagopa/pu/fileshare/service/ingestion/IngestionFlowFileServiceImpl.java @@ -15,18 +15,23 @@ public class IngestionFlowFileServiceImpl implements IngestionFlowFileService { private final UserAuthorizationService userAuthorizationService; private final FileService fileService; private final String validIngestionFlowFileExt; + private final String ingestionFlowFilePath; public IngestionFlowFileServiceImpl( UserAuthorizationService userAuthorizationService, FileService fileService, - @Value("${uploads.ingestion-flow-file.valid-extension}") String validIngestionFlowFileExt) { + @Value("${uploads.ingestion-flow-file.valid-extension}") String validIngestionFlowFileExt, + @Value("${folders.ingestion-flow-file.path}") String ingestionFlowFilePath + ) { this.userAuthorizationService = userAuthorizationService; this.fileService = fileService; this.validIngestionFlowFileExt = validIngestionFlowFileExt; + this.ingestionFlowFilePath = ingestionFlowFilePath; } @Override public void uploadIngestionFlowFile(Long organizationId, IngestionFlowFileType ingestionFlowFileType, MultipartFile ingestionFlowFile, UserInfo user, String accessToken) { userAuthorizationService.checkUserAuthorization(organizationId, user, accessToken); fileService.validateFile(ingestionFlowFile, validIngestionFlowFileExt); + fileService.saveToSharedFolder(ingestionFlowFile,ingestionFlowFilePath); } } diff --git a/src/main/resources/application.yml b/src/main/resources/application.yml index aff6dff..68b8998 100644 --- a/src/main/resources/application.yml +++ b/src/main/resources/application.yml @@ -51,6 +51,8 @@ folders: process-target-sub-folders: archive: "\${PROCESS_TARGET_SUB_FOLDER_ARCHIVE:archive}" errors: "\${PROCESS_TARGET_SUB_FOLDER_ERRORS:errors}" + ingestion-flow-file: + path: "\${INGESTION_FLOW_FILE_PATH:/ingestion_flow_file}" rest: default-timeout: @@ -64,3 +66,6 @@ rest: uploads: ingestion-flow-file: valid-extension: "\${INGESTION_FLOW_FILE_VALID_EXTENSION:.zip}" + +app: + fileEncryptPassword: "\${FILE_ENCRYPT_PASSWORD:ENCR_PSW}" diff --git a/src/test/java/it/gov/pagopa/pu/fileshare/exception/FileshareExceptionHandlerTest.java b/src/test/java/it/gov/pagopa/pu/fileshare/exception/FileshareExceptionHandlerTest.java index eaa6c58..4ce586d 100644 --- a/src/test/java/it/gov/pagopa/pu/fileshare/exception/FileshareExceptionHandlerTest.java +++ b/src/test/java/it/gov/pagopa/pu/fileshare/exception/FileshareExceptionHandlerTest.java @@ -3,6 +3,7 @@ import static org.mockito.Mockito.doThrow; import it.gov.pagopa.pu.fileshare.dto.generated.FileshareErrorDTO.CodeEnum; +import it.gov.pagopa.pu.fileshare.exception.custom.FileUploadException; import it.gov.pagopa.pu.fileshare.exception.custom.InvalidFileException; import lombok.extern.slf4j.Slf4j; import org.junit.jupiter.api.Test; @@ -58,4 +59,17 @@ void handleInvalidFileException() throws Exception { .andExpect(MockMvcResultMatchers.jsonPath("$.code").value(CodeEnum.INVALID_FILE.toString())) .andExpect(MockMvcResultMatchers.jsonPath("$.message").value("Error")); } + + @Test + void handleFileUploadException() throws Exception { + doThrow(new FileUploadException("Error")).when(testControllerSpy).testEndpoint(DATA); + + mockMvc.perform(MockMvcRequestBuilders.get("/test") + .param(DATA, DATA) + .contentType(MediaType.APPLICATION_JSON) + .accept(MediaType.APPLICATION_JSON)) + .andExpect(MockMvcResultMatchers.status().isInternalServerError()) + .andExpect(MockMvcResultMatchers.jsonPath("$.code").value(CodeEnum.FILE_UPLOAD_ERROR.toString())) + .andExpect(MockMvcResultMatchers.jsonPath("$.message").value("Error")); + } } diff --git a/src/test/java/it/gov/pagopa/pu/fileshare/service/FileServiceTest.java b/src/test/java/it/gov/pagopa/pu/fileshare/service/FileServiceTest.java index 802ed1c..5180fb3 100644 --- a/src/test/java/it/gov/pagopa/pu/fileshare/service/FileServiceTest.java +++ b/src/test/java/it/gov/pagopa/pu/fileshare/service/FileServiceTest.java @@ -1,10 +1,16 @@ package it.gov.pagopa.pu.fileshare.service; +import it.gov.pagopa.pu.fileshare.exception.custom.FileUploadException; import it.gov.pagopa.pu.fileshare.exception.custom.InvalidFileException; +import it.gov.pagopa.pu.fileshare.util.AESUtils; +import java.io.InputStream; +import java.nio.file.Files; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.MockedStatic; +import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.http.MediaType; import org.springframework.mock.web.MockMultipartFile; @@ -13,10 +19,12 @@ class FileServiceTest { private FileService fileService; private static final String VALID_FILE_EXTENSION = ".zip"; + private static final String SHARED_FOLDER_ROOT_PATH = "testPath"; + private static final String FILE_ENCRYPT_PASSWORD = "testPassword"; @BeforeEach void setUp() { - fileService = new FileService(); + fileService = new FileService(SHARED_FOLDER_ROOT_PATH,FILE_ENCRYPT_PASSWORD); } @Test @@ -47,4 +55,72 @@ void givenInvalidFileExtensionWhenValidateFileThenInvalidFileException(){ //do nothing } } + + @Test + void givenInvalidFileWhenSaveToSharedFolderThenIllegalStateException() { + try (MockedStatic aesUtilsMockedStatic = Mockito.mockStatic( + AESUtils.class); + MockedStatic filesMockedStatic = Mockito.mockStatic( + Files.class)) { + try { + fileService.saveToSharedFolder(null, ""); + Assertions.fail("Expected FileUploadException"); + } catch (FileUploadException e) { + aesUtilsMockedStatic.verifyNoInteractions(); + filesMockedStatic.verifyNoInteractions(); + } + } + } + + @Test + void givenErrorWhenSaveToSharedFolderThenIllegalStateException() { + MockMultipartFile file = new MockMultipartFile( + "ingestionFlowFile", + "test.txt", + MediaType.TEXT_PLAIN_VALUE, + "this is a test file".getBytes() + ); + + try (MockedStatic aesUtilsMockedStatic = Mockito.mockStatic( + AESUtils.class); + MockedStatic filesMockedStatic = Mockito.mockStatic( + Files.class)) { + Mockito.when(AESUtils.encrypt(Mockito.eq(FILE_ENCRYPT_PASSWORD), (InputStream) Mockito.any())) + .thenThrow(new RuntimeException()); + + try { + fileService.saveToSharedFolder(file, ""); + Assertions.fail("Expected FileUploadException"); + } catch (FileUploadException e) { + aesUtilsMockedStatic.verify(() -> AESUtils.encrypt(Mockito.anyString(),(InputStream) Mockito.any())); + aesUtilsMockedStatic.verifyNoMoreInteractions(); + filesMockedStatic.verify(() -> Files.createDirectories(Mockito.any())); + filesMockedStatic.verifyNoMoreInteractions(); + } + } + } + + @Test + void givenValidFileWhenSaveToSharedFolderThenOK() { + MockMultipartFile file = new MockMultipartFile( + "ingestionFlowFile", + "test.txt", + MediaType.TEXT_PLAIN_VALUE, + "this is a test file".getBytes() + ); + + try (MockedStatic aesUtilsMockedStatic = Mockito.mockStatic( + AESUtils.class); + MockedStatic filesMockedStatic = Mockito.mockStatic( + Files.class) + ) { + fileService.saveToSharedFolder(file, "/test"); + + aesUtilsMockedStatic.verify(() -> AESUtils.encrypt(Mockito.anyString(),(InputStream) Mockito.any())); + aesUtilsMockedStatic.verifyNoMoreInteractions(); + filesMockedStatic.verify(() -> Files.createDirectories(Mockito.any())); + filesMockedStatic.verify(() -> Files.copy((InputStream) Mockito.any(),Mockito.any(), Mockito.any())); + filesMockedStatic.verifyNoMoreInteractions(); + } + } } diff --git a/src/test/java/it/gov/pagopa/pu/fileshare/service/ingestion/IngestionFlowFileServiceImplTest.java b/src/test/java/it/gov/pagopa/pu/fileshare/service/ingestion/IngestionFlowFileServiceImplTest.java index a921aaf..e060c83 100644 --- a/src/test/java/it/gov/pagopa/pu/fileshare/service/ingestion/IngestionFlowFileServiceImplTest.java +++ b/src/test/java/it/gov/pagopa/pu/fileshare/service/ingestion/IngestionFlowFileServiceImplTest.java @@ -24,10 +24,11 @@ class IngestionFlowFileServiceImplTest { private final String accessToken = "TOKEN"; private final long organizationId = 1L; private static final String VALID_FILE_EXTENSION = ".zip"; + private static final String INGESTION_FLOW_FILE_PATH = "/test"; @BeforeEach void setUp() { - ingestionFlowFileService = new IngestionFlowFileServiceImpl(userAuthorizationServiceMock, fileServiceMock,VALID_FILE_EXTENSION); + ingestionFlowFileService = new IngestionFlowFileServiceImpl(userAuthorizationServiceMock, fileServiceMock,VALID_FILE_EXTENSION,INGESTION_FLOW_FILE_PATH); } @Test @@ -44,6 +45,7 @@ void givenAuthorizedUserWhenUploadIngestionFlowFileThenOk(){ Mockito.verify(userAuthorizationServiceMock).checkUserAuthorization(organizationId,TestUtils.getSampleUser(),accessToken); Mockito.verify(fileServiceMock).validateFile(file,VALID_FILE_EXTENSION); + Mockito.verify(fileServiceMock).saveToSharedFolder(file,INGESTION_FLOW_FILE_PATH); } } From 2582a8b4c55dd1afb6fc2157d1bdf5dd44924c1f Mon Sep 17 00:00:00 2001 From: mscarsel Date: Mon, 13 Jan 2025 18:00:27 +0100 Subject: [PATCH 02/12] [P4ADEV-1830] added filename validation --- .../pu/fileshare/service/FileService.java | 12 +++++++++++- .../pu/fileshare/service/FileServiceTest.java | 17 +++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/main/java/it/gov/pagopa/pu/fileshare/service/FileService.java b/src/main/java/it/gov/pagopa/pu/fileshare/service/FileService.java index 9025f6e..c36235c 100644 --- a/src/main/java/it/gov/pagopa/pu/fileshare/service/FileService.java +++ b/src/main/java/it/gov/pagopa/pu/fileshare/service/FileService.java @@ -9,6 +9,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.StandardCopyOption; +import java.util.stream.Stream; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.StringUtils; import org.springframework.beans.factory.annotation.Value; @@ -28,10 +29,19 @@ public FileService(@Value("${folders.shared}") String sharedFolderRootPath, } public void validateFile(MultipartFile ingestionFlowFile, String validFileExt) { - if( ingestionFlowFile == null || !StringUtils.defaultString(ingestionFlowFile.getOriginalFilename()).endsWith(validFileExt)){ + if( ingestionFlowFile == null){ + log.debug("Invalid ingestion flow file"); + throw new InvalidFileException("Invalid file"); + } + String filename = StringUtils.defaultString(ingestionFlowFile.getOriginalFilename()); + if(!filename.endsWith(validFileExt)){ log.debug("Invalid ingestion flow file extension"); throw new InvalidFileException("Invalid file extension"); } + if(Stream.of("..", "\\", "/").anyMatch(filename::contains)){ + log.debug("Invalid ingestion flow filename"); + throw new InvalidFileException("Invalid filename"); + } } public void saveToSharedFolder(MultipartFile file, String relativePath){ diff --git a/src/test/java/it/gov/pagopa/pu/fileshare/service/FileServiceTest.java b/src/test/java/it/gov/pagopa/pu/fileshare/service/FileServiceTest.java index 5180fb3..16d59ff 100644 --- a/src/test/java/it/gov/pagopa/pu/fileshare/service/FileServiceTest.java +++ b/src/test/java/it/gov/pagopa/pu/fileshare/service/FileServiceTest.java @@ -56,6 +56,23 @@ void givenInvalidFileExtensionWhenValidateFileThenInvalidFileException(){ } } + @Test + void givenInvalidFilenameWhenValidateFileThenInvalidFileException(){ + MockMultipartFile file = new MockMultipartFile( + "ingestionFlowFile", + "../test.zip", + MediaType.TEXT_PLAIN_VALUE, + "this is a test file".getBytes() + ); + + try{ + fileService.validateFile(file, VALID_FILE_EXTENSION); + Assertions.fail("Expected InvalidFileException"); + }catch(InvalidFileException e){ + //do nothing + } + } + @Test void givenInvalidFileWhenSaveToSharedFolderThenIllegalStateException() { try (MockedStatic aesUtilsMockedStatic = Mockito.mockStatic( From 15e67cdd2333448410c921f82f364d15f4786220 Mon Sep 17 00:00:00 2001 From: mscarsel Date: Mon, 13 Jan 2025 18:11:19 +0100 Subject: [PATCH 03/12] [P4ADEV-1830] added filename validation to saveToSharedFolder --- .../pu/fileshare/service/FileService.java | 17 +++++++++--- .../pu/fileshare/service/FileServiceTest.java | 27 +++++++++++++++++-- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/src/main/java/it/gov/pagopa/pu/fileshare/service/FileService.java b/src/main/java/it/gov/pagopa/pu/fileshare/service/FileService.java index c36235c..d5d7505 100644 --- a/src/main/java/it/gov/pagopa/pu/fileshare/service/FileService.java +++ b/src/main/java/it/gov/pagopa/pu/fileshare/service/FileService.java @@ -34,16 +34,24 @@ public void validateFile(MultipartFile ingestionFlowFile, String validFileExt) { throw new InvalidFileException("Invalid file"); } String filename = StringUtils.defaultString(ingestionFlowFile.getOriginalFilename()); - if(!filename.endsWith(validFileExt)){ - log.debug("Invalid ingestion flow file extension"); - throw new InvalidFileException("Invalid file extension"); - } + validateFileExtension(validFileExt, filename); + validateFilename(filename); + } + + private static void validateFilename(String filename) { if(Stream.of("..", "\\", "/").anyMatch(filename::contains)){ log.debug("Invalid ingestion flow filename"); throw new InvalidFileException("Invalid filename"); } } + private static void validateFileExtension(String validFileExt, String filename) { + if(!filename.endsWith(validFileExt)){ + log.debug("Invalid ingestion flow file extension"); + throw new InvalidFileException("Invalid file extension"); + } + } + public void saveToSharedFolder(MultipartFile file, String relativePath){ if(file==null){ log.debug("File is mandatory"); @@ -51,6 +59,7 @@ public void saveToSharedFolder(MultipartFile file, String relativePath){ } String filename = org.springframework.util.StringUtils.cleanPath(StringUtils.defaultString(file.getOriginalFilename())); + validateFilename(filename); Path fileLocation = Paths.get(sharedFolderRootPath, relativePath, filename); //create missing parent folder, if any try { diff --git a/src/test/java/it/gov/pagopa/pu/fileshare/service/FileServiceTest.java b/src/test/java/it/gov/pagopa/pu/fileshare/service/FileServiceTest.java index 16d59ff..a0c4197 100644 --- a/src/test/java/it/gov/pagopa/pu/fileshare/service/FileServiceTest.java +++ b/src/test/java/it/gov/pagopa/pu/fileshare/service/FileServiceTest.java @@ -74,7 +74,7 @@ void givenInvalidFilenameWhenValidateFileThenInvalidFileException(){ } @Test - void givenInvalidFileWhenSaveToSharedFolderThenIllegalStateException() { + void givenInvalidFileWhenSaveToSharedFolderThenFileUploadException() { try (MockedStatic aesUtilsMockedStatic = Mockito.mockStatic( AESUtils.class); MockedStatic filesMockedStatic = Mockito.mockStatic( @@ -90,7 +90,30 @@ void givenInvalidFileWhenSaveToSharedFolderThenIllegalStateException() { } @Test - void givenErrorWhenSaveToSharedFolderThenIllegalStateException() { + void givenInvalidFilenameWhenSaveToSharedFolderThenInvalidFileException() { + MockMultipartFile file = new MockMultipartFile( + "ingestionFlowFile", + "../test.txt", + MediaType.TEXT_PLAIN_VALUE, + "this is a test file".getBytes() + ); + + try (MockedStatic aesUtilsMockedStatic = Mockito.mockStatic( + AESUtils.class); + MockedStatic filesMockedStatic = Mockito.mockStatic( + Files.class)) { + try { + fileService.saveToSharedFolder(file, ""); + Assertions.fail("Expected InvalidFileException"); + } catch (InvalidFileException e) { + aesUtilsMockedStatic.verifyNoInteractions(); + filesMockedStatic.verifyNoInteractions(); + } + } + } + + @Test + void givenErrorWhenSaveToSharedFolderThenFileUploadException() { MockMultipartFile file = new MockMultipartFile( "ingestionFlowFile", "test.txt", From 0a9f22b183eaddc8fd233046cdfb607480153a69 Mon Sep 17 00:00:00 2001 From: mscarsel Date: Mon, 13 Jan 2025 19:06:09 +0100 Subject: [PATCH 04/12] [P4ADEV-1830] fixed file path generation --- .../pu/fileshare/service/FileService.java | 12 ++++- .../pu/fileshare/service/FileServiceTest.java | 48 +++++++++++++++++-- 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/src/main/java/it/gov/pagopa/pu/fileshare/service/FileService.java b/src/main/java/it/gov/pagopa/pu/fileshare/service/FileService.java index d5d7505..e11544a 100644 --- a/src/main/java/it/gov/pagopa/pu/fileshare/service/FileService.java +++ b/src/main/java/it/gov/pagopa/pu/fileshare/service/FileService.java @@ -52,6 +52,16 @@ private static void validateFileExtension(String validFileExt, String filename) } } + private Path getFilePath(String relativePath, String filename) { + String basePath = sharedFolderRootPath+relativePath; + Path fileLocation = Paths.get(basePath,filename).normalize(); + if(!fileLocation.startsWith(basePath)){ + log.debug("Invalid file path"); + throw new InvalidFileException("Invalid file path"); + } + return fileLocation; + } + public void saveToSharedFolder(MultipartFile file, String relativePath){ if(file==null){ log.debug("File is mandatory"); @@ -60,7 +70,7 @@ public void saveToSharedFolder(MultipartFile file, String relativePath){ String filename = org.springframework.util.StringUtils.cleanPath(StringUtils.defaultString(file.getOriginalFilename())); validateFilename(filename); - Path fileLocation = Paths.get(sharedFolderRootPath, relativePath, filename); + Path fileLocation = getFilePath(relativePath, filename); //create missing parent folder, if any try { if (!fileLocation.toAbsolutePath().getParent().toFile().exists()) diff --git a/src/test/java/it/gov/pagopa/pu/fileshare/service/FileServiceTest.java b/src/test/java/it/gov/pagopa/pu/fileshare/service/FileServiceTest.java index a0c4197..9af6b00 100644 --- a/src/test/java/it/gov/pagopa/pu/fileshare/service/FileServiceTest.java +++ b/src/test/java/it/gov/pagopa/pu/fileshare/service/FileServiceTest.java @@ -5,6 +5,7 @@ import it.gov.pagopa.pu.fileshare.util.AESUtils; import java.io.InputStream; import java.nio.file.Files; +import java.nio.file.Paths; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -19,7 +20,7 @@ class FileServiceTest { private FileService fileService; private static final String VALID_FILE_EXTENSION = ".zip"; - private static final String SHARED_FOLDER_ROOT_PATH = "testPath"; + private static final String SHARED_FOLDER_ROOT_PATH = "/shared"; private static final String FILE_ENCRYPT_PASSWORD = "testPassword"; @BeforeEach @@ -39,6 +40,16 @@ void givenValidFileExtensionWhenValidateFileThenOk(){ fileService.validateFile(file, VALID_FILE_EXTENSION); } + @Test + void givenNoFileWhenValidateFileThenInvalidFileException(){ + try{ + fileService.validateFile(null, VALID_FILE_EXTENSION); + Assertions.fail("Expected InvalidFileException"); + }catch(InvalidFileException e){ + //do nothing + } + } + @Test void givenInvalidFileExtensionWhenValidateFileThenInvalidFileException(){ MockMultipartFile file = new MockMultipartFile( @@ -141,7 +152,7 @@ void givenErrorWhenSaveToSharedFolderThenFileUploadException() { } @Test - void givenValidFileWhenSaveToSharedFolderThenOK() { + void givenInvalidPathWhenSaveToSharedFolderThenInvalidFileException() { MockMultipartFile file = new MockMultipartFile( "ingestionFlowFile", "test.txt", @@ -149,17 +160,44 @@ void givenValidFileWhenSaveToSharedFolderThenOK() { "this is a test file".getBytes() ); + try (MockedStatic aesUtilsMockedStatic = Mockito.mockStatic( + AESUtils.class); + MockedStatic filesMockedStatic = Mockito.mockStatic( + Files.class)) { + + try { + fileService.saveToSharedFolder(file, "/../relative"); + Assertions.fail("Expected InvalidFileException"); + } catch (InvalidFileException e) { + aesUtilsMockedStatic.verifyNoInteractions(); + filesMockedStatic.verifyNoInteractions(); + } + } + } + + @Test + void givenValidFileWhenSaveToSharedFolderThenOK() { + String filename = "test.txt"; + MockMultipartFile file = new MockMultipartFile( + "ingestionFlowFile", + filename, + MediaType.TEXT_PLAIN_VALUE, + "this is a test file".getBytes() + ); + try (MockedStatic aesUtilsMockedStatic = Mockito.mockStatic( AESUtils.class); MockedStatic filesMockedStatic = Mockito.mockStatic( Files.class) ) { - fileService.saveToSharedFolder(file, "/test"); + String relativePath = "/relative"; + fileService.saveToSharedFolder(file, relativePath); aesUtilsMockedStatic.verify(() -> AESUtils.encrypt(Mockito.anyString(),(InputStream) Mockito.any())); aesUtilsMockedStatic.verifyNoMoreInteractions(); - filesMockedStatic.verify(() -> Files.createDirectories(Mockito.any())); - filesMockedStatic.verify(() -> Files.copy((InputStream) Mockito.any(),Mockito.any(), Mockito.any())); + filesMockedStatic.verify(() -> Files.createDirectories(Mockito.eq( + Paths.get(SHARED_FOLDER_ROOT_PATH,relativePath)))); + filesMockedStatic.verify(() -> Files.copy((InputStream) Mockito.any(),Mockito.eq(Paths.get(SHARED_FOLDER_ROOT_PATH,relativePath,filename)), Mockito.any())); filesMockedStatic.verifyNoMoreInteractions(); } } From d6c5f62a43af3704a99b6726cd9193f3040eca47 Mon Sep 17 00:00:00 2001 From: mscarsel Date: Tue, 14 Jan 2025 12:14:18 +0100 Subject: [PATCH 05/12] [P4ADEV-1829] Moved file save logics to FileStorerService --- helm/values.yaml | 3 + .../fileshare/config/FoldersPathsConfig.java | 29 ++++ .../pu/fileshare/service/FileService.java | 63 +------ .../fileshare/service/FileStorerService.java | 78 +++++++++ .../IngestionFlowFileServiceImpl.java | 16 +- src/main/resources/application.yml | 9 +- .../config/FoldersPathsConfigTest.java | 47 ++++++ .../pu/fileshare/service/FileServiceTest.java | 129 +------------- .../service/FileStorerServiceTest.java | 159 ++++++++++++++++++ .../IngestionFlowFileServiceImplTest.java | 14 +- 10 files changed, 348 insertions(+), 199 deletions(-) create mode 100644 src/main/java/it/gov/pagopa/pu/fileshare/config/FoldersPathsConfig.java create mode 100644 src/main/java/it/gov/pagopa/pu/fileshare/service/FileStorerService.java create mode 100644 src/test/java/it/gov/pagopa/pu/fileshare/config/FoldersPathsConfigTest.java create mode 100644 src/test/java/it/gov/pagopa/pu/fileshare/service/FileStorerServiceTest.java diff --git a/helm/values.yaml b/helm/values.yaml index f08ddc1..1d27d0d 100644 --- a/helm/values.yaml +++ b/helm/values.yaml @@ -76,9 +76,12 @@ microservice-chart: SHARED_FOLDER_ROOT: "/shared" + ORGANIZATION_BASE_URL: "http://p4pa-organization-microservice-chart:8080" + AUTH_SERVER_BASE_URL: "http://p4pa-auth-microservice-chart:8080/payhub" envSecret: APPLICATIONINSIGHTS_CONNECTION_STRING: appinsights-connection-string + FILE_ENCRYPT_PASSWORD: file-encrypt-password # nodeSelector: {} diff --git a/src/main/java/it/gov/pagopa/pu/fileshare/config/FoldersPathsConfig.java b/src/main/java/it/gov/pagopa/pu/fileshare/config/FoldersPathsConfig.java new file mode 100644 index 0000000..97dba72 --- /dev/null +++ b/src/main/java/it/gov/pagopa/pu/fileshare/config/FoldersPathsConfig.java @@ -0,0 +1,29 @@ +package it.gov.pagopa.pu.fileshare.config; + +import it.gov.pagopa.pu.fileshare.dto.generated.IngestionFlowFileType; +import java.util.Map; +import java.util.Optional; +import lombok.Getter; +import lombok.Setter; +import lombok.extern.slf4j.Slf4j; +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.context.annotation.Configuration; + +@Slf4j +@Getter +@Setter +@Configuration +@ConfigurationProperties(prefix = "folders") +public class FoldersPathsConfig { + private String shared; + private Map ingestionFlowFileTypePaths; + + public String getIngestionFlowFilePath(IngestionFlowFileType ingestionFlowFileType) { + return Optional.ofNullable( + ingestionFlowFileTypePaths.get(ingestionFlowFileType)) + .orElseThrow(()-> { + log.debug("No path configured for ingestionFlowFileType {}",ingestionFlowFileType); + return new UnsupportedOperationException(); + }); + } +} diff --git a/src/main/java/it/gov/pagopa/pu/fileshare/service/FileService.java b/src/main/java/it/gov/pagopa/pu/fileshare/service/FileService.java index e11544a..8dee37b 100644 --- a/src/main/java/it/gov/pagopa/pu/fileshare/service/FileService.java +++ b/src/main/java/it/gov/pagopa/pu/fileshare/service/FileService.java @@ -1,32 +1,15 @@ package it.gov.pagopa.pu.fileshare.service; -import it.gov.pagopa.pu.fileshare.exception.custom.FileUploadException; import it.gov.pagopa.pu.fileshare.exception.custom.InvalidFileException; -import it.gov.pagopa.pu.fileshare.util.AESUtils; -import java.io.IOException; -import java.io.InputStream; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.Paths; -import java.nio.file.StandardCopyOption; import java.util.stream.Stream; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.StringUtils; -import org.springframework.beans.factory.annotation.Value; import org.springframework.stereotype.Service; import org.springframework.web.multipart.MultipartFile; @Slf4j @Service public class FileService { - private final String sharedFolderRootPath; - private final String fileEncryptPassword; - - public FileService(@Value("${folders.shared}") String sharedFolderRootPath, - @Value(("${app.fileEncryptPassword}")) String fileEncryptPassword) { - this.sharedFolderRootPath = sharedFolderRootPath; - this.fileEncryptPassword = fileEncryptPassword; - } public void validateFile(MultipartFile ingestionFlowFile, String validFileExt) { if( ingestionFlowFile == null){ @@ -38,7 +21,7 @@ public void validateFile(MultipartFile ingestionFlowFile, String validFileExt) { validateFilename(filename); } - private static void validateFilename(String filename) { + public static void validateFilename(String filename) { if(Stream.of("..", "\\", "/").anyMatch(filename::contains)){ log.debug("Invalid ingestion flow filename"); throw new InvalidFileException("Invalid filename"); @@ -51,48 +34,4 @@ private static void validateFileExtension(String validFileExt, String filename) throw new InvalidFileException("Invalid file extension"); } } - - private Path getFilePath(String relativePath, String filename) { - String basePath = sharedFolderRootPath+relativePath; - Path fileLocation = Paths.get(basePath,filename).normalize(); - if(!fileLocation.startsWith(basePath)){ - log.debug("Invalid file path"); - throw new InvalidFileException("Invalid file path"); - } - return fileLocation; - } - - public void saveToSharedFolder(MultipartFile file, String relativePath){ - if(file==null){ - log.debug("File is mandatory"); - throw new FileUploadException("File is mandatory"); - } - - String filename = org.springframework.util.StringUtils.cleanPath(StringUtils.defaultString(file.getOriginalFilename())); - validateFilename(filename); - Path fileLocation = getFilePath(relativePath, filename); - //create missing parent folder, if any - try { - if (!fileLocation.toAbsolutePath().getParent().toFile().exists()) - Files.createDirectories(fileLocation.toAbsolutePath().getParent()); - encryptAndSaveFile(file, fileLocation); - }catch (Exception e) { - log.debug( - "Error uploading file to folder %s%s".formatted(sharedFolderRootPath, - relativePath), e); - throw new FileUploadException( - "Error uploading file to folder %s%s [%s]".formatted( - sharedFolderRootPath, relativePath, e.getMessage())); - } - log.debug("File upload to folder %s%s completed".formatted(sharedFolderRootPath, - relativePath)); - } - - private void encryptAndSaveFile(MultipartFile file, Path fileLocation) - throws IOException { - try(InputStream is = file.getInputStream(); - InputStream cipherIs = AESUtils.encrypt(fileEncryptPassword, is)){ - Files.copy(cipherIs, fileLocation, StandardCopyOption.REPLACE_EXISTING); - } - } } diff --git a/src/main/java/it/gov/pagopa/pu/fileshare/service/FileStorerService.java b/src/main/java/it/gov/pagopa/pu/fileshare/service/FileStorerService.java new file mode 100644 index 0000000..35795b2 --- /dev/null +++ b/src/main/java/it/gov/pagopa/pu/fileshare/service/FileStorerService.java @@ -0,0 +1,78 @@ +package it.gov.pagopa.pu.fileshare.service; + +import it.gov.pagopa.pu.fileshare.config.FoldersPathsConfig; +import it.gov.pagopa.pu.fileshare.exception.custom.FileUploadException; +import it.gov.pagopa.pu.fileshare.exception.custom.InvalidFileException; +import it.gov.pagopa.pu.fileshare.util.AESUtils; +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.StandardCopyOption; +import lombok.extern.slf4j.Slf4j; +import org.apache.commons.lang3.StringUtils; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.stereotype.Service; +import org.springframework.web.multipart.MultipartFile; + +@Slf4j +@Service +public class FileStorerService { + private final FoldersPathsConfig foldersPathsConfig; + private final String fileEncryptPassword; + + public FileStorerService(FoldersPathsConfig foldersPathsConfig, + @Value(("${app.fileEncryptPassword}")) String fileEncryptPassword) { + this.foldersPathsConfig = foldersPathsConfig; + this.fileEncryptPassword = fileEncryptPassword; + } + + private Path getFilePath(String relativePath, String filename) { + String basePath = foldersPathsConfig.getShared()+relativePath; + Path fileLocation = Paths.get(basePath,filename).normalize(); + if(!fileLocation.startsWith(basePath)){ + log.debug("Invalid file path"); + throw new InvalidFileException("Invalid file path"); + } + return fileLocation; + } + + public String saveToSharedFolder(MultipartFile file, String relativePath){ + if(file==null){ + log.debug("File is mandatory"); + throw new FileUploadException("File is mandatory"); + } + + String sharedFolderRootPath = foldersPathsConfig.getShared(); + String filename = org.springframework.util.StringUtils.cleanPath( + StringUtils.defaultString(file.getOriginalFilename())); + FileService.validateFilename(filename); + Path fileLocation = getFilePath(relativePath, filename); + //create missing parent folder, if any + try { + if (!Files.exists(fileLocation.getParent())) + Files.createDirectories(fileLocation.getParent()); + encryptAndSaveFile(file, fileLocation); + }catch (Exception e) { + String errorMessage = "Error uploading file to folder %s%s".formatted( + sharedFolderRootPath, + relativePath); + log.debug( + errorMessage, e); + throw new FileUploadException( + errorMessage); + } + log.debug("File upload to folder %s%s completed".formatted(sharedFolderRootPath, + relativePath)); + return Paths.get(relativePath,filename).toString(); + } + + private void encryptAndSaveFile(MultipartFile file, Path fileLocation) + throws IOException { + try(InputStream is = file.getInputStream(); + InputStream cipherIs = AESUtils.encrypt(fileEncryptPassword, is)){ + Files.copy(cipherIs, fileLocation, StandardCopyOption.REPLACE_EXISTING); + } + } +} diff --git a/src/main/java/it/gov/pagopa/pu/fileshare/service/ingestion/IngestionFlowFileServiceImpl.java b/src/main/java/it/gov/pagopa/pu/fileshare/service/ingestion/IngestionFlowFileServiceImpl.java index 9c88898..e1df49a 100644 --- a/src/main/java/it/gov/pagopa/pu/fileshare/service/ingestion/IngestionFlowFileServiceImpl.java +++ b/src/main/java/it/gov/pagopa/pu/fileshare/service/ingestion/IngestionFlowFileServiceImpl.java @@ -1,7 +1,9 @@ package it.gov.pagopa.pu.fileshare.service.ingestion; +import it.gov.pagopa.pu.fileshare.config.FoldersPathsConfig; import it.gov.pagopa.pu.fileshare.dto.generated.IngestionFlowFileType; import it.gov.pagopa.pu.fileshare.service.FileService; +import it.gov.pagopa.pu.fileshare.service.FileStorerService; import it.gov.pagopa.pu.fileshare.service.UserAuthorizationService; import it.gov.pagopa.pu.p4paauth.dto.generated.UserInfo; import lombok.extern.slf4j.Slf4j; @@ -14,24 +16,28 @@ public class IngestionFlowFileServiceImpl implements IngestionFlowFileService { private final UserAuthorizationService userAuthorizationService; private final FileService fileService; + private final FileStorerService fileStorerService; + private final FoldersPathsConfig foldersPathsConfig; private final String validIngestionFlowFileExt; - private final String ingestionFlowFilePath; public IngestionFlowFileServiceImpl( UserAuthorizationService userAuthorizationService, FileService fileService, - @Value("${uploads.ingestion-flow-file.valid-extension}") String validIngestionFlowFileExt, - @Value("${folders.ingestion-flow-file.path}") String ingestionFlowFilePath + FileStorerService fileStorerService, + FoldersPathsConfig foldersPathsConfig, + @Value("${uploads.ingestion-flow-file.valid-extension}") String validIngestionFlowFileExt ) { this.userAuthorizationService = userAuthorizationService; this.fileService = fileService; + this.fileStorerService = fileStorerService; + this.foldersPathsConfig = foldersPathsConfig; this.validIngestionFlowFileExt = validIngestionFlowFileExt; - this.ingestionFlowFilePath = ingestionFlowFilePath; } @Override public void uploadIngestionFlowFile(Long organizationId, IngestionFlowFileType ingestionFlowFileType, MultipartFile ingestionFlowFile, UserInfo user, String accessToken) { userAuthorizationService.checkUserAuthorization(organizationId, user, accessToken); fileService.validateFile(ingestionFlowFile, validIngestionFlowFileExt); - fileService.saveToSharedFolder(ingestionFlowFile,ingestionFlowFilePath); + String savedFilePath = fileStorerService.saveToSharedFolder(ingestionFlowFile, + foldersPathsConfig.getIngestionFlowFilePath(ingestionFlowFileType)); } } diff --git a/src/main/resources/application.yml b/src/main/resources/application.yml index 68b8998..a26487e 100644 --- a/src/main/resources/application.yml +++ b/src/main/resources/application.yml @@ -51,8 +51,13 @@ folders: process-target-sub-folders: archive: "\${PROCESS_TARGET_SUB_FOLDER_ARCHIVE:archive}" errors: "\${PROCESS_TARGET_SUB_FOLDER_ERRORS:errors}" - ingestion-flow-file: - path: "\${INGESTION_FLOW_FILE_PATH:/ingestion_flow_file}" + ingestion-flow-file-type-paths: + RECEIPT: "\${INGESTION_FLOW_FILE_RECEIPT_PATH:/receipt}" + PAYMENTS_REPORTING: "\${INGESTION_FLOW_FILE_PAYMENTS_REPORTING_PATH:/payments_reporting}" + OPI: "\${INGESTION_FLOW_FILE_OPI_PATH:/opi}" + TREASURY_CSV: "\${INGESTION_FLOW_FILE_TREASURY_CSV_PATH:/treasury_csv}" + TREASURY_XLS: "\${INGESTION_FLOW_FILE_TREASURY_XLS_PATH:/treasury_xls}" + TREASURY_POSTE: "\${INGESTION_FLOW_FILE_TREASURY_POSTE_PATH:/treasury_poste}" rest: default-timeout: diff --git a/src/test/java/it/gov/pagopa/pu/fileshare/config/FoldersPathsConfigTest.java b/src/test/java/it/gov/pagopa/pu/fileshare/config/FoldersPathsConfigTest.java new file mode 100644 index 0000000..596113d --- /dev/null +++ b/src/test/java/it/gov/pagopa/pu/fileshare/config/FoldersPathsConfigTest.java @@ -0,0 +1,47 @@ +package it.gov.pagopa.pu.fileshare.config; + +import it.gov.pagopa.pu.fileshare.dto.generated.IngestionFlowFileType; +import java.util.EnumMap; +import java.util.Map; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class FoldersPathsConfigTest { + private FoldersPathsConfig foldersPathsConfig; + + @BeforeEach + void setUp() { + foldersPathsConfig = new FoldersPathsConfig(); + + } + + @Test + void givenPopulatedPathWhenGetIngestionFlowFilePathThenOK(){ + String expected = "/receipt"; + Map paths = new EnumMap<>( + IngestionFlowFileType.class); + paths.put(IngestionFlowFileType.RECEIPT, "/receipt"); + foldersPathsConfig.setIngestionFlowFileTypePaths(paths); + + String result = foldersPathsConfig.getIngestionFlowFilePath( + IngestionFlowFileType.RECEIPT); + + Assertions.assertEquals(expected,result); + } + + @Test + void givenNoPathWhenGetIngestionFlowFilePathThenUnsupportedOperation(){ + foldersPathsConfig.setIngestionFlowFileTypePaths(new EnumMap<>(IngestionFlowFileType.class)); + try { + foldersPathsConfig.getIngestionFlowFilePath( + IngestionFlowFileType.RECEIPT); + Assertions.fail("Expected UnsupportedOperationException"); + }catch (UnsupportedOperationException e){ + //do nothing + } + } +} diff --git a/src/test/java/it/gov/pagopa/pu/fileshare/service/FileServiceTest.java b/src/test/java/it/gov/pagopa/pu/fileshare/service/FileServiceTest.java index 9af6b00..856b99c 100644 --- a/src/test/java/it/gov/pagopa/pu/fileshare/service/FileServiceTest.java +++ b/src/test/java/it/gov/pagopa/pu/fileshare/service/FileServiceTest.java @@ -1,17 +1,10 @@ package it.gov.pagopa.pu.fileshare.service; -import it.gov.pagopa.pu.fileshare.exception.custom.FileUploadException; import it.gov.pagopa.pu.fileshare.exception.custom.InvalidFileException; -import it.gov.pagopa.pu.fileshare.util.AESUtils; -import java.io.InputStream; -import java.nio.file.Files; -import java.nio.file.Paths; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.MockedStatic; -import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.http.MediaType; import org.springframework.mock.web.MockMultipartFile; @@ -20,12 +13,10 @@ class FileServiceTest { private FileService fileService; private static final String VALID_FILE_EXTENSION = ".zip"; - private static final String SHARED_FOLDER_ROOT_PATH = "/shared"; - private static final String FILE_ENCRYPT_PASSWORD = "testPassword"; @BeforeEach void setUp() { - fileService = new FileService(SHARED_FOLDER_ROOT_PATH,FILE_ENCRYPT_PASSWORD); + fileService = new FileService(); } @Test @@ -83,122 +74,4 @@ void givenInvalidFilenameWhenValidateFileThenInvalidFileException(){ //do nothing } } - - @Test - void givenInvalidFileWhenSaveToSharedFolderThenFileUploadException() { - try (MockedStatic aesUtilsMockedStatic = Mockito.mockStatic( - AESUtils.class); - MockedStatic filesMockedStatic = Mockito.mockStatic( - Files.class)) { - try { - fileService.saveToSharedFolder(null, ""); - Assertions.fail("Expected FileUploadException"); - } catch (FileUploadException e) { - aesUtilsMockedStatic.verifyNoInteractions(); - filesMockedStatic.verifyNoInteractions(); - } - } - } - - @Test - void givenInvalidFilenameWhenSaveToSharedFolderThenInvalidFileException() { - MockMultipartFile file = new MockMultipartFile( - "ingestionFlowFile", - "../test.txt", - MediaType.TEXT_PLAIN_VALUE, - "this is a test file".getBytes() - ); - - try (MockedStatic aesUtilsMockedStatic = Mockito.mockStatic( - AESUtils.class); - MockedStatic filesMockedStatic = Mockito.mockStatic( - Files.class)) { - try { - fileService.saveToSharedFolder(file, ""); - Assertions.fail("Expected InvalidFileException"); - } catch (InvalidFileException e) { - aesUtilsMockedStatic.verifyNoInteractions(); - filesMockedStatic.verifyNoInteractions(); - } - } - } - - @Test - void givenErrorWhenSaveToSharedFolderThenFileUploadException() { - MockMultipartFile file = new MockMultipartFile( - "ingestionFlowFile", - "test.txt", - MediaType.TEXT_PLAIN_VALUE, - "this is a test file".getBytes() - ); - - try (MockedStatic aesUtilsMockedStatic = Mockito.mockStatic( - AESUtils.class); - MockedStatic filesMockedStatic = Mockito.mockStatic( - Files.class)) { - Mockito.when(AESUtils.encrypt(Mockito.eq(FILE_ENCRYPT_PASSWORD), (InputStream) Mockito.any())) - .thenThrow(new RuntimeException()); - - try { - fileService.saveToSharedFolder(file, ""); - Assertions.fail("Expected FileUploadException"); - } catch (FileUploadException e) { - aesUtilsMockedStatic.verify(() -> AESUtils.encrypt(Mockito.anyString(),(InputStream) Mockito.any())); - aesUtilsMockedStatic.verifyNoMoreInteractions(); - filesMockedStatic.verify(() -> Files.createDirectories(Mockito.any())); - filesMockedStatic.verifyNoMoreInteractions(); - } - } - } - - @Test - void givenInvalidPathWhenSaveToSharedFolderThenInvalidFileException() { - MockMultipartFile file = new MockMultipartFile( - "ingestionFlowFile", - "test.txt", - MediaType.TEXT_PLAIN_VALUE, - "this is a test file".getBytes() - ); - - try (MockedStatic aesUtilsMockedStatic = Mockito.mockStatic( - AESUtils.class); - MockedStatic filesMockedStatic = Mockito.mockStatic( - Files.class)) { - - try { - fileService.saveToSharedFolder(file, "/../relative"); - Assertions.fail("Expected InvalidFileException"); - } catch (InvalidFileException e) { - aesUtilsMockedStatic.verifyNoInteractions(); - filesMockedStatic.verifyNoInteractions(); - } - } - } - - @Test - void givenValidFileWhenSaveToSharedFolderThenOK() { - String filename = "test.txt"; - MockMultipartFile file = new MockMultipartFile( - "ingestionFlowFile", - filename, - MediaType.TEXT_PLAIN_VALUE, - "this is a test file".getBytes() - ); - - try (MockedStatic aesUtilsMockedStatic = Mockito.mockStatic( - AESUtils.class); - MockedStatic filesMockedStatic = Mockito.mockStatic( - Files.class) - ) { - String relativePath = "/relative"; - fileService.saveToSharedFolder(file, relativePath); - - aesUtilsMockedStatic.verify(() -> AESUtils.encrypt(Mockito.anyString(),(InputStream) Mockito.any())); - aesUtilsMockedStatic.verifyNoMoreInteractions(); - filesMockedStatic.verify(() -> Files.createDirectories(Mockito.eq( - Paths.get(SHARED_FOLDER_ROOT_PATH,relativePath)))); - filesMockedStatic.verify(() -> Files.copy((InputStream) Mockito.any(),Mockito.eq(Paths.get(SHARED_FOLDER_ROOT_PATH,relativePath,filename)), Mockito.any())); - filesMockedStatic.verifyNoMoreInteractions(); - } - } } diff --git a/src/test/java/it/gov/pagopa/pu/fileshare/service/FileStorerServiceTest.java b/src/test/java/it/gov/pagopa/pu/fileshare/service/FileStorerServiceTest.java new file mode 100644 index 0000000..a9be115 --- /dev/null +++ b/src/test/java/it/gov/pagopa/pu/fileshare/service/FileStorerServiceTest.java @@ -0,0 +1,159 @@ +package it.gov.pagopa.pu.fileshare.service; + +import it.gov.pagopa.pu.fileshare.config.FoldersPathsConfig; +import it.gov.pagopa.pu.fileshare.exception.custom.FileUploadException; +import it.gov.pagopa.pu.fileshare.exception.custom.InvalidFileException; +import it.gov.pagopa.pu.fileshare.util.AESUtils; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Paths; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.http.MediaType; +import org.springframework.mock.web.MockMultipartFile; + +@ExtendWith(MockitoExtension.class) +class FileStorerServiceTest { + private FileStorerService fileStorerService; + @Mock + private FoldersPathsConfig foldersPathsConfig; + private static final String FILE_ENCRYPT_PASSWORD = "testPassword"; + + @BeforeEach + void setUp() { + fileStorerService = new FileStorerService(foldersPathsConfig,FILE_ENCRYPT_PASSWORD); + } + + @Test + void givenInvalidFileWhenSaveToSharedFolderThenFileUploadException() { + try (MockedStatic aesUtilsMockedStatic = Mockito.mockStatic( + AESUtils.class); + MockedStatic filesMockedStatic = Mockito.mockStatic( + Files.class)) { + try { + fileStorerService.saveToSharedFolder(null, ""); + Assertions.fail("Expected FileUploadException"); + } catch (FileUploadException e) { + aesUtilsMockedStatic.verifyNoInteractions(); + filesMockedStatic.verifyNoInteractions(); + } + } + } + + @Test + void givenInvalidFilenameWhenSaveToSharedFolderThenInvalidFileException() { + MockMultipartFile file = new MockMultipartFile( + "ingestionFlowFile", + "../test.txt", + MediaType.TEXT_PLAIN_VALUE, + "this is a test file".getBytes() + ); + + try (MockedStatic aesUtilsMockedStatic = Mockito.mockStatic( + AESUtils.class); + MockedStatic filesMockedStatic = Mockito.mockStatic( + Files.class)) { + try { + fileStorerService.saveToSharedFolder(file, ""); + Assertions.fail("Expected InvalidFileException"); + } catch (InvalidFileException e) { + aesUtilsMockedStatic.verifyNoInteractions(); + filesMockedStatic.verifyNoInteractions(); + } + } + } + + @Test + void givenErrorWhenSaveToSharedFolderThenFileUploadException() { + MockMultipartFile file = new MockMultipartFile( + "ingestionFlowFile", + "test.txt", + MediaType.TEXT_PLAIN_VALUE, + "this is a test file".getBytes() + ); + + try (MockedStatic aesUtilsMockedStatic = Mockito.mockStatic( + AESUtils.class); + MockedStatic filesMockedStatic = Mockito.mockStatic( + Files.class)) { + Mockito.when(AESUtils.encrypt(Mockito.eq(FILE_ENCRYPT_PASSWORD), (InputStream) Mockito.any())) + .thenThrow(new RuntimeException()); + + try { + fileStorerService.saveToSharedFolder(file, ""); + Assertions.fail("Expected FileUploadException"); + } catch (FileUploadException e) { + aesUtilsMockedStatic.verify(() -> AESUtils.encrypt(Mockito.anyString(),(InputStream) Mockito.any())); + aesUtilsMockedStatic.verifyNoMoreInteractions(); + filesMockedStatic.verify(() -> Files.exists(Mockito.any())); + filesMockedStatic.verify(() -> Files.createDirectories(Mockito.any())); + filesMockedStatic.verifyNoMoreInteractions(); + } + } + } + + @Test + void givenInvalidPathWhenSaveToSharedFolderThenInvalidFileException() { + MockMultipartFile file = new MockMultipartFile( + "ingestionFlowFile", + "test.txt", + MediaType.TEXT_PLAIN_VALUE, + "this is a test file".getBytes() + ); + + try (MockedStatic aesUtilsMockedStatic = Mockito.mockStatic( + AESUtils.class); + MockedStatic filesMockedStatic = Mockito.mockStatic( + Files.class)) { + String sharedFolderPath = "/shared"; + Mockito.when(foldersPathsConfig.getShared()).thenReturn(sharedFolderPath); + + try { + fileStorerService.saveToSharedFolder(file, "/relative/../../test"); + Assertions.fail("Expected InvalidFileException"); + } catch (InvalidFileException e) { + aesUtilsMockedStatic.verifyNoInteractions(); + filesMockedStatic.verifyNoInteractions(); + } + } + } + + @Test + void givenValidFileWhenSaveToSharedFolderThenOK() { + String filename = "test.txt"; + MockMultipartFile file = new MockMultipartFile( + "ingestionFlowFile", + filename, + MediaType.TEXT_PLAIN_VALUE, + "this is a test file".getBytes() + ); + + try (MockedStatic aesUtilsMockedStatic = Mockito.mockStatic( + AESUtils.class); + MockedStatic filesMockedStatic = Mockito.mockStatic( + Files.class) + ) { + String sharedFolderPath = "/shared"; + Mockito.when(foldersPathsConfig.getShared()).thenReturn(sharedFolderPath); + String relativePath = "/relative"; + + String result = fileStorerService.saveToSharedFolder(file, relativePath); + + Assertions.assertEquals(Paths.get(relativePath,filename).toString(),result); + aesUtilsMockedStatic.verify(() -> AESUtils.encrypt(Mockito.anyString(),(InputStream) Mockito.any())); + aesUtilsMockedStatic.verifyNoMoreInteractions(); + filesMockedStatic.verify(() -> Files.exists(Mockito.eq( + Paths.get(sharedFolderPath,relativePath)))); + filesMockedStatic.verify(() -> Files.createDirectories(Mockito.eq( + Paths.get(sharedFolderPath,relativePath)))); + filesMockedStatic.verify(() -> Files.copy((InputStream) Mockito.any(),Mockito.eq(Paths.get(sharedFolderPath,relativePath,filename)), Mockito.any())); + filesMockedStatic.verifyNoMoreInteractions(); + } + } +} diff --git a/src/test/java/it/gov/pagopa/pu/fileshare/service/ingestion/IngestionFlowFileServiceImplTest.java b/src/test/java/it/gov/pagopa/pu/fileshare/service/ingestion/IngestionFlowFileServiceImplTest.java index e060c83..5b3df94 100644 --- a/src/test/java/it/gov/pagopa/pu/fileshare/service/ingestion/IngestionFlowFileServiceImplTest.java +++ b/src/test/java/it/gov/pagopa/pu/fileshare/service/ingestion/IngestionFlowFileServiceImplTest.java @@ -1,7 +1,9 @@ package it.gov.pagopa.pu.fileshare.service.ingestion; +import it.gov.pagopa.pu.fileshare.config.FoldersPathsConfig; import it.gov.pagopa.pu.fileshare.dto.generated.IngestionFlowFileType; import it.gov.pagopa.pu.fileshare.service.FileService; +import it.gov.pagopa.pu.fileshare.service.FileStorerService; import it.gov.pagopa.pu.fileshare.service.UserAuthorizationService; import it.gov.pagopa.pu.fileshare.util.TestUtils; import org.junit.jupiter.api.BeforeEach; @@ -20,6 +22,10 @@ class IngestionFlowFileServiceImplTest { private UserAuthorizationService userAuthorizationServiceMock; @Mock private FileService fileServiceMock; + @Mock + private FileStorerService fileStorerServiceMock; + @Mock + private FoldersPathsConfig foldersPathsConfigMock; private IngestionFlowFileServiceImpl ingestionFlowFileService; private final String accessToken = "TOKEN"; private final long organizationId = 1L; @@ -28,7 +34,8 @@ class IngestionFlowFileServiceImplTest { @BeforeEach void setUp() { - ingestionFlowFileService = new IngestionFlowFileServiceImpl(userAuthorizationServiceMock, fileServiceMock,VALID_FILE_EXTENSION,INGESTION_FLOW_FILE_PATH); + ingestionFlowFileService = new IngestionFlowFileServiceImpl(userAuthorizationServiceMock,fileServiceMock,fileStorerServiceMock, + foldersPathsConfigMock,VALID_FILE_EXTENSION); } @Test @@ -39,13 +46,16 @@ void givenAuthorizedUserWhenUploadIngestionFlowFileThenOk(){ MediaType.TEXT_PLAIN_VALUE, "this is a test file".getBytes() ); + Mockito.when(foldersPathsConfigMock.getIngestionFlowFilePath(IngestionFlowFileType.RECEIPT)) + .thenReturn(INGESTION_FLOW_FILE_PATH); ingestionFlowFileService.uploadIngestionFlowFile(organizationId, IngestionFlowFileType.RECEIPT, file, TestUtils.getSampleUser(),accessToken); Mockito.verify(userAuthorizationServiceMock).checkUserAuthorization(organizationId,TestUtils.getSampleUser(),accessToken); Mockito.verify(fileServiceMock).validateFile(file,VALID_FILE_EXTENSION); - Mockito.verify(fileServiceMock).saveToSharedFolder(file,INGESTION_FLOW_FILE_PATH); + Mockito.verify(foldersPathsConfigMock).getIngestionFlowFilePath(Mockito.any()); + Mockito.verify(fileStorerServiceMock).saveToSharedFolder(file,INGESTION_FLOW_FILE_PATH); } } From 1f0a5d0143774f4f102fb707c459943c21924b04 Mon Sep 17 00:00:00 2001 From: mscarsel Date: Tue, 14 Jan 2025 12:33:57 +0100 Subject: [PATCH 06/12] [P4ADEV-1830] removed savedFilePath string variable --- .../service/ingestion/IngestionFlowFileServiceImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/it/gov/pagopa/pu/fileshare/service/ingestion/IngestionFlowFileServiceImpl.java b/src/main/java/it/gov/pagopa/pu/fileshare/service/ingestion/IngestionFlowFileServiceImpl.java index e1df49a..1c3de2b 100644 --- a/src/main/java/it/gov/pagopa/pu/fileshare/service/ingestion/IngestionFlowFileServiceImpl.java +++ b/src/main/java/it/gov/pagopa/pu/fileshare/service/ingestion/IngestionFlowFileServiceImpl.java @@ -37,7 +37,7 @@ public IngestionFlowFileServiceImpl( public void uploadIngestionFlowFile(Long organizationId, IngestionFlowFileType ingestionFlowFileType, MultipartFile ingestionFlowFile, UserInfo user, String accessToken) { userAuthorizationService.checkUserAuthorization(organizationId, user, accessToken); fileService.validateFile(ingestionFlowFile, validIngestionFlowFileExt); - String savedFilePath = fileStorerService.saveToSharedFolder(ingestionFlowFile, + fileStorerService.saveToSharedFolder(ingestionFlowFile, foldersPathsConfig.getIngestionFlowFilePath(ingestionFlowFileType)); } } From 0715167f8a391c171885e253f0ca0c1063ae15f0 Mon Sep 17 00:00:00 2001 From: mscarsel Date: Tue, 14 Jan 2025 16:26:00 +0100 Subject: [PATCH 07/12] [P4ADEV-1830] Removed path verification and fixed IngestionFlowFileType value --- openapi/p4pa-fileshare.openapi.yaml | 2 +- .../fileshare/service/FileStorerService.java | 34 +++++++------------ src/main/resources/application.yml | 12 +++---- .../service/FileStorerServiceTest.java | 34 +++++-------------- 4 files changed, 28 insertions(+), 54 deletions(-) diff --git a/openapi/p4pa-fileshare.openapi.yaml b/openapi/p4pa-fileshare.openapi.yaml index 1c44608..9b79845 100644 --- a/openapi/p4pa-fileshare.openapi.yaml +++ b/openapi/p4pa-fileshare.openapi.yaml @@ -56,7 +56,7 @@ components: enum: - RECEIPT - PAYMENTS_REPORTING - - OPI + - TREASURY_OPI - TREASURY_CSV - TREASURY_XLS - TREASURY_POSTE diff --git a/src/main/java/it/gov/pagopa/pu/fileshare/service/FileStorerService.java b/src/main/java/it/gov/pagopa/pu/fileshare/service/FileStorerService.java index 35795b2..b23967f 100644 --- a/src/main/java/it/gov/pagopa/pu/fileshare/service/FileStorerService.java +++ b/src/main/java/it/gov/pagopa/pu/fileshare/service/FileStorerService.java @@ -2,7 +2,6 @@ import it.gov.pagopa.pu.fileshare.config.FoldersPathsConfig; import it.gov.pagopa.pu.fileshare.exception.custom.FileUploadException; -import it.gov.pagopa.pu.fileshare.exception.custom.InvalidFileException; import it.gov.pagopa.pu.fileshare.util.AESUtils; import java.io.IOException; import java.io.InputStream; @@ -28,44 +27,37 @@ public FileStorerService(FoldersPathsConfig foldersPathsConfig, this.fileEncryptPassword = fileEncryptPassword; } - private Path getFilePath(String relativePath, String filename) { - String basePath = foldersPathsConfig.getShared()+relativePath; - Path fileLocation = Paths.get(basePath,filename).normalize(); - if(!fileLocation.startsWith(basePath)){ - log.debug("Invalid file path"); - throw new InvalidFileException("Invalid file path"); - } - return fileLocation; - } - public String saveToSharedFolder(MultipartFile file, String relativePath){ if(file==null){ log.debug("File is mandatory"); throw new FileUploadException("File is mandatory"); } - String sharedFolderRootPath = foldersPathsConfig.getShared(); String filename = org.springframework.util.StringUtils.cleanPath( StringUtils.defaultString(file.getOriginalFilename())); + Path fileLocation = Paths.get(relativePath,filename).normalize(); FileService.validateFilename(filename); - Path fileLocation = getFilePath(relativePath, filename); + Path absolutePath = getAbsolutePath(foldersPathsConfig.getShared(), fileLocation.toString()); //create missing parent folder, if any try { - if (!Files.exists(fileLocation.getParent())) - Files.createDirectories(fileLocation.getParent()); - encryptAndSaveFile(file, fileLocation); + if (!Files.exists(absolutePath.getParent())){ + Files.createDirectories(absolutePath.getParent()); + } + encryptAndSaveFile(file, absolutePath); }catch (Exception e) { - String errorMessage = "Error uploading file to folder %s%s".formatted( - sharedFolderRootPath, + String errorMessage = "Error uploading file to shared folder %s".formatted( relativePath); log.debug( errorMessage, e); throw new FileUploadException( errorMessage); } - log.debug("File upload to folder %s%s completed".formatted(sharedFolderRootPath, - relativePath)); - return Paths.get(relativePath,filename).toString(); + log.debug("File upload to shared folder {} completed",relativePath); + return fileLocation.toString(); + } + + private Path getAbsolutePath(String sharedFolder, String fileLocation) { + return Paths.get(sharedFolder,fileLocation).normalize(); } private void encryptAndSaveFile(MultipartFile file, Path fileLocation) diff --git a/src/main/resources/application.yml b/src/main/resources/application.yml index a26487e..ce8b6b2 100644 --- a/src/main/resources/application.yml +++ b/src/main/resources/application.yml @@ -52,12 +52,12 @@ folders: archive: "\${PROCESS_TARGET_SUB_FOLDER_ARCHIVE:archive}" errors: "\${PROCESS_TARGET_SUB_FOLDER_ERRORS:errors}" ingestion-flow-file-type-paths: - RECEIPT: "\${INGESTION_FLOW_FILE_RECEIPT_PATH:/receipt}" - PAYMENTS_REPORTING: "\${INGESTION_FLOW_FILE_PAYMENTS_REPORTING_PATH:/payments_reporting}" - OPI: "\${INGESTION_FLOW_FILE_OPI_PATH:/opi}" - TREASURY_CSV: "\${INGESTION_FLOW_FILE_TREASURY_CSV_PATH:/treasury_csv}" - TREASURY_XLS: "\${INGESTION_FLOW_FILE_TREASURY_XLS_PATH:/treasury_xls}" - TREASURY_POSTE: "\${INGESTION_FLOW_FILE_TREASURY_POSTE_PATH:/treasury_poste}" + RECEIPT: "data/receipt}" + PAYMENTS_REPORTING: "data/payments_reporting}" + TREASURY_OPI: "data/treasury/opi}" + TREASURY_CSV: "data/treasury/csv}" + TREASURY_XLS: "data/treasury/xls}" + TREASURY_POSTE: "data/treasury/poste}" rest: default-timeout: diff --git a/src/test/java/it/gov/pagopa/pu/fileshare/service/FileStorerServiceTest.java b/src/test/java/it/gov/pagopa/pu/fileshare/service/FileStorerServiceTest.java index a9be115..180f8c3 100644 --- a/src/test/java/it/gov/pagopa/pu/fileshare/service/FileStorerServiceTest.java +++ b/src/test/java/it/gov/pagopa/pu/fileshare/service/FileStorerServiceTest.java @@ -40,6 +40,7 @@ void givenInvalidFileWhenSaveToSharedFolderThenFileUploadException() { fileStorerService.saveToSharedFolder(null, ""); Assertions.fail("Expected FileUploadException"); } catch (FileUploadException e) { + Mockito.verifyNoInteractions(foldersPathsConfig); aesUtilsMockedStatic.verifyNoInteractions(); filesMockedStatic.verifyNoInteractions(); } @@ -63,6 +64,7 @@ void givenInvalidFilenameWhenSaveToSharedFolderThenInvalidFileException() { fileStorerService.saveToSharedFolder(file, ""); Assertions.fail("Expected InvalidFileException"); } catch (InvalidFileException e) { + Mockito.verifyNoInteractions(foldersPathsConfig); aesUtilsMockedStatic.verifyNoInteractions(); filesMockedStatic.verifyNoInteractions(); } @@ -82,6 +84,8 @@ void givenErrorWhenSaveToSharedFolderThenFileUploadException() { AESUtils.class); MockedStatic filesMockedStatic = Mockito.mockStatic( Files.class)) { + String sharedFolderPath = "/shared"; + Mockito.when(foldersPathsConfig.getShared()).thenReturn(sharedFolderPath); Mockito.when(AESUtils.encrypt(Mockito.eq(FILE_ENCRYPT_PASSWORD), (InputStream) Mockito.any())) .thenThrow(new RuntimeException()); @@ -89,6 +93,8 @@ void givenErrorWhenSaveToSharedFolderThenFileUploadException() { fileStorerService.saveToSharedFolder(file, ""); Assertions.fail("Expected FileUploadException"); } catch (FileUploadException e) { + Mockito.verify(foldersPathsConfig).getShared(); + Mockito.verifyNoMoreInteractions(foldersPathsConfig); aesUtilsMockedStatic.verify(() -> AESUtils.encrypt(Mockito.anyString(),(InputStream) Mockito.any())); aesUtilsMockedStatic.verifyNoMoreInteractions(); filesMockedStatic.verify(() -> Files.exists(Mockito.any())); @@ -98,32 +104,6 @@ void givenErrorWhenSaveToSharedFolderThenFileUploadException() { } } - @Test - void givenInvalidPathWhenSaveToSharedFolderThenInvalidFileException() { - MockMultipartFile file = new MockMultipartFile( - "ingestionFlowFile", - "test.txt", - MediaType.TEXT_PLAIN_VALUE, - "this is a test file".getBytes() - ); - - try (MockedStatic aesUtilsMockedStatic = Mockito.mockStatic( - AESUtils.class); - MockedStatic filesMockedStatic = Mockito.mockStatic( - Files.class)) { - String sharedFolderPath = "/shared"; - Mockito.when(foldersPathsConfig.getShared()).thenReturn(sharedFolderPath); - - try { - fileStorerService.saveToSharedFolder(file, "/relative/../../test"); - Assertions.fail("Expected InvalidFileException"); - } catch (InvalidFileException e) { - aesUtilsMockedStatic.verifyNoInteractions(); - filesMockedStatic.verifyNoInteractions(); - } - } - } - @Test void givenValidFileWhenSaveToSharedFolderThenOK() { String filename = "test.txt"; @@ -146,6 +126,8 @@ void givenValidFileWhenSaveToSharedFolderThenOK() { String result = fileStorerService.saveToSharedFolder(file, relativePath); Assertions.assertEquals(Paths.get(relativePath,filename).toString(),result); + Mockito.verify(foldersPathsConfig).getShared(); + Mockito.verifyNoMoreInteractions(foldersPathsConfig); aesUtilsMockedStatic.verify(() -> AESUtils.encrypt(Mockito.anyString(),(InputStream) Mockito.any())); aesUtilsMockedStatic.verifyNoMoreInteractions(); filesMockedStatic.verify(() -> Files.exists(Mockito.eq( From 395fc3372650744850757c5955fae4f7264b4811 Mon Sep 17 00:00:00 2001 From: mscarsel Date: Tue, 14 Jan 2025 17:15:00 +0100 Subject: [PATCH 08/12] [P4ADEV-1830] fix --- .../it/gov/pagopa/pu/fileshare/service/FileStorerService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/it/gov/pagopa/pu/fileshare/service/FileStorerService.java b/src/main/java/it/gov/pagopa/pu/fileshare/service/FileStorerService.java index b23967f..0d5d6be 100644 --- a/src/main/java/it/gov/pagopa/pu/fileshare/service/FileStorerService.java +++ b/src/main/java/it/gov/pagopa/pu/fileshare/service/FileStorerService.java @@ -35,8 +35,8 @@ public String saveToSharedFolder(MultipartFile file, String relativePath){ String filename = org.springframework.util.StringUtils.cleanPath( StringUtils.defaultString(file.getOriginalFilename())); - Path fileLocation = Paths.get(relativePath,filename).normalize(); FileService.validateFilename(filename); + Path fileLocation = Paths.get(relativePath,filename).normalize(); Path absolutePath = getAbsolutePath(foldersPathsConfig.getShared(), fileLocation.toString()); //create missing parent folder, if any try { From ebf14776a14928006fd402d8a05cd1424deb90bb Mon Sep 17 00:00:00 2001 From: mscarsel Date: Tue, 14 Jan 2025 17:26:46 +0100 Subject: [PATCH 09/12] [P4ADEV-1830] fix --- .../pu/fileshare/service/FileStorerService.java | 14 ++++++++++---- .../fileshare/service/FileStorerServiceTest.java | 2 +- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/main/java/it/gov/pagopa/pu/fileshare/service/FileStorerService.java b/src/main/java/it/gov/pagopa/pu/fileshare/service/FileStorerService.java index 0d5d6be..562cda9 100644 --- a/src/main/java/it/gov/pagopa/pu/fileshare/service/FileStorerService.java +++ b/src/main/java/it/gov/pagopa/pu/fileshare/service/FileStorerService.java @@ -2,6 +2,7 @@ import it.gov.pagopa.pu.fileshare.config.FoldersPathsConfig; import it.gov.pagopa.pu.fileshare.exception.custom.FileUploadException; +import it.gov.pagopa.pu.fileshare.exception.custom.InvalidFileException; import it.gov.pagopa.pu.fileshare.util.AESUtils; import java.io.IOException; import java.io.InputStream; @@ -36,8 +37,8 @@ public String saveToSharedFolder(MultipartFile file, String relativePath){ String filename = org.springframework.util.StringUtils.cleanPath( StringUtils.defaultString(file.getOriginalFilename())); FileService.validateFilename(filename); - Path fileLocation = Paths.get(relativePath,filename).normalize(); - Path absolutePath = getAbsolutePath(foldersPathsConfig.getShared(), fileLocation.toString()); + Path fileLocation = concatenatePaths(relativePath,filename); + Path absolutePath = concatenatePaths(foldersPathsConfig.getShared(), fileLocation.toString()); //create missing parent folder, if any try { if (!Files.exists(absolutePath.getParent())){ @@ -56,8 +57,13 @@ public String saveToSharedFolder(MultipartFile file, String relativePath){ return fileLocation.toString(); } - private Path getAbsolutePath(String sharedFolder, String fileLocation) { - return Paths.get(sharedFolder,fileLocation).normalize(); + private Path concatenatePaths(String firstPath, String secondPath) { + Path concatenatedPath = Paths.get(firstPath, secondPath).normalize(); + if(!concatenatedPath.startsWith(firstPath)){ + log.debug("Invalid file path"); + throw new InvalidFileException("Invalid file path"); + } + return concatenatedPath; } private void encryptAndSaveFile(MultipartFile file, Path fileLocation) diff --git a/src/test/java/it/gov/pagopa/pu/fileshare/service/FileStorerServiceTest.java b/src/test/java/it/gov/pagopa/pu/fileshare/service/FileStorerServiceTest.java index 180f8c3..ae15c02 100644 --- a/src/test/java/it/gov/pagopa/pu/fileshare/service/FileStorerServiceTest.java +++ b/src/test/java/it/gov/pagopa/pu/fileshare/service/FileStorerServiceTest.java @@ -90,7 +90,7 @@ void givenErrorWhenSaveToSharedFolderThenFileUploadException() { .thenThrow(new RuntimeException()); try { - fileStorerService.saveToSharedFolder(file, ""); + fileStorerService.saveToSharedFolder(file, "/relative"); Assertions.fail("Expected FileUploadException"); } catch (FileUploadException e) { Mockito.verify(foldersPathsConfig).getShared(); From a3f966699179d30d3b219571b67bacd75d086bf6 Mon Sep 17 00:00:00 2001 From: mscarsel Date: Tue, 14 Jan 2025 18:14:08 +0100 Subject: [PATCH 10/12] [P4ADEV-1830] fix --- .../service/FileStorerServiceTest.java | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/test/java/it/gov/pagopa/pu/fileshare/service/FileStorerServiceTest.java b/src/test/java/it/gov/pagopa/pu/fileshare/service/FileStorerServiceTest.java index ae15c02..220ac4d 100644 --- a/src/test/java/it/gov/pagopa/pu/fileshare/service/FileStorerServiceTest.java +++ b/src/test/java/it/gov/pagopa/pu/fileshare/service/FileStorerServiceTest.java @@ -138,4 +138,28 @@ void givenValidFileWhenSaveToSharedFolderThenOK() { filesMockedStatic.verifyNoMoreInteractions(); } } + + @Test + void givenInvalidPathWhenSaveToSharedFolderThenInvalidFileException() { + MockMultipartFile file = new MockMultipartFile( + "ingestionFlowFile", + "test.txt", + MediaType.TEXT_PLAIN_VALUE, + "this is a test file".getBytes() + ); + + try (MockedStatic aesUtilsMockedStatic = Mockito.mockStatic( + AESUtils.class); + MockedStatic filesMockedStatic = Mockito.mockStatic( + Files.class)) { + try { + fileStorerService.saveToSharedFolder(file, "/relative/../../test"); + Assertions.fail("Expected InvalidFileException"); + } catch (InvalidFileException e) { + Mockito.verifyNoInteractions(foldersPathsConfig); + aesUtilsMockedStatic.verifyNoInteractions(); + filesMockedStatic.verifyNoInteractions(); + } + } + } } From 08d5f09e2b6ff39dccc069676214bd84b2a9284a Mon Sep 17 00:00:00 2001 From: mscarsel Date: Tue, 14 Jan 2025 18:36:59 +0100 Subject: [PATCH 11/12] [P4ADEV-1830] fix --- .../exception/custom/FileUploadException.java | 4 ++++ .../pu/fileshare/service/FileStorerService.java | 17 +++++++++-------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/main/java/it/gov/pagopa/pu/fileshare/exception/custom/FileUploadException.java b/src/main/java/it/gov/pagopa/pu/fileshare/exception/custom/FileUploadException.java index 67b2bdb..098a68f 100644 --- a/src/main/java/it/gov/pagopa/pu/fileshare/exception/custom/FileUploadException.java +++ b/src/main/java/it/gov/pagopa/pu/fileshare/exception/custom/FileUploadException.java @@ -4,4 +4,8 @@ public class FileUploadException extends RuntimeException { public FileUploadException(String message) { super(message); } + + public FileUploadException(String message, Throwable e) { + super(message,e); + } } diff --git a/src/main/java/it/gov/pagopa/pu/fileshare/service/FileStorerService.java b/src/main/java/it/gov/pagopa/pu/fileshare/service/FileStorerService.java index 562cda9..0bd7704 100644 --- a/src/main/java/it/gov/pagopa/pu/fileshare/service/FileStorerService.java +++ b/src/main/java/it/gov/pagopa/pu/fileshare/service/FileStorerService.java @@ -37,8 +37,8 @@ public String saveToSharedFolder(MultipartFile file, String relativePath){ String filename = org.springframework.util.StringUtils.cleanPath( StringUtils.defaultString(file.getOriginalFilename())); FileService.validateFilename(filename); - Path fileLocation = concatenatePaths(relativePath,filename); - Path absolutePath = concatenatePaths(foldersPathsConfig.getShared(), fileLocation.toString()); + Path relativeFileLocation = concatenatePaths(relativePath,filename); + Path absolutePath = concatenatePaths(foldersPathsConfig.getShared(), relativeFileLocation.toString()); //create missing parent folder, if any try { if (!Files.exists(absolutePath.getParent())){ @@ -46,17 +46,18 @@ public String saveToSharedFolder(MultipartFile file, String relativePath){ } encryptAndSaveFile(file, absolutePath); }catch (Exception e) { - String errorMessage = "Error uploading file to shared folder %s".formatted( - relativePath); - log.debug( - errorMessage, e); throw new FileUploadException( - errorMessage); + "Error uploading file to shared folder %s".formatted(relativePath) + ,e); } log.debug("File upload to shared folder {} completed",relativePath); - return fileLocation.toString(); + return relativeFileLocation.toString(); } + /** + * This method expects two paths whose concatenation does not resolve into an outer folder. + * The normalized path still starts with the first path. + * */ private Path concatenatePaths(String firstPath, String secondPath) { Path concatenatedPath = Paths.get(firstPath, secondPath).normalize(); if(!concatenatedPath.startsWith(firstPath)){ From 9aac23dbfb233594c741bce404b93b95c243b696 Mon Sep 17 00:00:00 2001 From: mscarsel Date: Tue, 14 Jan 2025 18:50:30 +0100 Subject: [PATCH 12/12] [P4ADEV-1830] fix --- src/main/resources/application.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/resources/application.yml b/src/main/resources/application.yml index ce8b6b2..3e4d4d9 100644 --- a/src/main/resources/application.yml +++ b/src/main/resources/application.yml @@ -52,12 +52,12 @@ folders: archive: "\${PROCESS_TARGET_SUB_FOLDER_ARCHIVE:archive}" errors: "\${PROCESS_TARGET_SUB_FOLDER_ERRORS:errors}" ingestion-flow-file-type-paths: - RECEIPT: "data/receipt}" - PAYMENTS_REPORTING: "data/payments_reporting}" - TREASURY_OPI: "data/treasury/opi}" - TREASURY_CSV: "data/treasury/csv}" - TREASURY_XLS: "data/treasury/xls}" - TREASURY_POSTE: "data/treasury/poste}" + RECEIPT: "data/receipt" + PAYMENTS_REPORTING: "data/payments_reporting" + TREASURY_OPI: "data/treasury/opi" + TREASURY_CSV: "data/treasury/csv" + TREASURY_XLS: "data/treasury/xls" + TREASURY_POSTE: "data/treasury/poste" rest: default-timeout: