git.iko.soy/typeable/oauth2-proxy
on master help sign in
(#649) Remove blank helthcheck user agents and paths when setting up the healthcheck middleware

A blank user agent is considered == to an empty string. When no -ping-user-agent option is specified, this is considered to be an empty string. This reveals two problems: - When no ping-user-agent is specified, main.go sets up a health check user agent of "" - When no user agent is specified, the empty string is still checked against the health check user agents. Now the health check middleware ignores blank user agents and paths in order to sanitise it's input to avoid this issue. Additional tests have been added to verify these situations.

commit6346dafc1e9c091cc79f2b6a8432f61c14eb767a authorJordan Crawford <jordan@crawford.kiwi> dateMon 6 Jul 2020, 13:39 (6 years ago) parent99481b3
3 files changed+46−7
CHANGELOG.md+1 pkg/middleware/healthcheck.go+6−2 pkg/middleware/healthcheck_test.go+39−5
CHANGELOG.md+1
@@ -16,6 +16,7 @@
1616- [#542](https://github.com/oauth2-proxy/oauth2-proxy/pull/542) Move SessionStore tests to independent package (@JoelSpeed)
1717- [#577](https://github.com/oauth2-proxy/oauth2-proxy/pull/577) Move Cipher and Session Store initialisation out of Validation (@JoelSpeed)
1818- [#635](https://github.com/oauth2-proxy/oauth2-proxy/pull/635) Support specifying alternative provider TLS trust source(s) (@k-wall)
19+- [#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)
1920
2021# v6.0.0
2122
pkg/middleware/healthcheck.go+6−2
@@ -17,13 +17,17 @@ func healthCheck(paths, userAgents []string, next http.Handler) http.Handler {
1717 // Use a map as a set to check health check paths
1818 pathSet := make(map[string]struct{})
1919 for _, path := range paths {
20− pathSet[path] = struct{}{}
20+ if len(path) > 0 {
21+ pathSet[path] = struct{}{}
22+ }
2123 }
2224
2325 // Use a map as a set to check health check paths
2426 userAgentSet := make(map[string]struct{})
2527 for _, userAgent := range userAgents {
26− userAgentSet[userAgent] = struct{}{}
28+ if len(userAgent) > 0 {
29+ userAgentSet[userAgent] = struct{}{}
30+ }
2731 }
2832
2933 return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
pkg/middleware/healthcheck_test.go+39−5
@@ -34,6 +34,14 @@ var _ = Describe("HealthCheck suite", func() {
3434 Expect(rw.Code).To(Equal(in.expectedStatus))
3535 Expect(rw.Body.String()).To(Equal(in.expectedBody))
3636 },
37+ Entry("when no health check paths are configured", &requestTableInput{
38+ healthCheckPaths: []string{},
39+ healthCheckUserAgents: []string{"hc/1.0"},
40+ requestString: "http://example.com/ping",
41+ headers: map[string]string{},
42+ expectedStatus: 404,
43+ expectedBody: "404 page not found\n",
44+ }),
3745 Entry("when requesting the healthcheck path", &requestTableInput{
3846 healthCheckPaths: []string{"/ping"},
3947 healthCheckUserAgents: []string{"hc/1.0"},
@@ -50,15 +58,23 @@ var _ = Describe("HealthCheck suite", func() {
5058 expectedStatus: 404,
5159 expectedBody: "404 page not found\n",
5260 }),
53− Entry("with a request from the health check user agent", &requestTableInput{
54− healthCheckPaths: []string{"/ping"},
61+ Entry("when a blank string is configured as a health check path and the request has no specific path", &requestTableInput{
62+ healthCheckPaths: []string{""},
5563 healthCheckUserAgents: []string{"hc/1.0"},
64+ requestString: "http://example.com",
65+ headers: map[string]string{},
66+ expectedStatus: 404,
67+ expectedBody: "404 page not found\n",
68+ }),
69+ Entry("with no health check user agents configured", &requestTableInput{
70+ healthCheckPaths: []string{"/ping"},
71+ healthCheckUserAgents: []string{},
5672 requestString: "http://example.com/abc",
5773 headers: map[string]string{
58− "User-Agent": "hc/1.0",
74+ "User-Agent": "user",
5975 },
60− expectedStatus: 200,
61− expectedBody: "OK",
76+ expectedStatus: 404,
77+ expectedBody: "404 page not found\n",
6278 }),
6379 Entry("with a request from a different user agent", &requestTableInput{
6480 healthCheckPaths: []string{"/ping"},
@@ -70,6 +86,24 @@ var _ = Describe("HealthCheck suite", func() {
7086 expectedStatus: 404,
7187 expectedBody: "404 page not found\n",
7288 }),
89+ Entry("with a request from the health check user agent", &requestTableInput{
90+ healthCheckPaths: []string{"/ping"},
91+ healthCheckUserAgents: []string{"hc/1.0"},
92+ requestString: "http://example.com/abc",
93+ headers: map[string]string{
94+ "User-Agent": "hc/1.0",
95+ },
96+ expectedStatus: 200,
97+ expectedBody: "OK",
98+ }),
99+ Entry("when a blank string is configured as a health check agent and a request has no user agent", &requestTableInput{
100+ healthCheckPaths: []string{"/ping"},
101+ healthCheckUserAgents: []string{""},
102+ requestString: "http://example.com/abc",
103+ headers: map[string]string{},
104+ expectedStatus: 404,
105+ expectedBody: "404 page not found\n",
106+ }),
73107 Entry("with multiple paths, request one of the healthcheck paths", &requestTableInput{
74108 healthCheckPaths: []string{"/ping", "/liveness_check", "/readiness_check"},
75109 healthCheckUserAgents: []string{"hc/1.0"},