fix(subadmin): disable group-admin feature by default behind allow_subadmins

The subadmin (group-admin) feature lets an admin delegate user management
of specific groups to a non-admin user. The related code path has known
security shortcomings, so the feature is now disabled by default as a
risk-mitigation. Deployments that rely on it can opt back in by setting
the new `allow_subadmins` system config to true.

Enforcement is centralized in the OC\SubAdmin manager, the single chokepoint
all consumers route through:
- isSubAdmin() keeps the real-admin short-circuit, then returns false for
  group-admin-only users when disabled, cascading to permission bypasses,
  legacy guards and the settings middleware.
- Read methods (isSubAdminofGroup, getSubAdminsGroups, getGroupsSubAdmins,
  getAllSubAdmins) behave as if no subadmins exist.
- createSubAdmin throws HintException; the two write callers
  (togglesubadmins.php, provisioning_api addSubAdmin) surface a clean error.
- deleteSubAdmin and the post-delete cleanup hooks stay enabled so admins
  can prune dormant assignments.

The Users settings page hides the group-admin column when disabled, and the
option is documented in config.sample.php with a security note.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com>

Author: DeepDiver1975

apps/provisioning_api/lib/Users.php

@@ -600,10 +600,14 @@ public function addSubAdmin($parameters) {
 			return new Result(null, 100);
 		}
 		// Go
-		if ($subAdminManager->createSubAdmin($user, $group)) {
-			return new Result(null, 100);
-		} else {
-			return new Result(null, 103, 'Unknown error occurred');
+		try {
+			if ($subAdminManager->createSubAdmin($user, $group)) {
+				return new Result(null, 100);
+			} else {
+				return new Result(null, 103, 'Unknown error occurred');
+			}
+		} catch (\OC\HintException $e) {
+			return new Result(null, 103, $e->getHint());
 		}
 	}
 

config/config.sample.php

@@ -218,6 +218,19 @@
  */
 'allow_user_to_change_mail_address' => true,
 
+/**
+ * Allow or disallow the group administrator (subadmin) feature
+ * `true` allows administrators to delegate user management of specific groups
+ * to non-admin users (group admins / subadmins).
+ * `false` (default) disables the feature entirely: existing group-admin
+ * assignments are ignored and no new ones can be created.
+ *
+ * SECURITY NOTE: this feature is disabled by default as a risk-mitigation.
+ * Only enable it if you require delegated group administration and understand
+ * that group admins can manage users within their groups.
+ */
+'allow_subadmins' => false,
+
 /**
  * Define the lifetime of the remember-login cookie
  * The remember-login cookie is set when the user clicks the `remember` checkbox

lib/private/Group/Manager.php

@@ -487,7 +487,8 @@ public function getSubAdmin() {
 			$this->subAdmin = new \OC\SubAdmin(
 				$this->userManager,
 				$this,
-				\OC::$server->getDatabaseConnection()
+				\OC::$server->getDatabaseConnection(),
+				\OC::$server->getConfig()
 			);
 		}
 

lib/private/SubAdmin.php

@@ -28,6 +28,7 @@
 
 use OC\Hooks\PublicEmitter;
 use OCP\Events\EventEmitterTrait;
+use OCP\IConfig;
 use OCP\IUser;
 use OCP\IUserManager;
 use OCP\IGroup;
@@ -45,21 +46,27 @@ class SubAdmin extends PublicEmitter {
 	/** @var IDBConnection */
 	private $dbConn;
 
+	/** @var IConfig */
+	private $config;
+
 	/**
 	 * @param IUserManager $userManager
 	 * @param IGroupManager $groupManager
 	 * @param IDBConnection $dbConn
+	 * @param IConfig $config
 	 */
 	public function __construct(
 		IUserManager $userManager,
 		IGroupManager $groupManager,
-		IDBConnection $dbConn
+		IDBConnection $dbConn,
+		IConfig $config
 	) {
 		'@phan-var \OC\User\Manager $userManager';
 		$this->userManager = $userManager;
 		'@phan-var \OC\Group\Manager $groupManager';
 		$this->groupManager = $groupManager;
 		$this->dbConn = $dbConn;
+		$this->config = $config;
 
 		$this->userManager->listen('\OC\User', 'postDelete', function ($user) {
 			$this->post_deleteUser($user);
@@ -69,13 +76,33 @@ public function __construct(
 		});
 	}
 
+	/**
+	 * Whether the group admin (subadmin) feature is enabled.
+	 *
+	 * Disabled by default as a security risk-mitigation; admins opt in via the
+	 * `allow_subadmins` system config.
+	 *
+	 * @return bool
+	 */
+	private function isEnabled() {
+		return $this->config->getSystemValue('allow_subadmins', false) !== false;
+	}
+
 	/**
 	 * add a SubAdmin
 	 * @param IUser $user user to be SubAdmin
 	 * @param IGroup $group group $user becomes subadmin of
 	 * @return bool
+	 * @throws \OC\HintException if the subadmin feature is disabled
 	 */
 	public function createSubAdmin(IUser $user, IGroup $group) {
+		if (!$this->isEnabled()) {
+			$l = \OC::$server->getL10N('lib');
+			throw new \OC\HintException(
+				$l->t('Group admin feature is disabled'),
+				$l->t('The group admin (subadmin) feature has been disabled by the administrator')
+			);
+		}
 		return $this->emittingCall(function () use (&$user, &$group) {
 			$qb = $this->dbConn->getQueryBuilder();
 
@@ -125,6 +152,9 @@ public function deleteSubAdmin(IUser $user, IGroup $group) {
 	 * @return IGroup[]
 	 */
 	public function getSubAdminsGroups(IUser $user) {
+		if (!$this->isEnabled()) {
+			return [];
+		}
 		$qb = $this->dbConn->getQueryBuilder();
 
 		$result = $qb->select('gid')
@@ -150,6 +180,9 @@ public function getSubAdminsGroups(IUser $user) {
 	 * @return IUser[]
 	 */
 	public function getGroupsSubAdmins(IGroup $group) {
+		if (!$this->isEnabled()) {
+			return [];
+		}
 		$qb = $this->dbConn->getQueryBuilder();
 
 		$result = $qb->select('uid')
@@ -174,6 +207,9 @@ public function getGroupsSubAdmins(IGroup $group) {
 	 * @return array
 	 */
 	public function getAllSubAdmins() {
+		if (!$this->isEnabled()) {
+			return [];
+		}
 		$qb = $this->dbConn->getQueryBuilder();
 
 		$result = $qb->select('*')
@@ -203,6 +239,9 @@ public function getAllSubAdmins() {
 	 * @return bool
 	 */
 	public function isSubAdminofGroup(IUser $user, IGroup $group) {
+		if (!$this->isEnabled()) {
+			return false;
+		}
 		$qb = $this->dbConn->getQueryBuilder();
 
 		/*
@@ -232,6 +271,12 @@ public function isSubAdmin(IUser $user) {
 			return true;
 		}
 
+		// When the feature is disabled, group-admin-only users have no
+		// elevated rights (real admins are handled by the short-circuit above)
+		if (!$this->isEnabled()) {
+			return false;
+		}
+
 		$qb = $this->dbConn->getQueryBuilder();
 
 		$result = $qb->select('gid')

settings/ajax/togglesubadmins.php

@@ -39,7 +39,12 @@
 if ($isSubAdminOfGroup) {
 	$subAdminManager->deleteSubAdmin($targetUserObject, $targetGroupObject);
 } else {
-	$subAdminManager->createSubAdmin($targetUserObject, $targetGroupObject);
+	try {
+		$subAdminManager->createSubAdmin($targetUserObject, $targetGroupObject);
+	} catch (\OC\HintException $e) {
+		OC_JSON::error(['data' => ['message' => $e->getHint()]]);
+		exit();
+	}
 }
 
 OC_JSON::success();

settings/users.php

@@ -72,7 +72,7 @@
 $recoveryAdminEnabled = OC_App::isEnabled('encryption') &&
 						$config->getAppValue('encryption', 'recoveryAdminEnabled', null);
 
-if ($isAdmin) {
+if ($isAdmin && $config->getSystemValue('allow_subadmins', false) !== false) {
 	$subAdmins = \OC::$server->getGroupManager()->getSubAdmin()->getAllSubAdmins();
 	// New class returns IUser[] so convert back
 	$result = [];

tests/acceptance/features/bootstrap/Provisioning.php

@@ -49,6 +49,12 @@ trait Provisioning {
 	private array $createdRemoteUsers = [];
 	private array $enabledApps = [];
 	private array $disabledApps = [];
+
+	/**
+	 * Whether this scenario enabled the group admin (subadmin) feature, which
+	 * is disabled by default. Used to revert the config after the scenario.
+	 */
+	private bool $subadminFeatureEnabled = false;
 	private array $startingGroups = [];
 	private array $createdRemoteGroups = [];
 	private array $createdGroups = [];
@@ -4262,6 +4268,9 @@ public function userMakesUserASubadminOfGroupUsingTheProvisioningApi(
 		string $otherUser,
 		string $group
 	):void {
+		// The subadmin feature is disabled by default; enable it so these
+		// scenarios can exercise it. Reverted in cleanupSubadminFeature().
+		$this->enableSubadminFeature();
 		$actualUser = $this->getActualUsername($user);
 		$actualPassword = $this->getUserPassword($actualUser);
 		$actualSubadminUsername = $this->getActualUsername($otherUser);
@@ -4279,6 +4288,43 @@ public function userMakesUserASubadminOfGroupUsingTheProvisioningApi(
 		);
 	}
 
+	/**
+	 * The group admin (subadmin) feature is disabled by default. Enable it so
+	 * scenarios that exercise subadmins keep working. Tracked so it can be
+	 * reverted after the scenario.
+	 *
+	 * @return void
+	 * @throws Exception
+	 */
+	public function enableSubadminFeature():void {
+		if ($this->subadminFeatureEnabled) {
+			return;
+		}
+		SetupHelper::setSystemConfig(
+			'allow_subadmins',
+			'true',
+			$this->getStepLineRef(),
+			'boolean'
+		);
+		$this->subadminFeatureEnabled = true;
+	}
+
+	/**
+	 * @AfterScenario
+	 *
+	 * @return void
+	 * @throws Exception
+	 */
+	public function cleanupSubadminFeature():void {
+		if ($this->subadminFeatureEnabled) {
+			SetupHelper::deleteSystemConfig(
+				'allow_subadmins',
+				$this->getStepLineRef()
+			);
+			$this->subadminFeatureEnabled = false;
+		}
+	}
+
 	/**
 	 * @When the administrator gets all the groups where user :user is subadmin using the provisioning API
 	 *