From e64eee61eb538135b72c546a2daca5a6b6d83621 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 18 Nov 2025 19:05:55 +0000 Subject: [PATCH 1/7] Initial plan From 51c62dab19baf034cd333af2d690bf02067a76fc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 18 Nov 2025 19:11:45 +0000 Subject: [PATCH 2/7] Add REQUIRE_AUTH_FOR_SEARCH configuration option Co-authored-by: anishapant21 <44058515+anishapant21@users.noreply.github.com> --- npm/src/LdapEngine.js | 11 +++++++++++ server/.env.example | 5 +++++ server/config/configurationLoader.js | 1 + server/serverMain.js | 3 ++- 4 files changed, 19 insertions(+), 1 deletion(-) diff --git a/npm/src/LdapEngine.js b/npm/src/LdapEngine.js index fbf7d6d..3fc597c 100644 --- a/npm/src/LdapEngine.js +++ b/npm/src/LdapEngine.js @@ -15,6 +15,7 @@ class LdapEngine extends EventEmitter { port: options.port || 389, certificate: options.certificate || null, key: options.key || null, + requireAuthForSearch: options.requireAuthForSearch || false, ...options }; @@ -197,6 +198,16 @@ class LdapEngine extends EventEmitter { const filterStr = req.filter.toString(); this.logger.debug(`LDAP Search - Filter: ${filterStr}, Attributes: ${req.attributes}`); + // Check if authentication is required for search operations + if (this.config.requireAuthForSearch) { + const isAnonymous = !req.connection.ldap.bindDN || req.connection.ldap.bindDN.toString() === ''; + if (isAnonymous) { + this.logger.debug('Anonymous search rejected - authentication required'); + const error = new ldap.InsufficientAccessRightsError('Authentication required for search operations'); + return next(error); + } + } + let entryCount = 0; const startTime = Date.now(); diff --git a/server/.env.example b/server/.env.example index 9433023..7b833c1 100644 --- a/server/.env.example +++ b/server/.env.example @@ -13,6 +13,11 @@ LDAP_COMMON_NAME=localhost LDAP_BASE_DN=dc=localhost # PORT=636 +# Require authentication for search operations +# Set to true to require bind before allowing searches (recommended for production) +# Default: false (allows anonymous searches for backward compatibility) +# REQUIRE_AUTH_FOR_SEARCH=false + # SSL/TLS Certificate Configuration # Option 1: Provide certificate content directly # LDAP_CERT_CONTENT="-----BEGIN CERTIFICATE-----..." diff --git a/server/config/configurationLoader.js b/server/config/configurationLoader.js index 6a4def6..da2a728 100644 --- a/server/config/configurationLoader.js +++ b/server/config/configurationLoader.js @@ -47,6 +47,7 @@ class ConfigurationLoader { bindIp: process.env.BIND_IP || '0.0.0.0', unencrypted: process.env.LDAP_UNENCRYPTED === 'true' || process.env.LDAP_UNENCRYPTED === '1', backendDir: process.env.BACKEND_DIR || null, + requireAuthForSearch: process.env.REQUIRE_AUTH_FOR_SEARCH === 'true' || process.env.REQUIRE_AUTH_FOR_SEARCH === '1', // Load certificates - this handles all certificate logic ...this._loadCertificates() }; diff --git a/server/serverMain.js b/server/serverMain.js index 0da205e..0de4b85 100644 --- a/server/serverMain.js +++ b/server/serverMain.js @@ -53,7 +53,8 @@ async function startServer(config) { key: config.keyContent, logger: logger, authProviders: selectedBackends, - directoryProvider: selectedDirectory + directoryProvider: selectedDirectory, + requireAuthForSearch: config.requireAuthForSearch }); // Set up event listeners for logging and monitoring From 0d352f1e458873ce76e28699bbb4b396fdd3ddfa Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 18 Nov 2025 19:25:55 +0000 Subject: [PATCH 3/7] Implement authentication requirement for search operations Track authenticated connections manually to determine if search should be allowed. This works around ldapjs not updating bindDN properly with async bind handlers. Co-authored-by: anishapant21 <44058515+anishapant21@users.noreply.github.com> --- npm/src/LdapEngine.js | 57 +++++++++++++++++++++++++++++++++---------- 1 file changed, 44 insertions(+), 13 deletions(-) diff --git a/npm/src/LdapEngine.js b/npm/src/LdapEngine.js index 3fc597c..3d78caa 100644 --- a/npm/src/LdapEngine.js +++ b/npm/src/LdapEngine.js @@ -24,6 +24,9 @@ class LdapEngine extends EventEmitter { this.server = null; this.logger = options.logger || console; this._stopping = false; + + // Track authenticated connections (workaround for ldapjs bindDN not updating) + this.authenticatedConnections = new Set(); } /** @@ -57,6 +60,9 @@ class LdapEngine extends EventEmitter { // Setup search handlers this._setupSearchHandlers(); + // Setup connection cleanup + this._setupConnectionHandlers(); + // Setup error handlers this._setupErrorHandlers(); @@ -157,10 +163,10 @@ class LdapEngine extends EventEmitter { res.end(); }); - // Authenticated bind + // Authenticated bind - catch all DNs under our base this.server.bind(this.config.baseDn, async (req, res, next) => { const { username, password } = this._extractCredentials(req); - this.logger.debug("Authenticated bind request", { username }); + this.logger.debug("Authenticated bind request", { username, dn: req.dn.toString() }); try { this.emit('bindRequest', { username, anonymous: false }); @@ -177,6 +183,8 @@ class LdapEngine extends EventEmitter { return next(error); } + // Track this connection as authenticated + this.authenticatedConnections.add(req.connection.ldap.id); this.emit('bindSuccess', { username, anonymous: false }); res.end(); } catch (error) { @@ -194,19 +202,28 @@ class LdapEngine extends EventEmitter { * @private */ _setupSearchHandlers() { - this.server.search(this.config.baseDn, async (req, res, next) => { - const filterStr = req.filter.toString(); - this.logger.debug(`LDAP Search - Filter: ${filterStr}, Attributes: ${req.attributes}`); + // Authorization middleware (if enabled) + const authorizeSearch = (req, res, next) => { + if (!this.config.requireAuthForSearch) { + return next(); + } - // Check if authentication is required for search operations - if (this.config.requireAuthForSearch) { - const isAnonymous = !req.connection.ldap.bindDN || req.connection.ldap.bindDN.toString() === ''; - if (isAnonymous) { - this.logger.debug('Anonymous search rejected - authentication required'); - const error = new ldap.InsufficientAccessRightsError('Authentication required for search operations'); - return next(error); - } + const connectionId = req.connection.ldap.id; + const isAuthenticated = this.authenticatedConnections.has(connectionId); + + if (!isAuthenticated) { + this.logger.debug(`Anonymous search rejected - authentication required (connection: ${connectionId})`); + return next(new ldap.InsufficientAccessRightsError('Authentication required for search operations')); } + + this.logger.debug(`Authenticated search allowed (connection: ${connectionId})`); + return next(); + }; + + // Search handler with authorization middleware + this.server.search(this.config.baseDn, authorizeSearch, async (req, res, next) => { + const filterStr = req.filter.toString(); + this.logger.debug(`LDAP Search - Filter: ${filterStr}, Attributes: ${req.attributes}`); let entryCount = 0; const startTime = Date.now(); @@ -245,6 +262,20 @@ class LdapEngine extends EventEmitter { }); } + /** + * Setup connection handlers for cleanup + * @private + */ + _setupConnectionHandlers() { + this.server.on('close', (connectionId) => { + // Clean up authenticated connections when they close + if (this.authenticatedConnections.has(connectionId)) { + this.authenticatedConnections.delete(connectionId); + this.logger.debug(`[CONNECTION] Removed closed connection ${connectionId} from authenticated set`); + } + }); + } + /** * Setup error handlers * @private From ed6094c5bdbce79891bc0affbc9fd975312832bb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 18 Nov 2025 19:27:25 +0000 Subject: [PATCH 4/7] Document REQUIRE_AUTH_FOR_SEARCH security setting in README Co-authored-by: anishapant21 <44058515+anishapant21@users.noreply.github.com> --- README.md | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/README.md b/README.md index 4b28c62..5406ab0 100644 --- a/README.md +++ b/README.md @@ -126,6 +126,11 @@ AUTH_BACKENDS=ldap # mysql | mongodb | ldap | proxmox | notification | mysq # LDAP Server Configuration LDAP_BASE_DN=dc=company,dc=com +# Security: Require authentication for search operations (recommended for production) +# When enabled, clients must bind with credentials before performing searches +# Default: false (allows anonymous searches for backward compatibility) +REQUIRE_AUTH_FOR_SEARCH=true + # MySQL configuration (for any MySQL-based system) MYSQL_HOST=localhost MYSQL_PORT=3306 @@ -143,6 +148,34 @@ LDAP_BIND_PASSWORD=ldap_service_password AD_DOMAIN=company.com ``` +### Security Settings + +#### Require Authentication for Search + +For production deployments, you should require authentication before allowing LDAP search operations: + +```ini +# Require bind before allowing searches (recommended for production) +REQUIRE_AUTH_FOR_SEARCH=true +``` + +**Behavior:** +- `false` (default): Allows anonymous searches - useful for development and testing +- `true`: Clients must authenticate with valid credentials before searching + +**Example:** +```bash +# Without authentication (fails when REQUIRE_AUTH_FOR_SEARCH=true) +ldapsearch -H ldaps://localhost:636 -x -b "dc=company,dc=com" "(uid=john)" +# Result: Insufficient access (error 50) + +# With authentication (succeeds) +ldapsearch -H ldaps://localhost:636 -x -D "uid=john,dc=company,dc=com" -w password -b "dc=company,dc=com" "(uid=john)" +# Result: Returns user information +``` + +**Recommendation:** Set to `true` for all production environments to prevent unauthorized directory enumeration. + ### Start Service ```bash From 0528ac38d5db966f7b610785db754e27d73c837b Mon Sep 17 00:00:00 2001 From: Anisha Pant <44058515+anishapant21@users.noreply.github.com> Date: Fri, 21 Nov 2025 07:56:42 -0500 Subject: [PATCH 5/7] Update npm/src/LdapEngine.js Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- npm/src/LdapEngine.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/npm/src/LdapEngine.js b/npm/src/LdapEngine.js index 3d78caa..b02f999 100644 --- a/npm/src/LdapEngine.js +++ b/npm/src/LdapEngine.js @@ -267,12 +267,15 @@ class LdapEngine extends EventEmitter { * @private */ _setupConnectionHandlers() { - this.server.on('close', (connectionId) => { - // Clean up authenticated connections when they close - if (this.authenticatedConnections.has(connectionId)) { - this.authenticatedConnections.delete(connectionId); - this.logger.debug(`[CONNECTION] Removed closed connection ${connectionId} from authenticated set`); - } + // Listen for new connections and attach cleanup handler + this.server.on('connection', (connection) => { + connection.on('close', () => { + const connectionId = connection.ldap.id; + if (this.authenticatedConnections.has(connectionId)) { + this.authenticatedConnections.delete(connectionId); + this.logger.debug(`[CONNECTION] Removed closed connection ${connectionId} from authenticated set`); + } + }); }); } From b7125756d42ae7cae4948ea80bf917a219412d7a Mon Sep 17 00:00:00 2001 From: anishapant21 Date: Fri, 21 Nov 2025 08:05:02 -0500 Subject: [PATCH 6/7] Set require bind search to true --- README.md | 19 +++++++++++-------- npm/src/LdapEngine.js | 2 +- server/.env.example | 6 +++--- server/config/configurationLoader.js | 2 +- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 5406ab0..97425ae 100644 --- a/README.md +++ b/README.md @@ -126,9 +126,9 @@ AUTH_BACKENDS=ldap # mysql | mongodb | ldap | proxmox | notification | mysq # LDAP Server Configuration LDAP_BASE_DN=dc=company,dc=com -# Security: Require authentication for search operations (recommended for production) -# When enabled, clients must bind with credentials before performing searches -# Default: false (allows anonymous searches for backward compatibility) +# Security: Require authentication for search operations +# Default: true (authentication required for security) +# Set to false only for development/testing if you need anonymous access REQUIRE_AUTH_FOR_SEARCH=true # MySQL configuration (for any MySQL-based system) @@ -152,16 +152,19 @@ AD_DOMAIN=company.com #### Require Authentication for Search -For production deployments, you should require authentication before allowing LDAP search operations: +By default, authentication is required before allowing LDAP search operations: ```ini -# Require bind before allowing searches (recommended for production) -REQUIRE_AUTH_FOR_SEARCH=true +# Authentication required by default (recommended for security) +REQUIRE_AUTH_FOR_SEARCH=true # This is the default + +# Only disable for development/testing if needed +# REQUIRE_AUTH_FOR_SEARCH=false ``` **Behavior:** -- `false` (default): Allows anonymous searches - useful for development and testing -- `true`: Clients must authenticate with valid credentials before searching +- `true` (default): Clients must authenticate with valid credentials before searching +- `false`: Allows anonymous searches - only use for development/testing **Example:** ```bash diff --git a/npm/src/LdapEngine.js b/npm/src/LdapEngine.js index b02f999..ef85f1b 100644 --- a/npm/src/LdapEngine.js +++ b/npm/src/LdapEngine.js @@ -15,7 +15,7 @@ class LdapEngine extends EventEmitter { port: options.port || 389, certificate: options.certificate || null, key: options.key || null, - requireAuthForSearch: options.requireAuthForSearch || false, + requireAuthForSearch: options.requireAuthForSearch !== false, ...options }; diff --git a/server/.env.example b/server/.env.example index 7b833c1..a7e7e9c 100644 --- a/server/.env.example +++ b/server/.env.example @@ -14,9 +14,9 @@ LDAP_BASE_DN=dc=localhost # PORT=636 # Require authentication for search operations -# Set to true to require bind before allowing searches (recommended for production) -# Default: false (allows anonymous searches for backward compatibility) -# REQUIRE_AUTH_FOR_SEARCH=false +# Default: true (authentication required) +# Set to false only for development/testing if you need anonymous access +# REQUIRE_AUTH_FOR_SEARCH=true # SSL/TLS Certificate Configuration # Option 1: Provide certificate content directly diff --git a/server/config/configurationLoader.js b/server/config/configurationLoader.js index da2a728..eba91cf 100644 --- a/server/config/configurationLoader.js +++ b/server/config/configurationLoader.js @@ -47,7 +47,7 @@ class ConfigurationLoader { bindIp: process.env.BIND_IP || '0.0.0.0', unencrypted: process.env.LDAP_UNENCRYPTED === 'true' || process.env.LDAP_UNENCRYPTED === '1', backendDir: process.env.BACKEND_DIR || null, - requireAuthForSearch: process.env.REQUIRE_AUTH_FOR_SEARCH === 'true' || process.env.REQUIRE_AUTH_FOR_SEARCH === '1', + requireAuthForSearch: process.env.REQUIRE_AUTH_FOR_SEARCH !== 'false', // Load certificates - this handles all certificate logic ...this._loadCertificates() }; From 289cf482dd11a0f21332ec71b13d99a492597c36 Mon Sep 17 00:00:00 2001 From: anishapant21 Date: Tue, 25 Nov 2025 12:22:50 -0500 Subject: [PATCH 7/7] Fixes for anon ldapsearch --- npm/src/LdapEngine.js | 37 +++++++------------------------------ 1 file changed, 7 insertions(+), 30 deletions(-) diff --git a/npm/src/LdapEngine.js b/npm/src/LdapEngine.js index ef85f1b..73664f5 100644 --- a/npm/src/LdapEngine.js +++ b/npm/src/LdapEngine.js @@ -24,9 +24,6 @@ class LdapEngine extends EventEmitter { this.server = null; this.logger = options.logger || console; this._stopping = false; - - // Track authenticated connections (workaround for ldapjs bindDN not updating) - this.authenticatedConnections = new Set(); } /** @@ -60,9 +57,6 @@ class LdapEngine extends EventEmitter { // Setup search handlers this._setupSearchHandlers(); - // Setup connection cleanup - this._setupConnectionHandlers(); - // Setup error handlers this._setupErrorHandlers(); @@ -183,8 +177,6 @@ class LdapEngine extends EventEmitter { return next(error); } - // Track this connection as authenticated - this.authenticatedConnections.add(req.connection.ldap.id); this.emit('bindSuccess', { username, anonymous: false }); res.end(); } catch (error) { @@ -208,15 +200,17 @@ class LdapEngine extends EventEmitter { return next(); } - const connectionId = req.connection.ldap.id; - const isAuthenticated = this.authenticatedConnections.has(connectionId); + // Check if connection has authenticated bindDN (not anonymous) + const bindDN = req.connection.ldap.bindDN; + const bindDNStr = bindDN ? bindDN.toString() : 'null'; + const isAnonymous = !bindDN || bindDNStr === 'cn=anonymous'; - if (!isAuthenticated) { - this.logger.debug(`Anonymous search rejected - authentication required (connection: ${connectionId})`); + if (isAnonymous) { + this.logger.debug(`Anonymous search rejected - authentication required`); return next(new ldap.InsufficientAccessRightsError('Authentication required for search operations')); } - this.logger.debug(`Authenticated search allowed (connection: ${connectionId})`); + this.logger.debug(`Authenticated search allowed for ${bindDNStr}`); return next(); }; @@ -262,23 +256,6 @@ class LdapEngine extends EventEmitter { }); } - /** - * Setup connection handlers for cleanup - * @private - */ - _setupConnectionHandlers() { - // Listen for new connections and attach cleanup handler - this.server.on('connection', (connection) => { - connection.on('close', () => { - const connectionId = connection.ldap.id; - if (this.authenticatedConnections.has(connectionId)) { - this.authenticatedConnections.delete(connectionId); - this.logger.debug(`[CONNECTION] Removed closed connection ${connectionId} from authenticated set`); - } - }); - }); - } - /** * Setup error handlers * @private