From 888902cf81f7ba4d94dea355ee61940dcae6d5e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Roumezin?= <94066559+RaphaelRoumezin@users.noreply.github.com> Date: Wed, 22 Jul 2026 13:19:30 +0200 Subject: [PATCH] fix: change db schema for lighter queries The main issue this is solving is that most of the endpoints where the settings need to be checked do not need the rest of the Organization structure. Also, depending on the DB backend, this might be a storage size improvement compared to storing 4 bools. --- .../down.sql | 5 +- .../up.sql | 5 +- .../down.sql | 5 +- .../up.sql | 5 +- .../down.sql | 5 +- .../up.sql | 5 +- src/api/core/organizations.rs | 17 +-- src/db/models/cipher.rs | 4 +- src/db/models/organization.rs | 125 +++++++++++++++--- src/db/schema.rs | 5 +- 10 files changed, 122 insertions(+), 59 deletions(-) diff --git a/migrations/mysql/2026-07-21-120000_add_collection_management/down.sql b/migrations/mysql/2026-07-21-120000_add_collection_management/down.sql index 300a4665..dbcd3e3f 100644 --- a/migrations/mysql/2026-07-21-120000_add_collection_management/down.sql +++ b/migrations/mysql/2026-07-21-120000_add_collection_management/down.sql @@ -1,4 +1 @@ -ALTER TABLE organizations DROP COLUMN allow_admin_access_to_all_collection_items; -ALTER TABLE organizations DROP COLUMN limit_collection_creation; -ALTER TABLE organizations DROP COLUMN limit_collection_deletion; -ALTER TABLE organizations DROP COLUMN limit_item_deletion; \ No newline at end of file +ALTER TABLE organizations DROP COLUMN collection_settings; \ No newline at end of file diff --git a/migrations/mysql/2026-07-21-120000_add_collection_management/up.sql b/migrations/mysql/2026-07-21-120000_add_collection_management/up.sql index 276752b8..002c45f0 100644 --- a/migrations/mysql/2026-07-21-120000_add_collection_management/up.sql +++ b/migrations/mysql/2026-07-21-120000_add_collection_management/up.sql @@ -1,4 +1 @@ -ALTER TABLE organizations ADD COLUMN allow_admin_access_to_all_collection_items BOOLEAN NOT NULL DEFAULT TRUE; -ALTER TABLE organizations ADD COLUMN limit_collection_creation BOOLEAN NOT NULL DEFAULT FALSE; -ALTER TABLE organizations ADD COLUMN limit_collection_deletion BOOLEAN NOT NULL DEFAULT FALSE; -ALTER TABLE organizations ADD COLUMN limit_item_deletion BOOLEAN NOT NULL DEFAULT FALSE; \ No newline at end of file +ALTER TABLE organizations ADD COLUMN collection_settings SMALLINT NOT NULL DEFAULT 1; \ No newline at end of file diff --git a/migrations/postgresql/2026-07-21-120000_add_collection_management/down.sql b/migrations/postgresql/2026-07-21-120000_add_collection_management/down.sql index 300a4665..dbcd3e3f 100644 --- a/migrations/postgresql/2026-07-21-120000_add_collection_management/down.sql +++ b/migrations/postgresql/2026-07-21-120000_add_collection_management/down.sql @@ -1,4 +1 @@ -ALTER TABLE organizations DROP COLUMN allow_admin_access_to_all_collection_items; -ALTER TABLE organizations DROP COLUMN limit_collection_creation; -ALTER TABLE organizations DROP COLUMN limit_collection_deletion; -ALTER TABLE organizations DROP COLUMN limit_item_deletion; \ No newline at end of file +ALTER TABLE organizations DROP COLUMN collection_settings; \ No newline at end of file diff --git a/migrations/postgresql/2026-07-21-120000_add_collection_management/up.sql b/migrations/postgresql/2026-07-21-120000_add_collection_management/up.sql index 276752b8..002c45f0 100644 --- a/migrations/postgresql/2026-07-21-120000_add_collection_management/up.sql +++ b/migrations/postgresql/2026-07-21-120000_add_collection_management/up.sql @@ -1,4 +1 @@ -ALTER TABLE organizations ADD COLUMN allow_admin_access_to_all_collection_items BOOLEAN NOT NULL DEFAULT TRUE; -ALTER TABLE organizations ADD COLUMN limit_collection_creation BOOLEAN NOT NULL DEFAULT FALSE; -ALTER TABLE organizations ADD COLUMN limit_collection_deletion BOOLEAN NOT NULL DEFAULT FALSE; -ALTER TABLE organizations ADD COLUMN limit_item_deletion BOOLEAN NOT NULL DEFAULT FALSE; \ No newline at end of file +ALTER TABLE organizations ADD COLUMN collection_settings SMALLINT NOT NULL DEFAULT 1; \ No newline at end of file diff --git a/migrations/sqlite/2026-07-21-120000_add_collection_management/down.sql b/migrations/sqlite/2026-07-21-120000_add_collection_management/down.sql index 300a4665..dbcd3e3f 100644 --- a/migrations/sqlite/2026-07-21-120000_add_collection_management/down.sql +++ b/migrations/sqlite/2026-07-21-120000_add_collection_management/down.sql @@ -1,4 +1 @@ -ALTER TABLE organizations DROP COLUMN allow_admin_access_to_all_collection_items; -ALTER TABLE organizations DROP COLUMN limit_collection_creation; -ALTER TABLE organizations DROP COLUMN limit_collection_deletion; -ALTER TABLE organizations DROP COLUMN limit_item_deletion; \ No newline at end of file +ALTER TABLE organizations DROP COLUMN collection_settings; \ No newline at end of file diff --git a/migrations/sqlite/2026-07-21-120000_add_collection_management/up.sql b/migrations/sqlite/2026-07-21-120000_add_collection_management/up.sql index 276752b8..002c45f0 100644 --- a/migrations/sqlite/2026-07-21-120000_add_collection_management/up.sql +++ b/migrations/sqlite/2026-07-21-120000_add_collection_management/up.sql @@ -1,4 +1 @@ -ALTER TABLE organizations ADD COLUMN allow_admin_access_to_all_collection_items BOOLEAN NOT NULL DEFAULT TRUE; -ALTER TABLE organizations ADD COLUMN limit_collection_creation BOOLEAN NOT NULL DEFAULT FALSE; -ALTER TABLE organizations ADD COLUMN limit_collection_deletion BOOLEAN NOT NULL DEFAULT FALSE; -ALTER TABLE organizations ADD COLUMN limit_item_deletion BOOLEAN NOT NULL DEFAULT FALSE; \ No newline at end of file +ALTER TABLE organizations ADD COLUMN collection_settings SMALLINT NOT NULL DEFAULT 1; \ No newline at end of file diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index 2cfbb3b4..f77cceb3 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -367,18 +367,11 @@ async fn put_organization_collection_management( err!("Organization not found") }; - if let Some(flag) = data.allow_admin_access_to_all_collection_items { - org.allow_admin_access_to_all_collection_items = flag; - }; - if let Some(flag) = data.limit_collection_creation { - org.limit_collection_creation = flag; - }; - if let Some(flag) = data.limit_collection_deletion { - org.limit_collection_deletion = flag; - }; - if let Some(flag) = data.limit_item_deletion { - org.limit_item_deletion = flag; - }; + org.collection_settings.allow_admin_access_to_all_collection_items = + data.allow_admin_access_to_all_collection_items.unwrap_or(false); + org.collection_settings.limit_collection_creation = data.limit_collection_creation.unwrap_or(false); + org.collection_settings.limit_collection_deletion = data.limit_collection_deletion.unwrap_or(false); + org.collection_settings.limit_item_deletion = data.limit_item_deletion.unwrap_or(false); org.save(&conn).await?; diff --git a/src/db/models/cipher.rs b/src/db/models/cipher.rs index f8e8545e..9f1dd782 100644 --- a/src/db/models/cipher.rs +++ b/src/db/models/cipher.rs @@ -727,8 +727,8 @@ impl Cipher { pub async fn is_deletable_by_user(&self, user_uuid: &UserId, conn: &DbConn) -> bool { let manage_required = match self.organization_uuid { - Some(ref org_id) => match Organization::find_by_uuid(&org_id, &conn).await { - Some(org) => org.limit_item_deletion, + Some(ref org_id) => match Organization::find_collection_settings_by_uuid(&org_id, &conn).await { + Some(settings) => settings.limit_item_deletion, None => false, }, None => false, diff --git a/src/db/models/organization.rs b/src/db/models/organization.rs index 56cdd340..6a85ade0 100644 --- a/src/db/models/organization.rs +++ b/src/db/models/organization.rs @@ -5,8 +5,15 @@ use std::{ use chrono::{NaiveDateTime, Utc}; use derive_more::{AsRef, Deref, Display, From}; -use diesel::prelude::*; +use diesel::{ + deserialize::FromSql, + prelude::*, + serialize::{IsNull, ToSql}, + sql_types::SmallInt, +}; use num_traits::FromPrimitive; +#[cfg(feature = "sqlite")] +use num_traits::ToPrimitive; use serde_json::Value; use crate::{ @@ -28,6 +35,77 @@ use super::{ OrgPolicyType, TwoFactor, User, UserId, }; +#[derive(Debug, Clone, Copy, FromSqlRow, AsExpression)] +#[diesel(sql_type = SmallInt)] +pub struct OrganizationCollectionSettings { + pub allow_admin_access_to_all_collection_items: bool, + pub limit_collection_creation: bool, + pub limit_collection_deletion: bool, + pub limit_item_deletion: bool, +} + +impl From<&OrganizationCollectionSettings> for i16 { + fn from(value: &OrganizationCollectionSettings) -> Self { + (0b1 * (value.allow_admin_access_to_all_collection_items as i16)) + | (0b10 * (value.limit_collection_creation as i16)) + | (0b100 * (value.limit_collection_deletion as i16)) + | (0b1000 * (value.limit_item_deletion as i16)) + } +} + +#[cfg(feature = "sqlite")] +use diesel::sqlite::Sqlite; +#[cfg(feature = "sqlite")] +impl ToSql for OrganizationCollectionSettings +where + i16: ToSql, +{ + fn to_sql<'b>(&'b self, out: &mut diesel::serialize::Output<'b, '_, Sqlite>) -> diesel::serialize::Result { + out.set_value(i16::from(self).to_i32()); + Ok(IsNull::No) + } +} + +#[cfg(feature = "postgresql")] +use diesel::pg::Pg; +#[cfg(feature = "postgresql")] +impl ToSql for OrganizationCollectionSettings +where + i16: ToSql, +{ + fn to_sql<'b>(&'b self, out: &mut diesel::serialize::Output<'b, '_, Pg>) -> diesel::serialize::Result { + >::to_sql(&i16::from(self), &mut out.reborrow()) + } +} + +#[cfg(feature = "mysql")] +use diesel::mysql::Mysql; +#[cfg(feature = "mysql")] +impl ToSql for OrganizationCollectionSettings +where + i16: ToSql, +{ + fn to_sql<'b>(&'b self, out: &mut diesel::serialize::Output<'b, '_, Mysql>) -> diesel::serialize::Result { + >::to_sql(&i16::from(self), &mut out.reborrow()) + } +} + +impl FromSql for OrganizationCollectionSettings +where + DB: diesel::backend::Backend, + i16: FromSql, +{ + fn from_sql(bytes: DB::RawValue<'_>) -> diesel::deserialize::Result { + let val = i16::from_sql(bytes)?; + Ok(Self { + allow_admin_access_to_all_collection_items: (val & 0b1) != 0, + limit_collection_creation: (val & 0b10) != 0, + limit_collection_deletion: (val & 0b100) != 0, + limit_item_deletion: (val & 0b1000) != 0, + }) + } +} + #[derive(Identifiable, Queryable, Insertable, AsChangeset)] #[diesel(table_name = organizations)] #[diesel(treat_none_as_null = true)] @@ -39,10 +117,7 @@ pub struct Organization { pub private_key: Option, pub public_key: Option, - pub allow_admin_access_to_all_collection_items: bool, - pub limit_collection_creation: bool, - pub limit_collection_deletion: bool, - pub limit_item_deletion: bool, + pub collection_settings: OrganizationCollectionSettings, } #[derive(Identifiable, Queryable, Insertable, AsChangeset)] @@ -198,10 +273,12 @@ impl Organization { billing_email, private_key, public_key, - allow_admin_access_to_all_collection_items: true, - limit_collection_creation: false, - limit_collection_deletion: false, - limit_item_deletion: false, + collection_settings: OrganizationCollectionSettings { + allow_admin_access_to_all_collection_items: true, + limit_collection_creation: false, + limit_collection_deletion: false, + limit_item_deletion: false, + }, } } // https://github.com/bitwarden/server/blob/9ebe16587175b1c0e9208f84397bb75d0d595510/src/Api/AdminConsole/Models/Response/Organizations/OrganizationResponseModel.cs @@ -228,10 +305,10 @@ impl Organization { "useApi": true, "hasPublicAndPrivateKeys": self.private_key.is_some() && self.public_key.is_some(), "useResetPassword": CONFIG.mail_enabled(), - "allowAdminAccessToAllCollectionItems": self.allow_admin_access_to_all_collection_items, - "limitCollectionCreation": self.limit_collection_creation, - "limitCollectionDeletion": self.limit_collection_deletion, - "limitItemDeletion": self.limit_item_deletion, + "allowAdminAccessToAllCollectionItems": self.collection_settings.allow_admin_access_to_all_collection_items, + "limitCollectionCreation": self.collection_settings.limit_collection_creation, + "limitCollectionDeletion": self.collection_settings.limit_collection_deletion, + "limitItemDeletion": self.collection_settings.limit_item_deletion, "businessName": self.name, "businessAddress1": null, @@ -406,6 +483,20 @@ impl Organization { conn.run(move |conn| organizations::table.filter(organizations::uuid.eq(uuid)).first::(conn).ok()).await } + pub async fn find_collection_settings_by_uuid( + uuid: &OrganizationId, + conn: &DbConn, + ) -> Option { + conn.run(move |conn| { + organizations::table + .filter(organizations::uuid.eq(uuid)) + .select(organizations::collection_settings) + .first::(conn) + .ok() + }) + .await + } + pub async fn find_by_name(name: &str, conn: &DbConn) -> Option { conn.run(move |conn| organizations::table.filter(organizations::name.eq(name)).first::(conn).ok()).await } @@ -520,10 +611,10 @@ impl Membership { "familySponsorshipToDelete": null, "accessSecretsManager": false, - "allowAdminAccessToAllCollectionItems": org.allow_admin_access_to_all_collection_items, - "limitCollectionCreation": org.limit_collection_creation, - "limitCollectionDeletion": org.limit_collection_deletion, - "limitItemDeletion": org.limit_item_deletion, + "allowAdminAccessToAllCollectionItems": org.collection_settings.allow_admin_access_to_all_collection_items, + "limitCollectionCreation": org.collection_settings.limit_collection_creation, + "limitCollectionDeletion": org.collection_settings.limit_collection_deletion, + "limitItemDeletion": org.collection_settings.limit_item_deletion, "userIsManagedByOrganization": false, // Means not managed via the Members UI, like SSO "userIsClaimedByOrganization": false, // The new key instead of the obsolete userIsManagedByOrganization diff --git a/src/db/schema.rs b/src/db/schema.rs index c146b23d..17332cd4 100644 --- a/src/db/schema.rs +++ b/src/db/schema.rs @@ -126,10 +126,7 @@ table! { billing_email -> Text, private_key -> Nullable, public_key -> Nullable, - allow_admin_access_to_all_collection_items -> Bool, - limit_collection_creation -> Bool, - limit_collection_deletion -> Bool, - limit_item_deletion -> Bool, + collection_settings -> SmallInt, } }