Is this Custom Principal in Base Controller ASP.NET MVC 3 terribly inefficient?

Question

Despite the fact that I've been on here for a while, this is my first ever question on SO, so please be gentle with me.

I'm using ASP.NET MVC 3 and I want to create a custom Principal so I can store a bit more info about the current user than is standard thus not have to go to the database too often. It's fairly standard stuff that I'm after. Let's just say email address and user id in the first instance.

I have decided to store the object in the cache as I am aware that it is not advised to store it in the session.

I also don't want to have to keep casting the User object, so I wanted to override the User object in the controller. So I can just go User.UserId and be guaranteed of something.

So I created a custom principal like this:

public class MyPrincipal : IPrincipal
{
    public MyPrincipal(IIdentity ident, List<string> roles, string email, Guid userId)
    {
        this._identity = ident;
        this._roles = roles;
        this._email = email;
        this._userId = userId;
    }

    IIdentity _identity;

    public IIdentity Identity
    {
        get { return _identity; }
    }

    private List<string> _roles;

    public bool IsInRole(string role)
    {
        return _roles.Contains(role);
    }

    private string _email;

    public string Email
    {
        get { return _email; }
    }

    private Guid _userId;

    public Guid UserId
    {
        get { return _userId; }
    }
}

And I have a Base Controller like this:

public class BaseController : Controller
    {
        protected virtual new MyPrincipal User
        {
            get
            {
                if (base.User is MyPrincipal)
                {
                    return base.User as MyPrincipal;
                }
                else
                {
                    return new MyPrincipal(base.User.Identity, new List<string>(0), "", Guid.Empty );
                }
            }
        }

        protected override void OnAuthorization(AuthorizationContext filterContext)
        {
            if (User != null)
            {
                if (User.Identity.IsAuthenticated)
                {
                    if (User.Identity is FormsIdentity)
                    {
                        FormsIdentity id = base.User.Identity as FormsIdentity;
                        MyPrincipal principal = (MyPrincipal)filterContext.HttpContext.Cache.Get(id.Name);
                        if (principal == null)
                        {
                            MembershipUser user = Membership.GetUser();

                            // Create and populate your Principal object with the needed data and Roles.
                            principal = new MyPrincipal(id, Roles.GetRolesForUser(id.Name).ToList(), user.Email, (Guid)user.ProviderUserKey);
                            filterContext.HttpContext.Cache.Add(
                            id.Name,
                            principal,
                            null,
                            System.Web.Caching.Cache.NoAbsoluteExpiration,
                            new System.TimeSpan(0, 30, 0),
                            System.Web.Caching.CacheItemPriority.Default,
                            null);
                        }
                        filterContext.HttpContext.User = principal;
                        System.Threading.Thread.CurrentPrincipal = principal;
                        base.OnAuthorization(filterContext);
                    }
                }
            }
        }
    }

If you have a look you will quickly realise that if the user has not logged in then any call to the User object will have to run through this bit of code:

return new MyPrincipal(base.User.Identity, new List<string>(0), "", Guid.Empty );

and this feels terribly inefficient to me, although it's only creating empty objects for the missing stuff.

It works fine.

So I guess I want to know if this is actually okay and I should stop being so anal about performance and efficiency, or if my fears are correct, in which case what should I be doing instead? [Please don't say "Getting a life, mate!"]

Answer 1

No - there is nothing specifically wrong with this code from a performance stand point that stands out. PLENTY of objects are creating on the back end in ASP.NET, your single object is a drop in the bucket. Since class instantiation is extremely fast I wouldn't be concerned about it.

Why are you ignoring sessions here? Session information doesn't have expiration dates, so there is no extra check behind the scenes. Unless you are using an out of proc session server, there is no serialization of your object (none with the cache either). The cache is for every user - so you right a chance (albeit slight) of a code error returning the wrong principal where a cache being per user - does not run the risk of that.

If you want this available for all requests there (not just MVC based) I would consider setting this in Application_PostAuthenticateRequest