Skip to content

Commit

Permalink
Sanitize http logs (#212)
Browse files Browse the repository at this point in the history
Put HTTP authentication in Header instead of URL. This will prevent leaking credentials in logs when error is logged with authentication information.

Fixes: #209
  • Loading branch information
tomez authored and janisz committed Mar 27, 2017
1 parent 8ccdebf commit b1622a3
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 17 deletions.
21 changes: 9 additions & 12 deletions marathon/marathon.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ type Marathon struct {
Location string
Protocol string
MyLeader string
Auth *url.Userinfo
username string
password string
client *http.Client
}

Expand All @@ -40,12 +41,6 @@ type LeaderResponse struct {
}

func New(config Config) (*Marathon, error) {
var auth *url.Userinfo
if len(config.Username) == 0 && len(config.Password) == 0 {
auth = nil
} else {
auth = url.UserPassword(config.Username, config.Password)
}
transport := &http.Transport{
Proxy: http.ProxyFromEnvironment,
TLSClientConfig: &tls.Config{
Expand All @@ -57,7 +52,8 @@ func New(config Config) (*Marathon, error) {
Location: config.Location,
Protocol: config.Protocol,
MyLeader: config.Leader,
Auth: auth,
username: config.Username,
password: config.Password,
client: &http.Client{
Transport: transport,
Timeout: config.Timeout.Duration,
Expand Down Expand Up @@ -129,7 +125,9 @@ func (m Marathon) EventStream(desiredEvents []string, retries, retryBackoff int)
}

return &Streamer{
subURL: subURL,
subURL: subURL,
username: m.username,
password: m.password,
client: &http.Client{
Transport: m.client.Transport,
},
Expand Down Expand Up @@ -177,6 +175,7 @@ func (m Marathon) get(url string) ([]byte, error) {
"Protocol": m.Protocol,
}).Debug("Sending GET request to marathon")

request.SetBasicAuth(m.username, m.password)
var response *http.Response
metrics.Time("marathon.get", func() { response, err = m.client.Do(request) })
if err != nil {
Expand Down Expand Up @@ -206,7 +205,7 @@ func (m Marathon) logHTTPError(resp *http.Response, err error) {
"Location": m.Location,
"Protocol": m.Protocol,
"statusCode": statusCode,
}).Error(err)
}).WithError(err).Warning("Error on http request")
}

func (m Marathon) url(path string) string {
Expand All @@ -224,14 +223,12 @@ func (m Marathon) urlWithQuery(path string, params params) string {
parts := strings.SplitN(m.Location, "/", 2)
marathon = url.URL{
Scheme: m.Protocol,
User: m.Auth,
Host: parts[0],
Path: "/" + parts[1] + path,
}
} else {
marathon = url.URL{
Scheme: m.Protocol,
User: m.Auth,
Host: m.Location,
Path: path,
}
Expand Down
26 changes: 24 additions & 2 deletions marathon/marathon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,14 +268,14 @@ func TestUrl_WithoutAuth(t *testing.T) {
assert.Equal(t, "http://localhost:8080/v2/apps", m.url("/v2/apps"))
}

func TestUrl_WithAuth(t *testing.T) {
func TestUrl_AuthURLShouldBeWithoutCreds(t *testing.T) {
t.Parallel()
// given
config := Config{Location: "localhost:8080", Protocol: "http", Username: "peter", Password: "parker"}
// when
m, _ := New(config)
// then
assert.Equal(t, "http://peter:parker@localhost:8080/v2/apps", m.url("/v2/apps"))
assert.Equal(t, "http://localhost:8080/v2/apps", m.url("/v2/apps"))
}

func TestLeader_SuccessfulResponse(t *testing.T) {
Expand Down Expand Up @@ -455,6 +455,28 @@ func TestUrlWithQuery_ProxyMarathon(t *testing.T) {
assert.Equal(t, "HTTP://localhost:8080/proxy/url/segments/testpath", path)
}

func TestGET_WithAuthOnErrorErrShouldNotContainUserAndPass(t *testing.T) {
t.Parallel()
// given
server, transport := mockServer(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(500)
})
defer server.Close()
srvURL, _ := url.Parse(server.URL)
config := Config{Location: srvURL.Host, Protocol: "HTTP", Username: "peter", Password: "parker"}
m, _ := New(config)
m.client.Transport = transport

// when
res, err := m.get("http://" + srvURL.Host)
auth := url.UserPassword(config.Username, config.Password)

//then
assert.Error(t, err)
assert.NotContains(t, err.Error(), auth.String())
assert.Nil(t, res)
}

// http://keighl.com/post/mocking-http-responses-in-golang/
func stubServer(uri string, body string) (*httptest.Server, *http.Transport) {
return mockServer(func(w http.ResponseWriter, r *http.Request) {
Expand Down
11 changes: 8 additions & 3 deletions marathon/streamer.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ type Streamer struct {
Scanner *bufio.Scanner
cancel context.CancelFunc
client *http.Client
username string
password string
subURL string
retries int
retryBackoff int
Expand All @@ -34,6 +36,7 @@ func (s *Streamer) Start() error {
log.Fatal("Unable to create Streamer request")
return nil
}
req.SetBasicAuth(s.username, s.password)
req.Header.Set("Accept", "text/event-stream")
ctx, cancel := context.WithCancel(context.Background())
s.cancel = cancel
Expand All @@ -45,12 +48,14 @@ func (s *Streamer) Start() error {
}
if res.StatusCode != http.StatusOK {
log.WithFields(log.Fields{
"Location": s.subURL,
"Method": "GET",
"Host": req.Host,
"URI": req.URL.RequestURI(),
"Method": "GET",
}).Errorf("Got status code : %d", res.StatusCode)
}
log.WithFields(log.Fields{
"SubUrl": s.subURL,
"Host": req.Host,
"URI": req.URL.RequestURI(),
"Method": "GET",
}).Debug("Subsciption success")
s.Scanner = bufio.NewScanner(res.Body)
Expand Down

0 comments on commit b1622a3

Please sign in to comment.