Skip to content
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ js
.vscode
.php_cs.cache
.php-cs-fixer.cache
.phpunit.result.cache
2 changes: 1 addition & 1 deletion appinfo/info.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
Guests accounts can be created from the share menu by entering either the recipients email or name and choosing "create guest account", once the share is created the guest user will receive an email notification about the mail with a link to set their password.

Guests users can only access files shared to them and cannot create any files outside of shares, additionally, the apps accessible to guest accounts are whitelisted.]]></description>
<version>4.7.0-dev.0</version>
<version>4.7.0-dev.1</version>
<licence>agpl</licence>
<author>Nextcloud</author>
<types>
Expand Down
12 changes: 10 additions & 2 deletions lib/Command/AddCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

namespace OCA\Guests\Command;

use OCA\Guests\Config;
use OCA\Guests\GuestManager;
use OCP\IUser;
use OCP\IUserManager;
Expand All @@ -27,6 +28,7 @@ public function __construct(
private readonly IUserManager $userManager,
private readonly IMailer $mailer,
private readonly GuestManager $guestManager,
private readonly Config $config,
) {
parent::__construct();
}
Expand Down Expand Up @@ -83,14 +85,20 @@ protected function execute(InputInterface $input, OutputInterface $output): int
return self::FAILURE;
}

$email = $input->getArgument('email');
if ($this->config->useHashedEmailAsUserID()) {
$email = strtolower($email);
Comment thread
nickvergessen marked this conversation as resolved.
$uid = hash('sha256', $email);
} else {
$uid = $email;
}

// same behavior like in the UsersController
$uid = $input->getArgument('email');
if ($this->userManager->userExists($uid)) {
$output->writeln('<error>The user "' . $uid . '" already exists.</error>');
return self::FAILURE;
}

$email = $input->getArgument('email');
if (!$this->mailer->validateMailAddress($email)) {
$output->writeln('<error>Invalid email address "' . $email . '".</error>');
return self::FAILURE;
Expand Down
2 changes: 1 addition & 1 deletion lib/Command/ListCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$this->writeArrayInOutputFormat($input, $output, $guests);
} else {
$table = new Table($output);
$table->setHeaders(['Email', 'Name', 'Invited By']);
$table->setHeaders(['Email', 'UserID', 'Name', 'Invited By']);
$table->setRows($guests);
$table->render();
}
Expand Down
8 changes: 8 additions & 0 deletions lib/Config.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,14 @@ public function setAllowExternalStorage(string|bool $allow): void {
$this->appConfig->setAppValueBool('allow_external_storage', $allow === true || $allow === 'true') ;
}

public function useHashedEmailAsUserID(): bool {
return $this->appConfig->getAppValueBool('hash_user_ids', true);
}

public function setUseHashedEmailAsUserID(bool $useHash): void {
$this->appConfig->setAppValueBool('hash_user_ids', $useHash) ;
}

public function hideOtherUsers(): bool {
return $this->appConfig->getAppValueBool('hide_users', true);
}
Expand Down
4 changes: 3 additions & 1 deletion lib/Controller/SettingsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ public function getConfig(): DataResponse {
'useWhitelist' => $useWhitelist,
'whitelist' => $whitelist,
'allowExternalStorage' => $allowExternalStorage,
'useHashedEmailAsUserID' => $this->config->useHashedEmailAsUserID(),
'hideUsers' => $hideUsers,
'whiteListableApps' => $this->appWhitelist->getWhitelistAbleApps(),
'sharingRestrictedToGroup' => $this->config->isSharingRestrictedToGroup(),
Expand All @@ -58,7 +59,7 @@ public function getConfig(): DataResponse {
/**
* @param list<string> $whitelist
*/
public function setConfig(bool $useWhitelist, array $whitelist, bool $allowExternalStorage, bool $hideUsers, array $createRestrictedToGroup): DataResponse {
public function setConfig(bool $useWhitelist, array $whitelist, bool $allowExternalStorage, bool $useHashedEmailAsUserID, bool $hideUsers, array $createRestrictedToGroup): DataResponse {
$newWhitelist = [];
foreach ($whitelist as $app) {
$newWhitelist[] = trim((string)$app);
Expand All @@ -67,6 +68,7 @@ public function setConfig(bool $useWhitelist, array $whitelist, bool $allowExter
$this->config->setUseWhitelist($useWhitelist);
$this->config->setAppWhitelist($newWhitelist);
$this->config->setAllowExternalStorage($allowExternalStorage);
$this->config->setUseHashedEmailAsUserID($useHashedEmailAsUserID);
$this->config->setHideOtherUsers($hideUsers);
$this->config->setCreateRestrictedToGroup($createRestrictedToGroup);

Expand Down
7 changes: 6 additions & 1 deletion lib/Controller/UsersController.php
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,12 @@ public function create(string $email, string $displayName, string $language, arr
);
}

$username = $email;
if ($this->config->useHashedEmailAsUserID()) {
$email = strtolower($email);
$username = hash('sha256', $email);
} else {
$username = $email;
}

$existingUsers = $this->userManager->getByEmail($email);
if (count($existingUsers) > 0) {
Expand Down
12 changes: 7 additions & 5 deletions lib/GuestManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ public function createGuest(?IUser $createdBy, string $userId, string $email, st
return null;
}

$this->userBackend->setInitialEmail($userId, $email);
$user->setSystemEMailAddress($email);
if ($createdBy instanceof IUser) {
$this->config->setUserValue($userId, 'guests', 'created_by', $createdBy->getUID());
Expand Down Expand Up @@ -125,11 +126,11 @@ public function listGuests(): array {
* }>
*/
public function getGuestsInfo(): array {
$displayNames = $this->userBackend->getDisplayNames();
$guests = array_keys($displayNames);
$guestsInfo = $this->userBackend->getAllGuestAccounts();
$guests = array_keys($guestsInfo);
$shareCounts = $this->getShareCountForUsers($guests);
$createdBy = $this->config->getUserValueForUsers('guests', 'created_by', $guests);
return array_map(function (string $uid) use ($createdBy, $displayNames, $shareCounts): array {
return array_map(function (string $uid) use ($createdBy, $guestsInfo, $shareCounts): array {
$allSharesCount = count(array_merge(
$this->shareManager->getSharedWith($uid, IShare::TYPE_USER, null, -1, 0),
$this->shareManager->getSharedWith($uid, IShare::TYPE_GROUP, null, -1, 0),
Expand All @@ -138,8 +139,9 @@ public function getGuestsInfo(): array {
$this->shareManager->getSharedWith($uid, IShare::TYPE_ROOM, null, -1, 0),
));
return [
'email' => $uid,
'display_name' => $displayNames[$uid] ?? $uid,
'email' => $guestsInfo[$uid]['email'] ?? $uid,
'uid' => $uid,
'display_name' => $guestsInfo[$uid]['displayname'] ?? $uid,
'created_by' => $createdBy[$uid] ?? '',
'share_count' => $shareCounts[$uid] ?? 0,
'share_count_with_circles' => $allSharesCount,
Expand Down
61 changes: 61 additions & 0 deletions lib/Migration/Version4002Date20250501195008.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\Guests\Migration;

use Closure;
use OCP\DB\ISchemaWrapper;
use OCP\IDBConnection;
use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;

/**
* Add a column for the email so we know the originally invited email as well
*/
class Version4002Date20250501195008 extends SimpleMigrationStep {
public function __construct(
protected IDBConnection $db,
) {
}

/**
* @param IOutput $output
* @param Closure(): ISchemaWrapper $schemaClosure
* @param array $options
* @return null|ISchemaWrapper
*/
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();

$table = $schema->getTable('guests_users');
if ($table->hasColumn('email')) {
return null;
}

$table->addColumn('email', 'string', [
'notnull' => false,
'length' => 64,
'default' => '',
]);
return $schema;
}

/**
* @param IOutput $output
* @param Closure(): ISchemaWrapper $schemaClosure
* @param array $options
*/
public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void {
$query = $this->db->getQueryBuilder();
$query->update('guests_users')
->set('email', 'uid_lower');
$query->executeStatement();
}
}
59 changes: 56 additions & 3 deletions lib/UserBackend.php
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,14 @@ public function deleteUser($uid): bool {
return (bool)$result;
}

public function setInitialEmail(string $uid, string $email): bool {
$query = $this->dbConn->getQueryBuilder();
$query->update('guests_users')
->set('email', $query->createNamedParameter($email))
->where($query->expr()->eq('uid_lower', $query->createNamedParameter(mb_strtolower($uid))));
return (bool)$query->executeStatement();
}

/**
* Change the password of a user
*/
Expand Down Expand Up @@ -231,14 +239,43 @@ public function getDisplayNames($search = '', $limit = null, $offset = null): ar
return $displayNames;
}

/**
* Get a list of all users with their email and display name
*
* @return array<string, array{email: string, displayname: string}>
*/
public function getAllGuestAccounts(?int $limit = null, ?int $offset = null): array {
if (!$this->allowListing) {
return [];
}

$query = $this->dbConn->getQueryBuilder();
$query->select('uid', 'email', 'displayname')
->from('guests_users')
->orderBy('uid_lower', 'ASC')
->setMaxResults($limit)
->setFirstResult($offset);

$result = $query->executeQuery();
$users = [];
while ($row = $result->fetch()) {
$users[(string)$row['uid']] = [
'email' => (string)$row['email'],
'displayname' => (string)$row['displayname'],
];
Comment thread
nickvergessen marked this conversation as resolved.
}

return $users;
}

/**
* Check if the password is correct without logging in the user
* returns the user id or false
*
* @return string|false
*/
public function checkPassword(string $loginName, string $password) {
if (!str_contains($loginName, '@')) {
if (!$this->potentialGuestUserId($loginName)) {
return false;
}

Expand Down Expand Up @@ -275,9 +312,13 @@ public function checkPassword(string $loginName, string $password) {
* @param string $uid the username
*/
private function loadUser($uid): bool {
if (isset($this->cache[$uid]) && $this->cache[$uid] === false) {
return false;
}

// guests $uid could be NULL or ''
// or is not an email anyway
if (!str_contains($uid, '@')) {
if (!$this->potentialGuestUserId($uid)) {
$this->cache[$uid] = false;
return false;
}
Expand Down Expand Up @@ -390,7 +431,7 @@ public function getBackendName(): string {
}

public function getRealUID(string $uid): string {
if (!str_contains($uid, '@')) {
if (!$this->potentialGuestUserId($uid)) {
throw new \RuntimeException($uid . ' does not exist');
}

Expand All @@ -404,4 +445,16 @@ public function getRealUID(string $uid): string {

return $this->cache[$uid]['uid'];
}

/**
* 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
*/
protected function potentialGuestUserId(string $userId): bool {
return str_contains($userId, '@') || preg_match('/^[a-f0-9]{64}$/', $userId);
}
}
9 changes: 8 additions & 1 deletion src/components/GuestList.vue
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
<th :class="getSortClass('email')" @click="setSort('email')">
{{ t('guests', 'Email') }}
</th>
<th :class="getSortClass('uid')" @click="setSort('uid')">
{{ t('guests', 'User ID') }}
</th>
<th :class="getSortClass('display_name')" @click="setSort('display_name')">
{{ t('guests', 'Name') }}
</th>
Expand All @@ -31,6 +34,10 @@
:title="guest.email">
{{ guest.email }}
</td>
<td class="uid"
:title="guest.uid">
{{ guest.uid }}
</td>
<td class="display_name"
:title="guest.display_name">
{{ guest.display_name }}
Expand All @@ -46,7 +53,7 @@
<tr v-if="guest.email === details_for"
:key="guest.email + '-details'"
class="details">
<td colspan="4">
<td colspan="5">
<GuestDetails :guest-id="guest.email" />
</td>
</tr>
Expand Down
7 changes: 7 additions & 0 deletions src/views/GuestSettings.vue
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@
{{ t('guests', 'Guest accounts can access mounted external storages') }}
</NcCheckboxRadioSwitch>

<NcCheckboxRadioSwitch :checked.sync="config.useHashedEmailAsUserID"
type="switch"
@update:checked="saveConfig">
{{ t('guests', 'Use a hash of the email as user ID for improved privacy') }}
</NcCheckboxRadioSwitch>

<NcCheckboxRadioSwitch :checked.sync="config.hideUsers"
type="switch"
@update:checked="saveConfig">
Expand Down Expand Up @@ -106,7 +112,7 @@
import NcNoteCard from '@nextcloud/vue/components/NcNoteCard'
import NcSelect from '@nextcloud/vue/components/NcSelect'
import NcSettingsSection from '@nextcloud/vue/components/NcSettingsSection'
import NcSettingsSelectGroup from '@nextcloud/vue/components/NcSettingsSelectGroup'

Check warning on line 115 in src/views/GuestSettings.vue

View workflow job for this annotation

GitHub Actions / NPM lint

Using exported name 'NcSettingsSelectGroup' as identifier for default import

import GuestList from '../components/GuestList.vue'
import { showError } from '@nextcloud/dialogs'
Expand All @@ -133,6 +139,7 @@
config: {
useWhitelist: false,
allowExternalStorage: false,
useHashedEmailAsUserID: true,
hideUsers: false,
whitelist: [],
whiteListableApps: [],
Expand Down
Loading
Loading