From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 99969922A7 for ; Fri, 10 Feb 2023 15:01:55 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 839AB2FFD8 for ; Fri, 10 Feb 2023 15:01:25 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Fri, 10 Feb 2023 15:01:24 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id DEB694670B for ; Fri, 10 Feb 2023 15:01:23 +0100 (CET) Message-ID: <496ccddd-344e-1c29-83ea-82630847c738@proxmox.com> Date: Fri, 10 Feb 2023 15:01:21 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.6.0 Content-Language: en-US To: Lukas Wagner References: <20230209133128.695211-1-l.wagner@proxmox.com> Cc: Proxmox Backup Server development discussion From: Friedrich Weber In-Reply-To: <20230209133128.695211-1-l.wagner@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.233 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -1.149 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [conf.py, proxmox-backup-manager.rs, docgen.rs, acl.rs, api.rs, ldap.rs, domains.rs, domain.rs, mod.rs, proxmox.com, tfa.rs, lib.rs, auth.rs, openid.rs, user.rs] Subject: Re: [pbs-devel] [PATCH v3 proxmox{, -backup, -widget-toolkit} 00/18] add LDAP realm support X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 10 Feb 2023 14:01:55 -0000 Tested this with slapd and it worked nicely! I found two small issues, see below. Tested-by: Friedrich Weber ## Sync fails if some LDAP user attribute violates API schema 1) Create LDAP users: * "a" with givenName "a" (one character) * "foo" with givenName "foo" 2) Add an LDAP realm, set "First Name attribute" to "givenName" 3) Sync->Preview succeeds 4) Sync->Sync fails with: 2023-02-10T14:44:14+01:00: starting realm sync for ldap 2023-02-10T14:44:14+01:00: creating user a@ldap 2023-02-10T14:44:14+01:00: creating user foo@ldap 2023-02-10T14:44:14+01:00: TASK ERROR: could not store user config No new users were created. Not creating "a" is okay, as the API schema requires firstnames to have length >=2, but the sync should probably just skip "a" with a warning and create "foo" anyway. ## Web UI shows an error after editing an LDAP realm 1) Add an LDAP realm with default settings 2) Edit the realm and set "user classes" to "inetorgperson", click OK 3) Error message dialog "parameter verification errors" / "delete: value 'filter,sync-defaults-options,sync-attributes' is not defined in the enumeration" The decoded form data of the corresponding PUT request sets: "delete": [ "server2", "port", "bind-dn", "password", "filter,sync-defaults-options,sync-attributes" ] ... which looks like some parsing logic got confused somewhere and fused the last three items together. On 09/02/2023 14:31, Lukas Wagner wrote: > This patch series adds support for adding LDAP realms, including user sync. > > The configuration scheme in `pbs-api-types` is based on the one from PVE, > with some slight differences: > * consistent use of kebab-case for properties > * only support `mode` instead of the deprecated `secure` property > > The GUI is mostly based on the implementation from PVE, with some slight > adaptations - for details, please refer to the commit messages. > The GUI components were added to the widget-toolkit repo, at some point PVE > could be adapted to use the same implemention as PBS. > > This patch series adds a new dependency to the `proxmox-ldap` crate, > introduced in [1]. This also brings in `ldap3` and `lber` as new transitive > dependencies. Both crates were already packaged and are available on the > repository, thanks to Fabian. > > The implementation was tested against the following LDAP servers: > * slapd 2.5.13 on Ubuntu Server 22.04 (LDAP, LDAPS, STARTTLS) > * Windows Server 2022 Active Directory (LDAP) > * glauth 2.1.0 (LDAP, LDAPS) > > Some notes for testers: > * I can recommend `glauth` for testing: It is an LDAP server implementation > in a statically-compiled Go binary that can be configured using a single, > simple to understand configuration file. I can share my config if needed. > > > Note: This patch series includes a cherry-picked commit from Hannes' series from > [2]. The functionality was needed for user sync. > > Changes v2 --> v3: > * Dropped the `Ldap` prefix for structs from the `proxmox-ldap` crate > * minor clippy fixes > * added a `OpenIdAuthenticator` that implements dummy-implements > `ProxmoxAuthenticator` - otherwise, manually adding users to > OpenId realms does not work > * Changed the naming of the different authenticators in `auth.rs` > e.g PAM --> PamAuthenticator, LDAP --> LdapAuthenticator > This allows us to drop some clippy-allow directives > > Changes v1 --> v2: > * add pbs_config::exists helper function > * Remove now unused `password` field from `LdapRealmConfig`, add > additional password paramter to routes which need it > * Only log a warning instead of failing completely when removing a > stored password does not work > * Proper naming for `DeleteableProperty` struct > * Document that the domain config lock must be held when > the LDAP password helper functions are called. > Also added a &BackupLockGuard as a parameter, to make sure that > at least *something* is locked. > * moved `handle_worker` function to the `proxmox_rest_server` crate, > so that it is usable for both, the LDAP management CLI and the debug > CLI. > * Made user authentication async, > `ProxmoxAuthenticator::authenticate_user` now returns a boxed future > * Promoted `src/server/ldap.rs` to be its own crate - this will be > useful when PVE uses the same LDAP implemenation via perlmod one > day. > > > [1] https://lists.proxmox.com/pipermail/pbs-devel/2023-January/005833.html > [2] https://lists.proxmox.com/pipermail/pbs-devel/2022-December/005774.html > > proxmox: > Lukas Wagner (1): > rest-server: add handle_worker from backup debug cli > > proxmox-rest-server/src/worker_task.rs | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > proxmox-backup: > Hannes Laimer (1): > pbs-config: add delete_authid to ACL-tree > > Lukas Wagner (12): > debug cli: use handle_worker in proxmox-rest-server > ui: add 'realm' field in user edit > api-types: add LDAP configuration type > api: add routes for managing LDAP realms > auth: add LDAP realm authenticator > api-types: add config options for LDAP user sync > server: add LDAP realm sync job > manager: add commands for managing LDAP realms > docs: add configuration file reference for domains.cfg > docs: add documentation for LDAP realms > auth: add dummy OpenIdAuthenticator struct > auth: unify naming for all authenticator implementations > > Cargo.toml | 2 + > docs/Makefile | 6 +- > docs/conf.py | 1 + > docs/config/domains/format.rst | 27 ++ > docs/config/domains/man5.rst | 21 ++ > docs/configuration-files.rst | 16 + > docs/user-management.rst | 58 ++++ > pbs-api-types/src/ldap.rs | 199 +++++++++++ > pbs-api-types/src/lib.rs | 5 + > pbs-api-types/src/user.rs | 2 +- > pbs-config/src/acl.rs | 71 ++++ > pbs-config/src/domains.rs | 43 ++- > src/api2/access/domain.rs | 85 ++++- > src/api2/access/mod.rs | 8 +- > src/api2/access/tfa.rs | 15 +- > src/api2/config/access/ldap.rs | 352 +++++++++++++++++++ > src/api2/config/access/mod.rs | 7 +- > src/api2/config/access/openid.rs | 5 +- > src/auth.rs | 208 +++++++++-- > src/auth_helpers.rs | 58 ++++ > src/bin/docgen.rs | 1 + > src/bin/proxmox-backup-manager.rs | 1 + > src/bin/proxmox_backup_debug/api.rs | 27 +- > src/bin/proxmox_backup_manager/ldap.rs | 152 ++++++++ > src/bin/proxmox_backup_manager/mod.rs | 2 + > src/server/mod.rs | 3 + > src/server/realm_sync_job.rs | 463 +++++++++++++++++++++++++ > www/OnlineHelpInfo.js | 8 + > www/Utils.js | 4 +- > www/window/UserEdit.js | 95 ++++- > 30 files changed, 1840 insertions(+), 105 deletions(-) > create mode 100644 docs/config/domains/format.rst > create mode 100644 docs/config/domains/man5.rst > create mode 100644 pbs-api-types/src/ldap.rs > create mode 100644 src/api2/config/access/ldap.rs > create mode 100644 src/bin/proxmox_backup_manager/ldap.rs > create mode 100644 src/server/realm_sync_job.rs > > promxox-widget-toolkit: > Lukas Wagner (4): > auth ui: add LDAP realm edit panel > auth ui: add LDAP sync UI > auth ui: add `onlineHelp` for AuthEditLDAP > auth ui: add `firstname` and `lastname` sync-attribute fields > > src/Makefile | 2 + > src/Schema.js | 12 ++ > src/panel/AuthView.js | 24 +++ > src/window/AuthEditLDAP.js | 367 +++++++++++++++++++++++++++++++++++++ > src/window/SyncWindow.js | 192 +++++++++++++++++++ > 5 files changed, 597 insertions(+) > create mode 100644 src/window/AuthEditLDAP.js > create mode 100644 src/window/SyncWindow.js > >