Skip to content

Conversation

MorquinDevlar
Copy link
Contributor

Added:

  • getClientIP helper function to extract real client IPs from proxy headers
  • X-Real-IP header parsing (higher priority)
  • X-Forwarded-For header parsing with comma-separated IP support
  • Security check to only trust headers from localhost connections

Changed:

  • Web request logging to use getClientIP instead of r.RemoteAddr
  • Log output now shows real client IPs when behind trusted reverse proxy

@MorquinDevlar MorquinDevlar self-assigned this Aug 11, 2025
@MorquinDevlar MorquinDevlar requested a review from Volte6 as a code owner August 11, 2025 20:08
@MorquinDevlar MorquinDevlar added the enhancement New feature or request label Aug 11, 2025
@Jasrags Jasrags requested a review from Copilot August 12, 2025 14:41
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds reverse proxy IP header support for web server logging and introduces secure telnet connection tracking capabilities. The main purpose is to properly log real client IP addresses when the web server is behind a trusted reverse proxy, and to add infrastructure for supporting TLS-enabled telnet connections.

Key changes:

  • Added getClientIP function to extract real client IPs from X-Real-IP and X-Forwarded-For headers when requests come from localhost
  • Added secure telnet port configuration and connection type tracking
  • Enhanced online user display to show connection types (Web, Telnet, TLS)

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/web/web.go Implements getClientIP function and updates logging to use real client IPs
internal/configs/config.network.go Adds SecureTelnetPort and SecureTelnetLocalPort configuration fields
internal/users/userrecord.go Adds connection type detection logic for TLS connections
internal/users/onlineinfo.go Adds ConnectionType field to track connection method
internal/connections/connections.go Adds GetConnectionPort function for port retrieval
internal/connections/connectiondetails.go Implements GetLocalPort method to extract local port from connections
main.go Adds secure telnet local port listening logic
world.go Updates stats collection to include secure telnet ports
internal/web/stats.go Adds SecureTelnetPorts field to stats structure
_datafiles/html/public/online.html Adds Connection column to online users table
_datafiles/html/public/index.html Updates homepage to display secure telnet ports
_datafiles/config.yaml Adds configuration documentation for secure telnet ports

MorquinDevlar and others added 10 commits August 15, 2025 12:12
Added:
  - getClientIP helper function to extract real client IPs from proxy
headers
  - X-Real-IP header parsing (higher priority)
  - X-Forwarded-For header parsing with comma-separated IP support
  - Security check to only trust headers from localhost connections

Changed:
  - Web request logging to use getClientIP instead of r.RemoteAddr
  - Log output now shows real client IPs when behind trusted reverse
proxy
Added:
  - SecureTelnetPort configuration field to Network struct
  - SecureTelnetPorts tracking in web Stats struct
  - Secure telnet port parsing in world stats update
  - Conditional display of secure telnet ports on homepage

  Changed:
  - Web request logging to show real client IPs behind proxy
  - Underlay width for ports display reduced for better visual balance
Overview:
Secure ports only listen to localhost like the LocalPort, and will
display on the website as secure if added to config.

Added:
  - ConnectionType field to OnlineInfo struct showing "Web", "Telnet",
or "Secure"
  - GetLocalPort method to ConnectionDetails for port detection
  - GetConnectionPort helper function in connections package
  - SecureTelnetPort configuration field for localhost-only secure
connections
  - Automatic listening on SecureTelnetPort (localhost-only, like
LocalPort)
  - Connection type detection based on WebSocket status and port number
  - Connection type column to online users HTML table

  Changed:
  - GetOnlineInfo method to determine and include connection type
  - Online page to display how each user is connected
  - Config documentation to clarify SecureTelnetPort behavior
- SecureTelnetPort is now display-only (shown on website, not bound)
- Added SecureTelnetLocalPort for internal binding (where TLS proxy forwards)
- Updated connection detection to check SecureTelnetLocalPort
- This allows proper stunnel4/HAProxy integration:
  * TLS proxy binds to public SecureTelnetPort (e.g., 33334)
  * TLS proxy forwards to SecureTelnetLocalPort (e.g., 9998)
  * Game binds to SecureTelnetLocalPort on localhost only
  * Connections via SecureTelnetLocalPort show as 'TLS' on online page
- Changed SecureTelnetLocalPort from single ConfigInt to ConfigSliceString
- Now supports multiple secure local ports: [9998, 9997]
- Updated connection detection to check all ports in the slice
- Allows multiple TLS proxies to forward to different local ports
- Consistent with SecureTelnetPort being a slice
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Removed:
- Debug logging in GetOnlineInfo that was firing every 10 seconds with stats updates
- These logs were creating unnecessary noise in production environments
Certain files - config.network.go, mapper.go, rooms.go and
userrecords.go always error out when doing the fmtcheck during the make
process.

This hopefully fixes this permanently by comitting the change always
enforced by fmtcheck.
@MorquinDevlar MorquinDevlar force-pushed the morq-reverse-proxy-ip branch from 310b595 to 1ae985b Compare August 15, 2025 10:13
Removed excessive debug comments and "what" comments. Only retained the
relevent stuff.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant