AuthZService: improve authz caching

[akshayutture-augment]

Test 2

---

Replicated from ai-code-review-evaluation/grafana-greptile#2

[greptile-apps]

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

[akshayutture-augment]

@greptileai

[greptile-apps]

Greptile Overview

Confidence Score: 2/5

• This PR has a critical logic bug that undermines the denial cache optimization
• The denial cache will not be populated when cached permissions indicate a denial, meaning the optimization only works for database-fetched denials. This is a functional correctness issue that reduces the effectiveness of the caching improvements
• pkg/services/authz/rbac/service.go requires attention - the Check method has a logic bug where the denial cache is not populated when using cached permissions

Important Files Changed

File Analysis

Filename	Score	Overview
pkg/services/authz/rbac/service.go	2/5	Introduced a permission denial cache and optimized Check/List methods to use cached permissions before DB lookup, but has critical logic bug where denials aren't cached when cached permissions indicate denial
pkg/services/authz/rbac/cache.go	5/5	Added new cache key function for permission denials with namespace, user, action, name, and parent folder as components
pkg/services/authz/rbac.go	4/5	Added NoopCache implementation for in-process client and refactored client creation to use authzlib options pattern
pkg/services/authz/rbac/service_test.go	5/5	Added comprehensive tests for cache behavior in Check and List operations, removed outdated test for removed caching logic in getUserPermissions

Sequence Diagram

sequenceDiagram
    participant Client
    participant Service
    participant DenialCache
    participant PermCache
    participant Database

    Client->>Service: Check(request)
    Service->>DenialCache: Get(permDenialKey)
    alt denial cached
        DenialCache-->>Service: found=true
        Service-->>Client: Allowed=false (cached)
    else denial not cached
        DenialCache-->>Service: found=false
        Service->>PermCache: Get(userPermKey)
        alt permissions cached
            PermCache-->>Service: cached permissions
            Service->>Service: checkPermission(cachedPerms)
            alt allowed
                Service-->>Client: Allowed=true (cached)
            else denied
                Note over Service,DenialCache: BUG: denial not cached here
                Service-->>Client: Allowed=false (not cached)
            end
        else permissions not cached
            PermCache-->>Service: not found
            Service->>Database: getIdentityPermissions()
            Database-->>Service: fresh permissions
            Service->>PermCache: Set(permissions)
            Service->>Service: checkPermission(permissions)
            alt denied
                Service->>DenialCache: Set(permDenialKey, true)
            end
            Service-->>Client: Allowed=result
        end
    end

Loading

[greptile-apps]

_{4 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

logic: denial cache won't be populated when cached permissions indicate denial

When getCachedIdentityPermissions returns cached permissions and checkPermission determines allowed=false, the code returns early without caching the denial (line 135 exits before reaching line 154 where denials are cached). This means subsequent identical requests will repeatedly call getCachedIdentityPermissions and checkPermission instead of hitting the denial cache.

The denial cache is only populated when permissions are fetched from the database (line 154), not when using cached permissions.

Prompt To Fix With AI

This is a comment left during a code review.
Path: pkg/services/authz/rbac/service.go
Line: 123:136

Comment:
**logic:** denial cache won't be populated when cached permissions indicate denial

When `getCachedIdentityPermissions` returns cached permissions and `checkPermission` determines `allowed=false`, the code returns early without caching the denial (line 135 exits before reaching line 154 where denials are cached). This means subsequent identical requests will repeatedly call `getCachedIdentityPermissions` and `checkPermission` instead of hitting the denial cache.

The denial cache is only populated when permissions are fetched from the database (line 154), not when using cached permissions.

How can I resolve this? If you propose a fix, please make it concise.