fix: [security] reworked the Individual handling of user creations / modifications
- 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-CERTdevelop-unstable
parent
7afcc3977f
commit
da2f904554
|
@ -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
|
||||
|
|
|
@ -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;
|
||||
}
|
||||
|
|
|
@ -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',
|
||||
|
|
Loading…
Reference in New Issue