diff --git a/AUTHORS.md b/AUTHORS.md index e2f77322..e9fd6cd0 100644 --- a/AUTHORS.md +++ b/AUTHORS.md @@ -15,5 +15,6 @@ - Jonas - Morris Jobke - rakekniven <2069590+rakekniven@users.noreply.github.com> +- Salvatore Martire - Sascha Wiswedel - Vincent Petry diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 62592cdb..8603ebb4 100644 --- a/lib/AppInfo/Application.php +++ b/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 { diff --git a/lib/Listeners/UserChangedListener.php b/lib/Listeners/UserChangedListener.php new file mode 100644 index 00000000..244f3ee3 --- /dev/null +++ b/lib/Listeners/UserChangedListener.php @@ -0,0 +1,52 @@ + + */ +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() + ); + } + } +} diff --git a/lib/Service/RetentionService.php b/lib/Service/RetentionService.php index 674eed3f..57935b00 100644 --- a/lib/Service/RetentionService.php +++ b/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) { diff --git a/tests/Listeners/UserChangedListenerTest.php b/tests/Listeners/UserChangedListenerTest.php new file mode 100644 index 00000000..966863ec --- /dev/null +++ b/tests/Listeners/UserChangedListenerTest.php @@ -0,0 +1,80 @@ +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); + } + +}