From f73f39ec3093b8113c00937366a69e951191f913 Mon Sep 17 00:00:00 2001 From: Martin Othamar Date: Fri, 14 Feb 2025 10:26:41 +0100 Subject: [PATCH] Fix case where querying for roles for a user without any roles threw exception (#1104) --- .../Authorization/AuthorizationClient.cs | 4 + .../Authorization/AuthorizationClientTests.cs | 224 +++++++++--------- 2 files changed, 117 insertions(+), 111 deletions(-) diff --git a/src/Altinn.App.Core/Infrastructure/Clients/Authorization/AuthorizationClient.cs b/src/Altinn.App.Core/Infrastructure/Clients/Authorization/AuthorizationClient.cs index 121ccd0d3..d5077e0f2 100644 --- a/src/Altinn.App.Core/Infrastructure/Clients/Authorization/AuthorizationClient.cs +++ b/src/Altinn.App.Core/Infrastructure/Clients/Authorization/AuthorizationClient.cs @@ -1,3 +1,4 @@ +using System.Net; using System.Net.Http.Headers; using System.Security.Claims; using Altinn.App.Core.Configuration; @@ -198,6 +199,9 @@ public async Task> GetUserRoles(int userId, int userPartyId) try { HttpResponseMessage response = await _client.GetAsync(token, apiUrl); + if (response.StatusCode == HttpStatusCode.NotFound) + return roles; + if (response.IsSuccessStatusCode) { string responseContent = await response.Content.ReadAsStringAsync(); diff --git a/test/Altinn.App.Core.Tests/Infrastructure/Clients/Authorization/AuthorizationClientTests.cs b/test/Altinn.App.Core.Tests/Infrastructure/Clients/Authorization/AuthorizationClientTests.cs index e909ff5ce..42d82d5f6 100644 --- a/test/Altinn.App.Core.Tests/Infrastructure/Clients/Authorization/AuthorizationClientTests.cs +++ b/test/Altinn.App.Core.Tests/Infrastructure/Clients/Authorization/AuthorizationClientTests.cs @@ -1,21 +1,24 @@ -#nullable disable using System.Security.Claims; using System.Text.Json; using Altinn.App.Common.Tests; using Altinn.App.Core.Configuration; using Altinn.App.Core.Constants; -using Altinn.App.Core.Features; using Altinn.App.Core.Infrastructure.Clients.Authorization; +using Altinn.App.Core.Internal.Auth; using Altinn.Authorization.ABAC.Xacml.JsonProfile; using Altinn.Common.PEP.Interfaces; using Altinn.Platform.Storage.Interface.Models; using Authorization.Platform.Authorization.Models; using FluentAssertions; +using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; using Moq; -using Moq.Protected; +using WireMock.RequestBuilders; +using WireMock.ResponseBuilders; +using WireMock.Server; namespace Altinn.App.Core.Tests.Infrastructure.Clients.Authorization; @@ -113,134 +116,131 @@ public async Task AuthorizeActions_returns_empty_dictionary_if_no_response_from_ } [Fact] - public async Task GetUserRolesAsync_returns_roles_on_success() + public async Task GetUserRoles_Handles_200() { - var userId = 1337; - var userPartyId = 2001; - var expectedRoles = new List() - { - new() { Type = "altinn", Value = "bobet" }, - new() { Type = "altinn", Value = "bobes" }, - }; - - var responseJson = JsonSerializer.Serialize(expectedRoles); - - var httpMessageHandler = new Mock(); - - httpMessageHandler - .Protected() - .Setup>( - "SendAsync", - ItExpr.Is(m => - m.RequestUri != null - && m.RequestUri.ToString() - .Contains($"roles?coveredByUserId={userId}&offeredByPartyId={userPartyId}") - ), - ItExpr.IsAny() - ) - .ReturnsAsync( - new HttpResponseMessage - { - StatusCode = System.Net.HttpStatusCode.OK, - Content = new StringContent(responseJson, System.Text.Encoding.UTF8, "application/json"), - } + await using var fixture = Fixture.Create(); + + Role[] expectedRoles = + [ + new Role { Type = "altinn", Value = "bobet" }, + new Role { Type = "altinn", Value = "bobes" }, + ]; + + var server = fixture.Server; + server + .Given(Request.Create().WithPath(Fixture.ApiPath).UsingGet()) + .RespondWith( + Response + .Create() + .WithStatusCode(200) + .WithHeader("Content-Type", "application/json") + .WithBodyAsJson(expectedRoles) ); - var httpClient = new HttpClient(httpMessageHandler.Object); - - TelemetrySink telemetrySink = new(); - var pdpMock = new Mock(); - var httpContext = new DefaultHttpContext(); - httpContext.Request.Headers["Cookie"] = "AltinnStudioRuntime=myFakeJwtToken"; + var actualRoles = await fixture.Client.GetUserRoles(1337, 2001); - var httpContextAccessorMock = new Mock(); - httpContextAccessorMock.Setup(_ => _.HttpContext).Returns(httpContext); + Assert.Equivalent(expectedRoles, actualRoles); + } - var appSettingsMock = new Mock>(); - appSettingsMock - .Setup(s => s.CurrentValue) - .Returns(new AppSettings { RuntimeCookieName = "AltinnStudioRuntime" }); + [Fact] + public async Task GetUserRoles_Handles_404() + { + await using var fixture = Fixture.Create(); - var platformSettings = Options.Create( - new PlatformSettings - { - ApiAuthorizationEndpoint = "http://authorization.test/", - SubscriptionKey = "dummyKey", - } - ); - var logger = NullLogger.Instance; + Role[] expectedRoles = []; - var client = new AuthorizationClient( - platformSettings, - httpContextAccessorMock.Object, - httpClient, - appSettingsMock.Object, - pdpMock.Object, - logger, - telemetrySink.Object - ); + var server = fixture.Server; + server + .Given(Request.Create().WithPath(Fixture.ApiPath).UsingGet()) + .RespondWith(Response.Create().WithStatusCode(404)); - var actualRoles = await client.GetUserRoles(userId, userPartyId); + var actualRoles = await fixture.Client.GetUserRoles(1337, 2001); - actualRoles.Should().BeEquivalentTo(expectedRoles); + Assert.Equivalent(expectedRoles, actualRoles); } [Fact] - public async Task GetUserRolesAsync_throws_exception_on_error_status_code() + public async Task GetUserRoles_Throws_On_500() { - var userId = 1337; - var userPartyId = 2001; - - var httpMessageHandler = new Mock(); - - httpMessageHandler - .Protected() - .Setup>( - "SendAsync", - ItExpr.Is(m => - m.RequestUri != null - && m.RequestUri.ToString() - .Contains($"roles?coveredByUserId={userId}&offeredByPartyId={userPartyId}") - ), - ItExpr.IsAny() + await using var fixture = Fixture.Create(); + var server = fixture.Server; + server + .Given(Request.Create().WithPath(Fixture.ApiPath).UsingGet()) + .RespondWith(Response.Create().WithStatusCode(500)); + + await Assert.ThrowsAnyAsync(() => fixture.Client.GetUserRoles(1337, 2001)); + } + + private sealed record Fixture(WebApplication App) : IAsyncDisposable + { + internal const string ApiPath = "/authorization/api/v1/roles"; + + public Mock HttpClientFactoryMock => + Mock.Get(App.Services.GetRequiredService()); + + public WireMockServer Server => App.Services.GetRequiredService(); + + public AuthorizationClient Client => + App.Services.GetServices().OfType().Single(); + + private sealed class ReqHandler(Action? onRequest = null) : DelegatingHandler + { + protected override Task SendAsync( + HttpRequestMessage request, + CancellationToken cancellationToken ) - .ReturnsAsync( - new HttpResponseMessage - { - StatusCode = System.Net.HttpStatusCode.InternalServerError, - Content = new StringContent("Internal Server Error"), - } - ); + { + onRequest?.Invoke(); + return base.SendAsync(request, cancellationToken); + } + } - var httpClient = new HttpClient(httpMessageHandler.Object); + public static Fixture Create( + Action? registerCustomAppServices = default, + Action? onRequest = null + ) + { + var server = WireMockServer.Start(); - var pdpMock = new Mock(); - var appSettingsMock = new Mock>(); - appSettingsMock - .Setup(s => s.CurrentValue) - .Returns(new AppSettings { RuntimeCookieName = "AltinnStudioRuntime" }); + var mockHttpClientFactory = new Mock(); + mockHttpClientFactory + .Setup(f => f.CreateClient(It.IsAny())) + .Returns(() => server.CreateClient(new ReqHandler(onRequest))); - var platformSettings = Options.Create(new PlatformSettings { SubscriptionKey = "subscription-key" }); - var logger = NullLogger.Instance; - TelemetrySink telemetry = new(); + var app = Api.Tests.TestUtils.AppBuilder.Build( + configData: new Dictionary() + { + // API endpoint is configured this way since we have our `PlatformSettings` + // while PEP has it's own `PlatformSettings` class. + // So if we went the `services.Configure` route we would have to do it twice, + // once for ours and once for PEP's. + { "PlatformSettings:ApiAuthorizationEndpoint", server.Url + ApiPath }, + { "PlatformSettings:SubscriptionKey", "dummyKey" }, + { "AppSettings:RuntimeCookieName", "AltinnStudioRuntime" }, + }, + registerCustomAppServices: services => + { + services.AddSingleton(_ => server); - var httpContext = new DefaultHttpContext(); - httpContext.Request.Headers["Cookie"] = "AltinnStudioRuntime=myFakeJwtToken"; + registerCustomAppServices?.Invoke(services); + }, + overrideAltinnAppServices: services => + { + var httpContext = new DefaultHttpContext(); + httpContext.Request.Headers["Cookie"] = "AltinnStudioRuntime=myFakeJwtToken"; - var httpContextAccessorMock = new Mock(); - httpContextAccessorMock.Setup(_ => _.HttpContext).Returns(httpContext); + var httpContextAccessorMock = new Mock(); + httpContextAccessorMock.Setup(_ => _.HttpContext).Returns(httpContext); - var client = new AuthorizationClient( - platformSettings, - httpContextAccessorMock.Object, - httpClient, - appSettingsMock.Object, - pdpMock.Object, - logger, - telemetry.Object - ); + services.AddSingleton(httpContextAccessorMock.Object); + services.AddSingleton(mockHttpClientFactory.Object); + } + ); + + return new Fixture(app); + } - await Assert.ThrowsAsync(() => client.GetUserRoles(userId, userPartyId)); + public async ValueTask DisposeAsync() => await App.DisposeAsync(); } private static ClaimsPrincipal GetClaims(string partyId) @@ -264,6 +264,8 @@ private static XacmlJsonResponse GetXacmlJsonRespons(string filename) var xacmlJesonRespons = File.ReadAllText( Path.Join("Infrastructure", "Clients", "Authorization", "TestData", $"{filename}.json") ); - return JsonSerializer.Deserialize(xacmlJesonRespons); + var response = JsonSerializer.Deserialize(xacmlJesonRespons); + Assert.NotNull(response); + return response; } }