[pbs-devel] [PATCH proxmox-backup v3 00/10] OpenID connect realms

[pbs-devel] [PATCH proxmox-backup v3 00/10] OpenID connect realms

[Dietmar Maurer]

This implements OpenID connect realms using the new
"proxmox-openid-rs" crate.

Note: The new src/tools/memcom.rs is from Wolfgang.

Changes since v2:
   fix CachedUserInfo by using a shared memory version counter

Changes since v1:
- really fix commit message of first patch
- change api endpoints (/access/openid/{login|auth-url})
- merged all api implementation patches

Changes since preview version (for Fabian):
- fix commit message
- reserve namen 'pam' and 'pbs'
- fix 'make deb'


Dietmar Maurer (10):
  depend on proxmox-openid-rs
  config: new domains.cfg to configure openid realm
  check_acl_path: add /access/domains and /access/openid
  add API to manage openid realms
  cli: add CLI to manage openid realms.
  implement new helper is_active_user_id()
  cleanup user/token is_active() check
  api: add openid redirect/login API
  ui: implement OpenId login
  fix CachedUserInfo by using a shared memory version counter

 Cargo.toml                               |   2 +
 src/api2/access.rs                       |   4 +-
 src/api2/access/domain.rs                |  18 ++
 src/api2/access/openid.rs                | 190 ++++++++++++++++
 src/api2/config/access/mod.rs            |   8 +-
 src/api2/config/access/openid.rs         | 274 +++++++++++++++++++++++
 src/bin/proxmox-backup-manager.rs        |   1 +
 src/bin/proxmox_backup_manager/mod.rs    |   2 +
 src/bin/proxmox_backup_manager/openid.rs |  99 ++++++++
 src/config.rs                            |   1 +
 src/config/acl.rs                        |   8 +-
 src/config/cached_user_info.rs           |  48 ++--
 src/config/domains.rs                    | 173 ++++++++++++++
 src/config/user.rs                       |  38 ++++
 src/tools.rs                             |   3 +
 src/tools/memcom.rs                      | 159 +++++++++++++
 www/Application.js                       |   8 +-
 www/LoginView.js                         | 100 ++++++++-
 www/Utils.js                             |   8 +
 19 files changed, 1115 insertions(+), 29 deletions(-)
 create mode 100644 src/api2/access/openid.rs
 create mode 100644 src/api2/config/access/openid.rs
 create mode 100644 src/bin/proxmox_backup_manager/openid.rs
 create mode 100644 src/config/domains.rs
 create mode 100644 src/tools/memcom.rs

-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v3 01/10] depend on proxmox-openid-rs

[Dietmar Maurer]

---
 Cargo.toml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Cargo.toml b/Cargo.toml
index 976f18bc..adb58c7b 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -84,6 +84,8 @@ crossbeam-channel = "0.5"
 
 proxmox-acme-rs = "0.2.1"
 
+proxmox-openid = "0.6.0"
+
 [features]
 default = []
 #valgrind = ["valgrind_request"]
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v3 02/10] config: new domains.cfg to configure openid realm

[Dietmar Maurer]

Or other realmy types...
---
 src/api2/access/domain.rs |  21 +++++++
 src/config.rs             |   1 +
 src/config/domains.rs     | 120 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 142 insertions(+)
 create mode 100644 src/config/domains.rs

diff --git a/src/api2/access/domain.rs b/src/api2/access/domain.rs
index a2fd746d..2dff9d32 100644
--- a/src/api2/access/domain.rs
+++ b/src/api2/access/domain.rs
@@ -38,10 +38,31 @@ use crate::api2::types::*;
 )]
 /// Authentication domain/realm index.
 fn list_domains() -> Result<Value, Error> {
+
     let mut list = Vec::new();
+
     list.push(json!({ "realm": "pam", "comment": "Linux PAM standard authentication", "default": true }));
     list.push(json!({ "realm": "pbs", "comment": "Proxmox Backup authentication server" }));
+
+    let (config, _digest) = crate::config::domains::config()?;
+
+    for (realm, (section_type, v)) in config.sections.iter() {
+        let mut item = json!({
+            "type": section_type,
+            "realm": realm,
+        });
+
+        if v["comment"].as_str().is_some() {
+            item["comment"] = v["comment"].clone();
+        }
+        list.push(item);
+
+    }
+
     Ok(list.into())
+
+
+
 }
 
 pub const ROUTER: Router = Router::new()
diff --git a/src/config.rs b/src/config.rs
index b9cd6281..329315ec 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -31,6 +31,7 @@ pub mod drive;
 pub mod media_pool;
 pub mod tape_encryption_keys;
 pub mod tape_job;
+pub mod domains;
 
 /// Check configuration directory permissions
 ///
diff --git a/src/config/domains.rs b/src/config/domains.rs
new file mode 100644
index 00000000..703acfe9
--- /dev/null
+++ b/src/config/domains.rs
@@ -0,0 +1,120 @@
+use anyhow::{Error};
+use lazy_static::lazy_static;
+use std::collections::HashMap;
+use serde::{Serialize, Deserialize};
+
+use proxmox::api::{
+    api,
+    schema::*,
+    section_config::{
+        SectionConfig,
+        SectionConfigData,
+        SectionConfigPlugin,
+    }
+};
+
+use proxmox::tools::fs::{
+    open_file_locked,
+    replace_file,
+    CreateOptions,
+};
+
+use crate::api2::types::*;
+
+lazy_static! {
+    pub static ref CONFIG: SectionConfig = init();
+}
+
+
+#[api(
+    properties: {
+        realm: {
+            schema: REALM_ID_SCHEMA,
+        },
+        "issuer-url": {
+            description: "OpenID Issuer Url",
+            type: String,
+        },
+        "client-id": {
+            description: "OpenID Client ID",
+            type: String,
+        },
+        "client-key": {
+            description: "OpenID Client Key",
+            type: String,
+            optional: true,
+        },
+        comment: {
+            optional: true,
+            schema: SINGLE_LINE_COMMENT_SCHEMA,
+        },
+    },
+)]
+#[derive(Serialize,Deserialize)]
+#[serde(rename_all="kebab-case")]
+/// OpenID configuration properties.
+pub struct OpenIdRealmConfig {
+    pub realm: String,
+    pub issuer_url: String,
+    pub client_id: String,
+    #[serde(skip_serializing_if="Option::is_none")]
+    pub client_key: Option<String>,
+    #[serde(skip_serializing_if="Option::is_none")]
+    pub comment: Option<String>,
+}
+
+fn init() -> SectionConfig {
+    let obj_schema = match OpenIdRealmConfig::API_SCHEMA {
+        Schema::Object(ref obj_schema) => obj_schema,
+        _ => unreachable!(),
+    };
+
+    let plugin = SectionConfigPlugin::new("openid".to_string(), Some(String::from("realm")), obj_schema);
+    let mut config = SectionConfig::new(&REALM_ID_SCHEMA);
+    config.register_plugin(plugin);
+
+    config
+}
+
+pub const DOMAINS_CFG_FILENAME: &str = "/etc/proxmox-backup/domains.cfg";
+pub const DOMAINS_CFG_LOCKFILE: &str = "/etc/proxmox-backup/.domains.lck";
+
+/// Get exclusive lock
+pub fn lock_config() -> Result<std::fs::File, Error> {
+    open_file_locked(DOMAINS_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)
+}
+
+pub fn config() -> Result<(SectionConfigData, [u8;32]), Error> {
+
+    let content = proxmox::tools::fs::file_read_optional_string(DOMAINS_CFG_FILENAME)?
+        .unwrap_or_else(|| "".to_string());
+
+    let digest = openssl::sha::sha256(content.as_bytes());
+    let data = CONFIG.parse(DOMAINS_CFG_FILENAME, &content)?;
+    Ok((data, digest))
+}
+
+pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
+    let raw = CONFIG.write(DOMAINS_CFG_FILENAME, &config)?;
+
+    let backup_user = crate::backup::backup_user()?;
+    let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640);
+    // set the correct owner/group/permissions while saving file
+    // owner(rw) = root, group(r)= backup
+    let options = CreateOptions::new()
+        .perm(mode)
+        .owner(nix::unistd::ROOT)
+        .group(backup_user.gid);
+
+    replace_file(DOMAINS_CFG_FILENAME, raw.as_bytes(), options)?;
+
+    Ok(())
+}
+
+// shell completion helper
+pub fn complete_realm_name(_arg: &str, _param: &HashMap<String, String>) -> Vec<String> {
+    match config() {
+        Ok((data, _digest)) => data.sections.iter().map(|(id, _)| id.to_string()).collect(),
+        Err(_) => return vec![],
+    }
+}
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v3 03/10] check_acl_path: add /access/domains and /access/openid

[Dietmar Maurer]

---
 src/config/acl.rs | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/config/acl.rs b/src/config/acl.rs
index 04d42854..e468586e 100644
--- a/src/config/acl.rs
+++ b/src/config/acl.rs
@@ -283,11 +283,17 @@ pub fn check_acl_path(path: &str) -> Result<(), Error> {
                 return Ok(());
             }
             match components[1] {
-                "acl" | "users" => {
+                "acl" | "users" | "domains" => {
                     if components_len == 2 {
                         return Ok(());
                     }
                 }
+                // /access/openid/{endpoint}
+                "openid" => {
+                    if components_len <= 3 {
+                        return Ok(());
+                    }
+                }
                 _ => {}
             }
         }
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v3 04/10] add API to manage openid realms

[Dietmar Maurer]

---
 src/api2/config/access/mod.rs    |   8 +-
 src/api2/config/access/openid.rs | 264 +++++++++++++++++++++++++++++++
 2 files changed, 271 insertions(+), 1 deletion(-)
 create mode 100644 src/api2/config/access/openid.rs

diff --git a/src/api2/config/access/mod.rs b/src/api2/config/access/mod.rs
index 659815e0..6e7d98be 100644
--- a/src/api2/config/access/mod.rs
+++ b/src/api2/config/access/mod.rs
@@ -1,9 +1,15 @@
 use proxmox::api::{Router, SubdirMap};
 use proxmox::list_subdirs_api_method;
+use proxmox::{identity, sortable};
 
 pub mod tfa;
+pub mod openid;
 
-const SUBDIRS: SubdirMap = &[("tfa", &tfa::ROUTER)];
+#[sortable]
+const SUBDIRS: SubdirMap = &sorted!([
+    ("openid", &openid::ROUTER),
+    ("tfa", &tfa::ROUTER),
+]);
 
 pub const ROUTER: Router = Router::new()
     .get(&list_subdirs_api_method!(SUBDIRS))
diff --git a/src/api2/config/access/openid.rs b/src/api2/config/access/openid.rs
new file mode 100644
index 00000000..15fddaf0
--- /dev/null
+++ b/src/api2/config/access/openid.rs
@@ -0,0 +1,264 @@
+/// Configure OpenId realms
+
+use anyhow::{bail, Error};
+use serde_json::Value;
+use ::serde::{Deserialize, Serialize};
+
+use proxmox::api::{api, Permission, Router, RpcEnvironment};
+
+use crate::config::domains::{self, OpenIdRealmConfig};
+use crate::config::acl::{PRIV_SYS_AUDIT, PRIV_PERMISSIONS_MODIFY};
+use crate::api2::types::*;
+
+#[api(
+    input: {
+        properties: {},
+    },
+    returns: {
+        description: "List of configured OpenId realms.",
+        type: Array,
+        items: { type: OpenIdRealmConfig },
+    },
+    access: {
+        permission: &Permission::Privilege(&["access", "domains"], PRIV_SYS_AUDIT, false),
+    },
+)]
+/// List configured OpenId realms
+pub fn list_openid_realms(
+    _param: Value,
+    mut rpcenv: &mut dyn RpcEnvironment,
+) -> Result<Vec<OpenIdRealmConfig>, Error> {
+
+    let (config, digest) = domains::config()?;
+
+    let list = config.convert_to_typed_array("openid")?;
+
+    rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into();
+
+    Ok(list)
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            config: {
+                type: OpenIdRealmConfig,
+                flatten: true,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&["access", "domains"], PRIV_PERMISSIONS_MODIFY, false),
+    },
+)]
+/// Create a new OpenId realm
+pub fn create_openid_realm(config: OpenIdRealmConfig) -> Result<(), Error> {
+
+    let _lock = domains::lock_config()?;
+
+    let (mut domains, _digest) = domains::config()?;
+
+    if config.realm == "pbs" ||
+        config.realm == "pam" ||
+        domains.sections.get(&config.realm).is_some()
+    {
+        bail!("realm '{}' already exists.", config.realm);
+    }
+
+    domains.set_data(&config.realm, "openid", &config)?;
+
+    domains::save_config(&domains)?;
+
+    Ok(())
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            realm: {
+                schema: REALM_ID_SCHEMA,
+            },
+            digest: {
+                optional: true,
+                schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&["access", "domains"], PRIV_PERMISSIONS_MODIFY, false),
+    },
+)]
+/// Remove a OpenID realm configuration
+pub fn delete_openid_realm(
+    realm: String,
+    digest: Option<String>,
+    _rpcenv: &mut dyn RpcEnvironment,
+) -> Result<(), Error> {
+
+    let _lock = domains::lock_config()?;
+
+    let (mut domains, expected_digest) = domains::config()?;
+
+    if let Some(ref digest) = digest {
+        let digest = proxmox::tools::hex_to_digest(digest)?;
+        crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?;
+    }
+
+    if domains.sections.remove(&realm).is_none()  {
+        bail!("realm '{}' does not exist.", realm);
+    }
+
+    domains::save_config(&domains)?;
+
+    Ok(())
+}
+
+#[api(
+    input: {
+        properties: {
+            realm: {
+                schema: REALM_ID_SCHEMA,
+            },
+        },
+    },
+    returns:  { type: OpenIdRealmConfig },
+    access: {
+        permission: &Permission::Privilege(&["access", "domains"], PRIV_SYS_AUDIT, false),
+    },
+)]
+/// Read the OpenID realm configuration
+pub fn read_openid_realm(
+    realm: String,
+    mut rpcenv: &mut dyn RpcEnvironment,
+) -> Result<OpenIdRealmConfig, Error> {
+
+    let (domains, digest) = domains::config()?;
+
+    let config = domains.lookup("openid", &realm)?;
+
+    rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into();
+
+    Ok(config)
+}
+
+#[api()]
+#[derive(Serialize, Deserialize)]
+#[serde(rename_all="kebab-case")]
+#[allow(non_camel_case_types)]
+/// Deletable property name
+pub enum DeletableProperty {
+    /// Delete the client key.
+    client_key,
+    /// Delete the comment property.
+    comment,
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            realm: {
+                schema: REALM_ID_SCHEMA,
+            },
+            "issuer-url": {
+                description: "OpenID Issuer Url",
+                type: String,
+                optional: true,
+            },
+            "client-id": {
+                description: "OpenID Client ID",
+                type: String,
+                optional: true,
+            },
+            "client-key": {
+                description: "OpenID Client Key",
+                type: String,
+                optional: true,
+            },
+            comment: {
+                schema: SINGLE_LINE_COMMENT_SCHEMA,
+                optional: true,
+            },
+            delete: {
+                description: "List of properties to delete.",
+                type: Array,
+                optional: true,
+                items: {
+                    type: DeletableProperty,
+                }
+            },
+            digest: {
+                optional: true,
+                schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
+            },
+        },
+    },
+    returns:  { type: OpenIdRealmConfig },
+    access: {
+        permission: &Permission::Privilege(&["access", "domains"], PRIV_PERMISSIONS_MODIFY, false),
+    },
+)]
+/// Update an OpenID realm configuration
+pub fn update_openid_realm(
+    realm: String,
+    issuer_url: Option<String>,
+    client_id: Option<String>,
+    client_key: Option<String>,
+    comment: Option<String>,
+    delete: Option<Vec<DeletableProperty>>,
+    digest: Option<String>,
+    _rpcenv: &mut dyn RpcEnvironment,
+) -> Result<(), Error> {
+
+    let _lock = domains::lock_config()?;
+
+    let (mut domains, expected_digest) = domains::config()?;
+
+    if let Some(ref digest) = digest {
+        let digest = proxmox::tools::hex_to_digest(digest)?;
+        crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?;
+    }
+
+    let mut config: OpenIdRealmConfig = domains.lookup("openid", &realm)?;
+
+    if let Some(delete) = delete {
+        for delete_prop in delete {
+            match delete_prop {
+                DeletableProperty::client_key => { config.client_key = None; },
+                DeletableProperty::comment => { config.comment = None; },
+            }
+        }
+    }
+
+    if let Some(comment) = comment {
+        let comment = comment.trim().to_string();
+        if comment.is_empty() {
+            config.comment = None;
+        } else {
+            config.comment = Some(comment);
+        }
+    }
+
+    if let Some(issuer_url) = issuer_url { config.issuer_url = issuer_url; }
+    if let Some(client_id) = client_id { config.client_id = client_id; }
+
+    if client_key.is_some() { config.client_key = client_key; }
+
+    domains.set_data(&realm, "openid", &config)?;
+
+    domains::save_config(&domains)?;
+
+    Ok(())
+}
+
+const ITEM_ROUTER: Router = Router::new()
+    .get(&API_METHOD_READ_OPENID_REALM)
+    .put(&API_METHOD_UPDATE_OPENID_REALM)
+    .delete(&API_METHOD_DELETE_OPENID_REALM);
+
+pub const ROUTER: Router = Router::new()
+    .get(&API_METHOD_LIST_OPENID_REALMS)
+    .post(&API_METHOD_CREATE_OPENID_REALM)
+    .match_all("id", &ITEM_ROUTER);
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v3 05/10] cli: add CLI to manage openid realms.

[Dietmar Maurer]

---
 src/bin/proxmox-backup-manager.rs        |  1 +
 src/bin/proxmox_backup_manager/mod.rs    |  2 +
 src/bin/proxmox_backup_manager/openid.rs | 99 ++++++++++++++++++++++++
 src/config/domains.rs                    |  9 +++
 4 files changed, 111 insertions(+)
 create mode 100644 src/bin/proxmox_backup_manager/openid.rs

diff --git a/src/bin/proxmox-backup-manager.rs b/src/bin/proxmox-backup-manager.rs
index c3806a31..461d45bd 100644
--- a/src/bin/proxmox-backup-manager.rs
+++ b/src/bin/proxmox-backup-manager.rs
@@ -354,6 +354,7 @@ fn main() {
         .insert("network", network_commands())
         .insert("node", node_commands())
         .insert("user", user_commands())
+        .insert("openid", openid_commands())
         .insert("remote", remote_commands())
         .insert("garbage-collection", garbage_collection_commands())
         .insert("acme", acme_mgmt_cli())
diff --git a/src/bin/proxmox_backup_manager/mod.rs b/src/bin/proxmox_backup_manager/mod.rs
index 21004bbe..a3a16246 100644
--- a/src/bin/proxmox_backup_manager/mod.rs
+++ b/src/bin/proxmox_backup_manager/mod.rs
@@ -24,3 +24,5 @@ mod disk;
 pub use disk::*;
 mod node;
 pub use node::*;
+mod openid;
+pub use openid::*;
diff --git a/src/bin/proxmox_backup_manager/openid.rs b/src/bin/proxmox_backup_manager/openid.rs
new file mode 100644
index 00000000..13915339
--- /dev/null
+++ b/src/bin/proxmox_backup_manager/openid.rs
@@ -0,0 +1,99 @@
+use anyhow::Error;
+use serde_json::Value;
+
+use proxmox::api::{api, cli::*, RpcEnvironment, ApiHandler};
+
+use proxmox_backup::{config, api2, api2::types::REALM_ID_SCHEMA};
+
+
+#[api(
+    input: {
+        properties: {
+            "output-format": {
+                schema: OUTPUT_FORMAT,
+                optional: true,
+            },
+        }
+    }
+)]
+/// List configured OpenId realms
+fn list_openid_realms(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
+
+    let output_format = get_output_format(&param);
+
+    let info = &api2::config::access::openid::API_METHOD_LIST_OPENID_REALMS;
+    let mut data = match info.handler {
+        ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
+        _ => unreachable!(),
+    };
+
+    let options = default_table_format_options()
+        .column(ColumnConfig::new("realm"))
+        .column(ColumnConfig::new("issuer-url"))
+        .column(ColumnConfig::new("comment"));
+
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
+
+    Ok(Value::Null)
+}
+#[api(
+    input: {
+        properties: {
+            realm: {
+                schema: REALM_ID_SCHEMA,
+            },
+            "output-format": {
+                schema: OUTPUT_FORMAT,
+                optional: true,
+            },
+        }
+    }
+)]
+
+/// Show OpenID realm configuration
+fn show_openid_realm(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
+
+    let output_format = get_output_format(&param);
+
+    let info = &api2::config::access::openid::API_METHOD_READ_OPENID_REALM;
+    let mut data = match info.handler {
+        ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
+        _ => unreachable!(),
+    };
+
+    let options = default_table_format_options();
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
+
+    Ok(Value::Null)
+}
+
+pub fn openid_commands() -> CommandLineInterface {
+
+    let cmd_def = CliCommandMap::new()
+        .insert("list", CliCommand::new(&&API_METHOD_LIST_OPENID_REALMS))
+        .insert("show", CliCommand::new(&&API_METHOD_SHOW_OPENID_REALM)
+                .arg_param(&["realm"])
+                .completion_cb("realm", config::domains::complete_openid_realm_name)
+        )
+        .insert("create",
+                CliCommand::new(&api2::config::access::openid::API_METHOD_CREATE_OPENID_REALM)
+                .arg_param(&["realm"])
+                .arg_param(&["realm"])
+                .completion_cb("realm", config::domains::complete_openid_realm_name)
+        )
+        .insert("update",
+                CliCommand::new(&api2::config::access::openid::API_METHOD_UPDATE_OPENID_REALM)
+                .arg_param(&["realm"])
+                .arg_param(&["realm"])
+                .completion_cb("realm", config::domains::complete_openid_realm_name)
+        )
+        .insert("delete",
+                CliCommand::new(&api2::config::access::openid::API_METHOD_DELETE_OPENID_REALM)
+                .arg_param(&["realm"])
+                .arg_param(&["realm"])
+                .completion_cb("realm", config::domains::complete_openid_realm_name)
+        )
+        ;
+
+       cmd_def.into()
+}
diff --git a/src/config/domains.rs b/src/config/domains.rs
index 703acfe9..3cdd4174 100644
--- a/src/config/domains.rs
+++ b/src/config/domains.rs
@@ -118,3 +118,12 @@ pub fn complete_realm_name(_arg: &str, _param: &HashMap<String, String>) -> Vec<
         Err(_) => return vec![],
     }
 }
+
+pub fn complete_openid_realm_name(_arg: &str, _param: &HashMap<String, String>) -> Vec<String> {
+    match config() {
+        Ok((data, _digest)) => data.sections.iter()
+            .filter_map(|(id, (t, _))| if t == "openid" { Some(id.to_string()) } else { None })
+            .collect(),
+        Err(_) => return vec![],
+    }
+}
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v3 06/10] implement new helper is_active_user_id()

[Dietmar Maurer]

---
 src/config/cached_user_info.rs | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/config/cached_user_info.rs b/src/config/cached_user_info.rs
index c85d643c..a574043f 100644
--- a/src/config/cached_user_info.rs
+++ b/src/config/cached_user_info.rs
@@ -65,10 +65,8 @@ impl CachedUserInfo {
         }
     }
 
-    /// Test if a authentication id is enabled and not expired
-    pub fn is_active_auth_id(&self, auth_id: &Authid) -> bool {
-        let userid = auth_id.user();
-
+    /// Test if a user_id is enabled and not expired
+    pub fn is_active_user_id(&self, userid: &Userid) -> bool {
         if let Ok(info) = self.user_cfg.lookup::<User>("user", userid.as_str()) {
             if !info.enable.unwrap_or(true) {
                 return false;
@@ -78,7 +76,17 @@ impl CachedUserInfo {
                     return false;
                 }
             }
+            true
         } else {
+            false
+        }
+    }
+
+    /// Test if a authentication id is enabled and not expired
+    pub fn is_active_auth_id(&self, auth_id: &Authid) -> bool {
+        let userid = auth_id.user();
+
+        if !self.is_active_user_id(userid) {
             return false;
         }
 
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v3 07/10] cleanup user/token is_active() check

[Dietmar Maurer]

---
 src/config/cached_user_info.rs | 25 ++++---------------------
 src/config/user.rs             | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 21 deletions(-)

diff --git a/src/config/cached_user_info.rs b/src/config/cached_user_info.rs
index a574043f..6cb64162 100644
--- a/src/config/cached_user_info.rs
+++ b/src/config/cached_user_info.rs
@@ -7,6 +7,7 @@ use anyhow::{Error, bail};
 use proxmox::api::section_config::SectionConfigData;
 use lazy_static::lazy_static;
 use proxmox::api::UserInformation;
+use proxmox::tools::time::epoch_i64;
 
 use super::acl::{AclTree, ROLE_NAMES, ROLE_ADMIN};
 use super::user::{ApiToken, User};
@@ -18,8 +19,6 @@ pub struct CachedUserInfo {
     acl_tree: Arc<AclTree>,
 }
 
-fn now() -> i64 { unsafe { libc::time(std::ptr::null_mut()) } }
-
 struct ConfigCache {
     data: Option<Arc<CachedUserInfo>>,
     last_update: i64,
@@ -35,7 +34,7 @@ impl CachedUserInfo {
 
     /// Returns a cached instance (up to 5 seconds old).
     pub fn new() -> Result<Arc<Self>, Error> {
-        let now = now();
+        let now = epoch_i64();
         { // limit scope
             let cache = CACHED_CONFIG.read().unwrap();
             if (now - cache.last_update) < 5 {
@@ -68,15 +67,7 @@ impl CachedUserInfo {
     /// Test if a user_id is enabled and not expired
     pub fn is_active_user_id(&self, userid: &Userid) -> bool {
         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;
-                }
-            }
-            true
+            info.is_active()
         } else {
             false
         }
@@ -92,15 +83,7 @@ impl CachedUserInfo {
 
         if auth_id.is_token() {
             if let Ok(info) = self.user_cfg.lookup::<ApiToken>("token", &auth_id.to_string()) {
-                if !info.enable.unwrap_or(true) {
-                    return false;
-                }
-                if let Some(expire) = info.expire {
-                    if expire > 0 && expire <= now() {
-                        return false;
-                    }
-                }
-                return true;
+                return info.is_active();
             } else {
                 return false;
             }
diff --git a/src/config/user.rs b/src/config/user.rs
index ff7e54e4..28e81876 100644
--- a/src/config/user.rs
+++ b/src/config/user.rs
@@ -83,6 +83,22 @@ pub struct ApiToken {
     pub expire: Option<i64>,
 }
 
+impl ApiToken {
+
+    pub fn is_active(&self) -> bool {
+        if !self.enable.unwrap_or(true) {
+            return false;
+        }
+        if let Some(expire) = self.expire {
+            let now =  proxmox::tools::time::epoch_i64();
+            if expire > 0 && expire <= now {
+                return false;
+            }
+        }
+        true
+    }
+}
+
 #[api(
     properties: {
         userid: {
@@ -132,6 +148,22 @@ pub struct User {
     pub email: Option<String>,
 }
 
+impl User {
+
+    pub fn is_active(&self) -> bool {
+        if !self.enable.unwrap_or(true) {
+            return false;
+        }
+        if let Some(expire) = self.expire {
+            let now =  proxmox::tools::time::epoch_i64();
+            if expire > 0 && expire <= now {
+                return false;
+            }
+        }
+        true
+    }
+}
+
 fn init() -> SectionConfig {
     let mut config = SectionConfig::new(&Authid::API_SCHEMA);
 
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v3 08/10] api: add openid redirect/login API

[Dietmar Maurer]

---
 src/api2/access.rs               |   4 +-
 src/api2/access/domain.rs        |   3 -
 src/api2/access/openid.rs        | 192 +++++++++++++++++++++++++++++++
 src/api2/config/access/openid.rs |  10 ++
 src/config/domains.rs            |  44 +++++++
 5 files changed, 249 insertions(+), 4 deletions(-)
 create mode 100644 src/api2/access/openid.rs

diff --git a/src/api2/access.rs b/src/api2/access.rs
index 46725c97..e5430f62 100644
--- a/src/api2/access.rs
+++ b/src/api2/access.rs
@@ -26,6 +26,7 @@ pub mod domain;
 pub mod role;
 pub mod tfa;
 pub mod user;
+pub mod openid;
 
 #[allow(clippy::large_enum_variant)]
 enum AuthResult {
@@ -335,7 +336,7 @@ pub fn list_permissions(
     let auth_id = match auth_id {
         Some(auth_id) if auth_id == current_auth_id => current_auth_id,
         Some(auth_id) => {
-            if user_privs & PRIV_SYS_AUDIT != 0 
+            if user_privs & PRIV_SYS_AUDIT != 0
                 || (auth_id.is_token()
                     && !current_auth_id.is_token()
                     && auth_id.user() == current_auth_id.user())
@@ -423,6 +424,7 @@ const SUBDIRS: SubdirMap = &sorted!([
         &Router::new().get(&API_METHOD_LIST_PERMISSIONS)
     ),
     ("ticket", &Router::new().post(&API_METHOD_CREATE_TICKET)),
+    ("openid", &openid::ROUTER),
     ("domains", &domain::ROUTER),
     ("roles", &role::ROUTER),
     ("users", &user::ROUTER),
diff --git a/src/api2/access/domain.rs b/src/api2/access/domain.rs
index 2dff9d32..69809acc 100644
--- a/src/api2/access/domain.rs
+++ b/src/api2/access/domain.rs
@@ -60,9 +60,6 @@ fn list_domains() -> Result<Value, Error> {
     }
 
     Ok(list.into())
-
-
-
 }
 
 pub const ROUTER: Router = Router::new()
diff --git a/src/api2/access/openid.rs b/src/api2/access/openid.rs
new file mode 100644
index 00000000..52ec4311
--- /dev/null
+++ b/src/api2/access/openid.rs
@@ -0,0 +1,192 @@
+//! OpenID redirect/login API
+use std::convert::TryFrom;
+
+use anyhow::{bail, Error};
+
+use serde_json::{json, Value};
+
+use proxmox::api::router::{Router, SubdirMap};
+use proxmox::api::{api, Permission, RpcEnvironment};
+use proxmox::{list_subdirs_api_method};
+use proxmox::{identity, sortable};
+use proxmox::tools::fs::open_file_locked;
+
+use proxmox_openid::OpenIdAuthenticator;
+
+use crate::server::ticket::ApiTicket;
+use crate::tools::ticket::Ticket;
+
+use crate::config::domains::{OpenIdUserAttribute, OpenIdRealmConfig};
+use crate::config::cached_user_info::CachedUserInfo;
+
+use crate::api2::types::*;
+use crate::auth_helpers::*;
+
+#[api(
+    input: {
+        properties: {
+            state: {
+                description: "OpenId state.",
+                type: String,
+            },
+            code: {
+                description: "OpenId authorization code.",
+                type: String,
+            },
+            "redirect-url": {
+                description: "Redirection Url. The client should set this to used server url.",
+                type: String,
+            },
+        },
+    },
+    returns: {
+        properties: {
+            username: {
+                type: String,
+                description: "User name.",
+            },
+            ticket: {
+                type: String,
+                description: "Auth ticket.",
+            },
+            CSRFPreventionToken: {
+                type: String,
+                description: "Cross Site Request Forgery Prevention Token.",
+            },
+        },
+    },
+    protected: true,
+    access: {
+        permission: &Permission::World,
+    },
+)]
+/// Verify OpenID authorization code and create a ticket
+///
+/// Returns: An authentication ticket with additional infos.
+pub fn openid_login(
+    state: String,
+    code: String,
+    redirect_url: String,
+    _rpcenv: &mut dyn RpcEnvironment,
+) -> Result<Value, Error> {
+    let user_info = CachedUserInfo::new()?;
+
+    let (realm, private_auth_state) =
+        OpenIdAuthenticator::verify_public_auth_state(PROXMOX_BACKUP_RUN_DIR_M!(), &state)?;
+
+    let (domains, _digest) = crate::config::domains::config()?;
+    let config: OpenIdRealmConfig = domains.lookup("openid", &realm)?;
+
+    let open_id = config.authenticator(&redirect_url)?;
+
+    let info = open_id.verify_authorization_code(&code, &private_auth_state)?;
+
+    // eprintln!("VERIFIED {} {:?} {:?}", info.subject().as_str(), info.name(), info.email());
+
+    let unique_name = match config.user_attr {
+        None | Some(OpenIdUserAttribute::Subject) => info.subject().as_str(),
+        Some(OpenIdUserAttribute::Username) => {
+            match info.preferred_username() {
+                Some(name) => name.as_str(),
+                None => bail!("missing claim 'preferred_name'"),
+            }
+        }
+        Some(OpenIdUserAttribute::Email) => {
+            match info.email() {
+                Some(name) => name.as_str(),
+                None => bail!("missing claim 'email'"),
+            }
+        }
+    };
+
+    let user_id = Userid::try_from(format!("{}@{}", unique_name, realm))?;
+
+    if !user_info.is_active_user_id(&user_id) {
+        if config.autocreate.unwrap_or(false) {
+            use crate::config::user;
+            let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
+            let user = user::User {
+                userid: user_id.clone(),
+                comment: None,
+                enable: None,
+                expire: None,
+                firstname: info.given_name().and_then(|n| n.get(None)).map(|n| n.to_string()),
+                lastname: info.family_name().and_then(|n| n.get(None)).map(|n| n.to_string()),
+                email: info.email().map(|e| e.to_string()),
+            };
+            let (mut config, _digest) = user::config()?;
+            if config.sections.get(user.userid.as_str()).is_some() {
+                bail!("autocreate user failed - '{}' already exists.", user.userid);
+            }
+            config.set_data(user.userid.as_str(), "user", &user)?;
+            user::save_config(&config)?;
+            // fixme: replace sleep with shared memory change notification
+            std::thread::sleep(std::time::Duration::new(6, 0));
+        } else {
+            bail!("user account '{}' missing, disabled or expired.", user_id);
+        }
+    }
+
+    let api_ticket = ApiTicket::full(user_id.clone());
+    let ticket = Ticket::new("PBS", &api_ticket)?.sign(private_auth_key(), None)?;
+    let token = assemble_csrf_prevention_token(csrf_secret(), &user_id);
+
+    crate::server::rest::auth_logger()?
+        .log(format!("successful auth for user '{}'", user_id));
+
+    Ok(json!({
+        "username": user_id,
+        "ticket": ticket,
+        "CSRFPreventionToken": token,
+    }))
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            realm: {
+                schema: REALM_ID_SCHEMA,
+            },
+            "redirect-url": {
+                description: "Redirection Url. The client should set this to used server url.",
+                type: String,
+            },
+        },
+    },
+    returns: {
+        description: "Redirection URL.",
+        type: String,
+    },
+    access: {
+        description: "Anyone can access this (before the user is authenticated).",
+        permission: &Permission::World,
+    },
+)]
+/// Create OpenID Redirect Session
+fn openid_auth_url(
+    realm: String,
+    redirect_url: String,
+    _rpcenv: &mut dyn RpcEnvironment,
+) -> Result<String, Error> {
+
+    let (domains, _digest) = crate::config::domains::config()?;
+    let config: OpenIdRealmConfig = domains.lookup("openid", &realm)?;
+
+    let open_id = config.authenticator(&redirect_url)?;
+
+    let url = open_id.authorize_url(PROXMOX_BACKUP_RUN_DIR_M!(), &realm)?
+        .to_string();
+
+    Ok(url.into())
+}
+
+#[sortable]
+const SUBDIRS: SubdirMap = &sorted!([
+    ("login", &Router::new().post(&API_METHOD_OPENID_LOGIN)),
+    ("auth-url", &Router::new().post(&API_METHOD_OPENID_AUTH_URL)),
+]);
+
+pub const ROUTER: Router = Router::new()
+    .get(&list_subdirs_api_method!(SUBDIRS))
+    .subdirs(SUBDIRS);
diff --git a/src/api2/config/access/openid.rs b/src/api2/config/access/openid.rs
index 15fddaf0..9325de94 100644
--- a/src/api2/config/access/openid.rs
+++ b/src/api2/config/access/openid.rs
@@ -153,6 +153,8 @@ pub enum DeletableProperty {
     client_key,
     /// Delete the comment property.
     comment,
+    /// Delete the autocreate property
+    autocreate,
 }
 
 #[api(
@@ -177,6 +179,11 @@ pub enum DeletableProperty {
                 type: String,
                 optional: true,
             },
+            autocreate: {
+                description: "Automatically create users if they do not exist.",
+                optional: true,
+                type: bool,
+            },
             comment: {
                 schema: SINGLE_LINE_COMMENT_SCHEMA,
                 optional: true,
@@ -206,6 +213,7 @@ pub fn update_openid_realm(
     issuer_url: Option<String>,
     client_id: Option<String>,
     client_key: Option<String>,
+    autocreate: Option<bool>,
     comment: Option<String>,
     delete: Option<Vec<DeletableProperty>>,
     digest: Option<String>,
@@ -228,6 +236,7 @@ pub fn update_openid_realm(
             match delete_prop {
                 DeletableProperty::client_key => { config.client_key = None; },
                 DeletableProperty::comment => { config.comment = None; },
+                DeletableProperty::autocreate => { config.autocreate = None; },
             }
         }
     }
@@ -245,6 +254,7 @@ pub fn update_openid_realm(
     if let Some(client_id) = client_id { config.client_id = client_id; }
 
     if client_key.is_some() { config.client_key = client_key; }
+    if autocreate.is_some() { config.autocreate = autocreate; }
 
     domains.set_data(&realm, "openid", &config)?;
 
diff --git a/src/config/domains.rs b/src/config/domains.rs
index 3cdd4174..c456bc45 100644
--- a/src/config/domains.rs
+++ b/src/config/domains.rs
@@ -3,6 +3,8 @@ use lazy_static::lazy_static;
 use std::collections::HashMap;
 use serde::{Serialize, Deserialize};
 
+use proxmox_openid::{OpenIdAuthenticator,  OpenIdConfig};
+
 use proxmox::api::{
     api,
     schema::*,
@@ -25,6 +27,22 @@ lazy_static! {
     pub static ref CONFIG: SectionConfig = init();
 }
 
+#[api()]
+#[derive(Eq, PartialEq, Debug, Serialize, Deserialize)]
+#[serde(rename_all = "lowercase")]
+/// Use the value of this attribute/claim as unique user name. It is
+/// up to the identity provider to guarantee the uniqueness. The
+/// OpenID specification only guarantees that Subject ('sub') is unique. Also
+/// make sure that the user is not allowed to change that attribute by
+/// himself!
+pub enum OpenIdUserAttribute {
+    /// Subject (OpenId 'sub' claim)
+    Subject,
+    /// Username (OpenId 'preferred_username' claim)
+    Username,
+    /// Email (OpenId 'email' claim)
+    Email,
+}
 
 #[api(
     properties: {
@@ -48,6 +66,16 @@ lazy_static! {
             optional: true,
             schema: SINGLE_LINE_COMMENT_SCHEMA,
         },
+        autocreate: {
+            description: "Automatically create users if they do not exist.",
+            optional: true,
+            type: bool,
+            default: false,
+        },
+        "user-attr": {
+            type: OpenIdUserAttribute,
+            optional: true,
+        },
     },
 )]
 #[derive(Serialize,Deserialize)]
@@ -61,6 +89,22 @@ pub struct OpenIdRealmConfig {
     pub client_key: Option<String>,
     #[serde(skip_serializing_if="Option::is_none")]
     pub comment: Option<String>,
+    #[serde(skip_serializing_if="Option::is_none")]
+    pub autocreate: Option<bool>,
+    #[serde(skip_serializing_if="Option::is_none")]
+    pub user_attr: Option<OpenIdUserAttribute>,
+}
+
+impl OpenIdRealmConfig {
+
+    pub fn authenticator(&self, redirect_url: &str) -> Result<OpenIdAuthenticator, Error> {
+        let config = OpenIdConfig {
+            issuer_url: self.issuer_url.clone(),
+            client_id: self.client_id.clone(),
+            client_key: self.client_key.clone(),
+        };
+        OpenIdAuthenticator::discover(&config, redirect_url)
+    }
 }
 
 fn init() -> SectionConfig {
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v3 09/10] ui: implement OpenId login

[Dietmar Maurer]

---
 www/Application.js |   8 +++-
 www/LoginView.js   | 100 ++++++++++++++++++++++++++++++++++++++++++++-
 www/Utils.js       |   8 ++++
 3 files changed, 114 insertions(+), 2 deletions(-)

diff --git a/www/Application.js b/www/Application.js
index c4df600e..20ae26bd 100644
--- a/www/Application.js
+++ b/www/Application.js
@@ -49,9 +49,15 @@ Ext.define('PBS.Application', {
 	var provider = new Ext.state.LocalStorageProvider({ prefix: 'ext-pbs-' });
 	Ext.state.Manager.setProvider(provider);
 
+	let openid_login = false;
+	let param = PBS.Utils.openid_login_param();
+	if (param !== undefined) {
+	    openid_login = true;
+	}
+
 	// show login window if not loggedin
 	var loggedin = Proxmox.Utils.authOK();
-	if (!loggedin) {
+	if (openid_login || !loggedin) {
 	    me.changeView('loginview', true);
 	} else {
 	    me.changeView('mainview', true);
diff --git a/www/LoginView.js b/www/LoginView.js
index 6dd09646..ff3b5540 100644
--- a/www/LoginView.js
+++ b/www/LoginView.js
@@ -2,6 +2,21 @@ Ext.define('PBS.LoginView', {
     extend: 'Ext.container.Container',
     xtype: 'loginview',
 
+    viewModel: {
+	data: {
+	    openid: false,
+	},
+	formulas: {
+	    button_text: function(get) {
+		if (get("openid") === true) {
+		    return gettext("Login (OpenID redirect)");
+		} else {
+		    return gettext("Login");
+		}
+	    },
+	},
+    },
+
     controller: {
 	xclass: 'Ext.app.ViewController',
 
@@ -15,8 +30,33 @@ Ext.define('PBS.LoginView', {
 		return;
 	    }
 
+	    let redirect_url = location.origin;
+
 	    let params = loginForm.getValues();
 
+	    if (this.getViewModel().data.openid === true) {
+		let realm = params.realm;
+		try {
+		    let resp = await PBS.Async.api2({
+			url: '/api2/extjs/access/openid/auth-url',
+			params: {
+			    realm: realm,
+			    "redirect-url": redirect_url,
+			},
+			method: 'POST',
+		    });
+		    window.location = resp.result.data;
+		} catch (error) {
+		    Proxmox.Utils.authClear();
+		    loginForm.unmask();
+		    Ext.MessageBox.alert(
+			gettext('Error'),
+			gettext('OpenId redirect failed. Please try again<br>Error: ' + error),
+		    );
+		}
+		return;
+	    }
+
 	    params.username = params.username + '@' + params.realm;
 	    delete params.realm;
 
@@ -98,6 +138,14 @@ Ext.define('PBS.LoginView', {
 		    window.location.reload();
 		},
 	    },
+	    'field[name=realm]': {
+		change: function(f, value) {
+		    let record = f.store.getById(value);
+		    if (record === undefined) return;
+		    let data = record.data;
+		    this.getViewModel().set("openid", data.type === "openid");
+		},
+	    },
 	    'button[reference=loginButton]': {
 		click: 'submitForm',
 	    },
@@ -116,6 +164,43 @@ Ext.define('PBS.LoginView', {
 			var pwField = this.lookupReference('passwordField');
 			pwField.focus();
 		    }
+
+		    let param = PBS.Utils.openid_login_param();
+		    if (param !== undefined) {
+			Proxmox.Utils.authClear();
+
+			let loginForm = this.lookupReference('loginForm');
+			loginForm.mask(gettext('OpenID login - please wait...'), 'x-mask-loading');
+
+			let redirect_url = location.origin;
+
+			Proxmox.Utils.API2Request({
+			    url: '/api2/extjs/access/openid/login',
+			    params: {
+				state: param.state,
+				code: param.code,
+				"redirect-url": redirect_url,
+			    },
+			    method: 'POST',
+			    failure: function(response) {
+				loginForm.unmask();
+				Ext.MessageBox.alert(
+				    gettext('Error'),
+				    gettext('Login failed. Please try again<br>Error: ' + response.htmlStatus),
+				    function() {
+					window.location = redirect_url;
+				    },
+				);
+			    },
+			    success: function(response, options) {
+				loginForm.unmask();
+				let data = response.result.data;
+				PBS.Utils.updateLoginData(data);
+				PBS.app.changeView('mainview');
+				history.replaceState(null, '', redirect_url + '#pbsDashboard');
+			    },
+			});
+		    }
 		},
 	    },
 	},
@@ -191,6 +276,10 @@ Ext.define('PBS.LoginView', {
 			    itemId: 'usernameField',
 			    reference: 'usernameField',
 			    stateId: 'login-username',
+			    bind: {
+				visible: "{!openid}",
+				disabled: "{openid}",
+			    },
 			},
 			{
 			    xtype: 'textfield',
@@ -199,6 +288,10 @@ Ext.define('PBS.LoginView', {
 			    name: 'password',
 			    itemId: 'passwordField',
 			    reference: 'passwordField',
+			    bind: {
+				visible: "{!openid}",
+				disabled: "{openid}",
+			    },
 			},
 			{
 			    xtype: 'pmxRealmComboBox',
@@ -223,9 +316,14 @@ Ext.define('PBS.LoginView', {
 			    labelWidth: 250,
 			    labelAlign: 'right',
 			    submitValue: false,
+			    bind: {
+				visible: "{!openid}",
+			    },
 			},
 			{
-			    text: gettext('Login'),
+			    bind: {
+				text: "{button_text}",
+			    },
 			    reference: 'loginButton',
 			    formBind: true,
 			},
diff --git a/www/Utils.js b/www/Utils.js
index 6b378355..677f2204 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -326,6 +326,14 @@ Ext.define('PBS.Utils', {
         };
     },
 
+    openid_login_param: function() {
+	let param = Ext.Object.fromQueryString(window.location.search);
+	if (param.state !== undefined && param.code !== undefined) {
+	    return param;
+	}
+	return undefined;
+    },
+
     calculate_dedup_factor: function(gcstatus) {
 	let dedup = 1.0;
 	if (gcstatus['disk-bytes'] > 0) {
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v3 10/10] fix CachedUserInfo by using a shared memory version counter

[Dietmar Maurer]

---
 src/api2/access/openid.rs      |   2 -
 src/config/cached_user_info.rs |  13 ++-
 src/config/user.rs             |   6 ++
 src/tools.rs                   |   3 +
 src/tools/memcom.rs            | 159 +++++++++++++++++++++++++++++++++
 5 files changed, 179 insertions(+), 4 deletions(-)
 create mode 100644 src/tools/memcom.rs

diff --git a/src/api2/access/openid.rs b/src/api2/access/openid.rs
index 52ec4311..27e3c08c 100644
--- a/src/api2/access/openid.rs
+++ b/src/api2/access/openid.rs
@@ -120,8 +120,6 @@ pub fn openid_login(
             }
             config.set_data(user.userid.as_str(), "user", &user)?;
             user::save_config(&config)?;
-            // fixme: replace sleep with shared memory change notification
-            std::thread::sleep(std::time::Duration::new(6, 0));
         } else {
             bail!("user account '{}' missing, disabled or expired.", user_id);
         }
diff --git a/src/config/cached_user_info.rs b/src/config/cached_user_info.rs
index 6cb64162..a4c4604f 100644
--- a/src/config/cached_user_info.rs
+++ b/src/config/cached_user_info.rs
@@ -12,6 +12,7 @@ use proxmox::tools::time::epoch_i64;
 use super::acl::{AclTree, ROLE_NAMES, ROLE_ADMIN};
 use super::user::{ApiToken, User};
 use crate::api2::types::{Authid, Userid};
+use crate::tools::Memcom;
 
 /// Cache User/Group/Token/Acl configuration data for fast permission tests
 pub struct CachedUserInfo {
@@ -22,11 +23,12 @@ pub struct CachedUserInfo {
 struct ConfigCache {
     data: Option<Arc<CachedUserInfo>>,
     last_update: i64,
+    last_user_cache_generation: usize,
 }
 
 lazy_static! {
     static ref CACHED_CONFIG: RwLock<ConfigCache> = RwLock::new(
-        ConfigCache { data: None, last_update: 0 }
+        ConfigCache { data: None, last_update: 0, last_user_cache_generation: 0 }
     );
 }
 
@@ -35,9 +37,15 @@ impl CachedUserInfo {
     /// Returns a cached instance (up to 5 seconds old).
     pub fn new() -> Result<Arc<Self>, Error> {
         let now = epoch_i64();
+
+        let memcom = Memcom::new()?;
+        let user_cache_generation = memcom.user_cache_generation();
+
         { // limit scope
             let cache = CACHED_CONFIG.read().unwrap();
-            if (now - cache.last_update) < 5 {
+            if (user_cache_generation == cache.last_user_cache_generation) &&
+                ((now - cache.last_update) < 5)
+            {
                 if let Some(ref config) = cache.data {
                     return Ok(config.clone());
                 }
@@ -51,6 +59,7 @@ impl CachedUserInfo {
 
         let mut cache = CACHED_CONFIG.write().unwrap();
         cache.last_update = now;
+        cache.last_user_cache_generation = user_cache_generation;
         cache.data = Some(config.clone());
 
         Ok(config)
diff --git a/src/config/user.rs b/src/config/user.rs
index 28e81876..bdec5fc1 100644
--- a/src/config/user.rs
+++ b/src/config/user.rs
@@ -18,6 +18,7 @@ use proxmox::api::{
 use proxmox::tools::{fs::replace_file, fs::CreateOptions};
 
 use crate::api2::types::*;
+use crate::tools::Memcom;
 
 lazy_static! {
     pub static ref CONFIG: SectionConfig = init();
@@ -270,6 +271,11 @@ pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
 
     replace_file(USER_CFG_FILENAME, raw.as_bytes(), options)?;
 
+    // increase user cache generation
+    // We use this in CachedUserInfo
+    let memcom = Memcom::new()?;
+    memcom.increase_user_cache_generation();
+
     Ok(())
 }
 
diff --git a/src/tools.rs b/src/tools.rs
index 59599339..3b0bf087 100644
--- a/src/tools.rs
+++ b/src/tools.rs
@@ -38,6 +38,9 @@ pub mod format;
 pub mod fs;
 pub mod fuse_loop;
 
+mod memcom;
+pub use memcom::Memcom;
+
 pub mod json;
 pub mod logrotate;
 pub mod loopdev;
diff --git a/src/tools/memcom.rs b/src/tools/memcom.rs
new file mode 100644
index 00000000..139a789a
--- /dev/null
+++ b/src/tools/memcom.rs
@@ -0,0 +1,159 @@
+//! Memory based communication channel between proxy & daemon for things such as cache
+//! invalidation.
+
+use std::ffi::CString;
+use std::io;
+use std::os::unix::io::AsRawFd;
+use std::sync::atomic::{AtomicUsize, Ordering};
+use std::sync::Arc;
+
+use anyhow::{bail, format_err, Error};
+use nix::errno::Errno;
+use nix::fcntl::OFlag;
+use nix::sys::mman::{MapFlags, ProtFlags};
+use nix::sys::stat::Mode;
+use once_cell::sync::OnceCell;
+
+use proxmox::sys::error::SysError;
+use proxmox::tools::fd::Fd;
+use proxmox::tools::mmap::Mmap;
+
+/// In-memory communication channel.
+pub struct Memcom {
+    mmap: Mmap<u8>,
+}
+
+#[repr(C)]
+struct Head {
+    // User (user.cfg) cache generation/version.
+    user_cache_generation: AtomicUsize,
+}
+
+static INSTANCE: OnceCell<Arc<Memcom>>  = OnceCell::new();
+
+const MEMCOM_FILE_PATH: &str = rundir!("/proxmox-backup-memcom");
+
+impl Memcom {
+    
+    /// Open the memory based communication channel singleton.
+    pub fn new() ->  Result<Arc<Self>, Error>  {
+        INSTANCE.get_or_try_init(Self::open).map(Arc::clone)
+    }
+
+    // Actual work of `new`:
+    fn open() ->  Result<Arc<Self>, Error>  {
+        let fd = match open_existing() {
+            Ok(fd) =>  fd,
+            Err(err) if err.not_found() =>  create_new()?,
+            Err(err) =>  bail!("failed to open {} - {}", MEMCOM_FILE_PATH, err),
+        };
+
+        let mmap = unsafe {
+            Mmap::<u8>::map_fd(
+                fd.as_raw_fd(),
+                0,
+                4096,
+                ProtFlags::PROT_READ | ProtFlags::PROT_WRITE,
+                MapFlags::MAP_SHARED | MapFlags::MAP_NORESERVE | MapFlags::MAP_POPULATE,
+            )?
+        };
+        
+        Ok(Arc::new(Self { mmap }))
+    }
+
+    // Shortcut to get the mapped `Head` as a `Head`.
+    fn head(&self) ->  &Head {
+        unsafe { &*(self.mmap.as_ptr() as *const u8 as *const Head) }
+    }
+
+    /// Returns the user cache generation number.
+    pub fn user_cache_generation(&self) ->  usize {
+        self.head().user_cache_generation.load(Ordering::Acquire)
+    }
+
+    /// Increase the user cache generation number.
+    pub fn increase_user_cache_generation(&self) {
+        self.head()
+            .user_cache_generation
+            .fetch_add(1, Ordering::AcqRel);
+    }
+}
+
+/// The fast path opens an existing file.
+fn open_existing() ->  Result<Fd, nix::Error>  {
+      Fd::open(MEMCOM_FILE_PATH, OFlag::O_RDWR | OFlag::O_CLOEXEC, Mode::empty())
+}
+
+/// Since we need to initialize the file, we also need a solid slow path where we create the file.
+/// In order to make sure the next user's `open()` vs `mmap()` race against our `truncate()` call,
+/// we create it in a temporary location and rotate it in place.
+fn create_new() ->  Result<Fd, Error>  {
+    // create a temporary file:
+    let temp_file_name = format!("{}.{}", MEMCOM_FILE_PATH, unsafe { libc::getpid() });
+    let fd = Fd::open(
+        temp_file_name.as_str(),
+        OFlag::O_CREAT | OFlag::O_EXCL | OFlag::O_RDWR | OFlag::O_CLOEXEC,
+        Mode::from_bits_truncate(0o660),
+    ).map_err(|err| {
+        format_err!(
+            "failed to create new in-memory communication file at {} - {}",
+            temp_file_name,
+            err
+        )
+    })?;
+
+    // let it be a page in size, it'll be initialized to zero by the kernel
+    nix::unistd::ftruncate(fd.as_raw_fd(), 4096)
+        .map_err(|err| format_err!("failed to set size of {} - {}", temp_file_name, err))?;
+
+    // if this is the pbs-daemon (running as root) rather than the proxy (running as backup user),
+    // make sure the backup user can access the file:
+    if let Ok(backup_user) = crate::backup::backup_user() {
+        match nix::unistd::fchown(fd.as_raw_fd(), None, Some(backup_user.gid)) {
+            Ok(()) =>  (),
+            Err(err) if err.is_errno(Errno::EPERM) =>  {
+                // we're not the daemon (root), so the file is already owned by the backup user
+            }
+            Err(err) =>  bail!(
+                "failed to set group to 'backup' for {} - {}",
+                temp_file_name,
+                err
+            ),
+        }
+    }
+
+    // rotate the file into place, but use `RENAME_NOREPLACE`, so in case 2 processes race against
+    // the initialization, the first one wins!
+    // TODO: nicer `renameat2()` wrapper in `proxmox::sys`?
+    let c_file_name = CString::new(temp_file_name.as_bytes()).unwrap();
+    let new_path = CString::new(MEMCOM_FILE_PATH).unwrap();
+      let rc = unsafe {
+          libc::renameat2(
+              -1,
+              c_file_name.as_ptr(),
+              -1,
+              new_path.as_ptr(),
+              libc::RENAME_NOREPLACE,
+          )
+      };
+    if rc == 0 {
+        return Ok(fd);
+    }
+    let err = io::Error::last_os_error();
+
+    // if another process has already raced ahead and created the file, let's just open theirs
+    // instead:
+    if err.kind() == io::ErrorKind::AlreadyExists {
+        // someone beat us to it:
+        drop(fd);
+        return open_existing().map_err(Error::from);
+    }
+    
+    // for any other errors, just bail out
+    bail!(
+        "failed to move file at {} into place at {} - {}",
+        temp_file_name,
+        MEMCOM_FILE_PATH,
+        err
+    );
+}
-- 
2.30.2

Re: [pbs-devel] [PATCH proxmox-backup v3 05/10] cli: add CLI to manage openid realms.

[Wolfgang Bumiller]

just a minor cleanup:

On Fri, Jun 25, 2021 at 11:20:45AM +0200, Dietmar Maurer wrote:
> diff --git a/src/bin/proxmox_backup_manager/openid.rs b/src/bin/proxmox_backup_manager/openid.rs
> new file mode 100644
> index 00000000..13915339
> --- /dev/null
> +++ b/src/bin/proxmox_backup_manager/openid.rs
> @@ -0,0 +1,99 @@
> +use anyhow::Error;
> +use serde_json::Value;
> +
> +use proxmox::api::{api, cli::*, RpcEnvironment, ApiHandler};
> +
> +use proxmox_backup::{config, api2, api2::types::REALM_ID_SCHEMA};
> +
> +
> +#[api(
> +    input: {
> +        properties: {
> +            "output-format": {
> +                schema: OUTPUT_FORMAT,
> +                optional: true,
> +            },
> +        }
> +    }
> +)]
> +/// List configured OpenId realms
> +fn list_openid_realms(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
> +
> +    let output_format = get_output_format(&param);
> +
> +    let info = &api2::config::access::openid::API_METHOD_LIST_OPENID_REALMS;
> +    let mut data = match info.handler {
> +        ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
> +        _ => unreachable!(),
> +    };
> +
> +    let options = default_table_format_options()
> +        .column(ColumnConfig::new("realm"))
> +        .column(ColumnConfig::new("issuer-url"))
> +        .column(ColumnConfig::new("comment"));
> +
> +    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
> +
> +    Ok(Value::Null)
> +}
> +#[api(
> +    input: {
> +        properties: {
> +            realm: {
> +                schema: REALM_ID_SCHEMA,
> +            },
> +            "output-format": {
> +                schema: OUTPUT_FORMAT,
> +                optional: true,
> +            },
> +        }
> +    }
> +)]
> +
> +/// Show OpenID realm configuration
> +fn show_openid_realm(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
> +
> +    let output_format = get_output_format(&param);
> +
> +    let info = &api2::config::access::openid::API_METHOD_READ_OPENID_REALM;
> +    let mut data = match info.handler {
> +        ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
> +        _ => unreachable!(),
> +    };
> +
> +    let options = default_table_format_options();
> +    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
> +
> +    Ok(Value::Null)
> +}
> +
> +pub fn openid_commands() -> CommandLineInterface {
> +
> +    let cmd_def = CliCommandMap::new()
> +        .insert("list", CliCommand::new(&&API_METHOD_LIST_OPENID_REALMS))
> +        .insert("show", CliCommand::new(&&API_METHOD_SHOW_OPENID_REALM)
> +                .arg_param(&["realm"])
> +                .completion_cb("realm", config::domains::complete_openid_realm_name)
> +        )
> +        .insert("create",
> +                CliCommand::new(&api2::config::access::openid::API_METHOD_CREATE_OPENID_REALM)
> +                .arg_param(&["realm"])
> +                .arg_param(&["realm"])

^ this line is duplicated

> +                .completion_cb("realm", config::domains::complete_openid_realm_name)
> +        )
> +        .insert("update",
> +                CliCommand::new(&api2::config::access::openid::API_METHOD_UPDATE_OPENID_REALM)
> +                .arg_param(&["realm"])
> +                .arg_param(&["realm"])

^ this line is duplicated

> +                .completion_cb("realm", config::domains::complete_openid_realm_name)
> +        )
> +        .insert("delete",
> +                CliCommand::new(&api2::config::access::openid::API_METHOD_DELETE_OPENID_REALM)
> +                .arg_param(&["realm"])
> +                .arg_param(&["realm"])

^ this line is duplicated

> +                .completion_cb("realm", config::domains::complete_openid_realm_name)
> +        )
> +        ;
> +
> +       cmd_def.into()
> +}

Re: [pbs-devel] [PATCH proxmox-backup v3 00/10] OpenID connect realms

[Fabian Grünbichler]

seems to work okay in general (and looks okay code-wise), some missing 
pieces:
- proxmox-openid-rs prints some information that looks like debug (full 
  returned info from provider)
- user-attr in proxmox-backup like in PVE
- GUI: user adding/editing needs some adaptation for openid (e.g., removing a 
  user prints a warning about unknown realm, but removes it anyway, 
  adding a user has no realm selection and requires a password)
- GUI: realm adding/editing is missing altogether

the check_acl_path change could be split and folded into the patches 
that first use that ACL path, but no hard feelings there..

the last patch probably requires some in-depth review, I only took a 
cursory glance (and it could be ordered before the patch with the fixme 
that it fixes)

On June 25, 2021 11:20 am, Dietmar Maurer wrote:
> This implements OpenID connect realms using the new
> "proxmox-openid-rs" crate.
> 
> Note: The new src/tools/memcom.rs is from Wolfgang.
> 
> Changes since v2:
>    fix CachedUserInfo by using a shared memory version counter
> 
> Changes since v1:
> - really fix commit message of first patch
> - change api endpoints (/access/openid/{login|auth-url})
> - merged all api implementation patches
> 
> Changes since preview version (for Fabian):
> - fix commit message
> - reserve namen 'pam' and 'pbs'
> - fix 'make deb'
> 
> 
> Dietmar Maurer (10):
>   depend on proxmox-openid-rs
>   config: new domains.cfg to configure openid realm
>   check_acl_path: add /access/domains and /access/openid
>   add API to manage openid realms
>   cli: add CLI to manage openid realms.
>   implement new helper is_active_user_id()
>   cleanup user/token is_active() check
>   api: add openid redirect/login API
>   ui: implement OpenId login
>   fix CachedUserInfo by using a shared memory version counter
> 
>  Cargo.toml                               |   2 +
>  src/api2/access.rs                       |   4 +-
>  src/api2/access/domain.rs                |  18 ++
>  src/api2/access/openid.rs                | 190 ++++++++++++++++
>  src/api2/config/access/mod.rs            |   8 +-
>  src/api2/config/access/openid.rs         | 274 +++++++++++++++++++++++
>  src/bin/proxmox-backup-manager.rs        |   1 +
>  src/bin/proxmox_backup_manager/mod.rs    |   2 +
>  src/bin/proxmox_backup_manager/openid.rs |  99 ++++++++
>  src/config.rs                            |   1 +
>  src/config/acl.rs                        |   8 +-
>  src/config/cached_user_info.rs           |  48 ++--
>  src/config/domains.rs                    | 173 ++++++++++++++
>  src/config/user.rs                       |  38 ++++
>  src/tools.rs                             |   3 +
>  src/tools/memcom.rs                      | 159 +++++++++++++
>  www/Application.js                       |   8 +-
>  www/LoginView.js                         | 100 ++++++++-
>  www/Utils.js                             |   8 +
>  19 files changed, 1115 insertions(+), 29 deletions(-)
>  create mode 100644 src/api2/access/openid.rs
>  create mode 100644 src/api2/config/access/openid.rs
>  create mode 100644 src/bin/proxmox_backup_manager/openid.rs
>  create mode 100644 src/config/domains.rs
>  create mode 100644 src/tools/memcom.rs
> 
> -- 
> 2.30.2
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
>