3

TLDR; I have near identical controllers that differ materially only by use of async/await (and of course Task). The non-async version returns the current user's username, as expected, but the async equivalent does not.

What I'd like to discover are

  1. The circumstances under which this might hold true
  2. That tools (besides Fiddler) available to me for investigating this

Note: I'm unable to debug against the ASP.NET Core source because permission hasn't been granted for running Set-ExecutionPolicy (which appears required for building the solution)

This works:

using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;

namespace Baffled.API.Controllers
{
    [Authorize]
    [ApiController]
    [Route("[controller]")]
    public class IdentityController : ControllerBase
    {
        private readonly string userName;

        public IdentityController(IHttpContextAccessor httpContextAccessor)
        {
            userName = httpContextAccessor?.HttpContext?.User.Identity?.Name;
        }

        [HttpGet]
        public IActionResultGetCurrentUser()
        {
            return Ok(new { userName });
        }
    }
}

This does not work:

using System.Threading.Tasks;
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.Logging;
using Baffled.Services;

namespace Baffled.API.Controllers
{
    [Authorize]
    [ApiController]
    [Route("[controller]")]
    public class IssueReproductionController : ControllerBase
    {
        private readonly HeartbeatService heartbeatService;
        private readonly ILogger<IssueReproductionController> logger;
        private readonly string userName;

        public IssueReproductionController(
            HeartbeatService heartbeatService,
            ILogger<IssueReproductionController> logger,
            IHttpContextAccessor httpContextAccessor)
        {
            this.heartbeatService = heartbeatService;
            this.logger = logger;

            userName = httpContextAccessor?.HttpContext?.User?.Identity?.Name;
        }

        [HttpGet]
        public async Task<IActionResult> ShowProblem()
        {
            var heartbeats = await heartbeatService.GetLatest();

            logger.LogInformation("Issue reproduction", new { heartbeats });

            var currentUser = userName ?? HttpContext?.User.Identity?.Name;

            return Ok(new { currentUser, heartbeats.Data });
        }
    }
}

I think the configuration is correct but for completeness it's included below:

Program.cs

using System;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Server.HttpSys;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.Hosting;

namespace Baffled.API
{
    public class Program
    {
        public static void Main(string[] args)
        {
            CreateHostBuilder(args).Build().Run();
        }

        public static IHostBuilder CreateHostBuilder(string[] args) =>
            Host.CreateDefaultBuilder(args)
                .ConfigureAppConfiguration((_, config) =>
                {
                    config.AddJsonFile("appsettings.json", true, true);
                    config.AddEnvironmentVariables();
                })
                .UseWindowsService()
                .ConfigureWebHostDefaults(webBuilder =>
                {
                    webBuilder.UseUrls("http://*:5002");
                    webBuilder.UseStartup<Startup>().UseHttpSys(Options);
                });

        private static void Options(HttpSysOptions options)
        {
            options.Authentication.AllowAnonymous = true;
            options.Authentication.Schemes =
                AuthenticationSchemes.NTLM | 
                AuthenticationSchemes.Negotiate | 
                AuthenticationSchemes.None;
        }
    }
}

Startup.cs

using System;
using System.Net.Http;
using Baffled.API.Configuration;
using Baffled.API.Hubs;
using Baffled.API.Services;
using Baffled.EF;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Server.HttpSys;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using Microsoft.Extensions.Logging;
using Newtonsoft.Json.Serialization;
using Serilog;
using Serilog.Events;

namespace Baffled.API
{
    public class Startup
    {
        public Startup(IConfiguration configuration)
        {
            Configuration = configuration;
        }

        public IConfiguration Configuration { get; }

        public void ConfigureServices(IServiceCollection services)
        {
            services.AddScoped<HeartbeatService, HeartbeatService>();

            services.AddHttpContextAccessor();
            services.AddAuthentication(HttpSysDefaults.AuthenticationScheme).AddNegotiate();
            services.AddHttpClient("Default").ConfigurePrimaryHttpMessageHandler(() => new HttpClientHandler { UseDefaultCredentials = true });
            services.AddControllers();

            var corsOrigins = Configuration.GetSection("CorsAllowedOrigins").Get<string[]>();

            services.AddCors(_ => _.AddPolicy("AllOriginPolicy", builder =>
            {
                builder.WithOrigins(corsOrigins)
                    .AllowAnyMethod()
                    .AllowAnyHeader()
                    .AllowCredentials();
            }));

            services.AddSwagger();
            services.AddSignalR();
            services.AddHostedService<Worker>();
            services.AddResponseCompression();

            // Logging correctly configured here
        }

        public void Configure(IApplicationBuilder app, IWebHostEnvironment env)
        {
            app.UseRouting();
            app.UseSwagger();
            app.UseAuthentication();
            app.UseAuthorization();
            app.UseResponseCompression();
            app.UseCors("AllOriginPolicy");

            app.UseEndpoints(endpoints =>
            {
                endpoints.MapControllers();
                endpoints.MapHub<UpdateHub>("/updatehub");
            });
        }
    }
}

I've been sitting on this for weeks hoping a solution might occur. None has. Please help.

Edit

I have this hosted via a windows service. The odd behaviour seems to be inconsistent and non-deterministic. On occasion I see my username being returned but my colleagues never do when hitting the problematic endpoint.

Amal K
  • 4,359
  • 2
  • 22
  • 44
Kofi Sarfo
  • 3,310
  • 6
  • 23
  • 24
  • 2
    What happens, if you save the `IHttpContextAccessor` in the in `IssueReproductionController` constructor, and use this saved accessor instead of the static `HttpContext` in ` ShowProblem()` ? – TheHvidsten Jun 20 '21 at 12:07
  • So if I understand correctly `userName` is empty in the constructor if you check it using a breakpoint? – MaartenDev Jun 20 '21 at 12:08
  • Yes, @MaartenDev, ```userName``` is empty in the ctor – Kofi Sarfo Jun 20 '21 at 12:53
  • Thanks @GTHvidsten, I believe I tried all possible combinations I could think of but I'll give that another go to be sure. – Kofi Sarfo Jun 20 '21 at 12:55
  • Move the retrieval of the user name out of the constructor and into the action method itself (store the context accessor into a field). The user only gets set after the authentication/authorization Middlewares have run. It appears (perhaps) in the non-async case that the controller isnt constructed until (perhaps) it is required, but in the task-returning case it's constructed up front (or at least prior to the async local storing the context and user have been populated). It's safest to extract the info from inside the action method and not the constructor. – pinkfloydx33 Jun 20 '21 at 13:51
  • In fact, I bet if you stored off the context instead (and not the username) from the constructor into a field you'd find that when you accessed it from the action that it was null (and since you are null propagating the username that would make sense to me) – pinkfloydx33 Jun 20 '21 at 13:53
  • 2
    But also note that as you are doing all of this inside of a controller and action method, the `IHttpContextAccesor` is **not required**. From within your action method you can access both the context (`this.HttpContext`) and the user (`this.HttpContext.User` or `this.User`) directly as they are properties of `ControllerBase`... No need for the accessor – pinkfloydx33 Jun 20 '21 at 13:57

2 Answers2

6

You shouldn’t attempt to access HttpContext-specific data within your controller constructor. The framework makes no guarantees when the constructor gets constructed, so it is very possible that the constructor is created here before the IHttpContextAccessor has access to the current HttpContext. Instead, you should access context-specific data only within your controller action itself since that is guaranteed to run as part of the request.

When using the IHttpContextAcccessor, you should always access its HttpContext only in that exact moment, when you need to look at the context. Otherwise, it is very likely that you are working with an outdated state which can cause all kinds of problems.

However, when using a controller, you don’t actually need to use IHttpContextAccessor at all. Instead, the framework will provide multiple properties for you so that you can access the HttpContext or even the user principal directly on the controller (with Razor pages and in Razor views, there are similar properties available to use):

Note though that these properties are only available within a controller action, and not the constructor. For more details on that, see this related post.


Coming to your example, your controller action should work just like this:

[HttpGet]
public async Task<IActionResult> ShowProblem()
{
    var heartbeats = await heartbeatService.GetLatest();
    logger.LogInformation("Issue reproduction", new { heartbeats });

    var currentUser = User.Identity.Name;

    return Ok(new { currentUser, heartbeats.Data });
}
poke
  • 369,085
  • 72
  • 557
  • 602
-1

In .NET Core, you cannot retrieve the user identity direct from HttpContext. You will also need to use dependency injection to obtain a reference to the http context instance.

This has always worked for me:

public class IssueReproductionController : ControllerBase
{
    private readonly HeartbeatService heartbeatService;
    private readonly ILogger<IssueReproductionController> logger;
    private readonly string userName;

    private readonly HttpContext context;

    public IssueReproductionController(
        HeartbeatService heartbeatService,
        ILogger<IssueReproductionController> logger,
        IHttpContextAccessor httpContextAccessor)
    {
        this.heartbeatService = heartbeatService;
        this.logger = logger;
        this.context = httpContextAccessor.HttpContext;
    }

    [HttpGet]
    public async Task<IActionResult> ShowProblem()
    {
       ...
       var currentUser = this.context.Request.HttpContext.User.Identity.Name;
       ...
    }
    ...
Andrew Halil
  • 1,195
  • 15
  • 15
  • 21
  • This is false. If the authentication middleware is working properly, `HttpContext.User` is populated. And you can access the user with `User` property or through `HttpContext.User`. https://github.com/dotnet/aspnetcore/blob/c5800e2a83d73e696088c455c7d2cb0b7ac9018a/src/Security/Authentication/Core/src/AuthenticationMiddleware.cs#L72 – abdusco Jun 20 '21 at 12:17
  • Second, a controller works inside the request context, meaning it always has access to `HttpContext`. You don't need to inject `IHttpContextAccessor` inside the controller, you can simply use `ControllerBase.HttpContext` property – abdusco Jun 20 '21 at 12:19
  • I have edited my answer. The request context can be injected in both cases, either within a controller or inside of a class outside of the controller. With .NET standard you can use HttpContext.User directly, however the same is not true in .NET Core. – Andrew Halil Jun 20 '21 at 12:25