diff --git a/api.raml b/api.raml index 83ce571d..ec99df8c 100644 --- a/api.raml +++ b/api.raml @@ -44,6 +44,24 @@ types: "code": 0, "status": 400 } + GroupsExternalUpdate: + type: object + properties: + email_address: string + app_prefix: + type: string + description: > + The app-specific prefix, without the trailing hyphen. + groups: + type: string[] + description: > + The desired list of groups, including the app-prefix. + example: | + { + "email_address": "john_smith@example.com", + "app_prefix": "myapp", + "groups": ["myapp-users", "myapp-managers"] + } UserResponse: description: > Information on user record. Password is not included if a password @@ -418,6 +436,25 @@ types: description: A server-side error occurred. body: type: Error + /external-groups: + put: + description: > + Update a user's list of external groups for a given app-prefix to be + exactly the given list, leaving external groups with other prefixes + unchanged. + body: + type: GroupsExternalUpdate + responses: + 204: + description: > + The user's external groups for that app-prefix were updated. + 404: + description: > + No such user found. + 422: + description: The given data does not satisfy some validation rule. + body: + type: Error /{employee_id}: get: description: Get information about a specific user. diff --git a/application/behat.yml b/application/behat.yml index 5a988363..9b3cf8de 100644 --- a/application/behat.yml +++ b/application/behat.yml @@ -23,6 +23,10 @@ default: paths: - "features/groups-external.feature" contexts: [ Sil\SilIdBroker\Behat\Context\GroupsExternalContext ] + groups_external_updates_features: + paths: + - "features/groups-external-updates.feature" + contexts: [ Sil\SilIdBroker\Behat\Context\GroupsExternalUpdatesContext ] hibp_unit_tests_features: paths: - "features/hibp-unit-tests.feature" diff --git a/application/common/models/User.php b/application/common/models/User.php index f14043e0..3f8c5760 100644 --- a/application/common/models/User.php +++ b/application/common/models/User.php @@ -1003,6 +1003,80 @@ public function isPromptForMfa(): bool return false; } + public function updateExternalGroups($appPrefix, $appExternalGroups): bool + { + foreach ($appExternalGroups as $appExternalGroup) { + if (! str_starts_with($appExternalGroup, $appPrefix . '-')) { + $this->addErrors([ + 'groups_external' => sprintf( + 'The given group %s does not start with the given prefix (%s)', + json_encode($appExternalGroup), + json_encode($appPrefix) + ), + ]); + return false; + } + } + $this->removeInMemoryExternalGroupsFor($appPrefix); + $this->addInMemoryExternalGroups($appExternalGroups); + + $this->scenario = self::SCENARIO_UPDATE_USER; + $saved = $this->save(true, ['groups_external']); + if ($saved) { + return true; + } else { + Yii::warning(sprintf( + 'Failed to update external groups for %s: %s', + $this->email, + join(', ', $this->getFirstErrors()) + )); + return false; + } + } + + /** + * Remove all entries from this User object's list of external groups that + * begin with the given prefix. + * + * NOTE: + * This only updates the property in memory. It leaves the calling code to + * call `save()` on this User when desired. + * + * @param $appPrefix + * @return void + */ + private function removeInMemoryExternalGroupsFor($appPrefix) + { + $currentExternalGroups = explode(',', $this->groups_external); + $newExternalGroups = []; + foreach ($currentExternalGroups as $externalGroup) { + if (! str_starts_with($externalGroup, $appPrefix . '-')) { + $newExternalGroups[] = $externalGroup; + } + } + $this->groups_external = join(',', $newExternalGroups); + } + + /** + * Add the given groups to this User objects' list of external groups. + * + * NOTE: + * This only updates the property in memory. It leaves the calling code to + * call `save()` on this User when desired. + * + * @param $newExternalGroups + * @return void + */ + private function addInMemoryExternalGroups($newExternalGroups) + { + $newCommaSeparatedExternalGroups = sprintf( + '%s,%s', + $this->groups_external, + join(',', $newExternalGroups) + ); + $this->groups_external = trim($newCommaSeparatedExternalGroups, ','); + } + /** * Update the date field that corresponds to the current nag state */ diff --git a/application/features/bootstrap/GroupsExternalContext.php b/application/features/bootstrap/GroupsExternalContext.php index a39c4851..4b270dca 100644 --- a/application/features/bootstrap/GroupsExternalContext.php +++ b/application/features/bootstrap/GroupsExternalContext.php @@ -67,6 +67,11 @@ private function setThatUsersPassword(string $password) )); } + protected function getUserEmailAddress(): string + { + return $this->userEmailAddress; + } + /** * @Given that user's list of groups is :commaSeparatedGroups */ diff --git a/application/features/bootstrap/GroupsExternalUpdatesContext.php b/application/features/bootstrap/GroupsExternalUpdatesContext.php new file mode 100644 index 00000000..61018287 --- /dev/null +++ b/application/features/bootstrap/GroupsExternalUpdatesContext.php @@ -0,0 +1,37 @@ +cleanRequestBody(); + $this->setRequestBody('email_address', $this->getUserEmailAddress()); + $this->setRequestBody('app_prefix', $appPrefix); + $this->setRequestBody('groups', $externalGroups); + + $this->iRequestTheResourceBe('/user/external-groups', 'updated'); + } + + /** + * @Then that user's list of external groups should be :commaSeparatedExternalGroups + */ + public function thatUsersListOfExternalGroupsShouldBe($commaSeparatedExternalGroups) + { + $user = User::findByEmail($this->getUserEmailAddress()); + Assert::same($user->groups_external, $commaSeparatedExternalGroups); + } +} diff --git a/application/features/groups-external-updates.feature b/application/features/groups-external-updates.feature new file mode 100644 index 00000000..c4c358d8 --- /dev/null +++ b/application/features/groups-external-updates.feature @@ -0,0 +1,42 @@ +Feature: Updating a User's list of external groups + + Background: + Given the requester is authorized + + Scenario: Add an external group to a user's list for a particular app + Given a user exists + And that user's list of external groups is "wiki-users" + When I update that user's list of "wiki" external groups to the following: + | externalGroup | + | wiki-users | + | wiki-managers | + Then the response status code should be 204 + And that user's list of external groups should be "wiki-users,wiki-managers" + + Scenario: Remove an external group from a user's list for a particular app + Given a user exists + And that user's list of external groups is "wiki-users,wiki-managers" + When I update that user's list of "wiki" external groups to the following: + | externalGroup | + | wiki-managers | + Then the response status code should be 204 + And that user's list of external groups should be "wiki-managers" + + Scenario: Leave a user's external groups for a different app unchanged + Given a user exists + And that user's list of external groups is "wiki-users,map-europe" + When I update that user's list of "map" external groups to the following: + | externalGroup | + | map-america | + Then the response status code should be 204 + And that user's list of external groups should be "wiki-users,map-america" + + Scenario: Try to add an external group that does not match the given app-prefix + Given a user exists + And that user's list of external groups is "wiki-users" + When I update that user's list of "wiki" external groups to the following: + | externalGroup | + | map-america | + Then the response status code should be 422 + And the response body should contain "prefix" + And that user's list of external groups should be "wiki-users" diff --git a/application/features/groups-external.feature b/application/features/groups-external.feature index f93e6232..3b6ef113 100644 --- a/application/features/groups-external.feature +++ b/application/features/groups-external.feature @@ -40,8 +40,6 @@ Feature: Incorporating custom (external) groups in a User's `members` list | two | | {idpName} | -# Scenario: Update a user's `groups_external` list, given a group prefix and list of groups - # # Scenarios that belong in the new "groups_external" sync: # Scenario: Send 1 notification email if sync finds group(s) for a user not in this IDP # Scenario: Add entries in the synced Google Sheet to the `groups_external` field diff --git a/application/frontend/config/main.php b/application/frontend/config/main.php index 13818255..ea56b04e 100644 --- a/application/frontend/config/main.php +++ b/application/frontend/config/main.php @@ -48,6 +48,7 @@ 'GET user' => 'user/index', 'GET user/' => 'user/view', 'POST user' => 'user/create', + 'PUT user/external-groups' => 'user/update-external-groups', 'PUT user/' => 'user/update', 'PUT user//password' => 'user/update-password', 'PUT user//password/assess' => 'user/assess-password', diff --git a/application/frontend/controllers/UserController.php b/application/frontend/controllers/UserController.php index 696a4808..6c413c64 100644 --- a/application/frontend/controllers/UserController.php +++ b/application/frontend/controllers/UserController.php @@ -75,6 +75,27 @@ public function actionUpdate(string $employeeId) return $user; } + public function actionUpdateExternalGroups() + { + $emailAddress = Yii::$app->request->getBodyParam('email_address'); + $appPrefix = Yii::$app->request->getBodyParam('app_prefix'); + $externalGroups = Yii::$app->request->getBodyParam('groups'); + + $user = User::findByEmail($emailAddress); + if ($user === null) { + Yii::$app->response->statusCode = 404; + return; + } + + if ($user->updateExternalGroups($appPrefix, $externalGroups)) { + Yii::$app->response->statusCode = 204; + return; + } + + $errors = join(', ', $user->getFirstErrors()); + throw new UnprocessableEntityHttpException($errors); + } + public function actionUpdatePassword(string $employeeId) { $user = User::findOne(['employee_id' => $employeeId]);