dotnet / dotnet/aspnetcore

UserManager.AccessFailedAsync results in overflow if DefaultLockoutTimeSpan is too large

Open
#60,181 3 comments 0 reactions 0 assignees View on GitHub
area-identity help wanted
Dominant language
C#
Stars
38.4k
Forks
10.9k
Avg merge
2d 9h
Merged PRs (30d)
276

Description

### Is there an existing issue for this?

- [x] I have searched the existing issues

### Describe the bug

When using ASP.NET Identity, under certain conditions, the method `UserManager.AccessFailedAsync()` fails with an overflow exception if the `DefaultLockoutTimeSpan` is too large.

An example scenario of where this may occur is, when a call is made to `SignInManager.TwoFactorySignInAsync()` that results in a failed MFA sign-in, the method `UserManager.AccessFailedAsync()` will be called.

The problem is this line:
```csharp
await store.SetLockoutEndDateAsync(user, DateTimeOffset.UtcNow.Add(Options.Lockout.DefaultLockoutTimeSpan),
CancellationToken).ConfigureAwait(false);
```

The use-case here is that, the `DefaultLockoutTimeSpan` was being set to `TimeSpan.MaxValue` as a way to basically enforce lockouts are not automatically removed as a way to lock an account for investigation of suspicious activity for certain operations - such as entering many invalid MFA codes over and over again.

### Expected Behavior

I think this should be more forgiving or have a way to set an indefinite lockout timespan. The only alternative currently available is setting to something large, but arbitrary, such as `TimeSpan.FromDays(50000)` which just looks worse - if you see 12/31/9999 as an end date, you have an idea that it's basically forever, but if you see 2/4/4737, for example, it doesn't mean anything.

So in UserManager either update the call to something like this:

```csharp
var now = DateTimeOffset.UtcNow;
DateTimeOffset lockoutEnd = now > DateTimeOffset.MaxValue - Options.Lockout.DefaultLockoutTimeSpan? DateTimeOffset.MaxValue : now.Add(Options.Lockout.DefaultLockoutTimeSpan);

await store.SetLockoutEndDateAsync(user, lockoutEnd,CancellationToken).ConfigureAwait(false);
```

Or a new option added to specify an endless lockout.

### Steps To Reproduce

_No response_

### Exceptions (if any)

_No response_

### .NET Version

.NET 9

### Anything else?

_No response_

Contributor guide

Open the contributing guide

Research direction

Start at UserManager.AccessFailedAsync and inspect the shown lockout-end calculation, then reproduce it with DefaultLockoutTimeSpan set to TimeSpan.MaxValue, including the failed MFA path through TwoFactorySignInAsync. Determine the expected handling for an effectively indefinite lockout and add coverage for the overflow case; done means the lockout is stored without an overflow exception.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
authentication
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.