From c78d1fe908f638203ec6e5c977f8df394842dafe Mon Sep 17 00:00:00 2001 From: skjnldsv Date: Thu, 26 Dec 2024 14:12:33 +0100 Subject: [PATCH] feat(federatedfilesharing): auto-accept shares from trusted servers Signed-off-by: skjnldsv --- .../Controller/RequestHandlerController.php | 3 - .../lib/FederatedShareProvider.php | 5 + .../lib/OCM/CloudFederationProviderFiles.php | 13 +++ .../lib/Settings/Admin.php | 2 + .../src/components/AdminSettings.vue | 7 ++ .../lib/BackgroundJob/GetSharedSecret.php | 2 +- .../lib/BackgroundJob/RequestSharedSecret.php | 2 +- apps/files_sharing/lib/External/Manager.php | 93 ++++++++++++------- 8 files changed, 91 insertions(+), 36 deletions(-) diff --git a/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php b/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php index 63de8ff605ea5..90e7f53da80a2 100644 --- a/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php +++ b/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php @@ -39,9 +39,6 @@ #[OpenAPI(scope: OpenAPI::SCOPE_FEDERATION)] class RequestHandlerController extends OCSController { - /** @var string */ - private $shareTable = 'share'; - public function __construct( string $appName, IRequest $request, diff --git a/apps/federatedfilesharing/lib/FederatedShareProvider.php b/apps/federatedfilesharing/lib/FederatedShareProvider.php index 139c873b0d6e4..4d2157c1b44da 100644 --- a/apps/federatedfilesharing/lib/FederatedShareProvider.php +++ b/apps/federatedfilesharing/lib/FederatedShareProvider.php @@ -999,6 +999,11 @@ public function isLookupServerUploadEnabled() { return ($result === 'yes'); } + public function isFederatedTrustedShareAutoAccept() { + $result = $this->config->getAppValue('files_sharing', 'federatedTrustedShareAutoAccept', 'yes'); + return ($result === 'yes'); + } + /** * @inheritdoc */ diff --git a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php index 5c633c0fbbfdb..9a35201a84e0a 100644 --- a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php +++ b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php @@ -10,6 +10,7 @@ use OC\Files\Filesystem; use OCA\FederatedFileSharing\AddressHandler; use OCA\FederatedFileSharing\FederatedShareProvider; +use OCA\Federation\TrustedServers; use OCA\Files_Sharing\Activity\Providers\RemoteShares; use OCA\Files_Sharing\External\Manager; use OCA\GlobalSiteSelector\Service\SlaveService; @@ -66,6 +67,7 @@ public function __construct( private LoggerInterface $logger, private IFilenameValidator $filenameValidator, private readonly IProviderFactory $shareProviderFactory, + private TrustedServers $trustedServers, ) { } @@ -163,6 +165,11 @@ public function shareReceived(ICloudFederationShare $share) { ->setObject('remote_share', $shareId, $name); \OC::$server->getActivityManager()->publish($event); $this->notifyAboutNewShare($shareWith, $shareId, $ownerFederatedId, $sharedByFederatedId, $name, $ownerDisplayName); + + // If auto-accept is enabled, accept the share + if ($this->federatedShareProvider->isFederatedTrustedShareAutoAccept()) { + $this->externalShareManager->acceptShare($shareId, $shareWith); + } } else { $groupMembers = $this->groupManager->get($shareWith)->getUsers(); foreach ($groupMembers as $user) { @@ -174,8 +181,14 @@ public function shareReceived(ICloudFederationShare $share) { ->setObject('remote_share', $shareId, $name); \OC::$server->getActivityManager()->publish($event); $this->notifyAboutNewShare($user->getUID(), $shareId, $ownerFederatedId, $sharedByFederatedId, $name, $ownerDisplayName); + + // If auto-accept is enabled, accept the share + if ($this->federatedShareProvider->isFederatedTrustedShareAutoAccept()) { + $this->externalShareManager->acceptShare($shareId, $user->getUID()); + } } } + return $shareId; } catch (\Exception $e) { $this->logger->error('Server can not add remote share.', [ diff --git a/apps/federatedfilesharing/lib/Settings/Admin.php b/apps/federatedfilesharing/lib/Settings/Admin.php index 1343513e65ae1..e21c34638adaf 100644 --- a/apps/federatedfilesharing/lib/Settings/Admin.php +++ b/apps/federatedfilesharing/lib/Settings/Admin.php @@ -40,6 +40,7 @@ public function getForm() { $this->initialState->provideInitialState('incomingServer2serverGroupShareEnabled', $this->fedShareProvider->isIncomingServer2serverGroupShareEnabled()); $this->initialState->provideInitialState('lookupServerEnabled', $this->fedShareProvider->isLookupServerQueriesEnabled()); $this->initialState->provideInitialState('lookupServerUploadEnabled', $this->fedShareProvider->isLookupServerUploadEnabled()); + $this->initialState->provideInitialState('federatedTrustedShareAutoAccept', $this->fedShareProvider->isFederatedTrustedShareAutoAccept()); return new TemplateResponse('federatedfilesharing', 'settings-admin', [], ''); } @@ -76,6 +77,7 @@ public function getAuthorizedAppConfig(): array { 'incomingServer2serverGroupShareEnabled', 'lookupServerEnabled', 'lookupServerUploadEnabled', + 'federatedTrustedShareAutoAccept', ], ]; } diff --git a/apps/federatedfilesharing/src/components/AdminSettings.vue b/apps/federatedfilesharing/src/components/AdminSettings.vue index dfafe64c0622b..4f2049942d54f 100644 --- a/apps/federatedfilesharing/src/components/AdminSettings.vue +++ b/apps/federatedfilesharing/src/components/AdminSettings.vue @@ -43,6 +43,12 @@ @update:checked="update('lookupServerUploadEnabled', lookupServerUploadEnabled)"> {{ t('federatedfilesharing', 'Allow people to publish their data to a global and public address book') }} + + + {{ t('federatedfilesharing', 'Automatically accept shares from federated accounts and groups by default') }} + @@ -74,6 +80,7 @@ export default { federatedGroupSharingSupported: loadState('federatedfilesharing', 'federatedGroupSharingSupported'), lookupServerEnabled: loadState('federatedfilesharing', 'lookupServerEnabled'), lookupServerUploadEnabled: loadState('federatedfilesharing', 'lookupServerUploadEnabled'), + federatedTrustedShareAutoAccept: loadState('federatedfilesharing', 'federatedTrustedShareAutoAccept'), internalOnly: loadState('federatedfilesharing', 'internalOnly'), sharingFederatedDocUrl: loadState('federatedfilesharing', 'sharingFederatedDocUrl'), } diff --git a/apps/federation/lib/BackgroundJob/GetSharedSecret.php b/apps/federation/lib/BackgroundJob/GetSharedSecret.php index 01dbf7b80b6ef..fe3b360dd18db 100644 --- a/apps/federation/lib/BackgroundJob/GetSharedSecret.php +++ b/apps/federation/lib/BackgroundJob/GetSharedSecret.php @@ -44,7 +44,7 @@ public function __construct( private LoggerInterface $logger, private IDiscoveryService $ocsDiscoveryService, ITimeFactory $timeFactory, - private IConfig $config + private IConfig $config, ) { parent::__construct($timeFactory); $this->httpClient = $httpClientService->newClient(); diff --git a/apps/federation/lib/BackgroundJob/RequestSharedSecret.php b/apps/federation/lib/BackgroundJob/RequestSharedSecret.php index 6691e39e68223..cc50ee75ca089 100644 --- a/apps/federation/lib/BackgroundJob/RequestSharedSecret.php +++ b/apps/federation/lib/BackgroundJob/RequestSharedSecret.php @@ -48,7 +48,7 @@ public function __construct( private IDiscoveryService $ocsDiscoveryService, private LoggerInterface $logger, ITimeFactory $timeFactory, - private IConfig $config + private IConfig $config, ) { parent::__construct($timeFactory); $this->httpClient = $httpClientService->newClient(); diff --git a/apps/files_sharing/lib/External/Manager.php b/apps/files_sharing/lib/External/Manager.php index ad37a8e0cf8a6..01ac00daa2152 100644 --- a/apps/files_sharing/lib/External/Manager.php +++ b/apps/files_sharing/lib/External/Manager.php @@ -129,7 +129,7 @@ public function addShare($remote, $token, $password, $name, $owner, $shareType, 'mountpoint' => $mountPoint, 'owner' => $owner ]; - return $this->mountShare($options); + return $this->mountShare($options, $user); } /** @@ -214,11 +214,12 @@ private function fetchUserShare($parentId, $uid) { * @param int $id share id * @return mixed share of false */ - public function getShare($id) { + public function getShare(int $id, ?string $user = null): array|false { + $user = $user ?? $this->uid; $share = $this->fetchShare($id); // check if the user is allowed to access it - if ($this->canAccessShare($share)) { + if ($this->canAccessShare($share, $user)) { return $share; } @@ -235,14 +236,14 @@ public function getShareByToken(string $token): array|false { $share = $this->fetchShareByToken($token); // check if the user is allowed to access it - if ($this->canAccessShare($share)) { + if ($this->canAccessShare($share, $this->uid)) { return $share; } return false; } - private function canAccessShare(array $share): bool { + private function canAccessShare(array $share, string $user): bool { $validShare = isset($share['share_type']) && isset($share['user']); if (!$validShare) { @@ -251,7 +252,7 @@ private function canAccessShare(array $share): bool { // If the share is a user share, check if the user is the recipient if ((int)$share['share_type'] === IShare::TYPE_USER - && $share['user'] === $this->uid) { + && $share['user'] === $user) { return true; } @@ -265,7 +266,7 @@ private function canAccessShare(array $share): bool { $groupShare = $share; } - $user = $this->userManager->get($this->uid); + $user = $this->userManager->get($user); if ($this->groupManager->get($groupShare['user'])->inGroup($user)) { return true; } @@ -294,13 +295,22 @@ private function updateAccepted(int $shareId, bool $accepted) : void { * @param int $id * @return bool True if the share could be accepted, false otherwise */ - public function acceptShare($id) { - $share = $this->getShare($id); + public function acceptShare(int $id, ?string $user = null) { + // If we're auto-accepting a share, we need to know the user id + // as there is no session available while processing the share + // from the remote server request. + $user = $user ?? $this->uid; + if ($user === null) { + $this->logger->error('No user specified for accepting share'); + return false; + } + + $share = $this->getShare($id, $user); $result = false; if ($share) { - \OC_Util::setupFS($this->uid); - $shareFolder = Helper::getShareFolder(null, $this->uid); + \OC_Util::setupFS($user); + $shareFolder = Helper::getShareFolder(null, $user); $mountPoint = Files::buildNotExistingFileName($shareFolder, $share['name']); $mountPoint = Filesystem::normalizePath($mountPoint); $hash = md5($mountPoint); @@ -313,14 +323,14 @@ public function acceptShare($id) { `mountpoint` = ?, `mountpoint_hash` = ? WHERE `id` = ? AND `user` = ?'); - $userShareAccepted = $acceptShare->execute([1, $mountPoint, $hash, $id, $this->uid]); + $userShareAccepted = $acceptShare->execute([1, $mountPoint, $hash, $id, $user]); } else { $parentId = (int)$share['parent']; if ($parentId !== -1) { // this is the sub-share $subshare = $share; } else { - $subshare = $this->fetchUserShare($id, $this->uid); + $subshare = $this->fetchUserShare($id, $user); } if ($subshare !== null) { @@ -331,7 +341,7 @@ public function acceptShare($id) { `mountpoint` = ?, `mountpoint_hash` = ? WHERE `id` = ? AND `user` = ?'); - $acceptShare->execute([1, $mountPoint, $hash, $subshare['id'], $this->uid]); + $acceptShare->execute([1, $mountPoint, $hash, $subshare['id'], $user]); $result = true; } catch (Exception $e) { $this->logger->emergency('Could not update share', ['exception' => $e]); @@ -345,7 +355,7 @@ public function acceptShare($id) { $share['password'], $share['name'], $share['owner'], - $this->uid, + $user, $mountPoint, $hash, 1, $share['remote_id'], $id, @@ -357,17 +367,18 @@ public function acceptShare($id) { } } } + if ($userShareAccepted !== false) { $this->sendFeedbackToRemote($share['remote'], $share['share_token'], $share['remote_id'], 'accept'); $event = new FederatedShareAddedEvent($share['remote']); $this->eventDispatcher->dispatchTyped($event); - $this->eventDispatcher->dispatchTyped(new InvalidateMountCacheEvent($this->userManager->get($this->uid))); + $this->eventDispatcher->dispatchTyped(new InvalidateMountCacheEvent($this->userManager->get($user))); $result = true; } } // Make sure the user has no notification for something that does not exist anymore. - $this->processNotification($id); + $this->processNotification($id, $user); return $result; } @@ -378,17 +389,23 @@ public function acceptShare($id) { * @param int $id * @return bool True if the share could be declined, false otherwise */ - public function declineShare($id) { - $share = $this->getShare($id); + public function declineShare(int $id, ?string $user = null) { + $user = $user ?? $this->uid; + if ($user === null) { + $this->logger->error('No user specified for declining share'); + return false; + } + + $share = $this->getShare($id, $user); $result = false; if ($share && (int)$share['share_type'] === IShare::TYPE_USER) { $removeShare = $this->connection->prepare(' DELETE FROM `*PREFIX*share_external` WHERE `id` = ? AND `user` = ?'); - $removeShare->execute([$id, $this->uid]); + $removeShare->execute([$id, $user]); $this->sendFeedbackToRemote($share['remote'], $share['share_token'], $share['remote_id'], 'decline'); - $this->processNotification($id); + $this->processNotification($id, $user); $result = true; } elseif ($share && (int)$share['share_type'] === IShare::TYPE_GROUP) { $parentId = (int)$share['parent']; @@ -396,7 +413,7 @@ public function declineShare($id) { // this is the sub-share $subshare = $share; } else { - $subshare = $this->fetchUserShare($id, $this->uid); + $subshare = $this->fetchUserShare($id, $user); } if ($subshare !== null) { @@ -415,7 +432,7 @@ public function declineShare($id) { $share['password'], $share['name'], $share['owner'], - $this->uid, + $user, $share['mountpoint'], $share['mountpoint_hash'], 0, @@ -428,16 +445,27 @@ public function declineShare($id) { $result = false; } } - $this->processNotification($id); + $this->processNotification($id, $user); } return $result; } - public function processNotification(int $remoteShare): void { + public function processNotification(int $remoteShare, ?string $user = null): void { + $user = $user ?? $this->uid; + if ($user === null) { + $this->logger->error('No user specified for processing notification'); + return; + } + + $share = $this->fetchShare($remoteShare); + if ($share === false) { + return; + } + $filter = $this->notificationManager->createNotification(); $filter->setApp('files_sharing') - ->setUser($this->uid) + ->setUser($user) ->setObject('remote_share', (string)$remoteShare); $this->notificationManager->markProcessed($filter); } @@ -537,9 +565,10 @@ protected function stripPath($path) { return rtrim(substr($path, strlen($prefix)), '/'); } - public function getMount($data) { + public function getMount($data, ?string $user = null) { + $user = $user ?? $this->uid; $data['manager'] = $this; - $mountPoint = '/' . $this->uid . '/files' . $data['mountpoint']; + $mountPoint = '/' . $user . '/files' . $data['mountpoint']; $data['mountpoint'] = $mountPoint; $data['certificateManager'] = \OC::$server->getCertificateManager(); return new Mount(self::STORAGE, $mountPoint, $data, $this, $this->storageLoader); @@ -549,8 +578,8 @@ public function getMount($data) { * @param array $data * @return Mount */ - protected function mountShare($data) { - $mount = $this->getMount($data); + protected function mountShare($data, ?string $user = null) { + $mount = $this->getMount($data, $user); $this->mountManager->addMount($mount); return $mount; } @@ -767,6 +796,8 @@ public function getAcceptedShares() { * @return list list of open server-to-server shares */ private function getShares($accepted) { + // Not allowing providing a user here, + // as we only want to retrieve shares for the current user. $user = $this->userManager->get($this->uid); $groups = $this->groupManager->getUserGroups($user); $userGroups = []; @@ -779,7 +810,7 @@ private function getShares($accepted) { ->from('share_external') ->where( $qb->expr()->orX( - $qb->expr()->eq('user', $qb->createNamedParameter($this->uid)), + $qb->expr()->eq('user', $qb->createNamedParameter($user->getUID())), $qb->expr()->in( 'user', $qb->createNamedParameter($userGroups, IQueryBuilder::PARAM_STR_ARRAY)