Skip to content

Commit

Permalink
fix: Add separate settings for parties cache, don't cache invalid res…
Browse files Browse the repository at this point in the history
…ponse 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)


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## 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.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
elsand authored Oct 1, 2024
1 parent 28a9c8b commit dbb79dc
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;

Expand All @@ -35,7 +37,8 @@ public AltinnAuthorizationClient(
ILogger<AltinnAuthorizationClient> 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));
}
Expand All @@ -54,7 +57,7 @@ public async Task<DialogDetailsAuthorizationResult> 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);
}

Expand All @@ -72,15 +75,15 @@ public async Task<DialogSearchAuthorizationResult> 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);
}

public async Task<AuthorizedPartiesResult> GetAuthorizedParties(IPartyIdentifier authenticatedParty, bool flatten = false,
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;
Expand Down Expand Up @@ -118,6 +121,11 @@ private async Task<AuthorizedPartiesResult> 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);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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) { }
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<DialogDbContext>((services, options) =>
Expand Down

0 comments on commit dbb79dc

Please sign in to comment.