From 0158aa2bd5256a5846d08dfbebc485a2e8455a34 Mon Sep 17 00:00:00 2001 From: Joel Unzain Date: Fri, 18 Jun 2021 12:48:58 -0700 Subject: [PATCH 1/2] update sanitize headers logic --- basculehttp/log.go | 19 +++++++++++++------ basculehttp/log_test.go | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/basculehttp/log.go b/basculehttp/log.go index 0df7aa5..7a8bbe7 100644 --- a/basculehttp/log.go +++ b/basculehttp/log.go @@ -51,20 +51,27 @@ func getZapLogger(f func(context.Context) *zap.Logger) func(context.Context) log } } +func sanitizeHeaders(headers http.Header) (filtered http.Header) { + filtered = headers.Clone() + if authHeader := filtered.Get("Authorization"); authHeader != "" { + filtered.Del("Authorization") + parts := strings.Split(authHeader, " ") + if len(parts) == 2 { + filtered.Set("Authorization-Type", parts[0]) + } + } + return +} + // SetLogger creates an alice constructor that sets up a zap logger that can be // used for all logging related to the current request. The logger is added to // the request's context. func SetLogger(logger *zap.Logger) alice.Constructor { return func(delegate http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - logHeader := r.Header.Clone() - if str := logHeader.Get("Authorization"); str != "" { - logHeader.Del("Authorization") - logHeader.Set("Authorization-Type", strings.Split(str, " ")[0]) - } r = r.WithContext(sallust.With(r.Context(), logger.With( - zap.Reflect("requestHeaders", logHeader), //lgtm [go/clear-text-logging] + zap.Reflect("requestHeaders", sanitizeHeaders(r.Header)), //lgtm [go/clear-text-logging] zap.String("requestURL", r.URL.EscapedPath()), zap.String("method", r.Method)))) delegate.ServeHTTP(w, r) diff --git a/basculehttp/log_test.go b/basculehttp/log_test.go index 4fbfb09..a863bdf 100644 --- a/basculehttp/log_test.go +++ b/basculehttp/log_test.go @@ -19,6 +19,7 @@ package basculehttp import ( "context" + "net/http" "testing" "github.com/stretchr/testify/assert" @@ -36,3 +37,36 @@ func TestGetZapLogger(t *testing.T) { result.Log("msg", "testing", "error", "nope", "level", "debug") }) } + +func TestSanitizeHeaders(t *testing.T) { + testCases := []struct { + Description string + Input http.Header + Expected http.Header + }{ + { + Description: "Filtered", + Input: http.Header{"Authorization": []string{"Basic xyz"}, "HeaderA": []string{"x"}}, + Expected: http.Header{"HeaderA": []string{"x"}, "Authorization-Type": []string{"Basic"}}, + }, + { + Description: "Handled human error", + Input: http.Header{"Authorization": []string{"BasicXYZ"}, "HeaderB": []string{"y"}}, + Expected: http.Header{"HeaderB": []string{"y"}}, + }, + { + Description: "Not a perfect system", + Input: http.Header{"Authorization": []string{"MySecret IWantToLeakIt"}}, + Expected: http.Header{"Authorization-Type": []string{"MySecret"}}, + }, + } + + for _, tc := range testCases { + t.Run(tc.Description, func(t *testing.T) { + assert := assert.New(t) + actual := sanitizeHeaders(tc.Input) + assert.Equal(tc.Expected, actual) + }) + + } +} From e516a1193dc84be730e5a9071351f2ce4f8ecce9 Mon Sep 17 00:00:00 2001 From: Joel Unzain Date: Fri, 18 Jun 2021 12:50:31 -0700 Subject: [PATCH 2/2] add release note --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d25dff..0bf02a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). ## [Unreleased] +- Update setLogger Authorization header filtering logic. [#111](https://github.com/xmidt-org/bascule/pull/111) ## [v0.10.1] - Added raw parsers for bearer acquirer. [#110](https://github.com/xmidt-org/bascule/pull/110)