# HG changeset patch # User Mads Kiilerich # Date 2018-05-29 10:25:41 # Node ID 083fbf531a5dc0550954373799e8727b7ccb8900 # Parent eeb8ddecaee24ee562e42e0614ce5a484c831e78 repos: only allow api repo creation in existing groups Fix problem with '../something' paths being allowed; '..' will always exist and can't be created. This also introduce a small API change: Repository groups must now exist before repositories can be created. This makes the API more explicit and simpler. This issue was found and reported by Kacper Szurek https://security.szurek.pl/ diff --git a/docs/api/api.rst b/docs/api/api.rst --- a/docs/api/api.rst +++ b/docs/api/api.rst @@ -750,10 +750,12 @@ OUTPUT:: create_repo ----------- -Create a repository. If the repository name contains "/", all needed repository -groups will be created. For example "foo/bar/baz" will create repository groups -"foo", "bar" (with "foo" as parent), and create "baz" repository with -"bar" as group. +Create a repository. If the repository name contains "/", the repository will be +created in the repository group indicated by that path. Any such repository +groups need to exist before calling this method, or the call will fail. +For example "foo/bar/baz" will create a repository "baz" inside the repository +group "bar" which itself is in a repository group "foo", but both "foo" and +"bar" already need to exist before calling this method. This command can only be executed using the api_key of a user with admin rights, or that of a regular user with create repository permission. Regular users cannot specify owner parameter. diff --git a/kallithea/controllers/api/api.py b/kallithea/controllers/api/api.py --- a/kallithea/controllers/api/api.py +++ b/kallithea/controllers/api/api.py @@ -1401,9 +1401,9 @@ class ApiController(JSONRPCController): enable_downloads=Optional(False), copy_permissions=Optional(False)): """ - Creates a repository. If repository name contains "/", all needed repository - groups will be created. For example "foo/bar/baz" will create groups - "foo", "bar" (with "foo" as parent), and create "baz" repository with + Creates a repository. The repository name contains the full path, but the + parent repository group must exist. For example "foo/bar/baz" require the groups + "foo" and "bar" (with "foo" as parent), and create "baz" repository with "bar" as group. This command can be executed only using api_key belonging to user with admin rights or regular user that have create repository permission. Regular users cannot specify owner parameter @@ -1485,11 +1485,15 @@ class ApiController(JSONRPCController): copy_permissions = Optional.extract(copy_permissions) try: - repo_name_cleaned = repo_name.split('/')[-1] - # create structure of groups and return the last group - repo_group = map_groups(repo_name) + repo_name_parts = repo_name.split('/') + repo_group = None + if len(repo_name_parts) > 1: + group_name = '/'.join(repo_name_parts[:-1]) + repo_group = RepoGroup.get_by_group_name(group_name) + if repo_group is None: + raise JSONRPCError("repo group `%s` not found" % group_name) data = dict( - repo_name=repo_name_cleaned, + repo_name=repo_name_parts[-1], repo_name_full=repo_name, repo_type=repo_type, repo_description=description, @@ -1673,14 +1677,18 @@ class ApiController(JSONRPCController): owner = get_user_or_error(owner) try: - # create structure of groups and return the last group - group = map_groups(fork_name) - fork_base_name = fork_name.rsplit('/', 1)[-1] + fork_name_parts = fork_name.split('/') + repo_group = None + if len(fork_name_parts) > 1: + group_name = '/'.join(fork_name_parts[:-1]) + repo_group = RepoGroup.get_by_group_name(group_name) + if repo_group is None: + raise JSONRPCError("repo group `%s` not found" % group_name) form_data = dict( - repo_name=fork_base_name, + repo_name=fork_name_parts[-1], repo_name_full=fork_name, - repo_group=group, + repo_group=repo_group, repo_type=repo.repo_type, description=Optional.extract(description), private=Optional.extract(private), diff --git a/kallithea/tests/api/api_base.py b/kallithea/tests/api/api_base.py --- a/kallithea/tests/api/api_base.py +++ b/kallithea/tests/api/api_base.py @@ -1018,6 +1018,20 @@ class _BaseTestApi(object): repo_group_name = 'my_gr' repo_name = '%s/api-repo' % repo_group_name + # repo creation can no longer also create repo group + id_, params = _build_data(self.apikey, 'create_repo', + repo_name=repo_name, + owner=TEST_USER_ADMIN_LOGIN, + repo_type=self.REPO_TYPE,) + response = api_call(self, params) + expected = u'repo group `%s` not found' % repo_group_name + self._compare_error(id_, expected, given=response.body) + assert RepoModel().get_by_repo_name(repo_name) is None + + # create group before creating repo + rg = fixture.create_repo_group(repo_group_name) + Session().commit() + id_, params = _build_data(self.apikey, 'create_repo', repo_name=repo_name, owner=TEST_USER_ADMIN_LOGIN, @@ -1144,7 +1158,7 @@ class _BaseTestApi(object): self._compare_error(id_, expected, given=response.body) def test_api_create_repo_dot_dot(self): - # FIXME: it should only be possible to create repositories in existing repo groups - and '..' can't be used + # it is only possible to create repositories in existing repo groups - and '..' can't be used group_name = '%s/..' % TEST_REPO_GROUP repo_name = '%s/%s' % (group_name, 'could-be-outside') id_, params = _build_data(self.apikey, 'create_repo', @@ -1152,12 +1166,8 @@ class _BaseTestApi(object): owner=TEST_USER_ADMIN_LOGIN, repo_type=self.REPO_TYPE,) response = api_call(self, params) - expected = { - u'msg': u"Created new repository `%s`" % repo_name, - u'success': True, - u'task': None, - } - self._compare_ok(id_, expected, given=response.body) + expected = u'repo group `%s` not found' % group_name + self._compare_error(id_, expected, given=response.body) fixture.destroy_repo(repo_name) @mock.patch.object(RepoModel, 'create', crash)