From db0e9fef9e348918b9a3d61d5ca8eb77812f8a0c Mon Sep 17 00:00:00 2001 From: Daniel Skovli Date: Wed, 27 Nov 2024 15:45:06 +0100 Subject: [PATCH 1/7] Populates UserContext.SocialSecurityNumber, refactors UserHelper, adds tests --- src/Altinn.App.Core/Helpers/UserHelper.cs | 69 ++++++++------- .../Helpers/UserHelperTest.cs | 85 +++++++++++++++++++ .../MaskinportenClientIntegrationTest.cs | 6 +- .../Telemetry/TelemetryConfigurationTests.cs | 4 +- .../TestUtils/AppBuilder.cs | 12 ++- .../CorrespondenceClientTests.cs | 2 +- .../Maskinporten/MaskinportenClientTest.cs | 2 +- 7 files changed, 136 insertions(+), 44 deletions(-) create mode 100644 test/Altinn.App.Api.Tests/Helpers/UserHelperTest.cs diff --git a/src/Altinn.App.Core/Helpers/UserHelper.cs b/src/Altinn.App.Core/Helpers/UserHelper.cs index 2ff295bc7..71a91cacf 100644 --- a/src/Altinn.App.Core/Helpers/UserHelper.cs +++ b/src/Altinn.App.Core/Helpers/UserHelper.cs @@ -50,28 +50,16 @@ public UserHelper( public async Task GetUserContext(HttpContext context) { using var activity = _telemetry?.StartGetUserContextActivity(); + string? cookieValue = context.Request.Cookies[_settings.GetAltinnPartyCookieName]; - UserContext userContext = new UserContext() { User = context.User }; - - foreach (Claim claim in context.User.Claims) + UserContext userContext = new() { - if (claim.Type.Equals(AltinnCoreClaimTypes.UserName, StringComparison.Ordinal)) - { - userContext.UserName = claim.Value; - } - else if (claim.Type.Equals(AltinnCoreClaimTypes.UserId, StringComparison.Ordinal)) - { - userContext.UserId = Convert.ToInt32(claim.Value, CultureInfo.InvariantCulture); - } - else if (claim.Type.Equals(AltinnCoreClaimTypes.PartyID, StringComparison.Ordinal)) - { - userContext.PartyId = Convert.ToInt32(claim.Value, CultureInfo.InvariantCulture); - } - else if (claim.Type.Equals(AltinnCoreClaimTypes.AuthenticationLevel, StringComparison.Ordinal)) - { - userContext.AuthenticationLevel = Convert.ToInt32(claim.Value, CultureInfo.InvariantCulture); - } - } + User = context.User, + UserName = GetClaim(context.User.Claims, AltinnCoreClaimTypes.UserName), + UserId = GetClaim(context.User.Claims, AltinnCoreClaimTypes.UserId), + PartyId = GetClaim(context.User.Claims, AltinnCoreClaimTypes.PartyID), + AuthenticationLevel = GetClaim(context.User.Claims, AltinnCoreClaimTypes.AuthenticationLevel), + }; if (userContext.UserId == default) { @@ -81,25 +69,40 @@ public async Task GetUserContext(HttpContext context) UserProfile userProfile = await _profileClient.GetUserProfile(userContext.UserId) ?? throw new Exception("Could not get user profile while getting user context"); + userContext.UserParty = userProfile.Party; - if (context.Request.Cookies[_settings.GetAltinnPartyCookieName] != null) - { - userContext.PartyId = Convert.ToInt32( - context.Request.Cookies[_settings.GetAltinnPartyCookieName], - CultureInfo.InvariantCulture - ); - } + userContext.PartyId = cookieValue is not null + ? Convert.ToInt32(cookieValue, CultureInfo.InvariantCulture) + : userContext.PartyId; - if (userContext.PartyId == userProfile.PartyId) + userContext.Party = userContext.PartyId.Equals(userProfile.Party?.PartyId) + ? userContext.Party = userProfile.Party + : await _altinnPartyClientService.GetParty(userContext.PartyId); + + userContext.SocialSecurityNumber = userContext.Party?.SSN ?? userContext.UserParty.SSN; + + return userContext; + } + + private static ClaimWrapper GetClaim(IEnumerable claims, string claimType) + { + var claim = claims.FirstOrDefault(x => x.Type.Equals(claimType, StringComparison.Ordinal))?.Value; + return new ClaimWrapper(claim); + } + + private readonly record struct ClaimWrapper(string? Value) + { + public static implicit operator string?(ClaimWrapper claimWrapper) { - userContext.Party = userProfile.Party; + return claimWrapper.Value; } - else + + public static implicit operator int(ClaimWrapper claimWrapper) { - userContext.Party = await _altinnPartyClientService.GetParty(userContext.PartyId); + return string.IsNullOrEmpty(claimWrapper.Value) + ? default + : Convert.ToInt32(claimWrapper.Value, CultureInfo.InvariantCulture); } - - return userContext; } } diff --git a/test/Altinn.App.Api.Tests/Helpers/UserHelperTest.cs b/test/Altinn.App.Api.Tests/Helpers/UserHelperTest.cs new file mode 100644 index 000000000..07be7e71c --- /dev/null +++ b/test/Altinn.App.Api.Tests/Helpers/UserHelperTest.cs @@ -0,0 +1,85 @@ +using System.Security.Claims; +using Altinn.App.Api.Tests.Mocks; +using Altinn.App.Api.Tests.Utils; +using Altinn.App.Core.Configuration; +using Altinn.App.Core.Helpers; +using Altinn.App.Core.Internal.Profile; +using Altinn.App.Core.Internal.Registers; +using FluentAssertions; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using Moq; + +namespace Altinn.App.Api.Tests.Helpers; + +public class UserHelperTest +{ + private sealed record Fixture(WebApplication App) : IAsyncDisposable + { + public readonly IOptions GeneralSettings = Options.Create(new GeneralSettings()); + public IProfileClient ProfileClientMock => App.Services.GetRequiredService(); + public IAltinnPartyClient AltinnPartyClientMock => App.Services.GetRequiredService(); + + public static Fixture Create(ClaimsPrincipal userPrincipal, string? partyCookieValue = null) + { + var app = TestUtils.AppBuilder.Build(postRegisterCustomAppServices: services => + { + var httpContextMock = new Mock(); + httpContextMock.Setup(x => x.Request.Cookies["AltinnPartyId"]).Returns(partyCookieValue); + httpContextMock.Setup(httpContext => httpContext.User).Returns(userPrincipal); + var httpContextAccessor = new Mock(); + httpContextAccessor.Setup(x => x.HttpContext).Returns(httpContextMock.Object); + + services.AddSingleton(httpContextAccessor.Object); + services.AddTransient(); + services.AddTransient(); + }); + return new Fixture(app); + } + + public async ValueTask DisposeAsync() => await App.DisposeAsync(); + } + + [Fact] + public async Task GetUserContext_PerformsCorrectLogic() + { + // Arrange + const int userId = 1337; + const int partyId = 501337; + const int authLevel = 3; + + var userPrincipal = PrincipalUtil.GetUserPrincipal(userId, partyId, authLevel); + await using var fixture = Fixture.Create(userPrincipal); + var userHelper = new UserHelper( + profileClient: fixture.ProfileClientMock, + altinnPartyClientService: fixture.AltinnPartyClientMock, + settings: fixture.GeneralSettings + ); + var httpContextAccessor = fixture.App.Services.GetRequiredService(); + var httpContext = httpContextAccessor.HttpContext; + var userProfile = await fixture.ProfileClientMock.GetUserProfile(userId); + var party = await fixture.AltinnPartyClientMock.GetParty(partyId); + + // Act + var result = await userHelper.GetUserContext(httpContext!); + + // Assert + result + .Should() + .BeEquivalentTo( + new Altinn.App.Core.Models.UserContext + { + SocialSecurityNumber = "01039012345", + UserName = $"User{userId}", + UserId = userId, + PartyId = partyId, + AuthenticationLevel = authLevel, + User = userPrincipal, + UserParty = userProfile!.Party, + Party = party, + } + ); + } +} diff --git a/test/Altinn.App.Api.Tests/Maskinporten/MaskinportenClientIntegrationTest.cs b/test/Altinn.App.Api.Tests/Maskinporten/MaskinportenClientIntegrationTest.cs index 6e6e8565a..ad6c6f2e2 100644 --- a/test/Altinn.App.Api.Tests/Maskinporten/MaskinportenClientIntegrationTest.cs +++ b/test/Altinn.App.Api.Tests/Maskinporten/MaskinportenClientIntegrationTest.cs @@ -28,7 +28,7 @@ public void ConfigureMaskinportenClient_OverridesDefaultMaskinportenConfiguratio var authority = "https://maskinporten.dev/"; // Act - var app = AppBuilder.Build(registerCustomAppServices: services => + var app = AppBuilder.Build(preRegisterCustomAppServices: services => { services.ConfigureMaskinportenClient(config => { @@ -101,7 +101,7 @@ public void ConfigureMaskinportenClient_BindsToSpecifiedConfigPath() // Act var app = AppBuilder.Build( configData: configData, - registerCustomAppServices: services => + preRegisterCustomAppServices: services => { services.ConfigureMaskinportenClient("CustomMaskinportenSettings"); } @@ -131,7 +131,7 @@ params string[] additionalScopes { // Arrange Enum.TryParse(tokenAuthority, false, out TokenAuthorities actualTokenAuthority); - var app = AppBuilder.Build(registerCustomAppServices: services => + var app = AppBuilder.Build(preRegisterCustomAppServices: services => { _ = actualTokenAuthority switch { diff --git a/test/Altinn.App.Api.Tests/Telemetry/TelemetryConfigurationTests.cs b/test/Altinn.App.Api.Tests/Telemetry/TelemetryConfigurationTests.cs index 88b8049c2..90bef8aca 100644 --- a/test/Altinn.App.Api.Tests/Telemetry/TelemetryConfigurationTests.cs +++ b/test/Altinn.App.Api.Tests/Telemetry/TelemetryConfigurationTests.cs @@ -300,7 +300,7 @@ public async Task OpenTelemetry_Sampler_Override_Is_Possible() var samplerToUse = new ParentBasedSampler(new AlwaysOnSampler()); await using var app = AppBuilder.Build( configData: configData, - registerCustomAppServices: services => + preRegisterCustomAppServices: services => { services.ConfigureOpenTelemetryTracerProvider(builder => { @@ -328,7 +328,7 @@ public async Task OpenTelemetry_MetricReaderOptions_Override_Is_Possible_Through var timeoutToUse = 4_000; await using var app = AppBuilder.Build( configData: configData, - registerCustomAppServices: services => + preRegisterCustomAppServices: services => { services.Configure(options => { diff --git a/test/Altinn.App.Api.Tests/TestUtils/AppBuilder.cs b/test/Altinn.App.Api.Tests/TestUtils/AppBuilder.cs index da954f912..75a61583b 100644 --- a/test/Altinn.App.Api.Tests/TestUtils/AppBuilder.cs +++ b/test/Altinn.App.Api.Tests/TestUtils/AppBuilder.cs @@ -9,7 +9,8 @@ public static class AppBuilder public static WebApplication Build( WebApplicationBuilder? builder = default, IEnumerable>? configData = default, - Action? registerCustomAppServices = default + Action? preRegisterCustomAppServices = default, + Action? postRegisterCustomAppServices = default ) { // Here we follow the order of operations currently present in the Program.cs generated by the template for apps, @@ -28,7 +29,7 @@ public static WebApplication Build( Altinn.App.Api.Extensions.ServiceCollectionExtensions.AddAltinnAppControllersWithViews(builder.Services); // 2. RegisterCustomAppServices - registerCustomAppServices?.Invoke(builder.Services); + preRegisterCustomAppServices?.Invoke(builder.Services); // 3. AddAltinnAppServices Altinn.App.Api.Extensions.ServiceCollectionExtensions.AddAltinnAppServices( @@ -37,10 +38,13 @@ public static WebApplication Build( builder.Environment ); - // 4. ConfigureAppWebHost + // 4. RegisterCustomAppServices + postRegisterCustomAppServices?.Invoke(builder.Services); + + // 5. ConfigureAppWebHost Altinn.App.Api.Extensions.WebHostBuilderExtensions.ConfigureAppWebHost(builder.WebHost, []); - // 5. UseAltinnAppCommonConfiguration + // 6. UseAltinnAppCommonConfiguration var app = builder.Build(); Altinn.App.Api.Extensions.WebApplicationBuilderExtensions.UseAltinnAppCommonConfiguration(app); diff --git a/test/Altinn.App.Core.Tests/Features/Correspondence/CorrespondenceClientTests.cs b/test/Altinn.App.Core.Tests/Features/Correspondence/CorrespondenceClientTests.cs index 45e62b47f..754f37e62 100644 --- a/test/Altinn.App.Core.Tests/Features/Correspondence/CorrespondenceClientTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Correspondence/CorrespondenceClientTests.cs @@ -32,7 +32,7 @@ public static Fixture Create() var mockHttpClientFactory = new Mock(); var mockMaskinportenClient = new Mock(); - var app = Api.Tests.TestUtils.AppBuilder.Build(registerCustomAppServices: services => + var app = Api.Tests.TestUtils.AppBuilder.Build(preRegisterCustomAppServices: services => { services.AddSingleton(mockHttpClientFactory.Object); services.AddSingleton(mockMaskinportenClient.Object); diff --git a/test/Altinn.App.Core.Tests/Features/Maskinporten/MaskinportenClientTest.cs b/test/Altinn.App.Core.Tests/Features/Maskinporten/MaskinportenClientTest.cs index 59518d080..e43b85d23 100644 --- a/test/Altinn.App.Core.Tests/Features/Maskinporten/MaskinportenClientTest.cs +++ b/test/Altinn.App.Core.Tests/Features/Maskinporten/MaskinportenClientTest.cs @@ -56,7 +56,7 @@ public static Fixture Create(bool configureMaskinporten = true) var mockHttpClientFactory = new Mock(); var fakeTimeProvider = new FakeTime(new DateTimeOffset(2024, 1, 1, 10, 0, 0, TimeSpan.Zero)); - var app = Api.Tests.TestUtils.AppBuilder.Build(registerCustomAppServices: services => + var app = Api.Tests.TestUtils.AppBuilder.Build(preRegisterCustomAppServices: services => { services.AddSingleton(mockHttpClientFactory.Object); services.Configure(options => options.Clock = fakeTimeProvider); From ab1458bc04664d74b579cb395097a46ba71cf748 Mon Sep 17 00:00:00 2001 From: Daniel Skovli Date: Thu, 28 Nov 2024 09:00:01 +0100 Subject: [PATCH 2/7] Changes fixture parameter name to `overrideAltinnAppServices` Reverts `registerCustomAppServices` --- test/Altinn.App.Api.Tests/Helpers/UserHelperTest.cs | 2 +- .../Maskinporten/MaskinportenClientIntegrationTest.cs | 6 +++--- .../Telemetry/TelemetryConfigurationTests.cs | 4 ++-- test/Altinn.App.Api.Tests/TestUtils/AppBuilder.cs | 10 +++++----- .../Correspondence/CorrespondenceClientTests.cs | 2 +- .../Features/Maskinporten/MaskinportenClientTest.cs | 2 +- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/test/Altinn.App.Api.Tests/Helpers/UserHelperTest.cs b/test/Altinn.App.Api.Tests/Helpers/UserHelperTest.cs index 07be7e71c..41d3cf2ab 100644 --- a/test/Altinn.App.Api.Tests/Helpers/UserHelperTest.cs +++ b/test/Altinn.App.Api.Tests/Helpers/UserHelperTest.cs @@ -24,7 +24,7 @@ private sealed record Fixture(WebApplication App) : IAsyncDisposable public static Fixture Create(ClaimsPrincipal userPrincipal, string? partyCookieValue = null) { - var app = TestUtils.AppBuilder.Build(postRegisterCustomAppServices: services => + var app = TestUtils.AppBuilder.Build(overrideAltinnAppServices: services => { var httpContextMock = new Mock(); httpContextMock.Setup(x => x.Request.Cookies["AltinnPartyId"]).Returns(partyCookieValue); diff --git a/test/Altinn.App.Api.Tests/Maskinporten/MaskinportenClientIntegrationTest.cs b/test/Altinn.App.Api.Tests/Maskinporten/MaskinportenClientIntegrationTest.cs index ad6c6f2e2..6e6e8565a 100644 --- a/test/Altinn.App.Api.Tests/Maskinporten/MaskinportenClientIntegrationTest.cs +++ b/test/Altinn.App.Api.Tests/Maskinporten/MaskinportenClientIntegrationTest.cs @@ -28,7 +28,7 @@ public void ConfigureMaskinportenClient_OverridesDefaultMaskinportenConfiguratio var authority = "https://maskinporten.dev/"; // Act - var app = AppBuilder.Build(preRegisterCustomAppServices: services => + var app = AppBuilder.Build(registerCustomAppServices: services => { services.ConfigureMaskinportenClient(config => { @@ -101,7 +101,7 @@ public void ConfigureMaskinportenClient_BindsToSpecifiedConfigPath() // Act var app = AppBuilder.Build( configData: configData, - preRegisterCustomAppServices: services => + registerCustomAppServices: services => { services.ConfigureMaskinportenClient("CustomMaskinportenSettings"); } @@ -131,7 +131,7 @@ params string[] additionalScopes { // Arrange Enum.TryParse(tokenAuthority, false, out TokenAuthorities actualTokenAuthority); - var app = AppBuilder.Build(preRegisterCustomAppServices: services => + var app = AppBuilder.Build(registerCustomAppServices: services => { _ = actualTokenAuthority switch { diff --git a/test/Altinn.App.Api.Tests/Telemetry/TelemetryConfigurationTests.cs b/test/Altinn.App.Api.Tests/Telemetry/TelemetryConfigurationTests.cs index 90bef8aca..88b8049c2 100644 --- a/test/Altinn.App.Api.Tests/Telemetry/TelemetryConfigurationTests.cs +++ b/test/Altinn.App.Api.Tests/Telemetry/TelemetryConfigurationTests.cs @@ -300,7 +300,7 @@ public async Task OpenTelemetry_Sampler_Override_Is_Possible() var samplerToUse = new ParentBasedSampler(new AlwaysOnSampler()); await using var app = AppBuilder.Build( configData: configData, - preRegisterCustomAppServices: services => + registerCustomAppServices: services => { services.ConfigureOpenTelemetryTracerProvider(builder => { @@ -328,7 +328,7 @@ public async Task OpenTelemetry_MetricReaderOptions_Override_Is_Possible_Through var timeoutToUse = 4_000; await using var app = AppBuilder.Build( configData: configData, - preRegisterCustomAppServices: services => + registerCustomAppServices: services => { services.Configure(options => { diff --git a/test/Altinn.App.Api.Tests/TestUtils/AppBuilder.cs b/test/Altinn.App.Api.Tests/TestUtils/AppBuilder.cs index 75a61583b..70609b688 100644 --- a/test/Altinn.App.Api.Tests/TestUtils/AppBuilder.cs +++ b/test/Altinn.App.Api.Tests/TestUtils/AppBuilder.cs @@ -9,8 +9,8 @@ public static class AppBuilder public static WebApplication Build( WebApplicationBuilder? builder = default, IEnumerable>? configData = default, - Action? preRegisterCustomAppServices = default, - Action? postRegisterCustomAppServices = default + Action? registerCustomAppServices = default, + Action? overrideAltinnAppServices = default ) { // Here we follow the order of operations currently present in the Program.cs generated by the template for apps, @@ -29,7 +29,7 @@ public static WebApplication Build( Altinn.App.Api.Extensions.ServiceCollectionExtensions.AddAltinnAppControllersWithViews(builder.Services); // 2. RegisterCustomAppServices - preRegisterCustomAppServices?.Invoke(builder.Services); + registerCustomAppServices?.Invoke(builder.Services); // 3. AddAltinnAppServices Altinn.App.Api.Extensions.ServiceCollectionExtensions.AddAltinnAppServices( @@ -38,8 +38,8 @@ public static WebApplication Build( builder.Environment ); - // 4. RegisterCustomAppServices - postRegisterCustomAppServices?.Invoke(builder.Services); + // 4. OverrideAltinnAppServices + overrideAltinnAppServices?.Invoke(builder.Services); // 5. ConfigureAppWebHost Altinn.App.Api.Extensions.WebHostBuilderExtensions.ConfigureAppWebHost(builder.WebHost, []); diff --git a/test/Altinn.App.Core.Tests/Features/Correspondence/CorrespondenceClientTests.cs b/test/Altinn.App.Core.Tests/Features/Correspondence/CorrespondenceClientTests.cs index 754f37e62..45e62b47f 100644 --- a/test/Altinn.App.Core.Tests/Features/Correspondence/CorrespondenceClientTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Correspondence/CorrespondenceClientTests.cs @@ -32,7 +32,7 @@ public static Fixture Create() var mockHttpClientFactory = new Mock(); var mockMaskinportenClient = new Mock(); - var app = Api.Tests.TestUtils.AppBuilder.Build(preRegisterCustomAppServices: services => + var app = Api.Tests.TestUtils.AppBuilder.Build(registerCustomAppServices: services => { services.AddSingleton(mockHttpClientFactory.Object); services.AddSingleton(mockMaskinportenClient.Object); diff --git a/test/Altinn.App.Core.Tests/Features/Maskinporten/MaskinportenClientTest.cs b/test/Altinn.App.Core.Tests/Features/Maskinporten/MaskinportenClientTest.cs index e43b85d23..59518d080 100644 --- a/test/Altinn.App.Core.Tests/Features/Maskinporten/MaskinportenClientTest.cs +++ b/test/Altinn.App.Core.Tests/Features/Maskinporten/MaskinportenClientTest.cs @@ -56,7 +56,7 @@ public static Fixture Create(bool configureMaskinporten = true) var mockHttpClientFactory = new Mock(); var fakeTimeProvider = new FakeTime(new DateTimeOffset(2024, 1, 1, 10, 0, 0, TimeSpan.Zero)); - var app = Api.Tests.TestUtils.AppBuilder.Build(preRegisterCustomAppServices: services => + var app = Api.Tests.TestUtils.AppBuilder.Build(registerCustomAppServices: services => { services.AddSingleton(mockHttpClientFactory.Object); services.Configure(options => options.Clock = fakeTimeProvider); From 5b449df53ffb6fb1b17d80b1b8ae68006f4c28e4 Mon Sep 17 00:00:00 2001 From: Daniel Skovli Date: Fri, 29 Nov 2024 13:41:04 +0100 Subject: [PATCH 3/7] Adjusts UserHelper and test method based on research/status quo --- src/Altinn.App.Core/Helpers/UserHelper.cs | 59 +++++++++---------- .../Data/Profile/User/1337.json | 5 +- .../Helpers/UserHelperTest.cs | 17 +++--- 3 files changed, 42 insertions(+), 39 deletions(-) diff --git a/src/Altinn.App.Core/Helpers/UserHelper.cs b/src/Altinn.App.Core/Helpers/UserHelper.cs index 71a91cacf..c7c3f4cce 100644 --- a/src/Altinn.App.Core/Helpers/UserHelper.cs +++ b/src/Altinn.App.Core/Helpers/UserHelper.cs @@ -50,15 +50,32 @@ public UserHelper( public async Task GetUserContext(HttpContext context) { using var activity = _telemetry?.StartGetUserContextActivity(); - string? cookieValue = context.Request.Cookies[_settings.GetAltinnPartyCookieName]; + string? partyCookieValue = context.Request.Cookies[_settings.GetAltinnPartyCookieName]; + Dictionary tokenClaims = context.User.Claims.ToDictionary( + x => x.Type, + y => y.Value, + StringComparer.Ordinal + ); UserContext userContext = new() { User = context.User, - UserName = GetClaim(context.User.Claims, AltinnCoreClaimTypes.UserName), - UserId = GetClaim(context.User.Claims, AltinnCoreClaimTypes.UserId), - PartyId = GetClaim(context.User.Claims, AltinnCoreClaimTypes.PartyID), - AuthenticationLevel = GetClaim(context.User.Claims, AltinnCoreClaimTypes.AuthenticationLevel), + UserName = tokenClaims[AltinnCoreClaimTypes.UserName], + UserId = tokenClaims[AltinnCoreClaimTypes.UserId] switch + { + { } value => Convert.ToInt32(value, CultureInfo.InvariantCulture), + _ => default, + }, + PartyId = tokenClaims[AltinnCoreClaimTypes.PartyID] switch + { + { } value => Convert.ToInt32(value, CultureInfo.InvariantCulture), + _ => default, + }, + AuthenticationLevel = tokenClaims[AltinnCoreClaimTypes.AuthenticationLevel] switch + { + { } value => Convert.ToInt32(value, CultureInfo.InvariantCulture), + _ => default, + }, }; if (userContext.UserId == default) @@ -72,37 +89,17 @@ await _profileClient.GetUserProfile(userContext.UserId) userContext.UserParty = userProfile.Party; - userContext.PartyId = cookieValue is not null - ? Convert.ToInt32(cookieValue, CultureInfo.InvariantCulture) + userContext.PartyId = partyCookieValue is not null + ? Convert.ToInt32(partyCookieValue, CultureInfo.InvariantCulture) : userContext.PartyId; - userContext.Party = userContext.PartyId.Equals(userProfile.Party?.PartyId) - ? userContext.Party = userProfile.Party + userContext.Party = userContext.PartyId.Equals(userProfile.PartyId) + ? userProfile.Party : await _altinnPartyClientService.GetParty(userContext.PartyId); - userContext.SocialSecurityNumber = userContext.Party?.SSN ?? userContext.UserParty.SSN; + userContext.SocialSecurityNumber = + userContext.Party?.SSN ?? userContext.Party?.Person?.SSN ?? userContext.UserParty.SSN; return userContext; } - - private static ClaimWrapper GetClaim(IEnumerable claims, string claimType) - { - var claim = claims.FirstOrDefault(x => x.Type.Equals(claimType, StringComparison.Ordinal))?.Value; - return new ClaimWrapper(claim); - } - - private readonly record struct ClaimWrapper(string? Value) - { - public static implicit operator string?(ClaimWrapper claimWrapper) - { - return claimWrapper.Value; - } - - public static implicit operator int(ClaimWrapper claimWrapper) - { - return string.IsNullOrEmpty(claimWrapper.Value) - ? default - : Convert.ToInt32(claimWrapper.Value, CultureInfo.InvariantCulture); - } - } } diff --git a/test/Altinn.App.Api.Tests/Data/Profile/User/1337.json b/test/Altinn.App.Api.Tests/Data/Profile/User/1337.json index 7624c62be..f4eecb1bd 100644 --- a/test/Altinn.App.Api.Tests/Data/Profile/User/1337.json +++ b/test/Altinn.App.Api.Tests/Data/Profile/User/1337.json @@ -4,7 +4,10 @@ "PhoneNumber": "90001337", "Email": "1337@altinnstudiotestusers.com", "PartyId": 501337, - "Party": {}, + "Party": { + "partyId": "501337", + "ssn": "01039012345" + }, "UserType": 1, "ProfileSettingPreference": { "Language": "nn", diff --git a/test/Altinn.App.Api.Tests/Helpers/UserHelperTest.cs b/test/Altinn.App.Api.Tests/Helpers/UserHelperTest.cs index 41d3cf2ab..8421f76d1 100644 --- a/test/Altinn.App.Api.Tests/Helpers/UserHelperTest.cs +++ b/test/Altinn.App.Api.Tests/Helpers/UserHelperTest.cs @@ -5,6 +5,7 @@ using Altinn.App.Core.Helpers; using Altinn.App.Core.Internal.Profile; using Altinn.App.Core.Internal.Registers; +using Altinn.Platform.Register.Models; using FluentAssertions; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; @@ -42,14 +43,14 @@ public static Fixture Create(ClaimsPrincipal userPrincipal, string? partyCookieV public async ValueTask DisposeAsync() => await App.DisposeAsync(); } - [Fact] - public async Task GetUserContext_PerformsCorrectLogic() + [Theory] + [InlineData(1337, 501337, "01039012345")] // Has `Party` containing correct SSN + [InlineData(1001, 510001, null)] // Has no SSN, because of empty `Party` + [InlineData(1337, 510001, "01899699552")] // `Party` mismatch, forcing load via `IAltinnPartyClient`, resulting in SSN belonging to party 510001 + public async Task GetUserContext_PerformsCorrectLogic(int userId, int partyId, string? ssn) { // Arrange - const int userId = 1337; - const int partyId = 501337; const int authLevel = 3; - var userPrincipal = PrincipalUtil.GetUserPrincipal(userId, partyId, authLevel); await using var fixture = Fixture.Create(userPrincipal); var userHelper = new UserHelper( @@ -60,7 +61,9 @@ public async Task GetUserContext_PerformsCorrectLogic() var httpContextAccessor = fixture.App.Services.GetRequiredService(); var httpContext = httpContextAccessor.HttpContext; var userProfile = await fixture.ProfileClientMock.GetUserProfile(userId); - var party = await fixture.AltinnPartyClientMock.GetParty(partyId); + var party = partyId.Equals(userProfile!.PartyId) + ? userProfile!.Party + : await fixture.AltinnPartyClientMock.GetParty(partyId); // Act var result = await userHelper.GetUserContext(httpContext!); @@ -71,7 +74,7 @@ public async Task GetUserContext_PerformsCorrectLogic() .BeEquivalentTo( new Altinn.App.Core.Models.UserContext { - SocialSecurityNumber = "01039012345", + SocialSecurityNumber = ssn, UserName = $"User{userId}", UserId = userId, PartyId = partyId, From 7bd98de0f76651eb68022671d35efeebc6c8f54c Mon Sep 17 00:00:00 2001 From: Daniel Skovli Date: Fri, 29 Nov 2024 13:45:27 +0100 Subject: [PATCH 4/7] Unused usings --- src/Altinn.App.Core/Helpers/UserHelper.cs | 1 - test/Altinn.App.Api.Tests/Helpers/UserHelperTest.cs | 1 - 2 files changed, 2 deletions(-) diff --git a/src/Altinn.App.Core/Helpers/UserHelper.cs b/src/Altinn.App.Core/Helpers/UserHelper.cs index c7c3f4cce..cbbf735a0 100644 --- a/src/Altinn.App.Core/Helpers/UserHelper.cs +++ b/src/Altinn.App.Core/Helpers/UserHelper.cs @@ -1,5 +1,4 @@ using System.Globalization; -using System.Security.Claims; using Altinn.App.Core.Configuration; using Altinn.App.Core.Features; using Altinn.App.Core.Internal.Profile; diff --git a/test/Altinn.App.Api.Tests/Helpers/UserHelperTest.cs b/test/Altinn.App.Api.Tests/Helpers/UserHelperTest.cs index 8421f76d1..db9a0cfa6 100644 --- a/test/Altinn.App.Api.Tests/Helpers/UserHelperTest.cs +++ b/test/Altinn.App.Api.Tests/Helpers/UserHelperTest.cs @@ -5,7 +5,6 @@ using Altinn.App.Core.Helpers; using Altinn.App.Core.Internal.Profile; using Altinn.App.Core.Internal.Registers; -using Altinn.Platform.Register.Models; using FluentAssertions; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; From ff0b3760cce71414a5df453e4787e297a6b87340 Mon Sep 17 00:00:00 2001 From: Daniel Skovli Date: Fri, 29 Nov 2024 14:36:18 +0100 Subject: [PATCH 5/7] Safe dictionary accessors, slight refactor, more tests --- src/Altinn.App.Core/Helpers/UserHelper.cs | 31 ++++------ .../Helpers/UserHelperTest.cs | 62 +++++++++++++++++++ 2 files changed, 75 insertions(+), 18 deletions(-) diff --git a/src/Altinn.App.Core/Helpers/UserHelper.cs b/src/Altinn.App.Core/Helpers/UserHelper.cs index cbbf735a0..d93a5b503 100644 --- a/src/Altinn.App.Core/Helpers/UserHelper.cs +++ b/src/Altinn.App.Core/Helpers/UserHelper.cs @@ -59,45 +59,40 @@ public async Task GetUserContext(HttpContext context) UserContext userContext = new() { User = context.User, - UserName = tokenClaims[AltinnCoreClaimTypes.UserName], - UserId = tokenClaims[AltinnCoreClaimTypes.UserId] switch + UserName = tokenClaims.GetValueOrDefault(AltinnCoreClaimTypes.UserName), + UserId = tokenClaims.GetValueOrDefault(AltinnCoreClaimTypes.UserId) switch { { } value => Convert.ToInt32(value, CultureInfo.InvariantCulture), - _ => default, + _ => throw new Exception("Could not get user profile - could not retrieve user ID from claims"), }, - PartyId = tokenClaims[AltinnCoreClaimTypes.PartyID] switch + PartyId = tokenClaims.GetValueOrDefault(AltinnCoreClaimTypes.PartyID) switch { { } value => Convert.ToInt32(value, CultureInfo.InvariantCulture), _ => default, }, - AuthenticationLevel = tokenClaims[AltinnCoreClaimTypes.AuthenticationLevel] switch + AuthenticationLevel = tokenClaims.GetValueOrDefault(AltinnCoreClaimTypes.AuthenticationLevel) switch { { } value => Convert.ToInt32(value, CultureInfo.InvariantCulture), _ => default, }, }; - if (userContext.UserId == default) - { - throw new Exception("Could not get user profile - could not retrieve user ID from claims"); - } - UserProfile userProfile = await _profileClient.GetUserProfile(userContext.UserId) ?? throw new Exception("Could not get user profile while getting user context"); - userContext.UserParty = userProfile.Party; + if (partyCookieValue is not null) + userContext.PartyId = Convert.ToInt32(partyCookieValue, CultureInfo.InvariantCulture); - userContext.PartyId = partyCookieValue is not null - ? Convert.ToInt32(partyCookieValue, CultureInfo.InvariantCulture) - : userContext.PartyId; + if (userContext.PartyId == userProfile.PartyId) + userContext.Party = userProfile.Party; + else if (userContext.PartyId > 0) + userContext.Party = await _altinnPartyClientService.GetParty(userContext.PartyId); - userContext.Party = userContext.PartyId.Equals(userProfile.PartyId) - ? userProfile.Party - : await _altinnPartyClientService.GetParty(userContext.PartyId); + userContext.UserParty = userProfile.Party; userContext.SocialSecurityNumber = - userContext.Party?.SSN ?? userContext.Party?.Person?.SSN ?? userContext.UserParty.SSN; + userContext.Party?.SSN ?? userContext.Party?.Person?.SSN ?? userContext.UserParty?.SSN; return userContext; } diff --git a/test/Altinn.App.Api.Tests/Helpers/UserHelperTest.cs b/test/Altinn.App.Api.Tests/Helpers/UserHelperTest.cs index db9a0cfa6..626a64729 100644 --- a/test/Altinn.App.Api.Tests/Helpers/UserHelperTest.cs +++ b/test/Altinn.App.Api.Tests/Helpers/UserHelperTest.cs @@ -84,4 +84,66 @@ public async Task GetUserContext_PerformsCorrectLogic(int userId, int partyId, s } ); } + + [Fact] + public async Task GetUserContext_HandlesMissingClaims() + { + // Arrange + const int userId = 1001; + const int authLevel = 3; + var userPrincipal = PrincipalUtil.GetUserPrincipal(userId, default, authLevel); + await using var fixture = Fixture.Create(userPrincipal); + var userHelper = new UserHelper( + profileClient: fixture.ProfileClientMock, + altinnPartyClientService: fixture.AltinnPartyClientMock, + settings: fixture.GeneralSettings + ); + var httpContextAccessor = fixture.App.Services.GetRequiredService(); + var httpContext = httpContextAccessor.HttpContext; + var userProfile = await fixture.ProfileClientMock.GetUserProfile(userId); + + // Act + var result = await userHelper.GetUserContext(httpContext!); + + // Assert + result + .Should() + .BeEquivalentTo( + new Altinn.App.Core.Models.UserContext + { + SocialSecurityNumber = null, + UserName = $"User{userId}", + UserId = userId, + PartyId = default, + AuthenticationLevel = authLevel, + User = userPrincipal, + UserParty = userProfile!.Party, + Party = null, + } + ); + } + + [Fact] + public async Task GetUserContext_ThrowsOnMissingUserId() + { + // Arrange + var userPrincipal = PrincipalUtil.GetUserPrincipal(default, default); + await using var fixture = Fixture.Create(userPrincipal); + var userHelper = new UserHelper( + profileClient: fixture.ProfileClientMock, + altinnPartyClientService: fixture.AltinnPartyClientMock, + settings: fixture.GeneralSettings + ); + var httpContextAccessor = fixture.App.Services.GetRequiredService(); + var httpContext = httpContextAccessor.HttpContext; + + // Act + var act = async () => + { + await userHelper.GetUserContext(httpContext!); + }; + + // Assert + await act.Should().ThrowAsync().WithMessage("*not*ID*from*claims*"); + } } From 2d505be8387c74ef0d28880f4ace16d86c54e114 Mon Sep 17 00:00:00 2001 From: Daniel Skovli Date: Fri, 29 Nov 2024 15:10:28 +0100 Subject: [PATCH 6/7] Restores `default` check for `UserContext.UserId` --- src/Altinn.App.Core/Helpers/UserHelper.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Altinn.App.Core/Helpers/UserHelper.cs b/src/Altinn.App.Core/Helpers/UserHelper.cs index d93a5b503..e13c9cc82 100644 --- a/src/Altinn.App.Core/Helpers/UserHelper.cs +++ b/src/Altinn.App.Core/Helpers/UserHelper.cs @@ -63,7 +63,7 @@ public async Task GetUserContext(HttpContext context) UserId = tokenClaims.GetValueOrDefault(AltinnCoreClaimTypes.UserId) switch { { } value => Convert.ToInt32(value, CultureInfo.InvariantCulture), - _ => throw new Exception("Could not get user profile - could not retrieve user ID from claims"), + _ => default, }, PartyId = tokenClaims.GetValueOrDefault(AltinnCoreClaimTypes.PartyID) switch { @@ -77,6 +77,11 @@ public async Task GetUserContext(HttpContext context) }, }; + if (userContext.UserId == default) + { + throw new Exception("Could not get user profile - could not retrieve user ID from claims"); + } + UserProfile userProfile = await _profileClient.GetUserProfile(userContext.UserId) ?? throw new Exception("Could not get user profile while getting user context"); From 5f10041c26f11165cd0efc19029dc04be287e8b3 Mon Sep 17 00:00:00 2001 From: Daniel Skovli Date: Fri, 29 Nov 2024 15:39:18 +0100 Subject: [PATCH 7/7] More robust SSN assignment check, compare PartyId with `default` instead of positive int --- src/Altinn.App.Core/Helpers/UserHelper.cs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/Altinn.App.Core/Helpers/UserHelper.cs b/src/Altinn.App.Core/Helpers/UserHelper.cs index e13c9cc82..742d2ee67 100644 --- a/src/Altinn.App.Core/Helpers/UserHelper.cs +++ b/src/Altinn.App.Core/Helpers/UserHelper.cs @@ -89,15 +89,19 @@ await _profileClient.GetUserProfile(userContext.UserId) if (partyCookieValue is not null) userContext.PartyId = Convert.ToInt32(partyCookieValue, CultureInfo.InvariantCulture); + userContext.UserParty = userProfile.Party; + if (userContext.PartyId == userProfile.PartyId) userContext.Party = userProfile.Party; - else if (userContext.PartyId > 0) + else if (userContext.PartyId != default) userContext.Party = await _altinnPartyClientService.GetParty(userContext.PartyId); - userContext.UserParty = userProfile.Party; - - userContext.SocialSecurityNumber = - userContext.Party?.SSN ?? userContext.Party?.Person?.SSN ?? userContext.UserParty?.SSN; + if (!string.IsNullOrWhiteSpace(userContext.Party?.SSN)) + userContext.SocialSecurityNumber = userContext.Party.SSN; + else if (!string.IsNullOrWhiteSpace(userContext.Party?.Person?.SSN)) + userContext.SocialSecurityNumber = userContext.Party.Person.SSN; + else if (!string.IsNullOrWhiteSpace(userContext.UserParty?.SSN)) + userContext.SocialSecurityNumber = userContext.UserParty.SSN; return userContext; }