Browse Source

Align bulk collection-access authorization with single-collection edit

post_bulk_access_collections authorized each requested collection via the
legacy Collection::is_manageable_by_user helper, which also accepts a member's
membership/group access_all. For a Custom member this diverged from the
single-collection edit endpoint (ManagerHeaders -> collection_edit_access),
which requires a real per-collection Manage grant and never treats a Custom
member's access_all as one. A flagless Custom member placed in an access_all
group could therefore rewrite collection user/group assignments in bulk while
the single-collection edit endpoint denied the exact same change.

Add auth::can_edit_collection - the collection_edit_access + can_manage_collection
pair the ManagerHeaders guard already uses - and call it per collection in the
bulk-access endpoint. Legacy Managers, Admins/Owners and Edit-any-collection
members are unaffected; only a Custom member's access_all shortcut is removed,
so bulk-access now enforces exactly what the single-collection edit endpoint does.
pull/7397/head
tom27052006 1 day ago
parent
commit
fe3111d9b5
  1. 18
      src/api/core/organizations.rs
  2. 18
      src/auth.rs

18
src/api/core/organizations.rs

@ -629,13 +629,15 @@ async fn post_bulk_access_collections(
err!("Can't find organization details")
}
// Security: authorization is enforced per collection below via `is_manageable_by_user`, which
// requires a real manage grant on each requested collection (direct users_collections.manage,
// a manage group, membership/group access_all, or Admin/Owner). This matches the single
// collection edit endpoint and the pre-existing behavior of this endpoint. A custom user with
// only manage_users / manage_groups / manage_policies holds no such grant and is therefore
// rejected, while a Manager/Custom member who manages some collections keeps the ability to
// bulk-edit exactly those (a blanket full-access requirement would wrongly deny that here).
// Security (F-1): authorization is enforced per collection below via `auth::can_edit_collection`,
// the exact same Custom-aware check the single-collection edit endpoint (`ManagerHeaders`) uses.
// Edit any collection (or Admin/Owner) may bulk-edit every collection; a legacy Manager keeps its
// broad per-collection helper; any other Custom member must hold a real per-collection Manage
// grant. In particular a Custom member's membership/group `access_all` does NOT satisfy this here
// (it did under the previous `is_manageable_by_user` check, which diverged from the single-edit
// endpoint). A custom user with only manage_users / manage_groups / manage_policies holds no such
// grant and is rejected, while a member who manages some collections keeps the ability to
// bulk-edit exactly those.
// Security (audit H-3) and atomicity (audit M-2): validate the whole request against this
// organization *before* mutating anything. Every collection must exist in the org and be
@ -659,7 +661,7 @@ async fn post_bulk_access_collections(
err!("Collection not found")
};
if !collection.is_manageable_by_user(&headers.membership.user_uuid, &conn).await {
if !crate::auth::can_edit_collection(&headers.membership, &collection.uuid, &conn).await {
err!("Collection not found", "The current user isn't a manager for this collection")
}

18
src/auth.rs

@ -1016,6 +1016,24 @@ async fn can_manage_collection(
}
}
/// Whether `membership` may edit (rewrite the access of) `collection_uuid`, using exactly the same
/// Custom-aware rules as the path-based `ManagerHeaders` guard (`collection_edit_access`): Edit any
/// collection (or Admin/Owner) may edit every collection, otherwise only collections on which the
/// member holds a real per-collection Manage grant. In particular, a Custom member's membership or
/// group `access_all` does NOT satisfy this — it must be an explicit `users_collections.manage` /
/// `collections_groups.manage` assignment, exactly as an in-path collection edit would require.
///
/// Body-param endpoints (e.g. bulk collection access) take collection ids in the request body and
/// therefore cannot use `ManagerHeaders`; they must run this per collection to stay consistent with
/// the single-collection edit endpoint.
pub(crate) async fn can_edit_collection(
membership: &Membership,
collection_uuid: &CollectionId,
conn: &DbConn,
) -> bool {
can_manage_collection(collection_edit_access(membership), membership, collection_uuid, conn).await
}
/// ManagerHeaders authorizes collection updates. A Custom member with Edit any collection can
/// update every collection; otherwise the caller must be at least a legacy Manager and have the
/// per-collection Manage permission. Read and delete use separate guards so Edit cannot

Loading…
Cancel
Save