Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redact tokens in SendMessage debug log #1215

Merged
merged 3 commits into from
Aug 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 19 additions & 4 deletions chat.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"io/ioutil"
"net/http"
"net/url"
"regexp"
"strconv"

"github.com/slack-go/slack/slackutilsx"
Expand All @@ -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
}
Expand Down Expand Up @@ -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 {
Expand All @@ -234,6 +235,20 @@ 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
// 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 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
// (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.
Expand Down
51 changes: 50 additions & 1 deletion chat_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package slack

import (
"bytes"
"encoding/json"
"io/ioutil"
"log"
"net/http"
"net/url"
"reflect"
"regexp"
"testing"
)

Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -296,3 +298,50 @@ func TestPostMessageWhenMsgOptionDeleteOriginalApplied(t *testing.T) {

_, _, _ = api.PostMessage("CXXX", MsgOptionDeleteOriginal(responseURL))
}

func TestSendMessageContextRedactsTokenInDebugLog(t *testing.T) {
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",
},
}
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)
}
})
}
}
8 changes: 4 additions & 4 deletions interactions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
Expand Down