From c8628c8fe5e7152b73370a3e5776e97998bacde7 Mon Sep 17 00:00:00 2001
From: Xiang Li <xiangli.cs@gmail.com>
Date: Tue, 23 Jun 2015 13:15:28 -0700
Subject: [PATCH] auth: separate the role create and update path

Giving both permission and grant/revoke is not allowed.
Creating an existing role is not allowed.
Updating a non-existing is not allowed.
---
 etcdserver/auth/auth.go            | 14 ++++----------
 etcdserver/etcdhttp/client_auth.go | 29 ++++++++++++++++++++++-------
 2 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/etcdserver/auth/auth.go b/etcdserver/auth/auth.go
index da0f04f88..eacfb5ee1 100644
--- a/etcdserver/auth/auth.go
+++ b/etcdserver/auth/auth.go
@@ -97,6 +97,10 @@ type Permissions struct {
 	KV rwPermission `json:"kv"`
 }
 
+func (p *Permissions) IsEmpty() bool {
+	return p == nil || (len(p.KV.Read) == 0 && len(p.KV.Write) == 0)
+}
+
 type rwPermission struct {
 	Read  []string `json:"read"`
 	Write []string `json:"write"`
@@ -297,16 +301,6 @@ func (s *Store) GetRole(name string) (Role, error) {
 	return r, nil
 }
 
-func (s *Store) CreateOrUpdateRole(r Role) (role Role, created bool, err error) {
-	_, err = s.GetRole(r.Role)
-	if err == nil {
-		role, err = s.UpdateRole(r)
-		created = false
-		return
-	}
-	return r, true, s.CreateRole(r)
-}
-
 func (s *Store) CreateRole(role Role) error {
 	if role.Role == RootRoleName {
 		return authErr(http.StatusForbidden, "Cannot modify role %s: is root role.", role.Role)
diff --git a/etcdserver/etcdhttp/client_auth.go b/etcdserver/etcdhttp/client_auth.go
index e871d1431..a5b2a5809 100644
--- a/etcdserver/etcdhttp/client_auth.go
+++ b/etcdserver/etcdhttp/client_auth.go
@@ -211,17 +211,32 @@ func (sh *authHandler) forRole(w http.ResponseWriter, r *http.Request, role stri
 			writeError(w, httptypes.NewHTTPError(http.StatusBadRequest, "Role JSON name does not match the name in the URL"))
 			return
 		}
-		newrole, created, err := sh.sec.CreateOrUpdateRole(in)
-		if err != nil {
-			writeError(w, err)
-			return
-		}
-		if created {
+
+		var out auth.Role
+
+		// create
+		if in.Grant.IsEmpty() && in.Revoke.IsEmpty() {
+			err = sh.sec.CreateRole(in)
+			if err != nil {
+				writeError(w, err)
+				return
+			}
 			w.WriteHeader(http.StatusCreated)
+			out = in
 		} else {
+			if !in.Permissions.IsEmpty() {
+				writeError(w, httptypes.NewHTTPError(http.StatusBadRequest, "Role JSON contains both permissions and grant/revoke"))
+				return
+			}
+			out, err = sh.sec.UpdateRole(in)
+			if err != nil {
+				writeError(w, err)
+				return
+			}
 			w.WriteHeader(http.StatusOK)
 		}
-		err = json.NewEncoder(w).Encode(newrole)
+
+		err = json.NewEncoder(w).Encode(out)
 		if err != nil {
 			plog.Warningf("forRole error encoding on %s", r.URL)
 			return