Skip to content

fix(ldap): prevent excessive logging when Group BaseDN/Filter not configured #38246

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.

Sign up for GitHub

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

Closed
smirk-dev wants to merge 6 commits into from
Closed

fix(ldap): prevent excessive logging when Group BaseDN/Filter not configured #38246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b9ab047
gitignore updated to include yarn.lock and deno.lock
smirk-dev Dec 23, 2025
622d2f8
chore(deps): update @rocket.chat/ui-contexts to version 25.0.2 in yar…
smirk-dev Jan 6, 2026
67925e7
chore: add .github/copilot-instructions.md to .gitignore
smirk-dev Jan 6, 2026
e1064ea
Delete packages/apps-engine/deno-runtime/deno.lock
smirk-dev Jan 16, 2026
1d3c5fb
Delete yarn.lock
smirk-dev Jan 16, 2026
2018cdf
fix(ldap): prevent excessive logging when Group BaseDN/Filter not con…
smirk-dev Jan 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .changeset/quiet-llamas-dance.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
"@rocket.chat/meteor": patch
---

Fixes LDAP excessive logging issue when Group BaseDN or Group Filter are not configured

When LDAP role/channel sync is enabled with "Validate membership for each group" strategy, but the
Group BaseDN or Group Filter settings are not configured, the system was logging an error for every
user and every LDAP group on every sync interval. This could result in thousands of log entries per
second, filling up ~1GB of logs per day.

This fix:
1. Adds early validation in `syncUserRoles` and `syncUserChannels` to check if BaseDN and Filter are
configured before entering the iteration loop
2. Adds log throttling to the `isUserInGroup` function as a defense in depth measure
Comment on lines +12 to +15
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 18, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Hyphenate “defense-in-depth”.

Minor grammar: compound modifier should be hyphenated.

✏️ Proposed edit 
-2. Adds log throttling to the `isUserInGroup` function as a defense in depth measure
+2. Adds log throttling to the `isUserInGroup` function as a defense-in-depth measure
📝 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.

Suggested change
This fix:
1. Adds early validation in `syncUserRoles` and `syncUserChannels` to check if BaseDN and Filter are
configured before entering the iteration loop
2. Adds log throttling to the `isUserInGroup` function as a defense in depth measure
This fix:
1. Adds early validation in `syncUserRoles` and `syncUserChannels` to check if BaseDN and Filter are
configured before entering the iteration loop
2. Adds log throttling to the `isUserInGroup` function as a defense-in-depth measure
🧰 Tools
🪛 LanguageTool

[grammar] ~15-~15: Use a hyphen to join words.
Context: ...he isUserInGroup function as a defense in depth measure

(QB_NEW_EN_HYPHEN)

[grammar] ~15-~15: Use a hyphen to join words.
Context: ...isUserInGroup function as a defense in depth measure

(QB_NEW_EN_HYPHEN)

🤖 Prompt for AI Agents 
In @.changeset/quiet-llamas-dance.md around lines 12 - 15, The phrase "defense
in depth" in the changelog should be hyphenated as "defense-in-depth"; update
the sentence that describes log throttling for the isUserInGroup function to
read "Adds log throttling to the `isUserInGroup` function as a defense-in-depth
measure" and ensure the functions `syncUserRoles` and `syncUserChannels` and
`isUserInGroup` are referenced exactly as shown.

3 changes: 3 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ storybook-static
!.yarn/releases
!.yarn/sdks
!.yarn/versions
yarn.lock
deno.lock

.nvmrc
.idea/
Expand All @@ -62,3 +64,4 @@ storybook-static
development/tempo-data/

.env
.github/copilot-instructions.md
23 changes: 22 additions & 1 deletion apps/meteor/ee/server/lib/ldap/Manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ import { syncUserRoles } from '../syncUserRoles';
import { copyCustomFieldsLDAP } from './copyCustomFieldsLDAP';

export class LDAPEEManager extends LDAPManager {
// Track if we've already logged the missing config error to prevent log spam
private static hasLoggedMissingGroupConfig = false;

public static async sync(): Promise<void> {
if (settings.get('LDAP_Enable') !== true || settings.get('LDAP_Background_Sync') !== true) {
return;
Expand Down Expand Up @@ -250,7 +253,11 @@ export class LDAPEEManager extends LDAPManager {
groupName: string,
): Promise<boolean> {
if (!filter || !baseDN) {
logger.error('Please setup LDAP Group Filter and LDAP Group BaseDN in LDAP Settings.');
// Log only once to prevent excessive log spam (can be thousands of calls per sync)
if (!this.hasLoggedMissingGroupConfig) {
logger.error('Please setup LDAP Group Filter and LDAP Group BaseDN in LDAP Settings.');
this.hasLoggedMissingGroupConfig = true;
}
return false;
}
const searchOptions: ldapjs.SearchOptions = {
Expand Down Expand Up @@ -300,6 +307,13 @@ export class LDAPEEManager extends LDAPManager {
return;
}

if (searchStrategy === 'each_group' && (!syncUserRolesBaseDN || !syncUserRolesFilter)) {
logger.warn(
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Jan 18, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

P2: Missing group-config warning logs once per user sync, still causing log spam when BaseDN/Filter are unset

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/ee/server/lib/ldap/Manager.ts, line 311:

<comment>Missing group-config warning logs once per user sync, still causing log spam when BaseDN/Filter are unset</comment>

<file context>
@@ -300,6 +307,13 @@ export class LDAPEEManager extends LDAPManager {
 		}
 
+		if (searchStrategy === 'each_group' && (!syncUserRolesBaseDN || !syncUserRolesFilter)) {
+			logger.warn(
+				'LDAP Sync User Roles: "Group BaseDN" and "Group Filter" are required when using "Validate membership for each group" strategy. Skipping role sync.',
+			);
</file context>
Fix with Cubic

'LDAP Sync User Roles: "Group BaseDN" and "Group Filter" are required when using "Validate membership for each group" strategy. Skipping role sync.',
);
return;
}

const roles = (await Roles.find(
{},
{
Expand Down Expand Up @@ -395,6 +409,13 @@ export class LDAPEEManager extends LDAPManager {
return;
}

if (searchStrategy === 'each_group' && (!syncUserChannelsBaseDN || !syncUserChannelsFilter)) {
logger.warn(
'LDAP Sync User Channels: "Group BaseDN" and "Group Filter" are required when using "Validate membership for each group" strategy. Skipping channel sync.',
);
return;
}

const groupsToRoomsMap = this.parseJson(syncUserChannelsFieldMap);
if (!groupsToRoomsMap) {
logger.debug('missing group channel mapping');
Expand Down
115 changes: 0 additions & 115 deletions packages/apps-engine/deno-runtime/deno.lock
View file
Open in desktop

This file was deleted.

Loading