Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release 6.8.1 - Ensure external-groups prefixes start with "ext-" #367

Closed
wants to merge 4 commits into from
Closed

Release 6.8.1 - Ensure external-groups prefixes start with "ext-" #367

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5098a99
Improve error message for test step confirming there was a sync error
forevermatt Sep 10, 2024
272f86b
Add test to ensure app-prefixes for external groups start with "ext-"
forevermatt Sep 10, 2024
a6daf7f
Require external-groups app-prefixes to start with "ext-"
forevermatt Sep 10, 2024
56f53db
Merge pull request #365 from silinternational/feature/IDP-1188-prefix…
forevermatt Sep 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions application/common/models/User.php
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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 |
| [email protected] | wiki-one |
And the "wiki" external groups list is the following:
| email | groups |
| [email protected] | wiki-one,wiki-two |
When I sync the list of "wiki" external groups
| email | groups |
| [email protected] | ext-wiki-one |
And the "ext-wiki" external groups list is the following:
| email | groups |
| [email protected] | 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 |
| [email protected] | wiki-one,wiki-two |
| email | groups |
| [email protected] | 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 |
| [email protected] | wiki-one |
And the "wiki" external groups list is the following:
| email | groups |
| [email protected] | wiki-two |
When I sync the list of "wiki" external groups
| email | groups |
| [email protected] | ext-wiki-one |
And the "ext-wiki" external groups list is the following:
| email | groups |
| [email protected] | 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 |
| [email protected] | wiki-two |
| email | groups |
| [email protected] | 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 |
| [email protected] | wiki-one,map-europe |
And the "wiki" external groups list is the following:
| email | groups |
| [email protected] | wiki-two |
When I sync the list of "wiki" external groups
| email | groups |
| [email protected] | ext-wiki-one,ext-map-europe |
And the "ext-wiki" external groups list is the following:
| email | groups |
| [email protected] | 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 |
| [email protected] | wiki-two,map-europe |
| email | groups |
| [email protected] | 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 |
| [email protected] | wiki-one,wiki-two |
And the "wiki" external groups list is the following:
| email | groups |
| [email protected] | wiki-one |
When I sync the list of "wiki" external groups
| email | groups |
| [email protected] | ext-wiki-one,ext-wiki-two |
And the "ext-wiki" external groups list is the following:
| email | groups |
| [email protected] | 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 |
| [email protected] | wiki-one |
| email | groups |
| [email protected] | 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 |
| [email protected] | wiki-one,wiki-two,map-asia |
And the "wiki" external groups list is the following:
| email | groups |
| [email protected] | 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 |
| [email protected] | map-asia |
| email | groups |
| [email protected] | 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 |
| [email protected] | wiki-one,wiki-two,map-asia |
And the "wiki" external groups list is the following:
| email | groups |
| [email protected] | ext-wiki-one,ext-wiki-two,ext-map-asia |
And the "ext-wiki" external groups list is the following:
| email | groups |
| [email protected] | |
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 |
| [email protected] | map-asia |
| email | groups |
| [email protected] | 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 |
| [email protected] | wiki-one |
And the "wiki" external groups list is the following:
| email | groups |
| [email protected] | wiki-one,map-asia |
When I sync the list of "wiki" external groups
| email | groups |
| [email protected] | ext-wiki-one |
And the "ext-wiki" external groups list is the following:
| email | groups |
| [email protected] | 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 |
| [email protected] | wiki-one |
| email | groups |
| [email protected] | ext-wiki-one |

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

Scenario: Properly handle an empty email address
Given the following users exist, with these external groups:
| email | groups |
| [email protected] | wiki-one |
And the "wiki" external groups list is the following:
| email | groups |
| | wiki-one |
| [email protected] | wiki-one,wiki-two |
When I sync the list of "wiki" external groups
| email | groups |
| [email protected] | ext-wiki-one |
And the "ext-wiki" external groups list is the following:
| email | groups |
| | ext-wiki-one |
| [email protected] | 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 |
| [email protected] | wiki-one,wiki-two |
| email | groups |
| [email protected] | 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
Open in desktop
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