diff --git a/apps/user_ldap/lib/AppInfo/Application.php b/apps/user_ldap/lib/AppInfo/Application.php index 25ca6f3a7ba..a01d6efb076 100644 --- a/apps/user_ldap/lib/AppInfo/Application.php +++ b/apps/user_ldap/lib/AppInfo/Application.php @@ -29,6 +29,8 @@ use OCP\AppFramework\Bootstrap\IBootContext; use OCP\AppFramework\Bootstrap\IBootstrap; use OCP\AppFramework\Bootstrap\IRegistrationContext; use OCP\AppFramework\IAppContainer; +use OCP\AppFramework\Services\IAppConfig; +use OCP\Config\IUserConfig; use OCP\EventDispatcher\IEventDispatcher; use OCP\IAvatarManager; use OCP\IConfig; @@ -87,6 +89,8 @@ class Application extends App implements IBootstrap { function (ContainerInterface $c) { return new Manager( $c->get(IConfig::class), + $c->get(IUserConfig::class), + $c->get(IAppConfig::class), $c->get(LoggerInterface::class), $c->get(IAvatarManager::class), $c->get(Image::class), diff --git a/apps/user_ldap/lib/User/Manager.php b/apps/user_ldap/lib/User/Manager.php index 88a001dd965..24ca655a55a 100644 --- a/apps/user_ldap/lib/User/Manager.php +++ b/apps/user_ldap/lib/User/Manager.php @@ -8,7 +8,9 @@ namespace OCA\User_LDAP\User; use OCA\User_LDAP\Access; +use OCP\AppFramework\Services\IAppConfig; use OCP\Cache\CappedMemoryCache; +use OCP\Config\IUserConfig; use OCP\IAvatarManager; use OCP\IConfig; use OCP\IDBConnection; @@ -34,6 +36,8 @@ class Manager { public function __construct( protected IConfig $ocConfig, + protected IUserConfig $userConfig, + protected IAppConfig $appConfig, protected LoggerInterface $logger, protected IAvatarManager $avatarManager, protected Image $image, @@ -63,7 +67,7 @@ class Manager { */ private function createAndCache($dn, $uid) { $this->checkAccess(); - $user = new User($uid, $dn, $this->access, $this->ocConfig, + $user = new User($uid, $dn, $this->access, $this->ocConfig, $this->userConfig, $this->appConfig, clone $this->image, $this->logger, $this->avatarManager, $this->userManager, $this->notificationManager); diff --git a/apps/user_ldap/lib/User/User.php b/apps/user_ldap/lib/User/User.php index 8f97ec1701f..809067d0737 100644 --- a/apps/user_ldap/lib/User/User.php +++ b/apps/user_ldap/lib/User/User.php @@ -15,6 +15,8 @@ use OCA\User_LDAP\Exceptions\AttributeNotSet; use OCA\User_LDAP\Service\BirthdateParserService; use OCP\Accounts\IAccountManager; use OCP\Accounts\PropertyDoesNotExistException; +use OCP\AppFramework\Services\IAppConfig; +use OCP\Config\IUserConfig; use OCP\IAvatarManager; use OCP\IConfig; use OCP\Image; @@ -56,6 +58,8 @@ class User { protected string $dn, protected Access $access, protected IConfig $config, + protected IUserConfig $userConfig, + protected IAppConfig $appConfig, protected Image $image, protected LoggerInterface $logger, protected IAvatarManager $avatarManager, @@ -78,13 +82,13 @@ class User { * @throws PreConditionNotMetException */ public function markUser(): void { - $curValue = $this->config->getUserValue($this->getUsername(), 'user_ldap', 'isDeleted', '0'); - if ($curValue === '1') { + $curValue = $this->userConfig->getValueBool($this->getUsername(), 'user_ldap', 'isDeleted'); + if ($curValue) { // the user is already marked, do not write to DB again return; } - $this->config->setUserValue($this->getUsername(), 'user_ldap', 'isDeleted', '1'); - $this->config->setUserValue($this->getUsername(), 'user_ldap', 'foundDeleted', (string)time()); + $this->userConfig->setValueBool($this->getUsername(), 'user_ldap', 'isDeleted', true); + $this->userConfig->setValueInt($this->getUsername(), 'user_ldap', 'foundDeleted', time()); } /** @@ -286,8 +290,8 @@ class User { $this->connection->writeToCache($cacheKey, $checksum // write array to cache. is waste of cache space , null); // use ldapCacheTTL from configuration // Update user profile - if ($this->config->getUserValue($username, 'user_ldap', 'lastProfileChecksum', null) !== $checksum) { - $this->config->setUserValue($username, 'user_ldap', 'lastProfileChecksum', $checksum); + if ($this->userConfig->getValueString($username, 'user_ldap', 'lastProfileChecksum') !== $checksum) { + $this->userConfig->setValueString($username, 'user_ldap', 'lastProfileChecksum', $checksum); $this->updateProfile($profileValues); $this->logger->info("updated profile uid=$username", ['app' => 'user_ldap']); } else { @@ -361,21 +365,21 @@ class User { } //we need it to store it in the DB as well in case a user gets //deleted so we can clean up afterwards - $this->config->setUserValue( + $this->userConfig->setValueString( $this->getUsername(), 'user_ldap', 'homePath', $path ); return $path; } if (!is_null($attr) - && $this->config->getAppValue('user_ldap', 'enforce_home_folder_naming_rule', 'true') + && $this->appConfig->getAppValueBool('enforce_home_folder_naming_rule', true) ) { // a naming rule attribute is defined, but it doesn't exist for that LDAP user throw new \Exception('Home dir attribute can\'t be read from LDAP for uid: ' . $this->getUsername()); } - //false will apply default behaviour as defined and done by OC_User - $this->config->setUserValue($this->getUsername(), 'user_ldap', 'homePath', ''); + // false will apply default behaviour as defined and done by OC_User + $this->userConfig->setValueString($this->getUsername(), 'user_ldap', 'homePath', ''); return false; } @@ -418,15 +422,15 @@ class User { * @brief marks the user as having logged in at least once */ public function markLogin(): void { - $this->config->setUserValue( - $this->uid, 'user_ldap', self::USER_PREFKEY_FIRSTLOGIN, '1'); + $this->userConfig->setValueBool( + $this->uid, 'user_ldap', self::USER_PREFKEY_FIRSTLOGIN, true); } /** * Stores a key-value pair in relation to this user */ private function store(string $key, string $value): void { - $this->config->setUserValue($this->uid, 'user_ldap', $key, $value); + $this->userConfig->setValueString($this->uid, 'user_ldap', $key, $value); } /** @@ -439,7 +443,7 @@ class User { if ($displayName2 !== '') { $displayName .= ' (' . $displayName2 . ')'; } - $oldName = $this->config->getUserValue($this->uid, 'user_ldap', 'displayName', null); + $oldName = $this->userConfig->getValueString($this->uid, 'user_ldap', 'displayName', ''); if ($oldName !== $displayName) { $this->store('displayName', $displayName); $user = $this->userManager->get($this->getUsername()); @@ -637,7 +641,7 @@ class User { // use the checksum before modifications $checksum = md5($this->image->data()); - if ($checksum === $this->config->getUserValue($this->uid, 'user_ldap', 'lastAvatarChecksum', '') && $this->avatarExists()) { + if ($checksum === $this->userConfig->getValueString($this->uid, 'user_ldap', 'lastAvatarChecksum', '') && $this->avatarExists()) { return true; } @@ -645,7 +649,7 @@ class User { if ($isSet) { // save checksum only after successful setting - $this->config->setUserValue($this->uid, 'user_ldap', 'lastAvatarChecksum', $checksum); + $this->userConfig->setValueString($this->uid, 'user_ldap', 'lastAvatarChecksum', $checksum); } return $isSet; @@ -693,7 +697,7 @@ class User { * @throws PreConditionNotMetException */ public function getExtStorageHome():string { - $value = $this->config->getUserValue($this->getUsername(), 'user_ldap', 'extStorageHome', ''); + $value = $this->userConfig->getValueString($this->getUsername(), 'user_ldap', 'extStorageHome', ''); if ($value !== '') { return $value; } @@ -720,10 +724,10 @@ class User { } if ($extHomeValues !== false && isset($extHomeValues[0])) { $extHome = $extHomeValues[0]; - $this->config->setUserValue($this->getUsername(), 'user_ldap', 'extStorageHome', $extHome); + $this->userConfig->setValueString($this->getUsername(), 'user_ldap', 'extStorageHome', $extHome); return $extHome; } else { - $this->config->deleteUserValue($this->getUsername(), 'user_ldap', 'extStorageHome'); + $this->userConfig->deleteUserConfig($this->getUsername(), 'user_ldap', 'extStorageHome'); return ''; } } @@ -771,7 +775,7 @@ class User { if (!empty($pwdGraceUseTime)) { //was this a grace login? if (!empty($pwdGraceAuthNLimit) && count($pwdGraceUseTime) < (int)$pwdGraceAuthNLimit[0]) { //at least one more grace login available? - $this->config->setUserValue($uid, 'user_ldap', 'needsPasswordReset', 'true'); + $this->userConfig->setValueBool($uid, 'user_ldap', 'needsPasswordReset', true); header('Location: ' . Server::get(IURLGenerator::class)->linkToRouteAbsolute( 'user_ldap.renewPassword.showRenewPasswordForm', ['user' => $uid])); } else { //no more grace login available @@ -782,7 +786,7 @@ class User { } //handle pwdReset attribute if (!empty($pwdReset) && $pwdReset[0] === 'TRUE') { //user must change their password - $this->config->setUserValue($uid, 'user_ldap', 'needsPasswordReset', 'true'); + $this->userConfig->setValueBool($uid, 'user_ldap', 'needsPasswordReset', true); header('Location: ' . Server::get(IURLGenerator::class)->linkToRouteAbsolute( 'user_ldap.renewPassword.showRenewPasswordForm', ['user' => $uid])); exit(); diff --git a/apps/user_ldap/tests/AccessTest.php b/apps/user_ldap/tests/AccessTest.php index 5bda7b8086b..1a45d71e257 100644 --- a/apps/user_ldap/tests/AccessTest.php +++ b/apps/user_ldap/tests/AccessTest.php @@ -20,6 +20,7 @@ use OCA\User_LDAP\Mapping\UserMapping; use OCA\User_LDAP\User\Manager; use OCA\User_LDAP\User\OfflineUser; use OCA\User_LDAP\User\User; +use OCP\Config\IUserConfig; use OCP\EventDispatcher\IEventDispatcher; use OCP\HintException; use OCP\IAppConfig; @@ -102,6 +103,8 @@ class AccessTest extends TestCase { $um = $this->getMockBuilder(Manager::class) ->setConstructorArgs([ $this->createMock(IConfig::class), + $this->createMock(IUserConfig::class), + $this->createMock(\OCP\AppFramework\Services\IAppConfig::class), $this->createMock(LoggerInterface::class), $this->createMock(IAvatarManager::class), $this->createMock(Image::class), diff --git a/apps/user_ldap/tests/User/ManagerTest.php b/apps/user_ldap/tests/User/ManagerTest.php index a8ce9cb3120..2140cbf272e 100644 --- a/apps/user_ldap/tests/User/ManagerTest.php +++ b/apps/user_ldap/tests/User/ManagerTest.php @@ -13,6 +13,8 @@ use OCA\User_LDAP\Connection; use OCA\User_LDAP\ILDAPWrapper; use OCA\User_LDAP\User\Manager; use OCA\User_LDAP\User\User; +use OCP\AppFramework\Services\IAppConfig; +use OCP\Config\IUserConfig; use OCP\IAvatarManager; use OCP\IConfig; use OCP\IDBConnection; @@ -33,6 +35,8 @@ use Psr\Log\LoggerInterface; class ManagerTest extends \Test\TestCase { protected Access&MockObject $access; protected IConfig&MockObject $config; + protected IUserConfig&MockObject $userConfig; + protected IAppConfig&MockObject $appConfig; protected LoggerInterface&MockObject $logger; protected IAvatarManager&MockObject $avatarManager; protected Image&MockObject $image; @@ -49,6 +53,8 @@ class ManagerTest extends \Test\TestCase { $this->access = $this->createMock(Access::class); $this->config = $this->createMock(IConfig::class); + $this->userConfig = $this->createMock(IUserConfig::class); + $this->appConfig = $this->createMock(IAppConfig::class); $this->logger = $this->createMock(LoggerInterface::class); $this->avatarManager = $this->createMock(IAvatarManager::class); $this->image = $this->createMock(Image::class); @@ -66,6 +72,8 @@ class ManagerTest extends \Test\TestCase { /** @noinspection PhpUnhandledExceptionInspection */ $this->manager = new Manager( $this->config, + $this->userConfig, + $this->appConfig, $this->logger, $this->avatarManager, $this->image, diff --git a/apps/user_ldap/tests/User/UserTest.php b/apps/user_ldap/tests/User/UserTest.php index 737ab5f6a86..4bf775efa9d 100644 --- a/apps/user_ldap/tests/User/UserTest.php +++ b/apps/user_ldap/tests/User/UserTest.php @@ -12,6 +12,8 @@ use OCA\User_LDAP\Access; use OCA\User_LDAP\Connection; use OCA\User_LDAP\ILDAPWrapper; use OCA\User_LDAP\User\User; +use OCP\AppFramework\Services\IAppConfig; +use OCP\Config\IUserConfig; use OCP\IAvatar; use OCP\IAvatarManager; use OCP\IConfig; @@ -35,6 +37,8 @@ class UserTest extends \Test\TestCase { protected Access&MockObject $access; protected Connection&MockObject $connection; protected IConfig&MockObject $config; + protected IUserConfig&MockObject $userConfig; + protected IAppConfig&MockObject $appConfig; protected INotificationManager&MockObject $notificationManager; protected IUserManager&MockObject $userManager; protected Image&MockObject $image; @@ -58,6 +62,8 @@ class UserTest extends \Test\TestCase { ->willReturn($this->connection); $this->config = $this->createMock(IConfig::class); + $this->userConfig = $this->createMock(IUserConfig::class); + $this->appConfig = $this->createMock(IAppConfig::class); $this->logger = $this->createMock(LoggerInterface::class); $this->avatarManager = $this->createMock(IAvatarManager::class); $this->image = $this->createMock(Image::class); @@ -69,6 +75,8 @@ class UserTest extends \Test\TestCase { $this->dn, $this->access, $this->config, + $this->userConfig, + $this->appConfig, $this->image, $this->logger, $this->avatarManager, @@ -118,8 +126,8 @@ class UserTest extends \Test\TestCase { $this->equalTo('email')) ->willReturn(false); - $this->config->expects($this->never()) - ->method('setUserValue'); + $this->userConfig->expects($this->never()) + ->method('setValueString'); $this->user->updateEmail(); } @@ -133,8 +141,8 @@ class UserTest extends \Test\TestCase { $this->access->expects($this->never()) ->method('readAttribute'); - $this->config->expects($this->never()) - ->method('setUserValue'); + $this->userConfig->expects($this->never()) + ->method('setValueString'); $this->user->updateEmail(); } @@ -296,8 +304,8 @@ class UserTest extends \Test\TestCase { ->method('get') ->with($this->uid); - $this->config->expects($this->never()) - ->method('setUserValue'); + $this->userConfig->expects($this->never()) + ->method('setValueString'); $this->user->updateQuota(); } @@ -320,8 +328,8 @@ class UserTest extends \Test\TestCase { $this->access->expects($this->never()) ->method('readAttribute'); - $this->config->expects($this->never()) - ->method('setUserValue'); + $this->userConfig->expects($this->never()) + ->method('setValueFloat'); $this->user->updateQuota(); } @@ -487,12 +495,12 @@ class UserTest extends \Test\TestCase { ->method('data') ->willReturn('this is a photo'); - $this->config->expects($this->once()) - ->method('getUserValue') + $this->userConfig->expects($this->once()) + ->method('getValueString') ->with($this->uid, 'user_ldap', 'lastAvatarChecksum', '') ->willReturn(''); - $this->config->expects($this->once()) - ->method('setUserValue') + $this->userConfig->expects($this->once()) + ->method('setValueString') ->with($this->uid, 'user_ldap', 'lastAvatarChecksum', md5('this is a photo')); $avatar = $this->createMock(IAvatar::class); @@ -535,12 +543,12 @@ class UserTest extends \Test\TestCase { ->method('data') ->willReturn('this is a photo'); - $this->config->expects($this->once()) - ->method('getUserValue') + $this->userConfig->expects($this->once()) + ->method('getValueString') ->with($this->uid, 'user_ldap', 'lastAvatarChecksum', '') ->willReturn(md5('this is a photo')); - $this->config->expects($this->never()) - ->method('setUserValue'); + $this->userConfig->expects($this->never()) + ->method('setValueString'); $avatar = $this->createMock(IAvatar::class); $avatar->expects($this->never()) @@ -598,12 +606,12 @@ class UserTest extends \Test\TestCase { ->method('data') ->willReturn('this is a photo'); - $this->config->expects($this->once()) - ->method('getUserValue') + $this->userConfig->expects($this->once()) + ->method('getValueString') ->with($this->uid, 'user_ldap', 'lastAvatarChecksum', '') ->willReturn(''); - $this->config->expects($this->once()) - ->method('setUserValue') + $this->userConfig->expects($this->once()) + ->method('setValueString') ->with($this->uid, 'user_ldap', 'lastAvatarChecksum', md5('this is a photo')); $avatar = $this->createMock(IAvatar::class); @@ -652,10 +660,10 @@ class UserTest extends \Test\TestCase { $this->image->expects($this->never()) ->method('data'); - $this->config->expects($this->never()) - ->method('getUserValue'); - $this->config->expects($this->never()) - ->method('setUserValue'); + $this->userConfig->expects($this->never()) + ->method('getValueString'); + $this->userConfig->expects($this->never()) + ->method('setValueString'); $avatar = $this->createMock(IAvatar::class); $avatar->expects($this->never()) @@ -705,12 +713,12 @@ class UserTest extends \Test\TestCase { ->method('data') ->willReturn('this is a photo'); - $this->config->expects($this->once()) - ->method('getUserValue') + $this->userConfig->expects($this->once()) + ->method('getValueString') ->with($this->uid, 'user_ldap', 'lastAvatarChecksum', '') ->willReturn(''); - $this->config->expects($this->never()) - ->method('setUserValue'); + $this->userConfig->expects($this->never()) + ->method('setValueString'); $avatar = $this->createMock(IAvatar::class); $avatar->expects($this->once()) @@ -756,10 +764,10 @@ class UserTest extends \Test\TestCase { $this->image->expects($this->never()) ->method('data'); - $this->config->expects($this->never()) - ->method('getUserValue'); - $this->config->expects($this->never()) - ->method('setUserValue'); + $this->userConfig->expects($this->never()) + ->method('getValueString'); + $this->userConfig->expects($this->never()) + ->method('setValueString'); $this->avatarManager->expects($this->never()) ->method('getAvatar'); @@ -800,12 +808,12 @@ class UserTest extends \Test\TestCase { } if ($expected !== '') { - $this->config->expects($this->once()) - ->method('setUserValue') + $this->userConfig->expects($this->once()) + ->method('setValueString') ->with($this->uid, 'user_ldap', 'extStorageHome', $expected); } else { - $this->config->expects($this->once()) - ->method('deleteUserValue') + $this->userConfig->expects($this->once()) + ->method('deleteUserConfig') ->with($this->uid, 'user_ldap', 'extStorageHome'); } @@ -814,12 +822,12 @@ class UserTest extends \Test\TestCase { } public function testMarkLogin(): void { - $this->config->expects($this->once()) - ->method('setUserValue') + $this->userConfig->expects($this->once()) + ->method('setValueBool') ->with($this->equalTo($this->uid), $this->equalTo('user_ldap'), $this->equalTo(User::USER_PREFKEY_FIRSTLOGIN), - $this->equalTo(1)) + $this->equalTo(true)) ->willReturn(true); $this->user->markLogin(); @@ -881,6 +889,8 @@ class UserTest extends \Test\TestCase { $this->dn, $this->access, $this->config, + $this->userConfig, + $this->appConfig, $this->image, $this->logger, $this->avatarManager, @@ -944,8 +954,8 @@ class UserTest extends \Test\TestCase { $this->access->expects($this->never()) ->method('readAttribute'); - $this->config->expects($this->never()) - ->method('getAppValue'); + $this->appConfig->expects($this->never()) + ->method('getAppValueBool'); /** @noinspection PhpUnhandledExceptionInspection */ $this->assertFalse($this->user->getHomePath()); @@ -966,8 +976,8 @@ class UserTest extends \Test\TestCase { ->willReturn($this->dn); // asks for "enforce_home_folder_naming_rule" - $this->config->expects($this->once()) - ->method('getAppValue') + $this->appConfig->expects($this->once()) + ->method('getAppValueBool') ->willReturn(false); /** @noinspection PhpUnhandledExceptionInspection */ @@ -992,8 +1002,8 @@ class UserTest extends \Test\TestCase { ->willReturn($this->dn); // asks for "enforce_home_folder_naming_rule" - $this->config->expects($this->once()) - ->method('getAppValue') + $this->appConfig->expects($this->once()) + ->method('getAppValueBool') ->willReturn(true); $this->user->getHomePath(); @@ -1010,12 +1020,12 @@ class UserTest extends \Test\TestCase { #[\PHPUnit\Framework\Attributes\DataProvider('displayNameProvider')] public function testComposeAndStoreDisplayName(string $part1, string $part2, string $expected, bool $expectTriggerChange): void { - $this->config->expects($this->once()) - ->method('setUserValue'); - $oldName = $expectTriggerChange ? 'xxGunslingerxx' : null; - $this->config->expects($this->once()) - ->method('getUserValue') - ->with($this->user->getUsername(), 'user_ldap', 'displayName', null) + $this->userConfig->expects($this->once()) + ->method('setValueString'); + $oldName = $expectTriggerChange ? 'xxGunslingerxx' : ''; + $this->userConfig->expects($this->once()) + ->method('getValueString') + ->with($this->user->getUsername(), 'user_ldap', 'displayName', '') ->willReturn($oldName); $ncUserObj = $this->createMock(\OC\User\User::class); @@ -1037,10 +1047,10 @@ class UserTest extends \Test\TestCase { public function testComposeAndStoreDisplayNameNoOverwrite(): void { $displayName = 'Randall Flagg'; - $this->config->expects($this->never()) - ->method('setUserValue'); - $this->config->expects($this->once()) - ->method('getUserValue') + $this->userConfig->expects($this->never()) + ->method('setValueString'); + $this->userConfig->expects($this->once()) + ->method('getValueString') ->willReturn($displayName); $this->userManager->expects($this->never())