feat(api): do not silently fail to update entity config

fix #426 Signed-off-by: Thierry Bugier <tbugier@teclib.com>
flyve-mdm · Apr 13, 2018 · 890ab63 · 890ab63
1 parent b7b0fa1
commit 890ab63
Show file tree

Hide file tree

Showing 3 changed files with 92 additions and 1 deletion.
diff --git a/inc/entityconfig.class.php b/inc/entityconfig.class.php
@@ -127,19 +127,32 @@ public function prepareInputForAdd($input) {
     * @return array|false
     */
    public function prepareInputForUpdate($input) {
+      $failure = false;
+
       if (!Session::haveRight(static::$rightname,
          PluginFlyvemdmEntityConfig::RIGHT_FLYVEMDM_DEVICE_COUNT_LIMIT)) {
          unset($input['device_limit']);
+         Session::addMessageAfterRedirect(__('You are not allowed to change the device limit', 'flyvemdm'), false, WARNING);
+         $failure = true;
       }
 
       if (!Session::haveRight(static::$rightname,
          PluginFlyvemdmEntityConfig::RIGHT_FLYVEMDM_APP_DOWNLOAD_URL)) {
          unset($input['download_url']);
+         Session::addMessageAfterRedirect(__('You are not allowed to download URL of the MDM agent', 'flyvemdm'), false, WARNING);
+         $failure = true;
       }
 
       if (!Session::haveRight(static::$rightname,
          PluginFlyvemdmEntityConfig::RIGHT_FLYVEMDM_INVITATION_TOKEN_LIFE)) {
          unset($input['agent_token_life']);
+         Session::addMessageAfterRedirect(__('You are not allowed to change the invitation token life', 'flyvemdm'), false, WARNING);
+         $failure = true;
+      }
+
+      // If the request is done from the API and changing a field is forbidden then fail
+      if (isAPI() && $failure) {
+         return false;
       }
 
       unset($input['entities_id']);

diff --git a/tests/src/Glpi/Tests/CommonTestCase.php b/tests/src/Glpi/Tests/CommonTestCase.php
@@ -190,4 +190,20 @@ protected function restartSession() {
          //$_SESSION["MESSAGE_AFTER_REDIRECT"] = [];
       }
    }
+
+   /**
+    * Tests the session has a specific message
+    * this may be replaced by a custom asserter for atoum
+    * @see http://docs.atoum.org/en/latest/asserters.html#custom-asserter
+    *
+    * @param string $message
+    * @param integer $message_type
+    */
+   protected function sessionHasMessage($message, $message_type = INFO) {
+      if (!is_array($message)) {
+         $message = [$message];
+      }
+      $this->array($_SESSION['MESSAGE_AFTER_REDIRECT'][$message_type])
+         ->containsValues($message);
+   }
 }
diff --git a/tests/suite-unit/PluginFlyvemdmEntityConfig.php b/tests/suite-unit/PluginFlyvemdmEntityConfig.php
@@ -45,7 +45,7 @@ public function beforeTestMethod($method) {
    }
 
    /**
-    * @engine inline
+    *
     */
    public function testCanAddAgent() {
       global $DB;
@@ -88,4 +88,66 @@ public function testCanAddAgent() {
       // Device count limit is reached now
       $this->boolean($entityConfig->canAddAgent($entityId))->isFalse();
    }
+
+   public function providerPrepareInputForUpdate() {
+      return [
+         [
+            'credentials' => ['glpi', 'glpi'],
+            'input' => [
+               'device_limit' => 42,
+               'download_url' => 'https://nothing.local/id=com.nothing.local',
+               'agent_token_life' => 'P99D',
+            ],
+            'output' => [
+               'device_limit' => 42,
+               'download_url' => 'https://nothing.local/id=com.nothing.local',
+               'agent_token_life' => 'P99D',
+            ],
+            'message' => ''
+         ],
+         [
+            ['normal', 'normal'],
+            [
+               'device_limit' => 42,
+               'download_url' => 'https://nothing.local/id=com.nothing.local',
+               'agent_token_life' => 'P99D',
+            ],
+            [],
+            [
+               'You are not allowed to change the device limit',
+               'You are not allowed to download URL of the MDM agent',
+               'You are not allowed to change the invitation token life',
+            ]
+         ]
+      ];
+   }
+
+   /**
+    * @engine inline
+    * @dataProvider providerPrepareInputForUpdate
+    *
+    * @param array $credentials credentials used for login
+    * @param array $input input of the tested method
+    * @param array|boolean $output expected output
+    * @param string $message expected output message (if $output === false or $output === [])
+    */
+   public function testPrepareInputForUpdate(array $credentials, array $input, $output, $message) {
+      // Login
+      $loginSuccess = $this->login($credentials[0], $credentials[1]);
+      $this->boolean($loginSuccess)->isTrue('Failed to login');
+
+      $instance = $this->newTestedInstance();
+      $actualOutput = $instance->prepareInputForUpdate($input);
+      if ($output === false) {
+         $this->boolean($actualOutput)->isFalse();
+         $this->sessionHasMessage($message, WARNING);
+      } else if ($output === []) {
+         $this->array($actualOutput)->isEmpty();
+         $this->sessionHasMessage($message, WARNING);
+      } else {
+         $this->array($actualOutput)->size->isEqualTo(count($output));
+         $this->array($actualOutput)->hasKeys(array_keys($output));
+         $this->array($actualOutput)->containsValues($output);
+      }
+   }
 }