feat(rpc): allow admin-issued preload UCANs to sign for claimed users#232
feat(rpc): allow admin-issued preload UCANs to sign for claimed users#232rabble wants to merge 2 commits into
Conversation
Drops the is_unclaimed gate in load_preloaded_user_handler so admin-minted preload UCANs continue to work after a user claims their account. Needed to publish additional legacy Vine archives discovered after handover. Mint side (`/api/admin/preload-user`) was already idempotent on vine_id and returned a fresh UCAN regardless of claim status; the signing path was the sole remaining block. Existence check is preserved (None → 404-equivalent). Audit logging for these signs is intentionally not added in this PR; tracked separately in #231 with schema and insertion-site notes.
irab
left a comment
There was a problem hiding this comment.
Looks fine - definitely would be good to change that expiry from 30D to however long the import script normally takes to run. I assume 1-4hrs?
There was a problem hiding this comment.
The new preload signing path needs tighter auth checks before this can merge.
It must only accept real preload UCANs, and cached preload handlers must stop at the UCAN expiry.
The PR description bounds the leaked-token window at 30 days. With the cache-expiry issue below, a warm cache pushes that to effectively unbounded, so the stated tradeoff doesn't hold yet.
Additional findings not shown inline
- Carry UCAN expiry into cached preload handlers
keycast/api/src/api/http/nostr_rpc.rs
Line 453 in dd4d4e4
Carry the UCAN expiry into cached preload handlers.
This path caches preload handlers withexpires_at: None, so after the first valid use, later cache hits return the handler without revalidating the UCAN.
The cache usestime_to_idle(3600), which resets on every access, so a token hit at least once per hour never evicts. A warm cache entry can keep signing past the 30-day UCAN lifetime until process restart, effectively unbounded for continuously-used tokens.
Store the UCAN expiration on the handler or cache entry, and add a test that a warm cache rejects the token after expiry.
| @@ -563,7 +563,8 @@ async fn get_handler( | |||
| h | |||
| } else if is_server_signed(&ucan) { | |||
There was a problem hiding this comment.
Restrict this branch to real preload UCANs before loading personal keys.
Right now any server-signed UCAN without bunker_pubkey reaches load_preloaded_user_handler. That means claim-session tokens, or the admin UCAN returned by GET /api/admin/token, can become full-access RPC signing tokens for whoever the UCAN's audience is, provided personal_keys exists for them.
Require preload-specific facts, such as redirect_origin == "preload" and issued_by_admin, or add a dedicated token-purpose fact before this path loads the handler.
Add tests that accept an admin preload UCAN for a claimed user and reject claim/admin server-signed no-bunker tokens.
…o cache Address Daniel review feedback (#232): 1. Only accept "real" preload UCANs in MODE 2. load_preloaded_user_handler now requires redirect_origin == "preload"; server-signed UCANs from other flows (admin sessions, claim, etc.) are rejected with InvalidToken. Previously, any server-signed UCAN without a bunker_pubkey routed here and could decrypt + sign as any user. 2. Carry UCAN expiry into the cached preload handler. get_handler extracts ucan.expires_at() and threads it through to HttpRpcHandler::new instead of passing expires_at: None. On cache hit, handler.is_valid() now returns false once the UCAN lifetime ends, so a warm cache entry can no longer keep signing past the UCAN's exp until idle eviction. Tests: - test_warm_cache_preload_handler_rejected_after_ucan_expiry: mints a preload UCAN with a 2s lifetime, succeeds once (populates cache), sleeps past exp, and asserts the second call (cache hit) returns InvalidToken and evicts the entry. - test_server_signed_non_preload_redirect_origin_rejected: mints a server-signed UCAN with redirect_origin: "admin" and asserts the signing path rejects it before touching the user's key. All 8 nostr_rpc_integration_test tests pass. Adjacent admin_preload_test, admin_token_test, admin_batch_claim_test tests also pass.
|
@dcadenas Both findings addressed in df843b9: 1. Real preload UCANs only — 2. UCAN expiry carried into cached handler — Tests added (
Both new tests pass. The 6 pre-existing tests in this file still pass, as do the adjacent |
dcadenas
left a comment
There was a problem hiding this comment.
🔑 claimed-user preload signing lands cleanly
Summary
is_unclaimedgate inload_preloaded_user_handler(api/src/api/http/nostr_rpc.rs) so admin-issued preload UCANs continue to work after a user claims their account.InvalidToken.load_preloaded_user_handlerand the MODE 2 branch inget_handlerto reflect the new behavior.Why
We've discovered more classic Vine archives belonging to accounts that users have already claimed, and users want them published under their accounts. The mint side (
/api/admin/preload-user) was already idempotent onvine_idand happily returned a fresh signing UCAN regardless of claim status; the signing endpoint atnostr_rpc.rswas the sole remaining block.Behavior change
Before: a server-signed preload UCAN could only sign for users where
email IS NULL. Once a user claimed, their preload UCANs became dead.After: the same preload UCAN works for both unclaimed and claimed users, until the UCAN itself expires (30 days) or gets evicted from cache.
The mint gate is unchanged: only
is_full_admincallers can mint these UCANs, so the externally-reachable attack surface is the same.What this expands
A leaked admin token can now sign as any user (claimed or not) for as long as a 30-day preload UCAN lives. Before, claim was a user-side seal that closed that door. Acceptable for our small ops team and the Vine republish workflow, but the threat model genuinely changed.
Follow-ups (not in this PR)
admin_audit_eventsso we can answer "who signed what as whom" months after the fact. Schema + insertion-site notes are in the issue.get_user_token(pubkey-based admin endpoint atadmin.rs:344-393) still has its ownis_unclaimedblock. Inconsistent with the new vine_id behavior, but the import script doesn't use that path so it's not blocking. Worth aligning later.Test plan
cargo build -p keycast_api)admin_preload_test,admin_token_test,admin_batch_claim_teststill pass (they test mint-side andis_unclaimedrepository behavior, both unchanged)/api/nostrwithsign_event, verify it returns a signed event instead ofInvalidTokenInvalidToken🤖 Generated with Claude Code