From 9dcc738f856d818b261507376290c544bf712d47 Mon Sep 17 00:00:00 2001 From: Stefan Melmuk <509385+stefan0xC@users.noreply.github.com> Date: Sat, 27 Apr 2024 22:09:00 +0200 Subject: [PATCH] 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 | 30 +++++++----------------------- src/auth.rs | 8 ++------ src/db/models/collection.rs | 26 ++++++++++++++------------ 3 files changed, 23 insertions(+), 41 deletions(-) diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index 38657ef6..c6556f77 100644 --- a/src/api/core/organizations.rs +++ b/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 = 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 = 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 = 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); diff --git a/src/auth.rs b/src/auth.rs index 3f92be76..f05eba65 100644 --- a/src/auth.rs +++ b/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 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!"); } } diff --git a/src/db/models/collection.rs b/src/db/models/collection.rs index 4d3ccd2e..ae70c76c 100644 --- a/src/db/models/collection.rs +++ b/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::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