diff --git a/src/frontend/src/lib/generated/internet_identity_idl.js b/src/frontend/src/lib/generated/internet_identity_idl.js index bf914b7ebd..670499547e 100644 --- a/src/frontend/src/lib/generated/internet_identity_idl.js +++ b/src/frontend/src/lib/generated/internet_identity_idl.js @@ -592,6 +592,7 @@ export const idlFactory = ({ IDL }) => { }); const BrowserInfo = IDL.Record({ 'id' : IDL.Nat32, + 'session_count' : IDL.Nat32, 'description' : BrowserDescription, 'created_at' : Timestamp, 'last_used' : Timestamp, diff --git a/src/frontend/src/lib/generated/internet_identity_types.d.ts b/src/frontend/src/lib/generated/internet_identity_types.d.ts index 9b922ade20..683f3dc93c 100644 --- a/src/frontend/src/lib/generated/internet_identity_types.d.ts +++ b/src/frontend/src/lib/generated/internet_identity_types.d.ts @@ -1587,6 +1587,10 @@ export interface BrowserDescription { } export interface BrowserInfo { 'id' : number, + /** + * Sessions this browser holds. Zero means it is signed in to nothing. + */ + 'session_count' : number, /** * Fixed at registration. A sign-in reporting something else registers its own entry, * so this describes a registration rather than the last sign-in. diff --git a/src/internet_identity/internet_identity.did b/src/internet_identity/internet_identity.did index 2809b7a9cf..9d05e94d62 100644 --- a/src/internet_identity/internet_identity.did +++ b/src/internet_identity/internet_identity.did @@ -1046,6 +1046,8 @@ type BrowserInfo = record { created_at : Timestamp; // Advanced by a sign-in from this browser and by every session refresh it drives. last_used : Timestamp; + // Sessions this browser holds. Zero means it is signed in to nothing. + session_count : nat32; }; type IdentityInfo = record { diff --git a/src/internet_identity/src/account_management.rs b/src/internet_identity/src/account_management.rs index e1b8ffa381..ed5c5b8eac 100644 --- a/src/internet_identity/src/account_management.rs +++ b/src/internet_identity/src/account_management.rs @@ -400,7 +400,11 @@ fn should_create_account_for_origin() { use crate::state::{storage_borrow_mut, storage_replace}; storage_replace(storage_with_salt()); - let anchor = storage_borrow_mut(|storage| storage.allocate_anchor(0).unwrap()); + let anchor = storage_borrow_mut(|storage| { + let anchor = storage.allocate_anchor(0).unwrap(); + storage.write(anchor.clone()).unwrap(); + anchor + }); let origin = "https://example.com".to_string(); let name = "Alice".to_string(); @@ -423,7 +427,11 @@ fn should_fail_to_create_accounts_above_max() { use crate::storage::MAX_ANCHOR_ACCOUNTS; storage_replace(storage_with_salt()); - let anchor = storage_borrow_mut(|storage| storage.allocate_anchor(0).unwrap()); + let anchor = storage_borrow_mut(|storage| { + let anchor = storage.allocate_anchor(0).unwrap(); + storage.write(anchor.clone()).unwrap(); + anchor + }); let name = "Alice".to_string(); for i in 0..=MAX_ANCHOR_ACCOUNTS { let origin = format!("https://example-{i}.com"); @@ -443,7 +451,11 @@ fn should_fail_to_update_default_accounts_above_max() { use crate::storage::MAX_ANCHOR_ACCOUNTS; storage_replace(storage_with_salt()); - let anchor = storage_borrow_mut(|storage| storage.allocate_anchor(0).unwrap()); + let anchor = storage_borrow_mut(|storage| { + let anchor = storage.allocate_anchor(0).unwrap(); + storage.write(anchor.clone()).unwrap(); + anchor + }); let name = "Alice".to_string(); for i in 0..MAX_ANCHOR_ACCOUNTS { let origin = format!("https://example-{i}.com"); @@ -468,7 +480,11 @@ fn should_get_accounts_for_origin() { use crate::state::{storage_borrow_mut, storage_replace}; storage_replace(storage_with_salt()); - let anchor = storage_borrow_mut(|storage| storage.allocate_anchor(0).unwrap()); + let anchor = storage_borrow_mut(|storage| { + let anchor = storage.allocate_anchor(0).unwrap(); + storage.write(anchor.clone()).unwrap(); + anchor + }); let origin = "https://example.com".to_string(); let name = "Alice".to_string(); let name_two = "Bob".to_string(); @@ -506,8 +522,16 @@ fn should_only_get_own_accounts_for_origin() { use crate::state::{storage_borrow_mut, storage_replace}; storage_replace(storage_with_salt()); - let anchor = storage_borrow_mut(|storage| storage.allocate_anchor(0).unwrap()); - let anchor_two = storage_borrow_mut(|storage| storage.allocate_anchor(0).unwrap()); + let anchor = storage_borrow_mut(|storage| { + let anchor = storage.allocate_anchor(0).unwrap(); + storage.write(anchor.clone()).unwrap(); + anchor + }); + let anchor_two = storage_borrow_mut(|storage| { + let anchor = storage.allocate_anchor(0).unwrap(); + storage.write(anchor.clone()).unwrap(); + anchor + }); let origin = "https://example.com".to_string(); let name = "Alice".to_string(); let name_two = "Bob".to_string(); @@ -553,7 +577,11 @@ fn should_update_account_for_origin() { use crate::state::{storage_borrow_mut, storage_replace}; storage_replace(storage_with_salt()); - let anchor = storage_borrow_mut(|storage| storage.allocate_anchor(0).unwrap()); + let anchor = storage_borrow_mut(|storage| { + let anchor = storage.allocate_anchor(0).unwrap(); + storage.write(anchor.clone()).unwrap(); + anchor + }); let origin = "https://example.com".to_string(); let name = "Alice".to_string(); let name_two = "Bob".to_string(); @@ -633,7 +661,11 @@ fn should_update_default_account_for_origin() { use crate::state::{storage_borrow_mut, storage_replace}; storage_replace(storage_with_salt()); - let anchor = storage_borrow_mut(|storage| storage.allocate_anchor(0).unwrap()); + let anchor = storage_borrow_mut(|storage| { + let anchor = storage.allocate_anchor(0).unwrap(); + storage.write(anchor.clone()).unwrap(); + anchor + }); let origin = "https://example.com".to_string(); let name = "Alice".to_string(); let name_two = "Bob".to_string(); @@ -782,7 +814,11 @@ fn should_get_default_account_for_origin() { use crate::state::{storage_borrow_mut, storage_replace}; storage_replace(storage_with_salt()); - let anchor = storage_borrow_mut(|storage| storage.allocate_anchor(0).unwrap()); + let anchor = storage_borrow_mut(|storage| { + let anchor = storage.allocate_anchor(0).unwrap(); + storage.write(anchor.clone()).unwrap(); + anchor + }); let origin = "https://example.com".to_string(); let anchor_number = anchor.anchor_number(); @@ -939,7 +975,11 @@ fn can_get_default_before_update_account_for_origin() { use crate::state::{storage_borrow_mut, storage_replace}; storage_replace(storage_with_salt()); - let anchor = storage_borrow_mut(|storage| storage.allocate_anchor(0).unwrap()); + let anchor = storage_borrow_mut(|storage| { + let anchor = storage.allocate_anchor(0).unwrap(); + storage.write(anchor.clone()).unwrap(); + anchor + }); let origin = "https://example.com".to_string(); let anchor_number = anchor.anchor_number(); @@ -957,7 +997,11 @@ fn should_get_updated_default_account_after_modification() { use crate::state::{storage_borrow_mut, storage_replace}; storage_replace(storage_with_salt()); - let anchor = storage_borrow_mut(|storage| storage.allocate_anchor(0).unwrap()); + let anchor = storage_borrow_mut(|storage| { + let anchor = storage.allocate_anchor(0).unwrap(); + storage.write(anchor.clone()).unwrap(); + anchor + }); let origin = "https://example.com".to_string(); let anchor_number = anchor.anchor_number(); @@ -1024,7 +1068,11 @@ fn should_fall_back_to_the_tracked_default_when_the_reservation_is_stale() { // to be there for the same test to hold all the way up the stack. storage.update_salt([17u8; 32]); storage_replace(storage); - let anchor = storage_borrow_mut(|storage| storage.allocate_anchor(0).unwrap()); + let anchor = storage_borrow_mut(|storage| { + let anchor = storage.allocate_anchor(0).unwrap(); + storage.write(anchor.clone()).unwrap(); + anchor + }); let anchor_number = anchor.anchor_number(); let origin = "https://example.com".to_string(); storage_borrow_mut(|storage| storage.write(anchor)).unwrap(); @@ -1058,7 +1106,11 @@ fn should_get_default_account_for_different_origins() { use crate::state::{storage_borrow_mut, storage_replace}; storage_replace(storage_with_salt()); - let anchor = storage_borrow_mut(|storage| storage.allocate_anchor(0).unwrap()); + let anchor = storage_borrow_mut(|storage| { + let anchor = storage.allocate_anchor(0).unwrap(); + storage.write(anchor.clone()).unwrap(); + anchor + }); let origin1 = "https://app1.com".to_string(); let origin2 = "https://app2.com".to_string(); let anchor_number = anchor.anchor_number(); diff --git a/src/internet_identity/src/browser_key.rs b/src/internet_identity/src/browser_key.rs index efd22607be..e142390ba0 100644 --- a/src/internet_identity/src/browser_key.rs +++ b/src/internet_identity/src/browser_key.rs @@ -3,7 +3,7 @@ //! Knows nothing of sessions or storage, so both layers may depend on it: the endpoint //! verifies, and storage requires the [`VerifiedBrowserKeys`] that verifying produces. //! -//! Nothing calls it yet: the sign-in ceremony that does is added on top of this. +//! `prepare_account_session`, the one caller that verifies, lands two PRs up. #![allow(dead_code)] use internet_identity_interface::internet_identity::types::{PublicKey, SessionKey}; @@ -35,6 +35,13 @@ pub struct VerifiedBrowserKeys { } impl VerifiedBrowserKeys { + /// Keys that were never verified, for tests about what happens *after* verification. + /// `#[cfg(test)]`, so no canister build can reach it and the type stays evidence. + #[cfg(test)] + pub fn unverified_for_test(current: PublicKey, next: PublicKey) -> Self { + Self { current, next } + } + /// The key this sign-in was reached by. pub fn current(&self) -> &PublicKey { &self.current diff --git a/src/internet_identity/src/delegation.rs b/src/internet_identity/src/delegation.rs index 50ea7769dd..33490b1c2b 100644 --- a/src/internet_identity/src/delegation.rs +++ b/src/internet_identity/src/delegation.rs @@ -124,6 +124,34 @@ pub fn calculate_account_seed_with_salt( hash_bytes(blob) } +const SESSION_SEED_PREFIX: &str = "session"; + +/// The seed of a session's canister-signed identity. +/// +/// Built on the account's own seed, so a session survives anything that leaves the +/// account's principal unchanged, including naming a default account. `session_id` is +/// the only other input, and no two sessions are ever allocated the same one, so a +/// revoked session's identity can never be arrived at a second time. Unguessability +/// comes from the salt. +pub fn calculate_session_seed_with_salt( + salt: &[u8; 32], + account_seed: &Hash, + session_id: SessionId, +) -> Hash { + fn push_field(blob: &mut Vec, data: &[u8]) { + blob.extend_from_slice(&(data.len() as u64).to_be_bytes()); + blob.extend_from_slice(data); + } + + let mut blob: Vec = vec![]; + push_field(&mut blob, salt); + push_field(&mut blob, SESSION_SEED_PREFIX.as_bytes()); + push_field(&mut blob, account_seed); + push_field(&mut blob, &session_id.to_be_bytes()); + + hash_bytes(blob) +} + fn hash_bytes(value: impl AsRef<[u8]>) -> Hash { let mut hasher = Sha256::new(); hasher.update(value.as_ref()); diff --git a/src/internet_identity/src/email_recovery/remove.rs b/src/internet_identity/src/email_recovery/remove.rs index 50d7551bd5..1ee52f55e5 100644 --- a/src/internet_identity/src/email_recovery/remove.rs +++ b/src/internet_identity/src/email_recovery/remove.rs @@ -77,6 +77,7 @@ mod tests { let mut a = Anchor { browsers: vec![], next_browser_id: 0, + session_count: 0, anchor_number: 1, devices: vec![], openid_credentials: vec![], diff --git a/src/internet_identity/src/storage.rs b/src/internet_identity/src/storage.rs index af6038f0f7..6740ebaa91 100644 --- a/src/internet_identity/src/storage.rs +++ b/src/internet_identity/src/storage.rs @@ -79,7 +79,7 @@ //! //! The archive buffer memory is managed by the [MemoryManager] and is currently limited to a single //! bucket of 128 pages. -use account::{Account, AccountKey, AccountsCounter}; +use account::{Account, AccountKey, AccountsCounter, SessionLocator}; use candid::{CandidType, Deserialize, Principal}; use ic_cdk::api::stable::WASM_PAGE_SIZE_IN_BYTES; use std::borrow::Cow; @@ -102,13 +102,16 @@ use ic_stable_structures::{ use identity_jose::jwk::Jwk; use internet_identity_interface::archive::types::BufferedEntry; -use crate::delegation::{self, check_frontend_length}; +use crate::browser_key::VerifiedBrowserKeys; +use crate::delegation::{self, calculate_session_seed_with_salt, check_frontend_length}; use crate::openid::OpenIdCredentialKey; use crate::state::PersistentState; use crate::stats::event_stats::AggregationKey; use crate::stats::event_stats::{EventData, EventKey}; -use crate::storage::account::AccountReference; -use crate::storage::anchor::Anchor; +use crate::storage::account::{ + AccountReference, Session, DEFAULT_SESSION_IDLE_NS, MIN_SESSION_IDLE_NS, +}; +use crate::storage::anchor::{Anchor, BrowserError, MAX_BROWSERS}; use crate::storage::memory_wrapper::MemoryWrapper; use crate::storage::registration_rates::RegistrationRates; use crate::storage::storable::account::StorableAccount; @@ -120,6 +123,8 @@ use crate::storage::storable::application::StorableOriginSha256; use crate::storage::storable::application_number::StorableApplicationNumber; use crate::storage::storable::passkey_credential::StorablePasskeyCredential; use crate::storage::storable::recovery_key::StorableRecoveryKey; +use crate::storage::storable::session_handle::StorableSessionHandle; +use crate::storage::storable::session_id::StorableSessionId; use internet_identity_interface::internet_identity::types::*; use storable::anchor::StorableAnchor; use storable::anchor_number::StorableAnchorNumber; @@ -208,6 +213,8 @@ const MCP_REGISTRATION_MEMORY_INDEX: u8 = 31u8; const SSO_STABLE_ID_INDEX_MEMORY_INDEX: u8 = 32u8; const NEXT_APPLICATION_NUMBER_MEMORY_INDEX: u8 = 33u8; const LOOKUP_ACCOUNT_WITH_PRINCIPAL_MEMORY_INDEX: u8 = 34u8; +const LOOKUP_SESSION_WITH_PRINCIPAL_MEMORY_INDEX: u8 = 35u8; +const NEXT_SESSION_ID_MEMORY_INDEX: u8 = 36u8; const ANCHOR_MEMORY_ID: MemoryId = MemoryId::new(ANCHOR_MEMORY_INDEX); const ARCHIVE_BUFFER_MEMORY_ID: MemoryId = MemoryId::new(ARCHIVE_BUFFER_MEMORY_INDEX); @@ -291,9 +298,15 @@ const NEXT_APPLICATION_NUMBER_MEMORY_ID: MemoryId = /// Reverse index from the principal a dapp sees to the account that produced it: /// `self_authenticating(der_encode_canister_sig_key(seed)) -> (anchor, application, account)`. +const LOOKUP_SESSION_WITH_PRINCIPAL_MEMORY_ID: MemoryId = + MemoryId::new(LOOKUP_SESSION_WITH_PRINCIPAL_MEMORY_INDEX); const LOOKUP_ACCOUNT_WITH_PRINCIPAL_MEMORY_ID: MemoryId = MemoryId::new(LOOKUP_ACCOUNT_WITH_PRINCIPAL_MEMORY_INDEX); +/// Monotonic [`SessionId`] allocator. A revoked session's id is retired, never reissued, +/// which is what makes the revocation final: the id is an input to the session seed. +const NEXT_SESSION_ID_MEMORY_ID: MemoryId = MemoryId::new(NEXT_SESSION_ID_MEMORY_INDEX); + // The bucket size 128 is relatively low, to avoid wasting memory when using // multiple virtual memories for smaller amounts of data. // This value results in 256 GB of total managed memory, which should be enough @@ -407,9 +420,19 @@ pub struct Storage { /// Memory wrapper used to report the size of the application-number allocator. next_application_number_memory_wrapper: MemoryWrapper>, next_application_number_memory: StableCell>, + /// Memory wrapper used to report the size of the session-id allocator. + next_session_id_memory_wrapper: MemoryWrapper>, + next_session_id_memory: StableCell>, lookup_account_with_principal_memory_wrapper: MemoryWrapper>, lookup_account_with_principal_memory: StableBTreeMap>, + /// Where a session lives, keyed by the principal its chain is rooted at. An app-facing + /// call carries nothing but that principal, so this is what turns `caller()` into a + /// session. + /// Memory wrapper used to report the size of the session index. + lookup_session_with_principal_memory_wrapper: MemoryWrapper>, + lookup_session_with_principal_memory: + StableBTreeMap>, /// Memory wrapper used to report the size of the lookup anchor with OpenID credential memory. lookup_anchor_with_openid_credential_memory_wrapper: MemoryWrapper>, lookup_anchor_with_openid_credential_memory: @@ -543,8 +566,11 @@ impl Storage { memory_manager.get(STABLE_DEFAULT_ACCOUNT_REFERENCE_MEMORY_ID); let stable_account_counter_memory = memory_manager.get(STABLE_ACCOUNT_COUNTER_MEMORY_ID); let next_application_number_memory = memory_manager.get(NEXT_APPLICATION_NUMBER_MEMORY_ID); + let next_session_id_memory = memory_manager.get(NEXT_SESSION_ID_MEMORY_ID); let lookup_account_with_principal_memory = memory_manager.get(LOOKUP_ACCOUNT_WITH_PRINCIPAL_MEMORY_ID); + let lookup_session_with_principal_memory = + memory_manager.get(LOOKUP_SESSION_WITH_PRINCIPAL_MEMORY_ID); let lookup_anchor_with_openid_credential_memory = memory_manager.get(LOOKUP_ANCHOR_WITH_OPENID_CREDENTIAL_MEMORY_ID); let lookup_anchor_with_passkey_credential_memory = @@ -630,9 +656,18 @@ impl Storage { ), next_application_number_memory: StableCell::init(next_application_number_memory, 0) .expect("next_application_number_memory"), + next_session_id_memory_wrapper: MemoryWrapper::new(next_session_id_memory.clone()), + next_session_id_memory: StableCell::init(next_session_id_memory, 0) + .expect("next_session_id_memory"), lookup_account_with_principal_memory_wrapper: MemoryWrapper::new( lookup_account_with_principal_memory.clone(), ), + lookup_session_with_principal_memory_wrapper: MemoryWrapper::new( + lookup_session_with_principal_memory.clone(), + ), + lookup_session_with_principal_memory: StableBTreeMap::init( + lookup_session_with_principal_memory, + ), lookup_account_with_principal_memory: StableBTreeMap::init( lookup_account_with_principal_memory, ), @@ -873,6 +908,7 @@ impl Storage { verified_emails: _, browsers: _, next_browser_id: _, + session_count: _, }) = previous_anchor_maybe { ( @@ -1567,6 +1603,48 @@ impl Storage { } } + /// Everything this identity has stored, keyed the way a write takes it. + /// + /// The symmetry is the point: an operation that touches many origins reads this, + /// changes what it means to change, and writes it back. Nothing has to range over + /// storage itself, and no application number reaches the caller. + /// + /// Stored entries only. An origin nothing has been stored under is not in the map — + /// there are unboundedly many of those — so a caller that means to write at one + /// reaches for [`Self::account_state_for_origin`] and puts it there. + fn account_state( + &self, + anchor_number: AnchorNumber, + ) -> BTreeMap { + self.stable_account_reference_list_memory + .range( + (anchor_number, ApplicationNumber::MIN)..=(anchor_number, ApplicationNumber::MAX), + ) + .filter_map(|((_, application_number), list)| { + // A reference list for an application that does not exist is illegal, and + // this is where it surfaces: the write gate is keyed by origin, so a list + // whose application number resolves to nothing cannot be handed to a + // caller. Skipping it is still the right answer — refusing here would + // block every sweep for the identity, including the origins that do + // resolve — but it is not something to pass over quietly. + let Some(application) = self.stable_application_memory.get(&application_number) + else { + ic_cdk::println!( + "ERROR: account reference list invariant violated: identity \ + {anchor_number} holds a list for application {application_number}, \ + which is not stored. Its sessions are unreachable and unrevocable." + ); + return None; + }; + let account_references = Vec::::from(list) + .into_iter() + .map(AccountReferenceWrite::from) + .collect(); + Some((application.origin, Some((account_references, None)))) + }) + .collect() + } + /// This identity's account state at one origin, in the shape a write takes it. /// /// An origin nothing has been stored under normalises to the derived default, so no @@ -1611,12 +1689,100 @@ impl Storage { /// Returns what the identity now holds — the input with the minted account numbers /// filled in, each list in the order it was given. fn write_account_state( + &mut self, + anchor: Anchor, + writes: BTreeMap, + ) -> Result, StorageError> { + let given_up = self.browsers_given_up(&anchor)?; + let validated = self.validate_account_state(anchor.anchor_number(), &given_up, writes)?; + Ok(self.apply_account_state(anchor, validated)) + } + + /// Browsers this write gives up, whose sessions have to go with them. + /// + /// Derived from the registry the write carries rather than stated by a caller: a + /// caller that has to say so is a caller that can forget to, and every path that + /// changes the registry passes through here. + /// + /// Only a full registry gives a browser up, so a smaller one answers without reading + /// the stored anchor — which keeps the anchor off the path of every write that cannot + /// have dropped anything, delegation refreshes included. + fn browsers_given_up(&self, anchor: &Anchor) -> Result, StorageError> { + if anchor.browsers().len() < MAX_BROWSERS { + return Ok(BTreeSet::new()); + } + let held: BTreeSet = anchor.browsers().iter().map(|one| one.id).collect(); + Ok(self + .read(anchor.anchor_number())? + .browsers() + .iter() + .map(|one| one.id) + .filter(|id| !held.contains(id)) + .collect()) + } + + /// [`Self::write_account_state`] for a test that has an anchor number rather than the + /// anchor. Production takes the anchor itself, so that a caller cannot hold a copy + /// across the write and put the session count back afterwards. + #[cfg(test)] + fn write_account_state_for_testing( &mut self, anchor_number: AnchorNumber, writes: BTreeMap, ) -> Result, StorageError> { - let validated = self.validate_account_state(anchor_number, writes)?; - Ok(self.apply_account_state(anchor_number, validated)) + let anchor = self.read(anchor_number)?; + self.write_account_state(anchor, writes) + } + + /// `writes`, less every session held by a browser this write gives up. + /// + /// The sessions of a browser that is gone are gone with it, wherever they are, and in + /// the same write — a browser retired while its sessions still minted delegations + /// would go on being signed in from a list nothing shows. + /// + /// Only the origins that hold such a session are added. Adding the rest would cost + /// nothing to store and everything to the eviction rule, which spares an origin the + /// write is already changing; and the scan itself only happens when a browser was + /// actually given up, so an ordinary sign-in still reads the one origin it names. + fn without_sessions_of( + &self, + anchor_number: AnchorNumber, + browsers_given_up: &BTreeSet, + mut writes: BTreeMap, + ) -> BTreeMap { + if browsers_given_up.is_empty() { + return writes; + } + + let holds_one = |held: &AccountReferenceListWrite| { + held.as_ref().is_some_and(|(account_references, _)| { + account_references.iter().any(|write| { + write + .account_reference + .sessions + .iter() + .any(|session| browsers_given_up.contains(&session.browser_id)) + }) + }) + }; + for (origin, held) in self.account_state(anchor_number) { + if holds_one(&held) { + writes.entry(origin).or_insert(held); + } + } + + for held in writes.values_mut() { + let Some((account_references, _)) = held else { + continue; + }; + for write in account_references.iter_mut() { + write + .account_reference + .sessions + .retain(|session| !browsers_given_up.contains(&session.browser_id)); + } + } + writes } /// Everything that can refuse. Reads what is stored, works out what would be minted @@ -1624,6 +1790,7 @@ impl Storage { fn validate_account_state( &self, anchor_number: AnchorNumber, + browsers_given_up: &BTreeSet, writes: BTreeMap, ) -> Result { let mut minting = MintingState { @@ -1631,6 +1798,7 @@ impl Storage { global: self.stable_account_counter_memory.get().clone(), }; + let writes = self.without_sessions_of(anchor_number, browsers_given_up, writes); let written_origins: BTreeSet = writes.keys().cloned().collect(); let mut validated = Vec::with_capacity(writes.len()); for (origin, write) in writes { @@ -1956,6 +2124,8 @@ impl Storage { config: None, deltas: ReferenceListDeltas::default(), principal_salt: None, + accounts_changed: false, + sessions_changed: false, previous_references: Vec::new(), current_references: Vec::new(), }); @@ -2019,7 +2189,11 @@ impl Storage { .any(|(previous, new)| previous.account_number != new.account_number)); // Resolved here, so a missing salt refuses with nothing written rather than // half-way through. - let principal_salt = if accounts_changed { + // Sessions are held on the account references too, and their index and the + // identity's session count both follow from the same pair of lists. + let sessions_changed = writes_a_list + && Self::session_ids_of(&previous_references) != Self::session_ids_of(&references); + let principal_salt = if accounts_changed || sessions_changed { Some(*self.salt().ok_or(StorageError::SaltNotSet)?) } else { None @@ -2057,6 +2231,8 @@ impl Storage { config, deltas, principal_salt, + accounts_changed, + sessions_changed, previous_references, current_references: references, }) @@ -2126,6 +2302,8 @@ impl Storage { config: None, deltas: ReferenceListDeltas::default(), principal_salt: None, + accounts_changed: false, + sessions_changed: false, previous_references: Vec::new(), current_references: Vec::new(), }; @@ -2182,6 +2360,8 @@ impl Storage { config: None, deltas, principal_salt, + accounts_changed: true, + sessions_changed: true, previous_references: previous, current_references: Vec::new(), }) @@ -2194,9 +2374,10 @@ impl Storage { /// is a broken invariant rather than a case to report. fn apply_account_state( &mut self, - anchor_number: AnchorNumber, + mut anchor: Anchor, validated: ValidatedAccountStateWrite, ) -> BTreeMap { + let anchor_number = anchor.anchor_number(); let ValidatedAccountStateWrite { writes, anchor_counter, @@ -2213,6 +2394,13 @@ impl Storage { self.stable_anchor_account_counter_memory .insert(anchor_number, anchor_counter); + // Moved once for the whole call rather than once per origin: the count lives on + // the anchor, and an operation spanning several origins would otherwise read, + // change and write the same record several times over. The same is true of the + // per-browser counts, which live on the browser entries of that same record. + let mut session_delta = 0i64; + let mut browser_deltas: BTreeMap = BTreeMap::new(); + let mut written = BTreeMap::new(); for one in writes { let ValidatedAccountReferenceListWrite { @@ -2223,6 +2411,8 @@ impl Storage { records, config, principal_salt, + accounts_changed, + sessions_changed, previous_references, current_references, written: result, @@ -2263,14 +2453,31 @@ impl Storage { // derived from an account's stored record, so one that is not in yet derives // nothing and a newly named account would get no entry. if let Some(salt) = principal_salt { - self.sync_account_principal_index( - anchor_number, - application_number, - &origin, - &salt, - &previous_references, - ¤t_references, - ); + if accounts_changed { + self.sync_account_principal_index( + anchor_number, + application_number, + &origin, + &salt, + &previous_references, + ¤t_references, + ); + } + if sessions_changed { + session_delta += self.sync_session_index( + anchor_number, + application_number, + &origin, + &salt, + &previous_references, + ¤t_references, + ); + Self::accumulate_browser_deltas( + &mut browser_deltas, + &previous_references, + ¤t_references, + ); + } } match list { @@ -2292,6 +2499,27 @@ impl Storage { written.insert(origin, result); } + if session_delta != 0 { + anchor.session_count = if session_delta < 0 { + anchor + .session_count + .saturating_sub(session_delta.unsigned_abs() as u32) + } else { + anchor.session_count.saturating_add(session_delta as u32) + }; + } + anchor.move_browser_session_counts(&browser_deltas); + + // Taking the identity record is taking the storing of it, so it is stored whatever + // was changed on it — the count above, or anything a caller changed before giving + // it up. Storing it only where this function's own change landed would discard the + // caller's, silently. + // + // Trapping rather than reporting: an `Err` on the IC commits everything above this + // line, so a record that could not be stored has to take the whole message with it. + self.write(anchor) + .expect("the identity record this write was handed cannot be written back"); + written } @@ -2487,6 +2715,383 @@ impl Storage { .collect() } + /// The principals a set of references derives to. A reference whose account list is + /// gone derives nothing and is skipped. + /// The account one reference names, built from the reference and the record it + /// points at. + /// + /// Not [`Self::read_account`], which reads the stored list and so answers `None` for + /// a reference that is being removed. This derives from the list it is handed, which + /// is what lets the index be diffed across a write. + fn account_of_reference( + &self, + anchor_number: AnchorNumber, + origin: &FrontendHostname, + reference: &AccountReference, + ) -> Option { + match reference.account_number { + None => Some(Account::new(anchor_number, origin.clone(), None, None)), + Some(account_number) => { + let stored = self.stable_account_memory.get(&account_number)?; + Some(Account::new_full( + anchor_number, + origin.clone(), + Some(stored.name), + Some(account_number), + reference.last_used, + stored.seed_from_anchor, + )) + } + } + } + + /// Hands out the next session id, which no session has held before. + /// + /// Refuses at the ceiling rather than saturating. The id is an input to the session + /// seed, so reissuing one would let a revoked session's identity be arrived at a + /// second time — the thing this counter exists to prevent. + fn allocate_session_id(&mut self) -> Result { + let session_id = *self.next_session_id_memory.get(); + self.next_session_id_memory + .set( + session_id + .checked_add(1) + .ok_or(StorageError::SessionIdOverflow)?, + ) + .map_err(|_| StorageError::ErrorUpdatingSessionIdAllocator)?; + Ok(session_id) + } + + // Called by `prepare_account_session`, which lands two PRs up. + #[allow(dead_code)] + /// Creates the session `prepare_account_session` mints an identity from, replacing + /// whatever this browser already held at this account. + pub fn create_session( + &mut self, + params: CreateSessionParams, + ) -> Result<(SessionLocator, Session), StorageError> { + let CreateSessionParams { + anchor_number, + origin, + account_number, + browser_keys, + browser_description, + valid_till_ns, + max_idle_ns, + read_only, + now_ns, + } = params; + + // A session that is over before it starts would be pruned by the sweep below, in + // the same call that created it, and this would return `Ok` naming a session no + // list holds. Refused here instead, where nothing has been read or stored yet. + if valid_till_ns <= now_ns { + return Err(StorageError::SessionAlreadyOver { anchor_number }); + } + + let mut anchor = self.read(anchor_number)?; + + // Defaulted and clamped here rather than at the caller, so every path that + // creates a session gets the same answer whatever it asked for. The ceiling is + // the life this session was actually granted: a bound longer than that could + // never be reached, and storing one would say something untrue about it. + // + // Raised then lowered rather than clamped in one call: `clamp` panics when its + // floor exceeds its ceiling, which a session granted less than the floor would + // do, and a trap is a poor answer to a short session. + let granted = valid_till_ns.saturating_sub(now_ns); + let max_idle_ns = max_idle_ns + .unwrap_or(DEFAULT_SESSION_IDLE_NS) + .max(MIN_SESSION_IDLE_NS) + .min(granted); + + // One write, not three. What the identity holds at this origin afterwards: the + // account reference list, created by this write where the origin is new, the + // session itself, and the dead sessions pruned off every reference beside it. + // Everything that can refuse does so before any of it is stored. + let stored = self + .lookup_application_number_with_origin(&origin) + .and_then(|application_number| { + self.stored_account_references(anchor_number, application_number) + }); + // A named account lives in a list that already exists, and an origin nothing has + // been stored under has none. + if stored.is_none() && account_number.is_some() { + return Err(StorageError::MissingAccount { + anchor_number, + name: origin, + }); + } + + // Resolved here rather than by a caller, because what follows from it is this + // function's to work out: the registry may be at its cap, in which case a browser + // is given up and every session it held has to go in the same write. A caller + // handed that consequence is a caller that can forget it. + // + // After the refusals above, so a ceremony that cannot happen registers nothing — + // the record reaches storage only through the write at the end. + let (browser_id, _) = anchor + .resolve_browser( + browser_keys.current().clone(), + browser_keys.next().clone(), + browser_description, + now_ns, + ) + .map_err(StorageError::Browser)?; + + // This origin, and only this one. A browser the registry gave up to make room for + // this one may hold sessions anywhere, but that is a consequence of the write + // rather than something this function reaches across the identity to do: the gate + // derives it from the registry this write carries and sweeps them in the same + // write. Handing the gate every origin instead would make each of them an origin + // this write is changing, and an origin a write is changing is never a candidate + // for its own eviction — so the one write that creates tracked defaults would be + // the one write that can never evict them. + let mut state = BTreeMap::from([( + origin.clone(), + Some(self.account_state_for_origin(anchor_number, &origin)), + )]); + + let (account_references, _) = state + .get_mut(&origin) + .and_then(Option::as_mut) + .expect("the origin was just put there"); + + let position = account_references + .iter() + .position(|write| write.account_reference.account_number == account_number) + .ok_or(StorageError::MissingAccount { + anchor_number, + name: String::new(), + })?; + let reference = &mut account_references[position].account_reference; + reference.last_used = Some(now_ns); + + // A ceremony replaces whatever this browser held here, rather than reusing it: the + // copy of an old session's chain stops working at the user's next sign-in instead of + // at its expiry. + let mut dropped: Vec<(Option, Session)> = vec![]; + reference.sessions.retain(|session| { + if session.browser_id == browser_id { + dropped.push((account_number, session.clone())); + return false; + } + true + }); + + // A gap costs nothing — ids need not be contiguous, and the write below can still + // refuse this ceremony. What must never happen is an id being handed out twice: + // it is an input to the session seed, so a reissued one would let a revoked + // session's identity be reached a second time. + let session_id = self.allocate_session_id()?; + let session = Session { + session_id, + created_at_ns: now_ns, + valid_till_ns, + max_idle_ns, + last_refreshed_ns: None, + browser_id, + read_only, + }; + reference.sessions.push(session.clone()); + + // The whole list, not just the reference being written: this list is about to be + // rewritten anyway, and a dead session on a sibling reference has nothing else + // coming for it. + for write in account_references.iter_mut() { + let account_number = write.account_reference.account_number; + write.account_reference.sessions.retain(|session| { + if session.is_expired_or_idle(now_ns) { + dropped.push((account_number, session.clone())); + return false; + } + true + }); + } + + // The list is the whole of it: the index entries for the session created here and + // for the ones pruned above, and the identity's session count, all follow from it. + // One write for all of it: the session created here, the dead ones pruned above, + // the sessions of every browser the registry gave up, the account reference list + // this origin gets if it did not have one, and the identity's session count. + self.write_account_state(anchor, state)?; + + let key = SessionLocator { + anchor_number, + origin, + account_number, + session_id, + }; + Ok((key, session)) + } + + /// The session `key` names, or `None` where the identity holds no such session. + /// + /// A key whose session was replaced reads as `None` rather than as its successor: + /// the successor was allocated an id of its own. + #[allow(dead_code)] // Read by `get_account_session`, which lands two PRs up. + pub fn read_session(&self, key: &SessionLocator) -> Option { + let application_number = self.lookup_application_number_with_origin(&key.origin)?; + + self.account_references(key.anchor_number, application_number) + .into_iter() + .find(|reference| reference.account_number == key.account_number)? + .sessions + .into_iter() + .find(|session| session.session_id == key.session_id) + } + + // Called by the `revoke_browser_sessions` endpoint, which lands six PRs up. + #[allow(dead_code)] + /// Signs one browser out of everything, in a single message. + pub fn revoke_browser_sessions( + &mut self, + anchor_number: AnchorNumber, + browser_id: BrowserId, + ) -> Result { + // Read what the identity holds, take the browser's sessions out of it, write it + // back. Nothing here ranges over storage itself and no application number reaches + // this function: the sweep is one write, so an `Err` cannot sign the browser out + // of some applications and report failure. + let anchor = self.read(anchor_number)?; + let mut state = self.account_state(anchor_number); + + let mut revoked = 0u64; + for held in state.values_mut() { + let Some((account_references, _)) = held else { + continue; + }; + for write in account_references.iter_mut() { + write.account_reference.sessions.retain(|session| { + let keep = session.browser_id != browser_id; + if !keep { + revoked += 1; + } + keep + }); + } + } + + self.write_account_state(anchor, state)?; + + Ok(revoked) + } + + /// The session index entries a reference list implies: one per session it holds, + /// each with the account entry its handle needs in order to resolve. + fn session_entries( + &self, + anchor_number: AnchorNumber, + application_number: ApplicationNumber, + origin: &FrontendHostname, + salt: &[u8; 32], + references: &[AccountReference], + ) -> BTreeMap { + let mut entries = BTreeMap::new(); + for reference in references { + let Some(account) = self.account_of_reference(anchor_number, origin, reference) else { + continue; + }; + let account_seed = account.calculate_seed_with_salt(salt); + let account_principal = + delegation::canister_sig_principal(canister_id(), account_seed.to_vec()); + for session in &reference.sessions { + let seed = + calculate_session_seed_with_salt(salt, &account_seed, session.session_id); + entries.insert( + delegation::canister_sig_principal(canister_id(), seed.to_vec()), + ( + StorableSessionHandle { + account_principal: account_principal.as_slice().to_vec(), + session_id: session.session_id, + }, + StorableAccountKey { + anchor_number, + application_number, + account_number: reference.account_number, + }, + ), + ); + } + } + entries + } + + /// The ids of every session a reference list holds, sorted, for comparing two + /// versions of a list. + fn session_ids_of(references: &[AccountReference]) -> Vec { + let mut ids: Vec = references + .iter() + .flat_map(|reference| reference.sessions.iter().map(|session| session.session_id)) + .collect(); + ids.sort_unstable(); + ids + } + + /// Adds what one reference-list write does to each browser's session count. + /// + /// Accumulated across the origins of a single write rather than returned per origin, + /// because a browser signed in at several origins is one entry on the identity + /// record, and the record is stored once. + fn accumulate_browser_deltas( + deltas: &mut BTreeMap, + previous: &[AccountReference], + current: &[AccountReference], + ) { + for (references, sign) in [(previous, -1i64), (current, 1i64)] { + for reference in references { + for session in &reference.sessions { + *deltas.entry(session.browser_id).or_default() += sign; + } + } + } + } + + /// Keeps the session index in step with one reference-list write, and reports what + /// the write does to the identity's session count. + /// + /// Sessions live on the reference, so a reference that goes takes its sessions with + /// it and this sees them as removed without any caller saying so. That is the point: + /// the list and everything derived from it move together, in the one place holding + /// both versions of it. + fn sync_session_index( + &mut self, + anchor_number: AnchorNumber, + application_number: ApplicationNumber, + origin: &FrontendHostname, + salt: &[u8; 32], + previous: &[AccountReference], + current: &[AccountReference], + ) -> i64 { + let before = + self.session_entries(anchor_number, application_number, origin, salt, previous); + let after = self.session_entries(anchor_number, application_number, origin, salt, current); + + for principal in before.keys() { + if !after.contains_key(principal) { + self.lookup_session_with_principal_memory.remove(principal); + } + } + for (principal, (handle, account)) in &after { + if before.contains_key(principal) { + continue; + } + // The account's entry goes in with the session's. A handle names its account + // by principal, and that index gains entries only where a list's set of + // account numbers changes, which a sign-in does not. Without this a session at + // a list that predates the index resolves to nothing, and nothing else would + // put it right. + self.lookup_account_with_principal_memory.insert( + Principal::from_slice(&handle.account_principal), + account.clone(), + ); + self.lookup_session_with_principal_memory + .insert(*principal, handle.clone()); + } + + after.len() as i64 - before.len() as i64 + } + /// Retires an application no anchor references any more. The number is never /// reissued. fn remove_unreferenced_application( @@ -2642,6 +3247,12 @@ impl Storage { // An absent list normalises to the derived default, which is how the first named // account at an origin does not cost the identity the default it had. A // tombstone normalises to nothing and stays that way. + // Read once and handed to the write, for two reasons: the gate moves this + // identity's session count and must not be handed a copy that has already gone + // stale, and an identity that does not exist has nothing to hold what is about to + // be written — the counters, the account reference lists and the session count all + // key on a record that would not be there. + let anchor = self.read(anchor_number)?; let (mut account_references, config) = self.account_state_for_origin(anchor_number, &origin); // Where the write leaves it, and so where its minted number comes back. @@ -2657,7 +3268,7 @@ impl Storage { }); let written = self.write_account_state( - anchor_number, + anchor, BTreeMap::from([(origin.clone(), Some((account_references, config)))]), )?; @@ -2708,6 +3319,7 @@ impl Storage { } } + let anchor = self.read(anchor_number)?; let (mut account_references, config) = self.account_state_for_origin(anchor_number, &origin); let Some(position) = account_references @@ -2757,7 +3369,7 @@ impl Storage { } let written = self.write_account_state( - anchor_number, + anchor, BTreeMap::from([(origin.clone(), Some((account_references, config)))]), )?; let write = &written[&origin] @@ -2800,6 +3412,7 @@ impl Storage { ) -> Result<(), StorageError> { check_frontend_length(&origin); + let anchor = self.read(anchor_number)?; let (account_references, _) = self.account_state_for_origin(anchor_number, &origin); // The stored config with one field moved, rather than a config built here: this // knows about the default account and nothing else the config may come to hold, @@ -2812,7 +3425,7 @@ impl Storage { .unwrap_or_default(); config.default_account_number = account_number; self.write_account_state( - anchor_number, + anchor, BTreeMap::from([(origin, Some((account_references, Some(config))))]), )?; Ok(()) @@ -3026,6 +3639,14 @@ impl Storage { "next_application_number".to_string(), self.next_application_number_memory_wrapper.size(), ), + ( + "lookup_session_with_principal".to_string(), + self.lookup_session_with_principal_memory_wrapper.size(), + ), + ( + "next_session_id".to_string(), + self.next_session_id_memory_wrapper.size(), + ), ( "stable_anchor_application_config".to_string(), self.stable_anchor_application_config_memory_wrapper.size(), @@ -3068,6 +3689,29 @@ impl Storage { } } +// Constructed by `prepare_account_session`, which lands two PRs up. +#[allow(dead_code)] +pub struct CreateSessionParams { + pub anchor_number: AnchorNumber, + pub origin: FrontendHostname, + pub account_number: Option, + /// The keys the browser proved it holds, and the successor it announced. Verified + /// rather than reported: registering an entry from keys nobody proved would let one + /// browser be claimed by whoever read its keys off the wire, so what this takes is + /// the evidence and not two byte strings. + /// + /// Its registry entry, its id, and whatever the cap gives up to make room for it are + /// all worked out inside the write, so no caller states any of them. + pub browser_keys: VerifiedBrowserKeys, + /// Taken only where this sign-in registers a browser. An entry that is advanced + /// keeps the description it was registered with. + pub browser_description: BrowserDescription, + pub valid_till_ns: Timestamp, + pub max_idle_ns: Option, + pub read_only: bool, + pub now_ns: Timestamp, +} + #[cfg(not(test))] fn canister_id() -> Principal { ic_cdk::id() @@ -3218,6 +3862,12 @@ struct ValidatedAccountReferenceListWrite { /// nothing written; the index itself is synced in apply, after the records, because a /// principal is derived from an account's stored record. principal_salt: Option<[u8; 32]>, + /// Whether the set of account numbers moved, and whether the set of sessions did. + /// They are tracked apart because a sign-in changes only the second, and recomputing + /// account principals it did not touch would put the hottest write in the system + /// through a per-account hash for nothing. + accounts_changed: bool, + sessions_changed: bool, previous_references: Vec, current_references: Vec, /// What this origin holds afterwards, handed back to the caller. @@ -3379,6 +4029,12 @@ pub enum StorageError { AccountLimitReached { anchor_number: AnchorNumber, }, + /// The browser presenting itself could not be resolved to a registry entry. + Browser(BrowserError), + /// A session was asked for that is already over, which no list would hold. + SessionAlreadyOver { + anchor_number: AnchorNumber, + }, AnchorNumberOutOfRange { anchor_number: AnchorNumber, range: (AnchorNumber, AnchorNumber), @@ -3415,6 +4071,11 @@ pub enum StorageError { /// put two origins on a single list. ApplicationsCounterOverflow, ErrorUpdatingApplicationNumberAllocator, + /// No session ids left to hand out. Refused rather than saturated: the id is an + /// input to the session seed, so reissuing one would resurrect a revoked session's + /// identity. + SessionIdOverflow, + ErrorUpdatingSessionIdAllocator, /// The references a write assembled cannot be stored as they stand. UnstorableAccountReferenceList { anchor_number: AnchorNumber, @@ -3429,6 +4090,12 @@ pub enum StorageError { stored: u64, delta: i64, }, + /// Reclaiming ran and the identity is still at the session cap. Unreachable unless + /// reclaiming stopped honouring its contract, which is why it is an error rather than a + /// refused sign-in: the sign-in is the thing this cap must never fail. + SessionCapNotReclaimed { + anchor_number: AnchorNumber, + }, /// Tried to bind a recovery email that's already on a different /// anchor. The "one anchor per address" invariant from design /// §8.2 is enforced at the storage layer; the caller surfaces @@ -3450,6 +4117,11 @@ impl fmt::Display for StorageError { range.0, range.1 ), Self::BadAnchorNumber(n) => write!(f, "bad Identity Anchor {n}"), + Self::Browser(err) => write!(f, "the browser could not be resolved: {err:?}"), + Self::SessionAlreadyOver { anchor_number } => write!( + f, + "a session for Identity Anchor {anchor_number} would be over before it started" + ), Self::DeserializationError(err) => { write!(f, "failed to deserialize a Candid value: {err}") } @@ -3506,6 +4178,10 @@ impl fmt::Display for StorageError { Self::ErrorUpdatingApplicationNumberAllocator => { write!(f, "Error updating the application number allocator") } + Self::SessionIdOverflow => write!(f, "No session ids left to allocate"), + Self::ErrorUpdatingSessionIdAllocator => { + write!(f, "Error updating the session id allocator") + } Self::UnstorableAccountReferenceList { anchor_number, application_number, @@ -3529,6 +4205,10 @@ impl fmt::Display for StorageError { f, "recovery email is already bound to a different anchor ({existing_anchor})", ), + Self::SessionCapNotReclaimed { anchor_number } => write!( + f, + "anchor {anchor_number} is at the session cap and reclaiming freed nothing" + ), } } } diff --git a/src/internet_identity/src/storage/account.rs b/src/internet_identity/src/storage/account.rs index b6ca9b123c..b6abd34d38 100644 --- a/src/internet_identity/src/storage/account.rs +++ b/src/internet_identity/src/storage/account.rs @@ -8,7 +8,7 @@ use ic_cdk::trap; use ic_certification::Hash; use internet_identity_interface::internet_identity::types::{ AccountInfo, AccountNameValidationError, AccountNumber, AnchorNumber, BrowserId, - FrontendHostname, Timestamp, UserKey, + FrontendHostname, SessionId, Timestamp, UserKey, }; use serde::{Deserialize, Serialize}; @@ -53,8 +53,54 @@ impl AccountReference { } } -/// A revocable session at one account. Only `last_refreshed` is mutable, which is why -/// it is the one field absent from the seed. +/// The shortest idle bound a session may be given. +/// +/// An app delegation lasts five minutes and an active application replaces it a +/// little before it expires, so a bound anywhere near that would end sessions +/// plainly in use. Ten minutes is already the floor on a session's own length, +/// so this shares that range rather than introducing a second one. +pub const MIN_SESSION_IDLE_NS: u64 = 10 * crate::MINUTE_NS; + +/// What a session gets when its ceremony asks for no bound of its own. +/// +/// Seven days of nobody touching an application ends the sign-in, well inside the +/// thirty days a session may otherwise live. It is the length of an absence rather +/// than of a session: coming back inside a week keeps you signed in indefinitely, +/// and a machine walked away from stops being signed in within one. +pub const DEFAULT_SESSION_IDLE_NS: u64 = 7 * crate::DAY_NS; + +/// Where one session is stored, and which session it is. +/// +/// The account addresses the list; `session_id` picks the record out of it. The id is +/// unique on its own, so every operation is compare-and-act: a key for a session that +/// was replaced reads as `None` and revokes nothing, instead of landing on its +/// successor. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct SessionLocator { + pub anchor_number: AnchorNumber, + pub origin: FrontendHostname, + pub account_number: Option, + pub session_id: SessionId, +} + +impl SessionLocator { + // Used by `app_prepare_delegation`, which lands three PRs up. + #[allow(dead_code)] + /// The account this session is at. + pub fn account_key(&self) -> AccountKey { + AccountKey { + anchor_number: self.anchor_number, + origin: self.origin.clone(), + account_number: self.account_number, + } + } +} + +/// A revocable session at one account. +/// +/// `session_id` is what the seed binds, so the identity this session signs with is +/// tied to the one record that was allocated that id. Every other field describes the +/// session and can be rewritten without changing who it signs as. #[derive(Clone, Debug, Eq, PartialEq, Ord, PartialOrd)] pub struct Session { pub created_at_ns: Timestamp, @@ -63,6 +109,7 @@ pub struct Session { pub last_refreshed_ns: Option, pub browser_id: BrowserId, pub read_only: bool, + pub session_id: SessionId, } impl Session { @@ -94,13 +141,14 @@ impl Session { /// /// The extension is what separates an app in weekly use from one opened once and /// abandoned, which recency alone gets backwards — the abandoned one was touched more - /// recently. `browser_id` only makes the order total. - pub fn reclaim_sort_key(&self, now: Timestamp) -> (bool, Timestamp, BrowserId) { + /// recently. `session_id` only makes the order total, which it can because no two + /// sessions share one. + pub fn reclaim_sort_key(&self, now: Timestamp) -> (bool, Timestamp, SessionId) { let last_used = self.last_refreshed_ns.unwrap_or(self.created_at_ns); ( !self.is_expired_or_idle(now), last_used.saturating_add(self.time_in_service_ns()), - self.browser_id, + self.session_id, ) } } diff --git a/src/internet_identity/src/storage/account/tests.rs b/src/internet_identity/src/storage/account/tests.rs index ee21ed3f62..7326295dc2 100644 --- a/src/internet_identity/src/storage/account/tests.rs +++ b/src/internet_identity/src/storage/account/tests.rs @@ -27,7 +27,9 @@ fn should_create_a_named_account() { storage.update_salt([17u8; 32]); // 1. Define additional account parameters - let anchor_number: AnchorNumber = 10_000; + let anchor = storage.allocate_anchor(0).unwrap(); + let anchor_number = anchor.anchor_number(); + storage.write(anchor).unwrap(); let origin: FrontendHostname = "https://some.origin".to_string(); let account_name = "account name".to_string(); @@ -99,7 +101,9 @@ fn should_list_accounts() { storage.update_salt([17u8; 32]); // 1. Define additional account parameters - let anchor_number: AnchorNumber = 10_000; + let anchor = storage.allocate_anchor(0).unwrap(); + let anchor_number = anchor.anchor_number(); + storage.write(anchor).unwrap(); let origin: FrontendHostname = "https://some.origin".to_string(); let account_name = "account name".to_string(); @@ -166,7 +170,9 @@ fn should_list_all_identity_accounts() { storage.update_salt([17u8; 32]); // 1. Define additional account parameters - let anchor_number: AnchorNumber = 10_000; + let anchor = storage.allocate_anchor(0).unwrap(); + let anchor_number = anchor.anchor_number(); + storage.write(anchor).unwrap(); let account_name = "account name".to_string(); let origin: FrontendHostname = "https://some.origin".to_string(); let origin_2: FrontendHostname = "https://some-other.origin".to_string(); @@ -223,7 +229,9 @@ fn should_update_default_account() { storage.update_salt([17u8; 32]); // 1. Define parameters - let anchor_number: AnchorNumber = 10_000; + let anchor = storage.allocate_anchor(0).unwrap(); + let anchor_number = anchor.anchor_number(); + storage.write(anchor).unwrap(); let origin: FrontendHostname = "https://some.origin".to_string(); let account_name = "account name".to_string(); @@ -279,7 +287,9 @@ fn should_update_additional_account() { storage.update_salt([17u8; 32]); // 1. Define additional account parameters - let anchor_number: AnchorNumber = 10_000; + let anchor = storage.allocate_anchor(0).unwrap(); + let anchor_number = anchor.anchor_number(); + storage.write(anchor).unwrap(); let origin: FrontendHostname = "https://some.origin".to_string(); let account_name = "account name".to_string(); let new_account_name = "new account name".to_string(); @@ -485,7 +495,9 @@ fn should_not_read_a_default_account_from_an_empty_reference_list() { let mut storage = Storage::new((10_000, 3_784_873), memory); // 1. Define parameters - let anchor_number: AnchorNumber = 10_000; + let anchor = storage.allocate_anchor(0).unwrap(); + let anchor_number = anchor.anchor_number(); + storage.write(anchor).unwrap(); let origin: FrontendHostname = "https://some.origin".to_string(); // 2. Create application but with empty account reference list @@ -510,7 +522,9 @@ fn should_read_a_synthetic_default_account_when_no_reference_list_exists() { let memory = VectorMemory::default(); let mut storage = Storage::new((10_000, 3_784_873), memory); - let anchor_number: AnchorNumber = 10_000; + let anchor = storage.allocate_anchor(0).unwrap(); + let anchor_number = anchor.anchor_number(); + storage.write(anchor).unwrap(); let origin: FrontendHostname = "https://some.origin".to_string(); // The origin is known, but this identity has no list under it. crate::storage::tests::application_number_for(&mut storage, &origin); @@ -536,9 +550,14 @@ fn should_not_read_account_from_wrong_anchor() { let mut storage = Storage::new((10_000, 3_784_873), memory); storage.update_salt([17u8; 32]); - // 1. Define parameters for two different anchors - let anchor_number_1: AnchorNumber = 10_000; - let anchor_number_2: AnchorNumber = 10_001; + // 1. Two different identities. Both are allocated, because a write against an + // identity that does not exist has nothing to write to. + let anchor_1 = storage.allocate_anchor(0).unwrap(); + let anchor_number_1 = anchor_1.anchor_number(); + storage.write(anchor_1).unwrap(); + let anchor_2 = storage.allocate_anchor(0).unwrap(); + let anchor_number_2 = anchor_2.anchor_number(); + storage.write(anchor_2).unwrap(); let origin: FrontendHostname = "https://some.origin".to_string(); let account_name = "account name".to_string(); diff --git a/src/internet_identity/src/storage/anchor.rs b/src/internet_identity/src/storage/anchor.rs index 7286498ccf..0e1f89693f 100644 --- a/src/internet_identity/src/storage/anchor.rs +++ b/src/internet_identity/src/storage/anchor.rs @@ -18,7 +18,7 @@ use internet_identity_interface::internet_identity::types::openid::OpenIdCredent use internet_identity_interface::internet_identity::types::verified_email::VerifiedEmail; use internet_identity_interface::internet_identity::types::*; use serde_bytes::ByteBuf; -use std::collections::HashMap; +use std::collections::{BTreeMap, HashMap}; use std::fmt; #[cfg(test)] @@ -43,6 +43,7 @@ pub struct Anchor { /// Capped by `MAX_BROWSERS`. pub(crate) browsers: Vec, pub(crate) next_browser_id: BrowserId, + pub(crate) session_count: u32, pub(crate) metadata: Option>, pub(crate) name: Option, pub(crate) created_at: Option, @@ -93,6 +94,9 @@ pub struct Browser { pub description: BrowserDescription, pub created_at: Timestamp, pub last_used: Timestamp, + /// Sessions this browser holds. Maintained by the write that changes the reference + /// lists holding them, so it counts stored records rather than live ones. + pub session_count: u32, } impl From for Browser { @@ -104,6 +108,7 @@ impl From for Browser { description: BrowserDescription::from(value.description), created_at: value.created_at, last_used: value.last_used, + session_count: value.session_count, } } } @@ -117,6 +122,7 @@ impl From for StorableBrowser { description: StorableBrowserDescription::from(value.description), created_at: value.created_at, last_used: value.last_used, + session_count: value.session_count, } } } @@ -255,6 +261,7 @@ impl From for (StorableFixedAnchor, StorableAnchor) { verified_emails, browsers, next_browser_id, + session_count, metadata, name, created_at, @@ -517,6 +524,7 @@ impl From for (StorableFixedAnchor, StorableAnchor) { verified_emails, browsers, next_browser_id, + session_count: Some(session_count), }, ) } @@ -534,6 +542,7 @@ impl From<(AnchorNumber, StorableAnchor)> for Anchor { verified_emails, browsers, next_browser_id, + session_count, } = storable_anchor; let name = name.clone(); @@ -654,6 +663,7 @@ impl From<(AnchorNumber, StorableAnchor)> for Anchor { verified_emails, browsers, next_browser_id, + session_count: session_count.unwrap_or_default(), devices, metadata, } @@ -677,6 +687,7 @@ impl From<(AnchorNumber, StorableFixedAnchor, Option)> for Ancho let Some(storable_anchor) = storable_anchor else { return Anchor { name: None, + session_count: 0, openid_credentials: vec![], email_recovery: vec![], verified_emails: vec![], @@ -723,6 +734,7 @@ impl From<(AnchorNumber, StorableFixedAnchor, Option)> for Ancho verified_emails, browsers, next_browser_id: storable_anchor.next_browser_id.unwrap_or_default(), + session_count: storable_anchor.session_count.unwrap_or_default(), metadata, name, created_at, @@ -735,6 +747,25 @@ impl Anchor { &self.browsers } + /// Moves each browser's session count by what a write added to or took from it. + /// + /// A delta against a browser no entry holds is dropped: the cap can retire an entry + /// while sessions it opened are still stored, and a count belongs to an entry that + /// exists. + pub fn move_browser_session_counts(&mut self, deltas: &BTreeMap) { + for browser in &mut self.browsers { + let Some(delta) = deltas.get(&browser.id) else { + continue; + }; + browser.session_count = match delta { + 0.. => browser.session_count.saturating_add(*delta as u32), + _ => browser + .session_count + .saturating_sub(delta.unsigned_abs() as u32), + }; + } + } + /// What a caller outside storage may know about this anchor's browsers: an /// identifier, what the browser said it was, and when. The keys stay here — they /// are how a sign-in proves which entry it is, so handing them out would let @@ -754,6 +785,7 @@ impl Anchor { description: browser.description.clone(), created_at: browser.created_at, last_used: browser.last_used, + session_count: browser.session_count, }) .collect(), ) @@ -831,6 +863,7 @@ impl Anchor { description, created_at: now, last_used: now, + session_count: 0, }); let mut dropped = vec![]; @@ -858,6 +891,7 @@ impl Anchor { Self { anchor_number, created_at: Some(created_at), + session_count: 0, devices: vec![], openid_credentials: vec![], email_recovery: vec![], diff --git a/src/internet_identity/src/storage/anchor/tests.rs b/src/internet_identity/src/storage/anchor/tests.rs index 6dedacd86a..3843a97179 100644 --- a/src/internet_identity/src/storage/anchor/tests.rs +++ b/src/internet_identity/src/storage/anchor/tests.rs @@ -225,6 +225,7 @@ fn should_prevent_mutation_when_invariants_are_violated() { let mut anchor = Anchor { browsers: vec![], next_browser_id: 0, + session_count: 0, anchor_number: ANCHOR_NUMBER, devices: vec![ device1.clone(), @@ -249,6 +250,7 @@ fn should_prevent_addition_when_invariants_are_violated() { let mut anchor = Anchor { browsers: vec![], next_browser_id: 0, + session_count: 0, anchor_number: ANCHOR_NUMBER, devices: vec![ recovery_phrase(1, DeviceProtection::Unprotected), @@ -273,6 +275,7 @@ fn should_allow_removal_when_invariants_are_violated() { let mut anchor = Anchor { browsers: vec![], next_browser_id: 0, + session_count: 0, anchor_number: ANCHOR_NUMBER, devices: vec![ device1.clone(), diff --git a/src/internet_identity/src/storage/storable.rs b/src/internet_identity/src/storage/storable.rs index afae1b51c2..42fe069a91 100644 --- a/src/internet_identity/src/storage/storable.rs +++ b/src/internet_identity/src/storage/storable.rs @@ -28,6 +28,8 @@ pub mod openid_jwks; pub mod passkey_credential; pub mod recovery_key; pub mod session; +pub mod session_handle; +pub mod session_id; pub mod special_device_migration; pub mod sso_stable_id_key; pub mod storable_persistent_state; diff --git a/src/internet_identity/src/storage/storable/anchor.rs b/src/internet_identity/src/storage/storable/anchor.rs index cff73754b3..657c6abf03 100644 --- a/src/internet_identity/src/storage/storable/anchor.rs +++ b/src/internet_identity/src/storage/storable/anchor.rs @@ -40,6 +40,10 @@ pub struct StorableAnchor { /// Monotonic per-anchor allocator for `browsers`. Ids are never reused. #[n(8)] pub next_browser_id: Option, + /// Stored sessions this anchor holds. Expiry removes a session with no write to + /// observe, so this can read higher than what is live until a write prunes them. + #[n(9)] + pub session_count: Option, } impl Storable for StorableAnchor { diff --git a/src/internet_identity/src/storage/storable/browser.rs b/src/internet_identity/src/storage/storable/browser.rs index 3f703b427d..820ee918fb 100644 --- a/src/internet_identity/src/storage/storable/browser.rs +++ b/src/internet_identity/src/storage/storable/browser.rs @@ -21,4 +21,8 @@ pub struct StorableBrowser { pub current_browser_key: Vec, #[cbor(n(5), with = "minicbor::bytes")] pub next_browser_key: Vec, + /// Sessions this browser holds, counted where the reference lists holding them are + /// written. + #[n(6)] + pub session_count: u32, } diff --git a/src/internet_identity/src/storage/storable/session.rs b/src/internet_identity/src/storage/storable/session.rs index 4322c57c11..ac59530007 100644 --- a/src/internet_identity/src/storage/storable/session.rs +++ b/src/internet_identity/src/storage/storable/session.rs @@ -1,6 +1,7 @@ use crate::storage::account::Session; use crate::storage::storable::browser_id::StorableBrowserId; use crate::storage::storable::duration::StorableDuration; +use crate::storage::storable::session_id::StorableSessionId; use crate::storage::storable::timestamp::StorableTimestamp; use ic_stable_structures::storable::Bound; use ic_stable_structures::Storable; @@ -22,6 +23,8 @@ pub struct StorableSession { pub browser_id: StorableBrowserId, #[n(5)] pub read_only: bool, + #[n(6)] + pub id: StorableSessionId, } impl Storable for StorableSession { @@ -43,10 +46,11 @@ impl From for Session { Session { created_at_ns: value.created_at_ns, valid_till_ns: value.valid_till_ns, - last_refreshed_ns: value.last_refreshed_ns, max_idle_ns: value.max_idle_ns, + last_refreshed_ns: value.last_refreshed_ns, browser_id: value.browser_id, read_only: value.read_only, + session_id: value.id, } } } @@ -56,10 +60,11 @@ impl From for StorableSession { StorableSession { created_at_ns: value.created_at_ns, valid_till_ns: value.valid_till_ns, - last_refreshed_ns: value.last_refreshed_ns, max_idle_ns: value.max_idle_ns, + last_refreshed_ns: value.last_refreshed_ns, browser_id: value.browser_id, read_only: value.read_only, + id: value.session_id, } } } diff --git a/src/internet_identity/src/storage/storable/session_handle.rs b/src/internet_identity/src/storage/storable/session_handle.rs new file mode 100644 index 0000000000..fb61be06d3 --- /dev/null +++ b/src/internet_identity/src/storage/storable/session_handle.rs @@ -0,0 +1,86 @@ +use crate::storage::storable::session_id::StorableSessionId; +use ic_stable_structures::storable::Bound; +use ic_stable_structures::Storable; +use minicbor::{Decode, Encode}; +use std::borrow::Cow; + +/// Where the session a caller authenticates as is stored. +/// +/// The account is named by its principal rather than by its locator because materialising a +/// default account changes the locator and leaves the principal alone, so a rename touches +/// one entry in the principal index instead of every session of that account. +/// +/// The session itself is named by its id, which is an input to the session seed, so an +/// entry can only ever resolve to the one session whose principal is its own key. +#[derive(Encode, Decode, Clone, Debug, Eq, PartialEq)] +// An array rather than a map, for the reason given on `StorableAccountKey`: a map's keys buy +// decoding records written before a field existed, which a derived index does not need. +// Nothing sweeps this one today, but the sessions themselves are held on the account +// references, so it can be rebuilt from them if a shape change ever calls for it. +#[cbor(array)] +pub struct StorableSessionHandle { + #[cbor(n(0), with = "minicbor::bytes")] + pub account_principal: Vec, + #[n(1)] + pub session_id: StorableSessionId, +} + +impl Storable for StorableSessionHandle { + fn to_bytes(&self) -> Cow<'_, [u8]> { + let mut buffer = Vec::new(); + minicbor::encode(self, &mut buffer).expect("failed to encode StorableSessionHandle"); + Cow::Owned(buffer) + } + + fn from_bytes(bytes: Cow<'_, [u8]>) -> Self { + minicbor::decode(&bytes).expect("failed to decode StorableSessionHandle") + } + + /// The array header, then the principal as a byte string — twenty-nine bytes and a + /// two-byte header, a self-authenticating principal being a hash and a tag — then a + /// maximal `u64` at nine. Declared so the map sizes its pages to what it stores rather + /// than to a default, which is only possible before it holds anything. + /// + /// `is_fixed_size` is false: the session id encodes shorter when it is small, and the + /// flag is read for keys alone in any case. + const BOUND: Bound = Bound::Bounded { + max_size: 41, + is_fixed_size: false, + }; +} + +#[cfg(test)] +mod tests { + use super::*; + use candid::Principal; + use ic_stable_structures::storable::Bound; + use pretty_assertions::assert_eq; + + /// A declared bound is a promise the derive does not keep: exceed it and the write + /// panics rather than the type failing to compile. So the maximum is asserted here, + /// where adding a field breaks a test instead of a canister. + #[test] + fn the_largest_handle_fits_the_declared_bound() { + let largest = StorableSessionHandle { + account_principal: vec![0xff; Principal::MAX_LENGTH_IN_BYTES], + session_id: u64::MAX, + }; + + let Bound::Bounded { max_size, .. } = StorableSessionHandle::BOUND else { + panic!("the bound is what this test is about"); + }; + assert_eq!(largest.to_bytes().len() as u32, max_size); + } + + /// What the index actually holds: `canister_sig_principal` is self-authenticating, so + /// the principal is always the full twenty-nine bytes and only the id varies. + #[test] + fn a_real_handle_is_shorter_than_the_bound() { + let handle = StorableSessionHandle { + account_principal: vec![0x01; Principal::MAX_LENGTH_IN_BYTES], + session_id: 1, + }; + + assert!(handle.to_bytes().len() < 41); + } +} diff --git a/src/internet_identity/src/storage/storable/session_id.rs b/src/internet_identity/src/storage/storable/session_id.rs new file mode 100644 index 0000000000..ccb2b09270 --- /dev/null +++ b/src/internet_identity/src/storage/storable/session_id.rs @@ -0,0 +1 @@ +pub type StorableSessionId = u64; diff --git a/src/internet_identity/src/storage/tests.rs b/src/internet_identity/src/storage/tests.rs index 2ddf55d2f8..964972bdc1 100644 --- a/src/internet_identity/src/storage/tests.rs +++ b/src/internet_identity/src/storage/tests.rs @@ -1,4 +1,5 @@ use crate::archive::{ArchiveData, ArchiveState}; +use crate::browser_key::VerifiedBrowserKeys; use crate::openid::OpenIdCredential; use crate::state::PersistentState; use crate::stats::activity_stats::activity_counter::active_anchor_counter::ActiveAnchorCounter; @@ -9,12 +10,13 @@ use crate::storage::anchor::{Anchor, Device}; use crate::storage::storable::account::StorableAccount; use crate::storage::storable::anchor_application_config::AnchorApplicationConfig; use crate::storage::{AccountReferenceListWrite, AccountReferenceWrite}; -use crate::storage::{Header, StorageError, MAX_ENTRIES}; +use crate::storage::{CreateSessionParams, Header, StorageError, MAX_ENTRIES}; use crate::Storage; use candid::Principal; use ic_stable_structures::{Memory, VectorMemory}; use internet_identity_interface::internet_identity::types::{ - AccountNumber, AnchorNumber, ApplicationNumber, FrontendHostname, Timestamp, + AccountNumber, AnchorNumber, ApplicationNumber, BrowserBrand, BrowserDescription, FormFactor, + FrontendHostname, OperatingSystem, PublicKey, Timestamp, }; use internet_identity_interface::internet_identity::types::{ ArchiveConfig, DeviceProtection, KeyType, Purpose, @@ -65,6 +67,60 @@ fn held_references( } /// One origin's worth of a write, in the shape the gate takes it. +/// The key a browser proves with at `generation`, and the successor it announces. +/// +/// A browser is registered by the key it presents and reached only by the successor it +/// announced, so signing in twice from one browser means presenting `generation` and +/// then `generation + 1` — which is what a real browser does when it rotates. +const SESSION_TEST_ORIGIN: &str = "https://example.com"; + +pub(crate) fn browser_key(seed: u8, generation: u8) -> PublicKey { + let mut key = vec![0u8; 32]; + key[0] = seed; + key[1] = generation; + ByteBuf::from(key) +} + +/// What the browser `seed` names reports about itself. The seed rides in `model`, the +/// description's one free-text field, so entries stay tellable apart. +pub(crate) fn description(seed: u8) -> BrowserDescription { + BrowserDescription { + brand: BrowserBrand::Chrome, + os: OperatingSystem::Macos, + form_factor: FormFactor::Desktop, + model: Some(format!("browser {seed}")), + } +} + +/// A first sign-in from the browser `seed` names. +pub(crate) fn params(anchor_number: AnchorNumber, seed: u8, now: u64) -> CreateSessionParams { + params_at(anchor_number, seed, 0, now) +} + +/// A sign-in from the browser `seed` names, presenting the key it holds after +/// `generation` rotations. +pub(crate) fn params_at( + anchor_number: AnchorNumber, + seed: u8, + generation: u8, + now: u64, +) -> CreateSessionParams { + CreateSessionParams { + anchor_number, + origin: SESSION_TEST_ORIGIN.to_string(), + account_number: None, + browser_keys: VerifiedBrowserKeys::unverified_for_test( + browser_key(seed, generation), + browser_key(seed, generation + 1), + ), + browser_description: description(seed), + valid_till_ns: now + 10_000, + max_idle_ns: None, + read_only: false, + now_ns: now, + } +} + pub(crate) fn write_at( origin: &FrontendHostname, account_references: Vec, @@ -123,10 +179,14 @@ pub(crate) fn application_number_for( // Under some other identity, so the one under test still holds nothing here. That is // a real state — an application exists because *someone* holds something at it — and // it is the only way to arrange it now that an application is never stored empty. - let other_identity = AnchorNumber::MAX; + let other = storage + .allocate_anchor(0) + .expect("an anchor to hold the application with"); + let other_identity = other.anchor_number(); + storage.write(other).expect("writing the other identity"); let (account_references, _) = storage.account_state_for_origin(other_identity, origin); storage - .write_account_state( + .write_account_state_for_testing( other_identity, BTreeMap::from([( origin.clone(), @@ -1412,6 +1472,7 @@ fn test_anchor_storage_migration_round_trip() { Anchor { browsers: vec![], next_browser_id: 0, + session_count: 0, anchor_number: 0, devices: vec![], openid_credentials: vec![], @@ -1446,6 +1507,7 @@ fn test_anchor_storage_migration_round_trip() { Anchor { browsers: vec![], next_browser_id: 0, + session_count: 0, anchor_number: 1, devices: vec![Device { pubkey: ByteBuf::from("recovery_key_pubkey"), @@ -1491,6 +1553,7 @@ fn test_anchor_storage_migration_round_trip() { Anchor { browsers: vec![], next_browser_id: 0, + session_count: 0, anchor_number: 2, devices: vec![Device { pubkey: ByteBuf::from("passkey_pubkey"), @@ -1536,6 +1599,7 @@ fn test_anchor_storage_migration_round_trip() { Anchor { browsers: vec![], next_browser_id: 0, + session_count: 0, anchor_number: 3, devices: vec![Device { pubkey: ByteBuf::from("passkey_no_origin"), @@ -1581,6 +1645,7 @@ fn test_anchor_storage_migration_round_trip() { Anchor { browsers: vec![], next_browser_id: 0, + session_count: 0, anchor_number: 4, devices: vec![Device { pubkey: ByteBuf::from("recovery_passkey"), @@ -1626,6 +1691,7 @@ fn test_anchor_storage_migration_round_trip() { Anchor { browsers: vec![], next_browser_id: 0, + session_count: 0, anchor_number: 5, devices: vec![Device { pubkey: ByteBuf::from("recovery_passkey_no_origin"), @@ -1671,6 +1737,7 @@ fn test_anchor_storage_migration_round_trip() { Anchor { browsers: vec![], next_browser_id: 0, + session_count: 0, anchor_number: 6, devices: vec![Device { pubkey: ByteBuf::from("browser_storage_key_auth"), @@ -1716,6 +1783,7 @@ fn test_anchor_storage_migration_round_trip() { Anchor { browsers: vec![], next_browser_id: 0, + session_count: 0, anchor_number: 7, devices: vec![Device { pubkey: ByteBuf::from("browser_storage_key_recovery"), @@ -1775,6 +1843,7 @@ fn test_anchor_storage_migration_round_trip() { Anchor { browsers: vec![], next_browser_id: 0, + session_count: 0, anchor_number: 8, devices: vec![ Device { @@ -1821,6 +1890,7 @@ fn test_anchor_storage_migration_round_trip() { Anchor { browsers: vec![], next_browser_id: 0, + session_count: 0, anchor_number: 9, devices: vec![], openid_credentials: vec![openid_credential(1)], @@ -1842,6 +1912,7 @@ fn test_anchor_storage_migration_round_trip() { Anchor { browsers: vec![], next_browser_id: 0, + session_count: 0, anchor_number: 10, devices: vec![], openid_credentials: vec![], @@ -1876,6 +1947,7 @@ fn test_anchor_storage_migration_round_trip() { Anchor { browsers: vec![], next_browser_id: 0, + session_count: 0, anchor_number: 11, devices: vec![Device { pubkey: ByteBuf::from("unknown_keytype_passkey"), @@ -1928,6 +2000,7 @@ fn test_anchor_storage_migration_round_trip() { Anchor { browsers: vec![], next_browser_id: 0, + session_count: 0, anchor_number: 12, devices: vec![Device { pubkey: ByteBuf::from("device_with_metadata"), @@ -1967,6 +2040,7 @@ fn test_anchor_storage_migration_round_trip() { Anchor { browsers: vec![], next_browser_id: 0, + session_count: 0, anchor_number: 13, devices: vec![], openid_credentials: vec![], @@ -2001,6 +2075,7 @@ fn test_anchor_storage_migration_round_trip() { Anchor { browsers: vec![], next_browser_id: 0, + session_count: 0, anchor_number: 14, devices: vec![Device { pubkey: ByteBuf::from("protected_recovery_key"), @@ -2049,6 +2124,7 @@ fn test_anchor_storage_migration_round_trip() { Anchor { browsers: vec![], next_browser_id: 0, + session_count: 0, anchor_number: 15, devices: vec![Device { pubkey: ByteBuf::from("protected_passkey"), @@ -2096,6 +2172,7 @@ fn test_anchor_storage_migration_round_trip() { Anchor { browsers: vec![], next_browser_id: 0, + session_count: 0, anchor_number: 16, devices: vec![Device { pubkey: ByteBuf::from("unusual_device"), @@ -2141,6 +2218,7 @@ fn test_anchor_storage_migration_round_trip() { Anchor { browsers: vec![], next_browser_id: 0, + session_count: 0, anchor_number: 17, devices: vec![Device { pubkey: ByteBuf::from("recovery_phrase_custom_alias"), @@ -2186,6 +2264,7 @@ fn test_anchor_storage_migration_round_trip() { Anchor { browsers: vec![], next_browser_id: 0, + session_count: 0, anchor_number: 18, devices: vec![Device { pubkey: ByteBuf::from("platform_passkey"), @@ -2231,6 +2310,7 @@ fn test_anchor_storage_migration_round_trip() { Anchor { browsers: vec![], next_browser_id: 0, + session_count: 0, anchor_number: 19, devices: vec![Device { pubkey: ByteBuf::from("unknown_keytype_passkey_2"), @@ -2296,6 +2376,24 @@ mod reference_list_write_path_tests { (storage, anchor_number) } + /// The write path takes the identity, not its number, so there is nothing to write + /// against when the identity does not exist. Writing anyway would leave counters, + /// account reference lists and an application keyed on an owner that never existed, + /// and nothing would ever prune them. + #[test] + fn an_account_write_for_an_identity_that_does_not_exist_is_refused() { + let (mut storage, anchor_number) = storage_with_anchor(); + let never_allocated = anchor_number + 1; + let origin = "https://example.com".to_string(); + + let result = storage.create_account(never_allocated, origin.clone(), "named".to_string()); + + assert!(matches!(result, Err(StorageError::BadAnchorNumber(_)))); + // Refused before anything was written, the application included. + assert_eq!(storage.lookup_application_number_with_origin(&origin), None); + assert_eq!(storage.get_total_application_count(), 0); + } + /// Everything a write derives, rebuilt from the account reference lists alone. /// /// A gate whose job is deriving values is only as good as a check that does the @@ -2329,7 +2427,7 @@ mod reference_list_write_path_tests { let state = storage.account_state_for_origin(anchor_number, &origin); storage - .write_account_state( + .write_account_state_for_testing( anchor_number, BTreeMap::from([(origin.clone(), Some(state))]), ) @@ -2355,7 +2453,7 @@ mod reference_list_write_path_tests { // for and no counter would ever retire. let state = storage.account_state_for_origin(anchor_number, &origin); storage - .write_account_state( + .write_account_state_for_testing( anchor_number, BTreeMap::from([(origin.clone(), Some(state))]), ) @@ -2403,7 +2501,7 @@ mod reference_list_write_path_tests { // A record on an account reference with no number is an account being named. The // caller never states the number; it comes back where the account reference was. let written = storage - .write_account_state( + .write_account_state_for_testing( anchor_number, BTreeMap::from([( origin.clone(), @@ -2444,7 +2542,7 @@ mod reference_list_write_path_tests { // says so — derived from the write, because the caller could not have named a // number that did not exist yet. let written = storage - .write_account_state( + .write_account_state_for_testing( anchor_number, write_at_with_record( &origin, @@ -2508,7 +2606,8 @@ mod reference_list_write_path_tests { // A tombstone, which nothing may create yet. An empty list holds no account // reference with a number, which is also true of a list holding only the derived // default — so this has to be told apart from the write that stores nothing. - let result = storage.write_account_state(anchor_number, write_at(&origin, vec![], None)); + let result = + storage.write_account_state_for_testing(anchor_number, write_at(&origin, vec![], None)); assert!(matches!( result, @@ -2541,7 +2640,7 @@ mod reference_list_write_path_tests { // Refused: nothing may write a tombstone. writes.extend(write_at(&bad, vec![], None)); - let result = storage.write_account_state(anchor_number, writes); + let result = storage.write_account_state_for_testing(anchor_number, writes); assert!(result.is_err()); // Not one of them half-happened: no application, no account number spent, no @@ -2576,7 +2675,9 @@ mod reference_list_write_path_tests { )); } - storage.write_account_state(anchor_number, writes).unwrap(); + storage + .write_account_state_for_testing(anchor_number, writes) + .unwrap(); // Two accounts, two account references, and two distinct numbers: the counters // shared by both origins are folded across the call rather than each computed @@ -2611,7 +2712,7 @@ mod reference_list_write_path_tests { let next_application_number = *storage.next_application_number_memory.get(); let allocator = storage.stable_account_counter_memory.get().stored_accounts; - let result = storage.write_account_state( + let result = storage.write_account_state_for_testing( anchor_number, write_at(&origin, vec![AccountReference::new(None, None)], None), ); @@ -2643,7 +2744,7 @@ mod reference_list_write_path_tests { ] { let state = storage.account_state_for_origin(anchor_number, &origin); storage - .write_account_state( + .write_account_state_for_testing( anchor_number, BTreeMap::from([(origin.clone(), Some(state))]), ) @@ -2702,7 +2803,7 @@ mod reference_list_write_path_tests { // A list with no default reference: the default was removed, so there is nothing // for a rename to name. storage - .write_account_state( + .write_account_state_for_testing( anchor_number, write_at(&origin, vec![AccountReference::new(Some(1), None)], None), ) @@ -2737,7 +2838,7 @@ mod reference_list_write_path_tests { let default_reference = AccountReference::new(None, None); let named_reference = AccountReference::new(Some(1), None); storage - .write_account_state( + .write_account_state_for_testing( anchor_number, write_at( &origin, @@ -2751,7 +2852,7 @@ mod reference_list_write_path_tests { // anchor counter no longer knows about, so dropping one under-runs it. storage.set_counters_for_testing(anchor_number, 0, 0); - let result = storage.write_account_state( + let result = storage.write_account_state_for_testing( anchor_number, write_at(&origin, vec![default_reference], None), ); @@ -2782,7 +2883,7 @@ mod reference_list_write_path_tests { let default_reference = AccountReference::new(None, None); let named_reference = AccountReference::new(Some(1), None); storage - .write_account_state( + .write_account_state_for_testing( anchor_number, write_at( &origin, @@ -2796,7 +2897,7 @@ mod reference_list_write_path_tests { // Dropping the tracked default takes a reference without taking a named // account, so the two deltas differ: 0 and -1. Only the reference count can // under-run here, and the refusal has to name that one rather than the other. - let result = storage.write_account_state( + let result = storage.write_account_state_for_testing( anchor_number, write_at(&origin, vec![named_reference], None), ); @@ -2821,7 +2922,8 @@ mod reference_list_write_path_tests { let origin = "https://example.com".to_string(); let application_number = application_number_for(&mut storage, &origin); - let result = storage.write_account_state(anchor_number, write_at(&origin, vec![], None)); + let result = + storage.write_account_state_for_testing(anchor_number, write_at(&origin, vec![], None)); assert!(matches!( result, @@ -2846,7 +2948,7 @@ mod reference_list_write_path_tests { .stable_application_memory .remove(&application_number); - let result = storage.write_account_state( + let result = storage.write_account_state_for_testing( anchor_number, write_at(&origin, vec![AccountReference::new(Some(1), None)], None), ); @@ -2868,7 +2970,10 @@ mod reference_list_write_path_tests { let application_number = application_number_for(&mut storage, &origin); let references = vec![AccountReference::new(Some(1), None)]; storage - .write_account_state(anchor_number, write_at(&origin, references.clone(), None)) + .write_account_state_for_testing( + anchor_number, + write_at(&origin, references.clone(), None), + ) .unwrap(); // Retiring the application makes a write visible: the write path refuses // without one, so a write that still went through it could not succeed here. @@ -2877,7 +2982,10 @@ mod reference_list_write_path_tests { .remove(&application_number); storage - .write_account_state(anchor_number, write_at(&origin, references.clone(), None)) + .write_account_state_for_testing( + anchor_number, + write_at(&origin, references.clone(), None), + ) .unwrap(); assert_eq!( @@ -2892,7 +3000,7 @@ mod reference_list_write_path_tests { let origin = "https://example.com".to_string(); storage - .write_account_state( + .write_account_state_for_testing( anchor_number, write_at( &origin, @@ -2927,13 +3035,13 @@ mod reference_list_write_path_tests { let origin = "https://example.com".to_string(); storage - .write_account_state( + .write_account_state_for_testing( anchor_number, write_at(&origin, vec![AccountReference::new(None, None)], None), ) .unwrap(); storage - .write_account_state( + .write_account_state_for_testing( anchor_number, write_at(&origin, vec![AccountReference::new(Some(3), None)], None), ) @@ -2955,12 +3063,15 @@ mod reference_list_write_path_tests { let references = vec![AccountReference::new(Some(1), None)]; storage - .write_account_state(anchor_number, write_at(&origin, references.clone(), None)) + .write_account_state_for_testing( + anchor_number, + write_at(&origin, references.clone(), None), + ) .unwrap(); let after_first_write = storage.get_account_counter(anchor_number); storage - .write_account_state( + .write_account_state_for_testing( anchor_number, write_at( &origin, @@ -3122,7 +3233,7 @@ mod account_reference_state_tests { // Drop just the default reference, as moving it away would. storage - .write_account_state( + .write_account_state_for_testing( anchor_number, write_at( &origin, @@ -3253,9 +3364,10 @@ mod account_reference_state_tests { // The write says what the identity holds afterwards, and this one no longer holds // the account the default names. No caller says anything about the default: the // point is that it moves without being told to. + let anchor = storage.read(anchor_number).unwrap(); storage .write_account_state( - anchor_number, + anchor, BTreeMap::from([( origin.clone(), Some(( @@ -3664,7 +3776,7 @@ mod default_account_tracking_tests { let origin = "https://example.com".to_string(); let application_number = application_number_for(&mut storage, &origin); storage - .write_account_state( + .write_account_state_for_testing( anchor_number, write_at(&origin, vec![AccountReference::new(Some(9), None)], None), ) @@ -3733,6 +3845,7 @@ mod default_account_tracking_tests { mod tracked_default_eviction_tests { use super::application_number_for; use super::held_references; + use super::params; use super::record_use; use super::remove_at; use super::write_at; @@ -3766,6 +3879,32 @@ mod tracked_default_eviction_tests { record_use(storage, anchor_number, origin_of(index), None, index + 1).unwrap(); } + /// Eviction on the sign-in path, which every other test here reaches through a + /// one-origin write instead. A sign-in that hands the gate every origin it holds + /// makes each of them a written origin, and a written origin is never a candidate for + /// its own eviction — so nothing is ever evicted by the one write that creates the + /// tracked defaults eviction exists to bound. + #[test] + fn a_sign_in_evicts_the_stale_defaults_too() { + let (mut storage, anchor_number) = storage_with_anchor(); + + // The cap reached by other origins, so the sign-in below is the write that has to + // make room rather than the list being made room for. + for index in 0..MAX_EVICTABLE_DEFAULT_ACCOUNTS { + sign_in_at(&mut storage, anchor_number, index); + } + + let mut params = params(anchor_number, 1, 1_000); + params.origin = origin_of(MAX_EVICTABLE_DEFAULT_ACCOUNTS); + storage.create_session(params).unwrap(); + + // Down to the watermark, and then the origin that triggered the pass on top of it. + assert_eq!( + storage.evictable_default_lists(anchor_number).len() as u64, + EVICTABLE_DEFAULT_ACCOUNTS_WATERMARK + 1 + ); + } + #[test] fn evicting_drops_the_least_recently_used_down_to_the_watermark() { let (mut storage, anchor_number) = storage_with_anchor(); @@ -4008,7 +4147,7 @@ mod tracked_default_eviction_tests { .expect("the writes above created the application"); storage - .write_account_state(anchor_number, remove_at(&origin)) + .write_account_state_for_testing(anchor_number, remove_at(&origin)) .unwrap(); assert!(storage @@ -4049,7 +4188,7 @@ mod tracked_default_eviction_tests { .unwrap(); storage - .write_account_state(anchor_number, remove_at(&origin)) + .write_account_state_for_testing(anchor_number, remove_at(&origin)) .unwrap(); record_use(&mut storage, anchor_number, origin.clone(), None, 2_000).unwrap(); @@ -4075,7 +4214,7 @@ mod tracked_default_eviction_tests { let _application_number = application_number_for(&mut storage, &origin); storage - .write_account_state(anchor_number, remove_at(&origin)) + .write_account_state_for_testing(anchor_number, remove_at(&origin)) .unwrap(); assert_eq!( @@ -4152,7 +4291,7 @@ mod tracked_default_eviction_tests { let origin = "https://example.com".to_string(); let _application_number = application_number_for(&mut storage, &origin); storage - .write_account_state( + .write_account_state_for_testing( anchor_number, write_at(&origin, vec![AccountReference::new(Some(1), None)], None), ) @@ -4198,7 +4337,7 @@ mod application_removal_tests { .unwrap(); storage - .write_account_state(anchor_number, remove_at(&origin)) + .write_account_state_for_testing(anchor_number, remove_at(&origin)) .unwrap(); assert!(storage @@ -4235,7 +4374,7 @@ mod application_removal_tests { plant_tombstone(&mut storage, other_anchor_number, application_number); storage - .write_account_state(anchor_number, remove_at(&origin)) + .write_account_state_for_testing(anchor_number, remove_at(&origin)) .unwrap(); assert_eq!( @@ -4268,7 +4407,7 @@ mod application_removal_tests { // The move back: the tombstoned list gains a reference again. storage - .write_account_state( + .write_account_state_for_testing( anchor_number, write_at( &origin, @@ -4286,7 +4425,7 @@ mod application_removal_tests { ); storage - .write_account_state(anchor_number, remove_at(&origin)) + .write_account_state_for_testing(anchor_number, remove_at(&origin)) .unwrap(); assert!(storage @@ -4349,7 +4488,7 @@ mod application_removal_tests { .unwrap(); storage - .write_account_state(anchor_number, remove_at(&origin)) + .write_account_state_for_testing(anchor_number, remove_at(&origin)) .unwrap(); assert_eq!( @@ -4395,7 +4534,7 @@ mod application_removal_tests { .unwrap(); storage - .write_account_state(anchor_number, remove_at(&removed_origin)) + .write_account_state_for_testing(anchor_number, remove_at(&removed_origin)) .unwrap(); record_use( &mut storage, @@ -4426,7 +4565,7 @@ mod application_removal_tests { .lookup_application_number_with_origin(&origin) .unwrap(); storage - .write_account_state(anchor_number, remove_at(&origin)) + .write_account_state_for_testing(anchor_number, remove_at(&origin)) .unwrap(); record_use(&mut storage, anchor_number, origin.clone(), None, 2_000).unwrap(); @@ -4452,7 +4591,7 @@ mod application_removal_tests { .lookup_application_number_with_origin(&origin) .unwrap(); storage - .write_account_state( + .write_account_state_for_testing( other_anchor_number, write_at( &origin, @@ -4463,7 +4602,7 @@ mod application_removal_tests { .unwrap(); storage - .write_account_state(other_anchor_number, remove_at(&origin)) + .write_account_state_for_testing(other_anchor_number, remove_at(&origin)) .unwrap(); assert_eq!( @@ -4489,7 +4628,7 @@ mod application_removal_tests { .unwrap(); storage - .write_account_state(anchor_number, remove_at(&origin)) + .write_account_state_for_testing(anchor_number, remove_at(&origin)) .unwrap(); assert_eq!( @@ -4511,7 +4650,7 @@ mod application_removal_tests { ); storage - .write_account_state(anchor_number, remove_at(&origin)) + .write_account_state_for_testing(anchor_number, remove_at(&origin)) .unwrap(); assert_eq!( @@ -4539,7 +4678,7 @@ mod application_removal_tests { .is_some()); storage - .write_account_state(anchor_number, remove_at(&origin)) + .write_account_state_for_testing(anchor_number, remove_at(&origin)) .unwrap(); assert!(storage @@ -4565,7 +4704,7 @@ mod application_removal_tests { .insert(StorableOriginSha256::from_origin(&origin), reallocated); storage - .write_account_state(anchor_number, remove_at(&origin)) + .write_account_state_for_testing(anchor_number, remove_at(&origin)) .unwrap(); assert_eq!( @@ -4755,7 +4894,7 @@ mod account_principal_index_tests { .unwrap(); storage - .write_account_state(anchor_number, remove_at(&origin)) + .write_account_state_for_testing(anchor_number, remove_at(&origin)) .unwrap(); assert_eq!( @@ -4787,7 +4926,7 @@ mod account_principal_index_tests { // One call holding every victim, the way eviction does it. storage - .write_account_state( + .write_account_state_for_testing( anchor_number, (0..5) .map(|index| (format!("https://dapp-{index}.com"), None)) @@ -4814,7 +4953,7 @@ mod account_principal_index_tests { // derives principals for it, so the write cannot be completed without one. let origin = "https://example.com".to_string(); - let result = storage.write_account_state( + let result = storage.write_account_state_for_testing( anchor_number, write_at(&origin, vec![AccountReference::new(Some(1), Some(1))], None), ); @@ -4844,7 +4983,7 @@ mod account_principal_index_tests { ); storage - .write_account_state(anchor_number, remove_at(&origin)) + .write_account_state_for_testing(anchor_number, remove_at(&origin)) .unwrap(); assert_eq!( @@ -4859,9 +4998,11 @@ mod account_principal_index_tests { } mod session_tests { + use super::record_use; use super::{application_number_for, write_at}; use crate::storage::account::{AccountReference, Session}; use crate::storage::storable::account_reference::StorableAccountReference; + use crate::storage::MAX_EVICTABLE_DEFAULT_ACCOUNTS; use crate::{Storage, DAY_NS, MINUTE_NS}; use ic_stable_structures::{Storable, VectorMemory}; use internet_identity_interface::internet_identity::types::AnchorNumber; @@ -4871,7 +5012,7 @@ mod session_tests { /// what the tests about the absolute bound want. const NEVER_IDLE: u64 = u64::MAX; - fn session(created_at_ns: u64, valid_till_ns: u64) -> Session { + fn session(session_id: u64, created_at_ns: u64, valid_till_ns: u64) -> Session { Session { created_at_ns, valid_till_ns, @@ -4879,6 +5020,7 @@ mod session_tests { last_refreshed_ns: None, browser_id: 1, read_only: false, + session_id, } } @@ -4907,14 +5049,16 @@ mod session_tests { last_refreshed_ns: Some(44), browser_id: 55, read_only: false, + session_id: 66, }, Session { - created_at_ns: 66, - valid_till_ns: 77, - max_idle_ns: 88, + created_at_ns: 77, + valid_till_ns: 88, + max_idle_ns: 99, last_refreshed_ns: None, - browser_id: 99, + browser_id: 111, read_only: true, + session_id: 122, }, ], }; @@ -4928,7 +5072,7 @@ mod session_tests { #[test] fn a_bound_further_out_than_the_session_never_bites() { - let record = session(0, DAY_NS); + let record = session(1, 0, DAY_NS); assert!(!record.is_expired_or_idle(0)); // Past its own lifetime, so over on the other bound — which is the point: @@ -4941,7 +5085,7 @@ mod session_tests { let record = Session { max_idle_ns: 30 * MINUTE_NS, last_refreshed_ns: Some(10 * MINUTE_NS), - ..session(0, DAY_NS) + ..session(1, 0, DAY_NS) }; assert!(!record.is_expired_or_idle(39 * MINUTE_NS)); @@ -4954,7 +5098,7 @@ mod session_tests { let record = Session { max_idle_ns: 30 * MINUTE_NS, last_refreshed_ns: None, - ..session(5 * MINUTE_NS, DAY_NS) + ..session(1, 5 * MINUTE_NS, DAY_NS) }; // Otherwise a session abandoned straight after sign-in would sit unbounded @@ -4995,14 +5139,14 @@ mod session_tests { let origin = "https://has-a-session.com".to_string(); let _application_number = application_number_for(&mut storage, &origin); storage - .write_account_state( + .write_account_state_for_testing( anchor_number, write_at( &origin, vec![AccountReference { account_number: None, last_used: Some(1), - sessions: vec![session(0, u64::MAX)], + sessions: vec![session(0, 0, u64::MAX)], }], None, ), @@ -5012,17 +5156,68 @@ mod session_tests { assert_eq!(storage.evictable_default_lists(anchor_number).len(), 1); } + /// Eviction orders on the list's `last_used`, which every refresh stamps, so a session + /// in use keeps its list at the newest end and survives the cap on its own. + #[test] + fn a_refreshed_session_keeps_its_list_and_a_stale_one_does_not() { + let (mut storage, anchor_number) = storage_with_anchor(); + let stale = "https://never-came-back.com".to_string(); + let refreshed = "https://still-in-use.com".to_string(); + for (origin, last_used) in [(&stale, 1), (&refreshed, u64::MAX)] { + storage + .write_account_state_for_testing( + anchor_number, + write_at( + origin, + vec![AccountReference { + account_number: None, + last_used: Some(last_used), + sessions: vec![session(1, 1, u64::MAX)], + }], + None, + ), + ) + .unwrap(); + } + let stale_application = storage + .lookup_application_number_with_origin(&stale) + .unwrap(); + let refreshed_application = storage + .lookup_application_number_with_origin(&refreshed) + .unwrap(); + + for index in 0..MAX_EVICTABLE_DEFAULT_ACCOUNTS { + record_use( + &mut storage, + anchor_number, + format!("https://app-{index}.com"), + None, + index + 2, + ) + .unwrap(); + } + + assert_eq!( + storage.stored_account_references(anchor_number, stale_application), + None + ); + assert_ne!( + storage.stored_account_references(anchor_number, refreshed_application), + None + ); + } + #[test] fn a_session_over_by_idleness_reclaims_like_a_dead_one() { let now = 100 * DAY_NS; let idle = Session { max_idle_ns: DAY_NS, last_refreshed_ns: Some(now - 10 * DAY_NS), - ..session(now - 20 * DAY_NS, now + DAY_NS) + ..session(1, now - 20 * DAY_NS, now + DAY_NS) }; let live = Session { last_refreshed_ns: Some(now - 1), - ..session(now - 20 * DAY_NS, now + DAY_NS) + ..session(2, now - 20 * DAY_NS, now + DAY_NS) }; // Both are inside their lifetime, so ranking on that alone would have them @@ -5033,13 +5228,13 @@ mod session_tests { #[test] fn reclaim_order_ranks_dead_sessions_first() { let now = 1_000; - let expired = session(1, 500); + let expired = session(1, 1, 500); let live = Session { max_idle_ns: NEVER_IDLE, last_refreshed_ns: Some(900), - ..session(400, 10_000) + ..session(2, 400, 10_000) }; - let live_untouched = session(400, 10_000); + let live_untouched = session(3, 400, 10_000); assert!(expired.reclaim_sort_key(now) < live.reclaim_sort_key(now)); assert!(expired.reclaim_sort_key(now) < live_untouched.reclaim_sort_key(now)); @@ -5051,14 +5246,14 @@ mod session_tests { let held = Session { max_idle_ns: NEVER_IDLE, last_refreshed_ns: Some(now - DAY_NS), - ..session(now - 20 * DAY_NS, now + DAY_NS) + ..session(501, now - 20 * DAY_NS, now + DAY_NS) }; // Created after the session it would have to outrank, which under a plain recency // order would protect it. let flood: Vec = (0..500) .map(|index| Session { browser_id: index, - ..session(now - 1, now + DAY_NS) + ..session(index as u64 + 1, now - 1, now + DAY_NS) }) .collect(); @@ -5074,13 +5269,13 @@ mod session_tests { let weekly = Session { max_idle_ns: NEVER_IDLE, last_refreshed_ns: Some(now - 3 * DAY_NS), - ..session(now - 90 * DAY_NS, now + DAY_NS) + ..session(1, now - 90 * DAY_NS, now + DAY_NS) }; // Signed in yesterday, used for five minutes, never opened again. let one_sitting = Session { max_idle_ns: NEVER_IDLE, last_refreshed_ns: Some(now - DAY_NS + 5 * MINUTE_NS), - ..session(now - DAY_NS, now + DAY_NS) + ..session(2, now - DAY_NS, now + DAY_NS) }; assert!( @@ -5089,3 +5284,851 @@ mod session_tests { ); } } + +mod session_creation_tests { + use super::held_references; + use super::{params, params_at}; + use crate::delegation::calculate_session_seed_with_salt; + use crate::storage::account::{Session, DEFAULT_SESSION_IDLE_NS, MIN_SESSION_IDLE_NS}; + use crate::storage::anchor::MAX_BROWSERS; + use crate::storage::CreateSessionParams; + use crate::storage::StorageError; + use crate::{Storage, DAY_NS, MINUTE_NS}; + use candid::Principal; + use ic_stable_structures::VectorMemory; + use internet_identity_interface::internet_identity::types::AnchorNumber; + use pretty_assertions::assert_eq; + + const SALT: [u8; 32] = [17u8; 32]; + const ORIGIN: &str = "https://example.com"; + + fn storage_with_anchor() -> (Storage, AnchorNumber) { + let mut storage = Storage::new((10_000, 3_784_873), VectorMemory::default()); + storage.update_salt(SALT); + let anchor = storage.allocate_anchor(0).unwrap(); + let anchor_number = anchor.anchor_number(); + storage.write(anchor).unwrap(); + (storage, anchor_number) + } + + /// A replaced session stops resolving, which nothing else here observes: creation + /// puts a principal in the index and revocation takes it out, and both are asked + /// about elsewhere, but a sign-in that supersedes a session removes the old principal + /// while inserting the new one in the same write. A superseded principal left behind + /// would still resolve to a record no list holds. + #[test] + fn replacing_a_session_takes_its_principal_out_of_the_index() { + let (mut storage, anchor_number) = storage_with_anchor(); + + storage + .create_session(params(anchor_number, 7, 1_000)) + .unwrap(); + let superseded: Vec = storage + .lookup_session_with_principal_memory + .iter() + .map(|(principal, _)| principal) + .collect(); + assert_eq!(superseded.len(), 1); + + // The same browser, the same origin, presenting the successor it announced: this + // replaces the session rather than adding one. + storage + .create_session(params_at(anchor_number, 7, 1, 2_000)) + .unwrap(); + + let held: Vec = storage + .lookup_session_with_principal_memory + .iter() + .map(|(principal, _)| principal) + .collect(); + assert_eq!(held.len(), 1, "one session, so one principal: {held:?}"); + assert!( + !held.contains(&superseded[0]), + "the superseded session's principal still resolves: {held:?}" + ); + } + + /// A browser the registry gave up takes its sessions with it, wherever they were, in + /// the write that made room for the browser replacing it. + /// + /// One write, not a loop at the caller: registering the browser, storing the anchor, + /// then one `revoke_browser_sessions` per browser dropped would end a browser in one + /// write and its sessions in others — and on the IC an `Err` from a later one commits + /// the earlier ones, so a browser could be left gone with its sessions still live. + #[test] + fn a_dropped_browser_takes_its_sessions_with_it() { + let (mut storage, anchor_number) = storage_with_anchor(); + let elsewhere = "https://elsewhere.example".to_string(); + + // The browser that will be given up, holding a session at each of two origins, so + // this also shows the sweep is not limited to the origin being written. Its second + // sign-in presents the successor it announced at its first. + storage + .create_session(params(anchor_number, 7, 1_000)) + .unwrap(); + storage + .create_session(CreateSessionParams { + origin: elsewhere.clone(), + ..params_at(anchor_number, 7, 1, 1_000) + }) + .unwrap(); + assert_eq!(storage.read(anchor_number).unwrap().session_count, 2); + + // Fill the registry, so the browser above is the least recently used when the one + // that does not fit arrives. Nothing here says which browser is given up, or that + // its sessions go with it: the write works both out. + for index in 0..MAX_BROWSERS { + storage + .create_session(params(anchor_number, 100 + index as u8, 2_000)) + .unwrap(); + } + + let held: Vec = storage + .account_state(anchor_number) + .into_values() + .flatten() + .flat_map(|(account_references, _)| account_references) + .flat_map(|write| write.account_reference.sessions) + .map(|session| session.browser_id) + .collect(); + assert!( + !held.contains(&0), + "the dropped browser's sessions outlived it: {held:?}" + ); + assert_eq!( + held.len(), + MAX_BROWSERS, + "one session each for the browsers still registered" + ); + assert_eq!( + storage.read(anchor_number).unwrap().session_count as usize, + MAX_BROWSERS, + "and the count followed in the same write" + ); + } + + /// A list that predates the principal index, which is every list an existing user has: + /// the index is written only where a list's set of account numbers changes. Emptied + /// here to stand in for one of those lists. + fn forget_account_principals(storage: &mut Storage) { + let principals: Vec<_> = storage + .lookup_account_with_principal_memory + .iter() + .map(|(principal, _)| principal) + .collect(); + for principal in principals { + storage + .lookup_account_with_principal_memory + .remove(&principal); + } + } + + /// Creating a session indexes its own account. A session handle names its account by + /// principal, so a handle whose account is not indexed resolves to nothing — which + /// would be every returning user, presenting as "sign in again". + #[test] + fn creating_a_session_indexes_the_account_it_belongs_to() { + let (mut storage, anchor_number) = storage_with_anchor(); + storage + .create_session(params(anchor_number, 1, 1_000)) + .unwrap(); + forget_account_principals(&mut storage); + + storage + .create_session(params(anchor_number, 2, 2_000)) + .unwrap(); + + let application_number = storage + .lookup_application_number_with_origin(&ORIGIN.to_string()) + .unwrap(); + assert_eq!( + storage + .lookup_account_with_principal_memory + .iter() + .map(|(_, stored)| ( + stored.anchor_number, + stored.application_number, + stored.account_number + )) + .collect::>(), + vec![(anchor_number, application_number, None)] + ); + } + + fn sessions_of(storage: &Storage, anchor_number: AnchorNumber) -> Vec { + let application_number = storage + .lookup_application_number_with_origin(&ORIGIN.to_string()) + .unwrap(); + held_references(storage, anchor_number, application_number) + .into_iter() + .find(|reference| reference.account_number.is_none()) + .unwrap() + .sessions + } + + #[test] + fn an_idle_bound_is_kept_as_asked_for_when_it_is_in_range() { + let (mut storage, anchor_number) = storage_with_anchor(); + let asked = 20 * MINUTE_NS; + + let session = storage + .create_session(CreateSessionParams { + max_idle_ns: Some(asked), + valid_till_ns: DAY_NS, + ..params(anchor_number, 1, 0) + }) + .unwrap() + .1; + + assert_eq!(session.max_idle_ns, asked); + } + + #[test] + fn an_idle_bound_below_the_floor_is_raised_to_it() { + let (mut storage, anchor_number) = storage_with_anchor(); + + let session = storage + .create_session(CreateSessionParams { + max_idle_ns: Some(MINUTE_NS), + valid_till_ns: DAY_NS, + ..params(anchor_number, 1, 0) + }) + .unwrap() + .1; + + // An app delegation lasts five minutes, so a bound under that would end a + // session between two mints of one that is plainly in use. + assert_eq!(session.max_idle_ns, MIN_SESSION_IDLE_NS); + } + + #[test] + fn an_idle_bound_longer_than_the_session_is_cut_to_it() { + let (mut storage, anchor_number) = storage_with_anchor(); + + let session = storage + .create_session(CreateSessionParams { + max_idle_ns: Some(400 * DAY_NS), + valid_till_ns: DAY_NS, + ..params(anchor_number, 1, 0) + }) + .unwrap() + .1; + + // A bound it could never reach says something about the session that is not + // true, so it is stored as the life the session actually got. + assert_eq!(session.max_idle_ns, DAY_NS); + } + + #[test] + fn asking_for_no_idle_bound_gets_the_default() { + let (mut storage, anchor_number) = storage_with_anchor(); + + let session = storage + .create_session(CreateSessionParams { + valid_till_ns: 30 * DAY_NS, + ..params(anchor_number, 1, 0) + }) + .unwrap() + .1; + + // Every session gets a bound now. A week of nobody touching the application + // ends the sign-in, well inside the thirty days it could otherwise live. + assert_eq!(session.max_idle_ns, DEFAULT_SESSION_IDLE_NS); + assert!(!session.is_expired_or_idle(6 * DAY_NS)); + assert!(session.is_expired_or_idle(7 * DAY_NS)); + } + + #[test] + fn a_session_shorter_than_the_idle_floor_is_bounded_by_its_own_life() { + let (mut storage, anchor_number) = storage_with_anchor(); + + // Under the floor the range inverts, and clamping in one call would trap. + let session = storage + .create_session(CreateSessionParams { + valid_till_ns: MINUTE_NS, + max_idle_ns: Some(30 * MINUTE_NS), + ..params(anchor_number, 1, 0) + }) + .unwrap() + .1; + + assert_eq!(session.max_idle_ns, MINUTE_NS); + } + + #[test] + fn creating_a_session_tracks_the_account_and_stores_the_record() { + let (mut storage, anchor_number) = storage_with_anchor(); + + let session = storage + .create_session(params(anchor_number, 1, 1_000)) + .unwrap() + .1; + + assert_eq!(session.created_at_ns, 1_000); + assert_eq!(session.valid_till_ns, 11_000); + assert_eq!(session.last_refreshed_ns, None); + // The registry minted it; the caller presented a key, not an id. + assert_eq!(session.browser_id, 0); + assert_eq!(sessions_of(&storage, anchor_number), vec![session]); + } + + /// The write path is handed the identity record, so it stores it — including what the + /// resolution above changed on it, not only the session count it moves itself. + /// + /// A browser signing in again at an account it already holds a session at *replaces* + /// that session, so the count does not move. A write that stored the record only where + /// the count moved would throw the key rotation away on exactly those sign-ins, leaving + /// a key that is good for one sign-in usable for good — and the next rotation, which + /// presents the successor of a key that was never stored, would be refused. + #[test] + fn a_rotation_survives_a_sign_in_that_moves_no_session_count() { + let (mut storage, anchor_number) = storage_with_anchor(); + + // Registers the browser. One session added, so the count moves. + storage + .create_session(params(anchor_number, 1, 1_000)) + .unwrap(); + // Replaces it: one session out, one in, and the count stays where it was. + storage + .create_session(params_at(anchor_number, 1, 1, 2_000)) + .unwrap(); + + // Only reachable if the rotation the write above performed was stored. + let third = storage + .create_session(params_at(anchor_number, 1, 2, 3_000)) + .expect("the rotation from a count-neutral sign-in was not stored"); + assert_eq!(third.1.browser_id, 0, "still the one registry entry"); + assert_eq!(storage.read(anchor_number).unwrap().browsers().len(), 1); + } + + /// A session whose life has already run out is refused rather than created, because the + /// sweep that prunes dead sessions runs in this same call and would take it straight + /// back out — leaving this returning `Ok` for a session no list holds. + #[test] + fn a_session_that_is_already_over_is_refused() { + let (mut storage, anchor_number) = storage_with_anchor(); + + let mut expired = params(anchor_number, 1, 5_000); + expired.valid_till_ns = 5_000; + + assert!(matches!( + storage.create_session(expired), + Err(StorageError::SessionAlreadyOver { .. }) + )); + assert_eq!(storage.read(anchor_number).unwrap().browsers().len(), 0); + } + + /// A ceremony replaces the browser's session rather than reusing it, so a copy of the + /// old one stops working at the user's next sign-in instead of at its expiry. + #[test] + fn the_same_device_replaces_its_session() { + let (mut storage, anchor_number) = storage_with_anchor(); + let first = storage + .create_session(params(anchor_number, 1, 1_000)) + .unwrap() + .1; + + let again = storage + .create_session(params_at(anchor_number, 1, 1, 5_000)) + .unwrap() + .1; + + assert_eq!( + again.browser_id, first.browser_id, + "the same registry entry" + ); + assert_ne!(again.created_at_ns, first.created_at_ns); + assert_eq!(sessions_of(&storage, anchor_number).len(), 1); + } + + #[test] + fn a_different_device_gets_its_own_session() { + let (mut storage, anchor_number) = storage_with_anchor(); + storage + .create_session(params(anchor_number, 1, 1_000)) + .unwrap(); + + storage + .create_session(params(anchor_number, 2, 1_000)) + .unwrap(); + + assert_eq!(sessions_of(&storage, anchor_number).len(), 2); + } + + #[test] + fn expired_sessions_are_pruned_when_the_list_is_written() { + let (mut storage, anchor_number) = storage_with_anchor(); + for seed in 0..3 { + storage + .create_session(params(anchor_number, seed, 1_000)) + .unwrap(); + } + + let latest = storage + .create_session(params(anchor_number, 9, 20_000)) + .unwrap() + .1; + + let sessions = sessions_of(&storage, anchor_number); + assert_eq!(sessions.len(), 1); + assert_eq!(sessions[0].browser_id, latest.browser_id); + } + + /// There is no per-reference cap: one browser holds one session per account, so the + /// reference is bounded by the browser registry rather than by a number of its own. + #[test] + fn one_reference_holds_one_session_per_browser() { + let (mut storage, anchor_number) = storage_with_anchor(); + for seed in 0..12u8 { + let mut p = params(anchor_number, seed, 1_000); + p.valid_till_ns = 1_000_000; + storage.create_session(p).unwrap(); + } + + let sessions = sessions_of(&storage, anchor_number); + assert_eq!(sessions.len(), 12); + assert!(sessions.iter().any(|s| s.browser_id == 0)); + } + + /// The account-principal index is keyed with one derivation and a session handle names + /// the account with another, so this crosses the two: what `create_session` stored has + /// to resolve back through the index it was derived against. + #[test] + fn a_session_handle_resolves_through_the_account_principal_index() { + let (mut storage, anchor_number) = storage_with_anchor(); + let session = storage + .create_session(params(anchor_number, 7, 1_000)) + .unwrap() + .1; + let application_number = storage + .lookup_application_number_with_origin(&ORIGIN.to_string()) + .unwrap(); + + // The principal the handle names, read back off the handle `create_session` + // wrote, which is the value the crossing is about. + let (_, handle) = storage + .lookup_session_with_principal_memory + .iter() + .next() + .expect("the session index holds the session just created"); + let account_principal = Principal::from_slice(&handle.account_principal); + let locator = storage + .lookup_account_with_principal_memory + .get(&account_principal) + .expect("the account principal index must resolve what create_session derived"); + + assert_eq!(locator.anchor_number, anchor_number); + assert_eq!(locator.application_number, application_number); + assert_eq!(session.browser_id, 0); + } + + #[test] + fn a_named_account_can_hold_its_own_sessions() { + let (mut storage, anchor_number) = storage_with_anchor(); + let named = storage + .create_account(anchor_number, ORIGIN.to_string(), "named".to_string()) + .unwrap(); + let mut p = params(anchor_number, 1, 1_000); + p.account_number = named.account_number; + + storage.create_session(p).unwrap(); + + assert_eq!(sessions_of(&storage, anchor_number).len(), 0); + let application_number = storage + .lookup_application_number_with_origin(&ORIGIN.to_string()) + .unwrap(); + let references = held_references(&storage, anchor_number, application_number); + let named_reference = references + .iter() + .find(|r| r.account_number == named.account_number) + .unwrap(); + assert_eq!(named_reference.sessions.len(), 1); + } + + #[test] + fn a_session_for_an_account_the_anchor_does_not_hold_is_refused() { + let (mut storage, anchor_number) = storage_with_anchor(); + let mut p = params(anchor_number, 1, 1_000); + p.account_number = Some(4_242); + + let result = storage.create_session(p); + + assert!(result.is_err()); + } + + /// The hazard the session id exists for: `time()` is constant across a consensus + /// round, so two records created in one round agree on every field that describes + /// them. If identity came from those fields, the second would sign as the first — + /// and a chain issued against a session that has since been replaced would verify + /// again. + #[test] + fn a_session_replaced_in_the_same_round_does_not_inherit_its_identity() { + let (mut storage, anchor_number) = storage_with_anchor(); + let same_round = |seed, generation| CreateSessionParams { + valid_till_ns: 10_000, + ..params_at(anchor_number, seed, generation, 1_000) + }; + + let first = storage.create_session(same_round(1, 0)).unwrap().1; + // The same browser again, presenting the successor it announced a moment ago. + let replacement = storage.create_session(same_round(1, 1)).unwrap().1; + let sibling = storage.create_session(same_round(2, 0)).unwrap().1; + + assert_eq!(first.created_at_ns, replacement.created_at_ns); + assert_eq!(first.browser_id, replacement.browser_id); + assert_ne!(first.session_id, replacement.session_id); + assert_ne!(replacement.session_id, sibling.session_id); + } + + /// Creating twice from one browser at one account replaces, so there is never a second + /// record to collide with in the same round. + #[test] + fn creating_twice_in_one_round_from_one_browser_yields_one_session() { + let (mut storage, anchor_number) = storage_with_anchor(); + // One browser, rotating as it must, signing in three times in the same round. + let attempt = |generation, read_only| CreateSessionParams { + valid_till_ns: u64::MAX, + read_only, + ..params_at(anchor_number, 1, generation, 1_000) + }; + + let first = storage.create_session(attempt(0, false)).unwrap().1; + storage.create_session(attempt(1, false)).unwrap(); + assert_eq!(sessions_of(&storage, anchor_number).len(), 1); + + let replaced = storage.create_session(attempt(2, true)).unwrap().1; + assert_ne!(replaced.read_only, first.read_only); + assert_eq!(sessions_of(&storage, anchor_number).len(), 1); + } + + #[test] + fn the_session_seed_binds_the_account_and_the_session_id() { + use crate::storage::account::Account; + + let account = Account::new(10_000, ORIGIN.to_string(), None, None); + let account_seed = account.calculate_seed_with_salt(&SALT); + let other_account = Account::new(10_001, ORIGIN.to_string(), None, None); + let other_seed = other_account.calculate_seed_with_salt(&SALT); + + let base = calculate_session_seed_with_salt(&SALT, &account_seed, 1); + + assert_ne!( + base, + calculate_session_seed_with_salt(&SALT, &other_seed, 1) + ); + assert_ne!( + base, + calculate_session_seed_with_salt(&SALT, &account_seed, 2) + ); + assert_ne!( + base, + calculate_session_seed_with_salt(&[18u8; 32], &account_seed, 1) + ); + assert_eq!( + base, + calculate_session_seed_with_salt(&SALT, &account_seed, 1) + ); + } + + #[test] + fn a_session_seed_is_distinct_from_the_account_seed_it_belongs_to() { + use crate::storage::account::Account; + + let account = Account::new(10_000, ORIGIN.to_string(), None, None); + let account_seed = account.calculate_seed_with_salt(&SALT); + let session_seed = calculate_session_seed_with_salt(&SALT, &account_seed, 1); + + assert_ne!(account_seed, session_seed); + } + + /// Naming a default account keeps its principal, so it must keep its sessions too. + #[test] + fn naming_a_default_account_leaves_its_session_identity_unchanged() { + use crate::storage::account::Account; + + let default = Account::new(10_000, ORIGIN.to_string(), None, None); + let before = + calculate_session_seed_with_salt(&SALT, &default.calculate_seed_with_salt(&SALT), 1); + + let named = Account::new_full( + 10_000, + ORIGIN.to_string(), + Some("work".to_string()), + Some(7), + None, + Some(10_000), + ); + let after = + calculate_session_seed_with_salt(&SALT, &named.calculate_seed_with_salt(&SALT), 1); + + assert_eq!(before, after); + } +} + +mod session_consent_change_tests { + use super::held_references; + use super::params_at; + use crate::storage::CreateSessionParams; + use crate::Storage; + use ic_stable_structures::VectorMemory; + use internet_identity_interface::internet_identity::types::AnchorNumber; + use pretty_assertions::assert_eq; + + const ORIGIN: &str = "https://example.com"; + + fn storage_with_anchor() -> (Storage, AnchorNumber) { + let mut storage = Storage::new((10_000, 3_784_873), VectorMemory::default()); + storage.update_salt([17u8; 32]); + let anchor = storage.allocate_anchor(0).unwrap(); + let anchor_number = anchor.anchor_number(); + storage.write(anchor).unwrap(); + (storage, anchor_number) + } + + /// One browser signing in again, presenting the successor it announced last time. + fn create( + storage: &mut Storage, + anchor_number: AnchorNumber, + generation: u8, + read_only: bool, + now: u64, + ) -> u64 { + storage + .create_session(CreateSessionParams { + valid_till_ns: u64::MAX, + read_only, + ..params_at(anchor_number, 1, generation, now) + }) + .unwrap() + .1 + .created_at_ns + } + + fn sessions(storage: &Storage, anchor_number: AnchorNumber) -> Vec { + let application_number = storage + .lookup_application_number_with_origin(&ORIGIN.to_string()) + .unwrap(); + held_references(storage, anchor_number, application_number) + .into_iter() + .find(|reference| reference.account_number.is_none()) + .unwrap() + .sessions + .into_iter() + .map(|session| session.read_only) + .collect() + } + + #[test] + fn the_same_consent_still_replaces_the_session() { + let (mut storage, anchor_number) = storage_with_anchor(); + let first = create(&mut storage, anchor_number, 0, false, 1_000); + + let again = create(&mut storage, anchor_number, 1, false, 2_000); + + assert_ne!(again, first); + assert_eq!(sessions(&storage, anchor_number), vec![false]); + } + + #[test] + fn a_downgraded_consent_replaces_the_session() { + let (mut storage, anchor_number) = storage_with_anchor(); + let full_access = create(&mut storage, anchor_number, 0, false, 1_000); + + let read_only = create(&mut storage, anchor_number, 1, true, 2_000); + + assert_ne!(read_only, full_access); + assert_eq!(sessions(&storage, anchor_number), vec![true]); + } + + #[test] + fn an_upgraded_consent_replaces_the_session() { + let (mut storage, anchor_number) = storage_with_anchor(); + create(&mut storage, anchor_number, 0, true, 1_000); + + create(&mut storage, anchor_number, 1, false, 2_000); + + assert_eq!(sessions(&storage, anchor_number), vec![false]); + } + + #[test] + fn a_consent_change_leaves_another_browser_alone() { + let (mut storage, anchor_number) = storage_with_anchor(); + storage + .create_session(CreateSessionParams { + valid_till_ns: u64::MAX, + ..params_at(anchor_number, 2, 0, 1_000) + }) + .unwrap(); + create(&mut storage, anchor_number, 0, false, 1_000); + + create(&mut storage, anchor_number, 1, true, 2_000); + + let mut held = sessions(&storage, anchor_number); + held.sort_unstable(); + assert_eq!(held, vec![false, true]); + } +} + +mod browser_session_count_tests { + use super::{params, params_at}; + use crate::storage::anchor::MAX_BROWSERS; + use crate::storage::CreateSessionParams; + use crate::Storage; + use ic_stable_structures::VectorMemory; + use internet_identity_interface::internet_identity::types::{AnchorNumber, BrowserId}; + use pretty_assertions::assert_eq; + use std::collections::BTreeMap; + + const SALT: [u8; 32] = [17u8; 32]; + + fn storage_with_anchor() -> (Storage, AnchorNumber) { + let mut storage = Storage::new((10_000, 3_784_873), VectorMemory::default()); + storage.update_salt(SALT); + let anchor = storage.allocate_anchor(0).unwrap(); + let anchor_number = anchor.anchor_number(); + storage.write(anchor).unwrap(); + (storage, anchor_number) + } + + /// What each registered browser says it holds, and what the identity says it holds. + /// Asserted together throughout: the per-browser counts are the same fact as the + /// identity's total at a finer grain, and a test that checked one without the other + /// would pass while they disagreed. + fn counts( + storage: &Storage, + anchor_number: AnchorNumber, + ) -> (BTreeMap, u32) { + let anchor = storage.read(anchor_number).unwrap(); + let per_browser = anchor + .browsers() + .iter() + .map(|browser| (browser.id, browser.session_count)) + .collect(); + (per_browser, anchor.session_count) + } + + #[test] + fn a_session_counts_against_the_browser_it_came_from() { + let (mut storage, anchor_number) = storage_with_anchor(); + + storage + .create_session(params(anchor_number, 7, 1_000)) + .unwrap(); + assert_eq!( + counts(&storage, anchor_number), + (BTreeMap::from([(0, 1)]), 1) + ); + + // A second browser, which is a second entry rather than a second session on the + // first. + storage + .create_session(params(anchor_number, 9, 2_000)) + .unwrap(); + assert_eq!( + counts(&storage, anchor_number), + (BTreeMap::from([(0, 1), (1, 1)]), 2) + ); + + // The first browser signing in again at the same origin, presenting the successor + // it announced. A ceremony replaces what that browser held there rather than + // adding to it, so the counts stand still — which is the case a counter kept by + // incrementing at the call site would get wrong. + storage + .create_session(params_at(anchor_number, 7, 1, 3_000)) + .unwrap(); + assert_eq!( + counts(&storage, anchor_number), + (BTreeMap::from([(0, 1), (1, 1)]), 2) + ); + } + + #[test] + fn sessions_at_several_origins_add_up_on_one_browser() { + let (mut storage, anchor_number) = storage_with_anchor(); + + storage + .create_session(params(anchor_number, 7, 1_000)) + .unwrap(); + storage + .create_session(CreateSessionParams { + origin: "https://elsewhere.example".to_string(), + ..params_at(anchor_number, 7, 1, 2_000) + }) + .unwrap(); + + // One entry, two origins: the count belongs to the browser, not to a list. + assert_eq!( + counts(&storage, anchor_number), + (BTreeMap::from([(0, 2)]), 2) + ); + } + + #[test] + fn revoking_a_browsers_sessions_returns_its_count_to_zero() { + let (mut storage, anchor_number) = storage_with_anchor(); + + storage + .create_session(params(anchor_number, 7, 1_000)) + .unwrap(); + storage + .create_session(CreateSessionParams { + origin: "https://elsewhere.example".to_string(), + ..params_at(anchor_number, 7, 1, 2_000) + }) + .unwrap(); + storage + .create_session(params(anchor_number, 9, 3_000)) + .unwrap(); + + assert_eq!( + storage.revoke_browser_sessions(anchor_number, 0).unwrap(), + 2 + ); + + // Zero is the whole point of the counter: it is what the settings page reads to + // say a browser is signed in to nothing, and it survives a reload because it was + // stored here rather than remembered by the page. + assert_eq!( + counts(&storage, anchor_number), + (BTreeMap::from([(0, 0), (1, 1)]), 1) + ); + } + + #[test] + fn a_browser_the_registry_gave_up_leaves_no_count_behind() { + let (mut storage, anchor_number) = storage_with_anchor(); + + // Two sessions on the browser that will be given up, so a count that outlived its + // entry would be visible rather than indistinguishable from a fresh one. At two + // origins, because a second sign-in at the same one replaces the session already + // there and would leave this browser holding one. + storage + .create_session(params(anchor_number, 7, 1_000)) + .unwrap(); + storage + .create_session(CreateSessionParams { + origin: "https://elsewhere.example".to_string(), + ..params_at(anchor_number, 7, 1, 1_000) + }) + .unwrap(); + assert_eq!(counts(&storage, anchor_number).0.get(&0), Some(&2)); + + for index in 0..MAX_BROWSERS { + storage + .create_session(params(anchor_number, 100 + index as u8, 2_000)) + .unwrap(); + } + + let (per_browser, total) = counts(&storage, anchor_number); + assert!( + !per_browser.contains_key(&0), + "the dropped browser is still counted: {per_browser:?}" + ); + assert_eq!(per_browser.len(), MAX_BROWSERS); + assert!( + per_browser.values().all(|count| *count == 1), + "one session each for the browsers still registered: {per_browser:?}" + ); + assert_eq!(total as usize, MAX_BROWSERS); + } +} diff --git a/src/internet_identity/src/verified_emails/remove.rs b/src/internet_identity/src/verified_emails/remove.rs index 691aabfb04..a7685f7275 100644 --- a/src/internet_identity/src/verified_emails/remove.rs +++ b/src/internet_identity/src/verified_emails/remove.rs @@ -33,6 +33,7 @@ mod tests { let mut a = Anchor { browsers: vec![], next_browser_id: 0, + session_count: 0, anchor_number: 1, devices: vec![], openid_credentials: vec![], diff --git a/src/internet_identity_interface/src/internet_identity/types.rs b/src/internet_identity_interface/src/internet_identity/types.rs index 1f69430997..b732c44746 100644 --- a/src/internet_identity_interface/src/internet_identity/types.rs +++ b/src/internet_identity_interface/src/internet_identity/types.rs @@ -16,6 +16,10 @@ pub type ApplicationNumber = u64; pub type Timestamp = u64; // in nanos since epoch /// Per-anchor label for one browser, so a browser's sessions can be revoked together. pub type BrowserId = u32; +/// Names one session for as long as the canister runs. Allocated from a single +/// counter, so no two sessions ever share one, and a revoked session's id is never +/// handed out again. +pub type SessionId = u64; /// Which browser a sign-in came from, as a token rather than a name to show. /// diff --git a/src/internet_identity_interface/src/internet_identity/types/api_v2.rs b/src/internet_identity_interface/src/internet_identity/types/api_v2.rs index c0d9c59f8b..a4063ef5a1 100644 --- a/src/internet_identity_interface/src/internet_identity/types/api_v2.rs +++ b/src/internet_identity_interface/src/internet_identity/types/api_v2.rs @@ -86,6 +86,8 @@ pub struct BrowserInfo { pub description: BrowserDescription, pub created_at: Timestamp, pub last_used: Timestamp, + /// Sessions this browser holds, counted from the stored records. + pub session_count: u32, } #[derive(Clone, Debug, CandidType, Deserialize, Eq, PartialEq)]