Skip to content

Commit

Permalink
Add sync adapter scope for administrative access
Browse files Browse the repository at this point in the history
  • Loading branch information
elsand committed Dec 12, 2024
1 parent fc0628c commit 069ebdb
Show file tree
Hide file tree
Showing 6 changed files with 127 additions and 1 deletion.
25 changes: 25 additions & 0 deletions src/Storage/Authorization/StorageAccessHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
using Altinn.Common.PEP.Constants;
using Altinn.Common.PEP.Helpers;
using Altinn.Common.PEP.Interfaces;
using Altinn.Platform.Storage.Configuration;
using Altinn.Platform.Storage.Extensions;
using Altinn.Platform.Storage.Helpers;
using Altinn.Platform.Storage.Interface.Models;
using Altinn.Platform.Storage.Repository;
using Microsoft.AspNetCore.Authorization;
Expand All @@ -31,6 +34,8 @@ public class StorageAccessHandler : AuthorizationHandler<AppAccessRequirement>
private readonly IInstanceRepository _instanceRepository;
private readonly IHttpContextAccessor _httpContextAccessor;
private readonly IPDP _pdp;
private readonly IAuthorization _authorizationService;
private readonly GeneralSettings _generalSettings;
private readonly ILogger _logger;
private readonly IMemoryCache _memoryCache;
private readonly PepSettings _pepSettings;
Expand All @@ -47,13 +52,17 @@ public class StorageAccessHandler : AuthorizationHandler<AppAccessRequirement>
public StorageAccessHandler(

Check warning on line 52 in src/Storage/Authorization/StorageAccessHandler.cs

View workflow job for this annotation

GitHub Actions / Build, test & analyze

Constructor has 8 parameters, which is greater than the 7 authorized. (https://rules.sonarsource.com/csharp/RSPEC-107)
IHttpContextAccessor httpContextAccessor,
IPDP pdp,
IAuthorization authorizationService,

Check warning on line 55 in src/Storage/Authorization/StorageAccessHandler.cs

View workflow job for this annotation

GitHub Actions / Analyze (csharp)

Parameter 'authorizationService' has no matching param tag in the XML comment for 'StorageAccessHandler.StorageAccessHandler(IHttpContextAccessor, IPDP, IAuthorization, IOptions<GeneralSettings>, IOptions<PepSettings>, ILogger<StorageAccessHandler>, IInstanceRepository, IMemoryCache)' (but other parameters do)

Check warning on line 55 in src/Storage/Authorization/StorageAccessHandler.cs

View workflow job for this annotation

GitHub Actions / Analyze (csharp)

Parameter 'authorizationService' has no matching param tag in the XML comment for 'StorageAccessHandler.StorageAccessHandler(IHttpContextAccessor, IPDP, IAuthorization, IOptions<GeneralSettings>, IOptions<PepSettings>, ILogger<StorageAccessHandler>, IInstanceRepository, IMemoryCache)' (but other parameters do)

Check warning on line 55 in src/Storage/Authorization/StorageAccessHandler.cs

View workflow job for this annotation

GitHub Actions / Build, test & analyze

Parameter 'authorizationService' has no matching param tag in the XML comment for 'StorageAccessHandler.StorageAccessHandler(IHttpContextAccessor, IPDP, IAuthorization, IOptions<GeneralSettings>, IOptions<PepSettings>, ILogger<StorageAccessHandler>, IInstanceRepository, IMemoryCache)' (but other parameters do)
IOptions<GeneralSettings> generalSettings,

Check warning on line 56 in src/Storage/Authorization/StorageAccessHandler.cs

View workflow job for this annotation

GitHub Actions / Analyze (csharp)

Parameter 'generalSettings' has no matching param tag in the XML comment for 'StorageAccessHandler.StorageAccessHandler(IHttpContextAccessor, IPDP, IAuthorization, IOptions<GeneralSettings>, IOptions<PepSettings>, ILogger<StorageAccessHandler>, IInstanceRepository, IMemoryCache)' (but other parameters do)

Check warning on line 56 in src/Storage/Authorization/StorageAccessHandler.cs

View workflow job for this annotation

GitHub Actions / Analyze (csharp)

Parameter 'generalSettings' has no matching param tag in the XML comment for 'StorageAccessHandler.StorageAccessHandler(IHttpContextAccessor, IPDP, IAuthorization, IOptions<GeneralSettings>, IOptions<PepSettings>, ILogger<StorageAccessHandler>, IInstanceRepository, IMemoryCache)' (but other parameters do)

Check warning on line 56 in src/Storage/Authorization/StorageAccessHandler.cs

View workflow job for this annotation

GitHub Actions / Build, test & analyze

Parameter 'generalSettings' has no matching param tag in the XML comment for 'StorageAccessHandler.StorageAccessHandler(IHttpContextAccessor, IPDP, IAuthorization, IOptions<GeneralSettings>, IOptions<PepSettings>, ILogger<StorageAccessHandler>, IInstanceRepository, IMemoryCache)' (but other parameters do)
IOptions<PepSettings> pepSettings,
ILogger<StorageAccessHandler> logger,
IInstanceRepository instanceRepository,
IMemoryCache memoryCache)
{
_httpContextAccessor = httpContextAccessor;
_pdp = pdp;
_authorizationService = authorizationService;
_generalSettings = generalSettings.Value;
_logger = logger;
_pepSettings = pepSettings.Value;
_instanceRepository = instanceRepository;
Expand All @@ -69,6 +78,12 @@ public StorageAccessHandler(
/// <returns>A Task</returns>
protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, AppAccessRequirement requirement)
{
if (IsValidAdapterRequest(context, requirement))
{
context.Succeed(requirement);
return;
}

XacmlJsonRequestRoot request = DecisionHelper.CreateDecisionRequest(context, requirement, _httpContextAccessor.HttpContext.GetRouteData());

_logger.LogInformation("// Storage PEP // AppAccessHandler // Request sent: {request}", JsonConvert.SerializeObject(request));

Check warning on line 89 in src/Storage/Authorization/StorageAccessHandler.cs

View workflow job for this annotation

GitHub Actions / Build, test & analyze

Use PascalCase for named placeholders. (https://rules.sonarsource.com/csharp/RSPEC-6678)
Expand Down Expand Up @@ -186,5 +201,15 @@ private static string GetCacheKeyForDecisionRequest(XacmlJsonRequestRoot request

return subjectKey.ToString() + actionKey.ToString() + resourceKey.ToString();
}

private bool IsValidAdapterRequest(AuthorizationHandlerContext context, AppAccessRequirement requirement)
{
if (requirement.ActionType != "read" && requirement.ActionType != "delete")
{
return false;
}

return _authorizationService.UserHasRequiredScope([_generalSettings.InstanceSyncAdapterScope]);
}
}
}
5 changes: 5 additions & 0 deletions src/Storage/Configuration/GeneralSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ public class GeneralSettings
/// </summary>
public List<string> InstanceReadScope { get; set; }

/// <summary>
/// Gets or sets the scope for storage sync adapters.
/// </summary>
public string InstanceSyncAdapterScope { get; set; }

/// <summary>
/// Gets or sets the cache lifetime for text resources.
/// </summary>
Expand Down
2 changes: 1 addition & 1 deletion src/Storage/Controllers/InstancesController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ public async Task<ActionResult<Instance>> Get(int instanceOwnerPartyId, Guid ins
{
(Instance result, _) = await _instanceRepository.GetOne(instanceGuid, true);

if (User.GetOrg() != result.Org)
if (User.GetOrg() != result.Org && !_authorizationService.UserHasRequiredScope([_generalSettings.InstanceSyncAdapterScope]))
{
FilterOutDeletedDataElements(result);
}
Expand Down
1 change: 1 addition & 0 deletions src/Storage/appsettings.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
"AppTitleCacheLifeTimeInSeconds": 3600,
"AppMetadataCacheLifeTimeInSeconds": 300,
"InstanceReadScope": [ "altinn:serviceowner/instances.read" ],
"InstanceSyncAdapterScope": "altinn:storage/instances.syncadapter",
"MigrationIpWhiteList": ""
},
"PlatformSettings": {
Expand Down
94 changes: 94 additions & 0 deletions test/UnitTest/TestingControllers/InstancesControllerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,29 @@ public async Task Get_One_Twice_Ok()
Assert.Equal(HttpStatusCode.OK, response2.StatusCode);
}

[Fact]
public async Task Get_One_With_SyncAdapterScope_Ok()
{
// Arrange
int instanceOwnerPartyId = 1337;
string instanceGuid = "377efa97-80ee-4cc6-8d48-09de12cc273d";
string requestUri = $"{BasePath}/{instanceOwnerPartyId}/{instanceGuid}";

HttpClient client = GetTestClient();
string token = PrincipalUtil.GetOrgToken("foo", scope: "altinn:storage/instances.syncadapter");
client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", token);

// Act
HttpResponseMessage response = await client.GetAsync(requestUri);

// Assert
Assert.Equal(0, RequestTracker.GetRequestCount("GetDecisionForRequest1337/377efa97-80ee-4cc6-8d48-09de12cc273d")); // We should not be hitting the PDP as sync adapter
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
string responseContent = await response.Content.ReadAsStringAsync();
Instance instance = (Instance)JsonConvert.DeserializeObject(responseContent, typeof(Instance));
Assert.Equal("1337", instance.InstanceOwner.PartyId);
}

/// <summary>
/// Test case: User tries to access element that he is not authorized for
/// Expected: Returns status forbidden.
Expand Down Expand Up @@ -178,6 +201,31 @@ public async Task Post_ReponseIsDeny_ReturnsStatusForbidden()
Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode);
}

/// <summary>
/// Test case: Sync adapters should not be allowed to write (only delete).
/// Expected: Returns status forbidden.
/// </summary>
[Fact]
public async Task Post_ReponseIsDenyForSyncAdapter_ReturnsStatusForbidden()
{
// Arrange
string appId = "tdd/endring-av-navn";
string requestUri = $"{BasePath}?appId={appId}";

HttpClient client = GetTestClient();
string token = PrincipalUtil.GetOrgToken("foo", scope: "altinn:storage/instances.syncadapter");
client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", token);

// Laste opp test instance..
Instance instance = new Instance() { InstanceOwner = new InstanceOwner() { PartyId = "1337" }, Org = "tdd", AppId = "tdd/endring-av-navn" };

// Act
HttpResponseMessage response = await client.PostAsync(requestUri, JsonContent.Create(instance, new MediaTypeHeaderValue("application/json")));

// Assert
Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode);
}

/// <summary>
/// Test case: User has to low authentication level.
/// Expected: Returns status forbidden.
Expand Down Expand Up @@ -310,6 +358,31 @@ public async Task Delete_OrgHardDeletesInstance_ReturnedInstanceHasStatusBothSof
Assert.Equal(deletedInstance.Status.HardDeleted, deletedInstance.Status.SoftDeleted);
}

[Fact]
public async Task Delete_With_SyncAdapterScope_Ok()
{
// Arrange
int instanceOwnerPartyId = 1337;
string instanceGuid = "377efa97-80ee-4cc6-8d48-09de12cc273d";
string requestUri = $"{BasePath}/{instanceOwnerPartyId}/{instanceGuid}?hard=true";

HttpClient client = GetTestClient();
string token = PrincipalUtil.GetOrgToken("foo", scope: "altinn:storage/instances.syncadapter");
client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", token);

// Act
HttpResponseMessage response = await client.DeleteAsync(requestUri);

string json = await response.Content.ReadAsStringAsync();
Instance deletedInstance = JsonConvert.DeserializeObject<Instance>(json);

// Assert
Assert.Equal(0, RequestTracker.GetRequestCount("GetDecisionForRequest1337/377efa97-80ee-4cc6-8d48-09de12cc273d")); // We should not be hitting the PDP as sync adapter
Assert.NotNull(deletedInstance.Status.HardDeleted);
Assert.NotNull(deletedInstance.Status.SoftDeleted);
Assert.Equal(deletedInstance.Status.HardDeleted, deletedInstance.Status.SoftDeleted);
}

/// <summary>
/// Test case: End user system tries to soft delete an instance
/// Expected: Returns success and deleted instance
Expand Down Expand Up @@ -805,6 +878,27 @@ public async Task GetMany_IncorrectScope_ReturnsForbidden()
Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode);
}

/// <summary>
/// Test case: Get Multiple instances using client with sync adapter scope should faile.
/// Expected: Returns status forbidden.
/// </summary>
[Fact]
public async Task GetMany_SyncAdapterScope_ReturnsForbidden()
{
// Arrange
string requestUri = $"{BasePath}?org=testOrg";

HttpClient client = GetTestClient();
string token = PrincipalUtil.GetOrgToken("testOrg", scope: "altinn:storage/instances.syncadapter");
client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", token);

// Act
HttpResponseMessage response = await client.GetAsync(requestUri);

// Assert
Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode);
}

/// <summary>
/// Scenario:
/// An app owner calls the API via apps-endpoints with a token that specifies a different
Expand Down
1 change: 1 addition & 0 deletions test/UnitTest/appsettings.unittest.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
"RuntimeCookieName": "AltinnStudioRuntime",
"BridgeApiAuthorizationEndpoint": "http://localhost:5055/sblbridge/authorization/api/",
"InstanceReadScope": [ "altinn:serviceowner/instances.read" ],
"InstanceSyncAdapterScope": "altinn:storage/instances.syncadapter",
"AppTitleCacheLifeTimeInSeconds": 60,
"AppMetadataCacheLifeTimeInSeconds": 60,
"TextResourceCacheLifeTimeInSeconds": 60,
Expand Down

0 comments on commit 069ebdb

Please sign in to comment.