Improve protected actions (#6411)

* Improve protected actions

* Match usage on two factor

* Use saturating add

* Don't delete token when tracking attempts

src/api/core/two_factor/email.rs

@@ -286,7 +286,7 @@ impl EmailTokenData {
     }
 
     pub fn add_attempt(&mut self) {
-        self.attempts += 1;
+        self.attempts = self.attempts.saturating_add(1);
     }
 
     pub fn to_json(&self) -> String {

src/api/core/two_factor/protected_actions.rs

@@ -1,4 +1,4 @@
-use chrono::{DateTime, TimeDelta, Utc};
+use chrono::{naive::serde::ts_seconds, NaiveDateTime, TimeDelta, Utc};
 use rocket::{serde::json::Json, Route};
 
 use crate::{
@@ -23,16 +23,17 @@ pub struct ProtectedActionData {
     /// Token issued to validate the protected action
     pub token: String,
     /// UNIX timestamp of token issue.
-    pub token_sent: i64,
+    #[serde(with = "ts_seconds")]
+    pub token_sent: NaiveDateTime,
     // The total amount of attempts
-    pub attempts: u8,
+    pub attempts: u64,
 }
 
 impl ProtectedActionData {
     pub fn new(token: String) -> Self {
         Self {
             token,
-            token_sent: Utc::now().timestamp(),
+            token_sent: Utc::now().naive_utc(),
             attempts: 0,
         }
     }
@@ -50,7 +51,11 @@ impl ProtectedActionData {
     }
 
     pub fn add_attempt(&mut self) {
-        self.attempts += 1;
+        self.attempts = self.attempts.saturating_add(1);
+    }
+
+    pub fn time_since_sent(&self) -> TimeDelta {
+        Utc::now().naive_utc() - self.token_sent
     }
 }
 
@@ -65,6 +70,13 @@ async fn request_otp(headers: Headers, conn: DbConn) -> EmptyResult {
     // Only one Protected Action per user is allowed to take place, delete the previous one
     if let Some(pa) = TwoFactor::find_by_user_and_type(&user.uuid, TwoFactorType::ProtectedActions as i32, &conn).await
     {
+        let pa_data = ProtectedActionData::from_json(&pa.data)?;
+        let elapsed = pa_data.time_since_sent().num_seconds();
+        let delay = 30;
+        if elapsed < delay {
+            err!(format!("Please wait {} seconds before requesting another code.", (delay - elapsed)));
+        }
+
         pa.delete(&conn).await?;
     }
 
@@ -107,24 +119,22 @@ pub async fn validate_protected_action_otp(
     delete_if_valid: bool,
     conn: &DbConn,
 ) -> EmptyResult {
-    let pa = TwoFactor::find_by_user_and_type(user_id, TwoFactorType::ProtectedActions as i32, conn)
+    let mut pa = TwoFactor::find_by_user_and_type(user_id, TwoFactorType::ProtectedActions as i32, conn)
         .await
         .map_res("Protected action token not found, try sending the code again or restart the process")?;
     let mut pa_data = ProtectedActionData::from_json(&pa.data)?;
-
     pa_data.add_attempt();
-    // Delete the token after x attempts if it has been used too many times
-    // We use the 6, which should be more then enough for invalid attempts and multiple valid checks
-    if pa_data.attempts > 6 {
-        pa.delete(conn).await?;
+    pa.data = pa_data.to_json();
+
+    // Fail after x attempts if the token has been used too many times.
+    // Don't delete it, as we use it to keep track of attempts.
+    if pa_data.attempts >= CONFIG.email_attempts_limit() {
         err!("Token has expired")
     }
 
     // Check if the token has expired (Using the email 2fa expiration time)
-    let date =
-        DateTime::from_timestamp(pa_data.token_sent, 0).expect("Protected Action token timestamp invalid.").naive_utc();
     let max_time = CONFIG.email_expiration_time() as i64;
-    if date + TimeDelta::try_seconds(max_time).unwrap() < Utc::now().naive_utc() {
+    if pa_data.time_since_sent().num_seconds() > max_time {
         pa.delete(conn).await?;
         err!("Token has expired")
     }