Skip to content

Alternatives authentification ldap - #484

Merged
kevin-atnos merged 13 commits into
masterfrom
alternatives-authentification-ldap
Nov 9, 2026
Merged

kevin-atnos merged 13 commits into
masterfrom
alternatives-authentification-ldap

Conversation

@brunto

@brunto brunto commented Nov 6, 2026 •

Copy link
Copy Markdown
Collaborator

This pull request removes the old devise_ldap_authenticatable integration and replaces it with a new, custom LDAP authentication implementation using the net-ldap gem. The new approach provides more robust configuration, better error handling, and improved documentation, while maintaining backward compatibility with previous environment variables (with deprecation warnings). It also improves random password generation and clarifies how LDAP users are matched and created.

LDAP Integration Overhaul

  • Replaces the devise_ldap_authenticatable gem with a new LdapAuthenticator service using net-ldap, supporting secure TLS/StartTLS connections, explicit certificate verification, and robust error handling. [1] [2]
  • Adds comprehensive LDAP configuration and usage documentation to README.md, including environment variable migration, encryption options, and user creation behavior.
  • Updates .env.example to introduce new environment variables (LDAP_ENABLED, LDAP_ADMIN_BIND), deprecating and documenting the old ones.
  • Issues warnings at boot if deprecated environment variables are used, to guide operators through migration.

Authentication Logic and User Model Improvements

  • Refactors Doorkeeper authentication to use the new LDAP service, ensuring local logins remain available if LDAP is down and handling user lookup/creation according to directory spelling.
  • Removes all devise_ldap_authenticatable configuration and exception handling from initializers and controllers. [1] [2]
  • Refines random password generation for new users, using a more secure and configurable approach, and ensures passwords meet model validation rules. [1] [2]

brunto added 13 commits November 9, 2026 14:12
devise_ldap_authenticatable is unmaintained. Authenticate LDAP users
directly with net-ldap through a new LdapAuthenticator service called
from the Doorkeeper password grant.

- Look the user up with the configured attribute (anonymous or admin
  bind), fall back to the legacy attribute=login,base DN, then bind with
  the user credentials. Ambiguous matches, empty passwords and NUL bytes
  are rejected, filter and DN values are escaped, and connect and request
  times are bounded.
- Always verify the server certificate and hostname for LDAPS and
  StartTLS. A private CA is set with encryption.tls_options.ca_file.
- Generate the password of LDAP-created users with User.random_password
  (64 characters from SecureRandom) instead of Array#sample. Use it in
  UsersController#create as well.
- Remove the Devise LDAP settings, the LdapException rescue_from handler
  and the already disabled authorization block of config/ldap.yml.
- Document the LDAP configuration in README.md and .env.example.
- Add unit, integration and wire-level tests against a fake LDAP server.

BREAKING CHANGE: TLS certificates were not verified before. LDAP
deployments with a self-signed or private CA certificate must now set a
trusted CA, and the certificate must match the configured host.
A user bind refused with a result code other than 49, 32 or 48 (for
example 19 or 53 for a locked or disabled account, 34 for an invalid
DN) raised LdapAuthenticator::Error and made the Doorkeeper password
grant return a 500. The legacy gem treated every failed bind as invalid
credentials.

Return nil for any refused user bind so the request ends with
invalid_grant or falls back to local authentication, and log the result
code at info level, never the login, DN or password. Remove the unused
INVALID_CREDENTIALS list. Directory search and admin or anonymous bind
failures still raise.

Update the unit tests and the README accordingly.
The net-ldap migration verifies the LDAP server certificate chain and hostname, whereas devise_ldap_authenticatable never validated the chain. Deployments using a self-signed or private-CA certificate therefore fail LDAP logins until the CA is trusted.

Add encryption.verify_certificates to the LDAP configuration. Verification stays enabled by default. Setting it to false disables certificate and hostname verification and logs a warning on every connection. The value must be a boolean, and tls_options cannot disable verification.

Report TLS verification failures with a message pointing to encryption.tls_options.ca_file. net-ldap flattens handshake errors into a plain Net::LDAP::Error, so the failure is recognised from its message.

Document both options in the README and cover them with unit and wire tests, including the StartTLS negative cases.
Rescue LdapAuthenticator::Error in the Doorkeeper password grant (connection, TLS verification, configuration and ambiguous search errors), log its message at error level and continue with local email/password authentication. Local administrator accounts are no longer locked out while the directory is down, and a failed LDAP check ends in invalid_grant instead of a 500.

Users created by a first LDAP login have a random local password, so the fallback cannot authenticate them. Cover the behavior with integration tests, move the capture_logs helper to test_helper.rb and update the README.
Directories match logins case-insensitively and ignore extra spaces, but the local account was looked up with the typed login. ALICE or " alice " could therefore miss the account stored as alice and create a duplicate, or fail on the email uniqueness check.

LdapAuthenticator::Result now carries the login as spelled by the directory. The search requests the configured login attribute and picks the exact stored value, then the first one equal to the typed login once case and spaces are normalized, and falls back to the typed login when the directory returns nothing usable. The result login is never blank.

The Doorkeeper hook looks the account up by the canonical login, then by the typed login so accounts created by the previous integration keep working, and creates new accounts with the canonical login. Local logins are deliberately not matched case-insensitively, since some directories distinguish Alice from alice.
A first LDAP login deliberately does not link itself to an existing local account by email, since the directory mail attribute is not proof of ownership of that account. Document that such a collision fails with a 422, how an administrator can link the account by setting its login, and that simultaneous first logins of one user can make one request fail until the user signs in again.
DEVISE_LDAP_LOGGER enabled LDAP for any non-empty value, even "false", and DEVISE_LDAP_LOGGER_ADMIN_BIND enabled the admin bind whenever it was defined. Both names were misleading. LDAP_ENABLED and LDAP_ADMIN_BIND are strict booleans (true/false, 1/0, yes/no, on/off); any other value raises LdapAuthenticator::Error, which is logged and falls back to local authentication.

The deprecated variables are still honored, with their historical semantics, while the new ones are unset or blank, so existing deployments keep working. A warning logged at boot states the value each deprecated variable currently stands for, or that it is ignored once its replacement is set.

Also log an info message, without the login, DN or password, when the legacy DN format is used because the search returned no user.

Add a with_env test helper and cover the settings, the deprecation warnings and the Doorkeeper integration. Document the variables in the README and .env.example.
The search time option was set from CONNECT_TIMEOUT. It is the server-side time limit of the search request, unrelated to the connection timeout, so give it its own SEARCH_TIME_LIMIT constant. The value stays 5 seconds.

Cover it with a test that overrides the constant, which a shared value could not detect.
validate_config! stored the normalized port in @PORT for connection_options to read later, which made the two methods depend on the order they are called in. Replace it with a side-effect free port method used by both, so validation no longer mutates the authenticator.

Add tests for numeric string ports and for the values rejected as ports, which were not covered.
User.random_password no longer restates the Devise complexity regexes: each candidate is validated through the User model, so it follows password_length and password_complexity if they change.

Candidates are drawn at most RANDOM_PASSWORD_ATTEMPTS times and a clear error is raised when none is accepted, instead of looping forever. Rejection sampling keeps the result uniform over accepted passwords.

Devise hashes a password as soon as it is assigned, so each generated password now costs one extra bcrypt hash.
UsersControllerTest was an empty skeleton, so UsersController#create and its use of User.random_password were untested. The tests cover the locked account, the generated password replacing a submitted one, the activation email, access types, a rejected duplicate email and a missing access token.

UsersController authorizes every action with Pundit, which ApplicationController only includes when ENABLE_AUTHENTICATION is set at boot, so the test includes it explicitly. The mail sender comes from credentials that the test environment lacks, so it is set (and restored) in the test setup.
The warning logged when no LDAP encryption is configured had no test. Cover the unset, nil, false and empty settings (logged exactly once per connection, without leaking the user or admin passwords) and check that no warning is logged when simple_tls or start_tls is configured.
The flag was set after Thread.new returned, so an exception raised before that point would still be reported on stderr. Set it from the thread itself, as its first statement.
@kevin-atnos
kevin-atnos force-pushed the alternatives-authentification-ldap branch from 15570ea to a8071dc Compare November 9, 2026 13:12
@kevin-atnos
kevin-atnos merged commit 2c9e9b2 into master Nov 9, 2026
4 checks passed
@kevin-atnos
kevin-atnos deleted the alternatives-authentification-ldap branch November 9, 2026 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants