feat: deprecate `--auth-method`, as both options are available (#279)

* feat: deprecate `--auth-method`, both are avaiable

* send one www-authenticate with two schemes

README.md

@@ -59,7 +59,6 @@ Options:
       --path-prefix <path>   Specify a path prefix
       --hidden <value>       Hide paths from directory listings, separated by `,`
   -a, --auth <rules>         Add auth role
-      --auth-method <value>  Select auth method [default: digest] [possible values: basic, digest]
   -A, --allow-all            Allow all operations
       --allow-upload         Allow upload files/folders
       --allow-delete         Allow delete files/folders
@@ -194,8 +193,8 @@ curl http://127.0.0.1:5000?q=Dockerfile&simple    # search for files, just like
 With authorization
 
 ```
-curl --user user:pass --digest http://192.168.8.10:5000/file  # digest auth
+curl http://192.168.8.10:5000/file --user user:pass                 # basic auth
-curl --user user:pass http://192.168.8.10:5000/file           # basic auth
+curl http://192.168.8.10:5000/file --user user:pass --digest        # digest auth
 ```
 
 <details>
@@ -314,7 +313,6 @@ All options can be set using environment variables prefixed with `DUFS_`.
       --path-prefix <path>    DUFS_PATH_PREFIX=/path
       --hidden <value>        DUFS_HIDDEN=*.log
   -a, --auth <rules>          DUFS_AUTH="admin:admin@/:rw|@/" 
-      --auth-method <value>   DUFS_AUTH_METHOD=basic
   -A, --allow-all             DUFS_ALLOW_ALL=true
       --allow-upload          DUFS_ALLOW_UPLOAD=true
       --allow-delete          DUFS_ALLOW_DELETE=true

src/args.rs

@@ -9,7 +9,6 @@ use std::net::IpAddr;
 use std::path::{Path, PathBuf};
 
 use crate::auth::AccessControl;
-use crate::auth::AuthMethod;
 use crate::log_http::{LogHttp, DEFAULT_LOG_FORMAT};
 #[cfg(feature = "tls")]
 use crate::tls::{load_certs, load_private_key};
@@ -83,6 +82,7 @@ pub fn build_cli() -> Command {
         )
         .arg(
             Arg::new("auth-method")
+                .hide(true)
                 .env("DUFS_AUTH_METHOD")
 				.hide_env(true)
                 .long("auth-method")
@@ -233,7 +233,6 @@ pub struct Args {
     pub path_prefix: String,
     pub uri_prefix: String,
     pub hidden: Vec<String>,
-    pub auth_method: AuthMethod,
     pub auth: AccessControl,
     pub allow_upload: bool,
     pub allow_delete: bool,
@@ -284,10 +283,6 @@ impl Args {
             .get_many::<String>("auth")
             .map(|auth| auth.map(|v| v.as_str()).collect())
             .unwrap_or_default();
-        let auth_method = match matches.get_one::<String>("auth-method").unwrap().as_str() {
-            "basic" => AuthMethod::Basic,
-            _ => AuthMethod::Digest,
-        };
         let auth = AccessControl::new(&auth)?;
         let allow_upload = matches.get_flag("allow-all") || matches.get_flag("allow-upload");
         let allow_delete = matches.get_flag("allow-all") || matches.get_flag("allow-delete");
@@ -329,7 +324,6 @@ impl Args {
             path_prefix,
             uri_prefix,
             hidden,
-            auth_method,
             auth,
             enable_cors,
             allow_delete,

src/auth.rs

@@ -14,7 +14,7 @@ use uuid::Uuid;
 use crate::utils::unix_now;
 
 const REALM: &str = "DUFS";
-const DIGEST_AUTH_TIMEOUT: u32 = 86400;
+const DIGEST_AUTH_TIMEOUT: u32 = 604800; // 7 days
 
 lazy_static! {
     static ref NONCESTARTHASH: Context = {
@@ -89,18 +89,14 @@ impl AccessControl {
         path: &str,
         method: &Method,
         authorization: Option<&HeaderValue>,
-        auth_method: AuthMethod,
     ) -> (Option<String>, Option<AccessPaths>) {
         if let Some(authorization) = authorization {
-            if let Some(user) = auth_method.get_user(authorization) {
+            if let Some(user) = get_auth_user(authorization) {
                 if let Some((pass, paths)) = self.users.get(&user) {
                     if method == Method::OPTIONS {
                         return (Some(user), Some(AccessPaths::new(AccessPerm::ReadOnly)));
                     }
-                    if auth_method
+                    if check_auth(authorization, method.as_str(), &user, pass).is_some() {
-                        .check(authorization, method.as_str(), &user, pass)
-                        .is_some()
-                    {
                         return (Some(user), paths.find(path, !is_readonly_method(method)));
                     } else {
                         return (None, None);
@@ -243,63 +239,37 @@ impl AccessPerm {
     }
 }
 
-fn is_readonly_method(method: &Method) -> bool {
+pub fn www_authenticate() -> Result<HeaderValue> {
-    method == Method::GET
+    let nonce = create_nonce()?;
-        || method == Method::OPTIONS
+    let value = format!(
-        || method == Method::HEAD
+        "Digest realm=\"{}\", nonce=\"{}\", qop=\"auth\", Basic realm=\"{}\"",
-        || method.as_str() == "PROPFIND"
+        REALM, nonce, REALM
+    );
+    Ok(HeaderValue::from_str(&value)?)
 }
 
-#[derive(Debug, Clone)]
+pub fn get_auth_user(authorization: &HeaderValue) -> Option<String> {
-pub enum AuthMethod {
+    if let Some(value) = strip_prefix(authorization.as_bytes(), b"Basic ") {
-    Basic,
+        let value: Vec<u8> = general_purpose::STANDARD.decode(value).ok()?;
-    Digest,
-}
-
-impl AuthMethod {
-    pub fn www_auth(&self) -> Result<String> {
-        match self {
-            AuthMethod::Basic => Ok(format!("Basic realm=\"{REALM}\"")),
-            AuthMethod::Digest => Ok(format!(
-                "Digest realm=\"{}\",nonce=\"{}\",qop=\"auth\"",
-                REALM,
-                create_nonce()?,
-            )),
-        }
-    }
-
-    pub fn get_user(&self, authorization: &HeaderValue) -> Option<String> {
-        match self {
-            AuthMethod::Basic => {
-                let value: Vec<u8> = general_purpose::STANDARD
-                    .decode(strip_prefix(authorization.as_bytes(), b"Basic ")?)
-                    .ok()?;
         let parts: Vec<&str> = std::str::from_utf8(&value).ok()?.split(':').collect();
         Some(parts[0].to_string())
-            }
+    } else if let Some(value) = strip_prefix(authorization.as_bytes(), b"Digest ") {
-            AuthMethod::Digest => {
+        let digest_map = to_headermap(value).ok()?;
-                let digest_value = strip_prefix(authorization.as_bytes(), b"Digest ")?;
+        let username = digest_map.get(b"username".as_ref())?;
-                let digest_map = to_headermap(digest_value).ok()?;
+        std::str::from_utf8(username).map(|v| v.to_string()).ok()
-                digest_map
+    } else {
-                    .get(b"username".as_ref())
+        None
-                    .and_then(|b| std::str::from_utf8(b).ok())
-                    .map(|v| v.to_string())
-            }
     }
 }
 
-    fn check(
+pub fn check_auth(
-        &self,
     authorization: &HeaderValue,
     method: &str,
     auth_user: &str,
     auth_pass: &str,
 ) -> Option<()> {
-        match self {
+    if let Some(value) = strip_prefix(authorization.as_bytes(), b"Basic ") {
-            AuthMethod::Basic => {
+        let basic_value: Vec<u8> = general_purpose::STANDARD.decode(value).ok()?;
-                let basic_value: Vec<u8> = general_purpose::STANDARD
-                    .decode(strip_prefix(authorization.as_bytes(), b"Basic ")?)
-                    .ok()?;
         let parts: Vec<&str> = std::str::from_utf8(&basic_value).ok()?.split(':').collect();
 
         if parts[0] != auth_user {
@@ -311,10 +281,8 @@ impl AuthMethod {
         }
 
         None
-            }
+    } else if let Some(value) = strip_prefix(authorization.as_bytes(), b"Digest ") {
-            AuthMethod::Digest => {
+        let digest_map = to_headermap(value).ok()?;
-                let digest_value = strip_prefix(authorization.as_bytes(), b"Digest ")?;
-                let digest_map = to_headermap(digest_value).ok()?;
         if let (Some(username), Some(nonce), Some(user_response)) = (
             digest_map
                 .get(b"username".as_ref())
@@ -382,8 +350,8 @@ impl AuthMethod {
             }
         }
         None
-            }
+    } else {
-        }
+        None
     }
 }
 
@@ -415,6 +383,13 @@ fn validate_nonce(nonce: &[u8]) -> Result<bool> {
     bail!("invalid nonce");
 }
 
+fn is_readonly_method(method: &Method) -> bool {
+    method == Method::GET
+        || method == Method::OPTIONS
+        || method == Method::HEAD
+        || method.as_str() == "PROPFIND"
+}
+
 fn strip_prefix<'a>(search: &'a [u8], prefix: &[u8]) -> Option<&'a [u8]> {
     let l = prefix.len();
     if search.len() < l {

src/log_http.rs

@@ -1,6 +1,6 @@
-use std::{collections::HashMap, str::FromStr, sync::Arc};
+use std::{collections::HashMap, str::FromStr};
 
-use crate::{args::Args, server::Request};
+use crate::{auth::get_auth_user, server::Request};
 
 pub const DEFAULT_LOG_FORMAT: &str = r#"$remote_addr "$request" $status"#;
 
@@ -17,7 +17,7 @@ enum LogElement {
 }
 
 impl LogHttp {
-    pub fn data(&self, req: &Request, args: &Arc<Args>) -> HashMap<String, String> {
+    pub fn data(&self, req: &Request) -> HashMap<String, String> {
         let mut data = HashMap::default();
         for element in self.elements.iter() {
             match element {
@@ -26,10 +26,8 @@ impl LogHttp {
                         data.insert(name.to_string(), format!("{} {}", req.method(), req.uri()));
                     }
                     "remote_user" => {
-                        if let Some(user) = req
+                        if let Some(user) =
-                            .headers()
+                            req.headers().get("authorization").and_then(get_auth_user)
-                            .get("authorization")
-                            .and_then(|v| args.auth_method.get_user(v))
                         {
                             data.insert(name.to_string(), user);
                         }

src/server.rs

@@ -1,6 +1,6 @@
 #![allow(clippy::too_many_arguments)]
 
-use crate::auth::{AccessPaths, AccessPerm};
+use crate::auth::{www_authenticate, AccessPaths, AccessPerm};
 use crate::streamer::Streamer;
 use crate::utils::{
     decode_uri, encode_uri, get_file_mtime_and_mode, get_file_name, glob, try_get_file_name,
@@ -98,7 +98,7 @@ impl Server {
         let uri = req.uri().clone();
         let assets_prefix = &self.assets_prefix;
         let enable_cors = self.args.enable_cors;
-        let mut http_log_data = self.args.log_http.data(&req, &self.args);
+        let mut http_log_data = self.args.log_http.data(&req);
         if let Some(addr) = addr {
             http_log_data.insert("remote_addr".to_string(), addr.ip().to_string());
         }
@@ -149,12 +149,7 @@ impl Server {
             }
         };
 
-        let guard = self.args.auth.guard(
+        let guard = self.args.auth.guard(&relative_path, &method, authorization);
-            &relative_path,
-            &method,
-            authorization,
-            self.args.auth_method.clone(),
-        );
 
         let (user, access_paths) = match guard {
             (None, None) => {
@@ -1026,9 +1021,9 @@ impl Server {
     }
 
     fn auth_reject(&self, res: &mut Response) -> Result<()> {
-        let value = self.args.auth_method.www_auth()?;
         set_webdav_headers(res);
-        res.headers_mut().insert(WWW_AUTHENTICATE, value.parse()?);
+        res.headers_mut()
+            .append(WWW_AUTHENTICATE, www_authenticate()?);
         // set 401 to make the browser pop up the login box
         *res.status_mut() = StatusCode::UNAUTHORIZED;
         Ok(())
@@ -1061,12 +1056,10 @@ impl Server {
         };
 
         let authorization = headers.get(AUTHORIZATION);
-        let guard = self.args.auth.guard(
+        let guard = self
-            &relative_path,
+            .args
-            req.method(),
+            .auth
-            authorization,
+            .guard(&relative_path, req.method(), authorization);
-            self.args.auth_method.clone(),
-        );
 
         match guard {
             (_, Some(_)) => {}

tests/auth.rs

@@ -125,8 +125,8 @@ fn auth_nest_share(
 }
 
 #[rstest]
-#[case(server(&["--auth", "user:pass@/:rw", "--auth-method", "basic", "-A"]), "user", "pass")]
+#[case(server(&["--auth", "user:pass@/:rw", "-A"]), "user", "pass")]
-#[case(server(&["--auth", "u1:p1@/:rw", "--auth-method", "basic", "-A"]), "u1", "p1")]
+#[case(server(&["--auth", "u1:p1@/:rw", "-A"]), "u1", "p1")]
 fn auth_basic(
     #[case] server: TestServer,
     #[case] user: &str,
@@ -216,7 +216,7 @@ fn no_auth_propfind_dir(
 
 #[rstest]
 fn auth_data(
-    #[with(&["--auth", "user:pass@/:rw|@/", "-A", "--auth-method", "basic"])] server: TestServer,
+    #[with(&["--auth", "user:pass@/:rw|@/", "-A"])] server: TestServer,
 ) -> Result<(), Error> {
     let resp = reqwest::blocking::get(server.url())?;
     let content = resp.text()?;

tests/log_http.rs

@@ -12,7 +12,7 @@ use std::process::{Command, Stdio};
 
 #[rstest]
 #[case(&["-a", "user:pass@/:rw", "--log-format", "$remote_user"], false)]
-#[case(&["-a", "user:pass@/:rw", "--log-format", "$remote_user", "--auth-method", "basic"], true)]
+#[case(&["-a", "user:pass@/:rw", "--log-format", "$remote_user"], true)]
 fn log_remote_user(
     tmpdir: TempDir,
     port: u16,
@@ -41,7 +41,7 @@ fn log_remote_user(
 
     assert_eq!(resp.status(), 200);
 
-    let mut buf = [0; 2000];
+    let mut buf = [0; 4096];
     let buf_len = stdout.read(&mut buf)?;
     let output = std::str::from_utf8(&buf[0..buf_len])?;
 
@@ -69,7 +69,7 @@ fn no_log(tmpdir: TempDir, port: u16, #[case] args: &[&str]) -> Result<(), Error
     let resp = fetch!(b"GET", &format!("http://localhost:{port}")).send()?;
     assert_eq!(resp.status(), 200);
 
-    let mut buf = [0; 1000];
+    let mut buf = [0; 4096];
     let buf_len = stdout.read(&mut buf)?;
     let output = std::str::from_utf8(&buf[0..buf_len])?;