Consider using AuthorizeAttribute instead of IAuthorizationService.AuthorizeAsync

[kevinchalet]

Many OrchardCore modules (including the OpenID module) use IAuthorizationService.AuthorizeAsync to determine whether a user is allowed to perform a given action:

if (!await _authorizationService.AuthorizeAsync(User, Permissions.ManageOpenIdApplications))
{
    return Unauthorized();
}

These (ugly) calls could be replaced by attribute-oriented programming, which would make the code cleaner and would help avoid code duplication when applied at the controller level:

[Authorize(Permissions.ManageOpenIdApplicationsPolicyName)]
public class AdminController : Controller { ... }

(note: authorization policies could be created dynamically by implementing a custom IAuthorizationPolicyProvider).

/cc @sebastienros @jersiovic

[sebastienros]

It's not AOP per se, but we surely can use an attribute in most cases. It also prevents from resolving the service if there is no need for it.

To be clear we have had the service based one since O1 as it allows to be called with some state (resource in netcore) and from Views or other services.

[kevinchalet]

It's not AOP per se, but we surely can use an attribute in most cases.

I guess we don't have the same definition, then :sweat_smile:

To be clear we have had the service based one since O1 as it allows to be called with some state

Yeah sure. The code paths using the resource-based AuthorizeAsync() overload shouldn't be updated (they are really in minority, AFAICT)

[sebastienros]

Alternative facts!

[sebastienros]

Not sure you used the correct AOP acronym.

https://en.wikipedia.org/wiki/Aspect-oriented_programming

[kevinchalet]

I was tempted to use [@]OP, but it would have sent a notification to https://github.com/OP :sweat_smile:

[timlunev]

@sebastienros @kevinchalet Is this task still relevant?

[sebastienros]

Meh, I am not a fan, but some people have expressed they like it. I wouldn't mind if it's available, but I would not encourage to use it by default in the codebase, as it constrains where it can be used in the action, the order of the calls, and doesn't support permission resources.

[timlunev]

I think this makes sense only if everyone will use it as a development guideline in the project. Then let's leave it as is.