Avoid immediate re-expiration of accounts

AUTHORS.md

@@ -15,5 +15,6 @@
 - Jonas <[email protected]>
 - Morris Jobke <[email protected]>
 - rakekniven <[email protected]>
+- Salvatore Martire <[email protected]>
 - Sascha Wiswedel <[email protected]>
 - Vincent Petry <[email protected]>

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) {

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);
+	}
+
+}