diff --git a/crates/common/src/audit.rs b/crates/common/src/audit.rs index eb882266..b9f4acf1 100644 --- a/crates/common/src/audit.rs +++ b/crates/common/src/audit.rs @@ -308,8 +308,8 @@ impl Server { /// AU-1.6: access to another account's data through `Impersonate` (or a /// blob through `FetchAnyBlob`), once an hour per session's account and - /// target. Access through a share or group membership isn't this: the - /// owner granted it. + /// target. Group membership isn't this. A share is: it covers only what + /// was shared, and nothing tells which part an `Impersonate` holder read. pub async fn audit_foreign_access(&self, token: &AccessToken, target_id: u32, blob: bool) { if target_id == token.account_id() || token.is_member_directly(target_id) { return; diff --git a/crates/common/src/auth/access_token.rs b/crates/common/src/auth/access_token.rs index 0d53b524..2e1f94eb 100644 --- a/crates/common/src/auth/access_token.rs +++ b/crates/common/src/auth/access_token.rs @@ -599,13 +599,13 @@ impl AccessToken { || self.has_permission(Permission::Impersonate) } - /// inbuxa: AU-1.6: whether the account is reachable without - /// impersonation: its own, a group's it belongs to, or one shared with - /// it. + /// inbuxa: AU-1.6: whether the whole account is the token's without + /// impersonation: its own, or a group's it belongs to. A share doesn't + /// count. It grants some folders or calendars, not the account, and an + /// `Impersonate` holder reaches everything else in it unchecked, so + /// their access to a sharing account is audited too (GHSA-m992-gp7g-wc2j). pub fn is_member_directly(&self, account_id: u32) -> bool { - self.inner.account_id == account_id - || self.inner.member_of.contains(&account_id) - || self.inner.access_to.iter().any(|a| a.account_id == account_id) + self.inner.account_id == account_id || self.inner.member_of.contains(&account_id) } /// inbuxa: MA-D0: in the account only because it is a group this token diff --git a/tests/src/system/audit.rs b/tests/src/system/audit.rs index 203fcedf..ed17d2c5 100644 --- a/tests/src/system/audit.rs +++ b/tests/src/system/audit.rs @@ -221,6 +221,66 @@ pub async fn test(test: &mut TestServer) { "test 3" ); + // Test 3b, AU-1.6 (GHSA-m992-gp7g-wc2j): an account that shared one + // folder with the administrator is still audited when the administrator + // reaches the rest of it. A share used to count as the owner's grant for + // the whole account, so nothing was recorded. + let share_secret = "another-secret-password-5521"; + let sharer_id = admin + .create_user_account( + "auditshare@example.org", + share_secret, + "Audit sharer", + &[], + vec![], + ) + .await + .id(); + let sharer = Account::new("auditshare@example.org", share_secret, &[], "", sharer_id); + let mail = ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail"]; + let get_mailboxes = json!([["Mailbox/get", {"accountId": sharer.id_string(), "ids": null}, "0"]]); + let mailboxes = sharer.jmap_request(&mail, get_mailboxes.clone()).await; + let inbox = mailboxes + .0 + .pointer("/methodResponses/0/1/list") + .and_then(|list| list.as_array()) + .and_then(|list| list.iter().find(|m| m["role"] == "inbox")) + .and_then(|m| m["id"].as_str()) + .unwrap_or_else(|| panic!("test 3b: no Inbox in {}", mailboxes.0)) + .to_string(); + let shared = sharer + .jmap_request( + &mail, + json!([["Mailbox/set", { + "accountId": sharer.id_string(), + "update": {inbox: {format!("shareWith/{admin_id}"): {"mayReadItems": true}}} + }, "0"]]), + ) + .await; + assert!( + shared + .0 + .pointer("/methodResponses/0/1/updated") + .is_some_and(|updated| !updated.is_null()), + "test 3b: share: {}", + shared.0 + ); + let response = admin.jmap_request(&mail, get_mailboxes).await; + assert_eq!( + response.0.pointer("/methodResponses/0/0"), + Some(&json!("Mailbox/get")), + "test 3b: {}", + response.0 + ); + let access = admin + .audit(json!({"action": "accountAccess", "accountId": sharer.id_string()})) + .await; + assert_eq!(access.len(), 1, "test 3b: {access:?}"); + assert_eq!( + access[0]["target"]["name"], "auditshare@example.org", + "test 3b" + ); + // AU-9: a plain user can't read the audit log let plain = Account::new( "audituser@example.org",