Cleanups and Fixes for Emergency Access

- Several cleanups and code optimizations for Emergency Access
- Fixed a race-condition regarding jobs for Emergency Access
- Some other small changes like `allow(clippy::)` removals

Fixes dani-garcia#2925

.env.template

@@ -102,12 +102,12 @@
 # INCOMPLETE_2FA_SCHEDULE="30 * * * * *"
 ##
 ## Cron schedule of the job that sends expiration reminders to emergency access grantors.
-## Defaults to hourly (5 minutes after the hour). Set blank to disable this job.
-# EMERGENCY_NOTIFICATION_REMINDER_SCHEDULE="0 5 * * * *"
+## Defaults to hourly (3 minutes after the hour). Set blank to disable this job.
+# EMERGENCY_NOTIFICATION_REMINDER_SCHEDULE="0 3 * * * *"
 ##
 ## Cron schedule of the job that grants emergency access requests that have met the required wait time.
-## Defaults to hourly (5 minutes after the hour). Set blank to disable this job.
-# EMERGENCY_REQUEST_TIMEOUT_SCHEDULE="0 5 * * * *"
+## Defaults to hourly (7 minutes after the hour). Set blank to disable this job.
+# EMERGENCY_REQUEST_TIMEOUT_SCHEDULE="0 7 * * * *"
 
 ## Enable extended logging, which shows timestamps and targets in the logs
 # EXTENDED_LOGGING=true

src/api/admin.rs

@@ -289,7 +289,7 @@ async fn invite_user(data: Json<InviteData>, _token: AdminToken, mut conn: DbCon
         if CONFIG.mail_enabled() {
             mail::send_invite(&user.email, &user.uuid, None, None, &CONFIG.invitation_org_name(), None).await
         } else {
-            let invitation = Invitation::new(user.email.clone());
+            let invitation = Invitation::new(&user.email);
             invitation.save(conn).await
         }
     }

src/api/core/organizations.rs

@@ -658,7 +658,7 @@ async fn send_invite(
                 }
 
                 if !CONFIG.mail_enabled() {
-                    let invitation = Invitation::new(email.clone());
+                    let invitation = Invitation::new(&email);
                     invitation.save(&mut conn).await?;
                 }
 
@@ -797,7 +797,7 @@ async fn _reinvite_user(org_id: &str, user_org: &str, invited_by_email: &str, co
         )
         .await?;
     } else {
-        let invitation = Invitation::new(user.email);
+        let invitation = Invitation::new(&user.email);
         invitation.save(conn).await?;
     }
 

src/api/icons.rs

@@ -260,7 +260,6 @@ mod tests {
 
 use cached::proc_macro::cached;
 #[cached(key = "String", convert = r#"{ domain.to_string() }"#, size = 16, time = 60)]
-#[allow(clippy::unused_async)] // This is needed because cached causes a false-positive here.
 async fn is_domain_blacklisted(domain: &str) -> bool {
     // First check the blacklist regex if there is a match.
     // This prevents the blocked domain(s) from being leaked via a DNS lookup.

src/auth.rs

@@ -177,17 +177,17 @@ pub struct EmergencyAccessInviteJwtClaims {
     pub sub: String,
 
     pub email: String,
-    pub emer_id: Option<String>,
-    pub grantor_name: Option<String>,
-    pub grantor_email: Option<String>,
+    pub emer_id: String,
+    pub grantor_name: String,
+    pub grantor_email: String,
 }
 
 pub fn generate_emergency_access_invite_claims(
     uuid: String,
     email: String,
-    emer_id: Option<String>,
-    grantor_name: Option<String>,
-    grantor_email: Option<String>,
+    emer_id: String,
+    grantor_name: String,
+    grantor_email: String,
 ) -> EmergencyAccessInviteJwtClaims {
     let time_now = Utc::now().naive_utc();
     let expire_hours = i64::from(CONFIG.invitation_expiration_hours());

src/config.rs

@@ -366,11 +366,11 @@ make_config! {
         /// Defaults to once every minute. Set blank to disable this job.
         incomplete_2fa_schedule: String, false,  def,   "30 * * * * *".to_string();
         /// Emergency notification reminder schedule |> Cron schedule of the job that sends expiration reminders to emergency access grantors.
-        /// Defaults to hourly. Set blank to disable this job.
-        emergency_notification_reminder_schedule:   String, false,  def,    "0 5 * * * *".to_string();
+        /// Defaults to hourly. (3 minutes after the hour) Set blank to disable this job.
+        emergency_notification_reminder_schedule:   String, false,  def,    "0 3 * * * *".to_string();
         /// Emergency request timeout schedule |> Cron schedule of the job that grants emergency access requests that have met the required wait time.
-        /// Defaults to hourly. Set blank to disable this job.
-        emergency_request_timeout_schedule:   String, false,  def,    "0 5 * * * *".to_string();
+        /// Defaults to hourly. (7 minutes after the hour) Set blank to disable this job.
+        emergency_request_timeout_schedule:   String, false,  def,    "0 7 * * * *".to_string();
     },
 
     /// General settings

src/db/mod.rs

@@ -125,7 +125,6 @@ macro_rules! generate_connections {
 
         impl DbPool {
             // For the given database URL, guess its type, run migrations, create pool, and return it
-            #[allow(clippy::diverging_sub_expression)]
             pub fn from_config() -> Result<Self, Error> {
                 let url = CONFIG.database_url();
                 let conn_type = DbConnType::from_url(&url)?;

src/db/models/emergency_access.rs

@@ -1,10 +1,12 @@
 use chrono::{NaiveDateTime, Utc};
 use serde_json::Value;
 
+use crate::{api::EmptyResult, db::DbConn, error::MapResult};
+
 use super::User;
 
 db_object! {
-    #[derive(Debug, Identifiable, Queryable, Insertable, AsChangeset)]
+    #[derive(Identifiable, Queryable, Insertable, AsChangeset)]
     #[diesel(table_name = emergency_access)]
     #[diesel(treat_none_as_null = true)]
     #[diesel(primary_key(uuid))]
@@ -27,14 +29,14 @@ db_object! {
 /// Local methods
 
 impl EmergencyAccess {
-    pub fn new(grantor_uuid: String, email: Option<String>, status: i32, atype: i32, wait_time_days: i32) -> Self {
+    pub fn new(grantor_uuid: String, email: String, status: i32, atype: i32, wait_time_days: i32) -> Self {
         let now = Utc::now().naive_utc();
 
         Self {
             uuid: crate::util::get_uuid(),
             grantor_uuid,
             grantee_uuid: None,
-            email,
+            email: Some(email),
             status,
             atype,
             wait_time_days,
@@ -54,14 +56,6 @@ impl EmergencyAccess {
         }
     }
 
-    pub fn has_type(&self, access_type: EmergencyAccessType) -> bool {
-        self.atype == access_type as i32
-    }
-
-    pub fn has_status(&self, status: EmergencyAccessStatus) -> bool {
-        self.status == status as i32
-    }
-
     pub fn to_json(&self) -> Value {
         json!({
             "Id": self.uuid,
@@ -87,7 +81,6 @@ impl EmergencyAccess {
         })
     }
 
-    #[allow(clippy::manual_map)]
     pub async fn to_json_grantee_details(&self, conn: &mut DbConn) -> Value {
         let grantee_user = if let Some(grantee_uuid) = self.grantee_uuid.as_deref() {
             Some(User::find_by_uuid(grantee_uuid, conn).await.expect("Grantee user not found."))
@@ -110,7 +103,7 @@ impl EmergencyAccess {
     }
 }
 
-#[derive(Copy, Clone, PartialEq, Eq, num_derive::FromPrimitive)]
+#[derive(Copy, Clone)]
 pub enum EmergencyAccessType {
     View = 0,
     Takeover = 1,
@@ -126,18 +119,6 @@ impl EmergencyAccessType {
     }
 }
 
-impl PartialEq<i32> for EmergencyAccessType {
-    fn eq(&self, other: &i32) -> bool {
-        *other == *self as i32
-    }
-}
-
-impl PartialEq<EmergencyAccessType> for i32 {
-    fn eq(&self, other: &EmergencyAccessType) -> bool {
-        *self == *other as i32
-    }
-}
-
 pub enum EmergencyAccessStatus {
     Invited = 0,
     Accepted = 1,
@@ -148,11 +129,6 @@ pub enum EmergencyAccessStatus {
 
 // region Database methods
 
-use crate::db::DbConn;
-
-use crate::api::EmptyResult;
-use crate::error::MapResult;
-
 impl EmergencyAccess {
     pub async fn save(&mut self, conn: &mut DbConn) -> EmptyResult {
         User::update_uuid_revision(&self.grantor_uuid, conn).await;
@@ -189,6 +165,45 @@ impl EmergencyAccess {
         }
     }
 
+    pub async fn update_access_status_and_save(
+        &mut self,
+        status: i32,
+        date: &NaiveDateTime,
+        conn: &mut DbConn,
+    ) -> EmptyResult {
+        // Update the grantee so that it will refresh it's status.
+        User::update_uuid_revision(self.grantee_uuid.as_ref().expect("Error getting grantee"), conn).await;
+        self.status = status;
+        self.updated_at = date.to_owned();
+
+        db_run! {conn: {
+            crate::util::retry(|| {
+                diesel::update(emergency_access::table.filter(emergency_access::uuid.eq(&self.uuid)))
+                    .set((emergency_access::status.eq(status), emergency_access::updated_at.eq(date)))
+                    .execute(conn)
+            }, 10)
+            .map_res("Error updating emergency access status")
+        }}
+    }
+
+    pub async fn update_last_notification_date_and_save(
+        &mut self,
+        date: &NaiveDateTime,
+        conn: &mut DbConn,
+    ) -> EmptyResult {
+        self.last_notification_at = Some(date.to_owned());
+        self.updated_at = date.to_owned();
+
+        db_run! {conn: {
+            crate::util::retry(|| {
+                diesel::update(emergency_access::table.filter(emergency_access::uuid.eq(&self.uuid)))
+                    .set((emergency_access::last_notification_at.eq(date), emergency_access::updated_at.eq(date)))
+                    .execute(conn)
+            }, 10)
+            .map_res("Error updating emergency access status")
+        }}
+    }
+
     pub async fn delete_all_by_user(user_uuid: &str, conn: &mut DbConn) -> EmptyResult {
         for ea in Self::find_all_by_grantor_uuid(user_uuid, conn).await {
             ea.delete(conn).await?;
@@ -233,10 +248,11 @@ impl EmergencyAccess {
         }}
     }
 
-    pub async fn find_all_recoveries(conn: &mut DbConn) -> Vec<Self> {
+    pub async fn find_all_recoveries_initiated(conn: &mut DbConn) -> Vec<Self> {
         db_run! { conn: {
             emergency_access::table
                 .filter(emergency_access::status.eq(EmergencyAccessStatus::RecoveryInitiated as i32))
+                .filter(emergency_access::recovery_initiated_at.is_not_null())
                 .load::<EmergencyAccessDb>(conn).expect("Error loading emergency_access").from_db()
         }}
     }

src/db/models/user.rs

@@ -364,7 +364,7 @@ impl User {
 }
 
 impl Invitation {
-    pub fn new(email: String) -> Self {
+    pub fn new(email: &str) -> Self {
         let email = email.to_lowercase();
         Self {
             email,

src/error.rs

@@ -154,7 +154,6 @@ impl<S> MapResult<S> for Option<S> {
     }
 }
 
-#[allow(clippy::unnecessary_wraps)]
 const fn _has_source<T>(e: T) -> Option<T> {
     Some(e)
 }

src/mail.rs

@@ -256,16 +256,16 @@ pub async fn send_invite(
 pub async fn send_emergency_access_invite(
     address: &str,
     uuid: &str,
-    emer_id: Option<String>,
-    grantor_name: Option<String>,
-    grantor_email: Option<String>,
+    emer_id: &str,
+    grantor_name: &str,
+    grantor_email: &str,
 ) -> EmptyResult {
     let claims = generate_emergency_access_invite_claims(
-        uuid.to_string(),
+        String::from(uuid),
         String::from(address),
-        emer_id.clone(),
-        grantor_name.clone(),
-        grantor_email,
+        String::from(emer_id),
+        String::from(grantor_name),
+        String::from(grantor_email),
     );
 
     let invite_token = encode_jwt(&claims);
@@ -275,7 +275,7 @@ pub async fn send_emergency_access_invite(
         json!({
             "url": CONFIG.domain(),
             "img_src": CONFIG._smtp_img_src(),
-            "emer_id": emer_id.unwrap_or_else(|| "_".to_string()),
+            "emer_id": emer_id,
             "email": percent_encode(address.as_bytes(), NON_ALPHANUMERIC).to_string(),
             "grantor_name": grantor_name,
             "token": invite_token,
@@ -328,7 +328,7 @@ pub async fn send_emergency_access_recovery_initiated(
     address: &str,
     grantee_name: &str,
     atype: &str,
-    wait_time_days: &str,
+    wait_time_days: &i32,
 ) -> EmptyResult {
     let (subject, body_html, body_text) = get_text(
         "email/emergency_access_recovery_initiated",