1

I'm trying to loop through 2 groups on macOS and remove users in the admin group if they don't exist in another group.

newadmins=$(dscl . -read Groups/newadmin GroupMembership | cut -c 18-)
adminUsers=$(dscl . -read Groups/admin GroupMembership | cut -c 18-)

for (user in $adminUsers && ! user in $newadmins)
do
        dseditgroup -o edit -d $user -t user admin
        if [ $? = 0 ]; then echo "Removed user $user from admin group"; fi
    else
        echo "Admin user $user left alone"
    fi
done

The above didn't work. I think I'm confusing shell with other languages. Any help would be appreciated. Thank!

The below script worked exactly as expected:

NEW_ADMIN_USERS=$(dscl . -read Groups/newadmin GroupMembership | cut -d ' ' -f 2-)
ADMIN_USERS=$(dscl . -read Groups/admin GroupMembership | cut -d ' ' -f 2-)

DEFUNCT_ADMIN_USERS=$(grep -vxFf <(echo ${NEW_ADMIN_USERS} | tr ' ' '\n') <(echo ${ADMIN_USERS} | tr ' ' '\n'))

for DEFUNCT_ADMIN_USER in ${DEFUNCT_ADMIN_USERS}
do
    if dseditgroup -o edit -d ${defunct_admin_user} -t user admin
  then
    echo "Removed user ${DEFUNCT_ADMIN_USER} from admin group"
  else
    echo "Admin user ${DEFUNCT_ADMIN_USER} left alone"
  fi  
done

Thanks @msbit for all the help!

Duetschpire
  • 167
  • 11

1 Answers1

1

I would consider doing something like this:

#!/usr/bin/env bash

set -eu

NEW_ADMIN_USERS=$(dscl . -read Groups/newadmin GroupMembership | cut -d ' ' -f 2-)
ADMIN_USERS=$(dscl . -read Groups/admin GroupMembership | cut -d ' ' -f 2-)

DEFUNCT_ADMIN_USERS=$(grep -vxFf <(echo ${NEW_ADMIN_USERS} | tr ' ' '\n') <(echo ${ADMIN_USERS} | tr ' ' '\n'))

for DEFUNCT_ADMIN_USER in ${DEFUNCT_ADMIN_USERS}
do
  if dseditgroup -o edit -d ${DEFUNCT_ADMIN_USER} -t user admin
  then
    echo "Removed user ${DEFUNCT_ADMIN_USER} from admin group"
  else
    echo "Admin user ${DEFUNCT_ADMIN_USER} left alone"
  fi  
done

The main thrust of this is using the grep command put forward by @Jetchisel with process substitution (<()) to prepare a list of admin users in the ADMIN_USERS variable but not in the NEW_ADMIN_USERS variable, then iterating over that variable.

This departs from your approach in a number of ways:

  • setting the errexit and nounset options which will cause the script to exit on any error code from a command, including use of unset variables (set -eu)
  • using the field argument of cut with delimiter set to space when parsing the output of dscl (cut -d ' ' -f 2-)
  • subsequently splitting the list of users into lines with tr (tr ' ' '\n')
  • passing the list through to for as appropriate (using ( was a syntax error, as I suspect the use of ! would be)
  • evaluating the return code of dseditgroup directly as that is what if is testing for
  • removing the trailing fi for the first if command, as it's not needed when you have the else (and would cause a syntax error due to an apparent floating else)

Please test thoroughly, preferably with a dummy command instead of dseditgroup before you're 100% happy that this works as expected, and consider setting the xtrace option (set -x which will echo all the commands as they are executed), while developing.

msbit
  • 4,152
  • 2
  • 9
  • 22
  • this is turning out nicer than I thought... DEFUNCT_ADMIN_USER is returning the correct result, all users who are in ADMIN_USERS but are not in NEW_ADMIN_USERS. But when I run the do it's removing the users who are in both groups – Duetschpire Nov 12 '21 at 05:02
  • _If_ `${DEFUNCT_ADMIN_USERS}` contains the correct values, then it's worth checking that `dseditgroup` behaves the way you expect, and also worth double checking that the command is being executed as you'd expect (via `set -x`) – msbit Nov 12 '21 at 05:27
  • I think I see the problem, and I've updated the answer. Basically, replacing the spaces with newlines before storing in the `*_ADMIN_USERS` variables wasn't correct, as `echo` would helpfully print them all out as one line. I've moved that replacement back into the process substitution command. This appears to provide the correct output for `${DEFUNCT_ADMIN_USERS}`, but please test to confirm. – msbit Nov 12 '21 at 05:38
  • Yes, this output is better. I can see now that it won't attempt to remove the users in the ADMIN_USERS. I've changed the command to remove user to: if dscl . -delete /Groups/admin GroupMembership ${DEFUNCT_ADMIN_USER} while this works if I run it on a single user, it doesn't seem to work in the loop. Could it be something to do with formatting the output still? Output (none of the below are in the admin group, I think it's treating all users as a single line): Record was not found. Admin user user15 user20 user21 user22 user25 left alone – Duetschpire Nov 12 '21 at 06:05
  • 1
    Thank you so much for your help! I figured the rest out. I was using `#!bin/zsh` and some functions aren't supported. `#!bin/bash` and the `dseditgroup` command worked perfectly fine. I will add the final script to the main thread for reference. – Duetschpire Nov 12 '21 at 06:27
  • @msbit: Wouldn't it be clearer (only from the viewpoint of readability and program maintenance) if you made `DEFUNCT_ADMIN_USERS` an array? After all it holds a collection of users. Of course in your case a simple string variable works too, because by construction, the elements (user names) can not contain spaces. – user1934428 Nov 12 '21 at 08:01
  • @user1934428 I lean towards no. For the reasons you've outlined, creating any of the variables is an exercise in string manipulation (`cut` or `grep` respectively), so creating them as arrays would add extra noise in the use case (addition of `[@]` to each) when the primary use case is in a `for` loop - a particularly well known bash-ism for iterating over a single variable containing elements separated by whitespace. But, if you'd like to explore the option for yourself, feel free to add an answer ✌️ – msbit Nov 12 '21 at 09:03
  • Get out of the habit of using ALLCAPS variable names, leave those as reserved by the shell. One day you'll write `PATH=something` and then [wonder why](https://stackoverflow.com/q/27555060/7552) your [script is broken](https://stackoverflow.com/q/28310594/7552). – glenn jackman Nov 12 '21 at 16:12
  • @glennjackman, ha I never wonder why my script is broken when I abuse `PATH` :) Also never fail to fall into that trap through refactoring my scripts. – msbit Nov 12 '21 at 16:20