Skip to content

Commit

Permalink
JAccess::checkGroup() doesn't work without an $asset
Browse files Browse the repository at this point in the history
# Executive summary

JAccess::checkGroup() does not work when the optional third argument $asset is not defined. This is in stark contrast with the docblock which states that it "Defaults to the global asset node."

# Technical explanation

The root caused is a botched copy-pasted if-block spanning lines 144-147. It was copied from lines 101-104 except the last line `$asset = $rootId` was never copied. As a result, whenever $asset is empty we look into the `#__assets` table, get the root asset ID... and never use it. This causes null to always be returned, even if the given group does have the requested global privilege enabled.

The solution is to merely change line 144 from `$rootId = $assets->getRootId();` to `$asset = $assets->getRootId();` as it was intended.

# Testing

Unless you are using the affected method in your own custom code you will not be able to reproduce this issue. Apparently everyone was avoiding to use checkGroup because it's buggy but nobody figured out why.
  • Loading branch information
Nicholas K. Dionysopoulos committed Aug 5, 2014
1 parent fe17688 commit 2411f29
Showing 1 changed file with 1 addition and 2 deletions.
3 changes: 1 addition & 2 deletions libraries/joomla/access/access.php
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,9 @@ public static function checkGroup($groupId, $action, $asset = null)
// Default to the root asset node.
if (empty($asset))
{
// TODO: $rootId doesn't seem to be used!
$db = JFactory::getDbo();
$assets = JTable::getInstance('Asset', 'JTable', array('dbo' => $db));
$rootId = $assets->getRootId();
$asset = $assets->getRootId();
}

// Get the rules for the asset recursively to root if not already retrieved.
Expand Down

0 comments on commit 2411f29

Please sign in to comment.