diff --git a/CHANGELOG.md b/CHANGELOG.md index 400e2642b3..7f9882ca1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,15 @@ [Full Changelog](In progress) +## ✨ What's New ✨ + ### Glean - Updated to v68.0.0 ([#7438](https://github.com/mozilla/application-services/issues/7438)) +### Logins + +- Add `LoginStore.bridgedEngine()`, which exposes the logins sync engine to Desktop's Sync. ([bug 2049263](https://bugzilla.mozilla.org/show_bug.cgi?id=2049263)) + # v153.0 (_2026-06-15_) ## ⚠️ Breaking Changes ⚠️ diff --git a/components/logins/src/error.rs b/components/logins/src/error.rs index 6198bd07a5..bc183effee 100644 --- a/components/logins/src/error.rs +++ b/components/logins/src/error.rs @@ -195,6 +195,17 @@ impl GetErrorHandling for Error { } } +// The bridged sync engine (`sync::bridge`) deals in `anyhow::Result`, as that's +// what the `sync15` BridgedEngine traits use. This lets UniFFI map those errors +// onto our public error type when the bridge methods are exposed via the UDL. +impl From for LoginsApiError { + fn from(value: anyhow::Error) -> Self { + LoginsApiError::UnexpectedLoginsApiError { + reason: value.to_string(), + } + } +} + impl From for LoginsApiError { fn from(error: uniffi::UnexpectedUniFFICallbackError) -> Self { LoginsApiError::UnexpectedLoginsApiError { diff --git a/components/logins/src/lib.rs b/components/logins/src/lib.rs index 47707ca92a..cd430b4e13 100644 --- a/components/logins/src/lib.rs +++ b/components/logins/src/lib.rs @@ -29,7 +29,7 @@ use crate::encryption::{check_canary, create_canary, create_key}; pub use crate::error::*; pub use crate::login::*; pub use crate::store::*; -pub use crate::sync::LoginsSyncEngine; +pub use crate::sync::{LoginsBridgedEngine, LoginsSyncEngine}; use std::sync::Arc; // Utility function to create a StaticKeyManager to be used for the time being until support lands diff --git a/components/logins/src/logins.udl b/components/logins/src/logins.udl index 8d306b8886..3feea9642d 100644 --- a/components/logins/src/logins.udl +++ b/components/logins/src/logins.udl @@ -301,10 +301,58 @@ interface LoginStore { [Self=ByArc] void register_with_sync_manager(); + /// Returns a bridged sync engine for Desktop's Sync framework. + /// Without this UDL entry the engine is invisible to JS: UniFFI generates + /// the XPCOM glue that lets JS call `rustStore.bridgedEngine()`. + [Throws=LoginsApiError, Self=ByArc] + LoginsBridgedEngine bridged_engine(); + [Self=ByArc] void shutdown(); }; +/// The Desktop-facing bridged sync engine. The canonical docs are in +/// https://searchfox.org/mozilla-central/source/services/interfaces/mozIBridgedSyncEngine.idl +/// It's only actually used on Desktop, but it's fine to expose this everywhere. +/// NOTE: all timestamps here are milliseconds. +interface LoginsBridgedEngine { + [Throws=LoginsApiError] + i64 last_sync(); + + [Throws=LoginsApiError] + void set_last_sync(i64 last_sync); + + [Throws=LoginsApiError] + string? sync_id(); + + [Throws=LoginsApiError] + string reset_sync_id(); + + [Throws=LoginsApiError] + string ensure_current_sync_id([ByRef]string new_sync_id); + + [Throws=LoginsApiError] + void sync_started(); + + [Throws=LoginsApiError] + void store_incoming(sequence incoming_envelopes_as_json); + + [Throws=LoginsApiError] + sequence apply(); + + [Throws=LoginsApiError] + void set_uploaded(i64 new_timestamp, sequence uploaded_ids); + + [Throws=LoginsApiError] + void sync_finished(); + + [Throws=LoginsApiError] + void reset(); + + [Throws=LoginsApiError] + void wipe(); +}; + dictionary RunMaintenanceOptions { // Wipe un-decryptable logins. These will hopefully come back on the next sync. boolean delete_undecryptable_records_for_remote_replacement=true; diff --git a/components/logins/src/sync/bridge.rs b/components/logins/src/sync/bridge.rs new file mode 100644 index 0000000000..ff45c53cfc --- /dev/null +++ b/components/logins/src/sync/bridge.rs @@ -0,0 +1,275 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +use crate::sync::engine::LoginsSyncEngine; +use crate::LoginStore; +use anyhow::Result; +use std::sync::Arc; +use sync15::bso::{IncomingBso, OutgoingBso}; +use sync15::engine::{BridgedEngine, BridgedEngineAdaptor}; +use sync15::ServerTimestamp; +use sync_guid::Guid as SyncGuid; + +impl LoginStore { + /// Returns a bridged sync engine for Desktop for this store. + /// + /// Unlike Tabs, constructing a `LoginsSyncEngine` locks the DB and can + /// fail, so this is fallible (and exposed as `[Throws]` in the UDL). The + /// internal error is surfaced via `anyhow`, which UniFFI maps onto + /// `LoginsApiError` through `From`. + pub fn bridged_engine(self: Arc) -> Result> { + let engine = LoginsSyncEngine::new(self)?; + let bridged_engine = LoginsBridgedEngineAdaptor { engine }; + Ok(Arc::new(LoginsBridgedEngine::new(Box::new(bridged_engine)))) + } +} + +/// `LoginsSyncEngine` only implements the internal `sync15::SyncEngine` trait, +/// which is what the mobile (Android/iOS) sync manager drives. Desktop's Sync +/// framework instead speaks the `mozIBridgedSyncEngine` interface, whose Rust +/// shape is `sync15::BridgedEngine`. This adaptor wraps our `SyncEngine` and, +/// via the blanket `impl BridgedEngine for A`, gives +/// us a `BridgedEngine` for free. The adaptor exists only because these two +/// sync-engine traits still live side by side; it can go away if they're ever +/// unified. +struct LoginsBridgedEngineAdaptor { + engine: LoginsSyncEngine, +} + +/// see sync15/src/engine/bridged_engine.rs for required functions for the trait +impl BridgedEngineAdaptor for LoginsBridgedEngineAdaptor { + fn last_sync(&self) -> Result { + // `get_last_sync` takes the `&LoginDb` to avoid deadlocking when called + // mid-sync (while the lock is already held). The bridge methods are + // always called outside a sync transaction, so we can lock here. + let db = self.engine.store.lock_db()?; + Ok(self + .engine + .get_last_sync(&db)? + .unwrap_or_default() + .as_millis()) + } + + fn set_last_sync(&self, last_sync_millis: i64) -> Result<()> { + let db = self.engine.store.lock_db()?; + self.engine + .set_last_sync(&db, ServerTimestamp::from_millis(last_sync_millis))?; + Ok(()) + } + + fn engine(&self) -> &dyn sync15::engine::SyncEngine { + &self.engine + } +} + +// This is what UniFFI exposes; it does nothing other than delegate back to the +// `BridgedEngine` trait object (and handle the JSON (de)serialization of BSOs +// that crosses the FFI boundary). +/// see services/interfaces/mozIBridgedSyncEngine.idl for contract +pub struct LoginsBridgedEngine { + bridge_impl: Box, +} + +impl LoginsBridgedEngine { + pub fn new(bridge_impl: Box) -> Self { + Self { bridge_impl } + } + + pub fn last_sync(&self) -> Result { + self.bridge_impl.last_sync() + } + + pub fn set_last_sync(&self, last_sync: i64) -> Result<()> { + self.bridge_impl.set_last_sync(last_sync) + } + + pub fn sync_id(&self) -> Result> { + self.bridge_impl.sync_id() + } + + pub fn reset_sync_id(&self) -> Result { + self.bridge_impl.reset_sync_id() + } + + pub fn ensure_current_sync_id(&self, sync_id: &str) -> Result { + self.bridge_impl.ensure_current_sync_id(sync_id) + } + + pub fn sync_started(&self) -> Result<()> { + self.bridge_impl.sync_started() + } + + // Decode the JSON-encoded IncomingBso's that UniFFI passes to us + fn convert_incoming_bsos(&self, incoming: Vec) -> Result> { + let mut bsos = Vec::with_capacity(incoming.len()); + for inc in incoming { + bsos.push(serde_json::from_str::(&inc)?); + } + Ok(bsos) + } + + // Encode OutgoingBso's into JSON for UniFFI + fn convert_outgoing_bsos(&self, outgoing: Vec) -> Result> { + let mut bsos = Vec::with_capacity(outgoing.len()); + for e in outgoing { + bsos.push(serde_json::to_string(&e)?); + } + Ok(bsos) + } + + pub fn store_incoming(&self, incoming: Vec) -> Result<()> { + self.bridge_impl + .store_incoming(self.convert_incoming_bsos(incoming)?) + } + + pub fn apply(&self) -> Result> { + let apply_results = self.bridge_impl.apply()?; + self.convert_outgoing_bsos(apply_results.records) + } + + pub fn set_uploaded(&self, server_modified_millis: i64, guids: Vec) -> Result<()> { + // UniFFI hands us plain strings; the bridge works in terms of `Guid`. + let guids: Vec = guids.into_iter().map(SyncGuid::from).collect(); + self.bridge_impl + .set_uploaded(server_modified_millis, &guids) + } + + pub fn sync_finished(&self) -> Result<()> { + self.bridge_impl.sync_finished() + } + + pub fn reset(&self) -> Result<()> { + self.bridge_impl.reset() + } + + pub fn wipe(&self) -> Result<()> { + self.bridge_impl.wipe() + } +} + +#[cfg(not(feature = "keydb"))] +#[cfg(test)] +mod tests { + use super::*; + use crate::db::test_utils::insert_login; + use nss_as::ensure_initialized; + use std::collections::HashMap; + + // Exercises the sync-metadata plumbing (last_sync / sync_id / reset) that + // Desktop's Sync framework drives through the bridge, mirroring the Tabs + // `test_sync_meta` test. + #[test] + fn test_sync_meta() { + ensure_initialized(); + error_support::init_for_tests(); + + let store = Arc::new(LoginStore::new_in_memory()); + let bridge = store.bridged_engine().expect("should create bridge"); + + // Fresh DB: never synced. + assert_eq!(bridge.last_sync().unwrap(), 0); + bridge.set_last_sync(3).unwrap(); + assert_eq!(bridge.last_sync().unwrap(), 3); + + assert!(bridge.sync_id().unwrap().is_none()); + + bridge.ensure_current_sync_id("some_guid").unwrap(); + assert_eq!(bridge.sync_id().unwrap(), Some("some_guid".to_string())); + // changing the sync ID should reset the timestamp + assert_eq!(bridge.last_sync().unwrap(), 0); + bridge.set_last_sync(3).unwrap(); + + bridge.reset_sync_id().unwrap(); + // should now be a random guid. + assert_ne!(bridge.sync_id().unwrap(), Some("some_guid".to_string())); + // should have reset the last sync timestamp. + assert_eq!(bridge.last_sync().unwrap(), 0); + bridge.set_last_sync(3).unwrap(); + + // `reset` clears the guid and the timestamp + bridge.reset().unwrap(); + assert_eq!(bridge.last_sync().unwrap(), 0); + assert!(bridge.sync_id().unwrap().is_none()); + } + + // A roundtrip through the bridge's data path: stage an incoming remote + // login, apply it, and confirm the local-only login comes back out for + // upload. Unlike `test_sync_meta`, this exercises the JSON (de)serialization + // of BSOs and the staged-incoming `Mutex`. Mirrors the Tabs + // `test_sync_via_bridge` test. + #[test] + fn test_sync_via_bridge() { + ensure_initialized(); + error_support::init_for_tests(); + + let store = Arc::new(LoginStore::new_in_memory()); + + // A local-only login: nothing on the server knows about it yet, so it + // should be uploaded. + insert_login( + &store.lock_db().unwrap(), + "local-only-aaaa", + Some("local-password"), + None, + ); + + let bridge = store + .clone() + .bridged_engine() + .expect("should create bridge"); + + bridge.sync_started().unwrap(); + + // An incoming remote login that isn't known locally. We build the + // envelope as raw JSON, exactly as the JS bridge hands it to us. + let incoming = vec![serde_json::json!({ + "id": "remote-only-bbbb", + "modified": 0, + "payload": serde_json::json!({ + "id": "remote-only-bbbb", + "hostname": "https://remote.example.com", + "formSubmitURL": "https://remote.example.com", + "username": "remote-user", + "password": "remote-password", + }) + .to_string(), + }) + .to_string()]; + bridge + .store_incoming(incoming) + .expect("should store incoming"); + + // Applying stores the remote record locally and returns the local-only + // login for upload. + let outgoing = bridge.apply().expect("should apply"); + let changes: HashMap = outgoing + .into_iter() + .map(|s| { + let bso: serde_json::Value = serde_json::from_str(&s).unwrap(); + let payload: serde_json::Value = + serde_json::from_str(bso["payload"].as_str().unwrap()).unwrap(); + (payload["id"].as_str().unwrap().to_string(), payload) + }) + .collect(); + + // Only the local login is outgoing; the just-applied remote one is not + // re-uploaded. + assert_eq!(changes.len(), 1); + assert_eq!(changes["local-only-aaaa"]["password"], "local-password"); + + // The incoming remote login was actually persisted. + let stored = store + .get("remote-only-bbbb") + .unwrap() + .expect("remote login should have been stored"); + assert_eq!(stored.password, "remote-password"); + + // Acknowledging the upload advances last_sync. + bridge + .set_uploaded(1234, vec!["local-only-aaaa".to_string()]) + .unwrap(); + bridge.sync_finished().unwrap(); + assert_eq!(bridge.last_sync().unwrap(), 1234); + } +} diff --git a/components/logins/src/sync/engine.rs b/components/logins/src/sync/engine.rs index 6e816ac719..c1e1a538d2 100644 --- a/components/logins/src/sync/engine.rs +++ b/components/logins/src/sync/engine.rs @@ -16,21 +16,29 @@ use crate::LoginStore; use interrupt_support::SqlInterruptScope; use rusqlite::named_params; use sql_support::ConnExt; -use std::cell::RefCell; use std::collections::HashSet; -use std::sync::Arc; +use std::sync::{Arc, Mutex}; use std::time::{Duration, UNIX_EPOCH}; use sync15::bso::{IncomingBso, OutgoingBso, OutgoingEnvelope}; use sync15::engine::{CollSyncIds, CollectionRequest, EngineSyncAssociation, SyncEngine}; use sync15::{telemetry, ServerTimestamp}; use sync_guid::Guid; +// The Desktop FxA session-credentials pseudo-login. Firefox stores its account +// credentials as a login under this origin; it must never be synced. This +// mirrors the exclusion the JS `PasswordEngine` does via +// `Utils.getSyncCredentialsHosts()`. Only relevant on Desktop (mobile never has +// such a login), but it's harmless to filter everywhere. +const FXA_CREDENTIALS_ORIGIN: &str = "chrome://FirefoxAccounts"; + // The sync engine. pub struct LoginsSyncEngine { pub store: Arc, pub scope: SqlInterruptScope, pub encdec: Arc, - pub staged: RefCell>, + // `Mutex` (rather than `RefCell`) so the engine is `Sync`, which the + // Desktop `BridgedEngineAdaptor` requires. Only ever locked briefly. + pub staged: Mutex>, } impl LoginsSyncEngine { @@ -43,7 +51,7 @@ impl LoginsSyncEngine { store, encdec, scope, - staged: RefCell::new(vec![]), + staged: Mutex::new(vec![]), }) } @@ -245,26 +253,31 @@ impl LoginsSyncEngine { let mut stmt = db.prepare_cached(&format!( "SELECT L.*, M.enc_unknown_fields FROM loginsL L LEFT JOIN loginsM M ON L.guid = M.guid - WHERE sync_status IS NOT {synced}", + WHERE sync_status IS NOT {synced} + -- Never sync Desktop's FxA session-credentials pseudo-login. + AND L.origin IS NOT :fxa_origin", synced = SyncStatus::Synced as u8 ))?; - let bsos = stmt.query_and_then([], |row| { - self.scope.err_if_interrupted()?; - Ok(if row.get::<_, bool>("is_deleted")? { - let envelope = OutgoingEnvelope { - id: row.get::<_, String>("guid")?.into(), - sortindex: Some(TOMBSTONE_SORTINDEX), - ..Default::default() - }; - OutgoingBso::new_tombstone(envelope) - } else { - let unknown = row.get::<_, Option>("enc_unknown_fields")?; - let mut bso = - EncryptedLogin::from_row(row)?.into_bso(self.encdec.as_ref(), unknown)?; - bso.envelope.sortindex = Some(DEFAULT_SORTINDEX); - bso - }) - })?; + let bsos = stmt.query_and_then( + named_params! { ":fxa_origin": FXA_CREDENTIALS_ORIGIN }, + |row| { + self.scope.err_if_interrupted()?; + Ok(if row.get::<_, bool>("is_deleted")? { + let envelope = OutgoingEnvelope { + id: row.get::<_, String>("guid")?.into(), + sortindex: Some(TOMBSTONE_SORTINDEX), + ..Default::default() + }; + OutgoingBso::new_tombstone(envelope) + } else { + let unknown = row.get::<_, Option>("enc_unknown_fields")?; + let mut bso = + EncryptedLogin::from_row(row)?.into_bso(self.encdec.as_ref(), unknown)?; + bso.envelope.sortindex = Some(DEFAULT_SORTINDEX); + bso + }) + }, + )?; bsos.collect::>() } @@ -292,9 +305,13 @@ impl LoginsSyncEngine { db.put_meta(schema::LAST_SYNC_META_KEY, &last_sync_millis) } - fn get_last_sync(&self, db: &LoginDb) -> Result> { - let millis = db.get_meta::(schema::LAST_SYNC_META_KEY)?.unwrap(); - Ok(Some(ServerTimestamp(millis))) + // Public so the bridged engine (`sync::bridge`) can read the last-sync + // timestamp without needing access to the private internals here. Returns + // `None` when we've never synced, rather than panicking on a fresh DB. + pub fn get_last_sync(&self, db: &LoginDb) -> Result> { + Ok(db + .get_meta::(schema::LAST_SYNC_META_KEY)? + .map(ServerTimestamp)) } fn mark_as_synchronized(&self, guids: &[&str], ts: ServerTimestamp) -> Result<()> { @@ -424,7 +441,7 @@ impl SyncEngine for LoginsSyncEngine { ) -> anyhow::Result<()> { // We don't have cross-item dependencies like bookmarks does, so we can // just apply now instead of "staging" - self.staged.borrow_mut().append(&mut inbound); + self.staged.lock().unwrap().append(&mut inbound); Ok(()) } @@ -433,7 +450,7 @@ impl SyncEngine for LoginsSyncEngine { timestamp: ServerTimestamp, telem: &mut telemetry::Engine, ) -> anyhow::Result> { - let inbound = (*self.staged.borrow_mut()).drain(..).collect(); + let inbound = self.staged.lock().unwrap().drain(..).collect(); Ok(self.do_apply_incoming(inbound, timestamp, telem)?) } @@ -803,6 +820,44 @@ mod tests { assert!(changes["changed"].get("deleted").is_none()); } + #[test] + fn test_fetch_outgoing_excludes_fxa_credentials() { + ensure_initialized(); + let store = LoginStore::new_in_memory(); + + // A normal local login that should be uploaded. + insert_login(&store.lock_db().unwrap(), "normal", Some("password"), None); + + // Desktop's FxA session-credentials pseudo-login must never be synced. + store + .add(LoginEntry { + origin: FXA_CREDENTIALS_ORIGIN.to_string(), + http_realm: Some("Firefox Accounts credentials".to_string()), + username: "uid".to_string(), + password: "sync-token".to_string(), + ..Default::default() + }) + .unwrap(); + + let changeset = run_fetch_outgoing(store); + let changes: HashMap = changeset + .into_iter() + .map(|b| { + ( + b.envelope.id.to_string(), + serde_json::from_str(&b.payload).unwrap(), + ) + }) + .collect(); + + // The normal login still uploads; nothing pointing at the FxA origin + // is outgoing. + assert!(changes.contains_key("normal")); + assert!(changes + .values() + .all(|payload| payload["hostname"] != FXA_CREDENTIALS_ORIGIN)); + } + #[test] fn test_bad_record() { ensure_initialized(); diff --git a/components/logins/src/sync/mod.rs b/components/logins/src/sync/mod.rs index 7e8ac45542..0ea4c679ae 100644 --- a/components/logins/src/sync/mod.rs +++ b/components/logins/src/sync/mod.rs @@ -2,11 +2,13 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +mod bridge; mod engine; pub(crate) mod merge; mod payload; mod update_plan; +pub use bridge::LoginsBridgedEngine; pub use engine::LoginsSyncEngine; use payload::{IncomingLogin, LoginPayload};