From 0f0afebe616669badcc6b4934dfeab36697cbd47 Mon Sep 17 00:00:00 2001 From: ernolf Date: Fri, 3 Jul 2026 19:08:25 +0200 Subject: [PATCH 1/3] feat: convert regular accounts to guests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - add "Convert to guest account" for regular, never-logged-in database accounts (admin … menu action + OCS endpoint), keeping the login name and password - add ConversionService for the transactional users -> guests_users move; extract GuestManager::setGuestQuota so converted accounts get the default guest quota - allow a free-form guest login name via `occ guests:add --uid` instead of deriving the user ID from the email address - loosen the guests backend user-id guard to accept any non-empty id Closes #1153 Assisted-by: ClaudeCode:claude-opus-4-8 Signed-off-by: ernolf --- appinfo/routes.php | 5 + lib/Command/AddCommand.php | 20 +++- lib/Controller/UsersController.php | 53 +++++++++ lib/GuestManager.php | 9 +- lib/Service/ConversionService.php | 81 +++++++++++++ lib/UserBackend.php | 13 +-- src/components/ConvertToGuestDialog.vue | 109 ++++++++++++++++++ src/users.ts | 33 +++++- tests/unit/Command/AddCommandTest.php | 45 ++++++++ tests/unit/Controller/UsersControllerTest.php | 68 ++++++++++- tests/unit/Service/ConversionServiceTest.php | 108 +++++++++++++++++ tests/unit/UserBackendTest.php | 15 +++ 12 files changed, 541 insertions(+), 18 deletions(-) create mode 100644 lib/Service/ConversionService.php create mode 100644 src/components/ConvertToGuestDialog.vue create mode 100644 tests/unit/Service/ConversionServiceTest.php diff --git a/appinfo/routes.php b/appinfo/routes.php index cb1e71ca..561412a1 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -50,6 +50,11 @@ 'url' => '/api/v1/transfer', 'verb' => 'POST', ], + [ + 'name' => 'users#convert', + 'url' => '/api/v1/convert', + 'verb' => 'POST', + ], [ 'name' => 'API#languages', 'url' => '/api/v1/languages', diff --git a/lib/Command/AddCommand.php b/lib/Command/AddCommand.php index 711d73f6..5eda87ee 100644 --- a/lib/Command/AddCommand.php +++ b/lib/Command/AddCommand.php @@ -48,6 +48,12 @@ protected function configure(): void { InputArgument::REQUIRED, 'Email address' ) + ->addOption( + 'uid', + null, + InputOption::VALUE_REQUIRED, + 'Login name (user ID) for the guest. If omitted, the email address is used (hashed when the privacy setting is enabled).' + ) ->addOption( 'generate-password', null, @@ -86,11 +92,15 @@ protected function execute(InputInterface $input, OutputInterface $output): int } $email = $input->getArgument('email'); - if ($this->config->useHashedEmailAsUserID()) { - $email = strtolower($email); - $uid = hash('sha256', $email); - } else { - $uid = $email; + + $uid = $input->getOption('uid'); + if ($uid === null || $uid === '') { + if ($this->config->useHashedEmailAsUserID()) { + $email = strtolower($email); + $uid = hash('sha256', $email); + } else { + $uid = $email; + } } // same behavior like in the UsersController diff --git a/lib/Controller/UsersController.php b/lib/Controller/UsersController.php index 441eab21..b8737ab7 100644 --- a/lib/Controller/UsersController.php +++ b/lib/Controller/UsersController.php @@ -14,6 +14,7 @@ use OCA\Guests\Db\Transfer; use OCA\Guests\Db\TransferMapper; use OCA\Guests\GuestManager; +use OCA\Guests\Service\ConversionService; use OCA\Guests\Service\InviteService; use OCA\Guests\TransferService; use OCP\AppFramework\Db\DoesNotExistException; @@ -29,6 +30,7 @@ use OCP\IUserManager; use OCP\IUserSession; use OCP\Mail\IMailer; +use Psr\Log\LoggerInterface; class UsersController extends OCSController { public function __construct( @@ -45,6 +47,8 @@ public function __construct( private readonly TransferService $transferService, private readonly TransferMapper $transferMapper, private readonly InviteService $inviteService, + private readonly ConversionService $conversionService, + private readonly LoggerInterface $logger, ) { parent::__construct($appName, $request); } @@ -245,4 +249,53 @@ public function transfer(string $guestUserId, string $targetUserId): DataRespons return new DataResponse([], Http::STATUS_CREATED); } + + /** + * Convert a regular, never-logged-in account into a guest account + */ + public function convert(string $userId): DataResponse { + $author = $this->userSession->getUser(); + if (!($author instanceof IUser)) { + return new DataResponse([ + 'message' => $this->l10n->t('Failed to authorize') + ], Http::STATUS_UNAUTHORIZED); + } + + $user = $this->userManager->get($userId); + if (!($user instanceof IUser)) { + return new DataResponse([ + 'message' => $this->l10n->t('Account not found') + ], Http::STATUS_NOT_FOUND); + } + + if ($this->guestManager->isGuest($user)) { + return new DataResponse([ + 'message' => $this->l10n->t('Account is already a guest') + ], Http::STATUS_CONFLICT); + } + + if ($user->getBackendClassName() !== 'Database') { + return new DataResponse([ + 'message' => $this->l10n->t('Only regular accounts can be converted to guests') + ], Http::STATUS_CONFLICT); + } + + if ($user->getLastLogin() !== 0) { + return new DataResponse([ + 'message' => $this->l10n->t('Only accounts that have never logged in can be converted') + ], Http::STATUS_CONFLICT); + } + + try { + $this->conversionService->convertToGuest($user, $author); + $this->guestManager->setGuestQuota($user); + } catch (\Throwable $e) { + $this->logger->error('Failed to convert account "' . $userId . '" to a guest', ['exception' => $e]); + return new DataResponse([ + 'message' => $this->l10n->t('An error occurred while converting the account') + ], Http::STATUS_INTERNAL_SERVER_ERROR); + } + + return new DataResponse([]); + } } diff --git a/lib/GuestManager.php b/lib/GuestManager.php index 9f9cba12..e4f09869 100644 --- a/lib/GuestManager.php +++ b/lib/GuestManager.php @@ -111,11 +111,18 @@ public function createGuest(?IUser $createdBy, string $userId, string $email, st ); } - $user->setQuota($this->appConfig->getAppValueString(ConfigLexicon::GUEST_DISK_QUOTA)); + $this->setGuestQuota($user); return $user; } + /** + * Apply the configured default guest quota to an account. + */ + public function setGuestQuota(IUser $user): void { + $user->setQuota($this->appConfig->getAppValueString(ConfigLexicon::GUEST_DISK_QUOTA)); + } + /** * @return list */ diff --git a/lib/Service/ConversionService.php b/lib/Service/ConversionService.php new file mode 100644 index 00000000..25487922 --- /dev/null +++ b/lib/Service/ConversionService.php @@ -0,0 +1,81 @@ +getUID(); + $uidLower = mb_strtolower($uid); + $displayName = $user->getDisplayName(); + $email = $user->getSystemEMailAddress() ?? ''; + + // Carry over the existing password hash from the database backend. + $query = $this->connection->getQueryBuilder(); + $query->select('password') + ->from('users') + ->where($query->expr()->eq('uid_lower', $query->createNamedParameter($uidLower))); + $result = $query->executeQuery(); + $passwordHash = $result->fetchOne(); + $result->closeCursor(); + if ($passwordHash === false) { + throw new \RuntimeException('No password hash found for "' . $uid . '"'); + } + + // Move the account between backends in a single transaction. The user ID + // is unchanged, so all data keyed by it (account, home storage, mounts, + // shares) stays valid. + $this->connection->beginTransaction(); + try { + $insert = $this->connection->getQueryBuilder(); + $insert->insert('guests_users') + ->values([ + 'uid' => $insert->createNamedParameter($uid), + 'uid_lower' => $insert->createNamedParameter($uidLower), + 'displayname' => $insert->createNamedParameter($displayName), + 'password' => $insert->createNamedParameter($passwordHash), + 'email' => $insert->createNamedParameter($email), + ]); + $insert->executeStatement(); + + $delete = $this->connection->getQueryBuilder(); + $delete->delete('users') + ->where($delete->expr()->eq('uid_lower', $delete->createNamedParameter($uidLower))); + $delete->executeStatement(); + + $this->connection->commit(); + } catch (\Throwable $e) { + $this->connection->rollBack(); + throw $e; + } + + $this->userConfig->setValueString($uid, Application::APP_ID, ConfigLexicon::USER_CREATED_BY, $createdBy->getUID()); + } +} diff --git a/lib/UserBackend.php b/lib/UserBackend.php index de51da50..d3114b4b 100644 --- a/lib/UserBackend.php +++ b/lib/UserBackend.php @@ -330,8 +330,7 @@ private function loadUser($uid): bool { return false; } - // guests $uid could be NULL or '' - // or is not an email anyway + // Skip empty IDs; any non-empty ID is resolved against the guests_users table. if (!$this->potentialGuestUserId($uid)) { $this->cache[$uid] = false; return false; @@ -473,14 +472,10 @@ public function getRealUID(string $uid): string { } /** - * Guest app user ids are: - * - either email addresses so they need to contain an @ - * - lowercase sha256 hashes of email addresses, 64 characters of a-f and 0-9 - * - * @param string $userId - * @return bool + * Guard against empty IDs only. Any non-empty ID may belong to a guest and is + * resolved against the guests_users table. */ protected function potentialGuestUserId(string $userId): bool { - return str_contains($userId, '@') || preg_match('/^[a-f0-9]{64}$/', $userId); + return $userId !== ''; } } diff --git a/src/components/ConvertToGuestDialog.vue b/src/components/ConvertToGuestDialog.vue new file mode 100644 index 00000000..bedc182f --- /dev/null +++ b/src/components/ConvertToGuestDialog.vue @@ -0,0 +1,109 @@ + + + + + + diff --git a/src/users.ts b/src/users.ts index 4ef2f240..57bd6396 100644 --- a/src/users.ts +++ b/src/users.ts @@ -5,11 +5,13 @@ import type { User } from './types.ts' +import SvgAccountArrowLeft from '@mdi/svg/svg/account-arrow-left.svg?raw' import SvgAccountArrowRight from '@mdi/svg/svg/account-arrow-right.svg?raw' import { showSuccess } from '@nextcloud/dialogs' import { subscribe } from '@nextcloud/event-bus' import { translate as t } from '@nextcloud/l10n' import { spawnDialog } from '@nextcloud/vue/functions/dialog' +import ConvertToGuestDialog from './components/ConvertToGuestDialog.vue' import TransferGuestDialog from './components/TransferGuestDialog.vue' /** @@ -31,7 +33,28 @@ function transferGuest(_event: MouseEvent, user: User): void { }, onClose) } -const enabled = (user: User) => user?.backend === 'Guests' +/** + * + * @param _event unused click event + * @param user the account from the user-management list + */ +function convertToGuest(_event: MouseEvent, user: User): void { + const onClose = (userId: null | string) => { + if (userId === null) { + return + } + showSuccess(t('guests', 'Account "{userId}" was converted into a guest account', { userId })) + window.location.reload() + } + + spawnDialog(ConvertToGuestDialog, { + user, + // @ts-expect-error callback parameters are known + }, onClose) +} + +const isGuest = (user: User) => user?.backend === 'Guests' +const isConvertibleAccount = (user: User) => user?.backend === 'Database' && !user?.lastLogin /** * @@ -41,7 +64,13 @@ function registerAction() { SvgAccountArrowRight, t('guests', 'Convert guest to regular account'), transferGuest, - enabled, + isGuest, + ) + window.OCA.Settings.UserList.registerAction( + SvgAccountArrowLeft, + t('guests', 'Convert to guest account'), + convertToGuest, + isConvertibleAccount, ) } diff --git a/tests/unit/Command/AddCommandTest.php b/tests/unit/Command/AddCommandTest.php index d904713c..b8c89e73 100644 --- a/tests/unit/Command/AddCommandTest.php +++ b/tests/unit/Command/AddCommandTest.php @@ -186,4 +186,49 @@ public function testCreateGuestInvalidEmail(): void { $this->assertStringContainsString('Invalid email address "guestid@@@".', $output); $this->assertEquals(1, $this->commandTester->getStatusCode()); } + + public function testCreateGuestWithCustomUid(): void { + $createdByUser = $this->createStub(IUser::class); + + $guestUser = $this->createMock(IUser::class); + $guestUser->method('getUID')->willReturn('karl'); + + $this->userManager->expects($this->once()) + ->method('get') + ->with('creator') + ->willReturn($createdByUser); + + // The explicit login name is used as the user ID, not the email or its hash. + $this->userManager->expects($this->once()) + ->method('userExists') + ->with('karl') + ->willReturn(false); + + $this->mailer->expects($this->once()) + ->method('validateMailAddress') + ->with('guestid@example.com') + ->willReturn(true); + + $this->guestManager->expects($this->once()) + ->method('createGuest') + ->with( + $createdByUser, + 'karl', + 'guestid@example.com', + '', + '', + null + ) + ->willReturn($guestUser); + + $this->commandTester->execute([ + 'created-by' => 'creator', + 'email' => 'guestid@example.com', + '--uid' => 'karl', + '--generate-password' => true, + ]); + + $output = $this->commandTester->getDisplay(); + $this->assertStringContainsString('The guest account user "karl" was created successfully', $output); + } } diff --git a/tests/unit/Controller/UsersControllerTest.php b/tests/unit/Controller/UsersControllerTest.php index e47ada89..a277a657 100644 --- a/tests/unit/Controller/UsersControllerTest.php +++ b/tests/unit/Controller/UsersControllerTest.php @@ -13,6 +13,7 @@ use OCA\Guests\Controller\UsersController; use OCA\Guests\Db\TransferMapper; use OCA\Guests\GuestManager; +use OCA\Guests\Service\ConversionService; use OCA\Guests\Service\InviteService; use OCA\Guests\TransferService; use OCP\AppFramework\Http; @@ -29,6 +30,7 @@ use OCP\IUserSession; use OCP\Mail\IMailer; use PHPUnit\Framework\MockObject\MockObject; +use Psr\Log\LoggerInterface; use Test\TestCase; class UsersControllerTest extends TestCase { @@ -46,6 +48,8 @@ class UsersControllerTest extends TestCase { private IAppConfig&MockObject $appConfig; private IConfig&MockObject $config; private InviteService&MockObject $inviteService; + private ConversionService&MockObject $conversionService; + private LoggerInterface&MockObject $logger; private Config $guestsConfig; private UsersController $controller; @@ -67,6 +71,8 @@ protected function setUp(): void { $this->appConfig = $this->createMock(IAppConfig::class); $this->config = $this->createMock(IConfig::class); $this->inviteService = $this->createMock(InviteService::class); + $this->conversionService = $this->createMock(ConversionService::class); + $this->logger = $this->createMock(LoggerInterface::class); $this->guestsConfig = new Config( $this->config, @@ -93,7 +99,9 @@ protected function setUp(): void { $this->groupManager, $this->transferService, $this->transferMapper, - $this->inviteService + $this->inviteService, + $this->conversionService, + $this->logger ); } @@ -752,4 +760,62 @@ public function testCreateException(): void { $this->assertEquals(Http::STATUS_UNPROCESSABLE_ENTITY, $response->getStatus()); $this->assertEquals(['errorMessages' => ['email' => 'Error creating guest']], $response->getData()); } + + public function testConvertSucceeds(): void { + $author = $this->createMock(IUser::class); + $this->userSession->method('getUser')->willReturn($author); + + $user = $this->createMock(IUser::class); + $user->method('getBackendClassName')->willReturn('Database'); + $user->method('getLastLogin')->willReturn(0); + + $this->userManager->method('get')->with('karl')->willReturn($user); + $this->guestManager->method('isGuest')->with($user)->willReturn(false); + + $this->conversionService->expects($this->once()) + ->method('convertToGuest') + ->with($user, $author); + + $this->guestManager->expects($this->once()) + ->method('setGuestQuota') + ->with($user); + + $response = $this->controller->convert('karl'); + $this->assertEquals(Http::STATUS_OK, $response->getStatus()); + } + + public function testConvertAccountNotFound(): void { + $this->userSession->method('getUser')->willReturn($this->createMock(IUser::class)); + $this->userManager->method('get')->with('karl')->willReturn(null); + + $response = $this->controller->convert('karl'); + $this->assertEquals(Http::STATUS_NOT_FOUND, $response->getStatus()); + } + + public function testConvertAlreadyGuest(): void { + $this->userSession->method('getUser')->willReturn($this->createMock(IUser::class)); + + $user = $this->createMock(IUser::class); + $this->userManager->method('get')->with('karl')->willReturn($user); + $this->guestManager->method('isGuest')->with($user)->willReturn(true); + + $response = $this->controller->convert('karl'); + $this->assertEquals(Http::STATUS_CONFLICT, $response->getStatus()); + } + + public function testConvertRejectsAccountThatLoggedIn(): void { + $this->userSession->method('getUser')->willReturn($this->createMock(IUser::class)); + + $user = $this->createMock(IUser::class); + $user->method('getBackendClassName')->willReturn('Database'); + $user->method('getLastLogin')->willReturn(1700000000); + + $this->userManager->method('get')->with('karl')->willReturn($user); + $this->guestManager->method('isGuest')->with($user)->willReturn(false); + + $this->conversionService->expects($this->never())->method('convertToGuest'); + + $response = $this->controller->convert('karl'); + $this->assertEquals(Http::STATUS_CONFLICT, $response->getStatus()); + } } diff --git a/tests/unit/Service/ConversionServiceTest.php b/tests/unit/Service/ConversionServiceTest.php new file mode 100644 index 00000000..b88d0405 --- /dev/null +++ b/tests/unit/Service/ConversionServiceTest.php @@ -0,0 +1,108 @@ +db = Server::get(IDBConnection::class); + $this->cleanup(); + $this->userConfig = $this->createMock(IUserConfig::class); + $this->service = new ConversionService($this->db, $this->userConfig); + } + + protected function tearDown(): void { + $this->cleanup(); + parent::tearDown(); + } + + private function cleanup(): void { + foreach (['guests_users', 'users'] as $table) { + $query = $this->db->getQueryBuilder(); + $query->delete($table) + ->where($query->expr()->eq('uid_lower', $query->createNamedParameter('karl'))); + $query->executeStatement(); + } + } + + private function rowExists(string $table): bool { + $query = $this->db->getQueryBuilder(); + $query->select('uid') + ->from($table) + ->where($query->expr()->eq('uid_lower', $query->createNamedParameter('karl'))); + $result = $query->executeQuery(); + $uid = $result->fetchOne(); + $result->closeCursor(); + return $uid !== false; + } + + public function testConvertToGuest(): void { + // Seed a regular (database backend) account. + $insert = $this->db->getQueryBuilder(); + $insert->insert('users') + ->values([ + 'uid' => $insert->createNamedParameter('karl'), + 'uid_lower' => $insert->createNamedParameter('karl'), + 'displayname' => $insert->createNamedParameter('Karl Doe'), + 'password' => $insert->createNamedParameter('3|$argon2id$v=19$dummyhash'), + ]); + $insert->executeStatement(); + + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('karl'); + $user->method('getDisplayName')->willReturn('Karl Doe'); + $user->method('getSystemEMailAddress')->willReturn('karl.doe@example.tld'); + + $createdBy = $this->createMock(IUser::class); + $createdBy->method('getUID')->willReturn('admin'); + + $this->userConfig->expects($this->once()) + ->method('setValueString') + ->with('karl', Application::APP_ID, ConfigLexicon::USER_CREATED_BY, 'admin'); + + $this->service->convertToGuest($user, $createdBy); + + // The account moved from the database backend to the guests backend, + // keeping its user ID, display name and password hash. + $this->assertFalse($this->rowExists('users')); + $this->assertTrue($this->rowExists('guests_users')); + + $query = $this->db->getQueryBuilder(); + $query->select('password', 'email', 'displayname') + ->from('guests_users') + ->where($query->expr()->eq('uid_lower', $query->createNamedParameter('karl'))); + $result = $query->executeQuery(); + $row = $result->fetch(); + $result->closeCursor(); + + $this->assertIsArray($row); + $this->assertSame('3|$argon2id$v=19$dummyhash', $row['password']); + $this->assertSame('karl.doe@example.tld', $row['email']); + $this->assertSame('Karl Doe', $row['displayname']); + } +} diff --git a/tests/unit/UserBackendTest.php b/tests/unit/UserBackendTest.php index 2e211e5c..6f7a6f72 100644 --- a/tests/unit/UserBackendTest.php +++ b/tests/unit/UserBackendTest.php @@ -87,4 +87,19 @@ public function testHashedUid(): void { $this->assertEquals(['foo'], array_values($this->backend->getDisplayNames($email))); $this->assertEquals(['foo'], array_values($this->backend->getDisplayNames(substr($email, 0, 10)))); } + + public function testCustomLoginNameUid(): void { + // A free-form UID (neither an email nor a sha256 hash) must be recognised. + $uid = 'karl'; + $email = 'karl.doe@example.tld'; + $this->backend->createUser($uid, 'bar'); + $this->backend->setInitialEmail($uid, $email); + $this->backend->setDisplayName($uid, 'Karl Doe'); + + $this->assertTrue($this->backend->userExists($uid)); + $this->assertEquals($uid, $this->backend->getRealUID($uid)); + $this->assertEquals($uid, $this->backend->checkPassword($uid, 'bar')); + $this->assertEquals($uid, $this->backend->checkPassword($email, 'bar')); + $this->assertEquals('Karl Doe', $this->backend->getDisplayName($uid)); + } } From 9d8d535b8d4a69b4e205441eb5914185d47c66f2 Mon Sep 17 00:00:00 2001 From: ernolf Date: Fri, 3 Jul 2026 19:12:41 +0200 Subject: [PATCH 2/3] docs: document account-to-guest conversion and custom guest login names Signed-off-by: ernolf --- README.md | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 3726d98e..a19fb0b8 100644 --- a/README.md +++ b/README.md @@ -59,7 +59,7 @@ Guests may be deleted in the same way you would remove (or disable) regular user The command `occ guests:add` can be used to create guest users on the command-line. ``` -php occ guests:add [--generate-password] [--password-from-env] [--display-name [DISPLAY-NAME]] [--language [LANGUAGE]] [--] +php occ guests:add [--uid [UID]] [--generate-password] [--password-from-env] [--display-name [DISPLAY-NAME]] [--language [LANGUAGE]] [--] ``` For example: @@ -70,6 +70,14 @@ OC_PASS=somepassword php occ guests:add --password-from-env --display-name "Max The user will then be able to login with "maxmustermann@example.com" using the given password. +By default the guest's login name (user ID) is derived from the email address, or a hash of it when the *"Use a hash of the email as user ID for improved privacy"* setting is enabled. To give the guest a free-form login name instead, pass `--uid`: + +```bash +OC_PASS=somepassword php occ guests:add --password-from-env --uid maxm admin maxmustermann@example.com +``` + +The guest can then log in with "maxm". + When using `--generate-password` instead of giving a password, a random password will be generated. The guest user should then use the "forgot password" link to reset it. > [!NOTE] @@ -168,6 +176,20 @@ Remove the override to fall back to the Quick presets default again: occ config:app:delete guests guest_quota ``` +### Converting accounts to guests + +An administrator can convert a regular account into a guest account. In *Administration settings → Users*, open the account's `…` menu and choose **Convert to guest account**. The account keeps its login name and password but becomes a limited guest, restricted to the apps allowed for guests, and receives the [default guest quota](#default-quota-for-new-guests). The conversion cannot be undone automatically. + +This is only possible for accounts that + +* use the database backend, +* are not already a guest, and +* have **never logged in**. + +A typical use case is self-registration through the [registration](https://apps.nextcloud.com/apps/registration) app with *"Require administrator approval"* enabled: those accounts are created disabled and never logged in, so instead of enabling them an administrator can downgrade them to guests. Once such an account has logged in, conversion is no longer possible. + +If you would rather registered users keep their email address as login name, enable *"Force email as login name"* in the registration app settings. + ### Converting guest users to full users Guest users can be automatically converted into full users (provided by any other user back end like SAML, LDAP, OAuth, database...) on their **first** login. When this happens they will retain their shares. @@ -185,4 +207,4 @@ By default the old (guest) account will be disabled after successful conversion. - Enhancement ideas: https://github.com/nextcloud/guests/issues - Pull requests: https://github.com/nextcloud/guests/pulls - Troubleshooting assistance: https://help.nextcloud.com -- Code: https://github.com/nextcloud/guests/tree/master +- Code: https://github.com/nextcloud/guests/tree/main From 77e31b8db2155ed46c9c3bafc8b4e611aa2dea96 Mon Sep 17 00:00:00 2001 From: ernolf Date: Wed, 22 Jul 2026 12:36:11 +0200 Subject: [PATCH 3/3] fix: reject conversion of accounts with administrative privileges - refuse converting members of the admin group and subadmins, both return 409 - add controller tests covering both rejection paths Assisted-by: ClaudeCode:claude-fable-5 Signed-off-by: ernolf --- lib/Controller/UsersController.php | 6 ++++ tests/unit/Controller/UsersControllerTest.php | 35 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/lib/Controller/UsersController.php b/lib/Controller/UsersController.php index b8737ab7..ad551142 100644 --- a/lib/Controller/UsersController.php +++ b/lib/Controller/UsersController.php @@ -286,6 +286,12 @@ public function convert(string $userId): DataResponse { ], Http::STATUS_CONFLICT); } + if ($this->groupManager->isAdmin($userId) || $this->subAdmin->isSubAdmin($user)) { + return new DataResponse([ + 'message' => $this->l10n->t('Accounts with administrative privileges cannot be converted to guests') + ], Http::STATUS_CONFLICT); + } + try { $this->conversionService->convertToGuest($user, $author); $this->guestManager->setGuestQuota($user); diff --git a/tests/unit/Controller/UsersControllerTest.php b/tests/unit/Controller/UsersControllerTest.php index a277a657..d382cd3c 100644 --- a/tests/unit/Controller/UsersControllerTest.php +++ b/tests/unit/Controller/UsersControllerTest.php @@ -803,6 +803,41 @@ public function testConvertAlreadyGuest(): void { $this->assertEquals(Http::STATUS_CONFLICT, $response->getStatus()); } + public function testConvertRejectsAdmin(): void { + $this->userSession->method('getUser')->willReturn($this->createMock(IUser::class)); + + $user = $this->createMock(IUser::class); + $user->method('getBackendClassName')->willReturn('Database'); + $user->method('getLastLogin')->willReturn(0); + + $this->userManager->method('get')->with('karl')->willReturn($user); + $this->guestManager->method('isGuest')->with($user)->willReturn(false); + $this->groupManager->method('isAdmin')->with('karl')->willReturn(true); + + $this->conversionService->expects($this->never())->method('convertToGuest'); + + $response = $this->controller->convert('karl'); + $this->assertEquals(Http::STATUS_CONFLICT, $response->getStatus()); + } + + public function testConvertRejectsSubAdmin(): void { + $this->userSession->method('getUser')->willReturn($this->createMock(IUser::class)); + + $user = $this->createMock(IUser::class); + $user->method('getBackendClassName')->willReturn('Database'); + $user->method('getLastLogin')->willReturn(0); + + $this->userManager->method('get')->with('karl')->willReturn($user); + $this->guestManager->method('isGuest')->with($user)->willReturn(false); + $this->groupManager->method('isAdmin')->with('karl')->willReturn(false); + $this->subAdmin->method('isSubAdmin')->with($user)->willReturn(true); + + $this->conversionService->expects($this->never())->method('convertToGuest'); + + $response = $this->controller->convert('karl'); + $this->assertEquals(Http::STATUS_CONFLICT, $response->getStatus()); + } + public function testConvertRejectsAccountThatLoggedIn(): void { $this->userSession->method('getUser')->willReturn($this->createMock(IUser::class));