From 1116fc6bb45cb9ce09e2b26b6f71f4f3cabd8cf1 Mon Sep 17 00:00:00 2001 From: Ludovic Vielle Date: Mon, 6 Jul 2026 11:43:39 +0200 Subject: [PATCH] Use clientip.Extract for esign and session IP capture Several HTTP entry points still parsed RemoteAddr directly, so behind a layer-7 proxy they recorded the load balancer IP instead of the signer's. Route NDA acceptance, signing events, document sign/approve, and session updates through clientip.Extract, which honors Forwarded and X-Forwarded-For when trustedproxy allows them. Signed-off-by: Ludovic Vielle --- pkg/server/api/authn/session_middleware.go | 9 ++--- pkg/server/api/clientip/clientip.go | 34 ++++++++++++------- pkg/server/api/clientip/clientip_test.go | 21 ++++++++++++ .../api/console/v1/document_resolvers.go | 12 ++----- pkg/server/api/trust/v1/nda_resolvers.go | 12 ++----- 5 files changed, 51 insertions(+), 37 deletions(-) diff --git a/pkg/server/api/authn/session_middleware.go b/pkg/server/api/authn/session_middleware.go index 74dc6d6d6..c28776ee9 100644 --- a/pkg/server/api/authn/session_middleware.go +++ b/pkg/server/api/authn/session_middleware.go @@ -27,6 +27,7 @@ import ( "go.probo.inc/probo/pkg/gid" "go.probo.inc/probo/pkg/iam" "go.probo.inc/probo/pkg/securecookie" + "go.probo.inc/probo/pkg/server/api/clientip" "go.probo.inc/probo/pkg/server/gqlutils" ) @@ -97,13 +98,7 @@ func NewSessionMiddleware(svc *iam.Service, cookieConfig securecookie.Config) fu } userAgent := r.UserAgent() - // TODO: will work well when no layer 7 proxy is in front of the server - var ipAddress net.IP - if host, _, err := net.SplitHostPort(r.RemoteAddr); err == nil { - ipAddress = net.ParseIP(host) - } else { - ipAddress = net.ParseIP(r.RemoteAddr) - } + ipAddress := net.ParseIP(clientip.Extract(r)) err = svc.SessionService.UpdateSessionInfo(ctx, session.ID, userAgent, ipAddress) if err != nil { diff --git a/pkg/server/api/clientip/clientip.go b/pkg/server/api/clientip/clientip.go index ceca37f0c..a97fdcc75 100644 --- a/pkg/server/api/clientip/clientip.go +++ b/pkg/server/api/clientip/clientip.go @@ -27,28 +27,38 @@ import ( // load balancer closest to us. func Extract(r *http.Request) string { if fwd := r.Header.Get("Forwarded"); fwd != "" { - if ip := parseForwardedFor(fwd); ip != "" { + if ip := parseForwardedFor(fwd); net.ParseIP(ip) != nil { return ip } } if xff := r.Header.Get("X-Forwarded-For"); xff != "" { - if i := strings.LastIndexByte(xff, ','); i != -1 { - xff = xff[i+1:] - } - - xff = strings.TrimSpace(xff) - - if ip, _, err := net.SplitHostPort(xff); err == nil { + if ip := parseXForwardedFor(xff); net.ParseIP(ip) != nil { return ip } - - return xff } - ip, _, err := net.SplitHostPort(r.RemoteAddr) + return extractRemoteAddr(r.RemoteAddr) +} + +func parseXForwardedFor(xff string) string { + if i := strings.LastIndexByte(xff, ','); i != -1 { + xff = xff[i+1:] + } + + xff = strings.TrimSpace(xff) + + if ip, _, err := net.SplitHostPort(xff); err == nil { + return ip + } + + return xff +} + +func extractRemoteAddr(remoteAddr string) string { + ip, _, err := net.SplitHostPort(remoteAddr) if err != nil { - return r.RemoteAddr + return remoteAddr } return ip diff --git a/pkg/server/api/clientip/clientip_test.go b/pkg/server/api/clientip/clientip_test.go index 9f34ad199..2c1fd58d6 100644 --- a/pkg/server/api/clientip/clientip_test.go +++ b/pkg/server/api/clientip/clientip_test.go @@ -110,6 +110,27 @@ func TestExtract(t *testing.T) { headers: map[string]string{"Forwarded": "for=198.51.100.17;proto=https;by=203.0.113.60"}, want: "198.51.100.17", }, + { + name: "unparseable forwarded falls back to remote addr", + remoteAddr: "10.0.0.1:1234", + headers: map[string]string{"Forwarded": "for=unknown"}, + want: "10.0.0.1", + }, + { + name: "unparseable x-forwarded-for falls back to remote addr", + remoteAddr: "10.0.0.1:1234", + headers: map[string]string{"X-Forwarded-For": "unknown"}, + want: "10.0.0.1", + }, + { + name: "unparseable forwarded falls through to x-forwarded-for", + remoteAddr: "10.0.0.1:1234", + headers: map[string]string{ + "Forwarded": "for=unknown", + "X-Forwarded-For": "203.0.113.50", + }, + want: "203.0.113.50", + }, } for _, tt := range tests { diff --git a/pkg/server/api/console/v1/document_resolvers.go b/pkg/server/api/console/v1/document_resolvers.go index d2eedbb37..7dba7929c 100644 --- a/pkg/server/api/console/v1/document_resolvers.go +++ b/pkg/server/api/console/v1/document_resolvers.go @@ -10,7 +10,6 @@ import ( "encoding/base64" "errors" "fmt" - "net" "github.com/vikstrous/dataloadgen" "go.gearno.de/kit/log" @@ -21,6 +20,7 @@ import ( "go.probo.inc/probo/pkg/probo" "go.probo.inc/probo/pkg/resourcealias" "go.probo.inc/probo/pkg/server/api/authn" + "go.probo.inc/probo/pkg/server/api/clientip" "go.probo.inc/probo/pkg/server/api/console/v1/dataloader" "go.probo.inc/probo/pkg/server/api/console/v1/schema" "go.probo.inc/probo/pkg/server/api/console/v1/types" @@ -1433,10 +1433,7 @@ func (r *mutationResolver) SignDocument(ctx context.Context, input types.SignDoc identity := authn.IdentityFromContext(ctx) httpReq := gqlutils.HTTPRequestFromContext(ctx) - signerIP, _, _ := net.SplitHostPort(httpReq.RemoteAddr) - if signerIP == "" { - signerIP = httpReq.RemoteAddr - } + signerIP := clientip.Extract(httpReq) documentVersionSignature, err := r.probo.Documents.SignDocumentVersionByIdentity( ctx, @@ -1487,10 +1484,7 @@ func (r *mutationResolver) ApproveDocumentVersion(ctx context.Context, input typ identity := authn.IdentityFromContext(ctx) httpReq := gqlutils.HTTPRequestFromContext(ctx) - signerIP, _, _ := net.SplitHostPort(httpReq.RemoteAddr) - if signerIP == "" { - signerIP = httpReq.RemoteAddr - } + signerIP := clientip.Extract(httpReq) decision, err := r.probo.DocumentApprovals.Approve(ctx, scope, probo.ApproveDocumentVersionRequest{ DocumentVersionID: input.DocumentVersionID, diff --git a/pkg/server/api/trust/v1/nda_resolvers.go b/pkg/server/api/trust/v1/nda_resolvers.go index 008093bb2..2d9090e86 100644 --- a/pkg/server/api/trust/v1/nda_resolvers.go +++ b/pkg/server/api/trust/v1/nda_resolvers.go @@ -7,13 +7,13 @@ package trust_v1 import ( "context" - "net" "time" "go.gearno.de/kit/log" "go.probo.inc/probo/pkg/coredata" "go.probo.inc/probo/pkg/esign" "go.probo.inc/probo/pkg/server/api/authn" + "go.probo.inc/probo/pkg/server/api/clientip" "go.probo.inc/probo/pkg/server/api/compliancepage" "go.probo.inc/probo/pkg/server/api/trust/v1/schema" "go.probo.inc/probo/pkg/server/api/trust/v1/types" @@ -27,10 +27,7 @@ func (r *mutationResolver) AcceptElectronicSignature(ctx context.Context, input httpReq = gqlutils.HTTPRequestFromContext(ctx) ) - signerIP, _, _ := net.SplitHostPort(httpReq.RemoteAddr) - if signerIP == "" { - signerIP = httpReq.RemoteAddr - } + signerIP := clientip.Extract(httpReq) signature, err := r.esign.AcceptSignature( ctx, @@ -59,10 +56,7 @@ func (r *mutationResolver) RecordSigningEvent(ctx context.Context, input types.R httpReq = gqlutils.HTTPRequestFromContext(ctx) ) - actorIP, _, _ := net.SplitHostPort(httpReq.RemoteAddr) - if actorIP == "" { - actorIP = httpReq.RemoteAddr - } + actorIP := clientip.Extract(httpReq) if err := r.esign.RecordEvent( ctx,