Skip to content
This repository has been archived by the owner on Dec 20, 2018. It is now read-only.

Commit

Permalink
CheckPassword only reset lockout when TFA disabled
Browse files Browse the repository at this point in the history
  • Loading branch information
HaoK committed May 23, 2018
1 parent 62f0ede commit 2e9ee41
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 8 deletions.
20 changes: 14 additions & 6 deletions src/Microsoft.AspNetCore.Identity/SignInManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,14 @@ public virtual async Task<TUser> ValidateSecurityStampAsync(ClaimsPrincipal prin
}
if (await UserManager.CheckPasswordAsync(user, password))
{
await ResetLockout(user);
return await SignInOrTwoFactorAsync(user, isPersistent);
var alwaysLockout = AppContext.TryGetSwitch("Microsoft.AspNetCore.Identity.CheckPasswordSignInAlwaysResetLockoutOnSuccess", out var enabled) && enabled;
// Only reset the lockout when TFA is not enabled when not in quirks mode
if (alwaysLockout || !await IsTfaEnabled(user))
{
await ResetLockout(user);
}

return SignInResult.Success;
}
Logger.LogWarning(2, "User {userId} failed to provide the correct password.", await UserManager.GetUserIdAsync(user));

Expand Down Expand Up @@ -501,7 +507,7 @@ public virtual async Task<IdentityResult> UpdateExternalAuthenticationTokensAsyn

return IdentityResult.Success;
}

/// <summary>
/// Configures the redirect URL and user identifier for the specified external login <paramref name="provider"/>.
/// </summary>
Expand Down Expand Up @@ -552,12 +558,14 @@ private ClaimsIdentity CreateIdentity(TwoFactorAuthenticationInfo info)
return identity;
}

private async Task<bool> IsTfaEnabled(TUser user)
=> UserManager.SupportsUserTwoFactor &&
await UserManager.GetTwoFactorEnabledAsync(user) &&
(await UserManager.GetValidTwoFactorProvidersAsync(user)).Count > 0;

private async Task<SignInResult> SignInOrTwoFactorAsync(TUser user, bool isPersistent, string loginProvider = null)
{
if (UserManager.SupportsUserTwoFactor &&
await UserManager.GetTwoFactorEnabledAsync(user) &&
(await UserManager.GetValidTwoFactorProvidersAsync(user)).Count > 0)
if (!bypassTwoFactor && await IsTfaEnabled(user))
{
if (!await IsTwoFactorClientRememberedAsync(user))
{
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.AspNetCore.Identity/project.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"description": "ASP.NET Core Identity is the membership system for building ASP.NET Core web applications, including membership, login, and user data. ASP.NET Core Identity allows you to add login features to your application and makes it easy to customize data about the logged in user.",
"version": "1.0.3",
"version": "1.0.4",
"buildOptions": {
"warningsAsErrors": true,
"keyFile": "../../tools/Key.snk",
Expand Down
58 changes: 58 additions & 0 deletions test/Microsoft.AspNetCore.Identity.Test/SignInManagerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,64 @@ public async Task PasswordSignInWorksWithNonTwoFactorStore()
auth.Verify();
}

[Theory]
[InlineData(true)]
[InlineData(false)]
public async Task CheckPasswordOnlyResetLockoutWhenTfaNotEnabled(bool tfaEnabled)
{
// Setup
var user = new TestUser { UserName = "Foo" };
var manager = SetupUserManager(user);
manager.Setup(m => m.SupportsUserLockout).Returns(true).Verifiable();
manager.Setup(m => m.IsLockedOutAsync(user)).ReturnsAsync(false).Verifiable();
manager.Setup(m => m.SupportsUserTwoFactor).Returns(tfaEnabled).Verifiable();
manager.Setup(m => m.CheckPasswordAsync(user, "password")).ReturnsAsync(true).Verifiable();

if (tfaEnabled)
{
manager.Setup(m => m.GetTwoFactorEnabledAsync(user)).ReturnsAsync(true).Verifiable();
manager.Setup(m => m.GetValidTwoFactorProvidersAsync(user)).ReturnsAsync(new string[1] {"Fake"}).Verifiable();
}
else
{
manager.Setup(m => m.ResetAccessFailedCountAsync(user)).ReturnsAsync(IdentityResult.Success).Verifiable();
}

var context = new DefaultHttpContext();
var helper = SetupSignInManager(manager.Object, context);

// Act
var result = await helper.CheckPasswordSignInAsync(user, "password", false);

// Assert
Assert.True(result.Succeeded);
manager.Verify();
}

[Fact]
public async Task CheckPasswordAlwaysResetLockoutWhenQuirked()
{
AppContext.SetSwitch("Microsoft.AspNetCore.Identity.CheckPasswordSignInAlwaysResetLockoutOnSuccess", true);

// Setup
var user = new TestUser { UserName = "Foo" };
var manager = SetupUserManager(user);
manager.Setup(m => m.SupportsUserLockout).Returns(true).Verifiable();
manager.Setup(m => m.IsLockedOutAsync(user)).ReturnsAsync(false).Verifiable();
manager.Setup(m => m.CheckPasswordAsync(user, "password")).ReturnsAsync(true).Verifiable();
manager.Setup(m => m.ResetAccessFailedCountAsync(user)).ReturnsAsync(IdentityResult.Success).Verifiable();

var context = new DefaultHttpContext();
var helper = SetupSignInManager(manager.Object, context);

// Act
var result = await helper.CheckPasswordSignInAsync(user, "password", false);

// Assert
Assert.True(result.Succeeded);
manager.Verify();
}

[Theory]
[InlineData(true)]
[InlineData(false)]
Expand Down
2 changes: 1 addition & 1 deletion test/Microsoft.AspNetCore.Identity.Test/project.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"dotnet-test-xunit": "1.0.0-rc3-000000-01",
"Microsoft.AspNetCore.Hosting": "1.0.3",
"Microsoft.AspNetCore.Http": "1.0.3",
"Microsoft.AspNetCore.Identity": "1.0.3",
"Microsoft.AspNetCore.Identity": "1.0.4",
"Microsoft.AspNetCore.Testing": "1.0.1",
"Microsoft.Extensions.Configuration": "1.0.2",
"Microsoft.Extensions.Options.ConfigurationExtensions": "1.0.2",
Expand Down

0 comments on commit 2e9ee41

Please sign in to comment.