Skip to content

Commit

Permalink
fix(#105): do unauthenticated health check on /
Browse files Browse the repository at this point in the history
  • Loading branch information
Jumpy-Squirrel committed Dec 13, 2023
1 parent 98ef2c9 commit d4eb9c3
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 7 deletions.
2 changes: 1 addition & 1 deletion internal/restapi/middleware/authentication_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ func TestCheckRequestAuthorization(t *testing.T) {
},
}

r, err := http.NewRequestWithContext(context.TODO(), "GET", "/", nil)
r, err := http.NewRequestWithContext(context.TODO(), "GET", "/not/health", nil)
require.NoError(t, err)

if tt.args.xAPIKeyHeader != "" {
Expand Down
9 changes: 7 additions & 2 deletions internal/restapi/middleware/security.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,15 @@ func checkIdToken(ctx context.Context, conf *config.SecurityConfig, idTokenValue
}

// --- top level ---
func checkAllAuthentication(ctx context.Context, conf *config.SecurityConfig, apiTokenHeaderValue string, authHeaderValue string, idTokenCookieValue string, accessTokenCookieValue string) (context.Context, string, error) {
func checkAllAuthentication(ctx context.Context, method string, urlPath string, conf *config.SecurityConfig, apiTokenHeaderValue string, authHeaderValue string, idTokenCookieValue string, accessTokenCookieValue string) (context.Context, string, error) {
var success bool
var err error

// health check on / is allowed through
if method == http.MethodGet && urlPath == "/" {
return ctx, "", nil
}

// try api token first
ctx, success, err = checkApiToken(ctx, conf, apiTokenHeaderValue)
if err != nil {
Expand Down Expand Up @@ -239,7 +244,7 @@ func CheckRequestAuthorization(conf *config.SecurityConfig) func(http.Handler) h
idTokenCookieValue := parseAuthCookie(r, conf.Oidc.IdTokenCookieName)
accessTokenCookieValue := parseAuthCookie(r, conf.Oidc.AccessTokenCookieName)

ctx, userFacingErrorMessage, err := checkAllAuthentication(ctx, conf, apiTokenHeaderValue, authHeaderValue, idTokenCookieValue, accessTokenCookieValue)
ctx, userFacingErrorMessage, err := checkAllAuthentication(ctx, r.Method, r.URL.Path, conf, apiTokenHeaderValue, authHeaderValue, idTokenCookieValue, accessTokenCookieValue)
if err != nil {
logger.Warn("authorization failed: %s: %s", userFacingErrorMessage, err.Error())
common.SendUnauthorizedResponse(w, reqID, logger, userFacingErrorMessage)
Expand Down
18 changes: 14 additions & 4 deletions internal/restapi/middleware/security_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,31 +90,41 @@ func tstRequireAuthServiceCall(t *testing.T, idToken string, accToken string) {
}

func tstNothingTestCase(t *testing.T, expectedMsg string, expectedErr string) context.Context {
ctx, actualMsg, actualErr := checkAllAuthentication(context.Background(), &securityConfig256, "", "", "", "")
ctx, actualMsg, actualErr := checkAllAuthentication(context.Background(), http.MethodGet, "/not/health", &securityConfig256, "", "", "", "")
tstRequire(t, actualMsg, actualErr, expectedMsg, expectedErr)
return ctx
}

func tstApiTokenTestCase(t *testing.T, apiTokenHeaderValue string, expectedUserMsg string, expectedLoggedErr string) context.Context {
ctx, actualMsg, actualErr := checkAllAuthentication(context.Background(), &securityConfig256, apiTokenHeaderValue, "", "", "")
ctx, actualMsg, actualErr := checkAllAuthentication(context.Background(), http.MethodGet, "/not/health", &securityConfig256, apiTokenHeaderValue, "", "", "")
tstRequire(t, actualMsg, actualErr, expectedUserMsg, expectedLoggedErr)
return ctx
}

func tstAuthHeaderTestCase(t *testing.T, authHeaderValue string, expectedUserMsg string, expectedLoggedErr string) context.Context {
ctx, actualMsg, actualErr := checkAllAuthentication(context.Background(), &securityConfig256, "", authHeaderValue, "", "")
ctx, actualMsg, actualErr := checkAllAuthentication(context.Background(), http.MethodGet, "/not/health", &securityConfig256, "", authHeaderValue, "", "")
tstRequire(t, actualMsg, actualErr, expectedUserMsg, expectedLoggedErr)
return ctx
}

func tstCookiesTestCase(t *testing.T, idTokenCookieValue string, accessTokenCookieValue string, expectedUserMsg string, expectedLoggedErr string) context.Context {
ctx, actualMsg, actualErr := checkAllAuthentication(context.Background(), &securityConfig256, "", "", idTokenCookieValue, accessTokenCookieValue)
ctx, actualMsg, actualErr := checkAllAuthentication(context.Background(), http.MethodGet, "/not/health", &securityConfig256, "", "", idTokenCookieValue, accessTokenCookieValue)
tstRequire(t, actualMsg, actualErr, expectedUserMsg, expectedLoggedErr)
return ctx
}

// --- test cases ---

func TestHealthCheck(t *testing.T) {
docs.Description("health check is allowed through")
ctx, actualMsg, actualErr := checkAllAuthentication(context.Background(), http.MethodGet, "/", &securityConfig256, "", "", "", "")
tstRequire(t, actualMsg, actualErr, "", "")
require.Nil(t, ctx.Value(common.CtxKeyAPIKey{}))
require.Nil(t, ctx.Value(common.CtxKeyIdToken{}))
require.Nil(t, ctx.Value(common.CtxKeyAccessToken{}))
require.Nil(t, ctx.Value(common.CtxKeyClaims{}))
}

func TestNothingProvided(t *testing.T) {
docs.Description("not providing any authorization fails for this service")
ctx := tstNothingTestCase(t, "you need to supply authorization", "no authorization presented")
Expand Down
1 change: 1 addition & 0 deletions internal/restapi/v1/health/health.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (

func Create(server chi.Router) {
server.Get("/info/health", healthGet)
server.Get("/", healthGet)
}

func healthGet(w http.ResponseWriter, r *http.Request) {
Expand Down

0 comments on commit d4eb9c3

Please sign in to comment.