auth: invalid for empty password

Fix #5182.

e2e/ctl_v2_test.go

@@ -158,7 +158,6 @@ func testCtlV2Watch(t *testing.T, cfg *etcdProcessClusterConfig, noSync bool) {
 
 func TestCtlV2GetRoleUser(t *testing.T)          { testCtlV2GetRoleUser(t, &configNoTLS) }
 func TestCtlV2GetRoleUserWithProxy(t *testing.T) { testCtlV2GetRoleUser(t, &configWithProxy) }
-
 func testCtlV2GetRoleUser(t *testing.T, cfg *etcdProcessClusterConfig) {
 	defer testutil.AfterTest(t)
 
@@ -181,6 +180,7 @@ func testCtlV2GetRoleUser(t *testing.T, cfg *etcdProcessClusterConfig) {
 	if err := etcdctlUserGet(epc, "username"); err != nil {
 		t.Fatalf("failed to get user (%v)", err)
 	}
+
 	// ensure double grant gives an error; was crashing in 2.3.1
 	regrantArgs := etcdctlPrefixArgs(epc)
 	regrantArgs = append(regrantArgs, "user", "grant", "--roles", "foo", "username")
@@ -191,7 +191,6 @@ func testCtlV2GetRoleUser(t *testing.T, cfg *etcdProcessClusterConfig) {
 
 func TestCtlV2UserListUsername(t *testing.T) { testCtlV2UserList(t, "username") }
 func TestCtlV2UserListRoot(t *testing.T)     { testCtlV2UserList(t, "root") }
-
 func testCtlV2UserList(t *testing.T, username string) {
 	defer testutil.AfterTest(t)
 
@@ -325,6 +324,11 @@ func etcdctlUserList(clus *etcdProcessCluster, expectedUser string) error {
 	return spawnWithExpect(cmdArgs, expectedUser)
 }
 
+func etcdctlAuthEnable(clus *etcdProcessCluster) error {
+	cmdArgs := append(etcdctlPrefixArgs(clus), "auth", "enable")
+	return spawnWithExpect(cmdArgs, "Authentication Enabled")
+}
+
 func mustEtcdctl(t *testing.T) {
 	if !fileutil.Exist("../bin/etcdctl") {
 		t.Fatalf("could not find etcdctl binary")

e2e/v2_curl_test.go

@@ -15,7 +15,9 @@
 package e2e
 
 import (
+	"fmt"
 	"math/rand"
+	"strings"
 	"testing"
 
 	"github.com/coreos/etcd/pkg/testutil"
@@ -30,7 +32,6 @@ func TestV2CurlProxyNoTLS(t *testing.T)   { testCurlPutGet(t, &configWithProxy)
 func TestV2CurlProxyTLS(t *testing.T)     { testCurlPutGet(t, &configWithProxyTLS) }
 func TestV2CurlProxyPeerTLS(t *testing.T) { testCurlPutGet(t, &configWithProxyPeerTLS) }
 func TestV2CurlClientBoth(t *testing.T)   { testCurlPutGet(t, &configClientBoth) }
-
 func testCurlPutGet(t *testing.T, cfg *etcdProcessClusterConfig) {
 	defer testutil.AfterTest(t)
 
@@ -52,62 +53,117 @@ func testCurlPutGet(t *testing.T, cfg *etcdProcessClusterConfig) {
 	expectGet := `{"action":"get","node":{"key":"/testKey","value":"foo","`
 
 	if cfg.clientTLS == clientTLSAndNonTLS {
-		if err := cURLPut(epc, "testKey", "foo", expectPut); err != nil {
+		if err := cURLPut(epc, "/v2/keys", "testKey", "foo", expectPut, "", "", false); err != nil {
 			t.Fatalf("failed put with curl (%v)", err)
 		}
 
-		if err := cURLGet(epc, "testKey", expectGet); err != nil {
+		if err := cURLGet(epc, "/v2/keys", "testKey", expectGet, "", ""); err != nil {
 			t.Fatalf("failed get with curl (%v)", err)
 		}
-		if err := cURLGetUseTLS(epc, "testKey", expectGet); err != nil {
+		if err := cURLGetUseTLS(epc, "/v2/keys", "testKey", expectGet, "", ""); err != nil {
 			t.Fatalf("failed get with curl (%v)", err)
 		}
 	} else {
-		if err := cURLPut(epc, "testKey", "foo", expectPut); err != nil {
+		if err := cURLPut(epc, "/v2/keys", "testKey", "foo", expectPut, "", "", false); err != nil {
 			t.Fatalf("failed put with curl (%v)", err)
 		}
 
-		if err := cURLGet(epc, "testKey", expectGet); err != nil {
+		if err := cURLGet(epc, "/v2/keys", "testKey", expectGet, "", ""); err != nil {
 			t.Fatalf("failed get with curl (%v)", err)
 		}
 	}
 }
 
+func TestV2CurlIssue5182(t *testing.T) {
+	defer testutil.AfterTest(t)
+
+	epc := setupEtcdctlTest(t, &configNoTLS, false)
+	defer func() {
+		if err := epc.Close(); err != nil {
+			t.Fatalf("error closing etcd processes (%v)", err)
+		}
+	}()
+
+	if err := cURLPut(epc, "/v2/keys", "foo", "bar", `{"action":"set","node":{"key":"/foo","value":"bar","`, "", "", false); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := cURLPut(epc, "/v2/auth/users", "foo", `{"user":"foo", "password":"foo"}`, `{"user":"foo","roles":null}`, "", "", true); err != nil {
+		t.Fatal(err)
+	}
+	if err := cURLPut(epc, "/v2/auth/roles", "foo", `{"role":"foo", "permissions": {"kv": {"read": ["/foo/*"]}}}`, `{"role":"foo","permissions":{"kv":{"read":["/foo/*"],"write":null}}`, "", "", true); err != nil {
+		t.Fatal(err)
+	}
+	if err := cURLPut(epc, "/v2/auth/users", "foo", `{"user": "foo", "grant": ["foo"]}`, `{"user":"foo","roles":["foo"]}`, "", "", true); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := etcdctlUserAdd(epc, "root", "a"); err != nil {
+		t.Fatal(err)
+	}
+	if err := etcdctlAuthEnable(epc); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := cURLGet(epc, "/v2/keys", "foo", "bar", "root", "a"); err != nil {
+		t.Fatal(err)
+	}
+	if err := cURLGet(epc, "/v2/keys", "foo", "bar", "foo", ""); err != nil {
+		if !strings.Contains(err.Error(), `The request requires user authentication`) {
+			t.Fatalf("expected 'The request requires user authentication' error, got %v", err)
+		}
+	} else {
+		t.Fatalf("expected 'The request requires user authentication' error")
+	}
+}
+
 // cURLPrefixArgs builds the beginning of a curl command for a given key
 // addressed to a random URL in the given cluster.
-func cURLPrefixArgs(clus *etcdProcessCluster, key string) []string {
+func cURLPrefixArgs(clus *etcdProcessCluster, prefix, key, user, passwd string) []string {
 	cmdArgs := []string{"curl"}
 	acurl := clus.procs[rand.Intn(clus.cfg.clusterSize)].cfg.acurl
 
 	if clus.cfg.clientTLS == clientTLS {
 		cmdArgs = append(cmdArgs, "--cacert", caPath, "--cert", certPath, "--key", privateKeyPath)
 	}
-	keyURL := acurl + "/v2/keys/testKey"
-	cmdArgs = append(cmdArgs, "-L", keyURL)
+	ep := acurl + fmt.Sprintf("%s/%s", prefix, key)
+	if user != "" || passwd != "" {
+		cmdArgs = append(cmdArgs, "-L", "-u", fmt.Sprintf("%s:%s", user, passwd), ep)
+	} else {
+		cmdArgs = append(cmdArgs, "-L", ep)
+	}
 	return cmdArgs
 }
 
-func cURLPrefixArgsUseTLS(clus *etcdProcessCluster, key string) []string {
+func cURLPrefixArgsUseTLS(clus *etcdProcessCluster, prefix, key, user, passwd string) []string {
 	cmdArgs := []string{"curl"}
 	if clus.cfg.clientTLS != clientTLSAndNonTLS {
 		panic("should not use cURLPrefixArgsUseTLS when serving only TLS or non-TLS")
 	}
 	cmdArgs = append(cmdArgs, "--cacert", caPath, "--cert", certPath, "--key", privateKeyPath)
 	acurl := clus.procs[rand.Intn(clus.cfg.clusterSize)].cfg.acurltls
-	keyURL := acurl + "/v2/keys/testKey"
-	cmdArgs = append(cmdArgs, "-L", keyURL)
+	ep := acurl + fmt.Sprintf("%s/%s", prefix, key)
+	if user != "" || passwd != "" {
+		cmdArgs = append(cmdArgs, "-L", "-u", fmt.Sprintf("%s:%s", user, passwd), ep)
+	} else {
+		cmdArgs = append(cmdArgs, "-L", ep)
+	}
 	return cmdArgs
 }
 
-func cURLPut(clus *etcdProcessCluster, key, val, expected string) error {
-	args := append(cURLPrefixArgs(clus, key), "-XPUT", "-d", "value="+val)
+func cURLPut(clus *etcdProcessCluster, prefix, key, val, expected, user, passwd string, isJSON bool) error {
+	vs := val
+	if !isJSON {
+		vs = "value=" + val
+	}
+	args := append(cURLPrefixArgs(clus, prefix, key, user, passwd), "-XPUT", "-d", vs)
 	return spawnWithExpect(args, expected)
 }
 
-func cURLGet(clus *etcdProcessCluster, key, expected string) error {
-	return spawnWithExpect(cURLPrefixArgs(clus, key), expected)
+func cURLGet(clus *etcdProcessCluster, prefix, key, expected, user, passwd string) error {
+	return spawnWithExpect(cURLPrefixArgs(clus, prefix, key, user, passwd), expected)
 }
 
-func cURLGetUseTLS(clus *etcdProcessCluster, key, expected string) error {
-	return spawnWithExpect(cURLPrefixArgsUseTLS(clus, key), expected)
+func cURLGetUseTLS(clus *etcdProcessCluster, prefix, key, expected, user, passwd string) error {
+	return spawnWithExpect(cURLPrefixArgsUseTLS(clus, prefix, key, user, passwd), expected)
 }

etcdserver/api/v2http/client_auth.go

@@ -90,8 +90,7 @@ func hasKeyPrefixAccess(sec auth.Store, r *http.Request, key string, recursive b
 		plog.Warningf("auth: no such user: %s.", username)
 		return false
 	}
-	authAsUser := sec.CheckPassword(user, password)
-	if !authAsUser {
+	if (user.Password != "" && password == "") || !sec.CheckPassword(user, password) {
 		plog.Warningf("auth: incorrect password for user: %s.", username)
 		return false
 	}