Repository: allegro/marathon-consul (go)

Issue:
Santize logs
Currenlty logs can contain passwords to Marathon. 

```bash
Get http://***:***@localhost:8080/v2/leader: dial tcp 127.0.0.1:8080: getsockopt: connection refused
```

We should not log it and best keep it in header.

Pull request:
Fixes #209 | Sanitize http logs
Put HTTP authentication in Header instead of URL. This will prevent leaking credentials in logs when error is logged with authentication information.

Patch:
diff --git a/marathon/marathon.go b/marathon/marathon.go
index 8121ab0..6cab932 100644
--- a/marathon/marathon.go
+++ b/marathon/marathon.go
@@ -31,7 +31,8 @@ type Marathon struct {
 	Location string
 	Protocol string
 	MyLeader string
-	Auth     *url.Userinfo
+	username string
+	password string
 	client   *http.Client
 }
 
@@ -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{
@@ -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,
@@ -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,
 		},
@@ -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 {
@@ -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 {
@@ -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,
 		}
diff --git a/marathon/streamer.go b/marathon/streamer.go
index 2d1e5c1..0353b2b 100644
--- a/marathon/streamer.go
+++ b/marathon/streamer.go
@@ -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
@@ -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
@@ -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)


Test patch:
diff --git a/marathon/marathon_test.go b/marathon/marathon_test.go
index 0e29b40..fc9b3a4 100644
--- a/marathon/marathon_test.go
+++ b/marathon/marathon_test.go
@@ -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) {
@@ -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) {