Merge pull request #833 from kobotoolbox/digest-nonce-cache

django-digest redis cache nonce storage backend

.github/workflows/pytest.yml

@@ -52,4 +52,5 @@ jobs:
         DJANGO_SECRET_KEY: ${{ secrets.DJANGO_SECRET_KEY }}
         TEST_DATABASE_URL: postgis://kobo:kobo@localhost:5432/kobocat_test
         REDIS_SESSION_URL: redis://localhost:6380/2
+        CACHE_URL: redis://localhost:6380/3
         USE_POSTGRESQL: True

.gitlab-ci.yml

@@ -21,7 +21,8 @@ test:
   variables:
     DJANGO_SECRET_KEY: abcdef
     TEST_DATABASE_URL: postgis://kobo:kobo@postgres:5432/kobocat_test
-    REDIS_SESSION_URL: redis://localhost:6380/2
+    REDIS_SESSION_URL: redis://redis_cache:6379/2
+    CACHE_URL: redis://redis_cache:6379/3
     USE_POSTGRESQL: "True"
     POSTGRES_USER: kobo
     POSTGRES_PASSWORD: kobo

dependencies/pip/dev.txt

@@ -88,6 +88,7 @@ django==2.2.28
     #   django-filter
     #   django-guardian
     #   django-oauth-toolkit
+    #   django-redis
     #   django-render-block
     #   django-reversion
     #   django-storages
@@ -118,6 +119,8 @@ django-oauth-toolkit==2.0.0
     # via -r dependencies/pip/requirements.in
 django-pure-pagination==0.3.0
     # via -r dependencies/pip/requirements.in
+django-redis==5.2.0
+    # via -r dependencies/pip/requirements.in
 django-redis-sessions==0.6.2
     # via -r dependencies/pip/requirements.in
 django-render-block==0.9.1
@@ -279,6 +282,7 @@ redis==4.2.2
     # via
     #   -r dependencies/pip/requirements.in
     #   celery
+    #   django-redis
     #   django-redis-sessions
 requests==2.27.1
     # via

dependencies/pip/prod.txt

@@ -78,6 +78,7 @@ django==2.2.28
     #   django-filter
     #   django-guardian
     #   django-oauth-toolkit
+    #   django-redis
     #   django-render-block
     #   django-reversion
     #   django-storages
@@ -108,6 +109,8 @@ django-oauth-toolkit==2.0.0
     # via -r dependencies/pip/requirements.in
 django-pure-pagination==0.3.0
     # via -r dependencies/pip/requirements.in
+django-redis==5.2.0
+    # via -r dependencies/pip/requirements.in
 django-redis-sessions==0.6.2
     # via -r dependencies/pip/requirements.in
 django-render-block==0.9.1
@@ -219,6 +222,7 @@ redis==4.2.2
     # via
     #   -r dependencies/pip/requirements.in
     #   celery
+    #   django-redis
     #   django-redis-sessions
 requests==2.27.1
     # via

dependencies/pip/requirements.in

@@ -13,6 +13,7 @@
 Django>=2.2,<2.3
 django-csp
 django-environ
+django-redis
 django-storages[azure,boto3]
 django-templated-email
 dpath

dependencies/pip/requirements.txt

@@ -78,6 +78,7 @@ django==2.2.28
     #   django-filter
     #   django-guardian
     #   django-oauth-toolkit
+    #   django-redis
     #   django-render-block
     #   django-reversion
     #   django-storages
@@ -108,6 +109,8 @@ django-oauth-toolkit==2.0.0
     # via -r dependencies/pip/requirements.in
 django-pure-pagination==0.3.0
     # via -r dependencies/pip/requirements.in
+django-redis==5.2.0
+    # via -r dependencies/pip/requirements.in
 django-redis-sessions==0.6.2
     # via -r dependencies/pip/requirements.in
 django-render-block==0.9.1
@@ -219,6 +222,7 @@ redis==4.2.2
     # via
     #   -r dependencies/pip/requirements.in
     #   celery
+    #   django-redis
     #   django-redis-sessions
 requests==2.27.1
     # via

onadata/apps/django_digest_backends/cache.py

@@ -0,0 +1,61 @@
+from django.core.cache import caches
+from django_digest.utils import get_setting
+from redis.exceptions import LockError
+
+NONCE_NO_COUNT = ''  # Needs to be something other than None to determine not set vs set to null
+
+
+class RedisCacheNonceStorage():
+    _blocking_timeout = 30
+
+    def _get_cache(self):
+        # Dynamic fetching of cache is necessary to work with override_settings
+        return caches[get_setting('DIGEST_NONCE_CACHE_NAME', 'default')]
+
+    def _get_timeout(self):
+        return get_setting('DIGEST_NONCE_TIMEOUT_IN_SECONDS', 5 * 60)
+
+    def _generate_cache_key(self, user, nonce):
+        return f'user_nonce_{user}_{nonce}'
+
+    def update_existing_nonce(self, user, nonce, nonce_count):
+        """
+        Check and update nonce record. If no record exists or has an invalid count,
+        return False. Create a lock to prevent a concurrent replay attack where
+        two requests are sent immediately and either may finish first.
+        """
+        cache = self._get_cache()
+        cache_key = self._generate_cache_key(user, nonce)
+
+        if nonce_count == None:  # No need to lock
+            existing = cache.get(cache_key)
+            if existing is None:
+                return False
+            cache.set(cache_key, NONCE_NO_COUNT, self._get_timeout())
+        else:
+            try:
+                with cache.lock(
+                    f'user_nonce_lock_{user}_{nonce}',
+                    timeout=self._get_timeout(),
+                    blocking_timeout=self._blocking_timeout
+                ):
+                    existing = cache.get(cache_key)
+                    if existing is None:
+                        return False
+                    if nonce_count <= existing:
+                        return False
+                    cache.set(cache_key, nonce_count, self._get_timeout())
+            except LockError:
+                cache.delete(cache_key)
+                return False
+        return True
+
+    def store_nonce(self, user, nonce, nonce_count):
+        # Nonce is required
+        if nonce is None or len(nonce) <= 1:
+            return False
+        if nonce_count is None:
+            nonce_count = NONCE_NO_COUNT
+        cache = self._get_cache()
+        cache_key = self._generate_cache_key(user, nonce)
+        return cache.set(cache_key, nonce_count, self._get_timeout())

onadata/apps/django_digest_backends/tests.py

@@ -0,0 +1,43 @@
+from django.core.cache import caches
+from django.test import TestCase
+
+from .cache import RedisCacheNonceStorage
+
+
+class TestCacheNonceStorage(TestCase):
+    def setUp(self):
+        self.test_user = 'bob'
+        self.cache = caches['default']
+        self.storage = RedisCacheNonceStorage()
+
+    def test_store_and_update(self):
+        self.storage.store_nonce(self.test_user, 'testnonce', '')
+        self.assertEqual(self.cache.get(f'user_nonce_{self.test_user}_testnonce'), '')
+
+        # Should return true if the user + nonce already exists
+        self.assertTrue(self.storage.update_existing_nonce(self.test_user, 'testnonce', None))
+
+        self.assertFalse(self.storage.update_existing_nonce(self.test_user, 'bogusnonce', None))
+        self.assertFalse(self.storage.update_existing_nonce('alice', 'testnonce', None))
+
+        self.cache.clear()
+        self.assertFalse(self.storage.update_existing_nonce(self.test_user, 'testnonce', None))
+        # update should never create
+        self.assertFalse(self.storage.update_existing_nonce(self.test_user, 'testnonce', None))
+
+    def test_update_count(self):
+        self.storage.store_nonce(self.test_user, 'testnonce', 2)
+
+        self.assertFalse(self.storage.update_existing_nonce(self.test_user, 'testnonce', 2))
+        self.assertTrue(self.storage.update_existing_nonce(self.test_user, 'testnonce', 3))
+
+    def test_nonce_lock(self):
+        """
+        Lock timeout should be considered False and delete the nonce
+        """
+        nonce = 'testnonce'
+        self.storage._blocking_timeout = 0.1
+        self.storage.store_nonce(self.test_user, nonce, 1)
+        with self.cache.lock(f'user_nonce_lock_{self.test_user}_{nonce}'):
+            self.assertFalse(self.storage.update_existing_nonce(self.test_user, nonce, 2))
+        self.assertFalse(self.cache.get(self.storage._generate_cache_key(self.test_user, nonce)))

onadata/settings/base.py

@@ -396,6 +396,13 @@ def skip_suspicious_operations(record):
     'socket_timeout': env.int('REDIS_SESSION_SOCKET_TIMEOUT', 1),
 }
 
+CACHES = {
+    # Set CACHE_URL to override. Only redis is supported.
+    'default': env.cache(default='redis://redis_cache:6380/3'),
+}
+
+DIGEST_NONCE_BACKEND = 'onadata.apps.django_digest_backends.cache.RedisCacheNonceStorage'
+
 ###################################
 # Django Rest Framework settings  #
 ###################################