Merge pull request #10763 from nextcloud/backport/10687/stable13

[stable13] don't blame randome people for background email updates

4 files changed, 72 insertions, 36 deletions

diff --git a/apps/admin_audit/lib/Actions/UserManagement.php b/apps/admin_audit/lib/Actions/UserManagement.php
index 9d0b237e303..d45863a1218 100644
--- a/apps/admin_audit/lib/Actions/UserManagement.php
+++ b/apps/admin_audit/lib/Actions/UserManagement.php
@@ -70,14 +70,25 @@ class UserManagement extends Action {
 	 * @param array $params
 	 */
 	public function change(array $params) {
-		if ($params['feature'] === 'enabled') {
-			$this->log(
-				$params['value'] === 'true' ? 'User enabled: "%s"' : 'User disabled: "%s"',
-				['user' => $params['user']->getUID()],
-				[
-					'user',
-				]
-			);
+		switch($params['feature']) {
+			case 'enabled':
+				$this->log(
+					$params['value'] === 'true' ? 'User enabled: "%s"' : 'User disabled: "%s"',
+					['user' => $params['user']->getUID()],
+					[
+						'user',
+					]
+				);
+				break;
+			case 'eMailAddress':
+				$this->log(
+					'Email address changed for user %s',
+					['user' => $params['user']->getUID()],
+					[
+						'user',
+					]
+				);
+				break;
 		}
 	}
 
diff --git a/apps/user_ldap/lib/User_LDAP.php b/apps/user_ldap/lib/User_LDAP.php
index 4eb72592789..41e58f0a07d 100644
--- a/apps/user_ldap/lib/User_LDAP.php
+++ b/apps/user_ldap/lib/User_LDAP.php
@@ -317,11 +317,6 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn
 		$dn = $user->getDN();
 		//check if user really still exists by reading its entry
 		if(!is_array($this->access->readAttribute($dn, '', $this->access->connection->ldapUserFilter))) {
-			$lcr = $this->access->connection->getConnectionResource();
-			if(is_null($lcr)) {
-				throw new \Exception('No LDAP Connection to server ' . $this->access->connection->ldapHost);
-			}
-
 			try {
 				$uuid = $this->access->getUserMapper()->getUUIDByDN($dn);
 				if (!$uuid) {
@@ -329,7 +324,7 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn
 				}
 				$newDn = $this->access->getUserDnByUuid($uuid);
 				//check if renamed user is still valid by reapplying the ldap filter
-				if (!is_array($this->access->readAttribute($newDn, '', $this->access->connection->ldapUserFilter))) {
+				if ($newDn === $dn || !is_array($this->access->readAttribute($newDn, '', $this->access->connection->ldapUserFilter))) {
 					return false;
 				}
 				$this->access->getUserMapper()->setDNbyUUID($newDn, $uuid);
@@ -375,9 +370,6 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn
 
 		$result = $this->userExistsOnLDAP($user);
 		$this->access->connection->writeToCache('userExists'.$uid, $result);
-		if($result === true) {
-			$user->update();
-		}
 		return $result;
 	}
 
diff --git a/apps/user_ldap/tests/User_LDAPTest.php b/apps/user_ldap/tests/User_LDAPTest.php
index 5136e62e7d1..e7d6669a997 100644
--- a/apps/user_ldap/tests/User_LDAPTest.php
+++ b/apps/user_ldap/tests/User_LDAPTest.php
@@ -558,14 +558,17 @@ class User_LDAPTest extends TestCase {
 		$this->assertTrue($result);
 	}
 
-	/**
-	 * @expectedException \Exception
-	 */
 	public function testUserExistsForDeleted() {
 		$access = $this->getAccessMock();
 		$backend = new UserLDAP($access, $this->createMock(IConfig::class), $this->createMock(INotificationManager::class), $this->createMock(Session::class), $this->getDefaultPluginManagerMock());
 		$this->prepareMockForUserExists($access);
 
+		$mapper = $this->createMock(UserMapping::class);
+		$mapper->expects($this->any())
+			->method('getUUIDByDN')
+			->with('dnOfFormerUser,dc=test')
+			->willReturn('45673458748');
+
 		$access->expects($this->any())
 			->method('readAttribute')
 			->will($this->returnCallback(function($dn) {
@@ -574,14 +577,25 @@ class User_LDAPTest extends TestCase {
 				}
 				return false;
 			}));
+		$access->expects($this->any())
+			->method('getUserMapper')
+			->willReturn($mapper);
+		$access->expects($this->once())
+			->method('getUserDnByUuid')
+			->willThrowException(new \Exception());
+
+		$user = $this->createMock(User::class);
+		$user->expects($this->any())
+			->method('getDN')
+			->willReturn('dnOfFormerUser,dc=test');
 
 		$access->userManager = $this->createMock(Manager::class);
 		$access->userManager->expects($this->atLeastOnce())
 			->method('get')
-			->willReturn($this->createMock(User::class));
+			->willReturn($user);
 
 		//test for deleted user
-		$backend->userExists('formerUser');
+		$this->assertFalse($backend->userExists('formerUser'));
 	}
 
 	public function testUserExistsForNeverExisting() {
@@ -634,15 +648,18 @@ class User_LDAPTest extends TestCase {
 		$this->assertTrue($result);
 	}
 
-	/**
-	 * @expectedException \Exception
-	 */
 	public function testUserExistsPublicAPIForDeleted() {
 		$access = $this->getAccessMock();
 		$backend = new UserLDAP($access, $this->createMock(IConfig::class), $this->createMock(INotificationManager::class), $this->createMock(Session::class), $this->getDefaultPluginManagerMock());
 		$this->prepareMockForUserExists($access);
 		\OC_User::useBackend($backend);
 
+		$mapper = $this->createMock(UserMapping::class);
+		$mapper->expects($this->any())
+			->method('getUUIDByDN')
+			->with('dnOfFormerUser,dc=test')
+			->willReturn('45673458748');
+
 		$access->expects($this->any())
 			->method('readAttribute')
 			->will($this->returnCallback(function($dn) {
@@ -651,13 +668,24 @@ class User_LDAPTest extends TestCase {
 				}
 				return false;
 			}));
-		$access->userManager = $this->createMock(Manager::class);
+		$access->expects($this->any())
+			->method('getUserMapper')
+			->willReturn($mapper);
+		$access->expects($this->once())
+			->method('getUserDnByUuid')
+			->willThrowException(new \Exception());
+
+		$user = $this->createMock(User::class);
+		$user->expects($this->any())
+			->method('getDN')
+			->willReturn('dnOfFormerUser,dc=test');
+
 		$access->userManager->expects($this->atLeastOnce())
 			->method('get')
-			->willReturn($this->createMock(User::class));
+			->willReturn($user);
 
 		//test for deleted user
-		\OCP\User::userExists('formerUser');
+		$this->assertFalse(\OC::$server->getUserManager()->userExists('formerUser'));
 	}
 
 	public function testUserExistsPublicAPIForNeverExisting() {
@@ -818,6 +846,14 @@ class User_LDAPTest extends TestCase {
 						return false;
 				}
 			}));
+		$access->connection->expects($this->any())
+			->method('getFromCache')
+			->willReturnCallback(function($key) {
+				if($key === 'userExistsnewyorker') {
+					return true;
+				}
+				return null;
+			});
 
 		$user = $this->createMock(User::class);
 		$user->expects($this->any())
diff --git a/settings/Hooks.php b/settings/Hooks.php
index 097d708a36a..f2b9e4fd086 100644
--- a/settings/Hooks.php
+++ b/settings/Hooks.php
@@ -165,6 +165,7 @@ class Hooks {
 
 		$actor = $this->userSession->getUser();
 		if ($actor instanceof IUser) {
+			$subject = Provider::EMAIL_CHANGED_SELF;
 			if ($actor->getUID() !== $user->getUID()) {
 				$this->l = $this->languageFactory->get(
 					'settings',
@@ -173,15 +174,11 @@ class Hooks {
 						$this->config->getSystemValue('default_language', 'en')
 					)
 				);
-
-				$text = $this->l->t('%1$s changed your email address on %2$s.', [$actor->getDisplayName(), $instanceUrl]);
-				$event->setAuthor($actor->getUID())
-					->setSubject(Provider::EMAIL_CHANGED_BY, [$actor->getUID()]);
-			} else {
-				$text = $this->l->t('Your email address on %s was changed.', [$instanceUrl]);
-				$event->setAuthor($actor->getUID())
-					->setSubject(Provider::EMAIL_CHANGED_SELF);
+				$subject = Provider::EMAIL_CHANGED;
 			}
+			$text = $this->l->t('Your email address on %s was changed.', [$instanceUrl]);
+			$event->setAuthor($actor->getUID())
+				->setSubject($subject);
 		} else {
 			$text = $this->l->t('Your email address on %s was changed by an administrator.', [$instanceUrl]);
 			$event->setSubject(Provider::EMAIL_CHANGED);