Enforcing ASP.NET MVC Conventions With Unit Tests, Or Solving The AJAX HTML Redirect To Login Page Problem

This blog post is about an ASP.NET MVC workaround we implemented in a previous project. We solved the problem by enforcing using a class that extends one of ASP.NET MVC classes, which in itself created another problem, as new developers joining the project may always use the old class. The solution to this problem was not something that I invented, but it’s also not a very common practice.

So, if you are interested, here’s the entire story…

Detecting Session Timeout In AJAX Requests

We wanted to solve a problem where in an AJAX heavy ASP.NET MVC application, if the user triggers an AJAX action after staying inactive for longer than our application timeout, the call to the controller action, which normally gets a JSON response, would instead get the HTML of the login page.

This is a known issue in ASP.NET (particularly System.Web). A feature that’s on by default is returning a redirect to the login page instead of a HTTP Unauthorized Status code (401). After the redirect, the response returned is a successful (HTTP Status Code 200) load of the login page. That means even our Angular.JS error interceptors (or jQuery handlers, etc.) don’t notice there was an error.

A fix for this was turning this feature off. We inherited the Authorize attribute as below:

/// <summary>
/// Suppresses Forms Authentication Redirects for Ajax requests
///     so the client side interceptors can handle it.
/// </summary>
[AttributeUsage(AttributeTargets.Class | AttributeTargets.Method,
    Inherited = true, AllowMultiple = true)]
public class AuthorizeRedirectAttribute : AuthorizeAttribute
{
    protected override void HandleUnauthorizedRequest(AuthorizationContext filterContext)
    {
        var context = filterContext.HttpContext;
        if (context.Request.IsAjaxRequest())
        {
            context.Response.SuppressFormsAuthenticationRedirect = true;
        }

        base.HandleUnauthorizedRequest(filterContext);
    }
}

SuppressFormsAuthenticationRedirect is the property that disables the login redirect. Microsoft set it to false by default so that it’s backwards compatible.

ASP.NET MVC doesn’t recognize AJAX requests through Request.IsAjaxRequest() via Accept header or so. It does via checking X-Requested-With header. Most AJAX-capable frameworks like jQuery and others offer a way to intercept all requests and add extra headers, for example, in that app, we configure Angular.JS to include the header with some code similar to this:

app.config(['$httpProvider', function ($httpProvider) {
   $httpProvider.defaults.headers.common["X-Requested-With"] = "XMLHttpRequest";
 }]);

Enforcing The Convention

The obvious problem with the previous solution is that we are ignoring The Power Of Defaults. Any other developer who may join the project needs to know that using Authorize is a no-no, and even for old devs (myself included), it’s very easy to just forget and use Authorize not AuthorizeRedirect just out of habit.

Solving this problem was quite easy though, we added the following test to our Unit Tests project:

[TestClass]
 public class AuthorizeRedirectTests
 {
     [TestMethod]
     public void All_Controllers_And_Actions_Do_Not_Inherit_Authorize_Directly()
     {
         var controllerType = typeof(IController);
         var invalidAuthorizeFilter = typeof(AuthorizeAttribute);
         var correctAuthorizeFilter = typeof (AuthorizeRedirectAttribute);

         var webProjectAssembly = correctAuthorizeFilter.Assembly;
         var controllers = webProjectAssembly
             .GetTypes()
             .Where(controllerType.IsAssignableFrom);

         Func<memberinfo, ienumerable="">>
             invalidAttributes = member =>
                 member
                     .GetCustomAttributes(invalidAuthorizeFilter)
                     // Note that AuthorizeRedirectAttribute inherits from AuthorizeAttribute
                     .SkipWhile(correctAuthorizeFilter.IsInstanceOfType);

         foreach (var controller in controllers)
         {
             Assert.IsTrue(invalidAttributes(controller).IsNullOrEmpty(),
                 "Controller {0} should not use {1} directly, " +
                 "use {2} instead",
                 controller.Name, 
                 invalidAuthorizeFilter.Name, correctAuthorizeFilter.Name);

             var actions = controller
                 // Simple check, might get some false positives but they won't hurt
                 .GetMethods(BindingFlags.Instance | BindingFlags.Public);

             foreach (var action in actions)
             {
                 Assert.IsTrue(invalidAttributes(action).IsNullOrEmpty(),
                     "Controller Action {0}.{1} should not use {2} directly, " +
                     "use {3} instead",
                     controller.Name, action.Name,
                     invalidAuthorizeFilter.Name, correctAuthorizeFilter.Name);
             }
         }
     }
 }

I hope the code is self explanatory. We check the web project assembly for all ASP.NET MVC Controllers, then we check the Controllers and all their Action methods for existence of the AuthorizeAttribute. We filter those that use the correct attribute (AuthorizeRedirectAttribute), and we Assert that there are no Controllers or action remaining, otherwise, we tell the developer which Controller or Action needs to be fixed, and how to fix it as well.

Room For Improvement

The drawback of this is that our Unit Test project had to reference the ASP.NET MVC assemblies and gets more stuff than most tests should need. We can overcome this by moving our “convention” tests into another project completely, but for this project, the conventions were very few and it seemed fine.

Of course, the same method can be applied to any other convention you enforce in your project. One obvious example is ensuring all Controllers inherit from a custom base Controller class instead of the ASP.NET MVC class directly. I know people who already do this, as I mentioned in the beginning, the technique is not new by any means, but it’s worth even more popularity.

Speaking of improvement, the code for this test class was optimized a bit while writing this blog post, there is always room for improvement. :)

IsNullOrEmpty()

In case you were reading the code carefully, the IsNullOrEmpty() method I used in assertions is a custom extension method we had in the project, a very simple one as you may expect:

public static bool IsNullOrEmpty<t>(this IEnumerable<t> source)
 {
     return source == null || !source.Any();
 }

And That’s It!

I hope you found the technique useful if you haven’t used it before, or found the post a good place to reference it to those who didn’t.