From 12c6044f209d5b5e2f4dc472077d917b9bfb7261 Mon Sep 17 00:00:00 2001 From: Timshel Date: Thu, 28 Aug 2025 17:08:45 +0200 Subject: [PATCH] Fix around singleorg policy --- src/api/admin.rs | 19 +---- src/api/core/mod.rs | 21 +----- src/api/core/organizations.rs | 74 ++++--------------- src/db/models/mod.rs | 2 +- src/db/models/org_policy.rs | 60 ++++++++------- src/db/models/organization.rs | 7 +- .../send_single_org_removed_from_org.hbs | 4 +- .../send_single_org_removed_from_org.html.hbs | 4 +- 8 files changed, 61 insertions(+), 130 deletions(-) diff --git a/src/api/admin.rs b/src/api/admin.rs index d52e24ef..b6cbd7f1 100644 --- a/src/api/admin.rs +++ b/src/api/admin.rs @@ -544,23 +544,9 @@ async fn update_membership_type(data: Json, token: AdminToke } } + member_to_edit.atype = new_type; // This check is also done at api::organizations::{accept_invite, _confirm_invite, _activate_member, edit_member}, update_membership_type - // It returns different error messages per function. - if new_type < MembershipType::Admin { - match OrgPolicy::is_user_allowed(&member_to_edit.user_uuid, &member_to_edit.org_uuid, true, &mut conn).await { - Ok(_) => {} - Err(OrgPolicyErr::TwoFactorMissing) => { - if CONFIG.email_2fa_auto_fallback() { - two_factor::email::find_and_activate_email_2fa(&member_to_edit.user_uuid, &mut conn).await?; - } else { - err!("You cannot modify this user to this type because they have not setup 2FA"); - } - } - Err(OrgPolicyErr::SingleOrgEnforced) => { - err!("You cannot modify this user to this type because it is a member of an organization which forbids it"); - } - } - } + OrgPolicy::check_user_allowed(&member_to_edit, "modify", &mut conn).await?; log_event( EventType::OrganizationUserUpdated as i32, @@ -573,7 +559,6 @@ async fn update_membership_type(data: Json, token: AdminToke ) .await; - member_to_edit.atype = new_type; member_to_edit.save(&mut conn).await } diff --git a/src/api/core/mod.rs b/src/api/core/mod.rs index 28737bdf..edb6fb10 100644 --- a/src/api/core/mod.rs +++ b/src/api/core/mod.rs @@ -271,27 +271,12 @@ async fn accept_org_invite( err!("User already accepted the invitation"); } - // This check is also done at accept_invite, _confirm_invite, _activate_member, edit_member, admin::update_membership_type - // It returns different error messages per function. - if member.atype < MembershipType::Admin { - match OrgPolicy::is_user_allowed(&member.user_uuid, &member.org_uuid, false, conn).await { - Ok(_) => {} - Err(OrgPolicyErr::TwoFactorMissing) => { - if crate::CONFIG.email_2fa_auto_fallback() { - two_factor::email::activate_email_2fa(user, conn).await?; - } else { - err!("You cannot join this organization until you enable two-step login on your user account"); - } - } - Err(OrgPolicyErr::SingleOrgEnforced) => { - err!("You cannot join this organization because you are a member of an organization which forbids it"); - } - } - } - member.status = MembershipStatus::Accepted as i32; member.reset_password_key = reset_password_key; + // This check is also done at accept_invite, _confirm_invite, _activate_member, edit_member, admin::update_membership_type + OrgPolicy::check_user_allowed(&member, "join", conn).await?; + member.save(conn).await?; if crate::CONFIG.mail_enabled() { diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index 22712003..8e15ed49 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -1466,27 +1466,12 @@ async fn _confirm_invite( err!("User in invalid state") } - // This check is also done at accept_invite, _confirm_invite, _activate_member, edit_member, admin::update_membership_type - // It returns different error messages per function. - if member_to_confirm.atype < MembershipType::Admin { - match OrgPolicy::is_user_allowed(&member_to_confirm.user_uuid, org_id, true, conn).await { - Ok(_) => {} - Err(OrgPolicyErr::TwoFactorMissing) => { - if CONFIG.email_2fa_auto_fallback() { - two_factor::email::find_and_activate_email_2fa(&member_to_confirm.user_uuid, conn).await?; - } else { - err!("You cannot confirm this user because they have not setup 2FA"); - } - } - Err(OrgPolicyErr::SingleOrgEnforced) => { - err!("You cannot confirm this user because they are a member of an organization which forbids it"); - } - } - } - member_to_confirm.status = MembershipStatus::Confirmed as i32; member_to_confirm.akey = key.to_string(); + // This check is also done at accept_invite, _confirm_invite, _activate_member, edit_member, admin::update_membership_type + OrgPolicy::check_user_allowed(&member_to_confirm, "confirm", conn).await?; + log_event( EventType::OrganizationUserConfirmed as i32, &member_to_confirm.uuid, @@ -1640,27 +1625,13 @@ async fn edit_member( } } - // This check is also done at accept_invite, _confirm_invite, _activate_member, edit_member, admin::update_membership_type - // It returns different error messages per function. - if new_type < MembershipType::Admin { - match OrgPolicy::is_user_allowed(&member_to_edit.user_uuid, &org_id, true, &mut conn).await { - Ok(_) => {} - Err(OrgPolicyErr::TwoFactorMissing) => { - if CONFIG.email_2fa_auto_fallback() { - two_factor::email::find_and_activate_email_2fa(&member_to_edit.user_uuid, &mut conn).await?; - } else { - err!("You cannot modify this user to this type because they have not setup 2FA"); - } - } - Err(OrgPolicyErr::SingleOrgEnforced) => { - err!("You cannot modify this user to this type because they are a member of an organization which forbids it"); - } - } - } - member_to_edit.access_all = access_all; member_to_edit.atype = new_type as i32; + // This check is also done at accept_invite, _confirm_invite, _activate_member, edit_member, admin::update_membership_type + // We need to perform the check after changing the type since `admin` is exempt. + OrgPolicy::check_user_allowed(&member_to_edit, "modify", &mut conn).await?; + // Delete all the odd collections for c in CollectionUser::find_by_organization_and_user_uuid(&org_id, &member_to_edit.user_uuid, &mut conn).await { c.delete(&mut conn).await?; @@ -2163,14 +2134,15 @@ async fn put_policy( // When enabling the SingleOrg policy, remove this org's members that are members of other orgs if pol_type_enum == OrgPolicyType::SingleOrg && data.enabled { - for member in Membership::find_by_org(&org_id, &mut conn).await.into_iter() { + for mut member in Membership::find_by_org(&org_id, &mut conn).await.into_iter() { // Policy only applies to non-Owner/non-Admin members who have accepted joining the org // Exclude invited and revoked users when checking for this policy. // Those users will not be allowed to accept or be activated because of the policy checks done there. - // We check if the count is larger then 1, because it includes this organization also. if member.atype < MembershipType::Admin && member.status != MembershipStatus::Invited as i32 - && Membership::count_accepted_and_confirmed_by_user(&member.user_uuid, &mut conn).await > 1 + && Membership::count_accepted_and_confirmed_by_user(&member.user_uuid, &member.org_uuid, &mut conn) + .await + > 0 { if CONFIG.mail_enabled() { let org = Organization::find_by_uuid(&member.org_uuid, &mut conn).await.unwrap(); @@ -2190,7 +2162,8 @@ async fn put_policy( ) .await; - member.delete(&mut conn).await?; + member.revoke(); + member.save(&mut conn).await?; } } } @@ -2627,25 +2600,10 @@ async fn _restore_member( err!("Only owners can restore other owners") } - // This check is also done at accept_invite, _confirm_invite, _activate_member, edit_member, admin::update_membership_type - // It returns different error messages per function. - if member.atype < MembershipType::Admin { - match OrgPolicy::is_user_allowed(&member.user_uuid, org_id, false, conn).await { - Ok(_) => {} - Err(OrgPolicyErr::TwoFactorMissing) => { - if CONFIG.email_2fa_auto_fallback() { - two_factor::email::find_and_activate_email_2fa(&member.user_uuid, conn).await?; - } else { - err!("You cannot restore this user because they have not setup 2FA"); - } - } - Err(OrgPolicyErr::SingleOrgEnforced) => { - err!("You cannot restore this user because they are a member of an organization which forbids it"); - } - } - } - member.restore(); + // This check is also done at accept_invite, _confirm_invite, _activate_member, edit_member, admin::update_membership_type + // This check need to be done after restoring to work with the correct status + OrgPolicy::check_user_allowed(&member, "restore", conn).await?; member.save(conn).await?; log_event( diff --git a/src/db/models/mod.rs b/src/db/models/mod.rs index a9406ed0..75c58626 100644 --- a/src/db/models/mod.rs +++ b/src/db/models/mod.rs @@ -27,7 +27,7 @@ pub use self::event::{Event, EventType}; pub use self::favorite::Favorite; pub use self::folder::{Folder, FolderCipher, FolderId}; pub use self::group::{CollectionGroup, Group, GroupId, GroupUser}; -pub use self::org_policy::{OrgPolicy, OrgPolicyErr, OrgPolicyId, OrgPolicyType}; +pub use self::org_policy::{OrgPolicy, OrgPolicyId, OrgPolicyType}; pub use self::organization::{ Membership, MembershipId, MembershipStatus, MembershipType, OrgApiKeyId, Organization, OrganizationApiKey, OrganizationId, diff --git a/src/db/models/org_policy.rs b/src/db/models/org_policy.rs index aac145cb..31e9af1c 100644 --- a/src/db/models/org_policy.rs +++ b/src/db/models/org_policy.rs @@ -2,9 +2,11 @@ use derive_more::{AsRef, From}; use serde::Deserialize; use serde_json::Value; +use crate::api::core::two_factor; use crate::api::EmptyResult; use crate::db::DbConn; use crate::error::MapResult; +use crate::CONFIG; use super::{Membership, MembershipId, MembershipStatus, MembershipType, OrganizationId, TwoFactor, UserId}; @@ -57,14 +59,6 @@ pub struct ResetPasswordDataModel { pub auto_enroll_enabled: bool, } -pub type OrgPolicyResult = Result<(), OrgPolicyErr>; - -#[derive(Debug)] -pub enum OrgPolicyErr { - TwoFactorMissing, - SingleOrgEnforced, -} - /// Local methods impl OrgPolicy { pub fn new(org_uuid: OrganizationId, atype: OrgPolicyType, enabled: bool, data: String) -> Self { @@ -285,31 +279,35 @@ impl OrgPolicy { false } - pub async fn is_user_allowed( - user_uuid: &UserId, - org_uuid: &OrganizationId, - exclude_current_org: bool, - conn: &mut DbConn, - ) -> OrgPolicyResult { - // Enforce TwoFactor/TwoStep login - if TwoFactor::find_by_user(user_uuid, conn).await.is_empty() { - match Self::find_by_org_and_type(org_uuid, OrgPolicyType::TwoFactorAuthentication, conn).await { - Some(p) if p.enabled => { - return Err(OrgPolicyErr::TwoFactorMissing); + pub async fn check_user_allowed(m: &Membership, action: &str, conn: &mut DbConn) -> EmptyResult { + if m.atype < MembershipType::Admin && m.status > (MembershipStatus::Invited as i32) { + // Enforce TwoFactor/TwoStep login + if let Some(p) = Self::find_by_org_and_type(&m.org_uuid, OrgPolicyType::TwoFactorAuthentication, conn).await + { + if p.enabled && TwoFactor::find_by_user(&m.user_uuid, conn).await.is_empty() { + if CONFIG.email_2fa_auto_fallback() { + two_factor::email::find_and_activate_email_2fa(&m.user_uuid, conn).await?; + } else { + err!(format!("Cannot {} because 2FA is required (membership {})", action, m.uuid)); + } } - _ => {} - }; - } + } + + // Check if the user is part of another Orgnization with SingleOrg activated + if Self::is_applicable_to_user(&m.user_uuid, OrgPolicyType::SingleOrg, Some(&m.org_uuid), conn).await { + err!(format!( + "Cannot {} because another organization policy forbids it (membership {})", + action, m.uuid + )); + } - // Enforce Single Organization Policy of other organizations user is a member of - // This check here needs to exclude this current org-id, else an accepted user can not be confirmed. - let exclude_org = if exclude_current_org { - Some(org_uuid) - } else { - None - }; - if Self::is_applicable_to_user(user_uuid, OrgPolicyType::SingleOrg, exclude_org, conn).await { - return Err(OrgPolicyErr::SingleOrgEnforced); + if let Some(p) = Self::find_by_org_and_type(&m.org_uuid, OrgPolicyType::SingleOrg, conn).await { + if p.enabled + && Membership::count_accepted_and_confirmed_by_user(&m.user_uuid, &m.org_uuid, conn).await > 0 + { + err!(format!("Cannot {} because the organization policy forbids being part of other organization (membership {})", action, m.uuid)); + } + } } Ok(()) diff --git a/src/db/models/organization.rs b/src/db/models/organization.rs index 77cf91c0..2c859a1a 100644 --- a/src/db/models/organization.rs +++ b/src/db/models/organization.rs @@ -894,10 +894,15 @@ impl Membership { }} } - pub async fn count_accepted_and_confirmed_by_user(user_uuid: &UserId, conn: &mut DbConn) -> i64 { + pub async fn count_accepted_and_confirmed_by_user( + user_uuid: &UserId, + excluded_org: &OrganizationId, + conn: &mut DbConn, + ) -> i64 { db_run! { conn: { users_organizations::table .filter(users_organizations::user_uuid.eq(user_uuid)) + .filter(users_organizations::org_uuid.ne(excluded_org)) .filter(users_organizations::status.eq(MembershipStatus::Accepted as i32).or(users_organizations::status.eq(MembershipStatus::Confirmed as i32))) .count() .first::(conn) diff --git a/src/static/templates/email/send_single_org_removed_from_org.hbs b/src/static/templates/email/send_single_org_removed_from_org.hbs index ec77cf63..16efcdd7 100644 --- a/src/static/templates/email/send_single_org_removed_from_org.hbs +++ b/src/static/templates/email/send_single_org_removed_from_org.hbs @@ -1,4 +1,4 @@ -You have been removed from {{{org_name}}} +Your access to {{{org_name}}} has been revoked -Your user account has been removed from the *{{org_name}}* organization because you are a part of another organization. The {{org_name}} organization has enabled a policy that prevents users from being a part of multiple organizations. Before you can re-join this organization you need to leave all other organizations or join with a different account. +Your acess to the *{{org_name}}* organization has been revoked because you are a part of another organization. The {{org_name}} organization has enabled a policy that prevents users from being a part of multiple organizations. Before your access can be restored you need to leave all other organizations or join with a different account. {{> email/email_footer_text }} diff --git a/src/static/templates/email/send_single_org_removed_from_org.html.hbs b/src/static/templates/email/send_single_org_removed_from_org.html.hbs index e4026628..147476d2 100644 --- a/src/static/templates/email/send_single_org_removed_from_org.html.hbs +++ b/src/static/templates/email/send_single_org_removed_from_org.html.hbs @@ -1,10 +1,10 @@ -You have been removed from {{{org_name}}} +Your access to {{{org_name}}} has been revoked {{> email/email_header }}
- Your user account has been removed from the {{org_name}} organization because you are a part of another organization. The {{org_name}} organization has enabled a policy that prevents users from being a part of multiple organizations. Before you can re-join this organization you need to leave all other organizations or join with a different account. + Your acess to the {{org_name}} organization has been revoked because you are a part of another organization. The {{org_name}} organization has enabled a policy that prevents users from being a part of multiple organizations. Before your access can be restored you need to leave all other organizations or join with a different account.