ASP.NET MVC - Optionally Redirect from a Base Class Method?

Question

Consider the following block of code that reappears in many of my controller actions. (I'm chiefly concerned with the first 6 lines of the method body).

[HttpGet]
public ActionResult OptOut()
{
    var user = this.SecurityPrincipal;
    if (user.IsReadOnlyUser)
    {
        this.TempData["ViewModel"] = new AuthorizationModel { User = user };
        return this.RedirectToAction("NotAuthorized", "Authorization");
    }

    var model = /* Elided for brevity */

    return this.View(model);
}

My controllers derive from a base class, SecuredController which, in turn, derives from Controller. SecurityPrincipal is a property of SecuredController, and contains extensive Active Directory data about the current user.

In an effort to eliminate duplicate code, I'd ideally like to move the functionality contained in the if {...} block into a base class method, but I can't think of any way to do so, since the return type of the method would have to be ActionResult, resulting in something ungainly like this:

if ((var result = this.RequireReadWrite()) != null)
{
    return result;
}

Can anyone suggest a way to do this, or am I simply out of luck here?

What about using [MVC Action Filters](http://www.asp.net/mvc/overview/older-versions-1/controllers-and-routing/understanding-action-filters-cs)? Have you considered that in your design? You'd have more control with the `filterContext` on what the `ActionResult` should be. — QuantumHive, Jul 15 '16 at 15:04
In general, if you're doing security in your methods, you're doing it wrong. Security is a cross cutting concern, and should be handled way before you even get to executing the method. As mentioned, filters are a better way to handle this, and something that MVC has an entire framework for. You're trying to roll your own security, and that's almost always an anti-pattern. Essentially, you have an expensive and well tested security system on your house, and you're leaving the door unlocked and asking a bored teenager to make sure the house is safe. — Erik Funkenbusch, Jul 15 '16 at 15:16
@QuantumHive That did the trick. Can't believe I missed that. :( I need an answer so I can upvote and accept. Then I will summarily present myself for a right fonging. — Mike Hofer, Jul 15 '16 at 15:30

score 2 · Accepted Answer · answered Jul 15 '16 at 15:57

As mentioned in the comments, especially noting that security is a cross cutting concern we've suggested using MVC Action Filters to be applied in your use case and design.
Microsoft's documentation is pretty informative and there are more examples that can be found on the web on how to use MVC Filters. I'll try to provide an example, but this will be based on a lot of assumptions on your software architecture, since I simply don't have the knowledge of that.

You could create the following class:

public class SecuredFilterAttribute : AuthorizeAttribute
{
    ...
}

If using a Dependency Injection framework, you could inject the SecurityPrincipal service. But again I don't know the architecture of your application, so it's up to you how you create that dependency.
When overriding the AuthorizeCore, you could implement it like so:

protected override bool AuthorizeCore(HttpContextBase httpContext)
{
    return !this.SecurityPrinciple.IsReadOnlyUser;
}

And when not authorized override the HandleUnauthorizedRequest method to redirect:

protected override void HandleUnauthorizedRequest(AuthorizationContext filterContext)
{
    var redirectRoute = ...; //your route to redirect to an unauthorized page
    filterContext.Result = new RedirectToRouteResult(redirectRoute);
    //do some other things, for example, setting some tempdata information
}

Again it's up to you on how you would use this Filter. You could register it globally, or apply it on a per controller or action basis. To register it globally, in your startup:

GlobalFilters.Filters.Add(new SecuredFilterAttribute());

I'd highly recommend [SimpleInjector](https://simpleinjector.org/index.html) as your DI Container. — QuantumHive, Jul 15 '16 at 15:58
Thanks very much. This was almost exactly what I did, and it works perfectly. Since I need this on only a few action methods, I decided to decorate those methods with the attributes rather than registering a global handler. Again, thanks! — Mike Hofer, Jul 15 '16 at 16:07

ASP.NET MVC - Optionally Redirect from a Base Class Method?

1 Answers1