Skip to content

Commit

Permalink
fixup! feat(federatedfilesharing): auto-accept shares from trusted se…
Browse files Browse the repository at this point in the history 
…rvers

Signed-off-by: skjnldsv <[email protected]>
  • Loading branch information
@skjnldsv
skjnldsv committed Dec 26, 2024
1 parent f381724 commit e29a165
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 39 deletions.
2 changes: 1 addition & 1 deletion apps/federation/lib/BackgroundJob/GetSharedSecret.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion apps/federation/lib/BackgroundJob/RequestSharedSecret.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
88 changes: 51 additions & 37 deletions apps/files_sharing/lib/External/Manager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -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;
}

Expand All @@ -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) {
Expand All @@ -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;
}

Expand All @@ -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;
}
Expand Down Expand Up @@ -294,24 +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, $userId = null) {
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.
if ($userId !== null) {
$user = $this->userManager->get($userId);
if ($user === null) {
return false;
}
$this->uid = $userId;
$user = $user ?? $this->uid;
if ($user === null) {
$this->logger->error('No user specified for accepting share');
return false;
}

$share = $this->getShare($id);
$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);
Expand All @@ -324,14 +323,14 @@ public function acceptShare($id, $userId = null) {
`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) {
Expand All @@ -342,7 +341,7 @@ public function acceptShare($id, $userId = null) {
`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]);
Expand All @@ -356,7 +355,7 @@ public function acceptShare($id, $userId = null) {
$share['password'],
$share['name'],
$share['owner'],
$this->uid,
$user,
$mountPoint, $hash, 1,
$share['remote_id'],
$id,
Expand All @@ -373,13 +372,13 @@ public function acceptShare($id, $userId = null) {
$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;
}
Expand All @@ -390,25 +389,31 @@ public function acceptShare($id, $userId = null) {
* @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'];
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) {
Expand All @@ -427,7 +432,7 @@ public function declineShare($id) {
$share['password'],
$share['name'],
$share['owner'],
$this->uid,
$user,
$share['mountpoint'],
$share['mountpoint_hash'],
0,
Expand All @@ -440,21 +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);
}
Expand Down Expand Up @@ -554,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);
Expand All @@ -566,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;
}
Expand Down Expand Up @@ -784,6 +796,8 @@ public function getAcceptedShares() {
* @return list<Files_SharingRemoteShare> 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 = [];
Expand All @@ -796,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)
Expand Down

0 comments on commit e29a165

Please sign in to comment.