From 0e48800c688dac7f82fa7d360a19543a76d2193e Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Wed, 21 Jan 2026 11:31:28 +0100 Subject: [PATCH 1/2] feat(GroupMigration): Add dry run mode to command And fix detection of old group. Signed-off-by: Carl Schwan --- lib/Command/GroupMigrationCopyIncomplete.php | 25 ++++++++++----- lib/Service/GroupMigration.php | 33 ++++++++++++++++++-- 2 files changed, 48 insertions(+), 10 deletions(-) diff --git a/lib/Command/GroupMigrationCopyIncomplete.php b/lib/Command/GroupMigrationCopyIncomplete.php index fd3f77022..8c8714c4e 100644 --- a/lib/Command/GroupMigrationCopyIncomplete.php +++ b/lib/Command/GroupMigrationCopyIncomplete.php @@ -11,6 +11,7 @@ use OCA\User_SAML\Service\GroupMigration; use Psr\Log\LoggerInterface; use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; use Throwable; @@ -23,20 +24,30 @@ public function __construct( } #[\Override] protected function configure(): void { - $this->setName('saml:group-migration:copy-incomplete-members'); - $this->setDescription('Transfers remaining group members from old local to current SAML groups'); + $this->setName('saml:group-migration:copy-incomplete-members') + ->setDescription('Transfers remaining group members from old local to current SAML groups') + ->addOption('dry-run', null, InputOption::VALUE_NONE, 'Output the SQL queries instead of running them.'); } protected function execute(InputInterface $input, OutputInterface $output): int { + $dryRun = $input->getOption('dry-run'); + $groupsToTreat = $this->groupMigration->findGroupsWithLocalMembers(); if (empty($groupsToTreat)) { - if ($output->isVerbose()) { + if ($output->isVerbose() || $dryRun) { $output->writeln('No pending group member transfer'); } return 0; } - if (!$this->doMemberTransfer($groupsToTreat, $output)) { + if ($dryRun) { + $output->writeln('Found the following SAML group with a corresponding local group:'); + foreach ($groupsToTreat as $group) { + $output->writeln('- ' . $group . ''); + } + } + + if (!$this->doMemberTransfer($groupsToTreat, $output, $dryRun)) { if (!$output->isQuiet()) { $output->writeln('Not all group members could be transferred completely. Rerun this command or check the Nextcloud log.'); } @@ -54,16 +65,16 @@ protected function execute(InputInterface $input, OutputInterface $output): int * @param OutputInterface $output * @return bool */ - protected function doMemberTransfer(array $groups, OutputInterface $output): bool { + protected function doMemberTransfer(array $groups, OutputInterface $output, bool $dryRun): bool { $errorOccurred = false; for ($i = 0; $i < 2; $i++) { $retry = []; foreach ($groups as $gid) { try { - $isComplete = $this->groupMigration->migrateGroupUsers($gid); + $isComplete = $this->groupMigration->migrateGroupUsers($gid, $output, $dryRun); if (!$isComplete) { $retry[] = $gid; - } else { + } elseif (!$dryRun) { $this->groupMigration->cleanUpOldGroupUsers($gid); if ($output->isVerbose()) { $output->writeln(sprintf('Members transferred successfully for group %s', $gid)); diff --git a/lib/Service/GroupMigration.php b/lib/Service/GroupMigration.php index 6fc759896..f5acc75b4 100644 --- a/lib/Service/GroupMigration.php +++ b/lib/Service/GroupMigration.php @@ -8,6 +8,7 @@ namespace OCA\User_SAML\Service; use OCA\User_SAML\GroupBackend; +use OCA\User_SAML\SAMLSettings; use OCP\AppFramework\Db\TTransactional; use OCP\DB\Exception; use OCP\DB\QueryBuilder\IQueryBuilder; @@ -15,6 +16,7 @@ use OCP\IGroupManager; use OCP\IUser; use Psr\Log\LoggerInterface; +use Symfony\Component\Console\Output\OutputInterface; use Throwable; class GroupMigration { @@ -42,6 +44,15 @@ public function findGroupsWithLocalMembers(): array { ->where($qb->expr()->in('gid', $qb->createParameter('gidList'))); $allOwnedGroups = $this->ownGroupBackend->getGroups(); + + // Remove prefix from group names + $allOwnedGroups = array_merge($allOwnedGroups, array_map(function (string $groupName): string { + if (substr($groupName, 0, strlen(SAMLSettings::DEFAULT_GROUP_PREFIX)) == SAMLSettings::DEFAULT_GROUP_PREFIX) { + $groupName = substr($groupName, strlen(SAMLSettings::DEFAULT_GROUP_PREFIX)); + } + return $groupName; + }, $allOwnedGroups)); + foreach (array_chunk($allOwnedGroups, self::CHUNK_SIZE) as $groupsChunk) { $qb->setParameter('gidList', $groupsChunk, IQueryBuilder::PARAM_STR_ARRAY); $result = $qb->executeQuery(); @@ -59,19 +70,35 @@ public function findGroupsWithLocalMembers(): array { * @throws Exception * @throws Throwable */ - public function migrateGroupUsers(string $gid): bool { + public function migrateGroupUsers(string $gid, ?OutputInterface $output = null, bool $dryRun = false): bool { $originalGroup = $this->groupManager->get($gid); $members = $originalGroup?->getUsers(); + $newGid = $gid; + if (!$this->ownGroupBackend->groupExists($gid)) { + if ($this->ownGroupBackend->groupExists(SAMLSettings::DEFAULT_GROUP_PREFIX . $gid)) { + $newGid = SAMLSettings::DEFAULT_GROUP_PREFIX . $gid; + } else { + $output->writeln("SAML group corresponding to the local $gid group does not exist"); + return true; + } + } + + if ($dryRun) { + assert($output instanceof OutputInterface); + $output->writeln('Found ' . count($members) . ' members in old local group ' . $gid . ' and migrating them to ' . $newGid); + return true; + } + $areAllInserted = true; foreach (array_chunk($members ?? [], (int)floor(self::CHUNK_SIZE / 2)) as $userBatch) { - $areAllInserted = ($this->atomic(function () use ($userBatch, $gid) { + $areAllInserted = ($this->atomic(function () use ($userBatch, $newGid) { /** @var IUser $user */ foreach ($userBatch as $user) { $this->dbc->insertIgnoreConflict( GroupBackend::TABLE_MEMBERS, [ - 'gid' => $gid, + 'gid' => $newGid, 'uid' => $user->getUID(), ] ); From 8a86ac091a0c6458c49569ab07ca05341043bf62 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Wed, 21 Jan 2026 14:20:12 +0100 Subject: [PATCH 2/2] feat(GroupMigration): Allow to execute the migration only for one group Signed-off-by: Carl Schwan --- lib/Command/GroupMigrationCopyIncomplete.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/Command/GroupMigrationCopyIncomplete.php b/lib/Command/GroupMigrationCopyIncomplete.php index 8c8714c4e..c84c7e00e 100644 --- a/lib/Command/GroupMigrationCopyIncomplete.php +++ b/lib/Command/GroupMigrationCopyIncomplete.php @@ -26,13 +26,20 @@ public function __construct( protected function configure(): void { $this->setName('saml:group-migration:copy-incomplete-members') ->setDescription('Transfers remaining group members from old local to current SAML groups') - ->addOption('dry-run', null, InputOption::VALUE_NONE, 'Output the SQL queries instead of running them.'); + ->addOption('dry-run', null, InputOption::VALUE_NONE, 'Output the SQL queries instead of running them.') + ->addOption('group', null, InputOption::VALUE_OPTIONAL, 'Group filter'); } protected function execute(InputInterface $input, OutputInterface $output): int { $dryRun = $input->getOption('dry-run'); $groupsToTreat = $this->groupMigration->findGroupsWithLocalMembers(); + $group = $input->getOption('group'); + if ($group) { + $groupsToTreat = array_filter($groupsToTreat, function (string $groupToTreat) use ($group): bool { + return $groupToTreat === $group; + }); + } if (empty($groupsToTreat)) { if ($output->isVerbose() || $dryRun) { $output->writeln('No pending group member transfer'); @@ -40,6 +47,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int return 0; } + if ($dryRun) { $output->writeln('Found the following SAML group with a corresponding local group:'); foreach ($groupsToTreat as $group) {