aspnet / Security

[Archived] Middleware for security and authorization of web apps. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
1.27k stars 600 forks source link

Authentication middleware prevents UseStatusCodePages middleware from handling Access Denied responses #1869

Closed hempels closed 5 years ago

hempels commented 5 years ago

I must be missing something because everything I've read seems to indicate that I can use app.UseStatusCodePages(..) in my MVC app to handle 401 responses. However, the application cookie middleware is handling it first and forcing a page redirect which loses the original request context,

I have tried overriding OnRedirectToAccessDenied in ConfigureApplicationCookie to set the Response.StatusCode to 401 and return, but that just causes a 401 response, the StatusCodePagesMiddleware never executes.

Note that this issue relates to an already authenticated user who is trying to access a page they are restricted from via a role-based Authorize attribute. It would seem that authentication need not even be involved at that level as the user has already passed authentication and is moving on to request authorization, no?

I am using: services.AddAuthentication() .AddCookie(CookieAuthenticationDefaults.AuthenticationScheme);

app.UseStatusCodePages(); app.UseAuthentication(); app.UseMvc();

Tratcher commented 5 years ago

CookieAuth does not intercept 401 responses, it's invoked directly by calls to Challenge or Forbid. When a user that is authenticated but not authorized the authorization system calls Forbid. That invokes cookie auth which redirects the user to the AccessDenied page.

What context do you need from the original request that you are trying to display?

hempels commented 5 years ago

Thanks Chris .. While I guess I understand what you're saying, I don't see the practical difference. The net effect is that the request is redirected which is not the behavior I want.

Ultimately I need to determine why authorization failed and display custom pages for different scenarios plus a default. The best I'd come up with is using some custom status code middleware which would pass the original action context (or something) - I'm still thrashing pretty hard trying to figure out how to accomplish what seems like it should be a simple task.

Specifically in our case, if the Teacher role is required for an action but the authenticated user is a Student, we don't want to say "access denied" - we want to say "this page is only available to teachers". Otherwise when they land on an access denied page, they think the app is broken and start hitting refresh, etc.

Tratcher commented 5 years ago

@HaoK you've got a couple of issues tracking detailed AuthZ failure responses?

Tratcher commented 5 years ago

You can also go the preventative route, hiding links to resources you know the user does not have access to.

hempels commented 5 years ago

Indeed - we don't show users links to resources they don't have access to. However, that's not quite how the wild wild internet works. Teachers copy and paste links for their students, people share computers, we create inbound links in 3rd party apps we don't control, etc. It simply isn't possible to prevent all users from hitting a resource they aren't meant to see. What we must be able to do is control the experience within our app when that happens.

hempels commented 5 years ago

It's not pretty, at all, but I managed to force the behavior I want. Something like this:

services.ConfigureApplicationCookie(options => {
    options.Events = new CookieAuthenticationEvents {
        OnRedirectToAccessDenied = async ctx => {
            if (ctx.HttpContext.User.Identity.IsAuthenticated && !ctx.HttpContext.User.IsInRole("Teacher") && ctx.Request.Path.HasValue) {
                // evaluate paths which are teacher-only
                ctx.Response.StatusCode = 401;
                ctx.Response.ContentType = "text/html";
                var service = (IViewRenderService)ctx.HttpContext.RequestServices.GetService(typeof(IViewRenderService));
                await ctx.Response.WriteAsync(await service.RenderToStringAsync("Errors/TeachersOnly", null), Encoding.ASCII);
                return;
            }
            ctx.Response.Redirect(ctx.RedirectUri);
        }
    };
}

Would be really nice if there was a way to let UseStatusCodePages handle access denied responses instead of doing it directly in the auth middleware, but this seems the way for now.

Eilon commented 5 years ago

We do plan to revisit this in the 3.0 release as part of https://github.com/aspnet/Security/issues/1359 (see under Make it easier to be able to display authZ failure reasons in an error page etc #1530).