diff --git a/src/db/mod.rs b/src/db/mod.rs index f5177c4e..622235a2 100644 --- a/src/db/mod.rs +++ b/src/db/mod.rs @@ -472,6 +472,26 @@ const CUSTOM_ROLE_REPAIR_MIGRATION: &str = "20260723120000"; const CUSTOM_COLLECTION_PERMISSIONS_MIGRATION: &str = "20260716120000"; const DROP_MEMBERSHIP_ACCESS_ALL_MIGRATION: &str = "20260724120000"; const CUSTOM_ROLE_SAME_RUN_MARKER_TABLE: &str = "__vw_custom_role_same_run_0716"; +const LEGACY_USER_ACCESS_ALL_RECOVERY_SQL: &str = concat!( + "\n\nReview every affected membership with this SQLite/MySQL/PostgreSQL-compatible query:\n", + "SELECT uuid, user_uuid, org_uuid, status\n", + "FROM users_organizations\n", + "WHERE atype = 2 AND access_all = TRUE;\n\n", + "After an organization owner has decided the intended outcome, replace and run exactly one ", + "guarded statement for that membership while every Vaultwarden instance is stopped. Do not bulk-promote these ", + "records.\n\n", + "Keep the User role and revoke organization-wide vault access:\n", + "UPDATE users_organizations\n", + "SET access_all = FALSE\n", + "WHERE uuid = '' AND atype = 2 AND access_all = TRUE;\n\n", + "Preserve organization-wide vault access by intentionally granting Custom Create/Edit/Delete-any collection ", + "authority:\n", + "UPDATE users_organizations\n", + "SET atype = 3\n", + "WHERE uuid = '' AND atype = 2 AND access_all = TRUE;\n\n", + "The second statement deliberately adds collection-management authority: the repair migration copies the retained ", + "access_all value to all three collection permissions before converting legacy role 3 to Custom role 4." +); #[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] #[expect( @@ -585,10 +605,15 @@ fn custom_role_preflight_error(decision: CustomRolePreflightDecision, facts: Cus unreachable!("successful preflight decisions do not produce errors") } }; + let recovery = if decision == CustomRolePreflightDecision::RefuseLegacyUserAccessAll { + LEGACY_USER_ACCESS_ALL_RECOVERY_SQL + } else { + "" + }; std::io::Error::other(format!( "Custom-role migration preflight stopped startup: {detail} Back up the database and resolve \ - the legacy membership state manually before restarting." + the legacy membership state manually before restarting.{recovery}" )) .into() } @@ -1122,9 +1147,11 @@ mod postgresql_migrations { #[cfg(test)] mod custom_role_migration_preflight_tests { + use std::error::Error as _; + use super::{ CustomRoleMigrationFacts as Facts, CustomRolePreflightDecision as Decision, custom_role_preflight_decision, - mysql_partial_unexpected_values_query, + custom_role_preflight_error, mysql_partial_unexpected_values_query, }; fn pending_repair() -> Facts { @@ -1191,16 +1218,33 @@ mod custom_role_migration_preflight_tests { #[test] fn legacy_user_access_all_requires_an_operator_decision() { - assert_eq!( - custom_role_preflight_decision( - Facts { - legacy_user_access_all_count: 1, - ..pending_repair() - }, - false, - ), - Decision::RefuseLegacyUserAccessAll - ); + let facts = Facts { + legacy_user_access_all_count: 1, + ..pending_repair() + }; + let decision = custom_role_preflight_decision(facts, false); + assert_eq!(decision, Decision::RefuseLegacyUserAccessAll); + + let error = custom_role_preflight_error(decision, facts); + let message = error.source().expect("preflight error should retain its I/O error source").to_string(); + assert!(message.contains("1 legacy User membership(s)")); + assert!(message.contains( + "SELECT uuid, user_uuid, org_uuid, status\n\ + FROM users_organizations\n\ + WHERE atype = 2 AND access_all = TRUE;" + )); + assert!(message.contains( + "SET access_all = FALSE\n\ + WHERE uuid = '' AND atype = 2 AND access_all = TRUE;" + )); + assert!(message.contains( + "SET atype = 3\n\ + WHERE uuid = '' AND atype = 2 AND access_all = TRUE;" + )); + assert!(message.contains("run exactly one guarded statement")); + assert!(message.contains("Do not bulk-promote")); + assert!(message.contains("converting legacy role 3 to Custom role 4")); + assert!(!message.contains("SET atype = 4")); } #[test] diff --git a/src/db/models/cipher.rs b/src/db/models/cipher.rs index ad27f3c3..2c58ed66 100644 --- a/src/db/models/cipher.rs +++ b/src/db/models/cipher.rs @@ -26,6 +26,7 @@ use macros::UuidFromParam; use super::{ Archive, Attachment, CollectionCipher, CollectionId, Favorite, FolderCipher, FolderId, Group, Membership, MembershipStatus, MembershipType, OrganizationId, User, UserId, + organization::custom_membership_with_edit_any_collection, }; #[derive(Identifiable, Queryable, Insertable, AsChangeset)] @@ -891,8 +892,7 @@ impl Cipher { .filter(ciphers::user_uuid.eq(user_uuid)) // Cipher owner // Edit any collection (Custom) or org admin/owner — the successor of access_all .or_filter( - users_organizations::edit_any_collection - .eq(true) + custom_membership_with_edit_any_collection() .or(users_organizations::atype.le(MembershipType::Admin as i32)), ) .or_filter(users_collections::user_uuid.eq(user_uuid)) // Access to collection @@ -933,8 +933,7 @@ impl Cipher { .filter(ciphers::user_uuid.eq(user_uuid)) // Cipher owner // Edit any collection (Custom) or org admin/owner — the successor of access_all .or_filter( - users_organizations::edit_any_collection - .eq(true) + custom_membership_with_edit_any_collection() .or(users_organizations::atype.le(MembershipType::Admin as i32)), ) .or_filter(users_collections::user_uuid.eq(user_uuid)) // Access to collection @@ -1059,8 +1058,7 @@ impl Cipher { .and(collections_groups::groups_uuid.eq(groups::uuid))), ) .filter( - users_organizations::edit_any_collection - .eq(true) // Custom "Edit any collection" (successor of access_all) + custom_membership_with_edit_any_collection() // Custom "Edit any collection" (successor of access_all) .or(users_organizations::atype.le(MembershipType::Admin as i32)) // or org admin/owner .or(users_collections::user_uuid .eq(user_uuid) // User has access to collection @@ -1091,8 +1089,7 @@ impl Cipher { .and(users_collections::user_uuid.eq(user_uuid.clone()))), ) .filter( - users_organizations::edit_any_collection - .eq(true) // Custom "Edit any collection" (successor of access_all) + custom_membership_with_edit_any_collection() // Custom "Edit any collection" (successor of access_all) .or(users_organizations::atype.le(MembershipType::Admin as i32)) // or org admin/owner .or(users_collections::user_uuid .eq(user_uuid) // User has access to collection @@ -1136,8 +1133,7 @@ impl Cipher { .and(collections_groups::groups_uuid.eq(groups::uuid))), ) .filter( - users_organizations::edit_any_collection - .eq(true) // Custom "Edit any collection" (successor of access_all) + custom_membership_with_edit_any_collection() // Custom "Edit any collection" (successor of access_all) .or(users_organizations::atype.le(MembershipType::Admin as i32)) // or org admin/owner .or(users_collections::user_uuid .eq(user_uuid) // User has access to collection @@ -1169,8 +1165,7 @@ impl Cipher { .and(users_collections::user_uuid.eq(user_uuid.clone()))), ) .filter( - users_organizations::edit_any_collection - .eq(true) // Custom "Edit any collection" (successor of access_all) + custom_membership_with_edit_any_collection() // Custom "Edit any collection" (successor of access_all) .or(users_organizations::atype.le(MembershipType::Admin as i32)) // or org admin/owner .or(users_collections::user_uuid .eq(user_uuid) // User has access to collection @@ -1216,7 +1211,7 @@ impl Cipher { .and(collections_groups::groups_uuid.eq(groups::uuid))), ) .or_filter(users_collections::user_uuid.eq(user_uuid)) // User has access to collection - .or_filter(users_organizations::edit_any_collection.eq(true)) // Custom "Edit any collection" (successor of access_all) + .or_filter(custom_membership_with_edit_any_collection()) // Custom "Edit any collection" (successor of access_all) .or_filter(users_organizations::atype.le(MembershipType::Admin as i32)) // User is admin or owner .or_filter(groups::access_all.eq(true)) //Access via group .or_filter(collections_groups::collections_uuid.is_not_null()) //Access via group diff --git a/src/db/models/collection.rs b/src/db/models/collection.rs index 17dfa090..cdea34d6 100644 --- a/src/db/models/collection.rs +++ b/src/db/models/collection.rs @@ -19,7 +19,7 @@ use macros::UuidFromParam; use super::{ CipherId, CollectionGroup, GroupUser, Membership, MembershipId, MembershipStatus, MembershipType, OrganizationId, - User, UserId, + User, UserId, organization::custom_membership_with_edit_any_collection, }; // See (v2026.7.0): https://github.com/bitwarden/server/blob/5d4461aa42cadbacfef8fe2166c5453a5c52773a/src/Core/AdminConsole/Entities/Collection.cs @@ -265,8 +265,7 @@ impl Collection { .or( // Full-access member: Custom "Edit any collection" or org admin/owner // (successor of the removed membership access_all) - users_organizations::edit_any_collection - .eq(true) + custom_membership_with_edit_any_collection() .or(users_organizations::atype.le(MembershipType::Admin as i32)), ) .or( @@ -303,8 +302,7 @@ impl Collection { users_collections::user_uuid.eq(user_uuid).or( // Full-access member: Custom "Edit any collection" or org admin/owner // (successor of the removed membership access_all) - users_organizations::edit_any_collection - .eq(true) + custom_membership_with_edit_any_collection() .or(users_organizations::atype.le(MembershipType::Admin as i32)), ), ) @@ -391,7 +389,7 @@ impl Collection { .eq(uuid) .or( // Directly accessed collection - users_organizations::edit_any_collection.eq(true).or( + custom_membership_with_edit_any_collection().or( // Custom "Edit any collection" or org admin/owner (successor of access_all) users_organizations::atype.le(MembershipType::Admin as i32), // Org admin or owner ), @@ -427,7 +425,7 @@ impl Collection { .filter(collections::uuid.eq(uuid)) .filter(users_collections::collection_uuid.eq(uuid).or( // Directly accessed collection - users_organizations::edit_any_collection.eq(true).or( + custom_membership_with_edit_any_collection().or( // Custom "Edit any collection" or org admin/owner (successor of access_all) users_organizations::atype.le(MembershipType::Admin as i32), // Org admin or owner ), @@ -472,7 +470,7 @@ impl Collection { .filter( users_organizations::atype .le(MembershipType::Admin as i32) // Org admin or owner - .or(users_organizations::edit_any_collection.eq(true)) // Custom "Edit any collection" (successor of access_all) + .or(custom_membership_with_edit_any_collection()) // Custom "Edit any collection" (successor of access_all) .or(users_collections::collection_uuid .eq(&self.uuid) // write access given to collection .and(users_collections::read_only.eq(false))) @@ -505,7 +503,7 @@ impl Collection { .filter( users_organizations::atype .le(MembershipType::Admin as i32) // Org admin or owner - .or(users_organizations::edit_any_collection.eq(true)) // Custom "Edit any collection" (successor of access_all) + .or(custom_membership_with_edit_any_collection()) // Custom "Edit any collection" (successor of access_all) .or(users_collections::collection_uuid .eq(&self.uuid) // write access given to collection .and(users_collections::read_only.eq(false))), @@ -552,7 +550,7 @@ impl Collection { .and(users_collections::hide_passwords.eq(true)) .or( // Directly accessed collection - users_organizations::edit_any_collection.eq(true).or( + custom_membership_with_edit_any_collection().or( // Custom "Edit any collection" or org admin/owner (successor of access_all) users_organizations::atype.le(MembershipType::Admin as i32), // Org admin or owner ), diff --git a/src/db/models/organization.rs b/src/db/models/organization.rs index 6922c56e..4e63b2bc 100644 --- a/src/db/models/organization.rs +++ b/src/db/models/organization.rs @@ -68,6 +68,17 @@ pub struct Membership { pub access_reports: bool, } +/// Diesel equivalent of [`Membership::has_edit_any_collection`]. +/// +/// Keep the role check in this shared predicate so a stale flag on any non-Custom membership +/// remains inert in every collection-access query. +pub(super) fn custom_membership_with_edit_any_collection() -> diesel::dsl::And< + diesel::dsl::Eq, + diesel::dsl::Eq, +> { + users_organizations::atype.eq(MembershipType::Custom as i32).and(users_organizations::edit_any_collection.eq(true)) +} + #[derive(Identifiable, Queryable, Insertable, AsChangeset)] #[diesel(table_name = organization_api_key)] #[diesel(primary_key(uuid, org_uuid))] @@ -1112,9 +1123,7 @@ impl Membership { .filter( users_organizations::atype .eq_any(vec![MembershipType::Owner as i32, MembershipType::Admin as i32]) - .or(users_organizations::atype - .eq(MembershipType::Custom as i32) - .and(users_organizations::edit_any_collection.eq(true))), + .or(custom_membership_with_edit_any_collection()), ) .load::(conn) .unwrap_or_default() @@ -1239,8 +1248,7 @@ impl Membership { .and(ciphers_collections::cipher_uuid.eq(&cipher_uuid))), ) .filter( - users_organizations::edit_any_collection - .eq(true) // Custom "Edit any collection" (successor of access_all) + custom_membership_with_edit_any_collection() // Custom "Edit any collection" (successor of access_all) .or(users_organizations::atype.le(MembershipType::Admin as i32)) // or org admin/owner .or(ciphers_collections::cipher_uuid.eq(&cipher_uuid)), // ..or access to collection with cipher ) @@ -1317,8 +1325,7 @@ impl Membership { .filter(users_organizations::org_uuid.eq(org_uuid)) .left_join(users_collections::table.on(users_collections::user_uuid.eq(users_organizations::user_uuid))) .filter( - users_organizations::edit_any_collection - .eq(true) // Custom "Edit any collection" (successor of access_all) + custom_membership_with_edit_any_collection() // Custom "Edit any collection" (successor of access_all) .or(users_organizations::atype.le(MembershipType::Admin as i32)) // or org admin/owner .or(users_collections::collection_uuid.eq(&collection_uuid)), // ..or access to collection ) @@ -1509,6 +1516,37 @@ mod tests { assert!(!member.has_full_access()); } + #[cfg(sqlite)] + #[test] + fn diesel_edit_any_collection_predicate_is_custom_type_gated() { + use diesel::{Connection, connection::SimpleConnection, sqlite::SqliteConnection}; + + let mut conn = SqliteConnection::establish(":memory:").unwrap(); + conn.batch_execute( + "CREATE TABLE users_organizations ( + atype INTEGER NOT NULL, + edit_any_collection BOOLEAN NOT NULL + ); + INSERT INTO users_organizations (atype, edit_any_collection) VALUES + (0, TRUE), + (1, TRUE), + (2, TRUE), + (3, TRUE), + (4, FALSE), + (4, TRUE), + (5, TRUE);", + ) + .unwrap(); + + let matching_types = users_organizations::table + .select(users_organizations::atype) + .filter(custom_membership_with_edit_any_collection()) + .load::(&mut conn) + .unwrap(); + + assert_eq!(matching_types, vec![MembershipType::Custom as i32]); + } + #[test] fn edit_any_collection_does_not_imply_create_or_delete() { let mut custom = membership(MembershipType::Custom);