diff --git a/crates/ely_app/src/shell/auth.rs b/crates/ely_app/src/shell/auth.rs index 18cb59a..3f39a9c 100644 --- a/crates/ely_app/src/shell/auth.rs +++ b/crates/ely_app/src/shell/auth.rs @@ -7,11 +7,14 @@ //! so the existing 8 ms tick is the single point that reconciles //! background-task state with `BrowserCore`. -use std::sync::mpsc::Sender; +use std::{path::Path, sync::mpsc::Sender}; use ely_browser_core::SyncEngine; -use ely_domain::{ProfileId, ProfileKind}; -use ely_sync_client::{ApiClientConfig, BearerToken, send_email_otp, verify_email_otp}; +use ely_domain::ProfileId; +use ely_sync_client::{ + ApiClientConfig, BearerToken, BearerTokenStore, SyncClientError, send_email_otp, + verify_email_otp, +}; use gpui::Context; use crate::services::servo_profile_data::{default_profile_data_root, sync_profile_data_dir}; @@ -63,6 +66,9 @@ impl ElyShell { /// through the shared `SyncStateUpdate` channel, which the next /// shell tick reconciles into the `auth_flow_phase`. pub(crate) fn submit_email_otp_request(&mut self, cx: &mut Context) { + if active_profile_sync_context_for(&self.state).is_none() { + return; + } let email = self.read_auth_email_input(cx); let Some(email) = normalize_email(&email) else { self.auth_flow_phase = AuthFlowPhase::Error { @@ -115,21 +121,16 @@ impl ElyShell { /// call to make, the token is the only artefact we own. pub(crate) fn submit_sign_out(&mut self, _cx: &mut Context) { self.auth_flow_phase = AuthFlowPhase::Idle; - let active_profile = match active_profile_sync_context_for(&self.state) { - Some(profile) => profile, + let active_profile_id = match active_profile_id_for(&self.state) { + Some(profile_id) => profile_id, None => return, }; let Some(profile_root) = default_profile_data_root() else { return; }; - let profile_dir = sync_profile_data_dir(&profile_root, &active_profile.id); - match SyncEngine::for_profile_dir(&profile_dir, "ELY", sync_platform_label()) { - Ok(mut engine) => { - let _ = engine.install_bearer(""); - } - Err(error) => { - tracing::warn!(target: "ely::sync", error = %error, "sign-out failed to load engine"); - } + let profile_dir = sync_profile_data_dir(&profile_root, &active_profile_id); + if let Err(error) = clear_persisted_bearer(&profile_dir) { + tracing::warn!(target: "ely::sync", error = %error, "sign-out failed to clear bearer"); } if let ShellState::Ready(core) = &mut self.state { core.set_sync_connection_state(ely_domain::SyncConnectionState::SignedOut); @@ -156,19 +157,27 @@ fn normalize_email(raw: &str) -> Option { #[derive(Clone, Debug, Eq, PartialEq)] struct ActiveProfileSyncContext { id: ProfileId, - name: String, - kind: ProfileKind, } fn active_profile_sync_context_for(state: &ShellState) -> Option { let ShellState::Ready(core) = state else { return None; }; - core.snapshot().ok().map(|snapshot| ActiveProfileSyncContext { - id: snapshot.active_profile_id, - name: snapshot.active_profile_name, - kind: snapshot.active_profile_kind, - }) + if !core.active_profile_allows_sync() { + return None; + } + active_profile_id_for(state).map(|id| ActiveProfileSyncContext { id }) +} + +fn active_profile_id_for(state: &ShellState) -> Option { + let ShellState::Ready(core) = state else { + return None; + }; + core.snapshot().ok().map(|snapshot| snapshot.active_profile_id) +} + +pub(super) fn clear_persisted_bearer(profile_dir: &Path) -> Result<(), SyncClientError> { + BearerTokenStore::new(profile_dir.join("sync").join("bearer.token")).clear() } fn spawn_send_otp(email: String, tx: Sender) { @@ -237,7 +246,10 @@ fn spawn_verify_otp( #[cfg(test)] mod tests { - use super::{AuthFlowPhase, normalize_email}; + use ely_browser_core::{BrowserCore, InitialBrowserConfig}; + + use super::{AuthFlowPhase, active_profile_sync_context_for, normalize_email}; + use crate::shell::ShellState; #[test] fn normalize_lowercases_and_trims() { @@ -265,4 +277,18 @@ mod tests { assert_eq!(phase.error_message(), Some("rate limited")); assert!(!phase.is_busy()); } + + #[test] + fn private_profile_has_no_sync_auth_context() -> Result<(), Box> { + let state = + ShellState::Ready(Box::new(BrowserCore::new(InitialBrowserConfig::private_window()?)?)); + + assert_eq!(active_profile_sync_context_for(&state), None); + + let state = + ShellState::Ready(Box::new(BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?)); + assert!(active_profile_sync_context_for(&state).is_some()); + + Ok(()) + } } diff --git a/crates/ely_app/src/shell/internal_pages/sync.rs b/crates/ely_app/src/shell/internal_pages/sync.rs index 7adaa16..0d8c507 100644 --- a/crates/ely_app/src/shell/internal_pages/sync.rs +++ b/crates/ely_app/src/shell/internal_pages/sync.rs @@ -1,6 +1,6 @@ use ely_browser_core::BrowserSnapshot; use ely_design_system::colors; -use ely_domain::{SyncConnectionState, SyncObjectKind, SyncObjectStatus}; +use ely_domain::{ProfileKind, SyncConnectionState, SyncObjectKind, SyncObjectStatus}; use gpui::{ AnyElement, Context, FontWeight, IntoElement, ParentElement, Styled, div, px, rgb, rgba, }; @@ -36,25 +36,48 @@ fn render_sync_body( snapshot: &BrowserSnapshot, cx: &mut Context, ) -> AnyElement { + let body = div().max_w(px(860.0)).flex().flex_col().gap(px(18.0)).child( + div() + .text_size(px(26.0)) + .font_weight(FontWeight(500.0)) + .text_color(rgb(colors::ink())) + .child("Sync"), + ); + if !profile_allows_sync_controls(&snapshot.active_profile_kind) { + return body.child(render_private_profile_card()).into_any_element(); + } + body.child( + div() + .grid() + .grid_cols(2) + .gap(px(18.0)) + .child(render_account_card(shell, snapshot, cx)) + .child(render_data_card(shell, snapshot, cx)), + ) + .into_any_element() +} + +fn profile_allows_sync_controls(profile_kind: &ProfileKind) -> bool { + profile_kind == &ProfileKind::Standard +} + +fn render_private_profile_card() -> AnyElement { div() - .max_w(px(860.0)) + .p(px(16.0)) + .rounded(px(12.0)) + .bg(rgba(card_bg())) .flex() .flex_col() - .gap(px(18.0)) + .gap(px(8.0)) .child( div() - .text_size(px(26.0)) + .text_size(px(14.0)) .font_weight(FontWeight(500.0)) .text_color(rgb(colors::ink())) - .child("Sync"), + .child("Private profile"), ) .child( - div() - .grid() - .grid_cols(2) - .gap(px(18.0)) - .child(render_account_card(shell, snapshot, cx)) - .child(render_data_card(shell, snapshot, cx)), + div().text_size(px(14.0)).text_color(rgb(colors::ink_3())).child("Local session only"), ) .into_any_element() } @@ -241,3 +264,16 @@ fn sync_object_kind_label(kind: SyncObjectKind) -> &'static str { fn card_bg() -> u32 { colors::pick(0xffffffd9, 0x1f1d1bd9) } + +#[cfg(test)] +mod tests { + use ely_domain::ProfileKind; + + use super::profile_allows_sync_controls; + + #[test] + fn private_profile_hides_sync_controls() { + assert!(!profile_allows_sync_controls(&ProfileKind::Private)); + assert!(profile_allows_sync_controls(&ProfileKind::Standard)); + } +} diff --git a/crates/ely_app/src/shell/settings_actions.rs b/crates/ely_app/src/shell/settings_actions.rs index 05309c6..4d81160 100644 --- a/crates/ely_app/src/shell/settings_actions.rs +++ b/crates/ely_app/src/shell/settings_actions.rs @@ -255,6 +255,15 @@ impl ElyShell { } fn trigger_cloud_sync_upload_with_clock_floor(&mut self, logical_clock_floor: Option) { + let active_profile_allows_sync = match &self.state { + ShellState::Ready(core) => core.active_profile_allows_sync(), + ShellState::StartupError(_) => false, + }; + if !active_profile_allows_sync { + self.sync_upload_scheduled = false; + self.clear_pending_cloud_sync_upload(); + return; + } if self.sync_upload_in_flight { self.sync_upload_scheduled = false; self.queue_cloud_sync_upload(logical_clock_floor); diff --git a/crates/ely_app/src/shell/sync_state.rs b/crates/ely_app/src/shell/sync_state.rs index 41c7772..c4ebb01 100644 --- a/crates/ely_app/src/shell/sync_state.rs +++ b/crates/ely_app/src/shell/sync_state.rs @@ -88,7 +88,7 @@ impl ElyShell { true } - fn clear_pending_cloud_sync_upload(&mut self) { + pub(super) fn clear_pending_cloud_sync_upload(&mut self) { self.sync_upload_pending = false; self.sync_upload_pending_logical_clock_floor = None; } @@ -103,34 +103,14 @@ impl ElyShell { /// Inspect the on-disk bearer token and seed `SyncConnectionState` /// so the Sync settings page reads the startup state on first render. pub(super) fn probe_initial_sync_state(&mut self) -> bool { - let ShellState::Ready(core) = &mut self.state else { - return false; - }; - let Some(snapshot) = core.snapshot().ok() else { - return false; - }; let Some(profile_root) = crate::services::servo_profile_data::default_profile_data_root() else { return false; }; - let profile_dir = crate::services::servo_profile_data::sync_profile_data_dir( - &profile_root, - &snapshot.active_profile_id, - ); - if snapshot.active_profile_name == "Default" - && matches!(snapshot.active_profile_kind, ProfileKind::Standard) - { - migrate_legacy_default_sync_dir(&profile_root, &profile_dir); - } - let bearer_path = profile_dir.join("sync").join("bearer.token"); - let bearer_present = bearer_token_file_present(&bearer_path); - let state = if bearer_present { - ely_domain::SyncConnectionState::SignedIn - } else { - ely_domain::SyncConnectionState::SignedOut + let ShellState::Ready(core) = &mut self.state else { + return false; }; - core.set_sync_connection_state(state); - bearer_present + probe_initial_sync_state_at(core, &profile_root) } /// Drain any sync upload outcomes the off-thread worker pushed @@ -225,6 +205,40 @@ impl ElyShell { } } +fn probe_initial_sync_state_at( + core: &mut ely_browser_core::BrowserCore, + profile_root: &Path, +) -> bool { + let Some(snapshot) = core.snapshot().ok() else { + return false; + }; + let profile_dir = crate::services::servo_profile_data::sync_profile_data_dir( + profile_root, + &snapshot.active_profile_id, + ); + if !core.active_profile_allows_sync() { + if let Err(error) = auth::clear_persisted_bearer(&profile_dir) { + tracing::warn!(target: "ely::sync", error = %error, "private bearer cleanup failed"); + } + core.set_sync_connection_state(SyncConnectionState::SignedOut); + return false; + } + if snapshot.active_profile_name == "Default" + && matches!(snapshot.active_profile_kind, ProfileKind::Standard) + { + migrate_legacy_default_sync_dir(profile_root, &profile_dir); + } + let bearer_path = profile_dir.join("sync").join("bearer.token"); + let bearer_present = bearer_token_file_present(&bearer_path); + let state = if bearer_present { + ely_domain::SyncConnectionState::SignedIn + } else { + ely_domain::SyncConnectionState::SignedOut + }; + core.set_sync_connection_state(state); + bearer_present +} + fn bearer_token_file_present(path: &Path) -> bool { std::fs::metadata(path).map(|metadata| metadata.len() > 0).unwrap_or(false) } @@ -266,7 +280,13 @@ fn copy_dir_recursive(source: &Path, destination: &Path) -> std::io::Result<()> #[cfg(test)] mod tests { - use super::{bearer_token_file_present, migrate_legacy_default_sync_dir}; + use ely_browser_core::{BrowserCore, InitialBrowserConfig}; + use ely_domain::SyncConnectionState; + + use super::{ + bearer_token_file_present, migrate_legacy_default_sync_dir, probe_initial_sync_state_at, + }; + use crate::services::servo_profile_data::sync_profile_data_dir; #[test] fn bearer_token_file_presence_requires_bytes() -> Result<(), Box> { @@ -315,4 +335,41 @@ mod tests { assert!(!stable.join("sync/bearer.token").exists()); Ok(()) } + + #[test] + fn private_startup_clears_a_persisted_bearer() -> Result<(), Box> { + let directory = tempfile::tempdir()?; + let mut core = BrowserCore::new(InitialBrowserConfig::private_window()?)?; + let profile_id = core.snapshot()?.active_profile_id; + let profile_dir = sync_profile_data_dir(directory.path(), &profile_id); + let bearer_path = profile_dir.join("sync/bearer.token"); + std::fs::create_dir_all(bearer_path.parent().ok_or("missing bearer parent")?)?; + std::fs::write(&bearer_path, "a".repeat(64))?; + std::fs::write(bearer_path.with_extension("tmp"), "b".repeat(64))?; + core.set_sync_connection_state(SyncConnectionState::SignedIn); + + assert!(!probe_initial_sync_state_at(&mut core, directory.path())); + assert!(!bearer_path.exists()); + assert!(!bearer_path.with_extension("tmp").exists()); + assert_eq!(core.snapshot()?.sync_status.connection(), &SyncConnectionState::SignedOut); + + Ok(()) + } + + #[test] + fn standard_startup_preserves_a_persisted_bearer() -> Result<(), Box> { + let directory = tempfile::tempdir()?; + let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + let profile_id = core.snapshot()?.active_profile_id; + let profile_dir = sync_profile_data_dir(directory.path(), &profile_id); + let bearer_path = profile_dir.join("sync/bearer.token"); + std::fs::create_dir_all(bearer_path.parent().ok_or("missing bearer parent")?)?; + std::fs::write(&bearer_path, "a".repeat(64))?; + + assert!(probe_initial_sync_state_at(&mut core, directory.path())); + assert!(bearer_path.exists()); + assert_eq!(core.snapshot()?.sync_status.connection(), &SyncConnectionState::SignedIn); + + Ok(()) + } } diff --git a/crates/ely_browser_core/src/state/bookmarks.rs b/crates/ely_browser_core/src/state/bookmarks.rs index 8792b29..3fb1b84 100644 --- a/crates/ely_browser_core/src/state/bookmarks.rs +++ b/crates/ely_browser_core/src/state/bookmarks.rs @@ -251,7 +251,10 @@ impl BrowserCore { } self.bookmarks .iter() - .filter(|bookmark| self.profile_allows_cloud_sync(bookmark.profile_id())) + .filter(|bookmark| { + self.profile_allows_cloud_sync(bookmark.profile_id()) + && self.space_allows_sync(bookmark.space_id()) + }) .collect() } diff --git a/crates/ely_browser_core/src/state/sync.rs b/crates/ely_browser_core/src/state/sync.rs index f2e07b6..4586320 100644 --- a/crates/ely_browser_core/src/state/sync.rs +++ b/crates/ely_browser_core/src/state/sync.rs @@ -90,19 +90,20 @@ impl BrowserCore { #[must_use] pub fn cloud_sync_upload_enabled(&self) -> bool { - matches!( - self.sync_connection_state, - SyncConnectionState::SignedIn - | SyncConnectionState::AwaitingDeviceApproval - | SyncConnectionState::SyncReady { .. } - | SyncConnectionState::SyncError { .. } - ) + self.active_profile_allows_sync() + && matches!( + self.sync_connection_state, + SyncConnectionState::SignedIn + | SyncConnectionState::AwaitingDeviceApproval + | SyncConnectionState::SyncReady { .. } + | SyncConnectionState::SyncError { .. } + ) } pub(crate) fn sync_space_name_for(&self, space_id: &SpaceId) -> Option { self.spaces .iter() - .find(|space| space.id() == space_id) + .find(|space| space.id() == space_id && self.space_allows_sync(space.id())) .map(|space| space.name().to_string()) } @@ -180,7 +181,11 @@ impl BrowserCore { } self.tabs .iter() - .filter(|tab| tab.sync_enabled() && self.profile_allows_cloud_sync(tab.profile_id())) + .filter(|tab| { + tab.sync_enabled() + && self.profile_allows_cloud_sync(tab.profile_id()) + && self.space_allows_sync(tab.space_id()) + }) .collect() } @@ -188,7 +193,7 @@ impl BrowserCore { if self.sync_object_policy(SyncObjectKind::Spaces) == SyncObjectPolicy::Paused { return Vec::new(); } - self.spaces.iter().collect() + self.spaces.iter().filter(|space| self.space_allows_sync(space.id())).collect() } pub(super) fn apply_space_sync_record( @@ -200,11 +205,15 @@ impl BrowserCore { let space_id = parse_space_id(&record.id)?; let default_profile_id = self.sync_profile_id(&record.default_profile_id, context)?; let archive_policy = ArchivePolicy::from(record.archive_policy.clone()); - let existing_index = - self.spaces.iter().position(|space| space.id() == &space_id).or_else(|| { - self.spaces - .iter() - .position(|space| space.name().eq_ignore_ascii_case(record.name.trim())) + let existing_index = self + .spaces + .iter() + .position(|space| space.id() == &space_id && self.space_allows_sync(space.id())) + .or_else(|| { + self.spaces.iter().position(|space| { + space.name().eq_ignore_ascii_case(record.name.trim()) + && self.space_allows_sync(space.id()) + }) }); match existing_index { @@ -382,20 +391,20 @@ impl BrowserCore { context: &SyncSnapshotApplyContext, ) -> Result { let profile_id = ProfileId::parse(raw).map_err(snapshot_schema_error)?; - if let Some(local_profile_id) = context.profile_alias(&profile_id) { + let local_profile_id = context + .profile_alias(&profile_id) + .or_else(|| { + self.profiles + .iter() + .any(|profile| profile.id() == &profile_id) + .then_some(profile_id) + }) + .unwrap_or_else(|| self.active_profile_id.clone()); + if self.profile_allows_sync(&local_profile_id) { return Ok(local_profile_id); } - if self.profiles.iter().any(|profile| profile.id() == &profile_id) { - return Ok(profile_id); - } - Ok(self.active_profile_id.clone()) - } - - pub(super) fn profile_allows_cloud_sync(&self, profile_id: &ProfileId) -> bool { - self.profiles.iter().any(|profile| { - profile.id() == profile_id - && profile.allows_sync() - && profile.sync_policy() == ely_domain::ProfileSyncPolicy::Enabled + Err(SyncClientError::SyncPolicy { + reason: "sync record targets a private profile".to_string(), }) } @@ -405,16 +414,22 @@ impl BrowserCore { space_name: Option<&str>, ) -> Result { let space_id = SpaceId::parse(raw).map_err(snapshot_schema_error)?; - if self.spaces.iter().any(|space| space.id() == &space_id) { + if self.space_allows_sync(&space_id) { return Ok(space_id); } if let Some(space_name) = space_name - && let Some(space) = - self.spaces.iter().find(|space| space.name().eq_ignore_ascii_case(space_name)) + && let Some(space) = self.spaces.iter().find(|space| { + space.name().eq_ignore_ascii_case(space_name) && self.space_allows_sync(space.id()) + }) { return Ok(space.id().clone()); } - Ok(self.active_space_id.clone()) + if self.space_allows_sync(&self.active_space_id) { + return Ok(self.active_space_id.clone()); + } + Err(SyncClientError::SyncPolicy { + reason: "sync record targets a private space".to_string(), + }) } fn ensure_synced_tab_indexes( diff --git a/crates/ely_browser_core/src/state/sync_history.rs b/crates/ely_browser_core/src/state/sync_history.rs index eb46881..333f3da 100644 --- a/crates/ely_browser_core/src/state/sync_history.rs +++ b/crates/ely_browser_core/src/state/sync_history.rs @@ -16,7 +16,10 @@ impl BrowserCore { } self.history_entries .iter() - .filter(|entry| self.profile_allows_cloud_sync(entry.profile_id())) + .filter(|entry| { + self.profile_allows_cloud_sync(entry.profile_id()) + && self.space_allows_sync(entry.space_id()) + }) .collect() } diff --git a/crates/ely_browser_core/src/state/sync_notes.rs b/crates/ely_browser_core/src/state/sync_notes.rs index ffeb141..31a396b 100644 --- a/crates/ely_browser_core/src/state/sync_notes.rs +++ b/crates/ely_browser_core/src/state/sync_notes.rs @@ -17,7 +17,13 @@ impl BrowserCore { if self.sync_object_policy(SyncObjectKind::Notes) == SyncObjectPolicy::Paused { return Vec::new(); } - self.notes.iter().filter(|note| self.profile_allows_cloud_sync(note.profile_id())).collect() + self.notes + .iter() + .filter(|note| { + self.profile_allows_cloud_sync(note.profile_id()) + && self.space_allows_sync(note.space_id()) + }) + .collect() } pub(super) fn apply_note_sync_record( diff --git a/crates/ely_browser_core/src/state/sync_profiles.rs b/crates/ely_browser_core/src/state/sync_profiles.rs index 3c4ace1..07028d6 100644 --- a/crates/ely_browser_core/src/state/sync_profiles.rs +++ b/crates/ely_browser_core/src/state/sync_profiles.rs @@ -1,10 +1,37 @@ -use ely_domain::{Profile, ProfileId, ProfileKind, SyncObjectKind, SyncObjectPolicy}; +use ely_domain::{Profile, ProfileId, ProfileKind, SpaceId, SyncObjectKind, SyncObjectPolicy}; use ely_sync_client::SyncClientError; use super::{BrowserCore, sync::snapshot_schema_error, sync_context::SyncSnapshotApplyContext}; use crate::{sync_engine::SyncSnapshotApplySummary, sync_records::ProfileSyncRecord}; impl BrowserCore { + #[must_use] + pub fn active_profile_allows_sync(&self) -> bool { + self.profile_allows_sync(&self.active_profile_id) + } + + pub(super) fn profile_allows_sync(&self, profile_id: &ProfileId) -> bool { + self.profiles + .iter() + .find(|profile| profile.id() == profile_id) + .is_some_and(|profile| profile.allows_sync()) + } + + pub(super) fn profile_allows_cloud_sync(&self, profile_id: &ProfileId) -> bool { + self.profiles.iter().any(|profile| { + profile.id() == profile_id + && profile.allows_sync() + && profile.sync_policy() == ely_domain::ProfileSyncPolicy::Enabled + }) + } + + pub(super) fn space_allows_sync(&self, space_id: &SpaceId) -> bool { + self.spaces + .iter() + .find(|space| space.id() == space_id) + .is_some_and(|space| self.profile_allows_sync(space.default_profile_id())) + } + pub(crate) fn visible_profiles_for_sync(&self) -> Vec<&Profile> { if self.sync_object_policy(SyncObjectKind::Profiles) == SyncObjectPolicy::Paused { return Vec::new(); @@ -21,8 +48,9 @@ impl BrowserCore { let profile_id = ProfileId::parse(&record.id).map_err(snapshot_schema_error)?; let kind = ProfileKind::from(record.kind); if kind == ProfileKind::Private { - summary.record_skipped(); - return Ok(()); + return Err(SyncClientError::SyncPolicy { + reason: "snapshot contains a private profile".to_string(), + }); } let name = record.name.trim().to_string(); diff --git a/crates/ely_browser_core/src/state/sync_reading_list.rs b/crates/ely_browser_core/src/state/sync_reading_list.rs index 78dbc66..70fd5ca 100644 --- a/crates/ely_browser_core/src/state/sync_reading_list.rs +++ b/crates/ely_browser_core/src/state/sync_reading_list.rs @@ -19,7 +19,10 @@ impl BrowserCore { } self.reading_list .iter() - .filter(|entry| self.profile_allows_cloud_sync(entry.profile_id())) + .filter(|entry| { + self.profile_allows_cloud_sync(entry.profile_id()) + && self.space_allows_sync(entry.space_id()) + }) .collect() } diff --git a/crates/ely_browser_core/src/sync_engine.rs b/crates/ely_browser_core/src/sync_engine.rs index b8aa370..be42321 100644 --- a/crates/ely_browser_core/src/sync_engine.rs +++ b/crates/ely_browser_core/src/sync_engine.rs @@ -291,6 +291,7 @@ impl BrowserCore { /// UI thread does the (synchronous, cheap) serialization before /// handing bytes off to the worker thread. pub fn build_sync_snapshot_bytes(&self) -> Result, SyncClientError> { + self.ensure_active_profile_allows_sync()?; let body = SyncSnapshotBody::from_core(self); serde_json::to_vec(&body).map_err(|error| SyncClientError::Json { endpoint: "snapshot".to_string(), @@ -302,6 +303,7 @@ impl BrowserCore { &mut self, bytes: &[u8], ) -> Result { + self.ensure_active_profile_allows_sync()?; let body: SyncSnapshotBody = serde_json::from_slice(bytes).map_err(|error| { SyncClientError::Json { endpoint: "snapshot".to_string(), source: error } })?; @@ -313,4 +315,11 @@ impl BrowserCore { } self.apply_sync_snapshot_body(body) } + + fn ensure_active_profile_allows_sync(&self) -> Result<(), SyncClientError> { + if self.active_profile_allows_sync() { + return Ok(()); + } + Err(SyncClientError::SyncPolicy { reason: "active profile is private".to_string() }) + } } diff --git a/crates/ely_browser_core/tests/sync_profiles.rs b/crates/ely_browser_core/tests/sync_profiles.rs index 591e328..32dedb0 100644 --- a/crates/ely_browser_core/tests/sync_profiles.rs +++ b/crates/ely_browser_core/tests/sync_profiles.rs @@ -3,8 +3,9 @@ use std::error::Error; use ely_browser_core::{BrowserCore, InitialBrowserConfig}; use ely_domain::{ ProfileKind, ProfileSyncPolicy, SiteOrigin, SitePermissionDecision, SitePermissionFeature, - UrlText, + SyncConnectionState, UrlText, }; +use ely_sync_client::SyncClientError; #[test] fn sync_snapshot_imports_remote_profiles_before_spaces() -> Result<(), Box> { @@ -124,6 +125,7 @@ fn standard_profile_sync_preserves_a_same_named_private_profile() -> Result<(), let mut target = BrowserCore::new(InitialBrowserConfig::private_window()?)?; let private_profile_id = target.snapshot()?.active_profile_id; target.navigate_active_tab(UrlText::parse("https://private.example/secret")?)?; + target.create_profile("Local", 0x26251e, ProfileKind::Standard)?; target.apply_sync_snapshot_bytes(&bytes)?; let snapshot = target.snapshot()?; @@ -135,8 +137,11 @@ fn standard_profile_sync_preserves_a_same_named_private_profile() -> Result<(), && profile.name() == "Private" && profile.kind() == &ProfileKind::Standard })); - let outbound = String::from_utf8(target.build_sync_snapshot_bytes()?)?; + let outbound_bytes = target.build_sync_snapshot_bytes()?; + let outbound = String::from_utf8(outbound_bytes.clone())?; assert!(!outbound.contains("https://private.example/secret")); + assert!(!outbound.contains(private_profile_id.as_str())); + assert!(!snapshot_contains_space_named(&outbound_bytes, "Private")?); assert!(outbound.contains("https://example.com/remote")); Ok(()) } @@ -144,8 +149,17 @@ fn standard_profile_sync_preserves_a_same_named_private_profile() -> Result<(), #[test] fn standard_profile_sync_remaps_a_private_profile_id_collision() -> Result<(), Box> { let mut target = BrowserCore::new(InitialBrowserConfig::private_window()?)?; - let private_profile_id = target.snapshot()?.active_profile_id; + let private_snapshot = target.snapshot()?; + let private_profile_id = private_snapshot.active_profile_id; + let private_space_id = private_snapshot.active_space_id; target.navigate_active_tab(UrlText::parse("https://private.example/id-secret")?)?; + target.create_profile("Local", 0x26251e, ProfileKind::Standard)?; + target.navigate_active_tab(UrlText::parse( + "https://private-space.example/standard-profile-secret", + )?)?; + target.bookmark_active_tab()?; + target.save_active_url_note("private space note")?; + target.save_active_tab_to_reading_list()?; let mut source_config = InitialBrowserConfig::ely_defaults()?; source_config.profile_id = Some(private_profile_id.clone()); @@ -172,8 +186,114 @@ fn standard_profile_sync_remaps_a_private_profile_id_collision() -> Result<(), B .count(), 1 ); - let outbound = String::from_utf8(target.build_sync_snapshot_bytes()?)?; + let outbound_bytes = target.build_sync_snapshot_bytes()?; + let outbound = String::from_utf8(outbound_bytes.clone())?; assert!(!outbound.contains("https://private.example/id-secret")); + assert!(!outbound.contains("https://private-space.example/standard-profile-secret")); + assert!(!outbound.contains(private_profile_id.as_str())); + assert!(!outbound.contains(private_space_id.as_str())); + assert!(!snapshot_contains_space_named(&outbound_bytes, "Private")?); + assert!(!outbound.contains("\"space_name\":\"Private\"")); assert!(outbound.contains("https://example.com/id-remote")); Ok(()) } + +#[test] +fn private_profile_blocks_snapshot_input_and_output() -> Result<(), Box> { + let source = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + let bytes = source.build_sync_snapshot_bytes()?; + let mut private = BrowserCore::new(InitialBrowserConfig::private_window()?)?; + private.set_sync_connection_state(SyncConnectionState::SignedIn); + + assert!(!private.active_profile_allows_sync()); + assert!(!private.cloud_sync_upload_enabled()); + assert!(matches!(private.build_sync_snapshot_bytes(), Err(SyncClientError::SyncPolicy { .. }))); + assert!(matches!( + private.apply_sync_snapshot_bytes(&bytes), + Err(SyncClientError::SyncPolicy { .. }) + )); + + Ok(()) +} + +#[test] +fn sync_snapshot_rejects_records_targeting_a_private_profile() -> Result<(), Box> { + let mut target = BrowserCore::new(InitialBrowserConfig::private_window()?)?; + let private_profile_id = target.snapshot()?.active_profile_id; + target.create_profile("Local", 0x26251e, ProfileKind::Standard)?; + + let mut source_config = InitialBrowserConfig::ely_defaults()?; + source_config.profile_id = Some(private_profile_id); + let source = BrowserCore::new(source_config)?; + let mut document: serde_json::Value = + serde_json::from_slice(&source.build_sync_snapshot_bytes()?)?; + document["profiles"] = serde_json::json!([]); + let bytes = serde_json::to_vec(&document)?; + + assert!(matches!( + target.apply_sync_snapshot_bytes(&bytes), + Err(SyncClientError::SyncPolicy { .. }) + )); + + Ok(()) +} + +#[test] +fn sync_snapshot_blocks_references_to_a_remote_private_profile() -> Result<(), Box> { + let mut source = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + source.navigate_active_tab(UrlText::parse("https://private.example/remote-record")?)?; + let mut document: serde_json::Value = + serde_json::from_slice(&source.build_sync_snapshot_bytes()?)?; + let profiles = document["profiles"].as_array_mut().ok_or("sync profiles must be an array")?; + let profile = profiles.first_mut().ok_or("sync snapshot must include a profile")?; + profile["kind"] = serde_json::json!("private"); + let bytes = serde_json::to_vec(&document)?; + let mut target = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + + assert!(matches!( + target.apply_sync_snapshot_bytes(&bytes), + Err(SyncClientError::SyncPolicy { .. }) + )); + assert!( + target + .snapshot()? + .tabs + .iter() + .all(|tab| tab.url().as_str() != "https://private.example/remote-record") + ); + + Ok(()) +} + +#[test] +fn sync_snapshot_rejects_records_targeting_a_private_space() -> Result<(), Box> { + let mut target = BrowserCore::new(InitialBrowserConfig::private_window()?)?; + let private_space_id = target.snapshot()?.active_space_id; + let standard_profile_id = target.create_profile("Local", 0x26251e, ProfileKind::Standard)?; + + let mut source_config = InitialBrowserConfig::ely_defaults()?; + source_config.profile_id = Some(standard_profile_id); + let source = BrowserCore::new(source_config)?; + let mut document: serde_json::Value = + serde_json::from_slice(&source.build_sync_snapshot_bytes()?)?; + document["profiles"] = serde_json::json!([]); + document["spaces"] = serde_json::json!([]); + for tab in document["tabs"].as_array_mut().ok_or("sync tabs must be an array")? { + tab["space_id"] = serde_json::json!(private_space_id.as_str()); + tab["space_name"] = serde_json::json!("Private"); + } + let bytes = serde_json::to_vec(&document)?; + + assert!(matches!( + target.apply_sync_snapshot_bytes(&bytes), + Err(SyncClientError::SyncPolicy { .. }) + )); + + Ok(()) +} + +fn snapshot_contains_space_named(bytes: &[u8], name: &str) -> Result> { + let document: serde_json::Value = serde_json::from_slice(bytes)?; + let spaces = document["spaces"].as_array().ok_or("sync snapshot spaces must be an array")?; + Ok(spaces.iter().any(|space| space["name"].as_str() == Some(name))) +} diff --git a/crates/ely_sync_client/src/auth.rs b/crates/ely_sync_client/src/auth.rs index 3f198ff..90fb789 100644 --- a/crates/ely_sync_client/src/auth.rs +++ b/crates/ely_sync_client/src/auth.rs @@ -85,11 +85,16 @@ impl BearerTokenStore { } pub fn clear(&self) -> Result<(), SyncClientError> { - match fs::remove_file(&self.path) { - Ok(()) => Ok(()), - Err(error) if error.kind() == ErrorKind::NotFound => Ok(()), - Err(error) => Err(SyncClientError::TokenStorage(error.to_string())), - } + remove_file_if_present(&self.path)?; + remove_file_if_present(&self.path.with_extension("tmp")) + } +} + +fn remove_file_if_present(path: &Path) -> Result<(), SyncClientError> { + match fs::remove_file(path) { + Ok(()) => Ok(()), + Err(error) if error.kind() == ErrorKind::NotFound => Ok(()), + Err(error) => Err(SyncClientError::TokenStorage(error.to_string())), } } @@ -126,9 +131,11 @@ mod tests { store.save(&token)?; assert_eq!(store.load()?, Some(token.clone())); + fs::write(store.path().with_extension("tmp"), token.as_str()).map_err(io_err)?; store.clear()?; assert_eq!(store.load()?, None); + assert!(!store.path().with_extension("tmp").exists()); Ok(()) } } diff --git a/crates/ely_sync_client/src/error.rs b/crates/ely_sync_client/src/error.rs index 585bdd4..3a13e08 100644 --- a/crates/ely_sync_client/src/error.rs +++ b/crates/ely_sync_client/src/error.rs @@ -34,6 +34,9 @@ pub enum SyncClientError { #[error("Snapshot schema is invalid: {0}")] SnapshotSchema(String), + #[error("Sync policy blocks this operation: {reason}")] + SyncPolicy { reason: String }, + #[error("Device {device_id} cannot sync with approval status {status}")] DeviceApprovalStatus { device_id: String, status: String }, }