refactor(handlers): resolve client IP via a clientHost helper

Route every HTTP read of the client IP through a single clientHost(r)
helper backed by chi's new middleware.GetClientIP, falling back to the
socket peer from r.RemoteAddr. AddDeviceToAccount now takes a bare client
host instead of a "host:port" RemoteAddr. Behavior is unchanged in this
commit (no ClientIP middleware is wired yet, so the fallback is always
taken); a follow-up wires middleware.ClientIP and removes the deprecated
middleware.RealIP.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tobias Gesellchen
2026-06-28 13:15:49 +02:00
co-authored by Claude Opus 4.8
parent f4c59c1e58
commit 28d7675fc4
7 changed files with 34 additions and 30 deletions
+12
View File
@@ -11,6 +11,7 @@ import (
"time"
"github.com/gesellix/bose-soundtouch/pkg/client"
"github.com/go-chi/chi/v5/middleware"
)
const (
@@ -147,6 +148,17 @@ func clientHostFromRemoteAddr(remoteAddr string) string {
return remoteAddr
}
// clientHost returns the resolved client IP for r: chi's middleware.GetClientIP
// (populated by the ClientIP middleware) when set, falling back to the socket
// peer host from r.RemoteAddr. Returns a bare IP (no port).
func clientHost(r *http.Request) string {
if ip := middleware.GetClientIP(r.Context()); ip != "" {
return ip
}
return clientHostFromRemoteAddr(r.RemoteAddr)
}
// dnsProbeSpeakerRequest is the JSON body for POST /setup/health/dns-path-probe.
type dnsProbeSpeakerRequest struct {
DeviceID string `json:"deviceId,omitempty"`
+1 -1
View File
@@ -78,7 +78,7 @@ func (s *Server) DeprecatedRouteMiddleware(next http.Handler) http.Handler {
if s.deprecatedRoutes.record(key) {
log.Printf("[deprecated-route] %s used by client=%s — use /api%s instead; "+
"the legacy path still works but is slated for removal in a future major release",
sanitizeLog(key), sanitizeLog(clientHostFromRemoteAddr(r.RemoteAddr)), sanitizeLog(pattern))
sanitizeLog(key), sanitizeLog(clientHost(r)), sanitizeLog(pattern))
}
})
}
+7 -11
View File
@@ -8,7 +8,6 @@ import (
"io"
"log"
"math/big"
"net"
"net/http"
"strconv"
"time"
@@ -244,7 +243,7 @@ func (s *Server) HandleMargePowerOn(w http.ResponseWriter, r *http.Request) {
log.Printf("[Marge] Failed to parse power_on body: %v", err)
// Fallback to remote address if body parsing fails
if host, _, err := net.SplitHostPort(r.RemoteAddr); err == nil {
if host := clientHost(r); host != "" {
go s.PrimeDeviceWithSpotify(host)
}
@@ -292,14 +291,11 @@ func (s *Server) HandleMargePowerOn(w http.ResponseWriter, r *http.Request) {
// Prefer the TCP source address over the body's self-reported IP for
// any outbound credential push. The body field is attacker-controllable
// (a malicious LAN-resident speaker can set it to any value), while
// r.RemoteAddr is the actual peer — and if the service runs behind a
// trusted reverse proxy, the TrustedRealIP middleware has already
// rewritten it from X-Real-IP / X-Forwarded-For. We log when the two
// clientHost(r) is the actual peer — and if the service runs behind a
// trusted reverse proxy, the ClientIP middleware has already populated
// the context from X-Forwarded-For. We log when the two
// disagree so the discrepancy is investigable but never trust the body.
remoteHost := ""
if h, _, err := net.SplitHostPort(r.RemoteAddr); err == nil {
remoteHost = h
}
remoteHost := clientHost(r)
if deviceIP != "" && remoteHost != "" && deviceIP != remoteHost {
log.Printf("[Marge] power_on body IP %q differs from TCP source %q for device %s — using TCP source for credential push",
@@ -588,7 +584,7 @@ func (s *Server) HandleMargeAddDevice(w http.ResponseWriter, r *http.Request) {
return
}
deviceID, data, err := marge.AddDeviceToAccount(s.ds, account, body, r.RemoteAddr)
deviceID, data, err := marge.AddDeviceToAccount(s.ds, account, body, clientHost(r))
if err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
return
@@ -657,7 +653,7 @@ func (s *Server) HandleMargeUpdateDevice(w http.ResponseWriter, r *http.Request)
return
}
_, data, err := marge.AddDeviceToAccount(s.ds, account, body, r.RemoteAddr)
_, data, err := marge.AddDeviceToAccount(s.ds, account, body, clientHost(r))
if err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
return
+1 -1
View File
@@ -151,7 +151,7 @@ func buildCustomPlaybackURL(base, audioURL, name string) string {
func (s *Server) HandleSpeakerAuth(w http.ResponseWriter, r *http.Request) {
token := r.Header.Get("Apikeyheader")
if token != "" && s.authProbes != nil {
if s.authProbes.observe(token, clientHostFromRemoteAddr(r.RemoteAddr)) {
if s.authProbes.observe(token, clientHost(r)) {
// Active DNS-path probe: the callback arrival already proved the
// speaker resolved a Bose host through AfterTouch. Returning 403
// makes the speaker treat the key as invalid and refuse the
+1 -1
View File
@@ -25,7 +25,7 @@ const maxUnsupportedBodyLog = 2048
// relies on the route we find out and restore it instead of silently breaking
// it during the refactor.
func (s *Server) HandleUnsupported(w http.ResponseWriter, r *http.Request) {
client := clientHostFromRemoteAddr(r.RemoteAddr)
client := clientHost(r)
var body []byte
if r.Body != nil {
@@ -1,7 +1,6 @@
package handlers
import (
"net"
"net/http"
"time"
@@ -21,8 +20,7 @@ import (
// request.
func (s *Server) PeerObserverMiddleware(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
host, _, err := net.SplitHostPort(r.RemoteAddr)
if err == nil && host != "" {
if host := clientHost(r); host != "" {
s.peerObserver.Signal(host, setup.PeerHit{Path: r.URL.Path, At: time.Now()})
}
+11 -13
View File
@@ -2224,11 +2224,11 @@ func formatRecentResponse(recentObj *models.ServiceRecent, matchingSrc *models.C
// handlers — the persistence layer doesn't distinguish; only the
// response status differs.
//
// remoteAddr is the speaker's address as seen by the HTTP server
// (r.RemoteAddr, "host:port"). When the request body doesn't carry
// an `<ipaddress>` and the datastore has no IP for this device yet,
// we fall back to remoteAddr's host portion. An empty remoteAddr
// is treated as "no fallback available" — never errors.
// clientHost is the speaker's resolved client IP (bare IP, no port),
// as returned by handlers.clientHost(r). When the request body doesn't
// carry an `<ipaddress>` and the datastore has no IP for this device
// yet, we fall back to clientHost. An empty or non-IP clientHost is
// treated as "no fallback available" — never errors.
//
// Timestamps:
// - CreatedOn is preserved from any existing datastore record so a
@@ -2238,7 +2238,7 @@ func formatRecentResponse(recentObj *models.ServiceRecent, matchingSrc *models.C
//
// Returns the persisted deviceID and the marge XML response shape
// (`<device deviceid="…"><createdOn/><ipaddress/><name/><updatedOn/></device>`).
func AddDeviceToAccount(ds *datastore.DataStore, account string, sourceXML []byte, remoteAddr string) (string, []byte, error) {
func AddDeviceToAccount(ds *datastore.DataStore, account string, sourceXML []byte, clientHost string) (string, []byte, error) {
var newDeviceElem struct {
DeviceID string `xml:"deviceid,attr"`
Name string `xml:"name"`
@@ -2277,14 +2277,12 @@ func AddDeviceToAccount(ds *datastore.DataStore, account string, sourceXML []byt
// hitting us through a different network path right now, e.g.
// SSH port-forward, and the persisted IP is the one other
// flows like DNS hints care about). Fall back to the inbound
// connection's remote address only when there's no existing
// IP to preserve. Invalid remoteAddr leaves info.IPAddress
// empty, which the merge then handles.
// connection's client host only when there's no existing
// IP to preserve. An empty or non-IP clientHost leaves
// info.IPAddress empty, which the merge then handles.
if existing == nil || existing.IPAddress == "" {
if remoteAddr != "" {
if host, _, splitErr := net.SplitHostPort(remoteAddr); splitErr == nil {
info.IPAddress = host
}
if clientHost != "" && net.ParseIP(clientHost) != nil {
info.IPAddress = clientHost
}
}