Bulk acl message fixup oss (#12470)

* First pass for helper for bulk changes

Signed-off-by: Mark Anderson <manderson@hashicorp.com>

* Convert ACLRead and ACLWrite to new form

Signed-off-by: Mark Anderson <manderson@hashicorp.com>

* AgentRead and AgentWRite

Signed-off-by: Mark Anderson <manderson@hashicorp.com>

* Fix EventWrite

Signed-off-by: Mark Anderson <manderson@hashicorp.com>

* KeyRead, KeyWrite, KeyList

Signed-off-by: Mark Anderson <manderson@hashicorp.com>

* KeyRing

Signed-off-by: Mark Anderson <manderson@hashicorp.com>

* NodeRead NodeWrite

Signed-off-by: Mark Anderson <manderson@hashicorp.com>

* OperatorRead and OperatorWrite

Signed-off-by: Mark Anderson <manderson@hashicorp.com>

* PreparedQuery

Signed-off-by: Mark Anderson <manderson@hashicorp.com>

* Intention partial

Signed-off-by: Mark Anderson <manderson@hashicorp.com>

* Fix ServiceRead, Write ,etc

Signed-off-by: Mark Anderson <manderson@hashicorp.com>

* Error check ServiceRead?

Signed-off-by: Mark Anderson <manderson@hashicorp.com>

* Fix Sessionread/Write

Signed-off-by: Mark Anderson <manderson@hashicorp.com>

* Fixup snapshot ACL

Signed-off-by: Mark Anderson <manderson@hashicorp.com>

* Error fixups for txn

Signed-off-by: Mark Anderson <manderson@hashicorp.com>

* Add changelog

Signed-off-by: Mark Anderson <manderson@hashicorp.com>

* Fixup review comments

Signed-off-by: Mark Anderson <manderson@hashicorp.com>

.changelog/12470.txt

@@ -0,0 +1,3 @@
+```release-note:enhancement
+acl: Provide fuller detail in the error messsage when an ACL denies access.
+```

acl/authorizer.go

@@ -161,6 +161,279 @@ type Authorizer interface {
 
 	// Embedded Interface for Consul Enterprise specific ACL enforcement
 	enterpriseAuthorizer
+
+	// ToAllowAuthorizer is needed until we can use ResolveResult in all the places this interface is used.
+	ToAllowAuthorizer() AllowAuthorizer
+}
+
+// AllowAuthorizer is a wrapper to expose the *Allowed methods.
+// This and the ToAllowAuthorizer function exist to tide us over until the ResolveResult struct
+// is moved into acl.
+type AllowAuthorizer struct {
+	Authorizer
+}
+
+// ACLReadAllowed checks for permission to list all the ACLs
+func (a AllowAuthorizer) ACLReadAllowed(ctx *AuthorizerContext) error {
+	if a.Authorizer.ACLRead(ctx) != Allow {
+		return PermissionDeniedByACLUnnamed(a, ctx, ResourceACL, AccessRead)
+	}
+	return nil
+}
+
+// ACLWriteAllowed checks for permission to manipulate ACLs
+func (a AllowAuthorizer) ACLWriteAllowed(ctx *AuthorizerContext) error {
+	if a.Authorizer.ACLWrite(ctx) != Allow {
+		return PermissionDeniedByACLUnnamed(a, ctx, ResourceACL, AccessWrite)
+	}
+	return nil
+}
+
+// AgentReadAllowed checks for permission to read from agent endpoints for a
+// given node.
+func (a AllowAuthorizer) AgentReadAllowed(name string, ctx *AuthorizerContext) error {
+	if a.Authorizer.AgentRead(name, ctx) != Allow {
+		return PermissionDeniedByACL(a, ctx, ResourceAgent, AccessRead, name)
+	}
+	return nil
+}
+
+// AgentWriteAllowed checks for permission to make changes via agent endpoints
+// for a given node.
+func (a AllowAuthorizer) AgentWriteAllowed(name string, ctx *AuthorizerContext) error {
+	if a.Authorizer.AgentWrite(name, ctx) != Allow {
+		return PermissionDeniedByACL(a, ctx, ResourceAgent, AccessWrite, name)
+	}
+	return nil
+}
+
+// EventReadAllowed determines if a specific event can be queried.
+func (a AllowAuthorizer) EventReadAllowed(name string, ctx *AuthorizerContext) error {
+	if a.Authorizer.EventRead(name, ctx) != Allow {
+		return PermissionDeniedByACL(a, ctx, ResourceEvent, AccessRead, name)
+	}
+	return nil
+}
+
+// EventWriteAllowed determines if a specific event may be fired.
+func (a AllowAuthorizer) EventWriteAllowed(name string, ctx *AuthorizerContext) error {
+	if a.Authorizer.EventWrite(name, ctx) != Allow {
+		return PermissionDeniedByACL(a, ctx, ResourceEvent, AccessWrite, name)
+	}
+	return nil
+}
+
+// IntentionDefaultAllowAllowed determines the default authorized behavior
+// when no intentions match a Connect request.
+func (a AllowAuthorizer) IntentionDefaultAllowAllowed(ctx *AuthorizerContext) error {
+	if a.Authorizer.IntentionDefaultAllow(ctx) != Allow {
+		// This is a bit nuanced, in that this isn't set by a rule, but inherited globally
+		// TODO(acl-error-enhancements) revisit when we have full accessor info
+		return PermissionDeniedError{Cause: "Denied by intention default"}
+	}
+	return nil
+}
+
+// IntentionReadAllowed determines if a specific intention can be read.
+func (a AllowAuthorizer) IntentionReadAllowed(name string, ctx *AuthorizerContext) error {
+	if a.Authorizer.IntentionRead(name, ctx) != Allow {
+		return PermissionDeniedByACL(a, ctx, ResourceIntention, AccessRead, name)
+	}
+	return nil
+}
+
+// IntentionWriteAllowed determines if a specific intention can be
+// created, modified, or deleted.
+func (a AllowAuthorizer) IntentionWriteAllowed(name string, ctx *AuthorizerContext) error {
+	if a.Authorizer.IntentionWrite(name, ctx) != Allow {
+		return PermissionDeniedByACL(a, ctx, ResourceIntention, AccessWrite, name)
+	}
+	return nil
+}
+
+// KeyListAllowed checks for permission to list keys under a prefix
+func (a AllowAuthorizer) KeyListAllowed(name string, ctx *AuthorizerContext) error {
+	if a.Authorizer.KeyList(name, ctx) != Allow {
+		return PermissionDeniedByACL(a, ctx, ResourceKey, AccessList, name)
+	}
+	return nil
+}
+
+// KeyReadAllowed checks for permission to read a given key
+func (a AllowAuthorizer) KeyReadAllowed(name string, ctx *AuthorizerContext) error {
+	if a.Authorizer.KeyRead(name, ctx) != Allow {
+		return PermissionDeniedByACL(a, ctx, ResourceKey, AccessRead, name)
+	}
+	return nil
+}
+
+// KeyWriteAllowed checks for permission to write a given key
+func (a AllowAuthorizer) KeyWriteAllowed(name string, ctx *AuthorizerContext) error {
+	if a.Authorizer.KeyWrite(name, ctx) != Allow {
+		return PermissionDeniedByACL(a, ctx, ResourceKey, AccessWrite, name)
+	}
+	return nil
+}
+
+// KeyWritePrefixAllowed checks for permission to write to an
+// entire key prefix. This means there must be no sub-policies
+// that deny a write.
+func (a AllowAuthorizer) KeyWritePrefixAllowed(name string, ctx *AuthorizerContext) error {
+	if a.Authorizer.KeyWritePrefix(name, ctx) != Allow {
+		// TODO(acl-error-enhancements) revisit this message; we may need to do some extra plumbing inside of KeyWritePrefix to
+		// return properly detailed information.
+		return PermissionDeniedByACL(a, ctx, ResourceKey, AccessWrite, name)
+	}
+	return nil
+}
+
+// KeyringReadAllowed determines if the encryption keyring used in
+// the gossip layer can be read.
+func (a AllowAuthorizer) KeyringReadAllowed(ctx *AuthorizerContext) error {
+	if a.Authorizer.KeyringRead(ctx) != Allow {
+		return PermissionDeniedByACLUnnamed(a, ctx, ResourceKeyring, AccessRead)
+	}
+	return nil
+}
+
+// KeyringWriteAllowed determines if the keyring can be manipulated
+func (a AllowAuthorizer) KeyringWriteAllowed(ctx *AuthorizerContext) error {
+	if a.Authorizer.KeyringWrite(ctx) != Allow {
+		return PermissionDeniedByACLUnnamed(a, ctx, ResourceKeyring, AccessWrite)
+	}
+	return nil
+}
+
+// MeshReadAllowed determines if the read-only Consul mesh functions
+// can be used.
+func (a AllowAuthorizer) MeshReadAllowed(ctx *AuthorizerContext) error {
+	if a.Authorizer.MeshRead(ctx) != Allow {
+		return PermissionDeniedByACLUnnamed(a, ctx, ResourceMesh, AccessRead)
+	}
+	return nil
+}
+
+// MeshWriteAllowed determines if the state-changing Consul mesh
+// functions can be used.
+func (a AllowAuthorizer) MeshWriteAllowed(ctx *AuthorizerContext) error {
+	if a.Authorizer.MeshWrite(ctx) != Allow {
+		return PermissionDeniedByACLUnnamed(a, ctx, ResourceMesh, AccessWrite)
+	}
+	return nil
+}
+
+// NodeReadAllowed checks for permission to read (discover) a given node.
+func (a AllowAuthorizer) NodeReadAllowed(name string, ctx *AuthorizerContext) error {
+	if a.Authorizer.NodeRead(name, ctx) != Allow {
+		return PermissionDeniedByACL(a, ctx, ResourceNode, AccessRead, name)
+	}
+	return nil
+}
+
+// NodeReadAllAllowed checks for permission to read (discover) all nodes.
+func (a AllowAuthorizer) NodeReadAllAllowed(ctx *AuthorizerContext) error {
+	if a.Authorizer.NodeReadAll(ctx) != Allow {
+		// This is only used to gate certain UI functions right now (e.g metrics)
+		return PermissionDeniedByACL(a, ctx, ResourceNode, AccessRead, "all nodes")
+	}
+	return nil
+}
+
+// NodeWriteAllowed checks for permission to create or update (register) a
+// given node.
+func (a AllowAuthorizer) NodeWriteAllowed(name string, ctx *AuthorizerContext) error {
+	if a.Authorizer.NodeWrite(name, ctx) != Allow {
+		return PermissionDeniedByACL(a, ctx, ResourceNode, AccessWrite, name)
+	}
+	return nil
+}
+
+// OperatorReadAllowed determines if the read-only Consul operator functions
+// can be used.
+func (a AllowAuthorizer) OperatorReadAllowed(ctx *AuthorizerContext) error {
+	if a.Authorizer.OperatorRead(ctx) != Allow {
+		return PermissionDeniedByACLUnnamed(a, ctx, ResourceOperator, AccessRead)
+	}
+	return nil
+}
+
+// OperatorWriteAllowed determines if the state-changing Consul operator
+// functions can be used.
+func (a AllowAuthorizer) OperatorWriteAllowed(ctx *AuthorizerContext) error {
+	if a.Authorizer.OperatorWrite(ctx) != Allow {
+		return PermissionDeniedByACLUnnamed(a, ctx, ResourceOperator, AccessWrite)
+	}
+	return nil
+}
+
+// PreparedQueryReadAllowed determines if a specific prepared query can be read
+// to show its contents (this is not used for execution).
+func (a AllowAuthorizer) PreparedQueryReadAllowed(name string, ctx *AuthorizerContext) error {
+	if a.Authorizer.PreparedQueryRead(name, ctx) != Allow {
+		return PermissionDeniedByACL(a, ctx, ResourceQuery, AccessRead, name)
+	}
+	return nil
+}
+
+// PreparedQueryWriteAllowed determines if a specific prepared query can be
+// created, modified, or deleted.
+func (a AllowAuthorizer) PreparedQueryWriteAllowed(name string, ctx *AuthorizerContext) error {
+	if a.Authorizer.PreparedQueryWrite(name, ctx) != Allow {
+		return PermissionDeniedByACL(a, ctx, ResourceQuery, AccessWrite, name)
+	}
+	return nil
+}
+
+// ServiceReadAllowed checks for permission to read a given service
+func (a AllowAuthorizer) ServiceReadAllowed(name string, ctx *AuthorizerContext) error {
+	if a.Authorizer.ServiceRead(name, ctx) != Allow {
+		return PermissionDeniedByACL(a, ctx, ResourceService, AccessRead, name)
+	}
+	return nil
+}
+
+// ServiceReadAllAllowed checks for permission to read all services
+func (a AllowAuthorizer) ServiceReadAllAllowed(ctx *AuthorizerContext) error {
+	if a.Authorizer.ServiceReadAll(ctx) != Allow {
+		// This is only used to gate certain UI functions right now (e.g metrics)
+		return PermissionDeniedByACL(a, ctx, ResourceService, AccessRead, "all services") // read
+	}
+	return nil
+}
+
+// ServiceWriteAllowed checks for permission to create or update a given
+// service
+func (a AllowAuthorizer) ServiceWriteAllowed(name string, ctx *AuthorizerContext) error {
+	if a.Authorizer.ServiceWrite(name, ctx) != Allow {
+		return PermissionDeniedByACL(a, ctx, ResourceService, AccessWrite, name)
+	}
+	return nil
+}
+
+// SessionReadAllowed checks for permission to read sessions for a given node.
+func (a AllowAuthorizer) SessionReadAllowed(name string, ctx *AuthorizerContext) error {
+	if a.Authorizer.SessionRead(name, ctx) != Allow {
+		return PermissionDeniedByACL(a, ctx, ResourceSession, AccessRead, name)
+	}
+	return nil
+}
+
+// SessionWriteAllowed checks for permission to create sessions for a given
+// node.
+func (a AllowAuthorizer) SessionWriteAllowed(name string, ctx *AuthorizerContext) error {
+	if a.Authorizer.SessionWrite(name, ctx) != Allow {
+		return PermissionDeniedByACL(a, ctx, ResourceSession, AccessWrite, name)
+	}
+	return nil
+}
+
+// SnapshotAllowed checks for permission to take and restore snapshots.
+func (a AllowAuthorizer) SnapshotAllowed(ctx *AuthorizerContext) error {
+	if a.Authorizer.Snapshot(ctx) != Allow {
+		// Implementation of this currently just checks acl write
+		return PermissionDeniedByACLUnnamed(a, ctx, ResourceACL, AccessWrite)
+	}
+	return nil
 }
 
 func Enforce(authz Authorizer, rsc Resource, segment string, access string, ctx *AuthorizerContext) (EnforcementDecision, error) {

acl/authorizer_test.go

@@ -204,6 +204,10 @@ func (m *mockAuthorizer) Snapshot(ctx *AuthorizerContext) EnforcementDecision {
 	return ret.Get(0).(EnforcementDecision)
 }
 
+func (p *mockAuthorizer) ToAllowAuthorizer() AllowAuthorizer {
+	return AllowAuthorizer{Authorizer: p}
+}
+
 func TestACL_Enforce(t *testing.T) {
 	type testCase struct {
 		method   string

acl/chained_authorizer.go

@@ -256,3 +256,7 @@ func (c *ChainedAuthorizer) Snapshot(entCtx *AuthorizerContext) EnforcementDecis
 		return authz.Snapshot(entCtx)
 	})
 }
+
+func (c *ChainedAuthorizer) ToAllowAuthorizer() AllowAuthorizer {
+	return AllowAuthorizer{Authorizer: c}
+}

acl/chained_authorizer_test.go

@@ -99,6 +99,10 @@ func (authz testAuthorizer) Snapshot(*AuthorizerContext) EnforcementDecision {
 	return EnforcementDecision(authz)
 }
 
+func (authz testAuthorizer) ToAllowAuthorizer() AllowAuthorizer {
+	return AllowAuthorizer{Authorizer: &authz}
+}
+
 func TestChainedAuthorizer(t *testing.T) {
 	t.Run("No Authorizers", func(t *testing.T) {
 		authz := NewChainedAuthorizer([]Authorizer{})

acl/errors.go

@@ -106,7 +106,7 @@ func (e PermissionDeniedError) Error() string {
 	fmt.Fprintf(&message, " lacks permission '%s:%s'", e.Resource, e.AccessLevel.String())
 
 	if e.ResourceID.Name != "" {
-		fmt.Fprintf(&message, " %s", e.ResourceID.ToString())
+		fmt.Fprintf(&message, " on %s", e.ResourceID.ToString())
 	}
 	return message.String()
 }

acl/errors_test.go

@@ -29,7 +29,7 @@ func TestPermissionDeniedError(t *testing.T) {
 		},
 		{
 			err:      PermissionDeniedByACL(&auth1, nil, ResourceService, AccessRead, "foobar"),
-			expected: "Permission denied: provided accessor lacks permission 'service:read' foobar",
+			expected: "Permission denied: provided accessor lacks permission 'service:read' on foobar",
 		},
 		{
 			err:      PermissionDeniedByACLUnnamed(&auth1, nil, ResourceService, AccessRead),

acl/policy_authorizer.go

@@ -787,3 +787,7 @@ func (p *policyAuthorizer) SessionWrite(node string, _ *AuthorizerContext) Enfor
 	}
 	return Default
 }
+
+func (p *policyAuthorizer) ToAllowAuthorizer() AllowAuthorizer {
+	return AllowAuthorizer{Authorizer: p}
+}

acl/static_authorizer.go

@@ -240,6 +240,10 @@ func (s *staticAuthorizer) Snapshot(_ *AuthorizerContext) EnforcementDecision {
 	return Deny
 }
 
+func (s *staticAuthorizer) ToAllowAuthorizer() AllowAuthorizer {
+	return AllowAuthorizer{Authorizer: s}
+}
+
 // AllowAll returns an Authorizer that allows all operations
 func AllowAll() Authorizer {
 	return allowAll

acl/testing.go

@@ -0,0 +1,47 @@
+package acl
+
+import (
+	"github.com/stretchr/testify/require"
+	"regexp"
+	"testing"
+)
+
+func RequirePermissionDeniedError(t testing.TB, err error, _ Authorizer, _ *AuthorizerContext, resource Resource, accessLevel AccessLevel, resourceID string) {
+	t.Helper()
+	if err == nil {
+		t.Fatal("An error is expected but got nil.")
+	}
+	if v, ok := err.(PermissionDeniedError); ok {
+		require.Equal(t, v.Resource, resource)
+		require.Equal(t, v.AccessLevel, accessLevel)
+		require.Equal(t, v.ResourceID.Name, resourceID)
+	} else {
+		t.Fatalf("Expected a permission denied error got %T %vp", err, err)
+	}
+}
+
+func RequirePermissionDeniedMessage(t testing.TB, msg string, auth Authorizer, _ *AuthorizerContext, resource Resource, accessLevel AccessLevel, resourceID string) {
+	require.NotEmpty(t, msg, "expected non-empty error message")
+
+	var resourceIDFound string
+	if auth == nil {
+		expr := "^Permission denied" + `: provided accessor lacks permission '(\S*):(\S*)' on (.*)\s*$`
+		re, _ := regexp.Compile(expr)
+		matched := re.FindStringSubmatch(msg)
+
+		require.Equal(t, string(resource), matched[1], "resource")
+		require.Equal(t, accessLevel.String(), matched[2], "access level")
+		resourceIDFound = matched[3]
+	} else {
+		expr := "^Permission denied" + `: accessor '(\S*)' lacks permission '(\S*):(\S*)' on (.*)\s*$`
+		re, _ := regexp.Compile(expr)
+		matched := re.FindStringSubmatch(msg)
+
+		require.Equal(t, auth, matched[1], "auth")
+		require.Equal(t, string(resource), matched[2], "resource")
+		require.Equal(t, accessLevel.String(), matched[3], "access level")
+		resourceIDFound = matched[4]
+	}
+	// AuthorizerContext information should be checked here
+	require.Contains(t, resourceIDFound, resourceID, "resource id")
+}

agent/acl.go

@@ -43,15 +43,15 @@ func (a *Agent) vetServiceRegisterWithAuthorizer(authz acl.Authorizer, service *
 
 	// Vet the service itself.
 	service.FillAuthzContext(&authzContext)
-	if authz.ServiceWrite(service.Service, &authzContext) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(service.Service, &authzContext); err != nil {
-		return acl.PermissionDeniedByACL(authz, &authzContext, acl.ResourceService, acl.AccessWrite, service.Service)
+		return err
 	}
 
 	// Vet any service that might be getting overwritten.
 	if existing := a.State.Service(service.CompoundServiceID()); existing != nil {
 		existing.FillAuthzContext(&authzContext)
-		if authz.ServiceWrite(existing.Service, &authzContext) != acl.Allow {
+		if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(existing.Service, &authzContext); err != nil {
-			return acl.PermissionDeniedByACL(authz, &authzContext, acl.ResourceService, acl.AccessWrite, existing.Service)
+			return err
 		}
 	}
 
@@ -59,8 +59,8 @@ func (a *Agent) vetServiceRegisterWithAuthorizer(authz acl.Authorizer, service *
 	// since it can be discovered as an instance of that service.
 	if service.Kind == structs.ServiceKindConnectProxy {
 		service.FillAuthzContext(&authzContext)
-		if authz.ServiceWrite(service.Proxy.DestinationServiceName, &authzContext) != acl.Allow {
+		if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(service.Proxy.DestinationServiceName, &authzContext); err != nil {
-			return acl.PermissionDeniedByACL(authz, &authzContext, acl.ResourceService, acl.AccessWrite, service.Proxy.DestinationServiceName)
+			return err
 		}
 	}
 
@@ -73,8 +73,9 @@ func (a *Agent) vetServiceUpdateWithAuthorizer(authz acl.Authorizer, serviceID s
 	// Vet any changes based on the existing services's info.
 	if existing := a.State.Service(serviceID); existing != nil {
 		existing.FillAuthzContext(&authzContext)
-		if authz.ServiceWrite(existing.Service, &authzContext) != acl.Allow {
+		if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(existing.Service, &authzContext); err != nil {
-			return acl.PermissionDeniedByACL(authz, &authzContext, acl.ResourceService, acl.AccessWrite, existing.Service)
+			return err
+
 		}
 	} else {
 		// Take care if modifying this error message.
@@ -95,13 +96,13 @@ func (a *Agent) vetCheckRegisterWithAuthorizer(authz acl.Authorizer, check *stru
 
 	// Vet the check itself.
 	if len(check.ServiceName) > 0 {
-		if authz.ServiceWrite(check.ServiceName, &authzContext) != acl.Allow {
+		if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(check.ServiceName, &authzContext); err != nil {
-			return acl.PermissionDeniedByACL(authz, &authzContext, acl.ResourceService, acl.AccessWrite, check.ServiceName)
+			return err
 		}
 	} else {
 		// N.B. Should this authzContext be derived from a.AgentEnterpriseMeta()
-		if authz.NodeWrite(a.config.NodeName, &authzContext) != acl.Allow {
+		if err := authz.ToAllowAuthorizer().NodeWriteAllowed(a.config.NodeName, &authzContext); err != nil {
-			return acl.PermissionDeniedByACL(authz, &authzContext, acl.ResourceNode, acl.AccessWrite, a.config.NodeName)
+			return err
 		}
 	}
 
@@ -109,13 +110,13 @@ func (a *Agent) vetCheckRegisterWithAuthorizer(authz acl.Authorizer, check *stru
 	if existing := a.State.Check(check.CompoundCheckID()); existing != nil {
 		if len(existing.ServiceName) > 0 {
 			// N.B. Should this authzContext be derived from existing.EnterpriseMeta?
-			if authz.ServiceWrite(existing.ServiceName, &authzContext) != acl.Allow {
+			if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(existing.ServiceName, &authzContext); err != nil {
-				return acl.PermissionDeniedByACL(authz, &authzContext, acl.ResourceService, acl.AccessWrite, existing.ServiceName)
+				return err
 			}
 		} else {
 			// N.B. Should this authzContext be derived from a.AgentEnterpriseMeta()
-			if authz.NodeWrite(a.config.NodeName, &authzContext) != acl.Allow {
+			if err := authz.ToAllowAuthorizer().NodeWriteAllowed(a.config.NodeName, &authzContext); err != nil {
-				return acl.PermissionDeniedByACL(authz, &authzContext, acl.ResourceNode, acl.AccessWrite, a.config.NodeName)
+				return err
 			}
 		}
 	}
@@ -130,12 +131,12 @@ func (a *Agent) vetCheckUpdateWithAuthorizer(authz acl.Authorizer, checkID struc
 	// Vet any changes based on the existing check's info.
 	if existing := a.State.Check(checkID); existing != nil {
 		if len(existing.ServiceName) > 0 {
-			if authz.ServiceWrite(existing.ServiceName, &authzContext) != acl.Allow {
+			if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(existing.ServiceName, &authzContext); err != nil {
-				return acl.PermissionDeniedByACL(authz, &authzContext, acl.ResourceService, acl.AccessWrite, existing.ServiceName)
+				return err
 			}
 		} else {
-			if authz.NodeWrite(a.config.NodeName, &authzContext) != acl.Allow {
+			if err := authz.ToAllowAuthorizer().NodeWriteAllowed(a.config.NodeName, &authzContext); err != nil {
-				return acl.PermissionDeniedByACL(authz, &authzContext, acl.ResourceNode, acl.AccessWrite, a.config.NodeName)
+				return err
 			}
 		}
 	} else {

agent/agent_endpoint.go

@@ -60,8 +60,8 @@ func (s *HTTPHandlers) AgentSelf(resp http.ResponseWriter, req *http.Request) (i
 	// Authorize using the agent's own enterprise meta, not the token.
 	var authzContext acl.AuthorizerContext
 	s.agent.AgentEnterpriseMeta().FillAuthzContext(&authzContext)
-	if authz.AgentRead(s.agent.config.NodeName, &authzContext) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().AgentReadAllowed(s.agent.config.NodeName, &authzContext); err != nil {
-		return nil, acl.ErrPermissionDenied
+		return nil, err
 	}
 
 	var cs lib.CoordinateSet
@@ -150,8 +150,8 @@ func (s *HTTPHandlers) AgentMetrics(resp http.ResponseWriter, req *http.Request)
 	// Authorize using the agent's own enterprise meta, not the token.
 	var authzContext acl.AuthorizerContext
 	s.agent.AgentEnterpriseMeta().FillAuthzContext(&authzContext)
-	if authz.AgentRead(s.agent.config.NodeName, &authzContext) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().AgentReadAllowed(s.agent.config.NodeName, &authzContext); err != nil {
-		return nil, acl.ErrPermissionDenied
+		return nil, err
 	}
 	if enablePrometheusOutput(req) {
 		if s.agent.config.Telemetry.PrometheusOpts.Expiration < 1 {
@@ -187,8 +187,8 @@ func (s *HTTPHandlers) AgentMetricsStream(resp http.ResponseWriter, req *http.Re
 	// Authorize using the agent's own enterprise meta, not the token.
 	var authzContext acl.AuthorizerContext
 	s.agent.AgentEnterpriseMeta().FillAuthzContext(&authzContext)
-	if authz.AgentRead(s.agent.config.NodeName, &authzContext) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().AgentReadAllowed(s.agent.config.NodeName, &authzContext); err != nil {
-		return nil, acl.ErrPermissionDenied
+		return nil, err
 	}
 
 	flusher, ok := resp.(http.Flusher)
@@ -240,8 +240,8 @@ func (s *HTTPHandlers) AgentReload(resp http.ResponseWriter, req *http.Request)
 	// Authorize using the agent's own enterprise meta, not the token.
 	var authzContext acl.AuthorizerContext
 	s.agent.AgentEnterpriseMeta().FillAuthzContext(&authzContext)
-	if authz.AgentWrite(s.agent.config.NodeName, &authzContext) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().AgentWriteAllowed(s.agent.config.NodeName, &authzContext); err != nil {
-		return nil, acl.ErrPermissionDenied
+		return nil, err
 	}
 
 	return nil, s.agent.ReloadConfig()
@@ -440,8 +440,8 @@ func (s *HTTPHandlers) AgentService(resp http.ResponseWriter, req *http.Request)
 			}
 			var authzContext acl.AuthorizerContext
 			svc.FillAuthzContext(&authzContext)
-			if authz.ServiceRead(svc.Service, &authzContext) != acl.Allow {
+			if err := authz.ToAllowAuthorizer().ServiceReadAllowed(svc.Service, &authzContext); err != nil {
-				return "", nil, acl.ErrPermissionDenied
+				return "", nil, err
 			}
 
 			// Calculate the content hash over the response, minus the hash field
@@ -621,8 +621,9 @@ func (s *HTTPHandlers) AgentJoin(resp http.ResponseWriter, req *http.Request) (i
 	// Authorize using the agent's own enterprise meta, not the token.
 	var authzContext acl.AuthorizerContext
 	s.agent.AgentEnterpriseMeta().FillAuthzContext(&authzContext)
-	if authz.AgentWrite(s.agent.config.NodeName, &authzContext) != acl.Allow {
+
-		return nil, acl.PermissionDeniedByACL(authz, &authzContext, acl.ResourceAgent, acl.AccessWrite, s.agent.config.NodeName)
+	if err := authz.ToAllowAuthorizer().AgentWriteAllowed(s.agent.config.NodeName, &authzContext); err != nil {
+		return nil, err
 	}
 
 	// Get the request partition and default to that of the agent.
@@ -666,8 +667,8 @@ func (s *HTTPHandlers) AgentLeave(resp http.ResponseWriter, req *http.Request) (
 	// Authorize using the agent's own enterprise meta, not the token.
 	var authzContext acl.AuthorizerContext
 	s.agent.AgentEnterpriseMeta().FillAuthzContext(&authzContext)
-	if authz.AgentWrite(s.agent.config.NodeName, &authzContext) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().AgentWriteAllowed(s.agent.config.NodeName, &authzContext); err != nil {
-		return nil, acl.ErrPermissionDenied
+		return nil, err
 	}
 
 	if err := s.agent.Leave(); err != nil {
@@ -685,8 +686,8 @@ func (s *HTTPHandlers) AgentForceLeave(resp http.ResponseWriter, req *http.Reque
 		return nil, err
 	}
 	// TODO(partitions): should this be possible in a partition?
-	if authz.OperatorWrite(nil) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().OperatorWriteAllowed(nil); err != nil {
-		return nil, acl.PermissionDeniedByACLUnnamed(authz, nil, acl.ResourceOperator, acl.AccessWrite)
+		return nil, err
 	}
 
 	// Get the request partition and default to that of the agent.
@@ -1007,8 +1008,8 @@ func (s *HTTPHandlers) AgentHealthServiceByID(resp http.ResponseWriter, req *htt
 	dc := s.agent.config.Datacenter
 
 	if service := s.agent.State.Service(sid); service != nil {
-		if authz.ServiceRead(service.Service, &authzContext) != acl.Allow {
+		if err := authz.ToAllowAuthorizer().ServiceReadAllowed(service.Service, &authzContext); err != nil {
-			return nil, acl.PermissionDeniedByACL(authz, &authzContext, acl.ResourceService, acl.AccessRead, service.Service)
+			return nil, err
 		}
 		code, status, healthChecks := agentHealthService(sid, s)
 		if returnTextPlain(req) {
@@ -1060,8 +1061,8 @@ func (s *HTTPHandlers) AgentHealthServiceByName(resp http.ResponseWriter, req *h
 		return nil, err
 	}
 
-	if authz.ServiceRead(serviceName, &authzContext) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().ServiceReadAllowed(serviceName, &authzContext); err != nil {
-		return nil, acl.PermissionDeniedByACL(authz, &authzContext, acl.ResourceService, acl.AccessRead, serviceName)
+		return nil, err
 	}
 
 	if !s.validateRequestPartition(resp, &entMeta) {
@@ -1374,8 +1375,8 @@ func (s *HTTPHandlers) AgentNodeMaintenance(resp http.ResponseWriter, req *http.
 
 	var authzContext acl.AuthorizerContext
 	s.agent.AgentEnterpriseMeta().FillAuthzContext(&authzContext)
-	if authz.NodeWrite(s.agent.config.NodeName, &authzContext) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().NodeWriteAllowed(s.agent.config.NodeName, &authzContext); err != nil {
-		return nil, acl.ErrPermissionDenied
+		return nil, err
 	}
 
 	if enable {
@@ -1399,8 +1400,8 @@ func (s *HTTPHandlers) AgentMonitor(resp http.ResponseWriter, req *http.Request)
 	// Authorize using the agent's own enterprise meta, not the token.
 	var authzContext acl.AuthorizerContext
 	s.agent.AgentEnterpriseMeta().FillAuthzContext(&authzContext)
-	if authz.AgentRead(s.agent.config.NodeName, &authzContext) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().AgentReadAllowed(s.agent.config.NodeName, &authzContext); err != nil {
-		return nil, acl.ErrPermissionDenied
+		return nil, err
 	}
 
 	// Get the provided loglevel.
@@ -1482,8 +1483,8 @@ func (s *HTTPHandlers) AgentToken(resp http.ResponseWriter, req *http.Request) (
 	// Authorize using the agent's own enterprise meta, not the token.
 	var authzContext acl.AuthorizerContext
 	s.agent.AgentEnterpriseMeta().FillAuthzContext(&authzContext)
-	if authz.AgentWrite(s.agent.config.NodeName, &authzContext) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().AgentWriteAllowed(s.agent.config.NodeName, &authzContext); err != nil {
-		return nil, acl.ErrPermissionDenied
+		return nil, err
 	}
 
 	// The body is just the token, but it's in a JSON object so we can add
@@ -1683,8 +1684,8 @@ func (s *HTTPHandlers) AgentHost(resp http.ResponseWriter, req *http.Request) (i
 	}
 
 	// TODO(partitions): should this be possible in a partition?
-	if authz.OperatorRead(nil) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().OperatorReadAllowed(nil); err != nil {
-		return nil, acl.PermissionDeniedByACLUnnamed(authz, nil, acl.ResourceOperator, acl.AccessRead)
+		return nil, err
 	}
 
 	return debug.CollectHostInfo(), nil

agent/connect_auth.go

@@ -65,8 +65,8 @@ func (a *Agent) ConnectAuthorize(token string,
 		return returnErr(err)
 	}
 
-	if authz.ServiceWrite(req.Target, &authzContext) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(req.Target, &authzContext); err != nil {
-		return returnErr(acl.ErrPermissionDenied)
+		return returnErr(err)
 	}
 
 	if !uriService.MatchesPartition(req.TargetPartition()) {

agent/consul/acl_endpoint.go

@@ -286,8 +286,8 @@ func (a *ACL) TokenRead(args *structs.ACLTokenGetRequest, reply *structs.ACLToke
 		// secrets will be redacted
 		if authz, err = a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext); err != nil {
 			return err
-		} else if authz.ACLRead(&authzContext) != acl.Allow {
+		} else if err := authz.ToAllowAuthorizer().ACLReadAllowed(&authzContext); err != nil {
-			return acl.ErrPermissionDenied
+			return err
 		}
 	}
 
@@ -354,8 +354,8 @@ func (a *ACL) TokenClone(args *structs.ACLTokenSetRequest, reply *structs.ACLTok
 	authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.ACLToken.EnterpriseMeta, &authzContext)
 	if err != nil {
 		return err
-	} else if authz.ACLWrite(&authzContext) != acl.Allow {
+	} else if err := authz.ToAllowAuthorizer().ACLWriteAllowed(&authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	_, token, err := a.srv.fsm.State().ACLTokenGetByAccessor(nil, args.ACLToken.AccessorID, &args.ACLToken.EnterpriseMeta)
@@ -425,8 +425,8 @@ func (a *ACL) TokenSet(args *structs.ACLTokenSetRequest, reply *structs.ACLToken
 	var authzContext acl.AuthorizerContext
 	if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.ACLToken.EnterpriseMeta, &authzContext); err != nil {
 		return err
-	} else if authz.ACLWrite(&authzContext) != acl.Allow {
+	} else if err := authz.ToAllowAuthorizer().ACLWriteAllowed(&authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	return a.tokenSetInternal(args, reply, false)
@@ -830,8 +830,8 @@ func (a *ACL) TokenDelete(args *structs.ACLTokenDeleteRequest, reply *string) er
 	var authzContext acl.AuthorizerContext
 	if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext); err != nil {
 		return err
-	} else if authz.ACLWrite(&authzContext) != acl.Allow {
+	} else if err := authz.ToAllowAuthorizer().ACLWriteAllowed(&authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	if _, err := uuid.ParseUUID(args.TokenID); err != nil {
@@ -919,8 +919,8 @@ func (a *ACL) TokenList(args *structs.ACLTokenListRequest, reply *structs.ACLTok
 	// merge the token default meta into the requests meta
 	args.EnterpriseMeta.Merge(&requestMeta)
 	args.EnterpriseMeta.FillAuthzContext(&authzContext)
-	if authz.ACLRead(&authzContext) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().ACLReadAllowed(&authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	var methodMeta *structs.EnterpriseMeta
@@ -1026,8 +1026,8 @@ func (a *ACL) PolicyRead(args *structs.ACLPolicyGetRequest, reply *structs.ACLPo
 	var authzContext acl.AuthorizerContext
 	if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext); err != nil {
 		return err
-	} else if authz.ACLRead(&authzContext) != acl.Allow {
+	} else if err := authz.ToAllowAuthorizer().ACLReadAllowed(&authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	return a.srv.blockingQuery(&args.QueryOptions, &reply.QueryMeta,
@@ -1107,8 +1107,8 @@ func (a *ACL) PolicySet(args *structs.ACLPolicySetRequest, reply *structs.ACLPol
 
 	if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.Policy.EnterpriseMeta, &authzContext); err != nil {
 		return err
-	} else if authz.ACLWrite(&authzContext) != acl.Allow {
+	} else if err := authz.ToAllowAuthorizer().ACLWriteAllowed(&authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	policy := &args.Policy
@@ -1237,8 +1237,8 @@ func (a *ACL) PolicyDelete(args *structs.ACLPolicyDeleteRequest, reply *string)
 
 	if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext); err != nil {
 		return err
-	} else if authz.ACLWrite(&authzContext) != acl.Allow {
+	} else if err := authz.ToAllowAuthorizer().ACLWriteAllowed(&authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	_, policy, err := a.srv.fsm.State().ACLPolicyGetByID(nil, args.PolicyID, &args.EnterpriseMeta)
@@ -1288,8 +1288,8 @@ func (a *ACL) PolicyList(args *structs.ACLPolicyListRequest, reply *structs.ACLP
 	authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext)
 	if err != nil {
 		return err
-	} else if authz.ACLRead(&authzContext) != acl.Allow {
+	} else if err := authz.ToAllowAuthorizer().ACLReadAllowed(&authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	return a.srv.blockingQuery(&args.QueryOptions, &reply.QueryMeta,
@@ -1412,8 +1412,8 @@ func (a *ACL) RoleRead(args *structs.ACLRoleGetRequest, reply *structs.ACLRoleRe
 
 	if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext); err != nil {
 		return err
-	} else if authz.ACLRead(&authzContext) != acl.Allow {
+	} else if err := authz.ToAllowAuthorizer().ACLReadAllowed(&authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	return a.srv.blockingQuery(&args.QueryOptions, &reply.QueryMeta,
@@ -1493,8 +1493,8 @@ func (a *ACL) RoleSet(args *structs.ACLRoleSetRequest, reply *structs.ACLRole) e
 
 	if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.Role.EnterpriseMeta, &authzContext); err != nil {
 		return err
-	} else if authz.ACLWrite(&authzContext) != acl.Allow {
+	} else if err := authz.ToAllowAuthorizer().ACLWriteAllowed(&authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	role := &args.Role
@@ -1651,8 +1651,8 @@ func (a *ACL) RoleDelete(args *structs.ACLRoleDeleteRequest, reply *string) erro
 
 	if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext); err != nil {
 		return err
-	} else if authz.ACLWrite(&authzContext) != acl.Allow {
+	} else if err := authz.ToAllowAuthorizer().ACLWriteAllowed(&authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	_, role, err := a.srv.fsm.State().ACLRoleGetByID(nil, args.RoleID, &args.EnterpriseMeta)
@@ -1698,8 +1698,8 @@ func (a *ACL) RoleList(args *structs.ACLRoleListRequest, reply *structs.ACLRoleL
 	authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext)
 	if err != nil {
 		return err
-	} else if authz.ACLRead(&authzContext) != acl.Allow {
+	} else if err := authz.ToAllowAuthorizer().ACLReadAllowed(&authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	return a.srv.blockingQuery(&args.QueryOptions, &reply.QueryMeta,
@@ -1797,8 +1797,8 @@ func (a *ACL) BindingRuleRead(args *structs.ACLBindingRuleGetRequest, reply *str
 	authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext)
 	if err != nil {
 		return err
-	} else if authz.ACLRead(&authzContext) != acl.Allow {
+	} else if err := authz.ToAllowAuthorizer().ACLReadAllowed(&authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	return a.srv.blockingQuery(&args.QueryOptions, &reply.QueryMeta,
@@ -1840,8 +1840,8 @@ func (a *ACL) BindingRuleSet(args *structs.ACLBindingRuleSetRequest, reply *stru
 	// Verify token is permitted to modify ACLs
 	if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.BindingRule.EnterpriseMeta, &authzContext); err != nil {
 		return err
-	} else if authz.ACLWrite(&authzContext) != acl.Allow {
+	} else if err := authz.ToAllowAuthorizer().ACLWriteAllowed(&authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	var existing *structs.ACLBindingRule
@@ -1969,8 +1969,8 @@ func (a *ACL) BindingRuleDelete(args *structs.ACLBindingRuleDeleteRequest, reply
 	// Verify token is permitted to modify ACLs
 	if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext); err != nil {
 		return err
-	} else if authz.ACLWrite(&authzContext) != acl.Allow {
+	} else if err := authz.ToAllowAuthorizer().ACLWriteAllowed(&authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	_, rule, err := a.srv.fsm.State().ACLBindingRuleGetByID(nil, args.BindingRuleID, &args.EnterpriseMeta)
@@ -2017,8 +2017,8 @@ func (a *ACL) BindingRuleList(args *structs.ACLBindingRuleListRequest, reply *st
 	authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext)
 	if err != nil {
 		return err
-	} else if authz.ACLRead(&authzContext) != acl.Allow {
+	} else if err := authz.ToAllowAuthorizer().ACLReadAllowed(&authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	return a.srv.blockingQuery(&args.QueryOptions, &reply.QueryMeta,
@@ -2056,8 +2056,8 @@ func (a *ACL) AuthMethodRead(args *structs.ACLAuthMethodGetRequest, reply *struc
 
 	if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext); err != nil {
 		return err
-	} else if authz.ACLRead(&authzContext) != acl.Allow {
+	} else if err := authz.ToAllowAuthorizer().ACLReadAllowed(&authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	return a.srv.blockingQuery(&args.QueryOptions, &reply.QueryMeta,
@@ -2101,8 +2101,8 @@ func (a *ACL) AuthMethodSet(args *structs.ACLAuthMethodSetRequest, reply *struct
 
 	if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.AuthMethod.EnterpriseMeta, &authzContext); err != nil {
 		return err
-	} else if authz.ACLWrite(&authzContext) != acl.Allow {
+	} else if err := authz.ToAllowAuthorizer().ACLWriteAllowed(&authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	method := &args.AuthMethod
@@ -2213,8 +2213,8 @@ func (a *ACL) AuthMethodDelete(args *structs.ACLAuthMethodDeleteRequest, reply *
 
 	if authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext); err != nil {
 		return err
-	} else if authz.ACLWrite(&authzContext) != acl.Allow {
+	} else if err := authz.ToAllowAuthorizer().ACLWriteAllowed(&authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	_, method, err := a.srv.fsm.State().ACLAuthMethodGetByName(nil, args.AuthMethodName, &args.EnterpriseMeta)
@@ -2267,8 +2267,8 @@ func (a *ACL) AuthMethodList(args *structs.ACLAuthMethodListRequest, reply *stru
 	authz, err := a.srv.ResolveTokenAndDefaultMeta(args.Token, &args.EnterpriseMeta, &authzContext)
 	if err != nil {
 		return err
-	} else if authz.ACLRead(&authzContext) != acl.Allow {
+	} else if err := authz.ToAllowAuthorizer().ACLReadAllowed(&authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	return a.srv.blockingQuery(&args.QueryOptions, &reply.QueryMeta,

agent/consul/catalog_endpoint.go

@@ -192,15 +192,15 @@ func servicePreApply(service *structs.NodeService, authz acl.Authorizer, authzCt
 	// later if version 0.8 is enabled, so we can eventually just
 	// delete this and do all the ACL checks down there.
 	if service.Service != structs.ConsulServiceName {
-		if authz.ServiceWrite(service.Service, &authzContext) != acl.Allow {
+		if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(service.Service, &authzContext); err != nil {
-			return acl.ErrPermissionDenied
+			return err
 		}
 	}
 
 	// Proxies must have write permission on their destination
 	if service.Kind == structs.ServiceKindConnectProxy {
-		if authz.ServiceWrite(service.Proxy.DestinationServiceName, &authzContext) != acl.Allow {
+		if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(service.Proxy.DestinationServiceName, &authzContext); err != nil {
-			return acl.ErrPermissionDenied
+			return err
 		}
 	}
 
@@ -241,16 +241,18 @@ func vetRegisterWithACL(
 	// privileges.
 	needsNode := ns == nil || subj.ChangesNode(ns.Node)
 
-	if needsNode && authz.NodeWrite(subj.Node, &authzContext) != acl.Allow {
+	if needsNode {
-		return acl.ErrPermissionDenied
+		if err := authz.ToAllowAuthorizer().NodeWriteAllowed(subj.Node, &authzContext); err != nil {
+			return err
+		}
 	}
 
 	// Vet the service change. This includes making sure they can register
 	// the given service, and that we can write to any existing service that
 	// is being modified by id (if any).
 	if subj.Service != nil {
-		if authz.ServiceWrite(subj.Service.Service, &authzContext) != acl.Allow {
+		if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(subj.Service.Service, &authzContext); err != nil {
-			return acl.ErrPermissionDenied
+			return err
 		}
 
 		if ns != nil {
@@ -263,7 +265,7 @@ func vetRegisterWithACL(
 				var secondaryCtx acl.AuthorizerContext
 				other.FillAuthzContext(&secondaryCtx)
 
-				if authz.ServiceWrite(other.Service, &secondaryCtx) != acl.Allow {
+				if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(other.Service, &secondaryCtx); err != nil {
 					return acl.ErrPermissionDenied
 				}
 			}
@@ -293,8 +295,8 @@ func vetRegisterWithACL(
 
 		// Node-level check.
 		if check.ServiceID == "" {
-			if authz.NodeWrite(subj.Node, &authzContext) != acl.Allow {
+			if err := authz.ToAllowAuthorizer().NodeWriteAllowed(subj.Node, &authzContext); err != nil {
-				return acl.ErrPermissionDenied
+				return err
 			}
 			continue
 		}
@@ -324,8 +326,8 @@ func vetRegisterWithACL(
 		var secondaryCtx acl.AuthorizerContext
 		other.FillAuthzContext(&secondaryCtx)
 
-		if authz.ServiceWrite(other.Service, &secondaryCtx) != acl.Allow {
+		if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(other.Service, &secondaryCtx); err != nil {
-			return acl.ErrPermissionDenied
+			return err
 		}
 	}
 
@@ -408,7 +410,8 @@ func vetDeregisterWithACL(
 	// Allow service deregistration if the token has write permission for the node.
 	// This accounts for cases where the agent no longer has a token with write permission
 	// on the service to deregister it.
-	if authz.NodeWrite(subj.Node, &authzContext) == acl.Allow {
+	nodeWriteErr := authz.ToAllowAuthorizer().NodeWriteAllowed(subj.Node, &authzContext)
+	if nodeWriteErr == nil {
 		return nil
 	}
 
@@ -423,8 +426,8 @@ func vetDeregisterWithACL(
 
 		ns.FillAuthzContext(&authzContext)
 
-		if authz.ServiceWrite(ns.Service, &authzContext) != acl.Allow {
+		if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(ns.Service, &authzContext); err != nil {
-			return acl.ErrPermissionDenied
+			return err
 		}
 	} else if subj.CheckID != "" {
 		if nc == nil {
@@ -434,18 +437,18 @@ func vetDeregisterWithACL(
 		nc.FillAuthzContext(&authzContext)
 
 		if nc.ServiceID != "" {
-			if authz.ServiceWrite(nc.ServiceName, &authzContext) != acl.Allow {
+			if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(nc.ServiceName, &authzContext); err != nil {
-				return acl.ErrPermissionDenied
+				return err
 			}
 		} else {
-			if authz.NodeWrite(subj.Node, &authzContext) != acl.Allow {
+			if err := authz.ToAllowAuthorizer().NodeWriteAllowed(subj.Node, &authzContext); err != nil {
-				return acl.ErrPermissionDenied
+				return err
 			}
 		}
 	} else {
 		// Since NodeWrite is not given - otherwise the earlier check
 		// would've returned already - we can deny here.
-		return acl.ErrPermissionDenied
+		return nodeWriteErr
 	}
 
 	return nil
@@ -647,6 +650,8 @@ func (c *Catalog) ServiceNodes(args *structs.ServiceSpecificRequest, reply *stru
 	// If we're doing a connect query, we need read access to the service
 	// we're trying to find proxies for, so check that.
 	if args.Connect {
+		// TODO(acl-error-enhancements) can this be improved? What happens if we returned an error here?
+		// Is this similar to filters where we might want to return a hint?
 		if authz.ServiceRead(args.ServiceName, &authzContext) != acl.Allow {
 			// Just return nil, which will return an empty response (tested)
 			return nil
@@ -862,8 +867,8 @@ func (c *Catalog) GatewayServices(args *structs.ServiceSpecificRequest, reply *s
 		return err
 	}
 
-	if authz.ServiceRead(args.ServiceName, &authzContext) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().ServiceReadAllowed(args.ServiceName, &authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	return c.srv.blockingQuery(
@@ -926,8 +931,8 @@ func (c *Catalog) VirtualIPForService(args *structs.ServiceSpecificRequest, repl
 		return err
 	}
 
-	if authz.ServiceRead(args.ServiceName, &authzContext) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().ServiceReadAllowed(args.ServiceName, &authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	state := c.srv.fsm.State()

agent/consul/config_endpoint.go

@@ -90,7 +90,7 @@ func (c *ConfigEntry) Apply(args *structs.ConfigEntryRequest, reply *bool) error
 	}
 
 	if !args.Entry.CanWrite(authz) {
-		return acl.ErrPermissionDenied
+		return acl.ErrPermissionDenied // TODO(acl-error-enhancements) Better errors await refactoring of CanWrite above.
 	}
 
 	if args.Op != structs.ConfigEntryUpsert && args.Op != structs.ConfigEntryUpsertCAS {
@@ -439,8 +439,8 @@ func (c *ConfigEntry) ResolveServiceConfig(args *structs.ServiceConfigRequest, r
 	if err != nil {
 		return err
 	}
-	if authz.ServiceRead(args.Name, &authzContext) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().ServiceReadAllowed(args.Name, &authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	var (

agent/consul/connect_ca_endpoint.go

@@ -65,8 +65,8 @@ func (s *ConnectCA) ConfigurationGet(
 	if err != nil {
 		return err
 	}
-	if authz.OperatorWrite(nil) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().OperatorWriteAllowed(nil); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	state := s.srv.fsm.State()
@@ -97,8 +97,8 @@ func (s *ConnectCA) ConfigurationSet(
 	if err != nil {
 		return err
 	}
-	if authz.OperatorWrite(nil) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().OperatorWriteAllowed(nil); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	return s.srv.caManager.UpdateConfiguration(args)
@@ -175,8 +175,8 @@ func (s *ConnectCA) Sign(
 	if isService {
 		entMeta.Merge(serviceID.GetEnterpriseMeta())
 		entMeta.FillAuthzContext(&authzContext)
-		if authz.ServiceWrite(serviceID.Service, &authzContext) != acl.Allow {
+		if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(serviceID.Service, &authzContext); err != nil {
-			return acl.ErrPermissionDenied
+			return err
 		}
 
 		// Verify that the DC in the service URI matches us. We might relax this
@@ -187,8 +187,8 @@ func (s *ConnectCA) Sign(
 		}
 	} else if isAgent {
 		agentID.GetEnterpriseMeta().FillAuthzContext(&authzContext)
-		if authz.NodeWrite(agentID.Agent, &authzContext) != acl.Allow {
+		if err := authz.ToAllowAuthorizer().NodeWriteAllowed(agentID.Agent, &authzContext); err != nil {
-			return acl.ErrPermissionDenied
+			return err
 		}
 	}
 
@@ -223,8 +223,8 @@ func (s *ConnectCA) SignIntermediate(
 	if err != nil {
 		return err
 	}
-	if authz.OperatorWrite(nil) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().OperatorWriteAllowed(nil); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	provider, _ := s.srv.caManager.getCAProvider()

agent/consul/coordinate_endpoint.go

@@ -152,8 +152,8 @@ func (c *Coordinate) Update(args *structs.CoordinateUpdateRequest, reply *struct
 		return err
 	}
 
-	if authz.NodeWrite(args.Node, &authzContext) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().NodeWriteAllowed(args.Node, &authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	// Add the coordinate to the map of pending updates.
@@ -245,8 +245,8 @@ func (c *Coordinate) Node(args *structs.NodeSpecificRequest, reply *structs.Inde
 		return err
 	}
 
-	if authz.NodeRead(args.Node, &authzContext) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().NodeReadAllowed(args.Node, &authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	return c.srv.blockingQuery(&args.QueryOptions,

agent/consul/discovery_chain_endpoint.go

@@ -36,8 +36,8 @@ func (c *DiscoveryChain) Get(args *structs.DiscoveryChainRequest, reply *structs
 	if err != nil {
 		return err
 	}
-	if authz.ServiceRead(args.Name, &authzContext) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().ServiceReadAllowed(args.Name, &authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	if args.Name == "" {

agent/consul/federation_state_endpoint.go

@@ -9,7 +9,6 @@ import (
 	"github.com/armon/go-metrics/prometheus"
 	memdb "github.com/hashicorp/go-memdb"
 
-	"github.com/hashicorp/consul/acl"
 	"github.com/hashicorp/consul/agent/consul/state"
 	"github.com/hashicorp/consul/agent/structs"
 )
@@ -63,8 +62,8 @@ func (c *FederationState) Apply(args *structs.FederationStateRequest, reply *boo
 	if err != nil {
 		return err
 	}
-	if authz.OperatorWrite(nil) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().OperatorWriteAllowed(nil); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	if args.State == nil || args.State.Datacenter == "" {
@@ -109,8 +108,8 @@ func (c *FederationState) Get(args *structs.FederationStateQuery, reply *structs
 	if err != nil {
 		return err
 	}
-	if authz.OperatorRead(nil) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().OperatorReadAllowed(nil); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	return c.srv.blockingQuery(
@@ -148,8 +147,8 @@ func (c *FederationState) List(args *structs.DCSpecificRequest, reply *structs.I
 	if err != nil {
 		return err
 	}
-	if authz.OperatorRead(nil) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().OperatorReadAllowed(nil); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	return c.srv.blockingQuery(

agent/consul/health_endpoint.go

@@ -220,6 +220,7 @@ func (h *Health) ServiceNodes(args *structs.ServiceSpecificRequest, reply *struc
 	// If we're doing a connect or ingress query, we need read access to the service
 	// we're trying to find proxies for, so check that.
 	if args.Connect || args.Ingress {
+		// TODO(acl-error-enhancements) Look for ways to percolate this information up to give any feedback to the user.
 		if authz.ServiceRead(args.ServiceName, &authzContext) != acl.Allow {
 			// Just return nil, which will return an empty response (tested)
 			return nil

agent/consul/intention_endpoint.go

@@ -610,11 +610,13 @@ func (s *Intention) Match(args *structs.IntentionQueryRequest, reply *structs.In
 	// matching, if you have it on the dest then perform a dest type match.
 	for _, entry := range args.Match.Entries {
 		entry.FillAuthzContext(&authzContext)
-		if prefix := entry.Name; prefix != "" && authz.IntentionRead(prefix, &authzContext) != acl.Allow {
+		if prefix := entry.Name; prefix != "" {
-			accessorID := authz.AccessorID()
+			if err := authz.ToAllowAuthorizer().IntentionReadAllowed(prefix, &authzContext); err != nil {
-			// todo(kit) Migrate intention access denial logging over to audit logging when we implement it
+				accessorID := authz.AccessorID()
-			s.logger.Warn("Operation on intention prefix denied due to ACLs", "prefix", prefix, "accessorID", accessorID)
+				// todo(kit) Migrate intention access denial logging over to audit logging when we implement it
-			return acl.ErrPermissionDenied
+				s.logger.Warn("Operation on intention prefix denied due to ACLs", "prefix", prefix, "accessorID", accessorID)
+				return err
+			}
 		}
 	}
 
@@ -733,11 +735,11 @@ func (s *Intention) Check(args *structs.IntentionQueryRequest, reply *structs.In
 	if prefix, ok := query.GetACLPrefix(); ok {
 		var authzContext acl.AuthorizerContext
 		query.FillAuthzContext(&authzContext)
-		if authz.ServiceRead(prefix, &authzContext) != acl.Allow {
+		if err := authz.ToAllowAuthorizer().ServiceReadAllowed(prefix, &authzContext); err != nil {
 			accessorID := authz.AccessorID()
 			// todo(kit) Migrate intention access denial logging over to audit logging when we implement it
 			s.logger.Warn("test on intention denied due to ACLs", "prefix", prefix, "accessorID", accessorID)
-			return acl.ErrPermissionDenied
+			return err
 		}
 	}
 

agent/consul/internal_endpoint.go

@@ -163,8 +163,8 @@ func (m *Internal) ServiceTopology(args *structs.ServiceSpecificRequest, reply *
 	if err := m.srv.validateEnterpriseRequest(&args.EnterpriseMeta, false); err != nil {
 		return err
 	}
-	if authz.ServiceRead(args.ServiceName, &authzContext) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().ServiceReadAllowed(args.ServiceName, &authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	return m.srv.blockingQuery(
@@ -272,8 +272,8 @@ func (m *Internal) GatewayServiceDump(args *structs.ServiceSpecificRequest, repl
 	}
 
 	// We need read access to the gateway we're trying to find services for, so check that first.
-	if authz.ServiceRead(args.ServiceName, &authzContext) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().ServiceReadAllowed(args.ServiceName, &authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	err = m.srv.blockingQuery(
@@ -356,8 +356,8 @@ func (m *Internal) GatewayIntentions(args *structs.IntentionQueryRequest, reply
 	}
 
 	// We need read access to the gateway we're trying to find intentions for, so check that first.
-	if authz.ServiceRead(args.Match.Entries[0].Name, &authzContext) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().ServiceReadAllowed(args.Match.Entries[0].Name, &authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	return m.srv.blockingQuery(
@@ -428,10 +428,10 @@ func (m *Internal) EventFire(args *structs.EventFireRequest,
 		return err
 	}
 
-	if authz.EventWrite(args.Name, nil) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().EventWriteAllowed(args.Name, nil); err != nil {
 		accessorID := authz.AccessorID()
 		m.logger.Warn("user event blocked by ACLs", "event", args.Name, "accessorID", accessorID)
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	// Set the query meta data
@@ -464,16 +464,16 @@ func (m *Internal) KeyringOperation(
 	}
 	switch args.Operation {
 	case structs.KeyringList:
-		if authz.KeyringRead(nil) != acl.Allow {
+		if err := authz.ToAllowAuthorizer().KeyringReadAllowed(nil); err != nil {
-			return fmt.Errorf("Reading keyring denied by ACLs")
+			return err
 		}
 	case structs.KeyringInstall:
 		fallthrough
 	case structs.KeyringUse:
 		fallthrough
 	case structs.KeyringRemove:
-		if authz.KeyringWrite(nil) != acl.Allow {
+		if err := authz.ToAllowAuthorizer().KeyringWriteAllowed(nil); err != nil {
-			return fmt.Errorf("Modifying keyring denied due to ACLs")
+			return err
 		}
 	default:
 		panic("Invalid keyring operation")

agent/consul/kvs_endpoint.go

@@ -44,8 +44,8 @@ func kvsPreApply(logger hclog.Logger, srv *Server, authz acl.Authorizer, op api.
 		var authzContext acl.AuthorizerContext
 		dirEnt.FillAuthzContext(&authzContext)
 
-		if authz.KeyWritePrefix(dirEnt.Key, &authzContext) != acl.Allow {
+		if err := authz.ToAllowAuthorizer().KeyWritePrefixAllowed(dirEnt.Key, &authzContext); err != nil {
-			return false, acl.ErrPermissionDenied
+			return false, err
 		}
 
 	case api.KVGet, api.KVGetTree:
@@ -58,16 +58,16 @@ func kvsPreApply(logger hclog.Logger, srv *Server, authz acl.Authorizer, op api.
 		var authzContext acl.AuthorizerContext
 		dirEnt.FillAuthzContext(&authzContext)
 
-		if authz.KeyRead(dirEnt.Key, &authzContext) != acl.Allow {
+		if err := authz.ToAllowAuthorizer().KeyReadAllowed(dirEnt.Key, &authzContext); err != nil {
-			return false, acl.ErrPermissionDenied
+			return false, err
 		}
 
 	default:
 		var authzContext acl.AuthorizerContext
 		dirEnt.FillAuthzContext(&authzContext)
 
-		if authz.KeyWrite(dirEnt.Key, &authzContext) != acl.Allow {
+		if err := authz.ToAllowAuthorizer().KeyWriteAllowed(dirEnt.Key, &authzContext); err != nil {
-			return false, acl.ErrPermissionDenied
+			return false, err
 		}
 	}
 
@@ -155,8 +155,8 @@ func (k *KVS) Get(args *structs.KeyRequest, reply *structs.IndexedDirEntries) er
 			if err != nil {
 				return err
 			}
-			if authz.KeyRead(args.Key, &authzContext) != acl.Allow {
+			if err := authz.ToAllowAuthorizer().KeyReadAllowed(args.Key, &authzContext); err != nil {
-				return acl.ErrPermissionDenied
+				return err
 			}
 
 			if ent == nil {
@@ -187,8 +187,10 @@ func (k *KVS) List(args *structs.KeyRequest, reply *structs.IndexedDirEntries) e
 		return err
 	}
 
-	if k.srv.config.ACLEnableKeyListPolicy && authz.KeyList(args.Key, &authzContext) != acl.Allow {
+	if k.srv.config.ACLEnableKeyListPolicy {
-		return acl.ErrPermissionDenied
+		if err := authz.ToAllowAuthorizer().KeyListAllowed(args.Key, &authzContext); err != nil {
+			return err
+		}
 	}
 
 	return k.srv.blockingQuery(
@@ -240,8 +242,10 @@ func (k *KVS) ListKeys(args *structs.KeyListRequest, reply *structs.IndexedKeyLi
 		return err
 	}
 
-	if k.srv.config.ACLEnableKeyListPolicy && authz.KeyList(args.Prefix, &authzContext) != acl.Allow {
+	if k.srv.config.ACLEnableKeyListPolicy {
-		return acl.ErrPermissionDenied
+		if err := authz.ToAllowAuthorizer().KeyListAllowed(args.Prefix, &authzContext); err != nil {
+			return err
+		}
 	}
 
 	return k.srv.blockingQuery(

agent/consul/operator_autopilot_endpoint.go

@@ -2,11 +2,9 @@ package consul
 
 import (
 	"fmt"
-
 	autopilot "github.com/hashicorp/raft-autopilot"
 	"github.com/hashicorp/serf/serf"
 
-	"github.com/hashicorp/consul/acl"
 	"github.com/hashicorp/consul/agent/structs"
 )
 
@@ -24,8 +22,9 @@ func (op *Operator) AutopilotGetConfiguration(args *structs.DCSpecificRequest, r
 	if err := op.srv.validateEnterpriseToken(authz.Identity()); err != nil {
 		return err
 	}
-	if authz.OperatorRead(nil) != acl.Allow {
+
-		return acl.PermissionDeniedByACLUnnamed(authz, nil, acl.ResourceOperator, acl.AccessRead)
+	if err := authz.ToAllowAuthorizer().OperatorReadAllowed(nil); err != nil {
+		return err
 	}
 
 	state := op.srv.fsm.State()
@@ -56,8 +55,9 @@ func (op *Operator) AutopilotSetConfiguration(args *structs.AutopilotSetConfigRe
 	if err := op.srv.validateEnterpriseToken(authz.Identity()); err != nil {
 		return err
 	}
-	if authz.OperatorWrite(nil) != acl.Allow {
+
-		return acl.PermissionDeniedByACLUnnamed(authz, nil, acl.ResourceOperator, acl.AccessWrite)
+	if err := authz.ToAllowAuthorizer().OperatorWriteAllowed(nil); err != nil {
+		return err
 	}
 
 	// Apply the update
@@ -91,8 +91,9 @@ func (op *Operator) ServerHealth(args *structs.DCSpecificRequest, reply *structs
 	if err := op.srv.validateEnterpriseToken(authz.Identity()); err != nil {
 		return err
 	}
-	if authz.OperatorRead(nil) != acl.Allow {
+
-		return acl.PermissionDeniedByACLUnnamed(authz, nil, acl.ResourceOperator, acl.AccessRead)
+	if err := authz.ToAllowAuthorizer().OperatorReadAllowed(nil); err != nil {
+		return err
 	}
 
 	state := op.srv.autopilot.GetState()
@@ -158,8 +159,9 @@ func (op *Operator) AutopilotState(args *structs.DCSpecificRequest, reply *autop
 	if err := op.srv.validateEnterpriseToken(authz.Identity()); err != nil {
 		return err
 	}
-	if authz.OperatorRead(nil) != acl.Allow {
+
-		return acl.PermissionDeniedByACLUnnamed(authz, nil, acl.ResourceOperator, acl.AccessRead)
+	if err := authz.ToAllowAuthorizer().OperatorReadAllowed(nil); err != nil {
+		return err
 	}
 
 	state := op.srv.autopilot.GetState()

agent/consul/operator_raft_endpoint.go

@@ -7,7 +7,6 @@ import (
 	"github.com/hashicorp/raft"
 	"github.com/hashicorp/serf/serf"
 
-	"github.com/hashicorp/consul/acl"
 	"github.com/hashicorp/consul/agent/metadata"
 	"github.com/hashicorp/consul/agent/structs"
 )
@@ -23,8 +22,8 @@ func (op *Operator) RaftGetConfiguration(args *structs.DCSpecificRequest, reply
 	if err != nil {
 		return err
 	}
-	if authz.OperatorRead(nil) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().OperatorReadAllowed(nil); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	// We can't fetch the leader and the configuration atomically with
@@ -88,8 +87,8 @@ func (op *Operator) RaftRemovePeerByAddress(args *structs.RaftRemovePeerRequest,
 	if err := op.srv.validateEnterpriseToken(authz.Identity()); err != nil {
 		return err
 	}
-	if authz.OperatorWrite(nil) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().OperatorWriteAllowed(nil); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	// Since this is an operation designed for humans to use, we will return
@@ -141,8 +140,8 @@ func (op *Operator) RaftRemovePeerByID(args *structs.RaftRemovePeerRequest, repl
 	if err := op.srv.validateEnterpriseToken(authz.Identity()); err != nil {
 		return err
 	}
-	if authz.OperatorWrite(nil) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().OperatorWriteAllowed(nil); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	// Since this is an operation designed for humans to use, we will return

agent/consul/prepared_query_endpoint.go

@@ -86,9 +86,9 @@ func (p *PreparedQuery) Apply(args *structs.PreparedQueryRequest, reply *string)
 	// need to make sure they have write access for whatever they are
 	// proposing.
 	if prefix, ok := args.Query.GetACLPrefix(); ok {
-		if authz.PreparedQueryWrite(prefix, nil) != acl.Allow {
+		if err := authz.ToAllowAuthorizer().PreparedQueryWriteAllowed(prefix, nil); err != nil {
 			p.logger.Warn("Operation on prepared query denied due to ACLs", "query", args.Query.ID)
-			return acl.ErrPermissionDenied
+			return err
 		}
 	}
 
@@ -106,9 +106,9 @@ func (p *PreparedQuery) Apply(args *structs.PreparedQueryRequest, reply *string)
 		}
 
 		if prefix, ok := query.GetACLPrefix(); ok {
-			if authz.PreparedQueryWrite(prefix, nil) != acl.Allow {
+			if err := authz.ToAllowAuthorizer().PreparedQueryWriteAllowed(prefix, nil); err != nil {
 				p.logger.Warn("Operation on prepared query denied due to ACLs", "query", args.Query.ID)
-				return acl.ErrPermissionDenied
+				return err
 			}
 		}
 	}

agent/consul/session_endpoint.go

@@ -82,13 +82,13 @@ func (s *Session) Apply(args *structs.SessionRequest, reply *string) error {
 		if existing == nil {
 			return nil
 		}
-		if authz.SessionWrite(existing.Node, &authzContext) != acl.Allow {
+		if err := authz.ToAllowAuthorizer().SessionWriteAllowed(existing.Node, &authzContext); err != nil {
-			return acl.ErrPermissionDenied
+			return err
 		}
 
 	case structs.SessionCreate:
-		if authz.SessionWrite(args.Session.Node, &authzContext) != acl.Allow {
+		if err := authz.ToAllowAuthorizer().SessionWriteAllowed(args.Session.Node, &authzContext); err != nil {
-			return acl.ErrPermissionDenied
+			return err
 		}
 
 	default:
@@ -303,8 +303,8 @@ func (s *Session) Renew(args *structs.SessionSpecificRequest,
 		return nil
 	}
 
-	if authz.SessionWrite(session.Node, &authzContext) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().SessionWriteAllowed(session.Node, &authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 
 	// Reset the session TTL timer.

agent/consul/snapshot_endpoint.go

@@ -18,7 +18,6 @@ import (
 
 	"github.com/hashicorp/consul-net-rpc/go-msgpack/codec"
 
-	"github.com/hashicorp/consul/acl"
 	"github.com/hashicorp/consul/agent/pool"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/snapshot"
@@ -62,8 +61,8 @@ func (s *Server) dispatchSnapshotRequest(args *structs.SnapshotRequest, in io.Re
 	// all the ACLs and you could escalate from there.
 	if authz, err := s.ResolveToken(args.Token); err != nil {
 		return nil, err
-	} else if authz.Snapshot(nil) != acl.Allow {
+	} else if err := authz.ToAllowAuthorizer().SnapshotAllowed(nil); err != nil {
-		return nil, acl.ErrPermissionDenied
+		return nil, err
 	}
 
 	// Dispatch the operation.

agent/consul/txn_endpoint.go

@@ -113,8 +113,8 @@ func vetNodeTxnOp(op *structs.TxnNodeOp, authz acl.Authorizer) error {
 	var authzContext acl.AuthorizerContext
 	op.FillAuthzContext(&authzContext)
 
-	if authz.NodeWrite(op.Node.Node, &authzContext) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().NodeWriteAllowed(op.Node.Node, &authzContext); err != nil {
-		return acl.ErrPermissionDenied
+		return err
 	}
 	return nil
 }
@@ -126,13 +126,13 @@ func vetCheckTxnOp(op *structs.TxnCheckOp, authz acl.Authorizer) error {
 
 	if op.Check.ServiceID == "" {
 		// Node-level check.
-		if authz.NodeWrite(op.Check.Node, &authzContext) != acl.Allow {
+		if err := authz.ToAllowAuthorizer().NodeWriteAllowed(op.Check.Node, &authzContext); err != nil {
-			return acl.ErrPermissionDenied
+			return err
 		}
 	} else {
 		// Service-level check.
-		if authz.ServiceWrite(op.Check.ServiceName, &authzContext) != acl.Allow {
+		if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(op.Check.ServiceName, &authzContext); err != nil {
-			return acl.ErrPermissionDenied
+			return err
 		}
 	}
 	return nil

agent/consul/txn_endpoint_test.go

@@ -554,57 +554,64 @@ func TestTxn_Apply_ACLDeny(t *testing.T) {
 	}
 
 	// Verify the transaction's return value.
-	var expected structs.TxnResponse
+	var outPos int
 	for i, op := range arg.Ops {
+		err := out.Errors[outPos]
 		switch {
 		case op.KV != nil:
 			switch op.KV.Verb {
 			case api.KVGet, api.KVGetTree:
 				// 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")
+				outPos++
 			default:
-				expected.Errors = append(expected.Errors, &structs.TxnError{
+				require.Equal(t, err.OpIndex, i)
-					OpIndex: i,
+				acl.RequirePermissionDeniedMessage(t, err.What, nil, nil, acl.ResourceKey, acl.AccessRead, "nope")
-					What:    acl.ErrPermissionDenied.Error(),
+				outPos++
-				})
 			}
 		case op.Node != nil:
 			switch op.Node.Verb {
 			case api.NodeGet:
 				// 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")
+				outPos++
 			default:
-				expected.Errors = append(expected.Errors, &structs.TxnError{
+				require.Equal(t, err.OpIndex, i)
-					OpIndex: i,
+				acl.RequirePermissionDeniedMessage(t, err.What, nil, nil, acl.ResourceNode, acl.AccessRead, "nope")
-					What:    acl.ErrPermissionDenied.Error(),
+				outPos++
-				})
 			}
 		case op.Service != nil:
 			switch op.Service.Verb {
 			case api.ServiceGet:
 				// 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")
+				outPos++
 			default:
-				expected.Errors = append(expected.Errors, &structs.TxnError{
+				require.Equal(t, err.OpIndex, i)
-					OpIndex: i,
+				acl.RequirePermissionDeniedMessage(t, err.What, nil, nil, acl.ResourceService, acl.AccessRead, "nope")
-					What:    acl.ErrPermissionDenied.Error(),
+				outPos++
-				})
 			}
 		case op.Check != nil:
 			switch op.Check.Verb {
 			case api.CheckGet:
 				// 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")
+				outPos++
 			default:
-				expected.Errors = append(expected.Errors, &structs.TxnError{
+				require.Equal(t, err.OpIndex, i)
-					OpIndex: i,
+				acl.RequirePermissionDeniedMessage(t, err.What, nil, nil, acl.ResourceNode, acl.AccessRead, "nope")
-					What:    acl.ErrPermissionDenied.Error(),
+				outPos++
-				})
 			}
 		}
 	}
-
-	require.Equal(t, expected, out)
 }
 
 func TestTxn_Apply_LockDelay(t *testing.T) {
@@ -927,10 +934,9 @@ func TestTxn_Read_ACLDeny(t *testing.T) {
 		var out structs.TxnReadResponse
 		err := msgpackrpc.CallWithCodec(codec, "Txn.Read", &arg, &out)
 		require.NoError(t, err)
-		require.Equal(t, structs.TxnErrors{
+		acl.RequirePermissionDeniedMessage(t, out.Errors[0].What, nil, nil, acl.ResourceKey, acl.AccessRead, "nope")
-			{OpIndex: 0, What: acl.ErrPermissionDenied.Error()},
+		acl.RequirePermissionDeniedMessage(t, out.Errors[1].What, nil, nil, acl.ResourceKey, acl.AccessRead, "nope")
-			{OpIndex: 1, What: acl.ErrPermissionDenied.Error()},
+
-		}, out.Errors)
 		require.Empty(t, out.Results)
 	})
 }

agent/http.go

@@ -273,6 +273,7 @@ func (s *HTTPHandlers) handler(enableDebug bool) http.Handler {
 			// If the token provided does not have the necessary permissions,
 			// write a forbidden response
 			// TODO(partitions): should this be possible in a partition?
+			// TODO((acl-error-enhancements)): We should return error details somehow here.
 			if authz.OperatorRead(nil) != acl.Allow {
 				resp.WriteHeader(http.StatusForbidden)
 				return

agent/ui_endpoint.go

@@ -642,8 +642,11 @@ func (s *HTTPHandlers) UIMetricsProxy(resp http.ResponseWriter, req *http.Reques
 	wildcardEntMeta := structs.WildcardEnterpriseMetaInPartition(structs.WildcardSpecifier)
 	wildcardEntMeta.FillAuthzContext(&authzContext)
 
-	if authz.NodeReadAll(&authzContext) != acl.Allow || authz.ServiceReadAll(&authzContext) != acl.Allow {
+	if err := authz.ToAllowAuthorizer().NodeReadAllAllowed(&authzContext); err != nil {
-		return nil, acl.ErrPermissionDenied
+		return nil, err
+	}
+	if err := authz.ToAllowAuthorizer().ServiceReadAllAllowed(&authzContext); err != nil {
+		return nil, err
 	}
 
 	log := s.agent.logger.Named(logging.UIMetricsProxy)

agent/xds/delta_test.go

@@ -1156,7 +1156,10 @@ func TestServer_DeltaAggregatedResources_v3_ACLEnforcement(t *testing.T) {
 			case err := <-errCh:
 				if tt.wantDenied {
 					require.Error(t, err)
-					require.Contains(t, err.Error(), "permission denied")
+					status, ok := status.FromError(err)
+					require.True(t, ok)
+					require.Equal(t, codes.PermissionDenied, status.Code())
+					require.Contains(t, err.Error(), "Permission denied")
 					mgr.AssertWatchCancelled(t, sid)
 				} else {
 					require.NoError(t, err)

agent/xds/server.go

@@ -266,7 +266,7 @@ func (s *Server) authorize(ctx context.Context, cfgSnap *proxycfg.ConfigSnapshot
 	if acl.IsErrNotFound(err) {
 		return status.Errorf(codes.Unauthenticated, "unauthenticated: %v", err)
 	} else if acl.IsErrPermissionDenied(err) {
-		return status.Errorf(codes.PermissionDenied, "permission denied: %v", err)
+		return status.Error(codes.PermissionDenied, err.Error())
 	} else if err != nil {
 		return status.Errorf(codes.Internal, "error resolving acl token: %v", err)
 	}
@@ -275,13 +275,13 @@ func (s *Server) authorize(ctx context.Context, cfgSnap *proxycfg.ConfigSnapshot
 	switch cfgSnap.Kind {
 	case structs.ServiceKindConnectProxy:
 		cfgSnap.ProxyID.EnterpriseMeta.FillAuthzContext(&authzContext)
-		if authz.ServiceWrite(cfgSnap.Proxy.DestinationServiceName, &authzContext) != acl.Allow {
+		if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(cfgSnap.Proxy.DestinationServiceName, &authzContext); err != nil {
-			return status.Errorf(codes.PermissionDenied, "permission denied")
+			return status.Errorf(codes.PermissionDenied, err.Error())
 		}
 	case structs.ServiceKindMeshGateway, structs.ServiceKindTerminatingGateway, structs.ServiceKindIngressGateway:
 		cfgSnap.ProxyID.EnterpriseMeta.FillAuthzContext(&authzContext)
-		if authz.ServiceWrite(cfgSnap.Service, &authzContext) != acl.Allow {
+		if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(cfgSnap.Service, &authzContext); err != nil {
-			return status.Errorf(codes.PermissionDenied, "permission denied")
+			return status.Errorf(codes.PermissionDenied, err.Error())
 		}
 	default:
 		return status.Errorf(codes.Internal, "Invalid service kind")