* [pdm-devel] [PATCH datacenter-manager/proxmox 0/6] fix #6914: add option to remove already existing token
@ 2025-12-05 18:04 Shan Shaji
2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 1/5] server: pbs-client: add delete admin token method Shan Shaji
` (6 more replies)
0 siblings, 7 replies; 24+ messages in thread
From: Shan Shaji @ 2025-12-05 18:04 UTC (permalink / raw)
To: pdm-devel
If a user removed a remote without deleting its associated token, PDM
would not allow re-adding the same remote unless the token name was
changed. To fix this, support for optionally deleting the token from
the remote has been added.
This includes:
- CLI: An optional flag to remove the token when removing the remote
- UI: A checkbox that allows users to delete the token along with the
remote
proxmox-datacenter-manager:
Shan Shaji (5):
server: pbs-client: add delete admin token method
server: api: add support to optionally delete token from remote
pdm-client: accept `delete-token` argument for deleting api token
cli: client: add `delete-token` option to delete token from remote
fix: ui: add remove confirmation dialog with optional token deletion
cli/client/src/remotes.rs | 11 ++-
lib/pdm-client/src/lib.rs | 6 +-
server/src/api/remotes.rs | 45 +++++++++++-
server/src/pbs_client.rs | 9 ++-
ui/src/remotes/config.rs | 42 +++++++----
ui/src/remotes/mod.rs | 3 +
ui/src/remotes/remove_remote.rs | 122 ++++++++++++++++++++++++++++++++
7 files changed, 217 insertions(+), 21 deletions(-)
create mode 100644 ui/src/remotes/remove_remote.rs
proxmox:
Shan Shaji (1):
pve-api-types: generate missing `delete_token` method
pve-api-types/generate.pl | 1 +
pve-api-types/src/generated/code.rs | 11 +++++++++++
2 files changed, 12 insertions(+)
Summary over all repositories:
9 files changed, 229 insertions(+), 21 deletions(-)
--
Generated by git-murpp 0.8.1
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 24+ messages in thread* [pdm-devel] [PATCH datacenter-manager 1/5] server: pbs-client: add delete admin token method 2025-12-05 18:04 [pdm-devel] [PATCH datacenter-manager/proxmox 0/6] fix #6914: add option to remove already existing token Shan Shaji @ 2025-12-05 18:04 ` Shan Shaji 2025-12-09 9:36 ` Shannon Sterz 2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 2/5] server: api: add support to optionally delete token from remote Shan Shaji ` (5 subsequent siblings) 6 siblings, 1 reply; 24+ messages in thread From: Shan Shaji @ 2025-12-05 18:04 UTC (permalink / raw) To: pdm-devel Inorder to allow deleting the generated token of PBS from PDM, add method inside the PbsClient to delete the admin token. Signed-off-by: Shan Shaji <s.shaji@proxmox.com> --- server/src/pbs_client.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/server/src/pbs_client.rs b/server/src/pbs_client.rs index f4f1f82..b1e01e1 100644 --- a/server/src/pbs_client.rs +++ b/server/src/pbs_client.rs @@ -260,7 +260,14 @@ impl PbsClient { Ok(token) } - /// Return the status the Proxmox Backup Server instance + // Delete API-Token from the PBS remote. + pub async fn delete_admin_token(&self, userid: &Userid, tokenid: &str) -> Result<(), Error> { + let path = format!("/api2/extjs/access/users/{userid}/token/{tokenid}"); + self.0.delete(&path).await?.nodata()?; + Ok(()) + } + + /// Return the status the Proxmox Backup Server instance: pub async fn node_status(&self) -> Result<pbs_api_types::NodeStatus, Error> { let path = "/api2/extjs/nodes/localhost/status"; Ok(self.0.get(path).await?.expect_json()?.data) -- 2.47.3 _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager 1/5] server: pbs-client: add delete admin token method 2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 1/5] server: pbs-client: add delete admin token method Shan Shaji @ 2025-12-09 9:36 ` Shannon Sterz 0 siblings, 0 replies; 24+ messages in thread From: Shannon Sterz @ 2025-12-09 9:36 UTC (permalink / raw) To: Shan Shaji; +Cc: Proxmox Datacenter Manager development discussion On Fri Dec 5, 2025 at 7:04 PM CET, Shan Shaji wrote: > Inorder to allow deleting the generated token of PBS from PDM, add > method inside the PbsClient to delete the admin token. > > Signed-off-by: Shan Shaji <s.shaji@proxmox.com> > --- > server/src/pbs_client.rs | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/server/src/pbs_client.rs b/server/src/pbs_client.rs > index f4f1f82..b1e01e1 100644 > --- a/server/src/pbs_client.rs > +++ b/server/src/pbs_client.rs > @@ -260,7 +260,14 @@ impl PbsClient { > Ok(token) > } > > - /// Return the status the Proxmox Backup Server instance > + // Delete API-Token from the PBS remote. nit: dropping a '/' here turns this from a doc comment to a regular comment. since this is a public function, please specify this as a doc comment. > + pub async fn delete_admin_token(&self, userid: &Userid, tokenid: &str) -> Result<(), Error> { > + let path = format!("/api2/extjs/access/users/{userid}/token/{tokenid}"); > + self.0.delete(&path).await?.nodata()?; > + Ok(()) > + } > + > + /// Return the status the Proxmox Backup Server instance: nit: any reason to add a colon here? > pub async fn node_status(&self) -> Result<pbs_api_types::NodeStatus, Error> { > let path = "/api2/extjs/nodes/localhost/status"; > Ok(self.0.get(path).await?.expect_json()?.data) _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 2/5] server: api: add support to optionally delete token from remote 2025-12-05 18:04 [pdm-devel] [PATCH datacenter-manager/proxmox 0/6] fix #6914: add option to remove already existing token Shan Shaji 2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 1/5] server: pbs-client: add delete admin token method Shan Shaji @ 2025-12-05 18:04 ` Shan Shaji 2025-12-09 9:36 ` Shannon Sterz 2025-12-09 12:34 ` Michael Köppl 2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 3/5] pdm-client: accept `delete-token` argument for deleting api token Shan Shaji ` (4 subsequent siblings) 6 siblings, 2 replies; 24+ messages in thread From: Shan Shaji @ 2025-12-05 18:04 UTC (permalink / raw) To: pdm-devel Previously, when removing a remote, the token was still present in the remote configuration. When users tried to add the remote again, they received an error because a token with the same name already existed. To support deleting the token from the remote, add an optional parameter to the API endpoint. Signed-off-by: Shan Shaji <s.shaji@proxmox.com> --- server/src/api/remotes.rs | 45 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/server/src/api/remotes.rs b/server/src/api/remotes.rs index 298ad13..9f9786c 100644 --- a/server/src/api/remotes.rs +++ b/server/src/api/remotes.rs @@ -27,6 +27,7 @@ use crate::api::remote_updates; use crate::metric_collection; use crate::{connection, pbs_client}; +use super::pbs; use super::pve; use super::rrd_common; use super::rrd_common::DataPoint; @@ -292,16 +293,56 @@ pub fn update_remote( input: { properties: { id: { schema: REMOTE_ID_SCHEMA }, + "delete-token": { + type: bool, + description: "Optional boolean value to delete the token from remote.", + optional: true, + } }, }, access: { permission: &Permission::Privilege(&["resource"], PRIV_RESOURCE_MODIFY, false), }, )] -/// List all the remotes this instance is managing. -pub fn remove_remote(id: String) -> Result<(), Error> { +/// Remove a remote that this instance is managing. +pub async fn remove_remote(id: String, delete_token: Option<bool>) -> Result<(), Error> { let (mut remotes, _) = pdm_config::remotes::config()?; + if delete_token.unwrap_or(false) { + let remote = remotes + .get_mut(&id) + .ok_or_else(|| http_err!(NOT_FOUND, "no such remote {id:?}"))?; + + let user = remote.authid.user(); + + let short_delete_err = |err: proxmox_client::Error| { + format_err!("error deleting token: {}", err.source().unwrap_or(&err)) + }; + + let token_name = remote + .authid + .tokenname() + .ok_or_else(|| format_err!("Unable to find the token for the remote {}", id))?; + + // connect to remote and delete the already existing token. + match remote.ty { + RemoteType::Pve => { + let client = pve::connect_or_login(&remote).await?; + client + .delete_token(user.as_str(), token_name.as_str()) + .await + .map_err(short_delete_err)? + } + RemoteType::Pbs => { + let client = pbs::connect_or_login(&remote).await?; + client + .delete_admin_token(&user, token_name.as_str()) + .await + .map_err(short_delete_err)? + } + }; + } + if remotes.remove(&id).is_none() { http_bail!(NOT_FOUND, "no such entry {id:?}"); } -- 2.47.3 _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager 2/5] server: api: add support to optionally delete token from remote 2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 2/5] server: api: add support to optionally delete token from remote Shan Shaji @ 2025-12-09 9:36 ` Shannon Sterz 2025-12-09 9:54 ` Shan Shaji 2025-12-09 12:34 ` Michael Köppl 1 sibling, 1 reply; 24+ messages in thread From: Shannon Sterz @ 2025-12-09 9:36 UTC (permalink / raw) To: Shan Shaji; +Cc: Proxmox Datacenter Manager development discussion On Fri Dec 5, 2025 at 7:04 PM CET, Shan Shaji wrote: > Previously, when removing a remote, the token was still present in the > remote configuration. When users tried to add the remote again, they > received an error because a token with the same name already existed. > To support deleting the token from the remote, add an optional > parameter to the API endpoint. > > Signed-off-by: Shan Shaji <s.shaji@proxmox.com> > --- > server/src/api/remotes.rs | 45 +++++++++++++++++++++++++++++++++++++-- > 1 file changed, 43 insertions(+), 2 deletions(-) > > diff --git a/server/src/api/remotes.rs b/server/src/api/remotes.rs > index 298ad13..9f9786c 100644 > --- a/server/src/api/remotes.rs > +++ b/server/src/api/remotes.rs > @@ -27,6 +27,7 @@ use crate::api::remote_updates; > use crate::metric_collection; > use crate::{connection, pbs_client}; > > +use super::pbs; > use super::pve; > use super::rrd_common; > use super::rrd_common::DataPoint; > @@ -292,16 +293,56 @@ pub fn update_remote( > input: { > properties: { > id: { schema: REMOTE_ID_SCHEMA }, > + "delete-token": { > + type: bool, > + description: "Optional boolean value to delete the token from remote.", > + optional: true, > + } > }, > }, > access: { > permission: &Permission::Privilege(&["resource"], PRIV_RESOURCE_MODIFY, false), > }, > )] > -/// List all the remotes this instance is managing. > -pub fn remove_remote(id: String) -> Result<(), Error> { > +/// Remove a remote that this instance is managing. > +pub async fn remove_remote(id: String, delete_token: Option<bool>) -> Result<(), Error> { > let (mut remotes, _) = pdm_config::remotes::config()?; > > + if delete_token.unwrap_or(false) { > + let remote = remotes > + .get_mut(&id) > + .ok_or_else(|| http_err!(NOT_FOUND, "no such remote {id:?}"))?; > + > + let user = remote.authid.user(); > + > + let short_delete_err = |err: proxmox_client::Error| { > + format_err!("error deleting token: {}", err.source().unwrap_or(&err)) > + }; > + > + let token_name = remote > + .authid > + .tokenname() > + .ok_or_else(|| format_err!("Unable to find the token for the remote {}", id))?; nit: you can use a format string here to in-line `id`. > + > + // connect to remote and delete the already existing token. > + match remote.ty { > + RemoteType::Pve => { > + let client = pve::connect_or_login(&remote).await?; > + client > + .delete_token(user.as_str(), token_name.as_str()) > + .await > + .map_err(short_delete_err)? > + } > + RemoteType::Pbs => { > + let client = pbs::connect_or_login(&remote).await?; > + client > + .delete_admin_token(&user, token_name.as_str()) > + .await > + .map_err(short_delete_err)? > + } > + }; > + } > + > if remotes.remove(&id).is_none() { > http_bail!(NOT_FOUND, "no such entry {id:?}"); > } _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager 2/5] server: api: add support to optionally delete token from remote 2025-12-09 9:36 ` Shannon Sterz @ 2025-12-09 9:54 ` Shan Shaji 0 siblings, 0 replies; 24+ messages in thread From: Shan Shaji @ 2025-12-09 9:54 UTC (permalink / raw) To: Shannon Sterz; +Cc: Proxmox Datacenter Manager development discussion On Tue Dec 9, 2025 at 10:36 AM CET, Shannon Sterz wrote: > On Fri Dec 5, 2025 at 7:04 PM CET, Shan Shaji wrote: >> Previously, when removing a remote, the token was still present in the >> remote configuration. When users tried to add the remote again, they >> received an error because a token with the same name already existed. >> To support deleting the token from the remote, add an optional >> parameter to the API endpoint. >> >> Signed-off-by: Shan Shaji <s.shaji@proxmox.com> >> --- >> server/src/api/remotes.rs | 45 +++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 43 insertions(+), 2 deletions(-) >> >> diff --git a/server/src/api/remotes.rs b/server/src/api/remotes.rs >> index 298ad13..9f9786c 100644 >> --- a/server/src/api/remotes.rs >> +++ b/server/src/api/remotes.rs >> @@ -27,6 +27,7 @@ use crate::api::remote_updates; >> use crate::metric_collection; >> use crate::{connection, pbs_client}; >> >> +use super::pbs; >> use super::pve; >> use super::rrd_common; >> use super::rrd_common::DataPoint; >> @@ -292,16 +293,56 @@ pub fn update_remote( >> input: { >> properties: { >> id: { schema: REMOTE_ID_SCHEMA }, >> + "delete-token": { >> + type: bool, >> + description: "Optional boolean value to delete the token from remote.", >> + optional: true, >> + } >> }, >> }, >> access: { >> permission: &Permission::Privilege(&["resource"], PRIV_RESOURCE_MODIFY, false), >> }, >> )] >> -/// List all the remotes this instance is managing. >> -pub fn remove_remote(id: String) -> Result<(), Error> { >> +/// Remove a remote that this instance is managing. >> +pub async fn remove_remote(id: String, delete_token: Option<bool>) -> Result<(), Error> { >> let (mut remotes, _) = pdm_config::remotes::config()?; >> >> + if delete_token.unwrap_or(false) { >> + let remote = remotes >> + .get_mut(&id) >> + .ok_or_else(|| http_err!(NOT_FOUND, "no such remote {id:?}"))?; >> + >> + let user = remote.authid.user(); >> + >> + let short_delete_err = |err: proxmox_client::Error| { >> + format_err!("error deleting token: {}", err.source().unwrap_or(&err)) >> + }; >> + >> + let token_name = remote >> + .authid >> + .tokenname() >> + .ok_or_else(|| format_err!("Unable to find the token for the remote {}", id))?; > > nit: you can use a format string here to in-line `id`. Will update it, thank you! >> + >> + // connect to remote and delete the already existing token. >> + match remote.ty { >> + RemoteType::Pve => { >> + let client = pve::connect_or_login(&remote).await?; >> + client >> + .delete_token(user.as_str(), token_name.as_str()) >> + .await >> + .map_err(short_delete_err)? >> + } >> + RemoteType::Pbs => { >> + let client = pbs::connect_or_login(&remote).await?; >> + client >> + .delete_admin_token(&user, token_name.as_str()) >> + .await >> + .map_err(short_delete_err)? >> + } >> + }; >> + } >> + >> if remotes.remove(&id).is_none() { >> http_bail!(NOT_FOUND, "no such entry {id:?}"); >> } _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager 2/5] server: api: add support to optionally delete token from remote 2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 2/5] server: api: add support to optionally delete token from remote Shan Shaji 2025-12-09 9:36 ` Shannon Sterz @ 2025-12-09 12:34 ` Michael Köppl 2025-12-09 13:37 ` Shan Shaji 1 sibling, 1 reply; 24+ messages in thread From: Michael Köppl @ 2025-12-09 12:34 UTC (permalink / raw) To: Proxmox Datacenter Manager development discussion; +Cc: pdm-devel On Fri Dec 5, 2025 at 7:04 PM CET, Shan Shaji wrote: > Previously, when removing a remote, the token was still present in the > remote configuration. When users tried to add the remote again, they > received an error because a token with the same name already existed. > To support deleting the token from the remote, add an optional > parameter to the API endpoint. > > Signed-off-by: Shan Shaji <s.shaji@proxmox.com> > --- > server/src/api/remotes.rs | 45 +++++++++++++++++++++++++++++++++++++-- > 1 file changed, 43 insertions(+), 2 deletions(-) > > diff --git a/server/src/api/remotes.rs b/server/src/api/remotes.rs > index 298ad13..9f9786c 100644 > --- a/server/src/api/remotes.rs > +++ b/server/src/api/remotes.rs > @@ -27,6 +27,7 @@ use crate::api::remote_updates; > use crate::metric_collection; > use crate::{connection, pbs_client}; > > +use super::pbs; > use super::pve; > use super::rrd_common; > use super::rrd_common::DataPoint; > @@ -292,16 +293,56 @@ pub fn update_remote( > input: { > properties: { > id: { schema: REMOTE_ID_SCHEMA }, > + "delete-token": { > + type: bool, > + description: "Optional boolean value to delete the token from remote.", > + optional: true, > + } > }, > }, > access: { > permission: &Permission::Privilege(&["resource"], PRIV_RESOURCE_MODIFY, false), > }, > )] > -/// List all the remotes this instance is managing. > -pub fn remove_remote(id: String) -> Result<(), Error> { > +/// Remove a remote that this instance is managing. > +pub async fn remove_remote(id: String, delete_token: Option<bool>) -> Result<(), Error> { > let (mut remotes, _) = pdm_config::remotes::config()?; Isn't that a bit racy, though, considering you're making asynchronous network calls if delete_token is true? By the time you arrive at the save_config(...) call, the list of remotes you fetched here might have changed because another call to remove a remote was made pretty much at the same time, causing you to overwrite those changes with the list of remotes you fetched here. Not too familiar with the code here, but we use pdm_config::remotes::lock_config()?; in other parts to avoid this, it seems. > > + if delete_token.unwrap_or(false) { > + let remote = remotes > + .get_mut(&id) nit: .get(&id) should be sufficient > + .ok_or_else(|| http_err!(NOT_FOUND, "no such remote {id:?}"))?; > + > + let user = remote.authid.user(); > + > + let short_delete_err = |err: proxmox_client::Error| { > + format_err!("error deleting token: {}", err.source().unwrap_or(&err)) > + }; > + > + let token_name = remote > + .authid > + .tokenname() > + .ok_or_else(|| format_err!("Unable to find the token for the remote {}", id))?; > + > + // connect to remote and delete the already existing token. > + match remote.ty { > + RemoteType::Pve => { > + let client = pve::connect_or_login(&remote).await?; nit: this could just be pve::connect_or_login(remote).await? > + client > + .delete_token(user.as_str(), token_name.as_str()) > + .await > + .map_err(short_delete_err)? > + } > + RemoteType::Pbs => { > + let client = pbs::connect_or_login(&remote).await?; nit: same here > + client > + .delete_admin_token(&user, token_name.as_str()) nit: same here for user > + .await > + .map_err(short_delete_err)? > + } > + }; > + } > + > if remotes.remove(&id).is_none() { > http_bail!(NOT_FOUND, "no such entry {id:?}"); > } _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager 2/5] server: api: add support to optionally delete token from remote 2025-12-09 12:34 ` Michael Köppl @ 2025-12-09 13:37 ` Shan Shaji 0 siblings, 0 replies; 24+ messages in thread From: Shan Shaji @ 2025-12-09 13:37 UTC (permalink / raw) To: Proxmox Datacenter Manager development discussion; +Cc: pdm-devel On Tue Dec 9, 2025 at 1:34 PM CET, Michael Köppl wrote: > On Fri Dec 5, 2025 at 7:04 PM CET, Shan Shaji wrote: >> Previously, when removing a remote, the token was still present in the >> remote configuration. When users tried to add the remote again, they >> received an error because a token with the same name already existed. >> To support deleting the token from the remote, add an optional >> parameter to the API endpoint. >> >> Signed-off-by: Shan Shaji <s.shaji@proxmox.com> >> --- >> server/src/api/remotes.rs | 45 +++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 43 insertions(+), 2 deletions(-) >> >> diff --git a/server/src/api/remotes.rs b/server/src/api/remotes.rs >> index 298ad13..9f9786c 100644 >> --- a/server/src/api/remotes.rs >> +++ b/server/src/api/remotes.rs >> @@ -27,6 +27,7 @@ use crate::api::remote_updates; >> use crate::metric_collection; >> use crate::{connection, pbs_client}; >> >> +use super::pbs; >> use super::pve; >> use super::rrd_common; >> use super::rrd_common::DataPoint; >> @@ -292,16 +293,56 @@ pub fn update_remote( >> input: { >> properties: { >> id: { schema: REMOTE_ID_SCHEMA }, >> + "delete-token": { >> + type: bool, >> + description: "Optional boolean value to delete the token from remote.", >> + optional: true, >> + } >> }, >> }, >> access: { >> permission: &Permission::Privilege(&["resource"], PRIV_RESOURCE_MODIFY, false), >> }, >> )] >> -/// List all the remotes this instance is managing. >> -pub fn remove_remote(id: String) -> Result<(), Error> { >> +/// Remove a remote that this instance is managing. >> +pub async fn remove_remote(id: String, delete_token: Option<bool>) -> Result<(), Error> { >> let (mut remotes, _) = pdm_config::remotes::config()?; > > Isn't that a bit racy, though, considering you're making asynchronous > network calls if delete_token is true? By the time you arrive at the > save_config(...) call, the list of remotes you fetched here might have > changed because another call to remove a remote was made pretty much at > the same time, causing you to overwrite those changes with the list of > remotes you fetched here. Not too familiar with the code here, but we > use pdm_config::remotes::lock_config()?; in other parts to avoid this, > it seems. You are right. There is a possibility of race condition. thanks for pointing it out. I will lock the config here. >> >> + if delete_token.unwrap_or(false) { >> + let remote = remotes >> + .get_mut(&id) > > nit: .get(&id) should be sufficient Thanks! Will update it. >> + .ok_or_else(|| http_err!(NOT_FOUND, "no such remote {id:?}"))?; >> + >> + let user = remote.authid.user(); >> + >> + let short_delete_err = |err: proxmox_client::Error| { >> + format_err!("error deleting token: {}", err.source().unwrap_or(&err)) >> + }; >> + >> + let token_name = remote >> + .authid >> + .tokenname() >> + .ok_or_else(|| format_err!("Unable to find the token for the remote {}", id))?; >> + >> + // connect to remote and delete the already existing token. >> + match remote.ty { >> + RemoteType::Pve => { >> + let client = pve::connect_or_login(&remote).await?; > > nit: this could just be pve::connect_or_login(remote).await? Will update it. Thanks! >> + client >> + .delete_token(user.as_str(), token_name.as_str()) >> + .await >> + .map_err(short_delete_err)? >> + } >> + RemoteType::Pbs => { >> + let client = pbs::connect_or_login(&remote).await?; > > nit: same here > >> + client >> + .delete_admin_token(&user, token_name.as_str()) > > nit: same here for user > >> + .await >> + .map_err(short_delete_err)? >> + } >> + }; >> + } >> + >> if remotes.remove(&id).is_none() { >> http_bail!(NOT_FOUND, "no such entry {id:?}"); >> } > > > > _______________________________________________ > 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 3/5] pdm-client: accept `delete-token` argument for deleting api token 2025-12-05 18:04 [pdm-devel] [PATCH datacenter-manager/proxmox 0/6] fix #6914: add option to remove already existing token Shan Shaji 2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 1/5] server: pbs-client: add delete admin token method Shan Shaji 2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 2/5] server: api: add support to optionally delete token from remote Shan Shaji @ 2025-12-05 18:04 ` Shan Shaji 2025-12-09 9:36 ` Shannon Sterz 2025-12-09 13:08 ` Michael Köppl 2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 4/5] cli: client: add `delete-token` option to delete token from remote Shan Shaji ` (3 subsequent siblings) 6 siblings, 2 replies; 24+ messages in thread From: Shan Shaji @ 2025-12-05 18:04 UTC (permalink / raw) To: pdm-devel Signed-off-by: Shan Shaji <s.shaji@proxmox.com> --- lib/pdm-client/src/lib.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/pdm-client/src/lib.rs b/lib/pdm-client/src/lib.rs index 01ee6f7..872d78d 100644 --- a/lib/pdm-client/src/lib.rs +++ b/lib/pdm-client/src/lib.rs @@ -136,8 +136,10 @@ impl<T: HttpApiClient> PdmClient<T> { Ok(()) } - pub async fn delete_remote(&self, remote: &str) -> Result<(), Error> { - let path = format!("/api2/extjs/remotes/remote/{remote}"); + pub async fn delete_remote(&self, remote: &str, delete_token: &Option<bool>) -> Result<(), Error> { + let path = ApiPathBuilder::new(format!("/api2/extjs/remotes/remote/{remote}")) + .maybe_arg("delete-token", delete_token) + .build(); self.0.delete(&path).await?.nodata()?; Ok(()) } -- 2.47.3 _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager 3/5] pdm-client: accept `delete-token` argument for deleting api token 2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 3/5] pdm-client: accept `delete-token` argument for deleting api token Shan Shaji @ 2025-12-09 9:36 ` Shannon Sterz 2025-12-09 13:08 ` Michael Köppl 1 sibling, 0 replies; 24+ messages in thread From: Shannon Sterz @ 2025-12-09 9:36 UTC (permalink / raw) To: Shan Shaji; +Cc: Proxmox Datacenter Manager development discussion On Fri Dec 5, 2025 at 7:04 PM CET, Shan Shaji wrote: > Signed-off-by: Shan Shaji <s.shaji@proxmox.com> > --- > lib/pdm-client/src/lib.rs | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/lib/pdm-client/src/lib.rs b/lib/pdm-client/src/lib.rs > index 01ee6f7..872d78d 100644 > --- a/lib/pdm-client/src/lib.rs > +++ b/lib/pdm-client/src/lib.rs > @@ -136,8 +136,10 @@ impl<T: HttpApiClient> PdmClient<T> { > Ok(()) > } > > - pub async fn delete_remote(&self, remote: &str) -> Result<(), Error> { > - let path = format!("/api2/extjs/remotes/remote/{remote}"); > + pub async fn delete_remote(&self, remote: &str, delete_token: &Option<bool>) -> Result<(), Error> { nit: add a doc comment to this public function since you are touching it already. > + let path = ApiPathBuilder::new(format!("/api2/extjs/remotes/remote/{remote}")) > + .maybe_arg("delete-token", delete_token) > + .build(); > self.0.delete(&path).await?.nodata()?; > Ok(()) > } _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager 3/5] pdm-client: accept `delete-token` argument for deleting api token 2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 3/5] pdm-client: accept `delete-token` argument for deleting api token Shan Shaji 2025-12-09 9:36 ` Shannon Sterz @ 2025-12-09 13:08 ` Michael Köppl 1 sibling, 0 replies; 24+ messages in thread From: Michael Köppl @ 2025-12-09 13:08 UTC (permalink / raw) To: Proxmox Datacenter Manager development discussion; +Cc: pdm-devel this needs to formatting On Fri Dec 5, 2025 at 7:04 PM CET, Shan Shaji wrote: > Signed-off-by: Shan Shaji <s.shaji@proxmox.com> > --- > lib/pdm-client/src/lib.rs | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/lib/pdm-client/src/lib.rs b/lib/pdm-client/src/lib.rs > index 01ee6f7..872d78d 100644 > --- a/lib/pdm-client/src/lib.rs > +++ b/lib/pdm-client/src/lib.rs > @@ -136,8 +136,10 @@ impl<T: HttpApiClient> PdmClient<T> { > Ok(()) > } > > - pub async fn delete_remote(&self, remote: &str) -> Result<(), Error> { > - let path = format!("/api2/extjs/remotes/remote/{remote}"); > + pub async fn delete_remote(&self, remote: &str, delete_token: &Option<bool>) -> Result<(), Error> { > + let path = ApiPathBuilder::new(format!("/api2/extjs/remotes/remote/{remote}")) > + .maybe_arg("delete-token", delete_token) > + .build(); > self.0.delete(&path).await?.nodata()?; > Ok(()) > } _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 4/5] cli: client: add `delete-token` option to delete token from remote 2025-12-05 18:04 [pdm-devel] [PATCH datacenter-manager/proxmox 0/6] fix #6914: add option to remove already existing token Shan Shaji ` (2 preceding siblings ...) 2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 3/5] pdm-client: accept `delete-token` argument for deleting api token Shan Shaji @ 2025-12-05 18:04 ` Shan Shaji 2025-12-09 14:52 ` Michael Köppl 2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 5/5] fix: ui: add remove confirmation dialog with optional token deletion Shan Shaji ` (2 subsequent siblings) 6 siblings, 1 reply; 24+ messages in thread From: Shan Shaji @ 2025-12-05 18:04 UTC (permalink / raw) To: pdm-devel Signed-off-by: Shan Shaji <s.shaji@proxmox.com> --- cli/client/src/remotes.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/cli/client/src/remotes.rs b/cli/client/src/remotes.rs index 4145dac..0b4df3c 100644 --- a/cli/client/src/remotes.rs +++ b/cli/client/src/remotes.rs @@ -116,12 +116,17 @@ async fn update_remote(id: String, updater: RemoteUpdater) -> Result<(), Error> input: { properties: { id: { schema: REMOTE_ID_SCHEMA }, + "delete-token": { + type: bool, + optional: true, + description: "Remove the API-Token from remote." + } } } )] -/// Add a new remote. -async fn delete_remote(id: String) -> Result<(), Error> { - client()?.delete_remote(&id).await?; +/// Delete a remote. +async fn delete_remote(id: String, delete_token: Option<bool>) -> Result<(), Error> { + client()?.delete_remote(&id, &delete_token).await?; Ok(()) } -- 2.47.3 _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager 4/5] cli: client: add `delete-token` option to delete token from remote 2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 4/5] cli: client: add `delete-token` option to delete token from remote Shan Shaji @ 2025-12-09 14:52 ` Michael Köppl 2025-12-09 16:25 ` Shan Shaji 0 siblings, 1 reply; 24+ messages in thread From: Michael Köppl @ 2025-12-09 14:52 UTC (permalink / raw) To: Proxmox Datacenter Manager development discussion; +Cc: pdm-devel shouldn't this also be implemented for the admin CLI? On Fri Dec 5, 2025 at 7:04 PM CET, Shan Shaji wrote: > Signed-off-by: Shan Shaji <s.shaji@proxmox.com> > --- > cli/client/src/remotes.rs | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/cli/client/src/remotes.rs b/cli/client/src/remotes.rs > index 4145dac..0b4df3c 100644 > --- a/cli/client/src/remotes.rs > +++ b/cli/client/src/remotes.rs > @@ -116,12 +116,17 @@ async fn update_remote(id: String, updater: RemoteUpdater) -> Result<(), Error> > input: { > properties: { > id: { schema: REMOTE_ID_SCHEMA }, > + "delete-token": { > + type: bool, > + optional: true, > + description: "Remove the API-Token from remote." > + } > } > } > )] > -/// Add a new remote. > -async fn delete_remote(id: String) -> Result<(), Error> { > - client()?.delete_remote(&id).await?; > +/// Delete a remote. > +async fn delete_remote(id: String, delete_token: Option<bool>) -> Result<(), Error> { > + client()?.delete_remote(&id, &delete_token).await?; > Ok(()) > } > _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager 4/5] cli: client: add `delete-token` option to delete token from remote 2025-12-09 14:52 ` Michael Köppl @ 2025-12-09 16:25 ` Shan Shaji 0 siblings, 0 replies; 24+ messages in thread From: Shan Shaji @ 2025-12-09 16:25 UTC (permalink / raw) To: Proxmox Datacenter Manager development discussion; +Cc: pdm-devel On Tue Dec 9, 2025 at 3:52 PM CET, Michael Köppl wrote: > shouldn't this also be implemented for the admin CLI? Yes, I missed to include that patch. Will send it in v2.Thank you for pointing it out. > On Fri Dec 5, 2025 at 7:04 PM CET, Shan Shaji wrote: >> Signed-off-by: Shan Shaji <s.shaji@proxmox.com> >> --- >> cli/client/src/remotes.rs | 11 ++++++++--- >> 1 file changed, 8 insertions(+), 3 deletions(-) >> >> diff --git a/cli/client/src/remotes.rs b/cli/client/src/remotes.rs >> index 4145dac..0b4df3c 100644 >> --- a/cli/client/src/remotes.rs >> +++ b/cli/client/src/remotes.rs >> @@ -116,12 +116,17 @@ async fn update_remote(id: String, updater: RemoteUpdater) -> Result<(), Error> >> input: { >> properties: { >> id: { schema: REMOTE_ID_SCHEMA }, >> + "delete-token": { >> + type: bool, >> + optional: true, >> + description: "Remove the API-Token from remote." >> + } >> } >> } >> )] >> -/// Add a new remote. >> -async fn delete_remote(id: String) -> Result<(), Error> { >> - client()?.delete_remote(&id).await?; >> +/// Delete a remote. >> +async fn delete_remote(id: String, delete_token: Option<bool>) -> Result<(), Error> { >> + client()?.delete_remote(&id, &delete_token).await?; >> Ok(()) >> } >> > > > > _______________________________________________ > 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 5/5] fix: ui: add remove confirmation dialog with optional token deletion 2025-12-05 18:04 [pdm-devel] [PATCH datacenter-manager/proxmox 0/6] fix #6914: add option to remove already existing token Shan Shaji ` (3 preceding siblings ...) 2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 4/5] cli: client: add `delete-token` option to delete token from remote Shan Shaji @ 2025-12-05 18:04 ` Shan Shaji 2025-12-09 9:36 ` Shannon Sterz 2025-12-09 13:38 ` Michael Köppl 2025-12-05 18:04 ` [pdm-devel] [PATCH proxmox 1/1] pve-api-types: generate missing `delete_token` method Shan Shaji 2025-12-09 14:56 ` [pdm-devel] [PATCH datacenter-manager/proxmox 0/6] fix #6914: add option to remove already existing token Michael Köppl 6 siblings, 2 replies; 24+ messages in thread From: Shan Shaji @ 2025-12-05 18:04 UTC (permalink / raw) To: pdm-devel Previously, removing a remote did not remove it's token, which prevented users from re-adding the same remote later with the same token name. To fix it a new checkbox option has been added to which when enabled the token will be deleted from the remote. Signed-off-by: Shan Shaji <s.shaji@proxmox.com> --- ui/src/remotes/config.rs | 42 +++++++---- ui/src/remotes/mod.rs | 3 + ui/src/remotes/remove_remote.rs | 122 ++++++++++++++++++++++++++++++++ 3 files changed, 154 insertions(+), 13 deletions(-) create mode 100644 ui/src/remotes/remove_remote.rs diff --git a/ui/src/remotes/config.rs b/ui/src/remotes/config.rs index ac3c0f1..86cb3d6 100644 --- a/ui/src/remotes/config.rs +++ b/ui/src/remotes/config.rs @@ -7,6 +7,7 @@ use anyhow::Error; use proxmox_schema::property_string::PropertyString; use crate::remotes::edit_remote::EditRemote; +use crate::remotes::remove_remote::RemoveRemote; //use pwt::widget::form::{Field, FormContext, InputType}; use pdm_api_types::remotes::Remote; @@ -17,7 +18,7 @@ use proxmox_yew_comp::percent_encoding::percent_encode_component; //use proxmox_schema::api_types::{CERT_FINGERPRINT_SHA256_SCHEMA, DNS_NAME_OR_IP_SCHEMA}; -use serde_json::Value; +use serde_json::{json, Value}; use yew::virtual_dom::{Key, VComp, VNode}; use pwt::prelude::*; @@ -31,9 +32,7 @@ use pwt::widget::{ //use pwt::widget::InputPanel; //use proxmox_yew_comp::EditWindow; -use proxmox_yew_comp::{ - ConfirmButton, LoadableComponent, LoadableComponentContext, LoadableComponentMaster, -}; +use proxmox_yew_comp::{LoadableComponent, LoadableComponentContext, LoadableComponentMaster}; use pdm_api_types::remotes::{NodeUrl, RemoteType}; @@ -41,10 +40,13 @@ async fn load_remotes() -> Result<Vec<Remote>, Error> { proxmox_yew_comp::http_get("/remotes/remote", None).await } -async fn delete_item(key: Key) -> Result<(), Error> { +async fn delete_item(key: Key, delete_token: bool) -> Result<(), Error> { let id = key.to_string(); - let url = format!("/remotes/remote/{}", percent_encode_component(&id)); - proxmox_yew_comp::http_delete(&url, None).await?; + let param = Some(json!({ + "delete-token": delete_token, + })); + let url = format!("/remotes/remote/{}", percent_encode_component(&id),); + proxmox_yew_comp::http_delete(&url, param).await?; Ok(()) } @@ -99,11 +101,12 @@ impl RemoteConfigPanel { pub enum ViewState { Add(RemoteType), Edit, + Remove, } pub enum Msg { SelectionChange, - RemoveItem, + RemoveItem(bool), } pub struct PbsRemoteConfigPanel { @@ -146,11 +149,11 @@ impl LoadableComponent for PbsRemoteConfigPanel { fn update(&mut self, ctx: &LoadableComponentContext<Self>, msg: Self::Message) -> bool { match msg { Msg::SelectionChange => true, - Msg::RemoveItem => { + Msg::RemoveItem(v) => { if let Some(key) = self.selection.selected_key() { let link = ctx.link(); link.clone().spawn(async move { - if let Err(err) = delete_item(key).await { + if let Err(err) = delete_item(key, v).await { link.show_error(tr!("Unable to delete item"), err, true); } link.send_reload(); @@ -195,10 +198,9 @@ impl LoadableComponent for PbsRemoteConfigPanel { .onclick(link.change_view_callback(|_| Some(ViewState::Edit))), ) .with_child( - ConfirmButton::new(tr!("Remove")) - .confirm_message(tr!("Are you sure you want to remove this remote?")) + Button::new(tr!("Remove")) .disabled(disabled) - .on_activate(link.callback(|_| Msg::RemoveItem)), + .onclick(link.change_view_callback(|_| Some(ViewState::Remove))), ) .with_flex_spacer() .with_child({ @@ -233,6 +235,7 @@ impl LoadableComponent for PbsRemoteConfigPanel { .selection .selected_key() .map(|key| self.create_edit_dialog(ctx, key)), + ViewState::Remove => Some(self.create_remove_remote_dialog(ctx)), } } } @@ -293,6 +296,19 @@ impl PbsRemoteConfigPanel { .on_done(ctx.link().change_view_callback(|_| None)) .into() } + + fn create_remove_remote_dialog(&self, ctx: &LoadableComponentContext<Self>) -> Html { + let link = ctx.link(); + let close = link.change_view_callback(|_| None); + + RemoveRemote::new() + .on_dismiss(close.clone()) + .on_confirm(Callback::from(move |v| { + link.send_message(Msg::RemoveItem(v)); + link.change_view(None); + })) + .into() + } } impl From<RemoteConfigPanel> for VNode { diff --git a/ui/src/remotes/mod.rs b/ui/src/remotes/mod.rs index 603077c..6912ca9 100644 --- a/ui/src/remotes/mod.rs +++ b/ui/src/remotes/mod.rs @@ -27,6 +27,9 @@ pub use tasks::RemoteTaskList; mod updates; pub use updates::UpdateTree; +mod remove_remote; +pub use remove_remote::RemoveRemote; + mod firewall; pub use firewall::FirewallTree; diff --git a/ui/src/remotes/remove_remote.rs b/ui/src/remotes/remove_remote.rs new file mode 100644 index 0000000..e26d563 --- /dev/null +++ b/ui/src/remotes/remove_remote.rs @@ -0,0 +1,122 @@ +use std::rc::Rc; + +use yew::{ + html::IntoEventCallback, + prelude::*, + virtual_dom::{VComp, VNode}, +}; + +use pwt::{ + css::{AlignItems, FontColor, JustifyContent}, + props::{ + ContainerBuilder, CssPaddingBuilder, EventSubscriber, WidgetBuilder, WidgetStyleBuilder, + }, + tr, + widget::{form::Checkbox, Button, Column, Container, Dialog, Fa, Row}, +}; +use pwt_macros::builder; + +#[derive(PartialEq, Properties)] +#[builder] +pub struct RemoveRemote { + /// A callback for an action that needs to be confirmed by the user. + #[prop_or_default] + #[builder_cb(IntoEventCallback, into_event_callback, bool)] + pub on_confirm: Option<Callback<bool>>, + + /// A callback that will trigger if the user dismisses the action. + #[prop_or_default] + #[builder_cb(IntoEventCallback, into_event_callback, ())] + pub on_dismiss: Option<Callback<()>>, +} + +impl RemoveRemote { + pub fn new() -> Self { + yew::props!(Self {}) + } +} + +pub enum Msg { + SelectCheckBox(bool), +} + +pub struct PdmRemoveRemote { + delete_token: bool, +} + +impl Component for PdmRemoveRemote { + type Message = Msg; + + type Properties = RemoveRemote; + + fn create(_ctx: &Context<Self>) -> Self { + Self { + delete_token: false, + } + } + + fn update(&mut self, _ctx: &Context<Self>, msg: Self::Message) -> bool { + match msg { + Msg::SelectCheckBox(v) => { + self.delete_token = v; + true + } + } + } + + fn view(&self, ctx: &Context<Self>) -> Html { + let props = ctx.props(); + let delete_token = self.delete_token.clone(); + + let on_confirm = props.on_confirm.clone(); + let on_dismiss = props.on_dismiss.clone(); + + Dialog::new(tr!("Confirm")) + .on_close(on_dismiss.clone()) + .min_height(100) + .with_child( + Column::new() + .padding(4) + .gap(2) + .with_child( + Row::new() + .gap(2) + .class(AlignItems::Center) + .with_child(Container::new().class("pwt-message-sign").with_child( + Fa::new("exclamation-triangle").class(FontColor::Error), + )) + .with_child(tr!("Are you sure you want to remove this remote?")), + ) + .with_child( + Checkbox::new() + .default(false) + .box_label(tr!("Delete API token from remote")) + .checked(self.delete_token) + .on_change(ctx.link().callback(|v| Msg::SelectCheckBox(v))), + ) + .with_child( + Row::new() + .gap(2) + .class(JustifyContent::Center) + .with_child(Button::new(tr!("Yes")).onclick(move |_| { + if let Some(on_confirm) = &on_confirm { + on_confirm.emit(delete_token); + } + })) + .with_child(Button::new(tr!("No")).onclick(move |_| { + if let Some(on_dismiss) = &on_dismiss { + on_dismiss.emit(()); + } + })), + ), + ) + .into() + } +} + +impl From<RemoveRemote> for VNode { + fn from(val: RemoveRemote) -> Self { + let comp = VComp::new::<PdmRemoveRemote>(Rc::new(val), None); + VNode::from(comp) + } +} -- 2.47.3 _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager 5/5] fix: ui: add remove confirmation dialog with optional token deletion 2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 5/5] fix: ui: add remove confirmation dialog with optional token deletion Shan Shaji @ 2025-12-09 9:36 ` Shannon Sterz 2025-12-09 10:08 ` Shan Shaji 2025-12-09 13:38 ` Michael Köppl 1 sibling, 1 reply; 24+ messages in thread From: Shannon Sterz @ 2025-12-09 9:36 UTC (permalink / raw) To: Shan Shaji; +Cc: Proxmox Datacenter Manager development discussion On Fri Dec 5, 2025 at 7:04 PM CET, Shan Shaji wrote: > Previously, removing a remote did not remove it's token, which > prevented users from re-adding the same remote later with the same token > name. To fix it a new checkbox option has been added to which when > enabled the token will be deleted from the remote. > > Signed-off-by: Shan Shaji <s.shaji@proxmox.com> > --- > ui/src/remotes/config.rs | 42 +++++++---- > ui/src/remotes/mod.rs | 3 + > ui/src/remotes/remove_remote.rs | 122 ++++++++++++++++++++++++++++++++ > 3 files changed, 154 insertions(+), 13 deletions(-) > create mode 100644 ui/src/remotes/remove_remote.rs > > diff --git a/ui/src/remotes/config.rs b/ui/src/remotes/config.rs > index ac3c0f1..86cb3d6 100644 > --- a/ui/src/remotes/config.rs > +++ b/ui/src/remotes/config.rs > @@ -7,6 +7,7 @@ use anyhow::Error; > use proxmox_schema::property_string::PropertyString; > > use crate::remotes::edit_remote::EditRemote; > +use crate::remotes::remove_remote::RemoveRemote; > //use pwt::widget::form::{Field, FormContext, InputType}; > > use pdm_api_types::remotes::Remote; > @@ -17,7 +18,7 @@ use proxmox_yew_comp::percent_encoding::percent_encode_component; > > //use proxmox_schema::api_types::{CERT_FINGERPRINT_SHA256_SCHEMA, DNS_NAME_OR_IP_SCHEMA}; > > -use serde_json::Value; > +use serde_json::{json, Value}; > use yew::virtual_dom::{Key, VComp, VNode}; > > use pwt::prelude::*; > @@ -31,9 +32,7 @@ use pwt::widget::{ > //use pwt::widget::InputPanel; > > //use proxmox_yew_comp::EditWindow; > -use proxmox_yew_comp::{ > - ConfirmButton, LoadableComponent, LoadableComponentContext, LoadableComponentMaster, > -}; > +use proxmox_yew_comp::{LoadableComponent, LoadableComponentContext, LoadableComponentMaster}; > > use pdm_api_types::remotes::{NodeUrl, RemoteType}; > > @@ -41,10 +40,13 @@ async fn load_remotes() -> Result<Vec<Remote>, Error> { > proxmox_yew_comp::http_get("/remotes/remote", None).await > } > > -async fn delete_item(key: Key) -> Result<(), Error> { > +async fn delete_item(key: Key, delete_token: bool) -> Result<(), Error> { > let id = key.to_string(); > - let url = format!("/remotes/remote/{}", percent_encode_component(&id)); > - proxmox_yew_comp::http_delete(&url, None).await?; > + let param = Some(json!({ > + "delete-token": delete_token, > + })); > + let url = format!("/remotes/remote/{}", percent_encode_component(&id),); > + proxmox_yew_comp::http_delete(&url, param).await?; > Ok(()) > } > > @@ -99,11 +101,12 @@ impl RemoteConfigPanel { > pub enum ViewState { > Add(RemoteType), > Edit, > + Remove, > } > > pub enum Msg { > SelectionChange, > - RemoveItem, > + RemoveItem(bool), > } > > pub struct PbsRemoteConfigPanel { > @@ -146,11 +149,11 @@ impl LoadableComponent for PbsRemoteConfigPanel { > fn update(&mut self, ctx: &LoadableComponentContext<Self>, msg: Self::Message) -> bool { > match msg { > Msg::SelectionChange => true, > - Msg::RemoveItem => { > + Msg::RemoveItem(v) => { > if let Some(key) = self.selection.selected_key() { > let link = ctx.link(); > link.clone().spawn(async move { > - if let Err(err) = delete_item(key).await { > + if let Err(err) = delete_item(key, v).await { > link.show_error(tr!("Unable to delete item"), err, true); > } > link.send_reload(); > @@ -195,10 +198,9 @@ impl LoadableComponent for PbsRemoteConfigPanel { > .onclick(link.change_view_callback(|_| Some(ViewState::Edit))), > ) > .with_child( > - ConfirmButton::new(tr!("Remove")) > - .confirm_message(tr!("Are you sure you want to remove this remote?")) > + Button::new(tr!("Remove")) > .disabled(disabled) > - .on_activate(link.callback(|_| Msg::RemoveItem)), > + .onclick(link.change_view_callback(|_| Some(ViewState::Remove))), nit: we generally try to use `on_activate` where possible nowadays. > ) > .with_flex_spacer() > .with_child({ > @@ -233,6 +235,7 @@ impl LoadableComponent for PbsRemoteConfigPanel { > .selection > .selected_key() > .map(|key| self.create_edit_dialog(ctx, key)), > + ViewState::Remove => Some(self.create_remove_remote_dialog(ctx)), > } > } > } > @@ -293,6 +296,19 @@ impl PbsRemoteConfigPanel { > .on_done(ctx.link().change_view_callback(|_| None)) > .into() > } > + > + fn create_remove_remote_dialog(&self, ctx: &LoadableComponentContext<Self>) -> Html { > + let link = ctx.link(); > + let close = link.change_view_callback(|_| None); > + > + RemoveRemote::new() > + .on_dismiss(close.clone()) > + .on_confirm(Callback::from(move |v| { > + link.send_message(Msg::RemoveItem(v)); > + link.change_view(None); > + })) > + .into() > + } > } > > impl From<RemoteConfigPanel> for VNode { > diff --git a/ui/src/remotes/mod.rs b/ui/src/remotes/mod.rs > index 603077c..6912ca9 100644 > --- a/ui/src/remotes/mod.rs > +++ b/ui/src/remotes/mod.rs > @@ -27,6 +27,9 @@ pub use tasks::RemoteTaskList; > mod updates; > pub use updates::UpdateTree; > > +mod remove_remote; > +pub use remove_remote::RemoveRemote; > + > mod firewall; > pub use firewall::FirewallTree; > > diff --git a/ui/src/remotes/remove_remote.rs b/ui/src/remotes/remove_remote.rs > new file mode 100644 > index 0000000..e26d563 > --- /dev/null > +++ b/ui/src/remotes/remove_remote.rs > @@ -0,0 +1,122 @@ > +use std::rc::Rc; > + > +use yew::{ > + html::IntoEventCallback, > + prelude::*, > + virtual_dom::{VComp, VNode}, > +}; > + > +use pwt::{ > + css::{AlignItems, FontColor, JustifyContent}, > + props::{ > + ContainerBuilder, CssPaddingBuilder, EventSubscriber, WidgetBuilder, WidgetStyleBuilder, > + }, > + tr, > + widget::{form::Checkbox, Button, Column, Container, Dialog, Fa, Row}, > +}; nit: use module level import for new components :) > +use pwt_macros::builder; > + > +#[derive(PartialEq, Properties)] > +#[builder] > +pub struct RemoveRemote { > + /// A callback for an action that needs to be confirmed by the user. > + #[prop_or_default] > + #[builder_cb(IntoEventCallback, into_event_callback, bool)] > + pub on_confirm: Option<Callback<bool>>, > + > + /// A callback that will trigger if the user dismisses the action. > + #[prop_or_default] > + #[builder_cb(IntoEventCallback, into_event_callback, ())] > + pub on_dismiss: Option<Callback<()>>, > +} > + > +impl RemoveRemote { > + pub fn new() -> Self { > + yew::props!(Self {}) > + } > +} > + > +pub enum Msg { > + SelectCheckBox(bool), > +} > + > +pub struct PdmRemoveRemote { > + delete_token: bool, > +} > + > +impl Component for PdmRemoveRemote { > + type Message = Msg; > + > + type Properties = RemoveRemote; > + > + fn create(_ctx: &Context<Self>) -> Self { > + Self { > + delete_token: false, > + } > + } > + > + fn update(&mut self, _ctx: &Context<Self>, msg: Self::Message) -> bool { > + match msg { > + Msg::SelectCheckBox(v) => { > + self.delete_token = v; > + true > + } > + } > + } > + > + fn view(&self, ctx: &Context<Self>) -> Html { > + let props = ctx.props(); > + let delete_token = self.delete_token.clone(); > + > + let on_confirm = props.on_confirm.clone(); > + let on_dismiss = props.on_dismiss.clone(); > + > + Dialog::new(tr!("Confirm")) > + .on_close(on_dismiss.clone()) > + .min_height(100) > + .with_child( > + Column::new() > + .padding(4) > + .gap(2) > + .with_child( > + Row::new() > + .gap(2) > + .class(AlignItems::Center) > + .with_child(Container::new().class("pwt-message-sign").with_child( > + Fa::new("exclamation-triangle").class(FontColor::Error), > + )) > + .with_child(tr!("Are you sure you want to remove this remote?")), > + ) > + .with_child( > + Checkbox::new() > + .default(false) > + .box_label(tr!("Delete API token from remote")) > + .checked(self.delete_token) > + .on_change(ctx.link().callback(|v| Msg::SelectCheckBox(v))), > + ) > + .with_child( > + Row::new() > + .gap(2) > + .class(JustifyContent::Center) > + .with_child(Button::new(tr!("Yes")).onclick(move |_| { > + if let Some(on_confirm) = &on_confirm { > + on_confirm.emit(delete_token); > + } > + })) > + .with_child(Button::new(tr!("No")).onclick(move |_| { > + if let Some(on_dismiss) = &on_dismiss { > + on_dismiss.emit(()); > + } > + })), > + ), > + ) > + .into() > + } > +} > + > +impl From<RemoveRemote> for VNode { > + fn from(val: RemoveRemote) -> Self { > + let comp = VComp::new::<PdmRemoveRemote>(Rc::new(val), None); > + VNode::from(comp) > + } > +} _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager 5/5] fix: ui: add remove confirmation dialog with optional token deletion 2025-12-09 9:36 ` Shannon Sterz @ 2025-12-09 10:08 ` Shan Shaji 0 siblings, 0 replies; 24+ messages in thread From: Shan Shaji @ 2025-12-09 10:08 UTC (permalink / raw) To: Shannon Sterz; +Cc: Proxmox Datacenter Manager development discussion On Tue Dec 9, 2025 at 10:36 AM CET, Shannon Sterz wrote: > On Fri Dec 5, 2025 at 7:04 PM CET, Shan Shaji wrote: >> Previously, removing a remote did not remove it's token, which >> prevented users from re-adding the same remote later with the same token >> name. To fix it a new checkbox option has been added to which when >> enabled the token will be deleted from the remote. >> >> Signed-off-by: Shan Shaji <s.shaji@proxmox.com> >> --- >> ui/src/remotes/config.rs | 42 +++++++---- >> ui/src/remotes/mod.rs | 3 + >> ui/src/remotes/remove_remote.rs | 122 ++++++++++++++++++++++++++++++++ >> 3 files changed, 154 insertions(+), 13 deletions(-) >> create mode 100644 ui/src/remotes/remove_remote.rs >> >> diff --git a/ui/src/remotes/config.rs b/ui/src/remotes/config.rs >> index ac3c0f1..86cb3d6 100644 >> --- a/ui/src/remotes/config.rs >> +++ b/ui/src/remotes/config.rs >> @@ -7,6 +7,7 @@ use anyhow::Error; >> use proxmox_schema::property_string::PropertyString; >> >> use crate::remotes::edit_remote::EditRemote; >> +use crate::remotes::remove_remote::RemoveRemote; >> //use pwt::widget::form::{Field, FormContext, InputType}; >> >> use pdm_api_types::remotes::Remote; >> @@ -17,7 +18,7 @@ use proxmox_yew_comp::percent_encoding::percent_encode_component; >> >> //use proxmox_schema::api_types::{CERT_FINGERPRINT_SHA256_SCHEMA, DNS_NAME_OR_IP_SCHEMA}; >> >> -use serde_json::Value; >> +use serde_json::{json, Value}; >> use yew::virtual_dom::{Key, VComp, VNode}; >> >> use pwt::prelude::*; >> @@ -31,9 +32,7 @@ use pwt::widget::{ >> //use pwt::widget::InputPanel; >> >> //use proxmox_yew_comp::EditWindow; >> -use proxmox_yew_comp::{ >> - ConfirmButton, LoadableComponent, LoadableComponentContext, LoadableComponentMaster, >> -}; >> +use proxmox_yew_comp::{LoadableComponent, LoadableComponentContext, LoadableComponentMaster}; >> >> use pdm_api_types::remotes::{NodeUrl, RemoteType}; >> >> @@ -41,10 +40,13 @@ async fn load_remotes() -> Result<Vec<Remote>, Error> { >> proxmox_yew_comp::http_get("/remotes/remote", None).await >> } >> >> -async fn delete_item(key: Key) -> Result<(), Error> { >> +async fn delete_item(key: Key, delete_token: bool) -> Result<(), Error> { >> let id = key.to_string(); >> - let url = format!("/remotes/remote/{}", percent_encode_component(&id)); >> - proxmox_yew_comp::http_delete(&url, None).await?; >> + let param = Some(json!({ >> + "delete-token": delete_token, >> + })); >> + let url = format!("/remotes/remote/{}", percent_encode_component(&id),); >> + proxmox_yew_comp::http_delete(&url, param).await?; >> Ok(()) >> } >> >> @@ -99,11 +101,12 @@ impl RemoteConfigPanel { >> pub enum ViewState { >> Add(RemoteType), >> Edit, >> + Remove, >> } >> >> pub enum Msg { >> SelectionChange, >> - RemoveItem, >> + RemoveItem(bool), >> } >> >> pub struct PbsRemoteConfigPanel { >> @@ -146,11 +149,11 @@ impl LoadableComponent for PbsRemoteConfigPanel { >> fn update(&mut self, ctx: &LoadableComponentContext<Self>, msg: Self::Message) -> bool { >> match msg { >> Msg::SelectionChange => true, >> - Msg::RemoveItem => { >> + Msg::RemoveItem(v) => { >> if let Some(key) = self.selection.selected_key() { >> let link = ctx.link(); >> link.clone().spawn(async move { >> - if let Err(err) = delete_item(key).await { >> + if let Err(err) = delete_item(key, v).await { >> link.show_error(tr!("Unable to delete item"), err, true); >> } >> link.send_reload(); >> @@ -195,10 +198,9 @@ impl LoadableComponent for PbsRemoteConfigPanel { >> .onclick(link.change_view_callback(|_| Some(ViewState::Edit))), >> ) >> .with_child( >> - ConfirmButton::new(tr!("Remove")) >> - .confirm_message(tr!("Are you sure you want to remove this remote?")) >> + Button::new(tr!("Remove")) >> .disabled(disabled) >> - .on_activate(link.callback(|_| Msg::RemoveItem)), >> + .onclick(link.change_view_callback(|_| Some(ViewState::Remove))), > > nit: we generally try to use `on_activate` where possible nowadays. Will update it, Thank You! >> ) >> .with_flex_spacer() >> .with_child({ >> @@ -233,6 +235,7 @@ impl LoadableComponent for PbsRemoteConfigPanel { >> .selection >> .selected_key() >> .map(|key| self.create_edit_dialog(ctx, key)), >> + ViewState::Remove => Some(self.create_remove_remote_dialog(ctx)), >> } >> } >> } >> @@ -293,6 +296,19 @@ impl PbsRemoteConfigPanel { >> .on_done(ctx.link().change_view_callback(|_| None)) >> .into() >> } >> + >> + fn create_remove_remote_dialog(&self, ctx: &LoadableComponentContext<Self>) -> Html { >> + let link = ctx.link(); >> + let close = link.change_view_callback(|_| None); >> + >> + RemoveRemote::new() >> + .on_dismiss(close.clone()) >> + .on_confirm(Callback::from(move |v| { >> + link.send_message(Msg::RemoveItem(v)); >> + link.change_view(None); >> + })) >> + .into() >> + } >> } >> >> impl From<RemoteConfigPanel> for VNode { >> diff --git a/ui/src/remotes/mod.rs b/ui/src/remotes/mod.rs >> index 603077c..6912ca9 100644 >> --- a/ui/src/remotes/mod.rs >> +++ b/ui/src/remotes/mod.rs >> @@ -27,6 +27,9 @@ pub use tasks::RemoteTaskList; >> mod updates; >> pub use updates::UpdateTree; >> >> +mod remove_remote; >> +pub use remove_remote::RemoveRemote; >> + >> mod firewall; >> pub use firewall::FirewallTree; >> >> diff --git a/ui/src/remotes/remove_remote.rs b/ui/src/remotes/remove_remote.rs >> new file mode 100644 >> index 0000000..e26d563 >> --- /dev/null >> +++ b/ui/src/remotes/remove_remote.rs >> @@ -0,0 +1,122 @@ >> +use std::rc::Rc; >> + >> +use yew::{ >> + html::IntoEventCallback, >> + prelude::*, >> + virtual_dom::{VComp, VNode}, >> +}; >> + >> +use pwt::{ >> + css::{AlignItems, FontColor, JustifyContent}, >> + props::{ >> + ContainerBuilder, CssPaddingBuilder, EventSubscriber, WidgetBuilder, WidgetStyleBuilder, >> + }, >> + tr, >> + widget::{form::Checkbox, Button, Column, Container, Dialog, Fa, Row}, >> +}; > > nit: use module level import for new components :) Will update it, Thank you! >> +use pwt_macros::builder; >> + >> +#[derive(PartialEq, Properties)] >> +#[builder] >> +pub struct RemoveRemote { >> + /// A callback for an action that needs to be confirmed by the user. >> + #[prop_or_default] >> + #[builder_cb(IntoEventCallback, into_event_callback, bool)] >> + pub on_confirm: Option<Callback<bool>>, >> + >> + /// A callback that will trigger if the user dismisses the action. >> + #[prop_or_default] >> + #[builder_cb(IntoEventCallback, into_event_callback, ())] >> + pub on_dismiss: Option<Callback<()>>, >> +} >> + >> +impl RemoveRemote { >> + pub fn new() -> Self { >> + yew::props!(Self {}) >> + } >> +} >> + >> +pub enum Msg { >> + SelectCheckBox(bool), >> +} >> + >> +pub struct PdmRemoveRemote { >> + delete_token: bool, >> +} >> + >> +impl Component for PdmRemoveRemote { >> + type Message = Msg; >> + >> + type Properties = RemoveRemote; >> + >> + fn create(_ctx: &Context<Self>) -> Self { >> + Self { >> + delete_token: false, >> + } >> + } >> + >> + fn update(&mut self, _ctx: &Context<Self>, msg: Self::Message) -> bool { >> + match msg { >> + Msg::SelectCheckBox(v) => { >> + self.delete_token = v; >> + true >> + } >> + } >> + } >> + >> + fn view(&self, ctx: &Context<Self>) -> Html { >> + let props = ctx.props(); >> + let delete_token = self.delete_token.clone(); >> + >> + let on_confirm = props.on_confirm.clone(); >> + let on_dismiss = props.on_dismiss.clone(); >> + >> + Dialog::new(tr!("Confirm")) >> + .on_close(on_dismiss.clone()) >> + .min_height(100) >> + .with_child( >> + Column::new() >> + .padding(4) >> + .gap(2) >> + .with_child( >> + Row::new() >> + .gap(2) >> + .class(AlignItems::Center) >> + .with_child(Container::new().class("pwt-message-sign").with_child( >> + Fa::new("exclamation-triangle").class(FontColor::Error), >> + )) >> + .with_child(tr!("Are you sure you want to remove this remote?")), >> + ) >> + .with_child( >> + Checkbox::new() >> + .default(false) >> + .box_label(tr!("Delete API token from remote")) >> + .checked(self.delete_token) >> + .on_change(ctx.link().callback(|v| Msg::SelectCheckBox(v))), >> + ) >> + .with_child( >> + Row::new() >> + .gap(2) >> + .class(JustifyContent::Center) >> + .with_child(Button::new(tr!("Yes")).onclick(move |_| { >> + if let Some(on_confirm) = &on_confirm { >> + on_confirm.emit(delete_token); >> + } >> + })) >> + .with_child(Button::new(tr!("No")).onclick(move |_| { >> + if let Some(on_dismiss) = &on_dismiss { >> + on_dismiss.emit(()); >> + } >> + })), >> + ), >> + ) >> + .into() >> + } >> +} >> + >> +impl From<RemoveRemote> for VNode { >> + fn from(val: RemoveRemote) -> Self { >> + let comp = VComp::new::<PdmRemoveRemote>(Rc::new(val), None); >> + VNode::from(comp) >> + } >> +} _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager 5/5] fix: ui: add remove confirmation dialog with optional token deletion 2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 5/5] fix: ui: add remove confirmation dialog with optional token deletion Shan Shaji 2025-12-09 9:36 ` Shannon Sterz @ 2025-12-09 13:38 ` Michael Köppl 2025-12-09 13:52 ` Shan Shaji 1 sibling, 1 reply; 24+ messages in thread From: Michael Köppl @ 2025-12-09 13:38 UTC (permalink / raw) To: Proxmox Datacenter Manager development discussion; +Cc: pdm-devel 4 comments inline On Fri Dec 5, 2025 at 7:04 PM CET, Shan Shaji wrote: > Previously, removing a remote did not remove it's token, which nit: s/it\'s/its > prevented users from re-adding the same remote later with the same token > name. To fix it a new checkbox option has been added to which when > enabled the token will be deleted from the remote. nit: was a bit difficult to grasp for me. Maybe something like "To fix this, add a new checkbox that lets users choose to also delete the token when deleting the remote". > > Signed-off-by: Shan Shaji <s.shaji@proxmox.com> > --- > ui/src/remotes/config.rs | 42 +++++++---- > ui/src/remotes/mod.rs | 3 + > ui/src/remotes/remove_remote.rs | 122 ++++++++++++++++++++++++++++++++ > 3 files changed, 154 insertions(+), 13 deletions(-) > create mode 100644 ui/src/remotes/remove_remote.rs > > diff --git a/ui/src/remotes/config.rs b/ui/src/remotes/config.rs > index ac3c0f1..86cb3d6 100644 > --- a/ui/src/remotes/config.rs > +++ b/ui/src/remotes/config.rs > @@ -7,6 +7,7 @@ use anyhow::Error; > use proxmox_schema::property_string::PropertyString; > > use crate::remotes::edit_remote::EditRemote; > +use crate::remotes::remove_remote::RemoveRemote; > //use pwt::widget::form::{Field, FormContext, InputType}; > > use pdm_api_types::remotes::Remote; > @@ -17,7 +18,7 @@ use proxmox_yew_comp::percent_encoding::percent_encode_component; > > //use proxmox_schema::api_types::{CERT_FINGERPRINT_SHA256_SCHEMA, DNS_NAME_OR_IP_SCHEMA}; > > -use serde_json::Value; > +use serde_json::{json, Value}; > use yew::virtual_dom::{Key, VComp, VNode}; > > use pwt::prelude::*; > @@ -31,9 +32,7 @@ use pwt::widget::{ > //use pwt::widget::InputPanel; > > //use proxmox_yew_comp::EditWindow; > -use proxmox_yew_comp::{ > - ConfirmButton, LoadableComponent, LoadableComponentContext, LoadableComponentMaster, > -}; > +use proxmox_yew_comp::{LoadableComponent, LoadableComponentContext, LoadableComponentMaster}; > > use pdm_api_types::remotes::{NodeUrl, RemoteType}; > > @@ -41,10 +40,13 @@ async fn load_remotes() -> Result<Vec<Remote>, Error> { > proxmox_yew_comp::http_get("/remotes/remote", None).await > } > > -async fn delete_item(key: Key) -> Result<(), Error> { > +async fn delete_item(key: Key, delete_token: bool) -> Result<(), Error> { > let id = key.to_string(); > - let url = format!("/remotes/remote/{}", percent_encode_component(&id)); > - proxmox_yew_comp::http_delete(&url, None).await?; > + let param = Some(json!({ > + "delete-token": delete_token, > + })); > + let url = format!("/remotes/remote/{}", percent_encode_component(&id),); nit: is there a reason for this added comma at the end? > + proxmox_yew_comp::http_delete(&url, param).await?; > Ok(()) > } > > @@ -99,11 +101,12 @@ impl RemoteConfigPanel { > pub enum ViewState { > Add(RemoteType), > Edit, > + Remove, > } > > pub enum Msg { > SelectionChange, > - RemoveItem, > + RemoveItem(bool), > } > > pub struct PbsRemoteConfigPanel { > @@ -146,11 +149,11 @@ impl LoadableComponent for PbsRemoteConfigPanel { > fn update(&mut self, ctx: &LoadableComponentContext<Self>, msg: Self::Message) -> bool { > match msg { > Msg::SelectionChange => true, > - Msg::RemoveItem => { > + Msg::RemoveItem(v) => { > if let Some(key) = self.selection.selected_key() { > let link = ctx.link(); > link.clone().spawn(async move { > - if let Err(err) = delete_item(key).await { > + if let Err(err) = delete_item(key, v).await { > link.show_error(tr!("Unable to delete item"), err, true); > } > link.send_reload(); > @@ -195,10 +198,9 @@ impl LoadableComponent for PbsRemoteConfigPanel { > .onclick(link.change_view_callback(|_| Some(ViewState::Edit))), > ) > .with_child( > - ConfirmButton::new(tr!("Remove")) > - .confirm_message(tr!("Are you sure you want to remove this remote?")) > + Button::new(tr!("Remove")) > .disabled(disabled) > - .on_activate(link.callback(|_| Msg::RemoveItem)), > + .onclick(link.change_view_callback(|_| Some(ViewState::Remove))), > ) > .with_flex_spacer() > .with_child({ > @@ -233,6 +235,7 @@ impl LoadableComponent for PbsRemoteConfigPanel { > .selection > .selected_key() > .map(|key| self.create_edit_dialog(ctx, key)), > + ViewState::Remove => Some(self.create_remove_remote_dialog(ctx)), > } > } > } > @@ -293,6 +296,19 @@ impl PbsRemoteConfigPanel { > .on_done(ctx.link().change_view_callback(|_| None)) > .into() > } > + > + fn create_remove_remote_dialog(&self, ctx: &LoadableComponentContext<Self>) -> Html { > + let link = ctx.link(); > + let close = link.change_view_callback(|_| None); > + > + RemoveRemote::new() > + .on_dismiss(close.clone()) > + .on_confirm(Callback::from(move |v| { > + link.send_message(Msg::RemoveItem(v)); > + link.change_view(None); > + })) > + .into() > + } > } > > impl From<RemoteConfigPanel> for VNode { > diff --git a/ui/src/remotes/mod.rs b/ui/src/remotes/mod.rs > index 603077c..6912ca9 100644 > --- a/ui/src/remotes/mod.rs > +++ b/ui/src/remotes/mod.rs > @@ -27,6 +27,9 @@ pub use tasks::RemoteTaskList; > mod updates; > pub use updates::UpdateTree; > > +mod remove_remote; > +pub use remove_remote::RemoveRemote; > + > mod firewall; > pub use firewall::FirewallTree; > > diff --git a/ui/src/remotes/remove_remote.rs b/ui/src/remotes/remove_remote.rs > new file mode 100644 > index 0000000..e26d563 > --- /dev/null > +++ b/ui/src/remotes/remove_remote.rs > @@ -0,0 +1,122 @@ > +use std::rc::Rc; > + > +use yew::{ > + html::IntoEventCallback, > + prelude::*, > + virtual_dom::{VComp, VNode}, > +}; > + > +use pwt::{ > + css::{AlignItems, FontColor, JustifyContent}, > + props::{ > + ContainerBuilder, CssPaddingBuilder, EventSubscriber, WidgetBuilder, WidgetStyleBuilder, > + }, > + tr, > + widget::{form::Checkbox, Button, Column, Container, Dialog, Fa, Row}, > +}; > +use pwt_macros::builder; > + > +#[derive(PartialEq, Properties)] > +#[builder] > +pub struct RemoveRemote { > + /// A callback for an action that needs to be confirmed by the user. > + #[prop_or_default] > + #[builder_cb(IntoEventCallback, into_event_callback, bool)] > + pub on_confirm: Option<Callback<bool>>, > + > + /// A callback that will trigger if the user dismisses the action. > + #[prop_or_default] > + #[builder_cb(IntoEventCallback, into_event_callback, ())] > + pub on_dismiss: Option<Callback<()>>, > +} > + > +impl RemoveRemote { > + pub fn new() -> Self { > + yew::props!(Self {}) > + } > +} > + > +pub enum Msg { > + SelectCheckBox(bool), > +} > + > +pub struct PdmRemoveRemote { > + delete_token: bool, > +} > + > +impl Component for PdmRemoveRemote { > + type Message = Msg; > + > + type Properties = RemoveRemote; > + > + fn create(_ctx: &Context<Self>) -> Self { > + Self { > + delete_token: false, > + } > + } > + > + fn update(&mut self, _ctx: &Context<Self>, msg: Self::Message) -> bool { > + match msg { > + Msg::SelectCheckBox(v) => { > + self.delete_token = v; > + true > + } > + } > + } > + > + fn view(&self, ctx: &Context<Self>) -> Html { > + let props = ctx.props(); > + let delete_token = self.delete_token.clone(); the clone() here is unnecessary > + > + let on_confirm = props.on_confirm.clone(); > + let on_dismiss = props.on_dismiss.clone(); > + > + Dialog::new(tr!("Confirm")) > + .on_close(on_dismiss.clone()) > + .min_height(100) > + .with_child( > + Column::new() > + .padding(4) > + .gap(2) > + .with_child( > + Row::new() > + .gap(2) > + .class(AlignItems::Center) > + .with_child(Container::new().class("pwt-message-sign").with_child( > + Fa::new("exclamation-triangle").class(FontColor::Error), > + )) > + .with_child(tr!("Are you sure you want to remove this remote?")), > + ) > + .with_child( > + Checkbox::new() > + .default(false) > + .box_label(tr!("Delete API token from remote")) > + .checked(self.delete_token) > + .on_change(ctx.link().callback(|v| Msg::SelectCheckBox(v))), > + ) > + .with_child( > + Row::new() > + .gap(2) > + .class(JustifyContent::Center) > + .with_child(Button::new(tr!("Yes")).onclick(move |_| { > + if let Some(on_confirm) = &on_confirm { > + on_confirm.emit(delete_token); > + } > + })) > + .with_child(Button::new(tr!("No")).onclick(move |_| { > + if let Some(on_dismiss) = &on_dismiss { > + on_dismiss.emit(()); > + } > + })), > + ), > + ) > + .into() > + } > +} > + > +impl From<RemoveRemote> for VNode { > + fn from(val: RemoveRemote) -> Self { > + let comp = VComp::new::<PdmRemoveRemote>(Rc::new(val), None); > + VNode::from(comp) > + } > +} _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager 5/5] fix: ui: add remove confirmation dialog with optional token deletion 2025-12-09 13:38 ` Michael Köppl @ 2025-12-09 13:52 ` Shan Shaji 0 siblings, 0 replies; 24+ messages in thread From: Shan Shaji @ 2025-12-09 13:52 UTC (permalink / raw) To: Proxmox Datacenter Manager development discussion; +Cc: pdm-devel On Tue Dec 9, 2025 at 2:38 PM CET, Michael Köppl wrote: > 4 comments inline > > On Fri Dec 5, 2025 at 7:04 PM CET, Shan Shaji wrote: >> Previously, removing a remote did not remove it's token, which > > nit: s/it\'s/its > >> prevented users from re-adding the same remote later with the same token >> name. To fix it a new checkbox option has been added to which when >> enabled the token will be deleted from the remote. > > nit: was a bit difficult to grasp for me. Maybe something like "To fix > this, add a new checkbox that lets users choose to also delete the > token when deleting the remote". Will update it. Thank you! >> >> Signed-off-by: Shan Shaji <s.shaji@proxmox.com> >> --- >> ui/src/remotes/config.rs | 42 +++++++---- >> ui/src/remotes/mod.rs | 3 + >> ui/src/remotes/remove_remote.rs | 122 ++++++++++++++++++++++++++++++++ >> 3 files changed, 154 insertions(+), 13 deletions(-) >> create mode 100644 ui/src/remotes/remove_remote.rs >> >> diff --git a/ui/src/remotes/config.rs b/ui/src/remotes/config.rs >> index ac3c0f1..86cb3d6 100644 >> --- a/ui/src/remotes/config.rs >> +++ b/ui/src/remotes/config.rs >> @@ -7,6 +7,7 @@ use anyhow::Error; >> use proxmox_schema::property_string::PropertyString; >> >> use crate::remotes::edit_remote::EditRemote; >> +use crate::remotes::remove_remote::RemoveRemote; >> //use pwt::widget::form::{Field, FormContext, InputType}; >> >> use pdm_api_types::remotes::Remote; >> @@ -17,7 +18,7 @@ use proxmox_yew_comp::percent_encoding::percent_encode_component; >> >> //use proxmox_schema::api_types::{CERT_FINGERPRINT_SHA256_SCHEMA, DNS_NAME_OR_IP_SCHEMA}; >> >> -use serde_json::Value; >> +use serde_json::{json, Value}; >> use yew::virtual_dom::{Key, VComp, VNode}; >> >> use pwt::prelude::*; >> @@ -31,9 +32,7 @@ use pwt::widget::{ >> //use pwt::widget::InputPanel; >> >> //use proxmox_yew_comp::EditWindow; >> -use proxmox_yew_comp::{ >> - ConfirmButton, LoadableComponent, LoadableComponentContext, LoadableComponentMaster, >> -}; >> +use proxmox_yew_comp::{LoadableComponent, LoadableComponentContext, LoadableComponentMaster}; >> >> use pdm_api_types::remotes::{NodeUrl, RemoteType}; >> >> @@ -41,10 +40,13 @@ async fn load_remotes() -> Result<Vec<Remote>, Error> { >> proxmox_yew_comp::http_get("/remotes/remote", None).await >> } >> >> -async fn delete_item(key: Key) -> Result<(), Error> { >> +async fn delete_item(key: Key, delete_token: bool) -> Result<(), Error> { >> let id = key.to_string(); >> - let url = format!("/remotes/remote/{}", percent_encode_component(&id)); >> - proxmox_yew_comp::http_delete(&url, None).await?; >> + let param = Some(json!({ >> + "delete-token": delete_token, >> + })); >> + let url = format!("/remotes/remote/{}", percent_encode_component(&id),); > > nit: is there a reason for this added comma at the end? It was not intended. Will remove it. >> + proxmox_yew_comp::http_delete(&url, param).await?; >> Ok(()) >> } >> >> @@ -99,11 +101,12 @@ impl RemoteConfigPanel { >> pub enum ViewState { >> Add(RemoteType), >> Edit, >> + Remove, >> } >> >> pub enum Msg { >> SelectionChange, >> - RemoveItem, >> + RemoveItem(bool), >> } >> >> pub struct PbsRemoteConfigPanel { >> @@ -146,11 +149,11 @@ impl LoadableComponent for PbsRemoteConfigPanel { >> fn update(&mut self, ctx: &LoadableComponentContext<Self>, msg: Self::Message) -> bool { >> match msg { >> Msg::SelectionChange => true, >> - Msg::RemoveItem => { >> + Msg::RemoveItem(v) => { >> if let Some(key) = self.selection.selected_key() { >> let link = ctx.link(); >> link.clone().spawn(async move { >> - if let Err(err) = delete_item(key).await { >> + if let Err(err) = delete_item(key, v).await { >> link.show_error(tr!("Unable to delete item"), err, true); >> } >> link.send_reload(); >> @@ -195,10 +198,9 @@ impl LoadableComponent for PbsRemoteConfigPanel { >> .onclick(link.change_view_callback(|_| Some(ViewState::Edit))), >> ) >> .with_child( >> - ConfirmButton::new(tr!("Remove")) >> - .confirm_message(tr!("Are you sure you want to remove this remote?")) >> + Button::new(tr!("Remove")) >> .disabled(disabled) >> - .on_activate(link.callback(|_| Msg::RemoveItem)), >> + .onclick(link.change_view_callback(|_| Some(ViewState::Remove))), >> ) >> .with_flex_spacer() >> .with_child({ >> @@ -233,6 +235,7 @@ impl LoadableComponent for PbsRemoteConfigPanel { >> .selection >> .selected_key() >> .map(|key| self.create_edit_dialog(ctx, key)), >> + ViewState::Remove => Some(self.create_remove_remote_dialog(ctx)), >> } >> } >> } >> @@ -293,6 +296,19 @@ impl PbsRemoteConfigPanel { >> .on_done(ctx.link().change_view_callback(|_| None)) >> .into() >> } >> + >> + fn create_remove_remote_dialog(&self, ctx: &LoadableComponentContext<Self>) -> Html { >> + let link = ctx.link(); >> + let close = link.change_view_callback(|_| None); >> + >> + RemoveRemote::new() >> + .on_dismiss(close.clone()) >> + .on_confirm(Callback::from(move |v| { >> + link.send_message(Msg::RemoveItem(v)); >> + link.change_view(None); >> + })) >> + .into() >> + } >> } >> >> impl From<RemoteConfigPanel> for VNode { >> diff --git a/ui/src/remotes/mod.rs b/ui/src/remotes/mod.rs >> index 603077c..6912ca9 100644 >> --- a/ui/src/remotes/mod.rs >> +++ b/ui/src/remotes/mod.rs >> @@ -27,6 +27,9 @@ pub use tasks::RemoteTaskList; >> mod updates; >> pub use updates::UpdateTree; >> >> +mod remove_remote; >> +pub use remove_remote::RemoveRemote; >> + >> mod firewall; >> pub use firewall::FirewallTree; >> >> diff --git a/ui/src/remotes/remove_remote.rs b/ui/src/remotes/remove_remote.rs >> new file mode 100644 >> index 0000000..e26d563 >> --- /dev/null >> +++ b/ui/src/remotes/remove_remote.rs >> @@ -0,0 +1,122 @@ >> +use std::rc::Rc; >> + >> +use yew::{ >> + html::IntoEventCallback, >> + prelude::*, >> + virtual_dom::{VComp, VNode}, >> +}; >> + >> +use pwt::{ >> + css::{AlignItems, FontColor, JustifyContent}, >> + props::{ >> + ContainerBuilder, CssPaddingBuilder, EventSubscriber, WidgetBuilder, WidgetStyleBuilder, >> + }, >> + tr, >> + widget::{form::Checkbox, Button, Column, Container, Dialog, Fa, Row}, >> +}; >> +use pwt_macros::builder; >> + >> +#[derive(PartialEq, Properties)] >> +#[builder] >> +pub struct RemoveRemote { >> + /// A callback for an action that needs to be confirmed by the user. >> + #[prop_or_default] >> + #[builder_cb(IntoEventCallback, into_event_callback, bool)] >> + pub on_confirm: Option<Callback<bool>>, >> + >> + /// A callback that will trigger if the user dismisses the action. >> + #[prop_or_default] >> + #[builder_cb(IntoEventCallback, into_event_callback, ())] >> + pub on_dismiss: Option<Callback<()>>, >> +} >> + >> +impl RemoveRemote { >> + pub fn new() -> Self { >> + yew::props!(Self {}) >> + } >> +} >> + >> +pub enum Msg { >> + SelectCheckBox(bool), >> +} >> + >> +pub struct PdmRemoveRemote { >> + delete_token: bool, >> +} >> + >> +impl Component for PdmRemoveRemote { >> + type Message = Msg; >> + >> + type Properties = RemoveRemote; >> + >> + fn create(_ctx: &Context<Self>) -> Self { >> + Self { >> + delete_token: false, >> + } >> + } >> + >> + fn update(&mut self, _ctx: &Context<Self>, msg: Self::Message) -> bool { >> + match msg { >> + Msg::SelectCheckBox(v) => { >> + self.delete_token = v; >> + true >> + } >> + } >> + } >> + >> + fn view(&self, ctx: &Context<Self>) -> Html { >> + let props = ctx.props(); >> + let delete_token = self.delete_token.clone(); > > the clone() here is unnecessary thanks will udpate it. >> + >> + let on_confirm = props.on_confirm.clone(); >> + let on_dismiss = props.on_dismiss.clone(); >> + >> + Dialog::new(tr!("Confirm")) >> + .on_close(on_dismiss.clone()) >> + .min_height(100) >> + .with_child( >> + Column::new() >> + .padding(4) >> + .gap(2) >> + .with_child( >> + Row::new() >> + .gap(2) >> + .class(AlignItems::Center) >> + .with_child(Container::new().class("pwt-message-sign").with_child( >> + Fa::new("exclamation-triangle").class(FontColor::Error), >> + )) >> + .with_child(tr!("Are you sure you want to remove this remote?")), >> + ) >> + .with_child( >> + Checkbox::new() >> + .default(false) >> + .box_label(tr!("Delete API token from remote")) >> + .checked(self.delete_token) >> + .on_change(ctx.link().callback(|v| Msg::SelectCheckBox(v))), >> + ) >> + .with_child( >> + Row::new() >> + .gap(2) >> + .class(JustifyContent::Center) >> + .with_child(Button::new(tr!("Yes")).onclick(move |_| { >> + if let Some(on_confirm) = &on_confirm { >> + on_confirm.emit(delete_token); >> + } >> + })) >> + .with_child(Button::new(tr!("No")).onclick(move |_| { >> + if let Some(on_dismiss) = &on_dismiss { >> + on_dismiss.emit(()); >> + } >> + })), >> + ), >> + ) >> + .into() >> + } >> +} >> + >> +impl From<RemoveRemote> for VNode { >> + fn from(val: RemoveRemote) -> Self { >> + let comp = VComp::new::<PdmRemoveRemote>(Rc::new(val), None); >> + VNode::from(comp) >> + } >> +} > > > > _______________________________________________ > 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [pdm-devel] [PATCH proxmox 1/1] pve-api-types: generate missing `delete_token` method 2025-12-05 18:04 [pdm-devel] [PATCH datacenter-manager/proxmox 0/6] fix #6914: add option to remove already existing token Shan Shaji ` (4 preceding siblings ...) 2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 5/5] fix: ui: add remove confirmation dialog with optional token deletion Shan Shaji @ 2025-12-05 18:04 ` Shan Shaji 2025-12-09 9:37 ` Shannon Sterz 2025-12-09 14:56 ` [pdm-devel] [PATCH datacenter-manager/proxmox 0/6] fix #6914: add option to remove already existing token Michael Köppl 6 siblings, 1 reply; 24+ messages in thread From: Shan Shaji @ 2025-12-05 18:04 UTC (permalink / raw) To: pdm-devel Removing a remote node from PDM and adding it again is prevented by the already existing token. Inorder to allow deletion of token from PVE generate the endpoint that is necessary to call from PDM. Signed-off-by: Shan Shaji <s.shaji@proxmox.com> --- pve-api-types/generate.pl | 1 + pve-api-types/src/generated/code.rs | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/pve-api-types/generate.pl b/pve-api-types/generate.pl index 3cebe321..ee02d91b 100755 --- a/pve-api-types/generate.pl +++ b/pve-api-types/generate.pl @@ -359,6 +359,7 @@ Schema2Rust::derive('ListRealm' => 'Clone', 'PartialEq'); # api(GET => '/access/users/{userid}', 'get_user', 'return-name' => 'User'); api(POST => '/access/users/{userid}/token/{tokenid}', 'create_token', 'param-name' => 'CreateToken'); Schema2Rust::derive('CreateToken' => 'Default'); +api(DELETE => '/access/users/{userid}/token/{tokenid}', 'delete_token'); api(GET => '/nodes/{node}/apt/update', 'list_available_updates', 'return-name' => 'AptUpdateInfo'); api(POST => '/nodes/{node}/apt/update', 'update_apt_database', 'output-type' => 'PveUpid', 'param-name' => 'AptUpdateParams'); diff --git a/pve-api-types/src/generated/code.rs b/pve-api-types/src/generated/code.rs index f364f9cd..3de2e554 100644 --- a/pve-api-types/src/generated/code.rs +++ b/pve-api-types/src/generated/code.rs @@ -450,6 +450,11 @@ pub trait PveClient { Err(Error::Other("get_apt_repositories not implemented")) } + /// Remove API token for a specific user. + async fn delete_token(&self, userid: &str, tokenid: &str) -> Result<(), Error> { + Err(Error::Other("delete_token not implemented")) + } + /// Get package changelogs. async fn get_package_changelog( &self, @@ -1089,6 +1094,12 @@ where Ok(self.0.get(url).await?.expect_json()?.data) } + /// Remove API token for a specific user. + async fn delete_token(&self, userid: &str, tokenid: &str) -> Result<(), Error> { + let url = &format!("/api2/extjs/access/users/{userid}/token/{tokenid}"); + self.0.delete(url).await?.nodata() + } + /// Get package changelogs. async fn get_package_changelog( &self, -- 2.47.3 _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [pdm-devel] [PATCH proxmox 1/1] pve-api-types: generate missing `delete_token` method 2025-12-05 18:04 ` [pdm-devel] [PATCH proxmox 1/1] pve-api-types: generate missing `delete_token` method Shan Shaji @ 2025-12-09 9:37 ` Shannon Sterz 0 siblings, 0 replies; 24+ messages in thread From: Shannon Sterz @ 2025-12-09 9:37 UTC (permalink / raw) To: Shan Shaji; +Cc: Proxmox Datacenter Manager development discussion other than the nits in-line, consider this: Reviewed-by: Shannon Sterz <s.sterz@proxmox.com> note i didn't test this, but other then the nits the code looks ok to me. On Fri Dec 5, 2025 at 7:04 PM CET, Shan Shaji wrote: > Removing a remote node from PDM and adding it again is prevented > by the already existing token. Inorder to allow deletion of token > from PVE generate the endpoint that is necessary to call from > PDM. > > Signed-off-by: Shan Shaji <s.shaji@proxmox.com> > --- > pve-api-types/generate.pl | 1 + > pve-api-types/src/generated/code.rs | 11 +++++++++++ > 2 files changed, 12 insertions(+) > > diff --git a/pve-api-types/generate.pl b/pve-api-types/generate.pl > index 3cebe321..ee02d91b 100755 > --- a/pve-api-types/generate.pl > +++ b/pve-api-types/generate.pl > @@ -359,6 +359,7 @@ Schema2Rust::derive('ListRealm' => 'Clone', 'PartialEq'); > # api(GET => '/access/users/{userid}', 'get_user', 'return-name' => 'User'); > api(POST => '/access/users/{userid}/token/{tokenid}', 'create_token', 'param-name' => 'CreateToken'); > Schema2Rust::derive('CreateToken' => 'Default'); > +api(DELETE => '/access/users/{userid}/token/{tokenid}', 'delete_token'); > > api(GET => '/nodes/{node}/apt/update', 'list_available_updates', 'return-name' => 'AptUpdateInfo'); > api(POST => '/nodes/{node}/apt/update', 'update_apt_database', 'output-type' => 'PveUpid', 'param-name' => 'AptUpdateParams'); > diff --git a/pve-api-types/src/generated/code.rs b/pve-api-types/src/generated/code.rs > index f364f9cd..3de2e554 100644 > --- a/pve-api-types/src/generated/code.rs > +++ b/pve-api-types/src/generated/code.rs > @@ -450,6 +450,11 @@ pub trait PveClient { > Err(Error::Other("get_apt_repositories not implemented")) > } > > + /// Remove API token for a specific user. > + async fn delete_token(&self, userid: &str, tokenid: &str) -> Result<(), Error> { > + Err(Error::Other("delete_token not implemented")) > + } > + > /// Get package changelogs. > async fn get_package_changelog( > &self, > @@ -1089,6 +1094,12 @@ where > Ok(self.0.get(url).await?.expect_json()?.data) > } > > + /// Remove API token for a specific user. > + async fn delete_token(&self, userid: &str, tokenid: &str) -> Result<(), Error> { > + let url = &format!("/api2/extjs/access/users/{userid}/token/{tokenid}"); > + self.0.delete(url).await?.nodata() > + } > + > /// Get package changelogs. > async fn get_package_changelog( > &self, _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager/proxmox 0/6] fix #6914: add option to remove already existing token 2025-12-05 18:04 [pdm-devel] [PATCH datacenter-manager/proxmox 0/6] fix #6914: add option to remove already existing token Shan Shaji ` (5 preceding siblings ...) 2025-12-05 18:04 ` [pdm-devel] [PATCH proxmox 1/1] pve-api-types: generate missing `delete_token` method Shan Shaji @ 2025-12-09 14:56 ` Michael Köppl 2025-12-09 17:02 ` Shan Shaji 6 siblings, 1 reply; 24+ messages in thread From: Michael Köppl @ 2025-12-09 14:56 UTC (permalink / raw) To: Proxmox Datacenter Manager development discussion; +Cc: pdm-devel Quickly tested this with my PDM instance. Noticed that: - when removing a remote and checking the option to also remove the token, the user will be unable to remove the remote if it is unreachable. The user will be presented an error, but the remote will remain since the call to remove the token will fail before. This is in contrast to the behavior for removing a remote without the token, which will remove the remote in any case. - the admin CLI does not have the delete-token parameter Other than that, seems to work as advertised. Also had a look at the code and left some notes on the individual patches. Consider this: Tested-by: Michael Köppl <m.koeppl@proxmox.com> On Fri Dec 5, 2025 at 7:04 PM CET, Shan Shaji wrote: > If a user removed a remote without deleting its associated token, PDM > would not allow re-adding the same remote unless the token name was > changed. To fix this, support for optionally deleting the token from > the remote has been added. > > This includes: > > - CLI: An optional flag to remove the token when removing the remote > - UI: A checkbox that allows users to delete the token along with the > remote > > proxmox-datacenter-manager: > > Shan Shaji (5): > server: pbs-client: add delete admin token method > server: api: add support to optionally delete token from remote > pdm-client: accept `delete-token` argument for deleting api token > cli: client: add `delete-token` option to delete token from remote > fix: ui: add remove confirmation dialog with optional token deletion > > cli/client/src/remotes.rs | 11 ++- > lib/pdm-client/src/lib.rs | 6 +- > server/src/api/remotes.rs | 45 +++++++++++- > server/src/pbs_client.rs | 9 ++- > ui/src/remotes/config.rs | 42 +++++++---- > ui/src/remotes/mod.rs | 3 + > ui/src/remotes/remove_remote.rs | 122 ++++++++++++++++++++++++++++++++ > 7 files changed, 217 insertions(+), 21 deletions(-) > create mode 100644 ui/src/remotes/remove_remote.rs > > > proxmox: > > Shan Shaji (1): > pve-api-types: generate missing `delete_token` method > > pve-api-types/generate.pl | 1 + > pve-api-types/src/generated/code.rs | 11 +++++++++++ > 2 files changed, 12 insertions(+) > > > Summary over all repositories: > 9 files changed, 229 insertions(+), 21 deletions(-) _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager/proxmox 0/6] fix #6914: add option to remove already existing token 2025-12-09 14:56 ` [pdm-devel] [PATCH datacenter-manager/proxmox 0/6] fix #6914: add option to remove already existing token Michael Köppl @ 2025-12-09 17:02 ` Shan Shaji 2025-12-10 16:40 ` Shan Shaji 0 siblings, 1 reply; 24+ messages in thread From: Shan Shaji @ 2025-12-09 17:02 UTC (permalink / raw) To: Proxmox Datacenter Manager development discussion; +Cc: pdm-devel Thank you so much for the nice reviews Michael. One small comment inline. On Tue Dec 9, 2025 at 3:56 PM CET, Michael Köppl wrote: > Quickly tested this with my PDM instance. Noticed that: > - when removing a remote and checking the option to also remove the > token, the user will be unable to remove the remote if it is > unreachable. The user will be presented an error, but the remote will > remain since the call to remove the token will fail before. This is in > contrast to the behavior for removing a remote without the token, > which will remove the remote in any case. This was actually intended. If the token deletion fails for some reason, to be on the safe side, i am delibrately not deleting the remote from our local config. > - the admin CLI does not have the delete-token parameter > > Other than that, seems to work as advertised. Also had a look at the > code and left some notes on the individual patches. > > Consider this: > Tested-by: Michael Köppl <m.koeppl@proxmox.com> > > On Fri Dec 5, 2025 at 7:04 PM CET, Shan Shaji wrote: >> If a user removed a remote without deleting its associated token, PDM >> would not allow re-adding the same remote unless the token name was >> changed. To fix this, support for optionally deleting the token from >> the remote has been added. >> >> This includes: >> >> - CLI: An optional flag to remove the token when removing the remote >> - UI: A checkbox that allows users to delete the token along with the >> remote >> >> proxmox-datacenter-manager: >> >> Shan Shaji (5): >> server: pbs-client: add delete admin token method >> server: api: add support to optionally delete token from remote >> pdm-client: accept `delete-token` argument for deleting api token >> cli: client: add `delete-token` option to delete token from remote >> fix: ui: add remove confirmation dialog with optional token deletion >> >> cli/client/src/remotes.rs | 11 ++- >> lib/pdm-client/src/lib.rs | 6 +- >> server/src/api/remotes.rs | 45 +++++++++++- >> server/src/pbs_client.rs | 9 ++- >> ui/src/remotes/config.rs | 42 +++++++---- >> ui/src/remotes/mod.rs | 3 + >> ui/src/remotes/remove_remote.rs | 122 ++++++++++++++++++++++++++++++++ >> 7 files changed, 217 insertions(+), 21 deletions(-) >> create mode 100644 ui/src/remotes/remove_remote.rs >> >> >> proxmox: >> >> Shan Shaji (1): >> pve-api-types: generate missing `delete_token` method >> >> pve-api-types/generate.pl | 1 + >> pve-api-types/src/generated/code.rs | 11 +++++++++++ >> 2 files changed, 12 insertions(+) >> >> >> Summary over all repositories: >> 9 files changed, 229 insertions(+), 21 deletions(-) > > > > _______________________________________________ > 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager/proxmox 0/6] fix #6914: add option to remove already existing token 2025-12-09 17:02 ` Shan Shaji @ 2025-12-10 16:40 ` Shan Shaji 0 siblings, 0 replies; 24+ messages in thread From: Shan Shaji @ 2025-12-10 16:40 UTC (permalink / raw) To: Shan Shaji, Proxmox Datacenter Manager development discussion; +Cc: pdm-devel Superseded by v2: https://lore.proxmox.com/pdm-devel/20251210163736.384795-1-s.shaji@proxmox.com/T/#t _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2025-12-10 16:39 UTC | newest] Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-12-05 18:04 [pdm-devel] [PATCH datacenter-manager/proxmox 0/6] fix #6914: add option to remove already existing token Shan Shaji 2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 1/5] server: pbs-client: add delete admin token method Shan Shaji 2025-12-09 9:36 ` Shannon Sterz 2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 2/5] server: api: add support to optionally delete token from remote Shan Shaji 2025-12-09 9:36 ` Shannon Sterz 2025-12-09 9:54 ` Shan Shaji 2025-12-09 12:34 ` Michael Köppl 2025-12-09 13:37 ` Shan Shaji 2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 3/5] pdm-client: accept `delete-token` argument for deleting api token Shan Shaji 2025-12-09 9:36 ` Shannon Sterz 2025-12-09 13:08 ` Michael Köppl 2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 4/5] cli: client: add `delete-token` option to delete token from remote Shan Shaji 2025-12-09 14:52 ` Michael Köppl 2025-12-09 16:25 ` Shan Shaji 2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 5/5] fix: ui: add remove confirmation dialog with optional token deletion Shan Shaji 2025-12-09 9:36 ` Shannon Sterz 2025-12-09 10:08 ` Shan Shaji 2025-12-09 13:38 ` Michael Köppl 2025-12-09 13:52 ` Shan Shaji 2025-12-05 18:04 ` [pdm-devel] [PATCH proxmox 1/1] pve-api-types: generate missing `delete_token` method Shan Shaji 2025-12-09 9:37 ` Shannon Sterz 2025-12-09 14:56 ` [pdm-devel] [PATCH datacenter-manager/proxmox 0/6] fix #6914: add option to remove already existing token Michael Köppl 2025-12-09 17:02 ` Shan Shaji 2025-12-10 16:40 ` Shan Shaji
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox