Switch to the user's tenant in AbpSignInManager.#24942
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds tenant context switching to sign-in operations in AbpSignInManager to ensure that tenant-specific settings and data filtering are correctly applied during authentication. The changes address a multi-tenancy bug where sign-in operations were not executing in the user's tenant context, which could cause incorrect behavior when users from different tenants authenticate.
Changes:
- Injected
ICurrentTenantdependency to enable tenant context switching - Wrapped sign-in operations (UpdateUserAsync, SignInOrTwoFactorAsync, PasswordSignInAsync) with
CurrentTenant.Change(user.TenantId)to ensure they execute in the correct tenant context - Refactored
_identityUserManagerfrom private field to protectedIdentityUserManagerproperty for better extensibility
Comments suppressed due to low confidence (2)
modules/identity/src/Volo.Abp.Identity.AspNetCore/Volo/Abp/Identity/AspNetCore/AbpSignInManager.cs:119
- Missing tenant context switch for ExternalLoginSignInAsync. The PreSignInCheck and SignInOrTwoFactorAsync calls access tenant-specific settings (e.g., password change period settings via SettingProvider in ShouldPeriodicallyChangePasswordAsync). These operations should execute within the user's tenant context to ensure correct settings and data filtering are applied. Wrap lines 113-118 with
using (CurrentTenant.Change(user.TenantId)) { ... }similar to the pattern used in PasswordSignInAsync at lines 99-102.
public override async Task<SignInResult> ExternalLoginSignInAsync(string loginProvider, string providerKey, bool isPersistent, bool bypassTwoFactor)
{
var user = await FindByLoginAsync(loginProvider, providerKey);
if (user == null)
{
return SignInResult.Failed;
}
var error = await PreSignInCheck(user);
if (error != null)
{
return error;
}
return await SignInOrTwoFactorAsync(user, isPersistent, loginProvider, bypassTwoFactor);
}
modules/identity/src/Volo.Abp.Identity.AspNetCore/Volo/Abp/Identity/AspNetCore/AbpSignInManager.cs:103
- Missing test coverage for multi-tenant sign-in scenarios. The changes introduce tenant context switching for sign-in operations, but there are no tests verifying that sign-in works correctly when users belong to different tenants. Consider adding tests that verify: 1) Password sign-in with tenant users and tenant-specific settings, 2) External login sign-in with tenant users, 3) ExternalLoginSignInAsync with tenant users. This is especially important given the critical bug fix for ExternalLoginSignInAsync missing tenant context.
public override async Task<SignInResult> PasswordSignInAsync(
string userName,
string password,
bool isPersistent,
bool lockoutOnFailure)
{
IdentityUser user;
foreach (var externalLoginProviderInfo in AbpOptions.ExternalLoginProviders.Values)
{
var externalLoginProvider = (IExternalLoginProvider)Context.RequestServices
.GetRequiredService(externalLoginProviderInfo.Type);
if (await externalLoginProvider.TryAuthenticateAsync(userName, password))
{
user = await FindByNameAsync(userName);
if (user == null)
{
if (externalLoginProvider is IExternalLoginProviderWithPassword externalLoginProviderWithPassword)
{
user = await externalLoginProviderWithPassword.CreateUserAsync(userName, externalLoginProviderInfo.Name, password);
}
else
{
user = await externalLoginProvider.CreateUserAsync(userName, externalLoginProviderInfo.Name);
}
}
else
{
using (CurrentTenant.Change(user.TenantId))
{
if (externalLoginProvider is IExternalLoginProviderWithPassword externalLoginProviderWithPassword)
{
await externalLoginProviderWithPassword.UpdateUserAsync(user, externalLoginProviderInfo.Name, password);
}
else
{
await externalLoginProvider.UpdateUserAsync(user, externalLoginProviderInfo.Name);
}
}
}
using (CurrentTenant.Change(user.TenantId))
{
return await SignInOrTwoFactorAsync(user, isPersistent);
}
}
}
user = await FindByNameAsync(userName);
if (user == null)
{
return SignInResult.Failed;
}
using (CurrentTenant.Change(user.TenantId))
{
return await PasswordSignInAsync(user, password, isPersistent, lockoutOnFailure);
}
}
EngincanV
approved these changes
Feb 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.