IdentityServer / IdentityServer2

[deprecated] Thinktecture IdentityServer is a light-weight security token service built with .NET 4.5, MVC 4, Web API and WCF.
Other
409 stars 291 forks source link

ProviderUserManagementRepository PasswordStrengthRegularExpression not working #345

Closed nestor2878 closed 11 years ago

nestor2878 commented 11 years ago

On line 170 of the repository the Regex.IsMatch method is being used backwards.

It accepts the input and then the pattern, but here it's being passed in the regex as the input and the password as the regex so this never works.

if (!String.IsNullOrWhiteSpace(provider.PasswordStrengthRegularExpression) &&
                !System.Text.RegularExpressions.Regex.IsMatch(provider.PasswordStrengthRegularExpression, newPassword))
    {
        throw new ValidationException(String.Format("Password does not match the regular expression {0}", provider.PasswordStrengthRegularExpression));
    }
nestor2878 commented 11 years ago

I also had some general questions about that method. Why is it doing so much manual checking of the password for validation when the MembershipUser's ChangePassword method checks all of that?

I also noticed there's not method to validate a password change based on the old password.

Something along the lines of this.

public void SetPassword(string userName, string oldPassword, string newPassword)
        {
            if (!Membership.ValidateUser(userName, oldPassword))
            {
                throw new ArgumentException(string.Format("User and password combination not found"));
            }

            var user = Membership.GetUser(userName);

            if (user != null)
            {
                user.ChangePassword(oldPassword, newPassword);
            }
        }
jd4u commented 11 years ago

Yes, there is scope to separate Password Strength validation from the SetPassword function, as suggested in second post.

Yes, the first post is pointing to a bug. The first param of IsMatch must be the input to validate and second param must be pattern. Correct: !System.Text.RegularExpressions.Regex.IsMatch(password, provider.PasswordStrengthRegularExpression)

jd4u commented 11 years ago

Pull request is created for the above bug fix.

brockallen commented 11 years ago

Issue is fixed from this PR: https://github.com/thinktecture/Thinktecture.IdentityServer.v2/pull/425

As for why we're checking, It was a while ago, so I forget the real reason. My recollection was that we weren't getting good messages for the admin page when setting passwords.