* [pbs-devel] [PATCH proxmox] fix #6913: auth-api: fix user ID parsing for 2-character realms
@ 2025-11-03 16:26 Samuel Rufinatscha
2025-11-11 10:40 ` Fabian Grünbichler
2025-11-14 10:34 ` [pbs-devel] applied: " Fabian Grünbichler
0 siblings, 2 replies; 4+ messages in thread
From: Samuel Rufinatscha @ 2025-11-03 16:26 UTC (permalink / raw)
To: pbs-devel
PVE and PBS both allow creating realms with names of length ≥ 2.
However, when creating a user, PBS rejected realms with 2 characters
(e.g. `test@aa`), while PVE accepted them. This issue was reported
in our bug tracker [1]. Since the issue appears in the underlying
`proxmox/proxmox-auth-api` crate, also PDM userid handling is
affected.
The issue is caused by a mismatch between realm creation and parsing
rules in `proxmox/proxmox-auth-api`. `REALM_ID_SCHEMA` allows
min_length(2), but `PROXMOX_AUTH_REALM_STRING_SCHEMA` enforced
min_length(3).
This patch lowers the minimum realm length in
`PROXMOX_AUTH_REALM_STRING_SCHEMA` from 3 to 2 to align PBS and PMG
with PVE.
## Testing
Please see the attached unit tests.
The changes were further verified using a rebuilt PBS .deb
deployment. PDM was tested using a non-package binary through the
provided client CLI.
## Maintainer notes:
Bump the `proxmox-auth-api` dependency, no breaking change.
PBS and PDM to use the new dependency.
[1] Bugzilla: https://bugzilla.proxmox.com/show_bug.cgi?id=6913
Fixes: #6913
Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
---
proxmox-auth-api/src/types.rs | 68 ++++++++++++++++++++++++++++++++++-
1 file changed, 67 insertions(+), 1 deletion(-)
diff --git a/proxmox-auth-api/src/types.rs b/proxmox-auth-api/src/types.rs
index 9bde661c..aa09fb93 100644
--- a/proxmox-auth-api/src/types.rs
+++ b/proxmox-auth-api/src/types.rs
@@ -95,7 +95,7 @@ pub const PROXMOX_GROUP_ID_SCHEMA: Schema = StringSchema::new("Group ID")
pub const PROXMOX_AUTH_REALM_STRING_SCHEMA: StringSchema =
StringSchema::new("Authentication domain ID")
.format(&proxmox_schema::api_types::SAFE_ID_FORMAT)
- .min_length(3)
+ .min_length(2)
.max_length(32);
pub const PROXMOX_AUTH_REALM_SCHEMA: Schema = PROXMOX_AUTH_REALM_STRING_SCHEMA.schema();
@@ -769,6 +769,72 @@ fn test_token_id() {
assert_eq!(auth_id.to_string(), "test@pam!bar".to_string());
}
+#[test]
+fn test_realm_validation() {
+ let empty_realm: Result<Realm, _> = "".to_string().try_into();
+ let one_char_realm: Result<Realm, _> = "a".to_string().try_into();
+ let two_char_realm: Result<Realm, _> = "aa".to_string().try_into();
+ let long_realm: Result<Realm, _> = "a".repeat(33).try_into();
+ let valid_realm: Result<Realm, _> = "pam".to_string().try_into();
+
+ assert!(empty_realm.is_err(), "Empty realm should fail validation");
+ assert!(
+ one_char_realm.is_err(),
+ "1-char realm should fail validation"
+ );
+ assert!(
+ two_char_realm.is_ok(),
+ "2-char realm should pass validation"
+ );
+ assert!(valid_realm.is_ok(), "Typical realm should pass validation");
+ assert!(
+ long_realm.is_err(),
+ "Realm >32 chars should fail validation"
+ );
+}
+
+#[test]
+fn test_userid_validation() {
+ let empty_str: Result<Userid, _> = "".parse();
+ let invalid_no_realm: Result<Userid, _> = "user".parse();
+ let invalid_empty_realm: Result<Userid, _> = "user@".parse();
+ let invalid_one_char_realm: Result<Userid, _> = "user@a".parse();
+ let valid_two_char_realm: Result<Userid, _> = "user@aa".parse();
+ let valid_long_realm: Result<Userid, _> = "user@pam".parse();
+ let invalid_long_realm: Result<Userid, _> = format!("user@{}", "a".repeat(33)).parse();
+ let invalid_empty_username: Result<Userid, _> = "@aa".parse();
+
+ assert!(empty_str.is_err(), "Empty userid should fail");
+ assert!(
+ invalid_no_realm.is_err(),
+ "Userid without realm should fail"
+ );
+ assert!(
+ invalid_empty_realm.is_err(),
+ "Userid with empty realm should fail"
+ );
+ assert!(
+ invalid_one_char_realm.is_err(),
+ "Userid with 1-char realm should fail"
+ );
+ assert!(
+ valid_two_char_realm.is_ok(),
+ "Userid with 2-char realm should pass"
+ );
+ assert!(
+ valid_long_realm.is_ok(),
+ "Userid with normal realm should pass"
+ );
+ assert!(
+ invalid_long_realm.is_err(),
+ "Userid with realm >32 chars should fail"
+ );
+ assert!(
+ invalid_empty_username.is_err(),
+ "Userid with empty username should fail"
+ );
+}
+
serde_plain::derive_deserialize_from_fromstr!(Userid, "valid user id");
serde_plain::derive_serialize_from_display!(Userid);
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pbs-devel] [PATCH proxmox] fix #6913: auth-api: fix user ID parsing for 2-character realms
2025-11-03 16:26 [pbs-devel] [PATCH proxmox] fix #6913: auth-api: fix user ID parsing for 2-character realms Samuel Rufinatscha
@ 2025-11-11 10:40 ` Fabian Grünbichler
2025-11-11 13:49 ` Samuel Rufinatscha
2025-11-14 10:34 ` [pbs-devel] applied: " Fabian Grünbichler
1 sibling, 1 reply; 4+ messages in thread
From: Fabian Grünbichler @ 2025-11-11 10:40 UTC (permalink / raw)
To: Proxmox Backup Server development discussion
On November 3, 2025 5:26 pm, Samuel Rufinatscha wrote:
> PVE and PBS both allow creating realms with names of length ≥ 2.
> However, when creating a user, PBS rejected realms with 2 characters
> (e.g. `test@aa`), while PVE accepted them. This issue was reported
> in our bug tracker [1]. Since the issue appears in the underlying
> `proxmox/proxmox-auth-api` crate, also PDM userid handling is
> affected.
>
> The issue is caused by a mismatch between realm creation and parsing
> rules in `proxmox/proxmox-auth-api`. `REALM_ID_SCHEMA` allows
> min_length(2), but `PROXMOX_AUTH_REALM_STRING_SCHEMA` enforced
> min_length(3).
>
> This patch lowers the minimum realm length in
> `PROXMOX_AUTH_REALM_STRING_SCHEMA` from 3 to 2 to align PBS and PMG
> with PVE.
>
> ## Testing
>
> Please see the attached unit tests.
> The changes were further verified using a rebuilt PBS .deb
> deployment. PDM was tested using a non-package binary through the
> provided client CLI.
>
> ## Maintainer notes:
>
> Bump the `proxmox-auth-api` dependency, no breaking change.
> PBS and PDM to use the new dependency.
this part here we'd usually put into the patch notes (the part below the
`---`), which doesn't show up in git history. you can manage those notes
using `git notes ..`, including (if you set your config accordingly),
preserving/merging them across rebases.
Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
>
> [1] Bugzilla: https://bugzilla.proxmox.com/show_bug.cgi?id=6913
>
> Fixes: #6913
> Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
> ---
> proxmox-auth-api/src/types.rs | 68 ++++++++++++++++++++++++++++++++++-
> 1 file changed, 67 insertions(+), 1 deletion(-)
>
> diff --git a/proxmox-auth-api/src/types.rs b/proxmox-auth-api/src/types.rs
> index 9bde661c..aa09fb93 100644
> --- a/proxmox-auth-api/src/types.rs
> +++ b/proxmox-auth-api/src/types.rs
> @@ -95,7 +95,7 @@ pub const PROXMOX_GROUP_ID_SCHEMA: Schema = StringSchema::new("Group ID")
> pub const PROXMOX_AUTH_REALM_STRING_SCHEMA: StringSchema =
> StringSchema::new("Authentication domain ID")
> .format(&proxmox_schema::api_types::SAFE_ID_FORMAT)
> - .min_length(3)
> + .min_length(2)
> .max_length(32);
> pub const PROXMOX_AUTH_REALM_SCHEMA: Schema = PROXMOX_AUTH_REALM_STRING_SCHEMA.schema();
>
> @@ -769,6 +769,72 @@ fn test_token_id() {
> assert_eq!(auth_id.to_string(), "test@pam!bar".to_string());
> }
>
> +#[test]
> +fn test_realm_validation() {
> + let empty_realm: Result<Realm, _> = "".to_string().try_into();
> + let one_char_realm: Result<Realm, _> = "a".to_string().try_into();
> + let two_char_realm: Result<Realm, _> = "aa".to_string().try_into();
> + let long_realm: Result<Realm, _> = "a".repeat(33).try_into();
> + let valid_realm: Result<Realm, _> = "pam".to_string().try_into();
> +
> + assert!(empty_realm.is_err(), "Empty realm should fail validation");
> + assert!(
> + one_char_realm.is_err(),
> + "1-char realm should fail validation"
> + );
> + assert!(
> + two_char_realm.is_ok(),
> + "2-char realm should pass validation"
> + );
> + assert!(valid_realm.is_ok(), "Typical realm should pass validation");
> + assert!(
> + long_realm.is_err(),
> + "Realm >32 chars should fail validation"
> + );
> +}
> +
> +#[test]
> +fn test_userid_validation() {
> + let empty_str: Result<Userid, _> = "".parse();
> + let invalid_no_realm: Result<Userid, _> = "user".parse();
> + let invalid_empty_realm: Result<Userid, _> = "user@".parse();
> + let invalid_one_char_realm: Result<Userid, _> = "user@a".parse();
> + let valid_two_char_realm: Result<Userid, _> = "user@aa".parse();
> + let valid_long_realm: Result<Userid, _> = "user@pam".parse();
> + let invalid_long_realm: Result<Userid, _> = format!("user@{}", "a".repeat(33)).parse();
> + let invalid_empty_username: Result<Userid, _> = "@aa".parse();
> +
> + assert!(empty_str.is_err(), "Empty userid should fail");
> + assert!(
> + invalid_no_realm.is_err(),
> + "Userid without realm should fail"
> + );
> + assert!(
> + invalid_empty_realm.is_err(),
> + "Userid with empty realm should fail"
> + );
> + assert!(
> + invalid_one_char_realm.is_err(),
> + "Userid with 1-char realm should fail"
> + );
> + assert!(
> + valid_two_char_realm.is_ok(),
> + "Userid with 2-char realm should pass"
> + );
> + assert!(
> + valid_long_realm.is_ok(),
> + "Userid with normal realm should pass"
> + );
> + assert!(
> + invalid_long_realm.is_err(),
> + "Userid with realm >32 chars should fail"
> + );
> + assert!(
> + invalid_empty_username.is_err(),
> + "Userid with empty username should fail"
> + );
> +}
these two are more or less tests validating our schema deserializer, but
as the types are rather core types they also don't hurt.
AFAICT we don't have in-depth tests in proxmox-schema that verify that
the schema constraints validation actually works as expected, there's
just some basic tests for query parameter handling and schema types
themselves - might be an area worth improving ;)
> +
> serde_plain::derive_deserialize_from_fromstr!(Userid, "valid user id");
> serde_plain::derive_serialize_from_display!(Userid);
>
> --
> 2.47.3
>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pbs-devel] [PATCH proxmox] fix #6913: auth-api: fix user ID parsing for 2-character realms
2025-11-11 10:40 ` Fabian Grünbichler
@ 2025-11-11 13:49 ` Samuel Rufinatscha
0 siblings, 0 replies; 4+ messages in thread
From: Samuel Rufinatscha @ 2025-11-11 13:49 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Fabian Grünbichler
On 11/11/25 11:40 AM, Fabian Grünbichler wrote:
> On November 3, 2025 5:26 pm, Samuel Rufinatscha wrote:
>> PVE and PBS both allow creating realms with names of length ≥ 2.
>> However, when creating a user, PBS rejected realms with 2 characters
>> (e.g. `test@aa`), while PVE accepted them. This issue was reported
>> in our bug tracker [1]. Since the issue appears in the underlying
>> `proxmox/proxmox-auth-api` crate, also PDM userid handling is
>> affected.
>>
>> The issue is caused by a mismatch between realm creation and parsing
>> rules in `proxmox/proxmox-auth-api`. `REALM_ID_SCHEMA` allows
>> min_length(2), but `PROXMOX_AUTH_REALM_STRING_SCHEMA` enforced
>> min_length(3).
>>
>> This patch lowers the minimum realm length in
>> `PROXMOX_AUTH_REALM_STRING_SCHEMA` from 3 to 2 to align PBS and PMG
>> with PVE.
>>
>> ## Testing
>>
>> Please see the attached unit tests.
>> The changes were further verified using a rebuilt PBS .deb
>> deployment. PDM was tested using a non-package binary through the
>> provided client CLI.
>>
>> ## Maintainer notes:
>>
>> Bump the `proxmox-auth-api` dependency, no breaking change.
>> PBS and PDM to use the new dependency.
>
> this part here we'd usually put into the patch notes (the part below the
> `---`), which doesn't show up in git history. you can manage those notes
> using `git notes ..`, including (if you set your config accordingly),
> preserving/merging them across rebases.
>
> Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
>
Thanks for the review Fabian - makes absolutely sense! I will keep this
in mind for my future patches.
>>
>> [1] Bugzilla: https://bugzilla.proxmox.com/show_bug.cgi?id=6913
>>
>> Fixes: #6913
>> Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
>> ---
>> proxmox-auth-api/src/types.rs | 68 ++++++++++++++++++++++++++++++++++-
>> 1 file changed, 67 insertions(+), 1 deletion(-)
>>
>> diff --git a/proxmox-auth-api/src/types.rs b/proxmox-auth-api/src/types.rs
>> index 9bde661c..aa09fb93 100644
>> --- a/proxmox-auth-api/src/types.rs
>> +++ b/proxmox-auth-api/src/types.rs
>> @@ -95,7 +95,7 @@ pub const PROXMOX_GROUP_ID_SCHEMA: Schema = StringSchema::new("Group ID")
>> pub const PROXMOX_AUTH_REALM_STRING_SCHEMA: StringSchema =
>> StringSchema::new("Authentication domain ID")
>> .format(&proxmox_schema::api_types::SAFE_ID_FORMAT)
>> - .min_length(3)
>> + .min_length(2)
>> .max_length(32);
>> pub const PROXMOX_AUTH_REALM_SCHEMA: Schema = PROXMOX_AUTH_REALM_STRING_SCHEMA.schema();
>>
>> @@ -769,6 +769,72 @@ fn test_token_id() {
>> assert_eq!(auth_id.to_string(), "test@pam!bar".to_string());
>> }
>>
>> +#[test]
>> +fn test_realm_validation() {
>> + let empty_realm: Result<Realm, _> = "".to_string().try_into();
>> + let one_char_realm: Result<Realm, _> = "a".to_string().try_into();
>> + let two_char_realm: Result<Realm, _> = "aa".to_string().try_into();
>> + let long_realm: Result<Realm, _> = "a".repeat(33).try_into();
>> + let valid_realm: Result<Realm, _> = "pam".to_string().try_into();
>> +
>> + assert!(empty_realm.is_err(), "Empty realm should fail validation");
>> + assert!(
>> + one_char_realm.is_err(),
>> + "1-char realm should fail validation"
>> + );
>> + assert!(
>> + two_char_realm.is_ok(),
>> + "2-char realm should pass validation"
>> + );
>> + assert!(valid_realm.is_ok(), "Typical realm should pass validation");
>> + assert!(
>> + long_realm.is_err(),
>> + "Realm >32 chars should fail validation"
>> + );
>> +}
>> +
>> +#[test]
>> +fn test_userid_validation() {
>> + let empty_str: Result<Userid, _> = "".parse();
>> + let invalid_no_realm: Result<Userid, _> = "user".parse();
>> + let invalid_empty_realm: Result<Userid, _> = "user@".parse();
>> + let invalid_one_char_realm: Result<Userid, _> = "user@a".parse();
>> + let valid_two_char_realm: Result<Userid, _> = "user@aa".parse();
>> + let valid_long_realm: Result<Userid, _> = "user@pam".parse();
>> + let invalid_long_realm: Result<Userid, _> = format!("user@{}", "a".repeat(33)).parse();
>> + let invalid_empty_username: Result<Userid, _> = "@aa".parse();
>> +
>> + assert!(empty_str.is_err(), "Empty userid should fail");
>> + assert!(
>> + invalid_no_realm.is_err(),
>> + "Userid without realm should fail"
>> + );
>> + assert!(
>> + invalid_empty_realm.is_err(),
>> + "Userid with empty realm should fail"
>> + );
>> + assert!(
>> + invalid_one_char_realm.is_err(),
>> + "Userid with 1-char realm should fail"
>> + );
>> + assert!(
>> + valid_two_char_realm.is_ok(),
>> + "Userid with 2-char realm should pass"
>> + );
>> + assert!(
>> + valid_long_realm.is_ok(),
>> + "Userid with normal realm should pass"
>> + );
>> + assert!(
>> + invalid_long_realm.is_err(),
>> + "Userid with realm >32 chars should fail"
>> + );
>> + assert!(
>> + invalid_empty_username.is_err(),
>> + "Userid with empty username should fail"
>> + );
>> +}
>
> these two are more or less tests validating our schema deserializer, but
> as the types are rather core types they also don't hurt.
>
> AFAICT we don't have in-depth tests in proxmox-schema that verify that
> the schema constraints validation actually works as expected, there's
> just some basic tests for query parameter handling and schema types
> themselves - might be an area worth improving ;)
>
Good point! Having tests for the schema constraints would be a great
follow-up and probably good-to-have, also we could move these tests
then.
>> +
>> serde_plain::derive_deserialize_from_fromstr!(Userid, "valid user id");
>> serde_plain::derive_serialize_from_display!(Userid);
>>
>> --
>> 2.47.3
>>
>>
>>
>> _______________________________________________
>> pbs-devel mailing list
>> pbs-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* [pbs-devel] applied: [PATCH proxmox] fix #6913: auth-api: fix user ID parsing for 2-character realms
2025-11-03 16:26 [pbs-devel] [PATCH proxmox] fix #6913: auth-api: fix user ID parsing for 2-character realms Samuel Rufinatscha
2025-11-11 10:40 ` Fabian Grünbichler
@ 2025-11-14 10:34 ` Fabian Grünbichler
1 sibling, 0 replies; 4+ messages in thread
From: Fabian Grünbichler @ 2025-11-14 10:34 UTC (permalink / raw)
To: pbs-devel, Samuel Rufinatscha
On Mon, 03 Nov 2025 17:26:05 +0100, Samuel Rufinatscha wrote:
> PVE and PBS both allow creating realms with names of length ≥ 2.
> However, when creating a user, PBS rejected realms with 2 characters
> (e.g. `test@aa`), while PVE accepted them. This issue was reported
> in our bug tracker [1]. Since the issue appears in the underlying
> `proxmox/proxmox-auth-api` crate, also PDM userid handling is
> affected.
>
> [...]
Applied, thanks!
[1/1] fix #6913: auth-api: fix user ID parsing for 2-character realms
commit: 2ac004ec5516beb93000544bb709dccd8c48f116
Best regards,
--
Fabian Grünbichler <f.gruenbichler@proxmox.com>
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-11-14 10:34 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-03 16:26 [pbs-devel] [PATCH proxmox] fix #6913: auth-api: fix user ID parsing for 2-character realms Samuel Rufinatscha
2025-11-11 10:40 ` Fabian Grünbichler
2025-11-11 13:49 ` Samuel Rufinatscha
2025-11-14 10:34 ` [pbs-devel] applied: " Fabian Grünbichler
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.