Skip to content

fix: update email template 'from' field to use only the email address#39896

Open
namann5 wants to merge 5 commits intoRocketChat:developfrom
namann5:develop
Open

fix: update email template 'from' field to use only the email address#39896
namann5 wants to merge 5 commits intoRocketChat:developfrom
namann5:develop

Conversation

@namann5
Copy link
Copy Markdown

@namann5 namann5 commented Mar 27, 2026

Fixed the issue in apps/meteor/app/authentication/server/startup/index.js.

What changed:

  • Accounts.emailTemplates.from was changed from:
    • ${settings.get('Site_Name')} <${settings.get('From_Email')}>
  • to:
    • settings.get('From_Email')

Why this fixes the bug:

  • Password recovery and email confirmation are sent through Meteor Accounts email templates.
  • Those flows were using a formatted From header like Site Name <user@mail.ru>.
  • Some SMTP providers, including the reported mail.ru setup, enforce that the actual From header exactly matches the authenticated SMTP user/address.
  • Test emails and some other Rocket.Chat mail flows already use the raw From_Email, which is why they were working while reset/verification emails were failing.
  • With this change, reset-password and verification emails now use the same plain sender address and should pass strict SMTP validation.

Files changed:

  • apps/meteor/app/authentication/server/startup/index.js

PR-ready summary:

  • Fix account emails to use raw From_Email in Meteor Accounts templates
  • Prevents SMTP rejection on providers that require the From header to exactly match the authenticated sender
  • Resolves failures for password recovery and email confirmation emails while preserving existing site name usage in subjects/content

Summary by CodeRabbit

  • Bug Fixes
    • Email sender address now uses only the configured email (site name no longer appears in the formatted sender).
    • IP whitelist handling improved: configured entries are now trimmed and empty values ignored, preventing mismatches from stray spaces.

@dionisio-bot
Copy link
Copy Markdown
Contributor

dionisio-bot bot commented Mar 27, 2026

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is missing the 'stat: QA assured' label
  • This PR is missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Mar 27, 2026

⚠️ No Changeset found

Latest commit: c86bce3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 27, 2026

CLA assistant check
All committers have signed the CLA.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2c793cce-a4d0-45e7-948f-7325dc9abe2d

📥 Commits

Reviewing files that changed from the base of the PR and between fa422c9 and 749540f.

📒 Files selected for processing (1)
  • apps/meteor/app/authentication/server/lib/restrictLoginAttempts.ts
📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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:

  • apps/meteor/app/authentication/server/lib/restrictLoginAttempts.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: smirk-dev
Repo: RocketChat/Rocket.Chat PR: 39625
File: apps/meteor/app/api/server/v1/push.ts:85-97
Timestamp: 2026-03-14T14:58:58.834Z
Learning: In RocketChat/Rocket.Chat, the `push.token` POST/DELETE endpoints in `apps/meteor/app/api/server/v1/push.ts` were already migrated to the chained router API pattern on `develop` prior to PR `#39625`. `cleanTokenResult` (which strips `authToken` and returns `PushTokenResult`) and `isPushTokenPOSTProps`/`isPushTokenDELETEProps` validators already exist on `develop`. PR `#39625` only migrates `push.get` and `push.info` to the chained pattern. Do not flag `cleanTokenResult` or `PushTokenResult` as newly introduced behavior-breaking changes when reviewing this PR.
📚 Learning: 2026-03-03T11:11:48.541Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39230
File: apps/meteor/app/api/server/v1/chat.ts:214-222
Timestamp: 2026-03-03T11:11:48.541Z
Learning: In apps/meteor/server/lib/moderation/reportMessage.ts, the reportMessage function validates that description is not empty or whitespace-only with `if (!description.trim())`. When migrating the chat.reportMessage endpoint to OpenAPI, adding minLength validation to the schema preserves this existing behavior.

Applied to files:

  • apps/meteor/app/authentication/server/lib/restrictLoginAttempts.ts
📚 Learning: 2025-10-07T15:08:37.419Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36942
File: apps/meteor/client/lib/e2ee/pbkdf2.ts:13-45
Timestamp: 2025-10-07T15:08:37.419Z
Learning: In apps/meteor/client/lib/e2ee/pbkdf2.ts, PBKDF2 iteration count validation is not enforced because the iterations parameter is outside the user's control and is system-managed.

Applied to files:

  • apps/meteor/app/authentication/server/lib/restrictLoginAttempts.ts
📚 Learning: 2026-03-20T13:51:23.302Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 39553
File: apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts:179-181
Timestamp: 2026-03-20T13:51:23.302Z
Learning: In `apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts`, the truthiness guards `...(integration.avatar && { avatar })`, `...(integration.emoji && { emoji })`, `...(integration.alias && { alias })`, and `...(integration.script && { script })` in the `$set` payload of `updateIncomingIntegration` are intentional. Empty-string values for these fields should NOT overwrite the stored value — only truthy values are persisted. Do not flag these as bugs preventing explicit clears.

Applied to files:

  • apps/meteor/app/authentication/server/lib/restrictLoginAttempts.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.

Applied to files:

  • apps/meteor/app/authentication/server/lib/restrictLoginAttempts.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.

Applied to files:

  • apps/meteor/app/authentication/server/lib/restrictLoginAttempts.ts
🔇 Additional comments (1)
apps/meteor/app/authentication/server/lib/restrictLoginAttempts.ts (1)

50-53: Whitelist normalization fix looks correct.

Trimming entries and filtering empty tokens makes IP whitelist matching robust against spaces/trailing commas and prevents false non-matches in whitelist.includes(ip).


Walkthrough

Updated authentication startup logic: the email template "from" value now uses only the configured From_Email (site name removed), and login-attempt IP whitelist parsing now trims entries and ignores empty values before matching.

Changes

Cohort / File(s) Summary
Email Template Configuration
apps/meteor/app/authentication/server/startup/index.js
Changed Accounts.emailTemplates.from from a formatted display-name (Site_Name <From_Email>) to use only From_Email. The watcher still listens for changes to Accounts_LoginExpiration, Site_Name, and From_Email.
Login Attempts / IP Whitelist
apps/meteor/app/authentication/server/lib/restrictLoginAttempts.ts
Updated isValidLoginAttemptByIp to split the whitelist string into entries, trim whitespace from each entry, and filter out empty values before checking whitelist.includes(ip).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

type: bug, area: authentication

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: updating the email template 'from' field to use only the email address instead of a formatted string with site name.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file


Since this is your first cubic review, here's how it works:

  • cubic automatically reviews your code and comments on bugs and improvements
  • Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
  • Add one-off context when rerunning by tagging @cubic-dev-ai with guidance or docs links (including llms.txt)
  • Ask questions if you need clarification on any suggestion

@namann5
Copy link
Copy Markdown
Author

namann5 commented Mar 27, 2026

Implemented a fix for the SMTP rejection affecting password recovery and email confirmation emails.

Root cause

Accounts.emailTemplates.from was using a formatted sender:

Site Name <From_Email>

Some SMTP providers, such as mail.ru, require the From header to exactly match the authenticated SMTP sender. Because of that, reset-password and verification emails were being rejected with:

550 Message was not accepted -- it contains invalid headers. More specifically, the 'From:' header must match the user you are sending mail from.

Fix

Changed the Accounts template sender to use the raw configured From_Email instead of the formatted Site Name <email> value.

File changed

  • apps/meteor/app/authentication/server/startup/index.js

Impact

This fixes:

  • forgot password emails
  • email verification / confirmation emails

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants