From cea06e1217a621cace4bdfc3879baf02edae99de Mon Sep 17 00:00:00 2001 From: Will Daly Date: Thu, 18 Jun 2015 09:00:46 -0400 Subject: [PATCH] Update the format of the credit provider timestamp. Use the number of seconds since the epoch (Jan 1 1970 UTC) instead of an ISO-formatted datetime string. This is easier for credit providers to parse. --- common/djangoapps/util/date_utils.py | 23 +++++++++++++- openedx/core/djangoapps/credit/api.py | 12 +++++-- openedx/core/djangoapps/credit/models.py | 6 ++-- .../core/djangoapps/credit/tests/test_api.py | 4 +-- .../djangoapps/credit/tests/test_views.py | 7 +++-- openedx/core/djangoapps/credit/views.py | 31 +++++++------------ 6 files changed, 51 insertions(+), 32 deletions(-) diff --git a/common/djangoapps/util/date_utils.py b/common/djangoapps/util/date_utils.py index 11ca25d087..fde0bfee2c 100644 --- a/common/djangoapps/util/date_utils.py +++ b/common/djangoapps/util/date_utils.py @@ -2,7 +2,7 @@ Convenience methods for working with datetime objects """ -from datetime import timedelta +from datetime import datetime, timedelta import re from pytz import timezone, UTC, UnknownTimeZoneError @@ -73,6 +73,27 @@ def almost_same_datetime(dt1, dt2, allowed_delta=timedelta(minutes=1)): return abs(dt1 - dt2) < allowed_delta +def to_timestamp(datetime_value): + """ + Convert a datetime into a timestamp, represented as the number + of seconds since January 1, 1970 UTC. + """ + return int((datetime_value - datetime(1970, 1, 1, tzinfo=UTC)).total_seconds()) + + +def from_timestamp(timestamp): + """ + Convert a timestamp (number of seconds since Jan 1, 1970 UTC) + into a timezone-aware datetime. + + If the timestamp cannot be converted, returns None instead. + """ + try: + return datetime.utcfromtimestamp(timestamp).replace(tzinfo=UTC) + except (ValueError, TypeError): + return None + + DEFAULT_SHORT_DATE_FORMAT = "%b %d, %Y" DEFAULT_LONG_DATE_FORMAT = "%A, %B %d, %Y" DEFAULT_TIME_FORMAT = "%I:%M:%S %p" diff --git a/openedx/core/djangoapps/credit/api.py b/openedx/core/djangoapps/credit/api.py index ebec6b977d..cf8287e217 100644 --- a/openedx/core/djangoapps/credit/api.py +++ b/openedx/core/djangoapps/credit/api.py @@ -4,9 +4,13 @@ Contains the APIs for course credit requirements. import logging import uuid +import datetime + +import pytz from django.db import transaction +from util.date_utils import to_timestamp from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey @@ -191,7 +195,7 @@ def create_credit_request(course_key, provider_id, username): "method": "POST", "parameters": { "request_uuid": "557168d0f7664fe59097106c67c3f847", - "timestamp": "2015-05-04T20:57:57.987119+00:00", + "timestamp": 1434631630, "course_org": "HogwartsX", "course_num": "Potions101", "course_run": "1T2015", @@ -263,6 +267,8 @@ def create_credit_request(course_key, provider_id, username): if created: credit_request.uuid = uuid.uuid4().hex + else: + credit_request.timestamp = datetime.datetime.now(pytz.UTC) # Retrieve user account and profile info user = User.objects.select_related('profile').get(username=username) @@ -285,7 +291,7 @@ def create_credit_request(course_key, provider_id, username): parameters = { "request_uuid": credit_request.uuid, - "timestamp": credit_request.timestamp.isoformat(), + "timestamp": to_timestamp(credit_request.timestamp), "course_org": course_key.org, "course_num": course_key.course, "course_run": course_key.run, @@ -391,7 +397,7 @@ def get_credit_requests_for_user(username): [ { "uuid": "557168d0f7664fe59097106c67c3f847", - "timestamp": "2015-05-04T20:57:57.987119+00:00", + "timestamp": 1434631630, "course_key": "course-v1:HogwartsX+Potions101+1T2015", "provider": { "id": "HogwartsX", diff --git a/openedx/core/djangoapps/credit/models.py b/openedx/core/djangoapps/credit/models.py index 2e273be16f..3d10723d73 100644 --- a/openedx/core/djangoapps/credit/models.py +++ b/openedx/core/djangoapps/credit/models.py @@ -13,8 +13,8 @@ from django.db import transaction from django.core.validators import RegexValidator from simple_history.models import HistoricalRecords - from jsonfield.fields import JSONField +from util.date_utils import to_timestamp from model_utils.models import TimeStampedModel from xmodule_django.models import CourseKeyField from django.utils.translation import ugettext_lazy @@ -378,7 +378,7 @@ class CreditRequest(TimeStampedModel): [ { "uuid": "557168d0f7664fe59097106c67c3f847", - "timestamp": "2015-05-04T20:57:57.987119+00:00", + "timestamp": 1434631630, "course_key": "course-v1:HogwartsX+Potions101+1T2015", "provider": { "id": "HogwartsX", @@ -393,7 +393,7 @@ class CreditRequest(TimeStampedModel): return [ { "uuid": request.uuid, - "timestamp": request.modified, + "timestamp": to_timestamp(request.modified), "course_key": request.course.course_key, "provider": { "id": request.provider.provider_id, diff --git a/openedx/core/djangoapps/credit/tests/test_api.py b/openedx/core/djangoapps/credit/tests/test_api.py index 2a8da65ab3..91a902a3f2 100644 --- a/openedx/core/djangoapps/credit/tests/test_api.py +++ b/openedx/core/djangoapps/credit/tests/test_api.py @@ -5,7 +5,6 @@ Tests for the API functions in the credit app. import datetime import ddt import pytz -import dateutil.parser as date_parser from django.test import TestCase from django.test.utils import override_settings from django.db import connection, transaction @@ -13,6 +12,7 @@ from django.db import connection, transaction from opaque_keys.edx.keys import CourseKey from student.tests.factories import UserFactory +from util.date_utils import from_timestamp from openedx.core.djangoapps.credit import api from openedx.core.djangoapps.credit.exceptions import ( InvalidCreditRequirements, @@ -340,7 +340,7 @@ class CreditProviderIntegrationApiTests(CreditApiTestBase): # Validate the timestamp self.assertIn('timestamp', parameters) - parsed_date = date_parser.parse(parameters['timestamp']) + parsed_date = from_timestamp(parameters['timestamp']) self.assertTrue(parsed_date < datetime.datetime.now(pytz.UTC)) # Validate course information diff --git a/openedx/core/djangoapps/credit/tests/test_views.py b/openedx/core/djangoapps/credit/tests/test_views.py index 558b7a2a0b..ec7a8ba67c 100644 --- a/openedx/core/djangoapps/credit/tests/test_views.py +++ b/openedx/core/djangoapps/credit/tests/test_views.py @@ -15,6 +15,7 @@ from django.conf import settings from student.tests.factories import UserFactory from util.testing import UrlResetMixin +from util.date_utils import to_timestamp from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.credit import api from openedx.core.djangoapps.credit.signature import signature @@ -186,8 +187,8 @@ class CreditProviderViewTests(UrlResetMixin, TestCase): # Simulate a callback from the credit provider with a timestamp too far in the past # (slightly more than 15 minutes) # Since the message isn't timely, respond with a 403. - timestamp = datetime.datetime.now(pytz.UTC) - datetime.timedelta(0, 60 * 15 + 1) - response = self._credit_provider_callback(request_uuid, "approved", timestamp=timestamp.isoformat()) + timestamp = to_timestamp(datetime.datetime.now(pytz.UTC) - datetime.timedelta(0, 60 * 15 + 1)) + response = self._credit_provider_callback(request_uuid, "approved", timestamp=timestamp) self.assertEqual(response.status_code, 403) def test_credit_provider_callback_is_idempotent(self): @@ -311,7 +312,7 @@ class CreditProviderViewTests(UrlResetMixin, TestCase): """ provider_id = kwargs.get("provider_id", self.PROVIDER_ID) secret_key = kwargs.get("secret_key", TEST_CREDIT_PROVIDER_SECRET_KEY) - timestamp = kwargs.get("timestamp", datetime.datetime.now(pytz.UTC).isoformat()) + timestamp = kwargs.get("timestamp", to_timestamp(datetime.datetime.now(pytz.UTC))) url = reverse("credit:provider_callback", args=[provider_id]) diff --git a/openedx/core/djangoapps/credit/views.py b/openedx/core/djangoapps/credit/views.py index de9a86d1b6..976477616d 100644 --- a/openedx/core/djangoapps/credit/views.py +++ b/openedx/core/djangoapps/credit/views.py @@ -4,8 +4,6 @@ Views for the credit Django app. import json import datetime import logging - -import dateutil import pytz from django.http import ( @@ -21,6 +19,7 @@ from opaque_keys.edx.keys import CourseKey from opaque_keys import InvalidKeyError from util.json_request import JsonResponse +from util.date_utils import from_timestamp from openedx.core.djangoapps.credit import api from openedx.core.djangoapps.credit.signature import signature, get_shared_secret_key from openedx.core.djangoapps.credit.exceptions import CreditApiBadRequest, CreditRequestNotFound @@ -57,7 +56,7 @@ def create_credit_request(request, provider_id): "method": "POST", "parameters": { request_uuid: "557168d0f7664fe59097106c67c3f847" - timestamp: "2015-05-04T20:57:57.987119+00:00" + timestamp: 1434631630, course_org: "ASUx" course_num: "DemoX" course_run: "1T2015" @@ -139,7 +138,7 @@ def credit_provider_callback(request, provider_id): { "request_uuid": "557168d0f7664fe59097106c67c3f847", "status": "approved", - "timestamp": "2015-05-04T20:57:57.987119+00:00", + "timestamp": 1434631630, "signature": "cRCNjkE4IzY+erIjRwOQCpRILgOvXx4q2qvx141BCqI=" } @@ -151,8 +150,8 @@ def credit_provider_callback(request, provider_id): * status (string): Either "approved" or "rejected". - * timestamp (string): The datetime at which the POST request was made, in ISO 8601 format. - This will always include time-zone information. + * timestamp (int): The datetime at which the POST request was made, represented + as the number of seconds since January 1, 1970 00:00:00 UTC. * signature (string): A digital signature of the request parameters, created using a secret key shared with the credit provider. @@ -253,29 +252,21 @@ def _validate_signature(parameters, provider_id): return HttpResponseForbidden("Invalid signature.") -def _validate_timestamp(timestamp_str, provider_id): +def _validate_timestamp(timestamp_value, provider_id): """ Check that the timestamp of the request is recent. Arguments: - timestamp_str (str): ISO-8601 datetime formatted string. + timestamp (int): Number of seconds since Jan. 1, 1970 UTC. provider_id (unicode): Identifier for the credit provider. Returns: HttpResponse or None """ - # If we can't parse the datetime string, reject the request. - try: - # dateutil's parser has some counter-intuitive behavior: - # for example, given an empty string or "a" it always returns the current datetime. - # It is the responsibility of the credit provider to send a valid ISO-8601 datetime - # so we can validate it; otherwise, this check might not take effect. - # (Note that the signature check ensures that the timestamp we receive hasn't - # been tampered with after being issued by the credit provider). - timestamp = dateutil.parser.parse(timestamp_str) - except ValueError: - msg = u'"{timestamp}" is not an ISO-8601 formatted datetime'.format(timestamp=timestamp_str) + timestamp = from_timestamp(timestamp_value) + if timestamp is None: + msg = u'"{timestamp}" is not a valid timestamp'.format(timestamp=timestamp_value) log.warning(msg) return HttpResponseBadRequest(msg) @@ -287,6 +278,6 @@ def _validate_timestamp(timestamp_str, provider_id): u'Timestamp %s is too far in the past (%s seconds), ' u'so we are rejecting the notification from the credit provider "%s".' ), - timestamp_str, elapsed_seconds, provider_id, + timestamp_value, elapsed_seconds, provider_id, ) return HttpResponseForbidden(u"Timestamp is too far in the past.")