AuthZService: improve authz caching

Hi there 👋

🌥️ Tech	🌤️ Feat
2 high, 2 medium, 4 low	1 ok, 3 mitigated

This MR introduces significant caching changes to the authz service to fix a staleness bug where newly created dashboards/folders are not immediately accessible to their creators. The client-side in-proc path correctly disables the authLib cache (using a NoopCache), and the server side adds a denial cache and a requery-on-miss path for Check. However, the review found several correctness issues that undermine the PR's stated goal:

• Stale denial cache: A denial entry is cached for up to shortCacheTTL (30s) and is never revalidated on hit. This re-introduces the exact staleness the MR is meant to fix — a user granted permission after a denial will still be denied for up to 30s.
• Renderer denial collision: The denial cache key omits identity type. Anonymous users and the render service both resolve to UID "0", so a cached anonymous denial can deny the renderer (which is unconditionally granted read) for public dashboards, because the denial cache is checked before the renderer's always-grant.
• List uses stale cache without requery: Unlike Check, List does not requery the DB on a stale cache hit, so newly created resources remain missing from listing results until TTL expiration.
• Metric ambiguity: Denial-cache hits are counted against permissionCacheUsage, muddying monitoring.
• Insufficient test coverage: The new denial-cache SET/short-circuit paths and List requery fallback are not actually tested; the "deny" test uses an impossible cached state.

On the client-side API, the changes are backward compatible; minor concerns are an unnecessarily exported NoopCache, an asymmetric caching behavior between in-proc and remote paths (intentional, but should be documented), and a misleading receiver name. Overall, the blocking issues on the server-side caching logic require fixes before merge.

---

These might need a close look

• 🔴 pkg/services/authz/rbac/service.go (L117)
  On a denial-cache hit, Check returns Allowed:false immediately and never requeries the DB. Unlike permCache, a denial entry is never invalidated or refreshed on hit — it lives until shortCacheTTL (30s). This re-introduces the staleness the MR is meant to fix: a user denied before being granted (e.g., after creating a dashboard or an admin grants a parent folder) will be denied for up to 30s. Document this tradeoff and ideally wire up invalidation on permission changes. Edge case: an action-level check with empty Name that currently has no permissions caches a denial under name="", parent="", masking later grants for that action.
• 🔴 pkg/services/authz/rbac/cache.go (L31)
  userPermDenialCacheKey is namespace + ".perm_" + userUID + "_" + action + "_" + name + "_" + parent and omits identity type. Anonymous and render subjects both resolve to UID "0", so they share the denial-cache namespace. Since the denial-cache lookup in Check runs before getCachedIdentityPermissions, the renderer's unconditional dashboards:read/folders:read grant is never consulted when a denial is cached. A cached anonymous denial on a dashboard can break rendering of public dashboards for up to 30s. Fix: include identityType in the key or skip the denial cache for TypeRenderService.

Worth checking

• 🟡 pkg/services/authz/rbac/service.go (L180)
  In List, when getCachedIdentityPermissions returns a cached scope map (even a stale one), it is used directly; getIdentityPermissions is only called on a full cache miss. There is no per-resource requery as in Check. A user who creates a dashboard and is granted permission will not see it in List results until the permCache TTL expires (30s). This is inconsistent with the new Check behavior and undermines the "fetch newly created dashboards/folders" goal for listing. Reconcile the TTL/requery policy or document eventual consistency.
• 🟡 pkg/services/authz/rbac/service_test.go (L963)
  The new denial-cache behavior is not properly tested: (1) "Should deny on explicit cache deny entry" seeds permCache with false values, but getScopeMap never produces false — the test returns false even without the denial cache, so it doesn't prove the short-circuit; it should seed true and assert the denial cache still wins. (2) There is no test for the denial-cache SET path (real DB deny populates cache, second Check returns fast without a DB call). (3) TestService_CacheList only covers the cache-hit path — no List cache-miss/requery fallback test. (4) No test covers the anonymous/render UID collision or the stale-denial-after-grant scenario.

Small things (take or leave)

• 🔵 pkg/services/authz/rbac/service.go (L118)
  On a denial-cache hit, permissionCacheUsage is incremented with label "true", conflating denial-cache hits with permission-cache hits. This inflates the permission-cache hit-rate metric and muddies monitoring. Consider a separate counter (or label) for denial-cache hits/misses. Also note a permCache hit that denies the resource and falls through to the DB is counted as "false" even though the permission cache was hit — semantics are now ambiguous.
• 🔵 pkg/services/authz/rbac.go (L239)
  NoopCache is a newly exported type in the public pkg/services/authz package but is only used internally (single usage at line 103). Exporting it pollutes the public API surface with no benefit since there are no external callers. Recommend unexporting it (noopCache), or add a doc comment if it must stay exported.
• 🔵 pkg/services/authz/rbac.go (L103)
  The in-proc client path now disables client-side caching entirely (NoopCache) while the remote path (newRemoteRBACClient, ~line 162) still uses a 30s TTL LocalCache. This asymmetry is intentional per the PR (server-side caches replace the client cache), but it should be explicitly documented so future readers don't "fix" it back and re-introduce the original staleness bug.
• 🔵 pkg/services/authz/rbac.go (L241)
  The receiver on all three NoopCache methods is named lc (reminiscent of "local cache"), which is misleading for a no-op implementation and likely copy-pasted from LocalCache. Consider renaming the receiver (e.g., n or nop) for clarity.

---

✅ Feature-level checklist

Looks good

• ✅ Remove the use of authLib cache when running authZ service in-proc
  The in-proc client path now uses NoopCache, which always misses, effectively disabling the client-side authLib cache. Backward compatible with existing exported signatures.

Partially covered

• ⚠️ Add a denial cache to return quickly when we know a user does not have permission on a specific resource
  The denial cache is implemented and short-circuits on hit. However, it serves stale denials up to shortCacheTTL without revalidation, and the key omits identity type, allowing a cached anonymous denial to block the renderer. These correctness issues must be addressed.
• ⚠️ If the permission denial cache is not hit and the requested resource is not found in cached permissions, requery permissions from the DB
  Check requeries the DB when the specific resource is absent from cached permissions. However, List does not requery on a stale cache hit, so newly created resources remain missing from listing results until TTL expiry — inconsistent with the stated requirement.
• ⚠️ Allow users to fetch newly created dashboards and folders (fixed by the above changes)
  Partially achieved: the Check path can now fetch newly created resources, but List still uses stale cached permissions without requery, and the stale denial cache can mask new grants for up to 30s. These gaps undermine the core goal and need resolution.