Skip to content

Commit 29e7a71

Browse files
authored
fix(sharing): an access link no longer tells a stranger who wrote each note (#3859)
* feat(shares): a resolved case token carries the object's public timeline A citizen following a track your case link came to find out what has happened, and a status with no history answers half the question. The resolve payload now carries the object's public entries. The filter runs here, on the server, through the same service the signed-in timeline reads: nothing marked internal crosses the boundary and no caller can ask for a different filter. The projection is a whitelist of five keys, so the author, the entry's own visibility and the raw source of an intake mail stay behind, along with whatever is added to the entry next. A timeline that cannot be read answers an empty list. The status page shipped before the timeline did, and an instance whose tables are not migrated yet must show the status rather than a 404 a citizen reads as a revoked link. * style(shares): the resolve docblock groups its param tags again The new paragraph landed between the @PARAM and the @return, which phpcs reads as a second parameter comment. It belongs above the tag block. * fix(sharing): an access link no longer tells a stranger who wrote each note The live anonymous surface is the access-link reader, and it handed every public note to the link holder exactly as the note service shapes it: with the author's Nextcloud user id and display name. Anyone with a link learned who at the organisation wrote every line. It also read notes only, so a kinded entry that exists as a record (a delivered decision, an announced status, a portal message) never appeared on the page at all. PublicTimeline is now the one anonymous timeline reader. It reads public records and public notes, lets a record win over the note it projects so an older note with no record is not dropped from view, sorts newest first by parsed moment rather than by string, and lets five keys out. Each source fails on its own. The access-link reader and the case-token resolve both ask it, so the two surfaces cannot come to disagree about what leaves. Mutation-checked: passing a note's fields through reddens the whitelist assertion, and skipping the notes reddens the older-note guard.
1 parent e366947 commit 29e7a71

6 files changed

Lines changed: 783 additions & 34 deletions

File tree

lib/Service/CaseTokenService.php

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@
4545
use InvalidArgumentException;
4646
use OCA\OpenRegister\Db\CaseToken;
4747
use OCA\OpenRegister\Db\CaseTokenMapper;
48+
use OCA\OpenRegister\Db\ObjectEntity;
49+
use OCA\OpenRegister\Service\Timeline\PublicTimeline;
4850
use OCP\IURLGenerator;
4951
use OCP\IUserSession;
5052
use OCP\Security\ISecureRandom;
@@ -70,6 +72,18 @@ class CaseTokenService {
7072
*/
7173
private const TOKEN_LENGTH = 43;
7274

75+
/**
76+
* How many public entries a resolved token carries at most.
77+
*
78+
* A public page is read on a phone and the newest entries are the ones
79+
* that answer "what is happening with my case". A case with a longer
80+
* history is not an error, it is a case that needs paging, and paging an
81+
* anonymous endpoint is a separate decision.
82+
*
83+
* @var int
84+
*/
85+
private const PUBLIC_TIMELINE_LIMIT = 50;
86+
7387
/**
7488
* Constructor.
7589
*
@@ -168,6 +182,13 @@ public function mint(
168182
* object missing, RBAC-denied) so the caller returns a uniform 404
169183
* and the endpoint is not an enumeration oracle.
170184
*
185+
* THE VIEW CARRIES THE OBJECT'S PUBLIC TIMELINE. A citizen following a
186+
* "track your case" link came to find out what has happened, and a status
187+
* with no history answers half the question. The entries are filtered on
188+
* `public` HERE, on the server, by the same service the signed-in timeline
189+
* reads: nothing that says `internal` crosses this boundary, and no caller
190+
* can ask this method for a different filter.
191+
*
171192
* @param string $token The opaque token.
172193
*
173194
* @return array<string,mixed>|null The public-safe object view, or
@@ -229,6 +250,7 @@ public function resolve(string $token): ?array {
229250
'token' => $row->getToken(),
230251
'label' => $row->getLabel(),
231252
'object' => $rendered,
253+
'timeline' => $this->publicTimeline(entity: $entity),
232254
];
233255
} catch (Throwable $e) {
234256
// RBAC-denied / not-found / any read failure → 404 (null).
@@ -241,6 +263,44 @@ public function resolve(string $token): ?array {
241263
}//end try
242264
}//end resolve()
243265

266+
/**
267+
* The public entries on one object, as a stranger may read them.
268+
*
269+
* The read and the five-key projection are {@see PublicTimeline}'s, the
270+
* same class the access-link reader asks. Two anonymous surfaces that each
271+
* decided for themselves what leaves would come to disagree, and the
272+
* disagreement would be a handler's name on a citizen's screen.
273+
*
274+
* SOFT BY DESIGN. A timeline that cannot be read answers the empty list:
275+
* the status page shipped before the timeline did, and an instance that
276+
* cannot build the reader must still show the status rather than a uniform
277+
* 404 a citizen reads as a revoked link. `PublicTimeline` already softens a
278+
* failed read per source; this catch covers the container failing to build
279+
* it at all, and anything that is not an entity has no timeline to read.
280+
*
281+
* @param object $entity The object the timeline hangs on.
282+
*
283+
* @return array<int, array<string,mixed>> The public entries, newest first.
284+
*
285+
* @spec openspec/specs/integration-leaf-foundation/spec.md
286+
*/
287+
private function publicTimeline(object $entity): array {
288+
if (($entity instanceof ObjectEntity) === false) {
289+
return [];
290+
}
291+
292+
try {
293+
$reader = $this->container->get(PublicTimeline::class);
294+
} catch (Throwable $e) {
295+
$this->logger->warning(
296+
'[CaseTokenService] the public timeline reader could not be built: ' . $e->getMessage()
297+
);
298+
return [];
299+
}
300+
301+
return $reader->forObject(object: $entity, limit: self::PUBLIC_TIMELINE_LIMIT);
302+
}//end publicTimeline()
303+
244304
/**
245305
* Revoke a token so it can no longer be resolved.
246306
*

lib/Service/Sharing/AccessLinkReader.php

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,9 @@
4747
use OCA\OpenRegister\Db\AccessLink;
4848
use OCA\OpenRegister\Db\ObjectEntity;
4949
use OCA\OpenRegister\Db\SchemaMapper;
50-
use OCA\OpenRegister\Service\NoteService;
5150
use OCA\OpenRegister\Service\ObjectService;
5251
use OCA\OpenRegister\Service\PropertyRbacHandler;
52+
use OCA\OpenRegister\Service\Timeline\PublicTimeline;
5353
use OCA\OpenRegister\Service\TimelineVisibilityService;
5454
use Psr\Log\LoggerInterface;
5555
use Throwable;
@@ -104,15 +104,15 @@ class AccessLinkReader {
104104
* @param ObjectService $objects The object read path.
105105
* @param SchemaMapper $schemas Resolves the schema whose rules apply.
106106
* @param PropertyRbacHandler $properties Strips write-only and unreadable properties.
107-
* @param NoteService $notes Reads the timeline.
107+
* @param PublicTimeline $timeline Reads and projects the public half of the timeline.
108108
* @param AccessLinkSubject $subjects Reads which object a subject names.
109109
* @param LoggerInterface $logger PSR logger.
110110
*/
111111
public function __construct(
112112
private readonly ObjectService $objects,
113113
private readonly SchemaMapper $schemas,
114114
private readonly PropertyRbacHandler $properties,
115-
private readonly NoteService $notes,
115+
private readonly PublicTimeline $timeline,
116116
private readonly AccessLinkSubject $subjects,
117117
private readonly LoggerInterface $logger,
118118
) {
@@ -344,24 +344,21 @@ private function filteredProperties(ObjectEntity $object): array {
344344
* who may manage the object, and a link must publish the public half
345345
* whatever session happens to be around it.
346346
*
347+
* PROJECTED, NOT PASSED THROUGH. This used to hand the link holder each
348+
* public note exactly as the note service shapes it, which carries the
349+
* author's user id and display name: a stranger with a link learned who
350+
* at the organisation wrote every line. It also read notes only, so a
351+
* kinded entry that exists as a record and not as a comment never
352+
* appeared. {@see PublicTimeline} reads both and lets five keys out.
353+
*
347354
* @param ObjectEntity $object The object.
348355
*
349-
* @return array<int, mixed> The published timeline entries.
356+
* @return array<int, array<string, mixed>> The published timeline entries.
350357
*
351358
* @spec openspec/changes/access-by-link-not-by-account/specs/public-access-links/spec.md#requirement-a-link-never-sees-past-the-objects-own-rules-req-abl-004
352359
*/
353360
private function publicTimeline(ObjectEntity $object): array {
354-
try {
355-
return $this->notes->getNotesForObject(
356-
objectUuid: (string)$object->getUuid(),
357-
visibility: TimelineVisibilityService::PUBLIC_ENTRY
358-
);
359-
} catch (Throwable $failure) {
360-
$this->logger->warning(
361-
'[AccessLinkReader] Could not read the timeline for a link: ' . $failure->getMessage()
362-
);
363-
return [];
364-
}
361+
return $this->timeline->forObject(object: $object);
365362
}//end publicTimeline()
366363

367364
/**
Lines changed: 239 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,239 @@
1+
<?php
2+
3+
/**
4+
* The public half of an object's timeline, as a stranger may read it.
5+
*
6+
* ONE CLASS FOR EVERY ANONYMOUS READER. Two surfaces publish a timeline to
7+
* somebody with no account: the access-link reader (`/api/public/links`) and
8+
* the case-token resolve (`/api/public/case-tokens`). Each used to decide for
9+
* itself what an entry looked like on the way out, and the access-link reader
10+
* decided nothing: it handed every public note to the link holder as the note
11+
* service shaped it, with the author's user id and display name on it. So the
12+
* decision lives here, once, and both surfaces ask for it.
13+
*
14+
* THE PROJECTION IS A WHITELIST. Five keys leave: `id`, `kind`, `message`,
15+
* `fields`, `occurredAt`. Naming the fields to drop would publish the next one
16+
* somebody adds to an entry; naming the fields to keep cannot.
17+
*
18+
* RECORDS AND NOTES, NOT RECORDS ALONE. A kinded entry (a delivered decision,
19+
* a status that was announced, a portal message) exists only as a timeline
20+
* record. A note exists as a comment, and is projected into a record when it
21+
* is written through the notes endpoint; a note written before records
22+
* existed has no record at all. Reading only records would drop those notes
23+
* from the public view without a word. Reading only notes, as the access-link
24+
* reader did, never showed a kinded entry. So both are read, and a note whose
25+
* record is present is dropped in favour of the record, which carries the kind.
26+
*
27+
* EACH SOURCE FAILS ON ITS OWN. A record table that cannot be read does not
28+
* hide the notes, and the other way round. Every failure is logged at warning.
29+
*
30+
* @category Service
31+
* @package OCA\OpenRegister\Service\Timeline
32+
*
33+
* @author Conduction Development Team <info@conduction.nl>
34+
* @copyright 2026 Conduction B.V.
35+
* @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12
36+
*
37+
* SPDX-FileCopyrightText: 2026 Conduction B.V. <info@conduction.nl>
38+
* SPDX-License-Identifier: EUPL-1.2
39+
*
40+
* @version GIT: <git-id>
41+
*
42+
* @link https://conduction.nl
43+
*
44+
* @spec openspec/changes/access-by-link-not-by-account/specs/public-access-links/spec.md#requirement-a-link-never-sees-past-the-objects-own-rules-req-abl-004
45+
*/
46+
47+
declare(strict_types=1);
48+
49+
namespace OCA\OpenRegister\Service\Timeline;
50+
51+
use OCA\OpenRegister\Db\ObjectEntity;
52+
use OCA\OpenRegister\Service\NoteService;
53+
use OCA\OpenRegister\Service\TimelineVisibilityService;
54+
use Psr\Log\LoggerInterface;
55+
use Throwable;
56+
57+
/**
58+
* Reads and projects the public timeline of one object.
59+
*
60+
* @spec openspec/changes/access-by-link-not-by-account/specs/public-access-links/spec.md#requirement-a-link-never-sees-past-the-objects-own-rules-req-abl-004
61+
*/
62+
class PublicTimeline {
63+
64+
/**
65+
* The only keys an entry carries out of the building.
66+
*
67+
* @var array<int, string>
68+
*/
69+
public const KEYS = ['id', 'kind', 'message', 'fields', 'occurredAt'];
70+
71+
/**
72+
* Constructor.
73+
*
74+
* @param TimelineEntryService $entries Reads the timeline records.
75+
* @param NoteService $notes Reads the notes a record may not exist for yet.
76+
* @param LoggerInterface $logger PSR logger.
77+
*/
78+
public function __construct(
79+
private readonly TimelineEntryService $entries,
80+
private readonly NoteService $notes,
81+
private readonly LoggerInterface $logger,
82+
) {
83+
}//end __construct()
84+
85+
/**
86+
* The public entries on one object, newest first, projected.
87+
*
88+
* The filter is fixed at `public` and is not a parameter, so no caller can
89+
* ask this class for the internal half.
90+
*
91+
* @param ObjectEntity $object The object the timeline hangs on.
92+
* @param integer $limit How many entries at most.
93+
*
94+
* @return array<int, array<string, mixed>> The entries, each with exactly the keys in {@see self::KEYS}.
95+
*
96+
* @spec openspec/changes/access-by-link-not-by-account/specs/public-access-links/spec.md#requirement-a-link-never-sees-past-the-objects-own-rules-req-abl-004
97+
*/
98+
public function forObject(ObjectEntity $object, int $limit = 50): array {
99+
$records = $this->records(object: $object, limit: $limit);
100+
101+
$projected = [];
102+
$noteIdsWithARecord = [];
103+
foreach ($records as $record) {
104+
$commentId = $record['commentId'] ?? null;
105+
if ($commentId !== null) {
106+
$noteIdsWithARecord[(string)$commentId] = true;
107+
}
108+
109+
$projected[] = $this->fromRecord(row: $record);
110+
}
111+
112+
foreach ($this->notesOf(object: $object, limit: $limit) as $note) {
113+
if (isset($noteIdsWithARecord[(string)($note['id'] ?? '')]) === true) {
114+
continue;
115+
}
116+
117+
$projected[] = $this->fromNote(note: $note);
118+
}
119+
120+
usort(
121+
$projected,
122+
fn (array $left, array $right): int => $this->moment(entry: $right) <=> $this->moment(entry: $left)
123+
);
124+
125+
return array_slice($projected, 0, $limit);
126+
}//end forObject()
127+
128+
/**
129+
* When an entry happened, as a number that sorts.
130+
*
131+
* Both sources write ISO 8601 with an offset, but not always the same
132+
* offset, so comparing the strings would order a summer entry against a
133+
* winter one by the digits of their offsets. An entry with no readable
134+
* moment sorts last rather than first.
135+
*
136+
* @param array<string, mixed> $entry A projected entry.
137+
*
138+
* @return integer The Unix time, or 0 when the moment cannot be read.
139+
*/
140+
private function moment(array $entry): int {
141+
$moment = strtotime((string)$entry['occurredAt']);
142+
if ($moment === false) {
143+
return 0;
144+
}
145+
146+
return $moment;
147+
}//end moment()
148+
149+
/**
150+
* The object's public timeline records, as plain rows.
151+
*
152+
* @param ObjectEntity $object The object.
153+
* @param integer $limit How many at most.
154+
*
155+
* @return array<int, array<string, mixed>> The rows, or none when the records cannot be read.
156+
*/
157+
private function records(ObjectEntity $object, int $limit): array {
158+
try {
159+
$entries = $this->entries->listForObject(
160+
object: $object,
161+
visibility: TimelineVisibilityService::PUBLIC_ENTRY,
162+
limit: $limit
163+
);
164+
} catch (Throwable $failure) {
165+
$this->logger->warning(
166+
'[PublicTimeline] Could not read the public timeline records: ' . $failure->getMessage()
167+
);
168+
return [];
169+
}
170+
171+
$rows = [];
172+
foreach ($entries as $entry) {
173+
$rows[] = $entry->jsonSerialize();
174+
}
175+
176+
return $rows;
177+
}//end records()
178+
179+
/**
180+
* The object's public notes.
181+
*
182+
* @param ObjectEntity $object The object.
183+
* @param integer $limit How many at most.
184+
*
185+
* @return array<int, array<string, mixed>> The notes, or none when they cannot be read.
186+
*/
187+
private function notesOf(ObjectEntity $object, int $limit): array {
188+
try {
189+
return $this->notes->getNotesForObject(
190+
objectUuid: (string)$object->getUuid(),
191+
limit: $limit,
192+
offset: 0,
193+
visibility: TimelineVisibilityService::PUBLIC_ENTRY
194+
);
195+
} catch (Throwable $failure) {
196+
$this->logger->warning(
197+
'[PublicTimeline] Could not read the public notes: ' . $failure->getMessage()
198+
);
199+
return [];
200+
}
201+
}//end notesOf()
202+
203+
/**
204+
* One record, cut down to the whitelist.
205+
*
206+
* @param array<string, mixed> $row The record as it serialises.
207+
*
208+
* @return array<string, mixed> The projection.
209+
*/
210+
private function fromRecord(array $row): array {
211+
return [
212+
'id' => (string)($row['id'] ?? ''),
213+
'kind' => (string)($row['kind'] ?? ''),
214+
'message' => (string)($row['message'] ?? ''),
215+
'fields' => (array)($row['fields'] ?? []),
216+
'occurredAt' => (string)($row['created'] ?? ''),
217+
];
218+
}//end fromRecord()
219+
220+
/**
221+
* One note, cut down to the same whitelist.
222+
*
223+
* A note has no kind and no fields, and says so with the empty values
224+
* rather than by leaving the keys out, so a reader sees one shape.
225+
*
226+
* @param array<string, mixed> $note The note as the note service shapes it.
227+
*
228+
* @return array<string, mixed> The projection.
229+
*/
230+
private function fromNote(array $note): array {
231+
return [
232+
'id' => (string)($note['id'] ?? ''),
233+
'kind' => '',
234+
'message' => (string)($note['message'] ?? ''),
235+
'fields' => [],
236+
'occurredAt' => (string)($note['createdAt'] ?? ''),
237+
];
238+
}//end fromNote()
239+
}//end class

0 commit comments

Comments
 (0)