Re: [pdm-devel] [PATCH proxmox v4 06/21] auth-api: introduce new CreateTicket and CreateTickeReponse api types

["Shannon Sterz" <s.sterz@proxmox.com>]

On Fri Mar 7, 2025 at 11:06 AM CET, Maximiliano Sandoval wrote:
>
> Wolfgang Bumiller <w.bumiller@proxmox.com> writes:
>
>> Sorry for missing this but...
>>
>> On Tue, Mar 04, 2025 at 01:04:51PM +0100, Shannon Sterz wrote:
>>> these types are used for creating a ticket and responding to a new
>>> ticket request.
>>>
>>> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
>>> ---
>>>  proxmox-auth-api/src/types.rs | 56 ++++++++++++++++++++++++++++++++++-
>>>  1 file changed, 55 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/proxmox-auth-api/src/types.rs b/proxmox-auth-api/src/types.rs
>>> index 64c580a5..81c43ab6 100644
>>> --- a/proxmox-auth-api/src/types.rs
>>> +++ b/proxmox-auth-api/src/types.rs
>>> @@ -417,7 +417,7 @@ impl<'a> TryFrom<&'a str> for &'a TokennameRef {
>>>  }
>>>
>>>  /// A complete user id consisting of a user name and a realm
>>> -#[derive(Clone, Debug, PartialEq, Eq, Hash, Ord, PartialOrd, UpdaterType)]
>>> +#[derive(Clone, Debug, Default, PartialEq, Eq, Hash, Ord, PartialOrd, UpdaterType)]
>>
>> ^ NAK
>>
>> An empty string is not a valid Userid, so we cannot derive `Default`
>> here.
>> There's no such thing as a "default" user id.
>>
>>>  pub struct Userid {
>>>      data: String,
>>>      name_len: usize,
>>> @@ -676,6 +676,60 @@ impl TryFrom<String> for Authid {
>>>      }
>>>  }
>>>
>>> +#[api]
>>> +/// The parameter object for creating new ticket.
>>> +#[derive(Debug, Default, Deserialize, Serialize)]
>>
>> ^ For this
>>
>>> +pub struct CreateTicket {
>>> +    /// User name
>>> +    pub username: Userid,
>>> +
>>> +    /// The secret password. This can also be a valid ticket. Only optional if the ticket is
>>> +    /// provided in a cookie header and only if the endpoint supports this.
>>> +    #[serde(default)]
>>> +    pub password: Option<String>,
>>> +
>>> +    /// Verify ticket, and check if user have access 'privs' on 'path'.
>>> +    #[serde(default, skip_serializing_if = "Option::is_none")]
>>> +    pub path: Option<String>,
>>> +
>>> +    /// Verify ticket, and check if user have access 'privs' on 'path'.
>>> +    #[serde(default, skip_serializing_if = "Option::is_none")]
>>> +    pub privs: Option<String>,
>>> +
>>> +    /// Port for verifying terminal tickets.
>>> +    #[serde(default, skip_serializing_if = "Option::is_none")]
>>> +    pub port: Option<u16>,
>>> +
>>> +    /// The signed TFA challenge string the user wants to respond to.
>>> +    #[serde(default, skip_serializing_if = "Option::is_none")]
>>> +    #[serde(rename = "tfa-challenge")]
>>> +    pub tfa_challenge: Option<String>,
>>> +}
>>> +
>>> +#[api]
>>> +/// The API response for a ticket call.
>>> +#[derive(Debug, Default, Deserialize, Serialize)]
>>
>> ... and this^
>>
>> if we need a convenient way to build a struct with "the rest set to
>> None", just add a method `fn new(username) -> Self` which you use in
>> place of any `..Default::default()` later on.
>
> `Self::new` with parameters trips clippy iirc. Generally it would be better to use
> `Self::with_username(username: String).`

not the case for me and this has been applied by now, so you should be
able to test that by running clippy in the proxmox repo.

clippy does trip up on `new()` without parameters if `Default` isn't
implemented, because there `Default` is useful to have.

>
>>
>>> +pub struct CreateTicketResponse {
>>> +    /// The CSRF prevention token.
>>> +    #[serde(default, skip_serializing_if = "Option::is_none")]
>>> +    #[serde(rename = "CSRFPreventionToken")]
>>> +    pub csrfprevention_token: Option<String>,
>>> +
>>> +    /// The ticket as is supposed to be used in the authentication header. Not provided here if the
>>> +    /// endpoint uses HttpOnly cookies to supply the actual ticket.
>>> +    #[serde(default, skip_serializing_if = "Option::is_none")]
>>> +    pub ticket: Option<String>,
>>> +
>>> +    /// Like a full ticket, except the signature is missing. Useful in HttpOnly-contexts
>>> +    /// (browsers).
>>> +    #[serde(default, skip_serializing_if = "Option::is_none")]
>>> +    #[serde(rename = "ticket-info")]
>>> +    pub ticket_info: Option<String>,
>>> +
>>> +    /// The userid.
>>> +    pub username: Userid,
>>> +}
>>> +
>>>  #[test]
>>>  fn test_token_id() {
>>>      let userid: Userid = "test@pam".parse().expect("parsing Userid failed");
>>> --
>>> 2.39.5
>>
>>
>> _______________________________________________
>> pdm-devel mailing list
>> pdm-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel



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