luxolus
/
open-consul
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

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

This commit is contained in:
James Phillips 2017-08-09 15:06:57 -07:00 committed by GitHub
parent bdf12bf338
commit 3518e27a76
5 changed files with 25 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
agent/consul/filter.go
View File

@ -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)]
if len(filtered) == 0 {
return nil, errPermissionDenied
}
return filtered, nil
return ent[:FilterEntries(&df)]
}
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)]
if len(filteredKeys) == 0 {
return nil, errPermissionDenied
}
return filteredKeys, nil
return keys[:FilterEntries(&kf)]
}
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)]
if len(filtered) == 0 {
return nil, errPermissionDenied
}
return filtered, nil
return results[:FilterEntries(&rf)]
}
// Filter interface is used with FilterEntries to do an

28
agent/consul/filter_test.go
View File

@ -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)
if err != tc.err {
t.Fatalf("Unexpected error, got %v, wanted %v", err, tc.err)
}
ents = FilterDirEnt(aclR, ents)
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)
if tc.err != err {
t.Fatalf("Unexpected error, got %v, wanted %v", err, tc.err)
}
out := FilterKeys(aclR, tc.in)
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)
if tc.err != err {
t.Fatalf("Unexpected error, got %v, wanted %v", err, tc.err)
}
results = FilterTxnResults(aclR, results)
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)
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
results = FilterTxnResults(aclR, results)
if len(results) != 1 {
t.Fatalf("should not have filtered non-KV result")
}

13
agent/consul/kvs_endpoint.go
View File

@ -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)
if err != nil {
return err
}
ent = FilterDirEnt(acl, ent)
}
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)
if err != nil {
return err
}
keys = FilterKeys(acl, keys)
}
reply.Keys = keys
return nil

10
agent/consul/kvs_endpoint_test.go
View File

@ -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) {

10
agent/consul/txn_endpoint.go
View File

@ -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)
if err != nil {
return err
}
txnResp.Results = FilterTxnResults(acl, txnResp.Results)
}
*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)
if err != nil {
return err
}
reply.Results = FilterTxnResults(acl, reply.Results)
}
return nil
}