Browse Source

Document accessReports scope and drop the unused reports guard

Clippy (with --all-targets) reported two issues introduced by this branch:

  events.rs:356  items_after_test_module
  events.rs:395  redundant_clone

Move the test module to the end of the file and drop the redundant clone.
The relocation is content-identical; the only semantic change is the removed
`org_id.clone()`, which was the last use of the binding. Note that CI runs
clippy without --all-targets, so neither of these was failing the pipeline.

Remove `AccessReportsHeaders` and its `can_access_reports` helper. The guard
was never applied to any route and could not be: Vaultwarden has no
server-side report endpoints, because the clients compute every report
locally from the organization cipher list. `accessReports` is therefore
enforced inline in `get_org_details`, where that list is actually served. A
note at both former sites records this, so the guard is not reintroduced and
an endpoint is not gated on "may call reports" instead of "may read these
ciphers".

Document the resulting authorization semantics above `get_org_details`:
the endpoint serializes with CipherSyncType::Organization, which skips the
per-cipher access restrictions, so readOnly/hidePasswords are not applied and
collection assignments are ignored. `accessReports` is by design a full
organization read permission. This matches Bitwarden's CanAccessAllCiphersAsync,
which admits Owner/Admin plus Custom members holding AccessImportExport,
EditAnyCollection or AccessReports. We stay deliberately 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.

No behavioural change.
pull/7397/head
tom27052006 3 weeks ago
parent
commit
52c02ce416
  1. 239
      src/api/core/events.rs
  2. 20
      src/api/core/organizations.rs
  3. 16
      src/auth.rs

239
src/api/core/events.rs

@ -352,6 +352,123 @@ async fn post_events_collect(data: Json<Vec<EventCollection>>, headers: Headers,
Ok(()) Ok(())
} }
pub async fn log_user_event(event_type: i32, user_id: &UserId, device_type: i32, ip: &IpAddr, conn: &DbConn) {
if !CONFIG.org_events_enabled() {
return;
}
log_user_event_impl(event_type, user_id, device_type, None, ip, conn).await;
}
async fn log_user_event_impl(
event_type: i32,
user_id: &UserId,
device_type: i32,
event_date: Option<NaiveDateTime>,
ip: &IpAddr,
conn: &DbConn,
) {
let memberships = Membership::find_confirmed_by_user(user_id, conn).await;
let mut events: Vec<Event> = Vec::with_capacity(memberships.len() + 1); // We need an event per org and one without an org
// Upstream saves the event also without any org_id.
let mut event = Event::new(event_type, event_date);
event.user_uuid = Some(user_id.clone());
event.act_user_uuid = Some(user_id.clone());
event.device_type = Some(device_type);
event.ip_address = Some(ip.to_string());
events.push(event);
// For each org a user is a member of store these events per org
for membership in memberships {
let mut event = Event::new(event_type, event_date);
event.user_uuid = Some(user_id.clone());
event.org_uuid = Some(membership.org_uuid);
event.org_user_uuid = Some(membership.uuid);
event.act_user_uuid = Some(user_id.clone());
event.device_type = Some(device_type);
event.ip_address = Some(ip.to_string());
events.push(event);
}
Event::save_user_event(events, conn).await.unwrap_or(());
}
pub async fn log_event(
event_type: i32,
source_uuid: &str,
org_id: &OrganizationId,
act_user_id: &UserId,
device_type: i32,
ip: &IpAddr,
conn: &DbConn,
) {
if !CONFIG.org_events_enabled() {
return;
}
log_event_impl(event_type, source_uuid, org_id, act_user_id, device_type, None, ip, conn).await;
}
#[expect(clippy::too_many_arguments)]
async fn log_event_impl(
event_type: i32,
source_uuid: &str,
org_id: &OrganizationId,
act_user_id: &UserId,
device_type: i32,
event_date: Option<NaiveDateTime>,
ip: &IpAddr,
conn: &DbConn,
) {
// Create a new empty event
let mut event = Event::new(event_type, event_date);
match event_type {
// 1000..=1099 Are user events, they need to be logged via log_user_event()
// Cipher Events
1100..=1199 => {
event.cipher_uuid = Some(source_uuid.to_owned().into());
}
// Collection Events
1300..=1399 => {
event.collection_uuid = Some(source_uuid.to_owned().into());
}
// Group Events
1400..=1499 => {
event.group_uuid = Some(source_uuid.to_owned().into());
}
// Org User Events
1500..=1599 => {
event.org_user_uuid = Some(source_uuid.to_owned().into());
}
// 1600..=1699 Are organizational events, and they do not need the source_uuid
// Policy Events
1700..=1799 => {
event.policy_uuid = Some(source_uuid.to_owned().into());
}
// Ignore others
_ => {}
}
event.org_uuid = Some(org_id.clone());
event.act_user_uuid = Some(act_user_id.clone());
event.device_type = Some(device_type);
event.ip_address = Some(ip.to_string());
event.save(conn).await.unwrap_or(());
}
pub async fn event_cleanup_job(pool: DbPool) {
debug!("Start events cleanup job");
if CONFIG.events_days_retain().is_none() {
debug!("events_days_retain is not configured, abort");
return;
}
if let Ok(conn) = pool.get().await {
Event::clean_events(&conn).await.ok();
} else {
error!("Failed to get DB connection while trying to cleanup the events table");
}
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
@ -390,10 +507,7 @@ mod tests {
cipher.organization_uuid = Some(org_id.clone()); cipher.organization_uuid = Some(org_id.clone());
let admin = membership(MembershipType::Admin, MembershipStatus::Confirmed); let admin = membership(MembershipType::Admin, MembershipStatus::Confirmed);
assert_eq!( assert_eq!(cipher_event_scope(&cipher, &user_id, Some(&admin)), Some(CipherEventScope::Organization(org_id)));
cipher_event_scope(&cipher, &user_id, Some(&admin)),
Some(CipherEventScope::Organization(org_id.clone()))
);
let accepted_admin = membership(MembershipType::Admin, MembershipStatus::Accepted); let accepted_admin = membership(MembershipType::Admin, MembershipStatus::Accepted);
assert_eq!(cipher_event_scope(&cipher, &user_id, Some(&accepted_admin)), None); assert_eq!(cipher_event_scope(&cipher, &user_id, Some(&accepted_admin)), None);
@ -503,120 +617,3 @@ mod tests {
assert!(validate_client_event_batch_size(MAX_CLIENT_EVENT_BATCH_SIZE + 1).is_err()); assert!(validate_client_event_batch_size(MAX_CLIENT_EVENT_BATCH_SIZE + 1).is_err());
} }
} }
pub async fn log_user_event(event_type: i32, user_id: &UserId, device_type: i32, ip: &IpAddr, conn: &DbConn) {
if !CONFIG.org_events_enabled() {
return;
}
log_user_event_impl(event_type, user_id, device_type, None, ip, conn).await;
}
async fn log_user_event_impl(
event_type: i32,
user_id: &UserId,
device_type: i32,
event_date: Option<NaiveDateTime>,
ip: &IpAddr,
conn: &DbConn,
) {
let memberships = Membership::find_confirmed_by_user(user_id, conn).await;
let mut events: Vec<Event> = Vec::with_capacity(memberships.len() + 1); // We need an event per org and one without an org
// Upstream saves the event also without any org_id.
let mut event = Event::new(event_type, event_date);
event.user_uuid = Some(user_id.clone());
event.act_user_uuid = Some(user_id.clone());
event.device_type = Some(device_type);
event.ip_address = Some(ip.to_string());
events.push(event);
// For each org a user is a member of store these events per org
for membership in memberships {
let mut event = Event::new(event_type, event_date);
event.user_uuid = Some(user_id.clone());
event.org_uuid = Some(membership.org_uuid);
event.org_user_uuid = Some(membership.uuid);
event.act_user_uuid = Some(user_id.clone());
event.device_type = Some(device_type);
event.ip_address = Some(ip.to_string());
events.push(event);
}
Event::save_user_event(events, conn).await.unwrap_or(());
}
pub async fn log_event(
event_type: i32,
source_uuid: &str,
org_id: &OrganizationId,
act_user_id: &UserId,
device_type: i32,
ip: &IpAddr,
conn: &DbConn,
) {
if !CONFIG.org_events_enabled() {
return;
}
log_event_impl(event_type, source_uuid, org_id, act_user_id, device_type, None, ip, conn).await;
}
#[expect(clippy::too_many_arguments)]
async fn log_event_impl(
event_type: i32,
source_uuid: &str,
org_id: &OrganizationId,
act_user_id: &UserId,
device_type: i32,
event_date: Option<NaiveDateTime>,
ip: &IpAddr,
conn: &DbConn,
) {
// Create a new empty event
let mut event = Event::new(event_type, event_date);
match event_type {
// 1000..=1099 Are user events, they need to be logged via log_user_event()
// Cipher Events
1100..=1199 => {
event.cipher_uuid = Some(source_uuid.to_owned().into());
}
// Collection Events
1300..=1399 => {
event.collection_uuid = Some(source_uuid.to_owned().into());
}
// Group Events
1400..=1499 => {
event.group_uuid = Some(source_uuid.to_owned().into());
}
// Org User Events
1500..=1599 => {
event.org_user_uuid = Some(source_uuid.to_owned().into());
}
// 1600..=1699 Are organizational events, and they do not need the source_uuid
// Policy Events
1700..=1799 => {
event.policy_uuid = Some(source_uuid.to_owned().into());
}
// Ignore others
_ => {}
}
event.org_uuid = Some(org_id.clone());
event.act_user_uuid = Some(act_user_id.clone());
event.device_type = Some(device_type);
event.ip_address = Some(ip.to_string());
event.save(conn).await.unwrap_or(());
}
pub async fn event_cleanup_job(pool: DbPool) {
debug!("Start events cleanup job");
if CONFIG.events_days_retain().is_none() {
debug!("events_days_retain is not configured, abort");
return;
}
if let Ok(conn) = pool.get().await {
Event::clean_events(&conn).await.ok();
} else {
error!("Failed to get DB connection while trying to cleanup the events table");
}
}

20
src/api/core/organizations.rs

@ -1069,6 +1069,26 @@ 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?<data..>")] #[get("/ciphers/organization-details?<data..>")]
async fn get_org_details(data: OrgIdData, headers: ManagerHeadersLoose, conn: DbConn) -> JsonResult { async fn get_org_details(data: OrgIdData, headers: ManagerHeadersLoose, conn: DbConn) -> JsonResult {
if data.organization_id != headers.membership.org_uuid { if data.organization_id != headers.membership.org_uuid {

16
src/auth.rs

@ -766,10 +766,11 @@ impl OrgHeaders {
self.is_confirmed() self.is_confirmed()
&& (self.membership_type >= MembershipType::Admin || self.membership.has_access_import_export()) && (self.membership_type >= MembershipType::Admin || self.membership.has_access_import_export())
} }
// NOTE: there is deliberately no `can_access_reports` guard helper. Vaultwarden has no
fn can_access_reports(&self) -> bool { // server-side report endpoints — the clients compute every report locally from the
self.is_confirmed() && (self.membership_type >= MembershipType::Admin || self.membership.has_access_reports()) // 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/<org_id>"), // org_id is usually the second path param ("/organizations/<org_id>"),
@ -970,11 +971,8 @@ generate_manage_headers!(
can_access_import_export, can_access_import_export,
"You need the 'Access Import/Export' permission, or to be an Admin or Owner, to call this endpoint" "You need the 'Access Import/Export' permission, or to be an Admin or Owner, to call this endpoint"
); );
generate_manage_headers!( // NOTE: no `AccessReportsHeaders`. See the note next to `can_access_import_export` above:
AccessReportsHeaders, // `accessReports` guards data (the organization cipher list), not a dedicated endpoint.
can_access_reports,
"You need the 'Access Reports' permission, or to be an Admin or Owner, to call this endpoint"
);
// col_id is usually the fourth path param ("/organizations/<org_id>/collections/<col_id>"), // col_id is usually the fourth path param ("/organizations/<org_id>/collections/<col_id>"),
// but there could be cases where it is a query value. // but there could be cases where it is a query value.

Loading…
Cancel
Save