Merge pull request #4690 from heyitsanthony/fix-etcdctl-fullroles

client: unmarshal user with full roles into user with role names

client/auth_user.go

@@ -36,6 +36,11 @@ type User struct {
 	Revoke   []string `json:"revoke,omitempty"`
 }
 
+type UserRoles struct {
+	User  string `json:"user"`
+	Roles []Role `json:"roles"`
+}
+
 func v2AuthURL(ep url.URL, action string, name string) *url.URL {
 	if name != "" {
 		ep.Path = path.Join(ep.Path, defaultV2AuthPrefix, action, name)
@@ -289,7 +294,14 @@ func (u *httpAuthUserAPI) modUser(ctx context.Context, req *authUserAPIAction) (
 	}
 	var user User
 	if err = json.Unmarshal(body, &user); err != nil {
-		return nil, err
+		var userR UserRoles
+		if urerr := json.Unmarshal(body, &userR); urerr != nil {
+			return nil, err
+		}
+		user.User = userR.User
+		for _, r := range userR.Roles {
+			user.Roles = append(user.Roles, r.Role)
+		}
 	}
 	return &user, nil
 }

e2e/etcdctl_test.go

@@ -29,9 +29,7 @@ func TestCtlV2SetTLS(t *testing.T)       { testCtlV2Set(t, &defaultConfigTLS, fa
 func testCtlV2Set(t *testing.T, cfg *etcdProcessClusterConfig, noSync bool) {
 	defer testutil.AfterTest(t)
 
-	if fileutil.Exist("../bin/etcdctl") == false {
-		t.Fatalf("could not find etcdctl binary")
-	}
+	mustEtcdctl(t)
 
 	epc, errC := newEtcdProcessCluster(cfg)
 	if errC != nil {
@@ -59,9 +57,7 @@ func TestCtlV2MkTLS(t *testing.T) { testCtlV2Mk(t, &defaultConfigTLS, false) }
 func testCtlV2Mk(t *testing.T, cfg *etcdProcessClusterConfig, noSync bool) {
 	defer testutil.AfterTest(t)
 
-	if fileutil.Exist("../bin/etcdctl") == false {
-		t.Fatalf("could not find etcdctl binary")
-	}
+	mustEtcdctl(t)
 
 	epc, errC := newEtcdProcessCluster(cfg)
 	if errC != nil {
@@ -92,9 +88,7 @@ func TestCtlV2RmTLS(t *testing.T) { testCtlV2Rm(t, &defaultConfigTLS, false) }
 func testCtlV2Rm(t *testing.T, cfg *etcdProcessClusterConfig, noSync bool) {
 	defer testutil.AfterTest(t)
 
-	if fileutil.Exist("../bin/etcdctl") == false {
-		t.Fatalf("could not find etcdctl binary")
-	}
+	mustEtcdctl(t)
 
 	epc, errC := newEtcdProcessCluster(cfg)
 	if errC != nil {
@@ -125,9 +119,7 @@ func TestCtlV2LsTLS(t *testing.T) { testCtlV2Ls(t, &defaultConfigTLS, false) }
 func testCtlV2Ls(t *testing.T, cfg *etcdProcessClusterConfig, noSync bool) {
 	defer testutil.AfterTest(t)
 
-	if fileutil.Exist("../bin/etcdctl") == false {
-		t.Fatalf("could not find etcdctl binary")
-	}
+	mustEtcdctl(t)
 
 	epc, errC := newEtcdProcessCluster(cfg)
 	if errC != nil {
@@ -157,9 +149,7 @@ func TestCtlV2WatchWithProxyNoSync(t *testing.T) { testCtlV2Watch(t, &defaultCon
 func testCtlV2Watch(t *testing.T, cfg *etcdProcessClusterConfig, noSync bool) {
 	defer testutil.AfterTest(t)
 
-	if fileutil.Exist("../bin/etcdctl") == false {
-		t.Fatalf("could not find etcdctl binary")
-	}
+	mustEtcdctl(t)
 
 	epc, errC := newEtcdProcessCluster(cfg)
 	if errC != nil {
@@ -187,6 +177,42 @@ func testCtlV2Watch(t *testing.T, cfg *etcdProcessClusterConfig, noSync bool) {
 	}
 }
 
+func TestCtlV2GetRoleUser(t *testing.T)          { testCtlV2GetRoleUser(t, &defaultConfig) }
+func TestCtlV2GetRoleUserWithProxy(t *testing.T) { testCtlV2GetRoleUser(t, &defaultConfigWithProxy) }
+
+func testCtlV2GetRoleUser(t *testing.T, cfg *etcdProcessClusterConfig) {
+	defer testutil.AfterTest(t)
+
+	mustEtcdctl(t)
+
+	epc, cerr := newEtcdProcessCluster(cfg)
+	if cerr != nil {
+		t.Fatalf("could not start etcd process cluster (%v)", cerr)
+	}
+	defer func() {
+		if err := epc.Close(); err != nil {
+			t.Fatalf("error closing etcd processes (%v)", err)
+		}
+	}()
+
+	// wait for the server capabilities to be updated based on the version;
+	// the update loop has a delay of 500ms, so 1s should be enough wait time
+	time.Sleep(time.Second)
+
+	if err := etcdctlAddRole(epc, "foo"); err != nil {
+		t.Fatalf("failed to add role (%v)", err)
+	}
+	if err := etcdctlUserAdd(epc, "username", "password"); err != nil {
+		t.Fatalf("failed to add user (%v)", err)
+	}
+	if err := etcdctlUserGrant(epc, "username", "foo"); err != nil {
+		t.Fatalf("failed to grant role (%v)", err)
+	}
+	if err := etcdctlUserGet(epc, "username"); err != nil {
+		t.Fatalf("failed to get user (%v)", err)
+	}
+}
+
 func etcdctlPrefixArgs(clus *etcdProcessCluster, noSync bool) []string {
 	endpoint := ""
 	if proxies := clus.proxies(); len(proxies) != 0 {
@@ -243,3 +269,29 @@ func etcdctlWatch(clus *etcdProcessCluster, key, value string, noSync bool) <-ch
 	}()
 	return errc
 }
+
+func etcdctlAddRole(clus *etcdProcessCluster, role string) error {
+	cmdArgs := append(etcdctlPrefixArgs(clus, false), "role", "add", role)
+	return spawnWithExpectedString(cmdArgs, role)
+}
+
+func etcdctlUserAdd(clus *etcdProcessCluster, user, pass string) error {
+	cmdArgs := append(etcdctlPrefixArgs(clus, false), "user", "add", user+":"+pass)
+	return spawnWithExpectedString(cmdArgs, "User "+user+" created")
+}
+
+func etcdctlUserGrant(clus *etcdProcessCluster, user, role string) error {
+	cmdArgs := append(etcdctlPrefixArgs(clus, false), "user", "grant", "--roles", role, user)
+	return spawnWithExpectedString(cmdArgs, "User "+user+" updated")
+}
+
+func etcdctlUserGet(clus *etcdProcessCluster, user string) error {
+	cmdArgs := append(etcdctlPrefixArgs(clus, false), "user", "get", user)
+	return spawnWithExpectedString(cmdArgs, "User: "+user)
+}
+
+func mustEtcdctl(t *testing.T) {
+	if !fileutil.Exist("../bin/etcdctl") {
+		t.Fatalf("could not find etcdctl binary")
+	}
+}

etcdctl/command/user_commands.go

@@ -109,15 +109,17 @@ func actionUserList(c *cli.Context) {
 }
 
 func actionUserAdd(c *cli.Context) {
-	api, user := mustUserAPIAndName(c)
+	api, userarg := mustUserAPIAndName(c)
 	ctx, cancel := contextWithTotalTimeout(c)
 	defer cancel()
+	user, _, _ := getUsernamePassword("", userarg+":")
 	currentUser, err := api.GetUser(ctx, user)
 	if currentUser != nil {
 		fmt.Fprintf(os.Stderr, "User %s already exists\n", user)
 		os.Exit(1)
 	}
-	pass, err := speakeasy.Ask("New password: ")
+
+	_, pass, err := getUsernamePassword("New password: ", userarg)
 	if err != nil {
 		fmt.Fprintln(os.Stderr, "Error reading password:", err)
 		os.Exit(1)

etcdctl/command/util.go

@@ -172,11 +172,15 @@ func getTransport(c *cli.Context) (*http.Transport, error) {
 }
 
 func getUsernamePasswordFromFlag(usernameFlag string) (username string, password string, err error) {
+	return getUsernamePassword("Password: ", usernameFlag)
+}
+
+func getUsernamePassword(prompt, usernameFlag string) (username string, password string, err error) {
 	colon := strings.Index(usernameFlag, ":")
 	if colon == -1 {
 		username = usernameFlag
 		// Prompt for the password.
-		password, err = speakeasy.Ask("Password: ")
+		password, err = speakeasy.Ask(prompt)
 		if err != nil {
 			return "", "", err
 		}