tfa: also reset counters when unlocking tfa

Since this requires access to the user data, we need to add
a generic parameter to the unlock methods.
To avoid having to create another major API bump affecting
all our products this short after release, we keep the old
version around with the old behavior.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>

proxmox-tfa/src/api/methods.rs

@@ -190,8 +190,30 @@ pub fn delete_tfa(config: &mut TfaConfig, userid: &str, id: &str) -> Result<bool
 /// Errors only if the user was not found.
 ///
 /// Returns `true` if the user was previously locked out, `false` if nothing was changed.
+#[deprecated(note = "use unlock_and_tfa instead")]
 pub fn unlock_tfa(config: &mut TfaConfig, userid: &str) -> Result<bool, Error> {
-    config.unlock_tfa(userid)
+    config.unlock_and_reset_tfa(&super::NoUserData, userid)
+}
+
+/// API call implementation for `PUT /users/{userid}/unlock-tfa`.
+///
+/// This should only be allowed for user administrators.
+///
+/// The TFA config must be WRITE locked.
+///
+/// The caller must *save* the config if `true` is returned!
+///
+/// Errors only if the user was not found.
+///
+/// This also resets the failure counts for the user.
+///
+/// Returns `true` if the user was previously locked out, `false` if nothing was changed.
+pub fn unlock_and_reset_tfa<A: ?Sized + OpenUserChallengeData>(
+    config: &mut TfaConfig,
+    access: &A,
+    userid: &str,
+) -> Result<bool, Error> {
+    config.unlock_and_reset_tfa(access, userid)
 }
 
 #[cfg_attr(feature = "api-types", api(

proxmox-tfa/src/api/mod.rs

@@ -75,6 +75,23 @@ pub trait OpenUserChallengeData {
     }
 }
 
+pub(self) struct NoUserData;
+
+impl OpenUserChallengeData for NoUserData {
+    fn open(&self, _userid: &str) -> Result<Box<dyn UserChallengeAccess>, Error> {
+        bail!("trying to create challenge data via incompatible API endpoint");
+    }
+
+    fn open_no_create(&self, _userid: &str) -> Result<Option<Box<dyn UserChallengeAccess>>, Error> {
+        Ok(None)
+    }
+
+    /// Should return `true` if something was removed, `false` if no data existed for the user.
+    fn remove(&self, _userid: &str) -> Result<bool, Error> {
+        Ok(false)
+    }
+}
+
 #[test]
 fn ensure_open_user_challenge_data_is_dyn_safe() {
     let _: Option<&dyn OpenUserChallengeData> = None;
@@ -145,7 +162,32 @@ fn check_webauthn<'a, 'config: 'a, 'origin: 'a>(
 impl TfaConfig {
     /// Unlock a user's 2nd factor authentication (including TOTP).
     /// Returns whether the user was locked before calling this method.
+    #[deprecated(note = "use unlock_and_reset_tfa instead")]
     pub fn unlock_tfa(&mut self, userid: &str) -> Result<bool, Error> {
+        self.unlock_and_reset_tfa(&NoUserData, userid)
+    }
+
+    /// Unlock a user's TOTP challenges.
+    #[deprecated(note = "use unlock_and_reset_totp instead")]
+    pub fn unlock_totp(&mut self, userid: &str) -> Result<(), Error> {
+        self.unlock_and_reset_totp(&NoUserData, userid)
+    }
+
+    /// Unlock a user's 2nd factor authentication (including TOTP).
+    /// Returns whether the user was locked before calling this method.
+    pub fn unlock_and_reset_tfa<A: ?Sized + OpenUserChallengeData>(
+        &mut self,
+        access: &A,
+        userid: &str,
+    ) -> Result<bool, Error> {
+        if let Some(mut data) = access.open_no_create(userid)? {
+            let access = data.get_mut();
+            if access.totp_failures != 0 || access.tfa_failures != 0 {
+                access.totp_failures = 0;
+                access.tfa_failures = 0;
+                data.save()?;
+            }
+        }
         match self.users.get_mut(userid) {
             Some(user) => {
                 let ret = user.totp_locked || user.tfa_is_locked();
@@ -158,7 +200,19 @@ impl TfaConfig {
     }
 
     /// Unlock a user's TOTP challenges.
-    pub fn unlock_totp(&mut self, userid: &str) -> Result<(), Error> {
+    pub fn unlock_and_reset_totp<A: ?Sized + OpenUserChallengeData>(
+        &mut self,
+        access: &A,
+        userid: &str,
+    ) -> Result<(), Error> {
+        if let Some(mut data) = access.open_no_create(userid)? {
+            let access = data.get_mut();
+            if access.totp_failures != 0 {
+                access.totp_failures = 0;
+                data.save()?;
+            }
+        }
+
         match self.users.get_mut(userid) {
             Some(user) => {
                 user.totp_locked = false;