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 346A49E7F2 for ; Tue, 28 Nov 2023 09:24:29 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 09ABA12682 for ; Tue, 28 Nov 2023 09:23:59 +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 ; Tue, 28 Nov 2023 09:23:58 +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 1559144F14 for ; Tue, 28 Nov 2023 09:23:58 +0100 (CET) Date: Tue, 28 Nov 2023 09:23:51 +0100 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox Backup Server development discussion References: <20230816144746.1265108-1-c.heiss@proxmox.com> <20230816144746.1265108-8-c.heiss@proxmox.com> In-Reply-To: <20230816144746.1265108-8-c.heiss@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.16.0 (https://github.com/astroidmail/astroid) Message-Id: <1701158734.9lqyuqtc22.astroid@yuna.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL -0.086 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment POISEN_SPAM_PILL 0.1 Meta: its spam POISEN_SPAM_PILL_1 0.1 random spam to be learned in bayes POISEN_SPAM_PILL_3 0.1 random spam to be learned in bayes SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [ad.rs] Subject: Re: [pbs-devel] [PATCH proxmox-backup v2 07/15] api: access: add routes for managing AD realms 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: Tue, 28 Nov 2023 08:24:29 -0000 On August 16, 2023 4:47 pm, Christoph Heiss wrote: > [..] > diff --git a/src/api2/config/access/ad.rs b/src/api2/config/access/ad.rs > new file mode 100644 > index 00000000..c202291a > --- /dev/null > +++ b/src/api2/config/access/ad.rs > @@ -0,0 +1,348 @@ > +use anyhow::{bail, format_err, Error}; > +use hex::FromHex; > +use serde::{Deserialize, Serialize}; > +use serde_json::Value; > > [..] > > +#[api( > + input: { > + properties: {}, > + }, > + returns: { > + description: "List of configured AD realms.", > + type: Array, > + items: { type: AdRealmConfig }, > + }, > + access: { > + permission: &Permission::Privilege(&["access", "domains"], PRIV_= REALM_ALLOCATE, false), this one here > + }, > +)] > +/// List configured AD realms > +pub fn list_ad_realms( > + _param: Value, > + rpcenv: &mut dyn RpcEnvironment, > +) -> Result, Error> { > + let (config, digest) =3D domains::config()?; > + > + let list =3D config.convert_to_typed_array("ad")?; > + > + rpcenv["digest"] =3D hex::encode(digest).into(); > + > + Ok(list) > +} > + > [..] > + > +#[api( > + input: { > + properties: { > + realm: { > + schema: REALM_ID_SCHEMA, > + }, > + }, > + }, > + returns: { type: AdRealmConfig }, > + access: { > + permission: &Permission::Privilege(&["access", "domains"], PRIV_= SYS_AUDIT, false), and this one here don't really agree - copied over from LDAP ;) also, maybe this one here should check on /access/domains/{realm} (although that might be postponed to do it in sync with the other endpoint(s), but it would be more in line with how we handle entity ACLs in general). > + }, > +)] > +/// Read the AD realm configuration > +pub fn read_ad_realm( > + realm: String, > + rpcenv: &mut dyn RpcEnvironment, > +) -> Result { > + let (domains, digest) =3D domains::config()?; > + > + let config =3D domains.lookup("ad", &realm)?; > + > + rpcenv["digest"] =3D hex::encode(digest).into(); > + > + Ok(config) > +} > + > [..] > + > +#[api( > + protected: true, > + input: { > + properties: { > + realm: { > + schema: REALM_ID_SCHEMA, > + }, > + update: { > + type: AdRealmConfigUpdater, > + flatten: true, > + }, > + password: { > + description: "AD bind password", > + optional: true, > + }, > + delete: { > + description: "List of properties to delete.", > + type: Array, > + optional: true, > + items: { > + type: DeletableProperty, > + } > + }, > + digest: { > + optional: true, > + schema: PROXMOX_CONFIG_DIGEST_SCHEMA, > + }, > + }, > + }, > + returns: { type: AdRealmConfig }, > + access: { > + permission: &Permission::Privilege(&["access", "domains"], PRIV_= REALM_ALLOCATE, false), > + }, > +)] this one here might check on /access/domains/{realm} as well - but the same caveat as above applies, ideally we'd change that together with the LDAP one at least. > +/// Update an AD realm configuration > +pub async fn update_ad_realm( > + realm: String, > + update: AdRealmConfigUpdater, > + password: Option, > + delete: Option>, > + digest: Option, > + _rpcenv: &mut dyn RpcEnvironment, > +) -> Result<(), Error> { > + let domain_config_lock =3D domains::lock_config()?; > + > + let (mut domains, expected_digest) =3D domains::config()?; > + > + if let Some(ref digest) =3D digest { > + let digest =3D <[u8; 32]>::from_hex(digest)?; > + crate::tools::detect_modified_configuration_file(&digest, &expec= ted_digest)?; > + } > + > + let mut config: AdRealmConfig =3D domains.lookup("ad", &realm)?; > + >=20 > [..] > > + domains::save_config(&domains)?; > + > + Ok(()) > +} > + > +async fn retrieve_default_naming_context(ldap_config: &LdapConfig) -> Re= sult { > + let conn =3D Connection::new(ldap_config.clone()); > + match conn.retrieve_root_dse_attr("defaultNamingContext").await { > + Ok(base_dn) if !base_dn.is_empty() =3D> Ok(base_dn[0].clone()), > + Ok(_) =3D> bail!("server did not provide `defaultNamingContext`"= ), > + Err(err) =3D> bail!("failed to determine base_dn: {err}"), > + } > +} > + > +const ITEM_ROUTER: Router =3D Router::new() > + .get(&API_METHOD_READ_AD_REALM) > + .put(&API_METHOD_UPDATE_AD_REALM) > + .delete(&super::ldap::API_METHOD_DELETE_LDAP_REALM); this seems a bit weird - as in - why doesn't that endpoint check that it's actually being passed an LDAP realm? > + > +pub const ROUTER: Router =3D Router::new() > + .get(&API_METHOD_LIST_AD_REALMS) > + .post(&API_METHOD_CREATE_AD_REALM) > + .match_all("realm", &ITEM_ROUTER);