From 28bc4246bff405367c9e5640bca356b307962026 Mon Sep 17 00:00:00 2001 From: Frédéric Guillot Date: Sat, 1 Mar 2014 19:51:09 -0500 Subject: Add acl and access list for projects --- assets/css/app.css | 7 ++ controllers/base.php | 51 +++++++------- controllers/board.php | 55 ++++++++++----- controllers/config.php | 4 -- controllers/project.php | 95 ++++++++++++++++++++------ controllers/task.php | 27 ++++++-- controllers/user.php | 12 +--- index.php | 3 + locales/fr_FR/translations.php | 11 +++ locales/pl_PL/translations.php | 13 ++++ models/acl.php | 64 ++++++++++++++++++ models/base.php | 5 +- models/config.php | 4 +- models/project.php | 144 ++++++++++++++++++++++++++++++++++++---- models/schema.php | 14 ++++ models/user.php | 4 +- scripts/make-archive.sh | 2 +- templates/project_forbidden.php | 9 +++ templates/project_index.php | 3 + templates/project_users.php | 44 ++++++++++++ tests/AclTest.php | 118 ++++++++++++++++++++++++++++++++ tests/ProjectTest.php | 63 ++++++++++++++++++ 22 files changed, 647 insertions(+), 105 deletions(-) create mode 100644 models/acl.php create mode 100644 templates/project_forbidden.php create mode 100644 templates/project_users.php create mode 100644 tests/AclTest.php create mode 100644 tests/ProjectTest.php diff --git a/assets/css/app.css b/assets/css/app.css index 14e77508..93a29e47 100644 --- a/assets/css/app.css +++ b/assets/css/app.css @@ -588,6 +588,7 @@ tr td.task-orange, } /* config page */ +.listing, .settings { border-radius: 4px; padding: 8px 35px 8px 14px; @@ -597,12 +598,18 @@ tr td.task-orange, background-color: #f0f0f0; } +.listing li, .settings li { list-style-type: square; margin-left: 20px; margin-bottom: 3px; } +.listing ul { + margin-top: 15px; + margin-bottom: 15px; +} + /* confirmation box */ .confirm { max-width: 700px; diff --git a/controllers/base.php b/controllers/base.php index da4ee8ae..cf423402 100644 --- a/controllers/base.php +++ b/controllers/base.php @@ -9,6 +9,7 @@ require __DIR__.'/../lib/template.php'; require __DIR__.'/../lib/helper.php'; require __DIR__.'/../lib/translator.php'; require __DIR__.'/../models/base.php'; +require __DIR__.'/../models/acl.php'; require __DIR__.'/../models/config.php'; require __DIR__.'/../models/user.php'; require __DIR__.'/../models/project.php'; @@ -26,6 +27,7 @@ abstract class Base protected $task; protected $board; protected $config; + protected $acl; public function __construct() { @@ -38,30 +40,20 @@ abstract class Base $this->project = new \Model\Project; $this->task = new \Model\Task; $this->board = new \Model\Board; - } - - private function noAuthAllowed($controller, $action) - { - $public = array( - 'user' => array('login', 'check'), - 'task' => array('add'), - 'board' => array('readonly'), - ); - - if (isset($public[$controller])) { - return in_array($action, $public[$controller]); - } - - return false; + $this->acl = new \Model\Acl; } public function beforeAction($controller, $action) { + // Start the session $this->session->open(dirname($_SERVER['PHP_SELF']), SESSION_SAVE_PATH); - if (! isset($_SESSION['user']) && ! $this->noAuthAllowed($controller, $action)) { - $this->response->redirect('?controller=user&action=login'); - } + // HTTP secure headers + $this->response->csp(); + $this->response->nosniff(); + $this->response->xss(); + $this->response->hsts(); + $this->response->xframe(); // Load translations $language = $this->config->get('language', 'en_US'); @@ -70,17 +62,24 @@ abstract class Base // Set timezone date_default_timezone_set($this->config->get('timezone', 'UTC')); - $this->response->csp(); - $this->response->nosniff(); - $this->response->xss(); - $this->response->hsts(); - $this->response->xframe(); + // If the user is not authenticated redirect to the login form, if the action is public continue + if (! isset($_SESSION['user']) && ! $this->acl->isPublicAction($controller, $action)) { + $this->response->redirect('?controller=user&action=login'); + } + + // Check if the user is allowed to see this page + if (! $this->acl->isPageAccessAllowed($controller, $action)) { + $this->response->redirect('?controller=user&action=forbidden'); + } } - public function checkPermissions() + public function checkProjectPermissions($project_id) { - if ($_SESSION['user']['is_admin'] == 0) { - $this->response->redirect('?controller=user&action=forbidden'); + if ($this->acl->isRegularUser()) { + + if ($project_id > 0 && ! $this->project->isUserAllowed($project_id, $this->acl->getUserId())) { + $this->response->redirect('?controller=project&action=forbidden'); + } } } diff --git a/controllers/board.php b/controllers/board.php index a0f00367..13714b3c 100644 --- a/controllers/board.php +++ b/controllers/board.php @@ -8,15 +8,20 @@ class Board extends Base public function assign() { $task = $this->task->getById($this->request->getIntegerParam('task_id')); - $project = $this->project->get($task['project_id']); + $project = $this->project->getById($task['project_id']); $projects = $this->project->getListByStatus(\Model\Project::ACTIVE); + if ($this->acl->isRegularUser()) { + $projects = $this->project->filterListByAccess($projects, $this->acl->getUserId()); + } + if (! $project) $this->notfound(); + $this->checkProjectPermissions($project['id']); $this->response->html($this->template->layout('board_assign', array( 'errors' => array(), 'values' => $task, - 'users_list' => $this->user->getList(), + 'users_list' => $this->project->getUsersList($project['id']), 'projects' => $projects, 'current_project_id' => $project['id'], 'current_project_name' => $project['name'], @@ -29,6 +34,8 @@ class Board extends Base public function assignTask() { $values = $this->request->getValues(); + $this->checkProjectPermissions($values['project_id']); + list($valid,) = $this->task->validateAssigneeModification($values); if ($valid && $this->task->update($values)) { @@ -68,8 +75,18 @@ class Board extends Base { $projects = $this->project->getListByStatus(\Model\Project::ACTIVE); - if (! count($projects)) { - $this->redirectNoProject(); + if ($this->acl->isRegularUser()) { + $projects = $this->project->filterListByAccess($projects, $this->acl->getUserId()); + } + + if (empty($projects)) { + + if ($this->acl->isAdminUser()) { + $this->redirectNoProject(); + } + else { + $this->response->redirect('?controller=project&action=forbidden'); + } } else if (! empty($_SESSION['user']['default_project_id']) && isset($projects[$_SESSION['user']['default_project_id']])) { $project_id = $_SESSION['user']['default_project_id']; @@ -79,6 +96,8 @@ class Board extends Base list($project_id, $project_name) = each($projects); } + $this->checkProjectPermissions($project_id); + $this->response->html($this->template->layout('board_index', array( 'projects' => $projects, 'current_project_id' => $project_id, @@ -93,8 +112,14 @@ class Board extends Base public function show() { $projects = $this->project->getListByStatus(\Model\Project::ACTIVE); + + if ($this->acl->isRegularUser()) { + $projects = $this->project->filterListByAccess($projects, $this->acl->getUserId()); + } + $project_id = $this->request->getIntegerParam('project_id'); + $this->checkProjectPermissions($project_id); if (! isset($projects[$project_id])) $this->notfound(); $project_name = $projects[$project_id]; @@ -112,10 +137,8 @@ class Board extends Base // Display a form to edit a board public function edit() { - $this->checkPermissions(); - $project_id = $this->request->getIntegerParam('project_id'); - $project = $this->project->get($project_id); + $project = $this->project->getById($project_id); if (! $project) $this->notfound(); @@ -140,10 +163,8 @@ class Board extends Base // Validate and update a board public function update() { - $this->checkPermissions(); - $project_id = $this->request->getIntegerParam('project_id'); - $project = $this->project->get($project_id); + $project = $this->project->getById($project_id); if (! $project) $this->notfound(); @@ -183,10 +204,8 @@ class Board extends Base // Validate and add a new column public function add() { - $this->checkPermissions(); - $project_id = $this->request->getIntegerParam('project_id'); - $project = $this->project->get($project_id); + $project = $this->project->getById($project_id); if (! $project) $this->notfound(); @@ -224,8 +243,6 @@ class Board extends Base // Confirmation dialog before removing a column public function confirm() { - $this->checkPermissions(); - $this->response->html($this->template->layout('board_remove', array( 'column' => $this->board->getColumn($this->request->getIntegerParam('column_id')), 'menu' => 'projects', @@ -236,8 +253,6 @@ class Board extends Base // Remove a column public function remove() { - $this->checkPermissions(); - $column = $this->board->getColumn($this->request->getIntegerParam('column_id')); if ($column && $this->board->removeColumn($column['id'])) { @@ -252,6 +267,12 @@ class Board extends Base // Save the board (Ajax request made by the drag and drop) public function save() { + $project_id = $this->request->getIntegerParam('project_id'); + + if ($project_id > 0 && ! $this->project->isUserAllowed($project_id, $this->acl->getUserId())) { + $this->response->json(array('result' => false), 401); + } + $this->response->json(array( 'result' => $this->board->saveTasksPosition($this->request->getValues()) )); diff --git a/controllers/config.php b/controllers/config.php index 5dfa828b..064fa06d 100644 --- a/controllers/config.php +++ b/controllers/config.php @@ -23,8 +23,6 @@ class Config extends Base // Validate and save settings public function save() { - $this->checkPermissions(); - $values = $this->request->getValues(); list($valid, $errors) = $this->config->validateModification($values); @@ -56,7 +54,6 @@ class Config extends Base // Download the database public function downloadDb() { - $this->checkPermissions(); $this->response->forceDownload('db.sqlite.gz'); $this->response->binary($this->config->downloadDatabase()); } @@ -64,7 +61,6 @@ class Config extends Base // Optimize the database public function optimizeDb() { - $this->checkPermissions(); $this->config->optimizeDatabase(); $this->session->flash(t('Database optimization done.')); $this->response->redirect('?controller=config'); diff --git a/controllers/project.php b/controllers/project.php index 1ad2e829..8d8584bc 100644 --- a/controllers/project.php +++ b/controllers/project.php @@ -4,17 +4,28 @@ namespace Controller; class Project extends Base { + // Display access forbidden page + public function forbidden() + { + $this->response->html($this->template->layout('project_forbidden', array( + 'menu' => 'projects', + 'title' => t('Access Forbidden') + ))); + } + // List of completed tasks for a given project public function tasks() { $project_id = $this->request->getIntegerParam('project_id'); - $project = $this->project->get($project_id); + $project = $this->project->getById($project_id); if (! $project) { $this->session->flashError(t('Project not found.')); $this->response->redirect('?controller=project'); } + $this->checkProjectPermissions($project['id']); + $tasks = $this->task->getAllByProjectId($project_id, array(0)); $nb_tasks = count($tasks); @@ -30,7 +41,7 @@ class Project extends Base // List of projects public function index() { - $projects = $this->project->getAll(true); + $projects = $this->project->getAll(true, $this->acl->isRegularUser()); $nb_projects = count($projects); $this->response->html($this->template->layout('project_index', array( @@ -44,8 +55,6 @@ class Project extends Base // Display a form to create a new project public function create() { - $this->checkPermissions(); - $this->response->html($this->template->layout('project_new', array( 'errors' => array(), 'values' => array(), @@ -57,8 +66,6 @@ class Project extends Base // Validate and save a new project public function save() { - $this->checkPermissions(); - $values = $this->request->getValues(); list($valid, $errors) = $this->project->validateCreation($values); @@ -84,9 +91,7 @@ class Project extends Base // Display a form to edit a project public function edit() { - $this->checkPermissions(); - - $project = $this->project->get($this->request->getIntegerParam('project_id')); + $project = $this->project->getById($this->request->getIntegerParam('project_id')); if (! $project) { $this->session->flashError(t('Project not found.')); @@ -104,8 +109,6 @@ class Project extends Base // Validate and update a project public function update() { - $this->checkPermissions(); - $values = $this->request->getValues() + array('is_active' => 0); list($valid, $errors) = $this->project->validateModification($values); @@ -131,9 +134,7 @@ class Project extends Base // Confirmation dialog before to remove a project public function confirm() { - $this->checkPermissions(); - - $project = $this->project->get($this->request->getIntegerParam('project_id')); + $project = $this->project->getById($this->request->getIntegerParam('project_id')); if (! $project) { $this->session->flashError(t('Project not found.')); @@ -150,8 +151,6 @@ class Project extends Base // Remove a project public function remove() { - $this->checkPermissions(); - $project_id = $this->request->getIntegerParam('project_id'); if ($project_id && $this->project->remove($project_id)) { @@ -166,8 +165,6 @@ class Project extends Base // Enable a project public function enable() { - $this->checkPermissions(); - $project_id = $this->request->getIntegerParam('project_id'); if ($project_id && $this->project->enable($project_id)) { @@ -182,8 +179,6 @@ class Project extends Base // Disable a project public function disable() { - $this->checkPermissions(); - $project_id = $this->request->getIntegerParam('project_id'); if ($project_id && $this->project->disable($project_id)) { @@ -194,4 +189,64 @@ class Project extends Base $this->response->redirect('?controller=project'); } + + // Users list for the selected project + public function users() + { + $project = $this->project->getById($this->request->getIntegerParam('project_id')); + + if (! $project) { + $this->session->flashError(t('Project not found.')); + $this->response->redirect('?controller=project'); + } + + $this->response->html($this->template->layout('project_users', array( + 'project' => $project, + 'users' => $this->project->getAllUsers($project['id']), + 'menu' => 'projects', + 'title' => t('Edit project access list') + ))); + } + + // Allow a specific user for the selected project + public function allow() + { + $values = $this->request->getValues(); + list($valid,) = $this->project->validateUserAccess($values); + + if ($valid) { + + if ($this->project->allowUser($values['project_id'], $values['user_id'])) { + $this->session->flash(t('Project updated successfully.')); + } + else { + $this->session->flashError(t('Unable to update this project.')); + } + } + + $this->response->redirect('?controller=project&action=users&project_id='.$values['project_id']); + } + + // Revoke user access + public function revoke() + { + $values = array( + 'project_id' => $this->request->getIntegerParam('project_id'), + 'user_id' => $this->request->getIntegerParam('user_id'), + ); + + list($valid,) = $this->project->validateUserAccess($values); + + if ($valid) { + + if ($this->project->revokeUser($values['project_id'], $values['user_id'])) { + $this->session->flash(t('Project updated successfully.')); + } + else { + $this->session->flashError(t('Unable to update this project.')); + } + } + + $this->response->redirect('?controller=project&action=users&project_id='.$values['project_id']); + } } diff --git a/controllers/task.php b/controllers/task.php index 3aa486d5..0057a531 100644 --- a/controllers/task.php +++ b/controllers/task.php @@ -45,6 +45,7 @@ class Task extends Base $task = $this->task->getById($this->request->getIntegerParam('task_id'), true); if (! $task) $this->notfound(); + $this->checkProjectPermissions($task['project_id']); $this->response->html($this->template->layout('task_show', array( 'task' => $task, @@ -59,6 +60,7 @@ class Task extends Base public function create() { $project_id = $this->request->getIntegerParam('project_id'); + $this->checkProjectPermissions($project_id); $this->response->html($this->template->layout('task_new', array( 'errors' => array(), @@ -71,7 +73,7 @@ class Task extends Base ), 'projects_list' => $this->project->getListByStatus(\Model\Project::ACTIVE), 'columns_list' => $this->board->getColumnsList($project_id), - 'users_list' => $this->user->getList(), + 'users_list' => $this->project->getUsersList($project_id), 'colors_list' => $this->task->getColors(), 'menu' => 'tasks', 'title' => t('New task') @@ -82,6 +84,8 @@ class Task extends Base public function save() { $values = $this->request->getValues(); + $this->checkProjectPermissions($values['project_id']); + list($valid, $errors) = $this->task->validateCreation($values); if ($valid) { @@ -108,7 +112,7 @@ class Task extends Base 'values' => $values, 'projects_list' => $this->project->getListByStatus(\Model\Project::ACTIVE), 'columns_list' => $this->board->getColumnsList($values['project_id']), - 'users_list' => $this->user->getList(), + 'users_list' => $this->project->getUsersList($values['project_id']), 'colors_list' => $this->task->getColors(), 'menu' => 'tasks', 'title' => t('New task') @@ -121,12 +125,13 @@ class Task extends Base $task = $this->task->getById($this->request->getIntegerParam('task_id')); if (! $task) $this->notfound(); + $this->checkProjectPermissions($task['project_id']); $this->response->html($this->template->layout('task_edit', array( 'errors' => array(), 'values' => $task, 'columns_list' => $this->board->getColumnsList($task['project_id']), - 'users_list' => $this->user->getList(), + 'users_list' => $this->project->getUsersList($task['project_id']), 'colors_list' => $this->task->getColors(), 'menu' => 'tasks', 'title' => t('Edit a task') @@ -137,6 +142,8 @@ class Task extends Base public function update() { $values = $this->request->getValues(); + $this->checkProjectPermissions($values['project_id']); + list($valid, $errors) = $this->task->validateModification($values); if ($valid) { @@ -154,7 +161,7 @@ class Task extends Base 'errors' => $errors, 'values' => $values, 'columns_list' => $this->board->getColumnsList($values['project_id']), - 'users_list' => $this->user->getList(), + 'users_list' => $this->project->getUsersList($values['project_id']), 'colors_list' => $this->task->getColors(), 'menu' => 'tasks', 'title' => t('Edit a task') @@ -166,7 +173,10 @@ class Task extends Base { $task = $this->task->getById($this->request->getIntegerParam('task_id')); - if ($task && $this->task->close($task['id'])) { + if (! $task) $this->notfound(); + $this->checkProjectPermissions($task['project_id']); + + if ($this->task->close($task['id'])) { $this->session->flash(t('Task closed successfully.')); } else { $this->session->flashError(t('Unable to close this task.')); @@ -181,6 +191,7 @@ class Task extends Base $task = $this->task->getById($this->request->getIntegerParam('task_id')); if (! $task) $this->notfound(); + $this->checkProjectPermissions($task['project_id']); $this->response->html($this->template->layout('task_close', array( 'task' => $task, @@ -194,7 +205,10 @@ class Task extends Base { $task = $this->task->getById($this->request->getIntegerParam('task_id')); - if ($task && $this->task->open($task['id'])) { + if (! $task) $this->notfound(); + $this->checkProjectPermissions($task['project_id']); + + if ($this->task->open($task['id'])) { $this->session->flash(t('Task opened successfully.')); } else { $this->session->flashError(t('Unable to open this task.')); @@ -209,6 +223,7 @@ class Task extends Base $task = $this->task->getById($this->request->getIntegerParam('task_id')); if (! $task) $this->notfound(); + $this->checkProjectPermissions($task['project_id']); $this->response->html($this->template->layout('task_open', array( 'task' => $task, diff --git a/controllers/user.php b/controllers/user.php index 9f9781ef..10d3ad21 100644 --- a/controllers/user.php +++ b/controllers/user.php @@ -68,8 +68,6 @@ class User extends Base // Display a form to create a new user public function create() { - $this->checkPermissions(); - $this->response->html($this->template->layout('user_new', array( 'projects' => $this->project->getList(), 'errors' => array(), @@ -82,8 +80,6 @@ class User extends Base // Validate and save a new user public function save() { - $this->checkPermissions(); - $values = $this->request->getValues(); list($valid, $errors) = $this->user->validateCreation($values); @@ -121,7 +117,7 @@ class User extends Base unset($user['password']); $this->response->html($this->template->layout('user_edit', array( - 'projects' => $this->project->getList(), + 'projects' => $this->project->filterListByAccess($this->project->getList(), $user['id']), 'errors' => array(), 'values' => $user, 'menu' => 'users', @@ -162,7 +158,7 @@ class User extends Base } $this->response->html($this->template->layout('user_edit', array( - 'projects' => $this->project->getList(), + 'projects' => $this->project->filterListByAccess($this->project->getList(), $values['id']), 'errors' => $errors, 'values' => $values, 'menu' => 'users', @@ -173,8 +169,6 @@ class User extends Base // Confirmation dialog before to remove a user public function confirm() { - $this->checkPermissions(); - $user = $this->user->getById($this->request->getIntegerParam('user_id')); if (! $user) $this->notfound(); @@ -189,8 +183,6 @@ class User extends Base // Remove a user public function remove() { - $this->checkPermissions(); - $user_id = $this->request->getIntegerParam('user_id'); if ($user_id && $this->user->remove($user_id)) { diff --git a/index.php b/index.php index 61a04537..8ab3dcba 100644 --- a/index.php +++ b/index.php @@ -12,5 +12,8 @@ defined('AUTO_REFRESH_DURATION') or define('AUTO_REFRESH_DURATION', 60); // Custom session save path defined('SESSION_SAVE_PATH') or define('SESSION_SAVE_PATH', ''); +// Database filename +defined('DB_FILENAME') or define('DB_FILENAME', 'data/db.sqlite'); + $router = new Router; $router->execute(); diff --git a/locales/fr_FR/translations.php b/locales/fr_FR/translations.php index b3cbafab..3a64f0c7 100644 --- a/locales/fr_FR/translations.php +++ b/locales/fr_FR/translations.php @@ -190,4 +190,15 @@ return array( 'limit' => 'limite', 'Task limit' => 'Nombre maximum de tâches', 'This value must be greater than %d' => 'Cette valeur doit être plus grande que %d', + 'Edit project access list' => 'Modifier l\'accès au projet', + 'Edit users access' => 'Modifier les utilisateurs autorisés', + 'Allow this user' => 'Autoriser cet utilisateur', + 'Project access list for "%s"' => 'Liste des accès au projet « %s »', + 'Only those users have access to this project:' => 'Seulement ces utilisateurs ont accès à ce projet :', + 'Don\'t forget that administrators have access to everything.' => 'N\'oubliez pas que les administrateurs ont accès à tout.', + 'revoke' => 'révoquer', + 'List of authorized users' => 'Liste des utilisateurs autorisés', + 'User' => 'Utilisateur', + 'Everybody have access to this project.' => 'Tout le monde a accès au projet.', + 'You are not allowed to access to this project.' => 'Vous n\'êtes pas autorisé à accéder à ce projet.', ); diff --git a/locales/pl_PL/translations.php b/locales/pl_PL/translations.php index 58d2323c..2c50e3ed 100644 --- a/locales/pl_PL/translations.php +++ b/locales/pl_PL/translations.php @@ -184,10 +184,23 @@ return array( 'Change assignee' => 'Zmień odpowiedzialną osobę', 'Change assignee for the task "%s"' => 'Zmień odpowiedzialną osobę dla zadania "%s"', 'Timezone' => 'Strefa czasowa', + // Missing translations: + //'Sorry, I didn\'t found this information in my database!' => '', //'Page not found' => '', //'Story Points' => '', //'limit' => '', //'Task limit' => '', //'This value must be greater than %d' => '', + // 'Edit project access list' => '', + // 'Edit users access' => '', + // 'Allow this user' => '', + // 'Project access list for "%s"' => '', + // 'Only those users have access to this project:' => '', + // 'Don\'t forget that administrators have access to everything.' => '', + // 'revoke' => '', + // 'List of authorized users' => '', + // 'User' => '', + // 'Everybody have access to this project.' => '', + // 'You are not allowed to access to this project.' => '', ); diff --git a/models/acl.php b/models/acl.php new file mode 100644 index 00000000..7c363272 --- /dev/null +++ b/models/acl.php @@ -0,0 +1,64 @@ + array('login', 'check'), + 'task' => array('add'), + 'board' => array('readonly'), + ); + + // Controllers and actions allowed for regular users + private $user_actions = array( + 'app' => array('index'), + 'board' => array('index', 'show', 'assign', 'assigntask', 'save'), + 'project' => array('tasks', 'index', 'forbidden'), + 'task' => array('show', 'create', 'save', 'edit', 'update', 'close', 'confirmclose', 'open', 'confirmopen'), + 'user' => array('index', 'edit', 'update', 'forbidden', 'logout', 'index'), + 'config' => array('index'), + ); + + public function isAllowedAction(array $acl, $controller, $action) + { + if (isset($acl[$controller])) { + return in_array($action, $acl[$controller]); + } + + return false; + } + + public function isPublicAction($controller, $action) + { + return $this->isAllowedAction($this->public_actions, $controller, $action); + } + + public function isUserAction($controller, $action) + { + return $this->isAllowedAction($this->user_actions, $controller, $action); + } + + public function isAdminUser() + { + return isset($_SESSION['user']['is_admin']) && $_SESSION['user']['is_admin'] === '1'; + } + + public function isRegularUser() + { + return isset($_SESSION['user']['is_admin']) && $_SESSION['user']['is_admin'] === '0'; + } + + public function getUserId() + { + return isset($_SESSION['user']['id']) ? (int) $_SESSION['user']['id'] : 0; + } + + public function isPageAccessAllowed($controller, $action) + { + return $this->isPublicAction($controller, $action) || + $this->isAdminUser() || + ($this->isRegularUser() && $this->isUserAction($controller, $action)); + } +} diff --git a/models/base.php b/models/base.php index c8e4cf19..44a8b6b2 100644 --- a/models/base.php +++ b/models/base.php @@ -18,8 +18,7 @@ require __DIR__.'/schema.php'; abstract class Base { const APP_VERSION = 'master'; - const DB_VERSION = 6; - const DB_FILENAME = 'data/db.sqlite'; + const DB_VERSION = 7; private static $dbInstance = null; protected $db; @@ -37,7 +36,7 @@ abstract class Base { $db = new \PicoDb\Database(array( 'driver' => 'sqlite', - 'filename' => self::DB_FILENAME + 'filename' => DB_FILENAME )); if ($db->schema()->check(self::DB_VERSION)) { diff --git a/models/config.php b/models/config.php index a00d0b7e..8f818a3b 100644 --- a/models/config.php +++ b/models/config.php @@ -79,11 +79,11 @@ class Config extends Base public function downloadDatabase() { - return gzencode(file_get_contents(self::DB_FILENAME)); + return gzencode(file_get_contents(DB_FILENAME)); } public function getDatabaseSize() { - return filesize(self::DB_FILENAME); + return filesize(DB_FILENAME); } } diff --git a/models/project.php b/models/project.php index cb96dccd..a2f66478 100644 --- a/models/project.php +++ b/models/project.php @@ -8,10 +8,89 @@ use \SimpleValidator\Validators; class Project extends Base { const TABLE = 'projects'; + const TABLE_USERS = 'project_has_users'; const ACTIVE = 1; const INACTIVE = 0; - public function get($project_id) + public function getUsersList($project_id) + { + $allowed_users = $this->getAllowedUsers($project_id); + + if (empty($allowed_users)) { + $userModel = new User; + $allowed_users = $userModel->getList(); + } + + return array(t('Unassigned')) + $allowed_users; + } + + public function getAllowedUsers($project_id) + { + return $this->db + ->table(self::TABLE_USERS) + ->join(\Model\User::TABLE, 'id', 'user_id') + ->eq('project_id', $project_id) + ->asc('username') + ->listing('user_id', 'username'); + } + + public function getAllUsers($project_id) + { + $users = array( + 'allowed' => array(), + 'not_allowed' => array(), + ); + + $userModel = new User; + $all_users = $userModel->getList(); + + $users['allowed'] = $this->getAllowedUsers($project_id); + + foreach ($all_users as $user_id => $username) { + + if (! isset($users['allowed'][$user_id])) { + $users['not_allowed'][$user_id] = $username; + } + } + + return $users; + } + + public function allowUser($project_id, $user_id) + { + return $this->db + ->table(self::TABLE_USERS) + ->save(array('project_id' => $project_id, 'user_id' => $user_id)); + } + + public function revokeUser($project_id, $user_id) + { + return $this->db + ->table(self::TABLE_USERS) + ->eq('project_id', $project_id) + ->eq('user_id', $user_id) + ->remove(); + } + + public function isUserAllowed($project_id, $user_id) + { + // If there is nobody specified, everybody have access to the project + $nb_users = $this->db + ->table(self::TABLE_USERS) + ->eq('project_id', $project_id) + ->count(); + + if ($nb_users < 1) return true; + + // Otherwise, allow only specific users + return (bool) $this->db + ->table(self::TABLE_USERS) + ->eq('project_id', $project_id) + ->eq('user_id', $user_id) + ->count(); + } + + public function getById($project_id) { return $this->db->table(self::TABLE)->eq('id', $project_id)->findOne(); } @@ -26,7 +105,7 @@ class Project extends Base return $this->db->table(self::TABLE)->findOne(); } - public function getAll($fetch_stats = false) + public function getAll($fetch_stats = false, $check_permissions = false) { if (! $fetch_stats) { return $this->db->table(self::TABLE)->asc('name')->findAll(); @@ -41,20 +120,27 @@ class Project extends Base $taskModel = new \Model\Task; $boardModel = new \Model\Board; + $aclModel = new \Model\Acl; - foreach ($projects as &$project) { + foreach ($projects as $pkey => &$project) { - $columns = $boardModel->getcolumns($project['id']); - $project['nb_active_tasks'] = 0; - - foreach ($columns as &$column) { - $column['nb_active_tasks'] = $taskModel->countByColumnId($project['id'], $column['id']); - $project['nb_active_tasks'] += $column['nb_active_tasks']; + if ($check_permissions && ! $this->isUserAllowed($project['id'], $aclModel->getUserId())) { + unset($projects[$pkey]); } + else { + + $columns = $boardModel->getcolumns($project['id']); + $project['nb_active_tasks'] = 0; - $project['columns'] = $columns; - $project['nb_tasks'] = $taskModel->countByProjectId($project['id']); - $project['nb_inactive_tasks'] = $project['nb_tasks'] - $project['nb_active_tasks']; + foreach ($columns as &$column) { + $column['nb_active_tasks'] = $taskModel->countByColumnId($project['id'], $column['id']); + $project['nb_active_tasks'] += $column['nb_active_tasks']; + } + + $project['columns'] = $columns; + $project['nb_tasks'] = $taskModel->countByProjectId($project['id']); + $project['nb_inactive_tasks'] = $project['nb_tasks'] - $project['nb_active_tasks']; + } } $this->db->closeTransaction(); @@ -93,12 +179,27 @@ class Project extends Base ->count(); } + public function filterListByAccess(array $projects, $user_id) + { + foreach ($projects as $project_id => $project_name) { + if (! $this->isUserAllowed($project_id, $user_id)) { + unset($projects[$project_id]); + } + } + + return $projects; + } + public function create(array $values) { $this->db->startTransaction(); $values['token'] = self::generateToken(); - $this->db->table(self::TABLE)->save($values); + + if (! $this->db->table(self::TABLE)->save($values)) { + $this->db->cancelTransaction(); + return false; + } $project_id = $this->db->getConnection()->getLastId(); @@ -112,7 +213,7 @@ class Project extends Base $this->db->closeTransaction(); - return $project_id; + return (int) $project_id; } public function update(array $values) @@ -170,4 +271,19 @@ class Project extends Base $v->getErrors() ); } + + public function validateUserAccess(array $values) + { + $v = new Validator($values, array( + new Validators\Required('project_id', t('The project id is required')), + new Validators\Integer('project_id', t('This value must be an integer')), + new Validators\Required('user_id', t('The user id is required')), + new Validators\Integer('user_id', t('This value must be an integer')), + )); + + return array( + $v->execute(), + $v->getErrors() + ); + } } diff --git a/models/schema.php b/models/schema.php index 4aa2a265..f98f0e69 100644 --- a/models/schema.php +++ b/models/schema.php @@ -2,6 +2,20 @@ namespace Schema; +function version_7($pdo) +{ + $pdo->exec(" + CREATE TABLE project_has_users ( + id INTEGER PRIMARY KEY, + project_id INTEGER, + user_id INTEGER, + FOREIGN KEY(project_id) REFERENCES projects(id) ON DELETE CASCADE, + FOREIGN KEY(user_id) REFERENCES users(id) ON DELETE CASCADE, + UNIQUE(project_id, user_id) + ) + "); +} + function version_6($pdo) { $pdo->exec("ALTER TABLE columns ADD COLUMN task_limit INTEGER DEFAULT '0'"); diff --git a/models/user.php b/models/user.php index f80d5edf..2832a3fb 100644 --- a/models/user.php +++ b/models/user.php @@ -30,12 +30,12 @@ class User extends Base public function getList() { - return array(t('Unassigned')) + $this->db->table(self::TABLE)->asc('username')->listing('id', 'username'); + return $this->db->table(self::TABLE)->asc('username')->listing('id', 'username'); } public function create(array $values) { - unset($values['confirmation']); + if (isset($values['confirmation'])) unset($values['confirmation']); $values['password'] = \password_hash($values['password'], PASSWORD_BCRYPT); return $this->db->table(self::TABLE)->save($values); diff --git a/scripts/make-archive.sh b/scripts/make-archive.sh index 3de519fd..586dbbdf 100755 --- a/scripts/make-archive.sh +++ b/scripts/make-archive.sh @@ -6,7 +6,7 @@ APP="kanboard" cd /tmp rm -rf /tmp/$APP /tmp/$APP-*.zip 2>/dev/null git clone git@github.com:fguillot/$APP.git -rm -rf $APP/data/*.sqlite $APP/.git $APP/.gitignore $APP/scripts $APP/examples +rm -rf $APP/data/*.sqlite $APP/.git $APP/.gitignore $APP/scripts $APP/tests $APP/Vagrantfile sed -i.bak s/master/$VERSION/g $APP/models/base.php && rm -f $APP/models/*.bak zip -r $APP-$VERSION.zip $APP mv $APP-*.zip ~/Devel/websites/$APP diff --git a/templates/project_forbidden.php b/templates/project_forbidden.php new file mode 100644 index 00000000..1cba7b58 --- /dev/null +++ b/templates/project_forbidden.php @@ -0,0 +1,9 @@ +
+ + +

+ +

+
\ No newline at end of file diff --git a/templates/project_index.php b/templates/project_index.php index 4048879b..7d3e1844 100644 --- a/templates/project_index.php +++ b/templates/project_index.php @@ -62,6 +62,9 @@
  • +
  • + +
  • diff --git a/templates/project_users.php b/templates/project_users.php new file mode 100644 index 00000000..0448004f --- /dev/null +++ b/templates/project_users.php @@ -0,0 +1,44 @@ +
    + +
    + + +
    + + $project['id'])) ?> + + +
    + +
    + + +
    +
    + + +

    + +
    + +
    +

    +
      + $username): ?> +
    • + + () +
    • + +
    +

    +
    + + +
    +
    \ No newline at end of file diff --git a/tests/AclTest.php b/tests/AclTest.php new file mode 100644 index 00000000..0996a51f --- /dev/null +++ b/tests/AclTest.php @@ -0,0 +1,118 @@ + array('action1', 'action3'), + ); + + $acl = new Acl; + $this->assertTrue($acl->isAllowedAction($acl_rules, 'controller1', 'action1')); + $this->assertTrue($acl->isAllowedAction($acl_rules, 'controller1', 'action3')); + $this->assertFalse($acl->isAllowedAction($acl_rules, 'controller1', 'action2')); + $this->assertFalse($acl->isAllowedAction($acl_rules, 'controller2', 'action2')); + $this->assertFalse($acl->isAllowedAction($acl_rules, 'controller2', 'action3')); + } + + public function testIsAdmin() + { + $acl = new Acl; + + $_SESSION = array(); + $this->assertFalse($acl->isAdminUser()); + + $_SESSION = array('user' => array()); + $this->assertFalse($acl->isAdminUser()); + + $_SESSION = array('user' => array('is_admin' => true)); + $this->assertFalse($acl->isAdminUser()); + + $_SESSION = array('user' => array('is_admin' => '0')); + $this->assertFalse($acl->isAdminUser()); + + $_SESSION = array('user' => array('is_admin' => '2')); + $this->assertFalse($acl->isAdminUser()); + + $_SESSION = array('user' => array('is_admin' => '1')); + $this->assertTrue($acl->isAdminUser()); + } + + public function testIsUser() + { + $acl = new Acl; + + $_SESSION = array(); + $this->assertFalse($acl->isRegularUser()); + + $_SESSION = array('user' => array()); + $this->assertFalse($acl->isRegularUser()); + + $_SESSION = array('user' => array('is_admin' => true)); + $this->assertFalse($acl->isRegularUser()); + + $_SESSION = array('user' => array('is_admin' => '1')); + $this->assertFalse($acl->isRegularUser()); + + $_SESSION = array('user' => array('is_admin' => '2')); + $this->assertFalse($acl->isRegularUser()); + + $_SESSION = array('user' => array('is_admin' => '0')); + $this->assertTrue($acl->isRegularUser()); + } + + public function testIsPageAllowed() + { + $acl = new Acl; + + // Public access + $_SESSION = array(); + $this->assertFalse($acl->isPageAccessAllowed('user', 'create')); + $this->assertFalse($acl->isPageAccessAllowed('user', 'save')); + $this->assertFalse($acl->isPageAccessAllowed('user', 'remove')); + $this->assertFalse($acl->isPageAccessAllowed('user', 'confirm')); + $this->assertFalse($acl->isPageAccessAllowed('app', 'index')); + $this->assertFalse($acl->isPageAccessAllowed('user', 'index')); + $this->assertTrue($acl->isPageAccessAllowed('user', 'login')); + $this->assertTrue($acl->isPageAccessAllowed('user', 'check')); + $this->assertTrue($acl->isPageAccessAllowed('task', 'add')); + $this->assertTrue($acl->isPageAccessAllowed('board', 'readonly')); + + // Regular user + $_SESSION = array('user' => array('is_admin' => '0')); + $this->assertFalse($acl->isPageAccessAllowed('user', 'create')); + $this->assertFalse($acl->isPageAccessAllowed('user', 'save')); + $this->assertFalse($acl->isPageAccessAllowed('user', 'remove')); + $this->assertFalse($acl->isPageAccessAllowed('user', 'confirm')); + $this->assertTrue($acl->isPageAccessAllowed('app', 'index')); + $this->assertTrue($acl->isPageAccessAllowed('user', 'index')); + $this->assertTrue($acl->isPageAccessAllowed('user', 'login')); + $this->assertTrue($acl->isPageAccessAllowed('user', 'check')); + $this->assertTrue($acl->isPageAccessAllowed('task', 'add')); + $this->assertTrue($acl->isPageAccessAllowed('board', 'readonly')); + + // Admin user + $_SESSION = array('user' => array('is_admin' => '1')); + $this->assertTrue($acl->isPageAccessAllowed('user', 'create')); + $this->assertTrue($acl->isPageAccessAllowed('user', 'save')); + $this->assertTrue($acl->isPageAccessAllowed('user', 'remove')); + $this->assertTrue($acl->isPageAccessAllowed('user', 'confirm')); + $this->assertTrue($acl->isPageAccessAllowed('app', 'index')); + $this->assertTrue($acl->isPageAccessAllowed('user', 'index')); + $this->assertTrue($acl->isPageAccessAllowed('user', 'login')); + $this->assertTrue($acl->isPageAccessAllowed('user', 'check')); + $this->assertTrue($acl->isPageAccessAllowed('task', 'add')); + $this->assertTrue($acl->isPageAccessAllowed('board', 'readonly')); + } +} diff --git a/tests/ProjectTest.php b/tests/ProjectTest.php new file mode 100644 index 00000000..6eb39f52 --- /dev/null +++ b/tests/ProjectTest.php @@ -0,0 +1,63 @@ +assertEquals(1, $p->create(array('name' => 'UnitTest'))); + $this->assertNotEmpty($p->getById(1)); + } + + public function testAllowUsers() + { + $p = new Project; + + // Everybody is allowed + $this->assertEmpty($p->getAllowedUsers(1)); + $this->assertTrue($p->isUserAllowed(1, 1)); + + // Allow one user + $this->assertTrue($p->allowUser(1, 1)); + $this->assertFalse($p->allowUser(50, 1)); + $this->assertFalse($p->allowUser(1, 50)); + $this->assertEquals(array('1' => 'admin'), $p->getAllowedUsers(1)); + $this->assertTrue($p->isUserAllowed(1, 1)); + + // Disallow one user + $this->assertTrue($p->revokeUser(1, 1)); + $this->assertEmpty($p->getAllowedUsers(1)); + $this->assertTrue($p->isUserAllowed(1, 1)); + + // Allow/disallow many users + $user = new User; + $user->create(array('username' => 'unittest', 'password' => 'unittest')); + + $this->assertTrue($p->allowUser(1, 1)); + $this->assertTrue($p->allowUser(1, 2)); + + $this->assertEquals(array('1' => 'admin', '2' => 'unittest'), $p->getAllowedUsers(1)); + $this->assertTrue($p->isUserAllowed(1, 1)); + $this->assertTrue($p->isUserAllowed(1, 2)); + + $this->assertTrue($p->revokeUser(1, 1)); + + $this->assertEquals(array('2' => 'unittest'), $p->getAllowedUsers(1)); + $this->assertFalse($p->isUserAllowed(1, 1)); + $this->assertTrue($p->isUserAllowed(1, 2)); + } +} -- cgit v1.2.3