nextcloud / server

☁️ Nextcloud server, a safe home for all your data
https://nextcloud.com
GNU Affero General Public License v3.0
27.22k stars 4.05k forks source link

Allow arbitrary configID prefixes for user_ldap #45144

Open ibizaman opened 5 months ago

ibizaman commented 5 months ago

How to use GitHub

Is your feature request related to a problem? Please describe. When creating a user_ldap config, I cannot choose the configID. This is an issue when automating the configuration of the LDAP app through the CLI.

Indeed, I want to be able to deploy my server with Nextcloud's LDAP app configured through code in an idempotent fashion. The code cannot just call occ ldap:create-empty-config on each deployment.

Describe the solution you'd like I'd like to be able to call ldap:create-empty-config with a prefix argument and it would create the config with the given prefix or fail if the prefix already exists.

Describe alternatives you've considered Instead of using the ldap:* CLI tool, I can use the config:app:set user_ldap CLI tool. Indeed, I can create an empty LDAP config with an arbitrary configID with:

occ config:app:set user_ldap s01ldap_configuration_active --value=1

This works but it's not discoverable.

Note also that this way allows to create any prefix, even a non-numerical one, causing havoc when later on you use the CLI tool to generate a new config because of a type-cast to int.

Additional context Currently, I circumvent this issue with a brittle implementation. To avoid creating a new LDAP config on every deploy, the code enumerates all configIDs and chooses to update a configID whose ldapHost == 127.0.0.1. If no such configID is found, it then calls .

ALL_CONFIG="$(${occ} ldap:show-config --output=json)"

MATCHING_CONFIG_IDs="$(echo "$ALL_CONFIG" | jq '[to_entries[] | select(.value.ldapHost=="127.0.0.1") | .key]')"
if [[ $(echo "$MATCHING_CONFIG_IDs" | jq 'length') > 0 ]]; then
    CONFIG_ID="$(echo "$MATCHING_CONFIG_IDs" | jq --raw-output '.[0]')"
else
    CONFIG_ID="$(${occ} ldap:create-empty-config --only-print-prefix)"
fi

Btw I'm fine creating a PR for this but wanted first to agree on the general feature and implementation.

susnux commented 5 months ago

CC @nextcloud/ldap

blizzz commented 5 months ago

Imo it's fine to extend ldap:create-empty-config to specify a certain prefix in the 's%02d' format. I'd welcome a PR adding this feature.