regression(federation): allow invite users with upper case username#37970
regression(federation): allow invite users with upper case username#37970ggazzo merged 3 commits intorelease-8.0.0from
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 |
|
WalkthroughThe PR refactors the federation-matrix module by extracting utility functions into dedicated helper modules. Functions Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## release-8.0.0 #37970 +/- ##
=================================================
+ Coverage 70.60% 70.61% +0.01%
=================================================
Files 3146 3147 +1
Lines 108690 108734 +44
Branches 19523 19539 +16
=================================================
+ Hits 76738 76781 +43
Misses 29947 29947
- Partials 2005 2006 +1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
📦 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 23:01", "12/02 21:57", "12/03 21:00", "12/04 18:17", "12/05 21:56", "12/08 20:15", "12/09 22:17", "12/10 23:26", "12/11 21:56", "12/12 22:45", "12/13 01:34", "12/15 22:31", "12/16 22:18", "12/17 21:04", "12/18 23:12", "12/19 23:27", "12/20 21:03", "12/22 18:54", "12/23 16:16", "12/24 19:05 (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, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 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, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 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, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 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, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 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, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 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, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 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.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.35]
Statistics (last 28 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Wed, 24 Dec 2025 19:05:54 GMT |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![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
🧹 Nitpick comments (1)
ee/packages/federation-matrix/src/helpers/createOrUpdateFederatedUser.ts (1)
16-16: Track the TODO for future refactoring.The TODO comment indicates that a specific method should be created for this upsert operation. Consider creating an issue to track this technical debt.
Do you want me to open a new issue to track the creation of a dedicated upsert method for federated users?
📜 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 (10)
ee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/events/room.tsee/packages/federation-matrix/src/helpers/createOrUpdateFederatedUser.tsee/packages/federation-matrix/src/helpers/extractDomainFromMatrixUserId.tsee/packages/federation-matrix/src/helpers/getUsernameServername.tsee/packages/federation-matrix/src/helpers/validateFederatedUsername.spec.tsee/packages/federation-matrix/src/helpers/validateFederatedUsername.tsee/packages/federation-matrix/src/index.tspackages/core-services/package.json
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
ee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/helpers/extractDomainFromMatrixUserId.tsee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/room.tsee/packages/federation-matrix/src/helpers/createOrUpdateFederatedUser.tsee/packages/federation-matrix/src/helpers/getUsernameServername.tsee/packages/federation-matrix/src/helpers/validateFederatedUsername.tsee/packages/federation-matrix/src/helpers/validateFederatedUsername.spec.tsee/packages/federation-matrix/src/index.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use.spec.tsextension for test files (e.g.,login.spec.ts)
Files:
ee/packages/federation-matrix/src/helpers/validateFederatedUsername.spec.ts
🧠 Learnings (18)
📓 Common learnings
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.
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: ee/packages/federation-matrix/src/setup.ts:103-120
Timestamp: 2025-11-05T21:04:35.787Z
Learning: In Rocket.Chat's federation-matrix setup (ee/packages/federation-matrix/src/setup.ts and apps/meteor/ee/server/startup/federation.ts), configureFederationMatrixSettings does not need to be called before setupFederationMatrix. The SDK's init() establishes infrastructure (database, event handlers, APIs) first, and the configuration can be applied later via settings watchers before actual federation events are processed. The config only matters when events actually occur, at which point all infrastructure is already configured.
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.
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.
📚 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:
packages/core-services/package.jsonee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/room.tsee/packages/federation-matrix/src/helpers/createOrUpdateFederatedUser.tsee/packages/federation-matrix/src/helpers/getUsernameServername.tsee/packages/federation-matrix/src/index.ts
📚 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:
packages/core-services/package.jsonee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/room.tsee/packages/federation-matrix/src/index.ts
📚 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:
ee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/room.tsee/packages/federation-matrix/src/helpers/createOrUpdateFederatedUser.tsee/packages/federation-matrix/src/index.ts
📚 Learning: 2025-11-05T21:04:35.787Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: ee/packages/federation-matrix/src/setup.ts:103-120
Timestamp: 2025-11-05T21:04:35.787Z
Learning: In Rocket.Chat's federation-matrix setup (ee/packages/federation-matrix/src/setup.ts and apps/meteor/ee/server/startup/federation.ts), configureFederationMatrixSettings does not need to be called before setupFederationMatrix. The SDK's init() establishes infrastructure (database, event handlers, APIs) first, and the configuration can be applied later via settings watchers before actual federation events are processed. The config only matters when events actually occur, at which point all infrastructure is already configured.
Applied to files:
ee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/room.tsee/packages/federation-matrix/src/index.ts
📚 Learning: 2025-12-09T20:01:00.324Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:00.324Z
Learning: When reviewing federation invite handling in Rocket.Chat (specifically under ee/packages/federation-matrix), understand that rejecting an invite via federationSDK.rejectInvite() triggers an event-driven cleanup: a leave event is emitted and handled by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, which calls Room.performUserRemoval() to remove the subscription. Do not add explicit cleanup in the reject branch of handleInvite(); rely on the existing leave-event flow for cleanup. If making changes, ensure this invariant remains and that any related paths still funnel cleanup through the leave event to avoid duplicate or missing removals.
Applied to files:
ee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/helpers/extractDomainFromMatrixUserId.tsee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/room.tsee/packages/federation-matrix/src/helpers/createOrUpdateFederatedUser.tsee/packages/federation-matrix/src/helpers/getUsernameServername.tsee/packages/federation-matrix/src/helpers/validateFederatedUsername.tsee/packages/federation-matrix/src/helpers/validateFederatedUsername.spec.tsee/packages/federation-matrix/src/index.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.
Applied to files:
ee/packages/federation-matrix/src/events/room.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.
Applied to files:
ee/packages/federation-matrix/src/events/room.ts
📚 Learning: 2025-11-27T17:56:26.050Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37557
File: apps/meteor/client/views/admin/ABAC/AdminABACRooms.tsx:115-116
Timestamp: 2025-11-27T17:56:26.050Z
Learning: In Rocket.Chat, the GET /v1/abac/rooms endpoint (implemented in ee/packages/abac/src/index.ts) only returns rooms where abacAttributes exists and is not an empty array (query: { abacAttributes: { $exists: true, $ne: [] } }). Therefore, in components consuming this endpoint (like AdminABACRooms.tsx), room.abacAttributes is guaranteed to be defined for all returned rooms, and optional chaining before calling array methods like .join() is sufficient without additional null coalescing.
Applied to files:
ee/packages/federation-matrix/src/events/room.ts
📚 Learning: 2025-12-10T21:00:43.645Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:43.645Z
Learning: Adopt the monorepo-wide Jest testMatch pattern: <rootDir>/src/**/*.spec.{ts,js,mjs} (represented here as '**/src/**/*.spec.{ts,js,mjs}') to ensure spec files under any package's src directory are picked up consistently across all packages in the Rocket.Chat monorepo. Apply this pattern in jest.config.ts for all relevant packages to maintain uniform test discovery.
Applied to files:
ee/packages/federation-matrix/src/helpers/validateFederatedUsername.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All test files must be created in `apps/meteor/tests/e2e/` directory
Applied to files:
ee/packages/federation-matrix/src/helpers/validateFederatedUsername.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
ee/packages/federation-matrix/src/helpers/validateFederatedUsername.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.spec.ts : Use `.spec.ts` extension for test files (e.g., `login.spec.ts`)
Applied to files:
ee/packages/federation-matrix/src/helpers/validateFederatedUsername.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
ee/packages/federation-matrix/src/helpers/validateFederatedUsername.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.spec.ts : Use descriptive test names that clearly communicate expected behavior in Playwright tests
Applied to files:
ee/packages/federation-matrix/src/helpers/validateFederatedUsername.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
ee/packages/federation-matrix/src/helpers/validateFederatedUsername.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
ee/packages/federation-matrix/src/helpers/validateFederatedUsername.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
ee/packages/federation-matrix/src/helpers/validateFederatedUsername.spec.ts
🧬 Code graph analysis (3)
ee/packages/federation-matrix/src/helpers/getUsernameServername.ts (1)
ee/packages/federation-matrix/src/helpers/extractDomainFromMatrixUserId.ts (1)
extractDomainFromMatrixUserId(1-7)
ee/packages/federation-matrix/src/helpers/validateFederatedUsername.ts (1)
ee/packages/federation-matrix/src/index.ts (1)
validateFederatedUsername(3-3)
ee/packages/federation-matrix/src/helpers/validateFederatedUsername.spec.ts (2)
ee/packages/federation-matrix/src/helpers/validateFederatedUsername.ts (1)
validateFederatedUsername(6-76)ee/packages/federation-matrix/src/index.ts (1)
validateFederatedUsername(3-3)
⏰ 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: cubic · AI code reviewer
🔇 Additional comments (13)
ee/packages/federation-matrix/src/events/room.ts (1)
5-5: LGTM! Import path correctly updated.The import relocation from the barrel FederationMatrix module to the dedicated helper module aligns with the refactoring objectives. The event handler logic remains unchanged.
ee/packages/federation-matrix/src/events/member.ts (1)
7-8: LGTM! Helper imports correctly relocated.The import paths have been updated to use dedicated helper modules instead of the barrel FederationMatrix module. The event handling logic remains unchanged.
ee/packages/federation-matrix/src/helpers/extractDomainFromMatrixUserId.ts (1)
1-7: LGTM! Clean domain extraction logic.The function correctly parses Matrix user IDs by locating the colon separator after the leading
@and extracting the domain portion. Error handling for invalid formats is appropriate.ee/packages/federation-matrix/src/helpers/validateFederatedUsername.spec.ts (1)
1-93: Excellent test coverage for the uppercase username fix.The test suite comprehensively validates both invalid and valid MXID formats. Lines 45-46 specifically test uppercase letters in the localpart, directly addressing the PR objectives FB-59 and FB-60. Coverage includes edge cases like IPv6 addresses, port ranges, encoded characters, and boundary conditions.
ee/packages/federation-matrix/src/helpers/getUsernameServername.ts (1)
9-21: LGTM! Username/servername extraction logic is correct.The function properly differentiates between local and remote users:
- Local users: extracts the username portion (strips
@and domain) and returnsisLocal=true- Remote users: preserves the full MXID and returns
isLocal=falseThe error handling is appropriate, though the check on lines 14-15 is somewhat redundant since
extractDomainFromMatrixUserIdalready validates the format on line 10.ee/packages/federation-matrix/src/helpers/validateFederatedUsername.ts (2)
17-17: Core fix: Uppercase letter support in localpart.Line 17 includes
a-zA-Zin the regex, which directly addresses the PR objectives by allowing uppercase letters in federated usernames. This resolves both FB-59 (user not found for mixed-case usernames) and FB-60 (500 error for existing mixed-case users).
1-76: Comprehensive Matrix ID validation with robust IPv6 support.The validation logic correctly handles:
- Localpart validation with uppercase support (line 17)
- IPv6 addresses in brackets with optional ports (lines 24-37)
- IPv4 addresses and hostnames with optional ports (lines 39-49)
- Port range validation 1-65535 (lines 68-73)
The implementation aligns with Matrix specifications and provides thorough error detection through early returns.
packages/core-services/package.json (1)
20-20: Dependency version is valid and free from vulnerabilities.The @rocket.chat/federation-sdk version 0.3.7 exists on npm, is the current latest release, and has no known security vulnerabilities. The bump from 0.3.5 to 0.3.7 is safe to proceed.
ee/packages/federation-matrix/src/FederationMatrix.ts (4)
12-12: LGTM: Helper imports support modular refactoring.The imports correctly reference the newly extracted helper modules and services, improving code organization and testability.
Also applies to: 17-21
710-755: LGTM: IPv6 handling improvement in Matrix ID parsing.Line 715 correctly handles Matrix IDs with IPv6 addresses in the homeserver domain by searching for the first ':' after the '@' symbol. This prevents incorrect splitting when the domain contains multiple colons (e.g.,
@user:[::1]:8008).
395-427: Case-sensitive federated username handling is correct.The invite flow properly validates federated usernames with
validateFederatedUsername(which accepts uppercase letters as confirmed by test cases) and passes the username directly touserIdSchema.parse()without transformation, preserving the case-sensitive User ID. Non-federated usernames follow the same pattern by constructing the full Matrix ID format.
130-151: Case preservation in federated user creation flow is correctly implemented.The helper chain preserves username case throughout:
validateFederatedUsernameaccepts mixed-case localparts via regex[a-zA-Z0-9._\-]extractDomainFromMatrixUserIdextracts only the domain portion, preserving localpart casingcreateOrUpdateFederatedUserstores the username as-is without transformation- No case-altering calls (toLowerCase/toUpperCase) exist in this package
This correctly supports FB-59's cross-domain mixed-case username lookups.
ee/packages/federation-matrix/src/index.ts (1)
3-5: Refactoring correctly improves modularity and handles uppercase usernames.The extraction of
validateFederatedUsernameto a dedicated helper module aligns with the PR objectives. The implementation correctly accepts uppercase letters via the regex pattern[a-zA-Z0-9._\-], and test coverage explicitly validates this behavior with the test case:validateFederatedUsername('@User:example.com')returnstrue.
| export async function createOrUpdateFederatedUser(options: { username: string; name?: string; origin: string }): Promise<IUser> { | ||
| const { username, name = username, origin } = options; | ||
|
|
||
| console.log('createOrUpdateFederatedUser ->', options); |
There was a problem hiding this comment.
Remove debug console.log statement.
The console.log statement should be removed before merging to production. Use a proper logger if debugging information is needed.
🔎 Proposed fix
- console.log('createOrUpdateFederatedUser ->', options);
-📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.log('createOrUpdateFederatedUser ->', options); |
🤖 Prompt for AI Agents
In ee/packages/federation-matrix/src/helpers/createOrUpdateFederatedUser.ts
around line 14, remove the debug console.log('createOrUpdateFederatedUser ->',
options); statement; if runtime debugging is required replace it with the
project logger (e.g., logger.debug/trace) and ensure sensitive data in options
is not logged, or simply delete the line to avoid console output in production.
3 participants
Proposed changes (including videos or screenshots)
There is one commit for each change easier understanding:
Issue(s)
FB-59
FB-60
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.