diff --git a/crates/defguard_common/src/db/models/user.rs b/crates/defguard_common/src/db/models/user.rs index ca9683579c..9238901fd9 100644 --- a/crates/defguard_common/src/db/models/user.rs +++ b/crates/defguard_common/src/db/models/user.rs @@ -497,6 +497,7 @@ impl User { .await?; WebAuthn::delete_all_for_user(pool, self.id).await?; + self.mfa_enabled = false; self.totp_secret = None; self.email_mfa_secret = None; self.totp_enabled = false; diff --git a/crates/defguard_core/tests/integration/api/user.rs b/crates/defguard_core/tests/integration/api/user.rs index 0716a93f48..db08ab75c2 100644 --- a/crates/defguard_core/tests/integration/api/user.rs +++ b/crates/defguard_core/tests/integration/api/user.rs @@ -1,7 +1,7 @@ use defguard_common::{ db::{ Id, - models::{device::AddDevice, oauth2client::OAuth2Client}, + models::{MFAMethod, WebAuthn, device::AddDevice, oauth2client::OAuth2Client}, }, types::user_info::UserInfo, }; @@ -22,6 +22,33 @@ use super::{ }; use crate::api::common::{get_db_device, get_db_location, get_db_user, make_client_with_db}; +async fn seed_user_with_mfa_artifacts(pool: &sqlx::PgPool, username: &str) -> Vec { + let test_user = get_db_user(pool, username).await; + let recovery_codes = vec!["recovery-code-1".to_string(), "recovery-code-2".to_string()]; + + sqlx::query( + "UPDATE \"user\" SET mfa_enabled = TRUE, totp_enabled = TRUE, email_mfa_enabled = TRUE, \ + totp_secret = $2, email_mfa_secret = $3, mfa_method = 'one_time_password', recovery_codes = $4 WHERE id = $1", + ) + .bind(test_user.id) + .bind(vec![1_u8, 2, 3]) + .bind(vec![4_u8, 5, 6]) + .bind(recovery_codes.clone()) + .execute(pool) + .await + .unwrap(); + + sqlx::query("INSERT INTO webauthn (user_id, name, passkey) VALUES ($1, $2, $3)") + .bind(test_user.id) + .bind("Test passkey") + .bind(vec![7_u8, 8, 9]) + .execute(pool) + .await + .unwrap(); + + recovery_codes +} + #[sqlx::test] async fn test_authenticate(_: PgPoolOptions, options: PgConnectOptions) { let pool = setup_pool(options).await; @@ -743,6 +770,97 @@ async fn test_disable(_: PgPoolOptions, options: PgConnectOptions) { ]); } +#[sqlx::test] +async fn test_admin_can_disable_another_users_mfa_emits_updated_event_and_cleans_db( + _: PgPoolOptions, + options: PgConnectOptions, +) { + let pool = setup_pool(options).await; + + let (mut client, pool) = make_client_with_db(pool).await; + + client.login_user("admin", "pass123").await; + + let admin_user = get_db_user(&pool, "admin").await; + let recovery_codes = seed_user_with_mfa_artifacts(&pool, "hpotter").await; + + let seeded_user = get_db_user(&pool, "hpotter").await; + assert!(seeded_user.mfa_enabled); + assert!(seeded_user.totp_enabled); + assert!(seeded_user.email_mfa_enabled); + assert!(seeded_user.totp_secret.is_some()); + assert!(seeded_user.email_mfa_secret.is_some()); + assert_eq!(seeded_user.mfa_method, MFAMethod::OneTimePassword); + assert_eq!(seeded_user.recovery_codes, recovery_codes); + assert_eq!( + WebAuthn::all_for_user(&pool, seeded_user.id) + .await + .unwrap() + .len(), + 1 + ); + + let response = client.delete("/api/v1/user/hpotter/mfa").send().await; + assert_eq!(response.status(), StatusCode::OK); + + let updated_user = get_db_user(&pool, "hpotter").await; + assert!(!updated_user.mfa_enabled); + assert!(!updated_user.totp_enabled); + assert!(!updated_user.email_mfa_enabled); + assert!(updated_user.totp_secret.is_none()); + assert!(updated_user.email_mfa_secret.is_none()); + assert_eq!(updated_user.mfa_method, MFAMethod::None); + assert!(updated_user.recovery_codes.is_empty()); + assert!( + WebAuthn::all_for_user(&pool, updated_user.id) + .await + .unwrap() + .is_empty() + ); + + client.verify_api_events_with_user(&[( + ApiEventType::UserMfaDisabled { + user: updated_user.clone(), + }, + admin_user.id, + "admin", + )]); +} + +#[sqlx::test] +async fn test_non_admin_cannot_disable_another_users_mfa_and_emits_no_event( + _: PgPoolOptions, + options: PgConnectOptions, +) { + let pool = setup_pool(options).await; + + let (mut client, pool) = make_client_with_db(pool).await; + + let recovery_codes = seed_user_with_mfa_artifacts(&pool, "admin").await; + client.login_user("hpotter", "pass123").await; + + let response = client.delete("/api/v1/user/admin/mfa").send().await; + assert_eq!(response.status(), StatusCode::FORBIDDEN); + + let admin_user = get_db_user(&pool, "admin").await; + assert!(admin_user.mfa_enabled); + assert!(admin_user.totp_enabled); + assert!(admin_user.email_mfa_enabled); + assert!(admin_user.totp_secret.is_some()); + assert!(admin_user.email_mfa_secret.is_some()); + assert_eq!(admin_user.mfa_method, MFAMethod::OneTimePassword); + assert_eq!(admin_user.recovery_codes, recovery_codes); + assert_eq!( + WebAuthn::all_for_user(&pool, admin_user.id) + .await + .unwrap() + .len(), + 1 + ); + + client.assert_event_queue_is_empty(); +} + #[sqlx::test] async fn test_unique_email(_: PgPoolOptions, options: PgConnectOptions) { let pool = setup_pool(options).await; diff --git a/web/messages/en/users.json b/web/messages/en/users.json index 6a7c6a49b1..3a8c261fb3 100644 --- a/web/messages/en/users.json +++ b/web/messages/en/users.json @@ -9,6 +9,7 @@ "users_col_name": "User name", "users_col_login": "Login", "users_col_phone": "Phone", + "users_col_mfa": "MFA", "users_col_groups": "Groups", "users_col_status": "Status", "users_col_assigned": "Assigned devices", @@ -20,6 +21,7 @@ "users_row_menu_disable": "Disable account", "users_row_menu_enable": "Enable account", "users_row_menu_add_auth": "Add authentication key", + "users_row_menu_disable_mfa": "Disable MFA", "users_row_menu_delete": "Delete account", "users_row_menu_edit": "Edit details", "users_row_menu_change_password": "Change password", @@ -27,5 +29,9 @@ "user_row_menu_add_new_device": "Add new device", "users_row_menu_initiate_self_enrollment": "Initiate self-enrollment", "users_row_menu_ip_settings": "User devices IP settings", + "users_modal_disable_mfa_title": "Disable MFA", + "users_modal_disable_mfa_content": "Are you sure you want to disable MFA for **{name}**? This will remove all configured MFA methods and recovery codes.", + "users_disable_mfa_success": "MFA disabled", + "users_disable_mfa_error": "Failed to disable MFA", "modal_edit_user_groups_title": "Edit user groups" } diff --git a/web/src/pages/UsersOverviewPage/UsersTable.tsx b/web/src/pages/UsersOverviewPage/UsersTable.tsx index 5e737d7306..5066f7bcc2 100644 --- a/web/src/pages/UsersOverviewPage/UsersTable.tsx +++ b/web/src/pages/UsersOverviewPage/UsersTable.tsx @@ -35,6 +35,7 @@ import { TableEditCell } from '../../shared/defguard-ui/components/table/TableEd import { TableRowContainer } from '../../shared/defguard-ui/components/table/TableRowContainer/TableRowContainer'; import { TableTop } from '../../shared/defguard-ui/components/table/TableTop/TableTop'; import { Snackbar } from '../../shared/defguard-ui/providers/snackbar/snackbar'; +import { ThemeVariable } from '../../shared/defguard-ui/types'; import { isPresent } from '../../shared/defguard-ui/utils/isPresent'; import { openModal } from '../../shared/hooks/modalControls/modalsSubjects'; import { ModalName } from '../../shared/hooks/modalControls/modalTypes'; @@ -196,20 +197,29 @@ export const UsersTable = () => { ), }), - columnHelper.accessor('phone', { - size: 175, - minSize: 175, - header: m.users_col_phone(), - enableSorting: false, - cell: (info) => { - const phone = info.getValue(); - const display = isPresent(phone) && phone.length ? phone : '~'; - return ( - - {display} - - ); - }, + columnHelper.accessor('email', { + header: m.form_label_email(), + size: 200, + minSize: 150, + enableSorting: true, + sortingFn: 'text', + cell: (info) => ( + + {info.getValue()} + + ), + }), + columnHelper.accessor('mfa_enabled', { + header: m.users_col_mfa(), + size: 60, + minSize: 60, + cell: (info) => ( + + {info.getValue() ? ( + + ) : null} + + ), }), columnHelper.accessor('groups', { header: m.users_col_groups(), @@ -251,6 +261,23 @@ export const UsersTable = () => { enableResizing: false, cell: (info) => { const rowData = info.row.original; + const accountStatusMenuGroup: MenuItemsGroup = { + items: [ + { + text: rowData.is_active + ? m.users_row_menu_disable() + : m.users_row_menu_enable(), + icon: rowData.is_active ? 'disabled' : 'check-circle', + testId: 'change-account-status', + onClick: () => { + changeAccountActiveState({ + active: !rowData.is_active, + username: rowData.username, + }); + }, + }, + ], + }; const menuItems: MenuItemsGroup[] = [ { @@ -333,23 +360,7 @@ export const UsersTable = () => { }, ], }, - { - items: [ - { - text: rowData.is_active - ? m.users_row_menu_disable() - : m.users_row_menu_enable(), - icon: rowData.is_active ? 'disabled' : 'check-circle', - testId: 'change-account-status', - onClick: () => { - changeAccountActiveState({ - active: !rowData.is_active, - username: rowData.username, - }); - }, - }, - ], - }, + accountStatusMenuGroup, { items: [ { @@ -405,6 +416,28 @@ export const UsersTable = () => { ], }); } + if (rowData.mfa_enabled) { + accountStatusMenuGroup.items.splice(1, 0, { + text: m.users_row_menu_disable_mfa(), + icon: 'disable-mfa', + onClick: () => { + openModal(ModalName.ConfirmAction, { + title: m.users_modal_disable_mfa_title(), + contentMd: m.users_modal_disable_mfa_content({ + name: rowData.name, + }), + actionPromise: () => api.user.disableMfa(rowData.username), + invalidateKeys: [['user-overview'], ['user'], ['session-info'], ['me']], + submitProps: { + text: m.users_row_menu_disable_mfa(), + variant: 'critical', + }, + onSuccess: () => Snackbar.default(m.users_disable_mfa_success()), + onError: () => Snackbar.error(m.users_disable_mfa_error()), + }); + }, + }); + } return ; },