From 1cbee771ab700723dc4bbf7af9ec8fbb817687cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Dybvik=20Langfors?= Date: Fri, 25 Oct 2024 08:31:06 +0200 Subject: [PATCH 1/8] Simplify subject attribute matching --- .../AltinnAuthorizationClient.cs | 1 - .../Authorization/DecisionRequestHelper.cs | 71 +++++++++++-------- .../DecisionRequestHelperTests.cs | 42 +++-------- 3 files changed, 51 insertions(+), 63 deletions(-) diff --git a/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/AltinnAuthorizationClient.cs b/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/AltinnAuthorizationClient.cs index 3d127348e..b676e36c9 100644 --- a/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/AltinnAuthorizationClient.cs +++ b/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/AltinnAuthorizationClient.cs @@ -1,5 +1,4 @@ using System.Diagnostics; -using System.Security.Claims; using System.Text.Json; using System.Text.Json.Serialization; using Altinn.Authorization.ABAC.Xacml.JsonProfile; diff --git a/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs b/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs index f2297fc3c..669b98e4c 100644 --- a/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs +++ b/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs @@ -1,5 +1,6 @@ using Altinn.Authorization.ABAC.Xacml.JsonProfile; using System.Security.Claims; + using Digdir.Domain.Dialogporten.Application.Common.Extensions; using Digdir.Domain.Dialogporten.Application.Externals.AltinnAuthorization; using Digdir.Domain.Dialogporten.Domain.Parties; @@ -10,20 +11,23 @@ namespace Digdir.Domain.Dialogporten.Infrastructure.Altinn.Authorization; internal static class DecisionRequestHelper { private const string SubjectId = "s1"; - private const string AltinnUrnNsPrefix = "urn:altinn:"; + private const string PidClaimType = "pid"; - private const string ConsumerClaimType = "consumer"; + private const string UserIdClaimType = "urn:altinn:userid"; + private const string RarAutorizationDetailsClaimType = "authorization_details"; + private const string AttributeIdAction = "urn:oasis:names:tc:xacml:1.0:action:action-id"; private const string AttributeIdResource = "urn:altinn:resource"; private const string AttributeIdResourceInstance = "urn:altinn:resourceinstance"; - private const string AltinnAutorizationDetailsClaim = "authorization_details"; private const string AttributeIdOrg = "urn:altinn:org"; private const string AttributeIdApp = "urn:altinn:app"; private const string AttributeIdSystemUser = "urn:altinn:systemuser:uuid"; - private const string AttributeIdUserId = "urn:altinn:userid"; - private const string ReservedResourcePrefixForApps = "app_"; private const string AttributeIdAppInstance = "urn:altinn:instance-id"; private const string AttributeIdSubResource = "urn:altinn:subresource"; + private const string AttributeIdUserId = "urn:altinn:userid"; + + private const string ReservedResourcePrefixForApps = "app_"; + private const string PermitResponse = "Permit"; public static XacmlJsonRequestRoot CreateDialogDetailsRequest(DialogDetailsAuthorizationRequest request) @@ -73,37 +77,46 @@ public static DialogDetailsAuthorizationResult CreateDialogDetailsResponse(List< private static List CreateAccessSubjectCategory(IEnumerable claims) { - var attributes = claims - .Select(x => x switch + // The PDP expects for the most part only a single subject attribute, and will even fail the request + // for some types (e.g. the urn:altinn:systemuser:uuid) if there are multiple subject attributes (for + // security reasons). We therefore need to filter out the relevant attributes and only include those, + // which in essence is the pid and the systemuser uuid. In addition, we also utilize urn:altinn:userid + // if present instead of the pid as a simple optimization as this offloads the PDP from having to look up + // the user id from the pid. + XacmlJsonAttribute? selectedAttribute = null; + + foreach (var claim in claims) + { + if (claim.Type == UserIdClaimType) { - { Type: PidClaimType } => new XacmlJsonAttribute { AttributeId = NorwegianPersonIdentifier.Prefix, Value = x.Value }, - { Type: var type } when type.StartsWith(AltinnUrnNsPrefix, StringComparison.Ordinal) => new() { AttributeId = type, Value = x.Value }, - { Type: ConsumerClaimType } when x.TryGetOrganizationNumber(out var organizationNumber) => new() { AttributeId = NorwegianOrganizationIdentifier.Prefix, Value = organizationNumber }, - { Type: AltinnAutorizationDetailsClaim } => new() { AttributeId = AttributeIdSystemUser, Value = GetSystemUserId(x) }, - _ => null - }) - .Where(x => x is not null) - .Cast() - .ToList(); + selectedAttribute = new XacmlJsonAttribute { AttributeId = AttributeIdUserId, Value = claim.Value }; + break; + } - // If we're authorizing a person (i.e. ID-porten token), we are not interested in the consumer-claim (organization number) - // as that is not relevant for the authorization decision (it's just the organization owning the OAuth client). - // The same goes if urn:altinn:userid is present, which might be present if using a legacy enterprise user token - if (attributes.Any(x => x.AttributeId == NorwegianPersonIdentifier.Prefix) || - attributes.Any(x => x.AttributeId == AttributeIdUserId)) - { - attributes.RemoveAll(x => x.AttributeId == NorwegianOrganizationIdentifier.Prefix); + if (claim.Type == PidClaimType) + { + selectedAttribute = new XacmlJsonAttribute { AttributeId = NorwegianPersonIdentifier.Prefix, Value = claim.Value }; + break; + } + + if (claim.Type == RarAutorizationDetailsClaimType) + { + var claimsPrincipal = new ClaimsPrincipal(new ClaimsIdentity(new[] { claim })); + if (claimsPrincipal.TryGetSystemUserId(out var systemUserId)) + { + selectedAttribute = new XacmlJsonAttribute { AttributeId = AttributeIdSystemUser, Value = systemUserId }; + } + break; + } } + var attributes = selectedAttribute != null + ? [selectedAttribute] + : new List(); + return [new() { Id = SubjectId, Attribute = attributes }]; } - private static string GetSystemUserId(Claim claim) - { - var claimsPrincipal = new ClaimsPrincipal(new ClaimsIdentity([claim])); - claimsPrincipal.TryGetSystemUserId(out var systemUserId); - return systemUserId!; - } private static List CreateActionCategories( List altinnActions, out Dictionary actionIdByName) diff --git a/tests/Digdir.Domain.Dialogporten.Infrastructure.Unit.Tests/DecisionRequestHelperTests.cs b/tests/Digdir.Domain.Dialogporten.Infrastructure.Unit.Tests/DecisionRequestHelperTests.cs index e23c180e5..6ef9e314b 100644 --- a/tests/Digdir.Domain.Dialogporten.Infrastructure.Unit.Tests/DecisionRequestHelperTests.cs +++ b/tests/Digdir.Domain.Dialogporten.Infrastructure.Unit.Tests/DecisionRequestHelperTests.cs @@ -23,7 +23,7 @@ public void CreateDialogDetailsRequestShouldReturnCorrectRequest() ("pid", "12345678901"), // This should not be copied as subject claim since there's a "pid"-claim - ("consumer", ConsumerClaimValue) + ("authorization_details", AuthorizationDetailsClaimValue) ), $"{NorwegianOrganizationIdentifier.PrefixWithSeparator}713330310"); var dialogId = request.DialogId; @@ -42,9 +42,8 @@ public void CreateDialogDetailsRequestShouldReturnCorrectRequest() // Check AccessSubject attributes var accessSubject = result.Request.AccessSubject.First(); Assert.Equal("s1", accessSubject.Id); - Assert.Contains(accessSubject.Attribute, a => a.AttributeId == "urn:altinn:foo" && a.Value == "bar"); Assert.Contains(accessSubject.Attribute, a => a.AttributeId == "urn:altinn:person:identifier-no" && a.Value == "12345678901"); - Assert.DoesNotContain(accessSubject.Attribute, a => a.AttributeId == "urn:altinn:organization:identifier-no"); + Assert.Single(accessSubject.Attribute); // Check Action attributes. var actionIdsByName = new Dictionary(); @@ -79,7 +78,7 @@ public void CreateDialogDetailsRequestShouldReturnCorrectRequest() } [Fact] - public void CreateDialogDetailsRequestShouldReturnCorrectRequestForLegacyEnterpriseUsers() + public void CreateDialogDetailsRequestShouldReturnCorrectRequestForExchangedTokens() { // Arrange var request = CreateDialogDetailsAuthorizationRequest( @@ -87,7 +86,7 @@ public void CreateDialogDetailsRequestShouldReturnCorrectRequestForLegacyEnterpr ("urn:altinn:userid", "5678901"), // This should not be copied as subject claim since there's a "urn:altinn:user-id"-claim - ("consumer", ConsumerClaimValue) + ("pid", "12345678901") ), $"{NorwegianOrganizationIdentifier.PrefixWithSeparator}713330310"); @@ -98,7 +97,7 @@ public void CreateDialogDetailsRequestShouldReturnCorrectRequestForLegacyEnterpr var accessSubject = result.Request.AccessSubject.First(); Assert.Equal("s1", accessSubject.Id); Assert.Contains(accessSubject.Attribute, a => a.AttributeId == "urn:altinn:userid" && a.Value == "5678901"); - Assert.DoesNotContain(accessSubject.Attribute, a => a.AttributeId == "urn:altinn:organization:identifier-no"); + Assert.Single(accessSubject.Attribute); } [Fact] @@ -136,7 +135,9 @@ public void CreateDialogDetailsRequestShouldReturnCorrectRequestForSystemUser() // Arrange var request = CreateDialogDetailsAuthorizationRequest( GetAsClaims( - ("authorization_details", AuthorizationDetailsClaimValue) + ("authorization_details", AuthorizationDetailsClaimValue), + ("pid", "12345678901"), + ("consumer", ConsumerClaimValue) ), $"{NorwegianOrganizationIdentifier.PrefixWithSeparator}713330310" ); @@ -151,33 +152,8 @@ public void CreateDialogDetailsRequestShouldReturnCorrectRequestForSystemUser() var accessSubject = result.Request.AccessSubject.First(); Assert.Equal("s1", accessSubject.Id); - Assert.Contains(accessSubject.Attribute, a => a.AttributeId == "urn:altinn:foo" && a.Value == "bar"); Assert.Contains(accessSubject.Attribute, a => a.AttributeId == "urn:altinn:systemuser:uuid" && a.Value == "unique_systemuser_id"); - } - - [Fact] - public void CreateDialogDetailsRequestShouldReturnCorrectRequestForConsumerOrgAndPersonParty() - { - // Arrange - var request = CreateDialogDetailsAuthorizationRequest( - GetAsClaims( - // Should be copied as subject claim since there's not a "pid"-claim - ("consumer", ConsumerClaimValue) - ), - $"{NorwegianPersonIdentifier.PrefixWithSeparator}16073422888"); - - // Act - var result = DecisionRequestHelper.CreateDialogDetailsRequest(request); - - // Assert - // Check that we have the organizationnumber - var accessSubject = result.Request.AccessSubject.First(); - Assert.Contains(accessSubject.Attribute, a => a.AttributeId == "urn:altinn:organization:identifier-no" && a.Value == "991825827"); - - // Check that we have the ssn attribute as resource owner - var resource1 = result.Request.Resource.FirstOrDefault(r => r.Id == "r1"); - Assert.NotNull(resource1); - Assert.Contains(resource1.Attribute, a => a.AttributeId == "urn:altinn:person:identifier-no" && a.Value == "16073422888"); + Assert.Single(accessSubject.Attribute); } [Fact] From 2cd9df06335bd973383fa23b119f283cfa94a99d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Dybvik=20Langfors?= Date: Fri, 25 Oct 2024 09:12:18 +0200 Subject: [PATCH 2/8] Add assertion --- .../Altinn/Authorization/DecisionRequestHelper.cs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs b/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs index 669b98e4c..7c529e7c0 100644 --- a/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs +++ b/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs @@ -1,4 +1,5 @@ -using Altinn.Authorization.ABAC.Xacml.JsonProfile; +using System.Diagnostics; +using Altinn.Authorization.ABAC.Xacml.JsonProfile; using System.Security.Claims; using Digdir.Domain.Dialogporten.Application.Common.Extensions; @@ -110,11 +111,9 @@ private static List CreateAccessSubjectCategory(IEnumerable(); - - return [new() { Id = SubjectId, Attribute = attributes }]; + return selectedAttribute == null + ? throw new UnreachableException("Unable to find a suitable subject attribute for the authorization request. Having a known user type should be enforced during authentication (see UserTypeValidationMiddleware).") + : ([new() { Id = SubjectId, Attribute = [selectedAttribute] }]); } From 291e2eb8ff53bec51013cb9cf4d604accd7e97ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Dybvik=20Langfors?= Date: Fri, 25 Oct 2024 09:36:18 +0200 Subject: [PATCH 3/8] Add assertion, clean up --- .../Authorization/DecisionRequestHelper.cs | 22 ++++++++----------- .../DecisionRequestHelperTests.cs | 9 ++++---- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs b/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs index 7c529e7c0..567f70ade 100644 --- a/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs +++ b/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs @@ -15,7 +15,7 @@ internal static class DecisionRequestHelper private const string PidClaimType = "pid"; private const string UserIdClaimType = "urn:altinn:userid"; - private const string RarAutorizationDetailsClaimType = "authorization_details"; + private const string RarAuthorizationDetailsClaimType = "authorization_details"; private const string AttributeIdAction = "urn:oasis:names:tc:xacml:1.0:action:action-id"; private const string AttributeIdResource = "urn:altinn:resource"; @@ -84,39 +84,35 @@ private static List CreateAccessSubjectCategory(IEnumerable CreateActionCategories( List altinnActions, out Dictionary actionIdByName) { diff --git a/tests/Digdir.Domain.Dialogporten.Infrastructure.Unit.Tests/DecisionRequestHelperTests.cs b/tests/Digdir.Domain.Dialogporten.Infrastructure.Unit.Tests/DecisionRequestHelperTests.cs index 6ef9e314b..583fa548b 100644 --- a/tests/Digdir.Domain.Dialogporten.Infrastructure.Unit.Tests/DecisionRequestHelperTests.cs +++ b/tests/Digdir.Domain.Dialogporten.Infrastructure.Unit.Tests/DecisionRequestHelperTests.cs @@ -162,7 +162,7 @@ public void CreateDialogDetailsRequestShouldReturnCorrectRequestForOverriddenRes // Arrange var request = CreateDialogDetailsAuthorizationRequest( GetAsClaims( - ("consumer", ConsumerClaimValue) + ("pid", "12345678901") ), $"{NorwegianPersonIdentifier.PrefixWithSeparator}16073422888"); @@ -187,7 +187,7 @@ public void CreateDialogDetailsRequestShouldReturnCorrectRequestForOverriddenRes // Arrange var request = CreateDialogDetailsAuthorizationRequest( GetAsClaims( - ("consumer", ConsumerClaimValue) + ("pid", "12345678901") ), $"{NorwegianPersonIdentifier.PrefixWithSeparator}16073422888"); @@ -209,12 +209,13 @@ public void CreateDialogDetailsRequestShouldReturnCorrectRequestForOverriddenRes } [Fact] + public void CreateDialogDetailsRequestShouldReturnCorrectRequestForFullyQualifiedSubresource() { // Arrange var request = CreateDialogDetailsAuthorizationRequest( GetAsClaims( - ("consumer", ConsumerClaimValue) + ("pid", "12345678901") ), $"{NorwegianPersonIdentifier.PrefixWithSeparator}16073422888"); @@ -239,7 +240,7 @@ public void CreateDialogDetailsResponseShouldReturnCorrectResponse() var request = CreateDialogDetailsAuthorizationRequest( GetAsClaims( // Should be copied as subject claim since there's not a "pid"-claim - ("consumer", ConsumerClaimValue) + ("pid", "12345678901") ), $"{NorwegianPersonIdentifier.PrefixWithSeparator}12345678901"); From 65bb180108e650031e574078e458a8823260b8d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Dybvik=20Langfors?= Date: Fri, 25 Oct 2024 10:01:06 +0200 Subject: [PATCH 4/8] More cleanups --- .../Authorization/DecisionRequestHelper.cs | 48 +++++++++---------- .../DecisionRequestHelperTests.cs | 24 +++++++--- 2 files changed, 40 insertions(+), 32 deletions(-) diff --git a/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs b/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs index 567f70ade..390db3c6f 100644 --- a/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs +++ b/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs @@ -76,42 +76,38 @@ public static DialogDetailsAuthorizationResult CreateDialogDetailsResponse(List< }; } - private static List CreateAccessSubjectCategory(IEnumerable claims) - { + private static List CreateAccessSubjectCategory(IEnumerable claims) => // The PDP expects for the most part only a single subject attribute, and will even fail the request // for some types (e.g. the urn:altinn:systemuser:uuid) if there are multiple subject attributes (for // security reasons). We therefore need to filter out the relevant attributes and only include those, - // which in essence is the pid and the systemuser uuid. In addition, we also utilize urn:altinn:userid + // which in essence is the pid and the system user uuid. In addition, we also utilize urn:altinn:userid // if present instead of the pid as a simple optimization as this offloads the PDP from having to look up // the user id from the pid. - foreach (var claim in claims) + claims.Select(claim => claim.Type switch { - if (claim.Type == UserIdClaimType) + UserIdClaimType => new XacmlJsonCategory { - return [new() { Id = SubjectId, Attribute = - [new XacmlJsonAttribute { AttributeId = AttributeIdUserId, Value = claim.Value }] }]; - } - - if (claim.Type == PidClaimType) + Id = SubjectId, + Attribute = [new() { AttributeId = AttributeIdUserId, Value = claim.Value }] + }, + PidClaimType => new XacmlJsonCategory { - return [new() { Id = SubjectId, Attribute = - [new XacmlJsonAttribute { AttributeId = NorwegianPersonIdentifier.Prefix, Value = claim.Value }] }]; - } - - if (claim.Type == RarAuthorizationDetailsClaimType) + Id = SubjectId, + Attribute = [new() { AttributeId = NorwegianPersonIdentifier.Prefix, Value = claim.Value }] + }, + RarAuthorizationDetailsClaimType when new ClaimsPrincipal(new ClaimsIdentity(new[] { claim })).TryGetSystemUserId(out var systemUserId) => new XacmlJsonCategory { - var claimsPrincipal = new ClaimsPrincipal(new ClaimsIdentity(new[] { claim })); - if (claimsPrincipal.TryGetSystemUserId(out var systemUserId)) - { - return [new() { Id = SubjectId, Attribute = - [new XacmlJsonAttribute { AttributeId = AttributeIdSystemUser, Value = systemUserId }] }]; - } - } - } - - throw new UnreachableException("Unable to find a suitable subject attribute for the authorization request. Having a known user type should be enforced during authentication (see UserTypeValidationMiddleware)."); - } + Id = SubjectId, + Attribute = [new XacmlJsonAttribute { AttributeId = AttributeIdSystemUser, Value = systemUserId }] + }, + _ => null + }) + .FirstOrDefault(x => x != null) switch + { + { } validCategory => new List { validCategory }, + _ => throw new UnreachableException("Unable to find a suitable subject attribute for the authorization request. Having a known user type should be enforced during authentication (see UserTypeValidationMiddleware)."), + }; private static List CreateActionCategories( List altinnActions, out Dictionary actionIdByName) diff --git a/tests/Digdir.Domain.Dialogporten.Infrastructure.Unit.Tests/DecisionRequestHelperTests.cs b/tests/Digdir.Domain.Dialogporten.Infrastructure.Unit.Tests/DecisionRequestHelperTests.cs index 583fa548b..623b7f94b 100644 --- a/tests/Digdir.Domain.Dialogporten.Infrastructure.Unit.Tests/DecisionRequestHelperTests.cs +++ b/tests/Digdir.Domain.Dialogporten.Infrastructure.Unit.Tests/DecisionRequestHelperTests.cs @@ -1,4 +1,5 @@ -using System.Security.Claims; +using System.Diagnostics; +using System.Security.Claims; using Altinn.Authorization.ABAC.Xacml.JsonProfile; using Digdir.Domain.Dialogporten.Application.Common.Authorization; using Digdir.Domain.Dialogporten.Application.Externals.AltinnAuthorization; @@ -10,8 +11,6 @@ namespace Digdir.Domain.Dialogporten.Infrastructure.Unit.Tests; public class DecisionRequestHelperTests { - private const string ConsumerClaimValue = /*lang=json,strict*/ "{\"authority\":\"iso6523-actorid-upis\",\"ID\":\"0192:991825827\"}"; - private const string AuthorizationDetailsClaimValue = /*lang=json,strict*/"[{\"type\":\"urn:altinn:systemuser\",\"systemuser_id\":[\"unique_systemuser_id\"]}]"; [Fact] @@ -136,8 +135,7 @@ public void CreateDialogDetailsRequestShouldReturnCorrectRequestForSystemUser() var request = CreateDialogDetailsAuthorizationRequest( GetAsClaims( ("authorization_details", AuthorizationDetailsClaimValue), - ("pid", "12345678901"), - ("consumer", ConsumerClaimValue) + ("pid", "12345678901") ), $"{NorwegianOrganizationIdentifier.PrefixWithSeparator}713330310" ); @@ -209,7 +207,6 @@ public void CreateDialogDetailsRequestShouldReturnCorrectRequestForOverriddenRes } [Fact] - public void CreateDialogDetailsRequestShouldReturnCorrectRequestForFullyQualifiedSubresource() { // Arrange @@ -264,6 +261,21 @@ public void CreateDialogDetailsResponseShouldReturnCorrectResponse() Assert.DoesNotContain(new AltinnAction("failaction", Constants.MainResource), response.AuthorizedAltinnActions); } + [Fact] + public void CreateDetailsRequestShouldThrowUnreachableExceptionIfNoValidUserType() + { + // Arrange + var request = CreateDialogDetailsAuthorizationRequest( + GetAsClaims( + ("consumer", "somevalue") + ), + $"{NorwegianOrganizationIdentifier.PrefixWithSeparator}713330310"); + + // Act / assert + Assert.Throws(() => DecisionRequestHelper.CreateDialogDetailsRequest(request)); + } + + private static DialogDetailsAuthorizationRequest CreateDialogDetailsAuthorizationRequest(List principalClaims, string party, bool isApp = false) { var allClaims = new List From 017787f3ee3477f0b90c68ea3bb1a16aad33ad60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Dybvik=20Langfors?= Date: Fri, 25 Oct 2024 10:31:24 +0200 Subject: [PATCH 5/8] Remove misleading comment --- .../DecisionRequestHelperTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Digdir.Domain.Dialogporten.Infrastructure.Unit.Tests/DecisionRequestHelperTests.cs b/tests/Digdir.Domain.Dialogporten.Infrastructure.Unit.Tests/DecisionRequestHelperTests.cs index 623b7f94b..9dca898be 100644 --- a/tests/Digdir.Domain.Dialogporten.Infrastructure.Unit.Tests/DecisionRequestHelperTests.cs +++ b/tests/Digdir.Domain.Dialogporten.Infrastructure.Unit.Tests/DecisionRequestHelperTests.cs @@ -236,7 +236,6 @@ public void CreateDialogDetailsResponseShouldReturnCorrectResponse() // Arrange var request = CreateDialogDetailsAuthorizationRequest( GetAsClaims( - // Should be copied as subject claim since there's not a "pid"-claim ("pid", "12345678901") ), $"{NorwegianPersonIdentifier.PrefixWithSeparator}12345678901"); From 6fd7d63a027db1c3188669b7f0451453f07ac792 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Dybvik=20Langfors?= Date: Fri, 25 Oct 2024 11:09:17 +0200 Subject: [PATCH 6/8] Explicit prioritization of which claims to use in mapping --- .../Authorization/DecisionRequestHelper.cs | 16 +++++++++++----- .../DecisionRequestHelperTests.cs | 13 +++++-------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs b/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs index 390db3c6f..1e6965951 100644 --- a/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs +++ b/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs @@ -26,6 +26,8 @@ internal static class DecisionRequestHelper private const string AttributeIdAppInstance = "urn:altinn:instance-id"; private const string AttributeIdSubResource = "urn:altinn:subresource"; private const string AttributeIdUserId = "urn:altinn:userid"; + // The order of these attribute types is important as we want to prioritize the most specific claim types. + private static readonly List PrioritizedClaimTypes = [AttributeIdUserId, NorwegianPersonIdentifier.Prefix, AttributeIdSystemUser]; private const string ReservedResourcePrefixForApps = "app_"; @@ -82,8 +84,7 @@ private static List CreateAccessSubjectCategory(IEnumerable claim.Type switch { UserIdClaimType => new XacmlJsonCategory @@ -99,14 +100,19 @@ private static List CreateAccessSubjectCategory(IEnumerable new XacmlJsonCategory { Id = SubjectId, - Attribute = [new XacmlJsonAttribute { AttributeId = AttributeIdSystemUser, Value = systemUserId }] + Attribute = + [ + new XacmlJsonAttribute { AttributeId = AttributeIdSystemUser, Value = systemUserId } + ] }, _ => null }) - .FirstOrDefault(x => x != null) switch + .Where(x => x != null) + .MinBy(x => PrioritizedClaimTypes.IndexOf(x!.Attribute.First().AttributeId)) switch { { } validCategory => new List { validCategory }, - _ => throw new UnreachableException("Unable to find a suitable subject attribute for the authorization request. Having a known user type should be enforced during authentication (see UserTypeValidationMiddleware)."), + _ => throw new UnreachableException( + "Unable to find a suitable subject attribute for the authorization request. Having a known user type should be enforced during authentication (see UserTypeValidationMiddleware)."), }; private static List CreateActionCategories( diff --git a/tests/Digdir.Domain.Dialogporten.Infrastructure.Unit.Tests/DecisionRequestHelperTests.cs b/tests/Digdir.Domain.Dialogporten.Infrastructure.Unit.Tests/DecisionRequestHelperTests.cs index 9dca898be..04682aa18 100644 --- a/tests/Digdir.Domain.Dialogporten.Infrastructure.Unit.Tests/DecisionRequestHelperTests.cs +++ b/tests/Digdir.Domain.Dialogporten.Infrastructure.Unit.Tests/DecisionRequestHelperTests.cs @@ -19,10 +19,9 @@ public void CreateDialogDetailsRequestShouldReturnCorrectRequest() // Arrange var request = CreateDialogDetailsAuthorizationRequest( GetAsClaims( - ("pid", "12345678901"), - // This should not be copied as subject claim since there's a "pid"-claim - ("authorization_details", AuthorizationDetailsClaimValue) + ("authorization_details", AuthorizationDetailsClaimValue), + ("pid", "12345678901") ), $"{NorwegianOrganizationIdentifier.PrefixWithSeparator}713330310"); var dialogId = request.DialogId; @@ -82,10 +81,9 @@ public void CreateDialogDetailsRequestShouldReturnCorrectRequestForExchangedToke // Arrange var request = CreateDialogDetailsAuthorizationRequest( GetAsClaims( - ("urn:altinn:userid", "5678901"), - // This should not be copied as subject claim since there's a "urn:altinn:user-id"-claim - ("pid", "12345678901") + ("pid", "12345678901"), + ("urn:altinn:userid", "5678901") ), $"{NorwegianOrganizationIdentifier.PrefixWithSeparator}713330310"); @@ -134,8 +132,7 @@ public void CreateDialogDetailsRequestShouldReturnCorrectRequestForSystemUser() // Arrange var request = CreateDialogDetailsAuthorizationRequest( GetAsClaims( - ("authorization_details", AuthorizationDetailsClaimValue), - ("pid", "12345678901") + ("authorization_details", AuthorizationDetailsClaimValue) ), $"{NorwegianOrganizationIdentifier.PrefixWithSeparator}713330310" ); From 21af193dfc553019640a78abe316e7ea44f280e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Dybvik=20Langfors?= Date: Fri, 25 Oct 2024 11:13:29 +0200 Subject: [PATCH 7/8] Clarify attribute ids --- .../Altinn/Authorization/DecisionRequestHelper.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs b/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs index 1e6965951..4449615c4 100644 --- a/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs +++ b/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs @@ -20,14 +20,18 @@ internal static class DecisionRequestHelper private const string AttributeIdAction = "urn:oasis:names:tc:xacml:1.0:action:action-id"; private const string AttributeIdResource = "urn:altinn:resource"; private const string AttributeIdResourceInstance = "urn:altinn:resourceinstance"; + private const string AttributeIdSubResource = "urn:altinn:subresource"; + private const string AttributeIdOrg = "urn:altinn:org"; private const string AttributeIdApp = "urn:altinn:app"; - private const string AttributeIdSystemUser = "urn:altinn:systemuser:uuid"; private const string AttributeIdAppInstance = "urn:altinn:instance-id"; - private const string AttributeIdSubResource = "urn:altinn:subresource"; + private const string AttributeIdUserId = "urn:altinn:userid"; + private const string AttributeIdPerson = "urn:altinn:person:identifier-no"; + private const string AttributeIdSystemUser = "urn:altinn:systemuser:uuid"; + // The order of these attribute types is important as we want to prioritize the most specific claim types. - private static readonly List PrioritizedClaimTypes = [AttributeIdUserId, NorwegianPersonIdentifier.Prefix, AttributeIdSystemUser]; + private static readonly List PrioritizedClaimTypes = [AttributeIdUserId, AttributeIdPerson, AttributeIdSystemUser]; private const string ReservedResourcePrefixForApps = "app_"; @@ -95,7 +99,7 @@ private static List CreateAccessSubjectCategory(IEnumerable new XacmlJsonCategory { Id = SubjectId, - Attribute = [new() { AttributeId = NorwegianPersonIdentifier.Prefix, Value = claim.Value }] + Attribute = [new() { AttributeId = AttributeIdPerson, Value = claim.Value }] }, RarAuthorizationDetailsClaimType when new ClaimsPrincipal(new ClaimsIdentity(new[] { claim })).TryGetSystemUserId(out var systemUserId) => new XacmlJsonCategory { From 9bf598254d3be75df486d8f3f1e991de998f2063 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Dybvik=20Langfors?= Date: Fri, 25 Oct 2024 11:15:29 +0200 Subject: [PATCH 8/8] Fix sonarcloud grievance --- .../Altinn/Authorization/DecisionRequestHelper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs b/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs index 4449615c4..a5f8f36b6 100644 --- a/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs +++ b/src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs @@ -112,7 +112,7 @@ private static List CreateAccessSubjectCategory(IEnumerable null }) .Where(x => x != null) - .MinBy(x => PrioritizedClaimTypes.IndexOf(x!.Attribute.First().AttributeId)) switch + .MinBy(x => PrioritizedClaimTypes.IndexOf(x!.Attribute[0].AttributeId)) switch { { } validCategory => new List { validCategory }, _ => throw new UnreachableException(