[LFXV2-920] Committee member visibility - Conditional Relation#54
Conversation
…date related structures 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]>
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]>
WalkthroughAdds basic-profile member visibility: chart version bumped, OpenFGA rules updated to require Changes
Sequence Diagram(s)sequenceDiagram
participant Writer as CommitteeWriter
participant Model as CommitteeAccessMessage
participant OpenFGA as OpenFGA API
Writer->>Model: build access message (include base relations)
alt committee settings.member_visibility == "basic_profile"
Model->>Model: append "self_for_member_basic_profile_access" to Self
end
Writer->>OpenFGA: send access control message (includes Self)
OpenFGA-->>Writer: acknowledge / enforce rules (uses basic_profile_viewer)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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)
📜 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)
⏰ 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). (1)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR implements a new "basic_profile" member visibility setting for committees, enabling members to view each other's basic profile information while maintaining access control for non-members. The implementation updates the authorization layer, access control messaging, and domain models to support this fine-grained visibility control.
Key Changes:
- Added support for basic_profile member visibility setting with corresponding OpenFGA relation
- Updated access control message structure to support self-access relations for committee members
- Modified authorization rules to check for basic_profile_viewer relation on member GET endpoints
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/constants/access_control.go | Added new constant RelationSelfForMemberBasicProfileAccess for self-access relation |
| internal/service/committee_writer.go | Updated buildAccessControlMessage to populate Self field when basic_profile visibility is enabled |
| internal/domain/model/committee_message.go | Added Self field to CommitteeAccessMessage struct for storing self-relations |
| internal/domain/model/committee_base.go | Added constant and IsMemberVisibilityBasicProfile method to check visibility setting |
| charts/lfx-v2-committee-service/templates/ruleset.yaml | Changed authorization relation from "viewer" to "basic_profile_viewer" for member GET endpoint |
| charts/lfx-v2-committee-service/Chart.yaml | Bumped chart version from 0.2.19 to 0.2.20 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…e method 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]>
![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
🤖 Fix all issues with AI agents
In @internal/domain/model/committee_message.go:
- Around line 112-113: The comment on the Self field is misleading; update the
comment for the Self []string field in committee_message.go to explain that Self
stores OpenFGA self-relation tuples enabling conditional member-to-member
visibility and that when populated members can view other members' basic
profiles according to committee visibility settings (this aligns with how the
field is populated in internal/service/committee_writer.go when
IsMemberVisibilityBasicProfile() is true).
🧹 Nitpick comments (1)
internal/domain/model/committee_base.go (1)
22-22: Consider defining constants for all member visibility levels.The codebase defines exactly two member visibility levels (
"hidden"and"basic_profile"), as documented in the API design. Currently onlybasic_profilehas a constant, whilehidden(the default) is hardcoded as string literals throughout the codebase. Define a constant for consistency and to avoid repeating magic strings.📝 Suggested additional constants
const ( categoryGovernmentAdvisoryCouncil = "Government Advisory Council" + memberVisibilityHidden = "hidden" memberVisibilityBasicProfileSetting = "basic_profile" )Consider also adding
IsMemberVisibilityHidden()to match the existingIsMemberVisibilityBasicProfile()method for symmetrical access.
📜 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 (6)
charts/lfx-v2-committee-service/Chart.yamlcharts/lfx-v2-committee-service/templates/ruleset.yamlinternal/domain/model/committee_base.gointernal/domain/model/committee_message.gointernal/service/committee_writer.gopkg/constants/access_control.go
🧰 Additional context used
🧬 Code graph analysis (1)
internal/domain/model/committee_base.go (2)
cmd/committee-api/design/type.go (1)
CommitteeSettings(35-39)internal/domain/model/committee_settings.go (1)
CommitteeSettings(11-22)
⏰ 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). (3)
- GitHub Check: Agent
- GitHub Check: CodeQL analysis (go)
- GitHub Check: MegaLinter
🔇 Additional comments (6)
charts/lfx-v2-committee-service/Chart.yaml (1)
8-8: LGTM!Appropriate chart version bump for the new basic profile visibility feature.
pkg/constants/access_control.go (1)
15-16: LGTM!The new constant is well-named, follows existing conventions, and has a clear explanatory comment.
internal/domain/model/committee_base.go (1)
161-168: LGTM!The method correctly implements a nil-safe check for the basic profile visibility setting. The defensive nil check on
CommitteeSettingsprevents potential panics.charts/lfx-v2-committee-service/templates/ruleset.yaml (2)
231-256: All member-related endpoints follow a consistent authorization pattern: read operations usebasic_profile_viewer, while write operations (POST, PUT, DELETE) usewriter. No list endpoint (GET /committees/:uid/members) exists in this ruleset that would require similar authorization updates.
248-249: Ensure deployment coordination with fga-sync service for thebasic_profile_viewerrelation.The ruleset checks for the
basic_profile_viewerrelation, which is managed by the fga-sync service, not defined in this repository. When a committee'smember_visibilityis set tobasic_profile, this service publishes aCommitteeAccessMessagewithself_for_member_basic_profile_accessto the fga-sync service. Verify that:
- The fga-sync service has been updated to handle the
self_for_member_basic_profile_accessrelation and map it to thebasic_profile_viewerOpenFGA relation- fga-sync is deployed and operational before this ruleset change is deployed
- The relation mapping between the service's constant (
self_for_member_basic_profile_access) and the ruleset check (basic_profile_viewer) is correctly configured in fga-syncinternal/service/committee_writer.go (1)
241-243: All related changes verified and in place.The implementation correctly adds the self-relation for basic profile member visibility. All dependencies verified:
IsMemberVisibilityBasicProfile()method exists and properly validates member visibility settingRelationSelfForMemberBasicProfileAccessconstant defined with correct valueSelffield added toCommitteeAccessMessagestruct with appropriate documentation- Logic follows established patterns and is consistent with Writers/Auditors handling
…ssage structure 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]>
…ofile 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]>
1 participant
Overview
Jira Ticket https://linuxfoundation.atlassian.net/browse/LFXV2-920
This pull request introduces support for a new "basic_profile" member visibility setting for committees, enabling fine-grained access control for member profile information. The changes update access control logic, constants, and configuration to reflect this new setting and its associated permissions.
Access Control & Configuration Updates:
ruleset.yamlfromviewertobasic_profile_viewerto align with the new visibility setting.RelationSelfForMemberBasicProfileAccessinaccess_control.goto represent self-access for committee members' basic profile information.Committee Model Enhancements:
memberVisibilityBasicProfileSettingconstant and theIsMemberVisibilityBasicProfilemethod to theCommitteemodel to check for the "basic_profile" visibility setting. [1] [2]CommitteeAccessMessagestruct to include a newSelffield for storing self-relations related to basic profile access.Selffield in access control messages when the "basic_profile" visibility is enabled.Chart Version Update:
lfx-v2-committee-servicefrom0.2.19to0.2.20to reflect these changes.Test
Setup
Committee Created:
Test committee - private 2026-01-08-0841070b93a757-2ebb-4195-95e2-24710bd2f399cbef1ed5-17dc-4a50-84e2-6cddd70f6878falseTest Users:
project_super_admin_token(full access)andres(UID:f326933b-6a4a-4e35-97ff-b7cca085682f)mauriciozanetti(UID:aa2ffc1e-5f12-4db6-916c-58f51c90e92d)batmanasithaTest Scenario 1: Hidden Member Visibility (Default)
Creating Committee
Response:
{ "member_visibility": "hidden", "name": "Test committee - private 2026-01-08-084107", "uid": "0b93a757-2ebb-4195-95e2-24710bd2f399", "writers": ["superman"], "auditors": ["batman"] }✅ Result: Committee created with
member_visibility: "hidden"as defaultAdding Members
Member 1 (andres):
{ "username": "andres", "first_name": "Andres", "last_name": "Tobon", "email": "[email protected]", "uid": "f326933b-6a4a-4e35-97ff-b7cca085682f", "role": { "name": "Chair", "start_date": "2025-01-01", "end_date": "2025-12-31" } }Member 2 (mauriciozanetti):
{ "username": "mauriciozanetti", "first_name": "Mauricio", "last_name": "Salomao", "email": "[email protected]", "uid": "aa2ffc1e-5f12-4db6-916c-58f51c90e92d", "role": { "name": "Chair" } }Access Control Tests - Hidden Mode
Admin User (Query Members)
✅ Result: Returns both members (andres and mauriciozanetti)
Member User (Query Members)
✅ Result:
{"resources": []}- No access to member listMember User (Get Specific Member)
✅ Result:
HTTP/1.1 403 Forbidden- Access deniedMember User (Query Committee)
✅ Result: Returns committee info (member can see the committee they belong to)
Test Scenario 2: Basic Profile Visibility
Updating Settings to Basic Profile
Response:
{ "business_email_required": false, "member_visibility": "basic_profile", "uid": "0b93a757-2ebb-4195-95e2-24710bd2f399", "updated_at": "2026-01-08T12:06:20Z" }✅ Result: Settings updated successfully
Access Control Tests - Basic Profile Mode
Member User (Query Members)
✅ Result: Returns both members with full profile data
{ "resources": [ { "data": { "username": "andres", "first_name": "Andres", "last_name": "Tobon", "email": "[email protected]", "uid": "f326933b-6a4a-4e35-97ff-b7cca085682f" } }, { "data": { "username": "mauriciozanetti", "first_name": "Mauricio", "last_name": "Salomao", "email": "[email protected]", "uid": "aa2ffc1e-5f12-4db6-916c-58f51c90e92d" } } ] }Member User (Get Specific Member)
✅ Result:
HTTP/1.1 200 OK- Member data returned successfullyAuditor User (Query Members)
✅ Result: Returns both members (auditors have access)
Non-Member User (Query Members)
✅ Result:
{"resources": []}- No accessNon-Member User (Get Specific Member)
✅ Result:
HTTP/1.1 403 Forbidden- Access deniedTest Scenario 3: Switching Back to Hidden
Updating Settings Back to Hidden
Response:
{ "business_email_required": false, "member_visibility": "hidden", "uid": "0b93a757-2ebb-4195-95e2-24710bd2f399", "updated_at": "2026-01-08T12:15:26Z" }✅ Result: Settings updated successfully
Access Control Tests - Back to Hidden Mode
Member User (Query Members)
✅ Result:
{"resources": []}- No access to member list (as expected)Member User (Query Committee)
✅ Result: Returns committee info (member can still see the committee)
Auditor User (Query Members)
✅ Result: Returns both members (auditors always have access)
Summary of Test Results
✅ Committee access
✅ Committee access
Key Findings
member_visibility: "hidden"