Re: [pmg-devel] [PATCH proxmox-perl-rs v5 2/10] move openid code from pve-rs to common

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

> Markus Frank <m.frank@proxmox.com> hat am 18.02.2025 17:18 CET geschrieben:
> 
>  
> Change pve-rs functions to be wrapper functions for common.
> 
> Signed-off-by: Markus Frank <m.frank@proxmox.com>

modulo the name change, which I am not sure is a real benefit unless we want to do it across the board (also for PVE/PMG/the low level rust crates), this looks okay to me.

note that there are two other OIDC related patch series open that might or might not require changes depending on the order things get applied in.

> ---
>  common/pkg/Makefile      |  1 +
>  common/src/mod.rs        |  1 +
>  common/src/oidc/mod.rs   | 63 ++++++++++++++++++++++++++++++++++++++++
>  pmg-rs/Cargo.toml        |  1 +
>  pmg-rs/debian/control    |  1 +
>  pve-rs/src/openid/mod.rs | 32 +++++---------------
>  6 files changed, 75 insertions(+), 24 deletions(-)
>  create mode 100644 common/src/oidc/mod.rs
> 
> diff --git a/common/pkg/Makefile b/common/pkg/Makefile
> index cbefdf7..5a537f9 100644
> --- a/common/pkg/Makefile
> +++ b/common/pkg/Makefile
> @@ -24,6 +24,7 @@ PERLMOD_PACKAGES := \
>  	  Proxmox::RS::APT::Repositories \
>  	  Proxmox::RS::CalendarEvent \
>  	  Proxmox::RS::Notify \
> +	  Proxmox::RS::OIDC \
>  	  Proxmox::RS::SharedCache \
>  	  Proxmox::RS::Subscription
>  
> diff --git a/common/src/mod.rs b/common/src/mod.rs
> index badfc98..06a7c20 100644
> --- a/common/src/mod.rs
> +++ b/common/src/mod.rs
> @@ -2,5 +2,6 @@ pub mod apt;
>  mod calendar_event;
>  pub mod logger;
>  pub mod notify;
> +pub mod oidc;
>  pub mod shared_cache;
>  mod subscription;
> diff --git a/common/src/oidc/mod.rs b/common/src/oidc/mod.rs
> new file mode 100644
> index 0000000..b1cddaa
> --- /dev/null
> +++ b/common/src/oidc/mod.rs
> @@ -0,0 +1,63 @@
> +#[perlmod::package(name = "Proxmox::RS::OIDC")]
> +pub mod export {
> +    use std::sync::Mutex;
> +
> +    use anyhow::Error;
> +
> +    use perlmod::{to_value, Value};
> +
> +    use proxmox_openid::{OpenIdAuthenticator, OpenIdConfig, PrivateAuthState};
> +
> +    perlmod::declare_magic!(Box<OIDC> : &OIDC as "Proxmox::RS::OIDC");
> +
> +    /// An OpenIdAuthenticator client instance.
> +    pub struct OIDC {
> +        inner: Mutex<OpenIdAuthenticator>,
> +    }
> +
> +    /// Create a new OIDC client instance
> +    #[export(raw_return)]
> +    pub fn discover(
> +        #[raw] class: Value,
> +        config: OpenIdConfig,
> +        redirect_url: &str,
> +    ) -> Result<Value, Error> {
> +        let oidc = OpenIdAuthenticator::discover(&config, redirect_url)?;
> +        Ok(perlmod::instantiate_magic!(
> +            &class,
> +            MAGIC => Box::new(OIDC {
> +                inner: Mutex::new(oidc),
> +            })
> +        ))
> +    }
> +
> +    #[export]
> +    pub fn authorize_url(
> +        #[try_from_ref] this: &OIDC,
> +        state_dir: &str,
> +        realm: &str,
> +    ) -> Result<String, Error> {
> +        let oidc = this.inner.lock().unwrap();
> +        oidc.authorize_url(state_dir, realm)
> +    }
> +
> +    #[export]
> +    pub fn verify_public_auth_state(
> +        state_dir: &str,
> +        state: &str,
> +    ) -> Result<(String, PrivateAuthState), Error> {
> +        OpenIdAuthenticator::verify_public_auth_state(state_dir, state)
> +    }
> +
> +    #[export(raw_return)]
> +    pub fn verify_authorization_code(
> +        #[try_from_ref] this: &OIDC,
> +        code: &str,
> +        private_auth_state: PrivateAuthState,
> +    ) -> Result<Value, Error> {
> +        let oidc = this.inner.lock().unwrap();
> +        let claims = oidc.verify_authorization_code_simple(code, &private_auth_state)?;
> +
> +        Ok(to_value(&claims)?)
> +    }
> +}
> diff --git a/pmg-rs/Cargo.toml b/pmg-rs/Cargo.toml
> index 1252671..ce715bf 100644
> --- a/pmg-rs/Cargo.toml
> +++ b/pmg-rs/Cargo.toml
> @@ -42,3 +42,4 @@ proxmox-subscription = "0.5"
>  proxmox-sys = "0.6"
>  proxmox-tfa = { version = "5", features = ["api"] }
>  proxmox-time = "2"
> +proxmox-openid =  "0.10.0"
> diff --git a/pmg-rs/debian/control b/pmg-rs/debian/control
> index 295dcb3..afd8cbb 100644
> --- a/pmg-rs/debian/control
> +++ b/pmg-rs/debian/control
> @@ -27,6 +27,7 @@ Build-Depends: cargo:native <!nocheck>,
>                 librust-proxmox-http-error-0.1+default-dev,
>                 librust-proxmox-log-0.2+default-dev,
>                 librust-proxmox-notify-0.5+default-dev,
> +               librust-proxmox-openid-0.10+default-dev,
>                 librust-proxmox-shared-cache-0.1+default-dev,
>                 librust-proxmox-subscription-0.5+default-dev,
>                 librust-proxmox-sys-0.6+default-dev,
> diff --git a/pve-rs/src/openid/mod.rs b/pve-rs/src/openid/mod.rs
> index 1fa7572..2adb8bb 100644
> --- a/pve-rs/src/openid/mod.rs
> +++ b/pve-rs/src/openid/mod.rs
> @@ -1,19 +1,13 @@
>  #[perlmod::package(name = "PVE::RS::OpenId", lib = "pve_rs")]
>  mod export {
> -    use std::sync::Mutex;
> -
>      use anyhow::Error;
>  
> -    use perlmod::{to_value, Value};
> -
> -    use proxmox_openid::{OpenIdAuthenticator, OpenIdConfig, PrivateAuthState};
> +    use perlmod::Value;
>  
> -    perlmod::declare_magic!(Box<OpenId> : &OpenId as "PVE::RS::OpenId");
> +    use proxmox_openid::{OpenIdConfig, PrivateAuthState};
>  
> -    /// An OpenIdAuthenticator client instance.
> -    pub struct OpenId {
> -        inner: Mutex<OpenIdAuthenticator>,
> -    }
> +    use crate::common::oidc::export as common;
> +    use crate::common::oidc::export::OIDC as OpenId;
>  
>      /// Create a new OpenId client instance
>      #[export(raw_return)]
> @@ -22,13 +16,7 @@ mod export {
>          config: OpenIdConfig,
>          redirect_url: &str,
>      ) -> Result<Value, Error> {
> -        let open_id = OpenIdAuthenticator::discover(&config, redirect_url)?;
> -        Ok(perlmod::instantiate_magic!(
> -            &class,
> -            MAGIC => Box::new(OpenId {
> -                inner: Mutex::new(open_id),
> -            })
> -        ))
> +        common::discover(class, config, redirect_url)
>      }
>  
>      #[export]
> @@ -37,8 +25,7 @@ mod export {
>          state_dir: &str,
>          realm: &str,
>      ) -> Result<String, Error> {
> -        let open_id = this.inner.lock().unwrap();
> -        open_id.authorize_url(state_dir, realm)
> +        common::authorize_url(this, state_dir, realm)
>      }
>  
>      #[export]
> @@ -46,7 +33,7 @@ mod export {
>          state_dir: &str,
>          state: &str,
>      ) -> Result<(String, PrivateAuthState), Error> {
> -        OpenIdAuthenticator::verify_public_auth_state(state_dir, state)
> +        common::verify_public_auth_state(state_dir, state)
>      }
>  
>      #[export(raw_return)]
> @@ -55,9 +42,6 @@ mod export {
>          code: &str,
>          private_auth_state: PrivateAuthState,
>      ) -> Result<Value, Error> {
> -        let open_id = this.inner.lock().unwrap();
> -        let claims = open_id.verify_authorization_code_simple(code, &private_auth_state)?;
> -
> -        Ok(to_value(&claims)?)
> +        common::verify_authorization_code(this, code, private_auth_state)
>      }
>  }
> -- 
> 2.39.5
> 
> 
> 
> _______________________________________________
> pmg-devel mailing list
> pmg-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel


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