Skip to content

Commit

Permalink
Merge pull request #355 from silinternational/feature/IDP-1182-update…
Browse files Browse the repository at this point in the history
…-groups-external

[IDP-1182] Add endpoint for updating `groups_external`
  • Loading branch information
forevermatt authored Aug 21, 2024
2 parents 9ad7b66 + 85032eb commit e6557b7
Show file tree
Hide file tree
Showing 9 changed files with 221 additions and 2 deletions.
37 changes: 37 additions & 0 deletions api.raml
Original file line number Diff line number Diff line change
Expand Up @@ -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": "[email protected]",
"app_prefix": "myapp",
"groups": ["myapp-users", "myapp-managers"]
}
UserResponse:
description: >
Information on user record. Password is not included if a password
Expand Down Expand Up @@ -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.
Expand Down
4 changes: 4 additions & 0 deletions application/behat.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
74 changes: 74 additions & 0 deletions application/common/models/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
5 changes: 5 additions & 0 deletions application/features/bootstrap/GroupsExternalContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
37 changes: 37 additions & 0 deletions application/features/bootstrap/GroupsExternalUpdatesContext.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php

namespace Sil\SilIdBroker\Behat\Context;

use Behat\Gherkin\Node\TableNode;
use common\models\User;
use Webmozart\Assert\Assert;

class GroupsExternalUpdatesContext extends GroupsExternalContext
{
/**
* @When I update that user's list of :appPrefix external groups to the following:
*/
public function iUpdateThatUsersListOfExternalGroupsToTheFollowing($appPrefix, TableNode $table)
{
$externalGroups = [];
foreach ($table as $row) {
$externalGroups[] = $row['externalGroup'];
}

$this->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);
}
}
42 changes: 42 additions & 0 deletions application/features/groups-external-updates.feature
Original file line number Diff line number Diff line change
@@ -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"
2 changes: 0 additions & 2 deletions application/features/groups-external.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions application/frontend/config/main.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
'GET user' => 'user/index',
'GET user/<employeeId:\w+>' => 'user/view',
'POST user' => 'user/create',
'PUT user/external-groups' => 'user/update-external-groups',
'PUT user/<employeeId:\w+>' => 'user/update',
'PUT user/<employeeId:\w+>/password' => 'user/update-password',
'PUT user/<employeeId:\w+>/password/assess' => 'user/assess-password',
Expand Down
21 changes: 21 additions & 0 deletions application/frontend/controllers/UserController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down

0 comments on commit e6557b7

Please sign in to comment.