Apply suggestions from code review

Co-authored-by: Aleksander <[email protected]>

Author: cpprian

src/db/models/enrollment.rs

@@ -247,7 +247,7 @@ impl Token {
         }
 
         let admin_id = self.admin_id.unwrap();
-        debug!("Try to find admin using id {}", admin_id);
+        debug!("Trying to find admin using id {admin_id}");
         let user = User::find_by_id(executor, admin_id).await?;
         debug!("Fetched admin {user:?}.");
         Ok(user)
@@ -392,16 +392,16 @@ impl User {
         mail_tx: UnboundedSender<Mail>,
     ) -> Result<String, TokenError> {
         info!(
-            "Start generating a new enrollment process for user {}.",
-            self.username
+            "User {} started a new enrollment process for user {}.",
+            admin.username, self.username
         );
         debug!(
             "Notify user by mail about the enrollment process: {}",
             send_user_notification
         );
-        debug!("Check is {} has a password.", self.username);
+        debug!("Check if {} has a password.", self.username);
         if self.has_password() {
-            debug!("User that you want to start enrollment process has already password.");
+            debug!("User {} that you want to start enrollment process for already has a password.", self.username);
             return Err(TokenError::AlreadyActive);
         }
 
@@ -497,7 +497,7 @@ impl User {
         send_user_notification: bool,
         mail_tx: UnboundedSender<Mail>,
     ) -> Result<String, TokenError> {
-        info!("Start a new desktop activation for user {}", self.username);
+        info!("User {} starting a new desktop activation for user {}", admin.username, self.username);
         debug!(
             "Notify {} by mail about the enrollment process: {}",
             self.username, send_user_notification

src/grpc/enrollment.rs

@@ -136,7 +136,7 @@ impl EnrollmentServer {
             let user = enrollment.fetch_user(&self.pool).await?;
             let admin = enrollment.fetch_admin(&self.pool).await?;
 
-            debug!("Check is {} an active user.", user.username);
+            debug!("Check if {} is an active user.", user.username);
             if !user.is_active {
                 warn!(
                     "Can't start enrollment for disabled user {}.",
@@ -210,7 +210,7 @@ impl EnrollmentServer {
 
             Ok(response)
         } else {
-            debug!("Invalid an enrollment token, the token does not have specified type.");
+            debug!("Invalid enrollment token, the token does not have specified type.");
             Err(Status::permission_denied("invalid token"))
         }
     }
@@ -236,26 +236,26 @@ impl EnrollmentServer {
         debug!("Ip address {}, device info {device_info:?}", ip_address);
 
         // check if password is strong enough
-        debug!("Verify is password strong enough to complete the activating user process.");
+        debug!("Verify if password is strong enough to complete the user activation process.");
         if let Err(err) = check_password_strength(&request.password) {
             error!("Password not strong enough: {err}");
             return Err(Status::invalid_argument("password not strong enough"));
         }
-        debug!("Password is strong enough to complete the activating user process.");
+        debug!("Password is strong enough to complete the user activation process.");
 
         // fetch related users
         let mut user = enrollment.fetch_user(&self.pool).await?;
         debug!(
-            "Fetching user {} data to check is the user has already password.",
+            "Fetching user {} data to check if the user already has a password.",
             user.username
         );
         if user.has_password() {
             error!("User {} already activated", user.username);
             return Err(Status::invalid_argument("user already activated"));
         }
-        debug!("User doesn't have a password yet. Continue activating user process...");
+        debug!("User doesn't have a password yet. Continue user activation process...");
 
-        debug!("Verify is the user active or disabled.");
+        debug!("Verify if the user is active or disabled.");
         if !user.is_active {
             warn!(
                 "Can't finalize enrollment for disabled user {}",
@@ -278,7 +278,7 @@ impl EnrollmentServer {
             error!("Failed to update user {}: {err}", user.username);
             Status::internal("unexpected error")
         })?;
-        debug!("Updating user details ends with success.");
+        debug!("Updating user details ended with success.");
 
         // sync with LDAP
         debug!("Add user to ldap: {}.", self.ldap_feature_active);
@@ -294,7 +294,7 @@ impl EnrollmentServer {
                 error!("Failed to get settings");
                 Status::internal("unexpected error")
             })?;
-        debug!("Successfully retrive settings.");
+        debug!("Successfully retrived settings.");
 
         // send welcome email
         debug!("Try to send welcome email...");
@@ -311,7 +311,7 @@ impl EnrollmentServer {
 
         // send success notification to admin
         debug!(
-            "Trying fetch admin data from the token to send notification about activating user."
+            "Trying to fetch admin data from the token to send notification about activating user."
         );
         let admin = enrollment.fetch_admin(&mut *transaction).await?;
 
@@ -348,7 +348,7 @@ impl EnrollmentServer {
         let user = enrollment.fetch_user(&self.pool).await?;
 
         // add device
-        debug!("Verifying is user active or disabled.");
+        debug!("Verifying if user is active or disabled.");
         if !user.is_active {
             error!("Can't create device for a disabled user {}", user.username);
             return Err(Status::invalid_argument(
@@ -368,15 +368,15 @@ impl EnrollmentServer {
         }
         debug!("Ip address {}, device info {device_info:?}", ip_address);
 
-        debug!("Start validating pubkey for create device process...");
+        debug!("Start validating pubkey for device creation process...");
         Device::validate_pubkey(&request.pubkey).map_err(|_| {
             error!("Invalid pubkey {}", request.pubkey);
             Status::invalid_argument("invalid pubkey")
         })?;
         debug!("Pubkey is validated.");
 
         // Make sure there is no device with the same pubkey, such state may lead to unexpected issues
-        debug!("Check is there a device that has the same pubey.");
+        debug!("Check if there is a device that has the same pubey.");
         if let Some(device) = Device::find_by_pubkey(&self.pool, &request.pubkey)
             .await
             .map_err(|_| {

src/grpc/mod.rs

@@ -388,7 +388,8 @@ pub async fn run_grpc_bidi_stream(
                     break 'message;
                 }
                 Ok(Some(received)) => {
-                    info!("Received message from proxy {received:?}");
+                    info!("Received message from proxy.");
+                    debug!("Received the following message from proxy: {received:?}");
                     let payload = match received.payload {
                         // rpc StartEnrollment (EnrollmentStartRequest) returns (EnrollmentStartResponse)
                         Some(core_request::Payload::EnrollmentStart(request)) => {

src/handlers/mod.rs

@@ -291,7 +291,7 @@ pub async fn user_for_admin_or_self(
     username: &str,
 ) -> Result<User, WebError> {
     if session.user.username == username || session.is_admin {
-        debug!("The user meets one or both of these conditions: 1) the user from the current session has admin privileges, 2) the user only wants for self to perform this operation.");
+        debug!("The user meets one or both of these conditions: 1) the user from the current session has admin privileges, 2) the user performs this operation on themself.");
         match User::find_by_username(pool, username).await? {
             Some(user) => {
                 debug!("User {} has been found in database.", user.username);
@@ -305,7 +305,7 @@ pub async fn user_for_admin_or_self(
             }
         }
     } else {
-        debug!("User from the current session don't have enough privileges to do this operation.");
+        debug!("User from the current session doesn't have enough privileges to do this operation.");
         Err(WebError::Forbidden("requires privileged access".into()))
     }
 }

src/handlers/user.rs

@@ -396,9 +396,9 @@ pub async fn start_enrollment(
         )
         .await?;
 
-    debug!("Try to submit transaction to save the enrollment token into the databse.");
+    debug!("Try to commit transaction to save the enrollment token into the databse.");
     transaction.commit().await?;
-    debug!("Transaction submitted.");
+    debug!("Transaction committed.");
 
     info!(
         "The enrollment process for {} has ended with success.",
@@ -446,7 +446,7 @@ pub async fn start_remote_desktop_configuration(
     Json(data): Json<StartEnrollmentRequest>,
 ) -> ApiResult {
     debug!(
-        "User {} has started a new desktop activation request.",
+        "User {} has started a new desktop activation for {username}.",
         session.user.username
     );
 
@@ -485,7 +485,7 @@ pub async fn start_remote_desktop_configuration(
     debug!("Transaction submitted.");
 
     info!(
-        "User {} added a new desktop activation.",
+        "User {} started a new desktop activation.",
         session.user.username
     );
     debug!(
@@ -620,7 +620,7 @@ pub async fn modify_user(
                 .await?
         {
             debug!(
-                "User {} has started a new enrollment request.",
+                "User {} changed {username} groups or status, syncing allowed network devices.",
                 session.user.username
             );
             let networks = WireguardNetwork::all(&mut *transaction).await?;

src/templates.rs

@@ -90,7 +90,7 @@ pub fn enrollment_start_mail(
     mut enrollment_service_url: Url,
     enrollment_token: &str,
 ) -> Result<String, TemplateError> {
-    debug!("Render a enrollment start mail template for the user.");
+    debug!("Render an enrollment start mail template for the user.");
     let (mut tera, mut context) = get_base_tera(Some(context), None, None, None)?;
 
     // add required context