Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Simplify subject attribute matching #1348

Merged
merged 9 commits into from
Oct 30, 2024
Merged

fix: Simplify subject attribute matching #1348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1cbee77
Simplify subject attribute matching
elsand Oct 25, 2024
2cd9df0
Add assertion
elsand Oct 25, 2024
291e2eb
Add assertion, clean up
elsand Oct 25, 2024
65bb180
More cleanups
elsand Oct 25, 2024
017787f
Remove misleading comment
elsand Oct 25, 2024
6fd7d63
Explicit prioritization of which claims to use in mapping
elsand Oct 25, 2024
21af193
Clarify attribute ids
elsand Oct 25, 2024
9bf5982
Fix sonarcloud grievance
elsand Oct 25, 2024
5ebf1b8
Merge branch 'main' into fix/simplify-subject-attribute-matching
elsand Oct 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 52 additions & 38 deletions src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/DecisionRequestHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
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;
using Digdir.Domain.Dialogporten.Application.Externals.AltinnAuthorization;
using Digdir.Domain.Dialogporten.Domain.Parties;
Expand All @@ -10,20 +12,29 @@ 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 RarAuthorizationDetailsClaimType = "authorization_details";

private const string AttributeIdAction = "urn:oasis:names:tc:xacml:1.0:action:action-id";
elsand marked this conversation as resolved.
Show resolved Hide resolved
private const string AttributeIdResource = "urn:altinn:resource";
private const string AttributeIdResourceInstance = "urn:altinn:resourceinstance";
private const string AltinnAutorizationDetailsClaim = "authorization_details";
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 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<string> PrioritizedClaimTypes = [AttributeIdUserId, AttributeIdPerson, AttributeIdSystemUser];

private const string ReservedResourcePrefixForApps = "app_";
private const string AttributeIdAppInstance = "urn:altinn:instance-id";
private const string AttributeIdSubResource = "urn:altinn:subresource";

private const string PermitResponse = "Permit";

public static XacmlJsonRequestRoot CreateDialogDetailsRequest(DialogDetailsAuthorizationRequest request)
Expand Down Expand Up @@ -71,39 +82,42 @@ public static DialogDetailsAuthorizationResult CreateDialogDetailsResponse(List<
};
}

private static List<XacmlJsonCategory> CreateAccessSubjectCategory(IEnumerable<Claim> claims)
{
var attributes = claims
.Select(x => x switch
private static List<XacmlJsonCategory> CreateAccessSubjectCategory(IEnumerable<Claim> 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 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. See PrioritizedClaimTypes for the order of prioritization.
claims.Select(claim => claim.Type switch
{
UserIdClaimType => new XacmlJsonCategory
{
{ 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<XacmlJsonAttribute>()
.ToList();

// 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))
Id = SubjectId,
Attribute = [new() { AttributeId = AttributeIdUserId, Value = claim.Value }]
},
PidClaimType => new XacmlJsonCategory
{
Id = SubjectId,
Attribute = [new() { AttributeId = AttributeIdPerson, Value = claim.Value }]
},
RarAuthorizationDetailsClaimType when new ClaimsPrincipal(new ClaimsIdentity(new[] { claim })).TryGetSystemUserId(out var systemUserId) => new XacmlJsonCategory
{
Id = SubjectId,
Attribute =
[
new XacmlJsonAttribute { AttributeId = AttributeIdSystemUser, Value = systemUserId }
]
},
_ => null
})
.Where(x => x != null)
.MinBy(x => PrioritizedClaimTypes.IndexOf(x!.Attribute[0].AttributeId)) switch
{
attributes.RemoveAll(x => x.AttributeId == NorwegianOrganizationIdentifier.Prefix);
}

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!;
}
{ } validCategory => new List<XacmlJsonCategory> { 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<XacmlJsonCategory> CreateActionCategories(
List<AltinnAction> altinnActions, out Dictionary<string, string> actionIdByName)
Expand Down
73 changes: 29 additions & 44 deletions tests/Digdir.Domain.Dialogporten.Infrastructure.Unit.Tests/DecisionRequestHelperTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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]
Expand All @@ -20,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
("consumer", ConsumerClaimValue)
("authorization_details", AuthorizationDetailsClaimValue),
("pid", "12345678901")
),
$"{NorwegianOrganizationIdentifier.PrefixWithSeparator}713330310");
var dialogId = request.DialogId;
Expand All @@ -42,9 +40,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<string, string>();
Expand Down Expand Up @@ -79,15 +76,14 @@ public void CreateDialogDetailsRequestShouldReturnCorrectRequest()
}

[Fact]
public void CreateDialogDetailsRequestShouldReturnCorrectRequestForLegacyEnterpriseUsers()
public void CreateDialogDetailsRequestShouldReturnCorrectRequestForExchangedTokens()
{
// 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
("consumer", ConsumerClaimValue)
("pid", "12345678901"),
("urn:altinn:userid", "5678901")
),
$"{NorwegianOrganizationIdentifier.PrefixWithSeparator}713330310");

Expand All @@ -98,7 +94,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]
Expand Down Expand Up @@ -151,33 +147,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]
Expand All @@ -186,7 +157,7 @@ public void CreateDialogDetailsRequestShouldReturnCorrectRequestForOverriddenRes
// Arrange
var request = CreateDialogDetailsAuthorizationRequest(
GetAsClaims(
("consumer", ConsumerClaimValue)
("pid", "12345678901")
),
$"{NorwegianPersonIdentifier.PrefixWithSeparator}16073422888");

Expand All @@ -211,7 +182,7 @@ public void CreateDialogDetailsRequestShouldReturnCorrectRequestForOverriddenRes
// Arrange
var request = CreateDialogDetailsAuthorizationRequest(
GetAsClaims(
("consumer", ConsumerClaimValue)
("pid", "12345678901")
),
$"{NorwegianPersonIdentifier.PrefixWithSeparator}16073422888");

Expand All @@ -238,7 +209,7 @@ public void CreateDialogDetailsRequestShouldReturnCorrectRequestForFullyQualifie
// Arrange
var request = CreateDialogDetailsAuthorizationRequest(
GetAsClaims(
("consumer", ConsumerClaimValue)
("pid", "12345678901")
),
$"{NorwegianPersonIdentifier.PrefixWithSeparator}16073422888");

Expand All @@ -262,8 +233,7 @@ public void CreateDialogDetailsResponseShouldReturnCorrectResponse()
// Arrange
var request = CreateDialogDetailsAuthorizationRequest(
GetAsClaims(
// Should be copied as subject claim since there's not a "pid"-claim
("consumer", ConsumerClaimValue)
("pid", "12345678901")
),
elsand marked this conversation as resolved.
Show resolved Hide resolved
$"{NorwegianPersonIdentifier.PrefixWithSeparator}12345678901");

Expand All @@ -287,6 +257,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<UnreachableException>(() => DecisionRequestHelper.CreateDialogDetailsRequest(request));
}


private static DialogDetailsAuthorizationRequest CreateDialogDetailsAuthorizationRequest(List<Claim> principalClaims, string party, bool isApp = false)
{
var allClaims = new List<Claim>
Expand Down
Loading