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 (#501)

* Pass ClaimsPrincipal as argument instead of HttpContextAccessor to make code cleaner and easier to test

* Webhooks do not use User
  • Loading branch information
@Ceredron
Ceredron committed Nov 19, 2024
1 parent 3bb985b commit b6733f0
Show file tree
Hide file tree
Showing 38 changed files with 132 additions and 101 deletions.
2 changes: 1 addition & 1 deletion Test/Altinn.Correspondence.Tests/Helpers/MaskinportenWebApplicationFactory.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ protected override void ConfigureWebHost(
HangfireBackgroundJobClient = new Mock<IBackgroundJobClient>();
services.AddSingleton(HangfireBackgroundJobClient.Object);
var altinnAuthorizationService = new Mock<IAltinnAuthorizationService>();
altinnAuthorizationService.Setup(x => x.CheckUserAccess(It.IsAny<string>(), It.IsAny<List<ResourceAccessLevel>>(), It.IsAny<CancellationToken>(), null)).ReturnsAsync(true);
altinnAuthorizationService.Setup(x => x.CheckUserAccess(It.IsAny<ClaimsPrincipal?>(), It.IsAny<string>(), It.IsAny<List<ResourceAccessLevel>>(), It.IsAny<CancellationToken>(), null)).ReturnsAsync(true);
altinnAuthorizationService.Setup(x => x.CheckMigrationAccess(It.IsAny<string>(), It.IsAny<List<ResourceAccessLevel>>(), It.IsAny<CancellationToken>())).ReturnsAsync(true);
services.AddSingleton(altinnAuthorizationService.Object);
});
Expand Down
2 changes: 1 addition & 1 deletion Test/Altinn.Correspondence.Tests/Helpers/UnitWebApplicationFactory.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ protected override void ConfigureWebHost(
config.UseMemoryStorage()
);
var altinnAuthorizationService = new Mock<IAltinnAuthorizationService>();
altinnAuthorizationService.Setup(x => x.CheckUserAccess(It.IsAny<string>(), It.IsAny<List<ResourceAccessLevel>>(), It.IsAny<CancellationToken>(), null)).ReturnsAsync(true);
altinnAuthorizationService.Setup(x => x.CheckUserAccess(It.IsAny<ClaimsPrincipal?>(), It.IsAny<string>(), It.IsAny<List<ResourceAccessLevel>>(), It.IsAny<CancellationToken>(), null)).ReturnsAsync(true);
services.AddSingleton(altinnAuthorizationService.Object);
if (_customServices is not null)
_customServices(services);
Expand Down
14 changes: 7 additions & 7 deletions src/Altinn.Correspondence.API/Controllers/AttachmentController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public class AttachmentController(ILogger<CorrespondenceController> logger) : Co
public async Task<ActionResult<Guid>> InitializeAttachment(InitializeAttachmentExt InitializeAttachmentExt, [FromServices] InitializeAttachmentHandler handler, CancellationToken cancellationToken)
{
var commandRequest = InitializeAttachmentMapper.MapToRequest(InitializeAttachmentExt);
var commandResult = await handler.Process(commandRequest, cancellationToken);
var commandResult = await handler.Process(commandRequest, HttpContext.User, cancellationToken);
_logger.LogInformation("Initialize attachment");

return commandResult.Match(
Expand Down Expand Up @@ -65,8 +65,8 @@ public async Task<ActionResult<AttachmentOverviewExt>> UploadAttachmentData(
AttachmentId = attachmentId,
UploadStream = Request.Body,
ContentLength = Request.ContentLength ?? Request.Body.Length
}, cancellationToken);
var attachmentOverviewResult = await attachmentOverviewHandler.Process(attachmentId, cancellationToken);
}, HttpContext.User, cancellationToken);
var attachmentOverviewResult = await attachmentOverviewHandler.Process(attachmentId, HttpContext.User, cancellationToken);
if (!attachmentOverviewResult.TryPickT0(out var attachmentOverview, out var error))
{
return Problem(error);
Expand All @@ -92,7 +92,7 @@ public async Task<ActionResult<AttachmentOverviewExt>> GetAttachmentOverview(
{
_logger.LogInformation("Get attachment overview {attachmentId}", attachmentId.ToString());

var commandResult = await handler.Process(attachmentId, cancellationToken);
var commandResult = await handler.Process(attachmentId, HttpContext.User, cancellationToken);

return commandResult.Match(
attachment => Ok(AttachmentOverviewMapper.MapToExternal(attachment)),
Expand All @@ -116,7 +116,7 @@ public async Task<ActionResult<AttachmentDetailsExt>> GetAttachmentDetails(

_logger.LogInformation("Get attachment details {attachmentId}", attachmentId.ToString());

var commandResult = await handler.Process(attachmentId, cancellationToken);
var commandResult = await handler.Process(attachmentId, HttpContext.User, cancellationToken);

return commandResult.Match(
attachment => Ok(AttachmentDetailsMapper.MapToExternal(attachment)),
Expand All @@ -142,7 +142,7 @@ public async Task<ActionResult<AttachmentOverviewExt>> DeleteAttachment(
{
_logger.LogInformation("Delete attachment {attachmentId}", attachmentId.ToString());

var commandResult = await handler.Process(attachmentId, cancellationToken);
var commandResult = await handler.Process(attachmentId, HttpContext.User, cancellationToken);

return commandResult.Match(
_ => Ok(null),
Expand All @@ -164,7 +164,7 @@ public async Task<ActionResult> DownloadAttachmentData(
var commandResult = await handler.Process(new DownloadAttachmentRequest()
{
AttachmentId = attachmentId
}, cancellationToken);
}, HttpContext.User, cancellationToken);
return commandResult.Match(
result => File(result, "application/octet-stream"),
Problem
Expand Down
24 changes: 12 additions & 12 deletions src/Altinn.Correspondence.API/Controllers/CorrespondenceController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public async Task<ActionResult<InitializeCorrespondencesResponseExt>> Initialize
_logger.LogInformation("Initialize correspondences");

var commandRequest = InitializeCorrespondencesMapper.MapToRequest(request.Correspondence, request.Recipients, null, request.ExistingAttachments);
var commandResult = await handler.Process(commandRequest, cancellationToken);
var commandResult = await handler.Process(commandRequest, HttpContext.User, cancellationToken);

return commandResult.Match(
data => Ok(InitializeCorrespondencesMapper.MapToExternal(data)),
Expand Down Expand Up @@ -82,7 +82,7 @@ public async Task<ActionResult<InitializeCorrespondencesResponseExt>> UploadCorr
Request.EnableBuffering();

var commandRequest = InitializeCorrespondencesMapper.MapToRequest(request.Correspondence, request.Recipients, attachments, request.ExistingAttachments);
var commandResult = await handler.Process(commandRequest, cancellationToken);
var commandResult = await handler.Process(commandRequest, HttpContext.User, cancellationToken);

return commandResult.Match(
data => Ok(InitializeCorrespondencesMapper.MapToExternal(data)),
Expand All @@ -108,7 +108,7 @@ public async Task<ActionResult<CorrespondenceOverviewExt>> GetCorrespondenceOver
{
_logger.LogInformation("Getting Correspondence overview for {correspondenceId}", correspondenceId.ToString());

var commandResult = await handler.Process(correspondenceId, cancellationToken);
var commandResult = await handler.Process(correspondenceId, HttpContext.User, cancellationToken);

return commandResult.Match(
data => Ok(CorrespondenceOverviewMapper.MapToExternal(data)),
Expand All @@ -134,7 +134,7 @@ public async Task<ActionResult<CorrespondenceDetailsExt>> GetCorrespondenceDetai
{
_logger.LogInformation("Getting Correspondence details for {correspondenceId}", correspondenceId.ToString());

var commandResult = await handler.Process(correspondenceId, cancellationToken);
var commandResult = await handler.Process(correspondenceId, HttpContext.User, cancellationToken);

return commandResult.Match(
data => Ok(CorrespondenceDetailsMapper.MapToExternal(data)),
Expand All @@ -161,7 +161,7 @@ public async Task<ActionResult> GetCorrespondenceContent(
{
_logger.LogInformation("Getting Correspondence content for {correspondenceId}", correspondenceId.ToString());

var commandResult = await handler.Process(correspondenceId, cancellationToken);
var commandResult = await handler.Process(correspondenceId, HttpContext.User, cancellationToken);
return commandResult.Match(
data => Ok(data.Content.MessageBody),
Problem
Expand Down Expand Up @@ -200,7 +200,7 @@ public async Task<ActionResult<CorrespondencesExt>> GetCorrespondences(
Offset = offset,
Status = status is null ? null : (CorrespondenceStatus)status,
Role = role,
}, cancellationToken);
}, HttpContext.User, cancellationToken);

return commandResult.Match(
data => Ok(data),
Expand Down Expand Up @@ -229,7 +229,7 @@ public async Task<ActionResult> MarkAsRead(
{
CorrespondenceId = correspondenceId,
Status = CorrespondenceStatus.Read
}, cancellationToken);
}, HttpContext.User, cancellationToken);

return commandResult.Match(
data => Ok(data),
Expand Down Expand Up @@ -259,7 +259,7 @@ public async Task<ActionResult> Confirm(
{
CorrespondenceId = correspondenceId,
Status = CorrespondenceStatus.Confirmed
}, cancellationToken);
}, HttpContext.User, cancellationToken);

return commandResult.Match(
data => Ok(data),
Expand Down Expand Up @@ -289,7 +289,7 @@ public async Task<ActionResult> Archive(
{
CorrespondenceId = correspondenceId,
Status = CorrespondenceStatus.Archived
}, cancellationToken);
}, HttpContext.User, cancellationToken);

return commandResult.Match(
data => Ok(data),
Expand All @@ -314,7 +314,7 @@ public async Task<ActionResult> Purge(
{
_logger.LogInformation("Purging Correspondence with id: {correspondenceId}", correspondenceId.ToString());

var commandResult = await handler.Process(correspondenceId, cancellationToken);
var commandResult = await handler.Process(correspondenceId, HttpContext.User, cancellationToken);

return commandResult.Match(
data => Ok(data),
Expand All @@ -341,7 +341,7 @@ public async Task<ActionResult> DownloadCorrespondenceAttachmentData(
{
CorrespondenceId = correspondenceId,
AttachmentId = attachmentId
}, cancellationToken);
}, HttpContext.User, cancellationToken);
return commandResult.Match(
result => File(result.Stream, "application/octet-stream", result.FileName),
Problem
Expand All @@ -361,7 +361,7 @@ public async Task<ActionResult> CheckNotification(
CancellationToken cancellationToken)
{
_logger.LogInformation("Checking notification for Correspondence with id: {correspondenceId}", correspondenceId.ToString());
var commandResult = await handler.Process(correspondenceId, cancellationToken);
var commandResult = await handler.Process(correspondenceId, HttpContext.User, cancellationToken);

return commandResult.Match(
data => Ok(data),
Expand Down
18 changes: 9 additions & 9 deletions src/Altinn.Correspondence.API/Controllers/LegacyCorrespondenceController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public async Task<ActionResult<CorrespondenceOverviewExt>> GetCorrespondenceOver
{
_logger.LogInformation("Getting Correspondence overview for {correspondenceId}", correspondenceId.ToString());

var commandResult = await handler.Process(correspondenceId, cancellationToken);
var commandResult = await handler.Process(correspondenceId, HttpContext.User, cancellationToken);
return commandResult.Match(
data => Ok(LegacyCorrespondenceOverviewMapper.MapToExternal(data)),
Problem
Expand All @@ -66,7 +66,7 @@ public async Task<ActionResult<LegacyGetCorrespondenceHistoryResponse>> GetCorre
{
_logger.LogInformation("Getting Correspondence history for {correspondenceId}", correspondenceId.ToString());

var commandResult = await handler.Process(correspondenceId, cancellationToken);
var commandResult = await handler.Process(correspondenceId, HttpContext.User, cancellationToken);

return commandResult.Match(
data => Ok(data),
Expand All @@ -86,9 +86,9 @@ public async Task<ActionResult<CorrespondencesExt>> GetCorrespondences(
{
_logger.LogInformation("Get correspondences for receiver");

LegacyGetCorrespondencesRequest req = LegacyGetCorrespondencesMapper.MapToRequest(request);
LegacyGetCorrespondencesRequest legacyRequest = LegacyGetCorrespondencesMapper.MapToRequest(request);

var commandResult = await handler.Process(req, cancellationToken);
var commandResult = await handler.Process(legacyRequest, HttpContext.User, cancellationToken);

return commandResult.Match(
data => Ok(data),
Expand All @@ -111,7 +111,7 @@ public async Task<ActionResult> DownloadCorrespondenceAttachment(
{
CorrespondenceId = correspondenceId,
AttachmentId = attachmentId
}, cancellationToken);
}, HttpContext.User, cancellationToken);

return commandResult.Match<ActionResult>(
result => File(result.Stream, "application/octet-stream", result.FileName),
Expand Down Expand Up @@ -139,7 +139,7 @@ public async Task<ActionResult> MarkAsRead(
{
CorrespondenceId = correspondenceId,
Status = CorrespondenceStatus.Read
}, cancellationToken);
}, HttpContext.User, cancellationToken);

return commandResult.Match(
data => Ok(data),
Expand All @@ -166,7 +166,7 @@ public async Task<ActionResult> Confirm(
{
CorrespondenceId = correspondenceId,
Status = CorrespondenceStatus.Confirmed
}, cancellationToken);
}, HttpContext.User, cancellationToken);

return commandResult.Match(
data => Ok(data),
Expand Down Expand Up @@ -194,7 +194,7 @@ public async Task<ActionResult> Archive(
{
CorrespondenceId = correspondenceId,
Status = CorrespondenceStatus.Archived
}, cancellationToken);
}, HttpContext.User, cancellationToken);

return commandResult.Match(
data => Ok(data),
Expand All @@ -218,7 +218,7 @@ public async Task<ActionResult> Purge(
{
_logger.LogInformation("Purging Correspondence with id: {correspondenceId}", correspondenceId.ToString());

var commandResult = await handler.Process(correspondenceId, cancellationToken);
var commandResult = await handler.Process(correspondenceId, HttpContext.User, cancellationToken);

return commandResult.Match(
data => Ok(data),
Expand Down
2 changes: 1 addition & 1 deletion src/Altinn.Correspondence.API/Controllers/MalwareScanController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ private async Task<ActionResult> ProcessSingleEvent(
_logger.LogError("Failed to deserialize malware scan result data");
return BadRequest();
}
var commandResult = await handler.Process(result, cancellationToken);
var commandResult = await handler.Process(result, null, cancellationToken);
return commandResult.Match(
Ok,
Problem
Expand Down
6 changes: 3 additions & 3 deletions src/Altinn.Correspondence.API/Controllers/MigrationController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public async Task<ActionResult<CorrespondenceMigrationStatusExt>> MigrateCorresp
_logger.LogInformation("Initialize correspondence");

var commandRequest = MigrateCorrespondenceMapper.MapToRequest(migrateCorrespondence);
var commandResult = await handler.Process(commandRequest, cancellationToken);
var commandResult = await handler.Process(commandRequest, HttpContext.User, cancellationToken);

return commandResult.Match(
data => Ok(MigrateCorrespondenceMapper.MapToExternal(data)),
Expand Down Expand Up @@ -85,7 +85,7 @@ public async Task<ActionResult<Guid>> MigrateAttachment(
{
_logger.LogInformation("{initializeAttachmentExt.SendersReference};Initializing attachment with sendersference", initializeAttachmentExt.SendersReference);
var commandRequest = InitializeAttachmentMapper.MapToRequest(initializeAttachmentExt);
var commandResult = await migrateInitializeAttachmentHandler.Process(commandRequest, cancellationToken);
var commandResult = await migrateInitializeAttachmentHandler.Process(commandRequest, HttpContext.User, cancellationToken);

return commandResult.Match(
attachmentId => Ok(attachmentId.ToString()),
Expand Down Expand Up @@ -115,7 +115,7 @@ public async Task<ActionResult<AttachmentOverviewExt>> UploadAttachmentData(
AttachmentId = attachmentId,
UploadStream = Request.Body,
ContentLength = Request.ContentLength ?? Request.Body.Length
}, cancellationToken);
}, HttpContext.User, cancellationToken);
return uploadAttachmentResult.Match(
attachment => Ok(AttachmentOverviewMapper.MapMigrateToExternal(attachment)),
Problem
Expand Down
3 changes: 2 additions & 1 deletion src/Altinn.Correspondence.Application/CancelNotification/CancelNotificationHandler.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Runtime.CompilerServices;
using System.Security.Claims;
using System.Threading;
using Altinn.Correspondence.Core.Models.Entities;
using Altinn.Correspondence.Core.Repositories;
Expand Down Expand Up @@ -41,7 +42,7 @@ public CancelNotificationHandler(
}

[AutomaticRetry(Attempts = MaxRetries)]
public async Task Process(PerformContext context, Guid correspondenceId, CancellationToken cancellationToken = default)
public async Task Process(PerformContext context, Guid correspondenceId, ClaimsPrincipal? _, CancellationToken cancellationToken = default)
{
var retryAttempts = context.GetJobParameter<int>(RetryCountKey);
_logger.LogInformation("Cancelling notifications for purged correspondence. Retry attempt: {retryAttempts}", retryAttempts);
Expand Down
3 changes: 2 additions & 1 deletion src/Altinn.Correspondence.Application/CheckNotification/CheckNotificationHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using Altinn.Correspondence.Core.Models.Enums;
using Altinn.Correspondence.Core.Repositories;
using OneOf;
using System.Security.Claims;

namespace Altinn.Correspondence.Application.CheckNotification;

Expand All @@ -13,7 +14,7 @@ public CheckNotificationHandler(ICorrespondenceRepository correspondenceReposito
_correspondenceRepository = correspondenceRepository;
}

public async Task<OneOf<CheckNotificationResponse, Error>> Process(Guid correspondenceId, CancellationToken cancellationToken)
public async Task<OneOf<CheckNotificationResponse, Error>> Process(Guid correspondenceId, ClaimsPrincipal? user, CancellationToken cancellationToken)
{
var correspondence = await _correspondenceRepository.GetCorrespondenceById(correspondenceId, true, true, cancellationToken);
var response = new CheckNotificationResponse
Expand Down
5 changes: 3 additions & 2 deletions src/Altinn.Correspondence.Application/DownloadAttachment/DownloadAttachmentHandler.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Runtime.CompilerServices;
using System.Security.Claims;
using Altinn.Correspondence.Application.Helpers;
using Altinn.Correspondence.Core.Models.Enums;
using Altinn.Correspondence.Core.Repositories;
Expand All @@ -22,14 +23,14 @@ public DownloadAttachmentHandler(IAltinnAuthorizationService altinnAuthorization
_userClaimsHelper = userClaimsHelper;
}

public async Task<OneOf<Stream, Error>> Process(DownloadAttachmentRequest request, CancellationToken cancellationToken)
public async Task<OneOf<Stream, Error>> Process(DownloadAttachmentRequest request, ClaimsPrincipal? user, CancellationToken cancellationToken)
{
var attachment = await _attachmentRepository.GetAttachmentById(request.AttachmentId, false, cancellationToken);
if (attachment is null)
{
return Errors.AttachmentNotFound;
}
var hasAccess = await _altinnAuthorizationService.CheckUserAccess(attachment.ResourceId, new List<ResourceAccessLevel> { ResourceAccessLevel.Write }, cancellationToken);
var hasAccess = await _altinnAuthorizationService.CheckUserAccess(user, attachment.ResourceId, new List<ResourceAccessLevel> { ResourceAccessLevel.Write }, cancellationToken);
if (!hasAccess)
{
return Errors.NoAccessToResource;
Expand Down
Loading

0 comments on commit b6733f0

Please sign in to comment.