Analytics are an aspect

On a recent project Jesse Kallhoff, a fellow nerd from The Nerdery, and I were tasked with integrating an existing analytics solution with our extensions to the client’s ASP.NET MVC web site. As it’s a commerce site, the customer’s tracking requirements are extensive.  Each page may have unique tracking requirements depending on a variety of conditions, both based on logged in state and query parameters.  Moreover, for browse and search pages, there is a tight coupling between the backend search technology and the query parameters supplied as part of the request we are tracking.

Much of the existing code incorporated a series of helpers within each action method, cluttering up the action method with code that wasn’t directly related to the view that was being rendered, but rather was related to the tracking parameters that were going to used by a script on the page to post back to the tracking solution provider.

I was initially tasked with the goal of investigating how we would incorporate analytics into the new pages that we were introducing. I wasn’t familiar with the product the customer had chosen so I dove into the existing code to see how it worked.  A familiar pattern appeared.

1. Load up some default parameters into the analytics object based on the action being invoked.
2. Set/change some parameters based on the search/browse/product context.
3. Set some additional parameters based on the users logged in state.
4. Dump some scripts on the page, including one script section that defined the parameter and their values that were to be posted back to the tracking provider’s web site via a script request.

Each action method in the solution repeated this pattern in one form or another, though complicated by some AJAX requests to local actions to retrieve the analytics values stored in local session state once the page was rendered.

I scoped out an an initial solution that would remove the analytics code out of the actions themselves and use action filters to provide, as much as possible, the values required.  This was relatively easy to do for items (1) and (3).  The filter defined a key that was used to access configuration data to find the default parameters for that action.

NOTE**: the code below is an approximation using the ideas we came up with.  Unfortunately I can’t share the actual code with you.

[Analytics(Key="Browse")]
public ActionResult Browse( string browseContext, ... )
{
...
}

In the OnActionExecuting method of the filter we use the key to extract the default values associated with the key.  We can also check the logged in state and set those parameters as well.

public void OnActionExecuting( ActionExecutingContext filterContext )
{
    if (!filterContext.Canceled && !filterContext.IsChildAction)
    {
        if (filterContext.Result is ViewResultBase) // we only care if we're delivering HTML
        {
            // retreive the base analytics object from the configuration
            var analytics = _analyticsConfiguration.Get(this.Key);
            if (analytics != null) // if we're doing analytics for this page
            {
                ... fill in user-related parameters from the current session/user

HttpContext.Items[“Analytics”] = analytics;
            }
        }
    }
}

The difficulty came with (2).  It seemed like we would still need to include some code in each action that would be able fill in the request specific context information in the analytics object.  To support this, we put the partially filled object into HttpContext.Items where the action could retrieve it as needed to add the parameter-related data.

To address (4), we used the OnActionExecuted method to put the analytics object in the ViewBag so that our master page could render the scripts on the page without using any further AJAX calls.

public void OnActionExecuted(ActionExecutedContext filterContext)
{
    if (!filterContext.Canceled && !filterContext.IsChildAction)
    {
        var result = filterContext.Result as ViewResultBase;
        if (result != null)
        {
            result.ViewBag.Analytics = HttpContext.Items["Analytics"];
        }
    }
}

And in the master view:

@if (ViewBag.Analytics != null)
{
    @Html.AnalyticsAPIScript()
    @Html.AnalyticsDataScript(ViewBag.Analytics)
    ... invoke analytics api ...
}

This is basically the architecture I had in place on my initial pass. At that point I wasn't familiar enough with the project to know how to address the browse/search/product details.  Before I could develop it further other aspects of the project rose in priority and I moved on to something else. Neither Jesse or I was particularly happy that we still had some analytics code that would be needed in each action, but it was difficult to see how we could avoid without duplicating code from the attribute in the action.

A few weeks later, Jesse took on the completion of the analytics task.  This is when one of those cool moments that I’ve come to regularly expect at The Nerdery happened. We were on site at the customer’s office and one evening I get a call from Jesse – he wants to brainstorm on how we were going to address a couple of different aspects of the project, including this one.  We spent a couple of hours working through the issues, bouncing ideas off each other back and forth. It was a pretty intense and enjoyable couple of hours, the kind of collaboration that I’ve come to expect from my awesome colleagues.

What we eventually hit on was using attribute inheritance in correspondence with our view model so that we would basically have a different analytics attribute for each base model type. This attribute would use the view model data available in OnActionExecuted to complete the population of the analytics data.

The convention is that you only apply the attribute to an action whose model is compatible with the model addressed by the attribute.  The attribute inherits the base behavior described above, but then uses its unique knowledge of the view model type to extract the rest of the data.  This allows us to put the code that would have been in the action into the attribute instead keeping our action code clean and localizing the analytics code within a few attributes.

Another approach, if the number of models you need to support is large, might use a factory within a common attribute to find and execute a model-specific strategy within OnActionExecuted. For us the number of models is low and inheritance was simpler and easier to comprehend.

There were a number of other problems specific to the customer’s domain that Jesse had to work through and our final solution differs in some respects from what I’ve described, but using attributes to attack analytics seems like a winning choice. Our code is cleaner and more focused, easier to understand, and much more extensible.

Revisiting custom authorization in ASP.NET MVC

Several months ago I blogged about custom attributes based on the AuthorizeAttribute class in ASP.NET MVC. One of the bits that I got wrong or, at least, not as right as I would like, is caching. I didn’t extend the caching to allow it to take into account the new tests for authorization that I’d developed. The end result of this was that if you derived your permission to access the method based on the new test, you always got served a fresh page. It was never delivered from the cache.

In my application, this didn’t cause too much of a performance it. In fact, most of the actions protected in such a way are explicitly not cached anyway so the drawback rarely (if ever, I haven’t checked all the methods) came into play. I got asked about it recently on a StackOverflow answer so I thought I’d take a stab at improving the cache handling to account for the new tests.

There are a couple of things you need to be aware of. First, the code in OnCacheAuthorization must be thread-safe. Instances of the attribute are reused for multiple requests. In particular, this means that you cannot store information from the request in the attribute and expect it to remain inviolate during the life of the attribute. Another request may come along later and overwrite your stored request data and cause your code to return incorrect results. Since we’re talking about authorization this is a serious problem since it could lead to users getting access to information they should not be able to see.

Second, you need to be careful not to reuse the request data associated with the request that creates the cached entry. While there is a mechanism that allows you to pass data to the CacheValidationHandler method I created in the first post, you need to be careful to only pass data that actually applies to all requests, since you wouldn’t want to depend on the data being the same in the request that is served up from the cache. It may be an entirely different user making that request.

Our attribute still derives from AuthorizeAttribute, with the same modifications due to the fact that it is specific to a particular request and so doesn’t make sense to apply it at the class level or have multiples.

 AttributeUsage( AttributeTargets.Method, Inherited = true, AllowMultiple = false )]
 public class RoleOrOwnerAuthorizationAttribute : AuthorizationAttribute
 {
        private IDataContextFactory ContextFactory { get; set; }

        private string routeParameter = "id";
        /// <summary>
        /// The name of the routing parameter to use to identify the owner of the data (user id) in question.  Default is "id".
        /// </summary>
        public string RouteParameter
        {
            get { return this.routeParameter; }
            set { this.routeParameter = value; }
        }

        ...
}

The constructors, including reliance on my data context factory and its constructor injection, remain the same as well as the methods that I’ve refactored from the original AuthorizeAttribute. Notice, though, that I've made them virtual now in case we want to derive from this attribute.

protected IDataContextFactory ContextFactory { get; set; }

public RoleOrOwnerAuthorizationAttribute()
    : this( null )
{
}

protected virtual void CacheValidateHandler( HttpContext context, object data, ref HttpValidationStatus validationStatus )
{
    validationStatus = OnCacheAuthorization( new HttpContextWrapper( context ) );
}

 protected virtual void SetCachePolicy( AuthorizationContext filterContext )
{
    // ** IMPORTANT **
    // Since we're performing authorization at the action level, the authorization code runs
    // after the output caching module. In the worst case this could allow an authorized user
    // to cause the page to be cached, then an unauthorized user would later be served the
    // cached page. We work around this by telling proxies not to cache the sensitive page,
    // then we hook our custom authorization code into the caching mechanism so that we have
     // the final say on whether a page should be served from the cache.
    HttpCachePolicyBase cachePolicy = filterContext.HttpContext.Response.Cache;
    cachePolicy.SetProxyMaxAge( new TimeSpan( 0 ) );
    cachePolicy.AddValidationCallback( CacheValidateHandler, null /* data */);
}

Now I introduce a new, inner class CurrentRequest to help in abstracting the different between the authorization request that creates the cached copy and subsequent cache requests that need to be served from the cache, or not. It stores the OwnerID of the item being requested and the current Username of the user making the request. Our IsOwner method will be refactored to use this class instead of the AuthorizationContext for this data. Note that it has three constructors; the default constructor will be used for unit testing since I won’t always want to mock up either an AuthorizationContext or an HttpContextBase object in my tests.  The other constructors are used for requests made from OnAuthorization and OnCacheAuthorization, respectively.  Note the real difference is what context we have available to the code. In the first case we can use RouteValues to extract the OwnerID, in the second, we use the Request parameters directly.

public class CurrentRequest
{

    public CurrentRequest()
    {
        this.OwnerID = -1;
    }

    public CurrentRequest( AuthorizationContext filterContext, string routeParameter )
        : this()
    {
        if (filterContext.RouteData.Values.ContainsKey( routeParameter ))
        {
            this.OwnerID = Convert.ToInt32( filterContext.RouteData.Values[routeParameter] );
        }

        this.Username = filterContext.HttpContext.User.Identity.Name;
    }

    public CurrentRequest( HttpContextBase httpContext, string routeParameter )
    {
        if (!string.IsNullOrEmpty( routeParameter ))
        {
            if (httpContext.Request.Params[routeParameter] != null)
            {
                this.OwnerID = GetOwnerID( httpContext.Request.Params[routeParameter] );
            }
            else if (string.Equals( "id", routeParameter, StringComparison.OrdinalIgnoreCase ))
            {
                // id may be last element in path if not included as a parameter
                this.OwnerID = GetOwnerID( httpContext.Request.Path.Split( '/' ).Last() );
            }
        }

        this.Username = httpContext.User.Identity.Name;
    }

    private int GetOwnerID( string id )
    {
        int ownerID;
        if (!int.TryParse( id, out ownerID ))
        {
            ownerID = -1;
        }
        return ownerID;
    }

    public int OwnerID { get; set; }
    public string Username { get; set; }
}

NOTE: An earlier version of this post did not handle getting the route parameter from the HttpContext properly, if the route parameter was the "id" property.  This version relies on the default route set up to place the id parameter at the end of the path but, with that caveat, now handles an "id" parameter properly.

The OnAuthorization method changes slightly to use this new class. Instead of simply passing the AuthorizationContext, we pass a CurrentRequest object constructed from it using the RouteParameter on the attribute.

public override void OnAuthorization( AuthorizationContext filterContext )
{
    if (filterContext == null)
    {
        throw new ArgumentNullException( "filterContext" );
    }

    if (AuthorizeCore( filterContext.HttpContext ))
    {
        SetCachePolicy( filterContext );
    }
    else if (!filterContext.HttpContext.User.Identity.IsAuthenticated)
    {
        // auth failed, redirect to login page
        filterContext.Result = new HttpUnauthorizedResult();
    }
    else if (filterContext.HttpContext.User.IsInRole( "SuperUser" ) || IsOwner( new CurrentRequest( filterContext, this.RouteParameter ) ))
    {
        SetCachePolicy( filterContext );
    }
    else
    {
        ViewDataDictionary viewData = new ViewDataDictionary();
        viewData.Add( "Message", "You do not have sufficient privileges for this operation." );
        filterContext.Result = new ViewResult { MasterName = this.MasterName, ViewName = this.ViewName, ViewData = viewData };
    }
}

This allows us to simplify IsOwner significantly as the CurrentRequest class now takes care of unpacking the data from the request. Note how we are able to use the ContextFactory (it’s thread-safe) to create a new data context within the method, ensuring that we don’t conflict with other threads that may be using the attribute. The data context itself needn’t be thread-safe since we create a new one each time and it’s local to the method.

private bool IsOwner( CurrentRequest requestData )
{
    using (var dc = this.ContextFactory.GetDataContextWrapper())
    {
        return dc.Table<User>().Where( p => p.UserName == requestData.Username && p.UserID == requestData.OwnerID ).Any();
    }
}

Lastly, we need to hook our IsOwner check into the authorization system.  We do this by overriding OnCacheAuthorization and doing our check when the base class method would normally invalidate the request because the user is not in the correct role. When they are allowed by the role, we needn’t perform our check.

protected override HttpValidationStatus OnCacheAuthorization( HttpContextBase httpContext )
{
    var status = base.OnCacheAuthorization( httpContext );
    if (status == HttpValidationStatus.IgnoreThisRequest && IsOwner( new CurrentRequest( httpContext, this.RouteParameter ) ))
    {
        status = HttpValidationStatus.Valid;
    }
    return status;
}

Note here that we use the constructor on CurrentRequest that takes an HttpContextBase (wrapper around HttpContext) instead because that's all we have available to use when being evaluated by the caching system. Because we've abstracted IsOwner to use this object, it doesn't need to know how we got the values that it will be checking.

Now the owners of the data will have the same cache-based performance as the administrative users who access the data via their roles.