diff --git a/proxmox-tfa/src/api/methods.rs b/proxmox-tfa/src/api/methods.rs index 501e7030..ebb37092 100644 --- a/proxmox-tfa/src/api/methods.rs +++ b/proxmox-tfa/src/api/methods.rs @@ -52,9 +52,9 @@ fn to_data(data: &TfaUserData) -> Vec { + data.u2f.len() + data.webauthn.len() + data.yubico.len() - + if data.recovery().is_some() { 1 } else { 0 }, + + if data.recovery.is_some() { 1 } else { 0 }, ); - if let Some(recovery) = data.recovery() { + if let Some(recovery) = &data.recovery { out.push(TypedTfaInfo { ty: TfaType::Recovery, info: TfaInfo::recovery(recovery.created), @@ -145,7 +145,7 @@ pub fn get_tfa_entry(config: &TfaConfig, userid: &str, id: &str) -> Option match user_data.recovery() { + Some((TfaType::Recovery, _)) => match &user_data.recovery { Some(recovery) => TypedTfaInfo { ty: TfaType::Recovery, info: TfaInfo::recovery(recovery.created), diff --git a/proxmox-tfa/src/api/mod.rs b/proxmox-tfa/src/api/mod.rs index 5bde7e40..6800eab1 100644 --- a/proxmox-tfa/src/api/mod.rs +++ b/proxmox-tfa/src/api/mod.rs @@ -320,7 +320,7 @@ pub struct TfaUserData { pub webauthn: Vec>, /// Recovery keys. (Unordered OTP values). - #[serde(skip_serializing_if = "Recovery::option_is_empty", default)] + #[serde(skip_serializing_if = "Option::is_none", default)] pub recovery: Option, /// Yubico keys for a user. NOTE: This is not directly supported currently, we just need this @@ -330,22 +330,13 @@ pub struct TfaUserData { } impl TfaUserData { - /// Shortcut to get the recovery entry only if it is not empty! - pub fn recovery(&self) -> Option<&Recovery> { - if Recovery::option_is_empty(&self.recovery) { - None - } else { - self.recovery.as_ref() - } - } - /// `true` if no second factors exist pub fn is_empty(&self) -> bool { self.totp.is_empty() && self.u2f.is_empty() && self.webauthn.is_empty() && self.yubico.is_empty() - && self.recovery().is_none() + && self.recovery.is_none() } /// Find an entry by id, except for the "recovery" entry which we're currently treating @@ -577,7 +568,7 @@ impl TfaUserData { Ok(Some(TfaChallenge { totp: self.totp.iter().any(|e| e.info.enable), - recovery: RecoveryState::from(&self.recovery), + recovery: self.recovery_state(), webauthn: match webauthn { Some(webauthn) => self.webauthn_challenge(access, userid, webauthn?)?, None => None, @@ -591,8 +582,8 @@ impl TfaUserData { } /// Get the recovery state. - pub fn recovery_state(&self) -> RecoveryState { - RecoveryState::from(&self.recovery) + pub fn recovery_state(&self) -> Option { + self.recovery.as_ref().map(RecoveryState::from) } /// Generate an optional webauthn challenge. @@ -855,8 +846,8 @@ pub struct TfaChallenge { pub totp: bool, /// Whether there are recovery keys available. - #[serde(skip_serializing_if = "RecoveryState::is_unavailable", default)] - pub recovery: RecoveryState, + #[serde(skip_serializing_if = "Option::is_none", default)] + pub recovery: Option, /// If the user has any u2f tokens registered, this will contain the U2F challenge data. #[serde(skip_serializing_if = "Option::is_none")] diff --git a/proxmox-tfa/src/api/recovery.rs b/proxmox-tfa/src/api/recovery.rs index 92c0e9dc..970770b6 100644 --- a/proxmox-tfa/src/api/recovery.rs +++ b/proxmox-tfa/src/api/recovery.rs @@ -96,12 +96,6 @@ impl Recovery { self.available().count() } - /// Convenience serde method to check if either the option is `None` or the content `is_empty`. - pub(super) fn option_is_empty(this: &Option) -> bool { - this.as_ref() - .map_or(true, |this| this.count_available() == 0) - } - /// Verify a key and remove it. Returns whether the key was valid. Errors on openssl errors. pub(super) fn verify(&mut self, key: &str) -> Result { let hash = self.hash(key.as_bytes())?; @@ -131,15 +125,6 @@ impl RecoveryState { } } -impl From<&Option> for RecoveryState { - fn from(r: &Option) -> Self { - match r { - Some(r) => Self::from(r), - None => Self::default(), - } - } -} - impl From<&Recovery> for RecoveryState { fn from(r: &Recovery) -> Self { Self(