-
Notifications
You must be signed in to change notification settings - Fork 4
[LFXV2-920] Committee member visibility - Conditional Relation #103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…y relations Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-920 Assisted by [Claude Code](https://claude.ai/code) Signed-off-by: Mauricio Zanetti Salomao <[email protected]>
WalkthroughBumps OpenFGA model minor version (1 → 2) for the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR updates the OpenFGA authorization model for committee member visibility controls by introducing conditional profile access relationships and refining permission hierarchies. The changes support JIRA ticket LFXV2-920 and enable more granular control over who can view committee member profiles.
- Introduced two new relations for conditional basic profile viewing access
- Modified the
auditorrelation to include writers in the auditor permission set - Removed project-level auditor inheritance from the committee
viewerrelation
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
charts/lfx-platform/templates/openfga/model.yaml (1)
50-50: Consider adding a comment to explain this self-referential pattern.The
self_for_member_basic_profile_accessrelation uses a self-referential pattern where the committee references itself to conditionally enable member access. This pattern isn't immediately obvious to future maintainers.📝 Suggested documentation
+ # The self_for_member_basic_profile_access relation enables conditional member visibility. + # When member_visibility is BASIC_PROFILE, write tuple: committee:<id>#self_for_member_basic_profile_access@committee:<id> + # When member_visibility is HIDDEN (default), omit the tuple so members cannot see other members' profiles. define self_for_member_basic_profile_access: [committee]
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
charts/lfx-platform/templates/openfga/model.yaml
⏰ 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). (2)
- GitHub Check: Agent
- GitHub Check: MegaLinter
🔇 Additional comments (3)
charts/lfx-platform/templates/openfga/model.yaml (3)
21-24: LGTM!Version bump from minor 1 to 2 is appropriate per the versioning guidelines for adding new relations (
self_for_member_basic_profile_accessandbasic_profile_viewer).
52-53: Verify consuming services are updated to usebasic_profile_viewerfor profile access checks.The
basic_profile_viewerrelation correctly implements the access rules:
- Auditors: Always have access (regardless of visibility setting)
- Members: Conditional access via
member from self_for_member_basic_profile_access(only when visibility ≠ HIDDEN)Ensure the FGA sync process writes the self-referential tuple when
member_visibilityisBASIC_PROFILE, and that downstream services (indexer, Heimdall, query-svc) are updated to checkbasic_profile_viewerfor member profile access.
54-54: LGTM - Redundant path removed.The removal of
auditor from projectfromvieweris not a breaking change. Project auditors retain viewer access because:
- Committee's
auditorincludesauditor from project(line 52)- Committee's
viewerincludesauditor(this line)This simplification removes the redundant direct path while maintaining equivalent access semantics.
Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-920 Reviewed with [GitHub Copilot](https://github.com/features/copilot) Signed-off-by: Mauricio Zanetti Salomao <[email protected]>
…ibility-conditional-relation
Overview
Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-920
This pull request updates the OpenFGA authorization model in
charts/lfx-platform/templates/openfga/model.yamlto introduce new relationship definitions and adjust existing ones, enabling more granular access control for profile viewing and auditing.Relates to