From 9dee038f7787520146dbb3682b13841f3d578b82 Mon Sep 17 00:00:00 2001 From: Sacha ATHIAS Date: Tue, 25 Aug 2026 16:16:59 +0200 Subject: [PATCH] feat(org-rbac): store custom permissions as bitflags Store organization custom permissions as integer bitflags and keep API JSON compatibility by mapping payload permissions to/from mask. Use finer authorization guards for org events/export (AccessEventLogs/AccessImportExport). Create permissions as integer directly in initial migrations and drop intermediate conversion migration path for cleaner pre-release history. --- .../up.sql | 2 +- .../up.sql | 2 +- .../up.sql | 2 +- src/api/core/events.rs | 11 ++- src/api/core/organizations.rs | 48 ++++----- src/auth.rs | 48 ++++++++- src/db/models/organization.rs | 99 ++++++++++++++++--- src/db/schema.rs | 2 +- 8 files changed, 168 insertions(+), 46 deletions(-) diff --git a/migrations/mysql/2026-08-24-120000_add_users_organizations_permissions/up.sql b/migrations/mysql/2026-08-24-120000_add_users_organizations_permissions/up.sql index c3f8340d..2cee0e2d 100644 --- a/migrations/mysql/2026-08-24-120000_add_users_organizations_permissions/up.sql +++ b/migrations/mysql/2026-08-24-120000_add_users_organizations_permissions/up.sql @@ -1 +1 @@ -ALTER TABLE users_organizations ADD COLUMN permissions TEXT DEFAULT NULL; +ALTER TABLE users_organizations ADD COLUMN permissions INTEGER DEFAULT NULL; diff --git a/migrations/postgresql/2026-08-24-120000_add_users_organizations_permissions/up.sql b/migrations/postgresql/2026-08-24-120000_add_users_organizations_permissions/up.sql index c3f8340d..2cee0e2d 100644 --- a/migrations/postgresql/2026-08-24-120000_add_users_organizations_permissions/up.sql +++ b/migrations/postgresql/2026-08-24-120000_add_users_organizations_permissions/up.sql @@ -1 +1 @@ -ALTER TABLE users_organizations ADD COLUMN permissions TEXT DEFAULT NULL; +ALTER TABLE users_organizations ADD COLUMN permissions INTEGER DEFAULT NULL; diff --git a/migrations/sqlite/2026-08-24-120000_add_users_organizations_permissions/up.sql b/migrations/sqlite/2026-08-24-120000_add_users_organizations_permissions/up.sql index c3f8340d..2cee0e2d 100644 --- a/migrations/sqlite/2026-08-24-120000_add_users_organizations_permissions/up.sql +++ b/migrations/sqlite/2026-08-24-120000_add_users_organizations_permissions/up.sql @@ -1 +1 @@ -ALTER TABLE users_organizations ADD COLUMN permissions TEXT DEFAULT NULL; +ALTER TABLE users_organizations ADD COLUMN permissions INTEGER DEFAULT NULL; diff --git a/src/api/core/events.rs b/src/api/core/events.rs index 5518fa3c..012be88f 100644 --- a/src/api/core/events.rs +++ b/src/api/core/events.rs @@ -7,7 +7,7 @@ use serde_json::Value; use crate::{ CONFIG, api::{EmptyResult, JsonResult}, - auth::{AdminHeaders, Headers}, + auth::{AccessEventLogsHeaders, Headers}, db::{ DbConn, DbPool, models::{Cipher, CipherId, Event, Membership, MembershipId, OrganizationId, UserId}, @@ -31,7 +31,12 @@ struct EventRange { // Upstream: https://github.com/bitwarden/server/blob/9ebe16587175b1c0e9208f84397bb75d0d595510/src/Api/AdminConsole/Controllers/EventsController.cs#L87 #[get("/organizations//events?")] -async fn get_org_events(org_id: OrganizationId, data: EventRange, headers: AdminHeaders, conn: DbConn) -> JsonResult { +async fn get_org_events( + org_id: OrganizationId, + data: EventRange, + headers: AccessEventLogsHeaders, + conn: DbConn, +) -> JsonResult { if org_id != headers.org_id { err!("Organization not found", "Organization id's do not match"); } @@ -93,7 +98,7 @@ async fn get_user_events( org_id: OrganizationId, member_id: MembershipId, data: EventRange, - headers: AdminHeaders, + headers: AccessEventLogsHeaders, conn: DbConn, ) -> JsonResult { if org_id != headers.org_id { diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index 71a9686a..a2a162e0 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, ManageResetPasswordHeaders, - ManageUsersHeaders, ManagerHeaders, ManagerHeadersLoose, OrgMemberHeaders, OwnerHeaders, decode_invite, + AccessImportExportHeaders, AdminHeaders, Headers, ManageGroupsHeaders, ManagePoliciesHeaders, + ManageResetPasswordHeaders, ManageUsersHeaders, ManagerHeaders, ManagerHeadersLoose, OrgMemberHeaders, + OwnerHeaders, decode_invite, }, db::{ DbConn, @@ -129,21 +130,15 @@ fn resolve_membership_custom_permissions( raw_type: &str, new_type: i32, permissions: &HashMap, -) -> Result<(bool, Option), crate::Error> { +) -> Result<(bool, Option), crate::Error> { let custom_permissions = parse_custom_permissions(raw_type, permissions)?; let access_all = new_type >= MembershipType::Admin || custom_permissions.as_ref().is_some_and(OrganizationUserPermissions::has_manage_all_collections); - let permissions_json = match custom_permissions { - Some(custom_permissions) => match custom_permissions.to_db_json() { - Ok(serialized) => Some(serialized), - Err(error) => err!(format!("Invalid custom role permissions payload: {error:#}")), - }, - None => None, - }; + let permissions_mask = custom_permissions.map(|custom_permissions| custom_permissions.to_mask()); - Ok((access_all, permissions_json)) + Ok((access_all, permissions_mask)) } #[cfg(test)] @@ -171,13 +166,16 @@ mod tests { fn invite_edit_store_custom_permissions_and_get_user_round_trip() { let payload = custom_permissions_payload(); - let (access_all, serialized_permissions) = + let (access_all, permission_mask) = resolve_membership_custom_permissions("4", MembershipType::Manager as i32, &payload).unwrap(); assert!(access_all); - let stored_json = serialized_permissions.expect("custom role payload should be persisted"); - let parsed = OrganizationUserPermissions::from_db_json(Some(&stored_json)).unwrap().unwrap(); + let stored_mask = permission_mask.expect("custom role payload should be persisted"); + assert_eq!(stored_mask, 637); + + let parsed = OrganizationUserPermissions::from_mask(stored_mask); + assert_eq!(stored_mask, parsed.to_mask()); assert_eq!( json!(parsed), @@ -202,11 +200,11 @@ mod tests { fn invite_edit_non_custom_role_keeps_permissions_unset() { let payload = custom_permissions_payload(); - let (access_all, serialized_permissions) = + let (access_all, permission_mask) = resolve_membership_custom_permissions("3", MembershipType::Manager as i32, &payload).unwrap(); assert!(!access_all); - assert!(serialized_permissions.is_none()); + assert!(permission_mask.is_none()); } #[test] @@ -218,13 +216,15 @@ mod tests { ("manageUsers".to_owned(), json!(true)), ]); - let (access_all, serialized_permissions) = + let (access_all, permission_mask) = resolve_membership_custom_permissions("Custom", MembershipType::Manager as i32, &payload).unwrap(); assert!(!access_all); - let stored_json = serialized_permissions.expect("custom role payload should be persisted"); - let parsed = OrganizationUserPermissions::from_db_json(Some(&stored_json)).unwrap().unwrap(); + let stored_mask = permission_mask.expect("custom role payload should be persisted"); + assert_eq!(stored_mask, 536); + + let parsed = OrganizationUserPermissions::from_mask(stored_mask); assert!(parsed.manage_users); assert!(parsed.create_new_collections); @@ -1192,7 +1192,7 @@ async fn send_invite( err!("Only Owners can invite Managers, Admins or Owners") } - let (access_all, permissions_json) = resolve_membership_custom_permissions(raw_type, new_type, &data.permissions)?; + let (access_all, permissions_mask) = resolve_membership_custom_permissions(raw_type, new_type, &data.permissions)?; let mut user_created: bool = false; for email in &data.emails { @@ -1237,7 +1237,7 @@ async fn send_invite( new_member.access_all = access_all; new_member.atype = new_type; new_member.status = member_status; - new_member.permissions = permissions_json.clone(); + new_member.permissions = permissions_mask; new_member.save(&conn).await?; if CONFIG.mail_enabled() { @@ -1680,7 +1680,7 @@ async fn edit_member( err!("Invalid type") }; - let (access_all, permissions_json) = + let (access_all, permissions_mask) = resolve_membership_custom_permissions(raw_type, new_type as i32, &data.permissions)?; let Some(mut member_to_edit) = Membership::find_by_uuid_and_org(&member_id, &org_id, &conn).await else { @@ -1710,7 +1710,7 @@ async fn edit_member( member_to_edit.access_all = access_all; member_to_edit.atype = new_type as i32; - member_to_edit.permissions = permissions_json; + member_to_edit.permissions = permissions_mask; // This check is also done at accept_invite, _confirm_invite, _activate_member, edit_member, admin::update_membership_type // We need to perform the check after changing the type since `admin` is exempt. @@ -3316,7 +3316,7 @@ async fn put_reset_password_enrollment( // Vaultwarden does not yet support exporting only managed collections! // https://github.com/bitwarden/server/blob/9ebe16587175b1c0e9208f84397bb75d0d595510/src/Api/Tools/Controllers/OrganizationExportController.cs#L52 #[get("/organizations//export")] -async fn get_org_export(org_id: OrganizationId, headers: AdminHeaders, conn: DbConn) -> JsonResult { +async fn get_org_export(org_id: OrganizationId, headers: AccessImportExportHeaders, conn: DbConn) -> JsonResult { if org_id != headers.org_id { err!("Organization not found", "Organization id's do not match"); } diff --git a/src/auth.rs b/src/auth.rs index 81931d78..b57eeecb 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -818,7 +818,6 @@ impl<'r> FromRequest<'r> for OrgHeaders { } pub struct AdminHeaders { - pub host: String, pub user: User, pub org_id: OrganizationId, } @@ -831,7 +830,6 @@ impl<'r> FromRequest<'r> for AdminHeaders { let headers = try_outcome!(OrgHeaders::from_request(request).await); if headers.is_confirmed_and_admin() { Outcome::Success(Self { - host: headers.host, user: headers.user, org_id: headers.membership.org_uuid, }) @@ -957,6 +955,52 @@ impl<'r> FromRequest<'r> for ManagePoliciesHeaders { } } +pub struct AccessEventLogsHeaders { + pub org_id: OrganizationId, +} + +#[rocket::async_trait] +impl<'r> FromRequest<'r> for AccessEventLogsHeaders { + type Error = &'static str; + + async fn from_request(request: &'r Request<'_>) -> Outcome { + let headers = try_outcome!(OrgHeaders::from_request(request).await); + let is_admin_or_owner = headers.is_confirmed_and_admin(); + if is_admin_or_owner || headers.has_permission(OrganizationUserPermission::AccessEventLogs) { + Outcome::Success(Self { + org_id: headers.membership.org_uuid, + }) + } else { + err_handler!("You need to be Admin/Owner or have Access Event Logs permission to call this endpoint") + } + } +} + +pub struct AccessImportExportHeaders { + pub host: String, + pub user: User, + pub org_id: OrganizationId, +} + +#[rocket::async_trait] +impl<'r> FromRequest<'r> for AccessImportExportHeaders { + type Error = &'static str; + + async fn from_request(request: &'r Request<'_>) -> Outcome { + let headers = try_outcome!(OrgHeaders::from_request(request).await); + let is_admin_or_owner = headers.is_confirmed_and_admin(); + if is_admin_or_owner || headers.has_permission(OrganizationUserPermission::AccessImportExport) { + Outcome::Success(Self { + host: headers.host, + user: headers.user, + org_id: headers.membership.org_uuid, + }) + } else { + err_handler!("You need to be Admin/Owner or have Access Import Export permission to call this endpoint") + } + } +} + // col_id is usually the fourth path param ("/organizations//collections/"), // but there could be cases where it is a query value. // First check the path, if this is not a valid uuid, try the query values. diff --git a/src/db/models/organization.rs b/src/db/models/organization.rs index 1fbe71c9..561574f3 100644 --- a/src/db/models/organization.rs +++ b/src/db/models/organization.rs @@ -58,7 +58,7 @@ pub struct Membership { pub atype: i32, pub reset_password_key: Option, pub external_id: Option, - pub permissions: Option, + pub permissions: Option, } #[allow(clippy::struct_excessive_bools)] @@ -96,6 +96,19 @@ pub enum OrganizationUserPermission { } impl OrganizationUserPermission { + pub const ACCESS_EVENT_LOGS_BIT: i32 = 1 << 0; + pub const ACCESS_IMPORT_EXPORT_BIT: i32 = 1 << 1; + pub const ACCESS_REPORTS_BIT: i32 = 1 << 2; + pub const CREATE_NEW_COLLECTIONS_BIT: i32 = 1 << 3; + pub const EDIT_ANY_COLLECTION_BIT: i32 = 1 << 4; + pub const DELETE_ANY_COLLECTION_BIT: i32 = 1 << 5; + pub const MANAGE_GROUPS_BIT: i32 = 1 << 6; + pub const MANAGE_POLICIES_BIT: i32 = 1 << 7; + pub const MANAGE_SSO_BIT: i32 = 1 << 8; + pub const MANAGE_USERS_BIT: i32 = 1 << 9; + pub const MANAGE_RESET_PASSWORD_BIT: i32 = 1 << 10; + pub const MANAGE_SCIM_BIT: i32 = 1 << 11; + pub fn from_key(key: &str) -> Option { match key { "accessEventLogs" | "access_event_logs" => Some(Self::AccessEventLogs), @@ -113,6 +126,23 @@ impl OrganizationUserPermission { _ => None, } } + + pub fn bit(self) -> i32 { + match self { + Self::AccessEventLogs => Self::ACCESS_EVENT_LOGS_BIT, + Self::AccessImportExport => Self::ACCESS_IMPORT_EXPORT_BIT, + Self::AccessReports => Self::ACCESS_REPORTS_BIT, + Self::CreateNewCollections => Self::CREATE_NEW_COLLECTIONS_BIT, + Self::EditAnyCollection => Self::EDIT_ANY_COLLECTION_BIT, + Self::DeleteAnyCollection => Self::DELETE_ANY_COLLECTION_BIT, + Self::ManageGroups => Self::MANAGE_GROUPS_BIT, + Self::ManagePolicies => Self::MANAGE_POLICIES_BIT, + Self::ManageSso => Self::MANAGE_SSO_BIT, + Self::ManageUsers => Self::MANAGE_USERS_BIT, + Self::ManageResetPassword => Self::MANAGE_RESET_PASSWORD_BIT, + Self::ManageScim => Self::MANAGE_SCIM_BIT, + } + } } impl OrganizationUserPermissions { @@ -121,12 +151,62 @@ impl OrganizationUserPermissions { serde_json::from_value(value) } - pub fn from_db_json(raw: Option<&str>) -> serde_json::Result> { - raw.map(serde_json::from_str).transpose() + pub fn from_mask(mask: i32) -> Self { + Self { + access_event_logs: mask & OrganizationUserPermission::AccessEventLogs.bit() != 0, + access_import_export: mask & OrganizationUserPermission::AccessImportExport.bit() != 0, + access_reports: mask & OrganizationUserPermission::AccessReports.bit() != 0, + create_new_collections: mask & OrganizationUserPermission::CreateNewCollections.bit() != 0, + edit_any_collection: mask & OrganizationUserPermission::EditAnyCollection.bit() != 0, + delete_any_collection: mask & OrganizationUserPermission::DeleteAnyCollection.bit() != 0, + manage_groups: mask & OrganizationUserPermission::ManageGroups.bit() != 0, + manage_policies: mask & OrganizationUserPermission::ManagePolicies.bit() != 0, + manage_sso: mask & OrganizationUserPermission::ManageSso.bit() != 0, + manage_users: mask & OrganizationUserPermission::ManageUsers.bit() != 0, + manage_reset_password: mask & OrganizationUserPermission::ManageResetPassword.bit() != 0, + manage_scim: mask & OrganizationUserPermission::ManageScim.bit() != 0, + } } - pub fn to_db_json(&self) -> serde_json::Result { - serde_json::to_string(self) + pub fn to_mask(&self) -> i32 { + let mut mask = 0; + if self.access_event_logs { + mask |= OrganizationUserPermission::AccessEventLogs.bit(); + } + if self.access_import_export { + mask |= OrganizationUserPermission::AccessImportExport.bit(); + } + if self.access_reports { + mask |= OrganizationUserPermission::AccessReports.bit(); + } + if self.create_new_collections { + mask |= OrganizationUserPermission::CreateNewCollections.bit(); + } + if self.edit_any_collection { + mask |= OrganizationUserPermission::EditAnyCollection.bit(); + } + if self.delete_any_collection { + mask |= OrganizationUserPermission::DeleteAnyCollection.bit(); + } + if self.manage_groups { + mask |= OrganizationUserPermission::ManageGroups.bit(); + } + if self.manage_policies { + mask |= OrganizationUserPermission::ManagePolicies.bit(); + } + if self.manage_sso { + mask |= OrganizationUserPermission::ManageSso.bit(); + } + if self.manage_users { + mask |= OrganizationUserPermission::ManageUsers.bit(); + } + if self.manage_reset_password { + mask |= OrganizationUserPermission::ManageResetPassword.bit(); + } + if self.manage_scim { + mask |= OrganizationUserPermission::ManageScim.bit(); + } + mask } pub fn is_enabled(&self, permission: OrganizationUserPermission) -> bool { @@ -432,14 +512,7 @@ impl Membership { pub fn custom_permissions(&self) -> Option { let legacy_permissions = OrganizationUserPermissions::from_legacy_access_all(self.access_all); - match OrganizationUserPermissions::from_db_json(self.permissions.as_deref()) { - Ok(Some(permissions)) => Some(permissions), - Ok(None) => legacy_permissions, - Err(error) => { - warn!("Invalid custom permissions for membership {}: {error:#}", self.uuid); - legacy_permissions - } - } + self.permissions.map(OrganizationUserPermissions::from_mask).or(legacy_permissions) } pub fn has_permission(&self, permission: OrganizationUserPermission) -> bool { diff --git a/src/db/schema.rs b/src/db/schema.rs index dc407f62..87fa4111 100644 --- a/src/db/schema.rs +++ b/src/db/schema.rs @@ -242,7 +242,7 @@ table! { atype -> Integer, reset_password_key -> Nullable, external_id -> Nullable, - permissions -> Nullable, + permissions -> Nullable, } }