Merge pull request #7170 from chrismeyersfsu/fix-4779

validate schedule extra_data

awx/main/models/schedules.py

@@ -5,12 +5,14 @@ import re
 import logging
 import datetime
 import dateutil.rrule
+import json
 
 # Django
 from django.db import models
 from django.db.models.query import QuerySet
 from django.utils.timezone import now, make_aware, get_default_timezone
 from django.utils.translation import ugettext_lazy as _
+from django.core.exceptions import ValidationError
 
 # AWX
 from awx.api.versioning import reverse
@@ -95,6 +97,40 @@ class Schedule(CommonModel):
         default={}
     )
 
+    # extra_data is actually a string with a JSON payload in it. This
+    # is technically OK because a string is a valid JSON. One day we will
+    # enforce non-string JSON.
+    def _clean_extra_data_system_jobs(self):
+        extra_data = self.extra_data
+        if not isinstance(extra_data, dict):
+            try:
+                extra_data = json.loads(self.extra_data)
+            except:
+                raise ValidationError(_("Expected JSON"))
+
+        if extra_data and 'days' in extra_data:
+            try:
+                if type(extra_data['days']) is bool:
+                    raise ValueError
+                if float(extra_data['days']) != int(extra_data['days']):
+                    raise ValueError
+                days = int(extra_data['days'])
+                if days < 0:
+                    raise ValueError
+            except ValueError:
+                raise ValidationError(_("days must be a positive integer."))
+        return self.extra_data
+
+    def clean_extra_data(self):
+        if not self.unified_job_template:
+            return self.extra_data
+
+        # Compare class by string name because it's hard to import SystemJobTemplate
+        if type(self.unified_job_template).__name__ is not 'SystemJobTemplate':
+            return self.extra_data
+
+        return self._clean_extra_data_system_jobs()
+
     def __unicode__(self):
         return u'%s_t%s_%s_%s' % (self.name, self.unified_job_template.id, self.id, self.next_run)
 

awx/main/tests/unit/models/test_schedules.py

@@ -0,0 +1,68 @@
+import pytest
+import json
+
+from django.core.exceptions import ValidationError
+
+from awx.main.models import (
+    Schedule,
+    SystemJobTemplate,
+    JobTemplate,
+)
+
+
+def test_clean_extra_data_system_job(mocker):
+    jt = SystemJobTemplate()
+    schedule = Schedule(unified_job_template=jt)
+    schedule._clean_extra_data_system_jobs = mocker.MagicMock()
+
+    schedule.clean_extra_data()
+
+    schedule._clean_extra_data_system_jobs.assert_called_once()
+ 
+
+def test_clean_extra_data_other_job(mocker):
+    jt = JobTemplate()
+    schedule = Schedule(unified_job_template=jt)
+    schedule._clean_extra_data_system_jobs = mocker.MagicMock()
+
+    schedule.clean_extra_data()
+
+    schedule._clean_extra_data_system_jobs.assert_not_called()
+    
+
+@pytest.mark.parametrize("extra_data", [
+    '{ "days": 1 }',
+    '{ "days": 100 }',
+    '{ "days": 0 }',
+    {"days": 0},
+    {"days": 1},
+    {"days": 13435},
+])
+def test_valid__clean_extra_data_system_jobs(extra_data):
+    schedule = Schedule()
+    schedule.extra_data = extra_data
+    schedule._clean_extra_data_system_jobs()
+
+
+@pytest.mark.parametrize("extra_data", [
+    '{ "days": 1.2 }',
+    '{ "days": -1.2 }',
+    '{ "days": -111 }',
+    '{ "days": "-111" }',
+    '{ "days": false }',
+    '{ "days": "foobar" }',
+    {"days": 1.2},
+    {"days": -1.2},
+    {"days": -111},
+    {"days": "-111"},
+    {"days": False},
+    {"days": "foobar"},
+])
+def test_invalid__clean_extra_data_system_jobs(extra_data):
+    schedule = Schedule()
+    schedule.extra_data = extra_data
+    with pytest.raises(ValidationError) as e:
+        schedule._clean_extra_data_system_jobs()
+
+    assert json.dumps(str(e.value)) == json.dumps(str([u'days must be a positive integer.']))
+