public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH access-control-5076/manager/proxmox-5076 v2 0/3] fix #5076: Added Open ID audiences
@ 2025-06-02 14:14 Alexander Abraham
  2025-06-02 14:14 ` [pve-devel] [PATCH proxmox v2 1/1] fix #5076: Added logic to handle OIDC audiences Alexander Abraham
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Alexander Abraham @ 2025-06-02 14:14 UTC (permalink / raw)
  To: pve-devel

This series adds support for handling Open ID audiences as
described in bug #5076. PVE's API schema was updated to
accept an optional field, an array of strings and the Rust
code was also updated to accordingly handle any incoming
audiences and compare them to the realm config's audiences.
In the realm dialogue for adding an Open ID realm, a new field
titled "Audiences" was added so that users can save any audiences
in their realm domains config file.

proxmox-5076:

Alexander Abraham (1):
  fix #5076: Added logic to handle OIDC audiences

 proxmox-openid/src/lib.rs | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)


pve-access-control-5076:

Alexander Abraham (1):
  fix #5076: Changed audiences to an array

 src/PVE/API2/OpenId.pm |  5 ++++-
 src/PVE/Auth/OpenId.pm | 11 ++++++++++-
 2 files changed, 14 insertions(+), 2 deletions(-)


pve-manager:

Alexander Abraham (1):
  fix #5076: Added an "audiences" field for Open ID

 www/manager6/Parser.js            | 27 +++++++++++++++++++++++++++
 www/manager6/dc/AuthEditBase.js   |  8 ++++++++
 www/manager6/dc/AuthEditOpenId.js | 10 +++++++++-
 3 files changed, 44 insertions(+), 1 deletion(-)


Summary over all repositories:
  6 files changed, 76 insertions(+), 5 deletions(-)

-- 
Generated by git-murpp 0.8.1


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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [pve-devel] [PATCH proxmox v2 1/1] fix #5076: Added logic to handle OIDC audiences
  2025-06-02 14:14 [pve-devel] [PATCH access-control-5076/manager/proxmox-5076 v2 0/3] fix #5076: Added Open ID audiences Alexander Abraham
@ 2025-06-02 14:14 ` Alexander Abraham
  2025-06-03  8:39   ` Shannon Sterz
  2025-06-02 14:14 ` [pve-devel] [PATCH pve-access-control v2 1/1] fix #5076: Changed audiences to an array Alexander Abraham
  2025-06-02 14:14 ` [pve-devel] [PATCH pve-manager v2 1/1] fix #5076: Added an "audiences" field for Open ID Alexander Abraham
  2 siblings, 1 reply; 5+ messages in thread
From: Alexander Abraham @ 2025-06-02 14:14 UTC (permalink / raw)
  To: pve-devel

A field for OIDC audiences was added, logic to handle these audiences,
and the audiences supplied by an OIDC IDP are validated against
the audiences a user saves in their realm domains
configuration.

Signed-off-by: Alexander Abraham <a.abraham@proxmox.com>
---
 proxmox-openid/src/lib.rs | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/proxmox-openid/src/lib.rs b/proxmox-openid/src/lib.rs
index fe65fded..fa22638a 100644
--- a/proxmox-openid/src/lib.rs
+++ b/proxmox-openid/src/lib.rs
@@ -53,6 +53,8 @@ pub struct OpenIdConfig {
     pub prompt: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub acr_values: Option<Vec<String>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub audiences: Option<Vec<String>>,
 }
 
 pub struct OpenIdAuthenticator {
@@ -205,12 +207,26 @@ impl OpenIdAuthenticator {
             .request(http_client)
             .map_err(|err| format_err!("Failed to contact token endpoint: {}", err))?;
 
-        let id_token_verifier: CoreIdTokenVerifier = self.client.id_token_verifier();
         let id_token_claims: &CoreIdTokenClaims = token_response
             .extra_fields()
             .id_token()
             .expect("Server did not return an ID token")
-            .claims(&id_token_verifier, &private_auth_state.nonce)
+            .claims(
+                &((self.client.id_token_verifier())
+                    .require_audience_match(true)
+                    .set_other_audience_verifier_fn(|aud| {
+                        let curr_aud: &String = aud;
+                        if &self.config.client_id == curr_aud {
+                            true
+                        } else {
+                            match self.config.audiences.as_ref() {
+                                Some(confd_auds) => confd_auds.contains(curr_aud),
+                                None => false,
+                            }
+                        }
+                    })),
+                &private_auth_state.nonce,
+            )
             .map_err(|err| format_err!("Failed to verify ID token: {}", err))?;
 
         let userinfo_claims: GenericUserInfoClaims = self
-- 
2.39.5



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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [pve-devel] [PATCH pve-access-control v2 1/1] fix #5076: Changed audiences to an array
  2025-06-02 14:14 [pve-devel] [PATCH access-control-5076/manager/proxmox-5076 v2 0/3] fix #5076: Added Open ID audiences Alexander Abraham
  2025-06-02 14:14 ` [pve-devel] [PATCH proxmox v2 1/1] fix #5076: Added logic to handle OIDC audiences Alexander Abraham
@ 2025-06-02 14:14 ` Alexander Abraham
  2025-06-02 14:14 ` [pve-devel] [PATCH pve-manager v2 1/1] fix #5076: Added an "audiences" field for Open ID Alexander Abraham
  2 siblings, 0 replies; 5+ messages in thread
From: Alexander Abraham @ 2025-06-02 14:14 UTC (permalink / raw)
  To: pve-devel

The API schema was updated so that audiences are treated
as an array of strings. The code for parsing audiences was
updated to also treat audiences like an array of strings
of a certain format.

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

diff --git a/src/PVE/API2/OpenId.pm b/src/PVE/API2/OpenId.pm
index 77410e6..97bac7c 100644
--- a/src/PVE/API2/OpenId.pm
+++ b/src/PVE/API2/OpenId.pm
@@ -45,6 +45,10 @@ my $lookup_openid_auth = sub {
 	$openid_config->{acr_values} = [ PVE::Tools::split_list($acr) ];
     }
 
+    if (defined(my $audiences = $config->{'audiences'})) {
+        $openid_config->{audiences} = $config->{'audiences'}
+    }
+
     my $openid = PVE::RS::OpenId->discover($openid_config, $redirect_url);
     return ($config, $openid);
 };
@@ -169,7 +173,6 @@ __PACKAGE__->register_method ({
 	    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'};
 
diff --git a/src/PVE/Auth/OpenId.pm b/src/PVE/Auth/OpenId.pm
index c8e4db9..4000142 100755
--- a/src/PVE/Auth/OpenId.pm
+++ b/src/PVE/Auth/OpenId.pm
@@ -63,6 +63,15 @@ sub properties {
 	    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
+            }
+	},
    };
 }
 
@@ -76,6 +85,7 @@ sub options {
 	prompt => { optional => 1 },
 	scopes => { optional => 1 },
 	"acr-values" => { optional => 1 },
+	"audiences" => { optional => 1 },
 	default => { optional => 1 },
 	comment => { optional => 1 },
     };
@@ -83,7 +93,6 @@ sub options {
 
 sub authenticate_user {
     my ($class, $config, $realm, $username, $password) = @_;
-
     die "OpenID realm does not allow password verification.\n";
 }
 
-- 
2.39.5



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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [pve-devel] [PATCH pve-manager v2 1/1] fix #5076: Added an "audiences" field for Open ID
  2025-06-02 14:14 [pve-devel] [PATCH access-control-5076/manager/proxmox-5076 v2 0/3] fix #5076: Added Open ID audiences Alexander Abraham
  2025-06-02 14:14 ` [pve-devel] [PATCH proxmox v2 1/1] fix #5076: Added logic to handle OIDC audiences Alexander Abraham
  2025-06-02 14:14 ` [pve-devel] [PATCH pve-access-control v2 1/1] fix #5076: Changed audiences to an array Alexander Abraham
@ 2025-06-02 14:14 ` Alexander Abraham
  2 siblings, 0 replies; 5+ messages in thread
From: Alexander Abraham @ 2025-06-02 14:14 UTC (permalink / raw)
  To: pve-devel

A field for audiences for OpenId was added for users to supply
Open ID audiences as a space-separated array of strings in their
realm configuration. This array of audiences is then saved in the
realm domains config file.

Signed-off-by: Alexander Abraham <a.abraham@proxmox.com>
---
 www/manager6/Parser.js            | 27 +++++++++++++++++++++++++++
 www/manager6/dc/AuthEditBase.js   |  8 ++++++++
 www/manager6/dc/AuthEditOpenId.js | 10 +++++++++-
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/www/manager6/Parser.js b/www/manager6/Parser.js
index 07eb9b17..4868777e 100644
--- a/www/manager6/Parser.js
+++ b/www/manager6/Parser.js
@@ -1,9 +1,36 @@
 // Some configuration values are complex strings - so we need parsers/generators for them.
 Ext.define('PVE.Parser', {
+
  statics: {
 
     // this class only contains static functions
+    checkKeys: function(obj, subject) {
+      var result = false;
+      for (const [key, _] of Object.entries(obj)) {
+        if (key === subject) {
+          result = true;
+        } else {
+          // Do nothing.
+        }
+      }
+      return result;
+    },
 
+    parseOpenIdAudiences: function(audiences) {
+      var result = [];
+      var container = [];
+      for (var i = 0; i < audiences.length; i++) {
+        var current = audiences[i];
+        if (current === ' ') {
+          result.push(container.join(''));
+          container = [];
+        } else {
+          container.push(current);
+        }
+      }
+      result.push(container.join(''));
+      return result;
+    },
     printACME: function(value) {
 	if (Ext.isArray(value.domains)) {
 	    value.domains = value.domains.join(';');
diff --git a/www/manager6/dc/AuthEditBase.js b/www/manager6/dc/AuthEditBase.js
index e18fbc3b..0110e191 100644
--- a/www/manager6/dc/AuthEditBase.js
+++ b/www/manager6/dc/AuthEditBase.js
@@ -14,6 +14,14 @@ Ext.define('PVE.panel.AuthBase', {
 	    delete values.port;
 	}
 
+        var audiences = [];
+        if (PVE.Parser.checkKeys(values, "audiences")) {
+          audiences = PVE.Parser.parseOpenIdAudiences(values.audiences);
+          console.log(audiences);
+          delete values.audiences;
+          values.audiences = audiences;
+        }
+
 	if (me.isCreate) {
 	    values.type = me.type;
 	}
diff --git a/www/manager6/dc/AuthEditOpenId.js b/www/manager6/dc/AuthEditOpenId.js
index 544c0de5..0f4b07a9 100644
--- a/www/manager6/dc/AuthEditOpenId.js
+++ b/www/manager6/dc/AuthEditOpenId.js
@@ -111,6 +111,15 @@ Ext.define('PVE.panel.OpenIDInputPanel', {
 		deleteEmpty: '{!isCreate}',
 	    },
 	},
+	{
+	    xtype: 'proxmoxtextfield',
+	    name: 'audiences',
+	    fieldLabel: gettext('Audiences'),
+	    submitEmpty: false,
+	    cbind: {
+		deleteEmpty: '{!isCreate}',
+	    },
+	},
     ],
 
     initComponent: function() {
@@ -123,4 +132,3 @@ Ext.define('PVE.panel.OpenIDInputPanel', {
 	me.callParent();
     },
 });
-
-- 
2.39.5



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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [pve-devel] [PATCH proxmox v2 1/1] fix #5076: Added logic to handle OIDC audiences
  2025-06-02 14:14 ` [pve-devel] [PATCH proxmox v2 1/1] fix #5076: Added logic to handle OIDC audiences Alexander Abraham
@ 2025-06-03  8:39   ` Shannon Sterz
  0 siblings, 0 replies; 5+ messages in thread
From: Shannon Sterz @ 2025-06-03  8:39 UTC (permalink / raw)
  To: Proxmox VE development discussion, Alexander Abraham

On Mon Jun 2, 2025 at 4:14 PM CEST, Alexander Abraham wrote:
> A field for OIDC audiences was added, logic to handle these audiences,
> and the audiences supplied by an OIDC IDP are validated against
> the audiences a user saves in their realm domains
> configuration.
>
> Signed-off-by: Alexander Abraham <a.abraham@proxmox.com>
> ---
>  proxmox-openid/src/lib.rs | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/proxmox-openid/src/lib.rs b/proxmox-openid/src/lib.rs
> index fe65fded..fa22638a 100644
> --- a/proxmox-openid/src/lib.rs
> +++ b/proxmox-openid/src/lib.rs
> @@ -53,6 +53,8 @@ pub struct OpenIdConfig {
>      pub prompt: Option<String>,
>      #[serde(skip_serializing_if = "Option::is_none")]
>      pub acr_values: Option<Vec<String>>,
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub audiences: Option<Vec<String>>,

this patch doesn't seem to apply anymore. also is there a reason this
couldn't be:

    #[serde(skip_serializing_if = "Vec::is_empty")]
    pub audiences: Vec<String>,

>  }
>
>  pub struct OpenIdAuthenticator {
> @@ -205,12 +207,26 @@ impl OpenIdAuthenticator {
>              .request(http_client)
>              .map_err(|err| format_err!("Failed to contact token endpoint: {}", err))?;
>
> -        let id_token_verifier: CoreIdTokenVerifier = self.client.id_token_verifier();
>          let id_token_claims: &CoreIdTokenClaims = token_response
>              .extra_fields()
>              .id_token()
>              .expect("Server did not return an ID token")
> -            .claims(&id_token_verifier, &private_auth_state.nonce)
> +            .claims(
> +                &((self.client.id_token_verifier())
> +                    .require_audience_match(true)
> +                    .set_other_audience_verifier_fn(|aud| {
> +                        let curr_aud: &String = aud;
> +                        if &self.config.client_id == curr_aud {
> +                            true
> +                        } else {
> +                            match self.config.audiences.as_ref() {
> +                                Some(confd_auds) => confd_auds.contains(curr_aud),
> +                                None => false,
> +                            }

then this could simply be:

self.config.audiences.contains(curr_aud)

> +                        }
> +                    })),
> +                &private_auth_state.nonce,
> +            )
>              .map_err(|err| format_err!("Failed to verify ID token: {}", err))?;
>
>          let userinfo_claims: GenericUserInfoClaims = self



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


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-06-03  8:39 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-06-02 14:14 [pve-devel] [PATCH access-control-5076/manager/proxmox-5076 v2 0/3] fix #5076: Added Open ID audiences Alexander Abraham
2025-06-02 14:14 ` [pve-devel] [PATCH proxmox v2 1/1] fix #5076: Added logic to handle OIDC audiences Alexander Abraham
2025-06-03  8:39   ` Shannon Sterz
2025-06-02 14:14 ` [pve-devel] [PATCH pve-access-control v2 1/1] fix #5076: Changed audiences to an array Alexander Abraham
2025-06-02 14:14 ` [pve-devel] [PATCH pve-manager v2 1/1] fix #5076: Added an "audiences" field for Open ID Alexander Abraham

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