public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
To: pve-devel@lists.proxmox.com, pbs-devel@lists.proxmox.com
Subject: [PATCH proxmox v5 08/27] notify: smtp: add state handling logic
Date: Tue,  5 May 2026 10:32:29 +0200	[thread overview]
Message-ID: <20260505083248.36450-9-a.bied-charreton@proxmox.com> (raw)
In-Reply-To: <20260505083248.36450-1-a.bied-charreton@proxmox.com>

Create new state file in add_endpoint, create/update existing one in
update_endpoint and delete it in delete_endpoint.

Add trigger_state_refresh to the Endpoint trait, with no-op default
implementation. Implement it in SmtpEndpoint's Endpoint impl to trigger
an OAuth2 token exchange, in order to rotate an existing token, or
extend its lifetime.

The intended callers are daily update jobs, so on a cluster
`trigger_state_refresh` may be invoked once per node within a
relatively short window. The `last_refreshed` cutoff suppresses
redundant token exchanges.

Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
---
 proxmox-notify/src/api/common.rs     | 19 +++++++
 proxmox-notify/src/api/smtp.rs       | 32 +++++++++++
 proxmox-notify/src/endpoints/smtp.rs | 80 +++++++++++++++++++++++++++-
 proxmox-notify/src/lib.rs            | 28 +++++++++-
 4 files changed, 157 insertions(+), 2 deletions(-)

diff --git a/proxmox-notify/src/api/common.rs b/proxmox-notify/src/api/common.rs
index fa2356e2..220be3de 100644
--- a/proxmox-notify/src/api/common.rs
+++ b/proxmox-notify/src/api/common.rs
@@ -3,6 +3,25 @@ use proxmox_http_error::HttpError;
 use super::http_err;
 use crate::{Bus, Config, Notification};
 
+/// Refresh all notification targets' internal state.
+///
+/// The caller is responsible for any needed permission checks and must hold a lock
+/// on the notifications config. Concurrent updates could otherwise leave the state
+/// files inconsistent with the notifications config (e.g. a stale state file for
+/// an endpoint that was deleted concurrently).
+pub fn trigger_state_refresh(config: &Config) -> Result<(), HttpError> {
+    let bus = Bus::from_config(config).map_err(|err| {
+        http_err!(
+            INTERNAL_SERVER_ERROR,
+            "Could not instantiate notification bus: {err}"
+        )
+    })?;
+
+    bus.trigger_state_refresh();
+
+    Ok(())
+}
+
 /// Send a notification to a given target.
 ///
 /// The caller is responsible for any needed permission checks.
diff --git a/proxmox-notify/src/api/smtp.rs b/proxmox-notify/src/api/smtp.rs
index 0668e89d..24d3d865 100644
--- a/proxmox-notify/src/api/smtp.rs
+++ b/proxmox-notify/src/api/smtp.rs
@@ -116,6 +116,19 @@ pub fn add_endpoint(
 
     let endpoint_config = infer_auth_method(endpoint_config, private_endpoint_config);
 
+    if let Some(token) = oauth2_refresh_token {
+        let oauth_state = State::new(token, proxmox_time::epoch_i64());
+        context()
+            .save_oauth_state(&endpoint_config.name, Some(oauth_state))
+            .map_err(|e| {
+                http_err!(
+                    INTERNAL_SERVER_ERROR,
+                    "could not create state file for '{}': {e}",
+                    &endpoint_config.name
+                )
+            })?;
+    }
+
     config
         .config
         .set_data(&endpoint_config.name, SMTP_TYPENAME, &endpoint_config)
@@ -233,6 +246,18 @@ pub fn update_endpoint(
 
     let endpoint = infer_auth_method(endpoint, get_private_config(config, name)?);
 
+    if let Some(token) = oauth2_refresh_token {
+        let oauth_state = context()
+            .load_oauth_state(name)
+            .map_err(|e| http_err!(INTERNAL_SERVER_ERROR, "{e}"))?
+            .set_oauth2_refresh_token(Some(token))
+            .set_last_refreshed(proxmox_time::epoch_i64());
+
+        context()
+            .save_oauth_state(name, Some(oauth_state))
+            .map_err(|e| http_err!(INTERNAL_SERVER_ERROR, "{e}"))?;
+    }
+
     config
         .config
         .set_data(name, SMTP_TYPENAME, &endpoint)
@@ -305,6 +330,13 @@ pub fn delete_endpoint(config: &mut Config, name: &str) -> Result<(), HttpError>
 
     super::remove_private_config_entry(config, name)?;
 
+    context().save_oauth_state(name, None).map_err(|e| {
+        http_err!(
+            INTERNAL_SERVER_ERROR,
+            "could not delete state for '{name}': {e}"
+        )
+    })?;
+
     config.config.sections.remove(name);
 
     Ok(())
diff --git a/proxmox-notify/src/endpoints/smtp.rs b/proxmox-notify/src/endpoints/smtp.rs
index 74297969..dcc01bb5 100644
--- a/proxmox-notify/src/endpoints/smtp.rs
+++ b/proxmox-notify/src/endpoints/smtp.rs
@@ -1,11 +1,13 @@
 use std::borrow::Cow;
-use std::time::Duration;
+use std::time::{Duration, SystemTime, UNIX_EPOCH};
 
 use lettre::message::header::{HeaderName, HeaderValue};
 use lettre::message::{Mailbox, MultiPart, SinglePart};
 use lettre::transport::smtp::client::{Tls, TlsParameters};
 use lettre::{message::header::ContentType, Message, SmtpTransport, Transport};
+use oauth2::{ClientId, ClientSecret, RefreshToken};
 use serde::{Deserialize, Serialize};
+use tracing::info;
 
 use proxmox_schema::api_types::COMMENT_SCHEMA;
 use proxmox_schema::{api, Updater};
@@ -22,6 +24,7 @@ const SMTP_PORT: u16 = 25;
 const SMTP_SUBMISSION_STARTTLS_PORT: u16 = 587;
 const SMTP_SUBMISSION_TLS_PORT: u16 = 465;
 const SMTP_TIMEOUT: u16 = 5;
+const SMTP_STATE_REFRESH_CUTOFF: Duration = Duration::from_secs(60 * 60 * 12);
 
 pub(crate) mod xoauth2;
 
@@ -203,6 +206,43 @@ pub struct SmtpEndpoint {
 }
 
 impl SmtpEndpoint {
+    fn get_access_token(
+        &self,
+        refresh_token: &str,
+        auth_method: &SmtpAuthMethod,
+    ) -> Result<xoauth2::TokenExchangeResult, Error> {
+        let client_id = ClientId::new(
+            self.config
+                .oauth2_client_id
+                .as_ref()
+                .ok_or_else(|| Error::Generic("oauth2-client-id not set".into()))?
+                .to_string(),
+        );
+        let client_secret = ClientSecret::new(
+            self.private_config
+                .oauth2_client_secret
+                .as_ref()
+                .ok_or_else(|| Error::Generic("oauth2-client-secret not set".into()))?
+                .to_string(),
+        );
+        let refresh_token = RefreshToken::new(refresh_token.into());
+
+        match auth_method {
+            SmtpAuthMethod::GoogleOAuth2 => {
+                xoauth2::get_google_token(client_id, client_secret, refresh_token)
+            }
+            SmtpAuthMethod::MicrosoftOAuth2 => xoauth2::get_microsoft_token(
+                client_id,
+                client_secret,
+                self.config.oauth2_tenant_id.as_ref().ok_or(Error::Generic(
+                    "tenant ID not set, required for Microsoft OAuth2".into(),
+                ))?,
+                refresh_token,
+            ),
+            _ => Err(Error::Generic("OAuth2 not configured".into())),
+        }
+    }
+
     fn build_transport(&self, tls: Tls, port: u16) -> Result<SmtpTransport, Error> {
         let mut transport_builder = SmtpTransport::builder_dangerous(&self.config.server)
             .tls(tls)
@@ -334,6 +374,44 @@ impl Endpoint for SmtpEndpoint {
     fn disabled(&self) -> bool {
         self.config.disable.unwrap_or_default()
     }
+
+    fn trigger_state_refresh(&self) -> Result<(), Error> {
+        let state = context().load_oauth_state(self.name())?;
+
+        let Some(refresh_token) = &state.oauth2_refresh_token else {
+            return Ok(());
+        };
+
+        // The intended callers are daily update jobs, so on a PVE cluster this function
+        // may be called once per node in a relatively short window. This cutoff prevents
+        // unnecessary token exchanges.
+        if SystemTime::now()
+            .duration_since(UNIX_EPOCH + Duration::from_secs(state.last_refreshed as u64))
+            .map_err(|e| Error::Generic(e.to_string()))?
+            < SMTP_STATE_REFRESH_CUTOFF
+        {
+            return Ok(());
+        }
+
+        let Some(auth_method) = self.config.auth_method.as_ref() else {
+            return Ok(());
+        };
+
+        let state = match self
+            .get_access_token(refresh_token, auth_method)?
+            .refresh_token
+        {
+            Some(tok) => state.set_oauth2_refresh_token(Some(tok.into_secret())), // New token was returned, rotate
+            None => state,
+        }
+        .set_last_refreshed(proxmox_time::epoch_i64());
+
+        context().save_oauth_state(self.name(), Some(state))?;
+
+        info!("OAuth2 state refreshed for endpoint `{}`", self.name());
+
+        Ok(())
+    }
 }
 
 /// Construct a lettre `Message` from a raw email message.
diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs
index 619dd7db..ee4ffcd3 100644
--- a/proxmox-notify/src/lib.rs
+++ b/proxmox-notify/src/lib.rs
@@ -9,7 +9,7 @@ use context::context;
 use serde::{Deserialize, Serialize};
 use serde_json::json;
 use serde_json::Value;
-use tracing::{error, info};
+use tracing::{debug, error, info};
 
 use proxmox_schema::api;
 use proxmox_section_config::SectionConfigData;
@@ -169,6 +169,15 @@ pub trait Endpoint {
 
     /// Check if the endpoint is disabled
     fn disabled(&self) -> bool;
+
+    /// Refresh endpoint's state.
+    ///
+    /// Implementations may persist endpoint-internal state (e.g. SMTP XOAUTH2 refresh
+    /// tokens). Callers must hold the notifications config lock so that state files
+    /// cannot drift relative to concurrently edited/deleted configs.
+    fn trigger_state_refresh(&self) -> Result<(), Error> {
+        Ok(())
+    }
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize)]
@@ -605,6 +614,23 @@ impl Bus {
 
         Ok(())
     }
+
+    /// Refresh all endpoints' internal state.
+    ///
+    /// The caller must hold a lock to the notifications config, see
+    /// [`Endpoint::trigger_state_refresh`]'s docs.
+    ///
+    /// This function works on a best effort basis, if an endpoint's state cannot
+    /// be updated for whatever reason, the error is logged and the next one(s)
+    /// are attempted.
+    pub fn trigger_state_refresh(&self) {
+        for (name, endpoint) in &self.endpoints {
+            match endpoint.trigger_state_refresh() {
+                Ok(()) => debug!("triggered state refresh for endpoint '{name}'"),
+                Err(e) => error!("could not trigger state refresh for endpoint '{name}': {e}"),
+            };
+        }
+    }
 }
 
 #[cfg(test)]
-- 
2.47.3




  parent reply	other threads:[~2026-05-05  8:36 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-05  8:32 [PATCH docs/manager/proxmox{,-perl-rs,-widget-toolkit,-backup} v5 00/27] fix #7238: Add XOAUTH2 authentication support for SMTP notification targets Arthur Bied-Charreton
2026-05-05  8:32 ` [PATCH proxmox v5 01/27] add oauth2 and ureq to workspace dependencies Arthur Bied-Charreton
2026-05-05  8:32 ` [PATCH proxmox v5 02/27] notify: smtp: introduce xoauth2 module Arthur Bied-Charreton
2026-05-05  8:32 ` [PATCH proxmox v5 03/27] notify: smtp: introduce state management Arthur Bied-Charreton
2026-05-05  8:32 ` [PATCH proxmox v5 04/27] notify: smtp: factor out transport building logic Arthur Bied-Charreton
2026-05-05  8:32 ` [PATCH proxmox v5 05/27] notify: smtp: update API with OAuth2 parameters Arthur Bied-Charreton
2026-05-05  8:32 ` [PATCH proxmox v5 06/27] notify: smtp: add API to exchange authorization code for refresh token Arthur Bied-Charreton
2026-05-05  8:32 ` [PATCH proxmox v5 07/27] notify: smtp: infer auth method for backwards compatibility Arthur Bied-Charreton
2026-05-05  8:32 ` Arthur Bied-Charreton [this message]
2026-05-05  8:32 ` [PATCH proxmox v5 09/27] notify: smtp: add XOAUTH2 authentication support Arthur Bied-Charreton
2026-05-05  8:32 ` [PATCH proxmox-perl-rs v5 10/27] pve-rs: notify: smtp: add OAuth2 parameters to bindings Arthur Bied-Charreton
2026-05-05  8:32 ` [PATCH proxmox-perl-rs v5 11/27] pve-rs: notify: add binding for triggering state refresh Arthur Bied-Charreton
2026-05-05  8:32 ` [PATCH proxmox-perl-rs v5 12/27] pve-rs: notify: add binding for initial OAuth2 refresh token exchange Arthur Bied-Charreton
2026-05-05  8:32 ` [PATCH proxmox-widget-toolkit v5 13/27] utils: add OAuth2 flow handlers Arthur Bied-Charreton
2026-05-05  8:32 ` [PATCH proxmox-widget-toolkit v5 14/27] utils: oauth2: add callback handler Arthur Bied-Charreton
2026-05-05  8:32 ` [PATCH proxmox-widget-toolkit v5 15/27] notifications: add opt-in OAuth2 support for SMTP targets Arthur Bied-Charreton
2026-05-05  8:32 ` [PATCH pve-manager v5 16/27] notifications: smtp: api: add XOAUTH2 parameters Arthur Bied-Charreton
2026-05-05  8:32 ` [PATCH pve-manager v5 17/27] notifications: add endpoint for initial OAuth2 refresh token exchange Arthur Bied-Charreton
2026-05-05  8:32 ` [PATCH pve-manager v5 18/27] pveupdate: refresh notification targets' OAuth2 state Arthur Bied-Charreton
2026-05-05  8:32 ` [PATCH pve-manager v5 19/27] login: handle OAuth2 callback Arthur Bied-Charreton
2026-05-05  8:32 ` [PATCH pve-manager v5 20/27] fix #7238: notifications: smtp: add XOAUTH2 support Arthur Bied-Charreton
2026-05-05  8:32 ` [PATCH proxmox-backup v5 21/27] notifications: add XOAUTH2 parameters to endpoints Arthur Bied-Charreton
2026-05-05  8:32 ` [PATCH proxmox-backup v5 22/27] notifications: add endpoint for initial OAuth2 refresh token exchange Arthur Bied-Charreton
2026-05-05  8:32 ` [PATCH proxmox-backup v5 23/27] login: handle OAuth2 callback Arthur Bied-Charreton
2026-05-05  8:32 ` [PATCH proxmox-backup v5 24/27] fix #7238: notifications: smtp: add XOAUTH2 support Arthur Bied-Charreton
2026-05-05  8:32 ` [PATCH proxmox-backup v5 25/27] daily-update: refresh OAuth2 state for SMTP notification endpoints Arthur Bied-Charreton
2026-05-05  8:32 ` [PATCH proxmox-backup v5 26/27] notifications: add OAuth2 section to SMTP targets docs Arthur Bied-Charreton
2026-05-05  8:32 ` [PATCH pve-docs v5 27/27] " Arthur Bied-Charreton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260505083248.36450-9-a.bied-charreton@proxmox.com \
    --to=a.bied-charreton@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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