From 2b367626f0309de1791a497ea7465ae2c4ffb11d Mon Sep 17 00:00:00 2001 From: Mark Anderson Date: Fri, 18 Mar 2022 10:32:25 -0700 Subject: [PATCH] Add source of authority annotations to the PermissionDeniedError output. (#12567) This extends the acl.AllowAuthorizer with source of authority information. The next step is to unify the AllowAuthorizer and ACLResolveResult structures; that will be done in a separate PR. Part of #12481 Signed-off-by: Mark Anderson --- .changelog/12567.txt | 3 ++ acl/authorizer.go | 1 + acl/errors.go | 19 ++++++-- acl/testing.go | 8 ++-- agent/consul/acl.go | 4 ++ agent/consul/acl_endpoint.go | 2 +- agent/consul/catalog_endpoint.go | 6 +-- agent/consul/catalog_endpoint_test.go | 69 +++++++++++++++++---------- agent/consul/kvs_endpoint.go | 2 +- agent/consul/txn_endpoint.go | 6 +-- agent/consul/txn_endpoint_test.go | 26 +++++----- agent/structs/config_entry.go | 1 + 12 files changed, 93 insertions(+), 54 deletions(-) create mode 100644 .changelog/12567.txt diff --git a/.changelog/12567.txt b/.changelog/12567.txt new file mode 100644 index 000000000..047493249 --- /dev/null +++ b/.changelog/12567.txt @@ -0,0 +1,3 @@ +```release-note:feature +acl: Add token information to PermissionDeniedErrors +``` diff --git a/acl/authorizer.go b/acl/authorizer.go index 91474b467..7dc961c57 100644 --- a/acl/authorizer.go +++ b/acl/authorizer.go @@ -171,6 +171,7 @@ type Authorizer interface { // is moved into acl. type AllowAuthorizer struct { Authorizer + AccessorID string } // ACLReadAllowed checks for permission to list all the ACLs diff --git a/acl/errors.go b/acl/errors.go index c46292339..c2363e2a1 100644 --- a/acl/errors.go +++ b/acl/errors.go @@ -117,12 +117,23 @@ func PermissionDenied(msg string, args ...interface{}) PermissionDeniedError { } // TODO Extract information from Authorizer -func PermissionDeniedByACL(_ Authorizer, context *AuthorizerContext, resource Resource, accessLevel AccessLevel, resourceID string) PermissionDeniedError { +func PermissionDeniedByACL(authz Authorizer, context *AuthorizerContext, resource Resource, accessLevel AccessLevel, resourceID string) PermissionDeniedError { desc := NewResourceDescriptor(resourceID, context) - return PermissionDeniedError{Accessor: "", Resource: resource, AccessLevel: accessLevel, ResourceID: desc} + return PermissionDeniedError{Accessor: extractAccessorID(authz), Resource: resource, AccessLevel: accessLevel, ResourceID: desc} } -func PermissionDeniedByACLUnnamed(_ Authorizer, context *AuthorizerContext, resource Resource, accessLevel AccessLevel) PermissionDeniedError { +func PermissionDeniedByACLUnnamed(authz Authorizer, context *AuthorizerContext, resource Resource, accessLevel AccessLevel) PermissionDeniedError { desc := NewResourceDescriptor("", context) - return PermissionDeniedError{Accessor: "", Resource: resource, AccessLevel: accessLevel, ResourceID: desc} + return PermissionDeniedError{Accessor: extractAccessorID(authz), Resource: resource, AccessLevel: accessLevel, ResourceID: desc} +} + +func extractAccessorID(authz interface{}) string { + var accessor string + switch v := authz.(type) { + case AllowAuthorizer: + accessor = v.AccessorID + case string: + accessor = v + } + return accessor } diff --git a/acl/testing.go b/acl/testing.go index 1d4873cd9..01399c630 100644 --- a/acl/testing.go +++ b/acl/testing.go @@ -6,7 +6,7 @@ import ( "testing" ) -func RequirePermissionDeniedError(t testing.TB, err error, _ Authorizer, _ *AuthorizerContext, resource Resource, accessLevel AccessLevel, resourceID string) { +func RequirePermissionDeniedError(t testing.TB, err error, authz Authorizer, _ *AuthorizerContext, resource Resource, accessLevel AccessLevel, resourceID string) { t.Helper() if err == nil { t.Fatal("An error is expected but got nil.") @@ -20,11 +20,11 @@ func RequirePermissionDeniedError(t testing.TB, err error, _ Authorizer, _ *Auth } } -func RequirePermissionDeniedMessage(t testing.TB, msg string, auth Authorizer, _ *AuthorizerContext, resource Resource, accessLevel AccessLevel, resourceID string) { +func RequirePermissionDeniedMessage(t testing.TB, msg string, authz interface{}, _ *AuthorizerContext, resource Resource, accessLevel AccessLevel, resourceID string) { require.NotEmpty(t, msg, "expected non-empty error message") var resourceIDFound string - if auth == nil { + if authz == nil { expr := "^Permission denied" + `: provided accessor lacks permission '(\S*):(\S*)' on (.*)\s*$` re, _ := regexp.Compile(expr) matched := re.FindStringSubmatch(msg) @@ -37,7 +37,7 @@ func RequirePermissionDeniedMessage(t testing.TB, msg string, auth Authorizer, _ re, _ := regexp.Compile(expr) matched := re.FindStringSubmatch(msg) - require.Equal(t, auth, matched[1], "auth") + require.Equal(t, extractAccessorID(authz), matched[1], "auth") require.Equal(t, string(resource), matched[2], "resource") require.Equal(t, accessLevel.String(), matched[3], "access level") resourceIDFound = matched[4] diff --git a/agent/consul/acl.go b/agent/consul/acl.go index 350f9993b..bd84857b6 100644 --- a/agent/consul/acl.go +++ b/agent/consul/acl.go @@ -1134,6 +1134,10 @@ func (a ACLResolveResult) Identity() structs.ACLIdentity { return a.ACLIdentity } +func (a ACLResolveResult) ToAllowAuthorizer() acl.AllowAuthorizer { + return acl.AllowAuthorizer{Authorizer: a, AccessorID: a.AccessorID()} +} + func (r *ACLResolver) ACLsEnabled() bool { // Whether we desire ACLs to be enabled according to configuration if !r.config.ACLsEnabled { diff --git a/agent/consul/acl_endpoint.go b/agent/consul/acl_endpoint.go index eeb2e8085..2541c36c1 100644 --- a/agent/consul/acl_endpoint.go +++ b/agent/consul/acl_endpoint.go @@ -276,7 +276,7 @@ func (a *ACL) TokenRead(args *structs.ACLTokenGetRequest, reply *structs.ACLToke return err } - var authz acl.Authorizer + var authz ACLResolveResult if args.TokenIDType == structs.ACLTokenAccessor { var err error diff --git a/agent/consul/catalog_endpoint.go b/agent/consul/catalog_endpoint.go index bb97dcd6a..77ac97e77 100644 --- a/agent/consul/catalog_endpoint.go +++ b/agent/consul/catalog_endpoint.go @@ -159,7 +159,7 @@ func nodePreApply(nodeName, nodeID string) error { return nil } -func servicePreApply(service *structs.NodeService, authz acl.Authorizer, authzCtxFill func(*acl.AuthorizerContext)) error { +func servicePreApply(service *structs.NodeService, authz ACLResolveResult, authzCtxFill func(*acl.AuthorizerContext)) error { // Validate the service. This is in addition to the below since // the above just hasn't been moved over yet. We should move it over // in time. @@ -229,7 +229,7 @@ func checkPreApply(check *structs.HealthCheck) { // worst let a service update revert a recent node update, so it doesn't open up // too much abuse). func vetRegisterWithACL( - authz acl.Authorizer, + authz ACLResolveResult, subj *structs.RegisterRequest, ns *structs.NodeServices, ) error { @@ -395,7 +395,7 @@ func (c *Catalog) Deregister(args *structs.DeregisterRequest, reply *struct{}) e // endpoint. The NodeService for the referenced service must be supplied, and can // be nil; similar for the HealthCheck for the referenced health check. func vetDeregisterWithACL( - authz acl.Authorizer, + authz ACLResolveResult, subj *structs.DeregisterRequest, ns *structs.NodeService, nc *structs.HealthCheck, diff --git a/agent/consul/catalog_endpoint_test.go b/agent/consul/catalog_endpoint_test.go index 194346dcf..cf4b024f0 100644 --- a/agent/consul/catalog_endpoint_test.go +++ b/agent/consul/catalog_endpoint_test.go @@ -262,12 +262,16 @@ node "foo" { } } -func createToken(t *testing.T, cc rpc.ClientCodec, policyRules string) string { +func createTokenFull(t *testing.T, cc rpc.ClientCodec, policyRules string) *structs.ACLToken { t.Helper() - return createTokenWithPolicyName(t, cc, "the-policy", policyRules, "root") + return createTokenWithPolicyNameFull(t, cc, "the-policy", policyRules, "root") } -func createTokenWithPolicyName(t *testing.T, cc rpc.ClientCodec, policyName string, policyRules string, token string) string { +func createToken(t *testing.T, cc rpc.ClientCodec, policyRules string) string { + return createTokenFull(t, cc, policyRules).SecretID +} + +func createTokenWithPolicyNameFull(t *testing.T, cc rpc.ClientCodec, policyName string, policyRules string, token string) *structs.ACLToken { t.Helper() reqPolicy := structs.ACLPolicySetRequest{ @@ -292,9 +296,16 @@ func createTokenWithPolicyName(t *testing.T, cc rpc.ClientCodec, policyName stri }, WriteRequest: structs.WriteRequest{Token: token}, } - err = msgpackrpc.CallWithCodec(cc, "ACL.TokenSet", &reqToken, &structs.ACLToken{}) + + resp := &structs.ACLToken{} + + err = msgpackrpc.CallWithCodec(cc, "ACL.TokenSet", &reqToken, &resp) require.NoError(t, err) - return secretId + return resp +} + +func createTokenWithPolicyName(t *testing.T, cc rpc.ClientCodec, policyName string, policyRules string, token string) string { + return createTokenWithPolicyNameFull(t, cc, policyName, policyRules, token).SecretID } func TestCatalog_Register_ForwardLeader(t *testing.T) { @@ -3449,10 +3460,11 @@ func TestVetRegisterWithACL(t *testing.T) { } // With an "allow all" authorizer the update should be allowed. - require.NoError(t, vetRegisterWithACL(acl.ManageAll(), args, nil)) + require.NoError(t, vetRegisterWithACL(ACLResolveResult{Authorizer: acl.ManageAll()}, args, nil)) }) var perms acl.Authorizer = acl.DenyAll() + var resolvedPerms ACLResolveResult args := &structs.RegisterRequest{ Node: "nope", @@ -3464,9 +3476,10 @@ func TestVetRegisterWithACL(t *testing.T) { node "node" { policy = "write" } `) + resolvedPerms = ACLResolveResult{Authorizer: perms} // With that policy, the update should now be blocked for node reasons. - err := vetRegisterWithACL(perms, args, nil) + err := vetRegisterWithACL(resolvedPerms, args, nil) require.True(t, acl.IsErrPermissionDenied(err)) // Now use a permitted node name. @@ -3474,7 +3487,7 @@ func TestVetRegisterWithACL(t *testing.T) { Node: "node", Address: "127.0.0.1", } - require.NoError(t, vetRegisterWithACL(perms, args, nil)) + require.NoError(t, vetRegisterWithACL(resolvedPerms, args, nil)) // Build some node info that matches what we have now. ns := &structs.NodeServices{ @@ -3494,7 +3507,7 @@ func TestVetRegisterWithACL(t *testing.T) { ID: "my-id", }, } - err = vetRegisterWithACL(perms, args, ns) + err = vetRegisterWithACL(ACLResolveResult{Authorizer: perms}, args, ns) require.True(t, acl.IsErrPermissionDenied(err)) // Chain on a basic service policy. @@ -3502,9 +3515,10 @@ func TestVetRegisterWithACL(t *testing.T) { service "service" { policy = "write" } `) + resolvedPerms = ACLResolveResult{Authorizer: perms} // With the service ACL, the update should go through. - require.NoError(t, vetRegisterWithACL(perms, args, ns)) + require.NoError(t, vetRegisterWithACL(resolvedPerms, args, ns)) // Add an existing service that they are clobbering and aren't allowed // to write to. @@ -3520,7 +3534,7 @@ func TestVetRegisterWithACL(t *testing.T) { }, }, } - err = vetRegisterWithACL(perms, args, ns) + err = vetRegisterWithACL(resolvedPerms, args, ns) require.True(t, acl.IsErrPermissionDenied(err)) // Chain on a policy that allows them to write to the other service. @@ -3528,14 +3542,15 @@ func TestVetRegisterWithACL(t *testing.T) { service "other" { policy = "write" } `) + resolvedPerms = ACLResolveResult{Authorizer: perms} // Now it should go through. - require.NoError(t, vetRegisterWithACL(perms, args, ns)) + require.NoError(t, vetRegisterWithACL(resolvedPerms, args, ns)) // Try creating the node and the service at once by having no existing // node record. This should be ok since we have node and service // permissions. - require.NoError(t, vetRegisterWithACL(perms, args, nil)) + require.NoError(t, vetRegisterWithACL(resolvedPerms, args, nil)) // Add a node-level check to the member, which should be rejected. args = &structs.RegisterRequest{ @@ -3549,7 +3564,7 @@ func TestVetRegisterWithACL(t *testing.T) { Node: "node", }, } - err = vetRegisterWithACL(perms, args, ns) + err = vetRegisterWithACL(resolvedPerms, args, ns) testutil.RequireErrorContains(t, err, "check member must be nil") // Move the check into the slice, but give a bad node name. @@ -3566,7 +3581,7 @@ func TestVetRegisterWithACL(t *testing.T) { }, }, } - err = vetRegisterWithACL(perms, args, ns) + err = vetRegisterWithACL(resolvedPerms, args, ns) testutil.RequireErrorContains(t, err, "doesn't match register request node") // Fix the node name, which should now go through. @@ -3583,7 +3598,7 @@ func TestVetRegisterWithACL(t *testing.T) { }, }, } - require.NoError(t, vetRegisterWithACL(perms, args, ns)) + require.NoError(t, vetRegisterWithACL(resolvedPerms, args, ns)) // Add a service-level check. args = &structs.RegisterRequest{ @@ -3603,12 +3618,12 @@ func TestVetRegisterWithACL(t *testing.T) { }, }, } - require.NoError(t, vetRegisterWithACL(perms, args, ns)) + require.NoError(t, vetRegisterWithACL(resolvedPerms, args, ns)) // Try creating everything at once. This should be ok since we have all // the permissions we need. It also makes sure that we can register a // new node, service, and associated checks. - require.NoError(t, vetRegisterWithACL(perms, args, nil)) + require.NoError(t, vetRegisterWithACL(resolvedPerms, args, nil)) // Nil out the service registration, which'll skip the special case // and force us to look at the ns data (it will look like we are @@ -3626,16 +3641,17 @@ func TestVetRegisterWithACL(t *testing.T) { }, }, } - require.NoError(t, vetRegisterWithACL(perms, args, ns)) + require.NoError(t, vetRegisterWithACL(resolvedPerms, args, ns)) // Chain on a policy that forbids them to write to the other service. perms = appendAuthz(t, perms, ` service "other" { policy = "deny" } `) + resolvedPerms = ACLResolveResult{Authorizer: perms} // This should get rejected. - err = vetRegisterWithACL(perms, args, ns) + err = vetRegisterWithACL(resolvedPerms, args, ns) require.True(t, acl.IsErrPermissionDenied(err)) // Change the existing service data to point to a service name they @@ -3652,16 +3668,17 @@ func TestVetRegisterWithACL(t *testing.T) { }, }, } - require.NoError(t, vetRegisterWithACL(perms, args, ns)) + require.NoError(t, vetRegisterWithACL(resolvedPerms, args, ns)) // Chain on a policy that forbids them to write to the node. perms = appendAuthz(t, perms, ` node "node" { policy = "deny" } `) + resolvedPerms = ACLResolveResult{Authorizer: perms} // This should get rejected because there's a node-level check in here. - err = vetRegisterWithACL(perms, args, ns) + err = vetRegisterWithACL(resolvedPerms, args, ns) require.True(t, acl.IsErrPermissionDenied(err)) // Change the node-level check into a service check, and then it should @@ -3680,7 +3697,7 @@ func TestVetRegisterWithACL(t *testing.T) { }, }, } - require.NoError(t, vetRegisterWithACL(perms, args, ns)) + require.NoError(t, vetRegisterWithACL(resolvedPerms, args, ns)) // Finally, attempt to update the node part of the data and make sure // that gets rejected since they no longer have permissions. @@ -3698,7 +3715,7 @@ func TestVetRegisterWithACL(t *testing.T) { }, }, } - err = vetRegisterWithACL(perms, args, ns) + err = vetRegisterWithACL(resolvedPerms, args, ns) require.True(t, acl.IsErrPermissionDenied(err)) } @@ -3709,7 +3726,7 @@ func TestVetDeregisterWithACL(t *testing.T) { } // With an "allow all" authorizer the update should be allowed. - if err := vetDeregisterWithACL(acl.ManageAll(), args, nil, nil); err != nil { + if err := vetDeregisterWithACL(ACLResolveResult{Authorizer: acl.ManageAll()}, args, nil, nil); err != nil { t.Fatalf("err: %v", err) } @@ -3942,7 +3959,7 @@ node "node" { }, } { t.Run(args.Name, func(t *testing.T) { - err = vetDeregisterWithACL(args.Perms, &args.DeregisterRequest, args.Service, args.Check) + err = vetDeregisterWithACL(ACLResolveResult{Authorizer: args.Perms}, &args.DeregisterRequest, args.Service, args.Check) if !args.Expected { if err == nil { t.Errorf("expected error with %+v", args.DeregisterRequest) diff --git a/agent/consul/kvs_endpoint.go b/agent/consul/kvs_endpoint.go index d90e740c6..24ee58e7e 100644 --- a/agent/consul/kvs_endpoint.go +++ b/agent/consul/kvs_endpoint.go @@ -32,7 +32,7 @@ type KVS struct { // preApply does all the verification of a KVS update that is performed BEFORE // we submit as a Raft log entry. This includes enforcing the lock delay which // must only be done on the leader. -func kvsPreApply(logger hclog.Logger, srv *Server, authz acl.Authorizer, op api.KVOp, dirEnt *structs.DirEntry) (bool, error) { +func kvsPreApply(logger hclog.Logger, srv *Server, authz ACLResolveResult, op api.KVOp, dirEnt *structs.DirEntry) (bool, error) { // Verify the entry. if dirEnt.Key == "" && op != api.KVDeleteTree { return false, fmt.Errorf("Must provide key") diff --git a/agent/consul/txn_endpoint.go b/agent/consul/txn_endpoint.go index 6b1e65163..06b928088 100644 --- a/agent/consul/txn_endpoint.go +++ b/agent/consul/txn_endpoint.go @@ -32,7 +32,7 @@ type Txn struct { // preCheck is used to verify the incoming operations before any further // processing takes place. This checks things like ACLs. -func (t *Txn) preCheck(authorizer acl.Authorizer, ops structs.TxnOps) structs.TxnErrors { +func (t *Txn) preCheck(authorizer ACLResolveResult, ops structs.TxnOps) structs.TxnErrors { var errors structs.TxnErrors // Perform the pre-apply checks for any KV operations. @@ -109,7 +109,7 @@ func (t *Txn) preCheck(authorizer acl.Authorizer, ops structs.TxnOps) structs.Tx } // vetNodeTxnOp applies the given ACL policy to a node transaction operation. -func vetNodeTxnOp(op *structs.TxnNodeOp, authz acl.Authorizer) error { +func vetNodeTxnOp(op *structs.TxnNodeOp, authz ACLResolveResult) error { var authzContext acl.AuthorizerContext op.FillAuthzContext(&authzContext) @@ -120,7 +120,7 @@ func vetNodeTxnOp(op *structs.TxnNodeOp, authz acl.Authorizer) error { } // vetCheckTxnOp applies the given ACL policy to a check transaction operation. -func vetCheckTxnOp(op *structs.TxnCheckOp, authz acl.Authorizer) error { +func vetCheckTxnOp(op *structs.TxnCheckOp, authz ACLResolveResult) error { var authzContext acl.AuthorizerContext op.FillAuthzContext(&authzContext) diff --git a/agent/consul/txn_endpoint_test.go b/agent/consul/txn_endpoint_test.go index 3c0defc5e..c48d1a035 100644 --- a/agent/consul/txn_endpoint_test.go +++ b/agent/consul/txn_endpoint_test.go @@ -345,7 +345,8 @@ func TestTxn_Apply_ACLDeny(t *testing.T) { check := structs.HealthCheck{Node: "nope", CheckID: types.CheckID("nope")} state.EnsureCheck(4, &check) - id := createToken(t, rpcClient(t, s1), testTxnRules) + token := createTokenFull(t, rpcClient(t, s1), testTxnRules) + id := token.SecretID // Set up a transaction where every operation should get blocked due to // ACLs. @@ -564,11 +565,11 @@ func TestTxn_Apply_ACLDeny(t *testing.T) { // These get filtered but won't result in an error. case api.KVSet, api.KVDelete, api.KVDeleteCAS, api.KVDeleteTree, api.KVCAS, api.KVLock, api.KVUnlock, api.KVCheckNotExists: require.Equal(t, err.OpIndex, i) - acl.RequirePermissionDeniedMessage(t, err.What, nil, nil, acl.ResourceKey, acl.AccessWrite, "nope") + acl.RequirePermissionDeniedMessage(t, err.What, token.AccessorID, nil, acl.ResourceKey, acl.AccessWrite, "nope") outPos++ default: require.Equal(t, err.OpIndex, i) - acl.RequirePermissionDeniedMessage(t, err.What, nil, nil, acl.ResourceKey, acl.AccessRead, "nope") + acl.RequirePermissionDeniedMessage(t, err.What, token.AccessorID, nil, acl.ResourceKey, acl.AccessRead, "nope") outPos++ } case op.Node != nil: @@ -577,11 +578,11 @@ func TestTxn_Apply_ACLDeny(t *testing.T) { // These get filtered but won't result in an error. case api.NodeSet, api.NodeDelete, api.NodeDeleteCAS, api.NodeCAS: require.Equal(t, err.OpIndex, i) - acl.RequirePermissionDeniedMessage(t, err.What, nil, nil, acl.ResourceNode, acl.AccessWrite, "nope") + acl.RequirePermissionDeniedMessage(t, err.What, token.AccessorID, nil, acl.ResourceNode, acl.AccessWrite, "nope") outPos++ default: require.Equal(t, err.OpIndex, i) - acl.RequirePermissionDeniedMessage(t, err.What, nil, nil, acl.ResourceNode, acl.AccessRead, "nope") + acl.RequirePermissionDeniedMessage(t, err.What, token.AccessorID, nil, acl.ResourceNode, acl.AccessRead, "nope") outPos++ } case op.Service != nil: @@ -590,11 +591,11 @@ func TestTxn_Apply_ACLDeny(t *testing.T) { // These get filtered but won't result in an error. case api.ServiceSet, api.ServiceCAS, api.ServiceDelete, api.ServiceDeleteCAS: require.Equal(t, err.OpIndex, i) - acl.RequirePermissionDeniedMessage(t, err.What, nil, nil, acl.ResourceService, acl.AccessWrite, "nope") + acl.RequirePermissionDeniedMessage(t, err.What, token.AccessorID, nil, acl.ResourceService, acl.AccessWrite, "nope") outPos++ default: require.Equal(t, err.OpIndex, i) - acl.RequirePermissionDeniedMessage(t, err.What, nil, nil, acl.ResourceService, acl.AccessRead, "nope") + acl.RequirePermissionDeniedMessage(t, err.What, token.AccessorID, nil, acl.ResourceService, acl.AccessRead, "nope") outPos++ } case op.Check != nil: @@ -603,11 +604,11 @@ func TestTxn_Apply_ACLDeny(t *testing.T) { // These get filtered but won't result in an error. case api.CheckSet, api.CheckCAS, api.CheckDelete, api.CheckDeleteCAS: require.Equal(t, err.OpIndex, i) - acl.RequirePermissionDeniedMessage(t, err.What, nil, nil, acl.ResourceNode, acl.AccessWrite, "nope") + acl.RequirePermissionDeniedMessage(t, err.What, token.AccessorID, nil, acl.ResourceNode, acl.AccessWrite, "nope") outPos++ default: require.Equal(t, err.OpIndex, i) - acl.RequirePermissionDeniedMessage(t, err.What, nil, nil, acl.ResourceNode, acl.AccessRead, "nope") + acl.RequirePermissionDeniedMessage(t, err.What, token.AccessorID, nil, acl.ResourceNode, acl.AccessRead, "nope") outPos++ } } @@ -873,7 +874,8 @@ func TestTxn_Read_ACLDeny(t *testing.T) { check := structs.HealthCheck{Node: "nope", CheckID: types.CheckID("nope")} state.EnsureCheck(4, &check) - id := createToken(t, codec, testTxnRules) + token := createTokenFull(t, codec, testTxnRules) + id := token.AccessorID t.Run("simple read operations (results get filtered out)", func(t *testing.T) { arg := structs.TxnReadRequest{ @@ -934,8 +936,8 @@ func TestTxn_Read_ACLDeny(t *testing.T) { var out structs.TxnReadResponse err := msgpackrpc.CallWithCodec(codec, "Txn.Read", &arg, &out) require.NoError(t, err) - acl.RequirePermissionDeniedMessage(t, out.Errors[0].What, nil, nil, acl.ResourceKey, acl.AccessRead, "nope") - acl.RequirePermissionDeniedMessage(t, out.Errors[1].What, nil, nil, acl.ResourceKey, acl.AccessRead, "nope") + acl.RequirePermissionDeniedMessage(t, out.Errors[0].What, token.AccessorID, nil, acl.ResourceKey, acl.AccessRead, "nope") + acl.RequirePermissionDeniedMessage(t, out.Errors[1].What, token.AccessorID, nil, acl.ResourceKey, acl.AccessRead, "nope") require.Empty(t, out.Results) }) diff --git a/agent/structs/config_entry.go b/agent/structs/config_entry.go index 69e823d1f..7222a1ec6 100644 --- a/agent/structs/config_entry.go +++ b/agent/structs/config_entry.go @@ -60,6 +60,7 @@ type ConfigEntry interface { // CanRead and CanWrite return whether or not the given Authorizer // has permission to read or write to the config entry, respectively. + // TODO(acl-error-enhancements) This should be ACLResolveResult or similar but we have to wait until we move things to the acl package CanRead(acl.Authorizer) error CanWrite(acl.Authorizer) error