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.

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

Feature/xacmlvalidation #504

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 1 addition & 2 deletions src/Altinn.ResourceRegistry.Core/Helpers/PolicyHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@
{
List<AttributeMatch> xacmlResources = GetResourceFromXacmlRule(policyRule);

if (xacmlResources.Any(r => r.Id.Equals(MatchAttributeIdentifiers.ResourceRegistryAttribute) && !r.Value.Equals(serviceResources.Identifier)))

Check warning on line 33 in src/Altinn.ResourceRegistry.Core/Helpers/PolicyHelper.cs

View workflow job for this annotation

GitHub Actions / Analyze

Collection-specific "Exists" method should be used instead of the "Any" extension. (https://rules.sonarsource.com/csharp/RSPEC-6605)
{
throw new ArgumentException("Policy not accepted: Contains rules for a different registry resource");
}

if (!xacmlResources.Any(r => r.Id.Equals(MatchAttributeIdentifiers.ResourceRegistryAttribute) && r.Value.Equals(serviceResources.Identifier)))

Check warning on line 38 in src/Altinn.ResourceRegistry.Core/Helpers/PolicyHelper.cs

View workflow job for this annotation

GitHub Actions / Analyze

Collection-specific "Exists" method should be used instead of the "Any" extension. (https://rules.sonarsource.com/csharp/RSPEC-6605)
{
throw new ArgumentException("Policy not accepted: Contains rule without reference to registry resource id");
}
Expand All @@ -47,7 +47,7 @@
/// </summary>
/// <param name="policy">The policy</param>
/// <returns>List of role codes</returns>
public static List<string> GetRolesWithAccess(XacmlPolicy policy)

Check warning on line 50 in src/Altinn.ResourceRegistry.Core/Helpers/PolicyHelper.cs

View workflow job for this annotation

GitHub Actions / Analyze

Refactor this method to reduce its Cognitive Complexity from 22 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
{
HashSet<string> roleCodes = new HashSet<string>();

Expand All @@ -59,7 +59,7 @@
{
foreach (XacmlAllOf allOf in anyOf.AllOf)
{
foreach (XacmlMatch xacmlMatch in allOf.Matches)

Check warning on line 62 in src/Altinn.ResourceRegistry.Core/Helpers/PolicyHelper.cs

View workflow job for this annotation

GitHub Actions / Analyze

Loops should be simplified using the "Where" LINQ method (https://rules.sonarsource.com/csharp/RSPEC-3267)
{
if (xacmlMatch.AttributeDesignator.AttributeId.Equals(AltinnXacmlConstants.MatchAttributeIdentifiers.RoleAttribute))
{
Expand Down Expand Up @@ -215,7 +215,7 @@
/// Subject have multiple matches in a AllOf element, but it is not used in the current implementation in Altinn. (requiring a user to have multiple roles to access a resource)
/// A resource can have multiple matched in a AllOf element to be able to match on multiple attributes. (app, task1 , task2 etc)
/// </summary>
private static void FlattenXacmlRule(XacmlRule xacmlRule, List<PolicyRule> policyRules)

Check warning on line 218 in src/Altinn.ResourceRegistry.Core/Helpers/PolicyHelper.cs

View workflow job for this annotation

GitHub Actions / Analyze

Refactor this method to reduce its Cognitive Complexity from 21 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
{
XacmlAnyOf anyOfSubjects = null;
XacmlAnyOf anyOfActions = null;
Expand All @@ -227,7 +227,7 @@

foreach (XacmlAllOf allOf in anyOf.AllOf)
{
foreach (XacmlMatch match in allOf.Matches)

Check warning on line 230 in src/Altinn.ResourceRegistry.Core/Helpers/PolicyHelper.cs

View workflow job for this annotation

GitHub Actions / Analyze

Loop should be simplified by calling Select(match => match.AttributeDesignator)) (https://rules.sonarsource.com/csharp/RSPEC-3267)
{
if (category == null)
{
Expand All @@ -240,7 +240,7 @@
}
}

if (category.Equals(XacmlConstants.MatchAttributeCategory.Action))

Check warning on line 243 in src/Altinn.ResourceRegistry.Core/Helpers/PolicyHelper.cs

View workflow job for this annotation

GitHub Actions / Analyze

'category' is null on at least one execution path. (https://rules.sonarsource.com/csharp/RSPEC-2259)
{
anyOfActions = anyOf;
}
Expand All @@ -254,9 +254,9 @@
}
}

foreach (XacmlAllOf allOfSubject in anyOfSubjects.AllOf)

Check warning on line 257 in src/Altinn.ResourceRegistry.Core/Helpers/PolicyHelper.cs

View workflow job for this annotation

GitHub Actions / Analyze

'anyOfSubjects' is null on at least one execution path. (https://rules.sonarsource.com/csharp/RSPEC-2259)
{
foreach (XacmlAllOf allOfAction in anyOfActions.AllOf)

Check warning on line 259 in src/Altinn.ResourceRegistry.Core/Helpers/PolicyHelper.cs

View workflow job for this annotation

GitHub Actions / Analyze

'anyOfActions' is null on at least one execution path. (https://rules.sonarsource.com/csharp/RSPEC-2259)
{
foreach (XacmlAllOf allOfResource in anyOfResourcs.AllOf)
{
Expand Down Expand Up @@ -308,7 +308,7 @@
return KeyValueUrn.Create($"{matchId}:{match.AttributeValue.Value}", matchId.Length + 1);
}

private static AttributeMatch GetActionValueFromRule(XacmlRule rule)

Check warning on line 311 in src/Altinn.ResourceRegistry.Core/Helpers/PolicyHelper.cs

View workflow job for this annotation

GitHub Actions / Analyze

Remove the unused private method 'GetActionValueFromRule'. (https://rules.sonarsource.com/csharp/RSPEC-1144)
{
foreach (XacmlAnyOf anyOf in rule.Target.AnyOf)
{
Expand Down Expand Up @@ -346,7 +346,6 @@
}

return result;
}

}
}
}
6 changes: 6 additions & 0 deletions src/Altinn.ResourceRegistry/Controllers/ResourceController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.ApplicationModels;
using Microsoft.AspNetCore.Mvc.ModelBinding.Validation;
using System.Xml;

Check failure on line 17 in src/Altinn.ResourceRegistry/Controllers/ResourceController.cs

View workflow job for this annotation

GitHub Actions / Analyze (csharp)

Using directive for 'System.Xml' should appear before directive for 'Microsoft.AspNetCore.Mvc.ModelBinding.Validation' (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1208.md)

Check failure on line 17 in src/Altinn.ResourceRegistry/Controllers/ResourceController.cs

View workflow job for this annotation

GitHub Actions / Analyze (csharp)

Using directive for 'System.Xml' should appear before directive for 'Microsoft.AspNetCore.Mvc.ModelBinding.Validation' (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1208.md)

Check failure on line 17 in src/Altinn.ResourceRegistry/Controllers/ResourceController.cs

View workflow job for this annotation

GitHub Actions / Analyze

Using directive for 'System.Xml' should appear before directive for 'Microsoft.AspNetCore.Mvc.ModelBinding.Validation' (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1208.md)

Check failure on line 17 in src/Altinn.ResourceRegistry/Controllers/ResourceController.cs

View workflow job for this annotation

GitHub Actions / Analyze

Using directive for 'System.Xml' should appear before directive for 'Microsoft.AspNetCore.Mvc.ModelBinding.Validation' (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1208.md)

Check failure on line 17 in src/Altinn.ResourceRegistry/Controllers/ResourceController.cs

View workflow job for this annotation

GitHub Actions / Build and Test

Using directive for 'System.Xml' should appear before directive for 'Microsoft.AspNetCore.Mvc.ModelBinding.Validation' (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1208.md)

Check failure on line 17 in src/Altinn.ResourceRegistry/Controllers/ResourceController.cs

View workflow job for this annotation

GitHub Actions / Build and Test

Using directive for 'System.Xml' should appear before directive for 'Microsoft.AspNetCore.Mvc.ModelBinding.Validation' (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1208.md)

namespace Altinn.ResourceRegistry.Controllers
{
Expand Down Expand Up @@ -395,6 +396,11 @@
_logger.LogError(ex.Message);
return BadRequest(ex.Message);
}
catch (XmlException ex)
{
_logger.LogError(ex.Message);
return BadRequest(ex.Message);
}
catch (Exception ex)
{
_logger.LogError(ex.ToString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@
<None Update="appsettings.test.json">
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
</None>
<None Update="Data\ResourcePolicies\altinn_access_management_missinganyof.xml">
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
</None>
<None Update="Data\ResourcePolicies\altinn_access_management\resourcepolicy.xml">
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
</None>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<?xml version="1.0" encoding="utf-8"?>
<xacml:Policy xmlns:xsl="http://www.w3.org/2001/XMLSchema-instance" xmlns:xacml="urn:oasis:names:tc:xacml:3.0:core:schema:wd-17" PolicyId="urn:altinn:example:policyid:1" Version="1.0" RuleCombiningAlgId="urn:oasis:names:tc:xacml:3.0:rule-combining-algorithm:deny-overrides">
<xacml:Target/>
<xacml:Rule RuleId="urn:altinn:example:ruleid:1" Effect="Permit">
<xacml:Description>Policy for altinn_access_management</xacml:Description>
<xacml:Target>
<xacml:AnyOf>
<xacml:AllOf>
<xacml:Match MatchId="urn:oasis:names:tc:xacml:3.0:function:string-equal-ignore-case">
<xacml:AttributeValue DataType="http://www.w3.org/2001/XMLSchema#string">ADMAI</xacml:AttributeValue>
<xacml:AttributeDesignator AttributeId="urn:altinn:rolecode" Category="urn:oasis:names:tc:xacml:1.0:subject-category:access-subject" DataType="http://www.w3.org/2001/XMLSchema#string" MustBePresent="false"/>
</xacml:Match>
</xacml:AllOf>
</xacml:AnyOf>
<xacml:AnyOf>
<xacml:AllOf>
<xacml:Match MatchId="urn:oasis:names:tc:xacml:1.0:function:string-equal">
<xacml:AttributeValue DataType="http://www.w3.org/2001/XMLSchema#string">altinn_access_management</xacml:AttributeValue>
<xacml:AttributeDesignator AttributeId="urn:altinn:resource" Category="urn:oasis:names:tc:xacml:3.0:attribute-category:resource" DataType="http://www.w3.org/2001/XMLSchema#string" MustBePresent="false"/>
</xacml:Match>
</xacml:AllOf>
</xacml:AnyOf>
<xacml:AllOf>
<xacml:Match MatchId="urn:oasis:names:tc:xacml:1.0:function:string-equal">
<xacml:AttributeValue DataType="http://www.w3.org/2001/XMLSchema#string">read</xacml:AttributeValue>
<xacml:AttributeDesignator AttributeId="urn:oasis:names:tc:xacml:1.0:action:action-id" Category="urn:oasis:names:tc:xacml:3.0:attribute-category:action" DataType="http://www.w3.org/2001/XMLSchema#string" MustBePresent="false"/>
</xacml:Match>
</xacml:AllOf>
<xacml:AllOf>
<xacml:Match MatchId="urn:oasis:names:tc:xacml:1.0:function:string-equal">
<xacml:AttributeValue DataType="http://www.w3.org/2001/XMLSchema#string">write</xacml:AttributeValue>
<xacml:AttributeDesignator AttributeId="urn:oasis:names:tc:xacml:1.0:action:action-id" Category="urn:oasis:names:tc:xacml:3.0:attribute-category:action" DataType="http://www.w3.org/2001/XMLSchema#string" MustBePresent="false"/>
</xacml:Match>
</xacml:AllOf>
</xacml:Target>
</xacml:Rule>
<xacml:ObligationExpressions>
<xacml:ObligationExpression FulfillOn="Permit" ObligationId="urn:altinn:obligation:authenticationLevel1">
<xacml:AttributeAssignmentExpression AttributeId="urn:altinn:obligation1-assignment1" Category="urn:altinn:minimum-authenticationlevel">
<xacml:AttributeValue DataType="http://www.w3.org/2001/XMLSchema#integer">2</xacml:AttributeValue>
</xacml:AttributeAssignmentExpression>
</xacml:ObligationExpression>
</xacml:ObligationExpressions>
</xacml:Policy>
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,52 @@ public async Task SetResourcePolicy_OK()

}

[Fact]
public async Task SetResourcePolicy_MissingAnyOf_()
{
// Add one that should be marked as deleted when updating with policy
await Repository.SetResourceSubjects(CreateResourceSubjects("urn:altinn:resource:altinn_access_management", ["urn:altinn:rolecode:tobedeleted"], "skd"));

ServiceResource resource = new ServiceResource()
{
Identifier = "altinn_access_management",
HasCompetentAuthority = new CompetentAuthority()
{
Organization = "974761076",
Orgcode = "skd"
}
};
await Repository.CreateResource(resource);

using var client = CreateClient();
string token = PrincipalUtil.GetOrgToken("skd", "974761076", "altinn:resourceregistry/resource.write");
client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", token);


string fileName = $"{resource.Identifier}_missinganyof.xml";
string filePath = $"Data/ResourcePolicies/{fileName}";

Uri requestUri = new Uri($"resourceregistry/api/v1/Resource/{resource.Identifier}/policy", UriKind.Relative);

ByteArrayContent fileContent = new ByteArrayContent(File.ReadAllBytes(filePath));
fileContent.Headers.ContentType = MediaTypeHeaderValue.Parse("text/xml");

MultipartFormDataContent content = new();
content.Add(fileContent, "policyFile", fileName);

HttpRequestMessage httpRequestMessage = new() { Method = HttpMethod.Post, RequestUri = requestUri, Content = content };
httpRequestMessage.Headers.Add("ContentType", "multipart/form-data");

HttpResponseMessage response = await client.SendAsync(httpRequestMessage);

Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);

string responseContent = await response.Content.ReadAsStringAsync();

Assert.Contains("invalid XmlNodeType", responseContent);

}

[Fact]
public async Task GetUpdatedResourceSubjects_Paginates()
{
Expand Down
Loading