feat: allow limiting guests creation to specific groups

lib/Config.php

@@ -8,79 +8,64 @@
 
 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 {
 		return $this->config->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data') . '/' . $uid;
 	}
 
 	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,17 +76,60 @@ 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 {
 		return $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes';
 	}
 
 	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);
 	}
 }

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();
 	}
 

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
 				);

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());
 		}

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<LoadAdditionalScriptsEvent>
  */
 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');
 	}
 }

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) {