From 9a8c6d6493191a09720a634c58c230dba1cafeeb Mon Sep 17 00:00:00 2001
From: Frederic Guillot <fred@kanboard.net>
Date: Sat, 8 Apr 2017 10:34:34 -0400
Subject: Improve task update restriction

---
 app/Controller/TaskModificationController.php | 25 +++++++++++++------------
 app/Template/task/dropdown.php                | 16 +++++++++-------
 app/Template/task/sidebar.php                 |  2 ++
 3 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/app/Controller/TaskModificationController.php b/app/Controller/TaskModificationController.php
index a3f68a8b..a53c1a38 100644
--- a/app/Controller/TaskModificationController.php
+++ b/app/Controller/TaskModificationController.php
@@ -40,6 +40,11 @@ class TaskModificationController extends BaseController
     public function edit(array $values = array(), array $errors = array())
     {
         $task = $this->getTask();
+
+        if (! $this->helper->projectRole->canUpdateTask($task)) {
+            throw new AccessForbiddenException(t('You are not allowed to update tasks assigned to someone else.'));
+        }
+
         $project = $this->projectModel->getById($task['project_id']);
 
         if (empty($values)) {
@@ -105,7 +110,14 @@ class TaskModificationController extends BaseController
 
     protected function updateTask(array &$task, array &$values, array &$errors)
     {
-        $this->checkPermission($task, $values);
+        if (isset($values['owner_id']) && $values['owner_id'] != $task['owner_id'] && !$this->helper->projectRole->canChangeAssignee($task)) {
+            throw new AccessForbiddenException(t('You are not allowed to change the assignee.'));
+        }
+
+        if (! $this->helper->projectRole->canUpdateTask($task)) {
+            throw new AccessForbiddenException(t('You are not allowed to update tasks assigned to someone else.'));
+        }
+
         $result = $this->taskModificationModel->update($values);
 
         if ($result && ! empty($task['external_uri'])) {
@@ -122,15 +134,4 @@ class TaskModificationController extends BaseController
 
         return $result;
     }
-
-    protected function checkPermission(array &$task, array &$values)
-    {
-        if (isset($values['owner_id']) && $values['owner_id'] != $task['owner_id'] && !$this->helper->projectRole->canChangeAssignee($task)) {
-            throw new AccessForbiddenException(t('You are not allowed to change the assignee.'));
-        }
-
-        if (! $this->helper->projectRole->canUpdateTask($task)) {
-            throw new AccessForbiddenException(t('You are not allowed to update tasks assigned to someone else.'));
-        }
-    }
 }
diff --git a/app/Template/task/dropdown.php b/app/Template/task/dropdown.php
index f35abc79..5135fb77 100644
--- a/app/Template/task/dropdown.php
+++ b/app/Template/task/dropdown.php
@@ -1,14 +1,16 @@
 <div class="dropdown">
     <a href="#" class="dropdown-menu dropdown-menu-link-icon"><strong>#<?= $task['id'] ?> <i class="fa fa-caret-down"></i></strong></a>
     <ul>
-        <?php if (array_key_exists('date_started', $task) && empty($task['date_started'])): ?>
-        <li>
-            <?= $this->url->icon('play', t('Set automatically the start date'), 'TaskModificationController', 'start', array('task_id' => $task['id'], 'project_id' => $task['project_id'])) ?>
-        </li>
+        <?php if ($this->projectRole->canUpdateTask($task)): ?>
+            <?php if (array_key_exists('date_started', $task) && empty($task['date_started'])): ?>
+            <li>
+                <?= $this->url->icon('play', t('Set automatically the start date'), 'TaskModificationController', 'start', array('task_id' => $task['id'], 'project_id' => $task['project_id'])) ?>
+            </li>
+            <?php endif ?>
+            <li>
+                <?= $this->modal->large('edit', t('Edit the task'), 'TaskModificationController', 'edit', array('task_id' => $task['id'], 'project_id' => $task['project_id'])) ?>
+            </li>
         <?php endif ?>
-        <li>
-            <?= $this->modal->large('edit', t('Edit the task'), 'TaskModificationController', 'edit', array('task_id' => $task['id'], 'project_id' => $task['project_id'])) ?>
-        </li>
         <li>
             <?= $this->modal->medium('plus', t('Add a sub-task'), 'SubtaskController', 'create', array('task_id' => $task['id'], 'project_id' => $task['project_id'])) ?>
         </li>
diff --git a/app/Template/task/sidebar.php b/app/Template/task/sidebar.php
index 952c3298..265c6ef0 100644
--- a/app/Template/task/sidebar.php
+++ b/app/Template/task/sidebar.php
@@ -29,12 +29,14 @@
         <h2><?= t('Actions') ?></h2>
     </div>
     <ul>
+        <?php if ($this->projectRole->canUpdateTask($task)): ?>
         <li>
             <?= $this->modal->large('edit', t('Edit the task'), 'TaskModificationController', 'edit', array('task_id' => $task['id'], 'project_id' => $task['project_id'])) ?>
         </li>
         <li>
             <?= $this->modal->medium('refresh fa-rotate-90', t('Edit recurrence'), 'TaskRecurrenceController', 'edit', array('task_id' => $task['id'], 'project_id' => $task['project_id'])) ?>
         </li>
+        <?php endif ?>
         <li>
             <?= $this->modal->medium('plus', t('Add a sub-task'), 'SubtaskController', 'create', array('task_id' => $task['id'], 'project_id' => $task['project_id'])) ?>
         </li>
-- 
cgit v1.2.3