[PATCH proxmox v2 02/16] notify: smtp: Introduce state management

[Arthur Bied-Charreton <a.bied-charreton@proxmox.com>]

Export a new State struct in the xoauth2 module with associated
functionality for loading, updating, and persisting the OAuth2 state
for SMTP endpoints.

The API for loading and saving the state is exposed through the
Context trait, in order to make migration as easy as possible in
a future where we might want to move towards KV storage instead
of files for secret management. It is made specific to oauth state,
because this implementation assumes invariants that hold for oauth2
refresh tokens (documented in the smtp::xoauth2 module's doc comments),
but are likely to be incorrect for other kinds of state that may be added
in the future.

The State struct is made public, to support the long-term goal for
the Context trait to be implemented by the products themselves.

The nix crate is added for the sys::stat::Mode struct, and
proxmox-sys is now pulled in unconditionally since it is now used in the
Context implementations.

Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
 proxmox-notify/Cargo.toml                    |  12 +-
 proxmox-notify/debian/control                |  41 +++----
 proxmox-notify/src/context/mod.rs            |  14 +++
 proxmox-notify/src/context/pbs.rs            |  14 +++
 proxmox-notify/src/context/pve.rs            |  17 ++-
 proxmox-notify/src/context/test.rs           |  14 +++
 proxmox-notify/src/endpoints/smtp.rs         |   2 +
 proxmox-notify/src/endpoints/smtp/xoauth2.rs | 115 +++++++++++++++++++
 proxmox-notify/src/lib.rs                    |  12 ++
 9 files changed, 208 insertions(+), 33 deletions(-)

diff --git a/proxmox-notify/Cargo.toml b/proxmox-notify/Cargo.toml
index 421bb6c3..dfd4b8a4 100644
--- a/proxmox-notify/Cargo.toml
+++ b/proxmox-notify/Cargo.toml
@@ -36,16 +36,18 @@ proxmox-schema = { workspace = true, features = ["api-macro", "api-types"] }
 proxmox-section-config = { workspace = true }
 proxmox-serde.workspace = true
 proxmox-sendmail = { workspace = true, optional = true }
-proxmox-sys = { workspace = true, optional = true }
+proxmox-sys = { workspace = true }
 proxmox-time.workspace = true
 proxmox-uuid = { workspace = true, features = ["serde"] }
+nix = { workspace = true }
+
 
 [features]
 default = ["sendmail", "gotify", "smtp", "webhook"]
-mail-forwarder = ["dep:mail-parser", "dep:proxmox-sys", "proxmox-sendmail/mail-forwarder"]
-sendmail = ["dep:proxmox-sys", "dep:proxmox-sendmail"]
+mail-forwarder = ["dep:mail-parser", "proxmox-sendmail/mail-forwarder"]
+sendmail = ["dep:proxmox-sendmail"]
 gotify = ["dep:proxmox-http", "dep:http"]
-pve-context = ["dep:proxmox-sys"]
-pbs-context = ["dep:proxmox-sys"]
+pve-context = []
+pbs-context = []
 smtp = ["dep:lettre", "dep:oauth2", "dep:ureq", "dep:http"]
 webhook = ["dep:http", "dep:percent-encoding", "dep:proxmox-base64", "dep:proxmox-http"]
diff --git a/proxmox-notify/debian/control b/proxmox-notify/debian/control
index 98e5475c..ca6e7567 100644
--- a/proxmox-notify/debian/control
+++ b/proxmox-notify/debian/control
@@ -11,6 +11,7 @@ Build-Depends-Arch: cargo:native <!nocheck>,
  librust-handlebars-5+default-dev <!nocheck>,
  librust-http-1+default-dev <!nocheck>,
  librust-lettre-0.11+default-dev (>= 0.11.1-~~) <!nocheck>,
+ librust-nix-0.29+default-dev <!nocheck>,
  librust-oauth2-5-dev <!nocheck>,
  librust-openssl-0.10+default-dev <!nocheck>,
  librust-percent-encoding-2+default-dev (>= 2.1-~~) <!nocheck>,
@@ -52,6 +53,7 @@ Depends:
  librust-anyhow-1+default-dev,
  librust-const-format-0.2+default-dev,
  librust-handlebars-5+default-dev,
+ librust-nix-0.29+default-dev,
  librust-openssl-0.10+default-dev,
  librust-proxmox-http-error-1+default-dev,
  librust-proxmox-human-byte-1+default-dev,
@@ -61,6 +63,7 @@ Depends:
  librust-proxmox-section-config-3+default-dev (>= 3.1.0-~~),
  librust-proxmox-serde-1+default-dev,
  librust-proxmox-serde-1+serde-json-dev,
+ librust-proxmox-sys-1+default-dev (>= 1.0.1-~~),
  librust-proxmox-time-2+default-dev (>= 2.1.0-~~),
  librust-proxmox-uuid-1+default-dev (>= 1.1.0-~~),
  librust-proxmox-uuid-1+serde-dev (>= 1.1.0-~~),
@@ -74,14 +77,21 @@ Recommends:
 Suggests:
  librust-proxmox-notify+gotify-dev (= ${binary:Version}),
  librust-proxmox-notify+mail-forwarder-dev (= ${binary:Version}),
- librust-proxmox-notify+pbs-context-dev (= ${binary:Version}),
  librust-proxmox-notify+sendmail-dev (= ${binary:Version}),
  librust-proxmox-notify+smtp-dev (= ${binary:Version}),
  librust-proxmox-notify+webhook-dev (= ${binary:Version})
 Provides:
+ librust-proxmox-notify+pbs-context-dev (= ${binary:Version}),
+ librust-proxmox-notify+pve-context-dev (= ${binary:Version}),
  librust-proxmox-notify-1-dev (= ${binary:Version}),
+ librust-proxmox-notify-1+pbs-context-dev (= ${binary:Version}),
+ librust-proxmox-notify-1+pve-context-dev (= ${binary:Version}),
  librust-proxmox-notify-1.0-dev (= ${binary:Version}),
- librust-proxmox-notify-1.0.3-dev (= ${binary:Version})
+ librust-proxmox-notify-1.0+pbs-context-dev (= ${binary:Version}),
+ librust-proxmox-notify-1.0+pve-context-dev (= ${binary:Version}),
+ librust-proxmox-notify-1.0.3-dev (= ${binary:Version}),
+ librust-proxmox-notify-1.0.3+pbs-context-dev (= ${binary:Version}),
+ librust-proxmox-notify-1.0.3+pve-context-dev (= ${binary:Version})
 Description: Notification base and plugins - Rust source code
  Source code for Debianized Rust crate "proxmox-notify"
 
@@ -127,8 +137,7 @@ Depends:
  ${misc:Depends},
  librust-proxmox-notify-dev (= ${binary:Version}),
  librust-mail-parser-0.11+default-dev,
- librust-proxmox-sendmail-1+mail-forwarder-dev (>= 1.0.2-~~),
- librust-proxmox-sys-1+default-dev (>= 1.0.1-~~)
+ librust-proxmox-sendmail-1+mail-forwarder-dev (>= 1.0.2-~~)
 Provides:
  librust-proxmox-notify-1+mail-forwarder-dev (= ${binary:Version}),
  librust-proxmox-notify-1.0+mail-forwarder-dev (= ${binary:Version}),
@@ -137,35 +146,13 @@ Description: Notification base and plugins - feature "mail-forwarder"
  This metapackage enables feature "mail-forwarder" for the Rust proxmox-notify
  crate, by pulling in any additional dependencies needed by that feature.
 
-Package: librust-proxmox-notify+pbs-context-dev
-Architecture: any
-Multi-Arch: same
-Depends:
- ${misc:Depends},
- librust-proxmox-notify-dev (= ${binary:Version}),
- librust-proxmox-sys-1+default-dev (>= 1.0.1-~~)
-Provides:
- librust-proxmox-notify+pve-context-dev (= ${binary:Version}),
- librust-proxmox-notify-1+pbs-context-dev (= ${binary:Version}),
- librust-proxmox-notify-1+pve-context-dev (= ${binary:Version}),
- librust-proxmox-notify-1.0+pbs-context-dev (= ${binary:Version}),
- librust-proxmox-notify-1.0+pve-context-dev (= ${binary:Version}),
- librust-proxmox-notify-1.0.3+pbs-context-dev (= ${binary:Version}),
- librust-proxmox-notify-1.0.3+pve-context-dev (= ${binary:Version})
-Description: Notification base and plugins - feature "pbs-context" and 1 more
- This metapackage enables feature "pbs-context" for the Rust proxmox-notify
- crate, by pulling in any additional dependencies needed by that feature.
- .
- Additionally, this package also provides the "pve-context" feature.
-
 Package: librust-proxmox-notify+sendmail-dev
 Architecture: any
 Multi-Arch: same
 Depends:
  ${misc:Depends},
  librust-proxmox-notify-dev (= ${binary:Version}),
- librust-proxmox-sendmail-1+default-dev (>= 1.0.2-~~),
- librust-proxmox-sys-1+default-dev (>= 1.0.1-~~)
+ librust-proxmox-sendmail-1+default-dev (>= 1.0.2-~~)
 Provides:
  librust-proxmox-notify-1+sendmail-dev (= ${binary:Version}),
  librust-proxmox-notify-1.0+sendmail-dev (= ${binary:Version}),
diff --git a/proxmox-notify/src/context/mod.rs b/proxmox-notify/src/context/mod.rs
index 8b6e2c43..783ac6da 100644
--- a/proxmox-notify/src/context/mod.rs
+++ b/proxmox-notify/src/context/mod.rs
@@ -1,6 +1,7 @@
 use std::fmt::Debug;
 use std::sync::Mutex;
 
+use crate::endpoints::smtp::State;
 use crate::renderer::TemplateSource;
 use crate::Error;
 
@@ -32,6 +33,19 @@ pub trait Context: Send + Sync + Debug {
         namespace: Option<&str>,
         source: TemplateSource,
     ) -> Result<Option<String>, Error>;
+    /// Load OAuth state for `endpoint_name`.
+    ///
+    /// The state file does not need to be locked, it is okay to just let the faster node "win"
+    /// as long as the invariants documented by [`smtp::xoauth2::get_microsoft_token`] and
+    /// [`smtp::xoauth2::get_google_token`] hold, see those functions' doc comments for details.
+    fn load_oauth_state(&self, endpoint_name: &str) -> Result<State, Error>;
+    /// Save OAuth state `state` for `endpoint_name`. Passing `None` deletes
+    /// the state file for `endpoint_name`.
+    ///
+    /// The state file does not need to be locked, it is okay to just let the faster node "win"
+    /// as long as the invariants documented by [`smtp::xoauth2::get_microsoft_token`] and
+    /// [`smtp::xoauth2::get_google_token`] hold, see those functions' doc comments for details.
+    fn save_oauth_state(&self, endpoint_name: &str, state: Option<State>) -> Result<(), Error>;
 }
 
 #[cfg(not(test))]
diff --git a/proxmox-notify/src/context/pbs.rs b/proxmox-notify/src/context/pbs.rs
index 3e5da59c..6c82a469 100644
--- a/proxmox-notify/src/context/pbs.rs
+++ b/proxmox-notify/src/context/pbs.rs
@@ -7,6 +7,7 @@ use proxmox_schema::{ObjectSchema, Schema, StringSchema};
 use proxmox_section_config::{SectionConfig, SectionConfigPlugin};
 
 use crate::context::{common, Context};
+use crate::endpoints::smtp::State;
 use crate::renderer::TemplateSource;
 use crate::Error;
 
@@ -125,6 +126,19 @@ impl Context for PBSContext {
             .map_err(|err| Error::Generic(format!("could not load template: {err}")))?;
         Ok(template_string)
     }
+
+    fn load_oauth_state(&self, endpoint_name: &str) -> Result<State, Error> {
+        let path = format!("/var/lib/proxmox-backup-priv/notifications/state-{endpoint_name}.json");
+        State::load(path)
+    }
+
+    fn save_oauth_state(&self, endpoint_name: &str, state: Option<State>) -> Result<(), Error> {
+        let path = format!("/var/lib/proxmox-backup-priv/notifications/state-{endpoint_name}.json");
+        match state {
+            Some(s) => s.save(path, nix::sys::stat::Mode::from_bits_truncate(0o600)),
+            None => Ok(State::delete(path)),
+        }
+    }
 }
 
 #[cfg(test)]
diff --git a/proxmox-notify/src/context/pve.rs b/proxmox-notify/src/context/pve.rs
index a97cce26..28d9ab82 100644
--- a/proxmox-notify/src/context/pve.rs
+++ b/proxmox-notify/src/context/pve.rs
@@ -1,7 +1,9 @@
+use std::path::Path;
+
 use crate::context::{common, Context};
+use crate::endpoints::smtp::State;
 use crate::renderer::TemplateSource;
 use crate::Error;
-use std::path::Path;
 
 fn lookup_mail_address(content: &str, user: &str) -> Option<String> {
     common::normalize_for_return(content.lines().find_map(|line| {
@@ -74,6 +76,19 @@ impl Context for PVEContext {
             .map_err(|err| Error::Generic(format!("could not load template: {err}")))?;
         Ok(template_string)
     }
+
+    fn load_oauth_state(&self, endpoint_name: &str) -> Result<State, Error> {
+        let path = format!("/etc/pve/priv/notifications/state-{endpoint_name}.json");
+        State::load(path)
+    }
+
+    fn save_oauth_state(&self, endpoint_name: &str, state: Option<State>) -> Result<(), Error> {
+        let path = format!("/etc/pve/priv/notifications/state-{endpoint_name}.json");
+        match state {
+            Some(s) => s.save(path, nix::sys::stat::Mode::from_bits_truncate(0o600)),
+            None => Ok(State::delete(path)),
+        }
+    }
 }
 
 pub static PVE_CONTEXT: PVEContext = PVEContext;
diff --git a/proxmox-notify/src/context/test.rs b/proxmox-notify/src/context/test.rs
index 2c236b4c..d02f2990 100644
--- a/proxmox-notify/src/context/test.rs
+++ b/proxmox-notify/src/context/test.rs
@@ -1,4 +1,5 @@
 use crate::context::Context;
+use crate::endpoints::smtp::State;
 use crate::renderer::TemplateSource;
 use crate::Error;
 
@@ -40,4 +41,17 @@ impl Context for TestContext {
     ) -> Result<Option<String>, Error> {
         Ok(Some(String::new()))
     }
+
+    fn load_oauth_state(&self, endpoint_name: &str) -> Result<State, Error> {
+        let path = format!("/tmp/notifications/state-{endpoint_name}.json");
+        State::load(path)
+    }
+
+    fn save_oauth_state(&self, endpoint_name: &str, state: Option<State>) -> Result<(), Error> {
+        let path = format!("/tmp/notifications/state-{endpoint_name}.json");
+        match state {
+            Some(s) => s.save(path, nix::sys::stat::Mode::from_bits_truncate(0o750)),
+            None => Ok(State::delete(path)),
+        }
+    }
 }
diff --git a/proxmox-notify/src/endpoints/smtp.rs b/proxmox-notify/src/endpoints/smtp.rs
index d1cdb540..172bcdba 100644
--- a/proxmox-notify/src/endpoints/smtp.rs
+++ b/proxmox-notify/src/endpoints/smtp.rs
@@ -25,6 +25,8 @@ const SMTP_TIMEOUT: u16 = 5;
 
 mod xoauth2;
 
+pub use xoauth2::State;
+
 #[api]
 #[derive(Debug, Serialize, Deserialize, Default, Clone, Copy)]
 #[serde(rename_all = "kebab-case")]
diff --git a/proxmox-notify/src/endpoints/smtp/xoauth2.rs b/proxmox-notify/src/endpoints/smtp/xoauth2.rs
index 90ee630f..97ea46d8 100644
--- a/proxmox-notify/src/endpoints/smtp/xoauth2.rs
+++ b/proxmox-notify/src/endpoints/smtp/xoauth2.rs
@@ -1,10 +1,125 @@
+use std::path::Path;
+
 use oauth2::{
     basic::BasicClient, AccessToken, AuthUrl, ClientId, ClientSecret, RefreshToken, TokenResponse,
     TokenUrl,
 };
+use serde::{Deserialize, Serialize};
+use tracing::debug;
 
 use crate::Error;
 
+#[derive(Serialize, Deserialize, Clone, Debug, Default)]
+#[serde(rename_all = "kebab-case")]
+pub struct State {
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub oauth2_refresh_token: Option<String>,
+    pub last_refreshed: i64,
+}
+
+impl From<Option<String>> for State {
+    fn from(value: Option<String>) -> Self {
+        Self {
+            oauth2_refresh_token: value,
+            last_refreshed: proxmox_time::epoch_i64(),
+        }
+    }
+}
+
+/// Attempt to create a directory at `path` with `mode`, returning `Ok(())` if the directory either already
+/// exists, or was successfully created.
+///
+/// `pmxcfs` automatically sets the x-bit in directory permissions, however it does not allow the user
+/// to set it, the `fchmod` fails with `EPERM` on directories with `0o700`.
+///
+/// The `proxmox_sys` version of this function unconditionally logs mode mismatches even with
+/// `enforce_permissions == false` AND calls `fchmod`. This means that if using that version, we would
+/// always get a permission mismatch warning in the `pvedaemon` logs, even though we do not need the
+/// `fchmod` call.
+fn ensure_dir_exists<P: AsRef<Path>>(path: P, mode: nix::sys::stat::Mode) -> Result<(), Error> {
+    match nix::unistd::mkdir(path.as_ref(), mode) {
+        Ok(()) | Err(nix::errno::Errno::EEXIST) => Ok(()),
+        Err(e) => Err(Error::StatePersistence(
+            path.as_ref().to_string_lossy().into(),
+            e.into(),
+        )),
+    }
+}
+
+impl State {
+    /// Load the state for the endpoint identified by `endpoint_name`, instantiating a default object
+    /// if yes state exists.
+    ///
+    /// # Errors
+    /// An [`Error`] is returned if deserialization of the state object fails.
+    pub(crate) fn load<P: AsRef<Path>>(path: P) -> Result<State, Error> {
+        let path_str = path.as_ref().to_string_lossy();
+        match proxmox_sys::fs::file_get_optional_contents(&path)
+            .map_err(|e| Error::StateRetrieval(path_str.to_string(), e.into()))?
+        {
+            Some(bytes) => {
+                debug!("loaded state file from {path_str}");
+                serde_json::from_slice(&bytes)
+                    .map_err(|e| Error::StateRetrieval(path_str.to_string(), e.into()))
+            }
+            None => {
+                debug!(
+                    "no existing state file found for endpoint at {path_str}, creating empty state"
+                );
+                Ok(State::default())
+            }
+        }
+    }
+
+    /// Persist the state for the endpoint identified by `endpoint_name`.
+    ///
+    /// # Errors
+    /// An [`Error`] is returned if serialization of the state object, or the final write, fail.
+    pub(crate) fn save<P: AsRef<Path>>(
+        self,
+        path: P,
+        mode: nix::sys::stat::Mode,
+    ) -> Result<(), Error> {
+        let path_str = path.as_ref().to_string_lossy();
+        let parent = path.as_ref().parent().unwrap();
+
+        debug!("attempting to persist state at {path_str}");
+
+        ensure_dir_exists(parent, mode)?;
+
+        let s = serde_json::to_string_pretty(&self)
+            .map_err(|e| Error::StatePersistence(path_str.to_string(), e.into()))?;
+
+        proxmox_sys::fs::replace_file(
+            &path,
+            s.as_bytes(),
+            proxmox_sys::fs::CreateOptions::new().perm(mode),
+            false,
+        )
+        .map_err(|e| Error::StatePersistence(path_str.to_string(), e.into()))
+    }
+
+    /// Delete the state for the endpoint identitied by `endpoint_name`.
+    ///
+    /// # Errors
+    /// An [`Error`] is returned if the state file cannot be deleted.
+    pub(crate) fn delete<P: AsRef<Path>>(path: P) {
+        let _ = std::fs::remove_file(&path);
+    }
+
+    /// Set `last_refreshed`.
+    pub fn set_last_refreshed(mut self, last_refreshed: i64) -> Self {
+        self.last_refreshed = last_refreshed;
+        self
+    }
+
+    /// Set `oauth2_refresh_token`.
+    pub fn set_oauth2_refresh_token(mut self, oauth2_refresh_token: Option<String>) -> Self {
+        self.oauth2_refresh_token = oauth2_refresh_token;
+        self
+    }
+}
+
 /// This newtype implements the `SyncHttpClient` trait for [`ureq::Agent`]. This allows
 /// us to avoid pulling in a different backend like `reqwest`.
 ///
diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs
index 879f8326..619dd7db 100644
--- a/proxmox-notify/src/lib.rs
+++ b/proxmox-notify/src/lib.rs
@@ -41,6 +41,10 @@ pub enum Error {
     FilterFailed(String),
     /// The notification's template string could not be rendered
     RenderError(Box<dyn StdError + Send + Sync>),
+    /// The state for an endpoint could not be persisted
+    StatePersistence(String, Box<dyn StdError + Send + Sync>),
+    /// The state for an endpoint could not be retrieved
+    StateRetrieval(String, Box<dyn StdError + Send + Sync>),
     /// Generic error for anything else
     Generic(String),
 }
@@ -70,6 +74,12 @@ impl Display for Error {
             Error::FilterFailed(message) => {
                 write!(f, "could not apply filter: {message}")
             }
+            Error::StatePersistence(path, err) => {
+                write!(f, "could not persist state at {path}: {err}")
+            }
+            Error::StateRetrieval(path, err) => {
+                write!(f, "could not retrieve state from {path}: {err}")
+            }
             Error::RenderError(err) => write!(f, "could not render notification template: {err}"),
             Error::Generic(message) => f.write_str(message),
         }
@@ -86,6 +96,8 @@ impl StdError for Error {
             Error::TargetTestFailed(errs) => Some(&*errs[0]),
             Error::FilterFailed(_) => None,
             Error::RenderError(err) => Some(&**err),
+            Error::StatePersistence(_, err) => Some(&**err),
+            Error::StateRetrieval(_, err) => Some(&**err),
             Error::Generic(_) => None,
         }
     }
-- 
2.47.3