Page 1 of 1

Password is invalid

Posted: Wed Dec 31, 2008 5:28 pm
by Cirdan
Okay, I have some code to make sure the user is entering valid data. I have this code that checks to make sure the user only enters letters, numbers,-,_ and spaces.

Code: Select all

function validateTextString($text)
{
    return preg_match('/^[a-zA-Z0-9\s\-_]+$/', $text) ? true : false;
}
 
For some reason when I give it the password 'test' it fails saying that it is invalid.

EDIT, in the code below, shouldn't the code stop executing once it hits a return? Because the code below doesn't...
 

Code: Select all

function validatePassword($password)
{
     if(!validateTextString($password))
        return ERROR_PASSWORD_INVALID;
 
     if(!checkLength($password, MIN_PASSWORD_LENGTH, MAX_PASSWORD_LENGTH))
        return ERROR_PASSWORD_LENGTH;
 
     return true;
}
 
I echoed the password here before before the switch to make sure that was fine, and it is.

Code: Select all

      switch(validatePassword($password))
        {
            case ERROR_PASSWORD_INVALID:
                $error[] = 'Password is invalid';
                break;
            case ERROR_PASSWORD_LENGTH:
                $error[] = 'Invalid password length';
                break;
            default:
                $password = sha1($password);
 
                // Check to see if the password is correct
                if(!checkPassword($username, $password))
                    $error[] = 'Incorrect password';
 
                // Check to see if the account is activated
                if(!isActivated($username))
                    $error[] = 'Account has not been activated';
                
        }
On a side note, I just realized I should allow other characters to let people make their password more secure.

Re: Password is invalid

Posted: Wed Dec 31, 2008 6:14 pm
by requinix
validatePassword is fine, it's the switch that's the problem.

switch compares loosely, meaning true and a positive number will match.
Since the function returns true, and the first case you have uses a positive number, there's a match and you get the invalid password notice.

Two ways to fix it:
1) If your functions return error codes (as opposed to true/false) then they should return 0 on success and not-zero on failure. You can't mix-and-match numbers with booleans.
2) Use an if structure instead

Code: Select all

$validate = validatePassword($password);
if ($validate === ERROR_PASSWORD_INVALID) // three =s means a strict comparison
    $error[] = 'Password is invalid';
else if ($validate === ERROR_PASSWORD_LENGTH)
    $error[] = 'Invalid password length';
else
{

Also on a side note, why do you care what people put in their passwords? You should be hashing the password (mostly for security) - it doesn't matter what the password actually is so long as the hashes match.