feat: support contextual tuples and context in checks#169
feat: support contextual tuples and context in checks#169Adrastopoulos wants to merge 3 commits into
Conversation
Add check(...) overloads that accept contextual tuples and a context object, which are evaluated as if those tuples existed in the store and used to satisfy conditions defined in the authorization model. Closes openfga#43
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughOpenFga adds check overloads that accept contextual tuples and an optional context object. When user ID is omitted, the method reads Spring Security authentication to derive it. Tests cover request propagation, principal-based resolution, and the missing-authentication failure case. ChangesContextual check support
Sequence Diagram(s)sequenceDiagram
participant OpenFga
participant SecurityContextHolder
participant fgaClient
OpenFga->>SecurityContextHolder: getContext().getAuthentication()
SecurityContextHolder-->>OpenFga: Authentication or null
OpenFga->>OpenFga: delegate check(... userId, contextualTuples, context)
OpenFga->>fgaClient: check(ClientCheckRequest)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
Adds first-class support for OpenFGA “contextual tuples” and a request context payload when performing authorization checks via the OpenFga helper, enabling advanced evaluation scenarios (e.g., request-time group membership and condition inputs) while preserving existing check(...) method signatures used by Spring Security SpEL.
Changes:
- Introduces new
OpenFga.check(...)overloads that acceptList<ClientTupleKey> contextualTuplesand anObject context(both explicit-user and security-context-derived user forms). - Keeps existing
check(...)APIs intact by delegating to the new overloads withnullcontextual parameters. - Adds unit tests for contextual tuples + context, and clears
SecurityContextHolderafter each test to avoid cross-test leakage.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/main/java/dev/openfga/OpenFga.java | Adds check(...) overloads to optionally attach contextual tuples and condition context to ClientCheckRequest. |
| src/test/java/dev/openfga/OpenFgaTest.java | Adds tests for contextual tuple/context variants and clears SecurityContextHolder after each test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/java/dev/openfga/OpenFga.java (1)
94-106: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDelegate the legacy principal-derived overload through this path.
The new overload duplicates the same
SecurityContextHolderlookup and exception message that already exists incheck(..., userType). Point the 4-arg overload at this method (or extract a small helper) so auth-resolution behavior only has one implementation to keep in sync.Suggested diff
public boolean check(String objectType, String objectId, String relation, String userType) { - var authentication = SecurityContextHolder.getContext().getAuthentication(); - if (authentication == null) { - throw new IllegalStateException( - "No user provided, and no authentication could be found in the security context"); - } - return check(objectType, objectId, relation, userType, authentication.getName()); + return check(objectType, objectId, relation, userType, null, null); }🤖 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 `@src/main/java/dev/openfga/OpenFga.java` around lines 94 - 106, The 4-arg check overload duplicates the SecurityContextHolder authentication lookup and exception handling already present in the principal-derived path. Update check(String objectType, String objectId, String relation, String userType, List<ClientTupleKey> contextualTuples, Object context) to delegate through the existing overload that resolves the principal, or extract a shared helper used by both check methods. Keep the authentication resolution and IllegalStateException message in one place so OpenFga.check stays consistent.
🤖 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.
Nitpick comments:
In `@src/main/java/dev/openfga/OpenFga.java`:
- Around line 94-106: The 4-arg check overload duplicates the
SecurityContextHolder authentication lookup and exception handling already
present in the principal-derived path. Update check(String objectType, String
objectId, String relation, String userType, List<ClientTupleKey>
contextualTuples, Object context) to delegate through the existing overload that
resolves the principal, or extract a shared helper used by both check methods.
Keep the authentication resolution and IllegalStateException message in one
place so OpenFga.check stays consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7c234694-6217-4eaa-b648-16fdbaed1add
📒 Files selected for processing (2)
src/main/java/dev/openfga/OpenFga.javasrc/test/java/dev/openfga/OpenFgaTest.java
Adds
check(...)overloads that take contextual tuples and acontextobject, for the scenarios in #43 (group membership supplied per-request, attribute/time conditions, etc.). The existing signatures are untouched and just delegate with nulls, so@PreAuthorizeSpEL usage keeps working.Closes #43.