myquay / MultiTenant.AspNetCore

Multi-tenancy support for ASP.NET Core 8
MIT License
20 stars 5 forks source link

Ability to choose middleware entry point #1

Closed brettwinters closed 6 months ago

brettwinters commented 6 months ago

Hello

Thanks for the great library!

I noticed that your library inserts the tenant middleware at the start of the request pipeline through the IStartupFilter

In my case I can only resolve the tenant after model binding (my strategies (which is another feature I'd like to request later) uses the request payload to determine the tenant).

So would it be possible to allow this:

app.UseModelBinding();
app.UseMultiTenant(); //<--
app.UseSomeMiddlewareThanIsTenantAware();
myquay commented 6 months ago

Hi,

Currently that's not possible, however I think it is a good idea. I will be useful to have the ability to control where in the pipeline it creates the tenant context.

I will give this some thought; perhaps the ability to pass in an option to instruct the library to not auto-register using IStartupFilter which then allows an app.UserMultiTenant();?

In terms of current capability do any of these options work in your scenario.

Use a header to specify tenant

Move the data that's currently in the request body, to a custom header such as x-tenant-id: XXXXXXXXX

Then implement a custom strategy to use the header.

public class HeaderResolutionStrategy(IHttpContextAccessor httpContextAccessor) : ITenantResolutionStrategy
{
    private readonly IHttpContextAccessor _httpContextAccessor = httpContextAccessor;

    public async Task<string> GetTenantIdentifierAsync()
    {
        if (_httpContextAccessor.HttpContext == null)
            throw new InvalidOperationException("HttpContext is not available");

        //Logic using _httpContextAccessor.HttpContext.Request?.Headers
        //to return the value from x-tenant-id
    }
}

And register the header during startup using .WithResolutionStrategy<HeaderResolutionStrategy>()

Use the request body directly

The only reason I mention using a header is that by default the request body is a read-once stream, so it won't be available to the binder if you access it early to extract that data you need during tenant identification.

If there's no other way then you can allow the stream to be read multiple times, the first time for you to get out the tenant specific data before binding, and the second time during actual binding. There are a few considerations: https://weblog.west-wind.com/posts/2024/Feb/20/Reading-Raw-ASPNET-RequestBody-Multiple-Times

Let me know if that helps you out.

brettwinters commented 6 months ago

Hi @myquay

Yeah to prevent breaking changes then having a feature switch is the best option.

I'm using the header and also subdomain strategies on other microservices but for this particular case it's a contract /document versioning system with multiple parties and the 'tenant' is actually the principal party to the agreement which depends on their status and where they want the document stored. I wish I could simplify it but I can't haha

But actually it's really useful to be able to pick the point in the pipeline where the app needs to be tenant-aware. If you went with a app.UseMultiTenant() option then would it also mean that you could only store the tenant-aware services in the _pipelinesCache, reducing the memory footprint? or maybe I haven't thought it through...

The model binding in my case puts the model into the context and then the authorisation, validation, command and query handlers read from this context - they don't need to stream the body again. It's not an MVC app - more like the minimal API.

All the best

Brett

myquay commented 6 months ago

Hi @brettwinters ,

Thanks for the additional context. I agree that would be useful.

I've added the ability to disable the automatic registration and choose the entry point for multi-tenancy.

If automatic registration is disabled, then the tenant specific services or context won't be available until the middleware registered with app.UseMultiTenancy<TestTenant>(); is run.

Let me know if that helps with your use case.

brettwinters commented 6 months ago

Thanks a lot @myquay - I'll give it a go and let you know