[pve-devel] [PATCH access-control 1/2] api: domains: add off-by-default `check-connection` parameter

[Lukas Wagner]

On Thu Jul 27, 2023 at 3:33 PM CEST, Christoph Heiss wrote:
> Removes the dreaded DN regex, instead introducing a connect/bind check
> on creation/update, aligning it with the way PBS does it. This is
> enabled by default for new realms, but opt-in for existing, to not break
> backwards-compatibility.
>
> Additionally, it has the benefit that instead of letting a sync fail on
> the first try due to e.g. bad bind credentials, it gives the user some
> direct feedback when trying to add/update a LDAP realm, if enabled.
>
> Should be rather a corner case, but it's the easiest way for us to
> accomodate and the most versatile for users needing this.
>

I think it would be enough to have the 'check-connection' parameter only for
the API call itself, I wouldn't store it in the domains.cfg configuration
file.

That would imply that the 'Check configuration' checkbox that you introduce in
the next patch could *always* be ticked, even for old realms. So whenever you
create/update an LDAP/AD realm configuration you have to explicitly tell it
"hey, I do not want the check right now, my LDAP server is down currently".

My main point in making the check behavior opt-in rather was so that
scripts/API consumers continue to work as before. For the GUI however, it
should be fine to just always check by default, unless the behavior is 
explicitly turned off.

> +	},
> +	'check-connection' => {
> +	    description => 'Check bind connection to LDAP server.',
> +	    type => 'boolean',
> +	    optional => 1,
> +	    # TODO: Make it enabled-by-default with PVE 9.0?
                ^ This wouldn't be necessary any more.
> +	    default => 0,
> +	},
>      };
>  }