diff --git a/acl/policy.go b/acl/policy.go index 4da26f45bf..c8b5a93bda 100644 --- a/acl/policy.go +++ b/acl/policy.go @@ -119,8 +119,6 @@ func isPolicyValid(policy string) bool { return true case PolicyWrite: return true - case PolicyList: - return true default: return false } @@ -178,7 +176,7 @@ func Parse(rules string, sentinel sentinel.Evaluator) (*Policy, error) { // Validate the key policy for _, kp := range p.Keys { - if !isPolicyValid(kp.Policy) { + if kp.Policy != PolicyList && !isPolicyValid(kp.Policy) { return nil, fmt.Errorf("Invalid key policy: %#v", kp) } if err := isSentinelValid(sentinel, kp.Policy, kp.Sentinel); err != nil { diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index b63ecc14a8..9caf9a216a 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -1932,7 +1932,7 @@ func TestFullConfig(t *testing.T) { "acl_default_policy": "ArK3WIfE", "acl_down_policy": "vZXMfMP0", "acl_enforce_version_8": true, - "acl_enable_key_list_policy": true, + "acl_enable_key_list_policy": true, "acl_master_token": "C1Q1oIwh", "acl_replication_token": "LMmgy5dO", "acl_token": "O1El0wan", @@ -2348,7 +2348,7 @@ func TestFullConfig(t *testing.T) { acl_default_policy = "ArK3WIfE" acl_down_policy = "vZXMfMP0" acl_enforce_version_8 = true - acl_enable_key_list_policy = true + acl_enable_key_list_policy = true acl_master_token = "C1Q1oIwh" acl_replication_token = "LMmgy5dO" acl_token = "O1El0wan" diff --git a/agent/consul/kvs_endpoint_test.go b/agent/consul/kvs_endpoint_test.go index 5a72bd0a22..07228411ec 100644 --- a/agent/consul/kvs_endpoint_test.go +++ b/agent/consul/kvs_endpoint_test.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/testrpc" "github.com/hashicorp/net-rpc-msgpackrpc" + "github.com/pascaldekloe/goe/verify" ) func TestKVS_Apply(t *testing.T) { @@ -541,46 +542,64 @@ key "zip" { id := out //recursive read on empty prefix should fail - getR := structs.KeyRequest{ + getReq := structs.KeyRequest{ Datacenter: "dc1", Key: "", QueryOptions: structs.QueryOptions{Token: id}, } var dirent structs.IndexedDirEntries - if err := msgpackrpc.CallWithCodec(codec, "KVS.List", &getR, &dirent); !acl.IsErrPermissionDenied(err) { + if err := msgpackrpc.CallWithCodec(codec, "KVS.List", &getReq, &dirent); !acl.IsErrPermissionDenied(err) { + t.Fatalf("expected %v but got err: %v", acl.ErrPermissionDenied, err) + } + + //listing keys on empty prefix should fail + getKeysReq := structs.KeyListRequest{ + Datacenter: "dc1", + Prefix: "", + Seperator: "/", + QueryOptions: structs.QueryOptions{Token: id}, + } + var keyList structs.IndexedKeyList + if err := msgpackrpc.CallWithCodec(codec, "KVS.ListKeys", &getKeysReq, &keyList); !acl.IsErrPermissionDenied(err) { t.Fatalf("expected %v but got err: %v", acl.ErrPermissionDenied, err) } // recursive read with a prefix that has list permissions should succeed - getR2 := structs.KeyRequest{ + getReq2 := structs.KeyRequest{ Datacenter: "dc1", Key: "bar", QueryOptions: structs.QueryOptions{Token: id}, } - if err := msgpackrpc.CallWithCodec(codec, "KVS.List", &getR2, &dirent); err != nil { + if err := msgpackrpc.CallWithCodec(codec, "KVS.List", &getReq2, &dirent); err != nil { t.Fatalf("err: %v", err) } - if dirent.Index == 0 { - t.Fatalf("Bad: %v", dirent) + expectedKeys := []string{"bar/bar1", "bar/bar2"} + var actualKeys []string + for _, entry := range dirent.Entries { + actualKeys = append(actualKeys, entry.Key) } - if len(dirent.Entries) != 2 { - t.Fatalf("Bad: %v", dirent.Entries) + + verify.Values(t, "", actualKeys, expectedKeys) + + // list keys with a prefix that has list permissions should succeed + getKeysReq2 := structs.KeyListRequest{ + Datacenter: "dc1", + Prefix: "bar", + QueryOptions: structs.QueryOptions{Token: id}, } - for i := 0; i < len(dirent.Entries); i++ { - d := dirent.Entries[i] - switch i { - case 0: - if d.Key != "bar/bar1" { - t.Fatalf("bad key %v", d.Key) - } - case 1: - if d.Key != "bar/bar2" { - t.Fatalf("bad key %v", d.Key) - } - } + if err := msgpackrpc.CallWithCodec(codec, "KVS.ListKeys", &getKeysReq2, &keyList); err != nil { + t.Fatalf("err: %v", err) } + actualKeys = []string{} + + for _, key := range keyList.Keys { + actualKeys = append(actualKeys, key) + } + + verify.Values(t, "", actualKeys, expectedKeys) + } func TestKVSEndpoint_ListKeys(t *testing.T) { @@ -732,109 +751,6 @@ func TestKVSEndpoint_ListKeys_ACLDeny(t *testing.T) { } } -func TestKVSEndpoint_ListKeys_ACLEnableKeyListPolicy(t *testing.T) { - t.Parallel() - dir1, s1 := testServerWithConfig(t, func(c *Config) { - c.ACLDatacenter = "dc1" - c.ACLMasterToken = "root" - c.ACLDefaultPolicy = "deny" - c.ACLEnableKeyListPolicy = true - }) - defer os.RemoveAll(dir1) - defer s1.Shutdown() - codec := rpcClient(t, s1) - defer codec.Close() - - testrpc.WaitForLeader(t, s1.RPC, "dc1") - - keys := []string{ - "abe", - "bar/bar1", - "bar/bar2", - "zip", - } - - for _, key := range keys { - arg := structs.KVSRequest{ - Datacenter: "dc1", - Op: api.KVSet, - DirEnt: structs.DirEntry{ - Key: key, - Flags: 1, - }, - WriteRequest: structs.WriteRequest{Token: "root"}, - } - var out bool - if err := msgpackrpc.CallWithCodec(codec, "KVS.Apply", &arg, &out); err != nil { - t.Fatalf("err: %v", err) - } - } - - //write acl policy that denies recursive reads on "" - var testListRules1 = ` -key "" { - policy = "deny" -} -key "bar" { - policy = "list" -} -key "zip" { - policy = "read" -} -` - - arg := structs.ACLRequest{ - Datacenter: "dc1", - Op: structs.ACLSet, - ACL: structs.ACL{ - Name: "User token", - Type: structs.ACLTypeClient, - Rules: testListRules1, - }, - WriteRequest: structs.WriteRequest{Token: "root"}, - } - var out string - if err := msgpackrpc.CallWithCodec(codec, "ACL.Apply", &arg, &out); err != nil { - t.Fatalf("err: %v", err) - } - id := out - - //recursive read on empty prefix should fail - getR := structs.KeyListRequest{ - Datacenter: "dc1", - Prefix: "", - Seperator: "/", - QueryOptions: structs.QueryOptions{Token: id}, - } - var dirent structs.IndexedKeyList - if err := msgpackrpc.CallWithCodec(codec, "KVS.ListKeys", &getR, &dirent); !acl.IsErrPermissionDenied(err) { - t.Fatalf("expected %v but got err: %v", acl.ErrPermissionDenied, err) - } - - // recursive read with a prefix that has list permissions should succeed - getR2 := structs.KeyListRequest{ - Datacenter: "dc1", - Prefix: "bar", - QueryOptions: structs.QueryOptions{Token: id}, - } - if err := msgpackrpc.CallWithCodec(codec, "KVS.ListKeys", &getR2, &dirent); err != nil { - t.Fatalf("err: %v", err) - } - - if dirent.Index == 0 { - t.Fatalf("Bad: %v", dirent) - } - if len(dirent.Keys) != 2 { - t.Fatalf("Bad: %v", dirent.Keys) - } - if dirent.Keys[0] != "bar/bar1" { - t.Fatalf("Bad: %v", dirent.Keys) - } - if dirent.Keys[1] != "bar/bar2" { - t.Fatalf("Bad: %v", dirent.Keys) - } - -} func TestKVS_Apply_LockDelay(t *testing.T) { t.Parallel()