Merge pull request #3380 from hashicorp/fix_acls

Ensure that we return a permission denied only if the list of keys/en…

agent/consul/filter.go

@@ -25,7 +25,7 @@ func (d *dirEntFilter) Move(dst, src, span int) {
 func FilterDirEnt(acl acl.ACL, ent structs.DirEntries) (structs.DirEntries, error) {
 	df := dirEntFilter{acl: acl, ent: ent}
 	filtered := ent[:FilterEntries(&df)]
-	if len(filtered) == 0 {
+	if len(ent) > 0 && len(filtered) == 0 {
 		return nil, errPermissionDenied
 	}
 	return filtered, nil
@@ -52,7 +52,7 @@ func (k *keyFilter) Move(dst, src, span int) {
 func FilterKeys(acl acl.ACL, keys []string) ([]string, error) {
 	kf := keyFilter{acl: acl, keys: keys}
 	filteredKeys := keys[:FilterEntries(&kf)]
-	if len(filteredKeys) == 0 {
+	if len(keys) > 0 && len(filteredKeys) == 0 {
 		return nil, errPermissionDenied
 	}
 	return filteredKeys, nil
@@ -84,7 +84,7 @@ func (t *txnResultsFilter) Move(dst, src, span int) {
 func FilterTxnResults(acl acl.ACL, results structs.TxnResults) (structs.TxnResults, error) {
 	rf := txnResultsFilter{acl: acl, results: results}
 	filtered := results[:FilterEntries(&rf)]
-	if len(filtered) == 0 {
+	if len(results) > 0 && len(filtered) == 0 {
 		return nil, errPermissionDenied
 	}
 	return filtered, nil

agent/consul/filter_test.go

@@ -32,6 +32,10 @@ func TestFilter_DirEnt(t *testing.T) {
 			in:  []string{"abe", "foo/1", "foo/2", "foo/3", "nope"},
 			out: []string{"foo/1", "foo/2", "foo/3"},
 		},
+		tcase{
+			in:  []string{},
+			out: nil,
+		},
 	}
 
 	for _, tc := range cases {
@@ -78,6 +82,10 @@ func TestFilter_Keys(t *testing.T) {
 			in:  []string{"abe", "foo/1", "foo/2", "foo/3", "nope"},
 			out: []string{"foo/1", "foo/2", "foo/3"},
 		},
+		tcase{
+			in:  []string{},
+			out: []string{},
+		},
 	}
 
 	for _, tc := range cases {
@@ -116,6 +124,10 @@ func TestFilter_TxnResults(t *testing.T) {
 			in:  []string{"abe", "foo/1", "foo/2", "foo/3", "nope"},
 			out: []string{"foo/1", "foo/2", "foo/3"},
 		},
+		tcase{
+			in:  []string{},
+			out: nil,
+		},
 	}
 
 	for _, tc := range cases {

agent/consul/kvs_endpoint_test.go

@@ -622,6 +622,57 @@ func TestKVSEndpoint_ListKeys_ACLDeny(t *testing.T) {
 	}
 }
 
+func TestKVSEndpoint_EmptyKeys_ACLAllow(t *testing.T) {
+	t.Parallel()
+	dir1, s1 := testServerWithConfig(t, func(c *Config) {
+		c.ACLDatacenter = "dc1"
+		c.ACLMasterToken = "root"
+		c.ACLDefaultPolicy = "deny"
+	})
+	defer os.RemoveAll(dir1)
+	defer s1.Shutdown()
+	codec := rpcClient(t, s1)
+	defer codec.Close()
+
+	testrpc.WaitForLeader(t, s1.RPC, "dc1")
+
+	getKeyR := structs.KeyListRequest{
+		Datacenter:   "dc1",
+		Prefix:       "abc",
+		Seperator:    "/",
+		QueryOptions: structs.QueryOptions{Token: "root"},
+	}
+	var keyent structs.IndexedKeyList
+	if err := msgpackrpc.CallWithCodec(codec, "KVS.ListKeys", &getKeyR, &keyent); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if keyent.Index == 0 {
+		t.Fatalf("Bad: %v", keyent)
+	}
+	if len(keyent.Keys) != 0 {
+		t.Fatalf("Bad: %v", keyent.Keys)
+	}
+
+	getR := structs.KeyRequest{
+		Datacenter:   "dc1",
+		Key:          "abc",
+		QueryOptions: structs.QueryOptions{Token: "root"},
+	}
+
+	var dirent structs.IndexedDirEntries
+	if err := msgpackrpc.CallWithCodec(codec, "KVS.List", &getR, &dirent); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	if dirent.Index == 0 {
+		t.Fatalf("Bad: %v", dirent)
+	}
+	if len(dirent.Entries) != 0 {
+		t.Fatalf("Bad: %v", dirent.Entries)
+	}
+
+}
+
 func TestKVS_Apply_LockDelay(t *testing.T) {
 	t.Parallel()
 	dir1, s1 := testServer(t)