summaryrefslogtreecommitdiffstats
path: root/apps/user_ldap
diff options
context:
space:
mode:
authorArthur Schiwon <blizzz@arthur-schiwon.de>2021-09-17 19:15:46 +0200
committerArthur Schiwon <blizzz@arthur-schiwon.de>2021-09-27 17:52:13 +0200
commit24f2974267684b364d0396db2acaa6e8dea1c1ad (patch)
treec89465ca48003a1565f1b7173a502870e7116abd /apps/user_ldap
parent62a5d27eb7633eaf046e50f8ada8e2a2b2239f83 (diff)
downloadnextcloud-server-24f2974267684b364d0396db2acaa6e8dea1c1ad.tar.gz
nextcloud-server-24f2974267684b364d0396db2acaa6e8dea1c1ad.zip
ensure that user and group IDs in LDAP's tables are also max 64chars
- limitation by core tables (e.g. sharing), IDs are always 64chars - when longer group IDs were requested they are hashed (does not affect displaynames) Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
Diffstat (limited to 'apps/user_ldap')
-rw-r--r--apps/user_ldap/appinfo/info.xml2
-rw-r--r--apps/user_ldap/composer/composer/autoload_classmap.php1
-rw-r--r--apps/user_ldap/composer/composer/autoload_static.php1
-rw-r--r--apps/user_ldap/composer/composer/installed.php4
-rw-r--r--apps/user_ldap/lib/Access.php25
-rw-r--r--apps/user_ldap/lib/Migration/Version1010Date20200630192842.php4
-rw-r--r--apps/user_ldap/lib/Migration/Version1120Date20210917155206.php133
-rw-r--r--apps/user_ldap/tests/AccessTest.php31
8 files changed, 194 insertions, 7 deletions
diff --git a/apps/user_ldap/appinfo/info.xml b/apps/user_ldap/appinfo/info.xml
index 46bff54cb1e..b5ea52f909f 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.
</description>
- <version>1.11.0</version>
+ <version>1.11.1</version>
<licence>agpl</licence>
<author>Dominik Schmidt</author>
<author>Arthur Schiwon</author>
diff --git a/apps/user_ldap/composer/composer/autoload_classmap.php b/apps/user_ldap/composer/composer/autoload_classmap.php
index 6d9d221c10f..4b4ba60da29 100644
--- a/apps/user_ldap/composer/composer/autoload_classmap.php
+++ b/apps/user_ldap/composer/composer/autoload_classmap.php
@@ -60,6 +60,7 @@ return array(
'OCA\\User_LDAP\\Migration\\UUIDFixInsert' => $baseDir . '/../lib/Migration/UUIDFixInsert.php',
'OCA\\User_LDAP\\Migration\\UUIDFixUser' => $baseDir . '/../lib/Migration/UUIDFixUser.php',
'OCA\\User_LDAP\\Migration\\Version1010Date20200630192842' => $baseDir . '/../lib/Migration/Version1010Date20200630192842.php',
+ 'OCA\\User_LDAP\\Migration\\Version1120Date20210917155206' => $baseDir . '/../lib/Migration/Version1120Date20210917155206.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\\Php54' => $baseDir . '/../lib/PagedResults/Php54.php',
diff --git a/apps/user_ldap/composer/composer/autoload_static.php b/apps/user_ldap/composer/composer/autoload_static.php
index afcda7d7810..8d28f09ba75 100644
--- a/apps/user_ldap/composer/composer/autoload_static.php
+++ b/apps/user_ldap/composer/composer/autoload_static.php
@@ -75,6 +75,7 @@ class ComposerStaticInitUser_LDAP
'OCA\\User_LDAP\\Migration\\UUIDFixInsert' => __DIR__ . '/..' . '/../lib/Migration/UUIDFixInsert.php',
'OCA\\User_LDAP\\Migration\\UUIDFixUser' => __DIR__ . '/..' . '/../lib/Migration/UUIDFixUser.php',
'OCA\\User_LDAP\\Migration\\Version1010Date20200630192842' => __DIR__ . '/..' . '/../lib/Migration/Version1010Date20200630192842.php',
+ 'OCA\\User_LDAP\\Migration\\Version1120Date20210917155206' => __DIR__ . '/..' . '/../lib/Migration/Version1120Date20210917155206.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\\Php54' => __DIR__ . '/..' . '/../lib/PagedResults/Php54.php',
diff --git a/apps/user_ldap/composer/composer/installed.php b/apps/user_ldap/composer/composer/installed.php
index 412956a2006..8acc70b43ac 100644
--- a/apps/user_ldap/composer/composer/installed.php
+++ b/apps/user_ldap/composer/composer/installed.php
@@ -5,7 +5,7 @@
'type' => 'library',
'install_path' => __DIR__ . '/../',
'aliases' => array(),
- 'reference' => '7b056b2ed72b2a7a5937e0c251858e1906277b4b',
+ 'reference' => 'add48a086275dcfb604556dbda95503345f57292',
'name' => '__root__',
'dev' => false,
),
@@ -16,7 +16,7 @@
'type' => 'library',
'install_path' => __DIR__ . '/../',
'aliases' => array(),
- 'reference' => '7b056b2ed72b2a7a5937e0c251858e1906277b4b',
+ 'reference' => 'add48a086275dcfb604556dbda95503345f57292',
'dev_requirement' => false,
),
),
diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php
index 0b243b8ad59..862089e9d30 100644
--- a/apps/user_ldap/lib/Access.php
+++ b/apps/user_ldap/lib/Access.php
@@ -60,6 +60,8 @@ use OCA\User_LDAP\User\OfflineUser;
use OCP\IConfig;
use OCP\ILogger;
use OCP\IUserManager;
+use function strlen;
+use function substr;
/**
* Class Access
@@ -579,7 +581,7 @@ class Access extends LDAPUtility {
return false;
}
} else {
- $intName = $ldapName;
+ $intName = $this->sanitizeGroupIDCandidate($ldapName);
}
//a new user/group! Add it only if it doesn't conflict with other backend's users or existing groups
@@ -838,6 +840,11 @@ class Access extends LDAPUtility {
* @return string|false with with the name to use in Nextcloud or false if unsuccessful
*/
private function createAltInternalOwnCloudName($name, $isUser) {
+ // ensure there is space for the "_1234" suffix
+ if (strlen($name) > 59) {
+ $name = substr($name, 0, 59);
+ }
+
$originalTTL = $this->connection->ldapCacheTTL;
$this->connection->setConfiguration(['ldapCacheTTL' => 0]);
if ($isUser) {
@@ -1432,6 +1439,10 @@ class Access extends LDAPUtility {
// Every remaining disallowed characters will be removed
$name = preg_replace('/[^a-zA-Z0-9_.@-]/u', '', $name);
+ if (strlen($name) > 64) {
+ $name = (string)hash('sha256', $name, false);
+ }
+
if ($name === '') {
throw new \InvalidArgumentException('provided name template for username does not contain any allowed characters');
}
@@ -1439,6 +1450,18 @@ class Access extends LDAPUtility {
return $name;
}
+ public function sanitizeGroupIDCandidate(string $candidate): string {
+ $candidate = trim($candidate);
+ if (strlen($candidate) > 64) {
+ $candidate = (string)hash('sha256', $candidate, false);
+ }
+ if ($candidate === '') {
+ throw new \InvalidArgumentException('provided name template for username does not contain any allowed characters');
+ }
+
+ return $candidate;
+ }
+
/**
* escapes (user provided) parts for LDAP filter
*
diff --git a/apps/user_ldap/lib/Migration/Version1010Date20200630192842.php b/apps/user_ldap/lib/Migration/Version1010Date20200630192842.php
index 754200405c8..3d9cc40b5b2 100644
--- a/apps/user_ldap/lib/Migration/Version1010Date20200630192842.php
+++ b/apps/user_ldap/lib/Migration/Version1010Date20200630192842.php
@@ -52,7 +52,7 @@ class Version1010Date20200630192842 extends SimpleMigrationStep {
]);
$table->addColumn('owncloud_name', Types::STRING, [
'notnull' => true,
- 'length' => 255,
+ 'length' => 64,
'default' => '',
]);
$table->addColumn('directory_uuid', Types::STRING, [
@@ -73,7 +73,7 @@ class Version1010Date20200630192842 extends SimpleMigrationStep {
]);
$table->addColumn('owncloud_name', Types::STRING, [
'notnull' => true,
- 'length' => 255,
+ 'length' => 64,
'default' => '',
]);
$table->addColumn('directory_uuid', Types::STRING, [
diff --git a/apps/user_ldap/lib/Migration/Version1120Date20210917155206.php b/apps/user_ldap/lib/Migration/Version1120Date20210917155206.php
new file mode 100644
index 00000000000..d46107733dc
--- /dev/null
+++ b/apps/user_ldap/lib/Migration/Version1120Date20210917155206.php
@@ -0,0 +1,133 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OCA\User_LDAP\Migration;
+
+use Closure;
+use OC\Hooks\PublicEmitter;
+use OCP\DB\Exception;
+use OCP\DB\ISchemaWrapper;
+use OCP\DB\QueryBuilder\IQueryBuilder;
+use OCP\DB\Types;
+use OCP\IDBConnection;
+use OCP\IUserManager;
+use OCP\Migration\IOutput;
+use OCP\Migration\SimpleMigrationStep;
+use Psr\Log\LoggerInterface;
+
+class Version1120Date20210917155206 extends SimpleMigrationStep {
+
+ /** @var IDBConnection */
+ private $dbc;
+ /** @var IUserManager */
+ private $userManager;
+ /** @var LoggerInterface */
+ private $logger;
+
+ public function __construct(IDBConnection $dbc, IUserManager $userManager, LoggerInterface $logger) {
+ $this->dbc = $dbc;
+ $this->userManager = $userManager;
+ $this->logger = $logger;
+ }
+
+ public function getName() {
+ return 'Adjust LDAP user and group id column lengths to match server lengths';
+ }
+
+ /**
+ * @param IOutput $output
+ * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
+ * @param array $options
+ */
+ public function preSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void {
+ // ensure that there is no user or group id longer than 64char in LDAP table
+ $this->handleIDs('ldap_group_mapping', false);
+ $this->handleIDs('ldap_user_mapping', true);
+ }
+
+ /**
+ * @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('owncloud_name');
+ if ($column->getLength() > 64) {
+ $column->setLength(64);
+ $changeSchema = true;
+ }
+ }
+
+ return $changeSchema ? $schema : null;
+ }
+
+ protected function handleIDs(string $table, bool $emitHooks) {
+ $q = $this->getSelectQuery($table);
+ $u = $this->getUpdateQuery($table);
+
+ $r = $q->executeQuery();
+ while ($row = $r->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);
+ try {
+ $u->executeStatement();
+ if ($emitHooks) {
+ $this->emitUnassign($row['owncloud_name'], false);
+ $this->emitAssign($newId);
+ }
+ } catch (Exception $e) {
+ $this->logger->error('Failed to shorten owncloud_name "{oldId}" to "{newId}" (UUID: "{uuid}" of {table})',
+ [
+ 'app' => 'user_ldap',
+ 'oldId' => $row['owncloud_name'],
+ 'newId' => $newId,
+ 'uuid' => $row['directory_uuid'],
+ 'table' => $table,
+ 'exception' => $e,
+ ]
+ );
+ }
+ }
+ $r->closeCursor();
+ }
+
+ protected function getSelectQuery(string $table): IQueryBuilder {
+ $q = $this->dbc->getQueryBuilder();
+ $q->select('owncloud_name', 'directory_uuid')
+ ->from($table)
+ ->where($q->expr()->like('owncloud_name', $q->createNamedParameter(str_repeat('_', 65) . '%'), Types::STRING));
+ return $q;
+ }
+
+ 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;
+ }
+
+ protected function emitUnassign(string $oldId, bool $pre): void {
+ if ($this->userManager instanceof PublicEmitter) {
+ $this->userManager->emit('\OC\User', $pre ? 'pre' : 'post' . 'UnassignedUserId', [$oldId]);
+ }
+ }
+
+ protected function emitAssign(string $newId): void {
+ if ($this->userManager instanceof PublicEmitter) {
+ $this->userManager->emit('\OC\User', 'assignedUserId', [$newId]);
+ }
+ }
+}
diff --git a/apps/user_ldap/tests/AccessTest.php b/apps/user_ldap/tests/AccessTest.php
index a532bd6fd7a..8c401722592 100644
--- a/apps/user_ldap/tests/AccessTest.php
+++ b/apps/user_ldap/tests/AccessTest.php
@@ -697,7 +697,28 @@ class AccessTest extends TestCase {
['epost@poste.test', 'epost@poste.test'],
['frรคnk', $translitExpected],
[' gerda ', 'gerda'],
- ['๐Ÿ•ฑ๐Ÿต๐Ÿ˜๐Ÿ‘', null]
+ ['๐Ÿ•ฑ๐Ÿต๐Ÿ˜๐Ÿ‘', null],
+ [
+ 'OneNameToRuleThemAllOneNameToFindThemOneNameToBringThemAllAndInTheDarknessBindThem',
+ '81ff71b5dd0f0092e2dc977b194089120093746e273f2ef88c11003762783127'
+ ]
+ ];
+ }
+
+ public function groupIDCandidateProvider() {
+ return [
+ ['alice', 'alice'],
+ ['b/ob', 'b/ob'],
+ ['charly๐Ÿฌ', 'charly๐Ÿฌ'],
+ ['debo rah', 'debo rah'],
+ ['epost@poste.test', 'epost@poste.test'],
+ ['frรคnk', 'frรคnk'],
+ [' gerda ', 'gerda'],
+ ['๐Ÿ•ฑ๐Ÿต๐Ÿ˜๐Ÿ‘', '๐Ÿ•ฑ๐Ÿต๐Ÿ˜๐Ÿ‘'],
+ [
+ 'OneNameToRuleThemAllOneNameToFindThemOneNameToBringThemAllAndInTheDarknessBindThem',
+ '81ff71b5dd0f0092e2dc977b194089120093746e273f2ef88c11003762783127'
+ ]
];
}
@@ -718,6 +739,14 @@ class AccessTest extends TestCase {
$this->assertSame($expected, $sanitizedName);
}
+ /**
+ * @dataProvider groupIDCandidateProvider
+ */
+ public function testSanitizeGroupIDCandidate(string $name, string $expected) {
+ $sanitizedName = $this->access->sanitizeGroupIDCandidate($name);
+ $this->assertSame($expected, $sanitizedName);
+ }
+
public function testUserStateUpdate() {
$this->connection->expects($this->any())
->method('__get')