Skip to content

Commit

Permalink
Make Authorization service more testable by passing ClaimsPrincipal a…
Browse files Browse the repository at this point in the history
…s argument (#589)

* Make Authorization service more testable by passing ClaimsPrincipal as argument

* Do not pass principal to background jobs due to lack of serializability
  • Loading branch information
Ceredron authored Nov 19, 2024
1 parent 23f259c commit cefcec0
Show file tree
Hide file tree
Showing 19 changed files with 77 additions and 53 deletions.
18 changes: 9 additions & 9 deletions src/Altinn.Broker.API/Controllers/FileTransferController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public async Task<ActionResult<Guid>> InitializeFileTransfer(FileTransferInitali
logger.LogInformation("Initializing file transfer");
var commandRequest = InitializeFileTransferMapper.MapToRequest(initializeExt, token);

var commandResult = await handler.Process(commandRequest, cancellationToken);
var commandResult = await handler.Process(commandRequest, HttpContext.User, cancellationToken);
return commandResult.Match(
fileTransferId => Ok(new FileTransferInitializeResponseExt()
{
Expand Down Expand Up @@ -83,7 +83,7 @@ CancellationToken cancellationToken
Token = token,
UploadStream = Request.Body,
ContentLength = Request.ContentLength.Value
}, cancellationToken);
}, HttpContext.User, cancellationToken);
return commandResult.Match(
fileTransferId => Ok(new FileTransferUploadResponseExt()
{
Expand Down Expand Up @@ -113,7 +113,7 @@ CancellationToken cancellationToken
LogContextHelpers.EnrichLogsWithToken(token);
logger.LogInformation("Initializing and uploading fileTransfer");
var initializeRequest = InitializeFileTransferMapper.MapToRequest(form.Metadata, token);
var initializeResult = await initializeFileTransferHandler.Process(initializeRequest, cancellationToken);
var initializeResult = await initializeFileTransferHandler.Process(initializeRequest, HttpContext.User, cancellationToken);
if (initializeResult.IsT1)
{
Problem(initializeResult.AsT1);
Expand All @@ -126,7 +126,7 @@ CancellationToken cancellationToken
FileTransferId = fileTransferId,
Token = token,
UploadStream = Request.Body
}, cancellationToken);
}, HttpContext.User, cancellationToken);
return uploadResult.Match(
FileId => Ok(FileId.ToString()),
Problem
Expand All @@ -152,7 +152,7 @@ public async Task<ActionResult<FileTransferOverviewExt>> GetFileTransferOverview
{
FileTransferId = fileTransferId,
Token = token
}, cancellationToken);
}, HttpContext.User, cancellationToken);
return queryResult.Match(
result => Ok(FileTransferStatusOverviewExtMapper.MapToExternalModel(result.FileTransfer)),
Problem
Expand All @@ -178,7 +178,7 @@ public async Task<ActionResult<FileTransferStatusDetailsExt>> GetFileTransferDet
{
FileTransferId = fileTransferId,
Token = token
}, cancellationToken);
}, HttpContext.User, cancellationToken);
return queryResult.Match(
result => Ok(FileTransferStatusDetailsExtMapper.MapToExternalModel(result.FileTransfer, result.FileTransferEvents, result.ActorEvents)),
Problem
Expand Down Expand Up @@ -212,7 +212,7 @@ public async Task<ActionResult<List<Guid>>> GetFileTransfers(
RecipientStatus = recipientStatus is not null ? (ActorFileTransferStatus)recipientStatus : null,
From = from,
To = to
}, cancellationToken);
}, HttpContext.User, cancellationToken);
return queryResult.Match(
Ok,
Problem
Expand All @@ -238,7 +238,7 @@ public async Task<ActionResult> DownloadFile(
{
FileTransferId = fileTransferId,
Token = token
}, cancellationToken);
}, HttpContext.User, cancellationToken);
return queryResult.Match<ActionResult>(
result => File(result.DownloadStream, "application/octet-stream", result.FileName),
Problem
Expand All @@ -265,7 +265,7 @@ public async Task<ActionResult> ConfirmDownload(
FileTransferId = fileTransferId,
Token = token
};
var proccessingFunction = new Func<Task<OneOf<Task, Error>>>(() => handler.Process(requestData, cancellationToken));
var proccessingFunction = new Func<Task<OneOf<Task, Error>>>(() => handler.Process(requestData, HttpContext.User, cancellationToken));
var uniqueString = $"confirmDownload_{fileTransferId}_{token.Consumer}";
var commandResult = await IdempotencyEventHelper.ProcessEvent(uniqueString, proccessingFunction, idempotencyEventRepository, cancellationToken);
return commandResult.Match(
Expand Down
12 changes: 6 additions & 6 deletions src/Altinn.Broker.API/Controllers/LegacyFileController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public async Task<ActionResult<Guid>> InitializeFile(LegacyFileInitalizeExt init
LogContextHelpers.EnrichLogsWithToken(legacyToken);
logger.LogInformation("Legacy - Initializing file");
var commandRequest = LegacyInitializeFileMapper.MapToRequest(initializeExt, token);
var commandResult = await handler.Process(commandRequest, cancellationToken);
var commandResult = await handler.Process(commandRequest, HttpContext.User, cancellationToken);
return commandResult.Match(
fileId => Ok(fileId.ToString()),
Problem
Expand Down Expand Up @@ -78,7 +78,7 @@ CancellationToken cancellationToken
Token = legacyToken,
UploadStream = Request.Body,
IsLegacy = true
}, cancellationToken);
}, HttpContext.User, cancellationToken);
return commandResult.Match(
fileId => Ok(fileId.ToString()),
Problem
Expand Down Expand Up @@ -107,7 +107,7 @@ public async Task<ActionResult<LegacyFileOverviewExt>> GetFileOverview(
FileTransferId = fileId,
Token = legacyToken,
IsLegacy = true
}, cancellationToken);
}, HttpContext.User, cancellationToken);
return queryResult.Match(
result => Ok(LegacyFileStatusOverviewExtMapper.MapToExternalModel(result.FileTransfer)),
Problem
Expand Down Expand Up @@ -160,7 +160,7 @@ public async Task<ActionResult<List<Guid>>> GetFiles(
From = from,
To = to,
Recipients = recipients
}, cancellationToken);
}, HttpContext.User, cancellationToken);
return queryResult.Match(
Ok,
Problem
Expand Down Expand Up @@ -188,7 +188,7 @@ public async Task<ActionResult> DownloadFile(
FileTransferId = fileId,
Token = legacyToken,
IsLegacy = true
}, cancellationToken);
}, HttpContext.User, cancellationToken);
return queryResult.Match<ActionResult>(
result => File(result.DownloadStream, "application/octet-stream", result.FileName),
Problem
Expand Down Expand Up @@ -216,7 +216,7 @@ public async Task<ActionResult> ConfirmDownload(
FileTransferId = fileId,
Token = legacyToken,
IsLegacy = true
}, cancellationToken);
}, HttpContext.User, cancellationToken);
return commandResult.Match(
Ok,
Problem
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public async Task<ActionResult> ProcessMalwareScanResult([FromServices] MalwareS
{
throw new InvalidOperationException("Failed to deserialize malware scan result data");
}
var processFunction = new Func<Task<OneOf<Task, Error>>>(() => handler.Process(result, cancellationToken));
var processFunction = new Func<Task<OneOf<Task, Error>>>(() => handler.Process(result, null, cancellationToken));
var commandResult = await IdempotencyEventHelper.ProcessEvent(result.ETag, processFunction, idempotencyEventRepository, cancellationToken);
return commandResult.Match(
Ok,
Expand Down
2 changes: 1 addition & 1 deletion src/Altinn.Broker.API/Controllers/ResourceController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public async Task<ActionResult> ConfigureResource(string resourceId, [FromBody]
UseManifestFileShim = resourceExt.UseManifestFileShim,
ExternalServiceCodeLegacy = resourceExt.ExternalServiceCodeLegacy,
ExternalServiceEditionCodeLegacy = resourceExt.ExternalServiceEditionCodeLegacy
}, cancellationToken);
}, HttpContext.User, cancellationToken);

return result.Match(
(_) => Ok(null),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Xml;
using System.Security.Claims;
using System.Xml;

using Altinn.Broker.Application.Settings;
using Altinn.Broker.Core.Application;
Expand All @@ -14,7 +15,7 @@
namespace Altinn.Broker.Application.ConfigureResource;
public class ConfigureResourceHandler(IResourceRepository resourceRepository, IHostEnvironment hostEnvironment, ILogger<ConfigureResourceHandler> logger) : IHandler<ConfigureResourceRequest, Task>
{
public async Task<OneOf<Task, Error>> Process(ConfigureResourceRequest request, CancellationToken cancellationToken)
public async Task<OneOf<Task, Error>> Process(ConfigureResourceRequest request, ClaimsPrincipal? user, CancellationToken cancellationToken)
{
logger.LogInformation("Processing request to configure resource {ResourceId}", request.ResourceId.SanitizeForLogs());
var resource = await resourceRepository.GetResource(request.ResourceId, cancellationToken);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Security.Claims;
using System.Xml;

using Altinn.Broker.Application;
Expand Down Expand Up @@ -28,15 +29,15 @@ public class ConfirmDownloadHandler(
IEventBus eventBus,
ILogger<ConfirmDownloadHandler> logger) : IHandler<ConfirmDownloadRequest, Task>
{
public async Task<OneOf<Task, Error>> Process(ConfirmDownloadRequest request, CancellationToken cancellationToken)
public async Task<OneOf<Task, Error>> Process(ConfirmDownloadRequest request, ClaimsPrincipal? user, CancellationToken cancellationToken)
{
logger.LogInformation("Confirming download for file transfer {fileTransferId}", request.FileTransferId);
var fileTransfer = await fileTransferRepository.GetFileTransfer(request.FileTransferId, cancellationToken);
if (fileTransfer is null)
{
return Errors.FileTransferNotFound;
}
var hasAccess = await resourceRightsRepository.CheckUserAccess(fileTransfer.ResourceId, new List<ResourceAccessLevel> { ResourceAccessLevel.Read }, request.IsLegacy, cancellationToken);
var hasAccess = await resourceRightsRepository.CheckUserAccess(user, fileTransfer.ResourceId, new List<ResourceAccessLevel> { ResourceAccessLevel.Read }, request.IsLegacy, cancellationToken);
if (!hasAccess)
{
return Errors.FileTransferNotFound;
Expand Down Expand Up @@ -79,7 +80,7 @@ public async Task<OneOf<Task, Error>> Process(ConfirmDownloadRequest request, Ca
{
FileTransferId = request.FileTransferId,
Force = true
}, cancellationToken));
}, null, cancellationToken));
}
else
{
Expand All @@ -88,7 +89,7 @@ public async Task<OneOf<Task, Error>> Process(ConfirmDownloadRequest request, Ca
{
FileTransferId = request.FileTransferId,
Force = true
}, cancellationToken), DateTime.UtcNow.Add(gracePeriod));
}, null, cancellationToken), DateTime.UtcNow.Add(gracePeriod));
}
}
return Task.CompletedTask;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using System.Security.Claims;

using Altinn.Broker.Core.Application;
using Altinn.Broker.Core.Domain.Enums;
using Altinn.Broker.Core.Helpers;
Expand All @@ -10,7 +12,7 @@
namespace Altinn.Broker.Application.DownloadFile;
public class DownloadFileHandler(IResourceRepository resourceRepository, IServiceOwnerRepository serviceOwnerRepository, IAuthorizationService resourceRightsRepository, IFileTransferRepository fileTransferRepository, IActorFileTransferStatusRepository actorFileTransferStatusRepository, IBrokerStorageService brokerStorageService, ILogger<DownloadFileHandler> logger) : IHandler<DownloadFileRequest, DownloadFileResponse>
{
public async Task<OneOf<DownloadFileResponse, Error>> Process(DownloadFileRequest request, CancellationToken cancellationToken)
public async Task<OneOf<DownloadFileResponse, Error>> Process(DownloadFileRequest request, ClaimsPrincipal? user, CancellationToken cancellationToken)
{
logger.LogInformation("Starting download of file transfer {FileTransferId}", request.FileTransferId);
var fileTransfer = await fileTransferRepository.GetFileTransfer(request.FileTransferId, cancellationToken);
Expand All @@ -30,7 +32,7 @@ public async Task<OneOf<DownloadFileResponse, Error>> Process(DownloadFileReques
{
return Errors.NoFileUploaded;
}
var hasAccess = await resourceRightsRepository.CheckUserAccess(fileTransfer.ResourceId, new List<ResourceAccessLevel> { ResourceAccessLevel.Read }, request.IsLegacy, cancellationToken);
var hasAccess = await resourceRightsRepository.CheckUserAccess(user, fileTransfer.ResourceId, new List<ResourceAccessLevel> { ResourceAccessLevel.Read }, request.IsLegacy, cancellationToken);
if (!hasAccess)
{
return Errors.NoAccessToResource;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Transactions;
using System.Security.Claims;
using System.Transactions;

using Altinn.Broker.Application.Settings;
using Altinn.Broker.Core.Application;
Expand All @@ -21,7 +22,7 @@ namespace Altinn.Broker.Application.ExpireFileTransfer;
public class ExpireFileTransferHandler(IFileTransferRepository fileTransferRepository, IFileTransferStatusRepository fileTransferStatusRepository, IServiceOwnerRepository serviceOwnerRepository, IBrokerStorageService brokerStorageService, IResourceRepository resourceRepository, IEventBus eventBus, ILogger<ExpireFileTransferHandler> logger) : IHandler<ExpireFileTransferRequest, Task>
{
[AutomaticRetry(Attempts = 0)]
public async Task<OneOf<Task, Error>> Process(ExpireFileTransferRequest request, CancellationToken cancellationToken)
public async Task<OneOf<Task, Error>> Process(ExpireFileTransferRequest request, ClaimsPrincipal? user, CancellationToken cancellationToken)
{
logger.LogInformation("Deleting file transfer with id {fileTransferId}", request.FileTransferId.ToString());
var fileTransfer = await GetFileTransferAsync(request.FileTransferId, cancellationToken);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using System.Security.Claims;

using Altinn.Broker.Core.Application;
using Altinn.Broker.Core.Domain.Enums;
using Altinn.Broker.Core.Repositories;
Expand All @@ -10,7 +12,7 @@ namespace Altinn.Broker.Application.GetFileTransferDetails;

public class GetFileTransferDetailsHandler(IFileTransferRepository fileTransferRepository, IAuthorizationService resourceRightsRepository, IFileTransferStatusRepository fileTransferStatusRepository, IActorFileTransferStatusRepository actorFileTransferStatusRepository, ILogger<GetFileTransferDetailsHandler> logger) : IHandler<GetFileTransferDetailsRequest, GetFileTransferDetailsResponse>
{
public async Task<OneOf<GetFileTransferDetailsResponse, Error>> Process(GetFileTransferDetailsRequest request, CancellationToken cancellationToken)
public async Task<OneOf<GetFileTransferDetailsResponse, Error>> Process(GetFileTransferDetailsRequest request, ClaimsPrincipal? user, CancellationToken cancellationToken)
{
logger.LogInformation("Getting file transfer details for {fileTransferId}.", request.FileTransferId);
var fileTransfer = await fileTransferRepository.GetFileTransfer(request.FileTransferId, cancellationToken);
Expand All @@ -23,7 +25,7 @@ public async Task<OneOf<GetFileTransferDetailsResponse, Error>> Process(GetFileT
{
return Errors.FileTransferNotFound;
}
var hasAccess = await resourceRightsRepository.CheckUserAccess(fileTransfer.ResourceId, new List<ResourceAccessLevel> { ResourceAccessLevel.Write, ResourceAccessLevel.Read }, cancellationToken: cancellationToken);
var hasAccess = await resourceRightsRepository.CheckUserAccess(user, fileTransfer.ResourceId, new List<ResourceAccessLevel> { ResourceAccessLevel.Write, ResourceAccessLevel.Read }, cancellationToken: cancellationToken);
if (!hasAccess)
{
return Errors.NoAccessToResource;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using System.Security.Claims;

using Altinn.Broker.Core.Application;
using Altinn.Broker.Core.Domain.Enums;
using Altinn.Broker.Core.Repositories;
Expand All @@ -11,7 +13,7 @@ namespace Altinn.Broker.Application.GetFileTransferOverview;

public class GetFileTransferOverviewHandler(IAuthorizationService resourceRightsRepository, IFileTransferRepository fileTransferRepository, ILogger<GetFileTransferOverviewHandler> logger) : IHandler<GetFileTransferOverviewRequest, GetFileTransferOverviewResponse>
{
public async Task<OneOf<GetFileTransferOverviewResponse, Error>> Process(GetFileTransferOverviewRequest request, CancellationToken cancellationToken)
public async Task<OneOf<GetFileTransferOverviewResponse, Error>> Process(GetFileTransferOverviewRequest request, ClaimsPrincipal? user, CancellationToken cancellationToken)
{
logger.LogInformation("Retrieving file overview for file transfer {fileTransferId}. Legacy: {legacy}", request.FileTransferId, request.IsLegacy);
var fileTransfer = await fileTransferRepository.GetFileTransfer(request.FileTransferId, cancellationToken);
Expand All @@ -24,7 +26,7 @@ public async Task<OneOf<GetFileTransferOverviewResponse, Error>> Process(GetFile
{
return Errors.FileTransferNotFound;
}
var hasAccess = await resourceRightsRepository.CheckUserAccess(fileTransfer.ResourceId, new List<ResourceAccessLevel> { ResourceAccessLevel.Write, ResourceAccessLevel.Read }, request.IsLegacy, cancellationToken);
var hasAccess = await resourceRightsRepository.CheckUserAccess(user, fileTransfer.ResourceId, new List<ResourceAccessLevel> { ResourceAccessLevel.Write, ResourceAccessLevel.Read }, request.IsLegacy, cancellationToken);
if (!hasAccess)
{
return Errors.NoAccessToResource;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using System.Security.Claims;

using Altinn.Broker.Core.Application;
using Altinn.Broker.Core.Domain;
using Altinn.Broker.Core.Domain.Enums;
Expand All @@ -12,10 +14,10 @@ namespace Altinn.Broker.Application.GetFileTransfers;

public class GetFileTransfersHandler(IAuthorizationService resourceRightsRepository, IResourceRepository resourceRepository, IFileTransferRepository fileTransferRepository, IActorRepository actorRepository, ILogger<GetFileTransfersHandler> logger) : IHandler<GetFileTransfersRequest, List<Guid>>
{
public async Task<OneOf<List<Guid>, Error>> Process(GetFileTransfersRequest request, CancellationToken cancellationToken)
public async Task<OneOf<List<Guid>, Error>> Process(GetFileTransfersRequest request, ClaimsPrincipal? user, CancellationToken cancellationToken)
{
logger.LogInformation("Getting file transfers for {resourceId}", request.ResourceId.SanitizeForLogs());
var hasAccess = await resourceRightsRepository.CheckUserAccess(request.ResourceId, new List<ResourceAccessLevel> { ResourceAccessLevel.Write, ResourceAccessLevel.Read }, cancellationToken: cancellationToken);
var hasAccess = await resourceRightsRepository.CheckUserAccess(user, request.ResourceId, new List<ResourceAccessLevel> { ResourceAccessLevel.Write, ResourceAccessLevel.Read }, cancellationToken: cancellationToken);
if (!hasAccess)
{
return Errors.NoAccessToResource;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using System.Security.Claims;

using Altinn.Broker.Core.Application;
using Altinn.Broker.Core.Domain;
using Altinn.Broker.Core.Helpers;
Expand Down Expand Up @@ -26,7 +28,7 @@ private async Task<List<ActorEntity>> GetActors(string[] recipients, Cancellatio
return actors;
}

public async Task<OneOf<List<Guid>, Error>> Process(LegacyGetFilesRequest request, CancellationToken cancellationToken)
public async Task<OneOf<List<Guid>, Error>> Process(LegacyGetFilesRequest request, ClaimsPrincipal? user, CancellationToken cancellationToken)
{
logger.LogInformation("Legacy get files for {resourceId}", request.ResourceId is null ? "all resources" : request.ResourceId.SanitizeForLogs());
LegacyFileSearchEntity fileSearch = new()
Expand Down
Loading

0 comments on commit cefcec0

Please sign in to comment.