From 584e648fdccb4b076c2d3ccf4fc572773d84cbb1 Mon Sep 17 00:00:00 2001 From: Adam DeHovitz Date: Wed, 12 May 2021 13:54:03 -0400 Subject: [PATCH 1/3] Improvement: report unauthorized error as a health check --- status/status.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/status/status.go b/status/status.go index 32e859a8..8d474ae4 100644 --- a/status/status.go +++ b/status/status.go @@ -15,6 +15,7 @@ package status import ( + "github.com/palantir/witchcraft-go-health/sources" "net/http" "sync/atomic" @@ -24,6 +25,13 @@ import ( "github.com/palantir/witchcraft-go-health/status" ) + +var healthStatusUnauthorized = health.HealthStatus{ + Checks: map[health.CheckType]health.HealthCheckResult{ + "HEALTH_STATUS_UNAUTHORIZED": sources.UnhealthyHealthCheckResult("HEALTH_STATUS_UNAUTHORIZED", "unauthorized to access health status; please verify the health-check-shared-secret", nil), + }, +} + // HealthHandler is responsible for checking the health-check-shared-secret if it is provided and // invoking a HealthCheckSource if the secret is correct or unset. type healthHandlerImpl struct { @@ -66,7 +74,7 @@ func (h *healthHandlerImpl) computeNewHealthStatus(req *http.Request) (health.He if sharedSecret := h.healthCheckSharedSecret.CurrentString(); sharedSecret != "" { token, err := httpserver.ParseBearerTokenHeader(req) if err != nil || sharedSecret != token { - return health.HealthStatus{}, http.StatusUnauthorized + return healthStatusUnauthorized, http.StatusUnauthorized } } metadata := h.check.HealthStatus(req.Context()) From c126397716e3345300020218297b4ae46ffef3ee Mon Sep 17 00:00:00 2001 From: Adam DeHovitz Date: Wed, 12 May 2021 17:54:03 +0000 Subject: [PATCH 2/3] Add generated changelog entries --- changelog/@unreleased/pr-295.v2.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/@unreleased/pr-295.v2.yml diff --git a/changelog/@unreleased/pr-295.v2.yml b/changelog/@unreleased/pr-295.v2.yml new file mode 100644 index 00000000..b99df172 --- /dev/null +++ b/changelog/@unreleased/pr-295.v2.yml @@ -0,0 +1,5 @@ +type: improvement +improvement: + description: If user is unauthorized to view health checks return an unauthorized error health check rather than no health check. + links: + - https://github.com/palantir/witchcraft-go-server/pull/295 From 5a930f8c28cfef0709cfa3ffed41ec4a068ef0b9 Mon Sep 17 00:00:00 2001 From: Adam DeHovitz Date: Wed, 12 May 2021 14:13:14 -0400 Subject: [PATCH 3/3] fix verify --- status/routes/routes_test.go | 5 ++++- status/status.go | 3 +-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/status/routes/routes_test.go b/status/routes/routes_test.go index db17d97e..6f99eeb8 100644 --- a/status/routes/routes_test.go +++ b/status/routes/routes_test.go @@ -26,6 +26,7 @@ import ( "github.com/palantir/pkg/refreshable" "github.com/palantir/witchcraft-go-health/conjure/witchcraft/api/health" + "github.com/palantir/witchcraft-go-health/sources" healthstatus "github.com/palantir/witchcraft-go-health/status" "github.com/palantir/witchcraft-go-server/v2/status" "github.com/palantir/witchcraft-go-server/v2/witchcraft/wresource" @@ -224,7 +225,9 @@ func TestAddHealthRoute(t *testing.T) { expectedChecks := test.metadata // 401 does not return any health check data if test.expectedStatus == 401 { - expectedChecks.Checks = map[health.CheckType]health.HealthCheckResult{} + expectedChecks.Checks = map[health.CheckType]health.HealthCheckResult{ + "HEALTH_STATUS_UNAUTHORIZED": sources.UnhealthyHealthCheckResult("HEALTH_STATUS_UNAUTHORIZED", "unauthorized to access health status; please verify the health-check-shared-secret", map[string]interface{}{}), + } } assert.Equal(t, expectedChecks, gotObj) }) diff --git a/status/status.go b/status/status.go index 8d474ae4..d8b446af 100644 --- a/status/status.go +++ b/status/status.go @@ -15,17 +15,16 @@ package status import ( - "github.com/palantir/witchcraft-go-health/sources" "net/http" "sync/atomic" "github.com/palantir/conjure-go-runtime/v2/conjure-go-server/httpserver" "github.com/palantir/pkg/refreshable" "github.com/palantir/witchcraft-go-health/conjure/witchcraft/api/health" + "github.com/palantir/witchcraft-go-health/sources" "github.com/palantir/witchcraft-go-health/status" ) - var healthStatusUnauthorized = health.HealthStatus{ Checks: map[health.CheckType]health.HealthCheckResult{ "HEALTH_STATUS_UNAUTHORIZED": sources.UnhealthyHealthCheckResult("HEALTH_STATUS_UNAUTHORIZED", "unauthorized to access health status; please verify the health-check-shared-secret", nil),