Mercurial > kallithea

changeset 7687:b2634df81a11

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
auth: explicit user permission should not blindly overrule permissions through user groups Before, explicit permissions of a user could shadow higher permissions that would otherwise be obtained through a group the user is member of. That was confusing and fragile: *removing* a permission could then suddenly give a user *more* permissions. Instead, change the flag for controlling internal permission computation to *not* use "explicit". Permissions will then add up, no matter if they are explicit or through groups. The change in auth.py is small, but read the body of __get_perms to see the actual impact ... and also the clean-up changeset that will come next. This might in some cases be a behaviour change and give users more access ... but it will probably only give the user that was intended. This change can thus be seen as a bugfix. Some tests assumed the old behaviour. Not for good reasons, but just because that is how they were written. These tests are updated to expect the new behaviour, and it has been reviewed that it makes sense. Note that this 'explicit' flag mostly is for repo permissions and independent of the 'user_inherit_default_permissions' that just was removed and is about global permissions.
author Mads Kiilerich <mads@kiilerich.com>
date Sat, 29 Dec 2018 17:48:07 +0100
parents 93834966ae01
children 1ab83bed8115
files kallithea/lib/auth.py kallithea/tests/api/api_base.py kallithea/tests/functional/test_forks.py kallithea/tests/models/test_permissions.py
diffstat 4 files changed, 44 insertions(+), 34 deletions(-) [+]
line wrap: on
line diff
--- a/kallithea/lib/auth.py	Mon Dec 31 02:25:11 2018 +0100
+++ b/kallithea/lib/auth.py	Sat Dec 29 17:48:07 2018 +0100
@@ -559,7 +559,7 @@
     def api_keys(self):
         return self._get_api_keys()
 
-    def __get_perms(self, user, explicit=True, cache=False):
+    def __get_perms(self, user, explicit=False, cache=False):
         """
         Fills user permission attribute with permissions taken from database
         works for permissions given for repositories, and for permissions that
--- a/kallithea/tests/api/api_base.py	Mon Dec 31 02:25:11 2018 +0100
+++ b/kallithea/tests/api/api_base.py	Sat Dec 29 17:48:07 2018 +0100
@@ -107,6 +107,7 @@
         Session().commit()
         cls.TEST_USER_LOGIN = cls.test_user.username
         cls.apikey_regular = cls.test_user.api_key
+        cls.default_user_username = User.get_default_user().username
 
     @classmethod
     def teardown_class(cls):
@@ -706,15 +707,23 @@
 
     def test_api_get_repo_by_non_admin_no_permission_to_repo(self):
         RepoModel().grant_user_permission(repo=self.REPO,
-                                          user=self.TEST_USER_LOGIN,
+                                          user=self.default_user_username,
                                           perm='repository.none')
+        try:
+            RepoModel().grant_user_permission(repo=self.REPO,
+                                              user=self.TEST_USER_LOGIN,
+                                              perm='repository.none')
 
-        id_, params = _build_data(self.apikey_regular, 'get_repo',
-                                  repoid=self.REPO)
-        response = api_call(self, params)
+            id_, params = _build_data(self.apikey_regular, 'get_repo',
+                                      repoid=self.REPO)
+            response = api_call(self, params)
 
-        expected = 'repository `%s` does not exist' % (self.REPO)
-        self._compare_error(id_, expected, given=response.body)
+            expected = 'repository `%s` does not exist' % (self.REPO)
+            self._compare_error(id_, expected, given=response.body)
+        finally:
+            RepoModel().grant_user_permission(repo=self.REPO,
+                                              user=self.default_user_username,
+                                              perm='repository.read')
 
     def test_api_get_repo_that_doesn_not_exist(self):
         id_, params = _build_data(self.apikey, 'get_repo',
@@ -1355,17 +1364,22 @@
 
     def test_api_fork_repo_non_admin_no_permission_to_fork(self):
         RepoModel().grant_user_permission(repo=self.REPO,
-                                          user=self.TEST_USER_LOGIN,
+                                          user=self.default_user_username,
                                           perm='repository.none')
-        fork_name = u'api-repo-fork'
-        id_, params = _build_data(self.apikey_regular, 'fork_repo',
-                                  repoid=self.REPO,
-                                  fork_name=fork_name,
-        )
-        response = api_call(self, params)
-        expected = 'repository `%s` does not exist' % (self.REPO)
-        self._compare_error(id_, expected, given=response.body)
-        fixture.destroy_repo(fork_name)
+        try:
+            fork_name = u'api-repo-fork'
+            id_, params = _build_data(self.apikey_regular, 'fork_repo',
+                                      repoid=self.REPO,
+                                      fork_name=fork_name,
+            )
+            response = api_call(self, params)
+            expected = 'repository `%s` does not exist' % (self.REPO)
+            self._compare_error(id_, expected, given=response.body)
+        finally:
+            RepoModel().grant_user_permission(repo=self.REPO,
+                                              user=self.default_user_username,
+                                              perm='repository.read')
+            fixture.destroy_repo(fork_name)
 
     @parametrize('name,perm', [
         ('read', 'repository.read'),
--- a/kallithea/tests/functional/test_forks.py	Mon Dec 31 02:25:11 2018 +0100
+++ b/kallithea/tests/functional/test_forks.py	Sat Dec 29 17:48:07 2018 +0100
@@ -249,9 +249,12 @@
         response.mustcontain('<div>fork of vcs test</div>')
 
         # remove permissions
+        default_user = User.get_default_user()
         try:
             RepoModel().grant_user_permission(repo=forks[0],
                                               user=usr, perm='repository.none')
+            RepoModel().grant_user_permission(repo=forks[0],
+                                              user=default_user, perm='repository.none')
             Session().commit()
 
             # fork shouldn't be visible
@@ -262,6 +265,8 @@
         finally:
             RepoModel().grant_user_permission(repo=forks[0],
                                               user=usr, perm='repository.read')
+            RepoModel().grant_user_permission(repo=forks[0],
+                                              user=default_user, perm='repository.read')
             RepoModel().delete(repo=forks[0])
 
 
--- a/kallithea/tests/models/test_permissions.py	Mon Dec 31 02:25:11 2018 +0100
+++ b/kallithea/tests/models/test_permissions.py	Sat Dec 29 17:48:07 2018 +0100
@@ -132,29 +132,20 @@
         self.ug1 = fixture.create_user_group(u'G1')
         UserGroupModel().add_user_to_group(self.ug1, self.u1)
 
-        # set permission to lower
-        new_perm = 'repository.none'
-        RepoModel().grant_user_permission(repo=HG_REPO, user=self.u1, perm=new_perm)
+        # set user permission none
+        RepoModel().grant_user_permission(repo=HG_REPO, user=self.u1, perm='repository.none')
         Session().commit()
         u1_auth = AuthUser(user_id=self.u1.user_id)
-        assert u1_auth.permissions['repositories'][HG_REPO] == new_perm
+        assert u1_auth.permissions['repositories'][HG_REPO] == 'repository.read' # inherit from default user
 
-        # grant perm for group this should not override permission from user
-        # since it has explicitly set
-        new_perm_gr = 'repository.write'
+        # grant perm for group this should override permission from user
         RepoModel().grant_user_group_permission(repo=HG_REPO,
                                                  group_name=self.ug1,
-                                                 perm=new_perm_gr)
-        # check perms
+                                                 perm='repository.write')
+
+        # verify that user group permissions win
         u1_auth = AuthUser(user_id=self.u1.user_id)
-        perms = {
-            'repositories_groups': {},
-            'global': set(['hg.create.repository', 'repository.read',
-                           'hg.register.manual_activate']),
-            'repositories': {HG_REPO: 'repository.read'}
-        }
-        assert u1_auth.permissions['repositories'][HG_REPO] == new_perm
-        assert u1_auth.permissions['repositories_groups'] == perms['repositories_groups']
+        assert u1_auth.permissions['repositories'][HG_REPO] == 'repository.write'
 
     def test_propagated_permission_from_users_group(self):
         # make group