Null User on HttpContext obtained from StructureMap
Asked Answered
A

1

0

Ok, my previous question/setup had too many variables, so I'm stripping this down to it's bare bones components.

Given the code below using StructureMap3...

//IoC setup
For<HttpContextBase>().UseSpecial(x => x.ConstructedBy(y => HttpContext.Current != null ? new HttpContextWrapper(HttpContext.Current) : null ));
For<ICurrentUser>().Use<CurrentUser>();

//Classes used
public class CurrentUser : ICurrentUser
{
    public CurrentUser(HttpContextBase httpContext)
    {
        if (httpContext == null) return;
        if (httpContext.User == null) return;
        var user = httpContext.User;
        if (!user.Identity.IsAuthenticated) return;
        UserId = httpContext.User.GetIdentityId().GetValueOrDefault();
        UserName = httpContext.User.Identity.Name;
    }

    public Guid UserId { get; set; }
    public string UserName { get; set; }
}

public static class ClaimsExtensionMethods
    public static Guid? GetIdentityId(this IPrincipal principal)
    {
        //Account for possible nulls
        var claimsPrincipal = principal as ClaimsPrincipal;
        if (claimsPrincipal == null)
            return null;
        var claimsIdentity = claimsPrincipal.Identity as ClaimsIdentity;
        if (claimsIdentity == null)
            return null;
        var claim = claimsIdentity.FindFirst(x => x.Type == ClaimTypes.NameIdentifier);
        if (claim == null)
            return null;

        //Account for possible invalid value since claim values are strings
        Guid? id = null;
        try
        {
            id = Guid.Parse(claim.Value);
        }
        catch (ArgumentNullException) { }
        catch (FormatException) { }
        return id;
    }
}

How is this possible in the Watch window?

enter image description here


I have a web application that I'm upgrading to using StructureMap 3.x from 2.x, but I'm getting odd behavior on specific dependency.

I have a ISecurityService that I use to obtain verify some things when a user requests a page. This service depends on a small interface that I've called ICurrentUser. The class implementation is pretty plain, really it could be a struct.

public interface ICurrentUser
{
    Guid UserId { get; }
    string UserName { get; }
}

This is obtained via dependency injection using the below code.

For<ICurrentUser>().Use(ctx => getCurrentUser(ctx.GetInstance<HttpContextBase>()));
For<HttpContextBase>().Use(() => getHttpContext());

private HttpContextBase getHttpContext()
{
    return new HttpContextWrapper(HttpContext.Current);
}

private ICurrentUser getCurrentUser(HttpContextBase httpContext)
{
    if (httpContext == null) return null;
    if (httpContext.User == null) return null; // <---
    var user = httpContext.User;
    if (!user.Identity.IsAuthenticated) return null;
    var personId = user.GetIdentityId().GetValueOrDefault();
    return new CurrentUser(personId, ClaimsPrincipal.Current.Identity.Name);
}

When a request comes in, my site wide authentication happens first, which depends on ISecurityService. This happens inside of OWIN and appears to occur before HttpContext.User has been populated, so it's null, so be it.

Later on, I have an ActionFilter that checks, via a ISecurityService, if the current user has agreed to the current version of the TermsOfUse for the site, if not they are redirected to the page to agree to them first.

This all worked fine in structuremap 2.x. For my migration to StructureMap3 I've installed the Nuget package StructureMap.MVC5 to help speed things up for me.

When my code gets to the line in my ActionFilter for checking the terms of use I have this.

var securityService = DependencyResolver.Current.GetService<ISecurityService>();
agreed = securityService.CheckLoginAgreedToTermsOfUse();

Inside of CheckLoginAgreedToTermsOfUse(), my instance of CurrentUser is null. Even though it would hazve succeeded, and my breakpoint inside of getCurrentUser() never seems to be hit. Its almost as if it's a foregone conclusion, since it was null the last time , even though it would have resolved this time.

I'm kind of baffled as to why getCurrentUser() is never called on the request for ISecurityService. I even tried explicitly sticking a .LifecycleIs<UniquePerRequestLifecycle>() on my hookup for handling ICurrentUser with no effect.

UPDATE: Ok so just a heads up, I've started using the method accepted below, and while it has worked great so far, it didn't resolve my core problem. Turns out the new StructureMap.MVC5, based on StructureMap3, uses NestedContainers. Which scope their requests to the lifetime of the NestedContainer, regardless of the default being Transient. So when I requested HttpContextBase for the first time, it will then return that same instance for the rest of the request (even though later on in the request lifespan, the context has changed. You need to either not use NestedContainer (which, as I understand it will complicate things ASP.NET vNext), or you explicitly set the lifecycle of the For<>().Use<>() mapping to give you a new instance per request. Note that this scoping per NestedContainer causes problems with Controllers as well in MVC. While the StructureMap.MVC5 package handles this with a ControllerConvention, it does not handle Views, and recursive views or views used multiple times will likely cause you problems as well. I'm still looking for a permanent fix for the Views problem, for the moment I've reverted to the DefaultContainer.

Ailee answered 21/11, 2014 at 0:9 Comment(2)
Could you post the code for your ISecurityService implementation?Sceptre
I had too many variables that would lead possible help down the wrong trail. I've simplified the question to it's bare bones problem nowAilee
S
5

I haven't worked with OWIN, but when hosting in IIS integrated mode the HttpContext is not populated until after the HttpApplication.Start event is complete. In terms of DI, this means that you cannot rely on using properties of HttpContext in any constructor.

This makes sense if you think about it because the application should be initialized outside of any individual user context.

To get around this, you could inject an abstract factory into your ICurrentUser implementation and to use a Singleton pattern to access it, which guarantees HttpContext won't be accessed until it is populated.

public interface IHttpContextFactory
{
    HttpContextBase Create();
}

public class HttpContextFactory
    : IHttpContextFactory
{
    public virtual HttpContextBase Create()
    {
        return new HttpContextWrapper(HttpContext.Current);
    }
}

public class CurrentUser // : ICurrentUser
{
    public CurrentUser(IHttpContextFactory httpContextFactory)
    {
        // Using a guard clause ensures that if the DI container fails
        // to provide the dependency you will get an exception
        if (httpContextFactory == null) throw new ArgumentNullException("httpContextFactory");

        this.httpContextFactory = httpContextFactory;
    }

    // Using a readonly variable ensures the value can only be set in the constructor
    private readonly IHttpContextFactory httpContextFactory;
    private HttpContextBase httpContext = null;
    private Guid userId = Guid.Empty;
    private string userName = null;

    // Singleton pattern to access HTTP context at the right time
    private HttpContextBase HttpContext
    {
        get
        {
            if (this.httpContext == null)
            {
                this.httpContext = this.httpContextFactory.Create();
            }
            return this.httpContext;
        }
    }

    public Guid UserId
    {
        get
        {
            var user = this.HttpContext.User;
            if (this.userId == Guid.Empty && user != null && user.Identity.IsAuthenticated)
            {
                this.userId = user.GetIdentityId().GetValueOrDefault();
            }
            return this.userId;
        }
        set { this.userId = value; }
    }

    public string UserName
    {
        get
        {
            var user = this.HttpContext.User;
            if (this.userName == null && user != null && user.Identity.IsAuthenticated)
            {
                this.userName = user.Identity.Name;
            }
            return this.userName;
        }
        set { this.userName = value; }
    }
}

Personally, I would make the UserId and UserName properties readonly, which would simplify the design and ensure they don't get hijacked elsewhere in the application. I would also make an IClaimsIdentityRetriever service that is injected into the constructor of ICurrentUser instead of retrieving the claims Id in an extension method. Extension methods go against the grain of DI and are generally only useful for tasks that are guaranteed not to have any dependencies (such as string or sequence manipulation). The loose coupling of making it a service also means you can easily swap or extend the implementation.

Of course, this implies that you cannot call the UserId or UserName properties of your CurrentUser class in any constructor as well. If any other class depends on ICurrentUser, you may also need an ICurrentUserFactory in order to safely use it.

Abstract factory is a lifesaver when dealing with difficult-to-inject dependencies and solves a host of problems including this one.

Sceptre answered 22/11, 2014 at 7:46 Comment(1)
Interesting, that's something I hadn't realized about Integrated Mode applications. I'm still not sure why StructureMap appears to behave the way it does in my example, but your suggested convention of using a combination of a factory with a singleton httpcontext, and readonly properties looks like it'll solve my problem. It also makes sense, given a few other parts of my application and how they behave, to adopt the same pattern elsewhere. Fantastic answer for what I was looking for. Thanks :-)Ailee

© 2022 - 2024 — McMap. All rights reserved.