Add CSRF protections

11 files changed, 93 insertions, 58 deletions

diff --git a/app/Controller/Action.php b/app/Controller/Action.php
index 2aa85c14..11dc3b29 100644
--- a/app/Controller/Action.php
+++ b/app/Controller/Action.php
@@ -129,6 +129,7 @@ class Action extends Base
      */
     public function remove()
     {
+        $this->checkCSRFParam();
         $action = $this->action->getById($this->request->getIntegerParam('action_id'));
 
         if ($action && $this->action->remove($action['id'])) {
diff --git a/app/Controller/Base.php b/app/Controller/Base.php
index 5829fc36..9b695a82 100644
--- a/app/Controller/Base.php
+++ b/app/Controller/Base.php
@@ -3,6 +3,7 @@
 namespace Controller;
 
 use Core\Registry;
+use Core\Security;
 use Core\Translator;
 use Model\LastLogin;
 
@@ -161,6 +162,28 @@ abstract class Base
     }
 
     /**
+     * Application forbidden page
+     *
+     * @access public
+     */
+    public function forbidden()
+    {
+        $this->response->html($this->template->layout('app_forbidden', array('title' => t('Access Forbidden'))));
+    }
+
+    /**
+     * Check if the CSRF token from the URL is correct
+     *
+     * @access protected
+     */
+    protected function checkCSRFParam()
+    {
+        if (! Security::validateCSRFToken($this->request->getStringParam('csrf_token'))) {
+            $this->forbidden();
+        }
+    }
+
+    /**
      * Check if the current user have access to the given project
      *
      * @access protected
@@ -171,7 +194,7 @@ abstract class Base
         if ($this->acl->isRegularUser()) {
 
             if ($project_id > 0 && ! $this->project->isUserAllowed($project_id, $this->acl->getUserId())) {
-                $this->response->redirect('?controller=project&action=forbidden');
+                $this->forbidden();
             }
         }
     }
diff --git a/app/Controller/Board.php b/app/Controller/Board.php
index 53fdeab9..67072895 100644
--- a/app/Controller/Board.php
+++ b/app/Controller/Board.php
@@ -4,6 +4,7 @@ namespace Controller;
 
 use Model\Project as ProjectModel;
 use Model\User as UserModel;
+use Core\Security;
 
 /**
  * Board controller
@@ -20,6 +21,7 @@ class Board extends Base
      */
     public function moveUp()
     {
+        $this->checkCSRFParam();
         $project_id = $this->request->getIntegerParam('project_id');
         $column_id = $this->request->getIntegerParam('column_id');
 
@@ -35,6 +37,7 @@ class Board extends Base
      */
     public function moveDown()
     {
+        $this->checkCSRFParam();
         $project_id = $this->request->getIntegerParam('project_id');
         $column_id = $this->request->getIntegerParam('column_id');
 
@@ -344,6 +347,7 @@ class Board extends Base
      */
     public function remove()
     {
+        $this->checkCSRFParam();
         $column = $this->board->getColumn($this->request->getIntegerParam('column_id'));
 
         if ($column && $this->board->removeColumn($column['id'])) {
@@ -362,25 +366,31 @@ class Board extends Base
      */
     public function save()
     {
-        $project_id = $this->request->getIntegerParam('project_id');
-        $values = $this->request->getValues();
+        if ($this->request->isAjax()) {
 
-        if ($project_id > 0 && ! $this->project->isUserAllowed($project_id, $this->acl->getUserId())) {
-            $this->response->text('Not Authorized', 401);
-        }
+            $project_id = $this->request->getIntegerParam('project_id');
+            $values = $this->request->getValues();
 
-        if (isset($values['positions'])) {
-            $this->board->saveTasksPosition($values['positions']);
-        }
+            if ($project_id > 0 && ! $this->project->isUserAllowed($project_id, $this->acl->getUserId())) {
+                $this->response->text('Not Authorized', 401);
+            }
+
+            if (isset($values['positions'])) {
+                $this->board->saveTasksPosition($values['positions']);
+            }
 
-        $this->response->html(
-            $this->template->load('board_show', array(
-                'current_project_id' => $project_id,
-                'board' => $this->board->get($project_id),
-                'categories' => $this->category->getList($project_id, false),
-            )),
-            201
-        );
+            $this->response->html(
+                $this->template->load('board_show', array(
+                    'current_project_id' => $project_id,
+                    'board' => $this->board->get($project_id),
+                    'categories' => $this->category->getList($project_id, false),
+                )),
+                201
+            );
+        }
+        else {
+            $this->response->status(401);
+        }
     }
 
     /**
@@ -390,24 +400,30 @@ class Board extends Base
      */
     public function check()
     {
-        $project_id = $this->request->getIntegerParam('project_id');
-        $timestamp = $this->request->getIntegerParam('timestamp');
+        if ($this->request->isAjax()) {
 
-        if ($project_id > 0 && ! $this->project->isUserAllowed($project_id, $this->acl->getUserId())) {
-            $this->response->text('Not Authorized', 401);
-        }
+            $project_id = $this->request->getIntegerParam('project_id');
+            $timestamp = $this->request->getIntegerParam('timestamp');
 
-        if ($this->project->isModifiedSince($project_id, $timestamp)) {
-            $this->response->html(
-                $this->template->load('board_show', array(
-                    'current_project_id' => $project_id,
-                    'board' => $this->board->get($project_id),
-                    'categories' => $this->category->getList($project_id, false),
-                ))
-            );
+            if ($project_id > 0 && ! $this->project->isUserAllowed($project_id, $this->acl->getUserId())) {
+                $this->response->text('Not Authorized', 401);
+            }
+
+            if ($this->project->isModifiedSince($project_id, $timestamp)) {
+                $this->response->html(
+                    $this->template->load('board_show', array(
+                        'current_project_id' => $project_id,
+                        'board' => $this->board->get($project_id),
+                        'categories' => $this->category->getList($project_id, false),
+                    ))
+                );
+            }
+            else {
+                $this->response->status(304);
+            }
         }
         else {
-            $this->response->status(304);
+            $this->response->status(401);
         }
     }
 }
diff --git a/app/Controller/Category.php b/app/Controller/Category.php
index f96c1d4a..9b73f207 100644
--- a/app/Controller/Category.php
+++ b/app/Controller/Category.php
@@ -175,6 +175,7 @@ class Category extends Base
      */
     public function remove()
     {
+        $this->checkCSRFParam();
         $project = $this->getProject();
         $category = $this->getCategory($project['id']);
 
diff --git a/app/Controller/Comment.php b/app/Controller/Comment.php
index 47eaf6b6..a0a11fc8 100644
--- a/app/Controller/Comment.php
+++ b/app/Controller/Comment.php
@@ -178,6 +178,7 @@ class Comment extends Base
      */
     public function remove()
     {
+        $this->checkCSRFParam();
         $task = $this->getTask();
         $comment = $this->getComment();
 
diff --git a/app/Controller/Config.php b/app/Controller/Config.php
index b4a5b8d3..daa57790 100644
--- a/app/Controller/Config.php
+++ b/app/Controller/Config.php
@@ -76,6 +76,7 @@ class Config extends Base
      */
     public function downloadDb()
     {
+        $this->checkCSRFParam();
         $this->response->forceDownload('db.sqlite.gz');
         $this->response->binary($this->config->downloadDatabase());
     }
@@ -87,6 +88,7 @@ class Config extends Base
      */
     public function optimizeDb()
     {
+        $this->checkCSRFParam();
         $this->config->optimizeDatabase();
         $this->session->flash(t('Database optimization done.'));
         $this->response->redirect('?controller=config');
@@ -99,6 +101,7 @@ class Config extends Base
      */
     public function tokens()
     {
+        $this->checkCSRFParam();
         $this->config->regenerateTokens();
         $this->session->flash(t('All tokens have been regenerated.'));
         $this->response->redirect('?controller=config');
@@ -111,6 +114,7 @@ class Config extends Base
      */
     public function removeRememberMeToken()
     {
+        $this->checkCSRFParam();
         $this->rememberMe->remove($this->request->getIntegerParam('id'));
         $this->response->redirect('?controller=config&action=index#remember-me');
     }
diff --git a/app/Controller/File.php b/app/Controller/File.php
index 38cb82ab..3c8c32d1 100644
--- a/app/Controller/File.php
+++ b/app/Controller/File.php
@@ -111,6 +111,7 @@ class File extends Base
      */
     public function remove()
     {
+        $this->checkCSRFParam();
         $task = $this->getTask();
         $file = $this->file->getById($this->request->getIntegerParam('file_id'));
 
diff --git a/app/Controller/Project.php b/app/Controller/Project.php
index e539f364..0de67691 100644
--- a/app/Controller/Project.php
+++ b/app/Controller/Project.php
@@ -13,19 +13,6 @@ use Model\Task as TaskModel;
 class Project extends Base
 {
     /**
-     * Display access forbidden page
-     *
-     * @access public
-     */
-    public function forbidden()
-    {
-        $this->response->html($this->template->layout('project_forbidden', array(
-            'menu' => 'projects',
-            'title' => t('Access Forbidden')
-        )));
-    }
-
-    /**
      * Task search for a given project
      *
      * @access public
@@ -254,6 +241,7 @@ class Project extends Base
      */
     public function remove()
     {
+        $this->checkCSRFParam();
         $project_id = $this->request->getIntegerParam('project_id');
 
         if ($project_id && $this->project->remove($project_id)) {
@@ -272,6 +260,7 @@ class Project extends Base
      */
     public function enable()
     {
+        $this->checkCSRFParam();
         $project_id = $this->request->getIntegerParam('project_id');
 
         if ($project_id && $this->project->enable($project_id)) {
@@ -290,6 +279,7 @@ class Project extends Base
      */
     public function disable()
     {
+        $this->checkCSRFParam();
         $project_id = $this->request->getIntegerParam('project_id');
 
         if ($project_id && $this->project->disable($project_id)) {
@@ -353,6 +343,8 @@ class Project extends Base
      */
     public function revoke()
     {
+        $this->checkCSRFParam();
+
         $values = array(
             'project_id' => $this->request->getIntegerParam('project_id'),
             'user_id' => $this->request->getIntegerParam('user_id'),
diff --git a/app/Controller/Subtask.php b/app/Controller/Subtask.php
index 5ef193c8..1c217fa2 100644
--- a/app/Controller/Subtask.php
+++ b/app/Controller/Subtask.php
@@ -170,6 +170,7 @@ class Subtask extends Base
      */
     public function remove()
     {
+        $this->checkCSRFParam();
         $task = $this->getTask();
         $subtask = $this->getSubtask();
 
diff --git a/app/Controller/Task.php b/app/Controller/Task.php
index 68e3728a..d44ba268 100644
--- a/app/Controller/Task.php
+++ b/app/Controller/Task.php
@@ -218,6 +218,7 @@ class Task extends Base
      */
     public function close()
     {
+        $this->checkCSRFParam();
         $task = $this->getTask();
 
         if ($this->task->close($task['id'])) {
@@ -252,6 +253,7 @@ class Task extends Base
      */
     public function open()
     {
+        $this->checkCSRFParam();
         $task = $this->getTask();
 
         if ($this->task->open($task['id'])) {
@@ -286,6 +288,7 @@ class Task extends Base
      */
     public function remove()
     {
+        $this->checkCSRFParam();
         $task = $this->getTask();
 
         if ($this->task->remove($task['id'])) {
diff --git a/app/Controller/User.php b/app/Controller/User.php
index e3fd8253..fca33b28 100644
--- a/app/Controller/User.php
+++ b/app/Controller/User.php
@@ -11,25 +11,13 @@ namespace Controller;
 class User extends Base
 {
     /**
-     * Display access forbidden page
-     *
-     * @access public
-     */
-    public function forbidden()
-    {
-        $this->response->html($this->template->layout('user_forbidden', array(
-            'menu' => 'users',
-            'title' => t('Access Forbidden')
-        )));
-    }
-
-    /**
      * Logout and destroy session
      *
      * @access public
      */
     public function logout()
     {
+        $this->checkCSRFParam();
         $this->rememberMe->destroy($this->acl->getUserId());
         $this->session->close();
         $this->response->redirect('?controller=user&action=login');
@@ -42,7 +30,9 @@ class User extends Base
      */
     public function login()
     {
-        if (isset($_SESSION['user'])) $this->response->redirect('?controller=app');
+        if (isset($_SESSION['user'])) {
+            $this->response->redirect('?controller=app');
+        }
 
         $this->response->html($this->template->layout('user_login', array(
             'errors' => array(),
@@ -236,6 +226,7 @@ class User extends Base
      */
     public function remove()
     {
+        $this->checkCSRFParam();
         $user_id = $this->request->getIntegerParam('user_id');
 
         if ($user_id && $this->user->remove($user_id)) {
@@ -298,6 +289,7 @@ class User extends Base
      */
     public function unlinkGoogle()
     {
+        $this->checkCSRFParam();
         if ($this->google->unlink($this->acl->getUserId())) {
             $this->session->flash(t('Your Google Account is not linked anymore to your profile.'));
         }