From a420aa5f750b0762c65ffa9a9b75041aae4614d6 Mon Sep 17 00:00:00 2001 From: michael Date: Mon, 27 Apr 2026 09:39:16 -0700 Subject: [PATCH] feat(daemon): real GenerateRecoveryCode + ListRecoveryCodes + RevokeRecoveryCode Replace the random-number stub at dbus.rs:132 with state.issue_recovery, add ListRecoveryCodes (returns Vec) and RevokeRecoveryCode. New polkit actions list-recovery / revoke-recovery (auth_admin_keep). Adds 4 D-Bus integration tests; the previously-stub generate-code test now exercises the real Argon2id-backed write path. Co-Authored-By: Claude Opus 4.7 (1M context) --- common/src/types.rs | 6 ++ daemon/src/dbus.rs | 85 ++++++++++++++++++++-- daemon/src/recovery.rs | 4 +- daemon/src/state.rs | 4 +- daemon/src/storage/recovery.rs | 3 +- daemon/src/storage/safe_user.rs | 7 +- debian/io.dangerousthings.AuthForge.policy | 20 +++++ 7 files changed, 114 insertions(+), 15 deletions(-) diff --git a/common/src/types.rs b/common/src/types.rs index 2563fa4..bb4cd0a 100644 --- a/common/src/types.rs +++ b/common/src/types.rs @@ -59,6 +59,12 @@ pub struct PolicyApplyResult { pub violations: Vec, } +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, zvariant::Type)] +pub struct RecoveryCodeSummary { + pub user: String, + pub expires_unix: u64, +} + #[cfg(test)] mod tests { use super::*; diff --git a/daemon/src/dbus.rs b/daemon/src/dbus.rs index 4cd6f70..0bc8bb3 100644 --- a/daemon/src/dbus.rs +++ b/daemon/src/dbus.rs @@ -1,7 +1,7 @@ use crate::polkit::Polkit; use crate::state::AppState; use authforge_common::policy::Policy; -use authforge_common::types::{Credential, PendingFlag, PolicyApplyResult}; +use authforge_common::types::{Credential, PendingFlag, PolicyApplyResult, RecoveryCodeSummary}; use std::sync::Arc; pub struct AuthForge { @@ -129,13 +129,39 @@ impl AuthForge { Ok(()) } - async fn generate_recovery_code(&self, _user: String) -> zbus::fdo::Result { + async fn generate_recovery_code(&self, user: String) -> zbus::fdo::Result { self.authz("io.dangerousthings.AuthForge.generate-recovery") .await?; - // Stub: 8 random digits. Phase 12 replaces with Argon2id-backed real flow. - use rand::Rng; - let n: u32 = rand::rng().random_range(0..100_000_000); - Ok(format!("{n:08}")) + self.state + .issue_recovery(&user) + .await + .map_err(|e| zbus::fdo::Error::Failed(e.to_string())) + } + + async fn list_recovery_codes(&self) -> zbus::fdo::Result> { + self.authz("io.dangerousthings.AuthForge.list-recovery") + .await?; + let entries = self + .state + .list_recovery() + .await + .map_err(|e| zbus::fdo::Error::Failed(e.to_string()))?; + Ok(entries + .into_iter() + .map(|e| RecoveryCodeSummary { + user: e.user, + expires_unix: e.expires_unix, + }) + .collect()) + } + + async fn revoke_recovery_code(&self, user: String) -> zbus::fdo::Result { + self.authz("io.dangerousthings.AuthForge.revoke-recovery") + .await?; + self.state + .revoke_recovery(&user) + .await + .map_err(|e| zbus::fdo::Error::Failed(e.to_string())) } /// Emitted by main.rs whenever the inotify watcher on the policy.d/ dir @@ -370,6 +396,53 @@ central_path = "{}" assert!(code.chars().all(|c| c.is_ascii_digit())); } + #[tokio::test] + async fn list_recovery_returns_issued_users() { + use authforge_common::types::RecoveryCodeSummary; + let (_srv, client, _state, _tmp) = p2p_pair().await; + let p = proxy(&client).await; + let _: String = p.call("GenerateRecoveryCode", &("alice",)).await.unwrap(); + let _: String = p.call("GenerateRecoveryCode", &("bob",)).await.unwrap(); + let entries: Vec = p.call("ListRecoveryCodes", &()).await.unwrap(); + let users: Vec<_> = entries.into_iter().map(|e| e.user).collect(); + assert_eq!(users, vec!["alice".to_string(), "bob".to_string()]); + } + + #[tokio::test] + async fn revoke_recovery_removes_entry() { + use authforge_common::types::RecoveryCodeSummary; + let (_srv, client, _state, _tmp) = p2p_pair().await; + let p = proxy(&client).await; + let _: String = p.call("GenerateRecoveryCode", &("alice",)).await.unwrap(); + let removed: bool = p.call("RevokeRecoveryCode", &("alice",)).await.unwrap(); + assert!(removed); + let entries: Vec = p.call("ListRecoveryCodes", &()).await.unwrap(); + assert!(entries.is_empty()); + } + + #[tokio::test] + async fn revoke_recovery_returns_false_on_missing_user() { + let (_srv, client, _state, _tmp) = p2p_pair().await; + let p = proxy(&client).await; + let removed: bool = p.call("RevokeRecoveryCode", &("ghost",)).await.unwrap(); + assert!(!removed); + } + + #[tokio::test] + async fn generate_recovery_code_persists_argon2id_hash_on_disk() { + let (_srv, client, _state, tmp) = p2p_pair().await; + let p = proxy(&client).await; + let _: String = p.call("GenerateRecoveryCode", &("alice",)).await.unwrap(); + let body = std::fs::read_to_string(tmp.path().join("recovery").join("alice")).unwrap(); + let mut lines = body.lines(); + // Line 1: expires_unix (numeric). + let expires: u64 = lines.next().unwrap().parse().unwrap(); + assert!(expires > 0); + // Line 2: argon2id PHC string. + let phc = lines.next().unwrap(); + assert!(phc.starts_with("$argon2id$"), "phc was {phc}"); + } + #[tokio::test] async fn enrollment_succeeded_signal_fires_on_enroll_own() { use futures_util::stream::StreamExt; diff --git a/daemon/src/recovery.rs b/daemon/src/recovery.rs index f5c3c54..4230bfa 100644 --- a/daemon/src/recovery.rs +++ b/daemon/src/recovery.rs @@ -4,7 +4,9 @@ #![allow(dead_code)] // wired through RecoveryStore (Task 4) and AppState (Task 5). use argon2::{ - password_hash::{rand_core::OsRng as PhcOsRng, PasswordHash, PasswordHasher, PasswordVerifier, SaltString}, + password_hash::{ + rand_core::OsRng as PhcOsRng, PasswordHash, PasswordHasher, PasswordVerifier, SaltString, + }, Argon2, }; use rand::Rng; diff --git a/daemon/src/state.rs b/daemon/src/state.rs index b5341d9..8aeaa87 100644 --- a/daemon/src/state.rs +++ b/daemon/src/state.rs @@ -233,7 +233,9 @@ impl AppState { #[allow(dead_code)] // wired through D-Bus in Task 6. pub async fn issue_recovery(&self, user: &str) -> Result { - Ok(self.recovery.issue(user, crate::recovery::DEFAULT_TTL_SECS)?) + Ok(self + .recovery + .issue(user, crate::recovery::DEFAULT_TTL_SECS)?) } #[allow(dead_code)] // wired through D-Bus in Task 6. diff --git a/daemon/src/storage/recovery.rs b/daemon/src/storage/recovery.rs index a5335cd..e0499b4 100644 --- a/daemon/src/storage/recovery.rs +++ b/daemon/src/storage/recovery.rs @@ -161,7 +161,8 @@ impl RecoveryStore { return Ok(false); } - let ok = verify_code(candidate, phc).map_err(|e| RecoveryStoreError::Hash(e.to_string()))?; + let ok = + verify_code(candidate, phc).map_err(|e| RecoveryStoreError::Hash(e.to_string()))?; if ok { let _ = fs::remove_file(&path); } diff --git a/daemon/src/storage/safe_user.rs b/daemon/src/storage/safe_user.rs index fdf84a7..413edd8 100644 --- a/daemon/src/storage/safe_user.rs +++ b/daemon/src/storage/safe_user.rs @@ -12,12 +12,7 @@ pub(crate) enum SegmentError { /// Reject any username that could escape a single path segment. Centralized /// so pending / recovery / future stores don't drift on what's "safe." pub(crate) fn join_user_segment(dir: &Path, user: &str) -> Result { - if user.is_empty() - || user == "." - || user == ".." - || user.contains('/') - || user.contains('\0') - { + if user.is_empty() || user == "." || user == ".." || user.contains('/') || user.contains('\0') { return Err(SegmentError::Invalid(user.to_string())); } Ok(dir.join(user)) diff --git a/debian/io.dangerousthings.AuthForge.policy b/debian/io.dangerousthings.AuthForge.policy index 4229ee8..5708116 100644 --- a/debian/io.dangerousthings.AuthForge.policy +++ b/debian/io.dangerousthings.AuthForge.policy @@ -77,4 +77,24 @@ + + List active recovery codes + Administrator authentication is required to list recovery codes. + + auth_admin_keep + auth_admin_keep + auth_admin_keep + + + + + Revoke a recovery code + Administrator authentication is required to revoke a recovery code. + + auth_admin_keep + auth_admin_keep + auth_admin_keep + + +