Merge pull request #371 from silinternational/feature/IDP-1153-fix-ca…

…pitalization-bug-in-sync Ignore upper/lowercase differences in email when syncing external groups
silinternational · Sep 18, 2024 · ca81524 · ca81524
2 parents fbe83f4 + 94d808e
commit ca81524
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 31 deletions.
diff --git a/application/common/models/User.php b/application/common/models/User.php
@@ -1052,17 +1052,24 @@ public static function updateUsersExternalGroups(
             return $errors;
         }
 
+        $desiredExternalGroupsByLowercaseUserEmail = [];
+        foreach ($desiredExternalGroupsByUserEmail as $email => $groups) {
+            $desiredExternalGroupsByLowercaseUserEmail[mb_strtolower($email)] = $groups;
+        }
+        unset($desiredExternalGroupsByUserEmail);
+
         $emailAddressesOfCurrentMatches = self::listUsersWithExternalGroupWith($appPrefix);
 
         // Indicate that users not in the "desired" list should not have any
         // such external groups.
         foreach ($emailAddressesOfCurrentMatches as $email) {
-            if (! array_key_exists($email, $desiredExternalGroupsByUserEmail)) {
-                $desiredExternalGroupsByUserEmail[$email] = '';
+            $lowercaseEmail = mb_strtolower($email);
+            if (! array_key_exists($lowercaseEmail, $desiredExternalGroupsByLowercaseUserEmail)) {
+                $desiredExternalGroupsByLowercaseUserEmail[$lowercaseEmail] = '';
             }
         }
 
-        foreach ($desiredExternalGroupsByUserEmail as $email => $groupsForPrefix) {
+        foreach ($desiredExternalGroupsByLowercaseUserEmail as $email => $groupsForPrefix) {
             $user = User::findByEmail($email);
             if ($user === null) {
                 $errors[] = 'No user found for email address ' . json_encode($email);

diff --git a/application/composer.lock b/application/composer.lock
diff --git a/application/dependencies.json b/application/dependencies.json
@@ -49,7 +49,7 @@
   },
   {
     "name": "google/apiclient-services",
-    "version": "v0.372.0"
+    "version": "v0.373.0"
   },
   {
     "name": "google/auth",
@@ -129,7 +129,7 @@
   },
   {
     "name": "phpseclib/phpseclib",
-    "version": "3.0.41"
+    "version": "3.0.42"
   },
   {
     "name": "phpspec/php-diff",
@@ -169,7 +169,7 @@
   },
   {
     "name": "roave/security-advisories",
-    "version": "dev-master 791bcaf"
+    "version": "dev-master cc2d3a5"
   },
   {
     "name": "sentry/sdk",

diff --git a/application/features/groups-external-sync.feature b/application/features/groups-external-sync.feature
@@ -121,4 +121,20 @@ Feature: Syncing a specific app-prefix of external groups with an external list
         | email                  | groups                    |
         | john_smith@example.org | ext-wiki-one,ext-wiki-two |
 
+  Scenario: Properly handle mismatched casing (uppercase/lowercase) in an email address
+    Given the following users exist, with these external groups:
+        | email                  | groups       |
+        | john_smith@example.org | ext-wiki-one |
+        | Jane_Doe@example.org   | ext-wiki-two |
+      And the "ext-wiki" external groups list is the following:
+        | email                  | groups       |
+        | John_smith@example.org | ext-wiki-one |
+        | jane_doe@example.org   | ext-wiki-two |
+    When I sync the list of "ext-wiki" external groups
+    Then there should not have been any sync errors
+      And the following users should have the following external groups:
+        | email                  | groups       |
+        | john_smith@example.org | ext-wiki-one |
+        | Jane_Doe@example.org   | ext-wiki-two |
+
   # Scenario: Send 1 notification email if sync finds group(s) for a user not in this IDP