mirror of
https://github.com/paymog/slack-cli.git
synced 2026-09-18 23:43:49 +08:00
fix: send Slack tokens as Authorization bearer headers
slack-go's postForm puts the token in an x-www-form-urlencoded `token` field and sends no Authorization header. Slack accepts either, but a body-carried credential is invisible to anything that inspects headers: Sinatra's egress proxy swaps an opaque `sin_` sentinel for the real bot token on the way out, saw no header to rewrite, passed the sentinel through verbatim, and every sandbox call came back `invalid_auth`. Promote the `token` form field to `Authorization: Bearer` in the shared HTTP client, so every Web API call authenticates the way brokers, MITM proxies, and audit tooling expect. Content-Length and GetBody are kept honest so slack-go's retries replay the rewritten body. Browser-session tokens (`xoxc-`, paired with the `d` cookie) stay in the body — they are not bearer credentials and the edge API wants them there.
This commit is contained in:
@@ -0,0 +1,82 @@
|
||||
package transport
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// AuthHeaderTransport moves the Slack `token` form field into an
|
||||
// `Authorization: Bearer` header.
|
||||
//
|
||||
// slack-go's postForm sends every Web API call as x-www-form-urlencoded with
|
||||
// the token in a `token` field and no Authorization header. Slack accepts
|
||||
// either, but body-carried credentials are invisible to the credential
|
||||
// brokers, MITM proxies, and audit tooling that sit in front of this CLI —
|
||||
// they only inspect headers, so a body token is passed through verbatim and
|
||||
// Slack answers `invalid_auth`.
|
||||
//
|
||||
// Browser-session credentials (`xoxc-`, paired with the `d` cookie) stay in
|
||||
// the body: they are not bearer tokens and the edge API expects them there.
|
||||
type AuthHeaderTransport struct {
|
||||
roundTripper http.RoundTripper
|
||||
}
|
||||
|
||||
// NewAuthHeaderTransport wraps rt so form-carried tokens become bearer headers.
|
||||
func NewAuthHeaderTransport(rt http.RoundTripper) *AuthHeaderTransport {
|
||||
return &AuthHeaderTransport{roundTripper: rt}
|
||||
}
|
||||
|
||||
// RoundTrip implements the http.RoundTripper interface.
|
||||
func (t *AuthHeaderTransport) RoundTrip(req *http.Request) (*http.Response, error) {
|
||||
rewritten, err := bearerizeFormToken(req)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return t.roundTripper.RoundTrip(rewritten)
|
||||
}
|
||||
|
||||
// bearerizeFormToken returns req with its `token` form field promoted to an
|
||||
// Authorization header, or req untouched when there is nothing to promote.
|
||||
func bearerizeFormToken(req *http.Request) (*http.Request, error) {
|
||||
if req.Body == nil || req.Header.Get("Authorization") != "" {
|
||||
return req, nil
|
||||
}
|
||||
if !strings.HasPrefix(req.Header.Get("Content-Type"), "application/x-www-form-urlencoded") {
|
||||
return req, nil
|
||||
}
|
||||
|
||||
body, err := io.ReadAll(req.Body)
|
||||
req.Body.Close()
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
// Any bail-out below must hand the consumed body back to the caller.
|
||||
restore := func() *http.Request {
|
||||
req.Body = io.NopCloser(bytes.NewReader(body))
|
||||
return req
|
||||
}
|
||||
|
||||
values, err := url.ParseQuery(string(body))
|
||||
if err != nil {
|
||||
return restore(), nil
|
||||
}
|
||||
token := values.Get("token")
|
||||
if token == "" || strings.HasPrefix(token, "xoxc-") || strings.HasPrefix(token, "xoxd-") {
|
||||
return restore(), nil
|
||||
}
|
||||
|
||||
values.Del("token")
|
||||
encoded := values.Encode()
|
||||
|
||||
out := req.Clone(req.Context())
|
||||
out.Header.Set("Authorization", "Bearer "+token)
|
||||
out.Body = io.NopCloser(strings.NewReader(encoded))
|
||||
out.ContentLength = int64(len(encoded))
|
||||
out.GetBody = func() (io.ReadCloser, error) {
|
||||
return io.NopCloser(strings.NewReader(encoded)), nil
|
||||
}
|
||||
return out, nil
|
||||
}
|
||||
@@ -0,0 +1,125 @@
|
||||
package transport
|
||||
|
||||
import (
|
||||
"io"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
type captureRoundTripper struct{ req *http.Request }
|
||||
|
||||
func (c *captureRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
|
||||
c.req = req
|
||||
return &http.Response{StatusCode: 200, Body: http.NoBody, Request: req}, nil
|
||||
}
|
||||
|
||||
func formRequest(t *testing.T, body string) *http.Request {
|
||||
t.Helper()
|
||||
req, err := http.NewRequest(http.MethodPost, "https://slack.com/api/auth.test", strings.NewReader(body))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
||||
return req
|
||||
}
|
||||
|
||||
func roundTrip(t *testing.T, req *http.Request) *http.Request {
|
||||
t.Helper()
|
||||
capture := &captureRoundTripper{}
|
||||
if _, err := NewAuthHeaderTransport(capture).RoundTrip(req); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return capture.req
|
||||
}
|
||||
|
||||
func sentBody(t *testing.T, req *http.Request) url.Values {
|
||||
t.Helper()
|
||||
raw, err := io.ReadAll(req.Body)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
values, err := url.ParseQuery(string(raw))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return values
|
||||
}
|
||||
|
||||
func TestUnitFormTokenBecomesBearerHeader(t *testing.T) {
|
||||
sent := roundTrip(t, formRequest(t, "token=xoxb-real&channel=C1"))
|
||||
|
||||
if got := sent.Header.Get("Authorization"); got != "Bearer xoxb-real" {
|
||||
t.Fatalf("Authorization = %q", got)
|
||||
}
|
||||
body := sentBody(t, sent)
|
||||
if body.Has("token") {
|
||||
t.Fatalf("token left in body: %v", body)
|
||||
}
|
||||
if body.Get("channel") != "C1" {
|
||||
t.Fatalf("other fields lost: %v", body)
|
||||
}
|
||||
// A wrong Content-Length makes Slack hang up or truncate the form.
|
||||
if want := int64(len(body.Encode())); sent.ContentLength != want {
|
||||
t.Fatalf("ContentLength = %d, want %d", sent.ContentLength, want)
|
||||
}
|
||||
// slack-go's retries replay the body.
|
||||
replay, err := sent.GetBody()
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
raw, _ := io.ReadAll(replay)
|
||||
if strings.Contains(string(raw), "token=") {
|
||||
t.Fatalf("replayed body still carries the token: %q", raw)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUnitProxySentinelIsPromoted(t *testing.T) {
|
||||
// The whole point: an opaque broker sentinel must reach the header path.
|
||||
sent := roundTrip(t, formRequest(t, "token=sin_abc123&channel=C1"))
|
||||
if got := sent.Header.Get("Authorization"); got != "Bearer sin_abc123" {
|
||||
t.Fatalf("Authorization = %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUnitBrowserTokenStaysInBody(t *testing.T) {
|
||||
sent := roundTrip(t, formRequest(t, "token=xoxc-browser&channel=C1"))
|
||||
|
||||
if got := sent.Header.Get("Authorization"); got != "" {
|
||||
t.Fatalf("Authorization = %q, want empty", got)
|
||||
}
|
||||
if got := sentBody(t, sent).Get("token"); got != "xoxc-browser" {
|
||||
t.Fatalf("token = %q, want xoxc-browser", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUnitNonFormRequestBodyIsPreserved(t *testing.T) {
|
||||
req, err := http.NewRequest(http.MethodPost, "https://slack.com/api/files.upload", strings.NewReader("token=xoxb-real"))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
req.Header.Set("Content-Type", "multipart/form-data; boundary=x")
|
||||
|
||||
sent := roundTrip(t, req)
|
||||
if got := sent.Header.Get("Authorization"); got != "" {
|
||||
t.Fatalf("Authorization = %q, want empty", got)
|
||||
}
|
||||
raw, _ := io.ReadAll(sent.Body)
|
||||
if string(raw) != "token=xoxb-real" {
|
||||
t.Fatalf("body = %q, want untouched", raw)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUnitExistingAuthorizationHeaderWins(t *testing.T) {
|
||||
req := formRequest(t, "token=xoxb-real")
|
||||
req.Header.Set("Authorization", "Bearer preset")
|
||||
|
||||
sent := roundTrip(t, req)
|
||||
if got := sent.Header.Get("Authorization"); got != "Bearer preset" {
|
||||
t.Fatalf("Authorization = %q", got)
|
||||
}
|
||||
if got := sentBody(t, sent).Get("token"); got != "xoxb-real" {
|
||||
t.Fatalf("body token = %q, want untouched", got)
|
||||
}
|
||||
}
|
||||
@@ -441,6 +441,7 @@ func ProvideHTTPClient(cookies []*http.Cookie, logger *zap.Logger) *http.Client
|
||||
}
|
||||
|
||||
transport = NewUserAgentTransport(transport, userAgent, cookies, logger)
|
||||
transport = NewAuthHeaderTransport(transport)
|
||||
|
||||
client := &http.Client{
|
||||
Transport: transport,
|
||||
|
||||
Reference in New Issue
Block a user