From cb36ee5415a021da7b8c3bc2019a833bbb172c85 Mon Sep 17 00:00:00 2001 From: Stefan Melmuk Date: Sat, 26 Aug 2023 23:21:26 +0200 Subject: [PATCH] enforce 2fa policy on removal of second factor users should be revoked when their second factors are removed. we want to revoke users so they don't have to be invited again and organization admins and owners are aware that they no longer have access. we make an exception for non-confirmed users to speed up the invitation process as they would have to be restored before they can accept their invitation or be confirmed. if email is enabled, invited users have to add a second factor before they can accept the invitation to an organization with 2fa policy. and if it is not enabled that check is done when confirming the user. --- src/api/admin.rs | 8 ++- src/api/core/organizations.rs | 42 ++++----------- src/api/core/two_factor/mod.rs | 96 ++++++++++++++++++++++++++++------ src/db/models/organization.rs | 10 ++++ 4 files changed, 105 insertions(+), 51 deletions(-) diff --git a/src/api/admin.rs b/src/api/admin.rs index ae304253..cdbad638 100644 --- a/src/api/admin.rs +++ b/src/api/admin.rs @@ -13,7 +13,10 @@ use rocket::{ }; use crate::{ - api::{core::log_event, unregister_push_device, ApiResult, EmptyResult, JsonResult, Notify, NumberOrString}, + api::{ + core::{log_event, two_factor}, + unregister_push_device, ApiResult, EmptyResult, JsonResult, Notify, NumberOrString, + }, auth::{decode_admin, encode_jwt, generate_admin_claims, ClientIp}, config::ConfigBuilder, db::{backup_database, get_sql_server_version, models::*, DbConn, DbConnType}, @@ -445,9 +448,10 @@ async fn enable_user(uuid: &str, _token: AdminToken, mut conn: DbConn) -> EmptyR } #[post("/users//remove-2fa")] -async fn remove_2fa(uuid: &str, _token: AdminToken, mut conn: DbConn) -> EmptyResult { +async fn remove_2fa(uuid: &str, token: AdminToken, mut conn: DbConn) -> EmptyResult { let mut user = get_user_or_404(uuid, &mut conn).await?; TwoFactor::delete_all_by_user(&user.uuid, &mut conn).await?; + two_factor::enforce_2fa_policy(&user, String::from(ACTING_ADMIN_USER), 14, &token.ip.ip, &mut conn).await?; user.totp_recover = None; user.save(&mut conn).await } diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index 59079e01..935cd07f 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -5,7 +5,7 @@ use serde_json::Value; use crate::{ api::{ - core::{log_event, CipherSyncData, CipherSyncType}, + core::{log_event, two_factor, CipherSyncData, CipherSyncType}, EmptyResult, JsonResult, JsonUpcase, JsonUpcaseVec, JsonVec, Notify, NumberOrString, PasswordOrOtpData, UpdateType, }, @@ -1697,38 +1697,16 @@ async fn put_policy( None => err!("Invalid or unsupported policy type"), }; - // When enabling the TwoFactorAuthentication policy, remove this org's members that do have 2FA + // When enabling the TwoFactorAuthentication policy, revoke all members that do not have 2FA if pol_type_enum == OrgPolicyType::TwoFactorAuthentication && data.enabled { - for member in UserOrganization::find_by_org(org_id, &mut conn).await.into_iter() { - let user_twofactor_disabled = TwoFactor::find_by_user(&member.user_uuid, &mut conn).await.is_empty(); - - // Policy only applies to non-Owner/non-Admin members who have accepted joining the org - // Invited users still need to accept the invite and will get an error when they try to accept the invite. - if user_twofactor_disabled - && member.atype < UserOrgType::Admin - && member.status != UserOrgStatus::Invited as i32 - { - if CONFIG.mail_enabled() { - let org = Organization::find_by_uuid(&member.org_uuid, &mut conn).await.unwrap(); - let user = User::find_by_uuid(&member.user_uuid, &mut conn).await.unwrap(); - - mail::send_2fa_removed_from_org(&user.email, &org.name).await?; - } - - log_event( - EventType::OrganizationUserRemoved as i32, - &member.uuid, - org_id, - headers.user.uuid.clone(), - headers.device.atype, - &headers.ip.ip, - &mut conn, - ) - .await; - - member.delete(&mut conn).await?; - } - } + two_factor::enforce_2fa_policy_for_org( + org_id, + headers.user.uuid.clone(), + headers.device.atype, + &headers.ip.ip, + &mut conn, + ) + .await?; } // When enabling the SingleOrg policy, remove this org's members that are members of other orgs diff --git a/src/api/core/two_factor/mod.rs b/src/api/core/two_factor/mod.rs index 41368666..18a4a20f 100644 --- a/src/api/core/two_factor/mod.rs +++ b/src/api/core/two_factor/mod.rs @@ -5,7 +5,10 @@ use rocket::Route; use serde_json::Value; use crate::{ - api::{core::log_user_event, JsonResult, JsonUpcase, NumberOrString, PasswordOrOtpData}, + api::{ + core::{log_event, log_user_event}, + EmptyResult, JsonResult, JsonUpcase, NumberOrString, PasswordOrOtpData, + }, auth::{ClientHeaders, Headers}, crypto, db::{models::*, DbConn, DbPool}, @@ -96,6 +99,7 @@ async fn recover(data: JsonUpcase, client_headers: ClientHeade // Remove all twofactors from the user TwoFactor::delete_all_by_user(&user.uuid, &mut conn).await?; + enforce_2fa_policy(&user, user.uuid.clone(), client_headers.device_type, &client_headers.ip.ip, &mut conn).await?; log_user_event( EventType::UserRecovered2fa as i32, @@ -149,22 +153,8 @@ async fn disable_twofactor(data: JsonUpcase, headers: Head .await; } - let twofactor_disabled = TwoFactor::find_by_user(&user.uuid, &mut conn).await.is_empty(); - - if twofactor_disabled { - for user_org in - UserOrganization::find_by_user_and_policy(&user.uuid, OrgPolicyType::TwoFactorAuthentication, &mut conn) - .await - .into_iter() - { - if user_org.atype < UserOrgType::Admin { - if CONFIG.mail_enabled() { - let org = Organization::find_by_uuid(&user_org.org_uuid, &mut conn).await.unwrap(); - mail::send_2fa_removed_from_org(&user.email, &org.name).await?; - } - user_org.delete(&mut conn).await?; - } - } + if TwoFactor::find_by_user(&user.uuid, &mut conn).await.is_empty() { + enforce_2fa_policy(&user, user.uuid.clone(), headers.device.atype, &headers.ip.ip, &mut conn).await?; } Ok(Json(json!({ @@ -179,6 +169,78 @@ async fn disable_twofactor_put(data: JsonUpcase, headers: disable_twofactor(data, headers, conn).await } +pub async fn enforce_2fa_policy( + user: &User, + act_uuid: String, + device_type: i32, + ip: &std::net::IpAddr, + conn: &mut DbConn, +) -> EmptyResult { + for member in UserOrganization::find_by_user_and_policy(&user.uuid, OrgPolicyType::TwoFactorAuthentication, conn) + .await + .into_iter() + { + // Policy only applies to non-Owner/non-Admin members who have accepted joining the org + if member.atype < UserOrgType::Admin { + if CONFIG.mail_enabled() { + let org = Organization::find_by_uuid(&member.org_uuid, conn).await.unwrap(); + mail::send_2fa_removed_from_org(&user.email, &org.name).await?; + } + let mut member = member; + member.revoke(); + member.save(conn).await?; + + log_event( + EventType::OrganizationUserRevoked as i32, + &member.uuid, + &member.org_uuid, + act_uuid.clone(), + device_type, + ip, + conn, + ) + .await; + } + } + + Ok(()) +} + +pub async fn enforce_2fa_policy_for_org( + org_uuid: &str, + act_uuid: String, + device_type: i32, + ip: &std::net::IpAddr, + conn: &mut DbConn, +) -> EmptyResult { + let org = Organization::find_by_uuid(org_uuid, conn).await.unwrap(); + for member in UserOrganization::find_confirmed_by_org(org_uuid, conn).await.into_iter() { + // Don't enforce the policy for Admins and Owners. + if member.atype < UserOrgType::Admin && TwoFactor::find_by_user(&member.user_uuid, conn).await.is_empty() { + if CONFIG.mail_enabled() { + let user = User::find_by_uuid(&member.user_uuid, conn).await.unwrap(); + mail::send_2fa_removed_from_org(&user.email, &org.name).await?; + } + let mut member = member; + member.revoke(); + member.save(conn).await?; + + log_event( + EventType::OrganizationUserRevoked as i32, + &member.uuid, + org_uuid, + act_uuid.clone(), + device_type, + ip, + conn, + ) + .await; + } + } + + Ok(()) +} + pub async fn send_incomplete_2fa_notifications(pool: DbPool) { debug!("Sending notifications for incomplete 2FA logins"); diff --git a/src/db/models/organization.rs b/src/db/models/organization.rs index 620d7428..144ea567 100644 --- a/src/db/models/organization.rs +++ b/src/db/models/organization.rs @@ -665,6 +665,16 @@ impl UserOrganization { }} } + pub async fn find_confirmed_by_org(org_uuid: &str, conn: &mut DbConn) -> Vec { + db_run! { conn: { + users_organizations::table + .filter(users_organizations::org_uuid.eq(org_uuid)) + .filter(users_organizations::status.eq(UserOrgStatus::Confirmed as i32)) + .load::(conn) + .unwrap_or_default().from_db() + }} + } + pub async fn count_by_org(org_uuid: &str, conn: &mut DbConn) -> i64 { db_run! { conn: { users_organizations::table