Re: [pbs-devel] [PATCH proxmox-backup 07/12] api: access: add routes for managing AD realms

[Christoph Heiss <c.heiss@proxmox.com>]

Thanks for the review!

On Wed, Aug 09, 2023 at 12:12:25PM +0200, Lukas Wagner wrote:
>
> On Tue Aug 8, 2023 at 2:22 PM CEST, Christoph Heiss wrote:
> > +/// Update an AD realm configuration
> > +pub async fn update_ad_realm(
> > +    realm: String,
> > +    update: AdRealmConfigUpdater,
> > +    password: Option<String>,
> > +    delete: Option<Vec<DeletableProperty>>,
> > +    digest: Option<String>,
> > +    _rpcenv: &mut dyn RpcEnvironment,
> > +) -> Result<(), Error> {
> (...)
> > + [..]
> > +    let conn = Connection::new(ldap_config);
> > +    proxmox_async::runtime::block_on(conn.check_connection()).map_err(|e| format_err!("{e:#}"))?;
> We are already in an async function, so we should be able to use .await the
> future? Unless I'm missing something.
Yeah, you're right. Seems I forgot about that when I made that function
async while adding the retrieve_default_naming_context() call. I'll fix
that up for v2.

>
> > +
> > +    if let Some(password) = password {
> > +        auth_helpers::store_ldap_bind_password(&realm, &password, &domain_config_lock)?;
> > +    }
> > +
> > +    domains.set_data(&realm, "ad", &config)?;
> > +
> > +    domains::save_config(&domains)?;
> > +
> > +    Ok(())
> > +}
>
> General remark regarding the update-handler: You are missing the
> 'case-sensitive' parameter, so updating that parameter does not work.
Good catch, thanks! I'll fix that up too.