feat(be): mint short-lived app delegations from a session - #4265
Merged
sea-snake merged 101 commits intoSep 10, 2026
Merged
Conversation
sea-snake
changed the base branch from
feat/session-create-and-cap
to
feat/sign-in-creates-a-session
August 22, 2026 18:08
sea-snake
force-pushed
the
feat/app-delegation-from-session
branch
from
August 22, 2026 18:08
9e8df39 to
2e9e1e5
Compare
sea-snake
added
the
feature:revocable-app-sessions
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 cfc7352. Security Overview
Detected Code Changes
|
sea-snake
force-pushed
the
feat/app-delegation-from-session
branch
from
August 22, 2026 18:44
2e9e1e5 to
898c4d2
Compare
sea-snake
force-pushed
the
feat/app-delegation-from-session
branch
2 times, most recently
from
August 22, 2026 19:25
247f6fc to
0cca468
Compare
An app holds a session chain and asks for a delegation with it. What it gets lasts five minutes and is not requestable any longer, so revoking the session ends access within one delegation lifetime. The call names nothing and attaches nothing. A new index maps the principal a session's chain is rooted at to the account and browser behind it, so `caller()` alone identifies the session: a hit is itself the proof, since only the holder of that session's key can arrive as that principal. The account is named by principal rather than by locator because materialising a default account changes the locator and leaves the principal alone. The index is matched on browser *and* creation time. A browser keeps its id across sign-ins, so on the browser alone an entry that outlived its session would authenticate its holder as whatever that browser created next. Every path that destroys a session drops its entry in the same write. The `get` re-derives the five-minute ceiling rather than trusting the expiration it is handed, because longer-lived delegations exist over the same account seed. An account's own principal is absent from the index, so an app delegation cannot mint its own replacement. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
sea-snake
force-pushed
the
feat/app-delegation-from-session
branch
from
August 22, 2026 19:49
0cca468 to
05514b2
Compare
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVi99RYo2jyi2kCurgovNJ
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVi99RYo2jyi2kCurgovNJ
MRmarioruci
reviewed
Sep 9, 2026
MRmarioruci
reviewed
Sep 9, 2026
…n-from-session # Conflicts: # src/internet_identity/src/sessions.rs # src/internet_identity/src/storage/account.rs
`authorize_session` handed back a pair its callers unpacked positionally, so what the values had been through lived in the function's doc rather than in what it returned. `AuthorizedSession` can only be built inside it, which makes holding one the evidence that the principal lookup and the liveness check both happened. The two `account()` methods returned two different types. The session handle's returns a principal and says so; the locator's answers with a key. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVi99RYo2jyi2kCurgovNJ
…ing session `app_get_delegation` answered `NoSuchSession` for two failures that are not the session's fault. Both mean the caller asked with an expiration `app_prepare_delegation` never returned: the ceiling guard can only fire on a made-up value — prepare hands back `min(now + 5min, session.valid_till)`, and `now` has only advanced since — and a signature that is absent was never added for those parameters, the session having just been proved live. `NoSuchDelegation` says so, and says what to do: prepare again, rather than sign in afresh. 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 #4276
September 9, 2026 20:34
sea-snake
added this pull request to stack #4323
September 9, 2026 20:34
sea-snake
removed this pull request from stack #4323
September 9, 2026 20:42
sea-snake
added this pull request to stack #4325
September 9, 2026 20:43
MRmarioruci
approved these changes
Sep 10, 2026
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Successful refreshes do not update session activity, causing actively used sessions to expire as idle.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds five-minute app delegations minted from authenticated, revocable account sessions.
Changes:
- Adds app delegation prepare/get endpoints and interface types.
- Resolves callers through session and account principal indexes.
- Adds storage and integration coverage for authorization, expiry, replacement, and permissions.
File summaries
| File | Description |
|---|---|
src/internet_identity/tests/integration/sessions.rs |
Tests app delegation and session lifecycle behavior. |
src/internet_identity/src/storage/tests.rs |
Tests account-principal lookup. |
src/internet_identity/src/storage/storable/session_handle.rs |
Converts stored account principals. |
src/internet_identity/src/storage/account.rs |
Enables session locator use. |
src/internet_identity/src/storage.rs |
Adds principal-based session/account lookup. |
src/internet_identity/src/sessions.rs |
Implements app delegation minting and retrieval. |
src/internet_identity/src/main.rs |
Exposes the new canister endpoints. |
src/internet_identity/internet_identity.did |
Defines the public Candid API. |
src/internet_identity_interface/src/internet_identity/types.rs |
Extends app-session errors. |
src/frontend/src/lib/generated/internet_identity_types.d.ts |
Updates generated TypeScript types. |
src/frontend/src/lib/generated/internet_identity_idl.js |
Updates generated JavaScript IDL. |
src/canister_tests/src/api/internet_identity/api_v2.rs |
Adds test API wrappers. |
Review details
- Files reviewed: 10/12 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
sea-snake
added a commit
that referenced
this pull request
Sep 10, 2026
…4273) 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_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 `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. **`targets` on 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_for` carries 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_permissions` now reports `ii_session_delegation` as a granted scope so an app can discover the method. `icrc34_delegation` itself 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, so `createSession` never 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 the `targets` restriction and the held-session path. Review `createSession` by reading it. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
nmattia
reviewed
Sep 11, 2026
| )) | ||
| } | ||
|
|
||
| /// The one window revocation cannot reach. Matches what MCP mints, and is not |
Collaborator
There was a problem hiding this comment.
I understand MCP was introduced first but arguably its TTL is not very relevant here
| }) | ||
| .ok_or(AppSessionError::NoSuchSession)?; | ||
|
|
||
| if session.is_expired_or_idle(now) { |
Collaborator
There was a problem hiding this comment.
nit (not introduced by this PR): ...or_idle() is confusing since we explicitly allow a session to idle for up to max_idle_ns, and it sounds like being idle for longer than max_idle_ns would expire the session, so I think it would make more sense to call this .is_expired().
| }) | ||
| .ok_or(AppSessionError::NoSuchSession)?; | ||
|
|
||
| if session.is_expired_or_idle(now) { |
Collaborator
There was a problem hiding this comment.
for my own understanding: are sessions removed from storage after they've expired? if so, who's responsible for this?
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 #4268 for the ceremony that creates a session.
This is the PR the feature turns on: after it, a delegation an app holds lasts five minutes and the session behind it can be revoked.
app_prepare_delegation/app_get_delegation. An app holds a session chain and asks for a delegation with it. What it gets is capped at five minutes and the cap is not requestable, so revoking the session ends access within one delegation lifetime.Finding the session from the call. The index mapping a session's rooted principal to
{account_principal, session_id}is written by #4266; this adds the two accessors that read it and the caller that needs them. The call names nothing and attaches nothing:caller()is looked up, the account principal resolves through the principal index from #4238, and the record is read from the list. A hit is itself the proof that the caller is that session, since only the holder of its key can arrive as that principal.The account is named by principal rather than by locator on purpose: materialising a default account changes its locator and leaves its principal alone, so naming an account touches one index entry instead of every session on it.
The entry names the session by its id. A browser keeps its id across sign-ins, so an index entry keyed on the browser and outliving its session would authenticate its holder as whatever that browser created next — a revoked chain coming back to life.
session_idis allocated per session and never reissued, so an entry can only ever resolve to the one session it was written for. Every path that destroys a session drops its index entry in the same write; the id is what makes a stale entry fail closed regardless.The
getre-derives the five-minute ceiling rather than trusting theexpirationit is handed, because longer-lived delegations exist over the same account seed. An account's own principal is absent from the session index, so an app delegation cannot mint its own replacement.Tests:
integration/sessions.rs(11), including a refresh refused once the session has expired, a session replaced by a new ceremony invalidating the previous chain, and the account principal minting nothing itself. Index accessor coverage lands inaccount_principal_index_testsandsession_creation_tests.