-
Notifications
You must be signed in to change notification settings - Fork 13k
feat(federation): validate user email domain #38356
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,4 +1,4 @@ | ||||||||||||||||||||||
| import { type IFederationMatrixService, Room, ServiceClass } from '@rocket.chat/core-services'; | ||||||||||||||||||||||
| import { Authorization, type IFederationMatrixService, Room, ServiceClass } from '@rocket.chat/core-services'; | ||||||||||||||||||||||
| import { | ||||||||||||||||||||||
| isDeletedMessage, | ||||||||||||||||||||||
| isMessageFromMatrixFederation, | ||||||||||||||||||||||
|
|
@@ -36,6 +36,8 @@ export class FederationMatrix extends ServiceClass implements IFederationMatrixS | |||||||||||||||||||||
|
|
||||||||||||||||||||||
| private processEDUPresence: boolean; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| private validateUserDomain: boolean; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| private readonly logger = new Logger(this.name); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| override async created(): Promise<void> { | ||||||||||||||||||||||
|
|
@@ -52,6 +54,8 @@ export class FederationMatrix extends ServiceClass implements IFederationMatrixS | |||||||||||||||||||||
| this.processEDUTyping = value; | ||||||||||||||||||||||
| } else if (_id === 'Federation_Service_EDU_Process_Presence' && typeof value === 'boolean') { | ||||||||||||||||||||||
| this.processEDUPresence = value; | ||||||||||||||||||||||
| } else if (_id === 'Federation_Service_Validate_User_Domain' && typeof value === 'boolean') { | ||||||||||||||||||||||
| this.validateUserDomain = value; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| }); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
@@ -98,6 +102,7 @@ export class FederationMatrix extends ServiceClass implements IFederationMatrixS | |||||||||||||||||||||
| this.serverName = (await Settings.getValueById<string>('Federation_Service_Domain')) || ''; | ||||||||||||||||||||||
| this.processEDUTyping = (await Settings.getValueById<boolean>('Federation_Service_EDU_Process_Typing')) || false; | ||||||||||||||||||||||
| this.processEDUPresence = (await Settings.getValueById<boolean>('Federation_Service_EDU_Process_Presence')) || false; | ||||||||||||||||||||||
| this.validateUserDomain = (await Settings.getValueById<boolean>('Federation_Service_Validate_User_Domain')) || false; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| async createRoom(room: IRoom, owner: IUser): Promise<{ room_id: string; event_id: string }> { | ||||||||||||||||||||||
|
|
@@ -802,4 +807,21 @@ export class FederationMatrix extends ServiceClass implements IFederationMatrixS | |||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| async canUserAccessFederation(user: IUser): Promise<boolean> { | ||||||||||||||||||||||
| if (!(await Authorization.hasPermission(user._id, 'access-federation'))) { | ||||||||||||||||||||||
| return false; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if (!this.validateUserDomain) { | ||||||||||||||||||||||
| return true; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| return ( | ||||||||||||||||||||||
| user.emails?.some((email) => { | ||||||||||||||||||||||
| const domain = email.address.split('@')[1]; | ||||||||||||||||||||||
| return domain === this.serverName && email.verified; | ||||||||||||||||||||||
| }) ?? false | ||||||||||||||||||||||
|
Comment on lines
+820
to
+824
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Email domain comparison should be case-insensitive. Per RFC 5321, domain names in email addresses are case-insensitive. If a user's email was stored as 🔧 Suggested fix return (
user.emails?.some((email) => {
- const domain = email.address.split('@')[1];
- return domain === this.serverName && email.verified;
+ const domain = email.address.split('@')[1]?.toLowerCase();
+ return domain === this.serverName.toLowerCase() && email.verified;
}) ?? false
);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we care about this?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The practical issue: Why it matters:
Recommendation: const domain = email.address.split('@')[1]?.toLowerCase();
return domain === this.serverName.toLowerCase() && email.verified;This ensures reliable domain matching regardless of how the email address was originally stored. 🧠 Learnings used |
||||||||||||||||||||||
| ); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
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.
🧩 Analysis chain
🏁 Script executed:
Repository: RocketChat/Rocket.Chat
Length of output: 622
🏁 Script executed:
Repository: RocketChat/Rocket.Chat
Length of output: 4100
🏁 Script executed:
Repository: RocketChat/Rocket.Chat
Length of output: 743
🏁 Script executed:
Repository: RocketChat/Rocket.Chat
Length of output: 595
🏁 Script executed:
Repository: RocketChat/Rocket.Chat
Length of output: 5921
🏁 Script executed:
Repository: RocketChat/Rocket.Chat
Length of output: 1253
🏁 Script executed:
Repository: RocketChat/Rocket.Chat
Length of output: 646
🏁 Script executed:
Repository: RocketChat/Rocket.Chat
Length of output: 729
🏁 Script executed:
Repository: RocketChat/Rocket.Chat
Length of output: 542
🏁 Script executed:
Repository: RocketChat/Rocket.Chat
Length of output: 2276
🏁 Script executed:
Repository: RocketChat/Rocket.Chat
Length of output: 577
🏁 Script executed:
Repository: RocketChat/Rocket.Chat
Length of output: 642
🏁 Script executed:
Repository: RocketChat/Rocket.Chat
Length of output: 187
🏁 Script executed:
Repository: RocketChat/Rocket.Chat
Length of output: 311
Fix caller types to pass complete user objects to
RoomService.join.The
userparameter expects fullIUsertype forcanUserAccessFederationto accessuser.emailsfor domain validation. Two call sites have issues:apps/meteor/app/slashcommands-join/server/server.ts:46— fetches user with only{ federated: 1, federation: 1 }projection, excludingemails. Includeemailsin the projection before passing toRoom.join.apps/meteor/app/lib/server/functions/getRoomByNameOrIdWithOptionToJoin.ts:17— function signature acceptsPick<IUser, '_id' | 'username' | 'federated' | 'federation'>but passes toRoom.joinexpecting fullIUser. Update the parameter type or ensure callers pass complete user objects.🤖 Prompt for AI Agents