From 6f24865bf8a55975778acca5e2610fda51deca35 Mon Sep 17 00:00:00 2001
From: Arthur Schiwon <blizzz@arthur-schiwon.de>
Date: Tue, 14 Aug 2018 17:06:57 +0200
Subject: [PATCH 1/3] don't blame email address changers
the information is being collected with admin_audit
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
---
.../lib/Actions/UserManagement.php | 27 +++++++++++++------
settings/Hooks.php | 13 ++++-----
2 files changed, 24 insertions(+), 16 deletions(-)
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/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);
From b497b0686790c250825b8d631b403bb27d2ba2c8 Mon Sep 17 00:00:00 2001
From: Arthur Schiwon <blizzz@arthur-schiwon.de>
Date: Tue, 14 Aug 2018 17:57:24 +0200
Subject: [PATCH 2/3] don't force LDAP updates on userExists anymore
and remove some deprecated code
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
---
apps/user_ldap/lib/User_LDAP.php | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
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;
}
From 2b903aa267a1969edea945538ac778a008ea7955 Mon Sep 17 00:00:00 2001
From: Arthur Schiwon <blizzz@arthur-schiwon.de>
Date: Wed, 15 Aug 2018 16:11:07 +0200
Subject: [PATCH 3/3] fix unit tests
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
---
apps/user_ldap/tests/User_LDAPTest.php | 57 +++++++++++++++++++++-----
1 file changed, 47 insertions(+), 10 deletions(-)
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())