diff --git a/appinfo/info.xml b/appinfo/info.xml index 3350c915..5814b0b2 100755 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -31,7 +31,7 @@ Guests users can only access files shared to them and cannot create any files ou https://raw.githubusercontent.com/nextcloud/guests/master/screenshots/settings.png https://raw.githubusercontent.com/nextcloud/guests/master/screenshots/dropdown.png - + diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 7582c3fa..7b9a873a 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -10,6 +10,7 @@ use OCA\Files\Event\LoadAdditionalScriptsEvent; use OCA\Guests\Capabilities; +use OCA\Guests\ConfigLexicon; use OCA\Guests\GroupBackend; use OCA\Guests\Listener\BeforeTemplateRenderedListener; use OCA\Guests\Listener\BeforeUserManagementRenderedListener; @@ -57,6 +58,7 @@ public function register(IRegistrationContext $context): void { $context->registerEventListener(UserFirstTimeLoggedInEvent::class, UserFirstTimeLoggedInListener::class); $context->registerNotifierService(Notifier::class); + $context->registerConfigLexicon(ConfigLexicon::class); } #[\Override] diff --git a/lib/Config.php b/lib/Config.php index 939e747d..e1103f76 100644 --- a/lib/Config.php +++ b/lib/Config.php @@ -11,6 +11,7 @@ use OCP\AppFramework\Services\IAppConfig; use OCP\Group\ISubAdmin; +use OCP\IAppConfig as IGlobalAppConfig; use OCP\IConfig; use OCP\IGroupManager; use OCP\IUserSession; @@ -18,6 +19,7 @@ class Config { public function __construct( private readonly IConfig $config, + private readonly IGlobalAppConfig $globalAppConfig, private readonly IAppConfig $appConfig, private readonly ISubAdmin $subAdmin, private readonly IUserSession $userSession, @@ -26,11 +28,11 @@ public function __construct( } public function allowExternalStorage(): bool { - return $this->appConfig->getAppValueBool('allow_external_storage', false); + return $this->appConfig->getAppValueBool(ConfigLexicon::EXTERNAL_STORAGE_ENABLED); } - public function setAllowExternalStorage(string|bool $allow): void { - $this->appConfig->setAppValueBool('allow_external_storage', $allow === true || $allow === 'true') ; + public function setAllowExternalStorage(bool $allow): void { + $this->appConfig->setAppValueBool(ConfigLexicon::EXTERNAL_STORAGE_ENABLED, $allow); } public function useHashedEmailAsUserID(): bool { @@ -42,11 +44,11 @@ public function setUseHashedEmailAsUserID(bool $useHash): void { } public function hideOtherUsers(): bool { - return $this->appConfig->getAppValueBool('hide_users', true); + return $this->appConfig->getAppValueBool(ConfigLexicon::HIDE_OTHER_ACCOUNTS); } - public function setHideOtherUsers(string|bool $hide): void { - $this->appConfig->setAppValueBool('hide_users', $hide === true || $hide === 'true') ; + public function setHideOtherUsers(bool $hide): void { + $this->appConfig->setAppValueBool(ConfigLexicon::HIDE_OTHER_ACCOUNTS, $hide); } public function getHome(string $uid): string { @@ -54,31 +56,26 @@ public function getHome(string $uid): string { } public function useWhitelist(): bool { - return $this->appConfig->getAppValueBool('usewhitelist', true); + return $this->appConfig->getAppValueBool(ConfigLexicon::WHITE_LIST_ENABLED); } - public function setUseWhitelist(string|bool $use): void { - $this->appConfig->setAppValueBool('usewhitelist', $use === true || $use === 'true') ; + public function setUseWhitelist(bool $use): void { + $this->appConfig->setAppValueBool(ConfigLexicon::WHITE_LIST_ENABLED, $use); } /** * @return list */ public function getAppWhitelist(): array { - $whitelist = $this->appConfig->getAppValueString('whitelist', AppWhitelist::DEFAULT_WHITELIST); - return explode(',', $whitelist); + return explode(',', $this->appConfig->getAppValueString(ConfigLexicon::WHITE_LIST)); } - public function setAppWhitelist(array|string $whitelist): void { - if (is_array($whitelist)) { - $whitelist = implode(',', $whitelist); - } - - $this->appConfig->setAppValueString('whitelist', $whitelist); + public function setAppWhitelist(array $whitelist): void { + $this->appConfig->setAppValueString(ConfigLexicon::WHITE_LIST, implode(',', $whitelist)); } public function isSharingRestrictedToGroup(): bool { - return $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes'; + return $this->globalAppConfig->getValueBool('core', 'shareapi_only_share_with_group_members'); } public function canCreateGuests(): bool { @@ -111,7 +108,7 @@ public function canCreateGuests(): bool { * @return list */ public function getCreateRestrictedToGroup(): array { - $groups = $this->appConfig->getAppValueArray('create_restricted_to_group', []); + $groups = $this->appConfig->getAppValueArray(ConfigLexicon::GROUP_LIMITATION); // If empty, it means there is no restriction if (empty($groups)) { return []; @@ -120,7 +117,7 @@ public function getCreateRestrictedToGroup(): array { // It does not matter at this point if the admin // group is in the list or not. We are checking it // anyway in the canCreateGuests method. - return array_values(array_unique($this->appConfig->getAppValueArray('create_restricted_to_group', []))); + return array_values(array_unique($this->appConfig->getAppValueArray(ConfigLexicon::GROUP_LIMITATION))); } /** diff --git a/lib/ConfigLexicon.php b/lib/ConfigLexicon.php new file mode 100644 index 00000000..a29a9c64 --- /dev/null +++ b/lib/ConfigLexicon.php @@ -0,0 +1,60 @@ + match ($p) { + Preset::PRIVATE, Preset::FAMILY => '1 GB', + Preset::SMALL, Preset::MEDIUM, Preset::LARGE => '10 GB', + default => '0 B', + }, definition: 'set default disk quota assigned to guest account at its creation'), + new Entry(self::GROUP_LIMITATION, ValueType::ARRAY, []), + ]; + } + + #[\Override] + public function getUserConfigs(): array { + return [ + new Entry(self::USER_CREATED_BY, ValueType::STRING, null, 'user that generated this guest account'), + ]; + } +} diff --git a/lib/Controller/SettingsController.php b/lib/Controller/SettingsController.php index c3c4a560..0b2ffcbb 100644 --- a/lib/Controller/SettingsController.php +++ b/lib/Controller/SettingsController.php @@ -13,8 +13,10 @@ use OCA\Guests\AppInfo\Application; use OCA\Guests\AppWhitelist; use OCA\Guests\Config; +use OCA\Guests\ConfigLexicon; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\Services\IAppConfig; use OCP\IRequest; /** @@ -24,10 +26,10 @@ * @package OCA\Guests\Controller */ class SettingsController extends Controller { - public function __construct( IRequest $request, private readonly Config $config, + private readonly IAppConfig $appConfig, private readonly AppWhitelist $appWhitelist, ) { parent::__construct(Application::APP_ID, $request); @@ -98,10 +100,9 @@ public function getWhitelist(): DataResponse { * @return DataResponse with the reset whitelist */ public function resetWhitelist(): DataResponse { - $this->config->setAppWhitelist(AppWhitelist::DEFAULT_WHITELIST); - + $this->appConfig->deleteAppValue(ConfigLexicon::WHITE_LIST); return new DataResponse([ - 'whitelist' => explode(',', AppWhitelist::DEFAULT_WHITELIST), + 'whitelist' => $this->config->getAppWhitelist(), ]); } } diff --git a/lib/GuestManager.php b/lib/GuestManager.php index c38018a1..7986d5ea 100644 --- a/lib/GuestManager.php +++ b/lib/GuestManager.php @@ -9,6 +9,10 @@ namespace OCA\Guests; +use OCA\Guests\AppInfo\Application; +use OCP\AppFramework\Services\IAppConfig; +use OCP\Config\IUserConfig; +use OCP\Config\ValueType; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\EventDispatcher\IEventDispatcher; use OCP\IConfig; @@ -25,6 +29,8 @@ class GuestManager { public function __construct( private readonly IConfig $config, + private readonly IAppConfig $appConfig, + private readonly IUserConfig $userConfig, private readonly UserBackend $userBackend, private readonly ISecureRandom $secureRandom, private readonly ICrypto $crypto, @@ -72,7 +78,7 @@ public function createGuest(?IUser $createdBy, string $userId, string $email, st $this->userBackend->setInitialEmail($userId, $email); $user->setSystemEMailAddress($email); if ($createdBy instanceof IUser) { - $this->config->setUserValue($userId, 'guests', 'created_by', $createdBy->getUID()); + $this->userConfig->setValueString($userId, Application::APP_ID, ConfigLexicon::USER_CREATED_BY, $createdBy->getUID()); } if ($displayName !== '') { @@ -104,7 +110,7 @@ public function createGuest(?IUser $createdBy, string $userId, string $email, st ); } - $user->setQuota('0 B'); + $user->setQuota($this->appConfig->getAppValueString(ConfigLexicon::GUEST_DISK_QUOTA)); return $user; } @@ -129,7 +135,8 @@ public function getGuestsInfo(): array { $guestsInfo = $this->userBackend->getAllGuestAccounts(); $guests = array_keys($guestsInfo); $shareCounts = $this->getShareCountForUsers($guests); - $createdBy = $this->config->getUserValueForUsers('guests', 'created_by', $guests); + $createdBy = $this->userConfig->getValuesByUsers(Application::APP_ID, ConfigLexicon::USER_CREATED_BY, ValueType::STRING, $guests); + return array_map(function (string $uid) use ($createdBy, $guestsInfo, $shareCounts): array { $allSharesCount = count(array_merge( $this->shareManager->getSharedWith($uid, IShare::TYPE_USER, null, -1, 0), diff --git a/tests/unit/ConfigTest.php b/tests/unit/ConfigTest.php index 6180bd14..890609ac 100644 --- a/tests/unit/ConfigTest.php +++ b/tests/unit/ConfigTest.php @@ -9,10 +9,10 @@ namespace OCA\Guests\Test\Unit; -use OCA\Guests\AppWhitelist; use OCA\Guests\Config; use OCP\AppFramework\Services\IAppConfig; use OCP\Group\ISubAdmin; +use OCP\IAppConfig as IGlobalAppConfig; use OCP\IConfig; use OCP\IGroupManager; use OCP\IUser; @@ -23,7 +23,8 @@ class ConfigTest extends TestCase { /** @var IConfig|MockObject */ private $config; - + /** @var IGlobalAppConfig|MockObject */ + private $globalAppConfig; /** @var IAppConfig|MockObject */ private $appConfig; @@ -42,6 +43,7 @@ protected function setUp(): void { parent::setUp(); $this->config = $this->createMock(IConfig::class); + $this->globalAppConfig = $this->createMock(IGlobalAppConfig::class); $this->appConfig = $this->createMock(IAppConfig::class); $this->subAdmin = $this->createMock(ISubAdmin::class); $this->userSession = $this->createMock(IUserSession::class); @@ -49,6 +51,7 @@ protected function setUp(): void { $this->guestConfig = new Config( $this->config, + $this->globalAppConfig, $this->appConfig, $this->subAdmin, $this->userSession, @@ -65,29 +68,27 @@ public function testAllowExternalStorage(): void { } public function testSetAllowExternalStorage(): void { - $this->appConfig->expects($this->exactly(2)) + $this->appConfig->expects($this->once()) ->method('setAppValueBool') ->with('allow_external_storage', true); $this->guestConfig->setAllowExternalStorage(true); - $this->guestConfig->setAllowExternalStorage('true'); } public function testHideOtherUsers(): void { $this->appConfig->method('getAppValueBool') - ->with('hide_users', true) + ->with('hide_users') ->willReturn(false); $this->assertFalse($this->guestConfig->hideOtherUsers()); } public function testSetHideOtherUsers(): void { - $this->appConfig->expects($this->exactly(2)) + $this->appConfig->expects($this->once()) ->method('setAppValueBool') ->with('hide_users', true); $this->guestConfig->setHideOtherUsers(true); - $this->guestConfig->setHideOtherUsers('true'); } public function testGetHome(): void { @@ -100,42 +101,40 @@ public function testGetHome(): void { public function testUseWhitelist(): void { $this->appConfig->method('getAppValueBool') - ->with('usewhitelist', true) + ->with('usewhitelist') ->willReturn(false); $this->assertFalse($this->guestConfig->useWhitelist()); } public function testSetUseWhitelist(): void { - $this->appConfig->expects($this->exactly(2)) + $this->appConfig->expects($this->once()) ->method('setAppValueBool') ->with('usewhitelist', true); $this->guestConfig->setUseWhitelist(true); - $this->guestConfig->setUseWhitelist('true'); } public function testGetAppWhitelist(): void { $this->appConfig->method('getAppValueString') - ->with('whitelist', AppWhitelist::DEFAULT_WHITELIST) + ->with('whitelist') ->willReturn('app1,app2,app3'); $this->assertEquals(['app1', 'app2', 'app3'], $this->guestConfig->getAppWhitelist()); } public function testSetAppWhitelistArray(): void { - $this->appConfig->expects($this->exactly(2)) + $this->appConfig->expects($this->once()) ->method('setAppValueString') ->with('whitelist', 'app1,app2,app3'); $this->guestConfig->setAppWhitelist(['app1', 'app2', 'app3']); - $this->guestConfig->setAppWhitelist('app1,app2,app3'); } public function testIsSharingRestrictedToGroup(): void { - $this->config->method('getAppValue') - ->with('core', 'shareapi_only_share_with_group_members', 'no') - ->willReturn('yes'); + $this->globalAppConfig->method('getValueBool') + ->with('core', 'shareapi_only_share_with_group_members') + ->willReturn(true); $this->assertTrue($this->guestConfig->isSharingRestrictedToGroup()); } @@ -200,9 +199,9 @@ public function testCanCreateGuestsWithSharingRestrictedButIsSubAdmin(): void { ->with('create_restricted_to_group', []) ->willReturn([]); - $this->config->method('getAppValue') - ->with('core', 'shareapi_only_share_with_group_members', 'no') - ->willReturn('yes'); + $this->globalAppConfig->method('getValueBool') + ->with('core', 'shareapi_only_share_with_group_members') + ->willReturn(true); $this->subAdmin->method('isSubAdmin') ->with($user) @@ -220,9 +219,9 @@ public function testCanCreateGuestsWithSharingRestrictedNotSubAdmin(): void { ->with('create_restricted_to_group', []) ->willReturn([]); - $this->config->method('getAppValue') - ->with('core', 'shareapi_only_share_with_group_members', 'no') - ->willReturn('yes'); + $this->globalAppConfig->method('getValueBool') + ->with('core', 'shareapi_only_share_with_group_members') + ->willReturn(true); $this->subAdmin->method('isSubAdmin') ->with($user) diff --git a/tests/unit/Controller/UsersControllerTest.php b/tests/unit/Controller/UsersControllerTest.php index fc5d155d..67cdc4f7 100644 --- a/tests/unit/Controller/UsersControllerTest.php +++ b/tests/unit/Controller/UsersControllerTest.php @@ -18,6 +18,7 @@ use OCP\AppFramework\Http; use OCP\AppFramework\Services\IAppConfig; use OCP\Group\ISubAdmin; +use OCP\IAppConfig as IGlobalAppConfig; use OCP\IConfig; use OCP\IGroup; use OCP\IGroupManager; @@ -32,32 +33,20 @@ class UsersControllerTest extends TestCase { private IRequest&MockObject $request; - private IUserManager&MockObject $userManager; - private IL10N&MockObject $l10n; - - private Config $guestsConfig; - private IMailer&MockObject $mailer; - private GuestManager&MockObject $guestManager; - private IUserSession&MockObject $userSession; - private ISubAdmin&MockObject $subAdmin; - private IGroupManager&MockObject $groupManager; - private TransferService&MockObject $transferService; - private TransferMapper&MockObject $transferMapper; - + private IGlobalAppConfig&MockObject $globalAppConfig; private IAppConfig&MockObject $appConfig; - private IConfig&MockObject $config; - private InviteService&MockObject $inviteService; + private Config $guestsConfig; private UsersController $controller; @@ -74,12 +63,14 @@ protected function setUp(): void { $this->groupManager = $this->createMock(IGroupManager::class); $this->transferService = $this->createMock(TransferService::class); $this->transferMapper = $this->createMock(TransferMapper::class); + $this->globalAppConfig = $this->createMock(IGlobalAppConfig::class); $this->appConfig = $this->createMock(IAppConfig::class); $this->config = $this->createMock(IConfig::class); $this->inviteService = $this->createMock(InviteService::class); $this->guestsConfig = new Config( $this->config, + $this->globalAppConfig, $this->appConfig, $this->subAdmin, $this->userSession, @@ -301,10 +292,9 @@ public function testCreateSucceedsForSubAdminWhenSharingIsRestricted(): void { ->willReturn([]); // Sharing is restricted to group - $this->config->method('getAppValue') - ->with('core', 'shareapi_only_share_with_group_members', 'no') - ->willReturn('yes'); - + $this->globalAppConfig->method('getValueBool') + ->with('core', 'shareapi_only_share_with_group_members') + ->willReturn(true); // User is a subadmin $this->subAdmin->method('isSubAdmin') ->with($currentUser) @@ -359,10 +349,9 @@ public function testCreateFailsForNonSubAdminWhenSharingIsRestricted(): void { ->willReturn([]); // Sharing is restricted to group - $this->config->method('getAppValue') - ->with('core', 'shareapi_only_share_with_group_members', 'no') - ->willReturn('yes'); - + $this->globalAppConfig->method('getValueBool') + ->with('core', 'shareapi_only_share_with_group_members') + ->willReturn(true); // User is NOT a subadmin $this->subAdmin->method('isSubAdmin') ->with($currentUser) @@ -440,9 +429,9 @@ public function testCreateWhenSharingRestrictedToGroupButNoGroups(): void { ->willReturn([]); // Sharing is restricted to group - $this->config->method('getAppValue') - ->with('core', 'shareapi_only_share_with_group_members', 'no') - ->willReturn('yes'); + $this->globalAppConfig->method('getValueBool') + ->with('core', 'shareapi_only_share_with_group_members') + ->willReturn(true); // User is an admin // so they are allowed to create guests @@ -479,9 +468,9 @@ public function testCreateWithNonExistentGroup(): void { ->willReturn(true); // Sharing is restricted to group - $this->config->method('getAppValue') - ->with('core', 'shareapi_only_share_with_group_members', 'no') - ->willReturn('yes'); + $this->globalAppConfig->method('getValueBool') + ->with('core', 'shareapi_only_share_with_group_members') + ->willReturn(true); $this->assertTrue($this->guestsConfig->canCreateGuests()); $this->assertTrue($this->guestsConfig->isSharingRestrictedToGroup()); @@ -527,9 +516,9 @@ public function testCreateWithGroupButNotSubAdmin(): void { ->willReturn(false); // Sharing is restricted to group - $this->config->method('getAppValue') - ->with('core', 'shareapi_only_share_with_group_members', 'no') - ->willReturn('yes'); + $this->globalAppConfig->method('getValueBool') + ->with('core', 'shareapi_only_share_with_group_members') + ->willReturn(true); // There is no group restriction in place $this->appConfig->method('getAppValueArray') @@ -710,10 +699,9 @@ public function testCreateSuccessWithGroupsAsSubadmin(): void { ->willReturn(['other_group1', 'other_group2']); // Sharing is restricted to group - $this->config->method('getAppValue') - ->with('core', 'shareapi_only_share_with_group_members', 'no') - ->willReturn('yes'); - + $this->globalAppConfig->method('getValueBool') + ->with('core', 'shareapi_only_share_with_group_members') + ->willReturn(true); $this->assertTrue($this->guestsConfig->canCreateGuests()); $this->assertTrue($this->guestsConfig->isSharingRestrictedToGroup()); diff --git a/tests/unit/GuestManagerTest.php b/tests/unit/GuestManagerTest.php index 0dc1f59e..9a073eb9 100644 --- a/tests/unit/GuestManagerTest.php +++ b/tests/unit/GuestManagerTest.php @@ -8,8 +8,11 @@ namespace OCA\Guests\Test\Unit; +use OCA\Guests\ConfigLexicon; use OCA\Guests\GuestManager; use OCA\Guests\UserBackend; +use OCP\AppFramework\Services\IAppConfig; +use OCP\Config\IUserConfig; use OCP\EventDispatcher\IEventDispatcher; use OCP\IConfig; use OCP\IDBConnection; @@ -34,8 +37,12 @@ class GuestManagerTest extends TestCase { /** @var IConfig|MockObject */ private $config; - + /** @var IAppConfig|MockObject */ + private $appConfig; + /** @var IUserConfig|MockObject */ + private $userConfig; /** @var ISecureRandom|MockObject */ + private $random; /** @var ICrypto|MockObject */ @@ -59,6 +66,8 @@ protected function setUp(): void { $this->userSession = $this->createMock(IUserSession::class); $this->userManager = $this->createMock(IUserManager::class); $this->config = $this->createMock(IConfig::class); + $this->appConfig = $this->createMock(IAppConfig::class); + $this->userConfig = $this->createMock(IUserConfig::class); $this->random = $this->createMock(ISecureRandom::class); $this->random->method('generate') ->willReturnCallback(fn ($count) => str_repeat('4', $count)); @@ -71,6 +80,8 @@ protected function setUp(): void { $this->guestManager = new GuestManager( $this->config, + $this->appConfig, + $this->userConfig, $this->userBackend, $this->random, $this->crypto, @@ -134,14 +145,18 @@ public function testIsGuestNull(): void { public function testCreateGuest(): void { $setValues = []; - $this->config->method('setUserValue') - ->willReturnCallback(function ($user, $app, $key, $value) use (&$setValues): void { - if (!isset($setValues[$app])) { - $setValues[$app] = []; - } + $fnSetValues = function ($user, $app, $key, $value) use (&$setValues): bool { + if (!isset($setValues[$app])) { + $setValues[$app] = []; + } + $setValues[$app][$key] = $value; + return true; + }; - $setValues[$app][$key] = $value; - }); + $this->config->method('setUserValue') + ->willReturnCallback($fnSetValues); + $this->userConfig->method('setValueString') + ->willReturnCallback($fnSetValues); $createdByUser = $this->createMock(IUser::class); $createdByUser->method('getUID') @@ -154,6 +169,11 @@ public function testCreateGuest(): void { ->with('guest@example.com', str_repeat('4', 20), $this->userBackend) ->willReturn($guestUser); + $this->appConfig->expects($this->once()) + ->method('getAppValueString') + ->with(ConfigLexicon::GUEST_DISK_QUOTA) + ->willReturn('0 B'); + $guestUser->expects($this->once()) ->method('setDisplayName') ->with('Example Guest');