Skip to content

chore: Clean up IP address tracking #2341

Merged
jnschaeffer merged 2 commits intomasterfrom
chore/ip-tracking-cleanup
Jan 27, 2026
Merged

chore: Clean up IP address tracking #2341
jnschaeffer merged 2 commits intomasterfrom
chore/ip-tracking-cleanup

Conversation

@jnschaeffer
Copy link
Contributor

@jnschaeffer jnschaeffer commented Jan 21, 2026

What kind of change does this PR introduce?

This PR cleans up various issues and inconsistencies in IP address tracking in Auth, including:

  • Fixing middleware ordering so request logs respect Sb-Forwarded-For as the canonical IP address source if the user enables it
  • Replacing all usage of request RemoteAddr values with utilities.GetIPAddress calls

What is the current behavior?

Request logs do not show the correct IP address when passing the Sb-Forwarded-For header with SbForwardedForEnabled set to true. Similarly, audit logs only show the value of the request's RemoteAddr struct field, which is populated by the github.com/sebest/xff middleware.

What is the new behavior?

IP address tracking should be consistent across all Auth code paths.

This commit changes the middleware order so request logging happens
after the Sb-Forwarded-For IP address is added to the request
context. This is necessary because otherwise request logs will always
use legacy behavior for IP address tracking, as logging happens in an
earlier middleware than the SBFF middleware.
This commit replaces all usage of r.RemoteAddr in Auth code with
utilities.GetIPAddress (with the exception of the field's usage in
GetIPAddress itself). The purpose of this commit is to ensure IP
address tracking is consistent across the Auth codebase.
@jnschaeffer jnschaeffer requested a review from a team as a code owner January 21, 2026 04:22
@coveralls
Copy link

coveralls commented Jan 21, 2026

Pull Request Test Coverage Report for Build 21404570306

Details

  • 11 of 12 (91.67%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 68.819%

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/api/mfa.go 8 9 88.89%
Totals Coverage Status
Change from base Build 21404033705: 0.0%
Covered Lines: 14849
Relevant Lines: 21577

💛 - Coveralls

@jnschaeffer jnschaeffer force-pushed the chore/ip-tracking-cleanup branch from 1012283 to 7ee9fe3 Compare January 27, 2026 16:07
@jnschaeffer jnschaeffer merged commit 1ae3a3d into master Jan 27, 2026
7 checks passed
@jnschaeffer jnschaeffer deleted the chore/ip-tracking-cleanup branch January 27, 2026 17:25
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.

3 participants