From b980d60e99eb7dcec5e3b6f01444c0d7cf07a20e Mon Sep 17 00:00:00 2001 From: Liam Allen Date: Fri, 28 Aug 2026 14:18:21 +0100 Subject: [PATCH] AAP-66689 - Fix JT admins unable to toggle ask_*_on_launch fields without project_inventory access (#16623) Fix JT admins unable to toggle ask_*_on_launch fields without project/inventory access --- awx/main/access.py | 30 +++++++++++----------- awx/main/tests/unit/test_access.py | 40 ++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 14 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index dc22c45529..c4c41bc164 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -24,6 +24,7 @@ from ansible_base.rbac.policies import visible_users from ansible_base.rbac import permission_registry # AWX +from awx.main.fields import AskForField from awx.main.utils import ( get_object_or_400, get_pk_from_dict, @@ -1592,12 +1593,6 @@ class JobTemplateAccess(NotificationAttachMixin, UnifiedCredentialsMixin, BaseAc 'job_tags', 'force_handlers', 'skip_tags', - 'ask_variables_on_launch', - 'ask_tags_on_launch', - 'ask_job_type_on_launch', - 'ask_skip_tags_on_launch', - 'ask_inventory_on_launch', - 'ask_credential_on_launch', 'survey_enabled', 'custom_virtualenv', 'diff_mode', @@ -1608,19 +1603,26 @@ class JobTemplateAccess(NotificationAttachMixin, UnifiedCredentialsMixin, BaseAc 'created', 'modified', ] + # ask_*_on_launch fields only toggle whether a field can be prompted for at launch + # time; they don't grant access to a new resource, so they are always non-sensitive + # regardless of which model defines them + ask_field_names = {f.name for f in obj._meta.get_fields() if isinstance(f, AskForField)} - for k, v in data.items(): - if k not in [x.name for x in obj._meta.concrete_fields]: + for field_name, new_value in data.items(): + if field_name not in [x.name for x in obj._meta.concrete_fields]: continue - if hasattr(obj, k) and getattr(obj, k) != v: - if ( - k not in allowed_fields - and v != getattr(obj, '%s_id' % k, None) - and not (hasattr(obj, '%s_id' % k) and getattr(obj, '%s_id' % k) is None and v == '') - ): # Equate '' to None in the case of foreign keys + if hasattr(obj, field_name) and getattr(obj, field_name) != new_value: + if field_name not in allowed_fields and field_name not in ask_field_names and not self._fk_value_unchanged(obj, field_name, new_value): return False return True + def _fk_value_unchanged(self, obj, field_name, new_value): + """True if new_value is just a different representation of the current FK value (id, or '' equated to None).""" + fk_id_attr = '%s_id' % field_name + if new_value == getattr(obj, fk_id_attr, None): + return True + return hasattr(obj, fk_id_attr) and getattr(obj, fk_id_attr) is None and new_value == '' + def can_delete(self, obj): return self.user.is_superuser or self.user in obj.admin_role diff --git a/awx/main/tests/unit/test_access.py b/awx/main/tests/unit/test_access.py index 08e1e66ab5..5e1631e031 100644 --- a/awx/main/tests/unit/test_access.py +++ b/awx/main/tests/unit/test_access.py @@ -6,11 +6,13 @@ from django.forms.models import model_to_dict from rest_framework.exceptions import ParseError from awx.main.access import BaseAccess, check_superuser, JobTemplateAccess, WorkflowJobTemplateAccess, vars_are_encrypted +from awx.main.fields import AskForField from awx.main.models import ( Credential, CredentialType, Inventory, + JobTemplate, Project, Role, Organization, @@ -150,6 +152,31 @@ def test_not_superuser(mocker): assert can_add(access, None) is False +def test_jt_clearing_unset_fk_with_empty_string_is_nonsensitive(job_template_with_ids, user_unit): + """Posting '' for an FK field that is already None (e.g. execution_environment) + should be treated as a no-op, not a sensitive change, since the UI sometimes + sends '' instead of null. + """ + access = JobTemplateAccess(user_unit) + assert job_template_with_ids.execution_environment_id is None + data = {'execution_environment': ''} + assert access.changes_are_non_sensitive(job_template_with_ids, data) + + +@pytest.mark.parametrize( + 'field_name, new_value, expected', + [ + ('inventory', 11, True), # matches current inventory_id + ('inventory', 12, False), # different id + ('execution_environment', '', True), # '' equated to None + ('execution_environment', 5, False), # not None, and not equal + ], +) +def test_fk_value_unchanged(job_template_with_ids, user_unit, field_name, new_value, expected): + access = JobTemplateAccess(user_unit) + assert access._fk_value_unchanged(job_template_with_ids, field_name, new_value) is expected + + def test_jt_existing_values_are_nonsensitive(job_template_with_ids, user_unit): """Assure that permission checks are not required if submitted data is identical to what the job template already has.""" @@ -160,6 +187,19 @@ def test_jt_existing_values_are_nonsensitive(job_template_with_ids, user_unit): assert access.changes_are_non_sensitive(job_template_with_ids, data) +@pytest.mark.parametrize('field_name', [f.name for f in JobTemplate._meta.get_fields() if isinstance(f, AskForField)]) +def test_jt_ask_fields_are_nonsensitive(job_template_with_ids, user_unit, field_name): + """Toggling any ask_*_on_launch field should be considered a non-sensitive + change so that JT admins without project/inventory use_role can still + modify these boolean prompt fields. + """ + access = JobTemplateAccess(user_unit) + # The field defaults to False on the template; flipping it to True should + # still be treated as non-sensitive. + data = {field_name: True} + assert access.changes_are_non_sensitive(job_template_with_ids, data) + + def test_change_jt_sensitive_data(job_template_with_ids, mocker, user_unit): """Assure that can_add is called with all ForeignKeys."""