Skip to content

Commit

Permalink
Merge pull request #365 from silinternational/feature/IDP-1188-prefix…
Browse files Browse the repository at this point in the history 
…-external-groups-prefixes-with-ext

Prefix external-groups prefixes with `ext-`
  • Loading branch information
@forevermatt
forevermatt authored Sep 11, 2024
2 parents 303b3be + a6daf7f commit 56f53db
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 85 deletions.
9 changes: 9 additions & 0 deletions application/common/models/User.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1043,6 +1043,15 @@ public static function updateUsersExternalGroups(
array $desiredExternalGroupsByUserEmail
): array {
$errors = [];

if (preg_match('/^ext-[a-z0-9]+$/', $appPrefix) === 0) {
$errors[] = sprintf(
"The external-groups app-prefix must begin with 'ext-', then "
. "some combination of lowercase letters and/or numbers."
);
return $errors;
}

$emailAddressesOfCurrentMatches = self::listUsersWithExternalGroupWith($appPrefix);

// Indicate that users not in the "desired" list should not have any
Expand Down
25 changes: 23 additions & 2 deletions application/features/bootstrap/GroupsExternalSyncContext.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public function iSyncTheListOfExternalGroups($appPrefix)
{
$this->syncErrors = User::updateUsersExternalGroups(
$appPrefix,
$this->externalGroupsLists[$appPrefix]
$this->externalGroupsLists[$appPrefix] ?? []
);
}

Expand Down Expand Up @@ -108,9 +108,30 @@ public function theFollowingUsersShouldHaveTheFollowingExternalGroups(TableNode
*/
public function thereShouldHaveBeenASyncError()
{
Assert::notEmpty($this->syncErrors);
Assert::notEmpty(
$this->syncErrors,
'Expected sync errors, but found none.'
);
foreach ($this->syncErrors as $syncError) {
echo $syncError . PHP_EOL;
}
}

/**
* @Then there should have been a sync error that mentions :text
*/
public function thereShouldHaveBeenASyncErrorThatMentions($text)
{
$foundMatch = false;
foreach ($this->syncErrors as $syncError) {
echo $syncError . PHP_EOL;
if (str_contains($syncError, $text)) {
$foundMatch = true;
}
}
Assert::true($foundMatch, sprintf(
"Did not find a sync error that mentions '%s'",
$text
));
}
}
142 changes: 73 additions & 69 deletions application/features/groups-external-sync.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,119 +2,123 @@ Feature: Syncing a specific app-prefix of external groups with an external list

Scenario: Add an external group to a user's list for a particular app
Given the following users exist, with these external groups:
| email | groups |
| john_smith@example.org | wiki-one |
And the "wiki" external groups list is the following:
| email | groups |
| john_smith@example.org | wiki-one,wiki-two |
When I sync the list of "wiki" external groups
| email | groups |
| john_smith@example.org | ext-wiki-one |
And the "ext-wiki" external groups list is the following:
| email | groups |
| john_smith@example.org | ext-wiki-one,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 | wiki-one,wiki-two |
| email | groups |
| john_smith@example.org | ext-wiki-one,ext-wiki-two |

Scenario: Change the external group in a user's list for a particular app
Given the following users exist, with these external groups:
| email | groups |
| john_smith@example.org | wiki-one |
And the "wiki" external groups list is the following:
| email | groups |
| john_smith@example.org | wiki-two |
When I sync the list of "wiki" external groups
| email | groups |
| john_smith@example.org | ext-wiki-one |
And the "ext-wiki" external groups list is the following:
| email | groups |
| john_smith@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 | wiki-two |
| email | groups |
| john_smith@example.org | ext-wiki-two |

Scenario: Leave a user's external groups for a different app unchanged
Given the following users exist, with these external groups:
| email | groups |
| john_smith@example.org | wiki-one,map-europe |
And the "wiki" external groups list is the following:
| email | groups |
| john_smith@example.org | wiki-two |
When I sync the list of "wiki" external groups
| email | groups |
| john_smith@example.org | ext-wiki-one,ext-map-europe |
And the "ext-wiki" external groups list is the following:
| email | groups |
| john_smith@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 | wiki-two,map-europe |
| email | groups |
| john_smith@example.org | ext-wiki-two,ext-map-europe |

Scenario: Remove an external group from a user's list for a particular app
Given the following users exist, with these external groups:
| email | groups |
| john_smith@example.org | wiki-one,wiki-two |
And the "wiki" external groups list is the following:
| email | groups |
| john_smith@example.org | wiki-one |
When I sync the list of "wiki" external groups
| email | groups |
| john_smith@example.org | ext-wiki-one,ext-wiki-two |
And the "ext-wiki" external groups list is the following:
| email | groups |
| john_smith@example.org | ext-wiki-one |
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 | wiki-one |
| email | groups |
| john_smith@example.org | ext-wiki-one |

Scenario: Remove all external groups from a user's list for a particular app (no entry in list)
Given the following users exist, with these external groups:
| email | groups |
| john_smith@example.org | wiki-one,wiki-two,map-asia |
And the "wiki" external groups list is the following:
| email | groups |
| john_smith@example.org | ext-wiki-one,ext-wiki-two,ext-map-asia |
And the "ext-wiki" external groups list is the following:
| email | groups |
When I sync the list of "wiki" external groups
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 | map-asia |
| email | groups |
| john_smith@example.org | ext-map-asia |

Scenario: Remove all external groups from a user's list for a particular app (blank entry in list)
Given the following users exist, with these external groups:
| email | groups |
| john_smith@example.org | wiki-one,wiki-two,map-asia |
And the "wiki" external groups list is the following:
| email | groups |
| john_smith@example.org | ext-wiki-one,ext-wiki-two,ext-map-asia |
And the "ext-wiki" external groups list is the following:
| email | groups |
| john_smith@example.org | |
When I sync the list of "wiki" external groups
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 | map-asia |
| email | groups |
| john_smith@example.org | ext-map-asia |

Scenario: Try to use an app-prefix that does not begin with "ext-"
When I sync the list of "wiki" external groups
Then there should have been a sync error that mentions "ext-"

Scenario: Try to add an external group that does not match the given app-prefix
Given the following users exist, with these external groups:
| email | groups |
| john_smith@example.org | wiki-one |
And the "wiki" external groups list is the following:
| email | groups |
| john_smith@example.org | wiki-one,map-asia |
When I sync the list of "wiki" external groups
| email | groups |
| john_smith@example.org | ext-wiki-one |
And the "ext-wiki" external groups list is the following:
| email | groups |
| john_smith@example.org | ext-wiki-one,ext-map-asia |
When I sync the list of "ext-wiki" external groups
Then there should have been a sync error
And the following users should have the following external groups:
| email | groups |
| john_smith@example.org | wiki-one |
| email | groups |
| john_smith@example.org | ext-wiki-one |

Scenario: Properly handle (trim) spaces around external groups
Given the following users exist, with these external groups:
| email | groups |
| john_smith@example.org | wiki-one |
And the "wiki" external groups list is the following:
| email | groups |
| john_smith@example.org | wiki-one, wiki-two |
When I sync the list of "wiki" external groups
| email | groups |
| john_smith@example.org | ext-wiki-one |
And the "ext-wiki" external groups list is the following:
| email | groups |
| john_smith@example.org | ext-wiki-one, 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 | wiki-one,wiki-two |
| email | groups |
| john_smith@example.org | ext-wiki-one,ext-wiki-two |

Scenario: Properly handle an empty email address
Given the following users exist, with these external groups:
| email | groups |
| john_smith@example.org | wiki-one |
And the "wiki" external groups list is the following:
| email | groups |
| | wiki-one |
| john_smith@example.org | wiki-one,wiki-two |
When I sync the list of "wiki" external groups
| email | groups |
| john_smith@example.org | ext-wiki-one |
And the "ext-wiki" external groups list is the following:
| email | groups |
| | ext-wiki-one |
| john_smith@example.org | ext-wiki-one,ext-wiki-two |
When I sync the list of "ext-wiki" external groups
Then there should have been a sync error
And the following users should have the following external groups:
| email | groups |
| john_smith@example.org | wiki-one,wiki-two |
| email | groups |
| john_smith@example.org | ext-wiki-one,ext-wiki-two |

# Scenario: Send 1 notification email if sync finds group(s) for a user not in this IDP
26 changes: 12 additions & 14 deletions application/features/groups-external.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,29 @@ Feature: Incorporating custom (external) groups in a User's `members` list
Background:
Given the requester is authorized

# Scenarios that belong here in ID Broker:

Scenario: Include external groups in a User's `members` list
Given a user exists
And that user's list of groups is "one,two"
And that user's list of external groups is "app-three,app-four"
And that user's list of external groups is "ext-app-three,ext-app-four"
When I sign in as that user
Then the response should contain a member array with only these elements:
| element |
| one |
| two |
| app-three |
| app-four |
| {idpName} |
| element |
| one |
| two |
| ext-app-three |
| ext-app-four |
| {idpName} |

Scenario: Gracefully handle an empty list of groups in a User's `members` list
Given a user exists
And that user's list of groups is ""
And that user's list of external groups is "app-three,app-four"
And that user's list of external groups is "ext-app-three,ext-app-four"
When I sign in as that user
Then the response should contain a member array with only these elements:
| element |
| app-three |
| app-four |
| {idpName} |
| element |
| ext-app-three |
| ext-app-four |
| {idpName} |

Scenario: Gracefully handle an empty list of external groups in a User's `members` list
Given a user exists
Expand Down

0 comments on commit 56f53db

Please sign in to comment.