map original gid by saml and avoid collissions

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>

lib/GroupBackend.php

@@ -7,7 +7,7 @@ use OCP\Group\Backend\ABackend;
 use OCP\Group\Backend\ICreateGroupBackend;
 use OCP\IDBConnection;
 
-class GroupBackend extends ABackend implements ICreateGroupBackend {
+class GroupBackend extends ABackend {
 	/** @var IDBConnection */
 	private $dbc;
 
@@ -106,6 +106,21 @@ class GroupBackend extends ABackend implements ICreateGroupBackend {
 		return false;
 	}
 
+	public function groupExistsWithDifferentGid($samlGid): ?string {
+		$qb = $this->dbc->getQueryBuilder();
+		$cursor = $qb->select('gid')
+			->from(self::TABLE_GROUPS)
+			->where($qb->expr()->eq('saml_gid', $qb->createNamedParameter($samlGid)))
+			->execute();
+		$result = $cursor->fetch();
+		$cursor->closeCursor();
+
+		if ($result !== false) {
+			return $result[0];
+		}
+		return null;
+	}
+
 	/**
 	 * @return string[] User ids
 	 */
@@ -140,16 +155,16 @@ class GroupBackend extends ABackend implements ICreateGroupBackend {
 		return $users;
 	}
 
-	/**
-	 * @since 14.0.0
-	 */
-	public function createGroup(string $gid): bool {
+	public function createGroup(string $gid, string $samlGid = null): bool {
 		try {
 			// Add group
 			$builder = $this->dbc->getQueryBuilder();
+			$displayName = $samlGid ? $gid . ' (SAML)' : $gid;
+			$samlGid = $samlGid ?? $gid;
 			$result = $builder->insert(self::TABLE_GROUPS)
 				->setValue('gid', $builder->createNamedParameter($gid))
-				->setValue('displayname', $builder->createNamedParameter($gid))
+				->setValue('displayname', $builder->createNamedParameter($displayName))
+				->setValue('saml_gid', $builder->createNamedParameter($samlGid))
 				->execute();
 		} catch(UniqueConstraintViolationException $e) {
 			$result = 0;

lib/GroupManager.php

@@ -7,6 +7,7 @@ use OC\Hooks\PublicEmitter;
 use OCA\User_SAML\Jobs\MigrateGroups;
 use OCP\IConfig;
 use OCP\IDBConnection;
+use OCP\IGroup;
 use OCP\IGroupManager;
 use OCP\IUser;
 use OCP\IUserManager;
@@ -54,14 +55,24 @@ class GroupManager
 		$this->jobList = $jobList;
 	}
 
-	public function replaceGroups($uid, $saml) {
+	public function replaceGroups($uid, $samlGroups) {
 		$user = $this->userManager->get($uid);
 		if($user === null) {
 			return;
 		}
+		$this->translateGroupToIds($samlGroups);
 		$assigned = $this->groupManager->getUserGroups($uid);
-		$this->removeGroups($user, array_diff($assigned, $saml));
-		$this->addGroups($uid, array_diff($saml, $assigned));
+		$this->removeGroups($user, array_diff($assigned, $samlGroups));
+		$this->addGroups($uid, array_diff($samlGroups, $assigned));
+	}
+
+	protected function translateGroupToIds(array &$samlGroups) {
+		array_walk($samlGroups, function (&$gid){
+			$altGid = $this->ownGroupBackend->groupExistsWithDifferentGid($gid);
+			if($altGid !== null) {
+				$gid = $altGid;
+			}
+		});
 	}
 
 	public function removeGroups(IUser $user, array $groupIds) {
@@ -85,23 +96,84 @@ class GroupManager
 	}
 
 	public function addGroup(IUser $user, $gid) {
-		$group = $this->groupManager->get($gid);
-		if($group === null) {
-			if($this->groupManager instanceof PublicEmitter) {
-				$this->groupManager->emit('\OC\Group', 'preCreate', array($gid));
-			}
-			if(!$this->ownGroupBackend->createGroup($gid)) {
-				return;
-			}
-
-			$group = $this->groupManager->get($gid);
-			if($this->groupManager instanceof PublicEmitter) {
-				$this->groupManager->emit('\OC\Group', 'postCreate', array($group));
+		try {
+			$group = $this->findGroup($gid);
+		} catch (\RuntimeException $e) {
+			if($e->getCode() === 1) {
+				$group = $this->createGroupInBackend($gid);
+			} else if($e->getCode() === 2) {
+				//FIXME: probably need config flag. Previous to 17, gid was used as displayname
+				$group = $this->createGroupInBackend('__saml__' . $gid, $gid);
+			} else {
+				throw $e;
 			}
 		}
+
 		$group->addUser($user);
 	}
 
+	protected function createGroupInBackend($gid, $originalGid = null) {
+		if($this->groupManager instanceof PublicEmitter) {
+			$this->groupManager->emit('\OC\Group', 'preCreate', array($gid));
+		}
+		if(!$this->ownGroupBackend->createGroup($gid, $originalGid ?? $gid)) {
+			return;
+		}
+
+		$group = $this->groupManager->get($gid);
+		if($this->groupManager instanceof PublicEmitter) {
+			$this->groupManager->emit('\OC\Group', 'postCreate', array($group));
+		}
+
+		return $group;
+	}
+
+	protected function findGroup($gid): IGroup {
+		$migrationWhiteList = $this->config->getAppValue(
+			'user_saml',
+			GroupManager::LOCAL_GROUPS_CHECK_FOR_MIGRATION,
+			null
+		);
+		$strictBackendCheck = null === $migrationWhiteList;
+		if(!$strictBackendCheck && in_array($gid, $migrationWhiteList['groups'], true)) {
+			$group = $this->groupManager->get($gid);
+			if($group === null) {
+				//FIXME: specific Exception and/or constant error code
+				throw new \RuntimeException('Group not found', 1);
+			}
+			return $group;
+		}
+		$group = $this->groupManager->get($gid);
+		if($group === null) {
+			//FIXME: specific Exception and/or constant error code
+			throw new \RuntimeException('Group not found', 1);
+		}
+		if($this->hasSamlBackend($group)) {
+			return $group;
+		}
+
+		$altGid = $this->ownGroupBackend->groupExistsWithDifferentGid($gid);
+		if($altGid) {
+			return $this->groupManager->get($altGid);
+		}
+
+		//FIXME: specific Exception and/or constant error code
+		throw new \RuntimeException('Non-migratable duplicate found', 2);
+	}
+
+	protected function hasSamlBackend(IGroup $group): bool {
+		$reflected = new \ReflectionClass($group);
+		$backendsProperty = $reflected->getProperty('backends');
+		$backendsProperty->setAccessible(true);
+		$backends = $backendsProperty->getValue();
+		foreach ($backends as $backend) {
+			if($backend instanceof GroupBackend) {
+				return true;
+			}
+		}
+		return false;
+	}
+
 	public function evaluateGroupMigrations(array $groups) {
 		$candidateInfo = $this->config->getAppValue('user_saml', self::LOCAL_GROUPS_CHECK_FOR_MIGRATION, null);
 		if($candidateInfo === null) {

lib/Migration/Version2500Date20191008134400.php

@@ -54,7 +54,13 @@ class Version2500Date20191008134400 extends SimpleMigrationStep {
 				'length' => 255,
 				'default' => '',
 			]);
+			$table->addColumn('saml_gid', Type::STRING, [
+				'notnull' => true,
+				'length' => 64,
+				'default' => '',
+			]);
 			$table->setPrimaryKey(['gid']);
+			$table->addUniqueIndex(['saml_gid']);
 		}
 
 		if (!$schema->hasTable('user_saml_auth_token')) {