Browse Source

Address custom-role review findings

- Migrate legacy `access_all` Users losslessly into per-collection
  assignments instead of refusing to start the server.
- Keep provisioning of Admin/Owner memberships Owner-only (invite,
  confirm, delete); state changes stay as they were.
- Drop the new organization-import gate; the import stays authorized per
  target collection, and now requires a confirmed membership.
- Scope `accessReports` to the caller's own collections instead of
  handing out every cipher in the organization.
- Let `createNewCollections` read the collection list it needs.
- Reject group changes a caller may not make instead of silently
  dropping them.
- Detect partially applied permission-column migrations on MySQL.
- Drop the migration bookkeeping table and remove dead code.
pull/7397/head
tom27052006 1 week ago
parent
commit
f61e4fe727
  1. 40
      migrations/mysql/2026-07-23-120000_reconcile_legacy_custom_roles/up.sql
  2. 11
      migrations/mysql/2026-07-24-120000_drop_membership_access_all/down.sql
  3. 8
      migrations/mysql/2026-07-24-140000_guard_custom_role_downgrade/up.sql
  4. 41
      migrations/postgresql/2026-07-23-120000_reconcile_legacy_custom_roles/up.sql
  5. 11
      migrations/postgresql/2026-07-24-120000_drop_membership_access_all/down.sql
  6. 8
      migrations/postgresql/2026-07-24-140000_guard_custom_role_downgrade/up.sql
  7. 40
      migrations/sqlite/2026-07-23-120000_reconcile_legacy_custom_roles/up.sql
  8. 11
      migrations/sqlite/2026-07-24-120000_drop_membership_access_all/down.sql
  9. 8
      migrations/sqlite/2026-07-24-140000_guard_custom_role_downgrade/up.sql
  10. 252
      src/api/core/organizations.rs
  11. 368
      src/db/mod.rs
  12. 23
      src/db/models/organization.rs

40
migrations/mysql/2026-07-23-120000_reconcile_legacy_custom_roles/up.sql

@ -1,15 +1,33 @@
-- A normal User with the historical membership-level access_all bit cannot be mapped to the
-- Custom role without adding collection-management authority. Stop before dropping the source bit.
CREATE TEMPORARY TABLE __vw_legacy_user_access_all_guard (
blocked INTEGER NOT NULL PRIMARY KEY
-- A normal User with the historical membership-level access_all bit reached every collection of the
-- organization with full read/write, but held no collection-management authority. Mapping that onto
-- the Custom role would add authority, clearing the bit would remove existing access — so instead,
-- materialize the reach as explicit per-collection assignments while the source bit still exists.
-- `manage` stays FALSE, so no management authority is invented. This is the same approach Bitwarden
-- took when it retired `accessAll`; the one behavioral difference is that the access is no longer
-- dynamic, i.e. collections created later are not added automatically.
--
-- Step 1: a pre-existing assignment was overridden by access_all (full read/write regardless of
-- read_only/hide_passwords), so relax it to match what the member actually had.
UPDATE users_collections
SET read_only = FALSE,
hide_passwords = FALSE
WHERE EXISTS (
SELECT 1
FROM users_organizations AS uo
INNER JOIN collections AS c ON c.org_uuid = uo.org_uuid
WHERE uo.atype = 2
AND uo.access_all = TRUE
AND uo.user_uuid = users_collections.user_uuid
AND c.uuid = users_collections.collection_uuid
);
INSERT INTO __vw_legacy_user_access_all_guard (blocked) VALUES (1);
INSERT INTO __vw_legacy_user_access_all_guard (blocked)
SELECT 1
FROM users_organizations
WHERE atype = 2 AND access_all = TRUE
LIMIT 1;
DROP TEMPORARY TABLE __vw_legacy_user_access_all_guard;
-- Step 2: add the assignments that did not exist yet. Existing rows are left to step 1.
INSERT IGNORE INTO users_collections (user_uuid, collection_uuid, read_only, hide_passwords, manage)
SELECT uo.user_uuid, c.uuid, FALSE, FALSE, FALSE
FROM users_organizations AS uo
INNER JOIN collections AS c ON c.org_uuid = uo.org_uuid
WHERE uo.atype = 2
AND uo.access_all = TRUE;
-- The current 2026-07-16 migration copied a legacy full-access group's dynamic authority to the
-- exact direct 0/1/1 pattern. While the same organization-local source group is still present,

11
migrations/mysql/2026-07-24-120000_drop_membership_access_all/down.sql

@ -1,6 +1,13 @@
-- Recreate the column and repopulate it from the role/permission model that replaced it, restoring
-- the invariant older server versions rely on: access_all == access to every collection. That is
-- exactly Owners/Admins, plus Custom members holding `edit_any_collection`.
-- the invariant the immediately preceding schema relies on: access_all == access to every collection.
-- That is exactly Owners/Admins, plus Custom members holding `edit_any_collection`.
--
-- NOTE: this only holds for reverting *this* migration. Reverting further down the chain,
-- 2026-07-16 deliberately recomputes access_all as (create AND edit AND delete) for Custom members,
-- because in that older schema access_all also meant the legacy Manager "Manage all collections"
-- authority -- so a member who only held `edit_any_collection` comes out as a Manager *without*
-- access_all rather than silently gaining collection deletion. That is intentional and fail-closed;
-- the full rollback is blocked by 2026-07-24-140000/down.sql anyway.
ALTER TABLE users_organizations ADD COLUMN access_all BOOLEAN NOT NULL DEFAULT FALSE;
UPDATE users_organizations SET access_all = TRUE WHERE atype IN (0, 1);
UPDATE users_organizations SET access_all = TRUE WHERE atype = 4 AND edit_any_collection = TRUE;

8
migrations/mysql/2026-07-24-140000_guard_custom_role_downgrade/up.sql

@ -1,3 +1,7 @@
-- Forward migration marker. Its down migration intentionally blocks an automatic lossy downgrade
-- Forward migration marker: its down migration intentionally blocks an automatic lossy downgrade
-- before any granular permission column is removed.
SELECT 1;
--
-- It also cleans up after 2026-07-15: the same-run bookkeeping table has served its purpose by now
-- (2026-07-23 consumed the marker), so it is not left behind in every database. A single DDL
-- statement is safe even on MySQL, where DDL commits implicitly -- re-running it is a no-op.
DROP TABLE IF EXISTS __vw_custom_role_same_run_0716;

41
migrations/postgresql/2026-07-23-120000_reconcile_legacy_custom_roles/up.sql

@ -1,15 +1,34 @@
-- A normal User with the historical membership-level access_all bit cannot be mapped to the
-- Custom role without adding collection-management authority. Stop before dropping the source bit.
CREATE TEMPORARY TABLE __vw_legacy_user_access_all_guard (
blocked INTEGER NOT NULL PRIMARY KEY
-- A normal User with the historical membership-level access_all bit reached every collection of the
-- organization with full read/write, but held no collection-management authority. Mapping that onto
-- the Custom role would add authority, clearing the bit would remove existing access — so instead,
-- materialize the reach as explicit per-collection assignments while the source bit still exists.
-- `manage` stays FALSE, so no management authority is invented. This is the same approach Bitwarden
-- took when it retired `accessAll`; the one behavioral difference is that the access is no longer
-- dynamic, i.e. collections created later are not added automatically.
--
-- Step 1: a pre-existing assignment was overridden by access_all (full read/write regardless of
-- read_only/hide_passwords), so relax it to match what the member actually had.
UPDATE users_collections
SET read_only = FALSE,
hide_passwords = FALSE
WHERE EXISTS (
SELECT 1
FROM users_organizations AS uo
INNER JOIN collections AS c ON c.org_uuid = uo.org_uuid
WHERE uo.atype = 2
AND uo.access_all = TRUE
AND uo.user_uuid = users_collections.user_uuid
AND c.uuid = users_collections.collection_uuid
);
INSERT INTO __vw_legacy_user_access_all_guard (blocked) VALUES (1);
INSERT INTO __vw_legacy_user_access_all_guard (blocked)
SELECT 1
FROM users_organizations
WHERE atype = 2 AND access_all = TRUE
LIMIT 1;
DROP TABLE __vw_legacy_user_access_all_guard;
-- Step 2: add the assignments that did not exist yet. Existing rows are left to step 1.
INSERT INTO users_collections (user_uuid, collection_uuid, read_only, hide_passwords, manage)
SELECT uo.user_uuid, c.uuid, FALSE, FALSE, FALSE
FROM users_organizations AS uo
INNER JOIN collections AS c ON c.org_uuid = uo.org_uuid
WHERE uo.atype = 2
AND uo.access_all = TRUE
ON CONFLICT (user_uuid, collection_uuid) DO NOTHING;
-- The current 2026-07-16 migration copied a legacy full-access group's dynamic authority to the
-- exact direct 0/1/1 pattern. While the same organization-local source group is still present,

11
migrations/postgresql/2026-07-24-120000_drop_membership_access_all/down.sql

@ -1,6 +1,13 @@
-- Recreate the column and repopulate it from the role/permission model that replaced it, restoring
-- the invariant older server versions rely on: access_all == access to every collection. That is
-- exactly Owners/Admins, plus Custom members holding `edit_any_collection`.
-- the invariant the immediately preceding schema relies on: access_all == access to every collection.
-- That is exactly Owners/Admins, plus Custom members holding `edit_any_collection`.
--
-- NOTE: this only holds for reverting *this* migration. Reverting further down the chain,
-- 2026-07-16 deliberately recomputes access_all as (create AND edit AND delete) for Custom members,
-- because in that older schema access_all also meant the legacy Manager "Manage all collections"
-- authority -- so a member who only held `edit_any_collection` comes out as a Manager *without*
-- access_all rather than silently gaining collection deletion. That is intentional and fail-closed;
-- the full rollback is blocked by 2026-07-24-140000/down.sql anyway.
ALTER TABLE users_organizations ADD COLUMN access_all BOOLEAN NOT NULL DEFAULT FALSE;
UPDATE users_organizations SET access_all = TRUE WHERE atype IN (0, 1);
UPDATE users_organizations SET access_all = TRUE WHERE atype = 4 AND edit_any_collection = TRUE;

8
migrations/postgresql/2026-07-24-140000_guard_custom_role_downgrade/up.sql

@ -1,3 +1,7 @@
-- Forward migration marker. Its down migration intentionally blocks an automatic lossy downgrade
-- Forward migration marker: its down migration intentionally blocks an automatic lossy downgrade
-- before any granular permission column is removed.
SELECT 1;
--
-- It also cleans up after 2026-07-15: the same-run bookkeeping table has served its purpose by now
-- (2026-07-23 consumed the marker), so it is not left behind in every database. A single DDL
-- statement is safe even on MySQL, where DDL commits implicitly -- re-running it is a no-op.
DROP TABLE IF EXISTS __vw_custom_role_same_run_0716;

40
migrations/sqlite/2026-07-23-120000_reconcile_legacy_custom_roles/up.sql

@ -1,15 +1,33 @@
-- A normal User with the historical membership-level access_all bit cannot be mapped to the
-- Custom role without adding collection-management authority. Stop before dropping the source bit.
CREATE TEMPORARY TABLE __vw_legacy_user_access_all_guard (
blocked INTEGER NOT NULL PRIMARY KEY
-- A normal User with the historical membership-level access_all bit reached every collection of the
-- organization with full read/write, but held no collection-management authority. Mapping that onto
-- the Custom role would add authority, clearing the bit would remove existing access — so instead,
-- materialize the reach as explicit per-collection assignments while the source bit still exists.
-- `manage` stays FALSE, so no management authority is invented. This is the same approach Bitwarden
-- took when it retired `accessAll`; the one behavioral difference is that the access is no longer
-- dynamic, i.e. collections created later are not added automatically.
--
-- Step 1: a pre-existing assignment was overridden by access_all (full read/write regardless of
-- read_only/hide_passwords), so relax it to match what the member actually had.
UPDATE users_collections
SET read_only = FALSE,
hide_passwords = FALSE
WHERE EXISTS (
SELECT 1
FROM users_organizations AS uo
INNER JOIN collections AS c ON c.org_uuid = uo.org_uuid
WHERE uo.atype = 2
AND uo.access_all = TRUE
AND uo.user_uuid = users_collections.user_uuid
AND c.uuid = users_collections.collection_uuid
);
INSERT INTO __vw_legacy_user_access_all_guard (blocked) VALUES (1);
INSERT INTO __vw_legacy_user_access_all_guard (blocked)
SELECT 1
FROM users_organizations
WHERE atype = 2 AND access_all = TRUE
LIMIT 1;
DROP TABLE __vw_legacy_user_access_all_guard;
-- Step 2: add the assignments that did not exist yet. Existing rows are left to step 1.
INSERT OR IGNORE INTO users_collections (user_uuid, collection_uuid, read_only, hide_passwords, manage)
SELECT uo.user_uuid, c.uuid, FALSE, FALSE, FALSE
FROM users_organizations AS uo
INNER JOIN collections AS c ON c.org_uuid = uo.org_uuid
WHERE uo.atype = 2
AND uo.access_all = TRUE;
-- The current 2026-07-16 migration copied a legacy full-access group's dynamic authority to the
-- exact direct 0/1/1 pattern. While the same organization-local source group is still present,

11
migrations/sqlite/2026-07-24-120000_drop_membership_access_all/down.sql

@ -1,6 +1,13 @@
-- Recreate the column and repopulate it from the role/permission model that replaced it, restoring
-- the invariant older server versions rely on: access_all == access to every collection. That is
-- exactly Owners/Admins, plus Custom members holding `edit_any_collection`.
-- the invariant the immediately preceding schema relies on: access_all == access to every collection.
-- That is exactly Owners/Admins, plus Custom members holding `edit_any_collection`.
--
-- NOTE: this only holds for reverting *this* migration. Reverting further down the chain,
-- 2026-07-16 deliberately recomputes access_all as (create AND edit AND delete) for Custom members,
-- because in that older schema access_all also meant the legacy Manager "Manage all collections"
-- authority -- so a member who only held `edit_any_collection` comes out as a Manager *without*
-- access_all rather than silently gaining collection deletion. That is intentional and fail-closed;
-- the full rollback is blocked by 2026-07-24-140000/down.sql anyway.
ALTER TABLE users_organizations ADD COLUMN access_all BOOLEAN NOT NULL DEFAULT FALSE;
UPDATE users_organizations SET access_all = TRUE WHERE atype IN (0, 1);
UPDATE users_organizations SET access_all = TRUE WHERE atype = 4 AND edit_any_collection = TRUE;

8
migrations/sqlite/2026-07-24-140000_guard_custom_role_downgrade/up.sql

@ -1,3 +1,7 @@
-- Forward migration marker. Its down migration intentionally blocks an automatic lossy downgrade
-- Forward migration marker: its down migration intentionally blocks an automatic lossy downgrade
-- before any granular permission column is removed.
SELECT 1;
--
-- It also cleans up after 2026-07-15: the same-run bookkeeping table has served its purpose by now
-- (2026-07-23 consumed the marker), so it is not left behind in every database. A single DDL
-- statement is safe even on MySQL, where DDL commits implicitly -- re-running it is a no-op.
DROP TABLE IF EXISTS __vw_custom_role_same_run_0716;

252
src/api/core/organizations.rs

@ -400,7 +400,10 @@ async fn get_org_collections(org_id: OrganizationId, headers: ManagerHeadersLoos
let can_read_collection_list = headers.membership.has_full_access()
|| headers.membership.has_manage_users()
|| headers.membership.has_manage_groups()
|| headers.membership.has_delete_any_collection();
|| headers.membership.has_delete_any_collection()
// Create new collections needs the list too: the client resolves the parent of a nested
// collection against it and refreshes it after a create.
|| headers.membership.has_create_new_collections();
if !can_read_collection_list {
err_code!("Resource not found.", "User does not have full access", rocket::http::Status::NotFound.code);
}
@ -437,8 +440,10 @@ async fn get_org_collections_details(org_id: OrganizationId, headers: ManagerHea
// (metadata only) so the web client can render member/group collection assignments
// without crashing on collections it can't otherwise see. This exposes names/ids
// only, never cipher contents. manage_policies does not need the collection list.
let can_read_collection_list =
member.has_manage_users() || member.has_manage_groups() || member.has_delete_any_collection();
let can_read_collection_list = member.has_manage_users()
|| member.has_manage_groups()
|| member.has_delete_any_collection()
|| member.has_create_new_collections();
// Delete any collection can reveal collection access metadata, matching Bitwarden's
// ReadAllWithAccess behavior, but still does not grant cipher access. Manage Users/Groups
// retain the narrower metadata-only view introduced by the base PR.
@ -1047,64 +1052,78 @@ async fn get_assigned_org_details(data: OrgIdData, headers: Headers, conn: DbCon
);
}
let ciphers = filter_ciphers_for_organization(
Cipher::find_by_user_visible(&headers.user.uuid, &conn).await,
&data.organization_id,
);
let cipher_sync_data = CipherSyncData::new(&headers.user.uuid, CipherSyncType::User, &conn).await;
Ok(Json(json!({
"data": assigned_org_ciphers_json(&data.organization_id, &headers.host, &headers.user.uuid, &conn).await?,
"object": "list",
"continuationToken": null,
})))
}
// Serialize exactly the organization ciphers the user is actually assigned to, directly or via a
// group. `CipherSyncType::User` keeps the per-cipher access restrictions in place, so nothing outside
// the caller's own collections is returned and every cipher carries its real `edit`/`viewPassword`
// flags.
//
// NOTE: as everywhere else in Vaultwarden (and in Bitwarden), a `hidePasswords` assignment is
// reported to the client as `viewPassword: false` rather than redacted server-side. This therefore
// returns exactly what the same member already receives from `/api/sync` — never more.
async fn assigned_org_ciphers_json(
org_id: &OrganizationId,
host: &str,
user_id: &UserId,
conn: &DbConn,
) -> Result<Value, crate::Error> {
let ciphers = filter_ciphers_for_organization(Cipher::find_by_user_visible(user_id, conn).await, org_id);
let cipher_sync_data = CipherSyncData::new(user_id, CipherSyncType::User, conn).await;
let mut ciphers_json = Vec::with_capacity(ciphers.len());
for cipher in ciphers {
ciphers_json.push(
cipher
.to_json(&headers.host, &headers.user.uuid, Some(&cipher_sync_data), CipherSyncType::User, &conn)
.await?,
);
ciphers_json.push(cipher.to_json(host, user_id, Some(&cipher_sync_data), CipherSyncType::User, conn).await?);
}
Ok(Json(json!({
"data": ciphers_json,
"object": "list",
"continuationToken": null,
})))
Ok(Value::Array(ciphers_json))
}
// Returns every cipher in the organization, serialized with `CipherSyncType::Organization` — which
// deliberately skips the per-cipher access restrictions, so `readOnly`/`hidePasswords` are not
// applied and collection assignments are ignored. Whoever passes the check below reads the whole
// organization vault.
// The organization cipher list the clients use for the admin vault view and for computing every
// report (Exposed/Reused/Weak Passwords, Unsecured Websites, Inactive 2FA, ...) locally — Vaultwarden
// has no server-side reports.
//
// Two different answers, depending on how much the caller may actually read:
//
// `accessReports` is therefore, by design, a full organization *read* permission and not merely
// "may open the reports screen". Vaultwarden implements no server-side reports: the clients fetch
// this list and compute Exposed/Reused/Weak Passwords, Unsecured Websites, Inactive 2FA etc.
// locally, so the permission cannot be satisfied with less data.
// * Members who already reach every collection (Admin/Owner, or Custom + `editAnyCollection`) get
// the whole organization, serialized with `CipherSyncType::Organization` which deliberately skips
// the per-cipher access restrictions. This is unchanged behavior.
//
// This matches Bitwarden upstream, which grants the same endpoint to Owner/Admin and to Custom
// members holding AccessImportExport, EditAnyCollection *or* AccessReports:
// https://github.com/bitwarden/server/blob/main/src/Api/Vault/Controllers/CiphersController.cs
// (`CanAccessAllCiphersAsync`)
// * `accessReports` opens the endpoint *without* widening what may be read: the response is built
// from the caller's own assignments with `CipherSyncType::User`, so `readOnly`/`hidePasswords`
// still apply and collections the member is not assigned to never appear. Their reports therefore
// cover exactly their own collections.
//
// We are intentionally *stricter* than Bitwarden for `accessImportExport`: it does not open this
// endpoint, and `get_org_export` scopes its output to the caller's own collections, so
// "may export" never widens what a member can read. Granting `accessReports` does widen it — that
// is the documented trade-off of staying Bitwarden-compatible, and administrators must treat
// `accessReports` as equivalent to read access to every collection in the organization.
// This mirrors `accessImportExport`/`get_org_export`: a permission decides *whether* a member may use
// a feature, never *what* they may read. Bitwarden upstream is more permissive here (its
// `CanAccessAllCiphersAsync` grants the full organization to AccessReports as well); we deliberately
// deviate so that ticking "Access reports" cannot hand out read access to every password in the
// organization.
#[get("/ciphers/organization-details?<data..>")]
async fn get_org_details(data: OrgIdData, headers: ManagerHeadersLoose, conn: DbConn) -> JsonResult {
if data.organization_id != headers.membership.org_uuid {
err_code!("Resource not found.", "Organization id's do not match", rocket::http::Status::NotFound.code);
}
if !headers.membership.has_full_access() && !headers.membership.has_access_reports() {
let ciphers_json = if headers.membership.has_full_access() {
get_org_details_impl(&data.organization_id, &headers.host, &headers.user.uuid, &conn).await?
} else if headers.membership.has_access_reports() {
assigned_org_ciphers_json(&data.organization_id, &headers.host, &headers.user.uuid, &conn).await?
} else {
err_code!(
"Resource not found.",
"User does not have permission to access all organization ciphers",
"User does not have permission to read the organization ciphers",
rocket::http::Status::NotFound.code
);
}
};
Ok(Json(json!({
"data": get_org_details_impl(&data.organization_id, &headers.host, &headers.user.uuid, &conn).await?,
"data": ciphers_json,
"object": "list",
"continuationToken": null,
})))
@ -1372,7 +1391,7 @@ async fn send_invite(
err!("Invalid type")
};
if !may_manage_member_type(headers.membership_type, new_type) {
if !may_provision_member_type(headers.membership_type, new_type) {
err!("You don't have permission to invite this role")
}
@ -1781,7 +1800,7 @@ async fn confirm_invite_impl(
err!("The specified user isn't a member of the organization")
};
if !may_manage_stored_member_type(headers.membership_type, member_to_confirm.atype) {
if !may_provision_stored_member_type(headers.membership_type, member_to_confirm.atype) {
err!("You don't have permission to confirm this user")
}
@ -2192,7 +2211,7 @@ async fn delete_member_impl(
err!("User to delete isn't member of the organization")
};
if !may_manage_stored_member_type(headers.membership_type, member_to_delete.atype) {
if !may_provision_stored_member_type(headers.membership_type, member_to_delete.atype) {
err!("You don't have permission to delete this user")
}
@ -2305,17 +2324,19 @@ async fn post_org_import(
err!("Organization not found", "Organization id's do not match");
}
// accessImportExport: importing into the organization requires the permission (or Admin/Owner),
// mirroring the export endpoint and the Bitwarden permission model. The web-vault only offers org
// import to members holding this permission; enforcing it server-side keeps the two consistent.
// NOTE: this tightens the previous member-level behaviour (any confirmed member could import into
// collections they could write) — see the branch notes.
if !(headers.membership.has_status(MembershipStatus::Confirmed)
&& (headers.membership.atype >= MembershipType::Admin || headers.membership.has_access_import_export()))
{
err!(
"You need the 'Access Import/Export' permission, or to be an Admin or Owner, to import into this organization"
)
// NOTE: no `accessImportExport` gate here on purpose. Bitwarden requires that permission for an
// organization import, but Vaultwarden has always authorized this endpoint per target collection,
// and adding an up-front role check would take a capability away from ordinary members that they
// have today. The real boundary is enforced below and is unchanged: an existing collection must be
// writable for the caller (`Collection::is_writable_by_user`), and creating a new one requires the
// independent `createNewCollections` permission. `accessImportExport` therefore governs the export
// side only.
//
// A confirmed membership is required though: both checks below are confirmed-gated, so an
// invited/accepted member could otherwise only import ciphers without any collection — which lands
// unreachable, unmanaged ciphers in the organization.
if !headers.membership.has_status(MembershipStatus::Confirmed) {
err!("You need to be a confirmed member of this organization to import into it")
}
let data: ImportData = data.into_inner();
@ -3154,13 +3175,24 @@ async fn post_groups(
None => false,
};
let mut group = group_request.to_group(&org_id);
// Security: `access_all` grants the group access to every collection, so it is a
// collection-access grant just like assigning collections. A custom user without
// collection-management rights must not be able to create an access_all group.
//
// API consistency: reject instead of silently creating a group without the requested access, so a
// caller never believes it granted something the server dropped. A request that grants nothing
// (no access_all, no collections) is still accepted, which is what the plain "new group" dialog
// sends for such a caller.
if !caller_can_manage_collections {
group.access_all = false;
if group_request.access_all {
err!("You don't have permission to create a group with access to all collections")
}
if !group_request.collections.is_empty() {
err!("You don't have permission to assign collections to a group")
}
}
let group = group_request.to_group(&org_id);
log_event(
EventType::GroupCreated as i32,
@ -3215,21 +3247,35 @@ async fn put_group(
// Security: only callers who can actually manage collections (Admins/Owners, or users with
// full access) may change a group's collection assignments. A custom user with only
// manage_groups must not be able to add/remove collection access. For them we keep the
// group's existing collection assignments untouched (neither cleared nor overwritten).
// manage_groups must not be able to add/remove collection access.
let caller_can_manage_collections = headers.membership_type >= MembershipType::Admin
|| match Membership::find_by_user_and_org(&headers.user.uuid, &org_id, &conn).await {
Some(m) => m.has_full_access(),
None => false,
};
// Preserve the current `access_all` grant for callers who can't manage collections, so a
// manage_groups-only user cannot turn a group into an access_all (all-collections) grant.
let previous_access_all = group.access_all;
let mut updated_group = group_request.update_group(group);
// API consistency: reject a collection-access change this caller may not make instead of
// answering 200 and silently keeping the old value — the same rule `edit_member` and
// `send_invite` follow. Only an actual difference is rejected (the regular group dialog echoes
// the current assignments back and has to keep working), and per-assignment flag differences
// (readOnly, hidePasswords, manage) stay ignored, exactly as for a member's assignments.
if !caller_can_manage_collections {
updated_group.access_all = previous_access_all;
if group_request.access_all != group.access_all {
err!("You don't have permission to change a group's access to all collections")
}
let requested: HashSet<CollectionId> = group_request.collections.iter().map(|c| c.id.clone()).collect();
let current: HashSet<CollectionId> = CollectionGroup::find_by_group(&group_id, &org_id, &conn)
.await
.into_iter()
.map(|cg| cg.collections_uuid)
.collect();
if requested != current {
err!("You don't have permission to change this group's collection assignments")
}
}
let updated_group = group_request.update_group(group);
if caller_can_manage_collections {
CollectionGroup::delete_all_by_group(&group_id, &org_id, &conn).await?;
@ -3319,6 +3365,30 @@ fn may_manage_stored_member_type(caller_type: MembershipType, target_atype: i32)
MembershipType::from_i32(target_atype).is_some_and(|target_type| may_manage_member_type(caller_type, target_type))
}
/// Whether a caller may *provision* a membership of `target_type` — create it (invite), activate it
/// (confirm) or remove it (delete).
///
/// This is deliberately stricter than [`may_manage_member_type`] and preserves the pre-existing
/// Vaultwarden rule that only Owners bring Admin (or Owner) memberships into or out of existence
/// ("Only Owners can invite Managers, Admins or Owners" / "Only Owners can delete Admins or Owners").
/// `edit_member` keeps that boundary too, via its dedicated Owner-only guard on Admin/Owner role
/// transitions, so an Admin must not be able to route around it by inviting a fresh Admin instead.
/// State changes that leave the membership in place (reinvite, revoke, restore, edit) keep using
/// [`may_manage_member_type`], which is what Vaultwarden allowed for them before this feature.
fn may_provision_member_type(caller_type: MembershipType, target_type: MembershipType) -> bool {
match caller_type {
MembershipType::Owner => true,
MembershipType::Admin => target_type < MembershipType::Admin,
MembershipType::Custom => target_type == MembershipType::User,
MembershipType::User => false,
}
}
fn may_provision_stored_member_type(caller_type: MembershipType, target_atype: i32) -> bool {
MembershipType::from_i32(target_atype)
.is_some_and(|target_type| may_provision_member_type(caller_type, target_type))
}
/// Returns true if being a member of `group_id` confers collection access — either because the
/// group has `access_all` set, or because it has collections assigned.
async fn group_confers_collection_access(group_id: &GroupId, org_id: &OrganizationId, conn: &DbConn) -> bool {
@ -3400,6 +3470,26 @@ async fn add_update_group(
conn: &DbConn,
caller_can_manage_collections: bool,
) -> JsonResult {
// Security: assigning members to a group that grants collection access (via `access_all` or
// assigned collections) would indirectly grant those members access to the collections' contents,
// and removing them would revoke it. Only callers who can manage collections may change the
// membership of such a group.
//
// API consistency: reject a membership change this caller may not make instead of answering 200
// and keeping the old membership — same rule as `edit_member`/`send_invite`. Checked before the
// first write so a rejected request leaves nothing behind. On create the group is brand new, so
// it grants no collection access yet and this never triggers.
if !caller_can_manage_collections
&& (group.access_all || !CollectionGroup::find_by_group(&group.uuid, &org_id, conn).await.is_empty())
{
let requested: HashSet<&MembershipId> = members.iter().collect();
let current_members = GroupUser::find_by_group(&group.uuid, &org_id, conn).await;
let current: HashSet<&MembershipId> = current_members.iter().map(|gu| &gu.users_organizations_uuid).collect();
if requested != current {
err!("You don't have permission to change the membership of a group that grants collection access")
}
}
group.save(conn).await?;
// Security (F-1): a `collections_groups.manage` grant carries collection delete authority, so a
@ -4096,6 +4186,7 @@ mod tests {
CustomRolePermissions, caller_manage_grant_role_check, collection_bearing_membership_unchanged,
filter_ciphers_for_organization, may_change_group_membership, may_change_member_type,
may_export_entire_organization, may_manage_member_type, may_manage_stored_member_type,
may_provision_member_type, may_provision_stored_member_type,
};
use crate::db::models::{Cipher, GroupId, Membership, MembershipStatus, MembershipType, OrganizationId};
@ -4244,6 +4335,43 @@ mod tests {
assert!(may_manage_stored_member_type(MembershipType::Custom, MembershipType::User as i32));
}
#[test]
fn only_owners_provision_admin_memberships() {
// REGRESSION: bringing an Admin (or Owner) membership into or out of existence stays
// Owner-only, exactly as before this feature ("Only Owners can invite Managers, Admins or
// Owners" / "Only Owners can delete Admins or Owners"). Otherwise an Admin could route around
// the Owner-only role-change guard in `edit_member` by inviting a fresh Admin instead.
for target in [MembershipType::Owner, MembershipType::Admin, MembershipType::Custom, MembershipType::User] {
assert!(may_provision_member_type(MembershipType::Owner, target));
}
assert!(!may_provision_member_type(MembershipType::Admin, MembershipType::Owner));
assert!(!may_provision_member_type(MembershipType::Admin, MembershipType::Admin));
assert!(may_provision_member_type(MembershipType::Admin, MembershipType::Custom));
assert!(may_provision_member_type(MembershipType::Admin, MembershipType::User));
// A Custom member with manage_users stays limited to ordinary Users, as for every other
// lifecycle action.
assert!(may_provision_member_type(MembershipType::Custom, MembershipType::User));
for target in [MembershipType::Owner, MembershipType::Admin, MembershipType::Custom] {
assert!(!may_provision_member_type(MembershipType::Custom, target));
}
for target in [MembershipType::Owner, MembershipType::Admin, MembershipType::Custom, MembershipType::User] {
assert!(!may_provision_member_type(MembershipType::User, target));
}
// Provisioning is strictly narrower than the state-change matrix: an Admin may still revoke,
// restore or edit a peer Admin (which Vaultwarden allowed before), but no longer create,
// confirm or delete one.
assert!(may_manage_member_type(MembershipType::Admin, MembershipType::Admin));
assert!(!may_provision_member_type(MembershipType::Admin, MembershipType::Admin));
assert!(may_provision_stored_member_type(MembershipType::Admin, MembershipType::User as i32));
assert!(!may_provision_stored_member_type(MembershipType::Admin, MembershipType::Admin as i32));
// An unknown stored role never qualifies.
assert!(!may_provision_stored_member_type(MembershipType::Owner, i32::MAX));
}
#[test]
fn only_collection_bearing_group_changes_are_rejected() {
let plain: GroupId = "plain".to_owned().into();

368
src/db/mod.rs

@ -471,26 +471,67 @@ impl<'r> FromRequest<'r> for DbConn {
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_MANAGE_PERMISSIONS_MIGRATION: &str = "20260630120000";
const CUSTOM_ACCESS_PERMISSIONS_MIGRATION: &str = "20260724130000";
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 <MEMBERSHIP_UUID> 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 = '<MEMBERSHIP_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 = '<MEMBERSHIP_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."
/// One of the three groups of granular permission columns, each added by its own migration.
///
/// A partially present group means the migration was interrupted between its `ALTER TABLE`
/// statements. On MySQL/MariaDB that is reachable because DDL commits implicitly, so the ledger entry
/// can be missing while some columns already exist; re-running the migration then fails forever with
/// `Duplicate column name`. Detect it and hand the operator an unambiguous fix instead.
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
enum PermissionColumnGroup {
Manage,
Collection,
Access,
}
impl PermissionColumnGroup {
const fn migration(self) -> &'static str {
match self {
Self::Manage => CUSTOM_ROLE_MANAGE_PERMISSIONS_MIGRATION,
Self::Collection => CUSTOM_COLLECTION_PERMISSIONS_MIGRATION,
Self::Access => CUSTOM_ACCESS_PERMISSIONS_MIGRATION,
}
}
/// SQL list literal of the group's column names, for the `IN (...)` lookups.
const fn column_list(self) -> &'static str {
match self {
Self::Manage => "'manage_users', 'manage_groups', 'manage_policies'",
Self::Collection => "'create_new_collections', 'edit_any_collection', 'delete_any_collection'",
Self::Access => "'access_event_logs', 'access_import_export', 'access_reports'",
}
}
const fn description(self) -> &'static str {
match self {
Self::Manage => "custom management-permission",
Self::Collection => "custom collection-permission",
Self::Access => "custom access-permission",
}
}
}
const PARTIAL_PERMISSION_COLUMNS_RECOVERY: &str = concat!(
"\n\nThis happens when a migration was interrupted between its ALTER TABLE statements (on ",
"MySQL/MariaDB every DDL statement commits on its own, so columns can exist without the ledger ",
"entry). The leftover columns only ever hold their FALSE default at this point, so dropping them ",
"loses nothing and lets the migration run again from a clean state.\n\n",
"List the columns that are already present:\n",
"SELECT column_name\n",
"FROM information_schema.columns\n",
"WHERE table_name = 'users_organizations'\n",
" AND column_name IN ('manage_users', 'manage_groups', 'manage_policies',\n",
" 'create_new_collections', 'edit_any_collection', 'delete_any_collection',\n",
" 'access_event_logs', 'access_import_export', 'access_reports');\n\n",
"(On SQLite: SELECT name FROM pragma_table_info('users_organizations');)\n\n",
"Then, with every Vaultwarden instance stopped and a backup taken, drop exactly the columns of ",
"the affected group that the message above names, e.g.:\n",
"ALTER TABLE users_organizations DROP COLUMN <COLUMN_NAME>;\n\n",
"Afterwards restart Vaultwarden so the migration applies the whole group in one go."
);
const AMBIGUOUS_DIRECT_PERMISSIONS_RECOVERY_SQL: &str = concat!(
@ -545,15 +586,35 @@ struct CustomRoleMigrationFacts {
memberships_table_exists: bool,
migration_table_exists: bool,
access_all_column_exists: bool,
manage_permission_columns: i64,
manage_permissions_migration_applied: bool,
collection_permission_columns: i64,
collection_permissions_migration_applied: bool,
access_permission_columns: i64,
access_permissions_migration_applied: bool,
repair_migration_applied: bool,
access_all_drop_migration_applied: bool,
legacy_user_access_all_count: i64,
ambiguous_direct_permission_count: i64,
same_run_0716_marker: bool,
}
impl CustomRoleMigrationFacts {
/// `(columns present, migration recorded)` for one permission column group.
const fn permission_columns(self, group: PermissionColumnGroup) -> (i64, bool) {
match group {
PermissionColumnGroup::Manage => {
(self.manage_permission_columns, self.manage_permissions_migration_applied)
}
PermissionColumnGroup::Collection => {
(self.collection_permission_columns, self.collection_permissions_migration_applied)
}
PermissionColumnGroup::Access => {
(self.access_permission_columns, self.access_permissions_migration_applied)
}
}
}
}
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
enum CustomRolePreflightDecision {
Proceed,
@ -561,10 +622,9 @@ enum CustomRolePreflightDecision {
RefuseAlreadyDropped,
RefuseMissingAccessAll,
RefuseMissingMigrationLedger,
RefuseLegacyUserAccessAll,
RefuseAmbiguousDirectPermissions,
RefusePartialCollectionSchema,
RefuseCollectionLedgerMismatch,
RefusePartialPermissionSchema(PermissionColumnGroup),
RefusePermissionLedgerMismatch(PermissionColumnGroup),
}
fn custom_role_preflight_decision(
@ -587,21 +647,27 @@ fn custom_role_preflight_decision(
return CustomRolePreflightDecision::RefuseMissingAccessAll;
}
if facts.legacy_user_access_all_count != 0 {
return CustomRolePreflightDecision::RefuseLegacyUserAccessAll;
}
if facts.ambiguous_direct_permission_count != 0 && !facts.same_run_0716_marker {
return CustomRolePreflightDecision::RefuseAmbiguousDirectPermissions;
}
match (facts.collection_permission_columns, facts.collection_permissions_migration_applied) {
(0, false) | (3, true) => CustomRolePreflightDecision::Proceed,
(3, false) if can_complete_mysql_partial_migration => {
CustomRolePreflightDecision::CompleteMysqlCollectionMigration
// Every permission column group must be either completely absent (its migration is still pending)
// or completely present with its ledger entry. Anything else is an interrupted migration whose
// re-run would fail with `Duplicate column name`, so refuse with an actionable message. The single
// historical exception is the collection group on MySQL, where the known-good partial state is
// completed in place.
for group in [PermissionColumnGroup::Manage, PermissionColumnGroup::Collection, PermissionColumnGroup::Access] {
match facts.permission_columns(group) {
(0, false) | (3, true) => {}
(3, false) if group == PermissionColumnGroup::Collection && can_complete_mysql_partial_migration => {
return CustomRolePreflightDecision::CompleteMysqlCollectionMigration;
}
(_, true) => return CustomRolePreflightDecision::RefusePermissionLedgerMismatch(group),
_ => return CustomRolePreflightDecision::RefusePartialPermissionSchema(group),
}
(_, true) => CustomRolePreflightDecision::RefuseCollectionLedgerMismatch,
_ => CustomRolePreflightDecision::RefusePartialCollectionSchema,
}
CustomRolePreflightDecision::Proceed
}
fn custom_role_preflight_error(decision: CustomRolePreflightDecision, facts: CustomRoleMigrationFacts) -> Error {
@ -621,36 +687,35 @@ fn custom_role_preflight_error(decision: CustomRolePreflightDecision, facts: Cus
Refusing to guess which schema and data migrations were previously applied."
.to_owned()
}
CustomRolePreflightDecision::RefuseLegacyUserAccessAll => format!(
"{} legacy User membership(s) still have membership access_all=true. Mapping these \
records to Custom/EditAny would add management authority, while clearing the bit \
would remove existing vault access.",
facts.legacy_user_access_all_count
),
CustomRolePreflightDecision::RefuseAmbiguousDirectPermissions => format!(
"Found {} membership(s) with an ambiguous 0/1/1 collection-permission pattern. It is \
not possible to distinguish an older group-derived backfill from an intentional \
direct Edit+Delete assignment.",
facts.ambiguous_direct_permission_count
),
CustomRolePreflightDecision::RefusePartialCollectionSchema => format!(
"Found {} of the three custom collection-permission columns without a completed \
{CUSTOM_COLLECTION_PERMISSIONS_MIGRATION} migration. This is not an automatically \
recoverable state for this database backend.",
facts.collection_permission_columns
CustomRolePreflightDecision::RefusePartialPermissionSchema(group) => format!(
"Found {} of the three {} columns ({}) without a completed {} migration. The migration \
was interrupted between its ALTER TABLE statements.",
facts.permission_columns(group).0,
group.description(),
group.column_list(),
group.migration()
),
CustomRolePreflightDecision::RefuseCollectionLedgerMismatch => format!(
"Migration {CUSTOM_COLLECTION_PERMISSIONS_MIGRATION} is recorded, but only {} of its \
three collection-permission columns exist.",
facts.collection_permission_columns
CustomRolePreflightDecision::RefusePermissionLedgerMismatch(group) => format!(
"Migration {} is recorded, but only {} of its three {} columns ({}) exist.",
group.migration(),
facts.permission_columns(group).0,
group.description(),
group.column_list()
),
CustomRolePreflightDecision::Proceed | CustomRolePreflightDecision::CompleteMysqlCollectionMigration => {
unreachable!("successful preflight decisions do not produce errors")
}
};
let recovery = match decision {
CustomRolePreflightDecision::RefuseLegacyUserAccessAll => LEGACY_USER_ACCESS_ALL_RECOVERY_SQL,
CustomRolePreflightDecision::RefuseAmbiguousDirectPermissions => AMBIGUOUS_DIRECT_PERMISSIONS_RECOVERY_SQL,
CustomRolePreflightDecision::RefusePartialPermissionSchema(_)
| CustomRolePreflightDecision::RefusePermissionLedgerMismatch(_) => PARTIAL_PERMISSION_COLUMNS_RECOVERY,
CustomRolePreflightDecision::RefuseAlreadyDropped => ALREADY_DROPPED_RECOVERY,
_ => "",
};
@ -761,14 +826,28 @@ mod sqlite_migrations {
"SELECT COUNT(*) AS count FROM pragma_table_info('users_organizations') \
WHERE name = 'access_all'",
)? != 0;
let collection_permission_columns = count(
let permission_columns = |connection: &mut diesel::sqlite::SqliteConnection,
group: super::PermissionColumnGroup|
-> Result<i64, diesel::result::Error> {
count(
connection,
format!(
"SELECT COUNT(*) AS count FROM pragma_table_info('users_organizations') \
WHERE name IN ('create_new_collections', 'edit_any_collection', 'delete_any_collection')",
)?;
WHERE name IN ({})",
group.column_list()
),
)
};
let manage_permission_columns = permission_columns(connection, super::PermissionColumnGroup::Manage)?;
let collection_permission_columns = permission_columns(connection, super::PermissionColumnGroup::Collection)?;
let access_permission_columns = permission_columns(connection, super::PermissionColumnGroup::Access)?;
let manage_permissions_migration_applied =
migration_table_exists && migration_applied(connection, super::CUSTOM_ROLE_MANAGE_PERMISSIONS_MIGRATION)?;
let collection_permissions_migration_applied =
migration_table_exists && migration_applied(connection, super::CUSTOM_COLLECTION_PERMISSIONS_MIGRATION)?;
let access_permissions_migration_applied =
migration_table_exists && migration_applied(connection, super::CUSTOM_ACCESS_PERMISSIONS_MIGRATION)?;
let repair_migration_applied =
migration_table_exists && migration_applied(connection, super::CUSTOM_ROLE_REPAIR_MIGRATION)?;
let access_all_drop_migration_applied =
@ -780,16 +859,6 @@ mod sqlite_migrations {
format!("SELECT COUNT(*) AS count FROM {} WHERE marker = 1", super::CUSTOM_ROLE_SAME_RUN_MARKER_TABLE),
)? != 0;
let legacy_user_access_all_count = if access_all_column_exists {
count(
connection,
"SELECT COUNT(*) AS count FROM users_organizations \
WHERE atype = 2 AND access_all = TRUE",
)?
} else {
0
};
let ambiguous_direct_permission_count = if access_all_column_exists && collection_permission_columns == 3 {
count(
connection,
@ -808,11 +877,14 @@ mod sqlite_migrations {
memberships_table_exists,
migration_table_exists,
access_all_column_exists,
manage_permission_columns,
manage_permissions_migration_applied,
collection_permission_columns,
collection_permissions_migration_applied,
access_permission_columns,
access_permissions_migration_applied,
repair_migration_applied,
access_all_drop_migration_applied,
legacy_user_access_all_count,
ambiguous_direct_permission_count,
same_run_0716_marker,
};
@ -968,17 +1040,27 @@ mod mysql_migrations {
AND table_name = 'users_organizations' \
AND column_name = 'access_all'",
)? != 0;
let collection_permission_columns = count(
let permission_columns = |connection: &mut diesel::mysql::MysqlConnection,
group: super::PermissionColumnGroup|
-> Result<i64, diesel::result::Error> {
count(
connection,
"SELECT COUNT(*) AS count FROM information_schema.columns \
WHERE table_schema = DATABASE() \
AND table_name = 'users_organizations' \
AND column_name IN \
('create_new_collections', 'edit_any_collection', 'delete_any_collection')",
)?;
format!(
"SELECT COUNT(*) AS count FROM information_schema.columns WHERE table_schema = DATABASE() AND table_name = 'users_organizations' AND column_name IN ({})",
group.column_list()
),
)
};
let manage_permission_columns = permission_columns(connection, super::PermissionColumnGroup::Manage)?;
let collection_permission_columns = permission_columns(connection, super::PermissionColumnGroup::Collection)?;
let access_permission_columns = permission_columns(connection, super::PermissionColumnGroup::Access)?;
let manage_permissions_migration_applied =
migration_table_exists && migration_applied(connection, super::CUSTOM_ROLE_MANAGE_PERMISSIONS_MIGRATION)?;
let collection_permissions_migration_applied =
migration_table_exists && migration_applied(connection, super::CUSTOM_COLLECTION_PERMISSIONS_MIGRATION)?;
let access_permissions_migration_applied =
migration_table_exists && migration_applied(connection, super::CUSTOM_ACCESS_PERMISSIONS_MIGRATION)?;
let repair_migration_applied =
migration_table_exists && migration_applied(connection, super::CUSTOM_ROLE_REPAIR_MIGRATION)?;
let access_all_drop_migration_applied =
@ -990,16 +1072,6 @@ mod mysql_migrations {
format!("SELECT COUNT(*) AS count FROM {} WHERE marker = 1", super::CUSTOM_ROLE_SAME_RUN_MARKER_TABLE),
)? != 0;
let legacy_user_access_all_count = if access_all_column_exists {
count(
connection,
"SELECT COUNT(*) AS count FROM users_organizations \
WHERE atype = 2 AND access_all = TRUE",
)?
} else {
0
};
let ambiguous_direct_permission_count = if access_all_column_exists && collection_permission_columns == 3 {
count(
connection,
@ -1018,11 +1090,14 @@ mod mysql_migrations {
memberships_table_exists,
migration_table_exists,
access_all_column_exists,
manage_permission_columns,
manage_permissions_migration_applied,
collection_permission_columns,
collection_permissions_migration_applied,
access_permission_columns,
access_permissions_migration_applied,
repair_migration_applied,
access_all_drop_migration_applied,
legacy_user_access_all_count,
ambiguous_direct_permission_count,
same_run_0716_marker,
};
@ -1111,17 +1186,27 @@ mod postgresql_migrations {
AND table_name = 'users_organizations' \
AND column_name = 'access_all'",
)? != 0;
let collection_permission_columns = count(
let permission_columns = |connection: &mut diesel::pg::PgConnection,
group: super::PermissionColumnGroup|
-> Result<i64, diesel::result::Error> {
count(
connection,
"SELECT COUNT(*) AS count FROM information_schema.columns \
WHERE table_schema = current_schema() \
AND table_name = 'users_organizations' \
AND column_name IN \
('create_new_collections', 'edit_any_collection', 'delete_any_collection')",
)?;
format!(
"SELECT COUNT(*) AS count FROM information_schema.columns WHERE table_schema = current_schema() AND table_name = 'users_organizations' AND column_name IN ({})",
group.column_list()
),
)
};
let manage_permission_columns = permission_columns(connection, super::PermissionColumnGroup::Manage)?;
let collection_permission_columns = permission_columns(connection, super::PermissionColumnGroup::Collection)?;
let access_permission_columns = permission_columns(connection, super::PermissionColumnGroup::Access)?;
let manage_permissions_migration_applied =
migration_table_exists && migration_applied(connection, super::CUSTOM_ROLE_MANAGE_PERMISSIONS_MIGRATION)?;
let collection_permissions_migration_applied =
migration_table_exists && migration_applied(connection, super::CUSTOM_COLLECTION_PERMISSIONS_MIGRATION)?;
let access_permissions_migration_applied =
migration_table_exists && migration_applied(connection, super::CUSTOM_ACCESS_PERMISSIONS_MIGRATION)?;
let repair_migration_applied =
migration_table_exists && migration_applied(connection, super::CUSTOM_ROLE_REPAIR_MIGRATION)?;
let access_all_drop_migration_applied =
@ -1133,16 +1218,6 @@ mod postgresql_migrations {
format!("SELECT COUNT(*) AS count FROM {} WHERE marker = 1", super::CUSTOM_ROLE_SAME_RUN_MARKER_TABLE),
)? != 0;
let legacy_user_access_all_count = if access_all_column_exists {
count(
connection,
"SELECT COUNT(*) AS count FROM users_organizations \
WHERE atype = 2 AND access_all = TRUE",
)?
} else {
0
};
let ambiguous_direct_permission_count = if access_all_column_exists && collection_permission_columns == 3 {
count(
connection,
@ -1161,11 +1236,14 @@ mod postgresql_migrations {
memberships_table_exists,
migration_table_exists,
access_all_column_exists,
manage_permission_columns,
manage_permissions_migration_applied,
collection_permission_columns,
collection_permissions_migration_applied,
access_permission_columns,
access_permissions_migration_applied,
repair_migration_applied,
access_all_drop_migration_applied,
legacy_user_access_all_count,
ambiguous_direct_permission_count,
same_run_0716_marker,
};
@ -1295,35 +1373,70 @@ mod custom_role_migration_preflight_tests {
assert!(message.contains("Restore the database backup"));
}
// REGRESSION: a legacy `User` membership with the historical access_all bit must NOT stop the
// upgrade. The 2026-07-23 migration materializes that reach as explicit per-collection
// assignments, so the preflight has nothing left to decide and every other fact stays untouched.
#[test]
fn legacy_user_access_all_requires_an_operator_decision() {
let facts = Facts {
legacy_user_access_all_count: 1,
fn legacy_user_access_all_no_longer_blocks_the_upgrade() {
assert_eq!(custom_role_preflight_decision(pending_repair(), false), Decision::Proceed);
assert_eq!(
custom_role_preflight_decision(
Facts {
collection_permission_columns: 3,
collection_permissions_migration_applied: true,
manage_permission_columns: 3,
manage_permissions_migration_applied: true,
..pending_repair()
};
let decision = custom_role_preflight_decision(facts, false);
assert_eq!(decision, Decision::RefuseLegacyUserAccessAll);
},
false,
),
Decision::Proceed
);
}
let error = custom_role_preflight_error(decision, facts);
#[test]
fn a_partial_permission_column_group_is_refused_with_an_actionable_message() {
// Every group is checked, not just the collection one: an interrupted MySQL migration can
// leave `manage_*` or `access_*` columns behind, and re-running it would fail forever with
// `Duplicate column name`.
for (facts, group, expected) in [
(
Facts {
manage_permission_columns: 2,
..pending_repair()
},
"manage_users",
Decision::RefusePartialPermissionSchema(super::PermissionColumnGroup::Manage),
),
(
Facts {
manage_permission_columns: 3,
manage_permissions_migration_applied: true,
access_permission_columns: 3,
..pending_repair()
},
"access_event_logs",
Decision::RefusePartialPermissionSchema(super::PermissionColumnGroup::Access),
),
(
Facts {
manage_permission_columns: 1,
manage_permissions_migration_applied: true,
..pending_repair()
},
"manage_users",
Decision::RefusePermissionLedgerMismatch(super::PermissionColumnGroup::Manage),
),
] {
// `true` = MySQL: only the historical collection-group state is auto-completed, never these.
assert_eq!(custom_role_preflight_decision(facts, true), expected);
assert_eq!(custom_role_preflight_decision(facts, false), expected);
let error = custom_role_preflight_error(expected, 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 = '<MEMBERSHIP_UUID>' AND atype = 2 AND access_all = TRUE;"
));
assert!(message.contains(
"SET atype = 3\n\
WHERE uuid = '<MEMBERSHIP_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"));
assert!(message.contains(group), "message should name the affected columns: {message}");
assert!(message.contains("ALTER TABLE users_organizations DROP COLUMN"));
}
}
#[test]
@ -1375,7 +1488,10 @@ mod custom_role_migration_preflight_tests {
..pending_repair()
};
assert_eq!(custom_role_preflight_decision(facts, true), Decision::CompleteMysqlCollectionMigration);
assert_eq!(custom_role_preflight_decision(facts, false), Decision::RefusePartialCollectionSchema);
assert_eq!(
custom_role_preflight_decision(facts, false),
Decision::RefusePartialPermissionSchema(super::PermissionColumnGroup::Collection)
);
}
#[test]
@ -1406,7 +1522,7 @@ mod custom_role_migration_preflight_tests {
},
true,
),
Decision::RefusePartialCollectionSchema
Decision::RefusePartialPermissionSchema(super::PermissionColumnGroup::Collection)
);
assert_eq!(
custom_role_preflight_decision(
@ -1417,7 +1533,7 @@ mod custom_role_migration_preflight_tests {
},
true,
),
Decision::RefuseCollectionLedgerMismatch
Decision::RefusePermissionLedgerMismatch(super::PermissionColumnGroup::Collection)
);
}
}

23
src/db/models/organization.rs

@ -15,7 +15,7 @@ use crate::{
db::{
DbConn,
schema::{
ciphers, ciphers_collections, collections, collections_groups, groups, groups_users, org_policies,
ciphers_collections, collections, collections_groups, groups, groups_users, org_policies,
organization_api_key, organizations, users, users_collections, users_organizations,
},
},
@ -1295,27 +1295,6 @@ impl Membership {
.await
}
pub async fn user_has_ge_admin_access_to_cipher(user_uuid: &UserId, cipher_uuid: &CipherId, conn: &DbConn) -> bool {
conn.run(move |conn| {
users_organizations::table
.inner_join(
ciphers::table.on(ciphers::uuid
.eq(cipher_uuid)
.and(ciphers::organization_uuid.eq(users_organizations::org_uuid.nullable()))),
)
.filter(users_organizations::user_uuid.eq(user_uuid))
.filter(
users_organizations::atype.eq_any(vec![MembershipType::Owner as i32, MembershipType::Admin as i32]),
)
.count()
.first::<i64>(conn)
.ok()
.unwrap_or(0)
!= 0
})
.await
}
pub async fn find_by_collection_and_org(
collection_uuid: &CollectionId,
org_uuid: &OrganizationId,

Loading…
Cancel
Save