0

I am using Membership.GeneratePassword() to generate a random password. Sometimes the returned value doesn't match needed validation requirements. A user can also supply a password, hence the validation requirements.

I basically want to check the returned password, if it doesn't contain an uppercase letter, and lowercase letter, and a digit then generate another one.

Sometimes my check will return true, when it obviously isn't.

I have attempted RegEx and some LINQ as so:

string generatePassword()
{
    password =  System.Web.Security.Membership.GeneratePassword(16, 8);

    if (password.Any(c => char.IsUpper(c)) && password.Any(c => char.IsDigit(c)) && password.Any(c => char.IsLower(c)))
    {
        return password;
    }
    else
    {
       generatePassword();
    }
}


void test() {
    var pattern = @"^(?=.*[a-z])(?=.*[A-Z])(?=.*[0-9])(?=.*[!@#$%^&*()_\-+=[{\]};:<>|.\/\?])(?=.{8,})";

    var count = 0;
    for (var i = 0; i < 10000; i++)
    {
        var password = generatePassword();
        if (!Regex.Match(password, pattern).Success)
        {
             Console.WriteLine(password);
             count++;
        }
    }

    Console.WriteLine("There were {0} non matches", count);
}

In my mind, the above should always return a valid password. I have a weird scenario wherein it will return a password that doesn't have a digit, so the check fails, but if I do the same passowrd.Any(char.IsDigit) inside of test against the returned password, it'll say true.

If it helps, the password needs to be at least 8 digits, contain an uppercase, lowercase, digit and a special char: !@#$%^&*()_-+=[{]};:<>|./?

Thanks.

1 Answers1

2

Maybe your regex is a valid one, but I think it is really hard to understand what it does.

Consider using the following code, which has the advantage that it will only enumerate your password until it has detected that it is valid. Besides it is easy to understand, easy to make little changes, like size should be at least 12 characters, or it doesn't have to have an uppercase character anymore, or it should contain two digits, ...

I'll program it as extension methods of string. If you decide that your password isn't a string anymore, then changes will be minimal. See extension methods demystified

static bool IsSpecialChar(this char c)
{
    const string specialChars = "!@#$%^&*()_-+=[{]};:<>|./?";
    return specialChars.Contains(c);
}

static bool IsValidPassword(this string password)
{
    // the password needs to be at least 8 characters,
    // contain an uppercase, lowercase, digit and a special char

    // TODO: exception if password == null;
    if (password.Length < 8) return false;

    bool uppercaseDetected = false;
    bool lowercaseDetected = false;
    bool digitDetected = false;
    bool specialCharDetected = false;

    using (IEnumerator<char> enumerator = password.GetEnumerator())
    {
        while (enumerator.MoveNext
           && !(uppercaseDetected && lowercaseDetected 
                 && digitDetected && specialCharDetected))
        {
            // a character is available, and we still don't know if it is a proper password
            char c = enumerator.Current;
            uppercaseDetected = uppercaseDetected || Char.IsUpperCase(c);
            lowercaseDetected = lowercaseDetected || Char.IsLowerCase(c);
            digitDetected = digitDetected || Char.IsDigit(c);
            specialCharDetected = specialCharDetected || c.IsSpecialChar();
        }

        return uppercaseDetected && lowercaseDetected 
            && digitDetected && specialCharDetected;
    }
}

Usage:

string GeneratePassword()
{
    string proposedPassword = ...;
    while (!proposedPassword.IsValidPassword())
    {
        proposedPassword = ...;
    }
}

To make sure that it ends, you could add a counter, and stop after 1000 tries; or just add a random Uppercase / LowerCase / SpecialChar

Harald Coppoolse
  • 28,834
  • 7
  • 67
  • 116