Use hashed keys for safe cache keys (#1662)

* Use hashed keys for safe cache keys

Prevents maximum recursion depth error.

Fixes #1661

* Update tests to use safe_key() for cache keys

onadata/apps/api/tests/viewsets/test_connect_viewset.py

@@ -22,6 +22,7 @@
 from onadata.apps.api.viewsets.project_viewset import ProjectViewSet
 from onadata.libs.authentication import DigestAuthentication
 from onadata.libs.serializers.project_serializer import ProjectSerializer
+from onadata.libs.utils.cache_tools import safe_key
 
 
 class TestConnectViewSet(TestAbstractViewSet):
@@ -369,10 +370,10 @@ def test_login_attempts(self, send_account_lockout_email):
             authentication_classes=(DigestAuthentication,))
         auth = DigestAuth('bob', 'bob')
         # clear cache
-        cache.delete('login_attempts-bob')
-        cache.delete('lockout_user-bob')
-        self.assertIsNone(cache.get('login_attempts-bob'))
-        self.assertIsNone(cache.get('lockout_user-bob'))
+        cache.delete(safe_key("login_attempts-bob"))
+        cache.delete(safe_key("lockout_user-bob"))
+        self.assertIsNone(cache.get(safe_key('login_attempts-bob')))
+        self.assertIsNone(cache.get(safe_key('lockout_user-bob')))
 
         request = self._get_request_session_with_auth(view, auth)
 
@@ -383,7 +384,7 @@ def test_login_attempts(self, send_account_lockout_email):
                          u"Invalid username/password. For security reasons, "
                          u"after 9 more failed login attempts you'll have to "
                          u"wait 30 minutes before trying again.")
-        self.assertEqual(cache.get('login_attempts-bob'), 1)
+        self.assertEqual(cache.get(safe_key('login_attempts-bob')), 1)
 
         # cache value increments with subsequent attempts
         response = view(request)
@@ -392,30 +393,30 @@ def test_login_attempts(self, send_account_lockout_email):
                          u"Invalid username/password. For security reasons, "
                          u"after 8 more failed login attempts you'll have to "
                          u"wait 30 minutes before trying again.")
-        self.assertEqual(cache.get('login_attempts-bob'), 2)
+        self.assertEqual(cache.get(safe_key('login_attempts-bob')), 2)
 
         # login_attempts doesn't increase with correct login
         auth = DigestAuth('bob', 'bobbob')
         request = self._get_request_session_with_auth(view, auth)
         response = view(request)
         self.assertEqual(response.status_code, 200)
-        self.assertEqual(cache.get('login_attempts-bob'), 2)
+        self.assertEqual(cache.get(safe_key('login_attempts-bob')), 2)
 
         # lockout_user cache created upon fifth attempt
         auth = DigestAuth('bob', 'bob')
         request = self._get_request_session_with_auth(view, auth)
         self.assertFalse(send_account_lockout_email.called)
-        cache.set('login_attempts-bob', 9)
-        self.assertIsNone(cache.get('lockout_user-bob'))
+        cache.set(safe_key('login_attempts-bob'), 9)
+        self.assertIsNone(cache.get(safe_key('lockout_user-bob')))
         response = view(request)
         self.assertEqual(response.status_code, 401)
         self.assertEqual(response.data['detail'],
                          u"Locked out. Too many wrong username/password "
                          u"attempts. Try again in 30 minutes.")
-        self.assertEqual(cache.get('login_attempts-bob'), 10)
-        self.assertIsNotNone(cache.get('lockout_user-bob'))
+        self.assertEqual(cache.get(safe_key('login_attempts-bob')), 10)
+        self.assertIsNotNone(cache.get(safe_key('lockout_user-bob')))
         lockout = datetime.strptime(
-            cache.get('lockout_user-bob'), '%Y-%m-%dT%H:%M:%S')
+            cache.get(safe_key('lockout_user-bob')), '%Y-%m-%dT%H:%M:%S')
         self.assertIsInstance(lockout, datetime)
 
         # email sent upon limit being reached with right arguments
@@ -436,5 +437,5 @@ def test_login_attempts(self, send_account_lockout_email):
                          u"Locked out. Too many wrong username/password "
                          u"attempts. Try again in 30 minutes.")
         # clear cache
-        cache.delete('login_attempts-bob')
-        cache.delete('lockout_user-bob')
+        cache.delete(safe_key("login_attempts-bob"))
+        cache.delete(safe_key("lockout_user-bob"))

onadata/libs/tests/utils/test_cache_tools.py

@@ -11,7 +11,7 @@ class TestCacheTools(TestCase):
     """Test onadata.libs.utils.cache_tools module class"""
 
     def test_safe_key(self):
-        """Test safe_key() functions returns a valid key"""
-        self.assertEqual(safe_key("hello_world"), "hello_world")
-        self.assertEqual(safe_key("hello world"), "hello-world")
-        self.assertEqual(safe_key("hello@world"), "hello@world")
+        """Test safe_key() function returns a hashed key"""
+        self.assertEqual(
+            safe_key("hello world"),
+            "b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9")

onadata/libs/utils/cache_tools.py

@@ -1,6 +1,7 @@
-import re
+import hashlib
 
 from django.core.cache import cache
+from django.utils.encoding import force_bytes
 
 # Cache names used in project serializer
 PROJ_PERM_CACHE = "ps-project_permissions-"
@@ -31,13 +32,10 @@
 
 
 def safe_delete(key):
-    cache.get(key) and cache.delete(key)
+    """Safely deletes a given key from the cache."""
+    _ = cache.get(key) and cache.delete(key)
 
 
 def safe_key(key):
-    """Return a valid cache key"""
-    valid_key_chars_re = re.compile("[\x21-\x7e\x80-\xff]+$")
-    if not valid_key_chars_re.match(key):
-        return safe_key(key.replace(" ", "-"))
-
-    return key
+    """Return a hashed key."""
+    return hashlib.sha256(force_bytes(key)).hexdigest()