Skip to content

Commit

Permalink
HOTFIX: psp-9813, psp-9814 (#4578)
Browse files Browse the repository at this point in the history 
* psp-9813

* psp-9813 test updates.

* psp-9814 pims IS96 database fails to build.

* increment hotfix version.

---------

Co-authored-by: Smith <[email protected]>
  • Loading branch information
@devinleighsmith
devinleighsmith and Smith authored Jan 9, 2025
1 parent 497df80 commit b17698a
Show file tree
Hide file tree
Showing 8 changed files with 166 additions and 61 deletions.
4 changes: 2 additions & 2 deletions source/backend/api/Pims.Api.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
<Project Sdk="Microsoft.NET.Sdk.Web">
<PropertyGroup>
<UserSecretsId>0ef6255f-9ea0-49ec-8c65-c172304b4926</UserSecretsId>
<Version>5.7.0-96.22</Version>
<AssemblyVersion>5.7.0.96</AssemblyVersion>
<Version>5.7.1-96.22</Version>
<AssemblyVersion>5.7.1.96</AssemblyVersion>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<ProjectGuid>{16BC0468-78F6-4C91-87DA-7403C919E646}</ProjectGuid>
<TargetFramework>net8.0</TargetFramework>
Expand Down
23 changes: 17 additions & 6 deletions source/backend/api/Services/DocumentFileService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -392,6 +393,16 @@ public async Task<ExternalResponse<string>> 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
Expand Down
55 changes: 13 additions & 42 deletions source/backend/api/Services/DocumentService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -490,22 +490,13 @@ public async Task<ExternalResponse<FileDownloadResponse>> DownloadFileAsync(long
this.User.ThrowIfNotAuthorized(Permissions.DocumentView);

ExternalResponse<FileDownloadResponse> 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<FileDownloadResponse>()
{
Status = ExternalResponseStatus.Error,
Message = $"Document with id ${mayanDocumentId} has an invalid extension",
};
throw GetMayanResponseError(downloadResult.Message);
}
return downloadResult;
}

public async Task<ExternalResponse<FileStreamResponse>> StreamFileAsync(long mayanDocumentId, long mayanFileId)
Expand All @@ -514,22 +505,13 @@ public async Task<ExternalResponse<FileStreamResponse>> StreamFileAsync(long may
this.User.ThrowIfNotAuthorized(Permissions.DocumentView);

ExternalResponse<FileStreamResponse> 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<FileStreamResponse>()
{
Status = ExternalResponseStatus.Error,
Message = $"Document with id ${mayanDocumentId} has an invalid extension",
};
throw GetMayanResponseError(downloadResult.Message);
}
return downloadResult;
}

public async Task<ExternalResponse<FileDownloadResponse>> DownloadFileLatestAsync(long mayanDocumentId)
Expand All @@ -541,19 +523,8 @@ public async Task<ExternalResponse<FileDownloadResponse>> DownloadFileLatestAsyn
{
if (documentResult.Payload != null)
{
if (IsValidDocumentExtension(documentResult.Payload.FileLatest.FileName))
{
ExternalResponse<FileDownloadResponse> downloadResult = await documentStorageRepository.TryDownloadFileAsync(documentResult.Payload.Id, documentResult.Payload.FileLatest.Id);
return downloadResult;
}
else
{
return new ExternalResponse<FileDownloadResponse>()
{
Status = ExternalResponseStatus.Error,
Message = $"Document with id ${mayanDocumentId} has an invalid extension",
};
}
ExternalResponse<FileDownloadResponse> downloadResult = await documentStorageRepository.TryDownloadFileAsync(documentResult.Payload.Id, documentResult.Payload.FileLatest.Id);
return downloadResult;
}
else
{
Expand Down Expand Up @@ -639,7 +610,7 @@ public async Task<HttpResponseMessage> 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);
Expand Down
3 changes: 1 addition & 2 deletions source/backend/tests/core/TestHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,10 +222,9 @@ public T CreateInstance<T>()
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);
Expand Down
102 changes: 102 additions & 0 deletions source/backend/tests/unit/api/Services/DocumentFileServiceTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,108 @@ public void UploadDocument_Disposition_ShouldThrowException_NotAuthorized()
documentService.Verify(x => x.UploadDocumentAsync(It.IsAny<DocumentUploadRequest>(), false), Times.Never);
}

[Fact]
public void UploadDocument_Research_ShouldThrowException_InvalidExtension()
{
// Arrange
var service = this.CreateDocumentFileServiceWithPermissions(Permissions.DocumentAdd);
var documentService = this._helper.GetService<Mock<IDocumentService>>();

DocumentUploadRequest uploadRequest = new() { DocumentTypeId = 1, File = this._helper.GetFormFile("Lorem Ipsum", "test.exe") };

// Assert
Func<Task> action = async () => await service.UploadResearchDocument(1, uploadRequest);

// Assert
action.Should().ThrowAsync<BusinessRuleViolationException>().WithMessage("This file has an invalid file extension.");
documentService.Verify(x => x.UploadDocumentAsync(It.IsAny<DocumentUploadRequest>(), false), Times.Never);
}

[Fact]
public void UploadDocument_Acquisition_ShouldThrowException_InvalidExtension()
{
// Arrange
var service = this.CreateDocumentFileServiceWithPermissions(Permissions.DocumentAdd);
var documentService = this._helper.GetService<Mock<IDocumentService>>();

DocumentUploadRequest uploadRequest = new() { DocumentTypeId = 1, File = this._helper.GetFormFile("Lorem Ipsum", "test.exe") };

// Assert
Func<Task> action = async () => await service.UploadAcquisitionDocument(1, uploadRequest);

// Assert
action.Should().ThrowAsync<BusinessRuleViolationException>().WithMessage("This file has an invalid file extension.");
documentService.Verify(x => x.UploadDocumentAsync(It.IsAny<DocumentUploadRequest>(), false), Times.Never);
}

[Fact]
public void UploadDocument_Project_ShouldThrowException_InvalidExtension()
{
// Arrange
var service = this.CreateDocumentFileServiceWithPermissions(Permissions.DocumentAdd);
var documentService = this._helper.GetService<Mock<IDocumentService>>();

DocumentUploadRequest uploadRequest = new() { DocumentTypeId = 1, File = this._helper.GetFormFile("Lorem Ipsum", "test.exe") };

// Assert
Func<Task> action = async () => await service.UploadProjectDocument(1, uploadRequest);

// Assert
action.Should().ThrowAsync<BusinessRuleViolationException>().WithMessage("This file has an invalid file extension.");
documentService.Verify(x => x.UploadDocumentAsync(It.IsAny<DocumentUploadRequest>(), false), Times.Never);
}

[Fact]
public void UploadDocument_Lease_ShouldThrowException_InvalidExtension()
{
// Arrange
var service = this.CreateDocumentFileServiceWithPermissions(Permissions.DocumentAdd);
var documentService = this._helper.GetService<Mock<IDocumentService>>();

DocumentUploadRequest uploadRequest = new() { DocumentTypeId = 1, File = this._helper.GetFormFile("Lorem Ipsum", "test.exe") };

// Assert
Func<Task> action = async () => await service.UploadLeaseDocument(1, uploadRequest);

// Assert
action.Should().ThrowAsync<BusinessRuleViolationException>().WithMessage("This file has an invalid file extension.");
documentService.Verify(x => x.UploadDocumentAsync(It.IsAny<DocumentUploadRequest>(), false), Times.Never);
}

[Fact]
public void UploadDocument_PropertyActivity_ShouldThrowException_InvalidExtension()
{
// Arrange
var service = this.CreateDocumentFileServiceWithPermissions(Permissions.DocumentAdd);
var documentService = this._helper.GetService<Mock<IDocumentService>>();

DocumentUploadRequest uploadRequest = new() { DocumentTypeId = 1, File = this._helper.GetFormFile("Lorem Ipsum", "test.exe") };

// Assert
Func<Task> action = async () => await service.UploadPropertyActivityDocument(1, uploadRequest);

// Assert
action.Should().ThrowAsync<BusinessRuleViolationException>().WithMessage("This file has an invalid file extension.");
documentService.Verify(x => x.UploadDocumentAsync(It.IsAny<DocumentUploadRequest>(), false), Times.Never);
}

[Fact]
public void UploadDocument_Disposition_ShouldThrowException_InvalidExtension()
{
// Arrange
var service = this.CreateDocumentFileServiceWithPermissions(Permissions.DocumentAdd);
var documentService = this._helper.GetService<Mock<IDocumentService>>();

DocumentUploadRequest uploadRequest = new() { DocumentTypeId = 1, File = this._helper.GetFormFile("Lorem Ipsum", "test.exe") };

// Assert
Func<Task> action = async () => await service.UploadDispositionDocument(1, uploadRequest);

// Assert
action.Should().ThrowAsync<BusinessRuleViolationException>().WithMessage("This file has an invalid file extension.");
documentService.Verify(x => x.UploadDocumentAsync(It.IsAny<DocumentUploadRequest>(), false), Times.Never);
}

[Fact]
public async void UploadDocument_Project_Success()
{
Expand Down
34 changes: 28 additions & 6 deletions source/backend/tests/unit/api/Services/DocumentServiceTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -853,13 +853,24 @@ public async void DownloadFileLatestAsync_InvalidExtension()
},
});

documentStorageRepository.Setup(x => x.TryDownloadFileAsync(It.IsAny<long>(), It.IsAny<long>()))
.ReturnsAsync(new ExternalResponse<FileDownloadResponse>()
{
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<long>()), Times.Once);
documentStorageRepository.Verify(x => x.TryDownloadFileAsync(It.IsAny<long>(), It.IsAny<long>()), Times.Never);
Assert.Equal(ExternalResponseStatus.Error, result.Status);
documentStorageRepository.Verify(x => x.TryDownloadFileAsync(It.IsAny<long>(), It.IsAny<long>()), Times.Once);
Assert.Equal(ExternalResponseStatus.Success, result.Status);
}

[Fact]
Expand Down Expand Up @@ -963,7 +974,7 @@ public async void DownloadFileAsync_InvalidExtension()
// Assert
documentStorageRepository.Verify(x => x.TryDownloadFileAsync(It.IsAny<long>(), It.IsAny<long>()), Times.Once);

Assert.Equal(ExternalResponseStatus.Error, result.Status);
Assert.Equal(ExternalResponseStatus.Success, result.Status);
}

[Fact]
Expand Down Expand Up @@ -1074,13 +1085,24 @@ public async void StreamFileLatestAsync_InvalidExtension()
},
});

documentStorageRepository.Setup(x => x.TryDownloadFileAsync(It.IsAny<long>(), It.IsAny<long>()))
.ReturnsAsync(new ExternalResponse<FileDownloadResponse>()
{
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<long>()), Times.Once);
documentStorageRepository.Verify(x => x.TryStreamFileAsync(It.IsAny<long>(), It.IsAny<long>()), Times.Never);
Assert.Equal(ExternalResponseStatus.Error, result.Status);
documentStorageRepository.Verify(x => x.TryDownloadFileAsync(It.IsAny<long>(), It.IsAny<long>()), Times.Once);
Assert.Equal(ExternalResponseStatus.Success, result.Status);
}

[Fact]
Expand Down Expand Up @@ -1183,7 +1205,7 @@ public async void StreamFileAsync_InvalidExtension()
// Assert
documentStorageRepository.Verify(x => x.TryStreamFileAsync(It.IsAny<long>(), It.IsAny<long>()), Times.Once);

Assert.Equal(ExternalResponseStatus.Error, result.Status);
Assert.Equal(ExternalResponseStatus.Success, result.Status);
}
}
}
4 changes: 2 additions & 2 deletions source/database/mssql/scripts/dbscripts/PSP_PIMS_S96_00/Build/075_DML_PIMS_ROLE_CLAIM.sql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
2 changes: 1 addition & 1 deletion source/frontend/package.json
View file
Original file line number Diff line number Diff line change
@@ -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",
Expand Down

0 comments on commit b17698a

Please sign in to comment.