Track and log the client IP address (#5734)

crates/cli/src/app_state.rs

@@ -7,13 +7,13 @@
 
 use std::{convert::Infallible, net::IpAddr, sync::Arc};
 
-use axum::extract::{FromRef, FromRequestParts};
+use axum::extract::{FromRef, FromRequestParts, State};
 use ipnetwork::IpNetwork;
 use mas_context::LogContext;
 use mas_data_model::{AppVersion, BoxClock, BoxRng, SiteConfig, SystemClock};
 use mas_handlers::{
-    ActivityTracker, BoundActivityTracker, CookieManager, ErrorWrapper, GraphQLSchema, Limiter,
-    MetadataCache, RequesterFingerprint, passwords::PasswordManager,
+    ActivityTracker, ClientIp, CookieManager, ErrorWrapper, GraphQLSchema, Limiter, MetadataCache,
+    passwords::PasswordManager,
 };
 use mas_i18n::Translator;
 use mas_keystore::{Encrypter, Keystore};
@@ -273,10 +273,11 @@ impl FromRequestParts<AppState> for ActivityTracker {
 }
 
 fn infer_client_ip(
-    parts: &axum::http::request::Parts,
+    extensions: &axum::http::Extensions,
+    headers: &axum::http::HeaderMap,
     trusted_proxies: &[IpNetwork],
 ) -> Option<IpAddr> {
-    let connection_info = parts.extensions.get::<mas_listener::ConnectionInfo>();
+    let connection_info = extensions.get::<mas_listener::ConnectionInfo>();
 
     let peer = if let Some(info) = connection_info {
         // We can always trust the proxy protocol to give us the correct IP address
@@ -292,8 +293,7 @@ fn infer_client_ip(
     };
 
     // Get the list of IPs from the X-Forwarded-For header
-    let peers_from_header = parts
-        .headers
+    let peers_from_header = headers
         .get("x-forwarded-for")
         .and_then(|value| value.to_str().ok())
         .map(|value| value.split(',').filter_map(|v| v.parse().ok()))
@@ -325,41 +325,23 @@ fn infer_client_ip(
     client_ip.or(fallback)
 }
 
-impl FromRequestParts<AppState> for BoundActivityTracker {
-    type Rejection = Infallible;
-
-    async fn from_request_parts(
-        parts: &mut axum::http::request::Parts,
-        state: &AppState,
-    ) -> Result<Self, Self::Rejection> {
-        // TODO: we may infer the IP twice, for the activity tracker and the limiter
-        let ip = infer_client_ip(parts, &state.trusted_proxies);
-        tracing::debug!(ip = ?ip, "Inferred client IP address");
-        Ok(state.activity_tracker.clone().bind(ip))
-    }
-}
-
-impl FromRequestParts<AppState> for RequesterFingerprint {
-    type Rejection = Infallible;
-
-    async fn from_request_parts(
-        parts: &mut axum::http::request::Parts,
-        state: &AppState,
-    ) -> Result<Self, Self::Rejection> {
-        // TODO: we may infer the IP twice, for the activity tracker and the limiter
-        let ip = infer_client_ip(parts, &state.trusted_proxies);
-
-        if let Some(ip) = ip {
-            Ok(RequesterFingerprint::new(ip))
-        } else {
-            // If we can't infer the IP address, we'll just use an empty fingerprint and
-            // warn about it
-            tracing::warn!(
-                "Could not infer client IP address for an operation which rate-limits based on IP addresses"
-            );
-            Ok(RequesterFingerprint::EMPTY)
-        }
-    }
+/// Middleware which infers the client IP address once, ahead of the rest of the
+/// request handling, and stores it in the request extensions as a [`ClientIp`].
+///
+/// This lets the logging middleware and the `BoundActivityTracker` /
+/// `RequesterFingerprint` extractors read the same IP without re-inferring it.
+pub(crate) async fn client_ip_middleware(
+    State(state): State<AppState>,
+    mut request: axum::extract::Request,
+    next: axum::middleware::Next,
+) -> axum::response::Response {
+    let ip = infer_client_ip(
+        request.extensions(),
+        request.headers(),
+        &state.trusted_proxies,
+    );
+    request.extensions_mut().insert(ClientIp(ip));
+    next.run(request).await
 }
 
 impl FromRequestParts<AppState> for BoxRepository {

crates/cli/src/server.rs

@@ -21,6 +21,7 @@ use hyper::{Method, Request, Response, StatusCode, Version, header::USER_AGENT};
 use listenfd::ListenFd;
 use mas_config::{HttpBindConfig, HttpResource, HttpTlsConfig, UnixOrTcp};
 use mas_context::LogContext;
+use mas_handlers::ClientIp;
 use mas_listener::{ConnectionInfo, unix_or_tcp::UnixOrTcpListener};
 use mas_router::Route;
 use mas_templates::Templates;
@@ -31,8 +32,9 @@ use mas_tower::{
 use opentelemetry::{Key, KeyValue};
 use opentelemetry_http::HeaderExtractor;
 use opentelemetry_semantic_conventions::trace::{
-    HTTP_REQUEST_METHOD, HTTP_RESPONSE_STATUS_CODE, HTTP_ROUTE, NETWORK_PROTOCOL_NAME,
-    NETWORK_PROTOCOL_VERSION, URL_PATH, URL_QUERY, URL_SCHEME, USER_AGENT_ORIGINAL,
+    CLIENT_ADDRESS, HTTP_REQUEST_METHOD, HTTP_RESPONSE_STATUS_CODE, HTTP_ROUTE,
+    NETWORK_PROTOCOL_NAME, NETWORK_PROTOCOL_VERSION, URL_PATH, URL_QUERY, URL_SCHEME,
+    USER_AGENT_ORIGINAL,
 };
 use rustls::ServerConfig;
 use sentry_tower::{NewSentryLayer, SentryHttpLayer};
@@ -41,7 +43,7 @@ use tower_http::services::{ServeDir, fs::ServeFileSystemResponseBody};
 use tracing::Span;
 use tracing_opentelemetry::OpenTelemetrySpanExt;
 
-use crate::app_state::AppState;
+use crate::app_state::{AppState, client_ip_middleware};
 
 const MAS_LISTENER_NAME: Key = Key::from_static_str("mas.listener.name");
 
@@ -98,6 +100,14 @@ fn otel_url_scheme<B>(request: &Request<B>) -> &'static str {
 fn make_http_span<B>(req: &Request<B>) -> Span {
     let method = otel_http_method(req);
     let route = otel_http_route(req);
+    // The client IP was inferred by `client_ip_middleware`, which wraps this
+    // layer, so the `ClientIp` extension is already set by the time the span is
+    // created.
+    let client_ip = req
+        .extensions()
+        .get::<ClientIp>()
+        .and_then(|ip| ip.0)
+        .map(tracing::field::display);
 
     let span_name = if let Some(route) = route.as_ref() {
         format!("{method} {route}")
@@ -119,6 +129,7 @@ fn make_http_span<B>(req: &Request<B>) -> Span {
         { URL_QUERY } = tracing::field::Empty,
         { URL_SCHEME } = otel_url_scheme(req),
         { USER_AGENT_ORIGINAL } = tracing::field::Empty,
+        { CLIENT_ADDRESS } = client_ip,
     );
 
     if let Some(route) = route.as_ref() {
@@ -188,6 +199,11 @@ async fn log_response_middleware(
     let method = otel_http_method(&request);
     let path = request.uri().path().to_owned();
     let version = otel_net_protocol_version(&request);
+    let client_ip = request
+        .extensions()
+        .get::<ClientIp>()
+        .and_then(|ip| ip.0)
+        .map(tracing::field::display);
 
     let response = next.run(request).await;
 
@@ -200,14 +216,17 @@ async fn log_response_middleware(
     match status_code.as_u16() {
         100..=399 => tracing::info!(
             name: "http.server.response",
+            { client.address = client_ip },
             "\"{method} {path} HTTP/{version}\" {status_code} {user_agent:?} [{stats}]",
         ),
         400..=499 => tracing::warn!(
             name: "http.server.response",
+            { client.address = client_ip },
             "\"{method} {path} HTTP/{version}\" {status_code} {user_agent:?} [{stats}]",
         ),
         500..=599 => tracing::error!(
             name: "http.server.response",
+            { client.address = client_ip },
             "\"{method} {path} HTTP/{version}\" {status_code} {user_agent:?} [{stats}]",
         ),
         _ => { /* This shouldn't happen */ }
@@ -335,6 +354,16 @@ pub fn build_router(
                 span.record("otel.status_code", "OK");
             }),
         )
+        // Infer the client IP once, ahead of the rest of the request handling,
+        // and store it in the request extensions. Placed *after* the `TraceLayer`
+        // in this list so it wraps it (the first layer is the innermost): the
+        // `ClientIp` extension is therefore set before `make_http_span` reads it
+        // to record `client.address` on the span, and before the logging
+        // middleware and the extractors further in read it too.
+        .layer(axum::middleware::from_fn_with_state(
+            state.clone(),
+            client_ip_middleware,
+        ))
         .layer(mas_context::LogContextLayer::new(|req| {
             otel_http_method(req).into()
         }))

crates/handlers/src/activity_tracker/bound.rs

@@ -1,16 +1,18 @@
+// Copyright 2026 Element Creations Ltd.
 // Copyright 2024, 2025 New Vector Ltd.
 // Copyright 2023, 2024 The Matrix.org Foundation C.I.C.
 //
 // SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
 // Please see LICENSE files in the repository root for full details.
 
-use std::net::IpAddr;
+use std::{convert::Infallible, net::IpAddr};
 
+use axum::extract::FromRequestParts;
 use mas_data_model::{
     BrowserSession, Clock, CompatSession, Session, personal::session::PersonalSession,
 };
 
-use crate::activity_tracker::ActivityTracker;
+use crate::{ClientIp, activity_tracker::ActivityTracker};
 
 /// An activity tracker with an IP address bound to it.
 #[derive(Clone)]
@@ -60,3 +62,27 @@ impl Bound {
             .await;
     }
 }
+
+/// Extracts a [`Bound`] activity tracker for any state that can provide an
+/// [`ActivityTracker`]. The client IP is read from the [`ClientIp`] request
+/// extension (set by the IP-detection middleware), defaulting to `None` when it
+/// is absent.
+impl<S> FromRequestParts<S> for Bound
+where
+    S: Send + Sync,
+    ActivityTracker: FromRequestParts<S, Rejection = Infallible>,
+{
+    type Rejection = Infallible;
+
+    async fn from_request_parts(
+        parts: &mut axum::http::request::Parts,
+        state: &S,
+    ) -> Result<Self, Self::Rejection> {
+        let ip = parts.extensions.get::<ClientIp>().and_then(|ip| ip.0);
+        let tracker = match ActivityTracker::from_request_parts(parts, state).await {
+            Ok(tracker) => tracker,
+            Err(infallible) => match infallible {},
+        };
+        Ok(tracker.bind(ip))
+    }
+}

crates/handlers/src/client_ip.rs

@@ -0,0 +1,19 @@
+// Copyright 2026 Element Creations Ltd.
+//
+// SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
+// Please see LICENSE files in the repository root for full details.
+
+use std::net::IpAddr;
+
+/// The client IP address of the request.
+///
+/// This is set in the request extensions by the IP-detection middleware, ahead
+/// of the rest of the request handling, so that the logging middleware and the
+/// [`BoundActivityTracker`]/[`RequesterFingerprint`] extractors all observe the
+/// same value without re-inferring it. It is `None` when the client IP could
+/// not be determined.
+///
+/// [`BoundActivityTracker`]: crate::BoundActivityTracker
+/// [`RequesterFingerprint`]: crate::RequesterFingerprint
+#[derive(Clone, Copy, Debug)]
+pub struct ClientIp(pub Option<IpAddr>);

crates/handlers/src/compat/login.rs

@@ -1202,6 +1202,53 @@ mod tests {
         user
     }
 
+    /// Test that the client IP injected on the request (as the IP-detection
+    /// middleware would in production) is recorded against the session by the
+    /// activity tracker.
+    #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")]
+    async fn test_login_records_client_ip(pool: PgPool) {
+        setup();
+        let state = TestState::from_pool(pool).await.unwrap();
+
+        let user = user_with_password(&state, "alice", "password", false).await;
+
+        let client_ip = std::net::IpAddr::from([1, 2, 3, 4]);
+        let request = Request::post("/_matrix/client/v3/login")
+            .client_ip(client_ip)
+            .json(serde_json::json!({
+                "type": "m.login.password",
+                "identifier": {
+                    "type": "m.id.user",
+                    "user": "alice",
+                },
+                "password": "password",
+            }));
+
+        let response = state.request(request).await;
+        response.assert_status(StatusCode::OK);
+
+        // Flush the activity tracker so the recorded IP is persisted
+        state.activity_tracker.flush().await;
+
+        let mut repo = state.repository().await.unwrap();
+        let compat_session_page = repo
+            .compat_session()
+            .list(
+                CompatSessionFilter::new().for_user(&user).active_only(),
+                Pagination::first(1),
+            )
+            .await
+            .unwrap();
+        let (compat_session, _) = compat_session_page
+            .edges
+            .into_iter()
+            .next()
+            .expect("a compat session should have been created")
+            .node;
+
+        assert_eq!(compat_session.last_active_ip, Some(client_ip));
+    }
+
     /// Test that a user can login with a password using the Matrix
     /// compatibility API.
     #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")]

crates/handlers/src/lib.rs

@@ -69,6 +69,7 @@ mod activity_tracker;
 mod captcha;
 #[cfg(test)]
 mod cleanup_tests;
+mod client_ip;
 mod preferred_language;
 mod rate_limit;
 mod session;
@@ -106,6 +107,7 @@ use mas_data_model::{BoxClock, BoxRng};
 pub use self::{
     activity_tracker::{ActivityTracker, Bound as BoundActivityTracker},
     admin::router as admin_api_router,
+    client_ip::ClientIp,
     graphql::{
         Schema as GraphQLSchema, schema as graphql_schema, schema_builder as graphql_schema_builder,
     },

crates/handlers/src/rate_limit.rs

@@ -5,13 +5,16 @@
 // SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
 // Please see LICENSE files in the repository root for full details.
 
-use std::{net::IpAddr, sync::Arc, time::Duration};
+use std::{convert::Infallible, net::IpAddr, sync::Arc, time::Duration};
 
+use axum::extract::FromRequestParts;
 use governor::{RateLimiter, clock::QuantaClock, state::keyed::DashMapStateStore};
 use mas_config::RateLimitingConfig;
 use mas_data_model::{User, UserEmailAuthentication};
 use ulid::Ulid;
 
+use crate::ClientIp;
+
 #[derive(Debug, Clone, thiserror::Error)]
 pub enum AccountRecoveryLimitedError {
     #[error("Too many account recovery requests for requester {0}")]
@@ -76,6 +79,28 @@ impl RequesterFingerprint {
     }
 }
 
+impl<S: Send + Sync> FromRequestParts<S> for RequesterFingerprint {
+    type Rejection = Infallible;
+
+    async fn from_request_parts(
+        parts: &mut axum::http::request::Parts,
+        _state: &S,
+    ) -> Result<Self, Self::Rejection> {
+        let ip = parts.extensions.get::<ClientIp>().and_then(|ip| ip.0);
+
+        if let Some(ip) = ip {
+            Ok(Self::new(ip))
+        } else {
+            // If we can't infer the IP address, we'll just use an empty fingerprint and
+            // warn about it
+            tracing::warn!(
+                "Could not infer client IP address for an operation which rate-limits based on IP addresses"
+            );
+            Ok(Self::EMPTY)
+        }
+    }
+}
+
 /// Rate limiters for the different operations
 #[derive(Debug, Clone)]
 pub struct Limiter {

crates/handlers/src/test_utils.rs

@@ -7,6 +7,7 @@
 
 use std::{
     convert::Infallible,
+    net::IpAddr,
     sync::{Arc, Mutex, RwLock},
     task::{Context, Poll},
 };
@@ -53,7 +54,7 @@ use tower::{Layer, Service, ServiceExt};
 use url::Url;
 
 use crate::{
-    ActivityTracker, BoundActivityTracker, Limiter, RequesterFingerprint, graphql,
+    ActivityTracker, ClientIp, Limiter, graphql,
     passwords::{Hasher, PasswordManager},
     upstream_oauth2::cache::MetadataCache,
 };
@@ -619,29 +620,6 @@ impl FromRequestParts<TestState> for ActivityTracker {
     }
 }
 
-impl FromRequestParts<TestState> for BoundActivityTracker {
-    type Rejection = Infallible;
-
-    async fn from_request_parts(
-        _parts: &mut axum::http::request::Parts,
-        state: &TestState,
-    ) -> Result<Self, Self::Rejection> {
-        let ip = None;
-        Ok(state.activity_tracker.clone().bind(ip))
-    }
-}
-
-impl FromRequestParts<TestState> for RequesterFingerprint {
-    type Rejection = Infallible;
-
-    async fn from_request_parts(
-        _parts: &mut axum::http::request::Parts,
-        _state: &TestState,
-    ) -> Result<Self, Self::Rejection> {
-        Ok(RequesterFingerprint::EMPTY)
-    }
-}
-
 impl FromRequestParts<TestState> for BoxClock {
     type Rejection = Infallible;
 
@@ -704,6 +682,14 @@ pub(crate) trait RequestBuilderExt {
     /// credentials.
     fn basic_auth(self, username: &str, password: &str) -> Self;
 
+    /// Sets the client IP address that the extractors will see for this
+    /// request.
+    ///
+    /// This mirrors what the IP-detection middleware does in production: it
+    /// stores a [`ClientIp`] in the request extensions, which is then read by
+    /// the `BoundActivityTracker` and `RequesterFingerprint` extractors.
+    fn client_ip(self, ip: IpAddr) -> Self;
+
     /// Builds the request with an empty body.
     fn empty(self) -> hyper::Request<String>;
 }
@@ -740,6 +726,10 @@ impl RequestBuilderExt for hyper::http::request::Builder {
         self
     }
 
+    fn client_ip(self, ip: IpAddr) -> Self {
+        self.extension(ClientIp(Some(ip)))
+    }
+
     fn empty(self) -> hyper::Request<String> {
         self.body(String::new()).unwrap()
     }