[pbs-devel] [PATCH proxmox] fix #6913: auth-api: fix user ID parsing for 2-character realms
Samuel Rufinatscha
s.rufinatscha at proxmox.com
Tue Nov 11 14:49:39 CET 2025
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 at 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 at 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 at 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 at 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 at a".parse();
>> + let valid_two_char_realm: Result<Userid, _> = "user at aa".parse();
>> + let valid_long_realm: Result<Userid, _> = "user at 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 at lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
More information about the pbs-devel
mailing list