Skip to content

Commit e38b116

Browse files
committed
Regression: Fix edit check in backend articles manager, always denying edit after soft deny (joomla#11511)
* Fix allow after soft deny in backend * Improve edit check in frontend similar to backend * Better comments for the code * Force better usage of allowEdit * Force better usage of allowEdit * Restore behaviour on zero record id to return component edit permissions * Restore behaviour on zero record id to return component edit permissions
1 parent 98fe47b commit e38b116

File tree

2 files changed

+29
-51
lines changed

2 files changed

+29
-51
lines changed

administrator/components/com_content/controllers/article.php

Lines changed: 11 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -84,45 +84,32 @@ protected function allowEdit($data = array(), $key = 'id')
8484
{
8585
$recordId = (int) isset($data[$key]) ? $data[$key] : 0;
8686
$user = JFactory::getUser();
87-
$userId = $user->get('id');
8887

89-
// If we get a deny at the component level, we cannot override here.
90-
if (!parent::allowEdit($data, $key))
88+
// Zero record (id:0), return component edit permission by calling parent controller method
89+
if (!$recordId)
9190
{
92-
return false;
91+
return parent::allowEdit($data, $key);
9392
}
9493

95-
// Check general edit permission first.
94+
// Check edit on the record asset (explicit or inherited)
9695
if ($user->authorise('core.edit', 'com_content.article.' . $recordId))
9796
{
9897
return true;
9998
}
10099

101-
// Fallback on edit.own.
102-
// First test if the permission is available.
100+
// Check edit own on the record asset (explicit or inherited)
103101
if ($user->authorise('core.edit.own', 'com_content.article.' . $recordId))
104102
{
105-
// Now test the owner is the user.
106-
$ownerId = (int) isset($data['created_by']) ? $data['created_by'] : 0;
103+
// Existing record already has an owner, get it
104+
$record = $this->getModel()->getItem($recordId);
107105

108-
if (empty($ownerId) && $recordId)
106+
if (empty($record))
109107
{
110-
// Need to do a lookup from the model.
111-
$record = $this->getModel()->getItem($recordId);
112-
113-
if (empty($record))
114-
{
115-
return false;
116-
}
117-
118-
$ownerId = $record->created_by;
108+
return false;
119109
}
120110

121-
// If the owner matches 'me' then permission is granted.
122-
if ($ownerId == $userId)
123-
{
124-
return true;
125-
}
111+
// Grant if current user is owner of the record
112+
return $user->get('id') == $record->created_by;
126113
}
127114

128115
return false;

components/com_content/controllers/article.php

Lines changed: 18 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -103,45 +103,36 @@ protected function allowAdd($data = array())
103103
protected function allowEdit($data = array(), $key = 'id')
104104
{
105105
$recordId = (int) isset($data[$key]) ? $data[$key] : 0;
106-
$user = JFactory::getUser();
107-
$userId = $user->get('id');
108-
$asset = 'com_content.article.' . $recordId;
106+
$user = JFactory::getUser();
109107

110-
// Check general edit permission first.
111-
if ($user->authorise('core.edit', $asset))
108+
// Zero record (id:0), return component edit permission by calling parent controller method
109+
if (!$recordId)
110+
{
111+
return parent::allowEdit($data, $key);
112+
}
113+
114+
// Check edit on the record asset (explicit or inherited)
115+
if ($user->authorise('core.edit', 'com_content.article.' . $recordId))
112116
{
113117
return true;
114118
}
115119

116-
// Fallback on edit.own.
117-
// First test if the permission is available.
118-
if ($user->authorise('core.edit.own', $asset))
120+
// Check edit own on the record asset (explicit or inherited)
121+
if ($user->authorise('core.edit.own', 'com_content.article.' . $recordId))
119122
{
120-
// Now test the owner is the user.
121-
$ownerId = (int) isset($data['created_by']) ? $data['created_by'] : 0;
123+
// Existing record already has an owner, get it
124+
$record = $this->getModel()->getItem($recordId);
122125

123-
if (empty($ownerId) && $recordId)
126+
if (empty($record))
124127
{
125-
// Need to do a lookup from the model.
126-
$record = $this->getModel()->getItem($recordId);
127-
128-
if (empty($record))
129-
{
130-
return false;
131-
}
132-
133-
$ownerId = $record->created_by;
128+
return false;
134129
}
135130

136-
// If the owner matches 'me' then do the test.
137-
if ($ownerId == $userId)
138-
{
139-
return true;
140-
}
131+
// Grant if current user is owner of the record
132+
return $user->get('id') == $record->created_by;
141133
}
142134

143-
// Since there is no asset tracking, revert to the component permissions.
144-
return parent::allowEdit($data, $key);
135+
return false;
145136
}
146137

147138
/**

0 commit comments

Comments
 (0)