diff --git a/Cargo.lock b/Cargo.lock --- a/Cargo.lock +++ b/Cargo.lock @@ -8897,6 +8897,7 @@ dependencies = [ "tauri", "tauri-plugin", "tauri-plugin-fs", + "tempfile", "thiserror 2.0.18", "tokio", "tracing", diff --git a/crates/desktop/artcraft/src/services/storyteller/commands/login_bridge.rs b/crates/desktop/artcraft/src/services/storyteller/commands/login_bridge.rs --- a/crates/desktop/artcraft/src/services/storyteller/commands/login_bridge.rs +++ b/crates/desktop/artcraft/src/services/storyteller/commands/login_bridge.rs @@ -334,14 +334,7 @@ fn install_session_cookie( } async fn persist_session_cookie(jar: &Arc) -> Result<(), DesktopLoginError> { - let saved = jar - .request_save() - .map_err(|_| DesktopLoginError::local("Unable to persist login cookie"))?; - tokio::task::spawn_blocking(move || saved.recv()) - .await - .map_err(|_| DesktopLoginError::local("Unable to persist login cookie"))? - .map_err(|_| DesktopLoginError::local("Unable to persist login cookie"))?; - Ok(()) + jar.flush().await.map_err(|_| DesktopLoginError::local("Unable to persist login cookie")) } fn expired() -> DesktopLoginOutcome { diff --git a/crates/desktop/artcraft/src/services/storyteller/commands/storyteller_purge_credentials_command.rs b/crates/desktop/artcraft/src/services/storyteller/commands/storyteller_purge_credentials_command.rs --- a/crates/desktop/artcraft/src/services/storyteller/commands/storyteller_purge_credentials_command.rs +++ b/crates/desktop/artcraft/src/services/storyteller/commands/storyteller_purge_credentials_command.rs @@ -10,7 +10,10 @@ use anyhow::anyhow; use errors::AnyhowResult; use log::{error, info}; use serde_derive::{Deserialize, Serialize}; -use tauri::{AppHandle, State}; +use tauri::{AppHandle, Manager, State}; +use crate::core::threads::main_window_thread::persist_storyteller_cookies_task::storyteller_cookie_url; +use crate::services::storyteller::state::purge_http_credentials::purge_http_credentials; +use tauri_plugin_http::Http; #[derive(Serialize)] pub struct PurgeStorytellerCredentialsCommandResponse { @@ -43,14 +46,8 @@ async fn reset( storyteller_creds_manager: &StorytellerCredentialManager ) -> AnyhowResult<()> { - clear_main_webview_window_storyteller_cookies(&app)?; - - storyteller_creds_manager.clear_credentials()?; - storyteller_creds_manager.delete_persisted_copies_on_disk()?; - - // NB: Twice to be sure. - storyteller_creds_manager.clear_credentials()?; - storyteller_creds_manager.delete_persisted_copies_on_disk()?; - - Ok(()) + let http = app.try_state::().ok_or_else(|| anyhow!("HTTP plugin unavailable"))?; + let purged = purge_http_credentials(&http.cookies_jar, storyteller_creds_manager, &storyteller_cookie_url(app)?).await; + let cleared_webview = clear_main_webview_window_storyteller_cookies(app); + purged.and(cleared_webview) } diff --git a/crates/desktop/artcraft/src/services/storyteller/state/mod.rs b/crates/desktop/artcraft/src/services/storyteller/state/mod.rs --- a/crates/desktop/artcraft/src/services/storyteller/state/mod.rs +++ b/crates/desktop/artcraft/src/services/storyteller/state/mod.rs @@ -1,3 +1,4 @@ pub mod read_storyteller_credentials_from_disk; pub mod storyteller_credential_holder; -pub mod storyteller_credential_manager; \ No newline at end of file +pub mod storyteller_credential_manager; +pub(crate) mod purge_http_credentials; diff --git a/crates/desktop/artcraft/src/services/storyteller/state/purge_http_credentials.rs b/crates/desktop/artcraft/src/services/storyteller/state/purge_http_credentials.rs new file mode 100644 --- /dev/null +++ b/crates/desktop/artcraft/src/services/storyteller/state/purge_http_credentials.rs @@ -0,0 +1,76 @@ +use anyhow::{anyhow, Result}; +use reqwest::Url; +use tauri_plugin_http::reqwest_cookie_store::CookieStoreMutex; + +use crate::services::storyteller::state::storyteller_credential_manager::StorytellerCredentialManager; + +pub(crate) async fn purge_http_credentials(jar: &CookieStoreMutex, manager: &StorytellerCredentialManager, api_url: &Url) -> Result<()> { + let cleanup = { + // The synchronization task uses this same jar lock before updating the + // manager. Holding it through both removals prevents a stale sync snapshot. + let mut store = jar.store.lock().map_err(|_| anyhow!("Failed to lock HTTP cookie jar"))?; + let keys: Vec<_> = store.iter_any().filter(|cookie| matches!(cookie.name(), "session" | "visitor") && cookie.domain.matches(api_url)).filter_map(|cookie| { + Some((cookie.domain.as_cow()?.into_owned(), cookie.path.to_string(), cookie.name().to_owned())) + }).collect(); + for (domain, path, name) in keys { store.remove(&domain, &path, &name); } + let cleared = manager.clear_credentials(); + let deleted = manager.delete_persisted_copies_on_disk(); + cleared.and(deleted) + }; + // A legacy-file error must not leave the authoritative disk jar signed in. + let persisted = jar.flush().await.map_err(|err| anyhow!("Failed to persist cleared HTTP cookies: {err}")); + cleanup.and(persisted) +} + +#[cfg(test)] +mod login_bridge_tests { + use super::*; + use crate::core::state::data_dir::app_data_root::AppDataRoot; + use crate::core::threads::main_window_thread::persist_storyteller_cookies_task::sync_tauri_credentials; + use tauri_plugin_http::reqwest_cookie_store::{CookieStore, RawCookie}; + + #[tokio::test] + async fn legacy_copy_removal_failure_still_flushes_the_authoritative_jar() { + let directory = tempfile::tempdir().unwrap(); + let root = AppDataRoot::create_existing(directory.path()).unwrap(); + let manager = StorytellerCredentialManager::initialize_empty(&root); + std::fs::create_dir(root.credentials_dir().get_storyteller_session_cookie_file_path()).unwrap(); + let url = Url::parse("https://api.storyteller.ai/").unwrap(); + let mut store = CookieStore::default(); + store.store_response_cookies([RawCookie::parse("session=old; Path=/; Max-Age=3600").unwrap().into_owned()].into_iter(), &url); + let path = directory.path().join("cookies.json"); + let jar = CookieStoreMutex::new(path.clone(), store); + jar.flush().await.unwrap(); + assert!(purge_http_credentials(&jar, &manager, &url).await.is_err()); + let reloaded: CookieStore = serde_json::from_slice(&std::fs::read(path).unwrap()).unwrap(); + assert!(reloaded.get_request_values(&url).next().is_none()); + } + + #[tokio::test] + async fn local_purge_survives_queued_save_sync_and_reload_without_server_logout() { + let directory = tempfile::tempdir().unwrap(); + let root = AppDataRoot::create_existing(directory.path()).unwrap(); + let manager = StorytellerCredentialManager::initialize_empty(&root); + let url = Url::parse("https://api.storyteller.ai/").unwrap(); + let mut store = CookieStore::default(); + for cookie in ["session=old; Domain=storyteller.ai; Path=/; Max-Age=3600", "session=path-session; Path=/v1; Max-Age=3600", "visitor=old-visitor; Path=/; Max-Age=3600", "theme=dark; Path=/; Max-Age=3600"] { + store.store_response_cookies([RawCookie::parse(cookie).unwrap().into_owned()].into_iter(), &url); + } + store.store_response_cookies([RawCookie::parse("session=other; Path=/; Max-Age=3600").unwrap().into_owned()].into_iter(), &Url::parse("https://unrelated.example/").unwrap()); + sync_tauri_credentials(&store, &manager, &url).unwrap(); + manager.persist_all_to_disk().unwrap(); + let path = directory.path().join("cookies.json"); + let jar = CookieStoreMutex::new(path.clone(), store); + let pending = jar.request_save().unwrap(); + purge_http_credentials(&jar, &manager, &url).await.unwrap(); + // The older writer may finish later, but must serialize the cleared jar. + tokio::task::spawn_blocking(move || pending.recv_timeout(std::time::Duration::from_secs(2))).await.unwrap().unwrap(); + let reloaded: CookieStore = serde_json::from_slice(&std::fs::read(path).unwrap()).unwrap(); + sync_tauri_credentials(&reloaded, &manager, &url).unwrap(); + assert!(manager.get_credentials_required().unwrap().session.is_none()); + assert!(!root.credentials_dir().get_storyteller_session_cookie_file_path().exists()); + assert_eq!(reloaded.get_request_values(&url).collect::>(), [("theme", "dark")]); + assert_eq!(reloaded.get_request_values(&Url::parse("https://unrelated.example/").unwrap()).collect::>(), [("session", "other")]); + assert!(reloaded.get_request_values(&url.join("v1").unwrap()).all(|(name, _)| name != "session")); + } +} diff --git a/crates/vendor/tauri-plugin-http/Cargo.toml b/crates/vendor/tauri-plugin-http/Cargo.toml --- a/crates/vendor/tauri-plugin-http/Cargo.toml +++ b/crates/vendor/tauri-plugin-http/Cargo.toml @@ -87,3 +87,6 @@ macos-system-configuration = ["reqwest/macos-system-configuration"] unsafe-headers = [] tracing = ["dep:tracing"] dangerous-settings = [] + +[dev-dependencies] +tempfile = "3" diff --git a/crates/vendor/tauri-plugin-http/src/reqwest_cookie_store.rs b/crates/vendor/tauri-plugin-http/src/reqwest_cookie_store.rs --- a/crates/vendor/tauri-plugin-http/src/reqwest_cookie_store.rs +++ b/crates/vendor/tauri-plugin-http/src/reqwest_cookie_store.rs @@ -6,7 +6,7 @@ use std::{ path::PathBuf, - sync::{mpsc::Receiver, Mutex}, + sync::{mpsc::Receiver, Arc, Mutex}, }; // NB(bt): Re-export these for reuse @@ -48,7 +48,8 @@ fn cookies(cookie_store: &CookieStore, url: &url::Url) -> Option { pub struct CookieStoreMutex { pub path: PathBuf, // NB(bt): We've made both of these public so we can access them. - pub store: Mutex, + pub store: Arc>, + save_lock: Arc>, pub save_task: Mutex>, } @@ -57,7 +58,8 @@ impl CookieStoreMutex { pub fn new(path: PathBuf, cookie_store: CookieStore) -> CookieStoreMutex { CookieStoreMutex { path, - store: Mutex::new(cookie_store), + store: Arc::new(Mutex::new(cookie_store)), + save_lock: Arc::new(tokio::sync::Mutex::new(())), save_task: Default::default(), } } @@ -70,42 +72,29 @@ impl CookieStoreMutex { .map(|store| CookieStoreMutex::new(path, store)) } - fn cookies_to_str(&self) -> Result { - let mut cookies = Vec::new(); - for cookie in self - .store - .lock() - .expect("poisoned cookie jar mutex") - .iter_unexpired() - { - if cookie.is_persistent() { - cookies.push(cookie.clone()); - } - } - serde_json::to_string(&cookies) + /// Persist the current jar and wait until it reaches disk. All writers share + /// one lock and take their snapshot only when they own it, so an older + /// queued save cannot overwrite a completed purge with stale cookies. + pub async fn flush(&self) -> cookie_store::Result<()> { + persist_current(&self.path, &self.store, &self.save_lock).await } pub fn request_save(&self) -> cookie_store::Result> { - let cookie_str = self.cookies_to_str()?; - let path = self.path.clone(); + let (path, store, save_lock) = (self.path.clone(), self.store.clone(), self.save_lock.clone()); let (tx, rx) = std::sync::mpsc::channel(); let task = tauri::async_runtime::spawn(async move { - match tokio::fs::write(&path, &cookie_str).await { - Ok(()) => { - let _ = tx.send(()); - } + match persist_current(&path, &store, &save_lock).await { + Ok(()) => { let _ = tx.send(()); } Err(_e) => { #[cfg(feature = "tracing")] tracing::error!("failed to save cookie jar: {_e}"); } } }); - self.save_task - .lock() - .unwrap() - .replace(CancellableTask(task)); + self.save_task.lock().unwrap().replace(CancellableTask(task)); Ok(rx) } + } impl reqwest::cookie::CookieStore for CookieStoreMutex { @@ -133,3 +122,37 @@ impl Drop for CancellableTask { self.0.abort(); } } + +async fn persist_current(path: &std::path::Path, store: &Mutex, save_lock: &tokio::sync::Mutex<()>) -> cookie_store::Result<()> { + let _save = save_lock.lock().await; + let text = { + let jar = store.lock().expect("poisoned cookie jar mutex"); + let cookies: Vec<_> = jar.iter_unexpired().filter(|cookie| cookie.is_persistent()).collect(); + serde_json::to_string(&cookies)? + }; + // No await between snapshot and write: cancellation of an older save must + // not release its lock while a detached tokio filesystem write is running. + std::fs::write(path, text)?; + Ok(()) +} + +#[cfg(test)] +mod persistence_tests { + use super::*; + + #[tokio::test] + async fn queued_save_snapshots_only_after_acquiring_writer_lock() { + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("cookies.json"); + let mut store = CookieStore::default(); + store.parse("session=old; Path=/; Max-Age=3600", &url::Url::parse("https://example.com/").unwrap()).unwrap(); + let jar = CookieStoreMutex::new(path.clone(), store); + let blocked = jar.save_lock.lock().await; + let pending = jar.request_save().unwrap(); + jar.store.lock().unwrap().clear(); + drop(blocked); + tokio::task::spawn_blocking(move || pending.recv_timeout(std::time::Duration::from_secs(2))).await.unwrap().unwrap(); + let saved: CookieStore = serde_json::from_slice(&std::fs::read(path).unwrap()).unwrap(); + assert_eq!(saved.iter_any().count(), 0); + } +} diff --git a/netlify.toml b/netlify.toml new file mode 100644 --- /dev/null +++ b/netlify.toml @@ -0,0 +1,7 @@ +# The `storyteller-docs` Netlify site still builds this repository, but its +# build command (./script/netlify_build_docs.sh) and the docs themselves now +# live in storytold/artcraft-services, so every deploy preview here fails. +# Skip those builds until the Netlify site is repointed or disconnected, then +# delete this file. +[build] + ignore = "exit 0"