Skip to content

Commit 8586d60

Browse files
blizzzPVince81
authored andcommitted
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>
1 parent 610bc60 commit 8586d60

File tree

8 files changed

+194
-7
lines changed

8 files changed

+194
-7
lines changed

apps/user_ldap/appinfo/info.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
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.
1010

1111
</description>
12-
<version>1.12.0</version>
12+
<version>1.12.1</version>
1313
<licence>agpl</licence>
1414
<author>Dominik Schmidt</author>
1515
<author>Arthur Schiwon</author>

apps/user_ldap/composer/composer/autoload_classmap.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
'OCA\\User_LDAP\\Migration\\UUIDFixUser' => $baseDir . '/../lib/Migration/UUIDFixUser.php',
6262
'OCA\\User_LDAP\\Migration\\UnsetDefaultProvider' => $baseDir . '/../lib/Migration/UnsetDefaultProvider.php',
6363
'OCA\\User_LDAP\\Migration\\Version1010Date20200630192842' => $baseDir . '/../lib/Migration/Version1010Date20200630192842.php',
64+
'OCA\\User_LDAP\\Migration\\Version1120Date20210917155206' => $baseDir . '/../lib/Migration/Version1120Date20210917155206.php',
6465
'OCA\\User_LDAP\\Notification\\Notifier' => $baseDir . '/../lib/Notification/Notifier.php',
6566
'OCA\\User_LDAP\\PagedResults\\IAdapter' => $baseDir . '/../lib/PagedResults/IAdapter.php',
6667
'OCA\\User_LDAP\\PagedResults\\Php73' => $baseDir . '/../lib/PagedResults/Php73.php',

apps/user_ldap/composer/composer/autoload_static.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ class ComposerStaticInitUser_LDAP
7676
'OCA\\User_LDAP\\Migration\\UUIDFixUser' => __DIR__ . '/..' . '/../lib/Migration/UUIDFixUser.php',
7777
'OCA\\User_LDAP\\Migration\\UnsetDefaultProvider' => __DIR__ . '/..' . '/../lib/Migration/UnsetDefaultProvider.php',
7878
'OCA\\User_LDAP\\Migration\\Version1010Date20200630192842' => __DIR__ . '/..' . '/../lib/Migration/Version1010Date20200630192842.php',
79+
'OCA\\User_LDAP\\Migration\\Version1120Date20210917155206' => __DIR__ . '/..' . '/../lib/Migration/Version1120Date20210917155206.php',
7980
'OCA\\User_LDAP\\Notification\\Notifier' => __DIR__ . '/..' . '/../lib/Notification/Notifier.php',
8081
'OCA\\User_LDAP\\PagedResults\\IAdapter' => __DIR__ . '/..' . '/../lib/PagedResults/IAdapter.php',
8182
'OCA\\User_LDAP\\PagedResults\\Php73' => __DIR__ . '/..' . '/../lib/PagedResults/Php73.php',

apps/user_ldap/composer/composer/installed.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
'type' => 'library',
66
'install_path' => __DIR__ . '/../',
77
'aliases' => array(),
8-
'reference' => 'fb5ee6087bfd1f4cc2f37cda7a7cab7072aaae86',
8+
'reference' => 'df2cee4882ed9227ebc06cfb490d71197eb61638',
99
'name' => '__root__',
1010
'dev' => false,
1111
),
@@ -16,7 +16,7 @@
1616
'type' => 'library',
1717
'install_path' => __DIR__ . '/../',
1818
'aliases' => array(),
19-
'reference' => 'fb5ee6087bfd1f4cc2f37cda7a7cab7072aaae86',
19+
'reference' => 'df2cee4882ed9227ebc06cfb490d71197eb61638',
2020
'dev_requirement' => false,
2121
),
2222
),

apps/user_ldap/lib/Access.php

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@
5959
use OCP\IConfig;
6060
use OCP\ILogger;
6161
use OCP\IUserManager;
62+
use function strlen;
63+
use function substr;
6264

6365
/**
6466
* Class Access
@@ -578,7 +580,7 @@ public function dn2ocname($fdn, $ldapName = null, $isUser = true, &$newlyMapped
578580
return false;
579581
}
580582
} else {
581-
$intName = $ldapName;
583+
$intName = $this->sanitizeGroupIDCandidate($ldapName);
582584
}
583585

584586
//a new user/group! Add it only if it doesn't conflict with other backend's users or existing groups
@@ -837,6 +839,11 @@ private function _createAltInternalOwnCloudNameForGroups($name) {
837839
* @return string|false with with the name to use in Nextcloud or false if unsuccessful
838840
*/
839841
private function createAltInternalOwnCloudName($name, $isUser) {
842+
// ensure there is space for the "_1234" suffix
843+
if (strlen($name) > 59) {
844+
$name = substr($name, 0, 59);
845+
}
846+
840847
$originalTTL = $this->connection->ldapCacheTTL;
841848
$this->connection->setConfiguration(['ldapCacheTTL' => 0]);
842849
if ($isUser) {
@@ -1431,13 +1438,29 @@ public function sanitizeUsername($name) {
14311438
// Every remaining disallowed characters will be removed
14321439
$name = preg_replace('/[^a-zA-Z0-9_.@-]/u', '', $name);
14331440

1441+
if (strlen($name) > 64) {
1442+
$name = (string)hash('sha256', $name, false);
1443+
}
1444+
14341445
if ($name === '') {
14351446
throw new \InvalidArgumentException('provided name template for username does not contain any allowed characters');
14361447
}
14371448

14381449
return $name;
14391450
}
14401451

1452+
public function sanitizeGroupIDCandidate(string $candidate): string {
1453+
$candidate = trim($candidate);
1454+
if (strlen($candidate) > 64) {
1455+
$candidate = (string)hash('sha256', $candidate, false);
1456+
}
1457+
if ($candidate === '') {
1458+
throw new \InvalidArgumentException('provided name template for username does not contain any allowed characters');
1459+
}
1460+
1461+
return $candidate;
1462+
}
1463+
14411464
/**
14421465
* escapes (user provided) parts for LDAP filter
14431466
*

apps/user_ldap/lib/Migration/Version1010Date20200630192842.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
5252
]);
5353
$table->addColumn('owncloud_name', Types::STRING, [
5454
'notnull' => true,
55-
'length' => 255,
55+
'length' => 64,
5656
'default' => '',
5757
]);
5858
$table->addColumn('directory_uuid', Types::STRING, [
@@ -73,7 +73,7 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
7373
]);
7474
$table->addColumn('owncloud_name', Types::STRING, [
7575
'notnull' => true,
76-
'length' => 255,
76+
'length' => 64,
7777
'default' => '',
7878
]);
7979
$table->addColumn('directory_uuid', Types::STRING, [
Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace OCA\User_LDAP\Migration;
6+
7+
use Closure;
8+
use OC\Hooks\PublicEmitter;
9+
use OCP\DB\Exception;
10+
use OCP\DB\ISchemaWrapper;
11+
use OCP\DB\QueryBuilder\IQueryBuilder;
12+
use OCP\DB\Types;
13+
use OCP\IDBConnection;
14+
use OCP\IUserManager;
15+
use OCP\Migration\IOutput;
16+
use OCP\Migration\SimpleMigrationStep;
17+
use Psr\Log\LoggerInterface;
18+
19+
class Version1120Date20210917155206 extends SimpleMigrationStep {
20+
21+
/** @var IDBConnection */
22+
private $dbc;
23+
/** @var IUserManager */
24+
private $userManager;
25+
/** @var LoggerInterface */
26+
private $logger;
27+
28+
public function __construct(IDBConnection $dbc, IUserManager $userManager, LoggerInterface $logger) {
29+
$this->dbc = $dbc;
30+
$this->userManager = $userManager;
31+
$this->logger = $logger;
32+
}
33+
34+
public function getName() {
35+
return 'Adjust LDAP user and group id column lengths to match server lengths';
36+
}
37+
38+
/**
39+
* @param IOutput $output
40+
* @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
41+
* @param array $options
42+
*/
43+
public function preSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void {
44+
// ensure that there is no user or group id longer than 64char in LDAP table
45+
$this->handleIDs('ldap_group_mapping', false);
46+
$this->handleIDs('ldap_user_mapping', true);
47+
}
48+
49+
/**
50+
* @param IOutput $output
51+
* @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
52+
* @param array $options
53+
* @return null|ISchemaWrapper
54+
*/
55+
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
56+
/** @var ISchemaWrapper $schema */
57+
$schema = $schemaClosure();
58+
59+
$changeSchema = false;
60+
foreach (['ldap_user_mapping', 'ldap_group_mapping'] as $tableName) {
61+
$table = $schema->getTable($tableName);
62+
$column = $table->getColumn('owncloud_name');
63+
if ($column->getLength() > 64) {
64+
$column->setLength(64);
65+
$changeSchema = true;
66+
}
67+
}
68+
69+
return $changeSchema ? $schema : null;
70+
}
71+
72+
protected function handleIDs(string $table, bool $emitHooks) {
73+
$q = $this->getSelectQuery($table);
74+
$u = $this->getUpdateQuery($table);
75+
76+
$r = $q->executeQuery();
77+
while ($row = $r->fetch()) {
78+
$newId = hash('sha256', $row['owncloud_name'], false);
79+
if ($emitHooks) {
80+
$this->emitUnassign($row['owncloud_name'], true);
81+
}
82+
$u->setParameter('uuid', $row['directory_uuid']);
83+
$u->setParameter('newId', $newId);
84+
try {
85+
$u->executeStatement();
86+
if ($emitHooks) {
87+
$this->emitUnassign($row['owncloud_name'], false);
88+
$this->emitAssign($newId);
89+
}
90+
} catch (Exception $e) {
91+
$this->logger->error('Failed to shorten owncloud_name "{oldId}" to "{newId}" (UUID: "{uuid}" of {table})',
92+
[
93+
'app' => 'user_ldap',
94+
'oldId' => $row['owncloud_name'],
95+
'newId' => $newId,
96+
'uuid' => $row['directory_uuid'],
97+
'table' => $table,
98+
'exception' => $e,
99+
]
100+
);
101+
}
102+
}
103+
$r->closeCursor();
104+
}
105+
106+
protected function getSelectQuery(string $table): IQueryBuilder {
107+
$q = $this->dbc->getQueryBuilder();
108+
$q->select('owncloud_name', 'directory_uuid')
109+
->from($table)
110+
->where($q->expr()->like('owncloud_name', $q->createNamedParameter(str_repeat('_', 65) . '%'), Types::STRING));
111+
return $q;
112+
}
113+
114+
protected function getUpdateQuery(string $table): IQueryBuilder {
115+
$q = $this->dbc->getQueryBuilder();
116+
$q->update($table)
117+
->set('owncloud_name', $q->createParameter('newId'))
118+
->where($q->expr()->eq('directory_uuid', $q->createParameter('uuid')));
119+
return $q;
120+
}
121+
122+
protected function emitUnassign(string $oldId, bool $pre): void {
123+
if ($this->userManager instanceof PublicEmitter) {
124+
$this->userManager->emit('\OC\User', $pre ? 'pre' : 'post' . 'UnassignedUserId', [$oldId]);
125+
}
126+
}
127+
128+
protected function emitAssign(string $newId): void {
129+
if ($this->userManager instanceof PublicEmitter) {
130+
$this->userManager->emit('\OC\User', 'assignedUserId', [$newId]);
131+
}
132+
}
133+
}

apps/user_ldap/tests/AccessTest.php

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -695,7 +695,28 @@ public function intUsernameProvider() {
695695
['epost@poste.test', 'epost@poste.test'],
696696
['fränk', $translitExpected],
697697
[' gerda ', 'gerda'],
698-
['🕱🐵🐘🐑', null]
698+
['🕱🐵🐘🐑', null],
699+
[
700+
'OneNameToRuleThemAllOneNameToFindThemOneNameToBringThemAllAndInTheDarknessBindThem',
701+
'81ff71b5dd0f0092e2dc977b194089120093746e273f2ef88c11003762783127'
702+
]
703+
];
704+
}
705+
706+
public function groupIDCandidateProvider() {
707+
return [
708+
['alice', 'alice'],
709+
['b/ob', 'b/ob'],
710+
['charly🐬', 'charly🐬'],
711+
['debo rah', 'debo rah'],
712+
['epost@poste.test', 'epost@poste.test'],
713+
['fränk', 'fränk'],
714+
[' gerda ', 'gerda'],
715+
['🕱🐵🐘🐑', '🕱🐵🐘🐑'],
716+
[
717+
'OneNameToRuleThemAllOneNameToFindThemOneNameToBringThemAllAndInTheDarknessBindThem',
718+
'81ff71b5dd0f0092e2dc977b194089120093746e273f2ef88c11003762783127'
719+
]
699720
];
700721
}
701722

@@ -716,6 +737,14 @@ public function testSanitizeUsername($name, $expected) {
716737
$this->assertSame($expected, $sanitizedName);
717738
}
718739

740+
/**
741+
* @dataProvider groupIDCandidateProvider
742+
*/
743+
public function testSanitizeGroupIDCandidate(string $name, string $expected) {
744+
$sanitizedName = $this->access->sanitizeGroupIDCandidate($name);
745+
$this->assertSame($expected, $sanitizedName);
746+
}
747+
719748
public function testUserStateUpdate() {
720749
$this->connection->expects($this->any())
721750
->method('__get')

0 commit comments

Comments
 (0)