Allow password validation failures to propogate (#6034)

src/Identity/Extensions.Core/src/UserManager.cs

@@ -2517,15 +2517,21 @@ namespace Microsoft.AspNetCore.Identity
         protected async Task<IdentityResult> ValidatePasswordAsync(TUser user, string password)
         {
             var errors = new List<IdentityError>();
+            var isValid = true;
             foreach (var v in PasswordValidators)
             {
                 var result = await v.ValidateAsync(this, user, password);
                 if (!result.Succeeded)
                 {
-                    errors.AddRange(result.Errors);
+                    if (result.Errors.Any())
+                    {
+                        errors.AddRange(result.Errors);
+                    }
+
+                    isValid = false;
                 }
             }
-            if (errors.Count > 0)
+            if (!isValid)
             {
                 Logger.LogWarning(14, "User {userId} password validation failed: {errors}.", await GetUserIdAsync(user), string.Join(";", errors.Select(e => e.Code)));
                 return IdentityResult.Failed(errors.ToArray());

src/Identity/Specification.Tests/src/IdentityResultAssert.cs

@@ -44,12 +44,15 @@ namespace Microsoft.AspNetCore.Identity.Test
         /// <summary>
         /// Asserts that the result has not Succeeded and that first error matches error's code and Description.
         /// </summary>
-        public static void IsFailure(IdentityResult result, IdentityError error)
+        public static void IsFailure(IdentityResult result, IdentityError error = null)
         {
             Assert.NotNull(result);
             Assert.False(result.Succeeded);
-            Assert.Equal(error.Description, result.Errors.First().Description);
+            if (error != null)
-            Assert.Equal(error.Code, result.Errors.First().Code);
+            {
+                Assert.Equal(error.Description, result.Errors.FirstOrDefault()?.Description);
+                Assert.Equal(error.Code, result.Errors.FirstOrDefault()?.Code);
+            }
         }
 
         /// <summary>

src/Identity/Specification.Tests/src/UserManagerSpecificationTests.cs

@@ -163,6 +163,39 @@ namespace Microsoft.AspNetCore.Identity.Test
             }
         }
 
+        private class EmptyBadValidator : IUserValidator<TUser>,
+            IPasswordValidator<TUser>
+        {
+            public Task<IdentityResult> ValidateAsync(UserManager<TUser> manager, TUser user, string password)
+            {
+                return Task.FromResult(IdentityResult.Failed());
+            }
+
+            public Task<IdentityResult> ValidateAsync(UserManager<TUser> manager, TUser user)
+            {
+                return Task.FromResult(IdentityResult.Failed());
+            }
+        }
+
+        /// <summary>
+        /// Test.
+        /// </summary>
+        /// <returns>Task</returns>
+        [Fact]
+        public async Task PasswordValidatorWithNoErrorsCanBlockAddPassword()
+        {
+            if (ShouldSkipDbTests())
+            {
+                return;
+            }
+            var manager = CreateManager();
+            var user = CreateTestUser();
+            IdentityResultAssert.IsSuccess(await manager.CreateAsync(user));
+            manager.PasswordValidators.Clear();
+            manager.PasswordValidators.Add(new EmptyBadValidator());
+            IdentityResultAssert.IsFailure(await manager.AddPasswordAsync(user, "password"));
+        }
+
         /// <summary>
         /// Test.
         /// </summary>
@@ -497,13 +530,78 @@ namespace Microsoft.AspNetCore.Identity.Test
             }
             var manager = CreateManager();
             manager.PasswordValidators.Clear();
-            manager.PasswordValidators.Add(new AlwaysBadValidator());
+            manager.PasswordValidators.Add(new EmptyBadValidator());
             manager.PasswordValidators.Add(new AlwaysBadValidator());
             var user = CreateTestUser();
             IdentityResultAssert.IsSuccess(await manager.CreateAsync(user));
             var result = await manager.AddPasswordAsync(user, "pwd");
             IdentityResultAssert.IsFailure(result, AlwaysBadValidator.ErrorMessage);
-            Assert.Equal(2, result.Errors.Count());
+            Assert.Single(result.Errors);
+        }
+
+        /// <summary>
+        /// Test.
+        /// </summary>
+        /// <returns>Task</returns>
+        [Fact]
+        public async Task PasswordValidatorWithNoErrorsCanBlockChangePassword()
+        {
+            if (ShouldSkipDbTests())
+            {
+                return;
+            }
+            var manager = CreateManager();
+            var user = CreateTestUser();
+            IdentityResultAssert.IsSuccess(await manager.CreateAsync(user, "password"));
+            manager.PasswordValidators.Clear();
+            manager.PasswordValidators.Add(new AlwaysBadValidator());
+            IdentityResultAssert.IsFailure(await manager.ChangePasswordAsync(user, "password", "new"));
+        }
+
+        /// <summary>
+        /// Test.
+        /// </summary>
+        /// <returns>Task</returns>
+        [Fact]
+        public async Task PasswordValidatorWithNoErrorsCanBlockCreateUser()
+        {
+            if (ShouldSkipDbTests())
+            {
+                return;
+            }
+            var manager = CreateManager();
+            var user = CreateTestUser();
+            manager.PasswordValidators.Clear();
+            manager.PasswordValidators.Add(new AlwaysBadValidator());
+            IdentityResultAssert.IsFailure(await manager.CreateAsync(user, "password"));
+        }
+
+        /// <summary>
+        /// Test.
+        /// </summary>
+        /// <returns>Task</returns>
+        [Fact]
+        public async Task PasswordValidatorWithNoErrorsCanBlockResetPasswordWithStaticTokenProvider()
+        {
+            if (ShouldSkipDbTests())
+            {
+                return;
+            }
+            var manager = CreateManager();
+            manager.RegisterTokenProvider("Static", new StaticTokenProvider());
+            manager.Options.Tokens.PasswordResetTokenProvider = "Static";
+            var user = CreateTestUser();
+            const string password = "password";
+            const string newPassword = "newpassword";
+            IdentityResultAssert.IsSuccess(await manager.CreateAsync(user, password));
+            var stamp = await manager.GetSecurityStampAsync(user);
+            Assert.NotNull(stamp);
+            var token = await manager.GeneratePasswordResetTokenAsync(user);
+            Assert.NotNull(token);
+            manager.PasswordValidators.Add(new AlwaysBadValidator());
+            IdentityResultAssert.IsFailure(await manager.ResetPasswordAsync(user, token, newPassword));
+            Assert.True(await manager.CheckPasswordAsync(user, password));
+            Assert.Equal(stamp, await manager.GetSecurityStampAsync(user));
         }
 
         /// <summary>

src/Identity/test/Identity.Test/UserManagerTest.cs

@@ -982,11 +982,21 @@ namespace Microsoft.AspNetCore.Identity.Test
             // TODO: Can switch to Mock eventually
             var manager = MockHelpers.TestUserManager(new EmptyStore());
             manager.PasswordValidators.Clear();
-            manager.PasswordValidators.Add(new BadPasswordValidator<PocoUser>());
+            manager.PasswordValidators.Add(new BadPasswordValidator<PocoUser>(true));
             IdentityResultAssert.IsFailure(await manager.CreateAsync(new PocoUser(), "password"),
                 BadPasswordValidator<PocoUser>.ErrorMessage);
         }
 
+        [Fact]
+        public async Task PasswordValidatorWithoutErrorsBlocksCreate()
+        {
+            // TODO: Can switch to Mock eventually
+            var manager = MockHelpers.TestUserManager(new EmptyStore());
+            manager.PasswordValidators.Clear();
+            manager.PasswordValidators.Add(new BadPasswordValidator<PocoUser>());
+            IdentityResultAssert.IsFailure(await manager.CreateAsync(new PocoUser(), "password"));
+        }
+
         [Fact]
         public async Task ResetTokenCallNoopForTokenValueZero()
         {
@@ -1175,10 +1185,22 @@ namespace Microsoft.AspNetCore.Identity.Test
         {
             public static readonly IdentityError ErrorMessage = new IdentityError { Description = "I'm Bad." };
 
-            public Task<IdentityResult> ValidateAsync(UserManager<TUser> manager, TUser user, string password)
+            private IdentityResult badResult;
+
+            public BadPasswordValidator(bool includeErrorMessage = false)
             {
-                return Task.FromResult(IdentityResult.Failed(ErrorMessage));
+                if (includeErrorMessage)
+                {
+                    badResult = IdentityResult.Failed(ErrorMessage);
+                }
+                else
+                {
+                    badResult = IdentityResult.Failed();
+                }
             }
+
+            public Task<IdentityResult> ValidateAsync(UserManager<TUser> manager, TUser user, string password)
+                => Task.FromResult(badResult);
         }
 
         private class EmptyStore :