2

I want to create a filter for my add, update, and delete actions in my controllers to automatically check if they

  1. were called in a POST, as opposed to GET or some other method
  2. and have the pageInstanceIDs which I set in the forms on my views
    • protects against xss
    • protects against double submission of a form
      • from submit button double click
      • from back button pressed after a submision
      • from a url being saved or bookmarked

Currently I extended \lithium\action\Controller using an AppController and have my add, update, and delete actions defined in there. I also have a boolean function in my AppController that checks if the appropriate pageInstanceIDs are in session or not.

Below is my code:

public function isNotPostBack() {

    // pull in the session
    $pageInstanceIDs = Session::read('pageInstanceIDs');
    $pageInstanceID = uniqid('', true);
    $this->set(compact('pageInstanceID'));
    $pageInstanceIDs[] = $pageInstanceID;
    Session::write('pageInstanceIDs', $pageInstanceIDs);

    // checks if this is a save operation
    if ($this->request->data){

        $pageInstanceIDs = Session::read('pageInstanceIDs');
        $pageIDIndex = array_search($this->request->data['pageInstanceID'], $pageInstanceIDs);

        if ($pageIDIndex !== false) {
            // remove the key
            unset($pageInstanceIDs[$pageIDIndex]);
            Session::write('pageInstanceIDs', $pageInstanceIDs);

            return true;

        }
        else
            return false;

    } else {
        return true;
    }

}

public function add() {
    if (!$this->request->is('post') && exist($this->request->data())) {
        $msg = "Add can only be called with http:post.";
        throw new DispatchException($msg);
    }

}

Then in my controllers I inherit from AppController and implement the action like so:

public function add() {
    parent::add();
    if (parent::isNotPostBack()){
        //do work

    }
    return $this->render(array('layout' => false));

}

which will ensure that the form used a POST and was not double submitted (back button or click happy users). This also helps protect against XSS.

I'm aware there is a plugin for this, but I want to implement this as a filter so that my controller methods are cleaner. Implented this way, the only code in my actions are the //do work portion and the return statement.

ton.yeung
  • 4,793
  • 6
  • 41
  • 72

4 Answers4

2

You should probably start with a filter on lithium\action\Dispatcher::run() here is some pseudo code. Can't help too much without seeing your parent::isNotPostBack() method but this should get you on the right track.

<?php
use lithium\action\Dispatcher;

Dispatcher::applyFilter('run', function($self, $params, $chain) {
    $request = $params['request'];

    // Request method is in $request->method
    // Post data is in $request->data

    if($not_your_conditions) {
        return new Response(); // set up your custom response
    }

    return $chain->next($self, $params, $chain); // to continue on the path of execution
});
Rob
  • 12,659
  • 4
  • 39
  • 56
2

First of all, use the integrated CSRF (XSRF) protection.

The RequestToken class creates cryptographically-secure tokens and keys that can be used to validate the authenticity of client requests.

http://li3.me/docs/lithium/security/validation/RequestToken

Check the CSRF token this way:

if ($this->request->data && !RequestToken::check($this->request)) {
    /* do your stuff */
}

You can even check the HTTP method used via is()

$this->request->is('post');

The problem of filters (for that use case) is that they are very generic. So if you don't want to write all your actions as filterable code (which might be painful and overkill), you'll have to find a way to define which method blocks what and filter the Dispatcher::_call.

botero
  • 598
  • 2
  • 11
  • 23
greut
  • 4,305
  • 1
  • 30
  • 49
  • I actually spent a few hours trying to get the integrated CRLF protection to work with session. The code I'm using now has been added to the OP. The generic nature of filters is the reason why I like the approach. Unless I misunderstood, I can filter for specific methods(actions: add, delete, update). Which are uniform in what they do, at least in my case – ton.yeung Mar 21 '12 at 17:14
  • I've added some clarification to the OP: the CRLF part is important, but preventing double submits on my form is also important. – ton.yeung Mar 21 '12 at 21:38
  • two tips to avoid double submit: once submitted, you freeze the form using javascript; every post action (that you don't want being double submitted) finishes with a redirection. – greut Mar 21 '12 at 22:52
  • PRG doesn't address the back button. Please see http://en.wikipedia.org/wiki/Post/Redirect/Get – ton.yeung Mar 22 '12 at 01:00
0

I implemented something similar in a recent project by subclassing \lithium\action\Controller as app\controllers\ApplicationController (abstract) and applying filters to invokeMethod(), as that's how the dispatcher invokes the action methods. Here's the pertinent chunk:

namespace app\controllers;

class ApplicationController extends \lithium\action\Controller {
    /**
     * Essential because you cannot invoke `parent::invokeMethod()` from within the closure passed to `_filter()`... But it makes me sad.
     *
     * @see \lithium\action\Controller::invokeMethod()
     *
     * @param string $method to be invoked with $arguments
     * @param array $arguments to pass to $method
     */
    public function _invokeMethod($method, array $arguments = array()) {
        return parent::invokeMethod($method, $arguments);
    }

    /**
     * Overridden to make action methods filterable with `applyFilter()`
     *
     * @see \lithium\action\Controller::invokeMethod()
     * @see \lithium\core\Object::applyFilter()
     *
     * @param string $method to be invoked with $arguments
     * @param array $arguments to pass to $method
     */
    public function invokeMethod($method, array $arguments = array()) {
        return $this->_filter(__METHOD__, compact('method', 'arguments'), function($self, $params){
            return $self->_invokeMethod($params['method'], $params['arguments']);
        });
    }
}

Then you can use applyFilter() inside of _init() to run filters on your method. Instead of checking $method in every filter, you can opt to change _filter(__METHOD__, . . .) to _filter($method, . . .), but we chose to keep the more generic filter.

AL the X
  • 1,004
  • 11
  • 15
0

For CSRF protection, I use something similar to greut's suggestion.

I have this in my extensions/action/Controller.php

protected function _init() {
    parent::_init();

    if ($this->request->is('post') ||
        $this->request->is('put') ||
        $this->request->is('delete')) {
        //on add, update and delete, if the security token exists, we will verify the token
        if ('' != Session::read('security.token') && !RequestToken::check($this->request)) {
            RequestToken::get(array('regenerate' => true));
            throw new DispatchException('There was an error submitting the form.');
        }
    }
}

Of course, this means you'd have to also add the following to the top of your file:

use \lithium\storage\Session;
use lithium\security\validation\RequestToken;
use lithium\action\DispatchException;

With this, I don't have to repeatedly check for CSRF.

Housni
  • 963
  • 1
  • 10
  • 23
  • True, but I don't think you can check if this is a form resubmit either. – ton.yeung Mar 21 '12 at 21:33
  • Well you could just generate and set the unique page instance ID's in the child controller and do your validation within the parent controller (i.e. the one I've posted, above), inside the _init() method. – Housni Mar 21 '12 at 22:20
  • sorry, I didn't realize that you also used a parent controller. I've edited my original post to show what I have, and its pretty close to what you have. I want to move away from this method and use a filter on my controller though, similar to ASP.NET MVC. Its much cleaner and more manageable. – ton.yeung Mar 21 '12 at 22:33