From b78a1d0388efaa54949638fc6673fc84a1ba5370 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Mon, 3 Dec 2018 10:23:09 -0800
Subject: [PATCH] Strip port and include remote IP in access logs
The port number is meaningless when the X-Forwarded-For header
is used, so let's just remove it.
Closes https://gitlab.com/gitlab-org/gitlab-workhorse/issues/201
---
internal/helper/logging.go | 3 +++
internal/helper/logging_test.go | 34 +++++++++++++++++++++++++++++++++
2 files changed, 37 insertions(+)
diff --git a/internal/helper/logging.go b/internal/helper/logging.go
index e548c8aa6a913..12fd76e48ca92 100644
--- a/internal/helper/logging.go
+++ b/internal/helper/logging.go
@@ -94,8 +94,11 @@ func (l *statsCollectingResponseWriter) writeAccessLog(r *http.Request) {
func (l *statsCollectingResponseWriter) accessLogFields(r *http.Request) log.Fields {
duration := time.Since(l.started)
+ ip, _, _ := net.SplitHostPort(r.RemoteAddr)
+
return log.Fields{
"host": r.Host,
+ "remoteIp": ip,
"remoteAddr": r.RemoteAddr,
"method": r.Method,
"uri": ScrubURLParams(r.RequestURI),
diff --git a/internal/helper/logging_test.go b/internal/helper/logging_test.go
index 1213f795b0b9d..6b7e057aaa09b 100644
--- a/internal/helper/logging_test.go
+++ b/internal/helper/logging_test.go
@@ -10,6 +10,40 @@ import (
"github.com/stretchr/testify/assert"
)
+func Test_statsCollectingResponseWriter_remoteIp_accessLogFields(t *testing.T) {
+ testCases := []struct {
+ remoteAddr string
+ expected string
+ }{
+ {remoteAddr: "", expected: ""},
+ {remoteAddr: "bogus", expected: ""},
+ {remoteAddr: "8.8.8.8:1234", expected: "8.8.8.8"},
+ {remoteAddr: "8.8.8.8", expected: ""},
+ {remoteAddr: "[2001:db8:85a3:8d3:1319:8a2e:370:7348]", expected: ""},
+ {remoteAddr: "[2001:db8:85a3:8d3:1319:8a2e:370:7348]:443", expected: "2001:db8:85a3:8d3:1319:8a2e:370:7348"},
+ }
+
+ for _, tc := range testCases {
+ req, err := http.NewRequest("GET", "/blah", nil)
+ req.RemoteAddr = tc.remoteAddr
+ assert.Nil(t, err)
+
+ l := &statsCollectingResponseWriter{
+ rw: nil,
+ status: 200,
+ wroteHeader: true,
+ written: 50,
+ started: time.Now(),
+ }
+
+ fields := l.accessLogFields(req)
+
+ ip := fields["remoteIp"].(string)
+
+ assert.Equal(t, tc.expected, ip)
+ }
+}
+
func Test_statsCollectingResponseWriter_accessLogFields(t *testing.T) {
passwords := []string{
"should_be_filtered", // Basic case
--
GitLab