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

Overwritten Authorization roles displayed #144

Open
govza opened this issue Mar 2, 2020 · 4 comments
Open

Overwritten Authorization roles displayed #144

govza opened this issue Mar 2, 2020 · 4 comments

Comments

@govza
Copy link

govza commented Mar 2, 2020

I'm trying to show Authorized roles information in my swagger UI.

Using this kind of structure in my Controller

[Authorize(Roles = "Admin,Employee")] // admin or employee
public class XController : Controller 
{
    [Authorize(Roles = "Admin")] // only admin
    public ActionResult ActionX() { ... }

    public ActionResult ActionX() { ... } // admin, employee
}

results to (Auth roles: Admin,Employee, Admin) for first method.
So if my method controller Authorize is overwritten it's not respected, is it possible to fix that without overwriting the logic in controllers. It's really easy to have default one for all the controller and precise if needed in methods.

@mattfrear
Copy link
Owner

I didn't realise that the Authorize attributes worked that way.
Are you sure that ActionX in your example above is only Admin, and not also Employee?

@govza
Copy link
Author

govza commented Mar 9, 2020

Yes it looks like it uses strategy as less restrictive rights applied on class level, and more restrictive to the method.
Also found out that it's not possible to add method level Authorize attribute with Role which is not applied in class-level.

[Authorize(Roles = "Admin,Employee")] // admin or employee
public class XController : Controller 
{
    [Authorize(Roles = "Admin")] // only admin
    public ActionResult ActionX() { ... }

    public ActionResult ActionY() { ... } // admin, employee
    
    [Authorize(Roles = "Admin,Patient")]
    public ActionResult ActionZ() { ... } // admin but not patient
}

@martyt
Copy link

martyt commented Jun 21, 2022

A follow up on this issue, since I'm trying to use the Authorize stuff in my project.

There's an implied "hierarchy" in the way the [Authorize(Roles="foo")] attribute is used, and I don't think this extension properly handles it.

When multiple roles are specified in a single [Authorize] attribute, those are logical OR values (you need one role or the other).

If multiple [Authorize] attributes are specified on the same method (or controller), they are a logical AND - you have to have both roles.

If an [Authorize] attribute is specified at the controller level with multiple roles, but a method in the controller has an [Authorize] attribute with only one of those roles, only the role specified at the method level is required. This part is definitely not handled correctly by your extension.

See https://docs.microsoft.com/en-us/aspnet/core/security/authorization/roles?view=aspnetcore-6.0#adding-role-checks for further examples and clarification.

@mattfrear
Copy link
Owner

OK, cool. I haven't used this filter in years, so you're welcome to fix it and submit a PR with tests.

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

No branches or pull requests

3 participants