Commit cea06e12 by Will Daly

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.
parent 86f17d02
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
Convenience methods for working with datetime objects Convenience methods for working with datetime objects
""" """
from datetime import timedelta from datetime import datetime, timedelta
import re import re
from pytz import timezone, UTC, UnknownTimeZoneError from pytz import timezone, UTC, UnknownTimeZoneError
...@@ -73,6 +73,27 @@ def almost_same_datetime(dt1, dt2, allowed_delta=timedelta(minutes=1)): ...@@ -73,6 +73,27 @@ def almost_same_datetime(dt1, dt2, allowed_delta=timedelta(minutes=1)):
return abs(dt1 - dt2) < allowed_delta 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_SHORT_DATE_FORMAT = "%b %d, %Y"
DEFAULT_LONG_DATE_FORMAT = "%A, %B %d, %Y" DEFAULT_LONG_DATE_FORMAT = "%A, %B %d, %Y"
DEFAULT_TIME_FORMAT = "%I:%M:%S %p" DEFAULT_TIME_FORMAT = "%I:%M:%S %p"
......
...@@ -4,9 +4,13 @@ Contains the APIs for course credit requirements. ...@@ -4,9 +4,13 @@ Contains the APIs for course credit requirements.
import logging import logging
import uuid import uuid
import datetime
import pytz
from django.db import transaction from django.db import transaction
from util.date_utils import to_timestamp
from opaque_keys import InvalidKeyError from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.keys import CourseKey
...@@ -191,7 +195,7 @@ def create_credit_request(course_key, provider_id, username): ...@@ -191,7 +195,7 @@ def create_credit_request(course_key, provider_id, username):
"method": "POST", "method": "POST",
"parameters": { "parameters": {
"request_uuid": "557168d0f7664fe59097106c67c3f847", "request_uuid": "557168d0f7664fe59097106c67c3f847",
"timestamp": "2015-05-04T20:57:57.987119+00:00", "timestamp": 1434631630,
"course_org": "HogwartsX", "course_org": "HogwartsX",
"course_num": "Potions101", "course_num": "Potions101",
"course_run": "1T2015", "course_run": "1T2015",
...@@ -263,6 +267,8 @@ def create_credit_request(course_key, provider_id, username): ...@@ -263,6 +267,8 @@ def create_credit_request(course_key, provider_id, username):
if created: if created:
credit_request.uuid = uuid.uuid4().hex credit_request.uuid = uuid.uuid4().hex
else:
credit_request.timestamp = datetime.datetime.now(pytz.UTC)
# Retrieve user account and profile info # Retrieve user account and profile info
user = User.objects.select_related('profile').get(username=username) user = User.objects.select_related('profile').get(username=username)
...@@ -285,7 +291,7 @@ def create_credit_request(course_key, provider_id, username): ...@@ -285,7 +291,7 @@ def create_credit_request(course_key, provider_id, username):
parameters = { parameters = {
"request_uuid": credit_request.uuid, "request_uuid": credit_request.uuid,
"timestamp": credit_request.timestamp.isoformat(), "timestamp": to_timestamp(credit_request.timestamp),
"course_org": course_key.org, "course_org": course_key.org,
"course_num": course_key.course, "course_num": course_key.course,
"course_run": course_key.run, "course_run": course_key.run,
...@@ -391,7 +397,7 @@ def get_credit_requests_for_user(username): ...@@ -391,7 +397,7 @@ def get_credit_requests_for_user(username):
[ [
{ {
"uuid": "557168d0f7664fe59097106c67c3f847", "uuid": "557168d0f7664fe59097106c67c3f847",
"timestamp": "2015-05-04T20:57:57.987119+00:00", "timestamp": 1434631630,
"course_key": "course-v1:HogwartsX+Potions101+1T2015", "course_key": "course-v1:HogwartsX+Potions101+1T2015",
"provider": { "provider": {
"id": "HogwartsX", "id": "HogwartsX",
......
...@@ -13,8 +13,8 @@ from django.db import transaction ...@@ -13,8 +13,8 @@ from django.db import transaction
from django.core.validators import RegexValidator from django.core.validators import RegexValidator
from simple_history.models import HistoricalRecords from simple_history.models import HistoricalRecords
from jsonfield.fields import JSONField from jsonfield.fields import JSONField
from util.date_utils import to_timestamp
from model_utils.models import TimeStampedModel from model_utils.models import TimeStampedModel
from xmodule_django.models import CourseKeyField from xmodule_django.models import CourseKeyField
from django.utils.translation import ugettext_lazy from django.utils.translation import ugettext_lazy
...@@ -378,7 +378,7 @@ class CreditRequest(TimeStampedModel): ...@@ -378,7 +378,7 @@ class CreditRequest(TimeStampedModel):
[ [
{ {
"uuid": "557168d0f7664fe59097106c67c3f847", "uuid": "557168d0f7664fe59097106c67c3f847",
"timestamp": "2015-05-04T20:57:57.987119+00:00", "timestamp": 1434631630,
"course_key": "course-v1:HogwartsX+Potions101+1T2015", "course_key": "course-v1:HogwartsX+Potions101+1T2015",
"provider": { "provider": {
"id": "HogwartsX", "id": "HogwartsX",
...@@ -393,7 +393,7 @@ class CreditRequest(TimeStampedModel): ...@@ -393,7 +393,7 @@ class CreditRequest(TimeStampedModel):
return [ return [
{ {
"uuid": request.uuid, "uuid": request.uuid,
"timestamp": request.modified, "timestamp": to_timestamp(request.modified),
"course_key": request.course.course_key, "course_key": request.course.course_key,
"provider": { "provider": {
"id": request.provider.provider_id, "id": request.provider.provider_id,
......
...@@ -5,7 +5,6 @@ Tests for the API functions in the credit app. ...@@ -5,7 +5,6 @@ Tests for the API functions in the credit app.
import datetime import datetime
import ddt import ddt
import pytz import pytz
import dateutil.parser as date_parser
from django.test import TestCase from django.test import TestCase
from django.test.utils import override_settings from django.test.utils import override_settings
from django.db import connection, transaction from django.db import connection, transaction
...@@ -13,6 +12,7 @@ from django.db import connection, transaction ...@@ -13,6 +12,7 @@ from django.db import connection, transaction
from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.keys import CourseKey
from student.tests.factories import UserFactory 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 import api
from openedx.core.djangoapps.credit.exceptions import ( from openedx.core.djangoapps.credit.exceptions import (
InvalidCreditRequirements, InvalidCreditRequirements,
...@@ -340,7 +340,7 @@ class CreditProviderIntegrationApiTests(CreditApiTestBase): ...@@ -340,7 +340,7 @@ class CreditProviderIntegrationApiTests(CreditApiTestBase):
# Validate the timestamp # Validate the timestamp
self.assertIn('timestamp', parameters) 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)) self.assertTrue(parsed_date < datetime.datetime.now(pytz.UTC))
# Validate course information # Validate course information
......
...@@ -15,6 +15,7 @@ from django.conf import settings ...@@ -15,6 +15,7 @@ from django.conf import settings
from student.tests.factories import UserFactory from student.tests.factories import UserFactory
from util.testing import UrlResetMixin from util.testing import UrlResetMixin
from util.date_utils import to_timestamp
from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.keys import CourseKey
from openedx.core.djangoapps.credit import api from openedx.core.djangoapps.credit import api
from openedx.core.djangoapps.credit.signature import signature from openedx.core.djangoapps.credit.signature import signature
...@@ -186,8 +187,8 @@ class CreditProviderViewTests(UrlResetMixin, TestCase): ...@@ -186,8 +187,8 @@ class CreditProviderViewTests(UrlResetMixin, TestCase):
# Simulate a callback from the credit provider with a timestamp too far in the past # Simulate a callback from the credit provider with a timestamp too far in the past
# (slightly more than 15 minutes) # (slightly more than 15 minutes)
# Since the message isn't timely, respond with a 403. # Since the message isn't timely, respond with a 403.
timestamp = datetime.datetime.now(pytz.UTC) - datetime.timedelta(0, 60 * 15 + 1) timestamp = to_timestamp(datetime.datetime.now(pytz.UTC) - datetime.timedelta(0, 60 * 15 + 1))
response = self._credit_provider_callback(request_uuid, "approved", timestamp=timestamp.isoformat()) response = self._credit_provider_callback(request_uuid, "approved", timestamp=timestamp)
self.assertEqual(response.status_code, 403) self.assertEqual(response.status_code, 403)
def test_credit_provider_callback_is_idempotent(self): def test_credit_provider_callback_is_idempotent(self):
...@@ -311,7 +312,7 @@ class CreditProviderViewTests(UrlResetMixin, TestCase): ...@@ -311,7 +312,7 @@ class CreditProviderViewTests(UrlResetMixin, TestCase):
""" """
provider_id = kwargs.get("provider_id", self.PROVIDER_ID) provider_id = kwargs.get("provider_id", self.PROVIDER_ID)
secret_key = kwargs.get("secret_key", TEST_CREDIT_PROVIDER_SECRET_KEY) 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]) url = reverse("credit:provider_callback", args=[provider_id])
......
...@@ -4,8 +4,6 @@ Views for the credit Django app. ...@@ -4,8 +4,6 @@ Views for the credit Django app.
import json import json
import datetime import datetime
import logging import logging
import dateutil
import pytz import pytz
from django.http import ( from django.http import (
...@@ -21,6 +19,7 @@ from opaque_keys.edx.keys import CourseKey ...@@ -21,6 +19,7 @@ from opaque_keys.edx.keys import CourseKey
from opaque_keys import InvalidKeyError from opaque_keys import InvalidKeyError
from util.json_request import JsonResponse 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 import api
from openedx.core.djangoapps.credit.signature import signature, get_shared_secret_key from openedx.core.djangoapps.credit.signature import signature, get_shared_secret_key
from openedx.core.djangoapps.credit.exceptions import CreditApiBadRequest, CreditRequestNotFound from openedx.core.djangoapps.credit.exceptions import CreditApiBadRequest, CreditRequestNotFound
...@@ -57,7 +56,7 @@ def create_credit_request(request, provider_id): ...@@ -57,7 +56,7 @@ def create_credit_request(request, provider_id):
"method": "POST", "method": "POST",
"parameters": { "parameters": {
request_uuid: "557168d0f7664fe59097106c67c3f847" request_uuid: "557168d0f7664fe59097106c67c3f847"
timestamp: "2015-05-04T20:57:57.987119+00:00" timestamp: 1434631630,
course_org: "ASUx" course_org: "ASUx"
course_num: "DemoX" course_num: "DemoX"
course_run: "1T2015" course_run: "1T2015"
...@@ -139,7 +138,7 @@ def credit_provider_callback(request, provider_id): ...@@ -139,7 +138,7 @@ def credit_provider_callback(request, provider_id):
{ {
"request_uuid": "557168d0f7664fe59097106c67c3f847", "request_uuid": "557168d0f7664fe59097106c67c3f847",
"status": "approved", "status": "approved",
"timestamp": "2015-05-04T20:57:57.987119+00:00", "timestamp": 1434631630,
"signature": "cRCNjkE4IzY+erIjRwOQCpRILgOvXx4q2qvx141BCqI=" "signature": "cRCNjkE4IzY+erIjRwOQCpRILgOvXx4q2qvx141BCqI="
} }
...@@ -151,8 +150,8 @@ def credit_provider_callback(request, provider_id): ...@@ -151,8 +150,8 @@ def credit_provider_callback(request, provider_id):
* status (string): Either "approved" or "rejected". * status (string): Either "approved" or "rejected".
* timestamp (string): The datetime at which the POST request was made, in ISO 8601 format. * timestamp (int): The datetime at which the POST request was made, represented
This will always include time-zone information. as the number of seconds since January 1, 1970 00:00:00 UTC.
* signature (string): A digital signature of the request parameters, * signature (string): A digital signature of the request parameters,
created using a secret key shared with the credit provider. created using a secret key shared with the credit provider.
...@@ -253,29 +252,21 @@ def _validate_signature(parameters, provider_id): ...@@ -253,29 +252,21 @@ def _validate_signature(parameters, provider_id):
return HttpResponseForbidden("Invalid signature.") 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. Check that the timestamp of the request is recent.
Arguments: 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. provider_id (unicode): Identifier for the credit provider.
Returns: Returns:
HttpResponse or None HttpResponse or None
""" """
# If we can't parse the datetime string, reject the request. timestamp = from_timestamp(timestamp_value)
try: if timestamp is None:
# dateutil's parser has some counter-intuitive behavior: msg = u'"{timestamp}" is not a valid timestamp'.format(timestamp=timestamp_value)
# 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)
log.warning(msg) log.warning(msg)
return HttpResponseBadRequest(msg) return HttpResponseBadRequest(msg)
...@@ -287,6 +278,6 @@ def _validate_timestamp(timestamp_str, provider_id): ...@@ -287,6 +278,6 @@ def _validate_timestamp(timestamp_str, provider_id):
u'Timestamp %s is too far in the past (%s seconds), ' u'Timestamp %s is too far in the past (%s seconds), '
u'so we are rejecting the notification from the credit provider "%s".' 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.") return HttpResponseForbidden(u"Timestamp is too far in the past.")
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment