Using user variables in dependency injection

Viewed 685

I've got a scoped service, that needs to instantiate with user specific variables.

builder.Services.AddScoped<IUserService, UserService>();

UserService currently has a logger as it's constructor.

I'm currently doing the following through a factory, in a hacky way:

public class UserServiceFactory
{
    private readonly ServiceProvider _sp;
    private readonly DbContext _db;

    public UserServiceFactory(ServiceProvider sp, DbContext db) { _sp = sp; _db = db; }

    public async Task<IUserService> GetUserServiceForUserAsync(Guid userId)
    {
         var (apiKey, apiSecret) = await _db.FetchApiKeyAndSecretAsync(userId);

         var userService = _sp.GetRequiredService<IUserService>();
         userService.InitUser(apiKey, apiSecret);
         return userService;
    }
}

I'm running into the following problems:

  • I can't use builder.Services.AddScoped<IUserService, UserService>(); with string parameters, because as soon as it attempts to register in DI, it can't resolve the string parameters in the constructor, even though the only place I'm going to be initializing it will be in the factory, and I'll be providing said string parameters.
  • If I don't use builder.Services.AddScoped<IUserService, UserService>();, I'd need to use Activator.CreateInstance<UserService>(...), which ties a concrete implementation to this class which is not ideal. In addition, I can't track said UserService for disposal when the scope gets disposed.
  • It was suggested that I register a Func<> to return a user service. If I do this, I don't believe it will be a scoped service, and thus not be disposed of properly when the scope is destroyed.

The implementation of UserService is essentially an an HTTP Client, that will make requests with an apiKey and apiSecret of the IdentityUser. I'd like it to exist for the duration of the scope (In the case of asp.net core, the request, or in the case of being called from a Quartz job, the duration of the job), and then dispose afterwards.

UserService contains about 20 various methods, such as FetchAccountAsync, BuyItemAsync(itemId, quantity), SellItemAsync(itemId), which should make requests using the initialized httpclient. I'd like to avoid trying to initialize the apiKey/apiSecret in each method, because this will add a level of synchronization that I don't feel is needed. HttpClient is by default multithreaded, so my methods are fairly pain free:

Task BuyItemAsync(string itemId, int quantity)
{
    var res = await _httpClient.GetAsync($"{_baseUrl}/buy?itemId={itemId}&qty={quantity}");
    res.EnsureSuccessStatusCode();
}

How can I initialize my UserService with these apiKeys, apiSecrets, to be used in a scoped manner?

3 Answers

NOTE: Some of these details I've added based on your comments. Some of these may appear a little contrived because I don't know your full logic, context, or needs.

Design

I suggest the following

  • Remove the factory.
  • Go ahead and use builder.Services.AddScoped<IUserService, UserService>();.
  • Change the constructor of UserService to accept an ISecurityContext that will provide the API key and secret. This context will also be registered with AddScoped.
  • Have the UserService use the ISecurityContext at runtime, and remove any properties/parameters for API key and secret (if you had them).
  • Have the SecurityService use an IUserProvider at runtime, and remove any properties/parameters for user ID (if you had them).

This means no runtime data needs to be injected, no hacky method to expose, no factory, and no injecting the service provider.

At startup register the interfaces. It is important that they be scoped. They are going to share the lifetime, which will be short.

...
builder.Services.AddScoped<IUserService, UserService>();
builder.Services.AddScoped<ISecurityContext, SecurityContext>();
builder.Services.AddScoped<IUserProvider, UserProvider>();
   

Then implement the classes and a Result that can return all the contextual data.

public class Result
{
    public Result(string apiKey, string apiSecret, Guid userId)
    {
        ApiKey = apiKey;
        ApiSecret = apiSecret;
        UserId = userId;
    }

    public string ApiKey { get; }
    public string ApiSecret { get; }
    public Guid UserId { get; }
}

public interface IUserProvider
{
    Guid GetUserId();
}

public class UserProvider : IUserProvider
{
    public async Task<Guid> GetUserId() => IdentityUser.GetUserId());
}

public interface ISecurityContext
{
    Task<Result> GetApiKeyAndSecretAsync();
}

public class SecurityContext : ISecurityContext
{
    private readonly DbContext _db;
    private readonly IUserProvider userProvider;

    // safe because this SecurityContext will be scoped!
    private Result _result;

    public SecurityContext(DbContext db, IUserProvider userProvider)
    {
        _db = db;
        _userProvider = userProvider;
    }

    public async Task<Result> GetApiKeyAndSecretAsync()
    {
        if (_result != null) return _result;
        
        var userId = _userProvider.GetUsedId();
        var (apiKey, apiSecret) = await _db.FetchApiKeyAndSecretAsync(userId);
        return _result = new Result(apiKey, apiSecret, userId);
    }
}

public interface IUserService
{
    Task DoWhatever();
}

public class UserService : IUserService
{
    private readonly ISecurityContext _context;

    public UserService(ISecurityContext context) => _context = context;

    public async Task DoWhatever()
    {
        // use the security context
        var result = await _context.GetApiKeyAndSecretAsync();

        // use the result; e.g. pass the key/secret/user ID
        // on to an HttpClient, RestClient, etc.
        ...
    }
    ...
}

Usage

Using an IUserService means injecting that into your Quartz.NET job, a message handler, a web controller... wherever. In each case you may realize that one single implementation of any of these interfaces is not enough. That's OK. There are ways in dependency injection to fix that (e.g. named resolutions of multiple different concrete implementations), but I leave that to you.

Here's an example usage for a web controller.

public class MyController
{
    private readonly IUserService _userService;
    public MyController(IUserService userService, ...)
    { 
        _userService = userService;
        ... 
    }

    [HttpGet]
    public async Task<IActionResult> GetStuff(...)
    {
        // gets the key and secret first time
        await _userService.DoWhatever();

        // uses cached versions of key, secret, guid across 
        // calls of _userService methods within scope
        var someResult = await _userService.GetSomethingElse();

        ...
    }

Commentary

This design has a few advantages

  • Security details are encapsulated behind an abstraction and not mixed into the UserService
  • The whole thing is more testable because the security details can be mocked when testing the UserService.
  • Key and secret are cached once within the scope and can be reused across methods in UserService that are invoked while in the same scope.

As @NKosi said in the comments, mixing runtime data at construction time is an anti-pattern. The link they referenced, Dependency Injection Code Smell: Injecting runtime data into components, is a good read and goes into more depth.

As you add more runtime data, you can expand the properties in Result and logic in SecurityContext or you can inject more context-like objects into UserService returning their own result-like instances.

There is a placeholder pattern that I have found useful here.

STARTUP CODE

Define dependencies in your application startup code, something like the following. Note that .NET does not allow you to run async processing in the factory method for IUserService:

app.UseMiddleware<DependencySetupMiddleware>();
services.AddSingleton(new MyDatabase());
services.AddScoped<UserServiceHolder>();
services.AddScoped<IUserService>(ctx =>
{
    return ctx.GetRequiredService<UserServiceHolder>().UserService;
});

The holder class just looks like this:

public class UserServiceHolder {
    public IUserService UserService { get; set; }
}

MIDDLEWARE CODE

The async processing can be done in a small middleware class. For the HTTP case you would do it like this, assuming that you get the User Id after authentication. Note that dependencies cannot be added to the .NET container at runtime, but you can update the holder object:

public class DependencySetupMiddleware

  public DependencySetupMiddleware(RequestDelegate next) {
  }

  public async Task Invoke(HttpContext context, MyDatabase db) {

    var userId = context.User.Claims.First(c => c.Type == "UserId")
    var (apiKey, apiSecret) = await db.FetchApiKeyAndSecretAsync(userId);

    var userService = new UserService(apiKey, apiSecret)
    context.GetRequiredService<UserServiceHolder>().UserService = userService;

    await next();
  }
}

For Quartz you would have a similar middleware class - a Job Factory, which reads the job's user ID rather than using claims or the HTTP context.

BUSINESS LOGIC

With this code in place you can inject an IUserService into your business logic and forget about the holder class:

class MyController {

   public MyController(IUserService userService) {
   }
}

I think you might already have an answer here, but let me give you a working example. Here's my assumption:

I want to have an instance of a class that has all the things about the user available.

Here's the approach I used for PopForums.

Step 1: You're using some kind of built-in ASP.NET authentication, probably cookies or something external. I won't cover that here, because there are many ways to do it, but look at HttpContext.SignInAsync() for more. The important part is to use a name or identifier that will be put into the token it reads back in the next step.

Step 2: Use middleware to get your user and make it stick. You'll start with a ClaimsIdentity when you use HttpContext.AuthenticateAsync(schemeName). For example:

    public async Task InvokeAsync(HttpContext context, IUserService userService)
    {
        var authResult = await context.AuthenticateAsync(schemeNameUsedFromSignIn);
        var identity = authResult?.Principal?.Identity as ClaimsIdentity;
        if (identity != null)
        {
            var user = userService.GetUserByName(identity.Name);
            if (user != null)
            {
                // add claims to the identity if you want
                // then stash your user object in the Items collection, which lasts the duration of the request
                context.Items["TheUser"] = user;
                context.User = new ClaimsPrincipal(identity);
            }
        }
        await _next.Invoke(context);

Step 3: Enable getting the user anywhere you want by pulling it out of the context of the request, but isolate it to an interface so there are no hard dependencies. Example:

public interface IUserRetrievalShim
{
    User GetUser();
}

public class UserRetrievalShim : IUserRetrievalShim
{
    private readonly IHttpContextAccessor _httpContextAccessor;

    public UserRetrievalShim(IHttpContextAccessor httpContextAccessor)
    {
        _httpContextAccessor = httpContextAccessor;
    }

    public User GetUser()
    {
        var user = _httpContextAccessor.HttpContext?.Items["TheUser"] as User;
        return user;
    }
}

What I like about this approach is that any classes up and down the dependency chain can be mocked out and unit tested without all of the HttpContext references. IHttpContextAccessor does a great job isolating it, and if it's not available, you'll get a null. And in this case, you're getting your user object, not the one tied to ASP.NET. You can still check HttpContext.User != null if you want, but this similarly will be null if there's no authenticated user. I only do the above with claims because maybe other app areas may want it.

Step 4: In your controllers, service classes or anything in between, inject IUserRetrievalShim and call its GetUser() method to get the user.

The bottom line here is that dependency injection is not the place to make the user stuff contextual. DI is purely setup and configuration, not run-time context. Use your UserService where ever you want, and combined with this shim, you can pass its ID or whatever to those service methods. You should not expect the service to be contextual out of the box by way of injection.

With that said, your User objects (not to be confused with HttpContext.User) can be composed of whatever you want, so long as you're OK with whatever the cost is to fetch that information and hydrate the object.

Related