changeset 8834:516a43cbd814

Merge stable
author Mads Kiilerich <mads@kiilerich.com>
date Thu, 07 Jan 2021 03:28:37 +0100
parents aafca212c8e2 (current diff) 7f3515800bd8 (diff)
children 385d1b31f386
files kallithea/controllers/api/api.py kallithea/templates/admin/permissions/permissions_globals.html kallithea/tests/api/api_base.py
diffstat 3 files changed, 46 insertions(+), 40 deletions(-) [+]
line wrap: on
line diff
--- a/kallithea/controllers/api/api.py	Tue Dec 29 22:23:01 2020 +0100
+++ b/kallithea/controllers/api/api.py	Thu Jan 07 03:28:37 2021 +0100
@@ -1162,7 +1162,7 @@
                 'failed to get repo: `%s` nodes' % repo.repo_name
             )
 
-    @HasPermissionAnyDecorator('hg.admin', 'hg.create.repository')
+    # permission check inside
     def create_repo(self, repo_name, owner=None,
                     repo_type=None, description='',
                     private=False, clone_uri=None,
@@ -1219,6 +1219,19 @@
           }
 
         """
+        group_name = None
+        repo_name_parts = repo_name.split('/')
+        if len(repo_name_parts) > 1:
+            group_name = '/'.join(repo_name_parts[:-1])
+            repo_group = db.RepoGroup.get_by_group_name(group_name)
+            if repo_group is None:
+                raise JSONRPCError("repo group `%s` not found" % group_name)
+            if not(HasPermissionAny('hg.admin')() or HasRepoGroupPermissionLevel('write')(group_name)):
+                raise JSONRPCError("no permission to create repo in %s" % group_name)
+        else:
+            if not HasPermissionAny('hg.admin', 'hg.create.repository')():
+                raise JSONRPCError("no permission to create top level repo")
+
         if not HasPermissionAny('hg.admin')():
             if owner is not None:
                 # forbid setting owner for non-admins
@@ -1244,13 +1257,6 @@
             enable_downloads = defs.get('repo_enable_downloads')
 
         try:
-            repo_name_parts = repo_name.split('/')
-            repo_group = None
-            if len(repo_name_parts) > 1:
-                group_name = '/'.join(repo_name_parts[:-1])
-                repo_group = db.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_parts[-1],
                 repo_name_full=repo_name,
@@ -1258,7 +1264,7 @@
                 repo_description=description,
                 repo_private=private,
                 clone_uri=clone_uri,
-                repo_group=repo_group,
+                repo_group=group_name,
                 repo_landing_rev=landing_rev,
                 enable_statistics=enable_statistics,
                 enable_downloads=enable_downloads,
@@ -1306,10 +1312,10 @@
             if not HasRepoPermissionLevel('admin')(repo.repo_name):
                 raise JSONRPCError('repository `%s` does not exist' % (repoid,))
 
-            if (name != repo.repo_name and
+            if (name != repo.repo_name and repo.group_id is None and
                 not HasPermissionAny('hg.create.repository')()
             ):
-                raise JSONRPCError('no permission to create (or move) repositories')
+                raise JSONRPCError('no permission to create (or move) top level repositories')
 
             if owner is not None:
                 # forbid setting owner for non-admins
@@ -1320,7 +1326,10 @@
         updates = {}
         repo_group = group
         if repo_group is not None:
-            repo_group = get_repo_group_or_error(repo_group)
+            repo_group = get_repo_group_or_error(repo_group)  # TODO: repos can thus currently not be moved to root
+            if repo_group.group_id != repo.group_id:
+                if not(HasPermissionAny('hg.admin')() or HasRepoGroupPermissionLevel('write')(repo_group.group_name)):
+                    raise JSONRPCError("no permission to create (or move) repo in %s" % repo_group.group_name)
             repo_group = repo_group.group_id
         try:
             store_update(updates, name, 'repo_name')
@@ -1343,6 +1352,7 @@
             log.error(traceback.format_exc())
             raise JSONRPCError('failed to update repo `%s`' % repoid)
 
+    # permission check inside
     @HasPermissionAnyDecorator('hg.admin', 'hg.fork.repository')
     def fork_repo(self, repoid, fork_name,
                   owner=None,
@@ -1397,6 +1407,19 @@
             type_ = 'fork' if _repo.fork else 'repo'
             raise JSONRPCError("%s `%s` already exist" % (type_, fork_name))
 
+        group_name = None
+        fork_name_parts = fork_name.split('/')
+        if len(fork_name_parts) > 1:
+            group_name = '/'.join(fork_name_parts[:-1])
+            repo_group = db.RepoGroup.get_by_group_name(group_name)
+            if repo_group is None:
+                raise JSONRPCError("repo group `%s` not found" % group_name)
+            if not(HasPermissionAny('hg.admin')() or HasRepoGroupPermissionLevel('write')(group_name)):
+                raise JSONRPCError("no permission to create repo in %s" % group_name)
+        else:
+            if not HasPermissionAny('hg.admin', 'hg.create.repository')():
+                raise JSONRPCError("no permission to create top level repo")
+
         if HasPermissionAny('hg.admin')():
             pass
         elif HasRepoPermissionLevel('read')(repo.repo_name):
@@ -1405,9 +1428,6 @@
                 raise JSONRPCError(
                     'Only Kallithea admin can specify `owner` param'
                 )
-
-            if not HasPermissionAny('hg.create.repository')():
-                raise JSONRPCError('no permission to create repositories')
         else:
             raise JSONRPCError('repository `%s` does not exist' % (repoid,))
 
@@ -1417,18 +1437,10 @@
         owner = get_user_or_error(owner)
 
         try:
-            fork_name_parts = fork_name.split('/')
-            repo_group = None
-            if len(fork_name_parts) > 1:
-                group_name = '/'.join(fork_name_parts[:-1])
-                repo_group = db.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_name_parts[-1],
                 repo_name_full=fork_name,
-                repo_group=repo_group,
+                repo_group=group_name,
                 repo_type=repo.repo_type,
                 description=description,
                 private=private,
--- a/kallithea/templates/admin/permissions/permissions_globals.html	Tue Dec 29 22:23:01 2020 +0100
+++ b/kallithea/templates/admin/permissions/permissions_globals.html	Thu Jan 07 03:28:37 2021 +0100
@@ -54,7 +54,6 @@
                 <div>
                     ${h.select('default_repo_create','',c.repo_create_choices,class_='form-control')}
                     <span class="help-block">${_('Enable this to allow non-admins to create repositories at the top level.')}</span>
-                    <span class="help-block">${_('Note: This will also give all users API access to create repositories everywhere. That might change in future versions.')}</span>
                 </div>
             </div>
             <div class="form-group">
--- a/kallithea/tests/api/api_base.py	Tue Dec 29 22:23:01 2020 +0100
+++ b/kallithea/tests/api/api_base.py	Thu Jan 07 03:28:37 2021 +0100
@@ -887,19 +887,11 @@
         )
         response = api_call(self, params)
 
-        # Current result when API access control is different from Web:
-        ret = {
-            'msg': 'Created new repository `%s`' % repo_name,
-            'success': True,
-        }
-        expected = ret
-        self._compare_ok(id_, expected, given=response.body)
+        # API access control match Web access control:
+        expected = 'no permission to create repo in test_repo_group/api-repo-repo'
+        self._compare_error(id_, expected, given=response.body)
+
         fixture.destroy_repo(repo_name)
-
-        # Expected and arguably more correct result:
-        #expected = 'failed to create repository `%s`' % repo_name
-        #self._compare_error(id_, expected, given=response.body)
-
         fixture.destroy_repo_group(repo_group_name)
 
     def test_api_create_repo_unknown_owner(self):
@@ -1121,7 +1113,7 @@
         finally:
             fixture.destroy_repo(repo_name)
 
-    def test_api_update_repo_regular_user_change_repo_name(self):
+    def test_api_update_repo_regular_user_change_top_level_repo_name(self):
         repo_name = 'admin_owned'
         new_repo_name = 'new_repo_name'
         fixture.create_repo(repo_name, repo_type=self.REPO_TYPE)
@@ -1135,7 +1127,7 @@
                                   repoid=repo_name, **updates)
         response = api_call(self, params)
         try:
-            expected = 'no permission to create (or move) repositories'
+            expected = 'no permission to create (or move) top level repositories'
             self._compare_error(id_, expected, given=response.body)
         finally:
             fixture.destroy_repo(repo_name)
@@ -1265,6 +1257,9 @@
         '%s/api-repo-fork' % TEST_REPO_GROUP,
     ])
     def test_api_fork_repo_non_admin(self, fork_name):
+        RepoGroupModel().grant_user_permission(TEST_REPO_GROUP,
+                                               self.TEST_USER_LOGIN,
+                                               'group.write')
         id_, params = _build_data(self.apikey_regular, 'fork_repo',
                                   repoid=self.REPO,
                                   fork_name=fork_name,
@@ -1330,7 +1325,7 @@
                                   fork_name=fork_name,
         )
         response = api_call(self, params)
-        expected = 'no permission to create repositories'
+        expected = 'no permission to create top level repo'
         self._compare_error(id_, expected, given=response.body)
         fixture.destroy_repo(fork_name)