From 08d10b25e073f468ad0095cbce779e4b1964e309 Mon Sep 17 00:00:00 2001 From: Tomasz Narloch Date: Fri, 3 Mar 2017 15:43:29 +0100 Subject: [PATCH] A few improvements for assets, by default new article should have set up inherited permissions --- .../com_config/model/application.php | 167 ++++++++---------- libraries/joomla/access/rules.php | 9 +- libraries/joomla/table/asset.php | 13 +- libraries/legacy/table/content.php | 7 - tests/unit/schema/ddl.sql | 1 + 5 files changed, 85 insertions(+), 112 deletions(-) diff --git a/administrator/components/com_config/model/application.php b/administrator/components/com_config/model/application.php index 7eba0aae27652..b279ba86c28dd 100644 --- a/administrator/components/com_config/model/application.php +++ b/administrator/components/com_config/model/application.php @@ -564,120 +564,101 @@ public function storePermissions($permission = null) try { - // Load the current settings for this component. - $query = $this->db->getQuery(true) - ->select($this->db->quoteName(array('name', 'rules'))) - ->from($this->db->quoteName('#__assets')) - ->where($this->db->quoteName('name') . ' = ' . $this->db->quote($permission['component'])); - - $this->db->setQuery($query); - - // Load the results as a list of stdClass objects (see later for more options on retrieving data). - $results = $this->db->loadAssocList(); - } - catch (Exception $e) - { - $app->enqueueMessage($e->getMessage(), 'error'); - - return false; - } - - // No record found, let's create one. - if (empty($results)) - { - $data = array(); - $data[$permission['action']] = array($permission['rule'] => $permission['value']); + $asset = JTable::getInstance('asset'); + $result = $asset->loadByName($permission['component']); - $rules = new JAccessRules($data); - $asset = JTable::getInstance('asset'); - $asset->rules = (string) $rules; - $asset->name = (string) $permission['component']; - $asset->title = (string) $permission['title']; - - // Get the parent asset id so we have a correct tree. - $parentAsset = JTable::getInstance('Asset'); - - if (strpos($asset->name, '.') !== false) + if ($result === false) { - $assetParts = explode('.', $asset->name); - $parentAsset->loadByName($assetParts[0]); - $parentAssetId = $parentAsset->id; - } - else - { - $parentAssetId = $parentAsset->getRootId(); - } - - /** - * @to do: incorrect ACL stored - * When changing a permission of an item that doesn't have a row in the asset table the row a new row is created. - * This works fine for item <-> component <-> global config scenario and component <-> global config scenario. - * But doesn't work properly for item <-> section(s) <-> component <-> global config scenario, - * because a wrong parent asset id (the component) is stored. - * Happens when there is no row in the asset table (ex: deleted or not created on update). - */ + $data = array($permission['action'] => array($permission['rule'] => $permission['value'])); - $asset->setLocation($parentAssetId, 'last-child'); - - if (!$asset->check() || !$asset->store()) - { - $app->enqueueMessage(JText::_('JLIB_UNKNOWN'), 'error'); + $rules = new JAccessRules($data); + $asset->rules = (string) $rules; + $asset->name = (string) $permission['component']; + $asset->title = (string) $permission['title']; - return false; - } - } - else - { - // Decode the rule settings. - $temp = json_decode($results[0]['rules'], true); + // Get the parent asset id so we have a correct tree. + $parentAsset = JTable::getInstance('Asset'); - // Check if a new value is to be set. - if (isset($permission['value'])) - { - // Check if we already have an action entry. - if (!isset($temp[$permission['action']])) + if (strpos($asset->name, '.') !== false) { - $temp[$permission['action']] = array(); + $assetParts = explode('.', $asset->name); + $parentAsset->loadByName($assetParts[0]); + $parentAssetId = $parentAsset->id; } - - // Check if we already have a rule entry. - if (!isset($temp[$permission['action']][$permission['rule']])) + else { - $temp[$permission['action']][$permission['rule']] = array(); + $parentAssetId = $parentAsset->getRootId(); } - // Set the new permission. - $temp[$permission['action']][$permission['rule']] = (int) $permission['value']; + /** + * @to do: incorrect ACL stored + * When changing a permission of an item that doesn't have a row in the asset table the row a new row is created. + * This works fine for item <-> component <-> global config scenario and component <-> global config scenario. + * But doesn't work properly for item <-> section(s) <-> component <-> global config scenario, + * because a wrong parent asset id (the component) is stored. + * Happens when there is no row in the asset table (ex: deleted or not created on update). + */ - // Check if we have an inherited setting. - if (strlen($permission['value']) === 0) - { - unset($temp[$permission['action']][$permission['rule']]); - } + $asset->setLocation($parentAssetId, 'last-child'); } else { - // There is no value so remove the action as it's not needed. - unset($temp[$permission['action']]); - } + // Decode the rule settings. + $temp = json_decode($asset->rules, true); - // Store the new permissions. - try - { - $query->clear() - ->update($this->db->quoteName('#__assets')) - ->set($this->db->quoteName('rules') . ' = ' . $this->db->quote(json_encode($temp))) - ->where($this->db->quoteName('name') . ' = ' . $this->db->quote($permission['component'])); + // Check if a new value is to be set. + if (isset($permission['value'])) + { + // Check if we already have an action entry. + if (!isset($temp[$permission['action']])) + { + $temp[$permission['action']] = array(); + } + + // Check if we already have a rule entry. + if (!isset($temp[$permission['action']][$permission['rule']])) + { + $temp[$permission['action']][$permission['rule']] = array(); + } + + // Set the new permission. + $temp[$permission['action']][$permission['rule']] = (int) $permission['value']; + + // Check if we have an inherited setting. + if ($permission['value'] === '') + { + unset($temp[$permission['action']][$permission['rule']]); + } + + // Check if we have any rules. + if (!$temp[$permission['action']]) + { + unset($temp[$permission['action']]); + } + } + else + { + // There is no value so remove the action as it's not needed. + unset($temp[$permission['action']]); + } - $this->db->setQuery($query)->execute(); + $asset->rules = json_encode($temp, JSON_FORCE_OBJECT); } - catch (Exception $e) + + if (!$asset->check() || !$asset->store()) { - $app->enqueueMessage($e->getMessage(), 'error'); + $app->enqueueMessage(JText::_('JLIB_UNKNOWN'), 'error'); return false; } } + catch (Exception $e) + { + $app->enqueueMessage($e->getMessage(), 'error'); + + return false; + } + // All checks done. $result = array( @@ -691,7 +672,7 @@ public function storePermissions($permission = null) try { // Get the asset id by the name of the component. - $query->clear() + $query = $this->db->getQuery(true) ->select($this->db->quoteName('id')) ->from($this->db->quoteName('#__assets')) ->where($this->db->quoteName('name') . ' = ' . $this->db->quote($permission['component'])); diff --git a/libraries/joomla/access/rules.php b/libraries/joomla/access/rules.php index 7c443205cf9c9..3f2db3fcfd049 100644 --- a/libraries/joomla/access/rules.php +++ b/libraries/joomla/access/rules.php @@ -209,11 +209,12 @@ public function __toString() foreach ($this->data as $name => $rule) { - // Convert the action to JSON, then back into an array otherwise - // re-encoding will quote the JSON for the identities in the action. - $temp[$name] = json_decode((string) $rule); + if ($data = $rule->getData()) + { + $temp[$name] = $data; + } } - return json_encode($temp); + return json_encode($temp, JSON_FORCE_OBJECT); } } diff --git a/libraries/joomla/table/asset.php b/libraries/joomla/table/asset.php index da207b70eb004..5b4ca02d721b2 100644 --- a/libraries/joomla/table/asset.php +++ b/libraries/joomla/table/asset.php @@ -101,27 +101,24 @@ public function check() { $this->rules = '{}'; } + // JTableNested does not allow parent_id = 0, override this. if ($this->parent_id > 0) { // Get the JDatabaseQuery object $query = $this->_db->getQuery(true) - ->select('COUNT(id)') + ->select('1') ->from($this->_db->quoteName($this->_tbl)) ->where($this->_db->quoteName('id') . ' = ' . $this->parent_id); - $this->_db->setQuery($query); - if ($this->_db->loadResult()) + if ($this->_db->setQuery($query, 0, 1)->loadResult()) { return true; } - else - { - $this->setError('Invalid Parent ID'); - return false; + $this->setError('Invalid Parent ID'); - } + return false; } return true; diff --git a/libraries/legacy/table/content.php b/libraries/legacy/table/content.php index 0f7729d8e8e03..c4814bdbd6fc7 100644 --- a/libraries/legacy/table/content.php +++ b/libraries/legacy/table/content.php @@ -231,13 +231,6 @@ public function check() { $this->metadata = '{}'; } - - // If we don't have any access rules set at this point just use an empty JAccessRules class - if (!$this->getRules()) - { - $rules = $this->getDefaultAssetValues('com_content'); - $this->setRules($rules); - } } // Check the publish down date is not earlier than publish up. diff --git a/tests/unit/schema/ddl.sql b/tests/unit/schema/ddl.sql index 4e8cb249658bf..56b75692a6a61 100644 --- a/tests/unit/schema/ddl.sql +++ b/tests/unit/schema/ddl.sql @@ -327,6 +327,7 @@ CREATE TABLE `jos_menu_types` ( CREATE TABLE `jos_modules` ( `id` INTEGER PRIMARY KEY AUTOINCREMENT, + `asset_id` INTEGER NOT NULL DEFAULT '0', `title` TEXT NOT NULL DEFAULT '', `note` TEXT NOT NULL DEFAULT '', `content` TEXT NOT NULL DEFAULT '',