Skip to content

fix: trim whitespace from IP whitelist entries in failed-login protection#39918

Open
Atharva76-cpu wants to merge 3 commits into
RocketChat:developfrom
Atharva76-cpu:fix/ip-whitelist-trim-spaces
Open

fix: trim whitespace from IP whitelist entries in failed-login protection#39918
Atharva76-cpu wants to merge 3 commits into
RocketChat:developfrom
Atharva76-cpu:fix/ip-whitelist-trim-spaces

Conversation

@Atharva76-cpu
Copy link
Copy Markdown

@Atharva76-cpu Atharva76-cpu commented Mar 28, 2026

Proposed changes (including videos or screenshots)

The Block_Multiple_Failed_Logins_Ip_Whitelist setting splits IPs by
comma but never trimmed whitespace, causing valid whitelisted IPs to
still get blocked when entries had spaces after commas.

192.168.0.10 (with leading space) failed strict equality against
actual IP 192.168.0.10.

Fix: Added .map((entry) => entry.trim()).filter(Boolean) after
.split(',') in restrictLoginAttempts.ts

Before:

Screenshot 2026-03-28 125913

After:

Screenshot 2026-03-28 125927

Issue(s)

Closes #39915

Steps to test or reproduce

  1. Enable Block_Multiple_Failed_Logins_Enabled
  2. Enable Block_Multiple_Failed_Logins_By_Ip
  3. Set whitelist to 127.0.0.1, 192.168.0.10 (space after comma)
  4. Attempt repeated failed logins from 192.168.0.10
  5. Verify IP is correctly whitelisted and not blocked

Further comments

Minimal, focused fix with no side effects. .filter(Boolean) also
handles empty strings from trailing commas in the setting value.

Note: A changeset file will be added if required by maintainers.

Summary by CodeRabbit

  • Bug Fixes
    • Improved IP whitelist handling for login restrictions: whitelist entries now ignore surrounding whitespace and skip empty items, preventing valid IPs from being blocked due to formatting issues. This makes whitelist configuration more forgiving and reduces accidental login denials.

@dionisio-bot
Copy link
Copy Markdown
Contributor

dionisio-bot Bot commented Mar 28, 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 28, 2026

🦋 Changeset detected

Latest commit: 7617fe1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 41 packages
Name Type
@rocket.chat/meteor Patch
@rocket.chat/core-typings Patch
@rocket.chat/rest-typings Patch
@rocket.chat/uikit-playground Patch
@rocket.chat/api-client Patch
@rocket.chat/apps Patch
@rocket.chat/core-services Patch
@rocket.chat/cron Patch
@rocket.chat/ddp-client Patch
@rocket.chat/fuselage-ui-kit Patch
@rocket.chat/gazzodown Patch
@rocket.chat/http-router Patch
@rocket.chat/livechat Patch
@rocket.chat/model-typings Patch
@rocket.chat/ui-avatar Patch
@rocket.chat/ui-client Patch
@rocket.chat/ui-contexts Patch
@rocket.chat/ui-voip Patch
@rocket.chat/web-ui-registration Patch
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/abac Patch
@rocket.chat/federation-matrix Patch
@rocket.chat/license Patch
@rocket.chat/media-calls Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/models Patch
@rocket.chat/network-broker Patch
@rocket.chat/omni-core-ee Patch
@rocket.chat/mock-providers Patch
@rocket.chat/ui-video-conf Patch
@rocket.chat/instance-status Patch
@rocket.chat/omni-core Patch
@rocket.chat/server-fetch Patch

Not sure what this means? Click here to learn what changesets are.

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

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 28, 2026

CLA assistant check
All committers have signed the CLA.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 28, 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: 1a99882b-463a-4d1f-ba31-c84f28fa0892

📥 Commits

Reviewing files that changed from the base of the PR and between 3e1fe3d and f88b9fa.

📒 Files selected for processing (1)
  • .changeset/plenty-buttons-trade.md
✅ Files skipped from review due to trivial changes (1)
  • .changeset/plenty-buttons-trade.md

Walkthrough

Fixes whitelist parsing in failed-login protection: the IP whitelist string is split on commas, each entry is trimmed of whitespace, and empty entries are removed before checking membership against the incoming IP.

Changes

Cohort / File(s) Summary
IP Whitelist Processing Fix
apps/meteor/app/authentication/server/lib/restrictLoginAttempts.ts
Normalize Block_Multiple_Failed_Logins_Ip_Whitelist by splitting on commas, trimming each entry, and filtering out empty strings before performing includes(ip) checks.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the main change: trimming whitespace from IP whitelist entries in failed-login protection.
Linked Issues check ✅ Passed The PR implements the exact solution specified in issue #39915: normalizing whitelist entries by splitting, trimming whitespace, and filtering empty entries.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the IP whitelist trimming bug described in issue #39915; no unrelated modifications detected.
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

@Atharva76-cpu
Copy link
Copy Markdown
Author

I have signed the CLA

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.

IP whitelist for failed-login protection does not trim spaces, so valid whitelisted IPs can still get blocked

3 participants