diff --git a/src/Controller/AuthKeysController.php b/src/Controller/AuthKeysController.php index 9ed43c3..978810c 100644 --- a/src/Controller/AuthKeysController.php +++ b/src/Controller/AuthKeysController.php @@ -84,7 +84,7 @@ class AuthKeysController extends AppController 'displayOnSuccess' => 'authkey_display', 'beforeSave' => function($data) use ($users) { if (!in_array($data['user_id'], array_keys($users))) { - return false; + throw new MethodNotAllowedException(__('You are not authorised to do that.')); } return $data; } diff --git a/src/Controller/Component/CRUDComponent.php b/src/Controller/Component/CRUDComponent.php index 14369ab..41fe9d1 100644 --- a/src/Controller/Component/CRUDComponent.php +++ b/src/Controller/Component/CRUDComponent.php @@ -169,6 +169,12 @@ class CRUDComponent extends Component if (!empty($params['fields'])) { $patchEntityParams['fields'] = $params['fields']; } + if (isset($params['beforeMarshal'])) { + $input = $params['beforeMarshal']($input); + if ($input === false) { + throw new NotFoundException(__('Could not save {0} due to the marshaling failing. Your input is bad and you should feel bad.', $this->ObjectAlias)); + } + } $data = $this->Table->patchEntity($data, $input, $patchEntityParams); if (isset($params['beforeSave'])) { $data = $params['beforeSave']($data); @@ -310,9 +316,18 @@ class CRUDComponent extends Component if (!empty($params['fields'])) { $patchEntityParams['fields'] = $params['fields']; } + if (isset($params['beforeMarshal'])) { + $input = $params['beforeMarshal']($input); + if ($input === false) { + throw new NotFoundException(__('Could not save {0} due to the marshaling failing. Your input is bad and you should feel bad.', $this->ObjectAlias)); + } + } $data = $this->Table->patchEntity($data, $input, $patchEntityParams); if (isset($params['beforeSave'])) { $data = $params['beforeSave']($data); + if ($data === false) { + throw new NotFoundException(__('Could not save {0} due to the input failing to meet expectations. Your input is bad and you should feel bad.', $this->ObjectAlias)); + } } $savedData = $this->Table->save($data); if ($savedData !== false) { diff --git a/src/Controller/Component/FloodProtectionComponent.php b/src/Controller/Component/FloodProtectionComponent.php index 6fbd0ec..91668b6 100644 --- a/src/Controller/Component/FloodProtectionComponent.php +++ b/src/Controller/Component/FloodProtectionComponent.php @@ -17,7 +17,14 @@ class FloodProtectionComponent extends Component public function initialize(array $config): void { $ip_source = Configure::check('security.logging.ip_source') ? Configure::read('security.logging.ip_source') : 'REMOTE_ADDR'; - $this->remote_ip = $_SERVER[$ip_source]; + if (!isset($_SERVER[$ip_source])) { + $ip_source = 'REMOTE_ADDR'; + } + if (isset($_SERVER[$ip_source])) { + $this->remote_ip = $_SERVER[$ip_source]; + } else { + $this->remote_ip = '127.0.0.1'; + } $temp = explode(PHP_EOL, $_SERVER[$ip_source]); if (count($temp) > 1) { $this->remote_ip = $temp[0]; diff --git a/src/Controller/SharingGroupsController.php b/src/Controller/SharingGroupsController.php index aa96cb4..e03aee5 100644 --- a/src/Controller/SharingGroupsController.php +++ b/src/Controller/SharingGroupsController.php @@ -37,10 +37,17 @@ class SharingGroupsController extends AppController public function add() { + $currentUser = $this->ACL->getUser(); $this->CRUD->add([ 'override' => [ 'user_id' => $this->ACL->getUser()['id'] - ] + ], + 'beforeSave' => function($data) use ($currentUser) { + if (!$currentUser['role']['perm_admin']) { + $data['organisation_id'] = $currentUser['organisation_id']; + } + return $data; + } ]); $dropdownData = [ 'organisation' => $this->getAvailableOrgForSg($this->ACL->getUser()) diff --git a/src/Controller/UsersController.php b/src/Controller/UsersController.php index 3d684a0..27687ba 100644 --- a/src/Controller/UsersController.php +++ b/src/Controller/UsersController.php @@ -9,6 +9,7 @@ use \Cake\Database\Expression\QueryExpression; use Cake\Http\Exception\UnauthorizedException; use Cake\Http\Exception\MethodNotAllowedException; use Cake\Core\Configure; +use Cake\Utility\Security; class UsersController extends AppController { @@ -44,15 +45,30 @@ class UsersController extends AppController { $currentUser = $this->ACL->getUser(); $validRoles = []; + $individuals_params = [ + 'sort' => ['email' => 'asc'] + ]; + $individual_ids = []; if (!$currentUser['role']['perm_admin']) { - $validRoles = $this->Users->Roles->find('list')->select(['id', 'name'])->order(['name' => 'asc'])->where(['perm_admin' => 0])->all()->toArray(); + $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(); } $defaultRole = $this->Users->Roles->find()->select(['id'])->first()->toArray(); - + $individuals = $this->Users->Individuals->find('list', $individuals_params)->toArray(); $this->CRUD->add([ - 'beforeSave' => function($data) use ($currentUser, $validRoles, $defaultRole) { + 'beforeMarshal' => function($data) { + if (empty($data['password'])) { + $data['password'] = Security::randomString(20); + } + return $data; + }, + 'beforeSave' => function($data) use ($currentUser, $validRoles, $defaultRole, $individual_ids) { if (!isset($data['role_id']) && !empty($defaultRole)) { $data['role_id'] = $defaultRole['id']; } @@ -62,6 +78,21 @@ class UsersController extends AppController throw new MethodNotAllowedException(__('You do not have permission to assign that role.')); } } + if ((!isset($data['individual_id']) || $data['individual_id'] === 'new') && !empty($data['individual'])) { + $existingOrg = $this->Users->Organisations->find('all')->where(['id' => $data['organisation_id']])->select(['uuid'])->first(); + if (empty($existingOrg)) { + 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']); + } 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.')); + } + } + if (empty($data['individual_id'])) { + throw new MethodNotAllowedException(__('No valid individual found. Either supply it in the request or set the individual_id to a valid value.')); + } $this->Users->enrollUserRouter($data); return $data; } @@ -84,9 +115,7 @@ class UsersController extends AppController } $dropdownData = [ 'role' => $validRoles, - 'individual' => $this->Users->Individuals->find('list', [ - 'sort' => ['email' => 'asc'] - ]), + 'individual' => $individuals, 'organisation' => $this->Users->Organisations->find('list', [ 'sort' => ['name' => 'asc'], 'conditions' => $org_conditions @@ -118,7 +147,7 @@ class UsersController extends AppController $currentUser = $this->ACL->getUser(); $validRoles = []; if (!$currentUser['role']['perm_admin']) { - $validRoles = $this->Users->Roles->find('list')->select(['id', 'name'])->order(['name' => 'asc'])->where(['perm_admin' => 0])->all()->toArray(); + $validRoles = $this->Users->Roles->find('list')->select(['id', 'name'])->order(['name' => 'asc'])->where(['perm_admin' => 0, 'perm_org_admin' => 0])->all()->toArray(); } else { $validRoles = $this->Users->Roles->find('list')->order(['name' => 'asc'])->all()->toArray(); } @@ -136,7 +165,7 @@ class UsersController extends AppController $params = [ 'get' => [ 'fields' => [ - 'id', 'individual_id', 'role_id', 'username', 'disabled' + 'id', 'individual_id', 'role_id', 'disabled', 'username' ] ], 'removeEmpty' => [ @@ -148,12 +177,10 @@ class UsersController extends AppController ]; if (!empty($this->ACL->getUser()['role']['perm_admin'])) { $params['fields'][] = 'individual_id'; - $params['fields'][] = 'username'; $params['fields'][] = 'role_id'; $params['fields'][] = 'organisation_id'; $params['fields'][] = 'disabled'; } else if (!empty($this->ACL->getUser()['role']['perm_org_admin'])) { - $params['fields'][] = 'username'; $params['fields'][] = 'role_id'; $params['fields'][] = 'disabled'; if (!$currentUser['role']['perm_admin']) { @@ -166,6 +193,12 @@ class UsersController extends AppController } return $data; }; + $params['beforeSave'] = function ($data) use ($currentUser, $validRoles) { + if (!in_array($data['role_id'], array_keys($validRoles))) { + throw new MethodNotAllowedException(__('You cannot assign the chosen role to a user.')); + } + return $data; + }; } } $this->CRUD->edit($id, $params); @@ -311,7 +344,7 @@ class UsersController extends AppController if (empty(Configure::read('security.registration.self-registration'))) { throw new UnauthorizedException(__('User self-registration is not open.')); } - if (!empty(Configure::read('security.registration.floodProtection'))) { + if (!Configure::check('security.registration.floodProtection') || Configure::read('security.registration.floodProtection')) { $this->FloodProtection->check('register'); } if ($this->request->is('post')) { diff --git a/src/Lib/default/local_tool_connectors/MispConnector.php b/src/Lib/default/local_tool_connectors/MispConnector.php index c872675..66f5024 100644 --- a/src/Lib/default/local_tool_connectors/MispConnector.php +++ b/src/Lib/default/local_tool_connectors/MispConnector.php @@ -132,9 +132,9 @@ class MispConnector extends CommonConnectorTools { return $validator ->requirePresence('url') - ->notEmpty('url', __('An URL must be provided')) + ->notEmptyString('url', __('An URL must be provided')) ->requirePresence('authkey') - ->notEmpty('authkey', __('An Authkey must be provided')) + ->notEmptyString('authkey', __('An Authkey must be provided')) ->lengthBetween('authkey', [40, 40], __('The authkey must be 40 character long')) ->boolean('skip_ssl'); } diff --git a/src/Model/Behavior/AuthKeycloakBehavior.php b/src/Model/Behavior/AuthKeycloakBehavior.php index 12bb3e2..7cbb5ca 100644 --- a/src/Model/Behavior/AuthKeycloakBehavior.php +++ b/src/Model/Behavior/AuthKeycloakBehavior.php @@ -137,6 +137,30 @@ class AuthKeycloakBehavior extends Behavior ] ] ); + $logChange = [ + 'username' => $data['username'], + 'individual_id' => $data['individual_id'], + 'role_id' => $data['role_id'] + ]; + if (!$response->isOk()) { + $logChange['error_code'] = $response->getStatusCode(); + $logChange['error_body'] = $response->getStringBody(); + $this->_table->auditLogs()->insert([ + 'request_action' => 'enrollUser', + 'model' => 'User', + 'model_id' => 0, + 'model_title' => __('Failed Keycloak enrollment for user {0}', $data['username']), + 'changed' => $logChange + ]); + } else { + $this->_table->auditLogs()->insert([ + 'request_action' => 'enrollUser', + 'model' => 'User', + 'model_id' => 0, + 'model_title' => __('Successful Keycloak enrollment for user {0}', $data['username']), + 'changed' => $logChange + ]); + } return true; } diff --git a/src/Model/Table/AlignmentsTable.php b/src/Model/Table/AlignmentsTable.php index c05ad90..3cbf5c5 100644 --- a/src/Model/Table/AlignmentsTable.php +++ b/src/Model/Table/AlignmentsTable.php @@ -42,6 +42,7 @@ class AlignmentsTable extends AppTable 'type' => $type ]; $this->patchEntity($alignment, $data); + $this->save($alignment); } } } diff --git a/src/Model/Table/IndividualsTable.php b/src/Model/Table/IndividualsTable.php index 16a3da2..de5b8e8 100644 --- a/src/Model/Table/IndividualsTable.php +++ b/src/Model/Table/IndividualsTable.php @@ -5,6 +5,8 @@ namespace App\Model\Table; use App\Model\Table\AppTable; use Cake\ORM\Table; use Cake\Validation\Validator; +use Cake\ORM\Query; + class IndividualsTable extends AppTable { @@ -55,7 +57,9 @@ class IndividualsTable extends AppTable 'uuid' => $individual['uuid'] ])->first(); } else { - return null; + $existingIndividual = $this->find()->where([ + 'email' => $individual['email'] + ])->first(); } if (empty($existingIndividual)) { $entityToSave = $this->newEmptyEntity(); @@ -90,4 +94,16 @@ class IndividualsTable extends AppTable } } } + + public function findAligned(Query $query, array $options) + { + $query = $query->select(['Individuals.id']); + if (empty($options['organisation_id'])) { + $query->leftJoinWith('Alignments')->where(['Alignments.organisation_id IS' => null]); + } else { + $query->innerJoinWith('Alignments') + ->where(['Alignments.organisation_id IN' => $options['organisation_id']]); + } + return $query->group(['Individuals.id', 'Individuals.uuid']); + } } diff --git a/src/Model/Table/SettingProviders/CerebrateSettingsProvider.php b/src/Model/Table/SettingProviders/CerebrateSettingsProvider.php index 330e589..9cf755d 100644 --- a/src/Model/Table/SettingProviders/CerebrateSettingsProvider.php +++ b/src/Model/Table/SettingProviders/CerebrateSettingsProvider.php @@ -8,6 +8,7 @@ require_once(APP . 'Model' . DS . 'Table' . DS . 'SettingProviders' . DS . 'Base use App\Settings\SettingsProvider\BaseSettingsProvider; use App\Settings\SettingsProvider\SettingValidator; +use Cake\Core\Configure; class CerebrateSettingsProvider extends BaseSettingsProvider { @@ -150,6 +151,17 @@ class CerebrateSettingsProvider extends BaseSettingsProvider ], 'Authentication' => [ 'Providers' => [ + 'PasswordAuth' => [ + 'password_auth.enabled' => [ + 'name' => 'Disable password authentication', + 'type' => 'boolean', + 'severity' => 'warning', + 'description' => __('Enable username/password authentication.'), + 'default' => true, + 'test' => 'testEnabledAuth', + 'authentication_type' => 'password_auth' + ], + ], 'KeyCloak' => [ 'keycloak.enabled' => [ 'name' => 'Enabled', @@ -157,6 +169,8 @@ class CerebrateSettingsProvider extends BaseSettingsProvider 'severity' => 'warning', 'description' => __('Enable keycloak authentication'), 'default' => false, + 'test' => 'testEnabledAuth', + 'authentication_type' => 'keycloak' ], 'keycloak.provider.applicationId' => [ 'name' => 'Client ID', @@ -300,8 +314,10 @@ class CerebrateSettingsProvider extends BaseSettingsProvider 'security.registration.floodProtection' => [ 'name' => __('Enable registration flood-protection'), 'type' => 'boolean', - 'description' => __('Enabling this setting will only allow 5 registrations / IP address every 15 minutes (rolling time-frame).'), - 'default' => false, + 'description' => (Configure::check('security.logging.ip_source') && Configure::read('security.logging.ip_source') !== 'REMOTE_ADDR') ? + __('Enabling this setting will only allow 5 registrations / IP address every 15 minutes (rolling time-frame). WARNING: Be aware that you are not using REMOTE_ADDR (as configured via security.logging.ip_source) - this could lead to an attacker being able to spoof their IP and circumvent the flood protection. Only rely on the client IP if your reverse proxy in front of Cerebrate is properly setting this header.'): + __('Enabling this setting will only allow 5 registrations / IP address every 15 minutes (rolling time-frame).'), + 'default' => true, ], ] ], @@ -371,4 +387,24 @@ class CerebrateSettingValidator extends SettingValidator } return true; } + + public function testEnabledAuth($value, &$setting) + { + $providers = [ + 'password_auth', + 'keycloak' + ]; + if (!$value) { + $foundEnabledAuth = __('Cannot make change - this would disable every possible authentication method.'); + foreach ($providers as $provider) { + if ($provider !== $setting['authentication_type']) { + if (Configure::read($provider . '.enabled')) { + $foundEnabledAuth = true; + } + } + } + return $foundEnabledAuth; + } + return true; + } } diff --git a/templates/Users/add.php b/templates/Users/add.php index d3fcb7f..1f68ae5 100644 --- a/templates/Users/add.php +++ b/templates/Users/add.php @@ -1,4 +1,12 @@ request->getParam('action') === 'add') { + $dropdownData['individual'] = ['new' => __('New individual')] + $dropdownData['individual']; + if (!Configure::check('password_auth.enabled') || Configure::read('password_auth.enabled')) { + $passwordRequired = 'required'; + } + } echo $this->element('genericElements/Form/genericForm', [ 'data' => [ 'description' => __('Roles define global rules for a set of users, including first and foremost access controls to certain functionalities.'), @@ -10,6 +18,32 @@ 'label' => __('Associated individual'), 'options' => $dropdownData['individual'] ], + [ + 'field' => 'individual.email', + 'stateDependence' => [ + 'source' => '#individual_id-field', + 'option' => 'new' + ], + 'required' => false + ], + [ + 'field' => 'individual.first_name', + 'label' => 'First name', + 'stateDependence' => [ + 'source' => '#individual_id-field', + 'option' => 'new' + ], + 'required' => false + ], + [ + 'field' => 'individual.last_name', + 'label' => 'Last name', + 'stateDependence' => [ + 'source' => '#individual_id-field', + 'option' => 'new' + ], + 'required' => false + ], [ 'field' => 'username', 'autocomplete' => 'off' @@ -25,16 +59,18 @@ 'field' => 'password', 'label' => __('Password'), 'type' => 'password', - 'required' => $this->request->getParam('action') === 'add' ? 'required' : false, + 'required' => $passwordRequired, 'autocomplete' => 'new-password', - 'value' => '' + 'value' => '', + 'requirements' => (bool)$passwordRequired ], [ 'field' => 'confirm_password', 'label' => __('Confirm Password'), 'type' => 'password', - 'required' => $this->request->getParam('action') === 'add' ? 'required' : false, - 'autocomplete' => 'off' + 'required' => $passwordRequired, + 'autocomplete' => 'off', + 'requirements' => (bool)$passwordRequired ], [ 'field' => 'role_id', diff --git a/templates/Users/login.php b/templates/Users/login.php index 8f2e3b2..7317a43 100644 --- a/templates/Users/login.php +++ b/templates/Users/login.php @@ -12,26 +12,29 @@ use Cake\Core\Configure; 'style' => ['filter: drop-shadow(4px 4px 4px #924da666);'] ]) ); - echo sprintf('

%s

', __('Sign In')); $template = [ 'inputContainer' => '
{{content}}
', 'formGroup' => '{{input}}{{label}}', 'submitContainer' => '
{{content}}
', ]; $this->Form->setTemplates($template); - echo $this->Form->create(null, ['url' => ['controller' => 'users', 'action' => 'login']]); - echo $this->Form->control('username', ['label' => 'Username', 'class' => 'form-control mb-2', 'placeholder' => __('Username')]); - echo $this->Form->control('password', ['type' => 'password', 'label' => 'Password', 'class' => 'form-control mb-3', 'placeholder' => __('Password')]); - echo $this->Form->control(__('Login'), ['type' => 'submit', 'class' => 'btn btn-primary']); - echo $this->Form->end(); - if (!empty(Configure::read('security.registration.self-registration'))) { - echo '
'; - echo sprintf('%s %s', __('Doesn\'t have an account?'), __('Sign up')); - echo '
'; + if (!Configure::check('password_auth.enabled') || Configure::read('password_auth.enabled')) { + echo sprintf('

%s

', __('Sign In')); + echo $this->Form->create(null, ['url' => ['controller' => 'users', 'action' => 'login']]); + echo $this->Form->control('username', ['label' => 'Username', 'class' => 'form-control mb-2', 'placeholder' => __('Username')]); + echo $this->Form->control('password', ['type' => 'password', 'label' => 'Password', 'class' => 'form-control mb-3', 'placeholder' => __('Password')]); + echo $this->Form->control(__('Login'), ['type' => 'submit', 'class' => 'btn btn-primary']); + echo $this->Form->end(); + if (!empty(Configure::read('security.registration.self-registration'))) { + echo '
'; + echo sprintf('%s %s', __('Don\'t have an account?'), __('Sign up')); + echo '
'; + } + if (!empty(Configure::read('keycloak.enabled'))) { + echo sprintf('
%s
', __('Or')); + } } - if (!empty(Configure::read('keycloak.enabled'))) { - echo sprintf('
%s
', __('Or')); echo $this->Form->create(null, [ 'url' => Cake\Routing\Router::url([ 'prefix' => false, diff --git a/templates/Users/view.php b/templates/Users/view.php index fbddf52..3af7c4c 100644 --- a/templates/Users/view.php +++ b/templates/Users/view.php @@ -17,8 +17,11 @@ echo $this->element( 'path' => 'username' ], [ + 'type' => 'generic', 'key' => __('Email'), - 'path' => 'individual.email' + 'path' => 'individual.email', + 'url' => '/individuals/view/{{0}}', + 'url_vars' => 'individual_id' ], [ 'type' => 'generic', diff --git a/templates/element/genericElements/IndexTable/Fields/actions.php b/templates/element/genericElements/IndexTable/Fields/actions.php index c280aad..8a215c4 100644 --- a/templates/element/genericElements/IndexTable/Fields/actions.php +++ b/templates/element/genericElements/IndexTable/Fields/actions.php @@ -98,7 +98,7 @@ ); } $reload_url = !empty($action['reload_url']) ? $action['reload_url'] : $this->Url->build(['action' => 'index']); - $action['onclick'] = sprintf('UI.submissionModalForIndex(\'%s\', \'%s\', \'%s\')', $modal_url, $reload_url, $tableRandomValue); + $action['onclick'] = sprintf('UI.submissionModalForIndex(\'%s\', \'%s\', \'%s\')', h($modal_url), h($reload_url), h($tableRandomValue)); } echo sprintf( ' ', diff --git a/templates/genericTemplates/delete.php b/templates/genericTemplates/delete.php index f2b7449..e838a6b 100644 --- a/templates/genericTemplates/delete.php +++ b/templates/genericTemplates/delete.php @@ -18,7 +18,11 @@ $form = $this->element('genericElements/Form/genericForm', [ ]); $formHTML = sprintf('
%s
', $form); -$bodyMessage = !empty($deletionText) ? __($deletionText) : __('Are you sure you want to delete {0} #{1}?', h(Cake\Utility\Inflector::singularize($this->request->getParam('controller'))), h($id)); +if (!empty($id)) { + $bodyMessage = !empty($deletionText) ? __($deletionText) : __('Are you sure you want to delete {0} #{1}?', h(Cake\Utility\Inflector::singularize($this->request->getParam('controller'))), h($id)); +} else { + $bodyMessage = !empty($deletionText) ? __($deletionText) : __('Are you sure you want to delete the given {0}?', h(Cake\Utility\Inflector::singularize($this->request->getParam('controller')))); +} $bodyHTML = sprintf('%s%s', $formHTML, $bodyMessage); echo $this->Bootstrap->modal([ diff --git a/tests/TestCase/Api/AuthKeys/AddAuthKeyApiTest.php b/tests/TestCase/Api/AuthKeys/AddAuthKeyApiTest.php index 2ede468..afb836f 100644 --- a/tests/TestCase/Api/AuthKeys/AddAuthKeyApiTest.php +++ b/tests/TestCase/Api/AuthKeys/AddAuthKeyApiTest.php @@ -65,8 +65,7 @@ class AddAuthKeyApiTest extends TestCase ] ); - $this->assertResponseCode(404); - $this->addWarning('Should return 405 Method Not Allowed instead of 404 Not Found'); + $this->assertResponseCode(405); $this->assertDbRecordNotExists('AuthKeys', ['uuid' => $uuid]); } }