Fix around singleorg policy (#6247)

Co-authored-by: Timshel <timshel@users.noreply.github.com>

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<MembershipTypeData>, 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<MembershipTypeData>, token: AdminToke
     )
     .await;
 
-    member_to_edit.atype = new_type;
     member_to_edit.save(&conn).await
 }
 

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() {

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(

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,

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(())

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::<i64>(conn)

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 }}

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 }}
 <table width="100%" cellpadding="0" cellspacing="0" style="margin: 0; font-family: 'Helvetica Neue', Helvetica, Arial, sans-serif; box-sizing: border-box; font-size: 16px; color: #333; line-height: 25px; -webkit-font-smoothing: antialiased; -webkit-text-size-adjust: none;">
    <tr style="margin: 0; font-family: 'Helvetica Neue', Helvetica, Arial, sans-serif; box-sizing: border-box; font-size: 16px; color: #333; line-height: 25px; -webkit-font-smoothing: antialiased; -webkit-text-size-adjust: none;">
       <td class="content-block" style="font-family: 'Helvetica Neue', Helvetica, Arial, sans-serif; box-sizing: border-box; font-size: 16px; color: #333; line-height: 25px; margin: 0; -webkit-font-smoothing: antialiased; padding: 0 0 10px; -webkit-text-size-adjust: none; text-align: center;" valign="top" align="center">
-         Your user account has been removed from the <b style="margin: 0; font-family: 'Helvetica Neue', Helvetica, Arial, sans-serif; box-sizing: border-box; font-size: 16px; color: #333; line-height: 25px; -webkit-font-smoothing: antialiased; -webkit-text-size-adjust: none;">{{org_name}}</b> 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 <b style="margin: 0; font-family: 'Helvetica Neue', Helvetica, Arial, sans-serif; box-sizing: border-box; font-size: 16px; color: #333; line-height: 25px; -webkit-font-smoothing: antialiased; -webkit-text-size-adjust: none;">{{org_name}}</b> 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.
       </td>
    </tr>
 </table>