mattfrear / Swashbuckle.AspNetCore.Filters

A bunch of useful filters for Swashbuckle.AspNetCore
MIT License
426 stars 80 forks source link

Overwritten Authorization roles displayed #144

Open govza opened 4 years ago

govza commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 2 years ago

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 commented 2 years ago

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