From 35e1a306f3399e3caf33791a759f16e47d3339e1 Mon Sep 17 00:00:00 2001 From: Timshel Date: Sun, 23 Nov 2025 21:54:37 +0100 Subject: [PATCH] Fix around singleorg policy (#6247) Co-authored-by: Timshel --- src/api/admin.rs | 21 +----- src/api/core/mod.rs | 23 +----- src/api/core/organizations.rs | 75 ++++--------------- 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, 63 insertions(+), 133 deletions(-) diff --git a/src/api/admin.rs b/src/api/admin.rs index 8b6101fb..d36da8f9 100644 --- a/src/api/admin.rs +++ b/src/api/admin.rs @@ -23,7 +23,7 @@ use crate::{ backup_sqlite, get_sql_server_version, models::{ Attachment, Cipher, Collection, Device, Event, EventType, Group, Invitation, Membership, MembershipId, - MembershipType, OrgPolicy, OrgPolicyErr, Organization, OrganizationId, SsoUser, TwoFactor, User, UserId, + MembershipType, OrgPolicy, Organization, OrganizationId, SsoUser, TwoFactor, User, UserId, }, DbConn, DbConnType, ACTIVE_DB_TYPE, }, @@ -556,23 +556,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, &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, &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", &conn).await?; log_event( EventType::OrganizationUserUpdated as i32, @@ -585,7 +571,6 @@ async fn update_membership_type(data: Json, token: AdminToke ) .await; - member_to_edit.atype = new_type; member_to_edit.save(&conn).await } diff --git a/src/api/core/mod.rs b/src/api/core/mod.rs index d5ca0cc9..173a06b6 100644 --- a/src/api/core/mod.rs +++ b/src/api/core/mod.rs @@ -53,7 +53,7 @@ use crate::{ api::{EmptyResult, JsonResult, Notify, UpdateType}, auth::Headers, db::{ - models::{Membership, MembershipStatus, MembershipType, OrgPolicy, OrgPolicyErr, Organization, User}, + models::{Membership, MembershipStatus, OrgPolicy, Organization, User}, DbConn, }, error::Error, @@ -269,27 +269,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 b8715ab7..e8cca467 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -15,7 +15,7 @@ use crate::{ models::{ Cipher, CipherId, Collection, CollectionCipher, CollectionGroup, CollectionId, CollectionUser, EventType, Group, GroupId, GroupUser, Invitation, Membership, MembershipId, MembershipStatus, MembershipType, - OrgPolicy, OrgPolicyErr, OrgPolicyType, Organization, OrganizationApiKey, OrganizationId, User, UserId, + OrgPolicy, OrgPolicyType, Organization, OrganizationApiKey, OrganizationId, User, UserId, }, DbConn, }, @@ -1463,27 +1463,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, @@ -1631,27 +1616,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, &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, &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", &conn).await?; + // Delete all the odd collections for c in CollectionUser::find_by_organization_and_user_uuid(&org_id, &member_to_edit.user_uuid, &conn).await { c.delete(&conn).await?; @@ -2154,14 +2125,14 @@ 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, &conn).await.into_iter() { + for mut member in Membership::find_by_org(&org_id, &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, &conn).await > 1 + && Membership::count_accepted_and_confirmed_by_user(&member.user_uuid, &member.org_uuid, &conn).await + > 0 { if CONFIG.mail_enabled() { let org = Organization::find_by_uuid(&member.org_uuid, &conn).await.unwrap(); @@ -2181,7 +2152,8 @@ async fn put_policy( ) .await; - member.delete(&conn).await?; + member.revoke(); + member.save(&conn).await?; } } } @@ -2628,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 92665574..9b4c8b34 100644 --- a/src/db/models/org_policy.rs +++ b/src/db/models/org_policy.rs @@ -2,10 +2,12 @@ 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::schema::{org_policies, users_organizations}; use crate::db::DbConn; use crate::error::MapResult; +use crate::CONFIG; use diesel::prelude::*; use super::{Membership, MembershipId, MembershipStatus, MembershipType, OrganizationId, TwoFactor, UserId}; @@ -58,14 +60,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 { @@ -280,31 +274,35 @@ impl OrgPolicy { false } - pub async fn is_user_allowed( - user_uuid: &UserId, - org_uuid: &OrganizationId, - exclude_current_org: bool, - conn: &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: &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)); + } } - _ => {} - }; - } + } - // 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); + // Check if the user is part of another Organization 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 + )); + } + + 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 640e47e7..0b722ef6 100644 --- a/src/db/models/organization.rs +++ b/src/db/models/organization.rs @@ -883,10 +883,15 @@ impl Membership { }} } - pub async fn count_accepted_and_confirmed_by_user(user_uuid: &UserId, conn: &DbConn) -> i64 { + pub async fn count_accepted_and_confirmed_by_user( + user_uuid: &UserId, + excluded_org: &OrganizationId, + conn: &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..5fe93902 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 access 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..39527f4e 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 access 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.