From 1b801406d6bae8d9773e62d841bba362b7880fbb Mon Sep 17 00:00:00 2001 From: Stefan Melmuk <509385+stefan0xC@users.noreply.github.com> Date: Thu, 25 Jan 2024 22:02:07 +0100 Subject: [PATCH] prevent side effects if groups are disabled (#4265) --- src/api/core/ciphers.rs | 21 ++-- src/api/core/organizations.rs | 2 +- src/db/models/cipher.rs | 124 +++++++++++++------- src/db/models/collection.rs | 205 +++++++++++++++++++++------------- 4 files changed, 226 insertions(+), 126 deletions(-) diff --git a/src/api/core/ciphers.rs b/src/api/core/ciphers.rs index a4a2d836..b07ed9d8 100644 --- a/src/api/core/ciphers.rs +++ b/src/api/core/ciphers.rs @@ -1788,15 +1788,22 @@ impl CipherSyncData { .collect(); // Generate a HashMap with the collections_uuid as key and the CollectionGroup record - let user_collections_groups: HashMap = CollectionGroup::find_by_user(user_uuid, conn) - .await - .into_iter() - .map(|collection_group| (collection_group.collections_uuid.clone(), collection_group)) - .collect(); + let user_collections_groups: HashMap = if CONFIG.org_groups_enabled() { + CollectionGroup::find_by_user(user_uuid, conn) + .await + .into_iter() + .map(|collection_group| (collection_group.collections_uuid.clone(), collection_group)) + .collect() + } else { + HashMap::new() + }; // Get all organizations that the user has full access to via group assignment - let user_group_full_access_for_organizations: HashSet = - Group::gather_user_organizations_full_access(user_uuid, conn).await.into_iter().collect(); + let user_group_full_access_for_organizations: HashSet = if CONFIG.org_groups_enabled() { + Group::gather_user_organizations_full_access(user_uuid, conn).await.into_iter().collect() + } else { + HashSet::new() + }; Self { cipher_attachments, diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index 25cd5355..eac2c34c 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -294,7 +294,7 @@ async fn post_organization( async fn get_user_collections(headers: Headers, mut conn: DbConn) -> Json { Json(json!({ "Data": - Collection::find_by_user_uuid(headers.user.uuid.clone(), &mut conn).await + Collection::find_by_user_uuid(headers.user.uuid, &mut conn).await .iter() .map(Collection::to_json) .collect::(), diff --git a/src/db/models/cipher.rs b/src/db/models/cipher.rs index ec5c5d6a..61683d85 100644 --- a/src/db/models/cipher.rs +++ b/src/db/models/cipher.rs @@ -426,6 +426,9 @@ impl Cipher { cipher_sync_data: Option<&CipherSyncData>, conn: &mut DbConn, ) -> bool { + if !CONFIG.org_groups_enabled() { + return false; + } if let Some(ref org_uuid) = self.organization_uuid { if let Some(cipher_sync_data) = cipher_sync_data { return cipher_sync_data.user_group_full_access_for_organizations.get(org_uuid).is_some(); @@ -521,6 +524,9 @@ impl Cipher { } async fn get_group_collections_access_flags(&self, user_uuid: &str, conn: &mut DbConn) -> Vec<(bool, bool)> { + if !CONFIG.org_groups_enabled() { + return Vec::new(); + } db_run! {conn: { ciphers::table .filter(ciphers::uuid.eq(&self.uuid)) @@ -602,50 +608,84 @@ impl Cipher { // result, those ciphers will not appear in "My Vault" for the org // owner/admin, but they can still be accessed via the org vault view. pub async fn find_by_user(user_uuid: &str, visible_only: bool, conn: &mut DbConn) -> Vec { - db_run! {conn: { - let mut query = ciphers::table - .left_join(ciphers_collections::table.on( - ciphers::uuid.eq(ciphers_collections::cipher_uuid) - )) - .left_join(users_organizations::table.on( - ciphers::organization_uuid.eq(users_organizations::org_uuid.nullable()) - .and(users_organizations::user_uuid.eq(user_uuid)) - .and(users_organizations::status.eq(UserOrgStatus::Confirmed as i32)) - )) - .left_join(users_collections::table.on( - ciphers_collections::collection_uuid.eq(users_collections::collection_uuid) - // Ensure that users_collections::user_uuid is NULL for unconfirmed users. - .and(users_organizations::user_uuid.eq(users_collections::user_uuid)) - )) - .left_join(groups_users::table.on( - groups_users::users_organizations_uuid.eq(users_organizations::uuid) - )) - .left_join(groups::table.on( - groups::uuid.eq(groups_users::groups_uuid) - )) - .left_join(collections_groups::table.on( - collections_groups::collections_uuid.eq(ciphers_collections::collection_uuid).and( - collections_groups::groups_uuid.eq(groups::uuid) - ) - )) - .filter(ciphers::user_uuid.eq(user_uuid)) // Cipher owner - .or_filter(users_organizations::access_all.eq(true)) // access_all in org - .or_filter(users_collections::user_uuid.eq(user_uuid)) // Access to collection - .or_filter(groups::access_all.eq(true)) // Access via groups - .or_filter(collections_groups::collections_uuid.is_not_null()) // Access via groups - .into_boxed(); + if CONFIG.org_groups_enabled() { + db_run! {conn: { + let mut query = ciphers::table + .left_join(ciphers_collections::table.on( + ciphers::uuid.eq(ciphers_collections::cipher_uuid) + )) + .left_join(users_organizations::table.on( + ciphers::organization_uuid.eq(users_organizations::org_uuid.nullable()) + .and(users_organizations::user_uuid.eq(user_uuid)) + .and(users_organizations::status.eq(UserOrgStatus::Confirmed as i32)) + )) + .left_join(users_collections::table.on( + ciphers_collections::collection_uuid.eq(users_collections::collection_uuid) + // Ensure that users_collections::user_uuid is NULL for unconfirmed users. + .and(users_organizations::user_uuid.eq(users_collections::user_uuid)) + )) + .left_join(groups_users::table.on( + groups_users::users_organizations_uuid.eq(users_organizations::uuid) + )) + .left_join(groups::table.on( + groups::uuid.eq(groups_users::groups_uuid) + )) + .left_join(collections_groups::table.on( + collections_groups::collections_uuid.eq(ciphers_collections::collection_uuid).and( + collections_groups::groups_uuid.eq(groups::uuid) + ) + )) + .filter(ciphers::user_uuid.eq(user_uuid)) // Cipher owner + .or_filter(users_organizations::access_all.eq(true)) // access_all in org + .or_filter(users_collections::user_uuid.eq(user_uuid)) // Access to collection + .or_filter(groups::access_all.eq(true)) // Access via groups + .or_filter(collections_groups::collections_uuid.is_not_null()) // Access via groups + .into_boxed(); - if !visible_only { - query = query.or_filter( - users_organizations::atype.le(UserOrgType::Admin as i32) // Org admin/owner - ); - } + if !visible_only { + query = query.or_filter( + users_organizations::atype.le(UserOrgType::Admin as i32) // Org admin/owner + ); + } - query - .select(ciphers::all_columns) - .distinct() - .load::(conn).expect("Error loading ciphers").from_db() - }} + query + .select(ciphers::all_columns) + .distinct() + .load::(conn).expect("Error loading ciphers").from_db() + }} + } else { + db_run! {conn: { + let mut query = ciphers::table + .left_join(ciphers_collections::table.on( + ciphers::uuid.eq(ciphers_collections::cipher_uuid) + )) + .left_join(users_organizations::table.on( + ciphers::organization_uuid.eq(users_organizations::org_uuid.nullable()) + .and(users_organizations::user_uuid.eq(user_uuid)) + .and(users_organizations::status.eq(UserOrgStatus::Confirmed as i32)) + )) + .left_join(users_collections::table.on( + ciphers_collections::collection_uuid.eq(users_collections::collection_uuid) + // Ensure that users_collections::user_uuid is NULL for unconfirmed users. + .and(users_organizations::user_uuid.eq(users_collections::user_uuid)) + )) + .filter(ciphers::user_uuid.eq(user_uuid)) // Cipher owner + .or_filter(users_organizations::access_all.eq(true)) // access_all in org + .or_filter(users_collections::user_uuid.eq(user_uuid)) // Access to collection + .into_boxed(); + + if !visible_only { + query = query.or_filter( + users_organizations::atype.le(UserOrgType::Admin as i32) // Org admin/owner + ); + } + + query + .select(ciphers::all_columns) + .distinct() + .load::(conn).expect("Error loading ciphers").from_db() + }} + } } // Find all ciphers visible to the specified user. diff --git a/src/db/models/collection.rs b/src/db/models/collection.rs index f2b04ce5..4d3ccd2e 100644 --- a/src/db/models/collection.rs +++ b/src/db/models/collection.rs @@ -1,6 +1,7 @@ use serde_json::Value; use super::{CollectionGroup, User, UserOrgStatus, UserOrgType, UserOrganization}; +use crate::CONFIG; db_object! { #[derive(Identifiable, Queryable, Insertable, AsChangeset)] @@ -181,47 +182,74 @@ impl Collection { } pub async fn find_by_user_uuid(user_uuid: String, conn: &mut DbConn) -> Vec { - db_run! { conn: { - collections::table - .left_join(users_collections::table.on( - users_collections::collection_uuid.eq(collections::uuid).and( - users_collections::user_uuid.eq(user_uuid.clone()) + if CONFIG.org_groups_enabled() { + db_run! { conn: { + collections::table + .left_join(users_collections::table.on( + users_collections::collection_uuid.eq(collections::uuid).and( + users_collections::user_uuid.eq(user_uuid.clone()) + ) + )) + .left_join(users_organizations::table.on( + collections::org_uuid.eq(users_organizations::org_uuid).and( + users_organizations::user_uuid.eq(user_uuid.clone()) + ) + )) + .left_join(groups_users::table.on( + groups_users::users_organizations_uuid.eq(users_organizations::uuid) + )) + .left_join(groups::table.on( + groups::uuid.eq(groups_users::groups_uuid) + )) + .left_join(collections_groups::table.on( + collections_groups::groups_uuid.eq(groups_users::groups_uuid).and( + collections_groups::collections_uuid.eq(collections::uuid) + ) + )) + .filter( + users_organizations::status.eq(UserOrgStatus::Confirmed as i32) ) - )) - .left_join(users_organizations::table.on( - collections::org_uuid.eq(users_organizations::org_uuid).and( - users_organizations::user_uuid.eq(user_uuid.clone()) - ) - )) - .left_join(groups_users::table.on( - groups_users::users_organizations_uuid.eq(users_organizations::uuid) - )) - .left_join(groups::table.on( - groups::uuid.eq(groups_users::groups_uuid) - )) - .left_join(collections_groups::table.on( - collections_groups::groups_uuid.eq(groups_users::groups_uuid).and( - collections_groups::collections_uuid.eq(collections::uuid) - ) - )) - .filter( - users_organizations::status.eq(UserOrgStatus::Confirmed as i32) - ) - .filter( - users_collections::user_uuid.eq(user_uuid).or( // Directly accessed collection - users_organizations::access_all.eq(true) // access_all in Organization - ).or( - groups::access_all.eq(true) // access_all in groups - ).or( // access via groups - groups_users::users_organizations_uuid.eq(users_organizations::uuid).and( - collections_groups::collections_uuid.is_not_null() + .filter( + users_collections::user_uuid.eq(user_uuid).or( // Directly accessed collection + users_organizations::access_all.eq(true) // access_all in Organization + ).or( + groups::access_all.eq(true) // access_all in groups + ).or( // access via groups + groups_users::users_organizations_uuid.eq(users_organizations::uuid).and( + collections_groups::collections_uuid.is_not_null() + ) ) ) - ) - .select(collections::all_columns) - .distinct() - .load::(conn).expect("Error loading collections").from_db() - }} + .select(collections::all_columns) + .distinct() + .load::(conn).expect("Error loading collections").from_db() + }} + } else { + db_run! { conn: { + collections::table + .left_join(users_collections::table.on( + users_collections::collection_uuid.eq(collections::uuid).and( + users_collections::user_uuid.eq(user_uuid.clone()) + ) + )) + .left_join(users_organizations::table.on( + collections::org_uuid.eq(users_organizations::org_uuid).and( + users_organizations::user_uuid.eq(user_uuid.clone()) + ) + )) + .filter( + users_organizations::status.eq(UserOrgStatus::Confirmed as i32) + ) + .filter( + users_collections::user_uuid.eq(user_uuid).or( // Directly accessed collection + users_organizations::access_all.eq(true) // access_all in Organization + ) + ) + .select(collections::all_columns) + .distinct() + .load::(conn).expect("Error loading collections").from_db() + }} + } } // Check if a user has access to a specific collection @@ -277,45 +305,70 @@ impl Collection { } pub async fn find_by_uuid_and_user(uuid: &str, user_uuid: String, conn: &mut DbConn) -> Option { - db_run! { conn: { - collections::table - .left_join(users_collections::table.on( - users_collections::collection_uuid.eq(collections::uuid).and( - users_collections::user_uuid.eq(user_uuid.clone()) - ) - )) - .left_join(users_organizations::table.on( - collections::org_uuid.eq(users_organizations::org_uuid).and( - users_organizations::user_uuid.eq(user_uuid) - ) - )) - .left_join(groups_users::table.on( - groups_users::users_organizations_uuid.eq(users_organizations::uuid) - )) - .left_join(groups::table.on( - groups::uuid.eq(groups_users::groups_uuid) - )) - .left_join(collections_groups::table.on( - collections_groups::groups_uuid.eq(groups_users::groups_uuid).and( - collections_groups::collections_uuid.eq(collections::uuid) - ) - )) - .filter(collections::uuid.eq(uuid)) - .filter( - users_collections::collection_uuid.eq(uuid).or( // Directly accessed collection - users_organizations::access_all.eq(true).or( // access_all in Organization - users_organizations::atype.le(UserOrgType::Admin as i32) // Org admin or owner - )).or( - groups::access_all.eq(true) // access_all in groups - ).or( // access via groups - groups_users::users_organizations_uuid.eq(users_organizations::uuid).and( - collections_groups::collections_uuid.is_not_null() + if CONFIG.org_groups_enabled() { + db_run! { conn: { + collections::table + .left_join(users_collections::table.on( + users_collections::collection_uuid.eq(collections::uuid).and( + users_collections::user_uuid.eq(user_uuid.clone()) ) - ) - ).select(collections::all_columns) - .first::(conn).ok() - .from_db() - }} + )) + .left_join(users_organizations::table.on( + collections::org_uuid.eq(users_organizations::org_uuid).and( + users_organizations::user_uuid.eq(user_uuid) + ) + )) + .left_join(groups_users::table.on( + groups_users::users_organizations_uuid.eq(users_organizations::uuid) + )) + .left_join(groups::table.on( + groups::uuid.eq(groups_users::groups_uuid) + )) + .left_join(collections_groups::table.on( + collections_groups::groups_uuid.eq(groups_users::groups_uuid).and( + collections_groups::collections_uuid.eq(collections::uuid) + ) + )) + .filter(collections::uuid.eq(uuid)) + .filter( + users_collections::collection_uuid.eq(uuid).or( // Directly accessed collection + users_organizations::access_all.eq(true).or( // access_all in Organization + users_organizations::atype.le(UserOrgType::Admin as i32) // Org admin or owner + )).or( + groups::access_all.eq(true) // access_all in groups + ).or( // access via groups + groups_users::users_organizations_uuid.eq(users_organizations::uuid).and( + collections_groups::collections_uuid.is_not_null() + ) + ) + ).select(collections::all_columns) + .first::(conn).ok() + .from_db() + }} + } else { + db_run! { conn: { + collections::table + .left_join(users_collections::table.on( + users_collections::collection_uuid.eq(collections::uuid).and( + users_collections::user_uuid.eq(user_uuid.clone()) + ) + )) + .left_join(users_organizations::table.on( + collections::org_uuid.eq(users_organizations::org_uuid).and( + users_organizations::user_uuid.eq(user_uuid) + ) + )) + .filter(collections::uuid.eq(uuid)) + .filter( + users_collections::collection_uuid.eq(uuid).or( // Directly accessed collection + users_organizations::access_all.eq(true).or( // access_all in Organization + users_organizations::atype.le(UserOrgType::Admin as i32) // Org admin or owner + )) + ).select(collections::all_columns) + .first::(conn).ok() + .from_db() + }} + } } pub async fn is_writable_by_user(&self, user_uuid: &str, conn: &mut DbConn) -> bool {