fix(federation): allow to join non-private or encrypted rooms based on settings #37227
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 |
|
WalkthroughAdds two federation feature toggles (join encrypted / non-private rooms), exposes them in the federation config, enforces invite policy with NotAllowedError -> 403 in the Matrix invite endpoint, adds i18n keys and a wording fix, and bumps the federation-sdk dependency. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant HS as External Homeserver
participant API as /_matrix/invite endpoint
participant Proc as invite.processInvite
participant Cfg as Federation Config
Note over Cfg: invite policy: allowedEncryptedRooms<br/>allowedNonPrivateRooms
HS->>API: POST /_matrix/client/.../invite (event)
API->>Proc: processInvite(event, Cfg.invite)
alt Allowed by policy
Proc-->>API: processed invite event
API-->>HS: 200 OK + event
API->>Proc: schedule join (async)
else NotAllowedError thrown
Proc--x API: throws NotAllowedError
API-->>HS: 403 M_FORBIDDEN (policy denial)
else Other error
Proc--x API: throws Error
API-->>HS: 500 M_UNKNOWN (error message)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
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 ignored due to path filters (1)
📒 Files selected for processing (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)
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 |
ricardogarim
changed the title
ricardogarim
force-pushed
the
fix/fed-non-private-or-encrypted-rooms-join
branch
3 times, most recently
from
303c5f5 to
d85c29a
Compare
ricardogarim
force-pushed
the
fix/fed-non-private-or-encrypted-rooms-join
branch
3 times, most recently
from
e1d3985 to
3ba1bb4
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 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)
apps/meteor/server/settings/federation-service.ts(1 hunks)ee/packages/federation-matrix/src/api/_matrix/invite.ts(2 hunks)ee/packages/federation-matrix/src/setup.ts(2 hunks)packages/i18n/src/locales/en.i18n.json(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
PR: RocketChat/Rocket.Chat#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:
ee/packages/federation-matrix/src/api/_matrix/invite.tsee/packages/federation-matrix/src/setup.tsapps/meteor/server/settings/federation-service.ts
🪛 GitHub Actions: CI
ee/packages/federation-matrix/src/api/_matrix/invite.ts
[error] 11-11: TS2305: Module "@rocket.chat/federation-sdk" has no exported member 'NotAllowedError'.
🔇 Additional comments (4)
apps/meteor/server/settings/federation-service.ts (1)
88-102: LGTM!The two new federation settings follow the established pattern and are correctly configured as enterprise-enabled, non-public booleans scoped to the federation module.
ee/packages/federation-matrix/src/setup.ts (2)
48-49: LGTM!The settings are correctly fetched following the same pattern as other federation settings in this file.
88-91: LGTM!The invite configuration is properly structured and injected into the ConfigService options, consistent with how other configuration sections (like
media) are handled.ee/packages/federation-matrix/src/api/_matrix/invite.ts (1)
11-11: Verify thatNotAllowedErroris exported by@rocket.chat/federation-sdk
We couldn’t locate this export in the SDK’s source or type definitions—confirm it’s defined and exported (or adjust the import/define the error locally).
ricardogarim
force-pushed
the
fix/fed-non-private-or-encrypted-rooms-join
branch
from
3ba1bb4 to
4299f18
Compare
ricardogarim
force-pushed
the
fix/fed-non-private-or-encrypted-rooms-join
branch
from
4299f18 to
c6b8c6f
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/i18n/src/locales/en.i18n.json (1)
2176-2176: Fix label to match federated setting name.String should say “Allow joining non-private federated rooms.” (with trailing period) to mirror the feature scope.
- "Federation_Service_Join_Non_Private_Rooms": "Allow joining non-private rooms", + "Federation_Service_Join_Non_Private_Rooms": "Allow joining non-private federated rooms.",
🧹 Nitpick comments (2)
apps/meteor/server/settings/federation-service.ts (1)
88-102: Verify if i18n labels and alert properties should be added for UI consistency.The new settings lack
i18nLabel,i18nDescription, andalertproperties that similar federation settings possess (e.g.,Federation_Service_EDU_Process_Typingon lines 70-77). If these settings will be exposed in the admin UI, consider adding these properties for consistency and better user experience.If these properties should be added, apply a diff similar to this:
await this.add('Federation_Service_Join_Encrypted_Rooms', false, { type: 'boolean', public: false, enterprise: true, modules: ['federation'], invalidValue: false, + i18nLabel: 'Federation_Service_Join_Encrypted_Rooms', + i18nDescription: 'Federation_Service_Join_Encrypted_Rooms_Description', + alert: 'Federation_Service_Join_Encrypted_Rooms_Alert', }); await this.add('Federation_Service_Join_Non_Private_Rooms', false, { type: 'boolean', public: false, enterprise: true, modules: ['federation'], invalidValue: false, + i18nLabel: 'Federation_Service_Join_Non_Private_Rooms', + i18nDescription: 'Federation_Service_Join_Non_Private_Rooms_Description', + alert: 'Federation_Service_Join_Non_Private_Rooms_Alert', });ee/packages/federation-matrix/src/api/_matrix/invite.ts (1)
358-403: Consider adding logging for error cases to aid debugging.The error handling correctly returns appropriate HTTP responses, but lacks logging for error scenarios. This can make troubleshooting production issues difficult, especially for the catch-all error case on lines 396-402.
Apply this diff to add error logging:
} catch (error) { if (error instanceof NotAllowedError) { + console.warn('Invite rejected due to federation settings', { roomId, eventId, error: error.message }); return { body: { errcode: 'M_FORBIDDEN', error: 'This server does not allow joining this type of room based on federation settings.', }, statusCode: 403, }; } + console.error('Error processing invite', { roomId, eventId, error }); return { body: { errcode: 'M_UNKNOWN', error: error instanceof Error ? error.message : 'Internal server error while processing request', }, statusCode: 500, }; }
📜 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)
apps/meteor/server/settings/federation-service.ts(1 hunks)ee/packages/federation-matrix/src/api/_matrix/invite.ts(2 hunks)ee/packages/federation-matrix/src/setup.ts(2 hunks)packages/i18n/src/locales/en.i18n.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- ee/packages/federation-matrix/src/setup.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
PR: RocketChat/Rocket.Chat#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/server/settings/federation-service.tsee/packages/federation-matrix/src/api/_matrix/invite.ts
⏰ 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: CodeQL-Build
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release-7.11.0 #37227 +/- ##
==================================================
- Coverage 66.39% 66.34% -0.05%
==================================================
Files 3386 3386
Lines 115618 115628 +10
Branches 21352 21353 +1
==================================================
- Hits 76765 76716 -49
- Misses 36251 36304 +53
- Partials 2602 2608 +6
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
sampaiodiego
added
the
stat: QA assured
Proposed changes (including videos or screenshots)
As per FDR-207, this PR adds a new setting that allows Rocket.Chat to control whether users can join non-private or non-encrypted rooms based on the invite event received from federation.
If an invite request does not meet the configured requirements, Rocket.Chat will respond with a 403 status code and will not store any data related to the rejected invite.
It’s important to note that this is not a formal rejection - the server returns an error response to the invite endpoint instead of sending a m.room.member rejection event, in accordance with the Matrix specification.
Related with RocketChat/homeserver#280 that adds the settings capabilities on homeserver side.
Issue(s)
Steps to test or reproduce
Scenario 1 – Encrypted rooms (allowed)
Federation_Service_Join_Encrypted_Roomssetting to true.Scenario 2 – Encrypted rooms (not allowed)
Federation_Service_Join_Encrypted_Roomssetting to false (default).Scenario 3 – Public rooms (allowed)
Federation_Service_Join_Non_Private_Roomssetting to true.Scenario 4 – Public rooms (not allowed)
Federation_Service_Join_Non_Private_Roomssetting to false (default).Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores