From da2f904554cc53894a48926b2249bd257a7896b2 Mon Sep 17 00:00:00 2001
From: iglocska <andras.iklody@gmail.com>
Date: Fri, 23 Dec 2022 16:47:44 +0100
Subject: [PATCH] fix: [security] reworked the Individual handling of user
 creations / modifications
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- creating a new user with the e-mail address of an already existing individual should NOT overwrite the first/last name fields
- it merely connects the individual to the new user

- disallow changing the individual behind an existing user altogether
- allow capturing individuals without updates

- As reported by Matúš Mikuláš, Adam Gajdošík, Milan Pikula of SK-CERT
---
 src/Controller/UsersController.php   | 20 ++------------------
 src/Model/Table/IndividualsTable.php |  5 ++++-
 templates/Users/add.php              |  3 ++-
 3 files changed, 8 insertions(+), 20 deletions(-)

diff --git a/src/Controller/UsersController.php b/src/Controller/UsersController.php
index bf4ac1a..2c56491 100644
--- a/src/Controller/UsersController.php
+++ b/src/Controller/UsersController.php
@@ -94,7 +94,7 @@ class UsersController extends AppController
                         throw new MethodNotAllowedException(__('No valid organisation found. Either encode the organisation separately or select a valid one.'));
                     }
                     $data['individual']['alignments'][] = ['type' => 'Member', 'organisation' => ['uuid' => $existingOrg['uuid']]];
-                    $data['individual_id'] = $this->Users->Individuals->captureIndividual($data['individual']);
+                    $data['individual_id'] = $this->Users->Individuals->captureIndividual($data['individual'], true);
                 } else if (!$currentUser['role']['perm_admin'] && isset($data['individual_id'])) {
                     if (!in_array($data['individual_id'], $individual_ids)) {
                         throw new MethodNotAllowedException(__('The selected individual is not aligned with your organisation. Creating a user for them is not permitted.'));
@@ -188,11 +188,6 @@ class UsersController extends AppController
         $individual_ids = [];
         if (!$currentUser['role']['perm_admin']) {
             $validRoles = $this->Users->Roles->find('list')->select(['id', 'name'])->order(['name' => 'asc'])->where(['perm_admin' => 0, 'perm_org_admin' => 0])->all()->toArray();
-            $individual_ids = $this->Users->Individuals->find('aligned', ['organisation_id' => $currentUser['organisation_id']])->all()->extract('id')->toArray();
-            if (empty($individual_ids)) {
-                $individual_ids = [-1];
-            }
-            $individuals_params['conditions'] = ['id IN' => $individual_ids];
         } else {
             $validRoles = $this->Users->Roles->find('list')->order(['name' => 'asc'])->all()->toArray();
         }
@@ -212,7 +207,7 @@ class UsersController extends AppController
             'contain' => ['Roles', ],
         ];
         if ($this->request->is(['get'])) {
-            $params['fields'] = array_merge($params['fields'], ['individual_id', 'role_id', 'disabled']);
+            $params['fields'] = array_merge($params['fields'], ['role_id', 'disabled']);
             if (!empty($this->ACL->getUser()['role']['perm_admin'])) {
                 $params['fields'][] = 'organisation_id';
             }
@@ -228,7 +223,6 @@ class UsersController extends AppController
             }
         }
         if ($this->request->is(['post', 'put']) && !empty($this->ACL->getUser()['role']['perm_admin'])) {
-            $params['fields'][] = 'individual_id';
             $params['fields'][] = 'role_id';
             $params['fields'][] = 'organisation_id';
             $params['fields'][] = 'disabled';
@@ -258,22 +252,12 @@ class UsersController extends AppController
         if (!empty($responsePayload)) {
             return $responsePayload;
         }
-        $dropdownData = [
-            'role' => $validRoles,
-            'individual' => $this->Users->Individuals->find('list', [
-                'sort' => ['email' => 'asc']
-            ]),
-            'organisation' => $this->Users->Organisations->find('list', [
-                'sort' => ['name' => 'asc']
-            ])
-        ];
         $org_conditions = [];
         if (empty($currentUser['role']['perm_admin'])) {
             $org_conditions = ['id' => $currentUser['organisation_id']];
         }
         $dropdownData = [
             'role' => $validRoles,
-            'individual' => $this->Users->Individuals->find('list', $individuals_params)->toArray(),
             'organisation' => $this->Users->Organisations->find('list', [
                 'sort' => ['name' => 'asc'],
                 'conditions' => $org_conditions
diff --git a/src/Model/Table/IndividualsTable.php b/src/Model/Table/IndividualsTable.php
index c0b81ab..04f80fb 100644
--- a/src/Model/Table/IndividualsTable.php
+++ b/src/Model/Table/IndividualsTable.php
@@ -54,7 +54,7 @@ class IndividualsTable extends AppTable
         return $validator;
     }
 
-    public function captureIndividual($individual): ?int
+    public function captureIndividual($individual, $skipUpdate = false): ?int
     {
         if (!empty($individual['uuid'])) {
             $existingIndividual = $this->find()->where([
@@ -71,6 +71,9 @@ class IndividualsTable extends AppTable
                 'accessibleFields' => $entityToSave->getAccessibleFieldForNew()
             ]);
         } else {
+            if ($skipUpdate) {
+                return $existingIndividual->id;
+            }
             $this->patchEntity($existingIndividual, $individual);
             $entityToSave = $existingIndividual;
         }
diff --git a/templates/Users/add.php b/templates/Users/add.php
index b83f84e..93a8d56 100644
--- a/templates/Users/add.php
+++ b/templates/Users/add.php
@@ -20,7 +20,8 @@
                     'field' => 'individual_id',
                     'type' => 'dropdown',
                     'label' => __('Associated individual'),
-                    'options' => $dropdownData['individual']
+                    'options' => isset($dropdownData['individual']) ? $dropdownData['individual'] : [],
+                    'conditions' => $this->request->getParam('action') === 'add'
                 ],
                 [
                     'field' => 'individual.email',