Merge pull request #352 from silinternational/feature/idp-1176-add-gr…

…oups-external-field

[IDP-1176] Add `groups_external` field to User

application/behat.yml

@@ -19,6 +19,10 @@ default:
             paths:
                 - "features/email.feature"
             contexts: [ Sil\SilIdBroker\Behat\Context\EmailContext ]
+        groups_external_features:
+            paths:
+                - "features/groups-external.feature"
+            contexts: [ Sil\SilIdBroker\Behat\Context\GroupsExternalContext ]
         hibp_unit_tests_features:
             paths:
                 - "features/hibp-unit-tests.feature"

application/common/models/User.php

@@ -571,7 +571,17 @@ public function fields(): array
             'member' => function (self $model) {
                 if (!empty($model->groups)) {
                     $member = explode(',', $model->groups);
+                } else {
+                    $member = [];
                 }
+
+                $externalGroups = explode(',', $model->groups_external);
+                foreach ($externalGroups as $externalGroup) {
+                    if (!empty($externalGroup)) {
+                        $member[] = $externalGroup;
+                    }
+                }
+
                 $member[] = \Yii::$app->params['idpName'];
                 return $member;
             },
@@ -969,6 +979,7 @@ public function attributeLabels()
         $labels['last_synced_utc'] = Yii::t('app', 'Last Synced (UTC)');
         $labels['created_utc'] = Yii::t('app', 'Created (UTC)');
         $labels['deactivated_utc'] = Yii::t('app', 'Deactivated (UTC)');
+        $labels['groups_external'] = Yii::t('app', 'Groups (External)');
 
         return $labels;
     }

application/common/models/UserBase.php

@@ -26,6 +26,7 @@
  * @property string|null $manager_email
  * @property string $hide
  * @property string|null $groups
+ * @property string $groups_external
  * @property string|null $personal_email
  * @property string|null $expires_on
  * @property string $nag_for_mfa_after
@@ -60,7 +61,7 @@ public function rules()
             [['active', 'locked', 'require_mfa', 'hide'], 'string'],
             [['last_changed_utc', 'last_synced_utc', 'review_profile_after', 'last_login_utc', 'expires_on', 'nag_for_mfa_after', 'nag_for_method_after', 'created_utc', 'deactivated_utc'], 'safe'],
             [['uuid'], 'string', 'max' => 64],
-            [['employee_id', 'first_name', 'last_name', 'display_name', 'username', 'email', 'manager_email', 'groups', 'personal_email'], 'string', 'max' => 255],
+            [['employee_id', 'first_name', 'last_name', 'display_name', 'username', 'email', 'manager_email', 'groups', 'groups_external', 'personal_email'], 'string', 'max' => 255],
             [['employee_id'], 'unique'],
             [['username'], 'unique'],
             [['email'], 'unique'],
@@ -93,6 +94,7 @@ public function attributeLabels()
             'manager_email' => Yii::t('app', 'Manager Email'),
             'hide' => Yii::t('app', 'Hide'),
             'groups' => Yii::t('app', 'Groups'),
+            'groups_external' => Yii::t('app', 'Groups External'),
             'personal_email' => Yii::t('app', 'Personal Email'),
             'expires_on' => Yii::t('app', 'Expires On'),
             'nag_for_mfa_after' => Yii::t('app', 'Nag For Mfa After'),

application/console/migrations/m240813_155757_add_groups_external_field_to_user.php

@@ -0,0 +1,23 @@
+<?php
+
+use yii\db\Migration;
+
+/**
+ * Class m240813_155757_add_groups_external_field_to_user
+ */
+class m240813_155757_add_groups_external_field_to_user extends Migration
+{
+    public function safeUp()
+    {
+        $this->addColumn(
+            '{{user}}',
+            'groups_external',
+            $this->string()->notNull()->defaultValue('')->after('groups')
+        );
+    }
+
+    public function safeDown()
+    {
+        $this->dropColumn('{{user}}', 'groups_external');
+    }
+}

application/features/bootstrap/FeatureContext.php

@@ -838,7 +838,7 @@ public function theResponseShouldContainAMemberArrayWithOnlyTheseElements($key,
             if ($want == '{idpName}') {
                 $want = \Yii::$app->params['idpName'];
             }
-            Assert::true(in_array($want, $property), '"' . $want . '" not in array');
+            Assert::true(in_array($want, $property), '"' . $want . '" not in array: ' . json_encode($property));
             $n++;
         }
 

application/features/bootstrap/GroupsExternalContext.php

@@ -0,0 +1,114 @@
+<?php
+
+namespace Sil\SilIdBroker\Behat\Context;
+
+use Behat\Gherkin\Node\TableNode;
+use common\models\User;
+use FeatureContext;
+use Webmozart\Assert\Assert;
+
+class GroupsExternalContext extends FeatureContext
+{
+    private User $user;
+    private string $userEmailAddress = '[email protected]';
+    private string $userPassword = 'dummy-password-#1';
+
+    /**
+     * @Given a user exists
+     */
+    public function aUserExists()
+    {
+        $this->deleteThatTestUser();
+        $this->createTestUser();
+        $this->setThatUsersPassword($this->userPassword);
+    }
+
+    private function deleteThatTestUser()
+    {
+        $user = User::findByEmail($this->userEmailAddress);
+        if ($user !== null) {
+            $didDeleteUser = $user->delete();
+            Assert::notFalse($didDeleteUser, sprintf(
+                'Failed to delete existing test user: %s',
+                join("\n", $user->getFirstErrors())
+            ));
+        }
+    }
+
+    private function createTestUser()
+    {
+        $user = new User([
+            'email' => $this->userEmailAddress,
+            'employee_id' => '11111',
+            'first_name' => 'John',
+            'last_name' => 'Smith',
+            'username' => 'john_smith',
+        ]);
+        $user->scenario = User::SCENARIO_NEW_USER;
+
+        $createdNewUser = $user->save();
+        Assert::true($createdNewUser, sprintf(
+            'Failed to create test user: %s',
+            join("\n", $user->getFirstErrors())
+        ));
+        $user->refresh();
+
+        $this->user = $user;
+    }
+
+    private function setThatUsersPassword(string $password)
+    {
+        $this->user->scenario = User::SCENARIO_UPDATE_PASSWORD;
+        $this->user->password = $password;
+
+        Assert::true($this->user->save(), sprintf(
+            "Failed to set the test user's password: %s",
+            join("\n", $this->user->getFirstErrors())
+        ));
+    }
+
+    /**
+     * @Given that user's list of groups is :commaSeparatedGroups
+     */
+    public function thatUsersListOfGroupsIs($commaSeparatedGroups)
+    {
+        $this->user->groups = $commaSeparatedGroups;
+        $this->user->scenario = User::SCENARIO_UPDATE_USER;
+
+        $savedChanges = $this->user->save();
+        Assert::true($savedChanges, sprintf(
+            'Failed to set list of `groups` on test user: %s',
+            join("\n", $this->user->getFirstErrors())
+        ));
+    }
+
+    /**
+     * @Given that user's list of external groups is :commaSeparatedExternalGroups
+     */
+    public function thatUsersListOfExternalGroupsIs($commaSeparatedExternalGroups)
+    {
+        $this->user->groups_external = $commaSeparatedExternalGroups;
+        $this->user->scenario = User::SCENARIO_UPDATE_USER;
+
+        $savedChanges = $this->user->save();
+        Assert::true($savedChanges, sprintf(
+            'Failed to set list of `groups_external` on test user: %s',
+            join("\n", $this->user->getFirstErrors())
+        ));
+    }
+
+    /**
+     * @When I sign in as that user
+     */
+    public function iSignInAsThatUser()
+    {
+        $dataForTableNode = [
+            ['property', 'value'],
+            ['username', $this->user->username],
+            ['password', $this->userPassword],
+        ];
+        $this->iProvideTheFollowingValidData(new TableNode($dataForTableNode));
+        $this->iRequestTheResourceBe('/authentication', 'created');
+        $this->theResponseStatusCodeShouldBe(200);
+    }
+}

application/features/groups-external.feature

@@ -0,0 +1,49 @@
+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"
+    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} |
+
+  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"
+    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} |
+
+  Scenario: Gracefully handle an empty list of 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 ""
+    When I sign in as that user
+    Then the response should contain a member array with only these elements:
+      | element   |
+      | one       |
+      | 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
+#  Scenario: Remove entries not in the synced Google Sheet from the `groups_external` field
+#  Scenario: Only use entries from the synced Google Sheet that specify this IDP

application/run-tests.sh

@@ -43,7 +43,7 @@ apachectl start
 rc=$?; if [[ $rc != 0 ]]; then exit $rc; fi
 
 # Run the feature tests
-./vendor/bin/behat --strict
+./vendor/bin/behat --strict --stop-on-failure
 
 # If they failed, exit.
 rc=$?; if [[ $rc != 0 ]]; then exit $rc; fi

docker-compose.yml

@@ -211,6 +211,19 @@ services:
             PMA_USER: user
             PMA_PASSWORD: pass
 
+    phpmyadmintest:
+        image: phpmyadmin:5
+        ports:
+            - "51142:80"
+        depends_on:
+            - testdb
+        env_file:
+            - ./local.env
+        environment:
+            PMA_HOST: testdb
+            PMA_USER: appfortests
+            PMA_PASSWORD: appfortests
+
     raml2html:
         image: mattjtodd/raml2html
         platform: linux/amd64