Receiving HTTP 401 when should be receiving 403

[chadly]

Logging a bug per @blowdart 's suggestion on the last comment here (also discussed here with @danludwig)

When using the Authorize attribute, MVC 6 is returning HTTP 401 when I believe it should be returning a 403. Here are repro steps:

//fake middleware to always authenticate a test user
public class TestAuthMiddleware
{
    readonly RequestDelegate next;

    public TestAuthMiddleware(RequestDelegate next)
    {
        this.next = next;
    }

    public async Task Invoke(HttpContext context)
    {
        IEnumerable<Claim> claims = new Claim[]
        {
            new Claim(ClaimTypes.Name, "homer.simpson"),
            new Claim(ClaimTypes.Email, "[email protected]"),
            new Claim("Permission", "eat donuts")
        };

        var identity = new ClaimsIdentity(claims, authenticationType: "testing", nameType: ClaimTypes.Name, roleType: "Permission");
        var principal = new ClaimsPrincipal(identity);

        context.User = principal;

        await next(context);
    }
}

//controller which requires authorization for the permission "eat donuts"
public class TestController : Controller
{
    [HttpGet]
    [Route("")]
    [Authorize(Roles = "eat donuts")]
    public object Index()
    {
        return new { hello = "world", username = User.Identity.Name };
    }
}

When hitting the root endpoint, it will correctly return:

{
  "hello": "world",
  "username": "homer.simpson"
}

If you comment out the line context.User = principal in the middleware, hitting the controller will correctly return a 401 (since no user is authenticated). If, however, you comment out the line new Claim("Permission", "eat donuts") in the middleware, it will still return a 401 when it should be returning a 403 (since the user is authenticated, he just doesn't have permission).

Using Microsoft.AspNet.Mvc v6.0.0-rc1-final

[HaoK]

@blowdart Remember the 401/403 behavior comes entirely from our implementations of our auth middleware (and using our base AuthenticationHandlers) This behavior is by design, in that there's nothing that would turn 401 into 403s given this setup...

[blowdart]

Ah, there you go @chadly - you need to either use the base class, or implement it yourself.

[chadly]

What base class? Is there a better way to implement it other than what I did here?

Also, why not make this the default behavior? Returning a 403 in this scenario seems more correct.

[danludwig]

Returning a 403 in this scenario seems more correct.

Returning a 403 is the correct thing to do when a user is authenticated but unauthorized. Both Authorize attributes got this wrong in previous versions of MVC and WebAPI, making tons of us have to override it in a custom attribute.

I still don't quite get what the proposed solution should be. Where did you move our cheese to? Why can't it just work right this time?

[blowdart]

@chadly There's an AuthenticationMiddlware base class, and an AuthenticationHandler base class.

You'll note they have rather specific events, HandleForbiddenAsync(ChallengeContext context) and HandleUnauthorizedAsync(ChallengeContext context)

As for why it's not the default, well, the behaviour is dependent on protocol. Authentication is not limited to HTTP as a protocol. Cookies, for example, redirect. Federation, depending on setup, might return to the IDP.

[blowdart]

@danludwig The provided middleware, cookie auth, JWT do the behaviour you expect, because that's what those protocols mandate. This would also be the default for any auth middleware deriving from the AuthenticationMiddleware base class.

However it's up to the middleware developer to return whatever the status code, redirect, or other behaviour is suitable for their protocol.

[chadly]

Ah, ok. That makes sense. From looking at the code, it looks like JwtBearerMiddleware will have the behavior I want.

Just out of curiosity, in my example above, what auth middleware would it have been using? If I don't explicitly register any specific authentication scheme, what is it using to return to me the 401?

e.g. in Startup.Configure, I only call app.UseMvc() and nothing else.

[blowdart]

I believe that's MVC at that point.

For test users what I usually end up doing is using cookie auth and dropping the cookie - https://github.com/blowdart/AspNetAuthorizationWorkshop/blob/master/src/Step%202%20-%20Authorize%20All%20The%20Things/Controllers/AccountController.cs

[Tratcher]

Wait, something in MVC is still setting a 401? I thought we got rid of all of that because we wanted the auth middleware to do it?

[chadly]

It appears so. In my example above, the only thing I had in my Startup.Configure was app.UseMvc().

[Tratcher]

Is this it? https://github.com/aspnet/Mvc/blob/fd3ee499872465ccc2c5534b58f8ad113a8c0190/src/Microsoft.AspNetCore.Mvc.Core/Filters/AuthorizationFilterAttribute.cs#L50
@blowdart should't this return a ChallengeResult rather than an UnauthorizedResult?

[HaoK]

That certainly is what's causing the 401

[blowdart]

It's sweet you think I'd know :) @HaoK do we own that or MVC? i.e. can you change it?

[HaoK]

Yeah this code used to return ChallengeResult when last I touched it, we should open a bug for MVC to change this back (its very counter intuitive so I can see why they switched it when they were fiddling with things)

[blowdart]

@Tratcher do you want open the bug?

[Tratcher]

aspnet/Mvc#4233