Shell syntax error near unexpected token `done'

Question

This shell script is supposed to add users to the system. The new users details are in a file. The shell is rejecting this script with the message:

syntax error near unexpected token 'done'.

What's wrong?

#!/bin/bash
#Purpose: Automatically add new users in a linux system based upon the data found within a text file
#         Assign encryped passwords to each user
#         Add users to groups or create new groups for these users
#         Report errors and successful operations where necessary in log files
#         post help options (echo)

#Root validation
if [[ $(id -u) -eq 0 ]]; then
  #Argument validation
  if [[ -z "$1" ]]; then
    echo "No arguments found!"
    echo "Please include a user detail text file as first argument"
    echo "Please include a report text file as second argument"
    echo "Please include an error report text file as the third argument"
    echo "Use the -h argument (i.e. ./script -h) for help"
  exit 1
fi

#Help validation and Help file
if [[ "$1" = "-h" ]]; then
  echo "This is the help information file"
  echo "This script is designed to add users to a linux system by reading information from a user detail file (such as userlist.txt)"
  echo "The format of this user detail text file is "username password groupname fullname" seperated using TAB spacing"
  echo "This script will read the first argument as the user detail file, the second and third arguments will be read as a success report file and error report file respectively"
  exit
fi

#Reads first argument as user detail file for data
cat userlist.txt | while read uname password gname fullname
#Reads /etc/passwd for Username
egrep -w "^$uname" /etc/passwd
#If Username is found then error reports
if [ $? == 0 ]; then
  echo "User Already Exists : Error adding user with username $uname;$gname;$fullname" >> Successes1.log
  exit 1
else
  #Reads /etc/group for Groupname
  egrep -w "^$gname" /etc/group
  #If Groupname is found then nothing
  if [ $? == 0 ]; then
    echo ""
  else
    #If Groupname not found then creates new group and reports
    groupadd "$gname"
    echo "Group Not Found: New Group $gname was created" >> Successes1.log
  fi
  #Retrieves Date
  createddate=$(date)
  #Perl password script takes input from Userlist
  pass=$(perl -e 'print crypt($ARGV[0], "Password")' "$password")
  #Adds Users with variables from userlist
  useradd "$uname" -g "$gname"  -c "$fullname" -p "$pass"
  #Reports information to successlist and errorlist report files
  if [ $? == 0 ]; then
    groupid=$(id -g $uname)
    userid=$(id -u $uname)
    echo "User Successfully Added: $uname;$userid;$gname;$groupid;$createddate;$fullname" >> Successes1.log
  else
    groupid=$(id -g $uname)
    userid=$(id -u $uname)
    echo "Useradd Error Occurred: $uname;$userid;$gname;$groupid;$createddate;$fullname" >> Errors1.log
    echo "Error: Must be root user to execute script"
    exit
  fi
done

---

Second attempt

Using some of the ideas from the answers, I came up with a second attempt:

#!/bin/bash
#Purpose: Automatically add new users in a linux system based upon the data found within a text file
#         Assign encryped passwords to each user
#         Add users to groups or create new groups for these users
#         Report errors and successful operations where necessary in log files
#         post help options (echo)

#Root validation
if [[ $(id -u) -eq 0 ]]; then
  #Argument validation
  if [[ -z "$1" ]]; then
    echo "Usage: $0 usernames report errors" 1>&2
    echo "Please include a user detail text file as first argument"
    echo "Please include a report text file as second argument"
    echo "Please include an error report text file as the third argument"
    echo "Use the -h argument (i.e. ./script -h) for help"
    exit 1
  fi
fi

#Help validation and Help file
if [[ "$1" = "-h" ]]; then
  echo "This is the help information file"
  echo "This script is designed to add users to a linux system by reading information from a user detail file (such as userlist.txt)"
  echo "The format of this user detail text file is "username password groupname fullname" seperated using TAB spacing"
  echo "This script will read the first argument as the user detail file, the second and third arguments will be read as a success report file and error report file respectively"
  exit
fi

#Reads first argument as user detail file for data
cat jan.txt | while read uname password gname fullname; do
#Reads /etc/passwd for Username
 egrep -w "^$uname:" /etc/passwd >/dev/null 2>&1
#If Username is found then error reports
if [ $? == 0 ] 
then
  echo "User Already Exists : Error adding user with username $uname;$gname;$fullname" >> Errors1.log

else
  #Reads /etc/group for Groupname
  egrep -w "^$gname" /etc/group
  #If Groupname is found then nothing
if [ $? == 0 ]; then
    echo ""
else
  #If Groupname not found then creates new group and reports
  groupadd "$gname"
    echo "Group Not Found: New Group $gname was created" >> Successes1.log
done < $1
#Retrieves Date
createddate=$(date)
#Perl password script takes input from Userlist
pass=$(perl -e 'print crypt($ARGV[0], "Password")' "$password")
#Adds Users with variables from userlist
useradd "$uname" -g "$gname"  -c "$fullname" -p "$pass"
#Reports information to successlist and errorlist report files
if [ $? == 0 ]
then
  groupid=$(id -g $uname)
  userid=$(id -u $uname)
  echo "User Successfully Added: $uname;$userid;$gname;$groupid;$createddate;$fullname" >> Successes1.log
else
  groupid=$(id -g $uname)
  userid=$(id -u $uname)
  echo "Useradd Error Occurred: $uname;$userid;$gname;$groupid;$createddate;$fullname" >> Errors1.log
  echo "Error: Must be root user to execute script"
  exit 1
fi
fi
done

This does not seem to work properly either. What's wrong now? Seems to show arguments and runs however no users nor group have been added therefore no logs have been created

Answer 1

The if starting at:

if [ $? == 0 ]; then
  echo "User Already Exists : Error adding user with username ...
  exit 1
else

is ended with the done instead of the fi that is required.

The while loop starting a couple of lines earlier:

cat userlist.txt | while read uname password gname fullname

is missing its do (another bug); if that was present, then it would also need the done at the end. Someone lost track of the indentation. (Using 2 characters per level is better than 0 or 1, but it is easier to track levels if you use 4 spaces per level.) Note that the shell hasn't gotten around to complaining about the lack of do because the syntax for a while loop is:

while cmd1
      cmd2
      cmd3 ...
do

and as far as the shell is concerned, it is still processing commands in the list cmd1, cmd2, cmd3, ....

---

Here's a semi-decently indented version of the script. There was a missing fi at the top of the script, too.

#!/bin/bash
#Purpose: Automatically add new users in a linux system based upon the data found within a text file
#         Assign encryped passwords to each user
#         Add users to groups or create new groups for these users
#         Report errors and successful operations where necessary in log files
#         post help options (echo)

#Root validation
if [[ $(id -u) -eq 0 ]]
then
    #Argument validation
    if [[ -z "$1" ]]
    then
        echo "No arguments found!"
        echo "Please include a user detail text file as first argument"
        echo "Please include a report text file as second argument"
        echo "Please include an error report text file as the third argument"
        echo "Use the -h argument (i.e. ./script -h) for help"
        exit 1
    fi
fi

#Help validation and Help file
if [[ "$1" = "-h" ]]
then
    echo "This is the help information file"
    echo "This script is designed to add users to a linux system by reading information from a user detail file (such as userlist.txt)"
    echo "The format of this user detail text file is "username password groupname fullname" seperated using TAB spacing"
    echo "This script will read the first argument as the user detail file, the second and third arguments will be read as a success report file and error report file respectively"
    exit
fi

#Reads first argument as user detail file for data
cat userlist.txt | while read uname password gname fullname
do
    #Reads /etc/passwd for Username
    egrep -w "^$uname" /etc/passwd
    #If Username is found then error reports
    if [ $? == 0 ]
    then
        echo "User Already Exists : Error adding user with username $uname;$gname;$fullname" >> Successes1.log
        exit 1
    else
        #Reads /etc/group for Groupname
        egrep -w "^$gname" /etc/group
        #If Groupname is found then nothing
        if [ $? == 0 ]
        then
            echo ""
        else
            #If Groupname not found then creates new group and reports
            groupadd "$gname"
            echo "Group Not Found: New Group $gname was created" >> Successes1.log
        fi
        #Retrieves Date
        createddate=$(date)
        #Perl password script takes input from Userlist
        pass=$(perl -e 'print crypt($ARGV[0], "Password")' $pass)
        #Adds Users with variables from userlist
        useradd "$uname" -g "$gname"  -c "$fullname" -p "$pass"
        #Reports information to successlist and errorlist report files
        if [ $? == 0 ]
        then
            groupid=$(id -g $uname)
            userid=$(id -u $uname)
            echo "User Successfully Added: $uname;$userid;$gname;$groupid;$createddate;$fullname" >> Successes1.log
        else
            groupid=$(id -g $uname)
            userid=$(id -u $uname)
            echo "Useradd Error Occurred: $uname;$userid;$gname;$groupid;$createddate;$fullname" >> Errors1.log
            echo "Error: Must be root user to execute script"
            exit
        fi
    fi
done

---

There is still much room for improvement. The root validation block should exit if the user is not root; that happens instead a mile further down inside the loop. You can check the number of arguments better: $# gives you the number of arguments. If I tried yourscript.sh '' arg2 arg3, you'd claim there were no arguments when in fact the problem is that $1 is present but is an empty string. The standard convention for reporting how to use a command is something like:

echo "Usage: $0 usernames report errors" 1>&2

This reports the command's name, and the arguments expected. The 1>&2 sends the message to standard error instead of standard output. The logic here is a little bizarre even so. You check that the user is root and only then check that there are arguments. If the user is not root, you don't check the arguments. Not entirely sensible, I submit.

We can debate the UUOC (Useless Use of Cat). There's actually an award for it; I don't think this qualifies. However, it would be possible to write:

while read uname password gname fullname
do
    ...
done < $1

Hmmm...the script is supposed to take a file name argument that specifies the users, but the cat takes a fixed file name, not the file name argument!

Similarly, arguments 2 and 3 are studiously ignored; the log files are hard-coded.

egrep -w "^$uname" /etc/passwd
#If Username is found then error reports
if [ $? == 0 ]

This fragment can be improved several ways:

if egrep -w "^$uname:" /etc/passwd >/dev/null 2>&1
then
    #If Username is found then error report

This tests the exit status of the egrep command directly; it also prevents a new user roo from being treated as pre-existing because of user root. It sends the output and error output to /dev/null so that you won't see anything when the user does exist.

It might be better not to exit when the user name is found; you could at least try to process the next entry. It is also odd that the report that the user exists (which terminates the processing) is recorded in Successes1.log rather than in Errors1.log; it is treated like an error.

The group check constructs are similar and should be similarly upgraded.

You read the password into $password with the read line; when it comes to creating the password, though, you have:

pass=$(perl -e 'print crypt($ARGV[0], "Password")' $pass)

On the first cycle, $pass is empty (most probably); you should have used $password in double quotes at the end:

pass=$(perl -e 'print crypt($ARGV[0], "Password")' "$password")

As with the egrep commands, you can check the status of the useradd command directly too. It is a bit sweeping to say if [ $? == 0 ] is the mark of a tyro, but it isn't too far off the truth.

The final exit in the script should be exit 1 to indicate an error exit. As already noted, this is preceded by the comment about 'you must be root', even though there was a check at the top for root privileges.

Caveat: I've not attempted to run the script; I could easily have missed some issues. It does, however, pass sh -v -n, so there are no gross syntactic errors left.

Once a shell script is syntactically correct, then you normally debug it using sh -x script (or, if it takes arguments, then sh -x script arg1 arg2 arg3 ...). This is the execution trace mode. The shell tells you, more or less inscrutably, what it is doing. The information is written to standard error. You can even trap the output for later scrutiny if you like with:

sh -x script arg1 arg2 arg3 2>script-x.log

The 2>script-x.log notation sends the standard error to the file script-x.log (choose your own meaningful name; I often use x or xxx for files I won't want to keep, but I also remove such files without necessarily even looking at them because I know they are throwaway files).

Answer 2

Your while is missing a do:

cat userlist.txt | while read uname password gname fullname
do 

or

cat userlist.txt | while read uname password gname fullname; do

Answer 3

"done" is supposed to be paired with a previous "do" It does not man th end of your script.