Consider dropping Active in AuthorizeAttribute.ActiveAuthenticationSchemes

[HaoK]

The active was removed everywhere else, but left in the attribute property. Would be nice to cleanup if possible.

[kevinchalet]

Ideally, you should consider removing the authentication part from AuthorizeAttribute and move that to a separate attribute, which would allow doing things like that:

[Authenticate("Bearer")]
public class ApiController : Controller
{
    public IActionResult ActionWithOptionalAuthentication() { ... }

    [Authorize]
    public IActionResult ActionWithRequiredAuthentication() { ... }

    [Authorize(Policy = "HasSuperPrivilege")]
    public IActionResult ActionWithRequiredAuthenticationAndSpecifiedPolicy() { ... }
}

[Authenticate("Cookie")]
public class WebController : Controller
{
    public IActionResult ActionWithOptionalAuthentication() { ... }

    [Authorize]
    public IActionResult ActionWithRequiredAuthentication() { ... }

    [Authorize(Policy = "HasSuperPrivilege")]
    public IActionResult ActionWithRequiredAuthenticationAndSpecifiedPolicy() { ... }
}

[kevinchalet]

/cc @sebastienros you might want to chime in as the outcome of this discussion will impact how things will work in OrchardCore when moving to 2.0.

[HaoK]

That doesn't sound unreasonable to me, @rynowak @blowdart @Tratcher thoughts?

[HaoK]

Would we keep the existing semantic today of [Authorize] => [Authenticate] implied?

[blowdart]

And what would happen if you put Authorize against a method without Authenticate anywhere near?

[kevinchalet]

And what would happen if you put Authorize against a method without Authenticate anywhere near?

It would use the default authentication scheme handler, as specified in the authentication options. If no scheme is specified, an exception would be thrown, exactly like what already happens in the 1.0 bits.

[blowdart]

That's not what it looks like.

Consider the average dev.

I want authentication, so I'm going to put authenticate on my controller. What happens now when there's no authorize attribute? Do we assume one on every method? This looks confusing.

[kevinchalet]

Would we keep the existing semantic today of [Authorize] => [Authenticate] implied?

Yeah, the default authorization policy should still use RequireAuthenticatedUser().

[kevinchalet]

I want authentication, so I'm going to put authenticate on my controller.

Authentication = determining who's the caller.
Authorization = determining whether the caller is allowed to perform a given action (which may or may not imply requiring authentication).

This design is exactly the same as the one that Web API + OWIN used to have (with HostAuthenticationAttribute), and I don't think people have ever found that confusing.

[HaoK]

I'd imagine [Authenticate] would be somewhat akin to DefaultAuthenticateScheme, where it will control what's on httpContext.User for the request. It'd affect how Authorize works since it controls what's the user being authorized against, but that's it.

[kevinchalet]

It'd affect how Authorize works since it controls what's the user being authorized against, but that's it.

Yep, that's the idea: decoupling the authorization policy evaluation from the authentication schemes.

[blowdart]

I'd put to you that the average developer doesn't know, or care about the differences here.

[kevinchalet]

The average developer is not as stupid as you think. And if you really cared about that, you'd have named AuthorizeAttribute RequireAuthenticationAttribute, which is a more descriptive name (at least for the default authorization policy)

Note: as I said, the design I suggest was already present in Web API/OWIN and didn't cause any confusion.