all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Christoph Heiss <c.heiss@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH common/access-control/manager v2 0/3] ldap: check bind connection on realm add/update
Date: Tue,  1 Aug 2023 14:37:16 +0200	[thread overview]
Message-ID: <20230801123734.446797-1-c.heiss@proxmox.com> (raw)

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) and Samba
4.18.5 (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
---------
v1: https://lists.proxmox.com/pipermail/pve-devel/2023-July/058551.html

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 - although I'm always open for other/better
suggestions to tackle this.

[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

--
2.41.0





             reply	other threads:[~2023-08-01 12:38 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-01 12:37 Christoph Heiss [this message]
2023-08-01 12:37 ` [pve-devel] [PATCH common v2 1/3] ldap: handle errors explicitly everywhere instead of simply `die`ing Christoph Heiss
2023-08-01 12:37 ` [pve-devel] [PATCH access-control v2 2/3] ldap: add opt-in `check-connection` param to perform a bind check Christoph Heiss
2023-08-10  7:55   ` Wolfgang Bumiller
2023-08-10  8:35     ` Christoph Heiss
2023-08-10  8:49       ` Wolfgang Bumiller
2023-08-01 12:37 ` [pve-devel] [PATCH manager v2 3/3] ui: ldap: add 'Check connection' checkbox as advanced option Christoph Heiss

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230801123734.446797-1-c.heiss@proxmox.com \
    --to=c.heiss@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal