From 4fc3746754317fb058b22520d71afca99a07aed2 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Mon, 8 Jun 2026 08:37:16 +0200 Subject: [PATCH] chore(logs): add explicit http logs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Default is now RUST_LOG=info,http=warn. Effect of each level on the access log: ┌────────────────────┬────────────────────────┐ │ Level on http │ Status classes emitted │ ├────────────────────┼────────────────────────┤ │ info │ 2xx/3xx + 4xx + 5xx │ ├────────────────────┼────────────────────────┤ │ warn (default) │ 4xx + 5xx │ ├────────────────────┼────────────────────────┤ │ error │ 5xx only │ ├────────────────────┼────────────────────────┤ │ off │ nothing │ └────────────────────┴────────────────────────┘ Target mapping: ┌────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┬─────────────────┐ │ Routes │ Target │ ├────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼─────────────────┤ │ health_routes │ http::probe │ ├────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼─────────────────┤ │ magic_link_router │ http::web │ ├────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼─────────────────┤ │ All /api/auth/* sub-routers (login, register, refresh, public, protected, app_pw, device_public, device_protected) │ http::api::auth │ ├────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼─────────────────┤ │ setup_router, public_api_routes, protected_api, wopi_api_protected │ http::api │ ├────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼─────────────────┤ │ well_known_router, caldav_protected, carddav_protected, webdav_protected │ http::dav │ ├────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼─────────────────┤ │ nc_router │ http::nextcloud │ ├────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼─────────────────┤ │ wopi_protocol │ http::wopi │ ├────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼─────────────────┤ │ web_routes (+ ServeDir fallback) │ http::web │ └────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┴─────────────────┘ # Default value: - **http=warn** if target http not specified - **http::web=error** if target http::web not specified Common operator overrides: # Server-error-only access logs (the new default) unset RUST_LOG # See login failures and other client errors on auth RUST_LOG=info,http=warn,http::api::auth=info # which is similar to RUST_LOG=info,http::api::auth=info # Full access log everywhere (heavy) RUST_LOG=info,http=info # Silence everything except errors RUST_LOG=warn --- src/interfaces/api/handlers/wopi_handler.rs | 19 +- src/interfaces/api/routes.rs | 27 +- src/interfaces/middleware/trace_span.rs | 268 ++++++++++++++++++-- src/interfaces/nextcloud/routes.rs | 39 ++- src/main.rs | 163 ++++++++---- 5 files changed, 438 insertions(+), 78 deletions(-) diff --git a/src/interfaces/api/handlers/wopi_handler.rs b/src/interfaces/api/handlers/wopi_handler.rs index 93e3320b..063423e6 100644 --- a/src/interfaces/api/handlers/wopi_handler.rs +++ b/src/interfaces/api/handlers/wopi_handler.rs @@ -15,7 +15,7 @@ use axum::{ extract::{Path, Query, State}, http::{HeaderMap, Request, StatusCode}, response::{Html, IntoResponse, Response}, - routing::{get, post}, + routing::{any, get, post}, }; use serde::{Deserialize, Serialize}; use std::sync::Arc; @@ -568,6 +568,12 @@ pub fn wopi_routes( .route("/edit/{file_id}", get(host_page)) // Supported extensions (public, no auth) .route("/supported-extensions", get(get_supported_extensions)) + // Collector for any unknown `/wopi/*` path — keeps the + // access-log target as `http::wopi` instead of letting + // M365/Collabora probes leak into `http::web` via the + // ServeDir fallback. Same rationale as the NC `/ocs/*` + // catch-all in interfaces/nextcloud/routes.rs. + .route("/{*rest}", any(wopi_not_found)) .with_state(wopi_state.clone()); let api_router = Router::new() @@ -576,3 +582,14 @@ pub fn wopi_routes( (protocol_router, api_router) } + +/// Catch-all 404 for unknown paths nested under `/wopi`. Exists +/// purely to anchor the access-log target to `http::wopi` instead +/// of letting the request fall through Axum's matcher to +/// ServeDir and being mis-attributed to `http::web`. +async fn wopi_not_found() -> Response { + Response::builder() + .status(StatusCode::NOT_FOUND) + .body(Body::empty()) + .unwrap() +} diff --git a/src/interfaces/api/routes.rs b/src/interfaces/api/routes.rs index 14c9d017..08f08896 100644 --- a/src/interfaces/api/routes.rs +++ b/src/interfaces/api/routes.rs @@ -2,10 +2,11 @@ use crate::application::services::batch_operations::BatchOperationService; use crate::common::di::AppState; use axum::{ Router, + body::Body, extract::{DefaultBodyLimit, State}, http::StatusCode, - response::{IntoResponse, Json as AxumJson}, - routing::{delete, get, post, put}, + response::{IntoResponse, Json as AxumJson, Response}, + routing::{any, delete, get, post, put}, }; use serde_json::json; use std::sync::Arc; @@ -588,6 +589,19 @@ pub fn create_api_routes(app_state: &Arc) -> Router> { .with_state(app_state.clone()); router = router.nest("/users", users_router); + // Collector for any unknown `/api/*` path. Without this, an + // unmatched API URL falls through Axum's matcher to the + // ServeDir fallback and is logged under `http::web` — wrong + // surface for operator triage. Adding the catch-all here + // anchors the 404 on whichever access-log layer the parent + // mount applies (i.e. `http::api`). + // + // The bare `/api/auth/*`, `/api/wopi/*`, and other more- + // specific nests are registered at higher specificity and + // still win over this catch-all — Axum's matcher prefers + // them on every overlapping request. + router = router.route("/{*rest}", any(api_not_found)); + // Compression is applied once, globally, in `main.rs` with a content-type // aware predicate that skips already-compressed media. Re-applying it here // would double-wrap `/api`: this inner layer (no predicate) would compress @@ -595,3 +609,12 @@ pub fn create_api_routes(app_state: &Arc) -> Router> { // So this router only adds tracing; compression is the global layer's job. router.layer(TraceLayer::new_for_http()) } + +/// Catch-all 404 for unknown `/api/*` paths. Pure log-anchoring +/// shim — see the comment in `create_api_routes`. +async fn api_not_found() -> Response { + Response::builder() + .status(StatusCode::NOT_FOUND) + .body(Body::empty()) + .unwrap() +} diff --git a/src/interfaces/middleware/trace_span.rs b/src/interfaces/middleware/trace_span.rs index 0241c7a1..881cdbb3 100644 --- a/src/interfaces/middleware/trace_span.rs +++ b/src/interfaces/middleware/trace_span.rs @@ -1,17 +1,58 @@ -//! Custom [`MakeSpan`], [`OnResponse`], and [`MakeRequestId`] for request tracing. +//! Per-request tracing primitives + the [`access_log!`] macro. //! -//! [`UuidRequestId`] — generates a UUID v4 per request for `SetRequestIdLayer`. +//! ## What lives here //! -//! [`ClientIpMakeSpan`] — records `request_id`, `client_ip`, `method`, `uri`, -//! and a placeholder `user_id` (filled by auth middleware) on every request span. +//! - [`UuidRequestId`] — generates a UUID v7 per request for +//! `tower_http::request_id::SetRequestIdLayer`. +//! - [`ClientIpMakeSpan`] — creates the per-request `req` span with +//! `request_id`, `client_ip`, `method`, `uri`, and deferred +//! `user_id` / `chroot_id` fields. The auth middlewares fill the +//! deferred fields via `Span::current().record(...)`. +//! - [`access_log!`] — macro that produces an `axum::middleware` +//! layer emitting one log event per request at a fixed tracing +//! target. Attach to each sub-router at its mount site. //! -//! [`LogBadRequest`] — emits a WARN for every HTTP 400 response, inheriting -//! all span fields so the log line includes request ID, IP, user, method, URI. +//! ## Why targets are declared at mount sites (not by URI prefix) +//! +//! The router topology — `Router::nest`, `merge`, `route` — already +//! describes "this group of routes belongs to surface X". Re-deriving +//! that grouping inside the middleware by URI-prefix matching would +//! duplicate the topology and silently drift when routes are added, +//! moved, or renamed. Declaring the target right next to the +//! `nest()` / `merge()` call keeps the two in lockstep: a route +//! group can't be reached except through its mount site, and the +//! mount site is now the single source of truth for its log target. +//! +//! ## Available targets +//! +//! Targets use Rust's `::` module-path separator so +//! `tracing_subscriber::EnvFilter` recognises the hierarchy: +//! `RUST_LOG=http=info` enables every sub-target; override a single +//! one with `RUST_LOG=http=warn,http::api::auth=info`. +//! +//! Status-class → tracing level mapping (see [`access_log!`] for +//! details): +//! - `INFO` — 2xx/3xx + 4xx + 5xx (full access trace) +//! - `WARN` — 4xx + 5xx (default — `http=warn`) +//! - `ERROR` — 5xx only (5xx-only firehose) +//! +//! Conventional targets: +//! - `http::api` — REST API under `/api/*`. +//! - `http::api::auth` — auth surface (login, refresh, app-pw, OIDC, +//! device-auth). High-value for security operators. +//! - `http::nextcloud` — NextCloud-flavoured surface (`/remote.php`, +//! `/ocs`, `/status.php`, `/login/v2`, `/index.php/204`). +//! - `http::dav` — CalDAV / CardDAV / WebDAV + RFC 6764 discovery. +//! - `http::wopi` — WOPI host protocol (M365 / Collabora). +//! - `http::probe` — `/health`, `/ready`, `/version`, `/openapi.json`. +//! - `http::web` — HTML pages + magic-link redemption. +//! - `http::static` — `ServeDir` fallback (CSS/JS/images at bare URLs). +//! - `http` — bare catch-all for routes that didn't get an explicit +//! layer (loud signal that wiring is missing). -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 tower_http::trace::MakeSpan; use tracing::Span; use uuid::Uuid; @@ -27,16 +68,19 @@ use uuid::Uuid; pub struct UuidRequestId; impl MakeRequestId for UuidRequestId { - fn make_request_id(&mut self, _request: &Request) -> Option { + fn make_request_id(&mut self, _request: &axum::http::Request) -> Option { let id = Uuid::now_v7().to_string(); - HeaderValue::from_str(&id).ok().map(RequestId::new) + axum::http::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`. +/// Implements [`MakeSpan`] so every HTTP request span carries +/// `request_id`, `client_ip`, `method`, `uri`, and deferred +/// `user_id` / `chroot_id` slots. /// /// `request_id` is read from the `x-request-id` header set by /// [`tower_http::request_id::SetRequestIdLayer`] (which must wrap this layer). @@ -44,13 +88,14 @@ impl MakeRequestId for UuidRequestId { pub struct ClientIpMakeSpan; impl MakeSpan for ClientIpMakeSpan { - fn make_span(&mut self, request: &Request) -> Span { + fn make_span(&mut self, request: &axum::http::Request) -> 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, @@ -58,24 +103,195 @@ impl MakeSpan for ClientIpMakeSpan { method = %request.method(), uri = %request.uri().path(), user_id = tracing::field::Empty, + // The Nextcloud chroot folder id, set by `basic_auth_middleware`. + chroot_id = tracing::field::Empty, ) } } -// ─── Response observer ─────────────────────────────────────────────────────── +// ─── Access log macro ──────────────────────────────────────────────────────── -/// Implements [`OnResponse`]: emits a WARN log for every HTTP 400 response. -#[derive(Clone, Debug, Default)] -pub struct LogBadRequest; +/// Returns an [`axum::middleware`] layer that emits one log event +/// per request at a fixed tracing `target`. +/// +/// **Attach at the mount site of each route group**, so the target +/// is declared next to the `nest()` / `merge()` it applies to: +/// +/// ```ignore +/// use oxicloud::access_log; +/// +/// app = app +/// .merge(health_routes.layer(access_log!("http::probe"))) +/// .merge(magic_link_router.layer(access_log!("http::web"))) +/// .nest("/api/auth", auth_router.layer(access_log!("http::api::auth"))) +/// .nest("/api", api_router.layer(access_log!("http::api"))) +/// .merge(webdav_router.layer(access_log!("http::dav"))) +/// .nest("/wopi", wopi_protocol.layer(access_log!("http::wopi"))) +/// .merge(web_routes.layer(access_log!("http::web"))); +/// ``` +/// +/// Level by status class: `2xx`/`3xx` → `INFO`, `4xx` → `WARN`, +/// `5xx` → `ERROR`. With the default `RUST_LOG=…,http=warn`, only +/// 4xx and 5xx are emitted; bump to `http=info` for full request +/// tracing or narrow to `http=error` for 5xx only. +/// +/// Each event inherits `request_id`, `client_ip`, `method`, `uri`, +/// `user_id`, `chroot_id` from the surrounding `req` span (created +/// by [`ClientIpMakeSpan`] at the `TraceLayer` site). +/// +/// ## Why a macro +/// +/// `tracing::info!(target: …)` requires the target argument to be a +/// **literal** at the macro expansion site — runtime variables are +/// rejected. The macro embeds the literal target into a `from_fn` +/// closure, so no runtime dispatch table is needed; the call site +/// is also the literal site. +#[macro_export] +macro_rules! access_log { + ($target:literal) => { + ::axum::middleware::from_fn( + |req: ::axum::extract::Request, next: ::axum::middleware::Next| async move { + // Cheaply hold the `user-agent` HeaderValue (a + // `bytes::Bytes` clone — one atomic increment, no + // allocation) so we can still read it after `req` is + // moved into `next.run`. The `&str` view + format is + // deferred to inside the per-level `enabled!` + // branches, so no work is wasted when the filter + // rejects the event (default `RUST_LOG=…,http=warn` + // → 2xx/3xx never format). + let user_agent_hv = req + .headers() + .get(::axum::http::header::USER_AGENT) + .cloned(); + let start = ::std::time::Instant::now(); + let response = next.run(req).await; + let status = response.status().as_u16(); + let latency_ms = start.elapsed().as_millis() as u64; -impl OnResponse for LogBadRequest { - fn on_response(self, response: &Response, latency: Duration, _span: &Span) { - if response.status() == StatusCode::BAD_REQUEST { - tracing::warn!( - status = 400, - latency_ms = latency.as_millis(), - "bad request", - ); - } + // Per-status-class emission. `enabled!` is an + // ~5 ns atomic-load + comparison; below it we + // extract `&str` views from the still-live + // HeaderValues without allocating. + // + // Level mapping (shifted one rung up from the + // historical DEBUG/INFO/WARN ladder so the default + // `http=warn` keeps 4xx+5xx and `http=error` + // narrows to 5xx only): + // 5xx → ERROR ("server_error") + // 4xx → WARN ("client_error") + // 2xx/3xx → INFO ("ok") + // + // `content_length` is 0 for streamed bodies + // (chunked transfer-encoding sets no Content-Length) + // — operators reading the log should interpret `0` + // as "empty OR streamed", not literally zero bytes. + if status >= 500 { + if ::tracing::enabled!(target: $target, ::tracing::Level::ERROR) { + let user_agent = user_agent_hv + .as_ref() + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + let content_type = response + .headers() + .get(::axum::http::header::CONTENT_TYPE) + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + let content_length = response + .headers() + .get(::axum::http::header::CONTENT_LENGTH) + .and_then(|v| v.to_str().ok()) + .and_then(|s| s.parse::().ok()) + .unwrap_or(0); + ::tracing::error!( + target: $target, + status, + latency_ms, + content_length, + content_type, + user_agent, + "server_error" + ); + } + } else if status >= 400 { + if ::tracing::enabled!(target: $target, ::tracing::Level::WARN) { + let user_agent = user_agent_hv + .as_ref() + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + let content_type = response + .headers() + .get(::axum::http::header::CONTENT_TYPE) + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + let content_length = response + .headers() + .get(::axum::http::header::CONTENT_LENGTH) + .and_then(|v| v.to_str().ok()) + .and_then(|s| s.parse::().ok()) + .unwrap_or(0); + ::tracing::warn!( + target: $target, + status, + latency_ms, + content_length, + content_type, + user_agent, + "client_error" + ); + } + } else if ::tracing::enabled!(target: $target, ::tracing::Level::INFO) { + let user_agent = user_agent_hv + .as_ref() + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + let content_type = response + .headers() + .get(::axum::http::header::CONTENT_TYPE) + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + let content_length = response + .headers() + .get(::axum::http::header::CONTENT_LENGTH) + .and_then(|v| v.to_str().ok()) + .and_then(|s| s.parse::().ok()) + .unwrap_or(0); + ::tracing::info!( + target: $target, + status, + latency_ms, + content_length, + content_type, + user_agent, + "ok" + ); + } + response + }, + ) + }; +} + +// Re-export the macro at this module path so `use +// crate::interfaces::middleware::trace_span::access_log` works in +// addition to `crate::access_log` (which `#[macro_export]` provides). +pub use access_log; + +// Convenience for tests / callers that want the same latency unit +// the macro uses. +#[doc(hidden)] +pub fn latency_ms(d: Duration) -> u64 { + d.as_millis() as u64 +} + +#[cfg(test)] +mod tests { + use super::latency_ms; + use std::time::Duration; + + #[test] + fn latency_ms_is_millis() { + assert_eq!(latency_ms(Duration::from_millis(0)), 0); + assert_eq!(latency_ms(Duration::from_millis(7)), 7); + assert_eq!(latency_ms(Duration::from_secs(3)), 3000); } } diff --git a/src/interfaces/nextcloud/routes.rs b/src/interfaces/nextcloud/routes.rs index a600a6ce..3e94b557 100644 --- a/src/interfaces/nextcloud/routes.rs +++ b/src/interfaces/nextcloud/routes.rs @@ -76,7 +76,33 @@ pub fn nextcloud_routes_with_state(state: Arc) -> Router .route( "/ocs/v2.php/cloud/capabilities", get(ocs_handler::handle_capabilities_v2), - ); + ) + // Final NC catch-alls. Any `/ocs/*` or `/remote.php/*` URL + // the routes above don't claim returns 404 here — so it's + // logged under the `http::nextcloud` access-log target the + // surrounding `.layer(access_log!(…))` in main.rs assigns, + // instead of falling through Axum's matcher to ServeDir + // and being mis-attributed to `http::web`. + // + // Concrete example: NC desktop probes + // `/ocs/v2.php/core/navigation/apps` to discover server + // features. We don't implement that endpoint; without these + // catch-alls the 404 was emitted at `http::web`, which is + // misleading for operators triaging Nextcloud client noise. + // + // Mounted on the PUBLIC sub-router (NOT behind basic-auth) + // so unknown-endpoint probes return 404 regardless of + // whether the client sent credentials. Moving them into + // `protected` would turn anonymous probes into 401 + // challenges, which breaks some clients' capability- + // detection paths. + // + // Axum routes more-specific paths first, so the specific + // NC routes above (and the protected ones below) still + // claim their requests; only genuinely unmatched paths + // reach these handlers. + .route("/ocs/{*rest}", any(handle_nc_not_found)) + .route("/remote.php/{*rest}", any(handle_nc_not_found)); // Protected routes — require Basic Auth via app passwords. let protected = Router::new() @@ -297,3 +323,14 @@ async fn handle_dav_discovery() -> Response { .body(Body::empty()) .unwrap() } + +/// Catch-all 404 for any `/ocs/*` or `/remote.php/*` path the NC +/// router doesn't recognize. Exists purely to anchor the access-log +/// target — see the comment on the routes above for the operator +/// rationale. +async fn handle_nc_not_found() -> Response { + Response::builder() + .status(StatusCode::NOT_FOUND) + .body(Body::empty()) + .unwrap() +} diff --git a/src/main.rs b/src/main.rs index d3625e80..6c7d18ce 100644 --- a/src/main.rs +++ b/src/main.rs @@ -12,9 +12,8 @@ use socket2::{Domain, Protocol, Socket, TcpKeepalive, Type}; use axum::Router; use axum::extract::DefaultBodyLimit; -use oxicloud::interfaces::middleware::trace_span::{ - ClientIpMakeSpan, LogBadRequest, UuidRequestId, -}; +use oxicloud::access_log; +use oxicloud::interfaces::middleware::trace_span::{ClientIpMakeSpan, UuidRequestId}; use tower_http::limit::RequestBodyLimitLayer; use tower_http::request_id::{PropagateRequestIdLayer, SetRequestIdLayer}; use tower_http::set_header::SetResponseHeaderLayer; @@ -178,11 +177,44 @@ async fn main() -> Result<(), Box> { } } - // Initialize tracing + // Initialize tracing. + // + // Default access-log policy — two independent directives are + // injected unless the operator has already named them: + // + // `http=warn` (4xx + 5xx for every access-log target) + // `http::web=error` (5xx only for static / ServeDir / catch-all) + // + // `http::web` is pulled down to ERROR because it's the noisiest + // surface (every CSS/JS/img/favicon request hits it) and its + // 4xx are almost always "browser asked for a file we don't ship", + // not a real signal. Operators investigating a 404 storm can + // promote it back: `RUST_LOG=info,http::web=warn`. + // + // The detection is substring-based: + // - `http=` in RUST_LOG → operator owns the http baseline. + // - `http::web=` in RUST_LOG → operator owns the web subtarget. + // The two are independent — supplying `http=info` still gets a + // free `http::web=error` unless the operator named that too. + // + // Empty / unset / no http directives → both defaults applied. + // Note that `http::web=…` does NOT contain `http=` as a substring + // (different characters around the `:`), so the two checks don't + // alias each other. + let rust_log = match std::env::var("RUST_LOG").ok().filter(|s| !s.is_empty()) { + None => "info,http=warn,http::web=error".to_string(), + Some(mut s) => { + if !s.contains("http=") { + s.push_str(",http=warn"); + } + if !s.contains("http::web=") { + s.push_str(",http::web=error"); + } + s + } + }; tracing_subscriber::registry() - .with(tracing_subscriber::EnvFilter::new( - std::env::var("RUST_LOG").unwrap_or_else(|_| "info".into()), - )) + .with(tracing_subscriber::EnvFilter::new(rust_log)) .with(tracing_subscriber::fmt::layer()) .init(); @@ -487,38 +519,68 @@ async fn main() -> Result<(), Box> { let magic_link_router = interfaces::api::handlers::magic_link_handler::magic_link_routes() .with_state(app_state.clone()); + // Access-log targets are declared per-mount via `access_log!(…)` + // — see `interfaces/middleware/trace_span.rs` for the catalogue. app = Router::new() // Health / readiness probes — no auth, mounted at root - .merge(health_routes) + .merge(health_routes.layer(access_log!("http::probe"))) // Magic-link redemption — top-level, no `/api/` prefix - .merge(magic_link_router) + .merge(magic_link_router.layer(access_log!("http::web"))) // Rate-limited auth endpoints (login, register, refresh) - .nest("/api/auth", auth_login) - .nest("/api/auth", auth_register) - .nest("/api/auth", auth_refresh) + .nest( + "/api/auth", + auth_login.layer(access_log!("http::api::auth")), + ) + .nest( + "/api/auth", + auth_register.layer(access_log!("http::api::auth")), + ) + .nest( + "/api/auth", + auth_refresh.layer(access_log!("http::api::auth")), + ) // Public auth endpoints (status, OIDC) - .nest("/api/auth", auth_public) + .nest( + "/api/auth", + auth_public.layer(access_log!("http::api::auth")), + ) // Protected auth endpoints (/me, /change-password, /logout) - .nest("/api/auth", auth_protected) + .nest( + "/api/auth", + auth_protected.layer(access_log!("http::api::auth")), + ) // App password management (create, list, revoke) - .nest("/api/auth", app_pw_protected) + .nest( + "/api/auth", + app_pw_protected.layer(access_log!("http::api::auth")), + ) // One-time setup endpoint — public, rate-limited - .nest("/api", setup_router) + .nest("/api", setup_router.layer(access_log!("http::api"))) // Device Auth Grant public endpoints (authorize + token polling) - .nest("/api/auth/device", device_public) + .nest( + "/api/auth/device", + device_public.layer(access_log!("http::api::auth")), + ) // Device Auth Grant protected endpoints (verify + device management) - .nest("/api/auth/device", device_protected) + .nest( + "/api/auth/device", + device_protected.layer(access_log!("http::api::auth")), + ) // Public API routes (share access, i18n) — no auth required - .nest("/api", public_api_routes) + .nest("/api", public_api_routes.layer(access_log!("http::api"))) // All other API routes are protected by auth middleware - .nest("/api", protected_api) + .nest("/api", protected_api.layer(access_log!("http::api"))) // RFC 6764 well-known discovery (public, no auth — just redirects) - .merge(well_known_router.clone()) + .merge(well_known_router.clone().layer(access_log!("http::dav"))) // CalDAV/CardDAV/WebDAV protocols merged at top-level for client compatibility - .merge(caldav_protected) - .merge(carddav_protected) - .merge(webdav_protected) - .merge(web_routes); + .merge(caldav_protected.layer(access_log!("http::dav"))) + .merge(carddav_protected.layer(access_log!("http::dav"))) + .merge(webdav_protected.layer(access_log!("http::dav"))) + // Web (HTML pages) — also the ServeDir fallback root, so + // static asset hits land here. We keep them on the `web` + // target for simplicity; switch to `http::static` when + // the static surface is split into its own router. + .merge(web_routes.layer(access_log!("http::web"))); // Mount Nextcloud routes (uses its own Basic Auth middleware). // **Merged BEFORE the trace + request-id layers** so NC requests @@ -526,7 +588,11 @@ async fn main() -> Result<(), Box> { // fields as every other surface — see // `interfaces/middleware/trace_span.rs::ClientIpMakeSpan`. if let Some(nc_router) = nextcloud_router { - app = app.merge(nc_router.with_state(app_state.clone())); + app = app.merge( + nc_router + .with_state(app_state.clone()) + .layer(access_log!("http::nextcloud")), + ); } // Mount WOPI routes (protocol routes use own token auth, API routes behind auth middleware). @@ -540,8 +606,11 @@ async fn main() -> Result<(), Box> { auth_middleware, )); app = app - .nest("/wopi", wopi_protocol) - .nest("/api/wopi", wopi_api_protected); + .nest("/wopi", wopi_protocol.layer(access_log!("http::wopi"))) + .nest( + "/api/wopi", + wopi_api_protected.layer(access_log!("http::api")), + ); } // ── Trace + request-id layers applied LAST so every route @@ -550,11 +619,7 @@ async fn main() -> Result<(), Box> { // only have to be merged before this point to get tracing // for free — no second site to remember to update. app = app - .layer( - TraceLayer::new_for_http() - .make_span_with(ClientIpMakeSpan) - .on_response(LogBadRequest), - ) + .layer(TraceLayer::new_for_http().make_span_with(ClientIpMakeSpan)) .layer(PropagateRequestIdLayer::x_request_id()) .layer(SetRequestIdLayer::x_request_id(UuidRequestId)); } else { @@ -562,38 +627,40 @@ async fn main() -> Result<(), Box> { tracing::warn!("Authentication is DISABLED — all API routes are publicly accessible"); app = Router::new() // Health / readiness probes — no auth, mounted at root - .merge(health_routes) - .nest("/api", public_api_routes) - .nest("/api", api_routes) + .merge(health_routes.layer(access_log!("http::probe"))) + .nest("/api", public_api_routes.layer(access_log!("http::api"))) + .nest("/api", api_routes.layer(access_log!("http::api"))) // RFC 6764 well-known discovery (just redirects) - .merge(well_known_router) + .merge(well_known_router.layer(access_log!("http::dav"))) // CalDAV/CardDAV/WebDAV protocols merged at top-level - .merge(caldav_router) - .merge(carddav_router) - .merge(webdav_router) - .merge(web_routes); + .merge(caldav_router.layer(access_log!("http::dav"))) + .merge(carddav_router.layer(access_log!("http::dav"))) + .merge(webdav_router.layer(access_log!("http::dav"))) + .merge(web_routes.layer(access_log!("http::web"))); // Mount Nextcloud routes — merged BEFORE the trace + request-id // layers so NC requests get the same span fields as every // other surface (matches the auth-enabled branch above). if let Some(nc_router) = nextcloud_router { - app = app.merge(nc_router.with_state(app_state.clone())); + app = app.merge( + nc_router + .with_state(app_state.clone()) + .layer(access_log!("http::nextcloud")), + ); } // Mount WOPI routes (no auth middleware when auth is disabled). // Same reasoning: merge before the trace layer. if let Some((wopi_protocol, wopi_api)) = wopi_routes { - app = app.nest("/wopi", wopi_protocol).nest("/api/wopi", wopi_api); + app = app + .nest("/wopi", wopi_protocol.layer(access_log!("http::wopi"))) + .nest("/api/wopi", wopi_api.layer(access_log!("http::api"))); } // ── Trace + request-id layers applied LAST. See the // auth-enabled branch above for the rationale. app = app - .layer( - TraceLayer::new_for_http() - .make_span_with(ClientIpMakeSpan) - .on_response(LogBadRequest), - ) + .layer(TraceLayer::new_for_http().make_span_with(ClientIpMakeSpan)) .layer(PropagateRequestIdLayer::x_request_id()) .layer(SetRequestIdLayer::x_request_id(UuidRequestId)); }