MultiTenant Auth breaks Finbuckle
Hi, I've created a test repo just using InMemory data stores.
https://github.com/JohnCampionJr/finbuckle-test
Trying to do a proof of concept for Finbuckle and I am missing something I think.
Issue 1: When logged out (no cookies), the tenant pages work as expected:
https://localhost:7017/acme
https://localhost:7017/initech
But going to the root, take me to /notenant as specified in Program.cs but it causes a redirect loop because that's not a tenant.
builder.Services.AddMultiTenant<TenantInfo>()
.WithBasePathStrategy(options => options.RebaseAspNetCorePathBase = true)
.ShortCircuitWhenTenantNotResolved(new Uri("/notenant", UriKind.Relative))
.WithEFCoreStore<EFCoreStoreDbContext<TenantInfo>, TenantInfo>()
.WithPerTenantAuthentication();
Issue 2: After registering (which works fine), and logging in on one of the tenants, it no longer works. The PerTenantDb no longer gets a TenantInfo so the whole thing stops.
Can I trouble you for a little help? What am I missing?
Hi, I think I can help. I will be able to check out the repo tomorrow. I appreciate the sponsorship as well.
Ok on the 1st issue:
You are adding [ExcludeFromMultiTenantResolution] on the model class and method. Instead add it on the razor page for NoTenant.cshtml below @page:
@using Finbuckle.MultiTenant.AspNetCore.Routing
@attribute [ExcludeFromMultiTenantResolution]
Then understand that the MultiTenant middleware will look for this in order to prevent the never-ending redirects, so you need to have UseRouting before UseMultiTenant for it to work otherwise the endpoint metadata isn't populated.
Then the static assets will get confused so remove WithStaticAssets and add the UseStaticFiles middleware before UseRouting. This is something I'll need to look into supporting.
Will report back with details on the 2nd issue. Let me know if this helps. Also one of your layout files is missing an import statement for TenantInfo namespace.
@AndrewTriesToCode That fixes the NoTenant, but it breaks the basepath strategy. The tenant routes no longer work:
/acme, /initech, /mega all generate 404 errors.
Well that’s no good! I have some more time today to dig in deeper.
Sorry to be a pest, was wondering if you'd had any luck? Once it's working I'd be happy to put it into a PR to be added to the samples for .NET 9.0
Yeah what I'm seeing is that we have fundamental issue with ExcludeFromMultiTenantResolution which relies on routing having already happened and the BasePath strategy which typically executes before routing (this fixes the issue with the tenant pages not working). I will have to think about that one.
BasePath strategy also has complications with static assets. The default template references css and javascript with ~/... which means they Wille include the tenant name since it is part of the path base. I recommend changing the CSS and JS paths not to use ~. Another option is to put UseStaticFiles after UseMultiTenant in which case the default template paths with ~ will work but it will look like the assets are under the tenant path, e.g. acme/css/site/css.
I usually recommend the route parameter strategy won't have these issues.
Regarding 2: When you don't have an explicit call to UseAuthentication an implicit one is placed at the beginning of the pipeline (similar to UseRouting). During the authentication it is creating the scoped db context in order to validate the cookie (aka security stamp validation, something that Identity sets up). It doesn't do that every request but the initial and then based on a timer. This is failing because at this point in the app pipeline the tenant is not known so the db context is initialized with no tenant id. So the authentication reports not authenticated, then the Index page handler gets the same db context via injection but of course it's ToDo query fails because the context has no tenant id set.
Simple fix, after UseMultiTenant add UseAuthentication. Then the authentication security stamp validation will create a properly initialized scoped db context with the correct tenant, and the page handler will use it as well. Does that make sense?
Thank you so much! I'll give that a try!
Sorry for delay. The adding UseAuthentication after UseMultiTenant solved the authentication. Still not able to get Exclude working. Tried changing to Route Strategy, but no luck. It seems the order for these two are mutually exclusive.
I’ll have to make exclude a little smarter. It was recently added and assumes endpoints are in play which is not always the case. It probably needs to be a delegate that accepts the context object and returns a true or false. Then each strategy can invoke it as desired. Or maybe it’s an option on some strategies and they each handle it as they see fit. There is already an option to ignore certain potential identifiers. I’ll have to think on this a bit.
OK, thanks. Not mission critical for me, but I'm glad to at least have the auth working :)