kallithea Changeset - b2634df81a11

Changeset - b2634df81a11

Parent rev.

Child rev.

[Not reviewed]

default

0 4 0

Mads Kiilerich - 7 years ago 2018-12-29 17:48:07
mads@kiilerich.com

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.

4 files changed with 29 insertions and 19 deletions:

kallithea/lib/auth.py

kallithea/tests/api/api_base.py

kallithea/tests/functional/test_forks.py

kallithea/tests/models/test_permissions.py

0 comments (0 inline, 0 general)

kallithea/lib/auth.py

➞

Show inline comments

@@ @@ -556,13 +556,13 @@ class AuthUser(object): @@
         return ok
     @property
     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
         are granted to groups
         :param user: `AuthUser` instance

kallithea/tests/api/api_base.py

➞

Show inline comments

@@ @@ -104,12 +104,13 @@ class _BaseTestApi(object): @@
             firstname=u'first',
             lastname=u'last'
+        )
         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):
         pass
     def setup_method(self, method):
@@ @@ -703,21 +704,29 @@ class _BaseTestApi(object): @@
             self._compare_ok(id_, expected, given=response.body)
         finally:
             RepoModel().revoke_user_permission(self.REPO, self.TEST_USER_LOGIN)
     def test_api_get_repo_by_non_admin_no_permission_to_repo(self):
         RepoModel().grant_user_permission(repo=self.REPO,
                                           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)
         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',
                                   repoid='no-such-repo')
         response = api_call(self, params)
@@ @@ -1352,22 +1361,27 @@ class _BaseTestApi(object): @@
         expected = 'Only Kallithea admin can specify `owner` param'
         self._compare_error(id_, expected, given=response.body)
         fixture.destroy_repo(fork_name)
     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')
         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'),
         ('write', 'repository.write'),
         ('admin', 'repository.admin'),

kallithea/tests/functional/test_forks.py

➞

Show inline comments

@@ @@ -246,25 +246,30 @@ class _BaseTestCase(TestController): @@
         response = self.app.get(url(controller='forks', action='forks',
                                     repo_name=repo_name))
         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
             response = self.app.get(url(controller='forks', action='forks',
                                         repo_name=repo_name))
             response.mustcontain('There are no forks yet')
         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])
 class TestGIT(_BaseTestCase):
     REPO = GIT_REPO
     NEW_REPO = NEW_GIT_REPO

kallithea/tests/models/test_permissions.py

➞

Show inline comments

@@ @@ -129,35 +129,26 @@ class TestPermissions(TestController): @@
     def test_propagated_permission_from_users_group_by_explicit_perms_exist(self):
         # make group
         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
         self.ug1 = fixture.create_user_group(u'G1')
         UserGroupModel().add_user_to_group(self.ug1, self.u3)

0 comments (0 inline, 0 general)