diff --git a/.changelog/12885.txt b/.changelog/12885.txt new file mode 100644 index 000000000..cfc9b3982 --- /dev/null +++ b/.changelog/12885.txt @@ -0,0 +1,3 @@ +```release-note:bug +acl: Fixed a bug where the ACL down policy wasn't being applied on remote errors from the primary datacenter. +``` diff --git a/agent/consul/acl.go b/agent/consul/acl.go index f76e71f8b..cf190ea71 100644 --- a/agent/consul/acl.go +++ b/agent/consul/acl.go @@ -402,6 +402,9 @@ func (r *ACLResolver) resolveIdentityFromToken(token string) (structs.ACLIdentit cacheEntry := r.cache.GetIdentity(tokenSecretCacheID(token)) if cacheEntry != nil && cacheEntry.Age() <= r.config.ACLTokenTTL { metrics.IncrCounter([]string{"acl", "token", "cache_hit"}, 1) + if cacheEntry.Error != nil && !acl.IsErrNotFound(cacheEntry.Error) { + return cacheEntry.Identity, ACLRemoteError{Err: cacheEntry.Error} + } return cacheEntry.Identity, cacheEntry.Error } @@ -416,6 +419,9 @@ func (r *ACLResolver) resolveIdentityFromToken(token string) (structs.ACLIdentit waitForResult := cacheEntry == nil || r.config.ACLDownPolicy != "async-cache" if !waitForResult { // waitForResult being false requires the cacheEntry to not be nil + if cacheEntry.Error != nil && !acl.IsErrNotFound(cacheEntry.Error) { + return cacheEntry.Identity, ACLRemoteError{Err: cacheEntry.Error} + } return cacheEntry.Identity, cacheEntry.Error } diff --git a/agent/consul/acl_test.go b/agent/consul/acl_test.go index 533987512..86e5ad478 100644 --- a/agent/consul/acl_test.go +++ b/agent/consul/acl_test.go @@ -1269,6 +1269,7 @@ func TestACLResolver_DownPolicy(t *testing.T) { foundToken := rawToken.(*structs.ACLToken) secretID := foundToken.SecretID + remoteErr := fmt.Errorf("network error") delegate := &ACLResolverTestDelegate{ enabled: true, datacenter: "dc1", @@ -1276,11 +1277,11 @@ func TestACLResolver_DownPolicy(t *testing.T) { localTokens: false, localPolicies: false, tokenReadFn: func(_ *structs.ACLTokenGetRequest, reply *structs.ACLTokenResponse) error { - return acl.ErrPermissionDenied + return remoteErr }, } r := newTestACLResolver(t, delegate, func(config *ACLResolverConfig) { - config.Config.ACLDownPolicy = "extend-cache" + config.Config.ACLDownPolicy = "deny" }) // Attempt to resolve the token - this should set up the cache entry with a nil @@ -1290,7 +1291,15 @@ func TestACLResolver_DownPolicy(t *testing.T) { require.NotNil(t, authz) entry := r.cache.GetIdentity(tokenSecretCacheID(secretID)) require.Nil(t, entry.Identity) - require.Equal(t, acl.ErrPermissionDenied, entry.Error) + require.Equal(t, remoteErr, entry.Error) + + // Attempt to resolve again to pull from the cache. + authz, err = r.ResolveToken(secretID) + require.NoError(t, err) + require.NotNil(t, authz) + entry = r.cache.GetIdentity(tokenSecretCacheID(secretID)) + require.Nil(t, entry.Identity) + require.Equal(t, remoteErr, entry.Error) }) t.Run("PolicyResolve-TokenNotFound", func(t *testing.T) {