Ensure that cache entries for tokens are prefixed “token-secret… (#6688)

This will be necessary once we store other types of identities in here.

agent/consul/acl.go

@@ -89,6 +89,10 @@ func IsACLRemoteError(err error) bool {
 	return ok
 }
 
+func tokenSecretCacheID(token string) string {
+	return "token-secret:" + token
+}
+
 type ACLResolverDelegate interface {
 	ACLsEnabled() bool
 	ACLDatacenter(legacy bool) string
@@ -343,6 +347,8 @@ func (r *ACLResolver) resolveTokenLegacy(token string) (acl.Authorizer, error) {
 }
 
 func (r *ACLResolver) fetchAndCacheIdentityFromToken(token string, cached *structs.IdentityCacheEntry) (structs.ACLIdentity, error) {
+	cacheID := tokenSecretCacheID(token)
+
 	req := structs.ACLTokenGetRequest{
 		Datacenter:  r.delegate.ACLDatacenter(false),
 		TokenID:     token,
@@ -357,17 +363,17 @@ func (r *ACLResolver) fetchAndCacheIdentityFromToken(token string, cached *struc
 	err := r.delegate.RPC("ACL.TokenRead", &req, &resp)
 	if err == nil {
 		if resp.Token == nil {
-			r.cache.PutIdentity(token, nil)
+			r.cache.PutIdentity(cacheID, nil)
 			return nil, acl.ErrNotFound
 		} else {
-			r.cache.PutIdentity(token, resp.Token)
+			r.cache.PutIdentity(cacheID, resp.Token)
 			return resp.Token, nil
 		}
 	}
 
 	if acl.IsErrNotFound(err) {
 		// Make sure to remove from the cache if it was deleted
-		r.cache.PutIdentity(token, nil)
+		r.cache.PutIdentity(cacheID, nil)
 		return nil, acl.ErrNotFound
 
 	}
@@ -375,11 +381,11 @@ func (r *ACLResolver) fetchAndCacheIdentityFromToken(token string, cached *struc
 	// some other RPC error
 	if cached != nil && (r.config.ACLDownPolicy == "extend-cache" || r.config.ACLDownPolicy == "async-cache") {
 		// extend the cache
-		r.cache.PutIdentity(token, cached.Identity)
+		r.cache.PutIdentity(cacheID, cached.Identity)
 		return cached.Identity, nil
 	}
 
-	r.cache.PutIdentity(token, nil)
+	r.cache.PutIdentity(cacheID, nil)
 	return nil, err
 }
 
@@ -390,7 +396,7 @@ func (r *ACLResolver) resolveIdentityFromToken(token string) (structs.ACLIdentit
 	}
 
 	// Check the cache before making any RPC requests
-	cacheEntry := r.cache.GetIdentity(token)
+	cacheEntry := r.cache.GetIdentity(tokenSecretCacheID(token))
 	if cacheEntry != nil && cacheEntry.Age() <= r.config.ACLTokenTTL {
 		metrics.IncrCounter([]string{"acl", "token", "cache_hit"}, 1)
 		return cacheEntry.Identity, nil
@@ -540,7 +546,7 @@ func (r *ACLResolver) maybeHandleIdentityErrorDuringFetch(identity structs.ACLId
 	if acl.IsErrNotFound(err) {
 		// make sure to indicate that this identity is no longer valid within
 		// the cache
-		r.cache.PutIdentity(identity.SecretToken(), nil)
+		r.cache.PutIdentity(tokenSecretCacheID(identity.SecretToken()), nil)
 
 		// Do not touch the cache. Getting a top level ACL not found error
 		// only indicates that the secret token used in the request
@@ -551,7 +557,7 @@ func (r *ACLResolver) maybeHandleIdentityErrorDuringFetch(identity structs.ACLId
 	if acl.IsErrPermissionDenied(err) {
 		// invalidate our ID cache so that identity resolution will take place
 		// again in the future
-		r.cache.RemoveIdentity(identity.SecretToken())
+		r.cache.RemoveIdentity(tokenSecretCacheID(identity.SecretToken()))
 
 		// Do not remove from the cache for permission denied
 		// what this does indicate is that our view of the token is out of date

agent/consul/acl_endpoint.go

@@ -634,7 +634,7 @@ func (a *ACL) tokenSetInternal(args *structs.ACLTokenSetRequest, reply *structs.
 	}
 
 	// Purge the identity from the cache to prevent using the previous definition of the identity
-	a.srv.acls.cache.RemoveIdentity(token.SecretID)
+	a.srv.acls.cache.RemoveIdentity(tokenSecretCacheID(token.SecretID))
 
 	if respErr, ok := resp.(error); ok {
 		return respErr
@@ -776,7 +776,7 @@ func (a *ACL) TokenDelete(args *structs.ACLTokenDeleteRequest, reply *string) er
 	}
 
 	// Purge the identity from the cache to prevent using the previous definition of the identity
-	a.srv.acls.cache.RemoveIdentity(token.SecretID)
+	a.srv.acls.cache.RemoveIdentity(tokenSecretCacheID(token.SecretID))
 
 	if respErr, ok := resp.(error); ok {
 		return respErr
@@ -2249,7 +2249,7 @@ func (a *ACL) Logout(args *structs.ACLLogoutRequest, reply *bool) error {
 
 	// Purge the identity from the cache to prevent using the previous definition of the identity
 	if token != nil {
-		a.srv.acls.cache.RemoveIdentity(token.SecretID)
+		a.srv.acls.cache.RemoveIdentity(tokenSecretCacheID(token.SecretID))
 	}
 
 	if respErr, ok := resp.(error); ok {

agent/consul/acl_endpoint_legacy.go

@@ -186,7 +186,7 @@ func (a *ACL) Apply(args *structs.ACLRequest, reply *string) error {
 
 	// Clear the cache if applicable
 	if args.ACL.ID != "" {
-		a.srv.acls.cache.RemoveIdentity(args.ACL.ID)
+		a.srv.acls.cache.RemoveIdentity(tokenSecretCacheID(args.ACL.ID))
 	}
 
 	return nil

agent/consul/acl_test.go

@@ -693,10 +693,10 @@ func TestACLResolver_ResolveRootACL(t *testing.T) {
 func TestACLResolver_DownPolicy(t *testing.T) {
 	t.Parallel()
 
-	requireIdentityCached := func(t *testing.T, r *ACLResolver, token string, present bool, msg string) {
+	requireIdentityCached := func(t *testing.T, r *ACLResolver, id string, present bool, msg string) {
 		t.Helper()
 
-		cacheVal := r.cache.GetIdentity(token)
+		cacheVal := r.cache.GetIdentity(id)
 		require.NotNil(t, cacheVal)
 		if present {
 			require.NotNil(t, cacheVal.Identity, msg)
@@ -738,7 +738,7 @@ func TestACLResolver_DownPolicy(t *testing.T) {
 		require.NotNil(t, authz)
 		require.Equal(t, authz, acl.DenyAll())
 
-		requireIdentityCached(t, r, "foo", false, "not present")
+		requireIdentityCached(t, r, tokenSecretCacheID("foo"), false, "not present")
 	})
 
 	t.Run("Allow", func(t *testing.T) {
@@ -763,7 +763,7 @@ func TestACLResolver_DownPolicy(t *testing.T) {
 		require.NotNil(t, authz)
 		require.Equal(t, authz, acl.AllowAll())
 
-		requireIdentityCached(t, r, "foo", false, "not present")
+		requireIdentityCached(t, r, tokenSecretCacheID("foo"), false, "not present")
 	})
 
 	t.Run("Expired-Policy", func(t *testing.T) {
@@ -857,7 +857,7 @@ func TestACLResolver_DownPolicy(t *testing.T) {
 		require.NotNil(t, authz)
 		require.Equal(t, acl.Allow, authz.NodeWrite("foo", nil))
 
-		requireIdentityCached(t, r, "found", true, "cached")
+		requireIdentityCached(t, r, tokenSecretCacheID("found"), true, "cached")
 
 		authz2, err := r.ResolveToken("found")
 		require.NoError(t, err)
@@ -888,7 +888,7 @@ func TestACLResolver_DownPolicy(t *testing.T) {
 		require.NotNil(t, authz)
 		require.Equal(t, acl.Allow, authz.NodeWrite("foo", nil))
 
-		requireIdentityCached(t, r, "found-role", true, "still cached")
+		requireIdentityCached(t, r, tokenSecretCacheID("found-role"), true, "still cached")
 
 		authz2, err := r.ResolveToken("found-role")
 		require.NoError(t, err)
@@ -1154,7 +1154,7 @@ func TestACLResolver_DownPolicy(t *testing.T) {
 		require.NotNil(t, authz)
 		require.Equal(t, acl.Allow, authz.NodeWrite("foo", nil))
 
-		requireIdentityCached(t, r, "found", true, "cached")
+		requireIdentityCached(t, r, tokenSecretCacheID("found"), true, "cached")
 
 		// The identity should have been cached so this should still be valid
 		authz2, err := r.ResolveToken("found")
@@ -1171,7 +1171,7 @@ func TestACLResolver_DownPolicy(t *testing.T) {
 			assert.Nil(t, authz3)
 		})
 
-		requireIdentityCached(t, r, "found", false, "no longer cached")
+		requireIdentityCached(t, r, tokenSecretCacheID("found"), false, "no longer cached")
 	})
 
 	t.Run("PolicyResolve-TokenNotFound", func(t *testing.T) {
@@ -1225,7 +1225,7 @@ func TestACLResolver_DownPolicy(t *testing.T) {
 		require.Equal(t, acl.Allow, authz.NodeWrite("foo", nil))
 
 		// Verify that the caches are setup properly.
-		requireIdentityCached(t, r, secretID, true, "cached")
+		requireIdentityCached(t, r, tokenSecretCacheID(secretID), true, "cached")
 		requirePolicyCached(t, r, "node-wr", true, "cached")    // from "found" token
 		requirePolicyCached(t, r, "dc2-key-wr", true, "cached") // from "found" token
 
@@ -1236,7 +1236,7 @@ func TestACLResolver_DownPolicy(t *testing.T) {
 		_, err = r.ResolveToken(secretID)
 		require.True(t, acl.IsErrNotFound(err))
 
-		requireIdentityCached(t, r, secretID, false, "identity not found cached")
+		requireIdentityCached(t, r, tokenSecretCacheID(secretID), false, "identity not found cached")
 		requirePolicyCached(t, r, "node-wr", true, "still cached")
 		require.Nil(t, r.cache.GetPolicy("dc2-key-wr"), "not stored at all")
 	})
@@ -1287,7 +1287,7 @@ func TestACLResolver_DownPolicy(t *testing.T) {
 		require.Equal(t, acl.Allow, authz.NodeWrite("foo", nil))
 
 		// Verify that the caches are setup properly.
-		requireIdentityCached(t, r, secretID, true, "cached")
+		requireIdentityCached(t, r, tokenSecretCacheID(secretID), true, "cached")
 		requirePolicyCached(t, r, "node-wr", true, "cached")    // from "found" token
 		requirePolicyCached(t, r, "dc2-key-wr", true, "cached") // from "found" token
 
@@ -1298,7 +1298,7 @@ func TestACLResolver_DownPolicy(t *testing.T) {
 		_, err = r.ResolveToken(secretID)
 		require.True(t, acl.IsErrPermissionDenied(err))
 
-		require.Nil(t, r.cache.GetIdentity(secretID), "identity not stored at all")
+		require.Nil(t, r.cache.GetIdentity(tokenSecretCacheID(secretID)), "identity not stored at all")
 		requirePolicyCached(t, r, "node-wr", true, "still cached")
 		require.Nil(t, r.cache.GetPolicy("dc2-key-wr"), "not stored at all")
 	})

agent/consul/acl_token_exp.go

@@ -106,7 +106,7 @@ func (s *Server) reapExpiredACLTokens(local, global bool) (int, error) {
 
 	// Purge the identities from the cache
 	for _, secretID := range secretIDs {
-		s.acls.cache.RemoveIdentity(secretID)
+		s.acls.cache.RemoveIdentity(tokenSecretCacheID(secretID))
 	}
 
 	if respErr, ok := resp.(error); ok {