Do not expose IDs in forms

kanboard · Sep 24, 2017 · 3e0f14a · 3e0f14a
1 parent 074f6c1
commit 3e0f14a
Show file tree

Hide file tree

Showing 20 changed files with 112 additions and 120 deletions.
diff --git a/app/Controller/BaseController.php b/app/Controller/BaseController.php
@@ -138,24 +138,70 @@ protected function getUser()
         return $user;
     }
 
-    /**
-     * Get the current subtask
-     *
-     * @access protected
-     * @return array
-     * @throws PageNotFoundException
-     */
-    protected function getSubtask()
+    protected function getSubtask(array $task)
     {
         $subtask = $this->subtaskModel->getById($this->request->getIntegerParam('subtask_id'));
 
         if (empty($subtask)) {
             throw new PageNotFoundException();
         }
 
+        if ($subtask['task_id'] != $task['id']) {
+            throw new AccessForbiddenException();
+        }
+
         return $subtask;
     }
 
+    protected function getComment(array $task)
+    {
+        $comment = $this->commentModel->getById($this->request->getIntegerParam('comment_id'));
+
+        if (empty($comment)) {
+            throw new PageNotFoundException();
+        }
+
+        if (! $this->userSession->isAdmin() && $comment['user_id'] != $this->userSession->getId()) {
+            throw new AccessForbiddenException();
+        }
+
+        if ($comment['task_id'] != $task['id']) {
+            throw new AccessForbiddenException();
+        }
+
+        return $comment;
+    }
+
+    protected function getExternalTaskLink(array $task)
+    {
+        $link = $this->taskExternalLinkModel->getById($this->request->getIntegerParam('link_id'));
+
+        if (empty($link)) {
+            throw new PageNotFoundException();
+        }
+
+        if ($link['task_id'] != $task['id']) {
+            throw new AccessForbiddenException();
+        }
+
+        return $link;
+    }
+
+    protected function getInternalTaskLink(array $task)
+    {
+        $link = $this->taskLinkModel->getById($this->request->getIntegerParam('link_id'));
+
+        if (empty($link)) {
+            throw new PageNotFoundException();
+        }
+
+        if ($link['task_id'] != $task['id']) {
+            throw new AccessForbiddenException();
+        }
+
+        return $link;
+    }
+
     protected function getColumn(array $project)
     {
         $column = $this->columnModel->getById($this->request->getIntegerParam('column_id'));

diff --git a/app/Controller/CommentController.php b/app/Controller/CommentController.php
@@ -13,29 +13,6 @@
  */
 class CommentController extends BaseController
 {
-    /**
-     * Get the current comment
-     *
-     * @access protected
-     * @return array
-     * @throws PageNotFoundException
-     * @throws AccessForbiddenException
-     */
-    protected function getComment()
-    {
-        $comment = $this->commentModel->getById($this->request->getIntegerParam('comment_id'));
-
-        if (empty($comment)) {
-            throw new PageNotFoundException();
-        }
-
-        if (! $this->userSession->isAdmin() && $comment['user_id'] != $this->userSession->getId()) {
-            throw new AccessForbiddenException();
-        }
-
-        return $comment;
-    }
-
     /**
      * Add comment form
      *
@@ -49,14 +26,6 @@ public function create(array $values = array(), array $errors = array())
     {
         $project = $this->getProject();
         $task = $this->getTask();
-
-        if (empty($values)) {
-            $values = array(
-                'user_id' => $this->userSession->getId(),
-                'task_id' => $task['id'],
-            );
-        }
-
         $values['project_id'] = $task['project_id'];
 
         $this->response->html($this->helper->layout->task('comment/create', array(
@@ -106,7 +75,7 @@ public function save()
     public function edit(array $values = array(), array $errors = array())
     {
         $task = $this->getTask();
-        $comment = $this->getComment();
+        $comment = $this->getComment($task);
 
         if (empty($values)) {
             $values = $comment;
@@ -130,9 +99,13 @@ public function edit(array $values = array(), array $errors = array())
     public function update()
     {
         $task = $this->getTask();
-        $this->getComment();
+        $comment = $this->getComment($task);
 
         $values = $this->request->getValues();
+        $values['id'] = $comment['id'];
+        $values['task_id'] = $task['id'];
+        $values['user_id'] = $comment['user_id'];
+
         list($valid, $errors) = $this->commentValidator->validateModification($values);
 
         if ($valid) {
@@ -157,7 +130,7 @@ public function update()
     public function confirm()
     {
         $task = $this->getTask();
-        $comment = $this->getComment();
+        $comment = $this->getComment($task);
 
         $this->response->html($this->template->render('comment/remove', array(
             'comment' => $comment,
@@ -175,7 +148,7 @@ public function remove()
     {
         $this->checkCSRFParam();
         $task = $this->getTask();
-        $comment = $this->getComment();
+        $comment = $this->getComment($task);
 
         if ($this->commentModel->remove($comment['id'])) {
             $this->flash->success(t('Comment removed successfully.'));

diff --git a/app/Controller/SubtaskController.php b/app/Controller/SubtaskController.php
@@ -66,6 +66,7 @@ public function save()
     {
         $task = $this->getTask();
         $values = $this->request->getValues();
+        $values['task_id'] = $task['id'];
 
         list($valid, $errors) = $this->subtaskValidator->validateCreation($values);
 
@@ -103,7 +104,7 @@ public function save()
     public function edit(array $values = array(), array $errors = array())
     {
         $task = $this->getTask();
-        $subtask = $this->getSubtask();
+        $subtask = $this->getSubtask($task);
 
         $this->response->html($this->template->render('subtask/edit', array(
             'values' => empty($values) ? $subtask : $values,
@@ -123,9 +124,12 @@ public function edit(array $values = array(), array $errors = array())
     public function update()
     {
         $task = $this->getTask();
-        $this->getSubtask();
+        $subtask = $this->getSubtask($task);
 
         $values = $this->request->getValues();
+        $values['id'] = $subtask['id'];
+        $values['task_id'] = $task['id'];
+
         list($valid, $errors) = $this->subtaskValidator->validateModification($values);
 
         if ($valid) {
@@ -149,7 +153,7 @@ public function update()
     public function confirm()
     {
         $task = $this->getTask();
-        $subtask = $this->getSubtask();
+        $subtask = $this->getSubtask($task);
 
         $this->response->html($this->template->render('subtask/remove', array(
             'subtask' => $subtask,
@@ -166,7 +170,7 @@ public function remove()
     {
         $this->checkCSRFParam();
         $task = $this->getTask();
-        $subtask = $this->getSubtask();
+        $subtask = $this->getSubtask($task);
 
         if ($this->subtaskModel->remove($subtask['id'])) {
             $this->flash->success(t('Sub-task removed successfully.'));

diff --git a/app/Controller/SubtaskConverterController.php b/app/Controller/SubtaskConverterController.php
@@ -13,7 +13,7 @@ class SubtaskConverterController extends BaseController
     public function show()
     {
         $task = $this->getTask();
-        $subtask = $this->getSubtask();
+        $subtask = $this->getSubtask($task);
 
         $this->response->html($this->template->render('subtask_converter/show', array(
             'subtask' => $subtask,
@@ -24,7 +24,8 @@ public function show()
     public function save()
     {
         $project = $this->getProject();
-        $subtask = $this->getSubtask();
+        $task = $this->getTask();
+        $subtask = $this->getSubtask($task);
 
         $task_id = $this->subtaskTaskConversionModel->convertToTask($project['id'], $subtask['id']);
 

diff --git a/app/Controller/SubtaskRestrictionController.php b/app/Controller/SubtaskRestrictionController.php
@@ -20,7 +20,7 @@ class SubtaskRestrictionController extends BaseController
     public function show()
     {
         $task = $this->getTask();
-        $subtask = $this->getSubtask();
+        $subtask = $this->getSubtask($task);
 
         $this->response->html($this->template->render('subtask_restriction/show', array(
             'status_list' => array(
@@ -41,7 +41,7 @@ public function show()
     public function save()
     {
         $task = $this->getTask();
-        $subtask = $this->getSubtask();
+        $subtask = $this->getSubtask($task);
         $values = $this->request->getValues();
 
         // Change status of the previous "in progress" subtask

diff --git a/app/Controller/SubtaskStatusController.php b/app/Controller/SubtaskStatusController.php
@@ -18,7 +18,7 @@ class SubtaskStatusController extends BaseController
     public function change()
     {
         $task = $this->getTask();
-        $subtask = $this->getSubtask();
+        $subtask = $this->getSubtask($task);
         $fragment = $this->request->getStringParam('fragment');
 
         $status = $this->subtaskStatusModel->toggleStatus($subtask['id']);
@@ -43,19 +43,19 @@ public function change()
     public function timer()
     {
         $task = $this->getTask();
-        $subtaskId = $this->request->getIntegerParam('subtask_id');
+        $subtask = $this->getSubtask($task);
         $timer = $this->request->getStringParam('timer');
 
         if ($timer === 'start') {
-            $this->subtaskTimeTrackingModel->logStartTime($subtaskId, $this->userSession->getId());
+            $this->subtaskTimeTrackingModel->logStartTime($subtask['id'], $this->userSession->getId());
         } elseif ($timer === 'stop') {
-            $this->subtaskTimeTrackingModel->logEndTime($subtaskId, $this->userSession->getId());
+            $this->subtaskTimeTrackingModel->logEndTime($subtask['id'], $this->userSession->getId());
             $this->subtaskTimeTrackingModel->updateTaskTimeTracking($task['id']);
         }
 
         $this->response->html($this->template->render('subtask/timer', array(
             'task'    => $task,
-            'subtask' => $this->subtaskModel->getByIdWithDetails($subtaskId),
+            'subtask' => $this->subtaskModel->getByIdWithDetails($subtask['id']),
         )));
     }
 

diff --git a/app/Controller/TaskExternalLinkController.php b/app/Controller/TaskExternalLinkController.php
@@ -74,6 +74,8 @@ public function save()
     {
         $task = $this->getTask();
         $values = $this->request->getValues();
+        $values['task_id'] = $task['id'];
+
         list($valid, $errors) = $this->externalLinkValidator->validateCreation($values);
 
         if ($valid) {
@@ -108,22 +110,14 @@ public function save()
     public function edit(array $values = array(), array $errors = array())
     {
         $task = $this->getTask();
-        $link_id = $this->request->getIntegerParam('link_id');
-
-        if ($link_id > 0) {
-            $values = $this->taskExternalLinkModel->getById($link_id);
-        }
-
-        if (empty($values)) {
-            throw new PageNotFoundException();
-        }
-
-        $provider = $this->externalLinkManager->getProvider($values['link_type']);
+        $link = $this->getExternalTaskLink($task);
+        $provider = $this->externalLinkManager->getProvider($link['link_type']);
 
         $this->response->html($this->template->render('task_external_link/edit', array(
-            'values' => $values,
-            'errors' => $errors,
-            'task' => $task,
+            'values'       => empty($values) ? $link : $values,
+            'errors'       => $errors,
+            'task'         => $task,
+            'link'         => $link,
             'dependencies' => $provider->getDependencies(),
         )));
     }
@@ -136,7 +130,12 @@ public function edit(array $values = array(), array $errors = array())
     public function update()
     {
         $task = $this->getTask();
+        $link = $this->getExternalTaskLink($task);
+
         $values = $this->request->getValues();
+        $values['id'] = $link['id'];
+        $values['task_id'] = $link['task_id'];
+
         list($valid, $errors) = $this->externalLinkValidator->validateModification($values);
 
         if ($valid && $this->taskExternalLinkModel->update($values)) {
@@ -155,12 +154,7 @@ public function update()
     public function confirm()
     {
         $task = $this->getTask();
-        $link_id = $this->request->getIntegerParam('link_id');
-        $link = $this->taskExternalLinkModel->getById($link_id);
-
-        if (empty($link)) {
-            throw new PageNotFoundException();
-        }
+        $link = $this->getExternalTaskLink($task);
 
         $this->response->html($this->template->render('task_external_link/remove', array(
             'link' => $link,
@@ -177,8 +171,9 @@ public function remove()
     {
         $this->checkCSRFParam();
         $task = $this->getTask();
+        $link = $this->getExternalTaskLink($task);
 
-        if ($this->taskExternalLinkModel->remove($this->request->getIntegerParam('link_id'))) {
+        if ($this->taskExternalLinkModel->remove($link['id'])) {
             $this->flash->success(t('Link removed successfully.'));
         } else {
             $this->flash->failure(t('Unable to remove this link.'));