From 00801f3b616b3b46fd21f6e9cc0c3fc81cba2983 Mon Sep 17 00:00:00 2001 From: ivan Date: Sun, 25 Jun 2017 19:03:56 -0300 Subject: [PATCH] Ivan - Fix issue #27, fix frontend<->backend data flow with staff editor, add 403 response --- .../src/app/admin/panel/staff/staff-editor.js | 2 +- client/src/reducers/config-reducer.js | 2 +- client/src/reducers/session-reducer.js | 4 ++-- server/controllers/staff/edit.php | 8 ++++++-- server/controllers/staff/get.php | 3 ++- server/controllers/system/download.php | 20 +++++++++---------- server/models/Response.php | 6 ++++++ tests/staff/get.rb | 2 ++ tests/system/file-upload-download.rb | 20 +------------------ 9 files changed, 31 insertions(+), 36 deletions(-) diff --git a/client/src/app/admin/panel/staff/staff-editor.js b/client/src/app/admin/panel/staff/staff-editor.js index 9b55cff6..e4a269ee 100644 --- a/client/src/app/admin/panel/staff/staff-editor.js +++ b/client/src/app/admin/panel/staff/staff-editor.js @@ -290,7 +290,7 @@ class StaffEditor extends React.Component { path: '/staff/edit', data: { staffId: this.props.staffId, - sendEmailOnNewTicket: form.sendEmailOnNewTicket, + sendEmailOnNewTicket: form.sendEmailOnNewTicket * 1, email: form.email, password: form.password, level: (form.level !== undefined) ? form.level + 1 : null, diff --git a/client/src/reducers/config-reducer.js b/client/src/reducers/config-reducer.js index d4aa5919..32e63508 100644 --- a/client/src/reducers/config-reducer.js +++ b/client/src/reducers/config-reducer.js @@ -44,7 +44,7 @@ class ConfigReducer extends Reducer { })); return _.extend({}, state, payload.data, { - language: currentLanguage || payload.data.language, + language: currentLanguage || payload.data.language || 'en', registration: !!(payload.data.registration * 1), 'user-system-enabled': !!(payload.data['user-system-enabled']* 1), 'allow-attachments': !!(payload.data['allow-attachments']* 1), diff --git a/client/src/reducers/session-reducer.js b/client/src/reducers/session-reducer.js index 436516dc..adc64843 100644 --- a/client/src/reducers/session-reducer.js +++ b/client/src/reducers/session-reducer.js @@ -114,7 +114,7 @@ class SessionReducer extends Reducer { userLevel: userData.level, userDepartments: userData.departments, userTickets: userData.tickets, - userSendEmailOnNewTicket: userData.sendEmailOnNewTicket + userSendEmailOnNewTicket: userData.sendEmailOnNewTicket * 1 }); } @@ -133,7 +133,7 @@ class SessionReducer extends Reducer { userDepartments: userData.departments, userTickets: userData.tickets, userId: userId, - userSendEmailOnNewTicket: userData.sendEmailOnNewTicket + userSendEmailOnNewTicket: userData.sendEmailOnNewTicket * 1 }); } diff --git a/server/controllers/staff/edit.php b/server/controllers/staff/edit.php index 6b7e0262..8b949959 100755 --- a/server/controllers/staff/edit.php +++ b/server/controllers/staff/edit.php @@ -75,7 +75,7 @@ class EditStaffController extends Controller { $this->staffInstance->password = Hashing::hashPassword(Controller::request('password')); } - if(Controller::request('level') && Controller::isStaffLogged(3) && Controller::request('staffId') !== Controller::getLoggedUser()->id) { + if(Controller::request('level') && Controller::isStaffLogged(3) && !$this->isModifyingCurrentStaff()) { $this->staffInstance->level = Controller::request('level'); } @@ -87,7 +87,7 @@ class EditStaffController extends Controller { $this->staffInstance->profilePic = ($fileUploader instanceof FileUploader) ? $fileUploader->getFileName() : null; } - if(Controller::request('sendEmailOnNewTicket') !== null && !Controller::request('staffId') ) { + if(Controller::request('sendEmailOnNewTicket') !== null && $this->isModifyingCurrentStaff()) { $this->staffInstance->sendEmailOnNewTicket = Controller::request('sendEmailOnNewTicket'); } @@ -141,4 +141,8 @@ class EditStaffController extends Controller { } } } + + private function isModifyingCurrentStaff() { + return Controller::request('staffId') === Controller::getLoggedUser()->id; + } } \ No newline at end of file diff --git a/server/controllers/staff/get.php b/server/controllers/staff/get.php index 90de8990..ee2363ae 100755 --- a/server/controllers/staff/get.php +++ b/server/controllers/staff/get.php @@ -67,7 +67,8 @@ class GetStaffController extends Controller { 'level' => $user->level, 'staff' => true, 'departments' => $parsedDepartmentList, - 'tickets' => $user->sharedTicketList->toArray() + 'tickets' => $user->sharedTicketList->toArray(), + 'sendEmailOnNewTicket' => $user->sendEmailOnNewTicket ]); } } \ No newline at end of file diff --git a/server/controllers/system/download.php b/server/controllers/system/download.php index 17707ace..8bcbf4ab 100755 --- a/server/controllers/system/download.php +++ b/server/controllers/system/download.php @@ -39,31 +39,31 @@ class DownloadController extends Controller { public function handler() { $fileName = Controller::request('file'); - $staffUser = Staff::getDataStore($fileName, 'profilePic'); + $isStaffProfilePic = !Staff::getDataStore($fileName, 'profilePic')->isNull(); - if($staffUser->isNull()) { + if(!$isStaffProfilePic) { $session = Session::getInstance(); $loggedUser = Controller::getLoggedUser(); if(!$session->sessionExists()) { - print ''; + Response::respond403(); return; } $ticket = Ticket::getTicket($fileName, 'file'); - if($ticket->isNull() || ($this->isNotAuthor($ticket, $loggedUser) && $this->isNotOwner($ticket, $loggedUser))) { + if($ticket->isNull() || ($this->isNotAuthor($ticket, $loggedUser) && $this->isNotDepartmentOwner($ticket, $loggedUser))) { $ticketEvent = Ticketevent::getDataStore($fileName, 'file'); if($ticketEvent->isNull()) { - print ''; + Response::respond403(); return; } $ticket = $ticketEvent->ticket; - if($this->isNotAuthor($ticket, $loggedUser) && $this->isNotOwner($ticket, $loggedUser)) { - print ''; + if($this->isNotAuthor($ticket, $loggedUser) && $this->isNotDepartmentOwner($ticket, $loggedUser)) { + Response::respond403(); return; } } @@ -80,17 +80,17 @@ class DownloadController extends Controller { if($session->getTicketNumber()) { return $session->getTicketNumber() !== $ticket->ticketNumber; } else { - return Controller::getLoggedUser()->level >= 1 || $ticket->author->id !== $loggedUser->id; + return $loggedUser->level >= 1 || $ticket->author->id !== $loggedUser->id; } } - private function isNotOwner($ticket, $loggedUser) { + private function isNotDepartmentOwner($ticket, $loggedUser) { $session = Session::getInstance(); if($session->getTicketNumber()) { return $session->getTicketNumber() !== $ticket->ticketNumber; } else { - return !(Controller::getLoggedUser()->level >= 1) || !$ticket->owner || $ticket->owner->id !== $loggedUser->id; + return !($loggedUser->level >= 1) || !$loggedUser->sharedDepartmentList->includesId($ticket->department->id); } } } \ No newline at end of file diff --git a/server/models/Response.php b/server/models/Response.php index 0220a225..077cac89 100755 --- a/server/models/Response.php +++ b/server/models/Response.php @@ -25,4 +25,10 @@ class Response { $app->response->setBody(json_encode($response)); $app->response->finalize(); } + + public static function respond403() { + $app = \Slim\Slim::getInstance(); + $app->response->setStatus(403); + $app->response->finalize(); + } } diff --git a/tests/staff/get.rb b/tests/staff/get.rb index 3d462f64..6470ea25 100644 --- a/tests/staff/get.rb +++ b/tests/staff/get.rb @@ -13,6 +13,7 @@ describe '/staff/get/' do (result['data']['staff']).should.equal(true) (result['data']['email']).should.equal('staff@opensupports.com') (result['data']['level']).should.equal('3') + (result['data']['sendEmailOnNewTicket']).should.equal('1') end it 'should return staff member data with staff Id' do result = request('/staff/get', { @@ -26,5 +27,6 @@ describe '/staff/get/' do (result['data']['staff']).should.equal(true) (result['data']['email']).should.equal('tyrion@opensupports.com') (result['data']['level']).should.equal('2') + (result['data']['sendEmailOnNewTicket']).should.equal('0') end end \ No newline at end of file diff --git a/tests/system/file-upload-download.rb b/tests/system/file-upload-download.rb index ac407196..42da8593 100644 --- a/tests/system/file-upload-download.rb +++ b/tests/system/file-upload-download.rb @@ -37,31 +37,13 @@ describe 'File Upload and Download' do (result.body).should.equal(file.read) end - it 'should not download if author is not logged' do + it 'should download if department owner is logged' do request('/user/logout') Scripts.login('staff@opensupports.com', 'staff', true) - ticket = $database.getLastRow('ticket') - - result = plainRequest('/system/download', { - 'csrf_userid' => $csrf_userid, - 'csrf_token' => $csrf_token, - 'file' => ticket['file'] - }, 'GET') - - (result.body).should.equal('') - end - - it 'should download if owner is logged' do ticket = $database.getLastRow('ticket') file = File.open("../server/files/" + ticket['file']) - request('/staff/assign-ticket', { - 'csrf_userid' => $csrf_userid, - 'csrf_token' => $csrf_token, - 'ticketNumber' => ticket['ticket_number'] - }) - result = plainRequest('/system/download', { 'csrf_userid' => $csrf_userid, 'csrf_token' => $csrf_token,