* [pmg-devel] applied: [PATCH api] oidc realm: add more flexiple access-role assignment
@ 2025-02-27 9:51 Thomas Lamprecht
2025-02-27 10:01 ` Stoiko Ivanov
0 siblings, 1 reply; 3+ messages in thread
From: Thomas Lamprecht @ 2025-02-27 9:51 UTC (permalink / raw)
To: pmg-devel
deprecate the fixed role assignment property and replace it with one
that allows to select the source of the role, for now allow static,
which is equivalent with what we had, and also to query the role from
a role claim, so that the OIDC provider can return the correct role
inside the returned user information.
Tested the dynamic role assignment with keycload by setting up an
'User Attribute' Mapper for a client and added added the respective
user attribute to some test user.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
It's not much code and most is schema and parsing, the implementation
itself is pretty straigt forward, basically the only questionable thing
is naming of properties.
src/PMG/API2/OIDC.pm | 26 ++++++++++++++++++++-
src/PMG/Auth/OIDC.pm | 54 +++++++++++++++++++++++++++++++++++++++++---
2 files changed, 76 insertions(+), 4 deletions(-)
diff --git a/src/PMG/API2/OIDC.pm b/src/PMG/API2/OIDC.pm
index 92ff88d..20acc41 100644
--- a/src/PMG/API2/OIDC.pm
+++ b/src/PMG/API2/OIDC.pm
@@ -13,6 +13,7 @@ use PVE::JSONSchema qw(get_standard_option);
use PVE::RESTHandler;
use PMG::AccessControl;
+use PMG::Auth::OIDC;
use PMG::Auth::Plugin;
use PMG::RESTEnvironment;
@@ -204,7 +205,30 @@ __PACKAGE__->register_method ({
if (defined(my $family_name = $info->{'family_name'})) {
$entry->{lastname} = $family_name;
}
- $entry->{role} = $config->{'autocreate-role'} // 'audit';
+
+ # NOTE: 'autocreate-role' is deprecated and has less priority than the more
+ # flexible 'autocreate-role-assignment'
+ $entry->{role} = $config->{'autocreate-role'} // 'audit'; # default
+ if (my $role_assignment_raw = $config->{'autocreate-role-assignment'}) {
+ my $role_assignment =
+ PMG::Auth::OIDC::parse_autocreate_role_assignment($role_assignment_raw);
+
+ if ($role_assignment->{source} eq 'fixed') {
+ $entry->{role} = $role_assignment->{'fixed-role'};
+ } elsif ($role_assignment->{source} eq 'from-claim') {
+ my $role_attr = $role_assignment->{'role-claim'};
+ if (my $role = $info->{$role_attr}) {
+ $role = lc($role); # normalize to lower-case
+ die "required '$role_attr' role-claim attribute not found, cannot autocreate user\n"
+ if $role !~ /^(?:admin|qmanager|audit|helpdesk)$/;
+ $entry->{role} = $role;
+ } else {
+ die "required '$role_attr' role-claim attribute not found, cannot autocreate user\n";
+ }
+ } else {
+ die "unkown role assignment source '$role_assignment->{source}' - implement me";
+ }
+ }
$entry->{userid} = $username;
$entry->{username} = $unique_name;
$entry->{realm} = $realm;
diff --git a/src/PMG/Auth/OIDC.pm b/src/PMG/Auth/OIDC.pm
index 4129d47..9082c75 100755
--- a/src/PMG/Auth/OIDC.pm
+++ b/src/PMG/Auth/OIDC.pm
@@ -4,6 +4,8 @@ use strict;
use warnings;
use PVE::Tools;
+use PVE::JSONSchema qw(parse_property_string);
+
use PMG::Auth::Plugin;
use base qw(PMG::Auth::Plugin);
@@ -12,6 +14,44 @@ sub type {
return 'oidc';
}
+my $autocreate_role_assignment_format = {
+ source => {
+ type => 'string',
+ enum => ['fixed', 'from-claim'],
+ default => 'fixed',
+ description => "How the access role for a newly auto-created user should be selected.",
+ },
+ 'fixed-role' => {
+ type => 'string',
+ enum => ['admin', 'qmanager', 'audit', 'helpdesk'],
+ default => 'audit',
+ optional => 1,
+ description => "The fixed role that should be assigned to auto-created users.",
+ },
+ 'role-claim' => {
+ description => "OIDC claim used to assign the unique username.",
+ type => 'string',
+ default => 'role',
+ optional => 1,
+ pattern => qr/^[a-zA-Z0-9._:-]+$/,
+ },
+};
+
+
+sub parse_autocreate_role_assignment {
+ my ($raw) = @_;
+ return undef if !$raw or !length($raw);
+
+ my $role_assignment = parse_property_string($autocreate_role_assignment_format, $raw);
+ $role_assignment->{'fixed-role'} = 'audit'
+ if $role_assignment->{'source'} eq 'fixed' && !defined($role_assignment->{'fixed-role'});
+
+ $role_assignment->{'role-claim'} = 'role'
+ if $role_assignment->{'source'} eq 'from-clain' && !defined($role_assignment->{'role-claim'});
+
+ return $role_assignment;
+}
+
sub properties {
return {
'issuer-url' => {
@@ -39,13 +79,20 @@ sub properties {
type => 'boolean',
default => 0,
},
- 'autocreate-role' => {
- description => "Automatically create users with a specific role.",
+ 'autocreate-role' => { # NOTE: depreacated since the beginning, just here for compat
+ description => "Automatically create users with a specific role."
+ ." NOTE: Depreacated, favor 'autocreate-role-assignment'",
type => 'string',
enum => ['admin', 'qmanager', 'audit', 'helpdesk'],
default => 'audit',
optional => 1,
},
+ 'autocreate-role-assignment' => {
+ description => "Defines which role should be assigned to auto-created users.",
+ type => 'string', format => $autocreate_role_assignment_format,
+ default => 'source=fixed,fixed-role=auditor',
+ optional => 1,
+ },
'username-claim' => {
description => "OpenID Connect claim used to generate the unique username.",
type => 'string',
@@ -84,7 +131,8 @@ sub options {
'client-id' => {},
'client-key' => { optional => 1 },
autocreate => { optional => 1 },
- 'autocreate-role' => { optional => 1 },
+ 'autocreate-role' => { optional => 1 }, # NOTE: depreacated in favor of 'autocreate-role-assignment'
+ 'autocreate-role-assignment' => { optional => 1 },
'username-claim' => { optional => 1, fixed => 1 },
prompt => { optional => 1 },
scopes => { optional => 1 },
--
2.39.5
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [pmg-devel] applied: [PATCH api] oidc realm: add more flexiple access-role assignment
2025-02-27 9:51 [pmg-devel] applied: [PATCH api] oidc realm: add more flexiple access-role assignment Thomas Lamprecht
@ 2025-02-27 10:01 ` Stoiko Ivanov
2025-02-27 10:04 ` Thomas Lamprecht
0 siblings, 1 reply; 3+ messages in thread
From: Stoiko Ivanov @ 2025-02-27 10:01 UTC (permalink / raw)
To: Thomas Lamprecht; +Cc: pmg-devel
Thanks! from a quick look - it seems sensible, and the naming is in order.
one typo and 2 minor nits inline:
On Thu, 27 Feb 2025 10:51:52 +0100
Thomas Lamprecht <t.lamprecht@proxmox.com> wrote:
> deprecate the fixed role assignment property and replace it with one
> that allows to select the source of the role, for now allow static,
> which is equivalent with what we had, and also to query the role from
> a role claim, so that the OIDC provider can return the correct role
> inside the returned user information.
>
> Tested the dynamic role assignment with keycload by setting up an
> 'User Attribute' Mapper for a client and added added the respective
> user attribute to some test user.
>
> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> ---
>
> It's not much code and most is schema and parsing, the implementation
> itself is pretty straigt forward, basically the only questionable thing
> is naming of properties.
>
> src/PMG/API2/OIDC.pm | 26 ++++++++++++++++++++-
> src/PMG/Auth/OIDC.pm | 54 +++++++++++++++++++++++++++++++++++++++++---
> 2 files changed, 76 insertions(+), 4 deletions(-)
>
> diff --git a/src/PMG/API2/OIDC.pm b/src/PMG/API2/OIDC.pm
> index 92ff88d..20acc41 100644
> --- a/src/PMG/API2/OIDC.pm
> +++ b/src/PMG/API2/OIDC.pm
> @@ -13,6 +13,7 @@ use PVE::JSONSchema qw(get_standard_option);
> use PVE::RESTHandler;
>
> use PMG::AccessControl;
> +use PMG::Auth::OIDC;
> use PMG::Auth::Plugin;
> use PMG::RESTEnvironment;
>
> @@ -204,7 +205,30 @@ __PACKAGE__->register_method ({
> if (defined(my $family_name = $info->{'family_name'})) {
> $entry->{lastname} = $family_name;
> }
> - $entry->{role} = $config->{'autocreate-role'} // 'audit';
> +
> + # NOTE: 'autocreate-role' is deprecated and has less priority than the more
> + # flexible 'autocreate-role-assignment'
> + $entry->{role} = $config->{'autocreate-role'} // 'audit'; # default
> + if (my $role_assignment_raw = $config->{'autocreate-role-assignment'}) {
> + my $role_assignment =
> + PMG::Auth::OIDC::parse_autocreate_role_assignment($role_assignment_raw);
> +
> + if ($role_assignment->{source} eq 'fixed') {
> + $entry->{role} = $role_assignment->{'fixed-role'};
> + } elsif ($role_assignment->{source} eq 'from-claim') {
> + my $role_attr = $role_assignment->{'role-claim'};
> + if (my $role = $info->{$role_attr}) {
> + $role = lc($role); # normalize to lower-case
> + die "required '$role_attr' role-claim attribute not found, cannot autocreate user\n"
> + if $role !~ /^(?:admin|qmanager|audit|helpdesk)$/;
> + $entry->{role} = $role;
> + } else {
> + die "required '$role_attr' role-claim attribute not found, cannot autocreate user\n";
> + }
> + } else {
> + die "unkown role assignment source '$role_assignment->{source}' - implement me";
> + }
> + }
> $entry->{userid} = $username;
> $entry->{username} = $unique_name;
> $entry->{realm} = $realm;
> diff --git a/src/PMG/Auth/OIDC.pm b/src/PMG/Auth/OIDC.pm
> index 4129d47..9082c75 100755
> --- a/src/PMG/Auth/OIDC.pm
> +++ b/src/PMG/Auth/OIDC.pm
> @@ -4,6 +4,8 @@ use strict;
> use warnings;
>
> use PVE::Tools;
> +use PVE::JSONSchema qw(parse_property_string);
> +
> use PMG::Auth::Plugin;
>
> use base qw(PMG::Auth::Plugin);
> @@ -12,6 +14,44 @@ sub type {
> return 'oidc';
> }
>
> +my $autocreate_role_assignment_format = {
> + source => {
> + type => 'string',
> + enum => ['fixed', 'from-claim'],
> + default => 'fixed',
> + description => "How the access role for a newly auto-created user should be selected.",
> + },
> + 'fixed-role' => {
> + type => 'string',
> + enum => ['admin', 'qmanager', 'audit', 'helpdesk'],
> + default => 'audit',
> + optional => 1,
> + description => "The fixed role that should be assigned to auto-created users.",
> + },
> + 'role-claim' => {
> + description => "OIDC claim used to assign the unique username.",
> + type => 'string',
> + default => 'role',
> + optional => 1,
> + pattern => qr/^[a-zA-Z0-9._:-]+$/,
> + },
> +};
> +
> +
> +sub parse_autocreate_role_assignment {
> + my ($raw) = @_;
> + return undef if !$raw or !length($raw);
> +
> + my $role_assignment = parse_property_string($autocreate_role_assignment_format, $raw);
> + $role_assignment->{'fixed-role'} = 'audit'
> + if $role_assignment->{'source'} eq 'fixed' && !defined($role_assignment->{'fixed-role'});
> +
> + $role_assignment->{'role-claim'} = 'role'
> + if $role_assignment->{'source'} eq 'from-clain' && !defined($role_assignment->{'role-claim'});
`s/from-clain/from-claim/`
> +
> + return $role_assignment;
> +}
> +
> sub properties {
> return {
> 'issuer-url' => {
> @@ -39,13 +79,20 @@ sub properties {
> type => 'boolean',
> default => 0,
> },
> - 'autocreate-role' => {
> - description => "Automatically create users with a specific role.",
> + 'autocreate-role' => { # NOTE: depreacated since the beginning, just here for compat
nit: `s/depreacated/deprecated/` (had to google myself, as I always get
that wrong)
> + description => "Automatically create users with a specific role."
> + ." NOTE: Depreacated, favor 'autocreate-role-assignment'",
nit: deprecated
> type => 'string',
> enum => ['admin', 'qmanager', 'audit', 'helpdesk'],
> default => 'audit',
> optional => 1,
> },
> + 'autocreate-role-assignment' => {
> + description => "Defines which role should be assigned to auto-created users.",
> + type => 'string', format => $autocreate_role_assignment_format,
> + default => 'source=fixed,fixed-role=auditor',
> + optional => 1,
> + },
> 'username-claim' => {
> description => "OpenID Connect claim used to generate the unique username.",
> type => 'string',
> @@ -84,7 +131,8 @@ sub options {
> 'client-id' => {},
> 'client-key' => { optional => 1 },
> autocreate => { optional => 1 },
> - 'autocreate-role' => { optional => 1 },
> + 'autocreate-role' => { optional => 1 }, # NOTE: depreacated in favor of 'autocreate-role-assignment'
nit: deprecated
> + 'autocreate-role-assignment' => { optional => 1 },
> 'username-claim' => { optional => 1, fixed => 1 },
> prompt => { optional => 1 },
> scopes => { optional => 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] 3+ messages in thread
* Re: [pmg-devel] applied: [PATCH api] oidc realm: add more flexiple access-role assignment
2025-02-27 10:01 ` Stoiko Ivanov
@ 2025-02-27 10:04 ` Thomas Lamprecht
0 siblings, 0 replies; 3+ messages in thread
From: Thomas Lamprecht @ 2025-02-27 10:04 UTC (permalink / raw)
To: Stoiko Ivanov; +Cc: pmg-devel
Am 27.02.25 um 11:01 schrieb Stoiko Ivanov:
> Thanks! from a quick look - it seems sensible, and the naming is in order.
>
> one typo and 2 minor nits inline:
thanks, pushed a fix with the errors you pointed out and an additional
s/unkown/unknown/ one.
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-02-27 10:05 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-02-27 9:51 [pmg-devel] applied: [PATCH api] oidc realm: add more flexiple access-role assignment Thomas Lamprecht
2025-02-27 10:01 ` Stoiko Ivanov
2025-02-27 10:04 ` Thomas Lamprecht
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal