From ecab7a50ea6947924d2e629b3a9cd843eeb99ec9 Mon Sep 17 00:00:00 2001 From: Stefan Melmuk <509385+stefan0xC@users.noreply.github.com> Date: Mon, 27 Jan 2025 18:21:22 +0100 Subject: [PATCH 1/5] hide already approved (or declined) devices (#5467) --- src/db/models/auth_request.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/db/models/auth_request.rs b/src/db/models/auth_request.rs index d8ca3fac..7f406581 100644 --- a/src/db/models/auth_request.rs +++ b/src/db/models/auth_request.rs @@ -150,6 +150,7 @@ impl AuthRequest { auth_requests::table .filter(auth_requests::user_uuid.eq(user_uuid)) .filter(auth_requests::request_device_identifier.eq(device_uuid)) + .filter(auth_requests::approved.is_null()) .order_by(auth_requests::creation_date.desc()) .first::(conn).ok().from_db() }} From 2c549984c0f78f35d6a8fcce7390a35b71c2344f Mon Sep 17 00:00:00 2001 From: Stefan Melmuk <509385+stefan0xC@users.noreply.github.com> Date: Mon, 27 Jan 2025 18:27:11 +0100 Subject: [PATCH 2/5] let invited members access OrgMemberHeaders (#5461) --- src/auth.rs | 53 ++++++++++++++++++++++++----------- src/db/models/organization.rs | 14 +++++++++ 2 files changed, 51 insertions(+), 16 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index e4827c80..cfb7c30b 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -542,10 +542,29 @@ pub struct OrgHeaders { pub device: Device, pub user: User, pub membership_type: MembershipType, + pub membership_status: MembershipStatus, pub membership: Membership, pub ip: ClientIp, } +impl OrgHeaders { + fn is_member(&self) -> bool { + // NOTE: we don't care about MembershipStatus at the moment because this is only used + // where an invited, accepted or confirmed user is expected if this ever changes or + // if from_i32 is changed to return Some(Revoked) this check needs to be changed accordingly + self.membership_type >= MembershipType::User + } + fn is_confirmed_and_admin(&self) -> bool { + self.membership_status == MembershipStatus::Confirmed && self.membership_type >= MembershipType::Admin + } + fn is_confirmed_and_manager(&self) -> bool { + self.membership_status == MembershipStatus::Confirmed && self.membership_type >= MembershipType::Manager + } + fn is_confirmed_and_owner(&self) -> bool { + self.membership_status == MembershipStatus::Confirmed && self.membership_type == MembershipType::Owner + } +} + #[rocket::async_trait] impl<'r> FromRequest<'r> for OrgHeaders { type Error = &'static str; @@ -574,15 +593,8 @@ impl<'r> FromRequest<'r> for OrgHeaders { }; let user = headers.user; - let membership = match Membership::find_by_user_and_org(&user.uuid, &org_id, &mut conn).await { - Some(member) => { - if member.status == MembershipStatus::Confirmed as i32 { - member - } else { - err_handler!("The current user isn't confirmed member of the organization") - } - } - None => err_handler!("The current user isn't member of the organization"), + let Some(membership) = Membership::find_by_user_and_org(&user.uuid, &org_id, &mut conn).await else { + err_handler!("The current user isn't member of the organization"); }; Outcome::Success(Self { @@ -590,13 +602,22 @@ impl<'r> FromRequest<'r> for OrgHeaders { device: headers.device, user, membership_type: { - if let Some(org_usr_type) = MembershipType::from_i32(membership.atype) { - org_usr_type + if let Some(member_type) = MembershipType::from_i32(membership.atype) { + member_type } else { // This should only happen if the DB is corrupted err_handler!("Unknown user type in the database") } }, + membership_status: { + if let Some(member_status) = MembershipStatus::from_i32(membership.status) { + // NOTE: add additional check for revoked if from_i32 is ever changed + // to return Revoked status. + member_status + } else { + err_handler!("User status is either revoked or invalid.") + } + }, membership, ip: headers.ip, }) @@ -621,7 +642,7 @@ impl<'r> FromRequest<'r> for AdminHeaders { async fn from_request(request: &'r Request<'_>) -> Outcome { let headers = try_outcome!(OrgHeaders::from_request(request).await); - if headers.membership_type >= MembershipType::Admin { + if headers.is_confirmed_and_admin() { Outcome::Success(Self { host: headers.host, device: headers.device, @@ -683,7 +704,7 @@ impl<'r> FromRequest<'r> for ManagerHeaders { async fn from_request(request: &'r Request<'_>) -> Outcome { let headers = try_outcome!(OrgHeaders::from_request(request).await); - if headers.membership_type >= MembershipType::Manager { + if headers.is_confirmed_and_manager() { match get_col_id(request) { Some(col_id) => { let mut conn = match DbConn::from_request(request).await { @@ -738,7 +759,7 @@ impl<'r> FromRequest<'r> for ManagerHeadersLoose { async fn from_request(request: &'r Request<'_>) -> Outcome { let headers = try_outcome!(OrgHeaders::from_request(request).await); - if headers.membership_type >= MembershipType::Manager { + if headers.is_confirmed_and_manager() { Outcome::Success(Self { host: headers.host, device: headers.device, @@ -801,7 +822,7 @@ impl<'r> FromRequest<'r> for OwnerHeaders { async fn from_request(request: &'r Request<'_>) -> Outcome { let headers = try_outcome!(OrgHeaders::from_request(request).await); - if headers.membership_type == MembershipType::Owner { + if headers.is_confirmed_and_owner() { Outcome::Success(Self { device: headers.device, user: headers.user, @@ -826,7 +847,7 @@ impl<'r> FromRequest<'r> for OrgMemberHeaders { async fn from_request(request: &'r Request<'_>) -> Outcome { let headers = try_outcome!(OrgHeaders::from_request(request).await); - if headers.membership_type >= MembershipType::User { + if headers.is_member() { Outcome::Success(Self { host: headers.host, user: headers.user, diff --git a/src/db/models/organization.rs b/src/db/models/organization.rs index 1aa5fb40..aa3e1d01 100644 --- a/src/db/models/organization.rs +++ b/src/db/models/organization.rs @@ -55,6 +55,7 @@ db_object! { } // https://github.com/bitwarden/server/blob/b86a04cef9f1e1b82cf18e49fc94e017c641130c/src/Core/Enums/OrganizationUserStatusType.cs +#[derive(PartialEq)] pub enum MembershipStatus { Revoked = -1, Invited = 0, @@ -62,6 +63,19 @@ pub enum MembershipStatus { Confirmed = 2, } +impl MembershipStatus { + pub fn from_i32(status: i32) -> Option { + match status { + 0 => Some(Self::Invited), + 1 => Some(Self::Accepted), + 2 => Some(Self::Confirmed), + // NOTE: we don't care about revoked members where this is used + // if this ever changes also adapt the OrgHeaders check. + _ => None, + } + } +} + #[derive(Copy, Clone, PartialEq, Eq, num_derive::FromPrimitive)] pub enum MembershipType { Owner = 0, From 1b46c803894091683bafef880c09f5d7f62f8b5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Win=E2=80=AE8201=E2=80=ADLinux=E2=80=AC?= Date: Tue, 28 Jan 2025 02:29:24 +0900 Subject: [PATCH 3/5] Make sure the icons are displayed correctly in desktop clients (#5469) --- src/util.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/util.rs b/src/util.rs index 76de40d1..ecd079cf 100644 --- a/src/util.rs +++ b/src/util.rs @@ -55,7 +55,10 @@ impl Fairing for AppHeaders { res.set_raw_header("Referrer-Policy", "same-origin"); res.set_raw_header("X-Content-Type-Options", "nosniff"); res.set_raw_header("X-Robots-Tag", "noindex, nofollow"); - res.set_raw_header("Cross-Origin-Resource-Policy", "same-origin"); + + if !res.headers().get_one("Content-Type").is_some_and(|v| v.starts_with("image/")) { + res.set_raw_header("Cross-Origin-Resource-Policy", "same-origin"); + } // Obsolete in modern browsers, unsafe (XS-Leak), and largely replaced by CSP res.set_raw_header("X-XSS-Protection", "0"); From c0ebe0d9828800a2a24e436c679b092eb5351841 Mon Sep 17 00:00:00 2001 From: Mathijs van Veluw Date: Mon, 27 Jan 2025 20:16:59 +0100 Subject: [PATCH 4/5] Fix passwordRevisionDate format (#5477) --- src/db/models/cipher.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/db/models/cipher.rs b/src/db/models/cipher.rs index c751491e..d9dbd28d 100644 --- a/src/db/models/cipher.rs +++ b/src/db/models/cipher.rs @@ -142,7 +142,7 @@ impl Cipher { sync_type: CipherSyncType, conn: &mut DbConn, ) -> Value { - use crate::util::format_date; + use crate::util::{format_date, validate_and_format_date}; let mut attachments_json: Value = Value::Null; if let Some(cipher_sync_data) = cipher_sync_data { @@ -220,7 +220,7 @@ impl Cipher { }) .map(|mut d| match d.get("lastUsedDate").and_then(|l| l.as_str()) { Some(l) => { - d["lastUsedDate"] = json!(crate::util::validate_and_format_date(l)); + d["lastUsedDate"] = json!(validate_and_format_date(l)); d } _ => { @@ -261,6 +261,11 @@ impl Cipher { type_data_json["uri"] = uris[0]["uri"].clone(); } } + + // Check if `passwordRevisionDate` is a valid date, else convert it + if let Some(pw_revision) = type_data_json["passwordRevisionDate"].as_str() { + type_data_json["passwordRevisionDate"] = json!(validate_and_format_date(pw_revision)); + } } // Fix secure note issues when data is invalid From a3dccee243a669fa860e0be9bdd21fda47184911 Mon Sep 17 00:00:00 2001 From: Stefan Melmuk <509385+stefan0xC@users.noreply.github.com> Date: Tue, 28 Jan 2025 11:25:53 +0100 Subject: [PATCH 5/5] add and use new event types (#5482) * add additional event_types * use correct event_type when leaving an org * use correct event type when deleting a user * also correctly log auth requests * add correct membership info to event log --- src/api/admin.rs | 2 +- src/api/core/accounts.rs | 26 ++++++++++++++++++++++++++ src/api/core/events.rs | 9 +++++---- src/api/core/organizations.rs | 2 +- src/db/models/event.rs | 15 +++++++++++++++ 5 files changed, 48 insertions(+), 6 deletions(-) diff --git a/src/api/admin.rs b/src/api/admin.rs index a5a12e8a..b3e703d9 100644 --- a/src/api/admin.rs +++ b/src/api/admin.rs @@ -403,7 +403,7 @@ async fn delete_user(user_id: UserId, token: AdminToken, mut conn: DbConn) -> Em for membership in memberships { log_event( - EventType::OrganizationUserRemoved as i32, + EventType::OrganizationUserDeleted as i32, &membership.uuid, &membership.org_uuid, &ACTING_ADMIN_USER.into(), diff --git a/src/api/core/accounts.rs b/src/api/core/accounts.rs index 473c0c86..3c573811 100644 --- a/src/api/core/accounts.rs +++ b/src/api/core/accounts.rs @@ -1206,6 +1206,15 @@ async fn post_auth_request( nt.send_auth_request(&user.uuid, &auth_request.uuid, &data.device_identifier, &mut conn).await; + log_user_event( + EventType::UserRequestedDeviceApproval as i32, + &user.uuid, + client_headers.device_type, + &client_headers.ip.ip, + &mut conn, + ) + .await; + Ok(Json(json!({ "id": auth_request.uuid, "publicKey": auth_request.public_key, @@ -1287,9 +1296,26 @@ async fn put_auth_request( ant.send_auth_response(&auth_request.user_uuid, &auth_request.uuid).await; nt.send_auth_response(&auth_request.user_uuid, &auth_request.uuid, &data.device_identifier, &mut conn).await; + + log_user_event( + EventType::OrganizationUserApprovedAuthRequest as i32, + &headers.user.uuid, + headers.device.atype, + &headers.ip.ip, + &mut conn, + ) + .await; } else { // If denied, there's no reason to keep the request auth_request.delete(&mut conn).await?; + log_user_event( + EventType::OrganizationUserRejectedAuthRequest as i32, + &headers.user.uuid, + headers.device.atype, + &headers.ip.ip, + &mut conn, + ) + .await; } Ok(Json(json!({ diff --git a/src/api/core/events.rs b/src/api/core/events.rs index 012f46cc..3a7d41f0 100644 --- a/src/api/core/events.rs +++ b/src/api/core/events.rs @@ -245,8 +245,8 @@ async fn _log_user_event( ip: &IpAddr, conn: &mut DbConn, ) { - let orgs = Membership::get_orgs_by_user(user_id, conn).await; - let mut events: Vec = Vec::with_capacity(orgs.len() + 1); // We need an event per org and one without an org + let memberships = Membership::find_by_user(user_id, conn).await; + let mut events: Vec = 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); @@ -257,10 +257,11 @@ async fn _log_user_event( events.push(event); // For each org a user is a member of store these events per org - for org_id in orgs { + for membership in memberships { let mut event = Event::new(event_type, event_date); event.user_uuid = Some(user_id.clone()); - event.org_uuid = Some(org_id); + 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()); diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index c610f5b5..08305839 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -251,7 +251,7 @@ async fn leave_organization(org_id: OrganizationId, headers: Headers, mut conn: } log_event( - EventType::OrganizationUserRemoved as i32, + EventType::OrganizationUserLeft as i32, &member.uuid, &org_id, &headers.user.uuid, diff --git a/src/db/models/event.rs b/src/db/models/event.rs index 5fea7160..ed4582b1 100644 --- a/src/db/models/event.rs +++ b/src/db/models/event.rs @@ -49,6 +49,8 @@ pub enum EventType { UserClientExportedVault = 1007, // UserUpdatedTempPassword = 1008, // Not supported // UserMigratedKeyToKeyConnector = 1009, // Not supported + UserRequestedDeviceApproval = 1010, + // UserTdeOffboardingPasswordSet = 1011, // Not supported // Cipher CipherCreated = 1100, @@ -69,6 +71,7 @@ pub enum EventType { CipherSoftDeleted = 1115, CipherRestored = 1116, CipherClientToggledCardNumberVisible = 1117, + CipherClientToggledTOTPSeedVisible = 1118, // Collection CollectionCreated = 1300, @@ -94,6 +97,10 @@ pub enum EventType { // OrganizationUserFirstSsoLogin = 1510, // Not supported OrganizationUserRevoked = 1511, OrganizationUserRestored = 1512, + OrganizationUserApprovedAuthRequest = 1513, + OrganizationUserRejectedAuthRequest = 1514, + OrganizationUserDeleted = 1515, + OrganizationUserLeft = 1516, // Organization OrganizationUpdated = 1600, @@ -105,6 +112,7 @@ pub enum EventType { // OrganizationEnabledKeyConnector = 1606, // Not supported // OrganizationDisabledKeyConnector = 1607, // Not supported // OrganizationSponsorshipsSynced = 1608, // Not supported + // OrganizationCollectionManagementUpdated = 1609, // Not supported // Policy PolicyUpdated = 1700, @@ -117,6 +125,13 @@ pub enum EventType { // ProviderOrganizationAdded = 1901, // Not supported // ProviderOrganizationRemoved = 1902, // Not supported // ProviderOrganizationVaultAccessed = 1903, // Not supported + + // OrganizationDomainAdded = 2000, // Not supported + // OrganizationDomainRemoved = 2001, // Not supported + // OrganizationDomainVerified = 2002, // Not supported + // OrganizationDomainNotVerified = 2003, // Not supported + + // SecretRetrieved = 2100, // Not supported } /// Local methods