diff --git a/platform/README.md b/platform/README.md index 11e7b4d9..1c2c89a1 100644 --- a/platform/README.md +++ b/platform/README.md @@ -51,6 +51,7 @@ host-provided service: | `documents` | Native file dialogs | Implements `documents::DocumentProvider` with the system picker | | `credentials` | System keychain (`SystemCredentialStore`) | Implements `CredentialStore` with the platform keystore | | `oauth_redirects` | Loopback redirect broker | Implements `OAuthRedirectBroker` (app links, custom schemes) | +| `external_urls` | System URL launcher | `ExternalUrlOpener::new` with the host's native URL bridge | Mobile hosts use `PlatformContext::app_private(data_root, cache_dir, documents, credentials, oauth_redirects)`, which keeps durable data below @@ -58,6 +59,23 @@ credentials, oauth_redirects)`, which keeps durable data below `UnavailableCredentialStore` so account setup reports the gap. Never fall back to a plaintext store. +The constructors default `external_urls` to the system URL launcher. A host +without desktop launchers configures its context before calling `run`: + +```rust,ignore +platform.external_urls = flectar_mail::ExternalUrlOpener::new(bridge::open_url); +``` + +This replaces the process-global `set_external_url_opener` registration. The +callback may capture host state; cloned contexts share it. Mail links invoke it +on the application event loop, and browser-based OAuth (including retries) +invokes it on a worker thread. The host must marshal calls onto any thread its +native API requires and return promptly after submitting the request. Errors +reach the UI without falling back to a desktop launcher. Email links retain +their `http`/`https`/`mailto`/`tel` allowlist, while browser authorization requires +a parsed HTTPS URL with a host. The existing in-app `mailto` compose route and +native OAuth broker flows remain in place. + Each mobile operating system also has a module in `src/platform/` that provides these crate-internal items, which shared code calls without a `cfg`: diff --git a/src/browser.rs b/src/browser.rs index b6288daf..177bcea4 100644 --- a/src/browser.rs +++ b/src/browser.rs @@ -1,4 +1,52 @@ -//! Open the system browser without leaking AppImage libraries into host programs. +//! Host URL dispatch, with separate policies for mail links and authorization. + +use std::sync::Arc; + +type OpenUrl = dyn Fn(&str) -> Result<(), String> + Send + Sync; + +/// The URL service carried by [`crate::PlatformContext`]. +/// +/// The default uses the system browser, preserving Linux's portal and AppImage +/// library-path handling for authorization. Hosts without desktop launchers +/// supply a callback with [`Self::new`]. Clones share that callback's state. +#[derive(Clone, Default)] +pub struct ExternalUrlOpener { + handler: Option>, +} + +impl ExternalUrlOpener { + /// Use a host callback for both external links and browser authorization. + /// + /// Mail links call this on the application event loop; authorization calls + /// it on a worker thread. Hosts must marshal native API calls to their + /// required thread. Return promptly after handing the URL to the host, + /// without waiting for the browser to close. Errors are shown to the user; + /// a failed host callback never falls back to a desktop launcher. + pub fn new(opener: impl Fn(&str) -> Result<(), String> + Send + Sync + 'static) -> Self { + Self { + handler: Some(Arc::new(opener)), + } + } + + pub(super) fn open_external(&self, url: &str) -> Result<(), String> { + match &self.handler { + Some(opener) => opener(url), + None => webbrowser::open(url).map_err(|error| error.to_string()), + } + } + + pub(super) fn open_authorization(&self, url: &str) -> Result<(), String> { + let address = url::Url::parse(url).map_err(|_| "Sign-in address is invalid".to_owned())?; + if address.scheme() != "https" || address.host_str().is_none() { + return Err("Sign-in requires an HTTPS address".into()); + } + match &self.handler { + Some(opener) => opener(address.as_str()), + None => open_system_authorization(&address), + } + } +} + #[cfg(linux_desktop)] use std::process::{Command, Stdio}; @@ -74,19 +122,14 @@ fn open_with_desktop_portal(address: &url::Url) -> Result<(), String> { }) } -pub(super) fn open(url: &str) -> Result<(), String> { - // This entry point is only for provider authorization, never a shell command. - let address = url::Url::parse(url).map_err(|_| "Sign-in address is invalid".to_owned())?; - if address.scheme() != "https" || address.host_str().is_none() { - return Err("Sign-in requires an HTTPS address".into()); - } +fn open_system_authorization(address: &url::Url) -> Result<(), String> { let url = address.as_str(); #[cfg(linux_desktop)] { // The portal is the reliable route from AppImages and desktop // containers to the user's host browser. Keep command-based fallbacks // for minimal desktops that do not run xdg-desktop-portal. - if open_with_desktop_portal(&address).is_ok() { + if open_with_desktop_portal(address).is_ok() { return Ok(()); } let mut xdg = host_command("xdg-open"); @@ -102,6 +145,111 @@ pub(super) fn open(url: &str) -> Result<(), String> { webbrowser::open(url).map_err(|e| e.to_string()) } +#[cfg(test)] +mod dispatch_tests { + use super::*; + use crate::mail_render_projection::open_email_link; + use std::sync::Mutex; + + #[test] + fn mail_links_preserve_allowed_addresses_and_reject_other_schemes() { + let addresses = Arc::new(Mutex::new(Vec::new())); + let recorded = addresses.clone(); + let opener = ExternalUrlOpener::new(move |url| { + recorded.lock().unwrap().push(url.to_owned()); + Ok(()) + }); + let allowed = [ + "http://example.com/path?q=1#part", + "https://example.com/a%20b?x=1&y=2", + "HTTPS://example.com/", + "mailto:person@example.com?subject=Hello%20there", + "tel:+123456789", + ]; + for url in allowed { + open_email_link(&opener, &format!(" {url}\n")).unwrap(); + } + for url in [ + "", + "example.com", + "/relative/path", + "#fragment", + "--help", + "javascript:alert(1)", + "data:text/html,hello", + "file:///tmp/message.html", + "intent://example.com", + ] { + assert!(open_email_link(&opener, url).is_err(), "{url}"); + } + assert_eq!(*addresses.lock().unwrap(), allowed); + } + + #[test] + fn authorization_validates_before_dispatch_for_default_and_custom_openers() { + let custom = ExternalUrlOpener::new(|_| panic!("invalid authorization reached the host")); + for opener in [ExternalUrlOpener::default(), custom] { + for url in [ + "http://example.com/", + "file:///tmp/test", + "mailto:person@example.com", + "javascript:alert(1)", + "--help", + "https://", + "https://[invalid]/", + "/relative/path", + ] { + assert!(opener.open_authorization(url).is_err(), "{url}"); + } + } + } + + #[test] + fn authorization_and_mail_share_the_host_across_threads() { + let addresses = Arc::new(Mutex::new(Vec::new())); + let recorded = addresses.clone(); + let opener = ExternalUrlOpener::new(move |url| { + recorded.lock().unwrap().push(url.to_owned()); + Ok(()) + }); + open_email_link(&opener, "https://example.com/mail").unwrap(); + let worker_opener = opener.clone(); + std::thread::spawn(move || { + worker_opener + .open_authorization("HTTPS://EXAMPLE.COM/sign-in?state=a%2Bb&code_challenge=c") + }) + .join() + .unwrap() + .unwrap(); + assert_eq!( + *addresses.lock().unwrap(), + [ + "https://example.com/mail", + "https://example.com/sign-in?state=a%2Bb&code_challenge=c", + ] + ); + } + + #[test] + fn host_errors_propagate_without_fallback_or_global_registration() { + let first = ExternalUrlOpener::new(|_| Err("first host unavailable".into())); + let second = ExternalUrlOpener::new(|_| Err("second host unavailable".into())); + for (opener, expected) in [ + (first, "first host unavailable"), + (second, "second host unavailable"), + ] { + assert_eq!( + open_email_link(&opener, "https://example.com/"), + Err(expected.to_owned()) + ); + assert_eq!( + opener.open_authorization("https://example.com/"), + Err(expected.to_owned()) + ); + } + } +} + #[cfg(all(test, linux_desktop))] mod tests { use super::*; @@ -118,12 +266,6 @@ mod tests { assert!(launch(command).is_ok()); } #[test] - fn rejects_non_https_authorization() { - assert!(open("file:///tmp/test").is_err()); - assert!(open("--help").is_err()); - assert!(open("https://").is_err()); - } - #[test] fn restores_host_libraries_only_for_bundled_builds() { let mut command = Command::new("xdg-open"); restore_library_path(&mut command, false, None); diff --git a/src/mail_render_projection.rs b/src/mail_render_projection.rs index ab327a8e..4368775a 100644 --- a/src/mail_render_projection.rs +++ b/src/mail_render_projection.rs @@ -384,13 +384,13 @@ pub(super) fn apply_cpu_frame(app: &AppWindow, rendered: RenderedEmail) { } } -pub(super) fn open_email_link(url: &str) -> Result<(), String> { +pub(super) fn open_email_link(opener: &ExternalUrlOpener, url: &str) -> Result<(), String> { let Some((scheme, _)) = url.trim().split_once(':') else { return Err("only absolute links can be opened".into()); }; match scheme.to_ascii_lowercase().as_str() { "http" | "https" | "mailto" | "tel" => { - webbrowser::open(url.trim()).map_err(|error| error.to_string())?; + opener.open_external(url.trim())?; Ok(()) } _ => Err("unsupported link type".into()), diff --git a/src/main.rs b/src/main.rs index 541a902e..717b6bb5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4,6 +4,7 @@ mod account_mail_preferences; mod account_removal_tests; mod attachment_controller; mod browser; +pub use browser::ExternalUrlOpener; mod calendar; mod compose_controller; mod compose_editor; @@ -1081,6 +1082,8 @@ impl InboxState { /// sandbox paths and native services. See platform/README.md. #[derive(Clone)] pub struct PlatformContext { + /// Host URL dispatch for mail links and browser-based authorization. + pub external_urls: ExternalUrlOpener, pub paths: Paths, pub documents: Arc, pub credentials: flectar_mail_core::accounts::credentials::CredentialStoreHandle, @@ -1094,6 +1097,7 @@ impl PlatformContext { let credentials = desktop_credential_store(&paths); Ok(Self { paths, + external_urls: ExternalUrlOpener::default(), documents: documents::default_provider(), credentials, oauth_redirects: Arc::new( @@ -1113,6 +1117,7 @@ impl PlatformContext { ) -> Self { Self { paths: Paths::new(data_root.join("data"), cache_dir), + external_urls: ExternalUrlOpener::default(), documents, credentials, oauth_redirects, @@ -6238,7 +6243,11 @@ pub fn run(platform: PlatformContext) -> Result<(), Box> let state_for_account = Rc::clone(&state); let runtime_for_account = Rc::clone(&runtime); let ui_task_tx_for_account = ui_task_tx.clone(); - oauth_browser::register(&app, platform.oauth_redirects.clone()); + oauth_browser::register( + &app, + platform.oauth_redirects.clone(), + platform.external_urls.clone(), + ); mail_setup::register(&app, &runtime); let suggestion_app = app.as_weak(); @@ -6432,6 +6441,7 @@ pub fn run(platform: PlatformContext) -> Result<(), Box> let state_for_oauth = Rc::clone(&state); let runtime_for_oauth = Rc::clone(&runtime); let ui_task_tx_for_oauth = ui_task_tx.clone(); + let opener_for_oauth = platform.external_urls.clone(); app.on_start_oauth(move |provider| { let Some(app) = app_weak.upgrade() else { return; @@ -6472,10 +6482,11 @@ pub fn run(platform: PlatformContext) -> Result<(), Box> )); let updates = ui_task_tx_for_oauth.clone(); let browser_app = app.as_weak(); + let opener = opener_for_oauth.clone(); runtime_for_oauth.spawn(async move { let result = core .start_oauth_with_calendar(provider, connect_calendar, move |url| { - oauth_browser::begin(browser_app.clone(), url) + oauth_browser::begin(browser_app.clone(), url, opener.clone()) }) .await; let update = match result { @@ -6518,6 +6529,7 @@ pub fn run(platform: PlatformContext) -> Result<(), Box> let state_for_reauth = Rc::clone(&state); let runtime_for_reauth = Rc::clone(&runtime); let ui_task_tx_for_reauth = ui_task_tx.clone(); + let opener_for_reauth = platform.external_urls.clone(); app.on_reauth_account(move |account_id| { let Some(app) = app_weak.upgrade() else { return; @@ -6545,10 +6557,11 @@ pub fn run(platform: PlatformContext) -> Result<(), Box> )); let updates = ui_task_tx_for_reauth.clone(); let browser_app = app.as_weak(); + let opener = opener_for_reauth.clone(); runtime_for_reauth.spawn(async move { let result = core .reauth_account(i64::from(account_id), move |url| { - oauth_browser::begin(browser_app.clone(), url) + oauth_browser::begin(browser_app.clone(), url, opener.clone()) }) .await; let update = match result { @@ -6603,6 +6616,7 @@ pub fn run(platform: PlatformContext) -> Result<(), Box> let state_for_calendar_connect = Rc::clone(&state); let runtime_for_calendar_connect = Rc::clone(&runtime); let updates_for_calendar_connect = ui_task_tx.clone(); + let opener_for_calendar_connect = platform.external_urls.clone(); app.on_connect_account_calendar(move |account_id| { let Some(app) = app_weak.upgrade() else { return; @@ -6641,10 +6655,11 @@ pub fn run(platform: PlatformContext) -> Result<(), Box> )); let updates = updates_for_calendar_connect.clone(); let browser_app = app.as_weak(); + let opener = opener_for_calendar_connect.clone(); runtime_for_calendar_connect.spawn(async move { let result = core .connect_provider_calendar(i64::from(account_id), provider, move |url| { - oauth_browser::begin(browser_app.clone(), url) + oauth_browser::begin(browser_app.clone(), url, opener.clone()) }) .await; let connections = core.load_calendar_connections().await.ok(); @@ -6898,7 +6913,12 @@ pub fn run(platform: PlatformContext) -> Result<(), Box> &calendar_state, ); - register_renderer_input_callbacks(&app, &email_renderer, use_wgpu); + register_renderer_input_callbacks( + &app, + &email_renderer, + use_wgpu, + platform.external_urls.clone(), + ); let app_weak = app.as_weak(); let state_for_search = Rc::clone(&state); diff --git a/src/oauth_browser.rs b/src/oauth_browser.rs index daa1fdaa..dfedc730 100644 --- a/src/oauth_browser.rs +++ b/src/oauth_browser.rs @@ -1,7 +1,11 @@ //! Keep the redirect listener alive when a desktop browser cannot be launched. use super::*; -pub(super) fn begin(app: slint::Weak, url: String) -> Result<(), String> { +pub(super) fn begin( + app: slint::Weak, + url: String, + opener: ExternalUrlOpener, +) -> Result<(), String> { let link = url.clone(); app.upgrade_in_event_loop(move |app| { if app.get_oauth_in_progress() { @@ -10,13 +14,13 @@ pub(super) fn begin(app: slint::Weak, url: String) -> Result<(), Stri } }) .map_err(|e| e.to_string())?; - launch(app, url); + launch(app, url, opener); Ok(()) } -fn launch(app: slint::Weak, url: String) { +fn launch(app: slint::Weak, url: String, opener: ExternalUrlOpener) { std::thread::spawn(move || { - let failed = browser::open(&url).is_err(); + let failed = opener.open_authorization(&url).is_err(); let _ = app.upgrade_in_event_loop(move |app| { if app.get_oauth_in_progress() && app.get_oauth_authorization_url().as_str() == url { app.set_oauth_browser_error(failed); @@ -28,6 +32,7 @@ fn launch(app: slint::Weak, url: String) { pub(super) fn register( app: &AppWindow, redirects: flectar_mail_core::oauth::redirect::OAuthRedirectBrokerHandle, + opener: ExternalUrlOpener, ) { let weak = app.as_weak(); app.on_retry_oauth_browser(move || { @@ -36,7 +41,7 @@ pub(super) fn register( }; let url = app.get_oauth_authorization_url().to_string(); if app.get_oauth_in_progress() && !url.is_empty() { - launch(app.as_weak(), url); + launch(app.as_weak(), url, opener.clone()); } }); let weak = app.as_weak(); @@ -83,15 +88,21 @@ mod tests { } } #[test] - fn onboarding_copies_active_link_without_a_mailbox_and_ignores_finished_sign_in() { + fn onboarding_copies_and_retries_active_link_and_ignores_finished_sign_in() { let window = MinimalSoftwareWindow::new(RepaintBufferType::NewBuffer); let clipboard = Rc::new(RefCell::new(String::new())); slint::platform::set_platform(Box::new(Headless(window.clone(), clipboard.clone()))) .unwrap(); let app = AppWindow::new().unwrap(); + let (opened, received) = std::sync::mpsc::channel(); + let opener = ExternalUrlOpener::new(move |url| { + opened.send(url.to_owned()).unwrap(); + Err("test host unavailable".into()) + }); register( &app, Arc::new(flectar_mail_core::oauth::redirect::LoopbackRedirectBroker::default()), + opener.clone(), ); app.set_startup_ready(true); app.set_oauth_in_progress(true); @@ -106,10 +117,21 @@ mod tests { *clipboard.borrow(), app.get_oauth_authorization_url().as_str() ); + assert!( + received.try_recv().is_err(), + "copy must not launch a browser" + ); + let url = app.get_oauth_authorization_url().to_string(); + launch(app.as_weak(), url.clone(), opener); + assert_eq!(received.recv_timeout(Duration::from_secs(5)).unwrap(), url); + app.invoke_retry_oauth_browser(); + assert_eq!(received.recv_timeout(Duration::from_secs(5)).unwrap(), url); app.set_oauth_in_progress(false); app.set_oauth_authorization_url("".into()); *clipboard.borrow_mut() = "keep clipboard".into(); app.invoke_copy_oauth_link(); + app.invoke_retry_oauth_browser(); assert_eq!(*clipboard.borrow(), "keep clipboard"); + assert!(received.try_recv().is_err()); } } diff --git a/src/platform/ios.rs b/src/platform/ios.rs index 5bef317d..a583a2ea 100644 --- a/src/platform/ios.rs +++ b/src/platform/ios.rs @@ -23,6 +23,7 @@ pub fn launch_context() -> Result Result() diff --git a/src/renderer_input_controller.rs b/src/renderer_input_controller.rs index d574b7c9..a3565a0c 100644 --- a/src/renderer_input_controller.rs +++ b/src/renderer_input_controller.rs @@ -78,6 +78,7 @@ pub(super) fn register_renderer_input_callbacks( app: &AppWindow, renderer: &Rc>, gpu: bool, + opener: ExternalUrlOpener, ) { let weak = app.as_weak(); let appearance_renderer = renderer.clone(); @@ -136,7 +137,7 @@ pub(super) fn register_renderer_input_callbacks( } return; } - if let Err(error) = open_email_link(&url) { + if let Err(error) = open_email_link(&opener, &url) { app.set_render_status(UiMessage::detail("Could not open link: {}", error)); } });