Add CSRF protections

This commit is contained in:
Frédéric Guillot
2014-05-28 15:14:52 -04:00
parent 75ab09e28b
commit 445ef6d148
60 changed files with 291 additions and 132 deletions

View File

@@ -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'])) {

View File

@@ -3,6 +3,7 @@
namespace Controller;
use Core\Registry;
use Core\Security;
use Core\Translator;
use Model\LastLogin;
@@ -160,6 +161,28 @@ abstract class Base
$this->response->html($this->template->layout('app_notfound', array('title' => t('Page not found'))));
}
/**
* 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
*
@@ -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();
}
}
}

View File

@@ -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 ($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
);
}
if (isset($values['positions'])) {
$this->board->saveTasksPosition($values['positions']);
else {
$this->response->status(401);
}
$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
);
}
/**
@@ -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);
}
}
}

View File

@@ -175,6 +175,7 @@ class Category extends Base
*/
public function remove()
{
$this->checkCSRFParam();
$project = $this->getProject();
$category = $this->getCategory($project['id']);

View File

@@ -178,6 +178,7 @@ class Comment extends Base
*/
public function remove()
{
$this->checkCSRFParam();
$task = $this->getTask();
$comment = $this->getComment();

View File

@@ -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');
}

View File

@@ -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'));

View File

@@ -12,19 +12,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
*
@@ -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'),

View File

@@ -170,6 +170,7 @@ class Subtask extends Base
*/
public function remove()
{
$this->checkCSRFParam();
$task = $this->getTask();
$subtask = $this->getSubtask();

View File

@@ -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'])) {

View File

@@ -10,19 +10,6 @@ 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
*
@@ -30,6 +17,7 @@ class User extends Base
*/
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.'));
}