Skip to content

feat(api): capture client connection info via ConnectInfo#842

Merged
gtema merged 1 commit into
openstack-experimental:mainfrom
ymh1874:feature/358-capture-client-info
Jun 25, 2026
Merged

feat(api): capture client connection info via ConnectInfo#842
gtema merged 1 commit into
openstack-experimental:mainfrom
ymh1874:feature/358-capture-client-info

Conversation

@ymh1874

@ymh1874 ymh1874 commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Closes #358.

What this does

Starts capturing client connection information (the client IP) in request processing, as the enabling step for a future IP-based login-control feature. The lockout/rate-limit mechanism itself is out of scope here — this is the plumbing that makes the client address available to handlers and middleware.

  • Switches the public listener's make-service to into_make_service_with_connect_info::<SocketAddr>(), so the raw TCP peer address is stored in a ConnectInfo<SocketAddr> request extension.
  • Surfaces it on the existing request tracing span as a client.addr field (reusing the previously-empty placeholder slot), so the capture is verifiable in request logs.

Mirrors Python Keystone

This deliberately follows how upstream Python Keystone captures the client address:

Python This PR
WSGI REMOTE_ADDR / flask.request.remote_addr (raw peer when no proxy parsing applies) ConnectInfo<SocketAddr>
remote_addr surfaced in the auth-failure log line client.addr field on the request span
Proxy-header parsing kept separate, config-gated via [oslo_middleware] enable_proxy_headers_parsing (default off) deliberately deferred — see follow-up below

Python captures a single REMOTE_ADDR at the WSGI front door; the analogue here is the public HTTP interface. The internal (SPIFFE mTLS/TCP) and admin (SPIFFE mTLS/UDS) interfaces are intentionally not covered: they bypass axum::serve, and a Unix socket has no meaningful SocketAddr.

Important: raw peer only — not proxy-aware

The captured address is the raw TCP peer. Behind a reverse proxy / load balancer (the normal production topology) it is the proxy's address, not the real client. It must not be used directly for IP-based login control until a trusted forwarded-header layer exists.

Required follow-up (after this merges)

A config-gated forwarded-header layer that parses X-Forwarded-For / RFC 7239 Forwarded, mirroring Python's [oslo_middleware] enable_proxy_headers_parsingoff by default, so a deployment that is not actually behind a trusted proxy cannot be tricked into trusting a spoofed header. (axum-client-ip is a candidate, or a hand-rolled middleware.) This is required before any IP-based login control is built on top of this capture.

Composition with #734

Issue #734 wraps the router with NormalizePathLayer from the outside, so the public listener serves a NormalizePath<Router>, not a plain Router. into_make_service_with_connect_info is available on the same axum::ServiceExt trait already in use (blanket impl for any Service), so it applies directly — no restructuring needed.

Tests

Added connect_info_is_captured_and_normalizes, which drives the exact production make-service path fully in-process and asserts a /echo/ request both normalizes the trailing slash and delivers the injected peer address to the handler — proving #358 and #734 compose. The existing trailing_slash_is_normalized regression still passes.

@ymh1874 ymh1874 marked this pull request as ready for review June 24, 2026 22:42
@ymh1874 ymh1874 force-pushed the feature/358-capture-client-info branch from 27c0b69 to 84c414a Compare June 25, 2026 11:31
Wire the public listener's make-service to
into_make_service_with_connect_info::<SocketAddr>() so the raw TCP peer
address is stored in a ConnectInfo<SocketAddr> request extension and
surfaced on the request tracing span as `client.addr`. This mirrors how
upstream Python Keystone exposes the client via WSGI REMOTE_ADDR /
flask.request.remote_addr, and is the enabling step for future IP-based
login control.

This captures the raw peer only; it is not proxy-aware. Behind a reverse
proxy/load balancer it is the proxy's address. A config-gated
forwarded-header layer (mirroring oslo_middleware's
enable_proxy_headers_parsing, off by default) is a required follow-up
before this is used for any IP-based control.

Composes with the NormalizePathLayer wrap from openstack-experimental#734: the connect-info
make-service is taken from axum's ServiceExt (blanket-impl'd for any
Service), so it applies to the NormalizePath<Router> value.

Closes openstack-experimental#358

Note: This commit was done with the help of AI.
Signed-off-by: Yousef Hussein <ymh1874@gmail.com>

@gtema gtema left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, but would need a followup

// `into_make_service_with_connect_info::<SocketAddr>` stores the
// raw TCP peer address in a `ConnectInfo<SocketAddr>` request
// extension (the analogue of Python Keystone's WSGI REMOTE_ADDR).
// This is the *raw* peer, not proxy-resolved: behind a reverse

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This need to be addressed of course as well. Follow-up should be absolutely fine

// `into_make_service_with_connect_info` on the public
// listener (the keystone-ng analogue of Python Keystone's
// WSGI REMOTE_ADDR / flask.request.remote_addr). `None` for
// the SPIFFE interfaces, which do not populate ConnectInfo.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is an interesting point. I agree for spiffe we currently go absolutely different way, but it should be possible to capture the information there as well. You can definitely capture the remote IP address and at a later phase the headers (for x-forward) should be also available. Please evaluate this in the followup

@gtema gtema merged commit fec86fc into openstack-experimental:main Jun 25, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Start capturing client information in the request processing

2 participants