fix(authz): bind eval cache key to user token so re-login forces re-evaluation#549
Merged
Merged
Conversation
…valuation Cache key was `(userEntityRef, action, resourcePath, environment)` — stable across sign-outs. A stale ABAC decision from before a binding change survived re-login for the full JWT TTL (up to 24h), and the only recovery was restarting the backend or waiting it out. Fold a token-derived hash into the key (mirroring AuthzProfileCache's existing buildKey pattern) so a new sign-in produces a new key and re-evaluates against the current binding state. Signed-off-by: Kavith Lokuhewage <kaviththiranga@gmail.com>
kaviththiranga
requested review from
sameerajayasoma and
stefinie123
as code owners
binoyPeries
approved these changes
May 15, 2026
📝 WalkthroughWalkthroughAuthorization evaluation cache entries are now isolated per user token session. ChangesToken-scoped evaluation cache keying
Sequence Diagram(s)sequenceDiagram
participant Service as AuthzProfileService
participant Cache as AuthzProfileCache
participant Backend as ABAC Backend
participant Token as User Token
Service->>Token: extract token from context
Service->>Service: compute tokenHash from userToken
Service->>Cache: getEvaluation(..., tokenHash, ...)
alt cache hit
Cache-->>Service: return cached allowed boolean
else cache miss
Service->>Backend: POST evaluate decision
Backend-->>Service: return allowed
Service->>Cache: setEvaluation(..., tokenHash, ..., allowed)
Cache-->>Service: cache stored
end
Service-->>Service: return allowed
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
so the updated conditions will only take effect from the backstage side only after a relogin? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugins/permission-backend-module-openchoreo-policy/src/services/AuthzProfileCache.ts (1)
186-191: ⚡ Quick winReuse the shared token-hash helper in
buildKey()to avoid hash drift.
buildKey()still inlines hashing whilehashToken()now centralizes the same logic. Using one helper prevents future divergence between capabilities and evaluation key partitioning.♻️ Proposed refactor
private buildKey( userToken: string, org: string, project?: string, component?: string, ): string { - const tokenHash = crypto - .createHash('sha256') - .update(userToken) - .digest('hex') - .substring(0, 16); + const tokenHash = AuthzProfileCache.hashToken(userToken); const parts = ['openchoreo', 'capabilities', tokenHash, org]; if (project) parts.push(project); if (component) parts.push(component);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/permission-backend-module-openchoreo-policy/src/services/AuthzProfileCache.ts` around lines 186 - 191, buildKey() currently duplicates the token hashing logic while hashToken() centralizes it; change buildKey() to call AuthzProfileCache.hashToken(userToken) (or the class/static method hashToken) instead of re-implementing the sha256 + substring logic so both key construction and token hashing share the same helper and avoid future drift between capabilities and evaluation key partitioning.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@plugins/permission-backend-module-openchoreo-policy/src/services/AuthzProfileService.ts`:
- Around line 347-355: The evaluate() method's docstring is out of date: update
the comment on AuthzProfileService.evaluate (and any related doc for
AuthzProfileCache) to include tokenHash (derived via
AuthzProfileCache.hashToken(userToken)) as part of the cache key in addition to
(userEntityRef, action, resourcePath, environment) so the documentation matches
the implementation that includes the token/session component in cache key
generation.
---
Nitpick comments:
In
`@plugins/permission-backend-module-openchoreo-policy/src/services/AuthzProfileCache.ts`:
- Around line 186-191: buildKey() currently duplicates the token hashing logic
while hashToken() centralizes it; change buildKey() to call
AuthzProfileCache.hashToken(userToken) (or the class/static method hashToken)
instead of re-implementing the sha256 + substring logic so both key construction
and token hashing share the same helper and avoid future drift between
capabilities and evaluation key partitioning.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9c3ce9f7-ae3f-4cdc-9f8c-4c238e5a0d74
📒 Files selected for processing (3)
plugins/permission-backend-module-openchoreo-policy/src/services/AuthzProfileCache.tsplugins/permission-backend-module-openchoreo-policy/src/services/AuthzProfileService.test.tsplugins/permission-backend-module-openchoreo-policy/src/services/AuthzProfileService.ts
Comment on lines
+347
to
+355
| // Include a token-derived component in the cache key so that signing out | ||
| // and back in produces a new key and forces a re-evaluation. Without this, | ||
| // a stale `false` from before an authz binding change would survive | ||
| // re-login for the full JWT TTL (up to 24h on our setup), making the only | ||
| // recovery paths "wait" or "restart the backend". | ||
| const tokenHash = userToken | ||
| ? AuthzProfileCache.hashToken(userToken) | ||
| : 'no-token'; | ||
|
|
There was a problem hiding this comment.
Update the evaluate() cache-key doc to include tokenHash.
The method comment still states caching is per (userEntityRef, action, resourcePath, environment), but the implementation now keys by token session too. Please align the docstring.
📝 Suggested doc fix
- * per `(userEntityRef, action, resourcePath, environment)` for the lifetime
+ * per `(userEntityRef, tokenHash, action, resourcePath, environment)` for the lifetime🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@plugins/permission-backend-module-openchoreo-policy/src/services/AuthzProfileService.ts`
around lines 347 - 355, The evaluate() method's docstring is out of date: update
the comment on AuthzProfileService.evaluate (and any related doc for
AuthzProfileCache) to include tokenHash (derived via
AuthzProfileCache.hashToken(userToken)) as part of the cache key in addition to
(userEntityRef, action, resourcePath, environment) so the documentation matches
the implementation that includes the token/session component in cache key
generation.
Contributor
Author
@binoyPeries yes - or after 24hours TTL |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cache key was
(userEntityRef, action, resourcePath, environment)—stable across sign-outs. A stale ABAC decision from before a binding
change survived re-login for the full JWT TTL (up to 24h), and the only
recovery was restarting the backend or waiting it out.
Fold a token-derived hash into the key (mirroring AuthzProfileCache's
existing buildKey pattern) so a new sign-in produces a new key and
re-evaluates against the current binding state.
Summary by CodeRabbit