From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id E5B31612D4 for ; Tue, 20 Oct 2020 11:42:54 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id D3C01D87C for ; Tue, 20 Oct 2020 11:42:24 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id D7E54D872 for ; Tue, 20 Oct 2020 11:42:23 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 9CA5F44AA5 for ; Tue, 20 Oct 2020 11:42:23 +0200 (CEST) Date: Tue, 20 Oct 2020 11:42:22 +0200 From: Wolfgang Bumiller To: Fabian =?utf-8?Q?Gr=C3=BCnbichler?= Cc: pbs-devel@lists.proxmox.com Message-ID: <20201020094222.2wsepswjngyle2ru@olga.proxmox.com> References: <20201019073919.588521-1-f.gruenbichler@proxmox.com> <20201019073919.588521-9-f.gruenbichler@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20201019073919.588521-9-f.gruenbichler@proxmox.com> User-Agent: NeoMutt/20180716 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.015 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [user.rs] Subject: Re: [pbs-devel] [RFC proxmox-backup 08/15] api: add API token endpoints X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 20 Oct 2020 09:42:55 -0000 On Mon, Oct 19, 2020 at 09:39:12AM +0200, Fabian Grünbichler wrote: > beneath the user endpoint. > > Signed-off-by: Fabian Grünbichler > --- > src/api2/access/user.rs | 327 +++++++++++++++++++++++++++++++++++++++- > 1 file changed, 324 insertions(+), 3 deletions(-) > > diff --git a/src/api2/access/user.rs b/src/api2/access/user.rs > index 6c292c2d..4197cf60 100644 > --- a/src/api2/access/user.rs > +++ b/src/api2/access/user.rs > @@ -1,12 +1,15 @@ > use anyhow::{bail, Error}; > use serde_json::Value; > +use std::convert::TryFrom; > > use proxmox::api::{api, ApiMethod, Router, RpcEnvironment, Permission}; > +use proxmox::api::router::SubdirMap; > use proxmox::api::schema::{Schema, StringSchema}; > use proxmox::tools::fs::open_file_locked; > > use crate::api2::types::*; > use crate::config::user; > +use crate::config::token_shadow; > use crate::config::acl::{PRIV_SYS_AUDIT, PRIV_PERMISSIONS_MODIFY}; > use crate::config::cached_user_info::CachedUserInfo; > > @@ -304,12 +307,330 @@ pub fn delete_user(userid: Userid, digest: Option) -> Result<(), Error> > Ok(()) > } > > -const ITEM_ROUTER: Router = Router::new() > +#[api( > + input: { > + properties: { > + userid: { > + schema: PROXMOX_USER_ID_SCHEMA, > + }, > + tokenname: { > + schema: PROXMOX_TOKEN_NAME_SCHEMA, > + }, > + }, > + }, > + returns: { > + description: "Get API token metadata (with config digest).", > + type: user::ApiToken, > + }, > + access: { > + permission: &Permission::Or(&[ > + &Permission::Privilege(&["access", "users"], PRIV_SYS_AUDIT, false), > + &Permission::UserParam("userid"), > + ]), > + }, > +)] > +/// Read user's API token metadata > +pub fn read_token( > + userid: Userid, > + tokenname: String, > + _info: &ApiMethod, > + mut rpcenv: &mut dyn RpcEnvironment, > +) -> Result { > + > + let (config, digest) = user::config()?; > + > + let tokenname = Tokenname::try_from(tokenname)?; > + > + let tokenid = Userid::from((userid.name(), userid.realm(), tokenname.as_ref())); > + > + rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into(); > + config.lookup("token", tokenid.as_str()) > +} > + > +#[api( > + protected: true, > + input: { > + properties: { > + userid: { > + schema: PROXMOX_USER_ID_SCHEMA, > + }, > + tokenname: { > + schema: PROXMOX_TOKEN_NAME_SCHEMA, > + }, > + comment: { > + optional: true, > + schema: SINGLE_LINE_COMMENT_SCHEMA, > + }, > + enable: { > + schema: user::ENABLE_USER_SCHEMA, > + optional: true, > + }, > + expire: { > + schema: user::EXPIRE_USER_SCHEMA, > + optional: true, > + }, > + digest: { > + optional: true, > + schema: PROXMOX_CONFIG_DIGEST_SCHEMA, > + }, > + }, > + }, > + access: { > + permission: &Permission::Or(&[ > + &Permission::Privilege(&["access", "users"], PRIV_PERMISSIONS_MODIFY, false), > + &Permission::UserParam("userid"), > + ]), > + }, > + returns: { > + description: "Secret API token value", > + type: String, > + }, > +)] > +/// Generate a new API token with given metadata > +pub fn generate_token( > + userid: Userid, > + tokenname: String, > + comment: Option, > + enable: Option, > + expire: Option, > + digest: Option, > +) -> Result { > + > + let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?; > + > + let (mut config, expected_digest) = 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 tokenname = Tokenname::try_from(tokenname)?; > + let tokenid = Userid::from((userid.name(), userid.realm(), tokenname.as_ref())); > + > + if let Some(_) = config.sections.get(tokenid.as_str()) { > + bail!("token '{}' for user '{}' already exists.", tokenname.as_str(), userid); > + } > + > + let secret = format!("{:x}", proxmox::tools::uuid::Uuid::generate()); > + token_shadow::set_secret(&tokenid, &secret)?; > + > + let token = user::ApiToken { > + tokenid: tokenid.clone(), > + comment, > + enable, > + expire, > + }; > + > + config.set_data(tokenid.as_str(), "token", &token)?; > + > + user::save_config(&config)?; > + > + Ok(secret) > +} > + > +#[api( > + protected: true, > + input: { > + properties: { > + userid: { > + schema: PROXMOX_USER_ID_SCHEMA, > + }, > + tokenname: { > + schema: PROXMOX_TOKEN_NAME_SCHEMA, > + }, > + comment: { > + optional: true, > + schema: SINGLE_LINE_COMMENT_SCHEMA, > + }, > + enable: { > + schema: user::ENABLE_USER_SCHEMA, > + optional: true, > + }, > + expire: { > + schema: user::EXPIRE_USER_SCHEMA, > + optional: true, > + }, > + digest: { > + optional: true, > + schema: PROXMOX_CONFIG_DIGEST_SCHEMA, > + }, > + }, > + }, > + access: { > + permission: &Permission::Or(&[ > + &Permission::Privilege(&["access", "users"], PRIV_PERMISSIONS_MODIFY, false), > + &Permission::UserParam("userid"), > + ]), > + }, > +)] > +/// Update user's API token metadata > +pub fn update_token( > + userid: Userid, > + tokenname: String, > + comment: Option, > + enable: Option, > + expire: Option, > + digest: Option, > +) -> Result<(), Error> { > + > + let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?; > + > + let (mut config, expected_digest) = 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 tokenname = Tokenname::try_from(tokenname)?; > + let tokenid = Userid::from((userid.name(), userid.realm(), tokenname.as_ref())); > + > + let mut data: user::ApiToken = config.lookup("token", tokenid.as_str())?; > + > + if let Some(comment) = comment { > + let comment = comment.trim().to_string(); > + if comment.is_empty() { > + data.comment = None; > + } else { > + data.comment = Some(comment); > + } > + } > + > + if let Some(enable) = enable { > + data.enable = if enable { None } else { Some(false) }; Really not a fan of single line if/else like this. Also with the `if let` together this isn't actually "fast" to read. How about: data.enabled = match enable { Some(true) => None, other => other, } or data.enabled = enable.filter(|&b| !b); > + } > + > + if let Some(expire) = expire { > + data.expire = if expire > 0 { Some(expire) } else { None }; > + } Similarly: data.expire = expire.filter(|&e| e > 0) or a match with a conditional arm: data.expire = match expire { Some(x) if x > 0 => Some(x), _ => None, } I find those much more readable than nesting conditions. > + > + config.set_data(tokenid.as_str(), "token", &data)?; > + > + user::save_config(&config)?; > + > + Ok(()) > +} > + > +#[api( > + protected: true, > + input: { > + properties: { > + userid: { > + schema: PROXMOX_USER_ID_SCHEMA, > + }, > + tokenname: { > + schema: PROXMOX_TOKEN_NAME_SCHEMA, > + }, > + digest: { > + optional: true, > + schema: PROXMOX_CONFIG_DIGEST_SCHEMA, > + }, > + }, > + }, > + access: { > + permission: &Permission::Or(&[ > + &Permission::Privilege(&["access", "users"], PRIV_PERMISSIONS_MODIFY, false), > + &Permission::UserParam("userid"), > + ]), > + }, > +)] > +/// Delete a user's API token > +pub fn delete_token( > + userid: Userid, > + tokenname: String, > + digest: Option, > +) -> Result<(), Error> { > + > + let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?; > + > + let (mut config, expected_digest) = 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 tokenname = Tokenname::try_from(tokenname)?; > + let tokenid = Userid::from((userid.name(), userid.realm(), tokenname.as_ref())); > + > + match config.sections.get(tokenid.as_str()) { > + Some(_) => { config.sections.remove(tokenid.as_str()); }, > + None => bail!("token '{}' of user '{}' does not exist.", tokenname.as_str(), userid), > + } > + > + token_shadow::delete_secret(&tokenid)?; > + > + user::save_config(&config)?; > + > + Ok(()) > +} > + > +#[api( > + input: { > + properties: { > + userid: { > + schema: PROXMOX_USER_ID_SCHEMA, > + }, > + }, > + }, > + returns: { > + description: "List user's API tokens (with config digest).", > + type: Array, > + items: { type: user::ApiToken }, > + }, > + access: { > + permission: &Permission::Or(&[ > + &Permission::Privilege(&["access", "users"], PRIV_SYS_AUDIT, false), > + &Permission::UserParam("userid"), > + ]), > + }, > +)] > +/// List user's API tokens > +pub fn list_tokens( > + userid: Userid, > + _info: &ApiMethod, > + mut rpcenv: &mut dyn RpcEnvironment, > +) -> Result, Error> { > + > + let (config, digest) = user::config()?; > + > + 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| { > + if let Ok(owner) = token.tokenid.owner() { > + owner == userid > + } else { > + false > + } > + }; > + > + Ok(list.into_iter().filter(filter_by_owner).collect()) > +} > + > +const TOKEN_ITEM_ROUTER: Router = Router::new() > + .get(&API_METHOD_READ_TOKEN) > + .put(&API_METHOD_UPDATE_TOKEN) > + .post(&API_METHOD_GENERATE_TOKEN) > + .delete(&API_METHOD_DELETE_TOKEN); > + > +const TOKEN_ROUTER: Router = Router::new() > + .get(&API_METHOD_LIST_TOKENS) > + .match_all("tokenname", &TOKEN_ITEM_ROUTER); > + > +const USER_SUBDIRS: SubdirMap = &[ > + ("token", &TOKEN_ROUTER), > +]; > + > +const USER_ROUTER: Router = Router::new() > .get(&API_METHOD_READ_USER) > .put(&API_METHOD_UPDATE_USER) > - .delete(&API_METHOD_DELETE_USER); > + .delete(&API_METHOD_DELETE_USER) > + .subdirs(USER_SUBDIRS); > > pub const ROUTER: Router = Router::new() > .get(&API_METHOD_LIST_USERS) > .post(&API_METHOD_CREATE_USER) > - .match_all("userid", &ITEM_ROUTER); > + .match_all("userid", &USER_ROUTER); > -- > 2.20.1