MikeFaith/authforge
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
481f7f85436166608f59c1fd16b81cd20df31649
authforge/docs/plans/2026-04-27-phase-2-storage.md

1619 lines
53 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Phase 2: Daemon Storage Layer — Implementation Plan
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task.
**Goal:** Replace `AppState`'s in-memory `HashMap` fixtures with real on-disk I/O — TOML policy directory with last-key-wins merge, JSON pending flags, `pam_u2f`-format credential files, sqlite user-enrollment cache. Daemon watches the policy dir via inotify and emits a `PolicyChanged` D-Bus signal on change.
**Architecture:** Pure parser/merger logic lives in `common/src/policy.rs` (CLI also calls it without spawning the daemon). Daemon wraps it in `daemon/src/storage/` modules that own the I/O paths and the inotify watcher. `AppState` keeps its public surface (`list_credentials`, `set_policy`, etc. — what `dbus.rs` calls) but delegates internally to a `Storage` aggregate. Tests use `tempfile::tempdir()` fixtures end-to-end; nothing is mocked.
**Tech Stack:** Rust 2021, `toml = "0.8"` (already in workspace deps), `tokio::fs` for async file I/O, `notify = "6"` for cross-platform fsnotify (uses inotify on Linux), `rusqlite = "0.31"` with `bundled` feature for the user-enrollment cache.
**Reference:**
- Design doc: [2026-04-26-authforge-design.md](2026-04-26-authforge-design.md) — § Policy file format, § Lockout safety simulation, Flow diagrams.
- Master plan (Phase 2 section): [2026-04-26-authforge-implementation.md](2026-04-26-authforge-implementation.md).
- Phase 1 (state/dbus contract): [2026-04-26-phase-1-dbus.md](2026-04-26-phase-1-dbus.md).
---
## Conventions
- One logical change per commit. Conventional prefixes (`feat:`, `test:`, `chore:`, `refactor:`).
- TDD where mechanical: failing test → run to confirm fail → minimal impl → run to confirm pass → commit.
- Always commit with `--no-gpg-sign` (smart card not present in dev env; persistent project memory).
- After each task: `cargo fmt --all && cargo clippy --workspace --all-targets -- -D warnings && cargo test -p authforge-{common,daemon}`. Fix before commit.
- `gui` is excluded from `default-members`; workspace tests run only common + daemon + cli.
- Tests use `tempfile::tempdir()` to write into ephemeral dirs — never touch `/etc` or `/var` from a test.
- All new files in `daemon/src/storage/` are `pub(crate)`. Only `AppState` is the public surface to `dbus.rs`.
---
## Parallelism Note
Tasks 2.9 (pending), 2.10–2.12 (credentials), and 2.13–2.14 (userdb) operate on independent file trees and are good candidates for parallel subagents (one per file group, all in the same worktree — they don't share files). Tasks 2.6–2.8 (policy storage + inotify watcher) must run after 2.3–2.5 land in `common`. The acceptance gate (2.17) must run last.
---
## Task 2.1: Add storage dependencies
**Files:**
- Modify: `Cargo.toml` (workspace root)
- Modify: `common/Cargo.toml`
- Modify: `daemon/Cargo.toml`
**Step 1:** Add to root `Cargo.toml` `[workspace.dependencies]`:
```toml
notify = "6"
rusqlite = { version = "0.31", features = ["bundled"] }
```
`bundled` ships SQLite as C source so we don't depend on libsqlite3-dev at build time.
**Step 2:** `common/Cargo.toml` — no changes needed yet (toml is already there).
**Step 3:** `daemon/Cargo.toml` — add to `[dependencies]`:
```toml
notify = { workspace = true }
rusqlite = { workspace = true }
toml = { workspace = true }
```
`tokio` is already in deps with `features = ["full"]`, which includes `fs` and `sync::watch`.
**Step 4:** Verify the workspace compiles:
```bash
cargo check -p authforge-common -p authforge-daemon
```
Expected: clean (the new deps are unused but pulled into the lockfile).
**Step 5:** Commit.
```bash
git add Cargo.toml Cargo.lock daemon/Cargo.toml
git commit --no-gpg-sign -m "chore: add notify + rusqlite for Phase 2 storage layer"
```
---
## Task 2.2: Move Policy types from `types.rs` to `policy.rs`
**Files:**
- Modify: `common/src/policy.rs`
- Modify: `common/src/types.rs`
**Why:** `common/src/policy.rs` has been a stub since Phase 0; per the master plan, the Policy types and parsing logic live in `policy.rs`. `types.rs` keeps the cross-cutting D-Bus wire types (`Mode`, `Method`, `Credential`, `PendingFlag`, etc.). Splitting now keeps Phase 2's parser additions tightly scoped.
**Step 1:** Move these item definitions from `common/src/types.rs` to `common/src/policy.rs`:
- `StorageBackend`
- `StackPolicy`
- `Storage`
- `Firstrun`
- `Policy`
- The two policy-related tests (`policy_default_is_all_disabled`, `policy_serde_via_json`).
`policy.rs` top of file:
```rust
//! Policy types and (Phase 2) parsing/merging logic. Wire-typed for D-Bus.
use crate::types::{Method, Mode};
use serde::{Deserialize, Serialize};
use std::collections::BTreeMap;
```
**Step 2:** Re-export from `policy.rs` if any external consumer expected `types::Policy`:
`common/src/lib.rs`:
```rust
pub mod policy;
pub mod types;
// Convenience re-exports — Phase 1 clients imported these from `types`. Keep
// the path stable to avoid churn in daemon/cli.
pub use policy::{Firstrun, Policy, StackPolicy, Storage, StorageBackend};
```
**Step 3:** Update daemon imports — `dbus.rs` imports `Policy`, `PolicyApplyResult`, etc. from `authforge_common::types`. Switch `Policy`-related ones to `authforge_common::policy::*` (or use the re-exports). `state.rs` likewise.
**Step 4:** Run:
```bash
cargo test -p authforge-common -p authforge-daemon
```
Expected: 7 common + 14 daemon tests pass; no test changes needed.
**Step 5:** Commit.
```bash
git add common/src/policy.rs common/src/types.rs common/src/lib.rs daemon/src/dbus.rs daemon/src/state.rs
git commit --no-gpg-sign -m "refactor(common): move Policy types from types.rs into policy.rs"
```
---
## Task 2.3: `Policy::load_from_dir` — single-file case
**Files:**
- Modify: `common/src/policy.rs`
**Step 1:** Add the failing test:
```rust
#[cfg(test)]
mod parse_tests {
use super::*;
use crate::types::Method;
use tempfile::tempdir;
fn write(dir: &std::path::Path, name: &str, body: &str) {
std::fs::write(dir.join(name), body).unwrap();
}
#[test]
fn load_single_file() {
let d = tempdir().unwrap();
write(
d.path(),
"00-base.conf",
r#"
[stacks.sudo]
mode = "required"
methods = ["fido2"]
"#,
);
let p = Policy::load_from_dir(d.path()).unwrap();
assert_eq!(p.stacks.len(), 1);
let sudo = p.stacks.get("sudo").unwrap();
assert_eq!(sudo.mode, Mode::Required);
assert_eq!(sudo.methods, vec![Method::Fido2]);
}
}
```
`tempfile` is in `[workspace.dependencies]` already; add it as a dev-dep on `common` if not yet there:
```toml
[dev-dependencies]
tempfile = { workspace = true }
```
**Step 2:** Run, confirm fail (`Policy::load_from_dir` undefined).
**Step 3:** Implement. Append to `policy.rs`:
```rust
use thiserror::Error;
#[derive(Debug, Error)]
pub enum PolicyError {
#[error("reading policy directory: {0}")]
Io(#[from] std::io::Error),
#[error("parsing policy file {path}: {source}")]
Toml { path: String, source: toml::de::Error },
}
impl Policy {
/// Load and merge all `*.conf` files in `dir`, lex-ascending. Last value
/// for any key wins. Missing dir → returns Self::default().
pub fn load_from_dir(dir: &std::path::Path) -> Result<Self, PolicyError> {
if !dir.exists() {
return Ok(Self::default());
}
let mut entries: Vec<_> = std::fs::read_dir(dir)?
.filter_map(Result::ok)
.filter(|e| {
e.path()
.extension()
.and_then(|s| s.to_str())
.map(|s| s.eq_ignore_ascii_case("conf"))
.unwrap_or(false)
})
.collect();
entries.sort_by_key(|e| e.file_name());
let mut acc = Self::default();
for e in entries {
let body = std::fs::read_to_string(e.path())?;
let next: Policy = toml::from_str(&body).map_err(|source| PolicyError::Toml {
path: e.path().display().to_string(),
source,
})?;
acc.merge(next);
}
Ok(acc)
}
fn merge(&mut self, other: Policy) {
for (k, v) in other.stacks {
self.stacks.insert(k, v);
}
if other.storage != Storage::default() {
self.storage = other.storage;
}
if other.firstrun != Firstrun::default() {
self.firstrun = other.firstrun;
}
}
}
```
**Step 4:** Run, confirm PASS.
**Step 5:** Commit.
```bash
git add common/Cargo.toml common/src/policy.rs
git commit --no-gpg-sign -m "feat(common): Policy::load_from_dir parses TOML policy files"
```
---
## Task 2.4: `Policy::load_from_dir` — multi-file last-wins merge
**Files:**
- Modify: `common/src/policy.rs`
**Step 1:** Add three more tests that prove the merge contract:
```rust
#[test]
fn last_file_wins_on_overlap() {
let d = tempdir().unwrap();
write(d.path(), "00-base.conf", r#"[stacks.sudo]
mode = "optional"
methods = ["fido2"]"#);
write(d.path(), "90-fleet.conf", r#"[stacks.sudo]
mode = "required"
methods = ["fido2","totp"]"#);
let p = Policy::load_from_dir(d.path()).unwrap();
let sudo = p.stacks.get("sudo").unwrap();
assert_eq!(sudo.mode, Mode::Required);
assert_eq!(sudo.methods.len(), 2);
}
#[test]
fn lex_order_not_filesystem_order() {
// Reverse-create to defeat filesystem creation-order ordering.
let d = tempdir().unwrap();
write(d.path(), "99-late.conf", r#"[stacks.sudo]
mode = "disabled"
methods = []"#);
write(d.path(), "10-early.conf", r#"[stacks.sudo]
mode = "required"
methods = ["fido2"]"#);
let p = Policy::load_from_dir(d.path()).unwrap();
// 99-late wins because lex > 10-early.
assert_eq!(p.stacks.get("sudo").unwrap().mode, Mode::Disabled);
}
#[test]
fn ignores_non_conf_files() {
let d = tempdir().unwrap();
write(d.path(), "garbage.txt", "this is not toml");
write(d.path(), "00-base.conf", r#"[stacks.sudo]
mode = "required"
methods = ["fido2"]"#);
let p = Policy::load_from_dir(d.path()).unwrap();
assert_eq!(p.stacks.get("sudo").unwrap().mode, Mode::Required);
}
#[test]
fn missing_dir_yields_default() {
let d = tempdir().unwrap();
let bogus = d.path().join("nope");
assert_eq!(Policy::load_from_dir(&bogus).unwrap(), Policy::default());
}
```
**Step 2:** Run. The merge logic from Task 2.3 should already pass `last_file_wins_on_overlap`, `lex_order_not_filesystem_order`, and `ignores_non_conf_files`. `missing_dir_yields_default` passes via the early `if !dir.exists()` branch.
If any fail, fix `merge()`/`load_from_dir`.
**Step 3:** Commit.
```bash
git add common/src/policy.rs
git commit --no-gpg-sign -m "test(common): cover last-wins, lex order, non-conf, missing-dir cases"
```
---
## Task 2.5: `Policy::save_local` — write `50-local.conf`, preserve siblings
**Files:**
- Modify: `common/src/policy.rs`
**Why:** GUI/CLI edits should land in `50-local.conf` so a fleet-managed `90-fleet.conf` still wins (lex). The save path writes only the local file; sibling files are untouched.
**Step 1:** Failing test:
```rust
#[test]
fn save_local_preserves_sibling_files() {
let d = tempdir().unwrap();
write(d.path(), "90-fleet.conf", r#"[stacks.sudo]
mode = "required"
methods = ["fido2"]"#);
let mut local = Policy::default();
local.stacks.insert(
"gdm-password".to_string(),
StackPolicy { mode: Mode::Optional, methods: vec![Method::Fido2] },
);
local.save_local(d.path()).unwrap();
// Sibling untouched.
let fleet_body = std::fs::read_to_string(d.path().join("90-fleet.conf")).unwrap();
assert!(fleet_body.contains("required"));
// 50-local.conf created.
assert!(d.path().join("50-local.conf").exists());
// Reload merges both.
let merged = Policy::load_from_dir(d.path()).unwrap();
assert_eq!(merged.stacks.get("sudo").unwrap().mode, Mode::Required); // from 90-fleet
assert_eq!(merged.stacks.get("gdm-password").unwrap().mode, Mode::Optional); // from 50-local
}
```
**Step 2:** Run, confirm fail.
**Step 3:** Implement on `impl Policy`:
```rust
/// Serialize `self` as TOML and write to `<dir>/50-local.conf`. Other files
/// in `dir` are not read, written, or removed.
pub fn save_local(&self, dir: &std::path::Path) -> Result<(), PolicyError> {
std::fs::create_dir_all(dir)?;
let body = toml::to_string_pretty(self).map_err(|e| PolicyError::Toml {
path: dir.join("50-local.conf").display().to_string(),
source: toml::de::Error::custom(e.to_string()),
})?;
std::fs::write(dir.join("50-local.conf"), body)?;
Ok(())
}
```
`toml::de::Error::custom` doesn't exist — use a fresh enum variant instead:
```rust
#[derive(Debug, Error)]
pub enum PolicyError {
#[error("reading policy directory: {0}")]
Io(#[from] std::io::Error),
#[error("parsing policy file {path}: {source}")]
Toml { path: String, source: toml::de::Error },
#[error("serializing policy: {0}")]
Serialize(#[from] toml::ser::Error),
}
```
Then `save_local` becomes:
```rust
pub fn save_local(&self, dir: &std::path::Path) -> Result<(), PolicyError> {
std::fs::create_dir_all(dir)?;
let body = toml::to_string_pretty(self)?;
std::fs::write(dir.join("50-local.conf"), body)?;
Ok(())
}
```
**Step 4:** Run; PASS.
**Step 5:** Commit.
```bash
git add common/src/policy.rs
git commit --no-gpg-sign -m "feat(common): Policy::save_local writes 50-local.conf preserving siblings"
```
---
## Task 2.6: `daemon/src/storage/policy.rs` — wraps the parser with config
**Files:**
- Create: `daemon/src/storage/mod.rs`
- Create: `daemon/src/storage/policy.rs`
- Modify: `daemon/src/main.rs` (add `mod storage;`)
**Step 1:** `daemon/src/storage/mod.rs`:
```rust
pub(crate) mod policy;
```
**Step 2:** `daemon/src/storage/policy.rs`:
```rust
use authforge_common::policy::{Policy, PolicyError};
use std::path::PathBuf;
pub(crate) struct PolicyStore {
dir: PathBuf,
}
impl PolicyStore {
pub fn new(dir: PathBuf) -> Self {
Self { dir }
}
pub fn load(&self) -> Result<Policy, PolicyError> {
Policy::load_from_dir(&self.dir)
}
pub fn save(&self, p: &Policy) -> Result<(), PolicyError> {
p.save_local(&self.dir)
}
}
#[cfg(test)]
mod tests {
use super::*;
use authforge_common::types::Method;
use authforge_common::policy::StackPolicy;
use authforge_common::types::Mode;
use tempfile::tempdir;
#[test]
fn save_then_load_roundtrip() {
let d = tempdir().unwrap();
let store = PolicyStore::new(d.path().to_path_buf());
let mut p = Policy::default();
p.stacks.insert(
"sudo".to_string(),
StackPolicy { mode: Mode::Required, methods: vec![Method::Fido2] },
);
store.save(&p).unwrap();
let back = store.load().unwrap();
assert_eq!(back, p);
}
}
```
**Step 3:** Add `mod storage;` to `daemon/src/main.rs`.
**Step 4:** Run `cargo test -p authforge-daemon storage`. Expected: 1 PASS.
**Step 5:** Commit.
```bash
git add daemon/src/storage/mod.rs daemon/src/storage/policy.rs daemon/src/main.rs
git commit --no-gpg-sign -m "feat(daemon): storage::policy wraps Policy::load_from_dir + save_local"
```
---
## Task 2.7: `PolicyChanged` D-Bus signal
**Files:**
- Modify: `daemon/src/dbus.rs`
**Step 1:** Add a `signal` method to the `#[zbus::interface]` impl. zbus 4 emits signals via methods marked `#[zbus(signal)]`:
```rust
#[zbus(signal)]
pub async fn policy_changed(
signal_emitter: &zbus::object_server::SignalEmitter<'_>,
) -> zbus::Result<()>;
```
(Must be declared inside the `#[interface]` impl. zbus generates the dispatcher.)
**Step 2:** Add an integration test that subscribes to the signal on the p2p connection:
```rust
#[tokio::test]
async fn policy_changed_signal_fires_on_emit() {
use futures_util::stream::StreamExt;
let (server, client, _state) = p2p_pair().await;
let proxy = proxy(&client).await;
let mut stream = proxy.receive_signal("PolicyChanged").await.unwrap();
// Emit from the server side.
let iface_ref: zbus::object_server::InterfaceRef<AuthForge> =
server.object_server().interface("/io/dangerousthings/AuthForge").await.unwrap();
AuthForge::policy_changed(iface_ref.signal_emitter()).await.unwrap();
let _msg = tokio::time::timeout(std::time::Duration::from_secs(2), stream.next())
.await
.expect("signal not received within 2s")
.expect("stream closed before signal");
}
```
`futures-util` isn't in deps; add to daemon `[dev-dependencies]`:
```toml
futures-util = "0.3"
```
(Add `futures-util = "0.3"` to workspace deps so `daemon`'s dev-dep can `{ workspace = true }`.)
**Step 3:** Run. Expected: PASS.
**Step 4:** Commit.
```bash
git add Cargo.toml daemon/Cargo.toml daemon/src/dbus.rs
git commit --no-gpg-sign -m "feat(daemon): emit PolicyChanged D-Bus signal"
```
---
## Task 2.8: inotify watcher → emits `PolicyChanged`
**Files:**
- Modify: `daemon/src/storage/policy.rs`
- Modify: `daemon/src/main.rs`
**Step 1:** Add a `watch()` method to `PolicyStore` that returns a `tokio::sync::watch::Receiver<()>` ticking on every fs change in the policy dir:
```rust
use notify::{RecommendedWatcher, RecursiveMode, Watcher};
impl PolicyStore {
/// Start watching `self.dir` for any change. The returned receiver yields
/// `()` each time something changes. Caller is responsible for keeping the
/// returned `RecommendedWatcher` alive — drop it to stop watching.
pub fn watch(&self) -> notify::Result<(RecommendedWatcher, tokio::sync::watch::Receiver<()>)> {
let (tx, rx) = tokio::sync::watch::channel(());
let mut watcher = notify::recommended_watcher(move |res: notify::Result<notify::Event>| {
if res.is_ok() {
let _ = tx.send(());
}
})?;
// Safe to call even if the dir doesn't yet exist — we created it via save_local.
std::fs::create_dir_all(&self.dir).ok();
watcher.watch(&self.dir, RecursiveMode::NonRecursive)?;
Ok((watcher, rx))
}
}
```
**Step 2:** Add a unit test that creates a tempdir, starts watching, touches a file, asserts a tick arrives:
```rust
#[tokio::test]
async fn watcher_ticks_on_file_change() {
let d = tempdir().unwrap();
let store = PolicyStore::new(d.path().to_path_buf());
let (_watcher, mut rx) = store.watch().unwrap();
std::fs::write(d.path().join("00-base.conf"), "[stacks.sudo]\nmode = \"required\"\nmethods = [\"fido2\"]\n").unwrap();
tokio::time::timeout(std::time::Duration::from_secs(2), rx.changed())
.await
.expect("no change within 2s")
.unwrap();
}
```
**Step 3:** Wire `main.rs` to spawn a task that calls `dbus::AuthForge::policy_changed(...)` whenever `rx.changed()` fires:
```rust
// After object_server().at(...).await? line:
let policy_dir = std::env::var_os("AUTHFORGE_POLICY_DIR")
.map(PathBuf::from)
.unwrap_or_else(|| PathBuf::from("/etc/authforge/policy.d"));
let store = storage::policy::PolicyStore::new(policy_dir);
let (watcher, mut rx) = store.watch()?;
let conn_for_signal = conn.clone();
tokio::spawn(async move {
while rx.changed().await.is_ok() {
let iface_ref = match conn_for_signal
.object_server()
.interface::<_, dbus::AuthForge>(OBJECT_PATH)
.await
{
Ok(r) => r,
Err(e) => { tracing::warn!(error=%e, "no AuthForge interface; skipping signal"); continue; }
};
if let Err(e) = dbus::AuthForge::policy_changed(iface_ref.signal_emitter()).await {
tracing::warn!(error=%e, "PolicyChanged emit failed");
}
}
});
// Keep watcher alive for the daemon's lifetime.
std::mem::forget(watcher);
```
`std::mem::forget(watcher)` is acceptable here — the daemon runs until SIGTERM and `notify::RecommendedWatcher` releases its inotify fd in its Drop. We deliberately leak it for lifetime simplicity. (Phase 5 lockout simulator may revisit.)
**Step 4:** Run `cargo test -p authforge-daemon`. Expected: all prior tests + the new watcher test pass.
**Step 5:** Commit.
```bash
git add daemon/src/storage/policy.rs daemon/src/main.rs
git commit --no-gpg-sign -m "feat(daemon): inotify-based policy watcher emitting PolicyChanged"
```
---
## Task 2.9: `daemon/src/storage/pending.rs` — JSON pending flag CRUD
**Files:**
- Create: `daemon/src/storage/pending.rs`
- Modify: `daemon/src/storage/mod.rs`
**Why:** `/var/lib/authforge/pending/<user>` is a small JSON file with `PendingFlag` content. PAM module (Phase 6) reads this; daemon writes it.
**Step 1:** Add `pub(crate) mod pending;` to `daemon/src/storage/mod.rs`.
**Step 2:** Failing tests + impl in one file:
```rust
use authforge_common::types::PendingFlag;
use std::path::{Path, PathBuf};
use thiserror::Error;
#[derive(Debug, Error)]
pub(crate) enum PendingError {
#[error("io: {0}")]
Io(#[from] std::io::Error),
#[error("invalid username: {0}")]
InvalidUser(String),
#[error("json: {0}")]
Json(#[from] serde_json::Error),
}
pub(crate) struct PendingStore {
dir: PathBuf,
}
impl PendingStore {
pub fn new(dir: PathBuf) -> Self {
Self { dir }
}
fn user_path(&self, user: &str) -> Result<PathBuf, PendingError> {
// Reject anything that could escape the dir or hit a special filename.
if user.is_empty()
|| user.contains('/')
|| user.contains('\0')
|| user == "."
|| user == ".."
{
return Err(PendingError::InvalidUser(user.to_string()));
}
Ok(self.dir.join(user))
}
pub fn set(&self, user: &str, flag: &PendingFlag) -> Result<(), PendingError> {
std::fs::create_dir_all(&self.dir)?;
let path = self.user_path(user)?;
let body = serde_json::to_vec_pretty(flag)?;
std::fs::write(path, body)?;
Ok(())
}
pub fn clear(&self, user: &str) -> Result<bool, PendingError> {
let path = self.user_path(user)?;
match std::fs::remove_file(path) {
Ok(()) => Ok(true),
Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(false),
Err(e) => Err(PendingError::Io(e)),
}
}
pub fn get(&self, user: &str) -> Result<Option<PendingFlag>, PendingError> {
let path = self.user_path(user)?;
match std::fs::read(&path) {
Ok(b) => Ok(Some(serde_json::from_slice(&b)?)),
Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None),
Err(e) => Err(PendingError::Io(e)),
}
}
}
#[cfg(test)]
mod tests {
use super::*;
use authforge_common::types::Method;
use tempfile::tempdir;
fn flag() -> PendingFlag {
PendingFlag {
required_methods: vec![Method::Fido2],
created_unix: 1,
deadline_unix: 0,
re_enroll: false,
}
}
#[test]
fn set_get_clear_roundtrip() {
let d = tempdir().unwrap();
let s = PendingStore::new(d.path().to_path_buf());
assert!(s.get("alice").unwrap().is_none());
s.set("alice", &flag()).unwrap();
assert_eq!(s.get("alice").unwrap().unwrap(), flag());
assert!(s.clear("alice").unwrap());
assert!(s.get("alice").unwrap().is_none());
assert!(!s.clear("alice").unwrap());
}
#[test]
fn rejects_path_traversal() {
let d = tempdir().unwrap();
let s = PendingStore::new(d.path().to_path_buf());
for evil in ["../etc", "a/b", "", ".", "..", "x\0y"] {
assert!(s.set(evil, &flag()).is_err());
assert!(s.get(evil).is_err());
assert!(s.clear(evil).is_err());
}
}
}
```
**Step 3:** Run; PASS.
**Step 4:** Commit.
```bash
git add daemon/src/storage/mod.rs daemon/src/storage/pending.rs
git commit --no-gpg-sign -m "feat(daemon): storage::pending JSON read/write/clear with path-traversal guards"
```
---
## Task 2.10: `daemon/src/storage/credentials.rs` — pam_u2f line parser
**Files:**
- Create: `daemon/src/storage/credentials.rs`
- Modify: `daemon/src/storage/mod.rs`
**Why:** `pam_u2f` reads either `~/.config/Yubico/u2f_keys` (per-user) or a central `authfile` like `/etc/u2f_mappings`. Format per pam_u2f source: each line is `username:credential[:credential...]`, where `credential` is `keyHandle,publicKey,coseType,attributes` (commas, hex-encoded). For Phase 2 we treat each colon-separated chunk after the username as an opaque blob (`pam_u2f` is the source of truth on internal format); we only need split, append, and remove-by-credId where credId = the first comma-separated field.
**Step 1:** `pub(crate) mod credentials;` in `storage/mod.rs`.
**Step 2:** Failing test + impl:
```rust
use thiserror::Error;
#[derive(Debug, Error)]
pub(crate) enum CredsError {
#[error("io: {0}")]
Io(#[from] std::io::Error),
#[error("malformed line for {user}: {reason}")]
Malformed { user: String, reason: &'static str },
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) struct CredEntry {
pub user: String,
pub creds: Vec<String>, // each = keyHandle,publicKey,coseType,attributes
}
impl CredEntry {
pub fn from_line(line: &str) -> Result<Self, CredsError> {
let mut it = line.splitn(2, ':');
let user = it.next().ok_or(CredsError::Malformed {
user: String::new(),
reason: "empty",
})?;
if user.is_empty() {
return Err(CredsError::Malformed { user: String::new(), reason: "empty user" });
}
let rest = it.next().unwrap_or("");
let creds: Vec<String> = if rest.is_empty() {
vec![]
} else {
rest.split(':').map(|s| s.to_string()).collect()
};
Ok(Self { user: user.to_string(), creds })
}
pub fn to_line(&self) -> String {
let mut s = String::with_capacity(self.user.len() + 64);
s.push_str(&self.user);
for c in &self.creds {
s.push(':');
s.push_str(c);
}
s
}
/// Returns the credential ID (the first comma-separated field) of `cred`.
pub fn cred_id(cred: &str) -> &str {
cred.split(',').next().unwrap_or("")
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn parse_user_with_two_credentials() {
let line = "alice:khA,pkA,es256,present:khB,pkB,es256,present";
let e = CredEntry::from_line(line).unwrap();
assert_eq!(e.user, "alice");
assert_eq!(e.creds.len(), 2);
assert_eq!(CredEntry::cred_id(&e.creds[0]), "khA");
}
#[test]
fn parse_user_no_credentials() {
let e = CredEntry::from_line("bob:").unwrap();
assert_eq!(e.user, "bob");
assert!(e.creds.is_empty());
}
#[test]
fn line_roundtrips() {
let line = "alice:khA,pkA,es256,present:khB,pkB,es256,present";
let e = CredEntry::from_line(line).unwrap();
assert_eq!(e.to_line(), line);
}
#[test]
fn rejects_empty_user() {
assert!(CredEntry::from_line(":khA,pkA,es256,present").is_err());
}
}
```
**Step 3:** Run; PASS.
**Step 4:** Commit.
```bash
git add daemon/src/storage/mod.rs daemon/src/storage/credentials.rs
git commit --no-gpg-sign -m "feat(daemon): storage::credentials parses pam_u2f line format"
```
---
## Task 2.11: `CredentialsStore` — append, remove-by-credId, persist
**Files:**
- Modify: `daemon/src/storage/credentials.rs`
**Step 1:** Failing tests:
```rust
#[test]
fn add_then_remove_credential_idempotent() {
let d = tempdir().unwrap();
let path = d.path().join("u2f_keys");
let store = CredentialsStore::new(path.clone());
store.add("alice", "kh1,pk1,es256,present").unwrap();
store.add("alice", "kh2,pk2,es256,present").unwrap();
// Idempotent: adding kh1 again should not duplicate.
store.add("alice", "kh1,pk1-renewed,es256,present").unwrap();
let creds = store.list("alice").unwrap();
assert_eq!(creds.len(), 2);
assert!(creds.iter().any(|c| CredEntry::cred_id(c) == "kh1"));
assert!(creds.iter().any(|c| CredEntry::cred_id(c) == "kh2"));
assert!(store.remove("alice", "kh1").unwrap());
let creds = store.list("alice").unwrap();
assert_eq!(creds.len(), 1);
assert_eq!(CredEntry::cred_id(&creds[0]), "kh2");
// Removing a non-existent credId returns false.
assert!(!store.remove("alice", "kh1").unwrap());
}
#[test]
fn list_for_unknown_user_is_empty() {
let d = tempdir().unwrap();
let store = CredentialsStore::new(d.path().join("u2f_keys"));
assert!(store.list("noone").unwrap().is_empty());
}
```
(`tempfile::tempdir` already in scope.)
**Step 2:** Run, confirm fail.
**Step 3:** Implement on top of existing module:
```rust
pub(crate) struct CredentialsStore {
path: std::path::PathBuf,
}
impl CredentialsStore {
pub fn new(path: std::path::PathBuf) -> Self {
Self { path }
}
fn read_all(&self) -> Result<Vec<CredEntry>, CredsError> {
match std::fs::read_to_string(&self.path) {
Ok(s) => s
.lines()
.filter(|l| !l.trim().is_empty())
.map(CredEntry::from_line)
.collect(),
Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(vec![]),
Err(e) => Err(e.into()),
}
}
fn write_all(&self, entries: &[CredEntry]) -> Result<(), CredsError> {
if let Some(parent) = self.path.parent() {
std::fs::create_dir_all(parent)?;
}
let body = entries
.iter()
.map(CredEntry::to_line)
.collect::<Vec<_>>()
.join("\n");
std::fs::write(&self.path, body)?;
Ok(())
}
pub fn list(&self, user: &str) -> Result<Vec<String>, CredsError> {
Ok(self
.read_all()?
.into_iter()
.find(|e| e.user == user)
.map(|e| e.creds)
.unwrap_or_default())
}
pub fn add(&self, user: &str, cred: &str) -> Result<(), CredsError> {
let new_id = CredEntry::cred_id(cred);
let mut entries = self.read_all()?;
match entries.iter_mut().find(|e| e.user == user) {
Some(entry) => {
if entry.creds.iter().any(|c| CredEntry::cred_id(c) == new_id) {
// Already present; treat as idempotent. (Phase 3 may decide
// to refresh the public key on re-enroll; not Phase 2's job.)
return Ok(());
}
entry.creds.push(cred.to_string());
}
None => entries.push(CredEntry { user: user.to_string(), creds: vec![cred.to_string()] }),
}
self.write_all(&entries)
}
pub fn remove(&self, user: &str, cred_id: &str) -> Result<bool, CredsError> {
let mut entries = self.read_all()?;
let mut changed = false;
if let Some(entry) = entries.iter_mut().find(|e| e.user == user) {
let before = entry.creds.len();
entry.creds.retain(|c| CredEntry::cred_id(c) != cred_id);
changed = entry.creds.len() != before;
}
if changed {
self.write_all(&entries)?;
}
Ok(changed)
}
}
```
**Step 4:** Run; PASS.
**Step 5:** Commit.
```bash
git add daemon/src/storage/credentials.rs
git commit --no-gpg-sign -m "feat(daemon): CredentialsStore add/remove/list against pam_u2f file"
```
---
## Task 2.12: Per-user vs central credential resolution
**Files:**
- Modify: `daemon/src/storage/credentials.rs`
**Why:** Per `Storage.backend`: `PerUser` writes to `~<user>/.config/Yubico/u2f_keys`; `Central` writes all users into `Storage.central_path`. The right choice is dictated by current Policy; daemon resolves the path before calling `CredentialsStore`.
**Step 1:** Failing test:
```rust
#[test]
fn per_user_path_resolution() {
// Without an actual user account, we just verify the path computation
// pivots on the configured backend.
let central = std::path::PathBuf::from("/etc/u2f_mappings");
let resolver = CredsPathResolver::new(authforge_common::policy::Storage {
backend: authforge_common::policy::StorageBackend::Central,
central_path: central.display().to_string(),
});
assert_eq!(resolver.path_for("alice").unwrap(), central);
let resolver = CredsPathResolver::new(authforge_common::policy::Storage {
backend: authforge_common::policy::StorageBackend::PerUser,
central_path: String::new(),
});
let p = resolver.path_for("alice").unwrap();
// We don't have alice in /etc/passwd in CI, so accept either a real
// home-dir-derived path or the documented fallback.
assert!(p.ends_with(".config/Yubico/u2f_keys"));
}
```
**Step 2:** Implement:
```rust
use authforge_common::policy::{Storage, StorageBackend};
pub(crate) struct CredsPathResolver {
storage: Storage,
}
impl CredsPathResolver {
pub fn new(storage: Storage) -> Self {
Self { storage }
}
pub fn path_for(&self, user: &str) -> Result<std::path::PathBuf, CredsError> {
match self.storage.backend {
StorageBackend::Central => Ok(std::path::PathBuf::from(&self.storage.central_path)),
StorageBackend::PerUser => {
// Lookup home dir via getpwnam. Tests against a CI machine without
// the user fall back to /home/<user>/.config/Yubico/u2f_keys —
// pam_u2f does the real lookup at auth time, so the daemon's
// chosen path only matters for *writes* on enrollment.
let home = nix::unistd::User::from_name(user)
.map_err(|e| CredsError::Io(std::io::Error::other(e)))?
.map(|u| u.dir)
.unwrap_or_else(|| std::path::PathBuf::from(format!("/home/{user}")));
Ok(home.join(".config/Yubico/u2f_keys"))
}
}
}
}
```
`nix::unistd::User::from_name` is in the `user` feature — already enabled in workspace deps.
**Step 3:** Run; PASS.
**Step 4:** Commit.
```bash
git add daemon/src/storage/credentials.rs
git commit --no-gpg-sign -m "feat(daemon): CredsPathResolver picks per-user vs central path"
```
---
## Task 2.13: `daemon/src/storage/userdb.rs` — sqlite open + schema
**Files:**
- Create: `daemon/src/storage/userdb.rs`
- Modify: `daemon/src/storage/mod.rs`
**Why:** Lockout simulation (Phase 5) needs a fast lookup of "every user with any enrolled method." Walking `/home/*/.config/Yubico/u2f_keys` at policy-apply time is O(users); a sqlite cache keeps it O(1). The cache is authoritative for *the simulator's input*, not for actual auth — `pam_u2f` reads the file directly at login.
**Step 1:** Add `pub(crate) mod userdb;` to `storage/mod.rs`.
**Step 2:** Create `userdb.rs`:
```rust
use authforge_common::types::Method;
use rusqlite::{params, Connection};
use std::path::Path;
use thiserror::Error;
#[derive(Debug, Error)]
pub(crate) enum UserDbError {
#[error("sqlite: {0}")]
Sqlite(#[from] rusqlite::Error),
}
pub(crate) struct UserDb {
conn: Connection,
}
impl UserDb {
pub fn open(path: &Path) -> Result<Self, UserDbError> {
if let Some(parent) = path.parent() {
std::fs::create_dir_all(parent).ok();
}
let conn = Connection::open(path)?;
Self::init(&conn)?;
Ok(Self { conn })
}
pub fn open_in_memory() -> Result<Self, UserDbError> {
let conn = Connection::open_in_memory()?;
Self::init(&conn)?;
Ok(Self { conn })
}
fn init(conn: &Connection) -> Result<(), UserDbError> {
conn.execute_batch(
r#"
CREATE TABLE IF NOT EXISTS users (
username TEXT PRIMARY KEY,
has_fido2 INTEGER NOT NULL DEFAULT 0,
has_totp INTEGER NOT NULL DEFAULT 0,
last_seen INTEGER NOT NULL DEFAULT 0
) WITHOUT ROWID;
"#,
)?;
Ok(())
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn open_in_memory_creates_schema() {
let db = UserDb::open_in_memory().unwrap();
let row: i64 = db
.conn
.query_row(
"SELECT count(*) FROM sqlite_master WHERE type='table' AND name='users'",
[],
|r| r.get(0),
)
.unwrap();
assert_eq!(row, 1);
}
}
```
**Step 3:** Run; PASS.
**Step 4:** Commit.
```bash
git add daemon/src/storage/mod.rs daemon/src/storage/userdb.rs
git commit --no-gpg-sign -m "feat(daemon): storage::userdb opens sqlite + initializes schema"
```
---
## Task 2.14: `UserDb` — record / query / list
**Files:**
- Modify: `daemon/src/storage/userdb.rs`
**Step 1:** Failing test:
```rust
#[test]
fn record_then_list_users_with_method() {
let db = UserDb::open_in_memory().unwrap();
db.record_enrollment("alice", Method::Fido2).unwrap();
db.record_enrollment("bob", Method::Totp).unwrap();
db.record_enrollment("alice", Method::Totp).unwrap();
let mut fido2 = db.users_with(Method::Fido2).unwrap();
fido2.sort();
assert_eq!(fido2, vec!["alice".to_string()]);
let mut totp = db.users_with(Method::Totp).unwrap();
totp.sort();
assert_eq!(totp, vec!["alice".to_string(), "bob".to_string()]);
assert!(db.has_any("alice").unwrap());
assert!(!db.has_any("nobody").unwrap());
// Drop alice's fido2; still has totp, still in users table.
db.drop_enrollment("alice", Method::Fido2).unwrap();
assert!(db.users_with(Method::Fido2).unwrap().is_empty());
assert!(db.has_any("alice").unwrap());
}
```
**Step 2:** Implement on `impl UserDb`:
```rust
pub fn record_enrollment(&self, user: &str, method: Method) -> Result<(), UserDbError> {
let now = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.map(|d| d.as_secs() as i64)
.unwrap_or(0);
let col = method_col(method);
let sql = format!(
"INSERT INTO users (username, {col}, last_seen) VALUES (?1, 1, ?2)
ON CONFLICT(username) DO UPDATE SET {col} = 1, last_seen = ?2"
);
self.conn.execute(&sql, params![user, now])?;
Ok(())
}
pub fn drop_enrollment(&self, user: &str, method: Method) -> Result<(), UserDbError> {
let col = method_col(method);
let sql = format!("UPDATE users SET {col} = 0 WHERE username = ?1");
self.conn.execute(&sql, params![user])?;
Ok(())
}
pub fn users_with(&self, method: Method) -> Result<Vec<String>, UserDbError> {
let col = method_col(method);
let sql = format!("SELECT username FROM users WHERE {col} = 1");
let mut stmt = self.conn.prepare(&sql)?;
let rows = stmt.query_map([], |r| r.get::<_, String>(0))?;
Ok(rows.filter_map(Result::ok).collect())
}
pub fn has_any(&self, user: &str) -> Result<bool, UserDbError> {
let row: Option<i64> = self
.conn
.query_row(
"SELECT 1 FROM users WHERE username = ?1 AND (has_fido2 = 1 OR has_totp = 1)",
params![user],
|r| r.get(0),
)
.ok();
Ok(row.is_some())
}
}
fn method_col(m: Method) -> &'static str {
match m {
Method::Fido2 => "has_fido2",
Method::Totp => "has_totp",
}
}
```
**Step 3:** Run; PASS.
**Step 4:** Commit.
```bash
git add daemon/src/storage/userdb.rs
git commit --no-gpg-sign -m "feat(daemon): UserDb record/drop/users_with/has_any"
```
---
## Task 2.15: Refactor `AppState` to delegate to storage modules
**Files:**
- Modify: `daemon/src/state.rs`
- Modify: `daemon/src/main.rs`
**Step 1:** Replace `state.rs` with a façade. New shape:
```rust
use crate::storage::{
credentials::{CredEntry, CredentialsStore, CredsPathResolver, CredsError},
pending::{PendingError, PendingStore},
policy::PolicyStore,
userdb::{UserDb, UserDbError},
};
use authforge_common::policy::Policy;
use authforge_common::types::{Credential, Method, PendingFlag, Transport};
use std::path::PathBuf;
use thiserror::Error;
use tokio::sync::Mutex;
#[derive(Debug, Error)]
pub enum StateError {
#[error(transparent)]
Policy(#[from] authforge_common::policy::PolicyError),
#[error(transparent)]
Pending(#[from] PendingError),
#[error(transparent)]
Creds(#[from] CredsError),
#[error(transparent)]
UserDb(#[from] UserDbError),
}
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>,
cfg: StorageConfig,
}
impl AppState {
pub fn open(cfg: StorageConfig) -> Result<Self, StateError> {
let policy = PolicyStore::new(cfg.policy_dir.clone());
let pending = PendingStore::new(cfg.pending_dir.clone());
let userdb = Mutex::new(UserDb::open(&cfg.userdb_path)?);
Ok(Self { policy, pending, userdb, cfg })
}
fn creds_for_current_policy(&self) -> Result<CredsPathResolver, StateError> {
let pol = self.policy.load()?;
Ok(CredsPathResolver::new(pol.storage))
}
pub async fn list_credentials(&self, user: &str) -> Vec<Credential> {
// Phase 2 returns opaque cred IDs; richer Credential metadata (nickname,
// transport) lands when Phase 3 records them at enrollment time. For
// now, surface the keyHandle as both id and nickname so the GUI shows
// *something* and Phase 3 can swap the source.
let resolver = match self.creds_for_current_policy() {
Ok(r) => r,
Err(_) => return vec![],
};
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 add_credential(&self, user: &str, c: Credential) -> Result<(), StateError> {
let resolver = self.creds_for_current_policy()?;
let path = resolver.path_for(user)?;
let store = CredentialsStore::new(path);
// Phase 2 stub: record id-only (Phase 3 fills the full pam_u2f line).
store.add(user, &c.id)?;
let mut db = self.userdb.lock().await;
db.record_enrollment(user, c.method)?;
Ok(())
}
pub async fn remove_credential(&self, user: &str, cred_id: &str) -> Result<bool, StateError> {
let resolver = self.creds_for_current_policy()?;
let path = resolver.path_for(user)?;
let store = CredentialsStore::new(path);
let removed = store.remove(user, cred_id)?;
// If user has no remaining creds, drop the fido2 flag. (Phase 5 simulator
// reads has_any/users_with, so keep the cache honest.)
if removed && store.list(user)?.is_empty() {
self.userdb.lock().await.drop_enrollment(user, Method::Fido2)?;
}
Ok(removed)
}
pub async fn get_policy(&self) -> Result<Policy, StateError> {
Ok(self.policy.load()?)
}
pub async fn set_policy(&self, p: Policy) -> Result<(), StateError> {
self.policy.save(&p)?;
Ok(())
}
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)?)
}
#[cfg(test)]
pub async fn has_pending(&self, user: &str) -> Result<bool, StateError> {
Ok(self.pending.get(user)?.is_some())
}
}
```
**Step 2:** Drop the Phase 1 `with_fixtures()` constructor — fixtures now come from a tempdir in tests. Update `state.rs`'s tests to use `tempdir + StorageConfig`:
```rust
#[cfg(test)]
mod tests {
use super::*;
use tempfile::tempdir;
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 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 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());
}
}
```
**Step 3:** Update `daemon/src/main.rs`:
```rust
let state = Arc::new(state::AppState::open(state::StorageConfig::from_env_or_defaults())?);
```
(replacing the old `with_fixtures()` call.)
**Step 4:** Run `cargo test -p authforge-daemon`.
Expected: state tests + dbus tests still green. The dbus p2p tests previously assumed alice had a fixture cred — UPDATE those tests in Task 2.16 to use a tempdir-backed AppState that gets seeded explicitly.
This task may show the dbus tests failing (red); fix in 2.16. It's acceptable to commit a temporarily-red dbus test suite *only if 2.16 lands immediately after* — pragmatically, finish 2.15 + 2.16 before pushing.
**Step 5:** Commit (with the understood pending-fix on dbus tests in 2.16).
```bash
git add daemon/src/state.rs daemon/src/main.rs
git commit --no-gpg-sign -m "refactor(daemon): AppState delegates to storage modules (fixtures dropped)"
```
---
## Task 2.16: Update dbus tests to use file-backed `AppState`
**Files:**
- Modify: `daemon/src/dbus.rs`
**Step 1:** The `p2p_pair()` helper currently constructs `AppState::with_fixtures()`. Switch to:
```rust
async fn p2p_pair() -> (Connection, Connection, Arc<AppState>, tempfile::TempDir) {
let tmp = tempfile::tempdir().unwrap();
let state = Arc::new(
AppState::open(crate::state::StorageConfig {
policy_dir: tmp.path().join("policy.d"),
pending_dir: tmp.path().join("pending"),
userdb_path: tmp.path().join("users.db"),
})
.unwrap(),
);
// ... rest of the function unchanged.
(server, client, state, tmp)
}
```
The `tempfile::TempDir` must be returned — dropping it at function end would delete the directory before the test runs.
`tempfile` is already a daemon dev-dep.
**Step 2:** Tests that previously relied on alice's fixture credential (`list_credentials_returns_fixture_for_alice`, `remove_own_drops_credential`) need to seed first:
```rust
#[tokio::test]
async fn list_credentials_after_enroll() {
let (_srv, client, state, _tmp) = p2p_pair().await;
// Seed via the same EnrollOwn path the GUI would use.
let p = proxy(&client).await;
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);
}
```
Replace `list_credentials_returns_fixture_for_alice` (delete) and rename test if cleaner. `remove_own_drops_credential` likewise — first call EnrollOwn, capture the returned Credential, then RemoveOwn against `cred.id`.
**Step 3:** Update the dbus interface to match the new error-returning storage. `dbus.rs`'s methods convert `StateError` to `zbus::fdo::Error::Failed(msg)`:
```rust
async fn enroll_own(&self, user: String, nickname: String) -> zbus::fdo::Result<Credential> {
self.authz("io.dangerousthings.AuthForge.enroll-own").await?;
let now = Self::now();
let cred = Credential {
id: format!("stub-{user}-{now}"),
nickname,
method: Method::Fido2,
transport: Transport::Usb,
created_unix: now,
};
self.state
.add_credential(&user, cred.clone())
.await
.map_err(|e| zbus::fdo::Error::Failed(e.to_string()))?;
Ok(cred)
}
```
Apply the same pattern to remove_own, set_policy, set_pending_flag, clear_pending_flag, get_policy.
**Step 4:** Run `cargo test -p authforge-daemon`. Expected: all tests green.
**Step 5:** Run `cargo fmt && cargo clippy --workspace --all-targets -- -D warnings`. Fix any issues.
**Step 6:** Commit.
```bash
git add daemon/src/dbus.rs
git commit --no-gpg-sign -m "test(daemon): dbus tests use tempdir-backed AppState; thread storage errors"
```
---
## Task 2.17: Phase 2 acceptance gate
Verify before declaring Phase 2 complete:
- [ ] `cargo build --workspace --release` succeeds (excludes gui as before).
- [ ] `cargo test -p authforge-common` — all common tests including 4 policy parse tests pass.
- [ ] `cargo test -p authforge-daemon` — all storage + state + dbus tests pass.
- [ ] `cargo clippy --workspace --all-targets -- -D warnings` clean.
- [ ] Manual: drop a `00-base.conf` into `/etc/authforge/policy.d/`, observe daemon emits `PolicyChanged` (`busctl monitor io.dangerousthings.AuthForge`).
- [ ] `debuild -us -uc -b` still produces 5 debs (no packaging regression — Phase 2 doesn't touch `debian/`).
Tag the milestone:
```bash
git tag -a v0.2.0-storage -m "Phase 2: storage layer (policy.d, pending, credentials, userdb)"
```
---
## Risks / known unknowns
| Risk | Mitigation |
|---|---|
| `notify` crate's debouncing — single file write may emit multiple events. | We send `()` ticks via a `watch` channel, which collapses bursts. The signal listener is fine with one or many ticks per change. Reconsider if the GUI shows flicker. |
| `pam_u2f` line format may change between minor versions. | We split on `:` and pass colons-and-commas through opaquely except for the credId field. If pam_u2f extends the comma-fields, we still round-trip the line losslessly. |
| `rusqlite` `bundled` adds ~500 KB to the binary. | Acceptable. Avoids dynamic libsqlite3 pinning across Ubuntu LTS versions. |
| Path traversal in `pending_dir + user`. | Explicit `InvalidUser` rejection for `.`, `..`, `/`, `\0`, empty in `PendingStore::user_path`. Test asserts. |
| Per-user u2f file path needs the user's home — fails for users without `getpwnam` entry (e.g., enrollment in CI). | Fall back to `/home/<user>/.config/Yubico/u2f_keys`. pam_u2f does the real lookup at auth, so the fallback only matters for write-time on a fresh enrollment, where the user does exist. |
| `std::mem::forget(watcher)` in main.rs is a deliberate leak. | Documented; daemon lifetime = process lifetime. Phase 5 may revisit if tests need teardown. |
---
## Execution Handoff
Plan complete and saved to `docs/plans/2026-04-27-phase-2-storage.md`. Phase 2 is broken into 17 bite-sized tasks. Independent file groups (pending / credentials / userdb in tasks 2.9–2.14) can be parallelized via subagents in the same worktree.
Two execution options:
**1. Subagent-Driven (this session)** — fresh subagent per task with two-stage review, parallelize 2.9 / 2.10–2.12 / 2.13–2.14 across 3 agents.
**2. Parallel Session (separate)** — open a new session in a worktree using `superpowers:executing-plans`.
Which approach?
Reference in New Issue View Git Blame Copy Permalink