From 77f86a4c60b19472a69cf03f81cb2fa1d66a9cdf Mon Sep 17 00:00:00 2001 From: sigoden Date: Thu, 21 Dec 2023 17:28:13 +0800 Subject: [PATCH] fix: auth precedence (#325) --- src/auth.rs | 74 ++++++++++++++++++++++++++++++++------------------- src/server.rs | 6 ++--- tests/auth.rs | 19 +++++++++++++ 3 files changed, 68 insertions(+), 31 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index 6f87e0b..78ba65a 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -47,10 +47,7 @@ impl AccessControl { if raw_rules.is_empty() { return Ok(Default::default()); } - let new_raw_rules = compact_split_rules(raw_rules); - if new_raw_rules.len() != raw_rules.len() { - eprintln!("Warning: deprecate the use of `|` to separate auth rules.") - } + let new_raw_rules = split_rules(raw_rules); let mut use_hashed_password = false; let create_err = |v: &str| anyhow!("Invalid auth `{v}`"); let mut anony = None; @@ -194,7 +191,11 @@ impl AccessPaths { } fn find_impl(&self, parts: &[&str], perm: AccessPerm) -> Option { - let perm = self.perm.max(perm); + let perm = if !self.perm.indexonly() { + self.perm + } else { + perm + }; if parts.is_empty() { if perm.indexonly() { return Some(self.clone()); @@ -215,24 +216,24 @@ impl AccessPaths { child.find_impl(&parts[1..], perm) } - pub fn child_paths(&self) -> Vec<&String> { + pub fn child_names(&self) -> Vec<&String> { self.children.keys().collect() } - pub fn leaf_paths(&self, base: &Path) -> Vec { + pub fn child_paths(&self, base: &Path) -> Vec { if !self.perm().indexonly() { return vec![base.to_path_buf()]; } let mut output = vec![]; - self.leaf_paths_impl(&mut output, base); + self.child_paths_impl(&mut output, base); output } - fn leaf_paths_impl(&self, output: &mut Vec, base: &Path) { + fn child_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.leaf_paths_impl(output, &base); + child.child_paths_impl(output, &base); } else { output.push(base) } @@ -489,8 +490,7 @@ fn split_account_paths(s: &str) -> Option<(&str, &str)> { Some((&s[0..i], &s[i + 1..])) } -/// Compatible with deprecated usage of `|` for role separation -fn compact_split_rules(rules: &[&str]) -> Vec { +fn split_rules(rules: &[&str]) -> Vec { let mut output = vec![]; for rule in rules { let parts: Vec<&str> = rule.split('|').collect(); @@ -540,15 +540,15 @@ mod tests { #[test] fn test_compact_split_rules() { assert_eq!( - compact_split_rules(&["user1:pass1@/:rw|user2:pass2@/:rw"]), + split_rules(&["user1:pass1@/:rw|user2:pass2@/:rw"]), ["user1:pass1@/:rw", "user2:pass2@/:rw"] ); assert_eq!( - compact_split_rules(&["user1:pa|ss1@/:rw|user2:pa|ss2@/:rw"]), + split_rules(&["user1:pa|ss1@/:rw|user2:pa|ss2@/:rw"]), ["user1:pa|ss1@/:rw", "user2:pa|ss2@/:rw"] ); assert_eq!( - compact_split_rules(&["user1:pa|ss1@/:rw|@/"]), + split_rules(&["user1:pa|ss1@/:rw|@/"]), ["user1:pa|ss1@/:rw", "@/"] ); } @@ -557,16 +557,18 @@ mod tests { fn test_access_paths() { let mut paths = AccessPaths::default(); paths.add("/dir1", AccessPerm::ReadWrite); - paths.add("/dir2/dir1", AccessPerm::ReadWrite); - paths.add("/dir2/dir2", AccessPerm::ReadOnly); - paths.add("/dir2/dir3/dir1", AccessPerm::ReadWrite); + paths.add("/dir2/dir21", AccessPerm::ReadWrite); + paths.add("/dir2/dir21/dir211", AccessPerm::ReadOnly); + paths.add("/dir2/dir22", AccessPerm::ReadOnly); + paths.add("/dir2/dir22/dir221", AccessPerm::ReadWrite); + paths.add("/dir2/dir23/dir231", AccessPerm::ReadWrite); assert_eq!( - paths.leaf_paths(Path::new("/tmp")), + paths.child_paths(Path::new("/tmp")), [ "/tmp/dir1", - "/tmp/dir2/dir1", - "/tmp/dir2/dir2", - "/tmp/dir2/dir3/dir1" + "/tmp/dir2/dir21", + "/tmp/dir2/dir22", + "/tmp/dir2/dir23/dir231", ] .iter() .map(PathBuf::from) @@ -575,16 +577,32 @@ mod tests { assert_eq!( paths .find("dir2", false) - .map(|v| v.leaf_paths(Path::new("/tmp/dir2"))), + .map(|v| v.child_paths(Path::new("/tmp/dir2"))), Some( - ["/tmp/dir2/dir1", "/tmp/dir2/dir2", "/tmp/dir2/dir3/dir1"] - .iter() - .map(PathBuf::from) - .collect::>() + [ + "/tmp/dir2/dir21", + "/tmp/dir2/dir22", + "/tmp/dir2/dir23/dir231" + ] + .iter() + .map(PathBuf::from) + .collect::>() ) ); assert_eq!(paths.find("dir2", true), None); - assert!(paths.find("dir1/file", true).is_some()); + assert_eq!( + paths.find("dir1/file", true), + Some(AccessPaths::new(AccessPerm::ReadWrite)) + ); + assert_eq!( + paths.find("dir2/dir21/file", true), + Some(AccessPaths::new(AccessPerm::ReadWrite)) + ); + assert_eq!( + paths.find("dir2/dir21/dir211/file", false), + Some(AccessPaths::new(AccessPerm::ReadOnly)) + ); + assert_eq!(paths.find("dir2/dir21/dir211/file", true), None); } #[test] diff --git a/src/server.rs b/src/server.rs index 7d7e208..8c426c6 100644 --- a/src/server.rs +++ b/src/server.rs @@ -514,7 +514,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.leaf_paths(&path_buf) { + for dir in access_paths.child_paths(&path_buf) { let mut it = WalkDir::new(&dir).into_iter(); it.next(); while let Some(Ok(entry)) = it.next() { @@ -1184,7 +1184,7 @@ impl Server { ) -> Result> { let mut paths: Vec = vec![]; if access_paths.perm().indexonly() { - for name in access_paths.child_paths() { + for name in access_paths.child_names() { let entry_path = entry_path.join(name); self.add_pathitem(&mut paths, base_path, &entry_path).await; } @@ -1465,7 +1465,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.leaf_paths(&dir_clone) { + for dir in access_paths.child_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 81e4d2a..e290916 100644 --- a/tests/auth.rs +++ b/tests/auth.rs @@ -282,3 +282,22 @@ fn auth_data( assert_eq!(json["allow_upload"], serde_json::Value::Bool(true)); Ok(()) } + +#[rstest] +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 = fetch!(b"PUT", &url) + .body(b"abc".to_vec()) + .send_with_digest_auth("user", "pass")?; + assert_eq!(resp.status(), 403); + + let url = format!("{}dir1/file1", server.url()); + let resp = fetch!(b"PUT", &url) + .body(b"abc".to_vec()) + .send_with_digest_auth("user", "pass")?; + assert_eq!(resp.status(), 201); + + Ok(()) +}