[stable29] Avoid immediate re-expiration of accounts

AUTHORS.md

@@ -15,5 +15,6 @@
 - Jonas <jonas@freesources.org>
 - Morris Jobke <hey@morrisjobke.de>
 - rakekniven <2069590+rakekniven@users.noreply.github.com>
+- Salvatore Martire <salvatore.martire@nextcloud.com>
 - Sascha Wiswedel <sascha.wiswedel@nextcloud.com>
 - Vincent Petry <vincent@nextcloud.com>

CHANGELOG.md

@@ -5,6 +5,10 @@
 # Changelog
 All notable changes to this project will be documented in this file.
 
+## 1.13.1 – 2025-06-06
+### Fixed
+- Fixed a bug that would disable accounts again during the next cron job run after an admin re-enabled them
+
 ## 1.13.0 – 2024-07-30
 ### Changed
 - ✨ Support Nextcloud 30 and require Nextcloud 28.

appinfo/info.xml

@@ -14,7 +14,7 @@
 * ⛔ Exclude accounts based on group memberships (default: admin group)
 * 🔑 Exclude accounts that never logged in (default: enabled)
 ]]></description>
-	<version>1.14.0-dev.0</version>
+	<version>1.13.1</version>
 	<licence>agpl</licence>
 	<author>Joas Schilling</author>
 	<namespace>UserRetention</namespace>
@@ -31,7 +31,7 @@
 	<screenshot>https://raw.githubusercontent.com/nextcloud/user_retention/main/docs/screenshot.png</screenshot>
 
 	<dependencies>
-		<nextcloud min-version="28" max-version="31" />
+		<nextcloud min-version="28" max-version="29" />
 	</dependencies>
 
 	<background-jobs>

lib/AppInfo/Application.php

@@ -7,10 +7,12 @@
  */
 namespace OCA\UserRetention\AppInfo;
 
+use OCA\UserRetention\Listeners\UserChangedListener;
 use OCP\AppFramework\App;
 use OCP\AppFramework\Bootstrap\IBootContext;
 use OCP\AppFramework\Bootstrap\IBootstrap;
 use OCP\AppFramework\Bootstrap\IRegistrationContext;
+use OCP\User\Events\UserChangedEvent;
 use OCP\Util;
 
 class Application extends App implements IBootstrap {
@@ -20,6 +22,7 @@ public function __construct(array $urlParams = []) {
 	}
 
 	public function register(IRegistrationContext $context): void {
+		$context->registerEventListener(UserChangedEvent::class, UserChangedListener::class);
 	}
 
 	public function boot(IBootContext $context): void {

lib/Listeners/UserChangedListener.php

@@ -0,0 +1,52 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+namespace OCA\UserRetention\Listeners;
+
+use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\EventDispatcher\Event;
+use OCP\EventDispatcher\IEventListener;
+use OCP\IConfig;
+use OCP\User\Events\UserChangedEvent;
+use Psr\Log\LoggerInterface;
+
+/**
+ * @template-implements IEventListener<Event&UserChangedEvent>
+ */
+class UserChangedListener implements IEventListener {
+
+	public function __construct(
+		protected LoggerInterface $logger,
+		protected IConfig $config,
+		protected ITimeFactory $timeFactory,
+	) {
+	}
+
+	public function handle(Event $event): void {
+		if (!($event instanceof UserChangedEvent)) {
+			return;
+		}
+
+		if ($event->getFeature() === 'enabled') {
+			$this->handleEnabledChange($event);
+		}
+	}
+
+	private function handleEnabledChange(UserChangedEvent $event): void {
+		$oldValue = $event->getOldValue();
+		$newValue = $event->getValue();
+		if ($oldValue === false && $newValue === true) {
+			$this->config->setUserValue(
+				$event->getUser()->getUID(),
+				'user_retention',
+				'user_reenabled_at',
+				(string)$this->timeFactory->getTime()
+			);
+		}
+	}
+}

lib/Service/RetentionService.php

@@ -157,6 +157,7 @@ public function executeRetentionPolicy(IUser $user): ?bool {
 				$this->logger->debug('Account already disabled, continuing with potential deletion: ' . $user->getUID());
 			} catch (SkipUserException $e) {
 				// Not disabling yet, continue with checking deletion
+				$this->logger->debug('Disable: ' . $e->getMessage(), $e->getLogParameters());
 			}
 		} else {
 			$this->logger->debug('No account disabling policy enabled for account: ' . $user->getUID());
@@ -183,6 +184,7 @@ public function executeRetentionPolicy(IUser $user): ?bool {
 				return true;
 			} catch (SkipUserException $e) {
 				// Not deleting yet, continue with checking reminders
+				$this->logger->debug('Delete: ' . $e->getMessage(), $e->getLogParameters());
 			}
 		} else {
 			$this->logger->debug('No account retention policy enabled for account: ' . $user->getUID());
@@ -202,7 +204,7 @@ public function executeRetentionPolicy(IUser $user): ?bool {
 
 				$this->sendReminder($user, $lastActivity, $policyDays, $policyDaysDisable);
 			} catch (SkipUserException $e) {
-				$this->logger->debug($e->getMessage(), $e->getLogParameters());
+				$this->logger->debug('Reminder: ' . $e->getMessage(), $e->getLogParameters());
 				continue;
 			}
 		}
@@ -221,11 +223,12 @@ protected function shouldPerformActionOnUser(IUser $user, int $skipIfNewerThan,
 		$discoveryTimestamp = $this->skipUserBasedOnDiscovery($user);
 		$lastWebLogin = $user->getLastLogin();
 		$authTokensLastActivity = $this->getAuthTokensLastActivity($user);
+		$userReenabledTimestamp = (int)$this->config->getUserValue($user->getUID(), 'user_retention', 'user_reenabled_at', 0);
 
 		if ($authTokensLastActivity === null) {
-			$lastAction = max($discoveryTimestamp, $lastWebLogin);
+			$lastAction = max($discoveryTimestamp, $lastWebLogin, $userReenabledTimestamp);
 		} else {
-			$lastAction = max($discoveryTimestamp, $lastWebLogin, $authTokensLastActivity);
+			$lastAction = max($discoveryTimestamp, $lastWebLogin, $authTokensLastActivity, $userReenabledTimestamp);
 		}
 
 		if ($this->keepUsersWithoutLogin && $lastAction === 0) {

package.json

@@ -1,7 +1,7 @@
 {
 	"name": "user_retention",
 	"description": "Deletes users that did not log in in the last days.",
-	"version": "1.14.0-dev.0",
+	"version": "1.13.1",
 	"author": "Joas Schilling <coding@schilljs.com>",
 	"license": "AGPL-3.0-or-later",
 	"private": true,

tests/Listeners/UserChangedListenerTest.php

@@ -0,0 +1,80 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+namespace OCA\UserRetention\Tests;
+
+use OCA\UserRetention\Listeners\UserChangedListener;
+use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\IConfig;
+use OCP\IUser;
+use OCP\User\Events\UserChangedEvent;
+use OCP\User\Events\UserCreatedEvent;
+use PHPUnit\Framework\MockObject\MockObject;
+use Psr\Log\LoggerInterface;
+use Test\TestCase;
+
+class UserChangedListenerTest extends TestCase {
+	private LoggerInterface|MockObject $logger;
+	private IConfig|MockObject $config;
+	private ITimeFactory|MockObject $timeFactory;
+
+	protected function setUp(): void {
+		parent::setUp();
+
+		$this->logger = $this->createMock(LoggerInterface::class);
+		$this->config = $this->createMock(IConfig::class);
+		$this->timeFactory = $this->createMock(ITimeFactory::class);
+	}
+
+	public function testUserEnabledShouldTriggerUserReenabledAtUpdate(): void {
+		$time = time();
+		$uid = 'user';
+		$user = $this->createMock(IUser::class);
+		$user->expects($this->once())->method('getUID')->willReturn($uid);
+		$this->timeFactory->expects($this->once())->method('getTime')->willReturn($time);
+		$this->config->expects($this->once())->method('setUserValue')->with($uid, 'user_retention', 'user_reenabled_at', $time);
+
+		$event = $this->createMock(UserChangedEvent::class);
+		$event->expects($this->once())->method('getUser')->willReturn($user);
+		$event->expects($this->once())->method('getFeature')->willReturn('enabled');
+		$event->expects($this->once())->method('getOldValue')->willReturn(false);
+		$event->expects($this->once())->method('getValue')->willReturn(true);
+
+		$listener = new UserChangedListener($this->logger, $this->config, $this->timeFactory);
+		$listener->handle($event);
+	}
+
+	public function testHandleShouldNotHandleOtherEvents(): void {
+		$event = $this->createMock(UserCreatedEvent::class);
+		$this->config->expects($this->never())->method('setUserValue');
+
+		$listener = new UserChangedListener($this->logger, $this->config, $this->timeFactory);
+		$listener->handle($event);
+	}
+
+	public function testHandleShouldOnlyHandleEnabledFeature(): void {
+		$event = $this->createMock(UserChangedEvent::class);
+		$event->expects($this->once())->method('getFeature')->willReturn('otherFeature');
+		$this->config->expects($this->never())->method('setUserValue');
+
+		$listener = new UserChangedListener($this->logger, $this->config, $this->timeFactory);
+		$listener->handle($event);
+	}
+
+	public function testDisabledUserShouldNotTriggerUserReenabledAtUpdate(): void {
+		$this->config->expects($this->never())->method('setUserValue');
+
+		$event = $this->createMock(UserChangedEvent::class);
+		$event->expects($this->once())->method('getFeature')->willReturn('enabled');
+		$event->expects($this->once())->method('getOldValue')->willReturn(true);
+		$event->expects($this->once())->method('getValue')->willReturn(false);
+
+		$listener = new UserChangedListener($this->logger, $this->config, $this->timeFactory);
+		$listener->handle($event);
+	}
+
+}