[pbs-devel] [PATCH proxmox-backup/pwt 0/14] fix #5379: introduce default auth realm option

[pbs-devel] [PATCH proxmox-backup/pwt 0/14] fix #5379: introduce default auth realm option

[Christoph Heiss]

Fixes #5379 [0].

First, it adds an updatable `default` field to all existing editable
realms. Then it converts the PAM and PBS built-in realms to proper
realms, instead of being hard-coded in-between somewhere. 
In turns this enables editing of these realms, allowing setting whether
these realms should be the default for login or not.

For proxmox-widget-toolkit, the first four patches could in principal be
applied on their own. The others depend on the API changes as introduced
in the proxmox-backup part.

W.r.t. to applying, proxmox-backup will need a bump of
proxmox-widget-toolkit afterwards.

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=5379

proxmox-widget-toolkit:

Christoph Heiss (6):
  window: AuthEditBase: include more information in thrown errors
  panel: AuthView: make `useTypeInUrl` property per-realm
  window: add panel for editing simple, built-in realms
  schema: make PAM realm editable using new AuthSimple panel
  fix #5379: panel: AuthView: add column displaying whether the realm is
    default
  fix #5379: window: AuthEdit{LDAP,OpenId}: add 'Default realm' checkbox

 src/Makefile                 |  1 +
 src/Schema.js                |  7 +++++-
 src/panel/AuthView.js        | 12 ++++++++---
 src/window/AuthEditBase.js   |  8 +++----
 src/window/AuthEditLDAP.js   | 14 +++++++++++-
 src/window/AuthEditOpenId.js | 13 ++++++++++++
 src/window/AuthEditSimple.js | 41 ++++++++++++++++++++++++++++++++++++
 7 files changed, 87 insertions(+), 9 deletions(-)
 create mode 100644 src/window/AuthEditSimple.js

proxmox-backup:

Christoph Heiss (8):
  fix #5379: api-types: add `default` field to all realm types
  fix #5379: api2: access: set default realm accordingly on individual
    update
  api-types: introduce proper types for PAM and PBS realms
  config: use new dedicated PAM and PBS realm types
  api2: access: add update support for built-in PAM realm
  api2: access: add update support for built-in PBS realm
  www: AccessControl: make `useTypeInUrl` property per-realm
  www: utils: make built-in pbs realm editable using new AuthSimplePanel

 pbs-api-types/src/ad.rs          |   7 ++
 pbs-api-types/src/ldap.rs        |   7 ++
 pbs-api-types/src/lib.rs         |  76 ++++++++++++++++++
 pbs-api-types/src/openid.rs      |   7 ++
 pbs-config/src/domains.rs        |  31 +++++++-
 src/api2/access/domain.rs        |  13 ----
 src/api2/config/access/ad.rs     |   7 ++
 src/api2/config/access/ldap.rs   |   7 ++
 src/api2/config/access/mod.rs    |   4 +
 src/api2/config/access/openid.rs |   7 ++
 src/api2/config/access/pam.rs    | 130 +++++++++++++++++++++++++++++++
 src/api2/config/access/pbs.rs    | 130 +++++++++++++++++++++++++++++++
 src/bin/proxmox-backup-api.rs    |   1 +
 src/config/mod.rs                |  34 ++++++++
 www/Utils.js                     |   4 +-
 www/panel/AccessControl.js       |   1 -
 16 files changed, 449 insertions(+), 17 deletions(-)
 create mode 100644 src/api2/config/access/pam.rs
 create mode 100644 src/api2/config/access/pbs.rs

-- 
2.45.1



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH widget-toolkit 1/6] window: AuthEditBase: include more information in thrown errors

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 src/window/AuthEditBase.js | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/window/AuthEditBase.js b/src/window/AuthEditBase.js
index 0f272e6..be547f9 100644
--- a/src/window/AuthEditBase.js
+++ b/src/window/AuthEditBase.js
@@ -29,9 +29,9 @@ Ext.define('Proxmox.window.AuthEditBase', {
 
 	let authConfig = Proxmox.Schema.authDomains[me.authType];
 	if (!authConfig) {
-	    throw 'unknown auth type';
+	    throw `unknown auth type ${me.authType}`;
 	} else if (!authConfig.add && me.isCreate) {
-	    throw 'trying to add non addable realm';
+	    throw `trying to add non addable realm of type ${me.authType}`;
 	}
 
 	me.subject = authConfig.name;
@@ -86,9 +86,9 @@ Ext.define('Proxmox.window.AuthEditBase', {
 		    var data = response.result.data || {};
 		    // just to be sure (should not happen)
 		    // only check this when the type is not in the api path
-		    if (!me.useTypeInUrl && data.type !== me.authType) {
+		    if (!me.useTypeInUrl && data.realm !== me.authType) {
 			me.close();
-			throw "got wrong auth type";
+			throw `got wrong auth type '${me.authType}' for realm '${data.realm}'`;
 		    }
 		    me.setValues(data);
 		},
-- 
2.45.1



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH widget-toolkit 2/6] panel: AuthView: make `useTypeInUrl` property per-realm

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 src/Schema.js         | 4 ++++
 src/panel/AuthView.js | 5 ++---
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/Schema.js b/src/Schema.js
index 42541e0..6921986 100644
--- a/src/Schema.js
+++ b/src/Schema.js
@@ -8,6 +8,7 @@ Ext.define('Proxmox.Schema', { // a singleton
 	    edit: false,
 	    pwchange: true,
 	    sync: false,
+	    useTypeInUrl: false,
 	},
 	openid: {
 	    name: gettext('OpenID Connect Server'),
@@ -18,6 +19,7 @@ Ext.define('Proxmox.Schema', { // a singleton
 	    pwchange: false,
 	    sync: false,
 	    iconCls: 'pmx-itype-icon-openid-logo',
+	    useTypeInUrl: true,
 	},
 	ldap: {
 	    name: gettext('LDAP Server'),
@@ -28,6 +30,7 @@ Ext.define('Proxmox.Schema', { // a singleton
 	    tfa: true,
 	    pwchange: false,
 	    sync: true,
+	    useTypeInUrl: true,
 	},
 	ad: {
 	    name: gettext('Active Directory Server'),
@@ -38,6 +41,7 @@ Ext.define('Proxmox.Schema', { // a singleton
 	    tfa: true,
 	    pwchange: false,
 	    sync: true,
+	    useTypeInUrl: true,
 	},
     },
     // to add or change existing for product specific ones
diff --git a/src/panel/AuthView.js b/src/panel/AuthView.js
index 52b6cac..944a812 100644
--- a/src/panel/AuthView.js
+++ b/src/panel/AuthView.js
@@ -11,7 +11,6 @@ Ext.define('Proxmox.panel.AuthView', {
     },
 
     baseUrl: '/access/domains',
-    useTypeInUrl: false,
 
     columns: [
 	{
@@ -47,7 +46,7 @@ Ext.define('Proxmox.panel.AuthView', {
 	let me = this;
 	Ext.create('Proxmox.window.AuthEditBase', {
 	    baseUrl: me.baseUrl,
-	    useTypeInUrl: me.useTypeInUrl,
+	    useTypeInUrl: Proxmox.Schema.authDomains[authType].useTypeInUrl,
 	    authType,
 	    realm,
 	    listeners: {
@@ -123,7 +122,7 @@ Ext.define('Proxmox.panel.AuthView', {
 		xtype: 'proxmoxStdRemoveButton',
 		getUrl: (rec) => {
 		    let url = me.baseUrl;
-		    if (me.useTypeInUrl) {
+		    if (Proxmox.Schema.authDomains[rec.data.type].useTypeInUrl) {
 			url += `/${rec.get('type')}`;
 		    }
 		    url += `/${rec.getId()}`;
-- 
2.45.1



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH widget-toolkit 3/6] window: add panel for editing simple, built-in realms

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 src/Makefile                 |  1 +
 src/window/AuthEditSimple.js | 41 ++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)
 create mode 100644 src/window/AuthEditSimple.js

diff --git a/src/Makefile b/src/Makefile
index 0478251..20ba77b 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -97,6 +97,7 @@ JSSRC=					\
 	window/AuthEditOpenId.js	\
 	window/AuthEditLDAP.js		\
 	window/AuthEditAD.js		\
+	window/AuthEditSimple.js        \
 	window/TfaWindow.js		\
 	window/AddTfaRecovery.js	\
 	window/AddTotp.js		\
diff --git a/src/window/AuthEditSimple.js b/src/window/AuthEditSimple.js
new file mode 100644
index 0000000..22932c0
--- /dev/null
+++ b/src/window/AuthEditSimple.js
@@ -0,0 +1,41 @@
+Ext.define('Proxmox.panel.SimpleRealmInputPanel', {
+    extend: 'Proxmox.panel.InputPanel',
+    xtype: 'pmxAuthSimplePanel',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    type: 'simple',
+
+    column1: [
+	{
+	    xtype: 'pmxDisplayEditField',
+	    name: 'realm',
+	    cbind: {
+		value: '{realm}',
+	    },
+	    fieldLabel: gettext('Realm'),
+	    allowBlank: false,
+	},
+	{
+	    xtype: 'proxmoxcheckbox',
+	    fieldLabel: gettext('Default realm'),
+	    name: 'default',
+	    value: 0,
+	    autoEl: {
+		tag: 'div',
+		'data-qtip': gettext('Set realm as default for login'),
+	    },
+	},
+    ],
+
+    column2: [
+    ],
+
+    columnB: [
+	{
+	    xtype: 'proxmoxtextfield',
+	    name: 'comment',
+	    fieldLabel: gettext('Comment'),
+	},
+    ],
+});
+
-- 
2.45.1



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH widget-toolkit 4/6] schema: make PAM realm editable using new AuthSimple panel

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 src/Schema.js | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/Schema.js b/src/Schema.js
index 6921986..3a84818 100644
--- a/src/Schema.js
+++ b/src/Schema.js
@@ -4,8 +4,9 @@ Ext.define('Proxmox.Schema', { // a singleton
     authDomains: {
 	pam: {
 	    name: 'Linux PAM',
+	    ipanel: 'pmxAuthSimplePanel',
 	    add: false,
-	    edit: false,
+	    edit: true,
 	    pwchange: true,
 	    sync: false,
 	    useTypeInUrl: false,
-- 
2.45.1



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH widget-toolkit 5/6] fix #5379: panel: AuthView: add column displaying whether the realm is default

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 src/panel/AuthView.js | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/panel/AuthView.js b/src/panel/AuthView.js
index 944a812..128d863 100644
--- a/src/panel/AuthView.js
+++ b/src/panel/AuthView.js
@@ -25,6 +25,13 @@ Ext.define('Proxmox.panel.AuthView', {
 	    sortable: true,
 	    dataIndex: 'type',
 	},
+	{
+	    header: gettext('Default'),
+	    width: 80,
+	    sortable: true,
+	    dataIndex: 'default',
+	    renderer: isDefault => isDefault ? Proxmox.Utils.renderEnabledIcon(true) : '',
+	},
 	{
 	    header: gettext('Comment'),
 	    sortable: false,
-- 
2.45.1



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH widget-toolkit 6/6] fix #5379: window: AuthEdit{LDAP, OpenId}: add 'Default realm' checkbox

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 src/window/AuthEditLDAP.js   | 14 +++++++++++++-
 src/window/AuthEditOpenId.js | 13 +++++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/src/window/AuthEditLDAP.js b/src/window/AuthEditLDAP.js
index 388fc02..4cd1020 100644
--- a/src/window/AuthEditLDAP.js
+++ b/src/window/AuthEditLDAP.js
@@ -82,6 +82,19 @@ Ext.define('Proxmox.panel.LDAPInputPanel', {
 	    fieldLabel: gettext('Realm'),
 	    allowBlank: false,
 	},
+	{
+	    xtype: 'proxmoxcheckbox',
+	    fieldLabel: gettext('Default realm'),
+	    name: 'default',
+	    value: 0,
+	    cbind: {
+		deleteEmpty: '{!isCreate}',
+	    },
+	    autoEl: {
+		tag: 'div',
+		'data-qtip': gettext('Set realm as default for login'),
+	    },
+	},
 	{
 	    xtype: 'proxmoxtextfield',
 	    fieldLabel: gettext('Base Domain Name'),
@@ -216,7 +229,6 @@ Ext.define('Proxmox.panel.LDAPInputPanel', {
 	    },
 	},
     ],
-
 });
 
 
diff --git a/src/window/AuthEditOpenId.js b/src/window/AuthEditOpenId.js
index 08ced99..a9ccb92 100644
--- a/src/window/AuthEditOpenId.js
+++ b/src/window/AuthEditOpenId.js
@@ -35,6 +35,19 @@ Ext.define('Proxmox.panel.OpenIDInputPanel', {
 	    fieldLabel: gettext('Realm'),
 	    allowBlank: false,
 	},
+	{
+	    xtype: 'proxmoxcheckbox',
+	    fieldLabel: gettext('Default realm'),
+	    name: 'default',
+	    value: 0,
+	    cbind: {
+		deleteEmpty: '{!isCreate}',
+	    },
+	    autoEl: {
+		tag: 'div',
+		'data-qtip': gettext('Set realm as default for login'),
+	    },
+	},
 	{
 	    xtype: 'proxmoxtextfield',
 	    fieldLabel: gettext('Client ID'),
-- 
2.45.1



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup 07/14] fix #5379: api-types: add `default` field to all realm types

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 pbs-api-types/src/ad.rs     | 7 +++++++
 pbs-api-types/src/ldap.rs   | 7 +++++++
 pbs-api-types/src/openid.rs | 7 +++++++
 3 files changed, 21 insertions(+)

diff --git a/pbs-api-types/src/ad.rs b/pbs-api-types/src/ad.rs
index 910571a0..5c236325 100644
--- a/pbs-api-types/src/ad.rs
+++ b/pbs-api-types/src/ad.rs
@@ -16,6 +16,10 @@ use super::{
             optional: true,
             schema: SINGLE_LINE_COMMENT_SCHEMA,
         },
+        "default": {
+            optional: true,
+            default: false,
+        },
         "verify": {
             optional: true,
             default: false,
@@ -64,6 +68,9 @@ pub struct AdRealmConfig {
     /// Comment
     #[serde(skip_serializing_if = "Option::is_none")]
     pub comment: Option<String>,
+    /// True if it should be the default realm to login in
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub default: Option<bool>,
     /// Connection security
     #[serde(skip_serializing_if = "Option::is_none")]
     pub mode: Option<LdapMode>,
diff --git a/pbs-api-types/src/ldap.rs b/pbs-api-types/src/ldap.rs
index f3df90a0..19d2cabb 100644
--- a/pbs-api-types/src/ldap.rs
+++ b/pbs-api-types/src/ldap.rs
@@ -29,6 +29,10 @@ pub enum LdapMode {
             optional: true,
             schema: SINGLE_LINE_COMMENT_SCHEMA,
         },
+        "default": {
+            optional: true,
+            default: false,
+        },
         "verify": {
             optional: true,
             default: false,
@@ -75,6 +79,9 @@ pub struct LdapRealmConfig {
     /// Comment
     #[serde(skip_serializing_if = "Option::is_none")]
     pub comment: Option<String>,
+    /// True if it should be the default realm to login in
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub default: Option<bool>,
     /// Connection security
     #[serde(skip_serializing_if = "Option::is_none")]
     pub mode: Option<LdapMode>,
diff --git a/pbs-api-types/src/openid.rs b/pbs-api-types/src/openid.rs
index 2c95c5c6..e8ec19d9 100644
--- a/pbs-api-types/src/openid.rs
+++ b/pbs-api-types/src/openid.rs
@@ -80,6 +80,10 @@ pub const OPENID_USERNAME_CLAIM_SCHEMA: Schema = StringSchema::new(
             optional: true,
             schema: SINGLE_LINE_COMMENT_SCHEMA,
         },
+        "default": {
+            optional: true,
+            default: false,
+        },
         autocreate: {
             optional: true,
             default: false,
@@ -111,6 +115,9 @@ pub struct OpenIdRealmConfig {
     pub client_key: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub comment: Option<String>,
+    /// True if it should be the default realm to login in
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub default: Option<bool>,
     /// Automatically create users if they do not exist.
     #[serde(skip_serializing_if = "Option::is_none")]
     pub autocreate: Option<bool>,
-- 
2.45.1



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup 08/14] fix #5379: api2: access: set default realm accordingly on individual update

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 pbs-config/src/domains.rs        | 10 ++++++++++
 src/api2/config/access/ad.rs     |  7 +++++++
 src/api2/config/access/ldap.rs   |  7 +++++++
 src/api2/config/access/openid.rs |  7 +++++++
 4 files changed, 31 insertions(+)

diff --git a/pbs-config/src/domains.rs b/pbs-config/src/domains.rs
index 5b6ce480..d372e913 100644
--- a/pbs-config/src/domains.rs
+++ b/pbs-config/src/domains.rs
@@ -63,6 +63,16 @@ pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
     replace_backup_config(DOMAINS_CFG_FILENAME, raw.as_bytes())
 }
 
+pub fn unset_default_realm(config: &mut SectionConfigData) -> Result<(), Error> {
+    for (_, data) in &mut config.sections.values_mut() {
+        if let Some(obj) = data.as_object_mut() {
+            obj.remove("default");
+        }
+    }
+
+    Ok(())
+}
+
 /// Check if a realm with the given name exists
 pub fn exists(domains: &SectionConfigData, realm: &str) -> bool {
     realm == "pbs" || realm == "pam" || domains.sections.contains_key(realm)
diff --git a/src/api2/config/access/ad.rs b/src/api2/config/access/ad.rs
index c202291a..04b84136 100644
--- a/src/api2/config/access/ad.rs
+++ b/src/api2/config/access/ad.rs
@@ -273,6 +273,13 @@ pub async fn update_ad_realm(
         }
     }
 
+    if let Some(true) = update.default {
+        pbs_config::domains::unset_default_realm(&mut domains)?;
+        config.default = Some(true);
+    } else {
+        config.default = None;
+    }
+
     if let Some(mode) = update.mode {
         config.mode = Some(mode);
     }
diff --git a/src/api2/config/access/ldap.rs b/src/api2/config/access/ldap.rs
index e60dc9c1..ec89e068 100644
--- a/src/api2/config/access/ldap.rs
+++ b/src/api2/config/access/ldap.rs
@@ -312,6 +312,13 @@ pub fn update_ldap_realm(
         }
     }
 
+    if let Some(true) = update.default {
+        pbs_config::domains::unset_default_realm(&mut domains)?;
+        config.default = Some(true);
+    } else {
+        config.default = None;
+    }
+
     if let Some(mode) = update.mode {
         config.mode = Some(mode);
     }
diff --git a/src/api2/config/access/openid.rs b/src/api2/config/access/openid.rs
index 4901880e..77ff98a3 100644
--- a/src/api2/config/access/openid.rs
+++ b/src/api2/config/access/openid.rs
@@ -242,6 +242,13 @@ pub fn update_openid_realm(
         }
     }
 
+    if let Some(true) = update.default {
+        pbs_config::domains::unset_default_realm(&mut domains)?;
+        config.default = Some(true);
+    } else {
+        config.default = None;
+    }
+
     if let Some(issuer_url) = update.issuer_url {
         config.issuer_url = issuer_url;
     }
-- 
2.45.1



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup 09/14] api-types: introduce proper types for PAM and PBS realms

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 pbs-api-types/src/lib.rs | 76 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/pbs-api-types/src/lib.rs b/pbs-api-types/src/lib.rs
index 40bcd8f1..01f8eb74 100644
--- a/pbs-api-types/src/lib.rs
+++ b/pbs-api-types/src/lib.rs
@@ -1,6 +1,8 @@
 //! Basic API types used by most of the PBS code.
 
 use const_format::concatcp;
+use proxmox_schema::EnumEntry;
+use proxmox_schema::Updater;
 use serde::{Deserialize, Serialize};
 
 pub mod percent_encoding;
@@ -218,6 +220,20 @@ pub const REALM_ID_SCHEMA: Schema = StringSchema::new("Realm name.")
     .max_length(32)
     .schema();
 
+const PAM_REALM_ID_SCHEMA: Schema = StringSchema::new("Realm name.")
+    .format(&ApiStringFormat::Enum(&[EnumEntry::new(
+        "pam",
+        "Default PAM realm.",
+    )]))
+    .schema();
+
+const PBS_REALM_ID_SCHEMA: Schema = StringSchema::new("Realm name.")
+    .format(&ApiStringFormat::Enum(&[EnumEntry::new(
+        "pbs",
+        "Default PBS realm.",
+    )]))
+    .schema();
+
 pub const SUBSCRIPTION_KEY_SCHEMA: Schema =
     StringSchema::new("Proxmox Backup Server subscription key.")
         .format(&SUBSCRIPTION_KEY_FORMAT)
@@ -394,3 +410,63 @@ pub struct BasicRealmInfo {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub comment: Option<String>,
 }
+
+#[api(
+    properties: {
+        "realm": {
+            schema: PAM_REALM_ID_SCHEMA,
+        },
+        "comment": {
+            optional: true,
+            schema: SINGLE_LINE_COMMENT_SCHEMA,
+        },
+        "default": {
+            optional: true,
+            default: false,
+        },
+    }
+)]
+#[derive(Serialize, Deserialize, Updater, Clone)]
+#[serde(rename_all = "kebab-case")]
+/// Built-in PAM realm configuration properties.
+pub struct PamRealmConfig {
+    /// Realm name. Always "pam".
+    #[updater(skip)]
+    pub realm: String,
+    /// Comment for this realm
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub comment: Option<String>,
+    /// True if it should be the default realm to login in
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub default: Option<bool>,
+}
+
+#[api(
+    properties: {
+        "realm": {
+            schema: PBS_REALM_ID_SCHEMA,
+        },
+        "comment": {
+            optional: true,
+            schema: SINGLE_LINE_COMMENT_SCHEMA,
+        },
+        "default": {
+            optional: true,
+            default: false,
+        },
+    }
+)]
+#[derive(Serialize, Deserialize, Updater, Clone)]
+#[serde(rename_all = "kebab-case")]
+/// Built-in Proxmox Backup Server realm configuration properties.
+pub struct PbsRealmConfig {
+    /// Realm name. Always "pbs".
+    #[updater(skip)]
+    pub realm: String,
+    /// Comment for this realm
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub comment: Option<String>,
+    /// True if it should be the default realm to login in
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub default: Option<bool>,
+}
-- 
2.45.1



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup 10/14] config: use new dedicated PAM and PBS realm types

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 pbs-config/src/domains.rs     | 21 +++++++++++++++++++--
 src/api2/access/domain.rs     | 13 -------------
 src/bin/proxmox-backup-api.rs |  1 +
 src/config/mod.rs             | 34 ++++++++++++++++++++++++++++++++++
 4 files changed, 54 insertions(+), 15 deletions(-)

diff --git a/pbs-config/src/domains.rs b/pbs-config/src/domains.rs
index d372e913..4565c36a 100644
--- a/pbs-config/src/domains.rs
+++ b/pbs-config/src/domains.rs
@@ -8,19 +8,36 @@ use proxmox_schema::{ApiType, ObjectSchema};
 use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
 
 use crate::{open_backup_lockfile, replace_backup_config, BackupLockGuard};
-use pbs_api_types::{AdRealmConfig, LdapRealmConfig, OpenIdRealmConfig, REALM_ID_SCHEMA};
+use pbs_api_types::{
+    AdRealmConfig, LdapRealmConfig, OpenIdRealmConfig, PamRealmConfig, PbsRealmConfig,
+    REALM_ID_SCHEMA,
+};
 
 lazy_static! {
     pub static ref CONFIG: SectionConfig = init();
 }
 
 fn init() -> SectionConfig {
+    const PAM_SCHEMA: &ObjectSchema = PamRealmConfig::API_SCHEMA.unwrap_object_schema();
+    const PBS_SCHEMA: &ObjectSchema = PbsRealmConfig::API_SCHEMA.unwrap_object_schema();
     const AD_SCHEMA: &ObjectSchema = AdRealmConfig::API_SCHEMA.unwrap_object_schema();
     const LDAP_SCHEMA: &ObjectSchema = LdapRealmConfig::API_SCHEMA.unwrap_object_schema();
     const OPENID_SCHEMA: &ObjectSchema = OpenIdRealmConfig::API_SCHEMA.unwrap_object_schema();
 
     let mut config = SectionConfig::new(&REALM_ID_SCHEMA);
 
+    config.register_plugin(SectionConfigPlugin::new(
+        "pam".to_owned(),
+        Some("realm".to_owned()),
+        PAM_SCHEMA,
+    ));
+
+    config.register_plugin(SectionConfigPlugin::new(
+        "pbs".to_owned(),
+        Some("realm".to_owned()),
+        PBS_SCHEMA,
+    ));
+
     let plugin = SectionConfigPlugin::new(
         "openid".to_string(),
         Some(String::from("realm")),
@@ -75,7 +92,7 @@ pub fn unset_default_realm(config: &mut SectionConfigData) -> Result<(), Error>
 
 /// Check if a realm with the given name exists
 pub fn exists(domains: &SectionConfigData, realm: &str) -> bool {
-    realm == "pbs" || realm == "pam" || domains.sections.contains_key(realm)
+    domains.sections.contains_key(realm)
 }
 
 // shell completion helper
diff --git a/src/api2/access/domain.rs b/src/api2/access/domain.rs
index 8f8eebda..cede714a 100644
--- a/src/api2/access/domain.rs
+++ b/src/api2/access/domain.rs
@@ -29,19 +29,6 @@ use crate::server::jobstate::Job;
 /// Authentication domain/realm index.
 fn list_domains(rpcenv: &mut dyn RpcEnvironment) -> Result<Vec<BasicRealmInfo>, Error> {
     let mut list = Vec::new();
-
-    list.push(serde_json::from_value(json!({
-        "realm": "pam",
-        "type": "pam",
-        "comment": "Linux PAM standard authentication",
-        "default": Some(true),
-    }))?);
-    list.push(serde_json::from_value(json!({
-        "realm": "pbs",
-        "type": "pbs",
-        "comment": "Proxmox Backup authentication server",
-    }))?);
-
     let (config, digest) = pbs_config::domains::config()?;
 
     for (_, (section_type, v)) in config.sections.iter() {
diff --git a/src/bin/proxmox-backup-api.rs b/src/bin/proxmox-backup-api.rs
index 95c14e41..4caea8a6 100644
--- a/src/bin/proxmox-backup-api.rs
+++ b/src/bin/proxmox-backup-api.rs
@@ -46,6 +46,7 @@ async fn run() -> Result<(), Error> {
     config::create_configdir()?;
 
     config::update_self_signed_cert(false)?;
+    config::update_default_realms()?;
 
     proxmox_backup::server::create_run_dir()?;
     proxmox_backup::server::create_state_dir()?;
diff --git a/src/config/mod.rs b/src/config/mod.rs
index 324fabca..3931eee9 100644
--- a/src/config/mod.rs
+++ b/src/config/mod.rs
@@ -12,6 +12,7 @@ use std::path::Path;
 
 use proxmox_lang::try_block;
 
+use pbs_api_types::{PamRealmConfig, PbsRealmConfig};
 use pbs_buildcfg::{self, configdir};
 
 pub mod acme;
@@ -194,3 +195,36 @@ pub(crate) fn set_proxy_certificate(cert_pem: &[u8], key_pem: &[u8]) -> Result<(
 
     Ok(())
 }
+
+pub fn update_default_realms() -> Result<(), Error> {
+    let _lock = pbs_config::domains::lock_config()?;
+    let (mut domains, _) = pbs_config::domains::config()?;
+
+    if !pbs_config::domains::exists(&domains, "pam") {
+        domains.set_data(
+            "pam",
+            "pam",
+            PamRealmConfig {
+                realm: "pam".to_owned(),
+                comment: Some("Linux PAM standard authentication".to_owned()),
+                // Setting it as default here is safe, because if we perform this
+                // migration, the user had not had any chance to set a custom default anyway.
+                default: Some(true),
+            },
+        )?;
+    }
+
+    if !pbs_config::domains::exists(&domains, "pbs") {
+        domains.set_data(
+            "pbs",
+            "pbs",
+            PbsRealmConfig {
+                realm: "pbs".to_owned(),
+                comment: Some("Proxmox Backup authentication server".to_owned()),
+                default: None,
+            },
+        )?;
+    }
+
+    pbs_config::domains::save_config(&domains)
+}
-- 
2.45.1



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup 11/14] api2: access: add update support for built-in PAM realm

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 src/api2/config/access/mod.rs |   2 +
 src/api2/config/access/pam.rs | 130 ++++++++++++++++++++++++++++++++++
 2 files changed, 132 insertions(+)
 create mode 100644 src/api2/config/access/pam.rs

diff --git a/src/api2/config/access/mod.rs b/src/api2/config/access/mod.rs
index b551e662..36ecd005 100644
--- a/src/api2/config/access/mod.rs
+++ b/src/api2/config/access/mod.rs
@@ -5,10 +5,12 @@ use proxmox_sortable_macro::sortable;
 pub mod ad;
 pub mod ldap;
 pub mod openid;
+pub mod pam;
 pub mod tfa;
 
 #[sortable]
 const SUBDIRS: SubdirMap = &sorted!([
+    ("pam", &pam::ROUTER),
     ("ad", &ad::ROUTER),
     ("ldap", &ldap::ROUTER),
     ("openid", &openid::ROUTER),
diff --git a/src/api2/config/access/pam.rs b/src/api2/config/access/pam.rs
new file mode 100644
index 00000000..04ae616b
--- /dev/null
+++ b/src/api2/config/access/pam.rs
@@ -0,0 +1,130 @@
+use ::serde::{Deserialize, Serialize};
+use anyhow::Error;
+use hex::FromHex;
+
+use proxmox_router::{Permission, Router, RpcEnvironment};
+use proxmox_schema::api;
+
+use pbs_api_types::{
+    PamRealmConfig, PamRealmConfigUpdater, PRIV_REALM_ALLOCATE, PRIV_SYS_AUDIT,
+    PROXMOX_CONFIG_DIGEST_SCHEMA,
+};
+
+use pbs_config::domains;
+
+#[api(
+    returns: {
+        type: PamRealmConfig,
+    },
+    access: {
+        permission: &Permission::Privilege(&["access", "domains"], PRIV_SYS_AUDIT, false),
+    },
+)]
+/// Read the PAM realm configuration
+pub fn read_pam_realm(rpcenv: &mut dyn RpcEnvironment) -> Result<PamRealmConfig, Error> {
+    let (domains, digest) = domains::config()?;
+
+    let config = domains.lookup("pam", "pam")?;
+
+    rpcenv["digest"] = hex::encode(digest).into();
+
+    Ok(config)
+}
+
+#[api]
+#[derive(Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
+/// Deletable property name
+pub enum DeletableProperty {
+    /// Delete the comment property.
+    Comment,
+    /// Delete the default property.
+    Default,
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            update: {
+                type: PamRealmConfigUpdater,
+                flatten: 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: PamRealmConfig,
+    },
+    access: {
+        permission: &Permission::Privilege(&["access", "domains"], PRIV_REALM_ALLOCATE, false),
+    },
+)]
+/// Update the PAM realm configuration
+pub fn update_pam_realm(
+    update: PamRealmConfigUpdater,
+    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 = <[u8; 32]>::from_hex(digest)?;
+        crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?;
+    }
+
+    let mut config: PamRealmConfig = domains.lookup("pam", "pam")?;
+
+    if let Some(delete) = delete {
+        for delete_prop in delete {
+            match delete_prop {
+                DeletableProperty::Comment => {
+                    config.comment = None;
+                }
+                DeletableProperty::Default => {
+                    config.default = None;
+                }
+            }
+        }
+    }
+
+    if let Some(comment) = update.comment {
+        let comment = comment.trim().to_string();
+        if comment.is_empty() {
+            config.comment = None;
+        } else {
+            config.comment = Some(comment);
+        }
+    }
+
+    if let Some(true) = update.default {
+        pbs_config::domains::unset_default_realm(&mut domains)?;
+        config.default = Some(true);
+    } else {
+        config.default = None;
+    }
+
+    domains.set_data("pam", "pam", &config)?;
+
+    domains::save_config(&domains)?;
+
+    Ok(())
+}
+
+pub const ROUTER: Router = Router::new()
+    .get(&API_METHOD_READ_PAM_REALM)
+    .put(&API_METHOD_UPDATE_PAM_REALM);
-- 
2.45.1



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup 12/14] api2: access: add update support for built-in PBS realm

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 src/api2/config/access/mod.rs |   2 +
 src/api2/config/access/pbs.rs | 130 ++++++++++++++++++++++++++++++++++
 2 files changed, 132 insertions(+)
 create mode 100644 src/api2/config/access/pbs.rs

diff --git a/src/api2/config/access/mod.rs b/src/api2/config/access/mod.rs
index 36ecd005..1e6070c7 100644
--- a/src/api2/config/access/mod.rs
+++ b/src/api2/config/access/mod.rs
@@ -6,11 +6,13 @@ pub mod ad;
 pub mod ldap;
 pub mod openid;
 pub mod pam;
+pub mod pbs;
 pub mod tfa;
 
 #[sortable]
 const SUBDIRS: SubdirMap = &sorted!([
     ("pam", &pam::ROUTER),
+    ("pbs", &pbs::ROUTER),
     ("ad", &ad::ROUTER),
     ("ldap", &ldap::ROUTER),
     ("openid", &openid::ROUTER),
diff --git a/src/api2/config/access/pbs.rs b/src/api2/config/access/pbs.rs
new file mode 100644
index 00000000..2873eabb
--- /dev/null
+++ b/src/api2/config/access/pbs.rs
@@ -0,0 +1,130 @@
+use ::serde::{Deserialize, Serialize};
+use anyhow::Error;
+use hex::FromHex;
+
+use proxmox_router::{Permission, Router, RpcEnvironment};
+use proxmox_schema::api;
+
+use pbs_api_types::{
+    PbsRealmConfig, PbsRealmConfigUpdater, PRIV_REALM_ALLOCATE, PRIV_SYS_AUDIT,
+    PROXMOX_CONFIG_DIGEST_SCHEMA,
+};
+
+use pbs_config::domains;
+
+#[api(
+    returns: {
+        type: PbsRealmConfig,
+    },
+    access: {
+        permission: &Permission::Privilege(&["access", "domains"], PRIV_SYS_AUDIT, false),
+    },
+)]
+/// Read the Proxmox Backup authentication server realm configuration
+pub fn read_pbs_realm(rpcenv: &mut dyn RpcEnvironment) -> Result<PbsRealmConfig, Error> {
+    let (domains, digest) = domains::config()?;
+
+    let config = domains.lookup("pbs", "pbs")?;
+
+    rpcenv["digest"] = hex::encode(digest).into();
+
+    Ok(config)
+}
+
+#[api]
+#[derive(Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
+/// Deletable property name
+pub enum DeletableProperty {
+    /// Delete the comment property.
+    Comment,
+    /// Delete the default property.
+    Default,
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            update: {
+                type: PbsRealmConfigUpdater,
+                flatten: 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: PbsRealmConfig,
+    },
+    access: {
+        permission: &Permission::Privilege(&["access", "domains"], PRIV_REALM_ALLOCATE, false),
+    },
+)]
+/// Update the Proxmox Backup authentication server realm configuration
+pub fn update_pbs_realm(
+    update: PbsRealmConfigUpdater,
+    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 = <[u8; 32]>::from_hex(digest)?;
+        crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?;
+    }
+
+    let mut config: PbsRealmConfig = domains.lookup("pbs", "pbs")?;
+
+    if let Some(delete) = delete {
+        for delete_prop in delete {
+            match delete_prop {
+                DeletableProperty::Comment => {
+                    config.comment = None;
+                }
+                DeletableProperty::Default => {
+                    config.default = None;
+                }
+            }
+        }
+    }
+
+    if let Some(comment) = update.comment {
+        let comment = comment.trim().to_string();
+        if comment.is_empty() {
+            config.comment = None;
+        } else {
+            config.comment = Some(comment);
+        }
+    }
+
+    if let Some(true) = update.default {
+        pbs_config::domains::unset_default_realm(&mut domains)?;
+        config.default = Some(true);
+    } else {
+        config.default = None;
+    }
+
+    domains.set_data("pbs", "pbs", &config)?;
+
+    domains::save_config(&domains)?;
+
+    Ok(())
+}
+
+pub const ROUTER: Router = Router::new()
+    .get(&API_METHOD_READ_PBS_REALM)
+    .put(&API_METHOD_UPDATE_PBS_REALM);
-- 
2.45.1



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup 13/14] www: AccessControl: make `useTypeInUrl` property per-realm

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 www/Utils.js               | 1 +
 www/panel/AccessControl.js | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/www/Utils.js b/www/Utils.js
index 4853be36..f6688ca4 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -459,6 +459,7 @@ Ext.define('PBS.Utils', {
 		edit: false,
 		pwchange: true,
 		sync: false,
+		useTypeInUrl: false,
 	    },
 	});
 
diff --git a/www/panel/AccessControl.js b/www/panel/AccessControl.js
index d10d0891..4910510e 100644
--- a/www/panel/AccessControl.js
+++ b/www/panel/AccessControl.js
@@ -40,7 +40,6 @@ Ext.define('PBS.AccessControlPanel', {
 	{
 	    xtype: 'pmxAuthView',
 	    baseUrl: '/config/access',
-	    useTypeInUrl: true,
 	    title: gettext('Realms'),
 	    itemId: 'domains',
 	    iconCls: 'fa fa-address-book-o',
-- 
2.45.1



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup 14/14] www: utils: make built-in pbs realm editable using new AuthSimplePanel

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 www/Utils.js | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/www/Utils.js b/www/Utils.js
index f6688ca4..15724958 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -455,8 +455,9 @@ Ext.define('PBS.Utils', {
 	Proxmox.Schema.overrideAuthDomains({
 	    pbs: {
 		name: 'Proxmox Backup authentication server',
+		ipanel: 'pmxAuthSimplePanel',
 		add: false,
-		edit: false,
+		edit: true,
 		pwchange: true,
 		sync: false,
 		useTypeInUrl: false,
-- 
2.45.1



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox-backup/pwt 0/14] fix #5379: introduce default auth realm option

[Lukas Wagner]

On  2024-07-16 15:44, Christoph Heiss wrote:
> Fixes #5379 [0].
> 
> First, it adds an updatable `default` field to all existing editable
> realms. Then it converts the PAM and PBS built-in realms to proper
> realms, instead of being hard-coded in-between somewhere. 
> In turns this enables editing of these realms, allowing setting whether
> these realms should be the default for login or not.
> 
> For proxmox-widget-toolkit, the first four patches could in principal be
> applied on their own. The others depend on the API changes as introduced
> in the proxmox-backup part.
> 
> W.r.t. to applying, proxmox-backup will need a bump of
> proxmox-widget-toolkit afterwards.

Codewise it looks good (apart from some tiny tiny nits), so consider this:

Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>

I also tested this on the latest master branch. Here is what I did:
 - Set the PBS realm as default
 - Add a new user to the PBS realm, for good measure
 - Logout
 - Make sure PBS is selected on the login page

I found that once you select PAM again and log in as root, the PAM realm will
be selected on next login again (even though 'save user' is not selected).
This seems to happen because the ext-pbs-pveloginrealm:"o%3Avalue%3Ds%253Apam"
key is set in local storage. If that one is cleared, it works again.
So it seems like we are interfering with the statefulness of the login widget.

Could you check if you can reproduce that?

-- 
- Lukas


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH widget-toolkit 5/6] fix #5379: panel: AuthView: add column displaying whether the realm is default

[Lukas Wagner]

On  2024-07-16 15:45, Christoph Heiss wrote:
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
>  src/panel/AuthView.js | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/src/panel/AuthView.js b/src/panel/AuthView.js
> index 944a812..128d863 100644
> --- a/src/panel/AuthView.js
> +++ b/src/panel/AuthView.js
> @@ -25,6 +25,13 @@ Ext.define('Proxmox.panel.AuthView', {
>  	    sortable: true,
>  	    dataIndex: 'type',
>  	},
> +	{
> +	    header: gettext('Default'),
> +	    width: 80,
> +	    sortable: true,
> +	    dataIndex: 'default',
> +	    renderer: isDefault => isDefault ? Proxmox.Utils.renderEnabledIcon(true) : '',
> +	},
>  	{
>  	    header: gettext('Comment'),
>  	    sortable: false,

I think the check mark should be centered in the column, see e.g. the enabled
column for notification targets/matchers. Looks a bit more consistent then 😄.

-- 
- Lukas


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox-backup 09/14] api-types: introduce proper types for PAM and PBS realms

[Lukas Wagner]

On  2024-07-16 15:45, Christoph Heiss wrote:
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
>  pbs-api-types/src/lib.rs | 76 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 76 insertions(+)
> 
> diff --git a/pbs-api-types/src/lib.rs b/pbs-api-types/src/lib.rs
> index 40bcd8f1..01f8eb74 100644
> --- a/pbs-api-types/src/lib.rs
> +++ b/pbs-api-types/src/lib.rs
> @@ -1,6 +1,8 @@
>  //! Basic API types used by most of the PBS code.
>  
>  use const_format::concatcp;
> +use proxmox_schema::EnumEntry;
> +use proxmox_schema::Updater;

Nit: proxmox_* imports should be separate from third-party imports

>  use serde::{Deserialize, Serialize};
>  
>  pub mod percent_encoding;
> @@ -218,6 +220,20 @@ pub const REALM_ID_SCHEMA: Schema = StringSchema::new("Realm name.")
>      .max_length(32)
>      .schema();
>  
> +const PAM_REALM_ID_SCHEMA: Schema = StringSchema::new("Realm name.")
> +    .format(&ApiStringFormat::Enum(&[EnumEntry::new(
> +        "pam",
> +        "Default PAM realm.",
> +    )]))
> +    .schema();
> +
> +const PBS_REALM_ID_SCHEMA: Schema = StringSchema::new("Realm name.")
> +    .format(&ApiStringFormat::Enum(&[EnumEntry::new(
> +        "pbs",
> +        "Default PBS realm.",
> +    )]))
> +    .schema();
> +
>  pub const SUBSCRIPTION_KEY_SCHEMA: Schema =
>      StringSchema::new("Proxmox Backup Server subscription key.")
>          .format(&SUBSCRIPTION_KEY_FORMAT)
> @@ -394,3 +410,63 @@ pub struct BasicRealmInfo {
>      #[serde(skip_serializing_if = "Option::is_none")]
>      pub comment: Option<String>,
>  }
> +
> +#[api(
> +    properties: {
> +        "realm": {
> +            schema: PAM_REALM_ID_SCHEMA,
> +        },
> +        "comment": {
> +            optional: true,
> +            schema: SINGLE_LINE_COMMENT_SCHEMA,
> +        },
> +        "default": {
> +            optional: true,
> +            default: false,
> +        },
> +    }
> +)]
> +#[derive(Serialize, Deserialize, Updater, Clone)]
> +#[serde(rename_all = "kebab-case")]
> +/// Built-in PAM realm configuration properties.
> +pub struct PamRealmConfig {
> +    /// Realm name. Always "pam".
> +    #[updater(skip)]
> +    pub realm: String,
> +    /// Comment for this realm
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub comment: Option<String>,
> +    /// True if it should be the default realm to login in
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub default: Option<bool>,
> +}
> +
> +#[api(
> +    properties: {
> +        "realm": {
> +            schema: PBS_REALM_ID_SCHEMA,
> +        },
> +        "comment": {
> +            optional: true,
> +            schema: SINGLE_LINE_COMMENT_SCHEMA,
> +        },
> +        "default": {
> +            optional: true,
> +            default: false,
> +        },
> +    }
> +)]
> +#[derive(Serialize, Deserialize, Updater, Clone)]
> +#[serde(rename_all = "kebab-case")]
> +/// Built-in Proxmox Backup Server realm configuration properties.
> +pub struct PbsRealmConfig {
> +    /// Realm name. Always "pbs".
> +    #[updater(skip)]
> +    pub realm: String,
> +    /// Comment for this realm
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub comment: Option<String>,
> +    /// True if it should be the default realm to login in
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub default: Option<bool>,
> +}

-- 
- Lukas


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox-backup 10/14] config: use new dedicated PAM and PBS realm types

[Lukas Wagner]

On  2024-07-16 15:45, Christoph Heiss wrote:
> diff --git a/src/config/mod.rs b/src/config/mod.rs
> index 324fabca..3931eee9 100644
> --- a/src/config/mod.rs
> +++ b/src/config/mod.rs
> @@ -12,6 +12,7 @@ use std::path::Path;
>  
>  use proxmox_lang::try_block;
>  
> +use pbs_api_types::{PamRealmConfig, PbsRealmConfig};
>  use pbs_buildcfg::{self, configdir};
>  
>  pub mod acme;
> @@ -194,3 +195,36 @@ pub(crate) fn set_proxy_certificate(cert_pem: &[u8], key_pem: &[u8]) -> Result<(
>  
>      Ok(())
>  }
> +
> +pub fn update_default_realms() -> Result<(), Error> {
> +    let _lock = pbs_config::domains::lock_config()?;
> +    let (mut domains, _) = pbs_config::domains::config()?;
> +
> +    if !pbs_config::domains::exists(&domains, "pam") {
> +        domains.set_data(
> +            "pam",
> +            "pam",
> +            PamRealmConfig {
> +                realm: "pam".to_owned(),
> +                comment: Some("Linux PAM standard authentication".to_owned()),
> +                // Setting it as default here is safe, because if we perform this
> +                // migration, the user had not had any chance to set a custom default anyway.
> +                default: Some(true),
> +            },
> +        )?;
> +    }
> +
> +    if !pbs_config::domains::exists(&domains, "pbs") {
> +        domains.set_data(
> +            "pbs",
> +            "pbs",
> +            PbsRealmConfig {
> +                realm: "pbs".to_owned(),
> +                comment: Some("Proxmox Backup authentication server".to_owned()),
> +                default: None,
> +            },
> +        )?;
> +    }

Just wondering, would it be a good idea to encode these defaults as
the `Default::default()` impl for these two types? What do you think?

> +
> +    pbs_config::domains::save_config(&domains)
> +}

-- 
- Lukas


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox-backup 10/14] config: use new dedicated PAM and PBS realm types

[Christoph Heiss]

Thanks for the review!

On Wed, Aug 07, 2024 at 11:24:19AM GMT, Lukas Wagner wrote:
>
> On  2024-07-16 15:45, Christoph Heiss wrote:
> > diff --git a/src/config/mod.rs b/src/config/mod.rs
> > index 324fabca..3931eee9 100644
> > --- a/src/config/mod.rs
> > +++ b/src/config/mod.rs
> > [..]
> > +pub fn update_default_realms() -> Result<(), Error> {
> > [..]
> > +    if !pbs_config::domains::exists(&domains, "pbs") {
> > +        domains.set_data(
> > +            "pbs",
> > +            "pbs",
> > +            PbsRealmConfig {
> > +                realm: "pbs".to_owned(),
> > +                comment: Some("Proxmox Backup authentication server".to_owned()),
> > +                default: None,
> > +            },
> > +        )?;
> > +    }
>
> Just wondering, would it be a good idea to encode these defaults as
> the `Default::default()` impl for these two types? What do you think?

Yep, sounds good to me, TBH. I'll change that in v2.


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox-backup/pwt 0/14] fix #5379: introduce default auth realm option

[Christoph Heiss]

Thanks for the review & testing!

On Wed, Aug 07, 2024 at 11:23:55AM GMT, Lukas Wagner wrote:
> [..]
> I found that once you select PAM again and log in as root, the PAM realm will
> be selected on next login again (even though 'save user' is not selected).
> This seems to happen because the ext-pbs-pveloginrealm:"o%3Avalue%3Ds%253Apam"
> key is set in local storage. If that one is cleared, it works again.
> So it seems like we are interfering with the statefulness of the login widget.
>
> Could you check if you can reproduce that?

Also, thanks for the investigation! I'll check if I can reproduce it,
the fix for it seems to be doable in a sensible way by clearing the key.



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox-backup/pwt 0/14] fix #5379: introduce default auth realm option

[Christoph Heiss]

On Wed, Aug 07, 2024 at 05:27:42PM GMT, Christoph Heiss wrote:
> Thanks for the review & testing!
>
> On Wed, Aug 07, 2024 at 11:23:55AM GMT, Lukas Wagner wrote:
> > [..]
> > I found that once you select PAM again and log in as root, the PAM realm will
> > be selected on next login again (even though 'save user' is not selected).
> > This seems to happen because the ext-pbs-pveloginrealm:"o%3Avalue%3Ds%253Apam"
> > key is set in local storage. If that one is cleared, it works again.
> > So it seems like we are interfering with the statefulness of the login widget.
> >
> > Could you check if you can reproduce that?
>
> Also, thanks for the investigation! I'll check if I can reproduce it,
> the fix for it seems to be doable in a sensible way by clearing the key.
>

Following up on this: PVE has the exact same behaviour. The login dialog
component (`Proxmox.form.RealmComboBox`, living in
proxmox-widget-toolkit) is in fact reused between both frontends.

PVE uses the key `ext-pveloginrealm` in local storage, PBS
`ext-pbs-pveloginrealm`.

So IMHO either we should keep the behaviour as-is or change it for both,
the latter preferably as a separate changeset.

I would even argue to keep the behaviour, since it makes sense to me -
if a user once logs in using a specific realm, he will probably want to
*always* log in with that realm.

What do you think?


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox-backup/pwt 0/14] fix #5379: introduce default auth realm option

[Lukas Wagner]

On  2024-08-08 16:25, Christoph Heiss wrote:
> Following up on this: PVE has the exact same behaviour. The login dialog
> component (`Proxmox.form.RealmComboBox`, living in
> proxmox-widget-toolkit) is in fact reused between both frontends.

ah yeah, I think I tried to reproduce this on PVE *before* I fully realized
to exact steps to trigger it...

> 
> PVE uses the key `ext-pveloginrealm` in local storage, PBS
> `ext-pbs-pveloginrealm`.
> 
> So IMHO either we should keep the behaviour as-is or change it for both,
> the latter preferably as a separate changeset.
> 
> I would even argue to keep the behaviour, since it makes sense to me -
> if a user once logs in using a specific realm, he will probably want to
> *always* log in with that realm.
> 
> What do you think?

Intuitively I'd expect this feature to work the other way round:
  - If a default is set, select that one
  - If not, take the one that was previously selected
  - fallback to PAM

The current behavior definitely took my by surprise.

But I think this is a matter of taste, so other opinions would
be valuable here as well. Whichever approach we choose, a short
paragraph in the docs would definitely be a good idea :)

-- 
- Lukas


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox-backup/pwt 0/14] fix #5379: introduce default auth realm option

[Christoph Heiss]

v2 out: https://lists.proxmox.com/pipermail/pbs-devel/2024-August/010521.html


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel