Finbuckle.MultiTenant icon indicating copy to clipboard operation
Finbuckle.MultiTenant copied to clipboard

ApplicationDbContext can't be used in WithEFCoreStore (TEFCoreStoreDbContext error)

Open spech66 opened this issue 3 years ago • 10 comments

Hi there, first thank you for the great project. I'm using .NET 5 and try to combine WithEFCoreStore with WithClaimStrategy. Having two DB contexts only the first one has Tenant information.

public class MultiTenantStoreDbContext : EFCoreStoreDbContext<TenantInfo>
public class ApplicationDbContext : IdentityDbContext, IMultiTenantDbContext

            services.AddMultiTenant<TenantInfo>()
                .WithEFCoreStore<MultiTenantStoreDbContext, TenantInfo>()
                .WithClaimStrategy()
                .WithPerTenantAuthentication();

However this also requires multiple migration folders and you can't use transactions in register method when Tenant, User and Claim should be created in a safe way. I tried to combine this in in one context but this results in an type error:

public class ApplicationDbContext : IdentityDbContext, IMultiTenantDbContext
or (both produce the same error)
public class ApplicationDbContext : MultiTenantIdentityDbContext

            services.AddMultiTenant<TenantInfo>()
                .WithEFCoreStore<ApplicationDbContext , TenantInfo>()
                .WithClaimStrategy()
                .WithPerTenantAuthentication();
Error
CS0311
The type 'ApplicationDbContext' cannot be used as type parameter
'TEFCoreStoreDbContext' in the generic type or method
'FinbuckleMultiTenantBuilderExtensions.WithEFCoreStore<TEFCoreStoreDbContext, TTenantInfo>(FinbuckleMultiTenantBuilder<TTenantInfo>)'.
There is no implicit reference conversion from 'ApplicationDbContext' to
'Finbuckle.MultiTenant.Stores.EFCoreStoreDbContext<Finbuckle.MultiTenant.TenantInfo>'

Shouldn't TEFCoreStoreDbContext be IMultiTenantDbContext?

spech66 avatar Mar 14 '21 08:03 spech66

Hello. I saw you closed the issue-did you figure it out?

it is possible to share the context but the EFCoreStore entities usually should not be multitenant. It doesn’t normally make sense that each tenant needs its own per-tenant multitenant store. Does that make sense?

AndrewTriesToCode avatar Mar 14 '21 16:03 AndrewTriesToCode

Hi @AndrewTriesToCode thank you for the reply. I was searching in the wrong direction with that error. However right now I have three DB Contexts as the Identity System caused trouble with a Tenant bound Context. The user can now visit the page without login. Identity would use the normal ApplicationDbContext. If the user is logged in (cookie + claims) the tenant is set in the TenantDBContext which is now the one I use in data related Views. might be the same problem from https://github.com/Finbuckle/Finbuckle.MultiTenant/issues/379

spech66 avatar Mar 14 '21 16:03 spech66

I'm not sure I understand exactly -- feel free to reopen this with more detail or open a new issue. So you have 3 db contexts?

  1. EFCoreStore
  2. ApplicationDbContext - not multiTenant for Identiy
  3. TenantDbContext - multitneant for Identity

is that right?

AndrewTriesToCode avatar Mar 14 '21 17:03 AndrewTriesToCode

Yes. I have a Razor Pages project and wanted to add Finbuckle.MultiTenant. The content is like this:

  • Have a few static landing pages
  • Allow registration of new Tenant together with the first user. Register will create the Tenant, the user and add a tenant claim.

If I only have the ApplicationDbContext derived from MultiTenantIdentityDbContext the application will throw Null Exceptions on all Identity related methods. For example the _signInManager.PasswordSignInAsync is failing.

Working:

ApplicationDbContext : IdentityDbContext
TenantDbContext: MultiTenantIdentityDbContext
MultiTenantStoreDbContext : EFCoreStoreDbContext<TenantInfo>

Exception below:

ApplicationDbContext : MultiTenantIdentityDbContext
MultiTenantStoreDbContext : EFCoreStoreDbContext<TenantInfo>

spech66 avatar Mar 14 '21 17:03 spech66

Just switch the ApplicationDbContext to MultiTenantIdentityDbContext to get you a stack trace:

An unhandled exception occurred while processing the request.
NullReferenceException: Object reference not set to an instance of an object.
lambda_method14(Closure , QueryContext )

MultiTenantException: Exception in Finbuckle.MultiTenant.Strategies.ClaimStrategy.GetIdentifierAsync.
Finbuckle.MultiTenant.Strategies.MultiTenantStrategyWrapper.GetIdentifierAsync(object context)

NullReferenceException: Object reference not set to an instance of an object.
lambda_method14(Closure , QueryContext )
Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.ExecuteAsync<TResult>(Expression query, CancellationToken cancellationToken)
Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryProvider.ExecuteAsync<TResult>(Expression expression, CancellationToken cancellationToken)
Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.ExecuteAsync<TSource, TResult>(MethodInfo operatorMethodInfo, IQueryable<TSource> source, Expression expression, CancellationToken cancellationToken)
Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.ExecuteAsync<TSource, TResult>(MethodInfo operatorMethodInfo, IQueryable<TSource> source, LambdaExpression expression, CancellationToken cancellationToken)
Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.FirstOrDefaultAsync<TSource>(IQueryable<TSource> source, Expression<Func<TSource, bool>> predicate, CancellationToken cancellationToken)
Microsoft.EntityFrameworkCore.Internal.EntityFinder<TEntity>.FindAsync(object[] keyValues, CancellationToken cancellationToken)
Microsoft.EntityFrameworkCore.Internal.InternalDbSet<TEntity>.FindAsync(object[] keyValues, CancellationToken cancellationToken)
Microsoft.AspNetCore.Identity.EntityFrameworkCore.UserOnlyStore<TUser, TContext, TKey, TUserClaim, TUserLogin, TUserToken>.FindByIdAsync(string userId, CancellationToken cancellationToken)
Microsoft.AspNetCore.Identity.UserManager<TUser>.FindByIdAsync(string userId)
Microsoft.AspNetCore.Identity.UserManager<TUser>.GetUserAsync(ClaimsPrincipal principal)
Microsoft.AspNetCore.Identity.SignInManager<TUser>.ValidateSecurityStampAsync(ClaimsPrincipal principal)
Microsoft.AspNetCore.Identity.SecurityStampValidator<TUser>.ValidateAsync(CookieValidatePrincipalContext context)
Microsoft.Extensions.DependencyInjection.FinbuckleMultiTenantBuilderExtensions+<>c__DisplayClass2_1<TTenantInfo>+<<WithPerTenantAuthenticationConventions>b__4>d.MoveNext()
Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationHandler.HandleAuthenticateAsync()
Microsoft.AspNetCore.Authentication.AuthenticationHandler<TOptions>.AuthenticateAsync()
Finbuckle.MultiTenant.Strategies.ClaimStrategy.GetIdentifierAsync(object context)
Finbuckle.MultiTenant.Strategies.MultiTenantStrategyWrapper.GetIdentifierAsync(object context)

MultiTenantException: Exception in Finbuckle.MultiTenant.Strategies.ClaimStrategy.GetIdentifierAsync.
Finbuckle.MultiTenant.Strategies.MultiTenantStrategyWrapper.GetIdentifierAsync(object context)
Finbuckle.MultiTenant.TenantResolver<T>.ResolveAsync(object context)
Finbuckle.MultiTenant.TenantResolver<T>.Finbuckle.MultiTenant.ITenantResolver.ResolveAsync(object context)
Finbuckle.MultiTenant.AspNetCore.MultiTenantMiddleware.Invoke(HttpContext context)
Microsoft.AspNetCore.Diagnostics.EntityFrameworkCore.MigrationsEndPointMiddleware.Invoke(HttpContext context)
Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context)

spech66 avatar Mar 14 '21 17:03 spech66

Thanks for the detail. I didn't anticipate that the security stamp validation would call into the db context but this makes sense.

The fix is to move line 76 on the snippet below down below the check for validation bypass. This is safe to do since this call to authenticate is not used to set the actual user -- just to check the tenant claim after which the results are discarded. The actual signin middleware will do the real validation and reject the user if the security stamp is invalid. The tenant would still be set though so be aware of that.

I'll include the fix in the next release. Thanks for you help.

https://github.com/Finbuckle/Finbuckle.MultiTenant/blob/baa26127ca5813c11d270369e24ccf2aea379bf2/src/Finbuckle.MultiTenant.AspNetCore/Extensions/MultiTenantBuilderExtensions.cs#L73-L80

AndrewTriesToCode avatar Mar 14 '21 18:03 AndrewTriesToCode

Thank you for the super fast replies. What do you mean by " The tenant would still be set though so be aware of that."?

I will have a watch for the next release to hit that donation button ;-)

spech66 avatar Mar 14 '21 18:03 spech66

It means the application will set the tenant based on the claim in the cookie even if Identity later rejects the cookie during the authentication middleware. In practice this means the request will most likely get redirected to the login page immediately (if the endpoint requires authorization).

I'm working to set up a private Nuget feed for sponsors to get updated versions out MUCH faster while still making available the regular public Nuget feed with cumulative updates every 3-4 months. Is that something that you think potential sponsors would like to see?


As a temp workaround you could potentially wrap the AspNet Identity validation with the same check by using ConfigureApplcicationCookie with a check similar to this before calling the preexisting Identity validation:

// Constants.TenantToken is internal but the value you can use for now is __tenant__
if(context.HttpContext.Items.Keys.Contains($"{Constants.TenantToken}__bypass_validate_principle__"))
    return;

AndrewTriesToCode avatar Mar 14 '21 18:03 AndrewTriesToCode

That sounds like a good behavior. But pages without authentication would just have the tenant set to null right?

For this project: It's one of the kind you build in your spare time to hope to release it one day ;-) In my professional life I'm working with node/react mostly atm however we use a lot of dependencies. For some of those I would really like an option to pay some money to get some problems fixed or at least PRs merged in time.

spech66 avatar Mar 14 '21 18:03 spech66

Pages without authentication, in this case, would have the tenant set to whatever the claim was.

AndrewTriesToCode avatar Mar 14 '21 18:03 AndrewTriesToCode

@AndrewTriesToCode I have the same error, how can I fix it?

HDScarpe avatar Oct 18 '22 10:10 HDScarpe

@HDScarpe Can you please open a new issue and provide a little more detail? What does your AddMultiTenant setup look like and how are your db context classes defined?

AndrewTriesToCode avatar Oct 18 '22 12:10 AndrewTriesToCode

As the original issue was resolved in a prior release I'm going to close this item. Feel free to open new issues if needed.

AndrewTriesToCode avatar Oct 18 '22 12:10 AndrewTriesToCode

@AndrewTriesToCode I created a new issue. Thanks for answering.

HDScarpe avatar Oct 20 '22 04:10 HDScarpe