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

Adding support for file based configuration of basic auth username in http client config #511

Merged
merged 7 commits into from Sep 4, 2023
45 changes: 35 additions & 10 deletions config/http_config.go
Expand Up @@ -129,6 +129,7 @@ func (tv *TLSVersion) String() string {
// BasicAuth contains basic HTTP authentication credentials.
type BasicAuth struct {
Username string `yaml:"username" json:"username"`
UsernameFile string `yaml:"username_file,omitempty" json:"username_file,omitempty"`
Password Secret `yaml:"password,omitempty" json:"password,omitempty"`
PasswordFile string `yaml:"password_file,omitempty" json:"password_file,omitempty"`
}
Expand All @@ -139,6 +140,7 @@ func (a *BasicAuth) SetDirectory(dir string) {
return
}
a.PasswordFile = JoinDir(dir, a.PasswordFile)
a.UsernameFile = JoinDir(dir, a.UsernameFile)
}

// Authorization contains HTTP authorization credentials.
Expand Down Expand Up @@ -334,6 +336,9 @@ func (c *HTTPClientConfig) Validate() error {
if (c.BasicAuth != nil || c.OAuth2 != nil) && (len(c.BearerToken) > 0 || len(c.BearerTokenFile) > 0) {
return fmt.Errorf("at most one of basic_auth, oauth2, bearer_token & bearer_token_file must be configured")
}
if c.BasicAuth != nil && (string(c.BasicAuth.Username) != "" && c.BasicAuth.UsernameFile != "") {
return fmt.Errorf("at most one of basic_auth username & username_file must be configured")
}
if c.BasicAuth != nil && (string(c.BasicAuth.Password) != "" && c.BasicAuth.PasswordFile != "") {
return fmt.Errorf("at most one of basic_auth password & password_file must be configured")
}
Expand Down Expand Up @@ -555,7 +560,7 @@ func NewRoundTripperFromConfig(cfg HTTPClientConfig, name string, optFuncs ...HT
}

if cfg.BasicAuth != nil {
rt = NewBasicAuthRoundTripper(cfg.BasicAuth.Username, cfg.BasicAuth.Password, cfg.BasicAuth.PasswordFile, rt)
rt = NewBasicAuthRoundTripper(cfg.BasicAuth.Username, cfg.BasicAuth.Password, cfg.BasicAuth.PasswordFile, cfg.BasicAuth.UsernameFile, rt)
}

if cfg.OAuth2 != nil {
Expand Down Expand Up @@ -646,32 +651,52 @@ type basicAuthRoundTripper struct {
username string
password Secret
passwordFile string
usernameFile string
rt http.RoundTripper
}

// NewBasicAuthRoundTripper will apply a BASIC auth authorization header to a request unless it has
// already been set.
func NewBasicAuthRoundTripper(username string, password Secret, passwordFile string, rt http.RoundTripper) http.RoundTripper {
return &basicAuthRoundTripper{username, password, passwordFile, rt}
func NewBasicAuthRoundTripper(username string, password Secret, passwordFile string, usernameFile string, rt http.RoundTripper) http.RoundTripper {
wasim-nihal marked this conversation as resolved.
Show resolved Hide resolved
return &basicAuthRoundTripper{username, password, passwordFile, usernameFile, rt}
wasim-nihal marked this conversation as resolved.
Show resolved Hide resolved
}

func (rt *basicAuthRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
if len(req.Header.Get("Authorization")) != 0 {
return rt.rt.RoundTrip(req)
}
username, err := getBasicAuthUsername(*rt)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think extra funtions are needed of they do not share code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addressed the comment. Moved this logic back to the basic auth RoundTrip method.

if err != nil {
return nil, err
}
password, err := getBasicAuthPassword(*rt)
if err != nil {
return nil, err
}
req = cloneRequest(req)
req.SetBasicAuth(username, password)
return rt.rt.RoundTrip(req)
}
func getBasicAuthUsername(rt basicAuthRoundTripper) (string, error) {
if rt.usernameFile != "" {
usernameBytes, err := os.ReadFile(rt.usernameFile)
if err != nil {
return "", fmt.Errorf("unable to read basic auth password file %s: %s", rt.usernameFile, err)
}
return strings.TrimSpace(string(usernameBytes)), nil
}
return rt.username, nil
}
func getBasicAuthPassword(rt basicAuthRoundTripper) (string, error) {
if rt.passwordFile != "" {
bs, err := os.ReadFile(rt.passwordFile)
passwordBytes, err := os.ReadFile(rt.passwordFile)
if err != nil {
return nil, fmt.Errorf("unable to read basic auth password file %s: %s", rt.passwordFile, err)
return "", fmt.Errorf("unable to read basic auth password file %s: %s", rt.passwordFile, err)
}
req.SetBasicAuth(rt.username, strings.TrimSpace(string(bs)))
} else {
req.SetBasicAuth(rt.username, strings.TrimSpace(string(rt.password)))
return strings.TrimSpace(string(passwordBytes)), nil
}
return rt.rt.RoundTrip(req)
return string(rt.password), nil
}

func (rt *basicAuthRoundTripper) CloseIdleConnections() {
if ci, ok := rt.rt.(closeIdler); ok {
ci.CloseIdleConnections()
Expand Down
30 changes: 30 additions & 0 deletions config/http_config_test.go
Expand Up @@ -83,6 +83,10 @@ var invalidHTTPClientConfigs = []struct {
httpClientConfigFile: "testdata/http.conf.basic-auth.too-much.bad.yaml",
errMsg: "at most one of basic_auth password & password_file must be configured",
},
{
httpClientConfigFile: "testdata/http.conf.basic-auth.bad-username.yaml",
errMsg: "at most one of basic_auth username & username_file must be configured",
},
{
httpClientConfigFile: "testdata/http.conf.mix-bearer-and-creds.bad.yaml",
errMsg: "authorization is not compatible with bearer_token & bearer_token_file",
Expand Down Expand Up @@ -896,6 +900,32 @@ func TestBasicAuthPasswordFile(t *testing.T) {
}
}

func TestBasicUsernameFile(t *testing.T) {
cfg, _, err := LoadHTTPConfigFile("testdata/http.conf.basic-auth.username-file.good.yaml")
if err != nil {
t.Fatalf("Error loading HTTP client config: %v", err)
}
client, err := NewClientFromConfig(*cfg, "test")
if err != nil {
t.Fatalf("Error creating HTTP Client: %v", err)
}

rt, ok := client.Transport.(*basicAuthRoundTripper)
if !ok {
t.Fatalf("Error casting to basic auth transport, %v", client.Transport)
}

if rt.username != "" {
t.Errorf("Bad HTTP client username: %s", rt.username)
}
if string(rt.usernameFile) != "testdata/basic-auth-username" {
t.Errorf("Bad HTTP client usernameFile: %s", rt.usernameFile)
}
if string(rt.passwordFile) != "testdata/basic-auth-password" {
t.Errorf("Bad HTTP client passwordFile: %s", rt.passwordFile)
}
}

func getCertificateBlobs(t *testing.T) map[string][]byte {
files := []string{
TLSCAChainPath,
Expand Down
1 change: 1 addition & 0 deletions config/testdata/basic-auth-username
@@ -0,0 +1 @@
testuser
4 changes: 4 additions & 0 deletions config/testdata/http.conf.basic-auth.bad-username.yaml
@@ -0,0 +1,4 @@
basic_auth:
username: user
username_file: testdata/basic-auth-username
password: foo
3 changes: 3 additions & 0 deletions config/testdata/http.conf.basic-auth.username-file.good.yaml
@@ -0,0 +1,3 @@
basic_auth:
username_file: testdata/basic-auth-username
password_file: testdata/basic-auth-password
You are viewing a condensed version of this merge commit. You can view the full changes here.