Search code examples
c#asp.netasp.net-mvciprincipal

IPrincipal property is null when accessed from ASP.NET MVC controller constructor


The following sample code throws NullReferenceException with message Object reference not set to an instance of an object.. The error refers to the User property which is null.

Base controller

public class BaseController : Controller
{
    public string UserName
    {
        get
        {
            return User.Identity.Name;
        }
    }
}

Home controller

[Authorize]
public class HomeController : BaseController
{
    private string username { get; set; }

    public HomeController()
    {
        username = UserName;
    }
    // GET: Home
    public ActionResult Index()
    {
        ViewBag.UserName = username;
        return View();
    }
}

On the other hand, if the UserName property is accessed directly inside the action then it works fine. So, the code works if it is changed to:

Home controller

[Authorize]
public class HomeController : BaseController
{
    // GET: Home
    public ActionResult Index()
    {
        ViewBag.UserName = this.UserName;
        return View();
    }
}

Any ideas why?

UPDATE:

So, the real problem is the HttpContext is unavailable during the construction of the controller. I really wonder why? Any explanation? Plus, what if I need to use constructor DI to instantiate a dependency class which in turn needs to know the currently logged in user, for example:

[Authorize]
public class HomeController : BaseController
{
    private IMyService service;

    public HomeController(IMyService service)
    {
        this.service = service;
        this.service.UserName = UserName;
    }

    // GET: Home
    public ActionResult Index()
    {
        // use service here ...
        return View();
    }
}

Solution

  • You do not have access to the HttpContext in the constructor. If you wanted a single place to add such retrieval logic you could do it in one of these ways.


    Call Base Controller directly

    Access UserName property from your base controller, there is no need to reset this as a property or field on the derived controller.

    [Authorize]
    public class HomeController : BaseController
    {
        // GET: Home
        public ActionResult Index()
        {
            ViewBag.UserName = UserName;
            return View();
        }
    }
    

    Hook into OnActionExecuting instead of the ctor

    Set the value in Controller.OnActionExecuting instead of the constructor as you have access to the HttpContext at this point. Add an override to the method to do this.


    Extension method

    Add a method extension instead. I prefer this approach because I do not like controller inheritence, just inherit from the standard Mvc controller instead.

    public static class ControllerExtension {
      public static string UserName(this Controller controller) {
        return controller.User.Identity.Name;
      }
    }
    

    And your code becomes

    [Authorize]
    public class HomeController : Controller
    {
        // GET: Home
        public ActionResult Index()
        {
            ViewBag.UserName = this.UserName();
            return View();
        }
    }
    

    Edit

    ... where constructor DI is used to instantiate a parameter which in turn has a UserName property that needs to be set...

    You would want to inject the HttpContextBase instance into your service and have it retrieve the authenticated user name that way. What you do not want is that the Controller instance needs to provide the user name to the service as this creates tightly coupled code and breaks SoC (Separation of Concerns).

    class MyService : IMyService
    {
        public string UserName {get;}
        // inject HttpContextBase
        public MyService(HttpContextBase context){
            this.UserName = context.User.Identity.Name;
        }
    }