Skip to content

Commit f56acad

Browse files
KiaraGrouwstraKiaraGrouwstra
committed
feat(groups): transitive group-in-group membership
Add a group_group edge table maintained by OC\Group\Database, an internal INestedGroupBackend capability interface, and BFS-based transitive closure in OC\Group\Manager with per-request memoization and batched backend queries. Public API gains getUserEffectiveGroupIds, addSubGroup/removeSubGroup, getDirectChildGroupIds/getDirectParentGroupIds, and getGroupEffectiveDescendantIds/getGroupEffectiveAncestorIds. Cycles are rejected inside a serialized transaction. SubGroupAdded/SubGroupRemovedEvent are dispatched along with per-user UserAdded/UserRemovedEvent (bounded by MAX_SYNTHESIZED_USER_EVENTS) so listeners such as apps/encryption stay consistent when nesting shifts the effective recipient set of a group share. See lib/private/Group/NESTED_GROUPS.md for caveats (encryption re-keying cap, LDAP enumeration cost, delete-middle-group semantics). Refs #36150. Signed-off-by: Kiara Grouwstra <cinereal@riseup.net>
1 parent 6afd7bf commit f56acad

14 files changed

Lines changed: 1530 additions & 1 deletion

‎core/Migrations/Version34000Date20260410120000.php‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OC\Core\Migrations;
11+
12+
use Closure;
13+
use OCP\DB\ISchemaWrapper;
14+
use OCP\Migration\Attributes\CreateTable;
15+
use OCP\Migration\IOutput;
16+
use OCP\Migration\SimpleMigrationStep;
17+
use Override;
18+
19+
/**
20+
* Introduce tables storing nested-group relationships:
21+
* - group_group(parent_gid, child_gid) — group G has subgroup H
22+
* - group_group_admin(admin_gid, gid) — group H administers group G
23+
*/
24+
#[CreateTable(table: 'group_group', description: 'Parent/child edges for nested group membership')]
25+
#[CreateTable(table: 'group_group_admin', description: 'Group-level sub-admin assignments')]
26+
class Version34000Date20260410120000 extends SimpleMigrationStep {
27+
#[Override]
28+
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
29+
/** @var ISchemaWrapper $schema */
30+
$schema = $schemaClosure();
31+
$changed = false;
32+
33+
if (!$schema->hasTable('group_group')) {
34+
$table = $schema->createTable('group_group');
35+
$table->addColumn('parent_gid', 'string', [
36+
'notnull' => true,
37+
'length' => 64,
38+
]);
39+
$table->addColumn('child_gid', 'string', [
40+
'notnull' => true,
41+
'length' => 64,
42+
]);
43+
$table->setPrimaryKey(['parent_gid', 'child_gid']);
44+
$table->addIndex(['child_gid'], 'gg_child_idx');
45+
$changed = true;
46+
}
47+
48+
if (!$schema->hasTable('group_group_admin')) {
49+
$table = $schema->createTable('group_group_admin');
50+
$table->addColumn('admin_gid', 'string', [
51+
'notnull' => true,
52+
'length' => 64,
53+
]);
54+
$table->addColumn('gid', 'string', [
55+
'notnull' => true,
56+
'length' => 64,
57+
]);
58+
$table->setPrimaryKey(['admin_gid', 'gid']);
59+
$table->addIndex(['gid'], 'gga_gid_idx');
60+
$changed = true;
61+
}
62+
63+
return $changed ? $schema : null;
64+
}
65+
}

‎lib/composer/composer/autoload_classmap.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -598,8 +598,13 @@
598598
'OCP\\Group\\Events\\GroupDeletedEvent' => $baseDir . '/lib/public/Group/Events/GroupDeletedEvent.php',
599599
'OCP\\Group\\Events\\SubAdminAddedEvent' => $baseDir . '/lib/public/Group/Events/SubAdminAddedEvent.php',
600600
'OCP\\Group\\Events\\SubAdminRemovedEvent' => $baseDir . '/lib/public/Group/Events/SubAdminRemovedEvent.php',
601+
'OCP\\Group\\Events\\SubGroupAddedEvent' => $baseDir . '/lib/public/Group/Events/SubGroupAddedEvent.php',
602+
'OCP\\Group\\Events\\SubGroupRemovedEvent' => $baseDir . '/lib/public/Group/Events/SubGroupRemovedEvent.php',
601603
'OCP\\Group\\Events\\UserAddedEvent' => $baseDir . '/lib/public/Group/Events/UserAddedEvent.php',
602604
'OCP\\Group\\Events\\UserRemovedEvent' => $baseDir . '/lib/public/Group/Events/UserRemovedEvent.php',
605+
'OCP\\Group\\Exception\\CycleDetectedException' => $baseDir . '/lib/public/Group/Exception/CycleDetectedException.php',
606+
'OCP\\Group\\Exception\\NestedGroupsNotSupportedException' => $baseDir . '/lib/public/Group/Exception/NestedGroupsNotSupportedException.php',
607+
'OCP\\Group\\INestedGroupBackend' => $baseDir . '/lib/public/Group/INestedGroupBackend.php',
603608
'OCP\\Group\\ISubAdmin' => $baseDir . '/lib/public/Group/ISubAdmin.php',
604609
'OCP\\HintException' => $baseDir . '/lib/public/HintException.php',
605610
'OCP\\Http\\Client\\IClient' => $baseDir . '/lib/public/Http/Client/IClient.php',
@@ -1597,6 +1602,7 @@
15971602
'OC\\Core\\Migrations\\Version33000Date20251209123503' => $baseDir . '/core/Migrations/Version33000Date20251209123503.php',
15981603
'OC\\Core\\Migrations\\Version33000Date20260126120000' => $baseDir . '/core/Migrations/Version33000Date20260126120000.php',
15991604
'OC\\Core\\Migrations\\Version34000Date20260318095645' => $baseDir . '/core/Migrations/Version34000Date20260318095645.php',
1605+
'OC\\Core\\Migrations\\Version34000Date20260410120000' => $baseDir . '/core/Migrations/Version34000Date20260410120000.php',
16001606
'OC\\Core\\Migrations\\Version34000Date20260415161745' => $baseDir . '/core/Migrations/Version34000Date20260415161745.php',
16011607
'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php',
16021608
'OC\\Core\\ResponseDefinitions' => $baseDir . '/core/ResponseDefinitions.php',

‎lib/composer/composer/autoload_static.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -639,8 +639,13 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
639639
'OCP\\Group\\Events\\GroupDeletedEvent' => __DIR__ . '/../../..' . '/lib/public/Group/Events/GroupDeletedEvent.php',
640640
'OCP\\Group\\Events\\SubAdminAddedEvent' => __DIR__ . '/../../..' . '/lib/public/Group/Events/SubAdminAddedEvent.php',
641641
'OCP\\Group\\Events\\SubAdminRemovedEvent' => __DIR__ . '/../../..' . '/lib/public/Group/Events/SubAdminRemovedEvent.php',
642+
'OCP\\Group\\Events\\SubGroupAddedEvent' => __DIR__ . '/../../..' . '/lib/public/Group/Events/SubGroupAddedEvent.php',
643+
'OCP\\Group\\Events\\SubGroupRemovedEvent' => __DIR__ . '/../../..' . '/lib/public/Group/Events/SubGroupRemovedEvent.php',
642644
'OCP\\Group\\Events\\UserAddedEvent' => __DIR__ . '/../../..' . '/lib/public/Group/Events/UserAddedEvent.php',
643645
'OCP\\Group\\Events\\UserRemovedEvent' => __DIR__ . '/../../..' . '/lib/public/Group/Events/UserRemovedEvent.php',
646+
'OCP\\Group\\Exception\\CycleDetectedException' => __DIR__ . '/../../..' . '/lib/public/Group/Exception/CycleDetectedException.php',
647+
'OCP\\Group\\Exception\\NestedGroupsNotSupportedException' => __DIR__ . '/../../..' . '/lib/public/Group/Exception/NestedGroupsNotSupportedException.php',
648+
'OCP\\Group\\INestedGroupBackend' => __DIR__ . '/../../..' . '/lib/public/Group/INestedGroupBackend.php',
644649
'OCP\\Group\\ISubAdmin' => __DIR__ . '/../../..' . '/lib/public/Group/ISubAdmin.php',
645650
'OCP\\HintException' => __DIR__ . '/../../..' . '/lib/public/HintException.php',
646651
'OCP\\Http\\Client\\IClient' => __DIR__ . '/../../..' . '/lib/public/Http/Client/IClient.php',
@@ -1638,6 +1643,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
16381643
'OC\\Core\\Migrations\\Version33000Date20251209123503' => __DIR__ . '/../../..' . '/core/Migrations/Version33000Date20251209123503.php',
16391644
'OC\\Core\\Migrations\\Version33000Date20260126120000' => __DIR__ . '/../../..' . '/core/Migrations/Version33000Date20260126120000.php',
16401645
'OC\\Core\\Migrations\\Version34000Date20260318095645' => __DIR__ . '/../../..' . '/core/Migrations/Version34000Date20260318095645.php',
1646+
'OC\\Core\\Migrations\\Version34000Date20260410120000' => __DIR__ . '/../../..' . '/core/Migrations/Version34000Date20260410120000.php',
16411647
'OC\\Core\\Migrations\\Version34000Date20260415161745' => __DIR__ . '/../../..' . '/core/Migrations/Version34000Date20260415161745.php',
16421648
'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php',
16431649
'OC\\Core\\ResponseDefinitions' => __DIR__ . '/../../..' . '/core/ResponseDefinitions.php',

‎lib/private/Group/Database.php‎

Lines changed: 213 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
use OCP\Group\Backend\IRemoveFromGroupBackend;
2424
use OCP\Group\Backend\ISearchableGroupBackend;
2525
use OCP\Group\Backend\ISetDisplayNameBackend;
26+
use OCP\Group\Exception\CycleDetectedException;
27+
use OCP\Group\INestedGroupBackend;
2628
use OCP\IDBConnection;
2729
use OCP\IUserManager;
2830
use OCP\Server;
@@ -42,7 +44,8 @@ class Database extends ABackend implements
4244
ISetDisplayNameBackend,
4345
ISearchableGroupBackend,
4446
IBatchMethodsBackend,
45-
INamedBackend {
47+
INamedBackend,
48+
INestedGroupBackend {
4649
/** @var array<string, array{gid: string, displayname: string}> */
4750
private $groupCache = [];
4851

@@ -121,6 +124,24 @@ public function deleteGroup(string $gid): bool {
121124
->where($qb->expr()->eq('gid', $qb->createNamedParameter($gid)))
122125
->executeStatement();
123126

127+
// Delete nested-group edges where this group appears on either side
128+
$qb = $this->dbConn->getQueryBuilder();
129+
$qb->delete('group_group')
130+
->where($qb->expr()->orX(
131+
$qb->expr()->eq('parent_gid', $qb->createNamedParameter($gid)),
132+
$qb->expr()->eq('child_gid', $qb->createNamedParameter($gid)),
133+
))
134+
->executeStatement();
135+
136+
// Delete group-level sub-admin edges on either side
137+
$qb = $this->dbConn->getQueryBuilder();
138+
$qb->delete('group_group_admin')
139+
->where($qb->expr()->orX(
140+
$qb->expr()->eq('admin_gid', $qb->createNamedParameter($gid)),
141+
$qb->expr()->eq('gid', $qb->createNamedParameter($gid)),
142+
))
143+
->executeStatement();
144+
124145
// Delete from cache
125146
unset($this->groupCache[$gid]);
126147

@@ -595,4 +616,195 @@ private function computeGid(string $displayName): string {
595616
? hash('sha256', $displayName)
596617
: $displayName;
597618
}
619+
620+
public function addGroupToGroup(string $childGid, string $parentGid): bool {
621+
$this->fixDI();
622+
623+
if ($childGid === $parentGid) {
624+
throw new CycleDetectedException('A group cannot be a subgroup of itself');
625+
}
626+
627+
// Serialize the cycle check and insert to close the TOCTOU window
628+
// between "is there already a path back to parent?" and "insert the edge".
629+
// Concurrent writers on the same backend will contend on this transaction.
630+
$this->dbConn->beginTransaction();
631+
try {
632+
// Reject if the edge would introduce a cycle: if $parent is already
633+
// reachable as a descendant of $child, adding parent -> child forms a loop.
634+
if ($this->isDescendantOf($parentGid, $childGid)) {
635+
$this->dbConn->rollBack();
636+
throw new CycleDetectedException(
637+
"Adding group '$childGid' under '$parentGid' would introduce a cycle"
638+
);
639+
}
640+
641+
if ($this->groupInGroup($childGid, $parentGid)) {
642+
$this->dbConn->rollBack();
643+
return false;
644+
}
645+
646+
try {
647+
$qb = $this->dbConn->getQueryBuilder();
648+
$qb->insert('group_group')
649+
->setValue('parent_gid', $qb->createNamedParameter($parentGid))
650+
->setValue('child_gid', $qb->createNamedParameter($childGid))
651+
->executeStatement();
652+
} catch (Exception $e) {
653+
$this->dbConn->rollBack();
654+
if ($e->getReason() === Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
655+
return false;
656+
}
657+
throw $e;
658+
}
659+
$this->dbConn->commit();
660+
} catch (CycleDetectedException $e) {
661+
// rollBack already called above
662+
throw $e;
663+
} catch (\Throwable $e) {
664+
if ($this->dbConn->inTransaction()) {
665+
$this->dbConn->rollBack();
666+
}
667+
throw $e;
668+
}
669+
return true;
670+
}
671+
672+
public function removeGroupFromGroup(string $childGid, string $parentGid): bool {
673+
$this->fixDI();
674+
675+
$qb = $this->dbConn->getQueryBuilder();
676+
$affected = $qb->delete('group_group')
677+
->where($qb->expr()->eq('parent_gid', $qb->createNamedParameter($parentGid)))
678+
->andWhere($qb->expr()->eq('child_gid', $qb->createNamedParameter($childGid)))
679+
->executeStatement();
680+
681+
return $affected > 0;
682+
}
683+
684+
public function getChildGroups(string $parentGid): array {
685+
$this->fixDI();
686+
687+
$qb = $this->dbConn->getQueryBuilder();
688+
$result = $qb->select('child_gid')
689+
->from('group_group')
690+
->where($qb->expr()->eq('parent_gid', $qb->createNamedParameter($parentGid)))
691+
->executeQuery();
692+
693+
$gids = [];
694+
while ($row = $result->fetch()) {
695+
$gids[] = $row['child_gid'];
696+
}
697+
$result->closeCursor();
698+
return $gids;
699+
}
700+
701+
public function getChildGroupsBatch(array $parentGids): array {
702+
if ($parentGids === []) {
703+
return [];
704+
}
705+
$this->fixDI();
706+
$result = [];
707+
foreach ($parentGids as $gid) {
708+
$result[$gid] = [];
709+
}
710+
711+
$qb = $this->dbConn->getQueryBuilder();
712+
$cursor = $qb->select('parent_gid', 'child_gid')
713+
->from('group_group')
714+
->where($qb->expr()->in(
715+
'parent_gid',
716+
$qb->createNamedParameter($parentGids, IQueryBuilder::PARAM_STR_ARRAY)
717+
))
718+
->executeQuery();
719+
while ($row = $cursor->fetch()) {
720+
$result[$row['parent_gid']][] = $row['child_gid'];
721+
}
722+
$cursor->closeCursor();
723+
return $result;
724+
}
725+
726+
public function getParentGroups(string $childGid): array {
727+
$this->fixDI();
728+
729+
$qb = $this->dbConn->getQueryBuilder();
730+
$result = $qb->select('parent_gid')
731+
->from('group_group')
732+
->where($qb->expr()->eq('child_gid', $qb->createNamedParameter($childGid)))
733+
->executeQuery();
734+
735+
$gids = [];
736+
while ($row = $result->fetch()) {
737+
$gids[] = $row['parent_gid'];
738+
}
739+
$result->closeCursor();
740+
return $gids;
741+
}
742+
743+
public function getParentGroupsBatch(array $childGids): array {
744+
if ($childGids === []) {
745+
return [];
746+
}
747+
$this->fixDI();
748+
$result = [];
749+
foreach ($childGids as $gid) {
750+
$result[$gid] = [];
751+
}
752+
753+
$qb = $this->dbConn->getQueryBuilder();
754+
$cursor = $qb->select('parent_gid', 'child_gid')
755+
->from('group_group')
756+
->where($qb->expr()->in(
757+
'child_gid',
758+
$qb->createNamedParameter($childGids, IQueryBuilder::PARAM_STR_ARRAY)
759+
))
760+
->executeQuery();
761+
while ($row = $cursor->fetch()) {
762+
$result[$row['child_gid']][] = $row['parent_gid'];
763+
}
764+
$cursor->closeCursor();
765+
return $result;
766+
}
767+
768+
public function groupInGroup(string $childGid, string $parentGid): bool {
769+
$this->fixDI();
770+
771+
$qb = $this->dbConn->getQueryBuilder();
772+
$result = $qb->select('parent_gid')
773+
->from('group_group')
774+
->where($qb->expr()->eq('parent_gid', $qb->createNamedParameter($parentGid)))
775+
->andWhere($qb->expr()->eq('child_gid', $qb->createNamedParameter($childGid)))
776+
->setMaxResults(1)
777+
->executeQuery();
778+
779+
$row = $result->fetch();
780+
$result->closeCursor();
781+
return $row !== false;
782+
}
783+
784+
/**
785+
* BFS: is $candidate reachable from $root by following parent -> child edges?
786+
*/
787+
private function isDescendantOf(string $candidate, string $root): bool {
788+
if ($candidate === $root) {
789+
return true;
790+
}
791+
$visited = [$root => true];
792+
$frontier = [$root];
793+
while ($frontier !== []) {
794+
$children = [];
795+
foreach ($frontier as $gid) {
796+
foreach ($this->getChildGroups($gid) as $child) {
797+
if ($child === $candidate) {
798+
return true;
799+
}
800+
if (!isset($visited[$child])) {
801+
$visited[$child] = true;
802+
$children[] = $child;
803+
}
804+
}
805+
}
806+
$frontier = $children;
807+
}
808+
return false;
809+
}
598810
}

0 commit comments

Comments
 (0)
⚡