diff --git a/src/application/services/app_password_service.rs b/src/application/services/app_password_service.rs index ee02434d..e603f65a 100644 --- a/src/application/services/app_password_service.rs +++ b/src/application/services/app_password_service.rs @@ -30,9 +30,25 @@ const NC_APP_PASSWORD_GROUP_LEN: usize = 5; const NC_PREFIX_LEN: usize = 8; /// TTL for cached Basic Auth verification results. -/// Balances performance (avoids repeated Argon2id + DB queries) with security -/// (limits the window during which a revoked app password remains usable). -const BASIC_AUTH_CACHE_TTL_SECS: u64 = 30; +/// +/// DAV sync clients (Nautilus, Windows Explorer, Apple Calendar, …) poll +/// continuously, and every cache miss costs a full Argon2id verification +/// (~50–100 ms of CPU) plus two DB round-trips. A 30 s TTL re-paid that +/// cost every 30 s per client; 5 min cuts it ~10× under steady sync load. +/// +/// Security envelope of this window: +/// - **Revocation is immediate**: `revoke()` calls `invalidate_entries_if` +/// on this cache for the user, so a revoked password never survives in +/// cache regardless of TTL. +/// - **Expiry / deactivation are bounded by the TTL**: `expires_at` and +/// `user.is_active()` are only re-checked on a cache *miss* (the DB +/// query filters them), so an app password that expires — or a user +/// deactivated via `set_user_active` — may keep authenticating from +/// cache for at most this long. 5 min is comparable to a typical JWT +/// access-token lifetime, so the grace window is consistent across +/// auth surfaces. Lengthen with care; shorten if a tighter bound on +/// post-deactivation access is required. +const BASIC_AUTH_CACHE_TTL_SECS: u64 = 300; /// Maximum number of cached Basic Auth verifications. /// Each entry is ~160 bytes (32-byte key + 4 small strings), so 10 000 @@ -61,9 +77,9 @@ pub struct AppPasswordService { /// /// **Value**: the authenticated identity (user_id, username, email, role). /// - /// **Eviction**: TTL-based (30 s) + capacity-based (10 000 entries). - /// Failed verifications are *never* cached, so brute-force attackers - /// always pay the full Argon2id cost. + /// **Eviction**: TTL-based (see `BASIC_AUTH_CACHE_TTL_SECS`) + + /// capacity-based (10 000 entries). Failed verifications are *never* + /// cached, so brute-force attackers always pay the full Argon2id cost. auth_cache: Cache<[u8; 32], CachedBasicAuthResult>, } diff --git a/src/infrastructure/services/webdav_lock_service.rs b/src/infrastructure/services/webdav_lock_service.rs index efcfaf48..2b4ee389 100644 --- a/src/infrastructure/services/webdav_lock_service.rs +++ b/src/infrastructure/services/webdav_lock_service.rs @@ -1,15 +1,23 @@ //! WebDAV lock store backed by Moka (in-memory cache with per-entry TTL). //! -//! Locks are automatically evicted when their timeout expires, preventing -//! orphaned locks from accumulating. Two caches are maintained: +//! Each lock expires automatically at its own RFC 4918 `Timeout`, enforced +//! by Moka's [`Expiry`](moka::Expiry) policy. There are **no background +//! tasks and no per-lock timers** — Office clients refresh locks +//! constantly, and spawning a `sleep` future per acquire/refresh used to +//! leave thousands of orphaned timers pinned in the runtime. Two caches are +//! maintained: //! -//! - `by_path` : path → `LockEntry` (for LOCK conflict detection) -//! - `by_token` : token → path (for fast UNLOCK / refresh lookups) +//! - `by_path` : path → `LockEntry` (source of truth; precise per-lock TTL) +//! - `by_token` : token → path (reverse index for UNLOCK / refresh) //! -//! Both caches share the same TTL so entries disappear together. +//! `by_path` carries the exact per-lock TTL via `Expiry`; `by_token` keeps a +//! 24 h backstop TTL. A reverse-index entry that outlives its lock is +//! harmless: every lookup resolves through `by_path`, which is +//! authoritative, so an expired lock reads as absent even before its token +//! mapping is evicted. use std::sync::Arc; -use std::time::Duration; +use std::time::{Duration, Instant}; use crate::application::adapters::webdav_adapter::{LockInfo, LockScope}; @@ -26,6 +34,40 @@ pub struct LockEntry { pub path: String, } +/// Per-entry expiration policy for the `by_path` cache. +/// +/// Moka calls this on insert (create) and re-insert (update, i.e. refresh) +/// to derive each lock's TTL from its own `Timeout` header — replacing the +/// old "global TTL + one spawned timer per lock" scheme. Reads do not +/// extend the lock (the default `expire_after_read` leaves the remaining +/// duration untouched). +struct LockExpiry; + +impl moka::Expiry for LockExpiry { + fn expire_after_create( + &self, + _path: &String, + entry: &LockEntry, + _created_at: Instant, + ) -> Option { + Some(WebDavLockStore::parse_timeout( + entry.info.timeout.as_deref(), + )) + } + + fn expire_after_update( + &self, + _path: &String, + entry: &LockEntry, + _updated_at: Instant, + _remaining: Option, + ) -> Option { + Some(WebDavLockStore::parse_timeout( + entry.info.timeout.as_deref(), + )) + } +} + /// In-memory WebDAV lock store with automatic TTL-based expiration. /// /// Uses Moka's `sync::Cache` — lock-free (sharded) reads, bounded size, @@ -42,13 +84,16 @@ impl WebDavLockStore { /// /// * `max_capacity` — upper bound on simultaneous locks (evicts LRU on overflow). pub fn new(max_capacity: u64) -> Self { - // We use `expire_after` (per-entry TTL) via insert with explicit ttl, - // so we configure a generous global time_to_live as a safety net. + // `by_path` is the source of truth: each lock expires at its own + // `Timeout` via the `LockExpiry` policy (no spawned timers). let by_path = moka::sync::Cache::builder() .max_capacity(max_capacity) - .time_to_live(Duration::from_secs(MAX_LOCK_TIMEOUT_SECS)) + .expire_after(LockExpiry) .build(); + // `by_token` is a reverse index; a 24 h backstop TTL bounds any + // mapping that outlives its lock. Lookups resolve through `by_path`, + // so a lingering entry here never resurrects an expired lock. let by_token = moka::sync::Cache::builder() .max_capacity(max_capacity) .time_to_live(Duration::from_secs(MAX_LOCK_TIMEOUT_SECS)) @@ -72,38 +117,17 @@ impl WebDavLockStore { return Err(existing); } - let ttl = Self::parse_timeout(info.timeout.as_deref()); let entry = LockEntry { info, path: path.to_owned(), }; + // `LockExpiry` derives the TTL from `entry.info.timeout` on insert — + // no spawned timer needed. self.by_path.insert(path.to_owned(), entry.clone()); self.by_token .insert(entry.info.token.clone(), path.to_owned()); - // Moka 0.12 does not expose per-entry set_expiration_after_insert at - // insert time. We rely on the global `time_to_live` as an upper bound - // and use the `invalidate_after` helper below for custom TTL. - // - // To implement shorter-than-max TTL we schedule an async invalidation. - if ttl.as_secs() < MAX_LOCK_TIMEOUT_SECS { - let by_path = self.by_path.clone(); - let by_token = self.by_token.clone(); - let token = entry.info.token.clone(); - let path_owned = path.to_owned(); - tokio::spawn(async move { - tokio::time::sleep(ttl).await; - // Only remove if the entry still matches (wasn't refreshed/replaced) - if let Some(e) = by_path.get(&path_owned) - && e.info.token == token - { - by_path.invalidate(&path_owned); - by_token.invalidate(&token); - } - }); - } - Ok(entry) } @@ -120,29 +144,13 @@ impl WebDavLockStore { } let ttl = Self::parse_timeout(new_timeout.or(entry.info.timeout.as_deref())); - let timeout_str = format!("Second-{}", ttl.as_secs()); - entry.info.timeout = Some(timeout_str.clone()); + // Normalize the stored timeout so `LockExpiry` recomputes the new TTL + // from it on re-insert (Moka fires `expire_after_update`). + entry.info.timeout = Some(format!("Second-{}", ttl.as_secs())); - // Re-insert to reset the TTL self.by_path.insert(path.clone(), entry.clone()); self.by_token.insert(token.to_owned(), path.clone()); - if ttl.as_secs() < MAX_LOCK_TIMEOUT_SECS { - let by_path = self.by_path.clone(); - let by_token = self.by_token.clone(); - let token_owned = token.to_owned(); - let path_owned = path.clone(); - tokio::spawn(async move { - tokio::time::sleep(ttl).await; - if let Some(e) = by_path.get(&path_owned) - && e.info.token == token_owned - { - by_path.invalidate(&path_owned); - by_token.invalidate(&token_owned); - } - }); - } - Some(entry) } @@ -212,3 +220,136 @@ pub fn create_webdav_lock_store() -> Arc { // if the cap is reached, so stale entries are cleaned automatically. Arc::new(WebDavLockStore::new(10_000)) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::application::adapters::webdav_adapter::LockType; + use moka::Expiry; + + fn lock_info(token: &str, timeout: Option<&str>, scope: LockScope) -> LockInfo { + LockInfo { + token: token.to_owned(), + owner: Some("tester".to_owned()), + depth: "0".to_owned(), + timeout: timeout.map(str::to_owned), + scope, + type_: LockType::Write, + } + } + + fn entry(token: &str, timeout: Option<&str>) -> LockEntry { + LockEntry { + info: lock_info(token, timeout, LockScope::Exclusive), + path: "/file.txt".to_owned(), + } + } + + #[test] + fn expiry_uses_per_entry_timeout() { + let now = Instant::now(); + let key = "/file.txt".to_owned(); + + // Explicit Second-NNN → that exact duration. + let e = entry("t", Some("Second-300")); + assert_eq!( + LockExpiry.expire_after_create(&key, &e, now), + Some(Duration::from_secs(300)) + ); + // Refresh path (update) recomputes from the (normalized) timeout. + assert_eq!( + LockExpiry.expire_after_update(&key, &e, now, None), + Some(Duration::from_secs(300)) + ); + } + + #[test] + fn expiry_clamps_infinite_and_defaults_none() { + let now = Instant::now(); + let key = "/file.txt".to_owned(); + + let infinite = entry("t", Some("Infinite")); + assert_eq!( + LockExpiry.expire_after_create(&key, &infinite, now), + Some(Duration::from_secs(MAX_LOCK_TIMEOUT_SECS)) + ); + + let none = entry("t", None); + assert_eq!( + LockExpiry.expire_after_create(&key, &none, now), + Some(Duration::from_secs(DEFAULT_LOCK_TIMEOUT_SECS)) + ); + + // Over-large requests are clamped to the maximum. + let huge = entry("t", Some("Second-999999999")); + assert_eq!( + LockExpiry.expire_after_create(&key, &huge, now), + Some(Duration::from_secs(MAX_LOCK_TIMEOUT_SECS)) + ); + } + + #[test] + fn acquire_get_release_roundtrip() { + let store = WebDavLockStore::new(16); + let info = lock_info("urn:token-1", Some("Second-600"), LockScope::Exclusive); + + let acquired = store.acquire("/a.txt", info).expect("acquire"); + assert_eq!(acquired.info.token, "urn:token-1"); + + // Resolvable by both indexes. + assert_eq!( + store.get_by_path("/a.txt").map(|e| e.info.token.clone()), + Some("urn:token-1".to_owned()) + ); + assert_eq!( + store.get_by_token("urn:token-1").map(|e| e.path.clone()), + Some("/a.txt".to_owned()) + ); + + assert!(store.release("urn:token-1")); + assert!(store.get_by_path("/a.txt").is_none()); + assert!(store.get_by_token("urn:token-1").is_none()); + // Releasing an unknown token reports nothing removed. + assert!(!store.release("urn:token-1")); + } + + #[test] + fn exclusive_lock_conflicts() { + let store = WebDavLockStore::new(16); + store + .acquire( + "/a.txt", + lock_info("urn:token-1", Some("Second-600"), LockScope::Exclusive), + ) + .expect("first acquire"); + + let conflict = store.acquire( + "/a.txt", + lock_info("urn:token-2", Some("Second-600"), LockScope::Exclusive), + ); + assert!(conflict.is_err()); + // The original holder is returned so the caller can report it. + assert_eq!(conflict.unwrap_err().info.token, "urn:token-1"); + } + + #[test] + fn refresh_normalizes_timeout_and_keeps_lock() { + let store = WebDavLockStore::new(16); + store + .acquire( + "/a.txt", + lock_info("urn:token-1", Some("Infinite"), LockScope::Exclusive), + ) + .expect("acquire"); + + let refreshed = store + .refresh("urn:token-1", Some("Second-120")) + .expect("refresh"); + assert_eq!(refreshed.info.timeout.as_deref(), Some("Second-120")); + // Still present and still addressable by token. + assert!(store.get_by_token("urn:token-1").is_some()); + + // Refreshing an unknown token yields None. + assert!(store.refresh("urn:unknown", Some("Second-120")).is_none()); + } +}