Scope Servo permissions by profile origin

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,

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,
 

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)]

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<WebViewId, ServoHostError> {
         let webview_id = WebViewId::new();
-        let delegate = Rc::new(HostWebViewDelegate::new(
+        let delegate =
-            tab_id.clone(),
+            Rc::new(HostWebViewDelegate::new(profile_id.clone(), self.permissions.clone()));
-            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(
+        set_permission_decision(&self.permissions, request, decision);
-            PermissionKey::new(request.profile_id, request.tab_id, request.feature),
-            decision,
-        );
         Ok(())
     }
 
@@ -399,7 +403,6 @@ impl HostWebView {
 }
 
 struct HostWebViewDelegate {
-    tab_id: TabId,
     profile_id: ProfileId,
     permissions: PermissionStore,
     state: RefCell<WebViewState>,
@@ -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<PermissionDecision> {
-        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) {
+    fn request_permission(&self, webview: WebView, permission_request: servo::PermissionRequest) {
-        let feature = format!("{:?}", permission_request.feature());
+        match permission_decision_for_webview(
-        match self.permission_decision(feature) {
+            &self.permissions,
+            &self.profile_id,
+            &webview,
+            self.url(),
+            permission_request.feature(),
+        ) {
             Some(PermissionDecision::AllowOnce | PermissionDecision::AllowAlways) => {
                 permission_request.allow();
             }

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<RefCell<HashMap<PermissionKey, PermissionDecision>>>;
 
 #[derive(Clone, Debug, Eq, Hash, PartialEq)]
 pub(super) struct PermissionKey {
     profile_id: ProfileId,
-    tab_id: TabId,
+    origin: SiteOrigin,
-    feature: String,
+    feature: SitePermissionFeature,
 }
 
 impl PermissionKey {
-    pub(super) fn new(profile_id: ProfileId, tab_id: TabId, feature: String) -> Self {
+    fn new(profile_id: ProfileId, origin: SiteOrigin, feature: SitePermissionFeature) -> Self {
-        Self { profile_id, tab_id, feature }
+        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<String>,
+    servo_feature: servo::PermissionFeature,
+) -> Option<PermissionDecision> {
+    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<PermissionDecision> {
+    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<String>) -> Option<SiteOrigin> {
+    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<SitePermissionFeature> {
+    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<dyn std::error::Error>> {
+        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<dyn std::error::Error>> {
+        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(())
     }
 }

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,
+    KeyboardTextRequest, MouseClickRequest, MouseDragRequest, NavigationRequest,
-    ScreenshotRequest, ScrollRequest, ServoHost, ServoHostError, ServoSurfaceSize,
+    PermissionDecision, PermissionRequest, ResizeRequest, ScreenshotRequest, ScrollRequest,
-    SoftwareServoHost, TouchTapRequest, WebViewState,
+    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<dyn Error>> {
     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 })?;