From 3bc8a31a785d4da91ac94fc451ad689a88ea20f7 Mon Sep 17 00:00:00 2001 From: Adit Sheth Date: Wed, 12 Feb 2025 11:45:42 -0800 Subject: [PATCH 1/8] Fixed bug 60181. --- .../Extensions.Core/src/LockoutOptions.cs | 6 +++ .../Extensions.Core/src/UserManager.cs | 45 ++++++++++++------- .../test/Identity.Test/UserManagerTest.cs | 18 ++++++++ 3 files changed, 54 insertions(+), 15 deletions(-) diff --git a/src/Identity/Extensions.Core/src/LockoutOptions.cs b/src/Identity/Extensions.Core/src/LockoutOptions.cs index def4e3605a1c..be19bde4edf1 100644 --- a/src/Identity/Extensions.Core/src/LockoutOptions.cs +++ b/src/Identity/Extensions.Core/src/LockoutOptions.cs @@ -32,4 +32,10 @@ public class LockoutOptions /// /// The a user is locked out for when a lockout occurs. public TimeSpan DefaultLockoutTimeSpan { get; set; } = TimeSpan.FromMinutes(5); + + /// + /// Specifies whether the lockout should be permanent. + /// If true, the user will be locked out indefinitely. + /// + public bool PermanentLockout { get; set; } = false; } diff --git a/src/Identity/Extensions.Core/src/UserManager.cs b/src/Identity/Extensions.Core/src/UserManager.cs index cca2005d10d0..22e00a84fb8c 100644 --- a/src/Identity/Extensions.Core/src/UserManager.cs +++ b/src/Identity/Extensions.Core/src/UserManager.cs @@ -1812,24 +1812,39 @@ public virtual async Task SetLockoutEndDateAsync(TUser user, Dat /// /// The user whose failed access count to increment. /// The that represents the asynchronous operation, containing the of the operation. - public virtual async Task AccessFailedAsync(TUser user) - { - ThrowIfDisposed(); - var store = GetUserLockoutStore(); - ArgumentNullThrowHelper.ThrowIfNull(user); +public virtual async Task AccessFailedAsync(TUser user) +{ + ThrowIfDisposed(); + var store = GetUserLockoutStore(); + ArgumentNullThrowHelper.ThrowIfNull(user); - // If this puts the user over the threshold for lockout, lock them out and reset the access failed count - var count = await store.IncrementAccessFailedCountAsync(user, CancellationToken).ConfigureAwait(false); - if (count < Options.Lockout.MaxFailedAccessAttempts) - { - return await UpdateUserAsync(user).ConfigureAwait(false); - } - Logger.LogDebug(LoggerEventIds.UserLockedOut, "User is locked out."); - await store.SetLockoutEndDateAsync(user, DateTimeOffset.UtcNow.Add(Options.Lockout.DefaultLockoutTimeSpan), - CancellationToken).ConfigureAwait(false); - await store.ResetAccessFailedCountAsync(user, CancellationToken).ConfigureAwait(false); +// If this puts the user over the threshold for lockout, lock them out and reset the access failed count + var count = await store.IncrementAccessFailedCountAsync(user, CancellationToken).ConfigureAwait(false); + if (count < Options.Lockout.MaxFailedAccessAttempts) + { return await UpdateUserAsync(user).ConfigureAwait(false); } + Logger.LogDebug(LoggerEventIds.UserLockedOut, "User is locked out."); + + // Prevent overflow when setting lockout end date + var now = DateTimeOffset.UtcNow; + DateTimeOffset lockoutEnd; + + if (Options.Lockout.DefaultLockoutTimeSpan == TimeSpan.MaxValue) + { + lockoutEnd = DateTimeOffset.MaxValue; + } + else + { + lockoutEnd = now > (DateTimeOffset.MaxValue - Options.Lockout.DefaultLockoutTimeSpan) + ? DateTimeOffset.MaxValue + : now.Add(Options.Lockout.DefaultLockoutTimeSpan); + } + + await store.SetLockoutEndDateAsync(user, lockoutEnd, CancellationToken).ConfigureAwait(false); + await store.ResetAccessFailedCountAsync(user, CancellationToken).ConfigureAwait(false); + return await UpdateUserAsync(user).ConfigureAwait(false); +} /// /// Resets the access failed count for the specified . diff --git a/src/Identity/test/Identity.Test/UserManagerTest.cs b/src/Identity/test/Identity.Test/UserManagerTest.cs index 04f9e2afa476..733344a4e6d8 100644 --- a/src/Identity/test/Identity.Test/UserManagerTest.cs +++ b/src/Identity/test/Identity.Test/UserManagerTest.cs @@ -1012,6 +1012,24 @@ public async Task ResetTokenCallNoopForTokenValueZero() IdentityResultAssert.IsSuccess(await manager.ResetAccessFailedCountAsync(user)); } + [Fact] + public async Task AccessFailedAsync_IncrementsAccessFailedCount() + { + // Arrange + var user = new PocoUser() { UserName = "testuser" }; + var store = new Mock>(); + store.Setup(x => x.GetAccessFailedCountAsync(user, It.IsAny())).ReturnsAsync(1); + store.Setup(x => x.IncrementAccessFailedCountAsync(user, It.IsAny())).ReturnsAsync(2); + + var manager = MockHelpers.TestUserManager(store.Object); + + // Act + var result = await manager.AccessFailedAsync(user); + + // Assert + Assert.Equal(2, result); + } + [Fact] public async Task ManagerPublicNullChecks() { From f3116b0fa2c9e97bbc5d91d91c6653965620cea9 Mon Sep 17 00:00:00 2001 From: Adit Sheth Date: Wed, 12 Feb 2025 11:53:02 -0800 Subject: [PATCH 2/8] Update src/Identity/test/Identity.Test/UserManagerTest.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/Identity/test/Identity.Test/UserManagerTest.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Identity/test/Identity.Test/UserManagerTest.cs b/src/Identity/test/Identity.Test/UserManagerTest.cs index 733344a4e6d8..1112f0407141 100644 --- a/src/Identity/test/Identity.Test/UserManagerTest.cs +++ b/src/Identity/test/Identity.Test/UserManagerTest.cs @@ -1027,7 +1027,10 @@ public async Task AccessFailedAsync_IncrementsAccessFailedCount() var result = await manager.AccessFailedAsync(user); // Assert - Assert.Equal(2, result); + IdentityResultAssert.IsSuccess(result); + store.Verify(x => x.IncrementAccessFailedCountAsync(user, It.IsAny()), Times.Once); + var failedCount = await manager.GetAccessFailedCountAsync(user); + Assert.Equal(2, failedCount); } [Fact] From d09ad4aab31e941cafe7ebe1eed93cf68f08c2a6 Mon Sep 17 00:00:00 2001 From: Adit Sheth Date: Wed, 12 Feb 2025 12:23:18 -0800 Subject: [PATCH 3/8] Added few missing code and changes. --- .../Extensions.Core/src/LockoutOptions.cs | 2 +- .../Extensions.Core/src/PublicAPI.Shipped.txt | 2 + .../Extensions.Core/src/UserManager.cs | 54 +++++++++---------- .../test/Identity.Test/UserManagerTest.cs | 16 ++++-- 4 files changed, 41 insertions(+), 33 deletions(-) diff --git a/src/Identity/Extensions.Core/src/LockoutOptions.cs b/src/Identity/Extensions.Core/src/LockoutOptions.cs index be19bde4edf1..aa3b31ca6e79 100644 --- a/src/Identity/Extensions.Core/src/LockoutOptions.cs +++ b/src/Identity/Extensions.Core/src/LockoutOptions.cs @@ -37,5 +37,5 @@ public class LockoutOptions /// Specifies whether the lockout should be permanent. /// If true, the user will be locked out indefinitely. /// - public bool PermanentLockout { get; set; } = false; + public bool PermanentLockout { get; set; } } diff --git a/src/Identity/Extensions.Core/src/PublicAPI.Shipped.txt b/src/Identity/Extensions.Core/src/PublicAPI.Shipped.txt index b30ce145ace6..4ccd1325150b 100644 --- a/src/Identity/Extensions.Core/src/PublicAPI.Shipped.txt +++ b/src/Identity/Extensions.Core/src/PublicAPI.Shipped.txt @@ -184,6 +184,8 @@ Microsoft.AspNetCore.Identity.LockoutOptions.AllowedForNewUsers.get -> bool Microsoft.AspNetCore.Identity.LockoutOptions.AllowedForNewUsers.set -> void Microsoft.AspNetCore.Identity.LockoutOptions.DefaultLockoutTimeSpan.get -> System.TimeSpan Microsoft.AspNetCore.Identity.LockoutOptions.DefaultLockoutTimeSpan.set -> void +Microsoft.AspNetCore.Identity.LockoutOptions.PermanentLockout.get -> bool +Microsoft.AspNetCore.Identity.LockoutOptions.PermanentLockout.set -> void Microsoft.AspNetCore.Identity.LockoutOptions.LockoutOptions() -> void Microsoft.AspNetCore.Identity.LockoutOptions.MaxFailedAccessAttempts.get -> int Microsoft.AspNetCore.Identity.LockoutOptions.MaxFailedAccessAttempts.set -> void diff --git a/src/Identity/Extensions.Core/src/UserManager.cs b/src/Identity/Extensions.Core/src/UserManager.cs index 22e00a84fb8c..bfda6254a862 100644 --- a/src/Identity/Extensions.Core/src/UserManager.cs +++ b/src/Identity/Extensions.Core/src/UserManager.cs @@ -1812,39 +1812,39 @@ public virtual async Task SetLockoutEndDateAsync(TUser user, Dat /// /// The user whose failed access count to increment. /// The that represents the asynchronous operation, containing the of the operation. -public virtual async Task AccessFailedAsync(TUser user) -{ - ThrowIfDisposed(); - var store = GetUserLockoutStore(); - ArgumentNullThrowHelper.ThrowIfNull(user); - -// If this puts the user over the threshold for lockout, lock them out and reset the access failed count - var count = await store.IncrementAccessFailedCountAsync(user, CancellationToken).ConfigureAwait(false); - if (count < Options.Lockout.MaxFailedAccessAttempts) + public virtual async Task AccessFailedAsync(TUser user) { - return await UpdateUserAsync(user).ConfigureAwait(false); - } - Logger.LogDebug(LoggerEventIds.UserLockedOut, "User is locked out."); + ThrowIfDisposed(); + var store = GetUserLockoutStore(); + ArgumentNullThrowHelper.ThrowIfNull(user); - // Prevent overflow when setting lockout end date - var now = DateTimeOffset.UtcNow; - DateTimeOffset lockoutEnd; + // If PermanentLockout is enabled, lock the user indefinitely + if (Options.Lockout.PermanentLockout) + { + Logger.LogDebug(LoggerEventIds.UserLockedOut, "User is permanently locked out."); + await store.SetLockoutEndDateAsync(user, DateTimeOffset.MaxValue, CancellationToken).ConfigureAwait(false); + return await UpdateUserAsync(user).ConfigureAwait(false); + } - if (Options.Lockout.DefaultLockoutTimeSpan == TimeSpan.MaxValue) - { - lockoutEnd = DateTimeOffset.MaxValue; - } - else - { - lockoutEnd = now > (DateTimeOffset.MaxValue - Options.Lockout.DefaultLockoutTimeSpan) + // Increment access failed count + var count = await store.IncrementAccessFailedCountAsync(user, CancellationToken).ConfigureAwait(false); + if (count < Options.Lockout.MaxFailedAccessAttempts) + { + return await UpdateUserAsync(user).ConfigureAwait(false); + } + + Logger.LogDebug(LoggerEventIds.UserLockedOut, "User is locked out."); + + // Set the lockout time based on configuration + var now = DateTimeOffset.UtcNow; + DateTimeOffset lockoutEnd = Options.Lockout.DefaultLockoutTimeSpan == TimeSpan.MaxValue ? DateTimeOffset.MaxValue : now.Add(Options.Lockout.DefaultLockoutTimeSpan); - } - await store.SetLockoutEndDateAsync(user, lockoutEnd, CancellationToken).ConfigureAwait(false); - await store.ResetAccessFailedCountAsync(user, CancellationToken).ConfigureAwait(false); - return await UpdateUserAsync(user).ConfigureAwait(false); -} + await store.SetLockoutEndDateAsync(user, lockoutEnd, CancellationToken).ConfigureAwait(false); + await store.ResetAccessFailedCountAsync(user, CancellationToken).ConfigureAwait(false); + return await UpdateUserAsync(user).ConfigureAwait(false); + } /// /// Resets the access failed count for the specified . diff --git a/src/Identity/test/Identity.Test/UserManagerTest.cs b/src/Identity/test/Identity.Test/UserManagerTest.cs index 1112f0407141..2c57273d67f9 100644 --- a/src/Identity/test/Identity.Test/UserManagerTest.cs +++ b/src/Identity/test/Identity.Test/UserManagerTest.cs @@ -1013,13 +1013,18 @@ public async Task ResetTokenCallNoopForTokenValueZero() } [Fact] - public async Task AccessFailedAsync_IncrementsAccessFailedCount() + public async Task AccessFailedAsyncIncrementsAccessFailedCount() { // Arrange var user = new PocoUser() { UserName = "testuser" }; var store = new Mock>(); - store.Setup(x => x.GetAccessFailedCountAsync(user, It.IsAny())).ReturnsAsync(1); - store.Setup(x => x.IncrementAccessFailedCountAsync(user, It.IsAny())).ReturnsAsync(2); + int failedCount = 1; // Simulated access failed count + + store.Setup(x => x.GetAccessFailedCountAsync(user, It.IsAny())) + .ReturnsAsync(() => failedCount); // Return the updated value dynamically + + store.Setup(x => x.IncrementAccessFailedCountAsync(user, It.IsAny())) + .ReturnsAsync(() => ++failedCount); // Increment and return the new count var manager = MockHelpers.TestUserManager(store.Object); @@ -1029,8 +1034,9 @@ public async Task AccessFailedAsync_IncrementsAccessFailedCount() // Assert IdentityResultAssert.IsSuccess(result); store.Verify(x => x.IncrementAccessFailedCountAsync(user, It.IsAny()), Times.Once); - var failedCount = await manager.GetAccessFailedCountAsync(user); - Assert.Equal(2, failedCount); + + var newFailedCount = await manager.GetAccessFailedCountAsync(user); + Assert.Equal(2, newFailedCount); // Ensure the count was actually updated } [Fact] From 2caf051e006b642d2fcd33224d1cb869347d3911 Mon Sep 17 00:00:00 2001 From: Adit Sheth Date: Wed, 12 Feb 2025 12:41:28 -0800 Subject: [PATCH 4/8] Moved new property to Public API Unshipped. --- src/Identity/Extensions.Core/src/PublicAPI.Shipped.txt | 2 -- src/Identity/Extensions.Core/src/PublicAPI.Unshipped.txt | 1 + 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Identity/Extensions.Core/src/PublicAPI.Shipped.txt b/src/Identity/Extensions.Core/src/PublicAPI.Shipped.txt index 4ccd1325150b..b30ce145ace6 100644 --- a/src/Identity/Extensions.Core/src/PublicAPI.Shipped.txt +++ b/src/Identity/Extensions.Core/src/PublicAPI.Shipped.txt @@ -184,8 +184,6 @@ Microsoft.AspNetCore.Identity.LockoutOptions.AllowedForNewUsers.get -> bool Microsoft.AspNetCore.Identity.LockoutOptions.AllowedForNewUsers.set -> void Microsoft.AspNetCore.Identity.LockoutOptions.DefaultLockoutTimeSpan.get -> System.TimeSpan Microsoft.AspNetCore.Identity.LockoutOptions.DefaultLockoutTimeSpan.set -> void -Microsoft.AspNetCore.Identity.LockoutOptions.PermanentLockout.get -> bool -Microsoft.AspNetCore.Identity.LockoutOptions.PermanentLockout.set -> void Microsoft.AspNetCore.Identity.LockoutOptions.LockoutOptions() -> void Microsoft.AspNetCore.Identity.LockoutOptions.MaxFailedAccessAttempts.get -> int Microsoft.AspNetCore.Identity.LockoutOptions.MaxFailedAccessAttempts.set -> void diff --git a/src/Identity/Extensions.Core/src/PublicAPI.Unshipped.txt b/src/Identity/Extensions.Core/src/PublicAPI.Unshipped.txt index 7dc5c58110bf..e625be50977d 100644 --- a/src/Identity/Extensions.Core/src/PublicAPI.Unshipped.txt +++ b/src/Identity/Extensions.Core/src/PublicAPI.Unshipped.txt @@ -1 +1,2 @@ #nullable enable +Microsoft.AspNetCore.Identity.LockoutOptions.PermanentLockout.set -> void From 078ff236e487093c3dd779136b5b5997f7b04bdf Mon Sep 17 00:00:00 2001 From: Adit Sheth Date: Wed, 12 Feb 2025 12:42:03 -0800 Subject: [PATCH 5/8] More changes. --- src/Identity/Extensions.Core/src/PublicAPI.Unshipped.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Identity/Extensions.Core/src/PublicAPI.Unshipped.txt b/src/Identity/Extensions.Core/src/PublicAPI.Unshipped.txt index e625be50977d..e3ad8f42a18b 100644 --- a/src/Identity/Extensions.Core/src/PublicAPI.Unshipped.txt +++ b/src/Identity/Extensions.Core/src/PublicAPI.Unshipped.txt @@ -1,2 +1,3 @@ #nullable enable +Microsoft.AspNetCore.Identity.LockoutOptions.PermanentLockout.get -> bool Microsoft.AspNetCore.Identity.LockoutOptions.PermanentLockout.set -> void From badef0ecab59139be6810d1e36056538277b7707 Mon Sep 17 00:00:00 2001 From: Adit Sheth Date: Thu, 13 Feb 2025 09:23:10 -0800 Subject: [PATCH 6/8] Update src/Identity/Extensions.Core/src/LockoutOptions.cs Co-authored-by: Rick Anderson <3605364+Rick-Anderson@users.noreply.github.com> --- src/Identity/Extensions.Core/src/LockoutOptions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Identity/Extensions.Core/src/LockoutOptions.cs b/src/Identity/Extensions.Core/src/LockoutOptions.cs index aa3b31ca6e79..82173d94bd54 100644 --- a/src/Identity/Extensions.Core/src/LockoutOptions.cs +++ b/src/Identity/Extensions.Core/src/LockoutOptions.cs @@ -35,7 +35,7 @@ public class LockoutOptions /// /// Specifies whether the lockout should be permanent. - /// If true, the user will be locked out indefinitely. + /// If true, the user is locked out. /// public bool PermanentLockout { get; set; } } From 4ec7a286b98b92fca5b6bc0d91a46d85e824842f Mon Sep 17 00:00:00 2001 From: Adit Sheth Date: Thu, 13 Feb 2025 09:23:21 -0800 Subject: [PATCH 7/8] Update src/Identity/Extensions.Core/src/UserManager.cs Co-authored-by: Rick Anderson <3605364+Rick-Anderson@users.noreply.github.com> --- src/Identity/Extensions.Core/src/UserManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Identity/Extensions.Core/src/UserManager.cs b/src/Identity/Extensions.Core/src/UserManager.cs index bfda6254a862..9898a1fa0ba3 100644 --- a/src/Identity/Extensions.Core/src/UserManager.cs +++ b/src/Identity/Extensions.Core/src/UserManager.cs @@ -1835,7 +1835,7 @@ public virtual async Task AccessFailedAsync(TUser user) Logger.LogDebug(LoggerEventIds.UserLockedOut, "User is locked out."); - // Set the lockout time based on configuration + // Set the lockout time based on configuration. var now = DateTimeOffset.UtcNow; DateTimeOffset lockoutEnd = Options.Lockout.DefaultLockoutTimeSpan == TimeSpan.MaxValue ? DateTimeOffset.MaxValue From 73b68906541e49b0ddb831ce7fbe9d9053db6809 Mon Sep 17 00:00:00 2001 From: Adit Sheth Date: Thu, 20 Feb 2025 12:01:06 -0800 Subject: [PATCH 8/8] Fixed tests. --- .../test/Identity.Test/UserManagerTest.cs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/Identity/test/Identity.Test/UserManagerTest.cs b/src/Identity/test/Identity.Test/UserManagerTest.cs index 2c57273d67f9..eb5f12a6401b 100644 --- a/src/Identity/test/Identity.Test/UserManagerTest.cs +++ b/src/Identity/test/Identity.Test/UserManagerTest.cs @@ -1018,25 +1018,34 @@ public async Task AccessFailedAsyncIncrementsAccessFailedCount() // Arrange var user = new PocoUser() { UserName = "testuser" }; var store = new Mock>(); - int failedCount = 1; // Simulated access failed count + int failedCount = 1; + + store.Setup(x => x.SupportsUserLockout).Returns(true); store.Setup(x => x.GetAccessFailedCountAsync(user, It.IsAny())) - .ReturnsAsync(() => failedCount); // Return the updated value dynamically + .ReturnsAsync(() => failedCount); store.Setup(x => x.IncrementAccessFailedCountAsync(user, It.IsAny())) - .ReturnsAsync(() => ++failedCount); // Increment and return the new count + .Callback(() => failedCount++) + .ReturnsAsync(() => failedCount); + + store.Setup(x => x.UpdateAsync(user, It.IsAny())) + .ReturnsAsync(IdentityResult.Success); var manager = MockHelpers.TestUserManager(store.Object); + manager?.Options?.Lockout?.PermanentLockout = false; // Act var result = await manager.AccessFailedAsync(user); // Assert - IdentityResultAssert.IsSuccess(result); + Assert.NotNull(result); + Assert.True(result.Succeeded, "AccessFailedAsync should return success."); + store.Verify(x => x.IncrementAccessFailedCountAsync(user, It.IsAny()), Times.Once); var newFailedCount = await manager.GetAccessFailedCountAsync(user); - Assert.Equal(2, newFailedCount); // Ensure the count was actually updated + Assert.Equal(2, newFailedCount); } [Fact]