Repository navigation
A token reads the entities of the bases it reaches - #1263
Conversation
WaylandYang
left a comment
There was a problem hiding this comment.
Thank you. The four routes are GETs only and use the same require_reader_kb as document content: the owner's role is the ceiling, and a base outside kb_ids is a 404. Every store read underneath is bound to the base in the path, so another base's entity or history can't be reached by id. The PATCH on the entity path stays session-only, and both tests run in CI and fail without the change.
One thing before merge: please rebase on dev now that #1261 is in. The two conflict in the fact_evidence handler, and the order matters. Without #1261, a token limited to base A could read base B's quotes through /kbs/A/facts/<B's fact>/evidence, so opening that route to tokens first would turn #1260 into a way around kb_ids. After the rebase the handler should call fact_evidence(&state.pool, kb_id, fact_id).
Two small things while you're there. status() in token_read_tests.rs takes a method and builds a JSON body for non-GET requests, but every call is a GET, so please drop that. And the 0014 status line lists #1262 as implemented while the graph-view, ontology and export reads are still open; it would read truer to add #1262 there with the last of those PRs.
|
Thanks. Rebased on |
1903bd0 to
505f0d0
Compare
… reaches, so a service mirroring into Utopia can read its own base back without a person's session Signed-off-by: Bogdan Szabo <[email protected]>
|
Rebased on |
505f0d0 to
08b1e68
Compare
|
Hi, and thank you again for the careful reviews this week. Just a gentle note that this one is rebased on |
WaylandYang
left a comment
There was a problem hiding this comment.
Thank you for the rebase and the cleanups. The evidence handler now calls fact_evidence(&state.pool, kb_id, fact_id) on top of #1261, so a token limited to one base can't reach another base's quotes through the fact id; the test helper is a plain GET; and #1262 stays off the 0014 status line until the last read PR. The four routes each go through require_reader_kb, every store read underneath is bound to the base in the path, and PATCH stays session-only. One small thing for a follow-up rather than this PR: require_reader_kb is now shared, so it would sit more naturally next to SessionOrToken than in documents_routes.rs. Merging.
Part of #1262.
What happened
A backend that mirrors into a base (#1243) could not read the base back: every graph read takes a session JWT, and a personal token gets 401.
What changed
Four reads now take
SessionOrToken, acting as the token's owner: entity search (GET /kbs/{id}/entities), entity detail (GET /kbs/{id}/entities/{eid}), its history (…/history) and a fact's evidence (GET /kbs/{id}/facts/{fid}/evidence). They callrequire_reader_kb, the check document content already uses (0052): the owner needs Viewer on the base, any token scope may read, and a token whosekb_idsexclude the base gets 404. The reading logic is unchanged, andPATCHon the entity path still takes a session.require_reader_kbbecomespub(crate)where it is.0014 gains decision 7, "A token reads the graph", with a reason per route;
docs/design/access-and-audit.mdnames the four reads.Fact evidence is only as narrow as its lookup: #1261 makes it answer within its own base, which matters more once a token limited by
kb_idscan call it.Tests
api::token_read_tests, against a real database: a read token of a base viewer gets 200 on all four routes of its base; a token limited to another base gets 404 on all four. Both fail without the change (401).Run in
rust:1-slim-bookwormagainst Postgres withUTOPIA_TEST_REQUIRE_DB=1:cargo fmt --all --checkandcargo clippy --workspace --all-targets -- -D warningsclean;cargo test -p utopia-server token_15 passed;cargo test -p utopia-server -- graph_routes documents_routes17 passed.