Skip to content

Commit

Permalink
Ignore upper/lowercase differences in email when syncing external groups
Browse files Browse the repository at this point in the history
Whether the Google Sheet has the non-lowercase email address or the ID
Broker database has the non-lowercase email, this will now correctly
sync the external groups (rather than removing them on every other run).
  • Loading branch information
forevermatt committed Sep 18, 2024
1 parent 5c05687 commit f805df5
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 3 deletions.
13 changes: 10 additions & 3 deletions application/common/models/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
16 changes: 16 additions & 0 deletions application/features/groups-external-sync.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit f805df5

Please sign in to comment.