Add some more validation for groups, collections and memberships

Signed-off-by: BlackDex <black.dex@gmail.com>

src/api/core/organizations.rs

@@ -131,6 +131,24 @@ struct FullCollectionData {
     external_id: Option<String>,
 }
 
+impl FullCollectionData {
+    pub async fn validate(&self, org_id: &OrganizationId, conn: &DbConn) -> EmptyResult {
+        let org_groups = Group::find_by_organization(org_id, conn).await;
+        let org_group_ids: HashSet<&GroupId> = org_groups.iter().map(|c| &c.uuid).collect();
+        if let Some(e) = self.groups.iter().find(|g| !org_group_ids.contains(&g.id)) {
+            err!("Invalid group", format!("Group {} does not belong to organization {}!", e.id, org_id))
+        }
+
+        let org_memberships = Membership::find_by_org(org_id, conn).await;
+        let org_membership_ids: HashSet<&MembershipId> = org_memberships.iter().map(|m| &m.uuid).collect();
+        if let Some(e) = self.users.iter().find(|m| !org_membership_ids.contains(&m.id)) {
+            err!("Invalid member", format!("Member {} does not belong to organization {}!", e.id, org_id))
+        }
+
+        Ok(())
+    }
+}
+
 #[derive(Deserialize)]
 #[serde(rename_all = "camelCase")]
 struct CollectionGroupData {
@@ -480,12 +498,9 @@ async fn post_organization_collections(
         err!("Organization not found", "Organization id's do not match");
     }
     let data: FullCollectionData = data.into_inner();
+    data.validate(&org_id, &conn).await?;
 
-    let Some(org) = Organization::find_by_uuid(&org_id, &conn).await else {
-        err!("Can't find organization details")
-    };
-
-    let collection = Collection::new(org.uuid, data.name, data.external_id);
+    let collection = Collection::new(org_id.clone(), data.name, data.external_id);
     collection.save(&conn).await?;
 
     log_event(
@@ -501,7 +516,7 @@ async fn post_organization_collections(
 
     for group in data.groups {
         CollectionGroup::new(collection.uuid.clone(), group.id, group.read_only, group.hide_passwords, group.manage)
-            .save(&conn)
+            .save(&org_id, &conn)
             .await?;
     }
 
@@ -579,10 +594,10 @@ async fn post_bulk_access_collections(
         )
         .await;
 
-        CollectionGroup::delete_all_by_collection(&col_id, &conn).await?;
+        CollectionGroup::delete_all_by_collection(&col_id, &org_id, &conn).await?;
         for group in &data.groups {
             CollectionGroup::new(col_id.clone(), group.id.clone(), group.read_only, group.hide_passwords, group.manage)
-                .save(&conn)
+                .save(&org_id, &conn)
                 .await?;
         }
 
@@ -627,6 +642,7 @@ async fn post_organization_collection_update(
         err!("Organization not found", "Organization id's do not match");
     }
     let data: FullCollectionData = data.into_inner();
+    data.validate(&org_id, &conn).await?;
 
     if Organization::find_by_uuid(&org_id, &conn).await.is_none() {
         err!("Can't find organization details")
@@ -655,11 +671,11 @@ async fn post_organization_collection_update(
     )
     .await;
 
-    CollectionGroup::delete_all_by_collection(&col_id, &conn).await?;
+    CollectionGroup::delete_all_by_collection(&col_id, &org_id, &conn).await?;
 
     for group in data.groups {
         CollectionGroup::new(col_id.clone(), group.id, group.read_only, group.hide_passwords, group.manage)
-            .save(&conn)
+            .save(&org_id, &conn)
             .await?;
     }
 
@@ -1003,6 +1019,24 @@ struct InviteData {
     permissions: HashMap<String, Value>,
 }
 
+impl InviteData {
+    async fn validate(&self, org_id: &OrganizationId, conn: &DbConn) -> EmptyResult {
+        let org_collections = Collection::find_by_organization(org_id, conn).await;
+        let org_collection_ids: HashSet<&CollectionId> = org_collections.iter().map(|c| &c.uuid).collect();
+        if let Some(e) = self.collections.iter().flatten().find(|c| !org_collection_ids.contains(&c.id)) {
+            err!("Invalid collection", format!("Collection {} does not belong to organization {}!", e.id, org_id))
+        }
+
+        let org_groups = Group::find_by_organization(org_id, conn).await;
+        let org_group_ids: HashSet<&GroupId> = org_groups.iter().map(|c| &c.uuid).collect();
+        if let Some(e) = self.groups.iter().find(|g| !org_group_ids.contains(g)) {
+            err!("Invalid group", format!("Group {} does not belong to organization {}!", e, org_id))
+        }
+
+        Ok(())
+    }
+}
+
 #[post("/organizations/<org_id>/users/invite", data = "<data>")]
 async fn send_invite(
     org_id: OrganizationId,
@@ -1014,6 +1048,7 @@ async fn send_invite(
         err!("Organization not found", "Organization id's do not match");
     }
     let data: InviteData = data.into_inner();
+    data.validate(&org_id, &conn).await?;
 
     // HACK: We need the raw user-type to be sure custom role is selected to determine the access_all permission
     // The from_str() will convert the custom role type into a manager role type
@@ -2429,6 +2464,23 @@ impl GroupRequest {
 
         group
     }
+
+    /// Validate if all the collections and members belong to the provided organization
+    pub async fn validate(&self, org_id: &OrganizationId, conn: &DbConn) -> EmptyResult {
+        let org_collections = Collection::find_by_organization(org_id, conn).await;
+        let org_collection_ids: HashSet<&CollectionId> = org_collections.iter().map(|c| &c.uuid).collect();
+        if let Some(e) = self.collections.iter().find(|c| !org_collection_ids.contains(&c.id)) {
+            err!("Invalid collection", format!("Collection {} does not belong to organization {}!", e.id, org_id))
+        }
+
+        let org_memberships = Membership::find_by_org(org_id, conn).await;
+        let org_membership_ids: HashSet<&MembershipId> = org_memberships.iter().map(|m| &m.uuid).collect();
+        if let Some(e) = self.users.iter().find(|m| !org_membership_ids.contains(m)) {
+            err!("Invalid member", format!("Member {} does not belong to organization {}!", e, org_id))
+        }
+
+        Ok(())
+    }
 }
 
 #[derive(Deserialize, Serialize)]
@@ -2472,6 +2524,8 @@ async fn post_groups(
     }
 
     let group_request = data.into_inner();
+    group_request.validate(&org_id, &conn).await?;
+
     let group = group_request.to_group(&org_id);
 
     log_event(
@@ -2508,10 +2562,12 @@ async fn put_group(
     };
 
     let group_request = data.into_inner();
+    group_request.validate(&org_id, &conn).await?;
+
     let updated_group = group_request.update_group(group);
 
-    CollectionGroup::delete_all_by_group(&group_id, &conn).await?;
-    GroupUser::delete_all_by_group(&group_id, &conn).await?;
+    CollectionGroup::delete_all_by_group(&group_id, &org_id, &conn).await?;
+    GroupUser::delete_all_by_group(&group_id, &org_id, &conn).await?;
 
     log_event(
         EventType::GroupUpdated as i32,
@@ -2539,7 +2595,7 @@ async fn add_update_group(
 
     for col_selection in collections {
         let mut collection_group = col_selection.to_collection_group(group.uuid.clone());
-        collection_group.save(conn).await?;
+        collection_group.save(&org_id, conn).await?;
     }
 
     for assigned_member in members {
@@ -2632,7 +2688,7 @@ async fn _delete_group(
     )
     .await;
 
-    group.delete(conn).await
+    group.delete(org_id, conn).await
 }
 
 #[delete("/organizations/<org_id>/groups", data = "<data>")]
@@ -2691,7 +2747,7 @@ async fn get_group_members(
         err!("Group could not be found!", "Group uuid is invalid or does not belong to the organization")
     };
 
-    let group_members: Vec<MembershipId> = GroupUser::find_by_group(&group_id, &conn)
+    let group_members: Vec<MembershipId> = GroupUser::find_by_group(&group_id, &org_id, &conn)
         .await
         .iter()
         .map(|entry| entry.users_organizations_uuid.clone())
@@ -2719,9 +2775,15 @@ async fn put_group_members(
         err!("Group could not be found!", "Group uuid is invalid or does not belong to the organization")
     };
 
-    GroupUser::delete_all_by_group(&group_id, &conn).await?;
-
     let assigned_members = data.into_inner();
+
+    let org_memberships = Membership::find_by_org(&org_id, &conn).await;
+    let org_membership_ids: HashSet<&MembershipId> = org_memberships.iter().map(|m| &m.uuid).collect();
+    if let Some(e) = assigned_members.iter().find(|m| !org_membership_ids.contains(m)) {
+        err!("Invalid member", format!("Member {} does not belong to organization {}!", e, org_id))
+    }
+
+    GroupUser::delete_all_by_group(&group_id, &org_id, &conn).await?;
     for assigned_member in assigned_members {
         let mut user_entry = GroupUser::new(group_id.clone(), assigned_member.clone());
         user_entry.save(&conn).await?;

src/api/core/public.rs

@@ -156,7 +156,7 @@ async fn ldap_import(data: Json<OrgImportData>, token: PublicToken, conn: DbConn
                 }
             };
 
-            GroupUser::delete_all_by_group(&group_uuid, &conn).await?;
+            GroupUser::delete_all_by_group(&group_uuid, &org_id, &conn).await?;
 
             for ext_id in &group_data.member_external_ids {
                 if let Some(member) = Membership::find_by_external_id_and_org(ext_id, &org_id, &conn).await {

src/db/models/cipher.rs

@@ -668,10 +668,12 @@ impl Cipher {
             ciphers::table
                 .filter(ciphers::uuid.eq(&self.uuid))
                 .inner_join(ciphers_collections::table.on(
-                    ciphers::uuid.eq(ciphers_collections::cipher_uuid)))
+                    ciphers::uuid.eq(ciphers_collections::cipher_uuid)
+                ))
                 .inner_join(users_collections::table.on(
                     ciphers_collections::collection_uuid.eq(users_collections::collection_uuid)
-                        .and(users_collections::user_uuid.eq(user_uuid))))
+                        .and(users_collections::user_uuid.eq(user_uuid))
+                ))
                 .select((users_collections::read_only, users_collections::hide_passwords, users_collections::manage))
                 .load::<(bool, bool, bool)>(conn)
                 .expect("Error getting user access restrictions")
@@ -697,6 +699,9 @@ impl Cipher {
                 .inner_join(users_organizations::table.on(
                     users_organizations::uuid.eq(groups_users::users_organizations_uuid)
                 ))
+                .inner_join(groups::table.on(groups::uuid.eq(collections_groups::groups_uuid)
+                    .and(groups::organizations_uuid.eq(users_organizations::org_uuid))
+                ))
                 .filter(users_organizations::user_uuid.eq(user_uuid))
                 .select((collections_groups::read_only, collections_groups::hide_passwords, collections_groups::manage))
                 .load::<(bool, bool, bool)>(conn)
@@ -795,28 +800,28 @@ impl Cipher {
                 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())
+                        ciphers::organization_uuid.eq(users_organizations::org_uuid.nullable())
                             .and(users_organizations::user_uuid.eq(user_uuid))
                             .and(users_organizations::status.eq(MembershipStatus::Confirmed as i32))
-                            ))
+                    ))
                     .left_join(users_collections::table.on(
-                            ciphers_collections::collection_uuid.eq(users_collections::collection_uuid)
+                        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)
-                            ))
+                        groups_users::users_organizations_uuid.eq(users_organizations::uuid)
+                    ))
+                    .left_join(groups::table.on(groups::uuid.eq(groups_users::groups_uuid)
+                        // Ensure that group and membership belong to the same org
+                        .and(groups::organizations_uuid.eq(users_organizations::org_uuid))
+                    ))
                     .left_join(collections_groups::table.on(
-                            collections_groups::collections_uuid.eq(ciphers_collections::collection_uuid).and(
-                                collections_groups::groups_uuid.eq(groups::uuid)
-                                )
-                            ))
+                        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
@@ -986,7 +991,9 @@ impl Cipher {
                     .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(groups::table.on(groups::uuid.eq(groups_users::groups_uuid)
+                        .and(groups::organizations_uuid.eq(users_organizations::org_uuid))
+                    ))
                     .left_join(collections_groups::table.on(
                         collections_groups::collections_uuid.eq(ciphers_collections::collection_uuid)
                         .and(collections_groups::groups_uuid.eq(groups::uuid))
@@ -1047,7 +1054,9 @@ impl Cipher {
                     .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(groups::table.on(groups::uuid.eq(groups_users::groups_uuid)
+                        .and(groups::organizations_uuid.eq(users_organizations::org_uuid))
+                    ))
                     .left_join(collections_groups::table.on(
                         collections_groups::collections_uuid.eq(ciphers_collections::collection_uuid)
                         .and(collections_groups::groups_uuid.eq(groups::uuid))
@@ -1115,8 +1124,8 @@ impl Cipher {
             .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(groups::table.on(groups::uuid.eq(groups_users::groups_uuid)
+                .and(groups::organizations_uuid.eq(users_organizations::org_uuid))
             ))
             .left_join(collections_groups::table.on(
                 collections_groups::collections_uuid.eq(ciphers_collections::collection_uuid).and(

src/db/models/collection.rs

@@ -191,7 +191,7 @@ impl Collection {
         self.update_users_revision(conn).await;
         CollectionCipher::delete_all_by_collection(&self.uuid, conn).await?;
         CollectionUser::delete_all_by_collection(&self.uuid, conn).await?;
-        CollectionGroup::delete_all_by_collection(&self.uuid, conn).await?;
+        CollectionGroup::delete_all_by_collection(&self.uuid, &self.org_uuid, conn).await?;
 
         db_run! { conn: {
             diesel::delete(collections::table.filter(collections::uuid.eq(self.uuid)))
@@ -239,8 +239,8 @@ impl Collection {
                 .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(groups::table.on(groups::uuid.eq(groups_users::groups_uuid)
+                    .and(groups::organizations_uuid.eq(users_organizations::org_uuid))
                 ))
                 .left_join(collections_groups::table.on(
                     collections_groups::groups_uuid.eq(groups_users::groups_uuid).and(
@@ -355,8 +355,8 @@ impl Collection {
                 .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(groups::table.on(groups::uuid.eq(groups_users::groups_uuid)
+                    .and(groups::organizations_uuid.eq(users_organizations::org_uuid))
                 ))
                 .left_join(collections_groups::table.on(
                     collections_groups::groups_uuid.eq(groups_users::groups_uuid).and(
@@ -422,8 +422,8 @@ impl Collection {
                     .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(groups::table.on(groups::uuid.eq(groups_users::groups_uuid)
+                        .and(groups::organizations_uuid.eq(users_organizations::org_uuid))
                     ))
                     .left_join(collections_groups::table.on(
                         collections_groups::groups_uuid.eq(groups_users::groups_uuid)
@@ -484,8 +484,8 @@ impl Collection {
             .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(groups::table.on(groups::uuid.eq(groups_users::groups_uuid)
+                .and(groups::organizations_uuid.eq(users_organizations::org_uuid))
             ))
             .left_join(collections_groups::table.on(
                 collections_groups::groups_uuid.eq(groups_users::groups_uuid).and(
@@ -531,8 +531,8 @@ impl Collection {
             .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(groups::table.on(groups::uuid.eq(groups_users::groups_uuid)
+                .and(groups::organizations_uuid.eq(users_organizations::org_uuid))
             ))
             .left_join(collections_groups::table.on(
                 collections_groups::groups_uuid.eq(groups_users::groups_uuid).and(

src/db/models/group.rs

@@ -1,6 +1,6 @@
 use super::{CollectionId, Membership, MembershipId, OrganizationId, User, UserId};
 use crate::api::EmptyResult;
-use crate::db::schema::{collections_groups, groups, groups_users, users_organizations};
+use crate::db::schema::{collections, collections_groups, groups, groups_users, users_organizations};
 use crate::db::DbConn;
 use crate::error::MapResult;
 use chrono::{NaiveDateTime, Utc};
@@ -81,7 +81,7 @@ impl Group {
         // If both read_only and hide_passwords are false, then manage should be true
         // You can't have an entry with read_only and manage, or hide_passwords and manage
         // Or an entry with everything to false
-        let collections_groups: Vec<Value> = CollectionGroup::find_by_group(&self.uuid, conn)
+        let collections_groups: Vec<Value> = CollectionGroup::find_by_group(&self.uuid, &self.organizations_uuid, conn)
             .await
             .iter()
             .map(|entry| {
@@ -191,7 +191,7 @@ impl Group {
 
     pub async fn delete_all_by_organization(org_uuid: &OrganizationId, conn: &DbConn) -> EmptyResult {
         for group in Self::find_by_organization(org_uuid, conn).await {
-            group.delete(conn).await?;
+            group.delete(org_uuid, conn).await?;
         }
         Ok(())
     }
@@ -246,8 +246,8 @@ impl Group {
                 .inner_join(users_organizations::table.on(
                     users_organizations::uuid.eq(groups_users::users_organizations_uuid)
                 ))
-                .inner_join(groups::table.on(
-                    groups::uuid.eq(groups_users::groups_uuid)
+                .inner_join(groups::table.on(groups::uuid.eq(groups_users::groups_uuid)
+                    .and(groups::organizations_uuid.eq(users_organizations::org_uuid))
                 ))
                 .filter(users_organizations::user_uuid.eq(user_uuid))
                 .filter(groups::access_all.eq(true))
@@ -276,9 +276,9 @@ impl Group {
         }}
     }
 
-    pub async fn delete(&self, conn: &DbConn) -> EmptyResult {
-        CollectionGroup::delete_all_by_group(&self.uuid, conn).await?;
-        GroupUser::delete_all_by_group(&self.uuid, conn).await?;
+    pub async fn delete(&self, org_uuid: &OrganizationId, conn: &DbConn) -> EmptyResult {
+        CollectionGroup::delete_all_by_group(&self.uuid, org_uuid, conn).await?;
+        GroupUser::delete_all_by_group(&self.uuid, org_uuid, conn).await?;
 
         db_run! { conn: {
             diesel::delete(groups::table.filter(groups::uuid.eq(&self.uuid)))
@@ -306,8 +306,8 @@ impl Group {
 }
 
 impl CollectionGroup {
-    pub async fn save(&mut self, conn: &DbConn) -> EmptyResult {
-        let group_users = GroupUser::find_by_group(&self.groups_uuid, conn).await;
+    pub async fn save(&mut self, org_uuid: &OrganizationId, conn: &DbConn) -> EmptyResult {
+        let group_users = GroupUser::find_by_group(&self.groups_uuid, org_uuid, conn).await;
         for group_user in group_users {
             group_user.update_user_revision(conn).await;
         }
@@ -365,10 +365,19 @@ impl CollectionGroup {
         }
     }
 
-    pub async fn find_by_group(group_uuid: &GroupId, conn: &DbConn) -> Vec<Self> {
+    pub async fn find_by_group(group_uuid: &GroupId, org_uuid: &OrganizationId, conn: &DbConn) -> Vec<Self> {
         db_run! { conn: {
             collections_groups::table
+                .inner_join(groups::table.on(
+                    groups::uuid.eq(collections_groups::groups_uuid)
+                ))
+                .inner_join(collections::table.on(
+                    collections::uuid.eq(collections_groups::collections_uuid)
+                        .and(collections::org_uuid.eq(groups::organizations_uuid))
+                ))
                 .filter(collections_groups::groups_uuid.eq(group_uuid))
+                .filter(collections::org_uuid.eq(org_uuid))
+                .select(collections_groups::all_columns)
                 .load::<Self>(conn)
                 .expect("Error loading collection groups")
         }}
@@ -383,6 +392,13 @@ impl CollectionGroup {
                 .inner_join(users_organizations::table.on(
                     users_organizations::uuid.eq(groups_users::users_organizations_uuid)
                 ))
+                .inner_join(groups::table.on(groups::uuid.eq(collections_groups::groups_uuid)
+                    .and(groups::organizations_uuid.eq(users_organizations::org_uuid))
+                ))
+                .inner_join(collections::table.on(
+                    collections::uuid.eq(collections_groups::collections_uuid)
+                        .and(collections::org_uuid.eq(groups::organizations_uuid))
+                ))
                 .filter(users_organizations::user_uuid.eq(user_uuid))
                 .select(collections_groups::all_columns)
                 .load::<Self>(conn)
@@ -394,14 +410,20 @@ impl CollectionGroup {
         db_run! { conn: {
             collections_groups::table
                 .filter(collections_groups::collections_uuid.eq(collection_uuid))
+                .inner_join(collections::table.on(
+                    collections::uuid.eq(collections_groups::collections_uuid)
+                ))
+                .inner_join(groups::table.on(groups::uuid.eq(collections_groups::groups_uuid)
+                    .and(groups::organizations_uuid.eq(collections::org_uuid))
+                ))
                 .select(collections_groups::all_columns)
                 .load::<Self>(conn)
                 .expect("Error loading collection groups")
         }}
     }
 
-    pub async fn delete(&self, conn: &DbConn) -> EmptyResult {
-        let group_users = GroupUser::find_by_group(&self.groups_uuid, conn).await;
+    pub async fn delete(&self, org_uuid: &OrganizationId, conn: &DbConn) -> EmptyResult {
+        let group_users = GroupUser::find_by_group(&self.groups_uuid, org_uuid, conn).await;
         for group_user in group_users {
             group_user.update_user_revision(conn).await;
         }
@@ -415,8 +437,8 @@ impl CollectionGroup {
         }}
     }
 
-    pub async fn delete_all_by_group(group_uuid: &GroupId, conn: &DbConn) -> EmptyResult {
-        let group_users = GroupUser::find_by_group(group_uuid, conn).await;
+    pub async fn delete_all_by_group(group_uuid: &GroupId, org_uuid: &OrganizationId, conn: &DbConn) -> EmptyResult {
+        let group_users = GroupUser::find_by_group(group_uuid, org_uuid, conn).await;
         for group_user in group_users {
             group_user.update_user_revision(conn).await;
         }
@@ -429,10 +451,14 @@ impl CollectionGroup {
         }}
     }
 
-    pub async fn delete_all_by_collection(collection_uuid: &CollectionId, conn: &DbConn) -> EmptyResult {
+    pub async fn delete_all_by_collection(
+        collection_uuid: &CollectionId,
+        org_uuid: &OrganizationId,
+        conn: &DbConn,
+    ) -> EmptyResult {
         let collection_assigned_to_groups = CollectionGroup::find_by_collection(collection_uuid, conn).await;
         for collection_assigned_to_group in collection_assigned_to_groups {
-            let group_users = GroupUser::find_by_group(&collection_assigned_to_group.groups_uuid, conn).await;
+            let group_users = GroupUser::find_by_group(&collection_assigned_to_group.groups_uuid, org_uuid, conn).await;
             for group_user in group_users {
                 group_user.update_user_revision(conn).await;
             }
@@ -494,10 +520,19 @@ impl GroupUser {
         }
     }
 
-    pub async fn find_by_group(group_uuid: &GroupId, conn: &DbConn) -> Vec<Self> {
+    pub async fn find_by_group(group_uuid: &GroupId, org_uuid: &OrganizationId, conn: &DbConn) -> Vec<Self> {
         db_run! { conn: {
             groups_users::table
+                .inner_join(groups::table.on(
+                    groups::uuid.eq(groups_users::groups_uuid)
+                ))
+                .inner_join(users_organizations::table.on(
+                    users_organizations::uuid.eq(groups_users::users_organizations_uuid)
+                        .and(users_organizations::org_uuid.eq(groups::organizations_uuid))
+                ))
                 .filter(groups_users::groups_uuid.eq(group_uuid))
+                .filter(groups::organizations_uuid.eq(org_uuid))
+                .select(groups_users::all_columns)
                 .load::<Self>(conn)
                 .expect("Error loading group users")
         }}
@@ -522,6 +557,13 @@ impl GroupUser {
                 .inner_join(collections_groups::table.on(
                     collections_groups::groups_uuid.eq(groups_users::groups_uuid)
                 ))
+                .inner_join(groups::table.on(
+                    groups::uuid.eq(groups_users::groups_uuid)
+                ))
+                .inner_join(collections::table.on(
+                    collections::uuid.eq(collections_groups::collections_uuid)
+                        .and(collections::org_uuid.eq(groups::organizations_uuid))
+                ))
                 .filter(collections_groups::collections_uuid.eq(collection_uuid))
                 .filter(groups_users::users_organizations_uuid.eq(member_uuid))
                 .count()
@@ -575,8 +617,8 @@ impl GroupUser {
         }}
     }
 
-    pub async fn delete_all_by_group(group_uuid: &GroupId, conn: &DbConn) -> EmptyResult {
-        let group_users = GroupUser::find_by_group(group_uuid, conn).await;
+    pub async fn delete_all_by_group(group_uuid: &GroupId, org_uuid: &OrganizationId, conn: &DbConn) -> EmptyResult {
+        let group_users = GroupUser::find_by_group(group_uuid, org_uuid, conn).await;
         for group_user in group_users {
             group_user.update_user_revision(conn).await;
         }

src/db/models/organization.rs

@@ -1073,7 +1073,9 @@ impl Membership {
             .left_join(collections_groups::table.on(
                 collections_groups::groups_uuid.eq(groups_users::groups_uuid)
             ))
-            .left_join(groups::table.on(groups::uuid.eq(groups_users::groups_uuid)))
+            .left_join(groups::table.on(groups::uuid.eq(groups_users::groups_uuid)
+                .and(groups::organizations_uuid.eq(users_organizations::org_uuid))
+            ))
             .left_join(ciphers_collections::table.on(
                     ciphers_collections::collection_uuid.eq(collections_groups::collections_uuid).and(ciphers_collections::cipher_uuid.eq(&cipher_uuid))