Skip to content

Commit

Permalink
feat: higher perm auth path shadows lower one (#521)
Browse files Browse the repository at this point in the history 
In `/:rw;/path1:ro`, the `/:rw` have higher perms, it shadow `/path1:ro`, make `/path1` granted read-write perms.
  • Loading branch information
@sigoden
sigoden authored Jan 2, 2025
1 parent af95ea1 commit e576ddc
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 33 deletions.
87 changes: 61 additions & 26 deletions src/auth.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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));
}
}
}
Expand All @@ -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)
Expand All @@ -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);
}
}

Expand All @@ -169,6 +175,25 @@ impl AccessPaths {
Some(())
}

pub fn guard(&self, path: &str, method: &Method) -> Option<Self> {
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() {
Expand All @@ -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<AccessPaths> {
pub fn find(&self, path: &str) -> Option<AccessPaths> {
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<AccessPaths> {
Expand Down Expand Up @@ -232,20 +256,20 @@ impl AccessPaths {
self.children.keys().collect()
}

pub fn child_paths(&self, base: &Path) -> Vec<PathBuf> {
pub fn entry_paths(&self, base: &Path) -> Vec<PathBuf> {
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<PathBuf>, base: &Path) {
fn entry_paths_impl(&self, output: &mut Vec<PathBuf>, 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)
}
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -603,19 +627,30 @@ mod tests {
.collect::<Vec<_>>()
)
);
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"),
Some(AccessPaths::new(AccessPerm::ReadWrite))
);
assert_eq!(
paths.find("dir2/dir21/file", true),
paths.find("dir2/dir21/dir211/file"),
Some(AccessPaths::new(AccessPerm::ReadWrite))
);
assert_eq!(
paths.find("dir2/dir21/dir211/file", false),
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))
);
}
}
4 changes: 2 additions & 2 deletions src/server.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,7 @@ impl Server {
let access_paths = access_paths.clone();
let search_paths = tokio::task::spawn_blocking(move || {
let mut paths: Vec<PathBuf> = 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() {
Expand Down Expand Up @@ -1604,7 +1604,7 @@ async fn zip_dir<W: AsyncWrite + Unpin>(
let dir_clone = dir.to_path_buf();
let zip_paths = tokio::task::spawn_blocking(move || {
let mut paths: Vec<PathBuf> = 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() {
Expand Down
9 changes: 4 additions & 5 deletions tests/auth.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down

0 comments on commit e576ddc

Please sign in to comment.