Skip to content

Commit 9abc565

Browse files
committed
Enhance user filtering and testing documentation
- Updated `docs/Improvements.md` to reflect the current testing strategy with checkboxes for completed tests. - Refactored `src/search.rs` to introduce a new shadow status retrieval mechanism, improving user filtering based on account states. - Added tests in `src/app/update.rs` to validate user and group management actions, including restrictions on system users and primary group modifications. - Expanded unit tests in `tests/unit_test.rs` to cover shadow filters and home directory existence checks, ensuring robust filtering functionality.
1 parent d080237 commit 9abc565

File tree

4 files changed

+342
-20
lines changed

4 files changed

+342
-20
lines changed

docs/Improvements.md

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,10 @@ On macOS, the information reported will not be accurate. The tool relies on the
6767
- Search: `fuzzy-matcher` (optional), simple substring by default
6868

6969
### Testing Strategy
70-
- Parser tests using fixture files; property tests for edge cases
71-
- Integration tests driving the update loop with synthetic input events
72-
- Snapshot tests for UI components using known tables
70+
- [x] Parser tests using fixture files
71+
- [x] Integration tests driving the update loop with synthetic input events
72+
- [ ] Property tests for edge cases
73+
- [ ] Snapshot tests for UI components using known tables
7374

7475
### Platform Notes
7576
- Linux/BSD: primary targets. `users` uses libc calls and should honor NSS.
@@ -91,10 +92,12 @@ See the milestone roadmap in [docs/roadmap.md](docs/roadmap.md) for v0.3, v0.4,
9192

9293
### UX & Navigation
9394

94-
- Core CRUD: Create/modify/delete users and groups; lock/unlock; login shell toggle; password set/reset with strength checks
95-
- Search & Filtering: Fuzzy find users/groups; filters (system vs human, inactive, expired, locked, no home, no password)
96-
- Layout & Interactions: Split view (list + detail), breadcrumbs, status bar hints, non‑blocking jobs panel
97-
- Accessibility & Theming: High‑contrast theme, mouse support, resizable panes, configurable keymaps, persistent settings file
95+
- [ ] Core CRUD: Create/modify/delete users and groups; lock/unlock; login shell toggle; password set/reset with strength checks
96+
- Search & Filtering:
97+
- [ ] Fuzzy find users/groups
98+
- [x] Filters (system vs human, inactive, expired, locked, no home, no password)
99+
- [ ] Layout & Interactions: Split view (list + detail), breadcrumbs, status bar hints, non‑blocking jobs panel
100+
- [ ] Accessibility & Theming: High‑contrast theme, mouse support, resizable panes, configurable keymaps, persistent settings file
98101

99102
### Security & Compliance
100103

@@ -144,13 +147,11 @@ User management is inherently high-risk from a security perspective
144147

145148
## Testing Overview
146149

147-
See [docs/testing.md](docs/testing.md) for the complete testing plan, including unit/integration/snapshot tests and safety checks.
148-
149150
Targets:
150151

151-
- Datasets up to 10,000 users/groups
152-
- Incremental search latency under 50 ms on large datasets
153-
- Clear separation of privileged vs non‑privileged flows
152+
- [x] Datasets up to 10,000 users/groups (unit test coverage for search performance)
153+
- [x] Incremental search latency under 50 ms on large datasets (assertions in unit tests)
154+
- [x] Clear separation of privileged vs non‑privileged flows (update-loop tests for SudoPrompt)
154155

155156
## Platform Support
156157

src/app/update.rs

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1962,4 +1962,189 @@ mod tests {
19621962
assert!(app.modal.is_none());
19631963
assert!(matches!(app.input_mode, InputMode::Normal));
19641964
}
1965+
1966+
#[test]
1967+
fn modify_groups_remove_primary_group_shows_info() {
1968+
// Create a user 'alice' with primary_gid 100 and groups including that primary group
1969+
let mut app = AppState::default();
1970+
app.users = vec![crate::sys::SystemUser {
1971+
uid: 1000,
1972+
name: "alice".to_string(),
1973+
primary_gid: 100,
1974+
full_name: None,
1975+
home_dir: "/home/alice".to_string(),
1976+
shell: "/bin/bash".to_string(),
1977+
}];
1978+
app.groups_all = vec![
1979+
crate::sys::SystemGroup { gid: 100, name: "users".to_string(), members: vec![] },
1980+
crate::sys::SystemGroup { gid: 10, name: "wheel".to_string(), members: vec!["alice".to_string()] },
1981+
];
1982+
app.selected_user_index = 0;
1983+
1984+
// Open ModifyGroupsRemove and select the primary group entry (index 0 in the filtered list)
1985+
app.input_mode = InputMode::Modal;
1986+
app.modal = Some(ModalState::ModifyGroupsRemove {
1987+
selected: 0,
1988+
offset: 0,
1989+
selected_multi: Vec::new(),
1990+
});
1991+
1992+
handle_modal_key(&mut app, key(KeyCode::Enter));
1993+
1994+
match &app.modal {
1995+
Some(ModalState::Info { message }) => {
1996+
assert!(message.contains("Cannot remove user from primary group"))
1997+
}
1998+
other => panic!("expected Info modal, got {:?}", other),
1999+
}
2000+
}
2001+
2002+
#[test]
2003+
fn actions_delete_blocked_for_non_user_uid_range() {
2004+
let mut app = AppState::default();
2005+
// Root-like user (UID < 1000) should be blocked
2006+
app.users = vec![crate::sys::SystemUser {
2007+
uid: 0,
2008+
name: "root".to_string(),
2009+
primary_gid: 0,
2010+
full_name: None,
2011+
home_dir: "/root".to_string(),
2012+
shell: "/bin/bash".to_string(),
2013+
}];
2014+
app.selected_user_index = 0;
2015+
app.input_mode = InputMode::Modal;
2016+
app.modal = Some(ModalState::Actions { selected: 1 }); // Delete
2017+
2018+
handle_modal_key(&mut app, key(KeyCode::Enter));
2019+
2020+
match &app.modal {
2021+
Some(ModalState::Info { message }) => {
2022+
assert!(message.contains("Deletion not allowed. Only UID 1000-1999 allowed"));
2023+
assert!(message.contains("root"));
2024+
}
2025+
other => panic!("expected Info modal, got {:?}", other),
2026+
}
2027+
}
2028+
2029+
#[test]
2030+
fn groups_rename_blocked_for_system_gid() {
2031+
let mut app = AppState::default();
2032+
app.groups = vec![
2033+
crate::sys::SystemGroup { gid: 10, name: "wheel".to_string(), members: vec![] },
2034+
crate::sys::SystemGroup { gid: 1000, name: "users".to_string(), members: vec![] },
2035+
];
2036+
app.selected_group_index = 0; // system group
2037+
app.input_mode = InputMode::Modal;
2038+
app.modal = Some(ModalState::GroupModifyMenu { selected: 2, target_gid: None }); // Rename
2039+
2040+
handle_modal_key(&mut app, key(KeyCode::Enter));
2041+
2042+
match &app.modal {
2043+
Some(ModalState::Info { message }) => {
2044+
assert!(message.contains("Renaming system groups is disabled"));
2045+
assert!(message.contains("wheel"));
2046+
assert!(message.contains("10"));
2047+
}
2048+
other => panic!("expected Info modal, got {:?}", other),
2049+
}
2050+
}
2051+
2052+
#[test]
2053+
fn privileged_action_opens_sudo_prompt_without_credentials() {
2054+
// Set up a normal user entry
2055+
let mut app = AppState::default();
2056+
app.users = vec![crate::sys::SystemUser {
2057+
uid: 1000,
2058+
name: "userx".to_string(),
2059+
primary_gid: 1000,
2060+
full_name: None,
2061+
home_dir: "/home/userx".to_string(),
2062+
shell: "/bin/bash".to_string(),
2063+
}];
2064+
app.selected_user_index = 0;
2065+
2066+
// Open ModifyPasswordMenu and choose Reset (selection 1) which requires privileges
2067+
app.input_mode = InputMode::Modal;
2068+
app.modal = Some(ModalState::ModifyPasswordMenu { selected: 1 });
2069+
2070+
handle_modal_key(&mut app, key(KeyCode::Enter));
2071+
2072+
match &app.modal {
2073+
Some(ModalState::SudoPrompt { next, password, error }) => {
2074+
// Should queue the reset action and prompt for sudo
2075+
match next {
2076+
PendingAction::ResetPassword { username } => {
2077+
assert_eq!(username, "userx");
2078+
}
2079+
other => panic!("unexpected pending: {:?}", other),
2080+
}
2081+
assert!(password.is_empty());
2082+
assert!(error.is_none());
2083+
}
2084+
other => panic!("expected SudoPrompt, got {:?}", other),
2085+
}
2086+
}
2087+
2088+
// Test-only helper: simulate effects of a subset of PendingAction without system calls
2089+
fn simulate_pending_action(app: &mut AppState, pending: PendingAction) {
2090+
match pending {
2091+
PendingAction::DeleteUser { username, delete_home: _ } => {
2092+
app.users_all.retain(|u| u.name != username);
2093+
app.users_all.sort_by_key(|u| u.uid);
2094+
apply_filters_and_search(app);
2095+
if app.selected_user_index >= app.users.len() {
2096+
app.selected_user_index = app.users.len().saturating_sub(1);
2097+
}
2098+
}
2099+
PendingAction::DeleteGroup { groupname } => {
2100+
app.groups_all.retain(|g| g.name != groupname);
2101+
app.groups_all.sort_by_key(|g| g.gid);
2102+
apply_filters_and_search(app);
2103+
if app.selected_group_index >= app.groups.len() {
2104+
app.selected_group_index = app.groups.len().saturating_sub(1);
2105+
}
2106+
}
2107+
_ => {}
2108+
}
2109+
}
2110+
2111+
#[test]
2112+
fn selected_user_index_clamps_after_delete() {
2113+
let mut app = AppState::default();
2114+
app.users_all = vec![
2115+
crate::sys::SystemUser { uid: 1000, name: "a".into(), primary_gid: 1000, full_name: None, home_dir: "/home/a".into(), shell: "/bin/bash".into() },
2116+
crate::sys::SystemUser { uid: 1001, name: "b".into(), primary_gid: 1001, full_name: None, home_dir: "/home/b".into(), shell: "/bin/bash".into() },
2117+
];
2118+
app.users = app.users_all.clone();
2119+
app.selected_user_index = 1; // last item
2120+
2121+
simulate_pending_action(
2122+
&mut app,
2123+
PendingAction::DeleteUser { username: "b".into(), delete_home: false },
2124+
);
2125+
2126+
assert_eq!(app.users.len(), 1);
2127+
assert_eq!(app.selected_user_index, 0);
2128+
assert_eq!(app.users[0].name, "a");
2129+
}
2130+
2131+
#[test]
2132+
fn selected_group_index_clamps_after_delete() {
2133+
let mut app = AppState::default();
2134+
app.groups_all = vec![
2135+
crate::sys::SystemGroup { gid: 1000, name: "g1".into(), members: vec![] },
2136+
crate::sys::SystemGroup { gid: 1001, name: "g2".into(), members: vec![] },
2137+
];
2138+
app.groups = app.groups_all.clone();
2139+
app.selected_group_index = 1; // last item
2140+
2141+
simulate_pending_action(
2142+
&mut app,
2143+
PendingAction::DeleteGroup { groupname: "g2".into() },
2144+
);
2145+
2146+
assert_eq!(app.groups.len(), 1);
2147+
assert_eq!(app.selected_group_index, 0);
2148+
assert_eq!(app.groups[0].name, "g1");
2149+
}
19652150
}

src/search.rs

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
//! based on the current input mode and query string.
55
//!
66
use crate::app::{AppState, GroupsFilter, InputMode, UsersFilter};
7+
use std::collections::HashMap;
78

89
/// Filter the visible users or groups of `app` according to the lowercase query.
910
///
@@ -42,7 +43,7 @@ pub fn apply_filters_and_search(app: &mut AppState) {
4243
}
4344
// System-backed filters via /etc/shadow (best-effort; ignored if unreadable)
4445
if chips.locked || chips.no_password || chips.expired {
45-
if let Some(shadow) = read_shadow_status().ok() {
46+
if let Some(shadow) = get_shadow_status().ok() {
4647
if chips.locked {
4748
users_view.retain(|u| shadow.get(&u.name).map(|s| s.locked).unwrap_or(false));
4849
}
@@ -99,14 +100,14 @@ pub fn apply_filters_and_search(app: &mut AppState) {
99100
}
100101

101102
// Lightweight shadow status used for filters
102-
struct ShadowStatus {
103-
locked: bool,
104-
no_password: bool,
105-
expired: bool,
103+
#[derive(Clone, Debug)]
104+
pub struct ShadowStatus {
105+
pub locked: bool,
106+
pub no_password: bool,
107+
pub expired: bool,
106108
}
107109

108-
fn read_shadow_status() -> std::io::Result<std::collections::HashMap<String, ShadowStatus>> {
109-
use std::collections::HashMap;
110+
fn read_shadow_status() -> std::io::Result<HashMap<String, ShadowStatus>> {
110111
use std::fs;
111112
use std::os::unix::fs::MetadataExt;
112113
use std::time::{SystemTime, UNIX_EPOCH};
@@ -153,6 +154,35 @@ fn read_shadow_status() -> std::io::Result<std::collections::HashMap<String, Sha
153154
Ok(map)
154155
}
155156

157+
fn get_shadow_status() -> std::io::Result<HashMap<String, ShadowStatus>> {
158+
if let Some(res) = SHADOW_PROVIDER.with(|p| p.borrow().as_ref().map(|f| f())) {
159+
return res;
160+
}
161+
read_shadow_status()
162+
}
163+
164+
thread_local! {
165+
static SHADOW_PROVIDER: std::cell::RefCell<Option<Box<dyn Fn() -> std::io::Result<HashMap<String, ShadowStatus>>>>> = std::cell::RefCell::new(None);
166+
}
167+
168+
#[allow(dead_code)]
169+
pub fn set_shadow_provider<F>(f: F)
170+
where
171+
F: Fn() -> std::io::Result<HashMap<String, ShadowStatus>> + 'static,
172+
{
173+
SHADOW_PROVIDER.with(|p| *p.borrow_mut() = Some(Box::new(f)));
174+
}
175+
176+
#[allow(dead_code)]
177+
pub fn clear_shadow_provider() {
178+
SHADOW_PROVIDER.with(|p| *p.borrow_mut() = None);
179+
}
180+
181+
#[allow(dead_code)]
182+
pub fn make_shadow_status(locked: bool, no_password: bool, expired: bool) -> ShadowStatus {
183+
ShadowStatus { locked, no_password, expired }
184+
}
185+
156186
#[cfg(test)]
157187
mod tests {
158188
use super::*;

0 commit comments

Comments
 (0)