1
0
mirror of https://github.com/oauth2-proxy/oauth2-proxy.git synced 2024-11-28 09:08:44 +02:00

(#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.
This commit is contained in:
Jordan Crawford 2020-07-06 13:39:06 +12:00
parent 99481b3a39
commit 6346dafc1e
3 changed files with 46 additions and 7 deletions

View File

@ -16,6 +16,7 @@
- [#542](https://github.com/oauth2-proxy/oauth2-proxy/pull/542) Move SessionStore tests to independent package (@JoelSpeed) - [#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) - [#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) - [#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 # v6.0.0

View File

@ -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 // Use a map as a set to check health check paths
pathSet := make(map[string]struct{}) pathSet := make(map[string]struct{})
for _, path := range paths { for _, path := range paths {
if len(path) > 0 {
pathSet[path] = struct{}{} pathSet[path] = struct{}{}
} }
}
// Use a map as a set to check health check paths // Use a map as a set to check health check paths
userAgentSet := make(map[string]struct{}) userAgentSet := make(map[string]struct{})
for _, userAgent := range userAgents { for _, userAgent := range userAgents {
if len(userAgent) > 0 {
userAgentSet[userAgent] = struct{}{} userAgentSet[userAgent] = struct{}{}
} }
}
return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
if isHealthCheckRequest(pathSet, userAgentSet, req) { if isHealthCheckRequest(pathSet, userAgentSet, req) {

View File

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