Skip to content

Commit

Permalink
enforce 2FA policy on removal of second factor and login (#3803)
Browse files Browse the repository at this point in the history
* 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.

* use &str instead of String in log_event()

* enforce the 2fa policy on login

if a user doesn't have a second factor check if they are in an
organization that has the 2fa policy enabled to revoke their access
  • Loading branch information
stefan0xC authored Jan 1, 2024
1 parent d672ad3 commit 2c36993
Show file tree
Hide file tree
Showing 7 changed files with 163 additions and 122 deletions.
12 changes: 8 additions & 4 deletions src/api/admin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -390,7 +393,7 @@ async fn delete_user(uuid: &str, token: AdminToken, mut conn: DbConn) -> EmptyRe
EventType::OrganizationUserRemoved as i32,
&user_org.uuid,
&user_org.org_uuid,
String::from(ACTING_ADMIN_USER),
ACTING_ADMIN_USER,
14, // Use UnknownBrowser type
&token.ip.ip,
&mut conn,
Expand Down Expand Up @@ -445,9 +448,10 @@ async fn enable_user(uuid: &str, _token: AdminToken, mut conn: DbConn) -> EmptyR
}

#[post("/users/<uuid>/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, ACTING_ADMIN_USER, 14, &token.ip.ip, &mut conn).await?;
user.totp_recover = None;
user.save(&mut conn).await
}
Expand Down Expand Up @@ -517,7 +521,7 @@ async fn update_user_org_type(data: Json<UserOrgTypeData>, token: AdminToken, mu
EventType::OrganizationUserUpdated as i32,
&user_to_edit.uuid,
&data.org_uuid,
String::from(ACTING_ADMIN_USER),
ACTING_ADMIN_USER,
14, // Use UnknownBrowser type
&token.ip.ip,
&mut conn,
Expand Down
24 changes: 8 additions & 16 deletions src/api/core/ciphers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ pub async fn update_cipher_from_data(
event_type as i32,
&cipher.uuid,
org_uuid,
headers.user.uuid.clone(),
&headers.user.uuid,
headers.device.atype,
&headers.ip.ip,
conn,
Expand Down Expand Up @@ -791,7 +791,7 @@ async fn post_collections_admin(
EventType::CipherUpdatedCollections as i32,
&cipher.uuid,
&cipher.organization_uuid.unwrap(),
headers.user.uuid.clone(),
&headers.user.uuid,
headers.device.atype,
&headers.ip.ip,
&mut conn,
Expand Down Expand Up @@ -1145,7 +1145,7 @@ async fn save_attachment(
EventType::CipherAttachmentCreated as i32,
&cipher.uuid,
org_uuid,
headers.user.uuid.clone(),
&headers.user.uuid,
headers.device.atype,
&headers.ip.ip,
&mut conn,
Expand Down Expand Up @@ -1479,7 +1479,7 @@ async fn delete_all(
EventType::OrganizationPurgedVault as i32,
&org_data.org_id,
&org_data.org_id,
user.uuid,
&user.uuid,
headers.device.atype,
&headers.ip.ip,
&mut conn,
Expand Down Expand Up @@ -1560,16 +1560,8 @@ async fn _delete_cipher_by_uuid(
false => EventType::CipherDeleted as i32,
};

log_event(
event_type,
&cipher.uuid,
&org_uuid,
headers.user.uuid.clone(),
headers.device.atype,
&headers.ip.ip,
conn,
)
.await;
log_event(event_type, &cipher.uuid, &org_uuid, &headers.user.uuid, headers.device.atype, &headers.ip.ip, conn)
.await;
}

Ok(())
Expand Down Expand Up @@ -1629,7 +1621,7 @@ async fn _restore_cipher_by_uuid(uuid: &str, headers: &Headers, conn: &mut DbCon
EventType::CipherRestored as i32,
&cipher.uuid.clone(),
org_uuid,
headers.user.uuid.clone(),
&headers.user.uuid,
headers.device.atype,
&headers.ip.ip,
conn,
Expand Down Expand Up @@ -1713,7 +1705,7 @@ async fn _delete_cipher_attachment_by_id(
EventType::CipherAttachmentDeleted as i32,
&cipher.uuid,
&org_uuid,
headers.user.uuid.clone(),
&headers.user.uuid,
headers.device.atype,
&headers.ip.ip,
conn,
Expand Down
4 changes: 2 additions & 2 deletions src/api/core/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,15 +263,15 @@ pub async fn log_event(
event_type: i32,
source_uuid: &str,
org_uuid: &str,
act_user_uuid: String,
act_user_uuid: &str,
device_type: i32,
ip: &IpAddr,
conn: &mut DbConn,
) {
if !CONFIG.org_events_enabled() {
return;
}
_log_event(event_type, source_uuid, org_uuid, &act_user_uuid, device_type, None, ip, conn).await;
_log_event(event_type, source_uuid, org_uuid, act_user_uuid, device_type, None, ip, conn).await;
}

#[allow(clippy::too_many_arguments)]
Expand Down
Loading

0 comments on commit 2c36993

Please sign in to comment.