From 787822854cb73aeb8375a4e69ea188aa5f02db02 Mon Sep 17 00:00:00 2001 From: Mathijs van Veluw Date: Sun, 29 Mar 2026 23:15:48 +0200 Subject: [PATCH] Misc org fixes (#7032) * Split vault org/personal purge endpoints Signed-off-by: BlackDex * Adjust several other call-sites Signed-off-by: BlackDex * Several other misc fixes Signed-off-by: BlackDex * Add some more validation for groups, collections and memberships Signed-off-by: BlackDex --------- Signed-off-by: BlackDex --- src/api/core/accounts.rs | 16 ++- src/api/core/ciphers.rs | 116 +++++++++++---------- src/api/core/events.rs | 2 +- src/api/core/organizations.rs | 188 +++++++++++++++++++++++----------- src/api/core/public.rs | 2 +- src/auth.rs | 51 ++++++--- src/db/models/cipher.rs | 51 +++++---- src/db/models/collection.rs | 22 ++-- src/db/models/group.rs | 82 +++++++++++---- src/db/models/org_policy.rs | 2 +- src/db/models/organization.rs | 4 +- src/db/models/two_factor.rs | 1 - 12 files changed, 343 insertions(+), 194 deletions(-) diff --git a/src/api/core/accounts.rs b/src/api/core/accounts.rs index e0869c63..ba36fc9b 100644 --- a/src/api/core/accounts.rs +++ b/src/api/core/accounts.rs @@ -106,7 +106,6 @@ pub struct RegisterData { name: Option, - #[allow(dead_code)] organization_user_id: Option, // Used only from the register/finish endpoint @@ -376,14 +375,12 @@ async fn post_set_password(data: Json, headers: Headers, conn: if let Some(identifier) = data.org_identifier { if identifier != crate::sso::FAKE_IDENTIFIER && identifier != crate::api::admin::FAKE_ADMIN_UUID { - let org = match Organization::find_by_uuid(&identifier.into(), &conn).await { - None => err!("Failed to retrieve the associated organization"), - Some(org) => org, + let Some(org) = Organization::find_by_uuid(&identifier.into(), &conn).await else { + err!("Failed to retrieve the associated organization") }; - let membership = match Membership::find_by_user_and_org(&user.uuid, &org.uuid, &conn).await { - None => err!("Failed to retrieve the invitation"), - Some(org) => org, + let Some(membership) = Membership::find_by_user_and_org(&user.uuid, &org.uuid, &conn).await else { + err!("Failed to retrieve the invitation") }; accept_org_invite(&user, membership, None, &conn).await?; @@ -583,7 +580,6 @@ fn set_kdf_data(user: &mut User, data: &KDFData) -> EmptyResult { Ok(()) } -#[allow(dead_code)] #[derive(Deserialize)] #[serde(rename_all = "camelCase")] struct AuthenticationData { @@ -592,7 +588,6 @@ struct AuthenticationData { master_password_authentication_hash: String, } -#[allow(dead_code)] #[derive(Deserialize)] #[serde(rename_all = "camelCase")] struct UnlockData { @@ -601,11 +596,12 @@ struct UnlockData { master_key_wrapped_user_key: String, } -#[allow(dead_code)] #[derive(Deserialize)] #[serde(rename_all = "camelCase")] struct ChangeKdfData { + #[allow(dead_code)] new_master_password_hash: String, + #[allow(dead_code)] key: String, authentication_data: AuthenticationData, unlock_data: UnlockData, diff --git a/src/api/core/ciphers.rs b/src/api/core/ciphers.rs index f7bf5cd3..8a8d9838 100644 --- a/src/api/core/ciphers.rs +++ b/src/api/core/ciphers.rs @@ -14,7 +14,7 @@ use crate::auth::ClientVersion; use crate::util::{save_temp_file, NumberOrString}; use crate::{ api::{self, core::log_event, EmptyResult, JsonResult, Notify, PasswordOrOtpData, UpdateType}, - auth::Headers, + auth::{Headers, OrgIdGuard, OwnerHeaders}, config::PathType, crypto, db::{ @@ -86,7 +86,8 @@ pub fn routes() -> Vec { restore_cipher_put_admin, restore_cipher_selected, restore_cipher_selected_admin, - delete_all, + purge_org_vault, + purge_personal_vault, move_cipher_selected, move_cipher_selected_put, put_collections2_update, @@ -425,7 +426,7 @@ pub async fn update_cipher_from_data( let transfer_cipher = cipher.organization_uuid.is_none() && data.organization_id.is_some(); if let Some(org_id) = data.organization_id { - match Membership::find_by_user_and_org(&headers.user.uuid, &org_id, conn).await { + match Membership::find_confirmed_by_user_and_org(&headers.user.uuid, &org_id, conn).await { None => err!("You don't have permission to add item to organization"), Some(member) => { if shared_to_collections.is_some() @@ -1642,9 +1643,51 @@ struct OrganizationIdData { org_id: OrganizationId, } +// Use the OrgIdGuard here, to ensure there an organization id present. +// If there is no organization id present, it should be forwarded to purge_personal_vault. +// This guard needs to be the first argument, else OwnerHeaders will be triggered which will logout the user. #[post("/ciphers/purge?", data = "")] -async fn delete_all( - organization: Option, +async fn purge_org_vault( + _org_id_guard: OrgIdGuard, + organization: OrganizationIdData, + data: Json, + headers: OwnerHeaders, + conn: DbConn, + nt: Notify<'_>, +) -> EmptyResult { + if organization.org_id != headers.org_id { + err!("Organization not found", "Organization id's do not match"); + } + + let data: PasswordOrOtpData = data.into_inner(); + let user = headers.user; + + data.validate(&user, true, &conn).await?; + + match Membership::find_confirmed_by_user_and_org(&user.uuid, &organization.org_id, &conn).await { + Some(member) if member.atype == MembershipType::Owner => { + Cipher::delete_all_by_organization(&organization.org_id, &conn).await?; + nt.send_user_update(UpdateType::SyncVault, &user, &headers.device.push_uuid, &conn).await; + + log_event( + EventType::OrganizationPurgedVault as i32, + &organization.org_id, + &organization.org_id, + &user.uuid, + headers.device.atype, + &headers.ip.ip, + &conn, + ) + .await; + + Ok(()) + } + _ => err!("You don't have permission to purge the organization vault"), + } +} + +#[post("/ciphers/purge", data = "")] +async fn purge_personal_vault( data: Json, headers: Headers, conn: DbConn, @@ -1655,52 +1698,18 @@ async fn delete_all( data.validate(&user, true, &conn).await?; - match organization { - Some(org_data) => { - // Organization ID in query params, purging organization vault - match Membership::find_by_user_and_org(&user.uuid, &org_data.org_id, &conn).await { - None => err!("You don't have permission to purge the organization vault"), - Some(member) => { - if member.atype == MembershipType::Owner { - Cipher::delete_all_by_organization(&org_data.org_id, &conn).await?; - nt.send_user_update(UpdateType::SyncVault, &user, &headers.device.push_uuid, &conn).await; - - log_event( - EventType::OrganizationPurgedVault as i32, - &org_data.org_id, - &org_data.org_id, - &user.uuid, - headers.device.atype, - &headers.ip.ip, - &conn, - ) - .await; - - Ok(()) - } else { - err!("You don't have permission to purge the organization vault"); - } - } - } - } - None => { - // No organization ID in query params, purging user vault - // Delete ciphers and their attachments - for cipher in Cipher::find_owned_by_user(&user.uuid, &conn).await { - cipher.delete(&conn).await?; - } - - // Delete folders - for f in Folder::find_by_user(&user.uuid, &conn).await { - f.delete(&conn).await?; - } - - user.update_revision(&conn).await?; - nt.send_user_update(UpdateType::SyncVault, &user, &headers.device.push_uuid, &conn).await; - - Ok(()) - } + for cipher in Cipher::find_owned_by_user(&user.uuid, &conn).await { + cipher.delete(&conn).await?; } + + for f in Folder::find_by_user(&user.uuid, &conn).await { + f.delete(&conn).await?; + } + + user.update_revision(&conn).await?; + nt.send_user_update(UpdateType::SyncVault, &user, &headers.device.push_uuid, &conn).await; + + Ok(()) } #[derive(PartialEq)] @@ -1980,8 +1989,11 @@ impl CipherSyncData { } // Generate a HashMap with the Organization UUID as key and the Membership record - let members: HashMap = - Membership::find_by_user(user_id, conn).await.into_iter().map(|m| (m.org_uuid.clone(), m)).collect(); + let members: HashMap = Membership::find_confirmed_by_user(user_id, conn) + .await + .into_iter() + .map(|m| (m.org_uuid.clone(), m)) + .collect(); // Generate a HashMap with the User_Collections UUID as key and the CollectionUser record let user_collections: HashMap = CollectionUser::find_by_user(user_id, conn) diff --git a/src/api/core/events.rs b/src/api/core/events.rs index 2f33a407..d1612255 100644 --- a/src/api/core/events.rs +++ b/src/api/core/events.rs @@ -240,7 +240,7 @@ async fn _log_user_event( ip: &IpAddr, conn: &DbConn, ) { - let memberships = Membership::find_by_user(user_id, conn).await; + let memberships = Membership::find_confirmed_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. diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index 0beb7036..36e3e4a0 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -131,6 +131,24 @@ struct FullCollectionData { external_id: Option, } +impl FullCollectionData { + pub async fn validate(&self, org_id: &OrganizationId, conn: &DbConn) -> EmptyResult { + let org_groups = Group::find_by_organization(org_id, conn).await; + let org_group_ids: HashSet<&GroupId> = org_groups.iter().map(|c| &c.uuid).collect(); + if let Some(e) = self.groups.iter().find(|g| !org_group_ids.contains(&g.id)) { + err!("Invalid group", format!("Group {} does not belong to organization {}!", e.id, org_id)) + } + + let org_memberships = Membership::find_by_org(org_id, conn).await; + let org_membership_ids: HashSet<&MembershipId> = org_memberships.iter().map(|m| &m.uuid).collect(); + if let Some(e) = self.users.iter().find(|m| !org_membership_ids.contains(&m.id)) { + err!("Invalid member", format!("Member {} does not belong to organization {}!", e.id, org_id)) + } + + Ok(()) + } +} + #[derive(Deserialize)] #[serde(rename_all = "camelCase")] struct CollectionGroupData { @@ -233,30 +251,30 @@ async fn post_delete_organization( } #[post("/organizations//leave")] -async fn leave_organization(org_id: OrganizationId, headers: Headers, conn: DbConn) -> EmptyResult { - match Membership::find_by_user_and_org(&headers.user.uuid, &org_id, &conn).await { - None => err!("User not part of organization"), - Some(member) => { - if member.atype == MembershipType::Owner - && Membership::count_confirmed_by_org_and_type(&org_id, MembershipType::Owner, &conn).await <= 1 - { - err!("The last owner can't leave") - } - - log_event( - EventType::OrganizationUserLeft as i32, - &member.uuid, - &org_id, - &headers.user.uuid, - headers.device.atype, - &headers.ip.ip, - &conn, - ) - .await; - - member.delete(&conn).await - } +async fn leave_organization(org_id: OrganizationId, headers: OrgMemberHeaders, conn: DbConn) -> EmptyResult { + if headers.membership.status != MembershipStatus::Confirmed as i32 { + err!("You need to be a Member of the Organization to call this endpoint") } + let membership = headers.membership; + + if membership.atype == MembershipType::Owner + && Membership::count_confirmed_by_org_and_type(&org_id, MembershipType::Owner, &conn).await <= 1 + { + err!("The last owner can't leave") + } + + log_event( + EventType::OrganizationUserLeft as i32, + &membership.uuid, + &org_id, + &headers.user.uuid, + headers.device.atype, + &headers.ip.ip, + &conn, + ) + .await; + + membership.delete(&conn).await } #[get("/organizations/")] @@ -480,12 +498,9 @@ async fn post_organization_collections( err!("Organization not found", "Organization id's do not match"); } let data: FullCollectionData = data.into_inner(); + data.validate(&org_id, &conn).await?; - let Some(org) = Organization::find_by_uuid(&org_id, &conn).await else { - err!("Can't find organization details") - }; - - let collection = Collection::new(org.uuid, data.name, data.external_id); + let collection = Collection::new(org_id.clone(), data.name, data.external_id); collection.save(&conn).await?; log_event( @@ -501,7 +516,7 @@ async fn post_organization_collections( for group in data.groups { CollectionGroup::new(collection.uuid.clone(), group.id, group.read_only, group.hide_passwords, group.manage) - .save(&conn) + .save(&org_id, &conn) .await?; } @@ -579,10 +594,10 @@ async fn post_bulk_access_collections( ) .await; - CollectionGroup::delete_all_by_collection(&col_id, &conn).await?; + CollectionGroup::delete_all_by_collection(&col_id, &org_id, &conn).await?; for group in &data.groups { CollectionGroup::new(col_id.clone(), group.id.clone(), group.read_only, group.hide_passwords, group.manage) - .save(&conn) + .save(&org_id, &conn) .await?; } @@ -627,6 +642,7 @@ async fn post_organization_collection_update( err!("Organization not found", "Organization id's do not match"); } let data: FullCollectionData = data.into_inner(); + data.validate(&org_id, &conn).await?; if Organization::find_by_uuid(&org_id, &conn).await.is_none() { err!("Can't find organization details") @@ -655,11 +671,11 @@ async fn post_organization_collection_update( ) .await; - CollectionGroup::delete_all_by_collection(&col_id, &conn).await?; + CollectionGroup::delete_all_by_collection(&col_id, &org_id, &conn).await?; for group in data.groups { CollectionGroup::new(col_id.clone(), group.id, group.read_only, group.hide_passwords, group.manage) - .save(&conn) + .save(&org_id, &conn) .await?; } @@ -1003,6 +1019,24 @@ struct InviteData { permissions: HashMap, } +impl InviteData { + async fn validate(&self, org_id: &OrganizationId, conn: &DbConn) -> EmptyResult { + let org_collections = Collection::find_by_organization(org_id, conn).await; + let org_collection_ids: HashSet<&CollectionId> = org_collections.iter().map(|c| &c.uuid).collect(); + if let Some(e) = self.collections.iter().flatten().find(|c| !org_collection_ids.contains(&c.id)) { + err!("Invalid collection", format!("Collection {} does not belong to organization {}!", e.id, org_id)) + } + + let org_groups = Group::find_by_organization(org_id, conn).await; + let org_group_ids: HashSet<&GroupId> = org_groups.iter().map(|c| &c.uuid).collect(); + if let Some(e) = self.groups.iter().find(|g| !org_group_ids.contains(g)) { + err!("Invalid group", format!("Group {} does not belong to organization {}!", e, org_id)) + } + + Ok(()) + } +} + #[post("/organizations//users/invite", data = "")] async fn send_invite( org_id: OrganizationId, @@ -1014,6 +1048,7 @@ async fn send_invite( err!("Organization not found", "Organization id's do not match"); } let data: InviteData = data.into_inner(); + data.validate(&org_id, &conn).await?; // HACK: We need the raw user-type to be sure custom role is selected to determine the access_all permission // The from_str() will convert the custom role type into a manager role type @@ -1273,20 +1308,20 @@ async fn accept_invite( // skip invitation logic when we were invited via the /admin panel if **member_id != FAKE_ADMIN_UUID { - let Some(mut member) = Membership::find_by_uuid_and_org(member_id, &claims.org_id, &conn).await else { + let Some(mut membership) = Membership::find_by_uuid_and_org(member_id, &claims.org_id, &conn).await else { err!("Error accepting the invitation") }; - let reset_password_key = match OrgPolicy::org_is_reset_password_auto_enroll(&member.org_uuid, &conn).await { + let reset_password_key = match OrgPolicy::org_is_reset_password_auto_enroll(&membership.org_uuid, &conn).await { true if data.reset_password_key.is_none() => err!("Reset password key is required, but not provided."), true => data.reset_password_key, false => None, }; // In case the user was invited before the mail was saved in db. - member.invited_by_email = member.invited_by_email.or(claims.invited_by_email); + membership.invited_by_email = membership.invited_by_email.or(claims.invited_by_email); - accept_org_invite(&headers.user, member, reset_password_key, &conn).await?; + accept_org_invite(&headers.user, membership, reset_password_key, &conn).await?; } else if CONFIG.mail_enabled() { // User was invited from /admin, so they are automatically confirmed let org_name = CONFIG.invitation_org_name(); @@ -1520,9 +1555,8 @@ async fn edit_member( && data.permissions.get("deleteAnyCollection") == Some(&json!(true)) && data.permissions.get("createNewCollections") == Some(&json!(true))); - let mut member_to_edit = match Membership::find_by_uuid_and_org(&member_id, &org_id, &conn).await { - Some(member) => member, - None => err!("The specified user isn't member of the organization"), + 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") }; if new_type != member_to_edit.atype @@ -1839,7 +1873,6 @@ async fn post_org_import( #[derive(Deserialize)] #[serde(rename_all = "camelCase")] -#[allow(dead_code)] struct BulkCollectionsData { organization_id: OrganizationId, cipher_ids: Vec, @@ -1853,6 +1886,10 @@ struct BulkCollectionsData { async fn post_bulk_collections(data: Json, headers: Headers, conn: DbConn) -> EmptyResult { let data: BulkCollectionsData = data.into_inner(); + if Membership::find_confirmed_by_user_and_org(&headers.user.uuid, &data.organization_id, &conn).await.is_none() { + err!("You need to be a Member of the Organization to call this endpoint") + } + // Get all the collection available to the user in one query // Also filter based upon the provided collections let user_collections: HashMap = @@ -1941,7 +1978,7 @@ async fn list_policies_token(org_id: OrganizationId, token: &str, conn: DbConn) // Called during the SSO enrollment. // Return the org policy if it exists, otherwise use the default one. #[get("/organizations//policies/master-password", rank = 1)] -async fn get_master_password_policy(org_id: OrganizationId, _headers: Headers, conn: DbConn) -> JsonResult { +async fn get_master_password_policy(org_id: OrganizationId, _headers: OrgMemberHeaders, conn: DbConn) -> JsonResult { let policy = OrgPolicy::find_by_org_and_type(&org_id, OrgPolicyType::MasterPassword, &conn).await.unwrap_or_else(|| { let (enabled, data) = match CONFIG.sso_master_password_policy_value() { @@ -2149,13 +2186,13 @@ fn get_plans() -> Json { } #[get("/organizations/<_org_id>/billing/metadata")] -fn get_billing_metadata(_org_id: OrganizationId, _headers: Headers) -> Json { +fn get_billing_metadata(_org_id: OrganizationId, _headers: OrgMemberHeaders) -> Json { // Prevent a 404 error, which also causes Javascript errors. Json(_empty_data_json()) } #[get("/organizations/<_org_id>/billing/vnext/warnings")] -fn get_billing_warnings(_org_id: OrganizationId, _headers: Headers) -> Json { +fn get_billing_warnings(_org_id: OrganizationId, _headers: OrgMemberHeaders) -> Json { Json(json!({ "freeTrial":null, "inactiveSubscription":null, @@ -2427,6 +2464,23 @@ impl GroupRequest { group } + + /// Validate if all the collections and members belong to the provided organization + pub async fn validate(&self, org_id: &OrganizationId, conn: &DbConn) -> EmptyResult { + let org_collections = Collection::find_by_organization(org_id, conn).await; + let org_collection_ids: HashSet<&CollectionId> = org_collections.iter().map(|c| &c.uuid).collect(); + if let Some(e) = self.collections.iter().find(|c| !org_collection_ids.contains(&c.id)) { + err!("Invalid collection", format!("Collection {} does not belong to organization {}!", e.id, org_id)) + } + + let org_memberships = Membership::find_by_org(org_id, conn).await; + let org_membership_ids: HashSet<&MembershipId> = org_memberships.iter().map(|m| &m.uuid).collect(); + if let Some(e) = self.users.iter().find(|m| !org_membership_ids.contains(m)) { + err!("Invalid member", format!("Member {} does not belong to organization {}!", e, org_id)) + } + + Ok(()) + } } #[derive(Deserialize, Serialize)] @@ -2470,6 +2524,8 @@ async fn post_groups( } let group_request = data.into_inner(); + group_request.validate(&org_id, &conn).await?; + let group = group_request.to_group(&org_id); log_event( @@ -2506,10 +2562,12 @@ async fn put_group( }; let group_request = data.into_inner(); + group_request.validate(&org_id, &conn).await?; + let updated_group = group_request.update_group(group); - CollectionGroup::delete_all_by_group(&group_id, &conn).await?; - GroupUser::delete_all_by_group(&group_id, &conn).await?; + CollectionGroup::delete_all_by_group(&group_id, &org_id, &conn).await?; + GroupUser::delete_all_by_group(&group_id, &org_id, &conn).await?; log_event( EventType::GroupUpdated as i32, @@ -2537,7 +2595,7 @@ async fn add_update_group( for col_selection in collections { let mut collection_group = col_selection.to_collection_group(group.uuid.clone()); - collection_group.save(conn).await?; + collection_group.save(&org_id, conn).await?; } for assigned_member in members { @@ -2630,7 +2688,7 @@ async fn _delete_group( ) .await; - group.delete(conn).await + group.delete(org_id, conn).await } #[delete("/organizations//groups", data = "")] @@ -2689,7 +2747,7 @@ async fn get_group_members( err!("Group could not be found!", "Group uuid is invalid or does not belong to the organization") }; - let group_members: Vec = GroupUser::find_by_group(&group_id, &conn) + let group_members: Vec = GroupUser::find_by_group(&group_id, &org_id, &conn) .await .iter() .map(|entry| entry.users_organizations_uuid.clone()) @@ -2717,9 +2775,15 @@ async fn put_group_members( err!("Group could not be found!", "Group uuid is invalid or does not belong to the organization") }; - GroupUser::delete_all_by_group(&group_id, &conn).await?; - let assigned_members = data.into_inner(); + + let org_memberships = Membership::find_by_org(&org_id, &conn).await; + let org_membership_ids: HashSet<&MembershipId> = org_memberships.iter().map(|m| &m.uuid).collect(); + if let Some(e) = assigned_members.iter().find(|m| !org_membership_ids.contains(m)) { + err!("Invalid member", format!("Member {} does not belong to organization {}!", e, org_id)) + } + + GroupUser::delete_all_by_group(&group_id, &org_id, &conn).await?; for assigned_member in assigned_members { let mut user_entry = GroupUser::new(group_id.clone(), assigned_member.clone()); user_entry.save(&conn).await?; @@ -2951,15 +3015,20 @@ async fn check_reset_password_applicable(org_id: &OrganizationId, conn: &DbConn) Ok(()) } -#[put("/organizations//users//reset-password-enrollment", data = "")] +#[put("/organizations//users//reset-password-enrollment", data = "")] async fn put_reset_password_enrollment( org_id: OrganizationId, - member_id: MembershipId, - headers: Headers, + user_id: UserId, + headers: OrgMemberHeaders, data: Json, conn: DbConn, ) -> EmptyResult { - let Some(mut member) = Membership::find_by_user_and_org(&headers.user.uuid, &org_id, &conn).await else { + if user_id != headers.user.uuid { + err!("User to enroll isn't member of required organization", "The user_id and acting user do not match"); + } + + let Some(mut membership) = Membership::find_confirmed_by_user_and_org(&headers.user.uuid, &org_id, &conn).await + else { err!("User to enroll isn't member of required organization") }; @@ -2986,16 +3055,17 @@ async fn put_reset_password_enrollment( .await?; } - member.reset_password_key = reset_password_key; - member.save(&conn).await?; + membership.reset_password_key = reset_password_key; + membership.save(&conn).await?; - let log_id = if member.reset_password_key.is_some() { + let event_type = if membership.reset_password_key.is_some() { EventType::OrganizationUserResetPasswordEnroll as i32 } else { EventType::OrganizationUserResetPasswordWithdraw as i32 }; - log_event(log_id, &member_id, &org_id, &headers.user.uuid, headers.device.atype, &headers.ip.ip, &conn).await; + log_event(event_type, &membership.uuid, &org_id, &headers.user.uuid, headers.device.atype, &headers.ip.ip, &conn) + .await; Ok(()) } diff --git a/src/api/core/public.rs b/src/api/core/public.rs index 6a317b96..d757d953 100644 --- a/src/api/core/public.rs +++ b/src/api/core/public.rs @@ -156,7 +156,7 @@ async fn ldap_import(data: Json, token: PublicToken, conn: DbConn } }; - GroupUser::delete_all_by_group(&group_uuid, &conn).await?; + GroupUser::delete_all_by_group(&group_uuid, &org_id, &conn).await?; for ext_id in &group_data.member_external_ids { if let Some(member) = Membership::find_by_external_id_and_org(ext_id, &org_id, &conn).await { diff --git a/src/auth.rs b/src/auth.rs index 99741277..43184369 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -704,10 +704,9 @@ pub struct OrgHeaders { 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 + // Only allow not revoked members, we can not use the Confirmed status here + // as some endpoints can be triggered by invited users during joining + self.membership_status != MembershipStatus::Revoked && self.membership_type >= MembershipType::User } fn is_confirmed_and_admin(&self) -> bool { self.membership_status == MembershipStatus::Confirmed && self.membership_type >= MembershipType::Admin @@ -720,6 +719,36 @@ impl OrgHeaders { } } +// org_id is usually the second path param ("/organizations/"), +// but there are cases where it is a query value. +// First check the path, if this is not a valid uuid, try the query values. +fn get_org_id(request: &Request<'_>) -> Option { + if let Some(Ok(org_id)) = request.param::(1) { + Some(org_id) + } else if let Some(Ok(org_id)) = request.query_value::("organizationId") { + Some(org_id) + } else { + None + } +} + +// Special Guard to ensure that there is an organization id present +// If there is no org id trigger the Outcome::Forward. +// This is useful for endpoints which work for both organization and personal vaults, like purge. +pub struct OrgIdGuard; + +#[rocket::async_trait] +impl<'r> FromRequest<'r> for OrgIdGuard { + type Error = &'static str; + + async fn from_request(request: &'r Request<'_>) -> Outcome { + match get_org_id(request) { + Some(_) => Outcome::Success(OrgIdGuard), + None => Outcome::Forward(rocket::http::Status::NotFound), + } + } +} + #[rocket::async_trait] impl<'r> FromRequest<'r> for OrgHeaders { type Error = &'static str; @@ -727,18 +756,8 @@ impl<'r> FromRequest<'r> for OrgHeaders { async fn from_request(request: &'r Request<'_>) -> Outcome { let headers = try_outcome!(Headers::from_request(request).await); - // org_id is usually the second path param ("/organizations/"), - // but there are cases where it is a query value. - // First check the path, if this is not a valid uuid, try the query values. - let url_org_id: Option = { - if let Some(Ok(org_id)) = request.param::(1) { - Some(org_id) - } else if let Some(Ok(org_id)) = request.query_value::("organizationId") { - Some(org_id) - } else { - None - } - }; + // Extract the org_id from the request + let url_org_id = get_org_id(request); match url_org_id { Some(org_id) if uuid::Uuid::parse_str(&org_id).is_ok() => { diff --git a/src/db/models/cipher.rs b/src/db/models/cipher.rs index b28a25cd..edc5f8c9 100644 --- a/src/db/models/cipher.rs +++ b/src/db/models/cipher.rs @@ -559,7 +559,7 @@ impl Cipher { if let Some(cached_member) = cipher_sync_data.members.get(org_uuid) { return cached_member.has_full_access(); } - } else if let Some(member) = Membership::find_by_user_and_org(user_uuid, org_uuid, conn).await { + } else if let Some(member) = Membership::find_confirmed_by_user_and_org(user_uuid, org_uuid, conn).await { return member.has_full_access(); } } @@ -668,10 +668,12 @@ impl Cipher { ciphers::table .filter(ciphers::uuid.eq(&self.uuid)) .inner_join(ciphers_collections::table.on( - ciphers::uuid.eq(ciphers_collections::cipher_uuid))) + ciphers::uuid.eq(ciphers_collections::cipher_uuid) + )) .inner_join(users_collections::table.on( ciphers_collections::collection_uuid.eq(users_collections::collection_uuid) - .and(users_collections::user_uuid.eq(user_uuid)))) + .and(users_collections::user_uuid.eq(user_uuid)) + )) .select((users_collections::read_only, users_collections::hide_passwords, users_collections::manage)) .load::<(bool, bool, bool)>(conn) .expect("Error getting user access restrictions") @@ -697,6 +699,9 @@ impl Cipher { .inner_join(users_organizations::table.on( users_organizations::uuid.eq(groups_users::users_organizations_uuid) )) + .inner_join(groups::table.on(groups::uuid.eq(collections_groups::groups_uuid) + .and(groups::organizations_uuid.eq(users_organizations::org_uuid)) + )) .filter(users_organizations::user_uuid.eq(user_uuid)) .select((collections_groups::read_only, collections_groups::hide_passwords, collections_groups::manage)) .load::<(bool, bool, bool)>(conn) @@ -795,28 +800,28 @@ impl Cipher { let mut query = ciphers::table .left_join(ciphers_collections::table.on( ciphers::uuid.eq(ciphers_collections::cipher_uuid) - )) + )) .left_join(users_organizations::table.on( - ciphers::organization_uuid.eq(users_organizations::org_uuid.nullable()) + ciphers::organization_uuid.eq(users_organizations::org_uuid.nullable()) .and(users_organizations::user_uuid.eq(user_uuid)) .and(users_organizations::status.eq(MembershipStatus::Confirmed as i32)) - )) + )) .left_join(users_collections::table.on( - ciphers_collections::collection_uuid.eq(users_collections::collection_uuid) + ciphers_collections::collection_uuid.eq(users_collections::collection_uuid) // Ensure that users_collections::user_uuid is NULL for unconfirmed users. .and(users_organizations::user_uuid.eq(users_collections::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) - )) + groups_users::users_organizations_uuid.eq(users_organizations::uuid) + )) + .left_join(groups::table.on(groups::uuid.eq(groups_users::groups_uuid) + // Ensure that group and membership belong to the same org + .and(groups::organizations_uuid.eq(users_organizations::org_uuid)) + )) .left_join(collections_groups::table.on( - collections_groups::collections_uuid.eq(ciphers_collections::collection_uuid).and( - collections_groups::groups_uuid.eq(groups::uuid) - ) - )) + collections_groups::collections_uuid.eq(ciphers_collections::collection_uuid) + .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 .or_filter(users_collections::user_uuid.eq(user_uuid)) // Access to collection @@ -986,7 +991,9 @@ impl Cipher { .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))) + .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::collections_uuid.eq(ciphers_collections::collection_uuid) .and(collections_groups::groups_uuid.eq(groups::uuid)) @@ -1047,7 +1054,9 @@ impl Cipher { .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))) + .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::collections_uuid.eq(ciphers_collections::collection_uuid) .and(collections_groups::groups_uuid.eq(groups::uuid)) @@ -1115,8 +1124,8 @@ impl Cipher { .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) + .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::collections_uuid.eq(ciphers_collections::collection_uuid).and( diff --git a/src/db/models/collection.rs b/src/db/models/collection.rs index 3e6ccf21..b1f82335 100644 --- a/src/db/models/collection.rs +++ b/src/db/models/collection.rs @@ -191,7 +191,7 @@ impl Collection { self.update_users_revision(conn).await; CollectionCipher::delete_all_by_collection(&self.uuid, conn).await?; CollectionUser::delete_all_by_collection(&self.uuid, conn).await?; - CollectionGroup::delete_all_by_collection(&self.uuid, conn).await?; + CollectionGroup::delete_all_by_collection(&self.uuid, &self.org_uuid, conn).await?; db_run! { conn: { diesel::delete(collections::table.filter(collections::uuid.eq(self.uuid))) @@ -239,8 +239,8 @@ impl Collection { .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) + .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( @@ -355,8 +355,8 @@ impl Collection { .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) + .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( @@ -422,8 +422,8 @@ impl Collection { .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) + .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) @@ -484,8 +484,8 @@ impl Collection { .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) + .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( @@ -531,8 +531,8 @@ impl Collection { .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) + .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( diff --git a/src/db/models/group.rs b/src/db/models/group.rs index a24b5325..f41ad9ca 100644 --- a/src/db/models/group.rs +++ b/src/db/models/group.rs @@ -1,6 +1,6 @@ use super::{CollectionId, Membership, MembershipId, OrganizationId, User, UserId}; use crate::api::EmptyResult; -use crate::db::schema::{collections_groups, groups, groups_users, users_organizations}; +use crate::db::schema::{collections, collections_groups, groups, groups_users, users_organizations}; use crate::db::DbConn; use crate::error::MapResult; use chrono::{NaiveDateTime, Utc}; @@ -81,7 +81,7 @@ impl Group { // If both read_only and hide_passwords are false, then manage should be true // You can't have an entry with read_only and manage, or hide_passwords and manage // Or an entry with everything to false - let collections_groups: Vec = CollectionGroup::find_by_group(&self.uuid, conn) + let collections_groups: Vec = CollectionGroup::find_by_group(&self.uuid, &self.organizations_uuid, conn) .await .iter() .map(|entry| { @@ -191,7 +191,7 @@ impl Group { pub async fn delete_all_by_organization(org_uuid: &OrganizationId, conn: &DbConn) -> EmptyResult { for group in Self::find_by_organization(org_uuid, conn).await { - group.delete(conn).await?; + group.delete(org_uuid, conn).await?; } Ok(()) } @@ -246,8 +246,8 @@ impl Group { .inner_join(users_organizations::table.on( users_organizations::uuid.eq(groups_users::users_organizations_uuid) )) - .inner_join(groups::table.on( - groups::uuid.eq(groups_users::groups_uuid) + .inner_join(groups::table.on(groups::uuid.eq(groups_users::groups_uuid) + .and(groups::organizations_uuid.eq(users_organizations::org_uuid)) )) .filter(users_organizations::user_uuid.eq(user_uuid)) .filter(groups::access_all.eq(true)) @@ -276,9 +276,9 @@ impl Group { }} } - pub async fn delete(&self, conn: &DbConn) -> EmptyResult { - CollectionGroup::delete_all_by_group(&self.uuid, conn).await?; - GroupUser::delete_all_by_group(&self.uuid, conn).await?; + pub async fn delete(&self, org_uuid: &OrganizationId, conn: &DbConn) -> EmptyResult { + CollectionGroup::delete_all_by_group(&self.uuid, org_uuid, conn).await?; + GroupUser::delete_all_by_group(&self.uuid, org_uuid, conn).await?; db_run! { conn: { diesel::delete(groups::table.filter(groups::uuid.eq(&self.uuid))) @@ -306,8 +306,8 @@ impl Group { } impl CollectionGroup { - pub async fn save(&mut self, conn: &DbConn) -> EmptyResult { - let group_users = GroupUser::find_by_group(&self.groups_uuid, conn).await; + pub async fn save(&mut self, org_uuid: &OrganizationId, conn: &DbConn) -> EmptyResult { + let group_users = GroupUser::find_by_group(&self.groups_uuid, org_uuid, conn).await; for group_user in group_users { group_user.update_user_revision(conn).await; } @@ -365,10 +365,19 @@ impl CollectionGroup { } } - pub async fn find_by_group(group_uuid: &GroupId, conn: &DbConn) -> Vec { + pub async fn find_by_group(group_uuid: &GroupId, org_uuid: &OrganizationId, conn: &DbConn) -> Vec { db_run! { conn: { collections_groups::table + .inner_join(groups::table.on( + groups::uuid.eq(collections_groups::groups_uuid) + )) + .inner_join(collections::table.on( + collections::uuid.eq(collections_groups::collections_uuid) + .and(collections::org_uuid.eq(groups::organizations_uuid)) + )) .filter(collections_groups::groups_uuid.eq(group_uuid)) + .filter(collections::org_uuid.eq(org_uuid)) + .select(collections_groups::all_columns) .load::(conn) .expect("Error loading collection groups") }} @@ -383,6 +392,13 @@ impl CollectionGroup { .inner_join(users_organizations::table.on( users_organizations::uuid.eq(groups_users::users_organizations_uuid) )) + .inner_join(groups::table.on(groups::uuid.eq(collections_groups::groups_uuid) + .and(groups::organizations_uuid.eq(users_organizations::org_uuid)) + )) + .inner_join(collections::table.on( + collections::uuid.eq(collections_groups::collections_uuid) + .and(collections::org_uuid.eq(groups::organizations_uuid)) + )) .filter(users_organizations::user_uuid.eq(user_uuid)) .select(collections_groups::all_columns) .load::(conn) @@ -394,14 +410,20 @@ impl CollectionGroup { db_run! { conn: { collections_groups::table .filter(collections_groups::collections_uuid.eq(collection_uuid)) + .inner_join(collections::table.on( + collections::uuid.eq(collections_groups::collections_uuid) + )) + .inner_join(groups::table.on(groups::uuid.eq(collections_groups::groups_uuid) + .and(groups::organizations_uuid.eq(collections::org_uuid)) + )) .select(collections_groups::all_columns) .load::(conn) .expect("Error loading collection groups") }} } - pub async fn delete(&self, conn: &DbConn) -> EmptyResult { - let group_users = GroupUser::find_by_group(&self.groups_uuid, conn).await; + pub async fn delete(&self, org_uuid: &OrganizationId, conn: &DbConn) -> EmptyResult { + let group_users = GroupUser::find_by_group(&self.groups_uuid, org_uuid, conn).await; for group_user in group_users { group_user.update_user_revision(conn).await; } @@ -415,8 +437,8 @@ impl CollectionGroup { }} } - pub async fn delete_all_by_group(group_uuid: &GroupId, conn: &DbConn) -> EmptyResult { - let group_users = GroupUser::find_by_group(group_uuid, conn).await; + pub async fn delete_all_by_group(group_uuid: &GroupId, org_uuid: &OrganizationId, conn: &DbConn) -> EmptyResult { + let group_users = GroupUser::find_by_group(group_uuid, org_uuid, conn).await; for group_user in group_users { group_user.update_user_revision(conn).await; } @@ -429,10 +451,14 @@ impl CollectionGroup { }} } - pub async fn delete_all_by_collection(collection_uuid: &CollectionId, conn: &DbConn) -> EmptyResult { + pub async fn delete_all_by_collection( + collection_uuid: &CollectionId, + org_uuid: &OrganizationId, + conn: &DbConn, + ) -> EmptyResult { let collection_assigned_to_groups = CollectionGroup::find_by_collection(collection_uuid, conn).await; for collection_assigned_to_group in collection_assigned_to_groups { - let group_users = GroupUser::find_by_group(&collection_assigned_to_group.groups_uuid, conn).await; + let group_users = GroupUser::find_by_group(&collection_assigned_to_group.groups_uuid, org_uuid, conn).await; for group_user in group_users { group_user.update_user_revision(conn).await; } @@ -494,10 +520,19 @@ impl GroupUser { } } - pub async fn find_by_group(group_uuid: &GroupId, conn: &DbConn) -> Vec { + pub async fn find_by_group(group_uuid: &GroupId, org_uuid: &OrganizationId, conn: &DbConn) -> Vec { db_run! { conn: { groups_users::table + .inner_join(groups::table.on( + groups::uuid.eq(groups_users::groups_uuid) + )) + .inner_join(users_organizations::table.on( + users_organizations::uuid.eq(groups_users::users_organizations_uuid) + .and(users_organizations::org_uuid.eq(groups::organizations_uuid)) + )) .filter(groups_users::groups_uuid.eq(group_uuid)) + .filter(groups::organizations_uuid.eq(org_uuid)) + .select(groups_users::all_columns) .load::(conn) .expect("Error loading group users") }} @@ -522,6 +557,13 @@ impl GroupUser { .inner_join(collections_groups::table.on( collections_groups::groups_uuid.eq(groups_users::groups_uuid) )) + .inner_join(groups::table.on( + groups::uuid.eq(groups_users::groups_uuid) + )) + .inner_join(collections::table.on( + collections::uuid.eq(collections_groups::collections_uuid) + .and(collections::org_uuid.eq(groups::organizations_uuid)) + )) .filter(collections_groups::collections_uuid.eq(collection_uuid)) .filter(groups_users::users_organizations_uuid.eq(member_uuid)) .count() @@ -575,8 +617,8 @@ impl GroupUser { }} } - pub async fn delete_all_by_group(group_uuid: &GroupId, conn: &DbConn) -> EmptyResult { - let group_users = GroupUser::find_by_group(group_uuid, conn).await; + pub async fn delete_all_by_group(group_uuid: &GroupId, org_uuid: &OrganizationId, conn: &DbConn) -> EmptyResult { + let group_users = GroupUser::find_by_group(group_uuid, org_uuid, conn).await; for group_user in group_users { group_user.update_user_revision(conn).await; } diff --git a/src/db/models/org_policy.rs b/src/db/models/org_policy.rs index 96811a2b..7e922f35 100644 --- a/src/db/models/org_policy.rs +++ b/src/db/models/org_policy.rs @@ -332,7 +332,7 @@ impl OrgPolicy { for policy in OrgPolicy::find_confirmed_by_user_and_active_policy(user_uuid, OrgPolicyType::SendOptions, conn).await { - if let Some(user) = Membership::find_by_user_and_org(user_uuid, &policy.org_uuid, conn).await { + if let Some(user) = Membership::find_confirmed_by_user_and_org(user_uuid, &policy.org_uuid, conn).await { if user.atype < MembershipType::Admin { match serde_json::from_str::(&policy.data) { Ok(opts) => { diff --git a/src/db/models/organization.rs b/src/db/models/organization.rs index 0b722ef6..9021c739 100644 --- a/src/db/models/organization.rs +++ b/src/db/models/organization.rs @@ -1073,7 +1073,9 @@ impl Membership { .left_join(collections_groups::table.on( collections_groups::groups_uuid.eq(groups_users::groups_uuid) )) - .left_join(groups::table.on(groups::uuid.eq(groups_users::groups_uuid))) + .left_join(groups::table.on(groups::uuid.eq(groups_users::groups_uuid) + .and(groups::organizations_uuid.eq(users_organizations::org_uuid)) + )) .left_join(ciphers_collections::table.on( ciphers_collections::collection_uuid.eq(collections_groups::collections_uuid).and(ciphers_collections::cipher_uuid.eq(&cipher_uuid)) diff --git a/src/db/models/two_factor.rs b/src/db/models/two_factor.rs index f0a1e663..0dc08e3e 100644 --- a/src/db/models/two_factor.rs +++ b/src/db/models/two_factor.rs @@ -20,7 +20,6 @@ pub struct TwoFactor { pub last_used: i64, } -#[allow(dead_code)] #[derive(num_derive::FromPrimitive)] pub enum TwoFactorType { Authenticator = 0,