feat(files_sharing): Toggle display for trusted server shares

Signed-off-by: nfebe <fenn25.fn@gmail.com>
pull/54149/head
nfebe 2025-06-24 13:16:22 +07:00
parent 384ad001e2
commit dea8324912
14 changed files with 269 additions and 26 deletions

@ -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),
];
}

@ -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());

@ -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');
}
}

@ -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,

@ -548,6 +548,9 @@
1
]
},
"is_trusted_server": {
"type": "boolean"
},
"is-mount-root": {
"type": "boolean"
},

@ -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')})`

@ -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

@ -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
}
}

@ -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)
}
}

@ -7,12 +7,6 @@
<ul v-if="canLinkShare"
:aria-label="t('files_sharing', 'Link shares')"
class="sharing-link-list">
<!-- If no link shares, show the add link default entry -->
<SharingEntryLink v-if="!hasLinkShares && canReshare"
:can-reshare="canReshare"
:file-info="fileInfo"
@add:share="addShare" />
<!-- Else we display the list -->
<template v-if="hasShares">
<!-- using shares[index] to work with .sync -->
@ -27,6 +21,12 @@
@remove:share="removeShare"
@open-sharing-details="openSharingDetails(share)" />
</template>
<!-- If no link shares, show the add link default entry -->
<SharingEntryLink v-if="!hasLinkShares && canReshare"
:can-reshare="canReshare"
:file-info="fileInfo"
@add:share="addShare" />
</ul>
</template>

@ -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)
}

@ -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,
);
}

@ -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']);
}
}

@ -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 })