diff --git a/engine/classes/Elgg/Database/AccessCollections.php b/engine/classes/Elgg/Database/AccessCollections.php index 0cd648642a8..368b158c9f0 100644 --- a/engine/classes/Elgg/Database/AccessCollections.php +++ b/engine/classes/Elgg/Database/AccessCollections.php @@ -2,6 +2,8 @@ namespace Elgg\Database; +use Elgg\Database\EntityTable\UserFetchFailureException; + /** * WARNING: API IN FLUX. DO NOT USE DIRECTLY. * @@ -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) { diff --git a/engine/classes/Elgg/Database/EntityTable.php b/engine/classes/Elgg/Database/EntityTable.php index b0f6593dc6a..d0a916f5d27 100644 --- a/engine/classes/Elgg/Database/EntityTable.php +++ b/engine/classes/Elgg/Database/EntityTable.php @@ -1,6 +1,7 @@ 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; } /** @@ -1235,4 +1265,31 @@ function updateLastAction($guid, $posted = null) { return false; } } -} \ No newline at end of file + + /** + * 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; + } +} diff --git a/engine/classes/Elgg/Database/EntityTable/UserFetchResultException.php b/engine/classes/Elgg/Database/EntityTable/UserFetchResultException.php new file mode 100644 index 00000000000..e2633076b32 --- /dev/null +++ b/engine/classes/Elgg/Database/EntityTable/UserFetchResultException.php @@ -0,0 +1,8 @@ +session->getLoggedInUser(); + try { + $user = _elgg_services()->entityTable->getUserForPermissionsCheck($user_guid); + } catch (UserFetchFailureException $e) { + return false; } $return = false; diff --git a/engine/classes/ElggAnnotation.php b/engine/classes/ElggAnnotation.php index 92cc622cb90..44b35ebba1b 100644 --- a/engine/classes/ElggAnnotation.php +++ b/engine/classes/ElggAnnotation.php @@ -1,4 +1,7 @@ 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; } } diff --git a/engine/classes/ElggEntity.php b/engine/classes/ElggEntity.php index 633e6b8b982..cb6b78d01a7 100644 --- a/engine/classes/ElggEntity.php +++ b/engine/classes/ElggEntity.php @@ -1,4 +1,7 @@ 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; } } @@ -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); @@ -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; } @@ -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 @@ -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; diff --git a/engine/lib/entities.php b/engine/lib/entities.php index f15c91b3130..e48eae6b717 100644 --- a/engine/lib/entities.php +++ b/engine/lib/entities.php @@ -6,6 +6,8 @@ * @subpackage DataModel.Entities */ +use Elgg\Database\EntityTable\UserFetchFailureException; + /** * Cache entities in memory once loaded. * @@ -302,15 +304,14 @@ function can_write_to_container($user_guid = 0, $container_guid = 0, $type = 'al $container = get_entity($container_guid); - $user_guid = (int)$user_guid; - if ($user_guid == 0) { - $user = elgg_get_logged_in_user_entity(); - $user_guid = elgg_get_logged_in_user_guid(); - } else { - $user = get_user($user_guid); - if (!$user) { - return false; - } + try { + $user = _elgg_services()->entityTable->getUserForPermissionsCheck($user_guid); + } catch (UserFetchFailureException $e) { + return false; + } + + if ($user) { + $user_guid = $user->guid; } $return = false; diff --git a/languages/en.php b/languages/en.php index 193cfd1c0f9..5d7b27477e4 100644 --- a/languages/en.php +++ b/languages/en.php @@ -100,6 +100,8 @@ 'LoginException:ChangePasswordFailure' => 'Failed current password check.', 'LoginException:Unknown' => 'We could not log you in due to an unknown error.', + 'UserFetchFailureException' => 'Cannot check permission for user_guid [%s] as the user does not exist.', + 'deprecatedfunction' => 'Warning: This code uses the deprecated function \'%s\' and is not compatible with this version of Elgg', 'pageownerunavailable' => 'Warning: The page owner %d is not accessible!', @@ -1287,7 +1289,7 @@ 'entity:delete:success' => 'Entity %s has been deleted', 'entity:delete:fail' => 'Entity %s could not be deleted', - 'entity:can_delete:invaliduser' => 'Can not check canDelete for user_guid [%s] as the user does not exist.', + 'entity:can_delete:invaliduser' => 'Cannot check canDelete for user_guid [%s] as the user does not exist.', /** * Action gatekeeper