Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable21] ensure that user and group IDs in LDAP's tables are also max 64chars #28969

Merged
merged 1 commit into from
Sep 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/user_ldap/appinfo/info.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
1 change: 1 addition & 0 deletions apps/user_ldap/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
'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',
Expand Down
1 change: 1 addition & 0 deletions apps/user_ldap/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
4 changes: 2 additions & 2 deletions apps/user_ldap/composer/composer/installed.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
'type' => 'library',
'install_path' => __DIR__ . '/../',
'aliases' => array(),
'reference' => '7b056b2ed72b2a7a5937e0c251858e1906277b4b',
'reference' => 'add48a086275dcfb604556dbda95503345f57292',
'name' => '__root__',
'dev' => false,
),
Expand All @@ -16,7 +16,7 @@
'type' => 'library',
'install_path' => __DIR__ . '/../',
'aliases' => array(),
'reference' => '7b056b2ed72b2a7a5937e0c251858e1906277b4b',
'reference' => 'add48a086275dcfb604556dbda95503345f57292',
'dev_requirement' => false,
),
),
Expand Down
25 changes: 24 additions & 1 deletion apps/user_ldap/lib/Access.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@
use OCP\IConfig;
use OCP\ILogger;
use OCP\IUserManager;
use function strlen;
use function substr;

/**
* Class Access
Expand Down Expand Up @@ -579,7 +581,7 @@ public function dn2ocname($fdn, $ldapName = null, $isUser = true, &$newlyMapped
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
Expand Down Expand Up @@ -838,6 +840,11 @@ private function _createAltInternalOwnCloudNameForGroups($name) {
* @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) {
Expand Down Expand Up @@ -1432,13 +1439,29 @@ public function sanitizeUsername($name) {
// 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');
}

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
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
]);
$table->addColumn('owncloud_name', Types::STRING, [
'notnull' => true,
'length' => 255,
'length' => 64,
'default' => '',
]);
$table->addColumn('directory_uuid', Types::STRING, [
Expand All @@ -73,7 +73,7 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
]);
$table->addColumn('owncloud_name', Types::STRING, [
'notnull' => true,
'length' => 255,
'length' => 64,
'default' => '',
]);
$table->addColumn('directory_uuid', Types::STRING, [
Expand Down
133 changes: 133 additions & 0 deletions apps/user_ldap/lib/Migration/Version1120Date20210917155206.php
Original file line number Diff line number Diff line change
@@ -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]);
}
}
}
31 changes: 30 additions & 1 deletion apps/user_ldap/tests/AccessTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,28 @@ public function intUsernameProvider() {
['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'
]
];
}

Expand All @@ -718,6 +739,14 @@ public function testSanitizeUsername($name, $expected) {
$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')
Expand Down