Phalcon CSRF in self submit form

Hi,

Could you try change your csrf element to this:

    {{ element.render('csrf', ['name': this.security.getTokenKey(), 'value': this.security.getToken()]) }}

And change your form element to this:

        $csrf->addValidator(
            new Identical([
            $this->security->checkToken() => 1,
            'message' => $this->translation->_('This request was aborted because it appears to be forged')
        ]));

Ah, I have not seen your edited comment.

I will try that.

Thanks.

Hi,

Could you try change your csrf element to this:

  {{ element.render('csrf', ['name': this.security.getTokenKey(), 'value': this.security.getToken()]) }}

And change your form element to this:

      $csrf->addValidator(
          new Identical([
          $this->security->checkToken() => 1,
          'message' => $this->translation->_('This request was aborted because it appears to be forged')
      ]));

Thank you Warwiks, tricky and sweet. It works.

Maybe I sould read source more than documentaions!

Regards

No problem at all, glad you found your solution. If you have any questions then please ask.

Sorry, I accidentally accept my comment so I delete it.

@Warosaurus last comment is the answer!

@Warosaurus Can you explain little more, how this setup works? Seams that it just bypas validator when I change session value manually and always returns true ! . (I record sessions in DB)

Thanks again.

No problem at all, glad you found your solution. If you have any questions then please ask.

I just tested and you're right. It does validate incorrect values.

I switched back to an older solution. Remove the validator and instead check the token in your controller. This is my controller now:

        $form = new AccountLoginForm();

        $this->view->form = $form;

        if ($this->request->isPost()) {

            if ($this->security->checkToken()) { // <- CSRF check

I just tested this changing the CSRF token in my session variable and it works. Let me know if it works for you.

Thanks for the heads up.

@Warosaurus you know what? $this->security->checkToken() always works but implementing OOP CSRF validator in form does not!

It's a big issue when you have many forms and I think that it's not controller responsibility to check form validation if we have \Phalcon\Form\Form abstract layer!

It back to bad code structure in validator so all problems related to these lines (https://github.com/phalcon/cphalcon/blob/master/phalcon/validation.zep#L112) When we define a field the first param picks as field name (not input name) and all validations use that name to retrives values then we cannot use dynamic field name to retrive value in CSRF check at all.

I implement this CSRF custom validator and it works for me:

Validator\Csrf.php

namespace Application\Validator;

use Phalcon\Validation;
use Phalcon\Validation\Message;
use Phalcon\Validation\Validator;
use Phalcon\Validation\ValidatorInterface;

class Csrf extends Validator implements ValidatorInterface
{
    const DEFAULT_MESSAGE = "CSRF Attack: This request was aborted!";

    /**
     * Executes the validation
     *
     * @param Validation $validator
     * @param string $field
     * @return boolean
     */
    public function validate(Validation $validator, $field)
    {
        $this->setOption('cancelOnFail', true);

        if(!$validator->security->checkToken())
        {
            $message = $this->getOption("message");
            if (empty($message)) {
                $message = $validator->translation->_(self::DEFAULT_MESSAGE);
            }

            $validator->appendMessage(new Message($message, $field, "Csrf"));
            return false;
        }

        return true;
    }
} 

Usage in Form class:

$csrf = new Hidden('csrf');
$csrf->addValidator(new Csrf(array(
'message' => $this->translation->_('This request was aborted because it appears to be forged')
)));
$this->add($csrf);

Volt Script:

{{ form.render('csrf', ['name': security.getTokenKey(), 'value': security.getToken()]) }}

Hope this helps.

Regards

Having the same issue, so thanks a lot for your workaround, but in my opinion, going OOP style is the more clean way and it should work too! Because actually it doesn't work I suppose this is a bug in Phalcon and it must be corrected.

Yes, I think so. Somthing smells wrong here :)

Having the same issue, so thanks a lot for your workaround, but in my opinion, going OOP style is the more clean way and it should work too! Because actually it doesn't work I suppose this is a bug in Phalcon and it must be corrected.

Have you filled a bug report yet?

Yes, I think so. Somthing smells wrong here :)

Having the same issue, so thanks a lot for your workaround, but in my opinion, going OOP style is the more clean way and it should work too! Because actually it doesn't work I suppose this is a bug in Phalcon and it must be corrected.

I do not think this is a bug... It seams it is a bad designed feature!

Have you filled a bug report yet?

Yes, I think so. Somthing smells wrong here :)

Having the same issue, so thanks a lot for your workaround, but in my opinion, going OOP style is the more clean way and it should work too! Because actually it doesn't work I suppose this is a bug in Phalcon and it must be corrected.

Working... In Form:

        $csrf = new Hidden('csrf');
        $csrf->addValidator(new Identical(array(
            'value' => $this->security->getSessionToken(),
            'message' => 'CSRF validation failed'
        )));
        $csrf->clear();
        $this->add($csrf);

in View:

{{ form.render('csrf', ['value': security.getToken()]) }}