Revert "Return 403 rather than a 404 when acls cause all results to be filter…"

agent/consul/filter.go

@@ -22,13 +22,9 @@ func (d *dirEntFilter) Move(dst, src, span int) {
 
 // FilterDirEnt is used to filter a list of directory entries
 // by applying an ACL policy
-func FilterDirEnt(acl acl.ACL, ent structs.DirEntries) (structs.DirEntries, error) {
+func FilterDirEnt(acl acl.ACL, ent structs.DirEntries) structs.DirEntries {
 	df := dirEntFilter{acl: acl, ent: ent}
-	filtered := ent[:FilterEntries(&df)]
+	return ent[:FilterEntries(&df)]
-	if len(filtered) == 0 {
-		return nil, errPermissionDenied
-	}
-	return filtered, nil
 }
 
 type keyFilter struct {
@@ -49,13 +45,9 @@ func (k *keyFilter) Move(dst, src, span int) {
 
 // FilterKeys is used to filter a list of keys by
 // applying an ACL policy
-func FilterKeys(acl acl.ACL, keys []string) ([]string, error) {
+func FilterKeys(acl acl.ACL, keys []string) []string {
 	kf := keyFilter{acl: acl, keys: keys}
-	filteredKeys := keys[:FilterEntries(&kf)]
+	return keys[:FilterEntries(&kf)]
-	if len(filteredKeys) == 0 {
-		return nil, errPermissionDenied
-	}
-	return filteredKeys, nil
 }
 
 type txnResultsFilter struct {
@@ -81,13 +73,9 @@ func (t *txnResultsFilter) Move(dst, src, span int) {
 
 // FilterTxnResults is used to filter a list of transaction results by
 // applying an ACL policy.
-func FilterTxnResults(acl acl.ACL, results structs.TxnResults) (structs.TxnResults, error) {
+func FilterTxnResults(acl acl.ACL, results structs.TxnResults) structs.TxnResults {
 	rf := txnResultsFilter{acl: acl, results: results}
-	filtered := results[:FilterEntries(&rf)]
+	return results[:FilterEntries(&rf)]
-	if len(filtered) == 0 {
-		return nil, errPermissionDenied
-	}
-	return filtered, nil
 }
 
 // Filter interface is used with FilterEntries to do an

agent/consul/filter_test.go

@@ -16,7 +16,6 @@ func TestFilter_DirEnt(t *testing.T) {
 	type tcase struct {
 		in  []string
 		out []string
-		err error
 	}
 	cases := []tcase{
 		tcase{
@@ -26,7 +25,6 @@ func TestFilter_DirEnt(t *testing.T) {
 		tcase{
 			in:  []string{"abe", "lincoln"},
 			out: nil,
-			err: errPermissionDenied,
 		},
 		tcase{
 			in:  []string{"abe", "foo/1", "foo/2", "foo/3", "nope"},
@@ -40,10 +38,7 @@ func TestFilter_DirEnt(t *testing.T) {
 			ents = append(ents, &structs.DirEntry{Key: in})
 		}
 
-		ents, err := FilterDirEnt(aclR, ents)
+		ents = FilterDirEnt(aclR, ents)
-		if err != tc.err {
-			t.Fatalf("Unexpected error, got %v, wanted %v", err, tc.err)
-		}
 		var outL []string
 		for _, e := range ents {
 			outL = append(outL, e.Key)
@@ -63,7 +58,6 @@ func TestFilter_Keys(t *testing.T) {
 	type tcase struct {
 		in  []string
 		out []string
-		err error
 	}
 	cases := []tcase{
 		tcase{
@@ -72,7 +66,7 @@ func TestFilter_Keys(t *testing.T) {
 		},
 		tcase{
 			in:  []string{"abe", "lincoln"},
-			err: errPermissionDenied,
+			out: []string{},
 		},
 		tcase{
 			in:  []string{"abe", "foo/1", "foo/2", "foo/3", "nope"},
@@ -81,14 +75,10 @@ func TestFilter_Keys(t *testing.T) {
 	}
 
 	for _, tc := range cases {
-		out, err := FilterKeys(aclR, tc.in)
+		out := FilterKeys(aclR, tc.in)
-		if tc.err != err {
-			t.Fatalf("Unexpected error, got %v, wanted %v", err, tc.err)
-		}
 		if !reflect.DeepEqual(out, tc.out) {
 			t.Fatalf("bad: %#v %#v", out, tc.out)
 		}
-
 	}
 }
 
@@ -100,7 +90,6 @@ func TestFilter_TxnResults(t *testing.T) {
 	type tcase struct {
 		in  []string
 		out []string
-		err error
 	}
 	cases := []tcase{
 		tcase{
@@ -110,7 +99,6 @@ func TestFilter_TxnResults(t *testing.T) {
 		tcase{
 			in:  []string{"abe", "lincoln"},
 			out: nil,
-			err: errPermissionDenied,
 		},
 		tcase{
 			in:  []string{"abe", "foo/1", "foo/2", "foo/3", "nope"},
@@ -124,10 +112,7 @@ func TestFilter_TxnResults(t *testing.T) {
 			results = append(results, &structs.TxnResult{KV: &structs.DirEntry{Key: in}})
 		}
 
-		results, err := FilterTxnResults(aclR, results)
+		results = FilterTxnResults(aclR, results)
-		if tc.err != err {
-			t.Fatalf("Unexpected error, got %v, wanted %v", err, tc.err)
-		}
 		var outL []string
 		for _, r := range results {
 			outL = append(outL, r.KV.Key)
@@ -141,10 +126,7 @@ func TestFilter_TxnResults(t *testing.T) {
 	// Run a non-KV result.
 	results := structs.TxnResults{}
 	results = append(results, &structs.TxnResult{})
-	results, err := FilterTxnResults(aclR, results)
+	results = FilterTxnResults(aclR, results)
-	if err != nil {
-		t.Fatalf("Unexpected error: %v", err)
-	}
 	if len(results) != 1 {
 		t.Fatalf("should not have filtered non-KV result")
 	}

agent/consul/kvs_endpoint.go

@@ -129,7 +129,7 @@ func (k *KVS) Get(args *structs.KeyRequest, reply *structs.IndexedDirEntries) er
 				return err
 			}
 			if acl != nil && !acl.KeyRead(args.Key) {
-				return errPermissionDenied
+				ent = nil
 			}
 			if ent == nil {
 				// Must provide non-zero index to prevent blocking
@@ -168,11 +168,9 @@ func (k *KVS) List(args *structs.KeyRequest, reply *structs.IndexedDirEntries) e
 				return err
 			}
 			if acl != nil {
-				ent, err = FilterDirEnt(acl, ent)
+				ent = FilterDirEnt(acl, ent)
-				if err != nil {
-					return err
-				}
 			}
+
 			if len(ent) == 0 {
 				// Must provide non-zero index to prevent blocking
 				// Index 1 is impossible anyways (due to Raft internals)
@@ -219,10 +217,7 @@ func (k *KVS) ListKeys(args *structs.KeyListRequest, reply *structs.IndexedKeyLi
 			}
 
 			if acl != nil {
-				keys, err = FilterKeys(acl, keys)
+				keys = FilterKeys(acl, keys)
-				if err != nil {
-					return err
-				}
 			}
 			reply.Keys = keys
 			return nil

agent/consul/kvs_endpoint_test.go

@@ -206,7 +206,7 @@ func TestKVS_Get_ACLDeny(t *testing.T) {
 	}
 	var out bool
 	if err := msgpackrpc.CallWithCodec(codec, "KVS.Apply", &arg, &out); err != nil {
-		t.Fatalf("Unexpected err: %v", err)
+		t.Fatalf("err: %v", err)
 	}
 
 	getR := structs.KeyRequest{
@@ -214,10 +214,16 @@ func TestKVS_Get_ACLDeny(t *testing.T) {
 		Key:        "zip",
 	}
 	var dirent structs.IndexedDirEntries
-	if err := msgpackrpc.CallWithCodec(codec, "KVS.Get", &getR, &dirent); err.Error() != errPermissionDenied.Error() {
+	if err := msgpackrpc.CallWithCodec(codec, "KVS.Get", &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)
+	}
 }
 
 func TestKVSEndpoint_List(t *testing.T) {

agent/consul/txn_endpoint.go

@@ -72,10 +72,7 @@ func (t *Txn) Apply(args *structs.TxnRequest, reply *structs.TxnResponse) error
 	// just taking the two slices.
 	if txnResp, ok := resp.(structs.TxnResponse); ok {
 		if acl != nil {
-			txnResp.Results, err = FilterTxnResults(acl, txnResp.Results)
+			txnResp.Results = FilterTxnResults(acl, txnResp.Results)
-			if err != nil {
-				return err
-			}
 		}
 		*reply = txnResp
 	} else {
@@ -116,10 +113,7 @@ func (t *Txn) Read(args *structs.TxnReadRequest, reply *structs.TxnReadResponse)
 	state := t.srv.fsm.State()
 	reply.Results, reply.Errors = state.TxnRO(args.Ops)
 	if acl != nil {
-		reply.Results, err = FilterTxnResults(acl, reply.Results)
+		reply.Results = FilterTxnResults(acl, reply.Results)
-		if err != nil {
-			return err
-		}
 	}
 	return nil
 }