Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated code better handle malware scan errors so that endusers can b… #412

Merged
merged 2 commits into from
May 2, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions Test/Altinn.Broker.Tests/LegacyFileControllerTests.cs
Original file line number Diff line number Diff line change
@@ -19,13 +19,15 @@ public class LegacyFileControllerTests : IClassFixture<CustomWebApplicationFacto
private readonly CustomWebApplicationFactory _factory;
private readonly HttpClient _senderClient;
private readonly HttpClient _legacyClient;
private readonly HttpClient _webhookClient;
private readonly JsonSerializerOptions _responseSerializerOptions;

public LegacyFileControllerTests(CustomWebApplicationFactory factory)
{
_factory = factory;
_senderClient = _factory.CreateClientWithAuthorization(TestConstants.DUMMY_SENDER_TOKEN);
_legacyClient = _factory.CreateClientWithAuthorization(TestConstants.DUMMY_LEGACY_TOKEN);
_webhookClient = factory.CreateClient();
_responseSerializerOptions = new JsonSerializerOptions(new JsonSerializerOptions()
{
PropertyNameCaseInsensitive = true
@@ -95,6 +97,36 @@ public async Task GetFiles_GetByMultipleRecipient_Success()
Assert.Contains(Guid.Parse(fileId), result);
}

[Fact]
public async Task InitializeAndUpload_FailsDueToAntivirus()
{
// Initialize
var initializeFileResponse = await _legacyClient.PostAsJsonAsync("broker/api/legacy/v1/file", FileTransferInitializeExtTestFactory.BasicFileTransfer());
string onBehalfOfConsumer = FileTransferInitializeExtTestFactory.BasicFileTransfer().Sender;
Assert.True(initializeFileResponse.IsSuccessStatusCode, await initializeFileResponse.Content.ReadAsStringAsync());
var fileId = await initializeFileResponse.Content.ReadAsStringAsync();
var fileAfterInitialize = await _legacyClient.GetFromJsonAsync<LegacyFileOverviewExt>($"broker/api/legacy/v1/file/{fileId}?onBehalfOfConsumer={onBehalfOfConsumer}", _responseSerializerOptions);
Assert.NotNull(fileAfterInitialize);
Assert.Equal(LegacyFileStatusExt.Initialized, fileAfterInitialize.FileStatus);

// Upload
var uploadedFileBytes = Encoding.UTF8.GetBytes(@"X5O!P%@AP[4\PZX54(P^)7CC)7}$EICAR-STANDARD-ANTIVIRUS-TEST-FILE!$H+H*");
using (var content = new ByteArrayContent(uploadedFileBytes))
{
content.Headers.ContentType = new MediaTypeHeaderValue("application/octet-stream");
var uploadResponse = await _legacyClient.PostAsync($"broker/api/legacy/v1/file/{fileId}/upload?onBehalfOfConsumer={onBehalfOfConsumer}", content);
Assert.True(uploadResponse.IsSuccessStatusCode, await uploadResponse.Content.ReadAsStringAsync());
}

var jsonBody = GetMalwareScanResultJson("Data/MalwareScanResult_Malicious.json", fileId);
await SendMalwareScanResult(jsonBody);

var fileAfterUpload = await _legacyClient.GetFromJsonAsync<LegacyFileOverviewExt>($"broker/api/legacy/v1/file/{fileId}?onBehalfOfConsumer={onBehalfOfConsumer}", _responseSerializerOptions);
Assert.NotNull(fileAfterUpload);
Assert.Equal(LegacyFileStatusExt.Failed, fileAfterUpload.FileStatus);
Assert.StartsWith("Malware", fileAfterUpload.FileStatusText);
}

[Fact]
public async Task GetFiles_GetByMultipleRecipient_Recipient1_Success()
{
@@ -388,6 +420,20 @@ public async Task Download_ConfirmDownloaded_Success()
Assert.Equal(LegacyRecipientFileStatusExt.DownloadConfirmed, result?.Recipients[0]?.CurrentRecipientFileStatusCode);
Assert.Equal(LegacyFileStatusExt.AllConfirmedDownloaded, result?.FileStatus);
}
private string GetMalwareScanResultJson(string filePath, string fileId)
{
string jsonBody = File.ReadAllText(filePath);
var randomizedEtagId = Guid.NewGuid().ToString();
jsonBody = jsonBody.Replace("--FILEID--", fileId);
jsonBody = jsonBody.Replace("--ETAGID--", randomizedEtagId.ToString());
return jsonBody;
}
private async Task<HttpResponseMessage> SendMalwareScanResult(string jsonBody)
{
var result = await _webhookClient.PostAsync("broker/api/v1/webhooks/malwarescanresults", new StringContent(jsonBody, Encoding.UTF8, "application/json"));
Assert.True(result.IsSuccessStatusCode, $"The request failed with status code {result.StatusCode}. Error message: {await result.Content.ReadAsStringAsync()}");
return result;
}

private async Task<string> InitializeFile()
{
6 changes: 3 additions & 3 deletions altinn-broker-v1.json
Original file line number Diff line number Diff line change
@@ -30,10 +30,10 @@
}
}
},
"no.altinn.broker.filedeleted": {
"no.altinn.broker.filepurged": {
"post": {
"requestBody": {
"description": "The file has been deleted from Broker",
"description": "The file has been purged from Broker",
"content": {
"application/cloudevents+json": {
"schema": {
@@ -789,7 +789,7 @@
"Published",
"Cancelled",
"AllConfirmedDownloaded",
"Deleted",
"Purged",
"Failed"
],
"type": "string"
2 changes: 1 addition & 1 deletion src/Altinn.Broker.API/Enums/FileTransferStatusExt.cs
Original file line number Diff line number Diff line change
@@ -8,7 +8,7 @@ public enum FileTransferStatusExt
Published,
Cancelled,
AllConfirmedDownloaded,
Deleted,
Purged,
Failed
}
}
4 changes: 2 additions & 2 deletions src/Altinn.Broker.API/Mappers/FileStatusOverviewExtMapper.cs
Original file line number Diff line number Diff line change
@@ -40,7 +40,7 @@ internal static FileTransferStatusExt MapToExternalEnum(FileTransferStatus domai
FileTransferStatus.Published => FileTransferStatusExt.Published,
FileTransferStatus.Cancelled => FileTransferStatusExt.Cancelled,
FileTransferStatus.AllConfirmedDownloaded => FileTransferStatusExt.AllConfirmedDownloaded,
FileTransferStatus.Deleted => FileTransferStatusExt.Deleted,
FileTransferStatus.Purged => FileTransferStatusExt.Purged,
FileTransferStatus.Failed => FileTransferStatusExt.Failed,
_ => throw new InvalidEnumArgumentException()
};
@@ -60,7 +60,7 @@ internal static string MapToFileTransferStatusText(FileTransferStatusEntity file
FileTransferStatus.Published => "Ready for download",
FileTransferStatus.Cancelled => "FileTransfer cancelled",
FileTransferStatus.AllConfirmedDownloaded => "All downloaded",
FileTransferStatus.Deleted => "FileTransfer has been deleted",
FileTransferStatus.Purged => "FileTransfer has been purged",
FileTransferStatus.Failed => "Upload failed",
_ => throw new InvalidEnumArgumentException()
};
Original file line number Diff line number Diff line change
@@ -21,7 +21,7 @@ internal static LegacyFileOverviewExt MapToExternalModel(FileTransferEntity file
FileStatus = MapToExternalEnum(fileTransfer.FileTransferStatusEntity.Status),
Sender = fileTransfer.Sender.ActorExternalId,
FileStatusChanged = fileTransfer.FileTransferStatusChanged,
FileStatusText = MapToFileStatusText(fileTransfer.FileTransferStatusEntity.Status),
FileStatusText = MapToFileStatusText(fileTransfer.FileTransferStatusEntity),
PropertyList = fileTransfer.PropertyList,
Recipients = MapToRecipients(fileTransfer.RecipientCurrentStatuses),
SendersFileReference = fileTransfer.SendersFileTransferReference,
@@ -40,24 +40,24 @@ internal static LegacyFileStatusExt MapToExternalEnum(FileTransferStatus domainE
FileTransferStatus.Published => LegacyFileStatusExt.Published,
FileTransferStatus.Cancelled => LegacyFileStatusExt.Cancelled,
FileTransferStatus.AllConfirmedDownloaded => LegacyFileStatusExt.AllConfirmedDownloaded,
FileTransferStatus.Deleted => LegacyFileStatusExt.Deleted,
FileTransferStatus.Purged => LegacyFileStatusExt.Deleted,
FileTransferStatus.Failed => LegacyFileStatusExt.Failed,
_ => throw new InvalidEnumArgumentException()
};
}

internal static string MapToFileStatusText(FileTransferStatus domainEnum)
internal static string MapToFileStatusText(FileTransferStatusEntity status)
{
return domainEnum switch
return status.Status switch
{
FileTransferStatus.Initialized => "Ready for upload",
FileTransferStatus.UploadStarted => "Upload started",
FileTransferStatus.UploadProcessing => "Processing upload",
FileTransferStatus.Published => "Ready for download",
FileTransferStatus.Cancelled => "File cancelled",
FileTransferStatus.AllConfirmedDownloaded => "All downloaded",
FileTransferStatus.Deleted => "File has been deleted",
FileTransferStatus.Failed => "Upload failed",
FileTransferStatus.Purged => "File has been purged",
FileTransferStatus.Failed => status.DetailedStatus ?? "Upload failed",
_ => throw new InvalidEnumArgumentException()
};
}
Original file line number Diff line number Diff line change
@@ -40,14 +40,14 @@ public async Task<OneOf<Task, Error>> Process(ExpireFileTransferCommandRequest r
var resource = await GetResource(fileTransfer.ResourceId, cancellationToken);
var serviceOwner = await GetServiceOwner(resource.ServiceOwnerId);

if (fileTransfer.FileTransferStatusEntity.Status == Core.Domain.Enums.FileTransferStatus.Deleted)
if (fileTransfer.FileTransferStatusEntity.Status == Core.Domain.Enums.FileTransferStatus.Purged)
{
_logger.LogInformation("FileTransfer has already been set to deleted");
_logger.LogInformation("FileTransfer has already been set to purged");
}
else
else if (!request.DoNotUpdateStatus)
{
await _fileTransferStatusRepository.InsertFileTransferStatus(fileTransfer.FileTransferId, Core.Domain.Enums.FileTransferStatus.Deleted, cancellationToken: cancellationToken);
await _eventBus.Publish(AltinnEventType.FileDeleted, fileTransfer.ResourceId, fileTransfer.FileTransferId.ToString(), null, cancellationToken);
await _fileTransferStatusRepository.InsertFileTransferStatus(fileTransfer.FileTransferId, Core.Domain.Enums.FileTransferStatus.Purged, cancellationToken: cancellationToken);
await _eventBus.Publish(AltinnEventType.FilePurged, fileTransfer.ResourceId, fileTransfer.FileTransferId.ToString(), null, cancellationToken);
}
if (request.Force || fileTransfer.ExpirationTime < DateTime.UtcNow)
{
@@ -58,11 +58,10 @@ public async Task<OneOf<Task, Error>> Process(ExpireFileTransferCommandRequest r
_logger.LogError("Recipient {recipientExternalReference} did not download the fileTransfer with id {fileTransferId}", recipient.Actor.ActorExternalId, recipient.FileTransferId.ToString());
await _eventBus.Publish(AltinnEventType.FileNeverConfirmedDownloaded, fileTransfer.ResourceId, fileTransfer.FileTransferId.ToString(), recipient.Actor.ActorExternalId, cancellationToken);
}

}
else
{
throw new Exception("FileTransfer has not expired, and should not be deleted");
throw new Exception("FileTransfer has not expired, and should not be purged");
}
return Task.CompletedTask;
}
Original file line number Diff line number Diff line change
@@ -4,4 +4,10 @@ public class ExpireFileTransferCommandRequest
{
public Guid FileTransferId { get; set; }
public bool Force { get; set; }

/// <summary>
/// When this is set, the ExpireFileTransferCommandHandler will delete the file from storage, but will refrain from setting the FileTransferStatus to Purged in FileTransferRepository.
/// This is used when MalwareScanResultHandler fails an uploaded file.
/// </summary>
public bool DoNotUpdateStatus { get; set; }
}
Original file line number Diff line number Diff line change
@@ -66,7 +66,8 @@ public async Task<OneOf<Task, Error>> Process(ScanResultData data, CancellationT
_backgroundJobClient.Enqueue<ExpireFileTransferCommandHandler>(handler => handler.Process(new ExpireFileTransferCommandRequest
{
FileTransferId = fileTransfer.FileTransferId,
Force = true
Force = true,
DoNotUpdateStatus = true
}, CancellationToken.None));
return Task.CompletedTask;
}
2 changes: 1 addition & 1 deletion src/Altinn.Broker.Core/Domain/Enums/FileTransferStatus.cs
Original file line number Diff line number Diff line change
@@ -8,6 +8,6 @@ public enum FileTransferStatus
Published = 3,
Cancelled = 4,
AllConfirmedDownloaded = 5,
Deleted = 6,
Purged = 6,
Failed = 7
}
2 changes: 1 addition & 1 deletion src/Altinn.Broker.Core/Services/Enums/AltinnEventType.cs
Original file line number Diff line number Diff line change
@@ -8,6 +8,6 @@ public enum AltinnEventType
UploadFailed,
DownloadConfirmed,
AllConfirmedDownloaded,
FileDeleted,
FilePurged,
FileNeverConfirmedDownloaded
}
Original file line number Diff line number Diff line change
@@ -6,7 +6,7 @@ VALUES
(3, 'Published'),
(4, 'Cancelled'),
(5, 'AllConfirmedDownloaded'),
(6, 'Deleted'),
(6, 'Purged'),
(7, 'Failed');

INSERT INTO broker.actor_file_transfer_status_description (actor_file_transfer_status_description_id_pk, actor_file_transfer_status_description)