Skip to content

Commit

Permalink
fix(permissions): All permissions functions handle user fetches consi…
Browse files Browse the repository at this point in the history 
…stently

All permissions functions now find hidden/disabled users and, when a
given GUID isn't found, they return false and issue a warning. Previously,
only some functions issued warnings, and some functions would silently
substitute the logged in user if a given GUID couldn't load.

This also adds functionality to the internal entity getter allowing fetching
entities that may be hidden/disabled.

Fixes #8941
Fixes #8038
  • Loading branch information
@mrclay
mrclay committed Sep 16, 2015
1 parent 367d3c3 commit 09d49f1
Show file tree
Hide file tree
Showing 8 changed files with 147 additions and 90 deletions.
14 changes: 8 additions & 6 deletions engine/classes/Elgg/Database/AccessCollections.php
View file
Expand Up @@ -2,6 +2,8 @@

namespace Elgg\Database;

use Elgg\Database\EntityTable\UserFetchFailureException;

/**
* WARNING: API IN FLUX. DO NOT USE DIRECTLY.
*
Expand Down Expand Up @@ -436,19 +438,19 @@ function getWriteAccessArray($user_guid = 0, $site_guid = 0, $flush = false, arr
* @return bool
*/
function canEdit($collection_id, $user_guid = null) {
if ($user_guid) {
$user = _elgg_services()->entityTable->get((int) $user_guid);
} else {
$user = _elgg_services()->session->getLoggedInUser();
try {
$user = _elgg_services()->entityTable->getUserForPermissionsCheck($user_guid);
} catch (UserFetchFailureException $e) {
return false;
}

$collection = get_access_collection($collection_id);

if (!($user instanceof \ElggUser) || !$collection) {
if (!$user || !$collection) {
return false;
}

$write_access = get_write_access_array($user->getGUID(), 0, true);
$write_access = get_write_access_array($user->guid, 0, true);

// don't ignore access when checking users.
if ($user_guid) {
Expand Down
83 changes: 70 additions & 13 deletions engine/classes/Elgg/Database/EntityTable.php
View file
@@ -1,6 +1,7 @@
<?php
namespace Elgg\Database;

use Elgg\Database\EntityTable\UserFetchFailureException;
use IncompleteEntityException;

/**
Expand Down Expand Up @@ -140,13 +141,16 @@ function rowToElggStar($row) {
/**
* Loads and returns an entity object from a guid.
*
* @param int $guid The GUID of the entity
* @param string $type The type of the entity. If given, even an existing entity with the given GUID
* will not be returned unless its type matches.
* @param int $guid Entity GUID
* @param string $type Entity type. If given, even an existing entity with the given GUID
* will not be returned unless its type matches.
* @param string $subtype Entity subtype (use "" for default subtype). If given, even an existing
* entity with the given GUID will not be returned unless this matches.
* @param bool $ignore_condition If true, hidden/disabled entities may be found
*
* @return \ElggEntity The correct Elgg or custom object based upon entity type and subtype
*/
function get($guid, $type = '') {
function get($guid, $type = '', $subtype = null, $ignore_condition = false) {
// We could also use: if (!(int) $guid) { return false },
// but that evaluates to a false positive for $guid = true.
// This is a bit slower, but more thorough.
Expand All @@ -157,22 +161,48 @@ function get($guid, $type = '') {
// Check local cache first
$new_entity = _elgg_retrieve_cached_entity($guid);
if ($new_entity) {
if ($type) {
return elgg_instanceof($new_entity, $type) ? $new_entity : false;
// manually check subtype https://github.com/Elgg/Elgg/issues/8943
if (is_string($subtype) && $new_entity->getSubtype() !== $subtype) {
return false;
}
if ($type && !elgg_instanceof($new_entity, $type)) {
return false;
}
return $new_entity;
}

$options = [
if ($ignore_condition) {
// need to ignore access and show hidden entities for potential hidden/disabled users
$ia = _elgg_services()->session->setIgnoreAccess(true);
$show_hidden = access_show_hidden_entities(true);
}

$entities = $this->getEntities([
'guid' => $guid,
'limit' => 1,
'site_guids' => ELGG_ENTITIES_ANY_VALUE, // for BC with get_entity, allow matching any site
];
if ($type) {
$options['type'] = $type;
]);
/* @var \ElggEntity[] $entities */

if ($ignore_condition) {
_elgg_services()->session->setIgnoreAccess($ia);
access_show_hidden_entities($show_hidden);
}

if (!$entities) {
return false;
}
$entity = $entities[0];

// manually check subtype https://github.com/Elgg/Elgg/issues/8943
if (is_string($subtype) && $entity->getSubtype() !== $subtype) {
return false;
}
if ($type && !elgg_instanceof($entity, $type)) {
return false;
}
$entities = $this->getEntities($options);
return $entities ? $entities[0] : false;

return $entity;
}

/**
Expand Down Expand Up @@ -1235,4 +1265,31 @@ function updateLastAction($guid, $posted = null) {
return false;
}
}
}

/**
* Get a user by GUID even if the entity is hidden or disabled
*
* @param int $guid User GUID. Default is logged in user
*
* @return \ElggUser|false
* @throws UserFetchFailureException
* @access private
*/
public function getUserForPermissionsCheck($guid = 0) {
if (!$guid) {
return _elgg_services()->session->getLoggedInUser();
}

$user = $this->get($guid, 'user', null, true);
if (!$user) {
// requested to check access for a specific user_guid, but there is no user entity, so the caller
// should cancel the check and return false
$message = _elgg_services()->translator->translate('UserFetchFailureException', array($guid));
_elgg_services()->logger->warn($message);

throw new UserFetchFailureException();
}

return $user;
}
}
8 changes: 8 additions & 0 deletions engine/classes/Elgg/Database/EntityTable/UserFetchResultException.php
View file
@@ -0,0 +1,8 @@
<?php

namespace Elgg\Database\EntityTable;

/**
* Exception indicating a user could not be looked up for a permissions check
*/
class UserFetchFailureException extends \RuntimeException {}
9 changes: 6 additions & 3 deletions engine/classes/Elgg/WidgetsService.php
View file
@@ -1,6 +1,8 @@
<?php
namespace Elgg;

use Elgg\Database\EntityTable\UserFetchFailureException;

/**
* WARNING: API IN FLUX. DO NOT USE DIRECTLY.
*
Expand Down Expand Up @@ -108,9 +110,10 @@ public function createWidget($owner_guid, $handler, $context, $access_id = null)
* @since 1.9.0
*/
public function canEditLayout($context, $user_guid = 0) {
$user = get_entity((int)$user_guid);
if (!$user) {
$user = _elgg_services()->session->getLoggedInUser();
try {
$user = _elgg_services()->entityTable->getUserForPermissionsCheck($user_guid);
} catch (UserFetchFailureException $e) {
return false;
}

$return = false;
Expand Down
23 changes: 11 additions & 12 deletions engine/classes/ElggAnnotation.php
View file
@@ -1,4 +1,7 @@
<?php

use Elgg\Database\EntityTable\UserFetchFailureException;

/**
* Elgg Annotations
*
Expand Down Expand Up @@ -120,27 +123,23 @@ public function enable() {
* @see elgg_set_ignore_access()
*/
public function canEdit($user_guid = 0) {

if ($user_guid) {
$user = get_user($user_guid);
if (!$user) {
return false;
}
} else {
$user = _elgg_services()->session->getLoggedInUser();
$user_guid = _elgg_services()->session->getLoggedInUserGuid();
try {
$user = _elgg_services()->entityTable->getUserForPermissionsCheck($user_guid);
} catch (UserFetchFailureException $e) {
return false;
}

$result = false;
$entity = $this->getEntity();

if ($user) {
// If the owner of annotation is the specified user, they can edit.
if ($this->getOwnerGUID() == $user_guid) {
if ($this->owner_guid == $user->guid) {
$result = true;
}

// If the user can edit the entity this is attached to, they can edit.
$entity = $this->getEntity();
if ($result == false && $entity->canEdit($user->getGUID())) {
if ($result == false && $entity->canEdit($user->guid)) {
$result = true;
}
}
Expand Down
77 changes: 31 additions & 46 deletions engine/classes/ElggEntity.php
View file
@@ -1,4 +1,7 @@
<?php

use Elgg\Database\EntityTable\UserFetchFailureException;

/**
* The parent class for all Elgg Entities.
*
Expand Down Expand Up @@ -991,30 +994,30 @@ public function countEntitiesFromRelationship($relationship, $inverse_relationsh
* @see elgg_set_ignore_access()
*/
public function canEdit($user_guid = 0) {
$user_guid = (int)$user_guid;
$user = get_entity($user_guid);
if (!$user) {
$user = _elgg_services()->session->getLoggedInUser();
try {
$user = _elgg_services()->entityTable->getUserForPermissionsCheck($user_guid);
} catch (UserFetchFailureException $e) {
return false;
}

$return = false;

// Test user if possible - should default to false unless a plugin hook says otherwise
if ($user) {
if ($this->getOwnerGUID() == $user->getGUID()) {
if ($this->owner_guid == $user->guid) {
$return = true;
}

if ($this->getContainerGUID() == $user->getGUID()) {
if ($this->container_guid == $user->guid) {
$return = true;
}

if ($this->getGUID() == $user->getGUID()) {
if ($this->guid == $user->guid) {
$return = true;
}

$container = $this->getContainerEntity();
if ($container && $container->canEdit($user->getGUID())) {
if ($container && $container->canEdit($user->guid)) {
$return = true;
}
}
Expand All @@ -1035,29 +1038,12 @@ public function canEdit($user_guid = 0) {
* @see elgg_set_ignore_access()
*/
public function canDelete($user_guid = 0) {
$user_guid = (int) $user_guid;

if (!$user_guid) {
$user_guid = _elgg_services()->session->getLoggedInUserGuid();
}

// need to ignore access and show hidden entities for potential hidden/disabled users
$ia = elgg_set_ignore_access(true);
$show_hidden = access_show_hidden_entities(true);

$user = _elgg_services()->entityTable->get($user_guid, 'user');

elgg_set_ignore_access($ia);
access_show_hidden_entities($show_hidden);

if ($user_guid & !$user) {
// requested to check access for a specific user_guid, but there is no user entity, so return false
$message = _elgg_services()->translator->translate('entity:can_delete:invaliduser', array($user_guid));
_elgg_services()->logger->warning($message);

try {
$user = _elgg_services()->entityTable->getUserForPermissionsCheck($user_guid);
} catch (UserFetchFailureException $e) {
return false;
}

$return = $this->canEdit($user_guid);

$params = array('entity' => $this, 'user' => $user);
Expand Down Expand Up @@ -1085,13 +1071,13 @@ public function canEditMetadata($metadata = null, $user_guid = 0) {
return false;
}

if ($user_guid) {
$user = get_user($user_guid);
if (!$user) {
return false;
}
} else {
$user = _elgg_services()->session->getLoggedInUser();
try {
$user = _elgg_services()->entityTable->getUserForPermissionsCheck($user_guid);
} catch (UserFetchFailureException $e) {
return false;
}

if ($user) {
$user_guid = $user->guid;
}

Expand Down Expand Up @@ -1136,10 +1122,11 @@ public function canWriteToContainer($user_guid = 0, $type = 'all', $subtype = 'a
* @return bool
*/
public function canComment($user_guid = 0) {
if ($user_guid == 0) {
$user_guid = _elgg_services()->session->getLoggedInUserGuid();
try {
$user = _elgg_services()->entityTable->getUserForPermissionsCheck($user_guid);
} catch (UserFetchFailureException $e) {
return false;
}
$user = get_entity($user_guid);

// By default, we don't take a position of whether commenting is allowed
// because it is handled by the subclasses of \ElggEntity
Expand All @@ -1162,15 +1149,13 @@ public function canComment($user_guid = 0) {
* @return bool
*/
public function canAnnotate($user_guid = 0, $annotation_name = '') {
if ($user_guid == 0) {
$user_guid = _elgg_services()->session->getLoggedInUserGuid();
try {
$user = _elgg_services()->entityTable->getUserForPermissionsCheck($user_guid);
} catch (UserFetchFailureException $e) {
return false;
}
$user = get_entity($user_guid);

$return = true;
if (!$user) {
$return = false;
}
$return = (bool)$user;

$hooks = _elgg_services()->hooks;

Expand Down

0 comments on commit 09d49f1

Please sign in to comment.