diff --git a/apps/admin_audit/lib/Actions/UserManagement.php b/apps/admin_audit/lib/Actions/UserManagement.php index 5cf1494df6e94..1ad6ba4e20f83 100644 --- a/apps/admin_audit/lib/Actions/UserManagement.php +++ b/apps/admin_audit/lib/Actions/UserManagement.php @@ -97,14 +97,25 @@ public function unassign(string $uid) { * @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 ca7e0b304eae6..11ed02f47ab04 100644 --- a/apps/user_ldap/lib/User_LDAP.php +++ b/apps/user_ldap/lib/User_LDAP.php @@ -318,11 +318,6 @@ public function userExistsOnLDAP($user) { $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) { @@ -330,7 +325,7 @@ public function userExistsOnLDAP($user) { } $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); @@ -376,9 +371,6 @@ public function userExists($uid) { $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 447b91decffa6..693159dc72b15 100644 --- a/apps/user_ldap/tests/User_LDAPTest.php +++ b/apps/user_ldap/tests/User_LDAPTest.php @@ -515,13 +515,16 @@ public function testUserExists() { $this->assertTrue($result); } - /** - * @expectedException \Exception - */ public function testUserExistsForDeleted() { $backend = new UserLDAP($this->access, $this->config, $this->notificationManager, $this->session, $this->pluginManager); $this->prepareMockForUserExists(); + $mapper = $this->createMock(UserMapping::class); + $mapper->expects($this->any()) + ->method('getUUIDByDN') + ->with('dnOfFormerUser,dc=test') + ->willReturn('45673458748'); + $this->access->expects($this->any()) ->method('readAttribute') ->will($this->returnCallback(function($dn) { @@ -530,13 +533,24 @@ public function testUserExistsForDeleted() { } return false; })); + $this->access->expects($this->any()) + ->method('getUserMapper') + ->willReturn($mapper); + $this->access->expects($this->once()) + ->method('getUserDnByUuid') + ->willThrowException(new \Exception()); + + $user = $this->createMock(User::class); + $user->expects($this->any()) + ->method('getDN') + ->willReturn('dnOfFormerUser,dc=test'); $this->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() { @@ -588,14 +602,17 @@ public function testUserExistsPublicAPI() { $this->assertTrue($result); } - /** - * @expectedException \Exception - */ public function testUserExistsPublicAPIForDeleted() { $backend = new UserLDAP($this->access, $this->config, $this->notificationManager, $this->session, $this->pluginManager); $this->prepareMockForUserExists(); \OC_User::useBackend($backend); + $mapper = $this->createMock(UserMapping::class); + $mapper->expects($this->any()) + ->method('getUUIDByDN') + ->with('dnOfFormerUser,dc=test') + ->willReturn('45673458748'); + $this->access->expects($this->any()) ->method('readAttribute') ->will($this->returnCallback(function($dn) { @@ -604,12 +621,24 @@ public function testUserExistsPublicAPIForDeleted() { } return false; })); + $this->access->expects($this->any()) + ->method('getUserMapper') + ->willReturn($mapper); + $this->access->expects($this->once()) + ->method('getUserDnByUuid') + ->willThrowException(new \Exception()); + + $user = $this->createMock(User::class); + $user->expects($this->any()) + ->method('getDN') + ->willReturn('dnOfFormerUser,dc=test'); + $this->userManager->expects($this->atLeastOnce()) ->method('get') - ->willReturn($this->createMock(User::class)); + ->willReturn($user); //test for deleted user - \OC::$server->getUserManager()->userExists('formerUser'); + $this->assertFalse(\OC::$server->getUserManager()->userExists('formerUser')); } public function testUserExistsPublicAPIForNeverExisting() { @@ -762,6 +791,14 @@ public function testGetHomeNoPath() { return false; } })); + $this->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 097d708a36abc..f2b9e4fd086b7 100644 --- a/settings/Hooks.php +++ b/settings/Hooks.php @@ -165,6 +165,7 @@ public function onChangeEmail(IUser $user, $oldMailAddress) { $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 @@ public function onChangeEmail(IUser $user, $oldMailAddress) { $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);