From 017c0a1cb6d62680d211251dd47a86882451b5ec Mon Sep 17 00:00:00 2001
From: Jakub Onderka <jakub.onderka@gmail.com>
Date: Mon, 24 Oct 2022 15:22:56 +0200
Subject: [PATCH] new: [acl] Check sighting deletion in ACLComponent

---
 app/Controller/Component/ACLComponent.php  | 26 ++++++++++++++++++++--
 app/Controller/SightingsController.php     | 11 ++-------
 app/View/Helper/AclHelper.php              | 10 +++++++++
 app/View/Sightings/ajax/list_sightings.ctp | 10 +++------
 app/webroot/js/misp.js                     |  2 +-
 5 files changed, 40 insertions(+), 19 deletions(-)

diff --git a/app/Controller/Component/ACLComponent.php b/app/Controller/Component/ACLComponent.php
index 0d095c19b..184932e1b 100644
--- a/app/Controller/Component/ACLComponent.php
+++ b/app/Controller/Component/ACLComponent.php
@@ -606,10 +606,10 @@ class ACLComponent extends Component
                 'add' => array('perm_sighting'),
                 'restSearch' => array('perm_sighting'),
                 'advanced' => array('perm_sighting'),
-                'delete' => array('perm_sighting'),
+                'delete' => ['AND' => ['perm_sighting', 'perm_modify_org']],
                 'index' => array('*'),
                 'listSightings' => array('*'),
-                'quickDelete' => array('perm_sighting'),
+                'quickDelete' => ['AND' => ['perm_sighting', 'perm_modify_org']],
                 'viewSightings' => array('*'),
                 'bulkSaveSightings' => array('OR' => array('perm_sync', 'perm_sighting')),
                 'filterSightingUuidsForPush' => ['perm_sync'],
@@ -969,6 +969,28 @@ class ACLComponent extends Component
         return $user['org_id'] == $tagCollection['TagCollection']['org_id'];
     }
 
+    /**
+     * Only users that can modify organisation can delete sightings as sighting is not linked to user.
+     *
+     * @param array $user
+     * @param array $sighting
+     * @return bool
+     */
+    public function canDeleteSighting(array $user, array $sighting)
+    {
+        if (!isset($sighting['Sighting'])) {
+            throw new InvalidArgumentException('Passed object does not contain a Sighting.');
+        }
+        // Site admin can delete any sighting
+        if ($user['Role']['perm_site_admin']) {
+            return true;
+        }
+        if (!$user['Role']['perm_modify_org']) {
+            return false;
+        }
+        return $sighting['Sighting']['org_id'] == $user['org_id'];
+    }
+
     private function __checkLoggedActions($user, $controller, $action)
     {
         $loggedActions = array(
diff --git a/app/Controller/SightingsController.php b/app/Controller/SightingsController.php
index 3bd1b547c..bb5259072 100644
--- a/app/Controller/SightingsController.php
+++ b/app/Controller/SightingsController.php
@@ -188,12 +188,8 @@ class SightingsController extends AppController
 
     public function quickDelete($id, $rawId, $context)
     {
-        if (!$this->userRole['perm_modify_org']) {
-            throw new MethodNotAllowedException('You are not authorised to remove sightings data as you don\'t have permission to modify your organisation\'s data.');
-        }
         if (!$this->request->is('post')) {
             $this->set('id', $id);
-            $sighting = $this->Sighting->find('first', array('conditions' => array('Sighting.id' => $id), 'recursive' => -1, 'fields' => array('Sighting.attribute_id')));
             $this->set('rawId', $rawId);
             $this->set('context', $context);
             $this->render('ajax/quickDeleteConfirmationForm');
@@ -205,7 +201,7 @@ class SightingsController extends AppController
                 if (empty($sighting)) {
                     return new CakeResponse(array('body' => json_encode(array('saved' => true, 'errors' => 'Invalid sighting.')), 'status' => 200, 'type' => 'json'));
                 }
-                if (!$this->_isSiteAdmin() && $sighting['Sighting']['org_id'] != $this->Auth->user('org_id')) {
+                if (!$this->ACL->canDeleteSighting($this->Auth->user(), $sighting)) {
                     return new CakeResponse(array('body' => json_encode(array('saved' => true, 'errors' => 'Invalid sighting.')), 'status' => 200, 'type' => 'json'));
                 }
                 $result = $this->Sighting->delete($id);
@@ -221,9 +217,6 @@ class SightingsController extends AppController
     // takes a sighting ID or UUID
     public function delete($id)
     {
-        if (!$this->userRole['perm_modify_org']) {
-            throw new MethodNotAllowedException('You are not authorised to remove sightings data as you don\'t have permission to modify your organisation\'s data.');
-        }
         if (!$this->request->is('post')) {
             throw new MethodNotAllowedException('This action can only be accessed via a post request.');
         }
@@ -235,7 +228,7 @@ class SightingsController extends AppController
         if (empty($sighting)) {
             throw new NotFoundException('Invalid sighting.');
         }
-        if (!$this->_isSiteAdmin() && $sighting['Sighting']['org_id'] != $this->Auth->user('org_id')) {
+        if (!$this->ACL->canDeleteSighting($this->Auth->user(), $sighting)) {
             throw new NotFoundException('Invalid sighting.');
         }
         $result = $this->Sighting->delete($sighting['Sighting']['id']);
diff --git a/app/View/Helper/AclHelper.php b/app/View/Helper/AclHelper.php
index d5418cd97..b03145c5d 100644
--- a/app/View/Helper/AclHelper.php
+++ b/app/View/Helper/AclHelper.php
@@ -73,4 +73,14 @@ class AclHelper extends Helper
         $me = $this->_View->viewVars['me'];
         return $this->ACL->canModifyTagCollection($me, $tagCollection);
     }
+
+    /**
+     * @param array $sighting
+     * @return bool
+     */
+    public function canDeleteSighting(array $sighting)
+    {
+        $me = $this->_View->viewVars['me'];
+        return $this->ACL->canDeleteSighting($me, $sighting);
+    }
 }
\ No newline at end of file
diff --git a/app/View/Sightings/ajax/list_sightings.ctp b/app/View/Sightings/ajax/list_sightings.ctp
index 5bf626c10..eb2954428 100644
--- a/app/View/Sightings/ajax/list_sightings.ctp
+++ b/app/View/Sightings/ajax/list_sightings.ctp
@@ -30,13 +30,9 @@ $deleteSightingTitle = __('Delete sighting');
         <td class="short"><?= h($item['Sighting']['event_id']) ?></td>
         <td class="short"><?= h($item['Sighting']['attribute_id']) ?></td>
         <td class="short action-links">
-          <?php
-            if ($isSiteAdmin || ($item['Sighting']['org_id'] == $me['org_id'] && $isAclAdd)):
-          ?>
-            <span class="fa fa-trash useCursorPointer" title="<?= $deleteSightingTitle ?>" role="button" tabindex="0" aria-label="<?= $deleteSightingTitle ?>" onClick="quickDeleteSighting('<?= h($item['Sighting']['id']) ?>', '<?= h($rawId) ?>', '<?= h($context) ?>');"></span>
-          <?php
-            endif;
-                ?>
+          <?php if ($this->Acl->canDeleteSighting($item)): ?>
+            <span class="fa fa-trash useCursorPointer" title="<?= $deleteSightingTitle ?>" role="button" tabindex="0" aria-label="<?= $deleteSightingTitle ?>" onclick="quickDeleteSighting('<?= h($item['Sighting']['id']) ?>', '<?= h($rawId) ?>', '<?= h($context) ?>');"></span>
+          <?php endif; ?>
         </td>
     </tr>
 <?php
diff --git a/app/webroot/js/misp.js b/app/webroot/js/misp.js
index 8fe762672..1d2d2b4c0 100644
--- a/app/webroot/js/misp.js
+++ b/app/webroot/js/misp.js
@@ -92,7 +92,7 @@ function deleteObject(type, action, id) {
 }
 
 function quickDeleteSighting(id, rawId, context) {
-    url = baseurl + "/sightings/quickDelete/" + id + "/" + rawId + "/" + context;
+    var url = baseurl + "/sightings/quickDelete/" + id + "/" + rawId + "/" + context;
     $.get(url, openConfirmation).fail(xhrFailCallback)
 }