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

txn: support `ResultsFilteredByACLs` flag in `Read` endpoint (#11632)

This commit is contained in:
Dan Upton 2021-12-03 20:41:03 +00:00 committed by GitHub
parent 001bcac084
commit 0a7ba5162e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 72 additions and 116 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
agent/consul/txn_endpoint.go
View File

@ -194,6 +194,14 @@ func (t *Txn) Read(args *structs.TxnReadRequest, reply *structs.TxnReadResponse)
if err != nil {
return err
}
// There are currently two different ways we handle permission issues.
//
// For simple reads such as KVGet and KVGetTree, the txn succeeds but the
// offending results are omitted. For more involved operations such as
// KVCheckIndex, the txn fails and permission denied errors are returned.
//
// TODO: Maybe we should unify these, or at least cover it in the docs?
reply.Errors = t.preCheck(authz, args.Ops)
if len(reply.Errors) > 0 {
return nil
@ -202,7 +210,10 @@ func (t *Txn) Read(args *structs.TxnReadRequest, reply *structs.TxnReadResponse)
// Run the read transaction.
state := t.srv.fsm.State()
reply.Results, reply.Errors = state.TxnRO(args.Ops)
total := len(reply.Results)
reply.Results = FilterTxnResults(authz, reply.Results)
reply.QueryMeta.ResultsFilteredByACLs = total != len(reply.Results)
// We have to do this ourselves since we are not doing a blocking RPC.
t.srv.setQueryMeta(&reply.QueryMeta, args.Token)

177
agent/consul/txn_endpoint_test.go
View File

@ -3,7 +3,6 @@ package consul
import (
"bytes"
"os"
"reflect"
"strings"
"testing"
"time"
@ -874,123 +873,69 @@ func TestTxn_Read_ACLDeny(t *testing.T) {
id := createToken(t, codec, testTxnRules)
// Set up a transaction where every operation should get blocked due to
// ACLs.
arg := structs.TxnReadRequest{
Datacenter: "dc1",
Ops: structs.TxnOps{
&structs.TxnOp{
KV: &structs.TxnKVOp{
Verb: api.KVGet,
DirEnt: structs.DirEntry{
Key: "nope",
t.Run("simple read operations (results get filtered out)", func(t *testing.T) {
arg := structs.TxnReadRequest{
Datacenter: "dc1",
QueryOptions: structs.QueryOptions{Token: id},
Ops: structs.TxnOps{
{
KV: &structs.TxnKVOp{
Verb: api.KVGet,
DirEnt: structs.DirEntry{
Key: "nope",
},
},
},
{
KV: &structs.TxnKVOp{
Verb: api.KVGetTree,
DirEnt: structs.DirEntry{
Key: "nope",
},
},
},
},
&structs.TxnOp{
KV: &structs.TxnKVOp{
Verb: api.KVGetTree,
DirEnt: structs.DirEntry{
Key: "nope",
},
},
},
&structs.TxnOp{
KV: &structs.TxnKVOp{
Verb: api.KVCheckSession,
DirEnt: structs.DirEntry{
Key: "nope",
},
},
},
&structs.TxnOp{
KV: &structs.TxnKVOp{
Verb: api.KVCheckIndex,
DirEnt: structs.DirEntry{
Key: "nope",
},
},
},
&structs.TxnOp{
Node: &structs.TxnNodeOp{
Verb: api.NodeGet,
Node: structs.Node{ID: node.ID, Node: node.Node},
},
},
&structs.TxnOp{
Service: &structs.TxnServiceOp{
Verb: api.ServiceGet,
Node: "foo",
Service: svc,
},
},
&structs.TxnOp{
Check: &structs.TxnCheckOp{
Verb: api.CheckGet,
Check: check,
},
},
},
QueryOptions: structs.QueryOptions{
Token: id,
},
}
var out structs.TxnReadResponse
if err := msgpackrpc.CallWithCodec(codec, "Txn.Read", &arg, &out); err != nil {
t.Fatalf("err: %v", err)
}
// Verify the transaction's return value.
var expected structs.TxnReadResponse
for i, op := range arg.Ops {
switch {
case op.KV != nil:
switch op.KV.Verb {
case api.KVGet, api.KVGetTree:
// These get filtered but won't result in an error.
default:
expected.Errors = append(expected.Errors, &structs.TxnError{
OpIndex: i,
What: acl.ErrPermissionDenied.Error(),
})
}
case op.Node != nil:
switch op.Node.Verb {
case api.NodeGet:
// These get filtered but won't result in an error.
default:
expected.Errors = append(expected.Errors, &structs.TxnError{
OpIndex: i,
What: acl.ErrPermissionDenied.Error(),
})
}
case op.Service != nil:
switch op.Service.Verb {
case api.ServiceGet:
// These get filtered but won't result in an error.
default:
expected.Errors = append(expected.Errors, &structs.TxnError{
OpIndex: i,
What: acl.ErrPermissionDenied.Error(),
})
}
case op.Check != nil:
switch op.Check.Verb {
case api.CheckGet:
// These get filtered but won't result in an error.
default:
expected.Errors = append(expected.Errors, &structs.TxnError{
OpIndex: i,
What: acl.ErrPermissionDenied.Error(),
})
}
}
}
if !reflect.DeepEqual(out, expected) {
t.Fatalf("bad %v", out)
}
var out structs.TxnReadResponse
err := msgpackrpc.CallWithCodec(codec, "Txn.Read", &arg, &out)
require.NoError(err)
require.Empty(out.Results)
require.Empty(out.Errors)
require.True(out.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
})
t.Run("complex operations (return permission denied errors)", func(t *testing.T) {
arg := structs.TxnReadRequest{
Datacenter: "dc1",
QueryOptions: structs.QueryOptions{Token: id},
Ops: structs.TxnOps{
{
KV: &structs.TxnKVOp{
Verb: api.KVCheckSession,
DirEnt: structs.DirEntry{
Key: "nope",
},
},
},
{
KV: &structs.TxnKVOp{
Verb: api.KVCheckIndex,
DirEnt: structs.DirEntry{
Key: "nope",
},
},
},
},
}
var out structs.TxnReadResponse
err := msgpackrpc.CallWithCodec(codec, "Txn.Read", &arg, &out)
require.NoError(err)
require.Equal(structs.TxnErrors{
{OpIndex: 0, What: acl.ErrPermissionDenied.Error()},
{OpIndex: 1, What: acl.ErrPermissionDenied.Error()},
}, out.Errors)
require.Empty(out.Results)
})
}