diff --git a/.github/workflows/lint-and-codestyle.yml b/.github/workflows/lint-and-codestyle.yml index d650d737cd68..ef690fd1c448 100644 --- a/.github/workflows/lint-and-codestyle.yml +++ b/.github/workflows/lint-and-codestyle.yml @@ -59,16 +59,3 @@ jobs: - name: Run PHP Phan run: | make test-php-phan - - git-commit-messages: - name: Git Commit Messages - runs-on: ubuntu-latest - steps: - - name: Checkout code - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - - name: Validate commit messages - uses: docker://aevea/commitsar:latest - diff --git a/CHANGELOG.md b/CHANGELOG.md index fe0f1a354395..070ab02d6960 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Table of Contents +* [Changelog for 10.16.3](#changelog-for-owncloud-core-10163-2026-05-22) * [Changelog for 10.16.2](#changelog-for-owncloud-core-10162-2026-04-02) * [Changelog for 10.16.1](#changelog-for-owncloud-core-10161-2026-02-18) * [Changelog for 10.16.0](#changelog-for-owncloud-core-10160-2025-10-23) @@ -27,6 +28,77 @@ * [Changelog for 10.4.1](#changelog-for-owncloud-core-1041-2020-03-30) * [Changelog for 10.4.0](#changelog-for-owncloud-core-1040-2020-02-10) * [Changelog for 10.3.2](#changelog-for-owncloud-core-1032-2019-12-04) +# Changelog for ownCloud Core [10.16.3] (2026-05-22) + +The following sections list the changes in ownCloud core 10.16.3 relevant to +ownCloud admins and users. + +[10.16.3]: https://github.com/owncloud/core/compare/v10.16.2...v10.16.3 + +## Summary + +* Security - Update phpseclib to 3.0.52 for CVE-2026-40194: [#41529](https://github.com/owncloud/core/pull/41529) +* Security - Restrict AppConfigController read methods to full admins only: [#41550](https://github.com/owncloud/core/pull/41550) +* Security - Update symfony/routing to 5.4.52 for CVE-2026-45065: [#41559](https://github.com/owncloud/core/pull/41559) +* Bugfix - Prevent mounting local storage if not allowed: [#41538](https://github.com/owncloud/core/pull/41538) +* Bugfix - Use the correct user ID when changing email via admin API: [#41539](https://github.com/owncloud/core/pull/41539) +* Bugfix - Prevent IDOR in WebDAV comments API: [#41558](https://github.com/owncloud/core/pull/41558) + +## Details + +* Security - Update phpseclib to 3.0.52 for CVE-2026-40194: [#41529](https://github.com/owncloud/core/pull/41529) + + CVE-2026-40194: Timing attack vulnerability in SSH binary packet processing. + Upgraded phpseclib/phpseclib from 3.0.50 to 3.0.52. + + https://github.com/owncloud/core/pull/41529 + https://github.com/owncloud/core/pull/41541 + https://github.com/phpseclib/phpseclib/releases/tag/3.0.51 + +* Security - Restrict AppConfigController read methods to full admins only: [#41550](https://github.com/owncloud/core/pull/41550) + + Subadmin users could read all oc_appconfig values including SMTP passwords, LDAP + bind credentials, and encryption master keys via the Settings API. Removed + @NoAdminRequired from getApps, getKeys, and getValue so that the AdminMiddleware + enforces full-admin-only access, consistent with the write methods. + + https://github.com/owncloud/core/pull/41550 + +* Security - Update symfony/routing to 5.4.52 for CVE-2026-45065: [#41559](https://github.com/owncloud/core/pull/41559) + + CVE-2026-45065: UrlGenerator route-requirement bypass via unanchored regex + alternation allowing off-site URL injection. Upgraded symfony/routing from + 5.4.48 to 5.4.52. + + https://github.com/owncloud/core/pull/41559 + https://symfony.com/cve-2026-45065 + +* Bugfix - Prevent mounting local storage if not allowed: [#41538](https://github.com/owncloud/core/pull/41538) + + Mounting a local storage was possible if the internal class name was used as + backend, despite local storage not allowed to be mounted. This problem is fixed + and the local storage can't be mounted if it was explicitly disallowed in the + configuration. + + https://github.com/owncloud/core/pull/41538 + +* Bugfix - Use the correct user ID when changing email via admin API: [#41539](https://github.com/owncloud/core/pull/41539) + + The admin API endpoint for changing a user's email address was incorrectly using + the requesting admin's user ID instead of the target user's ID, causing the + admin's email to be updated rather than the intended user's. + + https://github.com/owncloud/core/pull/41539 + +* Bugfix - Prevent IDOR in WebDAV comments API: [#41558](https://github.com/owncloud/core/pull/41558) + + Authenticated users could read, edit, or delete comments on files they have no + access to by supplying an arbitrary comment ID in the WebDAV comments endpoint. + The fix verifies that a requested comment belongs to the file in the URL before + returning it. + + https://github.com/owncloud/core/pull/41558 + # Changelog for ownCloud Core [10.16.2] (2026-04-02) The following sections list the changes in ownCloud core 10.16.2 relevant to diff --git a/apps/comments/lib/Dav/EntityCollection.php b/apps/comments/lib/Dav/EntityCollection.php index 5db5e15fe339..a60a6001b342 100644 --- a/apps/comments/lib/Dav/EntityCollection.php +++ b/apps/comments/lib/Dav/EntityCollection.php @@ -98,6 +98,10 @@ public function getId() { public function getChild($name) { try { $comment = $this->commentsManager->get($name); + // objectType/objectId identify the entity (e.g. file) the comment is attached to + if ($comment->getObjectType() !== $this->name || $comment->getObjectId() !== $this->id) { + throw new NotFound(); + } return new CommentNode( $this->commentsManager, $comment, @@ -151,8 +155,10 @@ public function findChildren($limit = 0, $offset = 0, \DateTime $datetime = null */ public function childExists($name) { try { - $this->commentsManager->get($name); - return true; + $comment = $this->commentsManager->get($name); + // objectType/objectId identify the entity (e.g. file) the comment is attached to + return $comment->getObjectType() === $this->name + && $comment->getObjectId() === $this->id; } catch (NotFoundException $e) { return false; } diff --git a/apps/comments/tests/unit/Dav/EntityCollectionTest.php b/apps/comments/tests/unit/Dav/EntityCollectionTest.php index 4a890c3a9304..b4e0c999a622 100644 --- a/apps/comments/tests/unit/Dav/EntityCollectionTest.php +++ b/apps/comments/tests/unit/Dav/EntityCollectionTest.php @@ -70,10 +70,14 @@ public function testGetId(): void { } public function testGetChild(): void { + $comment = $this->createMock(IComment::class); + $comment->method('getObjectType')->willReturn('files'); + $comment->method('getObjectId')->willReturn('19'); + $this->commentsManager->expects($this->once()) ->method('get') ->with('55') - ->will($this->returnValue($this->createMock(IComment::class))); + ->willReturn($comment); $node = $this->collection->getChild('55'); $this->assertInstanceOf(\OCA\Comments\Dav\CommentNode::class, $node); @@ -118,6 +122,15 @@ public function testFindChildren(): void { } public function testChildExistsTrue(): void { + $comment = $this->createMock(IComment::class); + $comment->method('getObjectType')->willReturn('files'); + $comment->method('getObjectId')->willReturn('19'); + + $this->commentsManager->expects($this->once()) + ->method('get') + ->with('44') + ->willReturn($comment); + $this->assertTrue($this->collection->childExists('44')); } @@ -129,4 +142,60 @@ public function testChildExistsFalse(): void { $this->assertFalse($this->collection->childExists('44')); } + + public function testGetChildWrongFile(): void { + $this->expectException(\Sabre\DAV\Exception\NotFound::class); + + $comment = $this->createMock(IComment::class); + $comment->method('getObjectType')->willReturn('files'); + $comment->method('getObjectId')->willReturn('999'); // different file + + $this->commentsManager->expects($this->once()) + ->method('get') + ->with('55') + ->willReturn($comment); + + $this->collection->getChild('55'); + } + + public function testGetChildWrongObjectType(): void { + $this->expectException(\Sabre\DAV\Exception\NotFound::class); + + $comment = $this->createMock(IComment::class); + $comment->method('getObjectType')->willReturn('albums'); // wrong type + $comment->method('getObjectId')->willReturn('19'); + + $this->commentsManager->expects($this->once()) + ->method('get') + ->with('55') + ->willReturn($comment); + + $this->collection->getChild('55'); + } + + public function testChildExistsWrongFile(): void { + $comment = $this->createMock(IComment::class); + $comment->method('getObjectType')->willReturn('files'); + $comment->method('getObjectId')->willReturn('999'); // different file + + $this->commentsManager->expects($this->once()) + ->method('get') + ->with('44') + ->willReturn($comment); + + $this->assertFalse($this->collection->childExists('44')); + } + + public function testChildExistsWrongObjectType(): void { + $comment = $this->createMock(IComment::class); + $comment->method('getObjectType')->willReturn('albums'); // wrong type + $comment->method('getObjectId')->willReturn('19'); + + $this->commentsManager->expects($this->once()) + ->method('get') + ->with('44') + ->willReturn($comment); + + $this->assertFalse($this->collection->childExists('44')); + } } diff --git a/apps/files_external/lib/Controller/GlobalStoragesController.php b/apps/files_external/lib/Controller/GlobalStoragesController.php index 9ee3c91783d9..6692bacc54d8 100644 --- a/apps/files_external/lib/Controller/GlobalStoragesController.php +++ b/apps/files_external/lib/Controller/GlobalStoragesController.php @@ -86,15 +86,10 @@ public function create( $applicableGroups, $priority ) { - $canCreateNewLocalStorage = \OC::$server->getConfig()->getSystemValue('files_external_allow_create_new_local', false); - - if ($backend === 'local' && $canCreateNewLocalStorage === false) { - return new DataResponse( - null, - Http::STATUS_FORBIDDEN - ); - } - + // NOTE: files_external_allow_create_new_local is checked during backend + // registration, and it will remove the related storage visibility + // if applicable. If the storage isn't visible, the `validate` method + // will fail. $newStorage = $this->createStorage( $mountPoint, $backend, diff --git a/apps/files_external/lib/Controller/StoragesController.php b/apps/files_external/lib/Controller/StoragesController.php index 9b4fdf85ab9e..21de089ab642 100644 --- a/apps/files_external/lib/Controller/StoragesController.php +++ b/apps/files_external/lib/Controller/StoragesController.php @@ -183,7 +183,7 @@ protected function validate(IStorageConfig $storage) { $backend->getIdentifier() ]) ], - Http::STATUS_UNPROCESSABLE_ENTITY + Http::STATUS_FORBIDDEN ); } if (!$authMechanism->isVisibleFor($this->service->getVisibilityType())) { @@ -194,7 +194,7 @@ protected function validate(IStorageConfig $storage) { $authMechanism->getIdentifier() ]) ], - Http::STATUS_UNPROCESSABLE_ENTITY + Http::STATUS_FORBIDDEN ); } @@ -308,7 +308,7 @@ public function show($id, $testOnly = true) { } catch (NotFoundException $e) { return new DataResponse( [ - 'message' => (string)$this->l10n->t('Storage with id "%i" not found', [$id]) + 'message' => (string)$this->l10n->t('Storage with id "%d" not found', [$id]) ], Http::STATUS_NOT_FOUND ); @@ -335,7 +335,7 @@ public function destroy($id) { } catch (NotFoundException $e) { return new DataResponse( [ - 'message' => (string)$this->l10n->t('Storage with id "%i" not found', [$id]) + 'message' => (string)$this->l10n->t('Storage with id "%d" not found', [$id]) ], Http::STATUS_NOT_FOUND ); diff --git a/apps/files_external/lib/Controller/UserStoragesController.php b/apps/files_external/lib/Controller/UserStoragesController.php index 8c7a1e4efaae..ef727275e65d 100644 --- a/apps/files_external/lib/Controller/UserStoragesController.php +++ b/apps/files_external/lib/Controller/UserStoragesController.php @@ -31,7 +31,6 @@ use OCP\Files\External\IStorageConfig; use OCP\Files\External\NotFoundException; use OCP\Files\External\Service\IUserStoragesService; -use OCP\IConfig; use OCP\IL10N; use OCP\ILogger; use OCP\IRequest; @@ -45,7 +44,6 @@ class UserStoragesController extends StoragesController { * @var IUserSession */ private $userSession; - private IConfig $config; public function __construct( $AppName, @@ -53,7 +51,6 @@ public function __construct( IL10N $l10n, IUserStoragesService $userStoragesService, IUserSession $userSession, - IConfig $config, ILogger $logger ) { parent::__construct( @@ -64,7 +61,6 @@ public function __construct( $logger ); $this->userSession = $userSession; - $this->config = $config; } protected function manipulateStorageConfig(IStorageConfig $storage) { @@ -82,12 +78,6 @@ protected function manipulateStorageConfig(IStorageConfig $storage) { * @return DataResponse */ public function index() { - if (!$this->isUserMountingAllowed()) { - return new DataResponse( - null, - Http::STATUS_FORBIDDEN - ); - } return parent::index(); } @@ -122,20 +112,11 @@ public function create( $backendOptions, $mountOptions ) { - if (!$this->isUserMountingAllowed()) { - return new DataResponse( - null, - Http::STATUS_FORBIDDEN - ); - } - $canCreateNewLocalStorage = \OC::$server->getConfig()->getSystemValue('files_external_allow_create_new_local', false); - if ($backend === 'local' && $canCreateNewLocalStorage === false) { - return new DataResponse( - null, - Http::STATUS_FORBIDDEN - ); - } - + // NOTE: local backend isn't allowed for regular users regardless + // of the value of files_external_allow_create_new_local. The backend + // won't be visible for regular users, so the `validate` method will fail. + // Only admins can create local storages (if files_external_allow_create_new_local + // is true) $newStorage = $this->createStorage( $mountPoint, $backend, @@ -188,12 +169,6 @@ public function update( $mountOptions, $testOnly = true ) { - if (!$this->isUserMountingAllowed()) { - return new DataResponse( - null, - Http::STATUS_FORBIDDEN - ); - } $storage = $this->createStorage( $mountPoint, $backend, @@ -241,17 +216,6 @@ public function update( * {@inheritdoc} */ public function destroy($id) { - if (!$this->isUserMountingAllowed()) { - return new DataResponse( - null, - Http::STATUS_FORBIDDEN - ); - } - return parent::destroy($id); } - - private function isUserMountingAllowed(): bool { - return $this->config->getAppValue('files_external', 'allow_user_mounting', 'no') === 'yes'; - } } diff --git a/apps/files_external/tests/Controller/GlobalStoragesControllerTest.php b/apps/files_external/tests/Controller/GlobalStoragesControllerTest.php index 6e45b6ca927f..dab05e43b6f0 100644 --- a/apps/files_external/tests/Controller/GlobalStoragesControllerTest.php +++ b/apps/files_external/tests/Controller/GlobalStoragesControllerTest.php @@ -111,6 +111,55 @@ public function testCreate() { $this->assertEquals($expectedStorage, $actual); } + public function localBackendNameProvider() { + return [ + ['local'], + ['\OC\Files\Storage\Local'], + ]; + } + + /** + * @dataProvider localBackendNameProvider + */ + public function testCreateLocal($localBackendName) { + $mount = 'randomMount'; + $backend = "identifier:{$localBackendName}"; + $auth = 'identifier:\Random\Missing\Auth\Class'; + $backendOpts = [ + 'datadir' => '/tmp', + ]; + $priority = 3; + + $storageConfig = $this->getNewStorageConfigMock([ + 'id' => 30, + 'backendClass' => '\OCA\Files_External\Lib\Backend', + 'backendStorageClass' => '\OC\Files\Storage\Local', + 'authClass' => '\Random\Missing\Auth\Class', + 'mountPoint' => $mount, + 'backendOpts' => $backendOpts, + 'priority' => $priority, + 'type' => IStorageConfig::MOUNT_TYPE_ADMIN, + ]); + + $backendMock = $storageConfig->getBackend(); + $backendMock->method('isVisibleFor')->willReturn(false); + $backendMock->method('validateStorage')->willReturn(true); + + $authMock = $storageConfig->getAuthMechanism(); + $authMock->method('isVisibleFor')->willReturn(true); + $authMock->method('validateStorage')->willReturn(true); + + $this->service->expects($this->once()) + ->method('createStorage') + ->willReturn($storageConfig); + $this->service->expects($this->never()) + ->method('addStorage') + ->will($this->returnArgument(0)); + + $result = $this->controller->create($mount, $backend, $auth, $backendOpts, [], [], [], $priority); + $this->assertEquals(Http::STATUS_FORBIDDEN, $result->getStatus()); + } + public function testUpdate() { $mount = 'randomMount'; $backend = 'identifier:\This\Doesnt\Exist'; diff --git a/apps/files_external/tests/Controller/StoragesControllerTest.php b/apps/files_external/tests/Controller/StoragesControllerTest.php index 97cc11a0c3a8..3df57c9d04df 100644 --- a/apps/files_external/tests/Controller/StoragesControllerTest.php +++ b/apps/files_external/tests/Controller/StoragesControllerTest.php @@ -170,7 +170,7 @@ public function testAddStorageWithoutConfig() { $data = $response->getData(); $this->assertEquals(Http::STATUS_FORBIDDEN, $response->getStatus()); - $this->assertNull($data); + $this->assertEquals(['message' => ''], $data); // message comes from translatable error string, which isn't set here. } public function testUpdateStorage() { diff --git a/apps/files_external/tests/Controller/UserStoragesControllerTest.php b/apps/files_external/tests/Controller/UserStoragesControllerTest.php index c20722b73c4f..fe6a14b8e35e 100644 --- a/apps/files_external/tests/Controller/UserStoragesControllerTest.php +++ b/apps/files_external/tests/Controller/UserStoragesControllerTest.php @@ -32,7 +32,6 @@ use OCP\Files\External\Service\IStoragesService; use OCP\Files\StorageNotAvailableException; use OCP\ILogger; -use OCP\IConfig; use OCP\IUserSession; use OCP\IL10N; use OCP\IRequest; @@ -45,44 +44,16 @@ public function setUp(): void { $this->service->method('getVisibilityType') ->willReturn(IStoragesBackendService::VISIBILITY_PERSONAL); - $this->config = $this->createMock(IConfig::class); - $this->config->method('getAppValue')->willReturn('yes'); - $this->controller = new UserStoragesController( 'files_external', $this->createMock(IRequest::class), $this->createMock(IL10N::class), $this->service, $this->createMock(IUserSession::class), - $this->config, $this->createMock(ILogger::class) ); } - public function testApiWhenDisabled(): void { - $config = $this->createMock(IConfig::class); - $config->method('getAppValue')->willReturn('no'); - - $controller = new UserStoragesController( - 'files_external', - $this->createMock(IRequest::class), - $this->createMock(IL10N::class), - $this->service, - $this->createMock(IUserSession::class), - $config, - $this->createMock(ILogger::class) - ); - - $resp = $controller->create( - '', - '', - '', - [], - [], - ); - $this->assertEquals(Http::STATUS_FORBIDDEN, $resp->getStatus()); - } - public function testAddOrUpdateStorageDisallowedBackend() { $backend = $this->getBackendMock(); $backend->method('isVisibleFor') @@ -115,7 +86,7 @@ public function testAddOrUpdateStorageDisallowedBackend() { null ); - $this->assertEquals(Http::STATUS_UNPROCESSABLE_ENTITY, $response->getStatus()); + $this->assertEquals(Http::STATUS_FORBIDDEN, $response->getStatus()); $response = $this->controller->update( 1, @@ -129,7 +100,7 @@ public function testAddOrUpdateStorageDisallowedBackend() { null ); - $this->assertEquals(Http::STATUS_UNPROCESSABLE_ENTITY, $response->getStatus()); + $this->assertEquals(Http::STATUS_FORBIDDEN, $response->getStatus()); } public function testCreate() { @@ -195,6 +166,114 @@ public function testCreate() { $this->assertEquals($expectedStorage, $actual); } + public function localBackendNameProvider() { + return [ + ['local'], + ['\OC\Files\Storage\Local'], + ]; + } + + /** + * @dataProvider localBackendNameProvider + */ + public function testCreateLocal($localBackendName) { + $mount = 'randomMount'; + $backend = "identifier:{$localBackendName}"; + $auth = 'identifier:\Random\Missing\Auth\Class'; + $backendOpts = [ + 'datadir' => '/tmp', + ]; + $priority = 3; + + $storageConfig = $this->getNewStorageConfigMock([ + 'id' => 30, + 'backendClass' => '\OCA\Files_External\Lib\Backend', + 'backendStorageClass' => '\OC\Files\Storage\Local', + 'authClass' => '\Random\Missing\Auth\Class', + 'mountPoint' => $mount, + 'backendOpts' => $backendOpts, + 'priority' => $priority, + 'type' => IStorageConfig::MOUNT_TYPE_ADMIN, + ]); + + $backendMock = $storageConfig->getBackend(); + $backendMock->method('isVisibleFor')->willReturn(true); + $backendMock->method('validateStorage')->willReturn(true); + + $authMock = $storageConfig->getAuthMechanism(); + $authMock->method('isVisibleFor')->willReturn(true); + $authMock->method('validateStorage')->willReturn(true); + + $this->service->expects($this->once()) + ->method('createStorage') + ->willReturn($storageConfig); + $this->service->expects($this->once()) + ->method('addStorage') + ->will($this->returnArgument(0)); + + $expectedStorage = [ + 'id' => 30, + 'mountPoint' => '/randomMount', + 'backend' => "identifier:\OCA\Files_External\Lib\Backend", + 'authMechanism' => 'identifier:\Random\Missing\Auth\Class', + 'backendOptions' => [ + 'datadir' => '/tmp', + ], + 'priority' => 3, + 'userProvided' => false, + 'type' => 'system', + 'status' => StorageNotAvailableException::STATUS_SUCCESS, // status check for the storage is skipped and always returns this value + ]; + + $result = $this->controller->create($mount, $backend, $auth, $backendOpts, [], [], [], $priority); + $actual = $result->getData()->jsonSerialize(); + $this->assertEquals(Http::STATUS_CREATED, $result->getStatus()); + $this->assertEquals($expectedStorage, $actual); + } + + /** + * @dataProvider localBackendNameProvider + */ + public function testCreateLocalNotAllowed($localBackendName) { + $mount = 'randomMount'; + $backend = "identifier:{$localBackendName}"; + $auth = 'identifier:\Random\Missing\Auth\Class'; + $backendOpts = [ + 'datadir' => '/tmp', + ]; + $priority = 3; + + $storageConfig = $this->getNewStorageConfigMock([ + 'id' => 30, + 'backendClass' => '\OCA\Files_External\Lib\Backend', + 'backendStorageClass' => '\OC\Files\Storage\Local', + 'authClass' => '\Random\Missing\Auth\Class', + 'mountPoint' => $mount, + 'backendOpts' => $backendOpts, + 'priority' => $priority, + 'type' => IStorageConfig::MOUNT_TYPE_ADMIN, + ]); + + // if not allowed, the backend must return false for its visibility + $backendMock = $storageConfig->getBackend(); + $backendMock->method('isVisibleFor')->willReturn(false); + $backendMock->method('validateStorage')->willReturn(true); + + $authMock = $storageConfig->getAuthMechanism(); + $authMock->method('isVisibleFor')->willReturn(true); + $authMock->method('validateStorage')->willReturn(true); + + $this->service->expects($this->once()) + ->method('createStorage') + ->willReturn($storageConfig); + $this->service->expects($this->never()) + ->method('addStorage') + ->will($this->returnArgument(0)); + + $result = $this->controller->create($mount, $backend, $auth, $backendOpts, [], [], [], $priority); + $this->assertEquals(Http::STATUS_FORBIDDEN, $result->getStatus()); + } + public function testUpdate() { $mount = 'randomMount'; $backend = 'identifier:\This\Doesnt\Exist'; diff --git a/changelog/10.16.3_2026-05-22/41538 b/changelog/10.16.3_2026-05-22/41538 new file mode 100644 index 000000000000..0598c79b4015 --- /dev/null +++ b/changelog/10.16.3_2026-05-22/41538 @@ -0,0 +1,8 @@ +Bugfix: Prevent mounting local storage if not allowed + +Mounting a local storage was possible if the internal class name was used as +backend, despite local storage not allowed to be mounted. This problem is +fixed and the local storage can't be mounted if it was explicitly disallowed in +the configuration. + +https://github.com/owncloud/core/pull/41538 diff --git a/changelog/10.16.3_2026-05-22/41539 b/changelog/10.16.3_2026-05-22/41539 new file mode 100644 index 000000000000..a5987e91fbde --- /dev/null +++ b/changelog/10.16.3_2026-05-22/41539 @@ -0,0 +1,5 @@ +Bugfix: Use the correct user ID when changing email via admin API + +The admin API endpoint for changing a user's email address was incorrectly using the requesting admin's user ID instead of the target user's ID, causing the admin's email to be updated rather than the intended user's. + +https://github.com/owncloud/core/pull/41539 diff --git a/changelog/10.16.3_2026-05-22/41550 b/changelog/10.16.3_2026-05-22/41550 new file mode 100644 index 000000000000..1d4c65526534 --- /dev/null +++ b/changelog/10.16.3_2026-05-22/41550 @@ -0,0 +1,9 @@ +Security: Restrict AppConfigController read methods to full admins only + +Subadmin users could read all oc_appconfig values including SMTP passwords, +LDAP bind credentials, and encryption master keys via the Settings API. +Removed @NoAdminRequired from getApps, getKeys, and getValue so that the +AdminMiddleware enforces full-admin-only access, consistent with the write +methods. + +https://github.com/owncloud/core/pull/41550 diff --git a/changelog/10.16.3_2026-05-22/41558 b/changelog/10.16.3_2026-05-22/41558 new file mode 100644 index 000000000000..73a55ae4f71e --- /dev/null +++ b/changelog/10.16.3_2026-05-22/41558 @@ -0,0 +1,5 @@ +Bugfix: Prevent IDOR in WebDAV comments API + +Authenticated users could read, edit, or delete comments on files they have no access to by supplying an arbitrary comment ID in the WebDAV comments endpoint. The fix verifies that a requested comment belongs to the file in the URL before returning it. + +https://github.com/owncloud/core/pull/41558 diff --git a/changelog/10.16.3_2026-05-22/phpseclib-security-2026-05 b/changelog/10.16.3_2026-05-22/phpseclib-security-2026-05 new file mode 100644 index 000000000000..19cf69a7dfae --- /dev/null +++ b/changelog/10.16.3_2026-05-22/phpseclib-security-2026-05 @@ -0,0 +1,8 @@ +Security: Update phpseclib to 3.0.52 for CVE-2026-40194 + +CVE-2026-40194: Timing attack vulnerability in SSH binary packet processing. +Upgraded phpseclib/phpseclib from 3.0.50 to 3.0.52. + +https://github.com/owncloud/core/pull/41529 +https://github.com/owncloud/core/pull/41541 +https://github.com/phpseclib/phpseclib/releases/tag/3.0.51 diff --git a/changelog/10.16.3_2026-05-22/symfony-security-2026-05 b/changelog/10.16.3_2026-05-22/symfony-security-2026-05 new file mode 100644 index 000000000000..fa4b5002de85 --- /dev/null +++ b/changelog/10.16.3_2026-05-22/symfony-security-2026-05 @@ -0,0 +1,8 @@ +Security: Update symfony/routing to 5.4.52 for CVE-2026-45065 + +CVE-2026-45065: UrlGenerator route-requirement bypass via unanchored regex +alternation allowing off-site URL injection. Upgraded symfony/routing from +5.4.48 to 5.4.52. + +https://github.com/owncloud/core/pull/41559 +https://symfony.com/cve-2026-45065 diff --git a/composer.lock b/composer.lock index 955230da95b2..2e734ef52ab0 100644 --- a/composer.lock +++ b/composer.lock @@ -2630,16 +2630,16 @@ }, { "name": "phpseclib/phpseclib", - "version": "3.0.50", + "version": "3.0.52", "source": { "type": "git", "url": "https://github.com/phpseclib/phpseclib.git", - "reference": "aa6ad8321ed103dc3624fb600a25b66ebf78ec7b" + "reference": "2adaefc83df2ec548558307690f376dd7d4f4fce" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/phpseclib/phpseclib/zipball/aa6ad8321ed103dc3624fb600a25b66ebf78ec7b", - "reference": "aa6ad8321ed103dc3624fb600a25b66ebf78ec7b", + "url": "https://api.github.com/repos/phpseclib/phpseclib/zipball/2adaefc83df2ec548558307690f376dd7d4f4fce", + "reference": "2adaefc83df2ec548558307690f376dd7d4f4fce", "shasum": "" }, "require": { @@ -2720,7 +2720,7 @@ ], "support": { "issues": "https://github.com/phpseclib/phpseclib/issues", - "source": "https://github.com/phpseclib/phpseclib/tree/3.0.50" + "source": "https://github.com/phpseclib/phpseclib/tree/3.0.52" }, "funding": [ { @@ -2736,7 +2736,7 @@ "type": "tidelift" } ], - "time": "2026-03-19T02:57:58+00:00" + "time": "2026-04-27T07:02:15+00:00" }, { "name": "pimple/pimple", @@ -4328,16 +4328,16 @@ }, { "name": "symfony/routing", - "version": "v5.4.48", + "version": "v5.4.52", "source": { "type": "git", "url": "https://github.com/symfony/routing.git", - "reference": "dd08c19879a9b37ff14fd30dcbdf99a4cf045db1" + "reference": "275b31328b2e58cab004be0cf086380e2a5c5ee7" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/routing/zipball/dd08c19879a9b37ff14fd30dcbdf99a4cf045db1", - "reference": "dd08c19879a9b37ff14fd30dcbdf99a4cf045db1", + "url": "https://api.github.com/repos/symfony/routing/zipball/275b31328b2e58cab004be0cf086380e2a5c5ee7", + "reference": "275b31328b2e58cab004be0cf086380e2a5c5ee7", "shasum": "" }, "require": { @@ -4398,7 +4398,7 @@ "url" ], "support": { - "source": "https://github.com/symfony/routing/tree/v5.4.48" + "source": "https://github.com/symfony/routing/tree/v5.4.52" }, "funding": [ { @@ -4409,12 +4409,16 @@ "url": "https://github.com/fabpot", "type": "github" }, + { + "url": "https://github.com/nicolas-grekas", + "type": "github" + }, { "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", "type": "tidelift" } ], - "time": "2024-11-12T18:20:21+00:00" + "time": "2026-04-16T14:40:03+00:00" }, { "name": "symfony/service-contracts", diff --git a/lib/private/Files/External/StoragesBackendChecker.php b/lib/private/Files/External/StoragesBackendChecker.php new file mode 100644 index 000000000000..3b48bb0b4f87 --- /dev/null +++ b/lib/private/Files/External/StoragesBackendChecker.php @@ -0,0 +1,87 @@ +config = $config; + } + + /** + * Checks if the regular users are allowed to mount external storages + * @return bool + */ + public function isUserMountingAllowed(): bool { + if ($this->allowUserMounting === null) { + $this->allowUserMounting = $this->config->getAppValue('files_external', 'allow_user_mounting', 'no') === 'yes'; + // if no backend is in the list an empty string is in the array and user mounting is disabled + if ($this->allowedBackendsForUsers() === ['']) { + $this->allowUserMounting = false; + } + } + return $this->allowUserMounting; + } + + private function allowedBackendsForUsers() { + if ($this->userMountingBackends === null) { + $user_mounting_backends = $this->config->getAppValue('files_external', 'user_mounting_backends', ''); + $this->userMountingBackends = \explode( + ',', + $user_mounting_backends + ); + } + return $this->userMountingBackends; + } + + /** + * Checks if the regular users are allowed to mount the specified backend. + * Note that the admin might still mount the backend. + * @param Backend $backend + * @return bool + */ + public function isAllowedUserBackend(Backend $backend): bool { + // Allowing users to mount local storage is a security risk, so it's + // blacklisted regardless of admin's decision. The admin can still + // setup a local mount for everyone (or chosen users) if he wants. + $blacklistedBackendsForUsers = ['\OC\Files\Storage\Local']; + if (\in_array($backend->getStorageClass(), $blacklistedBackendsForUsers, true)) { + return false; + } + + if ($this->isUserMountingAllowed() && \array_intersect($backend->getIdentifierAliases(), $this->allowedBackendsForUsers())) { + return true; + } + return false; + } + + /** + * Checks if the admin is allowed to mount the specified backend. + * @param Backend $backend + * @return bool + */ + public function isAllowedAdminBackend(Backend $backend): bool { + if ($this->canCreateNewLocalStorage === null) { + $this->canCreateNewLocalStorage = $this->config->getSystemValue('files_external_allow_create_new_local', false); + } + + if ($backend->getStorageClass() === '\OC\Files\Storage\Local' && !$this->canCreateNewLocalStorage) { + return false; + } + return true; + } +} diff --git a/lib/private/Files/External/StoragesBackendService.php b/lib/private/Files/External/StoragesBackendService.php index 7e845ab307c2..0accca21db33 100644 --- a/lib/private/Files/External/StoragesBackendService.php +++ b/lib/private/Files/External/StoragesBackendService.php @@ -25,6 +25,7 @@ use OCP\IConfig; +use OC\Files\External\StoragesBackendChecker; use OCP\Files\External\Backend\Backend; use OCP\Files\External\Auth\AuthMechanism; use OCP\Files\External\Config\IBackendProvider; @@ -35,14 +36,8 @@ * Service class to manage backend definitions */ class StoragesBackendService implements IStoragesBackendService { - /** @var IConfig */ - protected $config; - - /** @var bool */ - private $userMountingAllowed = true; - - /** @var string[] */ - private $userMountingBackends = []; + /** @var StoragesBackendChecker */ + protected $storagesBackendChecker; /** @var Backend[] */ private $backends = []; @@ -57,26 +52,12 @@ class StoragesBackendService implements IStoragesBackendService { private $authMechanismProviders = []; /** - * @param IConfig $config + * @param StoragesBackendChecker $storagesBackendChecker */ public function __construct( - IConfig $config + StoragesBackendChecker $storagesBackendChecker ) { - $this->config = $config; - - // Load config values - if ($this->config->getAppValue('files_external', 'allow_user_mounting', 'no') !== 'yes') { - $this->userMountingAllowed = false; - } - $this->userMountingBackends = \explode( - ',', - $this->config->getAppValue('files_external', 'user_mounting_backends', '') - ); - - // if no backend is in the list an empty string is in the array and user mounting is disabled - if ($this->userMountingBackends === ['']) { - $this->userMountingAllowed = false; - } + $this->storagesBackendChecker = $storagesBackendChecker; } /** @@ -123,6 +104,9 @@ public function registerBackend(Backend $backend) { if (!$this->isAllowedUserBackend($backend)) { $backend->removeVisibility(IStoragesBackendService::VISIBILITY_PERSONAL); } + if (!$this->isAllowedAdminBackend($backend)) { + $backend->removeVisibility(IStoragesBackendService::VISIBILITY_ADMIN); + } foreach ($backend->getIdentifierAliases() as $alias) { $this->backends[$alias] = $backend; } @@ -243,7 +227,7 @@ public function getAuthMechanism($identifier) { * @return bool */ public function isUserMountingAllowed() { - return $this->userMountingAllowed; + return $this->storagesBackendChecker->isUserMountingAllowed(); } /** @@ -253,12 +237,16 @@ public function isUserMountingAllowed() { * @return bool */ protected function isAllowedUserBackend(Backend $backend) { - if ($this->userMountingAllowed && - \array_intersect($backend->getIdentifierAliases(), $this->userMountingBackends) - ) { - return true; - } - return false; + return $this->storagesBackendChecker->isAllowedUserBackend($backend); + } + + /** + * Checks if the admin is allowed to mount the backend + * @param Backend $backend + * @return bool + */ + protected function isAllowedAdminBackend(Backend $backend) { + return $this->storagesBackendChecker->isAllowedAdminBackend($backend); } /** diff --git a/lib/private/Server.php b/lib/private/Server.php index 46172e9f0d97..f3f7ca4440fd 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -114,6 +114,7 @@ use Symfony\Component\EventDispatcher\EventDispatcher; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use OC\Files\External\StoragesBackendService; +use OC\Files\External\StoragesBackendChecker; use OC\Files\External\Service\UserStoragesService; use OC\Files\External\Service\UserGlobalStoragesService; use OC\Files\External\Service\GlobalStoragesService; @@ -846,7 +847,7 @@ public function __construct($webRoot, \OC\Config $config) { ); }); $this->registerService('StoragesBackendService', function (Server $c) { - $service = new StoragesBackendService($c->query('AllConfig')); + $service = new StoragesBackendService($c->query(StoragesBackendChecker::class)); // register auth mechanisms provided by core $provider = new \OC\Files\External\Auth\CoreAuthMechanismProvider($c, [ diff --git a/settings/Controller/AppConfigController.php b/settings/Controller/AppConfigController.php index 0e8da029fbad..b96339fb5cdf 100644 --- a/settings/Controller/AppConfigController.php +++ b/settings/Controller/AppConfigController.php @@ -27,9 +27,7 @@ /** * The code is mostly copied from core/ajax/appconfig.php - * Read methods (getApps, getKeys and getValue) are available to subadmins, - * which wasn't possible with the core/ajax/appconfig.php file. The rest of - * the methods require admin privileges. + * All methods require full admin privileges. * Note that the "hasKey" method is missing. You can do the same in a lot of * cases by trying to get the value of the key. * @@ -54,8 +52,6 @@ public function __construct( } /** - * @NoAdminRequired - * * Get the list of apps */ public function getApps() { @@ -63,8 +59,6 @@ public function getApps() { } /** - * @NoAdminRequired - * * Get the list of keys for that particular app * @param string $app */ @@ -73,8 +67,6 @@ public function getKeys($app) { } /** - * @NoAdminRequired - * * Get the value of the key for that app, or the default value provided * if it's missing. * @param string $app diff --git a/settings/Controller/UsersController.php b/settings/Controller/UsersController.php index 76cfca776cf4..e37746678b98 100644 --- a/settings/Controller/UsersController.php +++ b/settings/Controller/UsersController.php @@ -972,7 +972,7 @@ public function setMailAddress($id, $mailAddress) { // admins can set email without verification if ($mailAddress === '' || $this->isAdmin) { - $this->setEmailAddress($userId, $mailAddress); + $this->setEmailAddress($id, $mailAddress); return new DataResponse( [ 'status' => 'success', diff --git a/settings/js/users/users.js b/settings/js/users/users.js index a6d666911d1f..968173332d83 100644 --- a/settings/js/users/users.js +++ b/settings/js/users/users.js @@ -751,18 +751,16 @@ $(document).ready(function () { // TODO: move other init calls inside of initialize UserList.initialize($('#userlist')); - OC.AppConfig.getValue('core', 'umgmt_set_password', 'false', function (data) { - var showPassword = $.parseJSON(data); - if (showPassword === true) { + (function () { + var showPassword = $('#CheckBoxPasswordOnUserCreate').prop('checked'); + if (showPassword) { $("#newuserpassword").show(); $("#newemail").hide(); - $('#CheckBoxPasswordOnUserCreate').attr('checked', true); } else { $("#newemail").show(); $("#newuserpassword").hide(); - $('#CheckBoxPasswordOnUserCreate').attr('checked', false); } - }); + }()); $userListBody.on('click', '.password', function (event) { event.stopPropagation(); diff --git a/tests/Settings/Controller/AppConfigControllerTest.php b/tests/Settings/Controller/AppConfigControllerTest.php index 26982b653a20..39fd4b0ed4e3 100644 --- a/tests/Settings/Controller/AppConfigControllerTest.php +++ b/tests/Settings/Controller/AppConfigControllerTest.php @@ -156,4 +156,22 @@ public function testDeleteAppWrong(): void { $this->assertEquals([], $response->getData()); $this->assertSame(Http::STATUS_BAD_REQUEST, $response->getStatus()); } + + public function testGetAppsRequiresAdmin(): void { + $reflection = new \ReflectionMethod(AppConfigController::class, 'getApps'); + $docComment = (string)$reflection->getDocComment(); + $this->assertStringNotContainsString('@NoAdminRequired', $docComment); + } + + public function testGetKeysRequiresAdmin(): void { + $reflection = new \ReflectionMethod(AppConfigController::class, 'getKeys'); + $docComment = (string)$reflection->getDocComment(); + $this->assertStringNotContainsString('@NoAdminRequired', $docComment); + } + + public function testGetValueRequiresAdmin(): void { + $reflection = new \ReflectionMethod(AppConfigController::class, 'getValue'); + $docComment = (string)$reflection->getDocComment(); + $this->assertStringNotContainsString('@NoAdminRequired', $docComment); + } } diff --git a/tests/Settings/Controller/UsersControllerTest.php b/tests/Settings/Controller/UsersControllerTest.php index ebe2b64b1462..09f4638f4ed2 100644 --- a/tests/Settings/Controller/UsersControllerTest.php +++ b/tests/Settings/Controller/UsersControllerTest.php @@ -2178,28 +2178,31 @@ public function setEmailAddressData(): array { * @param string $mailAddress * @param bool $isValid * @param bool $expectsUpdate - * @param bool $chanChangeMailAddress + * @param bool $canChangeMailAddress * @param bool $responseCode */ - public function testSetEmailAddress($mailAddress, $isValid, $expectsUpdate, $chanChangeMailAddress, $responseCode): void { + public function testSetEmailAddress($mailAddress, $isValid, $expectsUpdate, $canChangeMailAddress, $responseCode): void { $this->container['IsAdmin'] = true; - $user = $this->getMockBuilder(User::class) - ->disableOriginalConstructor()->getMock(); - $user - ->method('getUID') - ->willReturn('foo'); - $user - ->method('getEMailAddress') - ->willReturn('foo@local'); - $user - ->method('canChangeMailAddress') - ->willReturn($chanChangeMailAddress); - $user - ->method('setEMailAddress') - ->with( - $this->equalTo($mailAddress) - ); + $user = $this->createMock(User::class); + $user->method('getUID')->willReturn('foo'); + $user->method('getEMailAddress')->willReturn('foo@local'); + $user->method('canChangeMailAddress')->willReturn($canChangeMailAddress); + $user->expects($this->never())->method('setEmailAddress'); + + $user2 = $this->createMock(User::class); + $user2->method('getUID')->willReturn('anotherUserId'); + $user2->method('getEMailAddress')->willReturn('another@local'); + $user2->method('canChangeMailAddress')->willReturn($canChangeMailAddress); + + if ($isValid && $canChangeMailAddress) { + $user2 + ->expects($this->once()) + ->method('setEMailAddress') + ->with( + $this->equalTo($mailAddress) + ); + } $this->container['UserSession'] ->expects($this->atLeastOnce()) @@ -2210,24 +2213,28 @@ public function testSetEmailAddress($mailAddress, $isValid, $expectsUpdate, $cha ->with($mailAddress) ->willReturn($isValid); - if ($isValid) { - $user->expects($this->atLeastOnce()) - ->method('canChangeMailAddress') - ->willReturn(true); - } - $this->container['Config'] ->method('getUserValue') - ->with('foo', 'owncloud', 'changeMail') - ->willReturn('12000:AVerySecretToken'); + ->willReturnMap([ + ['foo', 'owncloud', 'changeMail', '12000:AVerySecretToken'], + ['anotherUserId', 'owncloud', 'changeMail', '120:ASecretToken'], + ]); $this->container['TimeFactory'] ->method('getTime') ->willReturnOnConsecutiveCalls(12301, 12348); $this->container['UserManager'] ->expects($this->atLeastOnce()) ->method('get') - ->with('foo') - ->willReturn($user); + ->willReturnCallback(function ($id) use ($user, $user2) { + switch($id) { + case "foo": + return $user; + case "anotherUserId": + return $user2; + default: + return null; + } + }); $this->container['SecureRandom'] ->method('generate') ->with('21') @@ -2260,7 +2267,7 @@ public function testSetEmailAddress($mailAddress, $isValid, $expectsUpdate, $cha ->method('send') ->with($message); - $response = $this->container['UsersController']->setMailAddress($user->getUID(), $mailAddress); + $response = $this->container['UsersController']->setMailAddress("anotherUserId", $mailAddress); $this->assertSame($responseCode, $response->getStatus()); } diff --git a/tests/lib/Files/External/Service/GlobalStoragesServiceDeleteUserTest.php b/tests/lib/Files/External/Service/GlobalStoragesServiceDeleteUserTest.php index 67e36a61510f..3fa744dfaeca 100644 --- a/tests/lib/Files/External/Service/GlobalStoragesServiceDeleteUserTest.php +++ b/tests/lib/Files/External/Service/GlobalStoragesServiceDeleteUserTest.php @@ -23,6 +23,7 @@ use OC\Files\Config\UserMountCache; use OC\Files\External\Service\DBConfigService; use OC\Files\External\Service\GlobalStoragesService; +use OC\Files\External\StoragesBackendChecker; use OC\Files\External\StoragesBackendService; use OCP\Files\External\Backend\Backend; use OCP\IUser; @@ -125,7 +126,7 @@ public function providesDeleteAllUser() { * @param $storageParams */ public function testDeleteAllForUser($storageParams, $userId) { - $backendService = new StoragesBackendService(\OC::$server->getConfig()); + $backendService = new StoragesBackendService(new StoragesBackendChecker(\OC::$server->getConfig())); $dbConfigService = new DBConfigService(\OC::$server->getDatabaseConnection(), \OC::$server->getCrypto()); $userManager = \OC::$server->getUserManager(); $userMountCache = new UserMountCache(\OC::$server->getDatabaseConnection(), $userManager, \OC::$server->getLogger()); diff --git a/tests/lib/Files/External/StoragesBackendCheckerTest.php b/tests/lib/Files/External/StoragesBackendCheckerTest.php new file mode 100644 index 000000000000..8197687380af --- /dev/null +++ b/tests/lib/Files/External/StoragesBackendCheckerTest.php @@ -0,0 +1,125 @@ +config = $this->createMock(IConfig::class); + + $this->storagesBackendChecker = new StoragesBackendChecker($this->config); + } + + public function isUserMountingAllowedProvider() { + return [ + ['no', '', false], + ['no', 'local,smb', false], + ['yes', '', false], + ['yes', 'local,smb', true], + ['yes', 'smb', true], + ['', 'local,smb', false], + ['random', 'local,smb', false], + ]; + } + + /** + * @dataProvider isUserMountingAllowedProvider + */ + public function testIsUserMountingAllowed($allowUserMounting, $allowedBackends, $expectedResult) { + $this->config->method('getAppValue')->willReturnCallback(function ($app, $key, $default) use ($allowUserMounting, $allowedBackends) { + $map = [ + 'allow_user_mounting' => $allowUserMounting, + 'user_mounting_backends' => $allowedBackends, + ]; + if ($app === 'files_external') { + if (isset($map[$key])) { + return $map[$key]; + } + } + return $default; + }); + + $this->assertEquals($expectedResult, $this->storagesBackendChecker->isUserMountingAllowed()); + } + + public function isAllowedUserBackendProvider() { + $backend1 = $this->createMock(Backend::class); + $backend1->method('getStorageClass')->willReturn('\OC\Files\Storage\DAV'); + $backend1->method('getIdentifierAliases')->willReturn(['\OCA\Files_External\Lib\Backend\DAV', 'DAV']); + $backend2 = $this->createMock(Backend::class); + $backend2->method('getStorageClass')->willReturn('\OC\Files\Storage\Local'); + $backend2->method('getIdentifierAliases')->willReturn(['\OCA\Files_External\Lib\Backend\Local', 'local']); + return [ + ['no', '', $backend1, false], + ['no', '', $backend2, false], + ['no', 'DAV,local', $backend1, false], + ['no', 'DAV,local', $backend2, false], + ['no', '\OCA\Files_External\Lib\Backend\DAV,\OCA\Files_External\Lib\Backend\Local', $backend1, false], + ['no', '\OCA\Files_External\Lib\Backend\DAV,\OCA\Files_External\Lib\Backend\Local', $backend2, false], + ['yes', '', $backend1, false], + ['yes', '', $backend2, false], + ['yes', 'DAV,local', $backend1, true], + ['yes', 'DAV,local', $backend2, false], // local storage always disallowed + ['yes', '\OCA\Files_External\Lib\Backend\DAV,\OCA\Files_External\Lib\Backend\Local', $backend1, true], + ['yes', '\OCA\Files_External\Lib\Backend\DAV,\OCA\Files_External\Lib\Backend\Local', $backend2, false], + ]; + } + + /** + * @dataProvider isAllowedUserBackendProvider + */ + public function testIsAllowedUserBackend($allowUserMounting, $allowedBackends, $backend, $expectedResult) { + $this->config->method('getAppValue')->willReturnCallback(function ($app, $key, $default) use ($allowUserMounting, $allowedBackends) { + $map = [ + 'allow_user_mounting' => $allowUserMounting, + 'user_mounting_backends' => $allowedBackends, + ]; + if ($app === 'files_external') { + if (isset($map[$key])) { + return $map[$key]; + } + } + return $default; + }); + + $this->assertEquals($expectedResult, $this->storagesBackendChecker->isAllowedUserBackend($backend)); + } + + public function isAllowedAdminBackendProvider() { + $backend1 = $this->createMock(Backend::class); + $backend1->method('getStorageClass')->willReturn('\OC\Files\Storage\DAV'); + $backend1->method('getIdentifierAliases')->willReturn(['\OCA\Files_External\Lib\Backend\DAV', 'DAV']); + $backend2 = $this->createMock(Backend::class); + $backend2->method('getStorageClass')->willReturn('\OC\Files\Storage\Local'); + $backend2->method('getIdentifierAliases')->willReturn(['\OCA\Files_External\Lib\Backend\Local', 'local']); + return [ + [false, $backend1, true], + [false, $backend2, false], + [true, $backend1, true], + [true, $backend2, true], + ]; + } + + /** + * @dataProvider isAllowedAdminBackendProvider + */ + public function testIsAllowedAdminBackend($isLocalAllowed, $backend, $expectedResult) { + $this->config->method('getSystemValue')->willReturnCallback(function ($key, $default) use ($isLocalAllowed) { + if ($key === 'files_external_allow_create_new_local') { + return $isLocalAllowed; + } + return $default; + }); + + $this->assertEquals($expectedResult, $this->storagesBackendChecker->isAllowedAdminBackend($backend)); + } +} diff --git a/tests/lib/Files/External/StoragesBackendServiceTest.php b/tests/lib/Files/External/StoragesBackendServiceTest.php index a31d44cd8db5..b9745bd3577e 100644 --- a/tests/lib/Files/External/StoragesBackendServiceTest.php +++ b/tests/lib/Files/External/StoragesBackendServiceTest.php @@ -20,15 +20,21 @@ */ namespace Test\Files\External; +use OC\Files\External\StoragesBackendChecker; use OC\Files\External\StoragesBackendService; use OCP\Files\External\IStoragesBackendService; +use OCP\Files\External\Backend\Backend; +use PHPUnit\Framework\MockObject\MockObject; class StoragesBackendServiceTest extends \Test\TestCase { - /** @var \OCP\IConfig */ - protected $config; + /** @var StoragesBackendChecker */ + protected $storagesBackendChecker; protected function setUp(): void { - $this->config = $this->createMock('\OCP\IConfig'); + $this->storagesBackendChecker = $this->createMock(StoragesBackendChecker::class); + $this->storagesBackendChecker->method('isUserMountingAllowed')->willReturn(false); + $this->storagesBackendChecker->method('isAllowedUserBackend')->willReturn(false); + $this->storagesBackendChecker->method('isAllowedAdminBackend')->willReturn(true); } /** @@ -60,7 +66,7 @@ protected function getAuthMechanismMock($class) { } public function testRegisterBackend() { - $service = new StoragesBackendService($this->config); + $service = new StoragesBackendService($this->storagesBackendChecker); $backend = $this->getBackendMock('\Foo\Bar'); @@ -87,7 +93,7 @@ public function testRegisterBackend() { } public function testBackendProvider() { - $service = new StoragesBackendService($this->config); + $service = new StoragesBackendService($this->storagesBackendChecker); $backend1 = $this->getBackendMock('\Foo\Bar'); $backend2 = $this->getBackendMock('\Bar\Foo'); @@ -105,7 +111,7 @@ public function testBackendProvider() { } public function testAuthMechanismProvider() { - $service = new StoragesBackendService($this->config); + $service = new StoragesBackendService($this->storagesBackendChecker); $backend1 = $this->getAuthMechanismMock('\Foo\Bar'); $backend2 = $this->getAuthMechanismMock('\Bar\Foo'); @@ -123,7 +129,7 @@ public function testAuthMechanismProvider() { } public function testMultipleBackendProviders() { - $service = new StoragesBackendService($this->config); + $service = new StoragesBackendService($this->storagesBackendChecker); $backend1a = $this->getBackendMock('\Foo\Bar'); $backend1b = $this->getBackendMock('\Bar\Foo'); @@ -149,14 +155,18 @@ public function testMultipleBackendProviders() { } public function testUserMountingBackends() { - $this->config->expects($this->exactly(2)) - ->method('getAppValue') - ->will($this->returnValueMap([ - ['files_external', 'allow_user_mounting', 'no', 'yes'], - ['files_external', 'user_mounting_backends', '', 'identifier:\User\Mount\Allowed,identifier_alias'] - ])); - - $service = new StoragesBackendService($this->config); + $storagesBackendChecker = $this->createMock(StoragesBackendChecker::class); + $storagesBackendChecker->method('isUserMountingAllowed')->willReturn(true); + $storagesBackendChecker->method('isAllowedAdminBackend')->willReturn(true); + $storagesBackendChecker->method('isAllowedUserBackend')->willReturnCallback(function (Backend $backend) { + $backendAliases = $backend->getIdentifierAliases(); + if (\in_array('identifier:\User\Mount\Allowed', $backendAliases, true) || \in_array('identifier_alias', $backendAliases, true)) { + return true; + } + return false; + }); + + $service = new StoragesBackendService($storagesBackendChecker); $backendAllowed = $this->getBackendMock('\User\Mount\Allowed'); $backendAllowed->expects($this->never()) @@ -179,8 +189,89 @@ public function testUserMountingBackends() { $service->registerBackend($backendAlias); } + public function testAdminMountingBackends() { + $storagesBackendChecker = $this->createMock(StoragesBackendChecker::class); + $storagesBackendChecker->method('isUserMountingAllowed')->willReturn(true); + $storagesBackendChecker->method('isAllowedUserBackend')->willReturn(true); + $storagesBackendChecker->method('isAllowedAdminBackend')->willReturnCallback(function (Backend $backend) { + $backendAliases = $backend->getIdentifierAliases(); + if (\in_array('identifier:\User\Mount\Allowed', $backendAliases, true) || \in_array('identifier_alias', $backendAliases, true)) { + return true; + } + return false; + }); + + $service = new StoragesBackendService($storagesBackendChecker); + + $backendAllowed = $this->getBackendMock('\User\Mount\Allowed'); + $backendAllowed->expects($this->never()) + ->method('removeVisibility'); + $backendNotAllowed = $this->getBackendMock('\User\Mount\NotAllowed'); + $backendNotAllowed->expects($this->once()) + ->method('removeVisibility') + ->with(IStoragesBackendService::VISIBILITY_ADMIN); + + $backendAlias = $this->getMockBuilder('\OCP\Files\External\Backend\Backend') + ->disableOriginalConstructor() + ->getMock(); + $backendAlias->method('getIdentifierAliases') + ->willReturn(['identifier_real', 'identifier_alias']); + $backendAlias->expects($this->never()) + ->method('removeVisibility'); + + $service->registerBackend($backendAllowed); + $service->registerBackend($backendNotAllowed); + $service->registerBackend($backendAlias); + } + + public function testAdminAndUserMountingBackends() { + $storagesBackendChecker = $this->createMock(StoragesBackendChecker::class); + $storagesBackendChecker->method('isUserMountingAllowed')->willReturn(true); + $storagesBackendChecker->method('isAllowedUserBackend')->willReturnCallback(function (Backend $backend) { + $backendAliases = $backend->getIdentifierAliases(); + if (\in_array('identifier:\User\Mount\Allowed', $backendAliases, true) || \in_array('identifier_alias', $backendAliases, true)) { + return true; + } + return false; + }); + $storagesBackendChecker->method('isAllowedAdminBackend')->willReturnCallback(function (Backend $backend) { + $backendAliases = $backend->getIdentifierAliases(); + if (\in_array('identifier:\User\Mount\Allowed', $backendAliases, true) || \in_array('identifier_alias', $backendAliases, true)) { + return true; + } + return false; + }); + + $service = new StoragesBackendService($storagesBackendChecker); + + $backendAllowed = $this->getBackendMock('\User\Mount\Allowed'); + $backendAllowed->expects($this->never()) + ->method('removeVisibility'); + $backendNotAllowed = $this->getBackendMock('\User\Mount\NotAllowed'); + $backendNotAllowed->expects($this->exactly(2)) + ->method('removeVisibility') + ->with( + $this->logicalOr( + $this->identicalTo(IStoragesBackendService::VISIBILITY_ADMIN), + $this->identicalTo(IStoragesBackendService::VISIBILITY_PERSONAL) + ) + ); + + $backendAlias = $this->getMockBuilder('\OCP\Files\External\Backend\Backend') + ->disableOriginalConstructor() + ->getMock(); + $backendAlias->method('getIdentifierAliases') + ->willReturn(['identifier_real', 'identifier_alias']); + $backendAlias->expects($this->never()) + ->method('removeVisibility'); + + $service->registerBackend($backendAllowed); + $service->registerBackend($backendNotAllowed); + $service->registerBackend($backendAlias); + } + public function testGetAvailableBackends() { - $service = new StoragesBackendService($this->config); + $service = new StoragesBackendService($this->storagesBackendChecker); $backendAvailable = $this->getBackendMock('\Backend\Available'); $backendAvailable->expects($this->once()) diff --git a/version.php b/version.php index bb942d356f73..b5b81911aeef 100644 --- a/version.php +++ b/version.php @@ -25,10 +25,10 @@ // We only can count up. The 4. digit is only for the internal patch-level to trigger DB upgrades // between betas, final and RCs. This is _not_ the public version number. Reset minor/patch-level // when updating major/minor version number. -$OC_Version = [10, 16, 2, 0]; +$OC_Version = [10, 16, 3, 0]; // The human-readable string -$OC_VersionString = '10.16.2'; +$OC_VersionString = '10.16.3'; $OC_VersionCanBeUpgradedFrom = [[8, 2, 11],[9, 0, 9],[9, 1]];