Skip to content

Commit

Permalink
AM-377 Remove broken PartyId lookup for organization number (#381)
Browse files Browse the repository at this point in the history 
* AM-377 Remove broken PartyId lookup for organization number
#377
Old PartyId method in PartiesClient is not functional. It used to make calls to SBL bridge directly but has been changed to Register API but with incorrect model.

Removed old method and replaced with existing implementation using the PartyLookup model

* Fixed bug and code smells

* - Make admin/offered/received operations use the same helper method for building a complete Delegation model based on the change, offered/covered party and resource.
- Update missing fields in expected test data

* Reduced duplication

---------

Co-authored-by: Jon Kjetil Øye <acn-joye@ai-dev.no>
  • Loading branch information
@jonkjetiloye
jonkjetiloye and Jon Kjetil Øye authored Apr 12, 2023
1 parent 96b8553 commit e8b4fc5
Show file tree
Hide file tree
Showing 15 changed files with 140 additions and 310 deletions.
6 changes: 0 additions & 6 deletions src/Altinn.AccessManagement.Core/Clients/Interfaces/IPartiesClient.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,6 @@ public interface IPartiesClient
/// <returns>List of parties</returns>
Task<List<Party>> GetPartiesForUserAsync(int userId);

/// <summary>
/// Returns partyid for
/// </summary>
/// <returns>List of parties</returns>
Task<int> GetPartyId(string id);

/// <summary>
/// Method that fetches a list of PartyIds the given user id has key role access to (where the user inherit delegations to their organization)
/// </summary>
Expand Down
4 changes: 2 additions & 2 deletions src/Altinn.AccessManagement.Core/Models/Delegation.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,12 @@ public class Delegation
/// <summary>
/// Gets or sets the organization number that offered the delegation
/// </summary>
public int OfferedByOrganizationNumber { get; set; }
public string OfferedByOrganizationNumber { get; set; }

/// <summary>
/// Gets or sets the organization number that received the delegation
/// </summary>
public int CoveredByOrganizationNumber { get; set; }
public string CoveredByOrganizationNumber { get; set; }

/// <summary>
/// Gets or sets the resource id that is delegated
Expand Down
22 changes: 9 additions & 13 deletions src/Altinn.AccessManagement.Core/Services/ContextRetrievalService.cs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
using Altinn.AccessManagement.Core.Clients.Interfaces;
using Altinn.AccessManagement.Core.Configuration;
using Altinn.AccessManagement.Core.Helpers.Extensions;
using Altinn.AccessManagement.Core.Models.ResourceRegistry;
using Altinn.AccessManagement.Core.Models.SblBridge;
using Altinn.AccessManagement.Core.Services.Interfaces;
Expand All @@ -17,7 +16,6 @@ namespace Altinn.AccessManagement.Core.Services
/// </summary>
public class ContextRetrievalService : IContextRetrievalService
{
private readonly ILogger _logger;
private readonly CacheConfig _cacheConfig;
private readonly IMemoryCache _memoryCache;
private readonly IResourceRegistryClient _resourceRegistryClient;
Expand All @@ -27,15 +25,13 @@ public class ContextRetrievalService : IContextRetrievalService
/// <summary>
/// Initializes a new instance of the <see cref="ContextRetrievalService"/> class
/// </summary>
/// <param name="logger">Logger</param>
/// <param name="cacheConfig">Cache config</param>
/// <param name="memoryCache">The cache handler </param>
/// <param name="resourceRegistryClient">The client for integration with the ResourceRegistry</param>
/// <param name="altinnRolesClient">The client for integration with the SBL Bridge for role information</param>
/// <param name="partiesClient">The client for integration </param>
public ContextRetrievalService(ILogger<IContextRetrievalService> logger, IOptions<CacheConfig> cacheConfig, IMemoryCache memoryCache, IResourceRegistryClient resourceRegistryClient, IAltinnRolesClient altinnRolesClient, IPartiesClient partiesClient)
public ContextRetrievalService(IOptions<CacheConfig> cacheConfig, IMemoryCache memoryCache, IResourceRegistryClient resourceRegistryClient, IAltinnRolesClient altinnRolesClient, IPartiesClient partiesClient)
{
_logger = logger;
_cacheConfig = cacheConfig.Value;
_memoryCache = memoryCache;
_resourceRegistryClient = resourceRegistryClient;
Expand Down Expand Up @@ -128,25 +124,25 @@ public async Task<List<Party>> GetPartiesAsync(List<int> partyIds)
}

/// <inheritdoc/>
public async Task<int> GetPartyId(string ssnOrOrgno)
public async Task<Party> GetParty(string organizationNumber)
{
string cacheKey = $"ssnOrgno:{ssnOrOrgno}";
string cacheKey = $"orgNo:{organizationNumber}";

if (!_memoryCache.TryGetValue(cacheKey, out int partyId))
if (!_memoryCache.TryGetValue(cacheKey, out Party party))
{
partyId = await _partiesClient.GetPartyId(ssnOrOrgno);
party = await _partiesClient.LookupPartyBySSNOrOrgNo(new PartyLookup { OrgNo = organizationNumber });

if (partyId != 0)
if (party != null)
{
var cacheEntryOptions = new MemoryCacheEntryOptions()
.SetPriority(CacheItemPriority.High)
.SetAbsoluteExpiration(new TimeSpan(0, _cacheConfig.PartyCacheTimeout, 0));

_memoryCache.Set(cacheKey, partyId, cacheEntryOptions);
}
_memoryCache.Set(cacheKey, party, cacheEntryOptions);
}
}

return partyId;
return party;
}

/// <inheritdoc/>
Expand Down
163 changes: 69 additions & 94 deletions src/Altinn.AccessManagement.Core/Services/DelegationsService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ public class DelegationsService : IDelegationsService
private readonly IResourceAdministrationPoint _resourceAdministrationPoint;
private readonly IPolicyInformationPoint _pip;
private readonly IPolicyAdministrationPoint _pap;
private readonly IRegister _registerService;

/// <summary>
/// Initializes a new instance of the <see cref="DelegationsService"/> class.
Expand All @@ -33,16 +32,14 @@ public class DelegationsService : IDelegationsService
/// <param name="resourceAdministrationPoint">handler for resource registry</param>
/// <param name="pip">Service implementation for policy information point</param>
/// <param name="pap">Service implementation for policy administration point</param>
/// <param name="registerService">Service implementation for register lookup</param>
public DelegationsService(ILogger<IDelegationsService> logger, IDelegationMetadataRepository delegationRepository, IContextRetrievalService contextRetrievalService, IResourceAdministrationPoint resourceAdministrationPoint, IPolicyInformationPoint pip, IPolicyAdministrationPoint pap, IRegister registerService)
public DelegationsService(ILogger<IDelegationsService> logger, IDelegationMetadataRepository delegationRepository, IContextRetrievalService contextRetrievalService, IResourceAdministrationPoint resourceAdministrationPoint, IPolicyInformationPoint pip, IPolicyAdministrationPoint pap)
{
_logger = logger;
_delegationRepository = delegationRepository;
_contextRetrievalService = contextRetrievalService;
_resourceAdministrationPoint = resourceAdministrationPoint;
_pip = pip;
_pap = pap;
_registerService = registerService;
}

/// <inheritdoc/>
Expand Down Expand Up @@ -112,7 +109,8 @@ public async Task<List<Delegation>> GetOfferedMaskinportenSchemaDelegations(Attr
int offeredByPartyId = 0;
if (party.Id == AltinnXacmlConstants.MatchAttributeIdentifiers.OrganizationNumberAttribute)
{
offeredByPartyId = await _contextRetrievalService.GetPartyId(party.Value);
Party offeredByParty = await _contextRetrievalService.GetParty(party.Value);
offeredByPartyId = offeredByParty.PartyId;
}
else if (party.Id == AltinnXacmlConstants.MatchAttributeIdentifiers.PartyAttribute && (!int.TryParse(party.Value, out offeredByPartyId) || offeredByPartyId == 0))
{
Expand All @@ -133,7 +131,8 @@ public async Task<List<Delegation>> GetReceivedMaskinportenSchemaDelegations(Att
int coveredByPartyId = 0;
if (party.Id == AltinnXacmlConstants.MatchAttributeIdentifiers.OrganizationNumberAttribute)
{
coveredByPartyId = await _contextRetrievalService.GetPartyId(party.Value);
Party coveredByParty = await _contextRetrievalService.GetParty(party.Value);
coveredByPartyId = coveredByParty.PartyId;
}
else if (party.Id == AltinnXacmlConstants.MatchAttributeIdentifiers.PartyAttribute && (!int.TryParse(party.Value, out coveredByPartyId) || coveredByPartyId == 0))
{
Expand All @@ -146,8 +145,29 @@ public async Task<List<Delegation>> GetReceivedMaskinportenSchemaDelegations(Att
/// <inheritdoc/>
public async Task<List<Delegation>> GetMaskinportenSchemaDelegations(string supplierOrg, string consumerOrg, string scope)
{
int consumerPartyId = string.IsNullOrEmpty(consumerOrg) ? 0 : await _contextRetrievalService.GetPartyId(consumerOrg);
int supplierPartyId = string.IsNullOrEmpty(supplierOrg) ? 0 : await _contextRetrievalService.GetPartyId(supplierOrg);
int consumerPartyId = 0;
if (!string.IsNullOrEmpty(consumerOrg))
{
Party consumerParty = await _contextRetrievalService.GetParty(consumerOrg);
if (consumerParty == null)
{
throw new ArgumentException($"The specified consumerOrg: {consumerOrg}, is not a valid organization number");
}

consumerPartyId = consumerParty.PartyId;
}

int supplierPartyId = 0;
if (!string.IsNullOrEmpty(supplierOrg))
{
Party supplierParty = await _contextRetrievalService.GetParty(supplierOrg);
if (supplierParty == null)
{
throw new ArgumentException($"The specified supplierOrg: {supplierOrg}, is not a valid organization number");
}

supplierPartyId = supplierParty.PartyId;
}

if (!RegexUtil.IsValidMaskinportenScope(scope))
{
Expand Down Expand Up @@ -210,7 +230,7 @@ public async Task<DelegationActionResult> RevokeMaskinportenDelegation(int authe
Party fromParty = null;
if (DelegationHelper.TryGetOrganizationNumberFromAttributeMatch(delegation.From, out string fromOrgNo))
{
fromParty = await _registerService.GetOrganisation(fromOrgNo);
fromParty = await _contextRetrievalService.GetParty(fromOrgNo);
}
else if (DelegationHelper.TryGetPartyIdFromAttributeMatch(delegation.From, out int fromPartyId))
{
Expand All @@ -228,7 +248,7 @@ public async Task<DelegationActionResult> RevokeMaskinportenDelegation(int authe
Party toParty = null;
if (DelegationHelper.TryGetOrganizationNumberFromAttributeMatch(delegation.To, out string toOrgNo))
{
toParty = await _registerService.GetOrganisation(toOrgNo);
toParty = await _contextRetrievalService.GetParty(toOrgNo);
}
else if (DelegationHelper.TryGetPartyIdFromAttributeMatch(delegation.To, out int toPartyId))
{
Expand All @@ -255,43 +275,10 @@ private async Task<List<Delegation>> GetOfferedDelegations(int offeredByPartyId,
return delegations;
}

List<int> parties = new List<int>();
foreach (int party in delegationChanges.Select(d => d.CoveredByPartyId).Where(c => c != null).OfType<int>())
{
parties.Add(party);
}

List<ServiceResource> resources = new List<ServiceResource>();
List<Tuple<string, string>> resourceIds;
resourceIds = delegationChanges.Select(d => Tuple.Create(d.ResourceId, d.ResourceType)).ToList();

resources = await _resourceAdministrationPoint.GetResources(resourceIds);

List<Party> partyList = await _contextRetrievalService.GetPartiesAsync(parties);

foreach (DelegationChange delegationChange in delegationChanges)
{
Delegation delegation = new Delegation();
Party partyInfo = partyList.Find(p => p.PartyId == delegationChange.CoveredByPartyId);
delegation.CoveredByName = partyInfo?.Name;
delegation.CoveredByOrganizationNumber = Convert.ToInt32(partyInfo?.OrgNumber);
delegation.CoveredByPartyId = delegationChange.CoveredByPartyId;
delegation.OfferedByPartyId = delegationChange.OfferedByPartyId;
delegation.PerformedByUserId = delegationChange.PerformedByUserId;
delegation.PerformedByPartyId = delegationChange.PerformedByPartyId;
delegation.Created = delegationChange.Created.Value;
delegation.ResourceId = delegationChange.ResourceId;
ServiceResource resource = resources.Find(r => r.Identifier == delegationChange.ResourceId);
delegation.ResourceTitle = resource?.Title;
delegation.ResourceReferences = resource.ResourceReferences;
delegation.ResourceType = resource.ResourceType;
delegation.HasCompetentAuthority = resource.HasCompetentAuthority;
delegation.Description = resource.Description;
delegation.RightDescription = resource.RightDescription;
delegations.Add(delegation);
}
List<Tuple<string, string>> resourceIds = delegationChanges.Select(d => Tuple.Create(d.ResourceId, d.ResourceType)).ToList();
List<ServiceResource> resources = await _resourceAdministrationPoint.GetResources(resourceIds);

return delegations;
return await BuildDelegationsResponse(delegationChanges, resources);
}

private async Task<List<Delegation>> GetReceivedDelegations(int coveredByPartyId, ResourceType resourceType)
Expand All @@ -304,38 +291,10 @@ private async Task<List<Delegation>> GetReceivedDelegations(int coveredByPartyId
return delegations;
}

List<int> parties = new List<int>();
parties = delegationChanges.Select(d => d.OfferedByPartyId).ToList();

List<ServiceResource> resources = new List<ServiceResource>();
List<Tuple<string, string>> resourceIds;
resourceIds = delegationChanges.Select(d => Tuple.Create(d.ResourceId, d.ResourceType)).ToList();
resources = await _resourceAdministrationPoint.GetResources(resourceIds);

List<Party> partyList = await _contextRetrievalService.GetPartiesAsync(parties);

foreach (DelegationChange delegationChange in delegationChanges)
{
Delegation delegation = new Delegation();
Party partyInfo = partyList.Find(p => p.PartyId == delegationChange.OfferedByPartyId);
delegation.OfferedByName = partyInfo?.Name;
delegation.OfferedByOrganizationNumber = Convert.ToInt32(partyInfo?.OrgNumber);
delegation.CoveredByPartyId = delegationChange.CoveredByPartyId;
delegation.OfferedByPartyId = delegationChange.OfferedByPartyId;
delegation.PerformedByUserId = delegationChange.PerformedByUserId;
delegation.Created = delegationChange.Created.Value;
delegation.ResourceId = delegationChange.ResourceId;
ServiceResource resource = resources.Find(r => r.Identifier == delegationChange.ResourceId);
delegation.ResourceTitle = resource?.Title;
delegation.ResourceReferences = resource.ResourceReferences;
delegation.ResourceType = resource.ResourceType;
delegation.HasCompetentAuthority = resource.HasCompetentAuthority;
delegation.Description = resource.Description;
delegation.RightDescription = resource.RightDescription;
delegations.Add(delegation);
}
List<Tuple<string, string>> resourceIds = delegationChanges.Select(d => Tuple.Create(d.ResourceId, d.ResourceType)).ToList();
List<ServiceResource> resources = await _resourceAdministrationPoint.GetResources(resourceIds);

return delegations;
return await BuildDelegationsResponse(delegationChanges, resources);
}

private async Task<List<Delegation>> GetAllMaskinportenSchemaDelegations(int supplierPartyId, int consumerPartyId, string scopes)
Expand All @@ -354,33 +313,49 @@ private async Task<List<Delegation>> GetAllMaskinportenSchemaDelegations(int sup
return delegations;
}

return await BuildDelegationsResponse(delegationChanges, resources);
}

private async Task<List<Delegation>> BuildDelegationsResponse(List<DelegationChange> delegationChanges, List<ServiceResource> resources)
{
List<Delegation> delegations = new List<Delegation>();
List<int> parties = delegationChanges.Select(d => d.OfferedByPartyId).ToList();
parties.AddRange(delegationChanges.Select(d => d.CoveredByPartyId).Select(ds => Convert.ToInt32(ds)).ToList());

List<Party> partyList = await _contextRetrievalService.GetPartiesAsync(parties);

foreach (DelegationChange delegationChange in delegationChanges)
{
Delegation delegation = new Delegation();
Party partyInfo = partyList.Find(p => p.PartyId == delegationChange.OfferedByPartyId);
Party coveredByPartyInfo = partyList.Find(p => p.PartyId == delegationChange.CoveredByPartyId);
delegation.OfferedByName = partyInfo?.Name;
delegation.OfferedByOrganizationNumber = Convert.ToInt32(partyInfo?.OrgNumber);
delegation.CoveredByName = coveredByPartyInfo?.Name;
delegation.CoveredByOrganizationNumber = Convert.ToInt32(coveredByPartyInfo?.OrgNumber);
delegation.CoveredByPartyId = delegationChange.CoveredByPartyId;
delegation.OfferedByPartyId = delegationChange.OfferedByPartyId;
delegation.PerformedByUserId = delegationChange.PerformedByUserId;
delegation.Created = delegationChange.Created ?? DateTime.MinValue;
delegation.ResourceId = delegationChange.ResourceId;
Party offeredByParty = partyList.Find(p => p.PartyId == delegationChange.OfferedByPartyId);
Party coveredByParty = partyList.Find(p => p.PartyId == delegationChange.CoveredByPartyId);
ServiceResource resource = resources.Find(r => r.Identifier == delegationChange.ResourceId);
delegation.ResourceTitle = resource?.Title;
delegation.ResourceReferences = resource.ResourceReferences;
delegation.ResourceType = resource.ResourceType;
delegations.Add(delegation);
delegations.Add(BuildDelegationModel(delegationChange, offeredByParty, coveredByParty, resource));
}

return delegations;
}

private static Delegation BuildDelegationModel(DelegationChange delegationChange, Party offeredByParty, Party coveredByParty, ServiceResource resource)
{
return new Delegation
{
OfferedByPartyId = delegationChange.OfferedByPartyId,
OfferedByName = offeredByParty?.Name,
OfferedByOrganizationNumber = offeredByParty?.OrgNumber,
CoveredByPartyId = delegationChange.CoveredByPartyId,
CoveredByName = coveredByParty?.Name,
CoveredByOrganizationNumber = coveredByParty?.OrgNumber,
PerformedByUserId = delegationChange.PerformedByUserId,
PerformedByPartyId = delegationChange.PerformedByPartyId,
Created = delegationChange.Created ?? DateTime.MinValue,
ResourceId = delegationChange.ResourceId,
ResourceType = resource?.ResourceType ?? ResourceType.Default,
ResourceTitle = resource?.Title,
Description = resource?.Description,
RightDescription = resource?.RightDescription,
ResourceReferences = resource?.ResourceReferences,
HasCompetentAuthority = resource?.HasCompetentAuthority
};
}
}
}
Loading

0 comments on commit e8b4fc5

Please sign in to comment.