refactor(daemon): AppState delegates to storage modules; dbus tests use tempdir

Lands plan tasks 2.15 (AppState refactor with StorageConfig + open()) and 2.16
(dbus.rs tests switched to tempdir-backed AppState; storage errors threaded
through D-Bus methods as Failed). Bundled because the AppState surface change
forces dbus.rs adjustments in the same commit.

- daemon/src/state.rs: AppState::open(StorageConfig) replaces with_fixtures().
  StorageConfig.from_env_or_defaults() reads AUTHFORGE_POLICY_DIR /
  _PENDING_DIR / _USERDB env vars (defaults: /etc/authforge/policy.d,
  /var/lib/authforge/pending, /var/lib/authforge/users.db). State delegates
  list/add/remove credentials to CredsPathResolver + CredentialsStore picked
  per-call from current Policy; pending and userdb operate independently.
- daemon/src/main.rs: opens state via env-driven config; reuses cfg.policy_dir
  for the watcher to keep one source of truth.
- daemon/src/dbus.rs: every write method maps StateError to fdo::Error::Failed.
  p2p_pair seeds 00-test.conf with [storage] backend = central pointing into
  the tempdir so credential writes don't try to touch /home/<user>/...
  (alice/bob/carol aren't real accounts in tests).
- Renamed: list_credentials_returns_fixture_for_alice ->
  list_credentials_after_enroll. Removed: with_fixtures().
- .gitignore: add .claude/ so leftover Phase 1 worktree state isn't committed.

Test count: 26/26 daemon tests green (was 17). Common: 13/13. Clippy + fmt clean.
This commit is contained in:
michael
2026-04-27 06:44:07 -07:00
parent 90f7a0f4fc
commit 24237cbf8b
5 changed files with 244 additions and 145 deletions

1
.gitignore vendored
View File

@@ -14,3 +14,4 @@ pam/*.so
build/
.vscode/
*.swp
.claude/

View File

@@ -38,7 +38,10 @@ impl AuthForge {
}
async fn get_policy(&self) -> zbus::fdo::Result<Policy> {
Ok(self.state.get_policy().await)
self.state
.get_policy()
.await
.map_err(|e| zbus::fdo::Error::Failed(e.to_string()))
}
async fn enroll_own(&self, user: String, nickname: String) -> zbus::fdo::Result<Credential> {
@@ -52,14 +55,22 @@ impl AuthForge {
transport: Transport::Usb,
created_unix: now,
};
self.state.add_credential(&user, cred.clone()).await;
self.state
.add_credential(&user, cred.clone())
.await
.map_err(|e| zbus::fdo::Error::Failed(e.to_string()))?;
Ok(cred)
}
async fn remove_own(&self, user: String, cred_id: String) -> zbus::fdo::Result<()> {
self.authz("io.dangerousthings.AuthForge.remove-own")
.await?;
if self.state.remove_credential(&user, &cred_id).await {
let removed = self
.state
.remove_credential(&user, &cred_id)
.await
.map_err(|e| zbus::fdo::Error::Failed(e.to_string()))?;
if removed {
Ok(())
} else {
Err(zbus::fdo::Error::Failed(format!(
@@ -79,14 +90,20 @@ impl AuthForge {
transport: Transport::Usb,
created_unix: now,
};
self.state.add_credential(&user, cred.clone()).await;
self.state
.add_credential(&user, cred.clone())
.await
.map_err(|e| zbus::fdo::Error::Failed(e.to_string()))?;
Ok(cred)
}
async fn set_policy(&self, p: Policy) -> zbus::fdo::Result<PolicyApplyResult> {
self.authz("io.dangerousthings.AuthForge.set-policy")
.await?;
self.state.set_policy(p).await;
self.state
.set_policy(p)
.await
.map_err(|e| zbus::fdo::Error::Failed(e.to_string()))?;
Ok(PolicyApplyResult {
applied: true,
violations: vec![],
@@ -96,14 +113,20 @@ impl AuthForge {
async fn set_pending_flag(&self, user: String, flag: PendingFlag) -> zbus::fdo::Result<()> {
self.authz("io.dangerousthings.AuthForge.set-pending")
.await?;
self.state.set_pending(&user, flag).await;
self.state
.set_pending(&user, flag)
.await
.map_err(|e| zbus::fdo::Error::Failed(e.to_string()))?;
Ok(())
}
async fn clear_pending_flag(&self, user: String) -> zbus::fdo::Result<()> {
self.authz("io.dangerousthings.AuthForge.clear-pending")
.await?;
self.state.clear_pending(&user).await;
self.state
.clear_pending(&user)
.await
.map_err(|e| zbus::fdo::Error::Failed(e.to_string()))?;
Ok(())
}
@@ -133,11 +156,39 @@ mod tests {
use zbus::connection::Builder;
use zbus::Connection;
/// Build a peer-to-peer pair: server side has the AuthForge interface registered
/// at /io/dangerousthings/AuthForge, client side is a bare connection. No system
/// bus is involved.
async fn p2p_pair() -> (Connection, Connection, Arc<AppState>) {
let state = Arc::new(AppState::with_fixtures());
/// Build a peer-to-peer pair: server side has the AuthForge interface
/// registered at /io/dangerousthings/AuthForge, client side is a bare
/// connection. No system bus is involved. The TempDir must be returned —
/// dropping it before the test ends would delete the storage directories.
///
/// Storage is seeded with a central-mode policy pointing at a file inside
/// the tempdir so credential writes don't try to touch /home/<user>/...
/// (test users like alice/bob/carol aren't real accounts).
async fn p2p_pair() -> (Connection, Connection, Arc<AppState>, tempfile::TempDir) {
let tmp = tempfile::tempdir().unwrap();
let policy_dir = tmp.path().join("policy.d");
std::fs::create_dir_all(&policy_dir).unwrap();
let central = tmp.path().join("u2f_keys");
std::fs::write(
policy_dir.join("00-test.conf"),
format!(
r#"[storage]
backend = "central"
central_path = "{}"
"#,
central.display()
),
)
.unwrap();
let state = Arc::new(
AppState::open(crate::state::StorageConfig {
policy_dir,
pending_dir: tmp.path().join("pending"),
userdb_path: tmp.path().join("users.db"),
})
.unwrap(),
);
let auth = AuthForge {
state: state.clone(),
polkit: Arc::new(Polkit::permissive()),
@@ -159,7 +210,7 @@ mod tests {
let client_build = Builder::socket(client_sock).p2p().build();
let (server, client) = tokio::join!(server_build, client_build);
(server.unwrap(), client.unwrap(), state)
(server.unwrap(), client.unwrap(), state, tmp)
}
async fn proxy(client: &Connection) -> zbus::Proxy<'_> {
@@ -174,9 +225,12 @@ mod tests {
}
#[tokio::test]
async fn list_credentials_returns_fixture_for_alice() {
let (_srv, client, _state) = p2p_pair().await;
async fn list_credentials_after_enroll() {
let (_srv, client, _state, _tmp) = p2p_pair().await;
let p = proxy(&client).await;
// Seed via the same EnrollOwn path the GUI would use; previously this
// was a Phase 1 fixture in AppState, but file-backed storage starts empty.
let _: Credential = p.call("EnrollOwn", &("alice", "Yellow")).await.unwrap();
let creds: Vec<Credential> = p.call("ListCredentials", &("alice",)).await.unwrap();
assert_eq!(creds.len(), 1);
assert_eq!(creds[0].method, Method::Fido2);
@@ -184,7 +238,7 @@ mod tests {
#[tokio::test]
async fn get_policy_returns_default() {
let (_srv, client, _state) = p2p_pair().await;
let (_srv, client, _state, _tmp) = p2p_pair().await;
let p = proxy(&client).await;
let pol: Policy = p.call("GetPolicy", &()).await.unwrap();
assert!(pol.stacks.is_empty());
@@ -192,7 +246,7 @@ mod tests {
#[tokio::test]
async fn enroll_own_appends_credential() {
let (_srv, client, state) = p2p_pair().await;
let (_srv, client, state, _tmp) = p2p_pair().await;
let p = proxy(&client).await;
let _: Credential = p.call("EnrollOwn", &("bob", "Bob's Key")).await.unwrap();
assert_eq!(state.list_credentials("bob").await.len(), 1);
@@ -200,10 +254,12 @@ mod tests {
#[tokio::test]
async fn remove_own_drops_credential() {
let (_srv, client, state) = p2p_pair().await;
let (_srv, client, state, _tmp) = p2p_pair().await;
let p = proxy(&client).await;
// Enroll, then remove by the cred id we just got back.
let cred: Credential = p.call("EnrollOwn", &("alice", "Yellow")).await.unwrap();
let _: () = p
.call("RemoveOwn", &("alice", "fixture-cred-1"))
.call("RemoveOwn", &("alice", cred.id.as_str()))
.await
.unwrap();
assert!(state.list_credentials("alice").await.is_empty());
@@ -211,7 +267,7 @@ mod tests {
#[tokio::test]
async fn remove_own_unknown_id_errors() {
let (_srv, client, _state) = p2p_pair().await;
let (_srv, client, _state, _tmp) = p2p_pair().await;
let p = proxy(&client).await;
let r: Result<(), zbus::Error> = p.call("RemoveOwn", &("alice", "no-such-id")).await;
assert!(r.is_err());
@@ -219,15 +275,15 @@ mod tests {
#[tokio::test]
async fn enroll_other_creates_credential() {
let (_srv, client, state) = p2p_pair().await;
let (_srv, client, state, _tmp) = p2p_pair().await;
let p = proxy(&client).await;
let _: Credential = p.call("EnrollOther", &("carol", "Hers")).await.unwrap();
assert_eq!(state.list_credentials("carol").await.len(), 1);
}
#[tokio::test]
async fn set_policy_replaces_state() {
let (_srv, client, state) = p2p_pair().await;
async fn set_policy_persists_stacks() {
let (_srv, client, state, _tmp) = p2p_pair().await;
let p = proxy(&client).await;
let mut stacks = BTreeMap::new();
stacks.insert(
@@ -238,17 +294,20 @@ mod tests {
},
);
let pol = Policy {
stacks,
stacks: stacks.clone(),
..Default::default()
};
let r: PolicyApplyResult = p.call("SetPolicy", &(pol.clone(),)).await.unwrap();
let r: PolicyApplyResult = p.call("SetPolicy", &(pol,)).await.unwrap();
assert!(r.applied);
assert_eq!(state.get_policy().await, pol);
// p2p_pair seeds 00-test.conf with [storage]; SetPolicy writes 50-local.conf.
// The merged read shows our new stack alongside the seeded storage block.
let merged = state.get_policy().await.unwrap();
assert_eq!(merged.stacks, stacks);
}
#[tokio::test]
async fn set_and_clear_pending_flag() {
let (_srv, client, state) = p2p_pair().await;
let (_srv, client, state, _tmp) = p2p_pair().await;
let p = proxy(&client).await;
let flag = PendingFlag {
required_methods: vec![Method::Fido2],
@@ -257,14 +316,14 @@ mod tests {
re_enroll: false,
};
let _: () = p.call("SetPendingFlag", &("alice", flag)).await.unwrap();
assert!(state.has_pending("alice").await);
assert!(state.has_pending("alice").await.unwrap());
let _: () = p.call("ClearPendingFlag", &("alice",)).await.unwrap();
assert!(!state.has_pending("alice").await);
assert!(!state.has_pending("alice").await.unwrap());
}
#[tokio::test]
async fn generate_recovery_code_returns_8_digits() {
let (_srv, client, _state) = p2p_pair().await;
let (_srv, client, _state, _tmp) = p2p_pair().await;
let p = proxy(&client).await;
let code: String = p.call("GenerateRecoveryCode", &("alice",)).await.unwrap();
assert_eq!(code.len(), 8);
@@ -274,7 +333,7 @@ mod tests {
#[tokio::test]
async fn policy_changed_signal_fires_on_emit() {
use futures_util::stream::StreamExt;
let (server, client, _state) = p2p_pair().await;
let (server, client, _state, _tmp) = p2p_pair().await;
let proxy = proxy(&client).await;
let mut stream = proxy.receive_signal("PolicyChanged").await.unwrap();

View File

@@ -1,5 +1,4 @@
use anyhow::{Context, Result};
use std::path::PathBuf;
use std::sync::Arc;
use tracing::{info, warn};
@@ -31,7 +30,9 @@ async fn main() -> Result<()> {
polkit::Polkit::system(conn.clone()).await
};
let state = Arc::new(state::AppState::with_fixtures());
let cfg = state::StorageConfig::from_env_or_defaults();
let policy_dir = cfg.policy_dir.clone();
let state = Arc::new(state::AppState::open(cfg)?);
let auth = dbus::AuthForge {
state: state.clone(),
polkit: Arc::new(polkit),
@@ -45,9 +46,6 @@ async fn main() -> Result<()> {
info!("listening on D-Bus as {BUS_NAME} at {OBJECT_PATH}");
// Inotify watcher → PolicyChanged signal.
let policy_dir = std::env::var_os("AUTHFORGE_POLICY_DIR")
.map(PathBuf::from)
.unwrap_or_else(|| PathBuf::from("/etc/authforge/policy.d"));
let policy_store = storage::policy::PolicyStore::new(policy_dir.clone());
match policy_store.watch() {
Ok((watcher, mut rx)) => {

View File

@@ -1,143 +1,187 @@
use authforge_common::policy::Policy;
use crate::storage::credentials::{CredEntry, CredentialsStore, CredsError, CredsPathResolver};
use crate::storage::pending::{PendingError, PendingStore};
use crate::storage::policy::PolicyStore;
use crate::storage::userdb::{UserDb, UserDbError};
use authforge_common::policy::{Policy, PolicyError};
use authforge_common::types::{Credential, Method, PendingFlag, Transport};
use std::collections::HashMap;
use tokio::sync::RwLock;
use std::path::PathBuf;
use thiserror::Error;
use tokio::sync::Mutex;
#[derive(Default)]
pub struct AppState {
inner: RwLock<StateInner>,
#[derive(Debug, Error)]
pub enum StateError {
#[error(transparent)]
Policy(#[from] PolicyError),
#[error(transparent)]
Pending(#[from] PendingError),
#[error(transparent)]
Creds(#[from] CredsError),
#[error(transparent)]
UserDb(#[from] UserDbError),
}
#[derive(Default)]
struct StateInner {
credentials: HashMap<String, Vec<Credential>>,
policy: Policy,
pending: HashMap<String, PendingFlag>,
pub struct StorageConfig {
pub policy_dir: PathBuf,
pub pending_dir: PathBuf,
pub userdb_path: PathBuf,
}
impl StorageConfig {
pub fn from_env_or_defaults() -> Self {
let env = |k: &str, d: &str| -> PathBuf {
std::env::var_os(k)
.map(PathBuf::from)
.unwrap_or_else(|| PathBuf::from(d))
};
Self {
policy_dir: env("AUTHFORGE_POLICY_DIR", "/etc/authforge/policy.d"),
pending_dir: env("AUTHFORGE_PENDING_DIR", "/var/lib/authforge/pending"),
userdb_path: env("AUTHFORGE_USERDB", "/var/lib/authforge/users.db"),
}
}
}
pub struct AppState {
policy: PolicyStore,
pending: PendingStore,
userdb: Mutex<UserDb>,
}
impl AppState {
pub fn with_fixtures() -> Self {
let mut inner = StateInner::default();
inner.credentials.insert(
"alice".to_string(),
vec![Credential {
id: "fixture-cred-1".to_string(),
nickname: "Alice's Yubikey".to_string(),
method: Method::Fido2,
transport: Transport::Usb,
created_unix: 1_700_000_000,
}],
);
Self {
inner: RwLock::new(inner),
}
pub fn open(cfg: StorageConfig) -> Result<Self, StateError> {
let policy = PolicyStore::new(cfg.policy_dir);
let pending = PendingStore::new(cfg.pending_dir);
let userdb = Mutex::new(UserDb::open(&cfg.userdb_path)?);
Ok(Self {
policy,
pending,
userdb,
})
}
fn creds_resolver(&self) -> Result<CredsPathResolver, StateError> {
let pol = self.policy.load()?;
Ok(CredsPathResolver::new(pol.storage))
}
/// Phase 2 returns opaque cred IDs. Richer Credential metadata (real
/// nickname, transport, created_unix) is recorded at enrollment time
/// in Phase 3; until then the keyHandle stands in for both id and
/// nickname so the GUI shows *something*.
pub async fn list_credentials(&self, user: &str) -> Vec<Credential> {
self.inner
.read()
.await
.credentials
.get(user)
.cloned()
.unwrap_or_default()
}
pub async fn add_credential(&self, user: &str, c: Credential) {
self.inner
.write()
.await
.credentials
.entry(user.to_string())
.or_default()
.push(c);
}
pub async fn remove_credential(&self, user: &str, cred_id: &str) -> bool {
let mut g = self.inner.write().await;
let Some(list) = g.credentials.get_mut(user) else {
return false;
let resolver = match self.creds_resolver() {
Ok(r) => r,
Err(_) => return vec![],
};
let before = list.len();
list.retain(|c| c.id != cred_id);
list.len() != before
let path = match resolver.path_for(user) {
Ok(p) => p,
Err(_) => return vec![],
};
let store = CredentialsStore::new(path);
let lines = store.list(user).unwrap_or_default();
lines
.iter()
.map(|c| {
let id = CredEntry::cred_id(c).to_string();
Credential {
id: id.clone(),
nickname: id,
method: Method::Fido2,
transport: Transport::Unknown,
created_unix: 0,
}
})
.collect()
}
pub async fn get_policy(&self) -> Policy {
self.inner.read().await.policy.clone()
pub async fn add_credential(&self, user: &str, c: Credential) -> Result<(), StateError> {
let resolver = self.creds_resolver()?;
let path = resolver.path_for(user)?;
let store = CredentialsStore::new(path);
// Phase 2 records id-only; Phase 3 swaps in the full pam_u2f line.
store.add(user, &c.id)?;
let db = self.userdb.lock().await;
db.record_enrollment(user, c.method)?;
Ok(())
}
pub async fn set_policy(&self, p: Policy) {
self.inner.write().await.policy = p;
pub async fn remove_credential(&self, user: &str, cred_id: &str) -> Result<bool, StateError> {
let resolver = self.creds_resolver()?;
let path = resolver.path_for(user)?;
let store = CredentialsStore::new(path);
let removed = store.remove(user, cred_id)?;
if removed && store.list(user)?.is_empty() {
self.userdb
.lock()
.await
.drop_enrollment(user, Method::Fido2)?;
}
Ok(removed)
}
pub async fn set_pending(&self, user: &str, f: PendingFlag) {
self.inner.write().await.pending.insert(user.to_string(), f);
pub async fn get_policy(&self) -> Result<Policy, StateError> {
Ok(self.policy.load()?)
}
pub async fn clear_pending(&self, user: &str) -> bool {
self.inner.write().await.pending.remove(user).is_some()
pub async fn set_policy(&self, p: Policy) -> Result<(), StateError> {
self.policy.save(&p)?;
Ok(())
}
/// Tests assert pending flag round-trips through SetPendingFlag/ClearPendingFlag
/// via this read accessor. Production readers (PAM module, GUI status) come in
/// later phases and will read the on-disk file directly, not this in-memory cache.
pub async fn set_pending(&self, user: &str, f: PendingFlag) -> Result<(), StateError> {
self.pending.set(user, &f)?;
Ok(())
}
pub async fn clear_pending(&self, user: &str) -> Result<bool, StateError> {
Ok(self.pending.clear(user)?)
}
/// Tests assert pending flag round-trips through SetPendingFlag /
/// ClearPendingFlag via this read accessor. Production readers (PAM
/// module, GUI status) read the on-disk file directly in later phases.
#[cfg(test)]
pub async fn has_pending(&self, user: &str) -> bool {
self.inner.read().await.pending.contains_key(user)
pub async fn has_pending(&self, user: &str) -> Result<bool, StateError> {
Ok(self.pending.get(user)?.is_some())
}
}
#[cfg(test)]
mod tests {
use super::*;
use tempfile::tempdir;
#[tokio::test]
async fn fixtures_seeded_for_alice() {
let s = AppState::with_fixtures();
let creds = s.list_credentials("alice").await;
assert_eq!(creds.len(), 1);
assert_eq!(creds[0].nickname, "Alice's Yubikey");
fn open_in(d: &std::path::Path) -> AppState {
AppState::open(StorageConfig {
policy_dir: d.join("policy.d"),
pending_dir: d.join("pending"),
userdb_path: d.join("users.db"),
})
.unwrap()
}
#[tokio::test]
async fn unknown_user_has_no_creds() {
let s = AppState::with_fixtures();
assert!(s.list_credentials("nobody").await.is_empty());
}
#[tokio::test]
async fn add_then_remove_credential() {
let s = AppState::default();
let c = Credential {
id: "id1".to_string(),
nickname: "x".to_string(),
method: Method::Fido2,
transport: Transport::Usb,
created_unix: 0,
};
s.add_credential("bob", c).await;
assert_eq!(s.list_credentials("bob").await.len(), 1);
assert!(s.remove_credential("bob", "id1").await);
assert!(s.list_credentials("bob").await.is_empty());
async fn empty_state_has_nothing() {
let d = tempdir().unwrap();
let s = open_in(d.path());
assert!(s.list_credentials("alice").await.is_empty());
assert!(!s.has_pending("alice").await.unwrap());
assert!(s.get_policy().await.unwrap().stacks.is_empty());
}
#[tokio::test]
async fn pending_set_clear_roundtrip() {
let s = AppState::default();
assert!(!s.has_pending("alice").await);
s.set_pending(
"alice",
PendingFlag {
required_methods: vec![Method::Fido2],
created_unix: 1,
deadline_unix: 0,
re_enroll: false,
},
)
.await;
assert!(s.has_pending("alice").await);
assert!(s.clear_pending("alice").await);
assert!(!s.has_pending("alice").await);
let d = tempdir().unwrap();
let s = open_in(d.path());
let f = PendingFlag {
required_methods: vec![Method::Fido2],
created_unix: 1,
deadline_unix: 0,
re_enroll: false,
};
s.set_pending("alice", f).await.unwrap();
assert!(s.has_pending("alice").await.unwrap());
assert!(s.clear_pending("alice").await.unwrap());
assert!(!s.has_pending("alice").await.unwrap());
}
}

View File

@@ -7,10 +7,7 @@ pub(crate) enum CredsError {
#[error("io: {0}")]
Io(#[from] std::io::Error),
#[error("malformed line for {user:?}: {reason}")]
Malformed {
user: String,
reason: &'static str,
},
Malformed { user: String, reason: &'static str },
}
/// One line of a `pam_u2f` keys file. Format per pam_u2f(8):