From a707d604100ab283aa9b20662836a570f0426c60 Mon Sep 17 00:00:00 2001 From: Georgios Papadakis Date: Mon, 8 Aug 2016 03:40:11 +0300 Subject: [PATCH 1/7] Fix allow after soft deny in backend --- .../com_content/controllers/article.php | 35 ++++++------------- 1 file changed, 11 insertions(+), 24 deletions(-) diff --git a/administrator/components/com_content/controllers/article.php b/administrator/components/com_content/controllers/article.php index 077c82c63160d..151fd3454de08 100644 --- a/administrator/components/com_content/controllers/article.php +++ b/administrator/components/com_content/controllers/article.php @@ -84,45 +84,32 @@ protected function allowEdit($data = array(), $key = 'id') { $recordId = (int) isset($data[$key]) ? $data[$key] : 0; $user = JFactory::getUser(); - $userId = $user->get('id'); - // If we get a deny at the component level, we cannot override here. - if (!parent::allowEdit($data, $key)) + // For new record (id:0) return component permission + if (!$recordId) { - return false; + return parent::allowEdit($data, $key); } - // Check general edit permission first. + // Check edit on the record asset (explicit or inherited) if ($user->authorise('core.edit', 'com_content.article.' . $recordId)) { return true; } - // Fallback on edit.own. - // First test if the permission is available. + // Check edit own on the record asset (explicit or inherited) if ($user->authorise('core.edit.own', 'com_content.article.' . $recordId)) { - // Now test the owner is the user. - $ownerId = (int) isset($data['created_by']) ? $data['created_by'] : 0; + // Existing record already has an owner, get it + $record = $this->getModel()->getItem($recordId); - if (empty($ownerId) && $recordId) + if (empty($record)) { - // Need to do a lookup from the model. - $record = $this->getModel()->getItem($recordId); - - if (empty($record)) - { - return false; - } - - $ownerId = $record->created_by; + return false; } - // If the owner matches 'me' then permission is granted. - if ($ownerId == $userId) - { - return true; - } + // Grant if current user is the owner of the record + return $user->get('id') == $record->created_by; } return false; From 1ad1ce2867ac5c405e6bacdc7462facfbf669756 Mon Sep 17 00:00:00 2001 From: Georgios Papadakis Date: Mon, 8 Aug 2016 03:42:33 +0300 Subject: [PATCH 2/7] Improve edit check in frontend similar to backend --- .../com_content/controllers/article.php | 45 ++++++++----------- 1 file changed, 18 insertions(+), 27 deletions(-) diff --git a/components/com_content/controllers/article.php b/components/com_content/controllers/article.php index aea07d99c3750..48d694d174e6c 100644 --- a/components/com_content/controllers/article.php +++ b/components/com_content/controllers/article.php @@ -103,45 +103,36 @@ protected function allowAdd($data = array()) protected function allowEdit($data = array(), $key = 'id') { $recordId = (int) isset($data[$key]) ? $data[$key] : 0; - $user = JFactory::getUser(); - $userId = $user->get('id'); - $asset = 'com_content.article.' . $recordId; + $user = JFactory::getUser(); - // Check general edit permission first. - if ($user->authorise('core.edit', $asset)) + // For new record (id:0) return component permission + if (!$recordId) + { + return parent::allowEdit($data, $key); + } + + // Check edit on the record asset (explicit or inherited) + if ($user->authorise('core.edit', 'com_content.article.' . $recordId)) { return true; } - // Fallback on edit.own. - // First test if the permission is available. - if ($user->authorise('core.edit.own', $asset)) + // Check edit own on the record asset (explicit or inherited) + if ($user->authorise('core.edit.own', 'com_content.article.' . $recordId)) { - // Now test the owner is the user. - $ownerId = (int) isset($data['created_by']) ? $data['created_by'] : 0; + // Existing record already has an owner, get it + $record = $this->getModel()->getItem($recordId); - if (empty($ownerId) && $recordId) + if (empty($record)) { - // Need to do a lookup from the model. - $record = $this->getModel()->getItem($recordId); - - if (empty($record)) - { - return false; - } - - $ownerId = $record->created_by; + return false; } - // If the owner matches 'me' then do the test. - if ($ownerId == $userId) - { - return true; - } + // Grant if current user is the owner of the record + return $user->get('id') == $record->created_by; } - // Since there is no asset tracking, revert to the component permissions. - return parent::allowEdit($data, $key); + return false; } /** From df0bff1bead06f28bf80137927c5c1b7f9d8bce4 Mon Sep 17 00:00:00 2001 From: Georgios Papadakis Date: Wed, 10 Aug 2016 02:47:28 +0300 Subject: [PATCH 3/7] Better comments for the code --- administrator/components/com_content/controllers/article.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/administrator/components/com_content/controllers/article.php b/administrator/components/com_content/controllers/article.php index 151fd3454de08..c000e8e02c691 100644 --- a/administrator/components/com_content/controllers/article.php +++ b/administrator/components/com_content/controllers/article.php @@ -85,7 +85,8 @@ protected function allowEdit($data = array(), $key = 'id') $recordId = (int) isset($data[$key]) ? $data[$key] : 0; $user = JFactory::getUser(); - // For new record (id:0) return component permission + // Zero record (id:0) return component permission, e.g. show edit btn + // for creating new record, allowAdd() must be used instead (core.create) if (!$recordId) { return parent::allowEdit($data, $key); @@ -108,7 +109,7 @@ protected function allowEdit($data = array(), $key = 'id') return false; } - // Grant if current user is the owner of the record + // Grant if current user is owner of the record, note: zero id is guest return $user->get('id') == $record->created_by; } From 001e157796ae1d7d986176befaa76777bf39f568 Mon Sep 17 00:00:00 2001 From: Georgios Papadakis Date: Wed, 10 Aug 2016 03:05:12 +0300 Subject: [PATCH 4/7] Force better usage of allowEdit --- administrator/components/com_content/controllers/article.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/administrator/components/com_content/controllers/article.php b/administrator/components/com_content/controllers/article.php index c000e8e02c691..3eb2a4751370d 100644 --- a/administrator/components/com_content/controllers/article.php +++ b/administrator/components/com_content/controllers/article.php @@ -85,11 +85,10 @@ protected function allowEdit($data = array(), $key = 'id') $recordId = (int) isset($data[$key]) ? $data[$key] : 0; $user = JFactory::getUser(); - // Zero record (id:0) return component permission, e.g. show edit btn - // for creating new record, allowAdd() must be used instead (core.create) + // Zero record (id:0) return false, if caller wants component permissions just get them directly if (!$recordId) { - return parent::allowEdit($data, $key); + return false; } // Check edit on the record asset (explicit or inherited) From 0c7e0a82717e4bccb95b9600aa647dc32e6e58f5 Mon Sep 17 00:00:00 2001 From: Georgios Papadakis Date: Wed, 10 Aug 2016 03:06:42 +0300 Subject: [PATCH 5/7] Force better usage of allowEdit --- components/com_content/controllers/article.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/components/com_content/controllers/article.php b/components/com_content/controllers/article.php index 48d694d174e6c..8162d38b56154 100644 --- a/components/com_content/controllers/article.php +++ b/components/com_content/controllers/article.php @@ -105,10 +105,10 @@ protected function allowEdit($data = array(), $key = 'id') $recordId = (int) isset($data[$key]) ? $data[$key] : 0; $user = JFactory::getUser(); - // For new record (id:0) return component permission + // Zero record (id:0) return false, if caller wants component permissions just get them directly if (!$recordId) { - return parent::allowEdit($data, $key); + return false; } // Check edit on the record asset (explicit or inherited) @@ -128,7 +128,7 @@ protected function allowEdit($data = array(), $key = 'id') return false; } - // Grant if current user is the owner of the record + // Grant if current user is owner of the record, note: zero id is guest return $user->get('id') == $record->created_by; } From d5e378ba484c194164f76de916c249a1c60ab2fe Mon Sep 17 00:00:00 2001 From: Georgios Papadakis Date: Wed, 10 Aug 2016 18:06:18 +0300 Subject: [PATCH 6/7] Restore behaviour on zero record id to return component edit permissions --- .../components/com_content/controllers/article.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/administrator/components/com_content/controllers/article.php b/administrator/components/com_content/controllers/article.php index 3eb2a4751370d..00ba7fe32f4e5 100644 --- a/administrator/components/com_content/controllers/article.php +++ b/administrator/components/com_content/controllers/article.php @@ -85,10 +85,10 @@ protected function allowEdit($data = array(), $key = 'id') $recordId = (int) isset($data[$key]) ? $data[$key] : 0; $user = JFactory::getUser(); - // Zero record (id:0) return false, if caller wants component permissions just get them directly + // Zero record (id:0), return component edit permission by calling parent controller method if (!$recordId) { - return false; + return parent::allowEdit($data, $key); } // Check edit on the record asset (explicit or inherited) @@ -108,7 +108,7 @@ protected function allowEdit($data = array(), $key = 'id') return false; } - // Grant if current user is owner of the record, note: zero id is guest + // Grant if current user is owner of the record return $user->get('id') == $record->created_by; } From 84d0dad8e0c5af80e598dc6db20525ba4e8d2d41 Mon Sep 17 00:00:00 2001 From: Georgios Papadakis Date: Wed, 10 Aug 2016 18:07:17 +0300 Subject: [PATCH 7/7] Restore behaviour on zero record id to return component edit permissions --- components/com_content/controllers/article.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/components/com_content/controllers/article.php b/components/com_content/controllers/article.php index 8162d38b56154..be4ebf16b112d 100644 --- a/components/com_content/controllers/article.php +++ b/components/com_content/controllers/article.php @@ -105,10 +105,10 @@ protected function allowEdit($data = array(), $key = 'id') $recordId = (int) isset($data[$key]) ? $data[$key] : 0; $user = JFactory::getUser(); - // Zero record (id:0) return false, if caller wants component permissions just get them directly + // Zero record (id:0), return component edit permission by calling parent controller method if (!$recordId) { - return false; + return parent::allowEdit($data, $key); } // Check edit on the record asset (explicit or inherited) @@ -128,7 +128,7 @@ protected function allowEdit($data = array(), $key = 'id') return false; } - // Grant if current user is owner of the record, note: zero id is guest + // Grant if current user is owner of the record return $user->get('id') == $record->created_by; }