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 <manderson@hashicorp.com>

.changelog/12567.txt

@@ -0,0 +1,3 @@
+```release-note:feature
+acl: Add token information to PermissionDeniedErrors
+```

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

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
 }

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]

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 {

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

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,

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)

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")

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)
 

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[0].What, token.AccessorID, 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[1].What, token.AccessorID, nil, acl.ResourceKey, acl.AccessRead, "nope")
 
 		require.Empty(t, out.Results)
 	})

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