Fix LTI validation process (#767)

All client_key validation process is moved to validate_client_key method.
Method check_client_key was removed as redundant one.

In validate_timestamp_and_nonce is added timestamp checking that cannot be less than previous request's timestamp received from the certain LTI consumer. And nonce checking that cannot be duplicated.

Timestamp and nonce are storing in the cache with the TIMEOUT option which is by default set to 10 seconds and it is fair enough to prevent replay-attack through the LTI.

Add README file with the link to documentation.

StarCellBio/settings.py

@@ -161,6 +161,14 @@
     'default': {
         'BACKEND': 'django.core.cache.backends.filebased.FileBasedCache',
         'LOCATION': '/var/tmp/django_cache',
+    },
+    # NOTE(idegtiarov) LTI request validation required nonce and timestamp caching with small TIMEOUT param.
+    # To ensure all lti related data has the same lifetime and frequently deleted items are split from other cache data
+    # 'lti_cache' was added.
+    'lti_cache': {
+        'BACKEND': 'django.core.cache.backends.filebased.FileBasedCache',
+        'LOCATION': '/var/tmp/django_lti_cache',
+        'TIMEOUT': 10,
     }
 }
 

lti_provider/README.md

@@ -0,0 +1,5 @@
+# LTI Provider Configuration
+
+Documentation describing configuration on Provider and Consumer sides
+could be found in the google document by the link:
+[LTI Configurations](https://docs.google.com/document/d/1vW0HEbjVvxdG6ezZM-mkuijiJMqemG-745IXSHtGdb0/edit?usp=sharing)

lti_provider/unit_tests/test_validator.py

@@ -1,7 +1,9 @@
 import datetime
+import time
 
+from django.conf import settings
+from django.core.cache import caches
 from django.test import TestCase
-from oauthlib import oauth1
 
 from lti_provider.models import Consumer
 from lti_provider.validator import RequestValidator
@@ -12,6 +14,8 @@ class TestRequestValidator(TestCase):
     def setUpClass(cls):
         cls.key = 'starcellbio201707181230key'
         cls.secret = 'verysctonumk201783test_key'
+        cls.nonce = 'fake_nonce'
+
         cls.exp_date = datetime.date.today() + datetime.timedelta(days=1)
         cls.lti_consumer = Consumer.objects.create(
             consumer_name='testLTI',
@@ -20,39 +24,69 @@ def setUpClass(cls):
             expiration_date=cls.exp_date
         )
         cls.validator = RequestValidator()
+        cls.cache = caches['lti_cache']
 
     @classmethod
     def tearDownClass(cls):
         cls.lti_consumer.delete()
 
-    def test_check_client_key_valid(self):
-        is_valid = self.validator.check_client_key(self.key)
-        self.assertTrue(is_valid, msg='Consumer key is not valid')
-
-    def test_check_client_key_invalid(self):
-        key = 'fake_key'
-        msg = 'Consumer with the key {} is not found.'.format(key)
-        try:
-            self.validator.check_client_key(key)
-        except oauth1.OAuth1Error as err:
-            self.assertEqual(err.description, msg)
-
-    def test_validate_timestamp_valid(self):
-        is_valid = self.validator.validate_timestamp_and_nonce(self.key, 'fake_timestamp', 'fake_nonce', 'fake_request')
+    def test_validate_client_key_is_valid(self):
+        is_valid = self.validator.validate_client_key(self.key, 'fake_request')
         self.assertTrue(is_valid, msg='Consumer key is expired.')
 
-    def test_validate_timestamp_invalid(self):
+    def test_validate_client_key_is_invalid(self):
+        is_fake_key_valid = self.validator.validate_client_key('fake_key', 'fake_request')
+        self.assertFalse(is_fake_key_valid)
         exp_date = datetime.date.today() - datetime.timedelta(days=1)
         self.lti_consumer.expiration_date = exp_date
         self.lti_consumer.save()
-        msg = 'Consumer key {} is expired, expiration date is {}.'.format(self.key, exp_date)
-        try:
-            self.validator.validate_timestamp_and_nonce(self.key, 'fake_timestamp', 'fake_nonce', 'fake_request')
-        except oauth1.OAuth1Error as err:
-            self.assertEqual(err.description, msg)
+        is_valid = self.validator.validate_client_key(self.key, 'fake_request')
+        self.assertFalse(is_valid)
         self.lti_consumer.expiration_date = self.exp_date
         self.lti_consumer.save()
 
     def test_get_client_secret(self):
+        # NOTE(idegtiarov) validate_client_key is run to ensure RequestValidator instance initialize LTIConsumer
+        self.validator.validate_client_key(self.key, 'fake_request')
         client_secret = self.validator.get_client_secret(self.key, 'fake_request')
         self.assertEqual(client_secret, self.secret)
+
+    def test_validate_timestamp_and_nonce_valid(self):
+        timestamp = int(time.time())
+        is_valid = self.validator.validate_timestamp_and_nonce(self.key, timestamp, self.nonce, 'fake_request')
+        self.assertTrue(is_valid)
+        timestamp += 1
+        is_valid_updated_timestamp = self.validator.validate_timestamp_and_nonce(
+            self.key, timestamp, 'new_fake_nonce', 'fake_request'
+        )
+        self.assertTrue(is_valid_updated_timestamp)
+        self.cache.delete_many([self.nonce, 'new_fake_nonce', '{}_timestamp'.format(self.key)])
+
+    def test_validate_timestamp_and_nonce_invalid(self):
+        timestamp = int(time.time())
+        self.cache.set_many({'{}_timestamp'.format(self.key): timestamp, self.nonce: 1})
+        is_valid_past_timestamp = self.validator.validate_timestamp_and_nonce(
+            self.key, timestamp - 1, 'new_fake_nonce', 'fake_request'
+        )
+        self.assertFalse(is_valid_past_timestamp)
+        is_valid_duplicated_nonce = self.validator.validate_timestamp_and_nonce(
+            self.key, timestamp + 1, self.nonce, 'fake_request'
+        )
+        self.assertFalse(is_valid_duplicated_nonce)
+        is_valid = self.validator.validate_timestamp_and_nonce(
+            self.key, timestamp + 1, 'new_fake_nonce', 'fake_request'
+        )
+        self.assertTrue(is_valid)
+        self.cache.delete_many([self.nonce, 'new_fake_nonce', '{}_timestamp'.format(self.key)])
+
+    def test_cache_timelimit_for_validate_timestamp_and_nonce(self):
+        timestamp = int(time.time())
+        self.cache.set(self.nonce, 1)
+        is_valid = self.validator.validate_timestamp_and_nonce(self.key, timestamp, self.nonce, 'fake_request')
+        self.assertFalse(is_valid)
+        time.sleep(settings.CACHES['lti_cache']['TIMEOUT'])
+        is_valid_after_cache_cleared = self.validator.validate_timestamp_and_nonce(
+            self.key, timestamp , self.nonce, 'fake_request'
+        )
+        self.assertTrue(is_valid_after_cache_cleared)
+        self.cache.delete_many(self.nonce, '{}_timestamp'.format(self.key))

lti_provider/validator.py

@@ -1,10 +1,11 @@
+import datetime
 import logging
 
-import datetime
+from django.core.cache import caches
 from oauthlib import oauth1
 
 from lti_provider.models import Consumer
-from lti_provider import utils, lti_settings
+from lti_provider import lti_settings
 
 logger = logging.getLogger(__name__)
 
@@ -14,6 +15,7 @@ class RequestValidator(oauth1.RequestValidator):
     def __init__(self):
         super(RequestValidator, self).__init__()
         self.consumer = None
+        self.cache = caches['lti_cache']
 
     @property
     def enforce_ssl(self):
@@ -23,46 +25,61 @@ def enforce_ssl(self):
             ssl = True
         return ssl
 
-    def check_client_key(self, client_key):
+    def validate_timestamp_and_nonce(self, client_key, timestamp, nonce, request):
         """
-        Check client key is provided correctly and LII Consumer with that key exists
+        Validate LTI request's timestamp and nonce
 
-        :param client_key: client key from LTI request
-        :return: boolean flag
-        """
-        logger.debug('Client key is checking')
-        try:
-            self.consumer = Consumer.objects.get(consumer_key=client_key)
-        except Consumer.DoesNotExist:
-            raise oauth1.OAuth1Error('Consumer with the key {} is not found.'.format(client_key))
-        return super(RequestValidator, self).check_client_key(client_key)
+        Timestamp is validated to be equal or greater than the timestamp used in previous requests from certain
+        LTI Consumer.
 
-    def validate_timestamp_and_nonce(self, client_key, timestamp, nonce, request):
-        """
-        Validate LTI Consumer has not expired key
+        Nonce is validating to be unique in the time frame which is by default equal to 10 seconds.
+        Time frame could be configured in the StarCellBio settings as a TIMEOUT parameter of the CACHES['lti_cache']
 
         :param client_key: client key from LTI request
         :param timestamp: timestamp from LTI request
         :param nonce: nonce from LTI request
         :param request: LTI request
         :return: boolean flag
         """
+        message = "LTI request's {} is not valid."
+
         logger.debug('Timestamp validating is started.')
-        today = datetime.date.today()
-        consumer_expired_date = self.consumer.expiration_date
-        if consumer_expired_date and consumer_expired_date < today:
-            raise oauth1.OAuth1Error('Consumer Key is expired.')
+        timestamp = int(timestamp)
+        timestamp_key = '{}_timestamp'.format(client_key)
+        cache_timestamp = self.cache.get(timestamp_key, timestamp)
+        if cache_timestamp > timestamp:
+            logger.debug(message.format('timestamp'))
+            return False
+        self.cache.set(timestamp_key, timestamp)
+        logger.debug('Timestamp is valid.')
+
+        logger.debug('Nonce validating is started.')
+        if self.cache.get(nonce):
+            logger.debug(message.format('nonce'))
+            return False
+        self.cache.set(nonce, 1)
+        logger.debug('Nonce is valid.')
         return True
 
     def validate_client_key(self, client_key, request):
         """
-        Validate client key ...
+        Validate client key exists and is not expired
 
         :param client_key: client key from LTI request
         :param request: LTI request
         :return: boolean flag
         """
-        logger.debug('Client key validating is started.')
+        logger.debug('Consumer key relevance validating is started.')
+        try:
+            self.consumer = Consumer.objects.get(consumer_key=client_key)
+        except Consumer.DoesNotExist:
+            logger.error('Consumer with the key {} is not found.'.format(client_key))
+            return False
+        today = datetime.date.today()
+        consumer_expired_date = self.consumer.expiration_date
+        if consumer_expired_date and consumer_expired_date < today:
+            logger.error('Consumer Key is expired.')
+            return False
         return True
 
     def get_client_secret(self, client_key, request):

lti_provider/views.py

@@ -24,8 +24,11 @@
 
 
 def config(request):
-    # Code from lti_django example
-    # basic stuff
+    """
+    View for config endpoint lti/config
+
+    :return: XML with main info about Star Cell Bio source and LTI launch URL structure
+    """
     app_title = 'StarCellBio'
     app_description = 'Star Cell Bio LTI Application'
     launch_view_name = 'lti:launch_experiment'