How to get User at Service Layer

Question

I use ASP.NET Core 2.1 and would like to fetch User at a service level.

I've seen examples when HttpContextAccessor gets injected into some service and then we fetch the current User via UserManager

var user = await _userManager.GetUserAsync(accessor.HttpContext.User);

or in controller

var user = await _userManager.GetUserAsync(User);

---

Problems:

• Injecting HttpContextAccessor into service seems to be wrong - simply because we violate SRP and the Service Layer isn't isolated (it is dependant on http context).

• We can of course fetch user in a controller (a somewhat better approach), but we face a dilemma - we simply don't want to pass User as parameter in every single service method

I spent a few hours thinking about how best to implement it and have come up with a solution. I'm just not entirely sure my approach is adequate and doesn't violate any of the software-design principles.

Sharing my code in hopes to get recommendations from StackOverflow community.

The idea is the following:

First, I introduce SessionProvider which is registered as Singleton.

services.AddSingleton<SessionProvider>();

SessionProvider has a Session property which holds User, Tenant, etc.

Secondly, I introduce SessionMiddleware and register it

app.UseMiddleware<SessionMiddleware>();

In the Invoke method I resolve HttpContext, SessionProvider & UserManager.

• I fetch User

• Then I initialise Session property of ServiceProvider singleton:

sessionProvider.Initialise(user);

At this stage ServiceProvider has Session object containing the info we need.

Now we inject SessionProvider into any service and its Session object is ready for use.

---

Code:

SessionProvider:

public class SessionProvider
{
    public Session Session;

    public SessionProvider()
    {
        Session = new Session();
    }

    public void Initialise(ApplicationUser user)
    {
        Session.User = user;
        Session.UserId = user.Id;
        Session.Tenant = user.Tenant;
        Session.TenantId = user.TenantId;
        Session.Subdomain = user.Tenant.HostName;
    }
}

Session:

public class Session
{
    public ApplicationUser User { get; set; }

    public Tenant Tenant { get; set; }

    public long? UserId { get; set; }

    public int? TenantId { get; set; }

    public string Subdomain { get; set; }
}

SessionMiddleware:

public class SessionMiddleware
{
    private readonly RequestDelegate next;

    public SessionMiddleware(RequestDelegate next)
    {
        this.next = next ?? throw new ArgumentNullException(nameof(next));
    }

    public async Task Invoke(
        HttpContext context,
        SessionProvider sessionProvider,
        MultiTenancyUserManager<ApplicationUser> userManager
        )
    {
        await next(context);

        var user = await userManager.GetUserAsync(context.User);

        if (user != null)
        {
            sessionProvider.Initialise(user);
        }
    }
}

And now Service Layer code:

public class BaseService
{
    public readonly AppDbContext Context;
    public Session Session;

    public BaseService(
        AppDbContext context,
        SessionProvider sessionProvider
        )
    {
        Context = context;
        Session = sessionProvider.Session;
    }
}

So this is the base class for any service, as you can see we can now fetch Session object easily and it's ready for use:

public class VocabularyService : BaseService, IVocabularyService
{
    private readonly IVocabularyHighPerformanceService _vocabularyHighPerformanceService;
    private readonly IMapper _mapper;

    public VocabularyService(
        AppDbContext context,
        IVocabularyHighPerformanceService vocabularyHighPerformanceService,
        SessionProvider sessionProvider,
        IMapper mapper
        ) : base(
              context,
              sessionProvider
              )
    {
        _vocabularyHighPerformanceService = vocabularyHighPerformanceService;
        _mapper = mapper; 
    }

    public async Task<List<VocabularyDto>> GetAll()
    {
        List<VocabularyDto> dtos = _vocabularyHighPerformanceService.GetAll(Session.TenantId.Value);
        dtos = dtos.OrderBy(x => x.Name).ToList();
        return await Task.FromResult(dtos);
    }
}

Focus on the following bit:

.GetAll(Session.TenantId.Value);

also, we can easily get current user

Session.UserId.Value

or

Session.User

So, that's it.

I tested my code and it works well when several tabs are open - each tab has different subdomain in url (Tenant is resolved from subdomain - the data is being fetched correctly).

Milli

If this is working code and there really is not an actual problem other than the opinion that it is not a good design then I would say that the question is off topic for SO as it is more of a code review which should fit on codereview.stackexchange.com – Zaxis

@Zaxis Ah I understand. Next time I'll be asking this kind of questions on codereview. Thanks! – Milli

Note that you are focusing too much on implementation concerns where abstractions would work well. abstracting the session provider simplifies the services's dependencies. So it would not even matter that you are using the IHttpContextAccessor to get the current user. Again that is just my opinion on the topic of this question.. – Zaxis

I also just noticed that you are settings the session in the middleware AFTER passing the context down the pipeline, which would mean that the session would not be available to other handlers in the pipline – Zaxis

hi @Nkosi, you are absolutely right. I've just tried registering ServiceProvider as scoped, and the Session has all its fields being null. – Milli

@Zaxis However, if I pass the context down the pipeline at the end - it's impossible to fetch data from HttpContext needed to fetch current user. So that's another dilemma. – Milli

Which reveals itself as a flaw in your current design. By moving to a middleware you are too early in the pipeline to get the desired behavior. – Zaxis

@Zaxis Implemented Filter instead of MiddleWare - right now it works - updated my post. Thanks again for pointing out the problem. – Milli

Please do not edit your question to include the solution. If the answers are not definitive, post your own answer – Wachter

How would you use a repository on your service layer if you state that injecting a dependency in you service violates the SRP and the service layer isn't isolated? – Carnallite

@JoãoPaiva it is okay to use a higher-level dependency in a lower-level one, but not vice versa. Injecting repo into some service sounds correct and doesn't violate SRP. But e.g. injecting a controller-level HttpContextAccessor into a higher-level dependency like repo or service sounds wrong. – Milli

Answer 1

Using an action filter would ensure that your desired behavior is invoked late enough in the action invocation pipeline that the necessary dependencies have already been realized, (like HttpContext.User)

Reference Filters in ASP.NET Core

Implement an async action filter to avoid calling .Result blocking calls as it may cause deadlocks in the request pipeline.

public class SessionFilter : IAsyncActionFilter {
    public async Task OnActionExecutionAsync(
        ActionExecutingContext context,
        ActionExecutionDelegate next) {

        // do something before the action executes

        var serviceProvider = context.HttpContext.RequestServices;    
        var sessionProvider = serviceProvider.GetService<SessionProvider>();
        var userManager = serviceProvider.GetService<MultiTenancyUserManager<ApplicationUser>>()

        var user = await userManager.GetUserAsync(context.HttpContext.User);    
        if (user != null) {
            sessionProvider.Initialise(user);
        }

        //execute action
        var resultContext = await next();
        // do something after the action executes; resultContext.Result will be set
        //...
    }
}

Zaxis

Thanks, very useful. Was removing .Result earlier today, but copy-pasted it from clipboard by chance. Amended my code and tested - WORKS! – Milli

perfect solution – Pedicle

Answer 2

Here's a better workaround in my opinion - we no longer make a DB call per every single request, we just retrieve UserID & TenantID from Claims instead:

Please note that the lifetime of Session is Per Request - when the request starts we hook into it, resolve SessionContext instance, then populating it with UserID & TenantID - after this wherever we inject our Session (given the same request) - it will contain the values we need.

services.AddScoped<Session>();

Session.cs

public class Session
{
    public long? UserId { get; set; }

    public int? TenantId { get; set; }

    public string Subdomain { get; set; }
}

AppInitializationFilter.cs

public class AppInitializationFilter : IAsyncActionFilter
{
    private Session _session;
    private DBContextWithUserAuditing _dbContext;
    private ITenantService _tenantService;

    public AppInitializationFilter(
        Session session,
        DBContextWithUserAuditing dbContext,
        ITenantService tenantService
        )
    {
        _session = session;
        _dbContext = dbContext;
        _tenantService = tenantService;
    }

    public async Task OnActionExecutionAsync(
        ActionExecutingContext context,
        ActionExecutionDelegate next
        )
    {
        string userId = null;
        int? tenantId = null;

        var claimsIdentity = (ClaimsIdentity)context.HttpContext.User.Identity;

        var userIdClaim = claimsIdentity.Claims.SingleOrDefault(c => c.Type == ClaimTypes.NameIdentifier);
        if (userIdClaim != null)
        {
            userId = userIdClaim.Value;
        }

        var tenantIdClaim = claimsIdentity.Claims.SingleOrDefault(c => c.Type == CustomClaims.TenantId);
        if (tenantIdClaim != null)
        {
            tenantId = !string.IsNullOrEmpty(tenantIdClaim.Value) ? int.Parse(tenantIdClaim.Value) : (int?)null;
        }

        _dbContext.UserId = userId;
        _dbContext.TenantId = tenantId;

        string subdomain = context.HttpContext.Request.GetSubDomain();

        _session.UserId = userId;
        _session.TenantId = tenantId;
        _session.Subdomain = subdomain;

        _tenantService.SetSubDomain(subdomain);

        var resultContext = await next();
    }
}

AuthController.cs

[Route("api/[controller]/[action]")]
[ApiController]
public class AuthController : Controller
{
    public IConfigurationRoot Config { get; set; }
    public IUserService UserService { get; set; }
    public ITenantService TenantService { get; set; }

    [AllowAnonymous]
    [HttpPost]
    public async Task<AuthenticateOutput> Authenticate([FromBody] AuthenticateInput input)
    {
        var expires = input.RememberMe ? DateTime.UtcNow.AddDays(5) : DateTime.UtcNow.AddMinutes(20);

        var user = await UserService.Authenticate(input.UserName, input.Password);

        if (user == null)
        {
            throw new Exception("Unauthorised");
        }

        int? tenantId = TenantService.GetTenantId();
        string strTenantId = tenantId.HasValue ? tenantId.ToString() : string.Empty;

        var tokenHandler = new JwtSecurityTokenHandler();

        var tokenDescriptor = new SecurityTokenDescriptor
        {
            Expires = expires,
            Issuer = Config.GetValidIssuer(),
            Audience = Config.GetValidAudience(),
            SigningCredentials = new SigningCredentials(Config.GetSymmetricSecurityKey(), SecurityAlgorithms.HmacSha256),
            Subject = new ClaimsIdentity(new[]
            {
                new Claim(ClaimTypes.Name, user.UserName),
                new Claim(ClaimTypes.NameIdentifier, user.Id),
                new Claim(CustomClaims.TenantId, strTenantId)
            })
        };

        var token = tokenHandler.CreateToken(tokenDescriptor);
        string tokenString = tokenHandler.WriteToken(token);

        return new AuthenticateOutput() { Token = tokenString };
    }
}

Milli

Answer 3

Your approach seems to be correct. The only problem - you shouldn't register SessionProvider as Singleton, otherwise you'll have problems with simultaneous requests. Register it as Scoped to get a new instance for each request. Also, you have to fill SessionInfo before calling next middleware. As Nikosi mentioned middleware should be replaced with filter to obtain correct data regarding User. As for filter implementaion, it uses the service locator pattern which is considered as antipatern. The better way is to inject it with constructor and it is already supported by the framework. If you use it globally you just need to register it as:

public void ConfigureServices(IServiceCollection services)
{
    services.AddMvc(options =>
    {
        options.Filters.Add<SessionFilter>();
    });
}

or if you need it only with some actions you can apply filter with

[ServiceFilter(typeof(SessionFilter))]

In this case filter also should be registered:

public void ConfigureServices(IServiceCollection services)
{
    ...
    services.AddScoped<SessionFilter>();
    ...
}

Monomolecular

Hi Alex, thanks for your answer! Tried Scoped as well - doesn't work for some reason. Later I'll be publishing my code on GitHub - you'll be able to have a more detailed look. – Milli

Just BTW, we register HttpContextAccessor as singleton and it works pretty well for multuple requests - right? So there shouldn't be a problem with SessionProvider singleton either? (Unless I've missed something) – Milli

@AlexHerman HttpContextAccessor doesn't have state, while SessionProvider has – Monomolecular