From 9e26014b4df65acc44ee2c0e94fada0914843ec3 Mon Sep 17 00:00:00 2001 From: Mathijs van Veluw Date: Thu, 15 Aug 2024 12:36:00 +0200 Subject: [PATCH] Fix manager in web-vault v2024.6.2 for collections (#4860) The web-vault v2024.6.2 we use needs some extra information to allow managers to actually be able to manage collections. The v2024.6.2 web-vault has somewhat of a mixture of the newer roles and older manager roles. To at least fix this for the web-vault we bundle these changes will make the manager able to manage. For future web-vaults we would need a lot more changes to be done to fix this in a better way though. Fixes #4844 --- src/api/core/organizations.rs | 11 +++---- src/db/models/collection.rs | 34 +++++++++++++++++----- src/db/models/group.rs | 16 +++++----- src/db/models/organization.rs | 55 +++++++++++++++++++++++++++++------ 4 files changed, 85 insertions(+), 31 deletions(-) diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index 8d9487e4..8f4f7130 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -509,7 +509,7 @@ async fn post_organization_collection_update( CollectionUser::save(&org_user.user_uuid, col_id, user.read_only, user.hide_passwords, &mut conn).await?; } - Ok(Json(collection.to_json())) + Ok(Json(collection.to_json_details(&headers.user.uuid, None, &mut conn).await)) } #[delete("/organizations//collections//user/")] @@ -2276,13 +2276,14 @@ async fn _restore_organization_user( } #[get("/organizations//groups")] -async fn get_groups(org_id: &str, _headers: ManagerHeadersLoose, mut conn: DbConn) -> JsonResult { +async fn get_groups(org_id: &str, headers: ManagerHeadersLoose, mut conn: DbConn) -> JsonResult { let groups: Vec = if CONFIG.org_groups_enabled() { // Group::find_by_organization(&org_id, &mut conn).await.iter().map(Group::to_json).collect::() let groups = Group::find_by_organization(org_id, &mut conn).await; let mut groups_json = Vec::with_capacity(groups.len()); + for g in groups { - groups_json.push(g.to_json_details(&mut conn).await) + groups_json.push(g.to_json_details(&headers.org_user.atype, &mut conn).await) } groups_json } else { @@ -2470,7 +2471,7 @@ async fn add_update_group( } #[get("/organizations/<_org_id>/groups//details")] -async fn get_group_details(_org_id: &str, group_id: &str, _headers: AdminHeaders, mut conn: DbConn) -> JsonResult { +async fn get_group_details(_org_id: &str, group_id: &str, headers: AdminHeaders, mut conn: DbConn) -> JsonResult { if !CONFIG.org_groups_enabled() { err!("Group support is disabled"); } @@ -2480,7 +2481,7 @@ async fn get_group_details(_org_id: &str, group_id: &str, _headers: AdminHeaders _ => err!("Group could not be found!"), }; - Ok(Json(group.to_json_details(&mut conn).await)) + Ok(Json(group.to_json_details(&(headers.org_user_type as i32), &mut conn).await)) } #[post("/organizations//groups//delete")] diff --git a/src/db/models/collection.rs b/src/db/models/collection.rs index 3ba6c516..7fb17c66 100644 --- a/src/db/models/collection.rs +++ b/src/db/models/collection.rs @@ -78,28 +78,46 @@ impl Collection { cipher_sync_data: Option<&crate::api::core::CipherSyncData>, conn: &mut DbConn, ) -> Value { - let (read_only, hide_passwords) = if let Some(cipher_sync_data) = cipher_sync_data { + let (read_only, hide_passwords, can_manage) = if let Some(cipher_sync_data) = cipher_sync_data { match cipher_sync_data.user_organizations.get(&self.org_uuid) { - Some(uo) if uo.has_full_access() => (false, false), - Some(_) => { + // Only for Manager types Bitwarden returns true for the can_manage option + // Owners and Admins always have false, but they can manage all collections anyway + Some(uo) if uo.has_full_access() => (false, false, uo.atype == UserOrgType::Manager), + Some(uo) => { + // Only let a manager manage collections when the have full read/write access + let is_manager = uo.atype == UserOrgType::Manager; if let Some(uc) = cipher_sync_data.user_collections.get(&self.uuid) { - (uc.read_only, uc.hide_passwords) + (uc.read_only, uc.hide_passwords, is_manager && !uc.read_only && !uc.hide_passwords) } else if let Some(cg) = cipher_sync_data.user_collections_groups.get(&self.uuid) { - (cg.read_only, cg.hide_passwords) + (cg.read_only, cg.hide_passwords, is_manager && !cg.read_only && !cg.hide_passwords) } else { - (false, false) + (false, false, false) } } - _ => (true, true), + _ => (true, true, false), } } else { - (!self.is_writable_by_user(user_uuid, conn).await, self.hide_passwords_for_user(user_uuid, conn).await) + match UserOrganization::find_confirmed_by_user_and_org(user_uuid, &self.org_uuid, conn).await { + Some(ou) if ou.has_full_access() => (false, false, ou.atype == UserOrgType::Manager), + Some(ou) => { + let is_manager = ou.atype == UserOrgType::Manager; + let read_only = !self.is_writable_by_user(user_uuid, conn).await; + let hide_passwords = self.hide_passwords_for_user(user_uuid, conn).await; + (read_only, hide_passwords, is_manager && !read_only && !hide_passwords) + } + _ => ( + !self.is_writable_by_user(user_uuid, conn).await, + self.hide_passwords_for_user(user_uuid, conn).await, + false, + ), + } }; let mut json_object = self.to_json(); json_object["object"] = json!("collectionDetails"); json_object["readOnly"] = json!(read_only); json_object["hidePasswords"] = json!(hide_passwords); + json_object["manage"] = json!(can_manage); json_object } diff --git a/src/db/models/group.rs b/src/db/models/group.rs index f6ccc710..6e2db088 100644 --- a/src/db/models/group.rs +++ b/src/db/models/group.rs @@ -1,3 +1,7 @@ +use super::{User, UserOrgType, UserOrganization}; +use crate::api::EmptyResult; +use crate::db::DbConn; +use crate::error::MapResult; use chrono::{NaiveDateTime, Utc}; use serde_json::Value; @@ -69,7 +73,7 @@ impl Group { }) } - pub async fn to_json_details(&self, conn: &mut DbConn) -> Value { + pub async fn to_json_details(&self, user_org_type: &i32, conn: &mut DbConn) -> Value { let collections_groups: Vec = CollectionGroup::find_by_group(&self.uuid, conn) .await .iter() @@ -77,7 +81,8 @@ impl Group { json!({ "id": entry.collections_uuid, "readOnly": entry.read_only, - "hidePasswords": entry.hide_passwords + "hidePasswords": entry.hide_passwords, + "manage": *user_org_type == UserOrgType::Manager && !entry.read_only && !entry.hide_passwords }) }) .collect(); @@ -122,13 +127,6 @@ impl GroupUser { } } -use crate::db::DbConn; - -use crate::api::EmptyResult; -use crate::error::MapResult; - -use super::{User, UserOrganization}; - /// Database methods impl Group { pub async fn save(&mut self, conn: &mut DbConn) -> EmptyResult { diff --git a/src/db/models/organization.rs b/src/db/models/organization.rs index ec390262..cb787017 100644 --- a/src/db/models/organization.rs +++ b/src/db/models/organization.rs @@ -1,9 +1,13 @@ use chrono::{NaiveDateTime, Utc}; use num_traits::FromPrimitive; use serde_json::Value; -use std::cmp::Ordering; +use std::{ + cmp::Ordering, + collections::{HashMap, HashSet}, +}; use super::{CollectionUser, Group, GroupUser, OrgPolicy, OrgPolicyType, TwoFactor, User}; +use crate::db::models::{Collection, CollectionGroup}; use crate::CONFIG; db_object! { @@ -453,15 +457,47 @@ impl UserOrganization { }; let collections: Vec = if include_collections { - CollectionUser::find_by_organization_and_user_uuid(&self.org_uuid, &self.user_uuid, conn) + // Get all collections for the user here already to prevent more queries + let cu: HashMap = + CollectionUser::find_by_organization_and_user_uuid(&self.org_uuid, &self.user_uuid, conn) + .await + .into_iter() + .map(|cu| (cu.collection_uuid.clone(), cu)) + .collect(); + + // Get all collection groups for this user to prevent there inclusion + let cg: HashSet = CollectionGroup::find_by_user(&self.user_uuid, conn) .await - .iter() - .map(|cu| { - json!({ - "id": cu.collection_uuid, - "readOnly": cu.read_only, - "hidePasswords": cu.hide_passwords, - }) + .into_iter() + .map(|cg| cg.collections_uuid) + .collect(); + + Collection::find_by_organization_and_user_uuid(&self.org_uuid, &self.user_uuid, conn) + .await + .into_iter() + .filter_map(|c| { + let (read_only, hide_passwords, can_manage) = if self.has_full_access() { + (false, false, self.atype == UserOrgType::Manager) + } else if let Some(cu) = cu.get(&c.uuid) { + ( + cu.read_only, + cu.hide_passwords, + self.atype == UserOrgType::Manager && !cu.read_only && !cu.hide_passwords, + ) + // If previous checks failed it might be that this user has access via a group, but we should not return those elements here + // Those are returned via a special group endpoint + } else if cg.contains(&c.uuid) { + return None; + } else { + (true, true, false) + }; + + Some(json!({ + "id": c.uuid, + "readOnly": read_only, + "hidePasswords": hide_passwords, + "manage": can_manage, + })) }) .collect() } else { @@ -474,6 +510,7 @@ impl UserOrganization { "name": user.name, "email": user.email, "externalId": self.external_id, + "avatarColor": user.avatar_color, "groups": groups, "collections": collections,