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!"]

Tajo

Don't forget Knuth... "Premature optimization is the root of all evil." That is to say, have you actually seen performance issues? – Turin

+1 Nice one. No I haven't. But the thing is I was feeling pretty smart at having made it all work until I looked at that nasty line of code! Wow! I really am anal! I guess that's what comes of having started coding on the BBC Micro where clock cycles were few and far between... – Tajo

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

Lefthander

+1 Thanks Adam. Other than the security consideration there's no particular reason I'm ignoring the session. I deliberately didn't include it in Application_PostAuthenticateRequest as I reckon the code in there would be run for requests of things like css, images, javascript, etc whereas with my code it won't. Is that right? Any images that need to be protected are served by MVC FileContentResult methods and thus are still covered. – Tajo

Answer 2

This post may be of use. Notice the use of userdata in the authentication ticket.

ASP.NET MVC - Set custom IIdentity or IPrincipal

Delindadelineate

Thanks for this. I already know about using forms authentication tickets but I wanted to do it without using a cookie, unless you're suggesting I store it somewhere else. – Tajo