* [PATCH cluster/docs/manager/proxmox{,-perl-rs,-widget-toolkit} 00/17] fix #7238: Add XOAUTH2 authentication support for SMTP notification targets
@ 2026-02-13 16:03 Arthur Bied-Charreton
2026-02-13 16:03 ` [PATCH proxmox 1/7] notify (smtp): Introduce xoauth2 module Arthur Bied-Charreton
` (18 more replies)
0 siblings, 19 replies; 38+ messages in thread
From: Arthur Bied-Charreton @ 2026-02-13 16:03 UTC (permalink / raw)
To: pve-devel
This series adds XOAUTH2 support for SMTP notification targets, motivated by Microsoft's
upcoming deprecation of basic authentication for SMTP [0]. Google and Microsoft are supported as
OAuth2 providers.
OAuth2 refresh tokens may need to be rotated, therefore they are treated as state, not config.
They are persisted in separate JSON files and managed entirely from the Rust side. Each endpoint
has its own state file to avoid having to lock the entire file system everytime a state update
is made.
The oauth2 crate is used with a local ureq backend (newtype over ureq::Agent). oauth2's ureq
feature is currently patched out in Debian due to a ureq 2/3 version mismatch (oauth2 still
depends on ureq 2.x, and Debian only packages 3.x).
Since tokens that are not used for extended periods of time may expire, token refresh is triggered
proactively via pveupdate, additionally to every time a notification is sent, to prevent
unused endpoints' tokens from expiring.
The UI part of the OAuth2 flow is made opt-in in order to allow adding it only to PVE, and later
to PBS in a follow-up series.
This series requires the following version requirement bumps:
* pve-manager requires bumped proxmox-widget-toolkit and proxmox-perl-rs
* proxmox-perl-rs requires bumped proxmox-notify
Known issues:
- Microsoft OAuth2 support is untested (no test tenant, somehow impossible to create a free test
account). It is implemented by following the Microsoft Entra ID docs, but there might be something
I overlooked. I would highly appreciate it if someone was able to test it with their account.
Changes since RFC:
proxmox-notify:
* Simplify state handling logic simplified by special-casing SMTP endpoints instead of making it
generic
* Only pull oauth2 and ureq crates with the smtp feature
* Create state files in `/etc/pve/priv/notifications` subdirectory
* Create one state file per endpoint (as opposed to one for all) to avoid races
* Fix TOCTOU errors by using `proxmox_sys::fs::file_get_optional_contents` for IO
* Add logging and state-related error types
* Factor out SMTP transport building to its own function
pve-manager,proxmox-perl-rs:
* Pass SMTP endpoints configs as hash instead of flat list of parameters
pve-manager:
* Use `let` instead of `const` for JS variables
* Rename endpoint for triggering endpoint state refresh from `refresh-targets` to
`trigger-state-refresh`
pve-docs:
* Add some more provider-related tips/warnings
* Explain what is meant by "manual intervention"
* Show how to add an XOAUTH2 SMTP endpoint via `pvesh`
[0] https://techcommunity.microsoft.com/blog/exchange/updated-exchange-online-smtp-auth-basic-authentication-deprecation-timeline/4489835
[1] https://git.proxmox.com/?p=debcargo-conf.git;a=blob;f=src/oauth2/debian/patches/disable-ureq.patch;h=828b883a83a86927c5cd32df055226a5e78e8bea;hb=refs/heads/proxmox/trixie
proxmox:
Arthur Bied-Charreton (7):
notify (smtp): Introduce xoauth2 module
notify (smtp): Introduce state module
notify (smtp): Factor out transport building logic into own function
notify (smtp): Update API with OAuth2 parameters
notify (smtp): Add state handling logic
notify (smtp): Add XOAUTH2 authentication support
notify (smtp): Add logging and state-related error types
proxmox-notify/Cargo.toml | 6 +-
proxmox-notify/debian/control | 14 +-
proxmox-notify/src/api/common.rs | 16 ++
proxmox-notify/src/api/smtp.rs | 120 ++++++++--
proxmox-notify/src/context/mod.rs | 6 +
proxmox-notify/src/context/pbs.rs | 8 +
proxmox-notify/src/context/pve.rs | 8 +
proxmox-notify/src/context/test.rs | 8 +
proxmox-notify/src/endpoints/smtp.rs | 237 +++++++++++++++++--
proxmox-notify/src/endpoints/smtp/state.rs | 76 ++++++
proxmox-notify/src/endpoints/smtp/xoauth2.rs | 167 +++++++++++++
proxmox-notify/src/lib.rs | 35 ++-
12 files changed, 649 insertions(+), 52 deletions(-)
create mode 100644 proxmox-notify/src/endpoints/smtp/state.rs
create mode 100644 proxmox-notify/src/endpoints/smtp/xoauth2.rs
proxmox-perl-rs:
Arthur Bied-Charreton (1):
notify (smtp): add oauth2 parameters to bindings
common/src/bindings/notify.rs | 82 ++++++++++++-----------------------
1 file changed, 28 insertions(+), 54 deletions(-)
proxmox-widget-toolkit:
Arthur Bied-Charreton (2):
utils: Add OAuth2 flow handlers
notifications: Add opt-in OAuth2 support for SMTP targets
src/Utils.js | 88 +++++++++++++++
src/panel/SmtpEditPanel.js | 200 +++++++++++++++++++++++++++++++--
src/window/EndpointEditBase.js | 1 +
3 files changed, 278 insertions(+), 11 deletions(-)
pve-manager:
Arthur Bied-Charreton (5):
notifications: Add OAuth2 parameters to schema and add/update
endpoints
notifications: Add trigger-state-refresh endpoint
notifications: Trigger notification target refresh in pveupdate
notifications: Handle OAuth2 callback in login handler
notifications: Opt into OAuth2 authentication
PVE/API2/Cluster/Notifications.pm | 144 +++++++++++++++++++++++++-----
bin/pveupdate | 9 ++
www/manager6/Utils.js | 10 +++
www/manager6/Workspace.js | 20 +++++
4 files changed, 160 insertions(+), 23 deletions(-)
pve-cluster:
Arthur Bied-Charreton (1):
notifications: Add refresh_targets subroutine to PVE::Notify
src/PVE/Notify.pm | 6 ++++++
1 file changed, 6 insertions(+)
pve-docs:
Arthur Bied-Charreton (1):
notifications: Add section about OAuth2 to SMTP targets docs
notifications.adoc | 99 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 99 insertions(+)
Summary over all repositories:
22 files changed, 1220 insertions(+), 140 deletions(-)
--
Generated by murpp 0.9.0
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH proxmox 1/7] notify (smtp): Introduce xoauth2 module
2026-02-13 16:03 [PATCH cluster/docs/manager/proxmox{,-perl-rs,-widget-toolkit} 00/17] fix #7238: Add XOAUTH2 authentication support for SMTP notification targets Arthur Bied-Charreton
@ 2026-02-13 16:03 ` Arthur Bied-Charreton
2026-02-13 16:04 ` [PATCH proxmox 2/7] notify (smtp): Introduce state module Arthur Bied-Charreton
` (17 subsequent siblings)
18 siblings, 0 replies; 38+ messages in thread
From: Arthur Bied-Charreton @ 2026-02-13 16:03 UTC (permalink / raw)
To: pve-devel
Prepare proxmox-notify to use the oauth2 crate for SMTP XOAUTH2 support.
The xoauth2 module handles some of the implementation details related to
supporting XOAUTH2 for SMTP notification targets.
* Add a ureq::Agent newtype wrapper implementing the SyncHttpClient
trait to allow using ureq as oauth2 backend, since OAuth2 dropped the
ureq feature. Debian seems to have patched it out due to a ureq 2/3
version mismatch [1].
* Add get_{google,microsoft}_token functions
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
proxmox-notify/Cargo.toml | 6 +-
proxmox-notify/debian/control | 14 +-
proxmox-notify/src/endpoints/smtp.rs | 2 +
proxmox-notify/src/endpoints/smtp/xoauth2.rs | 167 +++++++++++++++++++
4 files changed, 186 insertions(+), 3 deletions(-)
create mode 100644 proxmox-notify/src/endpoints/smtp/xoauth2.rs
diff --git a/proxmox-notify/Cargo.toml b/proxmox-notify/Cargo.toml
index bc63e19d..d816c695 100644
--- a/proxmox-notify/Cargo.toml
+++ b/proxmox-notify/Cargo.toml
@@ -19,6 +19,9 @@ http = { workspace = true, optional = true }
lettre = { workspace = true, optional = true }
tracing.workspace = true
mail-parser = { workspace = true, optional = true }
+oauth2 = { version = "5.0.0", default-features = false, optional = true }
+ureq = { version = "3.0.11", features = ["platform-verifier"], optional = true }
+
openssl.workspace = true
percent-encoding = { workspace = true, optional = true }
regex.workspace = true
@@ -36,6 +39,7 @@ proxmox-sendmail = { workspace = true, optional = true }
proxmox-sys = { workspace = true, optional = true }
proxmox-time.workspace = true
proxmox-uuid = { workspace = true, features = ["serde"] }
+nix.workspace = true
[features]
default = ["sendmail", "gotify", "smtp", "webhook"]
@@ -44,5 +48,5 @@ sendmail = ["dep:proxmox-sys", "dep:proxmox-sendmail"]
gotify = ["dep:proxmox-http", "dep:http"]
pve-context = ["dep:proxmox-sys"]
pbs-context = ["dep:proxmox-sys"]
-smtp = ["dep:lettre"]
+smtp = ["dep:lettre", "dep:oauth2", "dep:ureq", "dep:http", "dep:proxmox-sys"]
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 e588e485..a84af040 100644
--- a/proxmox-notify/debian/control
+++ b/proxmox-notify/debian/control
@@ -11,6 +11,8 @@ 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>,
librust-proxmox-base64-1+default-dev <!nocheck>,
@@ -33,7 +35,9 @@ Build-Depends-Arch: cargo:native <!nocheck>,
librust-serde-1+default-dev <!nocheck>,
librust-serde-1+derive-dev <!nocheck>,
librust-serde-json-1+default-dev <!nocheck>,
- librust-tracing-0.1+default-dev <!nocheck>
+ librust-tracing-0.1+default-dev <!nocheck>,
+ librust-ureq-3+default-dev (>= 3.0.11-~~) <!nocheck>,
+ librust-ureq-3+platform-verifier-dev (>= 3.0.11-~~) <!nocheck>
Maintainer: Proxmox Support Team <support@proxmox.com>
Standards-Version: 4.7.2
Vcs-Git: git://git.proxmox.com/git/proxmox.git
@@ -49,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,
@@ -177,7 +182,12 @@ Multi-Arch: same
Depends:
${misc:Depends},
librust-proxmox-notify-dev (= ${binary:Version}),
- librust-lettre-0.11+default-dev (>= 0.11.1-~~)
+ librust-http-1+default-dev,
+ librust-lettre-0.11+default-dev (>= 0.11.1-~~),
+ librust-oauth2-5-dev,
+ librust-proxmox-sys-1+default-dev,
+ librust-ureq-3+default-dev (>= 3.0.11-~~),
+ librust-ureq-3+platform-verifier-dev (>= 3.0.11-~~)
Provides:
librust-proxmox-notify-1+smtp-dev (= ${binary:Version}),
librust-proxmox-notify-1.0+smtp-dev (= ${binary:Version}),
diff --git a/proxmox-notify/src/endpoints/smtp.rs b/proxmox-notify/src/endpoints/smtp.rs
index c888dee7..277b70f4 100644
--- a/proxmox-notify/src/endpoints/smtp.rs
+++ b/proxmox-notify/src/endpoints/smtp.rs
@@ -23,6 +23,8 @@ const SMTP_SUBMISSION_STARTTLS_PORT: u16 = 587;
const SMTP_SUBMISSION_TLS_PORT: u16 = 465;
const SMTP_TIMEOUT: u16 = 5;
+mod xoauth2;
+
#[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
new file mode 100644
index 00000000..90ee630f
--- /dev/null
+++ b/proxmox-notify/src/endpoints/smtp/xoauth2.rs
@@ -0,0 +1,167 @@
+use oauth2::{
+ basic::BasicClient, AccessToken, AuthUrl, ClientId, ClientSecret, RefreshToken, TokenResponse,
+ TokenUrl,
+};
+
+use crate::Error;
+
+/// This newtype implements the `SyncHttpClient` trait for [`ureq::Agent`]. This allows
+/// us to avoid pulling in a different backend like `reqwest`.
+///
+/// Debian patched out `[0]` the `ureq` backend due to a `ureq` 2-3 version
+/// mismatch in the `oauth2` crate.
+///
+/// There is an open PR `[1]` in `oauth2`, once/if this is merged, we can drop the
+/// custom client implementation.
+///
+/// `[0]`
+/// https://git.proxmox.com/?p=debcargo-conf.git;a=blob;f=src/oauth2/debian/patches/disable-ureq.patch;h=828b883a83a86927c5cd32df055226a5e78e8bea;hb=refs/heads/proxmox/trixie
+///
+/// `[1]` https://github.com/ramosbugs/oauth2-rs/pull/338
+pub(crate) struct UreqSyncHttpClient(ureq::Agent);
+
+impl Default for UreqSyncHttpClient {
+ /// Set `max_redirects` to 0 to prevent SSRF, see
+ /// https://docs.rs/oauth2/latest/oauth2/#security-warning
+ fn default() -> Self {
+ Self(ureq::Agent::new_with_config(
+ ureq::Agent::config_builder().max_redirects(0).build(),
+ ))
+ }
+}
+
+impl oauth2::SyncHttpClient for UreqSyncHttpClient {
+ type Error = oauth2::HttpClientError<ureq::Error>;
+
+ fn call(&self, request: oauth2::HttpRequest) -> Result<oauth2::HttpResponse, Self::Error> {
+ let uri = request.uri().to_string();
+
+ let response = match request.method() {
+ &http::Method::POST => {
+ let req = request
+ .headers()
+ .iter()
+ .fold(self.0.post(&uri), |req, (name, value)| {
+ req.header(name, value)
+ });
+ req.send(request.body()).map_err(Box::new)?
+ }
+ &http::Method::GET => {
+ let req = request
+ .headers()
+ .iter()
+ .fold(self.0.get(&uri), |req, (name, value)| {
+ req.header(name, value)
+ });
+ req.call().map_err(Box::new)?
+ }
+ m => {
+ return Err(oauth2::HttpClientError::Other(format!(
+ "unexpected method: {m}"
+ )));
+ }
+ };
+
+ let mut builder = http::Response::builder().status(response.status());
+
+ if let Some(content_type) = response.headers().get(http::header::CONTENT_TYPE) {
+ builder = builder.header(http::header::CONTENT_TYPE, content_type);
+ }
+
+ let (_, mut body) = response.into_parts();
+
+ let body = body.read_to_vec().map_err(Box::new)?;
+
+ builder.body(body).map_err(oauth2::HttpClientError::Http)
+ }
+}
+
+/// The result yielded by an OAuth2 token exchange.
+///
+/// A successful OAuth2 token exchange will always return an access token to be
+/// used for authentication.
+///
+/// Some providers additionally yield a new refresh token that should replace the
+/// old one.
+pub(crate) struct TokenExchangeResult {
+ pub access_token: AccessToken,
+ pub refresh_token: Option<RefreshToken>,
+}
+
+/// Perform a Microsoft OAuth2 token exchange.
+///
+/// This always yields a new refresh token, which should be persisted on a best-effort
+/// basis, replacing the one that was passed to this function.
+///
+/// Microsoft Identity Platform refresh tokens have static lifetimes of 90 days, with each
+/// token exchange yielding a new refresh token. The new refresh token is assigned a new
+/// static lifetime, starting from the moment the token exchange was performed.
+///
+/// The old refresh token is not invalidated, rather it keeps the static lifetime it was
+/// assigned at generation time. This means that at any given point in time, there can be
+/// many different refresh tokens that are *all* valid.
+///
+/// https://learn.microsoft.com/en-us/entra/identity-platform/refresh-tokens#token-lifetime
+pub(crate) fn get_microsoft_token(
+ client_id: ClientId,
+ client_secret: ClientSecret,
+ tenant_id: &str,
+ refresh_token: RefreshToken,
+) -> Result<TokenExchangeResult, Error> {
+ let client = BasicClient::new(client_id)
+ .set_client_secret(client_secret)
+ .set_auth_uri(
+ AuthUrl::new(format!(
+ "https://login.microsoftonline.com/{tenant_id}/oauth2/v2.0/authorize"
+ ))
+ .map_err(|e| Error::Generic(format!("invalid auth URL: {e}")))?,
+ )
+ .set_token_uri(
+ TokenUrl::new(format!(
+ "https://login.microsoftonline.com/{tenant_id}/oauth2/v2.0/token"
+ ))
+ .map_err(|e| Error::Generic(format!("invalid token URL: {e}")))?,
+ );
+
+ let token_result = client
+ .exchange_refresh_token(&refresh_token)
+ .request(&UreqSyncHttpClient::default())
+ .map_err(|e| Error::Generic(format!("could not get access token: {e}")))?;
+
+ Ok(TokenExchangeResult {
+ access_token: token_result.access_token().clone(),
+ refresh_token: token_result.refresh_token().cloned(),
+ })
+}
+
+/// Google refresh tokens' TTL is extended at every use. As long as
+/// a token has been used at least once in the past 6 months, and no
+/// other expiration reason applies, the same token can be kept.
+///
+/// https://developers.google.com/identity/protocols/oauth2#expiration
+pub(crate) fn get_google_token(
+ client_id: ClientId,
+ client_secret: ClientSecret,
+ refresh_token: RefreshToken,
+) -> Result<TokenExchangeResult, Error> {
+ let client = BasicClient::new(client_id)
+ .set_client_secret(client_secret)
+ .set_auth_uri(
+ AuthUrl::new("https://accounts.google.com/o/oauth2/v2/auth".into())
+ .map_err(|e| Error::Generic(format!("invalid auth URL: {e}")))?,
+ )
+ .set_token_uri(
+ TokenUrl::new("https://oauth2.googleapis.com/token".into())
+ .map_err(|e| Error::Generic(format!("invalid token URL: {e}")))?,
+ );
+
+ let token_result = client
+ .exchange_refresh_token(&refresh_token)
+ .request(&UreqSyncHttpClient::default())
+ .map_err(|e| Error::Generic(format!("could not get access token: {e}")))?;
+
+ Ok(TokenExchangeResult {
+ access_token: token_result.access_token().clone(),
+ refresh_token: token_result.refresh_token().cloned(),
+ })
+}
--
2.47.3
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH proxmox 2/7] notify (smtp): Introduce state module
2026-02-13 16:03 [PATCH cluster/docs/manager/proxmox{,-perl-rs,-widget-toolkit} 00/17] fix #7238: Add XOAUTH2 authentication support for SMTP notification targets Arthur Bied-Charreton
2026-02-13 16:03 ` [PATCH proxmox 1/7] notify (smtp): Introduce xoauth2 module Arthur Bied-Charreton
@ 2026-02-13 16:04 ` Arthur Bied-Charreton
2026-03-23 12:26 ` Lukas Wagner
2026-02-13 16:04 ` [PATCH proxmox 3/7] notify (smtp): Factor out transport building logic into own function Arthur Bied-Charreton
` (16 subsequent siblings)
18 siblings, 1 reply; 38+ messages in thread
From: Arthur Bied-Charreton @ 2026-02-13 16:04 UTC (permalink / raw)
To: pve-devel
The state module exports a new struct with associated functionality for
loading, updating, and persisting the state for SMTP endpoints with
OAuth2 configured as authentication method.
The path to the state files, as well as their create options, are
retrieved through new Context methods to allow portability between PVE
and PBS.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
proxmox-notify/src/context/mod.rs | 6 ++
proxmox-notify/src/context/pbs.rs | 8 +++
proxmox-notify/src/context/pve.rs | 8 +++
proxmox-notify/src/context/test.rs | 8 +++
proxmox-notify/src/endpoints/smtp.rs | 3 +
proxmox-notify/src/endpoints/smtp/state.rs | 67 ++++++++++++++++++++++
6 files changed, 100 insertions(+)
create mode 100644 proxmox-notify/src/endpoints/smtp/state.rs
diff --git a/proxmox-notify/src/context/mod.rs b/proxmox-notify/src/context/mod.rs
index 8b6e2c43..492442f9 100644
--- a/proxmox-notify/src/context/mod.rs
+++ b/proxmox-notify/src/context/mod.rs
@@ -1,6 +1,8 @@
use std::fmt::Debug;
use std::sync::Mutex;
+use proxmox_sys::fs::CreateOptions;
+
use crate::renderer::TemplateSource;
use crate::Error;
@@ -32,6 +34,10 @@ pub trait Context: Send + Sync + Debug {
namespace: Option<&str>,
source: TemplateSource,
) -> Result<Option<String>, Error>;
+ /// Return the path to the state file for this context.
+ fn state_file_path(&self, name: &str) -> String;
+ /// Create options to be used when writing files containing secrets.
+ fn secret_create_options(&self) -> CreateOptions;
}
#[cfg(not(test))]
diff --git a/proxmox-notify/src/context/pbs.rs b/proxmox-notify/src/context/pbs.rs
index 3e5da59c..4f93b45d 100644
--- a/proxmox-notify/src/context/pbs.rs
+++ b/proxmox-notify/src/context/pbs.rs
@@ -125,6 +125,14 @@ impl Context for PBSContext {
.map_err(|err| Error::Generic(format!("could not load template: {err}")))?;
Ok(template_string)
}
+
+ fn state_file_path(&self, name: &str) -> String {
+ format!("/var/lib/proxmox-backup/priv/notifications/{name}.json")
+ }
+
+ fn secret_create_options(&self) -> proxmox_sys::fs::CreateOptions {
+ proxmox_sys::fs::CreateOptions::new().perm(nix::sys::stat::Mode::from_bits_truncate(0o600))
+ }
}
#[cfg(test)]
diff --git a/proxmox-notify/src/context/pve.rs b/proxmox-notify/src/context/pve.rs
index a97cce26..e30f7b49 100644
--- a/proxmox-notify/src/context/pve.rs
+++ b/proxmox-notify/src/context/pve.rs
@@ -74,6 +74,14 @@ impl Context for PVEContext {
.map_err(|err| Error::Generic(format!("could not load template: {err}")))?;
Ok(template_string)
}
+
+ fn state_file_path(&self, name: &str) -> String {
+ format!("/etc/pve/priv/notifications/{name}.json")
+ }
+
+ fn secret_create_options(&self) -> proxmox_sys::fs::CreateOptions {
+ proxmox_sys::fs::CreateOptions::new().perm(nix::sys::stat::Mode::from_bits_truncate(0o600))
+ }
}
pub static PVE_CONTEXT: PVEContext = PVEContext;
diff --git a/proxmox-notify/src/context/test.rs b/proxmox-notify/src/context/test.rs
index 2c236b4c..7e29d36a 100644
--- a/proxmox-notify/src/context/test.rs
+++ b/proxmox-notify/src/context/test.rs
@@ -40,4 +40,12 @@ impl Context for TestContext {
) -> Result<Option<String>, Error> {
Ok(Some(String::new()))
}
+
+ fn state_file_path(&self, name: &str) -> String {
+ format!("/tmp/notifications/{name}.json")
+ }
+
+ fn secret_create_options(&self) -> proxmox_sys::fs::CreateOptions {
+ proxmox_sys::fs::CreateOptions::new().perm(nix::sys::stat::Mode::from_bits_truncate(0o755))
+ }
}
diff --git a/proxmox-notify/src/endpoints/smtp.rs b/proxmox-notify/src/endpoints/smtp.rs
index 277b70f4..699ed1c6 100644
--- a/proxmox-notify/src/endpoints/smtp.rs
+++ b/proxmox-notify/src/endpoints/smtp.rs
@@ -23,8 +23,11 @@ const SMTP_SUBMISSION_STARTTLS_PORT: u16 = 587;
const SMTP_SUBMISSION_TLS_PORT: u16 = 465;
const SMTP_TIMEOUT: u16 = 5;
+mod state;
mod xoauth2;
+pub(crate) use state::State;
+
#[api]
#[derive(Debug, Serialize, Deserialize, Default, Clone, Copy)]
#[serde(rename_all = "kebab-case")]
diff --git a/proxmox-notify/src/endpoints/smtp/state.rs b/proxmox-notify/src/endpoints/smtp/state.rs
new file mode 100644
index 00000000..60bef590
--- /dev/null
+++ b/proxmox-notify/src/endpoints/smtp/state.rs
@@ -0,0 +1,67 @@
+use serde::{Deserialize, Serialize};
+
+use crate::{context::context, Error};
+
+#[derive(Serialize, Deserialize, Clone, Debug, Default)]
+#[serde(rename_all = "kebab-case")]
+pub(crate) struct State {
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub oauth2_refresh_token: Option<String>,
+ pub last_refreshed: i64,
+}
+
+impl State {
+ /// Instantiate a new [`State`].
+ pub(crate) fn new(oauth2_refresh_token: Option<String>) -> Self {
+ Self {
+ oauth2_refresh_token,
+ last_refreshed: proxmox_time::epoch_i64(),
+ }
+ }
+
+ /// Load the state for the endpoint identified by `name`, instantiating a default object
+ /// if no state exists.
+ ///
+ /// # Errors
+ /// An [`Error`] is returned if deserialization of the state object fails.
+ pub(crate) fn load(name: &str) -> Result<State, Error> {
+ match proxmox_sys::fs::file_get_optional_contents(context().state_file_path(name))
+ .map_err(|e| Error::ConfigDeserialization(e.into()))?
+ {
+ Some(bytes) => {
+ serde_json::from_slice(&bytes).map_err(|e| Error::ConfigDeserialization(e.into()))
+ }
+ None => Ok(State::default()),
+ }
+ }
+
+ /// Persist the state for the endpoint identified by `name`.
+ ///
+ /// # Errors
+ /// An [`Error`] is returned if serialization of the state object, or the final write, fail.
+ pub(crate) fn store(self, name: &str) -> Result<(), Error> {
+ let path = context().state_file_path(name);
+ let parent = std::path::Path::new(&path).parent().unwrap();
+
+ proxmox_sys::fs::ensure_dir_exists(parent, &context().secret_create_options(), false)
+ .map_err(|e| Error::ConfigSerialization(e.into()))?;
+
+ let s = serde_json::to_string_pretty(&self)
+ .map_err(|e| Error::ConfigSerialization(e.into()))?;
+
+ proxmox_sys::fs::replace_file(path, s.as_bytes(), context().secret_create_options(), true)
+ .map_err(|e| Error::ConfigSerialization(e.into()))
+ }
+
+ /// Set `last_refreshed`.
+ pub(crate) fn set_last_refreshed(mut self, last_refreshed: i64) -> Self {
+ self.last_refreshed = last_refreshed;
+ self
+ }
+
+ /// Set `oauth2_refresh_token`.
+ pub(crate) fn set_oauth2_refresh_token(mut self, oauth2_refresh_token: Option<String>) -> Self {
+ self.oauth2_refresh_token = oauth2_refresh_token;
+ self
+ }
+}
--
2.47.3
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH proxmox 3/7] notify (smtp): Factor out transport building logic into own function
2026-02-13 16:03 [PATCH cluster/docs/manager/proxmox{,-perl-rs,-widget-toolkit} 00/17] fix #7238: Add XOAUTH2 authentication support for SMTP notification targets Arthur Bied-Charreton
2026-02-13 16:03 ` [PATCH proxmox 1/7] notify (smtp): Introduce xoauth2 module Arthur Bied-Charreton
2026-02-13 16:04 ` [PATCH proxmox 2/7] notify (smtp): Introduce state module Arthur Bied-Charreton
@ 2026-02-13 16:04 ` Arthur Bied-Charreton
2026-02-13 16:04 ` [PATCH proxmox 4/7] notify (smtp): Update API with OAuth2 parameters Arthur Bied-Charreton
` (15 subsequent siblings)
18 siblings, 0 replies; 38+ messages in thread
From: Arthur Bied-Charreton @ 2026-02-13 16:04 UTC (permalink / raw)
To: pve-devel
XOAUTH2 support will make the transport building logic a lot more
complex, having a separate function for it will make it more manageable.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
proxmox-notify/src/endpoints/smtp.rs | 44 ++++++++++++++++------------
1 file changed, 25 insertions(+), 19 deletions(-)
diff --git a/proxmox-notify/src/endpoints/smtp.rs b/proxmox-notify/src/endpoints/smtp.rs
index 699ed1c6..1340d8ea 100644
--- a/proxmox-notify/src/endpoints/smtp.rs
+++ b/proxmox-notify/src/endpoints/smtp.rs
@@ -162,6 +162,30 @@ pub struct SmtpEndpoint {
pub private_config: SmtpPrivateConfig,
}
+impl SmtpEndpoint {
+ fn build_transport(&self, tls: Tls, port: u16) -> Result<SmtpTransport, Error> {
+ let mut transport_builder = SmtpTransport::builder_dangerous(&self.config.server)
+ .tls(tls)
+ .port(port)
+ .timeout(Some(Duration::from_secs(SMTP_TIMEOUT.into())));
+
+ if let Some(username) = self.config.username.as_deref() {
+ if let Some(password) = self.private_config.password.as_deref() {
+ transport_builder = transport_builder.credentials((username, password).into());
+ } else {
+ return Err(Error::NotifyFailed(
+ self.name().into(),
+ Box::new(Error::Generic(
+ "username is set but no password was provided".to_owned(),
+ )),
+ ));
+ }
+ };
+
+ Ok(transport_builder.build())
+ }
+}
+
impl Endpoint for SmtpEndpoint {
fn send(&self, notification: &Notification) -> Result<(), Error> {
let tls_parameters = TlsParameters::new(self.config.server.clone())
@@ -182,25 +206,7 @@ impl Endpoint for SmtpEndpoint {
}
};
- let mut transport_builder = SmtpTransport::builder_dangerous(&self.config.server)
- .tls(tls)
- .port(port)
- .timeout(Some(Duration::from_secs(SMTP_TIMEOUT.into())));
-
- if let Some(username) = self.config.username.as_deref() {
- if let Some(password) = self.private_config.password.as_deref() {
- transport_builder = transport_builder.credentials((username, password).into());
- } else {
- return Err(Error::NotifyFailed(
- self.name().into(),
- Box::new(Error::Generic(
- "username is set but no password was provided".to_owned(),
- )),
- ));
- }
- }
-
- let transport = transport_builder.build();
+ let transport = self.build_transport(tls, port)?;
let recipients = mail::get_recipients(
self.config.mailto.as_slice(),
--
2.47.3
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH proxmox 4/7] notify (smtp): Update API with OAuth2 parameters
2026-02-13 16:03 [PATCH cluster/docs/manager/proxmox{,-perl-rs,-widget-toolkit} 00/17] fix #7238: Add XOAUTH2 authentication support for SMTP notification targets Arthur Bied-Charreton
` (2 preceding siblings ...)
2026-02-13 16:04 ` [PATCH proxmox 3/7] notify (smtp): Factor out transport building logic into own function Arthur Bied-Charreton
@ 2026-02-13 16:04 ` Arthur Bied-Charreton
2026-02-13 16:04 ` [PATCH proxmox 5/7] notify (smtp): Add state handling logic Arthur Bied-Charreton
` (14 subsequent siblings)
18 siblings, 0 replies; 38+ messages in thread
From: Arthur Bied-Charreton @ 2026-02-13 16:04 UTC (permalink / raw)
To: pve-devel
Add the OAuth2 client & tenant ID to the public config, client secret to
the private config.
The refresh token, which is to be managed separately as state, is taken
as an extra parameter by {add,update}_endpoint to avoid it landing in a
section config.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
proxmox-notify/src/api/smtp.rs | 91 ++++++++++++++++++++--------
proxmox-notify/src/endpoints/smtp.rs | 42 +++++++++++++
2 files changed, 109 insertions(+), 24 deletions(-)
diff --git a/proxmox-notify/src/api/smtp.rs b/proxmox-notify/src/api/smtp.rs
index 470701bf..4231cdae 100644
--- a/proxmox-notify/src/api/smtp.rs
+++ b/proxmox-notify/src/api/smtp.rs
@@ -38,10 +38,15 @@ pub fn get_endpoint(config: &Config, name: &str) -> Result<SmtpConfig, HttpError
/// - an entity with the same name already exists (`400 Bad request`)
/// - the configuration could not be saved (`500 Internal server error`)
/// - mailto *and* mailto_user are both set to `None`
+///
+/// `oauth2_refresh_token` is initially passed through the API when an OAuth2
+/// endpoint is created/updated, however its state is not managed through a
+/// config, which is why it is passed separately.
pub fn add_endpoint(
config: &mut Config,
endpoint_config: SmtpConfig,
private_endpoint_config: SmtpPrivateConfig,
+ oauth2_refresh_token: Option<String>,
) -> Result<(), HttpError> {
if endpoint_config.name != private_endpoint_config.name {
// Programming error by the user of the crate, thus we panic
@@ -76,6 +81,28 @@ pub fn add_endpoint(
})
}
+/// Apply `updater` to the private config identified by `name`, and set
+/// the private config entry afterwards.
+fn update_private_config(
+ config: &mut Config,
+ name: &str,
+ updater: impl FnOnce(&mut SmtpPrivateConfig),
+) -> Result<(), HttpError> {
+ let mut private_config: SmtpPrivateConfig = config
+ .private_config
+ .lookup(SMTP_TYPENAME, name)
+ .map_err(|e| {
+ http_err!(
+ INTERNAL_SERVER_ERROR,
+ "no private config found for SMTP endpoint: {e}"
+ )
+ })?;
+
+ updater(&mut private_config);
+
+ super::set_private_config_entry(config, private_config, SMTP_TYPENAME, name)
+}
+
/// Update existing smtp endpoint
///
/// The caller is responsible for any needed permission checks.
@@ -83,11 +110,16 @@ pub fn add_endpoint(
/// Returns a `HttpError` if:
/// - the configuration could not be saved (`500 Internal server error`)
/// - mailto *and* mailto_user are both set to `None`
+///
+/// `oauth2_refresh_token` is initially passed through the API when an OAuth2
+/// endpoint is created/updated, however its state is not managed through a
+/// config, which is why it is passed separately.
pub fn update_endpoint(
config: &mut Config,
name: &str,
updater: SmtpConfigUpdater,
private_endpoint_config_updater: SmtpPrivateConfigUpdater,
+ oauth2_refresh_token: Option<String>,
delete: Option<&[DeleteableSmtpProperty]>,
digest: Option<&[u8]>,
) -> Result<(), HttpError> {
@@ -103,20 +135,20 @@ pub fn update_endpoint(
DeleteableSmtpProperty::Disable => endpoint.disable = None,
DeleteableSmtpProperty::Mailto => endpoint.mailto.clear(),
DeleteableSmtpProperty::MailtoUser => endpoint.mailto_user.clear(),
- DeleteableSmtpProperty::Password => super::set_private_config_entry(
- config,
- SmtpPrivateConfig {
- name: name.to_string(),
- password: None,
- },
- SMTP_TYPENAME,
- name,
- )?,
+ DeleteableSmtpProperty::Password => {
+ update_private_config(config, name, |c| c.password = None)?
+ }
+ DeleteableSmtpProperty::AuthMethod => endpoint.auth_method = None,
+ DeleteableSmtpProperty::OAuth2ClientId => endpoint.oauth2_client_id = None,
+ DeleteableSmtpProperty::OAuth2ClientSecret => {
+ update_private_config(config, name, |c| c.oauth2_client_secret = None)?
+ }
+ DeleteableSmtpProperty::OAuth2TenantId => endpoint.oauth2_tenant_id = None,
DeleteableSmtpProperty::Port => endpoint.port = None,
DeleteableSmtpProperty::Username => endpoint.username = None,
}
}
- }
+ };
if let Some(mailto) = updater.mailto {
endpoint.mailto = mailto;
@@ -139,29 +171,24 @@ pub fn update_endpoint(
if let Some(mode) = updater.mode {
endpoint.mode = Some(mode);
}
- if let Some(password) = private_endpoint_config_updater.password {
- super::set_private_config_entry(
- config,
- SmtpPrivateConfig {
- name: name.into(),
- password: Some(password),
- },
- SMTP_TYPENAME,
- name,
- )?;
+ if let Some(auth_method) = updater.auth_method {
+ endpoint.auth_method = Some(auth_method);
}
-
if let Some(author) = updater.author {
endpoint.author = Some(author);
}
-
if let Some(comment) = updater.comment {
endpoint.comment = Some(comment);
}
-
if let Some(disable) = updater.disable {
endpoint.disable = Some(disable);
}
+ if let Some(oauth2_client_id) = updater.oauth2_client_id {
+ endpoint.oauth2_client_id = Some(oauth2_client_id);
+ }
+ if let Some(oauth2_tenant_id) = updater.oauth2_tenant_id {
+ endpoint.oauth2_tenant_id = Some(oauth2_tenant_id);
+ }
if endpoint.mailto.is_empty() && endpoint.mailto_user.is_empty() {
http_bail!(
@@ -170,6 +197,15 @@ pub fn update_endpoint(
);
}
+ update_private_config(config, name, |c| {
+ if let Some(password) = private_endpoint_config_updater.password {
+ c.password = Some(password);
+ }
+ if let Some(oauth2_client_secret) = private_endpoint_config_updater.oauth2_client_secret {
+ c.oauth2_client_secret = Some(oauth2_client_secret);
+ }
+ })?;
+
config
.config
.set_data(name, SMTP_TYPENAME, &endpoint)
@@ -204,7 +240,7 @@ pub fn delete_endpoint(config: &mut Config, name: &str) -> Result<(), HttpError>
pub mod tests {
use super::*;
use crate::api::test_helpers::*;
- use crate::endpoints::smtp::SmtpMode;
+ use crate::endpoints::smtp::{SmtpAuthMethod, SmtpMode};
pub fn add_smtp_endpoint_for_test(config: &mut Config, name: &str) -> Result<(), HttpError> {
add_endpoint(
@@ -217,6 +253,7 @@ pub mod tests {
author: Some("root".into()),
comment: Some("Comment".into()),
mode: Some(SmtpMode::StartTls),
+ auth_method: Some(SmtpAuthMethod::Plain),
server: "localhost".into(),
port: Some(555),
username: Some("username".into()),
@@ -225,7 +262,9 @@ pub mod tests {
SmtpPrivateConfig {
name: name.into(),
password: Some("password".into()),
+ oauth2_client_secret: None,
},
+ None,
)?;
assert!(get_endpoint(config, name).is_ok());
@@ -256,6 +295,7 @@ pub mod tests {
Default::default(),
None,
None,
+ None,
)
.is_err());
@@ -273,6 +313,7 @@ pub mod tests {
Default::default(),
Default::default(),
None,
+ None,
Some(&[0; 32]),
)
.is_err());
@@ -304,6 +345,7 @@ pub mod tests {
},
Default::default(),
None,
+ None,
Some(&digest),
)?;
@@ -327,6 +369,7 @@ pub mod tests {
"smtp-endpoint",
Default::default(),
Default::default(),
+ None,
Some(&[
DeleteableSmtpProperty::Author,
DeleteableSmtpProperty::MailtoUser,
diff --git a/proxmox-notify/src/endpoints/smtp.rs b/proxmox-notify/src/endpoints/smtp.rs
index 1340d8ea..361c4da9 100644
--- a/proxmox-notify/src/endpoints/smtp.rs
+++ b/proxmox-notify/src/endpoints/smtp.rs
@@ -84,11 +84,21 @@ pub struct SmtpConfig {
pub port: Option<u16>,
#[serde(skip_serializing_if = "Option::is_none")]
pub mode: Option<SmtpMode>,
+ /// Method to be used for authentication.
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub auth_method: Option<SmtpAuthMethod>,
/// Username to use during authentication.
/// If no username is set, no authentication will be performed.
/// The PLAIN and LOGIN authentication methods are supported
#[serde(skip_serializing_if = "Option::is_none")]
pub username: Option<String>,
+ /// Client ID for XOAUTH2 authentication method.
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub oauth2_client_id: Option<String>,
+ /// Tenant ID for XOAUTH2 authentication method. Only required for
+ /// Microsoft Exchange Online OAuth2.
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub oauth2_tenant_id: Option<String>,
/// Mail address to send a mail to.
#[serde(default, skip_serializing_if = "Vec::is_empty")]
#[updater(serde(skip_serializing_if = "Option::is_none"))]
@@ -135,12 +145,39 @@ pub enum DeleteableSmtpProperty {
MailtoUser,
/// Delete `password`
Password,
+ /// Delete `auth_method`
+ AuthMethod,
+ /// Delete `oauth2_client_id`
+ #[serde(rename = "oauth2-client-id")]
+ OAuth2ClientId,
+ /// Delete `oauth2_client_secret`
+ #[serde(rename = "oauth2-client-secret")]
+ OAuth2ClientSecret,
+ /// Delete `oauth2_tenant_id`
+ #[serde(rename = "oauth2-tenant-id")]
+ OAuth2TenantId,
/// Delete `port`
Port,
/// Delete `username`
Username,
}
+/// Authentication mode to use for SMTP.
+#[api]
+#[derive(Serialize, Deserialize, Clone, Debug, Default, Copy)]
+#[serde(rename_all = "kebab-case")]
+pub enum SmtpAuthMethod {
+ /// Username + password
+ #[default]
+ Plain,
+ /// Google OAuth2
+ #[serde(rename = "google-oauth2")]
+ GoogleOAuth2,
+ /// Microsoft OAuth2
+ #[serde(rename = "microsoft-oauth2")]
+ MicrosoftOAuth2,
+}
+
#[api]
#[derive(Serialize, Deserialize, Clone, Updater, Debug)]
#[serde(rename_all = "kebab-case")]
@@ -151,9 +188,14 @@ pub struct SmtpPrivateConfig {
/// Name of the endpoint
#[updater(skip)]
pub name: String,
+
/// The password to use during authentication.
#[serde(skip_serializing_if = "Option::is_none")]
pub password: Option<String>,
+
+ /// OAuth2 client secret
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub oauth2_client_secret: Option<String>,
}
/// A sendmail notification endpoint.
--
2.47.3
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH proxmox 5/7] notify (smtp): Add state handling logic
2026-02-13 16:03 [PATCH cluster/docs/manager/proxmox{,-perl-rs,-widget-toolkit} 00/17] fix #7238: Add XOAUTH2 authentication support for SMTP notification targets Arthur Bied-Charreton
` (3 preceding siblings ...)
2026-02-13 16:04 ` [PATCH proxmox 4/7] notify (smtp): Update API with OAuth2 parameters Arthur Bied-Charreton
@ 2026-02-13 16:04 ` Arthur Bied-Charreton
2026-03-23 12:26 ` Lukas Wagner
2026-02-13 16:04 ` [PATCH proxmox 6/7] notify (smtp): Add XOAUTH2 authentication support Arthur Bied-Charreton
` (13 subsequent siblings)
18 siblings, 1 reply; 38+ messages in thread
From: Arthur Bied-Charreton @ 2026-02-13 16:04 UTC (permalink / raw)
To: pve-devel
Create new state file in add_endpoint, and create/update existing one in
update_endpoint.
Add trigger_state_refresh to the Endpoint trait, with no-op default
implementation. Override trigger_state_refresh in SmtpEndpoint's
Endpoint impl to trigger an OAuth2 token exchange, in order to rotate
an existing token, or extend its lifetime.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
proxmox-notify/src/api/common.rs | 16 ++++++
proxmox-notify/src/api/smtp.rs | 29 ++++++++++-
proxmox-notify/src/endpoints/smtp.rs | 75 +++++++++++++++++++++++++++-
proxmox-notify/src/lib.rs | 19 +++++++
4 files changed, 137 insertions(+), 2 deletions(-)
diff --git a/proxmox-notify/src/api/common.rs b/proxmox-notify/src/api/common.rs
index fa2356e2..1e6b7d46 100644
--- a/proxmox-notify/src/api/common.rs
+++ b/proxmox-notify/src/api/common.rs
@@ -3,6 +3,22 @@ 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.
+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 4231cdae..8992e789 100644
--- a/proxmox-notify/src/api/smtp.rs
+++ b/proxmox-notify/src/api/smtp.rs
@@ -2,7 +2,7 @@ use proxmox_http_error::HttpError;
use crate::api::{http_bail, http_err};
use crate::endpoints::smtp::{
- DeleteableSmtpProperty, SmtpConfig, SmtpConfigUpdater, SmtpPrivateConfig,
+ self, DeleteableSmtpProperty, SmtpConfig, SmtpConfigUpdater, SmtpPrivateConfig,
SmtpPrivateConfigUpdater, SMTP_TYPENAME,
};
use crate::Config;
@@ -69,6 +69,16 @@ pub fn add_endpoint(
&endpoint_config.name,
)?;
+ smtp::State::new(oauth2_refresh_token)
+ .store(&endpoint_config.name)
+ .map_err(|e| {
+ http_err!(
+ INTERNAL_SERVER_ERROR,
+ "could not create state file for endpoint '{}': {e}",
+ endpoint_config.name
+ )
+ })?;
+
config
.config
.set_data(&endpoint_config.name, SMTP_TYPENAME, &endpoint_config)
@@ -206,6 +216,23 @@ pub fn update_endpoint(
}
})?;
+ smtp::State::load(name)
+ .map_err(|e| {
+ http_err!(
+ INTERNAL_SERVER_ERROR,
+ "could not load state for endpoint '{name}': {e}"
+ )
+ })?
+ .set_oauth2_refresh_token(oauth2_refresh_token)
+ .set_last_refreshed(proxmox_time::epoch_i64())
+ .store(name)
+ .map_err(|e| {
+ http_err!(
+ INTERNAL_SERVER_ERROR,
+ "could not persist state for endpoint '{name}': {e}"
+ )
+ })?;
+
config
.config
.set_data(name, SMTP_TYPENAME, &endpoint)
diff --git a/proxmox-notify/src/endpoints/smtp.rs b/proxmox-notify/src/endpoints/smtp.rs
index 361c4da9..244799fd 100644
--- a/proxmox-notify/src/endpoints/smtp.rs
+++ b/proxmox-notify/src/endpoints/smtp.rs
@@ -1,12 +1,15 @@
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::authentication::{Credentials, Mechanism};
use lettre::transport::smtp::client::{Tls, TlsParameters};
use lettre::{message::header::ContentType, Message, SmtpTransport, Transport};
use serde::{Deserialize, Serialize};
+use oauth2::{ClientId, ClientSecret, RefreshToken};
+
use proxmox_schema::api_types::COMMENT_SCHEMA;
use proxmox_schema::{api, Updater};
@@ -22,6 +25,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_SECONDS: u64 = 60 * 60 * 12;
mod state;
mod xoauth2;
@@ -205,6 +209,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)
@@ -336,6 +377,38 @@ impl Endpoint for SmtpEndpoint {
fn disabled(&self) -> bool {
self.config.disable.unwrap_or_default()
}
+
+ fn trigger_state_refresh(&self) -> Result<(), Error> {
+ let state = State::load(self.name())?;
+
+ let Some(refresh_token) = &state.oauth2_refresh_token else {
+ return Ok(());
+ };
+
+ // The refresh job is configured in pveupdate, which runs once for each node.
+ // Don't refresh if we already did it recently.
+ if SystemTime::now()
+ .duration_since(UNIX_EPOCH + Duration::from_secs(state.last_refreshed as u64))
+ .map_err(|e| Error::Generic(e.to_string()))?
+ < Duration::from_secs(SMTP_STATE_REFRESH_CUTOFF_SECONDS)
+ {
+ return Ok(());
+ }
+
+ let Some(auth_method) = self.config.auth_method.as_ref() else {
+ return Ok(());
+ };
+
+ 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())
+ .store(self.name())
+ }
}
/// Construct a lettre `Message` from a raw email message.
diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs
index 879f8326..c1a5e535 100644
--- a/proxmox-notify/src/lib.rs
+++ b/proxmox-notify/src/lib.rs
@@ -157,6 +157,11 @@ pub trait Endpoint {
/// Check if the endpoint is disabled
fn disabled(&self) -> bool;
+
+ /// Refresh endpoint's state
+ fn trigger_state_refresh(&self) -> Result<(), Error> {
+ Ok(())
+ }
}
#[derive(Debug, Clone, Serialize, Deserialize)]
@@ -593,6 +598,20 @@ impl Bus {
Ok(())
}
+
+ /// Refresh all endpoints' internal state.
+ ///
+ /// 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(()) => info!("triggered state refresh for endpoint '{name}'"),
+ Err(e) => error!("could not trigger state refresh for endpoint '{name}': {e}"),
+ };
+ }
+ }
}
#[cfg(test)]
--
2.47.3
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH proxmox 6/7] notify (smtp): Add XOAUTH2 authentication support
2026-02-13 16:03 [PATCH cluster/docs/manager/proxmox{,-perl-rs,-widget-toolkit} 00/17] fix #7238: Add XOAUTH2 authentication support for SMTP notification targets Arthur Bied-Charreton
` (4 preceding siblings ...)
2026-02-13 16:04 ` [PATCH proxmox 5/7] notify (smtp): Add state handling logic Arthur Bied-Charreton
@ 2026-02-13 16:04 ` Arthur Bied-Charreton
2026-03-23 12:26 ` Lukas Wagner
2026-02-13 16:04 ` [PATCH proxmox 7/7] notify (smtp): Add logging and state-related error types Arthur Bied-Charreton
` (12 subsequent siblings)
18 siblings, 1 reply; 38+ messages in thread
From: Arthur Bied-Charreton @ 2026-02-13 16:04 UTC (permalink / raw)
To: pve-devel
Extend the transport building logic to authenticate via XOAUTH2 if
configured, and manage the related state updates.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
proxmox-notify/src/endpoints/smtp.rs | 81 +++++++++++++++++++++++-----
proxmox-notify/src/lib.rs | 4 +-
2 files changed, 70 insertions(+), 15 deletions(-)
diff --git a/proxmox-notify/src/endpoints/smtp.rs b/proxmox-notify/src/endpoints/smtp.rs
index 244799fd..4364bd11 100644
--- a/proxmox-notify/src/endpoints/smtp.rs
+++ b/proxmox-notify/src/endpoints/smtp.rs
@@ -202,7 +202,7 @@ pub struct SmtpPrivateConfig {
pub oauth2_client_secret: Option<String>,
}
-/// A sendmail notification endpoint.
+/// A SMTP notification endpoint.
pub struct SmtpEndpoint {
pub config: SmtpConfig,
pub private_config: SmtpPrivateConfig,
@@ -246,22 +246,79 @@ impl SmtpEndpoint {
}
}
+ /// Infer the auth method based on the presence of a password field in the private config.
+ ///
+ /// This is required for backwards compatibility for configs created before the `auth_method`
+ /// field was added, i.e., the presence of a password implicitly meant plain authentication
+ /// was to be used.
+ fn auth_method(&self) -> Option<SmtpAuthMethod> {
+ self.config.auth_method.or_else(|| {
+ if self.private_config.password.is_some() {
+ Some(SmtpAuthMethod::Plain)
+ } else {
+ None
+ }
+ })
+ }
+
+ /// Build an [`SmtpTransport`].
+ ///
+ /// If OAuth2 authentication is configured, this method will additionally load,
+ /// update and store the OAuth2-related state for this endpoint.
fn build_transport(&self, tls: Tls, port: u16) -> Result<SmtpTransport, Error> {
- let mut transport_builder = SmtpTransport::builder_dangerous(&self.config.server)
+ let transport_builder = SmtpTransport::builder_dangerous(&self.config.server)
.tls(tls)
.port(port)
.timeout(Some(Duration::from_secs(SMTP_TIMEOUT.into())));
- if let Some(username) = self.config.username.as_deref() {
- if let Some(password) = self.private_config.password.as_deref() {
- transport_builder = transport_builder.credentials((username, password).into());
- } else {
- return Err(Error::NotifyFailed(
- self.name().into(),
- Box::new(Error::Generic(
- "username is set but no password was provided".to_owned(),
- )),
- ));
+ let transport_builder = match &self.auth_method() {
+ None => transport_builder,
+ Some(SmtpAuthMethod::Plain) => match (
+ self.config.username.as_deref(),
+ self.private_config.password.as_deref(),
+ ) {
+ (Some(username), Some(password)) => {
+ transport_builder.credentials((username, password).into())
+ }
+ (Some(_), None) => {
+ return Err(Error::NotifyFailed(
+ self.name().into(),
+ Box::new(Error::Generic(
+ "username is set but no password was provided".to_owned(),
+ )),
+ ))
+ }
+ _ => transport_builder,
+ },
+ Some(method) => {
+ let state = State::load(self.name())?;
+
+ let refresh_token =
+ state
+ .oauth2_refresh_token
+ .clone()
+ .ok_or(Error::NotifyFailed(
+ self.name().into(),
+ Box::new(Error::Generic("no refresh token found".into())),
+ ))?;
+ let token_exchange_result = self.get_access_token(&refresh_token, method)?;
+
+ state
+ .set_oauth2_refresh_token(Some(
+ token_exchange_result
+ .refresh_token
+ .map(|t| t.into_secret())
+ .unwrap_or_else(|| refresh_token),
+ ))
+ .set_last_refreshed(proxmox_time::epoch_i64())
+ .store(self.name())?;
+
+ transport_builder
+ .credentials(Credentials::new(
+ self.config.from_address.to_owned(),
+ token_exchange_result.access_token.into_secret(),
+ ))
+ .authentication(vec![Mechanism::Xoauth2])
}
};
diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs
index c1a5e535..996393c2 100644
--- a/proxmox-notify/src/lib.rs
+++ b/proxmox-notify/src/lib.rs
@@ -557,9 +557,7 @@ impl Bus {
}
match endpoint.send(notification) {
- Ok(_) => {
- info!("notified via target `{name}`");
- }
+ Ok(_) => info!("notified via target `{name}`"),
Err(e) => {
// Only log on errors, do not propagate fail to the caller.
error!("could not notify via target `{name}`: {e}");
--
2.47.3
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH proxmox 7/7] notify (smtp): Add logging and state-related error types
2026-02-13 16:03 [PATCH cluster/docs/manager/proxmox{,-perl-rs,-widget-toolkit} 00/17] fix #7238: Add XOAUTH2 authentication support for SMTP notification targets Arthur Bied-Charreton
` (5 preceding siblings ...)
2026-02-13 16:04 ` [PATCH proxmox 6/7] notify (smtp): Add XOAUTH2 authentication support Arthur Bied-Charreton
@ 2026-02-13 16:04 ` Arthur Bied-Charreton
2026-03-23 12:26 ` Lukas Wagner
2026-02-13 16:04 ` [PATCH proxmox-perl-rs 1/1] notify (smtp): add oauth2 parameters to bindings Arthur Bied-Charreton
` (11 subsequent siblings)
18 siblings, 1 reply; 38+ messages in thread
From: Arthur Bied-Charreton @ 2026-02-13 16:04 UTC (permalink / raw)
To: pve-devel
Log load/store events in SMTP state management, and add error types to make errors clearer (previously used Config(De)?Serialization errors, which led to confusing error messages for state-related errors)
Also add logs for OAuth2 token exchange events.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
proxmox-notify/src/endpoints/smtp.rs | 12 +++++++++++
proxmox-notify/src/endpoints/smtp/state.rs | 25 +++++++++++++++-------
proxmox-notify/src/lib.rs | 12 +++++++++++
3 files changed, 41 insertions(+), 8 deletions(-)
diff --git a/proxmox-notify/src/endpoints/smtp.rs b/proxmox-notify/src/endpoints/smtp.rs
index 4364bd11..0ae1ac9f 100644
--- a/proxmox-notify/src/endpoints/smtp.rs
+++ b/proxmox-notify/src/endpoints/smtp.rs
@@ -12,6 +12,7 @@ use oauth2::{ClientId, ClientSecret, RefreshToken};
use proxmox_schema::api_types::COMMENT_SCHEMA;
use proxmox_schema::{api, Updater};
+use tracing::info;
use crate::context::context;
use crate::endpoints::common::mail;
@@ -301,6 +302,12 @@ impl SmtpEndpoint {
self.name().into(),
Box::new(Error::Generic("no refresh token found".into())),
))?;
+
+ info!(
+ "requesting OAuth2 access token for endpoint '{}'",
+ self.config.name
+ );
+
let token_exchange_result = self.get_access_token(&refresh_token, method)?;
state
@@ -313,6 +320,11 @@ impl SmtpEndpoint {
.set_last_refreshed(proxmox_time::epoch_i64())
.store(self.name())?;
+ info!(
+ "OAuth2 token exchange successful for endpoint '{}'",
+ self.config.name
+ );
+
transport_builder
.credentials(Credentials::new(
self.config.from_address.to_owned(),
diff --git a/proxmox-notify/src/endpoints/smtp/state.rs b/proxmox-notify/src/endpoints/smtp/state.rs
index 60bef590..45d0db93 100644
--- a/proxmox-notify/src/endpoints/smtp/state.rs
+++ b/proxmox-notify/src/endpoints/smtp/state.rs
@@ -1,4 +1,5 @@
use serde::{Deserialize, Serialize};
+use tracing::debug;
use crate::{context::context, Error};
@@ -25,13 +26,19 @@ impl State {
/// # Errors
/// An [`Error`] is returned if deserialization of the state object fails.
pub(crate) fn load(name: &str) -> Result<State, Error> {
- match proxmox_sys::fs::file_get_optional_contents(context().state_file_path(name))
- .map_err(|e| Error::ConfigDeserialization(e.into()))?
+ let path = context().state_file_path(name);
+
+ match proxmox_sys::fs::file_get_optional_contents(&path)
+ .map_err(|e| Error::StateRetrieval(path.to_owned(), e.into()))?
{
Some(bytes) => {
- serde_json::from_slice(&bytes).map_err(|e| Error::ConfigDeserialization(e.into()))
+ debug!("loaded state file for endpoint '{name}' from {path}");
+ serde_json::from_slice(&bytes).map_err(|e| Error::StateRetrieval(path, e.into()))
+ }
+ None => {
+ debug!("no existing state file found for endpoint '{name}' at {path}, creating empty state");
+ Ok(State::default())
}
- None => Ok(State::default()),
}
}
@@ -43,14 +50,16 @@ impl State {
let path = context().state_file_path(name);
let parent = std::path::Path::new(&path).parent().unwrap();
+ debug!("attempting to persist state for endpoint '{name}' at {path}");
+
proxmox_sys::fs::ensure_dir_exists(parent, &context().secret_create_options(), false)
- .map_err(|e| Error::ConfigSerialization(e.into()))?;
+ .map_err(|e| Error::StatePersistence(path.to_owned(), e.into()))?;
let s = serde_json::to_string_pretty(&self)
- .map_err(|e| Error::ConfigSerialization(e.into()))?;
+ .map_err(|e| Error::StatePersistence(path.to_owned(), e.into()))?;
- proxmox_sys::fs::replace_file(path, s.as_bytes(), context().secret_create_options(), true)
- .map_err(|e| Error::ConfigSerialization(e.into()))
+ proxmox_sys::fs::replace_file(&path, s.as_bytes(), context().secret_create_options(), true)
+ .map_err(|e| Error::StatePersistence(path, e.into()))
}
/// Set `last_refreshed`.
diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs
index 996393c2..37237fc0 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
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH proxmox-perl-rs 1/1] notify (smtp): add oauth2 parameters to bindings
2026-02-13 16:03 [PATCH cluster/docs/manager/proxmox{,-perl-rs,-widget-toolkit} 00/17] fix #7238: Add XOAUTH2 authentication support for SMTP notification targets Arthur Bied-Charreton
` (6 preceding siblings ...)
2026-02-13 16:04 ` [PATCH proxmox 7/7] notify (smtp): Add logging and state-related error types Arthur Bied-Charreton
@ 2026-02-13 16:04 ` Arthur Bied-Charreton
2026-03-23 12:26 ` Lukas Wagner
2026-02-13 16:04 ` [PATCH proxmox-widget-toolkit 1/2] utils: Add OAuth2 flow handlers Arthur Bied-Charreton
` (10 subsequent siblings)
18 siblings, 1 reply; 38+ messages in thread
From: Arthur Bied-Charreton @ 2026-02-13 16:04 UTC (permalink / raw)
To: pve-devel
Update SMTP bindings to take the Smtp(Private)?Config structs directly,
and additionally the OAuth2 refresh token.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
common/src/bindings/notify.rs | 82 ++++++++++++-----------------------
1 file changed, 28 insertions(+), 54 deletions(-)
diff --git a/common/src/bindings/notify.rs b/common/src/bindings/notify.rs
index 409270a..137cc79 100644
--- a/common/src/bindings/notify.rs
+++ b/common/src/bindings/notify.rs
@@ -12,7 +12,7 @@ pub mod proxmox_rs_notify {
use std::collections::HashMap;
use std::sync::Mutex;
- use anyhow::{Error, bail};
+ use anyhow::{bail, Error};
use serde_json::Value as JSONValue;
use perlmod::Value;
@@ -26,7 +26,7 @@ pub mod proxmox_rs_notify {
DeleteableSendmailProperty, SendmailConfig, SendmailConfigUpdater,
};
use proxmox_notify::endpoints::smtp::{
- DeleteableSmtpProperty, SmtpConfig, SmtpConfigUpdater, SmtpMode, SmtpPrivateConfig,
+ DeleteableSmtpProperty, SmtpConfig, SmtpConfigUpdater, SmtpPrivateConfig,
SmtpPrivateConfigUpdater,
};
use proxmox_notify::endpoints::webhook::{
@@ -36,7 +36,7 @@ pub mod proxmox_rs_notify {
CalendarMatcher, DeleteableMatcherProperty, FieldMatcher, MatchModeOperator, MatcherConfig,
MatcherConfigUpdater, SeverityMatcher,
};
- use proxmox_notify::{Config, Notification, Severity, api};
+ use proxmox_notify::{api, Config, Notification, Severity};
/// A notification catalog instance.
///
@@ -141,6 +141,19 @@ pub mod proxmox_rs_notify {
api::common::send(&config, ¬ification)
}
+ /// Method: Refresh the state for all endpoints.
+ ///
+ /// This iterates through all configured targets, refreshing their state if needed.
+ ///
+ /// See [`api::common::refresh_targets`]
+ #[export(serialize_error)]
+ pub fn trigger_state_refresh(
+ #[try_from_ref] this: &NotificationConfig,
+ ) -> Result<(), HttpError> {
+ let config = this.config.lock().unwrap();
+ api::common::trigger_state_refresh(&config)
+ }
+
/// Method: Get a list of all notification targets.
///
/// See [`api::get_targets`].
@@ -390,37 +403,16 @@ pub mod proxmox_rs_notify {
#[allow(clippy::too_many_arguments)]
pub fn add_smtp_endpoint(
#[try_from_ref] this: &NotificationConfig,
- name: String,
- server: String,
- port: Option<u16>,
- mode: Option<SmtpMode>,
- username: Option<String>,
- password: Option<String>,
- mailto: Option<Vec<String>>,
- mailto_user: Option<Vec<String>>,
- from_address: String,
- author: Option<String>,
- comment: Option<String>,
- disable: Option<bool>,
+ smtp_config: SmtpConfig,
+ smtp_private_config: SmtpPrivateConfig,
+ oauth2_refresh_token: Option<String>,
) -> Result<(), HttpError> {
let mut config = this.config.lock().unwrap();
api::smtp::add_endpoint(
&mut config,
- SmtpConfig {
- name: name.clone(),
- server,
- port,
- mode,
- username,
- mailto: mailto.unwrap_or_default(),
- mailto_user: mailto_user.unwrap_or_default(),
- from_address,
- author,
- comment,
- disable,
- origin: None,
- },
- SmtpPrivateConfig { name, password },
+ smtp_config,
+ smtp_private_config,
+ oauth2_refresh_token,
)
}
@@ -432,17 +424,9 @@ pub mod proxmox_rs_notify {
pub fn update_smtp_endpoint(
#[try_from_ref] this: &NotificationConfig,
name: &str,
- server: Option<String>,
- port: Option<u16>,
- mode: Option<SmtpMode>,
- username: Option<String>,
- password: Option<String>,
- mailto: Option<Vec<String>>,
- mailto_user: Option<Vec<String>>,
- from_address: Option<String>,
- author: Option<String>,
- comment: Option<String>,
- disable: Option<bool>,
+ smtp_config_updater: SmtpConfigUpdater,
+ smtp_private_config_updater: SmtpPrivateConfigUpdater,
+ oauth2_refresh_token: Option<String>,
delete: Option<Vec<DeleteableSmtpProperty>>,
digest: Option<&str>,
) -> Result<(), HttpError> {
@@ -452,19 +436,9 @@ pub mod proxmox_rs_notify {
api::smtp::update_endpoint(
&mut config,
name,
- SmtpConfigUpdater {
- server,
- port,
- mode,
- username,
- mailto,
- mailto_user,
- from_address,
- author,
- comment,
- disable,
- },
- SmtpPrivateConfigUpdater { password },
+ smtp_config_updater,
+ smtp_private_config_updater,
+ oauth2_refresh_token,
delete.as_deref(),
digest.as_deref(),
)
--
2.47.3
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH proxmox-widget-toolkit 1/2] utils: Add OAuth2 flow handlers
2026-02-13 16:03 [PATCH cluster/docs/manager/proxmox{,-perl-rs,-widget-toolkit} 00/17] fix #7238: Add XOAUTH2 authentication support for SMTP notification targets Arthur Bied-Charreton
` (7 preceding siblings ...)
2026-02-13 16:04 ` [PATCH proxmox-perl-rs 1/1] notify (smtp): add oauth2 parameters to bindings Arthur Bied-Charreton
@ 2026-02-13 16:04 ` Arthur Bied-Charreton
2026-03-23 12:26 ` Lukas Wagner
2026-02-13 16:04 ` [PATCH proxmox-widget-toolkit 2/2] notifications: Add opt-in OAuth2 support for SMTP targets Arthur Bied-Charreton
` (9 subsequent siblings)
18 siblings, 1 reply; 38+ messages in thread
From: Arthur Bied-Charreton @ 2026-02-13 16:04 UTC (permalink / raw)
To: pve-devel
Introduce the Proxmox.OAuth2 singleton supporting Google and Microsoft
OAuth2. The flow is handled by opening a new window with the
authorization URL, and expects to receive the resulting authorization
code from the redirect handler via a [BroadcastChannel].
[BroadcastChannel]
https://developer.mozilla.org/en-US/docs/Web/API/BroadcastChannel
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
src/Utils.js | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 88 insertions(+)
diff --git a/src/Utils.js b/src/Utils.js
index 5457ffa..5cbe9b6 100644
--- a/src/Utils.js
+++ b/src/Utils.js
@@ -1723,6 +1723,94 @@ Ext.define('Proxmox.Utils', {
},
});
+Ext.define('Proxmox.OAuth2', {
+ singleton: true,
+
+ handleGoogleFlow: function (clientId, clientSecret) {
+ return this._handleFlow({
+ clientId,
+ clientSecret,
+ authUrl: 'https://accounts.google.com/o/oauth2/v2/auth',
+ tokenUrl: 'https://oauth2.googleapis.com/token',
+ scope: 'https://mail.google.com',
+ extraAuthParams: {
+ access_type: 'offline',
+ prompt: 'consent',
+ },
+ });
+ },
+
+ handleMicrosoftFlow: function(clientId, clientSecret, tenantId) {
+ return this._handleFlow({
+ clientId,
+ clientSecret,
+ authUrl: `https://login.microsoftonline.com/${tenantId}/oauth2/v2.0/authorize`,
+ tokenUrl: `https://login.microsoftonline.com/${tenantId}/oauth2/v2.0/token`,
+ scope: 'https://outlook.office.com/SMTP.Send offline_access',
+ extraAuthParams: {
+ prompt: 'consent',
+ },
+ });
+ },
+
+ _handleFlow: function (config) {
+ return new Promise((resolve, reject) => {
+ let redirectUri = window.location.origin;
+ let channelName = `oauth2_${crypto.randomUUID()}`;
+ let state = encodeURIComponent(JSON.stringify({ channelName }));
+
+ let authParams = new URLSearchParams({
+ client_id: config.clientId,
+ response_type: 'code',
+ redirect_uri: redirectUri,
+ scope: config.scope,
+ state,
+ ...config.extraAuthParams,
+ });
+
+ let authUrl = `${config.authUrl}?${authParams}`;
+
+ // Opens OAuth2 authentication window. The app's redirect handler must
+ // extract the authorization code from the callback URL and send it via:
+ // new BroadcastChannel(state.channelName).postMessage({ code })
+ let channel = new BroadcastChannel(channelName);
+ let popup = window.open(authUrl);
+ if (!popup) {
+ reject(new Error('Could not open authentication window'));
+ return;
+ }
+
+ channel.addEventListener('message', async (event) => {
+ if (popup && !popup.closed) {
+ popup.close();
+ }
+ channel.close();
+
+ try {
+ let response = await fetch(config.tokenUrl, {
+ method: 'POST',
+ headers: {
+ 'Content-Type': 'application/x-www-form-urlencoded',
+ },
+ body: new URLSearchParams({
+ grant_type: 'authorization_code',
+ code: event.data.code,
+ client_id: config.clientId,
+ client_secret: config.clientSecret,
+ redirect_uri: redirectUri,
+ }),
+ });
+
+ let tokens = await response.json();
+ resolve(tokens.refresh_token);
+ } catch (error) {
+ reject(error);
+ }
+ });
+ })
+ }
+})
+
Ext.define('Proxmox.Async', {
singleton: true,
--
2.47.3
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH proxmox-widget-toolkit 2/2] notifications: Add opt-in OAuth2 support for SMTP targets
2026-02-13 16:03 [PATCH cluster/docs/manager/proxmox{,-perl-rs,-widget-toolkit} 00/17] fix #7238: Add XOAUTH2 authentication support for SMTP notification targets Arthur Bied-Charreton
` (8 preceding siblings ...)
2026-02-13 16:04 ` [PATCH proxmox-widget-toolkit 1/2] utils: Add OAuth2 flow handlers Arthur Bied-Charreton
@ 2026-02-13 16:04 ` Arthur Bied-Charreton
2026-03-23 12:26 ` Lukas Wagner
2026-02-13 16:04 ` [PATCH pve-manager 1/5] notifications: Add OAuth2 parameters to schema and add/update endpoints Arthur Bied-Charreton
` (8 subsequent siblings)
18 siblings, 1 reply; 38+ messages in thread
From: Arthur Bied-Charreton @ 2026-02-13 16:04 UTC (permalink / raw)
To: pve-devel
Add Google & Microsoft OAuth2 authentication methods to SMTP endpoint
config.
The enableOAuth2 pmxSmtpEditPanel config flag allows consumers to opt
into the new feature, so it can be gradually introduced into services.
When disabled, no changes are visible from the UI, and only 'None' and
'Username/Password' are shown as
authentication methods. The flag is passed from the schema config, as it
is done for defaultMailAuthor.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
src/panel/SmtpEditPanel.js | 200 +++++++++++++++++++++++++++++++--
src/window/EndpointEditBase.js | 1 +
2 files changed, 190 insertions(+), 11 deletions(-)
diff --git a/src/panel/SmtpEditPanel.js b/src/panel/SmtpEditPanel.js
index 37e4d51..d6b03e3 100644
--- a/src/panel/SmtpEditPanel.js
+++ b/src/panel/SmtpEditPanel.js
@@ -6,10 +6,24 @@ Ext.define('Proxmox.panel.SmtpEditPanel', {
type: 'smtp',
+ enableOAuth2: false,
+
+ initConfig: function (config) {
+ this.callParent(arguments);
+ this.getViewModel().set('enableOAuth2', config.enableOAuth2 || false);
+ return config;
+ },
+
viewModel: {
xtype: 'viewmodel',
data: {
+ enableOAuth2: false,
mode: 'tls',
+ authMethod: 'plain',
+ oAuth2ClientId: '',
+ oAuth2ClientSecret: '',
+ oAuth2TenantId: '',
+ oAuth2RefreshToken: '',
authentication: true,
originalAuthentication: true,
},
@@ -39,6 +53,30 @@ Ext.define('Proxmox.panel.SmtpEditPanel', {
return shouldShowUnchanged ? gettext('Unchanged') : '';
},
+ isPlainAuthentication: function (get) {
+ return get('authMethod') === 'plain';
+ },
+ isOAuth2Authentication: function (get) {
+ if (!get('enableOAuth2')) { return false };
+ let authMethod = get('authMethod');
+ return authMethod === 'google-oauth2' || authMethod === 'microsoft-oauth2';
+ },
+ isMicrosoftOAuth2Authentication: function (get) {
+ if (!get('enableOAuth2')) { return false };
+ return get('authMethod') === 'microsoft-oauth2';
+ },
+ enableAuthenticate: function (get) {
+ if (!get('enableOAuth2')) { return false };
+ let clientId = get('oAuth2ClientId');
+ let clientSecret = get('oAuth2ClientSecret');
+
+ if (get('authMethod') === 'microsoft-oauth2') {
+ let tenantId = get('oAuth2TenantId');
+ return clientId && clientId.trim() !== '' && clientSecret && clientSecret.trim() !== '' && tenantId && tenantId.trim() !== '';
+ } else {
+ return clientId && clientId.trim() !== '' && clientSecret && clientSecret.trim() !== '';
+ }
+ }
},
},
@@ -102,11 +140,25 @@ Ext.define('Proxmox.panel.SmtpEditPanel', {
],
column2: [
{
- xtype: 'proxmoxcheckbox',
- fieldLabel: gettext('Authenticate'),
- name: 'authentication',
- bind: {
- value: '{authentication}',
+ xtype: 'proxmoxKVComboBox',
+ fieldLabel: gettext('Authentication'),
+ name: 'auth-method',
+ comboItems: [
+ ['none', gettext('None')],
+ ['plain', gettext('Username/Password')],
+ ['google-oauth2', gettext('OAuth2 (Google)')],
+ ['microsoft-oauth2', gettext('OAuth2 (Microsoft)')],
+ ],
+ bind: '{authMethod}',
+ cbind: {
+ deleteEmpty: '{!isCreate}',
+ },
+ listeners: {
+ render: function () {
+ if (!this.up('pmxSmtpEditPanel').enableOAuth2) {
+ this.getStore().filter('key', /^(none|plain)$/);
+ }
+ },
},
},
{
@@ -118,7 +170,8 @@ Ext.define('Proxmox.panel.SmtpEditPanel', {
deleteEmpty: '{!isCreate}',
},
bind: {
- disabled: '{!authentication}',
+ hidden: '{!isPlainAuthentication}',
+ disabled: '{!isPlainAuthentication}',
},
},
{
@@ -130,10 +183,96 @@ Ext.define('Proxmox.panel.SmtpEditPanel', {
allowBlank: '{!isCreate}',
},
bind: {
- disabled: '{!authentication}',
+ hidden: '{!isPlainAuthentication}',
+ disabled: '{!isPlainAuthentication}',
emptyText: '{passwordEmptyText}',
},
},
+ {
+ xtype: 'proxmoxtextfield',
+ fieldLabel: gettext('Client ID'),
+ name: 'oauth2-client-id',
+ allowBlank: false,
+ bind: {
+ hidden: '{!isOAuth2Authentication}',
+ disabled: '{!isOAuth2Authentication}',
+ value: '{oAuth2ClientId}',
+ },
+ cbind: {
+ deleteEmpty: '{!isCreate}',
+ },
+ },
+ {
+ xtype: 'proxmoxtextfield',
+ inputType: 'password',
+ fieldLabel: gettext('Client Secret'),
+ name: 'oauth2-client-secret',
+ allowBlank: false,
+ bind: {
+ hidden: '{!isOAuth2Authentication}',
+ disabled: '{!isOAuth2Authentication}',
+ value: '{oAuth2ClientSecret}',
+ },
+ cbind: {
+ deleteEmpty: '{!isCreate}',
+ },
+ },
+ {
+ xtype: 'proxmoxtextfield',
+ fieldLabel: gettext('Tenant ID'),
+ name: 'oauth2-tenant-id',
+ allowBlank: false,
+ bind: {
+ hidden: '{!isMicrosoftOAuth2Authentication}',
+ disabled: '{!isMicrosoftOAuth2Authentication}',
+ value: '{oAuth2TenantId}',
+ },
+ cbind: {
+ deleteEmpty: '{!isCreate}',
+ },
+ },
+ {
+ xtype: 'button',
+ text: gettext('Authenticate'),
+ fieldLabel: gettext('Authenticate'),
+ handler: async function () {
+ let panel = this.up('pmxSmtpEditPanel');
+ let form = panel.up('form');
+ let values = form.getValues();
+
+ try {
+ let refreshToken = await panel.handleOAuth2Flow(values);
+ panel.getViewModel().set('oAuth2RefreshToken', refreshToken);
+ } catch (e) {
+ Ext.Msg.alert('Error', e);
+ }
+ },
+ bind: {
+ hidden: '{!isOAuth2Authentication}',
+ disabled: '{!enableAuthenticate}',
+ },
+ cbind: {
+ deleteEmpty: '{!isCreate}',
+ },
+ },
+ {
+ xtype: 'hiddenfield',
+ name: 'oauth2-refresh-token',
+ allowBlank: false,
+ bind: {
+ value: '{oAuth2RefreshToken}',
+ disabled: '{!isOAuth2Authentication}',
+ },
+ getErrors: function () {
+ if (this.disabled) {
+ return [];
+ }
+ if (!this.getValue()) {
+ return [''];
+ }
+ return [];
+ }
+ }
],
columnB: [
{
@@ -159,7 +298,6 @@ Ext.define('Proxmox.panel.SmtpEditPanel', {
},
},
],
-
advancedColumnB: [
{
xtype: 'proxmoxtextfield',
@@ -172,7 +310,15 @@ Ext.define('Proxmox.panel.SmtpEditPanel', {
},
},
],
+ handleOAuth2Flow: function (values) {
+ let authMethod = values['auth-method'];
+ if (authMethod === 'microsoft-oauth2') {
+ return Proxmox.OAuth2.handleMicrosoftFlow(values['oauth2-client-id'], values['oauth2-client-secret'], values['oauth2-tenant-id']);
+ } else if (authMethod === 'google-oauth2') {
+ return Proxmox.OAuth2.handleGoogleFlow(values['oauth2-client-id'], values['oauth2-client-secret']);
+ }
+ },
onGetValues: function (values) {
let me = this;
@@ -180,9 +326,30 @@ Ext.define('Proxmox.panel.SmtpEditPanel', {
values.mailto = values.mailto.split(/[\s,;]+/);
}
- if (!values.authentication && !me.isCreate) {
+ if (!this.enableOAuth2) {
+ delete values['auth-method'];
+ if (!values.authentication && !me.isCreate) {
+ Proxmox.Utils.assemble_field_data(values, { delete: 'username' });
+ Proxmox.Utils.assemble_field_data(values, { delete: 'password' });
+ }
+ } else if (values['auth-method'] === 'none' && !me.isCreate) {
+ delete values['auth-method'];
Proxmox.Utils.assemble_field_data(values, { delete: 'username' });
Proxmox.Utils.assemble_field_data(values, { delete: 'password' });
+ Proxmox.Utils.assemble_field_data(values, { delete: 'oauth2-client-id' });
+ Proxmox.Utils.assemble_field_data(values, { delete: 'oauth2-client-secret' });
+ Proxmox.Utils.assemble_field_data(values, { delete: 'oauth2-tenant-id' });
+ } else if (values['auth-method'] === 'plain' && !me.isCreate) {
+ Proxmox.Utils.assemble_field_data(values, { delete: 'oauth2-client-id' });
+ Proxmox.Utils.assemble_field_data(values, { delete: 'oauth2-client-secret' });
+ Proxmox.Utils.assemble_field_data(values, { delete: 'oauth2-tenant-id' });
+ } else if (values['auth-method'] === 'microsoft-oauth2' && !me.isCreate) {
+ Proxmox.Utils.assemble_field_data(values, { delete: 'username' });
+ Proxmox.Utils.assemble_field_data(values, { delete: 'password' });
+ } else if (values['auth-method'] === 'google-oauth2' && !me.isCreate) {
+ Proxmox.Utils.assemble_field_data(values, { delete: 'username' });
+ Proxmox.Utils.assemble_field_data(values, { delete: 'password' });
+ Proxmox.Utils.assemble_field_data(values, { delete: 'oauth2-tenant-id' });
}
if (values.enable) {
@@ -199,12 +366,23 @@ Ext.define('Proxmox.panel.SmtpEditPanel', {
return values;
},
-
onSetValues: function (values) {
let me = this;
- values.authentication = !!values.username;
values.enable = !values.disable;
+
+ if (values['auth-method'] === undefined && this.enableOAuth2) {
+ if (values['oauth2-tenant-id']) {
+ values['auth-method'] = 'microsoft-oauth2';
+ } else if (values['oauth2-client-id']) {
+ values['auth-method'] = 'google-oauth2';
+ } else if (values.username) {
+ values['auth-method'] = 'plain'
+ } else {
+ values['auth-method'] = 'none';
+ }
+ }
+
delete values.disable;
// Fix race condition in chromium-based browsers. Without this, the
diff --git a/src/window/EndpointEditBase.js b/src/window/EndpointEditBase.js
index 8c1bfc1..8df2016 100644
--- a/src/window/EndpointEditBase.js
+++ b/src/window/EndpointEditBase.js
@@ -47,6 +47,7 @@ Ext.define('Proxmox.window.EndpointEditBase', {
baseUrl: me.baseUrl,
type: me.type,
defaultMailAuthor: endpointConfig.defaultMailAuthor,
+ enableOAuth2: endpointConfig.enableOAuth2,
},
],
});
--
2.47.3
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH pve-manager 1/5] notifications: Add OAuth2 parameters to schema and add/update endpoints
2026-02-13 16:03 [PATCH cluster/docs/manager/proxmox{,-perl-rs,-widget-toolkit} 00/17] fix #7238: Add XOAUTH2 authentication support for SMTP notification targets Arthur Bied-Charreton
` (9 preceding siblings ...)
2026-02-13 16:04 ` [PATCH proxmox-widget-toolkit 2/2] notifications: Add opt-in OAuth2 support for SMTP targets Arthur Bied-Charreton
@ 2026-02-13 16:04 ` Arthur Bied-Charreton
2026-03-23 12:26 ` Lukas Wagner
2026-02-13 16:04 ` [PATCH pve-manager 2/5] notifications: Add trigger-state-refresh endpoint Arthur Bied-Charreton
` (7 subsequent siblings)
18 siblings, 1 reply; 38+ messages in thread
From: Arthur Bied-Charreton @ 2026-02-13 16:04 UTC (permalink / raw)
To: pve-devel
Add auth-method, as well as optional
oauth2-{client-id,client-secret,tenant-id,refresh-token} parameters to
prepare for OAuth2 support.
The auth-method parameter was previously implicit and inferred by
proxmox-notify based on the presence of a password. It is now made
explicit, however still kept optional and explicitly inferred in the
{update,create}_endpoint handlers to avoid breaking the API.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
PVE/API2/Cluster/Notifications.pm | 110 +++++++++++++++++++++++-------
1 file changed, 87 insertions(+), 23 deletions(-)
diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm
index 8b455227..e7acea49 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -941,6 +941,13 @@ my $smtp_properties = {
default => 'tls',
optional => 1,
},
+ 'auth-method' => {
+ description =>
+ 'Determine which authentication method shall be used for the connection.',
+ type => 'string',
+ enum => [qw(google-oauth2 microsoft-oauth2 plain none)],
+ optional => 1,
+ },
username => {
description => 'Username for SMTP authentication',
type => 'string',
@@ -951,6 +958,26 @@ my $smtp_properties = {
type => 'string',
optional => 1,
},
+ 'oauth2-client-id' => {
+ description => 'OAuth2 client ID',
+ type => 'string',
+ optional => 1,
+ },
+ 'oauth2-client-secret' => {
+ description => 'OAuth2 client secret',
+ type => 'string',
+ optional => 1,
+ },
+ 'oauth2-tenant-id' => {
+ description => 'OAuth2 tenant ID',
+ type => 'string',
+ optional => 1,
+ },
+ 'oauth2-refresh-token' => {
+ description => 'OAuth2 refresh token',
+ type => 'string',
+ optional => 1,
+ },
mailto => {
type => 'array',
items => {
@@ -1108,6 +1135,11 @@ __PACKAGE__->register_method({
my $mode = extract_param($param, 'mode');
my $username = extract_param($param, 'username');
my $password = extract_param($param, 'password');
+ my $auth_method = extract_param($param, 'auth-method');
+ my $oauth2_client_secret = extract_param($param, 'oauth2-client-secret');
+ my $oauth2_client_id = extract_param($param, 'oauth2-client-id');
+ my $oauth2_tenant_id = extract_param($param, 'oauth2-tenant-id');
+ my $oauth2_refresh_token = extract_param($param, 'oauth2-refresh-token');
my $mailto = extract_param($param, 'mailto');
my $mailto_user = extract_param($param, 'mailto-user');
my $from_address = extract_param($param, 'from-address');
@@ -1115,23 +1147,37 @@ __PACKAGE__->register_method({
my $comment = extract_param($param, 'comment');
my $disable = extract_param($param, 'disable');
+ if (!defined $auth_method) {
+ $auth_method = defined($password) ? 'plain' : 'none';
+ }
+
eval {
PVE::Notify::lock_config(sub {
my $config = PVE::Notify::read_config();
$config->add_smtp_endpoint(
- $name,
- $server,
- $port,
- $mode,
- $username,
- $password,
- $mailto,
- $mailto_user,
- $from_address,
- $author,
- $comment,
- $disable,
+ {
+ name => $name,
+ server => $server,
+ port => $port,
+ mode => $mode,
+ username => $username,
+ 'auth-method' => $auth_method,
+ 'oauth2-client-id' => $oauth2_client_id,
+ 'oauth2-tenant-id' => $oauth2_tenant_id,
+ mailto => defined($mailto) ? $mailto : [],
+ 'mailto-user' => defined($mailto_user) ? $mailto_user : [],
+ 'from-address' => $from_address,
+ author => $author,
+ comment => $comment,
+ disable => $disable,
+ },
+ {
+ name => $name,
+ password => $password,
+ 'oauth2-client-secret' => $oauth2_client_secret,
+ },
+ $oauth2_refresh_token,
);
PVE::Notify::write_config($config);
@@ -1187,6 +1233,11 @@ __PACKAGE__->register_method({
my $mode = extract_param($param, 'mode');
my $username = extract_param($param, 'username');
my $password = extract_param($param, 'password');
+ my $auth_method = extract_param($param, 'auth-method');
+ my $oauth2_client_secret = extract_param($param, 'oauth2-client-secret');
+ my $oauth2_client_id = extract_param($param, 'oauth2-client-id');
+ my $oauth2_tenant_id = extract_param($param, 'oauth2-tenant-id');
+ my $oauth2_refresh_token = extract_param($param, 'oauth2-refresh-token');
my $mailto = extract_param($param, 'mailto');
my $mailto_user = extract_param($param, 'mailto-user');
my $from_address = extract_param($param, 'from-address');
@@ -1197,23 +1248,36 @@ __PACKAGE__->register_method({
my $delete = extract_param($param, 'delete');
my $digest = extract_param($param, 'digest');
+ if (!defined $auth_method) {
+ $auth_method = defined($password) ? 'plain' : 'none';
+ }
+
eval {
PVE::Notify::lock_config(sub {
my $config = PVE::Notify::read_config();
$config->update_smtp_endpoint(
$name,
- $server,
- $port,
- $mode,
- $username,
- $password,
- $mailto,
- $mailto_user,
- $from_address,
- $author,
- $comment,
- $disable,
+ {
+ server => $server,
+ port => $port,
+ mode => $mode,
+ username => $username,
+ 'auth-method' => $auth_method,
+ 'oauth2-client-id' => $oauth2_client_id,
+ 'oauth2-tenant-id' => $oauth2_tenant_id,
+ mailto => defined($mailto) ? $mailto : [],
+ 'mailto-user' => defined($mailto_user) ? $mailto_user : [],
+ 'from-address' => $from_address,
+ author => $author,
+ comment => $comment,
+ disable => $disable,
+ },
+ {
+ password => $password,
+ 'oauth2-client-secret' => $oauth2_client_secret,
+ },
+ $oauth2_refresh_token,
$delete,
$digest,
);
--
2.47.3
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH pve-manager 2/5] notifications: Add trigger-state-refresh endpoint
2026-02-13 16:03 [PATCH cluster/docs/manager/proxmox{,-perl-rs,-widget-toolkit} 00/17] fix #7238: Add XOAUTH2 authentication support for SMTP notification targets Arthur Bied-Charreton
` (10 preceding siblings ...)
2026-02-13 16:04 ` [PATCH pve-manager 1/5] notifications: Add OAuth2 parameters to schema and add/update endpoints Arthur Bied-Charreton
@ 2026-02-13 16:04 ` Arthur Bied-Charreton
2026-03-23 12:26 ` Lukas Wagner
2026-02-13 16:04 ` [PATCH pve-manager 3/5] notifications: Trigger notification target refresh in pveupdate Arthur Bied-Charreton
` (6 subsequent siblings)
18 siblings, 1 reply; 38+ messages in thread
From: Arthur Bied-Charreton @ 2026-02-13 16:04 UTC (permalink / raw)
To: pve-devel
This endpoint allows triggering a refresh of the notification targets'
state, e.g., to prevent OAuth2 refresh tokens from expiring.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
PVE/API2/Cluster/Notifications.pm | 34 +++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm
index e7acea49..127d6710 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -321,6 +321,40 @@ __PACKAGE__->register_method({
},
});
+__PACKAGE__->register_method({
+ name => "trigger_state_refresh",
+ path => 'trigger-state-refresh',
+ protected => 1,
+ method => 'POST',
+ description => 'Refresh state for all targets',
+ permissions => {
+ check => [
+ 'and',
+ ['perm', '/mapping/notifications', ['Mapping.Modify']],
+ [
+ 'or',
+ ['perm', '/', ['Sys.Audit', 'Sys.Modify']],
+ ['perm', '/', ['Sys.AccessNetwork']],
+ ],
+ ],
+ },
+ parameters => {
+ additionalProperties => 0,
+ properties => {},
+ },
+ returns => { type => 'null' },
+ code => sub {
+ eval {
+ my $config = PVE::Notify::read_config();
+ $config->trigger_state_refresh();
+ };
+
+ raise_api_error($@) if $@;
+
+ return;
+ },
+});
+
__PACKAGE__->register_method({
name => 'test_target',
path => 'targets/{name}/test',
--
2.47.3
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH pve-manager 3/5] notifications: Trigger notification target refresh in pveupdate
2026-02-13 16:03 [PATCH cluster/docs/manager/proxmox{,-perl-rs,-widget-toolkit} 00/17] fix #7238: Add XOAUTH2 authentication support for SMTP notification targets Arthur Bied-Charreton
` (11 preceding siblings ...)
2026-02-13 16:04 ` [PATCH pve-manager 2/5] notifications: Add trigger-state-refresh endpoint Arthur Bied-Charreton
@ 2026-02-13 16:04 ` Arthur Bied-Charreton
2026-02-13 16:04 ` [PATCH pve-manager 4/5] notifications: Handle OAuth2 callback in login handler Arthur Bied-Charreton
` (5 subsequent siblings)
18 siblings, 0 replies; 38+ messages in thread
From: Arthur Bied-Charreton @ 2026-02-13 16:04 UTC (permalink / raw)
To: pve-devel
Avoid OAuth2 refresh token expiry by forcing a notification target
refresh in PVE's daily-update job.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
bin/pveupdate | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/bin/pveupdate b/bin/pveupdate
index b1960c35..1b32b382 100755
--- a/bin/pveupdate
+++ b/bin/pveupdate
@@ -195,4 +195,13 @@ sub cleanup_tasks {
cleanup_tasks();
+eval {
+ my $config = PVE::Notify::read_config();
+ # Refresh internal state for notification targets
+ $config->trigger_state_refresh();
+};
+if (my $err = $@) {
+ syslog('err', "refresh notification targets failed: $err");
+}
+
exit(0);
--
2.47.3
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH pve-manager 4/5] notifications: Handle OAuth2 callback in login handler
2026-02-13 16:03 [PATCH cluster/docs/manager/proxmox{,-perl-rs,-widget-toolkit} 00/17] fix #7238: Add XOAUTH2 authentication support for SMTP notification targets Arthur Bied-Charreton
` (12 preceding siblings ...)
2026-02-13 16:04 ` [PATCH pve-manager 3/5] notifications: Trigger notification target refresh in pveupdate Arthur Bied-Charreton
@ 2026-02-13 16:04 ` Arthur Bied-Charreton
2026-02-13 16:04 ` [PATCH pve-manager 5/5] notifications: Opt into OAuth2 authentication Arthur Bied-Charreton
` (4 subsequent siblings)
18 siblings, 0 replies; 38+ messages in thread
From: Arthur Bied-Charreton @ 2026-02-13 16:04 UTC (permalink / raw)
To: pve-devel
The OAuth2 flow redirects to the service's origin
(window.location.origin) after successful authentication.
The callback handler infers whether the login was triggered as the
result of an OAuth2 redirect based on the presence of the code, scope,
and state URL parameters. It then communicates the authentication
results back to the parent window, which is responsible for closing it.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
www/manager6/Workspace.js | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/www/manager6/Workspace.js b/www/manager6/Workspace.js
index b8061c2a..cb15b9d3 100644
--- a/www/manager6/Workspace.js
+++ b/www/manager6/Workspace.js
@@ -150,9 +150,29 @@ Ext.define('PVE.StdWorkspace', {
me.down('pveResourceTree').selectById(nodeid);
},
+ handleOauth2Callback: function (params) {
+ let code = params.get('code');
+ let scope = params.get('scope');
+ let state = params.get('state');
+
+ // If true, this window was opened by the OAuth2 button handler from the
+ // SMTP notification targets edit panel.
+ //
+ // Since we got here through a redirect, this window is not script-closable,
+ // and we rely on the parent window to close it in its broadcast channel's
+ // message handler.
+ if (code && scope && state) {
+ let { channelName } = JSON.parse(decodeURIComponent(state));
+ let bc = new BroadcastChannel(channelName);
+ bc.postMessage({ code, scope });
+ }
+ },
+
onLogin: function (loginData) {
let me = this;
+ me.handleOauth2Callback(new URLSearchParams(window.location.search));
+
me.updateUserInfo();
if (loginData) {
--
2.47.3
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH pve-manager 5/5] notifications: Opt into OAuth2 authentication
2026-02-13 16:03 [PATCH cluster/docs/manager/proxmox{,-perl-rs,-widget-toolkit} 00/17] fix #7238: Add XOAUTH2 authentication support for SMTP notification targets Arthur Bied-Charreton
` (13 preceding siblings ...)
2026-02-13 16:04 ` [PATCH pve-manager 4/5] notifications: Handle OAuth2 callback in login handler Arthur Bied-Charreton
@ 2026-02-13 16:04 ` Arthur Bied-Charreton
2026-02-13 16:04 ` [PATCH pve-cluster 1/1] notifications: Add refresh_targets subroutine to PVE::Notify Arthur Bied-Charreton
` (3 subsequent siblings)
18 siblings, 0 replies; 38+ messages in thread
From: Arthur Bied-Charreton @ 2026-02-13 16:04 UTC (permalink / raw)
To: pve-devel
The OAuth2 authentication for SMTP is made opt-in in
proxmox-widget-toolkit to allow introducing it repo-by-repo.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
www/manager6/Utils.js | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 2992f655..9735184e 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -2185,5 +2185,15 @@ Ext.define('PVE.Utils', {
replication: gettext('Replication job notifications'),
fencing: gettext('Node fencing notifications'),
});
+
+ Proxmox.Schema.overrideEndpointTypes({
+ smtp: {
+ name: 'SMTP',
+ ipanel: 'pmxSmtpEditPanel',
+ iconCls: 'fa-envelope-o',
+ defaultMailAuthor: 'Proxmox VE',
+ enableOAuth2: true,
+ },
+ });
},
});
--
2.47.3
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH pve-cluster 1/1] notifications: Add refresh_targets subroutine to PVE::Notify
2026-02-13 16:03 [PATCH cluster/docs/manager/proxmox{,-perl-rs,-widget-toolkit} 00/17] fix #7238: Add XOAUTH2 authentication support for SMTP notification targets Arthur Bied-Charreton
` (14 preceding siblings ...)
2026-02-13 16:04 ` [PATCH pve-manager 5/5] notifications: Opt into OAuth2 authentication Arthur Bied-Charreton
@ 2026-02-13 16:04 ` Arthur Bied-Charreton
2026-03-23 12:26 ` Lukas Wagner
2026-02-13 16:04 ` [PATCH pve-docs 1/1] notifications: Add section about OAuth2 to SMTP targets docs Arthur Bied-Charreton
` (2 subsequent siblings)
18 siblings, 1 reply; 38+ messages in thread
From: Arthur Bied-Charreton @ 2026-02-13 16:04 UTC (permalink / raw)
To: pve-devel
Expose API to allow refreshing all notification targets' state from the
PVE backend, as well as from pveupdate.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
src/PVE/Notify.pm | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/PVE/Notify.pm b/src/PVE/Notify.pm
index b44b5b3..ebb2311 100644
--- a/src/PVE/Notify.pm
+++ b/src/PVE/Notify.pm
@@ -103,6 +103,12 @@ sub error {
);
}
+sub trigger_state_refresh {
+ my ($config) = @_;
+ $config = read_config() if !defined($config);
+ $config->trigger_state_refresh();
+}
+
sub check_may_use_target {
my ($target, $rpcenv) = @_;
my $user = $rpcenv->get_user();
--
2.47.3
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH pve-docs 1/1] notifications: Add section about OAuth2 to SMTP targets docs
2026-02-13 16:03 [PATCH cluster/docs/manager/proxmox{,-perl-rs,-widget-toolkit} 00/17] fix #7238: Add XOAUTH2 authentication support for SMTP notification targets Arthur Bied-Charreton
` (15 preceding siblings ...)
2026-02-13 16:04 ` [PATCH pve-cluster 1/1] notifications: Add refresh_targets subroutine to PVE::Notify Arthur Bied-Charreton
@ 2026-02-13 16:04 ` Arthur Bied-Charreton
2026-03-23 12:25 ` [PATCH cluster/docs/manager/proxmox{,-perl-rs,-widget-toolkit} 00/17] fix #7238: Add XOAUTH2 authentication support for SMTP notification targets Lukas Wagner
2026-03-25 13:16 ` superseded: " Arthur Bied-Charreton
18 siblings, 0 replies; 38+ messages in thread
From: Arthur Bied-Charreton @ 2026-02-13 16:04 UTC (permalink / raw)
To: pve-devel
Document the new config entries and the requirements to use XOAUTH2 for SMTP
notification targets, and add notes/warnings to communicate that:
1. User intervention is required for initial OAuth2 target setup, and
2. Microsoft OAuth2 apps *must not* be configured as SPAs by the user,
since it would prevent PVE from automatically extending the refresh
token's lifetime
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
notifications.adoc | 99 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 99 insertions(+)
diff --git a/notifications.adoc b/notifications.adoc
index 801b327..1ff0552 100644
--- a/notifications.adoc
+++ b/notifications.adoc
@@ -108,10 +108,17 @@ The configuration for SMTP target plugins has the following options:
* `from-address`: Sets the From-address of the email. SMTP relays might require
that this address is owned by the user in order to avoid spoofing. The `From`
header in the email will be set to `$author <$from-address>`.
+* `auth-method`: Sets the authentication method (`plain`, `google-oauth2` or
+ `microsoft-oauth2`).
* `username`: Username to use during authentication. If no username is set,
no authentication will be performed. The PLAIN and LOGIN authentication
methods are supported.
* `password`: Password to use when authenticating.
+* `oauth2-client-id`: Client ID for the OAuth2 application, if applicable.
+* `oauth2-client-secret`: Client secret for the OAuth2 application, if
+ applicable.
+* `oauth2-tenant-id`: Tenant ID for the OAuth2 application, if applicable.
+ Only required for Microsoft OAuth2.
* `mode`: Sets the encryption mode (`insecure`, `starttls` or `tls`). Defaults
to `tls`.
* `server`: Address/IP of the SMTP relay.
@@ -140,6 +147,98 @@ smtp: example
password somepassword
----
+[[notification_targets_smtp_oauth2]]
+==== OAuth2 Authentication
+
+{pve} supports OAuth2 authentication for SMTP targets via the XOAUTH2
+mechanism. This is currently available for Google and Microsoft mail providers.
+
+===== Creating an OAuth2 Application
+
+Before configuring OAuth2 in {pve}, you must register an OAuth2 application
+with your mail provider:
+
+* https://developers.google.com/identity/protocols/oauth2/web-server[Google]
+* https://learn.microsoft.com/en-us/entra/identity-platform/quickstart-register-app[Microsoft Entra ID]
+
+Choose *Web application* as application type.
+
+During registration, add a redirect URI pointing to the {pve} web interface
+URL from which you will perform the authorization flow, for example:
+
+* `https://pve1.yourdomain.com:8006`
+* `https://localhost:8006`
+
+You can add multiple redirect URIs to allow the authorization flow to work from
+any cluster node.
+
+NOTE: Google does not allow bare IP addresses as redirect URIs. See
+<<notification_targets_smtp_oauth2_google,Google-specific setup>> below for a
+workaround.
+
+===== Configuring OAuth2 in {pve}
+
+In the web UI, open the notification target's edit panel and select
+`OAuth2 (Google)` or `OAuth2 (Microsoft)` as the authentication method. Fill in
+the client ID and secret. For Microsoft, also fill in the tenant ID.
+
+Click *Authenticate*. This opens a new window where you can sign in with your
+mail provider and grant the requested permissions. On success, a refresh token
+is obtained and stored.
+
+Token refresh happens automatically, at least once every 24 hours. If the token
+expires due to extended cluster downtime or is revoked, you will need to
+re-authenticate: open the notification target's edit panel, fill in your client
+secret, and click *Authenticate* again.
+
+NOTE: OAuth2 cannot be configured through direct configuration file editing. Use
+the web interface, or alternatively `pvesh`, to configure OAuth2 targets. Note
+that when using `pvesh`, you are responsible for providing the initial refresh
+token.
+
+----
+pvesh create /cluster/notifications/endpoints/smtp \
+ --name oauth2-smtp \
+ --server smtp.example.com \
+ --from-address from@example.com \
+ --mailto-user root@pam \
+ --auth-method google-oauth2 \
+ --oauth2-client-id <client ID> \
+ --oauth2-client-secret <client secret> \
+ --oauth2-refresh-token <refresh token>
+----
+
+For Microsoft, use `--auth-method microsoft-oauth2` and add
+`--oauth2-tenant-id <tenant ID>`.
+
+[[notification_targets_smtp_oauth2_google]]
+===== Google
+
+Google does not allow bare IP addresses as redirect URIs. To work around this,
+add an entry to `/etc/hosts` *on the machine where your browser is running*,
+i.e., your local workstation.
+
+----
+# Replace <IP> with the IP address of your PVE node
+<IP> local.oauth2-redirect.com
+----
+
+You can now register `https://local.oauth2-redirect.com:8006` as a redirect
+URI in your Google OAuth2 application, and use that same URL in the browser
+when accessing the {pve} web interface to perform the authorization flow.
+
+[[notification_targets_smtp_oauth2_microsoft]]
+===== Microsoft
+
+WARNING: For Microsoft, the application must *not* be registered as a
+Single-Page Application (SPA). {pve} requires long-lived refresh tokens, and
+Microsoft does not allow extending the lifetime of refresh tokens granted for
+SPAs.
+
+Register your OAuth2 application as a standard *Web* application in the Entra
+admin center. In addition to the client ID and secret, you will also need the
+*tenant ID* from your application registration.
+
[[notification_targets_gotify]]
Gotify
~~~~~~
--
2.47.3
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH cluster/docs/manager/proxmox{,-perl-rs,-widget-toolkit} 00/17] fix #7238: Add XOAUTH2 authentication support for SMTP notification targets
2026-02-13 16:03 [PATCH cluster/docs/manager/proxmox{,-perl-rs,-widget-toolkit} 00/17] fix #7238: Add XOAUTH2 authentication support for SMTP notification targets Arthur Bied-Charreton
` (16 preceding siblings ...)
2026-02-13 16:04 ` [PATCH pve-docs 1/1] notifications: Add section about OAuth2 to SMTP targets docs Arthur Bied-Charreton
@ 2026-03-23 12:25 ` Lukas Wagner
2026-03-25 13:16 ` superseded: " Arthur Bied-Charreton
18 siblings, 0 replies; 38+ messages in thread
From: Lukas Wagner @ 2026-03-23 12:25 UTC (permalink / raw)
To: Arthur Bied-Charreton, pve-devel
On Fri Feb 13, 2026 at 5:03 PM CET, Arthur Bied-Charreton wrote:
> This series adds XOAUTH2 support for SMTP notification targets, motivated by Microsoft's
> upcoming deprecation of basic authentication for SMTP [0]. Google and Microsoft are supported as
> OAuth2 providers.
>
> OAuth2 refresh tokens may need to be rotated, therefore they are treated as state, not config.
> They are persisted in separate JSON files and managed entirely from the Rust side. Each endpoint
> has its own state file to avoid having to lock the entire file system everytime a state update
> is made.
>
> The oauth2 crate is used with a local ureq backend (newtype over ureq::Agent). oauth2's ureq
> feature is currently patched out in Debian due to a ureq 2/3 version mismatch (oauth2 still
> depends on ureq 2.x, and Debian only packages 3.x).
>
> Since tokens that are not used for extended periods of time may expire, token refresh is triggered
> proactively via pveupdate, additionally to every time a notification is sent, to prevent
> unused endpoints' tokens from expiring.
>
> The UI part of the OAuth2 flow is made opt-in in order to allow adding it only to PVE, and later
> to PBS in a follow-up series.
>
> This series requires the following version requirement bumps:
> * pve-manager requires bumped proxmox-widget-toolkit and proxmox-perl-rs
> * proxmox-perl-rs requires bumped proxmox-notify
>
> Known issues:
> - Microsoft OAuth2 support is untested (no test tenant, somehow impossible to create a free test
> account). It is implemented by following the Microsoft Entra ID docs, but there might be something
> I overlooked. I would highly appreciate it if someone was able to test it with their account.
>
Tested this with Gmail, seems to work fine!
Tested-by: Lukas Wagner <l.wagner@proxmox.com>
Very nice work overall!
I also verified that the feature-flag opt-in approach for the GUI works.
I installed the widget-toolkit .deb on a PBS installation, existing SMTP
with plain auth continued to work fine there.
See the individual patches for some suggestions for the next version,
but this is already in quite good shape, I think!
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH proxmox 2/7] notify (smtp): Introduce state module
2026-02-13 16:04 ` [PATCH proxmox 2/7] notify (smtp): Introduce state module Arthur Bied-Charreton
@ 2026-03-23 12:26 ` Lukas Wagner
2026-03-23 16:32 ` Arthur Bied-Charreton
2026-03-24 8:50 ` Arthur Bied-Charreton
0 siblings, 2 replies; 38+ messages in thread
From: Lukas Wagner @ 2026-03-23 12:26 UTC (permalink / raw)
To: Arthur Bied-Charreton, pve-devel
Hi Arthur!
Generally we'd write the commit subject rather like this:
"notify: smtp: ...."
instead of
"notify (smtp): ..."
(but not a big deal IMO, just mentioning it)
On Fri Feb 13, 2026 at 5:04 PM CET, Arthur Bied-Charreton wrote:
> The state module exports a new struct with associated functionality for
> loading, updating, and persisting the state for SMTP endpoints with
> OAuth2 configured as authentication method.
>
> The path to the state files, as well as their create options, are
> retrieved through new Context methods to allow portability between PVE
> and PBS.
>
> Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> ---
> proxmox-notify/src/context/mod.rs | 6 ++
> proxmox-notify/src/context/pbs.rs | 8 +++
> proxmox-notify/src/context/pve.rs | 8 +++
> proxmox-notify/src/context/test.rs | 8 +++
> proxmox-notify/src/endpoints/smtp.rs | 3 +
> proxmox-notify/src/endpoints/smtp/state.rs | 67 ++++++++++++++++++++++
> 6 files changed, 100 insertions(+)
> create mode 100644 proxmox-notify/src/endpoints/smtp/state.rs
>
> diff --git a/proxmox-notify/src/context/mod.rs b/proxmox-notify/src/context/mod.rs
> index 8b6e2c43..492442f9 100644
> --- a/proxmox-notify/src/context/mod.rs
> +++ b/proxmox-notify/src/context/mod.rs
> @@ -1,6 +1,8 @@
> use std::fmt::Debug;
> use std::sync::Mutex;
>
> +use proxmox_sys::fs::CreateOptions;
> +
> use crate::renderer::TemplateSource;
> use crate::Error;
>
> @@ -32,6 +34,10 @@ pub trait Context: Send + Sync + Debug {
> namespace: Option<&str>,
> source: TemplateSource,
> ) -> Result<Option<String>, Error>;
> + /// Return the path to the state file for this context.
> + fn state_file_path(&self, name: &str) -> String;
I think this should rather return a PathBuf. And rename `name` ->
`endpoint_name`?
> + /// Create options to be used when writing files containing secrets.
> + fn secret_create_options(&self) -> CreateOptions;
Due to CreateOptions being part of the `Context` trait now, we need to
pull in proxmox-sys unconditionally:
diff --git i/proxmox-notify/Cargo.toml w/proxmox-notify/Cargo.toml
index d816c695..35d5b8cb 100644
--- i/proxmox-notify/Cargo.toml
+++ w/proxmox-notify/Cargo.toml
@@ -36,17 +36,15 @@ 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"]
-smtp = ["dep:lettre", "dep:oauth2", "dep:ureq", "dep:http", "dep:proxmox-sys"]
+smtp = ["dep:lettre", "dep:oauth2", "dep:ureq", "dep:http"]
webhook = ["dep:http", "dep:percent-encoding", "dep:proxmox-base64", "dep:proxmox-http"]
> }
>
> #[cfg(not(test))]
> diff --git a/proxmox-notify/src/context/pbs.rs b/proxmox-notify/src/context/pbs.rs
> index 3e5da59c..4f93b45d 100644
> --- a/proxmox-notify/src/context/pbs.rs
> +++ b/proxmox-notify/src/context/pbs.rs
> @@ -125,6 +125,14 @@ impl Context for PBSContext {
> .map_err(|err| Error::Generic(format!("could not load template: {err}")))?;
> Ok(template_string)
> }
> +
> + fn state_file_path(&self, name: &str) -> String {
> + format!("/var/lib/proxmox-backup/priv/notifications/{name}.json")
> + }
> +
> + fn secret_create_options(&self) -> proxmox_sys::fs::CreateOptions {
> + proxmox_sys::fs::CreateOptions::new().perm(nix::sys::stat::Mode::from_bits_truncate(0o600))
(1) In the trait impl, you could get the appropriate CreateOptions via
proxmox-product-config...
> + }
> }
>
> #[cfg(test)]
> diff --git a/proxmox-notify/src/context/pve.rs b/proxmox-notify/src/context/pve.rs
> index a97cce26..e30f7b49 100644
> --- a/proxmox-notify/src/context/pve.rs
> +++ b/proxmox-notify/src/context/pve.rs
> @@ -74,6 +74,14 @@ impl Context for PVEContext {
> .map_err(|err| Error::Generic(format!("could not load template: {err}")))?;
> Ok(template_string)
> }
> +
> + fn state_file_path(&self, name: &str) -> String {
> + format!("/etc/pve/priv/notifications/{name}.json")
I think `state` should be somewhere in the path -- so maybe one of
- priv/notification-endpoint-state/{name}.json
- priv/notification-state/{name}.json
- priv/notifications/state-{name}.json
> + }
> +
> + fn secret_create_options(&self) -> proxmox_sys::fs::CreateOptions {
> + proxmox_sys::fs::CreateOptions::new().perm(nix::sys::stat::Mode::from_bits_truncate(0o600))
(2) ... here as well, but you probably need to initialize
proxmox-product-config in the proxmox-perl-rs bindings with the correct
UIDs.
> + }
> }
>
> pub static PVE_CONTEXT: PVEContext = PVEContext;
> diff --git a/proxmox-notify/src/context/test.rs b/proxmox-notify/src/context/test.rs
> index 2c236b4c..7e29d36a 100644
> --- a/proxmox-notify/src/context/test.rs
> +++ b/proxmox-notify/src/context/test.rs
> @@ -40,4 +40,12 @@ impl Context for TestContext {
> ) -> Result<Option<String>, Error> {
> Ok(Some(String::new()))
> }
> +
> + fn state_file_path(&self, name: &str) -> String {
> + format!("/tmp/notifications/{name}.json")
> + }
> +
> + fn secret_create_options(&self) -> proxmox_sys::fs::CreateOptions {
> + proxmox_sys::fs::CreateOptions::new().perm(nix::sys::stat::Mode::from_bits_truncate(0o755))
> + }
> }
> diff --git a/proxmox-notify/src/endpoints/smtp.rs b/proxmox-notify/src/endpoints/smtp.rs
> index 277b70f4..699ed1c6 100644
> --- a/proxmox-notify/src/endpoints/smtp.rs
> +++ b/proxmox-notify/src/endpoints/smtp.rs
> @@ -23,8 +23,11 @@ const SMTP_SUBMISSION_STARTTLS_PORT: u16 = 587;
> const SMTP_SUBMISSION_TLS_PORT: u16 = 465;
> const SMTP_TIMEOUT: u16 = 5;
>
> +mod state;
> mod xoauth2;
>
> +pub(crate) use state::State;
> +
> #[api]
> #[derive(Debug, Serialize, Deserialize, Default, Clone, Copy)]
> #[serde(rename_all = "kebab-case")]
> diff --git a/proxmox-notify/src/endpoints/smtp/state.rs b/proxmox-notify/src/endpoints/smtp/state.rs
> new file mode 100644
> index 00000000..60bef590
> --- /dev/null
> +++ b/proxmox-notify/src/endpoints/smtp/state.rs
> @@ -0,0 +1,67 @@
> +use serde::{Deserialize, Serialize};
> +
> +use crate::{context::context, Error};
> +
> +#[derive(Serialize, Deserialize, Clone, Debug, Default)]
> +#[serde(rename_all = "kebab-case")]
> +pub(crate) struct State {
> + #[serde(skip_serializing_if = "Option::is_none")]
> + pub oauth2_refresh_token: Option<String>,
> + pub last_refreshed: i64,
> +}
> +
> +impl State {
> + /// Instantiate a new [`State`].
> + pub(crate) fn new(oauth2_refresh_token: Option<String>) -> Self {
> + Self {
> + oauth2_refresh_token,
> + last_refreshed: proxmox_time::epoch_i64(),
> + }
> + }
> +
> + /// Load the state for the endpoint identified by `name`, instantiating a default object
> + /// if no state exists.
> + ///
> + /// # Errors
> + /// An [`Error`] is returned if deserialization of the state object fails.
> + pub(crate) fn load(name: &str) -> Result<State, Error> {
> + match proxmox_sys::fs::file_get_optional_contents(context().state_file_path(name))
> + .map_err(|e| Error::ConfigDeserialization(e.into()))?
> + {
> + Some(bytes) => {
> + serde_json::from_slice(&bytes).map_err(|e| Error::ConfigDeserialization(e.into()))
> + }
> + None => Ok(State::default()),
> + }
> + }
> +
> + /// Persist the state for the endpoint identified by `name`.
> + ///
> + /// # Errors
> + /// An [`Error`] is returned if serialization of the state object, or the final write, fail.
> + pub(crate) fn store(self, name: &str) -> Result<(), Error> {
> + let path = context().state_file_path(name);
> + let parent = std::path::Path::new(&path).parent().unwrap();
> +
> + proxmox_sys::fs::ensure_dir_exists(parent, &context().secret_create_options(), false)
> + .map_err(|e| Error::ConfigSerialization(e.into()))?;
I'm getting a "bad permissions on "/etc/pve/priv/notifications" (0o700
!= 0o600)" error in the system/task logs when the token is refreshed.
The CreateOptions provided by the context are 0o600, but I think pmxcfs
automatically enforces the x-bit on directories, leading to 0o700 perms
on the '/etc/pve/priv/notifications/' directory, which ultimately gives
us this error. I think you either have to add a separate trait method
for the directory permissions/CreateOptions, or add the x-bit to the
existing CreateOptions yourself here at the callsite.
> +
> + let s = serde_json::to_string_pretty(&self)
> + .map_err(|e| Error::ConfigSerialization(e.into()))?;
> +
> + proxmox_sys::fs::replace_file(path, s.as_bytes(), context().secret_create_options(), true)
> + .map_err(|e| Error::ConfigSerialization(e.into()))
> + }
> +
> + /// Set `last_refreshed`.
> + pub(crate) fn set_last_refreshed(mut self, last_refreshed: i64) -> Self {
> + self.last_refreshed = last_refreshed;
> + self
> + }
> +
> + /// Set `oauth2_refresh_token`.
> + pub(crate) fn set_oauth2_refresh_token(mut self, oauth2_refresh_token: Option<String>) -> Self {
> + self.oauth2_refresh_token = oauth2_refresh_token;
> + self
> + }
> +}
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH proxmox 5/7] notify (smtp): Add state handling logic
2026-02-13 16:04 ` [PATCH proxmox 5/7] notify (smtp): Add state handling logic Arthur Bied-Charreton
@ 2026-03-23 12:26 ` Lukas Wagner
0 siblings, 0 replies; 38+ messages in thread
From: Lukas Wagner @ 2026-03-23 12:26 UTC (permalink / raw)
To: Arthur Bied-Charreton, pve-devel
I think this misses removing the endpoint state file in
`delete_endpoint`. Some more notes inline.
On Fri Feb 13, 2026 at 5:04 PM CET, Arthur Bied-Charreton wrote:
> Create new state file in add_endpoint, and create/update existing one in
> update_endpoint.
>
> Add trigger_state_refresh to the Endpoint trait, with no-op default
> implementation. Override trigger_state_refresh in SmtpEndpoint's
> Endpoint impl to trigger an OAuth2 token exchange, in order to rotate
> an existing token, or extend its lifetime.
>
> Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> ---
> proxmox-notify/src/api/common.rs | 16 ++++++
> proxmox-notify/src/api/smtp.rs | 29 ++++++++++-
> proxmox-notify/src/endpoints/smtp.rs | 75 +++++++++++++++++++++++++++-
> proxmox-notify/src/lib.rs | 19 +++++++
> 4 files changed, 137 insertions(+), 2 deletions(-)
>
> diff --git a/proxmox-notify/src/api/common.rs b/proxmox-notify/src/api/common.rs
> index fa2356e2..1e6b7d46 100644
> --- a/proxmox-notify/src/api/common.rs
> +++ b/proxmox-notify/src/api/common.rs
> @@ -3,6 +3,22 @@ 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.
> +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 4231cdae..8992e789 100644
> --- a/proxmox-notify/src/api/smtp.rs
> +++ b/proxmox-notify/src/api/smtp.rs
> @@ -2,7 +2,7 @@ use proxmox_http_error::HttpError;
>
> use crate::api::{http_bail, http_err};
> use crate::endpoints::smtp::{
> - DeleteableSmtpProperty, SmtpConfig, SmtpConfigUpdater, SmtpPrivateConfig,
> + self, DeleteableSmtpProperty, SmtpConfig, SmtpConfigUpdater, SmtpPrivateConfig,
> SmtpPrivateConfigUpdater, SMTP_TYPENAME,
> };
> use crate::Config;
> @@ -69,6 +69,16 @@ pub fn add_endpoint(
> &endpoint_config.name,
> )?;
>
> + smtp::State::new(oauth2_refresh_token)
> + .store(&endpoint_config.name)
> + .map_err(|e| {
> + http_err!(
> + INTERNAL_SERVER_ERROR,
> + "could not create state file for endpoint '{}': {e}",
> + endpoint_config.name
> + )
> + })?;
> +
> config
> .config
> .set_data(&endpoint_config.name, SMTP_TYPENAME, &endpoint_config)
> @@ -206,6 +216,23 @@ pub fn update_endpoint(
> }
> })?;
>
> + smtp::State::load(name)
> + .map_err(|e| {
> + http_err!(
> + INTERNAL_SERVER_ERROR,
> + "could not load state for endpoint '{name}': {e}"
> + )
> + })?
> + .set_oauth2_refresh_token(oauth2_refresh_token)
> + .set_last_refreshed(proxmox_time::epoch_i64())
> + .store(name)
> + .map_err(|e| {
> + http_err!(
> + INTERNAL_SERVER_ERROR,
> + "could not persist state for endpoint '{name}': {e}"
> + )
> + })?;
> +
> config
> .config
> .set_data(name, SMTP_TYPENAME, &endpoint)
> diff --git a/proxmox-notify/src/endpoints/smtp.rs b/proxmox-notify/src/endpoints/smtp.rs
> index 361c4da9..244799fd 100644
> --- a/proxmox-notify/src/endpoints/smtp.rs
> +++ b/proxmox-notify/src/endpoints/smtp.rs
> @@ -1,12 +1,15 @@
> 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::authentication::{Credentials, Mechanism};
> use lettre::transport::smtp::client::{Tls, TlsParameters};
> use lettre::{message::header::ContentType, Message, SmtpTransport, Transport};
> use serde::{Deserialize, Serialize};
>
> +use oauth2::{ClientId, ClientSecret, RefreshToken};
> +
> use proxmox_schema::api_types::COMMENT_SCHEMA;
> use proxmox_schema::{api, Updater};
>
> @@ -22,6 +25,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_SECONDS: u64 = 60 * 60 * 12;
Duration::from_secs is actually a `const fn`, so you could
const SMTP_STATE_REFRESH_CUTOFF: Duration = Duration::from_secs(12 * 60 * 60);
>
> mod state;
> mod xoauth2;
> @@ -205,6 +209,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)
> @@ -336,6 +377,38 @@ impl Endpoint for SmtpEndpoint {
> fn disabled(&self) -> bool {
> self.config.disable.unwrap_or_default()
> }
> +
> + fn trigger_state_refresh(&self) -> Result<(), Error> {
> + let state = State::load(self.name())?;
> +
> + let Some(refresh_token) = &state.oauth2_refresh_token else {
> + return Ok(());
> + };
> +
> + // The refresh job is configured in pveupdate, which runs once for each node.
> + // Don't refresh if we already did it recently.
> + if SystemTime::now()
> + .duration_since(UNIX_EPOCH + Duration::from_secs(state.last_refreshed as u64))
> + .map_err(|e| Error::Generic(e.to_string()))?
> + < Duration::from_secs(SMTP_STATE_REFRESH_CUTOFF_SECONDS)
This reminds me, we deal with "raw" timestamps so often (especially in
PDM), we really need to have some better types/helpers for things like
these... ofc out of scope for this series...
> + {
> + return Ok(());
> + }
> +
> + let Some(auth_method) = self.config.auth_method.as_ref() else {
> + return Ok(());
> + };
> +
> + 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())
> + .store(self.name())
> + }
> }
>
> /// Construct a lettre `Message` from a raw email message.
> diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs
> index 879f8326..c1a5e535 100644
> --- a/proxmox-notify/src/lib.rs
> +++ b/proxmox-notify/src/lib.rs
> @@ -157,6 +157,11 @@ pub trait Endpoint {
>
> /// Check if the endpoint is disabled
> fn disabled(&self) -> bool;
> +
> + /// Refresh endpoint's state
> + fn trigger_state_refresh(&self) -> Result<(), Error> {
> + Ok(())
> + }
> }
>
> #[derive(Debug, Clone, Serialize, Deserialize)]
> @@ -593,6 +598,20 @@ impl Bus {
>
> Ok(())
> }
> +
> + /// Refresh all endpoints' internal state.
> + ///
> + /// 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(()) => info!("triggered state refresh for endpoint '{name}'"),
I'd probably rather use `debug!` here and then `info!` only if one endpoint
actually refreshed its token
> + Err(e) => error!("could not trigger state refresh for endpoint '{name}': {e}"),
> + };
> + }
> + }
> }
>
> #[cfg(test)]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH proxmox 6/7] notify (smtp): Add XOAUTH2 authentication support
2026-02-13 16:04 ` [PATCH proxmox 6/7] notify (smtp): Add XOAUTH2 authentication support Arthur Bied-Charreton
@ 2026-03-23 12:26 ` Lukas Wagner
0 siblings, 0 replies; 38+ messages in thread
From: Lukas Wagner @ 2026-03-23 12:26 UTC (permalink / raw)
To: Arthur Bied-Charreton, pve-devel
On Fri Feb 13, 2026 at 5:04 PM CET, Arthur Bied-Charreton wrote:
> Extend the transport building logic to authenticate via XOAUTH2 if
> configured, and manage the related state updates.
>
> Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> ---
> proxmox-notify/src/endpoints/smtp.rs | 81 +++++++++++++++++++++++-----
> proxmox-notify/src/lib.rs | 4 +-
> 2 files changed, 70 insertions(+), 15 deletions(-)
>
> diff --git a/proxmox-notify/src/endpoints/smtp.rs b/proxmox-notify/src/endpoints/smtp.rs
> index 244799fd..4364bd11 100644
> --- a/proxmox-notify/src/endpoints/smtp.rs
> +++ b/proxmox-notify/src/endpoints/smtp.rs
> @@ -202,7 +202,7 @@ pub struct SmtpPrivateConfig {
> pub oauth2_client_secret: Option<String>,
> }
>
> -/// A sendmail notification endpoint.
> +/// A SMTP notification endpoint.
> pub struct SmtpEndpoint {
> pub config: SmtpConfig,
> pub private_config: SmtpPrivateConfig,
> @@ -246,22 +246,79 @@ impl SmtpEndpoint {
> }
> }
>
> + /// Infer the auth method based on the presence of a password field in the private config.
> + ///
> + /// This is required for backwards compatibility for configs created before the `auth_method`
> + /// field was added, i.e., the presence of a password implicitly meant plain authentication
> + /// was to be used.
> + fn auth_method(&self) -> Option<SmtpAuthMethod> {
> + self.config.auth_method.or_else(|| {
> + if self.private_config.password.is_some() {
> + Some(SmtpAuthMethod::Plain)
> + } else {
> + None
> + }
> + })
> + }
> +
> + /// Build an [`SmtpTransport`].
> + ///
> + /// If OAuth2 authentication is configured, this method will additionally load,
> + /// update and store the OAuth2-related state for this endpoint.
> fn build_transport(&self, tls: Tls, port: u16) -> Result<SmtpTransport, Error> {
> - let mut transport_builder = SmtpTransport::builder_dangerous(&self.config.server)
> + let transport_builder = SmtpTransport::builder_dangerous(&self.config.server)
> .tls(tls)
> .port(port)
> .timeout(Some(Duration::from_secs(SMTP_TIMEOUT.into())));
>
> - if let Some(username) = self.config.username.as_deref() {
> - if let Some(password) = self.private_config.password.as_deref() {
> - transport_builder = transport_builder.credentials((username, password).into());
> - } else {
> - return Err(Error::NotifyFailed(
> - self.name().into(),
> - Box::new(Error::Generic(
> - "username is set but no password was provided".to_owned(),
> - )),
> - ));
> + let transport_builder = match &self.auth_method() {
> + None => transport_builder,
> + Some(SmtpAuthMethod::Plain) => match (
> + self.config.username.as_deref(),
> + self.private_config.password.as_deref(),
> + ) {
> + (Some(username), Some(password)) => {
> + transport_builder.credentials((username, password).into())
> + }
> + (Some(_), None) => {
> + return Err(Error::NotifyFailed(
> + self.name().into(),
> + Box::new(Error::Generic(
> + "username is set but no password was provided".to_owned(),
> + )),
> + ))
> + }
> + _ => transport_builder,
> + },
> + Some(method) => {
> + let state = State::load(self.name())?;
> +
> + let refresh_token =
> + state
> + .oauth2_refresh_token
> + .clone()
> + .ok_or(Error::NotifyFailed(
> + self.name().into(),
> + Box::new(Error::Generic("no refresh token found".into())),
> + ))?;
> + let token_exchange_result = self.get_access_token(&refresh_token, method)?;
> +
> + state
> + .set_oauth2_refresh_token(Some(
> + token_exchange_result
> + .refresh_token
> + .map(|t| t.into_secret())
> + .unwrap_or_else(|| refresh_token),
> + ))
> + .set_last_refreshed(proxmox_time::epoch_i64())
> + .store(self.name())?;
> +
> + transport_builder
> + .credentials(Credentials::new(
> + self.config.from_address.to_owned(),
> + token_exchange_result.access_token.into_secret(),
> + ))
> + .authentication(vec![Mechanism::Xoauth2])
> }
> };
>
> diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs
> index c1a5e535..996393c2 100644
> --- a/proxmox-notify/src/lib.rs
> +++ b/proxmox-notify/src/lib.rs
> @@ -557,9 +557,7 @@ impl Bus {
> }
>
> match endpoint.send(notification) {
> - Ok(_) => {
> - info!("notified via target `{name}`");
> - }
> + Ok(_) => info!("notified via target `{name}`"),
Unrelated change, if this the result of a `cargo fmt`, maybe move to a
separate commit at the start of the series?
> Err(e) => {
> // Only log on errors, do not propagate fail to the caller.
> error!("could not notify via target `{name}`: {e}");
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH proxmox 7/7] notify (smtp): Add logging and state-related error types
2026-02-13 16:04 ` [PATCH proxmox 7/7] notify (smtp): Add logging and state-related error types Arthur Bied-Charreton
@ 2026-03-23 12:26 ` Lukas Wagner
0 siblings, 0 replies; 38+ messages in thread
From: Lukas Wagner @ 2026-03-23 12:26 UTC (permalink / raw)
To: Arthur Bied-Charreton, pve-devel
On Fri Feb 13, 2026 at 5:04 PM CET, Arthur Bied-Charreton wrote:
> Log load/store events in SMTP state management, and add error types to make errors clearer (previously used Config(De)?Serialization errors, which led to confusing error messages for state-related errors)
Commit message should be properly wrapped :)
Also, I think this commit should be split, the error-part and the
logging part rather seem to be separate things. Both could also be
probably folded into previous commits (e.g. add the persistence errors
when you introduce the state handling), but no hard feelings about that,
I'm also fine with having this in a separate commit.
>
> Also add logs for OAuth2 token exchange events.
>
> Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> ---
> proxmox-notify/src/endpoints/smtp.rs | 12 +++++++++++
> proxmox-notify/src/endpoints/smtp/state.rs | 25 +++++++++++++++-------
> proxmox-notify/src/lib.rs | 12 +++++++++++
> 3 files changed, 41 insertions(+), 8 deletions(-)
>
> diff --git a/proxmox-notify/src/endpoints/smtp.rs b/proxmox-notify/src/endpoints/smtp.rs
> index 4364bd11..0ae1ac9f 100644
> --- a/proxmox-notify/src/endpoints/smtp.rs
> +++ b/proxmox-notify/src/endpoints/smtp.rs
> @@ -12,6 +12,7 @@ use oauth2::{ClientId, ClientSecret, RefreshToken};
>
> use proxmox_schema::api_types::COMMENT_SCHEMA;
> use proxmox_schema::{api, Updater};
> +use tracing::info;
nit: usually we try to group imports like:
- std
- 3rd party crates
- our on crates
- crate-level imports
>
> use crate::context::context;
> use crate::endpoints::common::mail;
> @@ -301,6 +302,12 @@ impl SmtpEndpoint {
> self.name().into(),
> Box::new(Error::Generic("no refresh token found".into())),
> ))?;
> +
> + info!(
> + "requesting OAuth2 access token for endpoint '{}'",
> + self.config.name
> + );
> +
Seeing this in practise, I think I'd rather use debug! here... Could be
considered a bit noisy otherwise, as this line would appear e.g. in task
logs for backup jobs.
> let token_exchange_result = self.get_access_token(&refresh_token, method)?;
>
> state
> @@ -313,6 +320,11 @@ impl SmtpEndpoint {
> .set_last_refreshed(proxmox_time::epoch_i64())
> .store(self.name())?;
>
> + info!(
> + "OAuth2 token exchange successful for endpoint '{}'",
> + self.config.name
> + );
> +
Same here
> transport_builder
> .credentials(Credentials::new(
> self.config.from_address.to_owned(),
> diff --git a/proxmox-notify/src/endpoints/smtp/state.rs b/proxmox-notify/src/endpoints/smtp/state.rs
> index 60bef590..45d0db93 100644
> --- a/proxmox-notify/src/endpoints/smtp/state.rs
> +++ b/proxmox-notify/src/endpoints/smtp/state.rs
> @@ -1,4 +1,5 @@
> use serde::{Deserialize, Serialize};
> +use tracing::debug;
>
> use crate::{context::context, Error};
>
> @@ -25,13 +26,19 @@ impl State {
> /// # Errors
> /// An [`Error`] is returned if deserialization of the state object fails.
> pub(crate) fn load(name: &str) -> Result<State, Error> {
> - match proxmox_sys::fs::file_get_optional_contents(context().state_file_path(name))
> - .map_err(|e| Error::ConfigDeserialization(e.into()))?
> + let path = context().state_file_path(name);
> +
> + match proxmox_sys::fs::file_get_optional_contents(&path)
> + .map_err(|e| Error::StateRetrieval(path.to_owned(), e.into()))?
> {
> Some(bytes) => {
> - serde_json::from_slice(&bytes).map_err(|e| Error::ConfigDeserialization(e.into()))
> + debug!("loaded state file for endpoint '{name}' from {path}");
> + serde_json::from_slice(&bytes).map_err(|e| Error::StateRetrieval(path, e.into()))
> + }
> + None => {
> + debug!("no existing state file found for endpoint '{name}' at {path}, creating empty state");
> + Ok(State::default())
> }
> - None => Ok(State::default()),
> }
> }
>
> @@ -43,14 +50,16 @@ impl State {
> let path = context().state_file_path(name);
> let parent = std::path::Path::new(&path).parent().unwrap();
>
> + debug!("attempting to persist state for endpoint '{name}' at {path}");
> +
> proxmox_sys::fs::ensure_dir_exists(parent, &context().secret_create_options(), false)
> - .map_err(|e| Error::ConfigSerialization(e.into()))?;
> + .map_err(|e| Error::StatePersistence(path.to_owned(), e.into()))?;
>
> let s = serde_json::to_string_pretty(&self)
> - .map_err(|e| Error::ConfigSerialization(e.into()))?;
> + .map_err(|e| Error::StatePersistence(path.to_owned(), e.into()))?;
>
> - proxmox_sys::fs::replace_file(path, s.as_bytes(), context().secret_create_options(), true)
> - .map_err(|e| Error::ConfigSerialization(e.into()))
> + proxmox_sys::fs::replace_file(&path, s.as_bytes(), context().secret_create_options(), true)
> + .map_err(|e| Error::StatePersistence(path, e.into()))
> }
>
> /// Set `last_refreshed`.
> diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs
> index 996393c2..37237fc0 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,
> }
> }
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH proxmox-perl-rs 1/1] notify (smtp): add oauth2 parameters to bindings
2026-02-13 16:04 ` [PATCH proxmox-perl-rs 1/1] notify (smtp): add oauth2 parameters to bindings Arthur Bied-Charreton
@ 2026-03-23 12:26 ` Lukas Wagner
2026-03-23 16:44 ` Arthur Bied-Charreton
0 siblings, 1 reply; 38+ messages in thread
From: Lukas Wagner @ 2026-03-23 12:26 UTC (permalink / raw)
To: Arthur Bied-Charreton, pve-devel
On Fri Feb 13, 2026 at 5:04 PM CET, Arthur Bied-Charreton wrote:
> Update SMTP bindings to take the Smtp(Private)?Config structs directly,
> and additionally the OAuth2 refresh token.
The commit message should probably go into more detail on *why* we now
use the struct itself instead of enumerating all parameters in the
function signature.
The addition of 'trigger_state_refresh' should probably its own commit.
The change of parameters for the existing functions is a breaking
change, so I think it would be nice to have these separated from the
addition of this new function.
>
> Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> ---
> common/src/bindings/notify.rs | 82 ++++++++++++-----------------------
> 1 file changed, 28 insertions(+), 54 deletions(-)
>
> diff --git a/common/src/bindings/notify.rs b/common/src/bindings/notify.rs
> index 409270a..137cc79 100644
> --- a/common/src/bindings/notify.rs
> +++ b/common/src/bindings/notify.rs
> @@ -12,7 +12,7 @@ pub mod proxmox_rs_notify {
> use std::collections::HashMap;
> use std::sync::Mutex;
>
> - use anyhow::{Error, bail};
> + use anyhow::{bail, Error};
> use serde_json::Value as JSONValue;
>
> use perlmod::Value;
> @@ -26,7 +26,7 @@ pub mod proxmox_rs_notify {
> DeleteableSendmailProperty, SendmailConfig, SendmailConfigUpdater,
> };
> use proxmox_notify::endpoints::smtp::{
> - DeleteableSmtpProperty, SmtpConfig, SmtpConfigUpdater, SmtpMode, SmtpPrivateConfig,
> + DeleteableSmtpProperty, SmtpConfig, SmtpConfigUpdater, SmtpPrivateConfig,
> SmtpPrivateConfigUpdater,
> };
> use proxmox_notify::endpoints::webhook::{
> @@ -36,7 +36,7 @@ pub mod proxmox_rs_notify {
> CalendarMatcher, DeleteableMatcherProperty, FieldMatcher, MatchModeOperator, MatcherConfig,
> MatcherConfigUpdater, SeverityMatcher,
> };
> - use proxmox_notify::{Config, Notification, Severity, api};
> + use proxmox_notify::{api, Config, Notification, Severity};
>
> /// A notification catalog instance.
> ///
> @@ -141,6 +141,19 @@ pub mod proxmox_rs_notify {
> api::common::send(&config, ¬ification)
> }
>
> + /// Method: Refresh the state for all endpoints.
> + ///
> + /// This iterates through all configured targets, refreshing their state if needed.
> + ///
> + /// See [`api::common::refresh_targets`]
> + #[export(serialize_error)]
> + pub fn trigger_state_refresh(
> + #[try_from_ref] this: &NotificationConfig,
> + ) -> Result<(), HttpError> {
> + let config = this.config.lock().unwrap();
> + api::common::trigger_state_refresh(&config)
> + }
> +
> /// Method: Get a list of all notification targets.
> ///
> /// See [`api::get_targets`].
> @@ -390,37 +403,16 @@ pub mod proxmox_rs_notify {
> #[allow(clippy::too_many_arguments)]
> pub fn add_smtp_endpoint(
> #[try_from_ref] this: &NotificationConfig,
> - name: String,
> - server: String,
> - port: Option<u16>,
> - mode: Option<SmtpMode>,
> - username: Option<String>,
> - password: Option<String>,
> - mailto: Option<Vec<String>>,
> - mailto_user: Option<Vec<String>>,
> - from_address: String,
> - author: Option<String>,
> - comment: Option<String>,
> - disable: Option<bool>,
> + smtp_config: SmtpConfig,
> + smtp_private_config: SmtpPrivateConfig,
> + oauth2_refresh_token: Option<String>,
> ) -> Result<(), HttpError> {
> let mut config = this.config.lock().unwrap();
> api::smtp::add_endpoint(
> &mut config,
> - SmtpConfig {
> - name: name.clone(),
> - server,
> - port,
> - mode,
> - username,
> - mailto: mailto.unwrap_or_default(),
> - mailto_user: mailto_user.unwrap_or_default(),
> - from_address,
> - author,
> - comment,
> - disable,
> - origin: None,
> - },
> - SmtpPrivateConfig { name, password },
> + smtp_config,
> + smtp_private_config,
> + oauth2_refresh_token,
> )
> }
>
> @@ -432,17 +424,9 @@ pub mod proxmox_rs_notify {
> pub fn update_smtp_endpoint(
> #[try_from_ref] this: &NotificationConfig,
> name: &str,
> - server: Option<String>,
> - port: Option<u16>,
> - mode: Option<SmtpMode>,
> - username: Option<String>,
> - password: Option<String>,
> - mailto: Option<Vec<String>>,
> - mailto_user: Option<Vec<String>>,
> - from_address: Option<String>,
> - author: Option<String>,
> - comment: Option<String>,
> - disable: Option<bool>,
> + smtp_config_updater: SmtpConfigUpdater,
> + smtp_private_config_updater: SmtpPrivateConfigUpdater,
> + oauth2_refresh_token: Option<String>,
> delete: Option<Vec<DeleteableSmtpProperty>>,
> digest: Option<&str>,
> ) -> Result<(), HttpError> {
> @@ -452,19 +436,9 @@ pub mod proxmox_rs_notify {
> api::smtp::update_endpoint(
> &mut config,
> name,
> - SmtpConfigUpdater {
> - server,
> - port,
> - mode,
> - username,
> - mailto,
> - mailto_user,
> - from_address,
> - author,
> - comment,
> - disable,
> - },
> - SmtpPrivateConfigUpdater { password },
> + smtp_config_updater,
> + smtp_private_config_updater,
> + oauth2_refresh_token,
> delete.as_deref(),
> digest.as_deref(),
> )
I wondered (and we shortly discussed off-list) whether it would make
sense to keep the existing functions as is and then add two *new*
functions which offer the 'new' struct/hash-based API.
It would have the benefit of being a non-breaking change which would
ease life for maintainers a bit, but then on the other hand it's also a
bit weird since we'd want to remove the old functions pretty soon anyway
-- we have only one caller to worry about anyway.
When removing the old variants we'd have to bump the version contraints
anyway, so in reality, it does not help that much. So after some
thought, I think the current approach is fine.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH proxmox-widget-toolkit 1/2] utils: Add OAuth2 flow handlers
2026-02-13 16:04 ` [PATCH proxmox-widget-toolkit 1/2] utils: Add OAuth2 flow handlers Arthur Bied-Charreton
@ 2026-03-23 12:26 ` Lukas Wagner
0 siblings, 0 replies; 38+ messages in thread
From: Lukas Wagner @ 2026-03-23 12:26 UTC (permalink / raw)
To: Arthur Bied-Charreton, pve-devel
Seems like formatting is off in this and the next patch - make sure to
run `make tidy` for your JS changes. :)
On Fri Feb 13, 2026 at 5:04 PM CET, Arthur Bied-Charreton wrote:
> Introduce the Proxmox.OAuth2 singleton supporting Google and Microsoft
> OAuth2. The flow is handled by opening a new window with the
> authorization URL, and expects to receive the resulting authorization
> code from the redirect handler via a [BroadcastChannel].
>
> [BroadcastChannel]
> https://developer.mozilla.org/en-US/docs/Web/API/BroadcastChannel
>
> Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> ---
> src/Utils.js | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 88 insertions(+)
>
> diff --git a/src/Utils.js b/src/Utils.js
> index 5457ffa..5cbe9b6 100644
> --- a/src/Utils.js
> +++ b/src/Utils.js
> @@ -1723,6 +1723,94 @@ Ext.define('Proxmox.Utils', {
> },
> });
>
> +Ext.define('Proxmox.OAuth2', {
> + singleton: true,
> +
> + handleGoogleFlow: function (clientId, clientSecret) {
> + return this._handleFlow({
> + clientId,
> + clientSecret,
> + authUrl: 'https://accounts.google.com/o/oauth2/v2/auth',
> + tokenUrl: 'https://oauth2.googleapis.com/token',
> + scope: 'https://mail.google.com',
> + extraAuthParams: {
> + access_type: 'offline',
> + prompt: 'consent',
> + },
> + });
> + },
> +
> + handleMicrosoftFlow: function(clientId, clientSecret, tenantId) {
> + return this._handleFlow({
> + clientId,
> + clientSecret,
> + authUrl: `https://login.microsoftonline.com/${tenantId}/oauth2/v2.0/authorize`,
> + tokenUrl: `https://login.microsoftonline.com/${tenantId}/oauth2/v2.0/token`,
> + scope: 'https://outlook.office.com/SMTP.Send offline_access',
> + extraAuthParams: {
> + prompt: 'consent',
> + },
> + });
> + },
> +
> + _handleFlow: function (config) {
> + return new Promise((resolve, reject) => {
> + let redirectUri = window.location.origin;
> + let channelName = `oauth2_${crypto.randomUUID()}`;
> + let state = encodeURIComponent(JSON.stringify({ channelName }));
> +
> + let authParams = new URLSearchParams({
> + client_id: config.clientId,
> + response_type: 'code',
> + redirect_uri: redirectUri,
> + scope: config.scope,
> + state,
> + ...config.extraAuthParams,
> + });
> +
> + let authUrl = `${config.authUrl}?${authParams}`;
> +
> + // Opens OAuth2 authentication window. The app's redirect handler must
> + // extract the authorization code from the callback URL and send it via:
> + // new BroadcastChannel(state.channelName).postMessage({ code })
> + let channel = new BroadcastChannel(channelName);
> + let popup = window.open(authUrl);
> + if (!popup) {
> + reject(new Error('Could not open authentication window'));
> + return;
> + }
> +
> + channel.addEventListener('message', async (event) => {
> + if (popup && !popup.closed) {
> + popup.close();
> + }
> + channel.close();
> +
> + try {
> + let response = await fetch(config.tokenUrl, {
> + method: 'POST',
> + headers: {
> + 'Content-Type': 'application/x-www-form-urlencoded',
> + },
> + body: new URLSearchParams({
> + grant_type: 'authorization_code',
> + code: event.data.code,
> + client_id: config.clientId,
> + client_secret: config.clientSecret,
> + redirect_uri: redirectUri,
> + }),
> + });
> +
> + let tokens = await response.json();
> + resolve(tokens.refresh_token);
> + } catch (error) {
> + reject(error);
> + }
> + });
> + })
> + }
> +})
> +
> Ext.define('Proxmox.Async', {
> singleton: true,
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH proxmox-widget-toolkit 2/2] notifications: Add opt-in OAuth2 support for SMTP targets
2026-02-13 16:04 ` [PATCH proxmox-widget-toolkit 2/2] notifications: Add opt-in OAuth2 support for SMTP targets Arthur Bied-Charreton
@ 2026-03-23 12:26 ` Lukas Wagner
2026-03-23 16:49 ` Arthur Bied-Charreton
0 siblings, 1 reply; 38+ messages in thread
From: Lukas Wagner @ 2026-03-23 12:26 UTC (permalink / raw)
To: Arthur Bied-Charreton, pve-devel
On Fri Feb 13, 2026 at 5:04 PM CET, Arthur Bied-Charreton wrote:
> Add Google & Microsoft OAuth2 authentication methods to SMTP endpoint
> config.
> The enableOAuth2 pmxSmtpEditPanel config flag allows consumers to opt
> into the new feature, so it can be gradually introduced into services.
>
> When disabled, no changes are visible from the UI, and only 'None' and
> 'Username/Password' are shown as
> authentication methods. The flag is passed from the schema config, as it
> is done for defaultMailAuthor.
>
> Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
Some notes:
- same as the other patch, this seems to be missing a `make tidy`
- when adding a new endpoint, after successful authorization, it would
be nice to get visual feedback in the dialog. Maybe something like a
green checkmark and "Authorized" in the UI next to the button?
- when editing an existing OAUTH SMTP endpoint, the "Client Secret"
field is marked invalid because it does not contain any text, also
making it impossible to change anything else in this endpoint.
This should handled similarly to how we handle the regular password
field: Only require a value if isCreate is true, and use a
"Unchanged" empty text otherwise.
- Not 100% sure, but from my understanding the term "Authorize" would
be more fitting then "Authenticate"
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH pve-manager 1/5] notifications: Add OAuth2 parameters to schema and add/update endpoints
2026-02-13 16:04 ` [PATCH pve-manager 1/5] notifications: Add OAuth2 parameters to schema and add/update endpoints Arthur Bied-Charreton
@ 2026-03-23 12:26 ` Lukas Wagner
0 siblings, 0 replies; 38+ messages in thread
From: Lukas Wagner @ 2026-03-23 12:26 UTC (permalink / raw)
To: Arthur Bied-Charreton, pve-devel
On Fri Feb 13, 2026 at 5:04 PM CET, Arthur Bied-Charreton wrote:
> Add auth-method, as well as optional
> oauth2-{client-id,client-secret,tenant-id,refresh-token} parameters to
> prepare for OAuth2 support.
>
> The auth-method parameter was previously implicit and inferred by
> proxmox-notify based on the presence of a password. It is now made
> explicit, however still kept optional and explicitly inferred in the
> {update,create}_endpoint handlers to avoid breaking the API.
This should also mention the API restructuring and why it was done.
>
> Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> ---
> PVE/API2/Cluster/Notifications.pm | 110 +++++++++++++++++++++++-------
> 1 file changed, 87 insertions(+), 23 deletions(-)
>
> diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm
> index 8b455227..e7acea49 100644
> --- a/PVE/API2/Cluster/Notifications.pm
> +++ b/PVE/API2/Cluster/Notifications.pm
> @@ -941,6 +941,13 @@ my $smtp_properties = {
> default => 'tls',
> optional => 1,
> },
> + 'auth-method' => {
> + description =>
> + 'Determine which authentication method shall be used for the connection.',
> + type => 'string',
> + enum => [qw(google-oauth2 microsoft-oauth2 plain none)],
> + optional => 1,
> + },
> username => {
> description => 'Username for SMTP authentication',
> type => 'string',
> @@ -951,6 +958,26 @@ my $smtp_properties = {
> type => 'string',
> optional => 1,
> },
> + 'oauth2-client-id' => {
> + description => 'OAuth2 client ID',
> + type => 'string',
> + optional => 1,
> + },
> + 'oauth2-client-secret' => {
> + description => 'OAuth2 client secret',
> + type => 'string',
> + optional => 1,
> + },
> + 'oauth2-tenant-id' => {
> + description => 'OAuth2 tenant ID',
The description here should probably mention that this is only needed
for microsoft-oauth2
> + type => 'string',
> + optional => 1,
> + },
> + 'oauth2-refresh-token' => {
> + description => 'OAuth2 refresh token',
> + type => 'string',
> + optional => 1,
> + },
> mailto => {
> type => 'array',
> items => {
> @@ -1108,6 +1135,11 @@ __PACKAGE__->register_method({
> my $mode = extract_param($param, 'mode');
> my $username = extract_param($param, 'username');
> my $password = extract_param($param, 'password');
> + my $auth_method = extract_param($param, 'auth-method');
> + my $oauth2_client_secret = extract_param($param, 'oauth2-client-secret');
> + my $oauth2_client_id = extract_param($param, 'oauth2-client-id');
> + my $oauth2_tenant_id = extract_param($param, 'oauth2-tenant-id');
> + my $oauth2_refresh_token = extract_param($param, 'oauth2-refresh-token');
> my $mailto = extract_param($param, 'mailto');
> my $mailto_user = extract_param($param, 'mailto-user');
> my $from_address = extract_param($param, 'from-address');
> @@ -1115,23 +1147,37 @@ __PACKAGE__->register_method({
> my $comment = extract_param($param, 'comment');
> my $disable = extract_param($param, 'disable');
>
> + if (!defined $auth_method) {
> + $auth_method = defined($password) ? 'plain' : 'none';
> + }
> +
> eval {
> PVE::Notify::lock_config(sub {
> my $config = PVE::Notify::read_config();
>
> $config->add_smtp_endpoint(
> - $name,
> - $server,
> - $port,
> - $mode,
> - $username,
> - $password,
> - $mailto,
> - $mailto_user,
> - $from_address,
> - $author,
> - $comment,
> - $disable,
> + {
> + name => $name,
> + server => $server,
> + port => $port,
> + mode => $mode,
> + username => $username,
> + 'auth-method' => $auth_method,
> + 'oauth2-client-id' => $oauth2_client_id,
> + 'oauth2-tenant-id' => $oauth2_tenant_id,
> + mailto => defined($mailto) ? $mailto : [],
> + 'mailto-user' => defined($mailto_user) ? $mailto_user : [],
> + 'from-address' => $from_address,
> + author => $author,
> + comment => $comment,
> + disable => $disable,
> + },
> + {
> + name => $name,
> + password => $password,
> + 'oauth2-client-secret' => $oauth2_client_secret,
> + },
> + $oauth2_refresh_token,
> );
>
> PVE::Notify::write_config($config);
> @@ -1187,6 +1233,11 @@ __PACKAGE__->register_method({
> my $mode = extract_param($param, 'mode');
> my $username = extract_param($param, 'username');
> my $password = extract_param($param, 'password');
> + my $auth_method = extract_param($param, 'auth-method');
> + my $oauth2_client_secret = extract_param($param, 'oauth2-client-secret');
> + my $oauth2_client_id = extract_param($param, 'oauth2-client-id');
> + my $oauth2_tenant_id = extract_param($param, 'oauth2-tenant-id');
> + my $oauth2_refresh_token = extract_param($param, 'oauth2-refresh-token');
> my $mailto = extract_param($param, 'mailto');
> my $mailto_user = extract_param($param, 'mailto-user');
> my $from_address = extract_param($param, 'from-address');
> @@ -1197,23 +1248,36 @@ __PACKAGE__->register_method({
> my $delete = extract_param($param, 'delete');
> my $digest = extract_param($param, 'digest');
>
> + if (!defined $auth_method) {
> + $auth_method = defined($password) ? 'plain' : 'none';
> + }
> +
> eval {
> PVE::Notify::lock_config(sub {
> my $config = PVE::Notify::read_config();
>
> $config->update_smtp_endpoint(
> $name,
> - $server,
> - $port,
> - $mode,
> - $username,
> - $password,
> - $mailto,
> - $mailto_user,
> - $from_address,
> - $author,
> - $comment,
> - $disable,
> + {
> + server => $server,
> + port => $port,
> + mode => $mode,
> + username => $username,
> + 'auth-method' => $auth_method,
> + 'oauth2-client-id' => $oauth2_client_id,
> + 'oauth2-tenant-id' => $oauth2_tenant_id,
> + mailto => defined($mailto) ? $mailto : [],
> + 'mailto-user' => defined($mailto_user) ? $mailto_user : [],
> + 'from-address' => $from_address,
> + author => $author,
> + comment => $comment,
> + disable => $disable,
> + },
> + {
> + password => $password,
> + 'oauth2-client-secret' => $oauth2_client_secret,
> + },
> + $oauth2_refresh_token,
> $delete,
> $digest,
> );
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH pve-manager 2/5] notifications: Add trigger-state-refresh endpoint
2026-02-13 16:04 ` [PATCH pve-manager 2/5] notifications: Add trigger-state-refresh endpoint Arthur Bied-Charreton
@ 2026-03-23 12:26 ` Lukas Wagner
0 siblings, 0 replies; 38+ messages in thread
From: Lukas Wagner @ 2026-03-23 12:26 UTC (permalink / raw)
To: Arthur Bied-Charreton, pve-devel
On Fri Feb 13, 2026 at 5:04 PM CET, Arthur Bied-Charreton wrote:
> This endpoint allows triggering a refresh of the notification targets'
> state, e.g., to prevent OAuth2 refresh tokens from expiring.
>
> Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> ---
> PVE/API2/Cluster/Notifications.pm | 34 +++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm
> index e7acea49..127d6710 100644
> --- a/PVE/API2/Cluster/Notifications.pm
> +++ b/PVE/API2/Cluster/Notifications.pm
> @@ -321,6 +321,40 @@ __PACKAGE__->register_method({
> },
> });
>
> +__PACKAGE__->register_method({
> + name => "trigger_state_refresh",
> + path => 'trigger-state-refresh',
> + protected => 1,
> + method => 'POST',
> + description => 'Refresh state for all targets',
Could be worth mentioning here that this will refresh tokens for OAUTH2
SMTP endpoints
> + permissions => {
> + check => [
> + 'and',
> + ['perm', '/mapping/notifications', ['Mapping.Modify']],
> + [
> + 'or',
> + ['perm', '/', ['Sys.Audit', 'Sys.Modify']],
> + ['perm', '/', ['Sys.AccessNetwork']],
> + ],
> + ],
> + },
> + parameters => {
> + additionalProperties => 0,
> + properties => {},
> + },
> + returns => { type => 'null' },
> + code => sub {
> + eval {
> + my $config = PVE::Notify::read_config();
> + $config->trigger_state_refresh();
> + };
> +
> + raise_api_error($@) if $@;
> +
> + return;
> + },
> +});
> +
> __PACKAGE__->register_method({
> name => 'test_target',
> path => 'targets/{name}/test',
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH pve-cluster 1/1] notifications: Add refresh_targets subroutine to PVE::Notify
2026-02-13 16:04 ` [PATCH pve-cluster 1/1] notifications: Add refresh_targets subroutine to PVE::Notify Arthur Bied-Charreton
@ 2026-03-23 12:26 ` Lukas Wagner
2026-03-23 16:54 ` Arthur Bied-Charreton
0 siblings, 1 reply; 38+ messages in thread
From: Lukas Wagner @ 2026-03-23 12:26 UTC (permalink / raw)
To: Arthur Bied-Charreton, pve-devel
On Fri Feb 13, 2026 at 5:04 PM CET, Arthur Bied-Charreton wrote:
> Expose API to allow refreshing all notification targets' state from the
> PVE backend, as well as from pveupdate.
>
> Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> ---
> src/PVE/Notify.pm | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/src/PVE/Notify.pm b/src/PVE/Notify.pm
> index b44b5b3..ebb2311 100644
> --- a/src/PVE/Notify.pm
> +++ b/src/PVE/Notify.pm
> @@ -103,6 +103,12 @@ sub error {
> );
> }
>
> +sub trigger_state_refresh {
> + my ($config) = @_;
> + $config = read_config() if !defined($config);
> + $config->trigger_state_refresh();
> +}
> +
maybe I'm missing something, but it does not seem like you actually
need/use this function?
As far as I could tell you always call into the perlmod-bidings directly
in the API handler as well as `pveupdate` and do not call this wrapper
here?
Calling this helper here would be a
PVE::Notify::trigger_state_refresh();
but you always do a
my $config = PVE::Notify::read_config();
$config->trigger_state_refresh();
I think both variants would be OK, but you should either remove this
patch here or actually use it at the call sites :)
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH proxmox 2/7] notify (smtp): Introduce state module
2026-03-23 12:26 ` Lukas Wagner
@ 2026-03-23 16:32 ` Arthur Bied-Charreton
2026-03-24 8:50 ` Arthur Bied-Charreton
1 sibling, 0 replies; 38+ messages in thread
From: Arthur Bied-Charreton @ 2026-03-23 16:32 UTC (permalink / raw)
To: Lukas Wagner; +Cc: pve-devel
On Mon, Mar 23, 2026 at 01:26:05PM +0100, Lukas Wagner wrote:
> Hi Arthur!
>
> Generally we'd write the commit subject rather like this:
>
> "notify: smtp: ...."
>
> instead of
>
> "notify (smtp): ..."
>
> (but not a big deal IMO, just mentioning it)
>
ACK, got my conventions confused, my bad
>
[...]
> > @@ -32,6 +34,10 @@ pub trait Context: Send + Sync + Debug {
> > namespace: Option<&str>,
> > source: TemplateSource,
> > ) -> Result<Option<String>, Error>;
> > + /// Return the path to the state file for this context.
> > + fn state_file_path(&self, name: &str) -> String;
>
> I think this should rather return a PathBuf. And rename `name` ->
> `endpoint_name`?
>
Thanks for catching that, I could not do the usual `P: AsRef<Path>` and
somehow fell back to `String` directly..
> > + /// Create options to be used when writing files containing secrets.
> > + fn secret_create_options(&self) -> CreateOptions;
>
> Due to CreateOptions being part of the `Context` trait now, we need to
> pull in proxmox-sys unconditionally:
>
> diff --git i/proxmox-notify/Cargo.toml w/proxmox-notify/Cargo.toml
> index d816c695..35d5b8cb 100644
> --- i/proxmox-notify/Cargo.toml
> +++ w/proxmox-notify/Cargo.toml
> @@ -36,17 +36,15 @@ 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"]
> -smtp = ["dep:lettre", "dep:oauth2", "dep:ureq", "dep:http", "dep:proxmox-sys"]
> +smtp = ["dep:lettre", "dep:oauth2", "dep:ureq", "dep:http"]
> webhook = ["dep:http", "dep:percent-encoding", "dep:proxmox-base64", "dep:proxmox-http"]
>
>
ACK
>
> > }
> >
> > #[cfg(not(test))]
> > diff --git a/proxmox-notify/src/context/pbs.rs b/proxmox-notify/src/context/pbs.rs
> > index 3e5da59c..4f93b45d 100644
> > --- a/proxmox-notify/src/context/pbs.rs
> > +++ b/proxmox-notify/src/context/pbs.rs
> > @@ -125,6 +125,14 @@ impl Context for PBSContext {
> > .map_err(|err| Error::Generic(format!("could not load template: {err}")))?;
> > Ok(template_string)
> > }
> > +
> > + fn state_file_path(&self, name: &str) -> String {
> > + format!("/var/lib/proxmox-backup/priv/notifications/{name}.json")
> > + }
> > +
> > + fn secret_create_options(&self) -> proxmox_sys::fs::CreateOptions {
> > + proxmox_sys::fs::CreateOptions::new().perm(nix::sys::stat::Mode::from_bits_truncate(0o600))
>
> (1) In the trait impl, you could get the appropriate CreateOptions via
> proxmox-product-config...
>
> > + }
> > }
> >
> > #[cfg(test)]
> > diff --git a/proxmox-notify/src/context/pve.rs b/proxmox-notify/src/context/pve.rs
> > index a97cce26..e30f7b49 100644
> > --- a/proxmox-notify/src/context/pve.rs
> > +++ b/proxmox-notify/src/context/pve.rs
> > @@ -74,6 +74,14 @@ impl Context for PVEContext {
> > .map_err(|err| Error::Generic(format!("could not load template: {err}")))?;
> > Ok(template_string)
> > }
> > +
> > + fn state_file_path(&self, name: &str) -> String {
> > + format!("/etc/pve/priv/notifications/{name}.json")
>
> I think `state` should be somewhere in the path -- so maybe one of
>
> - priv/notification-endpoint-state/{name}.json
> - priv/notification-state/{name}.json
> - priv/notifications/state-{name}.json
>
Makes sense, thanks for the suggestion. Will use the third one,
it allows using the already existing /priv/notifications directory.
> > + }
> > +
> > + fn secret_create_options(&self) -> proxmox_sys::fs::CreateOptions {
> > + proxmox_sys::fs::CreateOptions::new().perm(nix::sys::stat::Mode::from_bits_truncate(0o600))
>
> (2) ... here as well, but you probably need to initialize
> proxmox-product-config in the proxmox-perl-rs bindings with the correct
> UIDs.
>
ACK this and the other proxmox-product-config comment, will integrate
that crate in v2, thanks
[...]
> > + /// Persist the state for the endpoint identified by `name`.
> > + ///
> > + /// # Errors
> > + /// An [`Error`] is returned if serialization of the state object, or the final write, fail.
> > + pub(crate) fn store(self, name: &str) -> Result<(), Error> {
> > + let path = context().state_file_path(name);
> > + let parent = std::path::Path::new(&path).parent().unwrap();
> > +
> > + proxmox_sys::fs::ensure_dir_exists(parent, &context().secret_create_options(), false)
> > + .map_err(|e| Error::ConfigSerialization(e.into()))?;
>
> I'm getting a "bad permissions on "/etc/pve/priv/notifications" (0o700
> != 0o600)" error in the system/task logs when the token is refreshed.
> The CreateOptions provided by the context are 0o600, but I think pmxcfs
> automatically enforces the x-bit on directories, leading to 0o700 perms
> on the '/etc/pve/priv/notifications/' directory, which ultimately gives
> us this error. I think you either have to add a separate trait method
> for the directory permissions/CreateOptions, or add the x-bit to the
> existing CreateOptions yourself here at the callsite.
>
Thanks for catching that! Will fix in v2
> > +
> > + let s = serde_json::to_string_pretty(&self)
> > + .map_err(|e| Error::ConfigSerialization(e.into()))?;
> > +
> > + proxmox_sys::fs::replace_file(path, s.as_bytes(), context().secret_create_options(), true)
> > + .map_err(|e| Error::ConfigSerialization(e.into()))
> > + }
> > +
> > + /// Set `last_refreshed`.
> > + pub(crate) fn set_last_refreshed(mut self, last_refreshed: i64) -> Self {
> > + self.last_refreshed = last_refreshed;
> > + self
> > + }
> > +
> > + /// Set `oauth2_refresh_token`.
> > + pub(crate) fn set_oauth2_refresh_token(mut self, oauth2_refresh_token: Option<String>) -> Self {
> > + self.oauth2_refresh_token = oauth2_refresh_token;
> > + self
> > + }
> > +}
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH proxmox-perl-rs 1/1] notify (smtp): add oauth2 parameters to bindings
2026-03-23 12:26 ` Lukas Wagner
@ 2026-03-23 16:44 ` Arthur Bied-Charreton
0 siblings, 0 replies; 38+ messages in thread
From: Arthur Bied-Charreton @ 2026-03-23 16:44 UTC (permalink / raw)
To: Lukas Wagner; +Cc: pve-devel
On Mon, Mar 23, 2026 at 01:26:20PM +0100, Lukas Wagner wrote:
> On Fri Feb 13, 2026 at 5:04 PM CET, Arthur Bied-Charreton wrote:
> > Update SMTP bindings to take the Smtp(Private)?Config structs directly,
> > and additionally the OAuth2 refresh token.
>
> The commit message should probably go into more detail on *why* we now
> use the struct itself instead of enumerating all parameters in the
> function signature.
>
ACK
> The addition of 'trigger_state_refresh' should probably its own commit.
> The change of parameters for the existing functions is a breaking
> change, so I think it would be nice to have these separated from the
> addition of this new function.
>
ACK, will split this commit
> >
> > Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> > ---
> > common/src/bindings/notify.rs | 82 ++++++++++++-----------------------
> > 1 file changed, 28 insertions(+), 54 deletions(-)
> >
> > diff --git a/common/src/bindings/notify.rs b/common/src/bindings/notify.rs
> > index 409270a..137cc79 100644
> > --- a/common/src/bindings/notify.rs
> > +++ b/common/src/bindings/notify.rs
> > @@ -12,7 +12,7 @@ pub mod proxmox_rs_notify {
> > use std::collections::HashMap;
> > use std::sync::Mutex;
> >
> > - use anyhow::{Error, bail};
> > + use anyhow::{bail, Error};
> > use serde_json::Value as JSONValue;
> >
> > use perlmod::Value;
> > @@ -26,7 +26,7 @@ pub mod proxmox_rs_notify {
> > DeleteableSendmailProperty, SendmailConfig, SendmailConfigUpdater,
> > };
> > use proxmox_notify::endpoints::smtp::{
> > - DeleteableSmtpProperty, SmtpConfig, SmtpConfigUpdater, SmtpMode, SmtpPrivateConfig,
> > + DeleteableSmtpProperty, SmtpConfig, SmtpConfigUpdater, SmtpPrivateConfig,
> > SmtpPrivateConfigUpdater,
> > };
> > use proxmox_notify::endpoints::webhook::{
> > @@ -36,7 +36,7 @@ pub mod proxmox_rs_notify {
> > CalendarMatcher, DeleteableMatcherProperty, FieldMatcher, MatchModeOperator, MatcherConfig,
> > MatcherConfigUpdater, SeverityMatcher,
> > };
> > - use proxmox_notify::{Config, Notification, Severity, api};
> > + use proxmox_notify::{api, Config, Notification, Severity};
> >
> > /// A notification catalog instance.
> > ///
> > @@ -141,6 +141,19 @@ pub mod proxmox_rs_notify {
> > api::common::send(&config, ¬ification)
> > }
> >
> > + /// Method: Refresh the state for all endpoints.
> > + ///
> > + /// This iterates through all configured targets, refreshing their state if needed.
> > + ///
> > + /// See [`api::common::refresh_targets`]
> > + #[export(serialize_error)]
> > + pub fn trigger_state_refresh(
> > + #[try_from_ref] this: &NotificationConfig,
> > + ) -> Result<(), HttpError> {
> > + let config = this.config.lock().unwrap();
> > + api::common::trigger_state_refresh(&config)
> > + }
> > +
> > /// Method: Get a list of all notification targets.
> > ///
> > /// See [`api::get_targets`].
> > @@ -390,37 +403,16 @@ pub mod proxmox_rs_notify {
> > #[allow(clippy::too_many_arguments)]
> > pub fn add_smtp_endpoint(
> > #[try_from_ref] this: &NotificationConfig,
> > - name: String,
> > - server: String,
> > - port: Option<u16>,
> > - mode: Option<SmtpMode>,
> > - username: Option<String>,
> > - password: Option<String>,
> > - mailto: Option<Vec<String>>,
> > - mailto_user: Option<Vec<String>>,
> > - from_address: String,
> > - author: Option<String>,
> > - comment: Option<String>,
> > - disable: Option<bool>,
> > + smtp_config: SmtpConfig,
> > + smtp_private_config: SmtpPrivateConfig,
> > + oauth2_refresh_token: Option<String>,
> > ) -> Result<(), HttpError> {
> > let mut config = this.config.lock().unwrap();
> > api::smtp::add_endpoint(
> > &mut config,
> > - SmtpConfig {
> > - name: name.clone(),
> > - server,
> > - port,
> > - mode,
> > - username,
> > - mailto: mailto.unwrap_or_default(),
> > - mailto_user: mailto_user.unwrap_or_default(),
> > - from_address,
> > - author,
> > - comment,
> > - disable,
> > - origin: None,
> > - },
> > - SmtpPrivateConfig { name, password },
> > + smtp_config,
> > + smtp_private_config,
> > + oauth2_refresh_token,
> > )
> > }
> >
> > @@ -432,17 +424,9 @@ pub mod proxmox_rs_notify {
> > pub fn update_smtp_endpoint(
> > #[try_from_ref] this: &NotificationConfig,
> > name: &str,
> > - server: Option<String>,
> > - port: Option<u16>,
> > - mode: Option<SmtpMode>,
> > - username: Option<String>,
> > - password: Option<String>,
> > - mailto: Option<Vec<String>>,
> > - mailto_user: Option<Vec<String>>,
> > - from_address: Option<String>,
> > - author: Option<String>,
> > - comment: Option<String>,
> > - disable: Option<bool>,
> > + smtp_config_updater: SmtpConfigUpdater,
> > + smtp_private_config_updater: SmtpPrivateConfigUpdater,
> > + oauth2_refresh_token: Option<String>,
> > delete: Option<Vec<DeleteableSmtpProperty>>,
> > digest: Option<&str>,
> > ) -> Result<(), HttpError> {
> > @@ -452,19 +436,9 @@ pub mod proxmox_rs_notify {
> > api::smtp::update_endpoint(
> > &mut config,
> > name,
> > - SmtpConfigUpdater {
> > - server,
> > - port,
> > - mode,
> > - username,
> > - mailto,
> > - mailto_user,
> > - from_address,
> > - author,
> > - comment,
> > - disable,
> > - },
> > - SmtpPrivateConfigUpdater { password },
> > + smtp_config_updater,
> > + smtp_private_config_updater,
> > + oauth2_refresh_token,
> > delete.as_deref(),
> > digest.as_deref(),
> > )
>
> I wondered (and we shortly discussed off-list) whether it would make
> sense to keep the existing functions as is and then add two *new*
> functions which offer the 'new' struct/hash-based API.
>
> It would have the benefit of being a non-breaking change which would
> ease life for maintainers a bit, but then on the other hand it's also a
> bit weird since we'd want to remove the old functions pretty soon anyway
> -- we have only one caller to worry about anyway.
>
> When removing the old variants we'd have to bump the version contraints
> anyway, so in reality, it does not help that much. So after some
> thought, I think the current approach is fine.
Yes, I agree, as discussed off-list, the naming for those new functions
would be quite awkward ^^
I will make the breaking change clear in the commit message for v2
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH proxmox-widget-toolkit 2/2] notifications: Add opt-in OAuth2 support for SMTP targets
2026-03-23 12:26 ` Lukas Wagner
@ 2026-03-23 16:49 ` Arthur Bied-Charreton
0 siblings, 0 replies; 38+ messages in thread
From: Arthur Bied-Charreton @ 2026-03-23 16:49 UTC (permalink / raw)
To: Lukas Wagner; +Cc: pve-devel
On Mon, Mar 23, 2026 at 01:26:25PM +0100, Lukas Wagner wrote:
> On Fri Feb 13, 2026 at 5:04 PM CET, Arthur Bied-Charreton wrote:
> > Add Google & Microsoft OAuth2 authentication methods to SMTP endpoint
> > config.
> > The enableOAuth2 pmxSmtpEditPanel config flag allows consumers to opt
> > into the new feature, so it can be gradually introduced into services.
> >
> > When disabled, no changes are visible from the UI, and only 'None' and
> > 'Username/Password' are shown as
> > authentication methods. The flag is passed from the schema config, as it
> > is done for defaultMailAuthor.
> >
> > Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
>
> Some notes:
>
> - same as the other patch, this seems to be missing a `make tidy`
>
ACK this and the other patch where you mentioned it, thanks!
> - when adding a new endpoint, after successful authorization, it would
> be nice to get visual feedback in the dialog. Maybe something like a
> green checkmark and "Authorized" in the UI next to the button?
>
Will do, sounds like a nice addition
> - when editing an existing OAUTH SMTP endpoint, the "Client Secret"
> field is marked invalid because it does not contain any text, also
> making it impossible to change anything else in this endpoint.
> This should handled similarly to how we handle the regular password
> field: Only require a value if isCreate is true, and use a
> "Unchanged" empty text otherwise.
ACK, will do
>
> - Not 100% sure, but from my understanding the term "Authorize" would
> be more fitting then "Authenticate"
>
You are 100% correct, thanks for catching that
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH pve-cluster 1/1] notifications: Add refresh_targets subroutine to PVE::Notify
2026-03-23 12:26 ` Lukas Wagner
@ 2026-03-23 16:54 ` Arthur Bied-Charreton
0 siblings, 0 replies; 38+ messages in thread
From: Arthur Bied-Charreton @ 2026-03-23 16:54 UTC (permalink / raw)
To: Lukas Wagner; +Cc: pve-devel
On Mon, Mar 23, 2026 at 01:26:34PM +0100, Lukas Wagner wrote:
> On Fri Feb 13, 2026 at 5:04 PM CET, Arthur Bied-Charreton wrote:
> > Expose API to allow refreshing all notification targets' state from the
> > PVE backend, as well as from pveupdate.
> >
> > Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> > ---
> > src/PVE/Notify.pm | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/src/PVE/Notify.pm b/src/PVE/Notify.pm
> > index b44b5b3..ebb2311 100644
> > --- a/src/PVE/Notify.pm
> > +++ b/src/PVE/Notify.pm
> > @@ -103,6 +103,12 @@ sub error {
> > );
> > }
> >
> > +sub trigger_state_refresh {
> > + my ($config) = @_;
> > + $config = read_config() if !defined($config);
> > + $config->trigger_state_refresh();
> > +}
> > +
>
> maybe I'm missing something, but it does not seem like you actually
> need/use this function?
>
No, this is a remnant of a different approach I took in the beginning,
sorry about that - will be gone in v2
> As far as I could tell you always call into the perlmod-bidings directly
> in the API handler as well as `pveupdate` and do not call this wrapper
> here?
>
> Calling this helper here would be a
>
> PVE::Notify::trigger_state_refresh();
>
> but you always do a
>
> my $config = PVE::Notify::read_config();
> $config->trigger_state_refresh();
>
>
> I think both variants would be OK, but you should either remove this
> patch here or actually use it at the call sites :)
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH proxmox 2/7] notify (smtp): Introduce state module
2026-03-23 12:26 ` Lukas Wagner
2026-03-23 16:32 ` Arthur Bied-Charreton
@ 2026-03-24 8:50 ` Arthur Bied-Charreton
1 sibling, 0 replies; 38+ messages in thread
From: Arthur Bied-Charreton @ 2026-03-24 8:50 UTC (permalink / raw)
To: Lukas Wagner; +Cc: pve-devel
On Mon, Mar 23, 2026 at 01:26:05PM +0100, Lukas Wagner wrote:
[...]
> > + proxmox_sys::fs::ensure_dir_exists(parent, &context().secret_create_options(), false)
> > + .map_err(|e| Error::ConfigSerialization(e.into()))?;
>
> I'm getting a "bad permissions on "/etc/pve/priv/notifications" (0o700
> != 0o600)" error in the system/task logs when the token is refreshed.
> The CreateOptions provided by the context are 0o600, but I think pmxcfs
> automatically enforces the x-bit on directories, leading to 0o700 perms
> on the '/etc/pve/priv/notifications/' directory, which ultimately gives
> us this error. I think you either have to add a separate trait method
> for the directory permissions/CreateOptions, or add the x-bit to the
> existing CreateOptions yourself here at the callsite.
>
Just looked into that, there is a `create_secret_dir` function in
`proxmox-product-config` that does exactly what we need here.
> > +
> > + let s = serde_json::to_string_pretty(&self)
> > + .map_err(|e| Error::ConfigSerialization(e.into()))?;
> > +
> > + proxmox_sys::fs::replace_file(path, s.as_bytes(), context().secret_create_options(), true)
> > + .map_err(|e| Error::ConfigSerialization(e.into()))
> > + }
> > +
> > + /// Set `last_refreshed`.
> > + pub(crate) fn set_last_refreshed(mut self, last_refreshed: i64) -> Self {
> > + self.last_refreshed = last_refreshed;
> > + self
> > + }
> > +
> > + /// Set `oauth2_refresh_token`.
> > + pub(crate) fn set_oauth2_refresh_token(mut self, oauth2_refresh_token: Option<String>) -> Self {
> > + self.oauth2_refresh_token = oauth2_refresh_token;
> > + self
> > + }
> > +}
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* superseded: [PATCH cluster/docs/manager/proxmox{,-perl-rs,-widget-toolkit} 00/17] fix #7238: Add XOAUTH2 authentication support for SMTP notification targets
2026-02-13 16:03 [PATCH cluster/docs/manager/proxmox{,-perl-rs,-widget-toolkit} 00/17] fix #7238: Add XOAUTH2 authentication support for SMTP notification targets Arthur Bied-Charreton
` (17 preceding siblings ...)
2026-03-23 12:25 ` [PATCH cluster/docs/manager/proxmox{,-perl-rs,-widget-toolkit} 00/17] fix #7238: Add XOAUTH2 authentication support for SMTP notification targets Lukas Wagner
@ 2026-03-25 13:16 ` Arthur Bied-Charreton
18 siblings, 0 replies; 38+ messages in thread
From: Arthur Bied-Charreton @ 2026-03-25 13:16 UTC (permalink / raw)
To: pve-devel
Superseded-by: https://lore.proxmox.com/pve-devel/20260325131444.366808-1-a.bied-charreton@proxmox.com/T/#t
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH pve-manager 1/5] notifications: Add OAuth2 parameters to schema and add/update endpoints
2026-02-11 8:55 ` Lukas Wagner
@ 2026-02-11 12:47 ` Arthur Bied-Charreton
0 siblings, 0 replies; 38+ messages in thread
From: Arthur Bied-Charreton @ 2026-02-11 12:47 UTC (permalink / raw)
To: Lukas Wagner; +Cc: pve-devel
On Wed, Feb 11, 2026 at 09:55:23AM +0100, Lukas Wagner wrote:
> On Wed Feb 4, 2026 at 5:13 PM CET, Arthur Bied-Charreton wrote:
> > Add auth-method, as well as optional
> > oauth2-{client-id,client-secret,tenant-id,refresh-token} parameters to
> > prepare for OAuth2 support.
> >
> > The auth-method parameter was previously implicit and inferred by
> > proxmox-notify based on the presence of a password. It is now made
> > explicit, however still kept optional and explicitly inferred in the
> > {update,create}_endpoint handlers to avoid breaking the API.
> >
> > Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> > ---
> > PVE/API2/Cluster/Notifications.pm | 55 +++++++++++++++++++++++++++++++
> > 1 file changed, 55 insertions(+)
> > [...]
> > eval {
> > PVE::Notify::lock_config(sub {
> > my $config = PVE::Notify::read_config();
> > @@ -1208,6 +1258,11 @@ __PACKAGE__->register_method({
> > $mode,
> > $username,
> > $password,
> > + $auth_method,
> > + $oauth2_client_id,
> > + $oauth2_client_secret,
> > + $oauth2_tenant_id,
> > + $oauth2_refresh_token,
> > $mailto,
> > $mailto_user,
> > $from_address,
>
> As already explained off-list, I think it's time to switch from a flat
> list of parameters to passing hashes for the parameters for the
> `add_smtp_target` and `update_smtp_target` methods. This means, the
> bindings in proxmox-perl-rs would then directly take
> SmtpConfig/SmtpPrivateConfig and
> SmtpConfigUpdater/SmtpPrivateConfigUpdater. Then the call could look
> something like (not tested)
>
> $config->add_smtp_endpoint(
> $name,
> {
> server => $server,
> port => $port,
> ...
> },
> {
> password => $password,
> ...
> }
> );
>
> This makes it much harder to introduce bugs due to parameter ordering.
> Long-term we should do the same for the other endpoints, but no need to
> do it in this series, I think.
>
That makes a lot of sense, will update it
> For changes like these and in general it's pretty important to mention
> any breaking changes in the cover letter and maybe patch description,
> since the changes done in this series affect multiple packages that
> *could* be updated independently by our users. For instance, in the
> cover-letter you could write something like:
>
> The patch series requires the following version requirement bumps:
>
> pve-manager requires bumped proxmox-perl-rs
> proxmox-perl-rs requires bumped proxmox-notify*
>
>
> *.) although for this one it's not that critical, since its only a
> build-dependency, so there is no chance of customer systems breaking due
> to partial system updates
>
> This way the maintainer knows that the version requirements in
> debian/control must be adapted at some point after applying the patches.
I was wondering how this would work, thanks for the explanation!
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH pve-manager 1/5] notifications: Add OAuth2 parameters to schema and add/update endpoints
2026-02-04 16:13 ` [PATCH pve-manager 1/5] notifications: Add OAuth2 parameters to schema and add/update endpoints Arthur Bied-Charreton
@ 2026-02-11 8:55 ` Lukas Wagner
2026-02-11 12:47 ` Arthur Bied-Charreton
0 siblings, 1 reply; 38+ messages in thread
From: Lukas Wagner @ 2026-02-11 8:55 UTC (permalink / raw)
To: Arthur Bied-Charreton, pve-devel
On Wed Feb 4, 2026 at 5:13 PM CET, Arthur Bied-Charreton wrote:
> Add auth-method, as well as optional
> oauth2-{client-id,client-secret,tenant-id,refresh-token} parameters to
> prepare for OAuth2 support.
>
> The auth-method parameter was previously implicit and inferred by
> proxmox-notify based on the presence of a password. It is now made
> explicit, however still kept optional and explicitly inferred in the
> {update,create}_endpoint handlers to avoid breaking the API.
>
> Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> ---
> PVE/API2/Cluster/Notifications.pm | 55 +++++++++++++++++++++++++++++++
> 1 file changed, 55 insertions(+)
>
> diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm
> index 8b455227..a45a15b2 100644
> --- a/PVE/API2/Cluster/Notifications.pm
> +++ b/PVE/API2/Cluster/Notifications.pm
> @@ -941,6 +941,13 @@ my $smtp_properties = {
> default => 'tls',
> optional => 1,
> },
> + 'auth-method' => {
> + description =>
> + 'Determine which authentication method shall be used for the connection.',
> + type => 'string',
> + enum => [qw(google-oauth2 microsoft-oauth2 plain none)],
> + optional => 1,
> + },
> username => {
> description => 'Username for SMTP authentication',
> type => 'string',
> @@ -951,6 +958,26 @@ my $smtp_properties = {
> type => 'string',
> optional => 1,
> },
> + 'oauth2-client-id' => {
> + description => 'OAuth2 client ID',
> + type => 'string',
> + optional => 1,
> + },
> + 'oauth2-client-secret' => {
> + description => 'OAuth2 client secret',
> + type => 'string',
> + optional => 1,
> + },
> + 'oauth2-tenant-id' => {
> + description => 'OAuth2 tenant ID',
> + type => 'string',
> + optional => 1,
> + },
> + 'oauth2-refresh-token' => {
> + description => 'OAuth2 refresh token',
> + type => 'string',
> + optional => 1,
> + },
> mailto => {
> type => 'array',
> items => {
> @@ -1108,6 +1135,11 @@ __PACKAGE__->register_method({
> my $mode = extract_param($param, 'mode');
> my $username = extract_param($param, 'username');
> my $password = extract_param($param, 'password');
> + my $auth_method = extract_param($param, 'auth-method');
> + my $oauth2_client_secret = extract_param($param, 'oauth2-client-secret');
> + my $oauth2_client_id = extract_param($param, 'oauth2-client-id');
> + my $oauth2_tenant_id = extract_param($param, 'oauth2-tenant-id');
> + my $oauth2_refresh_token = extract_param($param, 'oauth2-refresh-token');
> my $mailto = extract_param($param, 'mailto');
> my $mailto_user = extract_param($param, 'mailto-user');
> my $from_address = extract_param($param, 'from-address');
> @@ -1115,6 +1147,10 @@ __PACKAGE__->register_method({
> my $comment = extract_param($param, 'comment');
> my $disable = extract_param($param, 'disable');
>
> + if (!defined $auth_method) {
> + $auth_method = defined($password) ? 'plain' : 'none';
> + }
> +
> eval {
> PVE::Notify::lock_config(sub {
> my $config = PVE::Notify::read_config();
> @@ -1126,6 +1162,11 @@ __PACKAGE__->register_method({
> $mode,
> $username,
> $password,
> + $auth_method,
> + $oauth2_client_id,
> + $oauth2_client_secret,
> + $oauth2_tenant_id,
> + $oauth2_refresh_token,
> $mailto,
> $mailto_user,
> $from_address,
> @@ -1187,6 +1228,11 @@ __PACKAGE__->register_method({
> my $mode = extract_param($param, 'mode');
> my $username = extract_param($param, 'username');
> my $password = extract_param($param, 'password');
> + my $auth_method = extract_param($param, 'auth-method');
> + my $oauth2_client_secret = extract_param($param, 'oauth2-client-secret');
> + my $oauth2_client_id = extract_param($param, 'oauth2-client-id');
> + my $oauth2_tenant_id = extract_param($param, 'oauth2-tenant-id');
> + my $oauth2_refresh_token = extract_param($param, 'oauth2-refresh-token');
> my $mailto = extract_param($param, 'mailto');
> my $mailto_user = extract_param($param, 'mailto-user');
> my $from_address = extract_param($param, 'from-address');
> @@ -1197,6 +1243,10 @@ __PACKAGE__->register_method({
> my $delete = extract_param($param, 'delete');
> my $digest = extract_param($param, 'digest');
>
> + if (!defined $auth_method) {
> + $auth_method = defined($password) ? 'plain' : 'none';
> + }
> +
> eval {
> PVE::Notify::lock_config(sub {
> my $config = PVE::Notify::read_config();
> @@ -1208,6 +1258,11 @@ __PACKAGE__->register_method({
> $mode,
> $username,
> $password,
> + $auth_method,
> + $oauth2_client_id,
> + $oauth2_client_secret,
> + $oauth2_tenant_id,
> + $oauth2_refresh_token,
> $mailto,
> $mailto_user,
> $from_address,
As already explained off-list, I think it's time to switch from a flat
list of parameters to passing hashes for the parameters for the
`add_smtp_target` and `update_smtp_target` methods. This means, the
bindings in proxmox-perl-rs would then directly take
SmtpConfig/SmtpPrivateConfig and
SmtpConfigUpdater/SmtpPrivateConfigUpdater. Then the call could look
something like (not tested)
$config->add_smtp_endpoint(
$name,
{
server => $server,
port => $port,
...
},
{
password => $password,
...
}
);
This makes it much harder to introduce bugs due to parameter ordering.
Long-term we should do the same for the other endpoints, but no need to
do it in this series, I think.
For changes like these and in general it's pretty important to mention
any breaking changes in the cover letter and maybe patch description,
since the changes done in this series affect multiple packages that
*could* be updated independently by our users. For instance, in the
cover-letter you could write something like:
The patch series requires the following version requirement bumps:
pve-manager requires bumped proxmox-perl-rs
proxmox-perl-rs requires bumped proxmox-notify*
*.) although for this one it's not that critical, since its only a
build-dependency, so there is no chance of customer systems breaking due
to partial system updates
This way the maintainer knows that the version requirements in
debian/control must be adapted at some point after applying the patches.
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH pve-manager 1/5] notifications: Add OAuth2 parameters to schema and add/update endpoints
2026-02-04 16:13 [RFC cluster/docs/manager/proxmox{,-perl-rs,-widget-toolkit} 00/15] " Arthur Bied-Charreton
@ 2026-02-04 16:13 ` Arthur Bied-Charreton
2026-02-11 8:55 ` Lukas Wagner
0 siblings, 1 reply; 38+ messages in thread
From: Arthur Bied-Charreton @ 2026-02-04 16:13 UTC (permalink / raw)
To: pve-devel
Add auth-method, as well as optional
oauth2-{client-id,client-secret,tenant-id,refresh-token} parameters to
prepare for OAuth2 support.
The auth-method parameter was previously implicit and inferred by
proxmox-notify based on the presence of a password. It is now made
explicit, however still kept optional and explicitly inferred in the
{update,create}_endpoint handlers to avoid breaking the API.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
PVE/API2/Cluster/Notifications.pm | 55 +++++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)
diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm
index 8b455227..a45a15b2 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -941,6 +941,13 @@ my $smtp_properties = {
default => 'tls',
optional => 1,
},
+ 'auth-method' => {
+ description =>
+ 'Determine which authentication method shall be used for the connection.',
+ type => 'string',
+ enum => [qw(google-oauth2 microsoft-oauth2 plain none)],
+ optional => 1,
+ },
username => {
description => 'Username for SMTP authentication',
type => 'string',
@@ -951,6 +958,26 @@ my $smtp_properties = {
type => 'string',
optional => 1,
},
+ 'oauth2-client-id' => {
+ description => 'OAuth2 client ID',
+ type => 'string',
+ optional => 1,
+ },
+ 'oauth2-client-secret' => {
+ description => 'OAuth2 client secret',
+ type => 'string',
+ optional => 1,
+ },
+ 'oauth2-tenant-id' => {
+ description => 'OAuth2 tenant ID',
+ type => 'string',
+ optional => 1,
+ },
+ 'oauth2-refresh-token' => {
+ description => 'OAuth2 refresh token',
+ type => 'string',
+ optional => 1,
+ },
mailto => {
type => 'array',
items => {
@@ -1108,6 +1135,11 @@ __PACKAGE__->register_method({
my $mode = extract_param($param, 'mode');
my $username = extract_param($param, 'username');
my $password = extract_param($param, 'password');
+ my $auth_method = extract_param($param, 'auth-method');
+ my $oauth2_client_secret = extract_param($param, 'oauth2-client-secret');
+ my $oauth2_client_id = extract_param($param, 'oauth2-client-id');
+ my $oauth2_tenant_id = extract_param($param, 'oauth2-tenant-id');
+ my $oauth2_refresh_token = extract_param($param, 'oauth2-refresh-token');
my $mailto = extract_param($param, 'mailto');
my $mailto_user = extract_param($param, 'mailto-user');
my $from_address = extract_param($param, 'from-address');
@@ -1115,6 +1147,10 @@ __PACKAGE__->register_method({
my $comment = extract_param($param, 'comment');
my $disable = extract_param($param, 'disable');
+ if (!defined $auth_method) {
+ $auth_method = defined($password) ? 'plain' : 'none';
+ }
+
eval {
PVE::Notify::lock_config(sub {
my $config = PVE::Notify::read_config();
@@ -1126,6 +1162,11 @@ __PACKAGE__->register_method({
$mode,
$username,
$password,
+ $auth_method,
+ $oauth2_client_id,
+ $oauth2_client_secret,
+ $oauth2_tenant_id,
+ $oauth2_refresh_token,
$mailto,
$mailto_user,
$from_address,
@@ -1187,6 +1228,11 @@ __PACKAGE__->register_method({
my $mode = extract_param($param, 'mode');
my $username = extract_param($param, 'username');
my $password = extract_param($param, 'password');
+ my $auth_method = extract_param($param, 'auth-method');
+ my $oauth2_client_secret = extract_param($param, 'oauth2-client-secret');
+ my $oauth2_client_id = extract_param($param, 'oauth2-client-id');
+ my $oauth2_tenant_id = extract_param($param, 'oauth2-tenant-id');
+ my $oauth2_refresh_token = extract_param($param, 'oauth2-refresh-token');
my $mailto = extract_param($param, 'mailto');
my $mailto_user = extract_param($param, 'mailto-user');
my $from_address = extract_param($param, 'from-address');
@@ -1197,6 +1243,10 @@ __PACKAGE__->register_method({
my $delete = extract_param($param, 'delete');
my $digest = extract_param($param, 'digest');
+ if (!defined $auth_method) {
+ $auth_method = defined($password) ? 'plain' : 'none';
+ }
+
eval {
PVE::Notify::lock_config(sub {
my $config = PVE::Notify::read_config();
@@ -1208,6 +1258,11 @@ __PACKAGE__->register_method({
$mode,
$username,
$password,
+ $auth_method,
+ $oauth2_client_id,
+ $oauth2_client_secret,
+ $oauth2_tenant_id,
+ $oauth2_refresh_token,
$mailto,
$mailto_user,
$from_address,
--
2.47.3
^ permalink raw reply [flat|nested] 38+ messages in thread
end of thread, other threads:[~2026-03-25 13:16 UTC | newest]
Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-02-13 16:03 [PATCH cluster/docs/manager/proxmox{,-perl-rs,-widget-toolkit} 00/17] fix #7238: Add XOAUTH2 authentication support for SMTP notification targets Arthur Bied-Charreton
2026-02-13 16:03 ` [PATCH proxmox 1/7] notify (smtp): Introduce xoauth2 module Arthur Bied-Charreton
2026-02-13 16:04 ` [PATCH proxmox 2/7] notify (smtp): Introduce state module Arthur Bied-Charreton
2026-03-23 12:26 ` Lukas Wagner
2026-03-23 16:32 ` Arthur Bied-Charreton
2026-03-24 8:50 ` Arthur Bied-Charreton
2026-02-13 16:04 ` [PATCH proxmox 3/7] notify (smtp): Factor out transport building logic into own function Arthur Bied-Charreton
2026-02-13 16:04 ` [PATCH proxmox 4/7] notify (smtp): Update API with OAuth2 parameters Arthur Bied-Charreton
2026-02-13 16:04 ` [PATCH proxmox 5/7] notify (smtp): Add state handling logic Arthur Bied-Charreton
2026-03-23 12:26 ` Lukas Wagner
2026-02-13 16:04 ` [PATCH proxmox 6/7] notify (smtp): Add XOAUTH2 authentication support Arthur Bied-Charreton
2026-03-23 12:26 ` Lukas Wagner
2026-02-13 16:04 ` [PATCH proxmox 7/7] notify (smtp): Add logging and state-related error types Arthur Bied-Charreton
2026-03-23 12:26 ` Lukas Wagner
2026-02-13 16:04 ` [PATCH proxmox-perl-rs 1/1] notify (smtp): add oauth2 parameters to bindings Arthur Bied-Charreton
2026-03-23 12:26 ` Lukas Wagner
2026-03-23 16:44 ` Arthur Bied-Charreton
2026-02-13 16:04 ` [PATCH proxmox-widget-toolkit 1/2] utils: Add OAuth2 flow handlers Arthur Bied-Charreton
2026-03-23 12:26 ` Lukas Wagner
2026-02-13 16:04 ` [PATCH proxmox-widget-toolkit 2/2] notifications: Add opt-in OAuth2 support for SMTP targets Arthur Bied-Charreton
2026-03-23 12:26 ` Lukas Wagner
2026-03-23 16:49 ` Arthur Bied-Charreton
2026-02-13 16:04 ` [PATCH pve-manager 1/5] notifications: Add OAuth2 parameters to schema and add/update endpoints Arthur Bied-Charreton
2026-03-23 12:26 ` Lukas Wagner
2026-02-13 16:04 ` [PATCH pve-manager 2/5] notifications: Add trigger-state-refresh endpoint Arthur Bied-Charreton
2026-03-23 12:26 ` Lukas Wagner
2026-02-13 16:04 ` [PATCH pve-manager 3/5] notifications: Trigger notification target refresh in pveupdate Arthur Bied-Charreton
2026-02-13 16:04 ` [PATCH pve-manager 4/5] notifications: Handle OAuth2 callback in login handler Arthur Bied-Charreton
2026-02-13 16:04 ` [PATCH pve-manager 5/5] notifications: Opt into OAuth2 authentication Arthur Bied-Charreton
2026-02-13 16:04 ` [PATCH pve-cluster 1/1] notifications: Add refresh_targets subroutine to PVE::Notify Arthur Bied-Charreton
2026-03-23 12:26 ` Lukas Wagner
2026-03-23 16:54 ` Arthur Bied-Charreton
2026-02-13 16:04 ` [PATCH pve-docs 1/1] notifications: Add section about OAuth2 to SMTP targets docs Arthur Bied-Charreton
2026-03-23 12:25 ` [PATCH cluster/docs/manager/proxmox{,-perl-rs,-widget-toolkit} 00/17] fix #7238: Add XOAUTH2 authentication support for SMTP notification targets Lukas Wagner
2026-03-25 13:16 ` superseded: " Arthur Bied-Charreton
-- strict thread matches above, loose matches on Subject: below --
2026-02-04 16:13 [RFC cluster/docs/manager/proxmox{,-perl-rs,-widget-toolkit} 00/15] " Arthur Bied-Charreton
2026-02-04 16:13 ` [PATCH pve-manager 1/5] notifications: Add OAuth2 parameters to schema and add/update endpoints Arthur Bied-Charreton
2026-02-11 8:55 ` Lukas Wagner
2026-02-11 12:47 ` Arthur Bied-Charreton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox