Yii captcha fails mid-action

As the title suggests, along Sitecore I’ve done some Yii (PHP) development lately.

On one of these Yii projects I noticed a rather strange behavior with the default captcha action. The weirdness is related to a registration form, a standard registration form where a controller is validating and saving an AR model, nothing fancy. Here’s some pseudo:

if($model->validate())
{
    $model->verifyPassword=$model->password=Bcrypt->password_hash($model->password);
    $model->status=User::STATUS_NOTACTIVE;
    $model->randomizeActivationKey();
    if ($model->save())
    {
        etc...
    }
}

Right, so from time to time I get a client-side error saying the captcha text is incorrect, when I know for a fact it’s not. Additionally, and this is what caught my eye, I also get the hashed password appearing in the password field. Instead of 8 dots, I see 60. This led me to believe that the save() method is the one setting the error, and of course that’s what’s actually happening. I knew that save() will call validate() but I did not know that captcha gets refreshed on validate() after a number of calls, three as is the default.

So if you validate your captcha three times, the forth will fail. As a result, when I input the captcha incorrectly twice and correctly the third time, the call to validate() will work but save() will fail.

The good news is that save() comes with default parameters, one of which is $runValidation=true. For my problem then, the easiest fix would be:

    if ($model->save(false))
    {
        etc...
    }

There’s another fix I can think of, that would be for me to extend the captcha action, override the validate() method and increment the validationCount only when validation fails. Which, I would think was the intended result anyway.

public function validate($input,$caseSensitive)
{
	$code = $this->getVerifyCode();
	$valid = $caseSensitive ? ($input === $code) : strcasecmp($input,$code)===0;
	if(!$valid)
	{
		$session = Yii::app()->session;
		$session->open();
		$name = $this->getSessionKey() . 'count';
		$session[$name] = $session[$name] + 1;
		if($session[$name] > $this->testLimit && $this->testLimit > 0)
			$this->getVerifyCode(true);	
	}
	return $valid;
}

With this approach I have to reset the captcha manually, after the user completes the registration process. Otherwise captcha might never reset, since it may have never been invalid.

The second revision looks like this, and I have dropped all that validationCount stuff. If the input is invalid, refresh captcha and be done. Further, this will more easily allow for ajax validation, because once validated the captcha does not refresh anymore. On the other side, that being the server-side, if the user has an error in the email field but he got captcha right, the latter will not refresh and only one error occurs, so it’s win-win result. The only downside I see at the moment is having to manually call reset.

$this->createAction('captcha')->reset();
class CaptchaAction extends CCaptchaAction
{
    public function validate($input,$caseSensitive)
    {
        $code = $this->getVerifyCode();
        $valid = $caseSensitive ? ($input === $code) : strcasecmp($input,$code)===0;

        if(!$valid)
            $this->getVerifyCode(true);

        return $valid;
    }

    public function reset()
    {
        $this->getVerifyCode(true);
    }
}

This has not been tested, not even a little bit.

Leave a Reply