From 91c47245ab3f7e71a2225a25e99a43cd6f42a645 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 8 Oct 2026 14:16:53 -0400 Subject: [PATCH] Linking a device is one flow in the core: sync::link The desktop's sync_link and the ffi's link_with_password/link_with_token were the same steps written out twice: probe, refuse an incompatible server before any credential is sent, log in or verify a pasted token, keep the link, and adopt the server's trash retention. link::authenticate(url, Credential) does the network half and link::store(conn, ..., seal) keeps it, sealed when the client has a seal. Each client now only reads its input and picks its seal. The desktop checks for a missing email/password before probing rather than after. Same error, sooner. DRY pass #2, batch 1, F2 (#5372). Co-Authored-By: Claude Opus 5.5 --- android/ffi/src/lib.rs | 56 +++++++--------- core/src/sync/link.rs | 90 ++++++++++++++++++++++++++ core/src/sync/mod.rs | 1 + desktop/src-tauri/src/commands/sync.rs | 60 +++++++---------- 4 files changed, 139 insertions(+), 68 deletions(-) create mode 100644 core/src/sync/link.rs diff --git a/android/ffi/src/lib.rs b/android/ffi/src/lib.rs index 9576f6f..9c9bd97 100644 --- a/android/ffi/src/lib.rs +++ b/android/ffi/src/lib.rs @@ -41,7 +41,7 @@ use std::sync::Arc; use inkwell_core::local::{self, Db}; use inkwell_core::sync::blobs::BlobStore; -use inkwell_core::sync::{client, compat, engine, push, sharing, state}; +use inkwell_core::sync::{client, compat, engine, link, push, sharing, state}; use models::{ patch_from, BodyItem, BodyTag, ClientUpdate, Directory, Identity, Label, Note, NoteDraft, @@ -502,18 +502,12 @@ impl Inkwell { password: String, device_name: String, ) -> Result { - let probe = client::probe(&url).await.map_err(CoreError::network)?; - if let compat::Compatibility::Incompatible { reason, .. } = &probe.compatibility { - return Err(CoreError::Network { - message: reason.clone(), - }); - } - let (token, identity) = - client::device_login(&probe.base_url, &email, &password, &device_name) - .await - .map_err(CoreError::network)?; - self.store_link(&probe.base_url, &token, probe.server.trash_retention_days)?; - Ok(identity.into()) + let credential = link::Credential::Password { + email: &email, + password: &password, + device_name: &device_name, + }; + self.link(&url, credential).await } /// Pair using a device token pasted from the web app — for anyone who would @@ -522,17 +516,7 @@ impl Inkwell { /// The token is verified before it is stored, so a copy/paste slip fails here /// rather than at the next sync. pub async fn link_with_token(&self, url: String, token: String) -> Result { - let probe = client::probe(&url).await.map_err(CoreError::network)?; - if let compat::Compatibility::Incompatible { reason, .. } = &probe.compatibility { - return Err(CoreError::Network { - message: reason.clone(), - }); - } - let identity = client::fetch_identity(&probe.base_url, &token) - .await - .map_err(CoreError::network)?; - self.store_link(&probe.base_url, &token, probe.server.trash_retention_days)?; - Ok(identity.into()) + self.link(&url, link::Credential::Token(&token)).await } /// Stop syncing, and retire this device's token on the server. @@ -757,9 +741,19 @@ impl Inkwell { link.ok_or(CoreError::NotLinked) } - /// Persist a fresh link, adopting the server's retention window at the same time - /// so the Trash view stops counting down against this device's offline default - /// the moment it is no longer the policy in force. + /// Both ways of linking, once the credential is chosen: ask the server, then + /// keep the link sealed. + async fn link( + &self, + url: &str, + credential: link::Credential<'_>, + ) -> Result { + let granted = link::authenticate(url, credential).await.map_err(CoreError::network)?; + self.store_link(&granted.base_url, &granted.token, granted.retention_days)?; + Ok(granted.identity.into()) + } + + /// Keep a fresh link, sealed (`link::store`). fn store_link( &self, base_url: &str, @@ -767,12 +761,8 @@ impl Inkwell { retention_days: Option, ) -> Result<(), CoreError> { let conn = self.db.conn().map_err(CoreError::store)?; - state::set_sealed_link(&conn, base_url, token, &self.seal).map_err(CoreError::store)?; - if let Some(days) = retention_days { - state::set_server_retention(&conn, days as i64).map_err(CoreError::store)?; - } - log::info!("linked to {base_url}"); - Ok(()) + link::store(&conn, base_url, token, retention_days, Some(&self.seal)) + .map_err(CoreError::store) } } diff --git a/core/src/sync/link.rs b/core/src/sync/link.rs new file mode 100644 index 0000000..08808f3 --- /dev/null +++ b/core/src/sync/link.rs @@ -0,0 +1,90 @@ +//! Linking a device to a server. +//! +//! One flow for every client. The desktop and Android each wrote it out, the same +//! steps in the same order; what differs between them is only how the token is +//! kept (Android seals it, `state::TokenSeal`) and how the result is reported. + +use rusqlite::Connection; + +use super::client::{self, Identity}; +use super::compat::Compatibility; +use super::state::{self, TokenSeal}; + +/// How a device proves whose it is. +pub enum Credential<'a> { + /// A password login, which mints a token named `device_name` on the server. + Password { + email: &'a str, + password: &'a str, + device_name: &'a str, + }, + /// A device token pasted from the web app, for anyone who would rather not + /// type a password into an app. Verified before it is kept, so a copy/paste + /// slip fails here rather than at the next sync. + Token(&'a str), +} + +/// A server that accepted this device, before anything is kept. +pub struct Granted { + pub base_url: String, + pub token: String, + pub identity: Identity, + /// Carried through so a client can warn about a `degraded` server right after + /// linking, instead of staying silent until a feature quietly does nothing. + pub compatibility: Compatibility, + pub retention_days: Option, +} + +/// Ask the server at `url` to accept this device. Nothing is stored. +/// +/// The handshake runs FIRST, and an incompatible server is refused before any +/// credential is sent: that is exactly the case where a later failure would be +/// hardest to attribute. +pub async fn authenticate(url: &str, credential: Credential<'_>) -> Result { + let probe = client::probe(url).await?; + if let Compatibility::Incompatible { reason, .. } = &probe.compatibility { + return Err(reason.clone()); + } + let base_url = probe.base_url; + let (token, identity) = match credential { + Credential::Password { + email, + password, + device_name, + } => client::device_login(&base_url, email, password, device_name).await?, + Credential::Token(token) => { + let identity = client::fetch_identity(&base_url, token).await?; + (token.to_string(), identity) + } + }; + Ok(Granted { + base_url, + token, + identity, + compatibility: probe.compatibility, + retention_days: probe.server.trash_retention_days, + }) +} + +/// Keep a link: sealed when the client has a seal, plain when it has none. +/// +/// The server's trash-retention window is adopted at the same time, so the Trash +/// view stops counting down against this device's offline default the moment it +/// is no longer the policy in force. +pub fn store( + conn: &Connection, + base_url: &str, + token: &str, + retention_days: Option, + seal: Option<&dyn TokenSeal>, +) -> rusqlite::Result<()> { + match seal { + Some(seal) => state::set_sealed_link(conn, base_url, token, seal)?, + None => state::set_link(conn, base_url, token)?, + } + if let Some(days) = retention_days { + state::set_server_retention(conn, i64::from(days))?; + } + log::info!("linked to {base_url}"); + Ok(()) +} diff --git a/core/src/sync/mod.rs b/core/src/sync/mod.rs index 22c265c..064908d 100644 --- a/core/src/sync/mod.rs +++ b/core/src/sync/mod.rs @@ -17,6 +17,7 @@ pub mod blobs; pub mod client; pub mod compat; pub mod engine; +pub mod link; pub mod pull; pub mod push; pub mod sharing; diff --git a/desktop/src-tauri/src/commands/sync.rs b/desktop/src-tauri/src/commands/sync.rs index 5968756..cd0f04e 100644 --- a/desktop/src-tauri/src/commands/sync.rs +++ b/desktop/src-tauri/src/commands/sync.rs @@ -11,6 +11,7 @@ use inkwell_core::sync::client::{self, Identity, ProbeResult}; use inkwell_core::sync::client::{Directory, NoteShare, ShareTarget}; use inkwell_core::sync::compat::Compatibility; use inkwell_core::sync::engine; +use inkwell_core::sync::link; use inkwell_core::sync::push; use inkwell_core::sync::sharing; use inkwell_core::sync::state; @@ -63,55 +64,44 @@ fn trimmed(value: &Option) -> Option<&str> { #[tauri::command] pub async fn sync_link(input: LinkInput, db: State<'_, Db>) -> Result { - // 1. Handshake FIRST. Never hand credentials to a server we've established we - // can't sync with — and an incompatible server is exactly the case where a - // later failure would be hardest to attribute. - let probe = client::probe(&input.url).await?; - if let Compatibility::Incompatible { reason, .. } = &probe.compatibility { - return Err(reason.clone()); - } - let base_url = probe.base_url; - - // 2. Obtain a credential. - let (token, identity) = match trimmed(&input.token) { - Some(token) => { - // Verify before storing: an unverified paste turns a copy/paste slip - // into a failure that only surfaces at the next sync. - let identity = client::fetch_identity(&base_url, token).await?; - (token.to_string(), identity) - } + let name = trimmed(&input.name) + .map(str::to_string) + .unwrap_or_else(default_device_name); + let credential = match trimmed(&input.token) { + Some(token) => link::Credential::Token(token), None => { let (Some(email), Some(password)) = (trimmed(&input.email), trimmed(&input.password)) else { return Err("Enter your email and password, or paste a device token.".to_string()); }; - let name = trimmed(&input.name) - .map(str::to_string) - .unwrap_or_else(default_device_name); - client::device_login(&base_url, email, password, &name).await? + link::Credential::Password { + email, + password, + device_name: &name, + } } }; + let granted = link::authenticate(&input.url, credential).await?; - // 3. Persist. The lock is taken only now, for two reasons: a std MutexGuard - // isn't Send so it cannot be held across an await, and holding the store - // locked for a network round-trip would freeze every note operation in the UI. + // The lock is taken only now: a std MutexGuard isn't Send so it cannot be held + // across an await, and holding the store locked for a network round-trip would + // freeze every note operation in the UI. let status = { let conn = db.conn()?; - state::set_link(&conn, &base_url, &token).map_err(|e| e.to_string())?; - // Adopt the server's trash-retention window immediately, so the Trash view - // stops counting down against this device's offline default the moment it's - // no longer the policy in force. - if let Some(days) = probe.server.trash_retention_days { - state::set_server_retention(&conn, days as i64).map_err(|e| e.to_string())?; - } + link::store( + &conn, + &granted.base_url, + &granted.token, + granted.retention_days, + None, + ) + .map_err(|e| e.to_string())?; state::status(&conn).map_err(|e| e.to_string())? }; - - log::info!("linked to {} as {}", base_url, identity.email); Ok(LinkResult { status, - identity, - compatibility: probe.compatibility, + identity: granted.identity, + compatibility: granted.compatibility, }) }