* [pbs-devel] [PATCH proxmox-backup 00/16] API tokens
@ 2020-10-28 11:36 Fabian Grünbichler
2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-widget-toolkit] add PermissionView Fabian Grünbichler
` (13 more replies)
0 siblings, 14 replies; 25+ messages in thread
From: Fabian Grünbichler @ 2020-10-28 11:36 UTC (permalink / raw)
To: pbs-devel
changes since RFC:
- reworked Userid with wrapping Authid
- rename RpcEnvironment user -> auth_id
- lots of churn
- lots of corner cases
- lots of rebasing
- ACL editing for unprivileged users
- more GUI stuff added
proxmox and proxmox-widget-toolkit patches needed for patches #3++ and
GUI respectively
^ permalink raw reply [flat|nested] 25+ messages in thread* [pbs-devel] [PATCH proxmox-widget-toolkit] add PermissionView 2020-10-28 11:36 [pbs-devel] [PATCH proxmox-backup 00/16] API tokens Fabian Grünbichler @ 2020-10-28 11:36 ` Fabian Grünbichler 2020-10-28 16:18 ` [pbs-devel] applied: " Thomas Lamprecht 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-backup 01/16] api: add Authid as wrapper around Userid Fabian Grünbichler ` (12 subsequent siblings) 13 siblings, 1 reply; 25+ messages in thread From: Fabian Grünbichler @ 2020-10-28 11:36 UTC (permalink / raw) To: pbs-devel copied from pve-manager, but handling both '1' and 'true' as propagate values, and making the authentication ID name/parameter configurable. Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> --- src/Makefile | 1 + src/panel/PermissionView.js | 153 ++++++++++++++++++++++++++++++++++++ 2 files changed, 154 insertions(+) create mode 100644 src/panel/PermissionView.js diff --git a/src/Makefile b/src/Makefile index cd0bf26..f984ac7 100644 --- a/src/Makefile +++ b/src/Makefile @@ -40,6 +40,7 @@ JSSRC= \ panel/InfoWidget.js \ panel/LogView.js \ panel/JournalView.js \ + panel/PermissionView.js \ panel/RRDChart.js \ panel/GaugeWidget.js \ window/Edit.js \ diff --git a/src/panel/PermissionView.js b/src/panel/PermissionView.js new file mode 100644 index 0000000..5aa74ff --- /dev/null +++ b/src/panel/PermissionView.js @@ -0,0 +1,153 @@ +Ext.define('pmx-permissions', { + extend: 'Ext.data.TreeModel', + fields: [ + 'text', 'type', + { + type: 'boolean', name: 'propagate', + }, + ], +}); + +Ext.define('Proxmox.panel.PermissionViewPanel', { + extend: 'Ext.tree.Panel', + xtype: 'proxmoxPermissionViewPanel', + + scrollable: true, + layout: 'fit', + rootVisible: false, + animate: false, + sortableColumns: false, + + auth_id_name: "userid", + auth_id: undefined, + + columns: [ + { + xtype: 'treecolumn', + header: gettext('Path') + '/' + gettext('Permission'), + dataIndex: 'text', + flex: 6, + }, + { + header: gettext('Propagate'), + dataIndex: 'propagate', + flex: 1, + renderer: function(value) { + if (Ext.isDefined(value)) { + return Proxmox.Utils.format_boolean(value); + } + return ''; + }, + }, + ], + + initComponent: function() { + let me = this; + + Proxmox.Utils.API2Request({ + url: '/access/permissions?' + encodeURIComponent(me.auth_id_name) + '=' + encodeURIComponent(me.auth_id), + method: 'GET', + failure: function(response, opts) { + Proxmox.Utils.setErrorMask(me, response.htmlStatus); + }, + success: function(response, opts) { + Proxmox.Utils.setErrorMask(me, false); + let result = Ext.decode(response.responseText); + let data = result.data || {}; + + let root = { + name: '__root', + expanded: true, + children: [], + }; + let idhash = { + '/': { + children: [], + text: '/', + type: 'path', + }, + }; + Ext.Object.each(data, function(path, perms) { + let path_item = { + text: path, + type: 'path', + children: [], + }; + Ext.Object.each(perms, function(perm, propagate) { + let perm_item = { + text: perm, + type: 'perm', + propagate: propagate === 1 || propagate === true, + iconCls: 'fa fa-fw fa-unlock', + leaf: true, + }; + path_item.children.push(perm_item); + path_item.expandable = true; + }); + idhash[path] = path_item; + }); + + Ext.Object.each(idhash, function(path, item) { + let parent_item = idhash['/']; + if (path === '/') { + parent_item = root; + item.expanded = true; + } else { + let split_path = path.split('/'); + while (split_path.pop()) { + let parent_path = split_path.join('/'); + if (idhash[parent_path]) { + parent_item = idhash[parent_path]; + break; + } + } + } + parent_item.children.push(item); + }); + + me.setRootNode(root); + }, + }); + + me.callParent(); + + me.store.sorters.add(new Ext.util.Sorter({ + sorterFn: function(rec1, rec2) { + let v1 = rec1.data.text, + v2 = rec2.data.text; + if (rec1.data.type !== rec2.data.type) { + v2 = rec1.data.type; + v1 = rec2.data.type; + } + if (v1 > v2) { + return 1; + } else if (v1 < v2) { + return -1; + } + return 0; + }, + })); + }, +}); + +Ext.define('Proxmox.PermissionView', { + extend: 'Ext.window.Window', + alias: 'widget.userShowPermissionWindow', + mixins: ['Proxmox.Mixin.CBind'], + + scrollable: true, + width: 800, + height: 600, + layout: 'fit', + cbind: { + title: (get) => Ext.String.htmlEncode(get('auth_id')) + + ` - ${gettext('Granted Permissions')}`, + }, + items: [{ + xtype: 'proxmoxPermissionViewPanel', + cbind: { + auth_id: '{auth_id}', + auth_id_name: '{auth_id_name}', + }, + }], +}); -- 2.20.1 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [pbs-devel] applied: [PATCH proxmox-widget-toolkit] add PermissionView 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-widget-toolkit] add PermissionView Fabian Grünbichler @ 2020-10-28 16:18 ` Thomas Lamprecht 0 siblings, 0 replies; 25+ messages in thread From: Thomas Lamprecht @ 2020-10-28 16:18 UTC (permalink / raw) To: Proxmox Backup Server development discussion, Fabian Grünbichler On 28.10.20 12:36, Fabian Grünbichler wrote: > copied from pve-manager, but handling both '1' and 'true' as propagate > values, and making the authentication ID name/parameter configurable. > > Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> > --- > src/Makefile | 1 + > src/panel/PermissionView.js | 153 ++++++++++++++++++++++++++++++++++++ > 2 files changed, 154 insertions(+) > create mode 100644 src/panel/PermissionView.js > > applied, thanks! ^ permalink raw reply [flat|nested] 25+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 01/16] api: add Authid as wrapper around Userid 2020-10-28 11:36 [pbs-devel] [PATCH proxmox-backup 00/16] API tokens Fabian Grünbichler 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-widget-toolkit] add PermissionView Fabian Grünbichler @ 2020-10-28 11:36 ` Fabian Grünbichler 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox] rpcenv: rename user to auth_id Fabian Grünbichler ` (11 subsequent siblings) 13 siblings, 0 replies; 25+ messages in thread From: Fabian Grünbichler @ 2020-10-28 11:36 UTC (permalink / raw) To: pbs-devel with an optional Tokenname, appended with '!' as delimiter in the string representation like for PVE. Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> --- Notes: changes since RFC: - rewrite, incorporating Wolfgang's and Thomas' suggestion to use an Authid as supertype of Userid, to make the distinction clearer src/api2/types/mod.rs | 4 +- src/api2/types/userid.rs | 383 +++++++++++++++++++++++++++++++++++---- 2 files changed, 355 insertions(+), 32 deletions(-) diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs index 1b9a305f..3f723e32 100644 --- a/src/api2/types/mod.rs +++ b/src/api2/types/mod.rs @@ -14,9 +14,11 @@ mod macros; #[macro_use] mod userid; pub use userid::{Realm, RealmRef}; +pub use userid::{Tokenname, TokennameRef}; pub use userid::{Username, UsernameRef}; pub use userid::Userid; -pub use userid::PROXMOX_GROUP_ID_SCHEMA; +pub use userid::Authid; +pub use userid::{PROXMOX_TOKEN_ID_SCHEMA, PROXMOX_TOKEN_NAME_SCHEMA, PROXMOX_GROUP_ID_SCHEMA}; // File names: may not contain slashes, may not start with "." pub const FILENAME_FORMAT: ApiStringFormat = ApiStringFormat::VerifyFn(|name| { diff --git a/src/api2/types/userid.rs b/src/api2/types/userid.rs index 44cd10b7..2b5b43af 100644 --- a/src/api2/types/userid.rs +++ b/src/api2/types/userid.rs @@ -1,6 +1,7 @@ //! Types for user handling. //! -//! We have [`Username`]s and [`Realm`]s. To uniquely identify a user, they must be combined into a [`Userid`]. +//! We have [`Username`]s, [`Realm`]s and [`Tokenname`]s. To uniquely identify a user/API token, they +//! must be combined into a [`Userid`] or [`Authid`]. //! //! Since they're all string types, they're organized as follows: //! @@ -9,13 +10,16 @@ //! with `String`, meaning you can only make references to it. //! * [`Realm`]: an owned realm (`String` equivalent). //! * [`RealmRef`]: a borrowed realm (`str` equivalent). -//! * [`Userid`]: an owned user id (`"user@realm"`). Note that this does not have a separate -//! borrowed type. +//! * [`Tokenname`]: an owned API token name (`String` equivalent) +//! * [`TokennameRef`]: a borrowed `Tokenname` (`str` equivalent). +//! * [`Userid`]: an owned user id (`"user@realm"`). +//! * [`Authid`]: an owned Authentication ID (a `Userid` with an optional `Tokenname`). +//! Note that `Userid` and `Authid` do not have a separate borrowed type. //! -//! Note that `Username`s are not unique, therefore they do not implement `Eq` and cannot be +//! Note that `Username`s and `Tokenname`s are not unique, therefore they do not implement `Eq` and cannot be //! compared directly. If a direct comparison is really required, they can be compared as strings -//! via the `as_str()` method. [`Realm`]s and [`Userid`]s on the other hand can be compared with -//! each other, as in those two cases the comparison has meaning. +//! via the `as_str()` method. [`Realm`]s, [`Userid`]s and [`Authid`]s on the other +//! hand can be compared with each other, as in those cases the comparison has meaning. use std::borrow::Borrow; use std::convert::TryFrom; @@ -36,19 +40,42 @@ use proxmox::const_regex; // also see "man useradd" macro_rules! USER_NAME_REGEX_STR { () => (r"(?:[^\s:/[:cntrl:]]+)") } macro_rules! GROUP_NAME_REGEX_STR { () => (USER_NAME_REGEX_STR!()) } +macro_rules! TOKEN_NAME_REGEX_STR { () => (PROXMOX_SAFE_ID_REGEX_STR!()) } macro_rules! USER_ID_REGEX_STR { () => (concat!(USER_NAME_REGEX_STR!(), r"@", PROXMOX_SAFE_ID_REGEX_STR!())) } +macro_rules! APITOKEN_ID_REGEX_STR { () => (concat!(USER_ID_REGEX_STR!() , r"!", TOKEN_NAME_REGEX_STR!())) } const_regex! { pub PROXMOX_USER_NAME_REGEX = concat!(r"^", USER_NAME_REGEX_STR!(), r"$"); + pub PROXMOX_TOKEN_NAME_REGEX = concat!(r"^", TOKEN_NAME_REGEX_STR!(), r"$"); pub PROXMOX_USER_ID_REGEX = concat!(r"^", USER_ID_REGEX_STR!(), r"$"); + pub PROXMOX_APITOKEN_ID_REGEX = concat!(r"^", APITOKEN_ID_REGEX_STR!(), r"$"); + pub PROXMOX_AUTH_ID_REGEX = concat!(r"^", r"(?:", USER_ID_REGEX_STR!(), r"|", APITOKEN_ID_REGEX_STR!(), r")$"); pub PROXMOX_GROUP_ID_REGEX = concat!(r"^", GROUP_NAME_REGEX_STR!(), r"$"); } pub const PROXMOX_USER_NAME_FORMAT: ApiStringFormat = ApiStringFormat::Pattern(&PROXMOX_USER_NAME_REGEX); +pub const PROXMOX_TOKEN_NAME_FORMAT: ApiStringFormat = + ApiStringFormat::Pattern(&PROXMOX_TOKEN_NAME_REGEX); pub const PROXMOX_USER_ID_FORMAT: ApiStringFormat = ApiStringFormat::Pattern(&PROXMOX_USER_ID_REGEX); +pub const PROXMOX_TOKEN_ID_FORMAT: ApiStringFormat = + ApiStringFormat::Pattern(&PROXMOX_APITOKEN_ID_REGEX); +pub const PROXMOX_AUTH_ID_FORMAT: ApiStringFormat = + ApiStringFormat::Pattern(&PROXMOX_AUTH_ID_REGEX); + +pub const PROXMOX_TOKEN_ID_SCHEMA: Schema = StringSchema::new("API Token ID") + .format(&PROXMOX_TOKEN_ID_FORMAT) + .min_length(3) + .max_length(64) + .schema(); + +pub const PROXMOX_TOKEN_NAME_SCHEMA: Schema = StringSchema::new("API Token name") + .format(&PROXMOX_TOKEN_NAME_FORMAT) + .min_length(3) + .max_length(64) + .schema(); pub const PROXMOX_GROUP_ID_FORMAT: ApiStringFormat = ApiStringFormat::Pattern(&PROXMOX_GROUP_ID_REGEX); @@ -91,26 +118,6 @@ pub struct Username(String); #[derive(Debug, Hash)] pub struct UsernameRef(str); -#[doc(hidden)] -/// ```compile_fail -/// let a: Username = unsafe { std::mem::zeroed() }; -/// let b: Username = unsafe { std::mem::zeroed() }; -/// let _ = <Username as PartialEq>::eq(&a, &b); -/// ``` -/// -/// ```compile_fail -/// let a: &UsernameRef = unsafe { std::mem::zeroed() }; -/// let b: &UsernameRef = unsafe { std::mem::zeroed() }; -/// let _ = <&UsernameRef as PartialEq>::eq(a, b); -/// ``` -/// -/// ```compile_fail -/// let a: &UsernameRef = unsafe { std::mem::zeroed() }; -/// let b: &UsernameRef = unsafe { std::mem::zeroed() }; -/// let _ = <&UsernameRef as PartialEq>::eq(&a, &b); -/// ``` -struct _AssertNoEqImpl; - impl UsernameRef { fn new(s: &str) -> &Self { unsafe { &*(s as *const str as *const UsernameRef) } @@ -286,7 +293,132 @@ impl PartialEq<Realm> for &RealmRef { } } -/// A complete user id consting of a user name and a realm. +#[api( + type: String, + format: &PROXMOX_TOKEN_NAME_FORMAT, +)] +/// The token ID part of an API token authentication id. +/// +/// This alone does NOT uniquely identify the API token and therefore does not implement `Eq`. In +/// order to compare token IDs directly, they need to be explicitly compared as strings by calling +/// `.as_str()`. +/// +/// ```compile_fail +/// fn test(a: Tokenname, b: Tokenname) -> bool { +/// a == b // illegal and does not compile +/// } +/// ``` +#[derive(Clone, Debug, Hash, Deserialize, Serialize)] +pub struct Tokenname(String); + +/// A reference to a token name part of an authentication id. This alone does NOT uniquely identify +/// the user. +/// +/// This is like a `str` to the `String` of a [`Tokenname`]. +#[derive(Debug, Hash)] +pub struct TokennameRef(str); + +#[doc(hidden)] +/// ```compile_fail +/// let a: Username = unsafe { std::mem::zeroed() }; +/// let b: Username = unsafe { std::mem::zeroed() }; +/// let _ = <Username as PartialEq>::eq(&a, &b); +/// ``` +/// +/// ```compile_fail +/// let a: &UsernameRef = unsafe { std::mem::zeroed() }; +/// let b: &UsernameRef = unsafe { std::mem::zeroed() }; +/// let _ = <&UsernameRef as PartialEq>::eq(a, b); +/// ``` +/// +/// ```compile_fail +/// let a: &UsernameRef = unsafe { std::mem::zeroed() }; +/// let b: &UsernameRef = unsafe { std::mem::zeroed() }; +/// let _ = <&UsernameRef as PartialEq>::eq(&a, &b); +/// ``` +/// +/// ```compile_fail +/// let a: Tokenname = unsafe { std::mem::zeroed() }; +/// let b: Tokenname = unsafe { std::mem::zeroed() }; +/// let _ = <Tokenname as PartialEq>::eq(&a, &b); +/// ``` +/// +/// ```compile_fail +/// let a: &TokennameRef = unsafe { std::mem::zeroed() }; +/// let b: &TokennameRef = unsafe { std::mem::zeroed() }; +/// let _ = <&TokennameRef as PartialEq>::eq(a, b); +/// ``` +/// +/// ```compile_fail +/// let a: &TokennameRef = unsafe { std::mem::zeroed() }; +/// let b: &TokennameRef = unsafe { std::mem::zeroed() }; +/// let _ = <&TokennameRef as PartialEq>::eq(&a, &b); +/// ``` +struct _AssertNoEqImpl; + +impl TokennameRef { + fn new(s: &str) -> &Self { + unsafe { &*(s as *const str as *const TokennameRef) } + } + + pub fn as_str(&self) -> &str { + &self.0 + } +} + +impl std::ops::Deref for Tokenname { + type Target = TokennameRef; + + fn deref(&self) -> &TokennameRef { + self.borrow() + } +} + +impl Borrow<TokennameRef> for Tokenname { + fn borrow(&self) -> &TokennameRef { + TokennameRef::new(self.0.as_str()) + } +} + +impl AsRef<TokennameRef> for Tokenname { + fn as_ref(&self) -> &TokennameRef { + self.borrow() + } +} + +impl ToOwned for TokennameRef { + type Owned = Tokenname; + + fn to_owned(&self) -> Self::Owned { + Tokenname(self.0.to_owned()) + } +} + +impl TryFrom<String> for Tokenname { + type Error = Error; + + fn try_from(s: String) -> Result<Self, Error> { + if !PROXMOX_TOKEN_NAME_REGEX.is_match(&s) { + bail!("invalid token name"); + } + + Ok(Self(s)) + } +} + +impl<'a> TryFrom<&'a str> for &'a TokennameRef { + type Error = Error; + + fn try_from(s: &'a str) -> Result<&'a TokennameRef, Error> { + if !PROXMOX_TOKEN_NAME_REGEX.is_match(s) { + bail!("invalid token name in user id"); + } + + Ok(TokennameRef::new(s)) + } +} + +/// A complete user id consisting of a user name and a realm #[derive(Clone, Debug, Hash)] pub struct Userid { data: String, @@ -366,10 +498,18 @@ impl std::str::FromStr for Userid { type Err = Error; fn from_str(id: &str) -> Result<Self, Error> { - let (name, realm) = match id.as_bytes().iter().rposition(|&b| b == b'@') { - Some(pos) => (&id[..pos], &id[(pos + 1)..]), - None => bail!("not a valid user id"), - }; + let name_len = id + .as_bytes() + .iter() + .rposition(|&b| b == b'@') + .ok_or_else(|| format_err!("not a valid user id"))?; + + let name = &id[..name_len]; + let realm = &id[(name_len + 1)..]; + + if !PROXMOX_USER_NAME_REGEX.is_match(name) { + bail!("invalid user name in user id"); + } PROXMOX_AUTH_REALM_STRING_SCHEMA.check_constraints(realm) .map_err(|_| format_err!("invalid realm in user id"))?; @@ -388,6 +528,10 @@ impl TryFrom<String> for Userid { .rposition(|&b| b == b'@') .ok_or_else(|| format_err!("not a valid user id"))?; + if !PROXMOX_USER_NAME_REGEX.is_match(&data[..name_len]) { + bail!("invalid user name in user id"); + } + PROXMOX_AUTH_REALM_STRING_SCHEMA.check_constraints(&data[(name_len + 1)..]) .map_err(|_| format_err!("invalid realm in user id"))?; @@ -413,5 +557,182 @@ impl PartialEq<String> for Userid { } } +/// A complete authentication id consisting of a user id and an optional token name. +#[derive(Clone, Debug, Hash)] +pub struct Authid { + user: Userid, + tokenname: Option<Tokenname> +} + +impl Authid { + pub const API_SCHEMA: Schema = StringSchema::new("Authentication ID") + .format(&PROXMOX_AUTH_ID_FORMAT) + .min_length(3) + .max_length(64) + .schema(); + + const fn new(user: Userid, tokenname: Option<Tokenname>) -> Self { + Self { user, tokenname } + } + + pub fn user(&self) -> &Userid { + &self.user + } + + pub fn is_token(&self) -> bool { + self.tokenname.is_some() + } + + pub fn tokenname(&self) -> Option<&TokennameRef> { + match &self.tokenname { + Some(name) => Some(&name), + None => None, + } + } + + /// Get the "backup@pam" auth id. + pub fn backup_auth_id() -> &'static Self { + &*BACKUP_AUTHID + } + + /// Get the "root@pam" auth id. + pub fn root_auth_id() -> &'static Self { + &*ROOT_AUTHID + } +} + +lazy_static! { + pub static ref BACKUP_AUTHID: Authid = Authid::from(Userid::new("backup@pam".to_string(), 6)); + pub static ref ROOT_AUTHID: Authid = Authid::from(Userid::new("root@pam".to_string(), 4)); +} + +impl Eq for Authid {} + +impl PartialEq for Authid { + fn eq(&self, rhs: &Self) -> bool { + self.user == rhs.user && match (&self.tokenname, &rhs.tokenname) { + (Some(ours), Some(theirs)) => ours.as_str() == theirs.as_str(), + (None, None) => true, + _ => false, + } + } +} + +impl From<Userid> for Authid { + fn from(parts: Userid) -> Self { + Self::new(parts, None) + } +} + +impl From<(Userid, Option<Tokenname>)> for Authid { + fn from(parts: (Userid, Option<Tokenname>)) -> Self { + Self::new(parts.0, parts.1) + } +} + +impl fmt::Display for Authid { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match &self.tokenname { + Some(token) => write!(f, "{}!{}", self.user, token.as_str()), + None => self.user.fmt(f), + } + } +} + +impl std::str::FromStr for Authid { + type Err = Error; + + fn from_str(id: &str) -> Result<Self, Error> { + let name_len = id + .as_bytes() + .iter() + .rposition(|&b| b == b'@') + .ok_or_else(|| format_err!("not a valid user id"))?; + + let realm_end = id + .as_bytes() + .iter() + .rposition(|&b| b == b'!') + .map(|pos| if pos < name_len { id.len() } else { pos }) + .unwrap_or(id.len()); + + if realm_end == id.len() - 1 { + bail!("empty token name in userid"); + } + + let user = Userid::from_str(&id[..realm_end])?; + + if id.len() > realm_end { + let token = Tokenname::try_from(id[(realm_end + 1)..].to_string())?; + Ok(Self::new(user, Some(token))) + } else { + Ok(Self::new(user, None)) + } + } +} + +impl TryFrom<String> for Authid { + type Error = Error; + + fn try_from(mut data: String) -> Result<Self, Error> { + let name_len = data + .as_bytes() + .iter() + .rposition(|&b| b == b'@') + .ok_or_else(|| format_err!("not a valid user id"))?; + + let realm_end = data + .as_bytes() + .iter() + .rposition(|&b| b == b'!') + .map(|pos| if pos < name_len { data.len() } else { pos }) + .unwrap_or(data.len()); + + if realm_end == data.len() - 1 { + bail!("empty token name in userid"); + } + + let tokenname = if data.len() > realm_end { + Some(Tokenname::try_from(data[(realm_end + 1)..].to_string())?) + } else { + None + }; + + data.truncate(realm_end); + + let user:Userid = data.parse()?; + + Ok(Self { user, tokenname }) + } +} + +#[test] +fn test_token_id() { + let userid: Userid = "test@pam".parse().expect("parsing Userid failed"); + assert_eq!(userid.name().as_str(), "test"); + assert_eq!(userid.realm(), "pam"); + assert_eq!(userid, "test@pam"); + + let auth_id: Authid = "test@pam".parse().expect("parsing user Authid failed"); + assert_eq!(auth_id.to_string(), "test@pam".to_string()); + assert!(!auth_id.is_token()); + + assert_eq!(auth_id.user(), &userid); + + let user_auth_id = Authid::from(userid.clone()); + assert_eq!(user_auth_id, auth_id); + assert!(!user_auth_id.is_token()); + + let auth_id: Authid = "test@pam!bar".parse().expect("parsing token Authid failed"); + let token_userid = auth_id.user(); + assert_eq!(&userid, token_userid); + assert!(auth_id.is_token()); + assert_eq!(auth_id.tokenname().expect("Token has tokenname").as_str(), TokennameRef::new("bar").as_str()); + assert_eq!(auth_id.to_string(), "test@pam!bar".to_string()); +} + proxmox::forward_deserialize_to_from_str!(Userid); proxmox::forward_serialize_to_display!(Userid); + +proxmox::forward_deserialize_to_from_str!(Authid); +proxmox::forward_serialize_to_display!(Authid); -- 2.20.1 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [pbs-devel] [PATCH proxmox] rpcenv: rename user to auth_id 2020-10-28 11:36 [pbs-devel] [PATCH proxmox-backup 00/16] API tokens Fabian Grünbichler 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-widget-toolkit] add PermissionView Fabian Grünbichler 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-backup 01/16] api: add Authid as wrapper around Userid Fabian Grünbichler @ 2020-10-28 11:36 ` Fabian Grünbichler 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-backup 02/16] config: add token.shadow file Fabian Grünbichler ` (10 subsequent siblings) 13 siblings, 0 replies; 25+ messages in thread From: Fabian Grünbichler @ 2020-10-28 11:36 UTC (permalink / raw) To: pbs-devel since it does no longer store just a userid, but potentially an API token identifier as well Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> --- proxmox/src/api/cli/environment.rs | 10 +++++----- proxmox/src/api/rpc_environment.rs | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/proxmox/src/api/cli/environment.rs b/proxmox/src/api/cli/environment.rs index 4831e3f..6085292 100644 --- a/proxmox/src/api/cli/environment.rs +++ b/proxmox/src/api/cli/environment.rs @@ -6,7 +6,7 @@ use crate::api::{RpcEnvironment, RpcEnvironmentType}; #[derive(Default)] pub struct CliEnvironment { result_attributes: Value, - user: Option<String>, + auth_id: Option<String>, } impl CliEnvironment { @@ -28,11 +28,11 @@ impl RpcEnvironment for CliEnvironment { RpcEnvironmentType::CLI } - fn set_user(&mut self, user: Option<String>) { - self.user = user; + fn set_auth_id(&mut self, auth_id: Option<String>) { + self.auth_id = auth_id; } - fn get_user(&self) -> Option<String> { - self.user.clone() + fn get_auth_id(&self) -> Option<String> { + self.auth_id.clone() } } diff --git a/proxmox/src/api/rpc_environment.rs b/proxmox/src/api/rpc_environment.rs index 430f3a6..0dc06d9 100644 --- a/proxmox/src/api/rpc_environment.rs +++ b/proxmox/src/api/rpc_environment.rs @@ -14,11 +14,11 @@ pub trait RpcEnvironment: std::any::Any + AsAny + Send { /// The environment type fn env_type(&self) -> RpcEnvironmentType; - /// Set user name - fn set_user(&mut self, user: Option<String>); + /// Set authentication id + fn set_auth_id(&mut self, user: Option<String>); - /// Get user name - fn get_user(&self) -> Option<String>; + /// Get authentication id + fn get_auth_id(&self) -> Option<String>; /// Set the client IP, should be re-set if a proxied connection was detected fn set_client_ip(&mut self, _client_ip: Option<std::net::SocketAddr>) { -- 2.20.1 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 02/16] config: add token.shadow file 2020-10-28 11:36 [pbs-devel] [PATCH proxmox-backup 00/16] API tokens Fabian Grünbichler ` (2 preceding siblings ...) 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox] rpcenv: rename user to auth_id Fabian Grünbichler @ 2020-10-28 11:36 ` Fabian Grünbichler 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-backup 03/16] replace Userid with Authid Fabian Grünbichler ` (9 subsequent siblings) 13 siblings, 0 replies; 25+ messages in thread From: Fabian Grünbichler @ 2020-10-28 11:36 UTC (permalink / raw) To: pbs-devel containing pairs of token ids and hashed secret values. Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> --- src/config.rs | 1 + src/config/token_shadow.rs | 91 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+) create mode 100644 src/config/token_shadow.rs diff --git a/src/config.rs b/src/config.rs index 65c0577e..6f19da7c 100644 --- a/src/config.rs +++ b/src/config.rs @@ -21,6 +21,7 @@ pub mod datastore; pub mod network; pub mod remote; pub mod sync; +pub mod token_shadow; pub mod user; pub mod verify; diff --git a/src/config/token_shadow.rs b/src/config/token_shadow.rs new file mode 100644 index 00000000..4033450b --- /dev/null +++ b/src/config/token_shadow.rs @@ -0,0 +1,91 @@ +use std::collections::HashMap; +use std::time::Duration; + +use anyhow::{bail, format_err, Error}; +use serde::{Serialize, Deserialize}; +use serde_json::{from_value, Value}; + +use proxmox::tools::fs::{open_file_locked, CreateOptions}; + +use crate::api2::types::Authid; +use crate::auth; + +const LOCK_FILE: &str = "/etc/proxmox-backup/token.shadow.lock"; +const CONF_FILE: &str = "/etc/proxmox-backup/token.shadow"; +const LOCK_TIMEOUT: Duration = Duration::from_secs(5); + +#[serde(rename_all="kebab-case")] +#[derive(Serialize, Deserialize)] +/// ApiToken id / secret pair +pub struct ApiTokenSecret { + pub tokenid: Authid, + pub secret: String, +} + +fn read_file() -> Result<HashMap<Authid, String>, Error> { + let json = proxmox::tools::fs::file_get_json(CONF_FILE, Some(Value::Null))?; + + if json == Value::Null { + Ok(HashMap::new()) + } else { + // swallow serde error which might contain sensitive data + from_value(json).map_err(|_err| format_err!("unable to parse '{}'", CONF_FILE)) + } +} + +fn write_file(data: HashMap<Authid, String>) -> Result<(), Error> { + let backup_user = crate::backup::backup_user()?; + let options = CreateOptions::new() + .perm(nix::sys::stat::Mode::from_bits_truncate(0o0640)) + .owner(backup_user.uid) + .group(backup_user.gid); + + let json = serde_json::to_vec(&data)?; + proxmox::tools::fs::replace_file(CONF_FILE, &json, options) +} + +/// Verifies that an entry for given tokenid / API token secret exists +pub fn verify_secret(tokenid: &Authid, secret: &str) -> Result<(), Error> { + if !tokenid.is_token() { + bail!("not an API token ID"); + } + + let data = read_file()?; + match data.get(tokenid) { + Some(hashed_secret) => { + auth::verify_crypt_pw(secret, &hashed_secret) + }, + None => bail!("invalid API token"), + } +} + +/// Adds a new entry for the given tokenid / API token secret. The secret is stored as salted hash. +pub fn set_secret(tokenid: &Authid, secret: &str) -> Result<(), Error> { + if !tokenid.is_token() { + bail!("not an API token ID"); + } + + let _guard = open_file_locked(LOCK_FILE, LOCK_TIMEOUT, true)?; + + let mut data = read_file()?; + let hashed_secret = auth::encrypt_pw(secret)?; + data.insert(tokenid.clone(), hashed_secret); + write_file(data)?; + + Ok(()) +} + +/// Deletes the entry for the given tokenid. +pub fn delete_secret(tokenid: &Authid) -> Result<(), Error> { + if !tokenid.is_token() { + bail!("not an API token ID"); + } + + let _guard = open_file_locked(LOCK_FILE, LOCK_TIMEOUT, true)?; + + let mut data = read_file()?; + data.remove(tokenid); + write_file(data)?; + + Ok(()) +} -- 2.20.1 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 03/16] replace Userid with Authid 2020-10-28 11:36 [pbs-devel] [PATCH proxmox-backup 00/16] API tokens Fabian Grünbichler ` (3 preceding siblings ...) 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-backup 02/16] config: add token.shadow file Fabian Grünbichler @ 2020-10-28 11:36 ` Fabian Grünbichler 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-backup 04/16] REST: extract and handle API tokens Fabian Grünbichler ` (8 subsequent siblings) 13 siblings, 0 replies; 25+ messages in thread From: Fabian Grünbichler @ 2020-10-28 11:36 UTC (permalink / raw) To: pbs-devel in most generic places. this is accompanied by a change in RpcEnvironment to purposefully break existing call sites. Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> --- Notes: requires corresponding proxmox patch + dependency bump some places remain untouched for more involved follow-ups, e.g. remote.cfg field names, termproxy stuff src/api2/access.rs | 11 +-- src/api2/access/acl.rs | 19 ++--- src/api2/access/user.rs | 7 +- src/api2/admin/datastore.rs | 103 +++++++++++++------------ src/api2/admin/sync.rs | 4 +- src/api2/admin/verify.rs | 4 +- src/api2/backup.rs | 14 ++-- src/api2/backup/environment.rs | 18 ++--- src/api2/config/datastore.rs | 4 +- src/api2/config/remote.rs | 4 +- src/api2/node.rs | 9 ++- src/api2/node/apt.rs | 6 +- src/api2/node/disks.rs | 6 +- src/api2/node/disks/directory.rs | 4 +- src/api2/node/disks/zfs.rs | 4 +- src/api2/node/network.rs | 4 +- src/api2/node/services.rs | 22 +++--- src/api2/node/subscription.rs | 6 +- src/api2/node/tasks.rs | 30 +++---- src/api2/pull.rs | 24 +++--- src/api2/reader.rs | 10 +-- src/api2/reader/environment.rs | 16 ++-- src/api2/status.rs | 12 +-- src/api2/types/mod.rs | 16 ++-- src/backup/datastore.rs | 16 ++-- src/bin/proxmox-backup-client.rs | 6 +- src/bin/proxmox-backup-manager.rs | 2 +- src/bin/proxmox-backup-proxy.rs | 16 ++-- src/bin/proxmox_backup_manager/acl.rs | 2 +- src/bin/proxmox_backup_manager/user.rs | 4 +- src/client/pull.rs | 8 +- src/config/acl.rs | 64 +++++++-------- src/config/cached_user_info.rs | 63 ++++++++++----- src/config/remote.rs | 2 +- src/config/user.rs | 69 +++++++++++++++-- src/server/environment.rs | 12 +-- src/server/rest.rs | 38 ++++----- src/server/upid.rs | 16 ++-- src/server/verify_job.rs | 6 +- src/server/worker_task.rs | 14 ++-- tests/worker-task-abort.rs | 2 +- www/config/ACLView.js | 2 +- www/window/ACLEdit.js | 2 +- 43 files changed, 399 insertions(+), 302 deletions(-) diff --git a/src/api2/access.rs b/src/api2/access.rs index c302e0c7..d0494c9a 100644 --- a/src/api2/access.rs +++ b/src/api2/access.rs @@ -31,7 +31,8 @@ fn authenticate_user( ) -> Result<bool, Error> { let user_info = CachedUserInfo::new()?; - if !user_info.is_active_user(&userid) { + let auth_id = Authid::from(userid.clone()); + if !user_info.is_active_auth_id(&auth_id) { bail!("user account disabled or expired."); } @@ -69,8 +70,7 @@ fn authenticate_user( path_vec.push(part); } } - - user_info.check_privs(userid, &path_vec, *privilege, false)?; + user_info.check_privs(&auth_id, &path_vec, *privilege, false)?; return Ok(false); } } @@ -213,9 +213,10 @@ fn change_password( ) -> Result<Value, Error> { let current_user: Userid = rpcenv - .get_user() + .get_auth_id() .ok_or_else(|| format_err!("unknown user"))? .parse()?; + let current_auth = Authid::from(current_user.clone()); let mut allowed = userid == current_user; @@ -223,7 +224,7 @@ fn change_password( if !allowed { let user_info = CachedUserInfo::new()?; - let privs = user_info.lookup_privs(¤t_user, &[]); + let privs = user_info.lookup_privs(¤t_auth, &[]); if (privs & PRIV_PERMISSIONS_MODIFY) != 0 { allowed = true; } } diff --git a/src/api2/access/acl.rs b/src/api2/access/acl.rs index 3282c66e..7211a6be 100644 --- a/src/api2/access/acl.rs +++ b/src/api2/access/acl.rs @@ -140,9 +140,9 @@ pub fn read_acl( optional: true, schema: ACL_PROPAGATE_SCHEMA, }, - userid: { + auth_id: { optional: true, - type: Userid, + type: Authid, }, group: { optional: true, @@ -168,7 +168,7 @@ pub fn update_acl( path: String, role: String, propagate: Option<bool>, - userid: Option<Userid>, + auth_id: Option<Authid>, group: Option<String>, delete: Option<bool>, digest: Option<String>, @@ -190,11 +190,12 @@ pub fn update_acl( if let Some(ref _group) = group { bail!("parameter 'group' - groups are currently not supported."); - } else if let Some(ref userid) = userid { + } else if let Some(ref auth_id) = auth_id { if !delete { // Note: we allow to delete non-existent users let user_cfg = crate::config::user::cached_config()?; - if user_cfg.sections.get(&userid.to_string()).is_none() { - bail!("no such user."); + if user_cfg.sections.get(&auth_id.to_string()).is_none() { + bail!(format!("no such {}.", + if auth_id.is_token() { "API token" } else { "user" })); } } } else { @@ -205,11 +206,11 @@ pub fn update_acl( acl::check_acl_path(&path)?; } - if let Some(userid) = userid { + if let Some(auth_id) = auth_id { if delete { - tree.delete_user_role(&path, &userid, &role); + tree.delete_user_role(&path, &auth_id, &role); } else { - tree.insert_user_role(&path, &userid, &role, propagate); + tree.insert_user_role(&path, &auth_id, &role, propagate); } } else if let Some(group) = group { if delete { diff --git a/src/api2/access/user.rs b/src/api2/access/user.rs index c041d804..47f8e1d1 100644 --- a/src/api2/access/user.rs +++ b/src/api2/access/user.rs @@ -39,10 +39,13 @@ pub fn list_users( let (config, digest) = user::config()?; - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + // intentionally user only for now + let userid: Userid = rpcenv.get_auth_id().unwrap().parse()?; + let auth_id = Authid::from(userid.clone()); + let user_info = CachedUserInfo::new()?; - let top_level_privs = user_info.lookup_privs(&userid, &["access", "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| { diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs index e1a0aacc..8862637d 100644 --- a/src/api2/admin/datastore.rs +++ b/src/api2/admin/datastore.rs @@ -47,11 +47,11 @@ use crate::config::acl::{ fn check_backup_owner( store: &DataStore, group: &BackupGroup, - userid: &Userid, + auth_id: &Authid, ) -> Result<(), Error> { let owner = store.get_owner(group)?; - if &owner != userid { - bail!("backup owner check failed ({} != {})", userid, owner); + if &owner != auth_id { + bail!("backup owner check failed ({} != {})", auth_id, owner); } Ok(()) } @@ -149,9 +149,9 @@ fn list_groups( rpcenv: &mut dyn RpcEnvironment, ) -> Result<Vec<GroupListItem>, Error> { - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let user_info = CachedUserInfo::new()?; - let user_privs = user_info.lookup_privs(&userid, &["datastore", &store]); + let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]); let datastore = DataStore::lookup_datastore(&store)?; @@ -171,7 +171,7 @@ fn list_groups( let list_all = (user_privs & PRIV_DATASTORE_AUDIT) != 0; let owner = datastore.get_owner(group)?; - if !list_all && owner != userid { + if !list_all && owner != auth_id { continue; } @@ -230,16 +230,16 @@ pub fn list_snapshot_files( rpcenv: &mut dyn RpcEnvironment, ) -> Result<Vec<BackupContent>, Error> { - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let user_info = CachedUserInfo::new()?; - let user_privs = user_info.lookup_privs(&userid, &["datastore", &store]); + let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]); let datastore = DataStore::lookup_datastore(&store)?; let snapshot = BackupDir::new(backup_type, backup_id, backup_time)?; let allowed = (user_privs & (PRIV_DATASTORE_AUDIT | PRIV_DATASTORE_READ)) != 0; - if !allowed { check_backup_owner(&datastore, snapshot.group(), &userid)?; } + if !allowed { check_backup_owner(&datastore, snapshot.group(), &auth_id)?; } let info = BackupInfo::new(&datastore.base_path(), snapshot)?; @@ -282,16 +282,16 @@ fn delete_snapshot( rpcenv: &mut dyn RpcEnvironment, ) -> Result<Value, Error> { - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let user_info = CachedUserInfo::new()?; - let user_privs = user_info.lookup_privs(&userid, &["datastore", &store]); + let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]); let snapshot = BackupDir::new(backup_type, backup_id, backup_time)?; let datastore = DataStore::lookup_datastore(&store)?; let allowed = (user_privs & PRIV_DATASTORE_MODIFY) != 0; - if !allowed { check_backup_owner(&datastore, snapshot.group(), &userid)?; } + if !allowed { check_backup_owner(&datastore, snapshot.group(), &auth_id)?; } datastore.remove_backup_dir(&snapshot, false)?; @@ -338,9 +338,9 @@ pub fn list_snapshots ( rpcenv: &mut dyn RpcEnvironment, ) -> Result<Vec<SnapshotListItem>, Error> { - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let user_info = CachedUserInfo::new()?; - let user_privs = user_info.lookup_privs(&userid, &["datastore", &store]); + let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]); let datastore = DataStore::lookup_datastore(&store)?; @@ -362,7 +362,7 @@ pub fn list_snapshots ( let list_all = (user_privs & PRIV_DATASTORE_AUDIT) != 0; let owner = datastore.get_owner(group)?; - if !list_all && owner != userid { + if !list_all && owner != auth_id { continue; } @@ -568,13 +568,13 @@ pub fn verify( _ => bail!("parameters do not specify a backup group or snapshot"), } - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let to_stdout = if rpcenv.env_type() == RpcEnvironmentType::CLI { true } else { false }; let upid_str = WorkerTask::new_thread( worker_type, Some(worker_id.clone()), - userid, + auth_id, to_stdout, move |worker| { let verified_chunks = Arc::new(Mutex::new(HashSet::with_capacity(1024*16))); @@ -701,9 +701,9 @@ fn prune( let backup_type = tools::required_string_param(¶m, "backup-type")?; let backup_id = tools::required_string_param(¶m, "backup-id")?; - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let user_info = CachedUserInfo::new()?; - let user_privs = user_info.lookup_privs(&userid, &["datastore", &store]); + let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]); let dry_run = param["dry-run"].as_bool().unwrap_or(false); @@ -712,7 +712,7 @@ fn prune( let datastore = DataStore::lookup_datastore(&store)?; let allowed = (user_privs & PRIV_DATASTORE_MODIFY) != 0; - if !allowed { check_backup_owner(&datastore, &group, &userid)?; } + if !allowed { check_backup_owner(&datastore, &group, &auth_id)?; } let prune_options = PruneOptions { keep_last: param["keep-last"].as_u64(), @@ -754,7 +754,7 @@ fn prune( // We use a WorkerTask just to have a task log, but run synchrounously - let worker = WorkerTask::new("prune", Some(worker_id), Userid::root_userid().clone(), true)?; + let worker = WorkerTask::new("prune", Some(worker_id), auth_id.clone(), true)?; if keep_all { worker.log("No prune selection - keeping all files."); @@ -829,6 +829,7 @@ fn start_garbage_collection( ) -> Result<Value, Error> { let datastore = DataStore::lookup_datastore(&store)?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; println!("Starting garbage collection on store {}", store); @@ -837,7 +838,7 @@ fn start_garbage_collection( let upid_str = WorkerTask::new_thread( "garbage_collection", Some(store.clone()), - Userid::root_userid().clone(), + auth_id.clone(), to_stdout, move |worker| { worker.log(format!("starting garbage collection on store {}", store)); @@ -907,13 +908,13 @@ fn get_datastore_list( let (config, _digest) = datastore::config()?; - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let user_info = CachedUserInfo::new()?; let mut list = Vec::new(); for (store, (_, data)) in &config.sections { - let user_privs = user_info.lookup_privs(&userid, &["datastore", &store]); + let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]); let allowed = (user_privs & (PRIV_DATASTORE_AUDIT| PRIV_DATASTORE_BACKUP)) != 0; if allowed { let mut entry = json!({ "store": store }); @@ -958,9 +959,9 @@ fn download_file( let store = tools::required_string_param(¶m, "store")?; let datastore = DataStore::lookup_datastore(store)?; - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let user_info = CachedUserInfo::new()?; - let user_privs = user_info.lookup_privs(&userid, &["datastore", &store]); + let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]); let file_name = tools::required_string_param(¶m, "file-name")?.to_owned(); @@ -971,7 +972,7 @@ fn download_file( let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?; let allowed = (user_privs & PRIV_DATASTORE_READ) != 0; - if !allowed { check_backup_owner(&datastore, backup_dir.group(), &userid)?; } + if !allowed { check_backup_owner(&datastore, backup_dir.group(), &auth_id)?; } println!("Download {} from {} ({}/{})", file_name, store, backup_dir, file_name); @@ -1031,9 +1032,9 @@ fn download_file_decoded( let store = tools::required_string_param(¶m, "store")?; let datastore = DataStore::lookup_datastore(store)?; - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let user_info = CachedUserInfo::new()?; - let user_privs = user_info.lookup_privs(&userid, &["datastore", &store]); + let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]); let file_name = tools::required_string_param(¶m, "file-name")?.to_owned(); @@ -1044,7 +1045,7 @@ fn download_file_decoded( let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?; let allowed = (user_privs & PRIV_DATASTORE_READ) != 0; - if !allowed { check_backup_owner(&datastore, backup_dir.group(), &userid)?; } + if !allowed { check_backup_owner(&datastore, backup_dir.group(), &auth_id)?; } let (manifest, files) = read_backup_index(&datastore, &backup_dir)?; for file in files { @@ -1156,8 +1157,8 @@ fn upload_backup_log( let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?; - let userid: Userid = rpcenv.get_user().unwrap().parse()?; - check_backup_owner(&datastore, backup_dir.group(), &userid)?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; + check_backup_owner(&datastore, backup_dir.group(), &auth_id)?; let mut path = datastore.base_path(); path.push(backup_dir.relative_path()); @@ -1226,14 +1227,14 @@ fn catalog( ) -> Result<Value, Error> { let datastore = DataStore::lookup_datastore(&store)?; - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let user_info = CachedUserInfo::new()?; - let user_privs = user_info.lookup_privs(&userid, &["datastore", &store]); + let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]); let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?; let allowed = (user_privs & PRIV_DATASTORE_READ) != 0; - if !allowed { check_backup_owner(&datastore, backup_dir.group(), &userid)?; } + if !allowed { check_backup_owner(&datastore, backup_dir.group(), &auth_id)?; } let file_name = CATALOG_NAME; @@ -1397,9 +1398,9 @@ fn pxar_file_download( let store = tools::required_string_param(¶m, "store")?; let datastore = DataStore::lookup_datastore(&store)?; - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let user_info = CachedUserInfo::new()?; - let user_privs = user_info.lookup_privs(&userid, &["datastore", &store]); + let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]); let filepath = tools::required_string_param(¶m, "filepath")?.to_owned(); @@ -1410,7 +1411,7 @@ fn pxar_file_download( let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?; let allowed = (user_privs & PRIV_DATASTORE_READ) != 0; - if !allowed { check_backup_owner(&datastore, backup_dir.group(), &userid)?; } + if !allowed { check_backup_owner(&datastore, backup_dir.group(), &auth_id)?; } let mut components = base64::decode(&filepath)?; if components.len() > 0 && components[0] == '/' as u8 { @@ -1576,14 +1577,14 @@ fn get_notes( ) -> Result<String, Error> { let datastore = DataStore::lookup_datastore(&store)?; - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let user_info = CachedUserInfo::new()?; - let user_privs = user_info.lookup_privs(&userid, &["datastore", &store]); + let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]); let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?; let allowed = (user_privs & PRIV_DATASTORE_READ) != 0; - if !allowed { check_backup_owner(&datastore, backup_dir.group(), &userid)?; } + if !allowed { check_backup_owner(&datastore, backup_dir.group(), &auth_id)?; } let (manifest, _) = datastore.load_manifest(&backup_dir)?; @@ -1629,14 +1630,14 @@ fn set_notes( ) -> Result<(), Error> { let datastore = DataStore::lookup_datastore(&store)?; - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let user_info = CachedUserInfo::new()?; - let user_privs = user_info.lookup_privs(&userid, &["datastore", &store]); + let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]); let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?; let allowed = (user_privs & PRIV_DATASTORE_READ) != 0; - if !allowed { check_backup_owner(&datastore, backup_dir.group(), &userid)?; } + if !allowed { check_backup_owner(&datastore, backup_dir.group(), &auth_id)?; } datastore.update_manifest(&backup_dir,|manifest| { manifest.unprotected["notes"] = notes.into(); @@ -1658,7 +1659,7 @@ fn set_notes( schema: BACKUP_ID_SCHEMA, }, "new-owner": { - type: Userid, + type: Authid, }, }, }, @@ -1671,7 +1672,7 @@ fn set_backup_owner( store: String, backup_type: String, backup_id: String, - new_owner: Userid, + new_owner: Authid, _rpcenv: &mut dyn RpcEnvironment, ) -> Result<(), Error> { @@ -1681,8 +1682,14 @@ fn set_backup_owner( let user_info = CachedUserInfo::new()?; - if !user_info.is_active_user(&new_owner) { - bail!("user '{}' is inactive or non-existent", new_owner); + if !user_info.is_active_auth_id(&new_owner) { + bail!("{} '{}' is inactive or non-existent", + if new_owner.is_token() { + "API token".to_string() + } else { + "user".to_string() + }, + new_owner); } datastore.set_owner(&backup_group, &new_owner, true)?; diff --git a/src/api2/admin/sync.rs b/src/api2/admin/sync.rs index 4a104441..f7032a3a 100644 --- a/src/api2/admin/sync.rs +++ b/src/api2/admin/sync.rs @@ -101,11 +101,11 @@ fn run_sync_job( let (config, _digest) = sync::config()?; let sync_job: SyncJobConfig = config.lookup("sync", &id)?; - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let job = Job::new("syncjob", &id)?; - let upid_str = do_sync_job(job, sync_job, &userid, None)?; + let upid_str = do_sync_job(job, sync_job, &auth_id, None)?; Ok(upid_str) } diff --git a/src/api2/admin/verify.rs b/src/api2/admin/verify.rs index 4bc184d3..02c1eef4 100644 --- a/src/api2/admin/verify.rs +++ b/src/api2/admin/verify.rs @@ -101,11 +101,11 @@ fn run_verification_job( let (config, _digest) = verify::config()?; let verification_job: VerificationJobConfig = config.lookup("verification", &id)?; - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let job = Job::new("verificationjob", &id)?; - let upid_str = do_verification_job(job, verification_job, &userid, None)?; + let upid_str = do_verification_job(job, verification_job, &auth_id, None)?; Ok(upid_str) } diff --git a/src/api2/backup.rs b/src/api2/backup.rs index 8e58efdd..e030d60d 100644 --- a/src/api2/backup.rs +++ b/src/api2/backup.rs @@ -59,12 +59,12 @@ async move { let debug = param["debug"].as_bool().unwrap_or(false); let benchmark = param["benchmark"].as_bool().unwrap_or(false); - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let store = tools::required_string_param(¶m, "store")?.to_owned(); let user_info = CachedUserInfo::new()?; - user_info.check_privs(&userid, &["datastore", &store], PRIV_DATASTORE_BACKUP, false)?; + user_info.check_privs(&auth_id, &["datastore", &store], PRIV_DATASTORE_BACKUP, false)?; let datastore = DataStore::lookup_datastore(&store)?; @@ -105,12 +105,12 @@ async move { }; // lock backup group to only allow one backup per group at a time - let (owner, _group_guard) = datastore.create_locked_backup_group(&backup_group, &userid)?; + let (owner, _group_guard) = datastore.create_locked_backup_group(&backup_group, &auth_id)?; // permission check - if owner != userid && worker_type != "benchmark" { + if owner != auth_id && worker_type != "benchmark" { // only the owner is allowed to create additional snapshots - bail!("backup owner check failed ({} != {})", userid, owner); + bail!("backup owner check failed ({} != {})", auth_id, owner); } let last_backup = { @@ -153,9 +153,9 @@ async move { if !is_new { bail!("backup directory already exists."); } - WorkerTask::spawn(worker_type, Some(worker_id), userid.clone(), true, move |worker| { + WorkerTask::spawn(worker_type, Some(worker_id), auth_id.clone(), true, move |worker| { let mut env = BackupEnvironment::new( - env_type, userid, worker.clone(), datastore, backup_dir); + env_type, auth_id, worker.clone(), datastore, backup_dir); env.debug = debug; env.last_backup = last_backup; diff --git a/src/api2/backup/environment.rs b/src/api2/backup/environment.rs index 7f0476c3..0fb37807 100644 --- a/src/api2/backup/environment.rs +++ b/src/api2/backup/environment.rs @@ -10,7 +10,7 @@ use proxmox::tools::digest_to_hex; use proxmox::tools::fs::{replace_file, CreateOptions}; use proxmox::api::{RpcEnvironment, RpcEnvironmentType}; -use crate::api2::types::Userid; +use crate::api2::types::Authid; use crate::backup::*; use crate::server::WorkerTask; use crate::server::formatter::*; @@ -104,7 +104,7 @@ impl SharedBackupState { pub struct BackupEnvironment { env_type: RpcEnvironmentType, result_attributes: Value, - user: Userid, + auth_id: Authid, pub debug: bool, pub formatter: &'static OutputFormatter, pub worker: Arc<WorkerTask>, @@ -117,7 +117,7 @@ pub struct BackupEnvironment { impl BackupEnvironment { pub fn new( env_type: RpcEnvironmentType, - user: Userid, + auth_id: Authid, worker: Arc<WorkerTask>, datastore: Arc<DataStore>, backup_dir: BackupDir, @@ -137,7 +137,7 @@ impl BackupEnvironment { Self { result_attributes: json!({}), env_type, - user, + auth_id, worker, datastore, debug: false, @@ -518,7 +518,7 @@ impl BackupEnvironment { WorkerTask::new_thread( "verify", Some(worker_id), - self.user.clone(), + self.auth_id.clone(), false, move |worker| { worker.log("Automatically verifying newly added snapshot"); @@ -598,12 +598,12 @@ impl RpcEnvironment for BackupEnvironment { self.env_type } - fn set_user(&mut self, _user: Option<String>) { - panic!("unable to change user"); + fn set_auth_id(&mut self, _auth_id: Option<String>) { + panic!("unable to change auth_id"); } - fn get_user(&self) -> Option<String> { - Some(self.user.to_string()) + fn get_auth_id(&self) -> Option<String> { + Some(self.auth_id.to_string()) } } diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs index 4847bdad..1da82593 100644 --- a/src/api2/config/datastore.rs +++ b/src/api2/config/datastore.rs @@ -35,14 +35,14 @@ pub fn list_datastores( let (config, digest) = datastore::config()?; - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let user_info = CachedUserInfo::new()?; rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into(); let list:Vec<DataStoreConfig> = config.convert_to_typed_array("datastore")?; let filter_by_privs = |store: &DataStoreConfig| { - let user_privs = user_info.lookup_privs(&userid, &["datastore", &store.name]); + let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store.name]); (user_privs & PRIV_DATASTORE_AUDIT) != 0 }; diff --git a/src/api2/config/remote.rs b/src/api2/config/remote.rs index d419be2b..dd2777c9 100644 --- a/src/api2/config/remote.rs +++ b/src/api2/config/remote.rs @@ -66,7 +66,7 @@ pub fn list_remotes( default: 8007, }, userid: { - type: Userid, + type: Authid, }, password: { schema: remote::REMOTE_PASSWORD_SCHEMA, @@ -167,7 +167,7 @@ pub enum DeletableProperty { }, userid: { optional: true, - type: Userid, + type: Authid, }, password: { optional: true, diff --git a/src/api2/node.rs b/src/api2/node.rs index 4689c494..d06a0cb6 100644 --- a/src/api2/node.rs +++ b/src/api2/node.rs @@ -91,10 +91,12 @@ async fn termproxy( cmd: Option<String>, rpcenv: &mut dyn RpcEnvironment, ) -> Result<Value, Error> { + // intentionally user only for now let userid: Userid = rpcenv - .get_user() + .get_auth_id() .ok_or_else(|| format_err!("unknown user"))? .parse()?; + let auth_id = Authid::from(userid.clone()); if userid.realm() != "pam" { bail!("only pam users can use the console"); @@ -137,7 +139,7 @@ async fn termproxy( let upid = WorkerTask::spawn( "termproxy", None, - userid, + auth_id, false, move |worker| async move { // move inside the worker so that it survives and does not close the port @@ -272,7 +274,8 @@ fn upgrade_to_websocket( rpcenv: Box<dyn RpcEnvironment>, ) -> ApiResponseFuture { async move { - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + // intentionally user only for now + let userid: Userid = rpcenv.get_auth_id().unwrap().parse()?; let ticket = tools::required_string_param(¶m, "vncticket")?; let port: u16 = tools::required_integer_param(¶m, "port")? as u16; diff --git a/src/api2/node/apt.rs b/src/api2/node/apt.rs index e8d4094b..ba72d352 100644 --- a/src/api2/node/apt.rs +++ b/src/api2/node/apt.rs @@ -12,7 +12,7 @@ use crate::server::WorkerTask; use crate::tools::http; use crate::config::acl::{PRIV_SYS_AUDIT, PRIV_SYS_MODIFY}; -use crate::api2::types::{APTUpdateInfo, NODE_SCHEMA, Userid, UPID_SCHEMA}; +use crate::api2::types::{Authid, APTUpdateInfo, NODE_SCHEMA, UPID_SCHEMA}; const_regex! { VERSION_EPOCH_REGEX = r"^\d+:"; @@ -351,11 +351,11 @@ pub fn apt_update_database( rpcenv: &mut dyn RpcEnvironment, ) -> Result<String, Error> { - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let to_stdout = if rpcenv.env_type() == RpcEnvironmentType::CLI { true } else { false }; let quiet = quiet.unwrap_or(API_METHOD_APT_UPDATE_DATABASE_PARAM_DEFAULT_QUIET); - let upid_str = WorkerTask::new_thread("aptupdate", None, userid, to_stdout, move |worker| { + let upid_str = WorkerTask::new_thread("aptupdate", None, auth_id, to_stdout, move |worker| { if !quiet { worker.log("starting apt-get update") } // TODO: set proxy /etc/apt/apt.conf.d/76pbsproxy like PVE diff --git a/src/api2/node/disks.rs b/src/api2/node/disks.rs index eed9e257..97e04edd 100644 --- a/src/api2/node/disks.rs +++ b/src/api2/node/disks.rs @@ -13,7 +13,7 @@ use crate::tools::disks::{ }; use crate::server::WorkerTask; -use crate::api2::types::{Userid, UPID_SCHEMA, NODE_SCHEMA, BLOCKDEVICE_NAME_SCHEMA}; +use crate::api2::types::{Authid, UPID_SCHEMA, NODE_SCHEMA, BLOCKDEVICE_NAME_SCHEMA}; pub mod directory; pub mod zfs; @@ -140,7 +140,7 @@ pub fn initialize_disk( let to_stdout = if rpcenv.env_type() == RpcEnvironmentType::CLI { true } else { false }; - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let info = get_disk_usage_info(&disk, true)?; @@ -149,7 +149,7 @@ pub fn initialize_disk( } let upid_str = WorkerTask::new_thread( - "diskinit", Some(disk.clone()), userid, to_stdout, move |worker| + "diskinit", Some(disk.clone()), auth_id, to_stdout, move |worker| { worker.log(format!("initialize disk {}", disk)); diff --git a/src/api2/node/disks/directory.rs b/src/api2/node/disks/directory.rs index 0d9ddeef..2a4780f2 100644 --- a/src/api2/node/disks/directory.rs +++ b/src/api2/node/disks/directory.rs @@ -134,7 +134,7 @@ pub fn create_datastore_disk( let to_stdout = if rpcenv.env_type() == RpcEnvironmentType::CLI { true } else { false }; - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let info = get_disk_usage_info(&disk, true)?; @@ -143,7 +143,7 @@ pub fn create_datastore_disk( } let upid_str = WorkerTask::new_thread( - "dircreate", Some(name.clone()), userid, to_stdout, move |worker| + "dircreate", Some(name.clone()), auth_id, to_stdout, move |worker| { worker.log(format!("create datastore '{}' on disk {}", name, disk)); diff --git a/src/api2/node/disks/zfs.rs b/src/api2/node/disks/zfs.rs index f70b91b6..79094012 100644 --- a/src/api2/node/disks/zfs.rs +++ b/src/api2/node/disks/zfs.rs @@ -256,7 +256,7 @@ pub fn create_zpool( let to_stdout = if rpcenv.env_type() == RpcEnvironmentType::CLI { true } else { false }; - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let add_datastore = add_datastore.unwrap_or(false); @@ -316,7 +316,7 @@ pub fn create_zpool( } let upid_str = WorkerTask::new_thread( - "zfscreate", Some(name.clone()), userid, to_stdout, move |worker| + "zfscreate", Some(name.clone()), auth_id, to_stdout, move |worker| { worker.log(format!("create {:?} zpool '{}' on devices '{}'", raidlevel, name, devices_text)); diff --git a/src/api2/node/network.rs b/src/api2/node/network.rs index efdc8afd..f737684d 100644 --- a/src/api2/node/network.rs +++ b/src/api2/node/network.rs @@ -684,9 +684,9 @@ pub async fn reload_network_config( network::assert_ifupdown2_installed()?; - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; - let upid_str = WorkerTask::spawn("srvreload", Some(String::from("networking")), userid, true, |_worker| async { + let upid_str = WorkerTask::spawn("srvreload", Some(String::from("networking")), auth_id, true, |_worker| async { let _ = std::fs::rename(network::NETWORK_INTERFACES_NEW_FILENAME, network::NETWORK_INTERFACES_FILENAME); diff --git a/src/api2/node/services.rs b/src/api2/node/services.rs index 849bead7..4c2a17b4 100644 --- a/src/api2/node/services.rs +++ b/src/api2/node/services.rs @@ -182,7 +182,7 @@ fn get_service_state( Ok(json_service_state(&service, status)) } -fn run_service_command(service: &str, cmd: &str, userid: Userid) -> Result<Value, Error> { +fn run_service_command(service: &str, cmd: &str, auth_id: Authid) -> Result<Value, Error> { let workerid = format!("srv{}", &cmd); @@ -196,7 +196,7 @@ fn run_service_command(service: &str, cmd: &str, userid: Userid) -> Result<Value let upid = WorkerTask::new_thread( &workerid, Some(service.clone()), - userid, + auth_id, false, move |_worker| { @@ -244,11 +244,11 @@ fn start_service( rpcenv: &mut dyn RpcEnvironment, ) -> Result<Value, Error> { - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; log::info!("starting service {}", service); - run_service_command(&service, "start", userid) + run_service_command(&service, "start", auth_id) } #[api( @@ -274,11 +274,11 @@ fn stop_service( rpcenv: &mut dyn RpcEnvironment, ) -> Result<Value, Error> { - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; log::info!("stopping service {}", service); - run_service_command(&service, "stop", userid) + run_service_command(&service, "stop", auth_id) } #[api( @@ -304,15 +304,15 @@ fn restart_service( rpcenv: &mut dyn RpcEnvironment, ) -> Result<Value, Error> { - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; log::info!("re-starting service {}", service); if &service == "proxmox-backup-proxy" { // special case, avoid aborting running tasks - run_service_command(&service, "reload", userid) + run_service_command(&service, "reload", auth_id) } else { - run_service_command(&service, "restart", userid) + run_service_command(&service, "restart", auth_id) } } @@ -339,11 +339,11 @@ fn reload_service( rpcenv: &mut dyn RpcEnvironment, ) -> Result<Value, Error> { - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; log::info!("reloading service {}", service); - run_service_command(&service, "reload", userid) + run_service_command(&service, "reload", auth_id) } diff --git a/src/api2/node/subscription.rs b/src/api2/node/subscription.rs index 0802f6a7..3ce7ce98 100644 --- a/src/api2/node/subscription.rs +++ b/src/api2/node/subscription.rs @@ -7,7 +7,7 @@ use crate::tools; use crate::tools::subscription::{self, SubscriptionStatus, SubscriptionInfo}; use crate::config::acl::{PRIV_SYS_AUDIT,PRIV_SYS_MODIFY}; use crate::config::cached_user_info::CachedUserInfo; -use crate::api2::types::{NODE_SCHEMA, Userid}; +use crate::api2::types::{NODE_SCHEMA, Authid}; #[api( input: { @@ -100,9 +100,9 @@ fn get_subscription( }, }; - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let user_info = CachedUserInfo::new()?; - let user_privs = user_info.lookup_privs(&userid, &[]); + let user_privs = user_info.lookup_privs(&auth_id, &[]); if (user_privs & PRIV_SYS_AUDIT) == 0 { // not enough privileges for full state diff --git a/src/api2/node/tasks.rs b/src/api2/node/tasks.rs index 4d16b20a..66af6d11 100644 --- a/src/api2/node/tasks.rs +++ b/src/api2/node/tasks.rs @@ -84,11 +84,11 @@ async fn get_task_status( let upid = extract_upid(¶m)?; - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; - if userid != upid.userid { + if auth_id != upid.auth_id { let user_info = CachedUserInfo::new()?; - user_info.check_privs(&userid, &["system", "tasks"], PRIV_SYS_AUDIT, false)?; + user_info.check_privs(&auth_id, &["system", "tasks"], PRIV_SYS_AUDIT, false)?; } let mut result = json!({ @@ -99,7 +99,7 @@ async fn get_task_status( "starttime": upid.starttime, "type": upid.worker_type, "id": upid.worker_id, - "user": upid.userid, + "user": upid.auth_id, }); if crate::server::worker_is_active(&upid).await? { @@ -161,11 +161,11 @@ async fn read_task_log( let upid = extract_upid(¶m)?; - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; - if userid != upid.userid { + if auth_id != upid.auth_id { let user_info = CachedUserInfo::new()?; - user_info.check_privs(&userid, &["system", "tasks"], PRIV_SYS_AUDIT, false)?; + user_info.check_privs(&auth_id, &["system", "tasks"], PRIV_SYS_AUDIT, false)?; } let test_status = param["test-status"].as_bool().unwrap_or(false); @@ -234,11 +234,11 @@ fn stop_task( let upid = extract_upid(¶m)?; - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; - if userid != upid.userid { + if auth_id != upid.auth_id { let user_info = CachedUserInfo::new()?; - user_info.check_privs(&userid, &["system", "tasks"], PRIV_SYS_MODIFY, false)?; + user_info.check_privs(&auth_id, &["system", "tasks"], PRIV_SYS_MODIFY, false)?; } server::abort_worker_async(upid); @@ -308,9 +308,9 @@ pub fn list_tasks( mut rpcenv: &mut dyn RpcEnvironment, ) -> Result<Vec<TaskListItem>, Error> { - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let user_info = CachedUserInfo::new()?; - let user_privs = user_info.lookup_privs(&userid, &["system", "tasks"]); + let user_privs = user_info.lookup_privs(&auth_id, &["system", "tasks"]); let list_all = (user_privs & PRIV_SYS_AUDIT) != 0; @@ -326,10 +326,10 @@ pub fn list_tasks( Err(_) => return None, }; - if !list_all && info.upid.userid != userid { return None; } + if !list_all && info.upid.auth_id != auth_id { return None; } - if let Some(userid) = &userfilter { - if !info.upid.userid.as_str().contains(userid) { return None; } + if let Some(needle) = &userfilter { + if !info.upid.auth_id.to_string().contains(needle) { return None; } } if let Some(store) = store { diff --git a/src/api2/pull.rs b/src/api2/pull.rs index 441701a5..aef7de4e 100644 --- a/src/api2/pull.rs +++ b/src/api2/pull.rs @@ -20,7 +20,7 @@ use crate::config::{ pub fn check_pull_privs( - userid: &Userid, + auth_id: &Authid, store: &str, remote: &str, remote_store: &str, @@ -29,11 +29,11 @@ pub fn check_pull_privs( let user_info = CachedUserInfo::new()?; - user_info.check_privs(userid, &["datastore", store], PRIV_DATASTORE_BACKUP, false)?; - user_info.check_privs(userid, &["remote", remote, remote_store], PRIV_REMOTE_READ, false)?; + user_info.check_privs(auth_id, &["datastore", store], PRIV_DATASTORE_BACKUP, false)?; + user_info.check_privs(auth_id, &["remote", remote, remote_store], PRIV_REMOTE_READ, false)?; if delete { - user_info.check_privs(userid, &["datastore", store], PRIV_DATASTORE_PRUNE, false)?; + user_info.check_privs(auth_id, &["datastore", store], PRIV_DATASTORE_PRUNE, false)?; } Ok(()) @@ -68,7 +68,7 @@ pub async fn get_pull_parameters( pub fn do_sync_job( mut job: Job, sync_job: SyncJobConfig, - userid: &Userid, + auth_id: &Authid, schedule: Option<String>, ) -> Result<String, Error> { @@ -78,7 +78,7 @@ pub fn do_sync_job( let upid_str = WorkerTask::spawn( &worker_type, Some(job.jobname().to_string()), - userid.clone(), + auth_id.clone(), false, move |worker| async move { @@ -98,7 +98,9 @@ pub fn do_sync_job( worker.log(format!("Sync datastore '{}' from '{}/{}'", sync_job.store, sync_job.remote, sync_job.remote_store)); - crate::client::pull::pull_store(&worker, &client, &src_repo, tgt_store.clone(), delete, Userid::backup_userid().clone()).await?; + let backup_auth_id = Authid::backup_auth_id(); + + crate::client::pull::pull_store(&worker, &client, &src_repo, tgt_store.clone(), delete, backup_auth_id.clone()).await?; worker.log(format!("sync job '{}' end", &job_id)); @@ -164,19 +166,19 @@ async fn pull ( rpcenv: &mut dyn RpcEnvironment, ) -> Result<String, Error> { - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let delete = remove_vanished.unwrap_or(true); - check_pull_privs(&userid, &store, &remote, &remote_store, delete)?; + check_pull_privs(&auth_id, &store, &remote, &remote_store, delete)?; let (client, src_repo, tgt_store) = get_pull_parameters(&store, &remote, &remote_store).await?; // fixme: set to_stdout to false? - let upid_str = WorkerTask::spawn("sync", Some(store.clone()), userid.clone(), true, move |worker| async move { + let upid_str = WorkerTask::spawn("sync", Some(store.clone()), auth_id.clone(), true, move |worker| async move { worker.log(format!("sync datastore '{}' start", store)); - let pull_future = pull_store(&worker, &client, &src_repo, tgt_store.clone(), delete, userid); + let pull_future = pull_store(&worker, &client, &src_repo, tgt_store.clone(), delete, auth_id); let future = select!{ success = pull_future.fuse() => success, abort = worker.abort_future().map(|_| Err(format_err!("pull aborted"))) => abort, diff --git a/src/api2/reader.rs b/src/api2/reader.rs index 0fa9b08e..3eeece52 100644 --- a/src/api2/reader.rs +++ b/src/api2/reader.rs @@ -55,11 +55,11 @@ fn upgrade_to_backup_reader_protocol( async move { let debug = param["debug"].as_bool().unwrap_or(false); - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let store = tools::required_string_param(¶m, "store")?.to_owned(); let user_info = CachedUserInfo::new()?; - let privs = user_info.lookup_privs(&userid, &["datastore", &store]); + let privs = user_info.lookup_privs(&auth_id, &["datastore", &store]); let priv_read = privs & PRIV_DATASTORE_READ != 0; let priv_backup = privs & PRIV_DATASTORE_BACKUP != 0; @@ -94,7 +94,7 @@ fn upgrade_to_backup_reader_protocol( let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?; if !priv_read { let owner = datastore.get_owner(backup_dir.group())?; - if owner != userid { + if owner != auth_id { bail!("backup owner check failed!"); } } @@ -110,10 +110,10 @@ fn upgrade_to_backup_reader_protocol( let worker_id = format!("{}:{}/{}/{:08X}", store, backup_type, backup_id, backup_dir.backup_time()); - WorkerTask::spawn("reader", Some(worker_id), userid.clone(), true, move |worker| { + WorkerTask::spawn("reader", Some(worker_id), auth_id.clone(), true, move |worker| { let mut env = ReaderEnvironment::new( env_type, - userid, + auth_id, worker.clone(), datastore, backup_dir, diff --git a/src/api2/reader/environment.rs b/src/api2/reader/environment.rs index 4c7db36f..e1cc2754 100644 --- a/src/api2/reader/environment.rs +++ b/src/api2/reader/environment.rs @@ -5,7 +5,7 @@ use serde_json::{json, Value}; use proxmox::api::{RpcEnvironment, RpcEnvironmentType}; -use crate::api2::types::Userid; +use crate::api2::types::Authid; use crate::backup::*; use crate::server::formatter::*; use crate::server::WorkerTask; @@ -17,7 +17,7 @@ use crate::server::WorkerTask; pub struct ReaderEnvironment { env_type: RpcEnvironmentType, result_attributes: Value, - user: Userid, + auth_id: Authid, pub debug: bool, pub formatter: &'static OutputFormatter, pub worker: Arc<WorkerTask>, @@ -29,7 +29,7 @@ pub struct ReaderEnvironment { impl ReaderEnvironment { pub fn new( env_type: RpcEnvironmentType, - user: Userid, + auth_id: Authid, worker: Arc<WorkerTask>, datastore: Arc<DataStore>, backup_dir: BackupDir, @@ -39,7 +39,7 @@ impl ReaderEnvironment { Self { result_attributes: json!({}), env_type, - user, + auth_id, worker, datastore, debug: false, @@ -82,12 +82,12 @@ impl RpcEnvironment for ReaderEnvironment { self.env_type } - fn set_user(&mut self, _user: Option<String>) { - panic!("unable to change user"); + fn set_auth_id(&mut self, _auth_id: Option<String>) { + panic!("unable to change auth_id"); } - fn get_user(&self) -> Option<String> { - Some(self.user.to_string()) + fn get_auth_id(&self) -> Option<String> { + Some(self.auth_id.to_string()) } } diff --git a/src/api2/status.rs b/src/api2/status.rs index 372b4822..02ba1a78 100644 --- a/src/api2/status.rs +++ b/src/api2/status.rs @@ -16,9 +16,9 @@ use crate::api2::types::{ DATASTORE_SCHEMA, RRDMode, RRDTimeFrameResolution, + Authid, TaskListItem, TaskStateType, - Userid, }; use crate::server; @@ -87,13 +87,13 @@ fn datastore_status( let (config, _digest) = datastore::config()?; - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let user_info = CachedUserInfo::new()?; let mut list = Vec::new(); for (store, (_, _)) in &config.sections { - let user_privs = user_info.lookup_privs(&userid, &["datastore", &store]); + let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]); let allowed = (user_privs & (PRIV_DATASTORE_AUDIT| PRIV_DATASTORE_BACKUP)) != 0; if !allowed { continue; @@ -221,9 +221,9 @@ pub fn list_tasks( rpcenv: &mut dyn RpcEnvironment, ) -> Result<Vec<TaskListItem>, Error> { - let userid: Userid = rpcenv.get_user().unwrap().parse()?; + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let user_info = CachedUserInfo::new()?; - let user_privs = user_info.lookup_privs(&userid, &["system", "tasks"]); + let user_privs = user_info.lookup_privs(&auth_id, &["system", "tasks"]); let list_all = (user_privs & PRIV_SYS_AUDIT) != 0; let since = since.unwrap_or_else(|| 0); @@ -238,7 +238,7 @@ pub fn list_tasks( .filter_map(|info| { match info { Ok(info) => { - if list_all || info.upid.userid == userid { + if list_all || info.upid.auth_id == auth_id { if let Some(filter) = &typefilter { if !info.upid.worker_type.contains(filter) { return None; diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs index 3f723e32..b1287583 100644 --- a/src/api2/types/mod.rs +++ b/src/api2/types/mod.rs @@ -376,7 +376,7 @@ pub const BLOCKDEVICE_NAME_SCHEMA: Schema = StringSchema::new("Block device name }, }, owner: { - type: Userid, + type: Authid, optional: true, }, }, @@ -394,7 +394,7 @@ pub struct GroupListItem { pub files: Vec<String>, /// The owner of group #[serde(skip_serializing_if="Option::is_none")] - pub owner: Option<Userid>, + pub owner: Option<Authid>, } #[api()] @@ -452,7 +452,7 @@ pub struct SnapshotVerifyState { }, }, owner: { - type: Userid, + type: Authid, optional: true, }, }, @@ -477,7 +477,7 @@ pub struct SnapshotListItem { pub size: Option<u64>, /// The owner of the snapshots group #[serde(skip_serializing_if="Option::is_none")] - pub owner: Option<Userid>, + pub owner: Option<Authid>, } #[api( @@ -627,7 +627,7 @@ pub struct StorageStatus { #[api( properties: { upid: { schema: UPID_SCHEMA }, - user: { type: Userid }, + userid: { type: Authid }, }, )] #[derive(Serialize, Deserialize)] @@ -646,8 +646,8 @@ pub struct TaskListItem { pub worker_type: String, /// Worker ID (arbitrary ASCII string) pub worker_id: Option<String>, - /// The user who started the task - pub user: Userid, + /// The authenticated entity who started the task + pub userid: Authid, /// The task end time (Epoch) #[serde(skip_serializing_if="Option::is_none")] pub endtime: Option<i64>, @@ -670,7 +670,7 @@ impl From<crate::server::TaskListInfo> for TaskListItem { starttime: info.upid.starttime, worker_type: info.upid.worker_type, worker_id: info.upid.worker_id, - user: info.upid.userid, + userid: info.upid.auth_id, endtime, status, } diff --git a/src/backup/datastore.rs b/src/backup/datastore.rs index 485a10ff..b0b04c00 100644 --- a/src/backup/datastore.rs +++ b/src/backup/datastore.rs @@ -23,7 +23,7 @@ use crate::task::TaskState; use crate::tools; use crate::tools::format::HumanByte; use crate::tools::fs::{lock_dir_noblock, DirLockGuard}; -use crate::api2::types::{GarbageCollectionStatus, Userid}; +use crate::api2::types::{Authid, GarbageCollectionStatus}; use crate::server::UPID; lazy_static! { @@ -276,8 +276,8 @@ impl DataStore { /// Returns the backup owner. /// - /// The backup owner is the user who first created the backup group. - pub fn get_owner(&self, backup_group: &BackupGroup) -> Result<Userid, Error> { + /// The backup owner is the entity who first created the backup group. + pub fn get_owner(&self, backup_group: &BackupGroup) -> Result<Authid, Error> { let mut full_path = self.base_path(); full_path.push(backup_group.group_path()); full_path.push("owner"); @@ -289,7 +289,7 @@ impl DataStore { pub fn set_owner( &self, backup_group: &BackupGroup, - userid: &Userid, + auth_id: &Authid, force: bool, ) -> Result<(), Error> { let mut path = self.base_path(); @@ -309,7 +309,7 @@ impl DataStore { let mut file = open_options.open(&path) .map_err(|err| format_err!("unable to create owner file {:?} - {}", path, err))?; - writeln!(file, "{}", userid) + writeln!(file, "{}", auth_id) .map_err(|err| format_err!("unable to write owner file {:?} - {}", path, err))?; Ok(()) @@ -324,8 +324,8 @@ impl DataStore { pub fn create_locked_backup_group( &self, backup_group: &BackupGroup, - userid: &Userid, - ) -> Result<(Userid, DirLockGuard), Error> { + auth_id: &Authid, + ) -> Result<(Authid, DirLockGuard), Error> { // create intermediate path first: let base_path = self.base_path(); @@ -339,7 +339,7 @@ impl DataStore { match std::fs::create_dir(&full_path) { Ok(_) => { let guard = lock_dir_noblock(&full_path, "backup group", "another backup is already running")?; - self.set_owner(backup_group, userid, false)?; + self.set_owner(backup_group, auth_id, false)?; let owner = self.get_owner(backup_group)?; // just to be sure Ok((owner, guard)) } diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs index 4bb05bd1..8c68ffd2 100644 --- a/src/bin/proxmox-backup-client.rs +++ b/src/bin/proxmox-backup-client.rs @@ -36,7 +36,7 @@ use proxmox_backup::api2::types::*; use proxmox_backup::api2::version; use proxmox_backup::client::*; use proxmox_backup::pxar::catalog::*; -use proxmox_backup::config::user::complete_user_name; +use proxmox_backup::config::user::complete_userid; use proxmox_backup::backup::{ archive_type, decrypt_key, @@ -425,7 +425,7 @@ async fn list_backup_groups(param: Value) -> Result<Value, Error> { description: "Backup group.", }, "new-owner": { - type: Userid, + type: Authid, }, } } @@ -2010,7 +2010,7 @@ fn main() { let change_owner_cmd_def = CliCommand::new(&API_METHOD_CHANGE_BACKUP_OWNER) .arg_param(&["group", "new-owner"]) .completion_cb("group", complete_backup_group) - .completion_cb("new-owner", complete_user_name) + .completion_cb("new-owner", complete_userid) .completion_cb("repository", complete_repository); let cmd_def = CliCommandMap::new() diff --git a/src/bin/proxmox-backup-manager.rs b/src/bin/proxmox-backup-manager.rs index f0dbea93..f13e55ee 100644 --- a/src/bin/proxmox-backup-manager.rs +++ b/src/bin/proxmox-backup-manager.rs @@ -388,7 +388,7 @@ fn main() { let mut rpcenv = CliEnvironment::new(); - rpcenv.set_user(Some(String::from("root@pam"))); + rpcenv.set_auth_id(Some(String::from("root@pam"))); proxmox_backup::tools::runtime::main(run_async_cli_command(cmd_def, rpcenv)); } diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs index f2612e1f..b8c22af3 100644 --- a/src/bin/proxmox-backup-proxy.rs +++ b/src/bin/proxmox-backup-proxy.rs @@ -30,7 +30,7 @@ use proxmox_backup::{ }; -use proxmox_backup::api2::types::Userid; +use proxmox_backup::api2::types::{Authid, Userid}; use proxmox_backup::configdir; use proxmox_backup::buildcfg; use proxmox_backup::server; @@ -334,7 +334,7 @@ async fn schedule_datastore_garbage_collection() { if let Err(err) = WorkerTask::new_thread( worker_type, Some(store.clone()), - Userid::backup_userid().clone(), + Authid::backup_auth_id().clone(), false, move |worker| { job.start(&worker.upid().to_string())?; @@ -463,7 +463,7 @@ async fn schedule_datastore_prune() { if let Err(err) = WorkerTask::new_thread( worker_type, Some(store.clone()), - Userid::backup_userid().clone(), + Authid::backup_auth_id().clone(), false, move |worker| { @@ -579,9 +579,9 @@ async fn schedule_datastore_sync_jobs() { Err(_) => continue, // could not get lock }; - let userid = Userid::backup_userid().clone(); + let auth_id = Authid::backup_auth_id(); - if let Err(err) = do_sync_job(job, job_config, &userid, Some(event_str)) { + if let Err(err) = do_sync_job(job, job_config, &auth_id, Some(event_str)) { eprintln!("unable to start datastore sync job {} - {}", &job_id, err); } } @@ -642,8 +642,8 @@ async fn schedule_datastore_verify_jobs() { Ok(job) => job, Err(_) => continue, // could not get lock }; - let userid = Userid::backup_userid().clone(); - if let Err(err) = do_verification_job(job, job_config, &userid, Some(event_str)) { + let auth_id = Authid::backup_auth_id(); + if let Err(err) = do_verification_job(job, job_config, &auth_id, Some(event_str)) { eprintln!("unable to start datastore verification job {} - {}", &job_id, err); } } @@ -704,7 +704,7 @@ async fn schedule_task_log_rotate() { if let Err(err) = WorkerTask::new_thread( worker_type, Some(job_id.to_string()), - Userid::backup_userid().clone(), + Authid::backup_auth_id().clone(), false, move |worker| { job.start(&worker.upid().to_string())?; diff --git a/src/bin/proxmox_backup_manager/acl.rs b/src/bin/proxmox_backup_manager/acl.rs index bc2e8f7a..3fbb3bcb 100644 --- a/src/bin/proxmox_backup_manager/acl.rs +++ b/src/bin/proxmox_backup_manager/acl.rs @@ -60,7 +60,7 @@ pub fn acl_commands() -> CommandLineInterface { "update", CliCommand::new(&api2::access::acl::API_METHOD_UPDATE_ACL) .arg_param(&["path", "role"]) - .completion_cb("userid", config::user::complete_user_name) + .completion_cb("userid", config::user::complete_userid) .completion_cb("path", config::datastore::complete_acl_path) ); diff --git a/src/bin/proxmox_backup_manager/user.rs b/src/bin/proxmox_backup_manager/user.rs index 80dbcb1b..af05e9b5 100644 --- a/src/bin/proxmox_backup_manager/user.rs +++ b/src/bin/proxmox_backup_manager/user.rs @@ -62,13 +62,13 @@ pub fn user_commands() -> CommandLineInterface { "update", CliCommand::new(&api2::access::user::API_METHOD_UPDATE_USER) .arg_param(&["userid"]) - .completion_cb("userid", config::user::complete_user_name) + .completion_cb("userid", config::user::complete_userid) ) .insert( "remove", CliCommand::new(&api2::access::user::API_METHOD_DELETE_USER) .arg_param(&["userid"]) - .completion_cb("userid", config::user::complete_user_name) + .completion_cb("userid", config::user::complete_userid) ); cmd_def.into() diff --git a/src/client/pull.rs b/src/client/pull.rs index a1d5ea48..7cb8d8e1 100644 --- a/src/client/pull.rs +++ b/src/client/pull.rs @@ -491,7 +491,7 @@ pub async fn pull_store( src_repo: &BackupRepository, tgt_store: Arc<DataStore>, delete: bool, - userid: Userid, + auth_id: Authid, ) -> Result<(), Error> { // explicit create shared lock to prevent GC on newly created chunks @@ -524,11 +524,11 @@ pub async fn pull_store( for (groups_done, item) in list.into_iter().enumerate() { let group = BackupGroup::new(&item.backup_type, &item.backup_id); - let (owner, _lock_guard) = tgt_store.create_locked_backup_group(&group, &userid)?; + let (owner, _lock_guard) = tgt_store.create_locked_backup_group(&group, &auth_id)?; // permission check - if userid != owner { // only the owner is allowed to create additional snapshots + if auth_id != owner { // only the owner is allowed to create additional snapshots worker.log(format!("sync group {}/{} failed - owner check failed ({} != {})", - item.backup_type, item.backup_id, userid, owner)); + item.backup_type, item.backup_id, auth_id, owner)); errors = true; // do not stop here, instead continue } else if let Err(err) = pull_group( diff --git a/src/config/acl.rs b/src/config/acl.rs index 39f9d030..12b5a851 100644 --- a/src/config/acl.rs +++ b/src/config/acl.rs @@ -15,7 +15,7 @@ use proxmox::tools::{fs::replace_file, fs::CreateOptions}; use proxmox::constnamedbitmap; use proxmox::api::{api, schema::*}; -use crate::api2::types::Userid; +use crate::api2::types::{Authid,Userid}; // define Privilege bitfield @@ -231,7 +231,7 @@ pub struct AclTree { } pub struct AclTreeNode { - pub users: HashMap<Userid, HashMap<String, bool>>, + pub users: HashMap<Authid, HashMap<String, bool>>, pub groups: HashMap<String, HashMap<String, bool>>, pub children: BTreeMap<String, AclTreeNode>, } @@ -246,21 +246,21 @@ impl AclTreeNode { } } - pub fn extract_roles(&self, user: &Userid, all: bool) -> HashSet<String> { - let user_roles = self.extract_user_roles(user, all); + pub fn extract_roles(&self, auth_id: &Authid, all: bool) -> HashSet<String> { + let user_roles = self.extract_user_roles(auth_id, all); if !user_roles.is_empty() { // user privs always override group privs return user_roles }; - self.extract_group_roles(user, all) + self.extract_group_roles(auth_id.user(), all) } - pub fn extract_user_roles(&self, user: &Userid, all: bool) -> HashSet<String> { + pub fn extract_user_roles(&self, auth_id: &Authid, all: bool) -> HashSet<String> { let mut set = HashSet::new(); - let roles = match self.users.get(user) { + let roles = match self.users.get(auth_id) { Some(m) => m, None => return set, }; @@ -312,8 +312,8 @@ impl AclTreeNode { roles.remove(role); } - pub fn delete_user_role(&mut self, userid: &Userid, role: &str) { - let roles = match self.users.get_mut(userid) { + pub fn delete_user_role(&mut self, auth_id: &Authid, role: &str) { + let roles = match self.users.get_mut(auth_id) { Some(r) => r, None => return, }; @@ -331,8 +331,8 @@ impl AclTreeNode { } } - pub fn insert_user_role(&mut self, user: Userid, role: String, propagate: bool) { - let map = self.users.entry(user).or_insert_with(|| HashMap::new()); + pub fn insert_user_role(&mut self, auth_id: Authid, role: String, propagate: bool) { + let map = self.users.entry(auth_id).or_insert_with(|| HashMap::new()); if role == ROLE_NAME_NO_ACCESS { map.clear(); map.insert(role, propagate); @@ -383,13 +383,13 @@ impl AclTree { node.delete_group_role(group, role); } - pub fn delete_user_role(&mut self, path: &str, userid: &Userid, role: &str) { + pub fn delete_user_role(&mut self, path: &str, auth_id: &Authid, role: &str) { let path = split_acl_path(path); let node = match self.get_node(&path) { Some(n) => n, None => return, }; - node.delete_user_role(userid, role); + node.delete_user_role(auth_id, role); } pub fn insert_group_role(&mut self, path: &str, group: &str, role: &str, propagate: bool) { @@ -398,10 +398,10 @@ impl AclTree { node.insert_group_role(group.to_string(), role.to_string(), propagate); } - pub fn insert_user_role(&mut self, path: &str, user: &Userid, role: &str, propagate: bool) { + pub fn insert_user_role(&mut self, path: &str, auth_id: &Authid, role: &str, propagate: bool) { let path = split_acl_path(path); let node = self.get_or_insert_node(&path); - node.insert_user_role(user.to_owned(), role.to_string(), propagate); + node.insert_user_role(auth_id.to_owned(), role.to_string(), propagate); } fn write_node_config( @@ -413,18 +413,18 @@ impl AclTree { let mut role_ug_map0 = HashMap::new(); let mut role_ug_map1 = HashMap::new(); - for (user, roles) in &node.users { + for (auth_id, roles) in &node.users { // no need to save, because root is always 'Administrator' - if user == "root@pam" { continue; } + if !auth_id.is_token() && auth_id.user() == "root@pam" { continue; } for (role, propagate) in roles { let role = role.as_str(); - let user = user.to_string(); + let auth_id = auth_id.to_string(); if *propagate { role_ug_map1.entry(role).or_insert_with(|| BTreeSet::new()) - .insert(user); + .insert(auth_id); } else { role_ug_map0.entry(role).or_insert_with(|| BTreeSet::new()) - .insert(user); + .insert(auth_id); } } } @@ -576,10 +576,10 @@ impl AclTree { Ok(tree) } - pub fn roles(&self, userid: &Userid, path: &[&str]) -> HashSet<String> { + pub fn roles(&self, auth_id: &Authid, path: &[&str]) -> HashSet<String> { let mut node = &self.root; - let mut role_set = node.extract_roles(userid, path.is_empty()); + let mut role_set = node.extract_roles(auth_id, path.is_empty()); for (pos, comp) in path.iter().enumerate() { let last_comp = (pos + 1) == path.len(); @@ -587,7 +587,7 @@ impl AclTree { Some(n) => n, None => return role_set, // path not found }; - let new_set = node.extract_roles(userid, last_comp); + let new_set = node.extract_roles(auth_id, last_comp); if !new_set.is_empty() { // overwrite previous settings role_set = new_set; @@ -675,22 +675,22 @@ mod test { use anyhow::{Error}; use super::AclTree; - use crate::api2::types::Userid; + use crate::api2::types::Authid; fn check_roles( tree: &AclTree, - user: &Userid, + auth_id: &Authid, path: &str, expected_roles: &str, ) { let path_vec = super::split_acl_path(path); - let mut roles = tree.roles(user, &path_vec) + let mut roles = tree.roles(auth_id, &path_vec) .iter().map(|v| v.clone()).collect::<Vec<String>>(); roles.sort(); let roles = roles.join(","); - assert_eq!(roles, expected_roles, "\nat check_roles for '{}' on '{}'", user, path); + assert_eq!(roles, expected_roles, "\nat check_roles for '{}' on '{}'", auth_id, path); } #[test] @@ -721,13 +721,13 @@ acl:1:/storage:user1@pbs:Admin acl:1:/storage/store1:user1@pbs:DatastoreBackup acl:1:/storage/store2:user2@pbs:DatastoreBackup "###)?; - let user1: Userid = "user1@pbs".parse()?; + let user1: Authid = "user1@pbs".parse()?; check_roles(&tree, &user1, "/", ""); check_roles(&tree, &user1, "/storage", "Admin"); check_roles(&tree, &user1, "/storage/store1", "DatastoreBackup"); check_roles(&tree, &user1, "/storage/store2", "Admin"); - let user2: Userid = "user2@pbs".parse()?; + let user2: Authid = "user2@pbs".parse()?; check_roles(&tree, &user2, "/", ""); check_roles(&tree, &user2, "/storage", ""); check_roles(&tree, &user2, "/storage/store1", ""); @@ -744,7 +744,7 @@ acl:1:/:user1@pbs:Admin acl:1:/storage:user1@pbs:NoAccess acl:1:/storage/store1:user1@pbs:DatastoreBackup "###)?; - let user1: Userid = "user1@pbs".parse()?; + let user1: Authid = "user1@pbs".parse()?; check_roles(&tree, &user1, "/", "Admin"); check_roles(&tree, &user1, "/storage", "NoAccess"); check_roles(&tree, &user1, "/storage/store1", "DatastoreBackup"); @@ -770,7 +770,7 @@ acl:1:/storage/store1:user1@pbs:DatastoreBackup let mut tree = AclTree::new(); - let user1: Userid = "user1@pbs".parse()?; + let user1: Authid = "user1@pbs".parse()?; tree.insert_user_role("/", &user1, "Admin", true); tree.insert_user_role("/", &user1, "Audit", true); @@ -794,7 +794,7 @@ acl:1:/storage/store1:user1@pbs:DatastoreBackup let mut tree = AclTree::new(); - let user1: Userid = "user1@pbs".parse()?; + let user1: Authid = "user1@pbs".parse()?; tree.insert_user_role("/storage", &user1, "NoAccess", true); diff --git a/src/config/cached_user_info.rs b/src/config/cached_user_info.rs index cf9c534d..57d53aac 100644 --- a/src/config/cached_user_info.rs +++ b/src/config/cached_user_info.rs @@ -9,10 +9,10 @@ use lazy_static::lazy_static; use proxmox::api::UserInformation; use super::acl::{AclTree, ROLE_NAMES, ROLE_ADMIN}; -use super::user::User; -use crate::api2::types::Userid; +use super::user::{ApiToken, User}; +use crate::api2::types::{Authid, Userid}; -/// Cache User/Group/Acl configuration data for fast permission tests +/// Cache User/Group/Token/Acl configuration data for fast permission tests pub struct CachedUserInfo { user_cfg: Arc<SectionConfigData>, acl_tree: Arc<AclTree>, @@ -57,8 +57,10 @@ impl CachedUserInfo { Ok(config) } - /// Test if a user account is enabled and not expired - pub fn is_active_user(&self, userid: &Userid) -> bool { + /// Test if a authentication id is enabled and not expired + pub fn is_active_auth_id(&self, auth_id: &Authid) -> bool { + let userid = auth_id.user(); + if let Ok(info) = self.user_cfg.lookup::<User>("user", userid.as_str()) { if !info.enable.unwrap_or(true) { return false; @@ -68,24 +70,41 @@ impl CachedUserInfo { return false; } } - return true; } else { return false; } + + if auth_id.is_token() { + if let Ok(info) = self.user_cfg.lookup::<ApiToken>("token", &auth_id.to_string()) { + if !info.enable.unwrap_or(true) { + return false; + } + if let Some(expire) = info.expire { + if expire > 0 && expire <= now() { + return false; + } + } + return true; + } else { + return false; + } + } + + return true; } pub fn check_privs( &self, - userid: &Userid, + auth_id: &Authid, path: &[&str], required_privs: u64, partial: bool, ) -> Result<(), Error> { - let user_privs = self.lookup_privs(&userid, path); + let privs = self.lookup_privs(&auth_id, path); let allowed = if partial { - (user_privs & required_privs) != 0 + (privs & required_privs) != 0 } else { - (user_privs & required_privs) == required_privs + (privs & required_privs) == required_privs }; if !allowed { // printing the path doesn't leaks any information as long as we @@ -95,27 +114,33 @@ impl CachedUserInfo { Ok(()) } - pub fn is_superuser(&self, userid: &Userid) -> bool { - userid == "root@pam" + pub fn is_superuser(&self, auth_id: &Authid) -> bool { + !auth_id.is_token() && auth_id.user() == "root@pam" } pub fn is_group_member(&self, _userid: &Userid, _group: &str) -> bool { false } - pub fn lookup_privs(&self, userid: &Userid, path: &[&str]) -> u64 { - - if self.is_superuser(userid) { + pub fn lookup_privs(&self, auth_id: &Authid, path: &[&str]) -> u64 { + if self.is_superuser(auth_id) { return ROLE_ADMIN; } - let roles = self.acl_tree.roles(userid, path); + let roles = self.acl_tree.roles(auth_id, path); let mut privs: u64 = 0; for role in roles { if let Some((role_privs, _)) = ROLE_NAMES.get(role.as_str()) { privs |= role_privs; } } + + if auth_id.is_token() { + // limit privs to that of owning user + let user_auth_id = Authid::from(auth_id.user().clone()); + privs &= self.lookup_privs(&user_auth_id, path); + } + privs } } @@ -129,9 +154,9 @@ impl UserInformation for CachedUserInfo { false } - fn lookup_privs(&self, userid: &str, path: &[&str]) -> u64 { - match userid.parse::<Userid>() { - Ok(userid) => Self::lookup_privs(self, &userid, path), + fn lookup_privs(&self, auth_id: &str, path: &[&str]) -> u64 { + match auth_id.parse::<Authid>() { + Ok(auth_id) => Self::lookup_privs(self, &auth_id, path), Err(_) => 0, } } diff --git a/src/config/remote.rs b/src/config/remote.rs index 9e597342..7ad653ac 100644 --- a/src/config/remote.rs +++ b/src/config/remote.rs @@ -45,7 +45,7 @@ pub const REMOTE_PASSWORD_SCHEMA: Schema = StringSchema::new("Password or auth t type: u16, }, userid: { - type: Userid, + type: Authid, }, password: { schema: REMOTE_PASSWORD_SCHEMA, diff --git a/src/config/user.rs b/src/config/user.rs index b72fa40b..78571daa 100644 --- a/src/config/user.rs +++ b/src/config/user.rs @@ -52,6 +52,36 @@ pub const EMAIL_SCHEMA: Schema = StringSchema::new("E-Mail Address.") .max_length(64) .schema(); +#[api( + properties: { + tokenid: { + schema: PROXMOX_TOKEN_ID_SCHEMA, + }, + comment: { + optional: true, + schema: SINGLE_LINE_COMMENT_SCHEMA, + }, + enable: { + optional: true, + schema: ENABLE_USER_SCHEMA, + }, + expire: { + optional: true, + schema: EXPIRE_USER_SCHEMA, + }, + } +)] +#[derive(Serialize,Deserialize)] +/// ApiToken properties. +pub struct ApiToken { + pub tokenid: Authid, + #[serde(skip_serializing_if="Option::is_none")] + pub comment: Option<String>, + #[serde(skip_serializing_if="Option::is_none")] + pub enable: Option<bool>, + #[serde(skip_serializing_if="Option::is_none")] + pub expire: Option<i64>, +} #[api( properties: { @@ -103,15 +133,21 @@ pub struct User { } fn init() -> SectionConfig { - let obj_schema = match User::API_SCHEMA { - Schema::Object(ref obj_schema) => obj_schema, + let mut config = SectionConfig::new(&Authid::API_SCHEMA); + + let user_schema = match User::API_SCHEMA { + Schema::Object(ref user_schema) => user_schema, _ => unreachable!(), }; + let user_plugin = SectionConfigPlugin::new("user".to_string(), Some("userid".to_string()), user_schema); + config.register_plugin(user_plugin); - let plugin = SectionConfigPlugin::new("user".to_string(), Some("userid".to_string()), obj_schema); - let mut config = SectionConfig::new(&Userid::API_SCHEMA); - - config.register_plugin(plugin); + let token_schema = match ApiToken::API_SCHEMA { + Schema::Object(ref token_schema) => token_schema, + _ => unreachable!(), + }; + let token_plugin = SectionConfigPlugin::new("token".to_string(), Some("tokenid".to_string()), token_schema); + config.register_plugin(token_plugin); config } @@ -206,9 +242,26 @@ pub fn save_config(config: &SectionConfigData) -> Result<(), Error> { } // shell completion helper -pub fn complete_user_name(_arg: &str, _param: &HashMap<String, String>) -> Vec<String> { +pub fn complete_userid(_arg: &str, _param: &HashMap<String, String>) -> Vec<String> { match config() { - Ok((data, _digest)) => data.sections.iter().map(|(id, _)| id.to_string()).collect(), + Ok((data, _digest)) => { + data.sections.iter() + .filter_map(|(id, (section_type, _))| { + if section_type == "user" { + Some(id.to_string()) + } else { + None + } + }).collect() + }, Err(_) => return vec![], } } + +// shell completion helper +pub fn complete_authid(_arg: &str, _param: &HashMap<String, String>) -> Vec<String> { + match config() { + Ok((data, _digest)) => data.sections.iter().map(|(id, _)| id.to_string()).collect(), + Err(_) => vec![], + } +} diff --git a/src/server/environment.rs b/src/server/environment.rs index 5fbff307..2577c379 100644 --- a/src/server/environment.rs +++ b/src/server/environment.rs @@ -6,7 +6,7 @@ use proxmox::api::{RpcEnvironment, RpcEnvironmentType}; pub struct RestEnvironment { env_type: RpcEnvironmentType, result_attributes: Value, - user: Option<String>, + auth_id: Option<String>, client_ip: Option<std::net::SocketAddr>, } @@ -14,7 +14,7 @@ impl RestEnvironment { pub fn new(env_type: RpcEnvironmentType) -> Self { Self { result_attributes: json!({}), - user: None, + auth_id: None, client_ip: None, env_type, } @@ -35,12 +35,12 @@ impl RpcEnvironment for RestEnvironment { self.env_type } - fn set_user(&mut self, user: Option<String>) { - self.user = user; + fn set_auth_id(&mut self, auth_id: Option<String>) { + self.auth_id = auth_id; } - fn get_user(&self) -> Option<String> { - self.user.clone() + fn get_auth_id(&self) -> Option<String> { + self.auth_id.clone() } fn set_client_ip(&mut self, client_ip: Option<std::net::SocketAddr>) { diff --git a/src/server/rest.rs b/src/server/rest.rs index c650a3aa..2b835c4a 100644 --- a/src/server/rest.rs +++ b/src/server/rest.rs @@ -42,7 +42,7 @@ use super::formatter::*; use super::ApiConfig; use crate::auth_helpers::*; -use crate::api2::types::Userid; +use crate::api2::types::{Authid, Userid}; use crate::tools; use crate::tools::FileLogger; use crate::tools::ticket::Ticket; @@ -138,9 +138,9 @@ fn log_response( log::error!("{} {}: {} {}: [client {}] {}", method.as_str(), path, status.as_str(), reason, peer, message); } if let Some(logfile) = logfile { - let user = match resp.extensions().get::<Userid>() { - Some(userid) => userid.as_str(), - None => "-", + let auth_id = match resp.extensions().get::<Authid>() { + Some(auth_id) => auth_id.to_string(), + None => "-".to_string(), }; let now = proxmox::tools::time::epoch_i64(); // time format which apache/nginx use (by default), copied from pve-http-server @@ -153,7 +153,7 @@ fn log_response( .log(format!( "{} - {} [{}] \"{} {}\" {} {} {}", peer.ip(), - user, + auth_id, datetime, method.as_str(), path, @@ -441,7 +441,7 @@ fn get_index( .unwrap(); if let Some(userid) = userid { - resp.extensions_mut().insert(userid); + resp.extensions_mut().insert(Authid::from((userid, None))); } resp @@ -555,14 +555,15 @@ fn check_auth( ticket: &Option<String>, csrf_token: &Option<String>, user_info: &CachedUserInfo, -) -> Result<Userid, Error> { +) -> Result<Authid, Error> { let ticket_lifetime = tools::ticket::TICKET_LIFETIME; let ticket = ticket.as_ref().map(String::as_str); let userid: Userid = Ticket::parse(&ticket.ok_or_else(|| format_err!("missing ticket"))?)? .verify_with_time_frame(public_auth_key(), "PBS", None, -300..ticket_lifetime)?; - if !user_info.is_active_user(&userid) { + let auth_id = Authid::from(userid.clone()); + if !user_info.is_active_auth_id(&auth_id) { bail!("user account disabled or expired."); } @@ -574,7 +575,7 @@ fn check_auth( } } - Ok(userid) + Ok(Authid::from(userid)) } async fn handle_request( @@ -632,7 +633,7 @@ async fn handle_request( if auth_required { let (ticket, csrf_token, _) = extract_auth_data(&parts.headers); match check_auth(&method, &ticket, &csrf_token, &user_info) { - Ok(userid) => rpcenv.set_user(Some(userid.to_string())), + Ok(authid) => rpcenv.set_auth_id(Some(authid.to_string())), Err(err) => { // always delay unauthorized calls by 3 seconds (from start of request) let err = http_err!(UNAUTHORIZED, "authentication failed - {}", err); @@ -648,8 +649,8 @@ async fn handle_request( return Ok((formatter.format_error)(err)); } Some(api_method) => { - let user = rpcenv.get_user(); - if !check_api_permission(api_method.access.permission, user.as_deref(), &uri_param, user_info.as_ref()) { + let auth_id = rpcenv.get_auth_id(); + if !check_api_permission(api_method.access.permission, auth_id.as_deref(), &uri_param, user_info.as_ref()) { let err = http_err!(FORBIDDEN, "permission check failed"); tokio::time::delay_until(Instant::from_std(access_forbidden_time)).await; return Ok((formatter.format_error)(err)); @@ -666,9 +667,9 @@ async fn handle_request( Err(err) => (formatter.format_error)(err), }; - if let Some(user) = user { - let userid: Userid = user.parse()?; - response.extensions_mut().insert(userid); + if let Some(auth_id) = auth_id { + let auth_id: Authid = auth_id.parse()?; + response.extensions_mut().insert(auth_id); } return Ok(response); @@ -687,9 +688,10 @@ async fn handle_request( let (ticket, csrf_token, language) = extract_auth_data(&parts.headers); if ticket != None { match check_auth(&method, &ticket, &csrf_token, &user_info) { - Ok(userid) => { - let new_csrf_token = assemble_csrf_prevention_token(csrf_secret(), &userid); - return Ok(get_index(Some(userid), Some(new_csrf_token), language, &api, parts)); + Ok(auth_id) => { + let userid = auth_id.user(); + let new_csrf_token = assemble_csrf_prevention_token(csrf_secret(), userid); + return Ok(get_index(Some(userid.clone()), Some(new_csrf_token), language, &api, parts)); } _ => { tokio::time::delay_until(Instant::from_std(delay_unauth_time)).await; diff --git a/src/server/upid.rs b/src/server/upid.rs index df027b07..3ca7cff2 100644 --- a/src/server/upid.rs +++ b/src/server/upid.rs @@ -6,7 +6,7 @@ use proxmox::api::schema::{ApiStringFormat, Schema, StringSchema}; use proxmox::const_regex; use proxmox::sys::linux::procfs; -use crate::api2::types::Userid; +use crate::api2::types::Authid; /// Unique Process/Task Identifier /// @@ -34,8 +34,8 @@ pub struct UPID { pub worker_type: String, /// Worker ID (arbitrary ASCII string) pub worker_id: Option<String>, - /// The user who started the task - pub userid: Userid, + /// The authenticated entity who started the task + pub auth_id: Authid, /// The node name. pub node: String, } @@ -47,7 +47,7 @@ const_regex! { pub PROXMOX_UPID_REGEX = concat!( r"^UPID:(?P<node>[a-zA-Z0-9]([a-zA-Z0-9\-]*[a-zA-Z0-9])?):(?P<pid>[0-9A-Fa-f]{8}):", r"(?P<pstart>[0-9A-Fa-f]{8,9}):(?P<task_id>[0-9A-Fa-f]{8,16}):(?P<starttime>[0-9A-Fa-f]{8}):", - r"(?P<wtype>[^:\s]+):(?P<wid>[^:\s]*):(?P<userid>[^:\s]+):$" + r"(?P<wtype>[^:\s]+):(?P<wid>[^:\s]*):(?P<authid>[^:\s]+):$" ); } @@ -65,7 +65,7 @@ impl UPID { pub fn new( worker_type: &str, worker_id: Option<String>, - userid: Userid, + auth_id: Authid, ) -> Result<Self, Error> { let pid = unsafe { libc::getpid() }; @@ -87,7 +87,7 @@ impl UPID { task_id, worker_type: worker_type.to_owned(), worker_id, - userid, + auth_id, node: proxmox::tools::nodename().to_owned(), }) } @@ -122,7 +122,7 @@ impl std::str::FromStr for UPID { task_id: usize::from_str_radix(&cap["task_id"], 16).unwrap(), worker_type: cap["wtype"].to_string(), worker_id, - userid: cap["userid"].parse()?, + auth_id: cap["authid"].parse()?, node: cap["node"].to_string(), }) } else { @@ -146,6 +146,6 @@ impl std::fmt::Display for UPID { // more that 8 characters for pstart write!(f, "UPID:{}:{:08X}:{:08X}:{:08X}:{:08X}:{}:{}:{}:", - self.node, self.pid, self.pstart, self.task_id, self.starttime, self.worker_type, wid, self.userid) + self.node, self.pid, self.pstart, self.task_id, self.starttime, self.worker_type, wid, self.auth_id) } } diff --git a/src/server/verify_job.rs b/src/server/verify_job.rs index 064fb2b7..169e1687 100644 --- a/src/server/verify_job.rs +++ b/src/server/verify_job.rs @@ -20,7 +20,7 @@ use crate::{ pub fn do_verification_job( mut job: Job, verification_job: VerificationJobConfig, - userid: &Userid, + auth_id: &Authid, schedule: Option<String>, ) -> Result<String, Error> { let datastore = DataStore::lookup_datastore(&verification_job.store)?; @@ -46,14 +46,14 @@ pub fn do_verification_job( }) } - let email = crate::server::lookup_user_email(userid); + let email = crate::server::lookup_user_email(auth_id.user()); let job_id = job.jobname().to_string(); let worker_type = job.jobtype().to_string(); let upid_str = WorkerTask::new_thread( &worker_type, Some(job.jobname().to_string()), - userid.clone(), + auth_id.clone(), false, move |worker| { job.start(&worker.upid().to_string())?; diff --git a/src/server/worker_task.rs b/src/server/worker_task.rs index 8ef0fde7..d811fdd9 100644 --- a/src/server/worker_task.rs +++ b/src/server/worker_task.rs @@ -21,7 +21,7 @@ use super::UPID; use crate::tools::logrotate::{LogRotate, LogRotateFiles}; use crate::tools::{FileLogger, FileLogOptions}; -use crate::api2::types::Userid; +use crate::api2::types::Authid; macro_rules! PROXMOX_BACKUP_VAR_RUN_DIR_M { () => ("/run/proxmox-backup") } macro_rules! PROXMOX_BACKUP_LOG_DIR_M { () => ("/var/log/proxmox-backup") } @@ -644,10 +644,10 @@ impl Drop for WorkerTask { impl WorkerTask { - pub fn new(worker_type: &str, worker_id: Option<String>, userid: Userid, to_stdout: bool) -> Result<Arc<Self>, Error> { + pub fn new(worker_type: &str, worker_id: Option<String>, auth_id: Authid, to_stdout: bool) -> Result<Arc<Self>, Error> { println!("register worker"); - let upid = UPID::new(worker_type, worker_id, userid)?; + let upid = UPID::new(worker_type, worker_id, auth_id)?; let task_id = upid.task_id; let mut path = std::path::PathBuf::from(PROXMOX_BACKUP_TASK_DIR); @@ -699,14 +699,14 @@ impl WorkerTask { pub fn spawn<F, T>( worker_type: &str, worker_id: Option<String>, - userid: Userid, + auth_id: Authid, to_stdout: bool, f: F, ) -> Result<String, Error> where F: Send + 'static + FnOnce(Arc<WorkerTask>) -> T, T: Send + 'static + Future<Output = Result<(), Error>>, { - let worker = WorkerTask::new(worker_type, worker_id, userid, to_stdout)?; + let worker = WorkerTask::new(worker_type, worker_id, auth_id, to_stdout)?; let upid_str = worker.upid.to_string(); let f = f(worker.clone()); tokio::spawn(async move { @@ -721,7 +721,7 @@ impl WorkerTask { pub fn new_thread<F>( worker_type: &str, worker_id: Option<String>, - userid: Userid, + auth_id: Authid, to_stdout: bool, f: F, ) -> Result<String, Error> @@ -729,7 +729,7 @@ impl WorkerTask { { println!("register worker thread"); - let worker = WorkerTask::new(worker_type, worker_id, userid, to_stdout)?; + let worker = WorkerTask::new(worker_type, worker_id, auth_id, to_stdout)?; let upid_str = worker.upid.to_string(); let _child = std::thread::Builder::new().name(upid_str.clone()).spawn(move || { diff --git a/tests/worker-task-abort.rs b/tests/worker-task-abort.rs index 360d17da..3cb41e32 100644 --- a/tests/worker-task-abort.rs +++ b/tests/worker-task-abort.rs @@ -57,7 +57,7 @@ fn worker_task_abort() -> Result<(), Error> { let res = server::WorkerTask::new_thread( "garbage_collection", None, - proxmox_backup::api2::types::Userid::root_userid().clone(), + proxmox_backup::api2::types::Authid::root_auth_id().clone(), true, move |worker| { println!("WORKER {}", worker); diff --git a/www/config/ACLView.js b/www/config/ACLView.js index f02d8de5..d552b029 100644 --- a/www/config/ACLView.js +++ b/www/config/ACLView.js @@ -53,7 +53,7 @@ Ext.define('PBS.config.ACLView', { 'delete': 1, path: rec.data.path, role: rec.data.roleid, - userid: rec.data.ugid, + auth_id: rec.data.ugid, }, callback: function() { me.reload(); diff --git a/www/window/ACLEdit.js b/www/window/ACLEdit.js index e33f1f36..ffeb9e81 100644 --- a/www/window/ACLEdit.js +++ b/www/window/ACLEdit.js @@ -40,7 +40,7 @@ Ext.define('PBS.window.ACLEdit', { { xtype: 'pbsUserSelector', fieldLabel: gettext('User'), - name: 'userid', + name: 'auth_id', allowBlank: false, }, { -- 2.20.1 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 04/16] REST: extract and handle API tokens 2020-10-28 11:36 [pbs-devel] [PATCH proxmox-backup 00/16] API tokens Fabian Grünbichler ` (4 preceding siblings ...) 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-backup 03/16] replace Userid with Authid Fabian Grünbichler @ 2020-10-28 11:36 ` Fabian Grünbichler 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-backup 05/16] api: add API token endpoints Fabian Grünbichler ` (7 subsequent siblings) 13 siblings, 0 replies; 25+ messages in thread From: Fabian Grünbichler @ 2020-10-28 11:36 UTC (permalink / raw) To: pbs-devel and refactor handling of headers in the REST server while we're at it. Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> --- src/server/rest.rs | 109 +++++++++++++++++++++++++++++++-------------- 1 file changed, 75 insertions(+), 34 deletions(-) diff --git a/src/server/rest.rs b/src/server/rest.rs index 2b835c4a..365e3570 100644 --- a/src/server/rest.rs +++ b/src/server/rest.rs @@ -531,51 +531,89 @@ async fn handle_static_file_download(filename: PathBuf) -> Result<Response<Body } } -fn extract_auth_data(headers: &http::HeaderMap) -> (Option<String>, Option<String>, Option<String>) { +fn extract_lang_header(headers: &http::HeaderMap) -> Option<String> { + if let Some(raw_cookie) = headers.get("COOKIE") { + if let Ok(cookie) = raw_cookie.to_str() { + return tools::extract_cookie(cookie, "PBSLangCookie"); + } + } - let mut ticket = None; - let mut language = None; + None +} + +struct UserAuthData{ + ticket: String, + csrf_token: Option<String>, +} + +enum AuthData { + User(UserAuthData), + ApiToken(String), +} + +fn extract_auth_data(headers: &http::HeaderMap) -> Option<AuthData> { if let Some(raw_cookie) = headers.get("COOKIE") { if let Ok(cookie) = raw_cookie.to_str() { - ticket = tools::extract_cookie(cookie, "PBSAuthCookie"); - language = tools::extract_cookie(cookie, "PBSLangCookie"); + if let Some(ticket) = tools::extract_cookie(cookie, "PBSAuthCookie") { + let csrf_token = match headers.get("CSRFPreventionToken").map(|v| v.to_str()) { + Some(Ok(v)) => Some(v.to_owned()), + _ => None, + }; + return Some(AuthData::User(UserAuthData { + ticket, + csrf_token, + })); + } } } - let csrf_token = match headers.get("CSRFPreventionToken").map(|v| v.to_str()) { - Some(Ok(v)) => Some(v.to_owned()), + match headers.get("AUTHORIZATION").map(|v| v.to_str()) { + Some(Ok(v)) => Some(AuthData::ApiToken(v.to_owned())), _ => None, - }; - - (ticket, csrf_token, language) + } } fn check_auth( method: &hyper::Method, - ticket: &Option<String>, - csrf_token: &Option<String>, + auth_data: &AuthData, user_info: &CachedUserInfo, ) -> Result<Authid, Error> { - let ticket_lifetime = tools::ticket::TICKET_LIFETIME; + match auth_data { + AuthData::User(user_auth_data) => { + let ticket = user_auth_data.ticket.clone(); + let ticket_lifetime = tools::ticket::TICKET_LIFETIME; - let ticket = ticket.as_ref().map(String::as_str); - let userid: Userid = Ticket::parse(&ticket.ok_or_else(|| format_err!("missing ticket"))?)? - .verify_with_time_frame(public_auth_key(), "PBS", None, -300..ticket_lifetime)?; + let userid: Userid = Ticket::parse(&ticket)? + .verify_with_time_frame(public_auth_key(), "PBS", None, -300..ticket_lifetime)?; - let auth_id = Authid::from(userid.clone()); - if !user_info.is_active_auth_id(&auth_id) { - bail!("user account disabled or expired."); - } + let auth_id = Authid::from(userid.clone()); + if !user_info.is_active_auth_id(&auth_id) { + bail!("user account disabled or expired."); + } - if method != hyper::Method::GET { - if let Some(csrf_token) = csrf_token { - verify_csrf_prevention_token(csrf_secret(), &userid, &csrf_token, -300, ticket_lifetime)?; - } else { - bail!("missing CSRF prevention token"); + if method != hyper::Method::GET { + if let Some(csrf_token) = &user_auth_data.csrf_token { + verify_csrf_prevention_token(csrf_secret(), &userid, &csrf_token, -300, ticket_lifetime)?; + } else { + bail!("missing CSRF prevention token"); + } + } + + Ok(auth_id) + }, + AuthData::ApiToken(api_token) => { + let mut parts = api_token.splitn(2, ':'); + let tokenid = parts.next() + .ok_or_else(|| format_err!("failed to split API token header"))?; + let tokenid: Authid = tokenid.parse()?; + + let tokensecret = parts.next() + .ok_or_else(|| format_err!("failed to split API token header"))?; + crate::config::token_shadow::verify_secret(&tokenid, &tokensecret)?; + + Ok(tokenid) } } - - Ok(Authid::from(userid)) } async fn handle_request( @@ -631,8 +669,11 @@ async fn handle_request( } if auth_required { - let (ticket, csrf_token, _) = extract_auth_data(&parts.headers); - match check_auth(&method, &ticket, &csrf_token, &user_info) { + let auth_result = match extract_auth_data(&parts.headers) { + Some(auth_data) => check_auth(&method, &auth_data, &user_info), + None => Err(format_err!("no authentication credentials provided.")), + }; + match auth_result { Ok(authid) => rpcenv.set_auth_id(Some(authid.to_string())), Err(err) => { // always delay unauthorized calls by 3 seconds (from start of request) @@ -685,14 +726,14 @@ async fn handle_request( } if comp_len == 0 { - let (ticket, csrf_token, language) = extract_auth_data(&parts.headers); - if ticket != None { - match check_auth(&method, &ticket, &csrf_token, &user_info) { - Ok(auth_id) => { + let language = extract_lang_header(&parts.headers); + if let Some(auth_data) = extract_auth_data(&parts.headers) { + match check_auth(&method, &auth_data, &user_info) { + Ok(auth_id) if !auth_id.is_token() => { let userid = auth_id.user(); let new_csrf_token = assemble_csrf_prevention_token(csrf_secret(), userid); return Ok(get_index(Some(userid.clone()), Some(new_csrf_token), language, &api, parts)); - } + }, _ => { tokio::time::delay_until(Instant::from_std(delay_unauth_time)).await; return Ok(get_index(None, None, language, &api, parts)); -- 2.20.1 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 05/16] api: add API token endpoints 2020-10-28 11:36 [pbs-devel] [PATCH proxmox-backup 00/16] API tokens Fabian Grünbichler ` (5 preceding siblings ...) 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-backup 04/16] REST: extract and handle API tokens Fabian Grünbichler @ 2020-10-28 11:36 ` Fabian Grünbichler 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-backup 06/16] api: allow listing users + tokens Fabian Grünbichler ` (6 subsequent siblings) 13 siblings, 0 replies; 25+ messages in thread From: Fabian Grünbichler @ 2020-10-28 11:36 UTC (permalink / raw) To: pbs-devel beneath the user endpoint. Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> --- src/api2/access/user.rs | 344 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 340 insertions(+), 4 deletions(-) diff --git a/src/api2/access/user.rs b/src/api2/access/user.rs index 47f8e1d1..597ffeaf 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 serde_json::{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; @@ -307,12 +310,345 @@ pub fn delete_user(userid: Userid, digest: Option<String>) -> Result<(), Error> Ok(()) } -const ITEM_ROUTER: Router = Router::new() +#[api( + input: { + properties: { + userid: { + type: Userid, + }, + 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<user::ApiToken, Error> { + + let (config, digest) = user::config()?; + + let tokenname = Tokenname::try_from(tokenname)?; + + let tokenid = Authid::from((userid, Some(tokenname))); + + rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into(); + config.lookup("token", &tokenid.to_string()) +} + +#[api( + protected: true, + input: { + properties: { + userid: { + type: Userid, + }, + 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: "API token identifier + generated secret.", + properties: { + value: { + type: String, + description: "The API token secret", + }, + tokenid: { + type: String, + description: "The API token identifier", + }, + }, + }, +)] +/// Generate a new API token with given metadata +pub fn generate_token( + userid: Userid, + tokenname: String, + comment: Option<String>, + enable: Option<bool>, + expire: Option<i64>, + digest: Option<String>, +) -> Result<Value, 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 = Authid::from((userid.clone(), Some(tokenname.clone()))); + let tokenid_string = tokenid.to_string(); + + if let Some(_) = config.sections.get(&tokenid_string) { + 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_string, "token", &token)?; + + user::save_config(&config)?; + + Ok(json!({ + "tokenid": tokenid_string, + "value": secret + })) +} + +#[api( + protected: true, + input: { + properties: { + userid: { + type: Userid, + }, + 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<String>, + enable: Option<bool>, + expire: Option<i64>, + digest: Option<String>, +) -> 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 = Authid::from((userid, Some(tokenname))); + let tokenid_string = tokenid.to_string(); + + let mut data: user::ApiToken = config.lookup("token", &tokenid_string)?; + + 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) }; + } + + if let Some(expire) = expire { + data.expire = if expire > 0 { Some(expire) } else { None }; + } + + config.set_data(&tokenid_string, "token", &data)?; + + user::save_config(&config)?; + + Ok(()) +} + +#[api( + protected: true, + input: { + properties: { + userid: { + type: Userid, + }, + 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<String>, +) -> 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 = Authid::from((userid.clone(), Some(tokenname.clone()))); + let tokenid_string = tokenid.to_string(); + + match config.sections.get(&tokenid_string) { + Some(_) => { config.sections.remove(&tokenid_string); }, + 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: { + type: Userid, + }, + }, + }, + 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<Vec<user::ApiToken>, Error> { + + let (config, digest) = user::config()?; + + let list:Vec<user::ApiToken> = config.convert_to_typed_array("token")?; + + rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into(); + + let filter_by_owner = |token: &user::ApiToken| { + if token.tokenid.is_token() { + token.tokenid.user() == &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 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 06/16] api: allow listing users + tokens 2020-10-28 11:36 [pbs-devel] [PATCH proxmox-backup 00/16] API tokens Fabian Grünbichler ` (6 preceding siblings ...) 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-backup 05/16] api: add API token endpoints Fabian Grünbichler @ 2020-10-28 11:36 ` Fabian Grünbichler 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-backup 07/16] api: add permissions endpoint Fabian Grünbichler ` (5 subsequent siblings) 13 siblings, 0 replies; 25+ messages in thread From: Fabian Grünbichler @ 2020-10-28 11:36 UTC (permalink / raw) To: pbs-devel since it's not possible to extend existing structs, UserWithTokens duplicates most of user::User.. to avoid duplicating user::ApiToken as well, this returns full API token IDs, not just the token name part. Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> --- src/api2/access/user.rs | 121 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 117 insertions(+), 4 deletions(-) diff --git a/src/api2/access/user.rs b/src/api2/access/user.rs index 597ffeaf..a3d8da6c 100644 --- a/src/api2/access/user.rs +++ b/src/api2/access/user.rs @@ -1,5 +1,7 @@ use anyhow::{bail, Error}; +use serde::{Serialize, Deserialize}; use serde_json::{json, Value}; +use std::collections::HashMap; use std::convert::TryFrom; use proxmox::api::{api, ApiMethod, Router, RpcEnvironment, Permission}; @@ -19,9 +21,91 @@ pub const PBS_PASSWORD_SCHEMA: Schema = StringSchema::new("User Password.") .max_length(64) .schema(); +#[api( + properties: { + userid: { + type: Userid, + }, + comment: { + optional: true, + schema: SINGLE_LINE_COMMENT_SCHEMA, + }, + enable: { + optional: true, + schema: user::ENABLE_USER_SCHEMA, + }, + expire: { + optional: true, + schema: user::EXPIRE_USER_SCHEMA, + }, + firstname: { + optional: true, + schema: user::FIRST_NAME_SCHEMA, + }, + lastname: { + schema: user::LAST_NAME_SCHEMA, + optional: true, + }, + email: { + schema: user::EMAIL_SCHEMA, + optional: true, + }, + tokens: { + type: Array, + optional: true, + description: "List of user's API tokens.", + items: { + type: user::ApiToken + }, + }, + } +)] +#[derive(Serialize,Deserialize)] +/// User properties with added list of ApiTokens +pub struct UserWithTokens { + pub userid: Userid, + #[serde(skip_serializing_if="Option::is_none")] + pub comment: Option<String>, + #[serde(skip_serializing_if="Option::is_none")] + pub enable: Option<bool>, + #[serde(skip_serializing_if="Option::is_none")] + pub expire: Option<i64>, + #[serde(skip_serializing_if="Option::is_none")] + pub firstname: Option<String>, + #[serde(skip_serializing_if="Option::is_none")] + pub lastname: Option<String>, + #[serde(skip_serializing_if="Option::is_none")] + pub email: Option<String>, + #[serde(skip_serializing_if="Option::is_none")] + pub tokens: Option<Vec<user::ApiToken>>, +} + +impl UserWithTokens { + fn new(user: user::User) -> Self { + Self { + userid: user.userid, + comment: user.comment, + enable: user.enable, + expire: user.expire, + firstname: user.firstname, + lastname: user.lastname, + email: user.email, + tokens: None, + } + } +} + + #[api( input: { - properties: {}, + properties: { + include_tokens: { + type: bool, + description: "Include user's API tokens in returned list.", + optional: true, + default: false, + }, + }, }, returns: { description: "List users (with config digest).", @@ -35,10 +119,10 @@ pub const PBS_PASSWORD_SCHEMA: Schema = StringSchema::new("User Password.") )] /// List users pub fn list_users( - _param: Value, + include_tokens: bool, _info: &ApiMethod, mut rpcenv: &mut dyn RpcEnvironment, -) -> Result<Vec<user::User>, Error> { +) -> Result<Vec<UserWithTokens>, Error> { let (config, digest) = user::config()?; @@ -55,11 +139,40 @@ pub fn list_users( top_level_allowed || user.userid == userid }; + let list:Vec<user::User> = config.convert_to_typed_array("user")?; rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into(); - Ok(list.into_iter().filter(filter_by_privs).collect()) + let iter = list.into_iter().filter(filter_by_privs); + let list = if include_tokens { + let tokens:Vec<user::ApiToken> = config.convert_to_typed_array("token")?; + let mut user_to_tokens = tokens + .into_iter() + .fold( + HashMap::new(), + |mut map: HashMap<Userid, Vec<user::ApiToken>>, token: user::ApiToken| { + if token.tokenid.is_token() { + map + .entry(token.tokenid.user().clone()) + .or_default() + .push(token); + } + map + }); + iter + .map(|user: user::User| { + let mut user = UserWithTokens::new(user); + user.tokens = user_to_tokens.remove(&user.userid); + user + }) + .collect() + } else { + iter.map(|user: user::User| UserWithTokens::new(user)) + .collect() + }; + + Ok(list) } #[api( -- 2.20.1 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 07/16] api: add permissions endpoint 2020-10-28 11:36 [pbs-devel] [PATCH proxmox-backup 00/16] API tokens Fabian Grünbichler ` (7 preceding siblings ...) 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-backup 06/16] api: allow listing users + tokens Fabian Grünbichler @ 2020-10-28 11:36 ` Fabian Grünbichler 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-backup 08/16] client/remote: allow using ApiToken + secret Fabian Grünbichler ` (4 subsequent siblings) 13 siblings, 0 replies; 25+ messages in thread From: Fabian Grünbichler @ 2020-10-28 11:36 UTC (permalink / raw) To: pbs-devel and adapt privilege calculation to return propagate flag Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> --- changes since RFC: - drop paths field from ACL tree again - filter out paths with no permissions src/api2/access.rs | 131 ++++++++++++++++++++++++++++++++- src/config/acl.rs | 66 +++++++++-------- src/config/cached_user_info.rs | 19 ++++- 3 files changed, 181 insertions(+), 35 deletions(-) diff --git a/src/api2/access.rs b/src/api2/access.rs index d0494c9a..5e74a6ee 100644 --- a/src/api2/access.rs +++ b/src/api2/access.rs @@ -1,6 +1,8 @@ use anyhow::{bail, format_err, Error}; use serde_json::{json, Value}; +use std::collections::HashMap; +use std::collections::HashSet; use proxmox::api::{api, RpcEnvironment, Permission}; use proxmox::api::router::{Router, SubdirMap}; @@ -12,8 +14,9 @@ use crate::auth_helpers::*; use crate::api2::types::*; use crate::tools::{FileLogOptions, FileLogger}; +use crate::config::acl as acl_config; +use crate::config::acl::{PRIVILEGES, PRIV_SYS_AUDIT, PRIV_PERMISSIONS_MODIFY}; use crate::config::cached_user_info::CachedUserInfo; -use crate::config::acl::{PRIVILEGES, PRIV_PERMISSIONS_MODIFY}; pub mod user; pub mod domain; @@ -238,6 +241,128 @@ fn change_password( Ok(Value::Null) } +#[api( + input: { + properties: { + auth_id: { + type: Authid, + optional: true, + }, + path: { + schema: ACL_PATH_SCHEMA, + optional: true, + }, + }, + }, + access: { + permission: &Permission::Anybody, + description: "Requires Sys.Audit on '/access', limited to own privileges otherwise.", + }, + returns: { + description: "Map of ACL path to Map of privilege to propagate bit", + type: Object, + properties: {}, + additional_properties: true, + }, +)] +/// List permissions of given or currently authenticated user / API token. +/// +/// Optionally limited to specific path. +pub fn list_permissions( + auth_id: Option<Authid>, + path: Option<String>, + rpcenv: &dyn RpcEnvironment, +) -> Result<HashMap<String, HashMap<String, bool>>, Error> { + let current_auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; + + let user_info = CachedUserInfo::new()?; + let user_privs = user_info.lookup_privs(¤t_auth_id, &["access"]); + + let auth_id = if user_privs & PRIV_SYS_AUDIT == 0 { + match auth_id { + Some(auth_id) => { + if auth_id == current_auth_id { + auth_id + } else if auth_id.is_token() + && !current_auth_id.is_token() + && auth_id.user() == current_auth_id.user() { + auth_id + } else { + bail!("not allowed to list permissions of {}", auth_id); + } + }, + None => current_auth_id, + } + } else { + match auth_id { + Some(auth_id) => auth_id, + None => current_auth_id, + } + }; + + + fn populate_acl_paths( + mut paths: HashSet<String>, + node: acl_config::AclTreeNode, + path: &str + ) -> HashSet<String> { + for (sub_path, child_node) in node.children { + let sub_path = format!("{}/{}", path, &sub_path); + paths = populate_acl_paths(paths, child_node, &sub_path); + paths.insert(sub_path); + } + paths + } + + let paths = match path { + Some(path) => { + let mut paths = HashSet::new(); + paths.insert(path); + paths + }, + None => { + let mut paths = HashSet::new(); + + let (acl_tree, _) = acl_config::config()?; + paths = populate_acl_paths(paths, acl_tree.root, ""); + + // default paths, returned even if no ACL exists + paths.insert("/".to_string()); + paths.insert("/access".to_string()); + paths.insert("/datastore".to_string()); + paths.insert("/remote".to_string()); + paths.insert("/system".to_string()); + + paths + }, + }; + + let map = paths + .into_iter() + .fold(HashMap::new(), |mut map: HashMap<String, HashMap<String, bool>>, path: String| { + let split_path = acl_config::split_acl_path(path.as_str()); + let (privs, propagated_privs) = user_info.lookup_privs_details(&auth_id, &split_path); + + match privs { + 0 => map, // Don't leak ACL paths where we don't have any privileges + _ => { + let priv_map = PRIVILEGES + .iter() + .fold(HashMap::new(), |mut priv_map, (name, value)| { + if value & privs != 0 { + priv_map.insert(name.to_string(), value & propagated_privs != 0); + } + priv_map + }); + + map.insert(path, priv_map); + map + }, + }}); + + Ok(map) +} + #[sortable] const SUBDIRS: SubdirMap = &sorted!([ ("acl", &acl::ROUTER), @@ -245,6 +370,10 @@ const SUBDIRS: SubdirMap = &sorted!([ "password", &Router::new() .put(&API_METHOD_CHANGE_PASSWORD) ), + ( + "permissions", &Router::new() + .get(&API_METHOD_LIST_PERMISSIONS) + ), ( "ticket", &Router::new() .post(&API_METHOD_CREATE_TICKET) diff --git a/src/config/acl.rs b/src/config/acl.rs index 12b5a851..f82d5903 100644 --- a/src/config/acl.rs +++ b/src/config/acl.rs @@ -1,5 +1,5 @@ use std::io::Write; -use std::collections::{HashMap, HashSet, BTreeMap, BTreeSet}; +use std::collections::{HashMap, BTreeMap, BTreeSet}; use std::path::{PathBuf, Path}; use std::sync::{Arc, RwLock}; use std::str::FromStr; @@ -246,9 +246,9 @@ impl AclTreeNode { } } - pub fn extract_roles(&self, auth_id: &Authid, all: bool) -> HashSet<String> { + pub fn extract_roles(&self, auth_id: &Authid, all: bool) -> HashMap<String, bool> { let user_roles = self.extract_user_roles(auth_id, all); - if !user_roles.is_empty() { + if !user_roles.is_empty() || auth_id.is_token() { // user privs always override group privs return user_roles }; @@ -256,33 +256,33 @@ impl AclTreeNode { self.extract_group_roles(auth_id.user(), all) } - pub fn extract_user_roles(&self, auth_id: &Authid, all: bool) -> HashSet<String> { + pub fn extract_user_roles(&self, auth_id: &Authid, all: bool) -> HashMap<String, bool> { - let mut set = HashSet::new(); + let mut map = HashMap::new(); let roles = match self.users.get(auth_id) { Some(m) => m, - None => return set, + None => return map, }; for (role, propagate) in roles { if *propagate || all { if role == ROLE_NAME_NO_ACCESS { - // return a set with a single role 'NoAccess' - let mut set = HashSet::new(); - set.insert(role.to_string()); - return set; + // return a map with a single role 'NoAccess' + let mut map = HashMap::new(); + map.insert(role.to_string(), false); + return map; } - set.insert(role.to_string()); + map.insert(role.to_string(), *propagate); } } - set + map } - pub fn extract_group_roles(&self, _user: &Userid, all: bool) -> HashSet<String> { + pub fn extract_group_roles(&self, _user: &Userid, all: bool) -> HashMap<String, bool> { - let mut set = HashSet::new(); + let mut map = HashMap::new(); for (_group, roles) in &self.groups { let is_member = false; // fixme: check if user is member of the group @@ -291,17 +291,17 @@ impl AclTreeNode { for (role, propagate) in roles { if *propagate || all { if role == ROLE_NAME_NO_ACCESS { - // return a set with a single role 'NoAccess' - let mut set = HashSet::new(); - set.insert(role.to_string()); - return set; + // return a map with a single role 'NoAccess' + let mut map = HashMap::new(); + map.insert(role.to_string(), false); + return map; } - set.insert(role.to_string()); + map.insert(role.to_string(), *propagate); } } } - set + map } pub fn delete_group_role(&mut self, group: &str, role: &str) { @@ -346,7 +346,9 @@ impl AclTreeNode { impl AclTree { pub fn new() -> Self { - Self { root: AclTreeNode::new() } + Self { + root: AclTreeNode::new(), + } } pub fn find_node(&mut self, path: &str) -> Option<&mut AclTreeNode> { @@ -512,7 +514,8 @@ impl AclTree { bail!("expected '0' or '1' for propagate flag."); }; - let path = split_acl_path(items[2]); + let path_str = items[2]; + let path = split_acl_path(path_str); let node = self.get_or_insert_node(&path); let uglist: Vec<&str> = items[3].split(',').map(|v| v.trim()).collect(); @@ -576,25 +579,26 @@ impl AclTree { Ok(tree) } - pub fn roles(&self, auth_id: &Authid, path: &[&str]) -> HashSet<String> { + pub fn roles(&self, auth_id: &Authid, path: &[&str]) -> HashMap<String, bool> { let mut node = &self.root; - let mut role_set = node.extract_roles(auth_id, path.is_empty()); + let mut role_map = node.extract_roles(auth_id, path.is_empty()); for (pos, comp) in path.iter().enumerate() { let last_comp = (pos + 1) == path.len(); node = match node.children.get(*comp) { Some(n) => n, - None => return role_set, // path not found + None => return role_map, // path not found }; - let new_set = node.extract_roles(auth_id, last_comp); - if !new_set.is_empty() { - // overwrite previous settings - role_set = new_set; + + let new_map = node.extract_roles(auth_id, last_comp); + if !new_map.is_empty() { + // overwrite previous maptings + role_map = new_map; } } - role_set + role_map } } @@ -686,7 +690,7 @@ mod test { let path_vec = super::split_acl_path(path); let mut roles = tree.roles(auth_id, &path_vec) - .iter().map(|v| v.clone()).collect::<Vec<String>>(); + .iter().map(|(v, _)| v.clone()).collect::<Vec<String>>(); roles.sort(); let roles = roles.join(","); diff --git a/src/config/cached_user_info.rs b/src/config/cached_user_info.rs index 57d53aac..f56c07a8 100644 --- a/src/config/cached_user_info.rs +++ b/src/config/cached_user_info.rs @@ -123,14 +123,23 @@ impl CachedUserInfo { } pub fn lookup_privs(&self, auth_id: &Authid, path: &[&str]) -> u64 { + let (privs, _) = self.lookup_privs_details(auth_id, path); + privs + } + + pub fn lookup_privs_details(&self, auth_id: &Authid, path: &[&str]) -> (u64, u64) { if self.is_superuser(auth_id) { - return ROLE_ADMIN; + return (ROLE_ADMIN, ROLE_ADMIN); } let roles = self.acl_tree.roles(auth_id, path); let mut privs: u64 = 0; - for role in roles { + let mut propagated_privs: u64 = 0; + for (role, propagate) in roles { if let Some((role_privs, _)) = ROLE_NAMES.get(role.as_str()) { + if propagate { + propagated_privs |= role_privs; + } privs |= role_privs; } } @@ -139,10 +148,14 @@ impl CachedUserInfo { // limit privs to that of owning user let user_auth_id = Authid::from(auth_id.user().clone()); privs &= self.lookup_privs(&user_auth_id, path); + let (owner_privs, owner_propagated_privs) = self.lookup_privs_details(&user_auth_id, path); + privs &= owner_privs; + propagated_privs &= owner_propagated_privs; } - privs + (privs, propagated_privs) } + } impl UserInformation for CachedUserInfo { -- 2.20.1 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 08/16] client/remote: allow using ApiToken + secret 2020-10-28 11:36 [pbs-devel] [PATCH proxmox-backup 00/16] API tokens Fabian Grünbichler ` (8 preceding siblings ...) 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-backup 07/16] api: add permissions endpoint Fabian Grünbichler @ 2020-10-28 11:36 ` Fabian Grünbichler 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-backup 09/16] owner checks: handle backups owned by API tokens Fabian Grünbichler ` (3 subsequent siblings) 13 siblings, 0 replies; 25+ messages in thread From: Fabian Grünbichler @ 2020-10-28 11:36 UTC (permalink / raw) To: pbs-devel in place of user + password. Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> --- Notes: this will require more follow-up to disentagle the CLI args properly, but it allows using tokens with the client/pull as is. examples/download-speed.rs | 6 +- examples/upload-speed.rs | 6 +- src/api2/config/remote.rs | 2 +- src/api2/pull.rs | 2 +- src/api2/types/mod.rs | 2 +- src/bin/proxmox-backup-client.rs | 32 +++++----- src/bin/proxmox-backup-manager.rs | 4 +- src/bin/proxmox_backup_client/benchmark.rs | 2 +- src/bin/proxmox_backup_client/catalog.rs | 4 +- src/bin/proxmox_backup_client/mount.rs | 2 +- src/bin/proxmox_backup_client/task.rs | 8 +-- src/client/backup_repo.rs | 25 +++++--- src/client/http_client.rs | 68 +++++++++++++++------- src/client/pull.rs | 2 +- src/config/remote.rs | 2 +- 15 files changed, 101 insertions(+), 66 deletions(-) diff --git a/examples/download-speed.rs b/examples/download-speed.rs index fa278436..3ccf4ce7 100644 --- a/examples/download-speed.rs +++ b/examples/download-speed.rs @@ -2,7 +2,7 @@ use std::io::Write; use anyhow::{Error}; -use proxmox_backup::api2::types::Userid; +use proxmox_backup::api2::types::Authid; use proxmox_backup::client::{HttpClient, HttpClientOptions, BackupReader}; pub struct DummyWriter { @@ -26,13 +26,13 @@ async fn run() -> Result<(), Error> { let host = "localhost"; - let username = Userid::root_userid(); + let auth_id = Authid::root_auth_id(); let options = HttpClientOptions::new() .interactive(true) .ticket_cache(true); - let client = HttpClient::new(host, 8007, username, options)?; + let client = HttpClient::new(host, 8007, auth_id, options)?; let backup_time = proxmox::tools::time::parse_rfc3339("2019-06-28T10:49:48Z")?; diff --git a/examples/upload-speed.rs b/examples/upload-speed.rs index d26e6db3..641ed952 100644 --- a/examples/upload-speed.rs +++ b/examples/upload-speed.rs @@ -1,6 +1,6 @@ use anyhow::{Error}; -use proxmox_backup::api2::types::Userid; +use proxmox_backup::api2::types::Authid; use proxmox_backup::client::*; async fn upload_speed() -> Result<f64, Error> { @@ -8,13 +8,13 @@ async fn upload_speed() -> Result<f64, Error> { let host = "localhost"; let datastore = "store2"; - let username = Userid::root_userid(); + let auth_id = Authid::root_auth_id(); let options = HttpClientOptions::new() .interactive(true) .ticket_cache(true); - let client = HttpClient::new(host, 8007, username, options)?; + let client = HttpClient::new(host, 8007, auth_id, options)?; let backup_time = proxmox::tools::time::epoch_i64(); diff --git a/src/api2/config/remote.rs b/src/api2/config/remote.rs index dd2777c9..96869695 100644 --- a/src/api2/config/remote.rs +++ b/src/api2/config/remote.rs @@ -201,7 +201,7 @@ pub fn update_remote( comment: Option<String>, host: Option<String>, port: Option<u16>, - userid: Option<Userid>, + userid: Option<Authid>, password: Option<String>, fingerprint: Option<String>, delete: Option<Vec<DeletableProperty>>, diff --git a/src/api2/pull.rs b/src/api2/pull.rs index aef7de4e..d70fe014 100644 --- a/src/api2/pull.rs +++ b/src/api2/pull.rs @@ -56,7 +56,7 @@ pub async fn get_pull_parameters( let src_repo = BackupRepository::new(Some(remote.userid.clone()), Some(remote.host.clone()), remote.port, remote_store.to_string()); - let client = HttpClient::new(&src_repo.host(), src_repo.port(), &src_repo.user(), options)?; + let client = HttpClient::new(&src_repo.host(), src_repo.port(), &src_repo.auth_id(), options)?; let _auth_info = client.login() // make sure we can auth .await .map_err(|err| format_err!("remote connection to '{}' failed - {}", remote.host, err))?; diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs index b1287583..4084112d 100644 --- a/src/api2/types/mod.rs +++ b/src/api2/types/mod.rs @@ -67,7 +67,7 @@ const_regex!{ pub DNS_NAME_OR_IP_REGEX = concat!(r"^(?:", DNS_NAME!(), "|", IPRE!(), r")$"); - pub BACKUP_REPO_URL_REGEX = concat!(r"^^(?:(?:(", USER_ID_REGEX_STR!(), ")@)?(", DNS_NAME!(), "|", IPRE_BRACKET!() ,"):)?(?:([0-9]{1,5}):)?(", PROXMOX_SAFE_ID_REGEX_STR!(), r")$"); + pub BACKUP_REPO_URL_REGEX = concat!(r"^^(?:(?:(", USER_ID_REGEX_STR!(), "|", APITOKEN_ID_REGEX_STR!(), ")@)?(", DNS_NAME!(), "|", IPRE_BRACKET!() ,"):)?(?:([0-9]{1,5}):)?(", PROXMOX_SAFE_ID_REGEX_STR!(), r")$"); pub CERT_FINGERPRINT_SHA256_REGEX = r"^(?:[0-9a-fA-F][0-9a-fA-F])(?::[0-9a-fA-F][0-9a-fA-F]){31}$"; diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs index 8c68ffd2..ce263a77 100644 --- a/src/bin/proxmox-backup-client.rs +++ b/src/bin/proxmox-backup-client.rs @@ -193,7 +193,7 @@ pub fn complete_repository(_arg: &str, _param: &HashMap<String, String>) -> Vec< result } -fn connect(server: &str, port: u16, userid: &Userid) -> Result<HttpClient, Error> { +fn connect(server: &str, port: u16, auth_id: &Authid) -> Result<HttpClient, Error> { let fingerprint = std::env::var(ENV_VAR_PBS_FINGERPRINT).ok(); @@ -212,7 +212,7 @@ fn connect(server: &str, port: u16, userid: &Userid) -> Result<HttpClient, Error .fingerprint_cache(true) .ticket_cache(true); - HttpClient::new(server, port, userid, options) + HttpClient::new(server, port, auth_id, options) } async fn view_task_result( @@ -366,7 +366,7 @@ async fn list_backup_groups(param: Value) -> Result<Value, Error> { let repo = extract_repository_from_value(¶m)?; - let client = connect(repo.host(), repo.port(), repo.user())?; + let client = connect(repo.host(), repo.port(), repo.auth_id())?; let path = format!("api2/json/admin/datastore/{}/groups", repo.store()); @@ -435,7 +435,7 @@ async fn change_backup_owner(group: String, mut param: Value) -> Result<(), Erro let repo = extract_repository_from_value(¶m)?; - let mut client = connect(repo.host(), repo.port(), repo.user())?; + let mut client = connect(repo.host(), repo.port(), repo.auth_id())?; param.as_object_mut().unwrap().remove("repository"); @@ -478,7 +478,7 @@ async fn list_snapshots(param: Value) -> Result<Value, Error> { let output_format = get_output_format(¶m); - let client = connect(repo.host(), repo.port(), repo.user())?; + let client = connect(repo.host(), repo.port(), repo.auth_id())?; let group: Option<BackupGroup> = if let Some(path) = param["group"].as_str() { Some(path.parse()?) @@ -543,7 +543,7 @@ async fn forget_snapshots(param: Value) -> Result<Value, Error> { let path = tools::required_string_param(¶m, "snapshot")?; let snapshot: BackupDir = path.parse()?; - let mut client = connect(repo.host(), repo.port(), repo.user())?; + let mut client = connect(repo.host(), repo.port(), repo.auth_id())?; let path = format!("api2/json/admin/datastore/{}/snapshots", repo.store()); @@ -573,7 +573,7 @@ async fn api_login(param: Value) -> Result<Value, Error> { let repo = extract_repository_from_value(¶m)?; - let client = connect(repo.host(), repo.port(), repo.user())?; + let client = connect(repo.host(), repo.port(), repo.auth_id())?; client.login().await?; record_repository(&repo); @@ -630,7 +630,7 @@ async fn api_version(param: Value) -> Result<(), Error> { let repo = extract_repository_from_value(¶m); if let Ok(repo) = repo { - let client = connect(repo.host(), repo.port(), repo.user())?; + let client = connect(repo.host(), repo.port(), repo.auth_id())?; match client.get("api2/json/version", None).await { Ok(mut result) => version_info["server"] = result["data"].take(), @@ -680,7 +680,7 @@ async fn list_snapshot_files(param: Value) -> Result<Value, Error> { let output_format = get_output_format(¶m); - let client = connect(repo.host(), repo.port(), repo.user())?; + let client = connect(repo.host(), repo.port(), repo.auth_id())?; let path = format!("api2/json/admin/datastore/{}/files", repo.store()); @@ -724,7 +724,7 @@ async fn start_garbage_collection(param: Value) -> Result<Value, Error> { let output_format = get_output_format(¶m); - let mut client = connect(repo.host(), repo.port(), repo.user())?; + let mut client = connect(repo.host(), repo.port(), repo.auth_id())?; let path = format!("api2/json/admin/datastore/{}/gc", repo.store()); @@ -1036,7 +1036,7 @@ async fn create_backup( let backup_time = backup_time_opt.unwrap_or_else(|| epoch_i64()); - let client = connect(repo.host(), repo.port(), repo.user())?; + let client = connect(repo.host(), repo.port(), repo.auth_id())?; record_repository(&repo); println!("Starting backup: {}/{}/{}", backup_type, backup_id, BackupDir::backup_time_to_string(backup_time)?); @@ -1339,7 +1339,7 @@ async fn restore(param: Value) -> Result<Value, Error> { let archive_name = tools::required_string_param(¶m, "archive-name")?; - let client = connect(repo.host(), repo.port(), repo.user())?; + let client = connect(repo.host(), repo.port(), repo.auth_id())?; record_repository(&repo); @@ -1512,7 +1512,7 @@ async fn upload_log(param: Value) -> Result<Value, Error> { let snapshot = tools::required_string_param(¶m, "snapshot")?; let snapshot: BackupDir = snapshot.parse()?; - let mut client = connect(repo.host(), repo.port(), repo.user())?; + let mut client = connect(repo.host(), repo.port(), repo.auth_id())?; let (keydata, crypt_mode) = keyfile_parameters(¶m)?; @@ -1583,7 +1583,7 @@ fn prune<'a>( async fn prune_async(mut param: Value) -> Result<Value, Error> { let repo = extract_repository_from_value(¶m)?; - let mut client = connect(repo.host(), repo.port(), repo.user())?; + let mut client = connect(repo.host(), repo.port(), repo.auth_id())?; let path = format!("api2/json/admin/datastore/{}/prune", repo.store()); @@ -1666,7 +1666,7 @@ async fn status(param: Value) -> Result<Value, Error> { let output_format = get_output_format(¶m); - let client = connect(repo.host(), repo.port(), repo.user())?; + let client = connect(repo.host(), repo.port(), repo.auth_id())?; let path = format!("api2/json/admin/datastore/{}/status", repo.store()); @@ -1711,7 +1711,7 @@ async fn try_get(repo: &BackupRepository, url: &str) -> Value { .fingerprint_cache(true) .ticket_cache(true); - let client = match HttpClient::new(repo.host(), repo.port(), repo.user(), options) { + let client = match HttpClient::new(repo.host(), repo.port(), repo.auth_id(), options) { Ok(v) => v, _ => return Value::Null, }; diff --git a/src/bin/proxmox-backup-manager.rs b/src/bin/proxmox-backup-manager.rs index f13e55ee..549be2ef 100644 --- a/src/bin/proxmox-backup-manager.rs +++ b/src/bin/proxmox-backup-manager.rs @@ -62,10 +62,10 @@ fn connect() -> Result<HttpClient, Error> { let ticket = Ticket::new("PBS", Userid::root_userid())? .sign(private_auth_key(), None)?; options = options.password(Some(ticket)); - HttpClient::new("localhost", 8007, Userid::root_userid(), options)? + HttpClient::new("localhost", 8007, Authid::root_auth_id(), options)? } else { options = options.ticket_cache(true).interactive(true); - HttpClient::new("localhost", 8007, Userid::root_userid(), options)? + HttpClient::new("localhost", 8007, Authid::root_auth_id(), options)? }; Ok(client) diff --git a/src/bin/proxmox_backup_client/benchmark.rs b/src/bin/proxmox_backup_client/benchmark.rs index 37bb87fb..b434956d 100644 --- a/src/bin/proxmox_backup_client/benchmark.rs +++ b/src/bin/proxmox_backup_client/benchmark.rs @@ -225,7 +225,7 @@ async fn test_upload_speed( let backup_time = proxmox::tools::time::epoch_i64(); - let client = connect(repo.host(), repo.port(), repo.user())?; + let client = connect(repo.host(), repo.port(), repo.auth_id())?; record_repository(&repo); if verbose { eprintln!("Connecting to backup server"); } diff --git a/src/bin/proxmox_backup_client/catalog.rs b/src/bin/proxmox_backup_client/catalog.rs index e35692f2..87d80f3b 100644 --- a/src/bin/proxmox_backup_client/catalog.rs +++ b/src/bin/proxmox_backup_client/catalog.rs @@ -79,7 +79,7 @@ async fn dump_catalog(param: Value) -> Result<Value, Error> { } }; - let client = connect(repo.host(), repo.port(), repo.user())?; + let client = connect(repo.host(), repo.port(), repo.auth_id())?; let client = BackupReader::start( client, @@ -153,7 +153,7 @@ async fn dump_catalog(param: Value) -> Result<Value, Error> { /// Shell to interactively inspect and restore snapshots. async fn catalog_shell(param: Value) -> Result<(), Error> { let repo = extract_repository_from_value(¶m)?; - let client = connect(repo.host(), repo.port(), repo.user())?; + let client = connect(repo.host(), repo.port(), repo.auth_id())?; let path = tools::required_string_param(¶m, "snapshot")?; let archive_name = tools::required_string_param(¶m, "archive-name")?; diff --git a/src/bin/proxmox_backup_client/mount.rs b/src/bin/proxmox_backup_client/mount.rs index 69ab6d3f..6c91c132 100644 --- a/src/bin/proxmox_backup_client/mount.rs +++ b/src/bin/proxmox_backup_client/mount.rs @@ -163,7 +163,7 @@ fn mount( async fn mount_do(param: Value, pipe: Option<RawFd>) -> Result<Value, Error> { let repo = extract_repository_from_value(¶m)?; let archive_name = tools::required_string_param(¶m, "archive-name")?; - let client = connect(repo.host(), repo.port(), repo.user())?; + let client = connect(repo.host(), repo.port(), repo.auth_id())?; let target = param["target"].as_str(); diff --git a/src/bin/proxmox_backup_client/task.rs b/src/bin/proxmox_backup_client/task.rs index 72d8095c..3bf817d8 100644 --- a/src/bin/proxmox_backup_client/task.rs +++ b/src/bin/proxmox_backup_client/task.rs @@ -48,7 +48,7 @@ async fn task_list(param: Value) -> Result<Value, Error> { let output_format = get_output_format(¶m); let repo = extract_repository_from_value(¶m)?; - let client = connect(repo.host(), repo.port(), repo.user())?; + let client = connect(repo.host(), repo.port(), repo.auth_id())?; let limit = param["limit"].as_u64().unwrap_or(50) as usize; let running = !param["all"].as_bool().unwrap_or(false); @@ -57,7 +57,7 @@ async fn task_list(param: Value) -> Result<Value, Error> { "running": running, "start": 0, "limit": limit, - "userfilter": repo.user(), + "userfilter": repo.auth_id(), "store": repo.store(), }); @@ -96,7 +96,7 @@ async fn task_log(param: Value) -> Result<Value, Error> { let repo = extract_repository_from_value(¶m)?; let upid = tools::required_string_param(¶m, "upid")?; - let client = connect(repo.host(), repo.port(), repo.user())?; + let client = connect(repo.host(), repo.port(), repo.auth_id())?; display_task_log(client, upid, true).await?; @@ -122,7 +122,7 @@ async fn task_stop(param: Value) -> Result<Value, Error> { let repo = extract_repository_from_value(¶m)?; let upid_str = tools::required_string_param(¶m, "upid")?; - let mut client = connect(repo.host(), repo.port(), repo.user())?; + let mut client = connect(repo.host(), repo.port(), repo.auth_id())?; let path = format!("api2/json/nodes/localhost/tasks/{}", upid_str); let _ = client.delete(&path, None).await?; diff --git a/src/client/backup_repo.rs b/src/client/backup_repo.rs index c33314bf..091d2707 100644 --- a/src/client/backup_repo.rs +++ b/src/client/backup_repo.rs @@ -16,7 +16,7 @@ pub const BACKUP_REPO_URL: ApiStringFormat = ApiStringFormat::Pattern(&BACKUP_RE #[derive(Debug)] pub struct BackupRepository { /// The user name used for Authentication - user: Option<Userid>, + auth_id: Option<Authid>, /// The host name or IP address host: Option<String>, /// The port @@ -27,20 +27,29 @@ pub struct BackupRepository { impl BackupRepository { - pub fn new(user: Option<Userid>, host: Option<String>, port: Option<u16>, store: String) -> Self { + pub fn new(auth_id: Option<Authid>, host: Option<String>, port: Option<u16>, store: String) -> Self { let host = match host { Some(host) if (IP_V6_REGEX.regex_obj)().is_match(&host) => { Some(format!("[{}]", host)) }, other => other, }; - Self { user, host, port, store } + Self { auth_id, host, port, store } + } + + pub fn auth_id(&self) -> &Authid { + if let Some(ref auth_id) = self.auth_id { + return auth_id; + } + + &Authid::root_auth_id() } pub fn user(&self) -> &Userid { - if let Some(ref user) = self.user { - return &user; + if let Some(auth_id) = &self.auth_id { + return auth_id.user(); } + Userid::root_userid() } @@ -65,8 +74,8 @@ impl BackupRepository { impl fmt::Display for BackupRepository { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match (&self.user, &self.host, self.port) { - (Some(user), _, _) => write!(f, "{}@{}:{}:{}", user, self.host(), self.port(), self.store), + match (&self.auth_id, &self.host, self.port) { + (Some(auth_id), _, _) => write!(f, "{}@{}:{}:{}", auth_id, self.host(), self.port(), self.store), (None, Some(host), None) => write!(f, "{}:{}", host, self.store), (None, _, Some(port)) => write!(f, "{}:{}:{}", self.host(), port, self.store), (None, None, None) => write!(f, "{}", self.store), @@ -88,7 +97,7 @@ impl std::str::FromStr for BackupRepository { .ok_or_else(|| format_err!("unable to parse repository url '{}'", url))?; Ok(Self { - user: cap.get(1).map(|m| Userid::try_from(m.as_str().to_owned())).transpose()?, + auth_id: cap.get(1).map(|m| Authid::try_from(m.as_str().to_owned())).transpose()?, host: cap.get(2).map(|m| m.as_str().to_owned()), port: cap.get(3).map(|m| m.as_str().parse::<u16>()).transpose()?, store: cap[4].to_owned(), diff --git a/src/client/http_client.rs b/src/client/http_client.rs index b57630f8..3b7597fe 100644 --- a/src/client/http_client.rs +++ b/src/client/http_client.rs @@ -21,7 +21,7 @@ use proxmox::{ }; use super::pipe_to_stream::PipeToSendStream; -use crate::api2::types::Userid; +use crate::api2::types::{Authid, Userid}; use crate::tools::{ self, BroadcastFuture, @@ -31,7 +31,7 @@ use crate::tools::{ #[derive(Clone)] pub struct AuthInfo { - pub userid: Userid, + pub auth_id: Authid, pub ticket: String, pub token: String, } @@ -102,7 +102,7 @@ pub struct HttpClient { server: String, port: u16, fingerprint: Arc<Mutex<Option<String>>>, - first_auth: BroadcastFuture<()>, + first_auth: Option<BroadcastFuture<()>>, auth: Arc<RwLock<AuthInfo>>, ticket_abort: futures::future::AbortHandle, _options: HttpClientOptions, @@ -251,7 +251,7 @@ impl HttpClient { pub fn new( server: &str, port: u16, - userid: &Userid, + auth_id: &Authid, mut options: HttpClientOptions, ) -> Result<Self, Error> { @@ -311,6 +311,11 @@ impl HttpClient { let password = if let Some(password) = password { password } else { + let userid = if auth_id.is_token() { + bail!("API token secret must be provided!"); + } else { + auth_id.user() + }; let mut ticket_info = None; if use_ticket_cache { ticket_info = load_ticket_info(options.prefix.as_ref().unwrap(), server, userid); @@ -323,7 +328,7 @@ impl HttpClient { }; let auth = Arc::new(RwLock::new(AuthInfo { - userid: userid.clone(), + auth_id: auth_id.clone(), ticket: password.clone(), token: "".to_string(), })); @@ -336,14 +341,14 @@ impl HttpClient { let renewal_future = async move { loop { tokio::time::delay_for(Duration::new(60*15, 0)).await; // 15 minutes - let (userid, ticket) = { + let (auth_id, ticket) = { let authinfo = auth2.read().unwrap().clone(); - (authinfo.userid, authinfo.ticket) + (authinfo.auth_id, authinfo.ticket) }; - match Self::credentials(client2.clone(), server2.clone(), port, userid, ticket).await { + match Self::credentials(client2.clone(), server2.clone(), port, auth_id.user().clone(), ticket).await { Ok(auth) => { if use_ticket_cache & &prefix2.is_some() { - let _ = store_ticket_info(prefix2.as_ref().unwrap(), &server2, &auth.userid.to_string(), &auth.ticket, &auth.token); + let _ = store_ticket_info(prefix2.as_ref().unwrap(), &server2, &auth.auth_id.to_string(), &auth.ticket, &auth.token); } *auth2.write().unwrap() = auth; }, @@ -361,7 +366,7 @@ impl HttpClient { client.clone(), server.to_owned(), port, - userid.to_owned(), + auth_id.user().clone(), password.to_owned(), ).map_ok({ let server = server.to_string(); @@ -370,13 +375,20 @@ impl HttpClient { move |auth| { if use_ticket_cache & &prefix.is_some() { - let _ = store_ticket_info(prefix.as_ref().unwrap(), &server, &auth.userid.to_string(), &auth.ticket, &auth.token); + let _ = store_ticket_info(prefix.as_ref().unwrap(), &server, &auth.auth_id.to_string(), &auth.ticket, &auth.token); } *authinfo.write().unwrap() = auth; tokio::spawn(renewal_future); } }); + let first_auth = if auth_id.is_token() { + // TODO check access here? + None + } else { + Some(BroadcastFuture::new(Box::new(login_future))) + }; + Ok(Self { client, server: String::from(server), @@ -384,7 +396,7 @@ impl HttpClient { fingerprint: verified_fingerprint, auth, ticket_abort, - first_auth: BroadcastFuture::new(Box::new(login_future)), + first_auth, _options: options, }) } @@ -394,7 +406,10 @@ impl HttpClient { /// Login is done on demand, so this is only required if you need /// access to authentication data in 'AuthInfo'. pub async fn login(&self) -> Result<AuthInfo, Error> { - self.first_auth.listen().await?; + if let Some(future) = &self.first_auth { + future.listen().await?; + } + let authinfo = self.auth.read().unwrap(); Ok(authinfo.clone()) } @@ -477,10 +492,14 @@ impl HttpClient { let client = self.client.clone(); let auth = self.login().await?; - - let enc_ticket = format!("PBSAuthCookie={}", percent_encode(auth.ticket.as_bytes(), DEFAULT_ENCODE_SET)); - req.headers_mut().insert("Cookie", HeaderValue::from_str(&enc_ticket).unwrap()); - req.headers_mut().insert("CSRFPreventionToken", HeaderValue::from_str(&auth.token).unwrap()); + if auth.auth_id.is_token() { + let enc_api_token = format!("{}:{}", auth.auth_id, percent_encode(auth.ticket.as_bytes(), DEFAULT_ENCODE_SET)); + req.headers_mut().insert("Authorization", HeaderValue::from_str(&enc_api_token).unwrap()); + } else { + let enc_ticket = format!("PBSAuthCookie={}", percent_encode(auth.ticket.as_bytes(), DEFAULT_ENCODE_SET)); + req.headers_mut().insert("Cookie", HeaderValue::from_str(&enc_ticket).unwrap()); + req.headers_mut().insert("CSRFPreventionToken", HeaderValue::from_str(&auth.token).unwrap()); + } Self::api_request(client, req).await } @@ -579,11 +598,18 @@ impl HttpClient { protocol_name: String, ) -> Result<(H2Client, futures::future::AbortHandle), Error> { - let auth = self.login().await?; let client = self.client.clone(); + let auth = self.login().await?; + + if auth.auth_id.is_token() { + let enc_api_token = format!("{}:{}", auth.auth_id, percent_encode(auth.ticket.as_bytes(), DEFAULT_ENCODE_SET)); + req.headers_mut().insert("Authorization", HeaderValue::from_str(&enc_api_token).unwrap()); + } else { + let enc_ticket = format!("PBSAuthCookie={}", percent_encode(auth.ticket.as_bytes(), DEFAULT_ENCODE_SET)); + req.headers_mut().insert("Cookie", HeaderValue::from_str(&enc_ticket).unwrap()); + req.headers_mut().insert("CSRFPreventionToken", HeaderValue::from_str(&auth.token).unwrap()); + } - let enc_ticket = format!("PBSAuthCookie={}", percent_encode(auth.ticket.as_bytes(), DEFAULT_ENCODE_SET)); - req.headers_mut().insert("Cookie", HeaderValue::from_str(&enc_ticket).unwrap()); req.headers_mut().insert("UPGRADE", HeaderValue::from_str(&protocol_name).unwrap()); let resp = client.request(req).await?; @@ -636,7 +662,7 @@ impl HttpClient { let req = Self::request_builder(&server, port, "POST", "/api2/json/access/ticket", Some(data))?; let cred = Self::api_request(client, req).await?; let auth = AuthInfo { - userid: cred["data"]["username"].as_str().unwrap().parse()?, + auth_id: cred["data"]["username"].as_str().unwrap().parse()?, ticket: cred["data"]["ticket"].as_str().unwrap().to_owned(), token: cred["data"]["CSRFPreventionToken"].as_str().unwrap().to_owned(), }; diff --git a/src/client/pull.rs b/src/client/pull.rs index 7cb8d8e1..a80b10dc 100644 --- a/src/client/pull.rs +++ b/src/client/pull.rs @@ -451,7 +451,7 @@ pub async fn pull_group( .password(Some(auth_info.ticket.clone())) .fingerprint(fingerprint.clone()); - let new_client = HttpClient::new(src_repo.host(), src_repo.port(), src_repo.user(), options)?; + let new_client = HttpClient::new(src_repo.host(), src_repo.port(), src_repo.auth_id(), options)?; let reader = BackupReader::start( new_client, diff --git a/src/config/remote.rs b/src/config/remote.rs index 7ad653ac..14c57c0e 100644 --- a/src/config/remote.rs +++ b/src/config/remote.rs @@ -65,7 +65,7 @@ pub struct Remote { pub host: String, #[serde(skip_serializing_if="Option::is_none")] pub port: Option<u16>, - pub userid: Userid, + pub userid: Authid, #[serde(skip_serializing_if="String::is_empty")] #[serde(with = "proxmox::tools::serde::string_as_base64")] pub password: String, -- 2.20.1 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 09/16] owner checks: handle backups owned by API tokens 2020-10-28 11:36 [pbs-devel] [PATCH proxmox-backup 00/16] API tokens Fabian Grünbichler ` (9 preceding siblings ...) 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-backup 08/16] client/remote: allow using ApiToken + secret Fabian Grünbichler @ 2020-10-28 11:36 ` Fabian Grünbichler 2020-10-28 11:37 ` [pbs-devel] [PATCH proxmox-backup 10/16] tasks: allow unpriv users to read their tokens' tasks Fabian Grünbichler ` (2 subsequent siblings) 13 siblings, 0 replies; 25+ messages in thread From: Fabian Grünbichler @ 2020-10-28 11:36 UTC (permalink / raw) To: pbs-devel a user should be allowed to read/list/overwrite backups owned by their own tokens, but a token should not be able to read/list/overwrite backups owned by their owning user. when changing ownership of a backup group, a user should be able to transfer ownership to/from their own tokens if the backup is owned by them (or one of their tokens). Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> --- src/api2/admin/datastore.rs | 128 ++++++++++++++++++++++-------------- src/api2/backup.rs | 5 +- src/api2/reader.rs | 5 +- 3 files changed, 88 insertions(+), 50 deletions(-) diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs index 8862637d..9d00ed8d 100644 --- a/src/api2/admin/datastore.rs +++ b/src/api2/admin/datastore.rs @@ -44,13 +44,29 @@ use crate::config::acl::{ PRIV_DATASTORE_BACKUP, }; -fn check_backup_owner( +fn check_priv_or_backup_owner( store: &DataStore, group: &BackupGroup, auth_id: &Authid, + required_privs: u64, +) -> Result<(), Error> { + let user_info = CachedUserInfo::new()?; + let privs = user_info.lookup_privs(&auth_id, &["datastore", store.name()]); + + if privs & required_privs == 0 { + let owner = store.get_owner(group)?; + check_backup_owner(&owner, auth_id)?; + } + Ok(()) +} + +fn check_backup_owner( + owner: &Authid, + auth_id: &Authid, ) -> Result<(), Error> { - let owner = store.get_owner(group)?; - if &owner != auth_id { + let correct_owner = owner == auth_id + || (owner.is_token() && &Authid::from(owner.user().clone()) == auth_id); + if !correct_owner { bail!("backup owner check failed ({} != {})", auth_id, owner); } Ok(()) @@ -171,7 +187,7 @@ fn list_groups( let list_all = (user_privs & PRIV_DATASTORE_AUDIT) != 0; let owner = datastore.get_owner(group)?; - if !list_all && owner != auth_id { + if !list_all && check_backup_owner(&owner, &auth_id).is_err() { continue; } @@ -231,15 +247,11 @@ pub fn list_snapshot_files( ) -> Result<Vec<BackupContent>, Error> { let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; - let user_info = CachedUserInfo::new()?; - let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]); - let datastore = DataStore::lookup_datastore(&store)?; let snapshot = BackupDir::new(backup_type, backup_id, backup_time)?; - let allowed = (user_privs & (PRIV_DATASTORE_AUDIT | PRIV_DATASTORE_READ)) != 0; - if !allowed { check_backup_owner(&datastore, snapshot.group(), &auth_id)?; } + check_priv_or_backup_owner(&datastore, snapshot.group(), &auth_id, PRIV_DATASTORE_AUDIT | PRIV_DATASTORE_READ)?; let info = BackupInfo::new(&datastore.base_path(), snapshot)?; @@ -283,15 +295,11 @@ fn delete_snapshot( ) -> Result<Value, Error> { let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; - let user_info = CachedUserInfo::new()?; - let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]); let snapshot = BackupDir::new(backup_type, backup_id, backup_time)?; - let datastore = DataStore::lookup_datastore(&store)?; - let allowed = (user_privs & PRIV_DATASTORE_MODIFY) != 0; - if !allowed { check_backup_owner(&datastore, snapshot.group(), &auth_id)?; } + check_priv_or_backup_owner(&datastore, snapshot.group(), &auth_id, PRIV_DATASTORE_MODIFY)?; datastore.remove_backup_dir(&snapshot, false)?; @@ -362,7 +370,7 @@ pub fn list_snapshots ( let list_all = (user_privs & PRIV_DATASTORE_AUDIT) != 0; let owner = datastore.get_owner(group)?; - if !list_all && owner != auth_id { + if !list_all && check_backup_owner(&owner, &auth_id).is_err() { continue; } @@ -702,8 +710,6 @@ fn prune( let backup_id = tools::required_string_param(¶m, "backup-id")?; let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; - let user_info = CachedUserInfo::new()?; - let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]); let dry_run = param["dry-run"].as_bool().unwrap_or(false); @@ -711,8 +717,7 @@ fn prune( let datastore = DataStore::lookup_datastore(&store)?; - let allowed = (user_privs & PRIV_DATASTORE_MODIFY) != 0; - if !allowed { check_backup_owner(&datastore, &group, &auth_id)?; } + check_priv_or_backup_owner(&datastore, &group, &auth_id, PRIV_DATASTORE_MODIFY)?; let prune_options = PruneOptions { keep_last: param["keep-last"].as_u64(), @@ -960,8 +965,6 @@ fn download_file( let datastore = DataStore::lookup_datastore(store)?; let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; - let user_info = CachedUserInfo::new()?; - let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]); let file_name = tools::required_string_param(¶m, "file-name")?.to_owned(); @@ -971,8 +974,7 @@ fn download_file( let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?; - let allowed = (user_privs & PRIV_DATASTORE_READ) != 0; - if !allowed { check_backup_owner(&datastore, backup_dir.group(), &auth_id)?; } + check_priv_or_backup_owner(&datastore, backup_dir.group(), &auth_id, PRIV_DATASTORE_READ)?; println!("Download {} from {} ({}/{})", file_name, store, backup_dir, file_name); @@ -1033,8 +1035,6 @@ fn download_file_decoded( let datastore = DataStore::lookup_datastore(store)?; let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; - let user_info = CachedUserInfo::new()?; - let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]); let file_name = tools::required_string_param(¶m, "file-name")?.to_owned(); @@ -1044,8 +1044,7 @@ fn download_file_decoded( let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?; - let allowed = (user_privs & PRIV_DATASTORE_READ) != 0; - if !allowed { check_backup_owner(&datastore, backup_dir.group(), &auth_id)?; } + check_priv_or_backup_owner(&datastore, backup_dir.group(), &auth_id, PRIV_DATASTORE_READ)?; let (manifest, files) = read_backup_index(&datastore, &backup_dir)?; for file in files { @@ -1158,7 +1157,8 @@ fn upload_backup_log( let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?; let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; - check_backup_owner(&datastore, backup_dir.group(), &auth_id)?; + let owner = datastore.get_owner(backup_dir.group())?; + check_backup_owner(&owner, &auth_id)?; let mut path = datastore.base_path(); path.push(backup_dir.relative_path()); @@ -1228,13 +1228,10 @@ fn catalog( let datastore = DataStore::lookup_datastore(&store)?; let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; - let user_info = CachedUserInfo::new()?; - let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]); let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?; - let allowed = (user_privs & PRIV_DATASTORE_READ) != 0; - if !allowed { check_backup_owner(&datastore, backup_dir.group(), &auth_id)?; } + check_priv_or_backup_owner(&datastore, backup_dir.group(), &auth_id, PRIV_DATASTORE_READ)?; let file_name = CATALOG_NAME; @@ -1399,8 +1396,6 @@ fn pxar_file_download( let datastore = DataStore::lookup_datastore(&store)?; let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; - let user_info = CachedUserInfo::new()?; - let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]); let filepath = tools::required_string_param(¶m, "filepath")?.to_owned(); @@ -1410,8 +1405,7 @@ fn pxar_file_download( let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?; - let allowed = (user_privs & PRIV_DATASTORE_READ) != 0; - if !allowed { check_backup_owner(&datastore, backup_dir.group(), &auth_id)?; } + check_priv_or_backup_owner(&datastore, backup_dir.group(), &auth_id, PRIV_DATASTORE_READ)?; let mut components = base64::decode(&filepath)?; if components.len() > 0 && components[0] == '/' as u8 { @@ -1578,13 +1572,9 @@ fn get_notes( let datastore = DataStore::lookup_datastore(&store)?; let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; - let user_info = CachedUserInfo::new()?; - let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]); - let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?; - let allowed = (user_privs & PRIV_DATASTORE_READ) != 0; - if !allowed { check_backup_owner(&datastore, backup_dir.group(), &auth_id)?; } + check_priv_or_backup_owner(&datastore, backup_dir.group(), &auth_id, PRIV_DATASTORE_READ)?; let (manifest, _) = datastore.load_manifest(&backup_dir)?; @@ -1631,13 +1621,9 @@ fn set_notes( let datastore = DataStore::lookup_datastore(&store)?; let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; - let user_info = CachedUserInfo::new()?; - let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]); - let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?; - let allowed = (user_privs & PRIV_DATASTORE_READ) != 0; - if !allowed { check_backup_owner(&datastore, backup_dir.group(), &auth_id)?; } + check_priv_or_backup_owner(&datastore, backup_dir.group(), &auth_id, PRIV_DATASTORE_READ)?; datastore.update_manifest(&backup_dir,|manifest| { manifest.unprotected["notes"] = notes.into(); @@ -1664,7 +1650,8 @@ fn set_notes( }, }, access: { - permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_MODIFY, true), + permission: &Permission::Anybody, + description: "Datastore.Modify on whole datastore, or changing ownership between user and a user's token for owned backups with Datastore.Backup" }, )] /// Change owner of a backup group @@ -1673,15 +1660,60 @@ fn set_backup_owner( backup_type: String, backup_id: String, new_owner: Authid, - _rpcenv: &mut dyn RpcEnvironment, + rpcenv: &mut dyn RpcEnvironment, ) -> Result<(), Error> { let datastore = DataStore::lookup_datastore(&store)?; let backup_group = BackupGroup::new(backup_type, backup_id); + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; + let user_info = CachedUserInfo::new()?; + let privs = user_info.lookup_privs(&auth_id, &["datastore", &store]); + + let allowed = if (privs & PRIV_DATASTORE_MODIFY) != 0 { + // High-privilege user/token + true + } else if (privs & PRIV_DATASTORE_BACKUP) != 0 { + let owner = datastore.get_owner(&backup_group)?; + + match (owner.is_token(), new_owner.is_token()) { + (true, true) => { + // API token to API token, owned by same user + let owner = owner.user(); + let new_owner = new_owner.user(); + owner == new_owner && Authid::from(owner.clone()) == auth_id + }, + (true, false) => { + // API token to API token owner + Authid::from(owner.user().clone()) == auth_id + && new_owner == auth_id + }, + (false, true) => { + // API token owner to API token + owner == auth_id + && Authid::from(new_owner.user().clone()) == auth_id + }, + (false, false) => { + // User to User, not allowed for unprivileged users + false + }, + } + } else { + false + }; + + if !allowed { + return Err(http_err!(UNAUTHORIZED, + "{} does not have permission to change owner of backup group '{}' to {}", + auth_id, + backup_group, + new_owner, + )); + } + if !user_info.is_active_auth_id(&new_owner) { bail!("{} '{}' is inactive or non-existent", if new_owner.is_token() { diff --git a/src/api2/backup.rs b/src/api2/backup.rs index e030d60d..ce9a34ae 100644 --- a/src/api2/backup.rs +++ b/src/api2/backup.rs @@ -108,7 +108,10 @@ async move { let (owner, _group_guard) = datastore.create_locked_backup_group(&backup_group, &auth_id)?; // permission check - if owner != auth_id && worker_type != "benchmark" { + let correct_owner = owner == auth_id + || (owner.is_token() + && Authid::from(owner.user().clone()) == auth_id); + if !correct_owner && worker_type != "benchmark" { // only the owner is allowed to create additional snapshots bail!("backup owner check failed ({} != {})", auth_id, owner); } diff --git a/src/api2/reader.rs b/src/api2/reader.rs index 3eeece52..010d73e3 100644 --- a/src/api2/reader.rs +++ b/src/api2/reader.rs @@ -94,7 +94,10 @@ fn upgrade_to_backup_reader_protocol( let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?; if !priv_read { let owner = datastore.get_owner(backup_dir.group())?; - if owner != auth_id { + let correct_owner = owner == auth_id + || (owner.is_token() + && Authid::from(owner.user().clone()) == auth_id); + if !correct_owner { bail!("backup owner check failed!"); } } -- 2.20.1 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 10/16] tasks: allow unpriv users to read their tokens' tasks 2020-10-28 11:36 [pbs-devel] [PATCH proxmox-backup 00/16] API tokens Fabian Grünbichler ` (10 preceding siblings ...) 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-backup 09/16] owner checks: handle backups owned by API tokens Fabian Grünbichler @ 2020-10-28 11:37 ` Fabian Grünbichler 2020-10-28 11:37 ` [pbs-devel] [PATCH proxmox-backup 11/16] manager: add token commands Fabian Grünbichler ` (5 more replies) 2020-10-29 14:23 ` [pbs-devel] applied: [PATCH proxmox-backup 00/16] API tokens Wolfgang Bumiller 2020-10-29 19:50 ` [pbs-devel] " Thomas Lamprecht 13 siblings, 6 replies; 25+ messages in thread From: Fabian Grünbichler @ 2020-10-28 11:37 UTC (permalink / raw) To: pbs-devel and tighten down the return schema while we're at it. Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> --- src/api2/node/tasks.rs | 37 +++++++++++++++++++++++++------------ src/api2/types/mod.rs | 6 +++--- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/src/api2/node/tasks.rs b/src/api2/node/tasks.rs index 66af6d11..555f59cd 100644 --- a/src/api2/node/tasks.rs +++ b/src/api2/node/tasks.rs @@ -14,6 +14,16 @@ use crate::server::{self, UPID, TaskState, TaskListInfoIterator}; use crate::config::acl::{PRIV_SYS_AUDIT, PRIV_SYS_MODIFY}; use crate::config::cached_user_info::CachedUserInfo; +fn check_task_access(auth_id: &Authid, upid: &UPID) -> Result<(), Error> { + let task_auth_id = &upid.auth_id; + if auth_id == task_auth_id + || (task_auth_id.is_token() && &Authid::from(task_auth_id.user().clone()) == auth_id) { + Ok(()) + } else { + let user_info = CachedUserInfo::new()?; + user_info.check_privs(auth_id, &["system", "tasks"], PRIV_SYS_AUDIT, false) + } +} #[api( input: { @@ -57,9 +67,13 @@ use crate::config::cached_user_info::CachedUserInfo; description: "Worker ID (arbitrary ASCII string)", }, user: { - type: String, + type: Userid, description: "The user who started the task.", }, + tokenid: { + schema: PROXMOX_TOKEN_NAME_SCHEMA, + optional: true, + }, status: { type: String, description: "'running' or 'stopped'", @@ -85,11 +99,7 @@ async fn get_task_status( let upid = extract_upid(¶m)?; let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; - - if auth_id != upid.auth_id { - let user_info = CachedUserInfo::new()?; - user_info.check_privs(&auth_id, &["system", "tasks"], PRIV_SYS_AUDIT, false)?; - } + check_task_access(&auth_id, &upid)?; let mut result = json!({ "upid": param["upid"], @@ -99,9 +109,13 @@ async fn get_task_status( "starttime": upid.starttime, "type": upid.worker_type, "id": upid.worker_id, - "user": upid.auth_id, + "user": upid.auth_id.user(), }); + if upid.auth_id.is_token() { + result["tokenid"] = Value::from(upid.auth_id.tokenname().unwrap().as_str()); + } + if crate::server::worker_is_active(&upid).await? { result["status"] = Value::from("running"); } else { @@ -163,10 +177,7 @@ async fn read_task_log( let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; - if auth_id != upid.auth_id { - let user_info = CachedUserInfo::new()?; - user_info.check_privs(&auth_id, &["system", "tasks"], PRIV_SYS_AUDIT, false)?; - } + check_task_access(&auth_id, &upid)?; let test_status = param["test-status"].as_bool().unwrap_or(false); @@ -326,7 +337,9 @@ pub fn list_tasks( Err(_) => return None, }; - if !list_all && info.upid.auth_id != auth_id { return None; } + if !list_all && check_task_access(&auth_id, &info.upid).is_err() { + return None; + } if let Some(needle) = &userfilter { if !info.upid.auth_id.to_string().contains(needle) { return None; } diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs index 4084112d..acb5d6d0 100644 --- a/src/api2/types/mod.rs +++ b/src/api2/types/mod.rs @@ -627,7 +627,7 @@ pub struct StorageStatus { #[api( properties: { upid: { schema: UPID_SCHEMA }, - userid: { type: Authid }, + user: { type: Authid }, }, )] #[derive(Serialize, Deserialize)] @@ -647,7 +647,7 @@ pub struct TaskListItem { /// Worker ID (arbitrary ASCII string) pub worker_id: Option<String>, /// The authenticated entity who started the task - pub userid: Authid, + pub user: Authid, /// The task end time (Epoch) #[serde(skip_serializing_if="Option::is_none")] pub endtime: Option<i64>, @@ -670,7 +670,7 @@ impl From<crate::server::TaskListInfo> for TaskListItem { starttime: info.upid.starttime, worker_type: info.upid.worker_type, worker_id: info.upid.worker_id, - userid: info.upid.auth_id, + user: info.upid.auth_id, endtime, status, } -- 2.20.1 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 11/16] manager: add token commands 2020-10-28 11:37 ` [pbs-devel] [PATCH proxmox-backup 10/16] tasks: allow unpriv users to read their tokens' tasks Fabian Grünbichler @ 2020-10-28 11:37 ` Fabian Grünbichler 2020-10-28 11:37 ` [pbs-devel] [PATCH proxmox-backup 12/16] manager: add user permissions command Fabian Grünbichler ` (4 subsequent siblings) 5 siblings, 0 replies; 25+ messages in thread From: Fabian Grünbichler @ 2020-10-28 11:37 UTC (permalink / raw) To: pbs-devel to generate, list and delete tokens. adding them to ACLs already works out of the box. Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> --- src/bin/proxmox_backup_manager/user.rs | 62 ++++++++++++++++++++++++++ src/config/user.rs | 31 +++++++++++++ 2 files changed, 93 insertions(+) diff --git a/src/bin/proxmox_backup_manager/user.rs b/src/bin/proxmox_backup_manager/user.rs index af05e9b5..00749cee 100644 --- a/src/bin/proxmox_backup_manager/user.rs +++ b/src/bin/proxmox_backup_manager/user.rs @@ -6,6 +6,7 @@ use proxmox::api::{api, cli::*, RpcEnvironment, ApiHandler}; use proxmox_backup::config; use proxmox_backup::tools; use proxmox_backup::api2; +use proxmox_backup::api2::types::Userid; #[api( input: { @@ -48,6 +49,48 @@ fn list_users(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Er Ok(Value::Null) } +#[api( + input: { + properties: { + "output-format": { + schema: OUTPUT_FORMAT, + optional: true, + }, + userid: { + type: Userid, + } + } + } +)] +/// List tokens associated with user. +fn list_tokens(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> { + + let output_format = get_output_format(¶m); + + let info = &api2::access::user::API_METHOD_LIST_TOKENS; + let mut data = match info.handler { + ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?, + _ => unreachable!(), + }; + + let options = default_table_format_options() + .column(ColumnConfig::new("tokenid")) + .column( + ColumnConfig::new("enable") + .renderer(tools::format::render_bool_with_default_true) + ) + .column( + ColumnConfig::new("expire") + .renderer(tools::format::render_epoch) + ) + .column(ColumnConfig::new("comment")); + + format_and_print_result_full(&mut data, info.returns, &output_format, &options); + + Ok(Value::Null) +} + + pub fn user_commands() -> CommandLineInterface { let cmd_def = CliCommandMap::new() @@ -69,6 +112,25 @@ pub fn user_commands() -> CommandLineInterface { CliCommand::new(&api2::access::user::API_METHOD_DELETE_USER) .arg_param(&["userid"]) .completion_cb("userid", config::user::complete_userid) + ) + .insert( + "list-tokens", + CliCommand::new(&&API_METHOD_LIST_TOKENS) + .arg_param(&["userid"]) + .completion_cb("userid", config::user::complete_userid) + ) + .insert( + "generate-token", + CliCommand::new(&api2::access::user::API_METHOD_GENERATE_TOKEN) + .arg_param(&["userid", "tokenname"]) + .completion_cb("userid", config::user::complete_userid) + ) + .insert( + "delete-token", + CliCommand::new(&api2::access::user::API_METHOD_DELETE_TOKEN) + .arg_param(&["userid", "tokenname"]) + .completion_cb("userid", config::user::complete_userid) + .completion_cb("tokenname", config::user::complete_token_name) ); cmd_def.into() diff --git a/src/config/user.rs b/src/config/user.rs index 78571daa..5966c96d 100644 --- a/src/config/user.rs +++ b/src/config/user.rs @@ -265,3 +265,34 @@ pub fn complete_authid(_arg: &str, _param: &HashMap<String, String>) -> Vec<Stri Err(_) => vec![], } } + +// shell completion helper +pub fn complete_token_name(_arg: &str, param: &HashMap<String, String>) -> Vec<String> { + let data = match config() { + Ok((data, _digest)) => data, + Err(_) => return Vec::new(), + }; + + match param.get("userid") { + Some(userid) => { + let user = data.lookup::<User>("user", userid); + let tokens = data.convert_to_typed_array("token"); + match (user, tokens) { + (Ok(_), Ok(tokens)) => { + tokens + .into_iter() + .filter_map(|token: ApiToken| { + let tokenid = token.tokenid; + if tokenid.is_token() && tokenid.user() == userid { + Some(tokenid.tokenname().unwrap().as_str().to_string()) + } else { + None + } + }).collect() + }, + _ => vec![], + } + }, + None => vec![], + } +} -- 2.20.1 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 12/16] manager: add user permissions command 2020-10-28 11:37 ` [pbs-devel] [PATCH proxmox-backup 10/16] tasks: allow unpriv users to read their tokens' tasks Fabian Grünbichler 2020-10-28 11:37 ` [pbs-devel] [PATCH proxmox-backup 11/16] manager: add token commands Fabian Grünbichler @ 2020-10-28 11:37 ` Fabian Grünbichler 2020-10-28 11:37 ` [pbs-devel] [PATCH proxmox-backup 13/16] gui: add permissions button to user view Fabian Grünbichler ` (3 subsequent siblings) 5 siblings, 0 replies; 25+ messages in thread From: Fabian Grünbichler @ 2020-10-28 11:37 UTC (permalink / raw) To: pbs-devel useful for debugging complex ACL setups. Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> --- src/bin/proxmox_backup_manager/user.rs | 69 +++++++++++++++++++++++++- 1 file changed, 68 insertions(+), 1 deletion(-) diff --git a/src/bin/proxmox_backup_manager/user.rs b/src/bin/proxmox_backup_manager/user.rs index 00749cee..b1887f4e 100644 --- a/src/bin/proxmox_backup_manager/user.rs +++ b/src/bin/proxmox_backup_manager/user.rs @@ -1,12 +1,14 @@ use anyhow::Error; use serde_json::Value; +use std::collections::HashMap; + use proxmox::api::{api, cli::*, RpcEnvironment, ApiHandler}; use proxmox_backup::config; use proxmox_backup::tools; use proxmox_backup::api2; -use proxmox_backup::api2::types::Userid; +use proxmox_backup::api2::types::{ACL_PATH_SCHEMA, Authid, Userid}; #[api( input: { @@ -91,6 +93,64 @@ fn list_tokens(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, E } +#[api( + input: { + properties: { + "output-format": { + schema: OUTPUT_FORMAT, + optional: true, + }, + auth_id: { + type: Authid, + }, + path: { + schema: ACL_PATH_SCHEMA, + optional: true, + }, + } + } +)] +/// List permissions of user/token. +fn list_permissions(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> { + + let output_format = get_output_format(¶m); + + let info = &api2::access::API_METHOD_LIST_PERMISSIONS; + let mut data = match info.handler { + ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?, + _ => unreachable!(), + }; + + if output_format == "text" { + println!("Privileges with (*) have the propagate flag set\n"); + let data:HashMap<String, HashMap<String, bool>> = serde_json::from_value(data)?; + let mut paths:Vec<String> = data.keys().cloned().collect(); + paths.sort_unstable(); + for path in paths { + println!("Path: {}", path); + let priv_map = data.get(&path).unwrap(); + let mut privs:Vec<String> = priv_map.keys().cloned().collect(); + if privs.is_empty() { + println!("- NoAccess"); + } else { + privs.sort_unstable(); + for privilege in privs { + if *priv_map.get(&privilege).unwrap() { + println!("- {} (*)", privilege); + } else { + println!("- {}", privilege); + } + } + } + } + } else { + format_and_print_result(&mut data, &output_format); + } + + Ok(Value::Null) +} + + pub fn user_commands() -> CommandLineInterface { let cmd_def = CliCommandMap::new() @@ -131,6 +191,13 @@ pub fn user_commands() -> CommandLineInterface { .arg_param(&["userid", "tokenname"]) .completion_cb("userid", config::user::complete_userid) .completion_cb("tokenname", config::user::complete_token_name) + ) + .insert( + "permissions", + CliCommand::new(&&API_METHOD_LIST_PERMISSIONS) + .arg_param(&["auth_id"]) + .completion_cb("auth_id", config::user::complete_authid) + .completion_cb("path", config::datastore::complete_acl_path) ); cmd_def.into() -- 2.20.1 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 13/16] gui: add permissions button to user view 2020-10-28 11:37 ` [pbs-devel] [PATCH proxmox-backup 10/16] tasks: allow unpriv users to read their tokens' tasks Fabian Grünbichler 2020-10-28 11:37 ` [pbs-devel] [PATCH proxmox-backup 11/16] manager: add token commands Fabian Grünbichler 2020-10-28 11:37 ` [pbs-devel] [PATCH proxmox-backup 12/16] manager: add user permissions command Fabian Grünbichler @ 2020-10-28 11:37 ` Fabian Grünbichler 2020-10-28 11:37 ` [pbs-devel] [PATCH proxmox-backup 14/16] gui: add API token UI Fabian Grünbichler ` (2 subsequent siblings) 5 siblings, 0 replies; 25+ messages in thread From: Fabian Grünbichler @ 2020-10-28 11:37 UTC (permalink / raw) To: pbs-devel Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> --- Notes: requires proxmox-widget-toolkit with PermissionView www/config/UserView.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/www/config/UserView.js b/www/config/UserView.js index 91e7a83a..eed96514 100644 --- a/www/config/UserView.js +++ b/www/config/UserView.js @@ -63,6 +63,19 @@ Ext.define('PBS.config.UserView', { }).show(); }, + showPermissions: function() { + let me = this; + let view = me.getView(); + let selection = view.getSelection(); + + if (selection.length < 1) return; + + Ext.create('Proxmox.PermissionView', { + auth_id: selection[0].data.userid, + auth_id_name: 'auth_id', + }).show(); + }, + renderUsername: function(userid) { return Ext.String.htmlEncode(userid.match(/^(.+)@([^@]+)$/)[1]); }, @@ -122,6 +135,12 @@ Ext.define('PBS.config.UserView', { enableFn: (rec) => rec.data.userid !== 'root@pam', callback: 'reload', }, + { + xtype: 'proxmoxButton', + text: gettext('Permissions'), + handler: 'showPermissions', + disabled: true, + }, ], viewConfig: { -- 2.20.1 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 14/16] gui: add API token UI 2020-10-28 11:37 ` [pbs-devel] [PATCH proxmox-backup 10/16] tasks: allow unpriv users to read their tokens' tasks Fabian Grünbichler ` (2 preceding siblings ...) 2020-10-28 11:37 ` [pbs-devel] [PATCH proxmox-backup 13/16] gui: add permissions button to user view Fabian Grünbichler @ 2020-10-28 11:37 ` Fabian Grünbichler 2020-10-28 11:37 ` [pbs-devel] [PATCH proxmox-backup 15/16] acls: allow viewing/editing user's token ACLs Fabian Grünbichler 2020-10-28 11:37 ` [pbs-devel] [PATCH proxmox-backup 16/16] gui: add API " Fabian Grünbichler 5 siblings, 0 replies; 25+ messages in thread From: Fabian Grünbichler @ 2020-10-28 11:37 UTC (permalink / raw) To: pbs-devel Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> --- www/Makefile | 2 + www/NavigationTree.js | 6 ++ www/Utils.js | 8 ++ www/config/TokenView.js | 218 ++++++++++++++++++++++++++++++++++++++++ www/window/TokenEdit.js | 213 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 447 insertions(+) create mode 100644 www/config/TokenView.js create mode 100644 www/window/TokenEdit.js diff --git a/www/Makefile b/www/Makefile index 75d389d9..ab056c8c 100644 --- a/www/Makefile +++ b/www/Makefile @@ -13,12 +13,14 @@ JSSRC= \ data/RunningTasksStore.js \ button/TaskButton.js \ config/UserView.js \ + config/TokenView.js \ config/RemoteView.js \ config/ACLView.js \ config/SyncView.js \ config/VerifyView.js \ window/UserEdit.js \ window/UserPassword.js \ + window/TokenEdit.js \ window/VerifyJobEdit.js \ window/RemoteEdit.js \ window/SyncJobEdit.js \ diff --git a/www/NavigationTree.js b/www/NavigationTree.js index 6524a5c3..d4e5d966 100644 --- a/www/NavigationTree.js +++ b/www/NavigationTree.js @@ -34,6 +34,12 @@ Ext.define('PBS.store.NavigationStore', { path: 'pbsUserView', leaf: true, }, + { + text: gettext('API Token'), + iconCls: 'fa fa-user-o', + path: 'pbsTokenView', + leaf: true, + }, { text: gettext('Permissions'), iconCls: 'fa fa-unlock', diff --git a/www/Utils.js b/www/Utils.js index 221a2f2b..58319345 100644 --- a/www/Utils.js +++ b/www/Utils.js @@ -84,6 +84,14 @@ Ext.define('PBS.Utils', { return `Datastore ${what} ${id}`; }, + extractTokenUser: function(tokenid) { + return tokenid.match(/^(.+)!([^!]+)$/)[1]; + }, + + extractTokenName: function(tokenid) { + return tokenid.match(/^(.+)!([^!]+)$/)[2]; + }, + constructor: function() { var me = this; diff --git a/www/config/TokenView.js b/www/config/TokenView.js new file mode 100644 index 00000000..88b3f194 --- /dev/null +++ b/www/config/TokenView.js @@ -0,0 +1,218 @@ +Ext.define('pbs-tokens', { + extend: 'Ext.data.Model', + fields: [ + 'tokenid', 'tokenname', 'user', 'comment', + { type: 'boolean', name: 'enable', defaultValue: true }, + { type: 'date', dateFormat: 'timestamp', name: 'expire' }, + ], + idProperty: 'tokenid', +}); + +Ext.define('pbs-users-with-tokens', { + extend: 'Ext.data.Model', + fields: [ + 'userid', 'firstname', 'lastname', 'email', 'comment', + { type: 'boolean', name: 'enable', defaultValue: true }, + { type: 'date', dateFormat: 'timestamp', name: 'expire' }, + 'tokens', + ], + idProperty: 'userid', + proxy: { + type: 'proxmox', + url: '/api2/json/access/users/?include_tokens=1', + }, +}); + +Ext.define('PBS.config.TokenView', { + extend: 'Ext.grid.GridPanel', + alias: 'widget.pbsTokenView', + + stateful: true, + stateId: 'grid-tokens', + + title: gettext('API Tokens'), + + controller: { + xclass: 'Ext.app.ViewController', + + init: function(view) { + view.userStore = Ext.create('Proxmox.data.UpdateStore', { + autoStart: true, + interval: 5 * 1000, + storeId: 'pbs-users-with-tokens', + storeid: 'pbs-users-with-tokens', + model: 'pbs-users-with-tokens', + }); + view.userStore.on('load', this.onLoad, this); + view.on('destroy', view.userStore.stopUpdate); + Proxmox.Utils.monStoreErrors(view, view.userStore); + }, + + reload: function() { this.getView().userStore.load(); }, + + onLoad: function(store, data, success) { + if (!success) return; + + let tokenStore = this.getView().store.rstore; + + let records = []; + Ext.Array.each(data, function(user) { + let tokens = user.data.tokens || []; + Ext.Array.each(tokens, function(token) { + let r = {}; + r.tokenid = token.tokenid; + r.comment = token.comment; + r.expire = token.expire; + r.enable = token.enable; + records.push(r); + }); + }); + + tokenStore.loadData(records); + tokenStore.fireEvent('load', tokenStore, records, true); + }, + + addToken: function() { + let me = this; + Ext.create('PBS.window.TokenEdit', { + isCreate: true, + listeners: { + destroy: function() { + me.reload(); + }, + }, + }).show(); + }, + + editToken: function() { + let me = this; + let view = me.getView(); + let selection = view.getSelection(); + if (selection.length < 1) return; + Ext.create('PBS.window.TokenEdit', { + user: PBS.Utils.extractTokenUser(selection[0].data.tokenid), + tokenname: PBS.Utils.extractTokenName(selection[0].data.tokenid), + listeners: { + destroy: function() { + me.reload(); + }, + }, + }).show(); + }, + + showPermissions: function() { + let me = this; + let view = me.getView(); + let selection = view.getSelection(); + + if (selection.length < 1) return; + + Ext.create('Proxmox.PermissionView', { + auth_id: selection[0].data.tokenid, + auth_id_name: 'auth_id', + }).show(); + }, + + renderUser: function(tokenid) { + return Ext.String.htmlEncode(PBS.Utils.extractTokenUser(tokenid)); + }, + + renderTokenname: function(tokenid) { + return Ext.String.htmlEncode(PBS.Utils.extractTokenName(tokenid)); + }, + + }, + + listeners: { + activate: 'reload', + itemdblclick: 'editToken', + }, + + store: { + type: 'diff', + autoDestroy: true, + autoDestroyRstore: true, + sorters: 'tokenid', + model: 'pbs-tokens', + rstore: { + type: 'store', + proxy: 'memory', + storeid: 'pbs-tokens', + model: 'pbs-tokens', + }, + }, + + tbar: [ + { + xtype: 'proxmoxButton', + text: gettext('Add'), + handler: 'addToken', + selModel: false, + }, + { + xtype: 'proxmoxButton', + text: gettext('Edit'), + handler: 'editToken', + disabled: true, + }, + { + xtype: 'proxmoxStdRemoveButton', + baseurl: '/access/users/', + callback: 'reload', + getUrl: function(rec) { + let tokenid = rec.getId(); + let user = PBS.Utils.extractTokenUser(tokenid); + let tokenname = PBS.Utils.extractTokenName(tokenid); + return '/access/users/' + encodeURIComponent(user) + '/token/' + encodeURIComponent(tokenname); + }, + }, + { + xtype: 'proxmoxButton', + text: gettext('Permissions'), + handler: 'showPermissions', + disabled: true, + }, + ], + + viewConfig: { + trackOver: false, + }, + + columns: [ + { + header: gettext('User'), + width: 200, + sortable: true, + renderer: 'renderUser', + dataIndex: 'tokenid', + }, + { + header: gettext('Token name'), + width: 100, + sortable: true, + renderer: 'renderTokenname', + dataIndex: 'tokenid', + }, + { + header: gettext('Enabled'), + width: 80, + sortable: true, + renderer: Proxmox.Utils.format_boolean, + dataIndex: 'enable', + }, + { + header: gettext('Expire'), + width: 80, + sortable: true, + renderer: Proxmox.Utils.format_expire, + dataIndex: 'expire', + }, + { + header: gettext('Comment'), + sortable: false, + renderer: Ext.String.htmlEncode, + dataIndex: 'comment', + flex: 1, + }, + ], +}); diff --git a/www/window/TokenEdit.js b/www/window/TokenEdit.js new file mode 100644 index 00000000..6b41ae9d --- /dev/null +++ b/www/window/TokenEdit.js @@ -0,0 +1,213 @@ +Ext.define('PBS.window.TokenEdit', { + extend: 'Proxmox.window.Edit', + alias: 'widget.pbsTokenEdit', + mixins: ['Proxmox.Mixin.CBind'], + + onlineHelp: 'user_mgmt', + + user: undefined, + tokenname: undefined, + + isAdd: true, + isCreate: false, + fixedUser: false, + + subject: gettext('API token'), + + fieldDefaults: { labelWidth: 120 }, + + items: { + xtype: 'inputpanel', + column1: [ + { + xtype: 'pmxDisplayEditField', + cbind: { + editable: (get) => get('isCreate') && !get('fixedUser'), + }, + editConfig: { + xtype: 'pbsUserSelector', + allowBlank: false, + }, + name: 'user', + value: Proxmox.UserName, + renderer: Ext.String.htmlEncode, + fieldLabel: gettext('User'), + }, + { + xtype: 'pmxDisplayEditField', + cbind: { + editable: '{isCreate}', + }, + name: 'tokenname', + fieldLabel: gettext('Token Name'), + minLength: 2, + allowBlank: false, + }, + ], + + column2: [ + { + xtype: 'datefield', + name: 'expire', + emptyText: Proxmox.Utils.neverText, + format: 'Y-m-d', + submitFormat: 'U', + fieldLabel: gettext('Expire'), + }, + { + xtype: 'proxmoxcheckbox', + fieldLabel: gettext('Enabled'), + name: 'enable', + uncheckedValue: 0, + defaultValue: 1, + checked: true, + }, + ], + + columnB: [ + { + xtype: 'proxmoxtextfield', + name: 'comment', + fieldLabel: gettext('Comment'), + }, + ], + }, + + getValues: function(dirtyOnly) { + var me = this; + + var values = me.callParent(arguments); + + // hack: ExtJS datefield does not submit 0, so we need to set that + if (!values.expire) { + values.expire = 0; + } + + if (me.isCreate) { + me.url = '/api2/extjs/access/users/'; + let uid = encodeURIComponent(values.user); + let tid = encodeURIComponent(values.tokenname); + delete values.user; + delete values.tokenname; + + me.url += `${uid}/token/${tid}`; + } + + return values; + }, + + setValues: function(values) { + var me = this; + + if (Ext.isDefined(values.expire)) { + if (values.expire) { + values.expire = new Date(values.expire * 1000); + } else { + // display 'never' instead of '1970-01-01' + values.expire = null; + } + } + + me.callParent([values]); + }, + + initComponent: function() { + let me = this; + + me.url = '/api2/extjs/access/users/'; + + me.callParent(); + + if (me.isCreate) { + me.method = 'POST'; + } else { + me.method = 'PUT'; + + let uid = encodeURIComponent(me.user); + let tid = encodeURIComponent(me.tokenname); + + me.url += `${uid}/token/${tid}`; + me.load({ + success: function(response, options) { + let values = response.result.data; + values.user = me.user; + values.tokenname = me.tokenname; + me.setValues(values); + }, + }); + } + }, + + apiCallDone: function(success, response, options) { + let res = response.result.data; + if (!success || !res || !res.value) { + return; + } + + Ext.create('PBS.window.TokenShow', { + autoShow: true, + tokenid: res.tokenid, + secret: res.value, + }); + }, +}); + +Ext.define('PBS.window.TokenShow', { + extend: 'Ext.window.Window', + alias: ['widget.pbsTokenShow'], + mixins: ['Proxmox.Mixin.CBind'], + + width: 600, + modal: true, + resizable: false, + title: gettext('Token Secret'), + + items: [ + { + xtype: 'container', + layout: 'form', + bodyPadding: 10, + border: false, + fieldDefaults: { + labelWidth: 100, + anchor: '100%', + }, + padding: '0 10 10 10', + items: [ + { + xtype: 'textfield', + fieldLabel: gettext('Token ID'), + cbind: { + value: '{tokenid}', + }, + editable: false, + }, + { + xtype: 'textfield', + fieldLabel: gettext('Secret'), + inputId: 'token-secret-value', + cbind: { + value: '{secret}', + }, + editable: false, + }, + ], + }, + { + xtype: 'component', + border: false, + padding: '10 10 10 10', + userCls: 'pmx-hint', + html: gettext('Please record the API token secret - it will only be displayed now'), + }, + ], + buttons: [ + { + handler: function(b) { + document.getElementById('token-secret-value').select(); + document.execCommand("copy"); + }, + text: gettext('Copy Secret Value'), + }, + ], +}); -- 2.20.1 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 15/16] acls: allow viewing/editing user's token ACLs 2020-10-28 11:37 ` [pbs-devel] [PATCH proxmox-backup 10/16] tasks: allow unpriv users to read their tokens' tasks Fabian Grünbichler ` (3 preceding siblings ...) 2020-10-28 11:37 ` [pbs-devel] [PATCH proxmox-backup 14/16] gui: add API token UI Fabian Grünbichler @ 2020-10-28 11:37 ` Fabian Grünbichler 2020-10-28 11:37 ` [pbs-devel] [PATCH proxmox-backup 16/16] gui: add API " Fabian Grünbichler 5 siblings, 0 replies; 25+ messages in thread From: Fabian Grünbichler @ 2020-10-28 11:37 UTC (permalink / raw) To: pbs-devel even for otherwise unprivileged users. since effective privileges of an API token are always intersected with those of their owning user, this does not allow an unprivileged user to elevate their privileges in practice, but avoids the need to involve a privileged user to deploy API tokens. Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> --- src/api2/access/acl.rs | 67 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 60 insertions(+), 7 deletions(-) diff --git a/src/api2/access/acl.rs b/src/api2/access/acl.rs index 7211a6be..a776ceaa 100644 --- a/src/api2/access/acl.rs +++ b/src/api2/access/acl.rs @@ -7,6 +7,7 @@ use proxmox::tools::fs::open_file_locked; use crate::api2::types::*; use crate::config::acl; use crate::config::acl::{Role, PRIV_SYS_AUDIT, PRIV_PERMISSIONS_MODIFY}; +use crate::config::cached_user_info::CachedUserInfo; #[api( properties: { @@ -43,8 +44,23 @@ fn extract_acl_node_data( path: &str, list: &mut Vec<AclListItem>, exact: bool, + token_user: &Option<Authid>, ) { + // tokens can't have tokens, so we can early return + if let Some(token_user) = token_user { + if token_user.is_token() { + return; + } + } + for (user, roles) in &node.users { + if let Some(token_user) = token_user { + if !user.is_token() + || user.user() != token_user.user() { + continue; + } + } + for (role, propagate) in roles { list.push(AclListItem { path: if path.is_empty() { String::from("/") } else { path.to_string() }, @@ -56,6 +72,10 @@ fn extract_acl_node_data( } } for (group, roles) in &node.groups { + if let Some(_) = token_user { + continue; + } + for (role, propagate) in roles { list.push(AclListItem { path: if path.is_empty() { String::from("/") } else { path.to_string() }, @@ -71,7 +91,7 @@ fn extract_acl_node_data( } for (comp, child) in &node.children { let new_path = format!("{}/{}", path, comp); - extract_acl_node_data(child, &new_path, list, exact); + extract_acl_node_data(child, &new_path, list, exact, token_user); } } @@ -98,7 +118,8 @@ fn extract_acl_node_data( } }, access: { - permission: &Permission::Privilege(&["access", "acl"], PRIV_SYS_AUDIT, false), + permission: &Permission::Anybody, + description: "Returns all ACLs if user has Sys.Audit on '/access/acl', or just the ACLs containing the user's API tokens.", }, )] /// Read Access Control List (ACLs). @@ -107,18 +128,26 @@ pub fn read_acl( exact: bool, mut rpcenv: &mut dyn RpcEnvironment, ) -> Result<Vec<AclListItem>, Error> { + let auth_id = rpcenv.get_auth_id().unwrap().parse()?; - //let auth_user = rpcenv.get_user().unwrap(); + let user_info = CachedUserInfo::new()?; + + let top_level_privs = user_info.lookup_privs(&auth_id, &["access", "acl"]); + let auth_id_filter = if (top_level_privs & PRIV_SYS_AUDIT) == 0 { + Some(auth_id) + } else { + None + }; let (mut tree, digest) = acl::config()?; let mut list: Vec<AclListItem> = Vec::new(); if let Some(path) = &path { if let Some(node) = &tree.find_node(path) { - extract_acl_node_data(&node, path, &mut list, exact); + extract_acl_node_data(&node, path, &mut list, exact, &auth_id_filter); } } else { - extract_acl_node_data(&tree.root, "", &mut list, exact); + extract_acl_node_data(&tree.root, "", &mut list, exact, &auth_id_filter); } rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into(); @@ -160,7 +189,8 @@ pub fn read_acl( }, }, access: { - permission: &Permission::Privilege(&["access", "acl"], PRIV_PERMISSIONS_MODIFY, false), + permission: &Permission::Anybody, + description: "Requires Permissions.Modify on '/access/acl', limited to updating ACLs of the user's API tokens otherwise." }, )] /// Update Access Control List (ACLs). @@ -172,8 +202,31 @@ pub fn update_acl( group: Option<String>, delete: Option<bool>, digest: Option<String>, - _rpcenv: &mut dyn RpcEnvironment, + rpcenv: &mut dyn RpcEnvironment, ) -> Result<(), Error> { + let current_auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; + + let user_info = CachedUserInfo::new()?; + + let top_level_privs = user_info.lookup_privs(¤t_auth_id, &["access", "acl"]); + if top_level_privs & PRIV_PERMISSIONS_MODIFY == 0 { + if let Some(_) = group { + bail!("Unprivileged users are not allowed to create group ACL item."); + } + + match &auth_id { + Some(auth_id) => { + if current_auth_id.is_token() { + bail!("Unprivileged API tokens can't set ACL items."); + } else if !auth_id.is_token() { + bail!("Unprivileged users can only set ACL items for API tokens."); + } else if auth_id.user() != current_auth_id.user() { + bail!("Unprivileged users can only set ACL items for their own API tokens."); + } + }, + None => { bail!("Unprivileged user needs to provide auth_id to update ACL item."); }, + }; + } let _lock = open_file_locked(acl::ACL_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?; -- 2.20.1 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 16/16] gui: add API token ACLs 2020-10-28 11:37 ` [pbs-devel] [PATCH proxmox-backup 10/16] tasks: allow unpriv users to read their tokens' tasks Fabian Grünbichler ` (4 preceding siblings ...) 2020-10-28 11:37 ` [pbs-devel] [PATCH proxmox-backup 15/16] acls: allow viewing/editing user's token ACLs Fabian Grünbichler @ 2020-10-28 11:37 ` Fabian Grünbichler 5 siblings, 0 replies; 25+ messages in thread From: Fabian Grünbichler @ 2020-10-28 11:37 UTC (permalink / raw) To: pbs-devel and the needed API token selector. Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> --- www/Makefile | 1 + www/config/ACLView.js | 42 +++++++++++++++++++---- www/form/TokenSelector.js | 72 +++++++++++++++++++++++++++++++++++++++ www/window/ACLEdit.js | 69 ++++++++++++++++++++++--------------- 4 files changed, 150 insertions(+), 34 deletions(-) create mode 100644 www/form/TokenSelector.js diff --git a/www/Makefile b/www/Makefile index ab056c8c..18e77b19 100644 --- a/www/Makefile +++ b/www/Makefile @@ -6,6 +6,7 @@ IMAGES := \ JSSRC= \ form/UserSelector.js \ + form/TokenSelector.js \ form/RemoteSelector.js \ form/DataStoreSelector.js \ form/CalendarEvent.js \ diff --git a/www/config/ACLView.js b/www/config/ACLView.js index d552b029..67bf04f8 100644 --- a/www/config/ACLView.js +++ b/www/config/ACLView.js @@ -31,19 +31,35 @@ Ext.define('PBS.config.ACLView', { controller: { xclass: 'Ext.app.ViewController', - addACL: function() { + addUserACL: function() { let me = this; let view = me.getView(); - Ext.create('PBS.window.ACLEdit', { + Ext.create('PBS.window.ACLEdit', { path: view.aclPath, + aclType: 'user', listeners: { destroy: function() { me.reload(); }, }, - }).show(); + }).show(); }, + addTokenACL: function() { + let me = this; + let view = me.getView(); + Ext.create('PBS.window.ACLEdit', { + path: view.aclPath, + aclType: 'token', + listeners: { + destroy: function() { + me.reload(); + }, + }, + }).show(); + }, + + removeACL: function(btn, event, rec) { let me = this; Proxmox.Utils.API2Request({ @@ -106,10 +122,22 @@ Ext.define('PBS.config.ACLView', { tbar: [ { - xtype: 'proxmoxButton', text: gettext('Add'), - handler: 'addACL', - selModel: false, + menu: { + xtype: 'menu', + items: [ + { + text: gettext('User Permission'), + iconCls: 'fa fa-fw fa-user', + handler: 'addUserACL', + }, + { + text: gettext('API Token Permission'), + iconCls: 'fa fa-fw fa-user-o', + handler: 'addTokenACL', + }, + ], + }, }, { xtype: 'proxmoxStdRemoveButton', @@ -127,7 +155,7 @@ Ext.define('PBS.config.ACLView', { dataIndex: 'path', }, { - header: gettext('User/Group'), + header: gettext('User/Group/API Token'), width: 100, sortable: true, renderer: Ext.String.htmlEncode, diff --git a/www/form/TokenSelector.js b/www/form/TokenSelector.js new file mode 100644 index 00000000..502fe827 --- /dev/null +++ b/www/form/TokenSelector.js @@ -0,0 +1,72 @@ +Ext.define('PBS.form.TokenSelector', { + extend: 'Proxmox.form.ComboGrid', + alias: 'widget.pbsTokenSelector', + + allowBlank: false, + autoSelect: false, + valueField: 'tokenid', + displayField: 'tokenid', + + editable: true, + anyMatch: true, + forceSelection: true, + + store: { + model: 'pbs-tokens', + params: { + enabled: 1, + }, + sorters: 'tokenid', + }, + + initComponent: function() { + let me = this; + me.userStore = Ext.create('Ext.data.Store', { + model: 'pbs-users-with-tokens', + }); + me.userStore.on('load', this.onLoad, this); + me.userStore.load(); + + me.callParent(); + }, + + onLoad: function(store, data, success) { + if (!success) return; + + let tokenStore = this.store; + + let records = []; + Ext.Array.each(data, function(user) { + let tokens = user.data.tokens || []; + Ext.Array.each(tokens, function(token) { + let r = {}; + r.tokenid = token.tokenid; + r.comment = token.comment; + r.expire = token.expire; + r.enable = token.enable; + records.push(r); + }); + }); + + tokenStore.loadData(records); + }, + + listConfig: { + columns: [ + { + header: gettext('API Token'), + sortable: true, + dataIndex: 'tokenid', + renderer: Ext.String.htmlEncode, + flex: 1, + }, + { + header: gettext('Comment'), + sortable: false, + dataIndex: 'comment', + renderer: Ext.String.htmlEncode, + flex: 1, + }, + ], + }, +}); diff --git a/www/window/ACLEdit.js b/www/window/ACLEdit.js index ffeb9e81..42db1ff6 100644 --- a/www/window/ACLEdit.js +++ b/www/window/ACLEdit.js @@ -14,47 +14,62 @@ Ext.define('PBS.window.ACLEdit', { // caller can give a static path path: undefined, - subject: gettext('User Permission'), - - getValues: function(dirtyOnly) { + initComponent: function() { let me = this; - let values = me.callParent(arguments); - if (me.path) { - values.path = me.path; - } - return values; - }, + me.items = []; - items: [ - { + me.items.push({ xtype: 'pbsPermissionPathSelector', fieldLabel: gettext('Path'), - cbind: { - editable: '{!path}', - value: '{path}', - }, + editable: !me.path, + value: me.path, name: 'path', allowBlank: false, - }, - { - xtype: 'pbsUserSelector', - fieldLabel: gettext('User'), - name: 'auth_id', - allowBlank: false, - }, - { + }); + + if (me.aclType === 'user') { + me.subject = gettext('User Permission'); + me.items.push({ + xtype: 'pbsUserSelector', + fieldLabel: gettext('User'), + name: 'auth_id', + allowBlank: false, + }); + } else if (me.aclType === 'token') { + me.subject = gettext('API Token Permission'); + me.items.push({ + xtype: 'pbsTokenSelector', + fieldLabel: gettext('API Token'), + name: 'auth_id', + allowBlank: false, + }); + } + me.items.push({ xtype: 'pmxRoleSelector', name: 'role', value: 'NoAccess', fieldLabel: gettext('Role'), - }, - { + }); + me.items.push({ xtype: 'proxmoxcheckbox', name: 'propagate', checked: true, uncheckedValue: 0, fieldLabel: gettext('Propagate'), - }, - ], + }); + + me.callParent(); + }, + + getValues: function(dirtyOnly) { + let me = this; + let values = me.callParent(arguments); + + if (me.path) { + values.path = me.path; + } + return values; + }, + }); -- 2.20.1 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [pbs-devel] applied: [PATCH proxmox-backup 00/16] API tokens 2020-10-28 11:36 [pbs-devel] [PATCH proxmox-backup 00/16] API tokens Fabian Grünbichler ` (11 preceding siblings ...) 2020-10-28 11:37 ` [pbs-devel] [PATCH proxmox-backup 10/16] tasks: allow unpriv users to read their tokens' tasks Fabian Grünbichler @ 2020-10-29 14:23 ` Wolfgang Bumiller 2020-10-29 19:50 ` [pbs-devel] " Thomas Lamprecht 13 siblings, 0 replies; 25+ messages in thread From: Wolfgang Bumiller @ 2020-10-29 14:23 UTC (permalink / raw) To: Fabian Grünbichler; +Cc: pbs-devel applied the rebased version from your git, added some fixups on top On Wed, Oct 28, 2020 at 12:36:21PM +0100, Fabian Grünbichler wrote: > changes since RFC: > - reworked Userid with wrapping Authid > - rename RpcEnvironment user -> auth_id > - lots of churn > - lots of corner cases > - lots of rebasing > - ACL editing for unprivileged users > - more GUI stuff added > > proxmox and proxmox-widget-toolkit patches needed for patches #3++ and > GUI respectively ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 00/16] API tokens 2020-10-28 11:36 [pbs-devel] [PATCH proxmox-backup 00/16] API tokens Fabian Grünbichler ` (12 preceding siblings ...) 2020-10-29 14:23 ` [pbs-devel] applied: [PATCH proxmox-backup 00/16] API tokens Wolfgang Bumiller @ 2020-10-29 19:50 ` Thomas Lamprecht 2020-10-30 8:03 ` Fabian Grünbichler 13 siblings, 1 reply; 25+ messages in thread From: Thomas Lamprecht @ 2020-10-29 19:50 UTC (permalink / raw) To: Proxmox Backup Server development discussion, Fabian Grünbichler On 28.10.20 12:36, Fabian Grünbichler wrote: > changes since RFC: > - reworked Userid with wrapping Authid > - rename RpcEnvironment user -> auth_id > - lots of churn > - lots of corner cases > - lots of rebasing > - ACL editing for unprivileged users > - more GUI stuff added > > proxmox and proxmox-widget-toolkit patches needed for patches #3++ and > GUI respectively > played a bit around, works nice in general, but please add some documentation, else one can have a bit of a hard time figuring the usage out. Also, I just may not remember if we discussed this, but why is there no "inherit the user permission" mode like PVE has? ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 00/16] API tokens 2020-10-29 19:50 ` [pbs-devel] " Thomas Lamprecht @ 2020-10-30 8:03 ` Fabian Grünbichler 2020-10-30 8:48 ` Thomas Lamprecht 0 siblings, 1 reply; 25+ messages in thread From: Fabian Grünbichler @ 2020-10-30 8:03 UTC (permalink / raw) To: Proxmox Backup Server development discussion, Thomas Lamprecht On October 29, 2020 8:50 pm, Thomas Lamprecht wrote: > On 28.10.20 12:36, Fabian Grünbichler wrote: >> changes since RFC: >> - reworked Userid with wrapping Authid >> - rename RpcEnvironment user -> auth_id >> - lots of churn >> - lots of corner cases >> - lots of rebasing >> - ACL editing for unprivileged users >> - more GUI stuff added >> >> proxmox and proxmox-widget-toolkit patches needed for patches #3++ and >> GUI respectively >> > > played a bit around, works nice in general, but please add some documentation, > else one can have a bit of a hard time figuring the usage out. yes, docs and some more improvements are on my todo list. > Also, I just may not remember if we discussed this, but why is there no > "inherit the user permission" mode like PVE has? because with the ACL changes (allowing a user to define arbitrary ACLs for their tokens) it's not really needed. I can just give Admin on '/' with propagation for the same effect. and also because I think it's better to make users think about which permissions to give a certain token (e.g., most tokens will probably just have Datastore.Backup on one datastore, which is something I'd like to put somewhere on the Datastore GUI as a two-step 'Create client token' thing) then nudging them into 'just tick this and everything works (but is not much better than just storing your username + password everywhere)'. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 00/16] API tokens 2020-10-30 8:03 ` Fabian Grünbichler @ 2020-10-30 8:48 ` Thomas Lamprecht 2020-10-30 9:55 ` Fabian Grünbichler 0 siblings, 1 reply; 25+ messages in thread From: Thomas Lamprecht @ 2020-10-30 8:48 UTC (permalink / raw) To: Fabian Grünbichler, Proxmox Backup Server development discussion On 30.10.20 09:03, Fabian Grünbichler wrote: > On October 29, 2020 8:50 pm, Thomas Lamprecht wrote: >> Also, I just may not remember if we discussed this, but why is there no >> "inherit the user permission" mode like PVE has? > > because with the ACL changes (allowing a user to define arbitrary ACLs > for their tokens) it's not really needed. I can just give Admin on '/' > with propagation for the same effect. hmm, true, but then I'd hint to the user that a newly created token has no permissions. > and also because I think it's > better to make users think about which permissions to give a certain > token (e.g., most tokens will probably just have Datastore.Backup on one > datastore, which is something I'd like to put somewhere on the Datastore > GUI as a two-step 'Create client token' thing) then nudging them into > 'just tick this and everything works (but is not much better than just > storing your username + password everywhere)'. > btw. can we enter a token in Proxmox VE storage add already? ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 00/16] API tokens 2020-10-30 8:48 ` Thomas Lamprecht @ 2020-10-30 9:55 ` Fabian Grünbichler 0 siblings, 0 replies; 25+ messages in thread From: Fabian Grünbichler @ 2020-10-30 9:55 UTC (permalink / raw) To: Proxmox Backup Server development discussion, Thomas Lamprecht On October 30, 2020 9:48 am, Thomas Lamprecht wrote: > On 30.10.20 09:03, Fabian Grünbichler wrote: >> On October 29, 2020 8:50 pm, Thomas Lamprecht wrote: >>> Also, I just may not remember if we discussed this, but why is there no >>> "inherit the user permission" mode like PVE has? >> >> because with the ACL changes (allowing a user to define arbitrary ACLs >> for their tokens) it's not really needed. I can just give Admin on '/' >> with propagation for the same effect. > > hmm, true, but then I'd hint to the user that a newly created token has > no permissions. might be a good addition to the 'Copy secret value' dialogue? > >> and also because I think it's >> better to make users think about which permissions to give a certain >> token (e.g., most tokens will probably just have Datastore.Backup on one >> datastore, which is something I'd like to put somewhere on the Datastore >> GUI as a two-step 'Create client token' thing) then nudging them into >> 'just tick this and everything works (but is not much better than just >> storing your username + password everywhere)'. >> > > btw. can we enter a token in Proxmox VE storage add already? yes, it should be usable everywhere by using tokenid + secret instead of userid + password (including remotes, PVE, proxmox-backup-client command-line), except for proxmox-backup-qemu for which I just sent a patch (Userid -> Authid). ^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2020-10-30 9:56 UTC | newest] Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-10-28 11:36 [pbs-devel] [PATCH proxmox-backup 00/16] API tokens Fabian Grünbichler 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-widget-toolkit] add PermissionView Fabian Grünbichler 2020-10-28 16:18 ` [pbs-devel] applied: " Thomas Lamprecht 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-backup 01/16] api: add Authid as wrapper around Userid Fabian Grünbichler 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox] rpcenv: rename user to auth_id Fabian Grünbichler 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-backup 02/16] config: add token.shadow file Fabian Grünbichler 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-backup 03/16] replace Userid with Authid Fabian Grünbichler 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-backup 04/16] REST: extract and handle API tokens Fabian Grünbichler 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-backup 05/16] api: add API token endpoints Fabian Grünbichler 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-backup 06/16] api: allow listing users + tokens Fabian Grünbichler 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-backup 07/16] api: add permissions endpoint Fabian Grünbichler 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-backup 08/16] client/remote: allow using ApiToken + secret Fabian Grünbichler 2020-10-28 11:36 ` [pbs-devel] [PATCH proxmox-backup 09/16] owner checks: handle backups owned by API tokens Fabian Grünbichler 2020-10-28 11:37 ` [pbs-devel] [PATCH proxmox-backup 10/16] tasks: allow unpriv users to read their tokens' tasks Fabian Grünbichler 2020-10-28 11:37 ` [pbs-devel] [PATCH proxmox-backup 11/16] manager: add token commands Fabian Grünbichler 2020-10-28 11:37 ` [pbs-devel] [PATCH proxmox-backup 12/16] manager: add user permissions command Fabian Grünbichler 2020-10-28 11:37 ` [pbs-devel] [PATCH proxmox-backup 13/16] gui: add permissions button to user view Fabian Grünbichler 2020-10-28 11:37 ` [pbs-devel] [PATCH proxmox-backup 14/16] gui: add API token UI Fabian Grünbichler 2020-10-28 11:37 ` [pbs-devel] [PATCH proxmox-backup 15/16] acls: allow viewing/editing user's token ACLs Fabian Grünbichler 2020-10-28 11:37 ` [pbs-devel] [PATCH proxmox-backup 16/16] gui: add API " Fabian Grünbichler 2020-10-29 14:23 ` [pbs-devel] applied: [PATCH proxmox-backup 00/16] API tokens Wolfgang Bumiller 2020-10-29 19:50 ` [pbs-devel] " Thomas Lamprecht 2020-10-30 8:03 ` Fabian Grünbichler 2020-10-30 8:48 ` Thomas Lamprecht 2020-10-30 9:55 ` Fabian Grünbichler
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox