From 55334b20628b492190705a32f2ee24f8724ef918 Mon Sep 17 00:00:00 2001 From: Brendan LE GLAUNEC Date: Thu, 25 Oct 2018 18:00:05 +0200 Subject: [PATCH] Fix display of client username field --- integration/access_log_test.go | 52 ++++++++++++++++++++-- middlewares/accesslog/logger.go | 14 +++--- middlewares/accesslog/logger_test.go | 2 +- middlewares/accesslog/save_username.go | 60 ++++++++++++++++++++++++++ middlewares/auth/authenticator.go | 12 ++++-- server/server_loadbalancer.go | 3 +- server/server_middlewares.go | 6 ++- 7 files changed, 130 insertions(+), 19 deletions(-) create mode 100644 middlewares/accesslog/save_username.go diff --git a/integration/access_log_test.go b/integration/access_log_test.go index 27da9adb8..46f4de648 100644 --- a/integration/access_log_test.go +++ b/integration/access_log_test.go @@ -49,7 +49,6 @@ func (s *AccessLogSuite) TearDownTest(c *check.C) { } func (s *AccessLogSuite) TestAccessLog(c *check.C) { - // Ensure working directory is clean ensureWorkingDirectoryIsClean() // Start Traefik @@ -94,7 +93,6 @@ func (s *AccessLogSuite) TestAccessLog(c *check.C) { } func (s *AccessLogSuite) TestAccessLogAuthFrontend(c *check.C) { - // Ensure working directory is clean ensureWorkingDirectoryIsClean() expected := []accessLogValue{ @@ -142,7 +140,6 @@ func (s *AccessLogSuite) TestAccessLogAuthFrontend(c *check.C) { } func (s *AccessLogSuite) TestAccessLogAuthEntrypoint(c *check.C) { - // Ensure working directory is clean ensureWorkingDirectoryIsClean() expected := []accessLogValue{ @@ -190,7 +187,6 @@ func (s *AccessLogSuite) TestAccessLogAuthEntrypoint(c *check.C) { } func (s *AccessLogSuite) TestAccessLogAuthEntrypointSuccess(c *check.C) { - // Ensure working directory is clean ensureWorkingDirectoryIsClean() expected := []accessLogValue{ @@ -642,6 +638,54 @@ func (s *AccessLogSuite) TestAccessLogFrontendWhitelist(c *check.C) { checkNoOtherTraefikProblems(c) } +func (s *AccessLogSuite) TestAccessLogAuthFrontendSuccess(c *check.C) { + ensureWorkingDirectoryIsClean() + + expected := []accessLogValue{ + { + formatOnly: false, + code: "200", + user: "test", + frontendName: "Host-frontend-auth-docker", + backendURL: "http://172.17.0", + }, + } + + // Start Traefik + cmd, display := s.traefikCmd(withConfigFile("fixtures/access_log_config.toml")) + defer display(c) + + err := cmd.Start() + c.Assert(err, checker.IsNil) + defer cmd.Process.Kill() + + checkStatsForLogFile(c) + + s.composeProject.Container(c, "authFrontend") + + waitForTraefik(c, "authFrontend") + + // Verify Traefik started OK + checkTraefikStarted(c) + + // Test auth entrypoint + req, err := http.NewRequest(http.MethodGet, "http://127.0.0.1:8006/", nil) + c.Assert(err, checker.IsNil) + req.Host = "frontend.auth.docker.local" + req.SetBasicAuth("test", "test") + + err = try.Request(req, 500*time.Millisecond, try.StatusCodeIs(http.StatusOK), try.HasBody()) + c.Assert(err, checker.IsNil) + + // Verify access.log output as expected + count := checkAccessLogExactValuesOutput(c, expected) + + c.Assert(count, checker.GreaterOrEqualThan, len(expected)) + + // Verify no other Traefik problems + checkNoOtherTraefikProblems(c) +} + func checkNoOtherTraefikProblems(c *check.C) { traefikLog, err := ioutil.ReadFile(traefikTestLogFile) c.Assert(err, checker.IsNil) diff --git a/middlewares/accesslog/logger.go b/middlewares/accesslog/logger.go index 166bb2bdf..c1a518cad 100644 --- a/middlewares/accesslog/logger.go +++ b/middlewares/accesslog/logger.go @@ -180,7 +180,7 @@ func (l *LogHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request, next h next.ServeHTTP(crw, reqWithDataTable) - core[ClientUsername] = usernameIfPresent(reqWithDataTable.URL) + core[ClientUsername] = formatUsernameForLog(core[ClientUsername]) logDataTable.DownstreamResponse = crw.Header() @@ -231,14 +231,12 @@ func silentSplitHostPort(value string) (host string, port string) { return host, port } -func usernameIfPresent(theURL *url.URL) string { - username := "-" - if theURL.User != nil { - if name := theURL.User.Username(); name != "" { - username = name - } +func formatUsernameForLog(usernameField interface{}) string { + username, ok := usernameField.(string) + if ok && len(username) != 0 { + return username } - return username + return "-" } // Logging handler to log frontend name, backend name, and elapsed time diff --git a/middlewares/accesslog/logger_test.go b/middlewares/accesslog/logger_test.go index cbf1da5fc..e7c8af991 100644 --- a/middlewares/accesslog/logger_test.go +++ b/middlewares/accesslog/logger_test.go @@ -619,7 +619,6 @@ func doLogging(t *testing.T, config *types.AccessLog) { Method: testMethod, RemoteAddr: fmt.Sprintf("%s:%d", testHostname, testPort), URL: &url.URL{ - User: url.UserPassword(testUsername, ""), Path: testPath, }, } @@ -639,4 +638,5 @@ func logWriterTestHandlerFunc(rw http.ResponseWriter, r *http.Request) { logDataTable.Core[RetryAttempts] = testRetryAttempts logDataTable.Core[StartUTC] = testStart.UTC() logDataTable.Core[StartLocal] = testStart.Local() + logDataTable.Core[ClientUsername] = testUsername } diff --git a/middlewares/accesslog/save_username.go b/middlewares/accesslog/save_username.go new file mode 100644 index 000000000..6debf7795 --- /dev/null +++ b/middlewares/accesslog/save_username.go @@ -0,0 +1,60 @@ +package accesslog + +import ( + "context" + "net/http" + + "github.com/urfave/negroni" +) + +const ( + clientUsernameKey key = "ClientUsername" +) + +// SaveUsername sends the Username name to the access logger. +type SaveUsername struct { + next http.Handler +} + +// NewSaveUsername creates a SaveUsername handler. +func NewSaveUsername(next http.Handler) http.Handler { + return &SaveUsername{next} +} + +func (sf *SaveUsername) ServeHTTP(rw http.ResponseWriter, r *http.Request) { + serveSaveUsername(r, func() { + sf.next.ServeHTTP(rw, r) + }) +} + +// SaveNegroniUsername adds the Username to the access logger data table. +type SaveNegroniUsername struct { + next negroni.Handler +} + +// NewSaveNegroniUsername creates a SaveNegroniUsername handler. +func NewSaveNegroniUsername(next negroni.Handler) negroni.Handler { + return &SaveNegroniUsername{next} +} + +func (sf *SaveNegroniUsername) ServeHTTP(rw http.ResponseWriter, r *http.Request, next http.HandlerFunc) { + serveSaveUsername(r, func() { + sf.next.ServeHTTP(rw, r, next) + }) +} + +func serveSaveUsername(r *http.Request, apply func()) { + table := GetLogDataTable(r) + + username, ok := r.Context().Value(clientUsernameKey).(string) + if ok { + table.Core[ClientUsername] = username + } + + apply() +} + +// WithUserName adds a username to a requests' context +func WithUserName(req *http.Request, username string) *http.Request { + return req.WithContext(context.WithValue(req.Context(), clientUsernameKey, username)) +} diff --git a/middlewares/auth/authenticator.go b/middlewares/auth/authenticator.go index ea223049e..8e2491605 100644 --- a/middlewares/auth/authenticator.go +++ b/middlewares/auth/authenticator.go @@ -4,11 +4,11 @@ import ( "fmt" "io/ioutil" "net/http" - "net/url" "strings" goauth "github.com/abbot/go-http-auth" "github.com/containous/traefik/log" + "github.com/containous/traefik/middlewares/accesslog" "github.com/containous/traefik/middlewares/tracing" "github.com/containous/traefik/types" "github.com/urfave/negroni" @@ -86,7 +86,10 @@ func createAuthDigestHandler(digestAuth *goauth.DigestAuth, authConfig *types.Au digestAuth.RequireAuth(w, r) } else { log.Debugf("Digest auth succeeded") - r.URL.User = url.User(username) + + // set username in request context + r = accesslog.WithUserName(r, username) + if authConfig.HeaderField != "" { r.Header[authConfig.HeaderField] = []string{username} } @@ -105,7 +108,10 @@ func createAuthBasicHandler(basicAuth *goauth.BasicAuth, authConfig *types.Auth) basicAuth.RequireAuth(w, r) } else { log.Debugf("Basic auth succeeded") - r.URL.User = url.User(username) + + // set username in request context + r = accesslog.WithUserName(r, username) + if authConfig.HeaderField != "" { r.Header[authConfig.HeaderField] = []string{username} } diff --git a/server/server_loadbalancer.go b/server/server_loadbalancer.go index 7d7efc6de..f8e8e4726 100644 --- a/server/server_loadbalancer.go +++ b/server/server_loadbalancer.go @@ -115,7 +115,8 @@ func (s *Server) buildLoadBalancer(frontendName string, backendName string, back var saveFrontend http.Handler if s.accessLoggerMiddleware != nil { - saveBackend := accesslog.NewSaveBackend(fwd, backendName) + saveUsername := accesslog.NewSaveUsername(fwd) + saveBackend := accesslog.NewSaveBackend(saveUsername, backendName) saveFrontend = accesslog.NewSaveFrontend(saveBackend, frontendName) rr, _ = roundrobin.New(saveFrontend) } else { diff --git a/server/server_middlewares.go b/server/server_middlewares.go index ae62f993d..4edf25c3d 100644 --- a/server/server_middlewares.go +++ b/server/server_middlewares.go @@ -309,7 +309,8 @@ func buildIPWhiteLister(whiteList *types.WhiteList, wlRange []string) (*middlewa func (s *Server) wrapNegroniHandlerWithAccessLog(handler negroni.Handler, frontendName string) negroni.Handler { if s.accessLoggerMiddleware != nil { - saveBackend := accesslog.NewSaveNegroniBackend(handler, "Traefik") + saveUsername := accesslog.NewSaveNegroniUsername(handler) + saveBackend := accesslog.NewSaveNegroniBackend(saveUsername, "Traefik") saveFrontend := accesslog.NewSaveNegroniFrontend(saveBackend, frontendName) return saveFrontend } @@ -318,7 +319,8 @@ func (s *Server) wrapNegroniHandlerWithAccessLog(handler negroni.Handler, fronte func (s *Server) wrapHTTPHandlerWithAccessLog(handler http.Handler, frontendName string) http.Handler { if s.accessLoggerMiddleware != nil { - saveBackend := accesslog.NewSaveBackend(handler, "Traefik") + saveUsername := accesslog.NewSaveUsername(handler) + saveBackend := accesslog.NewSaveBackend(saveUsername, "Traefik") saveFrontend := accesslog.NewSaveFrontend(saveBackend, frontendName) return saveFrontend }