diff --git a/migrations/mysql/2026-07-15-120000_mark_pending_custom_collection_migration/down.sql b/migrations/mysql/2026-07-15-120000_mark_pending_custom_collection_migration/down.sql new file mode 100644 index 00000000..04346743 --- /dev/null +++ b/migrations/mysql/2026-07-15-120000_mark_pending_custom_collection_migration/down.sql @@ -0,0 +1 @@ +DROP TABLE IF EXISTS __vw_custom_role_same_run_0716; diff --git a/migrations/mysql/2026-07-15-120000_mark_pending_custom_collection_migration/up.sql b/migrations/mysql/2026-07-15-120000_mark_pending_custom_collection_migration/up.sql new file mode 100644 index 00000000..1ba47e9d --- /dev/null +++ b/migrations/mysql/2026-07-15-120000_mark_pending_custom_collection_migration/up.sql @@ -0,0 +1,13 @@ +-- Record whether 2026-07-16 is about to run in this migration sequence. The durable marker lets a +-- retry distinguish its deterministic group-derived 0/1/1 backfill from older, ambiguous data. +CREATE TABLE IF NOT EXISTS __vw_custom_role_same_run_0716 ( + marker INTEGER NOT NULL PRIMARY KEY +); +INSERT IGNORE INTO __vw_custom_role_same_run_0716 (marker) +SELECT 1 +FROM DUAL +WHERE NOT EXISTS ( + SELECT 1 + FROM __diesel_schema_migrations + WHERE version = '20260716120000' +); diff --git a/migrations/mysql/2026-07-23-120000_reconcile_legacy_custom_roles/down.sql b/migrations/mysql/2026-07-23-120000_reconcile_legacy_custom_roles/down.sql new file mode 100644 index 00000000..b9d4e9e6 --- /dev/null +++ b/migrations/mysql/2026-07-23-120000_reconcile_legacy_custom_roles/down.sql @@ -0,0 +1,3 @@ +-- This is an idempotent data repair. Reverting it must not remove permissions or recreate the +-- invalid persisted Manager type; the older-schema migration performs its own safe conversion. +SELECT 1; diff --git a/migrations/mysql/2026-07-23-120000_reconcile_legacy_custom_roles/up.sql b/migrations/mysql/2026-07-23-120000_reconcile_legacy_custom_roles/up.sql new file mode 100644 index 00000000..3186fe6a --- /dev/null +++ b/migrations/mysql/2026-07-23-120000_reconcile_legacy_custom_roles/up.sql @@ -0,0 +1,67 @@ +-- 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 +); +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; + +-- 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, +-- remove that deterministic copy so later group removal also revokes the authority. +UPDATE users_organizations +SET edit_any_collection = FALSE, + delete_any_collection = FALSE +WHERE atype IN (3, 4) + AND access_all = FALSE + AND create_new_collections = FALSE + AND edit_any_collection = TRUE + AND delete_any_collection = TRUE + AND EXISTS (SELECT 1 FROM __vw_custom_role_same_run_0716 WHERE marker = 1) + AND EXISTS ( + SELECT 1 + FROM groups_users AS gu + INNER JOIN `groups` AS g ON g.uuid = gu.groups_uuid + WHERE gu.users_organizations_uuid = users_organizations.uuid + AND g.organizations_uuid = users_organizations.org_uuid + AND g.access_all = TRUE + ); + +-- A remaining 0/1/1 pattern may be either an intentional direct grant or an older derived grant +-- whose source group has already been removed. Do not guess which one it is. +CREATE TEMPORARY TABLE __vw_legacy_group_access_guard ( + blocked INTEGER NOT NULL PRIMARY KEY +); +INSERT INTO __vw_legacy_group_access_guard (blocked) VALUES (1); +INSERT INTO __vw_legacy_group_access_guard (blocked) +SELECT 1 +FROM users_organizations +WHERE atype IN (3, 4) + AND access_all = FALSE + AND create_new_collections = FALSE + AND edit_any_collection = TRUE + AND delete_any_collection = TRUE +LIMIT 1; +DROP TEMPORARY TABLE __vw_legacy_group_access_guard; + +-- Membership access_all on a legacy Manager/Custom represented all three collection capabilities. +-- Set only TRUE values so this repair never removes independently configured permissions. +UPDATE users_organizations +SET create_new_collections = TRUE, + edit_any_collection = TRUE, + delete_any_collection = TRUE +WHERE atype IN (3, 4) + AND access_all = TRUE; + +-- Convert only after the legacy bit has been copied. +UPDATE users_organizations SET atype = 4 WHERE atype = 3; + +-- Clear only the marker row as transactional DML. Keeping the empty bookkeeping table avoids +-- MySQL DDL implicit commits, so the permission repair, marker clear, and Diesel ledger insert +-- either commit together or are all retried. +DELETE FROM __vw_custom_role_same_run_0716 WHERE marker = 1; diff --git a/migrations/mysql/2026-07-24-120000_drop_membership_access_all/down.sql b/migrations/mysql/2026-07-24-120000_drop_membership_access_all/down.sql new file mode 100644 index 00000000..01295bf5 --- /dev/null +++ b/migrations/mysql/2026-07-24-120000_drop_membership_access_all/down.sql @@ -0,0 +1,6 @@ +-- 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`. +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; diff --git a/migrations/mysql/2026-07-24-120000_drop_membership_access_all/up.sql b/migrations/mysql/2026-07-24-120000_drop_membership_access_all/up.sql new file mode 100644 index 00000000..e11fb611 --- /dev/null +++ b/migrations/mysql/2026-07-24-120000_drop_membership_access_all/up.sql @@ -0,0 +1,5 @@ +-- The membership `access_all` flag was Vaultwarden's pre-permissions patch for "this member can +-- reach every collection". It is now fully represented by the role model: Owners/Admins hold it +-- implicitly, and a Custom member holds it via `edit_any_collection`. Drop the redundant column. +-- This only concerns users_organizations; groups.access_all is a separate, still-supported feature. +ALTER TABLE users_organizations DROP COLUMN access_all; diff --git a/migrations/mysql/2026-07-24-130000_add_custom_access_permissions/down.sql b/migrations/mysql/2026-07-24-130000_add_custom_access_permissions/down.sql new file mode 100644 index 00000000..f276ea5b --- /dev/null +++ b/migrations/mysql/2026-07-24-130000_add_custom_access_permissions/down.sql @@ -0,0 +1,3 @@ +ALTER TABLE users_organizations DROP COLUMN access_event_logs; +ALTER TABLE users_organizations DROP COLUMN access_import_export; +ALTER TABLE users_organizations DROP COLUMN access_reports; diff --git a/migrations/mysql/2026-07-24-130000_add_custom_access_permissions/up.sql b/migrations/mysql/2026-07-24-130000_add_custom_access_permissions/up.sql new file mode 100644 index 00000000..9d9c31ff --- /dev/null +++ b/migrations/mysql/2026-07-24-130000_add_custom_access_permissions/up.sql @@ -0,0 +1,5 @@ +-- Three additional Bitwarden Custom-role permissions. They are only meaningful for Custom members +-- (gated on the role in code); Owners/Admins hold every permission implicitly. +ALTER TABLE users_organizations ADD COLUMN access_event_logs BOOLEAN NOT NULL DEFAULT FALSE; +ALTER TABLE users_organizations ADD COLUMN access_import_export BOOLEAN NOT NULL DEFAULT FALSE; +ALTER TABLE users_organizations ADD COLUMN access_reports BOOLEAN NOT NULL DEFAULT FALSE; diff --git a/migrations/mysql/2026-07-24-140000_guard_custom_role_downgrade/down.sql b/migrations/mysql/2026-07-24-140000_guard_custom_role_downgrade/down.sql new file mode 100644 index 00000000..4eb19e97 --- /dev/null +++ b/migrations/mysql/2026-07-24-140000_guard_custom_role_downgrade/down.sql @@ -0,0 +1,7 @@ +-- Nine independent Custom-role permissions cannot be represented losslessly by the legacy +-- role/access_all schema. Always stop before any older down migration removes permission data. +CREATE TEMPORARY TABLE __vw_custom_role_downgrade_guard ( + blocked INTEGER NOT NULL PRIMARY KEY +); +INSERT INTO __vw_custom_role_downgrade_guard (blocked) VALUES (1); +INSERT INTO __vw_custom_role_downgrade_guard (blocked) VALUES (1); diff --git a/migrations/mysql/2026-07-24-140000_guard_custom_role_downgrade/up.sql b/migrations/mysql/2026-07-24-140000_guard_custom_role_downgrade/up.sql new file mode 100644 index 00000000..af5fed1b --- /dev/null +++ b/migrations/mysql/2026-07-24-140000_guard_custom_role_downgrade/up.sql @@ -0,0 +1,3 @@ +-- Forward migration marker. Its down migration intentionally blocks an automatic lossy downgrade +-- before any granular permission column is removed. +SELECT 1; diff --git a/migrations/postgresql/2026-07-15-120000_mark_pending_custom_collection_migration/down.sql b/migrations/postgresql/2026-07-15-120000_mark_pending_custom_collection_migration/down.sql new file mode 100644 index 00000000..04346743 --- /dev/null +++ b/migrations/postgresql/2026-07-15-120000_mark_pending_custom_collection_migration/down.sql @@ -0,0 +1 @@ +DROP TABLE IF EXISTS __vw_custom_role_same_run_0716; diff --git a/migrations/postgresql/2026-07-15-120000_mark_pending_custom_collection_migration/up.sql b/migrations/postgresql/2026-07-15-120000_mark_pending_custom_collection_migration/up.sql new file mode 100644 index 00000000..f4f6862e --- /dev/null +++ b/migrations/postgresql/2026-07-15-120000_mark_pending_custom_collection_migration/up.sql @@ -0,0 +1,13 @@ +-- Record whether 2026-07-16 is about to run in this migration sequence. The durable marker lets a +-- retry distinguish its deterministic group-derived 0/1/1 backfill from older, ambiguous data. +CREATE TABLE IF NOT EXISTS __vw_custom_role_same_run_0716 ( + marker INTEGER NOT NULL PRIMARY KEY +); +INSERT INTO __vw_custom_role_same_run_0716 (marker) +SELECT 1 +WHERE NOT EXISTS ( + SELECT 1 + FROM __diesel_schema_migrations + WHERE version = '20260716120000' +) +ON CONFLICT (marker) DO NOTHING; diff --git a/migrations/postgresql/2026-07-23-120000_reconcile_legacy_custom_roles/down.sql b/migrations/postgresql/2026-07-23-120000_reconcile_legacy_custom_roles/down.sql new file mode 100644 index 00000000..b9d4e9e6 --- /dev/null +++ b/migrations/postgresql/2026-07-23-120000_reconcile_legacy_custom_roles/down.sql @@ -0,0 +1,3 @@ +-- This is an idempotent data repair. Reverting it must not remove permissions or recreate the +-- invalid persisted Manager type; the older-schema migration performs its own safe conversion. +SELECT 1; diff --git a/migrations/postgresql/2026-07-23-120000_reconcile_legacy_custom_roles/up.sql b/migrations/postgresql/2026-07-23-120000_reconcile_legacy_custom_roles/up.sql new file mode 100644 index 00000000..6d75889c --- /dev/null +++ b/migrations/postgresql/2026-07-23-120000_reconcile_legacy_custom_roles/up.sql @@ -0,0 +1,65 @@ +-- 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 +); +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; + +-- 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, +-- remove that deterministic copy so later group removal also revokes the authority. +UPDATE users_organizations +SET edit_any_collection = FALSE, + delete_any_collection = FALSE +WHERE atype IN (3, 4) + AND access_all = FALSE + AND create_new_collections = FALSE + AND edit_any_collection = TRUE + AND delete_any_collection = TRUE + AND EXISTS (SELECT 1 FROM __vw_custom_role_same_run_0716 WHERE marker = 1) + AND EXISTS ( + SELECT 1 + FROM groups_users AS gu + INNER JOIN "groups" AS g ON g.uuid = gu.groups_uuid + WHERE gu.users_organizations_uuid = users_organizations.uuid + AND g.organizations_uuid = users_organizations.org_uuid + AND g.access_all = TRUE + ); + +-- A remaining 0/1/1 pattern may be either an intentional direct grant or an older derived grant +-- whose source group has already been removed. Do not guess which one it is. +CREATE TEMPORARY TABLE __vw_legacy_group_access_guard ( + blocked INTEGER NOT NULL PRIMARY KEY +); +INSERT INTO __vw_legacy_group_access_guard (blocked) VALUES (1); +INSERT INTO __vw_legacy_group_access_guard (blocked) +SELECT 1 +FROM users_organizations +WHERE atype IN (3, 4) + AND access_all = FALSE + AND create_new_collections = FALSE + AND edit_any_collection = TRUE + AND delete_any_collection = TRUE +LIMIT 1; +DROP TABLE __vw_legacy_group_access_guard; + +-- Membership access_all on a legacy Manager/Custom represented all three collection capabilities. +-- Set only TRUE values so this repair never removes independently configured permissions. +UPDATE users_organizations +SET create_new_collections = TRUE, + edit_any_collection = TRUE, + delete_any_collection = TRUE +WHERE atype IN (3, 4) + AND access_all = TRUE; + +-- Convert only after the legacy bit has been copied. +UPDATE users_organizations SET atype = 4 WHERE atype = 3; + +-- Clear the same-run marker only after every guard and permission update succeeds. +DELETE FROM __vw_custom_role_same_run_0716 WHERE marker = 1; diff --git a/migrations/postgresql/2026-07-24-120000_drop_membership_access_all/down.sql b/migrations/postgresql/2026-07-24-120000_drop_membership_access_all/down.sql new file mode 100644 index 00000000..01295bf5 --- /dev/null +++ b/migrations/postgresql/2026-07-24-120000_drop_membership_access_all/down.sql @@ -0,0 +1,6 @@ +-- 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`. +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; diff --git a/migrations/postgresql/2026-07-24-120000_drop_membership_access_all/up.sql b/migrations/postgresql/2026-07-24-120000_drop_membership_access_all/up.sql new file mode 100644 index 00000000..e11fb611 --- /dev/null +++ b/migrations/postgresql/2026-07-24-120000_drop_membership_access_all/up.sql @@ -0,0 +1,5 @@ +-- The membership `access_all` flag was Vaultwarden's pre-permissions patch for "this member can +-- reach every collection". It is now fully represented by the role model: Owners/Admins hold it +-- implicitly, and a Custom member holds it via `edit_any_collection`. Drop the redundant column. +-- This only concerns users_organizations; groups.access_all is a separate, still-supported feature. +ALTER TABLE users_organizations DROP COLUMN access_all; diff --git a/migrations/postgresql/2026-07-24-130000_add_custom_access_permissions/down.sql b/migrations/postgresql/2026-07-24-130000_add_custom_access_permissions/down.sql new file mode 100644 index 00000000..f276ea5b --- /dev/null +++ b/migrations/postgresql/2026-07-24-130000_add_custom_access_permissions/down.sql @@ -0,0 +1,3 @@ +ALTER TABLE users_organizations DROP COLUMN access_event_logs; +ALTER TABLE users_organizations DROP COLUMN access_import_export; +ALTER TABLE users_organizations DROP COLUMN access_reports; diff --git a/migrations/postgresql/2026-07-24-130000_add_custom_access_permissions/up.sql b/migrations/postgresql/2026-07-24-130000_add_custom_access_permissions/up.sql new file mode 100644 index 00000000..9d9c31ff --- /dev/null +++ b/migrations/postgresql/2026-07-24-130000_add_custom_access_permissions/up.sql @@ -0,0 +1,5 @@ +-- Three additional Bitwarden Custom-role permissions. They are only meaningful for Custom members +-- (gated on the role in code); Owners/Admins hold every permission implicitly. +ALTER TABLE users_organizations ADD COLUMN access_event_logs BOOLEAN NOT NULL DEFAULT FALSE; +ALTER TABLE users_organizations ADD COLUMN access_import_export BOOLEAN NOT NULL DEFAULT FALSE; +ALTER TABLE users_organizations ADD COLUMN access_reports BOOLEAN NOT NULL DEFAULT FALSE; diff --git a/migrations/postgresql/2026-07-24-140000_guard_custom_role_downgrade/down.sql b/migrations/postgresql/2026-07-24-140000_guard_custom_role_downgrade/down.sql new file mode 100644 index 00000000..4eb19e97 --- /dev/null +++ b/migrations/postgresql/2026-07-24-140000_guard_custom_role_downgrade/down.sql @@ -0,0 +1,7 @@ +-- Nine independent Custom-role permissions cannot be represented losslessly by the legacy +-- role/access_all schema. Always stop before any older down migration removes permission data. +CREATE TEMPORARY TABLE __vw_custom_role_downgrade_guard ( + blocked INTEGER NOT NULL PRIMARY KEY +); +INSERT INTO __vw_custom_role_downgrade_guard (blocked) VALUES (1); +INSERT INTO __vw_custom_role_downgrade_guard (blocked) VALUES (1); diff --git a/migrations/postgresql/2026-07-24-140000_guard_custom_role_downgrade/up.sql b/migrations/postgresql/2026-07-24-140000_guard_custom_role_downgrade/up.sql new file mode 100644 index 00000000..af5fed1b --- /dev/null +++ b/migrations/postgresql/2026-07-24-140000_guard_custom_role_downgrade/up.sql @@ -0,0 +1,3 @@ +-- Forward migration marker. Its down migration intentionally blocks an automatic lossy downgrade +-- before any granular permission column is removed. +SELECT 1; diff --git a/migrations/sqlite/2026-07-15-120000_mark_pending_custom_collection_migration/down.sql b/migrations/sqlite/2026-07-15-120000_mark_pending_custom_collection_migration/down.sql new file mode 100644 index 00000000..04346743 --- /dev/null +++ b/migrations/sqlite/2026-07-15-120000_mark_pending_custom_collection_migration/down.sql @@ -0,0 +1 @@ +DROP TABLE IF EXISTS __vw_custom_role_same_run_0716; diff --git a/migrations/sqlite/2026-07-15-120000_mark_pending_custom_collection_migration/up.sql b/migrations/sqlite/2026-07-15-120000_mark_pending_custom_collection_migration/up.sql new file mode 100644 index 00000000..53fd7671 --- /dev/null +++ b/migrations/sqlite/2026-07-15-120000_mark_pending_custom_collection_migration/up.sql @@ -0,0 +1,12 @@ +-- Record whether 2026-07-16 is about to run in this migration sequence. The durable marker lets a +-- retry distinguish its deterministic group-derived 0/1/1 backfill from older, ambiguous data. +CREATE TABLE IF NOT EXISTS __vw_custom_role_same_run_0716 ( + marker INTEGER NOT NULL PRIMARY KEY +); +INSERT OR IGNORE INTO __vw_custom_role_same_run_0716 (marker) +SELECT 1 +WHERE NOT EXISTS ( + SELECT 1 + FROM __diesel_schema_migrations + WHERE version = '20260716120000' +); diff --git a/migrations/sqlite/2026-07-23-120000_reconcile_legacy_custom_roles/down.sql b/migrations/sqlite/2026-07-23-120000_reconcile_legacy_custom_roles/down.sql new file mode 100644 index 00000000..b9d4e9e6 --- /dev/null +++ b/migrations/sqlite/2026-07-23-120000_reconcile_legacy_custom_roles/down.sql @@ -0,0 +1,3 @@ +-- This is an idempotent data repair. Reverting it must not remove permissions or recreate the +-- invalid persisted Manager type; the older-schema migration performs its own safe conversion. +SELECT 1; diff --git a/migrations/sqlite/2026-07-23-120000_reconcile_legacy_custom_roles/up.sql b/migrations/sqlite/2026-07-23-120000_reconcile_legacy_custom_roles/up.sql new file mode 100644 index 00000000..6d75889c --- /dev/null +++ b/migrations/sqlite/2026-07-23-120000_reconcile_legacy_custom_roles/up.sql @@ -0,0 +1,65 @@ +-- 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 +); +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; + +-- 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, +-- remove that deterministic copy so later group removal also revokes the authority. +UPDATE users_organizations +SET edit_any_collection = FALSE, + delete_any_collection = FALSE +WHERE atype IN (3, 4) + AND access_all = FALSE + AND create_new_collections = FALSE + AND edit_any_collection = TRUE + AND delete_any_collection = TRUE + AND EXISTS (SELECT 1 FROM __vw_custom_role_same_run_0716 WHERE marker = 1) + AND EXISTS ( + SELECT 1 + FROM groups_users AS gu + INNER JOIN "groups" AS g ON g.uuid = gu.groups_uuid + WHERE gu.users_organizations_uuid = users_organizations.uuid + AND g.organizations_uuid = users_organizations.org_uuid + AND g.access_all = TRUE + ); + +-- A remaining 0/1/1 pattern may be either an intentional direct grant or an older derived grant +-- whose source group has already been removed. Do not guess which one it is. +CREATE TEMPORARY TABLE __vw_legacy_group_access_guard ( + blocked INTEGER NOT NULL PRIMARY KEY +); +INSERT INTO __vw_legacy_group_access_guard (blocked) VALUES (1); +INSERT INTO __vw_legacy_group_access_guard (blocked) +SELECT 1 +FROM users_organizations +WHERE atype IN (3, 4) + AND access_all = FALSE + AND create_new_collections = FALSE + AND edit_any_collection = TRUE + AND delete_any_collection = TRUE +LIMIT 1; +DROP TABLE __vw_legacy_group_access_guard; + +-- Membership access_all on a legacy Manager/Custom represented all three collection capabilities. +-- Set only TRUE values so this repair never removes independently configured permissions. +UPDATE users_organizations +SET create_new_collections = TRUE, + edit_any_collection = TRUE, + delete_any_collection = TRUE +WHERE atype IN (3, 4) + AND access_all = TRUE; + +-- Convert only after the legacy bit has been copied. +UPDATE users_organizations SET atype = 4 WHERE atype = 3; + +-- Clear the same-run marker only after every guard and permission update succeeds. +DELETE FROM __vw_custom_role_same_run_0716 WHERE marker = 1; diff --git a/migrations/sqlite/2026-07-24-120000_drop_membership_access_all/down.sql b/migrations/sqlite/2026-07-24-120000_drop_membership_access_all/down.sql new file mode 100644 index 00000000..01295bf5 --- /dev/null +++ b/migrations/sqlite/2026-07-24-120000_drop_membership_access_all/down.sql @@ -0,0 +1,6 @@ +-- 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`. +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; diff --git a/migrations/sqlite/2026-07-24-120000_drop_membership_access_all/up.sql b/migrations/sqlite/2026-07-24-120000_drop_membership_access_all/up.sql new file mode 100644 index 00000000..e11fb611 --- /dev/null +++ b/migrations/sqlite/2026-07-24-120000_drop_membership_access_all/up.sql @@ -0,0 +1,5 @@ +-- The membership `access_all` flag was Vaultwarden's pre-permissions patch for "this member can +-- reach every collection". It is now fully represented by the role model: Owners/Admins hold it +-- implicitly, and a Custom member holds it via `edit_any_collection`. Drop the redundant column. +-- This only concerns users_organizations; groups.access_all is a separate, still-supported feature. +ALTER TABLE users_organizations DROP COLUMN access_all; diff --git a/migrations/sqlite/2026-07-24-130000_add_custom_access_permissions/down.sql b/migrations/sqlite/2026-07-24-130000_add_custom_access_permissions/down.sql new file mode 100644 index 00000000..f276ea5b --- /dev/null +++ b/migrations/sqlite/2026-07-24-130000_add_custom_access_permissions/down.sql @@ -0,0 +1,3 @@ +ALTER TABLE users_organizations DROP COLUMN access_event_logs; +ALTER TABLE users_organizations DROP COLUMN access_import_export; +ALTER TABLE users_organizations DROP COLUMN access_reports; diff --git a/migrations/sqlite/2026-07-24-130000_add_custom_access_permissions/up.sql b/migrations/sqlite/2026-07-24-130000_add_custom_access_permissions/up.sql new file mode 100644 index 00000000..9d9c31ff --- /dev/null +++ b/migrations/sqlite/2026-07-24-130000_add_custom_access_permissions/up.sql @@ -0,0 +1,5 @@ +-- Three additional Bitwarden Custom-role permissions. They are only meaningful for Custom members +-- (gated on the role in code); Owners/Admins hold every permission implicitly. +ALTER TABLE users_organizations ADD COLUMN access_event_logs BOOLEAN NOT NULL DEFAULT FALSE; +ALTER TABLE users_organizations ADD COLUMN access_import_export BOOLEAN NOT NULL DEFAULT FALSE; +ALTER TABLE users_organizations ADD COLUMN access_reports BOOLEAN NOT NULL DEFAULT FALSE; diff --git a/migrations/sqlite/2026-07-24-140000_guard_custom_role_downgrade/down.sql b/migrations/sqlite/2026-07-24-140000_guard_custom_role_downgrade/down.sql new file mode 100644 index 00000000..4eb19e97 --- /dev/null +++ b/migrations/sqlite/2026-07-24-140000_guard_custom_role_downgrade/down.sql @@ -0,0 +1,7 @@ +-- Nine independent Custom-role permissions cannot be represented losslessly by the legacy +-- role/access_all schema. Always stop before any older down migration removes permission data. +CREATE TEMPORARY TABLE __vw_custom_role_downgrade_guard ( + blocked INTEGER NOT NULL PRIMARY KEY +); +INSERT INTO __vw_custom_role_downgrade_guard (blocked) VALUES (1); +INSERT INTO __vw_custom_role_downgrade_guard (blocked) VALUES (1); diff --git a/migrations/sqlite/2026-07-24-140000_guard_custom_role_downgrade/up.sql b/migrations/sqlite/2026-07-24-140000_guard_custom_role_downgrade/up.sql new file mode 100644 index 00000000..af5fed1b --- /dev/null +++ b/migrations/sqlite/2026-07-24-140000_guard_custom_role_downgrade/up.sql @@ -0,0 +1,3 @@ +-- Forward migration marker. Its down migration intentionally blocks an automatic lossy downgrade +-- before any granular permission column is removed. +SELECT 1; diff --git a/src/api/admin.rs b/src/api/admin.rs index 8c60e0c7..5989efdb 100644 --- a/src/api/admin.rs +++ b/src/api/admin.rs @@ -545,38 +545,29 @@ struct MembershipTypeData { } fn apply_membership_type_change(membership: &mut Membership, new_type: MembershipType) { - let was_custom = membership.atype == MembershipType::Custom; - - // Leaving Custom for the legacy Manager role: `access_all` is the internal mirror of - // Edit any collection, but a Manager's `access_all` means the broad "manage all collections" - // grant (Create + Edit + Delete). Carrying an Edit-only mirror over would silently escalate the - // member to Create/Edit/Delete. Only preserve `access_all` when the member actually held the - // full manage-all grant, mirroring the collection-permissions down-migration. Must be evaluated - // before the flags are cleared below (and while the type is still Custom). - if was_custom && new_type == MembershipType::Manager { - membership.access_all = membership.has_manage_all_collections(); - } - - // Entering Custom through the Vaultwarden admin panel is deliberately fail-closed because - // that UI cannot select granular permissions; they can be granted later through the regular - // organization member dialog. - if new_type == MembershipType::Custom && !was_custom { - membership.clear_custom_permissions(); - membership.access_all = false; - } - if new_type != MembershipType::Custom { + // Entering Custom through the Vaultwarden admin panel is deliberately fail-closed because that + // UI cannot select granular permissions; they can be granted later through the regular + // organization member dialog. Any non-Custom role carries no custom flags at all. Only a member + // that is already Custom and stays Custom keeps its existing flags. + let stays_custom = new_type == MembershipType::Custom && membership.atype == MembershipType::Custom; + if !stays_custom { membership.clear_custom_permissions(); } - // Prevent stale access_all from surviving a demotion to User. Admins/Owners have implicit - // full access, while legacy Manager access_all is intentionally preserved for compatibility. - match new_type { - MembershipType::Owner | MembershipType::Admin => membership.access_all = true, - MembershipType::User => membership.access_all = false, - MembershipType::Manager | MembershipType::Custom => {} + membership.atype = new_type as i32; +} + +fn parse_admin_membership_type(user_type: NumberOrString) -> Option { + let raw_type = user_type.into_string(); + + // The public API still accepts the legacy Manager representation for compatibility and folds + // it into Custom. The admin panel must not do that: treating an apparent Manager demotion as a + // Custom-to-Custom update would preserve the member's existing granular permissions. + if matches!(raw_type.as_str(), "3" | "Manager") { + return None; } - membership.atype = new_type as i32; + MembershipType::from_str(&raw_type) } #[post("/users/org_type", format = "application/json", data = "")] @@ -588,7 +579,7 @@ async fn update_membership_type(data: Json, token: AdminToke err!("The specified user isn't member of the organization") }; - let Some(new_type) = MembershipType::from_str(&data.user_type.into_string()) else { + let Some(new_type) = parse_admin_membership_type(data.user_type) else { err!("Invalid type") }; @@ -936,9 +927,8 @@ mod tests { } #[test] - fn admin_type_changes_clear_custom_permissions_and_stale_access() { + fn admin_type_changes_clear_custom_permissions() { let mut custom = membership(MembershipType::Custom); - custom.access_all = true; custom.manage_users = true; custom.create_new_collections = true; custom.edit_any_collection = true; @@ -946,50 +936,48 @@ mod tests { apply_membership_type_change(&mut custom, MembershipType::User); assert_eq!(custom.atype, MembershipType::User as i32); - assert!(!custom.access_all); assert!(!custom.manage_users); assert!(!custom.create_new_collections); assert!(!custom.edit_any_collection); assert!(!custom.delete_any_collection); + // Entering Custom through the admin panel is fail-closed: no granular permissions are set. let mut admin = membership(MembershipType::Admin); - admin.access_all = true; apply_membership_type_change(&mut admin, MembershipType::Custom); assert_eq!(admin.atype, MembershipType::Custom as i32); - assert!(!admin.access_all, "entering Custom through this UI must be fail-closed"); assert!(!admin.has_manage_all_collections()); + assert!(!admin.edit_any_collection); + } + + #[test] + fn admin_custom_to_custom_keeps_flags_but_other_transitions_clear() { + // A member kept as Custom retains its granular flags: the admin panel does not touch them; + // they are managed through the regular organization member dialog. + let mut custom = membership(MembershipType::Custom); + custom.manage_users = true; + custom.edit_any_collection = true; + apply_membership_type_change(&mut custom, MembershipType::Custom); + assert_eq!(custom.atype, MembershipType::Custom as i32); + assert!(custom.manage_users); + assert!(custom.edit_any_collection); + + // Promoting to Admin/Owner drops any stale custom flags. + let mut promo = membership(MembershipType::Custom); + promo.edit_any_collection = true; + apply_membership_type_change(&mut promo, MembershipType::Admin); + assert_eq!(promo.atype, MembershipType::Admin as i32); + assert!(!promo.edit_any_collection); } #[test] - fn admin_and_legacy_manager_access_all_behavior_is_preserved() { - let mut user = membership(MembershipType::User); - apply_membership_type_change(&mut user, MembershipType::Admin); - assert!(user.access_all); - - // REGRESSION (privilege escalation, PR #7397 / finding F2): a Custom member whose - // `access_all` is only the Edit-any-collection mirror must NOT be turned into a legacy - // Manager with the broad "manage all collections" `access_all` grant. That would escalate - // an Edit-only member into Create + Edit + Delete. Mirrors the collection down-migration. - let mut edit_only = membership(MembershipType::Custom); - edit_only.access_all = true; - edit_only.edit_any_collection = true; - apply_membership_type_change(&mut edit_only, MembershipType::Manager); - assert_eq!(edit_only.atype, MembershipType::Manager as i32); - assert!(!edit_only.access_all, "Edit-only Custom must not become an access_all Manager"); - assert!(!edit_only.edit_any_collection); - - // A Custom member who genuinely held the full manage-all grant (all three collection - // flags) keeps the equivalent legacy Manager `access_all`. - let mut manage_all = membership(MembershipType::Custom); - manage_all.access_all = true; - manage_all.create_new_collections = true; - manage_all.edit_any_collection = true; - manage_all.delete_any_collection = true; - apply_membership_type_change(&mut manage_all, MembershipType::Manager); - assert_eq!(manage_all.atype, MembershipType::Manager as i32); - assert!(manage_all.access_all, "full manage-all Custom keeps legacy Manager access_all"); - assert!(!manage_all.create_new_collections); - assert!(!manage_all.edit_any_collection); - assert!(!manage_all.delete_any_collection); + fn admin_type_parser_rejects_legacy_manager_before_normalization() { + assert!(parse_admin_membership_type(NumberOrString::Number(3)).is_none()); + assert!(parse_admin_membership_type(NumberOrString::String("3".to_owned())).is_none()); + assert!(parse_admin_membership_type(NumberOrString::String("Manager".to_owned())).is_none()); + + assert!(parse_admin_membership_type(NumberOrString::Number(4)) == Some(MembershipType::Custom)); + assert!( + parse_admin_membership_type(NumberOrString::String("Custom".to_owned())) == Some(MembershipType::Custom) + ); } } diff --git a/src/api/core/events.rs b/src/api/core/events.rs index 5518fa3c..ba859d31 100644 --- a/src/api/core/events.rs +++ b/src/api/core/events.rs @@ -7,12 +7,15 @@ 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}, + models::{ + Cipher, CipherId, Event, EventType, Membership, MembershipId, MembershipStatus, MembershipType, + OrganizationId, UserId, + }, }, - util::parse_date, + util::try_parse_date, }; /// ############################################################################################################### @@ -29,9 +32,36 @@ struct EventRange { continuation_token: Option, } +fn parse_event_date(date: &str, field: &str) -> Result { + try_parse_date(date) + .map_err(|error| crate::Error::new("Invalid event date", format!("Invalid RFC 3339 {field}: {error}"))) +} + +fn parse_event_range(data: &EventRange) -> Result<(NaiveDateTime, NaiveDateTime), crate::Error> { + let start_date = parse_event_date(&data.start, "start date")?; + + let end_date = if let Some(continuation_token) = &data.continuation_token { + try_parse_date(continuation_token).map_err(|error| { + crate::Error::new( + "Invalid continuation token", + format!("Continuation token is not a valid RFC 3339 date: {error}"), + ) + })? + } else { + parse_event_date(&data.end, "end date")? + }; + + Ok((start_date, end_date)) +} + // 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"); } @@ -39,12 +69,7 @@ async fn get_org_events(org_id: OrganizationId, data: EventRange, headers: Admin // Return an empty vec when we org events are disabled. // This prevents client errors let events_json: Vec = if CONFIG.org_events_enabled() { - let start_date = parse_date(&data.start); - let end_date = if let Some(before_date) = &data.continuation_token { - parse_date(before_date) - } else { - parse_date(&data.end) - }; + let (start_date, end_date) = parse_event_range(&data)?; Event::find_by_organization_uuid(&org_id, &start_date, &end_date, &conn) .await @@ -62,21 +87,70 @@ async fn get_org_events(org_id: OrganizationId, data: EventRange, headers: Admin }))) } +#[derive(Debug, Eq, PartialEq)] +enum CipherEventScope { + Organization(OrganizationId), + Personal, +} + +impl CipherEventScope { + fn includes(&self, event: &Event) -> bool { + match self { + Self::Organization(org_id) => event.org_uuid.as_ref() == Some(org_id), + Self::Personal => event.org_uuid.is_none(), + } + } +} + +fn membership_can_access_event_logs(membership: &Membership) -> bool { + membership.has_status(MembershipStatus::Confirmed) + && (membership.atype >= MembershipType::Admin || membership.has_access_event_logs()) +} + +fn cipher_event_scope(cipher: &Cipher, user_id: &UserId, membership: Option<&Membership>) -> Option { + match &cipher.organization_uuid { + Some(org_id) + if membership.is_some_and(|membership| { + membership.user_uuid == *user_id + && membership.org_uuid == *org_id + && membership_can_access_event_logs(membership) + }) => + { + Some(CipherEventScope::Organization(org_id.clone())) + } + None if cipher.is_owned_by_user(user_id) => Some(CipherEventScope::Personal), + _ => None, + } +} + #[get("/ciphers//events?")] async fn get_cipher_events(cipher_id: CipherId, data: EventRange, headers: Headers, conn: DbConn) -> JsonResult { // Return an empty vec when org events are disabled. // This prevents client errors - let events_json: Vec = if CONFIG.org_events_enabled() - && Membership::user_has_ge_admin_access_to_cipher(&headers.user.uuid, &cipher_id, &conn).await - { - let start_date = parse_date(&data.start); - let end_date = if let Some(before_date) = &data.continuation_token { - parse_date(before_date) + let events_json: Vec = if CONFIG.org_events_enabled() { + let (start_date, end_date) = parse_event_range(&data)?; + + let scope = if let Some(cipher) = Cipher::find_by_uuid(&cipher_id, &conn).await { + let membership = if let Some(org_id) = &cipher.organization_uuid { + Membership::find_by_user_and_org(&headers.user.uuid, org_id, &conn).await + } else { + None + }; + cipher_event_scope(&cipher, &headers.user.uuid, membership.as_ref()) } else { - parse_date(&data.end) + None }; - Event::find_by_cipher_uuid(&cipher_id, &start_date, &end_date, &conn).await.iter().map(Event::to_json).collect() + if let Some(scope) = scope { + Event::find_by_cipher_uuid(&cipher_id, &start_date, &end_date, &conn) + .await + .iter() + .filter(|event| scope.includes(event)) + .map(Event::to_json) + .collect() + } else { + Vec::new() + } } else { Vec::new() }; @@ -93,21 +167,17 @@ 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 { err!("Organization not found", "Organization id's do not match"); } + // Return an empty vec when we org events are disabled. // This prevents client errors let events_json: Vec = if CONFIG.org_events_enabled() { - let start_date = parse_date(&data.start); - let end_date = if let Some(before_date) = &data.continuation_token { - parse_date(before_date) - } else { - parse_date(&data.end) - }; + let (start_date, end_date) = parse_event_range(&data)?; Event::find_by_org_and_member(&org_id, &member_id, &start_date, &end_date, &conn) .await @@ -158,6 +228,48 @@ struct EventCollection { organization_id: Option, } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum ClientEventKind { + User, + Cipher, + Organization, +} + +const MAX_CLIENT_EVENT_BATCH_SIZE: usize = 1_000; + +fn validate_client_event_batch_size(event_count: usize) -> Result<(), crate::Error> { + if event_count > MAX_CLIENT_EVENT_BATCH_SIZE { + return Err(crate::Error::new( + "Event batch is too large", + format!("At most {MAX_CLIENT_EVENT_BATCH_SIZE} events are accepted per request"), + )); + } + Ok(()) +} + +fn client_event_kind(event_type: i32) -> Option { + match event_type { + event_type if event_type == EventType::UserClientExportedVault as i32 => Some(ClientEventKind::User), + event_type + if event_type == EventType::CipherClientViewed as i32 + || event_type == EventType::CipherClientToggledPasswordVisible as i32 + || event_type == EventType::CipherClientToggledHiddenFieldVisible as i32 + || event_type == EventType::CipherClientToggledCardCodeVisible as i32 + || event_type == EventType::CipherClientCopiedPassword as i32 + || event_type == EventType::CipherClientCopiedHiddenField as i32 + || event_type == EventType::CipherClientCopiedCardCode as i32 + || event_type == EventType::CipherClientAutofilled as i32 + || event_type == EventType::CipherClientToggledCardNumberVisible as i32 => + { + Some(ClientEventKind::Cipher) + } + event_type if event_type == EventType::OrganizationClientExportedVault as i32 => { + Some(ClientEventKind::Organization) + } + _ => None, + } +} + // Upstream: // https://github.com/bitwarden/server/blob/9ebe16587175b1c0e9208f84397bb75d0d595510/src/Events/Controllers/CollectController.cs // https://github.com/bitwarden/server/blob/9ebe16587175b1c0e9208f84397bb75d0d595510/src/Core/AdminConsole/Services/Implementations/EventService.cs @@ -167,10 +279,25 @@ async fn post_events_collect(data: Json>, headers: Headers, return Ok(()); } + // Official clients normally submit small batches (upstream explicitly exercises batches of + // 100). Keep ample headroom while preventing one authenticated request from causing an + // effectively unbounded sequence of database reads and writes under the shared 20 MiB JSON + // limit. + validate_client_event_batch_size(data.len())?; + + // Validate all accepted client events before writing any of them. Unsupported event types are + // ignored, matching upstream, while malformed dates on accepted events produce a controlled + // 400 response instead of panicking after a partially processed batch. + let mut accepted_events = Vec::new(); for event in data.iter() { - let event_date = parse_date(&event.date); - match event.r#type { - 1000..=1099 => { + if let Some(kind) = client_event_kind(event.r#type) { + accepted_events.push((event, kind, parse_event_date(&event.date, "event date")?)); + } + } + + for (event, kind, event_date) in accepted_events { + match kind { + ClientEventKind::User => { log_user_event_impl( event.r#type, &headers.user.uuid, @@ -181,7 +308,7 @@ async fn post_events_collect(data: Json>, headers: Headers, ) .await; } - 1600..=1699 => { + ClientEventKind::Organization => { // Only allow logging events for an organization the user is actually a member of. if let Some(org_id) = &event.organization_id && Membership::find_confirmed_by_user_and_org(&headers.user.uuid, org_id, &conn).await.is_some() @@ -199,7 +326,7 @@ async fn post_events_collect(data: Json>, headers: Headers, .await; } } - _ => { + ClientEventKind::Cipher => { // The cipher determines the organization the event is logged to, so make sure the // user can actually access it instead of trusting the provided cipher uuid. if let Some(cipher_uuid) = &event.cipher_id @@ -341,3 +468,152 @@ pub async fn event_cleanup_job(pool: DbPool) { error!("Failed to get DB connection while trying to cleanup the events table"); } } + +#[cfg(test)] +mod tests { + use super::*; + + fn membership(member_type: MembershipType, status: MembershipStatus) -> Membership { + let mut membership = Membership::new("test-user".to_owned().into(), "test-org".to_owned().into(), None); + membership.atype = member_type as i32; + membership.status = status as i32; + membership + } + + #[test] + fn cipher_event_access_requires_confirmed_admin_or_access_event_logs() { + for member_type in [MembershipType::Owner, MembershipType::Admin] { + assert!(membership_can_access_event_logs(&membership(member_type, MembershipStatus::Confirmed))); + assert!(!membership_can_access_event_logs(&membership(member_type, MembershipStatus::Invited))); + assert!(!membership_can_access_event_logs(&membership(member_type, MembershipStatus::Accepted))); + assert!(!membership_can_access_event_logs(&membership(member_type, MembershipStatus::Revoked))); + } + + let mut custom = membership(MembershipType::Custom, MembershipStatus::Confirmed); + assert!(!membership_can_access_event_logs(&custom)); + custom.access_event_logs = true; + assert!(membership_can_access_event_logs(&custom)); + + custom.status = MembershipStatus::Revoked as i32; + assert!(!membership_can_access_event_logs(&custom)); + assert!(!membership_can_access_event_logs(&membership(MembershipType::User, MembershipStatus::Confirmed))); + } + + #[test] + fn cipher_event_scope_is_bound_to_cipher_org_or_personal_owner() { + let user_id: UserId = "test-user".to_owned().into(); + let org_id: OrganizationId = "test-org".to_owned().into(); + let mut cipher = Cipher::new(1, "test-cipher".to_owned()); + cipher.organization_uuid = Some(org_id.clone()); + + let admin = membership(MembershipType::Admin, MembershipStatus::Confirmed); + assert_eq!(cipher_event_scope(&cipher, &user_id, Some(&admin)), Some(CipherEventScope::Organization(org_id))); + + let accepted_admin = membership(MembershipType::Admin, MembershipStatus::Accepted); + assert_eq!(cipher_event_scope(&cipher, &user_id, Some(&accepted_admin)), None); + + let mut foreign_membership = membership(MembershipType::Admin, MembershipStatus::Confirmed); + foreign_membership.org_uuid = "other-org".to_owned().into(); + assert_eq!(cipher_event_scope(&cipher, &user_id, Some(&foreign_membership)), None); + + cipher.organization_uuid = None; + cipher.user_uuid = Some(user_id.clone()); + assert_eq!(cipher_event_scope(&cipher, &user_id, None), Some(CipherEventScope::Personal)); + assert_eq!(cipher_event_scope(&cipher, &"other-user".to_owned().into(), None), None); + } + + #[test] + fn cipher_event_rows_must_match_the_authorized_scope() { + let org_id: OrganizationId = "test-org".to_owned().into(); + let mut event = Event::new(EventType::CipherClientViewed as i32, None); + + assert!(CipherEventScope::Personal.includes(&event)); + event.org_uuid = Some(org_id.clone()); + assert!(!CipherEventScope::Personal.includes(&event)); + assert!(CipherEventScope::Organization(org_id).includes(&event)); + assert!(!CipherEventScope::Organization("other-org".to_owned().into()).includes(&event)); + } + + #[test] + fn event_range_rejects_invalid_dates_and_continuation_tokens() { + let valid = EventRange { + start: "2026-07-25T10:00:00Z".to_owned(), + end: "2026-07-25T11:00:00Z".to_owned(), + continuation_token: None, + }; + assert!(parse_event_range(&valid).is_ok()); + + let invalid_start = EventRange { + start: "not-a-date".to_owned(), + ..valid + }; + assert!(parse_event_range(&invalid_start).is_err()); + + let invalid_end = EventRange { + start: "2026-07-25T10:00:00Z".to_owned(), + end: "not-a-date".to_owned(), + continuation_token: None, + }; + assert!(parse_event_range(&invalid_end).is_err()); + + let invalid_token = EventRange { + start: "2026-07-25T10:00:00Z".to_owned(), + end: "2026-07-25T11:00:00Z".to_owned(), + continuation_token: Some("not-a-date".to_owned()), + }; + assert!(parse_event_range(&invalid_token).is_err()); + + let token_supersedes_end = EventRange { + start: "2026-07-25T10:00:00Z".to_owned(), + end: "legacy-client-value-that-is-not-used".to_owned(), + continuation_token: Some("2026-07-25T10:30:00Z".to_owned()), + }; + assert!(parse_event_range(&token_supersedes_end).is_ok()); + } + + #[test] + fn collect_accepts_only_official_client_generated_event_types() { + assert_eq!(client_event_kind(EventType::UserClientExportedVault as i32), Some(ClientEventKind::User)); + for event_type in [ + EventType::CipherClientViewed, + EventType::CipherClientToggledPasswordVisible, + EventType::CipherClientToggledHiddenFieldVisible, + EventType::CipherClientToggledCardCodeVisible, + EventType::CipherClientCopiedPassword, + EventType::CipherClientCopiedHiddenField, + EventType::CipherClientCopiedCardCode, + EventType::CipherClientAutofilled, + EventType::CipherClientToggledCardNumberVisible, + ] { + assert_eq!(client_event_kind(event_type as i32), Some(ClientEventKind::Cipher)); + } + assert_eq!( + client_event_kind(EventType::OrganizationClientExportedVault as i32), + Some(ClientEventKind::Organization) + ); + + for event_type in [ + EventType::UserLoggedIn, + EventType::UserChangedPassword, + EventType::CipherCreated, + EventType::CipherUpdated, + EventType::CipherDeleted, + EventType::OrganizationUpdated, + EventType::OrganizationPurgedVault, + EventType::PolicyUpdated, + ] { + assert_eq!(client_event_kind(event_type as i32), None); + } + assert_eq!(client_event_kind(1099), None); + assert_eq!(client_event_kind(1199), None); + assert_eq!(client_event_kind(1699), None); + } + + #[test] + fn collect_batch_limit_preserves_normal_batches_and_rejects_excess() { + assert!(validate_client_event_batch_size(0).is_ok()); + assert!(validate_client_event_batch_size(100).is_ok()); + assert!(validate_client_event_batch_size(MAX_CLIENT_EVENT_BATCH_SIZE).is_ok()); + assert!(validate_client_event_batch_size(MAX_CLIENT_EVENT_BATCH_SIZE + 1).is_err()); + } +} diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index 0d36d9bb..8e7c8057 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -12,9 +12,9 @@ use crate::{ core::{CipherSyncData, CipherSyncType, accept_org_invite, log_event, two_factor}, }, auth::{ - AdminHeaders, CollectionDeleteHeaders, CollectionReadHeaders, Headers, ManageGroupsHeaders, - ManagePoliciesHeaders, ManageUsersHeaders, ManageUsersOrGroupsHeaders, ManagerHeaders, ManagerHeadersLoose, - OrgMemberHeaders, OwnerHeaders, decode_invite, + AccessImportExportHeaders, AdminHeaders, CollectionDeleteHeaders, CollectionReadHeaders, Headers, + ManageGroupsHeaders, ManagePoliciesHeaders, ManageUsersHeaders, ManageUsersOrGroupsHeaders, ManagerHeaders, + ManagerHeadersLoose, OrgMemberHeaders, OwnerHeaders, decode_invite, }, db::{ DbConn, @@ -219,7 +219,6 @@ async fn create_organization(headers: Headers, data: Json, conn: DbConn let collection = Collection::new(org.uuid.clone(), data.collection_name, None); member.akey = data.key; - member.access_all = true; member.atype = MembershipType::Owner as i32; member.status = MembershipStatus::Confirmed as i32; @@ -539,8 +538,8 @@ async fn post_organization_collections( err!("Organization not found", "Organization id's do not match"); } - // Create is independent from Edit/Delete. In particular, Edit any collection's internal - // access_all representation must not implicitly grant this endpoint. + // Create is independent from Edit/Delete. In particular, Edit any collection (full access to + // every collection) must not implicitly grant this endpoint. if !headers.membership.can_create_new_collections() { err!("You don't have permission to create collections") } @@ -565,27 +564,25 @@ async fn post_organization_collections( let collection = Collection::new(org_id.clone(), data.name, data.external_id); collection.save(&conn).await?; - log_event( - EventType::CollectionCreated as i32, - &collection.uuid, - &org_id, - &headers.user.uuid, - headers.device.atype, - &headers.ip.ip, - &conn, - ) - .await; - // Security (F-3): a `manage` grant carries collection *delete*/administer authority // (`has_explicit_collection_manage_access` -> CollectionDeleteHeaders/ManagerHeaders), so only a // caller who could delete this collection may confer it — the same rule the collection-update and // bulk-access endpoints apply. Create is deliberately independent from Edit/Delete, so a Custom // member holding only `create_new_collections` must not be able to hand a manage row to another - // member or to a group (nor to itself) while creating the collection. For such callers the - // requested `manage` is forced to false; Admin/Owner, Custom-with-`delete_any_collection` and the - // legacy access_all Manager keep it. Evaluated after the collection exists so the per-collection - // lookup sees it. + // member or to a group while creating the collection. For such callers the requested `manage` + // is forced to false; Admin/Owner and Custom-with-`delete_any_collection` keep it. The creator's + // own object-scoped ownership is added separately below. Evaluated after the collection exists + // so the per-collection lookup sees it. let may_grant_manage = caller_may_grant_collection_manage(&headers.membership, &collection.uuid, &conn).await; + let creator_needs_assignment = !headers.membership.has_full_access(); + + // Persist the creator's object-scoped ownership before secondary assignments. If a later + // assignment write fails, the otherwise non-transactional create path still leaves the new + // collection recoverably manageable by its creator. An explicit self-assignment below is + // skipped so it cannot weaken this grant. + if creator_needs_assignment { + CollectionUser::save(&headers.membership.user_uuid, &collection.uuid, false, false, true, &conn).await?; + } for group in data.groups { CollectionGroup::new( @@ -604,7 +601,10 @@ async fn post_organization_collections( err!("User is not part of organization") }; - if member.access_all { + if member.grants_access_to_all_collections() { + continue; + } + if member.user_uuid == headers.membership.user_uuid && creator_needs_assignment { continue; } @@ -619,6 +619,19 @@ async fn post_organization_collections( .await?; } + // Emit the success event only after all requested assignments and the creator's object-scoped + // manage grant have been persisted. A later write failure must not leave a false audit record. + log_event( + EventType::CollectionCreated as i32, + &collection.uuid, + &org_id, + &headers.user.uuid, + headers.device.atype, + &headers.ip.ip, + &conn, + ) + .await; + Ok(Json(collection.to_json_details(&headers.membership.user_uuid, None, &conn).await)) } @@ -648,13 +661,12 @@ async fn post_bulk_access_collections( // Security (F-1): authorization is enforced per collection below via `auth::can_edit_collection`, // the exact same Custom-aware check the single-collection edit endpoint (`ManagerHeaders`) uses. - // Edit any collection (or Admin/Owner) may bulk-edit every collection; a legacy Manager keeps its - // broad per-collection helper; any other Custom member must hold a real per-collection Manage - // grant. In particular a Custom member's membership/group `access_all` does NOT satisfy this here - // (it did under the previous `is_manageable_by_user` check, which diverged from the single-edit - // endpoint). A custom user with only manage_users / manage_groups / manage_policies holds no such - // grant and is rejected, while a member who manages some collections keeps the ability to - // bulk-edit exactly those. + // Edit any collection (or Admin/Owner) may bulk-edit every collection; any other Custom member + // must hold a real per-collection Manage grant. In particular a Custom member's group + // `access_all` does NOT satisfy this here (it did under the previous `is_manageable_by_user` + // check, which diverged from the single-edit endpoint). A custom user with only manage_users / + // manage_groups / manage_policies holds no such grant and is rejected, while a member who manages + // some collections keeps the ability to bulk-edit exactly those. // Security (audit H-3) and atomicity (audit M-2): validate the whole request against this // organization *before* mutating anything. Every collection must exist in the org and be @@ -662,10 +674,10 @@ async fn post_bulk_access_collections( // once the entire request is known-valid do we begin the destructive delete/replace of // assignments, so a foreign-tenant group can never be linked and a later invalid element can no // longer leave earlier collections with their assignments already wiped. - for group in &data.groups { - if Group::find_by_uuid_and_org(&group.id, &org_id, &conn).await.is_none() { - err!("Group not found in this organization") - } + let org_groups = Group::find_by_organization(&org_id, &conn).await; + let org_group_ids: HashSet<&GroupId> = org_groups.iter().map(|g| &g.uuid).collect(); + if let Some(g) = data.groups.iter().find(|g| !org_group_ids.contains(&g.id)) { + err!("Invalid group", format!("Group {} does not belong to organization {}!", g.id, org_id)) } for user in &data.users { if Membership::find_by_uuid_and_org(&user.id, &org_id, &conn).await.is_none() { @@ -726,7 +738,7 @@ async fn post_bulk_access_collections( err!("User is not part of organization") }; - if member.access_all { + if member.grants_access_to_all_collections() { continue; } @@ -826,7 +838,7 @@ async fn post_organization_collection_update( err!("User is not part of organization") }; - if member.access_all { + if member.grants_access_to_all_collections() { continue; } @@ -1057,14 +1069,38 @@ async fn get_assigned_org_details(data: OrgIdData, headers: Headers, conn: DbCon }))) } +// 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. +// +// `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. +// +// 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`) +// +// 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. #[get("/ciphers/organization-details?")] 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() { - err_code!("Resource not found.", "User does not have full access", rocket::http::Status::NotFound.code); + if !headers.membership.has_full_access() && !headers.membership.has_access_reports() { + err_code!( + "Resource not found.", + "User does not have permission to access all organization ciphers", + rocket::http::Status::NotFound.code + ); } Ok(Json(json!({ @@ -1080,7 +1116,18 @@ async fn get_org_details_impl( user_id: &UserId, conn: &DbConn, ) -> Result { - let ciphers = Cipher::find_by_org(org_id, conn).await; + ciphers_to_org_json(Cipher::find_by_org(org_id, conn).await, host, user_id, conn).await +} + +// Serialize an already-authorized set of organization ciphers. The caller decides which ciphers go +// in: `CipherSyncType::Organization` skips the per-cipher access restrictions, so this must never be +// handed a cipher the user is not allowed to see. +async fn ciphers_to_org_json( + ciphers: Vec, + host: &str, + user_id: &UserId, + conn: &DbConn, +) -> Result { let cipher_sync_data = CipherSyncData::new(user_id, CipherSyncType::Organization, conn).await; let mut ciphers_json = Vec::with_capacity(ciphers.len()); @@ -1193,6 +1240,9 @@ struct CustomRolePermissions { create_new_collections: bool, edit_any_collection: bool, delete_any_collection: bool, + access_event_logs: bool, + access_import_export: bool, + access_reports: bool, } impl CustomRolePermissions { @@ -1209,16 +1259,47 @@ impl CustomRolePermissions { create_new_collections: enabled("createNewCollections"), edit_any_collection: enabled("editAnyCollection"), delete_any_collection: enabled("deleteAnyCollection"), + access_event_logs: enabled("accessEventLogs"), + access_import_export: enabled("accessImportExport"), + access_reports: enabled("accessReports"), } } - /// Bitwarden grants a Custom member with Edit any collection full read/edit/manage access to - /// organization ciphers. Vaultwarden's existing access_all flag is the internal data-plane - /// representation of that capability. Create and Delete remain completely independent. - fn access_all_for(self, member_type: MembershipType) -> bool { + /// Whether the requested role/permissions give this member access to *every* collection in the + /// org: Admins/Owners implicitly, and a Custom member holding Edit any collection. Such members + /// do not need (and must not be given) individual per-collection assignments. Create and Delete + /// remain completely independent of this. + fn grants_full_collection_access(self, member_type: MembershipType) -> bool { member_type >= MembershipType::Admin || (member_type == MembershipType::Custom && self.edit_any_collection) } + /// Parse permissions for an existing member without treating an omitted permissions object as + /// an instruction to clear every Custom-role grant. Older clients send legacy role value `3` + /// without the modern object; that value is normalized to Custom for compatibility. + fn from_edit_request( + member_type: MembershipType, + permissions: Option<&HashMap>, + membership: &Membership, + ) -> Self { + match permissions { + Some(permissions) => Self::from_request(member_type, permissions), + None if member_type == MembershipType::Custom && membership.atype == MembershipType::Custom as i32 => { + Self { + manage_users: membership.manage_users, + manage_groups: membership.manage_groups, + manage_policies: membership.manage_policies, + create_new_collections: membership.create_new_collections, + edit_any_collection: membership.edit_any_collection, + delete_any_collection: membership.delete_any_collection, + access_event_logs: membership.access_event_logs, + access_import_export: membership.access_import_export, + access_reports: membership.access_reports, + } + } + None => Self::default(), + } + } + fn differs_from(self, membership: &Membership) -> bool { self.manage_users != membership.manage_users || self.manage_groups != membership.manage_groups @@ -1226,6 +1307,9 @@ impl CustomRolePermissions { || self.create_new_collections != membership.create_new_collections || self.edit_any_collection != membership.edit_any_collection || self.delete_any_collection != membership.delete_any_collection + || self.access_event_logs != membership.access_event_logs + || self.access_import_export != membership.access_import_export + || self.access_reports != membership.access_reports } fn apply_to(self, membership: &mut Membership) { @@ -1235,6 +1319,9 @@ impl CustomRolePermissions { membership.create_new_collections = self.create_new_collections; membership.edit_any_collection = self.edit_any_collection; membership.delete_any_collection = self.delete_any_collection; + membership.access_event_logs = self.access_event_logs; + membership.access_import_export = self.access_import_export; + membership.access_reports = self.access_reports; } } @@ -1285,14 +1372,15 @@ async fn send_invite( err!("Invalid type") }; - if new_type != MembershipType::User && headers.membership_type != MembershipType::Owner { - err!("Only Owners can invite Managers, Admins or Owners") + if !may_manage_member_type(headers.membership_type, new_type) { + err!("You don't have permission to invite this role") } - // manageAllCollections is a client-only aggregate. Persist its three children independently; - // only Edit any collection maps to the existing all-cipher access representation. + // manageAllCollections is a client-only aggregate. Persist its three children independently. + // Whether the member reaches every collection (Admin/Owner, or Custom + Edit any collection) + // decides whether we skip creating individual per-collection assignments below. let custom_permissions = CustomRolePermissions::from_request(new_type, &data.permissions); - let access_all = custom_permissions.access_all_for(new_type); + let grants_full_access = custom_permissions.grants_full_collection_access(new_type); let mut user_created: bool = false; for email in &data.emails { @@ -1334,7 +1422,6 @@ async fn send_invite( }; let mut new_member = Membership::new(user.uuid.clone(), org_id.clone(), Some(headers.user.email.clone())); - new_member.access_all = access_all; new_member.atype = new_type as i32; custom_permissions.apply_to(&mut new_member); new_member.status = member_status; @@ -1387,8 +1474,8 @@ async fn send_invite( None => false, }; - // If no accessAll, add the collections received - if !access_all && caller_can_manage_collections { + // If the member does not already reach every collection, add the collections received + if !grants_full_access && caller_can_manage_collections { // Security (F-1): a per-collection `manage` grant carries delete authority, so the // caller may only confer it on collections they could delete themselves. Otherwise a // caller acting via Edit-any-collection could invite an account they control with a @@ -1421,6 +1508,8 @@ async fn send_invite( // Security: assigning groups can indirectly grant collection access via the groups' // collections. Only callers who may manage groups (Admins/Owners or users with // manage_groups) are allowed to assign groups when inviting. + // NOTE: every requested group was already validated against this organization in + // `InviteData::validate` above, before any record was created. let caller_can_manage_groups = headers.membership_type >= MembershipType::Admin || match Membership::find_by_user_and_org(&headers.user.uuid, &org_id, &conn).await { Some(m) => m.has_manage_groups(), @@ -1428,14 +1517,6 @@ async fn send_invite( }; if caller_can_manage_groups { - // Preserve main's same-organization validation before evaluating whether a group may - // confer collection access. This also produces a clear error for foreign group IDs. - for group_id in &data.groups { - if Group::find_by_uuid_and_org(group_id, &org_id, &conn).await.is_none() { - err!("Group not found in this organization") - } - } - for group_id in &data.groups { // Security: a caller who cannot manage collections must not grant collection // access to the invitee by placing them into a collection-bearing group. @@ -1468,7 +1549,7 @@ async fn bulk_reinvite_members( let mut bulk_response = Vec::new(); for member_id in data.ids { - let err_msg = match reinvite_member_impl(&org_id, &member_id, &headers.user.email, &conn).await { + let err_msg = match reinvite_member_impl(&org_id, &member_id, &headers, &conn).await { Ok(()) => String::new(), Err(e) => format!("{e:?}"), }; @@ -1499,19 +1580,23 @@ async fn reinvite_member( if org_id != headers.org_id { err!("Organization not found", "Organization id's do not match"); } - reinvite_member_impl(&org_id, &member_id, &headers.user.email, &conn).await + reinvite_member_impl(&org_id, &member_id, &headers, &conn).await } async fn reinvite_member_impl( org_id: &OrganizationId, member_id: &MembershipId, - invited_by_email: &str, + headers: &ManageUsersHeaders, conn: &DbConn, ) -> EmptyResult { let Some(member) = Membership::find_by_uuid_and_org(member_id, org_id, conn).await else { err!("The user hasn't been invited to the organization.") }; + if !may_manage_stored_member_type(headers.membership_type, member.atype) { + err!("You don't have permission to reinvite this user") + } + if member.status != MembershipStatus::Invited as i32 { err!("The user is already accepted or confirmed to the organization") } @@ -1531,7 +1616,7 @@ async fn reinvite_member_impl( }; if CONFIG.mail_enabled() { - mail::send_invite(&user, org_id.clone(), member.uuid, &org_name, Some(invited_by_email.to_owned())).await?; + mail::send_invite(&user, org_id.clone(), member.uuid, &org_name, Some(headers.user.email.clone())).await?; } else if user.password_hash.is_empty() { let invitation = Invitation::new(&user.email); invitation.save(conn).await?; @@ -1696,8 +1781,8 @@ async fn confirm_invite_impl( err!("The specified user isn't a member of the organization") }; - if member_to_confirm.atype != MembershipType::User && headers.membership_type != MembershipType::Owner { - err!("Only Owners can confirm Managers, Admins or Owners") + if !may_manage_stored_member_type(headers.membership_type, member_to_confirm.atype) { + err!("You don't have permission to confirm this user") } if member_to_confirm.status != MembershipStatus::Accepted as i32 { @@ -1788,8 +1873,7 @@ struct EditUserData { r#type: NumberOrString, collections: Option>, groups: Option>, - #[serde(default)] - permissions: HashMap, + permissions: Option>, } #[put("/organizations//users/", data = "", rank = 1)] @@ -1821,13 +1905,14 @@ async fn edit_member( err!("Invalid type") }; - let custom_permissions = CustomRolePermissions::from_request(new_type, &data.permissions); - let access_all = custom_permissions.access_all_for(new_type); - let Some(mut member_to_edit) = Membership::find_by_uuid_and_org(&member_id, &org_id, &conn).await else { err!("The specified user isn't member of the organization") }; + let custom_permissions = + CustomRolePermissions::from_edit_request(new_type, data.permissions.as_ref(), &member_to_edit); + let grants_full_access = custom_permissions.grants_full_collection_access(new_type); + if new_type != member_to_edit.atype && (member_to_edit.atype >= MembershipType::Admin || new_type >= MembershipType::Admin) && headers.membership_type != MembershipType::Owner @@ -1836,11 +1921,10 @@ async fn edit_member( } // Security: only Admins and Owners may change a member's role type at all. A Custom member - // with manage_users must not change roles: raising a member to Manager/Custom grants - // collection-"manage" on every collection they can already write (see the `atype >= Manager` - // branch in `Collection`/`Membership` json), and lowering it revokes that access — both are - // collection-access changes this caller is not entitled to make, even though the custom - // permission flags and access_all are already gated below. Requests that leave the role + // with manage_users must not change roles: raising a member to Custom can activate existing + // explicit collection-Manage assignments and other Custom-only authorization paths, while + // lowering it revokes them. Those authority changes are outside Manage Users even though + // granular permission changes are independently gated below. Requests that leave the role // unchanged are allowed, so such members can still use the regular edit dialog. The // Admin/Owner guard above still governs Admin/Owner transitions for Owners. if !may_change_member_type(headers.membership_type, member_to_edit.atype, new_type) { @@ -1880,14 +1964,10 @@ async fn edit_member( None => false, }; - // Security: `access_all` grants full access to every collection, so only callers who may - // manage collections are allowed to change it. Otherwise a custom user with only manage_users - // could enable Edit any collection on any member (including themselves) to grant full - // collection access — a privilege escalation. For everyone else we keep the member's existing - // access_all grant untouched (neither granted nor revoked). - if caller_can_manage_collections { - member_to_edit.access_all = access_all; - } + // Edit any collection (the successor of the removed access_all flag) grants full access to + // every collection. It is part of the granular custom permissions applied here, and the + // differs_from guard above already prevents a non-Admin caller from changing it — so a Custom + // member with only manage_users can never grant themselves or others full collection access. custom_permissions.apply_to(&mut member_to_edit); member_to_edit.atype = new_type as i32; @@ -1906,8 +1986,8 @@ async fn edit_member( // Edit-any-collection thus cannot hand another member a manage/delete grant it lacks. let caller = Membership::find_by_user_and_org(&headers.user.uuid, &org_id, &conn).await; - // If no accessAll, add the collections received - if !access_all { + // If the member does not already reach every collection, add the collections received + if !grants_full_access { for col in data.collections.iter().flatten() { match Collection::find_by_uuid_and_org(&col.id, &org_id, &conn).await { None => err!("Collection not found in Organization"), @@ -2066,8 +2146,8 @@ async fn delete_member_impl( err!("User to delete isn't member of the organization") }; - if member_to_delete.atype != MembershipType::User && headers.membership_type != MembershipType::Owner { - err!("Only Owners can delete Admins or Owners") + if !may_manage_stored_member_type(headers.membership_type, member_to_delete.atype) { + err!("You don't have permission to delete this user") } if member_to_delete.atype == MembershipType::Owner && member_to_delete.status == MembershipStatus::Confirmed as i32 @@ -2178,6 +2258,20 @@ async fn post_org_import( if org_id != headers.membership.org_uuid { 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" + ) + } + let data: ImportData = data.into_inner(); // Validate the import before continuing @@ -2202,6 +2296,9 @@ async fn post_org_import( } } + // Security (audit F8/upstream): index the existing collections by id so the per-collection + // authorization below can use the *write* predicate `is_writable_by_user`. A read-only + // assignment must not let an importer plant ciphers into a shared collection. let existing_collections: HashMap = Collection::find_by_organization(&org_id, &conn).await.into_iter().map(|c| (c.uuid.clone(), c)).collect(); let mut collections: Vec = Vec::with_capacity(data.collections.len()); @@ -2218,7 +2315,7 @@ async fn post_org_import( } else { // Collection creation through an organization import is governed by the same // independent permission as the regular create endpoint. In particular, - // Edit any collection's access_all mirror must not satisfy this check. + // Edit any collection (full access to every collection) must not satisfy this check. if !headers.membership.can_create_new_collections() { err!(Compact, "The current user isn't allowed to create new collections") } @@ -2273,10 +2370,7 @@ async fn post_org_import( // any future drift fails closed with an error instead of panicking. for (cipher_index, col_index) in relations { let (Some(cipher_id), Some(col_id)) = (ciphers.get(cipher_index), collections.get(col_index)) else { - err!( - "Invalid collection relationship", - "A collection relationship references a non-existent cipher or collection" - ) + err!(Compact, "Invalid collection relationship") }; CollectionCipher::save(cipher_id, col_id, &conn).await?; } @@ -2722,15 +2816,9 @@ async fn revoke_member_impl( if member.user_uuid == headers.user.uuid { err!("You cannot revoke yourself") } - // Security: a Custom user with manage_users must not be able to revoke Admins or - // Owners. Mirrors the restriction in delete_member_impl; the Owner-specific check - // below still guards Admin-vs-Owner actions. - if member.atype != MembershipType::User && headers.membership_type < MembershipType::Admin { + if !may_manage_stored_member_type(headers.membership_type, member.atype) { err!("You don't have permission to revoke this user") } - if member.atype == MembershipType::Owner && headers.membership_type != MembershipType::Owner { - err!("Only owners can revoke other owners") - } if member.atype == MembershipType::Owner && Membership::count_confirmed_by_org_and_type(org_id, MembershipType::Owner, conn).await <= 1 { @@ -2828,15 +2916,9 @@ async fn restore_member_impl( if member.user_uuid == headers.user.uuid { err!("You cannot restore yourself") } - // Security: a Custom user with manage_users must not be able to restore Admins or - // Owners. Mirrors the restriction in delete_member_impl; the Owner-specific check - // below still guards Admin-vs-Owner actions. - if member.atype != MembershipType::User && headers.membership_type < MembershipType::Admin { + if !may_manage_stored_member_type(headers.membership_type, member.atype) { err!("You don't have permission to restore this user") } - if member.atype == MembershipType::Owner && headers.membership_type != MembershipType::Owner { - err!("Only owners can restore other owners") - } member.restore(); // This check is also done at accept_invite, _confirm_invite, _activate_member, edit_member, admin::update_membership_type @@ -2861,7 +2943,28 @@ async fn restore_member_impl( Ok(()) } -async fn get_groups_data(details: bool, org_id: OrganizationId, conn: DbConn) -> JsonResult { +async fn get_groups_data(details: bool, org_id: OrganizationId, membership: &Membership, conn: DbConn) -> JsonResult { + // The details view (group→collection/user mappings) needs full org access; the plain list only + // needs manage access to a collection, so a manager of a collection (directly or via a group) + // can load it to assign groups. + // Custom roles: the 'Manage Users'/'Manage Groups' permissions are the authority for reading the + // group mappings (they are what the route guards enforce for the details view), so they satisfy + // this check as well even when the member reaches no collection of their own. + let has_full_access = membership.has_full_access() + || (CONFIG.org_groups_enabled() + && GroupUser::has_full_access_by_member(&org_id, &membership.uuid, &conn).await); + let can_manage_users_or_groups = membership.has_manage_users() || membership.has_manage_groups(); + let allowed = if details { + has_full_access || can_manage_users_or_groups + } else { + has_full_access + || can_manage_users_or_groups + || Collection::has_manageable_collection_by_user(&org_id, &membership.user_uuid, &conn).await + }; + if !allowed { + err_code!("Resource not found.", "User does not have access", rocket::http::Status::NotFound.code); + } + let groups: Vec = if CONFIG.org_groups_enabled() { let groups = Group::find_by_organization(&org_id, &conn).await; let mut groups_json = Vec::with_capacity(groups.len()); @@ -2889,30 +2992,15 @@ async fn get_groups_data(details: bool, org_id: OrganizationId, conn: DbConn) -> }))) } -// The plain group list exposes no access mappings, but the web vault needs it to resolve group -// names while managing users, groups or collections. Preserve main's restriction for legacy -// Managers while admitting Custom members whose explicit permission requires this metadata. +// The plain group list (id, name, externalId) exposes no access mappings, so it stays readable for +// members who have a reason to see it — the web vault needs it to render group names. The exact +// condition is enforced in `get_groups_data`. #[get("/organizations//groups")] async fn get_groups(org_id: OrganizationId, headers: ManagerHeadersLoose, conn: DbConn) -> JsonResult { if org_id != headers.membership.org_uuid { err!("Organization not found", "Organization id's do not match"); } - - let has_full_access = headers.membership.has_full_access() - || (CONFIG.org_groups_enabled() - && GroupUser::has_full_access_by_member(&org_id, &headers.membership.uuid, &conn).await); - let custom_permission_needs_group_metadata = headers.membership.has_manage_users() - || headers.membership.has_manage_groups() - || headers.membership.has_create_new_collections() - || headers.membership.has_delete_any_collection(); - let allowed = has_full_access - || custom_permission_needs_group_metadata - || Collection::has_manageable_collection_by_user(&org_id, &headers.membership.user_uuid, &conn).await; - if !allowed { - err_code!("Resource not found.", "User does not have access", rocket::http::Status::NotFound.code); - } - - get_groups_data(false, org_id, conn).await + get_groups_data(false, org_id, &headers.membership, conn).await } // Security (audit M-1): group *details* expose accessAll, external IDs and collection mappings, so @@ -2923,7 +3011,7 @@ async fn get_groups_details(org_id: OrganizationId, headers: ManageUsersOrGroups if org_id != headers.org_id { err!("Organization not found", "Organization id's do not match"); } - get_groups_data(true, org_id, conn).await + get_groups_data(true, org_id, &headers.membership, conn).await } #[derive(Deserialize)] @@ -3144,17 +3232,32 @@ fn may_change_group_membership(caller_can_manage_collections: bool, group_confer /// Whether a caller of `edit_member` may change a member's role type. /// /// Only Admins and Owners may change a member's role at all. A Custom member with `manage_users` -/// must not, because the role type has collection-access side effects: a member of type -/// `Manager`/`Custom` gains collection-"manage" on every collection they can write (the -/// `atype >= Manager` branches in `Collection`/`Membership`), so promoting grants that access and -/// demoting revokes it. `manage_users` covers the user lifecycle, not the data plane, so role -/// changes are reserved for Admins/Owners. Leaving the role unchanged is always allowed so +/// must not, because the role type changes organization-wide collection reach and which granular +/// permissions are effective. `manage_users` covers the user lifecycle, not the data plane, so +/// role changes are reserved for Admins/Owners. Leaving the role unchanged is always allowed so /// `manage_users` members can still use the regular edit dialog. Admin/Owner transitions are /// additionally governed by the dedicated Owner-only guard in `edit_member`. fn may_change_member_type(caller_type: MembershipType, current_atype: i32, new_type: MembershipType) -> bool { caller_type >= MembershipType::Admin || new_type == current_atype } +/// Whether a caller with user-management access may perform lifecycle actions on a target role. +/// +/// Owners may manage every role. Admins may manage Admin, Custom, and User memberships, but never +/// Owners. Custom members holding `manage_users` are limited to ordinary Users. +fn may_manage_member_type(caller_type: MembershipType, target_type: MembershipType) -> bool { + match caller_type { + MembershipType::Owner => true, + MembershipType::Admin => target_type != MembershipType::Owner, + MembershipType::Custom => target_type == MembershipType::User, + MembershipType::User => false, + } +} + +fn may_manage_stored_member_type(caller_type: MembershipType, target_atype: i32) -> bool { + MembershipType::from_i32(target_atype).is_some_and(|target_type| may_manage_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 { @@ -3169,37 +3272,42 @@ async fn group_confers_collection_access(group_id: &GroupId, org_id: &Organizati /// /// Security (F-1, edit-any -> delete-any escalation): a `manage` grant carries collection *delete* /// authority — `CollectionDeleteHeaders` accepts it via `has_explicit_collection_manage_access`. -/// Without this gate a Custom member holding only `edit_any_collection` (whose `access_all` mirror -/// makes every collection "manageable") could, through the collection-access / group endpoints, -/// hand a `manage` row to a group they belong to (or to a manager-level member) and thereby gain -/// deletion — a capability `edit_any_collection` must never imply. +/// Without this gate a Custom member holding only `edit_any_collection` (which grants full access to +/// every collection) could, through the collection-access / group endpoints, hand a `manage` row to +/// a group they belong to (or to a manager-level member) and thereby gain deletion — a capability +/// `edit_any_collection` must never imply. /// /// We therefore allow granting `manage` on a collection only to a caller who could delete that same /// collection themselves, mirroring `collection_delete_access` exactly so it can never hand out a -/// right the caller lacks: Admin/Owner and Custom-with-`delete_any_collection` always qualify; an -/// exact legacy Manager uses its per-collection manage helper; any other Custom member must hold a -/// real explicit manage grant. This is strictly subtractive — it can only ever downgrade a requested -/// `manage` to `false`, never grant it — so it opens no new access, and delete-capable members -/// (including all Admins/Owners) are unaffected. +/// right the caller lacks: Admin/Owner and Custom-with-`delete_any_collection` always qualify; any +/// other Custom member must hold a real explicit manage grant. This is strictly subtractive — it can +/// only ever downgrade a requested `manage` to `false`, never grant it — so it opens no new access, +/// and delete-capable members (including all Admins/Owners) are unaffected. async fn caller_may_grant_collection_manage(caller: &Membership, col_id: &CollectionId, conn: &DbConn) -> bool { match caller_manage_grant_role_check(caller) { // Role alone decides it (Admin/Owner or delete_any -> yes; User/unknown/unconfirmed -> no). Some(decision) => decision, - // Manager/Custom: the answer is per-collection and must reflect a *real* manage grant. + // Custom without delete_any: the answer is per-collection and must reflect a *real* manage + // grant. A Custom member must prove a real users_collections.manage / + // collections_groups.manage grant; Edit any collection deliberately does not count here. None => match MembershipType::from_i32(caller.atype) { - // The exact legacy Manager keeps its broad per-collection manage helper (which also - // honors membership/group access_all), matching its pre-existing delete authorization. - Some(MembershipType::Manager) => { - Collection::is_coll_manageable_by_user(col_id, &caller.user_uuid, conn).await - } - // A Custom member must prove a real users_collections.manage / collections_groups.manage - // grant; edit_any_collection's access_all mirror deliberately does not count here. Some(MembershipType::Custom) => caller.has_explicit_collection_manage_access(col_id, conn).await, _ => false, }, } } +/// Whether `caller` may export the *entire* organization instead of only their own assignments. +/// +/// Security (audit F1): the `AccessImportExportHeaders` guard on `get_org_export` decides whether a +/// member may export at all; it must not decide *what* they get. Only members who already reach +/// every collection — Admins/Owners, and Custom members holding `edit_any_collection` — may receive +/// the full organization dump. For anyone else the export is built from their own assigned +/// collections/ciphers, so 'Access Import/Export' can never turn into a full vault read. +fn may_export_entire_organization(caller: &Membership) -> bool { + caller.has_full_access() +} + /// Pure, collection-independent part of `caller_may_grant_collection_manage`. /// /// `Some(true)` -> the caller may grant `manage` on *any* collection (Admin/Owner, or a Custom @@ -3217,7 +3325,7 @@ fn caller_manage_grant_role_check(caller: &Membership) -> Option { return Some(false); } match MembershipType::from_i32(caller.atype) { - Some(MembershipType::Manager | MembershipType::Custom) => None, + Some(MembershipType::Custom) => None, _ => Some(false), } } @@ -3829,18 +3937,34 @@ async fn put_reset_password_enrollment( // NOTE: It seems clients can't handle uppercase-first keys!! // We need to convert all keys so they have the first character to be a lowercase. // Else the export will be just an empty JSON file. -// We currently only support exports by members of the Admin or Owner status. -// Vaultwarden does not yet support exporting only managed collections! +// Members with full access to the organization (Admin/Owner, or a Custom member with +// 'Edit any collection') export the whole organization; everyone else exports only what they can +// actually reach, like Bitwarden's export controller does. // 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"); } + // Security (audit F1): 'Access Import/Export' decides *whether* a member may export, it must not + // widen *what* they may read. Without this scoping a Custom member holding only this permission + // — assigned to no collection at all — would receive every cipher in the organization, because + // the organization sync type deliberately skips the per-cipher access restrictions. + let (collections, ciphers) = if may_export_entire_organization(&headers.membership) { + (Collection::find_by_organization(&org_id, &conn).await, Cipher::find_by_org(&org_id, &conn).await) + } else { + ( + Collection::find_by_organization_and_user_uuid(&org_id, &headers.user.uuid, &conn).await, + filter_ciphers_for_organization(Cipher::find_by_user_visible(&headers.user.uuid, &conn).await, &org_id), + ) + }; + + let collections_json: Value = collections.iter().map(Collection::to_json).collect(); + Ok(Json(json!({ - "collections": convert_json_key_lcase_first(get_org_collections_impl(&org_id, &conn).await), - "ciphers": convert_json_key_lcase_first(get_org_details_impl(&org_id, &headers.host, &headers.user.uuid, &conn).await?), + "collections": convert_json_key_lcase_first(collections_json), + "ciphers": convert_json_key_lcase_first(ciphers_to_org_json(ciphers, &headers.host, &headers.user.uuid, &conn).await?), }))) } @@ -3909,7 +4033,8 @@ mod tests { use super::{ CustomRolePermissions, caller_manage_grant_role_check, filter_ciphers_for_organization, - may_change_group_membership, may_change_member_type, + may_change_group_membership, may_change_member_type, may_export_entire_organization, may_manage_member_type, + may_manage_stored_member_type, }; use crate::db::models::{Cipher, Membership, MembershipStatus, MembershipType, OrganizationId}; @@ -3937,12 +4062,11 @@ mod tests { // manufacturing. This is what stops edit-any from escalating into delete-any. let mut edit_any = confirmed_member(MembershipType::Custom); edit_any.edit_any_collection = true; - edit_any.access_all = true; // the internal mirror of edit_any must not shortcut to yes assert_eq!(caller_manage_grant_role_check(&edit_any), None); - // A flagless Custom / exact Manager also defer to the per-collection DB check. + // A flagless Custom member (this is what a migrated legacy Manager becomes) also defers to + // the per-collection DB check. assert_eq!(caller_manage_grant_role_check(&confirmed_member(MembershipType::Custom)), None); - assert_eq!(caller_manage_grant_role_check(&confirmed_member(MembershipType::Manager)), None); // Plain User never qualifies. assert_eq!(caller_manage_grant_role_check(&confirmed_member(MembershipType::User)), Some(false)); @@ -3954,6 +4078,32 @@ mod tests { assert_eq!(caller_manage_grant_role_check(&unconfirmed), Some(false)); } + #[test] + fn access_import_export_alone_does_not_widen_the_export() { + // REGRESSION (audit F1): 'Access Import/Export' opens the export endpoint, but a Custom + // member holding only that permission reaches no collection of their own, so the export + // must be built from their assignments — never from the whole organization. + let mut import_export_only = confirmed_member(MembershipType::Custom); + import_export_only.access_import_export = true; + assert!(!may_export_entire_organization(&import_export_only)); + + // Custom members who already reach every collection keep the full dump. + let mut edit_any = confirmed_member(MembershipType::Custom); + edit_any.edit_any_collection = true; + edit_any.access_import_export = true; + assert!(may_export_entire_organization(&edit_any)); + + // Admins and Owners are unaffected. + assert!(may_export_entire_organization(&confirmed_member(MembershipType::Admin))); + assert!(may_export_entire_organization(&confirmed_member(MembershipType::Owner))); + + // An unconfirmed membership never qualifies, whatever its flags say. + let mut unconfirmed = confirmed_member(MembershipType::Custom); + unconfirmed.edit_any_collection = true; + unconfirmed.status = MembershipStatus::Accepted as i32; + assert!(!may_export_entire_organization(&unconfirmed)); + } + #[test] fn assigned_cipher_response_is_scoped_to_requested_organization() { let requested_org: OrganizationId = "requested-org".to_owned().into(); @@ -3979,27 +4129,49 @@ mod tests { #[test] fn manage_users_caller_cannot_change_member_role() { let user = MembershipType::User as i32; - let manager = MembershipType::Manager as i32; let custom = MembershipType::Custom as i32; // Admins and Owners may change a member's role. - assert!(may_change_member_type(MembershipType::Owner, user, MembershipType::Manager)); + assert!(may_change_member_type(MembershipType::Owner, user, MembershipType::Custom)); assert!(may_change_member_type(MembershipType::Admin, user, MembershipType::Custom)); - // A below-Admin caller (Manager / Custom-with-manage_users) may only submit an unchanged - // role, so the regular edit dialog keeps working. + // A below-Admin caller (Custom-with-manage_users) may only submit an unchanged role, so the + // regular edit dialog keeps working. assert!(may_change_member_type(MembershipType::Custom, user, MembershipType::User)); assert!(may_change_member_type(MembershipType::Custom, custom, MembershipType::Custom)); - assert!(may_change_member_type(MembershipType::Manager, manager, MembershipType::Manager)); // REGRESSION (privilege escalation, PR #7397 / finding F1): a caller below Admin must NOT - // be able to change a member's role. Promoting User -> Manager/Custom grants that member - // collection-"manage" on their writable collections (atype >= Manager), and demoting - // revokes it — collection-access changes a manage_users caller is not entitled to make. - assert!(!may_change_member_type(MembershipType::Custom, user, MembershipType::Manager)); + // be able to change a member's role. Promoting User -> Custom can activate explicit + // collection-Manage assignments and Custom-only authorization paths; demoting revokes + // them. A manage_users caller is not entitled to either authority change. assert!(!may_change_member_type(MembershipType::Custom, user, MembershipType::Custom)); - assert!(!may_change_member_type(MembershipType::Custom, manager, MembershipType::User)); - assert!(!may_change_member_type(MembershipType::Manager, custom, MembershipType::User)); + assert!(!may_change_member_type(MembershipType::Custom, custom, MembershipType::User)); + } + + #[test] + fn member_lifecycle_permissions_follow_the_role_hierarchy() { + let roles = [MembershipType::Owner, MembershipType::Admin, MembershipType::Custom, MembershipType::User]; + + for target in roles { + assert!(may_manage_member_type(MembershipType::Owner, target)); + } + + assert!(!may_manage_member_type(MembershipType::Admin, MembershipType::Owner)); + assert!(may_manage_member_type(MembershipType::Admin, MembershipType::Admin)); + assert!(may_manage_member_type(MembershipType::Admin, MembershipType::Custom)); + assert!(may_manage_member_type(MembershipType::Admin, MembershipType::User)); + + assert!(!may_manage_member_type(MembershipType::Custom, MembershipType::Owner)); + assert!(!may_manage_member_type(MembershipType::Custom, MembershipType::Admin)); + assert!(!may_manage_member_type(MembershipType::Custom, MembershipType::Custom)); + assert!(may_manage_member_type(MembershipType::Custom, MembershipType::User)); + + for target in roles { + assert!(!may_manage_member_type(MembershipType::User, target)); + } + + assert!(may_manage_stored_member_type(MembershipType::Admin, MembershipType::Custom as i32)); + assert!(!may_manage_stored_member_type(MembershipType::Owner, i32::MAX)); } #[test] @@ -4036,8 +4208,8 @@ mod tests { assert_eq!(parsed.create_new_collections, create, "mask={mask:03b}"); assert_eq!(parsed.edit_any_collection, edit, "mask={mask:03b}"); assert_eq!(parsed.delete_any_collection, delete, "mask={mask:03b}"); - // Only Edit any collection maps to all-cipher access. Create/Delete must never do so. - assert_eq!(parsed.access_all_for(MembershipType::Custom), edit, "mask={mask:03b}"); + // Only Edit any collection maps to all-collection access. Create/Delete must never do so. + assert_eq!(parsed.grants_full_collection_access(MembershipType::Custom), edit, "mask={mask:03b}"); } } @@ -4050,6 +4222,9 @@ mod tests { ("createNewCollections".to_owned(), json!(true)), ("editAnyCollection".to_owned(), json!(true)), ("deleteAnyCollection".to_owned(), json!(true)), + ("accessEventLogs".to_owned(), json!(true)), + ("accessImportExport".to_owned(), json!(true)), + ("accessReports".to_owned(), json!(true)), ]); let custom = CustomRolePermissions::from_request(MembershipType::Custom, &permissions); @@ -4059,14 +4234,17 @@ mod tests { assert!(custom.create_new_collections); assert!(custom.edit_any_collection); assert!(custom.delete_any_collection); + assert!(custom.access_event_logs); + assert!(custom.access_import_export); + assert!(custom.access_reports); let user = CustomRolePermissions::from_request(MembershipType::User, &permissions); assert_eq!(user, CustomRolePermissions::default()); - assert!(!user.access_all_for(MembershipType::User)); + assert!(!user.grants_full_collection_access(MembershipType::User)); let admin = CustomRolePermissions::from_request(MembershipType::Admin, &permissions); assert_eq!(admin, CustomRolePermissions::default()); - assert!(admin.access_all_for(MembershipType::Admin)); + assert!(admin.grants_full_collection_access(MembershipType::Admin)); } #[test] @@ -4079,6 +4257,9 @@ mod tests { create_new_collections: true, edit_any_collection: true, delete_any_collection: true, + access_event_logs: true, + access_import_export: true, + access_reports: true, ..CustomRolePermissions::default() }; @@ -4088,5 +4269,44 @@ mod tests { assert!(membership.create_new_collections); assert!(membership.edit_any_collection); assert!(membership.delete_any_collection); + assert!(membership.access_event_logs); + assert!(membership.access_import_export); + assert!(membership.access_reports); + } + + #[test] + fn omitted_edit_permissions_preserve_supported_custom_grants() { + let mut membership = confirmed_member(MembershipType::Custom); + membership.manage_users = true; + membership.manage_groups = true; + membership.manage_policies = true; + membership.create_new_collections = true; + membership.edit_any_collection = true; + membership.delete_any_collection = true; + membership.access_event_logs = true; + membership.access_import_export = true; + membership.access_reports = true; + + let preserved = CustomRolePermissions::from_edit_request(MembershipType::Custom, None, &membership); + assert!(preserved.manage_users); + assert!(preserved.manage_groups); + assert!(preserved.manage_policies); + assert!(preserved.create_new_collections); + assert!(preserved.edit_any_collection); + assert!(preserved.delete_any_collection); + assert!(preserved.access_event_logs); + assert!(preserved.access_import_export); + assert!(preserved.access_reports); + assert!(!preserved.differs_from(&membership)); + + let explicit_reset = HashMap::new(); + assert_eq!( + CustomRolePermissions::from_edit_request(MembershipType::Custom, Some(&explicit_reset), &membership), + CustomRolePermissions::default() + ); + assert_eq!( + CustomRolePermissions::from_edit_request(MembershipType::User, None, &membership), + CustomRolePermissions::default() + ); } } diff --git a/src/api/core/public.rs b/src/api/core/public.rs index 3db25df9..09e76846 100644 --- a/src/api/core/public.rs +++ b/src/api/core/public.rs @@ -125,7 +125,6 @@ async fn ldap_import(data: Json, token: PublicToken, conn: DbConn let mut new_member = Membership::new(user.uuid.clone(), org_id.clone(), Some(org_email.clone())); new_member.set_external_id(Some(user_data.external_id.clone())); - new_member.access_all = false; new_member.atype = MembershipType::User as i32; new_member.status = member_status; diff --git a/src/auth.rs b/src/auth.rs index c9979034..39533abd 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -725,8 +725,10 @@ impl OrgHeaders { fn is_confirmed_and_admin(&self) -> bool { self.membership_status == MembershipStatus::Confirmed && self.membership_type >= MembershipType::Admin } + // "Manager-level or above": a confirmed Custom, Admin or Owner member. (The legacy Manager role + // has been folded into Custom, which shares the same authorization rank.) fn is_confirmed_and_manager(&self) -> bool { - self.membership_status == MembershipStatus::Confirmed && self.membership_type >= MembershipType::Manager + self.membership_status == MembershipStatus::Confirmed && self.membership_type >= MembershipType::Custom } fn is_confirmed_and_owner(&self) -> bool { self.membership_status == MembershipStatus::Confirmed && self.membership_type == MembershipType::Owner @@ -756,6 +758,19 @@ impl OrgHeaders { || self.membership.has_manage_users() || self.membership.has_manage_groups()) } + fn can_access_event_logs(&self) -> bool { + self.is_confirmed() + && (self.membership_type >= MembershipType::Admin || self.membership.has_access_event_logs()) + } + fn can_access_import_export(&self) -> bool { + self.is_confirmed() + && (self.membership_type >= MembershipType::Admin || self.membership.has_access_import_export()) + } + // NOTE: there is deliberately no `can_access_reports` guard helper. Vaultwarden has no + // server-side report endpoints — the clients compute every report locally from the + // organization cipher list — so `accessReports` is enforced inline where that list is served + // (`get_org_details`), not through a request guard. A guard here would be dead code that + // invites gating an endpoint on "may call reports" instead of "may read these ciphers". } // org_id is usually the second path param ("/organizations/"), @@ -840,6 +855,9 @@ impl<'r> FromRequest<'r> for OrgHeaders { } pub struct AdminHeaders { + // Kept for parity with the other org header guards (and possible future use); the org export + // endpoint that used to read this now goes through `AccessImportExportHeaders` instead. + #[allow(dead_code)] pub host: String, pub device: Device, pub user: User, @@ -880,6 +898,10 @@ macro_rules! generate_manage_headers { pub device: Device, pub user: User, pub membership_type: MembershipType, + // The caller's membership record. Holding the permission that opens an endpoint says + // nothing about *which* data the caller may reach, so handlers need the membership to + // apply the regular full-access/per-collection checks on top of the guard. + pub membership: Membership, pub ip: ClientIp, pub org_id: OrganizationId, } @@ -897,7 +919,8 @@ macro_rules! generate_manage_headers { user: headers.user, membership_type: headers.membership_type, ip: headers.ip, - org_id: headers.membership.org_uuid, + org_id: headers.membership.org_uuid.clone(), + membership: headers.membership, }) } else { err_handler!($err) @@ -938,6 +961,18 @@ generate_manage_headers!( can_manage_users_or_groups, "You need the 'Manage Users' or 'Manage Groups' permission, or to be an Admin or Owner, to call this endpoint" ); +generate_manage_headers!( + AccessEventLogsHeaders, + can_access_event_logs, + "You need the 'Access Event Logs' permission, or to be an Admin or Owner, to call this endpoint" +); +generate_manage_headers!( + AccessImportExportHeaders, + can_access_import_export, + "You need the 'Access Import/Export' permission, or to be an Admin or Owner, to call this endpoint" +); +// NOTE: no `AccessReportsHeaders`. See the note next to `can_access_import_export` above: +// `accessReports` guards data (the organization cipher list), not a dedicated endpoint. // col_id is usually the fourth path param ("/organizations//collections/"), // but there could be cases where it is a query value. @@ -961,7 +996,6 @@ fn get_col_id(request: &Request<'_>) -> Option { #[derive(Clone, Copy, Debug, Eq, PartialEq)] enum CollectionManageAccess { Any, - LegacyManager, ExplicitManage, Denied, } @@ -973,9 +1007,6 @@ fn collection_access_by_role(membership: &Membership, custom_has_any_access: boo match MembershipType::from_i32(membership.atype) { Some(MembershipType::Owner | MembershipType::Admin) => CollectionManageAccess::Any, - // Keep the pre-Custom role's broad behavior isolated to an exact legacy Manager. Its - // existing helper intentionally accepts membership/group access_all. - Some(MembershipType::Manager) => CollectionManageAccess::LegacyManager, Some(MembershipType::Custom) if custom_has_any_access => CollectionManageAccess::Any, // A Custom member must prove an actual users_collections.manage or // collections_groups.manage assignment. In particular, groups.access_all is not Manage. @@ -1007,9 +1038,6 @@ async fn can_manage_collection( ) -> bool { match access { CollectionManageAccess::Any => true, - CollectionManageAccess::LegacyManager => { - Collection::is_coll_manageable_by_user(collection_uuid, &membership.user_uuid, conn).await - } CollectionManageAccess::ExplicitManage => { membership.has_explicit_collection_manage_access(collection_uuid, conn).await } @@ -1036,7 +1064,7 @@ pub(crate) async fn can_edit_collection( } /// ManagerHeaders authorizes collection updates. A Custom member with Edit any collection can -/// update every collection; otherwise the caller must be at least a legacy Manager and have the +/// update every collection; otherwise the caller must be a Custom member (or above) holding the /// per-collection Manage permission. Read and delete use separate guards so Edit cannot /// accidentally imply Delete. pub struct ManagerHeaders { @@ -1145,8 +1173,8 @@ impl From for Headers { /// limitCollectionDeletion=true, so deleting *any* collection requires the explicit Delete any /// collection permission (or Admin/Owner). Deleting an individual collection is additionally /// allowed for members holding the per-collection Manage grant on it. Custom members use the -/// explicit assignment only; unlike the exact legacy Manager path, membership/group access_all -/// never counts as their per-collection Manage grant. +/// explicit assignment only; a group `access_all` grant never counts as their per-collection Manage +/// grant. pub struct CollectionDeleteHeaders { pub host: String, pub device: Device, @@ -1172,11 +1200,11 @@ impl<'r> FromRequest<'r> for CollectionDeleteHeaders { match collection_delete_access(&headers.membership) { CollectionManageAccess::Any => {} CollectionManageAccess::Denied => { - // Custom is a distinct, fail-closed role. Edit any collection and access_all alone - // must not satisfy a Delete request without either Delete any or explicit Manage. + // Custom is a distinct, fail-closed role. Edit any collection alone must not satisfy + // a Delete request without either Delete any or an explicit per-collection Manage. err_handler!("You need the 'Delete any collection' permission to call this endpoint") } - access @ (CollectionManageAccess::LegacyManager | CollectionManageAccess::ExplicitManage) => { + access @ CollectionManageAccess::ExplicitManage => { let Outcome::Success(conn) = DbConn::from_request(request).await else { err_handler!("Error getting DB") }; @@ -1667,26 +1695,20 @@ mod tests { #[test] fn flagless_custom_requires_explicit_manage_for_edit_read_and_delete() { + // A flagless Custom member (this is what a migrated legacy Manager becomes) must prove a + // real per-collection Manage grant for every collection operation. ExplicitManage invokes + // the database helper that only accepts users_collections.manage / collections_groups.manage + // — an external groups.access_all grant deliberately does not switch to a broad helper. let custom = membership(MembershipType::Custom); assert_eq!(collection_edit_access(&custom), CollectionManageAccess::ExplicitManage); assert_eq!(collection_read_access(&custom), CollectionManageAccess::ExplicitManage); assert_eq!(collection_delete_access(&custom), CollectionManageAccess::ExplicitManage); - - // Neither a stale membership access_all value nor an external groups.access_all grant may - // switch a Custom member to the legacy broad helper. ExplicitManage invokes the database - // helper that only accepts users_collections.manage / collections_groups.manage. - let mut access_all = membership(MembershipType::Custom); - access_all.access_all = true; - assert_eq!(collection_edit_access(&access_all), CollectionManageAccess::ExplicitManage); - assert_eq!(collection_read_access(&access_all), CollectionManageAccess::ExplicitManage); - assert_eq!(collection_delete_access(&access_all), CollectionManageAccess::ExplicitManage); } #[test] fn custom_any_permissions_remain_independent() { let mut edit_any = membership(MembershipType::Custom); edit_any.edit_any_collection = true; - edit_any.access_all = true; assert_eq!(collection_edit_access(&edit_any), CollectionManageAccess::Any); assert_eq!(collection_read_access(&edit_any), CollectionManageAccess::Any); // Edit-any alone is not blanket Delete. It still permits deletion of an explicitly managed @@ -1701,18 +1723,15 @@ mod tests { } #[test] - fn exact_legacy_manager_keeps_broad_helper() { - let manager = membership(MembershipType::Manager); - assert_eq!(collection_edit_access(&manager), CollectionManageAccess::LegacyManager); - assert_eq!(collection_read_access(&manager), CollectionManageAccess::LegacyManager); - assert_eq!(collection_delete_access(&manager), CollectionManageAccess::LegacyManager); - + fn admin_and_user_collection_access_roles() { let admin = membership(MembershipType::Admin); assert_eq!(collection_edit_access(&admin), CollectionManageAccess::Any); + assert_eq!(collection_read_access(&admin), CollectionManageAccess::Any); assert_eq!(collection_delete_access(&admin), CollectionManageAccess::Any); let user = membership(MembershipType::User); assert_eq!(collection_edit_access(&user), CollectionManageAccess::Denied); + assert_eq!(collection_read_access(&user), CollectionManageAccess::Denied); assert_eq!(collection_delete_access(&user), CollectionManageAccess::Denied); } diff --git a/src/db/mod.rs b/src/db/mod.rs index 2eae3f3c..f5177c4e 100644 --- a/src/db/mod.rs +++ b/src/db/mod.rs @@ -468,6 +468,168 @@ 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_SAME_RUN_MARKER_TABLE: &str = "__vw_custom_role_same_run_0716"; + +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] +#[expect( + clippy::struct_excessive_bools, + reason = "These are independent facts read from a historical database schema and migration ledger" +)] +struct CustomRoleMigrationFacts { + memberships_table_exists: bool, + migration_table_exists: bool, + access_all_column_exists: bool, + collection_permission_columns: i64, + collection_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, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum CustomRolePreflightDecision { + Proceed, + CompleteMysqlCollectionMigration, + RefuseAlreadyDropped, + RefuseMissingAccessAll, + RefuseMissingMigrationLedger, + RefuseLegacyUserAccessAll, + RefuseAmbiguousDirectPermissions, + RefusePartialCollectionSchema, + RefuseCollectionLedgerMismatch, +} + +fn custom_role_preflight_decision( + facts: CustomRoleMigrationFacts, + can_complete_mysql_partial_migration: bool, +) -> CustomRolePreflightDecision { + if !facts.memberships_table_exists || facts.repair_migration_applied { + return CustomRolePreflightDecision::Proceed; + } + if !facts.migration_table_exists { + return CustomRolePreflightDecision::RefuseMissingMigrationLedger; + } + + // Once access_all has been dropped, its former value and the provenance of 0/1/1 + // collection permissions can no longer be reconstructed. Never guess at either. + if facts.access_all_drop_migration_applied { + return CustomRolePreflightDecision::RefuseAlreadyDropped; + } + if !facts.access_all_column_exists { + 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 + } + (_, true) => CustomRolePreflightDecision::RefuseCollectionLedgerMismatch, + _ => CustomRolePreflightDecision::RefusePartialCollectionSchema, + } +} + +fn custom_role_preflight_error(decision: CustomRolePreflightDecision, facts: CustomRoleMigrationFacts) -> Error { + let detail = match decision { + CustomRolePreflightDecision::RefuseAlreadyDropped => format!( + "The membership access_all column was already dropped by migration \ + {DROP_MEMBERSHIP_ACCESS_ALL_MIGRATION}, but the required repair migration \ + {CUSTOM_ROLE_REPAIR_MIGRATION} is not recorded. The former permission values cannot \ + be reconstructed safely." + ), + CustomRolePreflightDecision::RefuseMissingAccessAll => format!( + "The membership access_all column is missing before repair migration \ + {CUSTOM_ROLE_REPAIR_MIGRATION}; refusing to infer deleted permissions." + ), + CustomRolePreflightDecision::RefuseMissingMigrationLedger => { + "The users_organizations table exists, but the Diesel migration ledger does not. \ + 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::RefuseCollectionLedgerMismatch => format!( + "Migration {CUSTOM_COLLECTION_PERMISSIONS_MIGRATION} is recorded, but only {} of its \ + three collection-permission columns exist.", + facts.collection_permission_columns + ), + CustomRolePreflightDecision::Proceed | CustomRolePreflightDecision::CompleteMysqlCollectionMigration => { + unreachable!("successful preflight decisions do not produce errors") + } + }; + + 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." + )) + .into() +} + +#[cfg(any(mysql, test))] +fn mysql_partial_unexpected_values_query(allow_same_run_group_derived: bool) -> String { + let same_run_group_derived = if allow_same_run_group_derived { + " OR \ + (atype = 4 \ + AND access_all = FALSE \ + AND create_new_collections = FALSE \ + AND edit_any_collection = TRUE \ + AND delete_any_collection = TRUE \ + AND EXISTS ( \ + SELECT 1 \ + FROM groups_users AS gu \ + INNER JOIN `groups` AS g ON g.uuid = gu.groups_uuid \ + WHERE gu.users_organizations_uuid = users_organizations.uuid \ + AND g.organizations_uuid = users_organizations.org_uuid \ + AND g.access_all = TRUE \ + ))" + } else { + "" + }; + + format!( + "SELECT COUNT(*) AS count FROM users_organizations \ + WHERE NOT ( \ + (create_new_collections = FALSE \ + AND edit_any_collection = FALSE \ + AND delete_any_collection = FALSE) \ + OR \ + (atype = 4 \ + AND create_new_collections = access_all \ + AND edit_any_collection = access_all \ + AND delete_any_collection = access_all) \ + {same_run_group_derived} \ + )" + ) +} + // Embed the migrations from the migrations folder into the application // This way, the program automatically migrates the database to the latest version // https://docs.rs/diesel_migrations/*/diesel_migrations/macro.embed_migrations.html @@ -477,11 +639,130 @@ mod sqlite_migrations { use diesel_migrations::{EmbeddedMigrations, MigrationHarness}; pub const MIGRATIONS: EmbeddedMigrations = embed_migrations!("migrations/sqlite"); + #[derive(diesel::QueryableByName)] + struct Count { + #[diesel(sql_type = diesel::sql_types::BigInt)] + count: i64, + } + + fn count( + connection: &mut diesel::sqlite::SqliteConnection, + query: impl Into, + ) -> Result { + diesel::sql_query(query).get_result::(connection).map(|row| row.count) + } + + fn table_exists( + connection: &mut diesel::sqlite::SqliteConnection, + table: &str, + ) -> Result { + count( + connection, + format!( + "SELECT COUNT(*) AS count FROM sqlite_master \ + WHERE type = 'table' AND name = '{table}'" + ), + ) + .map(|value| value != 0) + } + + fn migration_applied( + connection: &mut diesel::sqlite::SqliteConnection, + version: &str, + ) -> Result { + count( + connection, + format!( + "SELECT COUNT(*) AS count FROM __diesel_schema_migrations \ + WHERE version = '{version}'" + ), + ) + .map(|value| value != 0) + } + + fn preflight(connection: &mut diesel::sqlite::SqliteConnection) -> Result<(), super::Error> { + let memberships_table_exists = table_exists(connection, "users_organizations")?; + if !memberships_table_exists { + return Ok(()); + } + + let migration_table_exists = table_exists(connection, "__diesel_schema_migrations")?; + let access_all_column_exists = count( + connection, + "SELECT COUNT(*) AS count FROM pragma_table_info('users_organizations') \ + WHERE name = 'access_all'", + )? != 0; + let collection_permission_columns = count( + connection, + "SELECT COUNT(*) AS count FROM pragma_table_info('users_organizations') \ + WHERE name IN ('create_new_collections', 'edit_any_collection', 'delete_any_collection')", + )?; + + let collection_permissions_migration_applied = + migration_table_exists && migration_applied(connection, super::CUSTOM_COLLECTION_PERMISSIONS_MIGRATION)?; + let repair_migration_applied = + migration_table_exists && migration_applied(connection, super::CUSTOM_ROLE_REPAIR_MIGRATION)?; + let access_all_drop_migration_applied = + migration_table_exists && migration_applied(connection, super::DROP_MEMBERSHIP_ACCESS_ALL_MIGRATION)?; + let same_run_marker_table_exists = table_exists(connection, super::CUSTOM_ROLE_SAME_RUN_MARKER_TABLE)?; + let same_run_0716_marker = same_run_marker_table_exists + && count( + connection, + 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, + "SELECT COUNT(*) AS count FROM users_organizations \ + WHERE atype IN (3, 4) \ + AND access_all = FALSE \ + AND create_new_collections = FALSE \ + AND edit_any_collection = TRUE \ + AND delete_any_collection = TRUE", + )? + } else { + 0 + }; + + let facts = super::CustomRoleMigrationFacts { + memberships_table_exists, + migration_table_exists, + access_all_column_exists, + collection_permission_columns, + collection_permissions_migration_applied, + repair_migration_applied, + access_all_drop_migration_applied, + legacy_user_access_all_count, + ambiguous_direct_permission_count, + same_run_0716_marker, + }; + + let decision = super::custom_role_preflight_decision(facts, false); + if decision == super::CustomRolePreflightDecision::Proceed { + Ok(()) + } else { + Err(super::custom_role_preflight_error(decision, facts)) + } + } + pub fn run_migrations(db_url: &str) -> Result<(), super::Error> { // Establish a connection to the sqlite database (this will create a new one, if it does // not exist, and exit if there is an error). let mut connection = diesel::sqlite::SqliteConnection::establish(db_url)?; + preflight(&mut connection)?; + // Run the migrations after successfully establishing a connection // Disable Foreign Key Checks during migration // Scoped to a connection. @@ -505,10 +786,193 @@ mod mysql_migrations { use diesel_migrations::{EmbeddedMigrations, MigrationHarness}; pub const MIGRATIONS: EmbeddedMigrations = embed_migrations!("migrations/mysql"); + #[derive(diesel::QueryableByName)] + struct Count { + #[diesel(sql_type = diesel::sql_types::BigInt)] + count: i64, + } + + fn count( + connection: &mut diesel::mysql::MysqlConnection, + query: impl Into, + ) -> Result { + diesel::sql_query(query).get_result::(connection).map(|row| row.count) + } + + fn table_exists( + connection: &mut diesel::mysql::MysqlConnection, + table: &str, + ) -> Result { + count( + connection, + format!( + "SELECT COUNT(*) AS count FROM information_schema.tables \ + WHERE table_schema = DATABASE() AND table_name = '{table}'" + ), + ) + .map(|value| value != 0) + } + + fn migration_applied( + connection: &mut diesel::mysql::MysqlConnection, + version: &str, + ) -> Result { + count( + connection, + format!( + "SELECT COUNT(*) AS count FROM __diesel_schema_migrations \ + WHERE version = '{version}'" + ), + ) + .map(|value| value != 0) + } + + fn complete_partial_collection_migration( + connection: &mut diesel::mysql::MysqlConnection, + allow_same_run_group_derived: bool, + ) -> Result<(), super::Error> { + // MySQL implicitly committed the three historical ALTER TABLE statements before the + // unquoted `groups` identifier made the migration fail. Complete that exact, known state + // without dropping columns or inventing values. + let matching_column_definitions = 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') \ + AND data_type = 'tinyint' \ + AND is_nullable = 'NO' \ + AND LOWER(COALESCE(CAST(column_default AS CHAR), '')) IN ('0', 'false')", + )?; + let unexpected_values = + count(connection, super::mysql_partial_unexpected_values_query(allow_same_run_group_derived))?; + + if matching_column_definitions != 3 || unexpected_values != 0 { + return Err(std::io::Error::other(format!( + "Custom-role migration preflight found the historical MySQL partial \ + {version} schema, but its column definitions or data were modified \ + (matching columns: {matching_column_definitions}/3, unexpected rows: \ + {unexpected_values}). Refusing automatic recovery. Back up the database and \ + resolve the partial migration manually before restarting.", + version = super::CUSTOM_COLLECTION_PERMISSIONS_MIGRATION, + )) + .into()); + } + + connection.transaction::<(), diesel::result::Error, _>(|connection| { + // This is the first data statement from the canonical migration. It also resets an + // exact, same-run group-derived 0/1/1 row to 0/0/0; that authority remains dynamically + // derived from the group, and the separate 07-23 repair then reconciles the role. + diesel::sql_query( + "UPDATE users_organizations \ + SET create_new_collections = access_all, \ + edit_any_collection = access_all, \ + delete_any_collection = access_all \ + WHERE atype = 4", + ) + .execute(connection)?; + + diesel::sql_query(format!( + "INSERT INTO __diesel_schema_migrations (version) \ + VALUES ('{}')", + super::CUSTOM_COLLECTION_PERMISSIONS_MIGRATION + )) + .execute(connection)?; + Ok(()) + })?; + + Ok(()) + } + + fn preflight(connection: &mut diesel::mysql::MysqlConnection) -> Result<(), super::Error> { + let memberships_table_exists = table_exists(connection, "users_organizations")?; + if !memberships_table_exists { + return Ok(()); + } + + let migration_table_exists = table_exists(connection, "__diesel_schema_migrations")?; + let access_all_column_exists = count( + connection, + "SELECT COUNT(*) AS count FROM information_schema.columns \ + WHERE table_schema = DATABASE() \ + AND table_name = 'users_organizations' \ + AND column_name = 'access_all'", + )? != 0; + let collection_permission_columns = 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')", + )?; + + let collection_permissions_migration_applied = + migration_table_exists && migration_applied(connection, super::CUSTOM_COLLECTION_PERMISSIONS_MIGRATION)?; + let repair_migration_applied = + migration_table_exists && migration_applied(connection, super::CUSTOM_ROLE_REPAIR_MIGRATION)?; + let access_all_drop_migration_applied = + migration_table_exists && migration_applied(connection, super::DROP_MEMBERSHIP_ACCESS_ALL_MIGRATION)?; + let same_run_marker_table_exists = table_exists(connection, super::CUSTOM_ROLE_SAME_RUN_MARKER_TABLE)?; + let same_run_0716_marker = same_run_marker_table_exists + && count( + connection, + 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, + "SELECT COUNT(*) AS count FROM users_organizations \ + WHERE atype IN (3, 4) \ + AND access_all = FALSE \ + AND create_new_collections = FALSE \ + AND edit_any_collection = TRUE \ + AND delete_any_collection = TRUE", + )? + } else { + 0 + }; + + let facts = super::CustomRoleMigrationFacts { + memberships_table_exists, + migration_table_exists, + access_all_column_exists, + collection_permission_columns, + collection_permissions_migration_applied, + repair_migration_applied, + access_all_drop_migration_applied, + legacy_user_access_all_count, + ambiguous_direct_permission_count, + same_run_0716_marker, + }; + + match super::custom_role_preflight_decision(facts, true) { + super::CustomRolePreflightDecision::Proceed => Ok(()), + super::CustomRolePreflightDecision::CompleteMysqlCollectionMigration => { + complete_partial_collection_migration(connection, same_run_0716_marker) + } + decision => Err(super::custom_role_preflight_error(decision, facts)), + } + } + pub fn run_migrations(db_url: &str) -> Result<(), super::Error> { // Make sure the database is up to date (create if it doesn't exist, or run the migrations) let mut connection = diesel::mysql::MysqlConnection::establish(db_url)?; + preflight(&mut connection)?; + // Disable Foreign Key Checks during migration // Scoped to a connection/session. diesel::sql_query("SET FOREIGN_KEY_CHECKS = 0") @@ -522,15 +986,315 @@ mod mysql_migrations { #[cfg(postgresql)] mod postgresql_migrations { - use diesel::Connection; + use diesel::{Connection, RunQueryDsl}; use diesel_migrations::{EmbeddedMigrations, MigrationHarness}; pub const MIGRATIONS: EmbeddedMigrations = embed_migrations!("migrations/postgresql"); + #[derive(diesel::QueryableByName)] + struct Count { + #[diesel(sql_type = diesel::sql_types::BigInt)] + count: i64, + } + + fn count( + connection: &mut diesel::pg::PgConnection, + query: impl Into, + ) -> Result { + diesel::sql_query(query).get_result::(connection).map(|row| row.count) + } + + fn table_exists(connection: &mut diesel::pg::PgConnection, table: &str) -> Result { + count( + connection, + format!( + "SELECT COUNT(*) AS count FROM information_schema.tables \ + WHERE table_schema = current_schema() AND table_name = '{table}'" + ), + ) + .map(|value| value != 0) + } + + fn migration_applied( + connection: &mut diesel::pg::PgConnection, + version: &str, + ) -> Result { + count( + connection, + format!( + "SELECT COUNT(*) AS count FROM __diesel_schema_migrations \ + WHERE version = '{version}'" + ), + ) + .map(|value| value != 0) + } + + fn preflight(connection: &mut diesel::pg::PgConnection) -> Result<(), super::Error> { + let memberships_table_exists = table_exists(connection, "users_organizations")?; + if !memberships_table_exists { + return Ok(()); + } + + let migration_table_exists = table_exists(connection, "__diesel_schema_migrations")?; + let access_all_column_exists = count( + connection, + "SELECT COUNT(*) AS count FROM information_schema.columns \ + WHERE table_schema = current_schema() \ + AND table_name = 'users_organizations' \ + AND column_name = 'access_all'", + )? != 0; + let collection_permission_columns = 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')", + )?; + + let collection_permissions_migration_applied = + migration_table_exists && migration_applied(connection, super::CUSTOM_COLLECTION_PERMISSIONS_MIGRATION)?; + let repair_migration_applied = + migration_table_exists && migration_applied(connection, super::CUSTOM_ROLE_REPAIR_MIGRATION)?; + let access_all_drop_migration_applied = + migration_table_exists && migration_applied(connection, super::DROP_MEMBERSHIP_ACCESS_ALL_MIGRATION)?; + let same_run_marker_table_exists = table_exists(connection, super::CUSTOM_ROLE_SAME_RUN_MARKER_TABLE)?; + let same_run_0716_marker = same_run_marker_table_exists + && count( + connection, + 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, + "SELECT COUNT(*) AS count FROM users_organizations \ + WHERE atype IN (3, 4) \ + AND access_all = FALSE \ + AND create_new_collections = FALSE \ + AND edit_any_collection = TRUE \ + AND delete_any_collection = TRUE", + )? + } else { + 0 + }; + + let facts = super::CustomRoleMigrationFacts { + memberships_table_exists, + migration_table_exists, + access_all_column_exists, + collection_permission_columns, + collection_permissions_migration_applied, + repair_migration_applied, + access_all_drop_migration_applied, + legacy_user_access_all_count, + ambiguous_direct_permission_count, + same_run_0716_marker, + }; + + let decision = super::custom_role_preflight_decision(facts, false); + if decision == super::CustomRolePreflightDecision::Proceed { + Ok(()) + } else { + Err(super::custom_role_preflight_error(decision, facts)) + } + } + pub fn run_migrations(db_url: &str) -> Result<(), super::Error> { // Make sure the database is up to date (create if it doesn't exist, or run the migrations) let mut connection = diesel::pg::PgConnection::establish(db_url)?; + preflight(&mut connection)?; + connection.run_pending_migrations(MIGRATIONS).expect("Error running migrations"); Ok(()) } } + +#[cfg(test)] +mod custom_role_migration_preflight_tests { + use super::{ + CustomRoleMigrationFacts as Facts, CustomRolePreflightDecision as Decision, custom_role_preflight_decision, + mysql_partial_unexpected_values_query, + }; + + fn pending_repair() -> Facts { + Facts { + memberships_table_exists: true, + migration_table_exists: true, + access_all_column_exists: true, + ..Facts::default() + } + } + + #[test] + fn empty_database_can_run_normal_migrations() { + assert_eq!(custom_role_preflight_decision(Facts::default(), false), Decision::Proceed); + } + + #[test] + fn existing_schema_without_a_ledger_is_not_guessed() { + assert_eq!( + custom_role_preflight_decision( + Facts { + memberships_table_exists: true, + access_all_column_exists: true, + ..Facts::default() + }, + false, + ), + Decision::RefuseMissingMigrationLedger + ); + } + + #[test] + fn repair_marker_makes_completed_state_idempotent() { + assert_eq!( + custom_role_preflight_decision( + Facts { + memberships_table_exists: true, + migration_table_exists: true, + repair_migration_applied: true, + access_all_drop_migration_applied: true, + collection_permission_columns: 3, + ..Facts::default() + }, + false, + ), + Decision::Proceed + ); + } + + #[test] + fn a_historical_drop_without_the_repair_is_refused() { + assert_eq!( + custom_role_preflight_decision( + Facts { + access_all_drop_migration_applied: true, + access_all_column_exists: false, + ..pending_repair() + }, + false, + ), + Decision::RefuseAlreadyDropped + ); + } + + #[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 + ); + } + + #[test] + fn group_derived_zero_permissions_are_safe_but_ambiguous_direct_permissions_are_refused() { + 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, + ..pending_repair() + }, + false, + ), + Decision::Proceed + ); + assert_eq!( + custom_role_preflight_decision( + Facts { + collection_permission_columns: 3, + collection_permissions_migration_applied: true, + ambiguous_direct_permission_count: 1, + ..pending_repair() + }, + false, + ), + Decision::RefuseAmbiguousDirectPermissions + ); + assert_eq!( + custom_role_preflight_decision( + Facts { + collection_permission_columns: 3, + collection_permissions_migration_applied: true, + ambiguous_direct_permission_count: 1, + same_run_0716_marker: true, + ..pending_repair() + }, + false, + ), + Decision::Proceed + ); + } + + #[test] + fn exact_mysql_partial_schema_uses_only_the_mysql_completion_path() { + let facts = Facts { + collection_permission_columns: 3, + collection_permissions_migration_applied: false, + ..pending_repair() + }; + assert_eq!(custom_role_preflight_decision(facts, true), Decision::CompleteMysqlCollectionMigration); + assert_eq!(custom_role_preflight_decision(facts, false), Decision::RefusePartialCollectionSchema); + } + + #[test] + fn historical_mysql_partial_query_does_not_require_the_new_marker_table() { + let query = mysql_partial_unexpected_values_query(false); + assert!(!query.contains(super::CUSTOM_ROLE_SAME_RUN_MARKER_TABLE)); + assert!(!query.contains("groups_users")); + } + + #[test] + fn same_run_mysql_partial_query_requires_the_current_group_source() { + let query = mysql_partial_unexpected_values_query(true); + assert!(query.contains("access_all = FALSE")); + assert!(query.contains("edit_any_collection = TRUE")); + assert!(query.contains("delete_any_collection = TRUE")); + assert!(query.contains("INNER JOIN `groups` AS g")); + assert!(query.contains("g.organizations_uuid = users_organizations.org_uuid")); + assert!(query.contains("g.access_all = TRUE")); + } + + #[test] + fn incomplete_columns_and_ledger_mismatch_are_refused() { + assert_eq!( + custom_role_preflight_decision( + Facts { + collection_permission_columns: 2, + ..pending_repair() + }, + true, + ), + Decision::RefusePartialCollectionSchema + ); + assert_eq!( + custom_role_preflight_decision( + Facts { + collection_permission_columns: 2, + collection_permissions_migration_applied: true, + ..pending_repair() + }, + true, + ), + Decision::RefuseCollectionLedgerMismatch + ); + } +} diff --git a/src/db/models/cipher.rs b/src/db/models/cipher.rs index f0e5b955..ad27f3c3 100644 --- a/src/db/models/cipher.rs +++ b/src/db/models/cipher.rs @@ -889,7 +889,12 @@ impl Cipher { .and(collections_groups::groups_uuid.eq(groups::uuid))), ) .filter(ciphers::user_uuid.eq(user_uuid)) // Cipher owner - .or_filter(users_organizations::access_all.eq(true)) // access_all in org + // Edit any collection (Custom) or org admin/owner — the successor of access_all + .or_filter( + users_organizations::edit_any_collection + .eq(true) + .or(users_organizations::atype.le(MembershipType::Admin as i32)), + ) .or_filter(users_collections::user_uuid.eq(user_uuid)) // Access to collection .or_filter(groups::access_all.eq(true)) // Access via groups .or_filter(collections_groups::collections_uuid.is_not_null()) // Access via groups @@ -926,7 +931,12 @@ impl Cipher { .and(users_organizations::user_uuid.eq(users_collections::user_uuid))), ) .filter(ciphers::user_uuid.eq(user_uuid)) // Cipher owner - .or_filter(users_organizations::access_all.eq(true)) // access_all in org + // Edit any collection (Custom) or org admin/owner — the successor of access_all + .or_filter( + users_organizations::edit_any_collection + .eq(true) + .or(users_organizations::atype.le(MembershipType::Admin as i32)), + ) .or_filter(users_collections::user_uuid.eq(user_uuid)) // Access to collection .into_boxed(); @@ -1049,8 +1059,9 @@ impl Cipher { .and(collections_groups::groups_uuid.eq(groups::uuid))), ) .filter( - users_organizations::access_all - .eq(true) // User has access all + users_organizations::edit_any_collection + .eq(true) // 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 .and(users_collections::read_only.eq(false))) @@ -1080,8 +1091,9 @@ impl Cipher { .and(users_collections::user_uuid.eq(user_uuid.clone()))), ) .filter( - users_organizations::access_all - .eq(true) // User has access all + users_organizations::edit_any_collection + .eq(true) // 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 .and(users_collections::read_only.eq(false))), @@ -1124,8 +1136,9 @@ impl Cipher { .and(collections_groups::groups_uuid.eq(groups::uuid))), ) .filter( - users_organizations::access_all - .eq(true) // User has access all + users_organizations::edit_any_collection + .eq(true) // 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 .and(users_collections::read_only.eq(false))) @@ -1156,8 +1169,9 @@ impl Cipher { .and(users_collections::user_uuid.eq(user_uuid.clone()))), ) .filter( - users_organizations::access_all - .eq(true) // User has access all + users_organizations::edit_any_collection + .eq(true) // 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 .and(users_collections::read_only.eq(false))) @@ -1202,7 +1216,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::access_all.eq(true)) // User has access all + .or_filter(users_organizations::edit_any_collection.eq(true)) // 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 e954e2f6..17dfa090 100644 --- a/src/db/models/collection.rs +++ b/src/db/models/collection.rs @@ -1,5 +1,6 @@ use derive_more::{AsRef, Deref, Display, From}; use diesel::prelude::*; +use num_traits::FromPrimitive; use serde_json::Value; use crate::{ @@ -52,6 +53,16 @@ pub struct CollectionCipher { pub collection_uuid: CollectionId, } +/// Serialize the assignment-level `manage` capability using the same role boundary as the +/// collection mutation guards. Read/write access is deliberately not management authority. +pub(super) fn assignment_manage_for_member(membership_type: i32, stored_manage: bool) -> bool { + match MembershipType::from_i32(membership_type) { + Some(MembershipType::Owner | MembershipType::Admin) => true, + Some(MembershipType::Custom) => stored_manage, + Some(MembershipType::User) | None => false, + } +} + /// Local methods impl Collection { pub fn new(org_uuid: OrganizationId, name: String, external_id: Option) -> Self { @@ -104,25 +115,14 @@ impl Collection { ) -> Value { let (read_only, hide_passwords, manage) = if let Some(cipher_sync_data) = cipher_sync_data { match cipher_sync_data.members.get(&self.org_uuid) { - // Only for Manager types Bitwarden returns true for the manage option - // Owners and Admins always have true. Users are not able to have full access - Some(m) if m.has_full_access() => (false, false, m.atype >= MembershipType::Manager), + // Full collection visibility is not collection-management authority. Admins and + // Owners manage implicitly; Custom members still need an explicit stored grant. + Some(m) if m.has_full_access() => (false, false, assignment_manage_for_member(m.atype, false)), Some(m) => { - // Only let a manager-level member (Manager or Custom) manage collections - // when they have full read/write access - let is_manager = m.atype >= MembershipType::Manager; if let Some(cu) = cipher_sync_data.user_collections.get(&self.uuid) { - ( - cu.read_only, - cu.hide_passwords, - is_manager && (cu.manage || (!cu.read_only && !cu.hide_passwords)), - ) + (cu.read_only, cu.hide_passwords, assignment_manage_for_member(m.atype, cu.manage)) } else if let Some(cg) = cipher_sync_data.user_collections_groups.get(&self.uuid) { - ( - cg.read_only, - cg.hide_passwords, - is_manager && (cg.manage || (!cg.read_only && !cg.hide_passwords)), - ) + (cg.read_only, cg.hide_passwords, assignment_manage_for_member(m.atype, cg.manage)) } else { (false, false, false) } @@ -131,15 +131,17 @@ impl Collection { } } else { match Membership::find_confirmed_by_user_and_org(user_uuid, &self.org_uuid, conn).await { - Some(m) if m.has_full_access() => (false, false, m.atype >= MembershipType::Manager), - Some(m) if m.atype >= MembershipType::Manager && self.is_manageable_by_user(user_uuid, conn).await => { + Some(m) if m.has_full_access() => (false, false, assignment_manage_for_member(m.atype, false)), + Some(m) + if m.atype >= MembershipType::Custom + && m.has_explicit_collection_manage_access(&self.uuid, conn).await => + { (false, false, true) } - Some(m) => { - let is_manager = m.atype >= MembershipType::Manager; + Some(_) => { let read_only = !self.is_writable_by_user(user_uuid, conn).await; let hide_passwords = self.hide_passwords_for_user(user_uuid, conn).await; - (read_only, hide_passwords, is_manager && !read_only && !hide_passwords) + (read_only, hide_passwords, false) } _ => (true, true, false), } @@ -261,8 +263,11 @@ impl Collection { users_collections::user_uuid .eq(user_uuid) .or( - // Directly accessed collection - users_organizations::access_all.eq(true), // access_all in Organization + // 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) + .or(users_organizations::atype.le(MembershipType::Admin as i32)), ) .or( groups::access_all.eq(true), // access_all in groups @@ -294,10 +299,15 @@ impl Collection { .and(users_organizations::user_uuid.eq(user_uuid.clone()))), ) .filter(users_organizations::status.eq(MembershipStatus::Confirmed as i32)) - .filter(users_collections::user_uuid.eq(user_uuid).or( - // Directly accessed collection - users_organizations::access_all.eq(true), // access_all in Organization - )) + .filter( + 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) + .or(users_organizations::atype.le(MembershipType::Admin as i32)), + ), + ) .select(collections::all_columns) .distinct() .load::(conn) @@ -381,8 +391,8 @@ impl Collection { .eq(uuid) .or( // Directly accessed collection - users_organizations::access_all.eq(true).or( - // access_all in Organization + users_organizations::edit_any_collection.eq(true).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 ), ) @@ -417,8 +427,8 @@ impl Collection { .filter(collections::uuid.eq(uuid)) .filter(users_collections::collection_uuid.eq(uuid).or( // Directly accessed collection - users_organizations::access_all.eq(true).or( - // access_all in Organization + users_organizations::edit_any_collection.eq(true).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 ), )) @@ -462,7 +472,7 @@ impl Collection { .filter( users_organizations::atype .le(MembershipType::Admin as i32) // Org admin or owner - .or(users_organizations::access_all.eq(true)) // access_all via membership + .or(users_organizations::edit_any_collection.eq(true)) // 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))) @@ -495,7 +505,7 @@ impl Collection { .filter( users_organizations::atype .le(MembershipType::Admin as i32) // Org admin or owner - .or(users_organizations::access_all.eq(true)) // access_all via membership + .or(users_organizations::edit_any_collection.eq(true)) // 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))), @@ -542,8 +552,8 @@ impl Collection { .and(users_collections::hide_passwords.eq(true)) .or( // Directly accessed collection - users_organizations::access_all.eq(true).or( - // access_all in Organization + users_organizations::edit_any_collection.eq(true).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 ), ) @@ -568,71 +578,8 @@ impl Collection { .await } - pub async fn is_coll_manageable_by_user(uuid: &CollectionId, user_uuid: &UserId, conn: &DbConn) -> bool { - let uuid = uuid.to_string(); - let user_uuid = user_uuid.to_string(); - conn.run(move |conn| { - collections::table - .left_join( - users_collections::table.on(users_collections::collection_uuid - .eq(collections::uuid) - .and(users_collections::user_uuid.eq(user_uuid.clone()))), - ) - .left_join( - users_organizations::table.on(collections::org_uuid - .eq(users_organizations::org_uuid) - .and(users_organizations::user_uuid.eq(user_uuid))), - ) - .left_join(groups_users::table.on(groups_users::users_organizations_uuid.eq(users_organizations::uuid))) - .left_join( - groups::table.on(groups::uuid - .eq(groups_users::groups_uuid) - .and(groups::organizations_uuid.eq(users_organizations::org_uuid))), - ) - .left_join( - collections_groups::table.on(collections_groups::groups_uuid - .eq(groups_users::groups_uuid) - .and(collections_groups::collections_uuid.eq(collections::uuid))), - ) - .filter(collections::uuid.eq(&uuid)) - .filter( - users_collections::collection_uuid - .eq(&uuid) - .and(users_collections::manage.eq(true)) - .or( - // Directly accessed collection - users_organizations::access_all.eq(true).or( - // access_all in Organization - users_organizations::atype.le(MembershipType::Admin as i32), // Org admin or owner - ), - ) - .or( - groups::access_all.eq(true), // access_all in groups - ) - .or( - // access via groups - groups_users::users_organizations_uuid.eq(users_organizations::uuid).and( - collections_groups::collections_uuid - .is_not_null() - .and(collections_groups::manage.eq(true)), - ), - ), - ) - .count() - .first::(conn) - .ok() - .unwrap_or(0) - != 0 - }) - .await - } - - pub async fn is_manageable_by_user(&self, user_uuid: &UserId, conn: &DbConn) -> bool { - Self::is_coll_manageable_by_user(&self.uuid, user_uuid, conn).await - } - // Whether the user has manage access to at least one collection in the org, directly or via a - // group. Org-scoped counterpart of is_coll_manageable_by_user. + // group. pub async fn has_manageable_collection_by_user( org_uuid: &OrganizationId, user_uuid: &UserId, @@ -659,6 +606,8 @@ impl Collection { .and(collections_groups::collections_uuid.eq(collections::uuid))), ) .filter(collections::org_uuid.eq(&org_uuid)) + .filter(users_organizations::status.eq(MembershipStatus::Confirmed as i32)) + .filter(users_organizations::atype.eq(MembershipType::Custom as i32)) .filter( // Manage permission on a collection assigned directly or via a group. users_collections::manage.eq(true).or(collections_groups::manage.eq(true)), @@ -991,11 +940,7 @@ impl CollectionMembership { "id": self.membership_uuid, "readOnly": self.read_only, "hidePasswords": self.hide_passwords, - "manage": membership_type >= MembershipType::Admin - || self.manage - || (membership_type >= MembershipType::Manager - && !self.read_only - && !self.hide_passwords), + "manage": assignment_manage_for_member(membership_type, self.manage), }) } } @@ -1029,3 +974,21 @@ impl From for CollectionMembership { UuidFromParam, )] pub struct CollectionId(String); + +#[cfg(test)] +mod tests { + use super::assignment_manage_for_member; + use crate::db::models::MembershipType; + + #[test] + fn assignment_manage_matches_collection_guard_role_boundaries() { + for role in [MembershipType::Owner, MembershipType::Admin] { + assert!(assignment_manage_for_member(role as i32, false)); + } + + assert!(assignment_manage_for_member(MembershipType::Custom as i32, true)); + assert!(!assignment_manage_for_member(MembershipType::Custom as i32, false)); + assert!(!assignment_manage_for_member(MembershipType::User as i32, true)); + assert!(!assignment_manage_for_member(i32::MAX, true)); + } +} diff --git a/src/db/models/organization.rs b/src/db/models/organization.rs index 7eab77c0..6922c56e 100644 --- a/src/db/models/organization.rs +++ b/src/db/models/organization.rs @@ -25,7 +25,7 @@ use macros::UuidFromParam; use super::{ Cipher, CipherId, Collection, CollectionGroup, CollectionId, CollectionUser, Group, GroupId, GroupUser, OrgPolicy, - OrgPolicyType, TwoFactor, User, UserId, + OrgPolicyType, TwoFactor, User, UserId, collection::assignment_manage_for_member as assignment_manage, }; #[derive(Identifiable, Queryable, Insertable, AsChangeset)] @@ -52,7 +52,6 @@ pub struct Membership { pub invited_by_email: Option, - pub access_all: bool, pub akey: String, pub status: i32, pub atype: i32, @@ -64,6 +63,9 @@ pub struct Membership { pub create_new_collections: bool, pub edit_any_collection: bool, pub delete_any_collection: bool, + pub access_event_logs: bool, + pub access_import_export: bool, + pub access_reports: bool, } #[derive(Identifiable, Queryable, Insertable, AsChangeset)] @@ -104,7 +106,10 @@ pub enum MembershipType { Owner = 0, Admin = 1, User = 2, - Manager = 3, + // NOTE: the legacy Manager role (wire value 3) has been folded into Custom. It is no longer a + // distinct variant: it is never persisted or emitted, and an incoming value 3 is mapped onto + // Custom for backward compatibility (see `from_str`). The Custom discriminant stays 4 because + // that is the only role modern Bitwarden clients understand as carrying custom permissions. Custom = 4, } @@ -114,8 +119,10 @@ impl MembershipType { "0" | "Owner" => Some(MembershipType::Owner), "1" | "Admin" => Some(MembershipType::Admin), "2" | "User" => Some(MembershipType::User), - "3" | "Manager" => Some(MembershipType::Manager), - "4" | "Custom" => Some(MembershipType::Custom), + // "3"/"Manager" is the legacy Manager role. Modern clients no longer offer it, but an old + // client or stored request may still send value 3. Custom supersedes Manager, so accept + // and fold it onto Custom. + "3" | "Manager" | "4" | "Custom" => Some(MembershipType::Custom), _ => None, } } @@ -123,7 +130,7 @@ impl MembershipType { const fn access_rank(self) -> u8 { match self { Self::User => 0, - Self::Manager | Self::Custom => 1, + Self::Custom => 1, Self::Admin => 2, Self::Owner => 3, } @@ -132,11 +139,9 @@ impl MembershipType { impl Ord for MembershipType { fn cmp(&self, other: &MembershipType) -> Ordering { - // Manager and Custom intentionally share the same authorization rank. A total ordering - // still has to distinguish unequal enum variants, otherwise `Ord` would disagree with - // `Eq` and ordered maps/sets could collapse one role into the other. The discriminant is a - // stable tie-breaker and places Custom after Manager, preserving `Custom >= Manager` while - // keeping both roles below Admin. + // Roles are ordered by their authorization rank, not by their raw discriminant (Custom's + // discriminant is 4 but it ranks between User and Admin). The discriminant is kept as a + // stable tie-breaker so `Ord` never disagrees with `Eq`. self.access_rank().cmp(&other.access_rank()).then_with(|| (*self as i32).cmp(&(*other as i32))) } } @@ -278,7 +283,6 @@ impl Membership { org_uuid, invited_by_email, - access_all: false, akey: String::new(), status: MembershipStatus::Accepted as i32, atype: MembershipType::User as i32, @@ -290,6 +294,9 @@ impl Membership { create_new_collections: false, edit_any_collection: false, delete_any_collection: false, + access_event_logs: false, + access_import_export: false, + access_reports: false, } } @@ -460,9 +467,9 @@ impl Membership { let membership_type = self.atype; let permissions = json!({ - "accessEventLogs": false, - "accessImportExport": false, - "accessReports": false, + "accessEventLogs": membership_type == MembershipType::Custom as i32 && self.access_event_logs, + "accessImportExport": membership_type == MembershipType::Custom as i32 && self.access_import_export, + "accessReports": membership_type == MembershipType::Custom as i32 && self.access_reports, "createNewCollections": membership_type == MembershipType::Custom as i32 && self.create_new_collections, "editAnyCollection": membership_type == MembershipType::Custom as i32 && self.edit_any_collection, "deleteAnyCollection": membership_type == MembershipType::Custom as i32 && self.delete_any_collection, @@ -474,10 +481,9 @@ impl Membership { "manageScim": false // Not supported (Not AGPLv3 Licensed) }); - // edit_any_collection is internally mirrored to access_all to provide Bitwarden-compatible - // cipher access, but it must not accidentally grant collection creation. The client treats - // limitCollectionCreation=false as an independent create grant, so compute it from the - // actual role/permission rather than access_all for Custom members. + // Edit any collection grants full read/edit access to every collection, but it must not + // accidentally grant collection creation. The client treats limitCollectionCreation=false as + // an independent create grant, so compute it from the actual role/permission. let limit_collection_creation = self.limit_collection_creation(); // https://github.com/bitwarden/server/blob/9ebe16587175b1c0e9208f84397bb75d0d595510/src/Api/AdminConsole/Models/Response/ProfileOrganizationResponseModel.cs @@ -578,53 +584,50 @@ impl Membership { CONFIG.org_groups_enabled() && Group::is_in_full_access_group(&self.user_uuid, &self.org_uuid, conn).await; // If collections are to be included, only include them if the user does not have full access via a group or defined to the user it self - let collections: Vec = if include_collections && !(full_access_group || self.access_all) { - // Get all collections for the user here already to prevent more queries - let cu: HashMap = - CollectionUser::find_by_organization_and_user_uuid(&self.org_uuid, &self.user_uuid, conn) + let collections: Vec = + if include_collections && !(full_access_group || self.grants_access_to_all_collections()) { + // Get all collections for the user here already to prevent more queries + let cu: HashMap = + CollectionUser::find_by_organization_and_user_uuid(&self.org_uuid, &self.user_uuid, conn) + .await + .into_iter() + .map(|cu| (cu.collection_uuid.clone(), cu)) + .collect(); + + // Get all collection groups for this user to prevent there inclusion + let cg: HashSet = CollectionGroup::find_by_user(&self.user_uuid, conn) .await .into_iter() - .map(|cu| (cu.collection_uuid.clone(), cu)) + .map(|cg| cg.collections_uuid) .collect(); - // Get all collection groups for this user to prevent there inclusion - let cg: HashSet = CollectionGroup::find_by_user(&self.user_uuid, conn) - .await - .into_iter() - .map(|cg| cg.collections_uuid) - .collect(); - - Collection::find_by_organization_and_user_uuid(&self.org_uuid, &self.user_uuid, conn) - .await - .into_iter() - .filter_map(|c| { - let (read_only, hide_passwords, manage) = if self.has_full_access() { - (false, false, self.atype >= MembershipType::Manager) - } else if let Some(cu) = cu.get(&c.uuid) { - ( - cu.read_only, - cu.hide_passwords, - cu.manage || (self.atype >= MembershipType::Manager && !cu.read_only && !cu.hide_passwords), - ) - // If previous checks failed it might be that this user has access via a group, but we should not return those elements here - // Those are returned via a special group endpoint - } else if cg.contains(&c.uuid) { - return None; - } else { - (true, true, false) - }; - - Some(json!({ - "id": c.uuid, - "readOnly": read_only, - "hidePasswords": hide_passwords, - "manage": manage, - })) - }) - .collect() - } else { - Vec::new() - }; + Collection::find_by_organization_and_user_uuid(&self.org_uuid, &self.user_uuid, conn) + .await + .into_iter() + .filter_map(|c| { + let (read_only, hide_passwords, manage) = if self.has_full_access() { + (false, false, assignment_manage(self.atype, false)) + } else if let Some(cu) = cu.get(&c.uuid) { + (cu.read_only, cu.hide_passwords, assignment_manage(self.atype, cu.manage)) + // If previous checks failed it might be that this user has access via a group, but we should not return those elements here + // Those are returned via a special group endpoint + } else if cg.contains(&c.uuid) { + return None; + } else { + (true, true, false) + }; + + Some(json!({ + "id": c.uuid, + "readOnly": read_only, + "hidePasswords": hide_passwords, + "manage": manage, + })) + }) + .collect() + } else { + Vec::new() + }; let membership_type = self.atype; @@ -632,9 +635,9 @@ impl Membership { // all-false defaults and the role itself supplies any elevated capabilities. let permissions = if membership_type == MembershipType::Custom as i32 { json!({ - "accessEventLogs": false, - "accessImportExport": false, - "accessReports": false, + "accessEventLogs": self.access_event_logs, + "accessImportExport": self.access_import_export, + "accessReports": self.access_reports, "createNewCollections": self.create_new_collections, "editAnyCollection": self.edit_any_collection, "deleteAnyCollection": self.delete_any_collection, @@ -661,7 +664,9 @@ impl Membership { "status": status, "type": membership_type, - "accessAll": self.access_all, + // `access_all` no longer exists as a stored flag; report the effective all-collection + // access so clients that still read this obsolete field keep seeing a consistent value. + "accessAll": self.grants_access_to_all_collections(), "twoFactorEnabled": twofactor_enabled, "resetPasswordEnrolled": self.reset_password_key.is_some(), "hasMasterPassword": !user.password_hash.is_empty(), @@ -688,7 +693,7 @@ impl Membership { } pub async fn to_json_details(&self, conn: &DbConn) -> Value { - let coll_uuids = if self.access_all { + let coll_uuids = if self.grants_access_to_all_collections() { vec![] // If we have complete access, no need to fill the array } else { let collections = @@ -720,7 +725,8 @@ impl Membership { "status": status, "type": self.atype, - "accessAll": self.access_all, + // Obsolete stored flag removed; report the effective all-collection access instead. + "accessAll": self.grants_access_to_all_collections(), "collections": coll_uuids, "object": "organizationUserDetails", @@ -829,10 +835,19 @@ impl Membership { } pub fn has_full_access(&self) -> bool { - (self.access_all || self.has_edit_any_collection() || self.atype >= MembershipType::Admin) + (self.has_edit_any_collection() || self.atype >= MembershipType::Admin) && self.has_status(MembershipStatus::Confirmed) } + /// Whether this membership reaches every collection in the org regardless of per-collection + /// assignments — Admins/Owners implicitly, and Custom members holding `edit_any_collection`. + /// This is the successor of the removed `access_all` flag: it backs the `accessAll` field the + /// Bitwarden clients still read, and it intentionally does not gate on status, matching the old + /// column's semantics. Authorization decisions use the status-aware `has_full_access` instead. + pub fn grants_access_to_all_collections(&self) -> bool { + self.atype >= MembershipType::Admin || self.has_edit_any_collection() + } + // The granular custom permission flags are only meaningful while the membership is of // the Custom type. Gating them on the type here ensures that a stale flag left over from // a type change (e.g. via the admin panel) can never grant anything. @@ -860,6 +875,18 @@ impl Membership { self.has_type(MembershipType::Custom) && self.delete_any_collection } + pub fn has_access_event_logs(&self) -> bool { + self.has_type(MembershipType::Custom) && self.access_event_logs + } + + pub fn has_access_import_export(&self) -> bool { + self.has_type(MembershipType::Custom) && self.access_import_export + } + + pub fn has_access_reports(&self) -> bool { + self.has_type(MembershipType::Custom) && self.access_reports + } + /// Check for an explicit per-collection Manage grant without treating any `access_all` value /// as such a grant. Custom-role collection guards use this instead of the legacy broad helper, /// because membership/group `access_all` must not manufacture a per-collection Manage grant. @@ -883,6 +910,7 @@ impl Membership { .filter(users_organizations::user_uuid.eq(user_uuid.clone())) .filter(users_organizations::org_uuid.eq(org_uuid.clone())) .filter(users_organizations::status.eq(MembershipStatus::Confirmed as i32)) + .filter(users_organizations::atype.eq(MembershipType::Custom as i32)) .filter(collections::uuid.eq(collection_uuid.clone())) .filter(users_collections::manage.eq(true)) .count() @@ -913,6 +941,7 @@ impl Membership { .filter(users_organizations::user_uuid.eq(user_uuid)) .filter(users_organizations::org_uuid.eq(org_uuid)) .filter(users_organizations::status.eq(MembershipStatus::Confirmed as i32)) + .filter(users_organizations::atype.eq(MembershipType::Custom as i32)) .filter(collections::uuid.eq(collection_uuid)) .filter(collections_groups::manage.eq(true)) .count() @@ -929,9 +958,8 @@ impl Membership { self.has_create_new_collections() && self.has_edit_any_collection() && self.has_delete_any_collection() } - /// Match Vaultwarden's existing collection-creation policy while keeping the new Custom - /// permission independent from edit/delete. Legacy Manager memberships retain their former - /// access_all-based behavior. + /// Match Vaultwarden's existing collection-creation policy while keeping the Custom + /// permission independent from edit/delete. pub fn can_create_new_collections(&self) -> bool { if !self.has_status(MembershipStatus::Confirmed) { return false; @@ -939,7 +967,6 @@ impl Membership { match MembershipType::from_i32(self.atype) { Some(MembershipType::Owner | MembershipType::Admin) => true, - Some(MembershipType::Manager) => self.access_all, Some(MembershipType::Custom) => self.create_new_collections, Some(MembershipType::User) | None => false, } @@ -948,7 +975,6 @@ impl Membership { pub fn limit_collection_creation(&self) -> bool { match MembershipType::from_i32(self.atype) { Some(MembershipType::Owner | MembershipType::Admin) => false, - Some(MembershipType::Manager) => !self.access_all, Some(MembershipType::Custom) => !self.create_new_collections, Some(MembershipType::User) | None => true, } @@ -966,6 +992,9 @@ impl Membership { self.create_new_collections = false; self.edit_any_collection = false; self.delete_any_collection = false; + self.access_event_logs = false; + self.access_import_export = false; + self.access_reports = false; } pub async fn find_by_uuid(uuid: &MembershipId, conn: &DbConn) -> Option { @@ -1074,7 +1103,7 @@ impl Membership { .await } - // Get all users which are either owner or admin, or a manager/custom member which can manage/access all + // Get all users which are either owner or admin, or a Custom member which can access all collections pub async fn find_confirmed_and_manage_all_by_org(org_uuid: &OrganizationId, conn: &DbConn) -> Vec { conn.run(move |conn| { users_organizations::table @@ -1084,8 +1113,8 @@ impl Membership { users_organizations::atype .eq_any(vec![MembershipType::Owner as i32, MembershipType::Admin as i32]) .or(users_organizations::atype - .eq_any(vec![MembershipType::Manager as i32, MembershipType::Custom as i32]) - .and(users_organizations::access_all.eq(true))), + .eq(MembershipType::Custom as i32) + .and(users_organizations::edit_any_collection.eq(true))), ) .load::(conn) .unwrap_or_default() @@ -1209,10 +1238,12 @@ impl Membership { .eq(users_collections::collection_uuid) .and(ciphers_collections::cipher_uuid.eq(&cipher_uuid))), ) - .filter(users_organizations::access_all.eq(true).or( - // AccessAll.. - ciphers_collections::cipher_uuid.eq(&cipher_uuid), // ..or access to collection with cipher - )) + .filter( + users_organizations::edit_any_collection + .eq(true) // 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 + ) .select(users_organizations::all_columns) .distinct() .load::(conn) @@ -1285,10 +1316,12 @@ impl Membership { users_organizations::table .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::access_all.eq(true).or( - // AccessAll.. - users_collections::collection_uuid.eq(&collection_uuid), // ..or access to collection with cipher - )) + .filter( + users_organizations::edit_any_collection + .eq(true) // 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 + ) .select(users_organizations::all_columns) .load::(conn) .expect("Error loading user organizations") @@ -1424,23 +1457,17 @@ mod tests { fn membership_type_order_preserves_access_rank_and_ord_contract() { assert!(MembershipType::Owner > MembershipType::Admin); assert!(MembershipType::Admin > MembershipType::Custom); - assert!(MembershipType::Custom > MembershipType::Manager); - assert!(MembershipType::Manager > MembershipType::User); + assert!(MembershipType::Custom > MembershipType::User); assert!(MembershipType::Custom == MembershipType::from_str("4").unwrap()); + // The legacy Manager wire value (3) is accepted and folded onto Custom. + assert!(MembershipType::Custom == MembershipType::from_str("3").unwrap()); // Permission comparisons continue to treat Custom as manager-level and below Admin. - assert!(MembershipType::Custom >= MembershipType::Manager); let custom = MembershipType::Custom as i32; - assert!(custom >= MembershipType::Manager); + assert!(custom >= MembershipType::Custom); assert!(custom < MembershipType::Admin); - let types = [ - MembershipType::Owner, - MembershipType::Admin, - MembershipType::User, - MembershipType::Manager, - MembershipType::Custom, - ]; + let types = [MembershipType::Owner, MembershipType::Admin, MembershipType::User, MembershipType::Custom]; for lhs in types { for rhs in types { assert_eq!(lhs.cmp(&rhs) == Ordering::Equal, lhs == rhs); @@ -1486,23 +1513,20 @@ mod tests { fn edit_any_collection_does_not_imply_create_or_delete() { let mut custom = membership(MembershipType::Custom); custom.edit_any_collection = true; - // The persisted access_all mirror is intentionally tested too: client-facing create and - // delete decisions must still use their dedicated permissions. - custom.access_all = true; + // Edit any collection grants full (read/edit) access to every collection, but client-facing + // create and delete decisions must still use their own dedicated permissions. assert!(custom.has_full_access()); + assert!(custom.grants_access_to_all_collections()); assert!(!custom.can_create_new_collections()); assert!(custom.limit_collection_creation()); assert!(!custom.can_delete_any_collection()); - let mut manager = membership(MembershipType::Manager); - manager.access_all = true; - assert!(manager.can_create_new_collections()); - let admin = membership(MembershipType::Admin); assert!(admin.can_create_new_collections()); assert!(!admin.limit_collection_creation()); assert!(admin.can_delete_any_collection()); + assert!(admin.grants_access_to_all_collections()); } #[test] @@ -1527,6 +1551,9 @@ mod tests { member.create_new_collections = true; member.edit_any_collection = true; member.delete_any_collection = true; + member.access_event_logs = true; + member.access_import_export = true; + member.access_reports = true; member.clear_custom_permissions(); @@ -1536,5 +1563,28 @@ mod tests { assert!(!member.create_new_collections); assert!(!member.edit_any_collection); assert!(!member.delete_any_collection); + assert!(!member.access_event_logs); + assert!(!member.access_import_export); + assert!(!member.access_reports); + } + + #[test] + fn custom_access_permissions_are_independent_and_type_gated() { + let mut member = membership(MembershipType::Custom); + member.access_event_logs = true; + assert!(member.has_access_event_logs()); + assert!(!member.has_access_import_export()); + + member.access_import_export = true; + member.access_reports = true; + assert!(member.has_access_import_export()); + // None of them imply collection or management capabilities. + assert!(!member.has_full_access()); + assert!(!member.has_manage_users()); + + // Stale flags on a non-Custom role grant nothing. + member.atype = MembershipType::User as i32; + assert!(!member.has_access_event_logs()); + assert!(!member.has_access_import_export()); } } diff --git a/src/db/schema.rs b/src/db/schema.rs index e8840acd..06023872 100644 --- a/src/db/schema.rs +++ b/src/db/schema.rs @@ -236,7 +236,6 @@ table! { user_uuid -> Text, org_uuid -> Text, invited_by_email -> Nullable, - access_all -> Bool, akey -> Text, status -> Integer, atype -> Integer, @@ -248,6 +247,9 @@ table! { create_new_collections -> Bool, edit_any_collection -> Bool, delete_any_collection -> Bool, + access_event_logs -> Bool, + access_import_export -> Bool, + access_reports -> Bool, } } diff --git a/src/static/scripts/admin_users.js b/src/static/scripts/admin_users.js index 1bae0aa3..03ec9712 100644 --- a/src/static/scripts/admin_users.js +++ b/src/static/scripts/admin_users.js @@ -174,10 +174,6 @@ const ORG_TYPES = { "name": "User", "bg": "blue" }, - "3": { - "name": "Manager", - "bg": "green" - }, "4": { "name": "Custom", "bg": "teal" @@ -215,12 +211,13 @@ jQuery.extend(jQuery.fn.dataTableExt.oSort, { const userOrgTypeDialog = document.getElementById("userOrgTypeDialog"); // Fill the form and title userOrgTypeDialog.addEventListener("show.bs.modal", function(event) { + document.getElementById("userOrgTypeForm").reset(); + // Get shared values const userEmail = event.relatedTarget.parentNode.dataset.vwUserEmail; const userUuid = event.relatedTarget.parentNode.dataset.vwUserUuid; // Get org specific values const userOrgType = event.relatedTarget.dataset.vwOrgType; - const userOrgTypeName = ORG_TYPES[userOrgType]["name"]; const orgName = event.relatedTarget.dataset.vwOrgName; const orgUuid = event.relatedTarget.dataset.vwOrgUuid; @@ -228,7 +225,9 @@ userOrgTypeDialog.addEventListener("show.bs.modal", function(event) { document.getElementById("userOrgTypeDialogUserEmail").textContent = userEmail; document.getElementById("userOrgTypeUserUuid").value = userUuid; document.getElementById("userOrgTypeOrgUuid").value = orgUuid; - document.getElementById(`userOrgType${userOrgTypeName}`).checked = true; + if (ORG_TYPES[userOrgType] !== undefined) { + document.getElementById(`userOrgType${ORG_TYPES[userOrgType].name}`).checked = true; + } }, false); // Prevent accidental submission of the form with valid elements after the modal has been hidden. @@ -255,7 +254,10 @@ function updateUserOrgType(event) { function initUserTable() { // Color all the org buttons per type document.querySelectorAll("button[data-vw-org-type]").forEach(function(e) { - const orgType = ORG_TYPES[e.dataset.vwOrgType]; + const orgType = ORG_TYPES[e.dataset.vwOrgType] ?? { + "name": "Unknown membership type", + "bg": "gray" + }; e.style.backgroundColor = orgType.bg; if (orgType.font !== undefined) { e.style.color = orgType.font; diff --git a/src/static/templates/admin/users.hbs b/src/static/templates/admin/users.hbs index 3bd63446..d848d894 100644 --- a/src/static/templates/admin/users.hbs +++ b/src/static/templates/admin/users.hbs @@ -130,10 +130,7 @@
- -
-
- +
diff --git a/src/static/templates/scss/vaultwarden.scss.hbs b/src/static/templates/scss/vaultwarden.scss.hbs index 11bcbad6..e9a5a514 100644 --- a/src/static/templates/scss/vaultwarden.scss.hbs +++ b/src/static/templates/scss/vaultwarden.scss.hbs @@ -116,9 +116,8 @@ app-security > app-two-factor-setup > form { } /* Hide unsupported Custom Role options */ -/* The collection permission group plus manageUsers, manageGroups, managePolicies are supported - by Vaultwarden and are intentionally not hidden here. */ -:is(bit-dialog, [bit-dialog]) div.tw-col-span-4:has(input[formcontrolname*="access"]), +/* Collection permissions, manageUsers, manageGroups, managePolicies, accessEventLogs, + accessImportExport and accessReports are supported by Vaultwarden and are intentionally not hidden here. */ :is(bit-dialog, [bit-dialog]) bit-form-control:has(input[formcontrolname="manageSso"]), :is(bit-dialog, [bit-dialog]) bit-form-control:has(input[formcontrolname="manageResetPassword"]) { @extend %vw-hide; diff --git a/src/util.rs b/src/util.rs index 91f075d1..4a12d080 100644 --- a/src/util.rs +++ b/src/util.rs @@ -484,8 +484,16 @@ pub fn format_datetime_http(dt: &DateTime) -> String { expiry_time.to_rfc2822().replace("+0000", "GMT") } +pub fn try_parse_date(date: &str) -> Result { + DateTime::parse_from_rfc3339(date).map(|date| date.naive_utc()) +} + +/// Parse an RFC 3339 date which is known to be valid. +/// +/// Request data should use [`try_parse_date`] so malformed input can be returned as a controlled +/// client error instead of panicking. pub fn parse_date(date: &str) -> NaiveDateTime { - DateTime::parse_from_rfc3339(date).unwrap().naive_utc() + try_parse_date(date).expect("trusted date must be valid RFC 3339") } /// Returns true or false if an email address is valid or not