[pbs-devel] [PATCH widget-toolkit/proxmox-backup] fix openid realm api usage

[pbs-devel] [PATCH widget-toolkit/proxmox-backup] fix openid realm api usage

[Dominik Csapak]

i missed that we already have the openid realm api. not in the usual
place but in '/config/access/openid'

this series adds the ability in the refactored widget to handle
different urls (also with the type in the path) and reverts the
unnecessary patches, so that we do not have a duplicate api

since the pve-manager patches were not applied yet (and would need
some changes anyway), this should not impact manager

proxmox-widget-toolkit:

Dominik Csapak (2):
  window/AuthEditBase: handle differenc config api urls
  panel/AuthView: handle different baseUrls for configuring realms

 src/panel/AuthView.js      | 14 +++++++++++++-
 src/window/AuthEditBase.js | 14 +++++++++++---
 2 files changed, 24 insertions(+), 4 deletions(-)

proxmox-backup:

Dominik Csapak (5):
  ui: panel/AccessControl: define baseUrland useTypeInUrl for AuthView
  api: config: access: openid: use correct parameter for matching
  api: config: access: openid: use better Privilige Realm.Allocate
  Revert "api: access: domains: add get/create/update/delete domain
    call"
  Revert "api: access: domains: add ExtraRealmInfo and RealmInfo
    structs"

 src/api2/access/domain.rs        | 344 +------------------------------
 src/api2/config/access/openid.rs |  12 +-
 www/panel/AccessControl.js       |   2 +
 3 files changed, 11 insertions(+), 347 deletions(-)

-- 
2.30.2

[pbs-devel] [PATCH widget-toolkit 1/2] window/AuthEditBase: handle differenc config api urls

[Dominik Csapak]

necessary for pbs. also give an option to use the type as url path
e.g. /config/access/<TYPE>/<ID>

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/window/AuthEditBase.js | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/window/AuthEditBase.js b/src/window/AuthEditBase.js
index 4faea34..43a2fd6 100644
--- a/src/window/AuthEditBase.js
+++ b/src/window/AuthEditBase.js
@@ -7,16 +7,23 @@ Ext.define('Proxmox.window.AuthEditBase', {
 	labelWidth: 120,
     },
 
+    baseurl: '/access/domains',
+    useTypeInUrl: false,
+
     initComponent: function() {
 	var me = this;
 
 	me.isCreate = !me.realm;
 
+	me.url = `/api2/extjs${me.baseUrl}`;
+	if (me.useTypeInUrl) {
+	    me.url += `/${me.authType}`;
+	}
+
 	if (me.isCreate) {
-	    me.url = '/api2/extjs/access/domains';
 	    me.method = 'POST';
 	} else {
-	    me.url = '/api2/extjs/access/domains/' + me.realm;
+	    me.url += `/${me.realm}`;
 	    me.method = 'PUT';
 	}
 
@@ -76,7 +83,8 @@ Ext.define('Proxmox.window.AuthEditBase', {
 		success: function(response, options) {
 		    var data = response.result.data || {};
 		    // just to be sure (should not happen)
-		    if (data.type !== me.authType) {
+		    // only check this when the type is not in the api path
+		    if (!me.useTypeInUrl && data.type !== me.authType) {
 			me.close();
 			throw "got wrong auth type";
 		    }
-- 
2.30.2

[pbs-devel] [PATCH widget-toolkit 2/2] panel/AuthView: handle different baseUrls for configuring realms

[Dominik Csapak]

we need that for pbs.

also give the option to use the type in the path, like in AuthEditBase

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/panel/AuthView.js | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/panel/AuthView.js b/src/panel/AuthView.js
index 9bd9b3f..781e84e 100644
--- a/src/panel/AuthView.js
+++ b/src/panel/AuthView.js
@@ -10,6 +10,9 @@ Ext.define('Proxmox.panel.AuthView', {
 	trackOver: false,
     },
 
+    baseUrl: '/access/domains',
+    useTypeInUrl: false,
+
     columns: [
 	{
 	    header: gettext('Realm'),
@@ -43,6 +46,8 @@ Ext.define('Proxmox.panel.AuthView', {
     openEditWindow: function(authType, realm) {
 	let me = this;
 	Ext.create('Proxmox.window.AuthEditBase', {
+	    baseUrl: me.baseUrl,
+	    useTypeInUrl: me.useTypeInUrl,
 	    authType,
 	    realm,
 	    listeners: {
@@ -99,7 +104,14 @@ Ext.define('Proxmox.panel.AuthView', {
 	    },
 	    {
 		xtype: 'proxmoxStdRemoveButton',
-		baseurl: '/access/domains/',
+		getUrl: (rec) => {
+		    let url = me.baseUrl;
+		    if (me.useTypeInUrl) {
+			url += `/${rec.get('type')}`;
+		    }
+		    url += `/${rec.getId()}`;
+		    return url;
+		},
 		enableFn: (rec) => Proxmox.Schema.authDomains[rec.data.type].add,
 		callback: () => me.reload(),
 	    },
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup 1/5] ui: panel/AccessControl: define baseUrland useTypeInUrl for AuthView

[Dominik Csapak]

both are not the default

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/panel/AccessControl.js | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/www/panel/AccessControl.js b/www/panel/AccessControl.js
index e8ddb905..2e65cc2e 100644
--- a/www/panel/AccessControl.js
+++ b/www/panel/AccessControl.js
@@ -39,6 +39,8 @@ Ext.define('PBS.AccessControlPanel', {
 	},
 	{
 	    xtype: 'pmxAuthView',
+	    baseUrl: '/config/access',
+	    useTypeInUrl: true,
 	    title: gettext('Realms'),
 	    itemId: 'domains',
 	    iconCls: 'fa fa-address-book-o',
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup 2/5] api: config: access: openid: use correct parameter for matching

[Dominik Csapak]

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/api2/config/access/openid.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/api2/config/access/openid.rs b/src/api2/config/access/openid.rs
index 9325de94..5ed1cdb3 100644
--- a/src/api2/config/access/openid.rs
+++ b/src/api2/config/access/openid.rs
@@ -271,4 +271,4 @@ const ITEM_ROUTER: Router = Router::new()
 pub const ROUTER: Router = Router::new()
     .get(&API_METHOD_LIST_OPENID_REALMS)
     .post(&API_METHOD_CREATE_OPENID_REALM)
-    .match_all("id", &ITEM_ROUTER);
+    .match_all("realm", &ITEM_ROUTER);
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup 3/5] api: config: access: openid: use better Privilige Realm.Allocate

[Dominik Csapak]

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/api2/config/access/openid.rs | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/api2/config/access/openid.rs b/src/api2/config/access/openid.rs
index 5ed1cdb3..b8b07306 100644
--- a/src/api2/config/access/openid.rs
+++ b/src/api2/config/access/openid.rs
@@ -7,7 +7,7 @@ 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::config::acl::{PRIV_SYS_AUDIT, PRIV_REALM_ALLOCATE};
 use crate::api2::types::*;
 
 #[api(
@@ -20,7 +20,7 @@ use crate::api2::types::*;
         items: { type: OpenIdRealmConfig },
     },
     access: {
-        permission: &Permission::Privilege(&["access", "domains"], PRIV_SYS_AUDIT, false),
+        permission: &Permission::Privilege(&["access", "domains"], PRIV_REALM_ALLOCATE, false),
     },
 )]
 /// List configured OpenId realms
@@ -49,7 +49,7 @@ pub fn list_openid_realms(
         },
     },
     access: {
-        permission: &Permission::Privilege(&["access", "domains"], PRIV_PERMISSIONS_MODIFY, false),
+        permission: &Permission::Privilege(&["access", "domains"], PRIV_REALM_ALLOCATE, false),
     },
 )]
 /// Create a new OpenId realm
@@ -87,7 +87,7 @@ pub fn create_openid_realm(config: OpenIdRealmConfig) -> Result<(), Error> {
         },
     },
     access: {
-        permission: &Permission::Privilege(&["access", "domains"], PRIV_PERMISSIONS_MODIFY, false),
+        permission: &Permission::Privilege(&["access", "domains"], PRIV_REALM_ALLOCATE, false),
     },
 )]
 /// Remove a OpenID realm configuration
@@ -204,7 +204,7 @@ pub enum DeletableProperty {
     },
     returns:  { type: OpenIdRealmConfig },
     access: {
-        permission: &Permission::Privilege(&["access", "domains"], PRIV_PERMISSIONS_MODIFY, false),
+        permission: &Permission::Privilege(&["access", "domains"], PRIV_REALM_ALLOCATE, false),
     },
 )]
 /// Update an OpenID realm configuration
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup 4/5] Revert "api: access: domains: add get/create/update/delete domain call"

[Dominik Csapak]

This reverts commit 5117cf4f1786e0b37d486413d658d78a4bcfdf53.

we already have that in api2/config/access

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/api2/access/domain.rs | 281 +-------------------------------------
 1 file changed, 4 insertions(+), 277 deletions(-)

diff --git a/src/api2/access/domain.rs b/src/api2/access/domain.rs
index 6b1a4743..bfc7c184 100644
--- a/src/api2/access/domain.rs
+++ b/src/api2/access/domain.rs
@@ -1,18 +1,15 @@
 //! List Authentication domains/realms
 
-use anyhow::{bail, Error};
+use anyhow::{Error};
 
 use serde::{Deserialize, Serialize};
 use serde_json::{json, Value};
 
 use proxmox::api::{api, Permission, Router, RpcEnvironment};
 
+use crate::config;
 use crate::api2::types::*;
-use crate::config::{
-    self,
-    acl::{PRIV_REALM_ALLOCATE, PRIV_SYS_AUDIT},
-    domains::{OpenIdRealmConfig, OpenIdUserAttribute},
-};
+use crate::config::domains::{OpenIdRealmConfig, OpenIdUserAttribute};
 
 #[api]
 #[derive(Deserialize, Serialize, PartialEq, Eq)]
@@ -162,275 +159,5 @@ fn list_domains(mut rpcenv: &mut dyn RpcEnvironment) -> Result<Vec<BasicRealmInf
     Ok(list)
 }
 
-#[api(
-    input: {
-        properties: {
-            realm: {
-                schema: REALM_ID_SCHEMA,
-            },
-        },
-    },
-    returns: {
-        type: RealmInfo,
-    },
-    access: {
-        permission: &Permission::Privilege(&["access", "domains"], PRIV_SYS_AUDIT | PRIV_REALM_ALLOCATE, true),
-    },
-)]
-/// Get information about a realm
-fn get_domain(realm: String, mut rpcenv: &mut dyn RpcEnvironment) -> Result<RealmInfo, Error> {
-    let entry = match realm.as_str() {
-        "pam" => json!({
-            "realm": "pam",
-            "type": "pam",
-            "comment": "Linux PAM standard authentication",
-            "default": Some(true),
-        }),
-        "pbs" => json!({
-            "realm": "pbs",
-            "type": "pbs",
-            "comment": "Proxmox Backup authentication server",
-        }),
-        _ => {
-            let (config, digest) = config::domains::config()?;
-            rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into();
-            if let Some((section_type, v)) = config.sections.get(&realm) {
-                let mut entry = v.clone();
-                entry["type"] = Value::from(section_type.clone());
-                entry
-            } else {
-                bail!("domain '{}' does not exist", realm);
-            }
-        }
-    };
-
-    Ok(serde_json::from_value(entry)?)
-}
-
-#[api(
-    protected: true,
-    input: {
-        properties: {
-            info: {
-                type: RealmInfo,
-                flatten: true,
-            },
-        },
-    },
-    access: {
-        permission: &Permission::Privilege(&["access", "domains"], PRIV_REALM_ALLOCATE, false),
-    },
-)]
-/// Create a realm
-fn create_domain(param: Value) -> Result<(), Error> {
-    let basic_info: BasicRealmInfo = serde_json::from_value(param.clone())?;
-
-    // for now we only have to care about openid
-    if basic_info.ty != RealmType::OpenId {
-        bail!(
-            "Cannot create realm of type '{}'",
-            serde_json::to_string(&basic_info.ty)?
-        );
-    }
-
-    let new_realm: OpenIdRealmConfig = serde_json::from_value(param)?;
-    let _lock = config::domains::lock_config()?;
-
-    let (mut config, _digest) = config::domains::config()?;
-
-    let existing: Vec<OpenIdRealmConfig> = config.convert_to_typed_array("openid")?;
-
-    for realm in existing {
-        if realm.realm == new_realm.realm {
-            bail!("Entry '{}' already exists", realm.realm);
-        }
-    }
-
-    config.set_data(&new_realm.realm, "openid", &new_realm)?;
-
-    config::domains::save_config(&config)?;
-
-    Ok(())
-}
-
-#[api]
-#[derive(Serialize, Deserialize)]
-#[serde(rename_all = "kebab-case")]
-#[allow(non_camel_case_types)]
-pub enum DeletableProperty {
-    /// Delete the comment property.
-    comment,
-    /// Delete the client-key property.
-    client_key,
-    /// Delete the autocreate property.
-    autocreate,
-}
-
-#[api(
-    protected: true,
-    input: {
-        properties: {
-            realm: {
-                schema: REALM_ID_SCHEMA,
-            },
-            comment: {
-                optional: true,
-                schema: SINGLE_LINE_COMMENT_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,
-            },
-            autocreate: {
-                description: "Automatically create users if they do not exist.",
-                optional: true,
-                type: bool,
-            },
-            delete: {
-                description: "List of properties to delete.",
-                type: Array,
-                optional: true,
-                items: {
-                    type: DeletableProperty,
-                }
-            },
-            digest: {
-                optional: true,
-                schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
-            },
-        },
-    },
-    access: {
-        permission: &Permission::Privilege(&["access", "domains"], PRIV_REALM_ALLOCATE, false),
-    },
-)]
-/// Update a realm
-fn update_domain(
-    realm: String,
-    comment: Option<String>,
-    issuer_url: Option<String>,
-    client_id: Option<String>,
-    client_key: Option<String>,
-    autocreate: Option<bool>,
-    delete: Option<Vec<DeletableProperty>>,
-    digest: Option<String>,
-    _rpcenv: &mut dyn RpcEnvironment,
-) -> Result<(), Error> {
-    let _lock = config::domains::lock_config()?;
-
-    let (mut config, expected_digest) = config::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)?;
-    }
-
-    // only have to worry about openid for now
-    let mut data: OpenIdRealmConfig = config.lookup("openid", realm.as_str())?;
-
-    if let Some(delete) = delete {
-        for delete_prop in delete {
-            match delete_prop {
-                DeletableProperty::comment => data.comment = None,
-                DeletableProperty::client_key => data.client_key = None,
-                DeletableProperty::autocreate => data.autocreate = None,
-            }
-        }
-    }
-
-    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(issuer_url) = issuer_url {
-        data.issuer_url = issuer_url
-    };
-    if let Some(client_id) = client_id {
-        data.client_id = client_id
-    };
-    if let Some(client_key) = client_key {
-        data.client_key = if client_key.is_empty() {
-            None
-        } else {
-            Some(client_key)
-        };
-    };
-    if let Some(autocreate) = autocreate {
-        data.autocreate = Some(autocreate)
-    };
-
-    config.set_data(&realm, "openid", &data)?;
-
-    config::domains::save_config(&config)?;
-
-    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_REALM_ALLOCATE, false),
-    },
-)]
-/// Delete a realm
-fn delete_domain(realm: String, digest: Option<String>) -> Result<(), Error> {
-    if realm == "pam" || realm == "pbs" {
-        bail!("cannot remove realm '{}'", realm);
-    }
-    let _lock = config::domains::lock_config()?;
-
-    let (mut config, expected_digest) = config::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)?;
-    }
-
-    match config.sections.get(&realm) {
-        Some(_) => {
-            config.sections.remove(&realm);
-        }
-        None => bail!("realm '{}' does not exist.", realm),
-    }
-
-    config::domains::save_config(&config)?;
-
-    Ok(())
-}
-
 pub const ROUTER: Router = Router::new()
-    .get(&API_METHOD_LIST_DOMAINS)
-    .post(&API_METHOD_CREATE_DOMAIN)
-    .match_all("realm", &DOMAIN_ROUTER);
-
-const DOMAIN_ROUTER: Router = Router::new()
-    .get(&API_METHOD_GET_DOMAIN)
-    .put(&API_METHOD_UPDATE_DOMAIN)
-    .delete(&API_METHOD_DELETE_DOMAIN);
+    .get(&API_METHOD_LIST_DOMAINS);
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup 5/5] Revert "api: access: domains: add ExtraRealmInfo and RealmInfo structs"

[Dominik Csapak]

This reverts commit da7ec1d2af1c28503052e1cd5954dfada81058cd.

not necessary, since we have the api in config/access/openid

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/api2/access/domain.rs | 65 ---------------------------------------
 1 file changed, 65 deletions(-)

diff --git a/src/api2/access/domain.rs b/src/api2/access/domain.rs
index bfc7c184..afa69269 100644
--- a/src/api2/access/domain.rs
+++ b/src/api2/access/domain.rs
@@ -9,7 +9,6 @@ use proxmox::api::{api, Permission, Router, RpcEnvironment};
 
 use crate::config;
 use crate::api2::types::*;
-use crate::config::domains::{OpenIdRealmConfig, OpenIdUserAttribute};
 
 #[api]
 #[derive(Deserialize, Serialize, PartialEq, Eq)]
@@ -52,70 +51,6 @@ pub struct BasicRealmInfo {
     pub comment: Option<String>,
 }
 
-#[api(
-    properties: {
-        "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,
-        },
-        autocreate: {
-            description: "Automatically create users if they do not exist.",
-            optional: true,
-            type: bool,
-            default: false,
-        },
-        "username-claim": {
-            type: OpenIdUserAttribute,
-            optional: true,
-        },
-    },
-)]
-#[derive(Deserialize, Serialize)]
-#[serde(rename_all = "kebab-case")]
-/// Extra Information about a realm
-pub struct ExtraRealmInfo {
-    #[serde(skip_serializing_if = "Option::is_none")]
-    pub issuer_url: Option<String>,
-    #[serde(skip_serializing_if = "Option::is_none")]
-    pub client_id: Option<String>,
-    #[serde(skip_serializing_if = "Option::is_none")]
-    pub client_key: Option<String>,
-    #[serde(skip_serializing_if = "Option::is_none")]
-    pub autocreate: Option<bool>,
-    #[serde(skip_serializing_if = "Option::is_none")]
-    pub username_claim: Option<OpenIdUserAttribute>,
-}
-
-#[api(
-    properties: {
-        "info": {
-            type: BasicRealmInfo,
-        },
-        "extra": {
-            type: ExtraRealmInfo,
-        },
-    },
-)]
-#[derive(Deserialize, Serialize)]
-#[serde(rename_all = "kebab-case")]
-/// Information about a realm
-pub struct RealmInfo {
-    #[serde(flatten)]
-    pub info: BasicRealmInfo,
-    #[serde(flatten)]
-    pub extra: ExtraRealmInfo,
-}
 
 #[api(
     returns: {
-- 
2.30.2

[pbs-devel] applied-series: [PATCH widget-toolkit/proxmox-backup] fix openid realm api usage

[Thomas Lamprecht]

On 12.07.21 09:48, Dominik Csapak wrote:
> i missed that we already have the openid realm api. not in the usual
> place but in '/config/access/openid'
> 
> this series adds the ability in the refactored widget to handle
> different urls (also with the type in the path) and reverts the
> unnecessary patches, so that we do not have a duplicate api
> 
> since the pve-manager patches were not applied yet (and would need
> some changes anyway), this should not impact manager
> 
> proxmox-widget-toolkit:
> 
> Dominik Csapak (2):
>   window/AuthEditBase: handle differenc config api urls
>   panel/AuthView: handle different baseUrls for configuring realms
> 
>  src/panel/AuthView.js      | 14 +++++++++++++-
>  src/window/AuthEditBase.js | 14 +++++++++++---
>  2 files changed, 24 insertions(+), 4 deletions(-)
> 
> proxmox-backup:
> 
> Dominik Csapak (5):
>   ui: panel/AccessControl: define baseUrland useTypeInUrl for AuthView
>   api: config: access: openid: use correct parameter for matching
>   api: config: access: openid: use better Privilige Realm.Allocate
>   Revert "api: access: domains: add get/create/update/delete domain
>     call"
>   Revert "api: access: domains: add ExtraRealmInfo and RealmInfo
>     structs"
> 
>  src/api2/access/domain.rs        | 344 +------------------------------
>  src/api2/config/access/openid.rs |  12 +-
>  www/panel/AccessControl.js       |   2 +
>  3 files changed, 11 insertions(+), 347 deletions(-)
> 



applied series, thanks!