SignInManager constructor incorrectly checks contextAccessor.HttpContext

[dotnetjunkie]

At line 44, the SignInManager's constructor verifies the contextAccessor input argument for null. This is correct, however, it incorrectly checks whether the HttpContext returns a not-null value:

    if (contextAccessor == null || contextAccessor.HttpContext == null)
    {
        throw new ArgumentNullException(nameof(contextAccessor));
    }

This behavior of the SignInManager is incorrect. The IHttpContextAccessor exists to be able to delay retrieving the HttpContext runtime value until after the object graph is constructed. During the phase where the object graph is constructed, certain runtime data might not be available. This is exactly the case when dealing with the IHttpContextAccessor in case our DI configuration is verified. This verification of our DI container (either the built-in container or a custom one) can be done either when the application is started, or can be done inside a unit test. In both cases the HttpContext property of the IHttpContextAccessor will return null. Checking that HttpContext value inside the constructor has the same effect as directly injecting the HttpContext into the constructor and bypassing the IHttpContextAccessor altogether, which is a bad idea btw.

In other words, the || contextAccessor.HttpContext == null check should be removed. This allows the SignInManager to be constructed outside the scope of an active HTTP context.

[davidfowl]

+1

[divega]

We should make sure the check goes away with the fix for #655 which will make IHttpContextAccessor optional. Assigning to RC2.

cc @HaoK @rustd @blowdart

[HaoK]

167bb54