fix: License validations for federated rooms #37523
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
🦋 Changeset detectedLatest commit: 8218c9f The changes in this PR will be included in the next version bump. This PR includes changesets to release 41 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughConsolidates federation rendering in the message composer: ComposerFederation now accepts a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-10-28T16:53:42.761ZApplied to files:
🧬 Code graph analysis (1)apps/meteor/client/views/room/composer/ComposerFederation/ComposerFederation.tsx (2)
⏰ 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)
🔇 Additional comments (3)
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 |
aleksandernsilva
changed the title
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #37523 +/- ##
===========================================
- Coverage 68.81% 68.80% -0.01%
===========================================
Files 3361 3361
Lines 114260 114260
Branches 20619 20619
===========================================
- Hits 78623 78616 -7
- Misses 33541 33549 +8
+ Partials 2096 2095 -1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![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
📜 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.
📒 Files selected for processing (4)
.changeset/lemon-garlics-check.md(1 hunks)apps/meteor/client/views/room/composer/ComposerContainer.tsx(1 hunks)apps/meteor/client/views/room/composer/ComposerFederation/ComposerFederation.spec.tsx(1 hunks)apps/meteor/client/views/room/composer/ComposerFederation/ComposerFederation.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
.changeset/lemon-garlics-check.mdapps/meteor/client/views/room/composer/ComposerContainer.tsx
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
.changeset/lemon-garlics-check.mdapps/meteor/client/views/room/composer/ComposerFederation/ComposerFederation.tsxapps/meteor/client/views/room/composer/ComposerContainer.tsx
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
apps/meteor/client/views/room/composer/ComposerFederation/ComposerFederation.tsxapps/meteor/client/views/room/composer/ComposerContainer.tsx
🧬 Code graph analysis (2)
apps/meteor/client/views/room/composer/ComposerFederation/ComposerFederation.spec.tsx (3)
packages/core-typings/src/license/LicenseModule.ts (1)
LicenseModule(30-30)packages/mock-providers/src/index.ts (1)
mockAppRoot(3-3)apps/meteor/tests/mocks/data.ts (1)
createFakeLicenseInfo(213-260)
apps/meteor/client/views/room/composer/ComposerFederation/ComposerFederation.tsx (2)
apps/meteor/client/views/room/composer/ComposerMessage.tsx (1)
ComposerMessageProps(12-25)apps/meteor/client/hooks/useIsFederationEnabled.ts (1)
useIsFederationEnabled(3-8)
⏰ 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: 🔨 Test UI (EE) / MongoDB 5.0 (5/5)
🔇 Additional comments (8)
.changeset/lemon-garlics-check.md (1)
1-5: LGTM! Changeset follows standard format.The changeset correctly declares a patch release for the meteor package with an appropriate description.
apps/meteor/client/views/room/composer/ComposerFederation/ComposerFederation.spec.tsx (2)
1-13: LGTM! Mock setup is appropriate.The mock for
ComposerMessageis correctly structured, and the imports align with the component dependencies.
31-62: Excellent test coverage!The test suite comprehensively validates all rendering branches:
- Blocked state for old federation rooms
- Federation globally disabled
- Missing license module (premium feature)
- Normal operation with valid license
Each test appropriately configures the mock environment and verifies the expected UI state.
apps/meteor/client/views/room/composer/ComposerContainer.tsx (2)
41-41: LGTM! Blocked state calculation is clear.The
isFederationBlockedvariable correctly identifies non-native federated rooms (old federation system rooms) that should be blocked.
58-60: Good refactor: Simplified container logic.Moving the blocked state handling into
ComposerFederationas a prop reduces conditional complexity in the container. The component now has a single, unified rendering path for federation with explicit state passing.apps/meteor/client/views/room/composer/ComposerFederation/ComposerFederation.tsx (3)
11-13: LGTM! Type definition properly extends base props.The
ComposerFederationPropstype correctly extendsComposerMessagePropsand adds the optionalblockedprop for controlling invalid version state.
19-21: LGTM! Blocked state handling is clear.The early return for blocked rooms correctly renders the invalid version component for old (non-Matrix) federated rooms.
27-29: Loading state returns'loading'string, notundefined.The
useHasLicenseModulehook atapps/meteor/client/hooks/useHasLicenseModule.tsreturns'loading' | boolean, specifically the string'loading'when license data is not yet available (via.data ?? 'loading'). The strict equality check=== trueat line 17 correctly makesfederationModuleEnabledfalse during this loading phase, causing the disabled message to display. This is safe behavior—users already with the federation license will briefly see the disabled state until the license data loads. If improved UX during this fetch is desired, a loading skeleton could replace the disabled message.
apps/meteor/client/views/room/composer/ComposerFederation/ComposerFederation.spec.tsx
Show resolved
Hide resolved
scuciatto
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
📦 Docker Image Size Report📈 Changes
📊 Historical Trend---
config:
theme: "dark"
xyChart:
width: 900
height: 400
---
xychart
title "Image Size Evolution by Service (Last 30 Days + This PR)"
x-axis ["11/15 22:28", "11/16 01:28", "11/17 23:50", "11/18 22:53", "11/19 23:02", "11/21 16:49", "11/24 17:34", "11/27 22:32", "11/28 19:05", "12/01 20:42", "12/01 20:54 (PR)"]
y-axis "Size (GB)" 0 --> 0.5
line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "authorization-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "ddp-streamer-service" [0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12]
line "omnichannel-transcript-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "queue-worker-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
line "rocketchat" [0.36, 0.36, 0.35, 0.35, 0.35, 0.34, 0.34, 0.34, 0.34, 0.34, 0.35]
line "stream-hub-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
Statistics (last 10 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Mon, 01 Dec 2025 20:54:42 GMT |
Proposed changes (including videos or screenshots)
This PR updates the federation composer license add-on validation to rely solely on the license module, instead of checking both the license and the subscription. Previously, users with a subscription but without the license module were still able to send messages in federated rooms. With this change, only the presence of the license module grants access.
Issue(s)
FB-28
FB-54
Steps to test or reproduce
Further comments
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.