From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pdm-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
	by lore.proxmox.com (Postfix) with ESMTPS id 5B3F11FF176
	for <inbox@lore.proxmox.com>; Fri,  7 Mar 2025 11:14:17 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 1E7EC181D7;
	Fri,  7 Mar 2025 11:14:12 +0100 (CET)
Mime-Version: 1.0
Date: Fri, 07 Mar 2025 11:14:09 +0100
Message-Id: <D89YDPLUTWMP.3AHC2HZMZR69Y@proxmox.com>
From: "Shannon Sterz" <s.sterz@proxmox.com>
To: "Maximiliano Sandoval" <m.sandoval@proxmox.com>, "Proxmox Datacenter
 Manager development discussion" <pdm-devel@lists.proxmox.com>
X-Mailer: aerc 0.20.1-0-g2ecb8770224a-dirty
References: <20250304120506.135617-1-s.sterz@proxmox.com>
 <20250304120506.135617-7-s.sterz@proxmox.com>
 <6qccylnkdjmctkmekjuprhdrwukgm7ekki756jh4dtlalfthjf@pe24qyc432tx>
 <s8oikol40ig.fsf@proxmox.com>
In-Reply-To: <s8oikol40ig.fsf@proxmox.com>
X-SPAM-LEVEL: Spam detection results:  0
 AWL -1.289 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 ENA_SUBJ_ODD_CASE         2.6 Subject has odd case
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
 http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
 information. [proxmox.com]
Subject: Re: [pdm-devel] [PATCH proxmox v4 06/21] auth-api: introduce new
 CreateTicket and CreateTickeReponse api types
X-BeenThere: pdm-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Datacenter Manager development discussion
 <pdm-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pdm-devel>, 
 <mailto:pdm-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pdm-devel/>
List-Post: <mailto:pdm-devel@lists.proxmox.com>
List-Help: <mailto:pdm-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel>, 
 <mailto:pdm-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox Datacenter Manager development discussion
 <pdm-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pdm-devel-bounces@lists.proxmox.com
Sender: "pdm-devel" <pdm-devel-bounces@lists.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