Skip to content

Commit

Permalink
HDX-8975 permission for specific users to set resource in quarantine …
Browse files Browse the repository at this point in the history
…and set datasets as qa complete
  • Loading branch information
alexandru-m-g committed Aug 24, 2023
1 parent fecbf67 commit 101835b
Show file tree
Hide file tree
Showing 6 changed files with 100 additions and 7 deletions.
13 changes: 9 additions & 4 deletions ckanext-hdx_package/ckanext/hdx_package/actions/authorize.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,15 @@ def hdx_resource_download(context, resource_dict):


def hdx_mark_qa_completed(context, data_dict=None):
'''
Only sysadmins are allowed to call this action
'''
return {'success': False, 'msg': _('Only sysadmins can change the qa_completed flag')}
username_or_id = context.get('user')
result = Permissions(username_or_id).has_permission(Permissions.PERMISSION_MANAGE_QA)
return {'success': result}


def hdx_mark_resource_in_quarantine(context, data_dict=None):
username_or_id = context.get('user')
result = Permissions(username_or_id).has_permission(Permissions.PERMISSION_MANAGE_QA)
return {'success': result}


def hdx_qa_resource_patch(context, data_dict=None):
Expand Down
19 changes: 19 additions & 0 deletions ckanext-hdx_package/ckanext/hdx_package/actions/patch.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,13 @@ def hdx_mark_broken_link_in_resource(context, data_dict):


def hdx_mark_qa_completed(context, data_dict):
'''
This action uses PERMISSIONS! Please be careful if changing the scope of its changes !
'''
_check_access('hdx_mark_qa_completed', context, data_dict)
_get_or_bust(data_dict, 'qa_completed')

context['ignore_auth'] = True
context['allow_qa_completed_field'] = True
context[BATCH_MODE] = BATCH_MODE_KEEP_OLD

Expand All @@ -152,6 +156,21 @@ def hdx_mark_qa_completed(context, data_dict):
# return _get_action('package_patch')(context, data_dict)


def hdx_mark_resource_in_quarantine(context, data_dict):
'''
This action uses PERMISSIONS! Please be careful if changing the scope of its changes !
'''
_check_access('hdx_mark_resource_in_quarantine', context, data_dict)
new_data_dict = {
'id': _get_or_bust(data_dict, 'id'),
'in_quarantine': _get_or_bust(data_dict, 'in_quarantine')

}
context['ignore_auth'] = True
result = _get_action('hdx_qa_resource_patch')(context, new_data_dict)
return result


def hdx_qa_resource_patch(context, data_dict):
_check_access('hdx_qa_resource_patch', context, data_dict)

Expand Down
2 changes: 2 additions & 0 deletions ckanext-hdx_package/ckanext/hdx_package/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,7 @@ def get_actions(self):
'hdx_get_s3_link_for_resource': hdx_get.hdx_get_s3_link_for_resource,
'hdx_mark_broken_link_in_resource': hdx_patch.hdx_mark_broken_link_in_resource,
'hdx_mark_qa_completed': hdx_patch.hdx_mark_qa_completed,
'hdx_mark_resource_in_quarantine': hdx_patch.hdx_mark_resource_in_quarantine,
'hdx_qa_resource_patch': hdx_patch.hdx_qa_resource_patch,
'hdx_fs_check_resource_revise': hdx_patch.hdx_fs_check_resource_revise,
'hdx_fs_check_resource_reset': hdx_patch.hdx_fs_check_resource_reset,
Expand Down Expand Up @@ -565,6 +566,7 @@ def get_auth_functions(self):
# 'hdx_create_screenshot_for_cod': authorize.hdx_create_screenshot_for_cod,
'hdx_resource_download': authorize.hdx_resource_download,
'hdx_mark_qa_completed': authorize.hdx_mark_qa_completed,
'hdx_mark_resource_in_quarantine': authorize.hdx_mark_resource_in_quarantine,
'hdx_package_qa_checklist_update': authorize.package_qa_checklist_update,
'hdx_qa_resource_patch': authorize.hdx_qa_resource_patch,
'hdx_fs_check_resource_revise': authorize.hdx_fs_check_resource_revise,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

from ckanext.hdx_package.actions.patch import _send_analytics_for_pii_if_needed, _send_analytics_for_sdc_if_needed
from ckanext.hdx_org_group.helpers.static_lists import ORGANIZATION_TYPE_LIST
from ckanext.hdx_users.helpers.permissions import Permissions

config = tk.config
NotAuthorized = tk.NotAuthorized
Expand Down Expand Up @@ -84,7 +85,8 @@ def setup_class(cls):
def test_normal_user_cannot_modify_quarantine(self, tag_s3_mock):
package_dict = self._change_resource_field_via_resource_patch(self.RESOURCE_ID, 'in_quarantine', True,
self.SYSADMIN_USER)
assert 'in_quarantine' not in package_dict['resources'][0]
assert ('in_quarantine' not in package_dict['resources'][0] or
package_dict['resources'][0]['in_quarantine'] is False)

package_dict = self._hdx_qa_resource_patch(self.RESOURCE_ID, 'in_quarantine', True,
self.SYSADMIN_USER)
Expand All @@ -106,6 +108,26 @@ def test_normal_user_cannot_modify_quarantine(self, tag_s3_mock):
self.SYSADMIN_USER)
assert package_dict['resources'][0]['in_quarantine'] is False

@mock.patch('ckanext.hdx_package.actions.patch.tag_s3_version_by_resource_id')
def test_permissions_user_can_modify_quarantine(self, tag_s3_mock):
PERMISSIONS_USER = 'permissions_qa_completed_user'
factories.User(name=PERMISSIONS_USER, email=PERMISSIONS_USER + '@hdx.hdxtest.org')

package_dict = self._hdx_mark_resource_in_quarantine(self.RESOURCE_ID, True, PERMISSIONS_USER)
assert ('in_quarantine' not in package_dict['resources'][0] or
package_dict['resources'][0]['in_quarantine'] is False)

Permissions(PERMISSIONS_USER).set_permissions(
{'model': model, 'session': model.Session, 'user': self.SYSADMIN_USER},
[Permissions.PERMISSION_MANAGE_QA]
)

package_dict = self._hdx_mark_resource_in_quarantine(self.RESOURCE_ID, True, PERMISSIONS_USER)
assert package_dict['resources'][0]['in_quarantine'] is True
package_dict = self._hdx_mark_resource_in_quarantine(self.RESOURCE_ID, False, PERMISSIONS_USER)
assert package_dict['resources'][0]['in_quarantine'] is False


def test_multiple_pii_fields_can_be_set(self):
context = {'model': model, 'session': model.Session, 'user': self.SYSADMIN_USER}
resource2 = {
Expand Down Expand Up @@ -254,3 +276,16 @@ def _hdx_qa_resource_patch(self, resource_id, key, new_value, username):
pass
return self._get_action('package_show')({}, {'id': self.PACKAGE_ID})

def _hdx_mark_resource_in_quarantine(self, resource_id, new_value, username):
try:
self._get_action('hdx_mark_resource_in_quarantine')(
{
'model': model, 'session': model.Session,
'user': username,
},
{'id': resource_id, 'in_quarantine': new_value}
)
except NotAuthorized as e:
pass
return self._get_action('package_show')({}, {'id': self.PACKAGE_ID})

Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import ckanext.hdx_theme.tests.hdx_test_base as hdx_test_base

from ckanext.hdx_org_group.helpers.static_lists import ORGANIZATION_TYPE_LIST
from ckanext.hdx_users.helpers.permissions import Permissions

config = tk.config
NotAuthorized = tk.NotAuthorized
Expand All @@ -30,7 +31,7 @@ def _get_action(cls, action_name):
@classmethod
def setup_class(cls):
super(TestQACompleted, cls).setup_class()
factories.User(name=cls.NORMAL_USER, email='qa_completed_user@hdx.hdxtest.org')
factories.User(name=cls.NORMAL_USER, email=cls.NORMAL_USER + '@hdx.hdxtest.org')
factories.Organization(
name='org_name_4_qa_completed',
title='ORG NAME FOR QA COMPLETED',
Expand Down Expand Up @@ -147,7 +148,34 @@ def test_qa_completed_reset_via_hdx_mark_qa_completed(self, analytics_g):
assert "qa_completed" in package_dict.get('package') and package_dict.get('package').get("qa_completed") is True

package_dict = self._hdx_mark_qa_completed_flag(package_dict_4.get('id'), False, self.SYSADMIN_USER)
assert "qa_completed" in package_dict.get('package') and package_dict.get('package').get("qa_completed") is False
assert "qa_completed" in package_dict.get('package') and package_dict.get('package').get(
"qa_completed") is False

@mock.patch('ckanext.hdx_package.helpers.analytics.g')
def test_qa_completed_with_permission_via_hdx_mark_qa_completed(self, analytics_g):
PERMISSIONS_USER = 'permissions_qa_completed_user'
factories.User(name=PERMISSIONS_USER, email=PERMISSIONS_USER + '@hdx.hdxtest.org')
package_dict = self._get_action('package_show')({}, {'id': self.PACKAGE_ID})
package_id = package_dict['id']

try:
# This user doesn't yet have access to change the qa_completed flag
self._hdx_mark_qa_completed_flag(package_id, True, PERMISSIONS_USER)
except NotAuthorized as e:
assert True

Permissions(PERMISSIONS_USER).set_permissions(
{'model': model, 'session': model.Session, 'user': self.SYSADMIN_USER},
[Permissions.PERMISSION_MANAGE_QA]
)

# This user now has permission and should be allowed to change the flag
package_dict = self._hdx_mark_qa_completed_flag(package_id, True, PERMISSIONS_USER)
assert "qa_completed" in package_dict.get('package') and package_dict.get('package').get("qa_completed") is True

package_dict = self._hdx_mark_qa_completed_flag(package_id, False, PERMISSIONS_USER)
assert "qa_completed" in package_dict.get('package') and package_dict.get('package').get(
"qa_completed") is False

def _package_patch_qa_completed_flag(self, package_id, qa_completed, user):
context = {
Expand Down
4 changes: 4 additions & 0 deletions ckanext-hdx_users/ckanext/hdx_users/helpers/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ class Permissions(object):
LABEL_PERMISSION_VIEW_REQUEST_DATA = 'View Request Data Dashboard'
PERMISSION_MANAGE_QUICK_LINKS = 'permission_manage_quick_links'
LABEL_PERMISSION_MANAGE_QUICK_LINKS = 'Manage Quick Links'
PERMISSION_MANAGE_QA = 'permission_manage_qa' # QA Complete and quarantine
LABEL_PERMISSION_MANAGE_QA = 'Manage QA'

# These are tasks that a bot needs to trigger: HDX daily stats, api token expiry emails.
# Note that this permission shouldn't allow for any change to be done to HDX
Expand All @@ -37,6 +39,7 @@ class Permissions(object):
PERMISSION_MANAGE_COD,
PERMISSION_MANAGE_DATASERIES,
PERMISSION_MANAGE_P_CODES,
PERMISSION_MANAGE_QA,
PERMISSION_MANAGE_CRISIS,
PERMISSION_VIEW_REQUEST_DATA,
PERMISSION_MANAGE_QUICK_LINKS,
Expand All @@ -52,6 +55,7 @@ class Permissions(object):
PERMISSION_VIEW_REQUEST_DATA: LABEL_PERMISSION_VIEW_REQUEST_DATA,
PERMISSION_MANAGE_QUICK_LINKS: LABEL_PERMISSION_MANAGE_QUICK_LINKS,
PERMISSION_MANAGE_BASIC_SCHEDULED_TASKS: LABEL_PERMISSION_MANAGE_BASIC_SCHEDULED_TASKS,
PERMISSION_MANAGE_QA: LABEL_PERMISSION_MANAGE_QA,
}

USER_EXTRA_FIELD = 'hdx_permissions'
Expand Down

0 comments on commit 101835b

Please sign in to comment.