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
This commit is contained in:
Mathijs van Veluw 2024-08-15 12:36:00 +02:00 committed by GitHub
parent 339612c917
commit 9e26014b4d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 85 additions and 31 deletions

View File

@ -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?; 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/<org_id>/collections/<col_id>/user/<org_user_id>")] #[delete("/organizations/<org_id>/collections/<col_id>/user/<org_user_id>")]
@ -2276,13 +2276,14 @@ async fn _restore_organization_user(
} }
#[get("/organizations/<org_id>/groups")] #[get("/organizations/<org_id>/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<Value> = if CONFIG.org_groups_enabled() { let groups: Vec<Value> = if CONFIG.org_groups_enabled() {
// Group::find_by_organization(&org_id, &mut conn).await.iter().map(Group::to_json).collect::<Value>() // Group::find_by_organization(&org_id, &mut conn).await.iter().map(Group::to_json).collect::<Value>()
let groups = Group::find_by_organization(org_id, &mut conn).await; let groups = Group::find_by_organization(org_id, &mut conn).await;
let mut groups_json = Vec::with_capacity(groups.len()); let mut groups_json = Vec::with_capacity(groups.len());
for g in groups { 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 groups_json
} else { } else {
@ -2470,7 +2471,7 @@ async fn add_update_group(
} }
#[get("/organizations/<_org_id>/groups/<group_id>/details")] #[get("/organizations/<_org_id>/groups/<group_id>/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() { if !CONFIG.org_groups_enabled() {
err!("Group support is disabled"); 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!"), _ => 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/<org_id>/groups/<group_id>/delete")] #[post("/organizations/<org_id>/groups/<group_id>/delete")]

View File

@ -78,28 +78,46 @@ impl Collection {
cipher_sync_data: Option<&crate::api::core::CipherSyncData>, cipher_sync_data: Option<&crate::api::core::CipherSyncData>,
conn: &mut DbConn, conn: &mut DbConn,
) -> Value { ) -> 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) { match cipher_sync_data.user_organizations.get(&self.org_uuid) {
Some(uo) if uo.has_full_access() => (false, false), // Only for Manager types Bitwarden returns true for the can_manage option
Some(_) => { // 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) { 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) { } 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 { } else {
(false, false) (false, false, false)
} }
} }
_ => (true, true), _ => (true, true, false),
} }
} else { } 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(); let mut json_object = self.to_json();
json_object["object"] = json!("collectionDetails"); json_object["object"] = json!("collectionDetails");
json_object["readOnly"] = json!(read_only); json_object["readOnly"] = json!(read_only);
json_object["hidePasswords"] = json!(hide_passwords); json_object["hidePasswords"] = json!(hide_passwords);
json_object["manage"] = json!(can_manage);
json_object json_object
} }

View File

@ -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 chrono::{NaiveDateTime, Utc};
use serde_json::Value; 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<Value> = CollectionGroup::find_by_group(&self.uuid, conn) let collections_groups: Vec<Value> = CollectionGroup::find_by_group(&self.uuid, conn)
.await .await
.iter() .iter()
@ -77,7 +81,8 @@ impl Group {
json!({ json!({
"id": entry.collections_uuid, "id": entry.collections_uuid,
"readOnly": entry.read_only, "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(); .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 /// Database methods
impl Group { impl Group {
pub async fn save(&mut self, conn: &mut DbConn) -> EmptyResult { pub async fn save(&mut self, conn: &mut DbConn) -> EmptyResult {

View File

@ -1,9 +1,13 @@
use chrono::{NaiveDateTime, Utc}; use chrono::{NaiveDateTime, Utc};
use num_traits::FromPrimitive; use num_traits::FromPrimitive;
use serde_json::Value; 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 super::{CollectionUser, Group, GroupUser, OrgPolicy, OrgPolicyType, TwoFactor, User};
use crate::db::models::{Collection, CollectionGroup};
use crate::CONFIG; use crate::CONFIG;
db_object! { db_object! {
@ -453,15 +457,47 @@ impl UserOrganization {
}; };
let collections: Vec<Value> = if include_collections { let collections: Vec<Value> = 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<String, CollectionUser> =
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<String> = CollectionGroup::find_by_user(&self.user_uuid, conn)
.await .await
.iter() .into_iter()
.map(|cu| { .map(|cg| cg.collections_uuid)
json!({ .collect();
"id": cu.collection_uuid,
"readOnly": cu.read_only, Collection::find_by_organization_and_user_uuid(&self.org_uuid, &self.user_uuid, conn)
"hidePasswords": cu.hide_passwords, .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() .collect()
} else { } else {
@ -474,6 +510,7 @@ impl UserOrganization {
"name": user.name, "name": user.name,
"email": user.email, "email": user.email,
"externalId": self.external_id, "externalId": self.external_id,
"avatarColor": user.avatar_color,
"groups": groups, "groups": groups,
"collections": collections, "collections": collections,