Skip to content

feat(be): add a session record to the account reference schema - #4241

Merged
sea-snake merged 266 commits into
mainfrom
feat/session-record-storage
Sep 9, 2026
Merged

sea-snake merged 266 commits into
mainfrom
feat/session-record-storage

Conversation

@sea-snake

@sea-snake sea-snake commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Design: #4224, on the storage of #4222. Overview: #4230. The schema only — #4266 is what writes a session.

An app delegation is unrevocable for as long as it is valid, which is up to 30 days: the client holds a self-contained canister-signed artifact whose verification never consults the canister again. The fix needs somewhere to put a long-lived, revocable record, and #4222 gives a capped, evictable per-account store to hang it on.

A session is (created_at, valid_till, last_refreshed, browser_id, read_only) on the account reference. Nothing else. Putting it there rather than in a map of its own means it inherits the per-anchor caps that already bound references, so revoking, expiring and evicting reuse machinery that exists.

  • The field is Option, so references written under the previous schema decode unchanged, and an empty list is not stored.
  • Only last_refreshed is mutable, which is why it is the one field that will not feed the session's seed: a mutable input would change the session's principal every time it was stamped.
  • A list carries no eviction exemption for holding a session. The list is what makes an app visible in settings, so sparing it would leave the user access they cannot see, and a session nobody can find is a session nobody can revoke. What an eviction costs is a ceremony, not an account: the account is computed, so it returns at the identical principal on the next sign-in.
  • reclaim_order ranks dead sessions first, then live ones on last_used + (last_used - created_at) — recency extended by how long the session stayed in service, so an app in weekly use outranks one opened once yesterday. feat(be): cap sessions per identity #4267 is what enforces a cap with it.

AccountReference::new replaces the struct literals, so adding the field did not spread across every construction site.

Tests: session_tests (11), including a reference written before sessions existed decoding with none, a list holding a session being evictable like any other, and a flood of unused sessions failing to displace a used one.

@sea-snake
sea-snake requested a review from a team as a code owner August 19, 2026 01:13
@zeropath-ai

zeropath-ai Bot commented Aug 19, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to 709ce21.

Security Overview
Detected Code Changes
Change Type Relevant files
Enhancement ► src/internet_identity/src/storage/account.rs
    Add AccountReference::new constructor and integrate with existing AccountReference usage
► src/internet_identity/src/storage/storable/account_reference.rs
    Update to store and convert Sessions within AccountReference
► src/internet_identity/src/storage/storable/session.rs
    Add StorableSession with (de)serialization support
► src/internet_identity/src/storage/storable/browser_id.rs
    Add StorableBrowserId type alias
► src/internet_identity/src/storage/storable/duration.rs
    Add StorableDuration type alias
► src/internet_identity/src/storage/storable/timestamp.rs
    Add StorableTimestamp type alias
► src/internet_identity/src/storage/storable/mod.rs
    Include new storable modules (browser_id, duration, session, timestamp)
Bug Fix ► src/internet_identity/src/storage.rs
    Replace direct AccountReference construction with AccountReference::new in multiple places
► src/internet_identity/src/storage/storable/account_reference_list.rs
    Use AccountReference::new for test reference construction
► src/internet_identity/src/storage/tests.rs
    Update test expectations to use AccountReference::new for default/named references
Refactor ► src/internet_identity/src/storage/storable/account_reference.rs
    Refactor AccountReference to include sessions and new constructor
► src/internet_identity/src/storage/storable/account_reference.rs
    Adjust From/Into implementations to handle new sessions field
► src/internet_identity_interface/src/internet_identity/types.rs
    Introduce BrowserId type alias for per-anchor browser labeling

Copilot AI lite review requested due to automatic review settings August 19, 2026 02:57
@sea-snake
sea-snake force-pushed the feat/session-record-storage branch from 6a6c5f6 to da0fd37 Compare August 19, 2026 02:57

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@sea-snake
sea-snake force-pushed the feat/session-record-storage branch from da0fd37 to 7d057e5 Compare August 19, 2026 06:28
@sea-snake
sea-snake force-pushed the feat/session-record-storage branch from 7d057e5 to 0573b6b Compare August 19, 2026 07:07
@sea-snake
sea-snake force-pushed the feat/session-record-storage branch from 0573b6b to e6333ae Compare August 19, 2026 08:17
@sea-snake
sea-snake force-pushed the feat/session-record-storage branch from e6333ae to dcee0af Compare August 19, 2026 08:37
@sea-snake
sea-snake force-pushed the feat/session-record-storage branch from dcee0af to 1e5b98f Compare August 20, 2026 10:57
@sea-snake
sea-snake force-pushed the feat/session-record-storage branch from 1e5b98f to 40a61a3 Compare August 20, 2026 13:08
@sea-snake
sea-snake force-pushed the feat/session-record-storage branch 3 times, most recently from a47c5f2 to 373891e Compare August 22, 2026 17:24
@sea-snake sea-snake added the feature:revocable-app-sessions Design: revocable app sessions label Aug 22, 2026
@sea-snake
sea-snake force-pushed the feat/session-record-storage branch 2 times, most recently from 28a2358 to cfb3840 Compare August 22, 2026 18:58
@sea-snake sea-snake changed the title feat(be): store revocable sessions on the account reference feat(be): add a session record to the account reference schema Aug 22, 2026
sea-snake and others added 3 commits August 25, 2026 17:49
…l-index

# Conflicts:
#	src/internet_identity/src/storage.rs
`remove_reference_list` discarded the `Result` that
`apply_reference_counter_deltas` now returns, so a failed counter write
was dropped on the path that retires a row. The tests do not see it
because an unused `Result` is a warning until CI runs clippy with
`-D warnings`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An error occurred while trying to automatically change base from feat/account-principal-index-backfill to feat/account-principal-index September 8, 2026 13:10
The six pull requests below this one were squash-merged, so main carries
their content under different commits. The trees are identical, so this
merge takes no content from main and only puts the branch back on top of
it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVi99RYo2jyi2kCurgovNJ
@sea-snake
sea-snake changed the base branch from feat/account-principal-index-backfill to main September 8, 2026 14:13
The identifier a session record carries names a browser, which is what the
registry above this stores and what the settings page shows. Naming it here
keeps the rename out of the branch that introduces the registry, and away
from `device`, which this repository already uses for a passkey.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVi99RYo2jyi2kCurgovNJ
Comment thread src/internet_identity/src/storage/account.rs Outdated
Comment thread src/internet_identity/src/storage/account.rs Outdated
Comment thread src/internet_identity/src/storage/account.rs Outdated
Comment thread src/internet_identity/src/storage/storable/session_record.rs Outdated
sea-snake and others added 2 commits September 8, 2026 17:31
The sweep has shipped and run everywhere it had to: it was in
release-2026-09-08, and the write path has kept the index in step by itself
since the index existed. What is left is a timer that walks every reference
list after each upgrade — its completion flag is heap state, so an upgrade
forgets it — to write entries that are already there.

Goes together with the monitoring query, the cursor and outcome types, and the
tests that covered them. The index itself and the write-path maintenance stay:
those are what keep it true.

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

# Conflicts:
#	src/internet_identity/src/storage/tests.rs
@sea-snake
sea-snake changed the base branch from main to chore/retire-account-principal-index-backfill September 8, 2026 15:53
Base automatically changed from chore/retire-account-principal-index-backfill to main September 9, 2026 10:53
sea-snake and others added 3 commits September 9, 2026 13:12
`is_over` left the caller to find out which bound it meant, and its doc had
to say "on either bound" to make up for it — so the name says it and the doc
explains why it is one question. `demonstrated_use` returns a span rather
than a use, and `reclaim_order` returns a key rather than an order.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVi99RYo2jyi2kCurgovNJ
#4312 and #4318 landed: the backfill is gone, and so is the account counter
discrepancy counter this stack never touched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVi99RYo2jyi2kCurgovNJ
`SessionRecord` is a session, not a record of one, and `Record` reads the way
`Object` would in JavaScript. The storable follows the sibling convention —
`StorableSession` in `storable/session.rs`, next to the other storables named
after their domain type.

`#[cbor(map)]` stores integer indices, so no field name reaches disk and there
is nothing to migrate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVi99RYo2jyi2kCurgovNJ
@sea-snake
sea-snake merged commit d37ebb5 into main Sep 9, 2026
43 checks passed
@sea-snake
sea-snake deleted the feat/session-record-storage branch September 9, 2026 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature:revocable-app-sessions Design: revocable app sessions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants