Support shared mode lookup by id for two-factor authentication#25304
Merged
Support shared mode lookup by id for two-factor authentication#25304
Conversation
- Add IdentityUserManager.FindSharedUserByIdAsync to resolve a user by id across tenants in shared user sharing strategy - Override AbpSignInManager.GetTwoFactorAuthenticationUserAsync to use it so the 2FA mid-flow can still find a tenant-scoped user when CurrentTenant is host - Cover the new method with unit tests
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a 2FA mid-flow failure in Shared tenant user sharing strategy by ensuring user lookup by ID can bypass the default IMultiTenant filter when CurrentTenant is null, aligning the 2FA user resolution with existing FindSharedUserBy* patterns.
Changes:
- Added
IdentityUserManager.FindSharedUserByIdAsync(string userId)for Shared-mode cross-tenant user lookup by ID. - Overrode
AbpSignInManager.GetTwoFactorAuthenticationUserAsync()to useFindSharedUserByIdAsyncinstead of the defaultFindByIdAsyncpath. - Added domain tests covering Shared-mode host/tenant context lookups for the new
FindSharedUserByIdAsyncmethod.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/IdentityUserManager.cs | Adds Shared-mode-safe user lookup by ID (disables IMultiTenant filter and re-resolves under the user’s tenant). |
| modules/identity/src/Volo.Abp.Identity.AspNetCore/Volo/Abp/Identity/AspNetCore/AbpSignInManager.cs | Routes 2FA user resolution through the new Shared-mode lookup to prevent null results when CurrentTenant is null. |
| modules/identity/test/Volo.Abp.Identity.Domain.Tests/Volo/Abp/Identity/IdentityUserManager_Tests.cs | Adds unit tests validating the new Shared-mode ID lookup behavior across host/tenant contexts. |
…d mode Exercises the full cookie round-trip: writes a TwoFactorUserId cookie carrying a tenant user id, then verifies that AbpSignInManager.GetTwoFactorAuthenticationUserAsync returns the tenant user when CurrentTenant is null.
Guards against regressing the data-access contract behind the 2FA redirect bug: login must find a tenant user by user name from a host context, and the 2FA mid-flow must then resolve the same tenant user by id from the same host context.
- Override IdentityUserManager.FindByIdAsync to fall back to a cross-tenant lookup in shared user sharing strategy so any caller that hits FindByIdAsync from a non-matching tenant context (including base SignInManager internals for TwoFactorSignInAsync and TwoFactorRecoveryCodeSignInAsync) can still resolve a tenant user by id - Drop the now-redundant AbpSignInManager.GetTwoFactorAuthenticationUserAsync override; the base implementation works automatically through the new FindByIdAsync behavior - Cover the new FindByIdAsync behavior with unit tests
Switching CurrentTenant to user.TenantId in PasswordSignInAsync without refreshing IdentityOptions meant that lockout, password policy and other tenant-scoped options used host values during the base sign-in call. Call IdentityOptions.SetAsync inside the tenant switch so downstream checks use the user's tenant configuration.
…Abp/Identity/AspNetCore/Isolated_TwoFactor_Tests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…Abp/Identity/AspNetCore/Shared_SignIn_Tests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…Abp/Identity/AspNetCore/GetTwoFactorAuthenticationUser_Tests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
ismcagdas
approved these changes
Apr 24, 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.
Fix shared-mode 2FA: AbpSignInManager now resolves the user via the new FindSharedUserByIdAsync and switches CurrentTenant + IdentityOptions before calling base sign-in methods, so tenant users can complete 2FA from host context. Covers TwoFactorSignInAsync, TwoFactorRecoveryCodeSignInAsync, PasswordSignInAsync and ExternalLoginSignInAsync.
Adds AspNetCore integration tests for each path, revert-verified. Isolated mode unchanged.