* [pbs-devel] [RFC proxmox-backup 00/15] API tokens
@ 2020-10-19 7:39 Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [PATCH proxmox-backup 01/15] fix indentation Fabian Grünbichler
` (14 more replies)
0 siblings, 15 replies; 23+ messages in thread
From: Fabian Grünbichler @ 2020-10-19 7:39 UTC (permalink / raw)
To: pbs-devel
sending as RFC since there are a few implementation details up for
discussion while I struggle with the JS/GUI part ;)
it's mostly modeled after API tokens in PVE, with two differences:
- tokens are always privilege separated
- the user+token list returns full tokenids, not token names
first three patches are just cleanups that fell out of developing this
series.
missing/room for follow-ups:
- allow setting arbitrary ACLs for tokens (since they are restricted to
the permissions of the user anyway)
- cache authenticated tokens to avoid round trip through hashing/shadow
file?
- GUI
Fabian Grünbichler (15):
fix indentation
fix typos
REST: rename token to csrf_token
Userid: extend schema with token name
add ApiToken to user.cfg and CachedUserInfo
config: add token.shadow file
REST: extract and handle API tokens
api: add API token endpoints
api: allow listing users + tokens
api: add permissions endpoint
client: allow using ApiToken + secret
owner checks: handle backups owned by API tokens
tasks: allow unpriv users to read their tokens' tasks
manager: add token commands
manager: add user permissions command
src/api2/access.rs | 99 +++++-
src/api2/access/acl.rs | 2 +-
src/api2/access/user.rs | 450 ++++++++++++++++++++++++-
src/api2/admin/datastore.rs | 134 +++++---
src/api2/backup.rs | 3 +-
src/api2/config/remote.rs | 4 +-
src/api2/node/tasks.rs | 30 +-
src/api2/reader.rs | 3 +-
src/api2/types/mod.rs | 15 +-
src/api2/types/userid.rs | 367 +++++++++++++++++---
src/bin/proxmox-backup-client.rs | 6 +-
src/bin/proxmox_backup_manager/acl.rs | 2 +-
src/bin/proxmox_backup_manager/user.rs | 129 +++++++
src/client/http_client.rs | 41 ++-
src/config.rs | 1 +
src/config/acl.rs | 67 ++--
src/config/cached_user_info.rs | 65 +++-
src/config/remote.rs | 2 +-
src/config/token_shadow.rs | 79 +++++
src/config/user.rs | 98 +++++-
src/server/rest.rs | 71 ++--
21 files changed, 1443 insertions(+), 225 deletions(-)
create mode 100644 src/config/token_shadow.rs
--
2.20.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 01/15] fix indentation
2020-10-19 7:39 [pbs-devel] [RFC proxmox-backup 00/15] API tokens Fabian Grünbichler
@ 2020-10-19 7:39 ` Fabian Grünbichler
2020-10-19 12:00 ` [pbs-devel] applied: " Thomas Lamprecht
2020-10-19 7:39 ` [pbs-devel] [PATCH proxmox-backup 02/15] fix typos Fabian Grünbichler
` (13 subsequent siblings)
14 siblings, 1 reply; 23+ messages in thread
From: Fabian Grünbichler @ 2020-10-19 7:39 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
src/api2/admin/datastore.rs | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 4f15c1cd..75e6d32b 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -1489,7 +1489,7 @@ fn set_notes(
}
#[api(
- input: {
+ input: {
properties: {
store: {
schema: DATASTORE_SCHEMA,
@@ -1504,10 +1504,10 @@ fn set_notes(
type: Userid,
},
},
- },
- access: {
- permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_MODIFY, true),
- },
+ },
+ access: {
+ permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_MODIFY, true),
+ },
)]
/// Change owner of a backup group
fn set_backup_owner(
--
2.20.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 02/15] fix typos
2020-10-19 7:39 [pbs-devel] [RFC proxmox-backup 00/15] API tokens Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [PATCH proxmox-backup 01/15] fix indentation Fabian Grünbichler
@ 2020-10-19 7:39 ` Fabian Grünbichler
2020-10-19 12:01 ` [pbs-devel] applied: " Thomas Lamprecht
2020-10-19 7:39 ` [pbs-devel] [PATCH proxmox-backup 03/15] REST: rename token to csrf_token Fabian Grünbichler
` (12 subsequent siblings)
14 siblings, 1 reply; 23+ messages in thread
From: Fabian Grünbichler @ 2020-10-19 7:39 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
src/api2/node/tasks.rs | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/api2/node/tasks.rs b/src/api2/node/tasks.rs
index 1440859b..b86eaec7 100644
--- a/src/api2/node/tasks.rs
+++ b/src/api2/node/tasks.rs
@@ -27,7 +27,7 @@ use crate::config::cached_user_info::CachedUserInfo;
},
},
returns: {
- description: "Task status nformation.",
+ description: "Task status information.",
properties: {
node: {
schema: NODE_SCHEMA,
@@ -72,7 +72,7 @@ use crate::config::cached_user_info::CachedUserInfo;
},
},
access: {
- description: "Users can access there own tasks, or need Sys.Audit on /system/tasks.",
+ description: "Users can access their own tasks, or need Sys.Audit on /system/tasks.",
permission: &Permission::Anybody,
},
)]
--
2.20.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 03/15] REST: rename token to csrf_token
2020-10-19 7:39 [pbs-devel] [RFC proxmox-backup 00/15] API tokens Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [PATCH proxmox-backup 01/15] fix indentation Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [PATCH proxmox-backup 02/15] fix typos Fabian Grünbichler
@ 2020-10-19 7:39 ` Fabian Grünbichler
2020-10-19 12:02 ` [pbs-devel] applied: " Thomas Lamprecht
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 04/15] Userid: extend schema with token name Fabian Grünbichler
` (11 subsequent siblings)
14 siblings, 1 reply; 23+ messages in thread
From: Fabian Grünbichler @ 2020-10-19 7:39 UTC (permalink / raw)
To: pbs-devel
for easier differentiation with (future) api_token
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
src/server/rest.rs | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/src/server/rest.rs b/src/server/rest.rs
index 7235a291..c650a3aa 100644
--- a/src/server/rest.rs
+++ b/src/server/rest.rs
@@ -542,18 +542,18 @@ fn extract_auth_data(headers: &http::HeaderMap) -> (Option<String>, Option<Strin
}
}
- let token = match headers.get("CSRFPreventionToken").map(|v| v.to_str()) {
+ let csrf_token = match headers.get("CSRFPreventionToken").map(|v| v.to_str()) {
Some(Ok(v)) => Some(v.to_owned()),
_ => None,
};
- (ticket, token, language)
+ (ticket, csrf_token, language)
}
fn check_auth(
method: &hyper::Method,
ticket: &Option<String>,
- token: &Option<String>,
+ csrf_token: &Option<String>,
user_info: &CachedUserInfo,
) -> Result<Userid, Error> {
let ticket_lifetime = tools::ticket::TICKET_LIFETIME;
@@ -567,8 +567,8 @@ fn check_auth(
}
if method != hyper::Method::GET {
- if let Some(token) = token {
- verify_csrf_prevention_token(csrf_secret(), &userid, &token, -300, ticket_lifetime)?;
+ 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");
}
@@ -630,8 +630,8 @@ async fn handle_request(
}
if auth_required {
- let (ticket, token, _) = extract_auth_data(&parts.headers);
- match check_auth(&method, &ticket, &token, &user_info) {
+ 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())),
Err(err) => {
// always delay unauthorized calls by 3 seconds (from start of request)
@@ -684,12 +684,12 @@ async fn handle_request(
}
if comp_len == 0 {
- let (ticket, token, language) = extract_auth_data(&parts.headers);
+ let (ticket, csrf_token, language) = extract_auth_data(&parts.headers);
if ticket != None {
- match check_auth(&method, &ticket, &token, &user_info) {
+ match check_auth(&method, &ticket, &csrf_token, &user_info) {
Ok(userid) => {
- let new_token = assemble_csrf_prevention_token(csrf_secret(), &userid);
- return Ok(get_index(Some(userid), Some(new_token), language, &api, parts));
+ let new_csrf_token = assemble_csrf_prevention_token(csrf_secret(), &userid);
+ return Ok(get_index(Some(userid), Some(new_csrf_token), language, &api, parts));
}
_ => {
tokio::time::delay_until(Instant::from_std(delay_unauth_time)).await;
--
2.20.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [pbs-devel] [RFC proxmox-backup 04/15] Userid: extend schema with token name
2020-10-19 7:39 [pbs-devel] [RFC proxmox-backup 00/15] API tokens Fabian Grünbichler
` (2 preceding siblings ...)
2020-10-19 7:39 ` [pbs-devel] [PATCH proxmox-backup 03/15] REST: rename token to csrf_token Fabian Grünbichler
@ 2020-10-19 7:39 ` Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 05/15] add ApiToken to user.cfg and CachedUserInfo Fabian Grünbichler
` (10 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Fabian Grünbichler @ 2020-10-19 7:39 UTC (permalink / raw)
To: pbs-devel
similar to PVE, allow adding a !TOKENNAME suffix for API tokens
belonging to a specifc user.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
Notes:
not too happy with the schema names here, suggestion welcome
src/api2/access.rs | 4 +-
src/api2/access/acl.rs | 2 +-
src/api2/access/user.rs | 8 +-
src/api2/admin/datastore.rs | 2 +-
src/api2/config/remote.rs | 4 +-
src/api2/types/mod.rs | 13 +-
src/api2/types/userid.rs | 367 +++++++++++++++++++++++++++----
src/bin/proxmox-backup-client.rs | 2 +-
src/config/remote.rs | 2 +-
src/config/user.rs | 4 +-
10 files changed, 343 insertions(+), 65 deletions(-)
diff --git a/src/api2/access.rs b/src/api2/access.rs
index c302e0c7..0c19dab6 100644
--- a/src/api2/access.rs
+++ b/src/api2/access.rs
@@ -87,7 +87,7 @@ fn authenticate_user(
input: {
properties: {
username: {
- type: Userid,
+ schema: PROXMOX_USER_ID_SCHEMA,
},
password: {
schema: PASSWORD_SCHEMA,
@@ -189,7 +189,7 @@ fn create_ticket(
input: {
properties: {
userid: {
- type: Userid,
+ schema: PROXMOX_USER_ID_SCHEMA,
},
password: {
schema: PASSWORD_SCHEMA,
diff --git a/src/api2/access/acl.rs b/src/api2/access/acl.rs
index 3282c66e..cf9671c9 100644
--- a/src/api2/access/acl.rs
+++ b/src/api2/access/acl.rs
@@ -142,7 +142,7 @@ pub fn read_acl(
},
userid: {
optional: true,
- type: Userid,
+ schema: PROXMOX_USER_OR_TOKEN_ID_SCHEMA,
},
group: {
optional: true,
diff --git a/src/api2/access/user.rs b/src/api2/access/user.rs
index c041d804..6c292c2d 100644
--- a/src/api2/access/user.rs
+++ b/src/api2/access/user.rs
@@ -61,7 +61,7 @@ pub fn list_users(
input: {
properties: {
userid: {
- type: Userid,
+ schema: PROXMOX_USER_ID_SCHEMA,
},
comment: {
schema: SINGLE_LINE_COMMENT_SCHEMA,
@@ -127,7 +127,7 @@ pub fn create_user(password: Option<String>, param: Value) -> Result<(), Error>
input: {
properties: {
userid: {
- type: Userid,
+ schema: PROXMOX_USER_ID_SCHEMA,
},
},
},
@@ -155,7 +155,7 @@ pub fn read_user(userid: Userid, mut rpcenv: &mut dyn RpcEnvironment) -> Result<
input: {
properties: {
userid: {
- type: Userid,
+ schema: PROXMOX_USER_ID_SCHEMA,
},
comment: {
optional: true,
@@ -267,7 +267,7 @@ pub fn update_user(
input: {
properties: {
userid: {
- type: Userid,
+ schema: PROXMOX_USER_ID_SCHEMA,
},
digest: {
optional: true,
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 75e6d32b..5c9902e1 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -1501,7 +1501,7 @@ fn set_notes(
schema: BACKUP_ID_SCHEMA,
},
"new-owner": {
- type: Userid,
+ schema: PROXMOX_USER_OR_TOKEN_ID_SCHEMA,
},
},
},
diff --git a/src/api2/config/remote.rs b/src/api2/config/remote.rs
index d419be2b..00a5de73 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,
+ schema: PROXMOX_USER_OR_TOKEN_ID_SCHEMA,
},
password: {
schema: remote::REMOTE_PASSWORD_SCHEMA,
@@ -167,7 +167,7 @@ pub enum DeletableProperty {
},
userid: {
optional: true,
- type: Userid,
+ schema: PROXMOX_USER_OR_TOKEN_ID_SCHEMA,
},
password: {
optional: true,
diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs
index 75b68879..65411f73 100644
--- a/src/api2/types/mod.rs
+++ b/src/api2/types/mod.rs
@@ -14,9 +14,10 @@ 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::{PROXMOX_USER_ID_SCHEMA, PROXMOX_TOKEN_ID_SCHEMA, PROXMOX_TOKEN_NAME_SCHEMA, PROXMOX_USER_OR_TOKEN_ID_SCHEMA, PROXMOX_GROUP_ID_SCHEMA};
// File names: may not contain slashes, may not start with "."
pub const FILENAME_FORMAT: ApiStringFormat = ApiStringFormat::VerifyFn(|name| {
@@ -364,7 +365,7 @@ pub const BLOCKDEVICE_NAME_SCHEMA: Schema = StringSchema::new("Block device name
},
},
owner: {
- type: Userid,
+ schema: PROXMOX_USER_OR_TOKEN_ID_SCHEMA,
optional: true,
},
},
@@ -440,7 +441,7 @@ pub struct SnapshotVerifyState {
},
},
owner: {
- type: Userid,
+ schema: PROXMOX_USER_OR_TOKEN_ID_SCHEMA,
optional: true,
},
},
@@ -612,7 +613,7 @@ pub struct StorageStatus {
#[api(
properties: {
upid: { schema: UPID_SCHEMA },
- user: { type: Userid },
+ user: { schema: PROXMOX_USER_OR_TOKEN_ID_SCHEMA },
},
)]
#[derive(Serialize, Deserialize)]
@@ -977,7 +978,7 @@ fn test_proxmox_user_id_schema() -> Result<(), anyhow::Error> {
];
for name in invalid_user_ids.iter() {
- if let Ok(_) = parse_simple_value(name, &Userid::API_SCHEMA) {
+ if let Ok(_) = parse_simple_value(name, &PROXMOX_USER_ID_SCHEMA) {
bail!("test userid '{}' failed - got Ok() while exception an error.", name);
}
}
@@ -991,7 +992,7 @@ fn test_proxmox_user_id_schema() -> Result<(), anyhow::Error> {
];
for name in valid_user_ids.iter() {
- let v = match parse_simple_value(name, &Userid::API_SCHEMA) {
+ let v = match parse_simple_value(name, &PROXMOX_USER_ID_SCHEMA) {
Ok(v) => v,
Err(err) => {
bail!("unable to parse userid '{}' - {}", name, err);
diff --git a/src/api2/types/userid.rs b/src/api2/types/userid.rs
index 44cd10b7..591c7d26 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`].
//!
//! Since they're all string types, they're organized as follows:
//!
@@ -9,10 +10,12 @@
//! 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 ID (`String` equivalent)
+//! * [`TokennameRef`]: a borrowed `Tokenname` (`str` equivalent).
+//! * [`Userid`]: an owned user id (`"user@realm"`), or API token ID (`"user@realm!tokenid"`). Note
+//! that this does 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.
@@ -36,19 +39,54 @@ 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_USER_OR_APITOKEN_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_USER_OR_TOKEN_ID_FORMAT: ApiStringFormat =
+ ApiStringFormat::Pattern(&PROXMOX_USER_OR_APITOKEN_ID_REGEX);
+
+pub const PROXMOX_USER_ID_SCHEMA: Schema = StringSchema::new("User ID")
+ .format(&PROXMOX_USER_ID_FORMAT)
+ .min_length(3)
+ .max_length(64)
+ .schema();
+
+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_USER_OR_TOKEN_ID_SCHEMA: Schema = StringSchema::new("User ID (with optional API token subid)")
+ .format(&PROXMOX_USER_OR_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 +129,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,24 +304,143 @@ 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 user 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 user name part of a user id. This alone does NOT uniquely identify the user.
+///
+/// This is like a `str` to the `String` of a [`Username`].
+#[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, and optional token name.
#[derive(Clone, Debug, Hash)]
pub struct Userid {
data: String,
name_len: usize,
+ token_len: usize,
//name: Username,
//realm: Realm,
}
impl Userid {
- pub const API_SCHEMA: Schema = StringSchema::new("User ID")
- .format(&PROXMOX_USER_ID_FORMAT)
- .min_length(3)
- .max_length(64)
- .schema();
-
- const fn new(data: String, name_len: usize) -> Self {
- Self { data, name_len }
+ const fn new(data: String, name_len: usize, token_len: usize) -> Self {
+ Self { data, name_len, token_len }
}
pub fn name(&self) -> &UsernameRef {
@@ -311,7 +448,33 @@ impl Userid {
}
pub fn realm(&self) -> &RealmRef {
- RealmRef::new(&self.data[(self.name_len + 1)..])
+ if self.token_len > 0 {
+ RealmRef::new(&self.data[(self.name_len + 1)..(self.data.len() - self.token_len - 1)])
+ } else {
+ RealmRef::new(&self.data[(self.name_len + 1)..])
+ }
+ }
+
+ pub fn tokenname(&self) -> Option<&TokennameRef> {
+ if self.token_len > 0 {
+ Some(TokennameRef::new(&self.data[(self.data.len() - self.token_len)..]))
+ } else {
+ None
+ }
+ }
+
+ pub fn is_tokenid(&self) -> bool {
+ self.token_len > 0
+ }
+
+ pub fn owner(&self) -> Result<Userid, Error> {
+ if !self.is_tokenid() {
+ bail!("userid is a regular user, not a token - can't determine owner");
+ }
+
+ let owner_str = &self.data.clone()[..self.data.len() - 1 - self.token_len];
+
+ Ok(Userid::new(owner_str.to_string(), self.name_len, 0))
}
pub fn as_str(&self) -> &str {
@@ -330,15 +493,17 @@ impl Userid {
}
lazy_static! {
- pub static ref BACKUP_USERID: Userid = Userid::new("backup@pam".to_string(), 6);
- pub static ref ROOT_USERID: Userid = Userid::new("root@pam".to_string(), 4);
+ pub static ref BACKUP_USERID: Userid = Userid::new("backup@pam".to_string(), 6, 0);
+ pub static ref ROOT_USERID: Userid = Userid::new("root@pam".to_string(), 4, 0);
}
impl Eq for Userid {}
impl PartialEq for Userid {
fn eq(&self, rhs: &Self) -> bool {
- self.data == rhs.data && self.name_len == rhs.name_len
+ self.data == rhs.data
+ && self.name_len == rhs.name_len
+ && self.token_len == rhs.token_len
}
}
@@ -352,7 +517,23 @@ impl From<(&UsernameRef, &RealmRef)> for Userid {
fn from(parts: (&UsernameRef, &RealmRef)) -> Self {
let data = format!("{}@{}", parts.0.as_str(), parts.1.as_str());
let name_len = parts.0.as_str().len();
- Self { data, name_len }
+ let token_len = 0;
+ Self { data, name_len, token_len }
+ }
+}
+
+impl From<(Username, Realm, Tokenname)> for Userid {
+ fn from(parts: (Username, Realm, Tokenname)) -> Self {
+ Self::from((parts.0.as_ref(), parts.1.as_ref(), parts.2.as_ref()))
+ }
+}
+
+impl From<(&UsernameRef, &RealmRef, &TokennameRef)> for Userid {
+ fn from(parts: (&UsernameRef, &RealmRef, &TokennameRef)) -> Self {
+ let data = format!("{}@{}!{}", parts.0.as_str(), parts.1.as_str(), parts.2.as_str());
+ let name_len = parts.0.as_str().len();
+ let token_len = parts.2.as_str().len();
+ Self { data, name_len, token_len }
}
}
@@ -366,15 +547,42 @@ 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 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 name = &id[..name_len];
+ let realm = &id[(name_len + 1)..realm_end];
+ 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"))?;
- Ok(Self::from((UsernameRef::new(name), RealmRef::new(realm))))
+ if id.len() > realm_end {
+ let token = &id[(realm_end + 1)..];
+ if !PROXMOX_TOKEN_NAME_REGEX.is_match(token) {
+ bail!("invalid token name in user id");
+ }
+ Ok(Self::from((UsernameRef::new(name), RealmRef::new(realm), TokennameRef::new(token))))
+ } else {
+ Ok(Self::from((UsernameRef::new(name), RealmRef::new(realm))))
+ }
}
}
@@ -388,10 +596,34 @@ impl TryFrom<String> for Userid {
.rposition(|&b| b == b'@')
.ok_or_else(|| format_err!("not a valid user id"))?;
- PROXMOX_AUTH_REALM_STRING_SCHEMA.check_constraints(&data[(name_len + 1)..])
+ 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");
+ }
+
+ 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)..realm_end])
.map_err(|_| format_err!("invalid realm in user id"))?;
- Ok(Self { data, name_len })
+ let token_len = if realm_end == data.len() {
+ 0
+ } else {
+ if !PROXMOX_TOKEN_NAME_REGEX.is_match(&data[(realm_end + 1)..]) {
+ bail!("invalid token name in user id");
+ }
+ data.len() - realm_end - 1
+ };
+
+ Ok(Self { data, name_len, token_len })
}
}
@@ -413,5 +645,50 @@ impl PartialEq<String> for Userid {
}
}
+#[test]
+fn test_token_id() {
+ use std::str::FromStr;
+ use std::convert::TryFrom;
+
+ let userid = Userid::new("test@pam!bar".to_string(), 4, 3);
+ assert_eq!(userid.name().as_str(), "test");
+ assert_eq!(userid.realm(), "pam");
+ assert_eq!(userid.tokenname().expect("token should return tokenname").as_str(), "bar");
+ assert_eq!(userid, "test@pam!bar");
+
+ assert_eq!(userid, Userid::from_str("test@pam!bar").expect("parsing token from str failed"));
+ assert_eq!(userid, Userid::try_from("test@pam!bar".to_string()).expect("parsing token from String failed"));
+
+ let userid = Userid::new("test@pam".to_string(), 4, 0);
+ assert_eq!(userid.name().as_str(), "test");
+ assert_eq!(userid.realm(), "pam");
+ // replace with .expect_none once that is stable
+ assert_eq!(userid.tokenname().is_none(), true);
+ assert_eq!(userid, "test@pam");
+
+ assert_eq!(userid, Userid::from_str("test@pam").expect("parsing user from str failed"));
+ assert_eq!(userid, Userid::try_from("test@pam".to_string()).expect("parsing user from String failed"));
+
+ Userid::from_str("test@pam!bar@baz").expect("username with @ and ! failed");
+ Userid::try_from("test@pam!bar@baz".to_string()).expect("username with @ and ! failed");
+
+ Userid::from_str("test@pam!").expect_err("empty token should fail");
+ Userid::from_str("t€st@pam").expect("strange chars in username allowed");
+ Userid::from_str("tes/@pam").expect_err("slash in username not allowed");
+ Userid::from_str("tes:@pam").expect_err("colon in username not allowed");
+ Userid::from_str("tes\n@pam").expect_err("newline in username not allowed");
+ Userid::from_str("tes\0@pam").expect_err("\0 in username not allowed");
+ Userid::from_str("test@¶am").expect_err("strange chars in realm not allowed");
+
+ let userid = Userid::new("test@pam".to_string(), 4, 0);
+ let (name, realm) = (userid.name(), userid.realm());
+ assert_eq!(userid, Userid::from((name, realm)));
+
+ let userid = Userid::new("test@pam!test".to_string(), 4, 4);
+ let (name, realm, tokenname) = (userid.name(), userid.realm(), userid.tokenname().expect("tokenid should return tokennameref"));
+ assert_eq!(userid, Userid::from((name, realm, tokenname)));
+
+}
+
proxmox::forward_deserialize_to_from_str!(Userid);
proxmox::forward_serialize_to_display!(Userid);
diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs
index ffe5b3dd..1fbbca09 100644
--- a/src/bin/proxmox-backup-client.rs
+++ b/src/bin/proxmox-backup-client.rs
@@ -425,7 +425,7 @@ async fn list_backup_groups(param: Value) -> Result<Value, Error> {
description: "Backup group.",
},
"new-owner": {
- type: Userid,
+ schema: PROXMOX_USER_OR_TOKEN_ID_SCHEMA,
},
}
}
diff --git a/src/config/remote.rs b/src/config/remote.rs
index 9e597342..ac6079ac 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,
+ schema: PROXMOX_USER_OR_TOKEN_ID_SCHEMA,
},
password: {
schema: REMOTE_PASSWORD_SCHEMA,
diff --git a/src/config/user.rs b/src/config/user.rs
index b72fa40b..efb346d8 100644
--- a/src/config/user.rs
+++ b/src/config/user.rs
@@ -56,7 +56,7 @@ pub const EMAIL_SCHEMA: Schema = StringSchema::new("E-Mail Address.")
#[api(
properties: {
userid: {
- type: Userid,
+ schema: PROXMOX_USER_ID_SCHEMA,
},
comment: {
optional: true,
@@ -109,7 +109,7 @@ fn init() -> SectionConfig {
};
let plugin = SectionConfigPlugin::new("user".to_string(), Some("userid".to_string()), obj_schema);
- let mut config = SectionConfig::new(&Userid::API_SCHEMA);
+ let mut config = SectionConfig::new(&PROXMOX_USER_ID_SCHEMA);
config.register_plugin(plugin);
--
2.20.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [pbs-devel] [RFC proxmox-backup 05/15] add ApiToken to user.cfg and CachedUserInfo
2020-10-19 7:39 [pbs-devel] [RFC proxmox-backup 00/15] API tokens Fabian Grünbichler
` (3 preceding siblings ...)
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 04/15] Userid: extend schema with token name Fabian Grünbichler
@ 2020-10-19 7:39 ` Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 06/15] config: add token.shadow file Fabian Grünbichler
` (9 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Fabian Grünbichler @ 2020-10-19 7:39 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
src/bin/proxmox-backup-client.rs | 4 +-
src/bin/proxmox_backup_manager/acl.rs | 2 +-
src/config/cached_user_info.rs | 48 +++++++++++++++----
src/config/user.rs | 67 ++++++++++++++++++++++++---
4 files changed, 102 insertions(+), 19 deletions(-)
diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs
index 1fbbca09..bf336243 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,
@@ -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/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/config/cached_user_info.rs b/src/config/cached_user_info.rs
index cf9c534d..93f360c8 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 super::user::{ApiToken, User};
use crate::api2::types::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,20 +57,44 @@ impl CachedUserInfo {
Ok(config)
}
- /// Test if a user account is enabled and not expired
+ /// Test if a userid is enabled and not expired
pub fn is_active_user(&self, userid: &Userid) -> bool {
- if let Ok(info) = self.user_cfg.lookup::<User>("user", userid.as_str()) {
- if !info.enable.unwrap_or(true) {
+ if userid.is_tokenid() {
+ if let Ok(owner) = userid.owner() {
+ if !self.is_active_user(&owner) {
+ return false;
+ }
+ } else {
return false;
}
- if let Some(expire) = info.expire {
- if expire > 0 && expire <= now() {
+
+ if let Ok(info) = self.user_cfg.lookup::<ApiToken>("token", userid.as_str()) {
+ 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;
} else {
- return false;
+ if let Ok(info) = self.user_cfg.lookup::<User>("user", userid.as_str()) {
+ 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;
+ }
}
}
@@ -116,6 +140,12 @@ impl CachedUserInfo {
privs |= role_privs;
}
}
+
+ if userid.is_tokenid() {
+ // limit privs to that of owning user
+ privs &= self.lookup_privs(&userid.owner().unwrap(), path);
+ }
+
privs
}
}
diff --git a/src/config/user.rs b/src/config/user.rs
index efb346d8..5ebb9f99 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: 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>,
+}
#[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(&PROXMOX_USER_OR_TOKEN_ID_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(&PROXMOX_USER_ID_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
}
@@ -208,7 +244,24 @@ pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
// shell completion helper
pub fn complete_user_name(_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_userid(_arg: &str, _param: &HashMap<String, String>) -> Vec<String> {
+ match config() {
+ Ok((data, _digest)) => data.sections.iter().map(|(id, _)| id.to_string()).collect(),
+ Err(_) => vec![],
+ }
+}
--
2.20.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [pbs-devel] [RFC proxmox-backup 06/15] config: add token.shadow file
2020-10-19 7:39 [pbs-devel] [RFC proxmox-backup 00/15] API tokens Fabian Grünbichler
` (4 preceding siblings ...)
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 05/15] add ApiToken to user.cfg and CachedUserInfo Fabian Grünbichler
@ 2020-10-19 7:39 ` Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 07/15] REST: extract and handle API tokens Fabian Grünbichler
` (8 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Fabian Grünbichler @ 2020-10-19 7:39 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>
---
Notes:
we could also do a simple
tokenid:crypt(tokensecret)
but then we'd need to be careful w.r.t. tokenid characters and quoting and whatnot..
src/config.rs | 1 +
src/config/token_shadow.rs | 79 ++++++++++++++++++++++++++++++++++++++
2 files changed, 80 insertions(+)
create mode 100644 src/config/token_shadow.rs
diff --git a/src/config.rs b/src/config.rs
index c2ac6da1..a70740e1 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -23,6 +23,7 @@ pub mod network;
pub mod remote;
pub mod sync;
pub mod user;
+pub mod token_shadow;
/// Check configuration directory permissions
///
diff --git a/src/config/token_shadow.rs b/src/config/token_shadow.rs
new file mode 100644
index 00000000..afec6135
--- /dev/null
+++ b/src/config/token_shadow.rs
@@ -0,0 +1,79 @@
+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::Userid;
+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: Userid,
+ pub secret: String,
+}
+
+fn read_file() -> Result<HashMap<Userid, 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<Userid, 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: &Userid, secret: &str) -> Result<(), Error> {
+ 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: &Userid, secret: &str) -> Result<(), Error> {
+ 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: &Userid) -> Result<(), Error> {
+ 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] 23+ messages in thread
* [pbs-devel] [RFC proxmox-backup 07/15] REST: extract and handle API tokens
2020-10-19 7:39 [pbs-devel] [RFC proxmox-backup 00/15] API tokens Fabian Grünbichler
` (5 preceding siblings ...)
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 06/15] config: add token.shadow file Fabian Grünbichler
@ 2020-10-19 7:39 ` Fabian Grünbichler
2020-10-20 8:34 ` Wolfgang Bumiller
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 08/15] api: add API token endpoints Fabian Grünbichler
` (7 subsequent siblings)
14 siblings, 1 reply; 23+ messages in thread
From: Fabian Grünbichler @ 2020-10-19 7:39 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
Notes:
exact format up for discussion, obviously. we probably want some sort of prefix
like with PVE
src/server/rest.rs | 63 +++++++++++++++++++++++++++++++---------------
1 file changed, 43 insertions(+), 20 deletions(-)
diff --git a/src/server/rest.rs b/src/server/rest.rs
index c650a3aa..a87b1e75 100644
--- a/src/server/rest.rs
+++ b/src/server/rest.rs
@@ -3,6 +3,7 @@ use std::future::Future;
use std::hash::BuildHasher;
use std::path::{Path, PathBuf};
use std::pin::Pin;
+use std::str::FromStr;
use std::sync::{Arc, Mutex};
use std::task::{Context, Poll};
@@ -531,7 +532,7 @@ 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_auth_data(headers: &http::HeaderMap) -> (Option<String>, Option<String>, Option<String>, Option<String>) {
let mut ticket = None;
let mut language = None;
@@ -542,37 +543,59 @@ fn extract_auth_data(headers: &http::HeaderMap) -> (Option<String>, Option<Strin
}
}
+ let api_token = match headers.get("AUTHORIZATION").map(|v| v.to_str()) {
+ Some(Ok(v)) => Some(v.to_owned()),
+ _ => None,
+ };
+
let csrf_token = match headers.get("CSRFPreventionToken").map(|v| v.to_str()) {
Some(Ok(v)) => Some(v.to_owned()),
_ => None,
};
- (ticket, csrf_token, language)
+ (ticket, csrf_token, language, api_token)
}
fn check_auth(
method: &hyper::Method,
ticket: &Option<String>,
csrf_token: &Option<String>,
+ api_token: &Option<String>,
user_info: &CachedUserInfo,
) -> Result<Userid, Error> {
- let ticket_lifetime = tools::ticket::TICKET_LIFETIME;
+ let userid = if let Some(ticket) = ticket {
+ 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 ticket = ticket.as_str();
+ let userid: Userid = Ticket::parse(ticket)?
+ .verify_with_time_frame(public_auth_key(), "PBS", None, -300..ticket_lifetime)?;
- if !user_info.is_active_user(&userid) {
- bail!("user account disabled or expired.");
- }
+ if !user_info.is_active_user(&userid) {
+ 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) = csrf_token {
+ verify_csrf_prevention_token(csrf_secret(), &userid, &csrf_token, -300, ticket_lifetime)?;
+ } else {
+ bail!("missing CSRF prevention token");
+ }
}
- }
+ userid
+ } else if let Some(api_token) = 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 = Userid::from_str(tokenid)?;
+
+ let tokensecret = parts.next()
+ .ok_or_else(|| format_err!("failed to split API token header"))?;
+ crate::config::token_shadow::verify_secret(&tokenid, &tokensecret)?;
+
+ tokenid
+ } else {
+ bail!("neither ticket nor API token provided - unauthorized");
+ };
Ok(userid)
}
@@ -630,8 +653,8 @@ 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 (ticket, csrf_token, _, api_token) = extract_auth_data(&parts.headers);
+ match check_auth(&method, &ticket, &csrf_token, &api_token, &user_info) {
Ok(userid) => rpcenv.set_user(Some(userid.to_string())),
Err(err) => {
// always delay unauthorized calls by 3 seconds (from start of request)
@@ -684,9 +707,9 @@ 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) {
+ let (ticket, csrf_token, language, api_token) = extract_auth_data(&parts.headers);
+ if ticket != None || api_token != None {
+ match check_auth(&method, &ticket, &csrf_token, &api_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));
--
2.20.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [pbs-devel] [RFC proxmox-backup 08/15] api: add API token endpoints
2020-10-19 7:39 [pbs-devel] [RFC proxmox-backup 00/15] API tokens Fabian Grünbichler
` (6 preceding siblings ...)
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 07/15] REST: extract and handle API tokens Fabian Grünbichler
@ 2020-10-19 7:39 ` Fabian Grünbichler
2020-10-20 9:42 ` Wolfgang Bumiller
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 09/15] api: allow listing users + tokens Fabian Grünbichler
` (6 subsequent siblings)
14 siblings, 1 reply; 23+ messages in thread
From: Fabian Grünbichler @ 2020-10-19 7:39 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 | 327 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 324 insertions(+), 3 deletions(-)
diff --git a/src/api2/access/user.rs b/src/api2/access/user.rs
index 6c292c2d..4197cf60 100644
--- a/src/api2/access/user.rs
+++ b/src/api2/access/user.rs
@@ -1,12 +1,15 @@
use anyhow::{bail, Error};
use serde_json::Value;
+use std::convert::TryFrom;
use proxmox::api::{api, ApiMethod, Router, RpcEnvironment, Permission};
+use proxmox::api::router::SubdirMap;
use proxmox::api::schema::{Schema, StringSchema};
use proxmox::tools::fs::open_file_locked;
use crate::api2::types::*;
use crate::config::user;
+use crate::config::token_shadow;
use crate::config::acl::{PRIV_SYS_AUDIT, PRIV_PERMISSIONS_MODIFY};
use crate::config::cached_user_info::CachedUserInfo;
@@ -304,12 +307,330 @@ pub fn delete_user(userid: Userid, digest: Option<String>) -> Result<(), Error>
Ok(())
}
-const ITEM_ROUTER: Router = Router::new()
+#[api(
+ input: {
+ properties: {
+ userid: {
+ schema: PROXMOX_USER_ID_SCHEMA,
+ },
+ tokenname: {
+ schema: PROXMOX_TOKEN_NAME_SCHEMA,
+ },
+ },
+ },
+ returns: {
+ description: "Get API token metadata (with config digest).",
+ type: user::ApiToken,
+ },
+ access: {
+ permission: &Permission::Or(&[
+ &Permission::Privilege(&["access", "users"], PRIV_SYS_AUDIT, false),
+ &Permission::UserParam("userid"),
+ ]),
+ },
+)]
+/// Read user's API token metadata
+pub fn read_token(
+ userid: Userid,
+ tokenname: String,
+ _info: &ApiMethod,
+ mut rpcenv: &mut dyn RpcEnvironment,
+) -> Result<user::ApiToken, Error> {
+
+ let (config, digest) = user::config()?;
+
+ let tokenname = Tokenname::try_from(tokenname)?;
+
+ let tokenid = Userid::from((userid.name(), userid.realm(), tokenname.as_ref()));
+
+ rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into();
+ config.lookup("token", tokenid.as_str())
+}
+
+#[api(
+ protected: true,
+ input: {
+ properties: {
+ userid: {
+ schema: PROXMOX_USER_ID_SCHEMA,
+ },
+ tokenname: {
+ schema: PROXMOX_TOKEN_NAME_SCHEMA,
+ },
+ comment: {
+ optional: true,
+ schema: SINGLE_LINE_COMMENT_SCHEMA,
+ },
+ enable: {
+ schema: user::ENABLE_USER_SCHEMA,
+ optional: true,
+ },
+ expire: {
+ schema: user::EXPIRE_USER_SCHEMA,
+ optional: true,
+ },
+ digest: {
+ optional: true,
+ schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
+ },
+ },
+ },
+ access: {
+ permission: &Permission::Or(&[
+ &Permission::Privilege(&["access", "users"], PRIV_PERMISSIONS_MODIFY, false),
+ &Permission::UserParam("userid"),
+ ]),
+ },
+ returns: {
+ description: "Secret API token value",
+ type: String,
+ },
+)]
+/// Generate a new API token with given metadata
+pub fn generate_token(
+ userid: Userid,
+ tokenname: String,
+ comment: Option<String>,
+ enable: Option<bool>,
+ expire: Option<i64>,
+ digest: Option<String>,
+) -> Result<String, Error> {
+
+ let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+
+ let (mut config, expected_digest) = user::config()?;
+
+ if let Some(ref digest) = digest {
+ let digest = proxmox::tools::hex_to_digest(digest)?;
+ crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?;
+ }
+
+ let tokenname = Tokenname::try_from(tokenname)?;
+ let tokenid = Userid::from((userid.name(), userid.realm(), tokenname.as_ref()));
+
+ if let Some(_) = config.sections.get(tokenid.as_str()) {
+ bail!("token '{}' for user '{}' already exists.", tokenname.as_str(), userid);
+ }
+
+ let secret = format!("{:x}", proxmox::tools::uuid::Uuid::generate());
+ token_shadow::set_secret(&tokenid, &secret)?;
+
+ let token = user::ApiToken {
+ tokenid: tokenid.clone(),
+ comment,
+ enable,
+ expire,
+ };
+
+ config.set_data(tokenid.as_str(), "token", &token)?;
+
+ user::save_config(&config)?;
+
+ Ok(secret)
+}
+
+#[api(
+ protected: true,
+ input: {
+ properties: {
+ userid: {
+ schema: PROXMOX_USER_ID_SCHEMA,
+ },
+ tokenname: {
+ schema: PROXMOX_TOKEN_NAME_SCHEMA,
+ },
+ comment: {
+ optional: true,
+ schema: SINGLE_LINE_COMMENT_SCHEMA,
+ },
+ enable: {
+ schema: user::ENABLE_USER_SCHEMA,
+ optional: true,
+ },
+ expire: {
+ schema: user::EXPIRE_USER_SCHEMA,
+ optional: true,
+ },
+ digest: {
+ optional: true,
+ schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
+ },
+ },
+ },
+ access: {
+ permission: &Permission::Or(&[
+ &Permission::Privilege(&["access", "users"], PRIV_PERMISSIONS_MODIFY, false),
+ &Permission::UserParam("userid"),
+ ]),
+ },
+)]
+/// Update user's API token metadata
+pub fn update_token(
+ userid: Userid,
+ tokenname: String,
+ comment: Option<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 = Userid::from((userid.name(), userid.realm(), tokenname.as_ref()));
+
+ let mut data: user::ApiToken = config.lookup("token", tokenid.as_str())?;
+
+ if let Some(comment) = comment {
+ let comment = comment.trim().to_string();
+ if comment.is_empty() {
+ data.comment = None;
+ } else {
+ data.comment = Some(comment);
+ }
+ }
+
+ if let Some(enable) = enable {
+ data.enable = if enable { None } else { Some(false) };
+ }
+
+ if let Some(expire) = expire {
+ data.expire = if expire > 0 { Some(expire) } else { None };
+ }
+
+ config.set_data(tokenid.as_str(), "token", &data)?;
+
+ user::save_config(&config)?;
+
+ Ok(())
+}
+
+#[api(
+ protected: true,
+ input: {
+ properties: {
+ userid: {
+ schema: PROXMOX_USER_ID_SCHEMA,
+ },
+ tokenname: {
+ schema: PROXMOX_TOKEN_NAME_SCHEMA,
+ },
+ digest: {
+ optional: true,
+ schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
+ },
+ },
+ },
+ access: {
+ permission: &Permission::Or(&[
+ &Permission::Privilege(&["access", "users"], PRIV_PERMISSIONS_MODIFY, false),
+ &Permission::UserParam("userid"),
+ ]),
+ },
+)]
+/// Delete a user's API token
+pub fn delete_token(
+ userid: Userid,
+ tokenname: String,
+ digest: Option<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 = Userid::from((userid.name(), userid.realm(), tokenname.as_ref()));
+
+ match config.sections.get(tokenid.as_str()) {
+ Some(_) => { config.sections.remove(tokenid.as_str()); },
+ None => bail!("token '{}' of user '{}' does not exist.", tokenname.as_str(), userid),
+ }
+
+ token_shadow::delete_secret(&tokenid)?;
+
+ user::save_config(&config)?;
+
+ Ok(())
+}
+
+#[api(
+ input: {
+ properties: {
+ userid: {
+ schema: PROXMOX_USER_ID_SCHEMA,
+ },
+ },
+ },
+ returns: {
+ description: "List user's API tokens (with config digest).",
+ type: Array,
+ items: { type: user::ApiToken },
+ },
+ access: {
+ permission: &Permission::Or(&[
+ &Permission::Privilege(&["access", "users"], PRIV_SYS_AUDIT, false),
+ &Permission::UserParam("userid"),
+ ]),
+ },
+)]
+/// List user's API tokens
+pub fn list_tokens(
+ userid: Userid,
+ _info: &ApiMethod,
+ mut rpcenv: &mut dyn RpcEnvironment,
+) -> Result<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 let Ok(owner) = token.tokenid.owner() {
+ owner == userid
+ } else {
+ false
+ }
+ };
+
+ Ok(list.into_iter().filter(filter_by_owner).collect())
+}
+
+const TOKEN_ITEM_ROUTER: Router = Router::new()
+ .get(&API_METHOD_READ_TOKEN)
+ .put(&API_METHOD_UPDATE_TOKEN)
+ .post(&API_METHOD_GENERATE_TOKEN)
+ .delete(&API_METHOD_DELETE_TOKEN);
+
+const TOKEN_ROUTER: Router = Router::new()
+ .get(&API_METHOD_LIST_TOKENS)
+ .match_all("tokenname", &TOKEN_ITEM_ROUTER);
+
+const USER_SUBDIRS: SubdirMap = &[
+ ("token", &TOKEN_ROUTER),
+];
+
+const USER_ROUTER: Router = Router::new()
.get(&API_METHOD_READ_USER)
.put(&API_METHOD_UPDATE_USER)
- .delete(&API_METHOD_DELETE_USER);
+ .delete(&API_METHOD_DELETE_USER)
+ .subdirs(USER_SUBDIRS);
pub const ROUTER: Router = Router::new()
.get(&API_METHOD_LIST_USERS)
.post(&API_METHOD_CREATE_USER)
- .match_all("userid", &ITEM_ROUTER);
+ .match_all("userid", &USER_ROUTER);
--
2.20.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [pbs-devel] [RFC proxmox-backup 09/15] api: allow listing users + tokens
2020-10-19 7:39 [pbs-devel] [RFC proxmox-backup 00/15] API tokens Fabian Grünbichler
` (7 preceding siblings ...)
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 08/15] api: add API token endpoints Fabian Grünbichler
@ 2020-10-19 7:39 ` Fabian Grünbichler
2020-10-20 10:10 ` Wolfgang Bumiller
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 10/15] api: add permissions endpoint Fabian Grünbichler
` (5 subsequent siblings)
14 siblings, 1 reply; 23+ messages in thread
From: Fabian Grünbichler @ 2020-10-19 7:39 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>
---
Notes:
returning the full tokenid is a difference compared to PVE
src/api2/access/user.rs | 115 ++++++++++++++++++++++++++++++++++++++--
1 file changed, 111 insertions(+), 4 deletions(-)
diff --git a/src/api2/access/user.rs b/src/api2/access/user.rs
index 4197cf60..8a019e53 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::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: {
+ schema: PROXMOX_USER_ID_SCHEMA,
+ },
+ 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()?;
@@ -52,11 +136,34 @@ 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 let Ok(owner) = token.tokenid.owner() {
+ map.entry(owner).or_insert_with(|| Vec::new()).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] 23+ messages in thread
* [pbs-devel] [RFC proxmox-backup 10/15] api: add permissions endpoint
2020-10-19 7:39 [pbs-devel] [RFC proxmox-backup 00/15] API tokens Fabian Grünbichler
` (8 preceding siblings ...)
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 09/15] api: allow listing users + tokens Fabian Grünbichler
@ 2020-10-19 7:39 ` Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 11/15] client: allow using ApiToken + secret Fabian Grünbichler
` (4 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Fabian Grünbichler @ 2020-10-19 7:39 UTC (permalink / raw)
To: pbs-devel
and adapt privilege calculation to return propagate flag
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
Notes:
not too happy with the paths addition to AclTree. could probably be solved with
some recursive function..
this is compatible with the permissions API call in PVE, except that true is encoded as 1 there.
src/api2/access.rs | 95 +++++++++++++++++++++++++++++++++-
src/config/acl.rs | 67 +++++++++++++-----------
src/config/cached_user_info.rs | 19 +++++--
3 files changed, 146 insertions(+), 35 deletions(-)
diff --git a/src/api2/access.rs b/src/api2/access.rs
index 0c19dab6..c898fd7d 100644
--- a/src/api2/access.rs
+++ b/src/api2/access.rs
@@ -1,6 +1,9 @@
use anyhow::{bail, format_err, Error};
use serde_json::{json, Value};
+use std::collections::HashMap;
+use std::collections::HashSet;
+use std::convert::TryFrom;
use proxmox::api::{api, RpcEnvironment, Permission};
use proxmox::api::router::{Router, SubdirMap};
@@ -12,8 +15,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;
@@ -237,6 +241,91 @@ fn change_password(
Ok(Value::Null)
}
+#[api(
+ input: {
+ properties: {
+ userid: {
+ schema: PROXMOX_USER_OR_TOKEN_ID_SCHEMA,
+ optional: true,
+ },
+ path: {
+ schema: ACL_PATH_SCHEMA,
+ optional: true,
+ },
+ },
+ },
+ access: {
+ permission: &Permission::Or(&[
+ &Permission::Privilege(&["access"], PRIV_SYS_AUDIT, false),
+ &Permission::UserParam("userid"),
+ ]),
+ },
+ 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(
+ userid: Option<Userid>,
+ path: Option<String>,
+ rpcenv: &dyn RpcEnvironment,
+) -> Result<HashMap<String, HashMap<String, bool>>, Error> {
+ let user = match userid {
+ Some(user) => user,
+ None => Userid::try_from(rpcenv.get_user().unwrap())?
+ };
+
+ let user_info = CachedUserInfo::new()?;
+
+ let paths = match path {
+ Some(path) => {
+ let mut paths = HashSet::new();
+ paths.insert(path);
+ paths
+ },
+ None => {
+ let mut paths = HashSet::new();
+
+ // 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());
+
+ let (acl_tree, _) = acl_config::config()?;
+ paths.extend(acl_tree.paths);
+
+ paths
+ },
+ };
+
+ let map = paths.into_iter().fold(HashMap::new(), |mut map: HashMap<String, HashMap<String, bool>>, path: String| {
+ let (privs, propagated_privs) = user_info.lookup_privs_details(&user, &acl_config::split_acl_path(path.as_str()));
+
+ let priv_map = match privs {
+ 0 => HashMap::new(),
+ _ => 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),
@@ -244,6 +333,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 39f9d030..b162b875 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;
@@ -228,6 +228,7 @@ pub fn check_acl_path(path: &str) -> Result<(), Error> {
pub struct AclTree {
pub root: AclTreeNode,
+ pub paths: Vec<String>,
}
pub struct AclTreeNode {
@@ -246,7 +247,7 @@ impl AclTreeNode {
}
}
- pub fn extract_roles(&self, user: &Userid, all: bool) -> HashSet<String> {
+ pub fn extract_roles(&self, user: &Userid, all: bool) -> HashMap<String, bool> {
let user_roles = self.extract_user_roles(user, all);
if !user_roles.is_empty() {
// user privs always override group privs
@@ -256,33 +257,33 @@ impl AclTreeNode {
self.extract_group_roles(user, all)
}
- pub fn extract_user_roles(&self, user: &Userid, all: bool) -> HashSet<String> {
+ pub fn extract_user_roles(&self, user: &Userid, all: bool) -> HashMap<String, bool> {
- let mut set = HashSet::new();
+ let mut map = HashMap::new();
let roles = match self.users.get(user) {
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 +292,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 +347,10 @@ impl AclTreeNode {
impl AclTree {
pub fn new() -> Self {
- Self { root: AclTreeNode::new() }
+ Self {
+ root: AclTreeNode::new(),
+ paths: Vec::new(),
+ }
}
pub fn find_node(&mut self, path: &str) -> Option<&mut AclTreeNode> {
@@ -512,7 +516,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();
@@ -533,6 +538,8 @@ impl AclTree {
}
}
+ self.paths.push(path_str.to_string());
+
Ok(())
}
@@ -576,25 +583,25 @@ impl AclTree {
Ok(tree)
}
- pub fn roles(&self, userid: &Userid, path: &[&str]) -> HashSet<String> {
+ pub fn roles(&self, userid: &Userid, path: &[&str]) -> HashMap<String, bool> {
let mut node = &self.root;
- let mut role_set = node.extract_roles(userid, path.is_empty());
+ let mut role_map = node.extract_roles(userid, 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(userid, last_comp);
- if !new_set.is_empty() {
- // overwrite previous settings
- role_set = new_set;
+ let new_map = node.extract_roles(userid, last_comp);
+ if !new_map.is_empty() {
+ // overwrite previous maptings
+ role_map = new_map;
}
}
- role_set
+ role_map
}
}
@@ -686,7 +693,7 @@ mod test {
let path_vec = super::split_acl_path(path);
let mut roles = tree.roles(user, &path_vec)
- .iter().map(|v| v.clone()).collect::<Vec<String>>();
+ .iter().map(|(role, _)| role.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 93f360c8..99e96e67 100644
--- a/src/config/cached_user_info.rs
+++ b/src/config/cached_user_info.rs
@@ -128,26 +128,37 @@ impl CachedUserInfo {
}
pub fn lookup_privs(&self, userid: &Userid, path: &[&str]) -> u64 {
+ let (privs, _) = self.lookup_privs_details(userid, path);
+ privs
+ }
+ pub fn lookup_privs_details(&self, userid: &Userid, path: &[&str]) -> (u64, u64) {
if self.is_superuser(userid) {
- return ROLE_ADMIN;
+ return (ROLE_ADMIN, ROLE_ADMIN);
}
let roles = self.acl_tree.roles(userid, 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;
}
}
if userid.is_tokenid() {
// limit privs to that of owning user
- privs &= self.lookup_privs(&userid.owner().unwrap(), path);
+ let (owner_privs, owner_propagated_privs) = self.lookup_privs_details(&userid.owner().unwrap(), 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] 23+ messages in thread
* [pbs-devel] [RFC proxmox-backup 11/15] client: allow using ApiToken + secret
2020-10-19 7:39 [pbs-devel] [RFC proxmox-backup 00/15] API tokens Fabian Grünbichler
` (9 preceding siblings ...)
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 10/15] api: add permissions endpoint Fabian Grünbichler
@ 2020-10-19 7:39 ` Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 12/15] owner checks: handle backups owned by API tokens Fabian Grünbichler
` (3 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Fabian Grünbichler @ 2020-10-19 7:39 UTC (permalink / raw)
To: pbs-devel
in place of user + password.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
Notes:
this will need a followup to disentangle user+password/tokenid+secret and host+port from the datastore
src/api2/types/mod.rs | 2 +-
src/client/http_client.rs | 41 +++++++++++++++++++++++++++++----------
2 files changed, 32 insertions(+), 11 deletions(-)
diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs
index 65411f73..67740ca8 100644
--- a/src/api2/types/mod.rs
+++ b/src/api2/types/mod.rs
@@ -66,7 +66,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/client/http_client.rs b/src/client/http_client.rs
index f0f6b9ce..8881d9a0 100644
--- a/src/client/http_client.rs
+++ b/src/client/http_client.rs
@@ -101,7 +101,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,
@@ -375,6 +375,13 @@ impl HttpClient {
}
});
+ let first_auth = if userid.is_tokenid() {
+ // TODO check access here?
+ None
+ } else {
+ Some(BroadcastFuture::new(Box::new(login_future)))
+ };
+
Ok(Self {
client,
server: String::from(server),
@@ -382,7 +389,7 @@ impl HttpClient {
fingerprint: verified_fingerprint,
auth,
ticket_abort,
- first_auth: BroadcastFuture::new(Box::new(login_future)),
+ first_auth,
_options: options,
})
}
@@ -392,7 +399,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())
}
@@ -475,10 +485,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.userid.is_tokenid() {
+ let enc_api_token = format!("{}:{}", auth.userid, 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
}
@@ -577,11 +591,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.userid.is_tokenid() {
+ let enc_api_token = format!("{}:{}", auth.userid, 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?;
--
2.20.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [pbs-devel] [RFC proxmox-backup 12/15] owner checks: handle backups owned by API tokens
2020-10-19 7:39 [pbs-devel] [RFC proxmox-backup 00/15] API tokens Fabian Grünbichler
` (10 preceding siblings ...)
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 11/15] client: allow using ApiToken + secret Fabian Grünbichler
@ 2020-10-19 7:39 ` Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 13/15] tasks: allow unpriv users to read their tokens' tasks Fabian Grünbichler
` (2 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Fabian Grünbichler @ 2020-10-19 7:39 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 | 124 +++++++++++++++++++++++-------------
src/api2/backup.rs | 3 +-
src/api2/reader.rs | 3 +-
3 files changed, 82 insertions(+), 48 deletions(-)
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 5c9902e1..a279d382 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -37,13 +37,29 @@ use crate::config::acl::{
PRIV_DATASTORE_BACKUP,
};
-fn check_backup_owner(
+fn check_priv_or_backup_owner(
store: &DataStore,
group: &BackupGroup,
userid: &Userid,
+ required_privs: u64,
) -> Result<(), Error> {
- let owner = store.get_owner(group)?;
- if &owner != userid {
+ let user_info = CachedUserInfo::new()?;
+ let user_privs = user_info.lookup_privs(&userid, &["datastore", store.name()]);
+
+ if user_privs & required_privs == 0 {
+ let owner = store.get_owner(group)?;
+ check_backup_owner(&owner, userid)?;
+ }
+
+ Ok(())
+}
+
+fn check_backup_owner(
+ owner: &Userid,
+ userid: &Userid,
+) -> Result<(), Error> {
+ let correct_owner = owner == userid || (owner.is_tokenid() && &owner.owner()? == userid);
+ if !correct_owner {
bail!("backup owner check failed ({} != {})", userid, owner);
}
Ok(())
@@ -164,7 +180,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 && check_backup_owner(&owner, &userid).is_err() {
continue;
}
@@ -224,15 +240,11 @@ pub fn list_snapshot_files(
) -> Result<Vec<BackupContent>, Error> {
let userid: Userid = rpcenv.get_user().unwrap().parse()?;
- let user_info = CachedUserInfo::new()?;
- let user_privs = user_info.lookup_privs(&userid, &["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)?; }
+ check_priv_or_backup_owner(&datastore, snapshot.group(), &userid, PRIV_DATASTORE_AUDIT | PRIV_DATASTORE_READ)?;
let info = BackupInfo::new(&datastore.base_path(), snapshot)?;
@@ -276,15 +288,11 @@ fn delete_snapshot(
) -> Result<Value, Error> {
let userid: Userid = rpcenv.get_user().unwrap().parse()?;
- let user_info = CachedUserInfo::new()?;
- let user_privs = user_info.lookup_privs(&userid, &["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)?; }
+ check_priv_or_backup_owner(&datastore, snapshot.group(), &userid, PRIV_DATASTORE_MODIFY)?;
datastore.remove_backup_dir(&snapshot, false)?;
@@ -355,7 +363,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 && check_backup_owner(&owner, &userid).is_err() {
continue;
}
@@ -637,8 +645,6 @@ fn prune(
let backup_id = tools::required_string_param(¶m, "backup-id")?;
let userid: Userid = rpcenv.get_user().unwrap().parse()?;
- let user_info = CachedUserInfo::new()?;
- let user_privs = user_info.lookup_privs(&userid, &["datastore", &store]);
let dry_run = param["dry-run"].as_bool().unwrap_or(false);
@@ -646,8 +652,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)?; }
+ check_priv_or_backup_owner(&datastore, &group, &userid, PRIV_DATASTORE_MODIFY)?;
let prune_options = PruneOptions {
keep_last: param["keep-last"].as_u64(),
@@ -894,8 +899,6 @@ fn download_file(
let datastore = DataStore::lookup_datastore(store)?;
let userid: Userid = rpcenv.get_user().unwrap().parse()?;
- let user_info = CachedUserInfo::new()?;
- let user_privs = user_info.lookup_privs(&userid, &["datastore", &store]);
let file_name = tools::required_string_param(¶m, "file-name")?.to_owned();
@@ -905,8 +908,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)?; }
+ check_priv_or_backup_owner(&datastore, backup_dir.group(), &userid, PRIV_DATASTORE_READ)?;
println!("Download {} from {} ({}/{})", file_name, store, backup_dir, file_name);
@@ -967,8 +969,6 @@ fn download_file_decoded(
let datastore = DataStore::lookup_datastore(store)?;
let userid: Userid = rpcenv.get_user().unwrap().parse()?;
- let user_info = CachedUserInfo::new()?;
- let user_privs = user_info.lookup_privs(&userid, &["datastore", &store]);
let file_name = tools::required_string_param(¶m, "file-name")?.to_owned();
@@ -978,8 +978,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)?; }
+ check_priv_or_backup_owner(&datastore, backup_dir.group(), &userid, PRIV_DATASTORE_READ)?;
let (manifest, files) = read_backup_index(&datastore, &backup_dir)?;
for file in files {
@@ -1092,7 +1091,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 owner = datastore.get_owner(backup_dir.group())?;
+ check_backup_owner(&owner, &userid)?;
let mut path = datastore.base_path();
path.push(backup_dir.relative_path());
@@ -1162,13 +1162,10 @@ fn catalog(
let datastore = DataStore::lookup_datastore(&store)?;
let userid: Userid = rpcenv.get_user().unwrap().parse()?;
- let user_info = CachedUserInfo::new()?;
- let user_privs = user_info.lookup_privs(&userid, &["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)?; }
+ check_priv_or_backup_owner(&datastore, backup_dir.group(), &userid, PRIV_DATASTORE_READ)?;
let file_name = CATALOG_NAME;
@@ -1273,8 +1270,6 @@ fn pxar_file_download(
let datastore = DataStore::lookup_datastore(&store)?;
let userid: Userid = rpcenv.get_user().unwrap().parse()?;
- let user_info = CachedUserInfo::new()?;
- let user_privs = user_info.lookup_privs(&userid, &["datastore", &store]);
let filepath = tools::required_string_param(¶m, "filepath")?.to_owned();
@@ -1284,8 +1279,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)?; }
+ check_priv_or_backup_owner(&datastore, backup_dir.group(), &userid, PRIV_DATASTORE_READ)?;
let mut components = base64::decode(&filepath)?;
if components.len() > 0 && components[0] == '/' as u8 {
@@ -1420,13 +1414,10 @@ fn get_notes(
let datastore = DataStore::lookup_datastore(&store)?;
let userid: Userid = rpcenv.get_user().unwrap().parse()?;
- let user_info = CachedUserInfo::new()?;
- let user_privs = user_info.lookup_privs(&userid, &["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)?; }
+ check_priv_or_backup_owner(&datastore, backup_dir.group(), &userid, PRIV_DATASTORE_READ)?;
let (manifest, _) = datastore.load_manifest(&backup_dir)?;
@@ -1473,13 +1464,10 @@ fn set_notes(
let datastore = DataStore::lookup_datastore(&store)?;
let userid: Userid = rpcenv.get_user().unwrap().parse()?;
- let user_info = CachedUserInfo::new()?;
- let user_privs = user_info.lookup_privs(&userid, &["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)?; }
+ check_priv_or_backup_owner(&datastore, backup_dir.group(), &userid, PRIV_DATASTORE_READ)?;
datastore.update_manifest(&backup_dir,|manifest| {
manifest.unprotected["notes"] = notes.into();
@@ -1506,7 +1494,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
@@ -1515,15 +1504,58 @@ fn set_backup_owner(
backup_type: String,
backup_id: String,
new_owner: Userid,
- _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 userid:Userid = rpcenv.get_user().unwrap().parse()?;
+
let user_info = CachedUserInfo::new()?;
+ let user_privs = user_info.lookup_privs(&userid, &["datastore", &store]);
+
+ let allowed = if (user_privs & PRIV_DATASTORE_MODIFY) != 0 {
+ // High-privilege user/token
+ true
+ } else if (user_privs & PRIV_DATASTORE_BACKUP) != 0 {
+ let owner = datastore.get_owner(&backup_group)?;
+
+ match (owner.is_tokenid(), new_owner.is_tokenid()) {
+ (true, true) => {
+ // API token to API token, owned by same user
+ let owner = owner.owner().unwrap();
+ let new_owner = new_owner.owner().unwrap();
+ owner == new_owner && owner == userid
+ },
+ (true, false) => {
+ // API token to API token owner
+ owner.owner().unwrap() == userid && new_owner == userid
+ },
+ (false, true) => {
+ // API token owner to API token
+ owner == userid && new_owner.owner().unwrap() == userid
+ },
+ (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 {}",
+ userid,
+ backup_group,
+ new_owner,
+ ));
+ }
+
if !user_info.is_active_user(&new_owner) {
bail!("user '{}' is inactive or non-existent", new_owner);
}
diff --git a/src/api2/backup.rs b/src/api2/backup.rs
index 8b2b0e1a..17da0558 100644
--- a/src/api2/backup.rs
+++ b/src/api2/backup.rs
@@ -108,7 +108,8 @@ async move {
let (owner, _group_guard) = datastore.create_locked_backup_group(&backup_group, &userid)?;
// permission check
- if owner != userid && worker_type != "benchmark" {
+ let correct_owner = owner == userid || (owner.is_tokenid() && owner.owner()? == userid);
+ if !correct_owner && worker_type != "benchmark" {
// only the owner is allowed to create additional snapshots
bail!("backup owner check failed ({} != {})", userid, owner);
}
diff --git a/src/api2/reader.rs b/src/api2/reader.rs
index 8f47047e..7ea066c4 100644
--- a/src/api2/reader.rs
+++ b/src/api2/reader.rs
@@ -94,7 +94,8 @@ 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 {
+ let correct_owner = owner == userid || (owner.is_tokenid() && owner.owner()? == userid);
+ if !correct_owner {
bail!("backup owner check failed!");
}
}
--
2.20.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [pbs-devel] [RFC proxmox-backup 13/15] tasks: allow unpriv users to read their tokens' tasks
2020-10-19 7:39 [pbs-devel] [RFC proxmox-backup 00/15] API tokens Fabian Grünbichler
` (11 preceding siblings ...)
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 12/15] owner checks: handle backups owned by API tokens Fabian Grünbichler
@ 2020-10-19 7:39 ` Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 14/15] manager: add token commands Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 15/15] manager: add user permissions command Fabian Grünbichler
14 siblings, 0 replies; 23+ messages in thread
From: Fabian Grünbichler @ 2020-10-19 7:39 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 | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/src/api2/node/tasks.rs b/src/api2/node/tasks.rs
index b86eaec7..379ec6fc 100644
--- a/src/api2/node/tasks.rs
+++ b/src/api2/node/tasks.rs
@@ -14,6 +14,14 @@ 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_user: &Userid, task_user: &Userid) -> Result<(), Error> {
+ if auth_user == task_user || (task_user.is_tokenid() && &task_user.owner().unwrap() == auth_user) {
+ Ok(())
+ } else {
+ let user_info = CachedUserInfo::new()?;
+ user_info.check_privs(auth_user, &["system", "tasks"], PRIV_SYS_AUDIT, false)
+ }
+}
#[api(
input: {
@@ -57,7 +65,7 @@ use crate::config::cached_user_info::CachedUserInfo;
description: "Worker ID (arbitrary ASCII string)",
},
user: {
- type: String,
+ schema: PROXMOX_USER_OR_TOKEN_ID_SCHEMA,
description: "The user who started the task.",
},
status: {
@@ -85,11 +93,7 @@ async fn get_task_status(
let upid = extract_upid(¶m)?;
let userid: Userid = rpcenv.get_user().unwrap().parse()?;
-
- if userid != upid.userid {
- let user_info = CachedUserInfo::new()?;
- user_info.check_privs(&userid, &["system", "tasks"], PRIV_SYS_AUDIT, false)?;
- }
+ check_task_access(&userid, &upid.userid)?;
let mut result = json!({
"upid": param["upid"],
@@ -162,11 +166,7 @@ async fn read_task_log(
let upid = extract_upid(¶m)?;
let userid: Userid = rpcenv.get_user().unwrap().parse()?;
-
- if userid != upid.userid {
- let user_info = CachedUserInfo::new()?;
- user_info.check_privs(&userid, &["system", "tasks"], PRIV_SYS_AUDIT, false)?;
- }
+ check_task_access(&userid, &upid.userid)?;
let test_status = param["test-status"].as_bool().unwrap_or(false);
@@ -326,7 +326,9 @@ pub fn list_tasks(
Err(_) => return None,
};
- if !list_all && info.upid.userid != userid { return None; }
+ if !list_all && check_task_access(&userid, &info.upid.userid).is_err() {
+ return None;
+ }
if let Some(userid) = &userfilter {
if !info.upid.userid.as_str().contains(userid) { return None; }
--
2.20.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [pbs-devel] [RFC proxmox-backup 14/15] manager: add token commands
2020-10-19 7:39 [pbs-devel] [RFC proxmox-backup 00/15] API tokens Fabian Grünbichler
` (12 preceding siblings ...)
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 13/15] tasks: allow unpriv users to read their tokens' tasks Fabian Grünbichler
@ 2020-10-19 7:39 ` Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 15/15] manager: add user permissions command Fabian Grünbichler
14 siblings, 0 replies; 23+ messages in thread
From: Fabian Grünbichler @ 2020-10-19 7:39 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>
---
Notes:
it might make sense to allow updating ACLs of tokens irrespective of
general ACL editing capabilities of the user, since the effective
permissions of a token are always a subset of that of the user
anyway. otherwise we'd need to implement fine-grained ACL update
checks for no gain to make API tokens usable for regular users.
src/bin/proxmox_backup_manager/user.rs | 62 ++++++++++++++++++++++++++
src/config/user.rs | 29 ++++++++++++
2 files changed, 91 insertions(+)
diff --git a/src/bin/proxmox_backup_manager/user.rs b/src/bin/proxmox_backup_manager/user.rs
index 80dbcb1b..31f74396 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::PROXMOX_USER_ID_SCHEMA;
#[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: {
+ schema: PROXMOX_USER_ID_SCHEMA,
+ }
+ }
+ }
+)]
+/// 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_user_name)
+ )
+ .insert(
+ "list-tokens",
+ CliCommand::new(&&API_METHOD_LIST_TOKENS)
+ .arg_param(&["userid"])
+ .completion_cb("userid", config::user::complete_user_name)
+ )
+ .insert(
+ "generate-token",
+ CliCommand::new(&api2::access::user::API_METHOD_GENERATE_TOKEN)
+ .arg_param(&["userid", "tokenname"])
+ .completion_cb("userid", config::user::complete_user_name)
+ )
+ .insert(
+ "delete-token",
+ CliCommand::new(&api2::access::user::API_METHOD_DELETE_TOKEN)
+ .arg_param(&["userid", "tokenname"])
+ .completion_cb("userid", config::user::complete_user_name)
+ .completion_cb("tokenname", config::user::complete_token_name)
);
cmd_def.into()
diff --git a/src/config/user.rs b/src/config/user.rs
index 5ebb9f99..2684a529 100644
--- a/src/config/user.rs
+++ b/src/config/user.rs
@@ -265,3 +265,32 @@ pub fn complete_userid(_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> {
+ match config() {
+ Ok((data, _digest)) => {
+ match param.get("userid") {
+ Some(userid) => {
+ match (data.lookup::<User>("user", userid), data.convert_to_typed_array("token")) {
+ (Ok(_), Ok(tokens)) => {
+ tokens
+ .into_iter()
+ .filter_map(|token:ApiToken| {
+ let tokenid = token.tokenid;
+ if tokenid.is_tokenid() && &tokenid.owner().unwrap() == userid {
+ Some(tokenid.tokenname().unwrap().as_str().to_string())
+ } else {
+ None
+ }
+ }).collect()
+ },
+ _ => vec![],
+ }
+ },
+ None => vec![],
+ }
+ },
+ Err(_) => vec![],
+ }
+}
--
2.20.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [pbs-devel] [RFC proxmox-backup 15/15] manager: add user permissions command
2020-10-19 7:39 [pbs-devel] [RFC proxmox-backup 00/15] API tokens Fabian Grünbichler
` (13 preceding siblings ...)
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 14/15] manager: add token commands Fabian Grünbichler
@ 2020-10-19 7:39 ` Fabian Grünbichler
14 siblings, 0 replies; 23+ messages in thread
From: Fabian Grünbichler @ 2020-10-19 7:39 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 31f74396..f4aa4220 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::PROXMOX_USER_ID_SCHEMA;
+use proxmox_backup::api2::types::{ACL_PATH_SCHEMA, PROXMOX_USER_OR_TOKEN_ID_SCHEMA, PROXMOX_USER_ID_SCHEMA};
#[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,
+ },
+ userid: {
+ schema: PROXMOX_USER_OR_TOKEN_ID_SCHEMA,
+ },
+ 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_user_name)
.completion_cb("tokenname", config::user::complete_token_name)
+ )
+ .insert(
+ "permissions",
+ CliCommand::new(&&API_METHOD_LIST_PERMISSIONS)
+ .arg_param(&["userid"])
+ .completion_cb("userid", config::user::complete_userid)
+ .completion_cb("path", config::datastore::complete_acl_path)
);
cmd_def.into()
--
2.20.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [pbs-devel] applied: [PATCH proxmox-backup 01/15] fix indentation
2020-10-19 7:39 ` [pbs-devel] [PATCH proxmox-backup 01/15] fix indentation Fabian Grünbichler
@ 2020-10-19 12:00 ` Thomas Lamprecht
0 siblings, 0 replies; 23+ messages in thread
From: Thomas Lamprecht @ 2020-10-19 12:00 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Fabian Grünbichler
On 19.10.20 09:39, Fabian Grünbichler wrote:
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> src/api2/admin/datastore.rs | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
>
applied, thanks!
^ permalink raw reply [flat|nested] 23+ messages in thread
* [pbs-devel] applied: [PATCH proxmox-backup 02/15] fix typos
2020-10-19 7:39 ` [pbs-devel] [PATCH proxmox-backup 02/15] fix typos Fabian Grünbichler
@ 2020-10-19 12:01 ` Thomas Lamprecht
0 siblings, 0 replies; 23+ messages in thread
From: Thomas Lamprecht @ 2020-10-19 12:01 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Fabian Grünbichler
On 19.10.20 09:39, Fabian Grünbichler wrote:
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> src/api2/node/tasks.rs | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>
applied, thanks!
^ permalink raw reply [flat|nested] 23+ messages in thread
* [pbs-devel] applied: [PATCH proxmox-backup 03/15] REST: rename token to csrf_token
2020-10-19 7:39 ` [pbs-devel] [PATCH proxmox-backup 03/15] REST: rename token to csrf_token Fabian Grünbichler
@ 2020-10-19 12:02 ` Thomas Lamprecht
0 siblings, 0 replies; 23+ messages in thread
From: Thomas Lamprecht @ 2020-10-19 12:02 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Fabian Grünbichler
On 19.10.20 09:39, Fabian Grünbichler wrote:
> for easier differentiation with (future) api_token
>
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> src/server/rest.rs | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
>
applied, thanks!
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [pbs-devel] [RFC proxmox-backup 07/15] REST: extract and handle API tokens
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 07/15] REST: extract and handle API tokens Fabian Grünbichler
@ 2020-10-20 8:34 ` Wolfgang Bumiller
0 siblings, 0 replies; 23+ messages in thread
From: Wolfgang Bumiller @ 2020-10-20 8:34 UTC (permalink / raw)
To: Fabian Grünbichler; +Cc: pbs-devel
On Mon, Oct 19, 2020 at 09:39:11AM +0200, Fabian Grünbichler wrote:
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
>
> Notes:
> exact format up for discussion, obviously. we probably want some sort of prefix
> like with PVE
>
> src/server/rest.rs | 63 +++++++++++++++++++++++++++++++---------------
> 1 file changed, 43 insertions(+), 20 deletions(-)
>
> diff --git a/src/server/rest.rs b/src/server/rest.rs
> index c650a3aa..a87b1e75 100644
> --- a/src/server/rest.rs
> +++ b/src/server/rest.rs
> @@ -3,6 +3,7 @@ use std::future::Future;
> use std::hash::BuildHasher;
> use std::path::{Path, PathBuf};
> use std::pin::Pin;
> +use std::str::FromStr;
> use std::sync::{Arc, Mutex};
> use std::task::{Context, Poll};
>
> @@ -531,7 +532,7 @@ 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_auth_data(headers: &http::HeaderMap) -> (Option<String>, Option<String>, Option<String>, Option<String>) {
Please turn this return type into a struct.
>
> let mut ticket = None;
> let mut language = None;
> @@ -542,37 +543,59 @@ fn extract_auth_data(headers: &http::HeaderMap) -> (Option<String>, Option<Strin
> }
> }
>
> + let api_token = match headers.get("AUTHORIZATION").map(|v| v.to_str()) {
> + Some(Ok(v)) => Some(v.to_owned()),
> + _ => None,
> + };
> +
> let csrf_token = match headers.get("CSRFPreventionToken").map(|v| v.to_str()) {
> Some(Ok(v)) => Some(v.to_owned()),
> _ => None,
> };
>
> - (ticket, csrf_token, language)
> + (ticket, csrf_token, language, api_token)
> }
>
> fn check_auth(
> method: &hyper::Method,
> ticket: &Option<String>,
> csrf_token: &Option<String>,
> + api_token: &Option<String>,
> user_info: &CachedUserInfo,
> ) -> Result<Userid, Error> {
> - let ticket_lifetime = tools::ticket::TICKET_LIFETIME;
> + let userid = if let Some(ticket) = ticket {
> + 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 ticket = ticket.as_str();
> + let userid: Userid = Ticket::parse(ticket)?
> + .verify_with_time_frame(public_auth_key(), "PBS", None, -300..ticket_lifetime)?;
>
> - if !user_info.is_active_user(&userid) {
> - bail!("user account disabled or expired.");
> - }
> + if !user_info.is_active_user(&userid) {
> + 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) = csrf_token {
> + verify_csrf_prevention_token(csrf_secret(), &userid, &csrf_token, -300, ticket_lifetime)?;
> + } else {
> + bail!("missing CSRF prevention token");
> + }
> }
> - }
> + userid
> + } else if let Some(api_token) = 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 = Userid::from_str(tokenid)?;
> +
> + let tokensecret = parts.next()
> + .ok_or_else(|| format_err!("failed to split API token header"))?;
> + crate::config::token_shadow::verify_secret(&tokenid, &tokensecret)?;
> +
> + tokenid
> + } else {
> + bail!("neither ticket nor API token provided - unauthorized");
> + };
>
> Ok(userid)
> }
> @@ -630,8 +653,8 @@ 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 (ticket, csrf_token, _, api_token) = extract_auth_data(&parts.headers);
> + match check_auth(&method, &ticket, &csrf_token, &api_token, &user_info) {
> Ok(userid) => rpcenv.set_user(Some(userid.to_string())),
> Err(err) => {
> // always delay unauthorized calls by 3 seconds (from start of request)
> @@ -684,9 +707,9 @@ 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) {
> + let (ticket, csrf_token, language, api_token) = extract_auth_data(&parts.headers);
> + if ticket != None || api_token != None {
> + match check_auth(&method, &ticket, &csrf_token, &api_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));
> --
> 2.20.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [pbs-devel] [RFC proxmox-backup 08/15] api: add API token endpoints
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 08/15] api: add API token endpoints Fabian Grünbichler
@ 2020-10-20 9:42 ` Wolfgang Bumiller
2020-10-20 10:15 ` Wolfgang Bumiller
0 siblings, 1 reply; 23+ messages in thread
From: Wolfgang Bumiller @ 2020-10-20 9:42 UTC (permalink / raw)
To: Fabian Grünbichler; +Cc: pbs-devel
On Mon, Oct 19, 2020 at 09:39:12AM +0200, Fabian Grünbichler wrote:
> beneath the user endpoint.
>
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> src/api2/access/user.rs | 327 +++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 324 insertions(+), 3 deletions(-)
>
> diff --git a/src/api2/access/user.rs b/src/api2/access/user.rs
> index 6c292c2d..4197cf60 100644
> --- a/src/api2/access/user.rs
> +++ b/src/api2/access/user.rs
> @@ -1,12 +1,15 @@
> use anyhow::{bail, Error};
> use serde_json::Value;
> +use std::convert::TryFrom;
>
> use proxmox::api::{api, ApiMethod, Router, RpcEnvironment, Permission};
> +use proxmox::api::router::SubdirMap;
> use proxmox::api::schema::{Schema, StringSchema};
> use proxmox::tools::fs::open_file_locked;
>
> use crate::api2::types::*;
> use crate::config::user;
> +use crate::config::token_shadow;
> use crate::config::acl::{PRIV_SYS_AUDIT, PRIV_PERMISSIONS_MODIFY};
> use crate::config::cached_user_info::CachedUserInfo;
>
> @@ -304,12 +307,330 @@ pub fn delete_user(userid: Userid, digest: Option<String>) -> Result<(), Error>
> Ok(())
> }
>
> -const ITEM_ROUTER: Router = Router::new()
> +#[api(
> + input: {
> + properties: {
> + userid: {
> + schema: PROXMOX_USER_ID_SCHEMA,
> + },
> + tokenname: {
> + schema: PROXMOX_TOKEN_NAME_SCHEMA,
> + },
> + },
> + },
> + returns: {
> + description: "Get API token metadata (with config digest).",
> + type: user::ApiToken,
> + },
> + access: {
> + permission: &Permission::Or(&[
> + &Permission::Privilege(&["access", "users"], PRIV_SYS_AUDIT, false),
> + &Permission::UserParam("userid"),
> + ]),
> + },
> +)]
> +/// Read user's API token metadata
> +pub fn read_token(
> + userid: Userid,
> + tokenname: String,
> + _info: &ApiMethod,
> + mut rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<user::ApiToken, Error> {
> +
> + let (config, digest) = user::config()?;
> +
> + let tokenname = Tokenname::try_from(tokenname)?;
> +
> + let tokenid = Userid::from((userid.name(), userid.realm(), tokenname.as_ref()));
> +
> + rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into();
> + config.lookup("token", tokenid.as_str())
> +}
> +
> +#[api(
> + protected: true,
> + input: {
> + properties: {
> + userid: {
> + schema: PROXMOX_USER_ID_SCHEMA,
> + },
> + tokenname: {
> + schema: PROXMOX_TOKEN_NAME_SCHEMA,
> + },
> + comment: {
> + optional: true,
> + schema: SINGLE_LINE_COMMENT_SCHEMA,
> + },
> + enable: {
> + schema: user::ENABLE_USER_SCHEMA,
> + optional: true,
> + },
> + expire: {
> + schema: user::EXPIRE_USER_SCHEMA,
> + optional: true,
> + },
> + digest: {
> + optional: true,
> + schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
> + },
> + },
> + },
> + access: {
> + permission: &Permission::Or(&[
> + &Permission::Privilege(&["access", "users"], PRIV_PERMISSIONS_MODIFY, false),
> + &Permission::UserParam("userid"),
> + ]),
> + },
> + returns: {
> + description: "Secret API token value",
> + type: String,
> + },
> +)]
> +/// Generate a new API token with given metadata
> +pub fn generate_token(
> + userid: Userid,
> + tokenname: String,
> + comment: Option<String>,
> + enable: Option<bool>,
> + expire: Option<i64>,
> + digest: Option<String>,
> +) -> Result<String, Error> {
> +
> + let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
> +
> + let (mut config, expected_digest) = user::config()?;
> +
> + if let Some(ref digest) = digest {
> + let digest = proxmox::tools::hex_to_digest(digest)?;
> + crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?;
> + }
> +
> + let tokenname = Tokenname::try_from(tokenname)?;
> + let tokenid = Userid::from((userid.name(), userid.realm(), tokenname.as_ref()));
> +
> + if let Some(_) = config.sections.get(tokenid.as_str()) {
> + bail!("token '{}' for user '{}' already exists.", tokenname.as_str(), userid);
> + }
> +
> + let secret = format!("{:x}", proxmox::tools::uuid::Uuid::generate());
> + token_shadow::set_secret(&tokenid, &secret)?;
> +
> + let token = user::ApiToken {
> + tokenid: tokenid.clone(),
> + comment,
> + enable,
> + expire,
> + };
> +
> + config.set_data(tokenid.as_str(), "token", &token)?;
> +
> + user::save_config(&config)?;
> +
> + Ok(secret)
> +}
> +
> +#[api(
> + protected: true,
> + input: {
> + properties: {
> + userid: {
> + schema: PROXMOX_USER_ID_SCHEMA,
> + },
> + tokenname: {
> + schema: PROXMOX_TOKEN_NAME_SCHEMA,
> + },
> + comment: {
> + optional: true,
> + schema: SINGLE_LINE_COMMENT_SCHEMA,
> + },
> + enable: {
> + schema: user::ENABLE_USER_SCHEMA,
> + optional: true,
> + },
> + expire: {
> + schema: user::EXPIRE_USER_SCHEMA,
> + optional: true,
> + },
> + digest: {
> + optional: true,
> + schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
> + },
> + },
> + },
> + access: {
> + permission: &Permission::Or(&[
> + &Permission::Privilege(&["access", "users"], PRIV_PERMISSIONS_MODIFY, false),
> + &Permission::UserParam("userid"),
> + ]),
> + },
> +)]
> +/// Update user's API token metadata
> +pub fn update_token(
> + userid: Userid,
> + tokenname: String,
> + comment: Option<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 = Userid::from((userid.name(), userid.realm(), tokenname.as_ref()));
> +
> + let mut data: user::ApiToken = config.lookup("token", tokenid.as_str())?;
> +
> + if let Some(comment) = comment {
> + let comment = comment.trim().to_string();
> + if comment.is_empty() {
> + data.comment = None;
> + } else {
> + data.comment = Some(comment);
> + }
> + }
> +
> + if let Some(enable) = enable {
> + data.enable = if enable { None } else { Some(false) };
Really not a fan of single line if/else like this. Also with the `if
let` together this isn't actually "fast" to read.
How about:
data.enabled = match enable {
Some(true) => None,
other => other,
}
or
data.enabled = enable.filter(|&b| !b);
> + }
> +
> + if let Some(expire) = expire {
> + data.expire = if expire > 0 { Some(expire) } else { None };
> + }
Similarly:
data.expire = expire.filter(|&e| e > 0)
or a match with a conditional arm:
data.expire = match expire {
Some(x) if x > 0 => Some(x),
_ => None,
}
I find those much more readable than nesting conditions.
> +
> + config.set_data(tokenid.as_str(), "token", &data)?;
> +
> + user::save_config(&config)?;
> +
> + Ok(())
> +}
> +
> +#[api(
> + protected: true,
> + input: {
> + properties: {
> + userid: {
> + schema: PROXMOX_USER_ID_SCHEMA,
> + },
> + tokenname: {
> + schema: PROXMOX_TOKEN_NAME_SCHEMA,
> + },
> + digest: {
> + optional: true,
> + schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
> + },
> + },
> + },
> + access: {
> + permission: &Permission::Or(&[
> + &Permission::Privilege(&["access", "users"], PRIV_PERMISSIONS_MODIFY, false),
> + &Permission::UserParam("userid"),
> + ]),
> + },
> +)]
> +/// Delete a user's API token
> +pub fn delete_token(
> + userid: Userid,
> + tokenname: String,
> + digest: Option<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 = Userid::from((userid.name(), userid.realm(), tokenname.as_ref()));
> +
> + match config.sections.get(tokenid.as_str()) {
> + Some(_) => { config.sections.remove(tokenid.as_str()); },
> + None => bail!("token '{}' of user '{}' does not exist.", tokenname.as_str(), userid),
> + }
> +
> + token_shadow::delete_secret(&tokenid)?;
> +
> + user::save_config(&config)?;
> +
> + Ok(())
> +}
> +
> +#[api(
> + input: {
> + properties: {
> + userid: {
> + schema: PROXMOX_USER_ID_SCHEMA,
> + },
> + },
> + },
> + returns: {
> + description: "List user's API tokens (with config digest).",
> + type: Array,
> + items: { type: user::ApiToken },
> + },
> + access: {
> + permission: &Permission::Or(&[
> + &Permission::Privilege(&["access", "users"], PRIV_SYS_AUDIT, false),
> + &Permission::UserParam("userid"),
> + ]),
> + },
> +)]
> +/// List user's API tokens
> +pub fn list_tokens(
> + userid: Userid,
> + _info: &ApiMethod,
> + mut rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<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 let Ok(owner) = token.tokenid.owner() {
> + owner == userid
> + } else {
> + false
> + }
> + };
> +
> + Ok(list.into_iter().filter(filter_by_owner).collect())
> +}
> +
> +const TOKEN_ITEM_ROUTER: Router = Router::new()
> + .get(&API_METHOD_READ_TOKEN)
> + .put(&API_METHOD_UPDATE_TOKEN)
> + .post(&API_METHOD_GENERATE_TOKEN)
> + .delete(&API_METHOD_DELETE_TOKEN);
> +
> +const TOKEN_ROUTER: Router = Router::new()
> + .get(&API_METHOD_LIST_TOKENS)
> + .match_all("tokenname", &TOKEN_ITEM_ROUTER);
> +
> +const USER_SUBDIRS: SubdirMap = &[
> + ("token", &TOKEN_ROUTER),
> +];
> +
> +const USER_ROUTER: Router = Router::new()
> .get(&API_METHOD_READ_USER)
> .put(&API_METHOD_UPDATE_USER)
> - .delete(&API_METHOD_DELETE_USER);
> + .delete(&API_METHOD_DELETE_USER)
> + .subdirs(USER_SUBDIRS);
>
> pub const ROUTER: Router = Router::new()
> .get(&API_METHOD_LIST_USERS)
> .post(&API_METHOD_CREATE_USER)
> - .match_all("userid", &ITEM_ROUTER);
> + .match_all("userid", &USER_ROUTER);
> --
> 2.20.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [pbs-devel] [RFC proxmox-backup 09/15] api: allow listing users + tokens
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 09/15] api: allow listing users + tokens Fabian Grünbichler
@ 2020-10-20 10:10 ` Wolfgang Bumiller
0 siblings, 0 replies; 23+ messages in thread
From: Wolfgang Bumiller @ 2020-10-20 10:10 UTC (permalink / raw)
To: Fabian Grünbichler; +Cc: pbs-devel
On Mon, Oct 19, 2020 at 09:39:13AM +0200, Fabian Grünbichler wrote:
> 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>
> ---
>
> Notes:
> returning the full tokenid is a difference compared to PVE
>
> src/api2/access/user.rs | 115 ++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 111 insertions(+), 4 deletions(-)
>
> diff --git a/src/api2/access/user.rs b/src/api2/access/user.rs
> index 4197cf60..8a019e53 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::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: {
> + schema: PROXMOX_USER_ID_SCHEMA,
> + },
> + 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()?;
>
> @@ -52,11 +136,34 @@ 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")?;
missing space after ':' ;-)
> + let mut user_to_tokens = tokens.into_iter()
> + .fold(HashMap::new(), |mut map: HashMap<Userid, Vec<user::ApiToken>>, token: user::ApiToken| {
too long (and .into_iter() should be on a new line)
> + if let Ok(owner) = token.tokenid.owner() {
> + map.entry(owner).or_insert_with(|| Vec::new()).push(token);
The `or_insert_with(...)` could be `or_default()`
> + }
> + 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] 23+ messages in thread
* Re: [pbs-devel] [RFC proxmox-backup 08/15] api: add API token endpoints
2020-10-20 9:42 ` Wolfgang Bumiller
@ 2020-10-20 10:15 ` Wolfgang Bumiller
0 siblings, 0 replies; 23+ messages in thread
From: Wolfgang Bumiller @ 2020-10-20 10:15 UTC (permalink / raw)
To: Fabian Grünbichler; +Cc: pbs-devel
On Tue, Oct 20, 2020 at 11:42:22AM +0200, Wolfgang Bumiller wrote:
> On Mon, Oct 19, 2020 at 09:39:12AM +0200, Fabian Grünbichler wrote:
> > + let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
> > +
> > + let (mut config, expected_digest) = user::config()?;
> > +
> > + if let Some(ref digest) = digest {
> > + let digest = proxmox::tools::hex_to_digest(digest)?;
> > + crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?;
> > + }
> > +
> > + let tokenname = Tokenname::try_from(tokenname)?;
> > + let tokenid = Userid::from((userid.name(), userid.realm(), tokenname.as_ref()));
> > +
> > + let mut data: user::ApiToken = config.lookup("token", tokenid.as_str())?;
> > +
> > + if let Some(comment) = comment {
> > + let comment = comment.trim().to_string();
> > + if comment.is_empty() {
> > + data.comment = None;
> > + } else {
> > + data.comment = Some(comment);
> > + }
> > + }
> > +
> > + if let Some(enable) = enable {
> > + data.enable = if enable { None } else { Some(false) };
>
> Really not a fan of single line if/else like this. Also with the `if
> let` together this isn't actually "fast" to read.
> How about:
>
> data.enabled = match enable {
> Some(true) => None,
> other => other,
> }
>
> or
>
> data.enabled = enable.filter(|&b| !b);
I missed that there was no `else` case which should leave it unchanged,
so only the `match` version really makes sense, but with explicit
`None => data.enabled`, maybe include comments:
data.enabled = match enable {
Some(true) => None, // enabled is default
Some(false) => Some(false),
None => data.enabled, // unchanged.
}
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2020-10-20 10:16 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-19 7:39 [pbs-devel] [RFC proxmox-backup 00/15] API tokens Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [PATCH proxmox-backup 01/15] fix indentation Fabian Grünbichler
2020-10-19 12:00 ` [pbs-devel] applied: " Thomas Lamprecht
2020-10-19 7:39 ` [pbs-devel] [PATCH proxmox-backup 02/15] fix typos Fabian Grünbichler
2020-10-19 12:01 ` [pbs-devel] applied: " Thomas Lamprecht
2020-10-19 7:39 ` [pbs-devel] [PATCH proxmox-backup 03/15] REST: rename token to csrf_token Fabian Grünbichler
2020-10-19 12:02 ` [pbs-devel] applied: " Thomas Lamprecht
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 04/15] Userid: extend schema with token name Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 05/15] add ApiToken to user.cfg and CachedUserInfo Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 06/15] config: add token.shadow file Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 07/15] REST: extract and handle API tokens Fabian Grünbichler
2020-10-20 8:34 ` Wolfgang Bumiller
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 08/15] api: add API token endpoints Fabian Grünbichler
2020-10-20 9:42 ` Wolfgang Bumiller
2020-10-20 10:15 ` Wolfgang Bumiller
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 09/15] api: allow listing users + tokens Fabian Grünbichler
2020-10-20 10:10 ` Wolfgang Bumiller
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 10/15] api: add permissions endpoint Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 11/15] client: allow using ApiToken + secret Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 12/15] owner checks: handle backups owned by API tokens Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 13/15] tasks: allow unpriv users to read their tokens' tasks Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 14/15] manager: add token commands Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 15/15] manager: add user permissions command 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