From 0522d45ab0635f7d335ce9a432023a649a8c067f Mon Sep 17 00:00:00 2001 From: Ryan Petrello Date: Tue, 6 Aug 2019 11:26:14 -0400 Subject: [PATCH] fixed a few issues related to approval role RBAC for normal users --- awx/api/permissions.py | 13 ++++++++++++- awx/api/serializers.py | 4 +++- awx/api/views/__init__.py | 5 ++++- awx/main/access.py | 4 +++- awx/main/migrations/0083_v360_workflow_approval.py | 10 ++++++++++ awx/main/models/organization.py | 2 +- awx/main/models/workflow.py | 3 ++- awx/main/scheduler/task_manager.py | 1 + 8 files changed, 36 insertions(+), 6 deletions(-) diff --git a/awx/api/permissions.py b/awx/api/permissions.py index 3c4de0ad27..c344778bea 100644 --- a/awx/api/permissions.py +++ b/awx/api/permissions.py @@ -17,7 +17,7 @@ logger = logging.getLogger('awx.api.permissions') __all__ = ['ModelAccessPermission', 'JobTemplateCallbackPermission', 'VariableDataPermission', 'TaskPermission', 'ProjectUpdatePermission', 'InventoryInventorySourcesUpdatePermission', - 'UserPermission', 'IsSuperUser', 'InstanceGroupTowerPermission',] + 'UserPermission', 'IsSuperUser', 'InstanceGroupTowerPermission', 'WorkflowApprovalPermission'] class ModelAccessPermission(permissions.BasePermission): @@ -196,6 +196,17 @@ class TaskPermission(ModelAccessPermission): return False +class WorkflowApprovalPermission(ModelAccessPermission): + ''' + Permission check used by workflow approval and deny views + to determine who can has access to approve and deny paused workflow nodes + ''' + + def check_post_permissions(self, request, view, obj=None): + approval = get_object_or_400(view.model, pk=view.kwargs['pk']) + return check_user_access(request.user, view.model, 'approve_or_deny', approval) + + class ProjectUpdatePermission(ModelAccessPermission): ''' Permission check used by ProjectUpdateView to determine who can update projects diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 16f2e70505..2e6314f50a 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -3527,7 +3527,9 @@ class LaunchConfigurationBaseSerializer(BaseSerializer): elif self.instance: ujt = self.instance.unified_job_template if ujt is None: - return {'workflow_job_template': attrs['workflow_job_template']} + if 'workflow_job_template' in attrs: + return {'workflow_job_template': attrs['workflow_job_template']} + return {} # build additional field survey_passwords to track redacted variables password_dict = {} diff --git a/awx/api/views/__init__.py b/awx/api/views/__init__.py index 6d3aa52af7..7e43a4a707 100644 --- a/awx/api/views/__init__.py +++ b/awx/api/views/__init__.py @@ -91,7 +91,8 @@ from awx.main.redact import UriCleaner from awx.api.permissions import ( JobTemplateCallbackPermission, TaskPermission, ProjectUpdatePermission, InventoryInventorySourcesUpdatePermission, UserPermission, - InstanceGroupTowerPermission, VariableDataPermission + InstanceGroupTowerPermission, VariableDataPermission, + WorkflowApprovalPermission ) from awx.api import renderers from awx.api import serializers @@ -4452,6 +4453,7 @@ class WorkflowApprovalDetail(UnifiedJobDeletionMixin, RetrieveDestroyAPIView): class WorkflowApprovalApprove(RetrieveAPIView): model = models.WorkflowApproval serializer_class = serializers.WorkflowApprovalViewSerializer + permission_classes = (WorkflowApprovalPermission,) def post(self, request, *args, **kwargs): obj = self.get_object() @@ -4465,6 +4467,7 @@ class WorkflowApprovalApprove(RetrieveAPIView): class WorkflowApprovalDeny(RetrieveAPIView): model = models.WorkflowApproval serializer_class = serializers.WorkflowApprovalViewSerializer + permission_classes = (WorkflowApprovalPermission,) def post(self, request, *args, **kwargs): obj = self.get_object() diff --git a/awx/main/access.py b/awx/main/access.py index ca0ccebf93..ccebf57c75 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -2799,9 +2799,11 @@ class WorkflowApprovalAccess(BaseAccess): def get_queryset(self): return super(WorkflowApprovalAccess, self).get_queryset().exclude( - workflow_approval_template__isnull=False) + workflow_approval_template__isnull=True) def can_approve_or_deny(self, obj): + if obj.status != 'pending': + return False wfjt = obj.unified_job_node.workflow_job.unified_job_template if self.user in wfjt.approval_role or self.user.is_superuser: return True diff --git a/awx/main/migrations/0083_v360_workflow_approval.py b/awx/main/migrations/0083_v360_workflow_approval.py index 03d8075a9e..9f2b079d97 100644 --- a/awx/main/migrations/0083_v360_workflow_approval.py +++ b/awx/main/migrations/0083_v360_workflow_approval.py @@ -60,4 +60,14 @@ class Migration(migrations.Migration): name='workflow_approval_template', field=models.ManyToManyField(blank=True, to='main.WorkflowApprovalTemplate'), ), + migrations.AlterField( + model_name='organization', + name='read_role', + field=awx.main.fields.ImplicitRoleField(editable=False, null='True', on_delete=django.db.models.deletion.CASCADE, parent_role=['member_role', 'auditor_role', 'execute_role', 'project_admin_role', 'inventory_admin_role', 'workflow_admin_role', 'notification_admin_role', 'credential_admin_role', 'job_template_admin_role', 'approval_role'], related_name='+', to='main.Role'), + ), + migrations.AlterField( + model_name='workflowjobtemplate', + name='read_role', + field=awx.main.fields.ImplicitRoleField(editable=False, null='True', on_delete=django.db.models.deletion.CASCADE, parent_role=['singleton:system_auditor', 'organization.auditor_role', 'execute_role', 'admin_role', 'approval_role'], related_name='+', to='main.Role'), + ), ] diff --git a/awx/main/models/organization.py b/awx/main/models/organization.py index 96b88a6b64..60505b6e0a 100644 --- a/awx/main/models/organization.py +++ b/awx/main/models/organization.py @@ -87,7 +87,7 @@ class Organization(CommonModel, NotificationFieldsModel, ResourceMixin, CustomVi 'execute_role', 'project_admin_role', 'inventory_admin_role', 'workflow_admin_role', 'notification_admin_role', 'credential_admin_role', - 'job_template_admin_role',], + 'job_template_admin_role', 'approval_role',], ) approval_role = ImplicitRoleField( parent_role='admin_role', diff --git a/awx/main/models/workflow.py b/awx/main/models/workflow.py index 24f1e88a70..4ac20e7872 100644 --- a/awx/main/models/workflow.py +++ b/awx/main/models/workflow.py @@ -393,7 +393,8 @@ class WorkflowJobTemplate(UnifiedJobTemplate, WorkflowJobOptions, SurveyJobTempl ]) read_role = ImplicitRoleField(parent_role=[ 'singleton:' + ROLE_SINGLETON_SYSTEM_AUDITOR, - 'organization.auditor_role', 'execute_role', 'admin_role' + 'organization.auditor_role', 'execute_role', 'admin_role', + 'approval_role', ]) approval_role = ImplicitRoleField(parent_role=[ 'singleton:' + ROLE_SINGLETON_SYSTEM_AUDITOR, diff --git a/awx/main/scheduler/task_manager.py b/awx/main/scheduler/task_manager.py index 8b70bb8db4..ce49d8120a 100644 --- a/awx/main/scheduler/task_manager.py +++ b/awx/main/scheduler/task_manager.py @@ -523,6 +523,7 @@ class TaskManager(): workflow_approval = WorkflowApproval.objects.filter(status='pending').prefetch_related('workflow_approval_template') now = tz_now() for task in workflow_approval: + # TODO: copy the timeout to the job itself at launch time, not the template approval_timeout_seconds = timedelta(seconds=task.workflow_approval_template.timeout) if task.workflow_approval_template.timeout == 0: continue