From e858b96ff1c53b2d631129368907ebf87be9640a Mon Sep 17 00:00:00 2001
From: Stefan Melmuk <stefan.melmuk@gmail.com>
Date: Fri, 5 Jan 2024 04:42:36 +0100
Subject: [PATCH] improve check if the user has access via groups

instead of returning the two lists of member ids and later checking if
they contain the uuid of the current user, we really only care if
the current user has full access via a group or if they have
access to a given collection via a group
---
 src/api/core/organizations.rs | 20 +++++++-------------
 src/db/models/group.rs        | 28 ++++++++++++++--------------
 2 files changed, 21 insertions(+), 27 deletions(-)

diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs
index 6792fb86..775d3ae9 100644
--- a/src/api/core/organizations.rs
+++ b/src/api/core/organizations.rs
@@ -321,14 +321,10 @@ async fn get_org_collections_details(org_id: &str, headers: ManagerHeadersLoose,
     };
 
     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);
+    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 = 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() {
@@ -359,12 +355,10 @@ async fn get_org_collections_details(org_id: &str, headers: ManagerHeadersLoose,
             })
             .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;
+        }
 
         let mut json_object = col.to_json();
         json_object["Assigned"] = json!(assigned);
diff --git a/src/db/models/group.rs b/src/db/models/group.rs
index 01e8911c..e50853e2 100644
--- a/src/db/models/group.rs
+++ b/src/db/models/group.rs
@@ -486,23 +486,25 @@ 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(
@@ -510,13 +512,11 @@ impl GroupUser {
                 ))
                 .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) {