docs(getting-started): review of authorization model for typos and clarity #2708
Conversation
📝 WalkthroughWalkthroughA single documentation file, Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
Pre-merge checks✅ Passed checks (3 passed)
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 |
mooreds
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/getting-started/modeling.mdx (2)
243-262: Multiple unresolved TODO comments in the Permissions section.Lines 244, 257, and 262 contain TODO markers indicating incomplete documentation. These should be addressed or removed before merging.
369-405: Remove or resolve three unresolved TODO markers in the ABAC section.The documentation contains three incomplete TODOs:
- Line 369: "TODO where are" — incomplete sentence fragment
- Line 398: "TODO is this still true" — explicitly questions whether the beta feature status (lines 395-397) is still accurate
- Line 403: "TODO" — bare marker with no explanation
Either complete these comments with specific guidance or remove them. For line 398 in particular, verify the current status of the "Using Attributes Across Entities" feature and either confirm it remains in beta or update the documentation accordingly.
🧹 Nitpick comments (1)
docs/getting-started/modeling.mdx (1)
172-172: Consider clarifying the phrasing.The current sentence is slightly confusing. The phrase "either be a
useror that this user must be ateam member" conflates two different concepts.🔎 Suggested clarification
-The maintainer relation shows that the maintainer can either be a `user` or that this user must be a `team member`. +The maintainer relation shows that the maintainer can either be a direct `user` reference or a `member` of a `team`.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/getting-started/modeling.mdx
🧰 Additional context used
🪛 LanguageTool
docs/getting-started/modeling.mdx
[style] ~17-~17: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...les such as admin, manager, or member. You can also use dynamic attributes in your...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~21-~21: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...permify/master/assets/permify-dsl.gif) You can define your entities, relations bet...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~36-~36: The official name of this software platform is spelled with a capital “H”.
Context: ...e full implementation of the simplified Github access control example using Permify Sc...
(GITHUB)
[grammar] ~308-~308: Use a hyphen to join words.
Context: ...ttributes can be found in the [Attribute Based Permissions (ABAC)](#attribute-bas...
(QB_NEW_EN_HYPHEN)
[grammar] ~329-~329: Use a hyphen to join words.
Context: ...mmon permission use cases. ## Attribute Based Permissions (ABAC) To support Att...
(QB_NEW_EN_HYPHEN)
[grammar] ~331-~331: Use a hyphen to join words.
Context: ...Permissions (ABAC) To support Attribute Based Access Control (ABAC), there are t...
(QB_NEW_EN_HYPHEN)
[uncategorized] ~468-~468: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...comprehensive schema examples from the [Real World Examples](/getting-started/examples/int...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~474-~474: Replacing this phrase with a shorter alternative might make your text sound more refined.
Context: ...ess rights** in the child entities that belong to it. - [Instagram]: how **public/private at...
(BELONG_TO_PRP)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Scan Container for Vulnerabilities
- GitHub Check: Analyze (go)
- GitHub Check: Test with Coverage
- GitHub Check: Test with Coverage
🔇 Additional comments (7)
docs/getting-started/modeling.mdx (7)
6-8: LGTM!The introduction clearly explains the core concept and properly expands the ReBAC acronym.
59-79: LGTM!Clear and practical explanation of entities with good advice on naming conventions.
110-145: LGTM!Clear explanation of roles, user types, and parent-child relationships with appropriate examples.
210-216: LGTM!Good clarification that
actionandpermissionkeywords are interchangeable.
318-325: LGTM!Clear explanation of permission unions with a practical example showing how
deleteinherits fromedit.
466-476: LGTM!The examples section provides a clear overview of what each comprehensive example demonstrates.
1-4: Overall: Good clarity improvements with a few items to address.This PR makes helpful improvements to the documentation's clarity and terminology. However, there are several unresolved TODO comments scattered throughout the document (approximately 8 instances) that should be addressed before merging. A quick
grep -n "TODO" docs/getting-started/modeling.mdxwill locate them all.
| Permify has its own language that you can model your authorization logic with it. The language allows to define arbitrary relations between users and objects, such as owner, editor, commenter or roles like admin, manager, member and also dynamic attributes such as boolean variables, IP range, time period, etc. | ||
| Permify has its own language that you can model your authorization logic with. The language allows you to define arbitrary relations between users and objects. | ||
|
|
||
| You can assign users associate them with arbitrary objects representing application concepts, such as teams, organizations, or stores. |
There was a problem hiding this comment.
Grammatical error: sentence is malformed.
The sentence appears to have conflated two phrasings. It currently reads "You can assign users associate them with arbitrary objects" which is grammatically incorrect.
🔎 Suggested fix
-You can assign users associate them with arbitrary objects representing application concepts, such as teams, organizations, or stores.
+You can associate users with arbitrary objects representing application concepts, such as teams, organizations, or stores.Or alternatively:
-You can assign users associate them with arbitrary objects representing application concepts, such as teams, organizations, or stores.
+You can assign users to arbitrary objects representing application concepts, such as teams, organizations, or stores.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| You can assign users associate them with arbitrary objects representing application concepts, such as teams, organizations, or stores. | |
| You can associate users with arbitrary objects representing application concepts, such as teams, organizations, or stores. |
🤖 Prompt for AI Agents
In docs/getting-started/modeling.mdx around line 14, the sentence "You can
assign users associate them with arbitrary objects representing application
concepts, such as teams, organizations, or stores." is grammatically malformed;
replace it with a correct phrasing such as "You can associate users with
arbitrary objects representing application concepts, such as teams,
organizations, or stores." (or "You can assign users to arbitrary objects
representing application concepts, such as teams, organizations, or stores.") so
the sentence is clear and syntactically correct.
| We'll follow a simplified version of the GitHub access control system, where teams and organizations have control over the viewing, editing, or deleting access rights for their repositories. | ||
|
|
||
| Before start I want to share the full implementation of simple Github access control example with using Permify Schema. | ||
| Before we start, here's the full implementation of the simplified Github access control example using Permify Schema. |
There was a problem hiding this comment.
Capitalize "GitHub" correctly.
The official spelling uses a capital "H".
🔎 Suggested fix
-Before we start, here's the full implementation of the simplified Github access control example using Permify Schema.
+Before we start, here's the full implementation of the simplified GitHub access control example using Permify Schema.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Before we start, here's the full implementation of the simplified Github access control example using Permify Schema. | |
| Before we start, here's the full implementation of the simplified GitHub access control example using Permify Schema. |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~36-~36: The official name of this software platform is spelled with a capital “H”.
Context: ...e full implementation of the simplified Github access control example using Permify Sc...
(GITHUB)
🤖 Prompt for AI Agents
In docs/getting-started/modeling.mdx around line 36, change the misspelled
"Github" to the correct "GitHub" in the sentence "Before we start, here's the
full implementation of the simplified Github access control example using
Permify Schema." — update the capitalization to "GitHub" (and scan nearby lines
for any other "Github" occurrences to correct them to "GitHub").
mooreds
left a comment
mooreds
left a comment
There was a problem hiding this comment.
Added notes where I had questions about content.
docs/getting-started/modeling.mdx
Outdated
| - **type:** relation type, basically the entity it’s related to (e.g. user, organization, document, etc.) | ||
| TODO, can it include _ or any other character | ||
| - **name:** the name of the relation, which can be any alphanumeric string | ||
| - **type:** the type of the related object, which must be defined in this schema previously |
There was a problem hiding this comment.
is this true? do you need to define entities before they can be referenced?
There was a problem hiding this comment.
it needs to be defined but not necessarily previously. also we tend to use the term entity or relation rather than object. in this case the object is a relation. my suggestion would be as following:
| - **type:** the type of the related object, which must be defined in this schema previously | |
| - **type:** the target entity type this relation references (e.g. user, organization, document), which must exist in the schema |
docs/getting-started/modeling.mdx
Outdated
| ``` | ||
|
|
||
| → If we examine the `read` action rules; user that is `organization admin` and following users can read the repository: `owner` of the repository, or `maintainer`, or `member` of the organization which repository belongs to. | ||
| → Looking at the `read` action, you can see the `read` permission is limited to a user that is an `organization admin` and also has one of the following relations: `owner` of the repository, or `maintainer`, or `member` of the organization which repository belongs to. |
There was a problem hiding this comment.
Is this true? Seems weird to limit read permissions to admins? Would it make more sense to change the action to delete?
There was a problem hiding this comment.
Yes, this statement is true. Also, any action name is acceptable. We can rename it to delete if that better reflects the intended semantics and aligns with common sense
docs/getting-started/modeling.mdx
Outdated
| </Note> | ||
|
|
||
| It's important to understand the nature of the `and` operation: it creates an intersection between relations but is not necessarily tied to specific entities. | ||
| The `and` operation creates an intersection between relations but is not tied to specific entities. |
There was a problem hiding this comment.
This is good to call out.
Is there a way to tie a permission to a specific entity? So that you could require the user in the example below to be both a member and an admin of organization 1?
There was a problem hiding this comment.
Yes, there is a workaround for this. If you define a permission in the organization entity requiring both then it will be tied to the same organization. need to test this though:
entity organization {
relation member @user
relation admin @user
permission view = member and admin
}There was a problem hiding this comment.
Hmmm. What you showed seems to be exactly what is in the doc and is called out as being surprising.
What am I missing?
There was a problem hiding this comment.
entity organization {
relation member @user
relation admin @user
permission view = member and admin
}
entity repository {
relation owner @user
relation org @organization
permission read = org.view or owner
permission read2 = (org.member and org.admin) or owner
}
The first read permission will be looking for member and admin for a particular organization. The second one is looking for member and admin for any organization (e.g being admin of org1 and member of org2 will work).
There was a problem hiding this comment.
Hmmm. I don't see this behavior.
If you import this YAML file into the playground, you'll see that u2 is denied read access with both types of permissions, even if they are an admin of a different org and a user of a current org.
There was a problem hiding this comment.
please check this out. https://play.permify.co/?s=1767724827612-6pdreg6
There was a problem hiding this comment.
thanks @ucatbas ! I think I understand now. I updated the documentation based on this.
docs/getting-started/modeling.mdx
Outdated
| ### Using Attributes Across Entities | ||
|
|
||
| <Info> | ||
| This feature is currently in beta and available upon request. Let us know, and we’ll give you early access! |
There was a problem hiding this comment.
Is this still in beta?
There was a problem hiding this comment.
After internal discussion, going to remove this.
docs/getting-started/modeling.mdx
Outdated
| You can use attributes across entities with rules. Rules can also be written inside entities to create entity-specific conditionals. | ||
|
|
||
| In the example below, we defined a check_confidentiality rule inside the organization entity. This rule is responsible for comparing the specific organization's authority level with the given authority level. | ||
| Below, the `check_confidentiality` rule is defined inside the `organization` entity. This rule compares the specific organization's authority level with the authority level associated with the repository. |
There was a problem hiding this comment.
This seemed to be what the rules were saying but please confirm I got this right.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
docs/getting-started/modeling.mdx (2)
14-14: Fix grammatical error in sentence.This sentence is grammatically incorrect: "You can assign users associate them with arbitrary objects" conflates two different phrasings.
🔎 Suggested fix
-You can assign users associate them with arbitrary objects representing application concepts, such as teams, organizations, or stores. +You can associate users with arbitrary objects representing application concepts, such as teams, organizations, or stores.Or alternatively:
-You can assign users associate them with arbitrary objects representing application concepts, such as teams, organizations, or stores. +You can assign users to arbitrary objects representing application concepts, such as teams, organizations, or stores.
36-36: Capitalize "GitHub" correctly.The official spelling uses a capital "H".
🔎 Suggested fix
-Before we start, here's the full implementation of the simplified Github access control example using Permify Schema. +Before we start, here's the full implementation of the simplified GitHub access control example using Permify Schema.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/getting-started/modeling.mdx
🧰 Additional context used
🪛 LanguageTool
docs/getting-started/modeling.mdx
[style] ~17-~17: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...les such as admin, manager, or member. You can also use dynamic attributes in your...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~21-~21: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...permify/master/assets/permify-dsl.gif) You can define your entities, relations bet...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~36-~36: The official name of this software platform is spelled with a capital “H”.
Context: ...e full implementation of the simplified Github access control example using Permify Sc...
(GITHUB)
[grammar] ~303-~303: Use a hyphen to join words.
Context: ...ttributes can be found in the [Attribute Based Permissions (ABAC)](#attribute-bas...
(QB_NEW_EN_HYPHEN)
[grammar] ~324-~324: Use a hyphen to join words.
Context: ...mmon permission use cases. ## Attribute Based Permissions (ABAC) To support Att...
(QB_NEW_EN_HYPHEN)
[grammar] ~326-~326: Use a hyphen to join words.
Context: ...Permissions (ABAC) To support Attribute Based Access Control (ABAC), there are t...
(QB_NEW_EN_HYPHEN)
[uncategorized] ~460-~460: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...comprehensive schema examples from the [Real World Examples](/getting-started/examples/int...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~466-~466: Replacing this phrase with a shorter alternative might make your text sound more refined.
Context: ...ess rights** in the child entities that belong to it. - [Instagram]: how **public/private at...
(BELONG_TO_PRP)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test with Coverage
- GitHub Check: Test with Coverage
- GitHub Check: Analyze (go)
- GitHub Check: Scan Container for Vulnerabilities
🔇 Additional comments (9)
docs/getting-started/modeling.mdx (9)
6-8: Clear introduction to relationship-based permissions.The rewrite effectively introduces the core concept of permission assignment through relationships, and the ReBAC terminology is properly introduced for consistent usage throughout the document.
59-79: Well-structured entity definition guidance.The entity definition section clearly explains the concept, provides good naming advice (matching database tables), and properly categorizes entity properties (relations, actions/permissions, attributes).
236-241: Verify the restrictive permission logic is intentional.The
readpermission requires a user to beorganization.adminAND (owner OR maintainer OR organization.member). This means even repository owners and maintainers cannot read unless they are also organization admins, which seems overly restrictive.Was this intended, or should the logic be:
action read = organization.admin or owner or maintainer or organization.memberThis would allow any of these roles to read the repository independently.
This concern was previously raised by mooreds, who suggested changing the action to
deleteif admin-only access was intended.
253-275: Verify the example logic for AND operator across entities.The explanation states the
andoperator "is not tied to specific entities," but the example appears inconsistent with the code:
- Line 257-258: States that
user:1(member oforganization:1and admin oforganization:2) has access to viewrepository:1(which belongs toorganization:1).- Line 274: The permission is defined as
permission view = parent.member and parent.admin.However, if
repository:1hasparent→organization:1, then foruser:1to satisfyparent.member and parent.admin, they would need to be both member and admin oforganization:1specifically. Being an admin oforganization:2wouldn't satisfy the condition.Is the example correct, or should it demonstrate a case where the user is both member and admin of the same organization?
This relates to mooreds' question about whether permissions can be tied to specific entities. The
parentrelation does appear to tie the permission check to the specific parent organization.
278-306: Clear explanation of the NOT operator.The exclusion example using the
restrictedattribute on posts effectively demonstrates how thenotoperator works in practice. The cross-reference to the ABAC section is helpful for readers wanting more detail.
307-321: Well-explained permission union concept.The permission union section clearly demonstrates how actions can inherit from other actions and how to create combined permission sets. The example effectively shows the
deleteaction inheriting fromedit.
387-428: Accurate explanation of cross-entity attribute usage.The explanation correctly describes how the
check_confidentialityrule compares the organization's authority level to the repository's authority level. Line 410-411 showsthis.authority >= authority, which returns true when the organization's authority is greater than or equal to the repository's authority—exactly as described.This addresses mooreds' question about accuracy confirmation.
435-474: Comprehensive modeling guides and examples section.The updated descriptions for each example clearly communicate what authorization patterns they demonstrate (organizational roles, inherited permissions, public/private attributes, etc.). The cross-references and links appear properly structured.
390-390: Statement is accurate. The cross-entity attributes feature is confirmed to be in beta and available upon request as of December 2025. No changes needed.
docs/getting-started/modeling.mdx
Outdated
| - **name:** the name of the relation, which can be any alphanumeric string | ||
| - **type:** the type of the related object, which must be defined in this schema previously |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Permify relation naming rules valid characters schema
💡 Result:
Do you want the official Permify rules for relation/entity/permission identifier characters (so I can look them up), or do you want a recommended safe naming pattern (practical rules that always work)?
🏁 Script executed:
# Check if this is a Permify repository and look for relation validation code
git ls-files | head -20Repository: Permify/permify
Length of output: 626
🏁 Script executed:
# Search for relation validation or naming constraints in the codebase
rg -i "relation.*name" --type go --type rs --type ts --type js -A 3 | head -50Repository: Permify/permify
Length of output: 85
🌐 Web query:
Permify official identifier naming rules relation type alphanumeric underscore
💡 Result:
Permify requires identifiers (entity types, permission names, relation names) to start with a letter and may contain only letters, digits and underscore; max length 64 bytes. Example regex: ^[A-Za-z][A-Za-z0-9_]{0,63}$. (tenant_id uses a different pattern: [A-Za-z0-9-,]+, max 64). [1][2][3]
Sources:
- Modeling / relations (Permify docs). [1]
- LookupEntity (entity_type / permission rules). [2]
- Expand API (permission rules / tenant_id pattern). [3]
🏁 Script executed:
# Look for parser or schema validation files that define relation rules
fd -e go | xargs rg -l "relation.*valid|identifier.*pattern" | head -10Repository: Permify/permify
Length of output: 147
🏁 Script executed:
# Search for schema examples or documentation showing valid relation names
fd -e mdx -o -e md | xargs rg "relation.*name" -B 2 -A 2 | head -100Repository: Permify/permify
Length of output: 7458
🏁 Script executed:
# Look for validation rules in Go source files
rg "alphanumeric|identifier|validation" --type go -A 3 | head -80Repository: Permify/permify
Length of output: 5794
🏁 Script executed:
# Examine the validation.go file to understand type constraints
cat -n pkg/dsl/ast/validation.go | head -150Repository: Permify/permify
Length of output: 2627
🏁 Script executed:
# Search for type validation and forward references
rg "type.*reference|forward|previously|defined" pkg/dsl/ast/validation.go -B 3 -A 3Repository: Permify/permify
Length of output: 981
🏁 Script executed:
# Look for examples of schema validation that show type handling
rg "relation.*type|entity.*type" pkg/dsl/parser/parser_test.go -B 2 -A 2 | head -80Repository: Permify/permify
Length of output: 834
Lines 89-90 contain inaccurate documentation that contradicts Permify's official specification.
-
Line 89 is incorrect: Relation names cannot be "any alphanumeric string." According to Permify's official rules, identifiers must start with a letter and can contain only letters, digits, and underscores (pattern:
^[A-Za-z][A-Za-z0-9_]{0,63}$, max 64 bytes). Revise to:"the name of the relation, which must start with a letter and contain only letters, digits, and underscores (max 64 bytes)" -
Line 90 is misleading: The wording "must be defined in this schema previously" suggests a temporal/order constraint. The actual requirement is simply that the type must be defined somewhere in the schema. Revise to:
"the type of the related object, which must be defined as an entity in this schema"
🤖 Prompt for AI Agents
In docs/getting-started/modeling.mdx around lines 89 to 90, update the two
relation field descriptions to match Permify's spec: change the name description
to say it "must start with a letter and contain only letters, digits, and
underscores (max 64 bytes)" and change the type description to clarify it "must
be defined as an entity in this schema" (removing any implication of temporal
ordering). Ensure the exact phrasing replaces the current incorrect lines.
docs/getting-started/modeling.mdx
Outdated
| The same `read` can also be defined using the **permission** keyword, as follows: | ||
|
|
||
| ```perm | ||
| permission read = org.admin and (owner or maintainer or org.member) |
There was a problem hiding this comment.
Fix inconsistent entity naming.
This line uses org.admin and org.member while the main example (line 236) uses organization.admin and organization.member. The entity names should be consistent throughout.
🔎 Suggested fix
- permission read = org.admin and (owner or maintainer or org.member)
+ permission read = organization.admin and (owner or maintainer or organization.member)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| permission read = org.admin and (owner or maintainer or org.member) | |
| permission read = organization.admin and (owner or maintainer or organization.member) |
🤖 Prompt for AI Agents
In docs/getting-started/modeling.mdx around line 247, the policy uses
inconsistent entity names `org.admin` and `org.member` while the rest of the
example uses `organization.admin` and `organization.member`; update the line to
use `organization.admin` and `organization.member` so entity naming matches the
main example (i.e., replace `org.` prefixes with `organization.` on that
permission line).
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2708 +/- ##
==========================================
- Coverage 82.56% 82.55% -0.01%
==========================================
Files 74 74
Lines 8125 8125
==========================================
- Hits 6708 6707 -1
- Misses 902 903 +1
Partials 515 515 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mooreds
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
docs/getting-started/modeling.mdx (3)
14-14: Fix grammatical error in sentence structure.The sentence "You can assign users associate them with arbitrary objects" is malformed. It should be either "You can associate users with arbitrary objects" or "You can assign users to arbitrary objects."
🔎 Suggested fix
-You can assign users associate them with arbitrary objects representing application concepts, such as teams, organizations, or stores. +You can associate users with arbitrary objects representing application concepts, such as teams, organizations, or stores.
36-36: Capitalize "GitHub" correctly.The official spelling uses a capital "H": "GitHub" not "Github".
🔎 Suggested fix
-Before we start, here's the full implementation of the simplified Github access control example using Permify Schema. +Before we start, here's the full implementation of the simplified GitHub access control example using Permify Schema.
247-247: Fix inconsistent entity naming.This line uses
org.adminandorg.memberwhile the main example usesorganization.adminandorganization.member. Entity names should be consistent throughout the documentation.🔎 Suggested fix
- permission read = org.admin and (owner or maintainer or org.member) + permission read = organization.admin and (owner or maintainer or organization.member)
🧹 Nitpick comments (1)
docs/getting-started/modeling.mdx (1)
213-213: Remove extra space before question mark.There's an extra space before the question mark in the authorization check format.
🔎 Suggested fix
-The basic form of an authorization check in Permify is **_Can the user U perform action X on a resource Y ?_**. +The basic form of an authorization check in Permify is **_Can the user U perform action X on a resource Y?_**.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/getting-started/modeling.mdx
🧰 Additional context used
🪛 LanguageTool
docs/getting-started/modeling.mdx
[style] ~17-~17: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...les such as admin, manager, or member. You can also use dynamic attributes in your...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~21-~21: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...permify/master/assets/permify-dsl.gif) You can define your entities, relations bet...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~36-~36: The official name of this software platform is spelled with a capital “H”.
Context: ...e full implementation of the simplified Github access control example using Permify Sc...
(GITHUB)
[grammar] ~303-~303: Use a hyphen to join words.
Context: ...ttributes can be found in the [Attribute Based Permissions (ABAC)](#attribute-bas...
(QB_NEW_EN_HYPHEN)
[grammar] ~324-~324: Use a hyphen to join words.
Context: ...mmon permission use cases. ## Attribute Based Permissions (ABAC) To support Att...
(QB_NEW_EN_HYPHEN)
[grammar] ~326-~326: Use a hyphen to join words.
Context: ...Permissions (ABAC) To support Attribute Based Access Control (ABAC), there are t...
(QB_NEW_EN_HYPHEN)
[uncategorized] ~418-~418: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...comprehensive schema examples from the [Real World Examples](/getting-started/examples/int...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~424-~424: Replacing this phrase with a shorter alternative might make your text sound more refined.
Context: ...ess rights** in the child entities that belong to it. - [Instagram]: how **public/private at...
(BELONG_TO_PRP)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Analyze (go)
- GitHub Check: Test with Coverage
- GitHub Check: Scan Container for Vulnerabilities
- GitHub Check: Test with Coverage
🔇 Additional comments (7)
docs/getting-started/modeling.mdx (7)
6-8: Clear introduction to ReBAC concepts.The revised introduction effectively explains relationship-based access control with concrete examples (manager-subordinate, group membership) and properly introduces the ReBAC terminology.
59-79: Well-structured entity definition section.The revised content clearly explains entities with good analogies (database tables) and properly lists the three types of entity properties. The recommendation to name entities matching database table names is practical advice.
89-90: Relation attribute descriptions are now accurate.The naming rules correctly reflect the Permify specification: names may contain only letters and underscores with a maximum of 64 characters. The type description properly clarifies that referenced entities must exist in the schema without implying a temporal ordering constraint.
Based on learnings from past review discussions.
125-206: Comprehensive relation examples with clear explanations.The revised content effectively explains:
- Parent-child relationships with practical examples
- Ownership concepts
- Multiple relation types syntax (
@user @team#member)- Feature locking with detailed tuple examples
The note about feature locking is particularly helpful, clearly showing how
@organization#memberallows specifying sets of users vs. individual users only.
218-322: Clear explanations of permission operators and unions.The revised content effectively explains:
- Intersection using
andoperator with practical repository example- Exclusion using
notoperator with the Instagram post example- Permission unions and inheritance (e.g.,
deleteinheriting fromedit)The examples clearly demonstrate how operators create intersections and exclusions, and the cross-reference to nested hierarchies is helpful.
326-391: Comprehensive ABAC section with clear examples.The attribute-based access control section effectively covers:
- Attribute definitions with all supported types (boolean, string, integer, double, and their arrays)
- Rules as condition-based functions
- Practical IP range checking example
- Appropriate reference to Common Expression Language (CEL)
The content is well-structured and informative.
395-432: Well-organized modeling guides and examples section.The updated cross-references and example descriptions effectively guide users to:
- Common permission patterns (RBAC, ReBAC, ABAC)
- Comprehensive real-world examples with clear focus descriptions
The summaries for Google Docs, Facebook Groups, Notion, Instagram, and Mercury examples are concise and informative.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
docs/getting-started/modeling.mdx (3)
36-36: Correct the spelling of "GitHub."This spelling issue was previously flagged but remains unfixed. The official capitalization of the company name uses a capital "H".
🔎 Suggested fix
-Before we start, here's the full implementation of the simplified Github access control example using Permify Schema. +Before we start, here's the full implementation of the simplified GitHub access control example using Permify Schema.
247-247: Fix inconsistent entity naming.This inconsistency was previously flagged but remains unfixed. The line uses
org.adminandorg.member, while the main example (line 236) and the rest of the documentation consistently useorganization.adminandorganization.member.🔎 Suggested fix
- permission delete = org.admin and (owner or maintainer or org.member) + permission delete = organization.admin and (owner or maintainer or organization.member)
14-14: Fix grammatical error: sentence is malformed.This grammar issue was previously flagged but remains unfixed. The sentence "You can assign users associate them with arbitrary objects" conflates two different phrasings.
🔎 Suggested fix
-You can assign users associate them with arbitrary objects representing application concepts, such as teams, organizations, or stores. +You can associate users with arbitrary objects representing application concepts, such as teams, organizations, or stores.Or alternatively:
-You can assign users associate them with arbitrary objects representing application concepts, such as teams, organizations, or stores. +You can assign users to arbitrary objects representing application concepts, such as teams, organizations, or stores.
🧹 Nitpick comments (3)
docs/getting-started/modeling.mdx (3)
14-18: Consider varying sentence structure to improve readability.Three consecutive sentences (lines 14, 16, and 18) begin with "You can," which creates a repetitive pattern. While this is clear and functional, varying the sentence openings could make the text more engaging.
💡 Possible rewording
For example, you could combine or rephrase:
You can associate users with arbitrary objects representing application concepts, such as teams, organizations, or stores, and give them roles such as admin, manager, or member. Your authorization model can also use dynamic attributes, such as boolean variables, IP ranges, or time periods.
324-326: Consider hyphenating compound adjectives for grammatical correctness.In lines 324 and 326, "Attribute Based" functions as a compound adjective modifying "Permissions" and "Access Control" respectively. Standard grammar rules suggest using a hyphen: "Attribute-Based Permissions (ABAC)" and "Attribute-Based Access Control (ABAC)."
The same pattern appears in line 303. While the current phrasing is understandable, adding hyphens would align with formal writing conventions.
🔎 Suggested changes
Line 303:
-More details about defining attributes can be found in the [Attribute Based Permissions (ABAC)](#attribute-based-permissions-abac) section. +More details about defining attributes can be found in the [Attribute-Based Permissions (ABAC)](#attribute-based-permissions-abac) section.Lines 324-326:
-## Attribute Based Permissions (ABAC) +## Attribute-Based Permissions (ABAC) -To support Attribute Based Access Control (ABAC), there are two additional components in our schema language: `attribute` and `rule`. +To support Attribute-Based Access Control (ABAC), there are two additional components in our schema language: `attribute` and `rule`.
418-418: Consider hyphenating "Real-World Examples" for clarity.When "real world" modifies "examples" as a compound adjective, standard grammar suggests using a hyphen: "Real-World Examples." This is a minor stylistic point that improves grammatical correctness.
🔎 Suggested change
-You can also check out comprehensive schema examples from the [Real World Examples](/getting-started/examples/intro) section. +You can also check out comprehensive schema examples from the [Real-World Examples](/getting-started/examples/intro) section.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/getting-started/modeling.mdx
🧰 Additional context used
🪛 LanguageTool
docs/getting-started/modeling.mdx
[style] ~17-~17: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...les such as admin, manager, or member. You can also use dynamic attributes in your...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~21-~21: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...permify/master/assets/permify-dsl.gif) You can define your entities, relations bet...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~36-~36: The official name of this software platform is spelled with a capital “H”.
Context: ...e full implementation of the simplified Github access control example using Permify Sc...
(GITHUB)
[grammar] ~303-~303: Use a hyphen to join words.
Context: ...ttributes can be found in the [Attribute Based Permissions (ABAC)](#attribute-bas...
(QB_NEW_EN_HYPHEN)
[grammar] ~324-~324: Use a hyphen to join words.
Context: ...mmon permission use cases. ## Attribute Based Permissions (ABAC) To support Att...
(QB_NEW_EN_HYPHEN)
[grammar] ~326-~326: Use a hyphen to join words.
Context: ...Permissions (ABAC) To support Attribute Based Access Control (ABAC), there are t...
(QB_NEW_EN_HYPHEN)
[uncategorized] ~418-~418: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...comprehensive schema examples from the [Real World Examples](/getting-started/examples/int...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~424-~424: Replacing this phrase with a shorter alternative might make your text sound more refined.
Context: ...ess rights** in the child entities that belong to it. - [Instagram]: how **public/private at...
(BELONG_TO_PRP)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test with Coverage
- GitHub Check: Scan Container for Vulnerabilities
- GitHub Check: Analyze (go)
- GitHub Check: Test with Coverage
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.