Skip to content

Commit

Permalink
fix: Collapse subject resource mappings before building sql query (#1579
Browse files Browse the repository at this point in the history
)

## Description

This implements a few optimizations for search authorization, which
particularly improves performance for users with access to a large
number of parties.
- SubjectsByParties gets collapsed and merged into ResourcesByParties.
This lets us avoid doing subselects in the OR-clauses added to the
search SQL
- The entire SubjectResources-table is cached, which lets us do the
above without an additional database hit.
- When constructing the SQL, parties with identical resource lists are
grouped, further collapsing multiple OR clauses into single ones with
two ANY checks. This vastly reduces the number of clauses for large
number of reportees, which in turn reduces the number of (identical)
query parameters being sent over the network.
- Minor improvements to data structures (mostly replacing List<T> with
HashSet<T>)

## Related Issue(s)

- N/A

## Verification

- [x] **Your** code builds clean without any errors or warnings
- [x] Manual testing done (required)
- [x] Relevant automated test added
  • Loading branch information
elsand authored Dec 12, 2024
1 parent b4440ca commit b39c376
Show file tree
Hide file tree
Showing 8 changed files with 271 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,15 @@
using System.Text;
using Digdir.Domain.Dialogporten.Application.Externals.AltinnAuthorization;
using Digdir.Domain.Dialogporten.Domain.Dialogs.Entities;
using Digdir.Domain.Dialogporten.Domain.SubjectResources;
using Microsoft.EntityFrameworkCore;

namespace Digdir.Domain.Dialogporten.Application.Common.Extensions;

public static class DbSetExtensions
{
public static IQueryable<DialogEntity> PrefilterAuthorizedDialogs(this DbSet<DialogEntity> dialogs, DialogSearchAuthorizationResult authorizedResources)
public static (string sql, object[] parameters) GeneratePrefilterAuthorizedDialogsSql(DialogSearchAuthorizationResult authorizedResources)
{
var parameters = new List<object>();

// lang=sql
var sb = new StringBuilder()
.AppendLine(CultureInfo.InvariantCulture, $"""
SELECT *
Expand All @@ -22,36 +19,50 @@ public static IQueryable<DialogEntity> PrefilterAuthorizedDialogs(this DbSet<Dia
""");
parameters.Add(authorizedResources.DialogIds);

foreach (var (party, resources) in authorizedResources.ResourcesByParties)
// Group parties that have the same resources
var groupedResult = authorizedResources.ResourcesByParties
.GroupBy(kv => kv.Value, new HashSetEqualityComparer<string>())
.ToDictionary(
g => g.Key,
g => new HashSet<string>(g.Select(kv => kv.Key))
);

foreach (var (resources, parties) in groupedResult)
{
// lang=sql
sb.AppendLine(CultureInfo.InvariantCulture, $"""
OR (
"{nameof(DialogEntity.Party)}" = @p{parameters.Count}
"{nameof(DialogEntity.Party)}" = ANY(@p{parameters.Count})
AND "{nameof(DialogEntity.ServiceResource)}" = ANY(@p{parameters.Count + 1})
)
""");
parameters.Add(party);
parameters.Add(parties);
parameters.Add(resources);
}

foreach (var (party, subjects) in authorizedResources.SubjectsByParties)
return (sb.ToString(), parameters.ToArray());
}

public static IQueryable<DialogEntity> PrefilterAuthorizedDialogs(this DbSet<DialogEntity> dialogs, DialogSearchAuthorizationResult authorizedResources)
{
var (sql, parameters) = GeneratePrefilterAuthorizedDialogsSql(authorizedResources);
return dialogs.FromSqlRaw(sql, parameters);
}
}


public sealed class HashSetEqualityComparer<T> : IEqualityComparer<HashSet<T>>
{
public bool Equals(HashSet<T>? x, HashSet<T>? y)
{
return ReferenceEquals(x, y) || (x is not null && y is not null && x.SetEquals(y));
}

public int GetHashCode(HashSet<T> obj)
{
ArgumentNullException.ThrowIfNull(obj);
unchecked
{
// lang=sql
sb.AppendLine(CultureInfo.InvariantCulture, $"""
OR (
"{nameof(DialogEntity.Party)}" = @p{parameters.Count}
AND "{nameof(DialogEntity.ServiceResource)}" = ANY(
SELECT "{nameof(SubjectResource.Resource)}"
FROM "{nameof(SubjectResource)}"
WHERE "{nameof(SubjectResource.Subject)}" = ANY(@p{parameters.Count + 1})
)
)
""");
parameters.Add(party);
parameters.Add(subjects);
return obj.Aggregate(0, (hash, item) => hash ^ (item?.GetHashCode() ?? 0));
}

return dialogs.FromSqlRaw(sb.ToString(), parameters.ToArray());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,10 @@ public sealed class DialogSearchAuthorizationResult
{
// Resources here are "main" resources, eg. something that represents an entry in the Resource Registry
// eg. "urn:altinn:resource:some-service" and referred to by "ServiceResource" in DialogEntity
public Dictionary<string, List<string>> ResourcesByParties { get; init; } = new();
public Dictionary<string, List<string>> SubjectsByParties { get; init; } = new();
public Dictionary<string, HashSet<string>> ResourcesByParties { get; init; } = new();
public List<Guid> DialogIds { get; init; } = [];

public bool HasNoAuthorizations =>
ResourcesByParties.Count == 0
&& DialogIds.Count == 0
&& SubjectsByParties.Count == 0;
&& DialogIds.Count == 0;
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@
using System.Text.Json.Serialization;
using Altinn.Authorization.ABAC.Xacml.JsonProfile;
using Digdir.Domain.Dialogporten.Application.Common.Extensions;
using Digdir.Domain.Dialogporten.Application.Externals;
using Digdir.Domain.Dialogporten.Application.Externals.AltinnAuthorization;
using Digdir.Domain.Dialogporten.Application.Externals.Presentation;
using Digdir.Domain.Dialogporten.Domain.Dialogs.Entities;
using Digdir.Domain.Dialogporten.Domain.Parties.Abstractions;
using Digdir.Domain.Dialogporten.Domain.SubjectResources;
using Digdir.Domain.Dialogporten.Infrastructure.Common.Exceptions;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging;
using ZiggyCreatures.Caching.Fusion;

Expand All @@ -20,7 +23,9 @@ internal sealed class AltinnAuthorizationClient : IAltinnAuthorization
private readonly HttpClient _httpClient;
private readonly IFusionCache _pdpCache;
private readonly IFusionCache _partiesCache;
private readonly IFusionCache _subjectResourcesCache;
private readonly IUser _user;
private readonly IDialogDbContext _dialogDbContext;
private readonly ILogger _logger;

private static readonly JsonSerializerOptions SerializerOptions = new()
Expand All @@ -33,12 +38,15 @@ public AltinnAuthorizationClient(
HttpClient client,
IFusionCacheProvider cacheProvider,
IUser user,
IDialogDbContext dialogDbContext,
ILogger<AltinnAuthorizationClient> logger)
{
_httpClient = client ?? throw new ArgumentNullException(nameof(client));
_pdpCache = cacheProvider.GetCache(nameof(Authorization)) ?? throw new ArgumentNullException(nameof(cacheProvider));
_partiesCache = cacheProvider.GetCache(nameof(AuthorizedPartiesResult)) ?? throw new ArgumentNullException(nameof(cacheProvider));
_subjectResourcesCache = cacheProvider.GetCache(nameof(SubjectResource)) ?? throw new ArgumentNullException(nameof(cacheProvider));
_user = user ?? throw new ArgumentNullException(nameof(user));
_dialogDbContext = dialogDbContext ?? throw new ArgumentNullException(nameof(dialogDbContext));
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
}

Expand Down Expand Up @@ -94,7 +102,6 @@ public async Task<bool> HasListAuthorizationForDialog(DialogEntity dialog, Cance
[dialog.Party], [dialog.ServiceResource], cancellationToken);

return authorizedResourcesForSearch.ResourcesByParties.Count > 0
|| authorizedResourcesForSearch.SubjectsByParties.Count > 0
|| authorizedResourcesForSearch.DialogIds.Contains(dialog.Id);
}

Expand Down Expand Up @@ -128,9 +135,9 @@ void Flatten(AuthorizedParty party, AuthorizedParty? parent = null)
}

private async Task<AuthorizedPartiesResult> PerformAuthorizedPartiesRequest(AuthorizedPartiesRequest authorizedPartiesRequest,
CancellationToken token)
CancellationToken cancellationToken)
{
var authorizedPartiesDto = await SendAuthorizedPartiesRequest(authorizedPartiesRequest, token);
var authorizedPartiesDto = await SendAuthorizedPartiesRequest(authorizedPartiesRequest, cancellationToken);
if (authorizedPartiesDto is null || authorizedPartiesDto.Count == 0)
{
throw new UpstreamServiceException("access-management returned no authorized parties, missing Altinn profile?");
Expand All @@ -139,10 +146,10 @@ private async Task<AuthorizedPartiesResult> PerformAuthorizedPartiesRequest(Auth
return AuthorizedPartiesHelper.CreateAuthorizedPartiesResult(authorizedPartiesDto, authorizedPartiesRequest);
}

private async Task<DialogSearchAuthorizationResult> PerformDialogSearchAuthorization(DialogSearchAuthorizationRequest request, CancellationToken token)
private async Task<DialogSearchAuthorizationResult> PerformDialogSearchAuthorization(DialogSearchAuthorizationRequest request, CancellationToken cancellationToken)
{
var partyIdentifier = request.Claims.GetEndUserPartyIdentifier() ?? throw new UnreachableException();
var authorizedParties = await GetAuthorizedParties(partyIdentifier, flatten: true, cancellationToken: token);
var authorizedParties = await GetAuthorizedParties(partyIdentifier, flatten: true, cancellationToken: cancellationToken);

if (request.ConstraintParties.Count > 0)
{
Expand All @@ -158,23 +165,27 @@ private async Task<DialogSearchAuthorizationResult> PerformDialogSearchAuthoriza
p => p.Party,
p => p.AuthorizedResources
.Where(r => request.ConstraintServiceResources.Count == 0 || request.ConstraintServiceResources.Contains(r))
.ToList())
.ToHashSet())
// Skip parties with no authorized resources
.Where(kv => kv.Value.Count != 0)
.ToDictionary(kv => kv.Key, kv => kv.Value),

SubjectsByParties = authorizedParties.AuthorizedParties
.ToDictionary(
p => p.Party,
p => p.AuthorizedRoles)
// Skip parties with no authorized roles
.Where(kv => kv.Value.Count != 0)
.ToDictionary(kv => kv.Key, kv => kv.Value)
};

await AuthorizationHelper.CollapseSubjectResources(
dialogSearchAuthorizationResult,
authorizedParties,
request.ConstraintServiceResources,
GetAllSubjectResources,
cancellationToken);

return dialogSearchAuthorizationResult;
}

private async Task<List<SubjectResource>> GetAllSubjectResources(CancellationToken cancellationToken) =>
await _subjectResourcesCache.GetOrSetAsync(nameof(SubjectResource), async ct
=> await _dialogDbContext.SubjectResources.ToListAsync(cancellationToken: ct),
token: cancellationToken);

private async Task<DialogDetailsAuthorizationResult> PerformDialogDetailsAuthorization(
DialogDetailsAuthorizationRequest request, CancellationToken cancellationToken)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
using Digdir.Domain.Dialogporten.Application.Externals.AltinnAuthorization;
using Digdir.Domain.Dialogporten.Domain.SubjectResources;

namespace Digdir.Domain.Dialogporten.Infrastructure.Altinn.Authorization;

internal static class AuthorizationHelper
{
public static async Task CollapseSubjectResources(
DialogSearchAuthorizationResult dialogSearchAuthorizationResult,
AuthorizedPartiesResult authorizedParties,
List<string> constraintResources,
Func<CancellationToken, Task<List<SubjectResource>>> getAllSubjectResources,
CancellationToken cancellationToken)
{
var authorizedPartiesWithRoles = authorizedParties.AuthorizedParties
.Where(p => p.AuthorizedRoles.Count != 0)
.ToList();

var uniqueSubjects = authorizedPartiesWithRoles
.SelectMany(p => p.AuthorizedRoles)
.ToHashSet();

var subjectResources = (await getAllSubjectResources(cancellationToken))
.Where(x => uniqueSubjects.Contains(x.Subject) && (constraintResources.Count == 0 || constraintResources.Contains(x.Resource))).ToList();

var subjectToResources = subjectResources
.GroupBy(sr => sr.Subject)
.ToDictionary(g => g.Key, g => g.Select(sr => sr.Resource).ToHashSet());

foreach (var partyEntry in authorizedPartiesWithRoles)
{
if (!dialogSearchAuthorizationResult.ResourcesByParties.TryGetValue(partyEntry.Party, out var resourceList))
{
resourceList = new HashSet<string>();
dialogSearchAuthorizationResult.ResourcesByParties[partyEntry.Party] = resourceList;
}

foreach (var subject in partyEntry.AuthorizedRoles)
{
if (subjectToResources.TryGetValue(subject, out var subjectResourceSet))
{
resourceList.UnionWith(subjectResourceSet);
}
}

if (resourceList.Count == 0)
{
dialogSearchAuthorizationResult.ResourcesByParties.Remove(partyEntry.Party);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,11 @@ public async Task<DialogSearchAuthorizationResult> GetAuthorizedResourcesForSear

// Keep the number of parties and resources reasonable
var allParties = dialogData.Select(x => x.Party).Distinct().Take(1000).ToList();
var allResources = dialogData.Select(x => x.ServiceResource).Distinct().Take(1000).ToList();
var allRoles = await _db.SubjectResources.Select(x => x.Subject).Distinct().Take(30).ToListAsync(cancellationToken);
var allResources = dialogData.Select(x => x.ServiceResource).Distinct().Take(1000).ToHashSet();

var authorizedResources = new DialogSearchAuthorizationResult
{
ResourcesByParties = allParties.ToDictionary(party => party, _ => allResources),
SubjectsByParties = allParties.ToDictionary(party => party, _ => allRoles)
ResourcesByParties = allParties.ToDictionary(party => party, _ => allResources)
};

return authorizedResources;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
using Digdir.Domain.Dialogporten.Application;
using Digdir.Domain.Dialogporten.Application.Common.Extensions;
using Digdir.Domain.Dialogporten.Application.Externals.AltinnAuthorization;
using Digdir.Domain.Dialogporten.Domain.SubjectResources;
using Digdir.Domain.Dialogporten.Infrastructure.Altinn.Authorization;
using Digdir.Domain.Dialogporten.Infrastructure.Altinn.Events;
using Digdir.Domain.Dialogporten.Infrastructure.Altinn.NameRegistry;
Expand Down Expand Up @@ -160,6 +161,10 @@ internal static void AddInfrastructure_Internal(InfrastructureBuilderContext bui
// Timeout for the cache to wait for the factory to complete, which when reached without fail-safe data
// will cause an exception to be thrown
FactoryHardTimeout = TimeSpan.FromSeconds(10)
})
.ConfigureFusionCache(nameof(SubjectResource), new()
{
Duration = TimeSpan.FromMinutes(20)
});

if (!environment.IsDevelopment())
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
using Digdir.Domain.Dialogporten.Application.Common.Extensions;
using Digdir.Domain.Dialogporten.Application.Externals.AltinnAuthorization;
using FluentAssertions;

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

public class DbSetExtensionsTests
{
[Fact]
public void PrefilterAuthorizedDialogs_GeneratesExpectedSql_ForGroupedParties()
{
// Arrange
var authorizedResources = new DialogSearchAuthorizationResult
{
DialogIds = [Guid.CreateVersion7()],
ResourcesByParties = new Dictionary<string, HashSet<string>>
{
{ "Party1", ["Resource1", "Resource2"] },
{ "Party2", ["Resource1", "Resource2"] },
{ "Party3", ["Resource1", "Resource2", "Resource3"] },
{ "Party4", ["Resource3"] },
{ "Party5", ["Resource4"] }
}
};

var expectedSql = """
SELECT *
FROM "Dialog"
WHERE "Id" = ANY(@p0)
OR (
"Party" = ANY(@p1)
AND "ServiceResource" = ANY(@p2)
)
OR (
"Party" = ANY(@p3)
AND "ServiceResource" = ANY(@p4)
)
OR (
"Party" = ANY(@p5)
AND "ServiceResource" = ANY(@p6)
)
OR (
"Party" = ANY(@p7)
AND "ServiceResource" = ANY(@p8)
)
""";
var expectedParameters = new object[]
{
authorizedResources.DialogIds,
new HashSet<string> { "Party1", "Party2" },
new HashSet<string> { "Resource1", "Resource2" },
new HashSet<string> { "Party3" },
new HashSet<string> { "Resource1", "Resource2", "Resource3" },
new HashSet<string> { "Party4" },
new HashSet<string> { "Resource3" },
new HashSet<string> { "Party5" },
new HashSet<string> { "Resource4" }
};

// Act
var (actualSql, actualParameters) = DbSetExtensions.GeneratePrefilterAuthorizedDialogsSql(authorizedResources);

// Assert
RemoveWhitespace(actualSql).Should().Be(RemoveWhitespace(expectedSql));
actualParameters.Should().BeEquivalentTo(expectedParameters);
}

private static string RemoveWhitespace(string input)
{
return string.Concat(input.Where(c => !char.IsWhiteSpace(c)));
}
}
Loading

0 comments on commit b39c376

Please sign in to comment.