Update the ACL Resolver to allow for Consul Enterprise specific hooks. (#6687)

acl/chained_authorizer.go

@@ -19,6 +19,10 @@ func NewChainedAuthorizer(chain []Authorizer) *ChainedAuthorizer {
 	}
 }
 
+func (c *ChainedAuthorizer) AuthorizerChain() []Authorizer {
+	return c.chain
+}
+
 func (c *ChainedAuthorizer) executeChain(enforce func(authz Authorizer) EnforcementDecision) EnforcementDecision {
 	for _, authz := range c.chain {
 		decision := enforce(authz)

agent/consul/acl.go

@@ -97,6 +97,7 @@ type ACLResolverDelegate interface {
 	ResolvePolicyFromID(policyID string) (bool, *structs.ACLPolicy, error)
 	ResolveRoleFromID(roleID string) (bool, *structs.ACLRole, error)
 	RPC(method string, args interface{}, reply interface{}) error
+	EnterpriseACLResolverDelegate
 }
 
 type policyOrRoleTokenError struct {
@@ -291,7 +292,12 @@ func (r *ACLResolver) resolveTokenLegacy(token string) (acl.Authorizer, error) {
 				return nil, err
 			}
 
-			return policies.Compile(acl.RootAuthorizer(r.config.ACLDefaultPolicy), r.cache, r.entConf)
+			authz, err := policies.Compile(r.cache, r.entConf)
+			if err != nil {
+				return nil, err
+			}
+
+			return acl.NewChainedAuthorizer([]acl.Authorizer{authz, acl.RootAuthorizer(r.config.ACLDefaultPolicy)}), nil
 		}
 
 		return nil, err
@@ -992,7 +998,7 @@ func (r *ACLResolver) ResolveToken(token string) (acl.Authorizer, error) {
 
 	defer metrics.MeasureSince([]string{"acl", "ResolveToken"}, time.Now())
 
-	policies, err := r.resolveTokenToPolicies(token)
+	identity, policies, err := r.resolveTokenToIdentityAndPolicies(token)
 	if err != nil {
 		r.disableACLsWhenUpstreamDisabled(err)
 		if IsACLRemoteError(err) {
@@ -1004,9 +1010,27 @@ func (r *ACLResolver) ResolveToken(token string) (acl.Authorizer, error) {
 	}
 
 	// Build the Authorizer
-	authorizer, err := policies.Compile(acl.RootAuthorizer(r.config.ACLDefaultPolicy), r.cache, r.entConf)
+	var chain []acl.Authorizer
-	return authorizer, err
 
+	authz, err := policies.Compile(r.cache, r.entConf)
+	if err != nil {
+		return nil, err
+	}
+	chain = append(chain, authz)
+
+	authz, err = r.resolveEnterpriseDefaultsForIdentity(identity)
+	if err != nil {
+		if IsACLRemoteError(err) {
+			r.logger.Printf("[ERR] consul.acl: %v", err)
+			return r.down, nil
+		}
+		return nil, err
+	} else if authz != nil {
+		chain = append(chain, authz)
+	}
+
+	chain = append(chain, acl.RootAuthorizer(r.config.ACLDefaultPolicy))
+	return acl.NewChainedAuthorizer(chain), nil
 }
 
 func (r *ACLResolver) ACLsEnabled() bool {

agent/consul/acl_oss.go

@@ -6,8 +6,16 @@ import (
 	"log"
 
 	"github.com/hashicorp/consul/acl"
+	"github.com/hashicorp/consul/agent/structs"
 )
 
+// EnterpriseACLResolverDelegate stub
+type EnterpriseACLResolverDelegate interface{}
+
 func newEnterpriseACLConfig(*log.Logger) *acl.EnterpriseACLConfig {
 	return nil
 }
+
+func (r *ACLResolver) resolveEnterpriseDefaultsForIdentity(identity structs.ACLIdentity) (acl.Authorizer, error) {
+	return nil, nil
+}

agent/consul/acl_oss_test.go

@@ -0,0 +1,28 @@
+// +build !consulent
+
+package consul
+
+import (
+	"github.com/hashicorp/consul/acl"
+	"github.com/hashicorp/consul/agent/structs"
+)
+
+func testIdentityForTokenEnterprise(string) (bool, structs.ACLIdentity, error) {
+	return true, nil, acl.ErrNotFound
+}
+
+func testPolicyForIDEnterprise(string) (bool, *structs.ACLPolicy, error) {
+	return true, nil, acl.ErrNotFound
+}
+
+func testRoleForIDEnterprise(string) (bool, *structs.ACLRole, error) {
+	return true, nil, acl.ErrNotFound
+}
+
+// EnterpriseACLResolverTestDelegate stub
+type EnterpriseACLResolverTestDelegate struct{}
+
+// RPC stub for the EnterpriseACLResolverTestDelegate
+func (d *EnterpriseACLResolverTestDelegate) RPC(string, interface{}, interface{}) (bool, error) {
+	return false, nil
+}

agent/consul/acl_test.go

@@ -40,6 +40,24 @@ type asyncResolutionResult struct {
 	err   error
 }
 
+func verifyAuthorizerChain(t *testing.T, expected acl.Authorizer, actual acl.Authorizer) {
+	expectedChainAuthz, ok := expected.(*acl.ChainedAuthorizer)
+	require.True(t, ok, "expected Authorizer is not a ChainedAuthorizer")
+	actualChainAuthz, ok := actual.(*acl.ChainedAuthorizer)
+	require.True(t, ok, "actual Authorizer is not a ChainedAuthorizer")
+
+	expectedChain := expectedChainAuthz.AuthorizerChain()
+	actualChain := actualChainAuthz.AuthorizerChain()
+
+	require.Equal(t, len(expectedChain), len(actualChain), "ChainedAuthorizers have different length chains")
+	for idx, expectedAuthz := range expectedChain {
+		actualAuthz := actualChain[idx]
+
+		// pointer equality - because we want to verify authorizer reuse
+		require.True(t, expectedAuthz == actualAuthz, "Authorizer pointers are not equal")
+	}
+}
+
 func resolveTokenAsync(r *ACLResolver, token string, ch chan *asyncResolutionResult) {
 	authz, err := r.ResolveToken(token)
 	ch <- &asyncResolutionResult{authz: authz, err: err}
@@ -226,7 +244,7 @@ func testIdentityForToken(token string) (bool, structs.ACLIdentity, error) {
 			},
 		}, nil
 	default:
-		return true, nil, acl.ErrNotFound
+		return testIdentityForTokenEnterprise(token)
 	}
 }
 
@@ -289,7 +307,7 @@ func testPolicyForID(policyID string) (bool, *structs.ACLPolicy, error) {
 			RaftIndex:   structs.RaftIndex{CreateIndex: 1, ModifyIndex: 2},
 		}, nil
 	default:
-		return true, nil, acl.ErrNotFound
+		return testPolicyForIDEnterprise(policyID)
 	}
 }
 
@@ -424,7 +442,7 @@ func testRoleForID(roleID string) (bool, *structs.ACLRole, error) {
 			},
 		}, nil
 	default:
-		return true, nil, acl.ErrNotFound
+		return testRoleForIDEnterprise(roleID)
 	}
 }
 
@@ -448,6 +466,8 @@ type ACLResolverTestDelegate struct {
 	policyCached bool
 	// state for the optional default resolver function defaultRoleResolveFn
 	roleCached bool
+
+	EnterpriseACLResolverTestDelegate
 }
 
 func (d *ACLResolverTestDelegate) Reset() {
@@ -590,6 +610,9 @@ func (d *ACLResolverTestDelegate) RPC(method string, args interface{}, reply int
 		}
 		panic("Bad Test Implementation: should provide a roleResolveFn to the ACLResolverTestDelegate")
 	}
+	if handled, err := d.EnterpriseACLResolverTestDelegate.RPC(method, args, reply); handled {
+		return err
+	}
 	panic("Bad Test Implementation: Was the ACLResolver updated to use new RPC methods")
 }
 
@@ -773,7 +796,7 @@ func TestACLResolver_DownPolicy(t *testing.T) {
 		authz2, err := r.ResolveToken("found")
 		require.NoError(t, err)
 		require.NotNil(t, authz2)
-		require.False(t, authz == authz2)
+		require.NotEqual(t, authz, authz2)
 		require.Equal(t, acl.Deny, authz2.NodeWrite("foo", nil))
 
 		requirePolicyCached(t, r, "node-wr", false, "expired")    // from "found" token
@@ -839,8 +862,7 @@ func TestACLResolver_DownPolicy(t *testing.T) {
 		authz2, err := r.ResolveToken("found")
 		require.NoError(t, err)
 		require.NotNil(t, authz2)
-		// testing pointer equality - these will be the same object because it is cached.
+		verifyAuthorizerChain(t, authz, authz2)
-		require.True(t, authz == authz2)
 		require.Equal(t, acl.Allow, authz2.NodeWrite("foo", nil))
 	})
 
@@ -872,7 +894,7 @@ func TestACLResolver_DownPolicy(t *testing.T) {
 		require.NoError(t, err)
 		require.NotNil(t, authz2)
 		// testing pointer equality - these will be the same object because it is cached.
-		require.True(t, authz == authz2)
+		verifyAuthorizerChain(t, authz, authz2)
 		require.Equal(t, acl.Allow, authz2.NodeWrite("foo", nil))
 	})
 
@@ -906,7 +928,7 @@ func TestACLResolver_DownPolicy(t *testing.T) {
 		authz2, err := r.ResolveToken("found")
 		require.NoError(t, err)
 		require.NotNil(t, authz2)
-		require.True(t, authz == authz2)
+		verifyAuthorizerChain(t, authz, authz2)
 		require.Equal(t, acl.Allow, authz.NodeWrite("foo", nil))
 
 		requirePolicyCached(t, r, "node-wr", true, "still cached")    // from "found" token
@@ -941,7 +963,8 @@ func TestACLResolver_DownPolicy(t *testing.T) {
 		authz2, err := r.ResolveToken("found-role")
 		require.NoError(t, err)
 		require.NotNil(t, authz2)
-		require.True(t, authz == authz2)
+		verifyAuthorizerChain(t, authz, authz2)
+
 		require.Equal(t, acl.Allow, authz.NodeWrite("foo", nil))
 	})
 
@@ -978,12 +1001,9 @@ func TestACLResolver_DownPolicy(t *testing.T) {
 		require.NoError(t, err)
 		require.NotNil(t, authz2)
 		// testing pointer equality - these will be the same object because it is cached.
-		require.True(t, authz == authz2)
+		verifyAuthorizerChain(t, authz, authz2)
 		require.Equal(t, acl.Allow, authz.NodeWrite("foo", nil))
 
-		requirePolicyCached(t, r, "node-wr", true, "cached")    // from "found" token
-		requirePolicyCached(t, r, "dc2-key-wr", true, "cached") // from "found" token
-
 		// the go routine spawned will eventually return with a authz that doesn't have the policy
 		retry.Run(t, func(t *retry.R) {
 			authz3, err := r.ResolveToken("found")
@@ -1027,7 +1047,7 @@ func TestACLResolver_DownPolicy(t *testing.T) {
 		require.NoError(t, err)
 		require.NotNil(t, authz2)
 		// testing pointer equality - these will be the same object because it is cached.
-		require.True(t, authz == authz2)
+		verifyAuthorizerChain(t, authz, authz2)
 		require.Equal(t, acl.Allow, authz.NodeWrite("foo", nil))
 
 		// the go routine spawned will eventually return with a authz that doesn't have the policy
@@ -1071,7 +1091,7 @@ func TestACLResolver_DownPolicy(t *testing.T) {
 		require.NoError(t, err)
 		require.NotNil(t, authz2)
 		// testing pointer equality - these will be the same object because it is cached.
-		require.True(t, authz == authz2)
+		verifyAuthorizerChain(t, authz, authz2)
 		require.Equal(t, acl.Allow, authz2.NodeWrite("foo", nil))
 	})
 
@@ -1108,7 +1128,7 @@ func TestACLResolver_DownPolicy(t *testing.T) {
 		require.NoError(t, err)
 		require.NotNil(t, authz2)
 		// testing pointer equality - these will be the same object because it is cached.
-		require.True(t, authz == authz2, "\n[1]={%+v} != \n[2]={%+v}", authz, authz2)
+		verifyAuthorizerChain(t, authz, authz2)
 		require.Equal(t, acl.Allow, authz2.NodeWrite("foo", nil))
 	})
 
@@ -1140,12 +1160,9 @@ func TestACLResolver_DownPolicy(t *testing.T) {
 		authz2, err := r.ResolveToken("found")
 		require.NoError(t, err)
 		require.NotNil(t, authz2)
-		// testing pointer equality - these will be the same object because it is cached.
+		verifyAuthorizerChain(t, authz, authz2)
-		require.True(t, authz == authz2)
 		require.Equal(t, acl.Allow, authz2.NodeWrite("foo", nil))
 
-		requireIdentityCached(t, r, "found", true, "cached")
-
 		// the go routine spawned will eventually return and this will be a not found error
 		retry.Run(t, func(t *retry.R) {
 			authz3, err := r.ResolveToken("found")

agent/structs/acl.go

@@ -169,7 +169,6 @@ func (s *ACLServiceIdentity) EstimateSize() int {
 func (s *ACLServiceIdentity) SyntheticPolicy(entMeta *EnterpriseMeta) *ACLPolicy {
 	// Given that we validate this string name before persisting, we do not
 	// have to escape it before doing the following interpolation.
-	// TODO (namespaces) include namespace
 	rules := aclServiceIdentityRules(s.ServiceName, entMeta)
 
 	hasher := fnv.New128a()
@@ -673,7 +672,7 @@ func (policies ACLPolicies) resolveWithCache(cache *ACLCaches, entConf *acl.Ente
 	return parsed, nil
 }
 
-func (policies ACLPolicies) Compile(parent acl.Authorizer, cache *ACLCaches, entConf *acl.EnterpriseACLConfig) (acl.Authorizer, error) {
+func (policies ACLPolicies) Compile(cache *ACLCaches, entConf *acl.EnterpriseACLConfig) (acl.Authorizer, error) {
 	// Determine the cache key
 	cacheKey := policies.HashKey()
 	entry := cache.GetAuthorizer(cacheKey)
@@ -688,7 +687,7 @@ func (policies ACLPolicies) Compile(parent acl.Authorizer, cache *ACLCaches, ent
 	}
 
 	// Create the ACL object
-	authorizer, err := acl.NewPolicyAuthorizerWithDefaults(parent, parsed, entConf)
+	authorizer, err := acl.NewPolicyAuthorizer(parsed, entConf)
 	if err != nil {
 		return nil, fmt.Errorf("failed to construct ACL Authorizer: %v", err)
 	}
@@ -1083,8 +1082,6 @@ type ACLTokenListResponse struct {
 type ACLTokenBatchGetRequest struct {
 	AccessorIDs []string // List of accessor ids to fetch
 	Datacenter  string   // The datacenter to perform the request within
-	// TODO (namespaces) should this use enterprise meta? - it would be hard to
-	// update in a backwards compatible manner an accessor ids should be unique
 	QueryOptions
 }
 
@@ -1111,7 +1108,6 @@ type ACLTokenBatchSetRequest struct {
 // multiple tokens need to be removed from the local DCs state.
 type ACLTokenBatchDeleteRequest struct {
 	TokenIDs []string // Tokens to delete
-	// TODO (namespaces) should we update with ent meta?
 }
 
 // ACLTokenBootstrapRequest is used only at the Raft layer
@@ -1496,8 +1492,6 @@ func (r *ACLLoginRequest) RequestDatacenter() string {
 
 type ACLLogoutRequest struct {
 	Datacenter string // The datacenter to perform the request within
-	// TODO (namespaces) do we need the ent meta here? tokens are again
-	// unique across namespaces so its likely we don't need it.
 	WriteRequest
 }
 

agent/structs/acl_test.go

@@ -661,7 +661,7 @@ func TestStructs_ACLPolicies_Compile(t *testing.T) {
 	}
 
 	t.Run("Cache Miss", func(t *testing.T) {
-		authz, err := testPolicies.Compile(acl.DenyAll(), cache, nil)
+		authz, err := testPolicies.Compile(cache, nil)
 		require.NoError(t, err)
 		require.NotNil(t, authz)
 
@@ -669,7 +669,7 @@ func TestStructs_ACLPolicies_Compile(t *testing.T) {
 		require.Equal(t, acl.Allow, authz.AgentRead("foo", nil))
 		require.Equal(t, acl.Allow, authz.KeyRead("foo", nil))
 		require.Equal(t, acl.Allow, authz.ServiceRead("foo", nil))
-		require.Equal(t, acl.Deny, authz.ACLRead(nil))
+		require.Equal(t, acl.Default, authz.ACLRead(nil))
 	})
 
 	t.Run("Check Cache", func(t *testing.T) {
@@ -682,18 +682,18 @@ func TestStructs_ACLPolicies_Compile(t *testing.T) {
 		require.Equal(t, acl.Allow, authz.AgentRead("foo", nil))
 		require.Equal(t, acl.Allow, authz.KeyRead("foo", nil))
 		require.Equal(t, acl.Allow, authz.ServiceRead("foo", nil))
-		require.Equal(t, acl.Deny, authz.ACLRead(nil))
+		require.Equal(t, acl.Default, authz.ACLRead(nil))
 
 		// setup the cache for the next test
 		cache.PutAuthorizer(testPolicies.HashKey(), acl.DenyAll())
 	})
 
 	t.Run("Cache Hit", func(t *testing.T) {
-		authz, err := testPolicies.Compile(acl.DenyAll(), cache, nil)
+		authz, err := testPolicies.Compile(cache, nil)
 		require.NoError(t, err)
 		require.NotNil(t, authz)
 
-		// we reset the Authorizer in the cache so now everything should be denied
+		// we reset the Authorizer in the cache so now everything should be defaulted
 		require.Equal(t, acl.Deny, authz.NodeRead("foo", nil))
 		require.Equal(t, acl.Deny, authz.AgentRead("foo", nil))
 		require.Equal(t, acl.Deny, authz.KeyRead("foo", nil))