all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Alexander Abraham <a.abraham@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH pve-access-control v3 1/1] fix #5076: Added an optional "audiences" field
Date: Tue,  3 Jun 2025 11:12:55 +0200	[thread overview]
Message-ID: <20250603091256.40923-3-a.abraham@proxmox.com> (raw)
In-Reply-To: <20250603091256.40923-1-a.abraham@proxmox.com>

An optional "audiences" field was added to the schema
to accept an array of strings for any audiences communicated
by using an Open ID realm in PVE.

Signed-off-by: Alexander Abraham <a.abraham@proxmox.com>
---
 src/PVE/API2/OpenId.pm | 463 +++++++++++++++++++++++------------------
 src/PVE/Auth/OpenId.pm | 141 +++++++------
 2 files changed, 335 insertions(+), 269 deletions(-)

diff --git a/src/PVE/API2/OpenId.pm b/src/PVE/API2/OpenId.pm
index 77410e6..90e047d 100644
--- a/src/PVE/API2/OpenId.pm
+++ b/src/PVE/API2/OpenId.pm
@@ -21,7 +21,7 @@ use base qw(PVE::RESTHandler);
 my $openid_state_path = "/var/lib/pve-manager";
 
 my $lookup_openid_auth = sub {
-    my ($realm, $redirect_url) = @_;
+    my ( $realm, $redirect_url ) = @_;
 
     my $cfg = cfs_read_file('domains.cfg');
     my $ids = $cfg->{ids};
@@ -29,221 +29,272 @@ my $lookup_openid_auth = sub {
     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";
+    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'},
+        issuer_url => $config->{'issuer-url'},
+        client_id  => $config->{'client-id'},
+        client_key => $config->{'client-key'},
     };
-    $openid_config->{prompt} = $config->{'prompt'} if defined($config->{'prompt'});
+    $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) ];
+    if ( defined( my $acr = $config->{'acr-values'} ) ) {
+        $openid_config->{acr_values} = [ PVE::Tools::split_list($acr) ];
     }
 
-    my $openid = PVE::RS::OpenId->discover($openid_config, $redirect_url);
-    return ($config, $openid);
+    if ( defined( my $audiences = $config->{'audiences'} ) ) {
+        $openid_config->{audiences} = $config->{'audiences'};
+    }
+
+    my $openid = PVE::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 => get_standard_option('realm'),
-	    '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 $dcconf = PVE::Cluster::cfs_read_file('datacenter.cfg');
-	local $ENV{all_proxy} = $dcconf->{http_proxy} if exists $dcconf->{http_proxy};
-
-	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,
+__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          => get_standard_option('realm'),
+                '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 $dcconf = PVE::Cluster::cfs_read_file('datacenter.cfg');
+            local $ENV{all_proxy} = $dcconf->{http_proxy}
+              if exists $dcconf->{http_proxy};
+
+            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,
+                },
             },
-	    code => {
-		description => "OpenId authorization code.",
-		type => 'string',
-		maxLength => 4096,
+        },
+        returns => {
+            properties => {
+                username            => { type => 'string' },
+                ticket              => { type => 'string' },
+                CSRFPreventionToken => { type => 'string' },
+                cap         => { type => 'object' },  # computed api permissions
+                clustername => { type => 'string', optional => 1 },
             },
-	    'redirect-url' => {
-		description => "Redirection Url. The client should set this to the used server url (location.origin).",
-		type => 'string',
-		maxLength => 255,
-	    },
-	},
-    },
-    returns => {
-	properties => {
-	    username => { type => 'string' },
-	    ticket => { type => 'string' },
-	    CSRFPreventionToken => { type => 'string' },
-	    cap => { type => 'object' },  # computed api permissions
-	    clustername => { type => 'string', optional => 1 },
-	},
-    },
-    permissions => { user => 'world' },
-    code => sub {
-	my ($param) = @_;
-
-	my $rpcenv = PVE::RPCEnvironment::get();
-
-	my $res;
-	eval {
-	    my $dcconf = PVE::Cluster::cfs_read_file('datacenter.cfg');
-	    local $ENV{all_proxy} = $dcconf->{http_proxy} if exists $dcconf->{http_proxy};
-
-	    my ($realm, $private_auth_state) = PVE::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
-	    PVE::Auth::Plugin::verify_username($username);
-
-	    if ($config->{'autocreate'} && !$rpcenv->check_user_exist($username, 1)) {
-		PVE::AccessControl::lock_user_config(sub {
-		    my $usercfg = cfs_read_file("user.cfg");
-
-		    die "user '$username' already exists\n" if $usercfg->{users}->{$username};
-
-		    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;
-		    }
-
-		    $usercfg->{users}->{$username} = $entry;
-
-		    cfs_write_file("user.cfg", $usercfg);
-		}, "autocreate openid user failed");
-	    } else {
-		# test if user exists and is enabled
-		$rpcenv->check_user_enabled($username);
-	    }
-
-	    my $ticket = PVE::AccessControl::assemble_ticket($username);
-	    my $csrftoken = PVE::AccessControl::assemble_csrf_prevention_token($username);
-	    my $cap = $rpcenv->compute_api_permission($username);
-
-	    $res = {
-		ticket => $ticket,
-		username => $username,
-		CSRFPreventionToken => $csrftoken,
-		cap => $cap,
-	    };
-
-	    my $clinfo = PVE::Cluster::get_clinfo();
-	    if ($clinfo->{cluster}->{name} && $rpcenv->check($username, '/', ['Sys.Audit'], 1)) {
-		$res->{clustername} = $clinfo->{cluster}->{name};
-	    }
-	};
-	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\n", code => 401);
-	}
-
-	PVE::Cluster::log_msg('info', 'root@pam', "successful openid auth for user '$res->{username}'");
-
-	return $res;
-    }});
+        },
+        permissions => { user => 'world' },
+        code        => sub {
+            my ($param) = @_;
+
+            my $rpcenv = PVE::RPCEnvironment::get();
+
+            my $res;
+            eval {
+                my $dcconf = PVE::Cluster::cfs_read_file('datacenter.cfg');
+                local $ENV{all_proxy} = $dcconf->{http_proxy}
+                  if exists $dcconf->{http_proxy};
+
+                my ( $realm, $private_auth_state ) =
+                  PVE::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
+                PVE::Auth::Plugin::verify_username($username);
+
+                if ( $config->{'autocreate'}
+                    && !$rpcenv->check_user_exist( $username, 1 ) )
+                {
+                    PVE::AccessControl::lock_user_config(
+                        sub {
+                            my $usercfg = cfs_read_file("user.cfg");
+
+                            die "user '$username' already exists\n"
+                              if $usercfg->{users}->{$username};
+
+                            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;
+                            }
+
+                            $usercfg->{users}->{$username} = $entry;
+
+                            cfs_write_file( "user.cfg", $usercfg );
+                        },
+                        "autocreate openid user failed"
+                    );
+                }
+                else {
+                    # test if user exists and is enabled
+                    $rpcenv->check_user_enabled($username);
+                }
+
+                my $ticket = PVE::AccessControl::assemble_ticket($username);
+                my $csrftoken =
+                  PVE::AccessControl::assemble_csrf_prevention_token($username);
+                my $cap = $rpcenv->compute_api_permission($username);
+
+                $res = {
+                    ticket              => $ticket,
+                    username            => $username,
+                    CSRFPreventionToken => $csrftoken,
+                    cap                 => $cap,
+                };
+
+                my $clinfo = PVE::Cluster::get_clinfo();
+                if (   $clinfo->{cluster}->{name}
+                    && $rpcenv->check( $username, '/', ['Sys.Audit'], 1 ) )
+                {
+                    $res->{clustername} = $clinfo->{cluster}->{name};
+                }
+            };
+            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\n",
+                    code => 401 );
+            }
+
+            PVE::Cluster::log_msg( 'info', 'root@pam',
+                "successful openid auth for user '$res->{username}'" );
+
+            return $res;
+        }
+    }
+);
diff --git a/src/PVE/Auth/OpenId.pm b/src/PVE/Auth/OpenId.pm
index c8e4db9..243f6f0 100755
--- a/src/PVE/Auth/OpenId.pm
+++ b/src/PVE/Auth/OpenId.pm
@@ -5,7 +5,8 @@ use warnings;
 
 use PVE::Tools;
 use PVE::Auth::Plugin;
-use PVE::Cluster qw(cfs_register_file cfs_read_file cfs_write_file cfs_lock_file);
+use PVE::Cluster
+  qw(cfs_register_file cfs_read_file cfs_write_file cfs_lock_file);
 
 use base qw(PVE::Auth::Plugin);
 
@@ -15,77 +16,91 @@ sub type {
 
 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',
-	    pattern => '^[^\x00-\x1F\x7F <>#"]*$', # Prohibit characters not allowed in URI RFC 2396.
-	    optional => 1,
-	},
-   };
+        "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',
+            pattern => '^[^\x00-\x1F\x7F <>#"]*$'
+            ,    # Prohibit characters not allowed in URI RFC 2396.
+            optional => 1,
+        },
+        'audiences' => {
+            description =>
+"Specifies the authentication claims neccessary for checking the privileges the requesting user has.",
+            type    => 'array',
+            'items' => {
+                type     => 'string',
+                pattern  => '^[a-zA-Z0-9-_+.]+$',
+                optional => 1
+            }
+        },
+    };
 }
 
 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 },
+        "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 },
+        "audiences"      => { optional => 1 },
+        default          => { optional => 1 },
+        comment          => { optional => 1 },
     };
 }
 
 sub authenticate_user {
-    my ($class, $config, $realm, $username, $password) = @_;
-
+    my ( $class, $config, $realm, $username, $password ) = @_;
     die "OpenID realm does not allow password verification.\n";
 }
 
-
 1;
-- 
2.39.5



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


  parent reply	other threads:[~2025-06-03  9:13 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-03  9:12 [pve-devel] [PATCH access-control/manager/proxmox v3 0/3] fix #5076: Added Open ID audiences Alexander Abraham
2025-06-03  9:12 ` [pve-devel] [PATCH proxmox v3 1/1] fix #5076: Added logic to handle OIDC audiences Alexander Abraham
2025-06-03  9:12 ` Alexander Abraham [this message]
2025-06-03  9:12 ` [pve-devel] [PATCH pve-manager v3 1/1] fix #5076: Added an "audiences" field for Open ID Alexander Abraham

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250603091256.40923-3-a.abraham@proxmox.com \
    --to=a.abraham@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal