Skip to content

Commit

Permalink
fix: redact password in logs if specified as part of URL (#413)
Browse files Browse the repository at this point in the history
**Requirements**

- [x] I have added test coverage for new or changed functionality
- [x] I have followed the repository's [pull request submission
guidelines](../blob/master/CONTRIBUTING.md#submitting-pull-requests)
- [x] I have validated my changes against all supported platform
versions

**Related issues**

(Internal: see sc-249267)

**Describe the solution you've provided**

When the connection URL is specified with a username and password, this
was logged in full, for example "Using proxy server at
http://my-user-name:my-password@my-proxy-server". This fix redacts any
password specified as part of the URL from the logs.
  • Loading branch information
sheenacarswell authored Jul 15, 2024
1 parent c8ccaae commit 0471d51
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 1 deletion.
2 changes: 1 addition & 1 deletion internal/httpconfig/httpconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func NewHTTPConfig(proxyConfig config.ProxyConfig, authKey credential.SDKCredent
return ret, errProxyAuthWithoutProxyURL
}
if proxyConfig.URL.IsDefined() {
loggers.Infof("Using proxy server at %s", proxyConfig.URL)
loggers.Infof("Using proxy server at %s", proxyConfig.URL.Get().Redacted())
}

caCertFiles := proxyConfig.CACertFiles.Values()
Expand Down
20 changes: 20 additions & 0 deletions internal/httpconfig/httpconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"crypto/x509"
"net/http"
"net/http/httptest"
"net/url"
"os"
"testing"

Expand Down Expand Up @@ -137,3 +138,22 @@ func TestNTLMProxyInvalidConfigs(t *testing.T) {
}
})
}

func TestLogsRedactConnectionPassword(t *testing.T) {
// Username and password are specified separately in NTLM auth won't show in logs as they're not part of server name
url1, _ := configtypes.NewOptURLAbsoluteFromString("http://my-proxy")
proxyConfig1 := config.ProxyConfig{NTLMAuth: true, URL: url1, User: "my-user", Password: "my-pass"}
mockLog1 := ldlogtest.NewMockLog()
_, err := NewHTTPConfig(proxyConfig1, nil, "", mockLog1.Loggers)
assert.NoError(t, err)
mockLog1.AssertMessageMatch(t, true, ldlog.Info, "Using proxy server at http://my-proxy$")

// When username and password are configured as part of server name, verify the password is redacted
url2, _ := url.Parse("http://my-user:my-password@my-proxy")
url2Absolute, _ := configtypes.NewOptURLAbsolute(url2)
proxyConfig2 := config.ProxyConfig{URL: url2Absolute}
mockLog2 := ldlogtest.NewMockLog()
_, err = NewHTTPConfig(proxyConfig2, nil, "", mockLog2.Loggers)
assert.NoError(t, err)
mockLog2.AssertMessageMatch(t, true, ldlog.Info, "Using proxy server at http://my-user:xxxxx@my-proxy$")
}

0 comments on commit 0471d51

Please sign in to comment.