From b65dfff574f44e5a60db3903d2226f4dbe386b7a Mon Sep 17 00:00:00 2001 From: Dietmar Maurer Date: Thu, 9 Sep 2021 13:14:28 +0200 Subject: [PATCH] cleanup User configuration: use Updater --- pbs-api-types/src/lib.rs | 5 +- pbs-api-types/src/user.rs | 7 +- src/api2/access/openid.rs | 5 +- src/api2/access/tfa.rs | 4 +- src/api2/access/user.rs | 191 +++++++++++------------------- src/config/cached_user_info.rs | 3 +- src/config/tfa.rs | 4 +- src/config/user.rs | 14 ++- src/server/email_notifications.rs | 6 +- 9 files changed, 90 insertions(+), 149 deletions(-) diff --git a/pbs-api-types/src/lib.rs b/pbs-api-types/src/lib.rs index 59bb6f6e..03f184ea 100644 --- a/pbs-api-types/src/lib.rs +++ b/pbs-api-types/src/lib.rs @@ -61,10 +61,7 @@ pub use userid::{PROXMOX_GROUP_ID_SCHEMA, PROXMOX_TOKEN_ID_SCHEMA, PROXMOX_TOKEN #[macro_use] mod user; -pub use user::{ApiToken, User, UserWithTokens}; -pub use user::{ - EMAIL_SCHEMA, ENABLE_USER_SCHEMA, EXPIRE_USER_SCHEMA, FIRST_NAME_SCHEMA, LAST_NAME_SCHEMA, -}; +pub use user::*; pub mod upid; pub use upid::UPID; diff --git a/pbs-api-types/src/user.rs b/pbs-api-types/src/user.rs index 9111ccea..8a7480ad 100644 --- a/pbs-api-types/src/user.rs +++ b/pbs-api-types/src/user.rs @@ -1,7 +1,9 @@ use serde::{Deserialize, Serialize}; use proxmox::api::api; -use proxmox::api::schema::{BooleanSchema, IntegerSchema, Schema, StringSchema}; +use proxmox::api::schema::{ + BooleanSchema, IntegerSchema, Schema, StringSchema, Updater, +}; use super::{SINGLE_LINE_COMMENT_FORMAT, SINGLE_LINE_COMMENT_SCHEMA}; use super::userid::{Authid, Userid, PROXMOX_TOKEN_ID_SCHEMA}; @@ -171,9 +173,10 @@ impl ApiToken { }, } )] -#[derive(Serialize,Deserialize)] +#[derive(Serialize,Deserialize,Updater)] /// User properties. pub struct User { + #[updater(skip)] pub userid: Userid, #[serde(skip_serializing_if="Option::is_none")] pub comment: Option, diff --git a/src/api2/access/openid.rs b/src/api2/access/openid.rs index a0e1b193..99636e95 100644 --- a/src/api2/access/openid.rs +++ b/src/api2/access/openid.rs @@ -12,18 +12,17 @@ use proxmox::{identity, sortable}; use proxmox_openid::{OpenIdAuthenticator, OpenIdConfig}; +use pbs_api_types::{Userid, User, REALM_ID_SCHEMA}; use pbs_buildcfg::PROXMOX_BACKUP_RUN_DIR_M; use pbs_tools::auth::private_auth_key; use pbs_tools::ticket::Ticket; use pbs_config::domains::{OpenIdUserAttribute, OpenIdRealmConfig}; use crate::server::ticket::ApiTicket; - use crate::config::cached_user_info::CachedUserInfo; use pbs_config::open_backup_lockfile; -use crate::api2::types::*; use crate::auth_helpers::*; fn openid_authenticator(realm_config: &OpenIdRealmConfig, redirect_url: &str) -> Result { @@ -119,7 +118,7 @@ pub fn openid_login( if config.autocreate.unwrap_or(false) { use crate::config::user; let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?; - let user = user::User { + let user = User { userid: user_id.clone(), comment: None, enable: None, diff --git a/src/api2/access/tfa.rs b/src/api2/access/tfa.rs index 3f46f769..8b0166c6 100644 --- a/src/api2/access/tfa.rs +++ b/src/api2/access/tfa.rs @@ -7,7 +7,7 @@ use proxmox::api::{api, Permission, Router, RpcEnvironment}; use proxmox::tools::tfa::totp::Totp; use proxmox::{http_bail, http_err}; -use pbs_api_types::{Authid, Userid, PASSWORD_SCHEMA, PRIV_PERMISSIONS_MODIFY, PRIV_SYS_AUDIT}; +use pbs_api_types::{Authid, Userid, User, PASSWORD_SCHEMA, PRIV_PERMISSIONS_MODIFY, PRIV_SYS_AUDIT}; use crate::config::cached_user_info::CachedUserInfo; use crate::config::tfa::{TfaInfo, TfaUserData}; @@ -37,7 +37,7 @@ fn tfa_update_auth( let (config, _digest) = crate::config::user::config()?; if config - .lookup::("user", userid.as_str()) + .lookup::("user", userid.as_str()) .is_err() { http_bail!(UNAUTHORIZED, "user '{}' does not exists.", userid); diff --git a/src/api2/access/user.rs b/src/api2/access/user.rs index 2d26cdcc..6a2fe83c 100644 --- a/src/api2/access/user.rs +++ b/src/api2/access/user.rs @@ -7,25 +7,18 @@ use std::collections::HashMap; use proxmox::api::{api, ApiMethod, Router, RpcEnvironment, Permission}; use proxmox::api::router::SubdirMap; -use proxmox::api::schema::{Schema, StringSchema}; use pbs_api_types::{ - PASSWORD_FORMAT, PROXMOX_CONFIG_DIGEST_SCHEMA, SINGLE_LINE_COMMENT_SCHEMA, Authid, - Tokenname, UserWithTokens, Userid, PRIV_SYS_AUDIT, PRIV_PERMISSIONS_MODIFY, + PROXMOX_CONFIG_DIGEST_SCHEMA, SINGLE_LINE_COMMENT_SCHEMA, Authid, + Tokenname, UserWithTokens, Userid, User, UserUpdater, ApiToken, + ENABLE_USER_SCHEMA, EXPIRE_USER_SCHEMA, PBS_PASSWORD_SCHEMA, + PRIV_SYS_AUDIT, PRIV_PERMISSIONS_MODIFY, }; use pbs_config::token_shadow; -use pbs_config::open_backup_lockfile; -use crate::config::user; use crate::config::cached_user_info::CachedUserInfo; -pub const PBS_PASSWORD_SCHEMA: Schema = StringSchema::new("User Password.") - .format(&PASSWORD_FORMAT) - .min_length(5) - .max_length(64) - .schema(); - -fn new_user_with_tokens(user: user::User) -> UserWithTokens { +fn new_user_with_tokens(user: User) -> UserWithTokens { UserWithTokens { userid: user.userid, comment: user.comment, @@ -66,7 +59,7 @@ pub fn list_users( mut rpcenv: &mut dyn RpcEnvironment, ) -> Result, Error> { - let (config, digest) = user::config()?; + let (config, digest) = crate::config::user::config()?; let auth_id: Authid = rpcenv .get_auth_id() @@ -80,23 +73,23 @@ pub fn list_users( let top_level_privs = user_info.lookup_privs(&auth_id, &["access", "users"]); let top_level_allowed = (top_level_privs & PRIV_SYS_AUDIT) != 0; - let filter_by_privs = |user: &user::User| { + let filter_by_privs = |user: &User| { top_level_allowed || user.userid == *userid }; - let list:Vec = config.convert_to_typed_array("user")?; + let list:Vec = config.convert_to_typed_array("user")?; rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into(); let iter = list.into_iter().filter(filter_by_privs); let list = if include_tokens { - let tokens: Vec = config.convert_to_typed_array("token")?; + let tokens: Vec = config.convert_to_typed_array("token")?; let mut user_to_tokens = tokens .into_iter() .fold( HashMap::new(), - |mut map: HashMap>, token: user::ApiToken| { + |mut map: HashMap>, token: ApiToken| { if token.tokenid.is_token() { map .entry(token.tokenid.user().clone()) @@ -106,7 +99,7 @@ pub fn list_users( map }); iter - .map(|user: user::User| { + .map(|user: User| { let mut user = new_user_with_tokens(user); user.tokens = user_to_tokens.remove(&user.userid).unwrap_or_default(); user @@ -124,37 +117,14 @@ pub fn list_users( protected: true, input: { properties: { - userid: { - type: Userid, - }, - comment: { - schema: SINGLE_LINE_COMMENT_SCHEMA, - optional: true, + config: { + type: User, + flatten: true, }, password: { schema: PBS_PASSWORD_SCHEMA, optional: true, }, - enable: { - schema: user::ENABLE_USER_SCHEMA, - optional: true, - }, - expire: { - schema: user::EXPIRE_USER_SCHEMA, - optional: true, - }, - firstname: { - schema: user::FIRST_NAME_SCHEMA, - optional: true, - }, - lastname: { - schema: user::LAST_NAME_SCHEMA, - optional: true, - }, - email: { - schema: user::EMAIL_SCHEMA, - optional: true, - }, }, }, access: { @@ -164,28 +134,26 @@ pub fn list_users( /// Create new user. pub fn create_user( password: Option, - param: Value, + config: User, rpcenv: &mut dyn RpcEnvironment ) -> Result<(), Error> { - let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?; + let _lock = crate::config::user::lock_config()?; - let user: user::User = serde_json::from_value(param)?; + let (mut section_config, _digest) = crate::config::user::config()?; - let (mut config, _digest) = user::config()?; - - if config.sections.get(user.userid.as_str()).is_some() { - bail!("user '{}' already exists.", user.userid); + if section_config.sections.get(config.userid.as_str()).is_some() { + bail!("user '{}' already exists.", config.userid); } - config.set_data(user.userid.as_str(), "user", &user)?; + section_config.set_data(config.userid.as_str(), "user", &config)?; - let realm = user.userid.realm(); + let realm = config.userid.realm(); // Fails if realm does not exist! let authenticator = crate::auth::lookup_authenticator(realm)?; - user::save_config(&config)?; + crate::config::user::save_config(§ion_config)?; if let Some(password) = password { let user_info = CachedUserInfo::new()?; @@ -193,7 +161,7 @@ pub fn create_user( if realm == "pam" && !user_info.is_superuser(¤t_auth_id) { bail!("only superuser can edit pam credentials!"); } - authenticator.store_password(user.userid.name(), &password)?; + authenticator.store_password(config.userid.name(), &password)?; } Ok(()) @@ -207,7 +175,7 @@ pub fn create_user( }, }, }, - returns: { type: user::User }, + returns: { type: User }, access: { permission: &Permission::Or(&[ &Permission::Privilege(&["access", "users"], PRIV_SYS_AUDIT, false), @@ -216,8 +184,8 @@ pub fn create_user( }, )] /// Read user configuration data. -pub fn read_user(userid: Userid, mut rpcenv: &mut dyn RpcEnvironment) -> Result { - let (config, digest) = user::config()?; +pub fn read_user(userid: Userid, mut rpcenv: &mut dyn RpcEnvironment) -> Result { + let (config, digest) = crate::config::user::config()?; let user = config.lookup("user", userid.as_str())?; rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into(); Ok(user) @@ -245,34 +213,14 @@ pub enum DeletableProperty { userid: { type: Userid, }, - comment: { - optional: true, - schema: SINGLE_LINE_COMMENT_SCHEMA, + update: { + type: UserUpdater, + flatten: true, }, password: { schema: PBS_PASSWORD_SCHEMA, optional: true, }, - enable: { - schema: user::ENABLE_USER_SCHEMA, - optional: true, - }, - expire: { - schema: user::EXPIRE_USER_SCHEMA, - optional: true, - }, - firstname: { - schema: user::FIRST_NAME_SCHEMA, - optional: true, - }, - lastname: { - schema: user::LAST_NAME_SCHEMA, - optional: true, - }, - email: { - schema: user::EMAIL_SCHEMA, - optional: true, - }, delete: { description: "List of properties to delete.", type: Array, @@ -298,28 +246,23 @@ pub enum DeletableProperty { #[allow(clippy::too_many_arguments)] pub fn update_user( userid: Userid, - comment: Option, - enable: Option, - expire: Option, + update: UserUpdater, password: Option, - firstname: Option, - lastname: Option, - email: Option, delete: Option>, digest: Option, rpcenv: &mut dyn RpcEnvironment, ) -> Result<(), Error> { - let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?; + let _lock = crate::config::user::lock_config()?; - let (mut config, expected_digest) = user::config()?; + let (mut config, expected_digest) = crate::config::user::config()?; if let Some(ref digest) = digest { let digest = proxmox::tools::hex_to_digest(digest)?; crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?; } - let mut data: user::User = config.lookup("user", userid.as_str())?; + let mut data: User = config.lookup("user", userid.as_str())?; if let Some(delete) = delete { for delete_prop in delete { @@ -332,7 +275,7 @@ pub fn update_user( } } - if let Some(comment) = comment { + if let Some(comment) = update.comment { let comment = comment.trim().to_string(); if comment.is_empty() { data.comment = None; @@ -341,11 +284,11 @@ pub fn update_user( } } - if let Some(enable) = enable { + if let Some(enable) = update.enable { data.enable = if enable { None } else { Some(false) }; } - if let Some(expire) = expire { + if let Some(expire) = update.expire { data.expire = if expire > 0 { Some(expire) } else { None }; } @@ -361,20 +304,20 @@ pub fn update_user( authenticator.store_password(userid.name(), &password)?; } - if let Some(firstname) = firstname { + if let Some(firstname) = update.firstname { data.firstname = if firstname.is_empty() { None } else { Some(firstname) }; } - if let Some(lastname) = lastname { + if let Some(lastname) = update.lastname { data.lastname = if lastname.is_empty() { None } else { Some(lastname) }; } - if let Some(email) = email { + if let Some(email) = update.email { data.email = if email.is_empty() { None } else { Some(email) }; } config.set_data(userid.as_str(), "user", &data)?; - user::save_config(&config)?; + crate::config::user::save_config(&config)?; Ok(()) } @@ -402,10 +345,10 @@ pub fn update_user( /// Remove a user from the configuration file. pub fn delete_user(userid: Userid, digest: Option) -> Result<(), Error> { + let _lock = crate::config::user::lock_config()?; let _tfa_lock = crate::config::tfa::write_lock()?; - let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?; - - let (mut config, expected_digest) = user::config()?; + + let (mut config, expected_digest) = crate::config::user::config()?; if let Some(ref digest) = digest { let digest = proxmox::tools::hex_to_digest(digest)?; @@ -417,7 +360,7 @@ pub fn delete_user(userid: Userid, digest: Option) -> Result<(), Error> None => bail!("user '{}' does not exist.", userid), } - user::save_config(&config)?; + crate::config::user::save_config(&config)?; let authenticator = crate::auth::lookup_authenticator(userid.realm())?; match authenticator.remove_password(userid.name()) { @@ -457,7 +400,7 @@ pub fn delete_user(userid: Userid, digest: Option) -> Result<(), Error> }, }, }, - returns: { type: user::ApiToken }, + returns: { type: ApiToken }, access: { permission: &Permission::Or(&[ &Permission::Privilege(&["access", "users"], PRIV_SYS_AUDIT, false), @@ -471,9 +414,9 @@ pub fn read_token( tokenname: Tokenname, _info: &ApiMethod, mut rpcenv: &mut dyn RpcEnvironment, -) -> Result { +) -> Result { - let (config, digest) = user::config()?; + let (config, digest) = crate::config::user::config()?; let tokenid = Authid::from((userid, Some(tokenname))); @@ -496,11 +439,11 @@ pub fn read_token( schema: SINGLE_LINE_COMMENT_SCHEMA, }, enable: { - schema: user::ENABLE_USER_SCHEMA, + schema: ENABLE_USER_SCHEMA, optional: true, }, expire: { - schema: user::EXPIRE_USER_SCHEMA, + schema: EXPIRE_USER_SCHEMA, optional: true, }, digest: { @@ -539,9 +482,9 @@ pub fn generate_token( digest: Option, ) -> Result { - let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?; + let _lock = crate::config::user::lock_config()?; - let (mut config, expected_digest) = user::config()?; + let (mut config, expected_digest) = crate::config::user::config()?; if let Some(ref digest) = digest { let digest = proxmox::tools::hex_to_digest(digest)?; @@ -558,7 +501,7 @@ pub fn generate_token( let secret = format!("{:x}", proxmox::tools::uuid::Uuid::generate()); token_shadow::set_secret(&tokenid, &secret)?; - let token = user::ApiToken { + let token = ApiToken { tokenid, comment, enable, @@ -567,7 +510,7 @@ pub fn generate_token( config.set_data(&tokenid_string, "token", &token)?; - user::save_config(&config)?; + crate::config::user::save_config(&config)?; Ok(json!({ "tokenid": tokenid_string, @@ -590,11 +533,11 @@ pub fn generate_token( schema: SINGLE_LINE_COMMENT_SCHEMA, }, enable: { - schema: user::ENABLE_USER_SCHEMA, + schema: ENABLE_USER_SCHEMA, optional: true, }, expire: { - schema: user::EXPIRE_USER_SCHEMA, + schema: EXPIRE_USER_SCHEMA, optional: true, }, digest: { @@ -620,9 +563,9 @@ pub fn update_token( digest: Option, ) -> Result<(), Error> { - let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?; + let _lock = crate::config::user::lock_config()?; - let (mut config, expected_digest) = user::config()?; + let (mut config, expected_digest) = crate::config::user::config()?; if let Some(ref digest) = digest { let digest = proxmox::tools::hex_to_digest(digest)?; @@ -632,7 +575,7 @@ pub fn update_token( let tokenid = Authid::from((userid, Some(tokenname))); let tokenid_string = tokenid.to_string(); - let mut data: user::ApiToken = config.lookup("token", &tokenid_string)?; + let mut data: ApiToken = config.lookup("token", &tokenid_string)?; if let Some(comment) = comment { let comment = comment.trim().to_string(); @@ -653,7 +596,7 @@ pub fn update_token( config.set_data(&tokenid_string, "token", &data)?; - user::save_config(&config)?; + crate::config::user::save_config(&config)?; Ok(()) } @@ -688,9 +631,9 @@ pub fn delete_token( digest: Option, ) -> Result<(), Error> { - let _lock = open_backup_lockfile(user::USER_CFG_LOCKFILE, None, true)?; + let _lock = crate::config::user::lock_config()?; - let (mut config, expected_digest) = user::config()?; + let (mut config, expected_digest) = crate::config::user::config()?; if let Some(ref digest) = digest { let digest = proxmox::tools::hex_to_digest(digest)?; @@ -707,7 +650,7 @@ pub fn delete_token( token_shadow::delete_secret(&tokenid)?; - user::save_config(&config)?; + crate::config::user::save_config(&config)?; Ok(()) } @@ -723,7 +666,7 @@ pub fn delete_token( returns: { description: "List user's API tokens (with config digest).", type: Array, - items: { type: user::ApiToken }, + items: { type: ApiToken }, }, access: { permission: &Permission::Or(&[ @@ -737,15 +680,15 @@ pub fn list_tokens( userid: Userid, _info: &ApiMethod, mut rpcenv: &mut dyn RpcEnvironment, -) -> Result, Error> { +) -> Result, Error> { - let (config, digest) = user::config()?; + let (config, digest) = crate::config::user::config()?; - let list:Vec = config.convert_to_typed_array("token")?; + let list:Vec = config.convert_to_typed_array("token")?; rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into(); - let filter_by_owner = |token: &user::ApiToken| { + let filter_by_owner = |token: &ApiToken| { if token.tokenid.is_token() { token.tokenid.user() == &userid } else { diff --git a/src/config/cached_user_info.rs b/src/config/cached_user_info.rs index 1ec480a0..47631575 100644 --- a/src/config/cached_user_info.rs +++ b/src/config/cached_user_info.rs @@ -9,10 +9,9 @@ use lazy_static::lazy_static; use proxmox::api::UserInformation; use proxmox::tools::time::epoch_i64; -use pbs_api_types::{Authid, Userid, ROLE_ADMIN}; +use pbs_api_types::{Authid, Userid, User, ApiToken, ROLE_ADMIN}; use pbs_config::acl::{AclTree, ROLE_NAMES}; -use super::user::{ApiToken, User}; use crate::tools::Memcom; /// Cache User/Group/Token/Acl configuration data for fast permission tests diff --git a/src/config/tfa.rs b/src/config/tfa.rs index b1c1d6c2..4820a768 100644 --- a/src/config/tfa.rs +++ b/src/config/tfa.rs @@ -27,8 +27,7 @@ use proxmox::tools::AsHex; use pbs_buildcfg::configdir; use pbs_config::{open_backup_lockfile, BackupLockGuard}; - -use crate::api2::types::Userid; +use pbs_api_types::{Userid, User}; /// Mapping of userid to TFA entry. pub type TfaUsers = HashMap; @@ -278,7 +277,6 @@ impl TfaConfig { /// Remove non-existent users. pub fn cleanup_users(&mut self, config: &proxmox::api::section_config::SectionConfigData) { - use crate::config::user::User; self.users .retain(|user, _| config.lookup::("user", user.as_str()).is_ok()); } diff --git a/src/config/user.rs b/src/config/user.rs index 97dea117..71ba5a49 100644 --- a/src/config/user.rs +++ b/src/config/user.rs @@ -13,11 +13,10 @@ use proxmox::api::{ } }; -use pbs_api_types::{Authid, Userid}; -pub use pbs_api_types::{ApiToken, User}; -pub use pbs_api_types::{ - EMAIL_SCHEMA, ENABLE_USER_SCHEMA, EXPIRE_USER_SCHEMA, FIRST_NAME_SCHEMA, LAST_NAME_SCHEMA, +use pbs_api_types::{ + Authid, Userid, ApiToken, User, }; +use pbs_config::{open_backup_lockfile, replace_backup_config, BackupLockGuard}; use crate::tools::Memcom; @@ -48,6 +47,11 @@ fn init() -> SectionConfig { pub const USER_CFG_FILENAME: &str = "/etc/proxmox-backup/user.cfg"; pub const USER_CFG_LOCKFILE: &str = "/etc/proxmox-backup/.user.lck"; +/// Get exclusive lock +pub fn lock_config() -> Result { + open_backup_lockfile(USER_CFG_LOCKFILE, None, true) +} + pub fn config() -> Result<(SectionConfigData, [u8;32]), Error> { let content = proxmox::tools::fs::file_read_optional_string(USER_CFG_FILENAME)? @@ -119,7 +123,7 @@ pub fn cached_config() -> Result, Error> { pub fn save_config(config: &SectionConfigData) -> Result<(), Error> { let raw = CONFIG.write(USER_CFG_FILENAME, &config)?; - pbs_config::replace_backup_config(USER_CFG_FILENAME, raw.as_bytes())?; + replace_backup_config(USER_CFG_FILENAME, raw.as_bytes())?; // increase user cache generation // We use this in CachedUserInfo diff --git a/src/server/email_notifications.rs b/src/server/email_notifications.rs index b0250b34..f26d2cea 100644 --- a/src/server/email_notifications.rs +++ b/src/server/email_notifications.rs @@ -9,7 +9,7 @@ use proxmox::try_block; use pbs_tools::format::HumanByte; use pbs_api_types::{ - TapeBackupJobSetup, SyncJobConfig, VerificationJobConfig, + User, TapeBackupJobSetup, SyncJobConfig, VerificationJobConfig, APTUpdateInfo, GarbageCollectionStatus, Userid, Notify, DatastoreNotify, }; @@ -548,9 +548,7 @@ pub fn send_updates_available( /// Lookup users email address pub fn lookup_user_email(userid: &Userid) -> Option { - use crate::config::user::{self, User}; - - if let Ok(user_config) = user::cached_config() { + if let Ok(user_config) = crate::config::user::cached_config() { if let Ok(user) = user_config.lookup::("user", userid.as_str()) { return user.email; }