[pbs-devel] [PATCH proxmox-backup 1/2] openid: move helper from config to api2

[pbs-devel] [PATCH proxmox-backup 1/2] openid: move helper from config to api2

[Fabian Grünbichler]

it's not really needed in the config module, and this makes it easier to
disable the proxmox-openid dependency linkage as a stop-gap measure.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/api2/access/openid.rs | 17 ++++++++++++++---
 src/config/domains.rs     | 14 --------------
 2 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/src/api2/access/openid.rs b/src/api2/access/openid.rs
index ea6133b4..91a26279 100644
--- a/src/api2/access/openid.rs
+++ b/src/api2/access/openid.rs
@@ -11,7 +11,8 @@ use proxmox::{list_subdirs_api_method};
 use proxmox::{identity, sortable};
 use proxmox::tools::fs::open_file_locked;
 
-use proxmox_openid::OpenIdAuthenticator;
+use proxmox_openid::{OpenIdAuthenticator,  OpenIdConfig};
+
 
 use crate::server::ticket::ApiTicket;
 use crate::tools::ticket::Ticket;
@@ -22,6 +23,16 @@ use crate::config::cached_user_info::CachedUserInfo;
 use crate::api2::types::*;
 use crate::auth_helpers::*;
 
+fn openid_authenticator(realm_config: &OpenIdRealmConfig, redirect_url: &str) -> Result<OpenIdAuthenticator, Error> {
+    let config = OpenIdConfig {
+        issuer_url: realm_config.issuer_url.clone(),
+        client_id: realm_config.client_id.clone(),
+        client_key: realm_config.client_key.clone(),
+    };
+    OpenIdAuthenticator::discover(&config, redirect_url)
+}
+
+
 #[api(
     input: {
         properties: {
@@ -77,7 +88,7 @@ pub fn openid_login(
     let (domains, _digest) = crate::config::domains::config()?;
     let config: OpenIdRealmConfig = domains.lookup("openid", &realm)?;
 
-    let open_id = config.authenticator(&redirect_url)?;
+    let open_id = openid_authenticator(&config, &redirect_url)?;
 
     let info = open_id.verify_authorization_code(&code, &private_auth_state)?;
 
@@ -171,7 +182,7 @@ fn openid_auth_url(
     let (domains, _digest) = crate::config::domains::config()?;
     let config: OpenIdRealmConfig = domains.lookup("openid", &realm)?;
 
-    let open_id = config.authenticator(&redirect_url)?;
+    let open_id = openid_authenticator(&config, &redirect_url)?;
 
     let url = open_id.authorize_url(PROXMOX_BACKUP_RUN_DIR_M!(), &realm)?
         .to_string();
diff --git a/src/config/domains.rs b/src/config/domains.rs
index d08efc24..775c02f3 100644
--- a/src/config/domains.rs
+++ b/src/config/domains.rs
@@ -3,8 +3,6 @@ use lazy_static::lazy_static;
 use std::collections::HashMap;
 use serde::{Serialize, Deserialize};
 
-use proxmox_openid::{OpenIdAuthenticator,  OpenIdConfig};
-
 use proxmox::api::{
     api,
     schema::*,
@@ -95,18 +93,6 @@ pub struct OpenIdRealmConfig {
     pub username_claim: Option<OpenIdUserAttribute>,
 }
 
-impl OpenIdRealmConfig {
-
-    pub fn authenticator(&self, redirect_url: &str) -> Result<OpenIdAuthenticator, Error> {
-        let config = OpenIdConfig {
-            issuer_url: self.issuer_url.clone(),
-            client_id: self.client_id.clone(),
-            client_key: self.client_key.clone(),
-        };
-        OpenIdAuthenticator::discover(&config, redirect_url)
-    }
-}
-
 fn init() -> SectionConfig {
     let obj_schema = match OpenIdRealmConfig::API_SCHEMA {
         Schema::Object(ref obj_schema) => obj_schema,
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup 2/2] openid: conditionally disable api endpoint

[Fabian Grünbichler]

since it pulls in lots of additional linked libraries for all binaries
compiled as part of proxmox-backup. it can easily be re-enabled with
`--cfg openid` added to the RUSTFLAGS env variable.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    to be reverted as part of splitting client/server/lib crates next week, but
    this is currently breaking file-restore builds for PVE 7.
    
    alternatively, a feature could be employed for the same effect, but since this
    is just a temporary measure a 'cfg' flag causes less churn/noise.
    
    the difference is quite big (ldd $bin | wc -l)
    
    usr/sbin/proxmox-backup-manager: 53 vs 28
    usr/lib/x86_64-linux-gnu/proxmox-backup/sg-tape-cmd: 41 vs 12
    usr/lib/x86_64-linux-gnu/proxmox-backup/proxmox-daily-update: 48 vs 23
    usr/lib/x86_64-linux-gnu/proxmox-backup/proxmox-backup-proxy: 54 vs 29
    usr/lib/x86_64-linux-gnu/proxmox-backup/proxmox-backup-banner: 6 vs 6 (doesn't use proxmox_backup at all!)
    usr/lib/x86_64-linux-gnu/proxmox-backup/proxmox-backup-api: 54 vs 29
    usr/lib/x86_64-linux-gnu/proxmox-backup/file-restore/proxmox-restore-daemon: 39 vs 10
    usr/bin/proxmox-tape: 43 vs 14
    usr/bin/pmtx: 40 vs 11
    usr/bin/pmt: 40 vs 11
    usr/bin/proxmox-file-restore: 43 vs 14
    usr/bin/pxar: 40 vs 11
    usr/bin/proxmox-backup-client: 43 vs 14

 src/api2/access.rs | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/api2/access.rs b/src/api2/access.rs
index e5430f62..1cd772d6 100644
--- a/src/api2/access.rs
+++ b/src/api2/access.rs
@@ -26,6 +26,8 @@ pub mod domain;
 pub mod role;
 pub mod tfa;
 pub mod user;
+
+#[cfg(openid)]
 pub mod openid;
 
 #[allow(clippy::large_enum_variant)]
@@ -415,6 +417,12 @@ pub fn list_permissions(
     Ok(map)
 }
 
+#[cfg(openid)]
+const OPENID_ROUTER: &Router = &openid::ROUTER;
+
+#[cfg(not(openid))]
+const OPENID_ROUTER: &Router = &Router::new();
+
 #[sortable]
 const SUBDIRS: SubdirMap = &sorted!([
     ("acl", &acl::ROUTER),
@@ -424,7 +432,7 @@ const SUBDIRS: SubdirMap = &sorted!([
         &Router::new().get(&API_METHOD_LIST_PERMISSIONS)
     ),
     ("ticket", &Router::new().post(&API_METHOD_CREATE_TICKET)),
-    ("openid", &openid::ROUTER),
+    ("openid", &OPENID_ROUTER),
     ("domains", &domain::ROUTER),
     ("roles", &role::ROUTER),
     ("users", &user::ROUTER),
-- 
2.30.2

[pbs-devel] applied-series: [PATCH proxmox-backup 1/2] openid: move helper from config to api2

[Thomas Lamprecht]

On 01.07.21 14:58, Fabian Grünbichler wrote:
> it's not really needed in the config module, and this makes it easier to
> disable the proxmox-openid dependency linkage as a stop-gap measure.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
>  src/api2/access/openid.rs | 17 ++++++++++++++---
>  src/config/domains.rs     | 14 --------------
>  2 files changed, 14 insertions(+), 17 deletions(-)
> 
>

applied both patches, thanks!