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

@@ -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
 

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."""