diff --git a/lib/Config.php b/lib/Config.php index faf7374d..8b82427c 100644 --- a/lib/Config.php +++ b/lib/Config.php @@ -8,57 +8,42 @@ namespace OCA\Guests; +use OCP\AppFramework\Services\IAppConfig; use OCP\Group\ISubAdmin; use OCP\IConfig; +use OCP\IGroupManager; use OCP\IUserSession; class Config { - private $config; - private $subAdmin; - private $userSession; - - public function __construct(IConfig $config, ISubAdmin $subAdmin, IUserSession $userSession) { - $this->config = $config; - $this->subAdmin = $subAdmin; - $this->userSession = $userSession; - } - - /** - * @param string|bool $value - * @return bool - */ - private function castToBool($value): bool { - return $value === 'true' || $value === true; - } - - /** - * @param string|bool $value - * @return string - */ - private function castToString($value): string { - return ($value === 'true' || $value === true) ? 'true' : 'false'; + public function __construct( + private IConfig $config, + private IAppConfig $appConfig, + private ISubAdmin $subAdmin, + private IUserSession $userSession, + private IGroupManager $groupManager, + ) { } public function allowExternalStorage(): bool { - return $this->castToBool($this->config->getAppValue('guests', 'allow_external_storage', 'false')); + return $this->appConfig->getAppValueBool('allow_external_storage', false); } /** * @param string|bool $allow */ public function setAllowExternalStorage($allow) { - $this->config->setAppValue('guests', 'allow_external_storage', $this->castToString($allow)) ; + $this->appConfig->setAppValueBool('allow_external_storage', $allow === true || $allow === 'true') ; } public function hideOtherUsers(): bool { - return $this->castToBool($this->config->getAppValue('guests', 'hide_users', 'true')); + return $this->appConfig->getAppValueBool('hide_users', true); } /** * @param string|bool $hide */ public function setHideOtherUsers($hide): void { - $this->config->setAppValue('guests', 'hide_users', $this->castToString($hide)) ; + $this->appConfig->setAppValueBool('hide_users', $hide === true || $hide === 'true') ; } public function getHome(string $uid): string { @@ -66,21 +51,21 @@ public function getHome(string $uid): string { } public function useWhitelist(): bool { - return $this->castToBool($this->config->getAppValue('guests', 'usewhitelist', 'true')); + return $this->appConfig->getAppValueBool('usewhitelist', true); } /** * @param string|bool $use */ public function setUseWhitelist($use) { - $this->config->setAppValue('guests', 'usewhitelist', $this->castToString($use)) ; + $this->appConfig->setAppValueBool('usewhitelist', $use === true || $use === 'true') ; } /** * @return string[] */ public function getAppWhitelist(): array { - $whitelist = $this->config->getAppValue('guests', 'whitelist', AppWhitelist::DEFAULT_WHITELIST); + $whitelist = $this->appConfig->getAppValueString('whitelist', AppWhitelist::DEFAULT_WHITELIST); return explode(',', $whitelist); } @@ -91,7 +76,7 @@ public function setAppWhitelist($whitelist): void { if (is_array($whitelist)) { $whitelist = implode(',', $whitelist); } - $this->config->setAppValue('guests', 'whitelist', $whitelist); + $this->appConfig->setAppValueString('whitelist', $whitelist); } public function isSharingRestrictedToGroup(): bool { @@ -99,9 +84,52 @@ public function isSharingRestrictedToGroup(): bool { } public function canCreateGuests(): bool { - if (!$this->userSession->getUser()) { + $user = $this->userSession->getUser(); + if (!$user) { return false; } - return (!$this->isSharingRestrictedToGroup()) || $this->subAdmin->isSubAdmin($this->userSession->getUser()); + + // Admins and sub-admins can always create guests + if ($this->groupManager->isAdmin($user->getUID()) + || $this->subAdmin->isSubAdmin($user)) { + return true; + } + + // Check if we have a group restriction + // and if the user belong to that group + $groupRestriction = $this->getCreateRestrictedToGroup(); + if (!empty($groupRestriction)) { + $userGroups = $this->groupManager->getUserGroupIds($user); + $groupRestriction = array_intersect($userGroups, $groupRestriction); + if (empty($groupRestriction)) { + return false; + } + } + + + return !$this->isSharingRestrictedToGroup(); + } + + /** + * @return string[] + */ + public function getCreateRestrictedToGroup(): array { + $groups = $this->appConfig->getAppValueArray('create_restricted_to_group', []); + // If empty, it means there is no restriction + if (empty($groups)) { + return []; + } + + // 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', []))); + } + + /** + * @param string[] $groups + */ + public function setCreateRestrictedToGroup(array $groups): void { + $this->appConfig->setAppValueArray('create_restricted_to_group', $groups); } } diff --git a/lib/Controller/SettingsController.php b/lib/Controller/SettingsController.php index 4879590a..2a6613c6 100644 --- a/lib/Controller/SettingsController.php +++ b/lib/Controller/SettingsController.php @@ -23,20 +23,12 @@ */ class SettingsController extends Controller { - /** @var Config */ - private $config; - - /** @var AppWhitelist */ - private $appWhitelist; - public function __construct( IRequest $request, - Config $config, - AppWhitelist $appWhitelist, + private Config $config, + private AppWhitelist $appWhitelist, ) { parent::__construct(Application::APP_ID, $request); - $this->config = $config; - $this->appWhitelist = $appWhitelist; } /** @@ -56,6 +48,7 @@ public function getConfig(): DataResponse { 'hideUsers' => $hideUsers, 'whiteListableApps' => $this->appWhitelist->getWhitelistAbleApps(), 'sharingRestrictedToGroup' => $this->config->isSharingRestrictedToGroup(), + 'createRestrictedToGroup' => $this->config->getCreateRestrictedToGroup(), ]); } @@ -66,7 +59,7 @@ public function getConfig(): DataResponse { * @param $hideUsers bool * @return DataResponse */ - public function setConfig(bool $useWhitelist, array $whitelist, bool $allowExternalStorage, bool $hideUsers): DataResponse { + public function setConfig(bool $useWhitelist, array $whitelist, bool $allowExternalStorage, bool $hideUsers, array $createRestrictedToGroup): DataResponse { $newWhitelist = []; foreach ($whitelist as $app) { $newWhitelist[] = trim($app); @@ -75,6 +68,7 @@ public function setConfig(bool $useWhitelist, array $whitelist, bool $allowExter $this->config->setAppWhitelist($newWhitelist); $this->config->setAllowExternalStorage($allowExternalStorage); $this->config->setHideOtherUsers($hideUsers); + $this->config->setCreateRestrictedToGroup($createRestrictedToGroup); return new DataResponse(); } diff --git a/lib/Controller/UsersController.php b/lib/Controller/UsersController.php index 38f4f5ec..de9ff413 100644 --- a/lib/Controller/UsersController.php +++ b/lib/Controller/UsersController.php @@ -67,10 +67,11 @@ public function create(string $email, string $displayName, string $language, arr Http::STATUS_FORBIDDEN ); } + if (!$this->config->canCreateGuests()) { return new DataResponse( [ - 'errorMessages' => ['This user is not allowed to create guests'], + 'errorMessages' => ['You are not allowed to create guests'], ], Http::STATUS_FORBIDDEN ); @@ -99,7 +100,7 @@ public function create(string $email, string $displayName, string $language, arr if (!($this->subAdmin->isSubAdminOfGroup($currentUser, $group) || $this->groupManager->isAdmin($currentUser->getUID()))) { return new DataResponse( [ - 'errorMessages' => ["This user is not allowed to add users to group $groupId"], + 'errorMessages' => ["You are not allowed to add users to group $groupId"], ], Http::STATUS_FORBIDDEN ); diff --git a/lib/GuestManager.php b/lib/GuestManager.php index 396da301..5db7917a 100644 --- a/lib/GuestManager.php +++ b/lib/GuestManager.php @@ -67,7 +67,7 @@ public function createGuest(?IUser $createdBy, string $userId, string $email, st $this->userBackend ); - $user->setEMailAddress($email); + $user->setSystemEMailAddress($email); if ($createdBy) { $this->config->setUserValue($userId, 'guests', 'created_by', $createdBy->getUID()); } diff --git a/lib/Listener/LoadAdditionalScriptsListener.php b/lib/Listener/LoadAdditionalScriptsListener.php index f23c06c2..9d431ea2 100644 --- a/lib/Listener/LoadAdditionalScriptsListener.php +++ b/lib/Listener/LoadAdditionalScriptsListener.php @@ -11,6 +11,7 @@ namespace OCA\Guests\Listener; use OCA\Files\Event\LoadAdditionalScriptsEvent; +use OCA\Guests\Config; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; use OCP\Util; @@ -19,7 +20,18 @@ * @template-implements IEventListener */ class LoadAdditionalScriptsListener implements IEventListener { + + public function __construct( + private Config $config, + ) { + } + public function handle(Event $event): void { + // If the user cannot create guests, we don't need to load the script + if (!$this->config->canCreateGuests()) { + return; + } + Util::addScript('guests', 'guests-main'); } } diff --git a/lib/UserBackend.php b/lib/UserBackend.php index 4d8f55e7..f0f7b260 100644 --- a/lib/UserBackend.php +++ b/lib/UserBackend.php @@ -9,7 +9,7 @@ namespace OCA\Guests; use InvalidArgumentException; -use OC\Cache\CappedMemoryCache; +use OCP\Cache\CappedMemoryCache; use OCP\EventDispatcher\IEventDispatcher; use OCP\IDBConnection; use OCP\Security\Events\ValidatePasswordPolicyEvent; @@ -38,28 +38,16 @@ class UserBackend extends ABackend implements IPasswordHashBackend { /** @var CappedMemoryCache */ private $cache; - /** @var IEventDispatcher */ - private $eventDispatcher; - /** @var IDBConnection */ - private $dbConn; - /** @var Config */ - private $config; - /** @var IHasher */ - private $hasher; /** @var bool */ private $allowListing = true; public function __construct( - IEventDispatcher $eventDispatcher, - IDBConnection $connection, - Config $config, - IHasher $hasher, + private IEventDispatcher $eventDispatcher, + private IDBConnection $dbConn, + private Config $config, + private IHasher $hasher, ) { $this->cache = new CappedMemoryCache(); - $this->eventDispatcher = $eventDispatcher; - $this->dbConn = $connection; - $this->config = $config; - $this->hasher = $hasher; } public function setAllowListing(bool $allow) { diff --git a/src/views/GuestForm.vue b/src/views/GuestForm.vue index bcb7717e..bfac7c81 100644 --- a/src/views/GuestForm.vue +++ b/src/views/GuestForm.vue @@ -12,6 +12,7 @@
+
+ +
+ +
+ +
- {{ t('guests', 'An error occurred, try again') }} + + {{ t('guests', 'An error occurred, try again') }} + @@ -73,15 +89,17 @@ diff --git a/tests/unit/ConfigTest.php b/tests/unit/ConfigTest.php new file mode 100644 index 00000000..d0c0116c --- /dev/null +++ b/tests/unit/ConfigTest.php @@ -0,0 +1,254 @@ +config = $this->createMock(IConfig::class); + $this->appConfig = $this->createMock(IAppConfig::class); + $this->subAdmin = $this->createMock(ISubAdmin::class); + $this->userSession = $this->createMock(IUserSession::class); + $this->groupManager = $this->createMock(IGroupManager::class); + + $this->guestConfig = new Config( + $this->config, + $this->appConfig, + $this->subAdmin, + $this->userSession, + $this->groupManager + ); + } + + public function testAllowExternalStorage() { + $this->appConfig->method('getAppValueBool') + ->with('allow_external_storage', false) + ->willReturn(true); + + $this->assertTrue($this->guestConfig->allowExternalStorage()); + } + + public function testSetAllowExternalStorage() { + $this->appConfig->expects($this->exactly(2)) + ->method('setAppValueBool') + ->with('allow_external_storage', true); + + $this->guestConfig->setAllowExternalStorage(true); + $this->guestConfig->setAllowExternalStorage('true'); + } + + public function testHideOtherUsers() { + $this->appConfig->method('getAppValueBool') + ->with('hide_users', true) + ->willReturn(false); + + $this->assertFalse($this->guestConfig->hideOtherUsers()); + } + + public function testSetHideOtherUsers() { + $this->appConfig->expects($this->exactly(2)) + ->method('setAppValueBool') + ->with('hide_users', true); + + $this->guestConfig->setHideOtherUsers(true); + $this->guestConfig->setHideOtherUsers('true'); + } + + public function testGetHome() { + $this->config->method('getSystemValue') + ->with('datadirectory', \OC::$SERVERROOT . '/data') + ->willReturn('/custom/path'); + + $this->assertEquals('/custom/path/test_user', $this->guestConfig->getHome('test_user')); + } + + public function testUseWhitelist() { + $this->appConfig->method('getAppValueBool') + ->with('usewhitelist', true) + ->willReturn(false); + + $this->assertFalse($this->guestConfig->useWhitelist()); + } + + public function testSetUseWhitelist() { + $this->appConfig->expects($this->exactly(2)) + ->method('setAppValueBool') + ->with('usewhitelist', true); + + $this->guestConfig->setUseWhitelist(true); + $this->guestConfig->setUseWhitelist('true'); + } + + public function testGetAppWhitelist() { + $this->appConfig->method('getAppValueString') + ->with('whitelist', AppWhitelist::DEFAULT_WHITELIST) + ->willReturn('app1,app2,app3'); + + $this->assertEquals(['app1', 'app2', 'app3'], $this->guestConfig->getAppWhitelist()); + } + + public function testSetAppWhitelistArray() { + $this->appConfig->expects($this->exactly(2)) + ->method('setAppValueString') + ->with('whitelist', 'app1,app2,app3'); + + $this->guestConfig->setAppWhitelist(['app1', 'app2', 'app3']); + $this->guestConfig->setAppWhitelist('app1,app2,app3'); + } + + public function testIsSharingRestrictedToGroup() { + $this->config->method('getAppValue') + ->with('core', 'shareapi_only_share_with_group_members', 'no') + ->willReturn('yes'); + + $this->assertTrue($this->guestConfig->isSharingRestrictedToGroup()); + } + + public function testIsSharingNotRestrictedToGroup() { + $this->config->method('getAppValue') + ->with('core', 'shareapi_only_share_with_group_members', 'no') + ->willReturn('no'); + + $this->assertFalse($this->guestConfig->isSharingRestrictedToGroup()); + } + + public function testCanCreateGuestsNoUser() { + $this->userSession->method('getUser') + ->willReturn(null); + + $this->assertFalse($this->guestConfig->canCreateGuests()); + } + + public function testCanCreateGuestsWithGroupRestrictionNoMatch() { + $user = $this->createMock(IUser::class); + $this->userSession->method('getUser') + ->willReturn($user); + + $this->appConfig->method('getAppValueArray') + ->with('create_restricted_to_group', []) + ->willReturn(['group1', 'group2']); + + $this->groupManager->method('getUserGroupIds') + ->with($user) + ->willReturn(['group3', 'group4']); + + $this->assertFalse($this->guestConfig->canCreateGuests()); + } + + public function testCanCreateGuestsWithGroupRestrictionWithMatch() { + $user = $this->createMock(IUser::class); + $this->userSession->method('getUser') + ->willReturn($user); + + $this->appConfig->method('getAppValueArray') + ->with('create_restricted_to_group', []) + ->willReturn(['group1', 'group2']); + + $this->groupManager->method('getUserGroupIds') + ->with($user) + ->willReturn(['group2', 'group3']); + + $this->config->method('getAppValue') + ->with('core', 'shareapi_only_share_with_group_members', 'no') + ->willReturn('no'); + + $this->assertTrue($this->guestConfig->canCreateGuests()); + } + + public function testCanCreateGuestsWithSharingRestrictedButIsSubAdmin() { + $user = $this->createMock(IUser::class); + $this->userSession->method('getUser') + ->willReturn($user); + + $this->appConfig->method('getAppValueArray') + ->with('create_restricted_to_group', []) + ->willReturn([]); + + $this->config->method('getAppValue') + ->with('core', 'shareapi_only_share_with_group_members', 'no') + ->willReturn('yes'); + + $this->subAdmin->method('isSubAdmin') + ->with($user) + ->willReturn(true); + + $this->assertTrue($this->guestConfig->canCreateGuests()); + } + + public function testCanCreateGuestsWithSharingRestrictedNotSubAdmin() { + $user = $this->createMock(IUser::class); + $this->userSession->method('getUser') + ->willReturn($user); + + $this->appConfig->method('getAppValueArray') + ->with('create_restricted_to_group', []) + ->willReturn([]); + + $this->config->method('getAppValue') + ->with('core', 'shareapi_only_share_with_group_members', 'no') + ->willReturn('yes'); + + $this->subAdmin->method('isSubAdmin') + ->with($user) + ->willReturn(false); + + $this->assertFalse($this->guestConfig->canCreateGuests()); + } + + public function testGetCreateRestrictedToGroupEmpty() { + $this->appConfig->method('getAppValueArray') + ->with('create_restricted_to_group', []) + ->willReturn([]); + + $this->assertEquals([], $this->guestConfig->getCreateRestrictedToGroup()); + } + + public function testGetCreateRestrictedToGroupWithAdmin() { + $this->appConfig->method('getAppValueArray') + ->with('create_restricted_to_group', []) + ->willReturn(['admin', 'group1']); + + $this->assertEquals(['admin', 'group1'], $this->guestConfig->getCreateRestrictedToGroup()); + } + + public function testSetCreateRestrictedToGroup() { + $this->appConfig->expects($this->once()) + ->method('setAppValueArray') + ->with('create_restricted_to_group', ['group1', 'group2']); + + $this->guestConfig->setCreateRestrictedToGroup(['group1', 'group2']); + } +} diff --git a/tests/unit/Controller/UsersControllerTest.php b/tests/unit/Controller/UsersControllerTest.php new file mode 100644 index 00000000..84d9110c --- /dev/null +++ b/tests/unit/Controller/UsersControllerTest.php @@ -0,0 +1,763 @@ +request = $this->createMock(IRequest::class); + $this->userManager = $this->createMock(IUserManager::class); + $this->l10n = $this->createMock(IL10N::class); + $this->mailer = $this->createMock(IMailer::class); + $this->guestManager = $this->createMock(GuestManager::class); + $this->userSession = $this->createMock(IUserSession::class); + $this->subAdmin = $this->createMock(ISubAdmin::class); + $this->groupManager = $this->createMock(IGroupManager::class); + $this->transferService = $this->createMock(TransferService::class); + $this->transferMapper = $this->createMock(TransferMapper::class); + $this->appConfig = $this->createMock(IAppConfig::class); + $this->config = $this->createMock(IConfig::class); + + $this->guestsConfig = new Config( + $this->config, + $this->appConfig, + $this->subAdmin, + $this->userSession, + $this->groupManager + ); + + $this->l10n->method('t') + ->willReturnCallback(function ($text) { + return $text; + }); + + $this->controller = new UsersController( + 'guests', + $this->request, + $this->userManager, + $this->l10n, + $this->guestsConfig, + $this->mailer, + $this->guestManager, + $this->userSession, + $this->subAdmin, + $this->groupManager, + $this->transferService, + $this->transferMapper + ); + } + + /** + * Test that creation fails when the current user is a guest. + */ + public function testCreateWhenCurrentUserIsGuest() { + $currentUser = $this->createMock(IUser::class); + $this->userSession->method('getUser') + ->willReturn($currentUser); + + $this->guestManager->method('isGuest') + ->with($currentUser) + ->willReturn(true); + + // No restrictions in place + $this->assertTrue($this->guestsConfig->canCreateGuests()); + + $response = $this->controller->create('test@example.com', 'Test User', 'en', []); + $this->assertEquals(Http::STATUS_FORBIDDEN, $response->getStatus()); + $this->assertEquals(['errorMessages' => ['Guests are not allowed to create guests']], $response->getData()); + } + + /** + * Test that creation fails when the user is not logged in. + */ + public function testCreateFailsWithNoUserSession() { + // Mock user session to return null (no logged-in user) + $this->userSession->method('getUser') + ->willReturn(null); + + // Empty user session will return false straight away + $this->assertFalse($this->guestsConfig->canCreateGuests()); + + $response = $this->controller->create('test@example.com', 'Test User', 'en', []); + $this->assertEquals(Http::STATUS_FORBIDDEN, $response->getStatus()); + $this->assertEquals(['errorMessages' => ['You are not allowed to create guests']], $response->getData()); + } + + /** + * Test that creation fails when user is not in allowed groups + */ + public function testCreateFailsWhenUserNotInAllowedGroups() { + // Mock current user + $currentUser = $this->createMock(IUser::class); + $currentUser->method('getUID') + ->willReturn('test_user'); + + $this->userSession->method('getUser') + ->willReturn($currentUser); + + $this->guestManager->method('isGuest') + ->with($currentUser) + ->willReturn(false); + + // Set up group restriction check + $this->appConfig->method('getAppValueArray') + ->with('create_restricted_to_group', []) + ->willReturn(['allowed_group1', 'allowed_group2']); + + // User's groups don't intersect with allowed groups + $this->groupManager->method('getUserGroupIds') + ->with($currentUser) + ->willReturn(['other_group1', 'other_group2']); + + $response = $this->controller->create('test@example.com', 'Test User', 'en', []); + $this->assertEquals(Http::STATUS_FORBIDDEN, $response->getStatus()); + $this->assertEquals(['errorMessages' => ['You are not allowed to create guests']], $response->getData()); + } + + /** + * Test that admin users can always create guests regardless of group restrictions + */ + public function testCreateSucceedsForAdminDespiteGroupRestrictions() { + // Mock current user + $currentUser = $this->createMock(IUser::class); + $currentUser->method('getUID') + ->willReturn('admin_user'); + + $this->userSession->method('getUser') + ->willReturn($currentUser); + + $this->guestManager->method('isGuest') + ->with($currentUser) + ->willReturn(false); + + // Set up group restriction check + $this->appConfig->method('getAppValueArray') + ->with('create_restricted_to_group', []) + ->willReturn(['allowed_group1', 'allowed_group2']); + + // User is in the admin group + $this->groupManager->method('getUserGroupIds') + ->with($currentUser) + ->willReturn(['admin', 'other_group']); + + // User is an admin + $this->groupManager->method('isAdmin') + ->with('admin_user') + ->willReturn(true); + + // Other necessary mocks for a successful creation + $this->mailer->method('validateMailAddress') + ->willReturn(true); + + $this->userManager->method('getByEmail') + ->willReturn([]); + + $this->userManager->method('userExists') + ->willReturn(false); + + $guestUser = $this->createMock(IUser::class); + $this->guestManager->method('createGuest') + ->willReturn($guestUser); + + $this->userManager->method('get') + ->willReturn($guestUser); + + $response = $this->controller->create('new@example.com', 'Test User', 'en', []); + $this->assertEquals(Http::STATUS_CREATED, $response->getStatus()); + } + + /** + * Test that creation succeeds when user is in one of the allowed groups + */ + public function testCreateSucceedsWhenUserInAllowedGroup() { + // Mock current user + $currentUser = $this->createMock(IUser::class); + $currentUser->method('getUID') + ->willReturn('test_user'); + + $this->userSession->method('getUser') + ->willReturn($currentUser); + + $this->guestManager->method('isGuest') + ->with($currentUser) + ->willReturn(false); + + // Set up group restriction check + $this->appConfig->method('getAppValueArray') + ->with('create_restricted_to_group', []) + ->willReturn(['allowed_group1', 'allowed_group2']); + + // User is in one of the allowed groups + $this->groupManager->method('getUserGroupIds') + ->with($currentUser) + ->willReturn(['other_group', 'allowed_group2']); + + // Other necessary mocks for a successful creation + $this->mailer->method('validateMailAddress') + ->willReturn(true); + + $this->userManager->method('getByEmail') + ->willReturn([]); + + $this->userManager->method('userExists') + ->willReturn(false); + + $guestUser = $this->createMock(IUser::class); + $this->guestManager->method('createGuest') + ->willReturn($guestUser); + + $this->userManager->method('get') + ->willReturn($guestUser); + + $response = $this->controller->create('new@example.com', 'Test User', 'en', []); + $this->assertEquals(Http::STATUS_CREATED, $response->getStatus()); + } + + /** + * Test that SubAdmin can create guests when sharing is restricted to group + */ + public function testCreateSucceedsForSubAdminWhenSharingIsRestricted() { + // Mock current user + $currentUser = $this->createMock(IUser::class); + $currentUser->method('getUID') + ->willReturn('subadmin_user'); + + $group = $this->createMock(IGroup::class); + $group->method('getGID') + ->willReturn('allowed_group1'); + $this->groupManager->method('get') + ->with('allowed_group1') + ->willReturn($group); + + $this->userSession->method('getUser') + ->willReturn($currentUser); + + $this->guestManager->method('isGuest') + ->with($currentUser) + ->willReturn(false); + + // No group restrictions + $this->appConfig->method('getAppValueArray') + ->with('create_restricted_to_group', []) + ->willReturn([]); + + // Sharing is restricted to group + $this->config->method('getAppValue') + ->with('core', 'shareapi_only_share_with_group_members', 'no') + ->willReturn('yes'); + + // User is a subadmin + $this->subAdmin->method('isSubAdmin') + ->with($currentUser) + ->willReturn(true); + $this->subAdmin->method('isSubAdminOfGroup') + ->with($currentUser, $group) + ->willReturn(true); + + // Other necessary mocks for a successful creation + $this->mailer->method('validateMailAddress') + ->willReturn(true); + + $this->userManager->method('getByEmail') + ->willReturn([]); + + $this->userManager->method('userExists') + ->willReturn(false); + + $guestUser = $this->createMock(IUser::class); + $this->guestManager->method('createGuest') + ->willReturn($guestUser); + + $this->userManager->method('get') + ->willReturn($guestUser); + + $this->assertTrue($this->guestsConfig->canCreateGuests()); + $this->assertTrue($this->guestsConfig->isSharingRestrictedToGroup()); + + $response = $this->controller->create('new@example.com', 'Test User', 'en', ['allowed_group1']); + $this->assertEquals(Http::STATUS_CREATED, $response->getStatus()); + } + + /** + * Test that non-SubAdmin cannot create guests when sharing is restricted to group + */ + public function testCreateFailsForNonSubAdminWhenSharingIsRestricted() { + // Mock current user + $currentUser = $this->createMock(IUser::class); + $currentUser->method('getUID') + ->willReturn('regular_user'); + + $this->userSession->method('getUser') + ->willReturn($currentUser); + + $this->guestManager->method('isGuest') + ->with($currentUser) + ->willReturn(false); + + // No group restrictions + $this->appConfig->method('getAppValueArray') + ->with('create_restricted_to_group', []) + ->willReturn([]); + + // Sharing is restricted to group + $this->config->method('getAppValue') + ->with('core', 'shareapi_only_share_with_group_members', 'no') + ->willReturn('yes'); + + // User is NOT a subadmin + $this->subAdmin->method('isSubAdmin') + ->with($currentUser) + ->willReturn(false); + + $response = $this->controller->create('test@example.com', 'Test User', 'en', []); + $this->assertEquals(Http::STATUS_FORBIDDEN, $response->getStatus()); + $this->assertEquals(['errorMessages' => ['You are not allowed to create guests']], $response->getData()); + } + + /** + * Test that creation succeeds when there are no restrictions + */ + public function testCreateSucceedsWhenNoRestrictions() { + // Mock current user + $currentUser = $this->createMock(IUser::class); + $currentUser->method('getUID') + ->willReturn('regular_user'); + + $this->userSession->method('getUser') + ->willReturn($currentUser); + + $this->guestManager->method('isGuest') + ->with($currentUser) + ->willReturn(false); + + // No group restrictions + $this->appConfig->method('getAppValueArray') + ->with('create_restricted_to_group', []) + ->willReturn([]); + + // Sharing is restricted to group + $this->config->method('getAppValue') + ->with('core', 'shareapi_only_share_with_group_members', 'no') + ->willReturn('no'); + + // Other necessary mocks for a successful creation + $this->mailer->method('validateMailAddress') + ->willReturn(true); + + $this->userManager->method('getByEmail') + ->willReturn([]); + + $this->userManager->method('userExists') + ->willReturn(false); + + $guestUser = $this->createMock(IUser::class); + $this->guestManager->method('createGuest') + ->willReturn($guestUser); + + $this->userManager->method('get') + ->willReturn($guestUser); + + $response = $this->controller->create('new@example.com', 'Test User', 'en', []); + $this->assertEquals(Http::STATUS_CREATED, $response->getStatus()); + } + + /** + * Test that creation fails when sharing is restricted to group but no groups are provided + */ + public function testCreateWhenSharingRestrictedToGroupButNoGroups() { + $currentUser = $this->createMock(IUser::class); + $currentUser->method('getUID') + ->willReturn('admin_user'); + $this->userSession->method('getUser') + ->willReturn($currentUser); + + $this->guestManager->method('isGuest') + ->with($currentUser) + ->willReturn(false); + + // Groups restriction is empty + $this->appConfig->method('getAppValueArray') + ->with('create_restricted_to_group', []) + ->willReturn([]); + + // Sharing is restricted to group + $this->config->method('getAppValue') + ->with('core', 'shareapi_only_share_with_group_members', 'no') + ->willReturn('yes'); + + // User is an admin + // so they are allowed to create guests + $this->groupManager->method('isAdmin') + ->with('admin_user') + ->willReturn(true); + + $this->assertTrue($this->guestsConfig->canCreateGuests()); + $this->assertTrue($this->guestsConfig->isSharingRestrictedToGroup()); + + $response = $this->controller->create('test@example.com', 'Test User', 'en', []); + $this->assertEquals(Http::STATUS_FORBIDDEN, $response->getStatus()); + $this->assertEquals(['errorMessages' => ['Guest user must be added to at least one group']], $response->getData()); + } + + /** + * Test that creation fails when group does not exist + */ + public function testCreateWithNonExistentGroup() { + $currentUser = $this->createMock(IUser::class); + $currentUser->method('getUID') + ->willReturn('admin_user'); + $this->userSession->method('getUser') + ->willReturn($currentUser); + + $this->guestManager->method('isGuest') + ->with($currentUser) + ->willReturn(false); + + // User is an admin + // so they are allowed to create guests + $this->groupManager->method('isAdmin') + ->with('admin_user') + ->willReturn(true); + + // Sharing is restricted to group + $this->config->method('getAppValue') + ->with('core', 'shareapi_only_share_with_group_members', 'no') + ->willReturn('yes'); + + $this->assertTrue($this->guestsConfig->canCreateGuests()); + $this->assertTrue($this->guestsConfig->isSharingRestrictedToGroup()); + + $this->groupManager->method('get') + ->with('nonexistent') + ->willReturn(null); + + $response = $this->controller->create('test@example.com', 'Test User', 'en', ['nonexistent']); + $this->assertEquals(Http::STATUS_BAD_REQUEST, $response->getStatus()); + $this->assertEquals(['errorMessages' => ['Group nonexistent not found']], $response->getData()); + } + + /** + * Test that creation fails when user is subadmin, but is not a subadmin of the group + */ + public function testCreateWithGroupButNotSubAdmin() { + $currentUser = $this->createMock(IUser::class); + $currentUser->method('getUID') + ->willReturn('current_user'); + $this->userSession->method('getUser') + ->willReturn($currentUser); + + $this->guestManager->method('isGuest') + ->with($currentUser) + ->willReturn(false); + + $group = $this->createMock(IGroup::class); + $this->groupManager->method('get') + ->with('testgroup') + ->willReturn($group); + + $this->subAdmin->method('isSubAdmin') + ->with($currentUser) + ->willReturn(true); + + $this->subAdmin->method('isSubAdminOfGroup') + ->with($currentUser, $group) + ->willReturn(false); + + $this->groupManager->method('isAdmin') + ->with('current_user') + ->willReturn(false); + + // Sharing is restricted to group + $this->config->method('getAppValue') + ->with('core', 'shareapi_only_share_with_group_members', 'no') + ->willReturn('yes'); + + // There is no group restriction in place + $this->appConfig->method('getAppValueArray') + ->with('create_restricted_to_group', []) + ->willReturn([]); + + $this->assertTrue($this->guestsConfig->canCreateGuests()); + $this->assertTrue($this->guestsConfig->isSharingRestrictedToGroup()); + + $response = $this->controller->create('test@example.com', 'Test User', 'en', ['testgroup']); + $this->assertEquals(Http::STATUS_FORBIDDEN, $response->getStatus()); + $this->assertEquals(['errorMessages' => ['You are not allowed to add users to group testgroup']], $response->getData()); + } + + /** + * Test that creation fails when email is invalid + */ + public function testCreateWithInvalidEmail() { + $currentUser = $this->createMock(IUser::class); + $currentUser->method('getUID') + ->willReturn('current_user'); + $this->userSession->method('getUser') + ->willReturn($currentUser); + + $this->guestManager->method('isGuest') + ->with($currentUser) + ->willReturn(false); + + $this->mailer->method('validateMailAddress') + ->with('invalid_email') + ->willReturn(false); + + $this->userManager->method('getByEmail') + ->with('invalid_email') + ->willReturn([]); + + $this->assertTrue($this->guestsConfig->canCreateGuests()); + + $response = $this->controller->create('invalid_email', 'Test User', 'en', []); + $this->assertEquals(Http::STATUS_UNPROCESSABLE_ENTITY, $response->getStatus()); + $this->assertEquals(['errorMessages' => ['email' => 'Invalid mail address']], $response->getData()); + } + + /** + * Test that creation fails when user with the same email already exists + */ + public function testCreateWithExistingEmailUser() { + $currentUser = $this->createMock(IUser::class); + $currentUser->method('getUID') + ->willReturn('current_user'); + $this->userSession->method('getUser') + ->willReturn($currentUser); + + $this->guestManager->method('isGuest') + ->with($currentUser) + ->willReturn(false); + + $this->mailer->method('validateMailAddress') + ->with('existing@example.com') + ->willReturn(true); + + $existingUser = $this->createMock(IUser::class); + $this->userManager->method('getByEmail') + ->with('existing@example.com') + ->willReturn([$existingUser]); + + $this->assertTrue($this->guestsConfig->canCreateGuests()); + + $response = $this->controller->create('existing@example.com', 'Test User', 'en', []); + $this->assertEquals(Http::STATUS_UNPROCESSABLE_ENTITY, $response->getStatus()); + $this->assertEquals(['errorMessages' => ['email' => 'A user with that email already exists.']], $response->getData()); + } + + /** + * Test that creation fails when user with the same username already exists + */ + public function testCreateWithExistingUsernameUser() { + $currentUser = $this->createMock(IUser::class); + $currentUser->method('getUID') + ->willReturn('current_user'); + $this->userSession->method('getUser') + ->willReturn($currentUser); + + $this->guestManager->method('isGuest') + ->with($currentUser) + ->willReturn(false); + + $this->mailer->method('validateMailAddress') + ->with('existing@example.com') + ->willReturn(true); + + $this->userManager->method('getByEmail') + ->with('existing@example.com') + ->willReturn([]); + + $this->userManager->method('userExists') + ->with('existing@example.com') + ->willReturn(true); + + $this->assertTrue($this->guestsConfig->canCreateGuests()); + + $response = $this->controller->create('existing@example.com', 'Test User', 'en', []); + $this->assertEquals(Http::STATUS_UNPROCESSABLE_ENTITY, $response->getStatus()); + $this->assertEquals(['errorMessages' => ['username' => 'A user with that name already exists.']], $response->getData()); + } + + /** + * Test that creation succeeds when user is a subadmin of groups + * the new guest is added to. But not a member of the allowed groups. + * Enforcing group restrictions won't have any effect + * since the user is a subadmin and can create guests anyway. + */ + public function testCreateSuccessWithGroupsAsSubadmin() { + $currentUser = $this->createMock(IUser::class); + $currentUser->method('getUID') + ->willReturn('current_user'); + $this->userSession->method('getUser') + ->willReturn($currentUser); + + $this->guestManager->method('isGuest') + ->with($currentUser) + ->willReturn(false); + + $group1 = $this->createMock(IGroup::class); + $group2 = $this->createMock(IGroup::class); + $this->groupManager->method('get') + ->willReturnMap([ + ['group1', $group1], + ['group2', $group2], + ]); + + $this->subAdmin->method('isSubAdmin') + ->willReturn(true); + $this->subAdmin->method('isSubAdminOfGroup') + ->with($currentUser, $group1) + ->willReturn(true); + $this->subAdmin->method('isSubAdminOfGroup') + ->with($currentUser, $group2) + ->willReturn(true); + + $this->mailer->method('validateMailAddress') + ->with('new@example.com') + ->willReturn(true); + + $this->userManager->method('getByEmail') + ->with('new@example.com') + ->willReturn([]); + + $this->userManager->method('userExists') + ->with('new@example.com') + ->willReturn(false); + + $guestUser = $this->createMock(IUser::class); + $this->userManager->method('get') + ->with('new@example.com') + ->willReturn($guestUser); + + $this->guestManager->expects($this->once()) + ->method('createGuest') + ->with($currentUser, 'new@example.com', 'new@example.com', 'Test User', 'en') + ->willReturn($guestUser); + + $group1->expects($this->once()) + ->method('addUser') + ->with($guestUser); + $group2->expects($this->once()) + ->method('addUser') + ->with($guestUser); + + // Enable restrictions + $this->appConfig->method('getAppValueArray') + ->with('create_restricted_to_group', []) + ->willReturn(['group1', 'group2']); + + // User's groups don't intersect with allowed groups + $this->groupManager->method('getUserGroupIds') + ->with($currentUser) + ->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->assertTrue($this->guestsConfig->canCreateGuests()); + $this->assertTrue($this->guestsConfig->isSharingRestrictedToGroup()); + + $response = $this->controller->create('new@example.com', 'Test User', 'en', ['group1', 'group2']); + $this->assertEquals(Http::STATUS_CREATED, $response->getStatus()); + $this->assertEquals(['message' => 'User successfully created'], $response->getData()); + } + + /** + * Test that creation fails when an exception is thrown. + * This is a generic test to ensure that the controller + * handles exceptions correctly. + */ + public function testCreateException() { + $currentUser = $this->createMock(IUser::class); + $currentUser->method('getUID') + ->willReturn('current_user'); + $this->userSession->method('getUser') + ->willReturn($currentUser); + + $this->guestManager->method('isGuest') + ->with($currentUser) + ->willReturn(false); + + $this->mailer->method('validateMailAddress') + ->with('new@example.com') + ->willReturn(true); + + $this->userManager->method('getByEmail') + ->with('new@example.com') + ->willReturn([]); + + $this->userManager->method('userExists') + ->with('new@example.com') + ->willReturn(false); + + $this->guestManager->expects($this->once()) + ->method('createGuest') + ->with($currentUser, 'new@example.com', 'new@example.com', 'Test User', 'en') + ->willThrowException(new \Exception('Error creating guest')); + + $this->assertTrue($this->guestsConfig->canCreateGuests()); + + $response = $this->controller->create('new@example.com', 'Test User', 'en', []); + $this->assertEquals(Http::STATUS_UNPROCESSABLE_ENTITY, $response->getStatus()); + $this->assertEquals(['errorMessages' => ['email' => 'Error creating guest']], $response->getData()); + } +} diff --git a/tests/unit/GuestManagerTest.php b/tests/unit/GuestManagerTest.php index 8c272c3e..696958a1 100644 --- a/tests/unit/GuestManagerTest.php +++ b/tests/unit/GuestManagerTest.php @@ -159,7 +159,7 @@ public function testCreateGuest() { $guestUser->expects($this->once()) ->method('setDisplayName') ->with('Example Guest'); - $guestUser->method('setEMailAddress') + $guestUser->method('setSystemEMailAddress') ->willReturnCallback(function ($email) { $this->config->setUserValue('guest@example.com', 'settings', 'email', $email); });