From 81ec0966cb96065d19ab0e89db026b25e98879b8 Mon Sep 17 00:00:00 2001 From: Tom <83423411+tom27052006@users.noreply.github.com> Date: Sat, 14 Nov 2026 19:51:44 +0100 Subject: [PATCH] Add Custom Roles group and policy permissions (#7962) --- src/api/core/organizations.rs | 267 +++++++++++++----- src/api/core/two_factor/mod.rs | 7 +- src/auth.rs | 11 + src/db/models/organization.rs | 4 +- .../templates/scss/vaultwarden.scss.hbs | 4 +- 5 files changed, 217 insertions(+), 76 deletions(-) diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index 2703b9ea..693c17ff 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -13,8 +13,8 @@ use crate::{ }, auth::{ AdminHeaders, CollectionAction, CollectionDeleteHeaders, CollectionReadHeaders, CollectionUsersReadHeaders, - Headers, ManageUsersHeaders, ManagerHeaders, ManagerHeadersLoose, OrgMemberHeaders, OwnerHeaders, - custom_member_may, decode_invite, + Headers, ManageGroupsHeaders, ManagePoliciesHeaders, ManageUsersHeaders, ManagerHeaders, ManagerHeadersLoose, + OrgMemberHeaders, OwnerHeaders, custom_member_may, decode_invite, }, db::{ DbConn, @@ -369,7 +369,10 @@ async fn get_org_collections(org_id: OrganizationId, headers: ManagerHeadersLoos err!("Organization not found", "Organization id's do not match"); } - let collections = if headers.membership.can_view_all_collections() { + // Manage groups may list organization collections for group access administration without editing them. + let collections = if headers.membership.can_view_all_collections() + || headers.membership.has_custom_permission(CustomPermission::ManageGroups) + { get_org_collections_impl(&org_id, &conn).await } else if headers.membership.has_type(MembershipType::Custom) { // A Custom member gets the collections they may manage @@ -413,13 +416,15 @@ async fn get_org_collections_details(org_id: OrganizationId, headers: ManagerHea // check if current user has full access to the organization (either directly or via any group) let has_full_access_to_org = member.has_full_access() || (CONFIG.org_groups_enabled() && GroupUser::has_full_access_by_member(&org_id, &member.uuid, &conn).await); - // Manage users may read collection access details without editing the collection itself. - let custom_collections = - if member.has_type(MembershipType::Custom) && !member.can_view_all_collection_access_details() { - Some(Collection::find_explicitly_manageable_by_user(&org_id, &member.user_uuid, &conn).await) - } else { - None - }; + // Manage users and Manage groups may read collection access details without editing collection metadata. + let custom_collections = if member.has_type(MembershipType::Custom) + && !member.can_view_all_collection_access_details() + && !member.has_custom_permission(CustomPermission::ManageGroups) + { + Some(Collection::find_explicitly_manageable_by_user(&org_id, &member.user_uuid, &conn).await) + } else { + None + }; // 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. @@ -591,13 +596,22 @@ async fn post_bulk_access_collections( err!("Invalid group", format!("Group {} does not belong to organization {}!", g.id.escape_debug(), org_id)) } - for col_id in data.collection_ids { - let Some(collection) = Collection::find_by_uuid_and_org(&col_id, &org_id, &conn).await else { + if data.collection_ids.is_empty() + || data.collection_ids.iter().collect::>().len() != data.collection_ids.len() + { + err!("Invalid collection list") + } + + let mut collections = Vec::with_capacity(data.collection_ids.len()); + for col_id in &data.collection_ids { + let Some(collection) = Collection::find_by_uuid_and_org(col_id, &org_id, &conn).await else { err!("Collection not found") }; let manageable = if headers.membership.has_type(MembershipType::Custom) { custom_member_may(CollectionAction::Edit, &headers.membership, &collection.uuid, &conn).await + || (headers.membership.has_custom_permission(CustomPermission::ManageUsers) + && headers.membership.has_custom_permission(CustomPermission::ManageGroups)) } else { collection.is_manageable_by_user(&headers.membership.user_uuid, &conn).await }; @@ -605,6 +619,19 @@ async fn post_bulk_access_collections( err!("Collection not found", "The current user isn't a manager for this collection") } + collections.push(collection); + } + + let mut members = Vec::with_capacity(data.users.len()); + for user in &data.users { + let Some(member) = Membership::find_by_uuid_and_org(&user.id, &org_id, &conn).await else { + err!("User is not part of organization") + }; + members.push(member); + } + + for collection in collections { + let col_id = collection.uuid.clone(); // update collection modification date collection.save(&conn).await?; @@ -619,19 +646,13 @@ async fn post_bulk_access_collections( ) .await; - CollectionGroup::delete_all_by_collection(&col_id, &org_id, &conn).await?; for group in &data.groups { CollectionGroup::new(col_id.clone(), group.id.clone(), group.read_only, group.hide_passwords, group.manage) .save(&org_id, &conn) .await?; } - CollectionUser::delete_all_by_collection(&col_id, &conn).await?; - for user in &data.users { - let Some(member) = Membership::find_by_uuid_and_org(&user.id, &org_id, &conn).await else { - err!("User is not part of organization") - }; - + for (user, member) in data.users.iter().zip(&members) { if member.access_all { continue; } @@ -2262,7 +2283,7 @@ async fn post_bulk_collections(data: Json, headers: Headers } #[get("/organizations//policies")] -async fn list_policies(org_id: OrganizationId, headers: AdminHeaders, conn: DbConn) -> JsonResult { +async fn list_policies(org_id: OrganizationId, headers: ManagePoliciesHeaders, conn: DbConn) -> JsonResult { if org_id != headers.org_id { err!("Organization not found", "Organization id's do not match"); } @@ -2328,7 +2349,7 @@ async fn get_master_password_policy(org_id: OrganizationId, _headers: OrgMemberH } #[get("/organizations//policies/", rank = 3)] -async fn get_policy(org_id: OrganizationId, pol_type: i32, headers: AdminHeaders, conn: DbConn) -> JsonResult { +async fn get_policy(org_id: OrganizationId, pol_type: i32, headers: ManagePoliciesHeaders, conn: DbConn) -> JsonResult { if org_id != headers.org_id { err!("Organization not found", "Organization id's do not match"); } @@ -2365,7 +2386,7 @@ async fn put_policy( org_id: OrganizationId, pol_type: i32, data: Json, - headers: AdminHeaders, + headers: ManagePoliciesHeaders, conn: DbConn, ) -> JsonResult { if org_id != headers.org_id { @@ -2463,8 +2484,11 @@ async fn put_policy( } } + // Bitwarden enforces these policies when they first become enabled, not on every update to an enabled policy. + let newly_enabled = data.enabled && !is_currently_enabled; + // When enabling the TwoFactorAuthentication policy, revoke all members that do not have 2FA - if pol_type_enum == OrgPolicyType::TwoFactorAuthentication && data.enabled { + if pol_type_enum == OrgPolicyType::TwoFactorAuthentication && newly_enabled { two_factor::enforce_2fa_policy_for_org( &org_id, &headers.user.uuid, @@ -2476,38 +2500,9 @@ async fn put_policy( } // When enabling the SingleOrg policy, remove this org's members that are members of other orgs - if pol_type_enum == OrgPolicyType::SingleOrg && data.enabled { - for mut member in Membership::find_by_org(&org_id, &conn).await { - // Policy only applies to non-Owner/non-Admin members who have accepted joining the org - // Exclude invited and revoked users when checking for this policy. - // Those users will not be allowed to accept or be activated because of the policy checks done there. - if member.atype < MembershipType::Admin - && member.status != MembershipStatus::Invited as i32 - && Membership::count_accepted_and_confirmed_by_user(&member.user_uuid, &member.org_uuid, &conn).await - > 0 - { - if CONFIG.mail_enabled() { - let org = Organization::find_by_uuid(&member.org_uuid, &conn).await.unwrap(); - let user = User::find_by_uuid(&member.user_uuid, &conn).await.unwrap(); - - mail::send_single_org_removed_from_org(&user.email, &org.name).await?; - } - - log_event( - EventType::OrganizationUserRevokedSingleOrganizationNonCompliance, - &member.uuid, - &org_id, - &headers.user.uuid, - headers.device.atype, - &headers.ip.ip, - &conn, - ) - .await; - - member.revoke(); - member.save(&conn).await?; - } - } + if pol_type_enum == OrgPolicyType::SingleOrg && newly_enabled { + enforce_single_org_policy_for_org(&org_id, &headers.user.uuid, headers.device.atype, &headers.ip.ip, &conn) + .await?; } let mut policy = match current_policy { @@ -2533,6 +2528,45 @@ async fn put_policy( Ok(Json(policy.to_json())) } +async fn enforce_single_org_policy_for_org( + org_id: &OrganizationId, + acting_user_id: &UserId, + device_type: i32, + ip: &std::net::IpAddr, + conn: &DbConn, +) -> EmptyResult { + for mut member in Membership::find_by_org(org_id, conn).await { + // Enforcement at activation excludes the actor, Owners, Admins, invited and revoked members. + // Later membership activation still checks the enabled policy independently. + if member.atype < MembershipType::Admin + && member.user_uuid != *acting_user_id + && member.status != MembershipStatus::Invited as i32 + && Membership::count_accepted_and_confirmed_by_user(&member.user_uuid, &member.org_uuid, conn).await > 0 + { + if CONFIG.mail_enabled() { + let org = Organization::find_by_uuid(&member.org_uuid, conn).await.unwrap(); + let user = User::find_by_uuid(&member.user_uuid, conn).await.unwrap(); + mail::send_single_org_removed_from_org(&user.email, &org.name).await?; + } + + log_event( + EventType::OrganizationUserRevokedSingleOrganizationNonCompliance, + &member.uuid, + org_id, + acting_user_id, + device_type, + ip, + conn, + ) + .await; + + member.revoke(); + member.save(conn).await?; + } + } + Ok(()) +} + #[get("/plans")] fn get_plans() -> Json { // Respond with a minimal json just enough to allow the creation of an new organization. @@ -2790,11 +2824,12 @@ async fn restore_member_impl( } /// Whether a member may give others access to collections, and so needs to see the members and groups: by seeing -/// or creating any collection, by managing the members, or by the manage option on a collection. +/// or creating any collection, by managing the members or groups, or by the manage option on a collection. async fn may_assign_collection_access(member: &Membership, conn: &DbConn) -> bool { member.can_view_all_collections() || member.has_custom_permission(CustomPermission::CreateNewCollections) || member.has_custom_permission(CustomPermission::ManageUsers) + || member.has_custom_permission(CustomPermission::ManageGroups) || Collection::has_manageable_collection_by_user(&member.org_uuid, &member.user_uuid, conn).await } @@ -2817,7 +2852,9 @@ async fn get_groups_data( && !headers.membership.has_type(MembershipType::Custom) && GroupUser::has_full_access_by_member(&org_id, &headers.membership.uuid, &conn).await); let allowed = if details { - has_full_access || headers.membership.has_custom_permission(CustomPermission::ManageUsers) + has_full_access + || headers.membership.has_custom_permission(CustomPermission::ManageUsers) + || headers.membership.has_custom_permission(CustomPermission::ManageGroups) } else { has_full_access || may_assign_collection_access(&headers.membership, &conn).await }; @@ -2866,8 +2903,7 @@ async fn get_groups_details(org_id: OrganizationId, headers: ManagerHeadersLoose #[serde(rename_all = "camelCase")] struct GroupRequest { name: String, - #[serde(default)] - access_all: bool, + access_all: Option, external_id: Option, collections: Vec, users: Vec, @@ -2875,12 +2911,14 @@ struct GroupRequest { impl GroupRequest { pub fn to_group(&self, org_uuid: &OrganizationId) -> Group { - Group::new(org_uuid.clone(), self.name.clone(), self.access_all, self.external_id.clone()) + Group::new(org_uuid.clone(), self.name.clone(), self.access_all.unwrap_or(false), self.external_id.clone()) } pub fn update_group(&self, mut group: Group) -> Group { group.name.clone_from(&self.name); - group.access_all = self.access_all; + if let Some(access_all) = self.access_all { + group.access_all = access_all; + } // Group Updates do not support changing the external_id // These input fields are in a disabled state, and can only be updated/added via ldap_import @@ -2926,7 +2964,7 @@ impl CollectionData { #[post("/organizations//groups", data = "")] async fn post_groups( org_id: OrganizationId, - headers: AdminHeaders, + headers: ManageGroupsHeaders, data: Json, conn: DbConn, ) -> JsonResult { @@ -2961,7 +2999,7 @@ async fn put_group( org_id: OrganizationId, group_id: GroupId, data: Json, - headers: AdminHeaders, + headers: ManageGroupsHeaders, conn: DbConn, ) -> JsonResult { if org_id != headers.org_id { @@ -3002,7 +3040,7 @@ async fn add_update_group( collections: Vec, members: Vec, org_id: OrganizationId, - headers: &AdminHeaders, + headers: &ManageGroupsHeaders, conn: &DbConn, ) -> JsonResult { group.save(conn).await?; @@ -3041,7 +3079,7 @@ async fn add_update_group( async fn get_group_details( org_id: OrganizationId, group_id: GroupId, - headers: AdminHeaders, + headers: ManageGroupsHeaders, conn: DbConn, ) -> JsonResult { if org_id != headers.org_id { @@ -3059,14 +3097,19 @@ async fn get_group_details( } #[delete("/organizations//groups/")] -async fn delete_group(org_id: OrganizationId, group_id: GroupId, headers: AdminHeaders, conn: DbConn) -> EmptyResult { +async fn delete_group( + org_id: OrganizationId, + group_id: GroupId, + headers: ManageGroupsHeaders, + conn: DbConn, +) -> EmptyResult { delete_group_impl(&org_id, &group_id, &headers, &conn).await } async fn delete_group_impl( org_id: &OrganizationId, group_id: &GroupId, - headers: &AdminHeaders, + headers: &ManageGroupsHeaders, conn: &DbConn, ) -> EmptyResult { if org_id != &headers.org_id { @@ -3098,7 +3141,7 @@ async fn delete_group_impl( async fn bulk_delete_groups( org_id: OrganizationId, data: Json, - headers: AdminHeaders, + headers: ManageGroupsHeaders, conn: DbConn, ) -> EmptyResult { if org_id != headers.org_id { @@ -3117,7 +3160,12 @@ async fn bulk_delete_groups( } #[get("/organizations//groups/", rank = 2)] -async fn get_group(org_id: OrganizationId, group_id: GroupId, headers: AdminHeaders, conn: DbConn) -> JsonResult { +async fn get_group( + org_id: OrganizationId, + group_id: GroupId, + headers: ManageGroupsHeaders, + conn: DbConn, +) -> JsonResult { if org_id != headers.org_id { err!("Organization not found", "Organization id's do not match"); } @@ -3136,7 +3184,7 @@ async fn get_group(org_id: OrganizationId, group_id: GroupId, headers: AdminHead async fn get_group_members( org_id: OrganizationId, group_id: GroupId, - headers: AdminHeaders, + headers: ManageGroupsHeaders, conn: DbConn, ) -> JsonResult { if org_id != headers.org_id { @@ -3666,4 +3714,83 @@ mod tests { ); }); } + #[cfg(sqlite)] + #[test] + fn group_requests_stay_in_their_organization_and_keep_access_all() { + test_db::run(async |db| { + let conn = db.conn(); + let (org, _, member) = test_db::custom_member(&conn, CustomPermission::ManageGroups).await; + let foreign = test_db::organization(&conn).await; + for org_id in [&org.uuid, &foreign.uuid] { + let collection = test_db::collection(&conn, org_id).await; + let request: GroupRequest = serde_json::from_value(json!({"name":"Group","collections":[ + {"id":collection.uuid,"readOnly":false,"hidePasswords":false,"manage":true}],"users":[]})) + .unwrap(); + assert_eq!(request.validate(&org.uuid, &conn).await.is_ok(), org_id == &org.uuid); + assert!(!Collection::can_access_collection(&member, &collection.uuid, &conn).await); + assert!(!custom_member_may(CollectionAction::Edit, &member, &collection.uuid, &conn).await); + } + let group = Group::new(org.uuid.clone(), "Group".into(), true, None); + let omitted: GroupRequest = + serde_json::from_value(json!({"name":"Group","collections":[],"users":[member.uuid]})).unwrap(); + assert!(omitted.update_group(group).access_all); + }); + } + + #[cfg(sqlite)] + #[test] + fn enabling_policy_exempts_actor_and_admin_and_revokes_only_on_transition() { + test_db::run(async |db| { + let conn = db.conn(); + for policy in [OrgPolicyType::TwoFactorAuthentication, OrgPolicyType::SingleOrg] { + let (org, _, actor) = test_db::custom_member(&conn, CustomPermission::ManagePolicies).await; + let (_, other) = test_db::member(&conn, &org.uuid, MembershipType::User, &[]).await; + let (_, admin) = test_db::member(&conn, &org.uuid, MembershipType::Admin, &[]).await; + if policy == OrgPolicyType::SingleOrg { + let foreign = test_db::organization(&conn).await; + for member in [&actor, &other, &admin] { + let mut m = Membership::new(member.user_uuid.clone(), foreign.uuid.clone(), None); + m.status = 2; + m.save(&conn).await.unwrap(); + } + } + for first_enable in [true, false] { + let headers = users_headers(&actor, &conn).await; + let headers = ManagePoliciesHeaders { + device: headers.device, + user: headers.user, + membership: headers.membership, + membership_type: headers.membership_type, + ip: headers.ip, + org_id: headers.org_id, + }; + put_policy( + org.uuid.clone(), + policy as i32, + Json(PutPolicy { + policy: PolicyData { + enabled: true, + data: None, + }, + }), + headers, + db.conn(), + ) + .await + .unwrap(); + for member in [&actor, &other, &admin] { + let mut after = Membership::find_by_uuid(&member.uuid, &conn).await.unwrap(); + assert_eq!( + after.has_status(MembershipStatus::Confirmed), + !first_enable || member.uuid != other.uuid + ); + if first_enable && member.uuid == other.uuid { + assert!(after.restore()); + after.save(&conn).await.unwrap(); + } + } + } + } + }); + } } diff --git a/src/api/core/two_factor/mod.rs b/src/api/core/two_factor/mod.rs index cfec98c7..55e374c0 100644 --- a/src/api/core/two_factor/mod.rs +++ b/src/api/core/two_factor/mod.rs @@ -175,8 +175,11 @@ pub async fn enforce_2fa_policy_for_org( ) -> EmptyResult { let org = Organization::find_by_uuid(org_id, conn).await.unwrap(); for member in Membership::find_confirmed_by_org(org_id, conn).await { - // Don't enforce the policy for Admins and Owners. - if member.atype < MembershipType::Admin && TwoFactor::find_by_user(&member.user_uuid, conn).await.is_empty() { + // Don't revoke Admins, Owners, or the authenticated administrator enabling the policy. + if member.atype < MembershipType::Admin + && member.user_uuid != *act_user_id + && TwoFactor::find_by_user(&member.user_uuid, conn).await.is_empty() + { if CONFIG.mail_enabled() { let user = User::find_by_uuid(&member.user_uuid, conn).await.unwrap(); mail::send_2fa_removed_from_org(&user.email, &org.name).await?; diff --git a/src/auth.rs b/src/auth.rs index fec9bad6..b19b9664 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -887,6 +887,7 @@ impl<'r> FromRequest<'r> for AdminHeaders { macro_rules! permission_headers { ($(#[$doc:meta])* $name:ident, $permission:expr) => { $(#[$doc])* + #[allow(dead_code, reason = "Not every route reads every field")] pub struct $name { pub device: Device, pub user: User, @@ -926,6 +927,16 @@ permission_headers!( ManageUsersHeaders, CustomPermission::ManageUsers ); +permission_headers!( + /// For the routes that manage the policies of an organization. + ManagePoliciesHeaders, + CustomPermission::ManagePolicies +); +permission_headers!( + /// For the routes that manage the groups of an organization. + ManageGroupsHeaders, + CustomPermission::ManageGroups +); // col_id is usually the fourth path param ("/organizations//collections/"), // but there could be cases where it is a query value. diff --git a/src/db/models/organization.rs b/src/db/models/organization.rs index 7eb2260b..4936742e 100644 --- a/src/db/models/organization.rs +++ b/src/db/models/organization.rs @@ -248,6 +248,8 @@ pub enum CustomPermission { /// grant of any other one grants nothing. They are implemented one at a time, so this grows with them. const SUPPORTED_CUSTOM_PERMISSIONS: &[CustomPermission] = &[ CustomPermission::ManageUsers, + CustomPermission::ManageGroups, + CustomPermission::ManagePolicies, CustomPermission::CreateNewCollections, CustomPermission::EditAnyCollection, CustomPermission::DeleteAnyCollection, @@ -1612,7 +1614,7 @@ mod tests { } assert_eq!( SUPPORTED_CUSTOM_PERMISSIONS, - [ManageUsers, CreateNewCollections, EditAnyCollection, DeleteAnyCollection] + [ManageUsers, ManageGroups, ManagePolicies, CreateNewCollections, EditAnyCollection, DeleteAnyCollection] ); } diff --git a/src/static/templates/scss/vaultwarden.scss.hbs b/src/static/templates/scss/vaultwarden.scss.hbs index 5328e82e..c2cda8ed 100644 --- a/src/static/templates/scss/vaultwarden.scss.hbs +++ b/src/static/templates/scss/vaultwarden.scss.hbs @@ -99,10 +99,8 @@ app-security > app-two-factor-setup > form { /* Hide unsupported Custom Role options */ {{#if custom_roles_enabled}} -/* Only the collection permissions and Manage users can be granted yet */ +/* Only the collection permissions, Manage users, Manage groups and Manage policies can be granted yet */ :is(bit-dialog, [bit-dialog]) div.tw-col-span-4:has(input[formcontrolname*="access"]), -:is(bit-dialog, [bit-dialog]) bit-form-control:has(input[formcontrolname="manageGroups"]), -:is(bit-dialog, [bit-dialog]) bit-form-control:has(input[formcontrolname="managePolicies"]), :is(bit-dialog, [bit-dialog]) bit-form-control:has(input[formcontrolname="manageSso"]), :is(bit-dialog, [bit-dialog]) bit-form-control:has(input[formcontrolname="manageResetPassword"]) { @extend %vw-hide;