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>

diff --git a/proxmox-tfa/src/api/methods.rs b/proxmox-tfa/src/api/methods.rs
index b981644f6a16881f3fe5d7b6336113184416b981..86781908e5034826a76626e09b047e6e9a4fa89b 100644 (file)
--- a/proxmox-tfa/src/api/methods.rs
+++ b/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(

diff --git a/proxmox-tfa/src/api/mod.rs b/proxmox-tfa/src/api/mod.rs
index fe0dd6d0c4757ba81bd72d16e956baabcad86b58..f7aeea5aafeaf7c81b294592ee6e20acedc61222 100644 (file)
--- a/proxmox-tfa/src/api/mod.rs
+++ b/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;