Skip to content

Commit e7fc3ef

Browse files
salmart-devsalmart-dev
committed
WIP: add support for permissions in operation
Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com>
1 parent 8050413 commit e7fc3ef

3 files changed

Lines changed: 78 additions & 29 deletions

File tree

‎lib/CacheWrapper.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,10 @@ protected function formatCacheEntry($entry) {
4545
$jailedPath = $storage->getJailedPath($path);
4646
$path = $jailedPath ?? $path;
4747
}
48-
$this->operation->checkFileAccess($path, $this->mountPoint, $entry['mimetype'] === 'httpd/unix-directory', $entry);
48+
$permissions = $this->operation->checkFileAccess($path, $this->mountPoint, $entry['mimetype'] === 'httpd/unix-directory', $entry, 0);
49+
if ($permissions !== null) {
50+
$entry['permissions'] &= $permissions;
51+
}
4952
} catch (ForbiddenException) {
5053
$entry['permissions'] &= $this->mask;
5154
}

‎lib/Operation.php‎

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use OC\Files\Node\Folder;
1414
use OC\Files\View;
1515
use OCA\WorkflowEngine\Entity\File;
16+
use OCP\Constants;
1617
use OCP\EventDispatcher\Event;
1718
use OCP\Files\Cache\ICacheEntry;
1819
use OCP\Files\ForbiddenException;
@@ -46,18 +47,19 @@ public function __construct(
4647

4748
/**
4849
* @param array|ICacheEntry|null $cacheEntry
50+
* @return int|null If access is not blocked, the permissions allowed by the operations or null if not relevant.
4951
* @throws ForbiddenException
5052
*/
51-
public function checkFileAccess(string $path, IMountPoint $mountPoint, bool $isDir, $cacheEntry = null): void {
53+
public function checkFileAccess(string $path, IMountPoint $mountPoint, bool $isDir, $cacheEntry = null, int $requiredPermissions = Constants::PERMISSION_ALL): ?int {
5254
if (!$this->isBlockablePath($mountPoint, $path) || $this->isCreatingSkeletonFiles() || $this->nestingLevel !== 0) {
5355
// Allow creating skeletons and theming
5456
// https://github.com/nextcloud/files_accesscontrol/issues/5
5557
// https://github.com/nextcloud/files_accesscontrol/issues/12
56-
return;
58+
return null;
5759
}
5860
$storage = $mountPoint->getStorage();
5961
if ($storage === null) {
60-
return;
62+
return null;
6163
}
6264

6365
$this->nestingLevel++;
@@ -70,16 +72,43 @@ public function checkFileAccess(string $path, IMountPoint $mountPoint, bool $isD
7072
$ruleMatcher->setEntitySubject($this->fileEntity, $node);
7173
}
7274
$ruleMatcher->setOperation($this);
73-
$match = $ruleMatcher->getFlows();
75+
$match = $ruleMatcher->getFlows(false);
7476

7577
$this->nestingLevel--;
7678

7779
if (!empty($match)) {
80+
$isDenied = false;
81+
$computedPermissions = 0;
82+
foreach ($match as $operation) {
83+
$operationString = $operation['operation'];
84+
if ($operationString === 'deny') {
85+
$isDenied = true;
86+
// block file access as if a deny operation is present it should take precedence
87+
break;
88+
}
89+
90+
try {
91+
$parsedOperationParams = json_decode($operationString, true, flags: JSON_THROW_ON_ERROR);
92+
} catch (\JsonException) {
93+
// if we can't decode as JSON ignore...
94+
continue;
95+
}
96+
97+
$computedPermissions |= (int)($parsedOperationParams['permissions'] ?? 0);
98+
}
99+
100+
if (!$isDenied && ($computedPermissions & $requiredPermissions) === $requiredPermissions) {
101+
// enough permissions to perform the operation
102+
return $computedPermissions;
103+
}
104+
78105
$e = new \RuntimeException('Access denied for path ' . $path . ' that is ' . ($isDir ? '' : 'not ') . 'a directory and matches rules: ' . (string)json_encode($match));
79106
$this->logger->debug($e->getMessage(), ['exception' => $e]);
80107
// All Checks of one operation matched: prevent access
81108
throw new ForbiddenException('Access denied by access control', false);
82109
}
110+
111+
return null;
83112
}
84113

85114
protected function isBlockablePath(IMountPoint $mountPoint, string $path): bool {

‎lib/StorageWrapper.php‎

Lines changed: 41 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,8 @@ public function __construct($parameters) {
4242
/**
4343
* @throws ForbiddenException
4444
*/
45-
protected function checkFileAccess(string $path, ?bool $isDir = null): void {
46-
$this->operation->checkFileAccess($path, $this->mount, is_bool($isDir) ? $isDir : $this->is_dir($path));
45+
protected function checkFileAccess(string $path, ?bool $isDir = null, ?int $permissions = null): ?int {
46+
return $this->operation->checkFileAccess($path, $this->mount, is_bool($isDir) ? $isDir : $this->is_dir($path), null, $permissions ?? Constants::PERMISSION_ALL);
4747
}
4848

4949
/*
@@ -59,7 +59,7 @@ protected function checkFileAccess(string $path, ?bool $isDir = null): void {
5959
*/
6060
#[\Override]
6161
public function mkdir($path): bool {
62-
$this->checkFileAccess($path, true);
62+
$this->checkFileAccess($path, true, Constants::PERMISSION_CREATE);
6363
return $this->storage->mkdir($path);
6464
}
6565

@@ -72,7 +72,7 @@ public function mkdir($path): bool {
7272
*/
7373
#[\Override]
7474
public function rmdir($path): bool {
75-
$this->checkFileAccess($path, true);
75+
$this->checkFileAccess($path, true, Constants::PERMISSION_DELETE);
7676
return $this->storage->rmdir($path);
7777
}
7878

@@ -85,7 +85,7 @@ public function rmdir($path): bool {
8585
#[\Override]
8686
public function isCreatable($path): bool {
8787
try {
88-
$this->checkFileAccess($path);
88+
$this->checkFileAccess($path, null, Constants::PERMISSION_CREATE);
8989
} catch (ForbiddenException $e) {
9090
return false;
9191
}
@@ -101,7 +101,7 @@ public function isCreatable($path): bool {
101101
#[\Override]
102102
public function isReadable($path): bool {
103103
try {
104-
$this->checkFileAccess($path);
104+
$this->checkFileAccess($path, null, Constants::PERMISSION_READ);
105105
} catch (ForbiddenException $e) {
106106
return false;
107107
}
@@ -117,7 +117,7 @@ public function isReadable($path): bool {
117117
#[\Override]
118118
public function isUpdatable($path): bool {
119119
try {
120-
$this->checkFileAccess($path);
120+
$this->checkFileAccess($path, null, Constants::PERMISSION_UPDATE);
121121
} catch (ForbiddenException $e) {
122122
return false;
123123
}
@@ -133,7 +133,7 @@ public function isUpdatable($path): bool {
133133
#[\Override]
134134
public function isDeletable($path): bool {
135135
try {
136-
$this->checkFileAccess($path);
136+
$this->checkFileAccess($path, null, Constants::PERMISSION_DELETE);
137137
} catch (ForbiddenException $e) {
138138
return false;
139139
}
@@ -143,10 +143,16 @@ public function isDeletable($path): bool {
143143
#[\Override]
144144
public function getPermissions($path): int {
145145
try {
146-
$this->checkFileAccess($path);
146+
$permissions = $this->checkFileAccess($path, null, 0);
147147
} catch (ForbiddenException $e) {
148148
return $this->mask;
149149
}
150+
151+
if ($permissions !== null) {
152+
// override with permissions granted by the operation, if any
153+
return $permissions & $this->storage->getPermissions($path);
154+
}
155+
150156
return $this->storage->getPermissions($path);
151157
}
152158

@@ -159,7 +165,7 @@ public function getPermissions($path): int {
159165
*/
160166
#[\Override]
161167
public function file_get_contents($path): string|false {
162-
$this->checkFileAccess($path, false);
168+
$this->checkFileAccess($path, false, Constants::PERMISSION_READ);
163169
return $this->storage->file_get_contents($path);
164170
}
165171

@@ -173,7 +179,7 @@ public function file_get_contents($path): string|false {
173179
*/
174180
#[\Override]
175181
public function file_put_contents(string $path, mixed $data): int|float|false {
176-
$this->checkFileAccess($path, false);
182+
$this->checkFileAccess($path, false, Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE);
177183
return $this->storage->file_put_contents($path, $data);
178184
}
179185

@@ -186,7 +192,7 @@ public function file_put_contents(string $path, mixed $data): int|float|false {
186192
*/
187193
#[\Override]
188194
public function unlink($path): bool {
189-
$this->checkFileAccess($path, false);
195+
$this->checkFileAccess($path, false, Constants::PERMISSION_DELETE);
190196
return $this->storage->unlink($path);
191197
}
192198

@@ -201,8 +207,8 @@ public function unlink($path): bool {
201207
#[\Override]
202208
public function rename($source, $target): bool {
203209
$isDir = $this->is_dir($source);
204-
$this->checkFileAccess($source, $isDir);
205-
$this->checkFileAccess($target, $isDir);
210+
$this->checkFileAccess($source, $isDir, Constants::PERMISSION_READ | Constants::PERMISSION_DELETE);
211+
$this->checkFileAccess($target, $isDir, Constants::PERMISSION_CREATE);
206212
return $this->storage->rename($source, $target);
207213
}
208214

@@ -217,8 +223,8 @@ public function rename($source, $target): bool {
217223
#[\Override]
218224
public function copy($source, $target): bool {
219225
$isDir = $this->is_dir($source);
220-
$this->checkFileAccess($source, $isDir);
221-
$this->checkFileAccess($target, $isDir);
226+
$this->checkFileAccess($source, $isDir, Constants::PERMISSION_READ);
227+
$this->checkFileAccess($target, $isDir, Constants::PERMISSION_CREATE);
222228
return $this->storage->copy($source, $target);
223229
}
224230

@@ -232,7 +238,18 @@ public function copy($source, $target): bool {
232238
*/
233239
#[\Override]
234240
public function fopen($path, $mode) {
235-
$this->checkFileAccess($path, false);
241+
$hasPlus = str_contains($mode, '+');
242+
$isRead = str_contains($mode, 'r');
243+
$isExclusive = str_contains($mode, 'x');
244+
$isWac = str_contains($mode, 'w') || str_contains($mode, 'a') || str_contains($mode, 'c');
245+
$checkPermissions = match (true) {
246+
$isWac => Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE | ($hasPlus ? Constants::PERMISSION_READ : 0),
247+
$isExclusive => Constants::PERMISSION_CREATE | ($hasPlus ? Constants::PERMISSION_READ : 0),
248+
$isRead => Constants::PERMISSION_READ | ($hasPlus ? Constants::PERMISSION_UPDATE : 0),
249+
default => Constants::PERMISSION_ALL,
250+
};
251+
252+
$this->checkFileAccess($path, false, $checkPermissions);
236253
return $this->storage->fopen($path, $mode);
237254
}
238255

@@ -247,7 +264,7 @@ public function fopen($path, $mode) {
247264
*/
248265
#[\Override]
249266
public function touch($path, $mtime = null): bool {
250-
$this->checkFileAccess($path, false);
267+
$this->checkFileAccess($path, false, Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE);
251268
return $this->storage->touch($path, $mtime);
252269
}
253270

@@ -278,7 +295,7 @@ public function getCache($path = '', $storage = null): ICache {
278295
*/
279296
#[\Override]
280297
public function getDirectDownload($path): array|false {
281-
$this->checkFileAccess($path, false);
298+
$this->checkFileAccess($path, false, Constants::PERMISSION_READ);
282299
return $this->storage->getDirectDownload($path);
283300
}
284301

@@ -302,7 +319,7 @@ public function getDirectDownloadById(string $fileId): array|false {
302319
// We would have actually a result, so lets see if the user should be able to access it
303320
$path = $this->getCache()->getPathById((int)$fileId);
304321
if ($path !== null) {
305-
$this->checkFileAccess($path, false);
322+
$this->checkFileAccess($path, false, Constants::PERMISSION_READ);
306323
}
307324

308325
return $data;
@@ -321,7 +338,7 @@ public function copyFromStorage(IStorage $sourceStorage, $sourceInternalPath, $t
321338
return $this->copy($sourceInternalPath, $targetInternalPath);
322339
}
323340

324-
$this->checkFileAccess($targetInternalPath, $sourceStorage->is_dir($sourceInternalPath));
341+
$this->checkFileAccess($targetInternalPath, $sourceStorage->is_dir($sourceInternalPath), Constants::PERMISSION_CREATE);
325342
return $this->storage->copyFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath);
326343
}
327344

@@ -338,7 +355,7 @@ public function moveFromStorage(IStorage $sourceStorage, $sourceInternalPath, $t
338355
return $this->rename($sourceInternalPath, $targetInternalPath);
339356
}
340357

341-
$this->checkFileAccess($targetInternalPath, $sourceStorage->is_dir($sourceInternalPath));
358+
$this->checkFileAccess($targetInternalPath, $sourceStorage->is_dir($sourceInternalPath), Constants::PERMISSION_CREATE);
342359
return $this->storage->moveFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath);
343360
}
344361

@@ -348,7 +365,7 @@ public function moveFromStorage(IStorage $sourceStorage, $sourceInternalPath, $t
348365
#[\Override]
349366
public function writeStream(string $path, $stream, ?int $size = null): int {
350367
if (!$this->isPartFile($path)) {
351-
$this->checkFileAccess($path, false);
368+
$this->checkFileAccess($path, false, Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE);
352369
}
353370

354371
$result = parent::writeStream($path, $stream, $size);
@@ -359,7 +376,7 @@ public function writeStream(string $path, $stream, ?int $size = null): int {
359376
// Required for object storage since part file is not in the storage so we cannot check it before moving it to the storage
360377
// As an alternative we might be able to check on the cache update/insert/delete though the Cache wrapper
361378
try {
362-
$this->checkFileAccess($path, false);
379+
$this->checkFileAccess($path, false, Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE);
363380
} catch (\Exception $e) {
364381
$this->storage->unlink($path);
365382
throw $e;

0 commit comments

Comments
 (0)