fix(permissions): make profile snapshots authoritative

This commit is contained in:
2026-07-09 21:56:36 -04:00
parent c28ec2bee8
commit b422ac1631
47 changed files with 1918 additions and 403 deletions
+8
View File
@@ -143,6 +143,12 @@ pub struct BrowserSnapshot {
pub command_query: String,
}
#[derive(Debug)]
struct TransferredSitePermission {
entry: SitePermissionEntry,
grant_revision: u64,
}
#[derive(Debug)]
pub struct BrowserCore {
spaces: Vec<Space>,
@@ -153,6 +159,7 @@ pub struct BrowserCore {
notes: Vec<NoteEntry>,
reading_list: Vec<ReadingListEntry>,
site_permissions: Vec<SitePermissionEntry>,
transferred_site_permissions: Vec<TransferredSitePermission>,
site_permission_audit_events: Vec<SitePermissionAuditEvent>,
download_entries: Vec<DownloadEntry>,
history_entries: Vec<HistoryEntry>,
@@ -238,6 +245,7 @@ impl BrowserCore {
notes: Vec::new(),
reading_list: Vec::new(),
site_permissions: Vec::new(),
transferred_site_permissions: Vec::new(),
site_permission_audit_events: Vec::new(),
download_entries: Vec::new(),
history_entries: Vec::new(),
@@ -224,6 +224,8 @@ pub(super) struct ElyLocalSitePermissionAuditRecord {
enum ElyLocalSitePermissionAuditActionRecord {
Set { decision: String },
Revoked,
Transferred,
Consumed,
}
impl ElyLocalProfileRecord {
@@ -436,6 +438,8 @@ impl ElyLocalSitePermissionAuditActionRecord {
Self::Set { decision: decision.as_str().to_string() }
}
SitePermissionAuditAction::Revoked => Self::Revoked,
SitePermissionAuditAction::Transferred => Self::Transferred,
SitePermissionAuditAction::Consumed => Self::Consumed,
}
}
}
@@ -11,7 +11,7 @@ use super::local_data_export_records::{
};
use crate::CoreError;
pub const ELYDATA_SCHEMA_VERSION: u16 = 1;
pub const ELYDATA_SCHEMA_VERSION: u16 = 2;
pub const ELYDATA_FILE_EXTENSION: &str = "elydata";
impl BrowserCore {
@@ -79,6 +79,7 @@ impl BrowserCore {
.site_permissions
.iter()
.filter(|entry| entry.profile_id() == profile_id)
.filter(|entry| entry.decision() != ely_domain::SitePermissionDecision::AllowOnce)
.map(ElyLocalSitePermissionRecord::from_site_permission)
.collect(),
site_permission_audit_events: self
@@ -132,6 +132,7 @@ impl BrowserCore {
.site_permissions
.iter()
.filter(|entry| entry.profile_id() == profile_id)
.filter(|entry| entry.decision() != ely_domain::SitePermissionDecision::AllowOnce)
.count(),
site_permission_audit_events: self
.site_permission_audit_events
@@ -75,6 +75,15 @@ impl BrowserCore {
true
}
});
self.transferred_site_permissions.retain(|permission| {
if permission.entry.profile_id() == profile_id && permission.entry.origin() == origin {
revoked_permissions
.push((permission.entry.origin().clone(), permission.entry.feature()));
false
} else {
true
}
});
let revoked_count = revoked_permissions.len();
for (origin, feature) in revoked_permissions {
@@ -7,7 +7,7 @@ use ely_domain::{
use crate::CoreError;
use super::BrowserCore;
use super::{BrowserCore, TransferredSitePermission};
impl BrowserCore {
pub fn set_site_permission(
@@ -45,21 +45,112 @@ impl BrowserCore {
pub(super) fn visible_site_permissions(&self) -> Vec<SitePermissionEntry> {
self.site_permissions
.iter()
.chain(self.transferred_site_permissions.iter().map(|permission| &permission.entry))
.filter(|entry| entry.profile_id() == &self.active_profile_id)
.cloned()
.collect()
}
pub fn site_permissions_for_profile_origin(
pub fn site_permissions_for_profile(&self, profile_id: &ProfileId) -> Vec<SitePermissionEntry> {
self.site_permissions
.iter()
.filter(|entry| entry.profile_id() == profile_id)
.cloned()
.collect()
}
pub fn transferred_site_permissions_for_profile(
&self,
profile_id: &ProfileId,
) -> Vec<(SitePermissionEntry, u64)> {
self.transferred_site_permissions
.iter()
.filter(|permission| permission.entry.profile_id() == profile_id)
.map(|permission| (permission.entry.clone(), permission.grant_revision))
.collect()
}
pub fn site_permission_audit_events_for_profile(
&self,
profile_id: &ProfileId,
) -> Vec<SitePermissionAuditEvent> {
self.site_permission_audit_events
.iter()
.filter(|event| event.profile_id() == profile_id)
.cloned()
.collect()
}
pub fn site_permission_revision(
&self,
profile_id: &ProfileId,
origin: &SiteOrigin,
) -> Vec<SitePermissionEntry> {
self.site_permissions
feature: SitePermissionFeature,
) -> u64 {
self.site_permission_audit_events
.iter()
.filter(|entry| entry.profile_id() == profile_id && entry.origin() == origin)
.cloned()
.collect()
.filter(|event| {
event.profile_id() == profile_id
&& event.origin() == origin
&& event.feature() == feature
})
.fold(0_u64, |revision, _| revision.saturating_add(1))
}
pub fn transfer_site_permission_once(
&mut self,
profile_id: &ProfileId,
origin: &SiteOrigin,
feature: SitePermissionFeature,
revision: u64,
) -> Result<bool, CoreError> {
self.require_profile(profile_id)?;
if self.site_permission_revision(profile_id, origin, feature) != revision {
return Ok(false);
}
let Some(index) = self.site_permission_entry_index(profile_id, origin, feature) else {
return Ok(false);
};
if self.site_permissions[index].decision() != SitePermissionDecision::AllowOnce {
return Ok(false);
}
let entry = self.site_permissions.remove(index);
self.record_site_permission_audit_event(
profile_id.clone(),
entry.origin().clone(),
feature,
SitePermissionAuditAction::Transferred,
);
self.transferred_site_permissions
.push(TransferredSitePermission { entry, grant_revision: revision });
Ok(true)
}
pub fn finish_site_permission_once(
&mut self,
profile_id: &ProfileId,
origin: &SiteOrigin,
feature: SitePermissionFeature,
grant_revision: u64,
) -> Result<bool, CoreError> {
self.require_profile(profile_id)?;
let Some(index) = self.transferred_site_permissions.iter().position(|permission| {
permission.entry.profile_id() == profile_id
&& permission.entry.origin() == origin
&& permission.entry.feature() == feature
&& permission.grant_revision == grant_revision
}) else {
return Ok(false);
};
let permission = self.transferred_site_permissions.remove(index);
self.record_site_permission_audit_event(
profile_id.clone(),
permission.entry.origin().clone(),
feature,
SitePermissionAuditAction::Consumed,
);
Ok(true)
}
pub(super) fn visible_site_permission_audit_events(&self) -> Vec<SitePermissionAuditEvent> {
@@ -79,6 +170,12 @@ impl BrowserCore {
) -> Result<(), CoreError> {
self.require_profile(profile_id)?;
self.transferred_site_permissions.retain(|permission| {
permission.entry.profile_id() != profile_id
|| permission.entry.origin() != &origin
|| permission.entry.feature() != feature
});
if let Some(entry) = self.site_permission_entry_mut(profile_id, &origin, feature) {
if entry.decision() == decision {
return Ok(());
@@ -109,11 +206,20 @@ impl BrowserCore {
feature: SitePermissionFeature,
) -> Result<(), CoreError> {
self.require_profile(profile_id)?;
let Some(index) = self.site_permission_entry_index(profile_id, origin, feature) else {
return Ok(());
};
let entry = self.site_permissions.remove(index);
let entry =
if let Some(index) = self.site_permission_entry_index(profile_id, origin, feature) {
self.site_permissions.remove(index)
} else if let Some(index) =
self.transferred_site_permissions.iter().position(|permission| {
permission.entry.profile_id() == profile_id
&& permission.entry.origin() == origin
&& permission.entry.feature() == feature
})
{
self.transferred_site_permissions.remove(index).entry
} else {
return Ok(());
};
self.record_site_permission_audit_event(
profile_id.clone(),
entry.origin().clone(),
@@ -138,6 +244,15 @@ impl BrowserCore {
true
}
});
self.transferred_site_permissions.retain(|permission| {
if permission.entry.profile_id() == profile_id {
revoked_permissions
.push((permission.entry.origin().clone(), permission.entry.feature()));
false
} else {
true
}
});
let revoked_count = revoked_permissions.len();
for (origin, feature) in revoked_permissions {
@@ -15,6 +15,7 @@ impl BrowserCore {
self.site_permissions
.iter()
.filter(|entry| self.profile_allows_cloud_sync(entry.profile_id()))
.filter(|entry| entry.decision() != SitePermissionDecision::AllowOnce)
.collect()
}
@@ -30,6 +31,15 @@ impl BrowserCore {
SitePermissionFeature::parse(&record.feature).map_err(snapshot_schema_error)?;
let decision =
SitePermissionDecision::parse(&record.decision).map_err(snapshot_schema_error)?;
if decision == SitePermissionDecision::AllowOnce {
summary.record_skipped();
return Ok(());
}
self.transferred_site_permissions.retain(|permission| {
permission.entry.profile_id() != &profile_id
|| permission.entry.origin() != &origin
|| permission.entry.feature() != feature
});
let existing_index = self.site_permissions.iter().position(|entry| {
entry.profile_id() == &profile_id
&& entry.origin() == &origin
@@ -51,3 +61,32 @@ impl BrowserCore {
Ok(())
}
}
#[cfg(test)]
mod tests {
use super::*;
use crate::{InitialBrowserConfig, sync_engine::SyncSnapshotApplySummary};
#[test]
fn legacy_allow_once_sync_record_is_skipped() -> Result<(), Box<dyn std::error::Error>> {
let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?;
let profile_id = core.snapshot()?.active_profile_id;
let record = SitePermissionSyncRecord {
profile_id: profile_id.as_str().to_string(),
origin: "https://example.com".to_string(),
feature: "camera".to_string(),
decision: "allow-once".to_string(),
};
let mut summary = SyncSnapshotApplySummary::default();
core.apply_site_permission_sync_record(
record,
&mut summary,
&SyncSnapshotApplyContext::default(),
)?;
assert_eq!(summary.skipped(), 1);
assert!(core.snapshot()?.site_permissions.is_empty());
Ok(())
}
}
+26
View File
@@ -136,6 +136,32 @@ fn export_local_data_command_opens_privacy_security_page() -> Result<(), Box<dyn
Ok(())
}
#[test]
fn local_data_export_omits_ephemeral_allow_once_state() -> Result<(), Box<dyn Error>> {
let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?;
let profile_id = core.snapshot()?.active_profile_id;
let origin = SiteOrigin::parse("https://example.com")?;
let feature = SitePermissionFeature::Camera;
core.set_site_permission(origin.clone(), feature, SitePermissionDecision::AllowOnce)?;
let document: serde_json::Value =
serde_json::from_str(&core.export_local_data_package_json()?)?;
let inventory = core.active_profile_local_data_inventory();
assert_eq!(inventory.site_permissions(), 0);
assert_eq!(array_len(&document, "site_permissions"), 0);
assert_eq!(array_len(&document, "site_permission_audit_events"), 1);
let revision = core.site_permission_revision(&profile_id, &origin, feature);
assert!(core.transfer_site_permission_once(&profile_id, &origin, feature, revision)?);
assert!(core.finish_site_permission_once(&profile_id, &origin, feature, revision)?);
let consumed: serde_json::Value =
serde_json::from_str(&core.export_local_data_package_json()?)?;
assert_eq!(array_len(&consumed, "site_permission_audit_events"), 3);
assert_eq!(consumed["site_permission_audit_events"][2]["action"]["kind"], "consumed");
Ok(())
}
fn array_len(document: &serde_json::Value, field: &str) -> usize {
document[field].as_array().map_or(0, Vec::len)
}
+10 -1
View File
@@ -91,8 +91,17 @@ fn clear_active_profile_site_data_reports_removed_counts() -> Result<(), Box<dyn
core.set_site_permission(
origin.clone(),
SitePermissionFeature::Camera,
SitePermissionDecision::AllowAlways,
SitePermissionDecision::AllowOnce,
)?;
let profile_id = core.snapshot()?.active_profile_id;
let revision =
core.site_permission_revision(&profile_id, &origin, SitePermissionFeature::Camera);
assert!(core.transfer_site_permission_once(
&profile_id,
&origin,
SitePermissionFeature::Camera,
revision,
)?);
core.set_site_permission(
origin,
SitePermissionFeature::Notifications,
@@ -140,6 +140,73 @@ fn clear_site_permissions_without_entries_is_empty_change() -> Result<(), Box<dy
Ok(())
}
#[test]
fn allow_once_transfer_and_finish_require_the_token_revision() -> Result<(), Box<dyn Error>> {
let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?;
let snapshot = core.snapshot()?;
let profile_id = snapshot.active_profile_id;
let origin = SiteOrigin::parse("https://example.com")?;
let feature = SitePermissionFeature::Camera;
core.set_site_permission(origin.clone(), feature, SitePermissionDecision::AllowOnce)?;
let revision = core.site_permission_revision(&profile_id, &origin, feature);
assert!(!core.transfer_site_permission_once(&profile_id, &origin, feature, revision + 1,)?);
assert!(core.transfer_site_permission_once(&profile_id, &origin, feature, revision)?);
let snapshot = core.snapshot()?;
assert_eq!(snapshot.site_permissions.len(), 1);
assert_eq!(
snapshot.site_permission_audit_events.last().map(|event| event.action()),
Some(&SitePermissionAuditAction::Transferred),
);
assert!(!core.finish_site_permission_once(&profile_id, &origin, feature, revision + 1)?);
assert!(core.finish_site_permission_once(&profile_id, &origin, feature, revision)?);
let snapshot = core.snapshot()?;
assert!(snapshot.site_permissions.is_empty());
assert_eq!(
snapshot.site_permission_audit_events.last().map(|event| event.action()),
Some(&SitePermissionAuditAction::Consumed),
);
Ok(())
}
#[test]
fn clear_site_permissions_revokes_transferred_allow_once() -> Result<(), Box<dyn Error>> {
let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?;
let profile_id = core.snapshot()?.active_profile_id;
let origin = SiteOrigin::parse("https://example.com")?;
let feature = SitePermissionFeature::Camera;
core.set_site_permission(origin.clone(), feature, SitePermissionDecision::AllowOnce)?;
let revision = core.site_permission_revision(&profile_id, &origin, feature);
assert!(core.transfer_site_permission_once(&profile_id, &origin, feature, revision)?);
assert_eq!(core.clear_active_profile_site_permissions()?, 1);
let snapshot = core.snapshot()?;
assert!(snapshot.site_permissions.is_empty());
assert_eq!(
snapshot.site_permission_audit_events.last().map(|event| event.action()),
Some(&SitePermissionAuditAction::Revoked),
);
Ok(())
}
#[test]
fn revoke_then_readd_advances_permission_revision() -> Result<(), Box<dyn Error>> {
let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?;
let profile_id = core.snapshot()?.active_profile_id;
let origin = SiteOrigin::parse("https://example.com")?;
let feature = SitePermissionFeature::Location;
core.set_site_permission(origin.clone(), feature, SitePermissionDecision::AllowOnce)?;
let first_revision = core.site_permission_revision(&profile_id, &origin, feature);
core.revoke_site_permission(&origin, feature)?;
core.set_site_permission(origin.clone(), feature, SitePermissionDecision::AllowOnce)?;
assert!(core.site_permission_revision(&profile_id, &origin, feature) > first_revision);
Ok(())
}
#[test]
fn site_settings_command_opens_active_origin() -> Result<(), Box<dyn Error>> {
let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?;
@@ -98,3 +98,25 @@ fn sync_snapshot_omits_paused_site_permissions() -> Result<(), Box<dyn Error>> {
assert!(snapshot.site_permissions.is_empty());
Ok(())
}
#[test]
fn sync_snapshot_omits_allow_once_permissions() -> Result<(), Box<dyn Error>> {
let mut source = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?;
let source_home_tab_id = source.snapshot()?.active_tab_id;
source.set_tab_sync_enabled(&source_home_tab_id, false)?;
source.set_site_permission(
SiteOrigin::parse("https://example.com")?,
SitePermissionFeature::Camera,
SitePermissionDecision::AllowOnce,
)?;
let bytes = source.build_sync_snapshot_bytes()?;
let mut target = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?;
let summary = target.apply_sync_snapshot_bytes(&bytes)?;
assert_eq!(summary.imported(), 0);
assert_eq!(summary.updated(), 0);
assert_eq!(summary.skipped(), 0);
assert!(target.snapshot()?.site_permissions.is_empty());
Ok(())
}