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

chore: Add authorization checks to transmissions #954

Merged
merged 9 commits into from
Aug 12, 2024
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
using Digdir.Domain.Dialogporten.Application.Common.Authorization;
using Digdir.Domain.Dialogporten.Application.Features.V1.EndUser.Dialogs.Queries.Get;
using Digdir.Domain.Dialogporten.Domain.Dialogs.Entities.Transmissions;

namespace Digdir.Domain.Dialogporten.Application.Externals.AltinnAuthorization;

Expand All @@ -12,17 +14,15 @@ public sealed class DialogDetailsAuthorizationResult
public bool HasReadAccessToMainResource() =>
AuthorizedAltinnActions.Contains(new(Constants.ReadAction, Constants.MainResource));

// TODO: Rename in https://github.com/digdir/dialogporten/issues/860
// public bool HasReadAccessToDialogTransmission(DialogTransmission dialogTransmission) =>
// HasReadAccessToDialogTransmission(dialogTransmission.AuthorizationAttribute);
//
// public bool HasReadAccessToDialogTransmission(GetDialogDialogTransmissionDto dialogTransmission) =>
// HasReadAccessToDialogTransmission(dialogTransmission.AuthorizationAttribute);
public bool HasReadAccessToDialogTransmission(DialogTransmission dialogTransmission) =>
HasReadAccessToDialogTransmission(dialogTransmission.AuthorizationAttribute);

public bool HasReadAccessToDialogTransmission(GetDialogDialogTransmissionDto dialogTransmission) =>
HasReadAccessToDialogTransmission(dialogTransmission.AuthorizationAttribute);

private bool HasReadAccessToDialogTransmission(string? authorizationAttribute)
{
return authorizationAttribute is not null
// TODO: Rename in https://github.com/digdir/dialogporten/issues/860
? ( // Dialog transmissions are authorized by either the read or read action, depending on the authorization attribute type
// The infrastructure will ensure that the correct action is used, so here we just check for either
AuthorizedAltinnActions.Contains(new(Constants.TransmissionReadAction, authorizationAttribute))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
using AutoMapper;
using Digdir.Domain.Dialogporten.Application.Common;
using Digdir.Domain.Dialogporten.Application.Common.ReturnTypes;
using Digdir.Domain.Dialogporten.Application.Common.Authorization;
using Digdir.Domain.Dialogporten.Application.Externals;
using Digdir.Domain.Dialogporten.Application.Externals.AltinnAuthorization;
using Digdir.Domain.Dialogporten.Domain.Dialogs.Entities;
using Digdir.Domain.Dialogporten.Domain.Dialogs.Entities.Transmissions;
using MediatR;
Expand All @@ -23,13 +25,13 @@ internal sealed class GetDialogTransmissionQueryHandler : IRequestHandler<GetDia
{
private readonly IMapper _mapper;
private readonly IDialogDbContext _dbContext;
private readonly IUserResourceRegistry _userResourceRegistry;
private readonly IAltinnAuthorization _altinnAuthorization;

public GetDialogTransmissionQueryHandler(IMapper mapper, IDialogDbContext dbContext, IUserResourceRegistry userResourceRegistry)
public GetDialogTransmissionQueryHandler(IMapper mapper, IDialogDbContext dbContext, IAltinnAuthorization altinnAuthorization)
{
_mapper = mapper ?? throw new ArgumentNullException(nameof(mapper));
_dbContext = dbContext ?? throw new ArgumentNullException(nameof(dbContext));
_userResourceRegistry = userResourceRegistry ?? throw new ArgumentNullException(nameof(userResourceRegistry));
_altinnAuthorization = altinnAuthorization ?? throw new ArgumentNullException(nameof(altinnAuthorization));
}

public async Task<GetDialogTransmissionResult> Handle(GetDialogTransmissionQuery request,
Expand All @@ -56,16 +58,38 @@ public async Task<GetDialogTransmissionResult> Handle(GetDialogTransmissionQuery
return new EntityNotFound<DialogEntity>(request.DialogId);
}

var authorizationResult = await _altinnAuthorization.GetDialogDetailsAuthorization(
dialog,
cancellationToken);

// If we cannot read the dialog at all, we don't allow access to any of the dialog transmissions.
if (!authorizationResult.HasReadAccessToMainResource())
{
return new EntityNotFound<DialogEntity>(request.DialogId);
}

if (dialog.Deleted)
{
return new EntityDeleted<DialogEntity>(request.DialogId);
}

var transmission = dialog.Transmissions.FirstOrDefault();
if (transmission is null)
{
return new EntityNotFound<DialogTransmission>(request.TransmissionId);
}

var dto = _mapper.Map<GetDialogTransmissionDto>(transmission);
dto.IsAuthorized = authorizationResult.HasReadAccessToDialogTransmission(transmission);

if (dto.IsAuthorized) return dto;

var urls = transmission.Attachments.SelectMany(a => a.Urls).ToList();
foreach (var url in urls)
{
url.Url = Constants.UnauthorizedUri;
}

// TODO: Check auth
return transmission is null
? (GetDialogTransmissionResult)new EntityNotFound<DialogTransmission>(request.TransmissionId)
: _mapper.Map<GetDialogTransmissionDto>(transmission);
return dto;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ public sealed class SearchDialogTransmissionDto
public Guid Id { get; set; }
public DateTimeOffset CreatedAt { get; set; }
public string? AuthorizationAttribute { get; set; }
public bool IsAuthorized { get; set; }
public string? ExtendedType { get; set; }
public Guid? RelatedTransmissionId { get; set; }
public DateTimeOffset? DeletedAt { get; set; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using Digdir.Domain.Dialogporten.Application.Common;
using Digdir.Domain.Dialogporten.Application.Common.ReturnTypes;
using Digdir.Domain.Dialogporten.Application.Externals;
using Digdir.Domain.Dialogporten.Application.Externals.AltinnAuthorization;
using Digdir.Domain.Dialogporten.Domain.Dialogs.Entities;
using MediatR;
using Microsoft.EntityFrameworkCore;
Expand All @@ -21,19 +22,17 @@ internal sealed class SearchDialogTransmissionQueryHandler : IRequestHandler<Sea
{
private readonly IDialogDbContext _db;
private readonly IMapper _mapper;
private readonly IUserResourceRegistry _userResourceRegistry;
private readonly IAltinnAuthorization _altinnAuthorization;

public SearchDialogTransmissionQueryHandler(IDialogDbContext db, IMapper mapper, IUserResourceRegistry userResourceRegistry)
public SearchDialogTransmissionQueryHandler(IDialogDbContext db, IMapper mapper, IAltinnAuthorization altinnAuthorization)
{
_db = db ?? throw new ArgumentNullException(nameof(db));
_mapper = mapper ?? throw new ArgumentNullException(nameof(mapper));
_userResourceRegistry = userResourceRegistry ?? throw new ArgumentNullException(nameof(userResourceRegistry));
_altinnAuthorization = altinnAuthorization ?? throw new ArgumentNullException(nameof(altinnAuthorization));
}

public async Task<SearchDialogTransmissionResult> Handle(SearchDialogTransmissionQuery request, CancellationToken cancellationToken)
{
var resourceIds = await _userResourceRegistry.GetCurrentUserResourceIds(cancellationToken);

var dialog = await _db.Dialogs
.Include(x => x.Transmissions)
.ThenInclude(x => x.Content.OrderBy(x => x.Id).ThenBy(x => x.CreatedAt))
Expand All @@ -47,7 +46,6 @@ public async Task<SearchDialogTransmissionResult> Handle(SearchDialogTransmissio
.Include(x => x.Transmissions)
.ThenInclude(x => x.Sender)
.IgnoreQueryFilters()
.Where(x => resourceIds.Contains(x.ServiceResource))
.FirstOrDefaultAsync(x => x.Id == request.DialogId,
cancellationToken: cancellationToken);

Expand All @@ -56,13 +54,21 @@ public async Task<SearchDialogTransmissionResult> Handle(SearchDialogTransmissio
return new EntityNotFound<DialogEntity>(request.DialogId);
}

var authorizationResult = await _altinnAuthorization.GetDialogDetailsAuthorization(
dialog,
cancellationToken);

// If we cannot read the dialog at all, we don't allow access to any of the activity history
if (!authorizationResult.HasReadAccessToMainResource())
{
return new EntityNotFound<DialogEntity>(request.DialogId);
}

if (dialog.Deleted)
{
return new EntityDeleted<DialogEntity>(request.DialogId);
}

// TODO: Check auth

return _mapper.Map<List<SearchDialogTransmissionDto>>(dialog.Transmissions);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -166,14 +166,11 @@ private static void DecorateWithAuthorization(GetDialogDto dto,
}
}

// TODO: Rename in https://github.com/digdir/dialogporten/issues/860
// foreach (var transmission in dto.Transmissions)
// {
// if (authorizationResult.HasReadAccessToDialogTransmission(transmission))
// {
// transmission.IsAuthorized = true;
// }
// }
var authorizedTransmissions = dto.Transmissions.Where(authorizationResult.HasReadAccessToDialogTransmission);
foreach (var transmission in authorizedTransmissions)
{
transmission.IsAuthorized = true;
}
}
}

Expand All @@ -193,9 +190,13 @@ private static void ReplaceUnauthorizedUrls(GetDialogDto dto)
}
}

// // Attachment URLs
// foreach (var dialogTransmission in dto.Transmissions.Where(e => !e.IsAuthorized))
// {
// }
foreach (var dialogTransmission in dto.Transmissions.Where(e => !e.IsAuthorized))
{
var urls = dialogTransmission.Attachments.SelectMany(a => a.Urls).ToList();
foreach (var url in urls)
{
url.Url = Constants.UnauthorizedUri;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,9 @@ static bool IsExternalResource(string? resource)
serviceResourceReferences.AddRange(request.GuiActions
.Where(action => IsExternalResource(action.AuthorizationAttribute))
.Select(action => action.AuthorizationAttribute!));
serviceResourceReferences.AddRange(request.Transmissions
.Where(transmission => IsExternalResource(transmission.AuthorizationAttribute))
.Select(transmission => transmission.AuthorizationAttribute!));

return serviceResourceReferences.Distinct().ToList();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,9 @@ static bool IsExternalResource(string? resource)
serviceResourceReferences.AddRange(request.GuiActions
.Where(action => IsExternalResource(action.AuthorizationAttribute))
.Select(action => action.AuthorizationAttribute!));
serviceResourceReferences.AddRange(request.Transmissions
.Where(transmission => IsExternalResource(transmission.AuthorizationAttribute))
.Select(transmission => transmission.AuthorizationAttribute!));

return serviceResourceReferences.Distinct().ToList();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,9 @@ public static List<AltinnAction> GetAltinnActions(this DialogEntity dialogEntity
.Select(x => new AltinnAction(x.Action, x.AuthorizationAttribute))
.Concat(dialogEntity.GuiActions
.Select(x => new AltinnAction(x.Action, x.AuthorizationAttribute)))
// TODO: Rename in https://github.com/digdir/dialogporten/issues/860
// .Concat(dialogEntity.Attachments
// .Where(x => x.AuthorizationAttribute is not null)
// .Select(x => new AltinnAction(GetReadActionForAuthorizationAttribute(x.AuthorizationAttribute!), x.AuthorizationAttribute)))
.Concat(dialogEntity.Transmissions
.Where(x => x.AuthorizationAttribute is not null)
.Select(x => new AltinnAction(GetReadActionForAuthorizationAttribute(x.AuthorizationAttribute!), x.AuthorizationAttribute)))
// We always need to check if the user can read the main resource
.Append(new AltinnAction(Constants.ReadAction, Constants.MainResource))
.ToList();
Expand All @@ -28,7 +27,6 @@ private static string GetReadActionForAuthorizationAttribute(string authorizatio
{
// Resource attributes may refer to either subresources/tasks that should be considered just another
// attribute to be matched within the same policy file, or they may refer to separate resources (and policies).
// TODO: Rename in https://github.com/digdir/dialogporten/issues/860
// In the former case, we need to use "transmissionread" as the action, as having "read" on the main resource would
// also give access to the subresource/task. In the latter case, we should use "read", as the resource is a
// separate entity.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@ public class CreateDialogTests
public async Task CreateDialogCommand_Should_Return_Forbidden_When_Scope_Is_Missing()
{
// Arrange
var dialogDbContextSub = Substitute.For<Externals.IDialogDbContext>();
var dialogDbContextSub = Substitute.For<IDialogDbContext>();

var mapper = new MapperConfiguration(cfg =>
{
cfg.AddMaps(typeof(CreateDialogCommandHandler).Assembly);
}).CreateMapper();

var unitOfWorkSub = Substitute.For<Externals.IUnitOfWork>();
var unitOfWorkSub = Substitute.For<IUnitOfWork>();
var domainContextSub = Substitute.For<IDomainContext>();
var userResourceRegistrySub = Substitute.For<IUserResourceRegistry>();
var userOrganizationRegistrySub = Substitute.For<IUserOrganizationRegistry>();
Expand Down Expand Up @@ -54,14 +54,14 @@ public async Task CreateDialogCommand_Should_Return_Forbidden_When_Scope_Is_Miss
public async Task CreateDialogCommand_Should_Return_ValidationError_When_Progress_Set_On_Correspondence()
{
// Arrange
var dialogDbContextSub = Substitute.For<Externals.IDialogDbContext>();
var dialogDbContextSub = Substitute.For<IDialogDbContext>();

var mapper = new MapperConfiguration(cfg =>
{
cfg.AddMaps(typeof(CreateDialogCommandHandler).Assembly);
}).CreateMapper();

var unitOfWorkSub = Substitute.For<Externals.IUnitOfWork>();
var unitOfWorkSub = Substitute.For<IUnitOfWork>();
var domainContextSub = Substitute.For<IDomainContext>();
var userResourceRegistrySub = Substitute.For<IUserResourceRegistry>();
var userOrganizationRegistrySub = Substitute.For<IUserOrganizationRegistry>();
Expand Down Expand Up @@ -90,4 +90,38 @@ public async Task CreateDialogCommand_Should_Return_ValidationError_When_Progres
Assert.Equal(CreateDialogCommandHandler.ProgressValidationFailure.ErrorMessage,
result.AsT2.Errors.First().ErrorMessage);
}

[Fact]
public async Task CreateDialogCommand_Should_Return_Forbidden_When_User_Is_Not_Owner()
{
// Arrange
var dialogDbContextSub = Substitute.For<IDialogDbContext>();

var mapper = new MapperConfiguration(cfg =>
{
cfg.AddMaps(typeof(CreateDialogCommandHandler).Assembly);
}).CreateMapper();

var unitOfWorkSub = Substitute.For<IUnitOfWork>();
var domainContextSub = Substitute.For<IDomainContext>();
var userResourceRegistrySub = Substitute.For<IUserResourceRegistry>();
var userOrganizationRegistrySub = Substitute.For<IUserOrganizationRegistry>();
var partyNameRegistrySub = Substitute.For<IPartyNameRegistry>();

var createCommand = DialogGenerator.GenerateSimpleFakeDialog();

userResourceRegistrySub
.CurrentUserIsOwner(createCommand.ServiceResource, Arg.Any<CancellationToken>())
.Returns(false);

var commandHandler = new CreateDialogCommandHandler(dialogDbContextSub,
mapper, unitOfWorkSub, domainContextSub, userResourceRegistrySub,
userOrganizationRegistrySub, partyNameRegistrySub);

// Act
var result = await commandHandler.Handle(createCommand, CancellationToken.None);

// Assert
Assert.True(result.IsT3); // Forbidden
}
}
Original file line number Diff line number Diff line change
@@ -1,30 +1,28 @@
using Digdir.Domain.Dialogporten.Application.Common.Authorization;
using Digdir.Domain.Dialogporten.Domain.Dialogs.Entities;
using Digdir.Domain.Dialogporten.Domain.Dialogs.Entities.Actions;
using Digdir.Domain.Dialogporten.Infrastructure.Altinn.Authorization;
using Xunit;

namespace Digdir.Domain.Dialogporten.Infrastructure.Unit.Tests;

public class DialogEntityExtensionsTests
{
[Fact(Skip = "Re-enable when transmission is implemented, https://github.com/digdir/dialogporten/issues/860")]
[Fact]
public void GetAltinnActionsShouldReturnCorrectActionsForTransmissionAuthorizationAttributes()
{
// Arrange
var dialogEntity = new DialogEntity
{
ApiActions = new List<DialogApiAction>(),
GuiActions = new List<DialogGuiAction>(),
// TODO: Rename in https://github.com/digdir/dialogporten/issues/860
// Transmissions = new List<Transmission>
// {
// new() { AuthorizationAttribute = "foo" },
// new() { AuthorizationAttribute = "urn:altinn:subresource:bar" },
// new() { AuthorizationAttribute = "urn:altinn:task:Task_1" },
// new() { AuthorizationAttribute = "urn:altinn:resource:some-service:element1" },
// new() { AuthorizationAttribute = "urn:altinn:resource:app_ttd_some-app" }
// }
ApiActions = [],
GuiActions = [],
Transmissions =
[
new() { AuthorizationAttribute = "foo" },
new() { AuthorizationAttribute = "urn:altinn:subresource:bar" },
new() { AuthorizationAttribute = "urn:altinn:task:Task_1" },
new() { AuthorizationAttribute = "urn:altinn:resource:some-service:element1" },
new() { AuthorizationAttribute = "urn:altinn:resource:app_ttd_some-app" }
]
};

// Act
Expand All @@ -33,10 +31,9 @@ public void GetAltinnActionsShouldReturnCorrectActionsForTransmissionAuthorizati
// Assert
Assert.NotNull(actions);
Assert.NotEmpty(actions);
// TODO: Rename in https://github.com/digdir/dialogporten/issues/860
// Assert.Contains(actions, a => a is { Name: Constants.TransmissionReadAction, AuthorizationAttribute: "foo" });
// Assert.Contains(actions, a => a is { Name: Constants.TransmissionReadAction, AuthorizationAttribute: "urn:altinn:subresource:bar" });
// Assert.Contains(actions, a => a is { Name: Constants.TransmissionReadAction, AuthorizationAttribute: "urn:altinn:task:Task_1" });
Assert.Contains(actions, a => a is { Name: Constants.TransmissionReadAction, AuthorizationAttribute: "foo" });
Assert.Contains(actions, a => a is { Name: Constants.TransmissionReadAction, AuthorizationAttribute: "urn:altinn:subresource:bar" });
Assert.Contains(actions, a => a is { Name: Constants.TransmissionReadAction, AuthorizationAttribute: "urn:altinn:task:Task_1" });
Assert.Contains(actions, a => a is { Name: Constants.ReadAction, AuthorizationAttribute: "urn:altinn:resource:some-service:element1" });
Assert.Contains(actions, a => a is { Name: Constants.ReadAction, AuthorizationAttribute: "urn:altinn:resource:app_ttd_some-app" });
}
Expand Down
Loading
Loading