fix(access): updates no longer mistakenly blocked in some scenarios

In order to fix `canEdit()` in 1.9, `update()` needed to load a fresh copy of
entity from the DB to check the persisted attributes. In that fix,
66eb9e6,
a corner case was checked out of my paranoia that a dev would load an
invisible entity with access control on, but try to delete it with access
off.

I now believe this check was unnecessary (we never did similar checks for
`delete()`). When the original `canEdit()` attributes issue was resolved in
#9434, I should've made this change.

engine/classes/ElggEntity.php

@@ -1592,12 +1592,6 @@ protected function update() {
 
 		_elgg_services()->boot->invalidateCache($this->guid);
 
-		if (!has_access_to_entity($this)) {
-			// Why worry about this case? If access control was off when the user fetched $this, but
-			// was turned back on again. Better to just bail than to turn access control off again.
-			return false;
-		}
-
 		if (!$this->canEdit()) {
 			return false;
 		}

engine/classes/ElggSession.php

@@ -209,7 +209,7 @@ public function setLoggedInUser(\ElggUser $user) {
 	/**
 	 * Gets the logged in user
 	 * 
-	 * @return \ElggUser
+	 * @return \ElggUser|null
 	 * @since 1.9
 	 */
 	public function getLoggedInUser() {

engine/tests/ElggCoreMetadataAPITest.php

@@ -161,8 +161,7 @@ public function test_elgg_metadata_multiple_values() {
 
 		// need to fake different logins.
 		// good times without mocking.
-		$original_user = elgg_get_logged_in_user_entity();
-		$_SESSION['user'] = $u1;
+		$original_user = $this->replaceSession($u1);
 
 		elgg_set_ignore_access(false);
 
@@ -182,7 +181,7 @@ public function test_elgg_metadata_multiple_values() {
 		}
 
 		// add md w/ same name as a different user
-		$_SESSION['user'] = $u2;
+		$this->replaceSession($u2);
 		$md_values2 = array(
 			'four',
 			'five',
@@ -202,7 +201,7 @@ public function test_elgg_metadata_multiple_values() {
 			$this->assertEqual('test', $md->name);
 		}
 
-		$_SESSION['user'] = $original_user;
+		$this->replaceSession($original_user);
 
 		$obj->delete();
 		$u1->delete();

engine/tests/ElggCoreUnitTest.php

@@ -48,6 +48,23 @@ public function assertIdenticalEntities(\ElggEntity $first, \ElggEntity $second,
 		return $this->assert(new IdenticalEntityExpectation($first), $second, $message);
 	}
 
+	/**
+	 * Replace the current user session
+	 *
+	 * @param ElggUser $user New user to login as (null to log out)
+	 * @return ElggUser|null Removed session user (or null)
+	 */
+	public function replaceSession(ElggUser $user = null) {
+		$session = elgg_get_session();
+		$old = $session->getLoggedInUser();
+		if ($user) {
+			$session->setLoggedInUser($user);
+		} else {
+			$session->removeLoggedInUser();
+		}
+		return $old;
+	}
+
 }
 
 /**

engine/tests/ElggEntityTest.php

@@ -379,4 +379,25 @@ public function testCreateWithContainerGuidEqualsZero() {
 		$user->delete();
 
 	}
+
+	public function testUpdateAbilityDependsOnCanEdit() {
+		$this->entity->access_id = ACCESS_PRIVATE;
+
+		$user = new ElggUser();
+		$user->save();
+		$old_user = $this->replaceSession($user);
+
+		$this->assertFalse($this->entity->save());
+
+		elgg_register_plugin_hook_handler('permissions_check', 'object', [Elgg\Values::class, 'getTrue']);
+
+		// even though this user can't look up the entity via the DB, permission allows update.
+		$this->assertFalse(has_access_to_entity($this->entity, $user));
+		$this->assertTrue($this->entity->save());
+
+		elgg_unregister_plugin_hook_handler('permissions_check', 'object', [Elgg\Values::class, 'getTrue']);
+
+		$this->replaceSession($old_user);
+		$user->delete();
+	}
 }

engine/tests/ElggObjectTest.php

@@ -201,14 +201,13 @@ public function testElggEntityRecursiveDisableWhenLoggedOut() {
 		$guid2 = $e2->getGUID();
 
 		// fake being logged out
-		$user = $_SESSION['user'];
-		unset($_SESSION['user']);
+		$old_user = $this->replaceSession();
 		$ia = elgg_set_ignore_access(true);
 
 		$this->assertTrue($e1->disable(null, true));
 
 		// "log in" original user
-		$_SESSION['user'] = $user;
+		$this->replaceSession($old_user);
 		elgg_set_ignore_access($ia);
 
 		$this->assertFalse(get_entity($guid1));

engine/tests/ElggUserTest.php

@@ -98,7 +98,7 @@ public function testElggUserConstructorWithGarbage() {
 	public function testElggUserConstructorByDbRow() {
 		$row = $this->fetchUser(elgg_get_logged_in_user_guid());
 		$user = new \ElggUser($row);
-		$this->assertIdenticalEntities($user, $_SESSION['user']);
+		$this->assertIdenticalEntities($user, elgg_get_logged_in_user_entity());
 	}
 
 	public function testElggUserSave() {