Compare commits

...

4 Commits

Author SHA1 Message Date
phundrak 931b122ee3 docs: clarify documentation
Publish Docker Images / build-docker (push) Successful in 8m34s
Publish Docker Images / coverage-and-sonar (push) Failing after 8m53s
Publish Docker Images / push-docker (push) Has been skipped
2026-06-06 15:49:19 +02:00
phundrak 9a24322cf0 feat(starttls): remove opportunistic value
This commit removes the `Opportunistic` value from the struct `StartTls`.
This value was strictly equivalent to `Always` and could potentially
cause confusion.
2026-06-06 15:49:19 +02:00
phundrak 2d00628598 chore(ci): add linting, formatting check, and auditing 2026-06-06 15:49:19 +02:00
phundrak 3c86e9eb36 fix(server): fix TOCTOU race condition in tests 2026-06-06 15:49:19 +02:00
6 changed files with 60 additions and 91 deletions
+14 -2
View File
@@ -40,9 +40,21 @@ jobs:
authToken: '${{ secrets.CACHIX_AUTH_TOKEN }}' authToken: '${{ secrets.CACHIX_AUTH_TOKEN }}'
skipPush: ${{ github.event_name == 'pull_request' }} skipPush: ${{ github.event_name == 'pull_request' }}
- name: Format Check
shell: bash -c "nix develop --no-pure-eval --accept-flake-config --command {0}"
run: just format-check
- name: Audit
shell: bash -c "nix develop --no-pure-eval --accept-flake-config --command {0}"
run: just audit
- name: Lint
shell: bash -c "nix develop --no-pure-eval --accept-flake-config --command {0}"
run: just lint-report
- name: Coverage - name: Coverage
run: | shell: bash -c "nix develop --no-pure-eval --accept-flake-config --command {0}"
nix develop --no-pure-eval --accept-flake-config --command just coverage run: just coverage-ci
- name: Sonar analysis - name: Sonar analysis
uses: SonarSource/sonarqube-scan-action@v6 uses: SonarSource/sonarqube-scan-action@v6
+1 -1
View File
@@ -321,7 +321,7 @@ backend/
The contact form supports multiple SMTP configurations: The contact form supports multiple SMTP configurations:
- **Implicit TLS (SMTPS)** - typically port 465 - **Implicit TLS (SMTPS)** - typically port 465
- **STARTTLS (Always/Opportunistic)** - typically port 587 - **STARTTLS (Always)** - typically port 587
- **Unencrypted** (for local dev) - with or without authentication - **Unencrypted** (for local dev) - with or without authentication
The `SmtpTransport` is built dynamically from `EmailSettings` based on The `SmtpTransport` is built dynamically from `EmailSettings` based on
+3 -6
View File
@@ -24,7 +24,7 @@ pub mod startup;
/// Logging and tracing setup /// Logging and tracing setup
pub mod telemetry; pub mod telemetry;
type MaybeListener = Option<poem::listener::TcpListener<String>>; type MaybeListener = Option<std::net::TcpListener>;
fn prepare(listener: MaybeListener) -> startup::Application { fn prepare(listener: MaybeListener) -> startup::Application {
dotenvy::dotenv().ok(); dotenvy::dotenv().ok();
@@ -70,11 +70,8 @@ pub async fn run(listener: MaybeListener) -> Result<(), std::io::Error> {
} }
#[cfg(test)] #[cfg(test)]
fn make_random_tcp_listener() -> poem::listener::TcpListener<String> { fn make_random_tcp_listener() -> std::net::TcpListener {
let tcp_listener = std::net::TcpListener::bind("127.0.0.1:0").expect("Failed to bind a random TCP listener")
std::net::TcpListener::bind("127.0.0.1:0").expect("Failed to bind a random TCP listener");
let port = tcp_listener.local_addr().unwrap().port();
poem::listener::TcpListener::bind(format!("127.0.0.1:{port}"))
} }
#[cfg(test)] #[cfg(test)]
+1 -18
View File
@@ -62,7 +62,7 @@ impl TryFrom<&EmailSettings> for SmtpTransport {
Ok(builder.credentials(creds).build()) Ok(builder.credentials(creds).build())
} }
} }
Starttls::Opportunistic | Starttls::Always => { Starttls::Always => {
// STARTTLS - typically port 587 // STARTTLS - typically port 587
tracing::event!(target: "backend::contact", tracing::Level::DEBUG, "Using STARTTLS"); tracing::event!(target: "backend::contact", tracing::Level::DEBUG, "Using STARTTLS");
let creds = Credentials::new(settings.user.clone(), settings.password.clone()); let creds = Credentials::new(settings.user.clone(), settings.password.clone());
@@ -429,23 +429,6 @@ mod tests {
assert!(result.is_ok()); assert!(result.is_ok());
} }
#[test]
fn smtp_transport_starttls_opportunistic() {
let settings = EmailSettings {
host: "smtp.example.com".to_string(),
port: 587,
user: "user@example.com".to_string(),
password: "password".to_string(),
from: "from@example.com".to_string(),
recipient: "to@example.com".to_string(),
tls: false,
starttls: Starttls::Opportunistic,
};
let result = SmtpTransport::try_from(&settings);
assert!(result.is_ok());
}
#[test] #[test]
fn smtp_transport_no_encryption_with_credentials() { fn smtp_transport_no_encryption_with_credentials() {
let settings = EmailSettings { let settings = EmailSettings {
+5 -41
View File
@@ -163,8 +163,9 @@ impl EmailSettings {
/// ///
/// # Errors /// # Errors
/// ///
/// Returns a `ContactError` if the email address in the `from` field cannot be parsed /// Returns a `ContactError` if the email address in the `from`
/// into a valid mailbox. This can occur if: /// field of `recipient` cannot be parsed into a valid mailbox.
/// This can occur if:
/// - The email address format is invalid /// - The email address format is invalid
/// - The email address contains invalid characters /// - The email address contains invalid characters
/// - The email address structure is malformed /// - The email address structure is malformed
@@ -196,8 +197,6 @@ pub enum Starttls {
/// Never use STARTTLS (unencrypted connection) /// Never use STARTTLS (unencrypted connection)
#[default] #[default]
Never, Never,
/// Use STARTTLS if available (opportunistic encryption)
Opportunistic,
/// Always use STARTTLS (required encryption) /// Always use STARTTLS (required encryption)
Always, Always,
} }
@@ -208,10 +207,9 @@ impl TryFrom<&str> for Starttls {
fn try_from(value: &str) -> Result<Self, Self::Error> { fn try_from(value: &str) -> Result<Self, Self::Error> {
match value.to_lowercase().as_str() { match value.to_lowercase().as_str() {
"off" | "no" | "never" => Ok(Self::Never), "off" | "no" | "never" => Ok(Self::Never),
"opportunistic" => Ok(Self::Opportunistic),
"yes" | "always" => Ok(Self::Always), "yes" | "always" => Ok(Self::Always),
other => Err(format!( other => Err(format!(
"{other} is not a supported option. Use either `yes`, `no`, or `opportunistic`" "{other} is not a supported option. Use either `yes` or `no`"
)), )),
} }
} }
@@ -234,7 +232,6 @@ impl std::fmt::Display for Starttls {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let self_str = match self { let self_str = match self {
Self::Never => "never", Self::Never => "never",
Self::Opportunistic => "opportunistic",
Self::Always => "always", Self::Always => "always",
}; };
write!(f, "{self_str}") write!(f, "{self_str}")
@@ -252,7 +249,7 @@ impl<'de> serde::Deserialize<'de> for Starttls {
type Value = Starttls; type Value = Starttls;
fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
formatter.write_str("a string or boolean representing STARTTLS setting (e.g., 'yes', 'no', 'opportunistic', true, false)") formatter.write_str("a string or boolean representing STARTTLS setting (e.g., 'yes', 'no', true, false)")
} }
fn visit_str<E>(self, value: &str) -> Result<Starttls, E> fn visit_str<E>(self, value: &str) -> Result<Starttls, E>
@@ -434,13 +431,6 @@ mod tests {
assert_eq!(result, Starttls::Always); assert_eq!(result, Starttls::Always);
} }
#[test]
fn startls_deserialize_from_string_opportunistic() {
let json = r#""opportunistic""#;
let result: Starttls = serde_json::from_str(json).unwrap();
assert_eq!(result, Starttls::Opportunistic);
}
#[test] #[test]
fn startls_deserialize_from_bool() { fn startls_deserialize_from_bool() {
let json = "true"; let json = "true";
@@ -482,18 +472,6 @@ mod tests {
assert_eq!(Starttls::try_from("Yes").unwrap(), Starttls::Always); assert_eq!(Starttls::try_from("Yes").unwrap(), Starttls::Always);
} }
#[test]
fn startls_try_from_str_opportunistic() {
assert_eq!(
Starttls::try_from("opportunistic").unwrap(),
Starttls::Opportunistic
);
assert_eq!(
Starttls::try_from("OPPORTUNISTIC").unwrap(),
Starttls::Opportunistic
);
}
#[test] #[test]
fn startls_try_from_str_invalid() { fn startls_try_from_str_invalid() {
let result = Starttls::try_from("invalid"); let result = Starttls::try_from("invalid");
@@ -517,14 +495,6 @@ mod tests {
); );
} }
#[test]
fn startls_try_from_string_opportunistic() {
assert_eq!(
Starttls::try_from("opportunistic".to_string()).unwrap(),
Starttls::Opportunistic
);
}
#[test] #[test]
fn startls_try_from_string_invalid() { fn startls_try_from_string_invalid() {
let result = Starttls::try_from("invalid".to_string()); let result = Starttls::try_from("invalid".to_string());
@@ -553,12 +523,6 @@ mod tests {
assert_eq!(startls.to_string(), "always"); assert_eq!(startls.to_string(), "always");
} }
#[test]
fn startls_display_opportunistic() {
let startls = Starttls::Opportunistic;
assert_eq!(startls.to_string(), "opportunistic");
}
#[test] #[test]
fn rate_limit_settings_default() { fn rate_limit_settings_default() {
let settings = RateLimitSettings::default(); let settings = RateLimitSettings::default();
+36 -23
View File
@@ -6,6 +6,7 @@
//! - Configuring CORS //! - Configuring CORS
//! - Starting the HTTP server //! - Starting the HTTP server
use poem::listener::{Listener, TcpAcceptor};
use poem::middleware::{AddDataEndpoint, Cors, CorsEndpoint}; use poem::middleware::{AddDataEndpoint, Cors, CorsEndpoint};
use poem::{EndpointExt, Route}; use poem::{EndpointExt, Route};
use poem_openapi::OpenApiService; use poem_openapi::OpenApiService;
@@ -19,10 +20,21 @@ use crate::{
use crate::middleware::rate_limit::RateLimitEndpoint; use crate::middleware::rate_limit::RateLimitEndpoint;
type Server = poem::Server<poem::listener::TcpListener<String>, std::convert::Infallible>; type Server = poem::Server<poem::listener::BoxListener, std::convert::Infallible>;
/// The configured application with rate limiting, CORS, and settings data. /// The configured application with rate limiting, CORS, and settings data.
pub type App = AddDataEndpoint<CorsEndpoint<RateLimitEndpoint<Route>>, Settings>; pub type App = AddDataEndpoint<CorsEndpoint<RateLimitEndpoint<Route>>, Settings>;
struct PreBoundListener(std::net::TcpListener);
impl Listener for PreBoundListener {
type Acceptor = TcpAcceptor;
async fn into_acceptor(self) -> std::io::Result<Self::Acceptor> {
TcpAcceptor::from_std(self.0)
}
}
/// Application builder that holds the server configuration before running. /// Application builder that holds the server configuration before running.
pub struct Application { pub struct Application {
server: Server, server: Server,
@@ -135,30 +147,33 @@ impl Application {
fn setup_server( fn setup_server(
settings: &Settings, settings: &Settings,
tcp_listener: Option<poem::listener::TcpListener<String>>, tcp_listener: Option<std::net::TcpListener>,
) -> Server { ) -> (Server, u16, String) {
let tcp_listener = tcp_listener.unwrap_or_else(|| { tcp_listener.map_or_else(
let address = format!( || {
"{}:{}", let port = settings.application.port;
settings.application.host, settings.application.port let host = settings.application.host.clone();
); let address = format!("{host}:{port}");
poem::listener::TcpListener::bind(address) let server = poem::Server::new(poem::listener::TcpListener::bind(address).boxed());
}); (server, port, host)
poem::Server::new(tcp_listener) },
|listener| {
let addr = listener.local_addr().expect("Failed to get bound address");
let port = addr.port();
let host = addr.ip().to_string();
let server = poem::Server::new(PreBoundListener(listener).boxed());
(server, port, host)
},
)
} }
/// Builds a new application with the given settings and optional TCP listener. /// Builds a new application with the given settings and optional TCP listener.
/// ///
/// If no listener is provided, one will be created based on the settings. /// If no listener is provided, one will be created based on the settings.
#[must_use] #[must_use]
pub fn build( pub fn build(settings: Settings, tcp_listener: Option<std::net::TcpListener>) -> Self {
settings: Settings, let (server, port, host) = Self::setup_server(&settings, tcp_listener);
tcp_listener: Option<poem::listener::TcpListener<String>>,
) -> Self {
let port = settings.application.port;
let host = settings.application.host.clone();
let app = Self::setup_app(&settings); let app = Self::setup_app(&settings);
let server = Self::setup_server(&settings, tcp_listener);
Self { Self {
server, server,
app, app,
@@ -243,13 +258,11 @@ mod tests {
#[test] #[test]
fn application_with_custom_listener() { fn application_with_custom_listener() {
let settings = create_test_settings(); let settings = create_test_settings();
let tcp_listener = let listener =
std::net::TcpListener::bind("127.0.0.1:0").expect("Failed to bind random port"); std::net::TcpListener::bind("127.0.0.1:0").expect("Failed to bind random port");
let port = tcp_listener.local_addr().unwrap().port(); let expected_port = listener.local_addr().unwrap().port();
let listener = poem::listener::TcpListener::bind(format!("127.0.0.1:{port}"));
let app = Application::build(settings, Some(listener)); let app = Application::build(settings, Some(listener));
assert_eq!(app.host(), "127.0.0.1"); assert_eq!(app.host(), "127.0.0.1");
assert_eq!(app.port(), 8080); assert_eq!(app.port(), expected_port);
} }
} }