feat(fe): hand apps a session to re-issue their own delegations from - #4273
Merged
sea-snake merged 129 commits intoSep 10, 2026
Conversation
sea-snake
added
the
feature:revocable-app-sessions
This was referenced Aug 22, 2026
sea-snake
marked this pull request as ready for review
August 22, 2026 18:16
|
✅ No security or compliance issues detected. Reviewed everything up to e0044a5. Security Overview
Detected Code Changes
|
sea-snake
force-pushed
the
fe/ii-session-delegation-handler
branch
2 times, most recently
from
August 22, 2026 18:45
ad26518 to
712f197
Compare
This was referenced Aug 22, 2026
sea-snake
force-pushed
the
fe/ii-session-delegation-handler
branch
2 times, most recently
from
August 22, 2026 19:25
d36160e to
c3cb305
Compare
`ii_session_delegation` is what an app calls instead of `icrc34_delegation` when it wants a session rather than a long-lived delegation. It answers with the session chain, extended to the app's own key, and the app mints five-minute delegations from it with no further browser involvement. The chain the app receives has `targets` restricted to the II canister. That is a developer guardrail, not a defence against a thief, who can refresh with it either way: revocability is the protection that matters. The consent duration is honoured — `valid_for` carries the lifetime the user chose, clamped by the canister — and an SSO organization's own cap still binds it, as it already does on the ICRC-34 path. The request carries only a session public key and an optional derivation origin. An app cannot ask for an access level or a lifetime, because both are the user's to decide at consent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
sea-snake
force-pushed
the
fe/ii-session-delegation-handler
branch
from
August 22, 2026 19:49
c3cb305 to
e0fdc2c
Compare
ii_session_delegation took no lifetime, so an application had no way to ask for a session shorter than whatever the consent picker offered, and the request context was set with undefined where the ICRC-34 handler passes what the app asked for. The method now accepts maxTimeToLive on the same terms as ICRC-34: a ceiling rather than a request. What the user picks at consent wins, an SSO organization's cap narrows it further, and the canister clamps the result to between ten minutes and thirty days. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The two are written together here; they are separable so that a later layer can decline the session and keep the mapping.
sea-snake
removed this pull request from stack #4277
September 9, 2026 20:32
sea-snake
added this pull request to stack #4324
September 9, 2026 20:34
sea-snake
removed this pull request from stack #4324
September 9, 2026 20:35
sea-snake
added this pull request to stack #4323
September 9, 2026 20:38
sea-snake
removed this pull request from stack #4323
September 9, 2026 20:42
sea-snake
added this pull request to stack #4326
September 9, 2026 20:43
MRmarioruci
reviewed
Sep 10, 2026
…ilence `StringToBigIntCodec` calls `BigInt` straight, and a throw inside a codec escapes `safeParse` rather than becoming an issue. The parse runs above the handler's `try`, so a duration like `"soon"` never reached the invalid-params branch and the app was told nothing at all. `Nat64StringCodec` reports it instead — and bounds it, since `BigInt` alone accepts `"-1"` and `""`. No regex: a second grammar beside `BigInt`'s own would be free to drift from it. `remapToLegacyDomain` moves to `urlUtils`, beside the two constants it already used, and is re-exported from `iiConnection` for its callers there. It is seven lines of string work with nothing to do with a connection, and importing it from there forced this handler's test to mock the whole legacy module — as the identity function, which quietly disabled the remap it was standing in for. The key soup gets names that say whose key each is: `recordKey` is where the session is stored, `iiSessionIdentity` and `iiSessionPublicKey` are II's own, and the request's `sessionPublicKey` stays the app's. With a comment on why that identity exists at all, which is a security property: over a redirect no verified origin identifies the requester, so the canister must not certify toward a key the request supplied. Tests reach the ceremony for the first time — the canister arguments, the stored record and the returned chain — plus the malformed duration above. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVi99RYo2jyi2kCurgovNJ
sea-snake
removed this pull request from stack #4326
September 10, 2026 12:55
sea-snake
added this pull request to stack #4328
September 10, 2026 12:55
The session credential is scoped to the II canister, and `targets` is part of the message the canister signs. Rebuilding the delegation from the reply left them out, so the chain handed to the app carried a first hop that hashes to nothing in the signature tree — every call made with it came back "Invalid canister signature: the signature tree doesn't contain sig/…/… path". The wire format was never at fault: the result codec carries `targets` both ways. They were gone before anything was encoded, and went into the stored `chainJson` with the rest. The test that should have caught this mocked `targets: []` — Candid's `None`, the shape from before the credential was scoped — so a rebuild that dropped them matched. It now answers as the canister does and asserts the hop keeps them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVi99RYo2jyi2kCurgovNJ
MRmarioruci
approved these changes
Sep 10, 2026
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The request shape conflicts with the documented API, and nat64 parsing accepts non-decimal inputs.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds the frontend JSON-RPC flow that issues revocable app-session chains.
Changes:
- Implements and registers
ii_session_delegation. - Persists sessions and restricts chains to the II canister.
- Advertises the new permission scope and adds handler tests.
File summaries
| File | Description |
|---|---|
urlUtils.ts |
Hosts gateway-domain remapping. |
transport/utils.ts |
Adds nat64 string decoding. |
iiConnection.ts |
Re-exports the moved remapping helper. |
channelStore.ts |
Registers the session handler. |
sessionDelegation.ts |
Creates, stores, and returns app sessions. |
sessionDelegation.test.ts |
Tests requests, issuance, and error mapping. |
icrc25.ts |
Advertises the new scope. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
BigInt reads more than the decimal strings JSON-RPC carries: "" and " " are 0n, "+1" is 1n, "0x10" is 16n, and the nat64 bounds reject none of them. A malformed duration therefore reached the canister as a number it silently clamped, instead of the invalid-params error the app could act on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVi99RYo2jyi2kCurgovNJ
sea-snake
added a commit
that referenced
this pull request
Sep 10, 2026
Design: #4224. Overview: #4230. Groundwork for #4273, which is the second caller. The delegation handler serialises its own requests so that two consent screens cannot race. The session handler coming in #4273 also authorizes, and it has to share that queue rather than keep one of its own — two queues would let one request paint over the other's screen. Lifts the queue out of the delegation handler into `serialize.ts` as `serializeAuthorizationRequest`. No change to what the delegation handler does with it. No tests: `handleDelegationRequest` has no unit coverage today, and the extracted function is a move of the existing queue. Its first test arrives with the second caller in #4273. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
sea-snake
added a commit
that referenced
this pull request
Sep 10, 2026
Design: #4224. Overview: #4230. Nothing imports this until the sign-in in #4273. Settings shows which browsers hold a session so the user can end one they do not recognise, and each entry needs to be something they *will* recognise. The client is the only party that can see what it is running on, so it is the one that has to say. `describeBrowser` resolves the four things the record holds (#4242) — a brand, an operating system, a form factor, and the hardware model where one is reported — rather than composing a label out of them. **Resolving rather than naming is the point.** A composed label fixes the wording at the moment of sign-in: a browser that registered before "Chrome OS" became "ChromeOS" would read the old name for as long as its record existed, and the agent it was parsed from is not kept, so nothing could re-derive it. With tokens the wording lives where the list is read (#4249), so renaming a product — or teaching this table to recognise a browser it used to miss — reaches every stored record instead of only the browsers that sign in afterwards. **Two sources, because neither covers the field alone.** The user agent carries the brand and the system on every engine, and its table stays ordered most specific first, because every later token appears inside the earlier ones' agents: matching `Chrome` first would claim every Chromium-based browser, and every Chromium agent carries `Safari/` as well. Client hints are Chromium-only — Safari and Firefox expose nothing at all — so they supply only what no agent can: the hardware model, which a reduced Android agent reports as the literal string `K`, and a form factor the browser states rather than one inferred. Only the plural `formFactors` is asked for; the singular shape Chromium shipped first is not, because only current browsers are a concern. A stated `Watch`, `XR`, `Automotive` or `EInk` resolves to `Unknown`: the canister names desktops, mobiles and tablets, and calling a watch a phone is worse than naming it as none of them. **A browser that hides reads as the one it imitates.** Brave ships a plain Chrome agent on purpose and strips the hints that would give it away, so it reads as Chrome — which is what the agent says, and the honest answer rather than a gap. Asking `navigator.brave.isBrave()` would name it, at the cost of a check per browser this frontend has heard of, which is the hardcoded list this table exists to avoid. A browser that does name itself in its agent, Vivaldi or DuckDuckGo, travels as the token it gave rather than resolving to Chrome, without needing a row of its own. An unrecognised browser or system is carried as the token that arrived, capped to what the canister accepts, because that row is the one this list exists to surface. `maxTouchPoints` is consumed here rather than stored: an iPad in desktop mode sends a Mac agent and exposes no hints, so it is the only thing that tells it from a Mac. Tests: `describeBrowser.test.ts` (37) — the 20 real user-agent strings, one per rung of both ladders, including the iPad and Mac pair separated only by `maxTouchPoints`; the model used when present and absent when empty or refused; a stated form factor, the four that resolve to `Unknown`, and a tablet stated beside one of them; a browser that hides itself reading as the one it imitates; and an unrecognised token capped to 64 bytes. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
sea-snake
added a commit
that referenced
this pull request
Sep 10, 2026
Design: #4224. Overview: #4230. Nothing imports this until the sign-in in #4273. The canister identifies a browser by a key it proves possession of (#4264), so the frontend has to hold one, and rotate it, so a key captured from one sign-in cannot claim that browser at the next. A non-extractable P-256 keypair in IndexedDB, per identity, plus a successor generated alongside it. A sign-in signs with the current key and announces the successor; the successor is promoted only once the canister has confirmed the sign-in. **Promoting after confirmation rather than before is what survives a lost response.** If the reply never arrives, the browser still holds the key the canister has, so its next attempt presents the same one rather than a successor the canister never saw — which the canister would treat as an unknown browser and enrol as a second entry for the same machine. **Sign-ins are serialised with a web lock**, because two at once would leave whichever wrote last holding a key the canister never accepted. Where the Web Locks API is missing the calls run unserialised: not blocking sign-in on it is the deliberate trade, and the window is one concurrent sign-in on the same device. **A browser that no longer matches what it registered as signs in as a new one.** The description it reported (#4270) is stored beside the key pair, and compared before anything is sent. A registered entry keeps the description it was created with — the canister ignores what a sign-in reports once an entry is being advanced — so a browser whose brand, system, form factor or model has changed would otherwise keep rotating an entry describing something it no longer is. Where it differs, this presents a fresh key pair, which no entry holds and which therefore registers under its own. Deciding here rather than being told keeps that a purely local operation: nothing has been sent when the comparison happens, so the entry left behind is untouched and a retry hits the same state and takes the same branch. The description is written with the key pair and never on its own, so the comparison is always against what the canister was actually sent. The private key never leaves IndexedDB and never appears in a delegation chain. Tests: `browser-key.store.test.ts` (21), including the successor promoted only after the call succeeds, a lost response leaving the current key in place, concurrent sign-ins serialised, the store working with the lock API absent, and a changed description signing in on a fresh key pair while an unchanged one keeps rotating. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
sea-snake
added a commit
that referenced
this pull request
Sep 10, 2026
Design: #4224. Overview: #4230. Consumed by #4273, which stores a record, and #4248, which reads and discards one. A session outlives the tab that created it. Without storing the chain, a sibling subdomain asking for the first time — or a user returning to a closed tab — would mean another ceremony, which is the cadence this design exists to remove. Records keyed by identity, account and origin, in IndexedDB alongside the identity's own session. A record within five minutes of its expiry is not treated as usable, so a chain is never handed over that dies mid-request. Five minutes is half the shortest session a caller can ask for, so a session near that floor is never answered from storage. The purge that already runs when an identity is discarded now takes these records with it, at the eight places that discard one. Leaving them would keep a chain for an identity the browser has forgotten. Three of the four exports have no caller here: `storeAppSession` gains one in #4273, `appSessionsForOrigin` and `discardAppSession` in #4248. `purgeAppSessions` is called by this PR. Tests: `app-session.store.test.ts` (11): storage keyed by identity, account and origin; a record for another origin not served; an expiring record not served; and the purge removing them. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Design: #4224. Overview: #4230. Depends on #4272 for the record store, #4271 for the browser key, and #4270 for the browser description — this is what gives all three their production caller.
The canister side is complete after #4265, but nothing hands an app a session yet. This is the JSON-RPC method that does.
ii_session_delegationis what an app calls instead oficrc34_delegationwhen it wants a session rather than a long-lived delegation. It answers with the session chain, extended to the app's own key, and the app mints five-minute delegations from it with no further browser involvement.The
ii_prefix is deliberate: an RPC method travels over a transport shared with other providers and has to say which one it belongs to, which is why every II-specific RPC method carries it.targetson the chain is restricted to the II canister. That is a developer guardrail, not a defence against a thief — a thief holding the chain can refresh with it either way. Revocability is the protection that matters, and saying otherwise in a security table would be dishonest.The consent duration is honoured, and an SSO organization's cap still binds it:
valid_forcarries the lifetime the user chose, and an SSO identity sends a duration even when the user picked none, matching the ICRC-34 path.The request carries only a session public key and an optional derivation origin. An app cannot ask for an access level or a lifetime, because both are the user's to decide at consent.
icrc25_permissionsnow reportsii_session_delegationas a granted scope so an app can discover the method.icrc34_delegationitself is unchanged.Tests:
sessionDelegation.test.ts(5) covers a request for another method being ignored and params carrying no session key being rejected. The success path has no unit test here, which is worth knowing rather than glossing: the harness does not stub the authentication store, socreateSessionnever completes in it, and neither this path's SSO clamp nor the ICRC-34 one is covered. #4248 stubs enough to drive the handler to an answer and exercises thetargetsrestriction and the held-session path. ReviewcreateSessionby reading it.