summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFrédéric Guillot <fred@kanboard.net>2014-09-23 15:17:04 +0200
committerFrédéric Guillot <fred@kanboard.net>2014-09-23 15:17:04 +0200
commit484c9614d1ed325448bf3a6e97e00a9f4448dc93 (patch)
tree99848dbe2e088b67152c3def675a9253c1b5ce8a
parent0bd0beba411991844d5a9b44b1b51a6eb903dff7 (diff)
Regular users can remove only their own tasks
-rw-r--r--app/Controller/Base.php5
-rw-r--r--app/Controller/Task.php4
-rw-r--r--app/Model/TaskPermission.php32
-rw-r--r--app/Templates/task_layout.php2
-rw-r--r--app/Templates/task_sidebar.php4
-rw-r--r--tests/units/TaskPermissionTest.php100
6 files changed, 145 insertions, 2 deletions
diff --git a/app/Controller/Base.php b/app/Controller/Base.php
index e9957bbd..e07aabf7 100644
--- a/app/Controller/Base.php
+++ b/app/Controller/Base.php
@@ -31,6 +31,7 @@ use Model\LastLogin;
* @property \Model\Task $task
* @property \Model\TaskHistory $taskHistory
* @property \Model\TaskExport $taskExport
+ * @property \Model\TaskPermission $taskPermission
* @property \Model\TaskValidator $taskValidator
* @property \Model\CommentHistory $commentHistory
* @property \Model\SubtaskHistory $subtaskHistory
@@ -242,6 +243,10 @@ abstract class Base
*/
protected function taskLayout($template, array $params)
{
+ if (isset($params['task']) && $this->taskPermission->canRemoveTask($params['task']) === false) {
+ $params['hide_remove_menu'] = true;
+ }
+
$content = $this->template->load($template, $params);
$params['task_content_for_layout'] = $content;
diff --git a/app/Controller/Task.php b/app/Controller/Task.php
index 7bb989c6..28db5c28 100644
--- a/app/Controller/Task.php
+++ b/app/Controller/Task.php
@@ -289,6 +289,10 @@ class Task extends Base
{
$task = $this->getTask();
+ if (! $this->taskPermission->canRemoveTask($task)) {
+ $this->forbidden();
+ }
+
if ($this->request->getStringParam('confirmation') === 'yes') {
$this->checkCSRFParam();
diff --git a/app/Model/TaskPermission.php b/app/Model/TaskPermission.php
new file mode 100644
index 00000000..2ab154f4
--- /dev/null
+++ b/app/Model/TaskPermission.php
@@ -0,0 +1,32 @@
+<?php
+
+namespace Model;
+
+/**
+ * Task permission model
+ *
+ * @package model
+ * @author Frederic Guillot
+ */
+class TaskPermission extends Base
+{
+ /**
+ * Return true if the user can remove a task
+ *
+ * Regular users can't remove tasks from other people
+ *
+ * @public
+ * @return boolean
+ */
+ public function canRemoveTask(array $task)
+ {
+ if ($this->acl->isAdminUser()) {
+ return true;
+ }
+ else if (isset($task['creator_id']) && $task['creator_id'] == $this->acl->getUserId()) {
+ return true;
+ }
+
+ return false;
+ }
+}
diff --git a/app/Templates/task_layout.php b/app/Templates/task_layout.php
index 96c45608..ca0a413f 100644
--- a/app/Templates/task_layout.php
+++ b/app/Templates/task_layout.php
@@ -7,7 +7,7 @@
</div>
<section class="task-show" id="task-section">
- <?= Helper\template('task_sidebar', array('task' => $task)) ?>
+ <?= Helper\template('task_sidebar', array('task' => $task, 'hide_remove_menu' => isset($hide_remove_menu))) ?>
<div class="task-show-main">
<?= $task_content_for_layout ?>
diff --git a/app/Templates/task_sidebar.php b/app/Templates/task_sidebar.php
index 4d363fec..4cffd5fa 100644
--- a/app/Templates/task_sidebar.php
+++ b/app/Templates/task_sidebar.php
@@ -18,7 +18,9 @@
<a href="?controller=task&amp;action=open&amp;task_id=<?= $task['id'] ?>"><?= t('Open this task') ?></a>
<?php endif ?>
</li>
- <li><a href="?controller=task&amp;action=remove&amp;task_id=<?= $task['id'] ?>"><?= t('Remove') ?></a></li>
+ <?php if (! $hide_remove_menu): ?>
+ <li><a href="?controller=task&amp;action=remove&amp;task_id=<?= $task['id'] ?>"><?= t('Remove') ?></a></li>
+ <?php endif ?>
</ul>
</div>
</div> \ No newline at end of file
diff --git a/tests/units/TaskPermissionTest.php b/tests/units/TaskPermissionTest.php
new file mode 100644
index 00000000..66036990
--- /dev/null
+++ b/tests/units/TaskPermissionTest.php
@@ -0,0 +1,100 @@
+<?php
+
+require_once __DIR__.'/Base.php';
+
+use Model\Task;
+use Model\TaskPermission;
+use Model\Project;
+use Model\Category;
+use Model\User;
+
+class TaskPermissionTest extends Base
+{
+ public function testPrepareCreation()
+ {
+ $t = new Task($this->registry);
+ $tp = new TaskPermission($this->registry);
+ $p = new Project($this->registry);
+ $u = new User($this->registry);
+
+ $this->assertTrue($u->create(array('username' => 'toto', 'password' => '123456')));
+ $this->assertTrue($u->create(array('username' => 'toto2', 'password' => '123456')));
+ $this->assertEquals(1, $p->create(array('name' => 'Project #1')));
+ $this->assertEquals(1, $t->create(array('title' => 'Task #1', 'project_id' => 1, 'creator_id' => 1)));
+ $this->assertEquals(2, $t->create(array('title' => 'Task #2', 'project_id' => 1, 'creator_id' => 2)));
+ $this->assertEquals(3, $t->create(array('title' => 'Task #3', 'project_id' => 1, 'creator_id' => 3)));
+ $this->assertEquals(4, $t->create(array('title' => 'Task #4', 'project_id' => 1)));
+
+ // User #1 can remove everything
+ $user = $u->getbyId(1);
+ $this->assertNotEmpty($user);
+ $u->updateSession($user);
+
+ $task = $t->getbyId(1);
+ $this->assertNotEmpty($task);
+ $this->assertTrue($tp->canRemoveTask($task));
+
+ // User #2 can't remove the task #1
+ $user = $u->getbyId(2);
+ $this->assertNotEmpty($user);
+ $u->updateSession($user);
+
+ $task = $t->getbyId(1);
+ $this->assertNotEmpty($task);
+ $this->assertFalse($tp->canRemoveTask($task));
+
+ // User #1 can remove everything
+ $user = $u->getbyId(1);
+ $this->assertNotEmpty($user);
+ $u->updateSession($user);
+
+ $task = $t->getbyId(2);
+ $this->assertNotEmpty($task);
+ $this->assertTrue($tp->canRemoveTask($task));
+
+ // User #2 can remove his own task
+ $user = $u->getbyId(2);
+ $this->assertNotEmpty($user);
+ $u->updateSession($user);
+
+ $task = $t->getbyId(2);
+ $this->assertNotEmpty($task);
+ $this->assertTrue($tp->canRemoveTask($task));
+
+ // User #1 can remove everything
+ $user = $u->getbyId(1);
+ $this->assertNotEmpty($user);
+ $u->updateSession($user);
+
+ $task = $t->getbyId(3);
+ $this->assertNotEmpty($task);
+ $this->assertTrue($tp->canRemoveTask($task));
+
+ // User #2 can't remove the task #3
+ $user = $u->getbyId(2);
+ $this->assertNotEmpty($user);
+ $u->updateSession($user);
+
+ $task = $t->getbyId(3);
+ $this->assertNotEmpty($task);
+ $this->assertFalse($tp->canRemoveTask($task));
+
+ // User #1 can remove everything
+ $user = $u->getbyId(1);
+ $this->assertNotEmpty($user);
+ $u->updateSession($user);
+
+ $task = $t->getbyId(4);
+ $this->assertNotEmpty($task);
+ $this->assertTrue($tp->canRemoveTask($task));
+
+ // User #2 can't remove the task #4
+ $user = $u->getbyId(2);
+ $this->assertNotEmpty($user);
+ $u->updateSession($user);
+
+ $task = $t->getbyId(4);
+ $this->assertNotEmpty($task);
+ $this->assertFalse($tp->canRemoveTask($task));
+ }
+}