Re: [pbs-devel] [PATCH proxmox] fix #6913: auth-api: fix user ID parsing for 2-character realms

[Samuel Rufinatscha <s.rufinatscha@proxmox.com>]

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