Skip to content

Commit

Permalink
enh(sharing): enable unsharing for sharees for DAV shares (calendars,…
Browse files Browse the repository at this point in the history
… address books)

Signed-off-by: Anna Larch <anna@nextcloud.com>
  • Loading branch information
miaulalala committed Jan 25, 2024
1 parent cb43fca commit 3131714
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 21 deletions.
8 changes: 0 additions & 8 deletions apps/dav/lib/CalDAV/Calendar.php
Original file line number Diff line number Diff line change
Expand Up @@ -236,14 +236,6 @@ public function delete() {
if (isset($this->calendarInfo['{http://owncloud.org/ns}owner-principal']) &&
$this->calendarInfo['{http://owncloud.org/ns}owner-principal'] !== $this->calendarInfo['principaluri']) {
$principal = 'principal:' . parent::getOwner();
$shares = $this->caldavBackend->getShares($this->getResourceId());
$shares = array_filter($shares, function ($share) use ($principal) {
return $share['href'] === $principal;
});
if (empty($shares)) {
throw new Forbidden();
}

$this->caldavBackend->updateShares($this, [], [
$principal
]);
Expand Down
4 changes: 4 additions & 0 deletions apps/dav/lib/Connector/Sabre/DavAclPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
*/
namespace OCA\DAV\Connector\Sabre;

use OCA\DAV\CalDAV\Calendar;
use OCA\DAV\CardDAV\AddressBook;
use Sabre\CalDAV\Principal\User;
use Sabre\DAV\Exception\NotFound;
Expand Down Expand Up @@ -58,6 +59,9 @@ public function checkPrivileges($uri, $privileges, $recursion = self::R_PARENT,
case AddressBook::class:
$type = 'Addressbook';
break;
case Calendar::class:
$type = 'Calendar';
break;
default:
$type = 'Node';
break;
Expand Down
49 changes: 36 additions & 13 deletions apps/dav/lib/DAV/Sharing/Backend.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ class Backend {
public const ACCESS_OWNER = 1;
public const ACCESS_READ_WRITE = 2;
public const ACCESS_READ = 3;
// 4 is already in use for public calendars
public const UNSHARE_USER = 5;

private CappedMemoryCache $shareCache;

Expand Down Expand Up @@ -160,11 +162,13 @@ private function unshare(IShareable $shareable, string $element): void {
}

$query = $this->db->getQueryBuilder();
$query->delete('dav_shares')
->where($query->expr()->eq('resourceid', $query->createNamedParameter($shareable->getResourceId())))
->andWhere($query->expr()->eq('type', $query->createNamedParameter($this->resourceType)))
->andWhere($query->expr()->eq('principaluri', $query->createNamedParameter($parts[1])))
;
$query->insert('dav_shares')
->values([
'principaluri' => $query->createNamedParameter($parts[1]),
'type' => $query->createNamedParameter($this->resourceType),
'access' => $query->createNamedParameter(self::UNSHARE_USER),
'resourceid' => $query->createNamedParameter($shareable->getResourceId())
]);
$query->executeStatement();
}

Expand All @@ -181,26 +185,29 @@ private function unshare(IShareable $shareable, string $element): void {
* @return list<array{href: string, commonName: string, status: int, readOnly: bool, '{http://owncloud.org/ns}principal': string, '{http://owncloud.org/ns}group-share': bool}>

Check failure on line 185 in apps/dav/lib/DAV/Sharing/Backend.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

InvalidReturnType

apps/dav/lib/DAV/Sharing/Backend.php:185:13: InvalidReturnType: The declared return type 'list<array{'{http://owncloud.org/ns}group-share': bool, '{http://owncloud.org/ns}principal': string, commonName: string, href: string, readOnly: bool, status: int}>' for OCA\DAV\DAV\Sharing\Backend::getShares is incorrect, got 'list<array{'{http://owncloud.org/ns}group-share': bool, '{http://owncloud.org/ns}principal': string, access: int, commonName: string, href: non-empty-string, readOnly: bool, resourceid: mixed, status: 1}>' which is different due to additional array shape fields (resourceid, access) (see https://psalm.dev/011)
*/
public function getShares(int $resourceId): array {
$cached = $this->shareCache->get($resourceId);
if ($cached) {
return $cached;
}
// $cached = $this->shareCache->get($resourceId);
// if ($cached) {
// return $cached;
// }
$query = $this->db->getQueryBuilder();
$result = $query->select(['principaluri', 'access'])
$result = $query->select(['principaluri', 'access', 'resourceid'])
->from('dav_shares')
->where($query->expr()->eq('resourceid', $query->createNamedParameter($resourceId, IQueryBuilder::PARAM_INT)))
->andWhere($query->expr()->eq('type', $query->createNamedParameter($this->resourceType)))
->groupBy(['principaluri', 'access'])
->groupBy(['principaluri', 'access', 'resourceid'])
->executeQuery();

$shares = [];
while ($row = $result->fetch()) {
$rows = $result->fetchAll();
foreach($rows as $row) {
$p = $this->principalBackend->getPrincipalByPath($row['principaluri']);
$shares[] = [
'resourceid' => $row['resourceid'],
'href' => "principal:{$row['principaluri']}",
'commonName' => isset($p['{DAV:}displayname']) ? (string)$p['{DAV:}displayname'] : '',
'status' => 1,
'readOnly' => (int) $row['access'] === self::ACCESS_READ,
'access' => (int) $row['access'],
'{http://owncloud.org/ns}principal' => (string)$row['principaluri'],
'{http://owncloud.org/ns}group-share' => isset($p['uri']) ? str_starts_with($p['uri'], 'principals/groups') : false
];
Expand Down Expand Up @@ -254,6 +261,9 @@ public function preloadShares(array $resourceIds): void {
public function applyShareAcl(int $resourceId, array $acl): array {
$shares = $this->getShares($resourceId);
foreach ($shares as $share) {
if($share['access'] === self::UNSHARE_USER) {

Check failure

Code scanning / Psalm

InvalidArrayOffset Error

Cannot access value on variable $share using offset value of 'access', expecting 'href', 'commonName', 'status', 'readOnly', '{http://owncloud.org/ns}principal' or '{http://owncloud.org/ns}group-share'

Check failure on line 264 in apps/dav/lib/DAV/Sharing/Backend.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

InvalidArrayOffset

apps/dav/lib/DAV/Sharing/Backend.php:264:7: InvalidArrayOffset: Cannot access value on variable $share using offset value of 'access', expecting 'href', 'commonName', 'status', 'readOnly', '{http://owncloud.org/ns}principal' or '{http://owncloud.org/ns}group-share' (see https://psalm.dev/115)
continue;
}
$acl[] = [
'privilege' => '{DAV:}read',
'principal' => $share['{' . \OCA\DAV\DAV\Sharing\Plugin::NS_OWNCLOUD . '}principal'],
Expand All @@ -275,6 +285,19 @@ public function applyShareAcl(int $resourceId, array $acl): array {
];
}
}
return $acl;

$principalsToRemove = array_map(function (array $unshare) {
return $unshare['{' . \OCA\DAV\DAV\Sharing\Plugin::NS_OWNCLOUD . '}principal'];
}, array_filter($shares, function (array $share) {
return $share['access'] === self::UNSHARE_USER;

Check failure

Code scanning / Psalm

InvalidArrayOffset Error

Cannot access value on variable $share using offset value of 'access', expecting 'href', 'commonName', 'status', 'readOnly', '{http://owncloud.org/ns}principal' or '{http://owncloud.org/ns}group-share'

Check failure on line 292 in apps/dav/lib/DAV/Sharing/Backend.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

InvalidArrayOffset

apps/dav/lib/DAV/Sharing/Backend.php:292:11: InvalidArrayOffset: Cannot access value on variable $share using offset value of 'access', expecting 'href', 'commonName', 'status', 'readOnly', '{http://owncloud.org/ns}principal' or '{http://owncloud.org/ns}group-share' (see https://psalm.dev/115)
}));

$toRemove = array_filter(array_map(function ($entry) use ($principalsToRemove) {
if(in_array($entry['principal'], $principalsToRemove)) {
return $entry;
}
}, $acl));

return array_diff_key($acl, $toRemove);

Check failure

Code scanning / Psalm

LessSpecificReturnStatement Error

The type 'array<int<0, max>, array{principal: string, privilege: string, protected: bool}>' is more general than the declared return type 'list<array{principal: string, privilege: string, protected: bool}>' for OCA\DAV\DAV\Sharing\Backend::applyShareAcl

Check failure on line 301 in apps/dav/lib/DAV/Sharing/Backend.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

LessSpecificReturnStatement

apps/dav/lib/DAV/Sharing/Backend.php:301:10: LessSpecificReturnStatement: The type 'array<int<0, max>, array{principal: string, privilege: string, protected: bool}>' is more general than the declared return type 'list<array{principal: string, privilege: string, protected: bool}>' for OCA\DAV\DAV\Sharing\Backend::applyShareAcl (see https://psalm.dev/129)
}
}

0 comments on commit 3131714

Please sign in to comment.