diff --git a/crates/ely_domain/src/site_permission.rs b/crates/ely_domain/src/site_permission.rs index 593518a..360e38a 100644 --- a/crates/ely_domain/src/site_permission.rs +++ b/crates/ely_domain/src/site_permission.rs @@ -4,10 +4,10 @@ use url::Url; use crate::{DomainError, ProfileId, UrlText}; -#[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)] +#[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] pub struct SiteOrigin(String); -#[derive(Clone, Copy, Debug, Eq, PartialEq)] +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] pub enum SitePermissionFeature { Camera, Microphone, diff --git a/crates/ely_servo_host/src/error.rs b/crates/ely_servo_host/src/error.rs index bb27ec2..c034886 100644 --- a/crates/ely_servo_host/src/error.rs +++ b/crates/ely_servo_host/src/error.rs @@ -1,4 +1,4 @@ -use ely_domain::WebViewId; +use ely_domain::{ProfileId, WebViewId}; use thiserror::Error; #[derive(Clone, Debug, Error, Eq, PartialEq)] @@ -12,6 +12,9 @@ pub enum ServoHostError { #[error("permission request missing profile context")] MissingProfileContext, + #[error("permission profile mismatch for {webview_id}: expected {expected}, got {actual}")] + PermissionProfileMismatch { webview_id: WebViewId, expected: ProfileId, actual: ProfileId }, + #[error("servo runtime is already started in this process")] RuntimeAlreadyStarted, diff --git a/crates/ely_servo_host/src/host.rs b/crates/ely_servo_host/src/host.rs index cbe684d..5026e24 100644 --- a/crates/ely_servo_host/src/host.rs +++ b/crates/ely_servo_host/src/host.rs @@ -1,4 +1,4 @@ -use ely_domain::{ProfileId, TabId, UrlText, WebViewId}; +use ely_domain::{ProfileId, SiteOrigin, SitePermissionFeature, TabId, UrlText, WebViewId}; use crate::ServoHostError; @@ -264,9 +264,9 @@ pub struct ScreenshotRequest { #[derive(Clone, Debug, Eq, PartialEq)] pub struct PermissionRequest { pub webview_id: WebViewId, - pub tab_id: TabId, pub profile_id: ProfileId, - pub feature: String, + pub origin: SiteOrigin, + pub feature: SitePermissionFeature, } #[derive(Clone, Debug, Eq, PartialEq)] diff --git a/crates/ely_servo_host/src/runtime.rs b/crates/ely_servo_host/src/runtime.rs index adf9d5a..3272903 100644 --- a/crates/ely_servo_host/src/runtime.rs +++ b/crates/ely_servo_host/src/runtime.rs @@ -25,7 +25,9 @@ use crate::{ PermissionDecision, PermissionRequest, RenderedFrame, ResizeRequest, ScreenshotRequest, ScrollRequest, ServoHost, ServoHostError, TouchTapRequest, WebViewSnapshot, WebViewState, runtime_input::{send_keyboard_text, send_mouse_click, send_mouse_drag, send_touch_tap}, - runtime_permissions::{PermissionKey, PermissionStore}, + runtime_permissions::{ + PermissionStore, permission_decision_for_webview, set_permission_decision, + }, runtime_waker::ServoWakeFlag, }; @@ -118,11 +120,8 @@ impl ServoHost for SoftwareServoHost { profile_id: ProfileId, ) -> Result { let webview_id = WebViewId::new(); - let delegate = Rc::new(HostWebViewDelegate::new( - tab_id.clone(), - profile_id.clone(), - self.permissions.clone(), - )); + let delegate = + Rc::new(HostWebViewDelegate::new(profile_id.clone(), self.permissions.clone())); let webview = WebViewBuilder::new(&self.servo, self.rendering_context.clone()) .delegate(delegate.clone()) .build(); @@ -281,14 +280,19 @@ impl ServoHost for SoftwareServoHost { request: PermissionRequest, decision: PermissionDecision, ) -> Result<(), ServoHostError> { - self.webviews + let webview = self + .webviews .get(&request.webview_id) .ok_or_else(|| ServoHostError::WebViewNotFound { id: request.webview_id.clone() })?; + if webview.profile_id != request.profile_id { + return Err(ServoHostError::PermissionProfileMismatch { + webview_id: request.webview_id, + expected: webview.profile_id.clone(), + actual: request.profile_id, + }); + } - self.permissions.borrow_mut().insert( - PermissionKey::new(request.profile_id, request.tab_id, request.feature), - decision, - ); + set_permission_decision(&self.permissions, request, decision); Ok(()) } @@ -399,7 +403,6 @@ impl HostWebView { } struct HostWebViewDelegate { - tab_id: TabId, profile_id: ProfileId, permissions: PermissionStore, state: RefCell, @@ -409,9 +412,8 @@ struct HostWebViewDelegate { } impl HostWebViewDelegate { - fn new(tab_id: TabId, profile_id: ProfileId, permissions: PermissionStore) -> Self { + fn new(profile_id: ProfileId, permissions: PermissionStore) -> Self { Self { - tab_id, profile_id, permissions, state: RefCell::new(WebViewState::Created), @@ -444,15 +446,6 @@ impl HostWebViewDelegate { fn mark_frame_presented(&self) { self.has_pending_frame.set(false); } - - fn permission_decision(&self, feature: String) -> Option { - let key = PermissionKey::new(self.profile_id.clone(), self.tab_id.clone(), feature); - let mut permissions = self.permissions.borrow_mut(); - match permissions.get(&key).cloned() { - Some(PermissionDecision::AllowOnce) => permissions.remove(&key), - decision => decision, - } - } } impl WebViewDelegate for HostWebViewDelegate { @@ -484,9 +477,14 @@ impl WebViewDelegate for HostWebViewDelegate { navigation_request.allow(); } - fn request_permission(&self, _webview: WebView, permission_request: servo::PermissionRequest) { - let feature = format!("{:?}", permission_request.feature()); - match self.permission_decision(feature) { + fn request_permission(&self, webview: WebView, permission_request: servo::PermissionRequest) { + match permission_decision_for_webview( + &self.permissions, + &self.profile_id, + &webview, + self.url(), + permission_request.feature(), + ) { Some(PermissionDecision::AllowOnce | PermissionDecision::AllowAlways) => { permission_request.allow(); } diff --git a/crates/ely_servo_host/src/runtime_permissions.rs b/crates/ely_servo_host/src/runtime_permissions.rs index 967b95c..7f56c20 100644 --- a/crates/ely_servo_host/src/runtime_permissions.rs +++ b/crates/ely_servo_host/src/runtime_permissions.rs @@ -1,20 +1,192 @@ use std::{cell::RefCell, collections::HashMap, rc::Rc}; -use ely_domain::{ProfileId, TabId}; +use ely_domain::{ProfileId, SiteOrigin, SitePermissionFeature}; +use servo::WebView; -use crate::PermissionDecision; +use crate::{PermissionDecision, PermissionRequest}; pub(super) type PermissionStore = Rc>>; #[derive(Clone, Debug, Eq, Hash, PartialEq)] pub(super) struct PermissionKey { profile_id: ProfileId, - tab_id: TabId, - feature: String, + origin: SiteOrigin, + feature: SitePermissionFeature, } impl PermissionKey { - pub(super) fn new(profile_id: ProfileId, tab_id: TabId, feature: String) -> Self { - Self { profile_id, tab_id, feature } + fn new(profile_id: ProfileId, origin: SiteOrigin, feature: SitePermissionFeature) -> Self { + Self { profile_id, origin, feature } + } +} + +pub(super) fn set_permission_decision( + permissions: &PermissionStore, + request: PermissionRequest, + decision: PermissionDecision, +) { + permissions + .borrow_mut() + .insert(PermissionKey::new(request.profile_id, request.origin, request.feature), decision); +} + +pub(super) fn permission_decision_for_webview( + permissions: &PermissionStore, + profile_id: &ProfileId, + webview: &WebView, + fallback_url: Option, + servo_feature: servo::PermissionFeature, +) -> Option { + let origin = site_origin_for_webview(webview, fallback_url)?; + let feature = site_permission_feature_for_servo(servo_feature)?; + take_permission_decision(permissions, profile_id, origin, feature) +} + +fn take_permission_decision( + permissions: &PermissionStore, + profile_id: &ProfileId, + origin: SiteOrigin, + feature: SitePermissionFeature, +) -> Option { + let key = PermissionKey::new(profile_id.clone(), origin, feature); + let mut permissions = permissions.borrow_mut(); + match permissions.get(&key).cloned() { + Some(PermissionDecision::AllowOnce) => permissions.remove(&key), + decision => decision, + } +} + +fn site_origin_for_webview(webview: &WebView, fallback_url: Option) -> Option { + webview + .url() + .map(|url| url.to_string()) + .or(fallback_url) + .and_then(|url| SiteOrigin::parse(url).ok()) +} + +fn site_permission_feature_for_servo( + feature: servo::PermissionFeature, +) -> Option { + match feature { + servo::PermissionFeature::Geolocation => Some(SitePermissionFeature::Location), + servo::PermissionFeature::Notifications => Some(SitePermissionFeature::Notifications), + servo::PermissionFeature::Camera => Some(SitePermissionFeature::Camera), + servo::PermissionFeature::Microphone => Some(SitePermissionFeature::Microphone), + servo::PermissionFeature::PersistentStorage => { + Some(SitePermissionFeature::StoragePersistence) + } + servo::PermissionFeature::Push + | servo::PermissionFeature::Midi + | servo::PermissionFeature::Speaker + | servo::PermissionFeature::DeviceInfo + | servo::PermissionFeature::BackgroundSync + | servo::PermissionFeature::Bluetooth => None, + } +} + +#[cfg(test)] +mod tests { + use ely_domain::{ProfileId, SiteOrigin, SitePermissionFeature}; + + use crate::{PermissionDecision, PermissionRequest}; + + use super::{PermissionStore, set_permission_decision, take_permission_decision}; + + #[test] + fn allow_once_is_consumed_after_one_matching_origin_request() + -> Result<(), Box> { + let permissions = PermissionStore::default(); + let profile_id = ProfileId::new(); + let origin = SiteOrigin::parse("https://example.com/path")?; + + set_permission_decision( + &permissions, + PermissionRequest { + webview_id: ely_domain::WebViewId::new(), + profile_id: profile_id.clone(), + origin: origin.clone(), + feature: SitePermissionFeature::Camera, + }, + PermissionDecision::AllowOnce, + ); + + assert_eq!( + take_permission_decision( + &permissions, + &profile_id, + origin.clone(), + SitePermissionFeature::Camera, + ), + Some(PermissionDecision::AllowOnce) + ); + assert_eq!( + take_permission_decision( + &permissions, + &profile_id, + origin, + SitePermissionFeature::Camera + ), + None + ); + Ok(()) + } + + #[test] + fn allow_always_stays_scoped_to_profile_origin_and_feature() + -> Result<(), Box> { + let permissions = PermissionStore::default(); + let profile_id = ProfileId::new(); + let other_profile_id = ProfileId::new(); + let origin = SiteOrigin::parse("https://example.com")?; + let other_origin = SiteOrigin::parse("https://example.org")?; + + set_permission_decision( + &permissions, + PermissionRequest { + webview_id: ely_domain::WebViewId::new(), + profile_id: profile_id.clone(), + origin: origin.clone(), + feature: SitePermissionFeature::Notifications, + }, + PermissionDecision::AllowAlways, + ); + + assert_eq!( + take_permission_decision( + &permissions, + &profile_id, + origin.clone(), + SitePermissionFeature::Notifications, + ), + Some(PermissionDecision::AllowAlways) + ); + assert_eq!( + take_permission_decision( + &permissions, + &other_profile_id, + origin, + SitePermissionFeature::Notifications, + ), + None + ); + assert_eq!( + take_permission_decision( + &permissions, + &profile_id, + other_origin, + SitePermissionFeature::Notifications, + ), + None + ); + assert_eq!( + take_permission_decision( + &permissions, + &profile_id, + SiteOrigin::parse("https://example.com")?, + SitePermissionFeature::Camera, + ), + None + ); + Ok(()) } } diff --git a/crates/ely_servo_host/tests/software_host.rs b/crates/ely_servo_host/tests/software_host.rs index 7e3b661..1161820 100644 --- a/crates/ely_servo_host/tests/software_host.rs +++ b/crates/ely_servo_host/tests/software_host.rs @@ -8,11 +8,11 @@ use std::{ time::Duration, }; -use ely_domain::{ProfileId, TabId, UrlText}; +use ely_domain::{ProfileId, SiteOrigin, SitePermissionFeature, TabId, UrlText}; use ely_servo_host::{ - KeyboardTextRequest, MouseClickRequest, MouseDragRequest, NavigationRequest, ResizeRequest, - ScreenshotRequest, ScrollRequest, ServoHost, ServoHostError, ServoSurfaceSize, - SoftwareServoHost, TouchTapRequest, WebViewState, + KeyboardTextRequest, MouseClickRequest, MouseDragRequest, NavigationRequest, + PermissionDecision, PermissionRequest, ResizeRequest, ScreenshotRequest, ScrollRequest, + ServoHost, ServoHostError, ServoSurfaceSize, SoftwareServoHost, TouchTapRequest, WebViewState, }; const MINIMUM_CONTENT_PIXELS: u64 = 1_000; @@ -80,6 +80,34 @@ fn exercise_real_servo_webview_lifecycle() -> Result<(), Box> { assert_eq!(snapshot.profile_id(), &profile_id); assert_eq!(snapshot.state(), &WebViewState::Created); + host.set_permission( + PermissionRequest { + webview_id: webview_id.clone(), + profile_id: profile_id.clone(), + origin: SiteOrigin::parse("https://example.com")?, + feature: SitePermissionFeature::Camera, + }, + PermissionDecision::AllowOnce, + )?; + let other_profile_id = ProfileId::new(); + let mismatch = host.set_permission( + PermissionRequest { + webview_id: webview_id.clone(), + profile_id: other_profile_id.clone(), + origin: SiteOrigin::parse("https://example.com")?, + feature: SitePermissionFeature::Camera, + }, + PermissionDecision::AllowAlways, + ); + assert!( + matches!( + mismatch, + Err(ServoHostError::PermissionProfileMismatch { ref expected, ref actual, .. }) + if expected == &profile_id && actual == &other_profile_id + ), + "mismatch: {mismatch:?}" + ); + let url = UrlText::parse(CLICK_PROBE_URL)?; host.navigate(NavigationRequest { webview_id: webview_id.clone(), tab_id, url })?;