From e291dfd74883286e0feb6c136d9bf99d92a44f71 Mon Sep 17 00:00:00 2001
From: Xiang Li <xiangli.cs@gmail.com>
Date: Tue, 23 Jun 2015 14:35:10 -0700
Subject: [PATCH] etcdhttp: improve user endpoint validation

Giving both roles and grant/revoke is not allowed.
Creating an existing user is not allowed.
Updating a non-existing user is not allowed.
---
 etcdserver/auth/auth.go            |  6 ++---
 etcdserver/etcdhttp/client_auth.go | 41 +++++++++++++++++++++++++-----
 2 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/etcdserver/auth/auth.go b/etcdserver/auth/auth.go
index eacfb5ee1..4885b2a07 100644
--- a/etcdserver/auth/auth.go
+++ b/etcdserver/auth/auth.go
@@ -177,12 +177,12 @@ func (s *Store) GetUser(name string) (User, error) {
 	return u, nil
 }
 
+// CreateOrUpdateUser should be only used for creating the new user or when you are not
+// sure if it is a create or update. (When only password is passed in, we are not sure
+// if it is a update or create)
 func (s *Store) CreateOrUpdateUser(user User) (out User, created bool, err error) {
 	_, err = s.GetUser(user.User)
 	if err == nil {
-		// Remove the update-user roles from updating downstream.
-		// Roles are granted or revoked, not changed directly.
-		user.Roles = nil
 		out, err := s.UpdateUser(user)
 		return out, false, err
 	}
diff --git a/etcdserver/etcdhttp/client_auth.go b/etcdserver/etcdhttp/client_auth.go
index a5b2a5809..d74f29e46 100644
--- a/etcdserver/etcdhttp/client_auth.go
+++ b/etcdserver/etcdhttp/client_auth.go
@@ -333,19 +333,48 @@ func (sh *authHandler) forUser(w http.ResponseWriter, r *http.Request, user stri
 			writeError(w, httptypes.NewHTTPError(http.StatusBadRequest, "User JSON name does not match the name in the URL"))
 			return
 		}
-		newuser, created, err := sh.sec.CreateOrUpdateUser(u)
-		if err != nil {
-			writeError(w, err)
-			return
+
+		var (
+			out     auth.User
+			created bool
+		)
+
+		if len(u.Grant) == 0 && len(u.Revoke) == 0 {
+			// create or update
+			if len(u.Roles) != 0 {
+				out, err = sh.sec.CreateUser(u)
+			} else {
+				// if user passes in both password and roles, we are unsure about his/her
+				// intention.
+				out, created, err = sh.sec.CreateOrUpdateUser(u)
+			}
+
+			if err != nil {
+				writeError(w, err)
+				return
+			}
+		} else {
+			// update case
+			if len(u.Roles) != 0 {
+				writeError(w, httptypes.NewHTTPError(http.StatusBadRequest, "User JSON contains both roles and grant/revoke"))
+				return
+			}
+			out, err = sh.sec.UpdateUser(u)
+			if err != nil {
+				writeError(w, err)
+				return
+			}
 		}
-		newuser.Password = ""
 
 		if created {
 			w.WriteHeader(http.StatusCreated)
 		} else {
 			w.WriteHeader(http.StatusOK)
 		}
-		err = json.NewEncoder(w).Encode(newuser)
+
+		out.Password = ""
+
+		err = json.NewEncoder(w).Encode(out)
 		if err != nil {
 			plog.Warningf("forUser error encoding on %s", r.URL)
 			return