From e576ddcbea57c03e3d8ae0168416d688411ded4c Mon Sep 17 00:00:00 2001 From: sigoden Date: Thu, 2 Jan 2025 09:00:28 +0800 Subject: [PATCH] feat: higher perm auth path shadows lower one (#521) In `/:rw;/path1:ro`, the `/:rw` have higher perms, it shadow `/path1:ro`, make `/path1` granted read-write perms. --- src/auth.rs | 87 ++++++++++++++++++++++++++++++++++++--------------- src/server.rs | 4 +-- tests/auth.rs | 9 +++--- 3 files changed, 67 insertions(+), 33 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index 02e965c..fae91bf 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -69,15 +69,20 @@ impl AccessControl { let mut anonymous = None; if let Some(paths) = annoy_paths { let mut access_paths = AccessPaths::default(); - access_paths.merge(paths); + access_paths + .merge(paths) + .ok_or_else(|| anyhow!("Invalid auth value `@{paths}"))?; anonymous = Some(access_paths); } let mut users = IndexMap::new(); for (user, pass, paths) in account_paths_pairs.into_iter() { - let mut access_paths = anonymous.clone().unwrap_or_default(); + let mut access_paths = AccessPaths::default(); access_paths .merge(paths) - .ok_or_else(|| anyhow!("Invalid auth `{user}:{pass}@{paths}"))?; + .ok_or_else(|| anyhow!("Invalid auth value `{user}:{pass}@{paths}"))?; + if let Some(paths) = annoy_paths { + access_paths.merge(paths); + } if pass.starts_with("$6$") { use_hashed_password = true; } @@ -107,12 +112,12 @@ impl AccessControl { } if let Some(authorization) = authorization { if let Some(user) = get_auth_user(authorization) { - if let Some((pass, paths)) = self.users.get(&user) { + if let Some((pass, ap)) = self.users.get(&user) { if method == Method::OPTIONS { return (Some(user), Some(AccessPaths::new(AccessPerm::ReadOnly))); } if check_auth(authorization, method.as_str(), &user, pass).is_some() { - return (Some(user), paths.find(path, !is_readonly_method(method))); + return (Some(user), ap.guard(path, method)); } } } @@ -124,8 +129,8 @@ impl AccessControl { return (None, Some(AccessPaths::new(AccessPerm::ReadOnly))); } - if let Some(paths) = self.anonymous.as_ref() { - return (None, paths.find(path, !is_readonly_method(method))); + if let Some(ap) = self.anonymous.as_ref() { + return (None, ap.guard(path, method)); } (None, None) @@ -151,8 +156,9 @@ impl AccessPaths { } pub fn set_perm(&mut self, perm: AccessPerm) { - if !perm.indexonly() { + if self.perm < perm { self.perm = perm; + self.recursively_purge_children(perm); } } @@ -169,6 +175,25 @@ impl AccessPaths { Some(()) } + pub fn guard(&self, path: &str, method: &Method) -> Option { + let target = self.find(path)?; + if !is_readonly_method(method) && !target.perm().readwrite() { + return None; + } + 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() { @@ -185,21 +210,20 @@ impl AccessPaths { self.set_perm(perm); return; } + if self.perm >= perm { + return; + } let child = self.children.entry(parts[0].to_string()).or_default(); child.add_impl(&parts[1..], perm) } - pub fn find(&self, path: &str, writable: bool) -> Option { + pub fn find(&self, path: &str) -> Option { let parts: Vec<&str> = path .trim_matches('/') .split('/') .filter(|v| !v.is_empty()) .collect(); - let target = self.find_impl(&parts, self.perm)?; - if writable && !target.perm().readwrite() { - return None; - } - Some(target) + self.find_impl(&parts, self.perm) } fn find_impl(&self, parts: &[&str], perm: AccessPerm) -> Option { @@ -232,20 +256,20 @@ impl AccessPaths { self.children.keys().collect() } - pub fn child_paths(&self, base: &Path) -> Vec { + pub fn entry_paths(&self, base: &Path) -> Vec { if !self.perm().indexonly() { return vec![base.to_path_buf()]; } let mut output = vec![]; - self.child_paths_impl(&mut output, base); + self.entry_paths_impl(&mut output, base); output } - fn child_paths_impl(&self, output: &mut Vec, base: &Path) { + fn entry_paths_impl(&self, output: &mut Vec, base: &Path) { for (name, child) in self.children.iter() { let base = base.join(name); if child.perm().indexonly() { - child.child_paths_impl(output, &base); + child.entry_paths_impl(output, &base); } else { output.push(base) } @@ -577,7 +601,7 @@ mod tests { paths.add("/dir2/dir22/dir221", AccessPerm::ReadWrite); paths.add("/dir2/dir23/dir231", AccessPerm::ReadWrite); assert_eq!( - paths.child_paths(Path::new("/tmp")), + paths.entry_paths(Path::new("/tmp")), [ "/tmp/dir1", "/tmp/dir2/dir21", @@ -590,8 +614,8 @@ mod tests { ); assert_eq!( paths - .find("dir2", false) - .map(|v| v.child_paths(Path::new("/tmp/dir2"))), + .find("dir2") + .map(|v| v.entry_paths(Path::new("/tmp/dir2"))), Some( [ "/tmp/dir2/dir21", @@ -603,19 +627,30 @@ mod tests { .collect::>() ) ); - assert_eq!(paths.find("dir2", true), None); assert_eq!( - paths.find("dir1/file", true), + paths.find("dir1/file"), Some(AccessPaths::new(AccessPerm::ReadWrite)) ); assert_eq!( - paths.find("dir2/dir21/file", true), + paths.find("dir2/dir21/file"), Some(AccessPaths::new(AccessPerm::ReadWrite)) ); assert_eq!( - paths.find("dir2/dir21/dir211/file", false), + paths.find("dir2/dir21/dir211/file"), + Some(AccessPaths::new(AccessPerm::ReadWrite)) + ); + assert_eq!( + paths.find("dir2/dir22/file"), Some(AccessPaths::new(AccessPerm::ReadOnly)) ); - assert_eq!(paths.find("dir2/dir21/dir211/file", true), None); + assert_eq!( + paths.find("dir2/dir22/dir221/file"), + Some(AccessPaths::new(AccessPerm::ReadWrite)) + ); + assert_eq!(paths.find("dir2/dir23/file"), None); + assert_eq!( + paths.find("dir2/dir23//dir231/file"), + Some(AccessPaths::new(AccessPerm::ReadWrite)) + ); } } diff --git a/src/server.rs b/src/server.rs index 5c404ca..2c59a3a 100644 --- a/src/server.rs +++ b/src/server.rs @@ -596,7 +596,7 @@ impl Server { let access_paths = access_paths.clone(); let search_paths = tokio::task::spawn_blocking(move || { let mut paths: Vec = vec![]; - for dir in access_paths.child_paths(&path_buf) { + for dir in access_paths.entry_paths(&path_buf) { let mut it = WalkDir::new(&dir).into_iter(); it.next(); while let Some(Ok(entry)) = it.next() { @@ -1604,7 +1604,7 @@ async fn zip_dir( let dir_clone = dir.to_path_buf(); let zip_paths = tokio::task::spawn_blocking(move || { let mut paths: Vec = vec![]; - for dir in access_paths.child_paths(&dir_clone) { + for dir in access_paths.entry_paths(&dir_clone) { let mut it = WalkDir::new(&dir).into_iter(); it.next(); while let Some(Ok(entry)) = it.next() { diff --git a/tests/auth.rs b/tests/auth.rs index 4b0e7e0..c2de81d 100644 --- a/tests/auth.rs +++ b/tests/auth.rs @@ -336,14 +336,13 @@ fn auth_data( } #[rstest] -fn auth_precedence( - #[with(&["--auth", "user:pass@/dir1:rw,/dir1/test.txt", "-A"])] server: TestServer, +fn auth_shadow( + #[with(&["--auth", "user:pass@/:rw", "-a", "@/dir1", "-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); + let resp = fetch!(b"PUT", &url).body(b"abc".to_vec()).send()?; + assert_eq!(resp.status(), 401); - let url = format!("{}dir1/file1", server.url()); let resp = send_with_digest_auth(fetch!(b"PUT", &url).body(b"abc".to_vec()), "user", "pass")?; assert_eq!(resp.status(), 201);