From 8d3d5c068ce707ec3b51090e39f2b73ac72d5ff7 Mon Sep 17 00:00:00 2001 From: Romain Date: Thu, 18 Jun 2020 16:02:04 +0200 Subject: [PATCH] Provide username in log data on auth failure --- integration/access_log_test.go | 52 +++++++++++++++++++++++++---- pkg/middlewares/auth/basic_auth.go | 48 +++++++++++++++----------- pkg/middlewares/auth/digest_auth.go | 13 ++++++++ 3 files changed, 87 insertions(+), 26 deletions(-) diff --git a/integration/access_log_test.go b/integration/access_log_test.go index 31e52e306..17d1420ae 100644 --- a/integration/access_log_test.go +++ b/integration/access_log_test.go @@ -111,6 +111,20 @@ func (s *AccessLogSuite) TestAccessLogAuthFrontend(c *check.C) { routerName: "rt-authFrontend", serviceURL: "-", }, + { + formatOnly: false, + code: "401", + user: "test", + routerName: "rt-authFrontend", + serviceURL: "-", + }, + { + formatOnly: false, + code: "200", + user: "test", + routerName: "rt-authFrontend", + serviceURL: "http://172.17.0", + }, } // Start Traefik @@ -130,7 +144,7 @@ func (s *AccessLogSuite) TestAccessLogAuthFrontend(c *check.C) { // Verify Traefik started OK checkTraefikStarted(c) - // Test auth frontend + // 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" @@ -138,6 +152,16 @@ func (s *AccessLogSuite) TestAccessLogAuthFrontend(c *check.C) { err = try.Request(req, 500*time.Millisecond, try.StatusCodeIs(http.StatusUnauthorized), try.HasBody()) c.Assert(err, checker.IsNil) + req.SetBasicAuth("test", "") + + err = try.Request(req, 500*time.Millisecond, try.StatusCodeIs(http.StatusUnauthorized), try.HasBody()) + c.Assert(err, checker.IsNil) + + 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) @@ -158,6 +182,13 @@ func (s *AccessLogSuite) TestAccessLogDigestAuthMiddleware(c *check.C) { routerName: "rt-digestAuthMiddleware", serviceURL: "-", }, + { + formatOnly: false, + code: "401", + user: "test", + routerName: "rt-digestAuthMiddleware", + serviceURL: "-", + }, { formatOnly: false, code: "200", @@ -192,15 +223,22 @@ func (s *AccessLogSuite) TestAccessLogDigestAuthMiddleware(c *check.C) { resp, err := try.ResponseUntilStatusCode(req, 500*time.Millisecond, http.StatusUnauthorized) c.Assert(err, checker.IsNil) - digestParts := digestParts(resp) - digestParts["uri"] = "/" - digestParts["method"] = http.MethodGet - digestParts["username"] = "test" - digestParts["password"] = "test" + digest := digestParts(resp) + digest["uri"] = "/" + digest["method"] = http.MethodGet + digest["username"] = "test" + digest["password"] = "wrong" - req.Header.Set("Authorization", getDigestAuthorization(digestParts)) + req.Header.Set("Authorization", getDigestAuthorization(digest)) req.Header.Set("Content-Type", "application/json") + err = try.Request(req, 500*time.Millisecond, try.StatusCodeIs(http.StatusUnauthorized), try.HasBody()) + c.Assert(err, checker.IsNil) + + digest["password"] = "test" + + req.Header.Set("Authorization", getDigestAuthorization(digest)) + err = try.Request(req, 500*time.Millisecond, try.StatusCodeIs(http.StatusOK), try.HasBody()) c.Assert(err, checker.IsNil) diff --git a/pkg/middlewares/auth/basic_auth.go b/pkg/middlewares/auth/basic_auth.go index e03fb3f6a..196f3bd30 100644 --- a/pkg/middlewares/auth/basic_auth.go +++ b/pkg/middlewares/auth/basic_auth.go @@ -62,29 +62,39 @@ func (b *basicAuth) GetTracingInformation() (string, ext.SpanKindEnum) { func (b *basicAuth) ServeHTTP(rw http.ResponseWriter, req *http.Request) { logger := log.FromContext(middlewares.GetLoggerCtx(req.Context(), b.name, basicTypeName)) - if username := b.auth.CheckAuth(req); username == "" { + user, password, ok := req.BasicAuth() + if ok { + secret := b.auth.Secrets(user, b.auth.Realm) + if secret == "" || !goauth.CheckSecret(password, secret) { + ok = false + } + } + + logData := accesslog.GetLogData(req) + if logData != nil { + logData.Core[accesslog.ClientUsername] = user + } + + if !ok { logger.Debug("Authentication failed") tracing.SetErrorWithEvent(req, "Authentication failed") + b.auth.RequireAuth(rw, req) - } else { - logger.Debug("Authentication succeeded") - req.URL.User = url.User(username) - - logData := accesslog.GetLogData(req) - if logData != nil { - logData.Core[accesslog.ClientUsername] = username - } - - if b.headerField != "" { - req.Header[b.headerField] = []string{username} - } - - if b.removeHeader { - logger.Debug("Removing authorization header") - req.Header.Del(authorizationHeader) - } - b.next.ServeHTTP(rw, req) + return } + + logger.Debug("Authentication succeeded") + req.URL.User = url.User(user) + + if b.headerField != "" { + req.Header[b.headerField] = []string{user} + } + + if b.removeHeader { + logger.Debug("Removing authorization header") + req.Header.Del(authorizationHeader) + } + b.next.ServeHTTP(rw, req) } func (b *basicAuth) secretBasic(user, realm string) string { diff --git a/pkg/middlewares/auth/digest_auth.go b/pkg/middlewares/auth/digest_auth.go index 70d1306ef..f16189469 100644 --- a/pkg/middlewares/auth/digest_auth.go +++ b/pkg/middlewares/auth/digest_auth.go @@ -63,6 +63,19 @@ func (d *digestAuth) ServeHTTP(rw http.ResponseWriter, req *http.Request) { username, authinfo := d.auth.CheckAuth(req) if username == "" { + headerField := d.headerField + if d.headerField == "" { + headerField = "Authorization" + } + + auth := goauth.DigestAuthParams(req.Header.Get(headerField)) + if auth["username"] != "" { + logData := accesslog.GetLogData(req) + if logData != nil { + logData.Core[accesslog.ClientUsername] = auth["username"] + } + } + if authinfo != nil && *authinfo == "stale" { logger.Debug("Digest authentication failed, possibly because out of order requests") tracing.SetErrorWithEvent(req, "Digest authentication failed, possibly because out of order requests")