From dbb79dc26cefc5f28c21a738f39199c36a49438f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Dybvik=20Langfors?= Date: Tue, 1 Oct 2024 15:49:52 +0200 Subject: [PATCH] fix: Add separate settings for parties cache, don't cache invalid response from Altinn 2 (#1194) ## Description Currently, if the user is missing a profile in Altinn 2, the reportee list returned from all auth/am APIs are empty. This is an error condition, so we treat it as such and don't cache the empty response. Also, this introduces a separate cache setting for parties, as this does not have the cardinality issues as the PDP cache, allowing us to utilize memory cache for this. ## Related Issue(s) - N/A ## Verification - [x] **Your** code builds clean without any errors or warnings - [x] Manual testing done (required) - [ ] Relevant automated test added (if you find this hard, leave it and we'll help out) ## Summary by CodeRabbit - **New Features** - Enhanced caching mechanism for authorized parties, improving performance and response times. - Introduced a new exception handling mechanism for upstream service errors. - **Bug Fixes** - Improved error handling in the `UpstreamServiceException` class to allow custom messages. - **Documentation** - Updated configuration parameters for caching authorized parties to ensure clarity on retention and timeout settings. --- .../Authorization/AltinnAuthorizationClient.cs | 18 +++++++++++++----- .../Exceptions/UpstreamServiceException.cs | 7 +++++-- .../InfrastructureExtensions.cs | 17 +++++++++++++++++ 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/AltinnAuthorizationClient.cs b/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/AltinnAuthorizationClient.cs index 54df56cc6..dfedb905f 100644 --- a/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/AltinnAuthorizationClient.cs +++ b/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/AltinnAuthorizationClient.cs @@ -8,6 +8,7 @@ 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.Infrastructure.Common.Exceptions; using Microsoft.Extensions.Logging; using ZiggyCreatures.Caching.Fusion; @@ -18,7 +19,8 @@ internal sealed class AltinnAuthorizationClient : IAltinnAuthorization private const string AuthorizedPartiesUrl = "/accessmanagement/api/v1/resourceowner/authorizedparties?includeAltinn2=true"; private readonly HttpClient _httpClient; - private readonly IFusionCache _cache; + private readonly IFusionCache _pdpCache; + private readonly IFusionCache _partiesCache; private readonly IUser _user; private readonly ILogger _logger; @@ -35,7 +37,8 @@ public AltinnAuthorizationClient( ILogger logger) { _httpClient = client ?? throw new ArgumentNullException(nameof(client)); - _cache = cacheProvider.GetCache(nameof(Authorization)) ?? throw new ArgumentNullException(nameof(cacheProvider)); + _pdpCache = cacheProvider.GetCache(nameof(Authorization)) ?? throw new ArgumentNullException(nameof(cacheProvider)); + _partiesCache = cacheProvider.GetCache(nameof(AuthorizedPartiesResult)) ?? throw new ArgumentNullException(nameof(cacheProvider)); _user = user ?? throw new ArgumentNullException(nameof(user)); _logger = logger ?? throw new ArgumentNullException(nameof(logger)); } @@ -54,7 +57,7 @@ public async Task GetDialogDetailsAuthorizatio AltinnActions = dialogEntity.GetAltinnActions() }; - return await _cache.GetOrSetAsync(request.GenerateCacheKey(), async token + return await _pdpCache.GetOrSetAsync(request.GenerateCacheKey(), async token => await PerformDialogDetailsAuthorization(request, token), token: cancellationToken); } @@ -72,7 +75,7 @@ public async Task GetAuthorizedResourcesForSear ConstraintServiceResources = serviceResources }; - return await _cache.GetOrSetAsync(request.GenerateCacheKey(), async token + return await _pdpCache.GetOrSetAsync(request.GenerateCacheKey(), async token => await PerformDialogSearchAuthorization(request, token), token: cancellationToken); } @@ -80,7 +83,7 @@ public async Task GetAuthorizedParties(IPartyIdentifier CancellationToken cancellationToken = default) { var authorizedPartiesRequest = new AuthorizedPartiesRequest(authenticatedParty); - var authorizedParties = await _cache.GetOrSetAsync(authorizedPartiesRequest.GenerateCacheKey(), async token + var authorizedParties = await _partiesCache.GetOrSetAsync(authorizedPartiesRequest.GenerateCacheKey(), async token => await PerformAuthorizedPartiesRequest(authorizedPartiesRequest, token), token: cancellationToken); return flatten ? GetFlattenedAuthorizedParties(authorizedParties) : authorizedParties; @@ -118,6 +121,11 @@ private async Task PerformAuthorizedPartiesRequest(Auth CancellationToken token) { var authorizedPartiesDto = await SendAuthorizedPartiesRequest(authorizedPartiesRequest, token); + if (authorizedPartiesDto is null || authorizedPartiesDto.Count == 0) + { + throw new UpstreamServiceException("access-management returned no authorized parties, missing Altinn profile?"); + } + return AuthorizedPartiesHelper.CreateAuthorizedPartiesResult(authorizedPartiesDto, authorizedPartiesRequest); } diff --git a/src/Digdir.Domain.Dialogporten.Infrastructure/Common/Exceptions/UpstreamServiceException.cs b/src/Digdir.Domain.Dialogporten.Infrastructure/Common/Exceptions/UpstreamServiceException.cs index 63d21111a..8b2e5fcea 100644 --- a/src/Digdir.Domain.Dialogporten.Infrastructure/Common/Exceptions/UpstreamServiceException.cs +++ b/src/Digdir.Domain.Dialogporten.Infrastructure/Common/Exceptions/UpstreamServiceException.cs @@ -1,5 +1,8 @@ namespace Digdir.Domain.Dialogporten.Infrastructure.Common.Exceptions; public interface IUpstreamServiceError; -internal sealed class UpstreamServiceException(Exception innerException) - : Exception(innerException.Message, innerException), IUpstreamServiceError; +internal sealed class UpstreamServiceException : Exception, IUpstreamServiceError +{ + public UpstreamServiceException(Exception innerException) : base(innerException.Message, innerException) { } + public UpstreamServiceException(string message) : base(message) { } +} diff --git a/src/Digdir.Domain.Dialogporten.Infrastructure/InfrastructureExtensions.cs b/src/Digdir.Domain.Dialogporten.Infrastructure/InfrastructureExtensions.cs index 0b40eff71..a6cd65d54 100644 --- a/src/Digdir.Domain.Dialogporten.Infrastructure/InfrastructureExtensions.cs +++ b/src/Digdir.Domain.Dialogporten.Infrastructure/InfrastructureExtensions.cs @@ -112,6 +112,23 @@ public static IServiceCollection AddInfrastructure(this IServiceCollection servi // 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(AuthorizedPartiesResult), new() + { + // We keep authorized parties in a separate cache key, as this originates from a different API + // and has lees cardinality than the dialog authorization cache (only one per user). We therefore + // allow a memory cache for this. + Duration = TimeSpan.FromMinutes(15), + // In case Altinn Access Management is down/overloaded, we allow the re-usage of stale authorization data + // for an additional 15 minutes. Using default FailSafeThrottleDuration. + FailSafeMaxDuration = TimeSpan.FromMinutes(30), + // If the request to Altinn AccessManagement takes too long, we allow the cache to return stale data + // temporarily whilst updating the cache in the background. Note that we are also using eager refresh + // and a backplane. + FactorySoftTimeout = TimeSpan.FromSeconds(2), + // 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) }); services.AddDbContext((services, options) =>