Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
6e34386
feat(sharereview): add listener registering Deck as a share source
AndyScherzinger Jun 11, 2026
5b23af6
feat(sharereview): add ShareReviewSource with constructor and getName()
AndyScherzinger Jun 11, 2026
4aad850
feat(sharereview): implement getShares() with board name lookup via JOIN
AndyScherzinger Jun 11, 2026
4998ccc
feat(sharereview): implement deleteShare() via direct SQL with logging
AndyScherzinger Jun 11, 2026
53834d9
feat(sharereview): register ShareReview listener on SourceEvent
AndyScherzinger Jun 11, 2026
cc3b781
style(sharereview): apply coding standards and Psalm fixes
AndyScherzinger Jun 11, 2026
495eea1
test(sharereview): add unit tests for ShareReviewSource
AndyScherzinger Jun 11, 2026
d12160b
fix(sharereview): harden and optimize implementation and testing
AndyScherzinger Jun 11, 2026
e494495
fix: Use the new share meta-data instead of a hard-coded date
AndyScherzinger Jun 19, 2026
9977163
feat(sharereview): gate ACL deletion behind event-based access check
AndyScherzinger Jun 22, 2026
92b6626
refactor(sharereview): address pre-review feedback on ShareReviewSource
AndyScherzinger Jun 22, 2026
85c7e73
refactor(sharereview): use OCP ShareReviewAccessCheckEvent from server
AndyScherzinger Jun 23, 2026
d61d70a
refactor(sharereview): adopt OCP\Share\ShareReview interface and even…
AndyScherzinger Jul 2, 2026
81b441b
refactor(sharereview): adopt latest ShareReview changes
AndyScherzinger Jul 7, 2026
378741b
refactor(sharereview): adopt reworked ShareReviewEntry fields
AndyScherzinger Jul 12, 2026
1ae86c4
refactor(sharereview): adopt reworked ShareReviewEntry fields
AndyScherzinger Jul 12, 2026
97045c5
refactor(sharereview): emit ShareReviewPermission entries instead of …
AndyScherzinger Jul 13, 2026
604291d
fix(l10n): translate permission labels from core in the target ap itself
AndyScherzinger Jul 13, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions lib/AppInfo/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
use OCA\Deck\Search\CardCommentProvider;
use OCA\Deck\Search\DeckProvider;
use OCA\Deck\Service\PermissionService;
use OCA\Deck\ShareReview\ShareReviewListener;
use OCA\Deck\Sharing\DeckShareProvider;
use OCA\Deck\Sharing\Listener;
use OCA\Deck\Teams\DeckTeamResourceProvider;
Expand Down Expand Up @@ -74,6 +75,7 @@
use OCP\OCM\Events\ResourceTypeRegisterEvent;
use OCP\Server;
use OCP\Share\IManager;
use OCP\Share\ShareReview\RegisterShareReviewSourceEvent;
use OCP\User\Events\UserDeletedEvent;
use OCP\Util;
use Psr\Container\ContainerInterface;
Expand Down Expand Up @@ -189,6 +191,8 @@ public function register(IRegistrationContext $context): void {
$context->registerTeamResourceProvider(DeckTeamResourceProvider::class);

$context->registerUserMigrator(DeckMigrator::class);

$context->registerEventListener(RegisterShareReviewSourceEvent::class, ShareReviewListener::class);
}

public function registerCommentsEntity(IEventDispatcher $eventDispatcher): void {
Expand Down
28 changes: 27 additions & 1 deletion lib/Db/AclMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,16 @@
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\AppFramework\Db\Entity;
use OCP\AppFramework\Db\MultipleObjectsReturnedException;
use OCP\DB\Exception;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\IDBConnection;

/** @template-extends DeckMapper<Acl> */
class AclMapper extends DeckMapper implements IPermissionMapper {
public const TABLE_NAME = 'deck_board_acl';

public function __construct(IDBConnection $db) {
parent::__construct($db, 'deck_board_acl', Acl::class);
parent::__construct($db, self::TABLE_NAME, Acl::class);
}

public function findByAccessToken(string $accessToken) {
Expand Down Expand Up @@ -129,6 +132,29 @@ public function findByType(int $type): array {
return $this->findEntities($qb);
}

/**
* Fetch all ACL rows with their board title and owner for ShareReview.
*
* @return list<array<string, mixed>>
* @throws Exception
*/
public function findAllForShareReview(): array {
$qb = $this->db->getQueryBuilder();
$qb->select(
'a.id', 'a.board_id', 'a.type', 'a.participant',
'a.permission_edit', 'a.permission_share', 'a.permission_manage', 'a.created_at', 'a.last_modified_at'
)
->selectAlias('b.title', 'board_title')
->selectAlias('b.owner', 'board_owner')
->from(self::TABLE_NAME, 'a')
->leftJoin('a', 'deck_boards', 'b', $qb->expr()->eq('a.board_id', 'b.id'))
->orderBy('a.id', 'ASC');
$result = $qb->executeQuery();
$rows = $result->fetchAll();
$result->closeCursor();
return $rows;
}

public function insert(Entity $entity): Entity {
/** @var Acl $entity */
$now = time();
Expand Down
3 changes: 2 additions & 1 deletion lib/Db/BoardMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

/** @template-extends QBMapper<Board> */
class BoardMapper extends QBMapper implements IPermissionMapper {
public const TABLE_NAME = 'deck_boards';
/** @var CappedMemoryCache<Board[]> */
private CappedMemoryCache $userBoardCache;
/** @var CappedMemoryCache<Board> */
Expand All @@ -36,7 +37,7 @@ public function __construct(
private ICloudIdManager $cloudIdManager,
private LoggerInterface $logger,
) {
parent::__construct($db, 'deck_boards', Board::class);
parent::__construct($db, self::TABLE_NAME, Board::class);

$this->userBoardCache = new CappedMemoryCache();
$this->boardCache = new CappedMemoryCache();
Expand Down
16 changes: 16 additions & 0 deletions lib/Service/BoardService.php
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,22 @@ public function deleteAcl(int $id): ?Acl {
return $deletedAcl;
}

/**
* Delete an ACL entry on behalf of a trusted share-review operation.
*
* PERMISSION_MANAGE is intentionally not checked. The caller must verify
* operator access via ShareReviewAccessCheckEvent before invoking this
* method. All other side effects are preserved so the deletion is auditable.
*
* @throws \OCP\AppFramework\Db\DoesNotExistException if $aclId does not exist
*/
public function deleteAclForShareReview(int $aclId): void {
$acl = $this->aclMapper->find($aclId);
$this->aclMapper->delete($acl);
$this->changeHelper->boardChanged($acl->getBoardId());
$this->eventDispatcher->dispatchTyped(new AclDeletedEvent($acl));
}

public function leave(int $boardId): ?Acl {
if ($this->permissionService->userIsBoardOwner($boardId)) {
throw new BadRequestException('Board owner cannot leave board');
Expand Down
27 changes: 27 additions & 0 deletions lib/ShareReview/ShareReviewListener.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\Deck\ShareReview;

use OCP\EventDispatcher\Event;
use OCP\EventDispatcher\IEventListener;
use OCP\Share\ShareReview\RegisterShareReviewSourceEvent;

/** @template-implements IEventListener<RegisterShareReviewSourceEvent> */
class ShareReviewListener implements IEventListener {
public function __construct() {
}

public function handle(Event $event): void {
if (!$event instanceof RegisterShareReviewSourceEvent) {
return;
}
$event->registerSource(ShareReviewSource::class);
}
}
158 changes: 158 additions & 0 deletions lib/ShareReview/ShareReviewSource.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\Deck\ShareReview;

use OCA\Deck\Db\Acl;
use OCA\Deck\Db\AclMapper;
use OCA\Deck\Service\BoardService;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\DB\Exception;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\IL10N;
use OCP\Share\IShare;
use OCP\Share\ShareReview\Events\ShareReviewAccessCheckEvent;
use OCP\Share\ShareReview\IShareReviewSource;
use OCP\Share\ShareReview\ShareReviewEntry;
use OCP\Share\ShareReview\ShareReviewPermission;
use Psr\Log\LoggerInterface;

class ShareReviewSource implements IShareReviewSource {

/** @var array<string, ShareReviewPermission>|null */
private ?array $permissionCatalog = null;

public function __construct(
private readonly AclMapper $aclMapper,
private readonly LoggerInterface $logger,
private readonly BoardService $boardService,
private readonly IEventDispatcher $eventDispatcher,
private readonly IL10N $l,
) {
}

public function getName(): string {
return 'Deck';
}

/**
* @return list<ShareReviewEntry>
*/
public function getShares(): array {
try {
$rawShares = $this->aclMapper->findAllForShareReview();
} catch (Exception $e) {
$this->logger->error('Deck ShareReview: failed to fetch shares: {message}', ['message' => $e->getMessage()]);
return [];
}
return array_map(
fn (array $share) => $this->buildEntry($share),
$rawShares,
);
}

public function deleteShare(string $shareId): bool {
if (!is_numeric($shareId)) {
return false;
}

$event = new ShareReviewAccessCheckEvent('Deck', $shareId);
$this->eventDispatcher->dispatchTyped($event);

if (!$event->isHandled() || !$event->isGranted()) {
return false;
}

try {
$this->boardService->deleteAclForShareReview((int)$shareId);
return true;
} catch (DoesNotExistException) {
return false;
}
}

/** @param array<string, mixed> $share */
private function buildEntry(array $share): ShareReviewEntry {
return new ShareReviewEntry(
id: (string)$share['id'],
object: $this->resolveObjectName($share),
initiator: (string)$share['board_owner'],
type: $this->mapParticipantType((int)$share['type']),
recipient: (string)$share['participant'],
lastModifiedTimestamp: max((int)$share['created_at'], (int)$share['last_modified_at']),
permissions: $this->buildPermissions($share),
);
}

/** @param array<string, mixed> $share */
private function resolveObjectName(array $share): string {
$title = (string)($share['board_title'] ?? '');
$boardId = (int)($share['board_id'] ?? $share['id']);
$label = $title !== '' ? $title : $this->l->t('Board %d', [$boardId]);
return $this->l->t('%s (Board)', [$label]);
}

private function mapParticipantType(int $type): int {
return match($type) {
Acl::PERMISSION_TYPE_USER => IShare::TYPE_USER,
Acl::PERMISSION_TYPE_GROUP => IShare::TYPE_GROUP,
Acl::PERMISSION_TYPE_REMOTE => IShare::TYPE_REMOTE,
Acl::PERMISSION_TYPE_CIRCLE => IShare::TYPE_CIRCLE,
default => $this->fallbackParticipantType($type),
};
}

private function fallbackParticipantType(int $type): int {
$this->logger->warning('Deck ShareReview: unknown ACL participant type {type}, defaulting to user share', ['type' => $type]);
return IShare::TYPE_USER;
}

/**
* @param array<string, mixed> $share
* @return list<ShareReviewPermission>
*/
private function buildPermissions(array $share): array {
$catalog = $this->permissionCatalog();
$permissions = [$catalog[ShareReviewPermission::CORE_READ]];
if ($share['permission_edit']) {
$permissions[] = $catalog[ShareReviewPermission::CORE_UPDATE];
$permissions[] = $catalog[ShareReviewPermission::CORE_CREATE];
$permissions[] = $catalog[ShareReviewPermission::CORE_DELETE];
}
if ($share['permission_share']) {
$permissions[] = $catalog[ShareReviewPermission::CORE_RESHARE];
}
if ($share['permission_manage']) {
$permissions[] = $catalog['deck:manage'];
}
return $permissions;
}

/**
* The permission objects are immutable and identical for every share row,
* so they are built once per request instead of once per row.
*
* Core permission labels are deliberately not translated here: the
* share-review app replaces them with its own central translations, the
* English label is only a fallback for other consumers. Only app-specific
* permissions carry translations from this app's catalog.
*
* @return array<string, ShareReviewPermission>
*/
private function permissionCatalog(): array {
return $this->permissionCatalog ??= [
ShareReviewPermission::CORE_READ => new ShareReviewPermission(ShareReviewPermission::CORE_READ, 'Read', priority: 80),
ShareReviewPermission::CORE_UPDATE => new ShareReviewPermission(ShareReviewPermission::CORE_UPDATE, 'Update', priority: 70),
ShareReviewPermission::CORE_CREATE => new ShareReviewPermission(ShareReviewPermission::CORE_CREATE, 'Create', priority: 60),
ShareReviewPermission::CORE_DELETE => new ShareReviewPermission(ShareReviewPermission::CORE_DELETE, 'Delete', priority: 50),
ShareReviewPermission::CORE_RESHARE => new ShareReviewPermission(ShareReviewPermission::CORE_RESHARE, 'Re-share', priority: 40),
'deck:manage' => new ShareReviewPermission('deck:manage', $this->l->t('Manage board'), $this->l->t('Administer participants and board settings'), 30),
];
}
}
4 changes: 4 additions & 0 deletions tests/bootstrap.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,7 @@

require_once __DIR__ . '/../../../tests/bootstrap.php';
require_once __DIR__ . '/../appinfo/autoload.php';

if (!interface_exists('OCP\Share\ShareReview\IShareReviewSource')) {
require_once __DIR__ . '/unit/ShareReview/Stubs.php';
}
2 changes: 1 addition & 1 deletion tests/phpunit.xml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<?xml version="1.0"?>
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" bootstrap="../../../tests/bootstrap.php" colors="true" convertDeprecationsToExceptions="true" xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd">
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" bootstrap="bootstrap.php" colors="true" convertDeprecationsToExceptions="true" xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd">
<coverage>
<include>
<directory suffix=".php">./../lib</directory>
Expand Down
Loading
Loading