From 3bd001fb298cc6b863841f8c7bee605a72df4b0e Mon Sep 17 00:00:00 2001
From: Kyle Havlovitz <kylehav@gmail.com>
Date: Tue, 3 May 2022 09:53:27 -0700
Subject: [PATCH] Return ACLRemoteError from cache and test it correctly

---
 .changelog/12885.txt     |  3 +++
 agent/consul/acl.go      |  6 ++++++
 agent/consul/acl_test.go | 15 ++++++++++++---
 3 files changed, 21 insertions(+), 3 deletions(-)
 create mode 100644 .changelog/12885.txt

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