From 5a2c583c8b62fa45444c374bc1bc9cafc3db92e6 Mon Sep 17 00:00:00 2001 From: Alex Ababii Date: Fri, 4 Sep 2026 14:40:39 +0200 Subject: [PATCH 1/3] upd deprecated chi RealIP middelware --- pkg/service/debug/service.go | 2 +- services/idp/pkg/server/http/server.go | 2 +- .../invitations/pkg/server/http/server.go | 2 +- services/ocs/pkg/server/http/server.go | 2 +- services/proxy/pkg/command/server.go | 3 +- services/proxy/pkg/middleware/accesslog.go | 2 +- .../pkg/middleware/clientip_chain_test.go | 69 +++++++++++++++++++ .../proxy/pkg/middleware/context_logger.go | 3 +- services/proxy/pkg/middleware/oidc_auth.go | 3 +- services/settings/pkg/server/http/server.go | 2 +- services/thumbnails/pkg/server/http/server.go | 2 +- services/web/pkg/server/http/server.go | 2 +- services/webdav/pkg/server/http/server.go | 2 +- services/webfinger/pkg/server/http/server.go | 2 +- 14 files changed, 85 insertions(+), 13 deletions(-) create mode 100644 services/proxy/pkg/middleware/clientip_chain_test.go diff --git a/pkg/service/debug/service.go b/pkg/service/debug/service.go index 026f2e90ec..8fda90584d 100644 --- a/pkg/service/debug/service.go +++ b/pkg/service/debug/service.go @@ -73,7 +73,7 @@ func NewService(opts ...Option) *http.Server { return baseCtx }, Handler: alice.New( - chimiddleware.RealIP, + chimiddleware.ClientIPFromRemoteAddr, chimiddleware.RequestID, middleware.NoCache, middleware.Cors( diff --git a/services/idp/pkg/server/http/server.go b/services/idp/pkg/server/http/server.go index 4a74785943..e500c632bc 100644 --- a/services/idp/pkg/server/http/server.go +++ b/services/idp/pkg/server/http/server.go @@ -57,7 +57,7 @@ func Server(opts ...Option) (http.Service, error) { svc.Logger(options.Logger), svc.Config(options.Config), svc.Middleware( - chimiddleware.RealIP, + chimiddleware.ClientIPFromRemoteAddr, chimiddleware.RequestID, middleware.TraceContext, middleware.NoCache, diff --git a/services/invitations/pkg/server/http/server.go b/services/invitations/pkg/server/http/server.go index cc60b27747..78dfdce3e9 100644 --- a/services/invitations/pkg/server/http/server.go +++ b/services/invitations/pkg/server/http/server.go @@ -43,7 +43,7 @@ func Server(opts ...Option) (ohttp.Service, error) { mux := chi.NewMux() - mux.Use(chimiddleware.RealIP) + mux.Use(chimiddleware.ClientIPFromRemoteAddr) mux.Use(chimiddleware.RequestID) mux.Use(middleware.TraceContext) mux.Use(middleware.NoCache) diff --git a/services/ocs/pkg/server/http/server.go b/services/ocs/pkg/server/http/server.go index 74c42b43c4..6dc39e235f 100644 --- a/services/ocs/pkg/server/http/server.go +++ b/services/ocs/pkg/server/http/server.go @@ -53,7 +53,7 @@ func Server(opts ...Option) (http.Service, error) { svc.Logger(options.Logger), svc.Config(options.Config), svc.Middleware( - chimiddleware.RealIP, + chimiddleware.ClientIPFromRemoteAddr, chimiddleware.RequestID, middleware.NoCache, middleware.Cors( diff --git a/services/proxy/pkg/command/server.go b/services/proxy/pkg/command/server.go index 2a9a5cc0ba..1ea957c77b 100644 --- a/services/proxy/pkg/command/server.go +++ b/services/proxy/pkg/command/server.go @@ -341,7 +341,8 @@ func loadMiddlewares(logger log.Logger, cfg *config.Config, } return alice.New( - chimiddleware.RealIP, + chimiddleware.ClientIPFromRemoteAddr, + chimiddleware.ClientIPFromXFF(), chimiddleware.RequestID, // 1. Logging & Tracing first diff --git a/services/proxy/pkg/middleware/accesslog.go b/services/proxy/pkg/middleware/accesslog.go index 23c58f4d24..316271d4f9 100644 --- a/services/proxy/pkg/middleware/accesslog.go +++ b/services/proxy/pkg/middleware/accesslog.go @@ -25,7 +25,7 @@ func AccessLog(logger log.Logger) func(http.Handler) http.Handler { Str("proto", r.Proto). Str(log.RequestIDString, requestID). Str("traceid", spanContext.TraceID().String()). - Str("remote-addr", r.RemoteAddr). + Str("remote-addr", middleware.GetClientIP(r.Context())). Str("method", r.Method). Int("status", wrap.Status()). Str("path", r.URL.Path). diff --git a/services/proxy/pkg/middleware/clientip_chain_test.go b/services/proxy/pkg/middleware/clientip_chain_test.go new file mode 100644 index 0000000000..1ac562ebd9 --- /dev/null +++ b/services/proxy/pkg/middleware/clientip_chain_test.go @@ -0,0 +1,69 @@ +package middleware_test + +import ( + "fmt" + "net/http" + "net/http/httptest" + "testing" + + chimiddleware "github.com/go-chi/chi/v5/middleware" + + "gotest.tools/v3/assert" +) + +// TestClientIPChain verifies the order in which the chi ClientIP middlewares +// are chained for the proxy: ClientIPFromRemoteAddr runs first and provides the +// fallback, ClientIPFromXFF overwrites it with the rightmost X-Forwarded-For +// value when the header is present. This mirrors the alice chain configured in +// services/proxy/pkg/command/server.go. +func TestClientIPChain(t *testing.T) { + handler := chimiddleware.ClientIPFromRemoteAddr(chimiddleware.ClientIPFromXFF()(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = fmt.Fprint(w, chimiddleware.GetClientIP(r.Context())) + }))) + + tests := []struct { + name string + remoteAddr string + forwarded []string + expected string + }{ + { + name: "reverse proxy sets X-Forwarded-For", + remoteAddr: "192.0.2.1:1234", + forwarded: []string{"198.51.100.10"}, + expected: "198.51.100.10", + }, + { + name: "rightmost forwarded entry wins", + remoteAddr: "192.0.2.1:1234", + forwarded: []string{"198.51.100.10, 203.0.113.5"}, + expected: "203.0.113.5", + }, + { + name: "direct connection falls back to remote addr", + remoteAddr: "192.0.2.1:1234", + expected: "192.0.2.1", + }, + { + name: "unparseable forwarded falls back to remote addr", + remoteAddr: "192.0.2.1:1234", + forwarded: []string{"not-an-ip"}, + expected: "192.0.2.1", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.RemoteAddr = tt.remoteAddr + for _, f := range tt.forwarded { + req.Header.Add("X-Forwarded-For", f) + } + + handler.ServeHTTP(rec, req) + + assert.Equal(t, rec.Body.String(), tt.expected) + }) + } +} diff --git a/services/proxy/pkg/middleware/context_logger.go b/services/proxy/pkg/middleware/context_logger.go index 5f8fd326d8..b323d0a111 100644 --- a/services/proxy/pkg/middleware/context_logger.go +++ b/services/proxy/pkg/middleware/context_logger.go @@ -3,6 +3,7 @@ package middleware import ( "net/http" + "github.com/go-chi/chi/v5/middleware" "github.com/opencloud-eu/opencloud/pkg/log" ) @@ -12,7 +13,7 @@ func ContextLogger(logger log.Logger) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctx := logger.With(). - Str("remoteAddr", r.RemoteAddr). + Str("remoteAddr", middleware.GetClientIP(r.Context())). Str(log.RequestIDString, r.Header.Get("X-Request-ID")). Str("proto", r.Proto). Str("method", r.Method). diff --git a/services/proxy/pkg/middleware/oidc_auth.go b/services/proxy/pkg/middleware/oidc_auth.go index b80f7fcb7a..059a486e50 100644 --- a/services/proxy/pkg/middleware/oidc_auth.go +++ b/services/proxy/pkg/middleware/oidc_auth.go @@ -14,6 +14,7 @@ import ( "golang.org/x/crypto/sha3" "golang.org/x/oauth2" + "github.com/go-chi/chi/v5/middleware" "github.com/opencloud-eu/opencloud/pkg/log" "github.com/opencloud-eu/opencloud/pkg/oidc" "github.com/opencloud-eu/opencloud/services/proxy/pkg/config" @@ -217,7 +218,7 @@ func (m *OIDCAuthenticator) Authenticate(r *http.Request) (*http.Request, bool) Str("authenticator", "oidc"). Str("path", r.URL.Path). Str("user_agent", r.UserAgent()). - Str("client.address", r.Header.Get("X-Forwarded-For")). + Str("client.address", middleware.GetClientIP(r.Context())). Str("network.peer.address", host). Str("network.peer.port", port). Msg("failed to authenticate the request") diff --git a/services/settings/pkg/server/http/server.go b/services/settings/pkg/server/http/server.go index 6c5b85c115..0b2156ae88 100644 --- a/services/settings/pkg/server/http/server.go +++ b/services/settings/pkg/server/http/server.go @@ -42,7 +42,7 @@ func Server(opts ...Option) (ohttp.Service, error) { mux := chi.NewMux() - mux.Use(chimiddleware.RealIP) + mux.Use(chimiddleware.ClientIPFromRemoteAddr) mux.Use(chimiddleware.RequestID) mux.Use(middleware.NoCache) mux.Use(middleware.Cors( diff --git a/services/thumbnails/pkg/server/http/server.go b/services/thumbnails/pkg/server/http/server.go index d9c2a1c1fa..bf3996a25a 100644 --- a/services/thumbnails/pkg/server/http/server.go +++ b/services/thumbnails/pkg/server/http/server.go @@ -38,7 +38,7 @@ func Server(opts ...Option) (http.Service, error) { svc.Logger(options.Logger), svc.Config(options.Config), svc.Middleware( - middleware.RealIP, + middleware.ClientIPFromRemoteAddr, middleware.RequestID, opencloudmiddleware.Cors( cors.Logger(options.Logger), diff --git a/services/web/pkg/server/http/server.go b/services/web/pkg/server/http/server.go index 0b9621c798..cd43f95073 100644 --- a/services/web/pkg/server/http/server.go +++ b/services/web/pkg/server/http/server.go @@ -118,7 +118,7 @@ func Server(opts ...Option) (http.Service, error) { svc.Config(options.Config), svc.GatewaySelector(gatewaySelector), svc.Middleware( - chimiddleware.RealIP, + chimiddleware.ClientIPFromRemoteAddr, chimiddleware.RequestID, chimiddleware.Compress(5), middleware.NoCache, diff --git a/services/webdav/pkg/server/http/server.go b/services/webdav/pkg/server/http/server.go index 4e7c794f24..30d53be1be 100644 --- a/services/webdav/pkg/server/http/server.go +++ b/services/webdav/pkg/server/http/server.go @@ -38,7 +38,7 @@ func Server(opts ...Option) (http.Service, error) { svc.Logger(options.Logger), svc.Config(options.Config), svc.Middleware( - chimiddleware.RealIP, + chimiddleware.ClientIPFromRemoteAddr, chimiddleware.RequestID, middleware.NoCache, middleware.Cors( diff --git a/services/webfinger/pkg/server/http/server.go b/services/webfinger/pkg/server/http/server.go index dbe91b0ca4..bd9677d185 100644 --- a/services/webfinger/pkg/server/http/server.go +++ b/services/webfinger/pkg/server/http/server.go @@ -45,7 +45,7 @@ func Server(opts ...Option) (ohttp.Service, error) { mux := chi.NewMux() - mux.Use(chimiddleware.RealIP) + mux.Use(chimiddleware.ClientIPFromRemoteAddr) mux.Use(chimiddleware.RequestID) mux.Use(middleware.TraceContext) mux.Use(middleware.NoCache) From 4f6324e64fc3b1f2562378750979f8b37300adaf Mon Sep 17 00:00:00 2001 From: Alex Ababii Date: Fri, 4 Sep 2026 14:50:35 +0200 Subject: [PATCH 2/3] upd test --- .../proxy/pkg/middleware/clientip_chain_test.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/services/proxy/pkg/middleware/clientip_chain_test.go b/services/proxy/pkg/middleware/clientip_chain_test.go index 1ac562ebd9..778023cfe7 100644 --- a/services/proxy/pkg/middleware/clientip_chain_test.go +++ b/services/proxy/pkg/middleware/clientip_chain_test.go @@ -1,7 +1,6 @@ package middleware_test import ( - "fmt" "net/http" "net/http/httptest" "testing" @@ -17,10 +16,6 @@ import ( // value when the header is present. This mirrors the alice chain configured in // services/proxy/pkg/command/server.go. func TestClientIPChain(t *testing.T) { - handler := chimiddleware.ClientIPFromRemoteAddr(chimiddleware.ClientIPFromXFF()(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - _, _ = fmt.Fprint(w, chimiddleware.GetClientIP(r.Context())) - }))) - tests := []struct { name string remoteAddr string @@ -54,16 +49,20 @@ func TestClientIPChain(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - rec := httptest.NewRecorder() + var got string + handler := chimiddleware.ClientIPFromRemoteAddr(chimiddleware.ClientIPFromXFF()(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + got = chimiddleware.GetClientIP(r.Context()) + }))) + req := httptest.NewRequest(http.MethodGet, "/", nil) req.RemoteAddr = tt.remoteAddr for _, f := range tt.forwarded { req.Header.Add("X-Forwarded-For", f) } - handler.ServeHTTP(rec, req) + handler.ServeHTTP(httptest.NewRecorder(), req) - assert.Equal(t, rec.Body.String(), tt.expected) + assert.Equal(t, got, tt.expected) }) } } From e2f4e7e38631c509b7988fa83f018913e93e3a74 Mon Sep 17 00:00:00 2001 From: Alex Ababii Date: Tue, 8 Sep 2026 16:02:34 +0200 Subject: [PATCH 3/3] select correct chi middleware to get real IP based on config values --- pkg/middleware/header.go | 4 + pkg/service/debug/service.go | 2 +- .../collaboration/pkg/middleware/accesslog.go | 2 +- .../collaboration/pkg/server/http/server.go | 1 + services/idp/pkg/server/http/server.go | 2 +- .../invitations/pkg/server/http/server.go | 2 +- services/ocs/pkg/server/http/server.go | 2 +- services/proxy/pkg/command/clientip_test.go | 100 ++++++++++++++++++ services/proxy/pkg/command/server.go | 20 +++- services/proxy/pkg/config/config.go | 15 +++ .../pkg/config/defaults/defaultconfig.go | 5 + services/proxy/pkg/config/parser/parse.go | 10 ++ .../pkg/middleware/clientip_chain_test.go | 68 ------------ services/proxy/pkg/proxy/proxy.go | 8 ++ services/proxy/pkg/proxy/rewrite_test.go | 73 +++++++++++++ services/settings/pkg/server/http/server.go | 2 +- services/thumbnails/pkg/server/http/server.go | 2 +- services/web/pkg/server/http/server.go | 2 +- services/webdav/pkg/server/http/server.go | 2 +- services/webfinger/pkg/server/http/server.go | 2 +- 20 files changed, 244 insertions(+), 80 deletions(-) create mode 100644 services/proxy/pkg/command/clientip_test.go delete mode 100644 services/proxy/pkg/middleware/clientip_chain_test.go create mode 100644 services/proxy/pkg/proxy/rewrite_test.go diff --git a/pkg/middleware/header.go b/pkg/middleware/header.go index 0f50a4e413..00ad140553 100644 --- a/pkg/middleware/header.go +++ b/pkg/middleware/header.go @@ -10,6 +10,10 @@ import ( rscors "github.com/rs/cors" ) +// DefaultClientIPHeader is the header the proxy uses to propagate the resolved +// client IP to the downstream services. +const DefaultClientIPHeader = "X-Client-Ip" + // NoCache writes required cache headers to all requests. func NoCache(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/pkg/service/debug/service.go b/pkg/service/debug/service.go index 8fda90584d..7cb3223cf5 100644 --- a/pkg/service/debug/service.go +++ b/pkg/service/debug/service.go @@ -73,7 +73,7 @@ func NewService(opts ...Option) *http.Server { return baseCtx }, Handler: alice.New( - chimiddleware.ClientIPFromRemoteAddr, + chimiddleware.ClientIPFromHeader(middleware.DefaultClientIPHeader), chimiddleware.RequestID, middleware.NoCache, middleware.Cors( diff --git a/services/collaboration/pkg/middleware/accesslog.go b/services/collaboration/pkg/middleware/accesslog.go index 7b765ad24e..69ddbc0872 100644 --- a/services/collaboration/pkg/middleware/accesslog.go +++ b/services/collaboration/pkg/middleware/accesslog.go @@ -25,7 +25,7 @@ func AccessLog(logger log.Logger) func(http.Handler) http.Handler { Str("proto", r.Proto). Str(log.RequestIDString, requestID). Str("traceid", spanContext.TraceID().String()). - Str("remote-addr", r.RemoteAddr). + Str("remote-addr", middleware.GetClientIP(r.Context())). Str("method", r.Method). Str("wopi-action", r.Header.Get("X-WOPI-Override")). Int("status", wrap.Status()). diff --git a/services/collaboration/pkg/server/http/server.go b/services/collaboration/pkg/server/http/server.go index 328763a209..17603a8ffb 100644 --- a/services/collaboration/pkg/server/http/server.go +++ b/services/collaboration/pkg/server/http/server.go @@ -40,6 +40,7 @@ func Server(opts ...Option) (http.Service, error) { } middlewares := []func(stdhttp.Handler) stdhttp.Handler{ + chimiddleware.ClientIPFromHeader(middleware.DefaultClientIPHeader), chimiddleware.RequestID, middleware.Version( options.Config.Service.Name, diff --git a/services/idp/pkg/server/http/server.go b/services/idp/pkg/server/http/server.go index e500c632bc..a782a52446 100644 --- a/services/idp/pkg/server/http/server.go +++ b/services/idp/pkg/server/http/server.go @@ -57,7 +57,7 @@ func Server(opts ...Option) (http.Service, error) { svc.Logger(options.Logger), svc.Config(options.Config), svc.Middleware( - chimiddleware.ClientIPFromRemoteAddr, + chimiddleware.ClientIPFromHeader(middleware.DefaultClientIPHeader), chimiddleware.RequestID, middleware.TraceContext, middleware.NoCache, diff --git a/services/invitations/pkg/server/http/server.go b/services/invitations/pkg/server/http/server.go index 78dfdce3e9..5955d8c8dc 100644 --- a/services/invitations/pkg/server/http/server.go +++ b/services/invitations/pkg/server/http/server.go @@ -43,7 +43,7 @@ func Server(opts ...Option) (ohttp.Service, error) { mux := chi.NewMux() - mux.Use(chimiddleware.ClientIPFromRemoteAddr) + mux.Use(chimiddleware.ClientIPFromHeader(middleware.DefaultClientIPHeader)) mux.Use(chimiddleware.RequestID) mux.Use(middleware.TraceContext) mux.Use(middleware.NoCache) diff --git a/services/ocs/pkg/server/http/server.go b/services/ocs/pkg/server/http/server.go index 6dc39e235f..8fe5ea70e7 100644 --- a/services/ocs/pkg/server/http/server.go +++ b/services/ocs/pkg/server/http/server.go @@ -53,7 +53,7 @@ func Server(opts ...Option) (http.Service, error) { svc.Logger(options.Logger), svc.Config(options.Config), svc.Middleware( - chimiddleware.ClientIPFromRemoteAddr, + chimiddleware.ClientIPFromHeader(middleware.DefaultClientIPHeader), chimiddleware.RequestID, middleware.NoCache, middleware.Cors( diff --git a/services/proxy/pkg/command/clientip_test.go b/services/proxy/pkg/command/clientip_test.go new file mode 100644 index 0000000000..08b4aa4635 --- /dev/null +++ b/services/proxy/pkg/command/clientip_test.go @@ -0,0 +1,100 @@ +package command + +import ( + "net/http" + "net/http/httptest" + "testing" + + chimiddleware "github.com/go-chi/chi/v5/middleware" + + "github.com/opencloud-eu/opencloud/services/proxy/pkg/config" + "gotest.tools/v3/assert" +) + +// TestClientIPMiddleware verifies that clientIPMiddleware maps the configured +// strategy to the right chi ClientIPFrom* middleware and that the resolved IP +// matches the expected behaviour for each supported deployment case. +func TestClientIPMiddleware(t *testing.T) { + tests := []struct { + name string + strategy string + cfg config.ClientIP + remoteAddr string + headers map[string]string + expected string + }{ + { + name: "remote_addr direct exposure", + strategy: "remote_addr", + remoteAddr: "192.0.2.1:1234", + expected: "192.0.2.1", + }, + { + name: "remote_addr ignores spoofed xff", + strategy: "remote_addr", + remoteAddr: "192.0.2.1:1234", + headers: map[string]string{"X-Forwarded-For": "203.0.113.9"}, + expected: "192.0.2.1", + }, + { + name: "header strategy reads trusted header", + strategy: "header", + cfg: config.ClientIP{Header: "X-Real-IP"}, + remoteAddr: "192.0.2.1:1234", + headers: map[string]string{"X-Real-IP": "198.51.100.7"}, + expected: "198.51.100.7", + }, + { + name: "xff rightmost entry wins", + strategy: "xff", + remoteAddr: "192.0.2.1:1234", + headers: map[string]string{"X-Forwarded-For": "198.51.100.10, 203.0.113.5"}, + expected: "203.0.113.5", + }, + { + name: "xff skips trusted prefixes", + strategy: "xff", + cfg: config.ClientIP{TrustedPrefixes: []string{"10.0.0.0/8"}}, + remoteAddr: "10.0.0.5:1234", + headers: map[string]string{"X-Forwarded-For": "198.51.100.10, 10.0.0.5"}, + expected: "198.51.100.10", + }, + { + name: "xff_trusted_hops reads nth hop", + strategy: "xff_trusted_hops", + cfg: config.ClientIP{TrustedHops: 1}, + remoteAddr: "10.0.0.5:1234", + headers: map[string]string{"X-Forwarded-For": "198.51.100.10, 10.0.0.5"}, + expected: "10.0.0.5", + }, + { + name: "unknown strategy falls back to remote_addr", + strategy: "something_else", + remoteAddr: "192.0.2.1:1234", + headers: map[string]string{"X-Forwarded-For": "203.0.113.9"}, + expected: "192.0.2.1", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := tt.cfg + cfg.Strategy = tt.strategy + + var got string + handler := clientIPMiddleware(&config.Config{ClientIP: cfg})(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + got = chimiddleware.GetClientIP(r.Context()) + })) + + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.RemoteAddr = tt.remoteAddr + for k, v := range tt.headers { + req.Header.Set(k, v) + } + + handler.ServeHTTP(httptest.NewRecorder(), req) + + assert.Equal(t, got, tt.expected) + }) + } +} diff --git a/services/proxy/pkg/command/server.go b/services/proxy/pkg/command/server.go index 1ea957c77b..509bd40637 100644 --- a/services/proxy/pkg/command/server.go +++ b/services/proxy/pkg/command/server.go @@ -341,8 +341,7 @@ func loadMiddlewares(logger log.Logger, cfg *config.Config, } return alice.New( - chimiddleware.ClientIPFromRemoteAddr, - chimiddleware.ClientIPFromXFF(), + clientIPMiddleware(cfg), chimiddleware.RequestID, // 1. Logging & Tracing first @@ -412,3 +411,20 @@ func loadMiddlewares(logger log.Logger, cfg *config.Config, ), ) } + +// clientIPMiddleware returns the chi ClientIPFrom* middleware matching the +// configured client IP strategy. +func clientIPMiddleware(cfg *config.Config) func(http.Handler) http.Handler { + switch cfg.ClientIP.Strategy { + case config.ClientIPStrategyHeader: + return chimiddleware.ClientIPFromHeader(cfg.ClientIP.Header) + case config.ClientIPStrategyXFF: + return chimiddleware.ClientIPFromXFF(cfg.ClientIP.TrustedPrefixes...) + case config.ClientIPStrategyXFFTrustedHops: + return chimiddleware.ClientIPFromXFFTrustedProxies(cfg.ClientIP.TrustedHops) + case config.ClientIPStrategyRemoteAddr: + fallthrough + default: + return chimiddleware.ClientIPFromRemoteAddr + } +} diff --git a/services/proxy/pkg/config/config.go b/services/proxy/pkg/config/config.go index 05344049a6..a3bb86706a 100644 --- a/services/proxy/pkg/config/config.go +++ b/services/proxy/pkg/config/config.go @@ -19,6 +19,8 @@ type Config struct { HTTP HTTP `yaml:"http"` + ClientIP ClientIP `yaml:"client_ip"` + Reva *shared.Reva `yaml:"reva"` GRPCClientTLS *shared.GRPCClientTLS `yaml:"grpc_client_tls"` GrpcClient client.Client `yaml:"-"` @@ -95,6 +97,14 @@ var ( RouteTypes = []RouteType{QueryRoute, RegexRoute, PrefixRoute} ) +// ClientIP configures how the proxy resolves the real client IP address. +type ClientIP struct { + Strategy string `yaml:"strategy" env:"PROXY_CLIENT_IP_STRATEGY" desc:"Determines how the proxy resolves the real client IP. Supported values: remote_addr, header, xff, xff_trusted_hops." introductionVersion:"%NEXT%"` + Header string `yaml:"header" env:"PROXY_CLIENT_IP_HEADER" desc:"The header name to use for the 'header' strategy, e.g. X-Real-IP or CF-Connecting-IP." introductionVersion:"%NEXT%"` + TrustedPrefixes []string `yaml:"trusted_prefixes" env:"PROXY_CLIENT_IP_TRUSTED_PREFIXES" desc:"CIDRs of trusted proxies used by the 'xff' strategy." introductionVersion:"%NEXT%"` + TrustedHops int `yaml:"trusted_hops" env:"PROXY_CLIENT_IP_TRUSTED_HOPS" desc:"Number of trusted proxy hops used by the 'xff_trusted_hops' strategy." introductionVersion:"%NEXT%"` +} + // AuthMiddleware configures the proxy http auth middleware. type AuthMiddleware struct { CredentialsByUserAgent map[string]string `yaml:"credentials_by_user_agent"` @@ -111,6 +121,11 @@ const ( AccessTokenVerificationJWT = "jwt" // tdb: // AccessTokenVerificationIntrospect = "introspect" + + ClientIPStrategyRemoteAddr = "remote_addr" + ClientIPStrategyHeader = "header" + ClientIPStrategyXFF = "xff" + ClientIPStrategyXFFTrustedHops = "xff_trusted_hops" ) // OIDC is the config for the OpenID-Connect middleware. If set the proxy will try to authenticate every request diff --git a/services/proxy/pkg/config/defaults/defaultconfig.go b/services/proxy/pkg/config/defaults/defaultconfig.go index 3614e1f0a8..bf4ae339d4 100644 --- a/services/proxy/pkg/config/defaults/defaultconfig.go +++ b/services/proxy/pkg/config/defaults/defaultconfig.go @@ -46,6 +46,11 @@ func DefaultConfig() *config.Config { ExpectContinueTimeout: 1 * time.Second, }, }, + ClientIP: config.ClientIP{ + Strategy: config.ClientIPStrategyRemoteAddr, + Header: "X-Real-IP", + TrustedHops: 1, // this might need to be adjusted + }, Service: config.Service{ Name: "proxy", }, diff --git a/services/proxy/pkg/config/parser/parse.go b/services/proxy/pkg/config/parser/parse.go index 7a8be8c7a8..f85fddbe5a 100644 --- a/services/proxy/pkg/config/parser/parse.go +++ b/services/proxy/pkg/config/parser/parse.go @@ -57,6 +57,16 @@ func Validate(cfg *config.Config) error { ) } + if cfg.ClientIP.Strategy != config.ClientIPStrategyRemoteAddr && cfg.ClientIP.Strategy != config.ClientIPStrategyHeader && + cfg.ClientIP.Strategy != config.ClientIPStrategyXFF && cfg.ClientIP.Strategy != config.ClientIPStrategyXFFTrustedHops { + return fmt.Errorf( + "Invalid value '%s' for 'client_ip.strategy' in service %s. Possible values are: '%s', '%s', '%s' or '%s'.", + cfg.ClientIP.Strategy, cfg.Service.Name, + config.ClientIPStrategyRemoteAddr, config.ClientIPStrategyHeader, + config.ClientIPStrategyXFF, config.ClientIPStrategyXFFTrustedHops, + ) + } + if cfg.ServiceAccount.ServiceAccountID == "" { return shared.MissingServiceAccountID(cfg.Service.Name) } diff --git a/services/proxy/pkg/middleware/clientip_chain_test.go b/services/proxy/pkg/middleware/clientip_chain_test.go deleted file mode 100644 index 778023cfe7..0000000000 --- a/services/proxy/pkg/middleware/clientip_chain_test.go +++ /dev/null @@ -1,68 +0,0 @@ -package middleware_test - -import ( - "net/http" - "net/http/httptest" - "testing" - - chimiddleware "github.com/go-chi/chi/v5/middleware" - - "gotest.tools/v3/assert" -) - -// TestClientIPChain verifies the order in which the chi ClientIP middlewares -// are chained for the proxy: ClientIPFromRemoteAddr runs first and provides the -// fallback, ClientIPFromXFF overwrites it with the rightmost X-Forwarded-For -// value when the header is present. This mirrors the alice chain configured in -// services/proxy/pkg/command/server.go. -func TestClientIPChain(t *testing.T) { - tests := []struct { - name string - remoteAddr string - forwarded []string - expected string - }{ - { - name: "reverse proxy sets X-Forwarded-For", - remoteAddr: "192.0.2.1:1234", - forwarded: []string{"198.51.100.10"}, - expected: "198.51.100.10", - }, - { - name: "rightmost forwarded entry wins", - remoteAddr: "192.0.2.1:1234", - forwarded: []string{"198.51.100.10, 203.0.113.5"}, - expected: "203.0.113.5", - }, - { - name: "direct connection falls back to remote addr", - remoteAddr: "192.0.2.1:1234", - expected: "192.0.2.1", - }, - { - name: "unparseable forwarded falls back to remote addr", - remoteAddr: "192.0.2.1:1234", - forwarded: []string{"not-an-ip"}, - expected: "192.0.2.1", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - var got string - handler := chimiddleware.ClientIPFromRemoteAddr(chimiddleware.ClientIPFromXFF()(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - got = chimiddleware.GetClientIP(r.Context()) - }))) - - req := httptest.NewRequest(http.MethodGet, "/", nil) - req.RemoteAddr = tt.remoteAddr - for _, f := range tt.forwarded { - req.Header.Add("X-Forwarded-For", f) - } - - handler.ServeHTTP(httptest.NewRecorder(), req) - - assert.Equal(t, got, tt.expected) - }) - } -} diff --git a/services/proxy/pkg/proxy/proxy.go b/services/proxy/pkg/proxy/proxy.go index 98f340b4b7..692a923ad0 100644 --- a/services/proxy/pkg/proxy/proxy.go +++ b/services/proxy/pkg/proxy/proxy.go @@ -16,6 +16,9 @@ import ( "github.com/opencloud-eu/opencloud/services/proxy/pkg/proxy/policy" "github.com/opencloud-eu/opencloud/services/proxy/pkg/router" "github.com/rs/zerolog" + + chimiddleware "github.com/go-chi/chi/v5/middleware" + pkgmiddleware "github.com/opencloud-eu/opencloud/pkg/middleware" ) // MultiHostReverseProxy extends "httputil" to support multiple hosts with different policies @@ -42,6 +45,11 @@ func NewMultiHostReverseProxy(opts ...Option) (*MultiHostReverseProxy, error) { } rp.Rewrite = func(r *httputil.ProxyRequest) { + // Set the resolved client IP to header so the downstream services can use it + if clientIP := chimiddleware.GetClientIP(r.In.Context()); clientIP != "" { + r.Out.Header.Set(pkgmiddleware.DefaultClientIPHeader, clientIP) + } + // Check if datagateway middleware already handled this request if skip, _ := r.In.Context().Value(middleware.DatagatewaySkipRoutingKey).(bool); skip { r.SetXForwarded() diff --git a/services/proxy/pkg/proxy/rewrite_test.go b/services/proxy/pkg/proxy/rewrite_test.go new file mode 100644 index 0000000000..f494834a9e --- /dev/null +++ b/services/proxy/pkg/proxy/rewrite_test.go @@ -0,0 +1,73 @@ +package proxy + +import ( + "bytes" + "context" + "io" + "net/http" + "net/http/httptest" + "net/http/httputil" + "testing" + + chimiddleware "github.com/go-chi/chi/v5/middleware" + + pkgmiddleware "github.com/opencloud-eu/opencloud/pkg/middleware" + "github.com/opencloud-eu/opencloud/services/proxy/pkg/middleware" + "gotest.tools/v3/assert" +) + +// TestRewriteForwardsClientIP verifies that the proxy overwrites the forwarded +// IP header on the outbound request with the client IP resolved by the +// ClientIPFrom* middleware, so downstream services can trust it. +func TestRewriteForwardsClientIP(t *testing.T) { + cfg := testConfig(nil) + + rp := newTestProxy(cfg, func(req *http.Request) *http.Response { + return &http.Response{StatusCode: 200, Body: io.NopCloser(bytes.NewBufferString("OK")), Header: make(http.Header)} + }) + + inReq := httptest.NewRequest(http.MethodGet, "/", nil) + inReq.RemoteAddr = "192.0.2.1:1234" + + // Resolve the client IP into the request context via a chi middleware. + var withCtx *http.Request + chimiddleware.ClientIPFromRemoteAddr(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + withCtx = r + })).ServeHTTP(httptest.NewRecorder(), inReq) + + // Take the datagateway skip path so Rewrite does not need routing info. + ctx := context.WithValue(withCtx.Context(), middleware.DatagatewaySkipRoutingKey, true) + withCtx = withCtx.WithContext(ctx) + + pr := &httputil.ProxyRequest{ + In: withCtx, + Out: withCtx.Clone(context.Background()), + } + rp.Rewrite(pr) + + assert.Equal(t, pr.Out.Header.Get(pkgmiddleware.DefaultClientIPHeader), "192.0.2.1") +} + +// TestRewriteDoesNotForwardEmptyClientIP verifies the header is left unset when +// the client IP could not be resolved (no middleware populated the context). +func TestRewriteDoesNotForwardEmptyClientIP(t *testing.T) { + cfg := testConfig(nil) + + rp := newTestProxy(cfg, func(req *http.Request) *http.Response { + return &http.Response{StatusCode: 200, Body: io.NopCloser(bytes.NewBufferString("OK")), Header: make(http.Header)} + }) + + inReq := httptest.NewRequest(http.MethodGet, "/", nil) + inReq.RemoteAddr = "192.0.2.1:1234" + + ctx := context.WithValue(inReq.Context(), middleware.DatagatewaySkipRoutingKey, true) + inReq = inReq.WithContext(ctx) + + pr := &httputil.ProxyRequest{ + In: inReq, + Out: inReq.Clone(context.Background()), + } + rp.Rewrite(pr) + + assert.Equal(t, pr.Out.Header.Get(pkgmiddleware.DefaultClientIPHeader), "") +} diff --git a/services/settings/pkg/server/http/server.go b/services/settings/pkg/server/http/server.go index 0b2156ae88..abd083f948 100644 --- a/services/settings/pkg/server/http/server.go +++ b/services/settings/pkg/server/http/server.go @@ -42,7 +42,7 @@ func Server(opts ...Option) (ohttp.Service, error) { mux := chi.NewMux() - mux.Use(chimiddleware.ClientIPFromRemoteAddr) + mux.Use(chimiddleware.ClientIPFromHeader(middleware.DefaultClientIPHeader)) mux.Use(chimiddleware.RequestID) mux.Use(middleware.NoCache) mux.Use(middleware.Cors( diff --git a/services/thumbnails/pkg/server/http/server.go b/services/thumbnails/pkg/server/http/server.go index bf3996a25a..978945f0a7 100644 --- a/services/thumbnails/pkg/server/http/server.go +++ b/services/thumbnails/pkg/server/http/server.go @@ -38,7 +38,7 @@ func Server(opts ...Option) (http.Service, error) { svc.Logger(options.Logger), svc.Config(options.Config), svc.Middleware( - middleware.ClientIPFromRemoteAddr, + middleware.ClientIPFromHeader(opencloudmiddleware.DefaultClientIPHeader), middleware.RequestID, opencloudmiddleware.Cors( cors.Logger(options.Logger), diff --git a/services/web/pkg/server/http/server.go b/services/web/pkg/server/http/server.go index cd43f95073..d8a0c9d3be 100644 --- a/services/web/pkg/server/http/server.go +++ b/services/web/pkg/server/http/server.go @@ -118,7 +118,7 @@ func Server(opts ...Option) (http.Service, error) { svc.Config(options.Config), svc.GatewaySelector(gatewaySelector), svc.Middleware( - chimiddleware.ClientIPFromRemoteAddr, + chimiddleware.ClientIPFromHeader(middleware.DefaultClientIPHeader), chimiddleware.RequestID, chimiddleware.Compress(5), middleware.NoCache, diff --git a/services/webdav/pkg/server/http/server.go b/services/webdav/pkg/server/http/server.go index 30d53be1be..eddebf90fa 100644 --- a/services/webdav/pkg/server/http/server.go +++ b/services/webdav/pkg/server/http/server.go @@ -38,7 +38,7 @@ func Server(opts ...Option) (http.Service, error) { svc.Logger(options.Logger), svc.Config(options.Config), svc.Middleware( - chimiddleware.ClientIPFromRemoteAddr, + chimiddleware.ClientIPFromHeader(middleware.DefaultClientIPHeader), chimiddleware.RequestID, middleware.NoCache, middleware.Cors( diff --git a/services/webfinger/pkg/server/http/server.go b/services/webfinger/pkg/server/http/server.go index bd9677d185..32d84ad945 100644 --- a/services/webfinger/pkg/server/http/server.go +++ b/services/webfinger/pkg/server/http/server.go @@ -45,7 +45,7 @@ func Server(opts ...Option) (ohttp.Service, error) { mux := chi.NewMux() - mux.Use(chimiddleware.ClientIPFromRemoteAddr) + mux.Use(chimiddleware.ClientIPFromHeader(middleware.DefaultClientIPHeader)) mux.Use(chimiddleware.RequestID) mux.Use(middleware.TraceContext) mux.Use(middleware.NoCache)