Fix dani-garcia#3624: fix manager permission within groups (dani-garc…

…ia#3754)

* Fix dani-garcia#3624: fix manager permission within groups

* Query returns UUID only

* Fix issue when user is manager and in a group having access to all collections

* optimize condition check

* fix(groups): renaming and optimizations

* fix: wrong organization group membership detection

* Simplify group membership check

Co-authored-by: Stefan Melmuk <[email protected]>

* Remove unused statement

* 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

* improve comments for get_org_collections_details

* small refactor to make it easier to review

* fix(groups): query full access via group only when necessary

Co-authored-by: Mathijs van Veluw <[email protected]>

* chore(fmt): apply rustfmt

---------

Co-authored-by: Stefan Melmuk <[email protected]>
Co-authored-by: Stefan Melmuk <[email protected]>
Co-authored-by: Mathijs van Veluw <[email protected]>

src/api/core/organizations.rs

@@ -320,41 +320,49 @@ 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;
 
+    // check if current user has full access to the organization (either directly or via any group)
+    let has_full_access_to_org = user_org.access_all
+        || (CONFIG.org_groups_enabled()
+            && GroupUser::has_full_access_by_member(org_id, &user_org.uuid, &mut conn).await);
+
     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 = false;
+        // 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 user_org.access_all {
-            assigned = true;
+        // 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);
         json_object["Users"] = json!(users);

src/db/models/group.rs

@@ -486,6 +486,39 @@ impl GroupUser {
         }}
     }
 
+    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))
+                .filter(groups_users::users_organizations_uuid.eq(member_uuid))
+                .count()
+                .first::<i64>(conn)
+                .unwrap_or(0) != 0
+        }}
+    }
+
+    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))
+                .filter(groups_users::users_organizations_uuid.eq(member_uuid))
+                .count()
+                .first::<i64>(conn)
+                .unwrap_or(0) != 0
+        }}
+    }
+
     pub async fn update_user_revision(&self, conn: &mut DbConn) {
         match UserOrganization::find_by_uuid(&self.users_organizations_uuid, conn).await {
             Some(user) => User::update_uuid_revision(&user.user_uuid, conn).await,