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 85fc91590fb2b..8d1236e73485a 100644 --- a/apps/user_ldap/lib/Mapping/AbstractMapping.php +++ b/apps/user_ldap/lib/Mapping/AbstractMapping.php @@ -67,6 +67,7 @@ public function __construct(\OCP\IDBConnection $dbc) { */ public function isColNameValid($col) { switch ($col) { + case 'ldap_full_dn': case 'ldap_dn': case 'owncloud_name': case 'directory_uuid': @@ -134,7 +135,7 @@ protected function modify(IPreparedStatement $statement, $parameters) { */ 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 @@ public function setDNbyUUID($fdn, $uuid) { $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 @@ public function setUUIDbyDN($uuid, $fdn) { 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 @@ public function setUUIDbyDN($uuid, $fdn) { */ 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 @@ public function getListOfIdsByDn(array $fdns): array { $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 @@ public function getNameByUUID($uuid) { } 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 @@ public function getDnByUUID($uuid) { * @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 @@ public function getUUIDByDN($dn) { 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 @@ public function getList($offset = null, $limit = null) { * @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 e2c78ed59f89e..9f0faf752a332 100644 --- a/apps/user_ldap/lib/Migration/Version1010Date20200630192842.php +++ b/apps/user_ldap/lib/Migration/Version1010Date20200630192842.php @@ -47,7 +47,12 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt $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 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt $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 8d1236e73485a..a2cc278aad5fe 100644 --- a/apps/user_ldap/lib/Mapping/AbstractMapping.php +++ b/apps/user_ldap/lib/Mapping/AbstractMapping.php @@ -67,8 +67,8 @@ public function __construct(\OCP\IDBConnection $dbc) { */ 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 @@ protected function modify(IPreparedStatement $statement, $parameters) { */ 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 @@ public function setDNbyUUID($fdn, $uuid) { $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 @@ public function setUUIDbyDN($uuid, $fdn) { $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 @@ public function setUUIDbyDN($uuid, $fdn) { } /** - * 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 @@ protected function getDNHash(string $fdn): string { */ 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 @@ public function getNameByDN($fdn) { */ 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 @@ public function getListOfIdsByDn(array $fdns): array { } 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 @@ public function getNameByUUID($uuid) { } 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 @@ public function getDnByUUID($uuid) { * @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 @@ public function getUUIDByDN($dn) { 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 @@ public function getList($offset = null, $limit = null) { */ 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 @@ public function clearCb(callable $preCallback, callable $postCallback): bool { */ 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 9f0faf752a332..e2c78ed59f89e 100644 --- a/apps/user_ldap/lib/Migration/Version1010Date20200630192842.php +++ b/apps/user_ldap/lib/Migration/Version1010Date20200630192842.php @@ -47,12 +47,7 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt $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 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt $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 0000000000000..1d8ec577b9c33 --- /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 1d8ec577b9c33..dbf53cf66b50c 100644 --- a/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php +++ b/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php @@ -57,7 +57,7 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt $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 @@ protected function getSelectQuery(string $table): IQueryBuilder { 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 208c221f36277..d670dab1ef773 100644 --- a/apps/user_ldap/composer/composer/autoload_classmap.php +++ b/apps/user_ldap/composer/composer/autoload_classmap.php @@ -62,6 +62,7 @@ '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 260a45fd67dd9..0dd682ffa7dd5 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 dbf53cf66b50c..198a79ef09516 100644 --- a/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php +++ b/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php @@ -43,8 +43,7 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt $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 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt $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 a2cc278aad5fe..d3ace159b0c7d 100644 --- a/apps/user_ldap/lib/Mapping/AbstractMapping.php +++ b/apps/user_ldap/lib/Mapping/AbstractMapping.php @@ -355,6 +355,17 @@ public function getList($offset = null, $limit = null) { * @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 198a79ef09516..829e0d5a58c96 100644 --- a/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php +++ b/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php @@ -45,9 +45,8 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt $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 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt $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 e2c78ed59f89e..8263cbae157cf 100644 --- a/apps/user_ldap/lib/Migration/Version1010Date20200630192842.php +++ b/apps/user_ldap/lib/Migration/Version1010Date20200630192842.php @@ -60,8 +60,12 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt '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 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt '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 d3ace159b0c7d..1d3e1a221f732 100644 --- a/apps/user_ldap/lib/Mapping/AbstractMapping.php +++ b/apps/user_ldap/lib/Mapping/AbstractMapping.php @@ -191,7 +191,12 @@ public function setUUIDbyDN($uuid, $fdn) { * 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 8263cbae157cf..939db69a6abf4 100644 --- a/apps/user_ldap/lib/Migration/Version1010Date20200630192842.php +++ b/apps/user_ldap/lib/Migration/Version1010Date20200630192842.php @@ -66,6 +66,7 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt ]); $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 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt ]); $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 829e0d5a58c96..144efad0eadcd 100644 --- a/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php +++ b/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php @@ -64,6 +64,10 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt $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 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt $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 d46107733dc5c..3ab069679ef00 100644 --- a/apps/user_ldap/lib/Migration/Version1120Date20210917155206.php +++ b/apps/user_ldap/lib/Migration/Version1120Date20210917155206.php @@ -70,19 +70,19 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt } 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 @@ protected function handleIDs(string $table, bool $emitHooks) { ); } } - $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 144efad0eadcd..b79dbabd5a0ae 100644 --- a/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php +++ b/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php @@ -103,16 +103,16 @@ public function postSchemaChange(IOutput $output, Closure $schemaClosure, array } 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 @@ protected function handleDNHashes(string $table): void { ); } } - $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 3ab069679ef00..d2b440c4dfe94 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 b79dbabd5a0ae..82afb4965deea 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 8633dac8d27b8..b07821942df4e 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