Merge branch 'release/2.2'

src/Identity/SignInManager.cs

@@ -89,7 +89,8 @@ namespace Microsoft.AspNetCore.Identity
         /// <summary>
         /// The <see cref="HttpContext"/> used.
         /// </summary>
-        public HttpContext Context { 
+        public HttpContext Context
+        {
             get
             {
                 var context = _context ?? _contextAccessor?.HttpContext;
@@ -257,7 +258,7 @@ namespace Microsoft.AspNetCore.Identity
         /// <param name="securityStamp">The expected security stamp value.</param>
         /// <returns>True if the stamp matches the persisted value, otherwise it will return false.</returns>
         public virtual async Task<bool> ValidateSecurityStampAsync(TUser user, string securityStamp)
-            => user != null && UserManager.SupportsUserSecurityStamp 
+            => user != null && UserManager.SupportsUserSecurityStamp
             && securityStamp == await UserManager.GetSecurityStampAsync(user);
 
         /// <summary>
@@ -279,7 +280,7 @@ namespace Microsoft.AspNetCore.Identity
             }
 
             var attempt = await CheckPasswordSignInAsync(user, password, lockoutOnFailure);
-            return attempt.Succeeded 
+            return attempt.Succeeded
                 ? await SignInOrTwoFactorAsync(user, isPersistent)
                 : attempt;
         }
@@ -330,7 +331,13 @@ namespace Microsoft.AspNetCore.Identity
 
             if (await UserManager.CheckPasswordAsync(user, password))
             {
-                await ResetLockout(user);
+                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));
@@ -534,7 +541,7 @@ namespace Microsoft.AspNetCore.Identity
         /// <param name="isPersistent">Flag indicating whether the sign-in cookie should persist after the browser is closed.</param>
         /// <returns>The task object representing the asynchronous operation containing the <see name="SignInResult"/>
         /// for the sign-in attempt.</returns>
-        public virtual Task<SignInResult> ExternalLoginSignInAsync(string loginProvider, string providerKey, bool isPersistent) 
+        public virtual Task<SignInResult> ExternalLoginSignInAsync(string loginProvider, string providerKey, bool isPersistent)
             => ExternalLoginSignInAsync(loginProvider, providerKey, isPersistent, bypassTwoFactor: false);
 
         /// <summary>
@@ -645,7 +652,7 @@ namespace Microsoft.AspNetCore.Identity
 
             return IdentityResult.Success;
         }
-        
+
         /// <summary>
         /// Configures the redirect URL and user identifier for the specified external login <paramref name="provider"/>.
         /// </summary>
@@ -708,7 +715,12 @@ namespace Microsoft.AspNetCore.Identity
             }
             return identity;
         }
-        
+
+        private async Task<bool> IsTfaEnabled(TUser user)
+            => UserManager.SupportsUserTwoFactor &&
+            await UserManager.GetTwoFactorEnabledAsync(user) &&
+            (await UserManager.GetValidTwoFactorProvidersAsync(user)).Count > 0;
+
         /// <summary>
         /// Signs in the specified <paramref name="user"/> if <paramref name="bypassTwoFactor"/> is set to false.
         /// Otherwise stores the <paramref name="user"/> for use after a two factor check.
@@ -720,10 +732,7 @@ namespace Microsoft.AspNetCore.Identity
         /// <returns>Returns a <see cref="SignInResult"/></returns>
         protected virtual async Task<SignInResult> SignInOrTwoFactorAsync(TUser user, bool isPersistent, string loginProvider = null, bool bypassTwoFactor = false)
         {
-            if (!bypassTwoFactor &&
+            if (!bypassTwoFactor && await IsTfaEnabled(user))
-                UserManager.SupportsUserTwoFactor &&
-                await UserManager.GetTwoFactorEnabledAsync(user) &&
-                (await UserManager.GetValidTwoFactorProvidersAsync(user)).Count > 0)
             {
                 if (!await IsTwoFactorClientRememberedAsync(user))
                 {

test/Identity.Test/SignInManagerTest.cs

@@ -266,6 +266,64 @@ namespace Microsoft.AspNetCore.Identity.Test
             auth.Verify();
         }
 
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public async Task CheckPasswordOnlyResetLockoutWhenTfaNotEnabled(bool tfaEnabled)
+        {
+            // Setup
+            var user = new PocoUser { 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 PocoUser { 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)]
@@ -285,10 +343,7 @@ namespace Microsoft.AspNetCore.Identity.Test
             manager.Setup(m => m.SupportsUserTwoFactor).Returns(true).Verifiable();
             manager.Setup(m => m.GetTwoFactorEnabledAsync(user)).ReturnsAsync(true).Verifiable();
             manager.Setup(m => m.CheckPasswordAsync(user, "password")).ReturnsAsync(true).Verifiable();
-            if (supportsLockout)
+            manager.Setup(m => m.GetValidTwoFactorProvidersAsync(user)).ReturnsAsync(new string[1] { "Fake" }).Verifiable();
-            {
-                manager.Setup(m => m.ResetAccessFailedCountAsync(user)).ReturnsAsync(IdentityResult.Success).Verifiable();
-            }
             var context = new DefaultHttpContext();
             var helper = SetupSignInManager(manager.Object, context);
             var auth = MockAuth(context);