diff --git a/src/auth/src/user_provider.rs b/src/auth/src/user_provider.rs index dce1e11429..dd3580ed9a 100644 --- a/src/auth/src/user_provider.rs +++ b/src/auth/src/user_provider.rs @@ -68,11 +68,15 @@ pub trait UserProvider: Send + Sync { /// Key is username, value is (password, permission_mode) pub type UserInfoMap = HashMap, PermissionMode)>; -fn load_credential_from_file(filepath: &str) -> Result> { +fn load_credential_from_file(filepath: &str) -> Result { // check valid path let path = Path::new(filepath); if !path.exists() { - return Ok(None); + return InvalidConfigSnafu { + value: filepath.to_string(), + msg: "UserProvider file must exist", + } + .fail(); } ensure!( @@ -109,7 +113,7 @@ fn load_credential_from_file(filepath: &str) -> Result> { } ); - Ok(Some(credential)) + Ok(credential) } /// Parse a line of credential in the format of `username=password` or `username:permission_mode=password` diff --git a/src/auth/src/user_provider/static_user_provider.rs b/src/auth/src/user_provider/static_user_provider.rs index 82a3d2c7b1..50cd3ab366 100644 --- a/src/auth/src/user_provider/static_user_provider.rs +++ b/src/auth/src/user_provider/static_user_provider.rs @@ -35,11 +35,7 @@ impl StaticUserProvider { })?; match mode { "file" => { - let users = load_credential_from_file(content)? - .context(InvalidConfigSnafu { - value: content.to_string(), - msg: "StaticFileUserProvider must be a valid file path", - })?; + let users = load_credential_from_file(content)?; Ok(StaticUserProvider { users }) } "cmd" => content diff --git a/src/auth/src/user_provider/watch_file_user_provider.rs b/src/auth/src/user_provider/watch_file_user_provider.rs index 2824b09d77..4df17502b7 100644 --- a/src/auth/src/user_provider/watch_file_user_provider.rs +++ b/src/auth/src/user_provider/watch_file_user_provider.rs @@ -22,17 +22,17 @@ use notify::{EventKind, RecursiveMode, Watcher}; use snafu::{ResultExt, ensure}; use crate::error::{FileWatchSnafu, InvalidConfigSnafu, Result}; -use crate::user_info::DefaultUserInfo; use crate::user_provider::{UserInfoMap, authenticate_with_credential, load_credential_from_file}; use crate::{Identity, Password, UserInfoRef, UserProvider}; pub(crate) const WATCH_FILE_USER_PROVIDER: &str = "watch_file_user_provider"; -type WatchedCredentialRef = Arc>>; +type WatchedCredentialRef = Arc>; /// A user provider that reads user credential from a file and watches the file for changes. /// -/// Empty file is invalid; but file not exist means every user can be authenticated. +/// Both empty file and non-existent file are invalid and will cause initialization to fail. +#[derive(Debug)] pub(crate) struct WatchFileUserProvider { users: WatchedCredentialRef, } @@ -107,16 +107,7 @@ impl UserProvider for WatchFileUserProvider { async fn authenticate(&self, id: Identity<'_>, password: Password<'_>) -> Result { let users = self.users.lock().expect("users credential must be valid"); - if let Some(users) = users.as_ref() { - authenticate_with_credential(users, id, password) - } else { - match id { - Identity::UserId(id, _) => { - warn!(id, "User provider file not exist, allow all users"); - Ok(DefaultUserInfo::with_name(id)) - } - } - } + authenticate_with_credential(&users, id, password) } async fn authorize(&self, _: &str, _: &str, _: &UserInfoRef) -> Result<()> { @@ -177,6 +168,21 @@ pub mod test { } } + #[tokio::test] + async fn test_file_provider_initialization_with_missing_file() { + common_telemetry::init_default_ut_logging(); + + let dir = create_temp_dir("test_missing_file"); + let file_path = format!("{}/non_existent_file", dir.path().to_str().unwrap()); + + // Try to create provider with non-existent file should fail + let result = WatchFileUserProvider::new(file_path.as_str()); + assert!(result.is_err()); + + let error = result.unwrap_err(); + assert!(error.to_string().contains("UserProvider file must exist")); + } + #[tokio::test] async fn test_file_provider() { common_telemetry::init_default_ut_logging(); @@ -201,9 +207,10 @@ pub mod test { // remove the tmp file assert!(std::fs::remove_file(&file_path).is_ok()); - test_authenticate(&provider, "root", "123456", true, Some(timeout)).await; + // When file is deleted during runtime, keep the last known good credentials test_authenticate(&provider, "root", "654321", true, Some(timeout)).await; - test_authenticate(&provider, "admin", "654321", true, Some(timeout)).await; + test_authenticate(&provider, "root", "123456", false, Some(timeout)).await; + test_authenticate(&provider, "admin", "654321", false, Some(timeout)).await; // recreate the tmp file assert!(std::fs::write(&file_path, "root=123456\n").is_ok());