[pve-devel] [PATCH common/access-control/manager v3 0/4] ldap: check bind connection on realm add/update

[Christoph Heiss]

First of, remove the dreaded LDAP DN regex.

Further, upon saving a LDAP realm in the UI, it tries to connect & bind
using the provided credentials, providing the user with immediate
feedback whether they are valid or not.

The same approach is already implemented in PBS [0], and I'll plan to
implement the same for PMG too, if & when the PVE side is done.

Testing
-------
Changes were tested against slapd 2.5.13+dfsg-5 (for LDAP), Samba 4.18.5
and Windows Server 2022 (for AD), using both the web UI and `pveum` to
create and update realms with different combinations of valid and
invalid parameters, mixed with using new `check-connection` parameter.

Prior art
---------
v2: https://lists.proxmox.com/pipermail/pve-devel/2023-August/058586.html
v1: https://lists.proxmox.com/pipermail/pve-devel/2023-July/058551.html

Notable changes v2 -> v3:
  * Move 'check-connection' param out of schema to extra param
    (thanks Wolfgang!)

Notable changes v1 -> v2:
  * Added patch #1 from previous series [1], missed that in v1
  * Do not store the 'check-connection' parameter in the realm config
  * Add "Check connection" checkbox to AD edit too

This series supersedes [1], which previously solved this using a new
schema format by validating DNs using Net::LDAP::Util::canonical_dn().
But this has the problem that it does not support AD-specific DN syntax.

After a off-list discussion with Lukas (summary [2] [3]), it was decided to
rather implement it much like PBS does it - simply drop the explicit
validation of DN parameters, instead just trying to connect & bind to
the target server.

[0] https://git.proxmox.com/?p=proxmox-backup.git;a=commitdiff;h=5210f3b5
[1] https://lists.proxmox.com/pipermail/pve-devel/2023-July/058392.html
[2] https://lists.proxmox.com/pipermail/pve-devel/2023-July/058540.html
[3] https://lists.proxmox.com/pipermail/pve-devel/2023-August/058582.html

pve-common:

Christoph Heiss (2):
  ldap: handle errors explicitly everywhere instead of simply `die`ing
  section config: allow base properties for {create,update}Schema()

 src/PVE/LDAP.pm          | 11 ++++++-----
 src/PVE/SectionConfig.pm |  8 ++++----
 2 files changed, 10 insertions(+), 9 deletions(-)

pve-access-control:

Christoph Heiss (1):
  ldap: add opt-in `check-connection` param to perform a bind check

 src/PVE/API2/Domains.pm | 36 +++++++++++++++++++++++++++++++++---
 src/PVE/Auth/LDAP.pm    | 18 +++++++++---------
 src/PVE/Auth/Plugin.pm  |  8 ++++++++
 3 files changed, 50 insertions(+), 12 deletions(-)

pve-manager:

Christoph Heiss (1):
  ui: ldap: add 'Check connection' checkbox as advanced option

 www/manager6/dc/AuthEditAD.js   | 15 +++++++++++++++
 www/manager6/dc/AuthEditLDAP.js | 15 +++++++++++++++
 2 files changed, 30 insertions(+)

--
2.41.0