Skip to content

Conversation

@andrest50
Copy link
Contributor

@andrest50 andrest50 commented Nov 5, 2025

Summary

This PR updates the OpenFGA authorization model to include two new attachment types:

  • meeting_attachment: Represents attachments for meetings
  • past_meeting_attachment: Represents attachments for past meetings

Both types include appropriate relations for writer, auditor, participant, and viewer permissions, following the same pattern as their parent meeting types.

Changes

  • Added meeting_attachment type with relations: meeting, writer, auditor, participant, and viewer
  • Added past_meeting_attachment type with relations: past_meeting, writer, auditor, participant, and viewer
  • Bumped model version from 5.3.2 to 6.0.0

Ticket

LFXV2-707

🤖 Generated with Claude Code

…eting attachment types

This commit adds two new types to the OpenFGA authorization model:
- meeting_attachment: Represents attachments for meetings
- past_meeting_attachment: Represents attachments for past meetings

Both types include appropriate relations for writer, auditor, participant,
and viewer permissions, following the same pattern as their parent meeting types.

The model version has been bumped from 5.3.2 to 6.0.0 to reflect this change.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: Andres Tobon <[email protected]>
@andrest50 andrest50 requested a review from emsearcy as a code owner November 5, 2025 21:01
Copilot AI review requested due to automatic review settings November 5, 2025 21:01
@andrest50 andrest50 requested a review from a team as a code owner November 5, 2025 21:01
@coderabbitai
Copy link

coderabbitai bot commented Nov 5, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Bumps Helm chart version to 0.3.6 and updates the OpenFGA model to 6.0.0; adds meeting_attachment and past_meeting_attachment types and adjusts past_meeting-related artifact relation definitions and viewer semantics.

Changes

Cohort / File(s) Summary
Chart version bump
charts/lfx-platform/Chart.yaml
Updated chart version from 0.3.50.3.6.
OpenFGA model — version & attachment types
charts/lfx-platform/templates/openfga/model.yaml
Bumped model version from 5.3.26.0.0. Added type meeting_attachment and type past_meeting_attachment with relations: meeting/past_meeting, writer (organizer), auditor (writer or auditor from meeting/past_meeting), participant (meeting/past_meeting role compositions), and viewer ([user:*] or participant or writer or auditor). Reordered and adjusted past_meeting, past_meeting_recording, past_meeting_transcript, and past_meeting_summary blocks to align attachment semantics and viewer comments.

Sequence Diagram(s)

(No sequence diagrams — model/schema-only changes.)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Focus review on charts/lfx-platform/templates/openfga/model.yaml for relation correctness, composed participant sets, and viewer union expressions.
  • Verify model version bump consistency with OpenFGA compatibility and downstream consumers.
  • Quick check of charts/lfx-platform/Chart.yaml for chart/versioning rules.

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main change: adding meeting and past meeting attachment types to the OpenFGA authorization model.
Description check ✅ Passed The description clearly explains the changes made, including the two new attachment types, their relations, and the version bump.
Linked Issues check ✅ Passed The PR fully implements the requirements from LFXV2-707 by adding new OpenFGA authorization model types for meeting and past-meeting attachments with appropriate relations.
Out of Scope Changes check ✅ Passed All changes are in scope: two new attachment types added with appropriate relations, and chart version updated. No unrelated changes detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch andrest50/meeting-attachments

📜 Recent review details

Configuration used: CodeRabbit 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.

📥 Commits

Reviewing files that changed from the base of the PR and between ad1790f and abf75a2.

📒 Files selected for processing (1)
  • charts/lfx-platform/templates/openfga/model.yaml (5 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: andrest50
Repo: linuxfoundation/lfx-v2-helm PR: 30
File: charts/lfx-platform/templates/openfga/model.yaml:66-69
Timestamp: 2025-08-08T21:57:43.678Z
Learning: In the lfx-platform OpenFGA authorization model for the meeting type, the viewer relation is intentionally defined as `[user:*] or participant or organizer or auditor from project`, which allows any user to view meetings by default. This is the correct and intended behavior for this system.
📚 Learning: 2025-08-08T21:57:43.678Z
Learnt from: andrest50
Repo: linuxfoundation/lfx-v2-helm PR: 30
File: charts/lfx-platform/templates/openfga/model.yaml:66-69
Timestamp: 2025-08-08T21:57:43.678Z
Learning: In the lfx-platform OpenFGA authorization model for the meeting type, the viewer relation is intentionally defined as `[user:*] or participant or organizer or auditor from project`, which allows any user to view meetings by default. This is the correct and intended behavior for this system.

Applied to files:

  • 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). (1)
  • GitHub Check: MegaLinter
🔇 Additional comments (4)
charts/lfx-platform/templates/openfga/model.yaml (4)

21-24: Appropriate major version bump for schema additions.

The version bump to 6.0.0 correctly reflects the addition of two new types to the OpenFGA schema, aligning with the documented versioning guidelines (line 17: "major: Modifications, deletions, or additions of type").


101-108: Meeting attachment permissions are well-structured and consistent.

The type properly inherits permissions from its parent meeting, with auditor relations including the writer as a passthrough (addressing the prior feedback about writer audit permissions). The public viewer access via [user:*] is consistent with the design intent.


134-141: Past meeting attachment permissions correctly reflect parent's role structure.

The type properly distinguishes host, invitee, and attendee as participant components, which mirrors the role definitions in the parent past_meeting type (lines 124–128). This differs intentionally from meeting_attachment but is semantically correct and maintains consistency with its parent type. Auditor relations include the writer as a passthrough, and public viewer access aligns with the design.


143-210: Artifact type comments clearly document artifact_visibility interaction.

The updated comments for past_meeting_recording, past_meeting_transcript, and past_meeting_summary now explicitly explain how the viewer relation interacts with the artifact_visibility setting (lines 155–164, 178–187, 201–210). This improves clarity for future maintainers and helps explain that while the model permits public access via [user:*], the actual visibility is controlled separately by the artifact_visibility backend setting.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

Copilot AI left a 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 pull request updates the OpenFGA authorization model to version 6.0.0 and adds support for meeting and past meeting attachments. The changes introduce new resource types with appropriate access control relations that inherit permissions from their parent meeting objects.

  • Bumped authorization model version from 5.3.2 to 6.0.0
  • Added meeting_attachment type with relations for meeting attachments
  • Added past_meeting_attachment type with relations for past meeting attachments

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: Andres Tobon <[email protected]>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit 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.

📥 Commits

Reviewing files that changed from the base of the PR and between a056f68 and 9d42678.

📒 Files selected for processing (2)
  • charts/lfx-platform/Chart.yaml (1 hunks)
  • charts/lfx-platform/templates/openfga/model.yaml (3 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: andrest50
Repo: linuxfoundation/lfx-v2-helm PR: 30
File: charts/lfx-platform/templates/openfga/model.yaml:66-69
Timestamp: 2025-08-08T21:57:43.678Z
Learning: In the lfx-platform OpenFGA authorization model for the meeting type, the viewer relation is intentionally defined as `[user:*] or participant or organizer or auditor from project`, which allows any user to view meetings by default. This is the correct and intended behavior for this system.
Learnt from: prabodhcs
Repo: linuxfoundation/lfx-v2-helm PR: 52
File: charts/lfx-platform/templates/openfga/model.yaml:73-80
Timestamp: 2025-09-09T15:09:56.135Z
Learning: In the OpenFGA authorization model for the lfx-platform Helm chart, the type for GroupsIO members is named "groupsio_member" rather than "groupsio_mailing_list_member", as preferred by the maintainers for brevity.
📚 Learning: 2025-08-08T21:57:43.678Z
Learnt from: andrest50
Repo: linuxfoundation/lfx-v2-helm PR: 30
File: charts/lfx-platform/templates/openfga/model.yaml:66-69
Timestamp: 2025-08-08T21:57:43.678Z
Learning: In the lfx-platform OpenFGA authorization model for the meeting type, the viewer relation is intentionally defined as `[user:*] or participant or organizer or auditor from project`, which allows any user to view meetings by default. This is the correct and intended behavior for this system.

Applied to files:

  • charts/lfx-platform/templates/openfga/model.yaml
📚 Learning: 2025-08-29T16:53:12.710Z
Learnt from: mauriciozanettisalomao
Repo: linuxfoundation/lfx-v2-helm PR: 49
File: charts/lfx-platform/Chart.yaml:8-8
Timestamp: 2025-08-29T16:53:12.710Z
Learning: The maintainer mauriciozanettisalomao prefers to keep chart version increments conservative, choosing 0.2.7 over suggested 0.3.0 even for changes that could be considered behavior-altering (like new middleware and JSON Content-Type enforcement in Heimdall).

Applied to files:

  • charts/lfx-platform/Chart.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). (1)
  • GitHub Check: MegaLinter
🔇 Additional comments (3)
charts/lfx-platform/Chart.yaml (1)

8-8: Conservative versioning approach aligns with maintainer preference.

The patch-level Helm chart bump (0.3.5 → 0.3.6) is proportional to the additive nature of the authorization model changes and follows the conservative versioning pattern noted in previous reviews.

charts/lfx-platform/templates/openfga/model.yaml (2)

22-24: Version bump follows documented guidelines for type additions.

The major version increment (5.3.2 → 6.0.0) correctly applies the versioning guideline at lines 15-20 for "additions of type."


108-108: Confirm viewer permission is intentional for attachments.

Both attachment types define viewer: [user:*] or participant or writer or auditor, which allows any authenticated user to view any attachment by default. This matches the retrieved learning that confirms the same pattern for meeting type is intentional. Please verify this permissive default is also the intended behavior for meeting and past-meeting attachments.

Also applies to: 141-141

relations
define meeting: [meeting]
define writer: organizer from meeting
define auditor: auditor from meeting
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

writer should also have audit permissions (if they differ from viewer). If auditor is only used as a kind of viewer, then perhaps leave a note since this differs from the other style of models where auditor has access to read things that viewers don't, typically.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright I can add the writer as a passthrough to the auditor relation. Technically auditor via meeting means organizer via meeting has auditor permission, but to be more explicit I'll just include it on this line.

… relation for meeting and past meeting attachments

Signed-off-by: Andres Tobon <[email protected]>
@andrest50 andrest50 requested a review from jordane November 7, 2025 16:31
@andrest50 andrest50 merged commit 2643106 into main Nov 7, 2025
4 checks passed
@andrest50 andrest50 deleted the andrest50/meeting-attachments branch November 7, 2025 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants