Filemanager - fix notification update acl check
authorPaul Mehrer <p.mehrer@metaways.de>
Mon, 10 Jul 2017 11:04:06 +0000 (13:04 +0200)
committerPaul Mehrer <p.mehrer@metaways.de>
Mon, 10 Jul 2017 12:23:00 +0000 (14:23 +0200)
Change-Id: I201c42f4c44a2d784dfe1780cb36bbec9130950b
Reviewed-on: http://gerrit.tine20.com/customers/5069
Tested-by: Jenkins CI (http://ci.tine20.com/)
Reviewed-by: Paul Mehrer <p.mehrer@metaways.de>
Tested-by: Paul Mehrer <p.mehrer@metaways.de>
tests/tine20/Filemanager/ControllerTests.php
tine20/Filemanager/Controller/Node.php
tine20/Tinebase/FileSystem.php

index b46d31a..25c6189 100644 (file)
@@ -107,14 +107,28 @@ class Filemanager_ControllerTests extends TestCase
             $node = $fileManager->update($node);
 
             // do update again, it should work now
+            // test that updates to other things than own notification are silently dropped
             Tinebase_Core::set(Tinebase_Core::USER, $sclever);
-            $scleverNotificationProps = array(
+            $notificationProps = array(array(
                 Tinebase_Model_Tree_Node::XPROPS_NOTIFICATION_ACCOUNT_ID => $sclever->getId(),
                 Tinebase_Model_Tree_Node::XPROPS_NOTIFICATION_ACCOUNT_TYPE => Tinebase_Acl_Rights::ACCOUNT_TYPE_USER,
                 Tinebase_Model_Tree_Node::XPROPS_NOTIFICATION_ACTIVE => true,
-            );
-            $node->xprops(Tinebase_Model_Tree_Node::XPROPS_NOTIFICATION)[] = $scleverNotificationProps;
+            ),array(
+                Tinebase_Model_Tree_Node::XPROPS_NOTIFICATION_ACCOUNT_ID => '1233',
+                Tinebase_Model_Tree_Node::XPROPS_NOTIFICATION_ACCOUNT_TYPE => Tinebase_Acl_Rights::ACCOUNT_TYPE_USER,
+                Tinebase_Model_Tree_Node::XPROPS_NOTIFICATION_ACTIVE => true,
+            ),array(
+                Tinebase_Model_Tree_Node::XPROPS_NOTIFICATION_ACCOUNT_ID => $sclever->getId(),
+                Tinebase_Model_Tree_Node::XPROPS_NOTIFICATION_ACCOUNT_TYPE => Tinebase_Acl_Rights::ACCOUNT_TYPE_GROUP,
+                Tinebase_Model_Tree_Node::XPROPS_NOTIFICATION_ACTIVE => true,
+            ));
+            $node->{Tinebase_Model_Tree_Node::XPROPS_NOTIFICATION} = $notificationProps;
+            $oldDescription = $node->description;
+            static::assertNotEquals('test', $oldDescription, 'test data bad, the description must not be "test"');
+            $node->description = 'test';
+
             $node = $fileManager->update($node);
+            static::assertEquals($oldDescription, $node->description, 'description should not have been updated!');
             static::assertEquals(1, count($node->xprops(Tinebase_Model_Tree_Node::XPROPS_NOTIFICATION)));
             static::assertTrue(
                 isset($node->xprops(Tinebase_Model_Tree_Node::XPROPS_NOTIFICATION)[0]) &&
index c45316c..7d5472c 100644 (file)
@@ -103,11 +103,12 @@ class Filemanager_Controller_Node extends Tinebase_Controller_Record_Abstract
      */
     public function update(Tinebase_Record_Interface $_record)
     {
-        if (! $this->_backend->checkACLNode($_record, 'update')) {
+        // we allow only notification updates for the current user itself if not admin right
+        if (! $this->_backend->checkACLNode($_record, 'admin')) {
             if (! $this->_backend->checkACLNode($_record, 'get')) {
                 throw new Tinebase_Exception_AccessDenied('No permission to update nodes.');
             }
-            // we allow only notification updates for the current user itself
+
             $usersNotificationSettings = null;
             $currentUserId = Tinebase_Core::getUser()->getId();
             foreach ($_record->xprops(Tinebase_Model_Tree_Node::XPROPS_NOTIFICATION) as $xpNotification) {
@@ -120,8 +121,18 @@ class Filemanager_Controller_Node extends Tinebase_Controller_Record_Abstract
                 }
             }
 
-            // we reset all input and then just apply the notification settings for the current user
-            $_record = $this->get($_record->getId());
+            $currentRecord = $this->get($_record->getId());
+
+            if (! $this->_backend->checkACLNode($_record, 'update')) {
+                // we reset all input and then just apply the notification settings for the current user
+                $_record = $currentRecord;
+                $hasUpdateGrant = false;
+            } else {
+                // we just reset the notification settings
+                $_record->{Tinebase_Model_Tree_Node::XPROPS_NOTIFICATION} = $currentRecord->xprops(Tinebase_Model_Tree_Node::XPROPS_NOTIFICATION);
+                $hasUpdateGrant = true;
+            }
+
             $found = false;
             foreach ($_record->xprops(Tinebase_Model_Tree_Node::XPROPS_NOTIFICATION) as $key => &$xpNotification) {
                 if (isset($xpNotification[Tinebase_Model_Tree_Node::XPROPS_NOTIFICATION_ACCOUNT_ID]) &&
@@ -141,7 +152,7 @@ class Filemanager_Controller_Node extends Tinebase_Controller_Record_Abstract
                 $_record->xprops(Tinebase_Model_Tree_Node::XPROPS_NOTIFICATION)[] = $usersNotificationSettings;
             }
 
-            if (false === $found && null === $usersNotificationSettings){
+            if (false === $hasUpdateGrant && false === $found && null === $usersNotificationSettings){
                 throw new Tinebase_Exception_AccessDenied('No permission to update nodes.');
             }
         }
index 4a70909..6654ca2 100644 (file)
@@ -2322,6 +2322,8 @@ class Tinebase_FileSystem implements
             case 'delete':
                 $requiredGrant = Tinebase_Model_Grants::GRANT_DELETE;
                 break;
+            case 'admin':
+                return false;
             default:
                 throw new Tinebase_Exception_UnexpectedValue('Unknown action: ' . $_action);
         }