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

Return 403 rather than a 404 when acls cause all results to be filtered out. This fixes #2637

This commit is contained in:
Preetha Appan 2017-07-31 13:40:21 -05:00
parent 5a19cce832
commit da29b74d03
5 changed files with 60 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
agent/consul/filter.go
View File

@ -22,9 +22,13 @@ 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 {
func FilterDirEnt(acl acl.ACL, ent structs.DirEntries) (structs.DirEntries, error) {
df := dirEntFilter{acl: acl, ent: ent}
return ent[:FilterEntries(&df)]
filtered := ent[:FilterEntries(&df)]
if len(filtered) == 0 {
return filtered, errPermissionDenied
}
return filtered, nil
}
type keyFilter struct {
@ -45,9 +49,13 @@ 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 {
func FilterKeys(acl acl.ACL, keys []string) ([]string, error) {
kf := keyFilter{acl: acl, keys: keys}
return keys[:FilterEntries(&kf)]
filteredKeys := keys[:FilterEntries(&kf)]
if len(filteredKeys) == 0 {
return filteredKeys, errPermissionDenied
}
return filteredKeys, nil
}
type txnResultsFilter struct {
@ -73,9 +81,13 @@ 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 {
func FilterTxnResults(acl acl.ACL, results structs.TxnResults) (structs.TxnResults, error) {
rf := txnResultsFilter{acl: acl, results: results}
return results[:FilterEntries(&rf)]
filtered := results[:FilterEntries(&rf)]
if len(filtered) == 0 {
return filtered, errPermissionDenied
}
return filtered, nil
}
// Filter interface is used with FilterEntries to do an

27
agent/consul/filter_test.go
View File

@ -16,6 +16,7 @@ func TestFilter_DirEnt(t *testing.T) {
type tcase struct {
in []string
out []string
err error
}
cases := []tcase{
tcase{
@ -25,6 +26,7 @@ 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"},
@ -38,7 +40,10 @@ func TestFilter_DirEnt(t *testing.T) {
ents = append(ents, &structs.DirEntry{Key: in})
}
ents = FilterDirEnt(aclR, ents)
ents, err := 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)
@ -58,6 +63,7 @@ func TestFilter_Keys(t *testing.T) {
type tcase struct {
in []string
out []string
err error
}
cases := []tcase{
tcase{
@ -67,6 +73,7 @@ func TestFilter_Keys(t *testing.T) {
tcase{
in: []string{"abe", "lincoln"},
out: []string{},
err: errPermissionDenied,
},
tcase{
in: []string{"abe", "foo/1", "foo/2", "foo/3", "nope"},
@ -75,10 +82,14 @@ func TestFilter_Keys(t *testing.T) {
}
for _, tc := range cases {
out := FilterKeys(aclR, tc.in)
out, err := 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)
}
}
}
@ -90,6 +101,7 @@ func TestFilter_TxnResults(t *testing.T) {
type tcase struct {
in []string
out []string
err error
}
cases := []tcase{
tcase{
@ -99,6 +111,7 @@ 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"},
@ -112,7 +125,10 @@ func TestFilter_TxnResults(t *testing.T) {
results = append(results, &structs.TxnResult{KV: &structs.DirEntry{Key: in}})
}
results = FilterTxnResults(aclR, results)
results, err := 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)
@ -126,7 +142,10 @@ func TestFilter_TxnResults(t *testing.T) {
// Run a non-KV result.
results := structs.TxnResults{}
results = append(results, &structs.TxnResult{})
results = FilterTxnResults(aclR, results)
results, err := 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")
}

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) {
ent = nil
return errPermissionDenied
}
if ent == nil {
// Must provide non-zero index to prevent blocking
@ -168,9 +168,11 @@ func (k *KVS) List(args *structs.KeyRequest, reply *structs.IndexedDirEntries) e
return err
}
if acl != nil {
ent = FilterDirEnt(acl, ent)
ent, err = 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)
@ -217,7 +219,10 @@ func (k *KVS) ListKeys(args *structs.KeyListRequest, reply *structs.IndexedKeyLi
}
if acl != nil {
keys = FilterKeys(acl, keys)
keys, err = FilterKeys(acl, keys)
if err != nil {
return err
}
}
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("err: %v", err)
t.Fatalf("Unexpected err: %v", err)
}
getR := structs.KeyRequest{
@ -214,16 +214,10 @@ func TestKVS_Get_ACLDeny(t *testing.T) {
Key: "zip",
}
var dirent structs.IndexedDirEntries
if err := msgpackrpc.CallWithCodec(codec, "KVS.Get", &getR, &dirent); err != nil {
if err := msgpackrpc.CallWithCodec(codec, "KVS.Get", &getR, &dirent); err.Error() != errPermissionDenied.Error() {
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,7 +72,10 @@ 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 = FilterTxnResults(acl, txnResp.Results)
txnResp.Results, err = FilterTxnResults(acl, txnResp.Results)
if err != nil {
return err
}
}
*reply = txnResp
} else {
@ -113,7 +116,10 @@ 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 = FilterTxnResults(acl, reply.Results)
reply.Results, err = FilterTxnResults(acl, reply.Results)
if err != nil {
return err
}
}
return nil
}