From 33fb3065552e5d5e4b85a9d0d871d0fa9e9f2db7 Mon Sep 17 00:00:00 2001 From: Richy Date: Thu, 3 Jul 2025 12:45:51 +0200 Subject: [PATCH 1/4] fix: resolve group permission conflicts with multiple groups When a user belonged to multiple groups with different permissions for the same collection, only the permissions from one group were applied instead of combining them properly. This caused users to see incorrect access levels when initially viewing collection items. The fix combines permissions from all user groups by taking the most permissive settings: - read_only: false if ANY group allows write access - hide_passwords: false if ANY group allows password viewing - manage: true if ANY group allows management This ensures users immediately see the correct permissions when opening collection entries, matching the behavior after editing and saving. --- src/api/core/ciphers.rs | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/api/core/ciphers.rs b/src/api/core/ciphers.rs index aecbe28a..b4a8e0ab 100644 --- a/src/api/core/ciphers.rs +++ b/src/api/core/ciphers.rs @@ -1924,11 +1924,24 @@ impl CipherSyncData { // Generate a HashMap with the collections_uuid as key and the CollectionGroup record let user_collections_groups: HashMap = if CONFIG.org_groups_enabled() { - CollectionGroup::find_by_user(user_id, conn) - .await - .into_iter() - .map(|collection_group| (collection_group.collections_uuid.clone(), collection_group)) - .collect() + let all_user_collection_groups = CollectionGroup::find_by_user(user_id, conn).await; + let mut combined_permissions: HashMap = HashMap::new(); + + for cg in all_user_collection_groups { + match combined_permissions.get_mut(&cg.collections_uuid) { + Some(existing) => { + // Combine permissions by taking the most permissive settings + existing.read_only = existing.read_only && cg.read_only; // false if ANY group allows write + existing.hide_passwords = existing.hide_passwords && cg.hide_passwords; // false if ANY group allows password view + existing.manage = existing.manage || cg.manage; // true if ANY group allows manage + } + None => { + // First group for this collection + combined_permissions.insert(cg.collections_uuid.clone(), cg); + } + } + } + combined_permissions } else { HashMap::new() }; From 0789a7567c1842d21b08d3631ae5bb2983b85376 Mon Sep 17 00:00:00 2001 From: Richy Date: Wed, 9 Jul 2025 18:54:29 +0200 Subject: [PATCH 2/4] Update src/api/core/ciphers.rs Co-authored-by: Mathijs van Veluw --- src/api/core/ciphers.rs | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/src/api/core/ciphers.rs b/src/api/core/ciphers.rs index b4a8e0ab..fd372b12 100644 --- a/src/api/core/ciphers.rs +++ b/src/api/core/ciphers.rs @@ -1924,24 +1924,20 @@ impl CipherSyncData { // Generate a HashMap with the collections_uuid as key and the CollectionGroup record let user_collections_groups: HashMap = if CONFIG.org_groups_enabled() { - let all_user_collection_groups = CollectionGroup::find_by_user(user_id, conn).await; - let mut combined_permissions: HashMap = HashMap::new(); - - for cg in all_user_collection_groups { - match combined_permissions.get_mut(&cg.collections_uuid) { - Some(existing) => { - // Combine permissions by taking the most permissive settings - existing.read_only = existing.read_only && cg.read_only; // false if ANY group allows write - existing.hide_passwords = existing.hide_passwords && cg.hide_passwords; // false if ANY group allows password view - existing.manage = existing.manage || cg.manage; // true if ANY group allows manage - } - None => { - // First group for this collection - combined_permissions.insert(cg.collections_uuid.clone(), cg); - } - } - } - combined_permissions + CollectionGroup::find_by_user(user_id, conn) + .await + .into_iter() + .fold(HashMap::new(), |mut combined_permissions, cg| { + combined_permissions.entry(cg.collections_uuid.clone()) + .and_modify(|existing| { + // Combine permissions: take the most permissive settings. + existing.read_only &= cg.read_only; // false if ANY group allows write + existing.hide_passwords &= cg.hide_passwords; // false if ANY group allows password view + existing.manage |= cg.manage; // true if ANY group allows manage + }) + .or_insert(cg); + combined_permissions + }) } else { HashMap::new() }; From e27941122c774bf6110d9456960f3aa309c55465 Mon Sep 17 00:00:00 2001 From: Richy Date: Wed, 9 Jul 2025 19:04:17 +0200 Subject: [PATCH 3/4] fix: format --- src/api/core/ciphers.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/api/core/ciphers.rs b/src/api/core/ciphers.rs index fd372b12..1c89aed7 100644 --- a/src/api/core/ciphers.rs +++ b/src/api/core/ciphers.rs @@ -1924,11 +1924,11 @@ impl CipherSyncData { // Generate a HashMap with the collections_uuid as key and the CollectionGroup record let user_collections_groups: HashMap = if CONFIG.org_groups_enabled() { - CollectionGroup::find_by_user(user_id, conn) - .await - .into_iter() - .fold(HashMap::new(), |mut combined_permissions, cg| { - combined_permissions.entry(cg.collections_uuid.clone()) + CollectionGroup::find_by_user(user_id, conn).await.into_iter().fold( + HashMap::new(), + |mut combined_permissions, cg| { + combined_permissions + .entry(cg.collections_uuid.clone()) .and_modify(|existing| { // Combine permissions: take the most permissive settings. existing.read_only &= cg.read_only; // false if ANY group allows write @@ -1937,7 +1937,8 @@ impl CipherSyncData { }) .or_insert(cg); combined_permissions - }) + }, + ) } else { HashMap::new() }; From fff8d86c3b41db9614d9d926d2eb5046a60e715c Mon Sep 17 00:00:00 2001 From: Richy Date: Sun, 20 Jul 2025 17:24:18 +0200 Subject: [PATCH 4/4] fix: restrict collection manage permissions to managers only Prevent users from getting logged out when they have manage permissions by only allowing manage permissions for MembershipType::Manager and higher roles. --- src/db/models/collection.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/db/models/collection.rs b/src/db/models/collection.rs index 4f192a25..c14c5946 100644 --- a/src/db/models/collection.rs +++ b/src/db/models/collection.rs @@ -97,13 +97,13 @@ impl Collection { ( cu.read_only, cu.hide_passwords, - cu.manage || (is_manager && !cu.read_only && !cu.hide_passwords), + is_manager && (cu.manage || (!cu.read_only && !cu.hide_passwords)), ) } else if let Some(cg) = cipher_sync_data.user_collections_groups.get(&self.uuid) { ( cg.read_only, cg.hide_passwords, - cg.manage || (is_manager && !cg.read_only && !cg.hide_passwords), + is_manager && (cg.manage || (!cg.read_only && !cg.hide_passwords)), ) } else { (false, false, false) @@ -114,7 +114,9 @@ impl Collection { } else { match Membership::find_confirmed_by_user_and_org(user_uuid, &self.org_uuid, conn).await { Some(m) if m.has_full_access() => (false, false, m.atype >= MembershipType::Manager), - Some(_) if self.is_manageable_by_user(user_uuid, conn).await => (false, false, true), + Some(m) if m.atype == MembershipType::Manager && self.is_manageable_by_user(user_uuid, conn).await => { + (false, false, true) + } Some(m) => { let is_manager = m.atype == MembershipType::Manager; let read_only = !self.is_writable_by_user(user_uuid, conn).await;