Cache API keys: 2nd attempt

Pipfile

@@ -21,6 +21,8 @@ uk-election-ids = "*"
 faker = "*"
 requests = "*"
 django-apiblueprint-view = "*"
+django-debug-toolbar = "*"
+python-memcached = "*"
 
 [dev-packages]
 black = "==20.8b1"

ec_api/apps/api_proxy/auth.py

@@ -1,13 +1,16 @@
+from django.core.cache import cache
+
 from rest_framework import exceptions
 from rest_framework.authentication import TokenAuthentication
 from rest_framework.permissions import BasePermission
 
-from users.models import APIKey
+from users.models import APIKey, CustomUser
 
 
 class TokenWithGetParamAuthentication(TokenAuthentication):
     model = APIKey
     keyword = "Token"
+    invalid_string = "invalid"
 
     def authenticate(self, request):
         """
@@ -25,12 +28,40 @@ def authenticate_credentials(self, key):
         Attempt to authenticate the api key. On success returns the user and
         API key object to be added to the request.
         """
-        model = self.get_model()
-        try:
-            token = model.objects.get(key=key)
-        except model.DoesNotExist:
+
+        # First try to get the key from the cache
+        cached_user = cache.get(key)
+        if cached_user is None:
+            # We've not cached this key before. We need to see if it's valid
+            model = self.get_model()
+            try:
+                token = model.objects.get(key=key)
+                # The User is valid, so set the cache with the value being a
+                # dict with the User PK
+                cached_user = {"pk": token.user.pk}
+                cache.set(key, cached_user, 60 * 5)
+            except model.DoesNotExist:
+                # This isn't a valid key. To prevent this being a way to bruit
+                # force the database, we'll cache the key with a string that
+                # tells us it's invalid
+
+                # Cache invalid keys for 30 minutes
+                cache.set(key, self.invalid_string, 60 * 30)
+                cached_user = self.invalid_string
+
+        if cached_user == self.invalid_string:
+            # If we've seen an invalid key, fail authrntication
             raise exceptions.AuthenticationFailed("Invalid token.")
-        return (token.user, token)
+
+        # Construct a CustomUser object to return. This isn't a saved object
+        # but if the actual object is needed downstream we can either call
+        # `refresh_from_db()` on it, or add more attributes to the cached dict
+        # above
+        user_obj = CustomUser(**cached_user)
+        user_obj.auth_check_keys = False
+
+        # return the user object, and the API key
+        return user_obj, key
 
 
 class IsValidAPIUser(BasePermission):
@@ -39,7 +70,7 @@ class IsValidAPIUser(BasePermission):
     """
 
     def has_permission(self, request, view):
-        try:
-            return request.user.api_keys.exists()
-        except AttributeError:
-            return False
+        if request.user.is_authenticated:
+            # If we're authenticated, we always have permission
+            return True
+        return False

ec_api/apps/api_proxy/tests/test_auth.py

@@ -72,24 +72,44 @@ def test_authenticate_credentials_valid_key(self, mocker, api_key):
         Test user and key are returned when key is authenticated
         """
         key = mocker.MagicMock(spec=APIKey, key=api_key)
+        key.user.pk = 1
         mocker.patch.object(APIKey.objects, "get", return_value=key)
 
         auth_obj = TokenWithGetParamAuthentication()
         result = auth_obj.authenticate_credentials(key=api_key)
 
-        assert result == (key.user, key)
+        assert type(result[0]) == CustomUser
+        assert result[1] == key.key
+
+    def test_cached_auth_database_calls(
+        self, django_user_model, api_key, db, django_assert_num_queries
+    ):
+        """
+        Test user and key are returned when key is authenticated
+        """
+        user = django_user_model.objects.create()
+        api_key = APIKey.objects.create(user=user)
+
+        # The first request should cause a DB hit
+        with django_assert_num_queries(2):
+            auth_obj = TokenWithGetParamAuthentication()
+            auth_obj.authenticate_credentials(key=api_key.key)
+
+        # The second should be cached
+        with django_assert_num_queries(0):
+            auth_obj = TokenWithGetParamAuthentication()
+            auth_obj.authenticate_credentials(key=api_key.key)
 
 
 class TestIsValidAPIUser:
-    @pytest.mark.parametrize("exists", [True, False])
-    def test_has_permission(self, exists, mocker):
+    @pytest.mark.parametrize("is_authenticated", [True, False])
+    def test_has_permission(self, is_authenticated, mocker):
         request = mocker.MagicMock()
-        request.user = mocker.MagicMock(spec=CustomUser)
-        request.user.api_keys.exists.return_value = exists
+        request.user = mocker.MagicMock(
+            spec=CustomUser, is_authenticated=is_authenticated
+        )
         perm_obj = IsValidAPIUser()
-
-        assert perm_obj.has_permission(request, "view") is exists
-        request.user.api_keys.exists.assert_called_once()
+        assert perm_obj.has_permission(request, "view") is is_authenticated
 
     def test_has_permission_anonymous_user(self, mocker):
         # use anonymous user so AttributeError is raised attempting to get keys

ec_api/apps/users/migrations/0004_customuser_name.py

@@ -0,0 +1,23 @@
+# Generated by Django 2.2.24 on 2021-08-12 14:38
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("users", "0003_auto_20210305_1213"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="customuser",
+            name="name",
+            field=models.CharField(
+                default="",
+                help_text="Either your name or an organisation name",
+                max_length=255,
+            ),
+            preserve_default=False,
+        ),
+    ]

ec_api/apps/users/models.py

@@ -19,6 +19,9 @@ class CustomUser(AbstractBaseUser, PermissionsMixin):
     """
 
     email = models.EmailField(_("email address"), unique=True)
+    name = models.CharField(
+        max_length=255, help_text=_("Either your name or an organisation name")
+    )
     is_staff = models.BooleanField(
         _("staff status"),
         default=False,

ec_api/settings/base.py

@@ -45,9 +45,11 @@ def root(*path):
     "rest_framework",
     "api_docs",
     "apiblueprint_view",
+    "debug_toolbar",
 ]
 
 MIDDLEWARE = [
+    "debug_toolbar.middleware.DebugToolbarMiddleware",
     "django.middleware.security.SecurityMiddleware",
     "whitenoise.middleware.WhiteNoiseMiddleware",
     "django.contrib.sessions.middleware.SessionMiddleware",

ec_api/settings/base_lambda.py

@@ -4,6 +4,9 @@
 ALLOWED_HOSTS = ["*"]
 DEBUG = os.environ.get("DEBUG", False)
 
+WHITENOISE_AUTOREFRESH = False
+WHITENOISE_STATIC_PREFIX = "/static/"
+
 if os.environ.get("APP_IS_BEHIND_CLOUDFRONT", False) in [
     True,
     "true",
@@ -14,6 +17,7 @@
 else:
     USE_X_FORWARDED_HOST = False
     FORCE_SCRIPT_NAME = "/Prod"
+    STATIC_URL = FORCE_SCRIPT_NAME + WHITENOISE_STATIC_PREFIX
 
 DATABASES = {
     "default": {
@@ -44,4 +48,14 @@
 
 STATIC_ROOT = os.path.join(BASE_DIR, "static_files")
 
+CACHE_URL = os.environ.get("CACHE_URL", None)
+if CACHE_URL:
+    CACHES = {
+        "default": {
+            "BACKEND": "django.core.cache.backends.memcached.MemcachedCache",
+            "LOCATION": f"{CACHE_URL}:11211",
+        }
+    }
+
+
 setup_sentry()

ec_api/urls.py

@@ -1,3 +1,4 @@
+import debug_toolbar
 from django.contrib import admin
 from django.urls import include, path
 
@@ -8,4 +9,6 @@
     path("", include("frontend.urls")),
     path("docs/", include("api_docs.urls")),
     path("api/", include("api_proxy.urls")),
+    # Debug only
+    path("__debug__/", include(debug_toolbar.urls)),
 ]

samconfig.toml.d/development.toml

@@ -25,6 +25,13 @@ parameter_overrides = """
   AppDjangoSettingsModule=ec_api.settings.base_lambda \
   AppSecretKey=badf00d \
   AppIsBehindCloudFront=False \
+  AppSentryDSN='' \
+  AppSamLambdaConfigEnv=dev \
+  AppDomain='' \
+  AppPostgresDatabaseName='' \
+  AppPostgresPassword='' \
+  GitHash="0000" \
+  AppPostgresHost='' \
 """
 
 [SYM.logs]

template.yaml

@@ -69,6 +69,14 @@ Parameters:
     Type: String
 
 Resources:
+  DjangoMemcache:
+    Type: AWS::ElastiCache::CacheCluster
+    Properties:
+      Engine: memcached
+      NumCacheNodes: 1
+      CacheNodeType: cache.t3.micro
+      CacheSecurityGroupNames:
+        - default
 
   DependenciesLayer:
     Type: AWS::Serverless::LayerVersion
@@ -102,6 +110,7 @@ Resources:
           POSTGRES_PASSWORD: !Ref AppPostgresPassword
           SAM_LAMBDA_CONFIG_ENV: !Ref AppSamLambdaConfigEnv
           APP_DOMAIN: !Ref AppDomain
+          CACHE_URL: !GetAtt DjangoMemcache.ConfigurationEndpoint.Address
       Events:
         HTTPRequests:
           Type: Api