Skip to content

fix(files_external): reject backend changes when updating mounts #64456

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.

Sign up for GitHub

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

Draft
joshtrichards wants to merge 4 commits into from
Draft

fix(files_external): reject backend changes when updating mounts #64456

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
88a7adc
docs(files_external): document mount update side effects and scope ch…
joshtrichards Sep 17, 2026
b4962ff
fix(files_external): reject backend changes when updating mounts
joshtrichards Sep 17, 2026
3d185d0
fix(files_external): persist mount priority updates
joshtrichards Sep 17, 2026
5c61b20
chore(files_external): revert persisting priority
joshtrichards Sep 17, 2026
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
34 changes: 24 additions & 10 deletions apps/files_external/lib/Service/StoragesService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@
use Psr\Log\LoggerInterface;

/**
* Service class to manage external storage
* An abstract service for managing Nextcloud’s external storage mounts and their associated configuration,
* including loading, validating, creating, updating, and removing mounts while coordinating related filesystem
* hooks and settings.
*
* @psalm-import-type ExternalMountInfo from DBConfigService
*/
Expand Down Expand Up @@ -317,22 +319,23 @@ protected function triggerApplicableHooks(string $signal, string $mountPoint, st
abstract protected function triggerHooks(StorageConfig $storage, string $signal): void;

/**
* Triggers signal_create_mount or signal_delete_mount to
* accommodate for additions/deletions in applicableUsers
* and applicableGroups fields.
* Trigger lifecycle hooks for changes to a storage's mount point or scope.
*
* @param StorageConfig $oldStorage old storage data
* @param StorageConfig $newStorage new storage data
* @param StorageConfig $oldStorage The storage configuration before the update.
* @param StorageConfig $newStorage The storage configuration after the update.
*/
abstract protected function triggerChangeHooks(StorageConfig $oldStorage, StorageConfig $newStorage): void;

/**
* Update storage to the configuration
* Reconcile the persisted configuration of an existing external storage mount
* with the supplied configuration.
*
* @param StorageConfig $updatedStorage storage attributes
* @param StorageConfig $updatedStorage The updated configuration. Its ID
* identifies the mount to modify.
* @return StorageConfig The updated configuration for the external storage mount.
*
* @return StorageConfig storage config
* @throws NotFoundException if the given storage does not exist in the config
* @throws NotFoundException If the mount does not exist or its backend is
* no longer available.
*/
public function updateStorage(StorageConfig $updatedStorage): StorageConfig {
$id = $updatedStorage->getId();
Expand All @@ -345,6 +348,10 @@ public function updateStorage(StorageConfig $updatedStorage): StorageConfig {

$oldStorage = $this->getStorageConfigFromDBMount($existingMount);

if ($updatedStorage->getBackend()->getIdentifier() !== $oldStorage->getBackend()->getIdentifier()) {
throw new \InvalidArgumentException('Changing the storage backend is not supported');
}

if ($oldStorage->getBackend() instanceof InvalidBackend) {
throw new NotFoundException('Storage with id "' . $id . '" cannot be edited due to missing backend');
}
Expand All @@ -354,13 +361,18 @@ public function updateStorage(StorageConfig $updatedStorage): StorageConfig {
$addedUsers = array_diff($updatedStorage->getApplicableUsers(), $oldStorage->getApplicableUsers());
$addedGroups = array_diff($updatedStorage->getApplicableGroups(), $oldStorage->getApplicableGroups());

// User-scoped mounts are distuignished from global by the presence (or lack thereof)
// of explicit users or groups.
$oldUserCount = count($oldStorage->getApplicableUsers());
$oldGroupCount = count($oldStorage->getApplicableGroups());
$newUserCount = count($updatedStorage->getApplicableUsers());
$newGroupCount = count($updatedStorage->getApplicableGroups());
// No explicit users or groups means the mount applies globally.
$wasGlobal = ($oldUserCount + $oldGroupCount) === 0;
$isGlobal = ($newUserCount + $newGroupCount) === 0;

// Update explicit applicability then update global applicability. This ordering avoids
// broadening access during the transition.
foreach ($removedUsers as $user) {
$this->dbConfig->removeApplicable($id, DBConfigService::APPLICABLE_TYPE_USER, $user);
}
Expand All @@ -384,6 +396,8 @@ public function updateStorage(StorageConfig $updatedStorage): StorageConfig {
$changedOptions = array_diff_assoc($updatedStorage->getMountOptions(), $oldStorage->getMountOptions());

foreach ($changedConfig as $key => $value) {
// The placeholder means that the caller did not provide a replacement
// value, so retain the existing backend configuration.
if ($value !== DefinitionParameter::UNMODIFIED_PLACEHOLDER) {
$this->dbConfig->setConfig($id, $key, $value);
}
Expand Down
Loading