diff --git a/lib/AutoGroupsManager.php b/lib/AutoGroupsManager.php index cb0fabc..2db5020 100644 --- a/lib/AutoGroupsManager.php +++ b/lib/AutoGroupsManager.php @@ -25,31 +25,29 @@ namespace OCA\AutoGroups; +use OCP\AppFramework\OCS\OCSBadRequestException; +use OCP\EventDispatcher\Event; use OCP\IGroupManager; use OCP\IConfig; use OCP\IL10N; - -use OCP\AppFramework\OCS\OCSBadRequestException; +use OCP\IUserManager; use Psr\Log\LoggerInterface; class AutoGroupsManager { - private $groupManager; - private $logger; - private $config; - private $l; /** * AutoGroupsManager constructor. */ - public function __construct(IGroupManager $groupManager, IConfig $config, LoggerInterface $logger, IL10N $l) + public function __construct( + private readonly IGroupManager $groupManager, + private readonly IUserManager $userManager, + private readonly IConfig $config, + private readonly LoggerInterface $logger, + private readonly IL10N $l, + ) { - $this->groupManager = $groupManager; - $this->logger = $logger; - $this->config = $config; - $this->l = $l; - // Migrate old config if necessary $creationOnly = $this->config->getAppValue("AutoGroups", "creation_only"); if ($creationOnly !== '') { @@ -92,7 +90,7 @@ public function __construct(IGroupManager $groupManager, IConfig $config, Logger /** * The event handler to check group assignment for a user */ - public function addAndRemoveAutoGroups($event) + public function addAndRemoveAutoGroups(Event $event): void { // Get configuration $groupNames = json_decode($this->config->getAppValue("auto_groups", "auto_groups", '[]')); @@ -100,6 +98,13 @@ public function addAndRemoveAutoGroups($event) // Get user information $user = $event->getUser(); + + if (!$this->userManager->userExists($user->getUID())) { + // Avoid doing any group manipulation when running inside + // OC\User\BackgroundJobs\CleanupDeletedUsers + return; + } + $userGroupNames = $this->groupManager->getUserGroupIds($user); // Notice message for Auto Group Hook Execution diff --git a/tests/Unit/AutoGroupsManagerTest.php b/tests/Unit/AutoGroupsManagerTest.php index 0ee1378..6d5b29b 100644 --- a/tests/Unit/AutoGroupsManagerTest.php +++ b/tests/Unit/AutoGroupsManagerTest.php @@ -25,6 +25,7 @@ namespace OCA\AutoGroups\Tests\Unit; use OCP\Group\Events\BeforeGroupDeletedEvent; +use OCP\IUserManager; use OCP\User\Events\UserCreatedEvent; use OCP\IGroupManager; @@ -38,6 +39,7 @@ use OCA\AutoGroups\AutoGroupsManager; +use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; use Test\TestCase; @@ -45,27 +47,31 @@ class AutoGroupsManagerTest extends TestCase { - private $groupManager; - private $config; - private $logger; - private $il10n; + private IGroupManager&MockObject $groupManager; + private IUserManager&MockObject $userManager; + private IConfig&MockObject $config; + private LoggerInterface&MockObject $logger; + private IL10N&MockObject $il10n; protected function setUp(): void { parent::setUp(); $this->groupManager = $this->createMock(IGroupManager::class); + $this->userManager = $this->createMock(IUserManager::class); $this->config = $this->createMock(IConfig::class); $this->logger = $this->createMock(LoggerInterface::class); $this->il10n = $this->createMock(IL10N::class); + $this->userManager->method('userExists')->willReturn(true); + $this->testUser = $this->createMock(IUser::class); $this->testUser->expects($this->any()) ->method('getDisplayName') ->willReturn('Test User'); } - private function createAutoGroupsManager($auto_groups = [], $override_groups = []) + private function createAutoGroupsManager($auto_groups = [], $override_groups = []): AutoGroupsManager { $this->config->method('getAppValue') ->willReturnCallback(function ($app, $key, $default = '') use ($auto_groups, $override_groups) { @@ -81,10 +87,10 @@ private function createAutoGroupsManager($auto_groups = [], $override_groups = [ return $default; }); - return new AutoGroupsManager($this->groupManager, $this->config, $this->logger, $this->il10n); + return new AutoGroupsManager($this->groupManager, $this->userManager, $this->config, $this->logger, $this->il10n); } - private function configMigrationTestImpl($creationOnly, $expectedModification) + private function configMigrationTestImpl($creationOnly, $expectedModification): AutoGroupsManager { $this->config->expects($this->exactly(2)) ->method('getAppValue') @@ -102,7 +108,7 @@ private function configMigrationTestImpl($creationOnly, $expectedModification) ->method('deleteAppValue') ->with('AutoGroups', 'creation_only'); - return new AutoGroupsManager($this->groupManager, $this->config, $this->logger, $this->il10n); + return new AutoGroupsManager($this->groupManager, $this->userManager, $this->config, $this->logger, $this->il10n); } public function testAddingToAutoGroups()