adhere to RFC5545 regarding UNTIL timezones

If the "DTSTART" property is specified as a date with UTC time or a date with
local time and time zone reference, then the UNTIL rule part MUST be specified
as a date with UTC time.

3 files changed, 51 insertions, 71 deletions

diff --git a/awx/main/models/schedules.py b/awx/main/models/schedules.py
index 3dc9ac2814..59629c89a7 100644
--- a/awx/main/models/schedules.py
+++ b/awx/main/models/schedules.py
@@ -10,7 +10,7 @@ from dateutil.tz import gettz, datetime_exists
 # Django
 from django.db import models
 from django.db.models.query import QuerySet
-from django.utils.timezone import now, make_aware
+from django.utils.timezone import now
 from django.utils.translation import ugettext_lazy as _
 
 # AWX
@@ -21,7 +21,6 @@ from awx.main.utils import ignore_inventory_computed_fields
 from awx.main.consumers import emit_channel_notification
 
 import pytz
-import six
 
 
 logger = logging.getLogger('awx.main.models.schedule')
@@ -103,70 +102,54 @@ class Schedule(CommonModel, LaunchTimeConfig):
     @classmethod
     def rrulestr(cls, rrule, **kwargs):
         """
-        Apply our own custom rrule parsing logic.  This applies some extensions
-        and limitations to parsing that are specific to our supported
-        implementation.  Namely:
+        Apply our own custom rrule parsing logic to support TZID=
 
-        * python-dateutil doesn't _natively_ support `DTSTART;TZID=`; this
-          function parses out the TZID= component and uses it to produce the
-          `tzinfos` keyword argument to `dateutil.rrule.rrulestr()`. In this
-          way, we translate:
+        python-dateutil doesn't _natively_ support `DTSTART;TZID=`; this
+        function parses out the TZID= component and uses it to produce the
+        `tzinfos` keyword argument to `dateutil.rrule.rrulestr()`. In this
+        way, we translate:
 
-          DTSTART;TZID=America/New_York:20180601T120000 RRULE:FREQ=DAILY;INTERVAL=1
+        DTSTART;TZID=America/New_York:20180601T120000 RRULE:FREQ=DAILY;INTERVAL=1
 
-          ...into...
+        ...into...
 
-          DTSTART:20180601T120000TZID RRULE:FREQ=DAILY;INTERVAL=1
+        DTSTART:20180601T120000TZID RRULE:FREQ=DAILY;INTERVAL=1
 
-          ...and we pass a hint about the local timezone to dateutil's parser:
-          `dateutil.rrule.rrulestr(rrule, {
-              'tzinfos': {
-                  'TZID': dateutil.tz.gettz('America/New_York')
-                }
-           })`
+        ...and we pass a hint about the local timezone to dateutil's parser:
+        `dateutil.rrule.rrulestr(rrule, {
+            'tzinfos': {
+                'TZID': dateutil.tz.gettz('America/New_York')
+              }
+         })`
 
-          it's possible that we can remove the custom code that performs this
-          parsing if TZID= gains support in upstream dateutil:
-          https://github.com/dateutil/dateutil/pull/615
-
-        * RFC5545 specifies that: if the "DTSTART" property is specified as
-          a date with local time (in our case, TZID=), then the UNTIL rule part
-          MUST also be treated as a date with local time.  If the "DTSTART"
-          property is specified as a date with UTC time (with a Z suffix),
-          then the UNTIL rule part MUST be specified as a date with UTC time.
-
-          this function provides additional parsing to translate RRULES like this:
-
-          DTSTART;TZID=America/New_York:20180601T120000 RRULE:FREQ=DAILY;INTERVAL=1;UNTIL=20180602T170000
-
-          ...into this (under the hood):
-
-          DTSTART;TZID=America/New_York:20180601T120000 RRULE:FREQ=DAILY;INTERVAL=1;UNTIL=20180602T210000Z
+        it's likely that we can remove the custom code that performs this
+        parsing if TZID= gains support in upstream dateutil:
+        https://github.com/dateutil/dateutil/pull/619
         """
-        source_rrule = rrule
+        kwargs['forceset'] = True
         kwargs['tzinfos'] = {}
         match = cls.TZID_REGEX.match(rrule)
         if match is not None:
             rrule = cls.TZID_REGEX.sub("DTSTART\g<stamp>TZI\g<rrule>", rrule)
             timezone = gettz(match.group('tzid'))
-            if 'until' in rrule.lower():
-                # if DTSTART;TZID= is used, coerce "naive" UNTIL values
-                # to the proper UTC date
-                match_until = re.match(".*?(?P<until>UNTIL\=[0-9]+T[0-9]+)(?P<utcflag>Z?)", rrule)
-                until_date = match_until.group('until').split("=")[1]
-                if len(match_until.group('utcflag')):
-                    raise ValueError(six.text_type(
-                        _('invalid rrule `{}` includes TZINFO= stanza and UTC-based UNTIL clause').format(source_rrule)
-                    ))  # noqa
-                localized = make_aware(
-                    datetime.datetime.strptime(until_date, "%Y%m%dT%H%M%S"),
-                    timezone
-                )
-                utc = localized.astimezone(pytz.utc).strftime('%Y%m%dT%H%M%SZ')
-                rrule = rrule.replace(match_until.group('until'), 'UNTIL={}'.format(utc))
             kwargs['tzinfos']['TZI'] = timezone
         x = dateutil.rrule.rrulestr(rrule, **kwargs)
 
+        for r in x._rrule:
+            if r._dtstart and r._until:
+                if all((
+                    r._dtstart.tzinfo != dateutil.tz.tzlocal(),
+                    r._until.tzinfo != dateutil.tz.tzutc(),
+                )):
+                    # According to RFC5545 Section 3.3.10:
+                    # https://tools.ietf.org/html/rfc5545#section-3.3.10
+                    #
+                    # > If the "DTSTART" property is specified as a date with UTC
+                    # > time or a date with local time and time zone reference,
+                    # > then the UNTIL rule part MUST be specified as a date with
+                    # > UTC time.
+                    raise ValueError('RRULE UNTIL values must be specified in UTC')
+
         try:
             first_event = x[0]
             if first_event < now() - datetime.timedelta(days=365 * 5):
@@ -192,7 +175,7 @@ class Schedule(CommonModel, LaunchTimeConfig):
         return job_kwargs
 
     def update_computed_fields(self):
-        future_rs = Schedule.rrulestr(self.rrule, forceset=True)
+        future_rs = Schedule.rrulestr(self.rrule)
         next_run_actual = future_rs.after(now())
 
         if next_run_actual is not None:
diff --git a/awx/main/tests/functional/models/test_schedule.py b/awx/main/tests/functional/models/test_schedule.py
index c4c6019503..3768058f4f 100644
--- a/awx/main/tests/functional/models/test_schedule.py
+++ b/awx/main/tests/functional/models/test_schedule.py
@@ -130,22 +130,19 @@ def test_utc_until(job_template, until, dtend):
 
 
 @pytest.mark.django_db
-@pytest.mark.parametrize('until, dtend', [
-    ['20180602T170000', '2018-06-02 16:00:00+00:00'],
-    ['20180602T000000', '2018-06-01 16:00:00+00:00'],
+@pytest.mark.parametrize('dtstart, until', [
+    ['20180601T120000Z', '20180602T170000'],
+    ['TZID=America/New_York:20180601T120000', '20180602T170000'],
 ])
-def test_tzinfo_until(job_template, until, dtend):
-    rrule = 'DTSTART;TZID=America/New_York:20180601T120000 RRULE:FREQ=DAILY;INTERVAL=1;UNTIL={}'.format(until)  # noqa
+def test_tzinfo_naive_until(job_template, dtstart, until):
+    rrule = 'DTSTART;{} RRULE:FREQ=DAILY;INTERVAL=1;UNTIL={}'.format(dtstart, until)  # noqa
     s = Schedule(
         name='Some Schedule',
         rrule=rrule,
         unified_job_template=job_template
     )
-    s.save()
-
-    assert str(s.next_run) == '2018-06-01 16:00:00+00:00'  # UTC = +4 EST
-    assert str(s.next_run) == str(s.dtstart)
-    assert str(s.dtend) == dtend
+    with pytest.raises(ValueError):
+        s.save()
 
 
 @pytest.mark.django_db
diff --git a/docs/schedules.md b/docs/schedules.md
index be826bf388..f733b37aa7 100644
--- a/docs/schedules.md
+++ b/docs/schedules.md
@@ -50,7 +50,10 @@ A list of _valid_ zone identifiers (which can vary by system) can be found at:
 
 UNTIL and Timezones
 ===================
-RFC5545 specifies that:
+`DTSTART` values provided to awx _must_ provide timezone information (they may
+not be naive dates).
+
+Additionally, RFC5545 specifies that:
 
 > Furthermore, if the "DTSTART" property is specified as a date with local
 > time, then the UNTIL rule part MUST also be specified as a date with local
@@ -58,20 +61,17 @@ RFC5545 specifies that:
 > a date with local time and time zone reference, then the UNTIL rule part
 > MUST be specified as a date with UTC time.
 
-Given this, this RRULE:
+Given this, `RRULE` values that specify `UNTIL` datetimes must *always* be in UTC.
 
+Valid:
     `DTSTART:20180601T120000Z RRULE:FREQ=DAILY;INTERVAL=1;UNTIL=20180606T170000Z`
+    `DTSTART;TZID=America/New_York:20180601T120000 RRULE:FREQ=DAILY;INTERVAL=1;UNTIL=20180606T170000Z`
 
-...will be interpretted as "Starting on June 1st, 2018 at noon UTC, repeat
-daily, ending on June 6th, 2018 at 5PM UTC".
-
-This RRULE:
+Not Valid:
 
+    `DTSTART:20180601T120000Z RRULE:FREQ=DAILY;INTERVAL=1;UNTIL=20180606T170000`
     `DTSTART;TZID=America/New_York:20180601T120000 RRULE:FREQ=DAILY;INTERVAL=1;UNTIL=20180606T170000`
 
-...will be interpretted as "Starting on June 1st, 2018 at noon EDT, repeat
-daily, ending on June 6th, 2018 at 5PM EDT".
-
 
 Previewing Schedules
 ====================