fix: improve CLIENT_FILEPATH handling and reduce code duplication (#1390)

## Summary

Follow-up improvements to #962:

- **Fix `getClientOwner()` edge case**: Now verifies the user actually
exists via `id` command before attempting to set ownership. Previously
only checked if `/home/$client` directory existed, which could fail if
the directory exists but the user doesn't.

- **Add directory creation for custom paths**: When `CLIENT_FILEPATH`
points to a non-existent directory, the script now creates it
automatically with `mkdir -p`.

- **Reduce code duplication**: Extract the repeated filepath/permission
logic from `newClient()` and `renewClient()` into a new
`writeClientConfig()` helper function, removing ~30 lines of duplicated
code.
This commit is contained in:
Stanislas
2025-12-13 23:31:52 +01:00
committed by GitHub
parent 08f6f1e7cc
commit a220d3a689

View File

@@ -1549,7 +1549,7 @@ verb 3" >>/etc/openvpn/server/client-template.txt
# Helper function to get the home directory for storing client configs
function getHomeDir() {
local client="$1"
if [ -e "/home/${client}" ]; then
if [ -d "/home/${client}" ]; then
echo "/home/${client}"
elif [ "${SUDO_USER}" ]; then
if [ "${SUDO_USER}" == "root" ]; then
@@ -1565,7 +1565,8 @@ function getHomeDir() {
# Helper function to get the owner of a client config file (if client matches a system user)
function getClientOwner() {
local client="$1"
if [ -e "/home/${client}" ]; then
# Check if client name corresponds to an existing system user with a home directory
if id "$client" &>/dev/null && [ -d "/home/${client}" ]; then
echo "${client}"
elif [ "${SUDO_USER}" ] && [ "${SUDO_USER}" != "root" ]; then
echo "${SUDO_USER}"
@@ -1585,6 +1586,41 @@ function setClientConfigPermissions() {
fi
}
# Helper function to write client config file with proper path and permissions
# Usage: writeClientConfig <client_name>
# Uses CLIENT_FILEPATH env var if set, otherwise defaults to home directory
# Side effects: sets GENERATED_CONFIG_PATH global variable with the final path
function writeClientConfig() {
local client="$1"
local clientFilePath
# Determine output file path
if [[ -n "$CLIENT_FILEPATH" ]]; then
clientFilePath="$CLIENT_FILEPATH"
# Ensure parent directory exists for custom paths
local parentDir
parentDir=$(dirname "$clientFilePath")
if [[ ! -d "$parentDir" ]]; then
run_cmd_fatal "Creating directory $parentDir" mkdir -p "$parentDir"
fi
else
local homeDir
homeDir=$(getHomeDir "$client")
clientFilePath="$homeDir/$client.ovpn"
fi
# Generate the .ovpn config file
generateClientConfig "$client" "$clientFilePath"
# Set proper ownership and permissions if client matches a system user
local clientOwner
clientOwner=$(getClientOwner "$client")
setClientConfigPermissions "$clientFilePath" "$clientOwner"
# Export path for caller to use
GENERATED_CONFIG_PATH="$clientFilePath"
}
# Helper function to regenerate the CRL after certificate changes
function regenerateCRL() {
export EASYRSA_CRL_DAYS=$DEFAULT_CRL_VALIDITY_DURATION_DAYS
@@ -1845,26 +1881,11 @@ function newClient() {
log_success "Client $CLIENT added and is valid for $CLIENT_CERT_DURATION_DAYS days."
fi
# Determine output file path
local clientFilePath
if [[ -n "$CLIENT_FILEPATH" ]]; then
clientFilePath="$CLIENT_FILEPATH"
else
local homeDir
homeDir=$(getHomeDir "$CLIENT")
clientFilePath="$homeDir/$CLIENT.ovpn"
fi
# Generate the .ovpn config file
generateClientConfig "$CLIENT" "$clientFilePath"
# Set proper ownership and permissions if client matches a system user
local clientOwner
clientOwner=$(getClientOwner "$CLIENT")
setClientConfigPermissions "$clientFilePath" "$clientOwner"
# Write the .ovpn config file with proper path and permissions
writeClientConfig "$CLIENT"
log_menu ""
log_success "The configuration file has been written to $clientFilePath."
log_success "The configuration file has been written to $GENERATED_CONFIG_PATH."
log_info "Download the .ovpn file and import it in your OpenVPN client."
exit 0
@@ -1921,27 +1942,12 @@ function renewClient() {
# Regenerate the CRL
regenerateCRL
# Determine output file path
local clientFilePath
if [[ -n "$CLIENT_FILEPATH" ]]; then
clientFilePath="$CLIENT_FILEPATH"
else
local homeDir
homeDir=$(getHomeDir "$CLIENT")
clientFilePath="$homeDir/$CLIENT.ovpn"
fi
# Regenerate the .ovpn file with the new certificate
generateClientConfig "$CLIENT" "$clientFilePath"
# Set proper ownership and permissions if client matches a system user
local clientOwner
clientOwner=$(getClientOwner "$CLIENT")
setClientConfigPermissions "$clientFilePath" "$clientOwner"
# Write the .ovpn config file with proper path and permissions
writeClientConfig "$CLIENT"
log_menu ""
log_success "Certificate for client $CLIENT renewed and is valid for $client_cert_duration_days days."
log_info "The new configuration file has been written to $clientFilePath."
log_info "The new configuration file has been written to $GENERATED_CONFIG_PATH."
log_info "Download the new .ovpn file and import it in your OpenVPN client."
}