[pmg-devel] [PATCH pmg-api/pwt 0/2] fix #6214: improve OIDC field validation

[pmg-devel] [PATCH pmg-api/pwt 0/2] fix #6214: improve OIDC field validation

[Christoph Heiss]

First and foremost, fixes #6214 [0].

The second patch just adds some more validation on the UI side of
things. While not strictly related to #6214, it's a nice touch.

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=6214

pmg-api:

Christoph Heiss (1):
  auth: oidc: fix pattern for `client-id` and `client-key`

 src/PMG/Auth/OIDC.pm | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

proxmox-widget-toolkit:

Christoph Heiss (1):
  window: AuthEditOpenId: add validation for some required fields

 src/Utils.js                 | 4 ++++
 src/window/AuthEditOpenId.js | 8 ++++++++
 2 files changed, 12 insertions(+)

-- 
2.48.1



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

[pmg-devel] [PATCH pmg-api 1/2] auth: oidc: fix pattern for `client-id` and `client-key`

[Christoph Heiss]

Fixes #6214 [0].

As per RFC 6749 Appendix A [1], `client-id` and `client-key` can contain
any printable ascii character, i.e. anything in the (inclusive) range
0x20-0x7E.

Reflect that in the pattern.

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=6214
[1] https://www.rfc-editor.org/rfc/rfc6749#appendix-A

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 src/PMG/Auth/OIDC.pm | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/PMG/Auth/OIDC.pm b/src/PMG/Auth/OIDC.pm
index b2b1a05..5f797d1 100755
--- a/src/PMG/Auth/OIDC.pm
+++ b/src/PMG/Auth/OIDC.pm
@@ -61,18 +61,22 @@ sub properties {
 	    maxLength => 256,
 	    pattern => qr/^(https?):\/\/([a-zA-Z0-9.-]+)(:[0-9]{1,5})?(\/[^\s]*)?$/,
 	},
+	# See RFC 6749, Appendix A for the allowed pattern for `client-id` and
+	# `client-key`.
+	# https://www.rfc-editor.org/rfc/rfc6749#appendix-A
+	# tl;dr: anything ASCII in the (inclusive) range 0x20-0x7E
 	'client-id' => {
 	    description => "OpenID Connect Client ID",
 	    type => 'string',
 	    maxLength => 256,
-	    pattern => qr/^[a-zA-Z0-9._:-]+$/,
+	    pattern => qr/^[\x20-\x7E]+$/,
 	},
 	'client-key' => {
 	    description => "OpenID Connect Client Key",
 	    type => 'string',
 	    optional => 1,
 	    maxLength => 256,
-	    pattern => qr/^[a-zA-Z0-9._:-]+$/,
+	    pattern => qr/^[\x20-\x7E]+$/,
 	},
 	autocreate => {
 	    description => "Automatically create users if they do not exist.",
-- 
2.48.1



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

[pmg-devel] [PATCH widget-toolkit 2/2] window: AuthEditOpenId: add validation for required fields

[Christoph Heiss]

For the `client-id` and `client-secret` OIDC-specific fields, allow the
format specified in RFC 6749, Appendix A [0].

As per that, the OIDC-specific `client-id` and `client-key` can contain
any printable ascii character, i.e. anything in the (inclusive) range
0x20-0x7E.

For the issuer URL, use our existing URL validation regex, while the
realm name itself must follow authentication realm naming scheme.

[0] https://www.rfc-editor.org/rfc/rfc6749#appendix-A

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 src/Utils.js                 | 4 ++++
 src/window/AuthEditOpenId.js | 8 ++++++++
 2 files changed, 12 insertions(+)

diff --git a/src/Utils.js b/src/Utils.js
index c873c85..e17bee8 100644
--- a/src/Utils.js
+++ b/src/Utils.js
@@ -1545,6 +1545,10 @@ utilities: {
 
 	// Same as SAFE_ID_REGEX in proxmox-schema
 	me.safeIdRegex = /^(?:[A-Za-z0-9_][A-Za-z0-9._\\-]*)$/;
+
+	me.printableAsciiRegex = /^[\x20-\x7E]+$/;
+
+	me.authRealmNameRegex = /^[A-Za-z][A-Za-z0-9.\-_]+$/;
     },
 });
 
diff --git a/src/window/AuthEditOpenId.js b/src/window/AuthEditOpenId.js
index ed0a6dc..5939516 100644
--- a/src/window/AuthEditOpenId.js
+++ b/src/window/AuthEditOpenId.js
@@ -23,6 +23,8 @@ Ext.define('Proxmox.panel.OpenIDInputPanel', {
 	    name: 'issuer-url',
 	    fieldLabel: gettext('Issuer URL'),
 	    allowBlank: false,
+	    regex: Proxmox.Utils.httpUrlRegex,
+	    regexText: gettext('Must be a valid URL'),
 	},
     ],
 
@@ -36,6 +38,8 @@ Ext.define('Proxmox.panel.OpenIDInputPanel', {
 	    },
 	    fieldLabel: gettext('Realm'),
 	    allowBlank: false,
+	    regex: Proxmox.Utils.authRealmNameRegex,
+	    regexText: gettext('Must be a valid realm name'),
 	},
 	{
 	    xtype: 'proxmoxcheckbox',
@@ -57,6 +61,8 @@ Ext.define('Proxmox.panel.OpenIDInputPanel', {
 	    fieldLabel: gettext('Client ID'),
 	    name: 'client-id',
 	    allowBlank: false,
+	    regex: Proxmox.Utils.printableAsciiRegex, // RFC 6749, Appendix A
+	    regexText: gettext('Must be a valid OIDC Client ID'),
 	},
 	{
 	    xtype: 'proxmoxtextfield',
@@ -65,6 +71,8 @@ Ext.define('Proxmox.panel.OpenIDInputPanel', {
 		deleteEmpty: '{!isCreate}',
 	    },
 	    name: 'client-key',
+	    regex: Proxmox.Utils.printableAsciiRegex, // RFC 6749, Appendix A
+	    regexText: gettext('Must be a valid OIDC Client Secret'),
 	},
     ],
 
-- 
2.48.1



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

Re: [pmg-devel] [PATCH pmg-api/pwt 0/2] fix #6214: improve OIDC field validation

[Christoph Heiss]

Ping, still applies.

On Tue Mar 18, 2025 at 2:59 PM CET, Christoph Heiss wrote:
> First and foremost, fixes #6214 [0].
>
> The second patch just adds some more validation on the UI side of
> things. While not strictly related to #6214, it's a nice touch.
>
> [0] https://bugzilla.proxmox.com/show_bug.cgi?id=6214
>
> pmg-api:
>
> Christoph Heiss (1):
>   auth: oidc: fix pattern for `client-id` and `client-key`
>
>  src/PMG/Auth/OIDC.pm | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> proxmox-widget-toolkit:
>
> Christoph Heiss (1):
>   window: AuthEditOpenId: add validation for some required fields
>
>  src/Utils.js                 | 4 ++++
>  src/window/AuthEditOpenId.js | 8 ++++++++
>  2 files changed, 12 insertions(+)



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

[pmg-devel] applied: [PATCH pmg-api 1/2] auth: oidc: fix pattern for `client-id` and `client-key`

[Stoiko Ivanov]

applied this one after skimming through the RFC - thanks for the patch!


On Tue, 18 Mar 2025 14:59:17 +0100
Christoph Heiss <c.heiss@proxmox.com> wrote:

> Fixes #6214 [0].
> 
> As per RFC 6749 Appendix A [1], `client-id` and `client-key` can contain
> any printable ascii character, i.e. anything in the (inclusive) range
> 0x20-0x7E.
> 
> Reflect that in the pattern.
> 
> [0] https://bugzilla.proxmox.com/show_bug.cgi?id=6214
> [1] https://www.rfc-editor.org/rfc/rfc6749#appendix-A
> 
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
>  src/PMG/Auth/OIDC.pm | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/src/PMG/Auth/OIDC.pm b/src/PMG/Auth/OIDC.pm
> index b2b1a05..5f797d1 100755
> --- a/src/PMG/Auth/OIDC.pm
> +++ b/src/PMG/Auth/OIDC.pm
> @@ -61,18 +61,22 @@ sub properties {
>  	    maxLength => 256,
>  	    pattern => qr/^(https?):\/\/([a-zA-Z0-9.-]+)(:[0-9]{1,5})?(\/[^\s]*)?$/,
>  	},
> +	# See RFC 6749, Appendix A for the allowed pattern for `client-id` and
> +	# `client-key`.
> +	# https://www.rfc-editor.org/rfc/rfc6749#appendix-A
> +	# tl;dr: anything ASCII in the (inclusive) range 0x20-0x7E
>  	'client-id' => {
>  	    description => "OpenID Connect Client ID",
>  	    type => 'string',
>  	    maxLength => 256,
> -	    pattern => qr/^[a-zA-Z0-9._:-]+$/,
> +	    pattern => qr/^[\x20-\x7E]+$/,
>  	},
>  	'client-key' => {
>  	    description => "OpenID Connect Client Key",
>  	    type => 'string',
>  	    optional => 1,
>  	    maxLength => 256,
> -	    pattern => qr/^[a-zA-Z0-9._:-]+$/,
> +	    pattern => qr/^[\x20-\x7E]+$/,
>  	},
>  	autocreate => {
>  	    description => "Automatically create users if they do not exist.",



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

Re: [pmg-devel] [PATCH widget-toolkit 2/2] window: AuthEditOpenId: add validation for required fields

[Stoiko Ivanov]

On Tue, 18 Mar 2025 14:59:18 +0100
Christoph Heiss <c.heiss@proxmox.com> wrote:

> For the `client-id` and `client-secret` OIDC-specific fields, allow the
> format specified in RFC 6749, Appendix A [0].
> 
> As per that, the OIDC-specific `client-id` and `client-key` can contain
> any printable ascii character, i.e. anything in the (inclusive) range
> 0x20-0x7E.
> 
> For the issuer URL, use our existing URL validation regex, while the
> realm name itself must follow authentication realm naming scheme.
> 
> [0] https://www.rfc-editor.org/rfc/rfc6749#appendix-A
> 
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>

not a maintainer for widget-toolkit - but from looking through the RFC
consider this:
Reviewed-by: Stoiko Ivanov <s.ivanov@proxmox.com>

> ---
>  src/Utils.js                 | 4 ++++
>  src/window/AuthEditOpenId.js | 8 ++++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/src/Utils.js b/src/Utils.js
> index c873c85..e17bee8 100644
> --- a/src/Utils.js
> +++ b/src/Utils.js
> @@ -1545,6 +1545,10 @@ utilities: {
>  
>  	// Same as SAFE_ID_REGEX in proxmox-schema
>  	me.safeIdRegex = /^(?:[A-Za-z0-9_][A-Za-z0-9._\\-]*)$/;
> +
> +	me.printableAsciiRegex = /^[\x20-\x7E]+$/;
> +
> +	me.authRealmNameRegex = /^[A-Za-z][A-Za-z0-9.\-_]+$/;
>      },
>  });
>  
> diff --git a/src/window/AuthEditOpenId.js b/src/window/AuthEditOpenId.js
> index ed0a6dc..5939516 100644
> --- a/src/window/AuthEditOpenId.js
> +++ b/src/window/AuthEditOpenId.js
> @@ -23,6 +23,8 @@ Ext.define('Proxmox.panel.OpenIDInputPanel', {
>  	    name: 'issuer-url',
>  	    fieldLabel: gettext('Issuer URL'),
>  	    allowBlank: false,
> +	    regex: Proxmox.Utils.httpUrlRegex,
> +	    regexText: gettext('Must be a valid URL'),
>  	},
>      ],
>  
> @@ -36,6 +38,8 @@ Ext.define('Proxmox.panel.OpenIDInputPanel', {
>  	    },
>  	    fieldLabel: gettext('Realm'),
>  	    allowBlank: false,
> +	    regex: Proxmox.Utils.authRealmNameRegex,
> +	    regexText: gettext('Must be a valid realm name'),
>  	},
>  	{
>  	    xtype: 'proxmoxcheckbox',
> @@ -57,6 +61,8 @@ Ext.define('Proxmox.panel.OpenIDInputPanel', {
>  	    fieldLabel: gettext('Client ID'),
>  	    name: 'client-id',
>  	    allowBlank: false,
> +	    regex: Proxmox.Utils.printableAsciiRegex, // RFC 6749, Appendix A
> +	    regexText: gettext('Must be a valid OIDC Client ID'),
>  	},
>  	{
>  	    xtype: 'proxmoxtextfield',
> @@ -65,6 +71,8 @@ Ext.define('Proxmox.panel.OpenIDInputPanel', {
>  		deleteEmpty: '{!isCreate}',
>  	    },
>  	    name: 'client-key',
> +	    regex: Proxmox.Utils.printableAsciiRegex, // RFC 6749, Appendix A
> +	    regexText: gettext('Must be a valid OIDC Client Secret'),
>  	},
>      ],
>  



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