Only allow 'list' policies within 'key' policy definitions. Consolidated two similar tests into one and fixed alignment.

acl/policy.go

@@ -119,8 +119,6 @@ func isPolicyValid(policy string) bool {
 		return true
 	case PolicyWrite:
 		return true
-	case PolicyList:
-		return true
 	default:
 		return false
 	}
@@ -178,7 +176,7 @@ func Parse(rules string, sentinel sentinel.Evaluator) (*Policy, error) {
 
 	// Validate the key policy
 	for _, kp := range p.Keys {
-		if !isPolicyValid(kp.Policy) {
+		if kp.Policy != PolicyList && !isPolicyValid(kp.Policy) {
 			return nil, fmt.Errorf("Invalid key policy: %#v", kp)
 		}
 		if err := isSentinelValid(sentinel, kp.Policy, kp.Sentinel); err != nil {

agent/consul/kvs_endpoint_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/hashicorp/consul/api"
 	"github.com/hashicorp/consul/testrpc"
 	"github.com/hashicorp/net-rpc-msgpackrpc"
+	"github.com/pascaldekloe/goe/verify"
 )
 
 func TestKVS_Apply(t *testing.T) {
@@ -541,46 +542,64 @@ key "zip" {
 	id := out
 
 	//recursive read on empty prefix should fail
-	getR := structs.KeyRequest{
+	getReq := structs.KeyRequest{
 		Datacenter:   "dc1",
 		Key:          "",
 		QueryOptions: structs.QueryOptions{Token: id},
 	}
 	var dirent structs.IndexedDirEntries
-	if err := msgpackrpc.CallWithCodec(codec, "KVS.List", &getR, &dirent); !acl.IsErrPermissionDenied(err) {
+	if err := msgpackrpc.CallWithCodec(codec, "KVS.List", &getReq, &dirent); !acl.IsErrPermissionDenied(err) {
+		t.Fatalf("expected %v but got err: %v", acl.ErrPermissionDenied, err)
+	}
+
+	//listing keys on empty prefix should fail
+	getKeysReq := structs.KeyListRequest{
+		Datacenter:   "dc1",
+		Prefix:       "",
+		Seperator:    "/",
+		QueryOptions: structs.QueryOptions{Token: id},
+	}
+	var keyList structs.IndexedKeyList
+	if err := msgpackrpc.CallWithCodec(codec, "KVS.ListKeys", &getKeysReq, &keyList); !acl.IsErrPermissionDenied(err) {
 		t.Fatalf("expected %v but got err: %v", acl.ErrPermissionDenied, err)
 	}
 
 	// recursive read with a prefix that has list permissions should succeed
-	getR2 := structs.KeyRequest{
+	getReq2 := structs.KeyRequest{
 		Datacenter:   "dc1",
 		Key:          "bar",
 		QueryOptions: structs.QueryOptions{Token: id},
 	}
-	if err := msgpackrpc.CallWithCodec(codec, "KVS.List", &getR2, &dirent); err != nil {
+	if err := msgpackrpc.CallWithCodec(codec, "KVS.List", &getReq2, &dirent); err != nil {
 		t.Fatalf("err: %v", err)
 	}
 
-	if dirent.Index == 0 {
+	expectedKeys := []string{"bar/bar1", "bar/bar2"}
-		t.Fatalf("Bad: %v", dirent)
+	var actualKeys []string
+	for _, entry := range dirent.Entries {
+		actualKeys = append(actualKeys, entry.Key)
 	}
-	if len(dirent.Entries) != 2 {
+
-		t.Fatalf("Bad: %v", dirent.Entries)
+	verify.Values(t, "", actualKeys, expectedKeys)
-	}
+
-	for i := 0; i < len(dirent.Entries); i++ {
+	// list keys with a prefix that has list permissions should succeed
-		d := dirent.Entries[i]
+	getKeysReq2 := structs.KeyListRequest{
-		switch i {
+		Datacenter:   "dc1",
-		case 0:
+		Prefix:       "bar",
-			if d.Key != "bar/bar1" {
+		QueryOptions: structs.QueryOptions{Token: id},
-				t.Fatalf("bad key %v", d.Key)
-			}
-		case 1:
-			if d.Key != "bar/bar2" {
-				t.Fatalf("bad key %v", d.Key)
 	}
+	if err := msgpackrpc.CallWithCodec(codec, "KVS.ListKeys", &getKeysReq2, &keyList); err != nil {
+		t.Fatalf("err: %v", err)
 	}
+
+	actualKeys = []string{}
+
+	for _, key := range keyList.Keys {
+		actualKeys = append(actualKeys, key)
 	}
 
+	verify.Values(t, "", actualKeys, expectedKeys)
+
 }
 
 func TestKVSEndpoint_ListKeys(t *testing.T) {
@@ -732,109 +751,6 @@ func TestKVSEndpoint_ListKeys_ACLDeny(t *testing.T) {
 	}
 }
 
-func TestKVSEndpoint_ListKeys_ACLEnableKeyListPolicy(t *testing.T) {
-	t.Parallel()
-	dir1, s1 := testServerWithConfig(t, func(c *Config) {
-		c.ACLDatacenter = "dc1"
-		c.ACLMasterToken = "root"
-		c.ACLDefaultPolicy = "deny"
-		c.ACLEnableKeyListPolicy = true
-	})
-	defer os.RemoveAll(dir1)
-	defer s1.Shutdown()
-	codec := rpcClient(t, s1)
-	defer codec.Close()
-
-	testrpc.WaitForLeader(t, s1.RPC, "dc1")
-
-	keys := []string{
-		"abe",
-		"bar/bar1",
-		"bar/bar2",
-		"zip",
-	}
-
-	for _, key := range keys {
-		arg := structs.KVSRequest{
-			Datacenter: "dc1",
-			Op:         api.KVSet,
-			DirEnt: structs.DirEntry{
-				Key:   key,
-				Flags: 1,
-			},
-			WriteRequest: structs.WriteRequest{Token: "root"},
-		}
-		var out bool
-		if err := msgpackrpc.CallWithCodec(codec, "KVS.Apply", &arg, &out); err != nil {
-			t.Fatalf("err: %v", err)
-		}
-	}
-
-	//write acl policy that denies recursive reads on ""
-	var testListRules1 = `
-key "" {
-	policy = "deny"
-}
-key "bar" {
-	policy = "list"
-}
-key "zip" {
-	policy = "read"
-}
-`
-
-	arg := structs.ACLRequest{
-		Datacenter: "dc1",
-		Op:         structs.ACLSet,
-		ACL: structs.ACL{
-			Name:  "User token",
-			Type:  structs.ACLTypeClient,
-			Rules: testListRules1,
-		},
-		WriteRequest: structs.WriteRequest{Token: "root"},
-	}
-	var out string
-	if err := msgpackrpc.CallWithCodec(codec, "ACL.Apply", &arg, &out); err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	id := out
-
-	//recursive read on empty prefix should fail
-	getR := structs.KeyListRequest{
-		Datacenter:   "dc1",
-		Prefix:       "",
-		Seperator:    "/",
-		QueryOptions: structs.QueryOptions{Token: id},
-	}
-	var dirent structs.IndexedKeyList
-	if err := msgpackrpc.CallWithCodec(codec, "KVS.ListKeys", &getR, &dirent); !acl.IsErrPermissionDenied(err) {
-		t.Fatalf("expected %v but got err: %v", acl.ErrPermissionDenied, err)
-	}
-
-	// recursive read with a prefix that has list permissions should succeed
-	getR2 := structs.KeyListRequest{
-		Datacenter:   "dc1",
-		Prefix:       "bar",
-		QueryOptions: structs.QueryOptions{Token: id},
-	}
-	if err := msgpackrpc.CallWithCodec(codec, "KVS.ListKeys", &getR2, &dirent); err != nil {
-		t.Fatalf("err: %v", err)
-	}
-
-	if dirent.Index == 0 {
-		t.Fatalf("Bad: %v", dirent)
-	}
-	if len(dirent.Keys) != 2 {
-		t.Fatalf("Bad: %v", dirent.Keys)
-	}
-	if dirent.Keys[0] != "bar/bar1" {
-		t.Fatalf("Bad: %v", dirent.Keys)
-	}
-	if dirent.Keys[1] != "bar/bar2" {
-		t.Fatalf("Bad: %v", dirent.Keys)
-	}
-
-}
 
 func TestKVS_Apply_LockDelay(t *testing.T) {
 	t.Parallel()