fix(userdb): throw clear error when ldap_uri is missing (#190)#349
fix(userdb): throw clear error when ldap_uri is missing (#190)#349JacobiusMakes wants to merge 3 commits intolinagora:devfrom
Conversation
When userdb_engine is set to "ldap" but ldap_uri is not configured, the constructor passed an empty string to ldapts.Client, which threw a cryptic "TypeError: is an invalid LDAP url (scope)" from deep inside the ldapjs URL parser. Now we validate ldap_uri at the top of the UserDBLDAP constructor and throw a descriptive error that tells the administrator exactly what to configure (the ldap_uri option) and offers the alternative of switching to sqlite/pg if LDAP is not needed. Closes linagora#190 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughFundamental Flaw
ChangesConstructor validation (lines 17–26): Added early guard rejecting
Single-pass URI trimming (line 29): Extract Credential masking for logging (lines 40–41): Regex replacement Eliminated empty-string fallback (line 46): Removed the pattern Log refactoring (lines 44, 113): Updated debug and error logs to use Data FlowURI validation is now front-loaded: if missing or blank, constructor throws before entering the Net EffectThree-line guard eliminates an entire class of silent failures at startup; coupled with credential masking, improves both observability and security in deployment troubleshooting. WalkthroughConstructor for Changes
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: bdd56c0d-0984-47a2-945f-8c5ae869c494
📒 Files selected for processing (1)
packages/matrix-identity-server/src/userdb/ldap.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx,json,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
npm run format:checkandnpm run format:fixfor code formatting checks
Files:
packages/matrix-identity-server/src/userdb/ldap.ts
packages/matrix-identity-server/src/**/*.{ts,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Routes are registered on
this.api.getandthis.api.postmaps and mounted by the parent server
Files:
packages/matrix-identity-server/src/userdb/ldap.ts
🧠 Learnings (18)
📓 Common learnings
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/matrix-identity-server/src/userdb/AGENTS.md:0-0
Timestamp: 2026-03-17T11:22:38.835Z
Learning: Code against the `UserDB` type abstraction, not against specific backend implementations (LDAP, PostgreSQL, SQLite, or empty)
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/matrix-identity-server/src/userdb/AGENTS.md:0-0
Timestamp: 2026-03-17T11:22:38.835Z
Learning: LDAP backend configuration uses the following keys: `ldap_uri`, `ldap_base`, `ldap_user`, `ldap_password`, `ldap_filter`
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/tom-server/src/user-info-api/AGENTS.md:0-0
Timestamp: 2026-03-17T11:24:38.144Z
Learning: Applies to packages/tom-server/src/user-info-api/user-info-api/index.ts : Apply `require-ldap.ts` middleware conditionally based on LDAP configuration — only enforce middleware when LDAP is enabled
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-23T09:08:39.061Z
Learning: Applies to packages/*/src/config.json : Use `userdb_engine` set to `'sqlite'`, `'pg'`, or `'ldap'`
📚 Learning: 2026-03-17T11:24:38.144Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/tom-server/src/user-info-api/AGENTS.md:0-0
Timestamp: 2026-03-17T11:24:38.144Z
Learning: Applies to packages/tom-server/src/user-info-api/user-info-api/index.ts : Apply `require-ldap.ts` middleware conditionally based on LDAP configuration — only enforce middleware when LDAP is enabled
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:21:40.136Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/matrix-identity-server/src/AGENTS.md:0-0
Timestamp: 2026-03-17T11:21:40.136Z
Learning: Applies to packages/matrix-identity-server/src/src/userdb/*.ts : Select userdb backend via `user_db` config key: `sqlite`, `pg`, `ldap`, or empty string
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:23:13.313Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/tom-server/AGENTS.md:0-0
Timestamp: 2026-03-17T11:23:13.313Z
Learning: Applies to packages/tom-server/src/config.ts : Enable LDAP middleware by setting LDAP configuration properties to integrate LDAP user directory
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:21:27.917Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/matrix-identity-server/AGENTS.md:0-0
Timestamp: 2026-03-17T11:21:27.917Z
Learning: Applies to packages/matrix-identity-server/src/index.ts : MatrixIdentityServer constructor takes (conf, confDesc?, db?) — conf must include server_name
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-23T09:08:39.061Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-23T09:08:39.061Z
Learning: Applies to packages/matrix-identity-server/src/db/**/*.{ts,js} : `MatrixIdentityServer` initializes two databases: `IdentityServerDb` (identity-server tables) and `UserDB` (user directory)
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:24:38.144Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/tom-server/src/user-info-api/AGENTS.md:0-0
Timestamp: 2026-03-17T11:24:38.144Z
Learning: Applies to packages/tom-server/src/user-info-api/user-info-api/**/*.ts : Use `UserInfoService` singleton to aggregate data from MatrixDB and UserDB (LDAP/SQL) when building enriched user profile responses
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:22:45.233Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/matrix-identity-server/src/utils/AGENTS.md:0-0
Timestamp: 2026-03-17T11:22:45.233Z
Learning: Applies to packages/matrix-identity-server/src/utils/**/utils/mailer.ts : SMTP configuration in `mailer.ts` should use keys: `smtp_server`, `smtp_port`, `smtp_user`, `smtp_password`, `smtp_tls`
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:21:27.917Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/matrix-identity-server/AGENTS.md:0-0
Timestamp: 2026-03-17T11:21:27.917Z
Learning: Applies to packages/matrix-identity-server/src/validate/**/*.ts : Email sending requires SMTP config keys: smtp_server, smtp_port, smtp_user, smtp_password
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:24:38.144Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/tom-server/src/user-info-api/AGENTS.md:0-0
Timestamp: 2026-03-17T11:24:38.144Z
Learning: Applies to packages/tom-server/src/user-info-api/user-info-api/**/*.ts : Store visibility settings in the identity server DB (not MatrixDB)
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:22:38.835Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/matrix-identity-server/src/userdb/AGENTS.md:0-0
Timestamp: 2026-03-17T11:22:38.835Z
Learning: Applies to packages/matrix-identity-server/src/userdb/userdb/**/*.ts : All backends in the userdb module must implement the same interface with methods `get()`, `getAll()`, and `match()` for querying user information
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:23:13.313Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/tom-server/AGENTS.md:0-0
Timestamp: 2026-03-17T11:23:13.313Z
Learning: Applies to packages/tom-server/**/*.{ts,tsx} : TwakeServer constructor requires `conf` parameter with mandatory `server_name` and `base_url` properties
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-23T09:08:39.061Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-23T09:08:39.061Z
Learning: Applies to packages/*/src/server.{ts,js} : Config is loaded via `twake/config-parser` from (in priority order): `/etc/twake/server.conf`, `TWAKE_SERVER_CONF` env var, or constructor argument
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:21:16.677Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/logger/AGENTS.md:0-0
Timestamp: 2026-03-17T11:21:16.677Z
Learning: Applies to packages/logger/**/*.{ts,tsx} : Use `getLogger(conf?, confDesc?)` as the main factory function to obtain a Winston `Logger` instance typed as `TwakeLogger`
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-23T09:08:39.061Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-23T09:08:39.061Z
Learning: Applies to packages/*/src/config.json : Use `userdb_engine` set to `'sqlite'`, `'pg'`, or `'ldap'`
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:23:51.582Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/tom-server/src/identity-server/AGENTS.md:0-0
Timestamp: 2026-03-17T11:23:51.582Z
Learning: Applies to packages/tom-server/src/identity-server/identity-server/index.ts : TwakeIdentityServer must add twakeDbCollections to extend the identity server database schema
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:21:16.677Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/logger/AGENTS.md:0-0
Timestamp: 2026-03-17T11:21:16.677Z
Learning: Applies to packages/logger/**/*.{ts,tsx} : Load logger configuration from environment variables or config object via `getLogger()` using twake/config-parser
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:22:38.835Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/matrix-identity-server/src/userdb/AGENTS.md:0-0
Timestamp: 2026-03-17T11:22:38.835Z
Learning: LDAP backend configuration uses the following keys: `ldap_uri`, `ldap_base`, `ldap_user`, `ldap_password`, `ldap_filter`
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
🪛 Biome (2.4.9)
packages/matrix-identity-server/src/userdb/ldap.ts
[error] 18-18: Using == may be unsafe if you are relying on type coercion.
(lint/suspicious/noDoubleEquals)
🔇 Additional comments (1)
packages/matrix-identity-server/src/userdb/ldap.ts (1)
44-44: Good change: no empty-string fallback when creating LDAP client.Passing the configured URI directly at Line 44 is the right fail-fast behavior once constructor validation is in place.
Address CodeRabbit review: the debug log was printing the full ldap_uri value, which can embed credentials (ldap://user:pass@host). Replace with a boolean confirmation that the URI is configured.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/matrix-identity-server/src/userdb/ldap.ts (1)
107-111:⚠️ Potential issue | 🟠 MajorYou plugged one leak but left another — URI still logged in error handler.
Lines 108-110 dump the full
conf.ldap_uriwhen the connection test fails. If someone embeds credentials in their URI, those end up in logs right here. This directly contradicts the commit message "stop logging raw LDAP URI to prevent credential leakage."Apply the same treatment as the debug log:
🔐 Proposed fix
this.logger.error( - `[UserDBLDAP][constructor] Configuration - URI: ${ - conf.ldap_uri ?? 'undefined' - }, Base: ${this.base}` + `[UserDBLDAP][constructor] Configuration - URI: [configured], Base: ${this.base}` )
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6935f98b-5136-472e-8d1f-01d33ffe9e95
📒 Files selected for processing (1)
packages/matrix-identity-server/src/userdb/ldap.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx,json,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
npm run format:checkandnpm run format:fixfor code formatting checks
Files:
packages/matrix-identity-server/src/userdb/ldap.ts
packages/matrix-identity-server/src/**/*.{ts,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Routes are registered on
this.api.getandthis.api.postmaps and mounted by the parent server
Files:
packages/matrix-identity-server/src/userdb/ldap.ts
🧠 Learnings (22)
📓 Common learnings
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/matrix-identity-server/src/userdb/AGENTS.md:0-0
Timestamp: 2026-03-17T11:22:38.835Z
Learning: Code against the `UserDB` type abstraction, not against specific backend implementations (LDAP, PostgreSQL, SQLite, or empty)
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/matrix-identity-server/src/userdb/AGENTS.md:0-0
Timestamp: 2026-03-17T11:22:38.835Z
Learning: LDAP backend configuration uses the following keys: `ldap_uri`, `ldap_base`, `ldap_user`, `ldap_password`, `ldap_filter`
📚 Learning: 2026-03-17T11:24:38.144Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/tom-server/src/user-info-api/AGENTS.md:0-0
Timestamp: 2026-03-17T11:24:38.144Z
Learning: Applies to packages/tom-server/src/user-info-api/user-info-api/index.ts : Apply `require-ldap.ts` middleware conditionally based on LDAP configuration — only enforce middleware when LDAP is enabled
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:21:27.917Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/matrix-identity-server/AGENTS.md:0-0
Timestamp: 2026-03-17T11:21:27.917Z
Learning: Applies to packages/matrix-identity-server/src/index.ts : MatrixIdentityServer constructor takes (conf, confDesc?, db?) — conf must include server_name
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:21:40.136Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/matrix-identity-server/src/AGENTS.md:0-0
Timestamp: 2026-03-17T11:21:40.136Z
Learning: Applies to packages/matrix-identity-server/src/src/userdb/*.ts : Select userdb backend via `user_db` config key: `sqlite`, `pg`, `ldap`, or empty string
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:24:38.144Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/tom-server/src/user-info-api/AGENTS.md:0-0
Timestamp: 2026-03-17T11:24:38.144Z
Learning: Applies to packages/tom-server/src/user-info-api/user-info-api/**/*.ts : Use `UserInfoService` singleton to aggregate data from MatrixDB and UserDB (LDAP/SQL) when building enriched user profile responses
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-23T09:08:39.061Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-23T09:08:39.061Z
Learning: Applies to packages/matrix-identity-server/src/db/**/*.{ts,js} : `MatrixIdentityServer` initializes two databases: `IdentityServerDb` (identity-server tables) and `UserDB` (user directory)
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:23:13.313Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/tom-server/AGENTS.md:0-0
Timestamp: 2026-03-17T11:23:13.313Z
Learning: Applies to packages/tom-server/src/config.ts : Enable LDAP middleware by setting LDAP configuration properties to integrate LDAP user directory
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:22:45.233Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/matrix-identity-server/src/utils/AGENTS.md:0-0
Timestamp: 2026-03-17T11:22:45.233Z
Learning: Applies to packages/matrix-identity-server/src/utils/**/utils/mailer.ts : SMTP configuration in `mailer.ts` should use keys: `smtp_server`, `smtp_port`, `smtp_user`, `smtp_password`, `smtp_tls`
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:22:38.835Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/matrix-identity-server/src/userdb/AGENTS.md:0-0
Timestamp: 2026-03-17T11:22:38.835Z
Learning: Applies to packages/matrix-identity-server/src/userdb/userdb/**/*.ts : All backends in the userdb module must implement the same interface with methods `get()`, `getAll()`, and `match()` for querying user information
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:23:51.582Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/tom-server/src/identity-server/AGENTS.md:0-0
Timestamp: 2026-03-17T11:23:51.582Z
Learning: Applies to packages/tom-server/src/identity-server/identity-server/index.ts : TwakeIdentityServer should extend MatrixIdentityServer with Twake-specific user lookup and search capabilities
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:22:24.131Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/matrix-identity-server/src/matrixDb/AGENTS.md:0-0
Timestamp: 2026-03-17T11:22:24.131Z
Learning: Applies to packages/matrix-identity-server/src/matrixDb/matrixDb/index.ts : Reference configuration keys: matrix_database_engine (pg/sqlite), matrix_database_host, matrix_database_name for database connections
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:22:06.022Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/matrix-identity-server/src/db/AGENTS.md:0-0
Timestamp: 2026-03-17T11:22:06.022Z
Learning: Applies to packages/matrix-identity-server/src/db/db/**/*.ts : All SQL queries must use parameterized statements — never string-interpolate user input into SQL
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-12T18:28:25.652Z
Learnt from: pm-McFly
Repo: linagora/ToM-server PR: 333
File: packages/tom-server/src/addressbook-api/controllers/index.ts:75-77
Timestamp: 2026-03-12T18:28:25.652Z
Learning: In `packages/tom-server/src/addressbook-api/controllers/index.ts`, the `uid` field from `UserInformation` is intentionally included in the enriched contact response payload (via the `...userInfo` spread). It is a legacy field that was sent before this PR and is planned to be deprecated in a future PR. Do not flag its presence in the response as a bug or security concern during reviews.
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:22:49.465Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/matrix-identity-server/src/validate/AGENTS.md:0-0
Timestamp: 2026-03-17T11:22:49.465Z
Learning: Applies to packages/matrix-identity-server/src/validate/validate/**/*.{ts,js} : Mark validation sessions as validated after successful token submission to enable 3PID binding
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-18T21:38:15.357Z
Learnt from: pm-McFly
Repo: linagora/ToM-server PR: 341
File: biome.json:94-94
Timestamp: 2026-03-18T21:38:15.357Z
Learning: In linagora/ToM-server, the `noEnum: "warn"` rule in `biome.json` is intentional. The linting pipeline is expected to fail temporarily while existing `enum` declarations (e.g., `ETransportType`, `ProfileField`, `ProfileVisibility`, `SMS_FOOTERS`, `SynapseAdminRetryMode`, `ConnectionState`) are progressively replaced with string unions and runtime type guards, as prescribed in CODING_STYLE.md. This is a deliberate enforcement strategy, not an oversight.
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:23:13.313Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/tom-server/AGENTS.md:0-0
Timestamp: 2026-03-17T11:23:13.313Z
Learning: Applies to packages/tom-server/**/*.{ts,tsx} : TwakeServer constructor requires `conf` parameter with mandatory `server_name` and `base_url` properties
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-23T09:08:39.061Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-23T09:08:39.061Z
Learning: Applies to packages/*/src/server.{ts,js} : Config is loaded via `twake/config-parser` from (in priority order): `/etc/twake/server.conf`, `TWAKE_SERVER_CONF` env var, or constructor argument
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:21:16.677Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/logger/AGENTS.md:0-0
Timestamp: 2026-03-17T11:21:16.677Z
Learning: Applies to packages/logger/**/*.{ts,tsx} : Use `getLogger(conf?, confDesc?)` as the main factory function to obtain a Winston `Logger` instance typed as `TwakeLogger`
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-23T09:08:39.061Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-23T09:08:39.061Z
Learning: Applies to packages/*/src/config.json : Use `userdb_engine` set to `'sqlite'`, `'pg'`, or `'ldap'`
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:23:51.582Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/tom-server/src/identity-server/AGENTS.md:0-0
Timestamp: 2026-03-17T11:23:51.582Z
Learning: Applies to packages/tom-server/src/identity-server/identity-server/index.ts : TwakeIdentityServer must add twakeDbCollections to extend the identity server database schema
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:21:16.677Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/logger/AGENTS.md:0-0
Timestamp: 2026-03-17T11:21:16.677Z
Learning: Applies to packages/logger/**/*.{ts,tsx} : Load logger configuration from environment variables or config object via `getLogger()` using twake/config-parser
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:22:38.835Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/matrix-identity-server/src/userdb/AGENTS.md:0-0
Timestamp: 2026-03-17T11:22:38.835Z
Learning: LDAP backend configuration uses the following keys: `ldap_uri`, `ldap_base`, `ldap_user`, `ldap_password`, `ldap_filter`
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
🪛 Biome (2.4.9)
packages/matrix-identity-server/src/userdb/ldap.ts
[error] 18-18: Using == may be unsafe if you are relying on type coercion.
(lint/suspicious/noDoubleEquals)
🔇 Additional comments (3)
packages/matrix-identity-server/src/userdb/ldap.ts (3)
18-26: Coercive== nulland whitespace-only URIs still slip through.The past review nailed this:
== nulltriggers Biome lint, and" ".lengthis 3, so whitespace-only URIs pass validation then explode in ldapts with the same opaque error you're trying to prevent.Trim the input and use strict equality:
- if (conf.ldap_uri == null || conf.ldap_uri.length === 0) { + const ldapUri = typeof conf.ldap_uri === 'string' ? conf.ldap_uri.trim() : '' + if (ldapUri.length === 0) {Then use
ldapUrithroughout the constructor instead ofconf.ldap_uri.
36-38: Good. Credentials stay out of logs now.Solid fix — URIs can embed
user:pass@hostand you've stopped that from hemorrhaging into debug output.
44-44: Direct assignment is correct after validation.The early-exit check guarantees
conf.ldap_uriis non-empty here, so dropping the fallback to''is the right move. Clean.
pm-McFly
left a comment
pm-McFly
left a comment
There was a problem hiding this comment.
Hi @JacobiusMakes
Thanks for suggesting this fix!
As you might have seen we are conducting rather important changes into our code base to make it more stable and hopefully cleaner in the end as well.
Your help in that direction is deeply appreciated)
pm-McFly
added
bug
Address review feedback: restore the debug log of the LDAP URI (useful for remote-deployment troubleshooting) but mask any embedded credentials before logging. Also add .trim() on the URI value and use a local `ldapUri` variable for the trimmed value throughout. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Good point — restored the debug log but now masking any credentials in the URI before logging. Best of both worlds: useful for remote troubleshooting without leaking sensitive data. |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6d2e7694-4ee6-4865-a02c-05b9ca901c54
📒 Files selected for processing (1)
packages/matrix-identity-server/src/userdb/ldap.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx,json,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
npm run format:checkandnpm run format:fixfor code formatting checks
Files:
packages/matrix-identity-server/src/userdb/ldap.ts
packages/matrix-identity-server/src/**/*.{ts,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Routes are registered on
this.api.getandthis.api.postmaps and mounted by the parent server
Files:
packages/matrix-identity-server/src/userdb/ldap.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (CODING_STYLE.md)
**/*.{js,ts,jsx,tsx}: Use 2 spaces for indentation, not 4, tabs, or other amounts. Enforce with a formatter.
Opening braces must go on the same line. Never place opening braces on a new line.
Use trailing commas in multi-line structures (arrays, objects, function parameters) to minimize diff noise.
Semicolons are required on all statements. Do not rely on Automatic Semicolon Insertion (ASI).
Enforce a hard line length limit of 120 characters. Break overly complex expressions into named sub-expressions.
Use camelCase for variable and function names.
Use PascalCase for types, interfaces, classes, and enums.
Use SCREAMING_SNAKE_CASE only for module-level primitives that are truly constant and never change. Do not use for local bindings.
Boolean variables must read as a question using prefixes likeis,has,can(e.g.,isLoading,hasPermission,canRetry). Never use bare noun forms.
Do not abbreviate variable or function names except for:i,jin tight loops;efor event parameters;errfor errors;ctxfor context;req/resin HTTP handlers.
Each function must do exactly one job. Do not write functions that combine multiple concerns (e.g., avoidfetchAndTransformUser). Extract compound operations into separate functions.
Functions must have a maximum of 5 parameters. For more than 5 parameters, group related data into a typed options object.
Keep functions short, with a reasonable ceiling of 25–40 lines. Functions should fit on one screen without scrolling.
Every function must return a meaningful value.voidreturn types are forbidden. UseActionResultfor functions with no natural data return.
Recursion must be tail-call or converted to an iterative loop. Non-tail-recursive functions are forbidden in production code because JavaScript engines do not reliably optimize tail calls.
Maximum nesting depth is 2 levels (level 0 is function body, level 1 is a block inside it, level 2 is a block inside that). Extract sub-problems into named functi...
Files:
packages/matrix-identity-server/src/userdb/ldap.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CODING_STYLE.md)
**/*.{ts,tsx}: Return types must be explicitly annotated on all non-trivial functions in TypeScript. Inference alone is not a contract.
Theanytype is forbidden without exception in TypeScript. Use a proper type, discriminated union,unknownwith a guard, or a generic instead.
Double casts viaas unknown as Tare forbidden. If the type model is inconsistent, fix the model instead.
Useunknownoveranyfor data from external sources (HTTP responses,JSON.parse, event payloads, database rows). Write a type guard to validate the data.
Prefertypefor unions and intersections; preferinterfacefor object shapes. Keep intent readable through consistent use.
Avoid TypeScriptenum. Use string union types for fully internal values. For values from external sources, provide a validation helper that narrows conversion from raw strings.
In TypeScript, caught values have typeunknown, notError. Useinstanceof Errorchecks before accessing Error-specific properties.
Do not use@ts-ignoreor@ts-expect-errorwithout an explanatory comment stating the reason and removal condition.Do not introduce new
anytypes in TypeScript — existinganywarnings are tech debt, but new ones are blockers
Files:
packages/matrix-identity-server/src/userdb/ldap.ts
🧠 Learnings (25)
📓 Common learnings
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/matrix-identity-server/src/userdb/AGENTS.md:0-0
Timestamp: 2026-03-17T11:22:38.835Z
Learning: Code against the `UserDB` type abstraction, not against specific backend implementations (LDAP, PostgreSQL, SQLite, or empty)
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/matrix-identity-server/src/userdb/AGENTS.md:0-0
Timestamp: 2026-03-17T11:22:38.835Z
Learning: LDAP backend configuration uses the following keys: `ldap_uri`, `ldap_base`, `ldap_user`, `ldap_password`, `ldap_filter`
📚 Learning: 2026-03-17T11:24:38.144Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/tom-server/src/user-info-api/AGENTS.md:0-0
Timestamp: 2026-03-17T11:24:38.144Z
Learning: Applies to packages/tom-server/src/user-info-api/user-info-api/index.ts : Apply `require-ldap.ts` middleware conditionally based on LDAP configuration — only enforce middleware when LDAP is enabled
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:21:40.136Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/matrix-identity-server/src/AGENTS.md:0-0
Timestamp: 2026-03-17T11:21:40.136Z
Learning: Applies to packages/matrix-identity-server/src/src/userdb/*.ts : Select userdb backend via `user_db` config key: `sqlite`, `pg`, `ldap`, or empty string
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:21:27.917Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/matrix-identity-server/AGENTS.md:0-0
Timestamp: 2026-03-17T11:21:27.917Z
Learning: Applies to packages/matrix-identity-server/src/index.ts : MatrixIdentityServer constructor takes (conf, confDesc?, db?) — conf must include server_name
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:24:38.144Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/tom-server/src/user-info-api/AGENTS.md:0-0
Timestamp: 2026-03-17T11:24:38.144Z
Learning: Applies to packages/tom-server/src/user-info-api/user-info-api/**/*.ts : Use `UserInfoService` singleton to aggregate data from MatrixDB and UserDB (LDAP/SQL) when building enriched user profile responses
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-23T09:08:39.061Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-23T09:08:39.061Z
Learning: Applies to packages/matrix-identity-server/src/db/**/*.{ts,js} : `MatrixIdentityServer` initializes two databases: `IdentityServerDb` (identity-server tables) and `UserDB` (user directory)
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:23:13.313Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/tom-server/AGENTS.md:0-0
Timestamp: 2026-03-17T11:23:13.313Z
Learning: Applies to packages/tom-server/src/config.ts : Enable LDAP middleware by setting LDAP configuration properties to integrate LDAP user directory
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:22:45.233Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/matrix-identity-server/src/utils/AGENTS.md:0-0
Timestamp: 2026-03-17T11:22:45.233Z
Learning: Applies to packages/matrix-identity-server/src/utils/**/utils/mailer.ts : SMTP configuration in `mailer.ts` should use keys: `smtp_server`, `smtp_port`, `smtp_user`, `smtp_password`, `smtp_tls`
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:24:38.144Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/tom-server/src/user-info-api/AGENTS.md:0-0
Timestamp: 2026-03-17T11:24:38.144Z
Learning: Applies to packages/tom-server/src/user-info-api/user-info-api/**/*.ts : Store visibility settings in the identity server DB (not MatrixDB)
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:22:24.131Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/matrix-identity-server/src/matrixDb/AGENTS.md:0-0
Timestamp: 2026-03-17T11:22:24.131Z
Learning: Applies to packages/matrix-identity-server/src/matrixDb/matrixDb/index.ts : Reference configuration keys: matrix_database_engine (pg/sqlite), matrix_database_host, matrix_database_name for database connections
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-12T18:28:25.652Z
Learnt from: pm-McFly
Repo: linagora/ToM-server PR: 333
File: packages/tom-server/src/addressbook-api/controllers/index.ts:75-77
Timestamp: 2026-03-12T18:28:25.652Z
Learning: In `packages/tom-server/src/addressbook-api/controllers/index.ts`, the `uid` field from `UserInformation` is intentionally included in the enriched contact response payload (via the `...userInfo` spread). It is a legacy field that was sent before this PR and is planned to be deprecated in a future PR. Do not flag its presence in the response as a bug or security concern during reviews.
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:22:06.022Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/matrix-identity-server/src/db/AGENTS.md:0-0
Timestamp: 2026-03-17T11:22:06.022Z
Learning: Applies to packages/matrix-identity-server/src/db/db/**/*.ts : All SQL queries must use parameterized statements — never string-interpolate user input into SQL
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-18T21:38:15.357Z
Learnt from: pm-McFly
Repo: linagora/ToM-server PR: 341
File: biome.json:94-94
Timestamp: 2026-03-18T21:38:15.357Z
Learning: In linagora/ToM-server, the `noEnum: "warn"` rule in `biome.json` is intentional. The linting pipeline is expected to fail temporarily while existing `enum` declarations (e.g., `ETransportType`, `ProfileField`, `ProfileVisibility`, `SMS_FOOTERS`, `SynapseAdminRetryMode`, `ConnectionState`) are progressively replaced with string unions and runtime type guards, as prescribed in CODING_STYLE.md. This is a deliberate enforcement strategy, not an oversight.
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:22:49.465Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/matrix-identity-server/src/validate/AGENTS.md:0-0
Timestamp: 2026-03-17T11:22:49.465Z
Learning: Applies to packages/matrix-identity-server/src/validate/validate/**/*.{ts,js} : Mark validation sessions as validated after successful token submission to enable 3PID binding
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:22:38.835Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/matrix-identity-server/src/userdb/AGENTS.md:0-0
Timestamp: 2026-03-17T11:22:38.835Z
Learning: Applies to packages/matrix-identity-server/src/userdb/userdb/**/*.ts : All backends in the userdb module must implement the same interface with methods `get()`, `getAll()`, and `match()` for querying user information
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-27T16:21:10.919Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: CODING_STYLE.md:0-0
Timestamp: 2026-03-27T16:21:10.919Z
Learning: Applies to **/*.{js,ts,jsx,tsx} : Use strict equality (`===`) instead of loose equality (`==`). Do not rely on JavaScript's implicit type coercion.
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:21:03.013Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/federated-identity-service/AGENTS.md:0-0
Timestamp: 2026-03-17T11:21:03.013Z
Learning: Applies to packages/federated-identity-service/src/**/*.ts : Implement authenticate() method override to validate requests come from trusted Matrix servers
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-23T09:08:39.061Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-23T09:08:39.061Z
Learning: Applies to packages/*/src/server.{ts,js} : Config is loaded via `twake/config-parser` from (in priority order): `/etc/twake/server.conf`, `TWAKE_SERVER_CONF` env var, or constructor argument
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:23:13.313Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/tom-server/AGENTS.md:0-0
Timestamp: 2026-03-17T11:23:13.313Z
Learning: Applies to packages/tom-server/**/*.{ts,tsx} : TwakeServer constructor requires `conf` parameter with mandatory `server_name` and `base_url` properties
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:21:16.677Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/logger/AGENTS.md:0-0
Timestamp: 2026-03-17T11:21:16.677Z
Learning: Applies to packages/logger/**/*.{ts,tsx} : Use `getLogger(conf?, confDesc?)` as the main factory function to obtain a Winston `Logger` instance typed as `TwakeLogger`
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:23:51.582Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/tom-server/src/identity-server/AGENTS.md:0-0
Timestamp: 2026-03-17T11:23:51.582Z
Learning: Applies to packages/tom-server/src/identity-server/identity-server/index.ts : TwakeIdentityServer must add twakeDbCollections to extend the identity server database schema
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:23:51.582Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/tom-server/src/identity-server/AGENTS.md:0-0
Timestamp: 2026-03-17T11:23:51.582Z
Learning: Applies to packages/tom-server/src/identity-server/identity-server/index.ts : TwakeIdentityServer should extend MatrixIdentityServer with Twake-specific user lookup and search capabilities
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:21:16.677Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/logger/AGENTS.md:0-0
Timestamp: 2026-03-17T11:21:16.677Z
Learning: Applies to packages/logger/**/*.{ts,tsx} : Load logger configuration from environment variables or config object via `getLogger()` using twake/config-parser
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-23T09:08:39.061Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-23T09:08:39.061Z
Learning: Applies to packages/*/src/config.json : Use `userdb_engine` set to `'sqlite'`, `'pg'`, or `'ldap'`
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
📚 Learning: 2026-03-17T11:22:38.835Z
Learnt from: CR
Repo: linagora/ToM-server PR: 0
File: packages/matrix-identity-server/src/userdb/AGENTS.md:0-0
Timestamp: 2026-03-17T11:22:38.835Z
Learning: LDAP backend configuration uses the following keys: `ldap_uri`, `ldap_base`, `ldap_user`, `ldap_password`, `ldap_filter`
Applied to files:
packages/matrix-identity-server/src/userdb/ldap.ts
🪛 Biome (2.4.9)
packages/matrix-identity-server/src/userdb/ldap.ts
[error] 18-18: Using == may be unsafe if you are relying on type coercion.
(lint/suspicious/noDoubleEquals)
🔇 Additional comments (3)
packages/matrix-identity-server/src/userdb/ldap.ts (3)
38-42: Clean credential masking — nicely done.The regex handles the standard
user:pass@hostpattern correctly without overcomplicating things. Greedy[^@]+ensures you capture the full password up to the@. The explanatory comment is exactly what comments should be: explaining why, not what.This addresses the previous security concern properly.
47-50: Correct — no more empty-string fallback nonsense.Passing
ldapUridirectly after validation means the constructor either has a valid URI or has already exploded with a useful error. The old fallback to empty string was the root cause of the cryptic ldapjs parser errors. This is the right fix.
112-114: Good — error diagnostics without credential leakage.Using
maskedUriin the failure path gives operators what they need for troubleshooting (the host/port) without spilling secrets into logs. Exactly what was requested in the previous review.
|
|
||
| if (conf.ldap_uri == null || conf.ldap_uri.trim().length === 0) { | ||
| const msg = | ||
| '[UserDBLDAP] userdb_engine is set to "ldap" but no LDAP URI is configured. ' + | ||
| 'Set the "ldap_uri" option in your server configuration ' + | ||
| '(e.g. "ldap://localhost:389" or "ldaps://ldap.example.com"). ' + | ||
| 'If you do not need LDAP, change userdb_engine to "sqlite" or "pg".' | ||
| this.logger.error(msg) | ||
| throw new Error(msg) | ||
| } | ||
|
|
||
| const ldapUri = conf.ldap_uri.trim() |
There was a problem hiding this comment.
Coercive == null violates strict equality rule; also trims twice needlessly.
Biome rightfully screams about line 18. The coding guidelines explicitly forbid == — use strict checks only. You're also calling .trim() twice: once in the condition, once when assigning ldapUri. Wasteful and harder to reason about.
The fix from the previous review was elegant — apply it:
🔧 Proposed fix
- if (conf.ldap_uri == null || conf.ldap_uri.trim().length === 0) {
+ const ldapUri =
+ typeof conf.ldap_uri === 'string' ? conf.ldap_uri.trim() : ''
+ if (ldapUri.length === 0) {
const msg =
'[UserDBLDAP] userdb_engine is set to "ldap" but no LDAP URI is configured. ' +
'Set the "ldap_uri" option in your server configuration ' +
'(e.g. "ldap://localhost:389" or "ldaps://ldap.example.com"). ' +
'If you do not need LDAP, change userdb_engine to "sqlite" or "pg".'
this.logger.error(msg)
throw new Error(msg)
}
-
- const ldapUri = conf.ldap_uri.trim()This defines ldapUri once, handles undefined/null/non-string cases without coercion, and eliminates the redundant trim. As per coding guidelines: "Use strict equality (===) instead of loose equality (==). Do not rely on JavaScript's implicit type coercion."
🧰 Tools
🪛 Biome (2.4.9)
[error] 18-18: Using == may be unsafe if you are relying on type coercion.
(lint/suspicious/noDoubleEquals)
There was a problem hiding this comment.
@JacobiusMakes This point is worth addressing.
Declare, then assign or declare and assign, then use is overall a good approach.
The suggested fix sounds right to me, do not hesitate to suggest a better implementation if any ;)
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected -t check -- --max-diagnostics none |
❌ Failed | 9s | View ↗ |
nx affected -t test |
✅ Succeeded | 2m 38s | View ↗ |
nx affected -t build |
✅ Succeeded | 13s | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-03-30 15:43:53 UTC
| ) | ||
| // Log the raw URI for remote-deployment troubleshooting, but mask | ||
| // any embedded credentials (e.g. ldap://user:pass@host → ldap://user:***@host). | ||
| const maskedUri = ldapUri.replace(/(\/\/)([^:]+):([^@]+)@/, '$1$2:***@') |
Check failure
Code scanning / CodeQL
Polynomial regular expression used on uncontrolled data High
pm-McFly
left a comment
pm-McFly
left a comment
There was a problem hiding this comment.
Once the ldapURI var declaration/use is done, this is good to go!
Please also rebase dev (our default branch) on yours so we can proceed with final testings and rebase/merge operation
|
|
||
| if (conf.ldap_uri == null || conf.ldap_uri.trim().length === 0) { | ||
| const msg = | ||
| '[UserDBLDAP] userdb_engine is set to "ldap" but no LDAP URI is configured. ' + | ||
| 'Set the "ldap_uri" option in your server configuration ' + | ||
| '(e.g. "ldap://localhost:389" or "ldaps://ldap.example.com"). ' + | ||
| 'If you do not need LDAP, change userdb_engine to "sqlite" or "pg".' | ||
| this.logger.error(msg) | ||
| throw new Error(msg) | ||
| } | ||
|
|
||
| const ldapUri = conf.ldap_uri.trim() |
There was a problem hiding this comment.
@JacobiusMakes This point is worth addressing.
Declare, then assign or declare and assign, then use is overall a good approach.
The suggested fix sounds right to me, do not hesitate to suggest a better implementation if any ;)
3 participants
Summary
Fixes #190. When
userdb_engineis set to"ldap"butldap_uriis not configured, the server crashed with a crypticTypeError: is an invalid LDAP url (scope)from deep inside the ldapjs URL parser.Root cause
The
UserDBLDAPconstructor passed an empty string tonew ldapts.Client({ url: '' })whenconf.ldap_uriwasundefined. The ldapts library then tried to parse""as an LDAP URL, which failed with an opaque error that gave no hint about what configuration was wrong.Fix
Validate
ldap_uriat the top of theUserDBLDAPconstructor. If it is missing or empty, throw a descriptive error that tells the administrator:ldap_urioption with an example)sqliteorpg)Also removes the fallback to empty string on line 33, since the constructor now guarantees
conf.ldap_uriis a non-empty string before reaching the client creation code.Test plan
userdb_engine: "ldap"and noldap_uri— should see clear error message instead of crashuserdb_engine: "ldap"and a validldap_uri— should work as beforeuserdb_engine: "sqlite"— unaffected🤖 Generated with Claude Code