diff --git a/src/internet_identity/internet_identity.did b/src/internet_identity/internet_identity.did index dd46471214..55cd466731 100644 --- a/src/internet_identity/internet_identity.did +++ b/src/internet_identity/internet_identity.did @@ -171,8 +171,6 @@ type InternetIdentityStats = record { }; archive_info: ArchiveInfo; canister_creation_cycles_cost: nat64; - max_num_latest_delegation_origins: nat64; - latest_delegation_origins: vec FrontendHostname }; // Configuration parameters related to the archive. diff --git a/src/internet_identity/src/delegation.rs b/src/internet_identity/src/delegation.rs index ccced7a97e..c296f513e5 100644 --- a/src/internet_identity/src/delegation.rs +++ b/src/internet_identity/src/delegation.rs @@ -2,7 +2,6 @@ use crate::activity_stats::event_stats::{ update_event_based_stats, Event, EventData, PrepareDelegationEvent, }; use crate::ii_domain::IIDomain; -use crate::state::persistent_state_mut; use crate::{hash, state, update_root_hash, DAY_NS, MINUTE_NS}; use candid::Principal; use canister_sig_util::signature_map::SignatureMap; @@ -70,21 +69,18 @@ fn delegation_bookkeeping( session_duration_ns, }), }); - if ii_domain.is_some() { - update_latest_delegation_origins(frontend); - } } } /// Filter out derivation origins that most likely point to development setups. -/// This is not bullet proof but given the data we collected so far it should be good for now. +/// This is not bulletproof but given the data we collected so far it should be good for now. fn is_dev_frontend(frontend: &FrontendHostname) -> bool { if frontend.starts_with("http://") || frontend.contains("localhost") { // we don't care about insecure origins or localhost return true; } - // lets check for local IP addresses + // let's check for local IP addresses if let Some(hostname) = frontend .strip_prefix("https://") .and_then(|s| s.split(':').next()) @@ -98,35 +94,6 @@ fn is_dev_frontend(frontend: &FrontendHostname) -> bool { false } -/// Add the current front-end to the list of latest used front-end origins. -fn update_latest_delegation_origins(frontend: FrontendHostname) { - let now_ns = time(); - - persistent_state_mut(|persistent_state| { - let latest_delegation_origins = &mut persistent_state.latest_delegation_origins; - - if let Some(timestamp_ns) = latest_delegation_origins.get_mut(&frontend) { - *timestamp_ns = now_ns; - } else { - latest_delegation_origins.insert(frontend, now_ns); - }; - - // drop entries older than 30 days - latest_delegation_origins.retain(|_, timestamp_ns| now_ns - *timestamp_ns < 30 * DAY_NS); - - // if we still have too many entries, drop the oldest - if latest_delegation_origins.len() as u64 - > persistent_state.max_num_latest_delegation_origins - { - // if this case is hit often (i.e. we routinely have more than 1000 entries), we should - // consider using a more efficient data structure - let mut values: Vec<_> = latest_delegation_origins.clone().into_iter().collect(); - values.sort_by(|(_, timestamp_1), (_, timestamp_2)| timestamp_1.cmp(timestamp_2)); - latest_delegation_origins.remove(&values[0].0); - }; - }); -} - pub fn get_delegation( anchor_number: AnchorNumber, frontend: FrontendHostname, diff --git a/src/internet_identity/src/http/metrics.rs b/src/internet_identity/src/http/metrics.rs index 02f09b7374..d0ae5c3190 100644 --- a/src/internet_identity/src/http/metrics.rs +++ b/src/internet_identity/src/http/metrics.rs @@ -229,12 +229,6 @@ fn persistent_state_metrics( .value(&[("type", "other")], counter.other_counter as f64)?; Ok(()) })?; - - w.encode_gauge( - "internet_identity_max_num_latest_delegation_origins", - persistent_state.max_num_latest_delegation_origins as f64, - "The maximum number of latest delegation origins that were used with II bound devices.", - )?; Ok(()) } diff --git a/src/internet_identity/src/main.rs b/src/internet_identity/src/main.rs index 544b83d865..bfc85ba5e9 100644 --- a/src/internet_identity/src/main.rs +++ b/src/internet_identity/src/main.rs @@ -336,24 +336,12 @@ fn stats() -> InternetIdentityStats { let canister_creation_cycles_cost = state::persistent_state(|persistent_state| persistent_state.canister_creation_cycles_cost); - let (latest_delegation_origins, max_num_latest_delegation_origins) = - state::persistent_state(|persistent_state| { - let origins = persistent_state - .latest_delegation_origins - .keys() - .cloned() - .collect(); - (origins, persistent_state.max_num_latest_delegation_origins) - }); - state::storage_borrow(|storage| InternetIdentityStats { assigned_user_number_range: storage.assigned_anchor_number_range(), users_registered: storage.anchor_count() as u64, archive_info, canister_creation_cycles_cost, storage_layout_version: storage.version(), - max_num_latest_delegation_origins, - latest_delegation_origins, }) } @@ -426,11 +414,6 @@ fn apply_install_arg(maybe_arg: Option) { persistent_state.registration_rate_limit = rate_limit; }) } - if let Some(limit) = arg.max_num_latest_delegation_origins { - state::persistent_state_mut(|persistent_state| { - persistent_state.max_num_latest_delegation_origins = limit; - }) - } if let Some(limit) = arg.max_inflight_captchas { state::persistent_state_mut(|persistent_state| { persistent_state.max_inflight_captchas = limit; diff --git a/src/internet_identity/src/state.rs b/src/internet_identity/src/state.rs index 410f138696..ec68ceea31 100644 --- a/src/internet_identity/src/state.rs +++ b/src/internet_identity/src/state.rs @@ -20,9 +20,6 @@ use std::time::Duration; mod temp_keys; -/// Default value for max number of delegation origins to store in the list of latest used delegation origins -pub const DEFAULT_MAX_DELEGATION_ORIGINS: u64 = 1000; - /// Default value for max number of inflight captchas. pub const DEFAULT_MAX_INFLIGHT_CAPTCHAS: u64 = 500; @@ -96,10 +93,6 @@ pub struct PersistentState { pub domain_active_anchor_stats: ActivityStats, // Daily and monthly active authentication methods on the II domains. pub active_authn_method_stats: ActivityStats, - // Hashmap of last used delegation origins - pub latest_delegation_origins: HashMap, - // Maximum number of latest delegation origins to store - pub max_num_latest_delegation_origins: u64, // Maximum number of inflight captchas pub max_inflight_captchas: u64, } @@ -114,8 +107,6 @@ impl Default for PersistentState { active_anchor_stats: ActivityStats::new(time), domain_active_anchor_stats: ActivityStats::new(time), active_authn_method_stats: ActivityStats::new(time), - latest_delegation_origins: HashMap::new(), - max_num_latest_delegation_origins: DEFAULT_MAX_DELEGATION_ORIGINS, max_inflight_captchas: DEFAULT_MAX_INFLIGHT_CAPTCHAS, } } diff --git a/src/internet_identity/src/storage/storable_persistent_state.rs b/src/internet_identity/src/storage/storable_persistent_state.rs index 12c2fd4d16..2b7193e75a 100644 --- a/src/internet_identity/src/storage/storable_persistent_state.rs +++ b/src/internet_identity/src/storage/storable_persistent_state.rs @@ -21,7 +21,9 @@ pub struct StorablePersistentState { active_anchor_stats: ActivityStats, domain_active_anchor_stats: ActivityStats, active_authn_method_stats: ActivityStats, + // unused, kept for stable memory compatibility latest_delegation_origins: HashMap, + // unused, kept for stable memory compatibility max_num_latest_delegation_origins: u64, max_inflight_captchas: u64, } @@ -53,8 +55,8 @@ impl From for StorablePersistentState { active_anchor_stats: s.active_anchor_stats, domain_active_anchor_stats: s.domain_active_anchor_stats, active_authn_method_stats: s.active_authn_method_stats, - latest_delegation_origins: s.latest_delegation_origins, - max_num_latest_delegation_origins: s.max_num_latest_delegation_origins, + latest_delegation_origins: Default::default(), + max_num_latest_delegation_origins: 0, max_inflight_captchas: s.max_inflight_captchas, } } @@ -69,8 +71,6 @@ impl From for PersistentState { active_anchor_stats: s.active_anchor_stats, domain_active_anchor_stats: s.domain_active_anchor_stats, active_authn_method_stats: s.active_authn_method_stats, - latest_delegation_origins: s.latest_delegation_origins, - max_num_latest_delegation_origins: s.max_num_latest_delegation_origins, max_inflight_captchas: s.max_inflight_captchas, } } @@ -79,7 +79,7 @@ impl From for PersistentState { #[cfg(test)] mod tests { use super::*; - use crate::state::{DEFAULT_MAX_DELEGATION_ORIGINS, DEFAULT_MAX_INFLIGHT_CAPTCHAS}; + use crate::state::{DEFAULT_MAX_INFLIGHT_CAPTCHAS}; use std::time::Duration; #[test] @@ -106,7 +106,7 @@ mod tests { domain_active_anchor_stats: ActivityStats::new(test_time), active_authn_method_stats: ActivityStats::new(test_time), latest_delegation_origins: HashMap::new(), - max_num_latest_delegation_origins: DEFAULT_MAX_DELEGATION_ORIGINS, + max_num_latest_delegation_origins: 0, max_inflight_captchas: DEFAULT_MAX_INFLIGHT_CAPTCHAS, }; @@ -122,8 +122,6 @@ mod tests { active_anchor_stats: ActivityStats::new(test_time), domain_active_anchor_stats: ActivityStats::new(test_time), active_authn_method_stats: ActivityStats::new(test_time), - latest_delegation_origins: HashMap::new(), - max_num_latest_delegation_origins: DEFAULT_MAX_DELEGATION_ORIGINS, max_inflight_captchas: DEFAULT_MAX_INFLIGHT_CAPTCHAS, }; assert_eq!(PersistentState::default(), expected_defaults); diff --git a/src/internet_identity/tests/integration/latest_delegation_origins.rs b/src/internet_identity/tests/integration/latest_delegation_origins.rs deleted file mode 100644 index 49b7ee203e..0000000000 --- a/src/internet_identity/tests/integration/latest_delegation_origins.rs +++ /dev/null @@ -1,151 +0,0 @@ -//! Tests for tracking the latest delegation origins. - -use canister_tests::api::internet_identity as api; -use canister_tests::flows; -use canister_tests::framework::{ - device_data_1, env, install_ii_canister, install_ii_canister_with_arg, principal_1, - upgrade_ii_canister, II_WASM, -}; -use ic_cdk::api::management_canister::main::CanisterId; -use ic_test_state_machine_client::{CallError, StateMachine}; -use internet_identity_interface::internet_identity::types::{ - AnchorNumber, DeviceData, InternetIdentityInit, -}; -use serde_bytes::ByteBuf; -use std::time::Duration; - -const DAY_SECONDS: u64 = 24 * 60 * 60; - -/// Verifies that origins are recorded. -#[test] -fn should_record_used_delegation_origin() -> Result<(), CallError> { - let env = env(); - let canister_id = install_ii_canister(&env, II_WASM.clone()); - let user_number = flows::register_anchor(&env, canister_id); - - delegation_for_origin(&env, canister_id, user_number, "https://some-dapp.com")?; - - let latest_origins = api::stats(&env, canister_id)?.latest_delegation_origins; - assert_eq!(latest_origins, vec!["https://some-dapp.com".to_string()]); - Ok(()) -} - -/// Verifies that dev origins (such as loopback and insecure origins) are not recorded. -#[test] -fn should_not_record_development_origins() -> Result<(), CallError> { - let env = env(); - let canister_id = install_ii_canister(&env, II_WASM.clone()); - let user_number = flows::register_anchor(&env, canister_id); - - delegation_for_origin(&env, canister_id, user_number, "http://some-dapp.com")?; - delegation_for_origin(&env, canister_id, user_number, "https://localhost:8443")?; - delegation_for_origin(&env, canister_id, user_number, "https://172.17.5.233:8443")?; - - let latest_origins = api::stats(&env, canister_id)?.latest_delegation_origins; - assert!(latest_origins.is_empty()); - Ok(()) -} - -/// Verifies that the configured limit for the number of latest used origins is respected. -#[test] -fn should_record_max_delegation_origins() -> Result<(), CallError> { - let env = env(); - let canister_id = install_ii_with_latest_origin_limit(&env, 10); - let user_number = flows::register_anchor(&env, canister_id); - - for i in 0..11 { - delegation_for_origin(&env, canister_id, user_number, &format!("dapp-{}", i))?; - env.advance_time(Duration::from_secs(1)); // let time pass so the ordering is deterministic - } - let latest_origins = api::stats(&env, canister_id)?.latest_delegation_origins; - - assert_eq!(latest_origins.len(), 10); - assert!(!latest_origins.contains(&"dapp-0".to_string())); // the oldest has been pruned - assert!(latest_origins.contains(&"dapp-10".to_string())); - Ok(()) -} - -/// Verifies that the latest origins are dropped after 30 days. -#[test] -fn should_drop_orgins_after_30_days() -> Result<(), CallError> { - let env = env(); - let canister_id = install_ii_canister(&env, II_WASM.clone()); - let user_number = flows::register_anchor(&env, canister_id); - - delegation_for_origin(&env, canister_id, user_number, "https://dropped.com")?; - delegation_for_origin(&env, canister_id, user_number, "https://not-dropped.com")?; - env.advance_time(Duration::from_secs(15 * DAY_SECONDS)); - // repeated activity to refresh the timestamp (for only one origin) - delegation_for_origin(&env, canister_id, user_number, "https://not-dropped.com")?; - env.advance_time(Duration::from_secs(15 * DAY_SECONDS)); - // activity to refresh the list - delegation_for_origin(&env, canister_id, user_number, "https://unrelated.com")?; - - let latest_origins = api::stats(&env, canister_id)?.latest_delegation_origins; - - assert!(latest_origins.contains(&"https://not-dropped.com".to_string())); - assert!(!latest_origins.contains(&"https://dropped.com".to_string())); - Ok(()) -} - -/// Verifies that the latest origins are kept across upgrades. -#[test] -fn should_keep_latest_origins_across_upgrades() -> Result<(), CallError> { - let env = env(); - let canister_id = install_ii_canister(&env, II_WASM.clone()); - let user_number = flows::register_anchor(&env, canister_id); - - delegation_for_origin(&env, canister_id, user_number, "https://some-dapp.com")?; - upgrade_ii_canister(&env, canister_id, II_WASM.clone()); - - let latest_origins = api::stats(&env, canister_id)?.latest_delegation_origins; - assert_eq!(latest_origins, vec!["https://some-dapp.com".to_string()]); - Ok(()) -} - -/// Verifies that origins are recorded only for devices on II domains. -#[test] -fn should_not_record_delegation_origin_for_devices_on_non_ii_domains() -> Result<(), CallError> { - let env = env(); - let canister_id = install_ii_canister(&env, II_WASM.clone()); - let device = DeviceData { - origin: Some("https://some-other-domain.com".to_string()), - ..device_data_1() - }; - let user_number = flows::register_anchor_with_device(&env, canister_id, &device); - - delegation_for_origin(&env, canister_id, user_number, "https://some-dapp.com")?; - - let latest_origins = api::stats(&env, canister_id)?.latest_delegation_origins; - assert!(latest_origins.is_empty()); - Ok(()) -} - -fn install_ii_with_latest_origin_limit(env: &StateMachine, limit: u64) -> CanisterId { - install_ii_canister_with_arg( - env, - II_WASM.clone(), - Some(InternetIdentityInit { - max_num_latest_delegation_origins: Some(limit), - ..Default::default() - }), - ) -} - -fn delegation_for_origin( - env: &StateMachine, - canister_id: CanisterId, - user_number: AnchorNumber, - frontend_hostname: &str, -) -> Result<(), CallError> { - api::prepare_delegation( - env, - canister_id, - principal_1(), - user_number, - frontend_hostname, - &ByteBuf::from("session public key"), - None, - )?; - Ok(()) -} diff --git a/src/internet_identity/tests/integration/main.rs b/src/internet_identity/tests/integration/main.rs index f81b0d852b..c7d3cd67c5 100644 --- a/src/internet_identity/tests/integration/main.rs +++ b/src/internet_identity/tests/integration/main.rs @@ -9,7 +9,6 @@ mod anchor_management; mod archive_integration; mod delegation; mod http; -mod latest_delegation_origins; mod rollback; mod stable_memory; mod upgrade; diff --git a/src/internet_identity_interface/src/internet_identity/types.rs b/src/internet_identity_interface/src/internet_identity/types.rs index 34287c70b6..6bc1653de1 100644 --- a/src/internet_identity_interface/src/internet_identity/types.rs +++ b/src/internet_identity_interface/src/internet_identity/types.rs @@ -207,8 +207,6 @@ pub struct InternetIdentityStats { pub archive_info: ArchiveInfo, pub canister_creation_cycles_cost: u64, pub storage_layout_version: u8, - pub max_num_latest_delegation_origins: u64, - pub latest_delegation_origins: Vec, } /// Information about the archive.