Merge pull request #320 from EdouardVanbelle/feat/client_ip

Cargo.toml

@@ -25,7 +25,7 @@ serde_json = "1.0.149"
 futures = "0.3.32"
 async-stream = "0.3.6"
 mime_guess = "2.0.5"
-uuid = { version = "1.23.0", features = ["v4", "serde"] }
+uuid = { version = "1.23.0", features = ["v4", "v7", "serde"] }
 thiserror = "2.0.18"
 
 mockall = { version = "0.14.0", optional = true }

example.env

@@ -201,3 +201,16 @@ MIMALLOC_PURGE_DELAY=0
 # When enabled with Linux Transparent Huge Pages (THP), partially-used 2 MiB
 # pages inflate the reported RSS by up to 20-30 MiB.
 MIMALLOC_ALLOW_LARGE_OS_PAGES=0
+
+# -----------------------------------------------------------------------------
+# PROXY
+# -----------------------------------------------------------------------------
+
+# Use this section if you are running OxiCloud behind a proxy
+
+# Trusted Proxy IPs. Format: coma separated list of CIDR
+# (default not defined = server without proxy)
+# if defined and proxy's IPs match, client_ip will be defined from
+# `X-Forwarded-For` / `X-Real-Ip`
+#OXICLOUD_TRUST_PROXY_CIDR=192.168.0.1/32,10.1.2.0/24
+

src/interfaces/api/handlers/favorites_handler.rs

@@ -43,7 +43,11 @@ pub async fn get_favorites(
 
     match favorites_service.get_favorites(user_id).await {
         Ok(favorites) => {
-            info!("Retrieved {} favorites for user", favorites.len());
+            info!(
+                "Retrieved {} favorites for user {}",
+                favorites.len(),
+                auth_user.id
+            );
             (StatusCode::OK, Json(serde_json::json!(favorites))).into_response()
         }
         Err(err) => {

src/interfaces/middleware/auth.rs

@@ -188,6 +188,7 @@ pub async fn auth_middleware(
                                 role: claims.role,
                             });
                             request.extensions_mut().insert(current_user);
+                            tracing::Span::current().record("user_id", user_id.to_string());
                             return Ok(next.run(request).await);
                         }
                         Err(e) => {
@@ -234,6 +235,7 @@ pub async fn auth_middleware(
                                 role,
                             });
                             request.extensions_mut().insert(current_user);
+                            tracing::Span::current().record("user_id", user_id.to_string());
                             return Ok(next.run(request).await);
                         }
                         Err(e) => {
@@ -291,6 +293,7 @@ pub async fn auth_middleware(
                         });
                         request.extensions_mut().insert(current_user);
                         request.extensions_mut().insert(CookieAuthenticated);
+                        tracing::Span::current().record("user_id", user_id.to_string());
                         return Ok(next.run(request).await);
                     }
                     Err(e) => {

src/interfaces/middleware/mod.rs

@@ -1,3 +1,5 @@
 pub mod auth;
 pub mod csrf;
 pub mod rate_limit;
+pub mod trace_span;
+pub mod trusted_proxy;

src/interfaces/middleware/rate_limit.rs

@@ -4,29 +4,21 @@
 //! counts per client IP.  Each protected endpoint group gets its own
 //! [`RateLimiter`] instance with independently tuneable limits.
 //!
-//! The middleware extracts the client IP from (in order):
-//! 1. `X-Forwarded-For` header (first entry — set by reverse proxies)
-//! 2. `X-Real-Ip` header
-//! 3. The TCP peer address from the connection info
+//! Client IP resolution is delegated to [`super::trusted_proxy::client_ip`],
+//! which honours `OXICLOUD_TRUST_PROXY_CIDR` for proxy-header forwarding.
 //!
 //! When the limit is exceeded a `429 Too Many Requests` response is returned
 //! with a `Retry-After` header indicating how many seconds to wait.
 
 use axum::{
-    extract::ConnectInfo,
     http::{HeaderValue, Request, StatusCode},
     middleware::Next,
     response::{IntoResponse, Response},
 };
 use moka::sync::Cache;
-use std::net::SocketAddr;
-use std::sync::{Arc, OnceLock};
+use std::sync::Arc;
 use std::time::Duration;
 
-/// Cached value of `OXICLOUD_TRUST_PROXY_HEADERS` env var.
-/// Read once on first access, never again — avoids a syscall per request.
-static TRUST_PROXY: OnceLock<bool> = OnceLock::new();
-
 /// A simple sliding-window counter keyed by IP address.
 ///
 /// Each key lives for `window` seconds; every request increments the counter.
@@ -94,46 +86,11 @@ impl RateLimiter {
 
 /// Extract the most-likely real client IP from headers / connection info.
 ///
-/// Proxy headers (`X-Forwarded-For`, `X-Real-Ip`) are only trusted when
-/// `OXICLOUD_TRUST_PROXY_HEADERS=true` is set.  Without a trusted reverse
-/// proxy in front of the app, an attacker can spoof these headers to bypass
-/// rate limiting.
+/// Proxy headers (`X-Forwarded-For`, `X-Real-Ip`) are only trusted when the
+/// TCP peer address falls within `OXICLOUD_TRUST_PROXY_CIDR`.  Without a
+/// configured CIDR list an attacker could spoof headers to bypass rate limiting.
 pub fn extract_client_ip<B>(req: &Request<B>) -> String {
-    let trust_proxy = *TRUST_PROXY.get_or_init(|| {
-        std::env::var("OXICLOUD_TRUST_PROXY_HEADERS")
-            .map(|v| v == "true" || v == "1")
-            .unwrap_or(false)
-    });
-
-    let headers = req.headers();
-
-    if trust_proxy {
-        // 1. X-Forwarded-For (first entry — closest to the client)
-        if let Some(xff) = headers.get("x-forwarded-for").and_then(|v| v.to_str().ok())
-            && let Some(first) = xff.split(',').next()
-        {
-            let ip = first.trim();
-            if !ip.is_empty() {
-                return ip.to_string();
-            }
-        }
-
-        // 2. X-Real-Ip
-        if let Some(xri) = headers.get("x-real-ip").and_then(|v| v.to_str().ok()) {
-            let ip = xri.trim();
-            if !ip.is_empty() {
-                return ip.to_string();
-            }
-        }
-    }
-
-    // 3. TCP peer (ConnectInfo extension set by axum::serve)
-    if let Some(addr) = req.extensions().get::<ConnectInfo<SocketAddr>>() {
-        return addr.0.ip().to_string();
-    }
-
-    // Fallback — should never happen behind axum::serve
-    "unknown".to_string()
+    super::trusted_proxy::client_ip(req, false)
 }
 
 /// Build a rate-limit response with the standard `Retry-After` header.

src/interfaces/middleware/trace_span.rs

@@ -0,0 +1,81 @@
+//! Custom [`MakeSpan`], [`OnResponse`], and [`MakeRequestId`] for request tracing.
+//!
+//! [`UuidRequestId`] — generates a UUID v4 per request for `SetRequestIdLayer`.
+//!
+//! [`ClientIpMakeSpan`] — records `request_id`, `client_ip`, `method`, `uri`,
+//! and a placeholder `user_id` (filled by auth middleware) on every request span.
+//!
+//! [`LogBadRequest`] — emits a WARN for every HTTP 400 response, inheriting
+//! all span fields so the log line includes request ID, IP, user, method, URI.
+
+use axum::http::{HeaderValue, Request, Response, StatusCode};
+use std::time::Duration;
+use tower_http::request_id::{MakeRequestId, RequestId};
+use tower_http::trace::{MakeSpan, OnResponse};
+use tracing::Span;
+use uuid::Uuid;
+
+// ─── Request ID generator ────────────────────────────────────────────────────
+
+/// Generates a UUID v7 (fast, timed, sortable) for each request.
+///
+/// Used with [`tower_http::request_id::SetRequestIdLayer`]:
+/// ```ignore
+/// .layer(SetRequestIdLayer::x_request_id(UuidRequestId))
+/// ```
+#[derive(Clone, Debug, Default)]
+pub struct UuidRequestId;
+
+impl MakeRequestId for UuidRequestId {
+    fn make_request_id<B>(&mut self, _request: &Request<B>) -> Option<RequestId> {
+        let id = Uuid::now_v7().to_string();
+        HeaderValue::from_str(&id).ok().map(RequestId::new)
+    }
+}
+
+// ─── Span factory ────────────────────────────────────────────────────────────
+
+/// Implements [`MakeSpan`] so that every HTTP request span carries
+/// `request_id`, `client_ip`, `method`, `uri`, and a deferred `user_id`.
+///
+/// `request_id` is read from the `x-request-id` header set by
+/// [`tower_http::request_id::SetRequestIdLayer`] (which must wrap this layer).
+#[derive(Clone, Debug, Default)]
+pub struct ClientIpMakeSpan;
+
+impl<B> MakeSpan<B> for ClientIpMakeSpan {
+    fn make_span(&mut self, request: &Request<B>) -> Span {
+        let ip = super::trusted_proxy::client_ip(request, true);
+        let request_id = request
+            .headers()
+            .get("x-request-id")
+            .and_then(|v| v.to_str().ok())
+            .unwrap_or("-");
+        tracing::info_span!(
+            "req",
+            request_id = request_id,
+            client_ip  = %ip,
+            method     = %request.method(),
+            uri        = %request.uri().path(),
+            user_id    = tracing::field::Empty,
+        )
+    }
+}
+
+// ─── Response observer ───────────────────────────────────────────────────────
+
+/// Implements [`OnResponse`]: emits a WARN log for every HTTP 400 response.
+#[derive(Clone, Debug, Default)]
+pub struct LogBadRequest;
+
+impl<B> OnResponse<B> for LogBadRequest {
+    fn on_response(self, response: &Response<B>, latency: Duration, _span: &Span) {
+        if response.status() == StatusCode::BAD_REQUEST {
+            tracing::warn!(
+                status = 400,
+                latency_ms = latency.as_millis(),
+                "bad request",
+            );
+        }
+    }
+}

src/interfaces/middleware/trusted_proxy.rs

@@ -0,0 +1,243 @@
+//! Trusted-proxy CIDR list and client IP resolution.
+//!
+//! Set `OXICLOUD_TRUST_PROXY_CIDR` to a comma-separated list of CIDR blocks
+//! whose source IPs are trusted to set `X-Forwarded-For` / `X-Real-Ip`:
+//!
+//! ```text
+//! OXICLOUD_TRUST_PROXY_CIDR=127.0.0.1/32,10.0.0.0/8,172.16.0.0/12,::1/128
+//! ```
+//!
+//! When the TCP peer address falls inside one of those CIDRs the leftmost
+//! entry of `X-Forwarded-For` (or `X-Real-Ip`) is used as the effective
+//! client IP.  When the peer is **not** trusted, or no proxy headers are
+//! present, the raw TCP peer address is returned.
+//!
+//! IPv4-mapped IPv6 addresses (`::ffff:x.x.x.x`) are automatically
+//! normalised to their IPv4 equivalent before CIDR lookup.
+
+use axum::extract::ConnectInfo;
+use axum::http::Request;
+use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr};
+use std::sync::OnceLock;
+
+// ─── CIDR list ───────────────────────────────────────────────────────────────
+
+static TRUSTED_CIDRS: OnceLock<Vec<(IpAddr, u8)>> = OnceLock::new();
+
+fn trusted_cidrs() -> &'static [(IpAddr, u8)] {
+    TRUSTED_CIDRS.get_or_init(|| {
+        let mut cidrs: Vec<(IpAddr, u8)> = std::env::var("OXICLOUD_TRUST_PROXY_CIDR")
+            .unwrap_or_default()
+            .split(',')
+            .filter_map(|s| parse_cidr(s.trim()))
+            .collect();
+
+        // Backward-compat: OXICLOUD_TRUST_PROXY_HEADERS=true trusts all source IPs.
+        let legacy = std::env::var("OXICLOUD_TRUST_PROXY_HEADERS")
+            .map(|v| v == "true" || v == "1")
+            .unwrap_or(false);
+        if legacy {
+            tracing::warn!(
+                "OXICLOUD_TRUST_PROXY_HEADERS is deprecated — \
+                 use OXICLOUD_TRUST_PROXY_CIDR to restrict trusted proxy source IPs. \
+                 Falling back to trust all IPs (0.0.0.0/0 and ::/0)."
+            );
+            cidrs.push(("0.0.0.0".parse().unwrap(), 0));
+            cidrs.push(("::".parse().unwrap(), 0));
+        }
+
+        cidrs
+    })
+}
+
+fn parse_cidr(s: &str) -> Option<(IpAddr, u8)> {
+    let (addr_s, prefix_s) = s.split_once('/')?;
+    let addr: IpAddr = addr_s.parse().ok()?;
+    let prefix: u8 = prefix_s.parse().ok()?;
+    let max = if addr.is_ipv4() { 32 } else { 128 };
+    if prefix > max {
+        return None;
+    }
+    Some((addr, prefix))
+}
+
+// ─── CIDR matching ───────────────────────────────────────────────────────────
+
+fn ipv4_in_cidr(base: Ipv4Addr, prefix: u8, ip: Ipv4Addr) -> bool {
+    if prefix == 0 {
+        return true;
+    }
+    let shift = 32 - u32::from(prefix);
+    let mask = !0u32 << shift;
+    (u32::from(base) & mask) == (u32::from(ip) & mask)
+}
+
+fn ipv6_in_cidr(base: Ipv6Addr, prefix: u8, ip: Ipv6Addr) -> bool {
+    if prefix == 0 {
+        return true;
+    }
+    let shift = 128 - u32::from(prefix);
+    let mask = !0u128 << shift;
+    (u128::from(base) & mask) == (u128::from(ip) & mask)
+}
+
+/// Normalise an IP to IPv4 if it is an IPv4-mapped IPv6 address.
+fn normalise(ip: IpAddr) -> IpAddr {
+    if let IpAddr::V6(v6) = ip
+        && let Some(v4) = v6.to_ipv4_mapped()
+    {
+        return IpAddr::V4(v4);
+    }
+    ip
+}
+
+fn cidr_contains(base: IpAddr, prefix: u8, target: IpAddr) -> bool {
+    let base = normalise(base);
+    let target = normalise(target);
+    match (base, target) {
+        (IpAddr::V4(b), IpAddr::V4(t)) => ipv4_in_cidr(b, prefix, t),
+        (IpAddr::V6(b), IpAddr::V6(t)) => ipv6_in_cidr(b, prefix, t),
+        _ => false,
+    }
+}
+
+/// Logs the loaded CIDR list at INFO level.  Call once at startup, after the
+/// tracing subscriber is initialised, to make the effective configuration
+/// visible in the logs.
+pub fn log_config() {
+    let cidrs = trusted_cidrs();
+    if cidrs.is_empty() {
+        tracing::info!(
+            "trusted proxy CIDRs: none — X-Forwarded-For/X-Real-Ip headers will be ignored"
+        );
+    } else {
+        let list: Vec<String> = cidrs
+            .iter()
+            .map(|(addr, prefix)| format!("{addr}/{prefix}"))
+            .collect();
+        tracing::info!("trusted proxy CIDRs: {}", list.join(", "));
+    }
+}
+
+/// Returns `true` when `peer` falls inside one of the configured CIDR ranges.
+pub fn is_trusted_proxy(peer: IpAddr) -> bool {
+    let cidrs = trusted_cidrs();
+    if cidrs.is_empty() {
+        return false;
+    }
+    cidrs
+        .iter()
+        .any(|&(base, prefix)| cidr_contains(base, prefix, peer))
+}
+
+// ─── IP extraction ───────────────────────────────────────────────────────────
+
+/// Resolve the effective client IP for a request.
+///
+/// * `include_port` — when `true` the returned string for direct (non-proxied)
+///   connections includes the port, e.g. `"127.0.0.1:12345"`.  Pass `false`
+///   for contexts that only need the bare IP (e.g. rate-limiting keys).
+pub fn client_ip<B>(req: &Request<B>, include_port: bool) -> String {
+    let peer: Option<SocketAddr> = req
+        .extensions()
+        .get::<ConnectInfo<SocketAddr>>()
+        .map(|ci| ci.0);
+
+    if let Some(peer_addr) = peer {
+        if is_trusted_proxy(peer_addr.ip()) {
+            // Try X-Forwarded-For first (leftmost = original client)
+            if let Some(xff) = req
+                .headers()
+                .get("x-forwarded-for")
+                .and_then(|v| v.to_str().ok())
+                && let Some(ip) = xff
+                    .split(',')
+                    .next()
+                    .map(str::trim)
+                    .filter(|s| !s.is_empty())
+            {
+                return ip.to_string();
+            }
+
+            // Then X-Real-Ip
+            if let Some(xri) = req
+                .headers()
+                .get("x-real-ip")
+                .and_then(|v| v.to_str().ok())
+                .map(str::trim)
+                .filter(|s| !s.is_empty())
+            {
+                return xri.to_string();
+            }
+        }
+
+        return if include_port {
+            peer_addr.to_string()
+        } else {
+            peer_addr.ip().to_string()
+        };
+    }
+
+    "unknown".to_string()
+}
+
+// ─── Tests ───────────────────────────────────────────────────────────────────
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn ipv4_cidr_host() {
+        let base: Ipv4Addr = "192.168.1.1".parse().unwrap();
+        assert!(ipv4_in_cidr(base, 32, "192.168.1.1".parse().unwrap()));
+        assert!(!ipv4_in_cidr(base, 32, "192.168.1.2".parse().unwrap()));
+    }
+
+    #[test]
+    fn ipv4_cidr_subnet() {
+        let base: Ipv4Addr = "10.0.0.0".parse().unwrap();
+        assert!(ipv4_in_cidr(base, 8, "10.255.255.255".parse().unwrap()));
+        assert!(!ipv4_in_cidr(base, 8, "11.0.0.1".parse().unwrap()));
+    }
+
+    #[test]
+    fn ipv4_cidr_any() {
+        let base: Ipv4Addr = "0.0.0.0".parse().unwrap();
+        assert!(ipv4_in_cidr(base, 0, "1.2.3.4".parse().unwrap()));
+    }
+
+    #[test]
+    fn ipv6_cidr_loopback() {
+        let base: Ipv6Addr = "::1".parse().unwrap();
+        assert!(ipv6_in_cidr(base, 128, "::1".parse().unwrap()));
+        assert!(!ipv6_in_cidr(base, 128, "::2".parse().unwrap()));
+    }
+
+    #[test]
+    fn ipv4_mapped_matches_v4_cidr() {
+        // ::ffff:127.0.0.1 should match 127.0.0.1/8
+        let base = IpAddr::V4("127.0.0.0".parse().unwrap());
+        let mapped = IpAddr::V6("::ffff:127.0.0.1".parse().unwrap());
+        assert!(cidr_contains(base, 8, mapped));
+    }
+
+    #[test]
+    fn parse_cidr_valid() {
+        assert_eq!(
+            parse_cidr("10.0.0.0/8"),
+            Some((IpAddr::V4("10.0.0.0".parse().unwrap()), 8))
+        );
+        assert_eq!(
+            parse_cidr("::1/128"),
+            Some((IpAddr::V6("::1".parse().unwrap()), 128))
+        );
+    }
+
+    #[test]
+    fn parse_cidr_invalid_prefix() {
+        assert!(parse_cidr("10.0.0.0/33").is_none());
+        assert!(parse_cidr("::1/129").is_none());
+        assert!(parse_cidr("notanip/8").is_none());
+    }
+}

src/main.rs

@@ -12,7 +12,11 @@ use socket2::{Domain, Protocol, Socket, TcpKeepalive, Type};
 
 use axum::Router;
 use axum::extract::DefaultBodyLimit;
+use oxicloud::interfaces::middleware::trace_span::{
+    ClientIpMakeSpan, LogBadRequest, UuidRequestId,
+};
 use tower_http::limit::RequestBodyLimitLayer;
+use tower_http::request_id::{PropagateRequestIdLayer, SetRequestIdLayer};
 use tower_http::set_header::SetResponseHeaderLayer;
 use tower_http::trace::TraceLayer;
 use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt};
@@ -61,6 +65,8 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
         .with(tracing_subscriber::fmt::layer())
         .init();
 
+    oxicloud::interfaces::middleware::trusted_proxy::log_config();
+
     // Load configuration from environment variables
     let config = common::config::AppConfig::from_env();
 
@@ -69,8 +75,6 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
     if !storage_path.exists() {
         std::fs::create_dir_all(&storage_path).expect("Failed to create storage directory");
     }
-    // Locales are embedded in the binary via rust-embed — no filesystem path needed.
-
     // Initialize database pools if auth is enabled
     let db_pools = if config.features.enable_auth {
         match create_database_pools(&config).await {
@@ -342,7 +346,13 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
             .merge(carddav_protected)
             .merge(webdav_protected)
             .merge(web_routes)
-            .layer(TraceLayer::new_for_http());
+            .layer(
+                TraceLayer::new_for_http()
+                    .make_span_with(ClientIpMakeSpan)
+                    .on_response(LogBadRequest),
+            )
+            .layer(PropagateRequestIdLayer::x_request_id())
+            .layer(SetRequestIdLayer::x_request_id(UuidRequestId));
 
         // Mount Nextcloud routes (uses its own Basic Auth middleware)
         if let Some(nc_router) = nextcloud_router {
@@ -374,7 +384,13 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
             .merge(carddav_router)
             .merge(webdav_router)
             .merge(web_routes)
-            .layer(TraceLayer::new_for_http());
+            .layer(
+                TraceLayer::new_for_http()
+                    .make_span_with(ClientIpMakeSpan)
+                    .on_response(LogBadRequest),
+            )
+            .layer(PropagateRequestIdLayer::x_request_id())
+            .layer(SetRequestIdLayer::x_request_id(UuidRequestId));
 
         // Mount Nextcloud routes
         if let Some(nc_router) = nextcloud_router {
@@ -510,7 +526,11 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
 
     // TCP_NODELAY is inherited from the listening socket on Linux,
     // so every accepted connection already has Nagle disabled.
-    axum::serve(listener, app).await?;
+    axum::serve(
+        listener,
+        app.into_make_service_with_connect_info::<SocketAddr>(),
+    )
+    .await?;
     tracing::info!("Server shutdown completed");
 
     Ok(())