From 074f6c104f3e49401ef0065540338fc2d4be79f0 Mon Sep 17 00:00:00 2001 From: Frederic Guillot Date: Sat, 23 Sep 2017 18:48:45 -0700 Subject: Avoid people to alter other projects by changing form data --- app/Controller/ActionController.php | 5 +- app/Controller/ActionCreationController.php | 7 ++- app/Controller/BaseController.php | 90 +++++++++++++++++++++++++++++ app/Controller/CategoryController.php | 31 +++------- app/Controller/ColumnController.php | 16 +++-- app/Controller/CustomFilterController.php | 7 ++- app/Controller/ProjectEditController.php | 2 + app/Controller/ProjectTagController.php | 33 ++++------- app/Controller/SwimlaneController.php | 43 +++++--------- app/Controller/TaskCreationController.php | 1 + 10 files changed, 150 insertions(+), 85 deletions(-) (limited to 'app/Controller') diff --git a/app/Controller/ActionController.php b/app/Controller/ActionController.php index c935125a..43acf590 100644 --- a/app/Controller/ActionController.php +++ b/app/Controller/ActionController.php @@ -46,9 +46,10 @@ class ActionController extends BaseController public function confirm() { $project = $this->getProject(); + $action = $this->getAction($project); $this->response->html($this->helper->layout->project('action/remove', array( - 'action' => $this->actionModel->getById($this->request->getIntegerParam('action_id')), + 'action' => $action, 'available_events' => $this->eventManager->getAll(), 'available_actions' => $this->actionManager->getAvailableActions(), 'project' => $project, @@ -65,7 +66,7 @@ class ActionController extends BaseController { $this->checkCSRFParam(); $project = $this->getProject(); - $action = $this->actionModel->getById($this->request->getIntegerParam('action_id')); + $action = $this->getAction($project); if (! empty($action) && $this->actionModel->remove($action['id'])) { $this->flash->success(t('Action removed successfully.')); diff --git a/app/Controller/ActionCreationController.php b/app/Controller/ActionCreationController.php index 7fee58d1..abb214e6 100644 --- a/app/Controller/ActionCreationController.php +++ b/app/Controller/ActionCreationController.php @@ -35,8 +35,9 @@ class ActionCreationController extends BaseController { $project = $this->getProject(); $values = $this->request->getValues(); + $values['project_id'] = $project['id']; - if (empty($values['action_name']) || empty($values['project_id'])) { + if (empty($values['action_name'])) { return $this->create(); } @@ -57,8 +58,9 @@ class ActionCreationController extends BaseController { $project = $this->getProject(); $values = $this->request->getValues(); + $values['project_id'] = $project['id']; - if (empty($values['action_name']) || empty($values['project_id']) || empty($values['event_name'])) { + if (empty($values['action_name']) || empty($values['event_name'])) { $this->create(); return; } @@ -109,6 +111,7 @@ class ActionCreationController extends BaseController */ private function doCreation(array $project, array $values) { + $values['project_id'] = $project['id']; list($valid, ) = $this->actionValidator->validateCreation($values); if ($valid) { diff --git a/app/Controller/BaseController.php b/app/Controller/BaseController.php index 5233e27f..1ac7ed20 100644 --- a/app/Controller/BaseController.php +++ b/app/Controller/BaseController.php @@ -155,4 +155,94 @@ abstract class BaseController extends Base return $subtask; } + + protected function getColumn(array $project) + { + $column = $this->columnModel->getById($this->request->getIntegerParam('column_id')); + + if (empty($column)) { + throw new PageNotFoundException(); + } + + if ($column['project_id'] != $project['id']) { + throw new AccessForbiddenException(); + } + + return $column; + } + + protected function getSwimlane(array $project) + { + $swimlane = $this->swimlaneModel->getById($this->request->getIntegerParam('swimlane_id')); + + if (empty($swimlane)) { + throw new PageNotFoundException(); + } + + if ($swimlane['project_id'] != $project['id']) { + throw new AccessForbiddenException(); + } + + return $swimlane; + } + + protected function getCategory(array $project) + { + $category = $this->categoryModel->getById($this->request->getIntegerParam('category_id')); + + if (empty($category)) { + throw new PageNotFoundException(); + } + + if ($category['project_id'] != $project['id']) { + throw new AccessForbiddenException(); + } + + return $category; + } + + protected function getProjectTag(array $project) + { + $tag = $this->tagModel->getById($this->request->getIntegerParam('tag_id')); + + if (empty($tag)) { + throw new PageNotFoundException(); + } + + if ($tag['project_id'] != $project['id']) { + throw new AccessForbiddenException(); + } + + return $tag; + } + + protected function getAction(array $project) + { + $action = $this->actionModel->getById($this->request->getIntegerParam('action_id')); + + if (empty($action)) { + throw new PageNotFoundException(); + } + + if ($action['project_id'] != $project['id']) { + throw new AccessForbiddenException(); + } + + return $action; + } + + protected function getCustomFilter(array $project) + { + $filter = $this->customFilterModel->getById($this->request->getIntegerParam('filter_id')); + + if (empty($filter)) { + throw new PageNotFoundException(); + } + + if ($filter['project_id'] != $project['id']) { + throw new AccessForbiddenException(); + } + + return $filter; + } } diff --git a/app/Controller/CategoryController.php b/app/Controller/CategoryController.php index 69bbad5a..e3f2406b 100644 --- a/app/Controller/CategoryController.php +++ b/app/Controller/CategoryController.php @@ -12,24 +12,6 @@ use Kanboard\Core\Controller\PageNotFoundException; */ class CategoryController extends BaseController { - /** - * Get the category (common method between actions) - * - * @access private - * @return array - * @throws PageNotFoundException - */ - private function getCategory() - { - $category = $this->categoryModel->getById($this->request->getIntegerParam('category_id')); - - if (empty($category)) { - throw new PageNotFoundException(); - } - - return $category; - } - /** * List of categories for a given project * @@ -72,8 +54,9 @@ class CategoryController extends BaseController public function save() { $project = $this->getProject(); - $values = $this->request->getValues(); + $values['project_id'] = $project['id']; + list($valid, $errors) = $this->categoryValidator->validateCreation($values); if ($valid) { @@ -100,7 +83,7 @@ class CategoryController extends BaseController public function edit(array $values = array(), array $errors = array()) { $project = $this->getProject(); - $category = $this->getCategory(); + $category = $this->getCategory($project); $this->response->html($this->template->render('category/edit', array( 'values' => empty($values) ? $category : $values, @@ -117,8 +100,12 @@ class CategoryController extends BaseController public function update() { $project = $this->getProject(); + $category = $this->getCategory($project); $values = $this->request->getValues(); + $values['project_id'] = $project['id']; + $values['id'] = $category['id']; + list($valid, $errors) = $this->categoryValidator->validateModification($values); if ($valid) { @@ -141,7 +128,7 @@ class CategoryController extends BaseController public function confirm() { $project = $this->getProject(); - $category = $this->getCategory(); + $category = $this->getCategory($project); $this->response->html($this->helper->layout->project('category/remove', array( 'project' => $project, @@ -158,7 +145,7 @@ class CategoryController extends BaseController { $this->checkCSRFParam(); $project = $this->getProject(); - $category = $this->getCategory(); + $category = $this->getCategory($project); if ($this->categoryModel->remove($category['id'])) { $this->flash->success(t('Category removed successfully.')); diff --git a/app/Controller/ColumnController.php b/app/Controller/ColumnController.php index 7047d30e..8e4712d9 100644 --- a/app/Controller/ColumnController.php +++ b/app/Controller/ColumnController.php @@ -61,6 +61,7 @@ class ColumnController extends BaseController { $project = $this->getProject(); $values = $this->request->getValues() + array('hide_in_dashboard' => 0); + $values['project_id'] = $project['id']; list($valid, $errors) = $this->columnValidator->validateCreation($values); @@ -95,7 +96,7 @@ class ColumnController extends BaseController public function edit(array $values = array(), array $errors = array()) { $project = $this->getProject(); - $column = $this->columnModel->getById($this->request->getIntegerParam('column_id')); + $column = $this->getColumn($project); $this->response->html($this->helper->layout->project('column/edit', array( 'errors' => $errors, @@ -113,7 +114,11 @@ class ColumnController extends BaseController public function update() { $project = $this->getProject(); + $column = $this->getColumn($project); + $values = $this->request->getValues() + array('hide_in_dashboard' => 0); + $values['project_id'] = $project['id']; + $values['id'] = $column['id']; list($valid, $errors) = $this->columnValidator->validateModification($values); @@ -164,9 +169,10 @@ class ColumnController extends BaseController public function confirm() { $project = $this->getProject(); + $column = $this->getColumn($project); $this->response->html($this->helper->layout->project('column/remove', array( - 'column' => $this->columnModel->getById($this->request->getIntegerParam('column_id')), + 'column' => $column, 'project' => $project, ))); } @@ -178,11 +184,11 @@ class ColumnController extends BaseController */ public function remove() { - $project = $this->getProject(); $this->checkCSRFParam(); - $column_id = $this->request->getIntegerParam('column_id'); + $project = $this->getProject(); + $column = $this->getColumn($project); - if ($this->columnModel->remove($column_id)) { + if ($this->columnModel->remove($column['id'])) { $this->flash->success(t('Column removed successfully.')); } else { $this->flash->failure(t('Unable to remove this column.')); diff --git a/app/Controller/CustomFilterController.php b/app/Controller/CustomFilterController.php index dfe1ffc4..1bf1617e 100644 --- a/app/Controller/CustomFilterController.php +++ b/app/Controller/CustomFilterController.php @@ -59,6 +59,7 @@ class CustomFilterController extends BaseController $project = $this->getProject(); $values = $this->request->getValues(); + $values['project_id'] = $project['id']; $values['user_id'] = $this->userSession->getId(); list($valid, $errors) = $this->customFilterValidator->validateCreation($values); @@ -84,7 +85,7 @@ class CustomFilterController extends BaseController public function confirm() { $project = $this->getProject(); - $filter = $this->customFilterModel->getById($this->request->getIntegerParam('filter_id')); + $filter = $this->getCustomFilter($project); $this->response->html($this->helper->layout->project('custom_filter/remove', array( 'project' => $project, @@ -102,7 +103,7 @@ class CustomFilterController extends BaseController { $this->checkCSRFParam(); $project = $this->getProject(); - $filter = $this->customFilterModel->getById($this->request->getIntegerParam('filter_id')); + $filter = $this->getCustomFilter($project); $this->checkPermission($project, $filter); @@ -153,6 +154,8 @@ class CustomFilterController extends BaseController $this->checkPermission($project, $filter); $values = $this->request->getValues(); + $values['id'] = $filter['id']; + $values['project_id'] = $project['id']; if (! isset($values['is_shared'])) { $values += array('is_shared' => 0); diff --git a/app/Controller/ProjectEditController.php b/app/Controller/ProjectEditController.php index ae39fdf3..dd534508 100644 --- a/app/Controller/ProjectEditController.php +++ b/app/Controller/ProjectEditController.php @@ -65,6 +65,8 @@ class ProjectEditController extends BaseController */ private function prepareValues(array $project, array $values) { + $values['id'] = $project['id']; + if (isset($values['is_private'])) { if (! $this->helper->user->hasProjectAccess('ProjectCreationController', 'create', $project['id'])) { unset($values['is_private']); diff --git a/app/Controller/ProjectTagController.php b/app/Controller/ProjectTagController.php index d225f0ca..c45e71e1 100644 --- a/app/Controller/ProjectTagController.php +++ b/app/Controller/ProjectTagController.php @@ -2,8 +2,6 @@ namespace Kanboard\Controller; -use Kanboard\Core\Controller\AccessForbiddenException; - /** * Class ProjectTagController * @@ -27,10 +25,6 @@ class ProjectTagController extends BaseController { $project = $this->getProject(); - if (empty($values)) { - $values['project_id'] = $project['id']; - } - $this->response->html($this->template->render('project_tag/create', array( 'project' => $project, 'values' => $values, @@ -42,6 +36,8 @@ class ProjectTagController extends BaseController { $project = $this->getProject(); $values = $this->request->getValues(); + $values['project_id'] = $project['id']; + list($valid, $errors) = $this->tagValidator->validateCreation($values); if ($valid) { @@ -60,8 +56,7 @@ class ProjectTagController extends BaseController public function edit(array $values = array(), array $errors = array()) { $project = $this->getProject(); - $tag_id = $this->request->getIntegerParam('tag_id'); - $tag = $this->tagModel->getById($tag_id); + $tag = $this->getProjectTag($project); if (empty($values)) { $values = $tag; @@ -78,14 +73,12 @@ class ProjectTagController extends BaseController public function update() { $project = $this->getProject(); - $tag_id = $this->request->getIntegerParam('tag_id'); - $tag = $this->tagModel->getById($tag_id); + $tag = $this->getProjectTag($project); $values = $this->request->getValues(); - list($valid, $errors) = $this->tagValidator->validateModification($values); + $values['project_id'] = $project['id']; + $values['id'] = $tag['id']; - if ($tag['project_id'] != $project['id']) { - throw new AccessForbiddenException(); - } + list($valid, $errors) = $this->tagValidator->validateModification($values); if ($valid) { if ($this->tagModel->update($values['id'], $values['name'])) { @@ -103,8 +96,7 @@ class ProjectTagController extends BaseController public function confirm() { $project = $this->getProject(); - $tag_id = $this->request->getIntegerParam('tag_id'); - $tag = $this->tagModel->getById($tag_id); + $tag = $this->getProjectTag($project); $this->response->html($this->template->render('project_tag/remove', array( 'tag' => $tag, @@ -116,14 +108,9 @@ class ProjectTagController extends BaseController { $this->checkCSRFParam(); $project = $this->getProject(); - $tag_id = $this->request->getIntegerParam('tag_id'); - $tag = $this->tagModel->getById($tag_id); - - if ($tag['project_id'] != $project['id']) { - throw new AccessForbiddenException(); - } + $tag = $this->getProjectTag($project); - if ($this->tagModel->remove($tag_id)) { + if ($this->tagModel->remove($tag['id'])) { $this->flash->success(t('Tag removed successfully.')); } else { $this->flash->failure(t('Unable to remove this tag.')); diff --git a/app/Controller/SwimlaneController.php b/app/Controller/SwimlaneController.php index 0d81d83c..e6368b24 100644 --- a/app/Controller/SwimlaneController.php +++ b/app/Controller/SwimlaneController.php @@ -3,8 +3,6 @@ namespace Kanboard\Controller; use Kanboard\Core\Controller\AccessForbiddenException; -use Kanboard\Core\Controller\PageNotFoundException; -use Kanboard\Model\SwimlaneModel; /** * Swimlanes Controller @@ -14,24 +12,6 @@ use Kanboard\Model\SwimlaneModel; */ class SwimlaneController extends BaseController { - /** - * Get the swimlane (common method between actions) - * - * @access private - * @return array - * @throws PageNotFoundException - */ - private function getSwimlane() - { - $swimlane = $this->swimlaneModel->getById($this->request->getIntegerParam('swimlane_id')); - - if (empty($swimlane)) { - throw new PageNotFoundException(); - } - - return $swimlane; - } - /** * List of swimlanes for a given project * @@ -78,6 +58,8 @@ class SwimlaneController extends BaseController { $project = $this->getProject(); $values = $this->request->getValues(); + $values['project_id'] = $project['id']; + list($valid, $errors) = $this->swimlaneValidator->validateCreation($values); if ($valid) { @@ -104,7 +86,7 @@ class SwimlaneController extends BaseController public function edit(array $values = array(), array $errors = array()) { $project = $this->getProject(); - $swimlane = $this->getSwimlane(); + $swimlane = $this->getSwimlane($project); $this->response->html($this->helper->layout->project('swimlane/edit', array( 'values' => empty($values) ? $swimlane : $values, @@ -121,8 +103,11 @@ class SwimlaneController extends BaseController public function update() { $project = $this->getProject(); - + $swimlane = $this->getSwimlane($project); $values = $this->request->getValues(); + $values['project_id'] = $project['id']; + $values['id'] = $swimlane['id']; + list($valid, $errors) = $this->swimlaneValidator->validateModification($values); if ($valid) { @@ -145,7 +130,7 @@ class SwimlaneController extends BaseController public function confirm() { $project = $this->getProject(); - $swimlane = $this->getSwimlane(); + $swimlane = $this->getSwimlane($project); $this->response->html($this->helper->layout->project('swimlane/remove', array( 'project' => $project, @@ -162,9 +147,9 @@ class SwimlaneController extends BaseController { $this->checkCSRFParam(); $project = $this->getProject(); - $swimlane_id = $this->request->getIntegerParam('swimlane_id'); + $swimlane = $this->getSwimlane($project); - if ($this->swimlaneModel->remove($project['id'], $swimlane_id)) { + if ($this->swimlaneModel->remove($project['id'], $swimlane['id'])) { $this->flash->success(t('Swimlane removed successfully.')); } else { $this->flash->failure(t('Unable to remove this swimlane.')); @@ -182,9 +167,9 @@ class SwimlaneController extends BaseController { $this->checkCSRFParam(); $project = $this->getProject(); - $swimlane_id = $this->request->getIntegerParam('swimlane_id'); + $swimlane = $this->getSwimlane($project); - if ($this->swimlaneModel->disable($project['id'], $swimlane_id)) { + if ($this->swimlaneModel->disable($project['id'], $swimlane['id'])) { $this->flash->success(t('Swimlane updated successfully.')); } else { $this->flash->failure(t('Unable to update this swimlane.')); @@ -202,9 +187,9 @@ class SwimlaneController extends BaseController { $this->checkCSRFParam(); $project = $this->getProject(); - $swimlane_id = $this->request->getIntegerParam('swimlane_id'); + $swimlane = $this->getSwimlane($project); - if ($this->swimlaneModel->enable($project['id'], $swimlane_id)) { + if ($this->swimlaneModel->enable($project['id'], $swimlane['id'])) { $this->flash->success(t('Swimlane updated successfully.')); } else { $this->flash->failure(t('Unable to update this swimlane.')); diff --git a/app/Controller/TaskCreationController.php b/app/Controller/TaskCreationController.php index 0d808c54..d050a60d 100644 --- a/app/Controller/TaskCreationController.php +++ b/app/Controller/TaskCreationController.php @@ -49,6 +49,7 @@ class TaskCreationController extends BaseController { $project = $this->getProject(); $values = $this->request->getValues(); + $values['project_id'] = $project['id']; list($valid, $errors) = $this->taskValidator->validateCreation($values); -- cgit v1.2.3