diff --git a/.sqlx/query-52c42c255fa0bd6bcf39784c54981b7e3ab3c7bd93a209e35545d22ec2cc98ad.json b/.sqlx/query-21d1f7c109dce37b1cb044d8940ba1d96961e3c4ddf46c75537a577e84b818ac.json similarity index 67% rename from .sqlx/query-52c42c255fa0bd6bcf39784c54981b7e3ab3c7bd93a209e35545d22ec2cc98ad.json rename to .sqlx/query-21d1f7c109dce37b1cb044d8940ba1d96961e3c4ddf46c75537a577e84b818ac.json index 248b62d47c..0e2828e895 100644 --- a/.sqlx/query-52c42c255fa0bd6bcf39784c54981b7e3ab3c7bd93a209e35545d22ec2cc98ad.json +++ b/.sqlx/query-21d1f7c109dce37b1cb044d8940ba1d96961e3c4ddf46c75537a577e84b818ac.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "UPDATE \"user\" SET mfa_enabled = $2, email_mfa_enabled = $3 AND email_mfa_secret = $4 WHERE id = $1", + "query": "UPDATE \"user\" SET mfa_enabled = $2, email_mfa_enabled = $3, email_mfa_secret = $4 WHERE id = $1", "describe": { "columns": [], "parameters": { @@ -13,5 +13,5 @@ }, "nullable": [] }, - "hash": "52c42c255fa0bd6bcf39784c54981b7e3ab3c7bd93a209e35545d22ec2cc98ad" + "hash": "21d1f7c109dce37b1cb044d8940ba1d96961e3c4ddf46c75537a577e84b818ac" } diff --git a/.sqlx/query-26d95a39c9d11c1fcd5d35f4f0d8883b366b46937b94f25a3788604c72da0ecc.json b/.sqlx/query-26d95a39c9d11c1fcd5d35f4f0d8883b366b46937b94f25a3788604c72da0ecc.json deleted file mode 100644 index 9076e114d2..0000000000 --- a/.sqlx/query-26d95a39c9d11c1fcd5d35f4f0d8883b366b46937b94f25a3788604c72da0ecc.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "DELETE FROM token WHERE user_id = $1 AND token_type = 'PASSWORD_RESET' AND used_at IS NULL", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Int8" - ] - }, - "nullable": [] - }, - "hash": "26d95a39c9d11c1fcd5d35f4f0d8883b366b46937b94f25a3788604c72da0ecc" -} diff --git a/.sqlx/query-381a91411dae9517c285d2f57cf81ac75d3bf0812c190ef118d6dce1b564b37d.json b/.sqlx/query-a333827515092e1bb0ae33457d8ed1b7260933957cbf86d0c9dca15f5af739de.json similarity index 69% rename from .sqlx/query-381a91411dae9517c285d2f57cf81ac75d3bf0812c190ef118d6dce1b564b37d.json rename to .sqlx/query-a333827515092e1bb0ae33457d8ed1b7260933957cbf86d0c9dca15f5af739de.json index 1331b30023..f83ff5c34f 100644 --- a/.sqlx/query-381a91411dae9517c285d2f57cf81ac75d3bf0812c190ef118d6dce1b564b37d.json +++ b/.sqlx/query-a333827515092e1bb0ae33457d8ed1b7260933957cbf86d0c9dca15f5af739de.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "UPDATE \"user\" SET mfa_enabled = $2, totp_enabled = $3 AND totp_secret = $4 WHERE id = $1", + "query": "UPDATE \"user\" SET mfa_enabled = $2, totp_enabled = $3, totp_secret = $4 WHERE id = $1", "describe": { "columns": [], "parameters": { @@ -13,5 +13,5 @@ }, "nullable": [] }, - "hash": "381a91411dae9517c285d2f57cf81ac75d3bf0812c190ef118d6dce1b564b37d" + "hash": "a333827515092e1bb0ae33457d8ed1b7260933957cbf86d0c9dca15f5af739de" } diff --git a/crates/defguard_common/src/db/models/user.rs b/crates/defguard_common/src/db/models/user.rs index c3b78b434d..ecb7403736 100644 --- a/crates/defguard_common/src/db/models/user.rs +++ b/crates/defguard_common/src/db/models/user.rs @@ -614,7 +614,7 @@ impl User { self.totp_secret = None; query!( - "UPDATE \"user\" SET mfa_enabled = $2, totp_enabled = $3 AND totp_secret = $4 \ + "UPDATE \"user\" SET mfa_enabled = $2, totp_enabled = $3, totp_secret = $4 \ WHERE id = $1", self.id, self.mfa_enabled, @@ -655,7 +655,7 @@ impl User { self.email_mfa_secret = None; query!( - "UPDATE \"user\" SET mfa_enabled = $2, email_mfa_enabled = $3 AND email_mfa_secret = $4 \ + "UPDATE \"user\" SET mfa_enabled = $2, email_mfa_enabled = $3, email_mfa_secret = $4 \ WHERE id = $1", self.id, self.mfa_enabled, @@ -1977,4 +1977,42 @@ mod test { let settings = settings_with_ldap_password_management(false); assert!(!user.password_management_disabled(false, &settings, false)); } + + #[sqlx::test] + async fn test_disable_mfa_method_clears_its_secret( + _: PgPoolOptions, + options: PgConnectOptions, + ) { + let pool = setup_pool(options).await; + let config = DefGuardConfig::new_test_config(); + let _ = SERVER_CONFIG.set(config); + + let mut user = User::new( + "hpotter", + Some("pass123"), + "Potter", + "Harry", + "h.potter@hogwart.edu.uk", + None, + ) + .save(&pool) + .await + .unwrap(); + + user.new_totp_secret(&pool).await.unwrap(); + user.new_email_secret(&pool).await.unwrap(); + user.enable_totp(&pool).await.unwrap(); + user.enable_email_mfa(&pool).await.unwrap(); + + user.disable_totp(&pool).await.unwrap(); + user.disable_email_mfa(&pool).await.unwrap(); + + // Clear the secret as well as the flag so stale authenticator entries cannot re-enable + // the factor. + let stored = User::find_by_id(&pool, user.id).await.unwrap().unwrap(); + assert!(!stored.totp_enabled); + assert!(!stored.email_mfa_enabled); + assert!(stored.totp_secret.is_none()); + assert!(stored.email_mfa_secret.is_none()); + } } diff --git a/crates/defguard_core/src/db/models/enrollment.rs b/crates/defguard_core/src/db/models/enrollment.rs index df386bff86..dd4db64e9e 100644 --- a/crates/defguard_core/src/db/models/enrollment.rs +++ b/crates/defguard_core/src/db/models/enrollment.rs @@ -19,6 +19,9 @@ use crate::mail::templates; pub static ENROLLMENT_TOKEN_TYPE: &str = "ENROLLMENT"; pub static PASSWORD_RESET_TOKEN_TYPE: &str = "PASSWORD_RESET"; +pub static MFA_CONFIG_TOKEN_TYPE: &str = "MFA_CONFIG"; +// One window covers both the time to authorize and the time to configure factors. +pub const MFA_CONFIG_SESSION_TIMEOUT: Duration = Duration::from_secs(60 * 60); #[derive(Error, Debug)] pub enum TokenError { @@ -313,18 +316,28 @@ impl Token { transaction: &mut PgConnection, user_id: Id, ) -> Result<(), TokenError> { - debug!("Deleting unused password reset tokens for user {user_id}"); - let result = query!( - "DELETE FROM token \ - WHERE user_id = $1 \ - AND token_type = 'PASSWORD_RESET' \ - AND used_at IS NULL", - user_id - ) - .execute(transaction) - .await?; + Self::delete_unused_user_tokens_of_type(transaction, user_id, PASSWORD_RESET_TOKEN_TYPE) + .await + } + + pub async fn delete_unused_user_tokens_of_type<'e, E>( + executor: E, + user_id: Id, + token_type: &str, + ) -> Result<(), TokenError> + where + E: PgExecutor<'e>, + { + debug!("Deleting unused {token_type} tokens for user {user_id}"); + // Plain query: the token type is a runtime parameter and needs no cached query data. + let result = + query("DELETE FROM token WHERE user_id = $1 AND token_type = $2 AND used_at IS NULL") + .bind(user_id) + .bind(token_type) + .execute(executor) + .await?; debug!( - "Deleted {} unused password reset tokens for user {user_id}", + "Deleted {} unused {token_type} tokens for user {user_id}", result.rows_affected() ); diff --git a/crates/defguard_proxy_manager/src/handler.rs b/crates/defguard_proxy_manager/src/handler.rs index e60b16ab33..60e22cf5f4 100644 --- a/crates/defguard_proxy_manager/src/handler.rs +++ b/crates/defguard_proxy_manager/src/handler.rs @@ -2,6 +2,8 @@ use std::path::PathBuf; use std::{ collections::HashMap, + future::Future, + pin::Pin, str::FromStr, sync::{Arc, RwLock}, time::Duration, @@ -45,11 +47,12 @@ use defguard_core::{ }; use defguard_grpc_tls::certs::proxy_mtls_channel; use defguard_proto::{ - client_types::AuthFlowType as ProtoAuthFlowType, + client_types::{AuthFlowType as ProtoAuthFlowType, AuthInfoRequest}, enterprise::posture::{DevicePostureCheckResponse, DevicePostureRejection}, proxy::{ - AuthCallbackResponse, AuthInfoResponse, CoreError, CoreRequest, CoreResponse, HttpsCerts, - InitialInfo, core_request, core_response, proxy_client::ProxyClient, + AuthCallbackRequest, AuthCallbackResponse, AuthInfoResponse, CoreError, CoreRequest, + CoreResponse, HttpsCerts, InitialInfo, core_request, core_response, + proxy_client::ProxyClient, }, }; use defguard_version::{ @@ -83,7 +86,7 @@ use tonic::{ use crate::ProxyManagerTestSupport; use crate::{ HandlerTxMap, ProxyError, ProxyTxSet, TEN_SECS, - servers::{EnrollmentServer, PasswordResetServer}, + servers::{EnrollmentServer, MfaConfigServer, PasswordResetServer}, }; const VERSION_ZERO: Version = Version::new(0, 0, 0); @@ -615,43 +618,78 @@ impl ProxyHandler { Ok(Some(received)) => { debug!("Received message from proxy; ID={}", received.id); let payload = match received.payload { - // rpc CodeMfaSetupStart return (CodeMfaSetupStartResponse) - Some(core_request::Payload::CodeMfaSetupStart(request)) => { - match self - .services - .enrollment - .register_code_mfa_start(request) + // rpc MfaConfigStart return (MfaConfigStartResponse) + Some(core_request::Payload::MfaConfigStart(request)) => { + match boxed(self.services.mfa_config.mfa_config_start(request)).await { + Ok(response) => { + Some(core_response::Payload::MfaConfigStart(response)) + } + Err(err) => { + error!("MFA config start error {err}"); + Some(core_response::Payload::CoreError(err.into())) + } + } + } + Some(core_request::Payload::MfaConfigSendCode(request)) => { + match boxed(self.services.mfa_config.mfa_config_send_code(request)) .await { + Ok(response) => { + Some(core_response::Payload::MfaConfigSendCode(response)) + } + Err(err) => { + error!("MFA config send code error {err}"); + Some(core_response::Payload::CoreError(err.into())) + } + } + } + Some(core_request::Payload::MfaConfigAuthorize(request)) => { + match boxed(self.services.mfa_config.mfa_config_authorize(request)) + .await + { + Ok(response) => { + Some(core_response::Payload::MfaConfigAuthorize(response)) + } + Err(err) => { + error!("MFA config authorize error {err}"); + Some(core_response::Payload::CoreError(err.into())) + } + } + } + // rpc CodeMfaSetupStart return (CodeMfaSetupStartResponse) + Some(core_request::Payload::CodeMfaSetupStart(request)) => { + match boxed(self.services.enrollment.mfa_setup_start(request)).await { Ok(response) => Some( core_response::Payload::CodeMfaSetupStartResponse(response), ), Err(err) => { - error!("Register mfa start error {err}"); + error!("MFA setup start error {err}"); Some(core_response::Payload::CoreError(err.into())) } } } // rpc CodeMfaSetupFinish return (CodeMfaSetupFinishResponse) Some(core_request::Payload::CodeMfaSetupFinish(request)) => { - match self - .services - .enrollment - .register_code_mfa_finish(request) - .await + match boxed( + self.services + .enrollment + .mfa_setup_finish(request, received.device_info), + ) + .await { Ok(response) => Some( core_response::Payload::CodeMfaSetupFinishResponse(response), ), Err(err) => { - error!("Register MFA finish error {err}"); + error!("MFA setup finish error {err}"); Some(core_response::Payload::CoreError(err.into())) } } } // rpc ClientMfaTokenValidation return (ClientMfaTokenValidationResponse) Some(core_request::Payload::ClientMfaTokenValidation(request)) => { - match self.services.client_mfa.validate_mfa_token(request).await { + match boxed(self.services.client_mfa.validate_mfa_token(request)).await + { Ok(response_payload) => { Some(core_response::Payload::ClientMfaTokenValidation( response_payload, @@ -665,7 +703,9 @@ impl ProxyHandler { } // rpc RegisterMobileAuth (RegisterMobileAuthRequest) return (google.protobuf.Empty) Some(core_request::Payload::RegisterMobileAuth(request)) => { - match self.services.enrollment.register_mobile_auth(request).await { + match boxed(self.services.enrollment.register_mobile_auth(request)) + .await + { Ok(()) => Some(core_response::Payload::Empty(())), Err(err) => { error!("Register mobile auth error {err}"); @@ -675,11 +715,12 @@ impl ProxyHandler { } // rpc StartEnrollment (EnrollmentStartRequest) returns (EnrollmentStartResponse) Some(core_request::Payload::EnrollmentStart(request)) => { - match self - .services - .enrollment - .start_enrollment(request, received.device_info) - .await + match boxed( + self.services + .enrollment + .start_enrollment(request, received.device_info), + ) + .await { Ok(response_payload) => { Some(core_response::Payload::EnrollmentStart(response_payload)) @@ -692,11 +733,12 @@ impl ProxyHandler { } // rpc ActivateUser (ActivateUserRequest) returns (google.protobuf.Empty) Some(core_request::Payload::ActivateUser(request)) => { - match self - .services - .enrollment - .activate_user(request, received.device_info) - .await + match boxed( + self.services + .enrollment + .activate_user(request, received.device_info), + ) + .await { Ok(()) => Some(core_response::Payload::Empty(())), Err(err) => { @@ -707,11 +749,12 @@ impl ProxyHandler { } // rpc CreateDevice (NewDevice) returns (DeviceConfigResponse) Some(core_request::Payload::NewDevice(request)) => { - match self - .services - .enrollment - .create_device(request, received.device_info) - .await + match boxed( + self.services + .enrollment + .create_device(request, received.device_info), + ) + .await { Ok(response_payload) => { Some(core_response::Payload::DeviceConfig(response_payload)) @@ -724,11 +767,12 @@ impl ProxyHandler { } // rpc GetNetworkInfo (ExistingDevice) returns (DeviceConfigResponse) Some(core_request::Payload::ExistingDevice(request)) => { - match self - .services - .enrollment - .get_network_info(request, received.device_info) - .await + match boxed( + self.services + .enrollment + .get_network_info(request, received.device_info), + ) + .await { Ok(response_payload) => { Some(core_response::Payload::DeviceConfig(response_payload)) @@ -741,11 +785,12 @@ impl ProxyHandler { } // rpc RequestPasswordReset (PasswordResetInitializeRequest) returns (google.protobuf.Empty) Some(core_request::Payload::PasswordResetInit(request)) => { - match self - .services - .password_reset - .request_password_reset(request, received.device_info) - .await + match boxed( + self.services + .password_reset + .request_password_reset(request, received.device_info), + ) + .await { Ok(()) => Some(core_response::Payload::Empty(())), Err(err) => { @@ -756,11 +801,12 @@ impl ProxyHandler { } // rpc StartPasswordReset (PasswordResetStartRequest) returns (PasswordResetStartResponse) Some(core_request::Payload::PasswordResetStart(request)) => { - match self - .services - .password_reset - .start_password_reset(request, received.device_info) - .await + match boxed( + self.services + .password_reset + .start_password_reset(request, received.device_info), + ) + .await { Ok(response_payload) => Some( core_response::Payload::PasswordResetStart(response_payload), @@ -773,11 +819,12 @@ impl ProxyHandler { } // rpc ResetPassword (PasswordResetRequest) returns (google.protobuf.Empty) Some(core_request::Payload::PasswordReset(request)) => { - match self - .services - .password_reset - .reset_password(request, received.device_info) - .await + match boxed( + self.services + .password_reset + .reset_password(request, received.device_info), + ) + .await { Ok(()) => Some(core_response::Payload::Empty(())), Err(err) => { @@ -788,11 +835,12 @@ impl ProxyHandler { } // rpc ClientMfaStart (ClientMfaStartRequest) returns (ClientMfaStartResponse) Some(core_request::Payload::ClientMfaStart(request)) => { - match self - .services - .client_mfa - .start_client_mfa_login(request, received.device_info) - .await + match boxed( + self.services + .client_mfa + .start_client_mfa_login(request, received.device_info), + ) + .await { Ok(ClientMfaStartOutcome::Approved(response_payload)) => { Some(core_response::Payload::ClientMfaStart(response_payload)) @@ -812,16 +860,13 @@ impl ProxyHandler { } // rpc ClientRemoteMfaFinish (ClientRemoteMfaFinishRequest) returns (ClientRemoteMfaFinishResponse) Some(core_request::Payload::AwaitRemoteMfaFinish(request)) => { - match self - .services - .client_mfa - .await_remote_mfa_login( - request, - tx.clone(), - received.id, - received.device_info, - ) - .await + match boxed(self.services.client_mfa.await_remote_mfa_login( + request, + tx.clone(), + received.id, + received.device_info, + )) + .await { Ok(()) => None, Err(err) => { @@ -832,11 +877,12 @@ impl ProxyHandler { } // rpc ClientMfaFinish (ClientMfaFinishRequest) returns (ClientMfaFinishResponse) Some(core_request::Payload::ClientMfaFinish(request)) => { - match self - .services - .client_mfa - .finish_client_mfa_login(request, received.device_info) - .await + match boxed( + self.services + .client_mfa + .finish_client_mfa_login(request, received.device_info), + ) + .await { Ok(response_payload) => { Some(core_response::Payload::ClientMfaFinish(response_payload)) @@ -858,11 +904,12 @@ impl ProxyHandler { } } Some(core_request::Payload::ClientMfaOidcAuthenticate(request)) => { - match self - .services - .client_mfa - .auth_mfa_session_with_oidc(request, received.device_info) - .await + match boxed( + self.services + .client_mfa + .auth_mfa_session_with_oidc(request, received.device_info), + ) + .await { Ok(()) => Some(core_response::Payload::Empty(())), Err(err) => { @@ -873,10 +920,7 @@ impl ProxyHandler { } // rpc LocationInfo (LocationInfoRequest) returns (LocationInfoResponse) Some(core_request::Payload::InstanceInfo(request)) => { - match self - .services - .polling - .info(request, received.device_info) + match boxed(self.services.polling.info(request, received.device_info)) .await { Ok(response_payload) => { @@ -890,7 +934,7 @@ impl ProxyHandler { debug!( "A client tried to fetch the instance config, but we are \ - not enterprise." + not enterprise." ); Some(core_response::Payload::CoreError(err.into())) } else { @@ -901,209 +945,16 @@ impl ProxyHandler { } } Some(core_request::Payload::AuthInfo(request)) => { - if is_business_license_active() { - let auth_flow_type = request.auth_flow_type(); - let redirect_url = match auth_flow_type { - ProtoAuthFlowType::Enrollment => { - let settings = Settings::get_current_settings(); - settings.edge_callback_url(AuthFlowType::Enrollment) - } - ProtoAuthFlowType::Mfa => { - let settings = Settings::get_current_settings(); - settings.edge_callback_url(AuthFlowType::Mfa) - } - // fall back for legacy pre-2.0 clients - ProtoAuthFlowType::Unspecified => - { - #[allow(deprecated)] - Url::parse(&request.redirect_url) - } - }; - - if let Ok(redirect_url) = redirect_url { - if let Some(provider) = - OpenIdProvider::get_current(&pool).await? - { - match make_oidc_client(redirect_url, &provider).await { - Ok((_client_id, client)) => { - match build_auth_info_state( - &pool, - auth_flow_type, - request.state, - ) - .await - { - Ok(state_data) => { - Some(build_auth_info_payload( - &client, &provider, state_data, - )) - } - Err(err) => { - error!( - "Failed to build OIDC state: {}", - err.message - ); - Some(core_response::Payload::CoreError(err)) - } - } - } - Err(err) => { - error!( - "Failed to setup external OIDC provider client: {err}" - ); - Some(core_response::Payload::CoreError( - CoreError::internal( - "failed to build OIDC client", - ), - )) - } - } - } else { - error!("Failed to get current OpenID provider"); - Some(core_response::Payload::CoreError( - CoreError::not_found( - "failed to get current OpenID provider", - ), - )) - } - } else { - error!("Invalid redirect URL in authentication info request"); - Some(core_response::Payload::CoreError(CoreError::internal( - "invalid redirect URL", - ))) - } - } else { - warn!("Enterprise license required"); - Some(core_response::Payload::CoreError( - CoreError::failed_precondition("no valid license"), - )) - } + boxed(self.handle_auth_info(request)).await? } Some(core_request::Payload::AuthCallback(request)) => { - match Settings::get_current_settings() - .edge_callback_url(AuthFlowType::Enrollment) - { - Ok(callback_url) => { - let code = AuthorizationCode::new(request.code); - match user_from_claims( - &pool, - Nonce::new(request.nonce), - code, - callback_url, - None, - None, - Some(&self.services.event_tx), - // Enrollment provisions the account. - ClaimsUserResolution::GetOrCreate, - ) - .await - { - Ok(mut user) => { - clear_unused_enrollment_tokens(&user, &pool).await?; - if let Err(err) = sync_user_groups_if_configured( - &user, - &pool, - &gateway_tx, - &self.services.ldap, - &self.services.dirsync, - ) - .await - { - error!( - "Failed to sync user groups for user {} with the \ - directory while the user was logging in through an \ - external provider: {err}", - user.username, - ); - } else { - ldap_update_user_state( - &mut user, - &pool, - &gateway_tx, - &self.services.ldap, - ) - .await; - } - debug!("Cleared unused tokens for {}.", user.username); - debug!( - "Creating a new desktop activation token for user {} \ - as a result of proxy OpenID auth callback.", - user.username - ); - let settings = Settings::get_current_settings(); - let desktop_configuration = Token::new( - user.id, - Some(user.id), - Some(user.email), - settings.enrollment_token_timeout().as_secs(), - Some(ENROLLMENT_TOKEN_TYPE.to_owned()), - ); - debug!("Saving a new desktop configuration token..."); - desktop_configuration.save(&pool).await?; - debug!( - "Saved desktop configuration token. Responding to \ - proxy with the token." - ); - let public_proxy_url = settings.proxy_public_url()?; - - Some(core_response::Payload::AuthCallback( - AuthCallbackResponse { - url: public_proxy_url.into(), - token: desktop_configuration.id, - }, - )) - } - Err(err) => { - error!("OpenID auth error {err}"); - let (status_code, message) = match err { - WebError::Authorization(message) => { - (Code::PermissionDenied as i32, message) - } - WebError::ObjectNotFound(message) => { - (Code::NotFound as i32, message) - } - WebError::Forbidden(message) => ( - Code::PermissionDenied as i32, - message.to_owned(), - ), - WebError::BadRequest(message) => { - (Code::InvalidArgument as i32, message) - } - WebError::LicenseLimitReached(message) => { - (Code::ResourceExhausted as i32, message) - } - _ => ( - Code::Internal as i32, - "OpenID authentication failed".to_owned(), - ), - }; - Some(core_response::Payload::CoreError(CoreError { - status_code, - message, - })) - } - } - } - Err(err) => { - error!( - "Proxy requested an OpenID authentication info for a \ - callback URL that couldn't be built. Details: {err}" - ); - Some(core_response::Payload::CoreError(CoreError::internal( - "invalid callback URL", - ))) - } - } + boxed(self.handle_auth_callback(request, &gateway_tx)).await? } - // Reply without payload. None => None, - // rpc AcmeCertificate: proxy completed ACME issuance. Some(core_request::Payload::AcmeCertificate(cert)) => { info!("Received AcmeCertificate from proxy, saving and broadcasting"); - // Parse the cert expiry from the PEM so we can store it. let acme_cert_expiry = parse_cert_expiry(&cert.cert_pem); - // Load current certificates row, patch ACME fields, and save. - match Certificates::get_or_default(&pool).await { + match boxed(Certificates::get_or_default(&pool)).await { Ok(mut certs) => { certs.proxy_http_cert_pem = Some(cert.cert_pem.clone()); certs.proxy_http_cert_key_pem = Some(cert.key_pem.clone()); @@ -1112,13 +963,12 @@ impl ProxyHandler { certs.proxy_http_cert_expiry = acme_cert_expiry; certs.proxy_http_cert_source = defguard_common::db::models::ProxyCertSource::LetsEncrypt; - if let Err(err) = certs.save(&pool).await { + if let Err(err) = boxed(certs.save(&pool)).await { error!( "Failed to save ACME certificate to certificates: {err}" ); } else { info!("ACME certificate saved to certificates"); - // Broadcast HttpsCerts to ALL connected proxies. let https_certs = CoreResponse { id: 0, payload: Some(core_response::Payload::HttpsCerts( @@ -1143,11 +993,12 @@ impl ProxyHandler { None } Some(core_request::Payload::DevicePostureCheck(request)) => { - match self - .services - .client_mfa - .handle_posture_check(request, received.device_info) - .await + match boxed( + self.services + .client_mfa + .handle_posture_check(request, received.device_info), + ) + .await { Ok(PostureCheckOutcome::Approved { preshared_key }) => { Some(core_response::Payload::DevicePostureCheck( @@ -1167,12 +1018,8 @@ impl ProxyHandler { } } } - // rpc ClientMfaStepStart (ClientMfaStepStartRequest) returns (ClientMfaStepStartResponse) Some(core_request::Payload::ClientMfaStepStart(request)) => { - match self - .services - .client_mfa - .client_mfa_step_start(request) + match boxed(self.services.client_mfa.client_mfa_step_start(request)) .await { Ok(response) => { @@ -1204,6 +1051,196 @@ impl ProxyHandler { Ok(()) } + + async fn handle_auth_info( + &self, + request: AuthInfoRequest, + ) -> Result, ProxyError> { + let pool = &self.pool; + let payload = if is_business_license_active() { + let auth_flow_type = request.auth_flow_type(); + let redirect_url = match auth_flow_type { + ProtoAuthFlowType::Enrollment => { + let settings = Settings::get_current_settings(); + settings.edge_callback_url(AuthFlowType::Enrollment) + } + ProtoAuthFlowType::Mfa => { + let settings = Settings::get_current_settings(); + settings.edge_callback_url(AuthFlowType::Mfa) + } + // fall back for legacy pre-2.0 clients + ProtoAuthFlowType::Unspecified => + { + #[allow(deprecated)] + Url::parse(&request.redirect_url) + } + }; + + if let Ok(redirect_url) = redirect_url { + if let Some(provider) = OpenIdProvider::get_current(pool).await? { + match make_oidc_client(redirect_url, &provider).await { + Ok((_client_id, client)) => { + match build_auth_info_state(pool, auth_flow_type, request.state).await { + Ok(state_data) => { + Some(build_auth_info_payload(&client, &provider, state_data)) + } + Err(err) => { + error!("Failed to build OIDC state: {}", err.message); + Some(core_response::Payload::CoreError(err)) + } + } + } + Err(err) => { + error!("Failed to setup external OIDC provider client: {err}"); + Some(core_response::Payload::CoreError(CoreError::internal( + "failed to build OIDC client", + ))) + } + } + } else { + error!("Failed to get current OpenID provider"); + Some(core_response::Payload::CoreError(CoreError::not_found( + "failed to get current OpenID provider", + ))) + } + } else { + error!("Invalid redirect URL in authentication info request"); + Some(core_response::Payload::CoreError(CoreError::internal( + "invalid redirect URL", + ))) + } + } else { + warn!("Enterprise license required"); + Some(core_response::Payload::CoreError( + CoreError::failed_precondition("no valid license"), + )) + }; + Ok(payload) + } + + async fn handle_auth_callback( + &self, + request: AuthCallbackRequest, + gateway_tx: &Sender, + ) -> Result, ProxyError> { + let pool = &self.pool; + let payload = match Settings::get_current_settings() + .edge_callback_url(AuthFlowType::Enrollment) + { + Ok(callback_url) => { + let code = AuthorizationCode::new(request.code); + match user_from_claims( + pool, + Nonce::new(request.nonce), + code, + callback_url, + None, + None, + Some(&self.services.event_tx), + // Enrollment provisions the account. + ClaimsUserResolution::GetOrCreate, + ) + .await + { + Ok(mut user) => { + clear_unused_enrollment_tokens(&user, pool).await?; + if let Err(err) = sync_user_groups_if_configured( + &user, + pool, + gateway_tx, + &self.services.ldap, + &self.services.dirsync, + ) + .await + { + error!( + "Failed to sync user groups for user {} with the \ + directory while the user was logging in through an \ + external provider: {err}", + user.username, + ); + } else { + ldap_update_user_state( + &mut user, + pool, + gateway_tx, + &self.services.ldap, + ) + .await; + } + debug!("Cleared unused tokens for {}.", user.username); + debug!( + "Creating a new desktop activation token for user {} \ + as a result of proxy OpenID auth callback.", + user.username + ); + let settings = Settings::get_current_settings(); + let desktop_configuration = Token::new( + user.id, + Some(user.id), + Some(user.email), + settings.enrollment_token_timeout().as_secs(), + Some(ENROLLMENT_TOKEN_TYPE.to_owned()), + ); + debug!("Saving a new desktop configuration token..."); + desktop_configuration.save(pool).await?; + debug!( + "Saved desktop configuration token. Responding to \ + proxy with the token." + ); + let public_proxy_url = settings.proxy_public_url()?; + + Some(core_response::Payload::AuthCallback(AuthCallbackResponse { + url: public_proxy_url.into(), + token: desktop_configuration.id, + })) + } + Err(err) => { + error!("OpenID auth error {err}"); + let (status_code, message) = match err { + WebError::Authorization(message) => { + (Code::PermissionDenied as i32, message) + } + WebError::ObjectNotFound(message) => (Code::NotFound as i32, message), + WebError::Forbidden(message) => { + (Code::PermissionDenied as i32, message.to_owned()) + } + WebError::BadRequest(message) => { + (Code::InvalidArgument as i32, message) + } + WebError::LicenseLimitReached(message) => { + (Code::ResourceExhausted as i32, message) + } + _ => ( + Code::Internal as i32, + "OpenID authentication failed".to_owned(), + ), + }; + Some(core_response::Payload::CoreError(CoreError { + status_code, + message, + })) + } + } + } + Err(err) => { + error!( + "Proxy requested an OpenID authentication info for a \ + callback URL that couldn't be built. Details: {err}" + ); + Some(core_response::Payload::CoreError(CoreError::internal( + "invalid callback URL", + ))) + } + }; + Ok(payload) + } +} + +fn boxed<'a, T>( + fut: impl Future + Send + 'a, +) -> Pin + Send + 'a>> { + Box::pin(fut) } #[cfg(test)] @@ -1366,6 +1403,7 @@ struct ProxyServices { password_reset: PasswordResetServer, client_mfa: ClientMfaServer, polling: PollingServer, + mfa_config: MfaConfigServer, ldap: UnboundedSender, dirsync: UnboundedSender, event_tx: UnboundedSender, @@ -1378,6 +1416,7 @@ impl ProxyServices { tx.wireguard.clone(), tx.bidi_events.clone(), tx.ldap.clone(), + tx.event_tx.clone(), ); let password_reset = PasswordResetServer::new(pool.clone(), tx.bidi_events.clone(), tx.ldap.clone()); @@ -1388,12 +1427,14 @@ impl ProxyServices { remote_mfa_responses, ); let polling = PollingServer::new(pool.clone()); + let mfa_config = MfaConfigServer::new(pool.clone()); Self { enrollment, password_reset, client_mfa, polling, + mfa_config, ldap: tx.ldap.clone(), dirsync: tx.dirsync.clone(), event_tx: tx.event_tx.clone(), diff --git a/crates/defguard_proxy_manager/src/servers/enrollment.rs b/crates/defguard_proxy_manager/src/servers/enrollment.rs index c1c33996bb..249b6bc226 100644 --- a/crates/defguard_proxy_manager/src/servers/enrollment.rs +++ b/crates/defguard_proxy_manager/src/servers/enrollment.rs @@ -14,7 +14,9 @@ use defguard_common::{ }, }; use defguard_core::{ - db::models::enrollment::{ENROLLMENT_TOKEN_TYPE, Token}, + db::models::enrollment::{ + ENROLLMENT_TOKEN_TYPE, MFA_CONFIG_SESSION_TIMEOUT, MFA_CONFIG_TOKEN_TYPE, Token, + }, device_access::{build_device_config, join_device_to_all_networks}, enterprise::{ db::models::{enterprise_settings::EnterpriseSettings, openid_provider::OpenIdProvider}, @@ -24,8 +26,8 @@ use defguard_core::{ limits::update_counts, }, events::{ - BidiRequestContext, BidiStreamEvent, BidiStreamEventType, EnrollmentEvent, - LdapSyncEventType, + ApiEvent, ApiEventType, ApiRequestContext, BidiRequestContext, BidiStreamEvent, + BidiStreamEventType, EnrollmentEvent, LdapSyncEventType, }, grpc::{ GatewayCommand, InstanceInfo, @@ -58,6 +60,7 @@ pub(crate) struct EnrollmentServer { gateway_tx: Sender, bidi_event_tx: UnboundedSender, ldap_tx: UnboundedSender, + event_tx: UnboundedSender, } impl EnrollmentServer { @@ -67,43 +70,62 @@ impl EnrollmentServer { gateway_tx: Sender, bidi_event_tx: UnboundedSender, ldap_tx: UnboundedSender, + event_tx: UnboundedSender, ) -> Self { Self { pool, gateway_tx, bidi_event_tx, ldap_tx, + event_tx, } } - /// Checks if token provided with request corresponds to a valid enrollment session + /// Validates an enrollment session token. async fn validate_session(&self, token: Option<&String>) -> Result { - info!("Validating enrollment session."); - let Some(token) = token else { - error!("Missing authorization header in request"); - return Err(Status::unauthenticated("Missing authorization header")); - }; - let enrollment = Token::find_by_id(&self.pool, token).await?; - debug!("Found matching token, verifying validity: {enrollment:?}."); - if enrollment - .token_type - .as_ref() - .is_none_or(|token_type| token_type != ENROLLMENT_TOKEN_TYPE) - { + let (token, is_enrollment) = self.validate_mfa_setup_session(token).await?; + if !is_enrollment { error!( "Invalid token type used in enrollment process: {:?}", - enrollment.token_type + token.token_type ); return Err(Status::permission_denied("invalid token")); } - let settings = Settings::get_current_settings(); - if enrollment.is_session_valid(settings.enrollment_session_timeout().as_secs()) { - info!("Enrollment session validated: {enrollment:?}"); - Ok(enrollment) + Ok(token) + } + + /// Validates an enrollment or MFA configuration session. + /// + /// The returned flag is `true` for an enrollment session. + async fn validate_mfa_setup_session( + &self, + token: Option<&String>, + ) -> Result<(Token, bool), Status> { + let Some(token) = token else { + error!("Missing authorization header in request"); + return Err(Status::unauthenticated("Missing authorization header")); + }; + let token = Token::find_by_id(&self.pool, token).await?; + let token_type = token.token_type.as_deref(); + let (is_enrollment, timeout) = if token_type == Some(ENROLLMENT_TOKEN_TYPE) { + ( + true, + Settings::get_current_settings() + .enrollment_session_timeout() + .as_secs(), + ) + } else if token_type == Some(MFA_CONFIG_TOKEN_TYPE) { + (false, MFA_CONFIG_SESSION_TIMEOUT.as_secs()) } else { - error!("Enrollment session expired: {enrollment:?}"); - Err(Status::unauthenticated("Session expired")) + error!("Invalid token type used in MFA setup: {token_type:?}"); + return Err(Status::permission_denied("invalid token")); + }; + // Setup tokens are valid only after authorization, when `used_at` is set. + if !token.is_session_valid(timeout) { + error!("MFA setup session is not valid: {token:?}"); + return Err(Status::unauthenticated("Session expired")); } + Ok((token, is_enrollment)) } /// Sends given `GatewayCommand` to be handled by gateway manager service @@ -331,7 +353,9 @@ impl EnrollmentServer { request: RegisterMobileAuthRequest, ) -> Result<(), Status> { debug!("Register mobile auth started"); - let enrollment = self.validate_session(Some(&request.token)).await?; + let (enrollment, _) = self + .validate_mfa_setup_session(Some(&request.token)) + .await?; let user = enrollment.fetch_user(&self.pool).await?; Device::validate_pubkey(&request.device_pub_key).map_err(|err| { error!( @@ -1075,21 +1099,22 @@ impl EnrollmentServer { build_device_config_response(&self.pool, device, Some(token), device_info).await } - // TODO: Add events #[instrument(skip_all)] - pub(crate) async fn register_code_mfa_start( + pub(crate) async fn mfa_setup_start( &self, request: CodeMfaSetupStartRequest, ) -> Result { - debug!("Begin enrollment code MFA setup start"); + debug!("Starting MFA setup"); let method = request.method(); if method != MfaMethod::Email && method != MfaMethod::Totp { return Err(Status::invalid_argument("Method not supported".to_owned())); } - let enrollment = Token::find_by_id(&self.pool, &request.token).await?; - let mut user = enrollment.fetch_user(&self.pool).await?; - // available only for unenrolled users - if user.is_enrolled() { + let (token, is_enrollment) = self + .validate_mfa_setup_session(Some(&request.token)) + .await?; + let mut user = token.fetch_user(&self.pool).await?; + // Enrollment cannot configure an already enrolled user; MFA configuration can. + if is_enrollment && user.is_enrolled() { return Err(Status::permission_denied("User is already enrolled")); } match method { @@ -1103,10 +1128,6 @@ impl EnrollmentServer { error!("Failed to create email secret"); Status::internal("Failed to setup email mfa".to_owned()) })?; - user.clear_recovery_codes(&self.pool).await.map_err(|e| { - error!("Failed to clear recovery codes: {e}"); - Status::internal("Failed to clear recovery codes".to_owned()) - })?; info!("Created email secret for {}", &user.username); let mut transaction = self.pool.begin().await.map_err(|err| { error!("Failed to begin database transaction\nReason:{err}"); @@ -1136,10 +1157,6 @@ impl EnrollmentServer { error!("Failed to make new TOTP secret"); Status::internal("Failed to make new TOTP secret".to_owned()) })?; - user.clear_recovery_codes(&self.pool).await.map_err(|e| { - error!("Failed to clear recovery codes: {e}"); - Status::internal("Failed to clear recovery codes".to_owned()) - })?; info!("New TOTP secret created for {}", &user.username); Ok(CodeMfaSetupStartResponse { totp_secret: Some(secret), @@ -1149,66 +1166,108 @@ impl EnrollmentServer { } } - // TODO: Add events #[instrument(skip_all)] - pub(crate) async fn register_code_mfa_finish( + pub(crate) async fn mfa_setup_finish( &self, request: CodeMfaSetupFinishRequest, + info: Option, ) -> Result { - debug!("Begin enrollment code mfa setup finish"); - let enrollment = self.validate_session(Some(&request.token)).await?; + debug!("Finishing MFA setup"); + let (token, is_enrollment) = self + .validate_mfa_setup_session(Some(&request.token)) + .await?; let method = request.method(); if method != MfaMethod::Totp && method != MfaMethod::Email { return Err(Status::invalid_argument("Method not supported")); } - let mut user = enrollment.fetch_user(&self.pool).await?; - // available only for unenrolled users - if user.is_enrolled() { + let mut user = token.fetch_user(&self.pool).await?; + // Enrollment cannot configure an already enrolled user; MFA configuration can. + if is_enrollment && user.is_enrolled() { return Err(Status::permission_denied("User is already enrolled")); } - // enable corresponding MFA - let mfa_method: MFAMethod = match method { + let mut transaction = self.pool.begin().await.map_err(|err| { + error!("Failed to begin database transaction: {err}"); + Status::internal("Failed to begin database transaction".to_owned()) + })?; + + let (mfa_method, event) = match method { MfaMethod::Email => { if !user.verify_email_mfa_code(&request.code) { return Err(Status::invalid_argument("Email code invalid".to_owned())); } - user.enable_email_mfa(&self.pool) + user.enable_email_mfa(&mut *transaction) .await .map_err(|_| Status::internal("Enabling method failed.".to_owned()))?; - MFAMethod::Email + (MFAMethod::Email, ApiEventType::MfaEmailEnabled) } MfaMethod::Totp => { if !user.verify_totp_code(&request.code) { return Err(Status::invalid_argument("Code invalid".to_owned())); } - user.enable_totp(&self.pool) + user.enable_totp(&mut *transaction) .await .map_err(|_| Status::internal("Enabling method failed.".to_owned()))?; - MFAMethod::OneTimePassword + (MFAMethod::OneTimePassword, ApiEventType::MfaTotpEnabled) } _ => { return Err(Status::invalid_argument("Method not supported")); } }; - user.enable_mfa(&self.pool) + // Enabling MFA invalidates all existing sessions in the same transaction. + user.logout_all_sessions(&mut *transaction) .await - .map_err(|_| Status::internal("Enabling MFA on the account failed.".to_owned()))?; + .map_err(|err| { + error!("Failed to log out user sessions: {err}"); + Status::internal("Failed to log out user sessions".to_owned()) + })?; + // New enrollments get fresh recovery codes. Existing users keep their current + // codes when adding a factor. + if is_enrollment { + user.clear_recovery_codes(&mut *transaction) + .await + .map_err(|err| { + error!("Failed to clear recovery codes: {err}"); + Status::internal("Failed to clear recovery codes".to_owned()) + })?; + } + // Existing recovery codes were already shown, so return an empty list. let recovery_codes = user - .get_recovery_codes(&self.pool) + .get_recovery_codes(&mut *transaction) .await .map_err(|_| Status::internal("Failed to get recovery codes.".to_owned()))? - .ok_or_else(|| Status::internal("Recovery codes not found".to_owned()))?; - if let Ok(mut conn) = self.pool.begin().await { - if let Err(err) = - mfa_configured_mail(&user.email, &mut conn, None, &mfa_method, &user.first_name) - .await - { - error!("Failed to send MFA configured email\nReason: {err}"); + .unwrap_or_default(); + transaction.commit().await.map_err(|err| { + error!("Failed to commit database transaction: {err}"); + Status::internal("Failed to commit database transaction".to_owned()) + })?; + + // Commit before reading the saved factor state or sending the confirmation email. + user.enable_mfa(&self.pool) + .await + .map_err(|_| Status::internal("Enabling MFA on the account failed.".to_owned()))?; + match self.pool.acquire().await { + Ok(mut conn) => { + if let Err(err) = + mfa_configured_mail(&user.email, &mut conn, None, &mfa_method, &user.first_name) + .await + { + error!("Failed to send MFA configured email\nReason: {err}"); + } } - } else { - error!("Failed to begin database session"); + Err(err) => error!("Failed to acquire database connection: {err}"), } + let (ip, user_agent) = parse_client_ip_agent(&info).map_err(Status::internal)?; + let context = ApiRequestContext::new(user.id, user.username.clone(), ip, user_agent); + self.event_tx + .send(ApiEvent { + context, + event: Box::new(event), + }) + .map_err(|err| { + error!("Failed to send event. Reason: {err}"); + Status::internal("unexpected error") + })?; info!( "Successfully enabled MFA method {} for user {}", method.as_str_name(), @@ -1337,7 +1396,9 @@ mod test { let (gateway_tx, _) = broadcast::channel(1); let (bidi_event_tx, _) = unbounded_channel(); let (ldap_tx, _) = unbounded_channel(); - let server = EnrollmentServer::new(pool.clone(), gateway_tx, bidi_event_tx, ldap_tx); + let (event_tx, _) = unbounded_channel(); + let server = + EnrollmentServer::new(pool.clone(), gateway_tx, bidi_event_tx, ldap_tx, event_tx); let mut transaction = pool.begin().await.unwrap(); let result = server @@ -1392,7 +1453,9 @@ mod test { let (gateway_tx, _gateway_rx) = broadcast::channel(1); let (bidi_event_tx, _bidi_events_rx) = unbounded_channel(); let (ldap_tx, _ldap_rx) = unbounded_channel(); - let server = EnrollmentServer::new(pool.clone(), gateway_tx, bidi_event_tx, ldap_tx); + let (event_tx, _event_rx) = unbounded_channel(); + let server = + EnrollmentServer::new(pool.clone(), gateway_tx, bidi_event_tx, ldap_tx, event_tx); let request = EnrollmentStartRequest { token: token.id.clone(), diff --git a/crates/defguard_proxy_manager/src/servers/mfa_config.rs b/crates/defguard_proxy_manager/src/servers/mfa_config.rs new file mode 100644 index 0000000000..3b337b176c --- /dev/null +++ b/crates/defguard_proxy_manager/src/servers/mfa_config.rs @@ -0,0 +1,330 @@ +use defguard_common::db::{ + Id, + models::{ + Device, Settings, User, polling_token::PollingToken, vpn_client_session::VpnClientMfaMethod, + }, +}; +use defguard_core::{ + db::models::enrollment::{MFA_CONFIG_SESSION_TIMEOUT, MFA_CONFIG_TOKEN_TYPE, Token}, + mail::templates::mfa_code_mail, +}; +use defguard_proto::client_types::{ + MfaConfigAuthorizeRequest, MfaConfigAuthorizeResponse, MfaConfigSendCodeRequest, + MfaConfigSendCodeResponse, MfaConfigStartRequest, MfaConfigStartResponse, MfaMethod, +}; +use sqlx::PgPool; +use tonic::Status; + +/// An unauthorized MFA configuration session and its user's current factor state. +struct MfaConfigSession { + token: Token, + user: User, + totp_configured: bool, + email_configured: bool, +} + +/// Handles MFA factor configuration requested by an already enrolled desktop client. +pub(crate) struct MfaConfigServer { + pool: PgPool, +} + +impl MfaConfigServer { + #[must_use] + pub fn new(pool: PgPool) -> Self { + Self { pool } + } + + async fn is_configured( + &self, + method: VpnClientMfaMethod, + user: &User, + device_id: Id, + smtp_configured: bool, + ) -> Result { + method + .is_configured(&self.pool, user, Some(device_id), smtp_configured, false) + .await + .map_err(|err| { + error!( + "MFA config: failed to read {method:?} state for user {}: {err}", + user.username + ); + Status::internal("unexpected error") + }) + } + + /// Starts an MFA configuration session for the device identified by the polling token. + /// + /// The returned token remains unused until the user proves an existing factor. + /// The client requests an email code separately with `mfa_config_send_code`. + /// When no factor is configured, an email code is the only authorization method. + #[instrument(skip_all)] + pub(crate) async fn mfa_config_start( + &self, + request: MfaConfigStartRequest, + ) -> Result { + debug!( + "Starting MFA configuration for device pubkey {}", + request.pubkey + ); + // Authenticate before any lookup so error codes cannot probe which pubkeys exist. + if request.token.is_empty() { + error!("MFA config start: missing polling token"); + return Err(Status::unauthenticated("missing token")); + } + let polling_token = PollingToken::find(&self.pool, &request.token) + .await + .map_err(|err| { + error!("MFA config start: failed to look up polling token: {err}"); + Status::internal("unexpected error") + })? + .ok_or_else(|| { + error!("MFA config start: unknown polling token"); + Status::unauthenticated("invalid token") + })?; + let Ok(Some(device)) = Device::find_by_pubkey(&self.pool, &request.pubkey).await else { + error!( + "MFA config start: device with pubkey {} not found", + request.pubkey + ); + return Err(Status::unauthenticated("invalid token")); + }; + if polling_token.device_id != device.id { + error!( + "MFA config start: polling token belongs to device {} but request claims device {}", + polling_token.device_id, device.id + ); + return Err(Status::unauthenticated("token does not match device")); + } + let Ok(Some(mut user)) = User::find_by_id(&self.pool, device.user_id).await else { + error!("MFA config start: user {} not found", device.user_id); + return Err(Status::internal("user not found")); + }; + if !user.is_active { + error!("MFA config start: user {} is inactive", user.username); + return Err(Status::permission_denied("user is inactive")); + } + + let settings = Settings::get_current_settings(); + let smtp_configured = settings.smtp_configured(); + let mut available_methods = Vec::with_capacity(2); + for method in [VpnClientMfaMethod::Totp, VpnClientMfaMethod::Email] { + if self + .is_configured(method, &user, device.id, smtp_configured) + .await? + { + available_methods.push(MfaMethod::from(method) as i32); + } + } + + let mut transaction = self.pool.begin().await.map_err(|err| { + error!("MFA config start: failed to begin transaction: {err}"); + Status::internal("unexpected error") + })?; + + let email_fallback = available_methods.is_empty(); + if email_fallback { + if !smtp_configured { + error!( + "MFA config start: user {} has no MFA method and SMTP is not configured", + user.username + ); + return Err(Status::failed_precondition("SMTP not configured")); + } + // Rotate the secret because a disabled email factor retains its old secret. + user.new_email_secret(&mut *transaction) + .await + .map_err(|err| { + error!("MFA config start: failed to create email secret: {err}"); + Status::internal("unexpected error") + })?; + } + + Token::delete_unused_user_tokens_of_type(&mut *transaction, user.id, MFA_CONFIG_TOKEN_TYPE) + .await?; + let mut token = Token::new( + user.id, + None, + None, + MFA_CONFIG_SESSION_TIMEOUT.as_secs(), + Some(MFA_CONFIG_TOKEN_TYPE.to_owned()), + ); + token.device_id = Some(device.id); + token.save(&mut *transaction).await?; + + transaction.commit().await.map_err(|err| { + error!("MFA config start: failed to commit transaction: {err}"); + Status::internal("unexpected error") + })?; + info!( + "User {} started MFA configuration from device {} (email fallback: {email_fallback})", + user.username, device.name + ); + + Ok(MfaConfigStartResponse { + session_token: token.id, + available_methods, + email_fallback, + deadline_timestamp: token.expires_at.and_utc().timestamp(), + }) + } + + /// Loads an unauthorized MFA configuration session and its user's current factor state. + async fn load_session(&self, session_token: &str) -> Result { + let token = Token::find_by_id(&self.pool, session_token).await?; + if token.token_type.as_deref() != Some(MFA_CONFIG_TOKEN_TYPE) { + error!( + "MFA config: token {} has type {:?}, expected {MFA_CONFIG_TOKEN_TYPE}", + token.id, token.token_type + ); + return Err(Status::unauthenticated("invalid token")); + } + if token.is_expired() { + error!("MFA config: token {} expired", token.id); + return Err(Status::unauthenticated("invalid token")); + } + if token.is_used() { + error!("MFA config: token {} is already authorized", token.id); + return Err(Status::failed_precondition("session already authorized")); + } + let user = token.fetch_user(&self.pool).await?; + if !user.is_active { + error!("MFA config: user {} is inactive", user.username); + return Err(Status::permission_denied("user is inactive")); + } + let Some(device_id) = token.device_id else { + error!("MFA config: token {} has no device", token.id); + return Err(Status::internal("unexpected error")); + }; + + let smtp_configured = Settings::get_current_settings().smtp_configured(); + let totp_configured = self + .is_configured(VpnClientMfaMethod::Totp, &user, device_id, smtp_configured) + .await?; + let email_configured = self + .is_configured(VpnClientMfaMethod::Email, &user, device_id, smtp_configured) + .await?; + + Ok(MfaConfigSession { + token, + user, + totp_configured, + email_configured, + }) + } + + /// Sends the email code used to authorize an MFA configuration session. + /// + /// The client calls this after the user selects Email, including the no-factor fallback. + #[instrument(skip_all)] + pub(crate) async fn mfa_config_send_code( + &self, + request: MfaConfigSendCodeRequest, + ) -> Result { + debug!("Sending MFA configuration email code"); + let session = self.load_session(&request.session_token).await?; + let user = &session.user; + let email_fallback = !session.totp_configured && !session.email_configured; + if !session.email_configured && !email_fallback { + error!( + "MFA config send code: user {} has no configured email MFA", + user.username + ); + return Err(Status::permission_denied("method not configured")); + } + + let code = user.generate_email_mfa_code().map_err(|err| { + error!("MFA config send code: failed to generate email code: {err}"); + Status::internal("unexpected error") + })?; + let mut conn = self.pool.acquire().await.map_err(|err| { + error!("MFA config send code: failed to acquire connection: {err}"); + Status::internal("unexpected error") + })?; + mfa_code_mail(&user.email, &mut conn, &user.first_name, &code, None, true) + .await + .map_err(|err| { + error!("MFA config send code: failed to send email code: {err}"); + Status::internal("unexpected error") + })?; + info!( + "Sent MFA configuration email code to user {}", + user.username + ); + + Ok(MfaConfigSendCodeResponse {}) + } + + /// Authorizes an MFA configuration session with a TOTP or email code. + /// + /// Authorization turns the token into a setup session. The email fallback proves mailbox + /// access, but only MFA setup enables a factor. + #[instrument(skip_all)] + pub(crate) async fn mfa_config_authorize( + &self, + request: MfaConfigAuthorizeRequest, + ) -> Result { + debug!("Authorizing MFA configuration session"); + let MfaConfigSession { + mut token, + user, + totp_configured, + email_configured, + } = self.load_session(&request.session_token).await?; + // With no configured factor, Email is the fallback authorization method. + let email_fallback = !totp_configured && !email_configured; + + let method = MfaMethod::try_from(request.method).map_err(|_| { + error!("MFA config authorize: unknown method {}", request.method); + Status::invalid_argument("unknown method") + })?; + let allowed = match method { + MfaMethod::Totp => totp_configured, + MfaMethod::Email => email_configured || email_fallback, + _ => { + error!("MFA config authorize: method {method} cannot authorize a session"); + return Err(Status::invalid_argument("method cannot authorize")); + } + }; + if !allowed { + error!( + "MFA config authorize: user {} has no configured {method}", + user.username + ); + return Err(Status::permission_denied("method not configured")); + } + + let valid = match method { + MfaMethod::Totp => user.verify_totp_code(&request.code), + _ => user.verify_email_mfa_code(&request.code), + }; + if !valid { + error!( + "MFA config authorize: invalid {method} code for user {}", + user.username + ); + return Err(Status::unauthenticated("invalid code")); + } + + let mut transaction = self.pool.begin().await.map_err(|err| { + error!("MFA config authorize: failed to begin transaction: {err}"); + Status::internal("unexpected error") + })?; + let deadline = token + .start_session(&mut transaction, MFA_CONFIG_SESSION_TIMEOUT.as_secs()) + .await?; + transaction.commit().await.map_err(|err| { + error!("MFA config authorize: failed to commit transaction: {err}"); + Status::internal("unexpected error") + })?; + + info!( + "User {} authorized MFA configuration with {method} (email fallback: {email_fallback})", + user.username + ); + + Ok(MfaConfigAuthorizeResponse { + deadline_timestamp: deadline.and_utc().timestamp(), + }) + } +} diff --git a/crates/defguard_proxy_manager/src/servers/mod.rs b/crates/defguard_proxy_manager/src/servers/mod.rs index 52ce7c164e..f9d02b362f 100644 --- a/crates/defguard_proxy_manager/src/servers/mod.rs +++ b/crates/defguard_proxy_manager/src/servers/mod.rs @@ -1,5 +1,7 @@ mod enrollment; +mod mfa_config; mod password_reset; pub(crate) use enrollment::EnrollmentServer; +pub(crate) use mfa_config::MfaConfigServer; pub(crate) use password_reset::PasswordResetServer; diff --git a/crates/defguard_proxy_manager/src/tests/proxy_manager/handler/mfa_config.rs b/crates/defguard_proxy_manager/src/tests/proxy_manager/handler/mfa_config.rs new file mode 100644 index 0000000000..fa976b5535 --- /dev/null +++ b/crates/defguard_proxy_manager/src/tests/proxy_manager/handler/mfa_config.rs @@ -0,0 +1,225 @@ +use defguard_common::{ + db::models::{Session, SessionState, User}, + testing::smtp::configure_working_smtp, +}; +use defguard_proto::{client_types::MfaMethod, proxy::core_response}; +use sqlx::postgres::{PgConnectOptions, PgPoolOptions}; + +use super::support::{ + assert_error_response, complete_proxy_handshake, create_polling_token, create_user_with_device, + generate_totp_code, send_code_mfa_setup_finish, send_code_mfa_setup_start, + send_mfa_config_authorize, send_mfa_config_send_code, send_mfa_config_start, + setup_user_totp_mfa, totp_code_from_base32_secret, +}; +use crate::tests::common::HandlerTestContext; + +/// The session token must gate factor setup until the user proves a configured factor. +#[sqlx::test] +async fn test_mfa_config_session_gates_factor_setup(_: PgPoolOptions, options: PgConnectOptions) { + let mut context = HandlerTestContext::new(options).await; + complete_proxy_handshake(&mut context).await; + + let (mut user, device) = create_user_with_device(&context.pool).await; + setup_user_totp_mfa(&context.pool, &mut user).await; + let polling_token = create_polling_token(&context.pool, device.id).await; + + let start_response = + send_mfa_config_start(&mut context, &polling_token, &device.wireguard_pubkey).await; + let session = match &start_response.payload { + Some(core_response::Payload::MfaConfigStart(response)) => response, + _ => panic!("expected MfaConfigStartResponse"), + }; + assert_eq!(session.available_methods, vec![MfaMethod::Totp as i32]); + assert!(!session.email_fallback); + let session_token = session.session_token.clone(); + + let unauthorized = + send_code_mfa_setup_start(&mut context, &session_token, MfaMethod::Totp).await; + assert_error_response(&unauthorized); + + let authorized = send_mfa_config_authorize( + &mut context, + &session_token, + MfaMethod::Totp, + &generate_totp_code(&user), + ) + .await; + assert!( + matches!( + authorized.payload, + Some(core_response::Payload::MfaConfigAuthorize(_)) + ), + "a valid TOTP code must authorize the session" + ); + + let setup = send_code_mfa_setup_start(&mut context, &session_token, MfaMethod::Totp).await; + match &setup.payload { + Some(core_response::Payload::CodeMfaSetupStartResponse(response)) => { + assert!(response.totp_secret.is_some()); + } + _ => panic!("expected CodeMfaSetupStartResponse"), + } + + let replayed = send_mfa_config_authorize( + &mut context, + &session_token, + MfaMethod::Totp, + &generate_totp_code(&user), + ) + .await; + assert_error_response(&replayed); + + context.finish().await.expect_server_finished().await; +} + +/// Email fallback authorizes the session without enabling a factor or issuing recovery codes. +#[sqlx::test] +async fn test_email_fallback_authorizes_without_enabling_a_factor( + _: PgPoolOptions, + options: PgConnectOptions, +) { + let mut context = HandlerTestContext::new(options).await; + complete_proxy_handshake(&mut context).await; + let _smtp = configure_working_smtp(&context.pool).await; + + let (user, device) = create_user_with_device(&context.pool).await; + let polling_token = create_polling_token(&context.pool, device.id).await; + // Verify that enabling MFA invalidates an existing password-only session. + let web_session = Session::new( + user.id, + SessionState::PasswordVerified, + "10.0.0.1".into(), + None, + ); + web_session.save(&context.pool).await.expect("save session"); + + let start_response = + send_mfa_config_start(&mut context, &polling_token, &device.wireguard_pubkey).await; + let session = match &start_response.payload { + Some(core_response::Payload::MfaConfigStart(response)) => response, + _ => panic!("expected MfaConfigStartResponse"), + }; + assert!(session.available_methods.is_empty()); + assert!(session.email_fallback); + let session_token = session.session_token.clone(); + + let sent = send_mfa_config_send_code(&mut context, &session_token).await; + assert!( + matches!( + sent.payload, + Some(core_response::Payload::MfaConfigSendCode(_)) + ), + "the fallback must send an email code" + ); + + // Reload because mfa_config_start rotated the email secret. + let user = User::find_by_id(&context.pool, user.id) + .await + .expect("find user") + .expect("user exists"); + let code = user + .generate_email_mfa_code() + .expect("generate_email_mfa_code"); + let authorized = + send_mfa_config_authorize(&mut context, &session_token, MfaMethod::Email, &code).await; + assert!( + matches!( + authorized.payload, + Some(core_response::Payload::MfaConfigAuthorize(_)) + ), + "a valid email code must authorize the session" + ); + + let user = User::find_by_id(&context.pool, user.id) + .await + .expect("find user") + .expect("user exists"); + assert!(!user.mfa_enabled, "authorization must not enable MFA"); + assert!( + !user.email_mfa_enabled, + "authorization must not enable the email factor" + ); + assert!( + user.recovery_codes.is_empty(), + "authorization must not issue recovery codes" + ); + + let setup = send_code_mfa_setup_start(&mut context, &session_token, MfaMethod::Totp).await; + let secret = match &setup.payload { + Some(core_response::Payload::CodeMfaSetupStartResponse(response)) => response + .totp_secret + .clone() + .expect("totp_secret must be returned"), + _ => panic!("expected CodeMfaSetupStartResponse"), + }; + let finish = send_code_mfa_setup_finish( + &mut context, + &session_token, + MfaMethod::Totp, + &totp_code_from_base32_secret(&secret), + ) + .await; + let issued = match &finish.payload { + Some(core_response::Payload::CodeMfaSetupFinishResponse(response)) => { + &response.recovery_codes + } + _ => panic!("expected CodeMfaSetupFinishResponse"), + }; + assert!(!issued.is_empty(), "setup must issue recovery codes"); + + let user = User::find_by_id(&context.pool, user.id) + .await + .expect("find user") + .expect("user exists"); + assert!(user.totp_enabled, "setup must enable the TOTP factor"); + assert_eq!( + &user.recovery_codes, issued, + "the response must contain the stored recovery codes" + ); + assert!( + Session::find_by_id(&context.pool, &web_session.id) + .await + .expect("find session") + .is_none(), + "enabling MFA must end password-only web sessions" + ); + + // Adding a second factor must keep the existing recovery codes. + let issued = issued.clone(); + let started = send_code_mfa_setup_start(&mut context, &session_token, MfaMethod::Email).await; + assert!( + matches!( + started.payload, + Some(core_response::Payload::CodeMfaSetupStartResponse(_)) + ), + "the session must allow a second factor" + ); + // Reload because setup start rotated the email secret. + let user = User::find_by_id(&context.pool, user.id) + .await + .expect("find user") + .expect("user exists"); + let code = user + .generate_email_mfa_code() + .expect("generate_email_mfa_code"); + let finish = + send_code_mfa_setup_finish(&mut context, &session_token, MfaMethod::Email, &code).await; + match &finish.payload { + Some(core_response::Payload::CodeMfaSetupFinishResponse(response)) => assert!( + response.recovery_codes.is_empty(), + "adding a second factor must not return new recovery codes" + ), + _ => panic!("expected CodeMfaSetupFinishResponse"), + } + let user = User::find_by_id(&context.pool, user.id) + .await + .expect("find user") + .expect("user exists"); + assert!(user.email_mfa_enabled, "setup must enable the email factor"); + assert_eq!( + user.recovery_codes, issued, + "adding a factor must not change the stored recovery codes" + ); + + context.finish().await.expect_server_finished().await; +} diff --git a/crates/defguard_proxy_manager/src/tests/proxy_manager/handler/mod.rs b/crates/defguard_proxy_manager/src/tests/proxy_manager/handler/mod.rs index a7dadd5e01..cd2e345e74 100644 --- a/crates/defguard_proxy_manager/src/tests/proxy_manager/handler/mod.rs +++ b/crates/defguard_proxy_manager/src/tests/proxy_manager/handler/mod.rs @@ -4,6 +4,7 @@ mod acme; mod enrollment; mod lifecycle; mod mfa; +mod mfa_config; mod mfa_legacy; mod oidc; mod password_reset; diff --git a/crates/defguard_proxy_manager/src/tests/proxy_manager/handler/support.rs b/crates/defguard_proxy_manager/src/tests/proxy_manager/handler/support.rs index db331018f7..290a3edb84 100644 --- a/crates/defguard_proxy_manager/src/tests/proxy_manager/handler/support.rs +++ b/crates/defguard_proxy_manager/src/tests/proxy_manager/handler/support.rs @@ -41,7 +41,8 @@ use defguard_proto::{ client_types::{ ActivateUserRequest, ClientMfaFinishRequest, ClientMfaStartRequest, ClientMfaStepStartRequest, ClientMfaStepStartResponse, CodeMfaSetupFinishRequest, - CodeMfaSetupStartRequest, DeviceConfigResponse, EnrollmentStartRequest, MfaMethod, + CodeMfaSetupStartRequest, DeviceConfigResponse, EnrollmentStartRequest, + MfaConfigAuthorizeRequest, MfaConfigSendCodeRequest, MfaConfigStartRequest, MfaMethod, }, proxy::{ ClientMfaTokenValidationRequest, CoreRequest, CoreResponse, DeviceInfo, @@ -1329,6 +1330,69 @@ pub(crate) async fn send_code_mfa_setup_finish( context.mock_proxy_mut().recv_outbound().await } +/// Send an `MfaConfigStart` request for the given polling token and device key. +pub(crate) async fn send_mfa_config_start( + context: &mut HandlerTestContext, + polling_token: &str, + pubkey: &str, +) -> CoreResponse { + static MFA_CONFIG_START_CTR: AtomicU64 = AtomicU64::new(4000); + let id = MFA_CONFIG_START_CTR.fetch_add(1, Ordering::Relaxed); + context.mock_proxy().send_request(CoreRequest { + id, + device_info: Some(make_device_info()), + payload: Some(core_request::Payload::MfaConfigStart( + MfaConfigStartRequest { + token: polling_token.to_owned(), + pubkey: pubkey.to_owned(), + }, + )), + }); + context.mock_proxy_mut().recv_outbound().await +} + +/// Request an email code for an MFA configuration session. +pub(crate) async fn send_mfa_config_send_code( + context: &mut HandlerTestContext, + session_token: &str, +) -> CoreResponse { + static MFA_CONFIG_CODE_CTR: AtomicU64 = AtomicU64::new(4250); + let id = MFA_CONFIG_CODE_CTR.fetch_add(1, Ordering::Relaxed); + context.mock_proxy().send_request(CoreRequest { + id, + device_info: Some(make_device_info()), + payload: Some(core_request::Payload::MfaConfigSendCode( + MfaConfigSendCodeRequest { + session_token: session_token.to_owned(), + }, + )), + }); + context.mock_proxy_mut().recv_outbound().await +} + +/// Send an `MfaConfigAuthorize` request for the given session token, method and code. +pub(crate) async fn send_mfa_config_authorize( + context: &mut HandlerTestContext, + session_token: &str, + method: MfaMethod, + code: &str, +) -> CoreResponse { + static MFA_CONFIG_AUTH_CTR: AtomicU64 = AtomicU64::new(4500); + let id = MFA_CONFIG_AUTH_CTR.fetch_add(1, Ordering::Relaxed); + context.mock_proxy().send_request(CoreRequest { + id, + device_info: Some(make_device_info()), + payload: Some(core_request::Payload::MfaConfigAuthorize( + MfaConfigAuthorizeRequest { + session_token: session_token.to_owned(), + method: method as i32, + code: code.to_owned(), + }, + )), + }); + context.mock_proxy_mut().recv_outbound().await +} + /// Set minimal SMTP fields on a [`Settings`] so that `smtp_configured()` returns `true`. pub(crate) fn configure_smtp(settings: &mut Settings) { settings.smtp.server = Some("smtp.example.com".into()); diff --git a/proto b/proto index 0509e3ebf2..cd1afb7277 160000 --- a/proto +++ b/proto @@ -1 +1 @@ -Subproject commit 0509e3ebf2508aca829e81ae2bb5423d748a49a8 +Subproject commit cd1afb727774b51118c470fafbb5efd808e4d405