diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index c2ba6853a..da1833082 100644 --- a/.github/workflows/pytest.yml +++ b/.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 diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 4ff30f879..078d2509c 100644 --- a/.gitlab-ci.yml +++ b/.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 diff --git a/dependencies/pip/dev.txt b/dependencies/pip/dev.txt index 8d9dceacd..3a2f41708 100644 --- a/dependencies/pip/dev.txt +++ b/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 diff --git a/dependencies/pip/prod.txt b/dependencies/pip/prod.txt index 9035e3ff9..1df0c277d 100644 --- a/dependencies/pip/prod.txt +++ b/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 diff --git a/dependencies/pip/requirements.in b/dependencies/pip/requirements.in index 47f236760..1a3de8bd7 100644 --- a/dependencies/pip/requirements.in +++ b/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 diff --git a/dependencies/pip/requirements.txt b/dependencies/pip/requirements.txt index ade94e7d2..f53b39905 100644 --- a/dependencies/pip/requirements.txt +++ b/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 diff --git a/onadata/apps/django_digest_backends/__init__.py b/onadata/apps/django_digest_backends/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/onadata/apps/django_digest_backends/cache.py b/onadata/apps/django_digest_backends/cache.py new file mode 100644 index 000000000..619b75c35 --- /dev/null +++ b/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()) diff --git a/onadata/apps/django_digest_backends/tests.py b/onadata/apps/django_digest_backends/tests.py new file mode 100644 index 000000000..3157e84c4 --- /dev/null +++ b/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))) diff --git a/onadata/settings/base.py b/onadata/settings/base.py index 64b153ea4..a3d6c1646 100644 --- a/onadata/settings/base.py +++ b/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 # ###################################