diff --git a/lib/Event/UserAccountChangeListener.php b/lib/Event/UserAccountChangeListener.php index 9bc2c1f..d62f36c 100644 --- a/lib/Event/UserAccountChangeListener.php +++ b/lib/Event/UserAccountChangeListener.php @@ -3,24 +3,21 @@ namespace OCA\NextMagentaCloudProvisioning\Event; use OCA\NextMagentaCloudProvisioning\Rules\UserAccountRules; - use OCA\UserOIDC\Event\UserAccountChangeEvent; use OCP\EventDispatcher\Event; - use OCP\EventDispatcher\IEventListener; - -use OCP\ILogger; +use Psr\Log\LoggerInterface; class UserAccountChangeListener implements IEventListener { - /** @var ILogger */ + /** @var LoggerInterface */ private $logger; /** @var UserAccountRules */ private $accountRules; - public function __construct(ILogger $logger, + public function __construct(LoggerInterface $logger, UserAccountRules $accountRules) { $this->logger = $logger; $this->accountRules = $accountRules; diff --git a/lib/Event/UserAttributeListener.php b/lib/Event/UserAttributeListener.php index 6b3e2d0..f72c890 100644 --- a/lib/Event/UserAttributeListener.php +++ b/lib/Event/UserAttributeListener.php @@ -4,18 +4,15 @@ use OCA\NextMagentaCloudProvisioning\Rules\DisplaynameRules; use OCA\NextMagentaCloudProvisioning\Rules\TariffRules; - use OCA\UserOIDC\Event\AttributeMappedEvent; use OCA\UserOIDC\Service\ProviderService; - use OCP\EventDispatcher\Event; - use OCP\EventDispatcher\IEventListener; -use OCP\ILogger; +use Psr\Log\LoggerInterface; class UserAttributeListener implements IEventListener { - /** @var ILogger */ + /** @var LoggerInterface */ private $logger; /** @var TariffRules */ @@ -25,7 +22,7 @@ class UserAttributeListener implements IEventListener { private $displaynameRules; - public function __construct(ILogger $logger, + public function __construct(LoggerInterface $logger, TariffRules $tariffRules, DisplaynameRules $displaynameRules) { $this->logger = $logger; diff --git a/lib/Rules/UserAccountRules.php b/lib/Rules/UserAccountRules.php index f1e9d18..98c6506 100644 --- a/lib/Rules/UserAccountRules.php +++ b/lib/Rules/UserAccountRules.php @@ -6,13 +6,13 @@ use OCA\UserOIDC\Db\ProviderMapper; use OCA\UserOIDC\Db\User; use OCP\IConfig; -use OCP\ILogger; use OCP\IUser; use OCP\IUserManager; +use Psr\Log\LoggerInterface; class UserAccountRules { - /** @var ILogger */ + /** @var LoggerInterface */ private $logger; /** @var IConfig */ @@ -28,7 +28,7 @@ class UserAccountRules { private $oidcProviderMapper; public function __construct(IConfig $config, - ILogger $logger, + LoggerInterface $logger, NmcUserService $nmcUserService, IUserManager $userManager, ProviderMapper $oidcProviderMapper) { @@ -139,7 +139,8 @@ public function deriveAccountState(string $uid, ?string $displayname, ?string $m 'slup_test_account_name' => '-test', 'slup_test_account_explode' => '@' ]); - if ($user = $this->nmcUserService->userExists($providerName, $uid, true)) { + $user = $this->nmcUserService->userExists($providerName, $uid, true); + if ($user instanceof OCA\UserOIDC\Db\User || $user instanceof OCP\IUser) { $this->logger->info("PROV {$uid}: Modify existing"); return $this->deriveExistingAccountState($user, $displayname, $mainEmail, $quota, $claims, $providerName); } elseif ($create || $config['slup_test_account_check'] && diff --git a/lib/User/NmcUserService.php b/lib/User/NmcUserService.php index b0334d3..52dff31 100644 --- a/lib/User/NmcUserService.php +++ b/lib/User/NmcUserService.php @@ -13,11 +13,10 @@ use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\IConfig; use OCP\IGroupManager; -use OCP\ILogger; -use OCP\IServerContainer; use OCP\IUser; use OCP\IUserManager; use OCP\Security\ISecureRandom; +use Psr\Log\LoggerInterface; // classes from user_oidc app @@ -29,10 +28,7 @@ class NmcUserService { /** @var IAccountManager */ private $accountManager; - /** @var IServerContainer */ - private $serverc; - - /** @var ILogger */ + /** @var LoggerInterface */ private $logger; /** @var IConfig */ @@ -56,8 +52,7 @@ class NmcUserService { public function __construct(IUserManager $userManager, IAccountManager $accountManager, - IServerContainer $serverContainer, - ILogger $logger, + LoggerInterface $logger, IConfig $config, UserMapper $oidcUserMapper, ProviderMapper $oidcProviderMapper, @@ -65,7 +60,6 @@ public function __construct(IUserManager $userManager, $this->groupTariffMapping = new GroupTariffMapping($config); $this->userManager = $userManager; $this->accountManager = $accountManager; - $this->serverc = $serverContainer; $this->logger = $logger; $this->config = $config; $this->oidcUserMapper = $oidcUserMapper; @@ -139,7 +133,7 @@ public function userExists(string $provider, string $username, bool $returnUser return true; } catch (NotFoundException $eNotFound) { - return false; + return $returnUser ? null : false; } } @@ -298,24 +292,6 @@ public function create(string $provider, $this->logger->info("UserID: ".$oidcUser->getUserId()); $this->createAccountUser($user, $email, $quota, $enabled); - /* try { - $this->logger->debug("PROV folder"); - $this->serverc->get('UserFolder')->create($user->getUID()); - $userFolder = $this->serverc->getUserFolder($user->getUID()); - \OC::$server->getLogger()->debug('nmcuser_oidc: User folder created "' . $user->getUID() . '", exists=' . ($this->serverc->getRootFolder()->nodeExists('/' . $user->getUID() . '/files') ? 'true' : 'false'), ['app' => 'debug_create']); - - // Write a temporary file to the user home to make sure it is properly setup - // FIXME: Remove once the issue with the missing user directory on concurrent webdav requests are sorted out - $file = $userFolder->newFile('.userCreateTemp'); - $file->delete(); - - // copy skeleton - \OC_Util::copySkeleton($user->getUID(), $userFolder); - } catch (NotPermittedException $ex) { - \OC::$server->getLogger()->logException($ex, ['app' => 'nmcuser_oidc']); - throw new ForbiddenException("Newly created user cannot init home folder. Reason:\n" . $ex->getMessage()); - }*/ - return [ 'id' => $oidcUser->getUserId() ]; diff --git a/tests/integration/UserDeletionDbTest.php b/tests/integration/UserDeletionDbTest.php index 2cf8083..e456829 100644 --- a/tests/integration/UserDeletionDbTest.php +++ b/tests/integration/UserDeletionDbTest.php @@ -11,14 +11,9 @@ use OCA\NextMagentaCloudProvisioning\User\NmcUserService; use OCA\UserOIDC\Db\ProviderMapper; use OCA\UserOIDC\Db\UserMapper; - - use OCP\Accounts\IAccountManager; - use OCP\IConfig; use OCP\ILogger; - -use OCP\IServerContainer; use OCP\IUserManager; class UserDeletionDbTest extends StackedCleanupTestCase { @@ -40,7 +35,6 @@ public function setUp(): void { $this->userServiceMock = $this->getMockBuilder(NmcUserService::class) ->setConstructorArgs([ $this->app->getContainer()->get(IUserManager::class), $this->app->getContainer()->get(IAccountManager::class), - $this->app->getContainer()->get(IServerContainer::class), $this->config, $this->app->getContainer()->get(UserMapper::class), $this->app->getContainer()->get(ProviderMapper::class)]) diff --git a/tests/unit/AttributeMappedDisplaynameTest.php b/tests/unit/AttributeMappedDisplaynameTest.php index a3427b8..5bfc937 100644 --- a/tests/unit/AttributeMappedDisplaynameTest.php +++ b/tests/unit/AttributeMappedDisplaynameTest.php @@ -5,18 +5,14 @@ namespace OCA\NextMagentaCloudProvisioning\UnitTest; use OCA\NextMagentaCloudProvisioning\AppInfo\Application; - use OCA\NextMagentaCloudProvisioning\Event\UserAttributeListener; use OCA\NextMagentaCloudProvisioning\Rules\DisplaynameRules; - use OCA\NextMagentaCloudProvisioning\Rules\TariffRules; use OCA\UserOIDC\Event\AttributeMappedEvent; use OCA\UserOIDC\Service\ProviderService; - use OCP\AppFramework\App; -use OCP\ILogger; - use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; class AttributeMappedDisplaynameTest extends TestCase { public const DEMO_CLAIM = <<listener = new UserAttributeListener($app->getContainer()->get(ILogger::class), + $this->listener = new UserAttributeListener($app->getContainer()->get(LoggerInterface::class), $app->getContainer()->get(TariffRules::class), $app->getContainer()->get(DisplaynameRules::class)); diff --git a/tests/unit/AttributeMappedQuotaTest.php b/tests/unit/AttributeMappedQuotaTest.php index 91f0804..a4fc2f1 100644 --- a/tests/unit/AttributeMappedQuotaTest.php +++ b/tests/unit/AttributeMappedQuotaTest.php @@ -5,18 +5,14 @@ namespace OCA\NextMagentaCloudProvisioning\UnitTest; use OCA\NextMagentaCloudProvisioning\AppInfo\Application; - use OCA\NextMagentaCloudProvisioning\Event\UserAttributeListener; use OCA\NextMagentaCloudProvisioning\Rules\DisplaynameRules; - use OCA\NextMagentaCloudProvisioning\Rules\TariffRules; use OCA\UserOIDC\Event\AttributeMappedEvent; use OCA\UserOIDC\Service\ProviderService; - use OCP\AppFramework\App; -use OCP\ILogger; - use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; class AttributeMappedQuotaTest extends TestCase { public const DEMO_CLAIM = <<listener = new UserAttributeListener($app->getContainer()->get(ILogger::class), + $this->listener = new UserAttributeListener($app->getContainer()->get(LoggerInterface::class), $app->getContainer()->get(TariffRules::class), $app->getContainer()->get(DisplaynameRules::class)); } diff --git a/tests/unit/UserAccounctDeletionJobTest.php b/tests/unit/UserAccounctDeletionJobTest.php index 8f8a646..8e78cbb 100644 --- a/tests/unit/UserAccounctDeletionJobTest.php +++ b/tests/unit/UserAccounctDeletionJobTest.php @@ -8,7 +8,6 @@ use OCA\NextMagentaCloudProvisioning\Db\UserQueries; use OCA\NextMagentaCloudProvisioning\User\UserAccountDeletionJob; use OCP\AppFramework\Utility\ITimeFactory; -use OCP\IConfig; use OCP\IUser; use OCP\IUserManager; use PHPUnit\Framework\TestCase; @@ -18,113 +17,69 @@ class UserAccountDeletionJobTest extends TestCase { public function setUp(): void { parent::setUp(); $this->app = new \OCP\AppFramework\App(Application::APP_ID); - $this->config = $this->getMockForAbstractClass(IConfig::class); $this->userQueries = $this->createMock(UserQueries::class); $this->userManager = $this->getMockForAbstractClass(IUserManager::class); $this->job = new UserAccountDeletionJob($this->app->getContainer()->get(ITimeFactory::class), $this->app->getContainer()->get(LoggerInterface::class), - $this->config, $this->userQueries, $this->userManager); } - // TODO: control job execution times better (e.g.only at night) - // The uncommented tests fail as NextCLoud has no working mechanism for it - // - // public function testJobIntervalEarly() { - // $this->config->expects($this->once()) - // ->method("getAppValue") - // ->with($this->equalTo('nmcprovisioning'), $this->equalTo('deletionjobtime')) - // ->willReturn('04:00:00'); - - // $refTime = new \DateTime("01:23:01"); - // $interval = $this->job->setLastRun($refTime->getTimestamp()); - // $this->assertEquals(59 + 36*60 + 2*3600 , $this->job->getInterval()); - // } - - // public function testJobIntervalLate() { - // $this->config->expects($this->once()) - // ->method("getAppValue") - // ->with($this->equalTo('nmcprovisioning'), $this->equalTo('deletionjobtime')) - // ->willReturn('05:00:00'); - - // $refTime = new \DateTime("11:23:33");Mura - // $interval = $this->job->computeDestinationInterval($refTime); - // $this->assertEquals(27 + 36*60 + 17*3600 , $interval); - - // } - - // public function testJobIntervalZero() { - // $this->config->expects($this->once()) - // ->method("getAppValue") - // ->with($this->equalTo('nmcprovisioning'), $this->equalTo('deletionjobtime')) - // ->willReturn('17:00:00'); - - // $refTime = new \DateTime("17:00:00"); - // $interval = $this->job->computeDestinationInterval($refTime); - // $this->assertEquals(24*3600 , $interval); - // } - - public function testJobRunNone() { - //$this->config->expects($this->once()) - // ->method("getAppValue") - // ->with($this->equalTo('nmcprovisioning'), $this->equalTo('deletionjobtime')) - // ->willReturn('04:00:00'); - $this->userQueries->expects($this->once()) ->method("findDeletions") - ->willReturn([]); - + ->willReturn([]); // Keine Benutzer zum Löschen + + // Mock für den Benutzer, der nie gelöscht wird $user = $this->getMockForAbstractClass(IUser::class); - $user->expects($this->never())->method("delete"); - $this->userManager->expects($this->never())->method("get"); - + $user->expects($this->never())->method("delete"); // delete wird niemals aufgerufen + $this->userManager->expects($this->never())->method("get"); // get wird auch nie aufgerufen + $this->job->run(null); } public function testJobRunMultiple() { - //$this->config->expects($this->once()) - // ->method("getAppValue") - // ->with($this->equalTo('nmcprovisioning'), $this->equalTo('deletionjobtime')) - // ->willReturn('04:00:00'); - - $this->userQueries->expects($this->once()) + // Simuliere mehrere Aufrufe von findDeletions, um Benutzer in verschiedenen Batches zu finden + $this->userQueries->expects($this->exactly(3)) // findDeletions wird dreimal aufgerufen ->method("findDeletions") - ->willReturn(['120042010000000004200001', - '120042010000000004200002', - '120042010000000004200003']); - + ->willReturnOnConsecutiveCalls( + ['120042010000000004200001', '120042010000000004200002'], // Erstes Batch + ['120042010000000004200003'], // Zweites Batch + [] // Drittens Batch, keine Benutzer mehr zu löschen + ); + + // Mock des Benutzers, der gelöscht werden soll $user = $this->getMockForAbstractClass(IUser::class); - $user->expects($this->exactly(3))->method("delete"); - $this->userManager->expects($this->exactly(3)) - ->method("get") - ->withConsecutive([$this->equalTo('120042010000000004200001')], - [$this->equalTo('120042010000000004200002')], - [$this->equalTo('120042010000000004200003')]) - ->willReturn($user); + $user->expects($this->exactly(3))->method("delete"); // delete wird für jeden Benutzer genau einmal aufgerufen + $this->userManager->expects($this->exactly(3)) // get wird für jeden Benutzer aufgerufen + ->method("get") + ->withConsecutive( + [$this->equalTo('120042010000000004200001')], + [$this->equalTo('120042010000000004200002')], + [$this->equalTo('120042010000000004200003')] + ) + ->willReturn($user); + $this->job->run(null); } public function testJobRunOne() { - //$this->config->expects($this->once()) - // ->method("getAppValue") - // ->with($this->equalTo('nmcprovisioning'), $this->equalTo('deletionjobtime')) - // ->willReturn('04:00:00'); - - $this->userQueries->expects($this->once()) + // Simuliere findDeletions mit nur einem Benutzer + $this->userQueries->expects($this->exactly(2)) ->method("findDeletions") - ->willReturn(['120042010000000004200001']); - + ->willReturnOnConsecutiveCalls( + ['120042010000000004200001'], // Erstes Batch + [] // Zweites Batch, keine Benutzer mehr zu löschen + ); + + // Mock des Benutzers, der gelöscht werden soll $user = $this->getMockForAbstractClass(IUser::class); - $user->expects($this->once())->method("delete"); - $this->userManager->expects($this->once(0)) - ->method("get") - ->with($this->equalTo('120042010000000004200001')) - ->willReturn($user); - + $user->expects($this->once())->method("delete"); // delete wird genau einmal aufgerufen + $this->userManager->expects($this->once()) // get wird genau einmal aufgerufen + ->method("get") + ->with($this->equalTo('120042010000000004200001')) + ->willReturn($user); + $this->job->run(null); } - - } diff --git a/tests/unit/UserAccountChangeListenerTest.php b/tests/unit/UserAccountChangeListenerTest.php index 62bef13..cbc8c20 100644 --- a/tests/unit/UserAccountChangeListenerTest.php +++ b/tests/unit/UserAccountChangeListenerTest.php @@ -9,29 +9,26 @@ use OCA\NextMagentaCloudProvisioning\Rules\UserAccountRules; use OCA\NextMagentaCloudProvisioning\User\NmcUserService; use OCA\UserOIDC\Db\ProviderMapper; - use OCA\UserOIDC\Db\UserMapper; - use OCA\UserOIDC\Event\UserAccountChangeEvent; use OCP\Accounts\IAccountManager; use OCP\IConfig; use OCP\IGroupManager; -use OCP\ILogger; - -use OCP\IServerContainer; use OCP\IUserManager; use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; class UserAccountChangeListenerTest extends TestCase { public function setUp(): void { parent::setUp(); $this->app = new \OCP\AppFramework\App(Application::APP_ID); $this->config = $this->getMockForAbstractClass(IConfig::class); - $this->logger = $this->app->getContainer()->get(ILogger::class); + $this->logger = $this->app->getContainer()->get(LoggerInterface::class); + $this->userManager = $this->getMockForAbstractClass(IUserManager::class); + $this->oidcProviderMapper = $this->app->getContainer()->get(ProviderMapper::class); $this->userService = $this->getMockBuilder(NmcUserService::class) ->setConstructorArgs([ $this->app->getContainer()->get(IUserManager::class), $this->app->getContainer()->get(IAccountManager::class), - $this->app->getContainer()->get(IServerContainer::class), $this->logger, $this->config, $this->app->getContainer()->get(UserMapper::class), @@ -41,7 +38,9 @@ public function setUp(): void { ->getMock(); $this->accountRules = new UserAccountRules($this->config, $this->logger, - $this->userService); + $this->userService, + $this->userManager, + $this->oidcProviderMapper); $this->listener = new UserAccountChangeListener($this->logger, $this->accountRules); $this->config->expects($this->any()) diff --git a/tests/unit/UserWithdrawTest.php b/tests/unit/UserWithdrawTest.php index ce47623..b1840cf 100644 --- a/tests/unit/UserWithdrawTest.php +++ b/tests/unit/UserWithdrawTest.php @@ -11,25 +11,22 @@ use OCA\UserOIDC\Db\UserMapper; use OCP\Accounts\IAccountManager; use OCP\IConfig; - use OCP\IGroupManager; - -use OCP\ILogger; -use OCP\IServerContainer; - use OCP\IUserManager; use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; class UserWithdrawTest extends TestCase { public function setUp(): void { parent::setUp(); $this->app = new \OCP\AppFramework\App(Application::APP_ID); $this->config = $this->getMockForAbstractClass(IConfig::class); - $this->logger = $this->app->getContainer()->get(ILogger::class); + $this->logger = $this->app->getContainer()->get(LoggerInterface::class); + $this->userManager = $this->getMockForAbstractClass(IUserManager::class); + $this->oidcProviderMapper = $this->app->getContainer()->get(ProviderMapper::class); $this->userServiceMock = $this->getMockBuilder(NmcUserService::class) ->setConstructorArgs([ $this->app->getContainer()->get(IUserManager::class), $this->app->getContainer()->get(IAccountManager::class), - $this->app->getContainer()->get(IServerContainer::class), $this->logger, $this->config, $this->app->getContainer()->get(UserMapper::class), @@ -39,7 +36,9 @@ public function setUp(): void { ->getMock(); $this->accountService = new UserAccountRules($this->config, $this->logger, - $this->userServiceMock); + $this->userServiceMock, + $this->userManager, + $this->oidcProviderMapper); } public function testDeletionDateOidcClaims() {