Skip to content

Add mypermissions endpoint#5538

Open
WilliamBZA wants to merge 5 commits into
authfrom
add-mypermissions-endpoint
Open

Add mypermissions endpoint#5538
WilliamBZA wants to merge 5 commits into
authfrom
add-mypermissions-endpoint

Conversation

@WilliamBZA

Copy link
Copy Markdown
Member

Adds 2 HTTP endpoints:

  1. One that returns all of the user's policy names that they have
  2. One that returns a summary of the permissions they have

… to readonly fields for improved clarity and consistency within the method.

[ApiController]
[Route("api")]
[Authorize]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Will this work when auth is disabled? Will we assume that SP will not invoke this API when auth is disabled?

Instead we would use AllowAnonymous and have fallback logic.

{
[HttpGet]
[Route("my/permissions/all")]
public ActionResult<PermissionsDescriptor> GetMyPermissions()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe use all as that is part of the route?

Suggested change
public ActionResult<PermissionsDescriptor> GetMyPermissions()
public ActionResult<PermissionsDescriptor> GetAllPermissions()

public ActionResult<PermissionsDescriptor> GetMyPermissions()
{
var descriptor = new PermissionsDescriptor(
User.FindFirst("sub")?.Value ?? User.Identity?.Name ?? "unknown",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What are we returning here? a user Id or display name? Can't we assume sub to always exist and not be null when a user is authenticated?

Suggested change
User.FindFirst("sub")?.Value ?? User.Identity?.Name ?? "unknown",
User.FindFirst("sub"),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@WilliamBZA in:

I added configurable settings for these keys

{
var descriptor = new PermissionsDescriptor(
User.FindFirst("sub")?.Value ?? User.Identity?.Name ?? "unknown",
GrantedPermissions().OrderBy(p => p, StringComparer.Ordinal).ToList());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we really need to order server side? Should we always order client side anyway?

Suggested change
GrantedPermissions().OrderBy(p => p, StringComparer.Ordinal).ToList());
GrantedPermissions()
).ToList());

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants