Merge pull request #1 from stefan0xC/3624_manager_groups

improve check if the user has access via groups

src/api/core/organizations.rs

@@ -321,51 +321,48 @@ async fn get_org_collections_details(org_id: &str, headers: ManagerHeadersLoose,
         None => err!("User is not part of organization"),
     };
 
+    // get all collection memberships for the current organization
     let coll_users = CollectionUser::find_by_organization(org_id, &mut conn).await;
-    // uuids of users in groups having access to all collections
-    let has_full_access_via_group = if CONFIG.org_groups_enabled() {
-        GroupUser::get_members_of_full_access_groups(org_id, &mut conn).await
-    } else {
-        vec![]
-    };
 
-    let has_full_access = user_org.access_all || has_full_access_via_group.contains(&user_org.uuid);
+    // check if current user has full access to the organization (either directly or via any group)
+    let has_full_access_via_group =
+        CONFIG.org_groups_enabled() && GroupUser::has_full_access_by_member(org_id, &user_org.uuid, &mut conn).await;
+    let has_full_access_to_org = user_org.access_all || has_full_access_via_group;
 
     for col in Collection::find_by_organization(org_id, &mut conn).await {
-        let groups: Vec<Value> = if CONFIG.org_groups_enabled() {
-            CollectionGroup::find_by_collection(&col.uuid, &mut conn)
-                .await
-                .iter()
-                .map(|collection_group| {
-                    SelectionReadOnly::to_collection_group_details_read_only(collection_group).to_json()
-                })
-                .collect()
-        } else {
-            // The Bitwarden clients seem to call this API regardless of whether groups are enabled,
-            // so just act as if there are no groups.
-            Vec::with_capacity(0)
-        };
+        // assigned indicates whether the current user has access to the given collection
+        let mut assigned = has_full_access_to_org;
 
-        let mut assigned = has_full_access;
+        // get the users assigned directly to the given collection
         let users: Vec<Value> = coll_users
             .iter()
             .filter(|collection_user| collection_user.collection_uuid == col.uuid)
             .map(|collection_user| {
-                // Remember `user_uuid` is swapped here with the `user_org.uuid` with a join during the `CollectionUser::find_by_organization` call.
-                // We check here if the current user is assigned to this collection or not.
+                // check if the current user is assigned to this collection directly
                 if collection_user.user_uuid == user_org.uuid {
                     assigned = true;
                 }
                 SelectionReadOnly::to_collection_user_details_read_only(collection_user).to_json()
             })
             .collect();
 
-        // if the current user is not assigned and groups are enabled,
-        // check if they have access to the given collection via a group
-        if !assigned && CONFIG.org_groups_enabled()
-        {
-            assigned = GroupUser::get_group_members_for_collection(&col.uuid, &mut conn).await.contains(&user_org.uuid);
-        }     
+        // check if the current user has access to the given collection via a group
+        if !assigned && CONFIG.org_groups_enabled() {
+            assigned = GroupUser::has_access_to_collection_by_member(&col.uuid, &user_org.uuid, &mut conn).await;
+        }
+
+        // get the group details for the given collection
+        let groups: Vec<Value> = if CONFIG.org_groups_enabled() {
+            CollectionGroup::find_by_collection(&col.uuid, &mut conn)
+                .await
+                .iter()
+                .map(|collection_group| {
+                    SelectionReadOnly::to_collection_group_details_read_only(collection_group).to_json()
+                })
+                .collect()
+        } else {
+            Vec::with_capacity(0)
+        };
 
         let mut json_object = col.to_json();
         json_object["Assigned"] = json!(assigned);

src/db/models/group.rs

@@ -486,37 +486,37 @@ impl GroupUser {
         }}
     }
 
-    pub async fn get_group_members_for_collection(collection_uuid: &str, conn: &mut DbConn) -> Vec<String> {
+    pub async fn has_access_to_collection_by_member(
+        collection_uuid: &str,
+        member_uuid: &str,
+        conn: &mut DbConn,
+    ) -> bool {
         db_run! { conn: {
             groups_users::table
                 .inner_join(collections_groups::table.on(
                     collections_groups::groups_uuid.eq(groups_users::groups_uuid)
                 ))
                 .filter(collections_groups::collections_uuid.eq(collection_uuid))
-                .select(groups_users::users_organizations_uuid)
-                .distinct()
-                .load::<String>(conn)
-                .expect("Error loading group users for collection")
+                .filter(groups_users::users_organizations_uuid.eq(member_uuid))
+                .count()
+                .first::<i64>(conn)
+                .unwrap_or(0) != 0
         }}
-        .into_iter()
-        .collect()
     }
 
-    pub async fn get_members_of_full_access_groups(org_uuid: &str, conn: &mut DbConn) -> Vec<String> {
+    pub async fn has_full_access_by_member(org_uuid: &str, member_uuid: &str, conn: &mut DbConn) -> bool {
         db_run! { conn: {
             groups_users::table
                 .inner_join(groups::table.on(
                     groups::uuid.eq(groups_users::groups_uuid)
                 ))
                 .filter(groups::organizations_uuid.eq(org_uuid))
                 .filter(groups::access_all.eq(true))
-                .select(groups_users::users_organizations_uuid)
-                .distinct()
-                .load::<String>(conn)
-                .expect("Error loading all access group users for organization")
+                .filter(groups_users::users_organizations_uuid.eq(member_uuid))
+                .count()
+                .first::<i64>(conn)
+                .unwrap_or(0) != 0
         }}
-        .into_iter()
-        .collect()
     }
 
     pub async fn update_user_revision(&self, conn: &mut DbConn) {