From 43c778182b9750c977b17a82400a848eab5412d6 Mon Sep 17 00:00:00 2001 From: sigoden Date: Sat, 25 Apr 2026 17:28:28 +0800 Subject: [PATCH] fix: tweak auth logic (#689) --- src/auth.rs | 68 ++++++++++++++++++++++++++++++++++----------------- tests/auth.rs | 13 +++++++++- 2 files changed, 58 insertions(+), 23 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index 9a01fd6..b0ff638 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -85,8 +85,14 @@ impl AccessControl { access_paths .merge(paths) .ok_or_else(|| anyhow!("Invalid auth value `{user}:{pass}@{paths}"))?; - if let Some(paths) = annoy_paths { - access_paths.merge(paths); + if let Some(anon_ap) = &anonymous { + let orig_user = access_paths.clone(); + access_paths.absorb_anon( + anon_ap, + &orig_user, + AccessPerm::IndexOnly, + AccessPerm::IndexOnly, + ); } if pass.starts_with("$6$") { use_hashed_password = true; @@ -219,9 +225,8 @@ impl AccessPaths { } pub fn set_perm(&mut self, perm: AccessPerm) { - if self.perm < perm { + if !perm.indexonly() { self.perm = perm; - self.recursively_purge_children(perm); } } @@ -246,17 +251,6 @@ impl AccessPaths { Some(target) } - fn recursively_purge_children(&mut self, perm: AccessPerm) { - self.children.retain(|_, child| { - if child.perm <= perm { - false - } else { - child.recursively_purge_children(perm); - true - } - }); - } - fn add(&mut self, path: &str, perm: AccessPerm) { let path = path.trim_matches('/'); if path.is_empty() { @@ -268,18 +262,48 @@ impl AccessPaths { } fn add_impl(&mut self, parts: &[&str], perm: AccessPerm) { - let parts_len = parts.len(); - if parts_len == 0 { - self.set_perm(perm); - return; - } - if self.perm >= perm { + if parts.is_empty() { + self.perm = perm; return; } let child = self.children.entry(parts[0].to_string()).or_default(); child.add_impl(&parts[1..], perm) } + /// Merge anonymous `AccessPaths` into `self` (a user's paths) with "higher perm wins" semantics. + /// `orig_user` is a snapshot of `self` before any anonymous merging begins, used so that + /// the user's own effective perm is measured against the pre-merge state. + fn absorb_anon( + &mut self, + anon: &AccessPaths, + orig_user: &AccessPaths, + user_inherited: AccessPerm, + anon_inherited: AccessPerm, + ) { + let anon_eff = if !anon.perm.indexonly() { + anon.perm + } else { + anon_inherited + }; + let orig_user_eff = if !orig_user.perm.indexonly() { + orig_user.perm + } else { + user_inherited + }; + + let combined = std::cmp::max(anon_eff, orig_user_eff); + if !combined.indexonly() && combined > self.perm { + self.perm = combined; + } + + let default_ap = AccessPaths::default(); + for (name, anon_child) in &anon.children { + let orig_user_child = orig_user.children.get(name).unwrap_or(&default_ap); + let user_child = self.children.entry(name.clone()).or_default(); + user_child.absorb_anon(anon_child, orig_user_child, orig_user_eff, anon_eff); + } + } + pub fn find(&self, path: &str) -> Option { let parts: Vec<&str> = path .trim_matches('/') @@ -706,7 +730,7 @@ mod tests { ); assert_eq!( paths.find("dir2/dir21/dir211/file"), - Some(AccessPaths::new(AccessPerm::ReadWrite)) + Some(AccessPaths::new(AccessPerm::ReadOnly)) ); assert_eq!( paths.find("dir2/dir22/file"), diff --git a/tests/auth.rs b/tests/auth.rs index 3743c21..1d2e47f 100644 --- a/tests/auth.rs +++ b/tests/auth.rs @@ -366,7 +366,18 @@ fn auth_data( } #[rstest] -fn auth_shadow( +fn auth_precedence( + #[with(&["--auth", "user:pass@/dir1:rw,/dir1/test.txt", "-A"])] server: TestServer, +) -> Result<(), Error> { + let url = format!("{}dir1/test.txt", server.url()); + let resp = send_with_digest_auth(fetch!(b"PUT", &url).body(b"abc".to_vec()), "user", "pass")?; + assert_eq!(resp.status(), 403); + + Ok(()) +} + +#[rstest] +fn auth_anonymous_no_precedence( #[with(&["--auth", "user:pass@/:rw", "-a", "@/dir1", "-A"])] server: TestServer, ) -> Result<(), Error> { let url = format!("{}dir1/test.txt", server.url());