From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 8B01B69C5E for ; Wed, 15 Sep 2021 12:31:58 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 7735517A17 for ; Wed, 15 Sep 2021 12:31:28 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 7FAA817A09 for ; Wed, 15 Sep 2021 12:31:27 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 4336E448A8 for ; Wed, 15 Sep 2021 12:31:27 +0200 (CEST) Message-ID: <91fa308c-620d-ac62-875f-4a89d39048f1@proxmox.com> Date: Wed, 15 Sep 2021 12:30:46 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:93.0) Gecko/20100101 Thunderbird/93.0 Content-Language: en-US To: Proxmox Backup Server development discussion , Dominik Csapak References: <20210913141829.2171301-1-d.csapak@proxmox.com> <20210913141829.2171301-5-d.csapak@proxmox.com> From: Thomas Lamprecht In-Reply-To: <20210913141829.2171301-5-d.csapak@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 1.252 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -1.969 Looks like a legit reply (A) 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. [user.rs] Subject: Re: [pbs-devel] [PATCH proxmox-backup v2 3/7] api2: add missing token list match_all property X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 15 Sep 2021 10:31:58 -0000 s/api2/api/ please. On 13.09.21 16:18, Dominik Csapak wrote: > to have the proper link between the token list and the sub routes > in the api, include the 'tokenname' property in the token listing > this sounds like that the link did not work or the like, but that is not exactly the case, isn't it? Can we please state what provoked this patch here. > Signed-off-by: Dominik Csapak > --- > src/api2/access/user.rs | 35 ++++++++++++++++++++++++++++------- > 1 file changed, 28 insertions(+), 7 deletions(-) > > diff --git a/src/api2/access/user.rs b/src/api2/access/user.rs > index 75071cf1..97c336ab 100644 > --- a/src/api2/access/user.rs > +++ b/src/api2/access/user.rs > @@ -655,6 +655,21 @@ pub fn delete_token( > Ok(()) > } > > +#[api( > + properties: { > + tokenname: { type: Tokenname }, we normally try to use kebab case, or is it not possible to use "token-name" here? > + token: { type: ApiToken }, > + } > +)] > +#[derive(Serialize, Deserialize)] > +/// A Token Entry of a user that doc-comment is "meh" at best, casing and info is lacking and not sure if "of an user" is relevant here. I'd mention that the token name is already encoded and explicitly split out here for API's sake. > +pub struct TokenInfo { I'd find some merit in renaming this to clarify that it is rather a API specific type that does not provides more info but just more explicit, that the original type is named "ApiToken" doesn't make this easier though, and atm. I have no good proposal, sorry, so just putting that opinion out there. > + /// The Token name > + pub tokenname: Tokenname, > + #[serde(flatten)] > + pub token: ApiToken, > +} > + > #[api( > input: { > properties: { > @@ -666,7 +681,7 @@ pub fn delete_token( > returns: { > description: "List user's API tokens (with config digest).", > type: Array, > - items: { type: ApiToken }, > + items: { type: TokenInfo }, > }, > access: { > permission: &Permission::Or(&[ > @@ -680,7 +695,7 @@ pub fn list_tokens( > userid: Userid, > _info: &ApiMethod, > mut rpcenv: &mut dyn RpcEnvironment, > -) -> Result, Error> { > +) -> Result, Error> { > > let (config, digest) = pbs_config::user::config()?; > > @@ -688,15 +703,21 @@ pub fn list_tokens( > > rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into(); > > - let filter_by_owner = |token: &ApiToken| { > - if token.tokenid.is_token() { > - token.tokenid.user() == &userid > + let filter_by_owner = |token: ApiToken| { > + if token.tokenid.is_token() && token.tokenid.user() == &userid { > + let tokenname = token.tokenid.tokenname().unwrap().to_owned(); I'd like to try adding comments about why unwrap is safe. A panic possibility is always a bit of a bummer, so showing that one thought about that and recording that in a comment would be nice. > + Some(TokenInfo { > + tokenname, > + token, > + }) > } else { > - false > + None > } > }; > > - Ok(list.into_iter().filter(filter_by_owner).collect()) > + let res = list.into_iter().filter_map(filter_by_owner).collect(); > + > + Ok(res) nit: why adding a (useless?) intermediated `res` variable in above change? > } > > const TOKEN_ITEM_ROUTER: Router = Router::new() >