improve access to collections via groups (#4441)

* refactor get_org_collections_details

* improve access to collection check

* fix get_org_collection_detail too

src/api/core/organizations.rs

@@ -329,27 +329,19 @@ async fn get_org_collections_details(org_id: &str, headers: ManagerHeadersLoose,
  && 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 {
- // assigned indicates whether the current user has access to the given collection
- let mut assigned = has_full_access_to_org;
+ // check whether the current user has access to the given collection
+ let assigned = has_full_access_to_org
+ || CollectionUser::has_access_to_collection_by_user(&col.uuid, &user_org.user_uuid, &mut conn).await
+ || (CONFIG.org_groups_enabled()
+ && GroupUser::has_access_to_collection_by_member(&col.uuid, &user_org.uuid, &mut conn).await);
 
  // 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| {
- // 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()
- })
+ .map(|collection_user| SelectionReadOnly::to_collection_user_details_read_only(collection_user).to_json())
  .collect();
 
- // 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)
@@ -672,24 +664,16 @@ async fn get_org_collection_detail(
  Vec::with_capacity(0)
  };
 
- let mut assigned = false;
  let users: Vec<Value> =
  CollectionUser::find_by_collection_swap_user_uuid_with_org_user_uuid(&collection.uuid, &mut conn)
  .await
  .iter()
  .map(|collection_user| {
- // Remember `user_uuid` is swapped here with the `user_org.uuid` with a join during the `find_by_collection_swap_user_uuid_with_org_user_uuid` call.
- // We check here if the current user is assigned to this collection or not.
- 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;
- }
+ let assigned = Collection::can_access_collection(&user_org, &collection.uuid, &mut conn).await;
 
  let mut json_object = collection.to_json();
  json_object["Assigned"] = json!(assigned);

src/auth.rs

@@ -689,7 +689,7 @@ impl<'r> FromRequest<'r> for ManagerHeaders {
  _ => err_handler!("Error getting DB"),
  };
 
- if !can_access_collection(&headers.org_user, &col_id, &mut conn).await {
+ if !Collection::can_access_collection(&headers.org_user, &col_id, &mut conn).await {
  err_handler!("The current user isn't a manager for this collection")
  }
  }
@@ -762,10 +762,6 @@ impl From<ManagerHeadersLoose> for Headers {
  }
  }
 }
-async fn can_access_collection(org_user: &UserOrganization, col_id: &str, conn: &mut DbConn) -> bool {
- org_user.has_full_access()
- || Collection::has_access_by_collection_and_user_uuid(col_id, &org_user.user_uuid, conn).await
-}
 
 impl ManagerHeaders {
  pub async fn from_loose(
@@ -777,7 +773,7 @@ impl ManagerHeaders {
  if uuid::Uuid::parse_str(col_id).is_err() {
  err!("Collection Id is malformed!");
  }
- if !can_access_collection(&h.org_user, col_id, conn).await {
+ if !Collection::can_access_collection(&h.org_user, col_id, conn).await {
  err!("You don't have access to all collections!");
  }
  }

src/db/models/collection.rs

@@ -1,6 +1,6 @@
 use serde_json::Value;
 
-use super::{CollectionGroup, User, UserOrgStatus, UserOrgType, UserOrganization};
+use super::{CollectionGroup, GroupUser, User, UserOrgStatus, UserOrgType, UserOrganization};
 use crate::CONFIG;
 
 db_object! {
@@ -102,6 +102,15 @@ impl Collection {
  json_object["HidePasswords"] = json!(hide_passwords);
  json_object
  }
+
+ pub async fn can_access_collection(org_user: &UserOrganization, col_id: &str, conn: &mut DbConn) -> bool {
+ org_user.has_status(UserOrgStatus::Confirmed)
+ && (org_user.has_full_access()
+ || CollectionUser::has_access_to_collection_by_user(col_id, &org_user.user_uuid, conn).await
+ || (CONFIG.org_groups_enabled()
+ && (GroupUser::has_full_access_by_member(&org_user.org_uuid, &org_user.uuid, conn).await
+ || GroupUser::has_access_to_collection_by_member(col_id, &org_user.uuid, conn).await)))
+ }
 }
 
 use crate::db::DbConn;
@@ -252,17 +261,6 @@ impl Collection {
  }
  }
 
- // Check if a user has access to a specific collection
- // FIXME: This needs to be reviewed. The query used by `find_by_user_uuid` could be adjusted to filter when needed.
- // For now this is a good solution without making to much changes.
- pub async fn has_access_by_collection_and_user_uuid(
- collection_uuid: &str,
- user_uuid: &str,
- conn: &mut DbConn,
- ) -> bool {
- Self::find_by_user_uuid(user_uuid.to_owned(), conn).await.into_iter().any(|c| c.uuid == collection_uuid)
- }
-
  pub async fn find_by_organization_and_user_uuid(org_uuid: &str, user_uuid: &str, conn: &mut DbConn) -> Vec<Self> {
  Self::find_by_user_uuid(user_uuid.to_owned(), conn)
  .await
@@ -644,6 +642,10 @@ impl CollectionUser {
  Ok(())
  }}
  }
+
+ pub async fn has_access_to_collection_by_user(col_id: &str, user_uuid: &str, conn: &mut DbConn) -> bool {
+ Self::find_by_collection_and_user(col_id, user_uuid, conn).await.is_some()
+ }
 }
 
 /// Database methods