Merge pull request #5640 from xiang90/permcheck

auth: clean permission checking

auth/range_perm_cache.go

@@ -114,46 +114,47 @@ func getMergedPerms(tx backend.BatchTx, userName string) *unifiedRangePermission
 	}
 }
 
-func checkCachedPerm(cachedPerms *unifiedRangePermissions, userName string, key, rangeEnd string, write, read bool) bool {
-	var perms []*rangePerm
-
-	if write {
-		perms = cachedPerms.writePerms
-	} else {
-		perms = cachedPerms.readPerms
+func checkKeyPerm(cachedPerms *unifiedRangePermissions, key, rangeEnd string, permtyp authpb.Permission_Type) bool {
+	var tocheck []*rangePerm
+
+	switch permtyp {
+	case authpb.READ:
+		tocheck = cachedPerms.readPerms
+	case authpb.WRITE:
+		tocheck = cachedPerms.writePerms
+	default:
+		plog.Panicf("unknown auth type: %v", permtyp)
 	}
 
-	for _, perm := range perms {
-		if strings.Compare(rangeEnd, "") != 0 {
+	for _, perm := range tocheck {
+		// check permission of a single key
+		if len(rangeEnd) == 0 {
 			if strings.Compare(perm.begin, key) <= 0 && strings.Compare(rangeEnd, perm.end) <= 0 {
 				return true
 			}
-		} else {
-			if strings.Compare(perm.begin, key) <= 0 && strings.Compare(key, perm.end) <= 0 {
-				return true
-			}
+		}
+
+		if strings.Compare(perm.begin, key) <= 0 && strings.Compare(perm.end, key) >= 0 {
+			return true
 		}
 	}
 
 	return false
 }
 
-func (as *authStore) isRangeOpPermitted(tx backend.BatchTx, userName string, key, rangeEnd string, write, read bool) bool {
+func (as *authStore) isRangeOpPermitted(tx backend.BatchTx, userName string, key, rangeEnd string, permtyp authpb.Permission_Type) bool {
 	// assumption: tx is Lock()ed
 	_, ok := as.rangePermCache[userName]
-	if ok {
-		return checkCachedPerm(as.rangePermCache[userName], userName, key, rangeEnd, write, read)
-	}
-
-	perms := getMergedPerms(tx, userName)
-	if perms == nil {
-		plog.Errorf("failed to create a unified permission of user %s", userName)
-		return false
+	if !ok {
+		perms := getMergedPerms(tx, userName)
+		if perms == nil {
+			plog.Errorf("failed to create a unified permission of user %s", userName)
+			return false
+		}
+		as.rangePermCache[userName] = perms
 	}
-	as.rangePermCache[userName] = perms
-
-	return checkCachedPerm(as.rangePermCache[userName], userName, key, rangeEnd, write, read)
 
+	return checkKeyPerm(as.rangePermCache[userName], key, rangeEnd, permtyp)
 }
 
 func (as *authStore) clearCachedPerm() {

auth/store.go

@@ -544,7 +544,7 @@ func (as *authStore) RoleGrantPermission(r *pb.AuthRoleGrantPermissionRequest) (
 	return &pb.AuthRoleGrantPermissionResponse{}, nil
 }
 
-func (as *authStore) isOpPermitted(userName string, key, rangeEnd string, write bool, read bool) bool {
+func (as *authStore) isOpPermitted(userName string, key, rangeEnd string, permTyp authpb.Permission_Type) bool {
 	// TODO(mitake): this function would be costly so we need a caching mechanism
 	if !as.isAuthEnabled() {
 		return true
@@ -576,18 +576,14 @@ func (as *authStore) isOpPermitted(userName string, key, rangeEnd string, write
 					return true
 				}
 
-				if write && !read && perm.PermType == authpb.WRITE {
-					return true
-				}
-
-				if read && !write && perm.PermType == authpb.READ {
+				if permTyp == perm.PermType {
 					return true
 				}
 			}
 		}
 	}
 
-	if as.isRangeOpPermitted(tx, userName, key, rangeEnd, write, read) {
+	if as.isRangeOpPermitted(tx, userName, key, rangeEnd, permTyp) {
 		return true
 	}
 
@@ -595,11 +591,11 @@ func (as *authStore) isOpPermitted(userName string, key, rangeEnd string, write
 }
 
 func (as *authStore) IsPutPermitted(header *pb.RequestHeader, key string) bool {
-	return as.isOpPermitted(header.Username, key, "", true, false)
+	return as.isOpPermitted(header.Username, key, "", authpb.WRITE)
 }
 
 func (as *authStore) IsRangePermitted(header *pb.RequestHeader, key, rangeEnd string) bool {
-	return as.isOpPermitted(header.Username, key, rangeEnd, false, true)
+	return as.isOpPermitted(header.Username, key, rangeEnd, authpb.READ)
 }
 
 func (as *authStore) IsAdminPermitted(username string) bool {