review: suppress redundant use of BasicAuth

traefik · Jun 18, 2020 · 832d968 · 832d968
1 parent 1fd65f4
commit 832d968
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 23 deletions.
diff --git a/integration/access_log_test.go b/integration/access_log_test.go
@@ -496,6 +496,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",
+			routerName: "rt-authFrontend",
+			serviceURL: "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/pkg/middlewares/auth/basic_auth.go b/pkg/middlewares/auth/basic_auth.go
@@ -62,37 +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")
 
-		if username, _, ok := req.BasicAuth(); ok {
-			logData := accesslog.GetLogData(req)
-			if logData != nil {
-				logData.Core[accesslog.ClientUsername] = username
-			}
-		}
-
 		b.auth.RequireAuth(rw, req)
-	} else {
-		logger.Debug("Authentication succeeded")
-		req.URL.User = url.User(username)
+		return
+	}
 
-		logData := accesslog.GetLogData(req)
-		if logData != nil {
-			logData.Core[accesslog.ClientUsername] = username
-		}
+	logger.Debug("Authentication succeeded")
+	req.URL.User = url.User(user)
 
-		if b.headerField != "" {
-			req.Header[b.headerField] = []string{username}
-		}
+	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)
+	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 {