From c3539d35a45ce1152bb3aaf511d5cebb2c75f0d9 Mon Sep 17 00:00:00 2001 From: ernolf Date: Wed, 10 Jun 2026 18:01:28 +0200 Subject: [PATCH] fix(settings): save the guest allowlist again and add a default quota selector - the allowlist NcSelect used @input, which @nextcloud/vue 9 no longer emits, so editing the allowed apps was never saved - rework the allowlist into a read-only chip list with an edit (pencil) button and a confirm (check) button that applies and saves; editing uses a draft, so closing the dropdown no longer saves - add a "Default quota for new guest accounts" selector (the preset-derived default shown with its value, Unlimited, common sizes, or a custom size with a required unit) - add Config getters/setters and SettingsController plumbing for guest_quota, reading the preset default via IAppConfig::getDetails() - cover the new quota config methods with unit tests - document the default quota and its Quick presets source in the README - fix three README links that still pointed to the removed `master` branch (now `main`) Assisted-by: ClaudeCode:claude-opus-4-8 Signed-off-by: ernolf --- README.md | 24 ++- lib/Config.php | 42 +++++ lib/Controller/SettingsController.php | 21 ++- src/views/GuestSettings.vue | 211 ++++++++++++++++++++++++-- tests/unit/ConfigTest.php | 58 +++++++ 5 files changed, 339 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 1141e034..3726d98e 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ The app is published in the [app store](https://apps.nextcloud.com/apps/guests). ## Development -Development is ongoing. A [CHANGELOG](https://github.com/nextcloud/guests/blob/master/CHANGELOG.md) covers the highlights. [New releases are also published](https://github.com/nextcloud-releases/guests/releases) on GitHub. +Development is ongoing. A [CHANGELOG](https://github.com/nextcloud/guests/blob/main/CHANGELOG.md) covers the highlights. [New releases are also published](https://github.com/nextcloud-releases/guests/releases) on GitHub. ## Usage @@ -43,12 +43,12 @@ Optionally, when creating a guest the following values may also be specified: Admins/Group admins also may: -* specify the group(s) to put the guest user in (see [Guest specific behavior and configuration](https://github.com/nextcloud/guests/blob/master/README.md#guest-specific-behavior-and-configuration) for details). +* specify the group(s) to put the guest user in (see [Guest specific behavior and configuration](https://github.com/nextcloud/guests/blob/main/README.md#guest-specific-behavior-and-configuration) for details). ![image](https://github.com/nextcloud/guests/assets/1731941/68edbd4f-fedc-45f0-8241-2e1cd12d04de) > [!WARNING] -> While it is easy to create a new Guest, it's important to understand the default behavior and how guests interact with other features in Nextcloud. See [Guest specific behavior and configuration](https://github.com/nextcloud/guests/blob/master/README.md#guest-specific-behavior-and-configuration) for details. +> While it is easy to create a new Guest, it's important to understand the default behavior and how guests interact with other features in Nextcloud. See [Guest specific behavior and configuration](https://github.com/nextcloud/guests/blob/main/README.md#guest-specific-behavior-and-configuration) for details. ### Deleting a guest @@ -150,6 +150,24 @@ to list users within that group (and, for example, share files with those users) As a result, guests will be able to see each other as they are part of the same `guest` group. To prevent that behavior, you can add the `guest` group to the "Exclude groups from sharing" settings. You can find more information in [our documentation about sharing](https://docs.nextcloud.com/server/21/admin_manual/configuration_files/file_sharing_configuration.html). +### Default quota for new guests + +New guest accounts are created with a default storage quota. This default comes from the **Quick presets** configuration (*Administration → Quick presets*), where it is listed as *"set default disk quota assigned to guest account at its creation"* (`guest_quota`). Its value depends on the selected preset: the **Default** preset uses `0 B`, while organization or family presets use a non-zero quota such as `1 GB` or `10 GB`. As a result, on many instances guests no longer receive `0 B` automatically. + +Administrators can review and override this default under **Administration settings → Guests → "Default quota for new guest accounts"**: pick a preset (the *Default* entry shows the current preset value), *Unlimited*, or enter a custom size such as `500 MB`. + +It can also be set on the command line: + +``` +occ config:app:set guests guest_quota --value "500 MB" +``` + +Remove the override to fall back to the Quick presets default again: + +``` +occ config:app:delete guests guest_quota +``` + ### 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. diff --git a/lib/Config.php b/lib/Config.php index e1103f76..3b8d9d83 100644 --- a/lib/Config.php +++ b/lib/Config.php @@ -9,6 +9,7 @@ namespace OCA\Guests; +use OCA\Guests\AppInfo\Application; use OCP\AppFramework\Services\IAppConfig; use OCP\Group\ISubAdmin; use OCP\IAppConfig as IGlobalAppConfig; @@ -51,6 +52,47 @@ public function setHideOtherUsers(bool $hide): void { $this->appConfig->setAppValueBool(ConfigLexicon::HIDE_OTHER_ACCOUNTS, $hide); } + /** + * Currently configured default quota for new guest accounts. Returns the + * explicit override if one is set, otherwise the preset-derived default. + */ + public function getGuestQuota(): string { + return $this->appConfig->getAppValueString(ConfigLexicon::GUEST_DISK_QUOTA); + } + + /** + * Whether an explicit default quota is stored, as opposed to falling back + * to the preset-derived default from the config lexicon. + */ + public function hasGuestQuotaOverride(): bool { + return $this->globalAppConfig->hasKey(Application::APP_ID, ConfigLexicon::GUEST_DISK_QUOTA); + } + + /** + * The preset-derived default quota, regardless of any override. This is the + * value the instance's configuration preset assigns to guests. + */ + public function getGuestQuotaDefault(): string { + // getDetails() only works once a value is stored; when no override is + // set, the app config already returns the preset-derived lexicon default. + if (!$this->hasGuestQuotaOverride()) { + return $this->appConfig->getAppValueString(ConfigLexicon::GUEST_DISK_QUOTA); + } + return (string)($this->globalAppConfig->getDetails(Application::APP_ID, ConfigLexicon::GUEST_DISK_QUOTA)['default'] ?? '0 B'); + } + + /** + * Store the default guest quota. An empty value or 'default' removes the + * override so the preset-derived default applies again. + */ + public function setGuestQuota(?string $quota): void { + if ($quota === null || $quota === '' || $quota === 'default') { + $this->appConfig->deleteAppValue(ConfigLexicon::GUEST_DISK_QUOTA); + return; + } + $this->appConfig->setAppValueString(ConfigLexicon::GUEST_DISK_QUOTA, $quota); + } + public function getHome(string $uid): string { return $this->config->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data') . '/' . $uid; } diff --git a/lib/Controller/SettingsController.php b/lib/Controller/SettingsController.php index 0b2ffcbb..d655a7f4 100644 --- a/lib/Controller/SettingsController.php +++ b/lib/Controller/SettingsController.php @@ -18,6 +18,7 @@ use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Services\IAppConfig; use OCP\IRequest; +use OCP\Util; /** * Class SettingsController is used to handle configuration changes on the @@ -55,13 +56,15 @@ public function getConfig(): DataResponse { 'whiteListableApps' => $this->appWhitelist->getWhitelistAbleApps(), 'sharingRestrictedToGroup' => $this->config->isSharingRestrictedToGroup(), 'createRestrictedToGroup' => $this->config->getCreateRestrictedToGroup(), + 'guestQuota' => $this->config->hasGuestQuotaOverride() ? $this->config->getGuestQuota() : 'default', + 'guestQuotaDefault' => $this->config->getGuestQuotaDefault(), ]); } /** * @param list $whitelist */ - public function setConfig(bool $useWhitelist, array $whitelist, bool $allowExternalStorage, bool $useHashedEmailAsUserID, bool $hideUsers, array $createRestrictedToGroup): DataResponse { + public function setConfig(bool $useWhitelist, array $whitelist, bool $allowExternalStorage, bool $useHashedEmailAsUserID, bool $hideUsers, array $createRestrictedToGroup, string $guestQuota = 'default'): DataResponse { $newWhitelist = []; foreach ($whitelist as $app) { $newWhitelist[] = trim((string)$app); @@ -73,10 +76,26 @@ public function setConfig(bool $useWhitelist, array $whitelist, bool $allowExter $this->config->setUseHashedEmailAsUserID($useHashedEmailAsUserID); $this->config->setHideOtherUsers($hideUsers); $this->config->setCreateRestrictedToGroup($createRestrictedToGroup); + if ($this->isValidQuota($guestQuota)) { + $this->config->setGuestQuota($guestQuota); + } return new DataResponse(); } + /** + * A quota value is acceptable if it is the "default" sentinel, "none" + * (unlimited) or a human-readable size such as "500 MB". + */ + private function isValidQuota(string $quota): bool { + if ($quota === 'default' || $quota === 'none') { + return true; + } + // Require an explicit unit so the stored value is unambiguous (e.g. "500 MB"). + return preg_match('/^\d+(\.\d+)?\s*[KMGTP]?B$/i', trim($quota)) === 1 + && Util::computerFileSize($quota) !== false; + } + /** * AJAX handler for getting the whitelisted apps * We do not set the whitelist to null when it is unused. This is by design. diff --git a/src/views/GuestSettings.vue b/src/views/GuestSettings.vue index 2f01e6f3..ad6031d4 100644 --- a/src/views/GuestSettings.vue +++ b/src/views/GuestSettings.vue @@ -84,21 +84,66 @@ {{ t('guests', 'Limit guest access to an app\'s allowlist') }} -

- +

+ +
+
    +
  • + {{ app }} +
    • +
+ {{ t('guests', 'No apps selected') }} + + + +
+ +
+ + + + +
{{ t('guests', 'Reset allowlist') }} -

+
+ + +
+ + +
@@ -121,13 +166,16 @@ 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' +import Check from 'vue-material-design-icons/Check.vue' import History from 'vue-material-design-icons/History.vue' +import Pencil from 'vue-material-design-icons/Pencil.vue' import GuestList from '../components/GuestList.vue' import { logger } from '../services/logger.ts' export default { name: 'GuestSettings', components: { + Check, GuestList, History, NcButton, @@ -136,6 +184,7 @@ export default { NcSelect, NcSettingsSection, NcSettingsSelectGroup, + Pencil, }, data() { @@ -145,6 +194,10 @@ export default { saved: false, saving: false, savingTimeout: null, + editingAllowlist: false, + whitelistDraft: [], + quotaInputId: 'guests-default-quota', + quotaSearch: '', config: { useWhitelist: false, allowExternalStorage: false, @@ -154,11 +207,66 @@ export default { whiteListableApps: [], sharingRestrictedToGroup: false, createRestrictedToGroup: [], + guestQuota: 'default', + guestQuotaDefault: '0 B', }, } }, computed: { + allowlistApps() { + return (this.config.whitelist ?? []) + .map((app) => (typeof app === 'string' ? app : (app.label ?? app.id ?? app.name ?? String(app)))) + }, + + baseQuotaOptions() { + return [ + { id: 'default', label: t('guests', 'Default quota ({size})', { size: this.config.guestQuotaDefault || '0 B' }) }, + { id: 'none', label: t('guests', 'Unlimited') }, + { id: '1 GB', label: '1 GB' }, + { id: '5 GB', label: '5 GB' }, + { id: '10 GB', label: '10 GB' }, + ] + }, + + // When a bare number is typed, offer it with selectable units (KB/MB/GB) + // instead of silently assuming bytes. A partially typed unit narrows the + // suggestions (e.g. "500 m" → "500 MB"). + quotaOptions() { + const search = (this.quotaSearch || '').trim() + const parts = search.match(/^(\d+(?:\.\d+)?)\s*([a-z]*)$/i) + if (parts) { + const value = parts[1] + const typedUnit = parts[2].toLowerCase() + const units = ['KB', 'MB', 'GB'] + const matching = typedUnit + ? units.filter((unit) => unit.toLowerCase().startsWith(typedUnit)) + : units + return (matching.length ? matching : units) + .map((unit) => ({ id: `${value} ${unit}`, label: `${value} ${unit}` })) + } + return this.baseQuotaOptions + }, + + quotaModel: { + get() { + const current = this.config.guestQuota + return this.baseQuotaOptions.find((option) => option.id === current) + ?? { id: current, label: current } + }, + + set(option) { + const value = (option && typeof option === 'object') ? option.id : option + if (!this.isValidQuota(value)) { + showError(t('guests', 'Please enter a quota with a unit, for example "500 MB" or "2 GB"')) + return + } + this.config.guestQuota = value + this.quotaSearch = '' + this.saveConfig() + }, + }, + statusText() { if (this.error) { return t('guests', 'Error') @@ -180,6 +288,36 @@ export default { methods: { t, + startEditAllowlist() { + this.whitelistDraft = [...this.config.whitelist] + this.editingAllowlist = true + }, + + confirmAllowlist() { + this.config.whitelist = [...this.whitelistDraft] + this.editingAllowlist = false + this.saveConfig() + }, + + normalizeQuota(value) { + // A unit is required so the stored value is unambiguous; a bare + // number like "500" is rejected (it would mean 500 bytes). + const match = String(value).trim().match(/^(\d+(?:\.\d+)?)\s*(b|kb|mb|gb|tb|pb)$/i) + if (!match) { + return null + } + // Canonical " " form, as expected by the backend. + return `${match[1]} ${match[2].toUpperCase()}` + }, + + onQuotaSearch(query) { + this.quotaSearch = query + }, + + isValidQuota(value) { + return value === 'default' || value === 'none' || this.normalizeQuota(value) !== null + }, + async loadConfig() { const { data } = await axios.get(generateUrl('apps/guests/config')) this.config = data @@ -208,6 +346,9 @@ export default { try { const { data } = await axios.post(generateUrl('apps/guests/whitelist/reset')) this.config.whitelist = data.whitelist + if (this.editingAllowlist) { + this.whitelistDraft = [...data.whitelist] + } this.saved = true } catch (error) { this.error = true @@ -266,12 +407,46 @@ export default { } .allowlist { - max-width: 500px; + max-width: 690px; + margin-inline-start: var(--default-clickable-area); - .multiselect { - width: calc(100% - 48px); - margin-right: 0; + &__row { + display: flex; + align-items: center; + gap: 8px; margin-top: 1rem; + + &--display { + align-items: flex-start; + } + } + + &__apps { + flex: 1 1 auto; + display: flex; + flex-wrap: wrap; + gap: 4px; + min-width: 0; + padding-top: 4px; + } + + &__app { + padding: 2px 10px; + background-color: var(--color-background-dark); + border-radius: var(--border-radius-pill, 1rem); + color: var(--color-text-maxcontrast); + font-size: 0.9em; + } + + &__empty { + flex: 1 1 auto; + padding-top: 6px; + color: var(--color-text-maxcontrast); + } + + &__select { + flex: 1 1 auto; + min-width: 0; } .reset-button { @@ -279,6 +454,16 @@ export default { } } +.guest-quota { + margin-top: 1.5rem; + max-width: 400px; + + &__label { + display: block; + margin-bottom: 4px; + } +} + .msg { color: white; padding: 5px; diff --git a/tests/unit/ConfigTest.php b/tests/unit/ConfigTest.php index 890609ac..9a04e36d 100644 --- a/tests/unit/ConfigTest.php +++ b/tests/unit/ConfigTest.php @@ -253,4 +253,62 @@ public function testSetCreateRestrictedToGroup(): void { $this->guestConfig->setCreateRestrictedToGroup(['group1', 'group2']); } + + public function testGetGuestQuota(): void { + $this->appConfig->method('getAppValueString') + ->with('guest_quota') + ->willReturn('5 GB'); + + $this->assertEquals('5 GB', $this->guestConfig->getGuestQuota()); + } + + public function testHasGuestQuotaOverride(): void { + $this->globalAppConfig->method('hasKey') + ->with('guests', 'guest_quota') + ->willReturn(true); + + $this->assertTrue($this->guestConfig->hasGuestQuotaOverride()); + } + + public function testGetGuestQuotaDefaultWithoutOverride(): void { + // No explicit value stored: the app config returns the preset-derived + // lexicon default, getDetails() is not consulted. + $this->globalAppConfig->method('hasKey') + ->with('guests', 'guest_quota') + ->willReturn(false); + $this->appConfig->method('getAppValueString') + ->with('guest_quota') + ->willReturn('1 GB'); + + $this->assertEquals('1 GB', $this->guestConfig->getGuestQuotaDefault()); + } + + public function testGetGuestQuotaDefaultWithOverride(): void { + // An override is stored, so the preset default is read from the lexicon + // details rather than from the (overridden) stored value. + $this->globalAppConfig->method('hasKey') + ->with('guests', 'guest_quota') + ->willReturn(true); + $this->globalAppConfig->method('getDetails') + ->with('guests', 'guest_quota') + ->willReturn(['default' => '10 GB']); + + $this->assertEquals('10 GB', $this->guestConfig->getGuestQuotaDefault()); + } + + public function testSetGuestQuotaValue(): void { + $this->appConfig->expects($this->once()) + ->method('setAppValueString') + ->with('guest_quota', '500 MB'); + + $this->guestConfig->setGuestQuota('500 MB'); + } + + public function testSetGuestQuotaDefaultRemovesOverride(): void { + $this->appConfig->expects($this->once()) + ->method('deleteAppValue') + ->with('guest_quota'); + + $this->guestConfig->setGuestQuota('default'); + } }