public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* Re: [pbs-devel] [PATCH proxmox-backup 1/6] src/server/worker_task.rs: Avoid using pbs-api-type::Authid
@ 2021-09-24  9:03 Dietmar Maurer
  0 siblings, 0 replies; 3+ messages in thread
From: Dietmar Maurer @ 2021-09-24  9:03 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Fabian Grünbichler

> On 09/23/2021 3:20 PM Fabian Grünbichler <f.gruenbichler@proxmox.com> wrote:
> 
>  
> On September 23, 2021 12:13 pm, Dietmar Maurer wrote:
> > Because we want to move worker_task.rs into proxmox-rest-server crate.
> 
> as discussed off-list, we could use a 
> 
> struct AuthidStr(String)
> 
> or a trait defined in proxmox instead of a regular String, to avoid 
> accidentally passing in other, non authid strings. all Authid 
> implementations would then need to implement Into<AuthidStr> (/the 
> trait), and the calls below to to_string() would become into() (remain 
> as they were).
> 
> can also be done as follow-up with a proxmox bump if we want to discuss 
> it more in-depth but still go ahead with the split/move now.

I am still not sure whats the best way to implement this, so I committed
what we have so far...




^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup 1/6] src/server/worker_task.rs: Avoid using pbs-api-type::Authid
  2021-09-23 10:13 Dietmar Maurer
@ 2021-09-23 13:20 ` Fabian Grünbichler
  0 siblings, 0 replies; 3+ messages in thread
From: Fabian Grünbichler @ 2021-09-23 13:20 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion

On September 23, 2021 12:13 pm, Dietmar Maurer wrote:
> Because we want to move worker_task.rs into proxmox-rest-server crate.

as discussed off-list, we could use a 

struct AuthidStr(String)

or a trait defined in proxmox instead of a regular String, to avoid 
accidentally passing in other, non authid strings. all Authid 
implementations would then need to implement Into<AuthidStr> (/the 
trait), and the calls below to to_string() would become into() (remain 
as they were).

can also be done as follow-up with a proxmox bump if we want to discuss 
it more in-depth but still go ahead with the split/move now.

> ---
>  pbs-api-types/src/upid.rs        | 13 +++++++------
>  src/api2/admin/datastore.rs      |  6 +++---
>  src/api2/backup/environment.rs   |  2 +-
>  src/api2/backup/mod.rs           |  2 +-
>  src/api2/config/acme.rs          |  6 +++---
>  src/api2/config/datastore.rs     |  4 ++--
>  src/api2/node/apt.rs             |  4 ++--
>  src/api2/node/certificates.rs    |  6 +++---
>  src/api2/node/disks/directory.rs |  4 ++--
>  src/api2/node/disks/mod.rs       |  4 ++--
>  src/api2/node/disks/zfs.rs       |  4 ++--
>  src/api2/node/mod.rs             |  2 +-
>  src/api2/node/network.rs         |  2 +-
>  src/api2/node/services.rs        |  2 +-
>  src/api2/node/tasks.rs           | 15 +++++++++------
>  src/api2/pull.rs                 |  4 ++--
>  src/api2/reader/mod.rs           |  2 +-
>  src/api2/tape/backup.rs          |  4 ++--
>  src/api2/tape/drive.rs           |  2 +-
>  src/api2/tape/restore.rs         |  2 +-
>  src/bin/proxmox-backup-proxy.rs  |  2 +-
>  src/server/gc_job.rs             |  2 +-
>  src/server/prune_job.rs          |  2 +-
>  src/server/verify_job.rs         |  2 +-
>  src/server/worker_task.rs        |  8 ++++----
>  25 files changed, 55 insertions(+), 51 deletions(-)
> 
> diff --git a/pbs-api-types/src/upid.rs b/pbs-api-types/src/upid.rs
> index ba23a646..29135bca 100644
> --- a/pbs-api-types/src/upid.rs
> +++ b/pbs-api-types/src/upid.rs
> @@ -8,8 +8,6 @@ use proxmox::api::schema::{ApiStringFormat, ApiType, Schema, StringSchema, Array
>  use proxmox::const_regex;
>  use proxmox::sys::linux::procfs;
>  
> -use crate::Authid;
> -
>  /// Unique Process/Task Identifier
>  ///
>  /// We use this to uniquely identify worker task. UPIDs have a short
> @@ -37,7 +35,7 @@ pub struct UPID {
>      /// Worker ID (arbitrary ASCII string)
>      pub worker_id: Option<String>,
>      /// The authenticated entity who started the task
> -    pub auth_id: Authid,
> +    pub auth_id: String,
>      /// The node name.
>      pub node: String,
>  }
> @@ -71,7 +69,7 @@ impl UPID {
>      pub fn new(
>          worker_type: &str,
>          worker_id: Option<String>,
> -        auth_id: Authid,
> +        auth_id: String,
>      ) -> Result<Self, Error> {
>  
>          let pid = unsafe { libc::getpid() };
> @@ -82,6 +80,10 @@ impl UPID {
>              bail!("illegal characters in worker type '{}'", worker_type);
>          }
>  
> +        if auth_id.contains(bad) {
> +            bail!("illegal characters in auth_id '{}'", auth_id);
> +        }
> +
>          static WORKER_TASK_NEXT_ID: AtomicUsize = AtomicUsize::new(0);
>  
>          let task_id = WORKER_TASK_NEXT_ID.fetch_add(1, Ordering::SeqCst);
> @@ -184,7 +186,7 @@ pub struct TaskListItem {
>      /// Worker ID (arbitrary ASCII string)
>      pub worker_id: Option<String>,
>      /// The authenticated entity who started the task
> -    pub user: Authid,
> +    pub user: String,
>      /// The task end time (Epoch)
>      #[serde(skip_serializing_if="Option::is_none")]
>      pub endtime: Option<i64>,
> @@ -200,4 +202,3 @@ pub const NODE_TASKS_LIST_TASKS_RETURN_TYPE: ReturnType = ReturnType {
>          &TaskListItem::API_SCHEMA,
>      ).schema(),
>  };
> -
> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
> index 0b14dfbf..fbb93f35 100644
> --- a/src/api2/admin/datastore.rs
> +++ b/src/api2/admin/datastore.rs
> @@ -722,7 +722,7 @@ pub fn verify(
>      let upid_str = WorkerTask::new_thread(
>          worker_type,
>          Some(worker_id),
> -        auth_id.clone(),
> +        auth_id.to_string(),
>          to_stdout,
>          move |worker| {
>              let verify_worker = crate::backup::VerifyWorker::new(worker.clone(), datastore);
> @@ -862,7 +862,7 @@ pub fn prune(
>  
>  
>      // We use a WorkerTask just to have a task log, but run synchrounously
> -    let worker = WorkerTask::new("prune", Some(worker_id), auth_id, true)?;
> +    let worker = WorkerTask::new("prune", Some(worker_id), auth_id.to_string(), true)?;
>  
>      if keep_all {
>          worker.log("No prune selection - keeping all files.");
> @@ -957,7 +957,7 @@ pub fn prune_datastore(
>      let upid_str = WorkerTask::new_thread(
>          "prune",
>          Some(store.clone()),
> -        auth_id.clone(),
> +        auth_id.to_string(),
>          to_stdout,
>          move |worker| crate::server::prune_datastore(
>              worker.clone(),
> diff --git a/src/api2/backup/environment.rs b/src/api2/backup/environment.rs
> index 129ebd2b..306f91ee 100644
> --- a/src/api2/backup/environment.rs
> +++ b/src/api2/backup/environment.rs
> @@ -525,7 +525,7 @@ impl BackupEnvironment {
>          WorkerTask::new_thread(
>              "verify",
>              Some(worker_id),
> -            self.auth_id.clone(),
> +            self.auth_id.to_string(),
>              false,
>              move |worker| {
>                  worker.log("Automatically verifying newly added snapshot");
> diff --git a/src/api2/backup/mod.rs b/src/api2/backup/mod.rs
> index 8f51f314..c14f19a4 100644
> --- a/src/api2/backup/mod.rs
> +++ b/src/api2/backup/mod.rs
> @@ -166,7 +166,7 @@ async move {
>      if !is_new { bail!("backup directory already exists."); }
>  
>  
> -    WorkerTask::spawn(worker_type, Some(worker_id), auth_id.clone(), true, move |worker| {
> +    WorkerTask::spawn(worker_type, Some(worker_id), auth_id.to_string(), true, move |worker| {
>          let mut env = BackupEnvironment::new(
>              env_type, auth_id, worker.clone(), datastore, backup_dir);
>  
> diff --git a/src/api2/config/acme.rs b/src/api2/config/acme.rs
> index 593b79a3..564cafae 100644
> --- a/src/api2/config/acme.rs
> +++ b/src/api2/config/acme.rs
> @@ -215,7 +215,7 @@ fn register_account(
>      WorkerTask::spawn(
>          "acme-register",
>          Some(name.to_string()),
> -        auth_id,
> +        auth_id.to_string(),
>          true,
>          move |worker| async move {
>              let mut client = AcmeClient::new(directory);
> @@ -275,7 +275,7 @@ pub fn update_account(
>      WorkerTask::spawn(
>          "acme-update",
>          Some(name.to_string()),
> -        auth_id,
> +        auth_id.to_string(),
>          true,
>          move |_worker| async move {
>              let data = match contact {
> @@ -320,7 +320,7 @@ pub fn deactivate_account(
>      WorkerTask::spawn(
>          "acme-deactivate",
>          Some(name.to_string()),
> -        auth_id,
> +        auth_id.to_string(),
>          true,
>          move |worker| async move {
>              match AcmeClient::load(&name)
> diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
> index c6036fc3..0e6529f8 100644
> --- a/src/api2/config/datastore.rs
> +++ b/src/api2/config/datastore.rs
> @@ -119,9 +119,9 @@ pub fn create_datastore(
>      WorkerTask::new_thread(
>          "create-datastore",
>          Some(config.name.to_string()),
> -        auth_id,
> +        auth_id.to_string(),
>          to_stdout,
> -        move |worker| do_create_datastore(lock, section_config, config, Some(&worker)),
> +       move |worker| do_create_datastore(lock, section_config, config, Some(&worker)),
>      )
>  }
>  
> diff --git a/src/api2/node/apt.rs b/src/api2/node/apt.rs
> index 8f4bc691..f02920c0 100644
> --- a/src/api2/node/apt.rs
> +++ b/src/api2/node/apt.rs
> @@ -14,7 +14,7 @@ use proxmox_apt::repositories::{
>  use proxmox_http::ProxyConfig;
>  
>  use pbs_api_types::{
> -    Authid, APTUpdateInfo, NODE_SCHEMA, PROXMOX_CONFIG_DIGEST_SCHEMA, UPID_SCHEMA,
> +    APTUpdateInfo, NODE_SCHEMA, PROXMOX_CONFIG_DIGEST_SCHEMA, UPID_SCHEMA,
>      PRIV_SYS_AUDIT, PRIV_SYS_MODIFY,
>  };
>  
> @@ -154,7 +154,7 @@ pub fn apt_update_database(
>      rpcenv: &mut dyn RpcEnvironment,
>  ) -> Result<String, Error> {
>  
> -    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> +    let auth_id = rpcenv.get_auth_id().unwrap();
>      let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
>  
>      let upid_str = WorkerTask::new_thread("aptupdate", None, auth_id, to_stdout, move |worker| {
> diff --git a/src/api2/node/certificates.rs b/src/api2/node/certificates.rs
> index 82fa028d..7b31861e 100644
> --- a/src/api2/node/certificates.rs
> +++ b/src/api2/node/certificates.rs
> @@ -11,7 +11,7 @@ use proxmox::api::router::SubdirMap;
>  use proxmox::api::{api, Permission, Router, RpcEnvironment};
>  use proxmox::list_subdirs_api_method;
>  
> -use pbs_api_types::{Authid, NODE_SCHEMA, PRIV_SYS_MODIFY};
> +use pbs_api_types::{NODE_SCHEMA, PRIV_SYS_MODIFY};
>  use pbs_buildcfg::configdir;
>  use pbs_tools::cert;
>  
> @@ -530,7 +530,7 @@ fn spawn_certificate_worker(
>  
>      let (node_config, _digest) = crate::config::node::config()?;
>  
> -    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> +    let auth_id = rpcenv.get_auth_id().unwrap();
>  
>      WorkerTask::spawn(name, None, auth_id, true, move |worker| async move {
>          if let Some(cert) = order_certificate(worker, &node_config).await? {
> @@ -559,7 +559,7 @@ pub fn revoke_acme_cert(rpcenv: &mut dyn RpcEnvironment) -> Result<String, Error
>  
>      let cert_pem = get_certificate_pem()?;
>  
> -    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> +    let auth_id = rpcenv.get_auth_id().unwrap();
>  
>      WorkerTask::spawn(
>          "acme-revoke-cert",
> diff --git a/src/api2/node/disks/directory.rs b/src/api2/node/disks/directory.rs
> index 38809dcf..2f4a738d 100644
> --- a/src/api2/node/disks/directory.rs
> +++ b/src/api2/node/disks/directory.rs
> @@ -7,7 +7,7 @@ use proxmox::api::section_config::SectionConfigData;
>  use proxmox::api::router::Router;
>  
>  use pbs_api_types::{
> -    Authid, DataStoreConfig, NODE_SCHEMA, BLOCKDEVICE_NAME_SCHEMA,
> +    DataStoreConfig, NODE_SCHEMA, BLOCKDEVICE_NAME_SCHEMA,
>      DATASTORE_SCHEMA, UPID_SCHEMA, PRIV_SYS_AUDIT, PRIV_SYS_MODIFY,
>  };
>  
> @@ -146,7 +146,7 @@ pub fn create_datastore_disk(
>  
>      let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
>  
> -    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> +    let auth_id = rpcenv.get_auth_id().unwrap();
>  
>      let info = get_disk_usage_info(&disk, true)?;
>  
> diff --git a/src/api2/node/disks/mod.rs b/src/api2/node/disks/mod.rs
> index 67f8f63a..b4001a54 100644
> --- a/src/api2/node/disks/mod.rs
> +++ b/src/api2/node/disks/mod.rs
> @@ -7,7 +7,7 @@ use proxmox::{sortable, identity};
>  use proxmox::{list_subdirs_api_method};
>  
>  use pbs_api_types::{
> -    Authid, UPID_SCHEMA, NODE_SCHEMA, BLOCKDEVICE_NAME_SCHEMA,
> +    UPID_SCHEMA, NODE_SCHEMA, BLOCKDEVICE_NAME_SCHEMA,
>      PRIV_SYS_AUDIT, PRIV_SYS_MODIFY,
>  };
>  
> @@ -144,7 +144,7 @@ pub fn initialize_disk(
>  
>      let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
>  
> -    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> +    let auth_id = rpcenv.get_auth_id().unwrap();
>  
>      let info = get_disk_usage_info(&disk, true)?;
>  
> diff --git a/src/api2/node/disks/zfs.rs b/src/api2/node/disks/zfs.rs
> index 14c2cfec..9fe0dac4 100644
> --- a/src/api2/node/disks/zfs.rs
> +++ b/src/api2/node/disks/zfs.rs
> @@ -8,7 +8,7 @@ use proxmox::api::{
>  use proxmox::api::router::Router;
>  
>  use pbs_api_types::{
> -    Authid, ZpoolListItem, ZfsRaidLevel, ZfsCompressionType, DataStoreConfig,
> +    ZpoolListItem, ZfsRaidLevel, ZfsCompressionType, DataStoreConfig,
>      NODE_SCHEMA, ZPOOL_NAME_SCHEMA, DATASTORE_SCHEMA, DISK_ARRAY_SCHEMA,
>      DISK_LIST_SCHEMA, ZFS_ASHIFT_SCHEMA, UPID_SCHEMA,
>      PRIV_SYS_AUDIT, PRIV_SYS_MODIFY,
> @@ -168,7 +168,7 @@ pub fn create_zpool(
>  
>      let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
>  
> -    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> +    let auth_id = rpcenv.get_auth_id().unwrap();
>  
>      let add_datastore = add_datastore.unwrap_or(false);
>  
> diff --git a/src/api2/node/mod.rs b/src/api2/node/mod.rs
> index 9b31d595..8e357311 100644
> --- a/src/api2/node/mod.rs
> +++ b/src/api2/node/mod.rs
> @@ -146,7 +146,7 @@ async fn termproxy(cmd: Option<String>, rpcenv: &mut dyn RpcEnvironment) -> Resu
>      let upid = WorkerTask::spawn(
>          "termproxy",
>          None,
> -        auth_id,
> +        auth_id.to_string(),
>          false,
>          move |worker| async move {
>              // move inside the worker so that it survives and does not close the port
> diff --git a/src/api2/node/network.rs b/src/api2/node/network.rs
> index 351fd11c..0fde9f2a 100644
> --- a/src/api2/node/network.rs
> +++ b/src/api2/node/network.rs
> @@ -703,7 +703,7 @@ pub async fn reload_network_config(
>  
>      let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
>  
> -    let upid_str = WorkerTask::spawn("srvreload", Some(String::from("networking")), auth_id, true, |_worker| async {
> +    let upid_str = WorkerTask::spawn("srvreload", Some(String::from("networking")), auth_id.to_string(), true, |_worker| async {
>  
>          let _ = std::fs::rename(network::NETWORK_INTERFACES_NEW_FILENAME, network::NETWORK_INTERFACES_FILENAME);
>  
> diff --git a/src/api2/node/services.rs b/src/api2/node/services.rs
> index 25edd1b6..6c757f43 100644
> --- a/src/api2/node/services.rs
> +++ b/src/api2/node/services.rs
> @@ -195,7 +195,7 @@ fn run_service_command(service: &str, cmd: &str, auth_id: Authid) -> Result<Valu
>      let upid = WorkerTask::new_thread(
>          &workerid,
>          Some(service.clone()),
> -        auth_id,
> +        auth_id.to_string(),
>          false,
>          move |_worker| {
>  
> diff --git a/src/api2/node/tasks.rs b/src/api2/node/tasks.rs
> index 7066f889..169a3d4d 100644
> --- a/src/api2/node/tasks.rs
> +++ b/src/api2/node/tasks.rs
> @@ -99,8 +99,8 @@ fn check_job_store(upid: &UPID, store: &str) -> bool {
>  }
>  
>  fn check_task_access(auth_id: &Authid, upid: &UPID) -> Result<(), Error> {
> -    let task_auth_id = &upid.auth_id;
> -    if auth_id == task_auth_id
> +    let task_auth_id: Authid = upid.auth_id.parse()?;
> +    if auth_id == &task_auth_id
>          || (task_auth_id.is_token() && &Authid::from(task_auth_id.user().clone()) == auth_id) {
>          // task owner can always read
>          Ok(())
> @@ -200,6 +200,8 @@ async fn get_task_status(
>      let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
>      check_task_access(&auth_id, &upid)?;
>  
> +    let task_auth_id: Authid = upid.auth_id.parse()?;
> +
>      let mut result = json!({
>          "upid": param["upid"],
>          "node": upid.node,
> @@ -208,11 +210,11 @@ async fn get_task_status(
>          "starttime": upid.starttime,
>          "type": upid.worker_type,
>          "id": upid.worker_id,
> -        "user": upid.auth_id.user(),
> +        "user": task_auth_id.user(),
>      });
>  
> -    if upid.auth_id.is_token() {
> -        result["tokenid"] = Value::from(upid.auth_id.tokenname().unwrap().as_str());
> +    if task_auth_id.is_token() {
> +        result["tokenid"] = Value::from(task_auth_id.tokenname().unwrap().as_str());
>      }
>  
>      if crate::server::worker_is_active(&upid).await? {
> @@ -344,10 +346,11 @@ fn stop_task(
>  
>      let upid = extract_upid(&param)?;
>  
> -    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> +    let auth_id = rpcenv.get_auth_id().unwrap();
>  
>      if auth_id != upid.auth_id {
>          let user_info = CachedUserInfo::new()?;
> +        let auth_id: Authid = auth_id.parse()?;
>          user_info.check_privs(&auth_id, &["system", "tasks"], PRIV_SYS_MODIFY, false)?;
>      }
>  
> diff --git a/src/api2/pull.rs b/src/api2/pull.rs
> index 1eb86ea3..e631920f 100644
> --- a/src/api2/pull.rs
> +++ b/src/api2/pull.rs
> @@ -81,7 +81,7 @@ pub fn do_sync_job(
>      let upid_str = WorkerTask::spawn(
>          &worker_type,
>          Some(job_id.clone()),
> -        auth_id.clone(),
> +        auth_id.to_string(),
>          to_stdout,
>          move |worker| async move {
>  
> @@ -183,7 +183,7 @@ async fn pull (
>      let (client, src_repo, tgt_store) = get_pull_parameters(&store, &remote, &remote_store).await?;
>  
>      // fixme: set to_stdout to false?
> -    let upid_str = WorkerTask::spawn("sync", Some(store.clone()), auth_id.clone(), true, move |worker| async move {
> +    let upid_str = WorkerTask::spawn("sync", Some(store.clone()), auth_id.to_string(), true, move |worker| async move {
>  
>          worker.log(format!("sync datastore '{}' start", store));
>  
> diff --git a/src/api2/reader/mod.rs b/src/api2/reader/mod.rs
> index 821e83c4..fada952c 100644
> --- a/src/api2/reader/mod.rs
> +++ b/src/api2/reader/mod.rs
> @@ -143,7 +143,7 @@ fn upgrade_to_backup_reader_protocol(
>  
>          let worker_id = format!("{}:{}/{}/{:08X}", store, backup_type, backup_id, backup_dir.backup_time());
>  
> -        WorkerTask::spawn("reader", Some(worker_id), auth_id.clone(), true, move |worker| async move {
> +        WorkerTask::spawn("reader", Some(worker_id), auth_id.to_string(), true, move |worker| async move {
>              let _guard = _guard;
>  
>              let mut env = ReaderEnvironment::new(
> diff --git a/src/api2/tape/backup.rs b/src/api2/tape/backup.rs
> index 6b533820..fadbfa3d 100644
> --- a/src/api2/tape/backup.rs
> +++ b/src/api2/tape/backup.rs
> @@ -195,7 +195,7 @@ pub fn do_tape_backup_job(
>      let upid_str = WorkerTask::new_thread(
>          &worker_type,
>          Some(job_id.clone()),
> -        auth_id.clone(),
> +        auth_id.to_string(),
>          to_stdout,
>          move |worker| {
>              job.start(&worker.upid().to_string())?;
> @@ -376,7 +376,7 @@ pub fn backup(
>      let upid_str = WorkerTask::new_thread(
>          "tape-backup",
>          Some(job_id),
> -        auth_id,
> +        auth_id.to_string(),
>          to_stdout,
>          move |worker| {
>              let _drive_lock = drive_lock; // keep lock guard
> diff --git a/src/api2/tape/drive.rs b/src/api2/tape/drive.rs
> index 58f49f43..10aa6842 100644
> --- a/src/api2/tape/drive.rs
> +++ b/src/api2/tape/drive.rs
> @@ -87,7 +87,7 @@ where
>      let (config, _digest) = pbs_config::drive::config()?;
>      let lock_guard = lock_tape_device(&config, &drive)?;
>  
> -    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> +    let auth_id = rpcenv.get_auth_id().unwrap();
>      let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
>  
>      WorkerTask::new_thread(worker_type, job_id, auth_id, to_stdout, move |worker| {
> diff --git a/src/api2/tape/restore.rs b/src/api2/tape/restore.rs
> index 4ab60e8f..7739d1a4 100644
> --- a/src/api2/tape/restore.rs
> +++ b/src/api2/tape/restore.rs
> @@ -275,7 +275,7 @@ pub fn restore(
>      let upid_str = WorkerTask::new_thread(
>          "tape-restore",
>          Some(taskid),
> -        auth_id.clone(),
> +        auth_id.to_string(),
>          to_stdout,
>          move |worker| {
>              let _drive_lock = drive_lock; // keep lock guard
> diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
> index 6734525b..518054bf 100644
> --- a/src/bin/proxmox-backup-proxy.rs
> +++ b/src/bin/proxmox-backup-proxy.rs
> @@ -745,7 +745,7 @@ async fn schedule_task_log_rotate() {
>      if let Err(err) = WorkerTask::new_thread(
>          worker_type,
>          None,
> -        Authid::root_auth_id().clone(),
> +        Authid::root_auth_id().to_string(),
>          false,
>          move |worker| {
>              job.start(&worker.upid().to_string())?;
> diff --git a/src/server/gc_job.rs b/src/server/gc_job.rs
> index 665b9631..317f4a36 100644
> --- a/src/server/gc_job.rs
> +++ b/src/server/gc_job.rs
> @@ -26,7 +26,7 @@ pub fn do_garbage_collection_job(
>      let upid_str = WorkerTask::new_thread(
>          &worker_type,
>          Some(store.clone()),
> -        auth_id.clone(),
> +        auth_id.to_string(),
>          to_stdout,
>          move |worker| {
>              job.start(&worker.upid().to_string())?;
> diff --git a/src/server/prune_job.rs b/src/server/prune_job.rs
> index f298a74c..8d971a1c 100644
> --- a/src/server/prune_job.rs
> +++ b/src/server/prune_job.rs
> @@ -105,7 +105,7 @@ pub fn do_prune_job(
>      let upid_str = WorkerTask::new_thread(
>          &worker_type,
>          Some(job.jobname().to_string()),
> -        auth_id.clone(),
> +        auth_id.to_string(),
>          false,
>          move |worker| {
>              job.start(&worker.upid().to_string())?;
> diff --git a/src/server/verify_job.rs b/src/server/verify_job.rs
> index 7f6d73ff..6005b706 100644
> --- a/src/server/verify_job.rs
> +++ b/src/server/verify_job.rs
> @@ -36,7 +36,7 @@ pub fn do_verification_job(
>      let upid_str = WorkerTask::new_thread(
>          &worker_type,
>          Some(job_id.clone()),
> -        auth_id.clone(),
> +        auth_id.to_string(),
>          to_stdout,
>          move |worker| {
>              job.start(&worker.upid().to_string())?;
> diff --git a/src/server/worker_task.rs b/src/server/worker_task.rs
> index a74b18e1..92ab50d7 100644
> --- a/src/server/worker_task.rs
> +++ b/src/server/worker_task.rs
> @@ -18,7 +18,7 @@ use proxmox::tools::fs::{create_path, replace_file, CreateOptions};
>  
>  use pbs_buildcfg;
>  use pbs_tools::logrotate::{LogRotate, LogRotateFiles};
> -use pbs_api_types::{Authid, UPID};
> +use pbs_api_types::UPID;
>  use pbs_config::{open_backup_lockfile, BackupLockGuard};
>  use proxmox_rest_server::{CommandoSocket, FileLogger, FileLogOptions};
>  
> @@ -589,7 +589,7 @@ struct WorkerTaskData {
>  
>  impl WorkerTask {
>  
> -    pub fn new(worker_type: &str, worker_id: Option<String>, auth_id: Authid, to_stdout: bool) -> Result<Arc<Self>, Error> {
> +    pub fn new(worker_type: &str, worker_id: Option<String>, auth_id: String, to_stdout: bool) -> Result<Arc<Self>, Error> {
>          let upid = UPID::new(worker_type, worker_id, auth_id)?;
>          let task_id = upid.task_id;
>  
> @@ -640,7 +640,7 @@ impl WorkerTask {
>      pub fn spawn<F, T>(
>          worker_type: &str,
>          worker_id: Option<String>,
> -        auth_id: Authid,
> +        auth_id: String,
>          to_stdout: bool,
>          f: F,
>      ) -> Result<String, Error>
> @@ -662,7 +662,7 @@ impl WorkerTask {
>      pub fn new_thread<F>(
>          worker_type: &str,
>          worker_id: Option<String>,
> -        auth_id: Authid,
> +        auth_id: String,
>          to_stdout: bool,
>          f: F,
>      ) -> Result<String, Error>
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
> 




^ permalink raw reply	[flat|nested] 3+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 1/6] src/server/worker_task.rs: Avoid using pbs-api-type::Authid
@ 2021-09-23 10:13 Dietmar Maurer
  2021-09-23 13:20 ` Fabian Grünbichler
  0 siblings, 1 reply; 3+ messages in thread
From: Dietmar Maurer @ 2021-09-23 10:13 UTC (permalink / raw)
  To: pbs-devel

Because we want to move worker_task.rs into proxmox-rest-server crate.
---
 pbs-api-types/src/upid.rs        | 13 +++++++------
 src/api2/admin/datastore.rs      |  6 +++---
 src/api2/backup/environment.rs   |  2 +-
 src/api2/backup/mod.rs           |  2 +-
 src/api2/config/acme.rs          |  6 +++---
 src/api2/config/datastore.rs     |  4 ++--
 src/api2/node/apt.rs             |  4 ++--
 src/api2/node/certificates.rs    |  6 +++---
 src/api2/node/disks/directory.rs |  4 ++--
 src/api2/node/disks/mod.rs       |  4 ++--
 src/api2/node/disks/zfs.rs       |  4 ++--
 src/api2/node/mod.rs             |  2 +-
 src/api2/node/network.rs         |  2 +-
 src/api2/node/services.rs        |  2 +-
 src/api2/node/tasks.rs           | 15 +++++++++------
 src/api2/pull.rs                 |  4 ++--
 src/api2/reader/mod.rs           |  2 +-
 src/api2/tape/backup.rs          |  4 ++--
 src/api2/tape/drive.rs           |  2 +-
 src/api2/tape/restore.rs         |  2 +-
 src/bin/proxmox-backup-proxy.rs  |  2 +-
 src/server/gc_job.rs             |  2 +-
 src/server/prune_job.rs          |  2 +-
 src/server/verify_job.rs         |  2 +-
 src/server/worker_task.rs        |  8 ++++----
 25 files changed, 55 insertions(+), 51 deletions(-)

diff --git a/pbs-api-types/src/upid.rs b/pbs-api-types/src/upid.rs
index ba23a646..29135bca 100644
--- a/pbs-api-types/src/upid.rs
+++ b/pbs-api-types/src/upid.rs
@@ -8,8 +8,6 @@ use proxmox::api::schema::{ApiStringFormat, ApiType, Schema, StringSchema, Array
 use proxmox::const_regex;
 use proxmox::sys::linux::procfs;
 
-use crate::Authid;
-
 /// Unique Process/Task Identifier
 ///
 /// We use this to uniquely identify worker task. UPIDs have a short
@@ -37,7 +35,7 @@ pub struct UPID {
     /// Worker ID (arbitrary ASCII string)
     pub worker_id: Option<String>,
     /// The authenticated entity who started the task
-    pub auth_id: Authid,
+    pub auth_id: String,
     /// The node name.
     pub node: String,
 }
@@ -71,7 +69,7 @@ impl UPID {
     pub fn new(
         worker_type: &str,
         worker_id: Option<String>,
-        auth_id: Authid,
+        auth_id: String,
     ) -> Result<Self, Error> {
 
         let pid = unsafe { libc::getpid() };
@@ -82,6 +80,10 @@ impl UPID {
             bail!("illegal characters in worker type '{}'", worker_type);
         }
 
+        if auth_id.contains(bad) {
+            bail!("illegal characters in auth_id '{}'", auth_id);
+        }
+
         static WORKER_TASK_NEXT_ID: AtomicUsize = AtomicUsize::new(0);
 
         let task_id = WORKER_TASK_NEXT_ID.fetch_add(1, Ordering::SeqCst);
@@ -184,7 +186,7 @@ pub struct TaskListItem {
     /// Worker ID (arbitrary ASCII string)
     pub worker_id: Option<String>,
     /// The authenticated entity who started the task
-    pub user: Authid,
+    pub user: String,
     /// The task end time (Epoch)
     #[serde(skip_serializing_if="Option::is_none")]
     pub endtime: Option<i64>,
@@ -200,4 +202,3 @@ pub const NODE_TASKS_LIST_TASKS_RETURN_TYPE: ReturnType = ReturnType {
         &TaskListItem::API_SCHEMA,
     ).schema(),
 };
-
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 0b14dfbf..fbb93f35 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -722,7 +722,7 @@ pub fn verify(
     let upid_str = WorkerTask::new_thread(
         worker_type,
         Some(worker_id),
-        auth_id.clone(),
+        auth_id.to_string(),
         to_stdout,
         move |worker| {
             let verify_worker = crate::backup::VerifyWorker::new(worker.clone(), datastore);
@@ -862,7 +862,7 @@ pub fn prune(
 
 
     // We use a WorkerTask just to have a task log, but run synchrounously
-    let worker = WorkerTask::new("prune", Some(worker_id), auth_id, true)?;
+    let worker = WorkerTask::new("prune", Some(worker_id), auth_id.to_string(), true)?;
 
     if keep_all {
         worker.log("No prune selection - keeping all files.");
@@ -957,7 +957,7 @@ pub fn prune_datastore(
     let upid_str = WorkerTask::new_thread(
         "prune",
         Some(store.clone()),
-        auth_id.clone(),
+        auth_id.to_string(),
         to_stdout,
         move |worker| crate::server::prune_datastore(
             worker.clone(),
diff --git a/src/api2/backup/environment.rs b/src/api2/backup/environment.rs
index 129ebd2b..306f91ee 100644
--- a/src/api2/backup/environment.rs
+++ b/src/api2/backup/environment.rs
@@ -525,7 +525,7 @@ impl BackupEnvironment {
         WorkerTask::new_thread(
             "verify",
             Some(worker_id),
-            self.auth_id.clone(),
+            self.auth_id.to_string(),
             false,
             move |worker| {
                 worker.log("Automatically verifying newly added snapshot");
diff --git a/src/api2/backup/mod.rs b/src/api2/backup/mod.rs
index 8f51f314..c14f19a4 100644
--- a/src/api2/backup/mod.rs
+++ b/src/api2/backup/mod.rs
@@ -166,7 +166,7 @@ async move {
     if !is_new { bail!("backup directory already exists."); }
 
 
-    WorkerTask::spawn(worker_type, Some(worker_id), auth_id.clone(), true, move |worker| {
+    WorkerTask::spawn(worker_type, Some(worker_id), auth_id.to_string(), true, move |worker| {
         let mut env = BackupEnvironment::new(
             env_type, auth_id, worker.clone(), datastore, backup_dir);
 
diff --git a/src/api2/config/acme.rs b/src/api2/config/acme.rs
index 593b79a3..564cafae 100644
--- a/src/api2/config/acme.rs
+++ b/src/api2/config/acme.rs
@@ -215,7 +215,7 @@ fn register_account(
     WorkerTask::spawn(
         "acme-register",
         Some(name.to_string()),
-        auth_id,
+        auth_id.to_string(),
         true,
         move |worker| async move {
             let mut client = AcmeClient::new(directory);
@@ -275,7 +275,7 @@ pub fn update_account(
     WorkerTask::spawn(
         "acme-update",
         Some(name.to_string()),
-        auth_id,
+        auth_id.to_string(),
         true,
         move |_worker| async move {
             let data = match contact {
@@ -320,7 +320,7 @@ pub fn deactivate_account(
     WorkerTask::spawn(
         "acme-deactivate",
         Some(name.to_string()),
-        auth_id,
+        auth_id.to_string(),
         true,
         move |worker| async move {
             match AcmeClient::load(&name)
diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index c6036fc3..0e6529f8 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -119,9 +119,9 @@ pub fn create_datastore(
     WorkerTask::new_thread(
         "create-datastore",
         Some(config.name.to_string()),
-        auth_id,
+        auth_id.to_string(),
         to_stdout,
-        move |worker| do_create_datastore(lock, section_config, config, Some(&worker)),
+       move |worker| do_create_datastore(lock, section_config, config, Some(&worker)),
     )
 }
 
diff --git a/src/api2/node/apt.rs b/src/api2/node/apt.rs
index 8f4bc691..f02920c0 100644
--- a/src/api2/node/apt.rs
+++ b/src/api2/node/apt.rs
@@ -14,7 +14,7 @@ use proxmox_apt::repositories::{
 use proxmox_http::ProxyConfig;
 
 use pbs_api_types::{
-    Authid, APTUpdateInfo, NODE_SCHEMA, PROXMOX_CONFIG_DIGEST_SCHEMA, UPID_SCHEMA,
+    APTUpdateInfo, NODE_SCHEMA, PROXMOX_CONFIG_DIGEST_SCHEMA, UPID_SCHEMA,
     PRIV_SYS_AUDIT, PRIV_SYS_MODIFY,
 };
 
@@ -154,7 +154,7 @@ pub fn apt_update_database(
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<String, Error> {
 
-    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    let auth_id = rpcenv.get_auth_id().unwrap();
     let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
 
     let upid_str = WorkerTask::new_thread("aptupdate", None, auth_id, to_stdout, move |worker| {
diff --git a/src/api2/node/certificates.rs b/src/api2/node/certificates.rs
index 82fa028d..7b31861e 100644
--- a/src/api2/node/certificates.rs
+++ b/src/api2/node/certificates.rs
@@ -11,7 +11,7 @@ use proxmox::api::router::SubdirMap;
 use proxmox::api::{api, Permission, Router, RpcEnvironment};
 use proxmox::list_subdirs_api_method;
 
-use pbs_api_types::{Authid, NODE_SCHEMA, PRIV_SYS_MODIFY};
+use pbs_api_types::{NODE_SCHEMA, PRIV_SYS_MODIFY};
 use pbs_buildcfg::configdir;
 use pbs_tools::cert;
 
@@ -530,7 +530,7 @@ fn spawn_certificate_worker(
 
     let (node_config, _digest) = crate::config::node::config()?;
 
-    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    let auth_id = rpcenv.get_auth_id().unwrap();
 
     WorkerTask::spawn(name, None, auth_id, true, move |worker| async move {
         if let Some(cert) = order_certificate(worker, &node_config).await? {
@@ -559,7 +559,7 @@ pub fn revoke_acme_cert(rpcenv: &mut dyn RpcEnvironment) -> Result<String, Error
 
     let cert_pem = get_certificate_pem()?;
 
-    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    let auth_id = rpcenv.get_auth_id().unwrap();
 
     WorkerTask::spawn(
         "acme-revoke-cert",
diff --git a/src/api2/node/disks/directory.rs b/src/api2/node/disks/directory.rs
index 38809dcf..2f4a738d 100644
--- a/src/api2/node/disks/directory.rs
+++ b/src/api2/node/disks/directory.rs
@@ -7,7 +7,7 @@ use proxmox::api::section_config::SectionConfigData;
 use proxmox::api::router::Router;
 
 use pbs_api_types::{
-    Authid, DataStoreConfig, NODE_SCHEMA, BLOCKDEVICE_NAME_SCHEMA,
+    DataStoreConfig, NODE_SCHEMA, BLOCKDEVICE_NAME_SCHEMA,
     DATASTORE_SCHEMA, UPID_SCHEMA, PRIV_SYS_AUDIT, PRIV_SYS_MODIFY,
 };
 
@@ -146,7 +146,7 @@ pub fn create_datastore_disk(
 
     let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
 
-    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    let auth_id = rpcenv.get_auth_id().unwrap();
 
     let info = get_disk_usage_info(&disk, true)?;
 
diff --git a/src/api2/node/disks/mod.rs b/src/api2/node/disks/mod.rs
index 67f8f63a..b4001a54 100644
--- a/src/api2/node/disks/mod.rs
+++ b/src/api2/node/disks/mod.rs
@@ -7,7 +7,7 @@ use proxmox::{sortable, identity};
 use proxmox::{list_subdirs_api_method};
 
 use pbs_api_types::{
-    Authid, UPID_SCHEMA, NODE_SCHEMA, BLOCKDEVICE_NAME_SCHEMA,
+    UPID_SCHEMA, NODE_SCHEMA, BLOCKDEVICE_NAME_SCHEMA,
     PRIV_SYS_AUDIT, PRIV_SYS_MODIFY,
 };
 
@@ -144,7 +144,7 @@ pub fn initialize_disk(
 
     let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
 
-    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    let auth_id = rpcenv.get_auth_id().unwrap();
 
     let info = get_disk_usage_info(&disk, true)?;
 
diff --git a/src/api2/node/disks/zfs.rs b/src/api2/node/disks/zfs.rs
index 14c2cfec..9fe0dac4 100644
--- a/src/api2/node/disks/zfs.rs
+++ b/src/api2/node/disks/zfs.rs
@@ -8,7 +8,7 @@ use proxmox::api::{
 use proxmox::api::router::Router;
 
 use pbs_api_types::{
-    Authid, ZpoolListItem, ZfsRaidLevel, ZfsCompressionType, DataStoreConfig,
+    ZpoolListItem, ZfsRaidLevel, ZfsCompressionType, DataStoreConfig,
     NODE_SCHEMA, ZPOOL_NAME_SCHEMA, DATASTORE_SCHEMA, DISK_ARRAY_SCHEMA,
     DISK_LIST_SCHEMA, ZFS_ASHIFT_SCHEMA, UPID_SCHEMA,
     PRIV_SYS_AUDIT, PRIV_SYS_MODIFY,
@@ -168,7 +168,7 @@ pub fn create_zpool(
 
     let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
 
-    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    let auth_id = rpcenv.get_auth_id().unwrap();
 
     let add_datastore = add_datastore.unwrap_or(false);
 
diff --git a/src/api2/node/mod.rs b/src/api2/node/mod.rs
index 9b31d595..8e357311 100644
--- a/src/api2/node/mod.rs
+++ b/src/api2/node/mod.rs
@@ -146,7 +146,7 @@ async fn termproxy(cmd: Option<String>, rpcenv: &mut dyn RpcEnvironment) -> Resu
     let upid = WorkerTask::spawn(
         "termproxy",
         None,
-        auth_id,
+        auth_id.to_string(),
         false,
         move |worker| async move {
             // move inside the worker so that it survives and does not close the port
diff --git a/src/api2/node/network.rs b/src/api2/node/network.rs
index 351fd11c..0fde9f2a 100644
--- a/src/api2/node/network.rs
+++ b/src/api2/node/network.rs
@@ -703,7 +703,7 @@ pub async fn reload_network_config(
 
     let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
 
-    let upid_str = WorkerTask::spawn("srvreload", Some(String::from("networking")), auth_id, true, |_worker| async {
+    let upid_str = WorkerTask::spawn("srvreload", Some(String::from("networking")), auth_id.to_string(), true, |_worker| async {
 
         let _ = std::fs::rename(network::NETWORK_INTERFACES_NEW_FILENAME, network::NETWORK_INTERFACES_FILENAME);
 
diff --git a/src/api2/node/services.rs b/src/api2/node/services.rs
index 25edd1b6..6c757f43 100644
--- a/src/api2/node/services.rs
+++ b/src/api2/node/services.rs
@@ -195,7 +195,7 @@ fn run_service_command(service: &str, cmd: &str, auth_id: Authid) -> Result<Valu
     let upid = WorkerTask::new_thread(
         &workerid,
         Some(service.clone()),
-        auth_id,
+        auth_id.to_string(),
         false,
         move |_worker| {
 
diff --git a/src/api2/node/tasks.rs b/src/api2/node/tasks.rs
index 7066f889..169a3d4d 100644
--- a/src/api2/node/tasks.rs
+++ b/src/api2/node/tasks.rs
@@ -99,8 +99,8 @@ fn check_job_store(upid: &UPID, store: &str) -> bool {
 }
 
 fn check_task_access(auth_id: &Authid, upid: &UPID) -> Result<(), Error> {
-    let task_auth_id = &upid.auth_id;
-    if auth_id == task_auth_id
+    let task_auth_id: Authid = upid.auth_id.parse()?;
+    if auth_id == &task_auth_id
         || (task_auth_id.is_token() && &Authid::from(task_auth_id.user().clone()) == auth_id) {
         // task owner can always read
         Ok(())
@@ -200,6 +200,8 @@ async fn get_task_status(
     let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
     check_task_access(&auth_id, &upid)?;
 
+    let task_auth_id: Authid = upid.auth_id.parse()?;
+
     let mut result = json!({
         "upid": param["upid"],
         "node": upid.node,
@@ -208,11 +210,11 @@ async fn get_task_status(
         "starttime": upid.starttime,
         "type": upid.worker_type,
         "id": upid.worker_id,
-        "user": upid.auth_id.user(),
+        "user": task_auth_id.user(),
     });
 
-    if upid.auth_id.is_token() {
-        result["tokenid"] = Value::from(upid.auth_id.tokenname().unwrap().as_str());
+    if task_auth_id.is_token() {
+        result["tokenid"] = Value::from(task_auth_id.tokenname().unwrap().as_str());
     }
 
     if crate::server::worker_is_active(&upid).await? {
@@ -344,10 +346,11 @@ fn stop_task(
 
     let upid = extract_upid(&param)?;
 
-    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    let auth_id = rpcenv.get_auth_id().unwrap();
 
     if auth_id != upid.auth_id {
         let user_info = CachedUserInfo::new()?;
+        let auth_id: Authid = auth_id.parse()?;
         user_info.check_privs(&auth_id, &["system", "tasks"], PRIV_SYS_MODIFY, false)?;
     }
 
diff --git a/src/api2/pull.rs b/src/api2/pull.rs
index 1eb86ea3..e631920f 100644
--- a/src/api2/pull.rs
+++ b/src/api2/pull.rs
@@ -81,7 +81,7 @@ pub fn do_sync_job(
     let upid_str = WorkerTask::spawn(
         &worker_type,
         Some(job_id.clone()),
-        auth_id.clone(),
+        auth_id.to_string(),
         to_stdout,
         move |worker| async move {
 
@@ -183,7 +183,7 @@ async fn pull (
     let (client, src_repo, tgt_store) = get_pull_parameters(&store, &remote, &remote_store).await?;
 
     // fixme: set to_stdout to false?
-    let upid_str = WorkerTask::spawn("sync", Some(store.clone()), auth_id.clone(), true, move |worker| async move {
+    let upid_str = WorkerTask::spawn("sync", Some(store.clone()), auth_id.to_string(), true, move |worker| async move {
 
         worker.log(format!("sync datastore '{}' start", store));
 
diff --git a/src/api2/reader/mod.rs b/src/api2/reader/mod.rs
index 821e83c4..fada952c 100644
--- a/src/api2/reader/mod.rs
+++ b/src/api2/reader/mod.rs
@@ -143,7 +143,7 @@ fn upgrade_to_backup_reader_protocol(
 
         let worker_id = format!("{}:{}/{}/{:08X}", store, backup_type, backup_id, backup_dir.backup_time());
 
-        WorkerTask::spawn("reader", Some(worker_id), auth_id.clone(), true, move |worker| async move {
+        WorkerTask::spawn("reader", Some(worker_id), auth_id.to_string(), true, move |worker| async move {
             let _guard = _guard;
 
             let mut env = ReaderEnvironment::new(
diff --git a/src/api2/tape/backup.rs b/src/api2/tape/backup.rs
index 6b533820..fadbfa3d 100644
--- a/src/api2/tape/backup.rs
+++ b/src/api2/tape/backup.rs
@@ -195,7 +195,7 @@ pub fn do_tape_backup_job(
     let upid_str = WorkerTask::new_thread(
         &worker_type,
         Some(job_id.clone()),
-        auth_id.clone(),
+        auth_id.to_string(),
         to_stdout,
         move |worker| {
             job.start(&worker.upid().to_string())?;
@@ -376,7 +376,7 @@ pub fn backup(
     let upid_str = WorkerTask::new_thread(
         "tape-backup",
         Some(job_id),
-        auth_id,
+        auth_id.to_string(),
         to_stdout,
         move |worker| {
             let _drive_lock = drive_lock; // keep lock guard
diff --git a/src/api2/tape/drive.rs b/src/api2/tape/drive.rs
index 58f49f43..10aa6842 100644
--- a/src/api2/tape/drive.rs
+++ b/src/api2/tape/drive.rs
@@ -87,7 +87,7 @@ where
     let (config, _digest) = pbs_config::drive::config()?;
     let lock_guard = lock_tape_device(&config, &drive)?;
 
-    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    let auth_id = rpcenv.get_auth_id().unwrap();
     let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
 
     WorkerTask::new_thread(worker_type, job_id, auth_id, to_stdout, move |worker| {
diff --git a/src/api2/tape/restore.rs b/src/api2/tape/restore.rs
index 4ab60e8f..7739d1a4 100644
--- a/src/api2/tape/restore.rs
+++ b/src/api2/tape/restore.rs
@@ -275,7 +275,7 @@ pub fn restore(
     let upid_str = WorkerTask::new_thread(
         "tape-restore",
         Some(taskid),
-        auth_id.clone(),
+        auth_id.to_string(),
         to_stdout,
         move |worker| {
             let _drive_lock = drive_lock; // keep lock guard
diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index 6734525b..518054bf 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -745,7 +745,7 @@ async fn schedule_task_log_rotate() {
     if let Err(err) = WorkerTask::new_thread(
         worker_type,
         None,
-        Authid::root_auth_id().clone(),
+        Authid::root_auth_id().to_string(),
         false,
         move |worker| {
             job.start(&worker.upid().to_string())?;
diff --git a/src/server/gc_job.rs b/src/server/gc_job.rs
index 665b9631..317f4a36 100644
--- a/src/server/gc_job.rs
+++ b/src/server/gc_job.rs
@@ -26,7 +26,7 @@ pub fn do_garbage_collection_job(
     let upid_str = WorkerTask::new_thread(
         &worker_type,
         Some(store.clone()),
-        auth_id.clone(),
+        auth_id.to_string(),
         to_stdout,
         move |worker| {
             job.start(&worker.upid().to_string())?;
diff --git a/src/server/prune_job.rs b/src/server/prune_job.rs
index f298a74c..8d971a1c 100644
--- a/src/server/prune_job.rs
+++ b/src/server/prune_job.rs
@@ -105,7 +105,7 @@ pub fn do_prune_job(
     let upid_str = WorkerTask::new_thread(
         &worker_type,
         Some(job.jobname().to_string()),
-        auth_id.clone(),
+        auth_id.to_string(),
         false,
         move |worker| {
             job.start(&worker.upid().to_string())?;
diff --git a/src/server/verify_job.rs b/src/server/verify_job.rs
index 7f6d73ff..6005b706 100644
--- a/src/server/verify_job.rs
+++ b/src/server/verify_job.rs
@@ -36,7 +36,7 @@ pub fn do_verification_job(
     let upid_str = WorkerTask::new_thread(
         &worker_type,
         Some(job_id.clone()),
-        auth_id.clone(),
+        auth_id.to_string(),
         to_stdout,
         move |worker| {
             job.start(&worker.upid().to_string())?;
diff --git a/src/server/worker_task.rs b/src/server/worker_task.rs
index a74b18e1..92ab50d7 100644
--- a/src/server/worker_task.rs
+++ b/src/server/worker_task.rs
@@ -18,7 +18,7 @@ use proxmox::tools::fs::{create_path, replace_file, CreateOptions};
 
 use pbs_buildcfg;
 use pbs_tools::logrotate::{LogRotate, LogRotateFiles};
-use pbs_api_types::{Authid, UPID};
+use pbs_api_types::UPID;
 use pbs_config::{open_backup_lockfile, BackupLockGuard};
 use proxmox_rest_server::{CommandoSocket, FileLogger, FileLogOptions};
 
@@ -589,7 +589,7 @@ struct WorkerTaskData {
 
 impl WorkerTask {
 
-    pub fn new(worker_type: &str, worker_id: Option<String>, auth_id: Authid, to_stdout: bool) -> Result<Arc<Self>, Error> {
+    pub fn new(worker_type: &str, worker_id: Option<String>, auth_id: String, to_stdout: bool) -> Result<Arc<Self>, Error> {
         let upid = UPID::new(worker_type, worker_id, auth_id)?;
         let task_id = upid.task_id;
 
@@ -640,7 +640,7 @@ impl WorkerTask {
     pub fn spawn<F, T>(
         worker_type: &str,
         worker_id: Option<String>,
-        auth_id: Authid,
+        auth_id: String,
         to_stdout: bool,
         f: F,
     ) -> Result<String, Error>
@@ -662,7 +662,7 @@ impl WorkerTask {
     pub fn new_thread<F>(
         worker_type: &str,
         worker_id: Option<String>,
-        auth_id: Authid,
+        auth_id: String,
         to_stdout: bool,
         f: F,
     ) -> Result<String, Error>
-- 
2.30.2





^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-09-24  9:04 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-24  9:03 [pbs-devel] [PATCH proxmox-backup 1/6] src/server/worker_task.rs: Avoid using pbs-api-type::Authid Dietmar Maurer
  -- strict thread matches above, loose matches on Subject: below --
2021-09-23 10:13 Dietmar Maurer
2021-09-23 13:20 ` Fabian Grünbichler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal