From 00611ea80d4b2e1ea5afa76c6dd6fc7290068195 Mon Sep 17 00:00:00 2001 From: Tom <83423411+tom27052006@users.noreply.github.com> Date: Sun, 15 Nov 2026 15:25:06 +0100 Subject: [PATCH] Add Custom Roles account recovery permission (#7965) --- .../down.sql | 13 ++ .../up.sql | 3 + .../down.sql | 11 ++ .../up.sql | 3 + .../down.sql | 11 ++ .../up.sql | 3 + src/api/core/organizations.rs | 119 ++++++++++++++---- src/auth.rs | 17 ++- src/db/models/organization.rs | 20 ++- src/db/schema.rs | 1 + .../templates/scss/vaultwarden.scss.hbs | 5 +- 11 files changed, 168 insertions(+), 38 deletions(-) create mode 100644 migrations/mysql/2026-11-14-120000_add_custom_role_manage_reset_password/down.sql create mode 100644 migrations/mysql/2026-11-14-120000_add_custom_role_manage_reset_password/up.sql create mode 100644 migrations/postgresql/2026-11-14-120000_add_custom_role_manage_reset_password/down.sql create mode 100644 migrations/postgresql/2026-11-14-120000_add_custom_role_manage_reset_password/up.sql create mode 100644 migrations/sqlite/2026-11-14-120000_add_custom_role_manage_reset_password/down.sql create mode 100644 migrations/sqlite/2026-11-14-120000_add_custom_role_manage_reset_password/up.sql diff --git a/migrations/mysql/2026-11-14-120000_add_custom_role_manage_reset_password/down.sql b/migrations/mysql/2026-11-14-120000_add_custom_role_manage_reset_password/down.sql new file mode 100644 index 00000000..394aa8e3 --- /dev/null +++ b/migrations/mysql/2026-11-14-120000_add_custom_role_manage_reset_password/down.sql @@ -0,0 +1,13 @@ +-- Dropping the permission would silently take it away from the Custom members holding it. So while there are any, +-- this fails on the duplicate key below, before anything is dropped: take it away first, or restore the backup +-- taken before the upgrade. +-- A failed attempt leaves the temporary table on this connection. Clear it before retrying. +DROP TEMPORARY TABLE IF EXISTS custom_role_downgrade; +CREATE TEMPORARY TABLE custom_role_downgrade (blocked_by VARCHAR(255) NOT NULL PRIMARY KEY); +INSERT INTO custom_role_downgrade VALUES ('Manage account recovery: take it away first, or restore the backup taken before the upgrade'); +INSERT INTO custom_role_downgrade + SELECT 'Manage account recovery: take it away first, or restore the backup taken before the upgrade' + FROM users_organizations WHERE manage_reset_password LIMIT 1; +DROP TEMPORARY TABLE custom_role_downgrade; + +ALTER TABLE users_organizations DROP COLUMN manage_reset_password; diff --git a/migrations/mysql/2026-11-14-120000_add_custom_role_manage_reset_password/up.sql b/migrations/mysql/2026-11-14-120000_add_custom_role_manage_reset_password/up.sql new file mode 100644 index 00000000..ce4ba9e6 --- /dev/null +++ b/migrations/mysql/2026-11-14-120000_add_custom_role_manage_reset_password/up.sql @@ -0,0 +1,3 @@ +-- The Manage account recovery permission of the Custom member role. Like the other permissions it only takes +-- effect on a Custom membership (atype 4) while EXPERIMENTAL_CUSTOM_ROLES is enabled, and nobody starts with it. +ALTER TABLE users_organizations ADD COLUMN manage_reset_password BOOLEAN NOT NULL DEFAULT FALSE; diff --git a/migrations/postgresql/2026-11-14-120000_add_custom_role_manage_reset_password/down.sql b/migrations/postgresql/2026-11-14-120000_add_custom_role_manage_reset_password/down.sql new file mode 100644 index 00000000..6f96d637 --- /dev/null +++ b/migrations/postgresql/2026-11-14-120000_add_custom_role_manage_reset_password/down.sql @@ -0,0 +1,11 @@ +-- Dropping the permission would silently take it away from the Custom members holding it. So while there are any, +-- this fails on the duplicate key below: take it away first, or restore the backup taken before the upgrade. +-- It needs to run in one transaction, as Diesel does. +CREATE TEMPORARY TABLE custom_role_downgrade (blocked_by VARCHAR(255) NOT NULL PRIMARY KEY); +INSERT INTO custom_role_downgrade VALUES ('Manage account recovery: take it away first, or restore the backup taken before the upgrade'); +INSERT INTO custom_role_downgrade + SELECT 'Manage account recovery: take it away first, or restore the backup taken before the upgrade' + FROM users_organizations WHERE manage_reset_password LIMIT 1; +DROP TABLE custom_role_downgrade; + +ALTER TABLE users_organizations DROP COLUMN manage_reset_password; diff --git a/migrations/postgresql/2026-11-14-120000_add_custom_role_manage_reset_password/up.sql b/migrations/postgresql/2026-11-14-120000_add_custom_role_manage_reset_password/up.sql new file mode 100644 index 00000000..ce4ba9e6 --- /dev/null +++ b/migrations/postgresql/2026-11-14-120000_add_custom_role_manage_reset_password/up.sql @@ -0,0 +1,3 @@ +-- The Manage account recovery permission of the Custom member role. Like the other permissions it only takes +-- effect on a Custom membership (atype 4) while EXPERIMENTAL_CUSTOM_ROLES is enabled, and nobody starts with it. +ALTER TABLE users_organizations ADD COLUMN manage_reset_password BOOLEAN NOT NULL DEFAULT FALSE; diff --git a/migrations/sqlite/2026-11-14-120000_add_custom_role_manage_reset_password/down.sql b/migrations/sqlite/2026-11-14-120000_add_custom_role_manage_reset_password/down.sql new file mode 100644 index 00000000..6f96d637 --- /dev/null +++ b/migrations/sqlite/2026-11-14-120000_add_custom_role_manage_reset_password/down.sql @@ -0,0 +1,11 @@ +-- Dropping the permission would silently take it away from the Custom members holding it. So while there are any, +-- this fails on the duplicate key below: take it away first, or restore the backup taken before the upgrade. +-- It needs to run in one transaction, as Diesel does. +CREATE TEMPORARY TABLE custom_role_downgrade (blocked_by VARCHAR(255) NOT NULL PRIMARY KEY); +INSERT INTO custom_role_downgrade VALUES ('Manage account recovery: take it away first, or restore the backup taken before the upgrade'); +INSERT INTO custom_role_downgrade + SELECT 'Manage account recovery: take it away first, or restore the backup taken before the upgrade' + FROM users_organizations WHERE manage_reset_password LIMIT 1; +DROP TABLE custom_role_downgrade; + +ALTER TABLE users_organizations DROP COLUMN manage_reset_password; diff --git a/migrations/sqlite/2026-11-14-120000_add_custom_role_manage_reset_password/up.sql b/migrations/sqlite/2026-11-14-120000_add_custom_role_manage_reset_password/up.sql new file mode 100644 index 00000000..05557077 --- /dev/null +++ b/migrations/sqlite/2026-11-14-120000_add_custom_role_manage_reset_password/up.sql @@ -0,0 +1,3 @@ +-- The Manage account recovery permission of the Custom member role. Like the other permissions it only takes +-- effect on a Custom membership (atype 4) while EXPERIMENTAL_CUSTOM_ROLES is enabled, and nobody starts with it. +ALTER TABLE users_organizations ADD COLUMN manage_reset_password BOOLEAN NOT NULL DEFAULT 0; -- FALSE diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index 0c59eedf..f0d44534 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -13,8 +13,9 @@ use crate::{ }, auth::{ AccessImportExportHeaders, AdminHeaders, CollectionAction, CollectionDeleteHeaders, CollectionReadHeaders, - CollectionUsersReadHeaders, Headers, ManageGroupsHeaders, ManagePoliciesHeaders, ManageUsersHeaders, - ManagerHeaders, ManagerHeadersLoose, OrgMemberHeaders, OwnerHeaders, custom_member_may, decode_invite, + CollectionUsersReadHeaders, Headers, ManageGroupsHeaders, ManagePoliciesHeaders, ManageResetPasswordHeaders, + ManageUsersHeaders, ManagerHeaders, ManagerHeadersLoose, OrgMemberHeaders, OwnerHeaders, custom_member_may, + decode_invite, }, db::{ DbConn, @@ -1007,13 +1008,18 @@ async fn get_members( err!("Organization not found", "Organization id's do not match"); } - if !headers.membership.has_full_access() && !headers.membership.has_custom_permission(CustomPermission::ManageUsers) - { + // As in Bitwarden, Manage account recovery alone only lists the members whose account it can recover + let recovery_only = !headers.membership.has_full_access() + && !headers.membership.has_custom_permission(CustomPermission::ManageUsers); + if recovery_only && !headers.membership.has_custom_permission(CustomPermission::ManageResetPassword) { err_code!("Resource not found.", "User does not have full access", Status::NotFound.code); } let mut users_json = Vec::new(); for u in Membership::find_by_org(&org_id, &conn).await { + if recovery_only && (!u.has_status(MembershipStatus::Confirmed) || u.reset_password_key.is_none()) { + continue; + } // The user can be a manager instead of an admin, but we've checked above that they have full access users_json.push( u.to_json_details_for_admin( @@ -1036,7 +1042,7 @@ async fn get_members( async fn post_org_keys( org_id: OrganizationId, data: Json, - headers: AdminHeaders, + headers: ManageResetPasswordHeaders, conn: DbConn, ) -> JsonResult { if org_id != headers.org_id { @@ -2883,9 +2889,10 @@ async fn get_groups_data( || headers.membership.has_custom_permission(CustomPermission::ManageUsers) || headers.membership.has_custom_permission(CustomPermission::ManageGroups) } else { - // Reports need the basic group list, not group details or management rights. + // Reports and account recovery need the basic group list, not group details or management rights. has_full_access || headers.membership.has_custom_permission(CustomPermission::AccessReports) + || headers.membership.has_custom_permission(CustomPermission::ManageResetPassword) || may_assign_collection_access(&headers.membership, &conn).await }; if !allowed { @@ -3322,7 +3329,7 @@ async fn get_organization_keys(org_id: OrganizationId, headers: Headers, conn: D async fn put_recover_account( org_id: OrganizationId, member_id: MembershipId, - headers: AdminHeaders, + headers: ManageResetPasswordHeaders, data: Json, conn: DbConn, nt: Notify<'_>, @@ -3333,7 +3340,7 @@ async fn put_recover_account( async fn recover_account( org_id: OrganizationId, member_id: MembershipId, - headers: AdminHeaders, + headers: ManageResetPasswordHeaders, req: OrganizationUserRecoverAccountRequest, conn: DbConn, nt: Notify<'_>, @@ -3353,7 +3360,7 @@ async fn recover_account( err!("User not found") }; - check_reset_password_applicable_and_permissions(&org_id, &member_id, &headers, &conn).await?; + check_reset_password_applicable_and_permissions(&org_id, &member_id, &headers.membership, &conn).await?; if member.reset_password_key.is_none() { err!("Password reset not or not correctly enrolled"); @@ -3423,12 +3430,14 @@ async fn recover_account( nt.send_logout(&user, None, &conn).await; - if req.reset_master_password { - headers.log_event(EventType::OrganizationUserAdminResetPassword, &member_id, &org_id, &conn).await; - } - - if req.reset_two_factor { - headers.log_event(EventType::OrganizationUserAdminResetTwoFactor, &member_id, &org_id, &conn).await; + for (reset, event_type) in [ + (req.reset_master_password, EventType::OrganizationUserAdminResetPassword), + (req.reset_two_factor, EventType::OrganizationUserAdminResetTwoFactor), + ] { + if reset { + log_event(event_type, &member_id, &org_id, &headers.user.uuid, headers.device.atype, &headers.ip.ip, &conn) + .await; + } } Ok(()) @@ -3599,7 +3608,7 @@ async fn get_organization_private_key(org_id: OrganizationId, headers: ManageUse async fn get_reset_password_details( org_id: OrganizationId, member_id: MembershipId, - headers: AdminHeaders, + headers: ManageResetPasswordHeaders, conn: DbConn, ) -> JsonResult { if org_id != headers.org_id { @@ -3617,7 +3626,7 @@ async fn get_reset_password_details( err!("User not found") }; - check_reset_password_applicable_and_permissions(&org_id, &member_id, &headers, &conn).await?; + check_reset_password_applicable_and_permissions(&org_id, &member_id, &headers.membership, &conn).await?; // https://github.com/bitwarden/server/blob/9ebe16587175b1c0e9208f84397bb75d0d595510/src/Api/AdminConsole/Models/Response/Organizations/OrganizationUserResponseModel.cs#L190 Ok(Json(json!({ @@ -3636,7 +3645,7 @@ async fn get_reset_password_details( async fn check_reset_password_applicable_and_permissions( org_id: &OrganizationId, member_id: &MembershipId, - headers: &AdminHeaders, + caller: &Membership, conn: &DbConn, ) -> EmptyResult { check_reset_password_applicable(org_id, conn).await?; @@ -3645,11 +3654,25 @@ async fn check_reset_password_applicable_and_permissions( err!("Reset target user not found") }; - // Resetting user must be higher/equal to user to reset - match headers.membership_type { - MembershipType::Owner => Ok(()), - MembershipType::Admin if target_user.atype <= MembershipType::Admin => Ok(()), - _ => err!("No permission to reset this user's password"), + if may_recover_account(caller, &target_user) { + Ok(()) + } else { + err!("No permission to reset this user's password") + } +} + +/// Whether `caller` may recover the account of `target`, who must not have more authority, as in Bitwarden: Owners +/// may recover every member, Admins every member but Owners, and Custom members with Manage account recovery Users +/// and the Custom members whose permissions they all have. +fn may_recover_account(caller: &Membership, target: &Membership) -> bool { + if caller.has_type(MembershipType::Owner) { + true + } else if caller.has_type(MembershipType::Admin) { + target.atype <= MembershipType::Admin + } else { + caller.has_custom_permission(CustomPermission::ManageResetPassword) + && custom_member_manages(target.atype) + && caller.has_custom_permissions_of(target) } } @@ -4035,4 +4058,54 @@ mod tests { } }); } + + #[cfg(sqlite)] + #[test] + fn account_recovery_authorization() { + test_db::run(async |db| { + use CustomPermission::{ManageResetPassword, ManageUsers}; + use MembershipType::*; + let conn = db.conn(); + let org = test_db::organization(&conn).await; + let mut members = Vec::new(); + for (role, permissions) in [ + (Owner, vec![]), + (Admin, vec![]), + (Manager, vec![]), + (User, vec![]), + (Custom, vec![ManageResetPassword]), + (Custom, vec![ManageResetPassword, ManageUsers]), + ] { + members.push(test_db::member(&conn, &org.uuid, role, &permissions).await.1); + } + // Who may recover whom: a row per caller, a column per target, both in the order above + for (caller, row) in members.iter().zip(["111111", "011111", "000000", "000000", "000110", "000111"]) { + for (target, allowed) in members.iter().zip(row.chars()) { + assert_eq!( + may_recover_account(caller, target), + allowed == '1', + "{} {}", + caller.atype, + target.atype + ); + } + } + // Manage account recovery alone lists only the confirmed members enrolled in account recovery + for (i, status) in [(2, MembershipStatus::Accepted), (3, MembershipStatus::Confirmed)] { + members[i].status = status as i32; + members[i].reset_password_key = Some("key".into()); + members[i].save(&conn).await.unwrap(); + } + let data = GetOrgUserData { + include_collections: None, + include_groups: None, + }; + let users = get_members(data, org.uuid.clone(), directory_headers(&members[4], &conn).await, db.conn()); + assert_eq!( + users.await.unwrap().0["data"], + json!([members[3].to_json_details_for_admin(false, false, &conn).await]) + ); + assert!(get_groups(org.uuid.clone(), directory_headers(&members[4], &conn).await, db.conn()).await.is_ok()); + }); + } } diff --git a/src/auth.rs b/src/auth.rs index 0a43b41b..9da34a85 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -27,14 +27,14 @@ use rocket::{ use crate::{ CONFIG, - api::{ApiResult, core::log_event}, + api::ApiResult, config::PathType, db::{ DbConn, models::{ AttachmentId, CipherId, Collection, CollectionId, CustomPermission, Device, DeviceId, DeviceType, - EmergencyAccessId, EventType, Membership, MembershipId, MembershipStatus, MembershipType, OrgApiKeyId, - OrganizationId, SendFileId, SendId, User, UserId, custom_roles_enabled, + EmergencyAccessId, Membership, MembershipId, MembershipStatus, MembershipType, OrgApiKeyId, OrganizationId, + SendFileId, SendId, User, UserId, custom_roles_enabled, }, }, error::Error, @@ -854,12 +854,6 @@ pub struct AdminHeaders { pub org_id: OrganizationId, } -impl AdminHeaders { - pub async fn log_event(&self, event_type: EventType, source_uuid: &str, org_id: &OrganizationId, conn: &DbConn) { - log_event(event_type, source_uuid, org_id, &self.user.uuid, self.device.atype, &self.ip.ip, conn).await; - } -} - #[rocket::async_trait] impl<'r> FromRequest<'r> for AdminHeaders { type Error = &'static str; @@ -947,6 +941,11 @@ permission_headers!( AccessImportExportHeaders, CustomPermission::AccessImportExport ); +permission_headers!( + /// For the routes that recover the accounts of the members of an organization. + ManageResetPasswordHeaders, + CustomPermission::ManageResetPassword +); // 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 8115d95c..3c02cb65 100644 --- a/src/db/models/organization.rs +++ b/src/db/models/organization.rs @@ -81,6 +81,7 @@ pub struct Membership { access_event_logs: bool, access_import_export: bool, access_reports: bool, + manage_reset_password: bool, /// The user's v2 upgrade token, for the org to unwrap the v2 user key after an upgrade. Opaque to us. pub v2_upgrade_token: Option, @@ -254,6 +255,7 @@ pub enum CustomPermission { AccessEventLogs, AccessImportExport, AccessReports, + ManageResetPassword, } /// The permissions this version enforces on every endpoint they open. Only these can be granted, a stored @@ -269,10 +271,11 @@ const SUPPORTED_CUSTOM_PERMISSIONS: &[CustomPermission] = &[ CustomPermission::AccessEventLogs, CustomPermission::AccessImportExport, CustomPermission::AccessReports, + CustomPermission::ManageResetPassword, ]; impl CustomPermission { - pub const ALL: [CustomPermission; 9] = [ + pub const ALL: [CustomPermission; 10] = [ CustomPermission::ManageUsers, CustomPermission::ManageGroups, CustomPermission::ManagePolicies, @@ -282,6 +285,7 @@ impl CustomPermission { CustomPermission::AccessEventLogs, CustomPermission::AccessImportExport, CustomPermission::AccessReports, + CustomPermission::ManageResetPassword, ]; /// The key of the permission in the `permissions` object of the Bitwarden clients. @@ -296,6 +300,7 @@ impl CustomPermission { CustomPermission::AccessEventLogs => "accessEventLogs", CustomPermission::AccessImportExport => "accessImportExport", CustomPermission::AccessReports => "accessReports", + CustomPermission::ManageResetPassword => "manageResetPassword", } } @@ -468,6 +473,7 @@ impl Membership { access_event_logs: false, access_import_export: false, access_reports: false, + manage_reset_password: false, v2_upgrade_token: None, } @@ -484,6 +490,7 @@ impl Membership { CustomPermission::AccessEventLogs => &mut self.access_event_logs, CustomPermission::AccessImportExport => &mut self.access_import_export, CustomPermission::AccessReports => &mut self.access_reports, + CustomPermission::ManageResetPassword => &mut self.manage_reset_password, } } @@ -553,6 +560,7 @@ impl Membership { CustomPermission::AccessEventLogs => self.access_event_logs, CustomPermission::AccessImportExport => self.access_import_export, CustomPermission::AccessReports => self.access_reports, + CustomPermission::ManageResetPassword => self.manage_reset_password, } } @@ -571,6 +579,12 @@ impl Membership { self.has_status(MembershipStatus::Confirmed) && self.grants_custom_permission(permission) } + /// Whether this member may act with every Custom role permission stored for `other`, as Bitwarden's + /// `Permissions.Includes`. + pub fn has_custom_permissions_of(&self, other: &Membership) -> bool { + CustomPermission::ALL.into_iter().all(|p| !other.custom_permission(p) || self.has_custom_permission(p)) + } + /// The permissions the "Manage all collections" a legacy Manager is shown with stands for: their `access_all`. pub fn legacy_manager_permissions(&self) -> HashSet { if self.has_type(MembershipType::Manager) && self.access_all { @@ -597,7 +611,6 @@ impl Membership { let shown = self.shown_custom_permissions(); let mut permissions = json!({ "manageSso": false, // Not supported - "manageResetPassword": false, "manageScim": false, // Not supported (Not AGPLv3 Licensed) }); for permission in CustomPermission::ALL { @@ -1720,7 +1733,8 @@ mod tests { DeleteAnyCollection, AccessEventLogs, AccessImportExport, - AccessReports + AccessReports, + ManageResetPassword ] ); } diff --git a/src/db/schema.rs b/src/db/schema.rs index 98792674..112e14e6 100644 --- a/src/db/schema.rs +++ b/src/db/schema.rs @@ -269,6 +269,7 @@ table! { access_event_logs -> Bool, access_import_export -> Bool, access_reports -> Bool, + manage_reset_password -> Bool, v2_upgrade_token -> Nullable, } } diff --git a/src/static/templates/scss/vaultwarden.scss.hbs b/src/static/templates/scss/vaultwarden.scss.hbs index a4c270bc..49cfb549 100644 --- a/src/static/templates/scss/vaultwarden.scss.hbs +++ b/src/static/templates/scss/vaultwarden.scss.hbs @@ -99,9 +99,8 @@ app-security > app-two-factor-setup > form { /* Hide unsupported Custom Role options */ {{#if custom_roles_enabled}} -/* Not supported: Manage SSO and Manage account recovery */ -:is(bit-dialog, [bit-dialog]) bit-form-control:has(input[formcontrolname="manageSso"]), -:is(bit-dialog, [bit-dialog]) bit-form-control:has(input[formcontrolname="manageResetPassword"]) { +/* Not supported: Manage SSO */ +:is(bit-dialog, [bit-dialog]) bit-form-control:has(input[formcontrolname="manageSso"]) { @extend %vw-hide; } {{else}}