Fixing CLI ACL token processing unexpected precedence (#15274)

* Fixing CLI ACL token processing unexpected precedence

* Minor flow format and add Changelog

* Fixed failed tests and improve error logging message

* Add unit test cases and minor changes from code review

* Unset env var once the test case finishes running

* remove label FINISH
This commit is contained in:
Dao Thanh Tung 2022-12-03 01:19:52 +08:00 committed by GitHub
parent c27e246715
commit 6d519c7343
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 81 additions and 5 deletions

3
.changelog/15274.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note: bug
cli: fix ACL token processing unexpected precedence
```

View File

@ -5,6 +5,7 @@ import (
"encoding/json" "encoding/json"
"errors" "errors"
"fmt" "fmt"
"github.com/stretchr/testify/assert"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"net/http/httputil" "net/http/httputil"
@ -263,6 +264,63 @@ func TestAgent_ServiceRegisterOpts_WithContextTimeout(t *testing.T) {
require.True(t, errors.Is(err, context.DeadlineExceeded), "expected timeout") require.True(t, errors.Is(err, context.DeadlineExceeded), "expected timeout")
} }
func TestAPI_NewClient_TokenFileCLIFirstPriority(t *testing.T) {
os.Setenv("CONSUL_HTTP_TOKEN_FILE", "httpTokenFile.txt")
os.Setenv("CONSUL_HTTP_TOKEN", "httpToken")
nonExistentTokenFile := "randomTokenFile.txt"
config := Config{
Token: "randomToken",
TokenFile: nonExistentTokenFile,
}
_, err := NewClient(&config)
errorMessage := fmt.Sprintf("Error loading token file %s : open %s: no such file or directory", nonExistentTokenFile, nonExistentTokenFile)
assert.EqualError(t, err, errorMessage)
os.Unsetenv("CONSUL_HTTP_TOKEN_FILE")
os.Unsetenv("CONSUL_HTTP_TOKEN")
}
func TestAPI_NewClient_TokenCLISecondPriority(t *testing.T) {
os.Setenv("CONSUL_HTTP_TOKEN_FILE", "httpTokenFile.txt")
os.Setenv("CONSUL_HTTP_TOKEN", "httpToken")
tokenString := "randomToken"
config := Config{
Token: tokenString,
}
c, err := NewClient(&config)
if err != nil {
t.Fatalf("Error Initializing new client: %v", err)
}
assert.Equal(t, c.config.Token, tokenString)
os.Unsetenv("CONSUL_HTTP_TOKEN_FILE")
os.Unsetenv("CONSUL_HTTP_TOKEN")
}
func TestAPI_NewClient_HttpTokenFileEnvVarThirdPriority(t *testing.T) {
nonExistentTokenFileEnvVar := "httpTokenFile.txt"
os.Setenv("CONSUL_HTTP_TOKEN_FILE", nonExistentTokenFileEnvVar)
os.Setenv("CONSUL_HTTP_TOKEN", "httpToken")
_, err := NewClient(DefaultConfig())
errorMessage := fmt.Sprintf("Error loading token file %s : open %s: no such file or directory", nonExistentTokenFileEnvVar, nonExistentTokenFileEnvVar)
assert.EqualError(t, err, errorMessage)
os.Unsetenv("CONSUL_HTTP_TOKEN_FILE")
os.Unsetenv("CONSUL_HTTP_TOKEN")
}
func TestAPI_NewClient_TokenEnvVarFinalPriority(t *testing.T) {
httpTokenEnvVar := "httpToken"
os.Setenv("CONSUL_HTTP_TOKEN", httpTokenEnvVar)
c, err := NewClient(DefaultConfig())
if err != nil {
t.Fatalf("Error Initializing new client: %v", err)
}
assert.Equal(t, c.config.Token, httpTokenEnvVar)
os.Unsetenv("CONSUL_HTTP_TOKEN")
}
func TestAPI_AgentServices(t *testing.T) { func TestAPI_AgentServices(t *testing.T) {
t.Parallel() t.Parallel()
c, s := makeClient(t) c, s := makeClient(t)

View File

@ -750,20 +750,35 @@ func NewClient(config *Config) (*Client, error) {
// If the TokenFile is set, always use that, even if a Token is configured. // If the TokenFile is set, always use that, even if a Token is configured.
// This is because when TokenFile is set it is read into the Token field. // This is because when TokenFile is set it is read into the Token field.
// We want any derived clients to have to re-read the token file. // We want any derived clients to have to re-read the token file.
if config.TokenFile != "" { // The precedence of ACL token should be:
// 1. -token-file cli option
// 2. -token cli option
// 3. CONSUL_HTTP_TOKEN_FILE environment variable
// 4. CONSUL_HTTP_TOKEN environment variable
if config.TokenFile != "" && config.TokenFile != defConfig.TokenFile {
data, err := os.ReadFile(config.TokenFile) data, err := os.ReadFile(config.TokenFile)
if err != nil { if err != nil {
return nil, fmt.Errorf("Error loading token file: %s", err) return nil, fmt.Errorf("Error loading token file %s : %s", config.TokenFile, err)
} }
if token := strings.TrimSpace(string(data)); token != "" { if token := strings.TrimSpace(string(data)); token != "" {
config.Token = token config.Token = token
} }
} } else if config.Token != "" && defConfig.Token != config.Token {
if config.Token == "" { // Fall through
} else if defConfig.TokenFile != "" {
data, err := os.ReadFile(defConfig.TokenFile)
if err != nil {
return nil, fmt.Errorf("Error loading token file %s : %s", defConfig.TokenFile, err)
}
if token := strings.TrimSpace(string(data)); token != "" {
config.Token = token
config.TokenFile = defConfig.TokenFile
}
} else {
config.Token = defConfig.Token config.Token = defConfig.Token
} }
return &Client{config: *config, headers: make(http.Header)}, nil return &Client{config: *config, headers: make(http.Header)}, nil
} }