Ivan - Fix issue #27, fix frontend<->backend data flow with staff editor, add 403 response

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,

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),

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
         });
     }
 

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;
+    }
 }

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
         ]);
     }
 }

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)) {
+                if($this->isNotAuthor($ticket, $loggedUser) && $this->isNotDepartmentOwner($ticket, $loggedUser)) {
-                    print '';
+                    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);
         }
     }
 }

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();
+    }
 }

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

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,