Merge pull request #4499 from nextcloud/downstream-26984

Trigger change when a user is enabled/disabled

4 files changed, 72 insertions, 2 deletions

diff --git a/apps/admin_audit/lib/actions/usermanagement.php b/apps/admin_audit/lib/actions/usermanagement.php
index 925d8b0a715..0ee192d9a31 100644
--- a/apps/admin_audit/lib/actions/usermanagement.php
+++ b/apps/admin_audit/lib/actions/usermanagement.php
@@ -61,6 +61,23 @@ class UserManagement extends Action {
 	}
 
 	/**
+	 * Log enabling of users
+	 *
+	 * @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',
+				]
+			);
+		}
+	}
+
+	/**
 	 * Logs changing of the user scope
 	 *
 	 * @param IUser $user
diff --git a/apps/admin_audit/lib/auditlogger.php b/apps/admin_audit/lib/auditlogger.php
index a01fec63019..4e1909c6475 100644
--- a/apps/admin_audit/lib/auditlogger.php
+++ b/apps/admin_audit/lib/auditlogger.php
@@ -90,6 +90,7 @@ class AuditLogger {
 
 		Util::connectHook('OC_User', 'post_createUser',	$userActions, 'create');
 		Util::connectHook('OC_User', 'post_deleteUser',	$userActions, 'delete');
+		Util::connectHook('OC_User', 'changeUser',	$userActions, 'change');
 		$this->userSession->listen('\OC\User', 'postSetPassword', [$userActions, 'setPassword']);
 	}
 	
diff --git a/lib/private/User/User.php b/lib/private/User/User.php
index a3be0c24bb9..f55807bc769 100644
--- a/lib/private/User/User.php
+++ b/lib/private/User/User.php
@@ -342,9 +342,13 @@ class User implements IUser {
 	 * @param bool $enabled
 	 */
 	public function setEnabled($enabled) {
+		$oldStatus = $this->isEnabled();
 		$this->enabled = $enabled;
 		$enabled = ($enabled) ? 'true' : 'false';
-		$this->config->setUserValue($this->uid, 'core', 'enabled', $enabled);
+		if ($oldStatus !== $this->enabled) {
+			$this->triggerChange('enabled', $enabled);
+			$this->config->setUserValue($this->uid, 'core', 'enabled', $enabled);
+		}
 	}
 
 	/**
diff --git a/tests/lib/User/UserTest.php b/tests/lib/User/UserTest.php
index 5fc07b692f7..b53d07b0d4e 100644
--- a/tests/lib/User/UserTest.php
+++ b/tests/lib/User/UserTest.php
@@ -705,7 +705,55 @@ class UserTest extends TestCase {
 				'false'
 			);
 
-		$user = new User('foo', $backend, null, $config);
+		$user = $this->getMockBuilder(User::class)
+			->setConstructorArgs([
+				'foo',
+				$backend,
+				null,
+				$config,
+			])
+			->setMethods(['isEnabled', 'triggerChange'])
+			->getMock();
+
+		$user->expects($this->once())
+			->method('isEnabled')
+			->willReturn(true);
+		$user->expects($this->once())
+			->method('triggerChange')
+			->with(
+				'enabled',
+				'false'
+			);
+
+		$user->setEnabled(false);
+	}
+
+	public function testSetDisabledAlreadyDisabled() {
+		/**
+		 * @var Backend | \PHPUnit_Framework_MockObject_MockObject $backend
+		 */
+		$backend = $this->createMock(\Test\Util\User\Dummy::class);
+
+		$config = $this->createMock(IConfig::class);
+		$config->expects($this->never())
+			->method('setUserValue');
+
+		$user = $this->getMockBuilder(User::class)
+			->setConstructorArgs([
+				'foo',
+				$backend,
+				null,
+				$config,
+			])
+			->setMethods(['isEnabled', 'triggerChange'])
+			->getMock();
+
+		$user->expects($this->once())
+			->method('isEnabled')
+			->willReturn(false);
+		$user->expects($this->never())
+			->method('triggerChange');
+
 		$user->setEnabled(false);
 	}