[#1665] Fix bug where we wouldn't get the group list even though we w…

…ere authorized

The problem was that we checked if there were roles with 'manage_group'
permission and returned an empty list if there weren't, even if the user
calling was a sysadmin (that didn't need a role with 'manage_group'
permission).

ckan/logic/action/get.py

@@ -490,20 +490,21 @@ def group_list_authz(context, data_dict):
 
     _check_access('group_list_authz', context, data_dict)
 
+    user_id = new_authz.get_user_id_for_username(user, allow_none=True)
+    if not user_id:
+        return []
+
     sysadmin = new_authz.is_sysadmin(user)
     default_perms_name = 'default_group_or_org_permissions'
     default_perms = new_authz.check_config_permission(default_perms_name)
     anyone_can_manage_groups = 'manage_group' in default_perms
-    show_all_groups = (sysadmin or anyone_can_manage_groups) and not am_member
-
-    roles = ckan.new_authz.get_roles_with_permission('manage_group')
-    if not roles:
-        return []
-    user_id = new_authz.get_user_id_for_username(user, allow_none=True)
-    if not user_id:
-        return []
+    show_all_groups = not am_member and (sysadmin or anyone_can_manage_groups)
 
     if not show_all_groups:
+        roles = ckan.new_authz.get_roles_with_permission('manage_group')
+        if not roles:
+            return []
+
         q = model.Session.query(model.Member) \
             .filter(model.Member.table_name == 'user') \
             .filter(model.Member.capacity.in_(roles)) \

ckan/new_tests/logic/action/test_get.py

@@ -435,6 +435,31 @@ def test_it_returns_empty_list_if_therere_no_roles_able_to_manage_groups(self, g
 
         assert_equals(result, [])
 
+    @mock.patch('ckan.new_authz.get_roles_with_permission')
+    def test_it_should_return_every_group_if_user_is_sysadmin_even_if_therere_no_roles_able_to_manage_groups(self, get_roles_with_permission):
+        get_roles_with_permission.return_value = []
+        sysadmin = factories.Sysadmin()
+        group_id = factories.Group()['id']
+        context = {'user': sysadmin['name']}
+
+        result = helpers.call_action('group_list_authz', context=context)
+
+        assert_equals(len(result), 1)
+        assert_equals(result[0]['id'], group_id)
+
+    @mock.patch('ckan.new_authz.get_roles_with_permission')
+    @helpers.change_config('ckan.auth.default_group_or_org_permissions', 'manage_group')
+    def test_it_should_return_every_group_if_everyone_is_able_to_manage_groups_even_if_therere_no_roles_with_this_permission(self, get_roles_with_permission):
+        get_roles_with_permission.return_value = []
+        user = factories.User()
+        group_id = factories.Group()['id']
+        context = {'user': user['name']}
+
+        result = helpers.call_action('group_list_authz', context=context)
+
+        assert_equals(len(result), 1)
+        assert_equals(result[0]['id'], group_id)
+
     @helpers.change_config('ckan.auth.default_group_or_org_permissions', 'manage_group')
     def test_it_returns_all_groups_if_ckan_is_configured_to_allow_everyone_to_manage_groups(self):
         user = factories.User()