MVC Custom Authentication, Authorization, and Roles Implementation

Question

Bear with me as I provide details for the issue...

I've got an MVC site, using FormsAuthentication and custom service classes for Authentication, Authorization, Roles/Membership, etc.

Authentication

There are three ways to sign-on: (1) Email + Alias, (2) OpenID, and (3) Username + Password. All three get the user an auth cookie and start a session. The first two are used by visitors (session only) and the third for authors/admin with db accounts.

public class BaseFormsAuthenticationService : IAuthenticationService
{
    // Disperse auth cookie and store user session info.
    public virtual void SignIn(UserBase user, bool persistentCookie)
    {
        var vmUser = new UserSessionInfoViewModel { Email = user.Email, Name = user.Name, Url = user.Url, Gravatar = user.Gravatar };

        if(user.GetType() == typeof(User)) {
            // roles go into view model as string not enum, see Roles enum below.
            var rolesInt = ((User)user).Roles;
            var rolesEnum = (Roles)rolesInt;
            var rolesString = rolesEnum.ToString();
            var rolesStringList = rolesString.Split(',').Select(role => role.Trim()).ToList();
            vmUser.Roles = rolesStringList;
        }

        // i was serializing the user data and stuffing it in the auth cookie
        // but I'm simply going to use the Session[] items collection now, so 
        // just ignore this variable and its inclusion in the cookie below.
        var userData = "";

        var ticket = new FormsAuthenticationTicket(1, user.Email, DateTime.UtcNow, DateTime.UtcNow.AddMinutes(30), false, userData, FormsAuthentication.FormsCookiePath);
        var encryptedTicket = FormsAuthentication.Encrypt(ticket);
        var authCookie = new HttpCookie(FormsAuthentication.FormsCookieName, encryptedTicket) { HttpOnly = true };
        HttpContext.Current.Response.Cookies.Add(authCookie);
        HttpContext.Current.Session["user"] = vmUser;
    }
}

Roles

A simple flags enum for permissions:

[Flags]
public enum Roles
{
    Guest = 0,
    Editor = 1,
    Author = 2,
    Administrator = 4
}

Enum extension to help enumerate flag enums (wow!).

public static class EnumExtensions
{
    private static void IsEnumWithFlags<T>()
    {
        if (!typeof(T).IsEnum)
            throw new ArgumentException(string.Format("Type '{0}' is not an enum", typeof (T).FullName));
        if (!Attribute.IsDefined(typeof(T), typeof(FlagsAttribute)))
            throw new ArgumentException(string.Format("Type '{0}' doesn't have the 'Flags' attribute", typeof(T).FullName));
    }

    public static IEnumerable<T> GetFlags<T>(this T value) where T : struct
    {
        IsEnumWithFlags<T>();
        return from flag in Enum.GetValues(typeof(T)).Cast<T>() let lValue = Convert.ToInt64(value) let lFlag = Convert.ToInt64(flag) where (lValue & lFlag) != 0 select flag;
    }
}

Authorization

Service offers methods for checking an authenticated user's roles.

public class AuthorizationService : IAuthorizationService
{
    // Convert role strings into a Roles enum flags using the additive "|" (OR) operand.
    public Roles AggregateRoles(IEnumerable<string> roles)
    {
        return roles.Aggregate(Roles.Guest, (current, role) => current | (Roles)Enum.Parse(typeof(Roles), role));
    }

    // Checks if a user's roles contains Administrator role.
    public bool IsAdministrator(Roles userRoles)
    {
        return userRoles.HasFlag(Roles.Administrator);
    }

    // Checks if user has ANY of the allowed role flags.
    public bool IsUserInAnyRoles(Roles userRoles, Roles allowedRoles)
    {
        var flags = allowedRoles.GetFlags();
        return flags.Any(flag => userRoles.HasFlag(flag));
    }

    // Checks if user has ALL required role flags.
    public bool IsUserInAllRoles(Roles userRoles, Roles requiredRoles)
    {
        return ((userRoles & requiredRoles) == requiredRoles);
    }

    // Validate authorization
    public bool IsAuthorized(UserSessionInfoViewModel user, Roles roles)
    {
        // convert comma delimited roles to enum flags, and check privileges.
        var userRoles = AggregateRoles(user.Roles);
        return IsAdministrator(userRoles) || IsUserInAnyRoles(userRoles, roles);
    }
}

I chose to use this in my controllers via an attribute:

public class AuthorizationFilter : IAuthorizationFilter
{
    private readonly IAuthorizationService _authorizationService;
    private readonly Roles _authorizedRoles;

    /// <summary>
    /// Constructor
    /// </summary>
    /// <remarks>The AuthorizedRolesAttribute is used on actions and designates the 
    /// required roles. Using dependency injection we inject the service, as well 
    /// as the attribute's constructor argument (Roles).</remarks>
    public AuthorizationFilter(IAuthorizationService authorizationService, Roles authorizedRoles)
    {
        _authorizationService = authorizationService;
        _authorizedRoles = authorizedRoles;
    }

    /// <summary>
    /// Uses injected authorization service to determine if the session user 
    /// has necessary role privileges.
    /// </summary>
    /// <remarks>As authorization code runs at the action level, after the 
    /// caching module, our authorization code is hooked into the caching 
    /// mechanics, to ensure unauthorized users are not served up a 
    /// prior-authorized page. 
    /// Note: Special thanks to TheCloudlessSky on StackOverflow.
    /// </remarks>
    public void OnAuthorization(AuthorizationContext filterContext)
    {
        // User must be authenticated and Session not be null
        if (!filterContext.HttpContext.User.Identity.IsAuthenticated || filterContext.HttpContext.Session == null)
            HandleUnauthorizedRequest(filterContext);
        else {
            // if authorized, handle cache validation
            if (_authorizationService.IsAuthorized((UserSessionInfoViewModel)filterContext.HttpContext.Session["user"], _authorizedRoles)) {
                var cache = filterContext.HttpContext.Response.Cache;
                cache.SetProxyMaxAge(new TimeSpan(0));
                cache.AddValidationCallback((HttpContext context, object o, ref HttpValidationStatus status) => AuthorizeCache(context), null);
            }
            else
                HandleUnauthorizedRequest(filterContext);             
        }
    }

I decorate Actions in my Controllers with this attribute, and like Microsoft's [Authorize] no params means let in anyone authenticated (for me it is Enum = 0, no required roles).

That about wraps up the background info (phew)... and writing all this out I answered my first question. At this point I am curious about the appropriateness of my setup:

1. Do I need to manually snag the auth cookie and populate the FormsIdentity principal for the HttpContext or should that be automatic?

2. Any issues with checking authentication within the attribute/filter OnAuthorization()?

3. What are tradeoffs in using Session[] to store my view model vs. serializing it within the auth cookie?

4. Does this solution seem to follow the 'separation of concerns' ideals well enough? (Bonus as it is more opinion-oriented question)

Answer 1

Cross-post from my CodeReview answer:

I'll take a stab at answering your questions and provide some suggestions:

1. If you have FormsAuthentication configured in web.config, it will automatically pull the cookie for you, so you shouldn't have to do any manual population of the FormsIdentity. This is pretty easy to test in any case.

2. You probably want to override both AuthorizeCore and OnAuthorization for an effective authorization attribute. The AuthorizeCore method returns a boolean and is used to determine whether the user has access to a given resource. The OnAuthorization doesn't return and is generally used to trigger other things based on the authentication status.

3. I think the session-vs-cookie question is largely preference, but I'd recommend going with the session for a few reasons. The biggest reason is that the cookie is transmitted with every request, and while right now you may only have a little bit of data in it, as time progresses who knows what you'll stuff in there. Add encryption overhead and it could get large enough to slow down requests. Storing it in the session also puts ownership of the data in your hands (versus putting it in the client's hands and relying on you to decrypt and use it). One suggestion I would make is wrapping that session access up in a static UserContext class, similar to HttpContext, so you could just make a call like UserContext.Current.UserData. See below for example code.

4. I can't really speak to whether it is a good separation of concerns, but it looks like a good solution to me. It's not unlike other MVC authentication approaches I've seen. I'm using something very similar in my apps in fact.

One last question -- why did you build and set the FormsAuthentication cookie manually instead of using FormsAuthentication.SetAuthCookie? Just curious.

Example code for static context class

public class UserContext
{
    private UserContext()
    {
    }

    public static UserContext Current
    {
        get
        {
            if (HttpContext.Current == null || HttpContext.Current.Session == null)
                return null;

            if (HttpContext.Current.Session["UserContext"] == null)
                BuildUserContext();

            return (UserContext)HttpContext.Current.Session["UserContext"];
        }
    }

    private static void BuildUserContext()
    {
        BuildUserContext(HttpContext.Current.User);
    }

    private static void BuildUserContext(IPrincipal user)
    {
        if (!user.Identity.IsAuthenticated) return;

        // For my application, I use DI to get a service to retrieve my domain
        // user by the IPrincipal
        var personService = DependencyResolver.Current.GetService<IUserBaseService>();
        var person = personService.FindBy(user);

        if (person == null) return;

        var uc = new UserContext { IsAuthenticated = true };

        // Here is where you would populate the user data (in my case a SiteUser object)
        var siteUser = new SiteUser();
        // This is a call to ValueInjecter, but you could map the properties however
        // you wanted. You might even be able to put your object in there if it's a POCO
        siteUser.InjectFrom<FlatLoopValueInjection>(person);

        // Next, stick the user data into the context
        uc.SiteUser = siteUser;

        // Finally, save it into your session
        HttpContext.Current.Session["UserContext"] = uc;
    }


    #region Class members
    public bool IsAuthenticated { get; internal set; }
    public SiteUser SiteUser { get; internal set; }

    // I have this method to allow me to pull my domain object from the context.
    // I can't store the domain object itself because I'm using NHibernate and
    // its proxy setup breaks this sort of thing
    public UserBase GetDomainUser()
    {
        var svc = DependencyResolver.Current.GetService<IUserBaseService>();
        return svc.FindBy(ActiveSiteUser.Id);
    }

    // I have these for some user-switching operations I support
    public void Refresh()
    {
        BuildUserContext();
    }

    public void Flush()
    {
        HttpContext.Current.Session["UserContext"] = null;
    }
    #endregion
}

In the past I had put properties directly on the UserContext class for accessing the user data I needed, but as I've used this for other, more complicated projects, I decided to move it to a SiteUser class:

public class SiteUser
{
    public int Id { get; set; }
    public string FirstName { get; set; }
    public string LastName { get; set; }
    public string FullName
    {
        get { return FirstName + " " + LastName; }
    }
    public string AvatarUrl { get; set; }

    public int TimezoneUtcOffset { get; set; }

    // Any other data I need...
}

Answer 2

While I think you're doing a fine job with this, I question why you are recreating the wheel. Since microsoft provides a system for this, called Membership and Role Providers. Why not just write a custom membership and role provider, then you don't have to create your own authization attribute and/or filters and can just use the inbuilt one.