# HG changeset patch # User Marcin Lulek # Date 2017-10-18 10:16:40 # Node ID d1b664006fba724b08dfa4475ab00ceeb9135bfb # Parent a3d55bf9709e3d11f335a2d0ce36c0cde61496da repo-settings: ensure deletion on repo settings model validate the settings id against the initialized model repo. - prevents from malicious deletions of settings by forgin IDs diff --git a/rhodecode/model/settings.py b/rhodecode/model/settings.py --- a/rhodecode/model/settings.py +++ b/rhodecode/model/settings.py @@ -44,8 +44,9 @@ SOCIAL_PLUGINS_LIST = ['github', 'bitbuc class SettingNotFound(Exception): - def __init__(self): - super(SettingNotFound, self).__init__('Setting is not found') + def __init__(self, setting_id): + msg = 'Setting `{}` is not found'.format(setting_id) + super(SettingNotFound, self).__init__(msg) class SettingsModel(BaseModel): @@ -155,7 +156,7 @@ class SettingsModel(BaseModel): def delete_ui(self, id_): ui = self.UiDbModel.get(id_) if not ui: - raise SettingNotFound() + raise SettingNotFound(id_) Session().delete(ui) def get_setting_by_name(self, name): @@ -635,7 +636,13 @@ class VcsSettingsModel(object): @assert_repo_settings def delete_repo_svn_pattern(self, id_): - self.repo_settings.delete_ui(id_) + ui = self.repo_settings.UiDbModel.get(id_) + if ui and ui.repository.repo_name == self.repo_settings.repo: + # only delete if it's the same repo as initialized settings + self.repo_settings.delete_ui(id_) + else: + # raise error as if we wouldn't find this option + self.repo_settings.delete_ui(-1) def delete_global_svn_pattern(self, id_): self.global_settings.delete_ui(id_) diff --git a/rhodecode/tests/models/settings/test_settings.py b/rhodecode/tests/models/settings/test_settings.py --- a/rhodecode/tests/models/settings/test_settings.py +++ b/rhodecode/tests/models/settings/test_settings.py @@ -491,7 +491,7 @@ class TestDeleteUiValue(object): model = SettingsModel() with pytest.raises(SettingNotFound) as exc_info: model.delete_ui(id_) - assert exc_info.value.message == 'Setting is not found' + assert exc_info.value.message == 'Setting `{}` is not found'.format(id_) def test_delete_ui_when_repo_is_not_set(self, settings_util): model = SettingsModel() diff --git a/rhodecode/tests/models/settings/test_vcs_settings.py b/rhodecode/tests/models/settings/test_vcs_settings.py --- a/rhodecode/tests/models/settings/test_vcs_settings.py +++ b/rhodecode/tests/models/settings/test_vcs_settings.py @@ -635,13 +635,24 @@ class TestCreateOrUpdateGlobalGitSetting class TestDeleteRepoSvnPattern(object): - def test_success_when_repo_is_set(self, backend_svn): + def test_success_when_repo_is_set(self, backend_svn, settings_util): + repo = backend_svn.create_repo() + repo_name = repo.repo_name + + model = VcsSettingsModel(repo=repo_name) + entry = settings_util.create_repo_rhodecode_ui( + repo, VcsSettingsModel.SVN_BRANCH_SECTION, 'svn-branch') + Session().commit() + + model.delete_repo_svn_pattern(entry.ui_id) + + def test_fail_when_delete_id_from_other_repo(self, backend_svn): repo_name = backend_svn.repo_name model = VcsSettingsModel(repo=repo_name) delete_ui_patch = mock.patch.object(model.repo_settings, 'delete_ui') with delete_ui_patch as delete_ui_mock: model.delete_repo_svn_pattern(123) - delete_ui_mock.assert_called_once_with(123) + delete_ui_mock.assert_called_once_with(-1) def test_raises_exception_when_repository_is_not_specified(self): model = VcsSettingsModel()