Release 6.8.0 - Add field and API endpoint for external groups (i.e. not from the HR system)

.github/pull_request_template.md

@@ -1,7 +1,14 @@
+(Backlog issue's ID and title)
+
+---
+
 ### Added
 - 
 
-### Changed
+### Changed (non-breaking)
+-
+
+### Changed (BREAKING)
 -
 
 ### Deprecated
@@ -20,6 +27,6 @@
 
 ### Feature PR Checklist
 - [ ] Documentation (README, local.env.dist, etc.)
-- [ ] Unit tests created or updated
+- [ ] Tests created or updated
 - [ ] Run `make composershow`
 - [ ] Run `make psr2`

Makefile

@@ -22,6 +22,7 @@ composershow:
 composerupdate:
 	docker compose run --rm cli composer update
 	make composershow
+	make basemodels
 
 db:
 	docker compose up -d db

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.

application/behat.yml

@@ -19,6 +19,14 @@ 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 ]
+        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"

application/common/models/EmailLogBase.php

@@ -34,7 +34,7 @@ public function rules()
             [['user_id'], 'integer'],
             [['message_type'], 'string'],
             [['sent_utc'], 'safe'],
-            [['user_id'], 'exist', 'skipOnError' => true, 'targetClass' => User::className(), 'targetAttribute' => ['user_id' => 'id']],
+            [['user_id'], 'exist', 'skipOnError' => true, 'targetClass' => User::class, 'targetAttribute' => ['user_id' => 'id']],
         ];
     }
 
@@ -58,6 +58,6 @@ public function attributeLabels()
      */
     public function getUser()
     {
-        return $this->hasOne(User::className(), ['id' => 'user_id']);
+        return $this->hasOne(User::class, ['id' => 'user_id']);
     }
 }

application/common/models/InviteBase.php

@@ -35,7 +35,7 @@ public function rules()
             [['user_id'], 'integer'],
             [['created_utc', 'expires_on'], 'safe'],
             [['uuid'], 'string', 'max' => 36],
-            [['user_id'], 'exist', 'skipOnError' => true, 'targetClass' => User::className(), 'targetAttribute' => ['user_id' => 'id']],
+            [['user_id'], 'exist', 'skipOnError' => true, 'targetClass' => User::class, 'targetAttribute' => ['user_id' => 'id']],
         ];
     }
 
@@ -60,6 +60,6 @@ public function attributeLabels()
      */
     public function getUser()
     {
-        return $this->hasOne(User::className(), ['id' => 'user_id']);
+        return $this->hasOne(User::class, ['id' => 'user_id']);
     }
 }

application/common/models/MethodBase.php

@@ -44,7 +44,7 @@ public function rules()
             [['uid'], 'unique'],
             [['user_id', 'value'], 'unique', 'targetAttribute' => ['user_id', 'value']],
             [['verification_code'], 'unique'],
-            [['user_id'], 'exist', 'skipOnError' => true, 'targetClass' => User::className(), 'targetAttribute' => ['user_id' => 'id']],
+            [['user_id'], 'exist', 'skipOnError' => true, 'targetClass' => User::class, 'targetAttribute' => ['user_id' => 'id']],
         ];
     }
 
@@ -73,6 +73,6 @@ public function attributeLabels()
      */
     public function getUser()
     {
-        return $this->hasOne(User::className(), ['id' => 'user_id']);
+        return $this->hasOne(User::class, ['id' => 'user_id']);
     }
 }

application/common/models/MfaBackupcodeBase.php

@@ -35,7 +35,7 @@ public function rules()
             [['mfa_id'], 'integer'],
             [['created_utc', 'expires_utc'], 'safe'],
             [['value'], 'string', 'max' => 255],
-            [['mfa_id'], 'exist', 'skipOnError' => true, 'targetClass' => Mfa::className(), 'targetAttribute' => ['mfa_id' => 'id']],
+            [['mfa_id'], 'exist', 'skipOnError' => true, 'targetClass' => Mfa::class, 'targetAttribute' => ['mfa_id' => 'id']],
         ];
     }
 
@@ -60,6 +60,6 @@ public function attributeLabels()
      */
     public function getMfa()
     {
-        return $this->hasOne(Mfa::className(), ['id' => 'mfa_id']);
+        return $this->hasOne(Mfa::class, ['id' => 'mfa_id']);
     }
 }

application/common/models/MfaBase.php

@@ -44,7 +44,7 @@ public function rules()
             [['created_utc', 'last_used_utc'], 'safe'],
             [['external_uuid', 'label'], 'string', 'max' => 64],
             [['key_handle_hash'], 'string', 'max' => 255],
-            [['user_id'], 'exist', 'skipOnError' => true, 'targetClass' => User::className(), 'targetAttribute' => ['user_id' => 'id']],
+            [['user_id'], 'exist', 'skipOnError' => true, 'targetClass' => User::class, 'targetAttribute' => ['user_id' => 'id']],
         ];
     }
 
@@ -73,7 +73,7 @@ public function attributeLabels()
      */
     public function getMfaBackupcodes()
     {
-        return $this->hasMany(MfaBackupcode::className(), ['mfa_id' => 'id']);
+        return $this->hasMany(MfaBackupcode::class, ['mfa_id' => 'id']);
     }
 
     /**
@@ -83,7 +83,7 @@ public function getMfaBackupcodes()
      */
     public function getMfaFailedAttempts()
     {
-        return $this->hasMany(MfaFailedAttempt::className(), ['mfa_id' => 'id']);
+        return $this->hasMany(MfaFailedAttempt::class, ['mfa_id' => 'id']);
     }
 
     /**
@@ -93,7 +93,7 @@ public function getMfaFailedAttempts()
      */
     public function getMfaWebauthns()
     {
-        return $this->hasMany(MfaWebauthn::className(), ['mfa_id' => 'id']);
+        return $this->hasMany(MfaWebauthn::class, ['mfa_id' => 'id']);
     }
 
     /**
@@ -103,6 +103,6 @@ public function getMfaWebauthns()
      */
     public function getUser()
     {
-        return $this->hasOne(User::className(), ['id' => 'user_id']);
+        return $this->hasOne(User::class, ['id' => 'user_id']);
     }
 }

application/common/models/MfaFailedAttemptBase.php

@@ -32,7 +32,7 @@ public function rules()
             [['mfa_id', 'at_utc'], 'required'],
             [['mfa_id'], 'integer'],
             [['at_utc'], 'safe'],
-            [['mfa_id'], 'exist', 'skipOnError' => true, 'targetClass' => Mfa::className(), 'targetAttribute' => ['mfa_id' => 'id']],
+            [['mfa_id'], 'exist', 'skipOnError' => true, 'targetClass' => Mfa::class, 'targetAttribute' => ['mfa_id' => 'id']],
         ];
     }
 
@@ -55,6 +55,6 @@ public function attributeLabels()
      */
     public function getMfa()
     {
-        return $this->hasOne(Mfa::className(), ['id' => 'mfa_id']);
+        return $this->hasOne(Mfa::class, ['id' => 'mfa_id']);
     }
 }

application/common/models/MfaWebauthnBase.php

@@ -37,7 +37,7 @@ public function rules()
             [['created_utc', 'last_used_utc'], 'safe'],
             [['key_handle_hash'], 'string', 'max' => 255],
             [['label'], 'string', 'max' => 64],
-            [['mfa_id'], 'exist', 'skipOnError' => true, 'targetClass' => Mfa::className(), 'targetAttribute' => ['mfa_id' => 'id']],
+            [['mfa_id'], 'exist', 'skipOnError' => true, 'targetClass' => Mfa::class, 'targetAttribute' => ['mfa_id' => 'id']],
         ];
     }
 
@@ -63,6 +63,6 @@ public function attributeLabels()
      */
     public function getMfa()
     {
-        return $this->hasOne(Mfa::className(), ['id' => 'mfa_id']);
+        return $this->hasOne(Mfa::class, ['id' => 'mfa_id']);
     }
 }

application/common/models/PasswordBase.php

@@ -66,6 +66,6 @@ public function attributeLabels()
      */
     public function getUsers()
     {
-        return $this->hasMany(User::className(), ['current_password_id' => 'id']);
+        return $this->hasMany(User::class, ['current_password_id' => 'id']);
     }
 }

application/common/models/User.php

@@ -569,11 +569,7 @@ public function fields(): array
             },
             'hide',
             'member' => function (self $model) {
-                if (!empty($model->groups)) {
-                    $member = explode(',', $model->groups);
-                }
-                $member[] = \Yii::$app->params['idpName'];
-                return $member;
+                return $model->getMemberList();
             },
             'mfa',
             'method' => function (self $model) {
@@ -605,6 +601,26 @@ public function getDisplayName()
         return $this->display_name ?? "$this->first_name $this->last_name";
     }
 
+    /** @return string[] */
+    public function getMemberList(): array
+    {
+        if (!empty($this->groups)) {
+            $member = explode(',', $this->groups);
+        } else {
+            $member = [];
+        }
+
+        $externalGroups = explode(',', $this->groups_external);
+        foreach ($externalGroups as $externalGroup) {
+            if (!empty($externalGroup)) {
+                $member[] = $externalGroup;
+            }
+        }
+
+        $member[] = \Yii::$app->params['idpName'];
+        return $member;
+    }
+
     /**
      * Based on current time and presence of MFA and Method options,
      * determine which "nag" to present to the user.
@@ -969,6 +985,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;
     }
@@ -986,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
      */