mirror of https://github.com/nextcloud/server
fix(groups): allows to save group names with more than 64 characters
Mimic behaviour from LDAP users and add a hard limit to 255 characters Signed-off-by: Benjamin Gaussorgues <benjamin.gaussorgues@nextcloud.com>
This commit is contained in:
parent
5fffbcfe86
commit
7a6b1f8ae8
|
@ -461,6 +461,7 @@ return array(
|
|||
'OCP\\Group\\Backend\\ICountDisabledInGroup' => $baseDir . '/lib/public/Group/Backend/ICountDisabledInGroup.php',
|
||||
'OCP\\Group\\Backend\\ICountUsersBackend' => $baseDir . '/lib/public/Group/Backend/ICountUsersBackend.php',
|
||||
'OCP\\Group\\Backend\\ICreateGroupBackend' => $baseDir . '/lib/public/Group/Backend/ICreateGroupBackend.php',
|
||||
'OCP\\Group\\Backend\\ICreateNamedGroupBackend' => $baseDir . '/lib/public/Group/Backend/ICreateNamedGroupBackend.php',
|
||||
'OCP\\Group\\Backend\\IDeleteGroupBackend' => $baseDir . '/lib/public/Group/Backend/IDeleteGroupBackend.php',
|
||||
'OCP\\Group\\Backend\\IGetDisplayNameBackend' => $baseDir . '/lib/public/Group/Backend/IGetDisplayNameBackend.php',
|
||||
'OCP\\Group\\Backend\\IGroupDetailsBackend' => $baseDir . '/lib/public/Group/Backend/IGroupDetailsBackend.php',
|
||||
|
|
|
@ -494,6 +494,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
|
|||
'OCP\\Group\\Backend\\ICountDisabledInGroup' => __DIR__ . '/../../..' . '/lib/public/Group/Backend/ICountDisabledInGroup.php',
|
||||
'OCP\\Group\\Backend\\ICountUsersBackend' => __DIR__ . '/../../..' . '/lib/public/Group/Backend/ICountUsersBackend.php',
|
||||
'OCP\\Group\\Backend\\ICreateGroupBackend' => __DIR__ . '/../../..' . '/lib/public/Group/Backend/ICreateGroupBackend.php',
|
||||
'OCP\\Group\\Backend\\ICreateNamedGroupBackend' => __DIR__ . '/../../..' . '/lib/public/Group/Backend/ICreateNamedGroupBackend.php',
|
||||
'OCP\\Group\\Backend\\IDeleteGroupBackend' => __DIR__ . '/../../..' . '/lib/public/Group/Backend/IDeleteGroupBackend.php',
|
||||
'OCP\\Group\\Backend\\IGetDisplayNameBackend' => __DIR__ . '/../../..' . '/lib/public/Group/Backend/IGetDisplayNameBackend.php',
|
||||
'OCP\\Group\\Backend\\IGroupDetailsBackend' => __DIR__ . '/../../..' . '/lib/public/Group/Backend/IGroupDetailsBackend.php',
|
||||
|
|
|
@ -37,7 +37,7 @@ use OCP\Group\Backend\IAddToGroupBackend;
|
|||
use OCP\Group\Backend\IBatchMethodsBackend;
|
||||
use OCP\Group\Backend\ICountDisabledInGroup;
|
||||
use OCP\Group\Backend\ICountUsersBackend;
|
||||
use OCP\Group\Backend\ICreateGroupBackend;
|
||||
use OCP\Group\Backend\ICreateNamedGroupBackend;
|
||||
use OCP\Group\Backend\IDeleteGroupBackend;
|
||||
use OCP\Group\Backend\IGetDisplayNameBackend;
|
||||
use OCP\Group\Backend\IGroupDetailsBackend;
|
||||
|
@ -55,7 +55,7 @@ class Database extends ABackend implements
|
|||
IAddToGroupBackend,
|
||||
ICountDisabledInGroup,
|
||||
ICountUsersBackend,
|
||||
ICreateGroupBackend,
|
||||
ICreateNamedGroupBackend,
|
||||
IDeleteGroupBackend,
|
||||
IGetDisplayNameBackend,
|
||||
IGroupDetailsBackend,
|
||||
|
@ -86,35 +86,28 @@ class Database extends ABackend implements
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Try to create a new group
|
||||
* @param string $gid The name of the group to create
|
||||
* @return bool
|
||||
*
|
||||
* Tries to create a new group. If the group name already exists, false will
|
||||
* be returned.
|
||||
*/
|
||||
public function createGroup(string $gid): bool {
|
||||
public function createGroup(string $name): ?string {
|
||||
$this->fixDI();
|
||||
|
||||
$gid = $this->computeGid($name);
|
||||
try {
|
||||
// Add group
|
||||
$builder = $this->dbConn->getQueryBuilder();
|
||||
$result = $builder->insert('groups')
|
||||
->setValue('gid', $builder->createNamedParameter($gid))
|
||||
->setValue('displayname', $builder->createNamedParameter($gid))
|
||||
->setValue('displayname', $builder->createNamedParameter($name))
|
||||
->execute();
|
||||
} catch (UniqueConstraintViolationException $e) {
|
||||
$result = 0;
|
||||
return null;
|
||||
}
|
||||
|
||||
// Add to cache
|
||||
$this->groupCache[$gid] = [
|
||||
'gid' => $gid,
|
||||
'displayname' => $gid
|
||||
'displayname' => $name
|
||||
];
|
||||
|
||||
return $result === 1;
|
||||
return $gid;
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -595,4 +588,13 @@ class Database extends ABackend implements
|
|||
public function getBackendName(): string {
|
||||
return 'Database';
|
||||
}
|
||||
|
||||
/**
|
||||
* Compute group ID from display name (GIDs are limited to 64 characters in database)
|
||||
*/
|
||||
private function computeGid(string $displayName): string {
|
||||
return mb_strlen($displayName) > 64
|
||||
? hash('sha256', $displayName)
|
||||
: $displayName;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -43,6 +43,7 @@ namespace OC\Group;
|
|||
use OC\Hooks\PublicEmitter;
|
||||
use OCP\EventDispatcher\IEventDispatcher;
|
||||
use OCP\Group\Backend\IBatchMethodsBackend;
|
||||
use OCP\Group\Backend\ICreateNamedGroupBackend;
|
||||
use OCP\Group\Backend\IGroupDetailsBackend;
|
||||
use OCP\Group\Events\BeforeGroupCreatedEvent;
|
||||
use OCP\Group\Events\GroupCreatedEvent;
|
||||
|
@ -89,6 +90,8 @@ class Manager extends PublicEmitter implements IGroupManager {
|
|||
|
||||
private DisplayNameCache $displayNameCache;
|
||||
|
||||
private const MAX_GROUP_LENGTH = 255;
|
||||
|
||||
public function __construct(\OC\User\Manager $userManager,
|
||||
IEventDispatcher $dispatcher,
|
||||
LoggerInterface $logger,
|
||||
|
@ -281,12 +284,22 @@ class Manager extends PublicEmitter implements IGroupManager {
|
|||
return null;
|
||||
} elseif ($group = $this->get($gid)) {
|
||||
return $group;
|
||||
} elseif (mb_strlen($gid) > self::MAX_GROUP_LENGTH) {
|
||||
throw new \Exception('Group name is limited to '. self::MAX_GROUP_LENGTH.' characters');
|
||||
} else {
|
||||
$this->dispatcher->dispatchTyped(new BeforeGroupCreatedEvent($gid));
|
||||
$this->emit('\OC\Group', 'preCreate', [$gid]);
|
||||
foreach ($this->backends as $backend) {
|
||||
if ($backend->implementsActions(Backend::CREATE_GROUP)) {
|
||||
if ($backend->createGroup($gid)) {
|
||||
if ($backend instanceof ICreateNamedGroupBackend) {
|
||||
$groupName = $gid;
|
||||
if (($gid = $backend->createGroup($groupName)) !== null) {
|
||||
$group = $this->getGroupObject($gid);
|
||||
$this->dispatcher->dispatchTyped(new GroupCreatedEvent($group));
|
||||
$this->emit('\OC\Group', 'postCreate', [$group]);
|
||||
return $group;
|
||||
}
|
||||
} elseif ($backend->createGroup($gid)) {
|
||||
$group = $this->getGroupObject($gid);
|
||||
$this->dispatcher->dispatchTyped(new GroupCreatedEvent($group));
|
||||
$this->emit('\OC\Group', 'postCreate', [$group]);
|
||||
|
|
|
@ -48,7 +48,7 @@ abstract class ABackend implements GroupInterface, IBatchMethodsBackend {
|
|||
if ($this instanceof ICountUsersBackend) {
|
||||
$implements |= GroupInterface::COUNT_USERS;
|
||||
}
|
||||
if ($this instanceof ICreateGroupBackend) {
|
||||
if ($this instanceof ICreateGroupBackend || $this instanceof ICreateNamedGroupBackend) {
|
||||
$implements |= GroupInterface::CREATE_GROUP;
|
||||
}
|
||||
if ($this instanceof IDeleteGroupBackend) {
|
||||
|
|
|
@ -27,6 +27,7 @@ namespace OCP\Group\Backend;
|
|||
|
||||
/**
|
||||
* @since 14.0.0
|
||||
* @deprecated 30.0.0 Use ICreateNamedGroupBackend instead
|
||||
*/
|
||||
interface ICreateGroupBackend {
|
||||
/**
|
||||
|
|
|
@ -0,0 +1,43 @@
|
|||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
/**
|
||||
* @copyright Copyright (c) 2024 Benjamin Gaussorgues <benjamin.gaussorgues@nextcloud.com>
|
||||
*
|
||||
* @author Benjamin Gaussorgues <benjamin.gaussorgues@nextcloud.com>
|
||||
*
|
||||
* @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 <http://www.gnu.org/licenses/>.
|
||||
*
|
||||
*/
|
||||
namespace OCP\Group\Backend;
|
||||
|
||||
/**
|
||||
* @since 30.0.0
|
||||
*/
|
||||
interface ICreateNamedGroupBackend {
|
||||
/**
|
||||
* Tries to create a group from its name.
|
||||
*
|
||||
* If group name already exists, null is returned.
|
||||
* Otherwise, new group ID is returned.
|
||||
*
|
||||
* @param string $name Group name
|
||||
* @return ?string Group ID in case of success, null in case of failure
|
||||
* @since 30.0.0
|
||||
*/
|
||||
public function createGroup(string $name): ?string;
|
||||
}
|
|
@ -36,10 +36,8 @@ class DatabaseTest extends Backend {
|
|||
/**
|
||||
* get a new unique group name
|
||||
* test cases can override this in order to clean up created groups
|
||||
*
|
||||
* @return string
|
||||
*/
|
||||
public function getGroupName($name = null) {
|
||||
public function getGroupName($name = null): string {
|
||||
$name = parent::getGroupName($name);
|
||||
$this->groups[] = $name;
|
||||
return $name;
|
||||
|
@ -57,12 +55,22 @@ class DatabaseTest extends Backend {
|
|||
parent::tearDown();
|
||||
}
|
||||
|
||||
public function testAddDoubleNoCache() {
|
||||
public function testAddDoubleNoCache(): void {
|
||||
$group = $this->getGroupName();
|
||||
|
||||
$this->backend->createGroup($group);
|
||||
|
||||
$backend = new \OC\Group\Database();
|
||||
$this->assertFalse($backend->createGroup($group));
|
||||
$this->assertNull($backend->createGroup($group));
|
||||
}
|
||||
|
||||
public function testAddLongGroupName(): void {
|
||||
$groupName = $this->getUniqueID('test_', 100);
|
||||
|
||||
$gidCreated = $this->backend->createGroup($groupName);
|
||||
$this->assertEquals(64, strlen($gidCreated));
|
||||
|
||||
$group = $this->backend->getGroupDetails($gidCreated);
|
||||
$this->assertEquals(['displayName' => $groupName], $group);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -241,6 +241,30 @@ class ManagerTest extends TestCase {
|
|||
$this->assertEquals(null, $group);
|
||||
}
|
||||
|
||||
public function testCreateTooLong() {
|
||||
/**@var \PHPUnit\Framework\MockObject\MockObject|\OC\Group\Backend $backend */
|
||||
$backendGroupCreated = false;
|
||||
$backend = $this->getTestBackend(
|
||||
GroupInterface::ADD_TO_GROUP |
|
||||
GroupInterface::REMOVE_FROM_GOUP |
|
||||
GroupInterface::COUNT_USERS |
|
||||
GroupInterface::CREATE_GROUP |
|
||||
GroupInterface::DELETE_GROUP |
|
||||
GroupInterface::GROUP_DETAILS
|
||||
);
|
||||
$groupName = str_repeat('x', 256);
|
||||
$backend->expects($this->any())
|
||||
->method('groupExists')
|
||||
->with($groupName)
|
||||
->willReturn(false);
|
||||
|
||||
$manager = new \OC\Group\Manager($this->userManager, $this->dispatcher, $this->logger, $this->cache);
|
||||
$manager->addBackend($backend);
|
||||
|
||||
$this->expectException(\Exception::class);
|
||||
$group = $manager->createGroup($groupName);
|
||||
}
|
||||
|
||||
public function testCreateExists() {
|
||||
/** @var \PHPUnit\Framework\MockObject\MockObject|\OC\Group\Backend $backend */
|
||||
$backend = $this->getTestBackend();
|
||||
|
|
Loading…
Reference in New Issue