diff --git a/apps/files_sharing/lib/Config/ConfigLexicon.php b/apps/files_sharing/lib/Config/ConfigLexicon.php index a463b4e7ef2..fb2821e7f56 100644 --- a/apps/files_sharing/lib/Config/ConfigLexicon.php +++ b/apps/files_sharing/lib/Config/ConfigLexicon.php @@ -22,6 +22,7 @@ use NCU\Config\ValueType; */ class ConfigLexicon implements IConfigLexicon { public const SHOW_FEDERATED_AS_INTERNAL = 'show_federated_shares_as_internal'; + public const SHOW_FEDERATED_TO_TRUSTED_AS_INTERNAL = 'show_federated_shares_to_trusted_servers_as_internal'; public function getStrictness(): ConfigLexiconStrictness { return ConfigLexiconStrictness::IGNORE; @@ -30,6 +31,7 @@ class ConfigLexicon implements IConfigLexicon { public function getAppConfigs(): array { return [ new ConfigLexiconEntry(self::SHOW_FEDERATED_AS_INTERNAL, ValueType::BOOL, false, 'shows federated shares as internal shares', true), + new ConfigLexiconEntry(self::SHOW_FEDERATED_TO_TRUSTED_AS_INTERNAL, ValueType::BOOL, false, 'shows federated shares to trusted servers as internal shares', true), ]; } diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 0ab3a02f2ff..8268cdec9bc 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -12,6 +12,8 @@ namespace OCA\Files_Sharing\Controller; use Exception; use OC\Files\Storage\Wrapper\Wrapper; use OCA\Circles\Api\v1\Circles; +use OCA\Deck\Sharing\ShareAPIHelper; +use OCA\Federation\TrustedServers; use OCA\Files\Helper; use OCA\Files_Sharing\Exceptions\SharingRightsException; use OCA\Files_Sharing\External\Storage; @@ -72,6 +74,7 @@ use Psr\Log\LoggerInterface; class ShareAPIController extends OCSController { private ?Node $lockedNode = null; + private array $trustedServerCache = []; /** * Share20OCS constructor. @@ -94,6 +97,8 @@ class ShareAPIController extends OCSController { private LoggerInterface $logger, private IProviderFactory $factory, private IMailer $mailer, + private ITagManager $tagManager, + private ?TrustedServers $trustedServers, private ?string $userId = null, ) { parent::__construct($appName, $request); @@ -196,6 +201,32 @@ class ShareAPIController extends OCSController { $result['item_size'] = $node->getSize(); $result['item_mtime'] = $node->getMTime(); + if ($this->trustedServers !== null && in_array($share->getShareType(), [IShare::TYPE_REMOTE, IShare::TYPE_REMOTE_GROUP], true)) { + $result['is_trusted_server'] = false; + $sharedWith = $share->getSharedWith(); + $remoteIdentifier = is_string($sharedWith) ? strrchr($sharedWith, '@') : false; + if ($remoteIdentifier !== false) { + $remote = substr($remoteIdentifier, 1); + + if (isset($this->trustedServerCache[$remote])) { + $result['is_trusted_server'] = $this->trustedServerCache[$remote]; + } else { + try { + $isTrusted = $this->trustedServers->isTrustedServer($remote); + $this->trustedServerCache[$remote] = $isTrusted; + $result['is_trusted_server'] = $isTrusted; + } catch (\Exception $e) { + // Server not found or other issue, we consider it not trusted + $this->trustedServerCache[$remote] = false; + $this->logger->error( + 'Error checking if remote server is trusted (treating as untrusted): ' . $e->getMessage(), + ['exception' => $e] + ); + } + } + } + } + $expiration = $share->getExpirationDate(); if ($expiration !== null) { $expiration->setTimezone($this->dateTimeZone->getTimeZone()); diff --git a/apps/files_sharing/lib/Listener/LoadSidebarListener.php b/apps/files_sharing/lib/Listener/LoadSidebarListener.php index 9f0eee9159a..88c39f38545 100644 --- a/apps/files_sharing/lib/Listener/LoadSidebarListener.php +++ b/apps/files_sharing/lib/Listener/LoadSidebarListener.php @@ -38,6 +38,7 @@ class LoadSidebarListener implements IEventListener { $appConfig = Server::get(IAppConfig::class); $this->initialState->provideInitialState('showFederatedSharesAsInternal', $appConfig->getValueBool('files_sharing', ConfigLexicon::SHOW_FEDERATED_AS_INTERNAL)); + $this->initialState->provideInitialState('showFederatedSharesToTrustedServersAsInternal', $appConfig->getValueBool('files_sharing', ConfigLexicon::SHOW_FEDERATED_TO_TRUSTED_AS_INTERNAL)); Util::addScript(Application::APP_ID, 'files_sharing_tab', 'files'); } } diff --git a/apps/files_sharing/lib/ResponseDefinitions.php b/apps/files_sharing/lib/ResponseDefinitions.php index 6b6b0fcc4b6..71a2b25a70c 100644 --- a/apps/files_sharing/lib/ResponseDefinitions.php +++ b/apps/files_sharing/lib/ResponseDefinitions.php @@ -22,6 +22,7 @@ namespace OCA\Files_Sharing; * file_target: string, * has_preview: bool, * hide_download: 0|1, + * is_trusted_server?: bool, * is-mount-root: bool, * id: string, * item_mtime: int, diff --git a/apps/files_sharing/openapi.json b/apps/files_sharing/openapi.json index 68420dde80e..af725b11aab 100644 --- a/apps/files_sharing/openapi.json +++ b/apps/files_sharing/openapi.json @@ -548,6 +548,9 @@ 1 ] }, + "is_trusted_server": { + "type": "boolean" + }, "is-mount-root": { "type": "boolean" }, diff --git a/apps/files_sharing/src/components/SharingEntry.vue b/apps/files_sharing/src/components/SharingEntry.vue index ae58855c9b1..9d50b188d0e 100644 --- a/apps/files_sharing/src/components/SharingEntry.vue +++ b/apps/files_sharing/src/components/SharingEntry.vue @@ -74,9 +74,9 @@ export default { title += ` (${t('files_sharing', 'group')})` } else if (this.share.type === ShareType.Room) { title += ` (${t('files_sharing', 'conversation')})` - } else if (this.share.type === ShareType.Remote) { + } else if (this.share.type === ShareType.Remote && !this.share.isTrustedServer) { title += ` (${t('files_sharing', 'remote')})` - } else if (this.share.type === ShareType.RemoteGroup) { + } else if (this.share.type === ShareType.RemoteGroup && !this.share.isTrustedServer) { title += ` (${t('files_sharing', 'remote group')})` } else if (this.share.type === ShareType.Guest) { title += ` (${t('files_sharing', 'guest')})` diff --git a/apps/files_sharing/src/components/SharingInput.vue b/apps/files_sharing/src/components/SharingInput.vue index 5be17ffb555..4ae5faefaa3 100644 --- a/apps/files_sharing/src/components/SharingInput.vue +++ b/apps/files_sharing/src/components/SharingInput.vue @@ -192,14 +192,27 @@ export default { lookup = true } - let shareType = [] - const remoteTypes = [ShareType.Remote, ShareType.RemoteGroup] - - if (this.isExternal && !this.config.showFederatedSharesAsInternal) { - shareType.push(...remoteTypes) + const shareType = [] + + const showFederatedAsInternal + = this.config.showFederatedSharesAsInternal + || this.config.showFederatedSharesToTrustedServersAsInternal + + const shouldAddRemoteTypes + // For internal users, add remote types if config says to show them as internal + = (!this.isExternal && showFederatedAsInternal) + // For external users, add them if config *doesn't* say to show them as internal + || (this.isExternal && !showFederatedAsInternal) + // Edge case: federated-to-trusted is a separate "add" trigger for external users + || (this.isExternal && this.config.showFederatedSharesToTrustedServersAsInternal) + + if (this.isExternal) { + if (getCapabilities().files_sharing.public.enabled === true) { + shareType.push(ShareType.Email) + } } else { - shareType = shareType.concat([ + shareType.push( ShareType.User, ShareType.Group, ShareType.Team, @@ -207,15 +220,11 @@ export default { ShareType.Guest, ShareType.Deck, ShareType.ScienceMesh, - ]) - - if (this.config.showFederatedSharesAsInternal) { - shareType.push(...remoteTypes) - } + ) } - if (getCapabilities().files_sharing.public.enabled === true && this.isExternal) { - shareType.push(ShareType.Email) + if (shouldAddRemoteTypes) { + shareType.push(...remoteTypes) } let request = null @@ -366,6 +375,11 @@ export default { // filter out existing mail shares if (share.value.shareType === ShareType.Email) { + // When sharing internally, we don't want to suggest email addresses + // that the user previously created shares to + if (!this.isExternal) { + return arr + } const emails = this.linkShares.map(elem => elem.shareWith) if (emails.indexOf(share.value.shareWith.trim()) !== -1) { return arr diff --git a/apps/files_sharing/src/models/Share.ts b/apps/files_sharing/src/models/Share.ts index fb76a655d53..b0638b29448 100644 --- a/apps/files_sharing/src/models/Share.ts +++ b/apps/files_sharing/src/models/Share.ts @@ -486,4 +486,11 @@ export default class Share { return this._share.status } + /** + * Is the share from a trusted server + */ + get isTrustedServer(): boolean { + return !!this._share.is_trusted_server + } + } diff --git a/apps/files_sharing/src/services/ConfigService.ts b/apps/files_sharing/src/services/ConfigService.ts index 2114e2d1bae..f75f34c7936 100644 --- a/apps/files_sharing/src/services/ConfigService.ts +++ b/apps/files_sharing/src/services/ConfigService.ts @@ -315,4 +315,12 @@ export default class Config { return loadState('files_sharing', 'showFederatedSharesAsInternal', false) } + /** + * Show federated shares to trusted servers as internal shares + * @return {boolean} + */ + get showFederatedSharesToTrustedServersAsInternal(): boolean { + return loadState('files_sharing', 'showFederatedSharesToTrustedServersAsInternal', false) + } + } diff --git a/apps/files_sharing/src/views/SharingLinkList.vue b/apps/files_sharing/src/views/SharingLinkList.vue index 3dd6fdf317b..c3d9a7f83dc 100644 --- a/apps/files_sharing/src/views/SharingLinkList.vue +++ b/apps/files_sharing/src/views/SharingLinkList.vue @@ -7,12 +7,6 @@ diff --git a/apps/files_sharing/src/views/SharingTab.vue b/apps/files_sharing/src/views/SharingTab.vue index f7be524ec6d..bf0ed658e03 100644 --- a/apps/files_sharing/src/views/SharingTab.vue +++ b/apps/files_sharing/src/views/SharingTab.vue @@ -399,7 +399,13 @@ export default { if ([ShareType.Link, ShareType.Email].includes(share.type)) { this.linkShares.push(share) } else if ([ShareType.Remote, ShareType.RemoteGroup].includes(share.type)) { - if (this.config.showFederatedSharesAsInternal) { + if (this.config.showFederatedSharesToTrustedServersAsInternal) { + if (share.isTrustedServer) { + this.shares.push(share) + } else { + this.externalShares.push(share) + } + } else if (this.config.showFederatedSharesAsInternal) { this.shares.push(share) } else { this.externalShares.push(share) @@ -475,6 +481,10 @@ export default { } else if ([ShareType.Remote, ShareType.RemoteGroup].includes(share.type)) { if (this.config.showFederatedSharesAsInternal) { this.shares.unshift(share) + } if (this.config.showFederatedSharesToTrustedServersAsInternal) { + if (share.isTrustedServer) { + this.shares.unshift(share) + } } else { this.externalShares.unshift(share) } diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php index c20644fc2fc..a537540292e 100644 --- a/apps/files_sharing/tests/ApiTest.php +++ b/apps/files_sharing/tests/ApiTest.php @@ -11,6 +11,7 @@ use OC\Files\FileInfo; use OC\Files\Filesystem; use OC\Files\Storage\Temporary; use OC\Files\View; +use OCA\Federation\TrustedServers; use OCA\Files_Sharing\Controller\ShareAPIController; use OCP\App\IAppManager; use OCP\AppFramework\OCS\OCSBadRequestException; @@ -24,6 +25,7 @@ use OCP\IDateTimeZone; use OCP\IL10N; use OCP\IPreview; use OCP\IRequest; +use OCP\ITagManager; use OCP\Mail\IMailer; use OCP\Share\IProviderFactory; use OCP\Share\IShare; @@ -106,6 +108,8 @@ class ApiTest extends TestCase { $logger = $this->createMock(LoggerInterface::class); $providerFactory = $this->createMock(IProviderFactory::class); $mailer = $this->createMock(IMailer::class); + $tagManager = $this->createMock(ITagManager::class); + $trustedServers = $this->createMock(TrustedServers::class); $dateTimeZone->method('getTimeZone')->willReturn(new \DateTimeZone(date_default_timezone_get())); return new ShareAPIController( @@ -126,6 +130,8 @@ class ApiTest extends TestCase { $logger, $providerFactory, $mailer, + $tagManager, + $trustedServers, $userId, ); } diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php index ac788dd6b28..4ced55ab7c9 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -6,6 +6,7 @@ */ namespace OCA\Files_Sharing\Tests\Controller; +use OCA\Federation\TrustedServers; use OCA\Files_Sharing\Controller\ShareAPIController; use OCP\App\IAppManager; use OCP\AppFramework\Http\DataResponse; @@ -28,6 +29,7 @@ use OCP\IGroupManager; use OCP\IL10N; use OCP\IPreview; use OCP\IRequest; +use OCP\ITagManager; use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserManager; @@ -75,6 +77,8 @@ class ShareAPIControllerTest extends TestCase { private LoggerInterface&MockObject $logger; private IProviderFactory&MockObject $factory; private IMailer&MockObject $mailer; + private ITagManager&MockObject $tagManager; + private TrustedServers&MockObject $trustedServers; protected function setUp(): void { $this->shareManager = $this->createMock(IManager::class); @@ -110,6 +114,8 @@ class ShareAPIControllerTest extends TestCase { $this->logger = $this->createMock(LoggerInterface::class); $this->factory = $this->createMock(IProviderFactory::class); $this->mailer = $this->createMock(IMailer::class); + $this->tagManager = $this->createMock(ITagManager::class); + $this->trustedServers = $this->createMock(TrustedServers::class); $this->ocs = new ShareAPIController( $this->appName, @@ -129,8 +135,11 @@ class ShareAPIControllerTest extends TestCase { $this->logger, $this->factory, $this->mailer, - $this->currentUser, + $this->tagManager, + $this->trustedServers, + $this->currentUser ); + } /** @@ -156,6 +165,8 @@ class ShareAPIControllerTest extends TestCase { $this->logger, $this->factory, $this->mailer, + $this->tagManager, + $this->trustedServers, $this->currentUser, ])->onlyMethods(['formatShare']) ->getMock(); @@ -840,6 +851,8 @@ class ShareAPIControllerTest extends TestCase { $this->logger, $this->factory, $this->mailer, + $this->tagManager, + $this->trustedServers, $this->currentUser, ]) @@ -1473,6 +1486,8 @@ class ShareAPIControllerTest extends TestCase { $this->logger, $this->factory, $this->mailer, + $this->tagManager, + $this->trustedServers, $this->currentUser, ]) ->onlyMethods(['formatShare']) @@ -1815,6 +1830,8 @@ class ShareAPIControllerTest extends TestCase { $this->logger, $this->factory, $this->mailer, + $this->tagManager, + $this->trustedServers, $this->currentUser, ])->setMethods(['formatShare']) ->getMock(); @@ -1912,6 +1929,8 @@ class ShareAPIControllerTest extends TestCase { $this->logger, $this->factory, $this->mailer, + $this->tagManager, + $this->trustedServers, $this->currentUser, ])->setMethods(['formatShare']) ->getMock(); @@ -2337,6 +2356,8 @@ class ShareAPIControllerTest extends TestCase { $this->logger, $this->factory, $this->mailer, + $this->tagManager, + $this->trustedServers, $this->currentUser, ])->setMethods(['formatShare']) ->getMock(); @@ -2407,6 +2428,8 @@ class ShareAPIControllerTest extends TestCase { $this->logger, $this->factory, $this->mailer, + $this->tagManager, + $this->trustedServers, $this->currentUser, ])->setMethods(['formatShare']) ->getMock(); @@ -2638,6 +2661,8 @@ class ShareAPIControllerTest extends TestCase { $this->logger, $this->factory, $this->mailer, + $this->tagManager, + $this->trustedServers, $this->currentUser, ])->setMethods(['formatShare']) ->getMock(); @@ -4430,6 +4455,7 @@ class ShareAPIControllerTest extends TestCase { 'mount-type' => '', 'attributes' => null, 'item_permissions' => 1, + 'is_trusted_server' => false, ], $share, [], false ]; @@ -4483,6 +4509,7 @@ class ShareAPIControllerTest extends TestCase { 'mount-type' => '', 'attributes' => null, 'item_permissions' => 1, + 'is_trusted_server' => false, ], $share, [], false ]; @@ -5137,4 +5164,139 @@ class ShareAPIControllerTest extends TestCase { $node->method('getId')->willReturn(42); return [$userFolder, $node]; } + + + public function trustedServerProvider(): array { + return [ + 'Trusted server' => [true, true], + 'Untrusted server' => [false, false], + ]; + } + + /** + * @dataProvider trustedServerProvider + */ + public function testFormatShareWithFederatedShare(bool $isKnownServer, bool $isTrusted): void { + $nodeId = 12; + $nodePath = '/test.txt'; + $share = $this->createShare( + 1, + IShare::TYPE_REMOTE, + 'recipient@remoteserver.com', // shared with + 'sender@testserver.com', // shared by + 'shareOwner', // share owner + $nodePath, // path + Constants::PERMISSION_READ, + time(), + null, + null, + $nodePath, + $nodeId + ); + + $node = $this->createMock(\OCP\Files\File::class); + $node->method('getId')->willReturn($nodeId); + $node->method('getPath')->willReturn($nodePath); + $node->method('getInternalPath')->willReturn(ltrim($nodePath, '/')); + $mountPoint = $this->createMock(\OCP\Files\Mount\IMountPoint::class); + $mountPoint->method('getMountType')->willReturn('local'); + $node->method('getMountPoint')->willReturn($mountPoint); + $node->method('getMimetype')->willReturn('text/plain'); + $storage = $this->createMock(\OCP\Files\Storage\IStorage::class); + $storageCache = $this->createMock(\OCP\Files\Cache\ICache::class); + $storageCache->method('getNumericStorageId')->willReturn(1); + $storage->method('getCache')->willReturn($storageCache); + $storage->method('getId')->willReturn('home::shareOwner'); + $node->method('getStorage')->willReturn($storage); + $parent = $this->createMock(\OCP\Files\Folder::class); + $parent->method('getId')->willReturn(2); + $node->method('getParent')->willReturn($parent); + $node->method('getSize')->willReturn(1234); + $node->method('getMTime')->willReturn(1234567890); + + $this->previewManager->method('isAvailable')->with($node)->willReturn(false); + + $this->rootFolder->method('getUserFolder') + ->with($this->currentUser) + ->willReturnSelf(); + + $this->rootFolder->method('getFirstNodeById') + ->with($share->getNodeId()) + ->willReturn($node); + + $this->rootFolder->method('getRelativePath') + ->with($node->getPath()) + ->willReturnArgument(0); + + $serverName = 'remoteserver.com'; + $this->trustedServers->method('isTrustedServer') + ->with($serverName) + ->willReturn($isKnownServer); + + $result = $this->invokePrivate($this->ocs, 'formatShare', [$share]); + + $this->assertSame($isTrusted, $result['is_trusted_server']); + } + + public function testFormatShareWithFederatedShareWithAtInUsername(): void { + $nodeId = 12; + $nodePath = '/test.txt'; + $share = $this->createShare( + 1, + IShare::TYPE_REMOTE, + 'recipient@domain.com@remoteserver.com', + 'sender@testserver.com', + 'shareOwner', + $nodePath, + Constants::PERMISSION_READ, + time(), + null, + null, + $nodePath, + $nodeId + ); + + $node = $this->createMock(\OCP\Files\File::class); + $node->method('getId')->willReturn($nodeId); + $node->method('getPath')->willReturn($nodePath); + $node->method('getInternalPath')->willReturn(ltrim($nodePath, '/')); + $mountPoint = $this->createMock(\OCP\Files\Mount\IMountPoint::class); + $mountPoint->method('getMountType')->willReturn('local'); + $node->method('getMountPoint')->willReturn($mountPoint); + $node->method('getMimetype')->willReturn('text/plain'); + $storage = $this->createMock(\OCP\Files\Storage\IStorage::class); + $storageCache = $this->createMock(\OCP\Files\Cache\ICache::class); + $storageCache->method('getNumericStorageId')->willReturn(1); + $storage->method('getCache')->willReturn($storageCache); + $storage->method('getId')->willReturn('home::shareOwner'); + $node->method('getStorage')->willReturn($storage); + $parent = $this->createMock(\OCP\Files\Folder::class); + $parent->method('getId')->willReturn(2); + $node->method('getParent')->willReturn($parent); + $node->method('getSize')->willReturn(1234); + $node->method('getMTime')->willReturn(1234567890); + + $this->previewManager->method('isAvailable')->with($node)->willReturn(false); + + $this->rootFolder->method('getUserFolder') + ->with($this->currentUser) + ->willReturnSelf(); + + $this->rootFolder->method('getFirstNodeById') + ->with($share->getNodeId()) + ->willReturn($node); + + $this->rootFolder->method('getRelativePath') + ->with($node->getPath()) + ->willReturnArgument(0); + + $serverName = 'remoteserver.com'; + $this->trustedServers->method('isTrustedServer') + ->with($serverName) + ->willReturn(true); + + $result = $this->invokePrivate($this->ocs, 'formatShare', [$share]); + + $this->assertTrue($result['is_trusted_server']); + } } diff --git a/cypress/e2e/files_sharing/public-share/download.cy.ts b/cypress/e2e/files_sharing/public-share/download.cy.ts index 3afae7139f1..aa300722b3e 100644 --- a/cypress/e2e/files_sharing/public-share/download.cy.ts +++ b/cypress/e2e/files_sharing/public-share/download.cy.ts @@ -212,8 +212,6 @@ describe('files_sharing: Public share - downloading files', { testIsolation: tru cy.reload() - getRowForFile('test').should('be.visible') - triggerActionForFile('test', 'details') openLinkShareDetails(0) cy.findByRole('checkbox', { name: /hide download/i }) .should('be.checked') @@ -257,7 +255,7 @@ describe('files_sharing: Public share - downloading files', { testIsolation: tru cy.wait('@update') - openLinkShareDetails(1) + openLinkShareDetails(0) cy.findByRole('button', { name: /advanced settings/i }) .click() cy.findByRole('checkbox', { name: /hide download/i })