Finbuckle / Finbuckle.MultiTenant

Finbuckle.MultiTenant is an open-source multitenancy middleware library for .NET. It enables tenant resolution, per-tenant app behavior, and per-tenant data isolation.
https://www.finbuckle.com/multitenant
Apache License 2.0
1.31k stars 266 forks source link

BasePathStrategy is not respecting the strategy chain order when setted with WithBasePathStrategy builder method #877

Open DiogoLuisBarros opened 1 week ago

DiogoLuisBarros commented 1 week ago

Steps to reproduce

  1. Add one non-HttpContext dependant strategy
  2. Register the BasePathStrategy with the WithBasePathStrategy builder method
  3. Run your code in a way that a tenant resolution is invoked
  4. You get a MultiTenantException with the following message: "BasePathStrategy expects HttpContext."

My code:

var multitenantBuilder = services
    .AddMultiTenant<TenantExpandedInfo>()
    .WithStrategy<ForcedTenantStrategy>(ServiceLifetime.Singleton)
    .WithBasePathStrategy()
    .WithEFCoreStore<TenantDbContext, TenantExpandedInfo>()
    .WithPerTenantAuthentication();

Calling WithBasePathStrategy will not respect the chain of strategies because there are some checks regarding HttpContext that are being done, even if the previous strategy is resolved (in this case it is my custom ForcedTenantStrategy).

Please, check the following code: https://github.com/Finbuckle/Finbuckle.MultiTenant/blob/main/src/Finbuckle.MultiTenant.AspNetCore/Extensions/MultiTenantBuilderExtensions.cs#L224C35-L224C106

Expected behaviour

These checks that are made because of the BasePathStrategy register process (during the OnTenantResolved delegate registration), but should should only be made if the strategy type is BasePathStrategy

DiogoLuisBarros commented 1 week ago

Proposed fix:

public static MultiTenantBuilder<TTenantInfo> WithBasePathStrategy<TTenantInfo>(
    this MultiTenantBuilder<TTenantInfo> builder, Action<BasePathStrategyOptions> configureOptions)
    where TTenantInfo : class, ITenantInfo, new()
{
    builder.Services.Configure(configureOptions);
    builder.Services.Configure<MultiTenantOptions>(options =>
    {
        var origOnTenantResolved = options.Events.OnTenantResolved;
        options.Events.OnTenantResolved = tenantResolvedContext =>
        {
            if (tenantResolvedContext.StrategyType == typeof(BasePathStrategy))
            {
                var httpContext = tenantResolvedContext.Context as HttpContext ??
                              throw new MultiTenantException("BasePathStrategy expects HttpContext.");

                if (httpContext.RequestServices.GetRequiredService<IOptions<BasePathStrategyOptions>>().Value
                        .RebaseAspNetCorePathBase)
                {
                    httpContext.Request.Path.StartsWithSegments($"/{tenantResolvedContext.TenantInfo?.Identifier}",
                        out var matched, out var
                            newPath);
                    httpContext.Request.PathBase = httpContext.Request.PathBase.Add(matched);
                    httpContext.Request.Path = newPath;
                }
            }

            return origOnTenantResolved(tenantResolvedContext);
        };
    });

    return builder.WithStrategy<BasePathStrategy>(ServiceLifetime.Singleton);
}

It worked for me and seems perfectly fine!