fix: parse userinfo provided in nomad address

internal/cli/helpers.go

@@ -5,8 +5,8 @@ package cli
 
 import (
 	"fmt"
+	"net/url"
 	"os"
-	"strings"
 
 	"github.com/hashicorp/nomad/api"
 
@@ -413,30 +413,31 @@ func clientOptsFromCLI(c *baseCommand) *api.Config {
 	return conf
 }
 
-// handlBasicAuth checks whether the NOMAD_ADDR string is in the user:pass@addr
-// format and if it is, it returns user, password and address. It returns "", "",
-// address otherwise.
-func handleBasicAuth(s string) (string, string, string) {
-	before, after, found := strings.Cut(s, "@")
-	if found {
-		user, pass, found := strings.Cut(before, ":")
-		if found {
-			return user, pass, after
-		}
+// removeBasicAuth removes userinfo from a URL and returns the adjusted URL.
+// The second return value is the userinfo that was removed.
+func removeBasicAuth(addr string) (string, *url.Userinfo) {
+	u, err := url.Parse(addr)
+	if err != nil {
+		return addr, nil
 	}
-	return "", "", before
+
+	userinfo := u.User
+	u.User = nil
+	return u.String(), userinfo
 }
 
 // clientOptsFromEnvironment populates api client conf with environment
 // variables present at the CLI's runtime.
 func clientOptsFromEnvironment(conf *api.Config) {
 	if v := os.Getenv("NOMAD_ADDR"); v != "" {
-		// we support user:pass@addr here
-		user, pass, addr := handleBasicAuth(v)
+		addr, userinfo := removeBasicAuth(v)
 		conf.Address = addr
-		if user != "" && pass != "" {
-			conf.HttpAuth.Username = user
-			conf.HttpAuth.Password = pass
+		if userinfo != nil {
+			pass, _ := userinfo.Password()
+			conf.HttpAuth = &api.HttpBasicAuth{
+				Username: userinfo.Username(),
+				Password: pass,
+			}
 		}
 	}
 	if v := os.Getenv("NOMAD_NAMESPACE"); v != "" {
@@ -468,12 +469,14 @@ func clientOptsFromEnvironment(conf *api.Config) {
 func clientOptsFromFlags(c *baseCommand, conf *api.Config) {
 	cfg := c.nomadConfig
 	if cfg.address != "" {
-		// we support user:pass@addr here
-		user, pass, addr := handleBasicAuth(cfg.address)
+		addr, userinfo := removeBasicAuth(cfg.address)
 		conf.Address = addr
-		if user != "" && pass != "" {
-			conf.HttpAuth.Username = user
-			conf.HttpAuth.Password = pass
+		if userinfo != nil {
+			pass, _ := userinfo.Password()
+			conf.HttpAuth = &api.HttpBasicAuth{
+				Username: userinfo.Username(),
+				Password: pass,
+			}
 		}
 	}
 	if cfg.namespace != "" {

internal/cli/helpers_test.go

@@ -0,0 +1,121 @@
+package cli
+
+import (
+	"net/url"
+	"testing"
+
+	"github.com/hashicorp/nomad/api"
+	"github.com/shoenig/test/must"
+)
+
+func TestHelpers_removeBasicAuth(t *testing.T) {
+	cases := []struct {
+		addr         string
+		expectedAddr string
+		expectedUser *url.Userinfo
+	}{
+		{"addr", "addr", nil},
+		{"addr:1337", "addr:1337", nil},
+		{"user:pass@addr", "user:pass@addr", nil},
+		{"user:pass@addr:1337", "user:pass@addr:1337", nil},
+		{"scheme://addr", "scheme://addr", nil},
+		{"foo@bar", "foo@bar", nil},
+		{"", "", nil},
+		{"scheme://user:pass@addr", "scheme://addr", url.UserPassword("user", "pass")},
+		{"scheme://user:pass@addr:1337", "scheme://addr:1337", url.UserPassword("user", "pass")},
+		{"scheme://user:@addr:1337", "scheme://addr:1337", url.UserPassword("user", "")},
+		{"scheme://:pass@addr:1337", "scheme://addr:1337", url.UserPassword("", "pass")},
+		{"//user:pass@addr:1337", "//addr:1337", url.UserPassword("user", "pass")},
+	}
+
+	for _, c := range cases {
+		t.Run(c.addr, func(t *testing.T) {
+			addr, userinfo := removeBasicAuth(c.addr)
+
+			must.Eq(t, c.expectedAddr, addr)
+			must.Eq(t, c.expectedUser, userinfo)
+		})
+	}
+}
+
+func TestHelpers_clientOptsFromEnvironment_Address(t *testing.T) {
+	cases := []struct {
+		addr             string
+		expectedAddress  string
+		expectedHttpAuth *api.HttpBasicAuth
+	}{
+		{
+			addr:             "addr",
+			expectedAddress:  "addr",
+			expectedHttpAuth: nil,
+		},
+		{
+			addr:             "scheme://user:pass@addr",
+			expectedAddress:  "scheme://addr",
+			expectedHttpAuth: &api.HttpBasicAuth{Username: "user", Password: "pass"},
+		},
+		{
+			addr:             "scheme://user:@addr",
+			expectedAddress:  "scheme://addr",
+			expectedHttpAuth: &api.HttpBasicAuth{Username: "user", Password: ""},
+		},
+		{
+			addr:             "scheme://:pass@addr",
+			expectedAddress:  "scheme://addr",
+			expectedHttpAuth: &api.HttpBasicAuth{Username: "", Password: "pass"},
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.addr, func(t *testing.T) {
+			t.Setenv("NOMAD_ADDR", c.addr)
+
+			conf := api.Config{HttpAuth: nil}
+			clientOptsFromEnvironment(&conf)
+
+			must.Eq(t, c.expectedAddress, conf.Address)
+			must.Eq(t, c.expectedHttpAuth, conf.HttpAuth)
+		})
+	}
+}
+
+func TestHelpers_clientOptsFromFlags_Address(t *testing.T) {
+	cases := []struct {
+		addr             string
+		expectedAddress  string
+		expectedHttpAuth *api.HttpBasicAuth
+	}{
+		{
+			addr:             "addr",
+			expectedAddress:  "addr",
+			expectedHttpAuth: nil,
+		},
+		{
+			addr:             "scheme://user:pass@addr",
+			expectedAddress:  "scheme://addr",
+			expectedHttpAuth: &api.HttpBasicAuth{Username: "user", Password: "pass"},
+		},
+		{
+			addr:             "scheme://user:@addr",
+			expectedAddress:  "scheme://addr",
+			expectedHttpAuth: &api.HttpBasicAuth{Username: "user", Password: ""},
+		},
+		{
+			addr:             "scheme://:pass@addr",
+			expectedAddress:  "scheme://addr",
+			expectedHttpAuth: &api.HttpBasicAuth{Username: "", Password: "pass"},
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.addr, func(t *testing.T) {
+			cmd := baseCommand{nomadConfig: nomadConfig{address: c.addr}}
+
+			conf := api.Config{HttpAuth: nil}
+			clientOptsFromFlags(&cmd, &conf)
+
+			must.Eq(t, c.expectedAddress, conf.Address)
+			must.Eq(t, c.expectedHttpAuth, conf.HttpAuth)
+		})
+	}
+}