fix: various sync-ad-objects fixes (#456)

* fix: DIRECTORY_NAME variable usage

* fix: yaml typo

* fix: changes to user sync

* fix: only manage yaml defined groups, not default  groups

* chore: add more echos for clarity

.github/workflows/mis-ad-user-group-sync.yml

@@ -27,7 +27,7 @@ jobs:
 
     runs-on: ubuntu-latest
     environment: delius-mis-${{ matrix.env_name }}-preapproved
-    
+
     steps:
       - name: Checkout repository
         uses: actions/[email protected]
@@ -44,7 +44,7 @@ jobs:
         run: |
           DIRECTORY_NAME="delius-mis-${{ matrix.env_name }}"
           DIRECTORY_ID=$(aws ds describe-directories \
-            --query "DirectoryDescriptions[?Name=='${{ env.DIRECTORY_NAME }}.internal'].DirectoryId" \
+            --query "DirectoryDescriptions[?Name=='$DIRECTORY_NAME.internal'].DirectoryId" \
             --output text)
 
           if [ -z "$DIRECTORY_ID" ]; then
@@ -61,12 +61,14 @@ jobs:
           yaml_groups=$(yq e '.groups[]' mis-directory/mis-users.yml | sort)
 
           # Get list of existing groups from AD
+          echo "Fetching existing AD groups..."
           existing_groups=$(aws ds-data list-groups \
             --directory-id ${{ steps.directory.outputs.directory_id }} \
             --query 'Groups[].SAMAccountName' \
             --output text | tr '\t' '\n' | sort)
 
           # Create new groups
+          echo "Processing groups..."
           for group in $yaml_groups; do
             if ! echo "$existing_groups" | grep -q "^${group}$"; then
               echo "Creating group: $group"
@@ -75,34 +77,47 @@ jobs:
                 --sam-account-name "$group" \
                 --group-scope "Global" \
                 --group-type "Security"
+            else
+              echo "Group already exists: $group"
             fi
           done
 
       - name: Sync Users
         run: |
-          # Get list of existing users from AD
+          # Get list of all users from AD first
+          echo "Fetching existing AD users..."
           existing_users=$(aws ds-data list-users \
             --directory-id ${{ steps.directory.outputs.directory_id }} \
-            --query 'Users[].SAMAccountName' \
-            --output text | tr '\t' '\n' | sort)
+            --query 'Users[].{username:SAMAccountName,enabled:Enabled}' \
+            --output json)
+
+          # Get users to be disabled
+          disabled_users=$(yq e '.disabled_users[]' mis-directory/mis-users.yml | jq -R -s 'split("\n") | map(select(length > 0))')
 
-          # Create/Update users
-          yq e '.users[]' mis-directory/mis-users.yml | while read -r user; do
-            username=$(echo "$user" | yq e '.username' -)
-            firstname=$(echo "$user" | yq e '.first_name' -)
-            lastname=$(echo "$user" | yq e '.last_name' -)
-            email=$(echo "$user" | yq e '.email' -)
+          # Create/Update active users
+          echo "Processing active users..."
+          active_users=$(yq eval-all -o=json mis-directory/mis-users.yml | \
+            jq -c '.users[] | {username:.username, first_name:.first_name, last_name:.last_name, email:.email}')
 
-            if ! echo "$existing_users" | grep -q "^${username}$"; then
-              echo "Creating user: $username"
+          echo "$active_users" | while read -r user_json; do
+            username=$(echo "$user_json" | jq -r '.username')
+            firstname=$(echo "$user_json" | jq -r '.first_name')
+            lastname=$(echo "$user_json" | jq -r '.last_name')
+            email=$(echo "$user_json" | jq -r '.email')
+
+            # Check if user exists in AD
+            user_exists=$(echo "$existing_users" | jq -r --arg uname "$username" '.[] | select(.username == $uname) | .username')
+
+            if [ -z "$user_exists" ]; then
+              echo "Creating new user: $username"
               aws ds-data create-user \
                 --directory-id ${{ steps.directory.outputs.directory_id }} \
                 --sam-account-name "$username" \
                 --given-name "$firstname" \
                 --surname "$lastname" \
                 --email-address "$email"
             else
-              echo "Updating user: $username"
+              echo "Updating existing user: $username"
               aws ds-data update-user \
                 --directory-id ${{ steps.directory.outputs.directory_id }} \
                 --sam-account-name "$username" \
@@ -113,74 +128,87 @@ jobs:
             fi
           done
 
-          # Process disabled users
-          yq e '.disabled_users[]' mis-directory/mis-users.yml | while read -r username; do
-            if [ ! -z "$username" ]; then
-              echo "Checking disabled user: $username"
-              # Check if user exists before trying to disable
-              if aws ds-data describe-user \
+          # Process users to be disabled
+          echo "Processing users to be disabled..."
+          echo "$disabled_users" | jq -r '.[]' | while read -r username; do
+            # Check if user exists and is enabled in AD
+            is_enabled=$(echo "$existing_users" | jq -r --arg uname "$username" '.[] | select(.username == $uname) | .enabled')
+
+            if [ "$is_enabled" = "true" ]; then
+              echo "Disabling user: $username"
+
+              # Remove from all groups
+              current_groups=$(aws ds-data list-groups-for-member \
                 --directory-id ${{ steps.directory.outputs.directory_id }} \
-                --sam-account-name "$username" 2>/dev/null; then
-                echo "Disabling user: $username"
-
-                # First remove from all groups
-                current_groups=$(aws ds-data list-groups-for-member \
-                  --directory-id ${{ steps.directory.outputs.directory_id }} \
-                  --sam-account-name "$username" \
-                  --query 'Groups[].SAMAccountName' \
-                  --output text | tr '\t' '\n' || echo "")
-
-                for group in $current_groups; do
-                  if [ ! -z "$group" ]; then
-                    echo "Removing $username from group: $group"
-                    aws ds-data remove-group-member \
-                      --directory-id ${{ steps.directory.outputs.directory_id }} \
-                      --group-name "$group" \
-                      --member-name "$username"
-                  fi
-                done
-
-                # Then disable the user
-                aws ds-data disable-user \
-                  --directory-id ${{ steps.directory.outputs.directory_id }} \
-                  --sam-account-name "$username"
-              fi
+                --sam-account-name "$username" \
+                --query 'Groups[].SAMAccountName' \
+                --output text | tr '\t' '\n' || echo "")
+
+              for group in $current_groups; do
+                if [ ! -z "$group" ]; then
+                  echo "Removing $username from group: $group"
+                  aws ds-data remove-group-member \
+                    --directory-id ${{ steps.directory.outputs.directory_id }} \
+                    --group-name "$group" \
+                    --member-name "$username"
+                fi
+              done
+
+              # Disable the user
+              aws ds-data disable-user \
+                --directory-id ${{ steps.directory.outputs.directory_id }} \
+                --sam-account-name "$username"
+            elif [ ! -z "$is_enabled" ]; then
+              echo "User $username is already disabled"
+            else
+              echo "User $username not found in AD"
             fi
           done
 
       - name: Sync Group Memberships
         run: |
-          # Process each user's group memberships
-          yq e '.users[]' mis-directory/mis-users.yml | while read -r user; do
-            username=$(echo "$user" | yq e '.username' -)
-            yaml_groups=$(echo "$user" | yq e '.groups[]' - | sort)
+          # Get list of managed groups from YAML
+          managed_groups=$(yq e '.groups[]' mis-directory/mis-users.yml | jq -R -s 'split("\n") | map(select(length > 0))')
+
+          # Process each active user's group memberships
+          active_users=$(yq eval-all -o=json mis-directory/mis-users.yml | jq -c '.users[]')
+
+          echo "$active_users" | while read -r user_json; do
+            username=$(echo "$user_json" | jq -r '.username')
+            yaml_groups=$(echo "$user_json" | jq -r '.groups[]' 2>/dev/null || echo "")
 
             # Get user's current groups from AD
             existing_groups=$(aws ds-data list-groups-for-member \
               --directory-id ${{ steps.directory.outputs.directory_id }} \
               --sam-account-name "$username" \
               --query 'Groups[].SAMAccountName' \
-              --output text | tr '\t' '\n' | sort || echo "")
-
-            # Add user to new groups
-            echo "$yaml_groups" | while read -r group; do
-              if [ ! -z "$group" ] && ! echo "$existing_groups" | grep -q "^${group}$"; then
-                echo "Adding $username to group: $group"
-                aws ds-data add-group-member \
-                  --directory-id ${{ steps.directory.outputs.directory_id }} \
-                  --group-name "$group" \
-                  --member-name "$username"
+              --output text | tr '\t' '\n' || echo "")
+
+            # Add user to new groups (only from managed groups)
+            for group in $yaml_groups; do
+              # Check if this is a group we manage
+              if echo "$managed_groups" | jq -e --arg g "$group" 'contains([$g])' >/dev/null; then
+                if ! echo "$existing_groups" | grep -q "^${group}$"; then
+                  echo "Adding $username to group: $group"
+                  aws ds-data add-group-member \
+                    --directory-id ${{ steps.directory.outputs.directory_id }} \
+                    --group-name "$group" \
+                    --member-name "$username"
+                fi
               fi
             done
 
-            # Remove user from groups they should no longer be in
-            echo "$existing_groups" | while read -r group; do
-              if [ ! -z "$group" ] && ! echo "$yaml_groups" | grep -q "^${group}$"; then
-                echo "Removing $username from group: $group"
-                aws ds-data remove-group-member \
-                  --directory-id ${{ steps.directory.outputs.directory_id }} \
-                  --group-name "$group" \
-                  --member-name "$username"
+            # Remove user from managed groups they should no longer be in
+            for group in $existing_groups; do
+              # Only process if it's a group we manage
+              if echo "$managed_groups" | jq -e --arg g "$group" 'contains([$g])' >/dev/null; then
+                if ! echo "$yaml_groups" | grep -q "^${group}$"; then
+                  echo "Removing $username from group: $group"
+                  aws ds-data remove-group-member \
+                    --directory-id ${{ steps.directory.outputs.directory_id }} \
+                    --group-name "$group" \
+                    --member-name "$username"
+                fi
               fi
             done
           done

mis-directory/mis-users.yml

@@ -13,7 +13,7 @@ users:
       - group2
   - username: jdoe
     first_name: Jane
-    lastjson_name: Doe
+    last_name: Doe
     email: [email protected]
     groups:
       - group2