From b17698aadfb3d10cbc12b0588beae24249607518 Mon Sep 17 00:00:00 2001 From: devinleighsmith <41091511+devinleighsmith@users.noreply.github.com> Date: Wed, 8 Jan 2025 23:31:50 -0800 Subject: [PATCH] HOTFIX: psp-9813, psp-9814 (#4578) * psp-9813 * psp-9813 test updates. * psp-9814 pims IS96 database fails to build. * increment hotfix version. --------- Co-authored-by: Smith --- source/backend/api/Pims.Api.csproj | 4 +- .../api/Services/DocumentFileService.cs | 23 ++-- .../backend/api/Services/DocumentService.cs | 55 +++------- source/backend/tests/core/TestHelper.cs | 3 +- .../api/Services/DocumentFileServiceTest.cs | 102 ++++++++++++++++++ .../unit/api/Services/DocumentServiceTest.cs | 34 ++++-- .../Build/075_DML_PIMS_ROLE_CLAIM.sql | 4 +- source/frontend/package.json | 2 +- 8 files changed, 166 insertions(+), 61 deletions(-) diff --git a/source/backend/api/Pims.Api.csproj b/source/backend/api/Pims.Api.csproj index c9dfb0da33..418d59ea12 100644 --- a/source/backend/api/Pims.Api.csproj +++ b/source/backend/api/Pims.Api.csproj @@ -2,8 +2,8 @@ 0ef6255f-9ea0-49ec-8c65-c172304b4926 - 5.7.0-96.22 - 5.7.0.96 + 5.7.1-96.22 + 5.7.1.96 true {16BC0468-78F6-4C91-87DA-7403C919E646} net8.0 diff --git a/source/backend/api/Services/DocumentFileService.cs b/source/backend/api/Services/DocumentFileService.cs index 55dd68affa..b6a959c1ed 100644 --- a/source/backend/api/Services/DocumentFileService.cs +++ b/source/backend/api/Services/DocumentFileService.cs @@ -13,6 +13,7 @@ using Pims.Api.Models.Requests.Http; using Pims.Core.Api.Exceptions; using Pims.Core.Api.Services; +using Pims.Core.Exceptions; using Pims.Core.Extensions; using Pims.Core.Security; using Pims.Dal.Entities; @@ -95,7 +96,7 @@ public async Task UploadAcquisitionDocument(long acquisitionFileId, DocumentUplo { Logger.LogInformation("Uploading document for single acquisition file"); User.ThrowIfNotAllAuthorized(Permissions.DocumentAdd, Permissions.AcquisitionFileEdit); - uploadRequest.ThrowInvalidFileSize(); + ValidateDocumentUpload(uploadRequest); PimsDocument pimsDocument = CreatePimsDocument(uploadRequest); _documentQueueRepository.SaveChanges(); @@ -117,7 +118,7 @@ public async Task UploadResearchDocument(long researchFileId, DocumentUploadRequ { Logger.LogInformation("Uploading document for single research file"); User.ThrowIfNotAllAuthorized(Permissions.DocumentAdd, Permissions.ResearchFileEdit); - uploadRequest.ThrowInvalidFileSize(); + ValidateDocumentUpload(uploadRequest); PimsDocument pimsDocument = CreatePimsDocument(uploadRequest); _documentQueueRepository.SaveChanges(); @@ -139,7 +140,7 @@ public async Task UploadProjectDocument(long projectId, DocumentUploadRequest up { Logger.LogInformation("Uploading document for single Project"); User.ThrowIfNotAllAuthorized(Permissions.DocumentAdd, Permissions.ProjectEdit); - uploadRequest.ThrowInvalidFileSize(); + ValidateDocumentUpload(uploadRequest); PimsDocument pimsDocument = CreatePimsDocument(uploadRequest); _documentQueueRepository.SaveChanges(); @@ -161,7 +162,7 @@ public async Task UploadLeaseDocument(long leaseId, DocumentUploadRequest upload { Logger.LogInformation("Uploading document for single Lease"); User.ThrowIfNotAllAuthorized(Permissions.DocumentAdd, Permissions.LeaseEdit); - uploadRequest.ThrowInvalidFileSize(); + ValidateDocumentUpload(uploadRequest); PimsDocument pimsDocument = CreatePimsDocument(uploadRequest); _documentQueueRepository.SaveChanges(); @@ -183,7 +184,7 @@ public async Task UploadPropertyActivityDocument(long propertyActivityId, Docume { Logger.LogInformation("Uploading document for single Property Activity"); User.ThrowIfNotAllAuthorized(Permissions.DocumentAdd, Permissions.ManagementEdit); - uploadRequest.ThrowInvalidFileSize(); + ValidateDocumentUpload(uploadRequest); PimsDocument pimsDocument = CreatePimsDocument(uploadRequest); _documentQueueRepository.SaveChanges(); @@ -205,7 +206,7 @@ public async Task UploadDispositionDocument(long dispositionFileId, DocumentUplo { Logger.LogInformation("Uploading document for single disposition file"); User.ThrowIfNotAllAuthorized(Permissions.DocumentAdd, Permissions.DispositionEdit); - uploadRequest.ThrowInvalidFileSize(); + ValidateDocumentUpload(uploadRequest); PimsDocument pimsDocument = CreatePimsDocument(uploadRequest); _documentQueueRepository.SaveChanges(); @@ -392,6 +393,16 @@ public async Task> DeleteDispositionDocumentAsync(PimsD return result; } + private static void ValidateDocumentUpload(DocumentUploadRequest uploadRequest) + { + uploadRequest.ThrowInvalidFileSize(); + + if (!DocumentService.IsValidDocumentExtension(uploadRequest.File.FileName)) + { + throw new BusinessRuleViolationException($"This file has an invalid file extension."); + } + } + private PimsDocument CreatePimsDocument(DocumentUploadRequest uploadRequest, string documentExternalId = null) { // Create the pims document diff --git a/source/backend/api/Services/DocumentService.cs b/source/backend/api/Services/DocumentService.cs index 2407d796ed..e458d594ff 100644 --- a/source/backend/api/Services/DocumentService.cs +++ b/source/backend/api/Services/DocumentService.cs @@ -490,22 +490,13 @@ public async Task> DownloadFileAsync(long this.User.ThrowIfNotAuthorized(Permissions.DocumentView); ExternalResponse downloadResult = await documentStorageRepository.TryDownloadFileAsync(mayanDocumentId, mayanFileId); - if (IsValidDocumentExtension(downloadResult.Payload.FileName)) - { - if (downloadResult.Status != ExternalResponseStatus.Success) - { - throw GetMayanResponseError(downloadResult.Message); - } - return downloadResult; - } - else + + // because we bypass file extension checks from legacy systems, we should not check the extension here. + if (downloadResult.Status != ExternalResponseStatus.Success) { - return new ExternalResponse() - { - Status = ExternalResponseStatus.Error, - Message = $"Document with id ${mayanDocumentId} has an invalid extension", - }; + throw GetMayanResponseError(downloadResult.Message); } + return downloadResult; } public async Task> StreamFileAsync(long mayanDocumentId, long mayanFileId) @@ -514,22 +505,13 @@ public async Task> StreamFileAsync(long may this.User.ThrowIfNotAuthorized(Permissions.DocumentView); ExternalResponse downloadResult = await documentStorageRepository.TryStreamFileAsync(mayanDocumentId, mayanFileId); - if (IsValidDocumentExtension(downloadResult.Payload.FileName)) + + // because we bypass file extension checks from legacy systems, we should not check the extension here. + if (downloadResult.Status != ExternalResponseStatus.Success) { - if (downloadResult.Status != ExternalResponseStatus.Success) - { - throw GetMayanResponseError(downloadResult.Message); - } - return downloadResult; - } - else - { - return new ExternalResponse() - { - Status = ExternalResponseStatus.Error, - Message = $"Document with id ${mayanDocumentId} has an invalid extension", - }; + throw GetMayanResponseError(downloadResult.Message); } + return downloadResult; } public async Task> DownloadFileLatestAsync(long mayanDocumentId) @@ -541,19 +523,8 @@ public async Task> DownloadFileLatestAsyn { if (documentResult.Payload != null) { - if (IsValidDocumentExtension(documentResult.Payload.FileLatest.FileName)) - { - ExternalResponse downloadResult = await documentStorageRepository.TryDownloadFileAsync(documentResult.Payload.Id, documentResult.Payload.FileLatest.Id); - return downloadResult; - } - else - { - return new ExternalResponse() - { - Status = ExternalResponseStatus.Error, - Message = $"Document with id ${mayanDocumentId} has an invalid extension", - }; - } + ExternalResponse downloadResult = await documentStorageRepository.TryDownloadFileAsync(documentResult.Payload.Id, documentResult.Payload.FileLatest.Id); + return downloadResult; } else { @@ -639,7 +610,7 @@ public async Task DownloadFilePageImageAsync(long mayanDocu return result; } - private static bool IsValidDocumentExtension(string fileName) + public static bool IsValidDocumentExtension(string fileName) { var fileNameExtension = Path.GetExtension(fileName).Replace(".", string.Empty).ToLower(); return ValidExtensions.Contains(fileNameExtension); diff --git a/source/backend/tests/core/TestHelper.cs b/source/backend/tests/core/TestHelper.cs index 575ded3176..06cc1bef94 100644 --- a/source/backend/tests/core/TestHelper.cs +++ b/source/backend/tests/core/TestHelper.cs @@ -222,10 +222,9 @@ public T CreateInstance() return (T)ActivatorUtilities.CreateInstance(this.Provider, typeof(T)); } - public IFormFile GetFormFile(string text) + public IFormFile GetFormFile(string text, string fileName = "test.pdf") { // Setup mock file using a memory stream - var fileName = "test.pdf"; var stream = new MemoryStream(); var writer = new StreamWriter(stream); writer.Write(text); diff --git a/source/backend/tests/unit/api/Services/DocumentFileServiceTest.cs b/source/backend/tests/unit/api/Services/DocumentFileServiceTest.cs index 245f44f232..0bd7a39a19 100644 --- a/source/backend/tests/unit/api/Services/DocumentFileServiceTest.cs +++ b/source/backend/tests/unit/api/Services/DocumentFileServiceTest.cs @@ -297,6 +297,108 @@ public void UploadDocument_Disposition_ShouldThrowException_NotAuthorized() documentService.Verify(x => x.UploadDocumentAsync(It.IsAny(), false), Times.Never); } + [Fact] + public void UploadDocument_Research_ShouldThrowException_InvalidExtension() + { + // Arrange + var service = this.CreateDocumentFileServiceWithPermissions(Permissions.DocumentAdd); + var documentService = this._helper.GetService>(); + + DocumentUploadRequest uploadRequest = new() { DocumentTypeId = 1, File = this._helper.GetFormFile("Lorem Ipsum", "test.exe") }; + + // Assert + Func action = async () => await service.UploadResearchDocument(1, uploadRequest); + + // Assert + action.Should().ThrowAsync().WithMessage("This file has an invalid file extension."); + documentService.Verify(x => x.UploadDocumentAsync(It.IsAny(), false), Times.Never); + } + + [Fact] + public void UploadDocument_Acquisition_ShouldThrowException_InvalidExtension() + { + // Arrange + var service = this.CreateDocumentFileServiceWithPermissions(Permissions.DocumentAdd); + var documentService = this._helper.GetService>(); + + DocumentUploadRequest uploadRequest = new() { DocumentTypeId = 1, File = this._helper.GetFormFile("Lorem Ipsum", "test.exe") }; + + // Assert + Func action = async () => await service.UploadAcquisitionDocument(1, uploadRequest); + + // Assert + action.Should().ThrowAsync().WithMessage("This file has an invalid file extension."); + documentService.Verify(x => x.UploadDocumentAsync(It.IsAny(), false), Times.Never); + } + + [Fact] + public void UploadDocument_Project_ShouldThrowException_InvalidExtension() + { + // Arrange + var service = this.CreateDocumentFileServiceWithPermissions(Permissions.DocumentAdd); + var documentService = this._helper.GetService>(); + + DocumentUploadRequest uploadRequest = new() { DocumentTypeId = 1, File = this._helper.GetFormFile("Lorem Ipsum", "test.exe") }; + + // Assert + Func action = async () => await service.UploadProjectDocument(1, uploadRequest); + + // Assert + action.Should().ThrowAsync().WithMessage("This file has an invalid file extension."); + documentService.Verify(x => x.UploadDocumentAsync(It.IsAny(), false), Times.Never); + } + + [Fact] + public void UploadDocument_Lease_ShouldThrowException_InvalidExtension() + { + // Arrange + var service = this.CreateDocumentFileServiceWithPermissions(Permissions.DocumentAdd); + var documentService = this._helper.GetService>(); + + DocumentUploadRequest uploadRequest = new() { DocumentTypeId = 1, File = this._helper.GetFormFile("Lorem Ipsum", "test.exe") }; + + // Assert + Func action = async () => await service.UploadLeaseDocument(1, uploadRequest); + + // Assert + action.Should().ThrowAsync().WithMessage("This file has an invalid file extension."); + documentService.Verify(x => x.UploadDocumentAsync(It.IsAny(), false), Times.Never); + } + + [Fact] + public void UploadDocument_PropertyActivity_ShouldThrowException_InvalidExtension() + { + // Arrange + var service = this.CreateDocumentFileServiceWithPermissions(Permissions.DocumentAdd); + var documentService = this._helper.GetService>(); + + DocumentUploadRequest uploadRequest = new() { DocumentTypeId = 1, File = this._helper.GetFormFile("Lorem Ipsum", "test.exe") }; + + // Assert + Func action = async () => await service.UploadPropertyActivityDocument(1, uploadRequest); + + // Assert + action.Should().ThrowAsync().WithMessage("This file has an invalid file extension."); + documentService.Verify(x => x.UploadDocumentAsync(It.IsAny(), false), Times.Never); + } + + [Fact] + public void UploadDocument_Disposition_ShouldThrowException_InvalidExtension() + { + // Arrange + var service = this.CreateDocumentFileServiceWithPermissions(Permissions.DocumentAdd); + var documentService = this._helper.GetService>(); + + DocumentUploadRequest uploadRequest = new() { DocumentTypeId = 1, File = this._helper.GetFormFile("Lorem Ipsum", "test.exe") }; + + // Assert + Func action = async () => await service.UploadDispositionDocument(1, uploadRequest); + + // Assert + action.Should().ThrowAsync().WithMessage("This file has an invalid file extension."); + documentService.Verify(x => x.UploadDocumentAsync(It.IsAny(), false), Times.Never); + } + [Fact] public async void UploadDocument_Project_Success() { diff --git a/source/backend/tests/unit/api/Services/DocumentServiceTest.cs b/source/backend/tests/unit/api/Services/DocumentServiceTest.cs index ce278d5de5..94571686da 100644 --- a/source/backend/tests/unit/api/Services/DocumentServiceTest.cs +++ b/source/backend/tests/unit/api/Services/DocumentServiceTest.cs @@ -853,13 +853,24 @@ public async void DownloadFileLatestAsync_InvalidExtension() }, }); + documentStorageRepository.Setup(x => x.TryDownloadFileAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync(new ExternalResponse() + { + HttpStatusCode = System.Net.HttpStatusCode.OK, + Status = ExternalResponseStatus.Success, + Payload = new FileDownloadResponse() + { + FileName = "Test", + }, + }); + // Act var result = await service.DownloadFileLatestAsync(1); // Assert documentStorageRepository.Verify(x => x.TryGetDocumentAsync(It.IsAny()), Times.Once); - documentStorageRepository.Verify(x => x.TryDownloadFileAsync(It.IsAny(), It.IsAny()), Times.Never); - Assert.Equal(ExternalResponseStatus.Error, result.Status); + documentStorageRepository.Verify(x => x.TryDownloadFileAsync(It.IsAny(), It.IsAny()), Times.Once); + Assert.Equal(ExternalResponseStatus.Success, result.Status); } [Fact] @@ -963,7 +974,7 @@ public async void DownloadFileAsync_InvalidExtension() // Assert documentStorageRepository.Verify(x => x.TryDownloadFileAsync(It.IsAny(), It.IsAny()), Times.Once); - Assert.Equal(ExternalResponseStatus.Error, result.Status); + Assert.Equal(ExternalResponseStatus.Success, result.Status); } [Fact] @@ -1074,13 +1085,24 @@ public async void StreamFileLatestAsync_InvalidExtension() }, }); + documentStorageRepository.Setup(x => x.TryDownloadFileAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync(new ExternalResponse() + { + HttpStatusCode = System.Net.HttpStatusCode.OK, + Status = ExternalResponseStatus.Success, + Payload = new FileDownloadResponse() + { + FileName = "Test", + }, + }); + // Act var result = await service.DownloadFileLatestAsync(1); // Assert documentStorageRepository.Verify(x => x.TryGetDocumentAsync(It.IsAny()), Times.Once); - documentStorageRepository.Verify(x => x.TryStreamFileAsync(It.IsAny(), It.IsAny()), Times.Never); - Assert.Equal(ExternalResponseStatus.Error, result.Status); + documentStorageRepository.Verify(x => x.TryDownloadFileAsync(It.IsAny(), It.IsAny()), Times.Once); + Assert.Equal(ExternalResponseStatus.Success, result.Status); } [Fact] @@ -1183,7 +1205,7 @@ public async void StreamFileAsync_InvalidExtension() // Assert documentStorageRepository.Verify(x => x.TryStreamFileAsync(It.IsAny(), It.IsAny()), Times.Once); - Assert.Equal(ExternalResponseStatus.Error, result.Status); + Assert.Equal(ExternalResponseStatus.Success, result.Status); } } } diff --git a/source/database/mssql/scripts/dbscripts/PSP_PIMS_S96_00/Build/075_DML_PIMS_ROLE_CLAIM.sql b/source/database/mssql/scripts/dbscripts/PSP_PIMS_S96_00/Build/075_DML_PIMS_ROLE_CLAIM.sql index 70eb3de11d..8ed3888fd7 100644 --- a/source/database/mssql/scripts/dbscripts/PSP_PIMS_S96_00/Build/075_DML_PIMS_ROLE_CLAIM.sql +++ b/source/database/mssql/scripts/dbscripts/PSP_PIMS_S96_00/Build/075_DML_PIMS_ROLE_CLAIM.sql @@ -21,8 +21,8 @@ DECLARE @sysadmn BIGINT; -- SELECT @acqfunc = ROLE_ID FROM PIMS_ROLE WHERE NAME = 'Acquisition functional'; SELECT @acgrdon = ROLE_ID FROM PIMS_ROLE WHERE NAME = 'Acquisition read-only'; -SELECT @llfunc = ROLE_ID FROM PIMS_ROLE WHERE NAME = 'Lease/License functional'; -SELECT @llrdon = ROLE_ID FROM PIMS_ROLE WHERE NAME = 'Lease/License read-only'; +SELECT @llfunc = ROLE_ID FROM PIMS_ROLE WHERE NAME = 'Lease License functional'; +SELECT @llrdon = ROLE_ID FROM PIMS_ROLE WHERE NAME = 'Lease License read-only'; SELECT @mafunc = ROLE_ID FROM PIMS_ROLE WHERE NAME = 'Management functional'; SELECT @mardon = ROLE_ID FROM PIMS_ROLE WHERE NAME = 'Management read-only'; SELECT @prjfunc = ROLE_ID FROM PIMS_ROLE WHERE NAME = 'Project functional'; diff --git a/source/frontend/package.json b/source/frontend/package.json index 529e4d4436..437c0108d0 100644 --- a/source/frontend/package.json +++ b/source/frontend/package.json @@ -1,6 +1,6 @@ { "name": "frontend", - "version": "5.7.0-96.22", + "version": "5.7.1-96.22", "private": true, "dependencies": { "@bcgov/bc-sans": "1.0.1",