From 107237c8e84c541d7f24095dcce7abaf5d973a7e Mon Sep 17 00:00:00 2001 From: Daenney Date: Sun, 21 May 2023 17:12:47 +0200 Subject: [PATCH] [feature] Make client IP logging configurable (#1799) --- cmd/gotosocial/action/server/server.go | 2 +- cmd/gotosocial/action/testrig/testrig.go | 2 +- example/config.yaml | 9 +++++---- internal/config/config.go | 1 + internal/config/defaults.go | 2 ++ internal/config/helpers.gen.go | 25 ++++++++++++++++++++++++ internal/gtscontext/log_hooks.go | 2 +- internal/middleware/logger.go | 7 ++----- test/envparsing.sh | 2 ++ 9 files changed, 40 insertions(+), 12 deletions(-) diff --git a/cmd/gotosocial/action/server/server.go b/cmd/gotosocial/action/server/server.go index e4158ea88..76e58c2f8 100644 --- a/cmd/gotosocial/action/server/server.go +++ b/cmd/gotosocial/action/server/server.go @@ -162,7 +162,7 @@ middlewares = append(middlewares, []gin.HandlerFunc{ // note: hooks adding ctx fields must be ABOVE // the logger, otherwise won't be accessible. - middleware.Logger(), + middleware.Logger(config.GetLogClientIP()), middleware.UserAgent(), middleware.CORS(), middleware.ExtraHeaders(), diff --git a/cmd/gotosocial/action/testrig/testrig.go b/cmd/gotosocial/action/testrig/testrig.go index 13e9a8f5b..5d4f20773 100644 --- a/cmd/gotosocial/action/testrig/testrig.go +++ b/cmd/gotosocial/action/testrig/testrig.go @@ -107,7 +107,7 @@ middlewares = append(middlewares, tracing.InstrumentGin()) } middlewares = append(middlewares, []gin.HandlerFunc{ - middleware.Logger(), + middleware.Logger(config.GetLogClientIP()), middleware.UserAgent(), middleware.CORS(), middleware.ExtraHeaders(), diff --git a/example/config.yaml b/example/config.yaml index 57cf5fd63..668d1729c 100644 --- a/example/config.yaml +++ b/example/config.yaml @@ -30,6 +30,11 @@ log-level: "info" # Default: false log-db-queries: false +# Bool. Include the client IP in the emitted log lines +# Options: [true, false] +# Default: true +log-client-ip: true + # String. Application name to use internally. # Examples: ["My Application","gotosocial"] # Default: "gotosocial" @@ -766,10 +771,6 @@ syslog-address: "localhost:514" ##### OBSERVABILITY SETTINGS ##### ################################## -# Bool. Enable generation/parsing of a request ID for each received HTTP Request. -# Default: true -request-id-enabled: true - # String. Header name to use to extract a request or trace ID from. Typically set by a # loadbalancer or proxy. # Default: "X-Request-Id" diff --git a/internal/config/config.go b/internal/config/config.go index b5c228a35..7119fc4a7 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -46,6 +46,7 @@ func fieldtag(field, tag string) string { type Configuration struct { LogLevel string `name:"log-level" usage:"Log level to run at: [trace, debug, info, warn, fatal]"` LogDbQueries bool `name:"log-db-queries" usage:"Log database queries verbosely when log-level is trace or debug"` + LogClientIP bool `name:"log-client-ip" usage:"Include the client IP in logs"` ApplicationName string `name:"application-name" usage:"Name of the application, used in various places internally"` LandingPageUser string `name:"landing-page-user" usage:"the user that should be shown on the instance's landing page"` ConfigPath string `name:"config-path" usage:"Path to a file containing gotosocial configuration. Values set in this file will be overwritten by values set as env vars or arguments"` diff --git a/internal/config/defaults.go b/internal/config/defaults.go index 54672c159..6e2d141be 100644 --- a/internal/config/defaults.go +++ b/internal/config/defaults.go @@ -198,4 +198,6 @@ AdminMediaPruneDryRun: true, RequestIDHeader: "X-Request-Id", + + LogClientIP: true, } diff --git a/internal/config/helpers.gen.go b/internal/config/helpers.gen.go index b635f7b8e..9993d7aaf 100644 --- a/internal/config/helpers.gen.go +++ b/internal/config/helpers.gen.go @@ -3679,3 +3679,28 @@ func GetRequestIDHeader() string { return global.GetRequestIDHeader() } // SetRequestIDHeader safely sets the value for global configuration 'RequestIDHeader' field func SetRequestIDHeader(v string) { global.SetRequestIDHeader(v) } + +// GetLogClientIP safely fetches the Configuration value for state's 'LogClientIP' field +func (st *ConfigState) GetLogClientIP() (v bool) { + st.mutex.Lock() + v = st.config.LogClientIP + st.mutex.Unlock() + return +} + +// SetLogClientIP safely sets the Configuration value for state's 'LogClientIP' field +func (st *ConfigState) SetLogClientIP(v bool) { + st.mutex.Lock() + defer st.mutex.Unlock() + st.config.LogClientIP = v + st.reloadToViper() +} + +// LogClientIPFlag returns the flag name for the 'LogClientIP' field +func LogClientIPFlag() string { return "log-client-ip" } + +// GetLogClientIP safely fetches the value for global configuration 'LogClientIP' field +func GetLogClientIP() bool { return global.GetLogClientIP() } + +// SetLogClientIP safely sets the value for global configuration 'LogClientIP' field +func SetLogClientIP(v bool) { global.SetLogClientIP(v) } diff --git a/internal/gtscontext/log_hooks.go b/internal/gtscontext/log_hooks.go index 2fe43e488..94300cbb7 100644 --- a/internal/gtscontext/log_hooks.go +++ b/internal/gtscontext/log_hooks.go @@ -34,7 +34,7 @@ func init() { } return kvs }) - // Client IP middleware hook. + // Public Key ID middleware hook. log.Hook(func(ctx context.Context, kvs []kv.Field) []kv.Field { if id := PublicKeyID(ctx); id != "" { return append(kvs, kv.Field{K: "pubKeyID", V: id}) diff --git a/internal/middleware/logger.go b/internal/middleware/logger.go index 50e5542c3..8acb742fb 100644 --- a/internal/middleware/logger.go +++ b/internal/middleware/logger.go @@ -31,7 +31,7 @@ ) // Logger returns a gin middleware which provides request logging and panic recovery. -func Logger() gin.HandlerFunc { +func Logger(logClientIP bool) gin.HandlerFunc { return func(c *gin.Context) { // Initialize the logging fields fields := make(kv.Fields, 5, 7) @@ -72,10 +72,7 @@ func Logger() gin.HandlerFunc { fields[2] = kv.Field{"method", c.Request.Method} fields[3] = kv.Field{"statusCode", code} fields[4] = kv.Field{"path", path} - if includeClientIP := true; includeClientIP { - // TODO: make this configurable. - // - // Include clientIP if enabled. + if logClientIP { fields = append(fields, kv.Field{ "clientIP", c.ClientIP(), }) diff --git a/test/envparsing.sh b/test/envparsing.sh index 3580e8517..688ff8d2c 100755 --- a/test/envparsing.sh +++ b/test/envparsing.sh @@ -98,6 +98,7 @@ EXPECT=$(cat <<"EOF" "letsencrypt-email-address": "", "letsencrypt-enabled": true, "letsencrypt-port": 80, + "log-client-ip": false, "log-db-queries": true, "log-level": "info", "media-description-max-chars": 5000, @@ -170,6 +171,7 @@ EOF # ensure that these are parsed without panic OUTPUT=$(GTS_LOG_LEVEL='info' \ GTS_LOG_DB_QUERIES=true \ +GTS_LOG_CLIENT_IP=false \ GTS_APPLICATION_NAME=gts \ GTS_LANDING_PAGE_USER=admin \ GTS_HOST=example.com \