Re: [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp 00/11] Add LDAP and AD realm support to Proxmox Datacenter Manager

["Shannon Sterz" <s.sterz@proxmox.com>]

sorry just noticed this never went to the list... so i'm resending
these...

On Fri Sep 19, 2025 at 12:02 PM CEST, Christoph Heiss wrote:
> Applied the series and did some testing.
>
> Tested with:
>
> - LDAP with OpenLDAP, slapd 2.5.13+dfsg-5
> - LDAP and AD with Samba 4.22.3
> - LDAP and AD with Windows Server 2022 Active Directory
>
> Some things I noticed:
>
> - No default realm is set by default. E.g. with this series applied, by
>   default all realms show an "X" in the default column. Should set PAM
>   as default realm if none is set yet.

yes the default realm logic is lacking in general. i am already working
on the following things:

- make the login component respect the default realm setting
- allow editing other realms in order to allow setting them as default
  realms

so i'll add marking pam as default if no realm is set as default. for
now, i'd leave the default realm parts of this series as-is or move them
out of this series and tag that on to adding default realm support in
general.

> - After syncing a realm, the changes are not immediately reflected in the
>   "User Management" tab, one has to refresh manually.

hm yeah, though that is then probably missing in the pre-existing
component there. i'll see what i can do, if it's trickier i'd send this
in a separate series, though.

> - I know the form layout is pretty much taken from PVE/PBS, but at least
>   the "Base Domain Name" and "Bind Domain Name" could use some longer
>   fields, or the dialog window more width as a whole.
>
>   These are usually pretty long, 50+ characters for the bind DN are not
>   unusual and currently these fields fit ~25 characters.

yes make sense, guess we could use some large fields here. though,
pve/pbs would also benefit from that then.

> - Unless I missed it, but the `proxmox-datacenter-manager-client` does
>   not (yet) have a `realm` subcommand to manage them from the shell?

yes that is missing, did not yet tackle the cli part here as i was
uncertain on whether we want to keep the api parts in pdm or in
proxmox-rs. as the cli would just depend on the respective endpoints
there then anyway.

i would tackle this in a follow-up if that's alright.

> - If "Enable new users" is set to "No", this isn't reflected/respected
>   in the sync panel. E.g. set it to "No", the sync panel will still show
>   "Default (Yes)" for the "Enable new" field.
>   Also, I'd also name it "Enable new users" in the sync panel for
>   consistency.
>
> - Continuing from the last one, the setting does not even seem to be
>   respected? Setting it to "No" in both the realm settings and the sync
>   panel _still_ creates new users.
>
> - Similarly, the "Remove vanished X" options are not reflected in the
>   sync panel, but _are_ respected if not touched.

these three seem strange, i'm pretty sure i had them working before, but
maybe i missed something. will take a look and send a v2.

> - The placeholder for the bind DN should probably use @example.com, as
>   that domain is explicitly reserved by RFC 2606 for such purposes.
>
> - When anonymous search is enabled, the bind DN field still shows
>   "user@company.net", which /might/ suggest to users that this is used
>   as default. Very much a nit tho.

both of these are taken from the pbs/pve implementation, though your
suggestions are sensible to me, will look into that in a v2.

> - PVE/PBS have the ability to also sync the `comment` field from some
>   attribute (e.g. `sync-attributes comment=userPrincipal`), for PDM
>   this currently is not allowed by the schema. But this can be part of a
>   future improvement IMO.

will take a look at this as well, must have missed that as it wasn't
exposed in the ui.

> On Tue Sep 16, 2025 at 4:48 PM CEST, Shannon Sterz wrote:
>> this patch series adds ldap and active directory (ad) support to proxmox
>> datacenter manager. the series first moves some of the sync logic for
>> ldap & ad realms out of proxmox backup manager into the proxmox-ldap
>> crate.
>>
>> the next series of patches fixes up the existing proxmox-yew-comp
>> components for adding and editing realms to function as intended for
>> adding, editing, removing and syncing ad and ldap realms.
>>
>> finally, we add the necessary backend infrastructure and api endpoints
>> to proxmox datacenter manager and expose the new ui components there.
>>
>> this series does not yet move proxmox backup server to use the new
>> common crate. doing so would mean that proxmox backup server would also
>> need to start using proxmox-access-control, which would be a lot more
>> involved and is beste handled in a separate series in my opinion.
>>
>> proxmox:
>>
>> Shannon Sterz (1):
>>   ldap: add types and sync features
>>
>>  Cargo.toml                    |   2 +-
>>  proxmox-ldap/Cargo.toml       |  22 ++
>>  proxmox-ldap/debian/control   |  31 ++-
>>  proxmox-ldap/debian/copyright |   2 +-
>>  proxmox-ldap/src/lib.rs       |   6 +
>>  proxmox-ldap/src/sync.rs      | 496 ++++++++++++++++++++++++++++++++++
>>  proxmox-ldap/src/types.rs     | 317 ++++++++++++++++++++++
>>  7 files changed, 873 insertions(+), 3 deletions(-)
>>  create mode 100644 proxmox-ldap/src/sync.rs
>>  create mode 100644 proxmox-ldap/src/types.rs
>>
>>
>> proxmox-yew-comp:
>>
>> Shannon Sterz (5):
>>   auth_view: add default column and allow setting ldap realms as default
>>   utils: add pdm realm to `get_auth_domain_info`
>>   auth_view/auth_edit_ldap: add support for active directory realms
>>   auth_edit_ldap: add helpers to properly edit ad & ldap realms
>>   auth_view: implement syncing ldap and ad realms
>>
>>  src/auth_edit_ldap.rs   | 165 ++++++++++++++++++++++++++++++++-----
>>  src/auth_view.rs        | 174 +++++++++++++++++++++++++++++++++++++---
>>  src/common_api_types.rs |   3 +
>>  src/utils.rs            |  18 ++---
>>  4 files changed, 315 insertions(+), 45 deletions(-)
>>
>>
>> proxmox-datacenter-manager:
>>
>> Shannon Sterz (5):
>>   config: add domain config plugins for ldap and ad realms
>>   server: add ldap and active directory authenticators
>>   server: api: add api endpoints for configuring ldap & ad realms
>>   api/auth: add endpoint to start ldap sync jobs
>>   ui: add a panel to allow handling realms
>>
>>  Cargo.toml                           |   1 +
>>  lib/pdm-api-types/src/acl.rs         |   3 +
>>  lib/pdm-api-types/src/lib.rs         |   7 +
>>  lib/pdm-config/Cargo.toml            |   1 +
>>  lib/pdm-config/src/domains.rs        |  35 +++
>>  server/Cargo.toml                    |   1 +
>>  server/src/api/access/domains.rs     |  90 ++++++-
>>  server/src/api/config/access/ad.rs   | 355 +++++++++++++++++++++++++
>>  server/src/api/config/access/ldap.rs | 372 +++++++++++++++++++++++++++
>>  server/src/api/config/access/mod.rs  |   8 +-
>>  server/src/auth/ldap.rs              | 315 +++++++++++++++++++++++
>>  server/src/auth/mod.rs               |  17 +-
>>  ui/src/configuration/mod.rs          |  14 +-
>>  13 files changed, 1210 insertions(+), 9 deletions(-)
>>  create mode 100644 server/src/api/config/access/ad.rs
>>  create mode 100644 server/src/api/config/access/ldap.rs
>>  create mode 100644 server/src/auth/ldap.rs
>>
>>
>> Summary over all repositories:
>>   24 files changed, 2398 insertions(+), 57 deletions(-)
>>
>> --
>> Generated by git-murpp 0.8.1
>>
>>
>> _______________________________________________
>> pdm-devel mailing list
>> pdm-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel



_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel