Skip to content

A token reads a base's graph overview and neighbourhood - #1264

Merged
WaylandYang merged 2 commits into
deeplethe:devfrom
gedaiu:feat/token-reads-graph-view
Oct 19, 2026
Merged

WaylandYang merged 2 commits into
deeplethe:devfrom
gedaiu:feat/token-reads-graph-view

Conversation

@gedaiu

@gedaiu gedaiu commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Part of #1262.

What happened

A backend that mirrors into a base could not look at the graph it built: the overview and neighbourhood reads take a session JWT, and a personal token gets 401.

What changed

GET /kbs/{id}/graph/overview and GET /kbs/{id}/graph/neighborhood take SessionOrToken and call require_reader_kb, the check document content and the reads from #1263 use (0052): the owner needs Viewer, any token scope may read, and a token whose kb_ids exclude the base gets 404. The reading logic is unchanged.

0014's decision 7 ("A token reads the graph", from #1263) now counts these two routes, with a reason for each, and its revision line is extended rather than a new one added. The SessionOrToken comment in auth.rs and docs/design/access-and-audit.md name the two reads.

Tests

The shared api::token_read_tests fixture now also covers the two routes, against a real database: a read token of a base viewer gets 200 on every graph read, and a token limited to another base gets 404 on every one. Both fail with 401 when either route is put back to session-only.

Run against Postgres with UTOPIA_TEST_REQUIRE_DB=1: cargo fmt --all -- --check and cargo clippy -p utopia-server --all-targets -- -D warnings clean; cargo test -p utopia-server 683 passed, 2 ignored.

@WaylandYang WaylandYang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, the code is right. Overview and neighbourhood are GETs, use require_reader_kb, and every query beneath them is bound to the base in the path. A neighbourhood started from an entity id in another base comes back empty rather than showing its neighbours, and as_of replays stay inside the base too. Both tests run in CI and fail without the change.

Before merge, the docs need to line up with #1263, which goes in first:

  1. #1263, this PR and #1265 each add a "decision 7" to 0014. One idea shouldn't become decisions 7, 8 and 9. After rebasing on #1263, please add these two routes, with their reasons, to its decision 7 ("A token reads the graph"), and extend its revision line instead of adding another. #1265 can then do the same.
  2. auth.rs:179 lists the routes that take SessionOrToken. #1263 updates it and this PR doesn't. Please add the graph view there too: that comment is where a reader learns which routes a token can reach.
  3. The rebase will also clear the conflicts in api/mod.rs and access-and-audit.md.

@gedaiu

gedaiu commented Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, agreed on all three. Once #1263 is in I'll rebase this onto it, fold the overview and neighbourhood routes with their reasons into its decision 7 and extend its revision line, and add the graph view to the SessionOrToken list in auth.rs. #1265 and #1266 will get the same treatment after this one.

@gedaiu

gedaiu commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

Moving this to draft so only one of the #1262 reads is open at a time, as you asked there. Once #1263 is in I'll rebase this onto it, fold the routes into decision 7 and the shared token_read_tests fixture as agreed above, and mark it ready again. Thanks.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Signed-off-by: Bogdan Szabo <[email protected]>
@gedaiu
gedaiu force-pushed the feat/token-reads-graph-view branch from 18fa84d to 40f901d Compare October 19, 2026 06:56
@gedaiu
gedaiu marked this pull request as ready for review October 19, 2026 06:56
@gedaiu

gedaiu commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

Thank you, WaylandYang, both for the review and for checking that the neighbourhood and as_of reads stay inside the base. I've rebased this onto dev now that #1263 is in, and it's down to one commit.

  • Decision 7: this PR no longer adds its own decision. The overview and neighbourhood are now in decision 7 ("A token reads the graph"), each with its reason: the overview shows a client the shape of what it pushed, and the neighbourhood lets it follow what a statement touches. Decision 7 now counts six reads, and its revision line has a 2026-10-19 note instead of a new line. The status line is unchanged, so A personal token could read the graph of the bases it reaches #1262 can go on it with the last read PR, as agreed on A token reads the entities of the bases it reaches #1263.
  • auth.rs: the SessionOrToken comment now names all six graph reads, overview and neighbourhood included.
  • Conflicts: the rebase cleared api/mod.rs and access-and-audit.md. The design doc now says "six graph reads" and names the two new ones. The duplicate pub(crate) edit to require_reader_kb is gone, since A token reads the entities of the bases it reaches #1263 already made it.
  • Tests: token_graph_view_tests is gone. The two routes are now two more paths in the shared token_read_tests fixture, so this PR only adds the routes it opens. Both tests now cover all six reads (a viewer's read token gets 200, a token limited to another base gets 404). I checked that they go red with 401 when either route is put back to session-only.

Ready for another look whenever you have a moment.

@WaylandYang WaylandYang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the careful rebase. Decision 7 now counts all six reads with a reason for each, the revision line is extended rather than duplicated, and the SessionOrToken comment lists the graph view. Both routes stay bound to the base in the path, so a neighbourhood started from another base's entity comes back empty, and folding the routes into the shared fixture kept the tests nicely small. Merging.

@WaylandYang
WaylandYang merged commit cd14e95 into deeplethe:dev Oct 19, 2026
6 checks passed
gedaiu added a commit to re-cinq/utopia that referenced this pull request Oct 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants