-
Notifications
You must be signed in to change notification settings - Fork 13.1k
chore(backend): prevent federation actions for old federation #36988
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
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
|
Warning Rate limit exceeded@ggazzo has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 13 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (98)
WalkthroughThis PR removes the legacy Federation v2 implementation across CE and EE: endpoints, services, domain models, Matrix bridge, converters, handlers, adapters, hooks, slash commands, and startup wiring. Room permission checks are simplified to boolean gates. Federation E2E tests are skipped. No public exports’ signatures are added or changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Client/UI
participant S as Server
Note over S: Before (removed)<br/>on room creation -> federation.beforeCreateRoom(...)
UI->>S: Create room
alt Now (after PR)
S->>S: if isFederationEnabled && isRoomNativeFederated then allow
else otherwise
S->>S: deny federated-specific actions
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
9af68de to
0e009ab
Compare
56161ef to
b448402
Compare
0e009ab to
b3755aa
Compare
b448402 to
6bfeb72
Compare
b3755aa to
29a751a
Compare
142d9e9 to
772a086
Compare
29a751a to
ec3a86b
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #36988 +/- ##
========================================
Coverage 66.33% 66.34%
========================================
Files 3391 3391
Lines 115308 115300 -8
Branches 21129 21128 -1
========================================
+ Hits 76494 76497 +3
+ Misses 36198 36190 -8
+ Partials 2616 2613 -3
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
c0723a5 to
f0184b6
Compare
153afd4 to
f5f4c3b
Compare
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 (8)
apps/meteor/ee/server/startup/index.ts (1)
7-7: Optional: lazy‑load services after broker/presence to avoid startup‑order coupling.Apply:
@@ -import './services'; +// (moved) lazy‑load after broker/presence is ready export const registerEEBroker = async (): Promise<void> => { // only starts network broker if running in micro services mode if (isRunningMs()) { const { startBroker } = await import('@rocket.chat/network-broker'); api.setBroker(startBroker()); await api.start(); + await import('./services'); } else { require('./presence'); + await import('./services'); } };apps/meteor/tests/end-to-end/api/federation.ts (1)
7-7: Suite skipped: confirm long-term intent or gate conditionallyIf this is a permanent removal of legacy federation, consider deleting the suite or skipping conditionally via a feature flag/env to keep the file runnable when native federation is enabled. Add a brief reason (ticket FDR-143) to avoid future confusion.
apps/meteor/app/lib/server/functions/deleteUser.ts (1)
50-53: Use a translatable/error-code-consistent messagePrefer consistent error code and metadata like other paths in this method (e.g., line 36). Proposal:
- throw new Meteor.Error('error-not-allowed', 'User participated in federation, this user can only be deactivated permanently', { - method: 'deleteUser', - }); + throw new Meteor.Error('error-action-not-allowed', 'Cannot delete federated users; deactivate instead', { + method: 'deleteUser', + action: 'Delete_user', + });apps/meteor/server/lib/rooms/roomTypes/private.ts (1)
15-18: DRY the federated-action gate across room typesThe same native+enabled gate is duplicated in private/direct/public. Extract a helper (e.g., canPerformFederatedAction(room)) and reuse.
- if (isRoomNativeFederated(room) && isFederationEnabled()) { - return true; - } - return false; + return canPerformFederatedAction(room);Outside this file (supporting change):
// apps/meteor/server/services/federation/utils.ts import type { IRoom } from '@rocket.chat/core-typings'; import { isRoomNativeFederated } from '@rocket.chat/core-typings'; export function canPerformFederatedAction(room: Partial<IRoom>): boolean { return isRoomNativeFederated(room) && isFederationEnabled(); }Also applies to: 37-43
apps/meteor/server/lib/rooms/roomTypes/direct.ts (1)
26-30: Same native+enabled gate duplicationMirror of private/public. Please centralize via a shared helper to avoid drift.
- if (isRoomNativeFederated(_room) && isFederationEnabled()) { - return true; - } - return false; + return canPerformFederatedAction(_room);Also applies to: 48-54
apps/meteor/app/lib/server/methods/addUsersToRoom.ts (3)
21-23: Harden federated-username detection (order + stricter match).Current check can misclassify any string containing both '@' and ':' (wrong order, extra spaces). Use an anchored regex to match Matrix-style IDs.
-const isAFederatedUsername = (username: string) => { - return username.includes('@') && username.includes(':'); -}; +// Matrix-style userId: '@user:domain' +const FEDERATED_USERNAME_RX = /^@[^:\s]+:[^:\s]+$/; +const isAFederatedUsername = (username: string): boolean => FEDERATED_USERNAME_RX.test(username);
88-90: Normalize/deduplicate invitees before processing.Prevents redundant DB lookups and duplicate invites; trims accidental whitespace.
-await Promise.all( - data.users.map(async (username) => { +const invitees = [...new Set(data.users.map((u) => u.trim()).filter(Boolean))]; +await Promise.all( + invitees.map(async (username) => {
82-86: Guard inviter presence for federation callback (defensive).inviter can be undefined if this method is reused outside DDP. Ensure the callback tolerates it or assert non-null here.
📜 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 (98)
apps/meteor/app/api/server/index.ts(0 hunks)apps/meteor/app/api/server/v1/federation.ts(0 hunks)apps/meteor/app/lib/server/functions/deleteUser.ts(2 hunks)apps/meteor/app/lib/server/methods/addUsersToRoom.ts(2 hunks)apps/meteor/ee/server/api/federation/index.ts(0 hunks)apps/meteor/ee/server/api/federation/rooms.ts(0 hunks)apps/meteor/ee/server/local-services/federation/application/AbstractFederationApplicationServiceEE.ts(0 hunks)apps/meteor/ee/server/local-services/federation/application/UserService.ts(0 hunks)apps/meteor/ee/server/local-services/federation/application/room/sender/DirectMessageRoomServiceSender.ts(0 hunks)apps/meteor/ee/server/local-services/federation/application/room/sender/RoomServiceSender.ts(0 hunks)apps/meteor/ee/server/local-services/federation/application/room/sender/input/RoomInputDto.ts(0 hunks)apps/meteor/ee/server/local-services/federation/application/room/sender/input/RoomSenderDto.ts(0 hunks)apps/meteor/ee/server/local-services/federation/domain/FederatedRoom.ts(0 hunks)apps/meteor/ee/server/local-services/federation/domain/FederatedUser.ts(0 hunks)apps/meteor/ee/server/local-services/federation/domain/IFederationBridge.ts(0 hunks)apps/meteor/ee/server/local-services/federation/infrastructure/Factory.ts(0 hunks)apps/meteor/ee/server/local-services/federation/infrastructure/matrix/Bridge.ts(0 hunks)apps/meteor/ee/server/local-services/federation/infrastructure/rocket-chat/adapters/Queue.ts(0 hunks)apps/meteor/ee/server/local-services/federation/infrastructure/rocket-chat/adapters/Room.ts(0 hunks)apps/meteor/ee/server/local-services/federation/infrastructure/rocket-chat/adapters/User.ts(0 hunks)apps/meteor/ee/server/local-services/federation/infrastructure/rocket-chat/converters/RoomSender.ts(0 hunks)apps/meteor/ee/server/local-services/federation/infrastructure/rocket-chat/hooks/index.ts(0 hunks)apps/meteor/ee/server/local-services/federation/infrastructure/rocket-chat/slash-commands/action.ts(0 hunks)apps/meteor/ee/server/local-services/federation/infrastructure/rocket-chat/slash-commands/index.ts(0 hunks)apps/meteor/ee/server/local-services/federation/service.ts(0 hunks)apps/meteor/ee/server/startup/index.ts(1 hunks)apps/meteor/ee/server/startup/services.ts(0 hunks)apps/meteor/server/lib/rooms/roomTypes/direct.ts(3 hunks)apps/meteor/server/lib/rooms/roomTypes/private.ts(2 hunks)apps/meteor/server/lib/rooms/roomTypes/public.ts(2 hunks)apps/meteor/server/services/federation/Federation.ts(0 hunks)apps/meteor/server/services/federation/application/AbstractFederationApplicationService.ts(0 hunks)apps/meteor/server/services/federation/application/room/input/MessageReceiverDto.ts(0 hunks)apps/meteor/server/services/federation/application/room/input/RoomReceiverDto.ts(0 hunks)apps/meteor/server/services/federation/application/room/input/RoomSenderDto.ts(0 hunks)apps/meteor/server/services/federation/application/room/input/UserReceiverDto.ts(0 hunks)apps/meteor/server/services/federation/application/room/message/receiver/MessageServiceReceiver.ts(0 hunks)apps/meteor/server/services/federation/application/room/message/receiver/message-redaction-helper.ts(0 hunks)apps/meteor/server/services/federation/application/room/message/sender/MessageServiceSender.ts(0 hunks)apps/meteor/server/services/federation/application/room/message/sender/message-sender-helper.ts(0 hunks)apps/meteor/server/services/federation/application/room/receiver/RoomServiceReceiver.ts(0 hunks)apps/meteor/server/services/federation/application/room/sender/RoomInternalValidator.ts(0 hunks)apps/meteor/server/services/federation/application/room/sender/RoomServiceSender.ts(0 hunks)apps/meteor/server/services/federation/application/user/receiver/UserServiceReceiver.ts(0 hunks)apps/meteor/server/services/federation/application/user/sender/UserServiceSender.ts(0 hunks)apps/meteor/server/services/federation/domain/FederatedRoom.ts(0 hunks)apps/meteor/server/services/federation/domain/FederatedUser.ts(0 hunks)apps/meteor/server/services/federation/domain/IFederationBridge.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/Factory.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/Bridge.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/converters/room/MessageReceiver.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/converters/room/RoomReceiver.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/converters/room/to-internal-parser-formatter.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/converters/user/UserReceiver.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/definitions/AbstractMatrixEvent.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/definitions/MatrixEventType.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/definitions/MatrixPowerLevels.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/definitions/MatrixRoomJoinRules.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/definitions/MatrixRoomType.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/definitions/MatrixRoomVisibility.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/definitions/events/MessageReacted.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/definitions/events/RoomCreated.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/definitions/events/RoomEventRedacted.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/definitions/events/RoomJoinRulesChanged.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/definitions/events/RoomMembershipChanged.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/definitions/events/RoomMessageSent.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/definitions/events/RoomNameChanged.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/definitions/events/RoomPowerLevelsChanged.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/definitions/events/RoomTopicChanged.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/definitions/events/UserTypingStatusChanged.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/handlers/BaseEvent.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/handlers/Message.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/handlers/Room.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/handlers/User.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/handlers/index.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/helpers/HtttpStatusCodes.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/helpers/MatrixIdStringTools.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/matrix/helpers/MatrixIdVerificationTypes.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/queue/InMemoryQueue.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/File.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Message.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Notification.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Room.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/User.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/federation-id-escape-helper.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/logger.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/rocket-chat/converters/RoomSender.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/rocket-chat/converters/to-external-parser-formatter.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/rocket-chat/definitions/FederatedRoomInternalRoles.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/rocket-chat/hooks/index.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/rocket-chat/slash-commands/action.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/rocket-chat/slash-commands/index.ts(0 hunks)apps/meteor/server/services/federation/infrastructure/rocket-chat/well-known.ts(0 hunks)apps/meteor/server/services/federation/service.ts(0 hunks)apps/meteor/server/services/federation/utils.ts(0 hunks)apps/meteor/startRocketChat.ts(0 hunks)apps/meteor/tests/end-to-end/api/federation.ts(1 hunks)
💤 Files with no reviewable changes (91)
- apps/meteor/app/api/server/index.ts
- apps/meteor/server/services/federation/infrastructure/rocket-chat/slash-commands/index.ts
- apps/meteor/server/services/federation/infrastructure/rocket-chat/definitions/FederatedRoomInternalRoles.ts
- apps/meteor/server/services/federation/utils.ts
- apps/meteor/ee/server/local-services/federation/infrastructure/rocket-chat/adapters/Queue.ts
- apps/meteor/server/services/federation/application/user/sender/UserServiceSender.ts
- apps/meteor/startRocketChat.ts
- apps/meteor/server/services/federation/infrastructure/matrix/definitions/MatrixEventType.ts
- apps/meteor/ee/server/local-services/federation/infrastructure/rocket-chat/slash-commands/index.ts
- apps/meteor/ee/server/local-services/federation/application/room/sender/DirectMessageRoomServiceSender.ts
- apps/meteor/server/services/federation/infrastructure/matrix/definitions/MatrixPowerLevels.ts
- apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Notification.ts
- apps/meteor/ee/server/api/federation/index.ts
- apps/meteor/ee/server/local-services/federation/application/UserService.ts
- apps/meteor/server/services/federation/infrastructure/matrix/handlers/index.ts
- apps/meteor/ee/server/local-services/federation/infrastructure/rocket-chat/adapters/User.ts
- apps/meteor/server/services/federation/infrastructure/matrix/definitions/events/RoomCreated.ts
- apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Room.ts
- apps/meteor/server/services/federation/infrastructure/matrix/converters/user/UserReceiver.ts
- apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts
- apps/meteor/server/services/federation/infrastructure/matrix/definitions/MatrixRoomJoinRules.ts
- apps/meteor/ee/server/local-services/federation/infrastructure/Factory.ts
- apps/meteor/server/services/federation/application/room/input/RoomSenderDto.ts
- apps/meteor/ee/server/local-services/federation/domain/FederatedUser.ts
- apps/meteor/server/services/federation/infrastructure/rocket-chat/hooks/index.ts
- apps/meteor/server/services/federation/infrastructure/matrix/definitions/AbstractMatrixEvent.ts
- apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Message.ts
- apps/meteor/server/services/federation/infrastructure/rocket-chat/converters/to-external-parser-formatter.ts
- apps/meteor/ee/server/local-services/federation/application/room/sender/input/RoomInputDto.ts
- apps/meteor/ee/server/local-services/federation/infrastructure/matrix/Bridge.ts
- apps/meteor/server/services/federation/infrastructure/matrix/handlers/User.ts
- apps/meteor/server/services/federation/infrastructure/matrix/definitions/events/RoomMessageSent.ts
- apps/meteor/server/services/federation/infrastructure/matrix/definitions/MatrixRoomVisibility.ts
- apps/meteor/server/services/federation/infrastructure/matrix/helpers/MatrixIdVerificationTypes.ts
- apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/logger.ts
- apps/meteor/ee/server/local-services/federation/infrastructure/rocket-chat/hooks/index.ts
- apps/meteor/server/services/federation/infrastructure/queue/InMemoryQueue.ts
- apps/meteor/ee/server/local-services/federation/infrastructure/rocket-chat/adapters/Room.ts
- apps/meteor/server/services/federation/application/room/input/MessageReceiverDto.ts
- apps/meteor/server/services/federation/infrastructure/rocket-chat/converters/RoomSender.ts
- apps/meteor/server/services/federation/infrastructure/matrix/definitions/events/RoomNameChanged.ts
- apps/meteor/server/services/federation/infrastructure/matrix/definitions/events/RoomPowerLevelsChanged.ts
- apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/File.ts
- apps/meteor/server/services/federation/domain/IFederationBridge.ts
- apps/meteor/server/services/federation/infrastructure/matrix/definitions/events/RoomMembershipChanged.ts
- apps/meteor/server/services/federation/domain/FederatedUser.ts
- apps/meteor/server/services/federation/infrastructure/matrix/helpers/HtttpStatusCodes.ts
- apps/meteor/server/services/federation/application/room/receiver/RoomServiceReceiver.ts
- apps/meteor/server/services/federation/infrastructure/matrix/definitions/MatrixRoomType.ts
- apps/meteor/server/services/federation/application/AbstractFederationApplicationService.ts
- apps/meteor/server/services/federation/infrastructure/matrix/definitions/events/RoomJoinRulesChanged.ts
- apps/meteor/ee/server/api/federation/rooms.ts
- apps/meteor/ee/server/startup/services.ts
- apps/meteor/ee/server/local-services/federation/application/AbstractFederationApplicationServiceEE.ts
- apps/meteor/ee/server/local-services/federation/infrastructure/rocket-chat/slash-commands/action.ts
- apps/meteor/server/services/federation/application/user/receiver/UserServiceReceiver.ts
- apps/meteor/server/services/federation/application/room/input/UserReceiverDto.ts
- apps/meteor/server/services/federation/infrastructure/matrix/handlers/BaseEvent.ts
- apps/meteor/ee/server/local-services/federation/infrastructure/rocket-chat/converters/RoomSender.ts
- apps/meteor/server/services/federation/infrastructure/rocket-chat/slash-commands/action.ts
- apps/meteor/server/services/federation/infrastructure/matrix/handlers/Message.ts
- apps/meteor/server/services/federation/infrastructure/matrix/converters/room/RoomReceiver.ts
- apps/meteor/server/services/federation/application/room/sender/RoomInternalValidator.ts
- apps/meteor/server/services/federation/application/room/sender/RoomServiceSender.ts
- apps/meteor/app/api/server/v1/federation.ts
- apps/meteor/server/services/federation/infrastructure/matrix/Bridge.ts
- apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/User.ts
- apps/meteor/server/services/federation/infrastructure/matrix/definitions/events/MessageReacted.ts
- apps/meteor/ee/server/local-services/federation/domain/FederatedRoom.ts
- apps/meteor/server/services/federation/infrastructure/matrix/definitions/events/RoomEventRedacted.ts
- apps/meteor/server/services/federation/infrastructure/matrix/definitions/events/UserTypingStatusChanged.ts
- apps/meteor/ee/server/local-services/federation/application/room/sender/input/RoomSenderDto.ts
- apps/meteor/server/services/federation/application/room/message/receiver/MessageServiceReceiver.ts
- apps/meteor/server/services/federation/infrastructure/matrix/helpers/MatrixIdStringTools.ts
- apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/federation-id-escape-helper.ts
- apps/meteor/server/services/federation/service.ts
- apps/meteor/server/services/federation/application/room/message/receiver/message-redaction-helper.ts
- apps/meteor/server/services/federation/infrastructure/matrix/definitions/events/RoomTopicChanged.ts
- apps/meteor/ee/server/local-services/federation/domain/IFederationBridge.ts
- apps/meteor/server/services/federation/application/room/message/sender/MessageServiceSender.ts
- apps/meteor/server/services/federation/infrastructure/matrix/converters/room/to-internal-parser-formatter.ts
- apps/meteor/server/services/federation/application/room/input/RoomReceiverDto.ts
- apps/meteor/ee/server/local-services/federation/application/room/sender/RoomServiceSender.ts
- apps/meteor/server/services/federation/infrastructure/matrix/handlers/Room.ts
- apps/meteor/server/services/federation/application/room/message/sender/message-sender-helper.ts
- apps/meteor/ee/server/local-services/federation/service.ts
- apps/meteor/server/services/federation/Federation.ts
- apps/meteor/server/services/federation/infrastructure/matrix/converters/room/MessageReceiver.ts
- apps/meteor/server/services/federation/infrastructure/rocket-chat/well-known.ts
- apps/meteor/server/services/federation/domain/FederatedRoom.ts
- apps/meteor/server/services/federation/infrastructure/Factory.ts
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
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.613Z
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.
📚 Learning: 2025-09-19T15:15:04.613Z
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.613Z
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/app/lib/server/methods/addUsersToRoom.tsapps/meteor/server/lib/rooms/roomTypes/public.tsapps/meteor/server/lib/rooms/roomTypes/private.tsapps/meteor/app/lib/server/functions/deleteUser.tsapps/meteor/server/lib/rooms/roomTypes/direct.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
PR: RocketChat/Rocket.Chat#0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/end-to-end/api/federation.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
PR: RocketChat/Rocket.Chat#0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases
Applied to files:
apps/meteor/tests/end-to-end/api/federation.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
PR: RocketChat/Rocket.Chat#0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use descriptive test names that clearly communicate expected behavior
Applied to files:
apps/meteor/tests/end-to-end/api/federation.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
PR: RocketChat/Rocket.Chat#0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
apps/meteor/tests/end-to-end/api/federation.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
PR: RocketChat/Rocket.Chat#0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Avoid code comments in the implementation
Applied to files:
apps/meteor/tests/end-to-end/api/federation.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
PR: RocketChat/Rocket.Chat#0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use test.beforeAll() and test.afterAll() for setup and teardown
Applied to files:
apps/meteor/tests/end-to-end/api/federation.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
PR: RocketChat/Rocket.Chat#0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use test.step() to organize complex test scenarios
Applied to files:
apps/meteor/tests/end-to-end/api/federation.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
PR: RocketChat/Rocket.Chat#0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure a clean state for each test execution
Applied to files:
apps/meteor/tests/end-to-end/api/federation.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
PR: RocketChat/Rocket.Chat#0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Write concise, technical TypeScript/JavaScript with accurate typing
Applied to files:
apps/meteor/tests/end-to-end/api/federation.ts
🧬 Code graph analysis (3)
apps/meteor/server/lib/rooms/roomTypes/public.ts (2)
packages/core-typings/src/IRoom.ts (3)
isRoomFederated(116-116)isRoomNativeFederated(118-119)IRoom(21-95)apps/meteor/server/services/federation/utils.ts (1)
isFederationEnabled(3-5)
apps/meteor/server/lib/rooms/roomTypes/private.ts (2)
packages/core-typings/src/IRoom.ts (3)
isRoomFederated(116-116)isRoomNativeFederated(118-119)IRoom(21-95)apps/meteor/server/services/federation/utils.ts (1)
isFederationEnabled(3-5)
apps/meteor/server/lib/rooms/roomTypes/direct.ts (2)
packages/core-typings/src/IRoom.ts (2)
isRoomNativeFederated(118-119)isRoomFederated(116-116)apps/meteor/server/services/federation/utils.ts (1)
isFederationEnabled(3-5)
🔇 Additional comments (7)
apps/meteor/ee/server/startup/index.ts (1)
7-7: LGTM — brief verification
- apps/meteor/ee/server/startup/services.ts imports api and calls api.registerService(...) for EnterpriseSettings, LDAPEEService, LicenseService, MessageReadsService, OmnichannelEE, VoipFreeSwitchService; InstanceService is registered only when !isRunningMs().
- No legacy federation startup references found in this file.
- No api.start or broker usage found here.
- Confirm whether api.registerService must be invoked only after api.start() in microservices mode; if so, gate the unguarded registrations (all except InstanceService).
apps/meteor/server/lib/rooms/roomTypes/direct.ts (1)
10-10: Confirm flag correctness for native federationAs in private.ts, verify that isFederationEnabled (backed by Federation_Matrix_enabled) reflects the correct feature gate for native federation.
apps/meteor/server/lib/rooms/roomTypes/public.ts (1)
17-20: Gating looks consistent—pending flag verificationLogic is consistent with private/direct. Pending confirmation that the “enabled” flag is the correct one for native federation, this looks good.
Also applies to: 39-42
apps/meteor/app/lib/server/functions/deleteUser.ts (2)
50-53: Clarify federated-user deletion scopeCurrent code uses isUserFederated(user) and therefore blocks deletion for any federated user (apps/meteor/app/lib/server/functions/deleteUser.ts:50–53). If the intent is to block only native/Matrix federated accounts, switch the check to isUserNativeFederated(user) — exported in packages/core-typings/src/IUser.ts (isUserNativeFederated at ~lines 269–272). Confirm intended behavior.
173-175: Verify necessity of FederationServers.refreshServers()Found at apps/meteor/app/lib/server/functions/deleteUser.ts:174. Earlier repo-limited search showed no other usages; a repo-wide ripgrep attempt here failed due to filters. Run a full search to confirm no other references (e.g. rg -n --hidden -S --no-ignore '\bFederationServers.refreshServers(') and remove the call if federation is removed to avoid unnecessary I/O.
apps/meteor/server/lib/rooms/roomTypes/private.ts (1)
15-19: Gating logic: Federation is gated by Federation_Matrix_enabled (verified)isFederationEnabled() returns settings.get('Federation_Matrix_enabled') (apps/meteor/server/services/federation/utils.ts); repo search found no other Federation_*_enabled flags — using isRoomNativeFederated(...) && isFederationEnabled() is consistent with the codebase.
apps/meteor/app/lib/server/methods/addUsersToRoom.ts (1)
91-99: addUserToRoom accepts string invitees — confirm intended UX for non‑federated rooms
- packages/core-services/src/types/IRoomService.ts: addUserToRoom(roomId: string, user: Pick<IUser,'_id'> | string, ...) — string allowed.
- apps/meteor/app/lib/server/methods/addUsersToRoom.ts: calls await addUserToRoom(data.rid, newUser || username, user) so federated‑style usernames will be forwarded as strings.
- apps/meteor/server/services/room/service.ts: wrapper typed as Pick<IUser,'_id'> delegates to addUserToRoom — verify this typing/delegation and confirm whether local (non‑federated) rooms should accept/reject federated usernames or be routed to federation logic.
…mproved federation checks in room types
174bc85 to
c8a7a32
Compare
https://rocketchat.atlassian.net/browse/FDR-143
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit