Raise a validation error if a credential is set while the service is not

awx/api/serializers.py

@@ -2836,11 +2836,13 @@ class JobTemplateMixin(object):
                 'webhook_credential': _("Must be a Personal Access Token."),
             })
 
-        if webhook_service and webhook_credential:
+        if webhook_credential:
-            if webhook_credential.kind != '{}_token'.format(webhook_service):
+            msg = {'webhook_credential': _("Must match the selected webhook service.")}
-                raise serializers.ValidationError({
+            if webhook_service:
-                    'webhook_credential': _("Must match the selected webhook service."),
+                if webhook_credential.kind != '{}_token'.format(webhook_service):
-                })
+                    raise serializers.ValidationError(msg)
+            else:
+                raise serializers.ValidationError(msg)
 
         return super().validate(attrs)
 

awx/main/models/mixins.py

@@ -530,7 +530,7 @@ class WebhookTemplateMixin(models.Model):
                 self.webhook_key = ''
 
             if update_fields and 'webhook_service' in update_fields:
-                update_fields.append('webhook_key')
+                update_fields.add('webhook_key')
 
         super().save(*args, **kwargs)
 

awx/main/tests/functional/api/test_webhooks.py

@@ -202,3 +202,59 @@ def test_set_wrong_service_webhook_credential(organization_factory, job_template
     assert jt.webhook_key != ''
     assert jt.webhook_credential is None
     assert response.data == {'webhook_credential': ["Must match the selected webhook service."]}
+
+
+@pytest.mark.django_db
+@pytest.mark.parametrize(
+    "service", [s for s, _ in WebhookTemplateMixin.SERVICES]
+)
+def test_set_webhook_credential_without_service(organization_factory, job_template_factory, patch, service):
+    objs = organization_factory("org", superusers=['admin'])
+    jt = job_template_factory("jt", organization=objs.organization,
+                              inventory='test_inv', project='test_proj').job_template
+    admin = objs.superusers.admin
+    assert jt.webhook_service == ''
+    assert jt.webhook_key == ''
+
+    cred_type = CredentialType.defaults['{}_token'.format(service)]()
+    cred_type.save()
+    cred = Credential.objects.create(credential_type=cred_type, name='test-cred',
+                                     inputs={'token': 'secret'})
+
+    url = reverse('api:job_template_detail', kwargs={'pk': jt.pk})
+    response = patch(url, {'webhook_credential': cred.pk}, user=admin, expect=400)
+    jt.refresh_from_db()
+
+    assert jt.webhook_service == ''
+    assert jt.webhook_key == ''
+    assert jt.webhook_credential is None
+    assert response.data == {'webhook_credential': ["Must match the selected webhook service."]}
+
+
+@pytest.mark.django_db
+@pytest.mark.parametrize(
+    "service", [s for s, _ in WebhookTemplateMixin.SERVICES]
+)
+def test_unset_webhook_service_with_credential(organization_factory, job_template_factory, patch, service):
+    objs = organization_factory("org", superusers=['admin'])
+    jt = job_template_factory("jt", organization=objs.organization, webhook_service=service,
+                              inventory='test_inv', project='test_proj').job_template
+    admin = objs.superusers.admin
+    assert jt.webhook_service == service
+    assert jt.webhook_key != ''
+
+    cred_type = CredentialType.defaults['{}_token'.format(service)]()
+    cred_type.save()
+    cred = Credential.objects.create(credential_type=cred_type, name='test-cred',
+                                     inputs={'token': 'secret'})
+    jt.webhook_credential = cred
+    jt.save()
+
+    url = reverse('api:job_template_detail', kwargs={'pk': jt.pk})
+    response = patch(url, {'webhook_service': ''}, user=admin, expect=400)
+    jt.refresh_from_db()
+
+    assert jt.webhook_service == service
+    assert jt.webhook_key != ''
+    assert jt.webhook_credential == cred
+    assert response.data == {'webhook_credential': ["Must match the selected webhook service."]}