Make 2FA flows tenant-aware and enforce pre-checks

modules/identity/src/Volo.Abp.Identity.AspNetCore/Volo/Abp/Identity/AspNetCore/AbpSignInManager.cs

@@ -1,4 +1,5 @@
-using System.Threading.Tasks;
+using System.Security.Claims;
+using System.Threading.Tasks;
 using Microsoft.AspNetCore.Authentication;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Identity;
@@ -133,6 +134,61 @@ public class AbpSignInManager : SignInManager<IdentityUser>
         return await IdentityUserManager.FindSharedUserByLoginAsync(loginProvider, providerKey);
     }
 
+    public override async Task<IdentityUser> GetTwoFactorAuthenticationUserAsync()
+    {
+        var result = await Context.AuthenticateAsync(IdentityConstants.TwoFactorUserIdScheme);
+        if (result?.Principal == null)
+        {
+            return null;
+        }
+
+        var userId = result.Principal.FindFirstValue(ClaimTypes.Name);
+        if (string.IsNullOrWhiteSpace(userId))
+        {
+            return null;
+        }
+
+        return await IdentityUserManager.FindSharedUserByIdAsync(userId);
+    }
+
+    public override async Task<SignInResult> TwoFactorSignInAsync(string provider, string code, bool isPersistent, bool rememberClient)
+    {
+        var user = await GetTwoFactorAuthenticationUserAsync();
+        if (user == null)
+        {
+            return SignInResult.Failed;
+        }
+
+        using (CurrentTenant.Change(user.TenantId))
+        {
+            return await base.TwoFactorSignInAsync(provider, code, isPersistent, rememberClient);
+        }
+    }
+
+    public override async Task<SignInResult> TwoFactorRecoveryCodeSignInAsync(string recoveryCode)
+    {
+        var user = await GetTwoFactorAuthenticationUserAsync();
+        if (user == null)
+        {
+            return SignInResult.Failed;
+        }
+
+        using (CurrentTenant.Change(user.TenantId))
+        {
+            // Base TwoFactorRecoveryCodeSignInAsync does not invoke PreSignInCheck, which means
+            // AbpSignInManager's IsActive / ShouldChangePassword checks would be bypassed. Run the
+            // same pre-sign-in checks here so recovery-code sign-in has the same gating as the
+            // regular two-factor sign-in path.
+            var preSignInCheckResult = await PreSignInCheck(user);
+            if (preSignInCheckResult != null)
+            {
+                return preSignInCheckResult;
+            }
+
+            return await base.TwoFactorRecoveryCodeSignInAsync(recoveryCode);
+        }
+    }
+
     /// <summary>
     /// This is to call the protection method PreSignInCheck
     /// </summary>

modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/IdentityUserManager.cs

@@ -745,21 +745,4 @@ public class IdentityUserManager : UserManager<IdentityUser>, IDomainService
             }
         }
     }
-
-    public override async Task<IdentityUser> FindByIdAsync(string userId)
-    {
-        // In shared user sharing strategy, a user id is globally unique. Fall back to
-        // a cross-tenant lookup so callers that hit FindByIdAsync from a non-matching
-        // tenant context (notably the 2FA mid-flow invoked by ASP.NET Core Identity
-        // base SignInManager paths) can still resolve the user by id.
-        // Downstream operations on the returned entity should still be scoped to
-        // user.TenantId explicitly via CurrentTenant.Change.
-        var user = await base.FindByIdAsync(userId);
-        if (user != null || MultiTenancyOptions.Value.UserSharingStrategy == TenantUserSharingStrategy.Isolated)
-        {
-            return user;
-        }
-
-        return await FindSharedUserByIdAsync(userId);
-    }
 }

modules/identity/test/Volo.Abp.Identity.AspNetCore.Tests/Volo/Abp/Identity/AspNetCore/GetTwoFactorAuthenticationUser_Tests.cs

@@ -59,4 +59,87 @@ public class GetTwoFactorAuthenticationUser_Tests : SharedAbpIdentityAspNetCoreT
 
         content.ShouldBe("null");
     }
+
+    [Fact]
+    public async Task TwoFactorSignInAsync_Should_Resolve_Tenant_User_In_Shared_Mode()
+    {
+        // If the tenant switch inside AbpSignInManager.TwoFactorSignInAsync regresses,
+        // the base implementation would not find the user and return SignInResult.Failed.
+        // An inactive tenant user lets PreSignInCheck return NotAllowed, proving the user
+        // was actually located and inspected under its own tenant context.
+        var tenantUserId = await CreateInactiveTenantUserAsync("shared-2fa-signin", "shared-2fa-signin@abp.io");
+
+        await WriteTwoFactorCookieAsync(tenantUserId);
+
+        var response = await Client.GetAsync("/api/signin-test/two-factor-signin?provider=Email&code=invalid");
+        response.EnsureSuccessStatusCode();
+        var result = await response.Content.ReadAsStringAsync();
+
+        result.ShouldBe("NotAllowed");
+    }
+
+    [Fact]
+    public async Task TwoFactorRecoveryCodeSignInAsync_Should_Resolve_Tenant_User_In_Shared_Mode()
+    {
+        // AbpSignInManager.TwoFactorRecoveryCodeSignInAsync runs PreSignInCheck after locating
+        // the user under its own tenant context. An inactive tenant user therefore produces
+        // NotAllowed only when the override actually resolves the user; a regression would
+        // return Failed instead.
+        var tenantUserId = await CreateInactiveTenantUserAsync("shared-2fa-recovery", "shared-2fa-recovery@abp.io");
+
+        await WriteTwoFactorCookieAsync(tenantUserId);
+
+        var response = await Client.GetAsync("/api/signin-test/two-factor-recovery-signin?recoveryCode=invalid");
+        response.EnsureSuccessStatusCode();
+        var result = await response.Content.ReadAsStringAsync();
+
+        result.ShouldBe("NotAllowed");
+    }
+
+    private async Task<Guid> CreateInactiveTenantUserAsync(string userName, string email)
+    {
+        return await CreateTenantUserAsync(userName, email, isActive: false);
+    }
+
+    private async Task<Guid> CreateTenantUserAsync(string userName, string email, bool isActive)
+    {
+        var userRepository = GetRequiredService<IIdentityUserRepository>();
+        var currentTenant = GetRequiredService<ICurrentTenant>();
+        var unitOfWorkManager = GetRequiredService<IUnitOfWorkManager>();
+
+        var tenantId = Guid.NewGuid();
+        Guid userId;
+
+        using (var uow = unitOfWorkManager.Begin())
+        {
+            using (currentTenant.Change(tenantId))
+            {
+                var user = new IdentityUser(Guid.NewGuid(), userName, email, tenantId);
+                if (!isActive)
+                {
+                    user.SetIsActive(false);
+                }
+                await userRepository.InsertAsync(user);
+                userId = user.Id;
+            }
+            await uow.CompleteAsync();
+        }
+
+        return userId;
+    }
+
+    private async Task WriteTwoFactorCookieAsync(Guid userId)
+    {
+        var writeResponse = await Client.GetAsync($"/api/signin-test/write-two-factor-cookie?userId={userId}");
+        writeResponse.EnsureSuccessStatusCode();
+
+        if (writeResponse.Headers.TryGetValues("Set-Cookie", out var setCookies))
+        {
+            Client.DefaultRequestHeaders.Remove("Cookie");
+            foreach (var cookie in setCookies)
+            {
+                Client.DefaultRequestHeaders.Add("Cookie", cookie.Split(';').First());
+            }
+        }
+    }
 }

modules/identity/test/Volo.Abp.Identity.AspNetCore.Tests/Volo/Abp/Identity/AspNetCore/SignInTestController.cs

@@ -45,4 +45,18 @@ public class SignInTestController : AbpController
         var user = await _signInManager.GetTwoFactorAuthenticationUserAsync();
         return Content(user?.Id.ToString() ?? "null");
     }
+
+    [Route("two-factor-signin")]
+    public async Task<ActionResult> TwoFactorSignIn(string provider, string code)
+    {
+        var result = await _signInManager.TwoFactorSignInAsync(provider, code, false, false);
+        return Content(result.ToString());
+    }
+
+    [Route("two-factor-recovery-signin")]
+    public async Task<ActionResult> TwoFactorRecoverySignIn(string recoveryCode)
+    {
+        var result = await _signInManager.TwoFactorRecoveryCodeSignInAsync(recoveryCode);
+        return Content(result.ToString());
+    }
 }

modules/identity/test/Volo.Abp.Identity.Domain.Tests/Volo/Abp/Identity/IdentityUserManager_Tests.cs

@@ -678,55 +678,6 @@ public class SharedTenantUserSharingStrategy_IdentityUserManager_Tests : AbpIden
         }
     }
 
-    [Fact]
-    public async Task FindByIdAsync_Should_Fall_Back_To_Cross_Tenant_Lookup_In_Shared_Mode()
-    {
-        // In shared user sharing strategy, UserManager.FindByIdAsync must resolve a
-        // tenant user even when CurrentTenant is host. This protects any caller that
-        // hits FindByIdAsync without an explicit shared-aware wrapper (e.g. ASP.NET
-        // Core Identity SignInManager internals for TwoFactorSignInAsync and
-        // TwoFactorRecoveryCodeSignInAsync).
-        var tenantId = Guid.NewGuid();
-        IdentityUser tenantUser;
-
-        using (var uow = _unitOfWorkManager.Begin())
-        {
-            tenantUser = await CreateUserAsync(tenantId, "shared-findbyid-fallback", "shared-findbyid-fallback@abp.io");
-            await uow.CompleteAsync();
-        }
-
-        using (_currentTenant.Change(null))
-        {
-            var user = await _identityUserManager.FindByIdAsync(tenantUser.Id.ToString());
-
-            user.ShouldNotBeNull();
-            user.Id.ShouldBe(tenantUser.Id);
-            user.TenantId.ShouldBe(tenantId);
-        }
-    }
-
-    [Fact]
-    public async Task FindByIdAsync_Should_Return_Current_Tenant_Result_Without_Fallback_When_Found()
-    {
-        var tenantId = Guid.NewGuid();
-        IdentityUser tenantUser;
-
-        using (var uow = _unitOfWorkManager.Begin())
-        {
-            tenantUser = await CreateUserAsync(tenantId, "shared-findbyid-hit", "shared-findbyid-hit@abp.io");
-            await uow.CompleteAsync();
-        }
-
-        using (_currentTenant.Change(tenantId))
-        {
-            var user = await _identityUserManager.FindByIdAsync(tenantUser.Id.ToString());
-
-            user.ShouldNotBeNull();
-            user.Id.ShouldBe(tenantUser.Id);
-            user.TenantId.ShouldBe(tenantId);
-        }
-    }
-
     [Fact]
     public async Task Login_Then_TwoFactor_MidFlow_Should_Resolve_Same_Tenant_User_In_Shared_Mode()
     {