refactor is_repo_admin func (#3003)

seahub/api2/endpoints/group_libraries.py

@@ -235,9 +235,9 @@ class GroupLibrary(APIView):
         """ Delete a group library.
 
         Permission checking:
-        1. is group admin;
         1. is repo owner;
-        1. repo is shared to group with `admin` permission;
+        2. is repo admin;
+        3. is group admin;
         """
 
         group_id = int(group_id)

seahub/share/utils.py

@@ -1,4 +1,6 @@
-from seahub.group.utils import is_group_member
+import logging
+
+from seahub.group.utils import is_group_admin
 from seahub.constants import PERMISSION_ADMIN, PERMISSION_READ_WRITE
 from seahub.share.models import ExtraSharePermission, ExtraGroupsSharePermission
 from seahub.utils import is_valid_org_id
@@ -6,17 +8,33 @@ from seahub.utils import is_valid_org_id
 import seaserv
 from seaserv import seafile_api
 
+logger = logging.getLogger(__name__)
+
 def is_repo_admin(username, repo_id):
-    is_administrator = ExtraSharePermission.objects.\
+
-            get_user_permission(repo_id, username) == PERMISSION_ADMIN
+    repo_owner = seafile_api.get_repo_owner(repo_id)
-    belong_to_admin_group = False
+
+    try:
+        if '@seafile_group' in repo_owner:
+            # is group owned repo
+            group_id = int(repo_owner.split('@')[0])
+            if is_group_admin(group_id, username):
+                return True
+        else:
+            user_share_permission = ExtraSharePermission.objects.\
+                    get_user_permission(repo_id, username)
+            if user_share_permission == PERMISSION_ADMIN:
+                return True
+
+            # get all groups that repo is shared to with admin permission
             group_ids = ExtraGroupsSharePermission.objects.get_admin_groups_by_repo(repo_id)
             for group_id in group_ids:
-        if is_group_member(group_id, username):
+                if is_group_admin(group_id, username):
-            belong_to_admin_group = True
+                    return True
-            break
+        return False
-
+    except Exception as e:
-    return is_administrator or belong_to_admin_group
+        logger.error(e)
+        return False
 
 def share_dir_to_user(repo, path, owner, share_from, share_to, permission, org_id=None):
     # Share  repo or subdir to user with permission(r, rw, admin).

tests/api/endpoints/test_group_libraries.py

@@ -228,7 +228,7 @@ class GroupLibraryTest(BaseTestCase):
 
         self.login_as(self.admin)
 
-        # admin user can not delete
+        # commont user can not delete
         resp = self.client.delete(self.group_library_url)
         self.assertEqual(403, resp.status_code)
 
@@ -240,7 +240,12 @@ class GroupLibraryTest(BaseTestCase):
         ExtraGroupsSharePermission.objects.create_share_permission(
                 self.repo_id, self.group_id, PERMISSION_ADMIN)
 
-        # admin user can delete
+        # repo admin user(not group admin) can not delete
+        resp = self.client.delete(self.group_library_url)
+        self.assertEqual(403, resp.status_code)
+
+        # repo admin user(also is group admin) can delete
+        ccnet_api.group_set_admin(self.group_id, self.admin_name)
         resp = self.client.delete(self.group_library_url)
         self.assertEqual(200, resp.status_code)