feat: clarify the impact of conditions in list-objects/list-users queries#1266
Open
aaguiarz wants to merge 1 commit into
Open
feat: clarify the impact of conditions in list-objects/list-users queries#1266aaguiarz wants to merge 1 commit into
aaguiarz wants to merge 1 commit into
Conversation
Contributor
WalkthroughThis PR adds documentation clarifying how OpenFGA Conditions interact with enumeration APIs and tuple uniqueness. A new "Limitations" subsection is added to the Conditions guide, with corresponding callouts in the List Objects, List Users, and ABAC modeling guides that link back to the limitations reference. ChangesConditions Limitations Documentation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 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 |
Contributor
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
docs/content/modeling/conditions.mdx (2)
273-274: ⚡ Quick winUse consistent terminology across documentation files.
This says "ListUsers does not work as expected" while lines 190 in
perform-list-users.mdxand 187 inperform-list-objects.mdxuse "may not return correct results." The latter phrasing is more precise since the API can return partial or incorrect results rather than failing entirely.Suggested consistency fix
- listUsers does not work as expected: each candidate user has a different `user_clearance`, but only one value can be passed per call. + ListUsers may not return correct results: each candidate user has a different `user_clearance`, but only one value can be passed per call.🤖 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 `@docs/content/modeling/conditions.mdx` around lines 273 - 274, Update the wording in the sentence referencing ListUsers so it uses the same phrasing as the other docs: replace "does not work as expected" with "may not return correct results" for the ListUsers example; locate the sentence mentioning "ListUsers" in conditions.mdx and make the exact phrasing change so all three docs use "may not return correct results" consistently.
270-274: 💤 Low valueClarify that the example condition is hypothetical.
The
clearance_sufficientcondition is introduced here as an example but isn't defined in the authorization model shown earlier in this document. Consider adding a brief note like "For example, consider a hypothetical condition..." to make it clear this is for illustration purposes.🤖 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 `@docs/content/modeling/conditions.mdx` around lines 270 - 274, The example uses an undefined condition name and should be labeled as hypothetical: update the paragraph introducing clearance_sufficient to explicitly say it is a hypothetical/example condition (e.g., "For example, consider a hypothetical condition clearance_sufficient(user_clearance: int, resource_sensitivity: int) ..."), so readers know this condition is not defined in the earlier authorization model; keep references to ListObjects, ListUsers, user_clearance, and resource_sensitivity unchanged to preserve the illustration.
🤖 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 `@docs/content/best-practices/modeling-abac.mdx`:
- Line 347: Add a period at the end of the sentence in the "Enumeration queries"
paragraph so the final clause reads "...See Conditions: Limitations for
details." Locate the paragraph that begins with the bold heading "Enumeration
queries" and references ListObjects and ListUsers and append a trailing period
after "for details".
---
Nitpick comments:
In `@docs/content/modeling/conditions.mdx`:
- Around line 273-274: Update the wording in the sentence referencing ListUsers
so it uses the same phrasing as the other docs: replace "does not work as
expected" with "may not return correct results" for the ListUsers example;
locate the sentence mentioning "ListUsers" in conditions.mdx and make the exact
phrasing change so all three docs use "may not return correct results"
consistently.
- Around line 270-274: The example uses an undefined condition name and should
be labeled as hypothetical: update the paragraph introducing
clearance_sufficient to explicitly say it is a hypothetical/example condition
(e.g., "For example, consider a hypothetical condition
clearance_sufficient(user_clearance: int, resource_sensitivity: int) ..."), so
readers know this condition is not defined in the earlier authorization model;
keep references to ListObjects, ListUsers, user_clearance, and
resource_sensitivity unchanged to preserve the illustration.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 660cbb5a-48c3-49b3-90cd-dc9dc402cb2f
📒 Files selected for processing (4)
docs/content/best-practices/modeling-abac.mdxdocs/content/getting-started/perform-list-objects.mdxdocs/content/getting-started/perform-list-users.mdxdocs/content/modeling/conditions.mdx
| - **Data volatility**: Use stored attributes for stable data, request-time attributes for dynamic data | ||
| - **Model complexity**: Start with simpler patterns (like `user:*`) and evolve to more complex ones as needed | ||
| - **Attribute types**: Use conditions when you need typed values beyond booleans | ||
| - **Enumeration queries**: Stored-attribute patterns (relationship tuples) work fully with [ListObjects](../getting-started/perform-list-objects.mdx) and [ListUsers](../getting-started/perform-list-users.mdx) without requiring context. Conditional relationship tuples have limitations with these queries — if a condition parameter varies per user, ListUsers cannot supply the correct value for each candidate, and similarly for ListObjects with per-object attributes not stored in the tuple context. See [Conditions: Limitations](../modeling/conditions.mdx#conditions-and-enumeration-queries-listobjects--listusers) for details |
Contributor
There was a problem hiding this comment.
Add missing period at end of sentence.
The line ends with "for details" but is missing a period.
Punctuation fix
-- **Enumeration queries**: Stored-attribute patterns (relationship tuples) work fully with [ListObjects](../getting-started/perform-list-objects.mdx) and [ListUsers](../getting-started/perform-list-users.mdx) without requiring context. Conditional relationship tuples have limitations with these queries — if a condition parameter varies per user, ListUsers cannot supply the correct value for each candidate, and similarly for ListObjects with per-object attributes not stored in the tuple context. See [Conditions: Limitations](../modeling/conditions.mdx#conditions-and-enumeration-queries-listobjects--listusers) for details
+- **Enumeration queries**: Stored-attribute patterns (relationship tuples) work fully with [ListObjects](../getting-started/perform-list-objects.mdx) and [ListUsers](../getting-started/perform-list-users.mdx) without requiring context. Conditional relationship tuples have limitations with these queries — if a condition parameter varies per user, ListUsers cannot supply the correct value for each candidate, and similarly for ListObjects with per-object attributes not stored in the tuple context. See [Conditions: Limitations](../modeling/conditions.mdx#conditions-and-enumeration-queries-listobjects--listusers) for details.🤖 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 `@docs/content/best-practices/modeling-abac.mdx` at line 347, Add a period at
the end of the sentence in the "Enumeration queries" paragraph so the final
clause reads "...See Conditions: Limitations for details." Locate the paragraph
that begins with the bold heading "Enumeration queries" and references
ListObjects and ListUsers and append a trailing period after "for details".
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the documentation around a known limitation of using conditions with enumeration-style relationship queries, clarifying when ListObjects / ListUsers may produce surprising results due to request context being shared across the whole call.
Changes:
- Added a new “Conditions and enumeration queries” limitation section to the conditions documentation.
- Added
:::infocallouts to the “Perform a List Objects call” and “Perform a List Users call” guides to highlight the limitation and link to the detailed explanation. - Updated the ABAC modeling best-practices page to mention enumeration-query implications when choosing between modeling approaches.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| docs/content/modeling/conditions.mdx | Documents enumeration-query limitations for conditions under the “Limitations” section. |
| docs/content/getting-started/perform-list-users.mdx | Adds an info callout warning about conditions with per-user attributes in ListUsers. |
| docs/content/getting-started/perform-list-objects.mdx | Adds an info callout warning about conditions with per-object attributes in ListObjects. |
| docs/content/best-practices/modeling-abac.mdx | Adds guidance about enumeration-query compatibility when selecting ABAC modeling patterns. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+264
to
+268
| The Check, ListObjects, and ListUsers APIs all accept a single `context` per call. This means every condition parameter must have one value for the entire query. When a condition compares a per-user attribute against a per-object attribute, one side varies across the enumeration set and cannot be supplied as a single value: | ||
|
|
||
| - **ListObjects** iterates over objects. If a condition parameter is a per-object attribute that is NOT stored in the tuple context, it varies across the enumeration set and there is no single value to pass. | ||
| - **ListUsers** iterates over users. If a condition parameter is a per-user attribute, it varies across the enumeration set and there is no single value to pass. | ||
| - **Check** evaluates a single (user, object) pair. Both sides are fixed, so conditions always work. |
| ::: | ||
|
|
||
| :::info Conditions and List Users | ||
| When a relation uses a [condition](../modeling/conditions.mdx) with a per-user attribute (e.g., `user_clearance`), the List Users API may not return correct results. The API accepts a single `context` per call, but each candidate user may have a different value for that attribute. If you need to enumerate users based on a per-user attribute comparison, consider modeling the attribute as a [type hierarchy using relationship tuples](../best-practices/modeling-abac.mdx) instead of a condition. See [Conditions: Limitations](../modeling/conditions.mdx#conditions-and-enumeration-queries-listobjects--listusers) for details. |
| - **Data volatility**: Use stored attributes for stable data, request-time attributes for dynamic data | ||
| - **Model complexity**: Start with simpler patterns (like `user:*`) and evolve to more complex ones as needed | ||
| - **Attribute types**: Use conditions when you need typed values beyond booleans | ||
| - **Enumeration queries**: Stored-attribute patterns (relationship tuples) work fully with [ListObjects](../getting-started/perform-list-objects.mdx) and [ListUsers](../getting-started/perform-list-users.mdx) without requiring context. Conditional relationship tuples have limitations with these queries — if a condition parameter varies per user, ListUsers cannot supply the correct value for each candidate, and similarly for ListObjects with per-object attributes not stored in the tuple context. See [Conditions: Limitations](../modeling/conditions.mdx#conditions-and-enumeration-queries-listobjects--listusers) for details |
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
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.
Description
What problem is being solved?
Documents a known limitation: ListObjects/ListUsers accept only a single context per call, so conditions with per-user or per-object attributes that vary across the enumeration set cannot be evaluated correctly.
How is it being solved?
What changes are made to solve it?
References
Review Checklist
mainSummary by CodeRabbit
Documentation