refactor: untangle Node|ICacheEntry parameter in shouldWatermark

Results in split methods as we do not have nice overloading in PHP. But
simplifies parameters when calling them, while implementation is being
hidden yet simplified as it does not have to deal with different objects
but a few primitive types.

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
This commit is contained in:
Arthur Schiwon 2025-10-07 20:01:10 +02:00
parent 07cad902f8
commit 3e90cc881c
No known key found for this signature in database
GPG key ID: 7424F1874854DF23
6 changed files with 83 additions and 19 deletions

View file

@ -136,7 +136,7 @@ class WopiController extends Controller {
$isTaskProcessingEnabled = $isSmartPickerEnabled && $this->taskProcessingManager->isTaskProcessingEnabled();
$share = $this->getShareForWopiToken($wopi, $file);
$shouldUseSecureView = $this->permissionManager->shouldWatermark($file, $wopi->getEditorUid(), $share);
$shouldUseSecureView = $this->permissionManager->shouldWatermarkByNode($file, $wopi->getEditorUid(), $share);
// If the file is locked manually by a user we want to open it read only for all others
$canWriteThroughLock = true;

View file

@ -49,7 +49,7 @@ class BeforeFetchPreviewListener implements IEventListener {
}
$userId = $this->userSession->getUser() ? $this->userSession->getUser()->getUID() : null;
if ($this->permissionManager->shouldWatermark($event->getNode(), $userId, $share)) {
if ($this->permissionManager->shouldWatermarkByNode($event->getNode(), $userId, $share)) {
throw new NotFoundException();
}
}

View file

@ -33,7 +33,7 @@ class OverwritePublicSharePropertiesListener implements IEventListener {
return;
}
if ($this->permissionManager->shouldWatermark($node, $this->userId, $share)) {
if ($this->permissionManager->shouldWatermarkByNode($node, $this->userId, $share)) {
$share->setHideDownload(true);
}
}

View file

@ -113,21 +113,54 @@ class PermissionManager {
return false;
}
public function shouldWatermark(Node|ICacheEntry $nodeOrCacheEntry, ?string $userId = null, ?IShare $share = null, ?string $ownerId = null): bool {
public function shouldWatermarkByNode(
Node $node,
?string $userId = null,
?IShare $share = null
): bool {
return $this->shouldWatermark(
$node->getId(),
$node->getMimetype(),
$node->isUpdateable(),
$userId,
$share,
$node->getOwner()?->getUID(),
);
}
public function shouldWatermarkByCacheEntry(
ICacheEntry $cacheEntry,
?string $userId = null,
?IShare $share = null,
?string $ownerId = null
): bool {
return $this->shouldWatermark(
$cacheEntry->getId(),
$cacheEntry->getMimetype(),
(bool)($cacheEntry->getPermissions() & Constants::PERMISSION_UPDATE),
$userId,
$share,
$ownerId,
);
}
protected function shouldWatermark(
int $fileId,
string $mimetype,
bool $isFileUpdatable,
?string $userId,
?IShare $share,
?string $ownerId,
): bool {
if ($this->config->getAppValue(AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_enabled', 'no') === 'no') {
return false;
}
if (!in_array($nodeOrCacheEntry->getMimetype(), $this->appConfig->getMimeTypes(), true)) {
if (!in_array($mimetype, $this->appConfig->getMimeTypes(), true)) {
return false;
}
$fileId = $nodeOrCacheEntry->getId();
$isUpdatable = $nodeOrCacheEntry instanceof Node
? $nodeOrCacheEntry->isUpdateable()
: $nodeOrCacheEntry->getPermissions() & Constants::PERMISSION_UPDATE;
$isUpdatable = $isUpdatable && (!$share || $share->getPermissions() & Constants::PERMISSION_UPDATE);
$isUpdatable = $isFileUpdatable && (!$share || $share->getPermissions() & Constants::PERMISSION_UPDATE);
$hasShareAttributes = $share && method_exists($share, 'getAttributes') && $share->getAttributes() instanceof IAttributes;
$isDisabledDownload = $hasShareAttributes && $share->getAttributes()->getAttribute('permissions', 'download') === false;
@ -157,9 +190,6 @@ class PermissionManager {
}
if ($this->config->getAppValue(AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_shareAll', 'no') === 'yes') {
if (!$ownerId && $nodeOrCacheEntry instanceof Node) {
$ownerId = $nodeOrCacheEntry->getOwner()?->getUID();
}
if ($userId === null || $ownerId !== $userId) {
return true;
}

View file

@ -107,7 +107,7 @@ class SecureViewWrapper extends Wrapper {
$share = $isSharedStorage ? $storage->getShare() : null;
$userId = $this->userSession->getUser()?->getUID();
return $this->permissionManager->shouldWatermark($cacheEntry, $userId, $share, $storage->getOwner($path) ?: null);
return $this->permissionManager->shouldWatermarkByCacheEntry($cacheEntry, $userId, $share, $storage->getOwner($path) ?: null);
}

View file

@ -8,6 +8,8 @@ namespace Tests\Richdocuments;
use OCA\Richdocuments\AppConfig;
use OCA\Richdocuments\Capabilities;
use OCA\Richdocuments\PermissionManager;
use OCP\Constants;
use OCP\Files\Cache\ICacheEntry;
use OCP\Files\Node;
use OCP\IConfig;
use OCP\IGroupManager;
@ -162,7 +164,7 @@ class PermissionManagerTest extends TestCase {
$this->systemTagMapper->expects($this->once())->method('getTagIdsForObjects')->willReturn(['testFileId' => $objectTagIds]);
$this->appConfig->expects($this->once())->method('getAppValueArray')->willReturn($watermarkTagIds);
$result = $this->permissionManager->shouldWatermark($node, $userId, $share);
$result = $this->permissionManager->shouldWatermarkByNode($node, $userId, $share);
$this->assertTrue($result);
}
@ -190,7 +192,7 @@ class PermissionManagerTest extends TestCase {
$this->systemTagMapper->expects($this->once())->method('getTagIdsForObjects')->willReturn(['testFileId' => $objectTagIds]);
$this->appConfig->expects($this->once())->method('getAppValueArray')->willReturn($watermarkTagIds);
$result = $this->permissionManager->shouldWatermark($node, $userId, null);
$result = $this->permissionManager->shouldWatermarkByNode($node, $userId, null);
$this->assertTrue($result);
}
@ -214,7 +216,31 @@ class PermissionManagerTest extends TestCase {
[AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_linkTags', 'no', 'no'],
[AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_shareTalkPublic', 'no', 'yes'],
]);
$result = $this->permissionManager->shouldWatermark($node, $userId, $share);
$result = $this->permissionManager->shouldWatermarkByNode($node, $userId, $share);
$this->assertTrue($result);
}
public function testShouldWatermarkCacheEntry(): void {
$cacheEntry = $this->createCacheEntryMock();
$share = $this->createShareMock(IShare::TYPE_ROOM);
$userId = null;
$this->appConfig->expects($this->any())
->method('getMimeTypes')
->willReturn(Capabilities::MIMETYPES);
$this->config
->method('getAppValue')
->willReturnMap([
[AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_enabled', 'no', 'yes'],
[AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_linkAll', 'no', 'no'],
[AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_linkRead', 'no', 'no'],
[AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_linkSecure', 'no', 'no'],
[AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_linkTags', 'no', 'no'],
[AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_shareTalkPublic', 'no', 'yes'],
]);
$result = $this->permissionManager->shouldWatermarkByCacheEntry($cacheEntry, $userId, $share);
$this->assertTrue($result);
}
@ -222,11 +248,19 @@ class PermissionManagerTest extends TestCase {
private function createNodeMock(): Node {
$node = $this->createMock(Node::class);
$node->expects($this->any())->method('getMimetype')->willReturn('application/vnd.oasis.opendocument.spreadsheet');
$node->expects($this->any())->method('getId')->willReturn('testFileId');
$node->expects($this->any())->method('getId')->willReturn(42);
$node->expects($this->any())->method('isUpdateable')->willReturn(false);
return $node;
}
private function createCacheEntryMock(): ICacheEntry {
$node = $this->createMock(ICacheEntry::class);
$node->expects($this->any())->method('getMimetype')->willReturn('application/vnd.oasis.opendocument.spreadsheet');
$node->expects($this->any())->method('getId')->willReturn(42);
$node->expects($this->any())->method('getPermissions')->willReturn(Constants::PERMISSION_READ);
return $node;
}
private function createShareMock(?int $shareType): ?IShare {
if ($shareType === null) {
return null;