* [pmg-devel] [PATCH proxmox-perl-rs 1/6] move openid code from pve-rs to common
2024-04-02 11:27 [pmg-devel] [PATCH proxmox-perl-rs/pmg-api/pmg-gui 0/6] fix #3892: OpenID Markus Frank
@ 2024-04-02 11:27 ` Markus Frank
2024-04-02 11:27 ` [pmg-devel] [PATCH pmg-api 2/6] config: add plugin system for realms & add openid type realms Markus Frank
` (4 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Markus Frank @ 2024-04-02 11:27 UTC (permalink / raw)
To: pmg-devel
Change pve-rs functions to be wrapper functions for common
and add similar wrapper functions for pmg-rs.
Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
common/src/mod.rs | 1 +
common/src/openid/mod.rs | 63 ++++++++++++++++++++++++++++++++++++++++
pmg-rs/Cargo.toml | 1 +
pmg-rs/src/lib.rs | 1 +
pmg-rs/src/openid/mod.rs | 47 ++++++++++++++++++++++++++++++
pve-rs/src/openid/mod.rs | 32 +++++---------------
6 files changed, 121 insertions(+), 24 deletions(-)
create mode 100644 common/src/openid/mod.rs
create mode 100644 pmg-rs/src/openid/mod.rs
diff --git a/common/src/mod.rs b/common/src/mod.rs
index c3574f4..8460439 100644
--- a/common/src/mod.rs
+++ b/common/src/mod.rs
@@ -3,3 +3,4 @@ mod calendar_event;
pub mod logger;
pub mod notify;
mod subscription;
+pub mod openid;
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 0d01b59..6f3e3df 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 = "1.1.3"
+proxmox-openid = "0.10.0"
diff --git a/pmg-rs/src/lib.rs b/pmg-rs/src/lib.rs
index 4a91632..1930423 100644
--- a/pmg-rs/src/lib.rs
+++ b/pmg-rs/src/lib.rs
@@ -5,6 +5,7 @@ pub mod acme;
pub mod apt;
pub mod csr;
pub mod tfa;
+pub mod openid;
#[perlmod::package(name = "Proxmox::Lib::PMG", lib = "pmg_rs")]
mod export {
diff --git a/pmg-rs/src/openid/mod.rs b/pmg-rs/src/openid/mod.rs
new file mode 100644
index 0000000..c0988d6
--- /dev/null
+++ b/pmg-rs/src/openid/mod.rs
@@ -0,0 +1,47 @@
+#[perlmod::package(name = "PMG::RS::OpenId", lib = "pmg_rs")]
+mod export {
+ use anyhow::Error;
+
+ use perlmod::Value;
+
+ use proxmox_openid::{OpenIdConfig, PrivateAuthState};
+
+ use crate::common::openid::export as common;
+ use crate::common::openid::export::OpenId as OpenId;
+
+ /// Create a new OpenId client instance
+ #[export(raw_return)]
+ pub fn discover(
+ #[raw] class: Value,
+ config: OpenIdConfig,
+ redirect_url: &str,
+ ) -> Result<Value, Error> {
+ common::discover(class, config, redirect_url)
+ }
+
+ #[export]
+ pub fn authorize_url(
+ #[try_from_ref] this: &OpenId,
+ state_dir: &str,
+ realm: &str,
+ ) -> Result<String, Error> {
+ common::authorize_url(this, state_dir, realm)
+ }
+
+ #[export]
+ pub fn verify_public_auth_state(
+ state_dir: &str,
+ state: &str,
+ ) -> Result<(String, PrivateAuthState), Error> {
+ common::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> {
+ common::verify_authorization_code(this, code, private_auth_state)
+ }
+}
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
^ permalink raw reply [flat|nested] 8+ messages in thread* [pmg-devel] [PATCH pmg-api 2/6] config: add plugin system for realms & add openid type realms
2024-04-02 11:27 [pmg-devel] [PATCH proxmox-perl-rs/pmg-api/pmg-gui 0/6] fix #3892: OpenID Markus Frank
2024-04-02 11:27 ` [pmg-devel] [PATCH proxmox-perl-rs 1/6] move openid code from pve-rs to common Markus Frank
@ 2024-04-02 11:27 ` Markus Frank
2024-04-04 11:57 ` Thomas Lamprecht
2024-04-02 11:27 ` [pmg-devel] [PATCH pmg-api 3/6] api: add/update/remove realms like in PVE Markus Frank
` (3 subsequent siblings)
5 siblings, 1 reply; 8+ messages in thread
From: Markus Frank @ 2024-04-02 11:27 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.
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 | 33 +++++
src/PMG/Auth/OpenId.pm | 99 ++++++++++++++
src/PMG/Auth/PMG.pm | 28 ++++
src/PMG/Auth/Plugin.pm | 269 +++++++++++++++++++++++++++++++++++++
src/PMG/RESTEnvironment.pm | 14 ++
src/PMG/UserConfig.pm | 26 ++--
src/PMG/Utils.pm | 24 +++-
8 files changed, 482 insertions(+), 14 deletions(-)
create mode 100755 src/PMG/Auth/OpenId.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..407cc4a 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/OpenId.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..0ab4dfa 100644
--- a/src/PMG/AccessControl.pm
+++ b/src/PMG/AccessControl.pm
@@ -7,12 +7,21 @@ use Authen::PAM;
use PVE::Tools;
use PVE::JSONSchema qw(get_standard_option);
use PVE::Exception qw(raise raise_perm_exc);
+use PVE::Tools qw(lock_file);
use PMG::UserConfig;
use PMG::LDAPConfig;
use PMG::LDAPSet;
use PMG::TFAConfig;
+use PMG::Auth::Plugin;
+use PMG::Auth::OpenId;
+use PMG::Auth::PMG;
+
+PMG::Auth::OpenId->register();
+PMG::Auth::PMG->register();
+PMG::Auth::Plugin->init();
+
sub normalize_path {
my $path = shift;
@@ -38,6 +47,8 @@ sub authenticate_user : prototype($$$) {
($username, $ruid, $realm) = PMG::Utils::verify_username($username);
+ my $valid_pmg_realms = PMG::Utils::valid_pmg_realms();
+ my $realm_list = join('|', @$valid_pmg_realms);
if ($realm eq 'pam') {
die "invalid pam user (only root allowed)\n" if $ruid ne 'root';
authenticate_pam_user($ruid, $password);
@@ -53,6 +64,7 @@ sub authenticate_user : prototype($$$) {
return ($pmail . '@quarantine', undef);
}
die "ldap login failed\n";
+ } elsif ($realm =~ m!(${realm_list})!) {
} else {
die "no such realm '$realm'\n";
}
@@ -79,6 +91,8 @@ sub set_user_password {
($username, $ruid, $realm) = PMG::Utils::verify_username($username);
+ my $valid_pmg_realms = PMG::Utils::valid_pmg_realms();
+ my $realm_list = join('|', @$valid_pmg_realms);
if ($realm eq 'pam') {
die "invalid pam user (only root allowed)\n" if $ruid ne 'root';
@@ -92,6 +106,7 @@ sub set_user_password {
} elsif ($realm eq 'pmg') {
PMG::UserConfig->set_user_password($username, $password);
+ } elsif ($realm =~ m!(${realm_list})!) {
} else {
die "no such realm '$realm'\n";
}
@@ -106,6 +121,8 @@ sub check_user_enabled {
($username, $ruid, $realm) = PMG::Utils::verify_username($username, 1);
+ my $valid_pmg_realms = PMG::Utils::valid_pmg_realms();
+ my $realm_list = join('|', @$valid_pmg_realms);
if ($realm && $ruid) {
if ($realm eq 'pam') {
return 'root' if $ruid eq 'root';
@@ -115,6 +132,10 @@ sub check_user_enabled {
return $data->{role} if $data && $data->{enable};
} elsif ($realm eq 'quarantine') {
return 'quser';
+ } elsif ($realm =~ m!(${realm_list})!) {
+ my $usercfg = PMG::UserConfig->new();
+ my $data = $usercfg->lookup_user_data($username, $noerr);
+ return $data->{role} if $data && $data->{enable};
}
}
@@ -123,6 +144,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/OpenId.pm b/src/PMG/Auth/OpenId.pm
new file mode 100755
index 0000000..44fe0fe
--- /dev/null
+++ b/src/PMG/Auth/OpenId.pm
@@ -0,0 +1,99 @@
+package PMG::Auth::OpenId;
+
+use strict;
+use warnings;
+
+use PVE::Tools;
+use PMG::Auth::Plugin;
+
+use base qw(PMG::Auth::Plugin);
+
+sub type {
+ return 'openid';
+}
+
+sub properties {
+ return {
+ "issuer-url" => {
+ description => "OpenID Issuer Url",
+ type => 'string',
+ maxLength => 256,
+ },
+ "client-id" => {
+ description => "OpenID Client ID",
+ type => 'string',
+ maxLength => 256,
+ },
+ "client-key" => {
+ description => "OpenID 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 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..a7ea596
--- /dev/null
+++ b/src/PMG/Auth/Plugin.pm
@@ -0,0 +1,269 @@
+package PMG::Auth::Plugin;
+
+use strict;
+use warnings;
+
+use Digest::SHA;
+use Encode;
+
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::SectionConfig;
+use PVE::Tools;
+use PVE::INotify;
+use PMG::Utils;
+
+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);
+ my $err = $@;
+ if ($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 $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('pmg-tfa-config', $tfa_format);
+
+PVE::JSONSchema::register_standard_option('tfa', {
+ description => "Use Two-factor authentication.",
+ type => 'string', format => 'pmg-tfa-config',
+ optional => 1,
+ maxLength => 128,
+});
+
+sub parse_tfa_config {
+ my ($data) = @_;
+
+ return PVE::JSONSchema::parse_property_string($tfa_format, $data);
+}
+
+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..8fb93c9 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,24 @@ sub write_user_conf {
$verify_entry->($d);
$cfg->{$d->{userid}} = $d;
+ my $valid_pmg_realms = PMG::Utils::valid_pmg_realms();
+ my $realm_list = join('|', @$valid_pmg_realms);
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_list})!;
}
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_list})$/;
+ $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..e173894 100644
--- a/src/PMG/Utils.pm
+++ b/src/PMG/Utils.pm
@@ -49,12 +49,27 @@ postgres_admin_cmd
try_decode_utf8
);
-my $valid_pmg_realms = ['pam', 'pmg', 'quarantine'];
+my $user_regex = qr![^\s:/]+!;
+
+sub valid_pmg_realms {
+ my $cfg = PVE::INotify::read_file('realms.cfg');
+ my $ids = $cfg->{ids};
+ my $realms = [];
+ my @list = keys %{$ids};
+ for my $realm (@list) {
+ push @$realms, $realm;
+ }
+ push @$realms, 'pam';
+ push @$realms, 'quarantine';
+ return $realms;
+}
+
+PVE::JSONSchema::register_format('pmg-realm', \&valid_pmg_realms);
PVE::JSONSchema::register_standard_option('realm', {
description => "Authentication domain ID",
type => 'string',
- enum => $valid_pmg_realms,
+ format => 'pmg-realm',
maxLength => 32,
});
@@ -82,7 +97,7 @@ sub verify_username {
die "user name '$username' is too short\n" if !$noerr;
return undef;
}
- if ($len > 64) {
+ if ($len > 128) {
die "user name '$username' is too long ($len > 64)\n" if !$noerr;
return undef;
}
@@ -90,8 +105,9 @@ sub verify_username {
# 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 $valid_pmg_realms = valid_pmg_realms();
my $realm_list = join('|', @$valid_pmg_realms);
- if ($username =~ m!^([^\s:/]+)\@(${realm_list})$!) {
+ if ($username =~ m!^(${user_regex})\@(${realm_list})$!) {
return wantarray ? ($username, $1, $2) : $username;
}
--
2.39.2
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [pmg-devel] [PATCH pmg-api 2/6] config: add plugin system for realms & add openid type realms
2024-04-02 11:27 ` [pmg-devel] [PATCH pmg-api 2/6] config: add plugin system for realms & add openid type realms Markus Frank
@ 2024-04-04 11:57 ` Thomas Lamprecht
0 siblings, 0 replies; 8+ messages in thread
From: Thomas Lamprecht @ 2024-04-04 11:57 UTC (permalink / raw)
To: Markus Frank, pmg-devel
Am 02/04/2024 um 13:27 schrieb Markus Frank:
> 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.
>
> Utils generates a list of valid realm names, including any newly added realms,
> to ensure proper validation of a specified realm name.
you might also talk a bit here about that this is adapted over from PVE,
how/why the static/limited realms worked earlier in PMG, how that is changed
and all that, having some background and reference can only help
>
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> ---
> src/Makefile | 3 +
> src/PMG/AccessControl.pm | 33 +++++
> src/PMG/Auth/OpenId.pm | 99 ++++++++++++++
> src/PMG/Auth/PMG.pm | 28 ++++
> src/PMG/Auth/Plugin.pm | 269 +++++++++++++++++++++++++++++++++++++
> src/PMG/RESTEnvironment.pm | 14 ++
> src/PMG/UserConfig.pm | 26 ++--
> src/PMG/Utils.pm | 24 +++-
> 8 files changed, 482 insertions(+), 14 deletions(-)
> create mode 100755 src/PMG/Auth/OpenId.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..407cc4a 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/OpenId.pm \
> + PMG/Auth/PMG.pm
nit: missing trailing backslash (so any new addition doesn't need to touch
existing lines)
>
> 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..0ab4dfa 100644
> --- a/src/PMG/AccessControl.pm
> +++ b/src/PMG/AccessControl.pm
> @@ -7,12 +7,21 @@ use Authen::PAM;
> use PVE::Tools;
> use PVE::JSONSchema qw(get_standard_option);
> use PVE::Exception qw(raise raise_perm_exc);
> +use PVE::Tools qw(lock_file);
Tools is already used three lines above this, add the import of lock_file there.
>
> use PMG::UserConfig;
> use PMG::LDAPConfig;
> use PMG::LDAPSet;
> use PMG::TFAConfig;
>
> +use PMG::Auth::Plugin;
> +use PMG::Auth::OpenId;
> +use PMG::Auth::PMG;
> +
> +PMG::Auth::OpenId->register();
> +PMG::Auth::PMG->register();
> +PMG::Auth::Plugin->init();
> +
> sub normalize_path {
> my $path = shift;
>
> @@ -38,6 +47,8 @@ sub authenticate_user : prototype($$$) {
>
> ($username, $ruid, $realm) = PMG::Utils::verify_username($username);
>
> + my $valid_pmg_realms = PMG::Utils::valid_pmg_realms();
> + my $realm_list = join('|', @$valid_pmg_realms);
> if ($realm eq 'pam') {
> die "invalid pam user (only root allowed)\n" if $ruid ne 'root';
> authenticate_pam_user($ruid, $password);
> @@ -53,6 +64,7 @@ sub authenticate_user : prototype($$$) {
> return ($pmail . '@quarantine', undef);
> }
> die "ldap login failed\n";
> + } elsif ($realm =~ m!(${realm_list})!) {
stumbling upon empty branches lets one always wonder what it's about, so
adding some comment here stating the reason this is done would be good.
> } else {
> die "no such realm '$realm'\n";
> }
> @@ -79,6 +91,8 @@ sub set_user_password {
>
> ($username, $ruid, $realm) = PMG::Utils::verify_username($username);
>
> + my $valid_pmg_realms = PMG::Utils::valid_pmg_realms();
> + my $realm_list = join('|', @$valid_pmg_realms);
> if ($realm eq 'pam') {
> die "invalid pam user (only root allowed)\n" if $ruid ne 'root';
>
> @@ -92,6 +106,7 @@ sub set_user_password {
>
> } elsif ($realm eq 'pmg') {
> PMG::UserConfig->set_user_password($username, $password);
> + } elsif ($realm =~ m!(${realm_list})!) {
same here
> } else {
> die "no such realm '$realm'\n";
> }
> @@ -106,6 +121,8 @@ sub check_user_enabled {
>
> ($username, $ruid, $realm) = PMG::Utils::verify_username($username, 1);
>
> + my $valid_pmg_realms = PMG::Utils::valid_pmg_realms();
> + my $realm_list = join('|', @$valid_pmg_realms);
> if ($realm && $ruid) {
> if ($realm eq 'pam') {
> return 'root' if $ruid eq 'root';
> @@ -115,6 +132,10 @@ sub check_user_enabled {
> return $data->{role} if $data && $data->{enable};
> } elsif ($realm eq 'quarantine') {
> return 'quser';
> + } elsif ($realm =~ m!(${realm_list})!) {
> + my $usercfg = PMG::UserConfig->new();
> + my $data = $usercfg->lookup_user_data($username, $noerr);
> + return $data->{role} if $data && $data->{enable};
> }
> }
>
> @@ -123,6 +144,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/OpenId.pm b/src/PMG/Auth/OpenId.pm
> new file mode 100755
> index 0000000..44fe0fe
> --- /dev/null
> +++ b/src/PMG/Auth/OpenId.pm
IIRC you took this from PVE, but maybe we should name this OIDC.pm (or, for
verbosity, OpenIDConnect.pm) here, as it's not really the (rather broken) OpenID,
but the successor OpenID Connect. Having this distinction encoded in the
filename would not hurt and might potentially soothe some paranoid admins
that had to much contact with security stuff.
> @@ -0,0 +1,99 @@
> +package PMG::Auth::OpenId;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::Tools;
> +use PMG::Auth::Plugin;
> +
> +use base qw(PMG::Auth::Plugin);
> +
> +sub type {
> + return 'openid';
same her w.r.t "OpenID" vs "OpenID Connect" (or it's abbreviation "OIDC").
> +}
> +
> +sub properties {
> + return {
> + "issuer-url" => {
> + description => "OpenID Issuer Url",
> + type => 'string',
> + maxLength => 256,
> + },
> + "client-id" => {
> + description => "OpenID Client ID",
> + type => 'string',
> + maxLength => 256,
> + },
> + "client-key" => {
> + description => "OpenID 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 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 {
small nit: here you use double-quotes for keys with a minus, but in the property
schema definition you use single-quotes for them – might also use them here.
> + "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..a7ea596
> --- /dev/null
> +++ b/src/PMG/Auth/Plugin.pm
> @@ -0,0 +1,269 @@
> +package PMG::Auth::Plugin;
> +
> +use strict;
> +use warnings;
> +
> +use Digest::SHA;
> +use Encode;
> +
> +use PVE::JSONSchema qw(get_standard_option);
> +use PVE::SectionConfig;
> +use PVE::Tools;
> +use PVE::INotify;
> +use PMG::Utils;
nit: group and sort
> +
> +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
nit, missing trailing comma
> +);
> +
> +sub lock_domain_config {
> + my ($code, $errmsg) = @_;
> +
> + PVE::Tools::lock_file($lockfile, undef, $code);
> + my $err = $@;
> + if ($err) {
nit: could avoid an extra line:
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 $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',
should we move this plus the other copied formats/regexs/verifiers/.. to a
common-schema package (pve-common would be due for a split since quite a
while already anyway ^^).
You'd not have to do all the work for that though, moving the definitions to
pve-common in a new module (e.g., PVE::Schema::Auth) would allow us to reuse
it now in PVE and PMG without to much work and allow to split this later out
into a new package relatively easily.
> +});
> +
> +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('pmg-tfa-config', $tfa_format);
> +
> +PVE::JSONSchema::register_standard_option('tfa', {
> + description => "Use Two-factor authentication.",
> + type => 'string', format => 'pmg-tfa-config',
> + optional => 1,
> + maxLength => 128,
> +});
> +
> +sub parse_tfa_config {
> + my ($data) = @_;
> +
> + return PVE::JSONSchema::parse_property_string($tfa_format, $data);
> +}
> +
> +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..8fb93c9 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});
Yuck on the pre-existing `$+{userid}` for , but oh well..
https://perldoc.perl.org/variables/$+
> + 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,24 @@ sub write_user_conf {
> $verify_entry->($d);
> $cfg->{$d->{userid}} = $d;
>
> + my $valid_pmg_realms = PMG::Utils::valid_pmg_realms();
> + my $realm_list = join('|', @$valid_pmg_realms);
> 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_list})!;
> }
>
> 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_list})$/;
> + $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..e173894 100644
> --- a/src/PMG/Utils.pm
> +++ b/src/PMG/Utils.pm
> @@ -49,12 +49,27 @@ postgres_admin_cmd
> try_decode_utf8
> );
>
> -my $valid_pmg_realms = ['pam', 'pmg', 'quarantine'];
> +my $user_regex = qr![^\s:/]+!;
> +
> +sub valid_pmg_realms {
> + my $cfg = PVE::INotify::read_file('realms.cfg');
> + my $ids = $cfg->{ids};
> + my $realms = [];
> + my @list = keys %{$ids};
> + for my $realm (@list) {
> + push @$realms, $realm;
> + }
This can be quite a bit reduced to:
my $realms = [ sort keys $cfg->{ids}->%* ];
> + push @$realms, 'pam';
> + push @$realms, 'quarantine';
can be fine like this, but FWIW, you could push multiple values at once or
even further reduce this to:
return [ 'pam', 'quarantine', keys $cfg->{ids}->%* ];
But, FWICT you always use this in a regex to test if something is a valid realm, so
why stop bothering with this and rather either:
- add (or transform this into) a valid_pmg_realm_regex method
- change it to a `is_valid_realm` method that takes the to-check realm value as param
and then implement that like:
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;
}
would be faster for the built-in ones (no config parse) and avoid some array
creation from a hash-map only to join into a regex by simply using the original
hash-map directly for all our use case.
> + return $realms;
> +}
> +
> +PVE::JSONSchema::register_format('pmg-realm', \&valid_pmg_realms);
>
> PVE::JSONSchema::register_standard_option('realm', {
> description => "Authentication domain ID",
> type => 'string',
> - enum => $valid_pmg_realms,
> + format => 'pmg-realm',
> maxLength => 32,
> });
>
> @@ -82,7 +97,7 @@ sub verify_username {
> die "user name '$username' is too short\n" if !$noerr;
> return undef;
> }
> - if ($len > 64) {
> + if ($len > 128) {
> die "user name '$username' is too long ($len > 64)\n" if !$noerr;
you missed updating the error message, so this would be rather confusing if one runs
into it.
> return undef;
> }
> @@ -90,8 +105,9 @@ sub verify_username {
> # 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 $valid_pmg_realms = valid_pmg_realms();
does this need the realm list here, i.e., isn't the existence of the realm checked
already elsewhere already so that we can use some general [a-z]+ for that part in
the regex here?
> my $realm_list = join('|', @$valid_pmg_realms);
> - if ($username =~ m!^([^\s:/]+)\@(${realm_list})$!) {
> + if ($username =~ m!^(${user_regex})\@(${realm_list})$!) {
> return wantarray ? ($username, $1, $2) : $username;
> }
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [pmg-devel] [PATCH pmg-api 3/6] api: add/update/remove realms like in PVE
2024-04-02 11:27 [pmg-devel] [PATCH proxmox-perl-rs/pmg-api/pmg-gui 0/6] fix #3892: OpenID Markus Frank
2024-04-02 11:27 ` [pmg-devel] [PATCH proxmox-perl-rs 1/6] move openid code from pve-rs to common Markus Frank
2024-04-02 11:27 ` [pmg-devel] [PATCH pmg-api 2/6] config: add plugin system for realms & add openid type realms Markus Frank
@ 2024-04-02 11:27 ` Markus Frank
2024-04-02 11:27 ` [pmg-devel] [PATCH pmg-api 4/6] api: openid login similar to PVE Markus Frank
` (2 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Markus Frank @ 2024-04-02 11:27 UTC (permalink / raw)
To: pmg-devel
The name Authdomains.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 | 11 +-
src/PMG/API2/Authdomains.pm | 272 ++++++++++++++++++++++++++++++++++
3 files changed, 283 insertions(+), 1 deletion(-)
create mode 100644 src/PMG/API2/Authdomains.pm
diff --git a/src/Makefile b/src/Makefile
index 407cc4a..92662b5 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..c720f59 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,9 @@ __PACKAGE__->register_method ({
my $username = $param->{username};
- if ($username !~ m/\@(pam|pmg|quarantine)$/) {
+ my $valid_pmg_realms = PMG::Utils::valid_pmg_realms();
+ my $realm_list = join('|', @$valid_pmg_realms);
+ if ($username !~ m/\@(${realm_list})$/) {
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..a283fcb
--- /dev/null
+++ b/src/PMG/API2/Authdomains.pm
@@ -0,0 +1,272 @@
+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;
+
+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 = PMG::Auth::Plugin::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');
+ 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
^ permalink raw reply [flat|nested] 8+ messages in thread* [pmg-devel] [PATCH pmg-api 4/6] api: openid login similar to PVE
2024-04-02 11:27 [pmg-devel] [PATCH proxmox-perl-rs/pmg-api/pmg-gui 0/6] fix #3892: OpenID Markus Frank
` (2 preceding siblings ...)
2024-04-02 11:27 ` [pmg-devel] [PATCH pmg-api 3/6] api: add/update/remove realms like in PVE Markus Frank
@ 2024-04-02 11:27 ` Markus Frank
2024-04-02 11:27 ` [pmg-devel] [PATCH pmg-gui 5/6] login: add option to login with OpenID realm Markus Frank
2024-04-02 11:27 ` [pmg-devel] [PATCH pmg-gui 6/6] add pmxAuthView panel to UserManagement Markus Frank
5 siblings, 0 replies; 8+ messages in thread
From: Markus Frank @ 2024-04-02 11:27 UTC (permalink / raw)
To: pmg-devel
Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
src/Makefile | 1 +
src/PMG/API2/AccessControl.pm | 7 +
src/PMG/API2/OpenId.pm | 243 ++++++++++++++++++++++++++++++++++
src/PMG/HTTPServer.pm | 2 +
4 files changed, 253 insertions(+)
create mode 100644 src/PMG/API2/OpenId.pm
diff --git a/src/Makefile b/src/Makefile
index 92662b5..6ebf54b 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/OpenId.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 c720f59..6272a9f 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::OpenId;
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::OpenId",
+ 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/OpenId.pm b/src/PMG/API2/OpenId.pm
new file mode 100644
index 0000000..52349a7
--- /dev/null
+++ b/src/PMG/API2/OpenId.pm
@@ -0,0 +1,243 @@
+package PMG::API2::OpenId;
+
+use strict;
+use warnings;
+
+use PVE::Tools qw(extract_param lock_file);
+use PMG::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} != openid)\n" if $config->{type} ne "openid";
+
+ 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 = PMG::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 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 authorization code and create a ticket.",
+ parameters => {
+ additionalProperties => 0,
+ properties => {
+ 'state' => {
+ description => "OpenId state.",
+ type => 'string',
+ maxLength => 1024,
+ },
+ code => {
+ description => "OpenId 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) = PMG::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 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 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 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
^ permalink raw reply [flat|nested] 8+ messages in thread* [pmg-devel] [PATCH pmg-gui 5/6] login: add option to login with OpenID realm
2024-04-02 11:27 [pmg-devel] [PATCH proxmox-perl-rs/pmg-api/pmg-gui 0/6] fix #3892: OpenID Markus Frank
` (3 preceding siblings ...)
2024-04-02 11:27 ` [pmg-devel] [PATCH pmg-api 4/6] api: openid login similar to PVE Markus Frank
@ 2024-04-02 11:27 ` Markus Frank
2024-04-02 11:27 ` [pmg-devel] [PATCH pmg-gui 6/6] add pmxAuthView panel to UserManagement Markus Frank
5 siblings, 0 replies; 8+ messages in thread
From: Markus Frank @ 2024-04-02 11:27 UTC (permalink / raw)
To: pmg-devel
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 63f4099..bdfedaf 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: {
+ openid: false,
+ },
+ formulas: {
+ button_text: function(get) {
+ if (get("openid") === true) {
+ return gettext("Login (OpenID redirect)");
+ } else {
+ return gettext("Login");
+ }
+ },
+ },
+ },
+
controller: {
xclass: 'Ext.app.ViewController',
@@ -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.openid === 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 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("openid", data.type === "openid");
+ },
+ },
'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 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 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);
+ },
+ });
+ }
}
},
},
@@ -249,6 +343,10 @@ Ext.define('PMG.LoginView', {
itemId: 'usernameField',
reference: 'usernameField',
stateId: 'login-username',
+ bind: {
+ visible: "{!openid}",
+ disabled: "{openid}",
+ },
},
{
xtype: 'textfield',
@@ -256,6 +354,16 @@ Ext.define('PMG.LoginView', {
fieldLabel: gettext('Password'),
name: 'password',
reference: 'passwordField',
+ bind: {
+ visible: "{!openid}",
+ disabled: "{openid}",
+ },
+ },
+ {
+ xtype: 'pmxRealmComboBox',
+ reference: 'realmfield',
+ name: 'realm',
+ value: 'pam',
},
{
xtype: 'proxmoxLanguageSelector',
@@ -264,12 +372,6 @@ Ext.define('PMG.LoginView', {
name: 'lang',
submitValue: false,
},
- {
- xtype: 'hiddenfield',
- reference: 'realmfield',
- name: 'realm',
- value: 'pmg',
- },
],
buttons: [
{
@@ -281,15 +383,19 @@ Ext.define('PMG.LoginView', {
labelAlign: 'right',
labelWidth: 150,
submitValue: false,
+ bind: {
+ visible: "{!openid}",
+ },
},
{
text: gettext('Request Quarantine Link'),
reference: 'quarantineButton',
},
{
- text: gettext('Login'),
+ bind: {
+ text: "{button_text}",
+ },
reference: 'loginButton',
- formBind: true,
},
],
},
--
2.39.2
^ permalink raw reply [flat|nested] 8+ messages in thread* [pmg-devel] [PATCH pmg-gui 6/6] add pmxAuthView panel to UserManagement
2024-04-02 11:27 [pmg-devel] [PATCH proxmox-perl-rs/pmg-api/pmg-gui 0/6] fix #3892: OpenID Markus Frank
` (4 preceding siblings ...)
2024-04-02 11:27 ` [pmg-devel] [PATCH pmg-gui 5/6] login: add option to login with OpenID realm Markus Frank
@ 2024-04-02 11:27 ` Markus Frank
5 siblings, 0 replies; 8+ messages in thread
From: Markus Frank @ 2024-04-02 11:27 UTC (permalink / raw)
To: pmg-devel
Make the realm configuration available in PMG and disable LDAP realms for
now.
Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
js/UserManagement.js | 6 ++++++
js/Utils.js | 9 +++++++++
2 files changed, 15 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 7fa154e..83d53f3 100644
--- a/js/Utils.js
+++ b/js/Utils.js
@@ -833,6 +833,15 @@ Ext.define('PMG.Utils', {
constructor: function() {
var me = this;
+ // Disable LDAP as a realm until LDAP login is implemented
+ Proxmox.Schema.authDomains.ldap.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
^ permalink raw reply [flat|nested] 8+ messages in thread