From 89beac94eb719f8febb6ec0bd6aa6442e68b5a88 Mon Sep 17 00:00:00 2001 From: Calvin Lee Date: Fri, 20 Sep 2024 15:19:10 +0000 Subject: [PATCH 1/4] Add `password_command for sasl and `nick_password` This commit builds off of #552 and allows the user to supply a command instead of a password file anywhere that a password file is used. If the password command ends unsuccessfully, then an error is displayed. Trailing whitespace is now trimmed from password commands, as it is commonplace for commands to leave a trailing newline after the password they output. Resolves #580 --- book/src/configuration/servers/README.md | 8 +++ data/src/config.rs | 18 ++++- data/src/config/server.rs | 5 ++ data/src/server.rs | 88 +++++++++++++++--------- src/main.rs | 41 ++++++----- 5 files changed, 106 insertions(+), 54 deletions(-) diff --git a/book/src/configuration/servers/README.md b/book/src/configuration/servers/README.md index dfd46067..c54b7a29 100644 --- a/book/src/configuration/servers/README.md +++ b/book/src/configuration/servers/README.md @@ -40,6 +40,14 @@ Read nick_password from the file at the given path.[^1] - **values**: any string - **default**: not set +## `nick_password_command` + +Executes the command with `sh` (or equivalent) and reads `nickname_password` as the output. + +- **type**: string +- **values**: any string +- **default**: not set + ## `nick_identify_syntax` The server's NICKSERV IDENTIFY syntax. diff --git a/data/src/config.rs b/data/src/config.rs index bc8ff08c..5a7dc366 100644 --- a/data/src/config.rs +++ b/data/src/config.rs @@ -1,5 +1,5 @@ use std::path::PathBuf; -use std::string; +use std::{string, str}; use tokio_stream::wrappers::ReadDirStream; use tokio_stream::StreamExt; @@ -306,16 +306,30 @@ fn default_tooltip() -> bool { pub enum Error { #[error("config could not be read: {0}")] Read(String), + #[error("command could not be run: {0}")] + Execute(String), #[error("{0}")] Io(String), #[error("{0}")] Parse(String), #[error("UTF8 parsing error: {0}")] - UI(#[from] string::FromUtf8Error), + StrUtf8Error(#[from] str::Utf8Error), + #[error("UTF8 parsing error: {0}")] + StringUtf8Error(#[from] string::FromUtf8Error), #[error(transparent)] LoadSounds(#[from] audio::LoadError), } +impl Error { + pub fn is_expected(&self) -> bool { + match self { + // If a user doesn't have a config when we start up, then we end up with a read error + Error::Read(_) => true, + _ => false, + } + } +} + impl From for Error { fn from(error: std::io::Error) -> Self { Self::Io(error.to_string()) diff --git a/data/src/config/server.rs b/data/src/config/server.rs index 6ea05ee6..77ae3325 100644 --- a/data/src/config/server.rs +++ b/data/src/config/server.rs @@ -15,6 +15,8 @@ pub struct Server { pub nick_password: Option, /// The client's NICKSERV password file. pub nick_password_file: Option, + /// The client's NICKSERV password command. + pub nick_password_command: Option, /// The server's NICKSERV IDENTIFY syntax. pub nick_identify_syntax: Option, /// Alternative nicknames for the client, if the default is taken. @@ -143,6 +145,7 @@ impl Default for Server { nickname: Default::default(), nick_password: Default::default(), nick_password_file: Default::default(), + nick_password_command: Default::default(), nick_identify_syntax: Default::default(), alt_nicks: Default::default(), username: Default::default(), @@ -189,6 +192,8 @@ pub enum Sasl { password: Option, /// Account password file password_file: Option, + /// Account password command + password_command: Option, }, External { /// The path to PEM encoded X509 user certificate for external auth diff --git a/data/src/server.rs b/data/src/server.rs index 753b4d53..85d9751a 100644 --- a/data/src/server.rs +++ b/data/src/server.rs @@ -1,5 +1,5 @@ use std::collections::BTreeMap; -use std::fmt; +use std::{fmt, str}; use tokio::fs; use tokio::process::Command; @@ -12,6 +12,9 @@ use crate::config::server::Sasl; use crate::config::Error; pub type Handle = Sender; +const DUP_PASS_MSG: &str = "Only one of password, password_file and password_command can be set."; +const DUP_NICK_PASS_MSG: &str = "Only one of nick_password, nick_password_file and nick_password_command can be set."; +const DUP_SASL_PASS_MSG: &str = "Exactly one of sasl.plain.password, sasl.plain.password_file or sasl.plain.password_command must be set."; #[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] pub struct Server(String); @@ -52,6 +55,29 @@ impl<'a> From<(&'a Server, &'a config::Server)> for Entry { #[derive(Debug, Clone, Default, Deserialize)] pub struct Map(BTreeMap); +async fn read_from_command(pass_command: &str) -> Result { + let output = if cfg!(target_os = "windows") { + Command::new("cmd") + .arg("/C") + .arg(pass_command) + .output() + .await? + } else { + Command::new("sh") + .arg("-c") + .arg(pass_command) + .output() + .await? + }; + if output.status.success() { + // we remove trailing whitespace, which might be present from unix pipelines with a + // trailing newline + Ok(str::from_utf8(&output.stdout)?.trim_end().to_string()) + } else { + Err(Error::Execute(String::from_utf8(output.stderr)?)) + } +} + impl Map { pub fn insert(&mut self, name: Server, server: config::Server) { self.0.insert(name, server); @@ -77,62 +103,62 @@ impl Map { for (_, config) in self.0.iter_mut() { if let Some(pass_file) = &config.password_file { if config.password.is_some() || config.password_command.is_some() { - return Err(Error::Parse( - "Only one of password, password_file and password_command can be set." - .to_string(), - )); + return Err(Error::Parse(DUP_PASS_MSG.to_string())); } let pass = fs::read_to_string(pass_file).await?; config.password = Some(pass); } if let Some(pass_command) = &config.password_command { if config.password.is_some() { - return Err(Error::Parse( - "Only one of password, password_file and password_command can be set." - .to_string(), - )); + return Err(Error::Parse(DUP_PASS_MSG.to_string())); } - let output = if cfg!(target_os = "windows") { - Command::new("cmd") - .args(["/C", pass_command]) - .output() - .await? - } else { - Command::new("sh") - .arg("-c") - .arg(pass_command) - .output() - .await? - }; - config.password = Some(String::from_utf8(output.stdout)?); + config.password = Some(read_from_command(pass_command).await?); } if let Some(nick_pass_file) = &config.nick_password_file { - if config.nick_password.is_some() { - return Err(Error::Parse( - "Only one of nick_password and nick_password_file can be set.".to_string(), - )); + if config.nick_password.is_some() || config.nick_password_command.is_some() { + return Err(Error::Parse(DUP_NICK_PASS_MSG.to_string())); } let nick_pass = fs::read_to_string(nick_pass_file).await?; config.nick_password = Some(nick_pass); } + if let Some(nick_pass_command) = &config.nick_password_command { + if config.password.is_some() { + return Err(Error::Parse(DUP_NICK_PASS_MSG.to_string())); + } + config.password = Some(read_from_command(nick_pass_command).await?); + } if let Some(sasl) = &mut config.sasl { match sasl { Sasl::Plain { password: Some(_), - password_file: Some(_), + password_file: None, + password_command: None, .. - } => { - return Err(Error::Parse("Exactly one of sasl.plain.password or sasl.plain.password_file must be set.".to_string())); - } + } => {}, Sasl::Plain { password: password @ None, password_file: Some(pass_file), + password_command: None, .. } => { let pass = fs::read_to_string(pass_file).await?; *password = Some(pass); } - _ => {} + Sasl::Plain { + password: password @ None, + password_file: None, + password_command: Some(pass_command), + .. + } => { + let pass = read_from_command(pass_command).await?; + *password = Some(pass); + } + Sasl::Plain { .. } => { + return Err(Error::Parse(DUP_SASL_PASS_MSG.to_string())); + } + Sasl::External { .. } => { + // no passwords to read + } } } } diff --git a/src/main.rs b/src/main.rs index 51122f27..37314185 100644 --- a/src/main.rs +++ b/src/main.rs @@ -151,29 +151,28 @@ impl Halloy { command.map(Message::Dashboard), ) } - Err(error) => match &error { - config::Error::Parse(_) | config::Error::LoadSounds(_) => ( - Screen::Help(screen::Help::new(error)), - Config::default(), - Task::none(), - ), - _ => { + Err(error) => { + if config::has_yaml_config() { // If we have a YAML file, but end up in this arm // it means the user tried to load Halloy with a YAML configuration, but it expected TOML. - if config::has_yaml_config() { - ( - Screen::Migration(screen::Migration::new()), - Config::default(), - Task::none(), - ) - } else { - // Otherwise, show regular welcome screen for new users. - ( - Screen::Welcome(screen::Welcome::new()), - Config::default(), - Task::none(), - ) - } + ( + Screen::Migration(screen::Migration::new()), + Config::default(), + Task::none(), + ) + } else if error.is_expected() { + // Show regular welcome screen for new users. + ( + Screen::Welcome(screen::Welcome::new()), + Config::default(), + Task::none(), + ) + } else { + ( + Screen::Help(screen::Help::new(error)), + Config::default(), + Task::none(), + ) } }, }; From b2c69be0332e7767474f927f8e907e9f09caaf48 Mon Sep 17 00:00:00 2001 From: Calvin Lee Date: Mon, 23 Sep 2024 12:33:40 +0000 Subject: [PATCH 2/4] change naming and add CHANGELOG entry --- CHANGELOG.md | 7 +++++++ data/src/config.rs | 10 +++++----- data/src/server.rs | 2 +- src/main.rs | 2 +- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e3efead0..993c00d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,11 @@ # Unreleased +- New configuration options + - Ability to define a shell command for loading a NICKSERV password. See [configuration](https://halloy.squidowl.org/configuration/servers/index.html#password_command) + - Ability to define a shell command for loading a SASL password. See [configuration](https://halloy.squidowl.org/configuration/servers/sasl/plain.html) + +Fixed: + +- Errors from password commands are now caught and displayed to the user. # 2024.12 (2024-09-17) diff --git a/data/src/config.rs b/data/src/config.rs index 5a7dc366..165dc5b7 100644 --- a/data/src/config.rs +++ b/data/src/config.rs @@ -161,7 +161,7 @@ impl Config { let path = Self::path(); let content = fs::read_to_string(path) .await - .map_err(|e| Error::Read(e.to_string()))?; + .map_err(|e| Error::LoadConfigFile(e.to_string()))?; let Configuration { theme, @@ -305,9 +305,9 @@ fn default_tooltip() -> bool { #[derive(Debug, Error, Clone)] pub enum Error { #[error("config could not be read: {0}")] - Read(String), + LoadConfigFile(String), #[error("command could not be run: {0}")] - Execute(String), + ExecutePasswordCommand(String), #[error("{0}")] Io(String), #[error("{0}")] @@ -321,10 +321,10 @@ pub enum Error { } impl Error { - pub fn is_expected(&self) -> bool { + pub fn is_expected_on_first_load(&self) -> bool { match self { // If a user doesn't have a config when we start up, then we end up with a read error - Error::Read(_) => true, + Error::LoadConfigFile(_) => true, _ => false, } } diff --git a/data/src/server.rs b/data/src/server.rs index 85d9751a..a5511f5d 100644 --- a/data/src/server.rs +++ b/data/src/server.rs @@ -74,7 +74,7 @@ async fn read_from_command(pass_command: &str) -> Result { // trailing newline Ok(str::from_utf8(&output.stdout)?.trim_end().to_string()) } else { - Err(Error::Execute(String::from_utf8(output.stderr)?)) + Err(Error::ExecutePasswordCommand(String::from_utf8(output.stderr)?)) } } diff --git a/src/main.rs b/src/main.rs index 37314185..04038edc 100644 --- a/src/main.rs +++ b/src/main.rs @@ -160,7 +160,7 @@ impl Halloy { Config::default(), Task::none(), ) - } else if error.is_expected() { + } else if error.is_expected_on_first_load() { // Show regular welcome screen for new users. ( Screen::Welcome(screen::Welcome::new()), From 2e7e8cec2d25912768eb0fbf3120f03d90fa2070 Mon Sep 17 00:00:00 2001 From: Calvin Lee Date: Tue, 24 Sep 2024 13:20:12 +0000 Subject: [PATCH 3/4] fix typo in CHANGELOG --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 993c00d1..d179bc37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Unreleased - New configuration options - - Ability to define a shell command for loading a NICKSERV password. See [configuration](https://halloy.squidowl.org/configuration/servers/index.html#password_command) + - Ability to define a shell command for loading a NICKSERV password. See [configuration](https://halloy.squidowl.org/configuration/servers/index.html#nick_password_command) - Ability to define a shell command for loading a SASL password. See [configuration](https://halloy.squidowl.org/configuration/servers/sasl/plain.html) Fixed: From 2e8a3751544c89c973f64cf0eee034bca7f35cd2 Mon Sep 17 00:00:00 2001 From: Calvin Lee Date: Thu, 26 Sep 2024 13:32:51 +0000 Subject: [PATCH 4/4] Use error variants instead of static strings --- data/src/config.rs | 25 +++++++++++++------------ data/src/server.rs | 13 +++++-------- src/main.rs | 46 ++++++++++++++++++++++------------------------ 3 files changed, 40 insertions(+), 44 deletions(-) diff --git a/data/src/config.rs b/data/src/config.rs index 165dc5b7..ec9d3d75 100644 --- a/data/src/config.rs +++ b/data/src/config.rs @@ -159,6 +159,9 @@ impl Config { } let path = Self::path(); + if !path.try_exists()? { + return Err(Error::ConfigMissing { has_yaml_config: has_yaml_config()? }); + } let content = fs::read_to_string(path) .await .map_err(|e| Error::LoadConfigFile(e.to_string()))?; @@ -294,8 +297,8 @@ pub fn random_nickname_with_seed(rng: &mut R) -> String { } /// Has YAML configuration file. -pub fn has_yaml_config() -> bool { - config_dir().join("config.yaml").exists() +fn has_yaml_config() -> Result { + Ok(config_dir().join("config.yaml").try_exists()?) } fn default_tooltip() -> bool { @@ -318,16 +321,14 @@ pub enum Error { StringUtf8Error(#[from] string::FromUtf8Error), #[error(transparent)] LoadSounds(#[from] audio::LoadError), -} - -impl Error { - pub fn is_expected_on_first_load(&self) -> bool { - match self { - // If a user doesn't have a config when we start up, then we end up with a read error - Error::LoadConfigFile(_) => true, - _ => false, - } - } + #[error("Only one of password, password_file and password_command can be set.")] + DuplicatePassword, + #[error("Only one of nick_password, nick_password_file and nick_password_command can be set.")] + DuplicateNickPassword, + #[error("Exactly one of sasl.plain.password, sasl.plain.password_file or sasl.plain.password_command must be set.")] + DuplicateSaslPassword, + #[error("Config does not exist")] + ConfigMissing { has_yaml_config: bool }, } impl From for Error { diff --git a/data/src/server.rs b/data/src/server.rs index a5511f5d..6aef893f 100644 --- a/data/src/server.rs +++ b/data/src/server.rs @@ -12,9 +12,6 @@ use crate::config::server::Sasl; use crate::config::Error; pub type Handle = Sender; -const DUP_PASS_MSG: &str = "Only one of password, password_file and password_command can be set."; -const DUP_NICK_PASS_MSG: &str = "Only one of nick_password, nick_password_file and nick_password_command can be set."; -const DUP_SASL_PASS_MSG: &str = "Exactly one of sasl.plain.password, sasl.plain.password_file or sasl.plain.password_command must be set."; #[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] pub struct Server(String); @@ -103,27 +100,27 @@ impl Map { for (_, config) in self.0.iter_mut() { if let Some(pass_file) = &config.password_file { if config.password.is_some() || config.password_command.is_some() { - return Err(Error::Parse(DUP_PASS_MSG.to_string())); + return Err(Error::DuplicatePassword); } let pass = fs::read_to_string(pass_file).await?; config.password = Some(pass); } if let Some(pass_command) = &config.password_command { if config.password.is_some() { - return Err(Error::Parse(DUP_PASS_MSG.to_string())); + return Err(Error::DuplicatePassword); } config.password = Some(read_from_command(pass_command).await?); } if let Some(nick_pass_file) = &config.nick_password_file { if config.nick_password.is_some() || config.nick_password_command.is_some() { - return Err(Error::Parse(DUP_NICK_PASS_MSG.to_string())); + return Err(Error::DuplicateNickPassword); } let nick_pass = fs::read_to_string(nick_pass_file).await?; config.nick_password = Some(nick_pass); } if let Some(nick_pass_command) = &config.nick_password_command { if config.password.is_some() { - return Err(Error::Parse(DUP_NICK_PASS_MSG.to_string())); + return Err(Error::DuplicateNickPassword); } config.password = Some(read_from_command(nick_pass_command).await?); } @@ -154,7 +151,7 @@ impl Map { *password = Some(pass); } Sasl::Plain { .. } => { - return Err(Error::Parse(DUP_SASL_PASS_MSG.to_string())); + return Err(Error::DuplicateSaslPassword); } Sasl::External { .. } => { // no passwords to read diff --git a/src/main.rs b/src/main.rs index 04038edc..59bad6a4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -151,30 +151,28 @@ impl Halloy { command.map(Message::Dashboard), ) } - Err(error) => { - if config::has_yaml_config() { - // If we have a YAML file, but end up in this arm - // it means the user tried to load Halloy with a YAML configuration, but it expected TOML. - ( - Screen::Migration(screen::Migration::new()), - Config::default(), - Task::none(), - ) - } else if error.is_expected_on_first_load() { - // Show regular welcome screen for new users. - ( - Screen::Welcome(screen::Welcome::new()), - Config::default(), - Task::none(), - ) - } else { - ( - Screen::Help(screen::Help::new(error)), - Config::default(), - Task::none(), - ) - } - }, + // If we have a YAML file, but end up in this arm + // it means the user tried to load Halloy with a YAML configuration, but it expected TOML. + Err(config::Error::ConfigMissing { + has_yaml_config: true, + }) => ( + Screen::Migration(screen::Migration::new()), + Config::default(), + Task::none(), + ), + // Show regular welcome screen for new users. + Err(config::Error::ConfigMissing { + has_yaml_config: false, + }) => ( + Screen::Welcome(screen::Welcome::new()), + Config::default(), + Task::none(), + ), + Err(error) => ( + Screen::Help(screen::Help::new(error)), + Config::default(), + Task::none(), + ), }; (