From 0843dda1bd1fd8f8b5be2078485f2fc5f993d568 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 2 Nov 2021 16:22:37 +0100 Subject: [PATCH 01/12] Support LDAP dns longer than 255 characters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an ldap_full_dn column to store the dn, and only store a sha256 hash in the ldap_dn which is shorter and can be indexed without trouble. Migration still needs to be implemented. Signed-off-by: Côme Chilliet --- .../user_ldap/lib/Mapping/AbstractMapping.php | 56 ++++++++++--------- .../Version1010Date20200630192842.php | 14 ++++- 2 files changed, 43 insertions(+), 27 deletions(-) diff --git a/apps/user_ldap/lib/Mapping/AbstractMapping.php b/apps/user_ldap/lib/Mapping/AbstractMapping.php index 85fc91590fb..8d1236e7348 100644 --- a/apps/user_ldap/lib/Mapping/AbstractMapping.php +++ b/apps/user_ldap/lib/Mapping/AbstractMapping.php @@ -67,6 +67,7 @@ abstract class AbstractMapping { */ public function isColNameValid($col) { switch ($col) { + case 'ldap_full_dn': case 'ldap_dn': case 'owncloud_name': case 'directory_uuid': @@ -134,7 +135,7 @@ abstract class AbstractMapping { */ public function getDNByName($name) { $dn = array_search($name, $this->cache); - if ($dn === false && ($dn = $this->getXbyY('ldap_dn', 'owncloud_name', $name)) !== false) { + if ($dn === false && ($dn = $this->getXbyY('ldap_full_dn', 'owncloud_name', $name)) !== false) { $this->cache[$dn] = $name; } return $dn; @@ -151,11 +152,11 @@ abstract class AbstractMapping { $oldDn = $this->getDnByUUID($uuid); $statement = $this->dbc->prepare(' UPDATE `' . $this->getTableName() . '` - SET `ldap_dn` = ? + SET `ldap_dn` = ?, `ldap_full_dn` = ? WHERE `directory_uuid` = ? '); - $r = $this->modify($statement, [$fdn, $uuid]); + $r = $this->modify($statement, [$this->getDNHash($fdn), $fdn, $uuid]); if ($r && is_string($oldDn) && isset($this->cache[$oldDn])) { $this->cache[$fdn] = $this->cache[$oldDn]; @@ -183,7 +184,14 @@ abstract class AbstractMapping { unset($this->cache[$fdn]); - return $this->modify($statement, [$uuid, $fdn]); + return $this->modify($statement, [$uuid, $this->getDNHash($fdn)]); + } + + /** + * Get the hash to store in database column ldap_dn for a given dn + */ + protected function getDNHash(string $fdn): string { + return (string)hash('sha256', $fdn, false); } /** @@ -194,28 +202,35 @@ abstract class AbstractMapping { */ public function getNameByDN($fdn) { if (!isset($this->cache[$fdn])) { - $this->cache[$fdn] = $this->getXbyY('owncloud_name', 'ldap_dn', $fdn); + $this->cache[$fdn] = $this->getXbyY('owncloud_name', 'ldap_dn', $this->getDNHash($fdn)); } return $this->cache[$fdn]; } - protected function prepareListOfIdsQuery(array $dnList): IQueryBuilder { + /** + * @param array $hashList + */ + protected function prepareListOfIdsQuery(array $hashList): IQueryBuilder { $qb = $this->dbc->getQueryBuilder(); - $qb->select('owncloud_name', 'ldap_dn') + $qb->select('owncloud_name', 'ldap_dn', 'ldap_full_dn') ->from($this->getTableName(false)) - ->where($qb->expr()->in('ldap_dn', $qb->createNamedParameter($dnList, QueryBuilder::PARAM_STR_ARRAY))); + ->where($qb->expr()->in('ldap_dn', $qb->createNamedParameter($hashList, QueryBuilder::PARAM_STR_ARRAY))); return $qb; } protected function collectResultsFromListOfIdsQuery(IQueryBuilder $qb, array &$results): void { $stmt = $qb->execute(); while ($entry = $stmt->fetch(\Doctrine\DBAL\FetchMode::ASSOCIATIVE)) { - $results[$entry['ldap_dn']] = $entry['owncloud_name']; - $this->cache[$entry['ldap_dn']] = $entry['owncloud_name']; + $results[$entry['ldap_full_dn']] = $entry['owncloud_name']; + $this->cache[$entry['ldap_full_dn']] = $entry['owncloud_name']; } $stmt->closeCursor(); } + /** + * @param array $fdns + * @return array + */ public function getListOfIdsByDn(array $fdns): array { $totalDBParamLimit = 65000; $sliceSize = 1000; @@ -223,6 +238,7 @@ abstract class AbstractMapping { $results = []; $slice = 1; + $fdns = array_map([$this, 'getDNHash'], $fdns); $fdnsSlice = count($fdns) > $sliceSize ? array_slice($fdns, 0, $sliceSize) : $fdns; $qb = $this->prepareListOfIdsQuery($fdnsSlice); @@ -294,7 +310,7 @@ abstract class AbstractMapping { } public function getDnByUUID($uuid) { - return $this->getXbyY('ldap_dn', 'directory_uuid', $uuid); + return $this->getXbyY('ldap_full_dn', 'directory_uuid', $uuid); } /** @@ -305,7 +321,7 @@ abstract class AbstractMapping { * @throws \Exception */ public function getUUIDByDN($dn) { - return $this->getXbyY('directory_uuid', 'ldap_dn', $dn); + return $this->getXbyY('directory_uuid', 'ldap_dn', $this->getDNHash($dn)); } /** @@ -318,7 +334,7 @@ abstract class AbstractMapping { public function getList($offset = null, $limit = null) { $query = $this->dbc->prepare(' SELECT - `ldap_dn` AS `dn`, + `ldap_full_dn` AS `dn`, `owncloud_name` AS `name`, `directory_uuid` AS `uuid` FROM `' . $this->getTableName() . '`', @@ -339,19 +355,9 @@ abstract class AbstractMapping { * @return bool */ public function map($fdn, $name, $uuid) { - if (mb_strlen($fdn) > 255) { - \OC::$server->getLogger()->error( - 'Cannot map, because the DN exceeds 255 characters: {dn}', - [ - 'app' => 'user_ldap', - 'dn' => $fdn, - ] - ); - return false; - } - $row = [ - 'ldap_dn' => $fdn, + 'ldap_dn' => $this->getDNHash($fdn), + 'ldap_full_dn' => $fdn, 'owncloud_name' => $name, 'directory_uuid' => $uuid ]; diff --git a/apps/user_ldap/lib/Migration/Version1010Date20200630192842.php b/apps/user_ldap/lib/Migration/Version1010Date20200630192842.php index e2c78ed59f8..9f0faf752a3 100644 --- a/apps/user_ldap/lib/Migration/Version1010Date20200630192842.php +++ b/apps/user_ldap/lib/Migration/Version1010Date20200630192842.php @@ -47,7 +47,12 @@ class Version1010Date20200630192842 extends SimpleMigrationStep { $table = $schema->createTable('ldap_user_mapping'); $table->addColumn('ldap_dn', Types::STRING, [ 'notnull' => true, - 'length' => 255, + 'length' => 64, + 'default' => '', + ]); + $table->addColumn('ldap_full_dn', Types::STRING, [ + 'notnull' => true, + 'length' => 4096, 'default' => '', ]); $table->addColumn('owncloud_name', Types::STRING, [ @@ -68,7 +73,12 @@ class Version1010Date20200630192842 extends SimpleMigrationStep { $table = $schema->createTable('ldap_group_mapping'); $table->addColumn('ldap_dn', Types::STRING, [ 'notnull' => true, - 'length' => 255, + 'length' => 64, + 'default' => '', + ]); + $table->addColumn('ldap_full_dn', Types::STRING, [ + 'notnull' => true, + 'length' => 4096, 'default' => '', ]); $table->addColumn('owncloud_name', Types::STRING, [ From 072897cdf8d83af46b599303fd2af882b446040d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 4 Nov 2021 12:06:59 +0100 Subject: [PATCH 02/12] Change column names to ldap_dn and ldap_dn_hash and add migration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../user_ldap/lib/Mapping/AbstractMapping.php | 34 ++--- .../Version1010Date20200630192842.php | 14 +- .../Version1130Date20211102154716.php | 139 ++++++++++++++++++ 3 files changed, 158 insertions(+), 29 deletions(-) create mode 100644 apps/user_ldap/lib/Migration/Version1130Date20211102154716.php diff --git a/apps/user_ldap/lib/Mapping/AbstractMapping.php b/apps/user_ldap/lib/Mapping/AbstractMapping.php index 8d1236e7348..a2cc278aad5 100644 --- a/apps/user_ldap/lib/Mapping/AbstractMapping.php +++ b/apps/user_ldap/lib/Mapping/AbstractMapping.php @@ -67,8 +67,8 @@ abstract class AbstractMapping { */ public function isColNameValid($col) { switch ($col) { - case 'ldap_full_dn': case 'ldap_dn': + case 'ldap_dn_hash': case 'owncloud_name': case 'directory_uuid': return true; @@ -135,7 +135,7 @@ abstract class AbstractMapping { */ public function getDNByName($name) { $dn = array_search($name, $this->cache); - if ($dn === false && ($dn = $this->getXbyY('ldap_full_dn', 'owncloud_name', $name)) !== false) { + if ($dn === false && ($dn = $this->getXbyY('ldap_dn', 'owncloud_name', $name)) !== false) { $this->cache[$dn] = $name; } return $dn; @@ -152,7 +152,7 @@ abstract class AbstractMapping { $oldDn = $this->getDnByUUID($uuid); $statement = $this->dbc->prepare(' UPDATE `' . $this->getTableName() . '` - SET `ldap_dn` = ?, `ldap_full_dn` = ? + SET `ldap_dn_hash` = ?, `ldap_dn` = ? WHERE `directory_uuid` = ? '); @@ -179,7 +179,7 @@ abstract class AbstractMapping { $statement = $this->dbc->prepare(' UPDATE `' . $this->getTableName() . '` SET `directory_uuid` = ? - WHERE `ldap_dn` = ? + WHERE `ldap_dn_hash` = ? '); unset($this->cache[$fdn]); @@ -188,7 +188,7 @@ abstract class AbstractMapping { } /** - * Get the hash to store in database column ldap_dn for a given dn + * Get the hash to store in database column ldap_dn_hash for a given dn */ protected function getDNHash(string $fdn): string { return (string)hash('sha256', $fdn, false); @@ -202,7 +202,7 @@ abstract class AbstractMapping { */ public function getNameByDN($fdn) { if (!isset($this->cache[$fdn])) { - $this->cache[$fdn] = $this->getXbyY('owncloud_name', 'ldap_dn', $this->getDNHash($fdn)); + $this->cache[$fdn] = $this->getXbyY('owncloud_name', 'ldap_dn_hash', $this->getDNHash($fdn)); } return $this->cache[$fdn]; } @@ -212,17 +212,17 @@ abstract class AbstractMapping { */ protected function prepareListOfIdsQuery(array $hashList): IQueryBuilder { $qb = $this->dbc->getQueryBuilder(); - $qb->select('owncloud_name', 'ldap_dn', 'ldap_full_dn') + $qb->select('owncloud_name', 'ldap_dn_hash', 'ldap_dn') ->from($this->getTableName(false)) - ->where($qb->expr()->in('ldap_dn', $qb->createNamedParameter($hashList, QueryBuilder::PARAM_STR_ARRAY))); + ->where($qb->expr()->in('ldap_dn_hash', $qb->createNamedParameter($hashList, QueryBuilder::PARAM_STR_ARRAY))); return $qb; } protected function collectResultsFromListOfIdsQuery(IQueryBuilder $qb, array &$results): void { $stmt = $qb->execute(); while ($entry = $stmt->fetch(\Doctrine\DBAL\FetchMode::ASSOCIATIVE)) { - $results[$entry['ldap_full_dn']] = $entry['owncloud_name']; - $this->cache[$entry['ldap_full_dn']] = $entry['owncloud_name']; + $results[$entry['ldap_dn']] = $entry['owncloud_name']; + $this->cache[$entry['ldap_dn']] = $entry['owncloud_name']; } $stmt->closeCursor(); } @@ -256,7 +256,7 @@ abstract class AbstractMapping { } if (!empty($fdnsSlice)) { - $qb->orWhere($qb->expr()->in('ldap_dn', $qb->createNamedParameter($fdnsSlice, QueryBuilder::PARAM_STR_ARRAY))); + $qb->orWhere($qb->expr()->in('ldap_dn_hash', $qb->createNamedParameter($fdnsSlice, QueryBuilder::PARAM_STR_ARRAY))); } if ($slice % $maxSlices === 0) { @@ -310,7 +310,7 @@ abstract class AbstractMapping { } public function getDnByUUID($uuid) { - return $this->getXbyY('ldap_full_dn', 'directory_uuid', $uuid); + return $this->getXbyY('ldap_dn', 'directory_uuid', $uuid); } /** @@ -321,7 +321,7 @@ abstract class AbstractMapping { * @throws \Exception */ public function getUUIDByDN($dn) { - return $this->getXbyY('directory_uuid', 'ldap_dn', $this->getDNHash($dn)); + return $this->getXbyY('directory_uuid', 'ldap_dn_hash', $this->getDNHash($dn)); } /** @@ -334,7 +334,7 @@ abstract class AbstractMapping { public function getList($offset = null, $limit = null) { $query = $this->dbc->prepare(' SELECT - `ldap_full_dn` AS `dn`, + `ldap_dn` AS `dn`, `owncloud_name` AS `name`, `directory_uuid` AS `uuid` FROM `' . $this->getTableName() . '`', @@ -356,8 +356,8 @@ abstract class AbstractMapping { */ public function map($fdn, $name, $uuid) { $row = [ - 'ldap_dn' => $this->getDNHash($fdn), - 'ldap_full_dn' => $fdn, + 'ldap_dn_hash' => $this->getDNHash($fdn), + 'ldap_dn' => $fdn, 'owncloud_name' => $name, 'directory_uuid' => $uuid ]; @@ -444,7 +444,7 @@ abstract class AbstractMapping { */ public function count() { $qb = $this->dbc->getQueryBuilder(); - $query = $qb->select($qb->func()->count('ldap_dn')) + $query = $qb->select($qb->func()->count('ldap_dn_hash')) ->from($this->getTableName()); $res = $query->execute(); $count = $res->fetchOne(); diff --git a/apps/user_ldap/lib/Migration/Version1010Date20200630192842.php b/apps/user_ldap/lib/Migration/Version1010Date20200630192842.php index 9f0faf752a3..e2c78ed59f8 100644 --- a/apps/user_ldap/lib/Migration/Version1010Date20200630192842.php +++ b/apps/user_ldap/lib/Migration/Version1010Date20200630192842.php @@ -47,12 +47,7 @@ class Version1010Date20200630192842 extends SimpleMigrationStep { $table = $schema->createTable('ldap_user_mapping'); $table->addColumn('ldap_dn', Types::STRING, [ 'notnull' => true, - 'length' => 64, - 'default' => '', - ]); - $table->addColumn('ldap_full_dn', Types::STRING, [ - 'notnull' => true, - 'length' => 4096, + 'length' => 255, 'default' => '', ]); $table->addColumn('owncloud_name', Types::STRING, [ @@ -73,12 +68,7 @@ class Version1010Date20200630192842 extends SimpleMigrationStep { $table = $schema->createTable('ldap_group_mapping'); $table->addColumn('ldap_dn', Types::STRING, [ 'notnull' => true, - 'length' => 64, - 'default' => '', - ]); - $table->addColumn('ldap_full_dn', Types::STRING, [ - 'notnull' => true, - 'length' => 4096, + 'length' => 255, 'default' => '', ]); $table->addColumn('owncloud_name', Types::STRING, [ diff --git a/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php b/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php new file mode 100644 index 00000000000..1d8ec577b9c --- /dev/null +++ b/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php @@ -0,0 +1,139 @@ +dbc = $dbc; + $this->logger = $logger; + } + + public function getName() { + return 'Adjust LDAP user and group ldap_dn column lengths and add ldap_dn_hash columns'; + } + + /** + * @param IOutput $output + * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper` + * @param array $options + * @return null|ISchemaWrapper + */ + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + + $changeSchema = false; + foreach (['ldap_user_mapping', 'ldap_group_mapping'] as $tableName) { + $table = $schema->getTable($tableName); + $column = $table->getColumn('ldap_dn_hash'); + if (!$column) { + $table->addColumn('ldap_dn_hash', Types::STRING, [ + 'notnull' => true, + 'length' => 64, + 'default' => '', + ]); + $changeSchema = true; + } + $column = $table->getColumn('ldap_dn'); + if ($column->getLength() < 4096) { + $column->setLength(4096); + $changeSchema = true; + } + if ($table === 'ldap_user_mapping') { + if ($table->hasIndex('ldap_dn_users')) { + $table->dropIndex('ldap_dn_users'); + $changeSchema = true; + } + if (!$table->hasIndex('ldap_user_dn_hashes')) { + $table->addUniqueIndex(['ldap_dn_hash'], 'ldap_user_dn_hashes'); + $changeSchema = true; + } + } else { + if ($table->hasIndex('owncloud_name_groups')) { + $table->dropIndex('owncloud_name_groups'); + $changeSchema = true; + } + if (!$table->hasIndex('ldap_group_dn_hashes')) { + $table->addUniqueIndex(['ldap_dn_hash'], 'ldap_group_dn_hashes'); + $changeSchema = true; + } + if ($table->getPrimaryKeyColumns() !== ['owncloud_name']) { + $table->setPrimaryKey(['owncloud_name']); + $changeSchema = true; + } + } + } + + return $changeSchema ? $schema : null; + } + + /** + * @param IOutput $output + * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper` + * @param array $options + */ + public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options) { + $this->handleDNHashes('ldap_group_mapping'); + $this->handleDNHashes('ldap_user_mapping'); + } + + protected function handleDNHashes(string $table): void { + $q = $this->getSelectQuery($table); + $u = $this->getUpdateQuery($table); + + $r = $q->executeQuery(); + while ($row = $r->fetch()) { + $dnHash = hash('sha256', $row['ldap_dn'], false); + $u->setParameter('name', $row['owncloud_name']); + $u->setParameter('dn_hash', $dnHash); + try { + $u->executeStatement(); + } catch (Exception $e) { + $this->logger->error('Failed to add hash "{dnHash}" ("{name}" of {table})', + [ + 'app' => 'user_ldap', + 'name' => $row['owncloud_name'], + 'dnHash' => $dnHash, + 'table' => $table, + 'exception' => $e, + ] + ); + } + } + $r->closeCursor(); + } + + protected function getSelectQuery(string $table): IQueryBuilder { + $q = $this->dbc->getQueryBuilder(); + $q->select('owncloud_name', 'ldap_dn', 'ldap_dn_hash') + ->from($table) + ->where($q->expr()->isNull('ldap_dn_hash')); + return $q; + } + + protected function getUpdateQuery(string $table): IQueryBuilder { + $q = $this->dbc->getQueryBuilder(); + $q->update($table) + ->set('ldap_dn_hash', $query->createParameter('dn_hash')) + ->where($q->expr()->eq('owncloud_name', $q->createParameter('name'))); + return $q; + } +} From 3ba2afbcdf2b3bf10eab41c2982a3a6b74bf617e Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 4 Nov 2021 12:13:18 +0100 Subject: [PATCH 03/12] Fix variable names Signed-off-by: Joas Schilling --- .../user_ldap/lib/Migration/Version1130Date20211102154716.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php b/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php index 1d8ec577b9c..dbf53cf66b5 100644 --- a/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php +++ b/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php @@ -57,7 +57,7 @@ class Version1130Date20211102154716 extends SimpleMigrationStep { $column->setLength(4096); $changeSchema = true; } - if ($table === 'ldap_user_mapping') { + if ($tableName === 'ldap_user_mapping') { if ($table->hasIndex('ldap_dn_users')) { $table->dropIndex('ldap_dn_users'); $changeSchema = true; @@ -132,7 +132,7 @@ class Version1130Date20211102154716 extends SimpleMigrationStep { protected function getUpdateQuery(string $table): IQueryBuilder { $q = $this->dbc->getQueryBuilder(); $q->update($table) - ->set('ldap_dn_hash', $query->createParameter('dn_hash')) + ->set('ldap_dn_hash', $q->createParameter('dn_hash')) ->where($q->expr()->eq('owncloud_name', $q->createParameter('name'))); return $q; } From 343989aa52d38b0834b599d81741516a13f1fbaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 4 Nov 2021 12:27:44 +0100 Subject: [PATCH 04/12] Fixed migration step for user_ldap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/user_ldap/composer/composer/autoload_classmap.php | 1 + apps/user_ldap/composer/composer/autoload_static.php | 1 + .../user_ldap/lib/Migration/Version1130Date20211102154716.php | 4 ++-- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/apps/user_ldap/composer/composer/autoload_classmap.php b/apps/user_ldap/composer/composer/autoload_classmap.php index 208c221f362..d670dab1ef7 100644 --- a/apps/user_ldap/composer/composer/autoload_classmap.php +++ b/apps/user_ldap/composer/composer/autoload_classmap.php @@ -62,6 +62,7 @@ return array( 'OCA\\User_LDAP\\Migration\\UnsetDefaultProvider' => $baseDir . '/../lib/Migration/UnsetDefaultProvider.php', 'OCA\\User_LDAP\\Migration\\Version1010Date20200630192842' => $baseDir . '/../lib/Migration/Version1010Date20200630192842.php', 'OCA\\User_LDAP\\Migration\\Version1120Date20210917155206' => $baseDir . '/../lib/Migration/Version1120Date20210917155206.php', + 'OCA\\User_LDAP\\Migration\\Version1130Date20211102154716' => $baseDir . '/../lib/Migration/Version1130Date20211102154716.php', 'OCA\\User_LDAP\\Notification\\Notifier' => $baseDir . '/../lib/Notification/Notifier.php', 'OCA\\User_LDAP\\PagedResults\\IAdapter' => $baseDir . '/../lib/PagedResults/IAdapter.php', 'OCA\\User_LDAP\\PagedResults\\Php73' => $baseDir . '/../lib/PagedResults/Php73.php', diff --git a/apps/user_ldap/composer/composer/autoload_static.php b/apps/user_ldap/composer/composer/autoload_static.php index 260a45fd67d..0dd682ffa7d 100644 --- a/apps/user_ldap/composer/composer/autoload_static.php +++ b/apps/user_ldap/composer/composer/autoload_static.php @@ -77,6 +77,7 @@ class ComposerStaticInitUser_LDAP 'OCA\\User_LDAP\\Migration\\UnsetDefaultProvider' => __DIR__ . '/..' . '/../lib/Migration/UnsetDefaultProvider.php', 'OCA\\User_LDAP\\Migration\\Version1010Date20200630192842' => __DIR__ . '/..' . '/../lib/Migration/Version1010Date20200630192842.php', 'OCA\\User_LDAP\\Migration\\Version1120Date20210917155206' => __DIR__ . '/..' . '/../lib/Migration/Version1120Date20210917155206.php', + 'OCA\\User_LDAP\\Migration\\Version1130Date20211102154716' => __DIR__ . '/..' . '/../lib/Migration/Version1130Date20211102154716.php', 'OCA\\User_LDAP\\Notification\\Notifier' => __DIR__ . '/..' . '/../lib/Notification/Notifier.php', 'OCA\\User_LDAP\\PagedResults\\IAdapter' => __DIR__ . '/..' . '/../lib/PagedResults/IAdapter.php', 'OCA\\User_LDAP\\PagedResults\\Php73' => __DIR__ . '/..' . '/../lib/PagedResults/Php73.php', diff --git a/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php b/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php index dbf53cf66b5..198a79ef095 100644 --- a/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php +++ b/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php @@ -43,8 +43,7 @@ class Version1130Date20211102154716 extends SimpleMigrationStep { $changeSchema = false; foreach (['ldap_user_mapping', 'ldap_group_mapping'] as $tableName) { $table = $schema->getTable($tableName); - $column = $table->getColumn('ldap_dn_hash'); - if (!$column) { + if (!$table->hasColumn('ldap_dn_hash')) { $table->addColumn('ldap_dn_hash', Types::STRING, [ 'notnull' => true, 'length' => 64, @@ -76,6 +75,7 @@ class Version1130Date20211102154716 extends SimpleMigrationStep { $changeSchema = true; } if ($table->getPrimaryKeyColumns() !== ['owncloud_name']) { + $table->dropPrimaryKey(); $table->setPrimaryKey(['owncloud_name']); $changeSchema = true; } From 581b1d8da6e3b65cc4847879e81d89c219947920 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 4 Nov 2021 12:30:38 +0100 Subject: [PATCH 05/12] Put back length check to have a clear error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/user_ldap/lib/Mapping/AbstractMapping.php | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/apps/user_ldap/lib/Mapping/AbstractMapping.php b/apps/user_ldap/lib/Mapping/AbstractMapping.php index a2cc278aad5..d3ace159b0c 100644 --- a/apps/user_ldap/lib/Mapping/AbstractMapping.php +++ b/apps/user_ldap/lib/Mapping/AbstractMapping.php @@ -355,6 +355,17 @@ abstract class AbstractMapping { * @return bool */ public function map($fdn, $name, $uuid) { + if (mb_strlen($fdn) > 4096) { + \OC::$server->getLogger()->error( + 'Cannot map, because the DN exceeds 4096 characters: {dn}', + [ + 'app' => 'user_ldap', + 'dn' => $fdn, + ] + ); + return false; + } + $row = [ 'ldap_dn_hash' => $this->getDNHash($fdn), 'ldap_dn' => $fdn, From aa65a4fe90ba40bcf5f6b79fdc7504de7818ea63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 4 Nov 2021 15:38:01 +0100 Subject: [PATCH 06/12] Fixes in migration step MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We cannot set ldap_dn_hash column as notnull because it is empty for existing users before postSchemaChange is called Signed-off-by: Côme Chilliet --- .../lib/Migration/Version1130Date20211102154716.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php b/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php index 198a79ef095..829e0d5a58c 100644 --- a/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php +++ b/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php @@ -45,9 +45,8 @@ class Version1130Date20211102154716 extends SimpleMigrationStep { $table = $schema->getTable($tableName); if (!$table->hasColumn('ldap_dn_hash')) { $table->addColumn('ldap_dn_hash', Types::STRING, [ - 'notnull' => true, + 'notnull' => false, 'length' => 64, - 'default' => '', ]); $changeSchema = true; } @@ -74,7 +73,7 @@ class Version1130Date20211102154716 extends SimpleMigrationStep { $table->addUniqueIndex(['ldap_dn_hash'], 'ldap_group_dn_hashes'); $changeSchema = true; } - if ($table->getPrimaryKeyColumns() !== ['owncloud_name']) { + if (!$table->hasPrimaryKey() || ($table->getPrimaryKeyColumns() !== ['owncloud_name'])) { $table->dropPrimaryKey(); $table->setPrimaryKey(['owncloud_name']); $changeSchema = true; From bae8799e80ced582ef5214eb775f47f2d1e4a44b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 18 Nov 2021 10:25:19 +0100 Subject: [PATCH 07/12] Add the columns and alter the index in Version1010Date20200630192842 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is to ensure new installations do not need to go through migration history. Signed-off-by: Côme Chilliet --- .../Migration/Version1010Date20200630192842.php | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/apps/user_ldap/lib/Migration/Version1010Date20200630192842.php b/apps/user_ldap/lib/Migration/Version1010Date20200630192842.php index e2c78ed59f8..8263cbae157 100644 --- a/apps/user_ldap/lib/Migration/Version1010Date20200630192842.php +++ b/apps/user_ldap/lib/Migration/Version1010Date20200630192842.php @@ -60,8 +60,12 @@ class Version1010Date20200630192842 extends SimpleMigrationStep { 'length' => 255, 'default' => '', ]); + $table->addColumn('ldap_dn_hash', Types::STRING, [ + 'notnull' => false, + 'length' => 64, + ]); $table->setPrimaryKey(['owncloud_name']); - $table->addUniqueIndex(['ldap_dn'], 'ldap_dn_users'); + $table->addUniqueIndex(['ldap_dn_hash'], 'ldap_user_dn_hashes'); } if (!$schema->hasTable('ldap_group_mapping')) { @@ -81,8 +85,12 @@ class Version1010Date20200630192842 extends SimpleMigrationStep { 'length' => 255, 'default' => '', ]); - $table->setPrimaryKey(['ldap_dn']); - $table->addUniqueIndex(['owncloud_name'], 'owncloud_name_groups'); + $table->addColumn('ldap_dn_hash', Types::STRING, [ + 'notnull' => false, + 'length' => 64, + ]); + $table->setPrimaryKey(['owncloud_name']); + $table->addUniqueIndex(['ldap_dn_hash'], 'ldap_group_dn_hashes'); } if (!$schema->hasTable('ldap_group_members')) { From bab9964c01a2918e3518844689dd7e729926b8e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 18 Nov 2021 10:30:35 +0100 Subject: [PATCH 08/12] Make sure that hash function returns a string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The documentation says it can return false, and even if that is highly unlikely for sha256, better safe than sorry. Signed-off-by: Côme Chilliet --- apps/user_ldap/lib/Mapping/AbstractMapping.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/apps/user_ldap/lib/Mapping/AbstractMapping.php b/apps/user_ldap/lib/Mapping/AbstractMapping.php index d3ace159b0c..1d3e1a221f7 100644 --- a/apps/user_ldap/lib/Mapping/AbstractMapping.php +++ b/apps/user_ldap/lib/Mapping/AbstractMapping.php @@ -191,7 +191,12 @@ abstract class AbstractMapping { * Get the hash to store in database column ldap_dn_hash for a given dn */ protected function getDNHash(string $fdn): string { - return (string)hash('sha256', $fdn, false); + $hash = hash('sha256', $fdn, false); + if (is_string($hash)) { + return $hash; + } else { + throw new \RuntimeException('hash function did not return a string'); + } } /** From ddb9727be12ab76e7ce57aa489519d80c3b09e2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 18 Nov 2021 10:40:55 +0100 Subject: [PATCH 09/12] Add an index for directory_uuid as well MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../lib/Migration/Version1010Date20200630192842.php | 2 ++ .../lib/Migration/Version1130Date20211102154716.php | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/apps/user_ldap/lib/Migration/Version1010Date20200630192842.php b/apps/user_ldap/lib/Migration/Version1010Date20200630192842.php index 8263cbae157..939db69a6ab 100644 --- a/apps/user_ldap/lib/Migration/Version1010Date20200630192842.php +++ b/apps/user_ldap/lib/Migration/Version1010Date20200630192842.php @@ -66,6 +66,7 @@ class Version1010Date20200630192842 extends SimpleMigrationStep { ]); $table->setPrimaryKey(['owncloud_name']); $table->addUniqueIndex(['ldap_dn_hash'], 'ldap_user_dn_hashes'); + $table->addUniqueIndex(['directory_uuid'], 'ldap_user_directory_uuid'); } if (!$schema->hasTable('ldap_group_mapping')) { @@ -91,6 +92,7 @@ class Version1010Date20200630192842 extends SimpleMigrationStep { ]); $table->setPrimaryKey(['owncloud_name']); $table->addUniqueIndex(['ldap_dn_hash'], 'ldap_group_dn_hashes'); + $table->addUniqueIndex(['directory_uuid'], 'ldap_group_directory_uuid'); } if (!$schema->hasTable('ldap_group_members')) { diff --git a/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php b/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php index 829e0d5a58c..144efad0ead 100644 --- a/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php +++ b/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php @@ -64,6 +64,10 @@ class Version1130Date20211102154716 extends SimpleMigrationStep { $table->addUniqueIndex(['ldap_dn_hash'], 'ldap_user_dn_hashes'); $changeSchema = true; } + if (!$table->hasIndex('ldap_user_directory_uuid')) { + $table->addUniqueIndex(['directory_uuid'], 'ldap_user_directory_uuid'); + $changeSchema = true; + } } else { if ($table->hasIndex('owncloud_name_groups')) { $table->dropIndex('owncloud_name_groups'); @@ -73,6 +77,10 @@ class Version1130Date20211102154716 extends SimpleMigrationStep { $table->addUniqueIndex(['ldap_dn_hash'], 'ldap_group_dn_hashes'); $changeSchema = true; } + if (!$table->hasIndex('ldap_group_directory_uuid')) { + $table->addUniqueIndex(['directory_uuid'], 'ldap_group_directory_uuid'); + $changeSchema = true; + } if (!$table->hasPrimaryKey() || ($table->getPrimaryKeyColumns() !== ['owncloud_name'])) { $table->dropPrimaryKey(); $table->setPrimaryKey(['owncloud_name']); From 30507846bc60d3e96cee0ce0f0d77d1958c0db6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 18 Nov 2021 10:48:41 +0100 Subject: [PATCH 10/12] Use clearer names for variables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../Version1120Date20210917155206.php | 34 +++++++++---------- .../Version1130Date20211102154716.php | 34 +++++++++---------- 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/apps/user_ldap/lib/Migration/Version1120Date20210917155206.php b/apps/user_ldap/lib/Migration/Version1120Date20210917155206.php index d46107733dc..3ab069679ef 100644 --- a/apps/user_ldap/lib/Migration/Version1120Date20210917155206.php +++ b/apps/user_ldap/lib/Migration/Version1120Date20210917155206.php @@ -70,19 +70,19 @@ class Version1120Date20210917155206 extends SimpleMigrationStep { } protected function handleIDs(string $table, bool $emitHooks) { - $q = $this->getSelectQuery($table); - $u = $this->getUpdateQuery($table); + $select = $this->getSelectQuery($table); + $update = $this->getUpdateQuery($table); - $r = $q->executeQuery(); - while ($row = $r->fetch()) { + $result = $select->executeQuery(); + while ($row = $result->fetch()) { $newId = hash('sha256', $row['owncloud_name'], false); if ($emitHooks) { $this->emitUnassign($row['owncloud_name'], true); } - $u->setParameter('uuid', $row['directory_uuid']); - $u->setParameter('newId', $newId); + $update->setParameter('uuid', $row['directory_uuid']); + $update->setParameter('newId', $newId); try { - $u->executeStatement(); + $update->executeStatement(); if ($emitHooks) { $this->emitUnassign($row['owncloud_name'], false); $this->emitAssign($newId); @@ -100,23 +100,23 @@ class Version1120Date20210917155206 extends SimpleMigrationStep { ); } } - $r->closeCursor(); + $result->closeCursor(); } protected function getSelectQuery(string $table): IQueryBuilder { - $q = $this->dbc->getQueryBuilder(); - $q->select('owncloud_name', 'directory_uuid') + $qb = $this->dbc->getQueryBuilder(); + $qb->select('owncloud_name', 'directory_uuid') ->from($table) - ->where($q->expr()->like('owncloud_name', $q->createNamedParameter(str_repeat('_', 65) . '%'), Types::STRING)); - return $q; + ->where($qb->expr()->like('owncloud_name', $qb->createNamedParameter(str_repeat('_', 65) . '%'), Types::STRING)); + return $qb; } protected function getUpdateQuery(string $table): IQueryBuilder { - $q = $this->dbc->getQueryBuilder(); - $q->update($table) - ->set('owncloud_name', $q->createParameter('newId')) - ->where($q->expr()->eq('directory_uuid', $q->createParameter('uuid'))); - return $q; + $qb = $this->dbc->getQueryBuilder(); + $qb->update($table) + ->set('owncloud_name', $qb->createParameter('newId')) + ->where($qb->expr()->eq('directory_uuid', $qb->createParameter('uuid'))); + return $qb; } protected function emitUnassign(string $oldId, bool $pre): void { diff --git a/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php b/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php index 144efad0ead..b79dbabd5a0 100644 --- a/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php +++ b/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php @@ -103,16 +103,16 @@ class Version1130Date20211102154716 extends SimpleMigrationStep { } protected function handleDNHashes(string $table): void { - $q = $this->getSelectQuery($table); - $u = $this->getUpdateQuery($table); + $select = $this->getSelectQuery($table); + $update = $this->getUpdateQuery($table); - $r = $q->executeQuery(); - while ($row = $r->fetch()) { + $result = $select->executeQuery(); + while ($row = $result->fetch()) { $dnHash = hash('sha256', $row['ldap_dn'], false); - $u->setParameter('name', $row['owncloud_name']); - $u->setParameter('dn_hash', $dnHash); + $update->setParameter('name', $row['owncloud_name']); + $update->setParameter('dn_hash', $dnHash); try { - $u->executeStatement(); + $update->executeStatement(); } catch (Exception $e) { $this->logger->error('Failed to add hash "{dnHash}" ("{name}" of {table})', [ @@ -125,22 +125,22 @@ class Version1130Date20211102154716 extends SimpleMigrationStep { ); } } - $r->closeCursor(); + $result->closeCursor(); } protected function getSelectQuery(string $table): IQueryBuilder { - $q = $this->dbc->getQueryBuilder(); - $q->select('owncloud_name', 'ldap_dn', 'ldap_dn_hash') + $qb = $this->dbc->getQueryBuilder(); + $qb->select('owncloud_name', 'ldap_dn', 'ldap_dn_hash') ->from($table) - ->where($q->expr()->isNull('ldap_dn_hash')); - return $q; + ->where($qb->expr()->isNull('ldap_dn_hash')); + return $qb; } protected function getUpdateQuery(string $table): IQueryBuilder { - $q = $this->dbc->getQueryBuilder(); - $q->update($table) - ->set('ldap_dn_hash', $q->createParameter('dn_hash')) - ->where($q->expr()->eq('owncloud_name', $q->createParameter('name'))); - return $q; + $qb = $this->dbc->getQueryBuilder(); + $qb->update($table) + ->set('ldap_dn_hash', $qb->createParameter('dn_hash')) + ->where($qb->expr()->eq('owncloud_name', $qb->createParameter('name'))); + return $qb; } } From 1523482047a0c4aeb98b95728df15268dc286384 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 9 Dec 2021 17:42:17 +0100 Subject: [PATCH 11/12] Add missing copyright headers in migration steps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../Version1120Date20210917155206.php | 22 +++++++++++++++++++ .../Version1130Date20211102154716.php | 22 +++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/apps/user_ldap/lib/Migration/Version1120Date20210917155206.php b/apps/user_ldap/lib/Migration/Version1120Date20210917155206.php index 3ab069679ef..d2b440c4dfe 100644 --- a/apps/user_ldap/lib/Migration/Version1120Date20210917155206.php +++ b/apps/user_ldap/lib/Migration/Version1120Date20210917155206.php @@ -2,6 +2,28 @@ declare(strict_types=1); +/** + * @copyright Copyright (c) 2020 Joas Schilling + * + * @author Arthur Schiwon + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + namespace OCA\User_LDAP\Migration; use Closure; diff --git a/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php b/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php index b79dbabd5a0..82afb4965de 100644 --- a/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php +++ b/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php @@ -2,6 +2,28 @@ declare(strict_types=1); +/** + * @copyright Copyright (c) 2020 Joas Schilling + * + * @author Côme Chilliet + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + namespace OCA\User_LDAP\Migration; use Closure; From eeefca265889426483899db15ccff266f7948a80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 9 Dec 2021 17:42:44 +0100 Subject: [PATCH 12/12] Bump user_ldap version to make sure the migration runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/user_ldap/appinfo/info.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/user_ldap/appinfo/info.xml b/apps/user_ldap/appinfo/info.xml index 8633dac8d27..b07821942df 100644 --- a/apps/user_ldap/appinfo/info.xml +++ b/apps/user_ldap/appinfo/info.xml @@ -9,7 +9,7 @@ A user logs into Nextcloud with their LDAP or AD credentials, and is granted access based on an authentication request handled by the LDAP or AD server. Nextcloud does not store LDAP or AD passwords, rather these credentials are used to authenticate a user and then Nextcloud uses a session for the user ID. More information is available in the LDAP User and Group Backend documentation. - 1.12.1 + 1.12.2 agpl Dominik Schmidt Arthur Schiwon