AuthorizeFilter prevents unauthenticated requests to be handled by IAuthorizationService

[malekpour]

Current implementation of AuthorizeFilter only calls IAuthorizationService.AuthorizeAsync when request is authenticated.

https://github.com/aspnet/Mvc/blob/dev/src/Microsoft.AspNetCore.Mvc.Core/Internal/AuthorizeFilter.cs#L78

I think DenyAnonymousAuthorizationRequirement should be used instead, otherwise there is no way to use authorization requirements without being authenticated.

[Eilon]

@blowdart @Tratcher thoughts?

[Tratcher]

@HaoK?

[blowdart]

By design. Why go into policies when you don't have an identity to evaluate anyway?

[kevinchalet]

By design.

Bad design then. The authorization service is already responsible of determining whether unauthenticated requests (= no principal/identity) are acceptable or not depending on the specific policy that applies, so there's no reason to have a duplicated logic in MVC.

What's the whole point of DenyAnonymousAuthorizationRequirement and policy.RequiredAuthenticatedUser() if the authorization service is not invoked when the user is not authenticated?

[malekpour]

For example you can have a requirement to grant or deny access by IP address only or you may grant guest users access in restricted hours.

Conceptually, in real world, there are situations that we are authorized to do things without being identified.

[blowdart]

For your IP example you're now attempt to authorize on something that isn't an identity at all. So you either write an authentication middleware which builds an identity from an IP address, or you go the filter route.

[malekpour]

This authentication vs. authorization has always been controversial. I've seen systems using IP, MAC address or computer name to identify and authorize group of users (e.g. Kiosk devices).

Considering the modular nature of AspNetCore, it does not make sense if MVC handles security, since the security project can take care of it very well.

In fact unauthenticated users have their own identity which is Anonymous. Lets have security project take care of them. The following code block functionality in the AuthorizeFilter can be replaced by a new requirement like AllowAnonymousAuthorizationRequirement

// Allow Anonymous skips all authorization
if (context.Filters.Any(item => item is IAllowAnonymousFilter))
{
    return;
}

[kevinchalet]

What's the whole point of DenyAnonymousAuthorizationRequirement and policy.RequiredAuthenticatedUser() if the authorization service is not invoked when the user is not authenticated?

Still no answer to this crucial question, it's a bit worrying IMHO.

[HaoK]

[Authorize] with nothing else specified uses the default policy of requiring any authenticated user. So this requirement and method exists mostly for that scenario

[kevinchalet]

But isn't the authorization filter always rejecting unauthenticated requests without invoking the authorization service no matter if you specify a policy?

https://github.com/aspnet/Mvc/blob/dev/src/Microsoft.AspNetCore.Mvc.Core/Authorization/AuthorizeFilter.cs#L77-L79

[HaoK]

Yeah the authorize filter probably shouldn't do that explicit check and leave that to the Authorization service. This seems like a legit bug to fix.