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

Endpoint for fetching current party roles #983

Open
wants to merge 6 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
121 changes: 75 additions & 46 deletions src/Altinn.App.Api/Controllers/AuthorizationController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
using Altinn.App.Core.Internal.Profile;
using Altinn.App.Core.Internal.Registers;
using Altinn.App.Core.Models;
using Altinn.Platform.Register.Models;
using Authorization.Platform.Authorization.Models;
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.Options;
Expand Down Expand Up @@ -44,57 +46,14 @@ IOptions<GeneralSettings> settings
[HttpGet("{org}/{app}/api/authorization/parties/current")]
public async Task<ActionResult> GetCurrentParty(bool returnPartyObject = false)
{
UserContext userContext = await _userHelper.GetUserContext(HttpContext);
int userId = userContext.UserId;

// If selected party is different than party for user self need to verify
if (userContext.UserParty == null || userContext.PartyId != userContext.UserParty.PartyId)
{
bool? isValid = await _authorization.ValidateSelectedParty(userId, userContext.PartyId);

if (isValid == true)
{
if (returnPartyObject)
{
return Ok(userContext.Party);
}

return Ok(userContext.PartyId);
}
else if (userContext.UserParty != null)
{
userContext.Party = userContext.UserParty;
userContext.PartyId = userContext.UserParty.PartyId;
}
else
{
userContext.Party = null;
userContext.PartyId = 0;
}
}

string? cookieValue = Request.Cookies[_settings.GetAltinnPartyCookieName];
if (!int.TryParse(cookieValue, out int partyIdFromCookie))
{
partyIdFromCookie = 0;
}

// Setting cookie to partyID of logged in user if it varies from previus value.
if (partyIdFromCookie != userContext.PartyId)
{
Response.Cookies.Append(
_settings.GetAltinnPartyCookieName,
userContext.PartyId.ToString(CultureInfo.InvariantCulture),
new CookieOptions { Domain = _settings.HostName }
);
}
(Party? currentParty, _) = await GetCurrentPartyAsync(HttpContext);

if (returnPartyObject)
{
return Ok(userContext.Party);
return Ok(currentParty);
}

return Ok(userContext.PartyId);
return Ok(currentParty?.PartyId ?? 0);
}

/// <summary>
Expand Down Expand Up @@ -123,4 +82,74 @@ public async Task<IActionResult> ValidateSelectedParty(int userId, int partyId)
return StatusCode(500, $"Something went wrong when trying to validate party {partyId} for user {userId}");
}
}

/// <summary>
/// Fetches roles for current party.
/// </summary>
/// <returns>List of roles for the current user and party.</returns>
[Authorize]
[HttpGet("{org}/{app}/api/authorization/roles")]
public async Task<IActionResult> GetRolesForCurrentParty()
{
(Party? currentParty, UserContext userContext) = await GetCurrentPartyAsync(HttpContext);

if (currentParty == null)
{
return BadRequest("Both userId and partyId must be provided.");
}

int userId = userContext.UserId;
List<Role> roles = await _authorization.GetUserRolesAsync(userId, currentParty.PartyId);

return Ok(roles);
}

/// <summary>
/// Helper method to retrieve the current party and user context from the HTTP context.
/// </summary>
/// <param name="context">The current HttpContext.</param>
/// <returns>A tuple containing the current party and user context.</returns>
private async Task<(Party? party, UserContext userContext)> GetCurrentPartyAsync(HttpContext context)
{
UserContext userContext = await _userHelper.GetUserContext(context);
int userId = userContext.UserId;

// If selected party is different than party for user self need to verify
if (userContext.UserParty == null || userContext.PartyId != userContext.UserParty.PartyId)
{
bool? isValid = await _authorization.ValidateSelectedParty(userId, userContext.PartyId);
if (isValid != true)
{
// Not valid, fall back to userParty if available
if (userContext.UserParty != null)
{
userContext.Party = userContext.UserParty;
userContext.PartyId = userContext.UserParty.PartyId;
}
else
{
userContext.Party = null;
userContext.PartyId = 0;
}
}
}

// Sync cookie if needed
string? cookieValue = Request.Cookies[_settings.GetAltinnPartyCookieName];
if (!int.TryParse(cookieValue, out int partyIdFromCookie))
{
partyIdFromCookie = 0;
}

if (partyIdFromCookie != userContext.PartyId)
{
Response.Cookies.Append(
_settings.GetAltinnPartyCookieName,
userContext.PartyId.ToString(CultureInfo.InvariantCulture),
new CookieOptions { Domain = _settings.HostName }
);
}

return (userContext.Party, userContext);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using Altinn.Platform.Register.Models;
using Altinn.Platform.Storage.Interface.Models;
using AltinnCore.Authentication.Utils;
using Authorization.Platform.Authorization.Models;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
Expand Down Expand Up @@ -179,4 +180,52 @@
}
return MultiDecisionHelper.ValidatePdpMultiDecision(actionsResult, response.Response, user);
}

/// <summary>
/// Retrieves roles for a user on a specified party.
/// </summary>
/// <param name="userId">The user id.</param>
/// <param name="userPartyId">The user party id.</param>
/// <returns>A list of roles for the user on the specified party.</returns>
public async Task<List<Role>> GetUserRolesAsync(int userId, int userPartyId)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should return IEnumerable<> or IReadOnlyList<> instead, to express the intended usage

{
List<Role> roles = new();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually we wrap the body in activity (distributed trace spans). You can probably find example usages of Telemetry

string apiUrl = $"roles?coveredByUserId={userId}&offeredByPartyId={userPartyId}";
string token = JwtTokenUtil.GetTokenFromContext(_httpContextAccessor.HttpContext, _settings.RuntimeCookieName);

try
{
HttpResponseMessage response = await _client.GetAsync(token, apiUrl);
if (response.IsSuccessStatusCode)
{
string responseContent = await response.Content.ReadAsStringAsync();
var deserialized = JsonConvert.DeserializeObject<List<Role>>(responseContent);
if (deserialized is not null)
{
roles = deserialized;
}
}
else
{
_logger.LogError(
"Failed to retrieve roles for userId {UserId} and partyId {PartyId}. StatusCode: {StatusCode}",
userId,
userPartyId,
response.StatusCode
);
}
}
catch (Exception ex)
{
_logger.LogError(
ex,
"An error occurred while retrieving roles for userId {UserId} and partyId {PartyId}",
userId,
userPartyId
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about the error handling here. If the API fails for some reason we return an empty list, which might make it look like the user has no roles when in fact the problem was that we couldn't find out at all

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point.

}
Comment on lines +218 to +226

Check notice

Code scanning / CodeQL

Generic catch clause Note

Generic catch clause.

return roles;
}

}
9 changes: 9 additions & 0 deletions src/Altinn.App.Core/Internal/Auth/IAuthorizationClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using Altinn.App.Core.Models;
using Altinn.Platform.Register.Models;
using Altinn.Platform.Storage.Interface.Models;
using Authorization.Platform.Authorization.Models;

namespace Altinn.App.Core.Internal.Auth;

Expand Down Expand Up @@ -50,4 +51,12 @@ Task<bool> AuthorizeAction(
/// <param name="actions"></param>
/// <returns></returns>
Task<Dictionary<string, bool>> AuthorizeActions(Instance instance, ClaimsPrincipal user, List<string> actions);

/// <summary>
/// Retrieves roles for a user on a specified party.
/// </summary>
/// <param name="userId">The user id.</param>
/// <param name="userPartyId">The user party id.</param>
/// <returns>A list of roles for the user on the specified party.</returns>
Task<List<Role>> GetUserRolesAsync(int userId, int userPartyId);
}
14 changes: 14 additions & 0 deletions test/Altinn.App.Api.Tests/Mocks/AuthorizationMock.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
using Altinn.App.Core.Models;
using Altinn.Platform.Register.Models;
using Altinn.Platform.Storage.Interface.Models;
using Authorization.Platform.Authorization.Models;
using Microsoft.AspNetCore.Http.HttpResults;

namespace Altinn.App.Api.Tests.Mocks;

Expand Down Expand Up @@ -69,4 +71,16 @@ List<string> actions

return authorizedActions;
}

public async Task<List<Role>> GetUserRolesAsync(int userId, int userPartyId)
{
await Task.CompletedTask;
List<Role> roles = new List<Role>
{
new Role { Type = "altinn", Value = "bobet" },
new Role { Type = "altinn", Value = "bobes" },
};

return roles;
}
}
24 changes: 15 additions & 9 deletions test/Altinn.App.Api.Tests/OpenApi/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -1804,7 +1804,7 @@
"description": "OK"
},
"425": {
"description": "Too Early"
"description": "Client Error"
},
"500": {
"description": "Internal Server Error"
Expand Down Expand Up @@ -5259,7 +5259,8 @@
"actions": {
"type": "object",
"additionalProperties": {
"type": "boolean"
"type": "boolean",
"nullable": true
},
"nullable": true
},
Expand Down Expand Up @@ -5365,8 +5366,7 @@
"type": "boolean"
},
"allowInSubform": {
"type": "boolean",
"deprecated": true
"type": "boolean"
},
"shadowFields": {
"$ref": "#/components/schemas/ShadowFields"
Expand Down Expand Up @@ -5464,7 +5464,7 @@
"copyInstanceSettings": {
"$ref": "#/components/schemas/CopyInstanceSettings"
},
"storageAccountNumber": {
"storageContainerNumber": {
"type": "integer",
"format": "int32",
"nullable": true
Expand All @@ -5479,7 +5479,8 @@
"features": {
"type": "object",
"additionalProperties": {
"type": "boolean"
"type": "boolean",
"nullable": true
},
"nullable": true
},
Expand Down Expand Up @@ -6502,8 +6503,12 @@
"JsonNodeOptions": {
"type": "object",
"properties": {
"propertyNameCaseInsensitive": {
"type": "boolean"
"hasValue": {
"type": "boolean",
"readOnly": true
},
"value": {
"$ref": "#/components/schemas/JsonNodeOptions"
}
},
"additionalProperties": false
Expand Down Expand Up @@ -7344,7 +7349,8 @@
"items": {
"$ref": "#/components/schemas/ValidationIssueWithSource"
}
}
},
"nullable": true
},
"nullable": true
},
Expand Down
13 changes: 9 additions & 4 deletions test/Altinn.App.Api.Tests/OpenApi/swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1101,7 +1101,7 @@ paths:
'200':
description: OK
'425':
description: Too Early
description: Client Error
'500':
description: Internal Server Error
'401':
Expand Down Expand Up @@ -3215,6 +3215,7 @@ components:
type: object
additionalProperties:
type: boolean
nullable: true
nullable: true
userActions:
type: array
Expand Down Expand Up @@ -3290,7 +3291,6 @@ components:
type: boolean
allowInSubform:
type: boolean
deprecated: true
shadowFields:
$ref: '#/components/schemas/ShadowFields'
additionalProperties: false
Expand Down Expand Up @@ -3361,7 +3361,7 @@ components:
$ref: '#/components/schemas/MessageBoxConfig'
copyInstanceSettings:
$ref: '#/components/schemas/CopyInstanceSettings'
storageAccountNumber:
storageContainerNumber:
type: integer
format: int32
nullable: true
Expand All @@ -3374,6 +3374,7 @@ components:
type: object
additionalProperties:
type: boolean
nullable: true
nullable: true
logo:
$ref: '#/components/schemas/Logo'
Expand Down Expand Up @@ -4114,8 +4115,11 @@ components:
JsonNodeOptions:
type: object
properties:
propertyNameCaseInsensitive:
hasValue:
type: boolean
readOnly: true
value:
$ref: '#/components/schemas/JsonNodeOptions'
additionalProperties: false
JsonPatch:
type: object
Expand Down Expand Up @@ -4723,6 +4727,7 @@ components:
type: array
items:
$ref: '#/components/schemas/ValidationIssueWithSource'
nullable: true
nullable: true
clientActions:
type: array
Expand Down
Loading