batch add users to nextcloud on docker with csv

Question

I have a working script that will batch add users to an instance of nextcloud running on top of docker. This version is the result of changes made after asking this question.

I guess I'm looking for a re-review to ensure that I haven't missed anything. I'm also interested if there are areas where I could make things better in terms of readability, performance, security, etc. You're the reviewer. My code is your oyster.

#!/bin/sh

set -eu

# Handle printing errors
die () {
  printf '%s\n' "$1" >&2
  exit 1
}

message () {
  printf '%s\n' "$1" >&2
}

usage () {
  cat <<'EOF'
NAME
      batch_up - batch upload users

SYNOPSIS
      batch_up -?
      batch_up -h
      batch_up --help
      batch_up -v
      batch_up -vv
      batch_up -vvv
      batch_up --verbose
      batch_up -d
      batch_up --delimiter
      batch_up --delimiter=

DESCRIPTION
      Batch_up is a program that adds a batch of users to an instance of NextCloud
      running inside of a docker container by reading a comma separated list from 
      stdin or a csv file. Stdin is the default and will be used if no file is 
      supplied. The delimiter does not have to be a comma, and can be set via the
      -d flag.

      CSV file should be formatted in one of the following configurations:
      username,Display Name,group,[email protected]
      username,Display Name,group
      username,Display Name
      username

      CSV files should not include the header.

OPTIONS
      -? or -h or --help
            This option displays a summary of the commands accepted by batch_up.

      -v or -vv or -vvv or --verbose
            This option sets the verbosity. Each v adds to the verbosity.
            See the occ command for OwnCloud/NextCloud for more details as this
            option is passed on to the occ command.

            If this option is not passed, the default behavior is to pass the
            -q option to occ (the quiet option), making occ non-verbose.

      -d or --delimiter or --delimiter=
            This option allows you to choose which delimiter you would like to use.
            The following are acceptable characters for this option. ,.;:|

ENVIRONMENT VARIABLES
      OC_PASS
            Sets the password for users added. The OC_PASS environment variable
            is required.

EXAMPLES
      The command:
            batch_up.sh foobar.csv
            Will add the users from foobar.csv. Users will be given the password
            in the OC_PASS environment variable. 

      The command:
            batch_up.sh <<< "jack,Jack Frost,users,[email protected]"
            Will add the user jack, set his display name to Jack Frost, add him
            to the group users, and set his email to [email protected].

      The command:
            echo "jack,Jack Frost,users,[email protected]" | batch_up.sh
            Will add the user jack, set his display name to Jack Frost, add him
            to the group users, and set his email to [email protected].

      The command:
             batch_up.sh -d : foobar.csv
            Will set the delimiter to : and add the users from foobar.csv.

EOF
}


# Set verbosity. Default is silent.
verbose=-q

# flags
while [ $# -gt 0 ]
do
  case $1 in
    -h|-\?|--help)
      usage # Display a usage synopsis.
      exit
      ;;
    -v|-vv|-vvv|--verbose)
      verbose=$1
      ;;
    -d|--delimiter)
      if [ $# -gt 1 ]
      then
        case $2 in
          ,|.|\;|:|\|)
            delimiter=$2
            shift
            ;;
          *)
            die 'Error: delimiter should be one of ,.;:| characters.'
        esac 
      else
        die 'Error: "--delimiter requires a non-empty option argument.'
      fi
      ;;
    --delimiter=?*)
      case ${1#*=} in # Delete everything up to the = and assign the remainder.
        ,|.|\;|:|\|)
          delimiter=${1#*=}
          shift
          ;;
        *)
          die 'Error: delimiter should be one of ,.;:| characters.'
      esac 
      ;;
    --delimiter=) # Handle the case of empty --delimiter=
      die 'Error: "--delimiter=" requires a non-empty option argument.'
      ;;
    --)
      shift
      break
      ;;
    -?*)
      printf 'WARN: Unknown option (ignored): %s\n' "$1" >&2
      ;;
    *) # Default case. No more options, so break out of the loop
      break
  esac

  shift
done

# Is the file readable?
if [ $# -gt 0 ] 
then
  [ -r "$1" ] || die "$1: Couldn't read file."
fi

# Is the OC_PASS environment variable set?
[ ${OC_PASS:-} ] || die "$0: No password specified. Run with --help for more info."

status=true                     # until a command fails

message 'Adding users'

while IFS=${delimiter:-,} read -r f1 f2 f3 f4
do
  if [ "$f1" ]
  then
    docker-compose exec -T -e OC_PASS --user www-data app php occ \
      user:add --password-from-env \
      ${verbose:+"$verbose"} \
      ${f2:+"--display-name=$f2"} \
      ${f3:+"--group=$f3"} \
      "$f1" </dev/null \
      || status=false

    # If there is a fourth value in the csv, use it to set the user email.
    if [ "$f4" ]
    then
      docker-compose exec -T \
        --user www-data app php occ \
        user:setting "$f1" settings email "$f4" \
        </dev/null \
        || status=false
    fi
  else
    echo "Expected at least one field, but none were supplied." >&2
    status=false
    continue
  fi
message '...'
done <"${1:-/dev/stdin}"

message 'Done'

exec $status

Answer 1

Don't repeat yourself

There is redundant logic in the processing of the -d and --delimiter=... options. I would eliminate that, to be something more like this:

-d|--delimiter)
  [ $# -gt 1 ] && arg=$1 || arg=
  parseDelimiter "$arg"
  ;;
--delimiter=*)
  parseDelimiter "${1#*=}"
  ;;

Where parseDelimiter is a function that will parse the passed argument and set delimiter appropriately, or exit with an error.

A simpler pattern in case (maybe)

Instead of ,|.|\;|:|\|), perhaps [,.\;:\|]) is slightly simpler and easier to type (less likely you mistype something, for example less likely to forget a | between the different allowed values).

Assign positional arguments to descriptive variables early on

Near the end of the script I see done <"${1:-/dev/stdin}", and wonder: what was $1 again? I think it's easier to understand when positional arguments are assigned to variables with descriptive names early on in a script.

Prefer comments on their own lines

I read code from top to bottom, and this line makes my eyes make an unnecessary detour to the right:

status=true                     # until a command fails

I would have preferred that comment on its own line just before the code it's referring to.

Incidentally, I don't understand what this comment is trying to say. The status variable is used to determine the exit status of the program. In this script, it will be "failure" if some failure happened in the loop body, otherwise "success".

I would have named this exit_status, and then at the end of the script do exit "$exit_status", rather than the current exec $status, which I find a bit odd.