diff --git a/app/Controller/UserSettingsController.php b/app/Controller/UserSettingsController.php index 3ce34a653..402d7f383 100644 --- a/app/Controller/UserSettingsController.php +++ b/app/Controller/UserSettingsController.php @@ -274,15 +274,30 @@ class UserSettingsController extends AppController */ return $this->RestResponse->describe('UserSettings', 'delete', false, $this->response->type()); } - // check if the ID is valid and whether a user setting with the given ID exists - if (empty($id) || !is_numeric($id)) { - throw new InvalidArgumentException(__('Invalid ID passed.')); + + if (!$this->request->is('post') && !$this->request->is('delete')) { + throw new MethodNotAllowedException(__('Expecting POST or DELETE request.')); } + + if (empty($id)) { + if (empty($this->request->data['setting'])) { + throw new BadRequestException("No setting name to delete provided."); + } + $conditions = ['UserSetting.setting' => $this->request->data['setting']]; + if (!empty($this->request->data['user_id'])) { + $conditions['UserSetting.user_id'] = $this->request->data['user_id']; + } else { + $conditions['UserSetting.user_id'] = $this->Auth->user('id'); // current user + } + } else if (is_numeric($id)) { + $conditions = ['UserSetting.id' => $id]; + } else { + throw new BadRequestException(__('Invalid ID passed.')); + } + $userSetting = $this->UserSetting->find('first', array( 'recursive' => -1, - 'conditions' => array( - 'UserSetting.id' => $id - ), + 'conditions' => $conditions, 'contain' => array('User.id', 'User.org_id') )); if (empty($userSetting)) { @@ -296,34 +311,30 @@ class UserSettingsController extends AppController if ($settingPermCheck !== true) { throw new MethodNotAllowedException(__('This setting is restricted and requires the following permission(s): %s', $settingPermCheck)); } - if ($this->request->is('post') || $this->request->is('delete')) { - // Delete the setting that we were after. - $result = $this->UserSetting->delete($userSetting['UserSetting']['id']); - if ($result) { - // set the response for both the UI and API - $message = __('Setting deleted.'); - if ($this->_isRest()) { - return $this->RestResponse->saveSuccessResponse('UserSettings', 'delete', $id, $this->response->type(), $message); - } else { - $this->Flash->success($message); - } + // Delete the setting that we were after. + $result = $this->UserSetting->delete($userSetting['UserSetting']['id']); + if ($result) { + // set the response for both the UI and API + $message = __('Setting deleted.'); + if ($this->_isRest()) { + return $this->RestResponse->saveSuccessResponse('UserSettings', 'delete', $userSetting['UserSetting']['id'], $this->response->type(), $message); } else { - // set the response for both the UI and API - $message = __('Setting could not be deleted.'); - if ($this->_isRest()) { - return $this->RestResponse->saveFailResponse('UserSettings', 'delete', $id, $message, $this->response->type()); - } else { - $this->Flash->error($message); - } + $this->Flash->success($message); } - /* - * The API responses stopped executing this function and returned a serialised response to the user. - * For UI users, redirect to where they issued the request from. - */ - $this->redirect($this->referer()); } else { - throw new MethodNotAllowedException(__('Expecting POST or DELETE request.')); + // set the response for both the UI and API + $message = __('Setting could not be deleted.'); + if ($this->_isRest()) { + return $this->RestResponse->saveFailResponse('UserSettings', 'delete', $userSetting['UserSetting']['id'], $message, $this->response->type()); + } else { + $this->Flash->error($message); + } } + /* + * The API responses stopped executing this function and returned a serialised response to the user. + * For UI users, redirect to where they issued the request from. + */ + $this->redirect($this->referer()); } public function setHomePage() diff --git a/tests/testlive_security.py b/tests/testlive_security.py index 4faa345ec..b0cba44e8 100644 --- a/tests/testlive_security.py +++ b/tests/testlive_security.py @@ -1515,6 +1515,25 @@ class TestSecurity(unittest.TestCase): self.admin_misp_connector.delete_user(user) self.admin_misp_connector.delete_organisation(org) + def test_user_setting_delete(self): + # Admin user can set their own user setting + setting = self.admin_misp_connector.set_user_setting('publish_alert_filter', {'Tag.name': 'test_publish_filter'}) + check_response(setting) + + logged_in = PyMISP(url, self.test_usr.authkey) + logged_in.global_pythonify = True + + # Normal user should not be able to delete setting for different user + deleted = logged_in.delete_user_setting('publish_alert_filter', self.admin_misp_connector._current_user) + self.assertEqual(deleted["errors"][0], 404, deleted) + + setting = self.admin_misp_connector.get_user_setting('publish_alert_filter') + check_response(setting) + self.assertEqual({'Tag.name': 'test_publish_filter'}, setting.value) + + # User should be able to delete self setting + check_response(self.admin_misp_connector.delete_user_setting('publish_alert_filter')) + def __generate_event(self, distribution: int = 1) -> MISPEvent: mispevent = MISPEvent() mispevent.info = 'This is a super simple test'