[release/11.0] Harden race condition when submitting 2FA during password reset

src/Identity/Core/src/SecurityStampValidator.cs

@@ -190,8 +190,7 @@ public static Task ValidatePrincipalAsync(CookieValidatePrincipalContext context
     /// </summary>
     /// <param name="context">The context containing the <see cref="System.Security.Claims.ClaimsPrincipal"/>
     /// and <see cref="AuthenticationProperties"/> to validate.</param>
-    /// <returns></returns>
-
+    /// <returns>The <see cref="Task"/> that represents the asynchronous validation operation.</returns>
     public static Task ValidateAsync<TValidator>(CookieValidatePrincipalContext context) where TValidator : ISecurityStampValidator
     {
         if (context.HttpContext.RequestServices == null)

src/Identity/Core/src/SignInManager.cs

@@ -1189,17 +1189,23 @@ public virtual AuthenticationProperties ConfigureExternalAuthenticationPropertie
     /// <summary>
     /// Creates a claims principal for the specified 2fa information.
     /// </summary>
-    /// <param name="userId">The user whose is logging in via 2fa.</param>
-    /// <param name="loginProvider">The 2fa provider.</param>
+    /// <param name="user">The user who is logging in via 2fa.</param>
+    /// <param name="loginProvider">The external login provider used to complete sign-in after 2FA (if applicable).</param>
     /// <returns>A <see cref="ClaimsPrincipal"/> containing the user 2fa information.</returns>
-    internal static ClaimsPrincipal StoreTwoFactorInfo(string userId, string? loginProvider)
+    internal async Task<ClaimsPrincipal> StoreTwoFactorInfo(TUser user, string? loginProvider)
     {
+        var userId = await UserManager.GetUserIdAsync(user);
         var identity = new ClaimsIdentity(IdentityConstants.TwoFactorUserIdScheme);
         identity.AddClaim(new Claim(ClaimTypes.Name, userId));
         if (loginProvider != null)
         {
             identity.AddClaim(new Claim(ClaimTypes.AuthenticationMethod, loginProvider));
         }
+        if (UserManager.SupportsUserSecurityStamp)
+        {
+            var stamp = await UserManager.GetSecurityStampAsync(user);
+            identity.AddClaim(new Claim(Options.ClaimsIdentity.SecurityStampClaimType, stamp));
+        }
         return new ClaimsPrincipal(identity);
     }
 
@@ -1253,9 +1259,8 @@ protected virtual async Task<SignInResult> SignInOrTwoFactorAsync(TUser user, bo
 
                 if (await _schemes.GetSchemeAsync(IdentityConstants.TwoFactorUserIdScheme) != null)
                 {
-                    // Store the userId for use after two factor check
-                    var userId = await UserManager.GetUserIdAsync(user);
-                    await Context.SignInAsync(IdentityConstants.TwoFactorUserIdScheme, StoreTwoFactorInfo(userId, loginProvider));
+                    // Store the user for use after two factor check
+                    await Context.SignInAsync(IdentityConstants.TwoFactorUserIdScheme, await StoreTwoFactorInfo(user, loginProvider));
                 }
 
                 return SignInResult.TwoFactorRequired;
@@ -1290,13 +1295,9 @@ protected virtual async Task<SignInResult> SignInOrTwoFactorAsync(TUser user, bo
             return null;
         }
 
-        var userId = result.Principal.FindFirstValue(ClaimTypes.Name);
-        if (userId == null)
-        {
-            return null;
-        }
-
-        var user = await UserManager.FindByIdAsync(userId);
+        // Validate the security stamp embedded in the two-factor principal so that a stale
+        // two-factor cookie (e.g. issued before a password reset) can no longer complete sign in.
+        var user = await ValidateTwoFactorSecurityStampAsync(result.Principal);
         if (user == null)
         {
             return null;

src/Identity/test/Identity.Test/SignInManagerTest.cs

@@ -748,7 +748,7 @@ public async Task CanTwoFactorAuthenticatorSignIn(string providerName, bool isPe
         {
             helper.Options.Tokens.AuthenticatorTokenProvider = providerName;
         }
-        var id = SignInManager<PocoUser>.StoreTwoFactorInfo(user.Id, null);
+        var id = await helper.StoreTwoFactorInfo(user, null);
         SetupSignIn(context, auth, user.Id, isPersistent);
         auth.Setup(a => a.AuthenticateAsync(context, IdentityConstants.TwoFactorUserIdScheme))
             .ReturnsAsync(AuthenticateResult.Success(new AuthenticationTicket(id, null, IdentityConstants.TwoFactorUserIdScheme))).Verifiable();
@@ -770,6 +770,39 @@ public async Task CanTwoFactorAuthenticatorSignIn(string providerName, bool isPe
         auth.Verify();
     }
 
+    [Fact]
+    public async Task TwoFactorAuthenticatorSignInFailsAfterSecurityStampChanges()
+    {
+        // Setup
+        var user = new PocoUser { UserName = "Foo" };
+        const string code = "3123";
+        var manager = SetupUserManager(user);
+        manager.Setup(m => m.SupportsUserSecurityStamp).Returns(true);
+        var stamp = "old-stamp";
+        manager.Setup(m => m.GetSecurityStampAsync(user)).ReturnsAsync(() => stamp);
+        manager.Setup(m => m.VerifyTwoFactorTokenAsync(user, TokenOptions.DefaultAuthenticatorProvider, code)).Throws(new Exception("Should not get called"));
+
+        var context = new DefaultHttpContext();
+        var auth = MockAuth(context);
+        var helper = SetupSignInManager(manager.Object, context);
+
+        // The two-factor cookie is issued while the current (old) security stamp is in effect.
+        var id = await helper.StoreTwoFactorInfo(user, null);
+        auth.Setup(a => a.AuthenticateAsync(context, IdentityConstants.TwoFactorUserIdScheme))
+            .ReturnsAsync(AuthenticateResult.Success(new AuthenticationTicket(id, null, IdentityConstants.TwoFactorUserIdScheme))).Verifiable();
+
+        // Simulate a password reset changing the security stamp before the 2FA code is submitted.
+        stamp = "new-stamp";
+
+        // Act
+        var result = await helper.TwoFactorAuthenticatorSignInAsync(code, isPersistent: false, rememberClient: false);
+
+        // Assert
+        Assert.Same(SignInResult.Failed, result);
+        manager.Verify();
+        auth.Verify();
+    }
+
     [Fact]
     public async Task TwoFactorAuthenticatorSignInFailWithoutLockout()
     {
@@ -790,7 +823,7 @@ public async Task TwoFactorAuthenticatorSignInFailWithoutLockout()
         {
             helper.Options.Tokens.AuthenticatorTokenProvider = providerName;
         }
-        var id = SignInManager<PocoUser>.StoreTwoFactorInfo(user.Id, null);
+        var id = await helper.StoreTwoFactorInfo(user, null);
         auth.Setup(a => a.AuthenticateAsync(context, IdentityConstants.TwoFactorUserIdScheme))
             .ReturnsAsync(AuthenticateResult.Success(new AuthenticationTicket(id, null, IdentityConstants.TwoFactorUserIdScheme))).Verifiable();
 
@@ -829,7 +862,7 @@ public async Task TwoFactorAuthenticatorSignInAsyncReturnsLockedOut()
         {
             helper.Options.Tokens.AuthenticatorTokenProvider = providerName;
         }
-        var id = SignInManager<PocoUser>.StoreTwoFactorInfo(user.Id, null);
+        var id = await helper.StoreTwoFactorInfo(user, null);
         auth.Setup(a => a.AuthenticateAsync(context, IdentityConstants.TwoFactorUserIdScheme))
             .ReturnsAsync(AuthenticateResult.Success(new AuthenticationTicket(id, null, IdentityConstants.TwoFactorUserIdScheme))).Verifiable();
 
@@ -905,7 +938,7 @@ public async Task CanTwoFactorRecoveryCodeSignIn(bool supportsLockout, bool exte
         var helper = SetupSignInManager(manager.Object, context);
         var twoFactorInfo = new SignInManager<PocoUser>.TwoFactorAuthenticationInfo { User = user };
         var loginProvider = "loginprovider";
-        var id = SignInManager<PocoUser>.StoreTwoFactorInfo(user.Id, externalLogin ? loginProvider : null);
+        var id = await helper.StoreTwoFactorInfo(user, externalLogin ? loginProvider : null);
         if (externalLogin)
         {
             auth.Setup(a => a.SignInAsync(context,
@@ -1157,7 +1190,7 @@ public async Task CanTwoFactorSignIn(bool isPersistent, bool supportsLockout, bo
         var helper = SetupSignInManager(manager.Object, context);
         var twoFactorInfo = new SignInManager<PocoUser>.TwoFactorAuthenticationInfo { User = user };
         var loginProvider = "loginprovider";
-        var id = SignInManager<PocoUser>.StoreTwoFactorInfo(user.Id, externalLogin ? loginProvider : null);
+        var id = await helper.StoreTwoFactorInfo(user, externalLogin ? loginProvider : null);
         if (externalLogin)
         {
             auth.Setup(a => a.SignInAsync(context,
@@ -1217,7 +1250,7 @@ public async Task TwoFactorSignInAsyncReturnsLockedOut()
         var context = new DefaultHttpContext();
         var auth = MockAuth(context);
         var helper = SetupSignInManager(manager.Object, context);
-        var id = SignInManager<PocoUser>.StoreTwoFactorInfo(user.Id, loginProvider: null);
+        var id = await helper.StoreTwoFactorInfo(user, loginProvider: null);
 
         auth.Setup(a => a.AuthenticateAsync(context, IdentityConstants.TwoFactorUserIdScheme))
             .ReturnsAsync(AuthenticateResult.Success(new AuthenticationTicket(id, null, IdentityConstants.TwoFactorUserIdScheme))).Verifiable();
@@ -1776,7 +1809,7 @@ public async Task TwoFactorSignFailsWhenResetLockoutFails()
         var context = new DefaultHttpContext();
         var auth = MockAuth(context);
         var helper = SetupSignInManager(manager.Object, context);
-        var id = SignInManager<PocoUser>.StoreTwoFactorInfo(user.Id, null);
+        var id = await helper.StoreTwoFactorInfo(user, null);
         auth.Setup(a => a.AuthenticateAsync(context, IdentityConstants.TwoFactorUserIdScheme))
             .ReturnsAsync(AuthenticateResult.Success(new AuthenticationTicket(id, null, IdentityConstants.TwoFactorUserIdScheme))).Verifiable();
 
@@ -1842,7 +1875,7 @@ public async Task TwoFactorSignInLockedOutResultIsDependentOnTheAccessFailedAsyn
         var context = new DefaultHttpContext();
         var auth = MockAuth(context);
         var helper = SetupSignInManager(manager.Object, context);
-        var id = SignInManager<PocoUser>.StoreTwoFactorInfo(user.Id, null);
+        var id = await helper.StoreTwoFactorInfo(user, null);
         auth.Setup(a => a.AuthenticateAsync(context, IdentityConstants.TwoFactorUserIdScheme))
             .ReturnsAsync(AuthenticateResult.Success(new AuthenticationTicket(id, null, IdentityConstants.TwoFactorUserIdScheme))).Verifiable();
 

src/Identity/test/InMemory.Test/FunctionalTest.cs

@@ -278,6 +278,65 @@ public async Task TwoFactorRememberCookieClearedBySecurityStampChange(bool testC
         Assert.Equal(HttpStatusCode.InternalServerError, transaction6.Response.StatusCode);
     }
 
+    [Theory]
+    [InlineData(true)]
+    [InlineData(false)]
+    public async Task PasskeyCeremonyStateSurvivesSecurityStampValidationWithZeroValidationInterval(bool testCore)
+    {
+        var timeProvider = new FakeTimeProvider();
+        var server = await CreateServer(services =>
+        {
+            services.AddSingleton<IPasskeyHandler<PocoUser>>(new TestPasskeyHandler());
+            services.Configure<SecurityStampValidatorOptions>(options =>
+            {
+                options.TimeProvider = timeProvider;
+                options.ValidationInterval = TimeSpan.Zero;
+            });
+            services.Configure<CookieAuthenticationOptions>(IdentityConstants.TwoFactorUserIdScheme, options =>
+            {
+                options.TimeProvider = timeProvider;
+            });
+        }, testCore: testCore);
+
+        var transaction1 = await SendAsync(server, "http://example.com/createMe");
+        Assert.Equal(HttpStatusCode.OK, transaction1.Response.StatusCode);
+
+        // Begin a passkey ceremony. This stashes ceremony state in the TwoFactorUserId cookie
+        // using an empty principal (no user id / security stamp claim).
+        var transaction2 = await SendAsync(server, "http://example.com/makePasskeyRequestOptions");
+        Assert.Equal(HttpStatusCode.OK, transaction2.Response.StatusCode);
+        Assert.Contains(IdentityConstants.TwoFactorUserIdScheme + "=", transaction2.SetCookie);
+
+        // Advance past the (zero) validation interval so the security stamp validator runs
+        // on the next request while the ceremony cookie is still within its 5 minute lifetime.
+        timeProvider.Advance(TimeSpan.FromMinutes(1));
+
+        // Retrieving the passkey ceremony state authenticates the TwoFactorUserId cookie, exactly
+        // as RetrievePasskeyAuthenticationInfoAsync does. The empty ceremony principal must not be
+        // rejected by the two-factor security stamp validator, otherwise the ceremony state is lost.
+        var transaction3 = await SendAsync(server, "http://example.com/hasTwoFactorUserId", transaction2.CookieNameValue);
+        Assert.Equal(HttpStatusCode.OK, transaction3.Response.StatusCode);
+    }
+
+    private sealed class TestPasskeyHandler : IPasskeyHandler<PocoUser>
+    {
+        public Task<PasskeyRequestOptionsResult> MakeRequestOptionsAsync(PocoUser user, HttpContext httpContext)
+            => Task.FromResult(new PasskeyRequestOptionsResult
+            {
+                RequestOptionsJson = "{}",
+                AssertionState = "test-assertion-state",
+            });
+
+        public Task<PasskeyCreationOptionsResult> MakeCreationOptionsAsync(PasskeyUserEntity userEntity, HttpContext httpContext)
+            => throw new NotSupportedException();
+
+        public Task<PasskeyAttestationResult> PerformAttestationAsync(PasskeyAttestationContext context)
+            => throw new NotSupportedException();
+
+        public Task<PasskeyAssertionResult<PocoUser>> PerformAssertionAsync(PasskeyAssertionContext context)
+            => throw new NotSupportedException();
+    }
+
     private static string FindClaimValue(Transaction transaction, string claimType)
     {
         var claim = transaction.ResponseElement.Elements("claim").SingleOrDefault(elt => elt.Attribute("type").Value == claimType);
@@ -355,6 +414,12 @@ private async Task<TestServer> CreateServer(Action<IServiceCollection> configure
                             var result = await context.AuthenticateAsync(IdentityConstants.TwoFactorUserIdScheme);
                             res.StatusCode = result.Succeeded ? 200 : 500;
                         }
+                        else if (req.Path == new PathString("/makePasskeyRequestOptions"))
+                        {
+                            var user = await userManager.FindByNameAsync("hao");
+                            await signInManager.MakePasskeyRequestOptionsAsync(user);
+                            res.StatusCode = 200;
+                        }
                         else if (req.Path == new PathString("/me"))
                         {
                             await DescribeAsync(res, AuthenticateResult.Success(new AuthenticationTicket(context.User, null, "Application")));