diff --git a/src/Identity/Core/src/SecurityStampValidator.cs b/src/Identity/Core/src/SecurityStampValidator.cs index 7e8c1bbc93a6..53a98e2310f2 100644 --- a/src/Identity/Core/src/SecurityStampValidator.cs +++ b/src/Identity/Core/src/SecurityStampValidator.cs @@ -190,8 +190,7 @@ public static Task ValidatePrincipalAsync(CookieValidatePrincipalContext context /// /// The context containing the /// and to validate. - /// - + /// The that represents the asynchronous validation operation. public static Task ValidateAsync(CookieValidatePrincipalContext context) where TValidator : ISecurityStampValidator { if (context.HttpContext.RequestServices == null) diff --git a/src/Identity/Core/src/SignInManager.cs b/src/Identity/Core/src/SignInManager.cs index 03b0dc49b3d5..45eb07870452 100644 --- a/src/Identity/Core/src/SignInManager.cs +++ b/src/Identity/Core/src/SignInManager.cs @@ -1189,17 +1189,23 @@ public virtual AuthenticationProperties ConfigureExternalAuthenticationPropertie /// /// Creates a claims principal for the specified 2fa information. /// - /// The user whose is logging in via 2fa. - /// The 2fa provider. + /// The user who is logging in via 2fa. + /// The external login provider used to complete sign-in after 2FA (if applicable). /// A containing the user 2fa information. - internal static ClaimsPrincipal StoreTwoFactorInfo(string userId, string? loginProvider) + internal async Task 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 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 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; diff --git a/src/Identity/test/Identity.Test/SignInManagerTest.cs b/src/Identity/test/Identity.Test/SignInManagerTest.cs index f6d9c3159c5f..9d0aadd02f56 100644 --- a/src/Identity/test/Identity.Test/SignInManagerTest.cs +++ b/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.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.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.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.TwoFactorAuthenticationInfo { User = user }; var loginProvider = "loginprovider"; - var id = SignInManager.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.TwoFactorAuthenticationInfo { User = user }; var loginProvider = "loginprovider"; - var id = SignInManager.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.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.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.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(); diff --git a/src/Identity/test/InMemory.Test/FunctionalTest.cs b/src/Identity/test/InMemory.Test/FunctionalTest.cs index fcb7b958312e..e2db70d011c6 100644 --- a/src/Identity/test/InMemory.Test/FunctionalTest.cs +++ b/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>(new TestPasskeyHandler()); + services.Configure(options => + { + options.TimeProvider = timeProvider; + options.ValidationInterval = TimeSpan.Zero; + }); + services.Configure(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 + { + public Task MakeRequestOptionsAsync(PocoUser user, HttpContext httpContext) + => Task.FromResult(new PasskeyRequestOptionsResult + { + RequestOptionsJson = "{}", + AssertionState = "test-assertion-state", + }); + + public Task MakeCreationOptionsAsync(PasskeyUserEntity userEntity, HttpContext httpContext) + => throw new NotSupportedException(); + + public Task PerformAttestationAsync(PasskeyAttestationContext context) + => throw new NotSupportedException(); + + public Task> 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 CreateServer(Action 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")));