From e69dc0f36eb2f87da911ee7c140b3ae841a9df99 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Tue, 10 Jan 2017 18:17:12 -0500 Subject: [PATCH 1/3] Clarification of copy RBAC messages --- awx/api/views.py | 2 ++ awx/main/access.py | 8 ++++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/awx/api/views.py b/awx/api/views.py index 7d0586b296..b3d877b67a 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -2910,6 +2910,8 @@ class WorkflowJobTemplateCopy(WorkflowsEnforcementMixin, GenericAPIView): copy_TF, messages = request.user.can_access_with_errors(self.model, 'copy', obj) data['can_copy'] = copy_TF data['warnings'] = messages + if not copy_TF: + data['warnings'] = _('You do not have permission to make a copy.') return Response(data) def post(self, request, *args, **kwargs): diff --git a/awx/main/access.py b/awx/main/access.py index f2f1f72367..0696d10970 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -1043,7 +1043,7 @@ class JobTemplateAccess(BaseAccess): Project.accessible_objects(self.user, 'use_role').exists() or Inventory.accessible_objects(self.user, 'use_role').exists()) - # if reference_obj is provided, determine if it can be coppied + # if reference_obj is provided, determine if it can be copied reference_obj = data.get('reference_obj', None) if 'job_type' in data and data['job_type'] == PERM_INVENTORY_SCAN: @@ -1543,13 +1543,13 @@ class WorkflowJobTemplateAccess(BaseAccess): for node in qs.all(): node_errors = {} if node.inventory and self.user not in node.inventory.use_role: - node_errors['inventory'] = 'Prompted inventory %s can not be coppied.' % node.inventory.name + node_errors['inventory'] = 'Prompted inventory %s can not be copied.' % node.inventory.name if node.credential and self.user not in node.credential.use_role: - node_errors['credential'] = 'Prompted credential %s can not be coppied.' % node.credential.name + node_errors['credential'] = 'Prompted credential %s can not be copied.' % node.credential.name ujt = node.unified_job_template if ujt and not self.user.can_access(UnifiedJobTemplate, 'start', ujt, validate_license=False): node_errors['unified_job_template'] = ( - 'Prompted %s %s can not be coppied.' % (ujt._meta.verbose_name_raw, ujt.name)) + 'Prompted %s %s can not be copied.' % (ujt._meta.verbose_name_raw, ujt.name)) if node_errors: wfjt_errors[node.id] = node_errors self.messages.update(wfjt_errors) From 2c97425291f32c355f48339dbb053c1b3ef3ef94 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Wed, 11 Jan 2017 12:23:57 -0500 Subject: [PATCH 2/3] prototype for new copy GET details --- awx/api/views.py | 16 ++++++++++------ awx/main/access.py | 18 ++++++++++++------ .../list/templates-list.controller.js | 14 +++++++------- 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/awx/api/views.py b/awx/api/views.py index b3d877b67a..7e8ea65540 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -2906,12 +2906,16 @@ class WorkflowJobTemplateCopy(WorkflowsEnforcementMixin, GenericAPIView): def get(self, request, *args, **kwargs): obj = self.get_object() - data = {} - copy_TF, messages = request.user.can_access_with_errors(self.model, 'copy', obj) - data['can_copy'] = copy_TF - data['warnings'] = messages - if not copy_TF: - data['warnings'] = _('You do not have permission to make a copy.') + can_copy, messages = request.user.can_access_with_errors(self.model, 'copy', obj) + data = { + 'can_copy': can_copy, 'can_copy_without_user_input': can_copy, + 'templates_unable_to_copy': [] if can_copy else ['all'], + 'credentials_unable_to_copy': [] if can_copy else ['all'], + 'inventories_unable_to_copy': [] if can_copy else ['all'] + } + if messages and can_copy: + data['can_copy_without_user_input'] = False + data.update(messages) return Response(data) def post(self, request, *args, **kwargs): diff --git a/awx/main/access.py b/awx/main/access.py index 0696d10970..75b53d2527 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -1537,22 +1537,28 @@ class WorkflowJobTemplateAccess(BaseAccess): def can_copy(self, obj): if self.save_messages: - wfjt_errors = {} + missing_ujt = [] + missing_credentials = [] + missing_inventories = [] qs = obj.workflow_job_template_nodes qs.select_related('unified_job_template', 'inventory', 'credential') for node in qs.all(): node_errors = {} if node.inventory and self.user not in node.inventory.use_role: - node_errors['inventory'] = 'Prompted inventory %s can not be copied.' % node.inventory.name + missing_inventories.append(node.inventory.name) if node.credential and self.user not in node.credential.use_role: - node_errors['credential'] = 'Prompted credential %s can not be copied.' % node.credential.name + missing_credentials.append(node.credential.name) ujt = node.unified_job_template if ujt and not self.user.can_access(UnifiedJobTemplate, 'start', ujt, validate_license=False): - node_errors['unified_job_template'] = ( - 'Prompted %s %s can not be copied.' % (ujt._meta.verbose_name_raw, ujt.name)) + missing_ujt.append(ujt.name) if node_errors: wfjt_errors[node.id] = node_errors - self.messages.update(wfjt_errors) + if missing_ujt: + self.messages['templates_unable_to_copy'] = missing_ujt + if missing_credentials: + self.messages['credentials_unable_to_copy'] = missing_credentials + if missing_inventories: + self.messages['inventories_unable_to_copy'] = missing_inventories return self.check_related('organization', Organization, {'reference_obj': obj}, mandatory=True) diff --git a/awx/ui/client/src/templates/list/templates-list.controller.js b/awx/ui/client/src/templates/list/templates-list.controller.js index 820f843852..a71f9573c3 100644 --- a/awx/ui/client/src/templates/list/templates-list.controller.js +++ b/awx/ui/client/src/templates/list/templates-list.controller.js @@ -220,7 +220,7 @@ export default ['$scope', '$rootScope', '$location', '$stateParams', 'Rest', .then(function(result) { if(result.data.can_copy) { - if(!result.data.warnings || _.isEmpty(result.data.warnings)) { + if(result.data.can_copy_without_user_input) { // Go ahead and copy the workflow - the user has full priveleges on all the resources TemplateCopyService.copyWorkflow(template.id) .then(function(result) { @@ -235,16 +235,16 @@ export default ['$scope', '$rootScope', '$location', '$stateParams', 'Rest', let bodyHtml = `
- You may not have access to all resources used by this workflow. Resources that you don\'t have access to will not be copied and may result in an incomplete workflow. + You do not have access to all resources used by this workflow. Resources that you don\'t have access to will not be copied and will result in an incomplete workflow.
`; // Go and grab all of the warning strings - _.forOwn(result.data.warnings, function(warning) { - if(warning) { - _.forOwn(warning, function(warningString) { - bodyHtml += '
' + warningString + '
'; - }); + _.forOwn(result.data.templates_unable_to_copy, function(ujt) { + if(ujt) { + // _.forOwn(ujts, function(warningString) { + bodyHtml += '
' + ujt + '
'; + // }); } } ); From a693aad95da401712efc519fd3b92979aaa46a21 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Wed, 11 Jan 2017 14:22:28 -0500 Subject: [PATCH 3/3] a workable version of the new copy GET schema implemented --- awx/api/views.py | 12 +++--- .../tests/functional/test_rbac_workflow.py | 6 +-- .../list/templates-list.controller.js | 38 +++++++++++++++---- 3 files changed, 38 insertions(+), 18 deletions(-) diff --git a/awx/api/views.py b/awx/api/views.py index 7e8ea65540..7c22bdf57f 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -2907,12 +2907,12 @@ class WorkflowJobTemplateCopy(WorkflowsEnforcementMixin, GenericAPIView): def get(self, request, *args, **kwargs): obj = self.get_object() can_copy, messages = request.user.can_access_with_errors(self.model, 'copy', obj) - data = { - 'can_copy': can_copy, 'can_copy_without_user_input': can_copy, - 'templates_unable_to_copy': [] if can_copy else ['all'], - 'credentials_unable_to_copy': [] if can_copy else ['all'], - 'inventories_unable_to_copy': [] if can_copy else ['all'] - } + data = OrderedDict([ + ('can_copy', can_copy), ('can_copy_without_user_input', can_copy), + ('templates_unable_to_copy', [] if can_copy else ['all']), + ('credentials_unable_to_copy', [] if can_copy else ['all']), + ('inventories_unable_to_copy', [] if can_copy else ['all']) + ]) if messages and can_copy: data['can_copy_without_user_input'] = False data.update(messages) diff --git a/awx/main/tests/functional/test_rbac_workflow.py b/awx/main/tests/functional/test_rbac_workflow.py index a0f0348e38..f2ce04404f 100644 --- a/awx/main/tests/functional/test_rbac_workflow.py +++ b/awx/main/tests/functional/test_rbac_workflow.py @@ -120,13 +120,11 @@ class TestWorkflowJobAccess: access = WorkflowJobTemplateAccess(rando, save_messages=True) assert not access.can_copy(wfjt) warnings = access.messages - assert 1 in warnings - assert 'inventory' in warnings[1] + assert 'inventories_unable_to_copy' in warnings def test_workflow_copy_warnings_jt(self, wfjt, rando, job_template): wfjt.workflow_job_template_nodes.create(unified_job_template=job_template) access = WorkflowJobTemplateAccess(rando, save_messages=True) assert not access.can_copy(wfjt) warnings = access.messages - assert 1 in warnings - assert 'unified_job_template' in warnings[1] + assert 'templates_unable_to_copy' in warnings diff --git a/awx/ui/client/src/templates/list/templates-list.controller.js b/awx/ui/client/src/templates/list/templates-list.controller.js index a71f9573c3..d0b910e6b9 100644 --- a/awx/ui/client/src/templates/list/templates-list.controller.js +++ b/awx/ui/client/src/templates/list/templates-list.controller.js @@ -239,14 +239,36 @@ export default ['$scope', '$rootScope', '$location', '$stateParams', 'Rest',
`; - // Go and grab all of the warning strings - _.forOwn(result.data.templates_unable_to_copy, function(ujt) { - if(ujt) { - // _.forOwn(ujts, function(warningString) { - bodyHtml += '
' + ujt + '
'; - // }); - } - } ); + // List the unified job templates user can not access + if (result.data.templates_unable_to_copy.length > 0) { + bodyHtml += '
Unified Job Templates that can not be copied
    '; + _.forOwn(result.data.templates_unable_to_copy, function(ujt) { + if(ujt) { + bodyHtml += '
  • ' + ujt + '
    • '; + } + }); + bodyHtml += '
'; + } + // List the prompted inventories user can not access + if (result.data.inventories_unable_to_copy.length > 0) { + bodyHtml += '
Node prompted inventories that can not be copied
    '; + _.forOwn(result.data.inventories_unable_to_copy, function(inv) { + if(inv) { + bodyHtml += '
  • ' + inv + '
    • '; + } + }); + bodyHtml += '
'; + } + // List the prompted credentials user can not access + if (result.data.credentials_unable_to_copy.length > 0) { + bodyHtml += '
Node prompted credentials that can not be copied
    '; + _.forOwn(result.data.credentials_unable_to_copy, function(cred) { + if(cred) { + bodyHtml += '
  • ' + cred + '
    • '; + } + }); + bodyHtml += '
'; + } bodyHtml += '
';