From 44139eb0c79ea429fc5e6e021686bc442b7a7fbf Mon Sep 17 00:00:00 2001 From: tom27052006 <83423411+tom27052006@users.noreply.github.com> Date: Thu, 16 Jul 2026 18:18:49 +0200 Subject: [PATCH] Add granular custom collection permissions Persist create, edit-any, and delete-any collection grants independently for Custom members. Enforce dedicated create/delete guards, preserve edit-any cipher access, cover imports and metadata reads, and add fail-closed role transitions, migrations, and regression tests. --- .../down.sql | 9 + .../up.sql | 11 + .../down.sql | 9 + .../up.sql | 11 + .../down.sql | 9 + .../up.sql | 11 + src/api/admin.rs | 86 +++++- src/api/core/organizations.rs | 274 ++++++++++++------ src/auth.rs | 216 +++++++++++++- src/db/models/organization.rs | 201 +++++++++++-- src/db/schema.rs | 3 + .../templates/scss/vaultwarden.scss.hbs | 5 +- 12 files changed, 716 insertions(+), 129 deletions(-) create mode 100644 migrations/mysql/2026-07-16-120000_add_custom_collection_permissions/down.sql create mode 100644 migrations/mysql/2026-07-16-120000_add_custom_collection_permissions/up.sql create mode 100644 migrations/postgresql/2026-07-16-120000_add_custom_collection_permissions/down.sql create mode 100644 migrations/postgresql/2026-07-16-120000_add_custom_collection_permissions/up.sql create mode 100644 migrations/sqlite/2026-07-16-120000_add_custom_collection_permissions/down.sql create mode 100644 migrations/sqlite/2026-07-16-120000_add_custom_collection_permissions/up.sql diff --git a/migrations/mysql/2026-07-16-120000_add_custom_collection_permissions/down.sql b/migrations/mysql/2026-07-16-120000_add_custom_collection_permissions/down.sql new file mode 100644 index 00000000..6506059d --- /dev/null +++ b/migrations/mysql/2026-07-16-120000_add_custom_collection_permissions/down.sql @@ -0,0 +1,9 @@ +-- The previous schema exposes access_all as the three collection permissions together. Avoid +-- turning Edit-only memberships into Create/Edit/Delete grants when rolling back. +UPDATE users_organizations +SET access_all = create_new_collections AND edit_any_collection AND delete_any_collection +WHERE atype = 4; + +ALTER TABLE users_organizations DROP COLUMN create_new_collections; +ALTER TABLE users_organizations DROP COLUMN edit_any_collection; +ALTER TABLE users_organizations DROP COLUMN delete_any_collection; diff --git a/migrations/mysql/2026-07-16-120000_add_custom_collection_permissions/up.sql b/migrations/mysql/2026-07-16-120000_add_custom_collection_permissions/up.sql new file mode 100644 index 00000000..c13662c4 --- /dev/null +++ b/migrations/mysql/2026-07-16-120000_add_custom_collection_permissions/up.sql @@ -0,0 +1,11 @@ +ALTER TABLE users_organizations ADD COLUMN create_new_collections BOOLEAN NOT NULL DEFAULT FALSE; +ALTER TABLE users_organizations ADD COLUMN edit_any_collection BOOLEAN NOT NULL DEFAULT FALSE; +ALTER TABLE users_organizations ADD COLUMN delete_any_collection BOOLEAN NOT NULL DEFAULT FALSE; + +-- Before these permissions were persisted independently, access_all represented the legacy +-- "Manage all collections" checkbox. Preserve that capability for existing Custom members. +UPDATE users_organizations +SET create_new_collections = access_all, + edit_any_collection = access_all, + delete_any_collection = access_all +WHERE atype = 4; diff --git a/migrations/postgresql/2026-07-16-120000_add_custom_collection_permissions/down.sql b/migrations/postgresql/2026-07-16-120000_add_custom_collection_permissions/down.sql new file mode 100644 index 00000000..6506059d --- /dev/null +++ b/migrations/postgresql/2026-07-16-120000_add_custom_collection_permissions/down.sql @@ -0,0 +1,9 @@ +-- The previous schema exposes access_all as the three collection permissions together. Avoid +-- turning Edit-only memberships into Create/Edit/Delete grants when rolling back. +UPDATE users_organizations +SET access_all = create_new_collections AND edit_any_collection AND delete_any_collection +WHERE atype = 4; + +ALTER TABLE users_organizations DROP COLUMN create_new_collections; +ALTER TABLE users_organizations DROP COLUMN edit_any_collection; +ALTER TABLE users_organizations DROP COLUMN delete_any_collection; diff --git a/migrations/postgresql/2026-07-16-120000_add_custom_collection_permissions/up.sql b/migrations/postgresql/2026-07-16-120000_add_custom_collection_permissions/up.sql new file mode 100644 index 00000000..c13662c4 --- /dev/null +++ b/migrations/postgresql/2026-07-16-120000_add_custom_collection_permissions/up.sql @@ -0,0 +1,11 @@ +ALTER TABLE users_organizations ADD COLUMN create_new_collections BOOLEAN NOT NULL DEFAULT FALSE; +ALTER TABLE users_organizations ADD COLUMN edit_any_collection BOOLEAN NOT NULL DEFAULT FALSE; +ALTER TABLE users_organizations ADD COLUMN delete_any_collection BOOLEAN NOT NULL DEFAULT FALSE; + +-- Before these permissions were persisted independently, access_all represented the legacy +-- "Manage all collections" checkbox. Preserve that capability for existing Custom members. +UPDATE users_organizations +SET create_new_collections = access_all, + edit_any_collection = access_all, + delete_any_collection = access_all +WHERE atype = 4; diff --git a/migrations/sqlite/2026-07-16-120000_add_custom_collection_permissions/down.sql b/migrations/sqlite/2026-07-16-120000_add_custom_collection_permissions/down.sql new file mode 100644 index 00000000..6506059d --- /dev/null +++ b/migrations/sqlite/2026-07-16-120000_add_custom_collection_permissions/down.sql @@ -0,0 +1,9 @@ +-- The previous schema exposes access_all as the three collection permissions together. Avoid +-- turning Edit-only memberships into Create/Edit/Delete grants when rolling back. +UPDATE users_organizations +SET access_all = create_new_collections AND edit_any_collection AND delete_any_collection +WHERE atype = 4; + +ALTER TABLE users_organizations DROP COLUMN create_new_collections; +ALTER TABLE users_organizations DROP COLUMN edit_any_collection; +ALTER TABLE users_organizations DROP COLUMN delete_any_collection; diff --git a/migrations/sqlite/2026-07-16-120000_add_custom_collection_permissions/up.sql b/migrations/sqlite/2026-07-16-120000_add_custom_collection_permissions/up.sql new file mode 100644 index 00000000..c13662c4 --- /dev/null +++ b/migrations/sqlite/2026-07-16-120000_add_custom_collection_permissions/up.sql @@ -0,0 +1,11 @@ +ALTER TABLE users_organizations ADD COLUMN create_new_collections BOOLEAN NOT NULL DEFAULT FALSE; +ALTER TABLE users_organizations ADD COLUMN edit_any_collection BOOLEAN NOT NULL DEFAULT FALSE; +ALTER TABLE users_organizations ADD COLUMN delete_any_collection BOOLEAN NOT NULL DEFAULT FALSE; + +-- Before these permissions were persisted independently, access_all represented the legacy +-- "Manage all collections" checkbox. Preserve that capability for existing Custom members. +UPDATE users_organizations +SET create_new_collections = access_all, + edit_any_collection = access_all, + delete_any_collection = access_all +WHERE atype = 4; diff --git a/src/api/admin.rs b/src/api/admin.rs index c16fb866..0c85d562 100644 --- a/src/api/admin.rs +++ b/src/api/admin.rs @@ -544,6 +544,31 @@ struct MembershipTypeData { org_uuid: OrganizationId, } +fn apply_membership_type_change(membership: &mut Membership, new_type: MembershipType) { + let was_custom = membership.atype == MembershipType::Custom; + + // Entering Custom through the Vaultwarden admin panel is deliberately fail-closed because + // that UI cannot select granular permissions; they can be granted later through the regular + // organization member dialog. + if new_type == MembershipType::Custom && !was_custom { + membership.clear_custom_permissions(); + membership.access_all = false; + } + if new_type != MembershipType::Custom { + membership.clear_custom_permissions(); + } + + // Prevent stale access_all from surviving a demotion to User. Admins/Owners have implicit + // full access, while legacy Manager access_all is intentionally preserved for compatibility. + match new_type { + MembershipType::Owner | MembershipType::Admin => membership.access_all = true, + MembershipType::User => membership.access_all = false, + MembershipType::Manager | MembershipType::Custom => {} + } + + membership.atype = new_type as i32; +} + #[post("/users/org_type", format = "application/json", data = "")] async fn update_membership_type(data: Json, token: AdminToken, conn: DbConn) -> EmptyResult { let data: MembershipTypeData = data.into_inner(); @@ -553,9 +578,7 @@ async fn update_membership_type(data: Json, token: AdminToke err!("The specified user isn't member of the organization") }; - let new_type = if let Some(new_type) = MembershipType::from_str(&data.user_type.into_string()) { - new_type as i32 - } else { + let Some(new_type) = MembershipType::from_str(&data.user_type.into_string()) else { err!("Invalid type") }; @@ -566,14 +589,7 @@ async fn update_membership_type(data: Json, token: AdminToke } } - member_to_edit.atype = new_type; - // The manage_* permission flags only apply to the Custom role; clear them on any other - // type so a member changed away from Custom does not retain stale management permissions. - if new_type != MembershipType::Custom { - member_to_edit.manage_users = false; - member_to_edit.manage_groups = false; - member_to_edit.manage_policies = false; - } + apply_membership_type_change(&mut member_to_edit, new_type); // This check is also done at api::organizations::{accept_invite, _confirm_invite, _activate_member, edit_member}, update_membership_type OrgPolicy::check_user_allowed(&member_to_edit, "modify", &conn).await?; @@ -876,6 +892,14 @@ impl<'r> FromRequest<'r> for AdminToken { #[cfg(test)] mod tests { use super::*; + use crate::db::models::MembershipStatus; + + fn membership(member_type: MembershipType) -> Membership { + let mut membership = Membership::new("test-user".to_owned().into(), "test-org".to_owned().into(), None); + membership.atype = member_type as i32; + membership.status = MembershipStatus::Confirmed as i32; + membership + } #[test] fn validate_web_vault_compare() { @@ -900,4 +924,44 @@ mod tests { assert!(web_vault_compare("2025.12.2+build.1", "2025.12.1+build.1") == 1); assert!(web_vault_compare("2025.12.1+build.3", "2025.12.1+build.2") == 1); } + + #[test] + fn admin_type_changes_clear_custom_permissions_and_stale_access() { + let mut custom = membership(MembershipType::Custom); + custom.access_all = true; + custom.manage_users = true; + custom.create_new_collections = true; + custom.edit_any_collection = true; + custom.delete_any_collection = true; + + apply_membership_type_change(&mut custom, MembershipType::User); + assert_eq!(custom.atype, MembershipType::User as i32); + assert!(!custom.access_all); + assert!(!custom.manage_users); + assert!(!custom.create_new_collections); + assert!(!custom.edit_any_collection); + assert!(!custom.delete_any_collection); + + let mut admin = membership(MembershipType::Admin); + admin.access_all = true; + apply_membership_type_change(&mut admin, MembershipType::Custom); + assert_eq!(admin.atype, MembershipType::Custom as i32); + assert!(!admin.access_all, "entering Custom through this UI must be fail-closed"); + assert!(!admin.has_manage_all_collections()); + } + + #[test] + fn admin_and_legacy_manager_access_all_behavior_is_preserved() { + let mut user = membership(MembershipType::User); + apply_membership_type_change(&mut user, MembershipType::Admin); + assert!(user.access_all); + + let mut custom = membership(MembershipType::Custom); + custom.access_all = true; + custom.edit_any_collection = true; + apply_membership_type_change(&mut custom, MembershipType::Manager); + assert_eq!(custom.atype, MembershipType::Manager as i32); + assert!(custom.access_all); + assert!(!custom.edit_any_collection); + } } diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index b2a2a6a0..63d142e8 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -12,8 +12,9 @@ use crate::{ core::{CipherSyncData, CipherSyncType, accept_org_invite, log_event, two_factor}, }, auth::{ - AdminHeaders, Headers, ManageGroupsHeaders, ManagePoliciesHeaders, ManageUsersHeaders, ManagerHeaders, - ManagerHeadersLoose, OrgMemberHeaders, OwnerHeaders, decode_invite, + AdminHeaders, CollectionDeleteHeaders, CollectionReadHeaders, Headers, ManageGroupsHeaders, + ManagePoliciesHeaders, ManageUsersHeaders, ManagerHeaders, ManagerHeadersLoose, OrgMemberHeaders, OwnerHeaders, + decode_invite, }, db::{ DbConn, @@ -398,7 +399,8 @@ async fn get_org_collections(org_id: OrganizationId, headers: ManagerHeadersLoos // expose cipher contents. manage_policies does not need the collection list. let can_read_collection_list = headers.membership.has_full_access() || headers.membership.has_manage_users() - || headers.membership.has_manage_groups(); + || headers.membership.has_manage_groups() + || headers.membership.has_delete_any_collection(); if !can_read_collection_list { err_code!("Resource not found.", "User does not have full access", rocket::http::Status::NotFound.code); } @@ -435,7 +437,12 @@ async fn get_org_collections_details(org_id: OrganizationId, headers: ManagerHea // (metadata only) so the web client can render member/group collection assignments // without crashing on collections it can't otherwise see. This exposes names/ids // only, never cipher contents. manage_policies does not need the collection list. - let can_read_collection_list = member.has_manage_users() || member.has_manage_groups(); + let can_read_collection_list = + member.has_manage_users() || member.has_manage_groups() || member.has_delete_any_collection(); + // Delete any collection can reveal collection access metadata, matching Bitwarden's + // ReadAllWithAccess behavior, but still does not grant cipher access. Manage Users/Groups + // retain the narrower metadata-only view introduced by the base PR. + let can_read_all_collection_access = member.has_edit_any_collection() || member.has_delete_any_collection(); // Get all admins, owners and managers who can manage/access all // Those are currently not listed in the col_users but need to be listed too. @@ -464,7 +471,7 @@ async fn get_org_collections_details(org_id: OrganizationId, headers: ManagerHea // Exception: custom users with a manage permission get a metadata-only entry (no user // or group access details) so the web client can resolve assignment references without // crashing. This never exposes cipher contents. - if !assigned { + if !assigned && !can_read_all_collection_access { if can_read_collection_list { let mut json_object = col.to_json_details(&headers.user.uuid, None, &conn).await; json_object["assigned"] = json!(false); @@ -530,16 +537,16 @@ async fn post_organization_collections( if org_id != headers.membership.org_uuid { err!("Organization not found", "Organization id's do not match"); } - let data: FullCollectionData = data.into_inner(); - data.validate(&org_id, &conn).await?; - // Managers and custom users may only create collections if they have full access. - // (A custom user with manage_users/manage_groups/manage_policies but no collection - // access must not be able to create collections.) - if !headers.membership.has_full_access() && headers.membership.atype < MembershipType::Admin { + // Create is independent from Edit/Delete. In particular, Edit any collection's internal + // access_all representation must not implicitly grant this endpoint. + if !headers.membership.can_create_new_collections() { err!("You don't have permission to create collections") } + let data: FullCollectionData = data.into_inner(); + data.validate(&org_id, &conn).await?; + let collection = Collection::new(org_id.clone(), data.name, data.external_id); collection.save(&conn).await?; @@ -743,7 +750,7 @@ async fn post_organization_collection_update( async fn delete_organization_collection_impl( org_id: &OrganizationId, col_id: &CollectionId, - headers: &ManagerHeaders, + headers: &CollectionDeleteHeaders, conn: &DbConn, ) -> EmptyResult { if org_id != &headers.org_id { @@ -769,7 +776,7 @@ async fn delete_organization_collection_impl( async fn delete_organization_collection( org_id: OrganizationId, col_id: CollectionId, - headers: ManagerHeaders, + headers: CollectionDeleteHeaders, conn: DbConn, ) -> EmptyResult { delete_organization_collection_impl(&org_id, &col_id, &headers, &conn).await @@ -779,7 +786,7 @@ async fn delete_organization_collection( async fn post_organization_collection_delete( org_id: OrganizationId, col_id: CollectionId, - headers: ManagerHeaders, + headers: CollectionDeleteHeaders, conn: DbConn, ) -> EmptyResult { delete_organization_collection_impl(&org_id, &col_id, &headers, &conn).await @@ -805,7 +812,7 @@ async fn bulk_delete_organization_collections( let collections = data.ids; - let headers = ManagerHeaders::from_loose(headers, &collections, &conn).await?; + let headers = CollectionDeleteHeaders::from_loose(headers, &collections, &conn).await?; for col_id in collections { delete_organization_collection_impl(&org_id, &col_id, &headers, &conn).await?; @@ -817,23 +824,19 @@ async fn bulk_delete_organization_collections( async fn get_org_collection_detail( org_id: OrganizationId, col_id: CollectionId, - headers: ManagerHeaders, + headers: CollectionReadHeaders, conn: DbConn, ) -> JsonResult { if org_id != headers.org_id { err!("Organization not found", "Organization id's do not match"); } - match Collection::find_by_uuid_and_user(&col_id, headers.user.uuid.clone(), &conn).await { + match Collection::find_by_uuid_and_org(&col_id, &org_id, &conn).await { None => err!("Collection not found"), Some(collection) => { if collection.org_uuid != org_id { err!("Collection is not owned by organization") } - let Some(member) = Membership::find_by_user_and_org(&headers.user.uuid, &org_id, &conn).await else { - err!("User is not part of organization") - }; - let groups: Vec = if CONFIG.org_groups_enabled() { CollectionGroup::find_by_collection(&collection.uuid, &conn) .await @@ -867,7 +870,7 @@ async fn get_org_collection_detail( }) .collect(); - let assigned = Collection::can_access_collection(&member, &collection.uuid, &conn).await; + let assigned = Collection::can_access_collection(&headers.membership, &collection.uuid, &conn).await; let mut json_object = collection.to_json_details(&headers.user.uuid, None, &conn).await; json_object["assigned"] = json!(assigned); @@ -884,7 +887,7 @@ async fn get_org_collection_detail( async fn get_collection_users( org_id: OrganizationId, col_id: CollectionId, - headers: ManagerHeaders, + headers: CollectionReadHeaders, conn: DbConn, ) -> JsonResult { if org_id != headers.org_id { @@ -1035,6 +1038,61 @@ async fn post_org_keys( }))) } +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] +// This is intentionally a permission bitmap: every field represents an independent API grant. +#[allow(clippy::struct_excessive_bools)] +struct CustomRolePermissions { + manage_users: bool, + manage_groups: bool, + manage_policies: bool, + create_new_collections: bool, + edit_any_collection: bool, + delete_any_collection: bool, +} + +impl CustomRolePermissions { + fn from_request(member_type: MembershipType, permissions: &HashMap) -> Self { + if member_type != MembershipType::Custom { + return Self::default(); + } + + let enabled = |key: &str| matches!(permissions.get(key), Some(Value::Bool(true))); + Self { + manage_users: enabled("manageUsers"), + manage_groups: enabled("manageGroups"), + manage_policies: enabled("managePolicies"), + create_new_collections: enabled("createNewCollections"), + edit_any_collection: enabled("editAnyCollection"), + delete_any_collection: enabled("deleteAnyCollection"), + } + } + + /// Bitwarden grants a Custom member with Edit any collection full read/edit/manage access to + /// organization ciphers. Vaultwarden's existing access_all flag is the internal data-plane + /// representation of that capability. Create and Delete remain completely independent. + fn access_all_for(self, member_type: MembershipType) -> bool { + member_type >= MembershipType::Admin || (member_type == MembershipType::Custom && self.edit_any_collection) + } + + fn differs_from(self, membership: &Membership) -> bool { + self.manage_users != membership.manage_users + || self.manage_groups != membership.manage_groups + || self.manage_policies != membership.manage_policies + || self.create_new_collections != membership.create_new_collections + || self.edit_any_collection != membership.edit_any_collection + || self.delete_any_collection != membership.delete_any_collection + } + + fn apply_to(self, membership: &mut Membership) { + membership.manage_users = self.manage_users; + membership.manage_groups = self.manage_groups; + membership.manage_policies = self.manage_policies; + membership.create_new_collections = self.create_new_collections; + membership.edit_any_collection = self.edit_any_collection; + membership.delete_any_collection = self.delete_any_collection; + } +} + #[derive(Deserialize)] #[serde(rename_all = "camelCase")] struct InviteData { @@ -1086,22 +1144,10 @@ async fn send_invite( err!("Only Owners can invite Managers, Admins or Owners") } - // For a Custom role, the "Manage all collections" parent checkbox is not sent to - // the server; we derive access_all from its three child checkboxes. Admins/Owners - // implicitly have access to all collections. - let access_all = new_type >= MembershipType::Admin - || (new_type == MembershipType::Custom - && data.permissions.get("editAnyCollection") == Some(&json!(true)) - && data.permissions.get("deleteAnyCollection") == Some(&json!(true)) - && data.permissions.get("createNewCollections") == Some(&json!(true))); - - // Read the explicit Custom-role management permissions. These only apply to the - // Custom type; for every other type they are forced to false. Only Owners can invite - // Custom members (checked above), so the caller is always authorized to grant these. - let perm = |key: &str| new_type == MembershipType::Custom && data.permissions.get(key) == Some(&json!(true)); - let manage_users = perm("manageUsers"); - let manage_groups = perm("manageGroups"); - let manage_policies = perm("managePolicies"); + // manageAllCollections is a client-only aggregate. Persist its three children independently; + // only Edit any collection maps to the existing all-cipher access representation. + let custom_permissions = CustomRolePermissions::from_request(new_type, &data.permissions); + let access_all = custom_permissions.access_all_for(new_type); let mut user_created: bool = false; for email in &data.emails { @@ -1145,9 +1191,7 @@ async fn send_invite( let mut new_member = Membership::new(user.uuid.clone(), org_id.clone(), Some(headers.user.email.clone())); new_member.access_all = access_all; new_member.atype = new_type as i32; - new_member.manage_users = manage_users; - new_member.manage_groups = manage_groups; - new_member.manage_policies = manage_policies; + custom_permissions.apply_to(&mut new_member); new_member.status = member_status; new_member.save(&conn).await?; @@ -1613,22 +1657,8 @@ async fn edit_member( err!("Invalid type") }; - // For a Custom role, the "Manage all collections" parent checkbox is not sent to - // the server; we derive access_all from its three child checkboxes. Admins/Owners - // implicitly have access to all collections. - let access_all = new_type >= MembershipType::Admin - || (new_type == MembershipType::Custom - && data.permissions.get("editAnyCollection") == Some(&json!(true)) - && data.permissions.get("deleteAnyCollection") == Some(&json!(true)) - && data.permissions.get("createNewCollections") == Some(&json!(true))); - - // Read the explicit Custom-role management permissions. These only apply to the - // Custom type; for every other type they are forced to false so that changing a - // member away from Custom clears any previously granted flags. - let perm = |key: &str| new_type == MembershipType::Custom && data.permissions.get(key) == Some(&json!(true)); - let manage_users = perm("manageUsers"); - let manage_groups = perm("manageGroups"); - let manage_policies = perm("managePolicies"); + let custom_permissions = CustomRolePermissions::from_request(new_type, &data.permissions); + let access_all = custom_permissions.access_all_for(new_type); let Some(mut member_to_edit) = Membership::find_by_uuid_and_org(&member_id, &org_id, &conn).await else { err!("The specified user isn't member of the organization") @@ -1645,10 +1675,10 @@ async fn edit_member( // with manage_users must not change roles: raising a member to Manager/Custom grants // collection-"manage" on every collection they can already write (see the `atype >= Manager` // branch in `Collection`/`Membership` json), and lowering it revokes that access — both are - // collection-access changes this caller is not entitled to make, even though the manage_* - // flags and access_all are already gated below. Requests that leave the role unchanged are - // allowed, so such members can still use the regular edit dialog. The Admin/Owner guard above - // still governs Admin/Owner transitions for Owners. + // collection-access changes this caller is not entitled to make, even though the custom + // permission flags and access_all are already gated below. Requests that leave the role + // unchanged are allowed, so such members can still use the regular edit dialog. The + // Admin/Owner guard above still governs Admin/Owner transitions for Owners. if !may_change_member_type(headers.membership_type, member_to_edit.atype, new_type) { err!("Only Admins or Owners can change a member's role") } @@ -1667,17 +1697,13 @@ async fn edit_member( } } - // Security: only Admins and Owners may change the granular custom-role management - // permissions. A Custom member with manage_users must not be able to grant them (to - // themselves or others — a privilege escalation) nor strip flags an Admin/Owner has - // granted to fellow Custom members. Requests that leave the flags unchanged are - // allowed, so such members can still use the regular edit dialog. - if headers.membership_type < MembershipType::Admin - && (manage_users != member_to_edit.manage_users - || manage_groups != member_to_edit.manage_groups - || manage_policies != member_to_edit.manage_policies) - { - err!("Only Admins or Owners can change custom management permissions") + // Security: only Admins and Owners may change the granular custom-role permissions. A Custom + // member with manage_users must not be able to grant them to themselves or others (a + // privilege escalation), nor strip flags an Admin/Owner has granted to fellow Custom members. + // Requests that leave the flags unchanged are allowed, so such members can still use the + // regular edit dialog. + if headers.membership_type < MembershipType::Admin && custom_permissions.differs_from(&member_to_edit) { + err!("Only Admins or Owners can change custom permissions") } // Security: only callers who can actually manage collections (Admins/Owners, or users @@ -1692,15 +1718,13 @@ async fn edit_member( // Security: `access_all` grants full access to every collection, so only callers who may // manage collections are allowed to change it. Otherwise a custom user with only manage_users - // could set the Custom "manage all collections" child boxes on any member (including - // themselves) to grant full collection access — a privilege escalation. For everyone else we - // keep the member's existing access_all grant untouched (neither granted nor revoked). + // could enable Edit any collection on any member (including themselves) to grant full + // collection access — a privilege escalation. For everyone else we keep the member's existing + // access_all grant untouched (neither granted nor revoked). if caller_can_manage_collections { member_to_edit.access_all = access_all; } - member_to_edit.manage_users = manage_users; - member_to_edit.manage_groups = manage_groups; - member_to_edit.manage_policies = manage_policies; + custom_permissions.apply_to(&mut member_to_edit); member_to_edit.atype = new_type as i32; // This check is also done at accept_invite, _confirm_invite, _activate_member, edit_member, admin::update_membership_type @@ -1992,13 +2016,21 @@ async fn post_org_import( } col_id } else { - // We do not allow users or managers which can not manage all collections to create new collections - // If there is any collection other than an existing import collection, abort the import. - if headers.membership.atype <= MembershipType::Manager && !headers.membership.has_full_access() { + // Collection creation through an organization import is governed by the same + // independent permission as the regular create endpoint. In particular, + // Edit any collection's access_all mirror must not satisfy this check. + if !headers.membership.can_create_new_collections() { err!(Compact, "The current user isn't allowed to create new collections") } let new_collection = Collection::new(org_id.clone(), col.name, col.external_id); new_collection.save(&conn).await?; + // Import-created collections do not carry the regular create endpoint's user access + // selections. Give a create-only importer Manage access to the collection they just + // created, matching Bitwarden's organization-import behavior. + if !headers.membership.has_full_access() { + CollectionUser::save(&headers.membership.user_uuid, &new_collection.uuid, false, false, true, &conn) + .await?; + } new_collection.uuid }; @@ -3568,8 +3600,12 @@ async fn rotate_api_key( #[cfg(test)] mod tests { - use super::{may_change_group_membership, may_change_member_type}; - use crate::db::models::MembershipType; + use std::collections::HashMap; + + use serde_json::{Value, json}; + + use super::{CustomRolePermissions, may_change_group_membership, may_change_member_type}; + use crate::db::models::{Membership, MembershipStatus, MembershipType}; #[test] fn manage_users_caller_cannot_change_member_role() { @@ -3614,4 +3650,74 @@ mod tests { // AND removing such memberships must be denied. assert!(!may_change_group_membership(false, true)); } + + #[test] + fn collection_permission_request_combinations_remain_independent() { + for mask in 0_u8..8 { + let create = mask & 0b001 != 0; + let edit = mask & 0b010 != 0; + let delete = mask & 0b100 != 0; + let permissions = HashMap::from([ + ("createNewCollections".to_owned(), json!(create)), + ("editAnyCollection".to_owned(), json!(edit)), + ("deleteAnyCollection".to_owned(), json!(delete)), + ]); + + let parsed = CustomRolePermissions::from_request(MembershipType::Custom, &permissions); + assert_eq!(parsed.create_new_collections, create, "mask={mask:03b}"); + assert_eq!(parsed.edit_any_collection, edit, "mask={mask:03b}"); + assert_eq!(parsed.delete_any_collection, delete, "mask={mask:03b}"); + // Only Edit any collection maps to all-cipher access. Create/Delete must never do so. + assert_eq!(parsed.access_all_for(MembershipType::Custom), edit, "mask={mask:03b}"); + } + } + + #[test] + fn custom_permission_parser_is_strict_and_non_custom_roles_are_fail_closed() { + let permissions = HashMap::from([ + ("manageUsers".to_owned(), Value::String("true".to_owned())), + ("manageGroups".to_owned(), json!(true)), + ("managePolicies".to_owned(), json!(true)), + ("createNewCollections".to_owned(), json!(true)), + ("editAnyCollection".to_owned(), json!(true)), + ("deleteAnyCollection".to_owned(), json!(true)), + ]); + + let custom = CustomRolePermissions::from_request(MembershipType::Custom, &permissions); + assert!(!custom.manage_users, "string values must not be accepted as booleans"); + assert!(custom.manage_groups); + assert!(custom.manage_policies); + assert!(custom.create_new_collections); + assert!(custom.edit_any_collection); + assert!(custom.delete_any_collection); + + let user = CustomRolePermissions::from_request(MembershipType::User, &permissions); + assert_eq!(user, CustomRolePermissions::default()); + assert!(!user.access_all_for(MembershipType::User)); + + let admin = CustomRolePermissions::from_request(MembershipType::Admin, &permissions); + assert_eq!(admin, CustomRolePermissions::default()); + assert!(admin.access_all_for(MembershipType::Admin)); + } + + #[test] + fn custom_permission_change_detection_covers_collection_flags() { + let mut membership = Membership::new("test-user".to_owned().into(), "test-org".to_owned().into(), None); + membership.atype = MembershipType::Custom as i32; + membership.status = MembershipStatus::Confirmed as i32; + + let requested = CustomRolePermissions { + create_new_collections: true, + edit_any_collection: true, + delete_any_collection: true, + ..CustomRolePermissions::default() + }; + + assert!(requested.differs_from(&membership)); + requested.apply_to(&mut membership); + assert!(!requested.differs_from(&membership)); + assert!(membership.create_new_collections); + assert!(membership.edit_any_collection); + assert!(membership.delete_any_collection); + } } diff --git a/src/auth.rs b/src/auth.rs index b7e02f89..bdaf2b6e 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -735,7 +735,7 @@ impl OrgHeaders { } // Custom-role permission checks. Admins and Owners implicitly hold every // permission; a Custom member holds a permission only if the matching flag - // is set on their Membership. The has_manage_* helpers gate the flags on the + // is set on their Membership. The has_* helpers gate the flags on the // Custom type, so stale flags on other types can never grant anything. fn can_manage_users(&self) -> bool { self.is_confirmed() && (self.membership_type >= MembershipType::Admin || self.membership.has_manage_users()) @@ -744,8 +744,7 @@ impl OrgHeaders { self.is_confirmed() && (self.membership_type >= MembershipType::Admin || self.membership.has_manage_groups()) } fn can_manage_policies(&self) -> bool { - self.is_confirmed() - && (self.membership_type >= MembershipType::Admin || self.membership.has_manage_policies()) + self.is_confirmed() && (self.membership_type >= MembershipType::Admin || self.membership.has_manage_policies()) } } @@ -944,9 +943,28 @@ fn get_col_id(request: &Request<'_>) -> Option { None } -/// The ManagerHeaders are used to check if you are at least a Manager -/// and have access to the specific collection provided via the /collections/collectionId. -/// This does strict checking on the collection_id, ManagerHeadersLoose does not. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum CollectionDeleteAccess { + Any, + ManagedOnly, + Denied, +} + +fn collection_delete_access(membership: &Membership) -> CollectionDeleteAccess { + if membership.can_delete_any_collection() { + CollectionDeleteAccess::Any + } else if membership.has_status(MembershipStatus::Confirmed) && membership.has_type(MembershipType::Manager) { + // Preserve the legacy Manager role's pre-existing per-collection deletion behavior. + CollectionDeleteAccess::ManagedOnly + } else { + CollectionDeleteAccess::Denied + } +} + +/// 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 +/// accidentally imply Delete. pub struct ManagerHeaders { pub host: String, pub device: Device, @@ -967,7 +985,9 @@ impl<'r> FromRequest<'r> for ManagerHeaders { err_handler!("Error getting DB") }; - if !Collection::is_coll_manageable_by_user(&col_id, &headers.membership.user_uuid, &conn).await { + if !headers.membership.has_edit_any_collection() + && !Collection::is_coll_manageable_by_user(&col_id, &headers.membership.user_uuid, &conn).await + { err_handler!("The current user isn't a manager for this collection") } } else { @@ -987,6 +1007,131 @@ impl<'r> FromRequest<'r> for ManagerHeaders { } } +/// Read access to collection metadata and assignment details. Delete any collection needs this +/// visibility to render the standard collection view, but it does not grant edit or cipher access. +pub struct CollectionReadHeaders { + pub host: String, + pub device: Device, + pub user: User, + pub membership: Membership, + pub ip: ClientIp, + pub org_id: OrganizationId, +} + +#[rocket::async_trait] +impl<'r> FromRequest<'r> for CollectionReadHeaders { + type Error = &'static str; + + async fn from_request(request: &'r Request<'_>) -> Outcome { + let headers = try_outcome!(OrgHeaders::from_request(request).await); + if !headers.is_confirmed_and_manager() { + err_handler!("You need collection read permission to call this endpoint") + } + + let Some(col_id) = get_col_id(request) else { + err_handler!("Error getting the collection id") + }; + + let can_read_any_collection = headers.is_confirmed_and_admin() + || headers.membership.has_edit_any_collection() + || headers.membership.has_delete_any_collection(); + + if !can_read_any_collection { + let Outcome::Success(conn) = DbConn::from_request(request).await else { + err_handler!("Error getting DB") + }; + + if !Collection::is_coll_manageable_by_user(&col_id, &headers.membership.user_uuid, &conn).await { + err_handler!("The current user isn't a manager for this collection") + } + } + + Outcome::Success(Self { + host: headers.host, + device: headers.device, + user: headers.user, + ip: headers.ip, + org_id: headers.membership.org_uuid.clone(), + membership: headers.membership, + }) + } +} + +impl From for Headers { + fn from(h: CollectionReadHeaders) -> Headers { + Headers { + host: h.host, + device: h.device, + user: h.user, + ip: h.ip, + } + } +} + +/// Delete is intentionally independent from Edit any collection. Vaultwarden advertises +/// limitCollectionDeletion=true, so Custom members require the explicit Delete any collection +/// permission. The legacy Manager role retains its previous per-collection behavior. +pub struct CollectionDeleteHeaders { + pub host: String, + pub device: Device, + pub user: User, + pub ip: ClientIp, + pub org_id: OrganizationId, +} + +#[rocket::async_trait] +impl<'r> FromRequest<'r> for CollectionDeleteHeaders { + type Error = &'static str; + + async fn from_request(request: &'r Request<'_>) -> Outcome { + let headers = try_outcome!(OrgHeaders::from_request(request).await); + if !headers.is_confirmed_and_manager() { + err_handler!("You need collection delete permission to call this endpoint") + } + + let Some(col_id) = get_col_id(request) else { + err_handler!("Error getting the collection id") + }; + + match collection_delete_access(&headers.membership) { + CollectionDeleteAccess::Any => {} + CollectionDeleteAccess::Denied => { + // Custom is a distinct, fail-closed role. In particular, Edit any collection and + // access_all must not satisfy a Delete request without the explicit delete flag. + err_handler!("You need the 'Delete any collection' permission to call this endpoint") + } + CollectionDeleteAccess::ManagedOnly => { + let Outcome::Success(conn) = DbConn::from_request(request).await else { + err_handler!("Error getting DB") + }; + + if !Collection::is_coll_manageable_by_user(&col_id, &headers.membership.user_uuid, &conn).await { + err_handler!("The current user isn't a manager for this collection") + } + } + } + + Outcome::Success(Self { + host: headers.host, + device: headers.device, + user: headers.user, + ip: headers.ip, + org_id: headers.membership.org_uuid, + }) + } +} + +impl From for Headers { + fn from(h: CollectionDeleteHeaders) -> Headers { + Headers { + host: h.host, + device: h.device, + user: h.user, + ip: h.ip, + } + } +} + impl From for Headers { fn from(h: ManagerHeaders) -> Headers { Headers { @@ -1039,22 +1184,32 @@ impl From for Headers { } } -impl ManagerHeaders { +impl CollectionDeleteHeaders { pub async fn from_loose( h: ManagerHeadersLoose, collections: &Vec, conn: &DbConn, - ) -> Result { + ) -> Result { + let delete_access = collection_delete_access(&h.membership); + if delete_access == CollectionDeleteAccess::Denied { + err!("You need the 'Delete any collection' permission to call this endpoint") + } + for col_id in collections { if uuid::Uuid::parse_str(col_id.as_ref()).is_err() { err!("Collection Id is malformed!"); } - if !Collection::is_coll_manageable_by_user(col_id, &h.membership.user_uuid, conn).await { + if Collection::find_by_uuid_and_org(col_id, &h.membership.org_uuid, conn).await.is_none() { + err!("Collection not found", "Collection does not exist or does not belong to this organization") + } + if delete_access == CollectionDeleteAccess::ManagedOnly + && !Collection::is_coll_manageable_by_user(col_id, &h.membership.user_uuid, conn).await + { err!("Collection not found", "The current user isn't a manager for this collection") } } - Ok(ManagerHeaders { + Ok(CollectionDeleteHeaders { host: h.host, device: h.device, user: h.user, @@ -1396,3 +1551,42 @@ pub async fn refresh_tokens( Ok((device, auth_tokens)) } + +#[cfg(test)] +mod tests { + use super::{CollectionDeleteAccess, collection_delete_access}; + use crate::db::models::{Membership, MembershipStatus, MembershipType}; + + fn membership(member_type: MembershipType) -> Membership { + let mut membership = Membership::new("test-user".to_owned().into(), "test-org".to_owned().into(), None); + membership.atype = member_type as i32; + membership.status = MembershipStatus::Confirmed as i32; + membership + } + + #[test] + fn collection_delete_permission_is_independent_from_edit_and_access_all() { + let mut custom = membership(MembershipType::Custom); + custom.edit_any_collection = true; + custom.access_all = true; + assert_eq!(collection_delete_access(&custom), CollectionDeleteAccess::Denied); + + custom.delete_any_collection = true; + assert_eq!(collection_delete_access(&custom), CollectionDeleteAccess::Any); + + custom.status = MembershipStatus::Accepted as i32; + assert_eq!(collection_delete_access(&custom), CollectionDeleteAccess::Denied); + } + + #[test] + fn collection_delete_permission_preserves_admin_and_legacy_manager_behavior() { + let admin = membership(MembershipType::Admin); + assert_eq!(collection_delete_access(&admin), CollectionDeleteAccess::Any); + + let manager = membership(MembershipType::Manager); + assert_eq!(collection_delete_access(&manager), CollectionDeleteAccess::ManagedOnly); + + let user = membership(MembershipType::User); + assert_eq!(collection_delete_access(&user), CollectionDeleteAccess::Denied); + } +} diff --git a/src/db/models/organization.rs b/src/db/models/organization.rs index 78228588..3fbfc789 100644 --- a/src/db/models/organization.rs +++ b/src/db/models/organization.rs @@ -61,6 +61,9 @@ pub struct Membership { pub manage_users: bool, pub manage_groups: bool, pub manage_policies: bool, + pub create_new_collections: bool, + pub edit_any_collection: bool, + pub delete_any_collection: bool, } #[derive(Identifiable, Queryable, Insertable, AsChangeset)] @@ -123,7 +126,7 @@ impl Ord for MembershipType { // For easy comparison, map each variant to an access level (where 0 is lowest). // Custom is treated as a low-privilege base role (same level as Manager for // ordering purposes); its elevated capabilities are governed by the explicit - // manage_* permission flags on the Membership, not by this ordering. + // custom permission flags on the Membership, not by this ordering. // // NOTE: Manager and Custom therefore share an access level while being distinct // variants: the derived `PartialEq` compares the role itself (Manager != Custom), @@ -279,6 +282,9 @@ impl Membership { manage_users: false, manage_groups: false, manage_policies: false, + create_new_collections: false, + edit_any_collection: false, + delete_any_collection: false, } } @@ -449,22 +455,26 @@ impl Membership { let membership_type = self.atype; let permissions = json!({ - // The 3 Collection roles below are linked to the access_all permission "accessEventLogs": false, "accessImportExport": false, "accessReports": false, - // If the following 3 Collection roles are set to true a custom user has access all permission - "createNewCollections": membership_type == 4 && self.access_all, - "editAnyCollection": membership_type == 4 && self.access_all, - "deleteAnyCollection": membership_type == 4 && self.access_all, - "manageGroups": self.manage_groups, - "managePolicies": self.manage_policies, + "createNewCollections": membership_type == MembershipType::Custom as i32 && self.create_new_collections, + "editAnyCollection": membership_type == MembershipType::Custom as i32 && self.edit_any_collection, + "deleteAnyCollection": membership_type == MembershipType::Custom as i32 && self.delete_any_collection, + "manageGroups": membership_type == MembershipType::Custom as i32 && self.manage_groups, + "managePolicies": membership_type == MembershipType::Custom as i32 && self.manage_policies, "manageSso": false, // Not supported - "manageUsers": self.manage_users, + "manageUsers": membership_type == MembershipType::Custom as i32 && self.manage_users, "manageResetPassword": false, "manageScim": false // Not supported (Not AGPLv3 Licensed) }); + // edit_any_collection is internally mirrored to access_all to provide Bitwarden-compatible + // cipher access, but it must not accidentally grant collection creation. The client treats + // limitCollectionCreation=false as an independent create grant, so compute it from the + // actual role/permission rather than access_all for Custom members. + let limit_collection_creation = self.limit_collection_creation(); + // https://github.com/bitwarden/server/blob/9ebe16587175b1c0e9208f84397bb75d0d595510/src/Api/AdminConsole/Models/Response/ProfileOrganizationResponseModel.cs json!({ "id": self.org_uuid, @@ -508,8 +518,7 @@ impl Membership { "familySponsorshipValidUntil": null, "familySponsorshipToDelete": null, "accessSecretsManager": false, - // limit collection creation to managers with access_all permission to prevent issues - "limitCollectionCreation": self.atype < MembershipType::Manager || !self.access_all, + "limitCollectionCreation": limit_collection_creation, "limitCollectionDeletion": true, "limitItemDeletion": false, "allowAdminAccessToAllCollectionItems": true, @@ -608,18 +617,16 @@ impl Membership { let membership_type = self.atype; - // Only return a permissions object for custom-type members. A custom member - // may have access_all (the 3 collection roles) and/or any of the explicit - // manage_* flags; otherwise Bitwarden assumes all-false defaults. + // Only return a permissions object for custom-type members. Otherwise Bitwarden assumes + // all-false defaults and the role itself supplies any elevated capabilities. let permissions = if membership_type == MembershipType::Custom as i32 { json!({ "accessEventLogs": false, "accessImportExport": false, "accessReports": false, - // If the following 3 Collection roles are set to true a custom user has access all permission - "createNewCollections": self.access_all, - "editAnyCollection": self.access_all, - "deleteAnyCollection": self.access_all, + "createNewCollections": self.create_new_collections, + "editAnyCollection": self.edit_any_collection, + "deleteAnyCollection": self.delete_any_collection, "manageGroups": self.manage_groups, "managePolicies": self.manage_policies, "manageSso": false, // Not supported @@ -811,10 +818,11 @@ impl Membership { } pub fn has_full_access(&self) -> bool { - (self.access_all || self.atype >= MembershipType::Admin) && self.has_status(MembershipStatus::Confirmed) + (self.access_all || self.has_edit_any_collection() || self.atype >= MembershipType::Admin) + && self.has_status(MembershipStatus::Confirmed) } - // The granular manage_* permission flags are only meaningful while the membership is of + // The granular custom permission flags are only meaningful while the membership is of // the Custom type. Gating them on the type here ensures that a stale flag left over from // a type change (e.g. via the admin panel) can never grant anything. pub fn has_manage_users(&self) -> bool { @@ -829,6 +837,63 @@ impl Membership { self.has_type(MembershipType::Custom) && self.manage_policies } + pub fn has_create_new_collections(&self) -> bool { + self.has_type(MembershipType::Custom) && self.create_new_collections + } + + pub fn has_edit_any_collection(&self) -> bool { + self.has_type(MembershipType::Custom) && self.edit_any_collection + } + + pub fn has_delete_any_collection(&self) -> bool { + self.has_type(MembershipType::Custom) && self.delete_any_collection + } + + /// `manageAllCollections` is a client-side aggregate checkbox, not a separately persisted + /// Bitwarden permission. It is selected exactly when all three child permissions are selected. + pub fn has_manage_all_collections(&self) -> bool { + self.has_create_new_collections() && self.has_edit_any_collection() && self.has_delete_any_collection() + } + + /// Match Vaultwarden's existing collection-creation policy while keeping the new Custom + /// permission independent from edit/delete. Legacy Manager memberships retain their former + /// access_all-based behavior. + pub fn can_create_new_collections(&self) -> bool { + if !self.has_status(MembershipStatus::Confirmed) { + return false; + } + + match MembershipType::from_i32(self.atype) { + Some(MembershipType::Owner | MembershipType::Admin) => true, + Some(MembershipType::Manager) => self.access_all, + Some(MembershipType::Custom) => self.create_new_collections, + Some(MembershipType::User) | None => false, + } + } + + pub fn limit_collection_creation(&self) -> bool { + match MembershipType::from_i32(self.atype) { + Some(MembershipType::Owner | MembershipType::Admin) => false, + Some(MembershipType::Manager) => !self.access_all, + Some(MembershipType::Custom) => !self.create_new_collections, + Some(MembershipType::User) | None => true, + } + } + + pub fn can_delete_any_collection(&self) -> bool { + self.has_status(MembershipStatus::Confirmed) + && (self.atype >= MembershipType::Admin || self.has_delete_any_collection()) + } + + pub fn clear_custom_permissions(&mut self) { + self.manage_users = false; + self.manage_groups = false; + self.manage_policies = false; + self.create_new_collections = false; + self.edit_any_collection = false; + self.delete_any_collection = false; + } + pub async fn find_by_uuid(uuid: &MembershipId, conn: &DbConn) -> Option { conn.run(move |conn| { users_organizations::table.filter(users_organizations::uuid.eq(uuid)).first::(conn).ok() @@ -1274,6 +1339,13 @@ pub struct OrgApiKeyId(String); mod tests { use super::*; + fn membership(member_type: MembershipType) -> Membership { + let mut membership = Membership::new("test-user".to_owned().into(), "test-org".to_owned().into(), None); + membership.atype = member_type as i32; + membership.status = MembershipStatus::Confirmed as i32; + membership + } + #[test] #[allow(non_snake_case)] fn partial_cmp_MembershipType() { @@ -1288,4 +1360,93 @@ mod tests { assert!(MembershipType::Custom > MembershipType::User); assert!(MembershipType::Admin > MembershipType::Custom); } + + #[test] + fn custom_collection_permissions_are_independent_and_type_gated() { + let mut member = membership(MembershipType::Custom); + member.create_new_collections = true; + + assert!(member.has_create_new_collections()); + assert!(member.can_create_new_collections()); + assert!(!member.limit_collection_creation()); + assert!(!member.has_full_access()); + assert!(!member.can_delete_any_collection()); + assert!(!member.has_manage_all_collections()); + + member.delete_any_collection = true; + assert!(member.has_delete_any_collection()); + assert!(member.can_delete_any_collection()); + assert!(!member.has_full_access()); + assert!(!member.has_manage_all_collections()); + + member.edit_any_collection = true; + assert!(member.has_edit_any_collection()); + assert!(member.has_full_access()); + assert!(member.has_manage_all_collections()); + + // Stale flags on a non-Custom role are inert. + member.atype = MembershipType::User as i32; + assert!(!member.has_create_new_collections()); + assert!(!member.has_edit_any_collection()); + assert!(!member.has_delete_any_collection()); + assert!(!member.can_create_new_collections()); + assert!(!member.can_delete_any_collection()); + assert!(!member.has_full_access()); + } + + #[test] + fn edit_any_collection_does_not_imply_create_or_delete() { + let mut custom = membership(MembershipType::Custom); + custom.edit_any_collection = true; + // The persisted access_all mirror is intentionally tested too: client-facing create and + // delete decisions must still use their dedicated permissions. + custom.access_all = true; + + assert!(custom.has_full_access()); + assert!(!custom.can_create_new_collections()); + assert!(custom.limit_collection_creation()); + assert!(!custom.can_delete_any_collection()); + + let mut manager = membership(MembershipType::Manager); + manager.access_all = true; + assert!(manager.can_create_new_collections()); + + let admin = membership(MembershipType::Admin); + assert!(admin.can_create_new_collections()); + assert!(!admin.limit_collection_creation()); + assert!(admin.can_delete_any_collection()); + } + + #[test] + fn custom_collection_permissions_require_confirmed_membership() { + let mut member = membership(MembershipType::Custom); + member.create_new_collections = true; + member.edit_any_collection = true; + member.delete_any_collection = true; + member.status = MembershipStatus::Accepted as i32; + + assert!(!member.can_create_new_collections()); + assert!(!member.can_delete_any_collection()); + assert!(!member.has_full_access()); + } + + #[test] + fn clearing_custom_permissions_clears_every_flag() { + let mut member = membership(MembershipType::Custom); + member.manage_users = true; + member.manage_groups = true; + member.manage_policies = true; + member.create_new_collections = true; + member.edit_any_collection = true; + member.delete_any_collection = true; + + member.clear_custom_permissions(); + + assert!(!member.manage_users); + assert!(!member.manage_groups); + assert!(!member.manage_policies); + assert!(!member.create_new_collections); + assert!(!member.edit_any_collection); + assert!(!member.delete_any_collection); + } } diff --git a/src/db/schema.rs b/src/db/schema.rs index 15c43b5f..e8840acd 100644 --- a/src/db/schema.rs +++ b/src/db/schema.rs @@ -245,6 +245,9 @@ table! { manage_users -> Bool, manage_groups -> Bool, manage_policies -> Bool, + create_new_collections -> Bool, + edit_any_collection -> Bool, + delete_any_collection -> Bool, } } diff --git a/src/static/templates/scss/vaultwarden.scss.hbs b/src/static/templates/scss/vaultwarden.scss.hbs index c5f790ac..6cf97cb6 100644 --- a/src/static/templates/scss/vaultwarden.scss.hbs +++ b/src/static/templates/scss/vaultwarden.scss.hbs @@ -116,9 +116,8 @@ app-security > app-two-factor-setup > form { } /* Hide unsupported Custom Role options */ -/* Note: manageUsers and managePolicies are supported by Vaultwarden - and are intentionally NOT hidden here. */ -bit-dialog div.tw-ml-4:has(bit-form-control input), +/* The collection permission group plus manageUsers, manageGroups, managePolicies are supported + by Vaultwarden and are intentionally not hidden here. */ bit-dialog div.tw-col-span-4:has(input[formcontrolname*="access"]), bit-dialog bit-form-control:has(input[formcontrolname="manageSso"]), bit-dialog bit-form-control:has(input[formcontrolname="manageResetPassword"]) {