PasswordValidator should not throw when password is null

[vjacquet]

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

The sole purpose of IPasswordValidator is to validate passwords.

The contract allows null password
Task ValidateAsync(UserManager manager, TUser user, string? password);

The implementation of PasswordValidator handle null password correctly:

aspnetcore/src/Identity/Extensions.Core/src/PasswordValidator.cs

Line 46 in 3f1acb5

if (string.IsNullOrWhiteSpace(password) || password.Length < options.RequiredLength)

So why does it throw when the password is null ?

aspnetcore/src/Identity/Extensions.Core/src/PasswordValidator.cs

Lines 40 to 42 in 3f1acb5

	public virtual Task<IdentityResult> ValidateAsync(UserManager<TUser> manager, TUser user, string? password)
	{
	ArgumentNullThrowHelper.ThrowIfNull(password);

Expected Behavior

Not throw an ArgumentNullException when the password is null, because the contract setup by the IPasswordValidator.ValidateAsync says it is allowed.

Steps To Reproduce

The following test should pass instead of failing with ArgumentNullException

    [Fact]
    public async Task ValidateDoesNotThrowsWithNullPasswordTest()
    {
        // Setup
        var validator = new PasswordValidator<PocoUser>();

        // Act
        var result = await validator.ValidateAsync(default!, default!, null);

        // Assert
        Assert.False(result.Succeeded);
    }

Exceptions (if any)

No response

.NET Version

8.0.400

Anything else?

No response

[halter73]

Thanks for the report @vjacquet. The nullable annotation here is indeed wrong. We're considering a PR (#58168) that would treat a null password as an empty string and report that it's "too short."

The main alternative would be to fix the annotation to mark the parameter as non-nullable and keep the behavior the same as it is today. Do you have a preference for how we fix this?

[vjacquet]

Thanks for your reply.

I prefer the first fix because I do not think the annotation was wrong, because the method is to validate the password, whatever value it has. And, as mentioned, you are already considering null string as too short in line 46, but line 42 prevents this test to ever happen.

Because of this exception, we test the password against null 3 times

• one by the caller, to prevent the exception
• one on line 42, to throw or not
• one on line 46, where it actually matter

Supporting null password in the signature also have a use case. Consider the following method that returns the password complexity requirements:

  public static async Task<List<IdentityError>> GetPasswordRulesAsync<TUser>(this UserManager<TUser> userManager, TUser user)
      where TUser : class
  {
      var result = new List<IdentityError>();
      foreach (var validator in userManager.PasswordValidators)
      {
          var validation = await validator.ValidateAsync(userManager, user, null);
          result.AddRange(validation.Errors);
      }
      return result;
  }

[halter73]

Thanks for the feedback. We went with the first fix which allows null.

Supporting null password in the signature also have a use case. Consider the following method that returns the password complexity requirements:

If you need to do this prior to .NET 10, passing in an empty string rather than null will give you the same behavior.