From 5bba6ba0ca4e42d633f25f37b664ce208d6c8988 Mon Sep 17 00:00:00 2001 From: Jaggob <37583151+Jaggob@users.noreply.github.com> Date: Tue, 5 May 2026 18:28:42 +0200 Subject: [PATCH 1/4] fix(card): use accessor for createdAt in VTODO serialization CardDetails extends Card and keeps its own default entity properties. Direct property access can therefore read null from the wrapper while the magic getter delegates through __call() to the inner Card. That can crash any code path serializing a Card-like object with a due date via getCalendarObject(), not only CalDAV PUT handling. Signed-off-by: Jaggob <37583151+Jaggob@users.noreply.github.com> --- lib/Db/Card.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Db/Card.php b/lib/Db/Card.php index c30e77130..29cdca955 100644 --- a/lib/Db/Card.php +++ b/lib/Db/Card.php @@ -141,7 +141,7 @@ public function getCalendarObject(): VCalendar { $event->UID = 'deck-card-' . $this->getId(); if ($this->getDuedate()) { $creationDate = new DateTime(); - $creationDate->setTimestamp($this->createdAt); + $creationDate->setTimestamp($this->getCreatedAt()); $event->DTSTAMP = $creationDate; $event->DUE = new DateTime($this->getDuedate()->format('c'), new DateTimeZone('UTC')); } From 2fc664d97b5484f666eb4571a98d4b38c9c79694 Mon Sep 17 00:00:00 2001 From: Jaggob <37583151+Jaggob@users.noreply.github.com> Date: Tue, 5 May 2026 18:29:03 +0200 Subject: [PATCH 2/4] feat(dav): allow CalDAV updates for existing Deck cards Map SUMMARY, DESCRIPTION, DUE, and STATUS/COMPLETED/PERCENT-COMPLETE to existing Deck cards. Ignore CATEGORIES, ATTENDEE, RRULE, ATTACH, DTSTART, RELATED-TO, and raw VTODO data. This intentionally accepts round-trip loss for unsupported properties in this first step. Reject CREATE with 403, DELETE with 403, and Stack writes with 403 by filtering write-content on CalendarObject ACLs. Return isShared(): false so Nextcloud's Schedule plugin does not treat Deck external calendars as shared scheduling calendars during writable DAV hooks. Map StatusException to 403, DoesNotExistException to 404, and invalid calendar payloads to InvalidDataException for 400 responses. Tested with Thunderbird 148.0.1 and macOS Reminders 26.4.1. Refs #2399: partial write access for existing items only. Signed-off-by: Jaggob <37583151+Jaggob@users.noreply.github.com> --- lib/DAV/Calendar.php | 29 +- lib/DAV/CalendarObject.php | 46 ++- lib/DAV/DeckCalendarBackend.php | 88 ++++- tests/psalm-baseline.xml | 12 - tests/stub.phpstub | 42 +++ tests/unit/DAV/CalendarObjectTest.php | 234 +++++++++++++ tests/unit/DAV/CalendarTest.php | 91 +++++ tests/unit/DAV/DeckCalendarBackendTest.php | 378 +++++++++++++++++++++ 8 files changed, 900 insertions(+), 20 deletions(-) create mode 100644 tests/unit/DAV/CalendarObjectTest.php create mode 100644 tests/unit/DAV/CalendarTest.php create mode 100644 tests/unit/DAV/DeckCalendarBackendTest.php diff --git a/lib/DAV/Calendar.php b/lib/DAV/Calendar.php index 2a8ae7fbc..9d32544c6 100644 --- a/lib/DAV/Calendar.php +++ b/lib/DAV/Calendar.php @@ -29,6 +29,8 @@ class Calendar extends ExternalCalendar { private $backend; /** @var Board */ private $board; + /** @var array|null */ + private $acl; public function __construct(string $principalUri, string $calendarUri, Board $board, DeckCalendarBackend $backend) { parent::__construct('deck', $calendarUri); @@ -43,9 +45,22 @@ public function getOwner() { return $this->principalUri; } + /** + * Nextcloud's DAV Schedule Plugin calls this for writable external + * calendars. Deck calendars are not shared scheduling calendars, and Deck + * does not process iTIP/ATTENDEE scheduling data here. + */ + public function isShared(): bool { + return false; + } + public function getACL() { - // the calendar should always have the read and the write-properties permissions - // write-properties is needed to allow the user to toggle the visibility of shared deck calendars + if ($this->acl !== null) { + return $this->acl; + } + + // write-content covers PUT on existing objects but not bind/unbind, so + // CREATE and DELETE are rejected at the ACL layer before any hooks run. $acl = [ [ 'privilege' => '{DAV:}read', @@ -58,8 +73,16 @@ public function getACL() { 'protected' => true, ] ]; + if ($this->backend->checkBoardPermission($this->board->getId(), Acl::PERMISSION_EDIT)) { + $acl[] = [ + 'privilege' => '{DAV:}write-content', + 'principal' => $this->getOwner(), + 'protected' => true, + ]; + } - return $acl; + $this->acl = $acl; + return $this->acl; } public function setACL(array $acl) { diff --git a/lib/DAV/CalendarObject.php b/lib/DAV/CalendarObject.php index 93dd94aba..80c7dec61 100644 --- a/lib/DAV/CalendarObject.php +++ b/lib/DAV/CalendarObject.php @@ -7,10 +7,15 @@ namespace OCA\Deck\DAV; +use OCA\Deck\BadRequestException; use OCA\Deck\Db\Card; use OCA\Deck\Db\Stack; +use OCA\Deck\StatusException; +use OCP\AppFramework\Db\DoesNotExistException; use Sabre\CalDAV\ICalendarObject; +use Sabre\DAV\Exception\BadRequest; use Sabre\DAV\Exception\Forbidden; +use Sabre\DAV\Exception\NotFound; use Sabre\DAVACL\IACL; use Sabre\VObject\Component\VCalendar; @@ -44,7 +49,14 @@ public function getGroup() { } public function getACL() { - return $this->calendar->getACL(); + $acl = $this->calendar->getACL(); + if ($this->sourceItem instanceof Stack) { + return array_values(array_filter($acl, static function (array $entry): bool { + return $entry['privilege'] !== '{DAV:}write-content'; + })); + } + + return $acl; } public function setACL(array $acl) { @@ -56,7 +68,35 @@ public function getSupportedPrivilegeSet() { } public function put($data) { - throw new Forbidden('This calendar-object is read-only'); + if (!($this->sourceItem instanceof Card)) { + throw new Forbidden('This calendar-object is read-only'); + } + + // MultipleObjectsReturnedException is intentionally not caught: + // a primary-key lookup returning multiple rows is a data integrity bug + // and should surface as a 500 in the log. + try { + $this->sourceItem = $this->backend->updateCardFromCalendarObject($this->sourceItem, $this->readPutData($data)); + } catch (DoesNotExistException $e) { + throw new NotFound($e->getMessage(), 0, $e); + } catch (BadRequestException $e) { + throw new BadRequest($e->getMessage(), 0, $e); + } catch (StatusException $e) { + throw new Forbidden($e->getMessage(), 0, $e); + } + $this->calendarObject = $this->sourceItem->getCalendarObject(); + } + + private function readPutData($data): string { + if (is_resource($data)) { + $content = stream_get_contents($data); + if ($content === false) { + throw new BadRequest('Could not read calendar-object data'); + } + return $content; + } + + return (string)$data; } public function get() { @@ -78,7 +118,7 @@ public function getSize() { } public function delete() { - throw new Forbidden('This calendar-object is read-only'); + throw new Forbidden('Deleting tasks via CalDAV is not supported'); } public function getName() { diff --git a/lib/DAV/DeckCalendarBackend.php b/lib/DAV/DeckCalendarBackend.php index ddb6c024f..934bb5c62 100644 --- a/lib/DAV/DeckCalendarBackend.php +++ b/lib/DAV/DeckCalendarBackend.php @@ -11,11 +11,16 @@ use OCA\Deck\Db\Board; use OCA\Deck\Db\BoardMapper; +use OCA\Deck\Db\Card; +use OCA\Deck\Model\OptionalNullableValue; use OCA\Deck\Service\BoardService; use OCA\Deck\Service\CardService; use OCA\Deck\Service\PermissionService; use OCA\Deck\Service\StackService; use Sabre\DAV\Exception\NotFound; +use Sabre\VObject\Component\VTodo; +use Sabre\VObject\InvalidDataException; +use Sabre\VObject\Reader; class DeckCalendarBackend { @@ -29,6 +34,8 @@ class DeckCalendarBackend { private $permissionService; /** @var BoardMapper */ private $boardMapper; + /** @var array> */ + private $permissionCache = []; public function __construct( BoardService $boardService, StackService $stackService, CardService $cardService, PermissionService $permissionService, @@ -54,8 +61,11 @@ public function getBoard(int $id): Board { } public function checkBoardPermission(int $id, int $permission): bool { - $permissions = $this->permissionService->getPermissions($id); - return isset($permissions[$permission]) ? $permissions[$permission] : false; + if (!isset($this->permissionCache[$id])) { + $this->permissionCache[$id] = $this->permissionService->getPermissions($id); + } + + return $this->permissionCache[$id][$permission] ?? false; } public function updateBoard(Board $board): bool { @@ -69,4 +79,78 @@ public function getChildren(int $id): array { $this->stackService->findCalendarEntries($id) ); } + + public function updateCardFromCalendarObject(Card $sourceCard, string $data): Card { + $todo = $this->extractTodo($data); + $card = $this->cardService->find($sourceCard->getId()); + + $title = trim((string)($todo->SUMMARY ?? '')); + if ($title === '') { + $title = $card->getTitle(); + } + + $description = isset($todo->DESCRIPTION) ? (string)$todo->DESCRIPTION : $card->getDescription(); + $dueDate = isset($todo->DUE) ? $todo->DUE->getDateTime()->format('c') : null; + $startDate = $card->getStartdate() ? $card->getStartdate()->format('c') : null; + + return $this->cardService->update( + id: $card->getId(), + title: $title, + stackId: $card->getStackId(), + type: $card->getType(), + owner: $card->getOwner() ?? '', + description: $description, + order: $card->getOrder(), + duedate: $dueDate, + deletedAt: $card->getDeletedAt(), + archived: $card->getArchived(), + done: $this->mapDoneFromTodo($todo, $card), + startdate: $startDate, + color: $card->getColor() + ); + } + + private function extractTodo(string $data): VTodo { + try { + $vObject = Reader::read($data); + } catch (\Exception $e) { + throw new InvalidDataException('Invalid calendar payload', 0, $e); + } + + $todos = $vObject->select('VTODO'); + if (count($todos) !== 1 || !($todos[0] instanceof VTodo)) { + throw new InvalidDataException('Calendar payload must contain exactly one VTODO'); + } + + return $todos[0]; + } + + private function mapDoneFromTodo(VTodo $todo, Card $card): OptionalNullableValue { + $done = $card->getDone(); + $percentComplete = isset($todo->{'PERCENT-COMPLETE'}) ? (int)((string)$todo->{'PERCENT-COMPLETE'}) : null; + $status = isset($todo->STATUS) ? strtoupper((string)$todo->STATUS) : null; + + // Deck only has a binary done state. IN-PROCESS maps to not done; + // statuses without a Deck equivalent, such as CANCELLED, keep the + // existing done value instead of inventing a new state. + if ($status === 'COMPLETED') { + $done = $this->computeDoneTimestamp($todo); + } elseif ($status === 'NEEDS-ACTION' || $status === 'IN-PROCESS') { + $done = null; + } elseif ($status === null) { + if (isset($todo->COMPLETED) || ($percentComplete !== null && $percentComplete >= 100)) { + $done = $this->computeDoneTimestamp($todo); + } elseif ($percentComplete !== null && $percentComplete === 0) { + $done = null; + } + } + + return new OptionalNullableValue($done); + } + + private function computeDoneTimestamp(VTodo $todo): \DateTime { + return isset($todo->COMPLETED) + ? \DateTime::createFromInterface($todo->COMPLETED->getDateTime()) + : new \DateTime(); + } } diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index 8d7ec75f7..211f21919 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -35,18 +35,6 @@ - - - - - - - - - - - - diff --git a/tests/stub.phpstub b/tests/stub.phpstub index 8ca73d9dd..b5bd7492d 100644 --- a/tests/stub.phpstub +++ b/tests/stub.phpstub @@ -725,3 +725,45 @@ namespace OCA\NotifyPush\Queue { namespace OCA\Text\Event { class LoadEditor extends \OCP\EventDispatcher\Event {} } + +namespace Sabre\VObject { + class InvalidDataException extends \Exception {} + + class Reader { + public static function read(string $data): Component\VCalendar {} + } +} + +namespace Sabre\VObject\Component { + class VCalendar { + /** @return array */ + public function select(string $name): array {} + public function createComponent(string $name): VTodo {} + public function add(mixed $value): void {} + public function serialize(): string {} + public function destroy(): void {} + } + + class VTodo { + public function __get(string $name): mixed {} + public function __set(string $name, mixed $value): void {} + public function __isset(string $name): bool {} + public function add(string $name, mixed $value): void {} + /** @var mixed */ + public $SUMMARY; + /** @var mixed */ + public $DESCRIPTION; + /** @var mixed */ + public $DUE; + /** @var mixed */ + public $COMPLETED; + /** @var mixed */ + public $STATUS; + } +} + +namespace Sabre\VObject\Property { + class DateTime { + public function getDateTime(): \DateTimeInterface {} + } +} diff --git a/tests/unit/DAV/CalendarObjectTest.php b/tests/unit/DAV/CalendarObjectTest.php new file mode 100644 index 000000000..5ea7057d5 --- /dev/null +++ b/tests/unit/DAV/CalendarObjectTest.php @@ -0,0 +1,234 @@ +setId($id); + $card->setTitle('Card'); + $card->setDescription('Description'); + $card->setStackId(10); + $card->setType('plain'); + $card->setOwner('user1'); + $card->setOrder(1); + $card->setCreatedAt(100); + $card->setLastModified(200); + $card->setArchived(false); + $card->setDone(null); + return $card; + } + + private function createStack(): Stack { + $stack = new Stack(); + $stack->setId(10); + $stack->setTitle('Stack'); + $stack->setBoardId(123); + $stack->setLastModified(200); + return $stack; + } + + /** + * @return Calendar&MockObject + */ + private function createCalendarMock(): Calendar { + $calendar = $this->getMockBuilder(Calendar::class) + ->disableOriginalConstructor() + ->onlyMethods(['getACL', 'getOwner', 'getGroup']) + ->getMock(); + $calendar->method('getACL')->willReturn([ + [ + 'privilege' => '{DAV:}read', + 'principal' => 'principals/users/user1', + 'protected' => true, + ], + [ + 'privilege' => '{DAV:}write-content', + 'principal' => 'principals/users/user1', + 'protected' => true, + ], + ]); + $calendar->method('getOwner')->willReturn('principals/users/user1'); + $calendar->method('getGroup')->willReturn([]); + return $calendar; + } + + public function testPutUpdatesCard(): void { + $calendar = $this->createCalendarMock(); + $sourceCard = $this->createCard(); + $updatedCard = $this->createCard(); + $updatedCard->setLastModified(300); + + $backend = $this->createMock(DeckCalendarBackend::class); + $backend->expects($this->once()) + ->method('updateCardFromCalendarObject') + ->with($sourceCard, "BEGIN:VCALENDAR\r\nEND:VCALENDAR") + ->willReturn($updatedCard); + + $object = new CalendarObject($calendar, 'card-1.ics', $backend, $sourceCard); + $object->put("BEGIN:VCALENDAR\r\nEND:VCALENDAR"); + + $this->assertSame(300, $object->getLastModified()); + } + + public function testPutReadsResourcePayload(): void { + $calendar = $this->createCalendarMock(); + $sourceCard = $this->createCard(); + $updatedCard = $this->createCard(); + $updatedCard->setLastModified(300); + $payload = "BEGIN:VCALENDAR\r\nEND:VCALENDAR"; + $stream = fopen('php://temp', 'r+'); + fwrite($stream, $payload); + rewind($stream); + + $backend = $this->createMock(DeckCalendarBackend::class); + $backend->expects($this->once()) + ->method('updateCardFromCalendarObject') + ->with($sourceCard, $payload) + ->willReturn($updatedCard); + + $object = new CalendarObject($calendar, 'card-1.ics', $backend, $sourceCard); + $object->put($stream); + fclose($stream); + + $this->assertSame(300, $object->getLastModified()); + } + + public function testPutRefreshesSerializedObjectAndKeepsEtagStableForNextGet(): void { + $calendar = $this->createCalendarMock(); + $sourceCard = $this->createCard(); + $updatedCard = $this->createCard(); + $updatedCard->setTitle('Updated card'); + $updatedCard->setLastModified(300); + + $backend = $this->createMock(DeckCalendarBackend::class); + $backend->method('updateCardFromCalendarObject')->willReturn($updatedCard); + + $object = new CalendarObject($calendar, 'card-1.ics', $backend, $sourceCard); + $object->put("BEGIN:VCALENDAR\r\nEND:VCALENDAR"); + + $etag = $object->getETag(); + $serialized = $object->get(); + + $this->assertSame($etag, $object->getETag()); + $this->assertStringContainsString('SUMMARY:Updated card', $serialized); + } + + public function testStackPutIsForbidden(): void { + $object = new CalendarObject( + $this->createCalendarMock(), + 'stack-10.ics', + $this->createMock(DeckCalendarBackend::class), + $this->createStack() + ); + + $this->expectException(Forbidden::class); + $object->put("BEGIN:VCALENDAR\r\nEND:VCALENDAR"); + } + + public function testDeleteStaysForbiddenForCards(): void { + $object = new CalendarObject( + $this->createCalendarMock(), + 'card-1.ics', + $this->createMock(DeckCalendarBackend::class), + $this->createCard() + ); + + $this->expectException(Forbidden::class); + $object->delete(); + } + + public function testStackAclDoesNotExposeWriteContent(): void { + $object = new CalendarObject( + $this->createCalendarMock(), + 'stack-10.ics', + $this->createMock(DeckCalendarBackend::class), + $this->createStack() + ); + + $privileges = array_column($object->getACL(), 'privilege'); + $this->assertContains('{DAV:}read', $privileges); + $this->assertNotContains('{DAV:}write-content', $privileges); + } + + public function testPutMapsNoPermissionExceptionToForbidden(): void { + $backend = $this->createMock(DeckCalendarBackend::class); + $backend->method('updateCardFromCalendarObject') + ->willThrowException(new NoPermissionException('No edit permission')); + + $object = new CalendarObject( + $this->createCalendarMock(), + 'card-1.ics', + $backend, + $this->createCard() + ); + + $this->expectException(Forbidden::class); + $object->put("BEGIN:VCALENDAR\r\nEND:VCALENDAR"); + } + + public function testPutMapsStatusExceptionToForbidden(): void { + $backend = $this->createMock(DeckCalendarBackend::class); + $backend->method('updateCardFromCalendarObject') + ->willThrowException(new StatusException('Operation not allowed. This board is archived.')); + + $object = new CalendarObject( + $this->createCalendarMock(), + 'card-1.ics', + $backend, + $this->createCard() + ); + + $this->expectException(Forbidden::class); + $object->put("BEGIN:VCALENDAR\r\nEND:VCALENDAR"); + } + + public function testPutMapsBadRequestExceptionToBadRequest(): void { + $backend = $this->createMock(DeckCalendarBackend::class); + $backend->method('updateCardFromCalendarObject') + ->willThrowException(new BadRequestException('Invalid card data')); + + $object = new CalendarObject( + $this->createCalendarMock(), + 'card-1.ics', + $backend, + $this->createCard() + ); + + $this->expectException(BadRequest::class); + $object->put("BEGIN:VCALENDAR\r\nEND:VCALENDAR"); + } + + public function testPutMapsDoesNotExistExceptionToNotFound(): void { + $backend = $this->createMock(DeckCalendarBackend::class); + $backend->method('updateCardFromCalendarObject') + ->willThrowException(new DoesNotExistException('Card not found')); + + $object = new CalendarObject( + $this->createCalendarMock(), + 'card-1.ics', + $backend, + $this->createCard() + ); + + $this->expectException(NotFound::class); + $object->put("BEGIN:VCALENDAR\r\nEND:VCALENDAR"); + } +} diff --git a/tests/unit/DAV/CalendarTest.php b/tests/unit/DAV/CalendarTest.php new file mode 100644 index 000000000..78f9b314f --- /dev/null +++ b/tests/unit/DAV/CalendarTest.php @@ -0,0 +1,91 @@ +setId(123); + $board->setTitle('Board'); + $board->setColor('ff0000'); + $board->setLastModified(100); + return $board; + } + + public function testCalendarAclExposesWriteContentForEditors(): void { + $backend = $this->createMock(DeckCalendarBackend::class); + $backend->method('checkBoardPermission') + ->willReturnMap([ + [123, Acl::PERMISSION_EDIT, true], + ]); + + $calendar = new Calendar('principals/users/user1', 'board-123', $this->createBoard(), $backend); + $privileges = array_column($calendar->getACL(), 'privilege'); + + $this->assertContains('{DAV:}read', $privileges); + $this->assertContains('{DAV:}write-properties', $privileges); + $this->assertContains('{DAV:}write-content', $privileges); + $this->assertNotContains('{DAV:}write', $privileges); + $this->assertNotContains('{DAV:}bind', $privileges); + $this->assertNotContains('{DAV:}unbind', $privileges); + } + + public function testCalendarAclCachesPermissionCheck(): void { + $backend = $this->createMock(DeckCalendarBackend::class); + $backend->expects($this->once()) + ->method('checkBoardPermission') + ->with(123, Acl::PERMISSION_EDIT) + ->willReturn(true); + + $calendar = new Calendar('principals/users/user1', 'board-123', $this->createBoard(), $backend); + + $this->assertSame($calendar->getACL(), $calendar->getACL()); + } + + public function testCalendarIsNotSharedForDavSchedulePlugin(): void { + $calendar = new Calendar( + 'principals/users/user1', + 'board-123', + $this->createBoard(), + $this->createMock(DeckCalendarBackend::class) + ); + + $this->assertFalse($calendar->isShared()); + } + + public function testCalendarAclDoesNotExposeWriteContentForReadOnlyUsers(): void { + $backend = $this->createMock(DeckCalendarBackend::class); + $backend->method('checkBoardPermission') + ->willReturnMap([ + [123, Acl::PERMISSION_EDIT, false], + ]); + + $calendar = new Calendar('principals/users/user1', 'board-123', $this->createBoard(), $backend); + $privileges = array_column($calendar->getACL(), 'privilege'); + + $this->assertContains('{DAV:}read', $privileges); + $this->assertContains('{DAV:}write-properties', $privileges); + $this->assertNotContains('{DAV:}write-content', $privileges); + } + + public function testCreateFileStaysForbidden(): void { + $calendar = new Calendar( + 'principals/users/user1', + 'board-123', + $this->createBoard(), + $this->createMock(DeckCalendarBackend::class) + ); + + $this->expectException(Forbidden::class); + $calendar->createFile('client-generated.ics', "BEGIN:VCALENDAR\r\nEND:VCALENDAR"); + } +} diff --git a/tests/unit/DAV/DeckCalendarBackendTest.php b/tests/unit/DAV/DeckCalendarBackendTest.php new file mode 100644 index 000000000..57b084e44 --- /dev/null +++ b/tests/unit/DAV/DeckCalendarBackendTest.php @@ -0,0 +1,378 @@ +cardService = $this->createMock(CardService::class); + $this->permissionService = $this->createMock(PermissionService::class); + $this->backend = new DeckCalendarBackend( + $this->createMock(BoardService::class), + $this->createMock(StackService::class), + $this->cardService, + $this->permissionService, + $this->createMock(BoardMapper::class) + ); + } + + private function createCard(?\DateTimeInterface $done = null): Card { + $card = new Card(); + $card->setId(1); + $card->setTitle('Old title'); + $card->setDescription('Old description'); + $card->setStackId(10); + $card->setType('plain'); + $card->setOwner('user1'); + $card->setOrder(3); + $card->setDeletedAt(0); + $card->setArchived(false); + $card->setDone($done ? \DateTime::createFromInterface($done) : null); + $card->setStartdate(new \DateTime('2026-01-01T09:00:00+00:00')); + $card->setColor('ff0000'); + return $card; + } + + private function todoPayload(string $todo): string { + return "BEGIN:VCALENDAR\r\nVERSION:2.0\r\nPRODID:-//Nextcloud Deck Test//EN\r\n" . $todo . "\r\nEND:VCALENDAR\r\n"; + } + + public function testBoardPermissionsAreCachedPerBoard(): void { + $this->permissionService->expects($this->once()) + ->method('getPermissions') + ->with(123) + ->willReturn([ + 1 => true, + 2 => false, + ]); + + $this->assertTrue($this->backend->checkBoardPermission(123, 1)); + $this->assertFalse($this->backend->checkBoardPermission(123, 2)); + $this->assertFalse($this->backend->checkBoardPermission(123, 3)); + } + + public function testUpdateCardMapsSupportedFields(): void { + $sourceCard = $this->createCard(); + $currentCard = $this->createCard(); + $updatedCard = $this->createCard(); + $payload = $this->todoPayload( + "BEGIN:VTODO\r\n" + . "UID:deck-card-1\r\n" + . "SUMMARY:New title\r\n" + . "DESCRIPTION:New description\r\n" + . "DUE:20260507T100000Z\r\n" + . "STATUS:COMPLETED\r\n" + . 'END:VTODO' + ); + + $this->cardService->expects($this->once()) + ->method('find') + ->with(1) + ->willReturn($currentCard); + $this->cardService->expects($this->once()) + ->method('update') + ->with( + 1, + 'New title', + 10, + 'plain', + 'user1', + 'New description', + 3, + $this->callback(static fn (?string $value): bool => $value !== null && (new \DateTime($value))->getTimestamp() === 1778148000), + 0, + false, + $this->callback(static fn (OptionalNullableValue $value): bool => $value->getValue() instanceof \DateTimeInterface), + $this->callback(static fn (?string $value): bool => $value !== null && (new \DateTime($value))->getTimestamp() === 1767258000), + 'ff0000' + ) + ->willReturn($updatedCard); + + $this->assertSame($updatedCard, $this->backend->updateCardFromCalendarObject($sourceCard, $payload)); + } + + public function testPercentCompleteMiddleValueKeepsDoneState(): void { + $done = new \DateTime('2026-01-02T09:00:00+00:00'); + $sourceCard = $this->createCard($done); + $currentCard = $this->createCard($done); + $payload = $this->todoPayload( + "BEGIN:VTODO\r\n" + . "UID:deck-card-1\r\n" + . "SUMMARY:Old title\r\n" + . "PERCENT-COMPLETE:50\r\n" + . 'END:VTODO' + ); + + $this->cardService->method('find')->willReturn($currentCard); + $this->cardService->expects($this->once()) + ->method('update') + ->with( + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->callback(static fn (OptionalNullableValue $value): bool => $value->getValue() instanceof \DateTimeInterface + && $value->getValue()->getTimestamp() === $done->getTimestamp()), + $this->anything(), + $this->anything() + ) + ->willReturn($currentCard); + + $this->backend->updateCardFromCalendarObject($sourceCard, $payload); + } + + public function testNeedsActionStatusWinsOverStaleCompletedProperty(): void { + $done = new \DateTime('2026-01-02T09:00:00+00:00'); + $sourceCard = $this->createCard($done); + $currentCard = $this->createCard($done); + $payload = $this->todoPayload( + "BEGIN:VTODO\r\n" + . "UID:deck-card-1\r\n" + . "SUMMARY:Old title\r\n" + . "STATUS:NEEDS-ACTION\r\n" + . "COMPLETED:20260102T090000Z\r\n" + . 'END:VTODO' + ); + + $this->cardService->method('find')->willReturn($currentCard); + $this->cardService->expects($this->once()) + ->method('update') + ->with( + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->callback(static fn (OptionalNullableValue $value): bool => $value->getValue() === null), + $this->anything(), + $this->anything() + ) + ->willReturn($currentCard); + + $this->backend->updateCardFromCalendarObject($sourceCard, $payload); + } + + public function testCancelledStatusKeepsDoneState(): void { + $done = new \DateTime('2026-01-02T09:00:00+00:00'); + $sourceCard = $this->createCard($done); + $currentCard = $this->createCard($done); + $payload = $this->todoPayload( + "BEGIN:VTODO\r\n" + . "UID:deck-card-1\r\n" + . "SUMMARY:Old title\r\n" + . "STATUS:CANCELLED\r\n" + . "PERCENT-COMPLETE:0\r\n" + . 'END:VTODO' + ); + + $this->cardService->method('find')->willReturn($currentCard); + $this->cardService->expects($this->once()) + ->method('update') + ->with( + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->callback(static fn (OptionalNullableValue $value): bool => $value->getValue() instanceof \DateTimeInterface + && $value->getValue()->getTimestamp() === $done->getTimestamp()), + $this->anything(), + $this->anything() + ) + ->willReturn($currentCard); + + $this->backend->updateCardFromCalendarObject($sourceCard, $payload); + } + + public function testEmptyDescriptionClearsDescription(): void { + $sourceCard = $this->createCard(); + $currentCard = $this->createCard(); + $payload = $this->todoPayload( + "BEGIN:VTODO\r\n" + . "UID:deck-card-1\r\n" + . "SUMMARY:Old title\r\n" + . "DESCRIPTION:\r\n" + . 'END:VTODO' + ); + + $this->cardService->method('find')->willReturn($currentCard); + $this->cardService->expects($this->once()) + ->method('update') + ->with( + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + '', + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything() + ) + ->willReturn($currentCard); + + $this->backend->updateCardFromCalendarObject($sourceCard, $payload); + } + + public function testMissingDescriptionKeepsCurrentDescription(): void { + $sourceCard = $this->createCard(); + $currentCard = $this->createCard(); + $payload = $this->todoPayload( + "BEGIN:VTODO\r\n" + . "UID:deck-card-1\r\n" + . "SUMMARY:Old title\r\n" + . 'END:VTODO' + ); + + $this->cardService->method('find')->willReturn($currentCard); + $this->cardService->expects($this->once()) + ->method('update') + ->with( + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + 'Old description', + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything() + ) + ->willReturn($currentCard); + + $this->backend->updateCardFromCalendarObject($sourceCard, $payload); + } + + public function testEmptySummaryFallsBackToCurrentTitle(): void { + $sourceCard = $this->createCard(); + $currentCard = $this->createCard(); + $payload = $this->todoPayload( + "BEGIN:VTODO\r\n" + . "UID:deck-card-1\r\n" + . "SUMMARY:\r\n" + . "STATUS:NEEDS-ACTION\r\n" + . 'END:VTODO' + ); + + $this->cardService->method('find')->willReturn($currentCard); + $this->cardService->expects($this->once()) + ->method('update') + ->with( + $this->anything(), + 'Old title', + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->callback(static fn (OptionalNullableValue $value): bool => $value->getValue() === null), + $this->anything(), + $this->anything() + ) + ->willReturn($currentCard); + + $this->backend->updateCardFromCalendarObject($sourceCard, $payload); + } + + public function testPayloadMustContainExactlyOneTodo(): void { + $this->expectException(InvalidDataException::class); + $this->backend->updateCardFromCalendarObject($this->createCard(), "BEGIN:VCALENDAR\r\nEND:VCALENDAR\r\n"); + } + + public function testInvalidCalendarPayloadThrowsInvalidDataException(): void { + $this->expectException(InvalidDataException::class); + $this->backend->updateCardFromCalendarObject($this->createCard(), 'not an ics payload'); + } + + public function testDtStartFromPayloadIsIgnored(): void { + $sourceCard = $this->createCard(); + $currentCard = new Card(); + $currentCard->setId(1); + $currentCard->setTitle('Old title'); + $currentCard->setDescription('Old description'); + $currentCard->setStackId(10); + $currentCard->setType('plain'); + $currentCard->setOwner('user1'); + $currentCard->setOrder(3); + $currentCard->setDeletedAt(0); + $currentCard->setArchived(false); + $currentCard->setDone(null); + $currentCard->setStartdate(null); + $currentCard->setColor(null); + + $payload = $this->todoPayload( + "BEGIN:VTODO\r\n" + . "UID:deck-card-1\r\n" + . "SUMMARY:Title\r\n" + . "DTSTART:20260506T100000Z\r\n" + . 'END:VTODO' + ); + + $this->cardService->method('find')->willReturn($currentCard); + $this->cardService->expects($this->once()) + ->method('update') + ->with( + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->anything(), + $this->isNull(), + $this->anything() + ) + ->willReturn($currentCard); + + $this->backend->updateCardFromCalendarObject($sourceCard, $payload); + } +} From 771fe11a97e2ab46e30e43f98f290977341a9440 Mon Sep 17 00:00:00 2001 From: Jaggob <37583151+Jaggob@users.noreply.github.com> Date: Tue, 5 May 2026 21:17:28 +0200 Subject: [PATCH 3/4] test(integration): update query count baseline Account for the expected integration query count increase from permission-aware CalDAV ACLs. Calendar and backend permission checks are cached, but exposing write-content only to board editors still requires real permission lookups during integration requests. Signed-off-by: Jaggob <37583151+Jaggob@users.noreply.github.com> --- tests/integration/base-query-count.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/base-query-count.txt b/tests/integration/base-query-count.txt index ce8c60422..c488fdacf 100644 --- a/tests/integration/base-query-count.txt +++ b/tests/integration/base-query-count.txt @@ -1 +1 @@ -93102 +96882 From 25ec200fc9c36b7072d268ee394c72ee4d1e9e06 Mon Sep 17 00:00:00 2001 From: Jaggob <37583151+Jaggob@users.noreply.github.com> Date: Thu, 11 Jun 2026 21:42:20 +0200 Subject: [PATCH 4/4] fix(dav): map invalid CalDAV payloads to 400 instead of 500 extractTodo() throws Sabre\VObject\InvalidDataException for malformed ICS or payloads that do not contain exactly one VTODO. That exception does not extend Sabre\DAV\Exception, so the DAV server mapped it to 500 instead of the 400 the PR description promises. Catch it in CalendarObject::put() and rethrow as BadRequest, and cover the put() mapping with a unit test. Signed-off-by: Jaggob <37583151+Jaggob@users.noreply.github.com> --- lib/DAV/CalendarObject.php | 3 +++ tests/unit/DAV/CalendarObjectTest.php | 17 +++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/lib/DAV/CalendarObject.php b/lib/DAV/CalendarObject.php index 80c7dec61..4f3c97a43 100644 --- a/lib/DAV/CalendarObject.php +++ b/lib/DAV/CalendarObject.php @@ -18,6 +18,7 @@ use Sabre\DAV\Exception\NotFound; use Sabre\DAVACL\IACL; use Sabre\VObject\Component\VCalendar; +use Sabre\VObject\InvalidDataException; class CalendarObject implements ICalendarObject, IACL { @@ -79,6 +80,8 @@ public function put($data) { $this->sourceItem = $this->backend->updateCardFromCalendarObject($this->sourceItem, $this->readPutData($data)); } catch (DoesNotExistException $e) { throw new NotFound($e->getMessage(), 0, $e); + } catch (InvalidDataException $e) { + throw new BadRequest($e->getMessage(), 0, $e); } catch (BadRequestException $e) { throw new BadRequest($e->getMessage(), 0, $e); } catch (StatusException $e) { diff --git a/tests/unit/DAV/CalendarObjectTest.php b/tests/unit/DAV/CalendarObjectTest.php index 5ea7057d5..d28f257c2 100644 --- a/tests/unit/DAV/CalendarObjectTest.php +++ b/tests/unit/DAV/CalendarObjectTest.php @@ -17,6 +17,7 @@ use Sabre\DAV\Exception\BadRequest; use Sabre\DAV\Exception\Forbidden; use Sabre\DAV\Exception\NotFound; +use Sabre\VObject\InvalidDataException; use Test\TestCase; class CalendarObjectTest extends TestCase { @@ -216,6 +217,22 @@ public function testPutMapsBadRequestExceptionToBadRequest(): void { $object->put("BEGIN:VCALENDAR\r\nEND:VCALENDAR"); } + public function testPutMapsInvalidDataExceptionToBadRequest(): void { + $backend = $this->createMock(DeckCalendarBackend::class); + $backend->method('updateCardFromCalendarObject') + ->willThrowException(new InvalidDataException('Calendar payload must contain exactly one VTODO')); + + $object = new CalendarObject( + $this->createCalendarMock(), + 'card-1.ics', + $backend, + $this->createCard() + ); + + $this->expectException(BadRequest::class); + $object->put("BEGIN:VCALENDAR\r\nEND:VCALENDAR"); + } + public function testPutMapsDoesNotExistExceptionToNotFound(): void { $backend = $this->createMock(DeckCalendarBackend::class); $backend->method('updateCardFromCalendarObject')