* [pmg-devel] [PATCH pve-common/proxmox-perl-rs/pmg-api/pmg-gui v3 0/8] fix #3892: OpenID
@ 2024-06-24 9:08 Markus Frank
2024-06-24 9:08 ` [pmg-devel] [PATCH pve-common v3 1/8] add Schema package with auth module that contains realm sync options Markus Frank
` (8 more replies)
0 siblings, 9 replies; 15+ messages in thread
From: Markus Frank @ 2024-06-24 9:08 UTC (permalink / raw)
To: pmg-devel
Patch-series to enable OpenID Login for PMG
apply/compile order:
1. pve-common: add Schema package with auth module that contains realm sync options
2. proxmox-perl-rs: move openid code from pve-rs to common
3. proxmox-perl-rs: remove empty PMG::RS::OpenId package to avoid confusion
4. pmg-api: config: add plugin system for realms & add openid type realms
5. pmg-api: api: add/update/remove realms like in PVE
6. pmg-api: api: openid login similar to PVE
7. pmg-gui: login: add option to login with OpenID realm
8. pmg-gui: add panel for realms to User Management
v3 changed only in proxmox-perl-rs and "pmg-api: api: openid login similar to PVE"
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:
v3: removed PMG wrapper as Proxmox::RS:OpenId can be used instead.
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/openid/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/openid/mod.rs
pmg-api:
v3: use Proxmox::RS:OpenId instead of PMG::RS::OpenId
Markus Frank (3):
config: add plugin system for realms & add openid type realms
api: add/update/remove realms like in PVE
api: openid login similar to PVE
src/Makefile | 5 +
src/PMG/API2/AccessControl.pm | 17 ++-
src/PMG/API2/Authdomains.pm | 274 ++++++++++++++++++++++++++++++++++
src/PMG/API2/OIDC.pm | 243 ++++++++++++++++++++++++++++++
src/PMG/AccessControl.pm | 31 ++++
src/PMG/Auth/OIDC.pm | 99 ++++++++++++
src/PMG/Auth/PMG.pm | 28 ++++
src/PMG/Auth/Plugin.pm | 193 ++++++++++++++++++++++++
src/PMG/HTTPServer.pm | 2 +
src/PMG/RESTEnvironment.pm | 14 ++
src/PMG/UserConfig.pm | 25 ++--
src/PMG/Utils.pm | 29 +++-
12 files changed, 943 insertions(+), 17 deletions(-)
create mode 100644 src/PMG/API2/Authdomains.pm
create mode 100644 src/PMG/API2/OIDC.pm
create mode 100755 src/PMG/Auth/OIDC.pm
create mode 100755 src/PMG/Auth/PMG.pm
create mode 100755 src/PMG/Auth/Plugin.pm
pmg-gui:
Markus Frank (2):
login: add OpenID realms
add panel for realms to User Management
js/LoginView.js | 200 +++++++++++++++++++++++++++++++++----------
js/UserManagement.js | 6 ++
js/Utils.js | 15 ++++
3 files changed, 174 insertions(+), 47 deletions(-)
--
2.39.2
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [pmg-devel] [PATCH pve-common v3 1/8] add Schema package with auth module that contains realm sync options
2024-06-24 9:08 [pmg-devel] [PATCH pve-common/proxmox-perl-rs/pmg-api/pmg-gui v3 0/8] fix #3892: OpenID Markus Frank
@ 2024-06-24 9:08 ` Markus Frank
2024-06-24 9:08 ` [pmg-devel] [PATCH proxmox-perl-rs v3 2/8] move openid code from pve-rs to common Markus Frank
` (7 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Markus Frank @ 2024-06-24 9:08 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.2
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [pmg-devel] [PATCH proxmox-perl-rs v3 2/8] move openid code from pve-rs to common
2024-06-24 9:08 [pmg-devel] [PATCH pve-common/proxmox-perl-rs/pmg-api/pmg-gui v3 0/8] fix #3892: OpenID Markus Frank
2024-06-24 9:08 ` [pmg-devel] [PATCH pve-common v3 1/8] add Schema package with auth module that contains realm sync options Markus Frank
@ 2024-06-24 9:08 ` Markus Frank
2024-10-09 11:30 ` Christoph Heiss
2024-06-24 9:08 ` [pmg-devel] [PATCH proxmox-perl-rs v3 3/8] remove empty PMG::RS::OpenId package to avoid confusion Markus Frank
` (6 subsequent siblings)
8 siblings, 1 reply; 15+ messages in thread
From: Markus Frank @ 2024-06-24 9:08 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/openid/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/openid/mod.rs
diff --git a/common/pkg/Makefile b/common/pkg/Makefile
index 78f2d64..a31264f 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::OpenId \
Proxmox::RS::Subscription
PERLMOD_PACKAGE_FILES := $(addsuffix .pm,$(subst ::,/,$(PERLMOD_PACKAGES)))
diff --git a/common/src/mod.rs b/common/src/mod.rs
index c3574f4..8c22a46 100644
--- a/common/src/mod.rs
+++ b/common/src/mod.rs
@@ -2,4 +2,5 @@ pub mod apt;
mod calendar_event;
pub mod logger;
pub mod notify;
+pub mod openid;
mod subscription;
diff --git a/common/src/openid/mod.rs b/common/src/openid/mod.rs
new file mode 100644
index 0000000..13bbaab
--- /dev/null
+++ b/common/src/openid/mod.rs
@@ -0,0 +1,63 @@
+#[perlmod::package(name = "Proxmox::RS::OpenId")]
+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<OpenId> : &OpenId as "Proxmox::RS::OpenId");
+
+ /// An OpenIdAuthenticator client instance.
+ pub struct OpenId {
+ inner: Mutex<OpenIdAuthenticator>,
+ }
+
+ /// Create a new OpenId client instance
+ #[export(raw_return)]
+ pub fn discover(
+ #[raw] class: Value,
+ 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),
+ })
+ ))
+ }
+
+ #[export]
+ pub fn authorize_url(
+ #[try_from_ref] this: &OpenId,
+ state_dir: &str,
+ realm: &str,
+ ) -> Result<String, Error> {
+ let open_id = this.inner.lock().unwrap();
+ open_id.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: &OpenId,
+ 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)?)
+ }
+}
diff --git a/pmg-rs/Cargo.toml b/pmg-rs/Cargo.toml
index 6557605..4b9568f 100644
--- a/pmg-rs/Cargo.toml
+++ b/pmg-rs/Cargo.toml
@@ -41,3 +41,4 @@ proxmox-subscription = "0.4"
proxmox-sys = "0.5"
proxmox-tfa = { version = "4.0.4", features = ["api"] }
proxmox-time = "2"
+proxmox-openid = "0.10.0"
diff --git a/pmg-rs/debian/control b/pmg-rs/debian/control
index 5a63b5d..dcc32eb 100644
--- a/pmg-rs/debian/control
+++ b/pmg-rs/debian/control
@@ -24,6 +24,7 @@ Build-Depends: cargo:native <!nocheck>,
librust-proxmox-http-error-0.1+default-dev,
librust-proxmox-notify-0.4+default-dev,
librust-proxmox-subscription-0.4+default-dev,
+ librust-proxmox-openid-0.10+default-dev,
librust-proxmox-sys-0.5+default-dev,
librust-proxmox-tfa-4+api-dev (>= 4.0.4-~~),
librust-proxmox-tfa-4+default-dev (>= 4.0.4-~~),
diff --git a/pve-rs/src/openid/mod.rs b/pve-rs/src/openid/mod.rs
index 1fa7572..d3ad5a5 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::openid::export as common;
+ use crate::common::openid::export::OpenId 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.2
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [pmg-devel] [PATCH proxmox-perl-rs v3 3/8] remove empty PMG::RS::OpenId package to avoid confusion
2024-06-24 9:08 [pmg-devel] [PATCH pve-common/proxmox-perl-rs/pmg-api/pmg-gui v3 0/8] fix #3892: OpenID Markus Frank
2024-06-24 9:08 ` [pmg-devel] [PATCH pve-common v3 1/8] add Schema package with auth module that contains realm sync options Markus Frank
2024-06-24 9:08 ` [pmg-devel] [PATCH proxmox-perl-rs v3 2/8] move openid code from pve-rs to common Markus Frank
@ 2024-06-24 9:08 ` Markus Frank
2024-06-24 9:08 ` [pmg-devel] [PATCH pmg-api v3 4/8] config: add plugin system for realms & add openid type realms Markus Frank
` (5 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Markus Frank @ 2024-06-24 9:08 UTC (permalink / raw)
To: pmg-devel
The common Proxmox::RS:OpenId 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.2
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [pmg-devel] [PATCH pmg-api v3 4/8] config: add plugin system for realms & add openid type realms
2024-06-24 9:08 [pmg-devel] [PATCH pve-common/proxmox-perl-rs/pmg-api/pmg-gui v3 0/8] fix #3892: OpenID Markus Frank
` (2 preceding siblings ...)
2024-06-24 9:08 ` [pmg-devel] [PATCH proxmox-perl-rs v3 3/8] remove empty PMG::RS::OpenId package to avoid confusion Markus Frank
@ 2024-06-24 9:08 ` Markus Frank
2024-10-10 8:46 ` Christoph Heiss
2024-10-18 12:07 ` Christoph Heiss
2024-06-24 9:08 ` [pmg-devel] [PATCH pmg-api v3 5/8] api: add/update/remove realms like in PVE Markus Frank
` (4 subsequent siblings)
8 siblings, 2 replies; 15+ messages in thread
From: Markus Frank @ 2024-06-24 9:08 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.
---
src/Makefile | 3 +
src/PMG/AccessControl.pm | 31 ++++++
src/PMG/Auth/OIDC.pm | 99 +++++++++++++++++++
src/PMG/Auth/PMG.pm | 28 ++++++
src/PMG/Auth/Plugin.pm | 193 +++++++++++++++++++++++++++++++++++++
src/PMG/RESTEnvironment.pm | 14 +++
src/PMG/UserConfig.pm | 25 +++--
src/PMG/Utils.pm | 29 ++++--
8 files changed, 406 insertions(+), 16 deletions(-)
create mode 100755 src/PMG/Auth/OIDC.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 8e49a10..4140698 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -164,6 +164,9 @@ LIBSOURCES = \
PMG/API2/ACMEPlugin.pm \
PMG/API2/NodeConfig.pm \
PMG/API2.pm \
+ PMG/Auth/Plugin.pm \
+ PMG/Auth/OIDC.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..b8c6cd9 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::OIDC;
+use PMG::Auth::PMG;
+
+PMG::Auth::OIDC->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/OIDC.pm b/src/PMG/Auth/OIDC.pm
new file mode 100755
index 0000000..3bb758b
--- /dev/null
+++ b/src/PMG/Auth/OIDC.pm
@@ -0,0 +1,99 @@
+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,
+ },
+ '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,
+ },
+ default => {
+ description => "Use this as default realm",
+ type => 'boolean',
+ optional => 1,
+ },
+ comment => {
+ description => "Description.",
+ type => 'string',
+ optional => 1,
+ maxLength => 4096,
+ },
+ };
+}
+
+sub options {
+ return {
+ 'issuer-url' => {},
+ 'client-id' => {},
+ 'client-key' => { optional => 1 },
+ autocreate => { 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 realm does not allow password verification.\n";
+}
+
+1;
diff --git a/src/PMG/Auth/PMG.pm b/src/PMG/Auth/PMG.pm
new file mode 100755
index 0000000..0eb136c
--- /dev/null
+++ b/src/PMG/Auth/PMG.pm
@@ -0,0 +1,28 @@
+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 {
+ 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..dc88aff
--- /dev/null
+++ b/src/PMG/Auth/Plugin.pm
@@ -0,0 +1,193 @@
+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/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};
+
+ 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 5d9ded4..844eb23 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.2
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [pmg-devel] [PATCH pmg-api v3 5/8] api: add/update/remove realms like in PVE
2024-06-24 9:08 [pmg-devel] [PATCH pve-common/proxmox-perl-rs/pmg-api/pmg-gui v3 0/8] fix #3892: OpenID Markus Frank
` (3 preceding siblings ...)
2024-06-24 9:08 ` [pmg-devel] [PATCH pmg-api v3 4/8] config: add plugin system for realms & add openid type realms Markus Frank
@ 2024-06-24 9:08 ` Markus Frank
2024-06-24 9:08 ` [pmg-devel] [PATCH pmg-api v3 6/8] api: openid login similar to PVE Markus Frank
` (3 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Markus Frank @ 2024-06-24 9:08 UTC (permalink / raw)
To: pmg-devel
The name Authdomains.pm was chosen because a Domain.pm already exists.
---
src/Makefile | 1 +
src/PMG/API2/AccessControl.pm | 10 +-
src/PMG/API2/Authdomains.pm | 274 ++++++++++++++++++++++++++++++++++
3 files changed, 284 insertions(+), 1 deletion(-)
create mode 100644 src/PMG/API2/Authdomains.pm
diff --git a/src/Makefile b/src/Makefile
index 4140698..111b931 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -149,6 +149,7 @@ LIBSOURCES = \
PMG/API2/Postfix.pm \
PMG/API2/Quarantine.pm \
PMG/API2/AccessControl.pm \
+ PMG/API2/Authdomains.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..dad679c 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::Authdomains;
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::Authdomains",
+ 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/Authdomains.pm b/src/PMG/API2/Authdomains.pm
new file mode 100644
index 0000000..43070b9
--- /dev/null
+++ b/src/PMG/API2/Authdomains.pm
@@ -0,0 +1,274 @@
+package PMG::API2::Authdomains;
+
+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 'pve');
+
+ die "unable to create builtin type '$type'\n"
+ if ($type eq 'pam' || $type eq 'pve');
+
+ 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.2
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [pmg-devel] [PATCH pmg-api v3 6/8] api: openid login similar to PVE
2024-06-24 9:08 [pmg-devel] [PATCH pve-common/proxmox-perl-rs/pmg-api/pmg-gui v3 0/8] fix #3892: OpenID Markus Frank
` (4 preceding siblings ...)
2024-06-24 9:08 ` [pmg-devel] [PATCH pmg-api v3 5/8] api: add/update/remove realms like in PVE Markus Frank
@ 2024-06-24 9:08 ` Markus Frank
2024-06-24 9:08 ` [pmg-devel] [PATCH pmg-gui v3 7/8] login: add OpenID realms Markus Frank
` (2 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Markus Frank @ 2024-06-24 9:08 UTC (permalink / raw)
To: pmg-devel
Allow OpenID Connect login using the Rust OpenID module.
---
v3: use Proxmox::RS:OpenId instead of PMG::RS::OpenId
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 111b931..4491aad 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -150,6 +150,7 @@ LIBSOURCES = \
PMG/API2/Quarantine.pm \
PMG/API2/AccessControl.pm \
PMG/API2/Authdomains.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 dad679c..6dda63f 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::Authdomains;
+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 => 'openid',
+});
+
__PACKAGE__->register_method ({
name => 'index',
path => '',
@@ -64,6 +70,7 @@ __PACKAGE__->register_method ({
my $res = [
{ subdir => 'ticket' },
{ subdir => 'domains' },
+ { subdir => 'openid' },
{ subdir => 'password' },
{ subdir => 'users' },
];
diff --git a/src/PMG/API2/OIDC.pm b/src/PMG/API2/OIDC.pm
new file mode 100644
index 0000000..539ce77
--- /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::OpenId;
+
+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 $openid_state_path = "/var/lib/pmg";
+
+my $lookup_openid_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 $openid_config = {
+ issuer_url => $config->{'issuer-url'},
+ client_id => $config->{'client-id'},
+ client_key => $config->{'client-key'},
+ };
+ $openid_config->{prompt} = $config->{'prompt'} if defined($config->{'prompt'});
+
+ my $scopes = $config->{'scopes'} // 'email profile';
+ $openid_config->{scopes} = [ PVE::Tools::split_list($scopes) ];
+
+ if (defined(my $acr = $config->{'acr-values'})) {
+ $openid_config->{acr_values} = [ PVE::Tools::split_list($acr) ];
+ }
+
+ my $openid = Proxmox::RS::OpenId->discover($openid_config, $redirect_url);
+ return ($config, $openid);
+};
+
+__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, $openid) = $lookup_openid_auth->($realm, $redirect_url);
+ my $url = $openid->authorize_url($openid_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::OpenId::verify_public_auth_state(
+ $openid_state_path, $param->{'state'});
+
+ my $redirect_url = extract_param($param, 'redirect-url');
+
+ my ($config, $openid) = $lookup_openid_auth->($realm, $redirect_url);
+
+ my $info = $openid->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} //= '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 b6c50d9..f043142 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/openid/login' && $method eq 'POST') ||
+ ($rel_uri eq '/access/openid/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.2
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [pmg-devel] [PATCH pmg-gui v3 7/8] login: add OpenID realms
2024-06-24 9:08 [pmg-devel] [PATCH pve-common/proxmox-perl-rs/pmg-api/pmg-gui v3 0/8] fix #3892: OpenID Markus Frank
` (5 preceding siblings ...)
2024-06-24 9:08 ` [pmg-devel] [PATCH pmg-api v3 6/8] api: openid login similar to PVE Markus Frank
@ 2024-06-24 9:08 ` Markus Frank
2024-06-24 9:08 ` [pmg-devel] [PATCH pmg-gui v3 8/8] add panel for realms to User Management Markus Frank
2024-10-09 11:30 ` [pmg-devel] [PATCH pve-common/proxmox-perl-rs/pmg-api/pmg-gui v3 0/8] fix #3892: OpenID Christoph Heiss
8 siblings, 0 replies; 15+ messages in thread
From: Markus Frank @ 2024-06-24 9:08 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 | 200 ++++++++++++++++++++++++++++++++++++------------
1 file changed, 153 insertions(+), 47 deletions(-)
diff --git a/js/LoginView.js b/js/LoginView.js
index b5da19a..87c013b 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',
@@ -45,51 +60,78 @@ 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');
- }
+ var me = this;
- // 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());
+ var loginForm = this.lookupReference('loginForm');
+ var unField = this.lookupReference('usernameField');
+ var saveunField = this.lookupReference('saveunField');
+ var view = this.getView();
+
+ 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') {
+ 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());
+ }
- let creds = loginForm.getValues();
+ let creds = loginForm.getValues();
- try {
- let resp = await Proxmox.Async.api2({
- url: '/api2/extjs/access/ticket',
- params: creds,
- method: 'POST',
- });
+ if (this.getViewModel().data.oidc === true) {
+ const redirectURL = location.origin;
+ Proxmox.Utils.API2Request({
+ url: '/api2/extjs/access/openid/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;
+ }
- 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'),
- );
+ 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'),
+ );
}
},
@@ -115,6 +157,15 @@ Ext.define('PMG.LoginView', {
return resp.result.data;
},
+ success: function(data) {
+ var me = this;
+ var view = me.getView();
+ var 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',
},
@@ -174,6 +233,41 @@ Ext.define('PMG.LoginView', {
var 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/openid/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.2
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [pmg-devel] [PATCH pmg-gui v3 8/8] add panel for realms to User Management
2024-06-24 9:08 [pmg-devel] [PATCH pve-common/proxmox-perl-rs/pmg-api/pmg-gui v3 0/8] fix #3892: OpenID Markus Frank
` (6 preceding siblings ...)
2024-06-24 9:08 ` [pmg-devel] [PATCH pmg-gui v3 7/8] login: add OpenID realms Markus Frank
@ 2024-06-24 9:08 ` Markus Frank
2024-10-09 11:30 ` [pmg-devel] [PATCH pve-common/proxmox-perl-rs/pmg-api/pmg-gui v3 0/8] fix #3892: OpenID Christoph Heiss
8 siblings, 0 replies; 15+ messages in thread
From: Markus Frank @ 2024-06-24 9:08 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 | 15 +++++++++++++++
2 files changed, 21 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 149abd6..8151035 100644
--- a/js/Utils.js
+++ b/js/Utils.js
@@ -851,6 +851,21 @@ Ext.define('PMG.Utils', {
constructor: function() {
var me = this;
+ // use oidc instead of openid
+ Proxmox.Schema.authDomains.oidc = Proxmox.Schema.authDomains.openid;
+ 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,
+ };
+
// do whatever you want here
Proxmox.Utils.override_task_descriptions({
applycustomscores: ['', gettext('Apply custom SpamAssassin scores')],
--
2.39.2
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [pmg-devel] [PATCH pve-common/proxmox-perl-rs/pmg-api/pmg-gui v3 0/8] fix #3892: OpenID
2024-06-24 9:08 [pmg-devel] [PATCH pve-common/proxmox-perl-rs/pmg-api/pmg-gui v3 0/8] fix #3892: OpenID Markus Frank
` (7 preceding siblings ...)
2024-06-24 9:08 ` [pmg-devel] [PATCH pmg-gui v3 8/8] add panel for realms to User Management Markus Frank
@ 2024-10-09 11:30 ` Christoph Heiss
2024-11-14 16:19 ` Markus Frank
8 siblings, 1 reply; 15+ messages in thread
From: Christoph Heiss @ 2024-10-09 11:30 UTC (permalink / raw)
To: Markus Frank; +Cc: pmg-devel
Just tested this series using Keycloak 26.0.0 as an OpenID provider.
Everything worked fine once it was set up for the OpenID side! Login via
OpenID worked, also tested the "Autocreate Users" feature (but see below
on that). The GUI dialog panel is from proxmox-widget-toolkit, so
nothing really new there.
I noticed however that there seems to be no dedicated PAM realm in the
login window, only PMG authentication server - but you can still login
with PAM credentials. These two should be real separate realms, much
like we have it for PVE/PBS.
Also, when using the "Autocreate Users" feature - should the (PMG) role
assigned to the user maybe be configurable? Since it currently just
defaults to Auditor, as it seems. (or am I missing something?)
Lastly, patches #2 and #3 need to be rebased on the latest master
of proxmox-perl-rs, they failed to apply (resolved that manually myself
to test them out for now). All other patches in this series apply on
their respective master cleanly.
Didn't really have a look at the code yet, so might do some reviews on
that too.
On Mon, Jun 24, 2024 at 11:08:42AM GMT, Markus Frank wrote:
> Patch-series to enable OpenID Login for PMG
>
> apply/compile order:
>
> 1. pve-common: add Schema package with auth module that contains realm sync options
> 2. proxmox-perl-rs: move openid code from pve-rs to common
> 3. proxmox-perl-rs: remove empty PMG::RS::OpenId package to avoid confusion
> 4. pmg-api: config: add plugin system for realms & add openid type realms
> 5. pmg-api: api: add/update/remove realms like in PVE
> 6. pmg-api: api: openid login similar to PVE
> 7. pmg-gui: login: add option to login with OpenID realm
> 8. pmg-gui: add panel for realms to User Management
>
>
> v3 changed only in proxmox-perl-rs and "pmg-api: api: openid login similar to PVE"
>
>
> 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:
>
> v3: removed PMG wrapper as Proxmox::RS:OpenId can be used instead.
>
> 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/openid/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/openid/mod.rs
>
>
> pmg-api:
>
> v3: use Proxmox::RS:OpenId instead of PMG::RS::OpenId
>
> Markus Frank (3):
> config: add plugin system for realms & add openid type realms
> api: add/update/remove realms like in PVE
> api: openid login similar to PVE
>
> src/Makefile | 5 +
> src/PMG/API2/AccessControl.pm | 17 ++-
> src/PMG/API2/Authdomains.pm | 274 ++++++++++++++++++++++++++++++++++
> src/PMG/API2/OIDC.pm | 243 ++++++++++++++++++++++++++++++
> src/PMG/AccessControl.pm | 31 ++++
> src/PMG/Auth/OIDC.pm | 99 ++++++++++++
> src/PMG/Auth/PMG.pm | 28 ++++
> src/PMG/Auth/Plugin.pm | 193 ++++++++++++++++++++++++
> src/PMG/HTTPServer.pm | 2 +
> src/PMG/RESTEnvironment.pm | 14 ++
> src/PMG/UserConfig.pm | 25 ++--
> src/PMG/Utils.pm | 29 +++-
> 12 files changed, 943 insertions(+), 17 deletions(-)
> create mode 100644 src/PMG/API2/Authdomains.pm
> create mode 100644 src/PMG/API2/OIDC.pm
> create mode 100755 src/PMG/Auth/OIDC.pm
> create mode 100755 src/PMG/Auth/PMG.pm
> create mode 100755 src/PMG/Auth/Plugin.pm
>
>
> pmg-gui:
>
> Markus Frank (2):
> login: add OpenID realms
> add panel for realms to User Management
>
> js/LoginView.js | 200 +++++++++++++++++++++++++++++++++----------
> js/UserManagement.js | 6 ++
> js/Utils.js | 15 ++++
> 3 files changed, 174 insertions(+), 47 deletions(-)
>
> --
> 2.39.2
>
>
>
> _______________________________________________
> 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] 15+ messages in thread
* Re: [pmg-devel] [PATCH proxmox-perl-rs v3 2/8] move openid code from pve-rs to common
2024-06-24 9:08 ` [pmg-devel] [PATCH proxmox-perl-rs v3 2/8] move openid code from pve-rs to common Markus Frank
@ 2024-10-09 11:30 ` Christoph Heiss
0 siblings, 0 replies; 15+ messages in thread
From: Christoph Heiss @ 2024-10-09 11:30 UTC (permalink / raw)
To: Markus Frank; +Cc: pmg-devel
Patch unfortunately does not apply anymore, so needs to be rebased, as
well as the next/other one for proxmox-perl-rs.
On Mon, Jun 24, 2024 at 11:08:44AM GMT, Markus Frank wrote:
> 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/openid/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/openid/mod.rs
>
> diff --git a/common/pkg/Makefile b/common/pkg/Makefile
> index 78f2d64..a31264f 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::OpenId \
> Proxmox::RS::Subscription
>
> PERLMOD_PACKAGE_FILES := $(addsuffix .pm,$(subst ::,/,$(PERLMOD_PACKAGES)))
> diff --git a/common/src/mod.rs b/common/src/mod.rs
> index c3574f4..8c22a46 100644
> --- a/common/src/mod.rs
> +++ b/common/src/mod.rs
> @@ -2,4 +2,5 @@ pub mod apt;
> mod calendar_event;
> pub mod logger;
> pub mod notify;
> +pub mod openid;
> mod subscription;
> diff --git a/common/src/openid/mod.rs b/common/src/openid/mod.rs
> new file mode 100644
> index 0000000..13bbaab
> --- /dev/null
> +++ b/common/src/openid/mod.rs
> @@ -0,0 +1,63 @@
> +#[perlmod::package(name = "Proxmox::RS::OpenId")]
> +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<OpenId> : &OpenId as "Proxmox::RS::OpenId");
> +
> + /// An OpenIdAuthenticator client instance.
> + pub struct OpenId {
> + inner: Mutex<OpenIdAuthenticator>,
> + }
> +
> + /// Create a new OpenId client instance
> + #[export(raw_return)]
> + pub fn discover(
> + #[raw] class: Value,
> + 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),
> + })
> + ))
> + }
> +
> + #[export]
> + pub fn authorize_url(
> + #[try_from_ref] this: &OpenId,
> + state_dir: &str,
> + realm: &str,
> + ) -> Result<String, Error> {
> + let open_id = this.inner.lock().unwrap();
> + open_id.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: &OpenId,
> + 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)?)
> + }
> +}
> diff --git a/pmg-rs/Cargo.toml b/pmg-rs/Cargo.toml
> index 6557605..4b9568f 100644
> --- a/pmg-rs/Cargo.toml
> +++ b/pmg-rs/Cargo.toml
> @@ -41,3 +41,4 @@ proxmox-subscription = "0.4"
> proxmox-sys = "0.5"
> proxmox-tfa = { version = "4.0.4", features = ["api"] }
> proxmox-time = "2"
> +proxmox-openid = "0.10.0"
> diff --git a/pmg-rs/debian/control b/pmg-rs/debian/control
> index 5a63b5d..dcc32eb 100644
> --- a/pmg-rs/debian/control
> +++ b/pmg-rs/debian/control
> @@ -24,6 +24,7 @@ Build-Depends: cargo:native <!nocheck>,
> librust-proxmox-http-error-0.1+default-dev,
> librust-proxmox-notify-0.4+default-dev,
> librust-proxmox-subscription-0.4+default-dev,
> + librust-proxmox-openid-0.10+default-dev,
> librust-proxmox-sys-0.5+default-dev,
> librust-proxmox-tfa-4+api-dev (>= 4.0.4-~~),
> librust-proxmox-tfa-4+default-dev (>= 4.0.4-~~),
> diff --git a/pve-rs/src/openid/mod.rs b/pve-rs/src/openid/mod.rs
> index 1fa7572..d3ad5a5 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::openid::export as common;
> + use crate::common::openid::export::OpenId 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.2
>
>
>
> _______________________________________________
> 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] 15+ messages in thread
* Re: [pmg-devel] [PATCH pmg-api v3 4/8] config: add plugin system for realms & add openid type realms
2024-06-24 9:08 ` [pmg-devel] [PATCH pmg-api v3 4/8] config: add plugin system for realms & add openid type realms Markus Frank
@ 2024-10-10 8:46 ` Christoph Heiss
2024-10-18 12:07 ` Christoph Heiss
1 sibling, 0 replies; 15+ messages in thread
From: Christoph Heiss @ 2024-10-10 8:46 UTC (permalink / raw)
To: Markus Frank; +Cc: pmg-devel
On Mon, Jun 24, 2024 at 11:08:46AM GMT, Markus Frank wrote:
[..]
> diff --git a/src/PMG/Auth/OIDC.pm b/src/PMG/Auth/OIDC.pm
> new file mode 100755
> index 0000000..3bb758b
> --- /dev/null
> +++ b/src/PMG/Auth/OIDC.pm
> @@ -0,0 +1,99 @@
> +package PMG::Auth::OIDC;
From the looks of it, this module is basically just a 1:1 copy of
pve-access-control/src/PVE/Auth/OpenId.pm, right?
Would it make sense to re-use that instead of duplicating it? Or are
there any differences that would make it rather cumbersome?
Also FWIW w.r.t the naming, you seem to switch between "OIDC" and
"OpenId" completely random. Everywhere else (i.e. PVE, PBS) we just call
it "OpenID" (or "OpenId" for modules/structs). Sticking to one naming
scheme for consistency sake might be good.
> +
> +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,
> + },
> + '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,
> + },
> + default => {
> + description => "Use this as default realm",
> + type => 'boolean',
> + optional => 1,
> + },
> + comment => {
> + description => "Description.",
> + type => 'string',
> + optional => 1,
> + maxLength => 4096,
> + },
> + };
> +}
> +
> +sub options {
> + return {
> + 'issuer-url' => {},
> + 'client-id' => {},
> + 'client-key' => { optional => 1 },
> + autocreate => { 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 realm does not allow password verification.\n";
> +}
> +
> +1;
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [pmg-devel] [PATCH pmg-api v3 4/8] config: add plugin system for realms & add openid type realms
2024-06-24 9:08 ` [pmg-devel] [PATCH pmg-api v3 4/8] config: add plugin system for realms & add openid type realms Markus Frank
2024-10-10 8:46 ` Christoph Heiss
@ 2024-10-18 12:07 ` Christoph Heiss
1 sibling, 0 replies; 15+ messages in thread
From: Christoph Heiss @ 2024-10-18 12:07 UTC (permalink / raw)
To: Markus Frank; +Cc: pmg-devel
This patch should probably be split into two, one adding the actual
plugin system and the second one adding the openid realm definitions -
even the patch subject suggests that it does two completely different
things.
Would also make things a bit clearer.
On Mon, Jun 24, 2024 at 11:08:46AM GMT, Markus Frank wrote:
[..]
> diff --git a/src/PMG/Auth/Plugin.pm b/src/PMG/Auth/Plugin.pm
> new file mode 100755
> index 0000000..dc88aff
> --- /dev/null
> +++ b/src/PMG/Auth/Plugin.pm
> @@ -0,0 +1,193 @@
> +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/realms.lck";
Should be /var/lock/pmg-realms.lck, to make it clear that it belongs to
PMG - in line with all the other lockfiles PMG creates/uses.
> +
> +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);
^
Unnecessary whitespace
> +}
> +
> +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 {
Can be a `my sub`, since it's not used anywhere else AFAICS, right?
> + 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};
As noted in the cover letter, there should be separate PAM and PMG
realms, much like PVE/PBS.
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [pmg-devel] [PATCH pve-common/proxmox-perl-rs/pmg-api/pmg-gui v3 0/8] fix #3892: OpenID
2024-10-09 11:30 ` [pmg-devel] [PATCH pve-common/proxmox-perl-rs/pmg-api/pmg-gui v3 0/8] fix #3892: OpenID Christoph Heiss
@ 2024-11-14 16:19 ` Markus Frank
2024-11-22 9:12 ` Christoph Heiss
0 siblings, 1 reply; 15+ messages in thread
From: Markus Frank @ 2024-11-14 16:19 UTC (permalink / raw)
To: Christoph Heiss; +Cc: pmg-devel
Thanks for the review and sorry for the late reply.
Comments inline:
On 2024-10-09 13:30, Christoph Heiss wrote:
> Just tested this series using Keycloak 26.0.0 as an OpenID provider.
>
> Everything worked fine once it was set up for the OpenID side! Login via
> OpenID worked, also tested the "Autocreate Users" feature (but see below
> on that). The GUI dialog panel is from proxmox-widget-toolkit, so
> nothing really new there.
>
> I noticed however that there seems to be no dedicated PAM realm in the
> login window, only PMG authentication server - but you can still login
> with PAM credentials. These two should be real separate realms, much
> like we have it for PVE/PBS.
But you can only login as root with PAM afaict.
Should we separate it just for the root user or are we planning to add PAM login for other users?
>
> Also, when using the "Autocreate Users" feature - should the (PMG) role
> assigned to the user maybe be configurable? Since it currently just
> defaults to Auditor, as it seems. (or am I missing something?)
Okay, that sounds reasonable. I will add such an option.
>
> Lastly, patches #2 and #3 need to be rebased on the latest master
> of proxmox-perl-rs, they failed to apply (resolved that manually myself
> to test them out for now). All other patches in this series apply on
> their respective master cleanly.
>
> Didn't really have a look at the code yet, so might do some reviews on
> that too.
>
> On Mon, Jun 24, 2024 at 11:08:42AM GMT, Markus Frank wrote:
>> Patch-series to enable OpenID Login for PMG
>>
>> apply/compile order:
>>
>> 1. pve-common: add Schema package with auth module that contains realm sync options
>> 2. proxmox-perl-rs: move openid code from pve-rs to common
>> 3. proxmox-perl-rs: remove empty PMG::RS::OpenId package to avoid confusion
>> 4. pmg-api: config: add plugin system for realms & add openid type realms
>> 5. pmg-api: api: add/update/remove realms like in PVE
>> 6. pmg-api: api: openid login similar to PVE
>> 7. pmg-gui: login: add option to login with OpenID realm
>> 8. pmg-gui: add panel for realms to User Management
>>
>>
>> v3 changed only in proxmox-perl-rs and "pmg-api: api: openid login similar to PVE"
>>
>>
>> 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:
>>
>> v3: removed PMG wrapper as Proxmox::RS:OpenId can be used instead.
>>
>> 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/openid/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/openid/mod.rs
>>
>>
>> pmg-api:
>>
>> v3: use Proxmox::RS:OpenId instead of PMG::RS::OpenId
>>
>> Markus Frank (3):
>> config: add plugin system for realms & add openid type realms
>> api: add/update/remove realms like in PVE
>> api: openid login similar to PVE
>>
>> src/Makefile | 5 +
>> src/PMG/API2/AccessControl.pm | 17 ++-
>> src/PMG/API2/Authdomains.pm | 274 ++++++++++++++++++++++++++++++++++
>> src/PMG/API2/OIDC.pm | 243 ++++++++++++++++++++++++++++++
>> src/PMG/AccessControl.pm | 31 ++++
>> src/PMG/Auth/OIDC.pm | 99 ++++++++++++
>> src/PMG/Auth/PMG.pm | 28 ++++
>> src/PMG/Auth/Plugin.pm | 193 ++++++++++++++++++++++++
>> src/PMG/HTTPServer.pm | 2 +
>> src/PMG/RESTEnvironment.pm | 14 ++
>> src/PMG/UserConfig.pm | 25 ++--
>> src/PMG/Utils.pm | 29 +++-
>> 12 files changed, 943 insertions(+), 17 deletions(-)
>> create mode 100644 src/PMG/API2/Authdomains.pm
>> create mode 100644 src/PMG/API2/OIDC.pm
>> create mode 100755 src/PMG/Auth/OIDC.pm
>> create mode 100755 src/PMG/Auth/PMG.pm
>> create mode 100755 src/PMG/Auth/Plugin.pm
>>
>>
>> pmg-gui:
>>
>> Markus Frank (2):
>> login: add OpenID realms
>> add panel for realms to User Management
>>
>> js/LoginView.js | 200 +++++++++++++++++++++++++++++++++----------
>> js/UserManagement.js | 6 ++
>> js/Utils.js | 15 ++++
>> 3 files changed, 174 insertions(+), 47 deletions(-)
>>
>> --
>> 2.39.2
>>
>>
>>
>> _______________________________________________
>> 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] 15+ messages in thread
* Re: [pmg-devel] [PATCH pve-common/proxmox-perl-rs/pmg-api/pmg-gui v3 0/8] fix #3892: OpenID
2024-11-14 16:19 ` Markus Frank
@ 2024-11-22 9:12 ` Christoph Heiss
0 siblings, 0 replies; 15+ messages in thread
From: Christoph Heiss @ 2024-11-22 9:12 UTC (permalink / raw)
To: Markus Frank; +Cc: pmg-devel
On Thu, Nov 14, 2024 at 05:19:38PM +0100, Markus Frank wrote:
> Thanks for the review and sorry for the late reply.
>
> Comments inline:
>
> On 2024-10-09 13:30, Christoph Heiss wrote:
> > Just tested this series using Keycloak 26.0.0 as an OpenID provider.
> > [..]
> >
> > I noticed however that there seems to be no dedicated PAM realm in the
> > login window, only PMG authentication server - but you can still login
> > with PAM credentials. These two should be real separate realms, much
> > like we have it for PVE/PBS.
> But you can only login as root with PAM afaict.
> Should we separate it just for the root user or are we planning to add PAM login for other users?
Hm, not sure - or at least not for me to decide.
But - it was a bit surprising/confusing, since you can set PMG as
authentication realm and then use root (at) pam as username. Especially
also when comparing to PVE/PBS, how it works there.
I guess just for the sake of consistency between products would be worth
it to split them. Although user creation/management for such a PAM realm
can be left for later, as to not explode this series.
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2024-11-22 9:12 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-24 9:08 [pmg-devel] [PATCH pve-common/proxmox-perl-rs/pmg-api/pmg-gui v3 0/8] fix #3892: OpenID Markus Frank
2024-06-24 9:08 ` [pmg-devel] [PATCH pve-common v3 1/8] add Schema package with auth module that contains realm sync options Markus Frank
2024-06-24 9:08 ` [pmg-devel] [PATCH proxmox-perl-rs v3 2/8] move openid code from pve-rs to common Markus Frank
2024-10-09 11:30 ` Christoph Heiss
2024-06-24 9:08 ` [pmg-devel] [PATCH proxmox-perl-rs v3 3/8] remove empty PMG::RS::OpenId package to avoid confusion Markus Frank
2024-06-24 9:08 ` [pmg-devel] [PATCH pmg-api v3 4/8] config: add plugin system for realms & add openid type realms Markus Frank
2024-10-10 8:46 ` Christoph Heiss
2024-10-18 12:07 ` Christoph Heiss
2024-06-24 9:08 ` [pmg-devel] [PATCH pmg-api v3 5/8] api: add/update/remove realms like in PVE Markus Frank
2024-06-24 9:08 ` [pmg-devel] [PATCH pmg-api v3 6/8] api: openid login similar to PVE Markus Frank
2024-06-24 9:08 ` [pmg-devel] [PATCH pmg-gui v3 7/8] login: add OpenID realms Markus Frank
2024-06-24 9:08 ` [pmg-devel] [PATCH pmg-gui v3 8/8] add panel for realms to User Management Markus Frank
2024-10-09 11:30 ` [pmg-devel] [PATCH pve-common/proxmox-perl-rs/pmg-api/pmg-gui v3 0/8] fix #3892: OpenID Christoph Heiss
2024-11-14 16:19 ` Markus Frank
2024-11-22 9:12 ` Christoph Heiss
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox