From e54a868045b1a399ca9c85033d504f9b3f55e6dc Mon Sep 17 00:00:00 2001 From: Daniel Abraham Date: Fri, 14 Jul 2023 16:01:44 -0700 Subject: [PATCH 1/3] Redact tokens in debug log --- chat.go | 21 +++++++++++++++++---- chat_test.go | 32 +++++++++++++++++++++++++++++++- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/chat.go b/chat.go index 4448e9f40..21bf7c74d 100644 --- a/chat.go +++ b/chat.go @@ -7,6 +7,7 @@ import ( "io/ioutil" "net/http" "net/url" + "regexp" "strconv" "github.com/slack-go/slack/slackutilsx" @@ -29,9 +30,9 @@ const ( type chatResponseFull struct { Channel string `json:"channel"` - Timestamp string `json:"ts"` //Regular message timestamp - MessageTimeStamp string `json:"message_ts"` //Ephemeral message timestamp - ScheduledMessageID string `json:"scheduled_message_id,omitempty"` //Scheduled message id + Timestamp string `json:"ts"` // Regular message timestamp + MessageTimeStamp string `json:"message_ts"` // Ephemeral message timestamp + ScheduledMessageID string `json:"scheduled_message_id,omitempty"` // Scheduled message id Text string `json:"text"` SlackResponse } @@ -224,7 +225,7 @@ func (api *Client) SendMessageContext(ctx context.Context, channelID string, opt return "", "", "", err } req.Body = ioutil.NopCloser(bytes.NewBuffer(reqBody)) - api.Debugf("Sending request: %s", string(reqBody)) + api.Debugf("Sending request: %s", redactToken(reqBody)) } if err = doPost(ctx, api.httpclient, req, parser(&response), api); err != nil { @@ -234,6 +235,18 @@ func (api *Client) SendMessageContext(ctx context.Context, channelID string, opt return response.Channel, response.getMessageTimestamp(), response.Text, response.Err() } +func redactToken(b []byte) []byte { + // See https://api.slack.com/authentication/token-types + re, err := regexp.Compile(`(token=x[a-z]+)-[0-9A-Za-z-]+`) + if err != nil { + // The regular expression above should always work, but just in case, do no harm. + return b + } + // Keep "token=" and the first element of the token, which identifies its type + // (this could be useful for debugging, e.g. when using a wrong token). + return re.ReplaceAll(b, []byte("$1-REDACTED")) +} + // UnsafeApplyMsgOptions utility function for debugging/testing chat requests. // NOTE: USE AT YOUR OWN RISK: No issues relating to the use of this function // will be supported by the library. diff --git a/chat_test.go b/chat_test.go index 7a549fa90..b9cda0727 100644 --- a/chat_test.go +++ b/chat_test.go @@ -1,11 +1,14 @@ package slack import ( + "bytes" "encoding/json" "io/ioutil" + "log" "net/http" "net/url" "reflect" + "regexp" "testing" ) @@ -39,7 +42,6 @@ func TestGetPermalink(t *testing.T) { timeStamp := "p135854651500008" http.HandleFunc("/chat.getPermalink", func(rw http.ResponseWriter, r *http.Request) { - if got, want := r.Header.Get("Content-Type"), "application/x-www-form-urlencoded"; got != want { t.Errorf("request uses unexpected content type: got %s, want %s", got, want) } @@ -296,3 +298,31 @@ func TestPostMessageWhenMsgOptionDeleteOriginalApplied(t *testing.T) { _, _, _ = api.PostMessage("CXXX", MsgOptionDeleteOriginal(responseURL)) } + +func TestSendMessageContextRedactsTokenInDebugLog(t *testing.T) { + once.Do(startServer) + buf := bytes.NewBufferString("") + + token := "xtest-token-1234-abcd" + opts := []Option{ + OptionAPIURL("http://" + serverAddr + "/"), + OptionLog(log.New(buf, "", log.Lshortfile)), + OptionDebug(true), + } + api := New(token, opts...) + // Why send the token in the message text too? To test that we're not + // redacting substrings in the request which look like a token but aren't. + api.SendMessage("CXXX", MsgOptionText(token, false)) + s := buf.String() + + re := regexp.MustCompile(`token=[\w-]*`) + want := "token=xtest-REDACTED" + if got := re.FindString(s); got != want { + t.Errorf("Logged token in SendMessageContext(): got %q, want %q", got, want) + } + re = regexp.MustCompile(`text=[\w-]*`) + want = "text=" + token + if got := re.FindString(s); got != want { + t.Errorf("Logged text in SendMessageContext(): got %q, want %q", got, want) + } +} From 05af10ccc2c333ce900a99e8c12a85c70f687a48 Mon Sep 17 00:00:00 2001 From: Daniel Abraham Date: Fri, 14 Jul 2023 16:02:11 -0700 Subject: [PATCH 2/3] Tiny unrelated lint fix --- interactions_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/interactions_test.go b/interactions_test.go index f492b1fd0..989690e27 100644 --- a/interactions_test.go +++ b/interactions_test.go @@ -286,14 +286,14 @@ func TestViewSubmissionCallback(t *testing.T) { }, State: &ViewState{ Values: map[string]map[string]BlockAction{ - "multi-line": map[string]BlockAction{ - "ml-value": BlockAction{ + "multi-line": { + "ml-value": { Type: "plain_text_input", Value: "No onions", }, }, - "target_channel": map[string]BlockAction{ - "target_select": BlockAction{ + "target_channel": { + "target_select": { Type: "conversations_select", Value: "C1AB2C3DE", }, From fcee000beb31655570d7213e2102641dee6162a0 Mon Sep 17 00:00:00 2001 From: Daniel Abraham Date: Sat, 15 Jul 2023 17:43:00 -0700 Subject: [PATCH 3/3] Add support for refresh tokens --- chat.go | 6 +++-- chat_test.go | 65 +++++++++++++++++++++++++++++++++------------------- 2 files changed, 46 insertions(+), 25 deletions(-) diff --git a/chat.go b/chat.go index 21bf7c74d..35ffbbcf8 100644 --- a/chat.go +++ b/chat.go @@ -237,9 +237,11 @@ func (api *Client) SendMessageContext(ctx context.Context, channelID string, opt func redactToken(b []byte) []byte { // See https://api.slack.com/authentication/token-types - re, err := regexp.Compile(`(token=x[a-z]+)-[0-9A-Za-z-]+`) + // and https://api.slack.com/authentication/rotation + re, err := regexp.Compile(`(token=x[a-z.]+)-[0-9A-Za-z-]+`) if err != nil { - // The regular expression above should always work, but just in case, do no harm. + // The regular expression above should never result in errors, + // but just in case, do no harm. return b } // Keep "token=" and the first element of the token, which identifies its type diff --git a/chat_test.go b/chat_test.go index b9cda0727..fb372259a 100644 --- a/chat_test.go +++ b/chat_test.go @@ -300,29 +300,48 @@ func TestPostMessageWhenMsgOptionDeleteOriginalApplied(t *testing.T) { } func TestSendMessageContextRedactsTokenInDebugLog(t *testing.T) { - once.Do(startServer) - buf := bytes.NewBufferString("") - - token := "xtest-token-1234-abcd" - opts := []Option{ - OptionAPIURL("http://" + serverAddr + "/"), - OptionLog(log.New(buf, "", log.Lshortfile)), - OptionDebug(true), - } - api := New(token, opts...) - // Why send the token in the message text too? To test that we're not - // redacting substrings in the request which look like a token but aren't. - api.SendMessage("CXXX", MsgOptionText(token, false)) - s := buf.String() - - re := regexp.MustCompile(`token=[\w-]*`) - want := "token=xtest-REDACTED" - if got := re.FindString(s); got != want { - t.Errorf("Logged token in SendMessageContext(): got %q, want %q", got, want) + tests := []struct { + name string + token string + want string + }{ + { + name: "regular token", + token: "xtest-token-1234-abcd", + want: "xtest-REDACTED", + }, + { + name: "refresh token", + token: "xoxe.xtest-token-1234-abcd", + want: "xoxe.xtest-REDACTED", + }, } - re = regexp.MustCompile(`text=[\w-]*`) - want = "text=" + token - if got := re.FindString(s); got != want { - t.Errorf("Logged text in SendMessageContext(): got %q, want %q", got, want) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + once.Do(startServer) + buf := bytes.NewBufferString("") + + opts := []Option{ + OptionAPIURL("http://" + serverAddr + "/"), + OptionLog(log.New(buf, "", log.Lshortfile)), + OptionDebug(true), + } + api := New(tt.token, opts...) + // Why send the token in the message text too? To test that we're not + // redacting substrings in the request which look like a token but aren't. + api.SendMessage("CXXX", MsgOptionText(token, false)) + s := buf.String() + + re := regexp.MustCompile(`token=[\w.-]*`) + want := "token=" + tt.want + if got := re.FindString(s); got != want { + t.Errorf("Logged token in SendMessageContext(): got %q, want %q", got, want) + } + re = regexp.MustCompile(`text=[\w.-]*`) + want = "text=" + token + if got := re.FindString(s); got != want { + t.Errorf("Logged text in SendMessageContext(): got %q, want %q", got, want) + } + }) } }