From 9be65414eb0ce7cba390a5e7231e0ff2b7ce0917 Mon Sep 17 00:00:00 2001
From: Xiang Li <xiangli.cs@gmail.com>
Date: Mon, 13 Jun 2016 12:57:14 -0700
Subject: [PATCH 1/3] auth: add key support in merge func

---
 auth/range_perm_cache.go      | 26 ++++++++++++++++----------
 auth/range_perm_cache_test.go | 24 +++++++++++++++++++++++-
 2 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/auth/range_perm_cache.go b/auth/range_perm_cache.go
index 613deecbf..b205544b7 100644
--- a/auth/range_perm_cache.go
+++ b/auth/range_perm_cache.go
@@ -22,9 +22,20 @@ import (
 	"github.com/coreos/etcd/mvcc/backend"
 )
 
+// isSubset returns true if a is a subset of b
 func isSubset(a, b *rangePerm) bool {
-	// return true if a is a subset of b
-	return 0 <= bytes.Compare(a.begin, b.begin) && bytes.Compare(a.end, b.end) <= 0
+	switch {
+	case len(a.end) == 0 && len(b.end) == 0:
+		// a, b are both keys
+		return bytes.Compare(a.begin, b.begin) == 0
+	case len(b.end) == 0:
+		// b is a key, a is a range
+		return false
+	case len(a.end) == 0:
+		return 0 <= bytes.Compare(a.begin, b.begin) && bytes.Compare(a.begin, b.end) <= 0
+	default:
+		return 0 <= bytes.Compare(a.begin, b.begin) && bytes.Compare(a.end, b.end) <= 0
+	}
 }
 
 // removeSubsetRangePerms removes any rangePerms that are subsets of other rangePerms.
@@ -126,15 +137,10 @@ func checkKeyPerm(cachedPerms *unifiedRangePermissions, key, rangeEnd []byte, pe
 		plog.Panicf("unknown auth type: %v", permtyp)
 	}
 
-	for _, perm := range tocheck {
-		// check permission of a single key
-		if len(rangeEnd) == 0 {
-			if bytes.Compare(perm.begin, key) <= 0 && bytes.Compare(rangeEnd, perm.end) <= 0 {
-				return true
-			}
-		}
+	requiredPerm := &rangePerm{begin: key, end: rangeEnd}
 
-		if bytes.Compare(perm.begin, key) <= 0 && bytes.Compare(perm.end, key) >= 0 {
+	for _, perm := range tocheck {
+		if isSubset(requiredPerm, perm) {
 			return true
 		}
 	}
diff --git a/auth/range_perm_cache_test.go b/auth/range_perm_cache_test.go
index 2df3e6848..7f7dff6e3 100644
--- a/auth/range_perm_cache_test.go
+++ b/auth/range_perm_cache_test.go
@@ -37,7 +37,7 @@ func isPermsEqual(a, b []*rangePerm) bool {
 	return true
 }
 
-func TestUnifyParams(t *testing.T) {
+func TestGetMergedPerms(t *testing.T) {
 	tests := []struct {
 		params []*rangePerm
 		want   []*rangePerm
@@ -86,6 +86,28 @@ func TestUnifyParams(t *testing.T) {
 			[]*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("b"), []byte("c")}, {[]byte("c"), []byte("d")}, {[]byte("d"), []byte("f")}, {[]byte("1"), []byte("9")}},
 			[]*rangePerm{{[]byte("1"), []byte("9")}, {[]byte("a"), []byte("f")}},
 		},
+		// overlapping
+		{
+			[]*rangePerm{{[]byte("a"), []byte("f")}, {[]byte("b"), []byte("g")}},
+			[]*rangePerm{{[]byte("a"), []byte("g")}},
+		},
+		// keys
+		{
+			[]*rangePerm{{[]byte("a"), []byte("")}, {[]byte("b"), []byte("")}},
+			[]*rangePerm{{[]byte("a"), []byte("")}, {[]byte("b"), []byte("")}},
+		},
+		{
+			[]*rangePerm{{[]byte("a"), []byte("")}, {[]byte("a"), []byte("c")}},
+			[]*rangePerm{{[]byte("a"), []byte("c")}},
+		},
+		{
+			[]*rangePerm{{[]byte("a"), []byte("")}, {[]byte("a"), []byte("c")}, {[]byte("b"), []byte("")}},
+			[]*rangePerm{{[]byte("a"), []byte("c")}},
+		},
+		{
+			[]*rangePerm{{[]byte("a"), []byte("")}, {[]byte("b"), []byte("c")}, {[]byte("b"), []byte("")}, {[]byte("c"), []byte("")}, {[]byte("d"), []byte("")}},
+			[]*rangePerm{{[]byte("a"), []byte("")}, {[]byte("b"), []byte("c")}, {[]byte("d"), []byte("")}},
+		},
 	}
 
 	for i, tt := range tests {

From 390c89b7f935529bc6598dfc5df38c44e25fe336 Mon Sep 17 00:00:00 2001
From: Xiang Li <xiangli.cs@gmail.com>
Date: Mon, 13 Jun 2016 13:18:22 -0700
Subject: [PATCH 2/3] auth: remove the special checking case for key auth

---
 auth/range_perm_cache.go |  3 ---
 auth/store.go            | 23 -----------------------
 2 files changed, 26 deletions(-)

diff --git a/auth/range_perm_cache.go b/auth/range_perm_cache.go
index b205544b7..2cf47ee75 100644
--- a/auth/range_perm_cache.go
+++ b/auth/range_perm_cache.go
@@ -100,9 +100,6 @@ func getMergedPerms(tx backend.BatchTx, userName string) *unifiedRangePermission
 		}
 
 		for _, perm := range role.KeyPermission {
-			if len(perm.RangeEnd) == 0 {
-				continue
-			}
 			rp := &rangePerm{begin: perm.Key, end: perm.RangeEnd}
 
 			switch perm.PermType {
diff --git a/auth/store.go b/auth/store.go
index 653fd78b7..62feed053 100644
--- a/auth/store.go
+++ b/auth/store.go
@@ -560,29 +560,6 @@ func (as *authStore) isOpPermitted(userName string, key, rangeEnd []byte, permTy
 		return false
 	}
 
-	if len(rangeEnd) == 0 {
-		for _, roleName := range user.Roles {
-			role := getRole(tx, roleName)
-			if role == nil {
-				continue
-			}
-
-			for _, perm := range role.KeyPermission {
-				if !bytes.Equal(perm.Key, key) {
-					continue
-				}
-
-				if perm.PermType == authpb.READWRITE {
-					return true
-				}
-
-				if permTyp == perm.PermType {
-					return true
-				}
-			}
-		}
-	}
-
 	if as.isRangeOpPermitted(tx, userName, key, rangeEnd, permTyp) {
 		return true
 	}

From 38546a9d246275a5fc7d6a746095c963d1b6fbf3 Mon Sep 17 00:00:00 2001
From: Xiang Li <xiangli.cs@gmail.com>
Date: Mon, 13 Jun 2016 15:42:49 -0700
Subject: [PATCH 3/3] auth: use bytes equal when possible

---
 auth/range_perm_cache.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/auth/range_perm_cache.go b/auth/range_perm_cache.go
index 2cf47ee75..a3b2ebea9 100644
--- a/auth/range_perm_cache.go
+++ b/auth/range_perm_cache.go
@@ -27,7 +27,7 @@ func isSubset(a, b *rangePerm) bool {
 	switch {
 	case len(a.end) == 0 && len(b.end) == 0:
 		// a, b are both keys
-		return bytes.Compare(a.begin, b.begin) == 0
+		return bytes.Equal(a.begin, b.begin)
 	case len(b.end) == 0:
 		// b is a key, a is a range
 		return false