* [pmg-devel] [PATCH pve-common/perl-rs/pmg-api/widget-toolkit/pmg-gui v5 0/10] fix #3892: OpenID Connect
@ 2025-02-18 16:18 Markus Frank
2025-02-18 16:18 ` [pmg-devel] [PATCH pve-common v5 1/10] add Schema package with auth module that contains realm sync options Markus Frank
` (10 more replies)
0 siblings, 11 replies; 27+ messages in thread
From: Markus Frank @ 2025-02-18 16:18 UTC (permalink / raw)
To: pmg-devel
Patch-series to enable OpenID Connect Login for PMG
apply/compile order:
pve-common:
1 add Schema package with auth module that contains realm sync options
proxmox-perl-rs:
2 move openid code from pve-rs to common
3 remove empty PMG::RS::OpenId package to avoid confusion
pmg-api:
4 config: add plugin system for realms
5 config: add oidc type realm
6 api: add/update/remove realms like in PVE
7 api: oidc login similar to PVE
proxmox-widget-toolkit:
8 fix: window: AuthEditBase: rename variable 'realm' to 'type'
pmg-gui:
9 login: add option to login with OIDC realm
10 add panel for realms to User Management
v5:
* renamed openid/OpenId variables, filenames and modules to oidc/OIDC
wherever possible
* renamed Authdomains to Realm
v4:
* split "config: add plugin system for realms & add openid type realms"
patch into two patches
* use the name 'OpenId' for filenames, but use 'OIDC' as realm type name
* added autocreate-role option to set the role for automatically created
users in a realm, but currently not exposed in GUI (needs a lot of
changes in pmg-gui and proxmox-widget-toolkit)
pve-common:
Markus Frank (1):
add Schema package with auth module that contains realm sync options
src/Makefile | 2 ++
src/PVE/Schema/Auth.pm | 82 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 84 insertions(+)
create mode 100644 src/PVE/Schema/Auth.pm
proxmox-perl-rs:
Markus Frank (2):
move openid code from pve-rs to common
remove empty PMG::RS::OpenId package to avoid confusion
common/pkg/Makefile | 1 +
common/src/mod.rs | 1 +
common/src/oidc/mod.rs | 63 ++++++++++++++++++++++++++++++++++++++++
pmg-rs/Cargo.toml | 1 +
pmg-rs/Makefile | 1 -
pmg-rs/debian/control | 1 +
pve-rs/src/openid/mod.rs | 32 +++++---------------
7 files changed, 75 insertions(+), 25 deletions(-)
create mode 100644 common/src/oidc/mod.rs
pmg-api:
Markus Frank (4):
config: add plugin system for realms
config: add oidc type realm
api: add/update/remove realms like in PVE
api: oidc login similar to PVE
src/Makefile | 6 +
src/PMG/API2/AccessControl.pm | 17 ++-
src/PMG/API2/OIDC.pm | 243 ++++++++++++++++++++++++++++++
src/PMG/API2/Realm.pm | 274 ++++++++++++++++++++++++++++++++++
src/PMG/AccessControl.pm | 33 ++++
src/PMG/Auth/OIDC.pm | 95 ++++++++++++
src/PMG/Auth/PAM.pm | 22 +++
src/PMG/Auth/PMG.pm | 39 +++++
src/PMG/Auth/Plugin.pm | 199 ++++++++++++++++++++++++
src/PMG/HTTPServer.pm | 2 +
src/PMG/RESTEnvironment.pm | 14 ++
src/PMG/UserConfig.pm | 25 ++--
src/PMG/Utils.pm | 29 +++-
13 files changed, 981 insertions(+), 17 deletions(-)
create mode 100644 src/PMG/API2/OIDC.pm
create mode 100644 src/PMG/API2/Realm.pm
create mode 100755 src/PMG/Auth/OIDC.pm
create mode 100755 src/PMG/Auth/PAM.pm
create mode 100755 src/PMG/Auth/PMG.pm
create mode 100755 src/PMG/Auth/Plugin.pm
widget-toolkit:
Markus Frank (1):
fix: window: AuthEditBase: rename variable 'realm' to 'type'
src/window/AuthEditBase.js | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
pmg-gui:
Markus Frank (2):
login: add option to login with OIDC realm
add panel for realms to User Management
js/LoginView.js | 208 ++++++++++++++++++++++++++++++++-----------
js/UserManagement.js | 6 ++
js/Utils.js | 23 +++++
3 files changed, 186 insertions(+), 51 deletions(-)
--
2.39.5
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* [pmg-devel] [PATCH pve-common v5 1/10] add Schema package with auth module that contains realm sync options
2025-02-18 16:18 [pmg-devel] [PATCH pve-common/perl-rs/pmg-api/widget-toolkit/pmg-gui v5 0/10] fix #3892: OpenID Connect Markus Frank
@ 2025-02-18 16:18 ` Markus Frank
2025-02-19 18:18 ` Stoiko Ivanov
2025-02-21 12:22 ` Fabian Grünbichler
2025-02-18 16:18 ` [pmg-devel] [PATCH proxmox-perl-rs v5 2/10] move openid code from pve-rs to common Markus Frank
` (9 subsequent siblings)
10 siblings, 2 replies; 27+ messages in thread
From: Markus Frank @ 2025-02-18 16:18 UTC (permalink / raw)
To: pmg-devel
This is because these standard options & formats are used by both PVE
and PMG.
Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
src/Makefile | 2 ++
src/PVE/Schema/Auth.pm | 82 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 84 insertions(+)
create mode 100644 src/PVE/Schema/Auth.pm
diff --git a/src/Makefile b/src/Makefile
index 2d8bdc4..833bbc1 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -29,6 +29,7 @@ LIB_SOURCES = \
RESTEnvironment.pm \
RESTHandler.pm \
SafeSyslog.pm \
+ Schema/Auth.pm \
SectionConfig.pm \
SysFSTools.pm \
Syscall.pm \
@@ -41,6 +42,7 @@ all:
install: $(addprefix PVE/,${LIB_SOURCES})
install -d -m 0755 ${DESTDIR}${PERLDIR}/PVE
install -d -m 0755 ${DESTDIR}${PERLDIR}/PVE/Job
+ install -d -m 0755 ${DESTDIR}${PERLDIR}/PVE/Schema
for i in ${LIB_SOURCES}; do install -D -m 0644 PVE/$$i ${DESTDIR}${PERLDIR}/PVE/$$i; done
diff --git a/src/PVE/Schema/Auth.pm b/src/PVE/Schema/Auth.pm
new file mode 100644
index 0000000..1f7f586
--- /dev/null
+++ b/src/PVE/Schema/Auth.pm
@@ -0,0 +1,82 @@
+package PVE::Schema::Auth;
+
+use strict;
+use warnings;
+
+use PVE::JSONSchema qw(get_standard_option parse_property_string);
+
+my $remove_options = "(?:acl|properties|entry)";
+
+PVE::JSONSchema::register_standard_option('sync-scope', {
+ description => "Select what to sync.",
+ type => 'string',
+ enum => [qw(users groups both)],
+ optional => '1',
+});
+
+PVE::JSONSchema::register_standard_option('sync-remove-vanished', {
+ description => "A semicolon-seperated list of things to remove when they or the user"
+ ." vanishes during a sync. The following values are possible: 'entry' removes the"
+ ." user/group when not returned from the sync. 'properties' removes the set"
+ ." properties on existing user/group that do not appear in the source (even custom ones)."
+ ." 'acl' removes acls when the user/group is not returned from the sync."
+ ." Instead of a list it also can be 'none' (the default).",
+ type => 'string',
+ default => 'none',
+ typetext => "([acl];[properties];[entry])|none",
+ pattern => "(?:(?:$remove_options\;)*$remove_options)|none",
+ optional => '1',
+});
+
+my $realm_sync_options_desc = {
+ scope => get_standard_option('sync-scope'),
+ 'remove-vanished' => get_standard_option('sync-remove-vanished'),
+ 'enable-new' => {
+ description => "Enable newly synced users immediately.",
+ type => 'boolean',
+ default => '1',
+ optional => '1',
+ },
+};
+PVE::JSONSchema::register_standard_option('realm-sync-options', $realm_sync_options_desc);
+PVE::JSONSchema::register_format('realm-sync-options', $realm_sync_options_desc);
+
+my $tfa_format = {
+ type => {
+ description => "The type of 2nd factor authentication.",
+ format_description => 'TFATYPE',
+ type => 'string',
+ enum => [qw(oath)],
+ },
+ digits => {
+ description => "TOTP digits.",
+ format_description => 'COUNT',
+ type => 'integer',
+ minimum => 6, maximum => 8,
+ default => 6,
+ optional => 1,
+ },
+ step => {
+ description => "TOTP time period.",
+ format_description => 'SECONDS',
+ type => 'integer',
+ minimum => 10,
+ default => 30,
+ optional => 1,
+ },
+};
+
+PVE::JSONSchema::register_format('pve-tfa-config', $tfa_format);
+
+PVE::JSONSchema::register_standard_option('tfa', {
+ description => "Use Two-factor authentication.",
+ type => 'string', format => 'pve-tfa-config',
+ optional => 1,
+ maxLength => 128,
+});
+
+sub parse_tfa_config {
+ my ($data) = @_;
+
+ return parse_property_string($tfa_format, $data);
+}
--
2.39.5
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* [pmg-devel] [PATCH proxmox-perl-rs v5 2/10] move openid code from pve-rs to common
2025-02-18 16:18 [pmg-devel] [PATCH pve-common/perl-rs/pmg-api/widget-toolkit/pmg-gui v5 0/10] fix #3892: OpenID Connect Markus Frank
2025-02-18 16:18 ` [pmg-devel] [PATCH pve-common v5 1/10] add Schema package with auth module that contains realm sync options Markus Frank
@ 2025-02-18 16:18 ` Markus Frank
2025-02-21 12:25 ` Fabian Grünbichler
2025-02-18 16:18 ` [pmg-devel] [PATCH proxmox-perl-rs v5 3/10] remove empty PMG::RS::OpenId package to avoid confusion Markus Frank
` (8 subsequent siblings)
10 siblings, 1 reply; 27+ messages in thread
From: Markus Frank @ 2025-02-18 16:18 UTC (permalink / raw)
To: pmg-devel
Change pve-rs functions to be wrapper functions for common.
Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
common/pkg/Makefile | 1 +
common/src/mod.rs | 1 +
common/src/oidc/mod.rs | 63 ++++++++++++++++++++++++++++++++++++++++
pmg-rs/Cargo.toml | 1 +
pmg-rs/debian/control | 1 +
pve-rs/src/openid/mod.rs | 32 +++++---------------
6 files changed, 75 insertions(+), 24 deletions(-)
create mode 100644 common/src/oidc/mod.rs
diff --git a/common/pkg/Makefile b/common/pkg/Makefile
index cbefdf7..5a537f9 100644
--- a/common/pkg/Makefile
+++ b/common/pkg/Makefile
@@ -24,6 +24,7 @@ PERLMOD_PACKAGES := \
Proxmox::RS::APT::Repositories \
Proxmox::RS::CalendarEvent \
Proxmox::RS::Notify \
+ Proxmox::RS::OIDC \
Proxmox::RS::SharedCache \
Proxmox::RS::Subscription
diff --git a/common/src/mod.rs b/common/src/mod.rs
index badfc98..06a7c20 100644
--- a/common/src/mod.rs
+++ b/common/src/mod.rs
@@ -2,5 +2,6 @@ pub mod apt;
mod calendar_event;
pub mod logger;
pub mod notify;
+pub mod oidc;
pub mod shared_cache;
mod subscription;
diff --git a/common/src/oidc/mod.rs b/common/src/oidc/mod.rs
new file mode 100644
index 0000000..b1cddaa
--- /dev/null
+++ b/common/src/oidc/mod.rs
@@ -0,0 +1,63 @@
+#[perlmod::package(name = "Proxmox::RS::OIDC")]
+pub mod export {
+ use std::sync::Mutex;
+
+ use anyhow::Error;
+
+ use perlmod::{to_value, Value};
+
+ use proxmox_openid::{OpenIdAuthenticator, OpenIdConfig, PrivateAuthState};
+
+ perlmod::declare_magic!(Box<OIDC> : &OIDC as "Proxmox::RS::OIDC");
+
+ /// An OpenIdAuthenticator client instance.
+ pub struct OIDC {
+ inner: Mutex<OpenIdAuthenticator>,
+ }
+
+ /// Create a new OIDC client instance
+ #[export(raw_return)]
+ pub fn discover(
+ #[raw] class: Value,
+ config: OpenIdConfig,
+ redirect_url: &str,
+ ) -> Result<Value, Error> {
+ let oidc = OpenIdAuthenticator::discover(&config, redirect_url)?;
+ Ok(perlmod::instantiate_magic!(
+ &class,
+ MAGIC => Box::new(OIDC {
+ inner: Mutex::new(oidc),
+ })
+ ))
+ }
+
+ #[export]
+ pub fn authorize_url(
+ #[try_from_ref] this: &OIDC,
+ state_dir: &str,
+ realm: &str,
+ ) -> Result<String, Error> {
+ let oidc = this.inner.lock().unwrap();
+ oidc.authorize_url(state_dir, realm)
+ }
+
+ #[export]
+ pub fn verify_public_auth_state(
+ state_dir: &str,
+ state: &str,
+ ) -> Result<(String, PrivateAuthState), Error> {
+ OpenIdAuthenticator::verify_public_auth_state(state_dir, state)
+ }
+
+ #[export(raw_return)]
+ pub fn verify_authorization_code(
+ #[try_from_ref] this: &OIDC,
+ code: &str,
+ private_auth_state: PrivateAuthState,
+ ) -> Result<Value, Error> {
+ let oidc = this.inner.lock().unwrap();
+ let claims = oidc.verify_authorization_code_simple(code, &private_auth_state)?;
+
+ Ok(to_value(&claims)?)
+ }
+}
diff --git a/pmg-rs/Cargo.toml b/pmg-rs/Cargo.toml
index 1252671..ce715bf 100644
--- a/pmg-rs/Cargo.toml
+++ b/pmg-rs/Cargo.toml
@@ -42,3 +42,4 @@ proxmox-subscription = "0.5"
proxmox-sys = "0.6"
proxmox-tfa = { version = "5", features = ["api"] }
proxmox-time = "2"
+proxmox-openid = "0.10.0"
diff --git a/pmg-rs/debian/control b/pmg-rs/debian/control
index 295dcb3..afd8cbb 100644
--- a/pmg-rs/debian/control
+++ b/pmg-rs/debian/control
@@ -27,6 +27,7 @@ Build-Depends: cargo:native <!nocheck>,
librust-proxmox-http-error-0.1+default-dev,
librust-proxmox-log-0.2+default-dev,
librust-proxmox-notify-0.5+default-dev,
+ librust-proxmox-openid-0.10+default-dev,
librust-proxmox-shared-cache-0.1+default-dev,
librust-proxmox-subscription-0.5+default-dev,
librust-proxmox-sys-0.6+default-dev,
diff --git a/pve-rs/src/openid/mod.rs b/pve-rs/src/openid/mod.rs
index 1fa7572..2adb8bb 100644
--- a/pve-rs/src/openid/mod.rs
+++ b/pve-rs/src/openid/mod.rs
@@ -1,19 +1,13 @@
#[perlmod::package(name = "PVE::RS::OpenId", lib = "pve_rs")]
mod export {
- use std::sync::Mutex;
-
use anyhow::Error;
- use perlmod::{to_value, Value};
-
- use proxmox_openid::{OpenIdAuthenticator, OpenIdConfig, PrivateAuthState};
+ use perlmod::Value;
- perlmod::declare_magic!(Box<OpenId> : &OpenId as "PVE::RS::OpenId");
+ use proxmox_openid::{OpenIdConfig, PrivateAuthState};
- /// An OpenIdAuthenticator client instance.
- pub struct OpenId {
- inner: Mutex<OpenIdAuthenticator>,
- }
+ use crate::common::oidc::export as common;
+ use crate::common::oidc::export::OIDC as OpenId;
/// Create a new OpenId client instance
#[export(raw_return)]
@@ -22,13 +16,7 @@ mod export {
config: OpenIdConfig,
redirect_url: &str,
) -> Result<Value, Error> {
- let open_id = OpenIdAuthenticator::discover(&config, redirect_url)?;
- Ok(perlmod::instantiate_magic!(
- &class,
- MAGIC => Box::new(OpenId {
- inner: Mutex::new(open_id),
- })
- ))
+ common::discover(class, config, redirect_url)
}
#[export]
@@ -37,8 +25,7 @@ mod export {
state_dir: &str,
realm: &str,
) -> Result<String, Error> {
- let open_id = this.inner.lock().unwrap();
- open_id.authorize_url(state_dir, realm)
+ common::authorize_url(this, state_dir, realm)
}
#[export]
@@ -46,7 +33,7 @@ mod export {
state_dir: &str,
state: &str,
) -> Result<(String, PrivateAuthState), Error> {
- OpenIdAuthenticator::verify_public_auth_state(state_dir, state)
+ common::verify_public_auth_state(state_dir, state)
}
#[export(raw_return)]
@@ -55,9 +42,6 @@ mod export {
code: &str,
private_auth_state: PrivateAuthState,
) -> Result<Value, Error> {
- let open_id = this.inner.lock().unwrap();
- let claims = open_id.verify_authorization_code_simple(code, &private_auth_state)?;
-
- Ok(to_value(&claims)?)
+ common::verify_authorization_code(this, code, private_auth_state)
}
}
--
2.39.5
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* [pmg-devel] [PATCH proxmox-perl-rs v5 3/10] remove empty PMG::RS::OpenId package to avoid confusion
2025-02-18 16:18 [pmg-devel] [PATCH pve-common/perl-rs/pmg-api/widget-toolkit/pmg-gui v5 0/10] fix #3892: OpenID Connect Markus Frank
2025-02-18 16:18 ` [pmg-devel] [PATCH pve-common v5 1/10] add Schema package with auth module that contains realm sync options Markus Frank
2025-02-18 16:18 ` [pmg-devel] [PATCH proxmox-perl-rs v5 2/10] move openid code from pve-rs to common Markus Frank
@ 2025-02-18 16:18 ` Markus Frank
2025-02-18 16:18 ` [pmg-devel] [PATCH pmg-api v5 4/10] config: add plugin system for realms Markus Frank
` (7 subsequent siblings)
10 siblings, 0 replies; 27+ messages in thread
From: Markus Frank @ 2025-02-18 16:18 UTC (permalink / raw)
To: pmg-devel
The common Proxmox::RS:OIDC package can be used instead.
Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
pmg-rs/Makefile | 1 -
1 file changed, 1 deletion(-)
diff --git a/pmg-rs/Makefile b/pmg-rs/Makefile
index 073a284..0b48a37 100644
--- a/pmg-rs/Makefile
+++ b/pmg-rs/Makefile
@@ -29,7 +29,6 @@ PERLMOD_PACKAGES := \
PMG::RS::APT::Repositories \
PMG::RS::Acme \
PMG::RS::CSR \
- PMG::RS::OpenId \
PMG::RS::TFA
PERLMOD_PACKAGE_FILES := $(addsuffix .pm,$(subst ::,/,$(PERLMOD_PACKAGES)))
--
2.39.5
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* [pmg-devel] [PATCH pmg-api v5 4/10] config: add plugin system for realms
2025-02-18 16:18 [pmg-devel] [PATCH pve-common/perl-rs/pmg-api/widget-toolkit/pmg-gui v5 0/10] fix #3892: OpenID Connect Markus Frank
` (2 preceding siblings ...)
2025-02-18 16:18 ` [pmg-devel] [PATCH proxmox-perl-rs v5 3/10] remove empty PMG::RS::OpenId package to avoid confusion Markus Frank
@ 2025-02-18 16:18 ` Markus Frank
2025-02-21 12:35 ` Fabian Grünbichler
2025-02-18 16:19 ` [pmg-devel] [PATCH pmg-api v5 5/10] config: add oidc type realm Markus Frank
` (6 subsequent siblings)
10 siblings, 1 reply; 27+ messages in thread
From: Markus Frank @ 2025-02-18 16:18 UTC (permalink / raw)
To: pmg-devel
To differentiate between usernames, the realm is also stored in the
user.conf file. Old config file syntax can be read, but will be
overwritten after a change.
This is a carryover from PVE. Previously there was no realm for a
username. Now the realm is also stored after an @ sign in user.conf.
Utils generates a list of valid realm names, including any newly added
realms, to ensure proper validation of a specified realm name.
Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
src/Makefile | 3 +
src/PMG/AccessControl.pm | 31 ++++++
src/PMG/Auth/PAM.pm | 22 ++++
src/PMG/Auth/PMG.pm | 39 ++++++++
src/PMG/Auth/Plugin.pm | 199 +++++++++++++++++++++++++++++++++++++
src/PMG/RESTEnvironment.pm | 14 +++
src/PMG/UserConfig.pm | 25 +++--
src/PMG/Utils.pm | 29 ++++--
8 files changed, 346 insertions(+), 16 deletions(-)
create mode 100755 src/PMG/Auth/PAM.pm
create mode 100755 src/PMG/Auth/PMG.pm
create mode 100755 src/PMG/Auth/Plugin.pm
diff --git a/src/Makefile b/src/Makefile
index 1232880..659a666 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -166,6 +166,9 @@ LIBSOURCES = \
PMG/API2/ACMEPlugin.pm \
PMG/API2/NodeConfig.pm \
PMG/API2.pm \
+ PMG/Auth/Plugin.pm \
+ PMG/Auth/PAM.pm \
+ PMG/Auth/PMG.pm \
SOURCES = ${LIBSOURCES} ${CLI_BINARIES} ${TEMPLATES_FILES} ${CONF_MANS} ${CLI_MANS} ${SERVICE_MANS} ${SERVICE_UNITS} ${TIMER_UNITS} pmg-sources.list pmg-initramfs.conf
diff --git a/src/PMG/AccessControl.pm b/src/PMG/AccessControl.pm
index e12d7cf..ced5f9a 100644
--- a/src/PMG/AccessControl.pm
+++ b/src/PMG/AccessControl.pm
@@ -13,6 +13,14 @@ use PMG::LDAPConfig;
use PMG::LDAPSet;
use PMG::TFAConfig;
+use PMG::Auth::Plugin;
+use PMG::Auth::PAM;
+use PMG::Auth::PMG;
+
+PMG::Auth::PAM->register();
+PMG::Auth::PMG->register();
+PMG::Auth::Plugin->init();
+
sub normalize_path {
my $path = shift;
@@ -38,6 +46,7 @@ sub authenticate_user : prototype($$$) {
($username, $ruid, $realm) = PMG::Utils::verify_username($username);
+ my $realm_regex = PMG::Utils::valid_pmg_realm_regex();
if ($realm eq 'pam') {
die "invalid pam user (only root allowed)\n" if $ruid ne 'root';
authenticate_pam_user($ruid, $password);
@@ -53,6 +62,8 @@ sub authenticate_user : prototype($$$) {
return ($pmail . '@quarantine', undef);
}
die "ldap login failed\n";
+ } elsif ($realm =~ m!(${realm_regex})!) {
+ # nothing to do for self-defined realms
} else {
die "no such realm '$realm'\n";
}
@@ -79,6 +90,7 @@ sub set_user_password {
($username, $ruid, $realm) = PMG::Utils::verify_username($username);
+ my $realm_regex = PMG::Utils::valid_pmg_realm_regex();
if ($realm eq 'pam') {
die "invalid pam user (only root allowed)\n" if $ruid ne 'root';
@@ -92,6 +104,8 @@ sub set_user_password {
} elsif ($realm eq 'pmg') {
PMG::UserConfig->set_user_password($username, $password);
+ } elsif ($realm =~ m!(${realm_regex})!) {
+ # nothing to do for self-defined realms
} else {
die "no such realm '$realm'\n";
}
@@ -106,6 +120,7 @@ sub check_user_enabled {
($username, $ruid, $realm) = PMG::Utils::verify_username($username, 1);
+ my $realm_regex = PMG::Utils::valid_pmg_realm_regex();
if ($realm && $ruid) {
if ($realm eq 'pam') {
return 'root' if $ruid eq 'root';
@@ -115,6 +130,10 @@ sub check_user_enabled {
return $data->{role} if $data && $data->{enable};
} elsif ($realm eq 'quarantine') {
return 'quser';
+ } elsif ($realm =~ m!(${realm_regex})!) {
+ my $usercfg = PMG::UserConfig->new();
+ my $data = $usercfg->lookup_user_data($username, $noerr);
+ return $data->{role} if $data && $data->{enable};
}
}
@@ -123,6 +142,18 @@ sub check_user_enabled {
return undef;
}
+sub check_user_exist {
+ my ($usercfg, $username, $noerr) = @_;
+
+ $username = PMG::Utils::verify_username($username, $noerr);
+ return undef if !$username;
+ return $usercfg->{$username} if $usercfg && $usercfg->{$username};
+
+ die "no such user ('$username')\n" if !$noerr;
+
+ return undef;
+}
+
sub authenticate_pam_user {
my ($username, $password) = @_;
diff --git a/src/PMG/Auth/PAM.pm b/src/PMG/Auth/PAM.pm
new file mode 100755
index 0000000..3ffd8a6
--- /dev/null
+++ b/src/PMG/Auth/PAM.pm
@@ -0,0 +1,22 @@
+package PMG::Auth::PAM;
+
+use strict;
+use warnings;
+
+use PMG::Auth::Plugin;
+
+use base qw(PMG::Auth::Plugin);
+
+sub type {
+ return 'pam';
+}
+
+sub options {
+ return {
+ default => { optional => 1 },
+ comment => { optional => 1 },
+ tfa => { optional => 1 },
+ };
+}
+
+1;
diff --git a/src/PMG/Auth/PMG.pm b/src/PMG/Auth/PMG.pm
new file mode 100755
index 0000000..b083692
--- /dev/null
+++ b/src/PMG/Auth/PMG.pm
@@ -0,0 +1,39 @@
+package PMG::Auth::PMG;
+
+use strict;
+use warnings;
+
+use PMG::Auth::Plugin;
+
+use base qw(PMG::Auth::Plugin);
+
+sub type {
+ return 'pmg';
+}
+
+sub properties {
+ return {
+ default => {
+ description => "Use this as default realm",
+ type => 'boolean',
+ optional => 1,
+ },
+ comment => {
+ description => "Description.",
+ type => 'string',
+ optional => 1,
+ maxLength => 4096,
+ },
+ tfa => PVE::JSONSchema::get_standard_option('tfa'),
+ };
+}
+
+sub options {
+ return {
+ default => { optional => 1 },
+ comment => { optional => 1 },
+ tfa => { optional => 1 },
+ };
+}
+
+1;
diff --git a/src/PMG/Auth/Plugin.pm b/src/PMG/Auth/Plugin.pm
new file mode 100755
index 0000000..b336d0c
--- /dev/null
+++ b/src/PMG/Auth/Plugin.pm
@@ -0,0 +1,199 @@
+package PMG::Auth::Plugin;
+
+use strict;
+use warnings;
+
+use Digest::SHA;
+use Encode;
+
+use PMG::Utils;
+use PVE::INotify;
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::Schema::Auth;
+use PVE::SectionConfig;
+use PVE::Tools;
+
+use base qw(PVE::SectionConfig);
+
+my $domainconfigfile = "realms.cfg";
+my $lockfile = "/var/lock/pmg-realms.lck";
+
+sub read_realms_conf {
+ my ($filename, $fh) = @_;
+
+ my $raw;
+ $raw = do { local $/ = undef; <$fh> } if defined($fh);
+
+ return PMG::Auth::Plugin->parse_config($filename, $raw);
+}
+
+sub write_realms_conf {
+ my ($filename, $fh, $cfg) = @_;
+
+ my $raw = PMG::Auth::Plugin->write_config($filename, $cfg);
+
+ PVE::Tools::safe_print($filename, $fh, $raw);
+}
+
+PVE::INotify::register_file(
+ $domainconfigfile,
+ "/etc/pmg/realms.cfg",
+ \&read_realms_conf,
+ \&write_realms_conf,
+ undef,
+ always_call_parser => 1,
+);
+
+sub lock_domain_config {
+ my ($code, $errmsg) = @_;
+
+ PVE::Tools::lock_file($lockfile, undef, $code);
+ if (my $err = $@) {
+ $errmsg ? die "$errmsg: $err" : die $err;
+ }
+}
+
+my $realm_regex = qr/[A-Za-z][A-Za-z0-9\.\-_]+/;
+
+sub pmg_verify_realm {
+ my ($realm, $noerr) = @_;
+
+ if ($realm !~ m/^${realm_regex}$/) {
+ return undef if $noerr;
+ die "value does not look like a valid realm\n";
+ }
+ return $realm;
+}
+
+my $defaultData = {
+ propertyList => {
+ type => { description => "Realm type." },
+ realm => get_standard_option('realm'),
+ },
+};
+
+sub private {
+ return $defaultData;
+}
+
+sub parse_section_header {
+ my ($class, $line) = @_;
+
+ if ($line =~ m/^(\S+):\s*(\S+)\s*$/) {
+ my ($type, $realm) = (lc($1), $2);
+ my $errmsg = undef; # set if you want to skip whole section
+ eval { pmg_verify_realm($realm); };
+ $errmsg = $@ if $@;
+ my $config = {}; # to return additional attributes
+ return ($type, $realm, $errmsg, $config);
+ }
+ return undef;
+}
+
+sub parse_config {
+ my ($class, $filename, $raw) = @_;
+
+ my $cfg = $class->SUPER::parse_config($filename, $raw);
+
+ my $default;
+ foreach my $realm (keys %{$cfg->{ids}}) {
+ my $data = $cfg->{ids}->{$realm};
+ # make sure there is only one default marker
+ if ($data->{default}) {
+ if ($default) {
+ delete $data->{default};
+ } else {
+ $default = $realm;
+ }
+ }
+
+ if ($data->{comment}) {
+ $data->{comment} = PVE::Tools::decode_text($data->{comment});
+ }
+
+ }
+
+ # add default domains
+ $cfg->{ids}->{pmg}->{type} = 'pmg'; # force type
+ $cfg->{ids}->{pmg}->{comment} = "Proxmox Mail Gateway authentication server"
+ if !$cfg->{ids}->{pmg}->{comment};
+ $cfg->{ids}->{pmg}->{default} = 1
+ if !$cfg->{ids}->{pmg}->{default};
+
+ $cfg->{ids}->{pam}->{type} = 'pam'; # force type
+ $cfg->{ids}->{pam}->{comment} = "Linux PAM standard authentication"
+ if !$cfg->{ids}->{pam}->{comment};
+
+ return $cfg;
+};
+
+sub write_config {
+ my ($class, $filename, $cfg) = @_;
+
+ foreach my $realm (keys %{$cfg->{ids}}) {
+ my $data = $cfg->{ids}->{$realm};
+ if ($data->{comment}) {
+ $data->{comment} = PVE::Tools::encode_text($data->{comment});
+ }
+ }
+
+ $class->SUPER::write_config($filename, $cfg);
+}
+
+sub authenticate_user {
+ my ($class, $config, $realm, $username, $password) = @_;
+
+ die "overwrite me";
+}
+
+sub store_password {
+ my ($class, $config, $realm, $username, $password) = @_;
+
+ my $type = $class->type();
+
+ die "can't set password on auth type '$type'\n";
+}
+
+sub delete_user {
+ my ($class, $config, $realm, $username) = @_;
+
+ # do nothing by default
+}
+
+# called during addition of realm (before the new domain config got written)
+# `password` is moved to %param to avoid writing it out to the config
+# die to abort addition if there are (grave) problems
+# NOTE: runs in a domain config *locked* context
+sub on_add_hook {
+ my ($class, $realm, $config, %param) = @_;
+ # do nothing by default
+}
+
+# called during domain configuration update (before the updated domain config got
+# written). `password` is moved to %param to avoid writing it out to the config
+# die to abort the update if there are (grave) problems
+# NOTE: runs in a domain config *locked* context
+sub on_update_hook {
+ my ($class, $realm, $config, %param) = @_;
+ # do nothing by default
+}
+
+# called during deletion of realms (before the new domain config got written)
+# and if the activate check on addition fails, to cleanup all storage traces
+# which on_add_hook may have created.
+# die to abort deletion if there are (very grave) problems
+# NOTE: runs in a domain config *locked* context
+sub on_delete_hook {
+ my ($class, $realm, $config) = @_;
+ # do nothing by default
+}
+
+# called during addition and updates of realms (before the new domain config gets written)
+# die to abort addition/update in case the connection/bind fails
+# NOTE: runs in a domain config *locked* context
+sub check_connection {
+ my ($class, $realm, $config, %param) = @_;
+ # do nothing by default
+}
+
+1;
diff --git a/src/PMG/RESTEnvironment.pm b/src/PMG/RESTEnvironment.pm
index 3875720..f6ff449 100644
--- a/src/PMG/RESTEnvironment.pm
+++ b/src/PMG/RESTEnvironment.pm
@@ -88,6 +88,20 @@ sub get_role {
return $self->{role};
}
+sub check_user_enabled {
+ my ($self, $user, $noerr) = @_;
+
+ my $cfg = $self->{usercfg};
+ return PMG::AccessControl::check_user_enabled($cfg, $user, $noerr);
+}
+
+sub check_user_exist {
+ my ($self, $user, $noerr) = @_;
+
+ my $cfg = $self->{usercfg};
+ return PMG::AccessControl::check_user_exist($cfg, $user, $noerr);
+}
+
sub check_node_is_master {
my ($self, $noerr);
diff --git a/src/PMG/UserConfig.pm b/src/PMG/UserConfig.pm
index b9a83a7..fe6d2c8 100644
--- a/src/PMG/UserConfig.pm
+++ b/src/PMG/UserConfig.pm
@@ -80,7 +80,7 @@ my $schema = {
realm => {
description => "Authentication realm.",
type => 'string',
- enum => ['pam', 'pmg'],
+ format => 'pmg-realm',
default => 'pmg',
optional => 1,
},
@@ -219,10 +219,13 @@ sub read_user_conf {
(?<keys>(?:[^:]*)) :
$/x
) {
+ my @username_parts = split('@', $+{userid});
+ my $username = $username_parts[0];
+ my $realm = defined($username_parts[1]) ? $username_parts[1] : "pmg";
my $d = {
- username => $+{userid},
- userid => $+{userid} . '@pmg',
- realm => 'pmg',
+ username => $username,
+ userid => $username . '@' . $realm,
+ realm => $realm,
enable => $+{enable} || 0,
expire => $+{expire} || 0,
role => $+{role},
@@ -235,8 +238,9 @@ sub read_user_conf {
eval {
$verify_entry->($d);
$cfg->{$d->{userid}} = $d;
- die "role 'root' is reserved\n"
- if $d->{role} eq 'root' && $d->{userid} ne 'root@pmg';
+ if ($d->{role} eq 'root' && $d->{userid} !~ /^root@(pmg|pam)$/) {
+ die "role 'root' is reserved\n";
+ }
};
if (my $err = $@) {
warn "$filename: $err";
@@ -274,22 +278,23 @@ sub write_user_conf {
$verify_entry->($d);
$cfg->{$d->{userid}} = $d;
+ my $realm_regex = PMG::Utils::valid_pmg_realm_regex();
if ($d->{userid} ne 'root@pam') {
die "role 'root' is reserved\n" if $d->{role} eq 'root';
die "unable to add users for realm '$d->{realm}'\n"
- if $d->{realm} && $d->{realm} ne 'pmg';
+ if $d->{realm} && $d->{realm} !~ m!(${realm_regex})!;
}
my $line;
if ($userid eq 'root@pam') {
- $line = 'root:';
+ $line = 'root@pam:';
$d->{crypt_pass} = '',
$d->{expire} = '0',
$d->{role} = 'root';
} else {
- next if $userid !~ m/^(?<username>.+)\@pmg$/;
- $line = "$+{username}:";
+ next if $userid !~ m/^(?<username>.+)\@(${realm_regex})$/;
+ $line = "$d->{userid}:";
}
for my $k (qw(enable expire crypt_pass role email firstname lastname keys)) {
diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
index 0b8945f..04a3a2e 100644
--- a/src/PMG/Utils.pm
+++ b/src/PMG/Utils.pm
@@ -49,12 +49,30 @@ postgres_admin_cmd
try_decode_utf8
);
-my $valid_pmg_realms = ['pam', 'pmg', 'quarantine'];
+my $user_regex = qr![^\s:/]+!;
+
+sub valid_pmg_realm_regex {
+ my $cfg = PVE::INotify::read_file('realms.cfg');
+ my $ids = $cfg->{ids};
+ my $realms = ['pam', 'quarantine', sort keys $cfg->{ids}->%* ];
+ return join('|', @$realms);
+}
+
+sub is_valid_realm {
+ my ($realm) = @_;
+ return 0 if !$realm;
+ return 1 if $realm eq 'pam' || $realm eq 'quarantine'; # built-in ones
+
+ my $cfg = PVE::INotify::read_file('realms.cfg');
+ return exists($cfg->{ids}->{$realm}) ? 1 : 0;
+}
+
+PVE::JSONSchema::register_format('pmg-realm', \&is_valid_realm);
PVE::JSONSchema::register_standard_option('realm', {
description => "Authentication domain ID",
type => 'string',
- enum => $valid_pmg_realms,
+ format => 'pmg-realm',
maxLength => 32,
});
@@ -82,16 +100,15 @@ sub verify_username {
die "user name '$username' is too short\n" if !$noerr;
return undef;
}
- if ($len > 64) {
- die "user name '$username' is too long ($len > 64)\n" if !$noerr;
+ if ($len > 128) {
+ die "user name '$username' is too long ($len > 128)\n" if !$noerr;
return undef;
}
# we only allow a limited set of characters. Colons aren't allowed, because we store usernames
# with colon separated lists! slashes aren't allowed because it is used as pve API delimiter
# also see "man useradd"
- my $realm_list = join('|', @$valid_pmg_realms);
- if ($username =~ m!^([^\s:/]+)\@(${realm_list})$!) {
+ if ($username =~ m!^(${user_regex})\@([A-Za-z][A-Za-z0-9\.\-_]+)$!) {
return wantarray ? ($username, $1, $2) : $username;
}
--
2.39.5
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* [pmg-devel] [PATCH pmg-api v5 5/10] config: add oidc type realm
2025-02-18 16:18 [pmg-devel] [PATCH pve-common/perl-rs/pmg-api/widget-toolkit/pmg-gui v5 0/10] fix #3892: OpenID Connect Markus Frank
` (3 preceding siblings ...)
2025-02-18 16:18 ` [pmg-devel] [PATCH pmg-api v5 4/10] config: add plugin system for realms Markus Frank
@ 2025-02-18 16:19 ` Markus Frank
2025-02-21 12:38 ` Fabian Grünbichler
2025-02-18 16:19 ` [pmg-devel] [PATCH pmg-api v5 6/10] api: add/update/remove realms like in PVE Markus Frank
` (5 subsequent siblings)
10 siblings, 1 reply; 27+ messages in thread
From: Markus Frank @ 2025-02-18 16:19 UTC (permalink / raw)
To: pmg-devel
If the autocreate option is enabled, the user is automatically created
with the Audit role. To change the role for automatically created users,
set the autocreate-role option to the preferred role.
Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
src/Makefile | 1 +
src/PMG/AccessControl.pm | 2 +
src/PMG/Auth/OIDC.pm | 95 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 98 insertions(+)
create mode 100755 src/PMG/Auth/OIDC.pm
diff --git a/src/Makefile b/src/Makefile
index 659a666..3cae7c7 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -169,6 +169,7 @@ LIBSOURCES = \
PMG/Auth/Plugin.pm \
PMG/Auth/PAM.pm \
PMG/Auth/PMG.pm \
+ PMG/Auth/OIDC.pm \
SOURCES = ${LIBSOURCES} ${CLI_BINARIES} ${TEMPLATES_FILES} ${CONF_MANS} ${CLI_MANS} ${SERVICE_MANS} ${SERVICE_UNITS} ${TIMER_UNITS} pmg-sources.list pmg-initramfs.conf
diff --git a/src/PMG/AccessControl.pm b/src/PMG/AccessControl.pm
index ced5f9a..75ef486 100644
--- a/src/PMG/AccessControl.pm
+++ b/src/PMG/AccessControl.pm
@@ -14,9 +14,11 @@ use PMG::LDAPSet;
use PMG::TFAConfig;
use PMG::Auth::Plugin;
+use PMG::Auth::OIDC;
use PMG::Auth::PAM;
use PMG::Auth::PMG;
+PMG::Auth::OIDC->register();
PMG::Auth::PAM->register();
PMG::Auth::PMG->register();
PMG::Auth::Plugin->init();
diff --git a/src/PMG/Auth/OIDC.pm b/src/PMG/Auth/OIDC.pm
new file mode 100755
index 0000000..0acde7d
--- /dev/null
+++ b/src/PMG/Auth/OIDC.pm
@@ -0,0 +1,95 @@
+package PMG::Auth::OIDC;
+
+use strict;
+use warnings;
+
+use PVE::Tools;
+use PMG::Auth::Plugin;
+
+use base qw(PMG::Auth::Plugin);
+
+sub type {
+ return 'oidc';
+}
+
+sub properties {
+ return {
+ 'issuer-url' => {
+ description => "OpenID Connect Issuer Url",
+ type => 'string',
+ maxLength => 256,
+ },
+ 'client-id' => {
+ description => "OpenID Connect Client ID",
+ type => 'string',
+ maxLength => 256,
+ },
+ 'client-key' => {
+ description => "OpenID Connect Client Key",
+ type => 'string',
+ optional => 1,
+ maxLength => 256,
+ },
+ autocreate => {
+ description => "Automatically create users if they do not exist.",
+ optional => 1,
+ type => 'boolean',
+ default => 0,
+ },
+ 'autocreate-role' => {
+ description => "Automatically create users with a specific role.",
+ type => 'string',
+ enum => ['admin', 'qmanager', 'audit', 'helpdesk'],
+ optional => 1,
+ },
+ 'username-claim' => {
+ description => "OpenID Connect claim used to generate the unique username.",
+ type => 'string',
+ optional => 1,
+ },
+ prompt => {
+ description => "Specifies whether the Authorization Server prompts the End-User for"
+ ." reauthentication and consent.",
+ type => 'string',
+ pattern => '(?:none|login|consent|select_account|\S+)', # \S+ is the extension variant
+ optional => 1,
+ },
+ scopes => {
+ description => "Specifies the scopes (user details) that should be authorized and"
+ ." returned, for example 'email' or 'profile'.",
+ type => 'string', # format => 'some-safe-id-list', # FIXME: TODO
+ default => "email profile",
+ optional => 1,
+ },
+ 'acr-values' => {
+ description => "Specifies the Authentication Context Class Reference values that the"
+ ."Authorization Server is being requested to use for the Auth Request.",
+ type => 'string', # format => 'some-safe-id-list', # FIXME: TODO
+ optional => 1,
+ },
+ };
+}
+
+sub options {
+ return {
+ 'issuer-url' => {},
+ 'client-id' => {},
+ 'client-key' => { optional => 1 },
+ autocreate => { optional => 1 },
+ 'autocreate-role' => { optional => 1 },
+ 'username-claim' => { optional => 1, fixed => 1 },
+ prompt => { optional => 1 },
+ scopes => { optional => 1 },
+ 'acr-values' => { optional => 1 },
+ default => { optional => 1 },
+ comment => { optional => 1 },
+ };
+}
+
+sub authenticate_user {
+ my ($class, $config, $realm, $username, $password) = @_;
+
+ die "OpenID Connect realm does not allow password verification.\n";
+}
+
+1;
--
2.39.5
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* [pmg-devel] [PATCH pmg-api v5 6/10] api: add/update/remove realms like in PVE
2025-02-18 16:18 [pmg-devel] [PATCH pve-common/perl-rs/pmg-api/widget-toolkit/pmg-gui v5 0/10] fix #3892: OpenID Connect Markus Frank
` (4 preceding siblings ...)
2025-02-18 16:19 ` [pmg-devel] [PATCH pmg-api v5 5/10] config: add oidc type realm Markus Frank
@ 2025-02-18 16:19 ` Markus Frank
2025-02-21 12:41 ` Fabian Grünbichler
2025-02-18 16:19 ` [pmg-devel] [PATCH pmg-api v5 7/10] api: oidc login similar to PVE Markus Frank
` (4 subsequent siblings)
10 siblings, 1 reply; 27+ messages in thread
From: Markus Frank @ 2025-02-18 16:19 UTC (permalink / raw)
To: pmg-devel
The name Realm.pm was chosen because a Domain.pm already exists.
Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
src/Makefile | 1 +
src/PMG/API2/AccessControl.pm | 10 +-
src/PMG/API2/Realm.pm | 274 ++++++++++++++++++++++++++++++++++
3 files changed, 284 insertions(+), 1 deletion(-)
create mode 100644 src/PMG/API2/Realm.pm
diff --git a/src/Makefile b/src/Makefile
index 3cae7c7..e939bbd 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -151,6 +151,7 @@ LIBSOURCES = \
PMG/API2/Postfix.pm \
PMG/API2/Quarantine.pm \
PMG/API2/AccessControl.pm \
+ PMG/API2/Realm.pm \
PMG/API2/TFA.pm \
PMG/API2/TFAConfig.pm \
PMG/API2/ObjectGroupHelpers.pm \
diff --git a/src/PMG/API2/AccessControl.pm b/src/PMG/API2/AccessControl.pm
index e26ae70..f4cbc81 100644
--- a/src/PMG/API2/AccessControl.pm
+++ b/src/PMG/API2/AccessControl.pm
@@ -12,6 +12,7 @@ use PVE::JSONSchema qw(get_standard_option);
use PMG::Utils;
use PMG::UserConfig;
use PMG::AccessControl;
+use PMG::API2::Realm;
use PMG::API2::Users;
use PMG::API2::TFA;
use PMG::TFAConfig;
@@ -30,6 +31,11 @@ __PACKAGE__->register_method ({
path => 'tfa',
});
+__PACKAGE__->register_method ({
+ subclass => "PMG::API2::Realm",
+ path => 'domains',
+});
+
__PACKAGE__->register_method ({
name => 'index',
path => '',
@@ -57,6 +63,7 @@ __PACKAGE__->register_method ({
my $res = [
{ subdir => 'ticket' },
+ { subdir => 'domains' },
{ subdir => 'password' },
{ subdir => 'users' },
];
@@ -248,7 +255,8 @@ __PACKAGE__->register_method ({
my $username = $param->{username};
- if ($username !~ m/\@(pam|pmg|quarantine)$/) {
+ my $realm_regex = PMG::Utils::valid_pmg_realm_regex();
+ if ($username !~ m/\@(${realm_regex})$/) {
my $realm = $param->{realm} // 'quarantine';
$username .= "\@$realm";
}
diff --git a/src/PMG/API2/Realm.pm b/src/PMG/API2/Realm.pm
new file mode 100644
index 0000000..da2072a
--- /dev/null
+++ b/src/PMG/API2/Realm.pm
@@ -0,0 +1,274 @@
+package PMG::API2::Realm;
+
+use strict;
+use warnings;
+
+use PVE::Exception qw(raise_param_exc);
+use PVE::INotify;
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::RESTHandler;
+use PVE::SafeSyslog;
+use PVE::Tools qw(extract_param);
+
+use PMG::AccessControl;
+use PMG::Auth::Plugin;
+use PVE::Schema::Auth;
+
+my $domainconfigfile = "realms.cfg";
+
+use base qw(PVE::RESTHandler);
+
+__PACKAGE__->register_method ({
+ name => 'index',
+ path => '',
+ method => 'GET',
+ description => "Authentication domain index.",
+ permissions => {
+ description => "Anyone can access that, because we need that list for the login box (before the user is authenticated).",
+ user => 'world',
+ },
+ parameters => {
+ additionalProperties => 0,
+ properties => {},
+ },
+ returns => {
+ type => 'array',
+ items => {
+ type => "object",
+ properties => {
+ realm => { type => 'string' },
+ type => { type => 'string' },
+ tfa => {
+ description => "Two-factor authentication provider.",
+ type => 'string',
+ enum => [ 'yubico', 'oath' ],
+ optional => 1,
+ },
+ comment => {
+ description => "A comment. The GUI use this text when you select a domain (Realm) on the login window.",
+ type => 'string',
+ optional => 1,
+ },
+ },
+ },
+ links => [ { rel => 'child', href => "{realm}" } ],
+ },
+ code => sub {
+ my ($param) = @_;
+
+ my $res = [];
+
+ my $cfg = PVE::INotify::read_file($domainconfigfile);
+ my $ids = $cfg->{ids};
+
+ for my $realm (keys %$ids) {
+ my $d = $ids->{$realm};
+ my $entry = { realm => $realm, type => $d->{type} };
+ $entry->{comment} = $d->{comment} if $d->{comment};
+ $entry->{default} = 1 if $d->{default};
+ if ($d->{tfa} && (my $tfa_cfg = PVE::Schema::Auth::parse_tfa_config($d->{tfa}))) {
+ $entry->{tfa} = $tfa_cfg->{type};
+ }
+ push @$res, $entry;
+ }
+
+ return $res;
+ }});
+
+__PACKAGE__->register_method ({
+ name => 'create',
+ protected => 1,
+ path => '',
+ method => 'POST',
+ permissions => { check => [ 'admin' ] },
+ description => "Add an authentication server.",
+ parameters => PMG::Auth::Plugin->createSchema(0),
+ returns => { type => 'null' },
+ code => sub {
+ my ($param) = @_;
+
+ # always extract, add it with hook
+ my $password = extract_param($param, 'password');
+
+ PMG::Auth::Plugin::lock_domain_config(
+ sub {
+ my $cfg = PVE::INotify::read_file($domainconfigfile);
+ my $ids = $cfg->{ids};
+
+ my $realm = extract_param($param, 'realm');
+ PMG::Auth::Plugin::pmg_verify_realm($realm);
+ my $type = $param->{type};
+ my $check_connection = extract_param($param, 'check-connection');
+
+ die "domain '$realm' already exists\n"
+ if $ids->{$realm};
+
+ die "unable to use reserved name '$realm'\n"
+ if ($realm eq 'pam' || $realm eq 'pmg');
+
+ die "unable to create builtin type '$type'\n"
+ if ($type eq 'pam' || $type eq 'pmg');
+
+ my $plugin = PMG::Auth::Plugin->lookup($type);
+ my $config = $plugin->check_config($realm, $param, 1, 1);
+
+ if ($config->{default}) {
+ for my $r (keys %$ids) {
+ delete $ids->{$r}->{default};
+ }
+ }
+
+ $ids->{$realm} = $config;
+
+ my $opts = $plugin->options();
+ if (defined($password) && !defined($opts->{password})) {
+ $password = undef;
+ warn "ignoring password parameter";
+ }
+ $plugin->on_add_hook($realm, $config, password => $password);
+
+ PVE::INotify::write_file($domainconfigfile, $cfg);
+ },
+ "add auth server failed",
+ );
+ return undef;
+ }});
+
+__PACKAGE__->register_method ({
+ name => 'update',
+ path => '{realm}',
+ method => 'PUT',
+ permissions => { check => [ 'admin' ] },
+ description => "Update authentication server settings.",
+ protected => 1,
+ parameters => PMG::Auth::Plugin->updateSchema(0),
+ returns => { type => 'null' },
+ code => sub {
+ my ($param) = @_;
+
+ # always extract, update in hook
+ my $password = extract_param($param, 'password');
+
+ PMG::Auth::Plugin::lock_domain_config(
+ sub {
+ my $cfg = PVE::INotify::read_file($domainconfigfile);
+ my $ids = $cfg->{ids};
+
+ my $digest = extract_param($param, 'digest');
+ PVE::SectionConfig::assert_if_modified($cfg, $digest);
+
+ my $realm = extract_param($param, 'realm');
+ my $type = $ids->{$realm}->{type};
+ my $check_connection = extract_param($param, 'check-connection');
+
+ die "domain '$realm' does not exist\n"
+ if !$ids->{$realm};
+
+ my $delete_str = extract_param($param, 'delete');
+ die "no options specified\n"
+ if !$delete_str && !scalar(keys %$param) && !defined($password);
+
+ my $delete_pw = 0;
+ for my $opt (PVE::Tools::split_list($delete_str)) {
+ delete $ids->{$realm}->{$opt};
+ $delete_pw = 1 if $opt eq 'password';
+ }
+
+ my $plugin = PMG::Auth::Plugin->lookup($type);
+ my $config = $plugin->check_config($realm, $param, 0, 1);
+
+ if ($config->{default}) {
+ for my $r (keys %$ids) {
+ delete $ids->{$r}->{default};
+ }
+ }
+
+ for my $p (keys %$config) {
+ $ids->{$realm}->{$p} = $config->{$p};
+ }
+
+ my $opts = $plugin->options();
+ if ($delete_pw || defined($password)) {
+ $plugin->on_update_hook($realm, $config, password => $password);
+ } else {
+ $plugin->on_update_hook($realm, $config);
+ }
+
+ PVE::INotify::write_file($domainconfigfile, $cfg);
+ },
+ "update auth server failed"
+ );
+ return undef;
+ }});
+
+# fixme: return format!
+__PACKAGE__->register_method ({
+ name => 'read',
+ path => '{realm}',
+ method => 'GET',
+ description => "Get auth server configuration.",
+ permissions => { check => [ 'admin', 'qmanager', 'audit' ] },
+ parameters => {
+ additionalProperties => 0,
+ properties => {
+ realm => get_standard_option('realm'),
+ },
+ },
+ returns => {},
+ code => sub {
+ my ($param) = @_;
+
+ my $cfg = PVE::INotify::read_file($domainconfigfile);
+
+ my $realm = $param->{realm};
+
+ my $data = $cfg->{ids}->{$realm};
+ die "domain '$realm' does not exist\n" if !$data;
+
+ my $type = $data->{type};
+
+ $data->{digest} = $cfg->{digest};
+
+ return $data;
+ }});
+
+
+__PACKAGE__->register_method ({
+ name => 'delete',
+ path => '{realm}',
+ method => 'DELETE',
+ permissions => { check => [ 'admin' ] },
+ description => "Delete an authentication server.",
+ protected => 1,
+ parameters => {
+ additionalProperties => 0,
+ properties => {
+ realm => get_standard_option('realm'),
+ }
+ },
+ returns => { type => 'null' },
+ code => sub {
+ my ($param) = @_;
+
+ PMG::Auth::Plugin::lock_domain_config(
+ sub {
+ my $cfg = PVE::INotify::read_file($domainconfigfile);
+ my $ids = $cfg->{ids};
+ my $realm = $param->{realm};
+
+ die "authentication domain '$realm' does not exist\n" if !$ids->{$realm};
+
+ my $plugin = PMG::Auth::Plugin->lookup($ids->{$realm}->{type});
+
+ $plugin->on_delete_hook($realm, $ids->{$realm});
+
+ delete $ids->{$realm};
+
+ PVE::INotify::write_file($domainconfigfile, $cfg);
+ },
+ "delete auth server failed",
+ );
+ return undef;
+ }});
+
+1;
--
2.39.5
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* [pmg-devel] [PATCH pmg-api v5 7/10] api: oidc login similar to PVE
2025-02-18 16:18 [pmg-devel] [PATCH pve-common/perl-rs/pmg-api/widget-toolkit/pmg-gui v5 0/10] fix #3892: OpenID Connect Markus Frank
` (5 preceding siblings ...)
2025-02-18 16:19 ` [pmg-devel] [PATCH pmg-api v5 6/10] api: add/update/remove realms like in PVE Markus Frank
@ 2025-02-18 16:19 ` Markus Frank
2025-02-19 18:31 ` Stoiko Ivanov
2025-02-21 12:44 ` Fabian Grünbichler
2025-02-18 16:19 ` [pmg-devel] [PATCH widget-toolkit v5 8/10] fix: window: AuthEditBase: rename variable 'realm' to 'type' Markus Frank
` (3 subsequent siblings)
10 siblings, 2 replies; 27+ messages in thread
From: Markus Frank @ 2025-02-18 16:19 UTC (permalink / raw)
To: pmg-devel
Allow OpenID Connect login using the Rust OIDC module.
Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
src/Makefile | 1 +
src/PMG/API2/AccessControl.pm | 7 +
src/PMG/API2/OIDC.pm | 243 ++++++++++++++++++++++++++++++++++
src/PMG/HTTPServer.pm | 2 +
4 files changed, 253 insertions(+)
create mode 100644 src/PMG/API2/OIDC.pm
diff --git a/src/Makefile b/src/Makefile
index e939bbd..7033a66 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -152,6 +152,7 @@ LIBSOURCES = \
PMG/API2/Quarantine.pm \
PMG/API2/AccessControl.pm \
PMG/API2/Realm.pm \
+ PMG/API2/OIDC.pm \
PMG/API2/TFA.pm \
PMG/API2/TFAConfig.pm \
PMG/API2/ObjectGroupHelpers.pm \
diff --git a/src/PMG/API2/AccessControl.pm b/src/PMG/API2/AccessControl.pm
index f4cbc81..1881fac 100644
--- a/src/PMG/API2/AccessControl.pm
+++ b/src/PMG/API2/AccessControl.pm
@@ -13,6 +13,7 @@ use PMG::Utils;
use PMG::UserConfig;
use PMG::AccessControl;
use PMG::API2::Realm;
+use PMG::API2::OIDC;
use PMG::API2::Users;
use PMG::API2::TFA;
use PMG::TFAConfig;
@@ -36,6 +37,11 @@ __PACKAGE__->register_method ({
path => 'domains',
});
+__PACKAGE__->register_method ({
+ subclass => "PMG::API2::OIDC",
+ path => 'oidc',
+});
+
__PACKAGE__->register_method ({
name => 'index',
path => '',
@@ -64,6 +70,7 @@ __PACKAGE__->register_method ({
my $res = [
{ subdir => 'ticket' },
{ subdir => 'domains' },
+ { subdir => 'oidc' },
{ subdir => 'password' },
{ subdir => 'users' },
];
diff --git a/src/PMG/API2/OIDC.pm b/src/PMG/API2/OIDC.pm
new file mode 100644
index 0000000..fdab44d
--- /dev/null
+++ b/src/PMG/API2/OIDC.pm
@@ -0,0 +1,243 @@
+package PMG::API2::OIDC;
+
+use strict;
+use warnings;
+
+use PVE::Tools qw(extract_param lock_file);
+use Proxmox::RS::OIDC;
+
+use PVE::Exception qw(raise raise_perm_exc raise_param_exc);
+use PVE::SafeSyslog;
+use PVE::INotify;
+use PVE::JSONSchema qw(get_standard_option);
+
+use PMG::AccessControl;
+use PMG::RESTEnvironment;
+use PVE::RESTHandler;
+
+use base qw(PVE::RESTHandler);
+
+my $oidc_state_path = "/var/lib/pmg";
+
+my $lookup_oidc_auth = sub {
+ my ($realm, $redirect_url) = @_;
+
+ my $cfg = PVE::INotify::read_file('realms.cfg');
+ my $ids = $cfg->{ids};
+
+ die "authentication domain '$realm' does not exist\n" if !$ids->{$realm};
+
+ my $config = $ids->{$realm};
+ die "wrong realm type ($config->{type} != oidc)\n" if $config->{type} ne "oidc";
+
+ my $oidc_config = {
+ issuer_url => $config->{'issuer-url'},
+ client_id => $config->{'client-id'},
+ client_key => $config->{'client-key'},
+ };
+ $oidc_config->{prompt} = $config->{'prompt'} if defined($config->{'prompt'});
+
+ my $scopes = $config->{'scopes'} // 'email profile';
+ $oidc_config->{scopes} = [ PVE::Tools::split_list($scopes) ];
+
+ if (defined(my $acr = $config->{'acr-values'})) {
+ $oidc_config->{acr_values} = [ PVE::Tools::split_list($acr) ];
+ }
+
+ my $oidc = Proxmox::RS::OIDC->discover($oidc_config, $redirect_url);
+ return ($config, $oidc);
+};
+
+__PACKAGE__->register_method ({
+ name => 'index',
+ path => '',
+ method => 'GET',
+ description => "Directory index.",
+ permissions => {
+ user => 'all',
+ },
+ parameters => {
+ additionalProperties => 0,
+ properties => {},
+ },
+ returns => {
+ type => 'array',
+ items => {
+ type => "object",
+ properties => {
+ subdir => { type => 'string' },
+ },
+ },
+ links => [ { rel => 'child', href => "{subdir}" } ],
+ },
+ code => sub {
+ my ($param) = @_;
+
+ return [
+ { subdir => 'auth-url' },
+ { subdir => 'login' },
+ ];
+ }});
+
+__PACKAGE__->register_method ({
+ name => 'auth_url',
+ path => 'auth-url',
+ method => 'POST',
+ protected => 1,
+ description => "Get the OpenId Connect Authorization Url for the specified realm.",
+ parameters => {
+ additionalProperties => 0,
+ properties => {
+ realm => {
+ description => "Authentication domain ID",
+ type => 'string',
+ pattern => qr/[A-Za-z][A-Za-z0-9\.\-_]+/,
+ maxLength => 32,
+ },
+ 'redirect-url' => {
+ description => "Redirection Url. The client should set this to the used server url (location.origin).",
+ type => 'string',
+ maxLength => 255,
+ },
+ },
+ },
+ returns => {
+ type => "string",
+ description => "Redirection URL.",
+ },
+ permissions => { user => 'world' },
+ code => sub {
+ my ($param) = @_;
+
+ my $realm = extract_param($param, 'realm');
+ my $redirect_url = extract_param($param, 'redirect-url');
+
+ my ($config, $oidc) = $lookup_oidc_auth->($realm, $redirect_url);
+ my $url = $oidc->authorize_url($oidc_state_path , $realm);
+
+ return $url;
+ }});
+
+__PACKAGE__->register_method ({
+ name => 'login',
+ path => 'login',
+ method => 'POST',
+ protected => 1,
+ description => " Verify OpenID Connect authorization code and create a ticket.",
+ parameters => {
+ additionalProperties => 0,
+ properties => {
+ 'state' => {
+ description => "OpenId Connect state.",
+ type => 'string',
+ maxLength => 1024,
+ },
+ code => {
+ description => "OpenId Connect authorization code.",
+ type => 'string',
+ maxLength => 4096,
+ },
+ 'redirect-url' => {
+ description => "Redirection Url. The client should set this to the used server url (location.origin).",
+ type => 'string',
+ maxLength => 255,
+ },
+ },
+ },
+ returns => {
+ properties => {
+ role => { type => 'string', optional => 1},
+ username => { type => 'string' },
+ ticket => { type => 'string' },
+ CSRFPreventionToken => { type => 'string' },
+ },
+ },
+ permissions => { user => 'world' },
+ code => sub {
+ my ($param) = @_;
+
+ my $rpcenv = PMG::RESTEnvironment->get();
+
+ my $res;
+ eval {
+ my ($realm, $private_auth_state) = Proxmox::RS::OIDC::verify_public_auth_state(
+ $oidc_state_path, $param->{'state'});
+
+ my $redirect_url = extract_param($param, 'redirect-url');
+
+ my ($config, $oidc) = $lookup_oidc_auth->($realm, $redirect_url);
+
+ my $info = $oidc->verify_authorization_code($param->{code}, $private_auth_state);
+ my $subject = $info->{'sub'};
+
+ my $unique_name;
+
+ my $user_attr = $config->{'username-claim'} // 'sub';
+ if (defined($info->{$user_attr})) {
+ $unique_name = $info->{$user_attr};
+ } elsif ($user_attr eq 'subject') { # stay compat with old versions
+ $unique_name = $subject;
+ } elsif ($user_attr eq 'username') { # stay compat with old versions
+ my $username = $info->{'preferred_username'};
+ die "missing claim 'preferred_username'\n" if !defined($username);
+ $unique_name = $username;
+ } else {
+ # neither the attr nor fallback are defined in info..
+ die "missing configured claim '$user_attr' in returned info object\n";
+ }
+
+ my $username = "${unique_name}\@${realm}";
+ # first, check if $username respects our naming conventions
+ PMG::Utils::verify_username($username);
+ if ($config->{'autocreate'} && !$rpcenv->check_user_exist($username, 1)) {
+ my $code = sub {
+ my $usercfg = PMG::UserConfig->new();
+
+ my $entry = { enable => 1 };
+ if (defined(my $email = $info->{'email'})) {
+ $entry->{email} = $email;
+ }
+ if (defined(my $given_name = $info->{'given_name'})) {
+ $entry->{firstname} = $given_name;
+ }
+ if (defined(my $family_name = $info->{'family_name'})) {
+ $entry->{lastname} = $family_name;
+ }
+ $entry->{role} = $config->{'autocreate-role'} // 'audit';
+ $entry->{userid} = $username;
+ $entry->{username} = $unique_name;
+ $entry->{realm} = $realm;
+
+ die "User '$username' already exists\n"
+ if $usercfg->{$username};
+
+ $usercfg->{$username} = $entry;
+
+ $usercfg->write();
+ };
+ PMG::UserConfig::lock_config($code, "autocreate openid connect user failed");
+ }
+ my $role = $rpcenv->check_user_enabled($username);
+
+ my $ticket = PMG::Ticket::assemble_ticket($username);
+ my $csrftoken = PMG::Ticket::assemble_csrf_prevention_token($username);
+
+ $res = {
+ ticket => $ticket,
+ username => $username,
+ CSRFPreventionToken => $csrftoken,
+ role => $role,
+ };
+
+ };
+ if (my $err = $@) {
+ my $clientip = $rpcenv->get_client_ip() || '';
+ syslog('err', "openid connect authentication failure; rhost=$clientip msg=$err");
+ # do not return any info to prevent user enumeration attacks
+ die PVE::Exception->new("authentication failure $err\n", code => 401);
+ }
+
+ syslog('info', 'root@pam', "successful openid connect auth for user '$res->{username}'");
+
+ return $res;
+ }});
diff --git a/src/PMG/HTTPServer.pm b/src/PMG/HTTPServer.pm
index 49724fe..27a313d 100644
--- a/src/PMG/HTTPServer.pm
+++ b/src/PMG/HTTPServer.pm
@@ -58,6 +58,8 @@ sub auth_handler {
# explicitly allow some calls without auth
if (($rel_uri eq '/access/domains' && $method eq 'GET') ||
+ ($rel_uri eq '/access/oidc/login' && $method eq 'POST') ||
+ ($rel_uri eq '/access/oidc/auth-url' && $method eq 'POST') ||
($rel_uri eq '/quarantine/sendlink' && ($method eq 'GET' || $method eq 'POST')) ||
($rel_uri eq '/access/ticket' && ($method eq 'GET' || $method eq 'POST'))) {
$require_auth = 0;
--
2.39.5
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* [pmg-devel] [PATCH widget-toolkit v5 8/10] fix: window: AuthEditBase: rename variable 'realm' to 'type'
2025-02-18 16:18 [pmg-devel] [PATCH pve-common/perl-rs/pmg-api/widget-toolkit/pmg-gui v5 0/10] fix #3892: OpenID Connect Markus Frank
` (6 preceding siblings ...)
2025-02-18 16:19 ` [pmg-devel] [PATCH pmg-api v5 7/10] api: oidc login similar to PVE Markus Frank
@ 2025-02-18 16:19 ` Markus Frank
2025-02-21 12:45 ` Fabian Grünbichler
2025-02-18 16:19 ` [pmg-devel] [PATCH pmg-gui v5 09/10] login: add option to login with OIDC realm Markus Frank
` (2 subsequent siblings)
10 siblings, 1 reply; 27+ messages in thread
From: Markus Frank @ 2025-02-18 16:19 UTC (permalink / raw)
To: pmg-devel
PVE/PMG API returns a variable called 'type' instead of 'realm'
Fixes: 3822a031ddbe4136aa847476f2e3785934a41547
Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
src/window/AuthEditBase.js | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/window/AuthEditBase.js b/src/window/AuthEditBase.js
index 73c1fee..e044235 100644
--- a/src/window/AuthEditBase.js
+++ b/src/window/AuthEditBase.js
@@ -91,9 +91,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.realm !== me.authType) {
+ if (!me.useTypeInUrl && data.type !== me.authType) {
me.close();
- throw `got wrong auth type '${me.authType}' for realm '${data.realm}'`;
+ throw `got wrong auth type '${me.authType}' for realm '${data.type}'`;
}
me.setValues(data);
},
--
2.39.5
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* [pmg-devel] [PATCH pmg-gui v5 09/10] login: add option to login with OIDC realm
2025-02-18 16:18 [pmg-devel] [PATCH pve-common/perl-rs/pmg-api/widget-toolkit/pmg-gui v5 0/10] fix #3892: OpenID Connect Markus Frank
` (7 preceding siblings ...)
2025-02-18 16:19 ` [pmg-devel] [PATCH widget-toolkit v5 8/10] fix: window: AuthEditBase: rename variable 'realm' to 'type' Markus Frank
@ 2025-02-18 16:19 ` Markus Frank
2025-02-18 16:19 ` [pmg-devel] [PATCH pmg-gui v5 10/10] add panel for realms to User Management Markus Frank
2025-02-19 18:39 ` [pmg-devel] [PATCH pve-common/perl-rs/pmg-api/widget-toolkit/pmg-gui v5 0/10] fix #3892: OpenID Connect Stoiko Ivanov
10 siblings, 0 replies; 27+ messages in thread
From: Markus Frank @ 2025-02-18 16:19 UTC (permalink / raw)
To: pmg-devel
By adding a viewModel with an oidc variable, the username & password
fields are disabled/hidden when an OIDC realm is selected.
Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
js/LoginView.js | 208 ++++++++++++++++++++++++++++++++++++------------
1 file changed, 157 insertions(+), 51 deletions(-)
diff --git a/js/LoginView.js b/js/LoginView.js
index b5da19a..971eebe 100644
--- a/js/LoginView.js
+++ b/js/LoginView.js
@@ -2,6 +2,21 @@ Ext.define('PMG.LoginView', {
extend: 'Ext.container.Container',
xtype: 'loginview',
+ viewModel: {
+ data: {
+ oidc: false,
+ },
+ formulas: {
+ button_text: function(get) {
+ if (get("oidc") === true) {
+ return gettext("Login (OpenID Connect redirect)");
+ } else {
+ return gettext("Login");
+ }
+ },
+ },
+ },
+
controller: {
xclass: 'Ext.app.ViewController',
@@ -46,50 +61,77 @@ Ext.define('PMG.LoginView', {
submitForm: async function() {
let me = this;
- let view = me.getView();
- let loginForm = me.lookupReference('loginForm');
- var unField = me.lookupReference('usernameField');
- var saveunField = me.lookupReference('saveunField');
- if (loginForm.isValid()) {
- if (loginForm.isVisible()) {
- loginForm.mask(gettext('Please wait...'), 'x-mask-loading');
- }
+ let loginForm = this.lookupReference('loginForm');
+ let unField = this.lookupReference('usernameField');
+ let saveunField = this.lookupReference('saveunField');
+ let view = this.getView();
- // set or clear username for admin view
- if (view.targetview !== 'quarantineview') {
- var sp = Ext.state.Manager.getProvider();
- if (saveunField.getValue() === true) {
- sp.set(unField.getStateId(), unField.getValue());
- } else {
- sp.clear(unField.getStateId());
- }
- sp.set(saveunField.getStateId(), saveunField.getValue());
+ if (!loginForm.isValid()) {
+ return;
+ }
+
+ if (loginForm.isVisible()) {
+ loginForm.mask(gettext('Please wait...'), 'x-mask-loading');
+ }
+
+ // set or clear username for admin view
+ if (view.targetview !== 'quarantineview') {
+ let sp = Ext.state.Manager.getProvider();
+ if (saveunField.getValue() === true) {
+ sp.set(unField.getStateId(), unField.getValue());
+ } else {
+ sp.clear(unField.getStateId());
}
+ sp.set(saveunField.getStateId(), saveunField.getValue());
+ }
+
+ let creds = loginForm.getValues();
- let creds = loginForm.getValues();
+ if (this.getViewModel().data.oidc === true) {
+ const redirectURL = location.origin;
+ Proxmox.Utils.API2Request({
+ url: '/api2/extjs/access/oidc/auth-url',
+ params: {
+ realm: creds.realm,
+ "redirect-url": redirectURL,
+ },
+ method: 'POST',
+ success: function(resp, opts) {
+ window.location = resp.result.data;
+ },
+ failure: function(resp, opts) {
+ Proxmox.Utils.authClear();
+ loginForm.unmask();
+ Ext.MessageBox.alert(
+ gettext('Error'),
+ gettext('OpenID Connect redirect failed.') + `<br>${resp.htmlStatus}`,
+ );
+ },
+ });
+ return;
+ }
- try {
- let resp = await Proxmox.Async.api2({
- url: '/api2/extjs/access/ticket',
- params: creds,
- method: 'POST',
- });
+ try {
+ let resp = await Proxmox.Async.api2({
+ url: '/api2/extjs/access/ticket',
+ params: creds,
+ method: 'POST',
+ });
- let data = resp.result.data;
- if (data.ticket.startsWith('PMG:!tfa!')) {
- data = await me.performTFAChallenge(data);
- }
- PMG.Utils.updateLoginData(data);
- PMG.app.changeView(view.targetview);
- } catch (error) {
- Proxmox.Utils.authClear();
- loginForm.unmask();
- Ext.MessageBox.alert(
- gettext('Error'),
- gettext('Login failed. Please try again'),
- );
+ let data = resp.result.data;
+ if (data.ticket.startsWith('PMG:!tfa!')) {
+ data = await me.performTFAChallenge(data);
}
+ PMG.Utils.updateLoginData(data);
+ PMG.app.changeView(view.targetview);
+ } catch (error) {
+ Proxmox.Utils.authClear();
+ loginForm.unmask();
+ Ext.MessageBox.alert(
+ gettext('Error'),
+ gettext('Login failed. Please try again'),
+ );
}
},
@@ -115,6 +157,15 @@ Ext.define('PMG.LoginView', {
return resp.result.data;
},
+ success: function(data) {
+ let me = this;
+ let view = me.getView();
+ let handler = view.handler || Ext.emptyFn;
+ handler.call(me, data);
+ PMG.Utils.updateLoginData(data);
+ PMG.app.changeView(view.targetview);
+ },
+
openQuarantineLinkWindow: function() {
let me = this;
me.lookup('loginwindow').setVisible(false);
@@ -150,6 +201,14 @@ Ext.define('PMG.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("oidc", data.type === "oidc");
+ },
+ },
'button[reference=quarantineButton]': {
click: 'openQuarantineLinkWindow',
},
@@ -161,19 +220,54 @@ Ext.define('PMG.LoginView', {
let me = this;
let view = me.getView();
if (view.targetview !== 'quarantineview') {
- var sp = Ext.state.Manager.getProvider();
- var checkboxField = this.lookupReference('saveunField');
- var unField = this.lookupReference('usernameField');
+ let sp = Ext.state.Manager.getProvider();
+ let checkboxField = this.lookupReference('saveunField');
+ let unField = this.lookupReference('usernameField');
- var checked = sp.get(checkboxField.getStateId());
+ let checked = sp.get(checkboxField.getStateId());
checkboxField.setValue(checked);
if (checked === true) {
- var username = sp.get(unField.getStateId());
+ let username = sp.get(unField.getStateId());
unField.setValue(username);
- var pwField = this.lookupReference('passwordField');
+ let pwField = this.lookupReference('passwordField');
pwField.focus();
}
+
+ let auth = Proxmox.Utils.getOpenIDRedirectionAuthorization();
+ if (auth !== undefined) {
+ Proxmox.Utils.authClear();
+
+ let loginForm = this.lookupReference('loginForm');
+ loginForm.mask(gettext('OpenID Connect login - please wait...'), 'x-mask-loading');
+
+ const redirectURL = location.origin;
+
+ Proxmox.Utils.API2Request({
+ url: '/api2/extjs/access/oidc/login',
+ params: {
+ state: auth.state,
+ code: auth.code,
+ "redirect-url": redirectURL,
+ },
+ method: 'POST',
+ failure: function(response) {
+ loginForm.unmask();
+ let error = response.htmlStatus;
+ Ext.MessageBox.alert(
+ gettext('Error'),
+ gettext('OpenID Connect login failed, please try again') + `<br>${error}`,
+ () => { window.location = redirectURL; },
+ );
+ },
+ success: function(response, options) {
+ loginForm.unmask();
+ let data = response.result.data;
+ history.replaceState(null, '', redirectURL);
+ me.success(data);
+ },
+ });
+ }
}
},
},
@@ -250,6 +344,10 @@ Ext.define('PMG.LoginView', {
reference: 'usernameField',
stateId: 'login-username',
inputAttrTpl: 'autocomplete=username',
+ bind: {
+ visible: "{!oidc}",
+ disabled: "{oidc}",
+ },
},
{
xtype: 'textfield',
@@ -258,6 +356,16 @@ Ext.define('PMG.LoginView', {
name: 'password',
reference: 'passwordField',
inputAttrTpl: 'autocomplete=current-password',
+ bind: {
+ visible: "{!oidc}",
+ disabled: "{oidc}",
+ },
+ },
+ {
+ xtype: 'pmxRealmComboBox',
+ reference: 'realmfield',
+ name: 'realm',
+ value: 'pam',
},
{
xtype: 'proxmoxLanguageSelector',
@@ -266,12 +374,6 @@ Ext.define('PMG.LoginView', {
name: 'lang',
submitValue: false,
},
- {
- xtype: 'hiddenfield',
- reference: 'realmfield',
- name: 'realm',
- value: 'pmg',
- },
],
buttons: [
{
@@ -283,15 +385,19 @@ Ext.define('PMG.LoginView', {
labelAlign: 'right',
labelWidth: 150,
submitValue: false,
+ bind: {
+ visible: "{!oidc}",
+ },
},
{
text: gettext('Request Quarantine Link'),
reference: 'quarantineButton',
},
{
- text: gettext('Login'),
+ bind: {
+ text: "{button_text}",
+ },
reference: 'loginButton',
- formBind: true,
},
],
},
--
2.39.5
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* [pmg-devel] [PATCH pmg-gui v5 10/10] add panel for realms to User Management
2025-02-18 16:18 [pmg-devel] [PATCH pve-common/perl-rs/pmg-api/widget-toolkit/pmg-gui v5 0/10] fix #3892: OpenID Connect Markus Frank
` (8 preceding siblings ...)
2025-02-18 16:19 ` [pmg-devel] [PATCH pmg-gui v5 09/10] login: add option to login with OIDC realm Markus Frank
@ 2025-02-18 16:19 ` Markus Frank
2025-02-21 9:22 ` Christoph Heiss
2025-02-21 12:45 ` Fabian Grünbichler
2025-02-19 18:39 ` [pmg-devel] [PATCH pve-common/perl-rs/pmg-api/widget-toolkit/pmg-gui v5 0/10] fix #3892: OpenID Connect Stoiko Ivanov
10 siblings, 2 replies; 27+ messages in thread
From: Markus Frank @ 2025-02-18 16:19 UTC (permalink / raw)
To: pmg-devel
Make the realm configuration available in PMG and disable LDAP/AD
realms for now and use the name oidc instead of openid.
Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
js/UserManagement.js | 6 ++++++
js/Utils.js | 23 +++++++++++++++++++++++
2 files changed, 29 insertions(+)
diff --git a/js/UserManagement.js b/js/UserManagement.js
index 65fabbf..54493b1 100644
--- a/js/UserManagement.js
+++ b/js/UserManagement.js
@@ -34,5 +34,11 @@ Ext.define('PMG.UserManagement', {
itemId: 'pop',
iconCls: 'fa fa-reply-all',
},
+ {
+ xtype: 'pmxAuthView',
+ title: gettext('Realms'),
+ itemId: 'domains',
+ iconCls: 'fa fa-address-book-o',
+ },
],
});
diff --git a/js/Utils.js b/js/Utils.js
index 9b5f054..055b938 100644
--- a/js/Utils.js
+++ b/js/Utils.js
@@ -851,6 +851,29 @@ Ext.define('PMG.Utils', {
constructor: function() {
var me = this;
+ // use oidc instead of openid
+ Proxmox.Schema.authDomains.oidc = Proxmox.Schema.authDomains.openid;
+ Proxmox.Schema.authDomains.oidc.useTypeInUrl = false;
+ delete Proxmox.Schema.authDomains.openid;
+
+ // Disable LDAP/AD as a realm until LDAP/AD login is implemented
+ Proxmox.Schema.authDomains.ldap.add = false;
+ Proxmox.Schema.authDomains.ad.add = false;
+
+ Proxmox.Schema.authDomains.pmg = {
+ add: false,
+ edit: false,
+ pwchange: false,
+ sync: false,
+ };
+
+ Proxmox.Schema.authDomains.pam = {
+ add: false,
+ edit: false,
+ pwchange: false,
+ sync: false,
+ };
+
// do whatever you want here
Proxmox.Utils.override_task_descriptions({
applycustomscores: ['', gettext('Apply custom SpamAssassin scores')],
--
2.39.5
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [pmg-devel] [PATCH pve-common v5 1/10] add Schema package with auth module that contains realm sync options
2025-02-18 16:18 ` [pmg-devel] [PATCH pve-common v5 1/10] add Schema package with auth module that contains realm sync options Markus Frank
@ 2025-02-19 18:18 ` Stoiko Ivanov
2025-02-21 12:22 ` Fabian Grünbichler
1 sibling, 0 replies; 27+ messages in thread
From: Stoiko Ivanov @ 2025-02-19 18:18 UTC (permalink / raw)
To: Markus Frank; +Cc: pmg-devel
On Tue, Feb 18, 2025 at 05:18:56PM +0100, Markus Frank wrote:
> This is because these standard options & formats are used by both PVE
> and PMG.
I'd add the source of the data to the commit messages, as part of the
information is based on the implementation in PVE (it helps if you need to
understand why certain things are implemented as they are
e.g. for this patch
schema-definitions are based on pve-access-control/src/PVE/Auth/Plugin.pm
>
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> ---
> src/Makefile | 2 ++
> src/PVE/Schema/Auth.pm | 82 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 84 insertions(+)
> create mode 100644 src/PVE/Schema/Auth.pm
>
> diff --git a/src/Makefile b/src/Makefile
> index 2d8bdc4..833bbc1 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -29,6 +29,7 @@ LIB_SOURCES = \
> RESTEnvironment.pm \
> RESTHandler.pm \
> SafeSyslog.pm \
> + Schema/Auth.pm \
> SectionConfig.pm \
> SysFSTools.pm \
> Syscall.pm \
> @@ -41,6 +42,7 @@ all:
> install: $(addprefix PVE/,${LIB_SOURCES})
> install -d -m 0755 ${DESTDIR}${PERLDIR}/PVE
> install -d -m 0755 ${DESTDIR}${PERLDIR}/PVE/Job
> + install -d -m 0755 ${DESTDIR}${PERLDIR}/PVE/Schema
> for i in ${LIB_SOURCES}; do install -D -m 0644 PVE/$$i ${DESTDIR}${PERLDIR}/PVE/$$i; done
>
>
> diff --git a/src/PVE/Schema/Auth.pm b/src/PVE/Schema/Auth.pm
> new file mode 100644
> index 0000000..1f7f586
> --- /dev/null
> +++ b/src/PVE/Schema/Auth.pm
> @@ -0,0 +1,82 @@
> +package PVE::Schema::Auth;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::JSONSchema qw(get_standard_option parse_property_string);
> +
> +my $remove_options = "(?:acl|properties|entry)";
> +
> +PVE::JSONSchema::register_standard_option('sync-scope', {
> + description => "Select what to sync.",
> + type => 'string',
> + enum => [qw(users groups both)],
> + optional => '1',
> +});
> +
> +PVE::JSONSchema::register_standard_option('sync-remove-vanished', {
> + description => "A semicolon-seperated list of things to remove when they or the user"
> + ." vanishes during a sync. The following values are possible: 'entry' removes the"
> + ." user/group when not returned from the sync. 'properties' removes the set"
> + ." properties on existing user/group that do not appear in the source (even custom ones)."
> + ." 'acl' removes acls when the user/group is not returned from the sync."
> + ." Instead of a list it also can be 'none' (the default).",
> + type => 'string',
> + default => 'none',
> + typetext => "([acl];[properties];[entry])|none",
> + pattern => "(?:(?:$remove_options\;)*$remove_options)|none",
> + optional => '1',
> +});
> +
> +my $realm_sync_options_desc = {
> + scope => get_standard_option('sync-scope'),
> + 'remove-vanished' => get_standard_option('sync-remove-vanished'),
> + 'enable-new' => {
> + description => "Enable newly synced users immediately.",
> + type => 'boolean',
> + default => '1',
> + optional => '1',
> + },
> +};
> +PVE::JSONSchema::register_standard_option('realm-sync-options', $realm_sync_options_desc);
> +PVE::JSONSchema::register_format('realm-sync-options', $realm_sync_options_desc);
> +
> +my $tfa_format = {
> + type => {
> + description => "The type of 2nd factor authentication.",
> + format_description => 'TFATYPE',
> + type => 'string',
> + enum => [qw(oath)],
> + },
> + digits => {
> + description => "TOTP digits.",
> + format_description => 'COUNT',
> + type => 'integer',
> + minimum => 6, maximum => 8,
> + default => 6,
> + optional => 1,
> + },
> + step => {
> + description => "TOTP time period.",
> + format_description => 'SECONDS',
> + type => 'integer',
> + minimum => 10,
> + default => 30,
> + optional => 1,
> + },
> +};
> +
> +PVE::JSONSchema::register_format('pve-tfa-config', $tfa_format);
> +
> +PVE::JSONSchema::register_standard_option('tfa', {
> + description => "Use Two-factor authentication.",
> + type => 'string', format => 'pve-tfa-config',
> + optional => 1,
> + maxLength => 128,
> +});
> +
> +sub parse_tfa_config {
> + my ($data) = @_;
> +
> + return parse_property_string($tfa_format, $data);
> +}
> --
> 2.39.5
>
>
>
> _______________________________________________
> pmg-devel mailing list
> pmg-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
>
>
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [pmg-devel] [PATCH pmg-api v5 7/10] api: oidc login similar to PVE
2025-02-18 16:19 ` [pmg-devel] [PATCH pmg-api v5 7/10] api: oidc login similar to PVE Markus Frank
@ 2025-02-19 18:31 ` Stoiko Ivanov
2025-02-21 12:44 ` Fabian Grünbichler
1 sibling, 0 replies; 27+ messages in thread
From: Stoiko Ivanov @ 2025-02-19 18:31 UTC (permalink / raw)
To: Markus Frank; +Cc: pmg-devel
On Tue, Feb 18, 2025 at 05:19:02PM +0100, Markus Frank wrote:
> Allow OpenID Connect login using the Rust OIDC module.
>
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> ---
> src/Makefile | 1 +
> src/PMG/API2/AccessControl.pm | 7 +
> src/PMG/API2/OIDC.pm | 243 ++++++++++++++++++++++++++++++++++
> src/PMG/HTTPServer.pm | 2 +
> 4 files changed, 253 insertions(+)
> create mode 100644 src/PMG/API2/OIDC.pm
>
> diff --git a/src/Makefile b/src/Makefile
> index e939bbd..7033a66 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -152,6 +152,7 @@ LIBSOURCES = \
> PMG/API2/Quarantine.pm \
> PMG/API2/AccessControl.pm \
> PMG/API2/Realm.pm \
> + PMG/API2/OIDC.pm \
> PMG/API2/TFA.pm \
> PMG/API2/TFAConfig.pm \
> PMG/API2/ObjectGroupHelpers.pm \
> diff --git a/src/PMG/API2/AccessControl.pm b/src/PMG/API2/AccessControl.pm
> index f4cbc81..1881fac 100644
> --- a/src/PMG/API2/AccessControl.pm
> +++ b/src/PMG/API2/AccessControl.pm
> @@ -13,6 +13,7 @@ use PMG::Utils;
> use PMG::UserConfig;
> use PMG::AccessControl;
> use PMG::API2::Realm;
> +use PMG::API2::OIDC;
> use PMG::API2::Users;
> use PMG::API2::TFA;
> use PMG::TFAConfig;
> @@ -36,6 +37,11 @@ __PACKAGE__->register_method ({
> path => 'domains',
> });
>
> +__PACKAGE__->register_method ({
> + subclass => "PMG::API2::OIDC",
> + path => 'oidc',
> +});
> +
> __PACKAGE__->register_method ({
> name => 'index',
> path => '',
> @@ -64,6 +70,7 @@ __PACKAGE__->register_method ({
> my $res = [
> { subdir => 'ticket' },
> { subdir => 'domains' },
> + { subdir => 'oidc' },
> { subdir => 'password' },
> { subdir => 'users' },
> ];
> diff --git a/src/PMG/API2/OIDC.pm b/src/PMG/API2/OIDC.pm
> new file mode 100644
> index 0000000..fdab44d
> --- /dev/null
> +++ b/src/PMG/API2/OIDC.pm
> @@ -0,0 +1,243 @@
> +package PMG::API2::OIDC;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::Tools qw(extract_param lock_file);
> +use Proxmox::RS::OIDC;
> +
> +use PVE::Exception qw(raise raise_perm_exc raise_param_exc);
> +use PVE::SafeSyslog;
> +use PVE::INotify;
> +use PVE::JSONSchema qw(get_standard_option);
> +
> +use PMG::AccessControl;
> +use PMG::RESTEnvironment;
> +use PVE::RESTHandler;
> +
> +use base qw(PVE::RESTHandler);
> +
> +my $oidc_state_path = "/var/lib/pmg";
> +
> +my $lookup_oidc_auth = sub {
> + my ($realm, $redirect_url) = @_;
> +
> + my $cfg = PVE::INotify::read_file('realms.cfg');
> + my $ids = $cfg->{ids};
> +
> + die "authentication domain '$realm' does not exist\n" if !$ids->{$realm};
> +
> + my $config = $ids->{$realm};
> + die "wrong realm type ($config->{type} != oidc)\n" if $config->{type} ne "oidc";
> +
> + my $oidc_config = {
> + issuer_url => $config->{'issuer-url'},
> + client_id => $config->{'client-id'},
> + client_key => $config->{'client-key'},
> + };
> + $oidc_config->{prompt} = $config->{'prompt'} if defined($config->{'prompt'});
> +
> + my $scopes = $config->{'scopes'} // 'email profile';
> + $oidc_config->{scopes} = [ PVE::Tools::split_list($scopes) ];
> +
> + if (defined(my $acr = $config->{'acr-values'})) {
> + $oidc_config->{acr_values} = [ PVE::Tools::split_list($acr) ];
> + }
> +
> + my $oidc = Proxmox::RS::OIDC->discover($oidc_config, $redirect_url);
> + return ($config, $oidc);
> +};
> +
> +__PACKAGE__->register_method ({
> + name => 'index',
> + path => '',
> + method => 'GET',
> + description => "Directory index.",
> + permissions => {
> + user => 'all',
> + },
> + parameters => {
> + additionalProperties => 0,
> + properties => {},
> + },
> + returns => {
> + type => 'array',
> + items => {
> + type => "object",
> + properties => {
> + subdir => { type => 'string' },
> + },
> + },
> + links => [ { rel => 'child', href => "{subdir}" } ],
> + },
> + code => sub {
> + my ($param) = @_;
> +
> + return [
> + { subdir => 'auth-url' },
> + { subdir => 'login' },
> + ];
> + }});
> +
> +__PACKAGE__->register_method ({
> + name => 'auth_url',
> + path => 'auth-url',
> + method => 'POST',
> + protected => 1,
> + description => "Get the OpenId Connect Authorization Url for the specified realm.",
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + realm => {
> + description => "Authentication domain ID",
> + type => 'string',
> + pattern => qr/[A-Za-z][A-Za-z0-9\.\-_]+/,
> + maxLength => 32,
> + },
> + 'redirect-url' => {
> + description => "Redirection Url. The client should set this to the used server url (location.origin).",
> + type => 'string',
> + maxLength => 255,
> + },
> + },
> + },
> + returns => {
> + type => "string",
> + description => "Redirection URL.",
> + },
> + permissions => { user => 'world' },
> + code => sub {
> + my ($param) = @_;
> +
> + my $realm = extract_param($param, 'realm');
> + my $redirect_url = extract_param($param, 'redirect-url');
> +
> + my ($config, $oidc) = $lookup_oidc_auth->($realm, $redirect_url);
> + my $url = $oidc->authorize_url($oidc_state_path , $realm);
> +
> + return $url;
> + }});
> +
> +__PACKAGE__->register_method ({
> + name => 'login',
> + path => 'login',
> + method => 'POST',
> + protected => 1,
> + description => " Verify OpenID Connect authorization code and create a ticket.",
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + 'state' => {
> + description => "OpenId Connect state.",
> + type => 'string',
> + maxLength => 1024,
> + },
> + code => {
> + description => "OpenId Connect authorization code.",
> + type => 'string',
> + maxLength => 4096,
> + },
> + 'redirect-url' => {
> + description => "Redirection Url. The client should set this to the used server url (location.origin).",
> + type => 'string',
> + maxLength => 255,
> + },
> + },
> + },
> + returns => {
> + properties => {
> + role => { type => 'string', optional => 1},
> + username => { type => 'string' },
> + ticket => { type => 'string' },
> + CSRFPreventionToken => { type => 'string' },
> + },
> + },
> + permissions => { user => 'world' },
> + code => sub {
> + my ($param) = @_;
> +
> + my $rpcenv = PMG::RESTEnvironment->get();
> +
> + my $res;
> + eval {
> + my ($realm, $private_auth_state) = Proxmox::RS::OIDC::verify_public_auth_state(
> + $oidc_state_path, $param->{'state'});
> +
> + my $redirect_url = extract_param($param, 'redirect-url');
> +
> + my ($config, $oidc) = $lookup_oidc_auth->($realm, $redirect_url);
> +
> + my $info = $oidc->verify_authorization_code($param->{code}, $private_auth_state);
> + my $subject = $info->{'sub'};
> +
> + my $unique_name;
> +
> + my $user_attr = $config->{'username-claim'} // 'sub';
> + if (defined($info->{$user_attr})) {
> + $unique_name = $info->{$user_attr};
> + } elsif ($user_attr eq 'subject') { # stay compat with old versions
> + $unique_name = $subject;
> + } elsif ($user_attr eq 'username') { # stay compat with old versions
> + my $username = $info->{'preferred_username'};
> + die "missing claim 'preferred_username'\n" if !defined($username);
> + $unique_name = $username;
> + } else {
> + # neither the attr nor fallback are defined in info..
> + die "missing configured claim '$user_attr' in returned info object\n";
> + }
> +
> + my $username = "${unique_name}\@${realm}";
> + # first, check if $username respects our naming conventions
> + PMG::Utils::verify_username($username);
> + if ($config->{'autocreate'} && !$rpcenv->check_user_exist($username, 1)) {
> + my $code = sub {
> + my $usercfg = PMG::UserConfig->new();
> +
> + my $entry = { enable => 1 };
> + if (defined(my $email = $info->{'email'})) {
> + $entry->{email} = $email;
> + }
> + if (defined(my $given_name = $info->{'given_name'})) {
> + $entry->{firstname} = $given_name;
> + }
> + if (defined(my $family_name = $info->{'family_name'})) {
> + $entry->{lastname} = $family_name;
> + }
> + $entry->{role} = $config->{'autocreate-role'} // 'audit';
> + $entry->{userid} = $username;
> + $entry->{username} = $unique_name;
> + $entry->{realm} = $realm;
> +
> + die "User '$username' already exists\n"
> + if $usercfg->{$username};
> +
> + $usercfg->{$username} = $entry;
> +
> + $usercfg->write();
> + };
> + PMG::UserConfig::lock_config($code, "autocreate openid connect user failed");
this part failed in a testsetup of Mira - with a user who did not have an
e-mail set:
$usercfg->write() above calls verify_entry in PMG::UserConfig, which checks
if the entries match the schema.
I could not reproduce this with Keycloak easily (as it refuses to store
something as e-mail which does not match its validation, but maybe
Authentik, which Mira uses? sends an empty string if no e-mail is set.
from a quick look I think PVE does not verify the schema (or accepts '' as
a valid entry)
depending on if it's authentik sending '' as non-defined e-mail we maybe
should change the
if (defined(my $email = $info->{'email'})
to if (my $email = $info->{'email'}
> + }
> + my $role = $rpcenv->check_user_enabled($username);
> +
> + my $ticket = PMG::Ticket::assemble_ticket($username);
> + my $csrftoken = PMG::Ticket::assemble_csrf_prevention_token($username);
> +
> + $res = {
> + ticket => $ticket,
> + username => $username,
> + CSRFPreventionToken => $csrftoken,
> + role => $role,
> + };
> +
> + };
> + if (my $err = $@) {
> + my $clientip = $rpcenv->get_client_ip() || '';
> + syslog('err', "openid connect authentication failure; rhost=$clientip msg=$err");
> + # do not return any info to prevent user enumeration attacks
> + die PVE::Exception->new("authentication failure $err\n", code => 401);
> + }
> +
> + syslog('info', 'root@pam', "successful openid connect auth for user '$res->{username}'");
this causes
```
Redundant argument in sprintf at /usr/lib/x86_64-linux-gnu/perl/5.36/Sys/Syslog.pm line 454.
```
to be printed to the syslog (the 'root@pam' argument should simply be
dropped) - the remaining log calls look ok from a quick glance.
> + return $res;
> + }});
> diff --git a/src/PMG/HTTPServer.pm b/src/PMG/HTTPServer.pm
> index 49724fe..27a313d 100644
> --- a/src/PMG/HTTPServer.pm
> +++ b/src/PMG/HTTPServer.pm
> @@ -58,6 +58,8 @@ sub auth_handler {
>
> # explicitly allow some calls without auth
> if (($rel_uri eq '/access/domains' && $method eq 'GET') ||
> + ($rel_uri eq '/access/oidc/login' && $method eq 'POST') ||
> + ($rel_uri eq '/access/oidc/auth-url' && $method eq 'POST') ||
> ($rel_uri eq '/quarantine/sendlink' && ($method eq 'GET' || $method eq 'POST')) ||
> ($rel_uri eq '/access/ticket' && ($method eq 'GET' || $method eq 'POST'))) {
> $require_auth = 0;
> --
> 2.39.5
>
>
>
> _______________________________________________
> pmg-devel mailing list
> pmg-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
>
>
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [pmg-devel] [PATCH pve-common/perl-rs/pmg-api/widget-toolkit/pmg-gui v5 0/10] fix #3892: OpenID Connect
2025-02-18 16:18 [pmg-devel] [PATCH pve-common/perl-rs/pmg-api/widget-toolkit/pmg-gui v5 0/10] fix #3892: OpenID Connect Markus Frank
` (9 preceding siblings ...)
2025-02-18 16:19 ` [pmg-devel] [PATCH pmg-gui v5 10/10] add panel for realms to User Management Markus Frank
@ 2025-02-19 18:39 ` Stoiko Ivanov
10 siblings, 0 replies; 27+ messages in thread
From: Stoiko Ivanov @ 2025-02-19 18:39 UTC (permalink / raw)
To: Markus Frank; +Cc: pmg-devel
Thanks for the fast iteration on this!
applied the patches and compared them to your v4 - looks good from my
perspective and quick tests!
gave it another spin - my tests worked ok - one small glitch (wrong
syslog() call), and one thing that still needs a closer check (empty e-mail
addresses for OIDC users when 'autocreate' is set - see the comment on
patch 7/10.
if you send a v6, please add a hint to the patches which are based on code
from PVE where the code for PVE is (as suggested in the comment on 1/10).
On Tue, Feb 18, 2025 at 05:18:55PM +0100, Markus Frank wrote:
> Patch-series to enable OpenID Connect Login for PMG
>
> apply/compile order:
>
> pve-common:
> 1 add Schema package with auth module that contains realm sync options
>
> proxmox-perl-rs:
> 2 move openid code from pve-rs to common
> 3 remove empty PMG::RS::OpenId package to avoid confusion
>
> pmg-api:
> 4 config: add plugin system for realms
> 5 config: add oidc type realm
> 6 api: add/update/remove realms like in PVE
> 7 api: oidc login similar to PVE
>
> proxmox-widget-toolkit:
> 8 fix: window: AuthEditBase: rename variable 'realm' to 'type'
>
> pmg-gui:
> 9 login: add option to login with OIDC realm
> 10 add panel for realms to User Management
>
>
> v5:
> * renamed openid/OpenId variables, filenames and modules to oidc/OIDC
> wherever possible
> * renamed Authdomains to Realm
>
> v4:
> * split "config: add plugin system for realms & add openid type realms"
> patch into two patches
> * use the name 'OpenId' for filenames, but use 'OIDC' as realm type name
> * added autocreate-role option to set the role for automatically created
> users in a realm, but currently not exposed in GUI (needs a lot of
> changes in pmg-gui and proxmox-widget-toolkit)
>
>
>
> pve-common:
>
> Markus Frank (1):
> add Schema package with auth module that contains realm sync options
>
> src/Makefile | 2 ++
> src/PVE/Schema/Auth.pm | 82 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 84 insertions(+)
> create mode 100644 src/PVE/Schema/Auth.pm
>
>
> proxmox-perl-rs:
>
> Markus Frank (2):
> move openid code from pve-rs to common
> remove empty PMG::RS::OpenId package to avoid confusion
>
> common/pkg/Makefile | 1 +
> common/src/mod.rs | 1 +
> common/src/oidc/mod.rs | 63 ++++++++++++++++++++++++++++++++++++++++
> pmg-rs/Cargo.toml | 1 +
> pmg-rs/Makefile | 1 -
> pmg-rs/debian/control | 1 +
> pve-rs/src/openid/mod.rs | 32 +++++---------------
> 7 files changed, 75 insertions(+), 25 deletions(-)
> create mode 100644 common/src/oidc/mod.rs
>
>
> pmg-api:
>
> Markus Frank (4):
> config: add plugin system for realms
> config: add oidc type realm
> api: add/update/remove realms like in PVE
> api: oidc login similar to PVE
>
> src/Makefile | 6 +
> src/PMG/API2/AccessControl.pm | 17 ++-
> src/PMG/API2/OIDC.pm | 243 ++++++++++++++++++++++++++++++
> src/PMG/API2/Realm.pm | 274 ++++++++++++++++++++++++++++++++++
> src/PMG/AccessControl.pm | 33 ++++
> src/PMG/Auth/OIDC.pm | 95 ++++++++++++
> src/PMG/Auth/PAM.pm | 22 +++
> src/PMG/Auth/PMG.pm | 39 +++++
> src/PMG/Auth/Plugin.pm | 199 ++++++++++++++++++++++++
> src/PMG/HTTPServer.pm | 2 +
> src/PMG/RESTEnvironment.pm | 14 ++
> src/PMG/UserConfig.pm | 25 ++--
> src/PMG/Utils.pm | 29 +++-
> 13 files changed, 981 insertions(+), 17 deletions(-)
> create mode 100644 src/PMG/API2/OIDC.pm
> create mode 100644 src/PMG/API2/Realm.pm
> create mode 100755 src/PMG/Auth/OIDC.pm
> create mode 100755 src/PMG/Auth/PAM.pm
> create mode 100755 src/PMG/Auth/PMG.pm
> create mode 100755 src/PMG/Auth/Plugin.pm
>
>
> widget-toolkit:
>
> Markus Frank (1):
> fix: window: AuthEditBase: rename variable 'realm' to 'type'
>
> src/window/AuthEditBase.js | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>
> pmg-gui:
>
> Markus Frank (2):
> login: add option to login with OIDC realm
> add panel for realms to User Management
>
> js/LoginView.js | 208 ++++++++++++++++++++++++++++++++-----------
> js/UserManagement.js | 6 ++
> js/Utils.js | 23 +++++
> 3 files changed, 186 insertions(+), 51 deletions(-)
>
> --
> 2.39.5
>
>
>
> _______________________________________________
> pmg-devel mailing list
> pmg-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
>
>
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [pmg-devel] [PATCH pmg-gui v5 10/10] add panel for realms to User Management
2025-02-18 16:19 ` [pmg-devel] [PATCH pmg-gui v5 10/10] add panel for realms to User Management Markus Frank
@ 2025-02-21 9:22 ` Christoph Heiss
2025-02-21 12:45 ` Fabian Grünbichler
1 sibling, 0 replies; 27+ messages in thread
From: Christoph Heiss @ 2025-02-21 9:22 UTC (permalink / raw)
To: Markus Frank; +Cc: pmg-devel
On Tue Feb 18, 2025 at 5:19 PM CET, Markus Frank wrote:
> diff --git a/js/Utils.js b/js/Utils.js
> index 9b5f054..055b938 100644
> --- a/js/Utils.js
> +++ b/js/Utils.js
> @@ -851,6 +851,29 @@ Ext.define('PMG.Utils', {
[..]
> +
> + Proxmox.Schema.authDomains.pam = {
> + add: false,
> + edit: false,
> + pwchange: false,
> + sync: false,
> + };
nit: `add: false` and `sync: false` are already default values for PAM
as defined in proxmox-widget-toolkit, so could be dropped here.
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [pmg-devel] [PATCH pve-common v5 1/10] add Schema package with auth module that contains realm sync options
2025-02-18 16:18 ` [pmg-devel] [PATCH pve-common v5 1/10] add Schema package with auth module that contains realm sync options Markus Frank
2025-02-19 18:18 ` Stoiko Ivanov
@ 2025-02-21 12:22 ` Fabian Grünbichler
1 sibling, 0 replies; 27+ messages in thread
From: Fabian Grünbichler @ 2025-02-21 12:22 UTC (permalink / raw)
To: Markus Frank, pmg-devel
> Markus Frank <m.frank@proxmox.com> hat am 18.02.2025 17:18 CET geschrieben:
> This is because these standard options & formats are used by both PVE
> and PMG.
but this is not yet true - it's only used by PMG, and duplicated from the one in PVE? I am not sure if it makes sense to extract only this part and put it into common code, because the rest of the auth things are still very much different between PVE/PBS/PMG.. or are you specifically working on unifying those and this is the first step of doing so? ;)
>
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> ---
> src/Makefile | 2 ++
> src/PVE/Schema/Auth.pm | 82 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 84 insertions(+)
> create mode 100644 src/PVE/Schema/Auth.pm
>
> diff --git a/src/Makefile b/src/Makefile
> index 2d8bdc4..833bbc1 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -29,6 +29,7 @@ LIB_SOURCES = \
> RESTEnvironment.pm \
> RESTHandler.pm \
> SafeSyslog.pm \
> + Schema/Auth.pm \
> SectionConfig.pm \
> SysFSTools.pm \
> Syscall.pm \
> @@ -41,6 +42,7 @@ all:
> install: $(addprefix PVE/,${LIB_SOURCES})
> install -d -m 0755 ${DESTDIR}${PERLDIR}/PVE
> install -d -m 0755 ${DESTDIR}${PERLDIR}/PVE/Job
> + install -d -m 0755 ${DESTDIR}${PERLDIR}/PVE/Schema
> for i in ${LIB_SOURCES}; do install -D -m 0644 PVE/$$i ${DESTDIR}${PERLDIR}/PVE/$$i; done
>
>
> diff --git a/src/PVE/Schema/Auth.pm b/src/PVE/Schema/Auth.pm
> new file mode 100644
> index 0000000..1f7f586
> --- /dev/null
> +++ b/src/PVE/Schema/Auth.pm
> @@ -0,0 +1,82 @@
> +package PVE::Schema::Auth;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::JSONSchema qw(get_standard_option parse_property_string);
> +
> +my $remove_options = "(?:acl|properties|entry)";
> +
> +PVE::JSONSchema::register_standard_option('sync-scope', {
> + description => "Select what to sync.",
> + type => 'string',
> + enum => [qw(users groups both)],
for example - PMG doesn't have groups like PVE, so this doesn't make any sense?
> + optional => '1',
> +});
> +
> +PVE::JSONSchema::register_standard_option('sync-remove-vanished', {
> + description => "A semicolon-seperated list of things to remove when they or the user"
> + ." vanishes during a sync. The following values are possible: 'entry' removes the"
> + ." user/group when not returned from the sync. 'properties' removes the set"
> + ." properties on existing user/group that do not appear in the source (even custom ones)."
> + ." 'acl' removes acls when the user/group is not returned from the sync."
> + ." Instead of a list it also can be 'none' (the default).",
> + type => 'string',
> + default => 'none',
> + typetext => "([acl];[properties];[entry])|none",
> + pattern => "(?:(?:$remove_options\;)*$remove_options)|none",
> + optional => '1',
> +});
> +
> +my $realm_sync_options_desc = {
> + scope => get_standard_option('sync-scope'),
> + 'remove-vanished' => get_standard_option('sync-remove-vanished'),
> + 'enable-new' => {
> + description => "Enable newly synced users immediately.",
> + type => 'boolean',
> + default => '1',
> + optional => '1',
> + },
> +};
> +PVE::JSONSchema::register_standard_option('realm-sync-options', $realm_sync_options_desc);
> +PVE::JSONSchema::register_format('realm-sync-options', $realm_sync_options_desc);
> +
> +my $tfa_format = {
> + type => {
> + description => "The type of 2nd factor authentication.",
> + format_description => 'TFATYPE',
> + type => 'string',
> + enum => [qw(oath)],
> + },
> + digits => {
> + description => "TOTP digits.",
> + format_description => 'COUNT',
> + type => 'integer',
> + minimum => 6, maximum => 8,
> + default => 6,
> + optional => 1,
> + },
> + step => {
> + description => "TOTP time period.",
> + format_description => 'SECONDS',
> + type => 'integer',
> + minimum => 10,
> + default => 30,
> + optional => 1,
> + },
> +};
> +
> +PVE::JSONSchema::register_format('pve-tfa-config', $tfa_format);
> +
> +PVE::JSONSchema::register_standard_option('tfa', {
> + description => "Use Two-factor authentication.",
> + type => 'string', format => 'pve-tfa-config',
> + optional => 1,
> + maxLength => 128,
> +});
> +
> +sub parse_tfa_config {
> + my ($data) = @_;
> +
> + return parse_property_string($tfa_format, $data);
> +}
> --
> 2.39.5
>
>
>
> _______________________________________________
> pmg-devel mailing list
> pmg-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [pmg-devel] [PATCH proxmox-perl-rs v5 2/10] move openid code from pve-rs to common
2025-02-18 16:18 ` [pmg-devel] [PATCH proxmox-perl-rs v5 2/10] move openid code from pve-rs to common Markus Frank
@ 2025-02-21 12:25 ` Fabian Grünbichler
0 siblings, 0 replies; 27+ messages in thread
From: Fabian Grünbichler @ 2025-02-21 12:25 UTC (permalink / raw)
To: Markus Frank, pmg-devel
> Markus Frank <m.frank@proxmox.com> hat am 18.02.2025 17:18 CET geschrieben:
>
>
> Change pve-rs functions to be wrapper functions for common.
>
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
modulo the name change, which I am not sure is a real benefit unless we want to do it across the board (also for PVE/PMG/the low level rust crates), this looks okay to me.
note that there are two other OIDC related patch series open that might or might not require changes depending on the order things get applied in.
> ---
> common/pkg/Makefile | 1 +
> common/src/mod.rs | 1 +
> common/src/oidc/mod.rs | 63 ++++++++++++++++++++++++++++++++++++++++
> pmg-rs/Cargo.toml | 1 +
> pmg-rs/debian/control | 1 +
> pve-rs/src/openid/mod.rs | 32 +++++---------------
> 6 files changed, 75 insertions(+), 24 deletions(-)
> create mode 100644 common/src/oidc/mod.rs
>
> diff --git a/common/pkg/Makefile b/common/pkg/Makefile
> index cbefdf7..5a537f9 100644
> --- a/common/pkg/Makefile
> +++ b/common/pkg/Makefile
> @@ -24,6 +24,7 @@ PERLMOD_PACKAGES := \
> Proxmox::RS::APT::Repositories \
> Proxmox::RS::CalendarEvent \
> Proxmox::RS::Notify \
> + Proxmox::RS::OIDC \
> Proxmox::RS::SharedCache \
> Proxmox::RS::Subscription
>
> diff --git a/common/src/mod.rs b/common/src/mod.rs
> index badfc98..06a7c20 100644
> --- a/common/src/mod.rs
> +++ b/common/src/mod.rs
> @@ -2,5 +2,6 @@ pub mod apt;
> mod calendar_event;
> pub mod logger;
> pub mod notify;
> +pub mod oidc;
> pub mod shared_cache;
> mod subscription;
> diff --git a/common/src/oidc/mod.rs b/common/src/oidc/mod.rs
> new file mode 100644
> index 0000000..b1cddaa
> --- /dev/null
> +++ b/common/src/oidc/mod.rs
> @@ -0,0 +1,63 @@
> +#[perlmod::package(name = "Proxmox::RS::OIDC")]
> +pub mod export {
> + use std::sync::Mutex;
> +
> + use anyhow::Error;
> +
> + use perlmod::{to_value, Value};
> +
> + use proxmox_openid::{OpenIdAuthenticator, OpenIdConfig, PrivateAuthState};
> +
> + perlmod::declare_magic!(Box<OIDC> : &OIDC as "Proxmox::RS::OIDC");
> +
> + /// An OpenIdAuthenticator client instance.
> + pub struct OIDC {
> + inner: Mutex<OpenIdAuthenticator>,
> + }
> +
> + /// Create a new OIDC client instance
> + #[export(raw_return)]
> + pub fn discover(
> + #[raw] class: Value,
> + config: OpenIdConfig,
> + redirect_url: &str,
> + ) -> Result<Value, Error> {
> + let oidc = OpenIdAuthenticator::discover(&config, redirect_url)?;
> + Ok(perlmod::instantiate_magic!(
> + &class,
> + MAGIC => Box::new(OIDC {
> + inner: Mutex::new(oidc),
> + })
> + ))
> + }
> +
> + #[export]
> + pub fn authorize_url(
> + #[try_from_ref] this: &OIDC,
> + state_dir: &str,
> + realm: &str,
> + ) -> Result<String, Error> {
> + let oidc = this.inner.lock().unwrap();
> + oidc.authorize_url(state_dir, realm)
> + }
> +
> + #[export]
> + pub fn verify_public_auth_state(
> + state_dir: &str,
> + state: &str,
> + ) -> Result<(String, PrivateAuthState), Error> {
> + OpenIdAuthenticator::verify_public_auth_state(state_dir, state)
> + }
> +
> + #[export(raw_return)]
> + pub fn verify_authorization_code(
> + #[try_from_ref] this: &OIDC,
> + code: &str,
> + private_auth_state: PrivateAuthState,
> + ) -> Result<Value, Error> {
> + let oidc = this.inner.lock().unwrap();
> + let claims = oidc.verify_authorization_code_simple(code, &private_auth_state)?;
> +
> + Ok(to_value(&claims)?)
> + }
> +}
> diff --git a/pmg-rs/Cargo.toml b/pmg-rs/Cargo.toml
> index 1252671..ce715bf 100644
> --- a/pmg-rs/Cargo.toml
> +++ b/pmg-rs/Cargo.toml
> @@ -42,3 +42,4 @@ proxmox-subscription = "0.5"
> proxmox-sys = "0.6"
> proxmox-tfa = { version = "5", features = ["api"] }
> proxmox-time = "2"
> +proxmox-openid = "0.10.0"
> diff --git a/pmg-rs/debian/control b/pmg-rs/debian/control
> index 295dcb3..afd8cbb 100644
> --- a/pmg-rs/debian/control
> +++ b/pmg-rs/debian/control
> @@ -27,6 +27,7 @@ Build-Depends: cargo:native <!nocheck>,
> librust-proxmox-http-error-0.1+default-dev,
> librust-proxmox-log-0.2+default-dev,
> librust-proxmox-notify-0.5+default-dev,
> + librust-proxmox-openid-0.10+default-dev,
> librust-proxmox-shared-cache-0.1+default-dev,
> librust-proxmox-subscription-0.5+default-dev,
> librust-proxmox-sys-0.6+default-dev,
> diff --git a/pve-rs/src/openid/mod.rs b/pve-rs/src/openid/mod.rs
> index 1fa7572..2adb8bb 100644
> --- a/pve-rs/src/openid/mod.rs
> +++ b/pve-rs/src/openid/mod.rs
> @@ -1,19 +1,13 @@
> #[perlmod::package(name = "PVE::RS::OpenId", lib = "pve_rs")]
> mod export {
> - use std::sync::Mutex;
> -
> use anyhow::Error;
>
> - use perlmod::{to_value, Value};
> -
> - use proxmox_openid::{OpenIdAuthenticator, OpenIdConfig, PrivateAuthState};
> + use perlmod::Value;
>
> - perlmod::declare_magic!(Box<OpenId> : &OpenId as "PVE::RS::OpenId");
> + use proxmox_openid::{OpenIdConfig, PrivateAuthState};
>
> - /// An OpenIdAuthenticator client instance.
> - pub struct OpenId {
> - inner: Mutex<OpenIdAuthenticator>,
> - }
> + use crate::common::oidc::export as common;
> + use crate::common::oidc::export::OIDC as OpenId;
>
> /// Create a new OpenId client instance
> #[export(raw_return)]
> @@ -22,13 +16,7 @@ mod export {
> config: OpenIdConfig,
> redirect_url: &str,
> ) -> Result<Value, Error> {
> - let open_id = OpenIdAuthenticator::discover(&config, redirect_url)?;
> - Ok(perlmod::instantiate_magic!(
> - &class,
> - MAGIC => Box::new(OpenId {
> - inner: Mutex::new(open_id),
> - })
> - ))
> + common::discover(class, config, redirect_url)
> }
>
> #[export]
> @@ -37,8 +25,7 @@ mod export {
> state_dir: &str,
> realm: &str,
> ) -> Result<String, Error> {
> - let open_id = this.inner.lock().unwrap();
> - open_id.authorize_url(state_dir, realm)
> + common::authorize_url(this, state_dir, realm)
> }
>
> #[export]
> @@ -46,7 +33,7 @@ mod export {
> state_dir: &str,
> state: &str,
> ) -> Result<(String, PrivateAuthState), Error> {
> - OpenIdAuthenticator::verify_public_auth_state(state_dir, state)
> + common::verify_public_auth_state(state_dir, state)
> }
>
> #[export(raw_return)]
> @@ -55,9 +42,6 @@ mod export {
> code: &str,
> private_auth_state: PrivateAuthState,
> ) -> Result<Value, Error> {
> - let open_id = this.inner.lock().unwrap();
> - let claims = open_id.verify_authorization_code_simple(code, &private_auth_state)?;
> -
> - Ok(to_value(&claims)?)
> + common::verify_authorization_code(this, code, private_auth_state)
> }
> }
> --
> 2.39.5
>
>
>
> _______________________________________________
> pmg-devel mailing list
> pmg-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [pmg-devel] [PATCH pmg-api v5 4/10] config: add plugin system for realms
2025-02-18 16:18 ` [pmg-devel] [PATCH pmg-api v5 4/10] config: add plugin system for realms Markus Frank
@ 2025-02-21 12:35 ` Fabian Grünbichler
0 siblings, 0 replies; 27+ messages in thread
From: Fabian Grünbichler @ 2025-02-21 12:35 UTC (permalink / raw)
To: Markus Frank, pmg-devel
> Markus Frank <m.frank@proxmox.com> hat am 18.02.2025 17:18 CET geschrieben:
>
>
> To differentiate between usernames, the realm is also stored in the
> user.conf file. Old config file syntax can be read, but will be
> overwritten after a change.
>
> This is a carryover from PVE. Previously there was no realm for a
> username. Now the realm is also stored after an @ sign in user.conf.
>
> Utils generates a list of valid realm names, including any newly added
> realms, to ensure proper validation of a specified realm name.
>
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> ---
> src/Makefile | 3 +
> src/PMG/AccessControl.pm | 31 ++++++
> src/PMG/Auth/PAM.pm | 22 ++++
> src/PMG/Auth/PMG.pm | 39 ++++++++
> src/PMG/Auth/Plugin.pm | 199 +++++++++++++++++++++++++++++++++++++
> src/PMG/RESTEnvironment.pm | 14 +++
> src/PMG/UserConfig.pm | 25 +++--
> src/PMG/Utils.pm | 29 ++++--
> 8 files changed, 346 insertions(+), 16 deletions(-)
> create mode 100755 src/PMG/Auth/PAM.pm
> create mode 100755 src/PMG/Auth/PMG.pm
> create mode 100755 src/PMG/Auth/Plugin.pm
>
> diff --git a/src/Makefile b/src/Makefile
> index 1232880..659a666 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -166,6 +166,9 @@ LIBSOURCES = \
> PMG/API2/ACMEPlugin.pm \
> PMG/API2/NodeConfig.pm \
> PMG/API2.pm \
> + PMG/Auth/Plugin.pm \
> + PMG/Auth/PAM.pm \
> + PMG/Auth/PMG.pm \
>
> SOURCES = ${LIBSOURCES} ${CLI_BINARIES} ${TEMPLATES_FILES} ${CONF_MANS} ${CLI_MANS} ${SERVICE_MANS} ${SERVICE_UNITS} ${TIMER_UNITS} pmg-sources.list pmg-initramfs.conf
>
> diff --git a/src/PMG/AccessControl.pm b/src/PMG/AccessControl.pm
> index e12d7cf..ced5f9a 100644
> --- a/src/PMG/AccessControl.pm
> +++ b/src/PMG/AccessControl.pm
> @@ -13,6 +13,14 @@ use PMG::LDAPConfig;
> use PMG::LDAPSet;
> use PMG::TFAConfig;
>
> +use PMG::Auth::Plugin;
> +use PMG::Auth::PAM;
> +use PMG::Auth::PMG;
> +
> +PMG::Auth::PAM->register();
> +PMG::Auth::PMG->register();
> +PMG::Auth::Plugin->init();
> +
> sub normalize_path {
> my $path = shift;
>
> @@ -38,6 +46,7 @@ sub authenticate_user : prototype($$$) {
>
> ($username, $ruid, $realm) = PMG::Utils::verify_username($username);
>
> + my $realm_regex = PMG::Utils::valid_pmg_realm_regex();
> if ($realm eq 'pam') {
> die "invalid pam user (only root allowed)\n" if $ruid ne 'root';
> authenticate_pam_user($ruid, $password);
> @@ -53,6 +62,8 @@ sub authenticate_user : prototype($$$) {
> return ($pmail . '@quarantine', undef);
> }
> die "ldap login failed\n";
> + } elsif ($realm =~ m!(${realm_regex})!) {
> + # nothing to do for self-defined realms
this is really dangerous and this whole code should be restructured like it is in PVE! i.e., the authentication should happen in the plugin as plugin method, and OIDC should die because we should never end up there!
> } else {
> die "no such realm '$realm'\n";
> }
> @@ -79,6 +90,7 @@ sub set_user_password {
>
> ($username, $ruid, $realm) = PMG::Utils::verify_username($username);
>
> + my $realm_regex = PMG::Utils::valid_pmg_realm_regex();
> if ($realm eq 'pam') {
> die "invalid pam user (only root allowed)\n" if $ruid ne 'root';
>
> @@ -92,6 +104,8 @@ sub set_user_password {
>
> } elsif ($realm eq 'pmg') {
> PMG::UserConfig->set_user_password($username, $password);
> + } elsif ($realm =~ m!(${realm_regex})!) {
> + # nothing to do for self-defined realms
same here
> } else {
> die "no such realm '$realm'\n";
> }
> @@ -106,6 +120,7 @@ sub check_user_enabled {
>
> ($username, $ruid, $realm) = PMG::Utils::verify_username($username, 1);
>
> + my $realm_regex = PMG::Utils::valid_pmg_realm_regex();
> if ($realm && $ruid) {
> if ($realm eq 'pam') {
> return 'root' if $ruid eq 'root';
> @@ -115,6 +130,10 @@ sub check_user_enabled {
> return $data->{role} if $data && $data->{enable};
> } elsif ($realm eq 'quarantine') {
> return 'quser';
> + } elsif ($realm =~ m!(${realm_regex})!) {
> + my $usercfg = PMG::UserConfig->new();
> + my $data = $usercfg->lookup_user_data($username, $noerr);
> + return $data->{role} if $data && $data->{enable};
> }
this is a bit pre-existing, but.. this whole method is weird and should be refactored to
- first check if the user is enabled in the config
- then call the plugin to get its role
> }
>
> @@ -123,6 +142,18 @@ sub check_user_enabled {
> return undef;
> }
>
> +sub check_user_exist {
what is this for? it is defined here, and re-exported in PMG::RESTEnvironment, but not used anywhere?
> + my ($usercfg, $username, $noerr) = @_;
> +
> + $username = PMG::Utils::verify_username($username, $noerr);
> + return undef if !$username;
> + return $usercfg->{$username} if $usercfg && $usercfg->{$username};
> +
> + die "no such user ('$username')\n" if !$noerr;
> +
> + return undef;
> +}
> +
> sub authenticate_pam_user {
> my ($username, $password) = @_;
>
> diff --git a/src/PMG/Auth/PAM.pm b/src/PMG/Auth/PAM.pm
> new file mode 100755
> index 0000000..3ffd8a6
> --- /dev/null
> +++ b/src/PMG/Auth/PAM.pm
> @@ -0,0 +1,22 @@
> +package PMG::Auth::PAM;
> +
> +use strict;
> +use warnings;
> +
> +use PMG::Auth::Plugin;
> +
> +use base qw(PMG::Auth::Plugin);
> +
> +sub type {
> + return 'pam';
> +}
> +
> +sub options {
> + return {
> + default => { optional => 1 },
> + comment => { optional => 1 },
> + tfa => { optional => 1 },
> + };
> +}
> +
> +1;
> diff --git a/src/PMG/Auth/PMG.pm b/src/PMG/Auth/PMG.pm
> new file mode 100755
> index 0000000..b083692
> --- /dev/null
> +++ b/src/PMG/Auth/PMG.pm
> @@ -0,0 +1,39 @@
> +package PMG::Auth::PMG;
> +
> +use strict;
> +use warnings;
> +
> +use PMG::Auth::Plugin;
> +
> +use base qw(PMG::Auth::Plugin);
> +
> +sub type {
> + return 'pmg';
> +}
> +
> +sub properties {
> + return {
> + default => {
> + description => "Use this as default realm",
> + type => 'boolean',
> + optional => 1,
> + },
> + comment => {
> + description => "Description.",
> + type => 'string',
> + optional => 1,
> + maxLength => 4096,
> + },
> + tfa => PVE::JSONSchema::get_standard_option('tfa'),
> + };
> +}
> +
> +sub options {
> + return {
> + default => { optional => 1 },
> + comment => { optional => 1 },
> + tfa => { optional => 1 },
> + };
> +}
> +
> +1;
> diff --git a/src/PMG/Auth/Plugin.pm b/src/PMG/Auth/Plugin.pm
> new file mode 100755
> index 0000000..b336d0c
> --- /dev/null
> +++ b/src/PMG/Auth/Plugin.pm
> @@ -0,0 +1,199 @@
> +package PMG::Auth::Plugin;
> +
> +use strict;
> +use warnings;
> +
> +use Digest::SHA;
> +use Encode;
> +
> +use PMG::Utils;
> +use PVE::INotify;
> +use PVE::JSONSchema qw(get_standard_option);
> +use PVE::Schema::Auth;
> +use PVE::SectionConfig;
> +use PVE::Tools;
> +
> +use base qw(PVE::SectionConfig);
> +
> +my $domainconfigfile = "realms.cfg";
> +my $lockfile = "/var/lock/pmg-realms.lck";
> +
> +sub read_realms_conf {
> + my ($filename, $fh) = @_;
> +
> + my $raw;
> + $raw = do { local $/ = undef; <$fh> } if defined($fh);
> +
> + return PMG::Auth::Plugin->parse_config($filename, $raw);
> +}
> +
> +sub write_realms_conf {
> + my ($filename, $fh, $cfg) = @_;
> +
> + my $raw = PMG::Auth::Plugin->write_config($filename, $cfg);
> +
> + PVE::Tools::safe_print($filename, $fh, $raw);
> +}
> +
> +PVE::INotify::register_file(
> + $domainconfigfile,
> + "/etc/pmg/realms.cfg",
> + \&read_realms_conf,
> + \&write_realms_conf,
> + undef,
> + always_call_parser => 1,
> +);
> +
> +sub lock_domain_config {
> + my ($code, $errmsg) = @_;
> +
> + PVE::Tools::lock_file($lockfile, undef, $code);
> + if (my $err = $@) {
> + $errmsg ? die "$errmsg: $err" : die $err;
> + }
> +}
> +
> +my $realm_regex = qr/[A-Za-z][A-Za-z0-9\.\-_]+/;
> +
> +sub pmg_verify_realm {
> + my ($realm, $noerr) = @_;
> +
> + if ($realm !~ m/^${realm_regex}$/) {
> + return undef if $noerr;
> + die "value does not look like a valid realm\n";
> + }
> + return $realm;
> +}
> +
> +my $defaultData = {
> + propertyList => {
> + type => { description => "Realm type." },
> + realm => get_standard_option('realm'),
> + },
> +};
> +
> +sub private {
> + return $defaultData;
> +}
> +
> +sub parse_section_header {
> + my ($class, $line) = @_;
> +
> + if ($line =~ m/^(\S+):\s*(\S+)\s*$/) {
> + my ($type, $realm) = (lc($1), $2);
> + my $errmsg = undef; # set if you want to skip whole section
> + eval { pmg_verify_realm($realm); };
> + $errmsg = $@ if $@;
> + my $config = {}; # to return additional attributes
> + return ($type, $realm, $errmsg, $config);
> + }
> + return undef;
> +}
> +
> +sub parse_config {
> + my ($class, $filename, $raw) = @_;
> +
> + my $cfg = $class->SUPER::parse_config($filename, $raw);
> +
> + my $default;
> + foreach my $realm (keys %{$cfg->{ids}}) {
> + my $data = $cfg->{ids}->{$realm};
> + # make sure there is only one default marker
> + if ($data->{default}) {
> + if ($default) {
> + delete $data->{default};
> + } else {
> + $default = $realm;
> + }
> + }
> +
> + if ($data->{comment}) {
> + $data->{comment} = PVE::Tools::decode_text($data->{comment});
> + }
> +
> + }
> +
> + # add default domains
> + $cfg->{ids}->{pmg}->{type} = 'pmg'; # force type
> + $cfg->{ids}->{pmg}->{comment} = "Proxmox Mail Gateway authentication server"
> + if !$cfg->{ids}->{pmg}->{comment};
> + $cfg->{ids}->{pmg}->{default} = 1
> + if !$cfg->{ids}->{pmg}->{default};
> +
> + $cfg->{ids}->{pam}->{type} = 'pam'; # force type
> + $cfg->{ids}->{pam}->{comment} = "Linux PAM standard authentication"
> + if !$cfg->{ids}->{pam}->{comment};
> +
> + return $cfg;
> +};
> +
> +sub write_config {
> + my ($class, $filename, $cfg) = @_;
> +
> + foreach my $realm (keys %{$cfg->{ids}}) {
> + my $data = $cfg->{ids}->{$realm};
> + if ($data->{comment}) {
> + $data->{comment} = PVE::Tools::encode_text($data->{comment});
> + }
> + }
> +
> + $class->SUPER::write_config($filename, $cfg);
> +}
> +
> +sub authenticate_user {
> + my ($class, $config, $realm, $username, $password) = @_;
> +
> + die "overwrite me";
> +}
> +
> +sub store_password {
> + my ($class, $config, $realm, $username, $password) = @_;
> +
> + my $type = $class->type();
> +
> + die "can't set password on auth type '$type'\n";
> +}
> +
> +sub delete_user {
> + my ($class, $config, $realm, $username) = @_;
> +
> + # do nothing by default
> +}
> +
> +# called during addition of realm (before the new domain config got written)
> +# `password` is moved to %param to avoid writing it out to the config
> +# die to abort addition if there are (grave) problems
> +# NOTE: runs in a domain config *locked* context
> +sub on_add_hook {
> + my ($class, $realm, $config, %param) = @_;
> + # do nothing by default
> +}
> +
> +# called during domain configuration update (before the updated domain config got
> +# written). `password` is moved to %param to avoid writing it out to the config
> +# die to abort the update if there are (grave) problems
> +# NOTE: runs in a domain config *locked* context
> +sub on_update_hook {
> + my ($class, $realm, $config, %param) = @_;
> + # do nothing by default
> +}
> +
> +# called during deletion of realms (before the new domain config got written)
> +# and if the activate check on addition fails, to cleanup all storage traces
> +# which on_add_hook may have created.
> +# die to abort deletion if there are (very grave) problems
> +# NOTE: runs in a domain config *locked* context
> +sub on_delete_hook {
> + my ($class, $realm, $config) = @_;
> + # do nothing by default
> +}
> +
> +# called during addition and updates of realms (before the new domain config gets written)
> +# die to abort addition/update in case the connection/bind fails
> +# NOTE: runs in a domain config *locked* context
> +sub check_connection {
> + my ($class, $realm, $config, %param) = @_;
> + # do nothing by default
> +}
> +
> +1;
> diff --git a/src/PMG/RESTEnvironment.pm b/src/PMG/RESTEnvironment.pm
> index 3875720..f6ff449 100644
> --- a/src/PMG/RESTEnvironment.pm
> +++ b/src/PMG/RESTEnvironment.pm
> @@ -88,6 +88,20 @@ sub get_role {
> return $self->{role};
> }
>
> +sub check_user_enabled {
> + my ($self, $user, $noerr) = @_;
> +
> + my $cfg = $self->{usercfg};
> + return PMG::AccessControl::check_user_enabled($cfg, $user, $noerr);
> +}
> +
> +sub check_user_exist {
> + my ($self, $user, $noerr) = @_;
> +
> + my $cfg = $self->{usercfg};
> + return PMG::AccessControl::check_user_exist($cfg, $user, $noerr);
see above
> +}
> +
> sub check_node_is_master {
> my ($self, $noerr);
>
> diff --git a/src/PMG/UserConfig.pm b/src/PMG/UserConfig.pm
> index b9a83a7..fe6d2c8 100644
> --- a/src/PMG/UserConfig.pm
> +++ b/src/PMG/UserConfig.pm
> @@ -80,7 +80,7 @@ my $schema = {
> realm => {
> description => "Authentication realm.",
> type => 'string',
> - enum => ['pam', 'pmg'],
> + format => 'pmg-realm',
> default => 'pmg',
> optional => 1,
> },
> @@ -219,10 +219,13 @@ sub read_user_conf {
> (?<keys>(?:[^:]*)) :
> $/x
> ) {
> + my @username_parts = split('@', $+{userid});
this is broken if you add a user with '@' in the username part (which is allowed I think?)
> + my $username = $username_parts[0];
> + my $realm = defined($username_parts[1]) ? $username_parts[1] : "pmg";
> my $d = {
> - username => $+{userid},
> - userid => $+{userid} . '@pmg',
> - realm => 'pmg',
> + username => $username,
> + userid => $username . '@' . $realm,
> + realm => $realm,
> enable => $+{enable} || 0,
> expire => $+{expire} || 0,
> role => $+{role},
> @@ -235,8 +238,9 @@ sub read_user_conf {
> eval {
> $verify_entry->($d);
> $cfg->{$d->{userid}} = $d;
> - die "role 'root' is reserved\n"
> - if $d->{role} eq 'root' && $d->{userid} ne 'root@pmg';
> + if ($d->{role} eq 'root' && $d->{userid} !~ /^root@(pmg|pam)$/) {
> + die "role 'root' is reserved\n";
> + }
> };
> if (my $err = $@) {
> warn "$filename: $err";
> @@ -274,22 +278,23 @@ sub write_user_conf {
> $verify_entry->($d);
> $cfg->{$d->{userid}} = $d;
>
> + my $realm_regex = PMG::Utils::valid_pmg_realm_regex();
> if ($d->{userid} ne 'root@pam') {
> die "role 'root' is reserved\n" if $d->{role} eq 'root';
> die "unable to add users for realm '$d->{realm}'\n"
> - if $d->{realm} && $d->{realm} ne 'pmg';
> + if $d->{realm} && $d->{realm} !~ m!(${realm_regex})!;
> }
>
> my $line;
>
> if ($userid eq 'root@pam') {
> - $line = 'root:';
> + $line = 'root@pam:';
> $d->{crypt_pass} = '',
> $d->{expire} = '0',
> $d->{role} = 'root';
> } else {
> - next if $userid !~ m/^(?<username>.+)\@pmg$/;
> - $line = "$+{username}:";
> + next if $userid !~ m/^(?<username>.+)\@(${realm_regex})$/;
> + $line = "$d->{userid}:";
> }
>
> for my $k (qw(enable expire crypt_pass role email firstname lastname keys)) {
> diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
> index 0b8945f..04a3a2e 100644
> --- a/src/PMG/Utils.pm
> +++ b/src/PMG/Utils.pm
> @@ -49,12 +49,30 @@ postgres_admin_cmd
> try_decode_utf8
> );
>
> -my $valid_pmg_realms = ['pam', 'pmg', 'quarantine'];
> +my $user_regex = qr![^\s:/]+!;
> +
> +sub valid_pmg_realm_regex {
> + my $cfg = PVE::INotify::read_file('realms.cfg');
> + my $ids = $cfg->{ids};
> + my $realms = ['pam', 'quarantine', sort keys $cfg->{ids}->%* ];
> + return join('|', @$realms);
> +}
> +
> +sub is_valid_realm {
> + my ($realm) = @_;
> + return 0 if !$realm;
> + return 1 if $realm eq 'pam' || $realm eq 'quarantine'; # built-in ones
> +
> + my $cfg = PVE::INotify::read_file('realms.cfg');
> + return exists($cfg->{ids}->{$realm}) ? 1 : 0;
> +}
> +
> +PVE::JSONSchema::register_format('pmg-realm', \&is_valid_realm);
>
> PVE::JSONSchema::register_standard_option('realm', {
> description => "Authentication domain ID",
> type => 'string',
> - enum => $valid_pmg_realms,
> + format => 'pmg-realm',
> maxLength => 32,
> });
>
> @@ -82,16 +100,15 @@ sub verify_username {
> die "user name '$username' is too short\n" if !$noerr;
> return undef;
> }
> - if ($len > 64) {
> - die "user name '$username' is too long ($len > 64)\n" if !$noerr;
> + if ($len > 128) {
> + die "user name '$username' is too long ($len > 128)\n" if !$noerr;
> return undef;
> }
>
> # we only allow a limited set of characters. Colons aren't allowed, because we store usernames
> # with colon separated lists! slashes aren't allowed because it is used as pve API delimiter
> # also see "man useradd"
> - my $realm_list = join('|', @$valid_pmg_realms);
> - if ($username =~ m!^([^\s:/]+)\@(${realm_list})$!) {
> + if ($username =~ m!^(${user_regex})\@([A-Za-z][A-Za-z0-9\.\-_]+)$!) {
> return wantarray ? ($username, $1, $2) : $username;
> }
>
> --
> 2.39.5
>
>
>
> _______________________________________________
> pmg-devel mailing list
> pmg-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [pmg-devel] [PATCH pmg-api v5 5/10] config: add oidc type realm
2025-02-18 16:19 ` [pmg-devel] [PATCH pmg-api v5 5/10] config: add oidc type realm Markus Frank
@ 2025-02-21 12:38 ` Fabian Grünbichler
0 siblings, 0 replies; 27+ messages in thread
From: Fabian Grünbichler @ 2025-02-21 12:38 UTC (permalink / raw)
To: Markus Frank, pmg-devel
> Markus Frank <m.frank@proxmox.com> hat am 18.02.2025 17:19 CET geschrieben:
>
>
> If the autocreate option is enabled, the user is automatically created
> with the Audit role. To change the role for automatically created users,
> set the autocreate-role option to the preferred role.
>
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> ---
> src/Makefile | 1 +
> src/PMG/AccessControl.pm | 2 +
> src/PMG/Auth/OIDC.pm | 95 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 98 insertions(+)
> create mode 100755 src/PMG/Auth/OIDC.pm
>
> diff --git a/src/Makefile b/src/Makefile
> index 659a666..3cae7c7 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -169,6 +169,7 @@ LIBSOURCES = \
> PMG/Auth/Plugin.pm \
> PMG/Auth/PAM.pm \
> PMG/Auth/PMG.pm \
> + PMG/Auth/OIDC.pm \
>
> SOURCES = ${LIBSOURCES} ${CLI_BINARIES} ${TEMPLATES_FILES} ${CONF_MANS} ${CLI_MANS} ${SERVICE_MANS} ${SERVICE_UNITS} ${TIMER_UNITS} pmg-sources.list pmg-initramfs.conf
>
> diff --git a/src/PMG/AccessControl.pm b/src/PMG/AccessControl.pm
> index ced5f9a..75ef486 100644
> --- a/src/PMG/AccessControl.pm
> +++ b/src/PMG/AccessControl.pm
> @@ -14,9 +14,11 @@ use PMG::LDAPSet;
> use PMG::TFAConfig;
>
> use PMG::Auth::Plugin;
> +use PMG::Auth::OIDC;
> use PMG::Auth::PAM;
> use PMG::Auth::PMG;
>
> +PMG::Auth::OIDC->register();
> PMG::Auth::PAM->register();
> PMG::Auth::PMG->register();
> PMG::Auth::Plugin->init();
> diff --git a/src/PMG/Auth/OIDC.pm b/src/PMG/Auth/OIDC.pm
> new file mode 100755
> index 0000000..0acde7d
> --- /dev/null
> +++ b/src/PMG/Auth/OIDC.pm
> @@ -0,0 +1,95 @@
> +package PMG::Auth::OIDC;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::Tools;
> +use PMG::Auth::Plugin;
> +
> +use base qw(PMG::Auth::Plugin);
> +
> +sub type {
> + return 'oidc';
> +}
> +
> +sub properties {
> + return {
> + 'issuer-url' => {
> + description => "OpenID Connect Issuer Url",
> + type => 'string',
> + maxLength => 256,
copied from PVE, I know - but can we please have proper formats and not add *more* unrestricted string types? we should rather start too restrictive and then lift restrictions based on real world use cases, the opposite is hard to impossible..
> + },
> + 'client-id' => {
> + description => "OpenID Connect Client ID",
> + type => 'string',
> + maxLength => 256,
> + },
> + 'client-key' => {
> + description => "OpenID Connect Client Key",
> + type => 'string',
> + optional => 1,
> + maxLength => 256,
> + },
> + autocreate => {
> + description => "Automatically create users if they do not exist.",
> + optional => 1,
> + type => 'boolean',
> + default => 0,
> + },
> + 'autocreate-role' => {
> + description => "Automatically create users with a specific role.",
> + type => 'string',
> + enum => ['admin', 'qmanager', 'audit', 'helpdesk'],
> + optional => 1,
> + },
> + 'username-claim' => {
> + description => "OpenID Connect claim used to generate the unique username.",
> + type => 'string',
> + optional => 1,
> + },
> + prompt => {
> + description => "Specifies whether the Authorization Server prompts the End-User for"
> + ." reauthentication and consent.",
> + type => 'string',
> + pattern => '(?:none|login|consent|select_account|\S+)', # \S+ is the extension variant
> + optional => 1,
> + },
> + scopes => {
> + description => "Specifies the scopes (user details) that should be authorized and"
> + ." returned, for example 'email' or 'profile'.",
> + type => 'string', # format => 'some-safe-id-list', # FIXME: TODO
> + default => "email profile",
> + optional => 1,
> + },
> + 'acr-values' => {
> + description => "Specifies the Authentication Context Class Reference values that the"
> + ."Authorization Server is being requested to use for the Auth Request.",
> + type => 'string', # format => 'some-safe-id-list', # FIXME: TODO
> + optional => 1,
> + },
> + };
> +}
> +
> +sub options {
> + return {
> + 'issuer-url' => {},
> + 'client-id' => {},
> + 'client-key' => { optional => 1 },
> + autocreate => { optional => 1 },
> + 'autocreate-role' => { optional => 1 },
> + 'username-claim' => { optional => 1, fixed => 1 },
> + prompt => { optional => 1 },
> + scopes => { optional => 1 },
> + 'acr-values' => { optional => 1 },
> + default => { optional => 1 },
> + comment => { optional => 1 },
> + };
> +}
> +
> +sub authenticate_user {
> + my ($class, $config, $realm, $username, $password) = @_;
> +
> + die "OpenID Connect realm does not allow password verification.\n";
> +}
this is dead code (but it should be called, see my other comments ;))
> +
> +1;
> --
> 2.39.5
>
>
>
> _______________________________________________
> pmg-devel mailing list
> pmg-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [pmg-devel] [PATCH pmg-api v5 6/10] api: add/update/remove realms like in PVE
2025-02-18 16:19 ` [pmg-devel] [PATCH pmg-api v5 6/10] api: add/update/remove realms like in PVE Markus Frank
@ 2025-02-21 12:41 ` Fabian Grünbichler
2025-02-21 13:44 ` Markus Frank
0 siblings, 1 reply; 27+ messages in thread
From: Fabian Grünbichler @ 2025-02-21 12:41 UTC (permalink / raw)
To: Markus Frank, pmg-devel
> Markus Frank <m.frank@proxmox.com> hat am 18.02.2025 17:19 CET geschrieben:
>
>
> The name Realm.pm was chosen because a Domain.pm already exists.
but the API path is still domains, and the naming inside the code/descriptions/.. is also rather inconsistent. should we settle on one or the other?
>
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> ---
> src/Makefile | 1 +
> src/PMG/API2/AccessControl.pm | 10 +-
> src/PMG/API2/Realm.pm | 274 ++++++++++++++++++++++++++++++++++
> 3 files changed, 284 insertions(+), 1 deletion(-)
> create mode 100644 src/PMG/API2/Realm.pm
>
> diff --git a/src/Makefile b/src/Makefile
> index 3cae7c7..e939bbd 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -151,6 +151,7 @@ LIBSOURCES = \
> PMG/API2/Postfix.pm \
> PMG/API2/Quarantine.pm \
> PMG/API2/AccessControl.pm \
> + PMG/API2/Realm.pm \
> PMG/API2/TFA.pm \
> PMG/API2/TFAConfig.pm \
> PMG/API2/ObjectGroupHelpers.pm \
> diff --git a/src/PMG/API2/AccessControl.pm b/src/PMG/API2/AccessControl.pm
> index e26ae70..f4cbc81 100644
> --- a/src/PMG/API2/AccessControl.pm
> +++ b/src/PMG/API2/AccessControl.pm
> @@ -12,6 +12,7 @@ use PVE::JSONSchema qw(get_standard_option);
> use PMG::Utils;
> use PMG::UserConfig;
> use PMG::AccessControl;
> +use PMG::API2::Realm;
> use PMG::API2::Users;
> use PMG::API2::TFA;
> use PMG::TFAConfig;
> @@ -30,6 +31,11 @@ __PACKAGE__->register_method ({
> path => 'tfa',
> });
>
> +__PACKAGE__->register_method ({
> + subclass => "PMG::API2::Realm",
> + path => 'domains',
realm vs domains
> +});
> +
> __PACKAGE__->register_method ({
> name => 'index',
> path => '',
> @@ -57,6 +63,7 @@ __PACKAGE__->register_method ({
>
> my $res = [
> { subdir => 'ticket' },
> + { subdir => 'domains' },
> { subdir => 'password' },
> { subdir => 'users' },
> ];
> @@ -248,7 +255,8 @@ __PACKAGE__->register_method ({
>
> my $username = $param->{username};
>
> - if ($username !~ m/\@(pam|pmg|quarantine)$/) {
> + my $realm_regex = PMG::Utils::valid_pmg_realm_regex();
> + if ($username !~ m/\@(${realm_regex})$/) {
> my $realm = $param->{realm} // 'quarantine';
> $username .= "\@$realm";
> }
> diff --git a/src/PMG/API2/Realm.pm b/src/PMG/API2/Realm.pm
> new file mode 100644
> index 0000000..da2072a
> --- /dev/null
> +++ b/src/PMG/API2/Realm.pm
> @@ -0,0 +1,274 @@
> +package PMG::API2::Realm;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::Exception qw(raise_param_exc);
> +use PVE::INotify;
> +use PVE::JSONSchema qw(get_standard_option);
> +use PVE::RESTHandler;
> +use PVE::SafeSyslog;
> +use PVE::Tools qw(extract_param);
> +
> +use PMG::AccessControl;
> +use PMG::Auth::Plugin;
> +use PVE::Schema::Auth;
> +
> +my $domainconfigfile = "realms.cfg";
> +
> +use base qw(PVE::RESTHandler);
> +
> +__PACKAGE__->register_method ({
> + name => 'index',
> + path => '',
> + method => 'GET',
> + description => "Authentication domain index.",
and here it is still an authentication domain ;)
> + permissions => {
> + description => "Anyone can access that, because we need that list for the login box (before the user is authenticated).",
> + user => 'world',
> + },
> + parameters => {
> + additionalProperties => 0,
> + properties => {},
> + },
> + returns => {
> + type => 'array',
> + items => {
> + type => "object",
> + properties => {
> + realm => { type => 'string' },
even though it returns realms
> + type => { type => 'string' },
> + tfa => {
> + description => "Two-factor authentication provider.",
> + type => 'string',
> + enum => [ 'yubico', 'oath' ],
> + optional => 1,
> + },
> + comment => {
> + description => "A comment. The GUI use this text when you select a domain (Realm) on the login window.",
> + type => 'string',
> + optional => 1,
> + },
> + },
> + },
> + links => [ { rel => 'child', href => "{realm}" } ],
> + },
> + code => sub {
> + my ($param) = @_;
> +
> + my $res = [];
> +
> + my $cfg = PVE::INotify::read_file($domainconfigfile);
> + my $ids = $cfg->{ids};
> +
> + for my $realm (keys %$ids) {
> + my $d = $ids->{$realm};
> + my $entry = { realm => $realm, type => $d->{type} };
> + $entry->{comment} = $d->{comment} if $d->{comment};
> + $entry->{default} = 1 if $d->{default};
> + if ($d->{tfa} && (my $tfa_cfg = PVE::Schema::Auth::parse_tfa_config($d->{tfa}))) {
> + $entry->{tfa} = $tfa_cfg->{type};
> + }
> + push @$res, $entry;
> + }
> +
> + return $res;
> + }});
> +
> +__PACKAGE__->register_method ({
> + name => 'create',
> + protected => 1,
> + path => '',
> + method => 'POST',
> + permissions => { check => [ 'admin' ] },
> + description => "Add an authentication server.",
> + parameters => PMG::Auth::Plugin->createSchema(0),
> + returns => { type => 'null' },
> + code => sub {
> + my ($param) = @_;
> +
> + # always extract, add it with hook
> + my $password = extract_param($param, 'password');
> +
> + PMG::Auth::Plugin::lock_domain_config(
> + sub {
> + my $cfg = PVE::INotify::read_file($domainconfigfile);
> + my $ids = $cfg->{ids};
> +
> + my $realm = extract_param($param, 'realm');
> + PMG::Auth::Plugin::pmg_verify_realm($realm);
> + my $type = $param->{type};
> + my $check_connection = extract_param($param, 'check-connection');
> +
> + die "domain '$realm' already exists\n"
> + if $ids->{$realm};
> +
> + die "unable to use reserved name '$realm'\n"
> + if ($realm eq 'pam' || $realm eq 'pmg');
> +
> + die "unable to create builtin type '$type'\n"
> + if ($type eq 'pam' || $type eq 'pmg');
> +
> + my $plugin = PMG::Auth::Plugin->lookup($type);
> + my $config = $plugin->check_config($realm, $param, 1, 1);
> +
> + if ($config->{default}) {
> + for my $r (keys %$ids) {
> + delete $ids->{$r}->{default};
> + }
> + }
> +
> + $ids->{$realm} = $config;
> +
> + my $opts = $plugin->options();
> + if (defined($password) && !defined($opts->{password})) {
> + $password = undef;
> + warn "ignoring password parameter";
> + }
> + $plugin->on_add_hook($realm, $config, password => $password);
> +
> + PVE::INotify::write_file($domainconfigfile, $cfg);
> + },
> + "add auth server failed",
> + );
> + return undef;
> + }});
> +
> +__PACKAGE__->register_method ({
> + name => 'update',
> + path => '{realm}',
> + method => 'PUT',
> + permissions => { check => [ 'admin' ] },
> + description => "Update authentication server settings.",
> + protected => 1,
> + parameters => PMG::Auth::Plugin->updateSchema(0),
> + returns => { type => 'null' },
> + code => sub {
> + my ($param) = @_;
> +
> + # always extract, update in hook
> + my $password = extract_param($param, 'password');
> +
> + PMG::Auth::Plugin::lock_domain_config(
> + sub {
> + my $cfg = PVE::INotify::read_file($domainconfigfile);
> + my $ids = $cfg->{ids};
> +
> + my $digest = extract_param($param, 'digest');
> + PVE::SectionConfig::assert_if_modified($cfg, $digest);
> +
> + my $realm = extract_param($param, 'realm');
> + my $type = $ids->{$realm}->{type};
> + my $check_connection = extract_param($param, 'check-connection');
> +
> + die "domain '$realm' does not exist\n"
> + if !$ids->{$realm};
> +
> + my $delete_str = extract_param($param, 'delete');
> + die "no options specified\n"
> + if !$delete_str && !scalar(keys %$param) && !defined($password);
> +
> + my $delete_pw = 0;
> + for my $opt (PVE::Tools::split_list($delete_str)) {
> + delete $ids->{$realm}->{$opt};
> + $delete_pw = 1 if $opt eq 'password';
> + }
> +
> + my $plugin = PMG::Auth::Plugin->lookup($type);
> + my $config = $plugin->check_config($realm, $param, 0, 1);
> +
> + if ($config->{default}) {
> + for my $r (keys %$ids) {
> + delete $ids->{$r}->{default};
> + }
> + }
> +
> + for my $p (keys %$config) {
> + $ids->{$realm}->{$p} = $config->{$p};
> + }
> +
> + my $opts = $plugin->options();
> + if ($delete_pw || defined($password)) {
> + $plugin->on_update_hook($realm, $config, password => $password);
> + } else {
> + $plugin->on_update_hook($realm, $config);
> + }
> +
> + PVE::INotify::write_file($domainconfigfile, $cfg);
> + },
> + "update auth server failed"
> + );
> + return undef;
> + }});
> +
> +# fixme: return format!
> +__PACKAGE__->register_method ({
> + name => 'read',
> + path => '{realm}',
> + method => 'GET',
> + description => "Get auth server configuration.",
> + permissions => { check => [ 'admin', 'qmanager', 'audit' ] },
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + realm => get_standard_option('realm'),
> + },
> + },
> + returns => {},
> + code => sub {
> + my ($param) = @_;
> +
> + my $cfg = PVE::INotify::read_file($domainconfigfile);
> +
> + my $realm = $param->{realm};
> +
> + my $data = $cfg->{ids}->{$realm};
> + die "domain '$realm' does not exist\n" if !$data;
> +
> + my $type = $data->{type};
> +
> + $data->{digest} = $cfg->{digest};
> +
> + return $data;
> + }});
> +
> +
> +__PACKAGE__->register_method ({
> + name => 'delete',
> + path => '{realm}',
> + method => 'DELETE',
> + permissions => { check => [ 'admin' ] },
> + description => "Delete an authentication server.",
> + protected => 1,
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + realm => get_standard_option('realm'),
> + }
> + },
> + returns => { type => 'null' },
> + code => sub {
> + my ($param) = @_;
> +
> + PMG::Auth::Plugin::lock_domain_config(
> + sub {
> + my $cfg = PVE::INotify::read_file($domainconfigfile);
> + my $ids = $cfg->{ids};
> + my $realm = $param->{realm};
> +
> + die "authentication domain '$realm' does not exist\n" if !$ids->{$realm};
> +
> + my $plugin = PMG::Auth::Plugin->lookup($ids->{$realm}->{type});
> +
> + $plugin->on_delete_hook($realm, $ids->{$realm});
> +
> + delete $ids->{$realm};
> +
> + PVE::INotify::write_file($domainconfigfile, $cfg);
> + },
> + "delete auth server failed",
> + );
> + return undef;
> + }});
> +
> +1;
> --
> 2.39.5
>
>
>
> _______________________________________________
> pmg-devel mailing list
> pmg-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [pmg-devel] [PATCH pmg-api v5 7/10] api: oidc login similar to PVE
2025-02-18 16:19 ` [pmg-devel] [PATCH pmg-api v5 7/10] api: oidc login similar to PVE Markus Frank
2025-02-19 18:31 ` Stoiko Ivanov
@ 2025-02-21 12:44 ` Fabian Grünbichler
1 sibling, 0 replies; 27+ messages in thread
From: Fabian Grünbichler @ 2025-02-21 12:44 UTC (permalink / raw)
To: Markus Frank, pmg-devel
> Markus Frank <m.frank@proxmox.com> hat am 18.02.2025 17:19 CET geschrieben:
>
>
> Allow OpenID Connect login using the Rust OIDC module.
>
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> ---
> src/Makefile | 1 +
> src/PMG/API2/AccessControl.pm | 7 +
> src/PMG/API2/OIDC.pm | 243 ++++++++++++++++++++++++++++++++++
> src/PMG/HTTPServer.pm | 2 +
> 4 files changed, 253 insertions(+)
> create mode 100644 src/PMG/API2/OIDC.pm
>
> diff --git a/src/Makefile b/src/Makefile
> index e939bbd..7033a66 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -152,6 +152,7 @@ LIBSOURCES = \
> PMG/API2/Quarantine.pm \
> PMG/API2/AccessControl.pm \
> PMG/API2/Realm.pm \
> + PMG/API2/OIDC.pm \
> PMG/API2/TFA.pm \
> PMG/API2/TFAConfig.pm \
> PMG/API2/ObjectGroupHelpers.pm \
> diff --git a/src/PMG/API2/AccessControl.pm b/src/PMG/API2/AccessControl.pm
> index f4cbc81..1881fac 100644
> --- a/src/PMG/API2/AccessControl.pm
> +++ b/src/PMG/API2/AccessControl.pm
> @@ -13,6 +13,7 @@ use PMG::Utils;
> use PMG::UserConfig;
> use PMG::AccessControl;
> use PMG::API2::Realm;
> +use PMG::API2::OIDC;
> use PMG::API2::Users;
> use PMG::API2::TFA;
> use PMG::TFAConfig;
> @@ -36,6 +37,11 @@ __PACKAGE__->register_method ({
> path => 'domains',
> });
>
> +__PACKAGE__->register_method ({
> + subclass => "PMG::API2::OIDC",
> + path => 'oidc',
> +});
> +
> __PACKAGE__->register_method ({
> name => 'index',
> path => '',
> @@ -64,6 +70,7 @@ __PACKAGE__->register_method ({
> my $res = [
> { subdir => 'ticket' },
> { subdir => 'domains' },
> + { subdir => 'oidc' },
> { subdir => 'password' },
> { subdir => 'users' },
> ];
> diff --git a/src/PMG/API2/OIDC.pm b/src/PMG/API2/OIDC.pm
> new file mode 100644
> index 0000000..fdab44d
> --- /dev/null
> +++ b/src/PMG/API2/OIDC.pm
> @@ -0,0 +1,243 @@
> +package PMG::API2::OIDC;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::Tools qw(extract_param lock_file);
> +use Proxmox::RS::OIDC;
> +
> +use PVE::Exception qw(raise raise_perm_exc raise_param_exc);
> +use PVE::SafeSyslog;
> +use PVE::INotify;
> +use PVE::JSONSchema qw(get_standard_option);
> +
> +use PMG::AccessControl;
> +use PMG::RESTEnvironment;
> +use PVE::RESTHandler;
> +
> +use base qw(PVE::RESTHandler);
> +
> +my $oidc_state_path = "/var/lib/pmg";
> +
> +my $lookup_oidc_auth = sub {
> + my ($realm, $redirect_url) = @_;
> +
> + my $cfg = PVE::INotify::read_file('realms.cfg');
> + my $ids = $cfg->{ids};
> +
> + die "authentication domain '$realm' does not exist\n" if !$ids->{$realm};
> +
> + my $config = $ids->{$realm};
> + die "wrong realm type ($config->{type} != oidc)\n" if $config->{type} ne "oidc";
> +
> + my $oidc_config = {
> + issuer_url => $config->{'issuer-url'},
> + client_id => $config->{'client-id'},
> + client_key => $config->{'client-key'},
> + };
> + $oidc_config->{prompt} = $config->{'prompt'} if defined($config->{'prompt'});
> +
> + my $scopes = $config->{'scopes'} // 'email profile';
> + $oidc_config->{scopes} = [ PVE::Tools::split_list($scopes) ];
> +
> + if (defined(my $acr = $config->{'acr-values'})) {
> + $oidc_config->{acr_values} = [ PVE::Tools::split_list($acr) ];
> + }
> +
> + my $oidc = Proxmox::RS::OIDC->discover($oidc_config, $redirect_url);
> + return ($config, $oidc);
> +};
> +
> +__PACKAGE__->register_method ({
> + name => 'index',
> + path => '',
> + method => 'GET',
> + description => "Directory index.",
> + permissions => {
> + user => 'all',
> + },
> + parameters => {
> + additionalProperties => 0,
> + properties => {},
> + },
> + returns => {
> + type => 'array',
> + items => {
> + type => "object",
> + properties => {
> + subdir => { type => 'string' },
> + },
> + },
> + links => [ { rel => 'child', href => "{subdir}" } ],
> + },
> + code => sub {
> + my ($param) = @_;
> +
> + return [
> + { subdir => 'auth-url' },
> + { subdir => 'login' },
> + ];
> + }});
> +
> +__PACKAGE__->register_method ({
> + name => 'auth_url',
> + path => 'auth-url',
> + method => 'POST',
> + protected => 1,
> + description => "Get the OpenId Connect Authorization Url for the specified realm.",
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + realm => {
> + description => "Authentication domain ID",
> + type => 'string',
> + pattern => qr/[A-Za-z][A-Za-z0-9\.\-_]+/,
> + maxLength => 32,
> + },
> + 'redirect-url' => {
> + description => "Redirection Url. The client should set this to the used server url (location.origin).",
> + type => 'string',
> + maxLength => 255,
> + },
> + },
> + },
> + returns => {
> + type => "string",
> + description => "Redirection URL.",
> + },
> + permissions => { user => 'world' },
> + code => sub {
> + my ($param) = @_;
> +
> + my $realm = extract_param($param, 'realm');
> + my $redirect_url = extract_param($param, 'redirect-url');
> +
> + my ($config, $oidc) = $lookup_oidc_auth->($realm, $redirect_url);
> + my $url = $oidc->authorize_url($oidc_state_path , $realm);
> +
> + return $url;
> + }});
> +
> +__PACKAGE__->register_method ({
> + name => 'login',
> + path => 'login',
> + method => 'POST',
> + protected => 1,
> + description => " Verify OpenID Connect authorization code and create a ticket.",
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + 'state' => {
> + description => "OpenId Connect state.",
> + type => 'string',
> + maxLength => 1024,
> + },
> + code => {
> + description => "OpenId Connect authorization code.",
> + type => 'string',
> + maxLength => 4096,
> + },
> + 'redirect-url' => {
> + description => "Redirection Url. The client should set this to the used server url (location.origin).",
> + type => 'string',
> + maxLength => 255,
> + },
> + },
> + },
> + returns => {
> + properties => {
> + role => { type => 'string', optional => 1},
> + username => { type => 'string' },
> + ticket => { type => 'string' },
> + CSRFPreventionToken => { type => 'string' },
> + },
> + },
> + permissions => { user => 'world' },
> + code => sub {
> + my ($param) = @_;
> +
> + my $rpcenv = PMG::RESTEnvironment->get();
> +
> + my $res;
> + eval {
> + my ($realm, $private_auth_state) = Proxmox::RS::OIDC::verify_public_auth_state(
> + $oidc_state_path, $param->{'state'});
> +
> + my $redirect_url = extract_param($param, 'redirect-url');
> +
> + my ($config, $oidc) = $lookup_oidc_auth->($realm, $redirect_url);
> +
> + my $info = $oidc->verify_authorization_code($param->{code}, $private_auth_state);
> + my $subject = $info->{'sub'};
> +
> + my $unique_name;
> +
> + my $user_attr = $config->{'username-claim'} // 'sub';
> + if (defined($info->{$user_attr})) {
> + $unique_name = $info->{$user_attr};
> + } elsif ($user_attr eq 'subject') { # stay compat with old versions
> + $unique_name = $subject;
> + } elsif ($user_attr eq 'username') { # stay compat with old versions
> + my $username = $info->{'preferred_username'};
> + die "missing claim 'preferred_username'\n" if !defined($username);
> + $unique_name = $username;
> + } else {
> + # neither the attr nor fallback are defined in info..
> + die "missing configured claim '$user_attr' in returned info object\n";
> + }
> +
> + my $username = "${unique_name}\@${realm}";
> + # first, check if $username respects our naming conventions
> + PMG::Utils::verify_username($username);
> + if ($config->{'autocreate'} && !$rpcenv->check_user_exist($username, 1)) {
ah - I overlooked this call earlier!
> + my $code = sub {
> + my $usercfg = PMG::UserConfig->new();
> +
> + my $entry = { enable => 1 };
> + if (defined(my $email = $info->{'email'})) {
> + $entry->{email} = $email;
> + }
> + if (defined(my $given_name = $info->{'given_name'})) {
> + $entry->{firstname} = $given_name;
> + }
> + if (defined(my $family_name = $info->{'family_name'})) {
> + $entry->{lastname} = $family_name;
> + }
> + $entry->{role} = $config->{'autocreate-role'} // 'audit';
> + $entry->{userid} = $username;
> + $entry->{username} = $unique_name;
> + $entry->{realm} = $realm;
> +
> + die "User '$username' already exists\n"
> + if $usercfg->{$username};
> +
> + $usercfg->{$username} = $entry;
> +
> + $usercfg->write();
> + };
> + PMG::UserConfig::lock_config($code, "autocreate openid connect user failed");
> + }
> + my $role = $rpcenv->check_user_enabled($username);
> +
> + my $ticket = PMG::Ticket::assemble_ticket($username);
> + my $csrftoken = PMG::Ticket::assemble_csrf_prevention_token($username);
> +
> + $res = {
> + ticket => $ticket,
> + username => $username,
> + CSRFPreventionToken => $csrftoken,
> + role => $role,
> + };
> +
> + };
> + if (my $err = $@) {
> + my $clientip = $rpcenv->get_client_ip() || '';
> + syslog('err', "openid connect authentication failure; rhost=$clientip msg=$err");
> + # do not return any info to prevent user enumeration attacks
> + die PVE::Exception->new("authentication failure $err\n", code => 401);
> + }
> +
> + syslog('info', 'root@pam', "successful openid connect auth for user '$res->{username}'");
> +
> + return $res;
> + }});
> diff --git a/src/PMG/HTTPServer.pm b/src/PMG/HTTPServer.pm
> index 49724fe..27a313d 100644
> --- a/src/PMG/HTTPServer.pm
> +++ b/src/PMG/HTTPServer.pm
> @@ -58,6 +58,8 @@ sub auth_handler {
>
> # explicitly allow some calls without auth
> if (($rel_uri eq '/access/domains' && $method eq 'GET') ||
> + ($rel_uri eq '/access/oidc/login' && $method eq 'POST') ||
> + ($rel_uri eq '/access/oidc/auth-url' && $method eq 'POST') ||
> ($rel_uri eq '/quarantine/sendlink' && ($method eq 'GET' || $method eq 'POST')) ||
> ($rel_uri eq '/access/ticket' && ($method eq 'GET' || $method eq 'POST'))) {
> $require_auth = 0;
> --
> 2.39.5
>
>
>
> _______________________________________________
> pmg-devel mailing list
> pmg-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [pmg-devel] [PATCH widget-toolkit v5 8/10] fix: window: AuthEditBase: rename variable 'realm' to 'type'
2025-02-18 16:19 ` [pmg-devel] [PATCH widget-toolkit v5 8/10] fix: window: AuthEditBase: rename variable 'realm' to 'type' Markus Frank
@ 2025-02-21 12:45 ` Fabian Grünbichler
0 siblings, 0 replies; 27+ messages in thread
From: Fabian Grünbichler @ 2025-02-21 12:45 UTC (permalink / raw)
To: Markus Frank, pmg-devel
> Markus Frank <m.frank@proxmox.com> hat am 18.02.2025 17:19 CET geschrieben:
>
>
> PVE/PMG API returns a variable called 'type' instead of 'realm'
>
> Fixes: 3822a031ddbe4136aa847476f2e3785934a41547
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
this looks like an independent bug fix?
> ---
> src/window/AuthEditBase.js | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/window/AuthEditBase.js b/src/window/AuthEditBase.js
> index 73c1fee..e044235 100644
> --- a/src/window/AuthEditBase.js
> +++ b/src/window/AuthEditBase.js
> @@ -91,9 +91,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.realm !== me.authType) {
> + if (!me.useTypeInUrl && data.type !== me.authType) {
> me.close();
> - throw `got wrong auth type '${me.authType}' for realm '${data.realm}'`;
> + throw `got wrong auth type '${me.authType}' for realm '${data.type}'`;
> }
> me.setValues(data);
> },
> --
> 2.39.5
>
>
>
> _______________________________________________
> pmg-devel mailing list
> pmg-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [pmg-devel] [PATCH pmg-gui v5 10/10] add panel for realms to User Management
2025-02-18 16:19 ` [pmg-devel] [PATCH pmg-gui v5 10/10] add panel for realms to User Management Markus Frank
2025-02-21 9:22 ` Christoph Heiss
@ 2025-02-21 12:45 ` Fabian Grünbichler
1 sibling, 0 replies; 27+ messages in thread
From: Fabian Grünbichler @ 2025-02-21 12:45 UTC (permalink / raw)
To: Markus Frank, pmg-devel
> Markus Frank <m.frank@proxmox.com> hat am 18.02.2025 17:19 CET geschrieben:
>
>
> Make the realm configuration available in PMG and disable LDAP/AD
> realms for now and use the name oidc instead of openid.
>
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> ---
> js/UserManagement.js | 6 ++++++
> js/Utils.js | 23 +++++++++++++++++++++++
> 2 files changed, 29 insertions(+)
>
> diff --git a/js/UserManagement.js b/js/UserManagement.js
> index 65fabbf..54493b1 100644
> --- a/js/UserManagement.js
> +++ b/js/UserManagement.js
> @@ -34,5 +34,11 @@ Ext.define('PMG.UserManagement', {
> itemId: 'pop',
> iconCls: 'fa fa-reply-all',
> },
> + {
> + xtype: 'pmxAuthView',
> + title: gettext('Realms'),
> + itemId: 'domains',
> + iconCls: 'fa fa-address-book-o',
> + },
> ],
> });
> diff --git a/js/Utils.js b/js/Utils.js
> index 9b5f054..055b938 100644
> --- a/js/Utils.js
> +++ b/js/Utils.js
> @@ -851,6 +851,29 @@ Ext.define('PMG.Utils', {
> constructor: function() {
> var me = this;
>
> + // use oidc instead of openid
> + Proxmox.Schema.authDomains.oidc = Proxmox.Schema.authDomains.openid;
> + Proxmox.Schema.authDomains.oidc.useTypeInUrl = false;
> + delete Proxmox.Schema.authDomains.openid;
unless we want to rename it to oidc everywhere, this makes me think that it's not a good idea to deviate from the existing naming scheme in PMG?
> +
> + // Disable LDAP/AD as a realm until LDAP/AD login is implemented
> + Proxmox.Schema.authDomains.ldap.add = false;
> + Proxmox.Schema.authDomains.ad.add = false;
> +
> + Proxmox.Schema.authDomains.pmg = {
> + add: false,
> + edit: false,
> + pwchange: false,
> + sync: false,
> + };
> +
> + Proxmox.Schema.authDomains.pam = {
> + add: false,
> + edit: false,
> + pwchange: false,
> + sync: false,
> + };
> +
> // do whatever you want here
> Proxmox.Utils.override_task_descriptions({
> applycustomscores: ['', gettext('Apply custom SpamAssassin scores')],
> --
> 2.39.5
>
>
>
> _______________________________________________
> pmg-devel mailing list
> pmg-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [pmg-devel] [PATCH pmg-api v5 6/10] api: add/update/remove realms like in PVE
2025-02-21 12:41 ` Fabian Grünbichler
@ 2025-02-21 13:44 ` Markus Frank
2025-02-21 13:52 ` Fabian Grünbichler
0 siblings, 1 reply; 27+ messages in thread
From: Markus Frank @ 2025-02-21 13:44 UTC (permalink / raw)
To: Fabian Grünbichler, pmg-devel
Thank you for reviewing this patch series.
On 2025-02-21 13:41, Fabian Grünbichler wrote:
>
>> Markus Frank <m.frank@proxmox.com> hat am 18.02.2025 17:19 CET geschrieben:
>>
>>
>> The name Realm.pm was chosen because a Domain.pm already exists.
>
> but the API path is still domains, and the naming inside the code/descriptions/.. is also rather inconsistent. should we settle on one or the other?
We use /access/domain in PVE/PBS and already allow /access/domains in PMG/HTTPServer.pm:
```
# explicitly allow some calls without auth
if (($rel_uri eq '/access/domains' && $method eq 'GET') ||
($rel_uri eq '/quarantine/sendlink' && ($method eq 'GET' || $method eq 'POST')) ||
($rel_uri eq '/access/ticket' && ($method eq 'GET' || $method eq 'POST'))) {
```
Before renaming it to Realm, I was using Authdomain as the file/module name.
If we want to stick to one name, we either use Authdomains (or something similar) again, or we change everything to realm and use a different api path than PVE/PBS.
I think I would prefer using Authdomains and /access/domain.
Any opinions?
>
>>
>> Signed-off-by: Markus Frank <m.frank@proxmox.com>
>> ---
>> src/Makefile | 1 +
>> src/PMG/API2/AccessControl.pm | 10 +-
>> src/PMG/API2/Realm.pm | 274 ++++++++++++++++++++++++++++++++++
>> 3 files changed, 284 insertions(+), 1 deletion(-)
>> create mode 100644 src/PMG/API2/Realm.pm
>>
>> diff --git a/src/Makefile b/src/Makefile
>> index 3cae7c7..e939bbd 100644
>> --- a/src/Makefile
>> +++ b/src/Makefile
>> @@ -151,6 +151,7 @@ LIBSOURCES = \
>> PMG/API2/Postfix.pm \
>> PMG/API2/Quarantine.pm \
>> PMG/API2/AccessControl.pm \
>> + PMG/API2/Realm.pm \
>> PMG/API2/TFA.pm \
>> PMG/API2/TFAConfig.pm \
>> PMG/API2/ObjectGroupHelpers.pm \
>> diff --git a/src/PMG/API2/AccessControl.pm b/src/PMG/API2/AccessControl.pm
>> index e26ae70..f4cbc81 100644
>> --- a/src/PMG/API2/AccessControl.pm
>> +++ b/src/PMG/API2/AccessControl.pm
>> @@ -12,6 +12,7 @@ use PVE::JSONSchema qw(get_standard_option);
>> use PMG::Utils;
>> use PMG::UserConfig;
>> use PMG::AccessControl;
>> +use PMG::API2::Realm;
>> use PMG::API2::Users;
>> use PMG::API2::TFA;
>> use PMG::TFAConfig;
>> @@ -30,6 +31,11 @@ __PACKAGE__->register_method ({
>> path => 'tfa',
>> });
>>
>> +__PACKAGE__->register_method ({
>> + subclass => "PMG::API2::Realm",
>> + path => 'domains',
>
> realm vs domains
>
>> +});
>> +
>> __PACKAGE__->register_method ({
>> name => 'index',
>> path => '',
>> @@ -57,6 +63,7 @@ __PACKAGE__->register_method ({
>>
>> my $res = [
>> { subdir => 'ticket' },
>> + { subdir => 'domains' },
>> { subdir => 'password' },
>> { subdir => 'users' },
>> ];
>> @@ -248,7 +255,8 @@ __PACKAGE__->register_method ({
>>
>> my $username = $param->{username};
>>
>> - if ($username !~ m/\@(pam|pmg|quarantine)$/) {
>> + my $realm_regex = PMG::Utils::valid_pmg_realm_regex();
>> + if ($username !~ m/\@(${realm_regex})$/) {
>> my $realm = $param->{realm} // 'quarantine';
>> $username .= "\@$realm";
>> }
>> diff --git a/src/PMG/API2/Realm.pm b/src/PMG/API2/Realm.pm
>> new file mode 100644
>> index 0000000..da2072a
>> --- /dev/null
>> +++ b/src/PMG/API2/Realm.pm
>> @@ -0,0 +1,274 @@
>> +package PMG::API2::Realm;
>> +
>> +use strict;
>> +use warnings;
>> +
>> +use PVE::Exception qw(raise_param_exc);
>> +use PVE::INotify;
>> +use PVE::JSONSchema qw(get_standard_option);
>> +use PVE::RESTHandler;
>> +use PVE::SafeSyslog;
>> +use PVE::Tools qw(extract_param);
>> +
>> +use PMG::AccessControl;
>> +use PMG::Auth::Plugin;
>> +use PVE::Schema::Auth;
>> +
>> +my $domainconfigfile = "realms.cfg";
>> +
>> +use base qw(PVE::RESTHandler);
>> +
>> +__PACKAGE__->register_method ({
>> + name => 'index',
>> + path => '',
>> + method => 'GET',
>> + description => "Authentication domain index.",
>
> and here it is still an authentication domain ;)
>
>> + permissions => {
>> + description => "Anyone can access that, because we need that list for the login box (before the user is authenticated).",
>> + user => 'world',
>> + },
>> + parameters => {
>> + additionalProperties => 0,
>> + properties => {},
>> + },
>> + returns => {
>> + type => 'array',
>> + items => {
>> + type => "object",
>> + properties => {
>> + realm => { type => 'string' },
>
> even though it returns realms
>
>> + type => { type => 'string' },
>> + tfa => {
>> + description => "Two-factor authentication provider.",
>> + type => 'string',
>> + enum => [ 'yubico', 'oath' ],
>> + optional => 1,
>> + },
>> + comment => {
>> + description => "A comment. The GUI use this text when you select a domain (Realm) on the login window.",
>> + type => 'string',
>> + optional => 1,
>> + },
>> + },
>> + },
>> + links => [ { rel => 'child', href => "{realm}" } ],
>> + },
>> + code => sub {
>> + my ($param) = @_;
>> +
>> + my $res = [];
>> +
>> + my $cfg = PVE::INotify::read_file($domainconfigfile);
>> + my $ids = $cfg->{ids};
>> +
>> + for my $realm (keys %$ids) {
>> + my $d = $ids->{$realm};
>> + my $entry = { realm => $realm, type => $d->{type} };
>> + $entry->{comment} = $d->{comment} if $d->{comment};
>> + $entry->{default} = 1 if $d->{default};
>> + if ($d->{tfa} && (my $tfa_cfg = PVE::Schema::Auth::parse_tfa_config($d->{tfa}))) {
>> + $entry->{tfa} = $tfa_cfg->{type};
>> + }
>> + push @$res, $entry;
>> + }
>> +
>> + return $res;
>> + }});
>> +
>> +__PACKAGE__->register_method ({
>> + name => 'create',
>> + protected => 1,
>> + path => '',
>> + method => 'POST',
>> + permissions => { check => [ 'admin' ] },
>> + description => "Add an authentication server.",
>> + parameters => PMG::Auth::Plugin->createSchema(0),
>> + returns => { type => 'null' },
>> + code => sub {
>> + my ($param) = @_;
>> +
>> + # always extract, add it with hook
>> + my $password = extract_param($param, 'password');
>> +
>> + PMG::Auth::Plugin::lock_domain_config(
>> + sub {
>> + my $cfg = PVE::INotify::read_file($domainconfigfile);
>> + my $ids = $cfg->{ids};
>> +
>> + my $realm = extract_param($param, 'realm');
>> + PMG::Auth::Plugin::pmg_verify_realm($realm);
>> + my $type = $param->{type};
>> + my $check_connection = extract_param($param, 'check-connection');
>> +
>> + die "domain '$realm' already exists\n"
>> + if $ids->{$realm};
>> +
>> + die "unable to use reserved name '$realm'\n"
>> + if ($realm eq 'pam' || $realm eq 'pmg');
>> +
>> + die "unable to create builtin type '$type'\n"
>> + if ($type eq 'pam' || $type eq 'pmg');
>> +
>> + my $plugin = PMG::Auth::Plugin->lookup($type);
>> + my $config = $plugin->check_config($realm, $param, 1, 1);
>> +
>> + if ($config->{default}) {
>> + for my $r (keys %$ids) {
>> + delete $ids->{$r}->{default};
>> + }
>> + }
>> +
>> + $ids->{$realm} = $config;
>> +
>> + my $opts = $plugin->options();
>> + if (defined($password) && !defined($opts->{password})) {
>> + $password = undef;
>> + warn "ignoring password parameter";
>> + }
>> + $plugin->on_add_hook($realm, $config, password => $password);
>> +
>> + PVE::INotify::write_file($domainconfigfile, $cfg);
>> + },
>> + "add auth server failed",
>> + );
>> + return undef;
>> + }});
>> +
>> +__PACKAGE__->register_method ({
>> + name => 'update',
>> + path => '{realm}',
>> + method => 'PUT',
>> + permissions => { check => [ 'admin' ] },
>> + description => "Update authentication server settings.",
>> + protected => 1,
>> + parameters => PMG::Auth::Plugin->updateSchema(0),
>> + returns => { type => 'null' },
>> + code => sub {
>> + my ($param) = @_;
>> +
>> + # always extract, update in hook
>> + my $password = extract_param($param, 'password');
>> +
>> + PMG::Auth::Plugin::lock_domain_config(
>> + sub {
>> + my $cfg = PVE::INotify::read_file($domainconfigfile);
>> + my $ids = $cfg->{ids};
>> +
>> + my $digest = extract_param($param, 'digest');
>> + PVE::SectionConfig::assert_if_modified($cfg, $digest);
>> +
>> + my $realm = extract_param($param, 'realm');
>> + my $type = $ids->{$realm}->{type};
>> + my $check_connection = extract_param($param, 'check-connection');
>> +
>> + die "domain '$realm' does not exist\n"
>> + if !$ids->{$realm};
>> +
>> + my $delete_str = extract_param($param, 'delete');
>> + die "no options specified\n"
>> + if !$delete_str && !scalar(keys %$param) && !defined($password);
>> +
>> + my $delete_pw = 0;
>> + for my $opt (PVE::Tools::split_list($delete_str)) {
>> + delete $ids->{$realm}->{$opt};
>> + $delete_pw = 1 if $opt eq 'password';
>> + }
>> +
>> + my $plugin = PMG::Auth::Plugin->lookup($type);
>> + my $config = $plugin->check_config($realm, $param, 0, 1);
>> +
>> + if ($config->{default}) {
>> + for my $r (keys %$ids) {
>> + delete $ids->{$r}->{default};
>> + }
>> + }
>> +
>> + for my $p (keys %$config) {
>> + $ids->{$realm}->{$p} = $config->{$p};
>> + }
>> +
>> + my $opts = $plugin->options();
>> + if ($delete_pw || defined($password)) {
>> + $plugin->on_update_hook($realm, $config, password => $password);
>> + } else {
>> + $plugin->on_update_hook($realm, $config);
>> + }
>> +
>> + PVE::INotify::write_file($domainconfigfile, $cfg);
>> + },
>> + "update auth server failed"
>> + );
>> + return undef;
>> + }});
>> +
>> +# fixme: return format!
>> +__PACKAGE__->register_method ({
>> + name => 'read',
>> + path => '{realm}',
>> + method => 'GET',
>> + description => "Get auth server configuration.",
>> + permissions => { check => [ 'admin', 'qmanager', 'audit' ] },
>> + parameters => {
>> + additionalProperties => 0,
>> + properties => {
>> + realm => get_standard_option('realm'),
>> + },
>> + },
>> + returns => {},
>> + code => sub {
>> + my ($param) = @_;
>> +
>> + my $cfg = PVE::INotify::read_file($domainconfigfile);
>> +
>> + my $realm = $param->{realm};
>> +
>> + my $data = $cfg->{ids}->{$realm};
>> + die "domain '$realm' does not exist\n" if !$data;
>> +
>> + my $type = $data->{type};
>> +
>> + $data->{digest} = $cfg->{digest};
>> +
>> + return $data;
>> + }});
>> +
>> +
>> +__PACKAGE__->register_method ({
>> + name => 'delete',
>> + path => '{realm}',
>> + method => 'DELETE',
>> + permissions => { check => [ 'admin' ] },
>> + description => "Delete an authentication server.",
>> + protected => 1,
>> + parameters => {
>> + additionalProperties => 0,
>> + properties => {
>> + realm => get_standard_option('realm'),
>> + }
>> + },
>> + returns => { type => 'null' },
>> + code => sub {
>> + my ($param) = @_;
>> +
>> + PMG::Auth::Plugin::lock_domain_config(
>> + sub {
>> + my $cfg = PVE::INotify::read_file($domainconfigfile);
>> + my $ids = $cfg->{ids};
>> + my $realm = $param->{realm};
>> +
>> + die "authentication domain '$realm' does not exist\n" if !$ids->{$realm};
>> +
>> + my $plugin = PMG::Auth::Plugin->lookup($ids->{$realm}->{type});
>> +
>> + $plugin->on_delete_hook($realm, $ids->{$realm});
>> +
>> + delete $ids->{$realm};
>> +
>> + PVE::INotify::write_file($domainconfigfile, $cfg);
>> + },
>> + "delete auth server failed",
>> + );
>> + return undef;
>> + }});
>> +
>> +1;
>> --
>> 2.39.5
>>
>>
>>
>> _______________________________________________
>> pmg-devel mailing list
>> pmg-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [pmg-devel] [PATCH pmg-api v5 6/10] api: add/update/remove realms like in PVE
2025-02-21 13:44 ` Markus Frank
@ 2025-02-21 13:52 ` Fabian Grünbichler
2025-02-21 14:38 ` Stoiko Ivanov
2025-02-21 16:45 ` Thomas Lamprecht
0 siblings, 2 replies; 27+ messages in thread
From: Fabian Grünbichler @ 2025-02-21 13:52 UTC (permalink / raw)
To: Markus Frank, pmg-devel
> Markus Frank <m.frank@proxmox.com> hat am 21.02.2025 14:44 CET geschrieben:
>
>
> Thank you for reviewing this patch series.
>
> On 2025-02-21 13:41, Fabian Grünbichler wrote:
> >
> >> Markus Frank <m.frank@proxmox.com> hat am 18.02.2025 17:19 CET geschrieben:
> >>
> >>
> >> The name Realm.pm was chosen because a Domain.pm already exists.
> >
> > but the API path is still domains, and the naming inside the code/descriptions/.. is also rather inconsistent. should we settle on one or the other?
>
> We use /access/domain in PVE/PBS and already allow /access/domains in PMG/HTTPServer.pm:
> ```
> # explicitly allow some calls without auth
> if (($rel_uri eq '/access/domains' && $method eq 'GET') ||
> ($rel_uri eq '/quarantine/sendlink' && ($method eq 'GET' || $method eq 'POST')) ||
> ($rel_uri eq '/access/ticket' && ($method eq 'GET' || $method eq 'POST'))) {
> ```
>
> Before renaming it to Realm, I was using Authdomain as the file/module name.
> If we want to stick to one name, we either use Authdomains (or something similar) again, or we change everything to realm and use a different api path than PVE/PBS.
> I think I would prefer using Authdomains and /access/domain.
>
> Any opinions?
I think we have three options:
- use domains just for the api path, rename it to realm across the board otherwise in PMG (this is a bit what the v5 of the patch does, but it doesn't do it 100% ;))
- use realm everywhere in PMG (might require adaptations in pwt and other common code to allow this, and probably requires API clients to adapt to that as well if shared across PMG/PBS/PVE?), and migrate PVE and PBS to that terminology as well at some point
- use domains and realm interchangeably like in PVE (requires to name at least the perl module differently in PMG, and might be confusing?)
this is a bit of a historic issue, and not the fault of this patch series - I'd just like to avoid making it worse by calling the same thing "realm", "domain", "authdomain", "authentication domain" while also having other "domain"s in PMG if we can avoid it ;) for that reason alone the third option is the least attractive to me.
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [pmg-devel] [PATCH pmg-api v5 6/10] api: add/update/remove realms like in PVE
2025-02-21 13:52 ` Fabian Grünbichler
@ 2025-02-21 14:38 ` Stoiko Ivanov
2025-02-21 16:45 ` Thomas Lamprecht
1 sibling, 0 replies; 27+ messages in thread
From: Stoiko Ivanov @ 2025-02-21 14:38 UTC (permalink / raw)
To: Fabian Grünbichler; +Cc: pmg-devel
On Fri, 21 Feb 2025 14:52:48 +0100 (CET)
Fabian Grünbichler <f.gruenbichler@proxmox.com> wrote:
> > Markus Frank <m.frank@proxmox.com> hat am 21.02.2025 14:44 CET geschrieben:
> >
> >
> > Thank you for reviewing this patch series.
> >
> > On 2025-02-21 13:41, Fabian Grünbichler wrote:
> > >
> > >> Markus Frank <m.frank@proxmox.com> hat am 18.02.2025 17:19 CET geschrieben:
> > >>
> > >>
> > >> The name Realm.pm was chosen because a Domain.pm already exists.
> > >
> > > but the API path is still domains, and the naming inside the code/descriptions/.. is also rather inconsistent. should we settle on one or the other?
> >
> > We use /access/domain in PVE/PBS and already allow /access/domains in PMG/HTTPServer.pm:
> > ```
> > # explicitly allow some calls without auth
> > if (($rel_uri eq '/access/domains' && $method eq 'GET') ||
> > ($rel_uri eq '/quarantine/sendlink' && ($method eq 'GET' || $method eq 'POST')) ||
> > ($rel_uri eq '/access/ticket' && ($method eq 'GET' || $method eq 'POST'))) {
> > ```
> >
> > Before renaming it to Realm, I was using Authdomain as the file/module name.
> > If we want to stick to one name, we either use Authdomains (or something similar) again, or we change everything to realm and use a different api path than PVE/PBS.
> > I think I would prefer using Authdomains and /access/domain.
> >
> > Any opinions?
>
> I think we have three options:
> - use domains just for the api path, rename it to realm across the board otherwise in PMG (this is a bit what the v5 of the patch does, but it doesn't do it 100% ;))
> - use realm everywhere in PMG (might require adaptations in pwt and other common code to allow this, and probably requires API clients to adapt to that as well if shared across PMG/PBS/PVE?), and migrate PVE and PBS to that terminology as well at some point
> - use domains and realm interchangeably like in PVE (requires to name at least the perl module differently in PMG, and might be confusing?)
>
> this is a bit of a historic issue, and not the fault of this patch series - I'd just like to avoid making it worse by calling the same thing "realm", "domain", "authdomain", "authentication domain" while also having other "domain"s in PMG if we can avoid it ;) for that reason alone the third option is the least attractive to me.
I prefer the second option - and if we pick it it would be a good time to
consider if 'realm' might be used in a different context (now or later on)
as well?
I've only heard the term in context of authentication(/authorization) -
and a quick search online did not show too much other uses - but before we
have to rename API-paths again in the future - I thought I'd ask now (if
there's other uses 'authrealm', 'authentication realm' should be unique
enough I hope)
>
>
> _______________________________________________
> pmg-devel mailing list
> pmg-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [pmg-devel] [PATCH pmg-api v5 6/10] api: add/update/remove realms like in PVE
2025-02-21 13:52 ` Fabian Grünbichler
2025-02-21 14:38 ` Stoiko Ivanov
@ 2025-02-21 16:45 ` Thomas Lamprecht
1 sibling, 0 replies; 27+ messages in thread
From: Thomas Lamprecht @ 2025-02-21 16:45 UTC (permalink / raw)
To: Fabian Grünbichler, Markus Frank, pmg-devel
Am 21.02.25 um 14:52 schrieb Fabian Grünbichler:
>> Markus Frank <m.frank@proxmox.com> hat am 21.02.2025 14:44 CET geschrieben:
>> We use /access/domain in PVE/PBS and already allow /access/domains in PMG/HTTPServer.pm:
>> ```
>> # explicitly allow some calls without auth
>> if (($rel_uri eq '/access/domains' && $method eq 'GET') ||
>> ($rel_uri eq '/quarantine/sendlink' && ($method eq 'GET' || $method eq 'POST')) ||
>> ($rel_uri eq '/access/ticket' && ($method eq 'GET' || $method eq 'POST'))) {
>> ```
>>
>> Before renaming it to Realm, I was using Authdomain as the
>> file/module name.
>> If we want to stick to one name, we either use Authdomains (or
Whatever we end up with, let's please ensure to use CamelCase for the
module name though.
>> something similar) again, or we change everything to realm and use a
>> different api path than PVE/PBS.
>> I think I would prefer using Authdomains and /access/domain.
>>
>> Any opinions?
>
> I think we have three options:
> - use domains just for the api path, rename it to realm across the
> board otherwise in PMG (this is a bit what the v5 of the patch does,
> but it doesn't do it 100% ;))
meh, but something I'd be OK to accept if it helps bringin this over the
line faster, but changing this just for PMG should not be _that_ much
work.
> - use realm everywhere in PMG (might require adaptations in pwt and
> other common code to allow this, and probably requires API clients
> to adapt to that as well if shared across PMG/PBS/PVE?), and migrate
> PVE and PBS to that terminology as well at some point
Would also favour that, but IMO it could be indeed fine to switch to
something very close to authentication-realm or probably better, as it
would be shorter but still as telling, auth-realm to make it even
clearer what realm means in this conetxt. I.e., get a tiny bit more
benefits out of changing this, especially if we want to align our other
projects in the future.
That said, I do not have _that_ strong feelings about the "auth" part
being included, so just reaml is fine too.
> - use domains and realm interchangeably like in PVE (requires to name
> at least the perl module differently in PMG, and might be
> confusing?)
yeah that would be worse in PMG than it already is in PVE/PBS due to
frequent use of the domain term for FQDNs.
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2025-02-21 16:45 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-02-18 16:18 [pmg-devel] [PATCH pve-common/perl-rs/pmg-api/widget-toolkit/pmg-gui v5 0/10] fix #3892: OpenID Connect Markus Frank
2025-02-18 16:18 ` [pmg-devel] [PATCH pve-common v5 1/10] add Schema package with auth module that contains realm sync options Markus Frank
2025-02-19 18:18 ` Stoiko Ivanov
2025-02-21 12:22 ` Fabian Grünbichler
2025-02-18 16:18 ` [pmg-devel] [PATCH proxmox-perl-rs v5 2/10] move openid code from pve-rs to common Markus Frank
2025-02-21 12:25 ` Fabian Grünbichler
2025-02-18 16:18 ` [pmg-devel] [PATCH proxmox-perl-rs v5 3/10] remove empty PMG::RS::OpenId package to avoid confusion Markus Frank
2025-02-18 16:18 ` [pmg-devel] [PATCH pmg-api v5 4/10] config: add plugin system for realms Markus Frank
2025-02-21 12:35 ` Fabian Grünbichler
2025-02-18 16:19 ` [pmg-devel] [PATCH pmg-api v5 5/10] config: add oidc type realm Markus Frank
2025-02-21 12:38 ` Fabian Grünbichler
2025-02-18 16:19 ` [pmg-devel] [PATCH pmg-api v5 6/10] api: add/update/remove realms like in PVE Markus Frank
2025-02-21 12:41 ` Fabian Grünbichler
2025-02-21 13:44 ` Markus Frank
2025-02-21 13:52 ` Fabian Grünbichler
2025-02-21 14:38 ` Stoiko Ivanov
2025-02-21 16:45 ` Thomas Lamprecht
2025-02-18 16:19 ` [pmg-devel] [PATCH pmg-api v5 7/10] api: oidc login similar to PVE Markus Frank
2025-02-19 18:31 ` Stoiko Ivanov
2025-02-21 12:44 ` Fabian Grünbichler
2025-02-18 16:19 ` [pmg-devel] [PATCH widget-toolkit v5 8/10] fix: window: AuthEditBase: rename variable 'realm' to 'type' Markus Frank
2025-02-21 12:45 ` Fabian Grünbichler
2025-02-18 16:19 ` [pmg-devel] [PATCH pmg-gui v5 09/10] login: add option to login with OIDC realm Markus Frank
2025-02-18 16:19 ` [pmg-devel] [PATCH pmg-gui v5 10/10] add panel for realms to User Management Markus Frank
2025-02-21 9:22 ` Christoph Heiss
2025-02-21 12:45 ` Fabian Grünbichler
2025-02-19 18:39 ` [pmg-devel] [PATCH pve-common/perl-rs/pmg-api/widget-toolkit/pmg-gui v5 0/10] fix #3892: OpenID Connect Stoiko Ivanov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal