From d28900b199741ef43c3edd3e147a346c98af896f Mon Sep 17 00:00:00 2001 From: Ross Kirkpatrick Date: Tue, 7 May 2024 11:29:18 -0400 Subject: [PATCH 1/3] mask the value of the Authorization header if debug is enabled Signed-off-by: Ross Kirkpatrick Signed-off-by: rosskirkpat --- client.go | 12 +++++++- client_test.go | 59 +++++++++++++++++++++++++++++++++++++++ internal/testutil/mock.go | 39 ++++++++++++++++++++++++++ 3 files changed, 109 insertions(+), 1 deletion(-) diff --git a/client.go b/client.go index 2b6f90e5b..7866ca637 100644 --- a/client.go +++ b/client.go @@ -90,7 +90,7 @@ type ( ) func init() { - // Wether or not we will enable Resty debugging output + // Whether we will enable Resty debugging output if apiDebug, ok := os.LookupEnv("LINODE_DEBUG"); ok { if parsed, err := strconv.ParseBool(apiDebug); err == nil { envDebug = parsed @@ -122,6 +122,8 @@ func (c *Client) R(ctx context.Context) *resty.Request { func (c *Client) SetDebug(debug bool) *Client { c.debug = debug c.resty.SetDebug(debug) + // this ensures that if there is an Authorization header present, the value is sanitized/masked + c.SanitizeAuthorizationHeader() return c } @@ -412,6 +414,14 @@ func (c *Client) SetHeader(name, value string) { c.resty.SetHeader(name, value) } +func (c *Client) SanitizeAuthorizationHeader() { + c.resty.OnRequestLog(func(r *resty.RequestLog) error { + // masking authorization header + r.Header.Set("Authorization", "Bearer *******************************") + return nil + }) +} + // NewClient factory to create new Client struct func NewClient(hc *http.Client) (client Client) { if hc != nil { diff --git a/client_test.go b/client_test.go index 7176d6cb7..3a4a38a1e 100644 --- a/client_test.go +++ b/client_test.go @@ -1,10 +1,16 @@ package linodego import ( + "bytes" + "context" "fmt" + "reflect" + "strings" "testing" "github.com/google/go-cmp/cmp" + "github.com/jarcoal/httpmock" + "github.com/linode/linodego/internal/testutil" ) func TestClient_SetAPIVersion(t *testing.T) { @@ -139,3 +145,56 @@ api_version = v4beta [cool] token = blah ` + +func TestDebugLogSanitization(t *testing.T) { + type instanceResponse struct { + ID int `json:"id"` + Region string `json:"region"` + Label string `json:"label"` + } + + var testResp = instanceResponse{ + ID: 100, + Region: "test-central", + Label: "this-is-a-test-linode", + } + var lgr bytes.Buffer + + plainTextToken := "NOTANAPIKEY" + + mockClient := testutil.CreateMockClient(t, NewClient) + logger := testutil.CreateLogger() + mockClient.SetLogger(logger) + logger.L.SetOutput(&lgr) + + mockClient.SetDebug(true) + if !mockClient.resty.Debug { + t.Fatal("debug should be enabled") + } + mockClient.SetHeader("Authorization", fmt.Sprintf("Bearer %s", plainTextToken)) + + if mockClient.resty.Header.Get("Authorization") != fmt.Sprintf("Bearer %s", plainTextToken) { + t.Fatal("token not found in auth header") + } + + httpmock.RegisterRegexpResponder("GET", testutil.MockRequestURL("/linode/instances"), + httpmock.NewJsonResponderOrPanic(200, &testResp)) + + result, err := doGETRequest[instanceResponse]( + context.Background(), + mockClient, + "/linode/instances", + ) + if err != nil { + t.Fatal(err) + } + + logInfo := lgr.String() + if !strings.Contains(logInfo, "Bearer *******************************") { + t.Fatal("sanitized bearer token was expected") + } + + if !reflect.DeepEqual(*result, testResp) { + t.Fatalf("actual response does not equal desired response: %s", cmp.Diff(result, testResponse)) + } +} diff --git a/internal/testutil/mock.go b/internal/testutil/mock.go index 4afe4a92f..a44e1506b 100644 --- a/internal/testutil/mock.go +++ b/internal/testutil/mock.go @@ -4,7 +4,9 @@ import ( "encoding/json" "fmt" "io" + "log" "net/http" + "os" "reflect" "regexp" "strings" @@ -87,3 +89,40 @@ func CreateMockClient[T any](t *testing.T, createFunc func(*http.Client) T) *T { result := createFunc(client) return &result } + +type Logger interface { + Errorf(format string, v ...interface{}) + Warnf(format string, v ...interface{}) + Debugf(format string, v ...interface{}) +} + +func CreateLogger() *TestLogger { + l := &TestLogger{L: log.New(os.Stderr, "", log.Ldate|log.Lmicroseconds)} + return l +} + +var _ Logger = (*TestLogger)(nil) + +type TestLogger struct { + L *log.Logger +} + +func (l *TestLogger) Errorf(format string, v ...interface{}) { + l.output("ERROR RESTY "+format, v...) +} + +func (l *TestLogger) Warnf(format string, v ...interface{}) { + l.output("WARN RESTY "+format, v...) +} + +func (l *TestLogger) Debugf(format string, v ...interface{}) { + l.output("DEBUG RESTY "+format, v...) +} + +func (l *TestLogger) output(format string, v ...interface{}) { + if len(v) == 0 { + l.L.Print(format) + return + } + l.L.Printf(format, v...) +} From e5dac7cf1ce975809786f04b3941afe3e66f75f9 Mon Sep 17 00:00:00 2001 From: Ross Kirkpatrick Date: Tue, 7 May 2024 13:56:17 -0400 Subject: [PATCH 2/3] fix linters Signed-off-by: Ross Kirkpatrick --- internal/testutil/mock.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/testutil/mock.go b/internal/testutil/mock.go index a44e1506b..aaaeebb47 100644 --- a/internal/testutil/mock.go +++ b/internal/testutil/mock.go @@ -108,18 +108,18 @@ type TestLogger struct { } func (l *TestLogger) Errorf(format string, v ...interface{}) { - l.output("ERROR RESTY "+format, v...) + l.outputf("ERROR RESTY "+format, v...) } func (l *TestLogger) Warnf(format string, v ...interface{}) { - l.output("WARN RESTY "+format, v...) + l.outputf("WARN RESTY "+format, v...) } func (l *TestLogger) Debugf(format string, v ...interface{}) { - l.output("DEBUG RESTY "+format, v...) + l.outputf("DEBUG RESTY "+format, v...) } -func (l *TestLogger) output(format string, v ...interface{}) { +func (l *TestLogger) outputf(format string, v ...interface{}) { if len(v) == 0 { l.L.Print(format) return From 708fc2ad17c7a1efac6838230f864672730438f8 Mon Sep 17 00:00:00 2001 From: Ross Kirkpatrick Date: Tue, 14 May 2024 10:37:09 -0400 Subject: [PATCH 3/3] make the auth header sanitization function private Signed-off-by: Ross Kirkpatrick --- client.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client.go b/client.go index 7866ca637..bd730bc04 100644 --- a/client.go +++ b/client.go @@ -123,7 +123,7 @@ func (c *Client) SetDebug(debug bool) *Client { c.debug = debug c.resty.SetDebug(debug) // this ensures that if there is an Authorization header present, the value is sanitized/masked - c.SanitizeAuthorizationHeader() + c.sanitizeAuthorizationHeader() return c } @@ -414,7 +414,7 @@ func (c *Client) SetHeader(name, value string) { c.resty.SetHeader(name, value) } -func (c *Client) SanitizeAuthorizationHeader() { +func (c *Client) sanitizeAuthorizationHeader() { c.resty.OnRequestLog(func(r *resty.RequestLog) error { // masking authorization header r.Header.Set("Authorization", "Bearer *******************************")