Merge pull request #650 from jordancrawfordnz/issue-649

Only set healthcheck user agents when the ping-user-agent is set, and don't check blank user agents against healthcheck user agents

CHANGELOG.md

@@ -16,6 +16,7 @@
 - [#542](https://github.com/oauth2-proxy/oauth2-proxy/pull/542) Move SessionStore tests to independent package (@JoelSpeed)
 - [#577](https://github.com/oauth2-proxy/oauth2-proxy/pull/577) Move Cipher and Session Store initialisation out of Validation (@JoelSpeed)
 - [#635](https://github.com/oauth2-proxy/oauth2-proxy/pull/635) Support specifying alternative provider TLS trust source(s) (@k-wall)
+- [#649](https://github.com/oauth2-proxy/oauth2-proxy/pull/650) Resolve an issue where an empty healthcheck URL and ping-user-agent returns the healthcheck response (@jordancrawfordnz)
 
 # v6.0.0
 

pkg/middleware/healthcheck.go

@@ -17,14 +17,18 @@ func healthCheck(paths, userAgents []string, next http.Handler) http.Handler {
 	// Use a map as a set to check health check paths
 	pathSet := make(map[string]struct{})
 	for _, path := range paths {
+		if len(path) > 0 {
 			pathSet[path] = struct{}{}
 		}
+	}
 
 	// Use a map as a set to check health check paths
 	userAgentSet := make(map[string]struct{})
 	for _, userAgent := range userAgents {
+		if len(userAgent) > 0 {
 			userAgentSet[userAgent] = struct{}{}
 		}
+	}
 
 	return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
 		if isHealthCheckRequest(pathSet, userAgentSet, req) {

pkg/middleware/healthcheck_test.go

@@ -34,6 +34,14 @@ var _ = Describe("HealthCheck suite", func() {
 			Expect(rw.Code).To(Equal(in.expectedStatus))
 			Expect(rw.Body.String()).To(Equal(in.expectedBody))
 		},
+		Entry("when no health check paths are configured", &requestTableInput{
+			healthCheckPaths:      []string{},
+			healthCheckUserAgents: []string{"hc/1.0"},
+			requestString:         "http://example.com/ping",
+			headers:               map[string]string{},
+			expectedStatus:        404,
+			expectedBody:          "404 page not found\n",
+		}),
 		Entry("when requesting the healthcheck path", &requestTableInput{
 			healthCheckPaths:      []string{"/ping"},
 			healthCheckUserAgents: []string{"hc/1.0"},
@@ -50,6 +58,34 @@ var _ = Describe("HealthCheck suite", func() {
 			expectedStatus:        404,
 			expectedBody:          "404 page not found\n",
 		}),
+		Entry("when a blank string is configured as a health check path and the request has no specific path", &requestTableInput{
+			healthCheckPaths:      []string{""},
+			healthCheckUserAgents: []string{"hc/1.0"},
+			requestString:         "http://example.com",
+			headers:               map[string]string{},
+			expectedStatus:        404,
+			expectedBody:          "404 page not found\n",
+		}),
+		Entry("with no health check user agents configured", &requestTableInput{
+			healthCheckPaths:      []string{"/ping"},
+			healthCheckUserAgents: []string{},
+			requestString:         "http://example.com/abc",
+			headers: map[string]string{
+				"User-Agent": "user",
+			},
+			expectedStatus: 404,
+			expectedBody:   "404 page not found\n",
+		}),
+		Entry("with a request from a different user agent", &requestTableInput{
+			healthCheckPaths:      []string{"/ping"},
+			healthCheckUserAgents: []string{"hc/1.0"},
+			requestString:         "http://example.com/abc",
+			headers: map[string]string{
+				"User-Agent": "different",
+			},
+			expectedStatus: 404,
+			expectedBody:   "404 page not found\n",
+		}),
 		Entry("with a request from the health check user agent", &requestTableInput{
 			healthCheckPaths:      []string{"/ping"},
 			healthCheckUserAgents: []string{"hc/1.0"},
@@ -60,13 +96,11 @@ var _ = Describe("HealthCheck suite", func() {
 			expectedStatus: 200,
 			expectedBody:   "OK",
 		}),
-		Entry("with a request from a different user agent", &requestTableInput{
+		Entry("when a blank string is configured as a health check agent and a request has no user agent", &requestTableInput{
 			healthCheckPaths:      []string{"/ping"},
-			healthCheckUserAgents: []string{"hc/1.0"},
+			healthCheckUserAgents: []string{""},
 			requestString:         "http://example.com/abc",
-			headers: map[string]string{
+			headers:               map[string]string{},
-				"User-Agent": "different",
-			},
 			expectedStatus:        404,
 			expectedBody:          "404 page not found\n",
 		}),