diff --git a/src/api/controllers/Users.php b/src/api/controllers/Users.php index b1310d4..0ed721c 100644 --- a/src/api/controllers/Users.php +++ b/src/api/controllers/Users.php @@ -58,12 +58,7 @@ class Users extends BaseController { $user = R::dispense('user'); if (isset($data->username)) { - $existing = R::findOne('user', 'username = ?', [ $data->username ]); - - if ($existing) { - $this->apiJson->addAlert('error', 'Username already exists. ' . - 'Change the username and try again.'); - + if ($this->checkUsernameExists($data)) { return $this->jsonResponse($response); } } @@ -134,22 +129,15 @@ class Users extends BaseController { $update = R::load('user', $data->id); $actor = R::load('user', Auth::GetUserId($request)); - if ((int)$actor->id !== (int)$user->id) { - if ((int)$actor->security_level > SecurityLevel::BOARD_ADMIN) { - $this->apiJson->addAlert('error', 'Access restricted.'); - - return $this->jsonResponse($response, 403); - } + if (!$this->checkUserAccess($actor, $user)) { + return $this->jsonResponse($response, 403); } $data->password_hash = $user->password_hash; if (isset($data->new_password) && isset($data->old_password)) { - if (!password_verify($data->old_password, $user->password_hash)) { + if (!$this->verifyPassword($data, $user)) { $this->logger->addError('Update User: ', [$user, $update]); - $this->apiJson->addAlert('error', 'Error updating user. ' . - 'Incorrect current password.'); - return $this->jsonResponse($response); } @@ -178,33 +166,12 @@ class Users extends BaseController { } if ($user->username !== $update->username) { - $existing = R::findOne('user', 'username = ?', [ $update->username ]); - - if ($existing !== null) { - $this->logger->addError('Update User: ', [$user, $update]); - $this->apiJson->addAlert('error', 'Error updating username. ' . - 'That username is already taken.'); - + if ($this->checkUsernameExists($update)){ return $this->jsonResponse($response); } } - if ($user->default_board_id !== $update->default_board_id && - (int)$update->default_board_id !== 0) { - if (isset($data->boardAccess) && - !in_array($data->default_board_id, $data->boardAccess)) { - $data->boardAccess[] = $data->default_board_id; - } - } - - if ((int)$user->default_board_id !== 0 && - (int)$update->default_board_id === 0) { - $key = array_search($user->default_board_id, $data->boardAccess); - - if ($key) { - unset($data->boardAccess[$key]); - } - } + $this->updateDefaultBoardId($data, $user, $update); $this->updateBoardAccess($data, $request); R::store($update); @@ -420,5 +387,59 @@ class Users extends BaseController { } } } + + private function checkUsernameExists($data) { + $existing = R::findOne('user', 'username = ?', [ $data->username ]); + + if ($existing) { + $this->apiJson->addAlert('error', 'Username already exists. ' . + 'Change the username and try again.'); + return true; + } + + return false; + } + + private function checkUserAccess($actor, $user) { + if ((int)$actor->id !== (int)$user->id) { + if ((int)$actor->security_level === SecurityLevel::ADMIN) { + return true; + } + + $this->apiJson->addAlert('error', 'Access restricted.'); + return false; + } + + return true; + } + + private function verifyPassword($data, $user) { + if (!password_verify($data->old_password, $user->password_hash)) { + $this->apiJson->addAlert('error', 'Error updating user. ' . + 'Incorrect current password.'); + return false; + } + + return true; + } + + private function updateDefaultBoardId(&$data, $user, $update) { + if ($user->default_board_id !== $update->default_board_id && + (int)$update->default_board_id !== 0) { + if (isset($data->boardAccess) && + !in_array($data->default_board_id, $data->boardAccess)) { + $data->boardAccess[] = $data->default_board_id; + } + } + + if ((int)$user->default_board_id !== 0 && + (int)$update->default_board_id === 0) { + $key = array_search($user->default_board_id, $data->boardAccess); + + if ($key) { + unset($data->boardAccess[$key]); + } + } + } } diff --git a/test/api/controllers/BoardsTest.php b/test/api/controllers/BoardsTest.php index 567f683..ffe8f1b 100644 --- a/test/api/controllers/BoardsTest.php +++ b/test/api/controllers/BoardsTest.php @@ -321,6 +321,7 @@ class BoardsTest extends PHPUnit_Framework_TestCase { $column->name = 'col2'; $column->position = 1; + $column->id = 1; $column->board_id = 1; $category->name = 'cat 1'; diff --git a/test/api/controllers/UsersTest.php b/test/api/controllers/UsersTest.php index 1ac3bfb..49d066c 100644 --- a/test/api/controllers/UsersTest.php +++ b/test/api/controllers/UsersTest.php @@ -160,6 +160,9 @@ class UsersTest extends PHPUnit_Framework_TestCase { $this->assertEquals('error', $response->alerts[0]['type']); } + /** + * @group single + */ public function testUpdateUser() { $this->createUser();