From 4303b2fa3d78745fdcf28f2267920c40d5fd49a8 Mon Sep 17 00:00:00 2001 From: Aleksander <170264518+t-aleksander@users.noreply.github.com> Date: Thu, 12 Sep 2024 13:15:31 +0200 Subject: [PATCH 1/5] instance settings fetching rework --- .../db/models/enterprise_settings.rs | 2 +- src/enterprise/grpc/polling.rs | 6 +-- src/grpc/enrollment.rs | 43 ++++++++++++++++--- src/grpc/mod.rs | 22 ++++++++-- src/grpc/utils.rs | 31 ++++--------- 5 files changed, 67 insertions(+), 37 deletions(-) diff --git a/src/enterprise/db/models/enterprise_settings.rs b/src/enterprise/db/models/enterprise_settings.rs index 6b1a4f33ca..91b9be6416 100644 --- a/src/enterprise/db/models/enterprise_settings.rs +++ b/src/enterprise/db/models/enterprise_settings.rs @@ -4,7 +4,7 @@ use struct_patch::Patch; use crate::enterprise::license::{get_cached_license, validate_license}; -#[derive(Model, Deserialize, Serialize, Patch)] +#[derive(Debug, Model, Deserialize, Serialize, Patch)] #[patch(attribute(derive(Serialize, Deserialize)))] pub struct EnterpriseSettings { #[serde(skip)] diff --git a/src/enterprise/grpc/polling.rs b/src/enterprise/grpc/polling.rs index 88fb28a07f..79964e5b52 100644 --- a/src/enterprise/grpc/polling.rs +++ b/src/enterprise/grpc/polling.rs @@ -5,7 +5,7 @@ use crate::{ enterprise::license::{get_cached_license, validate_license}, grpc::{ proto::{InstanceInfoRequest, InstanceInfoResponse}, - utils::{build_device_config_response, build_instance_config_response}, + utils::build_device_config_response, }, }; @@ -26,7 +26,7 @@ impl PollingServer { // Polling service is enterprise-only, check the lincense if validate_license(get_cached_license().as_ref()).is_err() { debug!("No valid license, denying instance polling info"); - return Err(Status::permission_denied("no valid license")); + return Err(Status::failed_precondition("no valid license")); } // Validate the token @@ -83,10 +83,8 @@ impl PollingServer { // Build & return polling info let device_config = build_device_config_response(&self.pool, &device.wireguard_pubkey).await?; - let instance_config = build_instance_config_response(&self.pool).await?; Ok(InstanceInfoResponse { device_config: Some(device_config), - instance_config: Some(instance_config), }) } } diff --git a/src/grpc/enrollment.rs b/src/grpc/enrollment.rs index 21ac12f783..2072fb58da 100644 --- a/src/grpc/enrollment.rs +++ b/src/grpc/enrollment.rs @@ -126,7 +126,10 @@ impl EnrollmentServer { ); return Err(Status::permission_denied("user is disabled")); }; - info!("User {}({:?}) is active", user.username, user.id); + info!( + "User {}({:?}) is active, proceeding with enrollment", + user.username, user.id + ); let mut transaction = self.pool.begin().await.map_err(|_| { error!("Failed to begin a transaction for enrollment."); @@ -150,7 +153,7 @@ impl EnrollmentServer { ); debug!( - "Retrieving settings for enrollment purpose for user {}({:?}).", + "Retrieving settings for enrollment of user {}({:?}).", user.username, user.id ); let settings = Settings::get_settings(&mut *transaction) @@ -161,12 +164,25 @@ impl EnrollmentServer { })?; debug!("Settings: {settings:?}"); + debug!( + "Retrieving enterprise settings for enrollment of user {}({:?}).", + user.username, user.id + ); + let enterprise_settings = + EnterpriseSettings::get(&mut *transaction) + .await + .map_err(|_| { + error!("Failed to get enterprise settings."); + Status::internal("unexpected error") + })?; + debug!("Enterprise settings: {enterprise_settings:?}"); + let vpn_setup_optional = settings.enrollment_vpn_step_optional; debug!( "Retrieving instance info for user {}({:?}).", user.username, user.id ); - let instance_info = InstanceInfo::new(settings, &user.username); + let instance_info = InstanceInfo::new(settings, &user.username, enterprise_settings); debug!("Instance info {instance_info:?}"); debug!( @@ -185,7 +201,7 @@ impl EnrollmentServer { })?; debug!("User info {user_info:?}"); - debug!("Try to get basic admin info..."); + debug!("Trying to get basic admin info..."); let admin_info = admin.map(AdminInfo::from); debug!("Admin info {admin_info:?}"); @@ -498,7 +514,22 @@ impl EnrollmentServer { ); Status::internal("unexpected error") })?; - debug!("Settings {settings:?}"); + debug!("Settings: {settings:?}"); + + debug!( + "Fetching enterprise settings for device {} creation process for user {}({:?})", + device.wireguard_pubkey, user.username, user.id, + ); + let enterprise_settings = EnterpriseSettings::get(&mut *transaction) + .await + .map_err(|err| { + error!( + "Failed to fetch enterprise settings for device {} creation process for user {}({:?}): {err}", + device.wireguard_pubkey, user.username, user.id, + ); + Status::internal("unexpected error") + })?; + debug!("Enterprise settings: {enterprise_settings:?}"); // create polling token for further client communication debug!( @@ -560,7 +591,7 @@ impl EnrollmentServer { let response = DeviceConfigResponse { device: Some(device.into()), configs: configs.into_iter().map(Into::into).collect(), - instance: Some(InstanceInfo::new(settings, &user.username).into()), + instance: Some(InstanceInfo::new(settings, &user.username, enterprise_settings).into()), token: Some(token.token), }; debug!("{response:?}."); diff --git a/src/grpc/mod.rs b/src/grpc/mod.rs index ef63907146..55429ccd26 100644 --- a/src/grpc/mod.rs +++ b/src/grpc/mod.rs @@ -45,7 +45,11 @@ use self::{ use crate::{ auth::failed_login::FailedLoginMap, db::{AppEvent, Settings}, - enterprise::grpc::polling::PollingServer, + enterprise::{ + db::models::enterprise_settings::EnterpriseSettings, + grpc::polling::PollingServer, + license::{get_cached_license, validate_license}, + }, handlers::mail::send_gateway_disconnected_email, mail::Mail, server_config, @@ -522,7 +526,9 @@ pub async fn run_grpc_bidi_stream( Some(core_response::Payload::InstanceInfo(response_payload)) } Err(err) => { - error!("Instance info error {err}"); + // The error should already be logged in the info method, hence + // only debug here. + debug!("Instance info error {err}"); Some(core_response::Payload::CoreError(err.into())) } } @@ -649,10 +655,16 @@ pub struct InstanceInfo { url: Url, proxy_url: Url, username: String, + disable_all_traffic: bool, + enterprise_enabled: bool, } impl InstanceInfo { - pub fn new>(settings: Settings, username: S) -> Self { + pub fn new>( + settings: Settings, + username: S, + enterprise_settings: EnterpriseSettings, + ) -> Self { let config = server_config(); InstanceInfo { id: settings.uuid, @@ -660,6 +672,8 @@ impl InstanceInfo { url: config.url.clone(), proxy_url: config.enrollment_url.clone(), username: username.into(), + disable_all_traffic: enterprise_settings.disable_all_traffic, + enterprise_enabled: validate_license(get_cached_license().as_ref()).is_ok(), } } } @@ -672,6 +686,8 @@ impl From for crate::grpc::proto::InstanceInfo { url: instance.url.to_string(), proxy_url: instance.proxy_url.to_string(), username: instance.username, + disable_all_traffic: instance.disable_all_traffic, + enterprise_enabled: instance.enterprise_enabled, } } } diff --git a/src/grpc/utils.rs b/src/grpc/utils.rs index 38bd8a1ed6..87cb177cab 100644 --- a/src/grpc/utils.rs +++ b/src/grpc/utils.rs @@ -2,7 +2,7 @@ use ipnetwork::IpNetwork; use tonic::Status; use super::{ - proto::{DeviceConfig as ProtoDeviceConfig, DeviceConfigResponse, InstanceConfigResponse}, + proto::{DeviceConfig as ProtoDeviceConfig, DeviceConfigResponse}, InstanceInfo, }; use crate::{ @@ -10,10 +10,7 @@ use crate::{ models::{device::WireguardNetworkDevice, wireguard::WireguardNetwork}, DbPool, Device, Settings, User, }, - enterprise::{ - db::models::enterprise_settings::EnterpriseSettings, - license::{get_cached_license, validate_license}, - }, + enterprise::db::models::enterprise_settings::EnterpriseSettings, }; pub(crate) async fn build_device_config_response( @@ -39,6 +36,11 @@ pub(crate) async fn build_device_config_response( Status::internal(format!("unexpected error: {err}")) })?; + let enterprise_settings = EnterpriseSettings::get(pool).await.map_err(|err| { + error!("Failed to get enterprise settings: {err}"); + Status::internal(format!("unexpected error: {err}")) + })?; + let mut configs: Vec = Vec::new(); let Some(device) = device else { return Err(Status::internal("device not found error")); @@ -97,24 +99,7 @@ pub(crate) async fn build_device_config_response( Ok(DeviceConfigResponse { device: Some(device.into()), configs, - instance: Some(InstanceInfo::new(settings, &user.username).into()), + instance: Some(InstanceInfo::new(settings, &user.username, enterprise_settings).into()), token: None, }) } - -pub(crate) async fn build_instance_config_response( - pool: &DbPool, -) -> Result { - debug!("Building instance config response"); - let enterprise = validate_license(get_cached_license().as_ref()).is_ok(); - let enterprise_settings = EnterpriseSettings::get(pool).await.map_err(|err| { - error!("Failed to get enterprise settings while building instance config response: {err}"); - Status::internal("unexpected error") - })?; - debug!("Instance config response built"); - - Ok(InstanceConfigResponse { - enterprise, - disable_all_traffic: enterprise_settings.disable_all_traffic, - }) -} From 6c9a88067fe0031ecd413896c12774982484d099 Mon Sep 17 00:00:00 2001 From: Jacek Chmielewski Date: Thu, 12 Sep 2024 18:45:07 +0200 Subject: [PATCH 2/5] Log errors --- src/grpc/enrollment.rs | 44 +++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/grpc/enrollment.rs b/src/grpc/enrollment.rs index 2072fb58da..c6f4a1872c 100644 --- a/src/grpc/enrollment.rs +++ b/src/grpc/enrollment.rs @@ -131,8 +131,8 @@ impl EnrollmentServer { user.username, user.id ); - let mut transaction = self.pool.begin().await.map_err(|_| { - error!("Failed to begin a transaction for enrollment."); + let mut transaction = self.pool.begin().await.map_err(|err| { + error!("Failed to begin a transaction for enrollment: {err}"); Status::internal("unexpected error") })?; @@ -158,8 +158,8 @@ impl EnrollmentServer { ); let settings = Settings::get_settings(&mut *transaction) .await - .map_err(|_| { - error!("Failed to get settings."); + .map_err(|err| { + error!("Failed to get settings: {err}"); Status::internal("unexpected error") })?; debug!("Settings: {settings:?}"); @@ -171,8 +171,8 @@ impl EnrollmentServer { let enterprise_settings = EnterpriseSettings::get(&mut *transaction) .await - .map_err(|_| { - error!("Failed to get enterprise settings."); + .map_err(|err| { + error!("Failed to get enterprise settings: {err}"); Status::internal("unexpected error") })?; debug!("Enterprise settings: {enterprise_settings:?}"); @@ -192,9 +192,9 @@ impl EnrollmentServer { let (username, user_id) = (user.username.clone(), user.id); let user_info = InitialUserInfo::from_user(&self.pool, user) .await - .map_err(|_| { + .map_err(|err| { error!( - "Failed to get user info for user {}({:?})", + "Failed to get user info for user {}({:?}): {err}", username, user_id, ); Status::internal("unexpected error") @@ -212,8 +212,8 @@ impl EnrollmentServer { let enterprise_settings = EnterpriseSettings::get(&mut *transaction) .await - .map_err(|_| { - error!("Failed to get enterprise settings"); + .map_err(|err| { + error!("Failed to get enterprise settings: {err}"); Status::internal("unexpected error") })?; let enrollment_settings = super::proto::Settings { @@ -232,8 +232,8 @@ impl EnrollmentServer { }; debug!("Response {response:?}"); - transaction.commit().await.map_err(|_| { - error!("Failed to commit transaction"); + transaction.commit().await.map_err(|err| { + error!("Failed to commit transaction: {err}"); Status::internal("unexpected error") })?; @@ -294,8 +294,8 @@ impl EnrollmentServer { } debug!("User is active."); - let mut transaction = self.pool.begin().await.map_err(|_| { - error!("Failed to begin transaction"); + let mut transaction = self.pool.begin().await.map_err(|err| { + error!("Failed to begin transaction: {err}"); Status::internal("unexpected error") })?; @@ -319,8 +319,8 @@ impl EnrollmentServer { debug!("Retriving settings to send welcome email..."); let settings = Settings::get_settings(&mut *transaction) .await - .map_err(|_| { - error!("Failed to get settings"); + .map_err(|err| { + error!("Failed to get settings: {err}"); Status::internal("unexpected error") })?; debug!("Successfully retrived settings."); @@ -355,8 +355,8 @@ impl EnrollmentServer { )?; } - transaction.commit().await.map_err(|_| { - error!("Failed to commit transaction"); + transaction.commit().await.map_err(|err| { + error!("Failed to commit transaction: {err}"); Status::internal("unexpected error") })?; @@ -429,8 +429,8 @@ impl EnrollmentServer { ); if let Some(device) = Device::find_by_pubkey(&self.pool, &request.pubkey) .await - .map_err(|_| { - error!("Failed to get device by its pubkey: {}", request.pubkey); + .map_err(|err| { + error!("Failed to get device {} by its pubkey: {err}", request.pubkey); Status::internal("unexpected error") })? { @@ -455,8 +455,8 @@ impl EnrollmentServer { user.username, user.id, ); - let mut transaction = self.pool.begin().await.map_err(|_| { - error!("Failed to begin transaction"); + let mut transaction = self.pool.begin().await.map_err(|err| { + error!("Failed to begin transaction: {err}"); Status::internal("unexpected error") })?; device.save(&mut *transaction).await.map_err(|err| { From 8534b8f709e31ea5cec1753f7516ef55611fb3af Mon Sep 17 00:00:00 2001 From: Aleksander <170264518+t-aleksander@users.noreply.github.com> Date: Fri, 13 Sep 2024 09:31:13 +0200 Subject: [PATCH 3/5] debug -> error --- src/grpc/mod.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/grpc/mod.rs b/src/grpc/mod.rs index 55429ccd26..95a4d0fd6b 100644 --- a/src/grpc/mod.rs +++ b/src/grpc/mod.rs @@ -526,9 +526,7 @@ pub async fn run_grpc_bidi_stream( Some(core_response::Payload::InstanceInfo(response_payload)) } Err(err) => { - // The error should already be logged in the info method, hence - // only debug here. - debug!("Instance info error {err}"); + error!("Instance info error {err}"); Some(core_response::Payload::CoreError(err.into())) } } From 712ced1895aac4b3c04c675399f2e17cba7e8928 Mon Sep 17 00:00:00 2001 From: Aleksander <170264518+t-aleksander@users.noreply.github.com> Date: Fri, 13 Sep 2024 09:35:15 +0200 Subject: [PATCH 4/5] update protobufs --- proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proto b/proto index de58067ab6..8309982b94 160000 --- a/proto +++ b/proto @@ -1 +1 @@ -Subproject commit de58067ab652f6e5ddf84c7bbc6e4d2363914738 +Subproject commit 8309982b94e82a7cbe39dd529967f43e49b3ef1d From d70e4c7ca324020a9d95b162d89bfad0d516b422 Mon Sep 17 00:00:00 2001 From: Aleksander <170264518+t-aleksander@users.noreply.github.com> Date: Fri, 13 Sep 2024 09:52:23 +0200 Subject: [PATCH 5/5] cargo fmt --- src/grpc/enrollment.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/grpc/enrollment.rs b/src/grpc/enrollment.rs index c6f4a1872c..656ca07d2b 100644 --- a/src/grpc/enrollment.rs +++ b/src/grpc/enrollment.rs @@ -430,7 +430,10 @@ impl EnrollmentServer { if let Some(device) = Device::find_by_pubkey(&self.pool, &request.pubkey) .await .map_err(|err| { - error!("Failed to get device {} by its pubkey: {err}", request.pubkey); + error!( + "Failed to get device {} by its pubkey: {err}", + request.pubkey + ); Status::internal("unexpected error") })? {