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

Set redirect from authorization handler #1906

Closed justinhelgerson closed 5 years ago

justinhelgerson commented 5 years ago

I'm creating an AuthorizationHandler to handle authorization to one of my controllers. If the requirement isn't met, I'd like to redirect the user based on route data in the request. I can achieve this with some ugly code in the cookie OnRedirectToAccessDenied method, but I'd like to define it as part of my handler since it seems like a natural place to define this.

I tried putting something together with the code below, but no dice:

protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, siteAuthorizationRequirement requirement) {
    if (context.Resource is AuthorizationFilterContext mvcContext) {
        var actionFilterContext = (AuthorizationFilterContext)context.Resource;

        object name;
        if (actionFilterContext.RouteData.Values.TryGetValue("urlSlug", out name)) {
            // do a bunch of stuff... omitted for brevity

            if (hasAccess) {
                context.Succeed(requirement);
                return;
            } else {
                actionFilterContext.Result = new RedirectToActionResult("AccessDenied", "Foo", new { urlSlug = name });
                context.Fail();
                return;
            }
        }
    }

    return;
}

If I change the context.Fail() to Succeed, it works. However, that feels wrong so I'm wondering if I'm going about this the wrong way?

blowdart commented 5 years ago

Authorization policies are completely separate from UI concerns. If policy evaluation fails we flow up to the authentication handler which takes care of the response, be it a redirect (for web pages), a 401 or 403 for apis or redirects for things like ADFS/OIDC.

So this isn't the right place to do this, you'd set the forbidden action on the authN handler.

justinhelgerson commented 5 years ago

Thanks @blowdart. That separation makes sense.

Is the OnRedirectToAccessDenied event of the cookie authentication handler really the best spot for this then? It felt a little clunky with not having access to the IActionResult implementations.

blowdart commented 5 years ago

Yea, we've had that feedback, finding a better way to flow information out is on the backlog.

blowdart commented 5 years ago

Linking to https://github.com/aspnet/Security/issues/1359

justinhelgerson commented 5 years ago

Got it, thanks for the info. I'll stick with my string parsing event handler then. Hopefully this will bubble up to a release in 2019.