scim: Downgrade SCIMClient from a model to an ephemeral dataclass.

SCIMClient is a type-unsafe workaround for django-scim2’s conflation
of SCIM users with Django users.  Given that a SCIMClient is not a
UserProfile, it might as well not be a model at all, since it’s only
used to satisfy django-scim2’s request.user.is_authenticated queries.

This doesn’t solve the type safety issue with assigning a SCIMClient
to request.user, nor the performance issue with running the SCIM
middleware on non-SCIM requests.  But it reduces the risk of potential
consequences worse than crashing, since there’s no longer a
request.user.id for Django to confuse with the ID of an actual
UserProfile.

Signed-off-by: Anders Kaseorg <anders@zulip.com>

zerver/lib/export.py

@@ -152,7 +152,6 @@ ALL_ZULIP_TABLES = {
     "zerver_scheduledemail_users",
     "zerver_scheduledmessage",
     "zerver_scheduledmessagenotificationemail",
-    "zerver_scimclient",
     "zerver_service",
     "zerver_stream",
     "zerver_submessage",
@@ -203,8 +202,6 @@ NON_EXPORTED_TABLES = {
     "zerver_scheduledemail",
     "zerver_scheduledemail_users",
     "zerver_scheduledmessage",
-    # SCIMClient should be manually created for the new realm after importing.
-    "zerver_scimclient",
     # These tables are related to a user's 2FA authentication
     # configuration, which will need to be set up again on the new
     # server.

zerver/management/commands/add_scim_client.py

@@ -1,20 +0,0 @@
-from argparse import ArgumentParser
-from typing import Any
-
-from zerver.lib.management import ZulipBaseCommand
-from zerver.models import SCIMClient
-
-
-class Command(ZulipBaseCommand):
-    help = """Create a SCIM client entry in the database."""
-
-    def add_arguments(self, parser: ArgumentParser) -> None:
-        self.add_realm_args(parser)
-        parser.add_argument("name", help="name of the client")
-
-    def handle(self, *args: Any, **options: Any) -> None:
-        client_name = options["name"]
-        realm = self.get_realm(options)
-        assert realm
-
-        SCIMClient.objects.create(realm=realm, name=client_name)

zerver/middleware.py

@@ -2,6 +2,7 @@ import cProfile
 import logging
 import time
 import traceback
+from dataclasses import dataclass
 from typing import Any, AnyStr, Callable, Dict, Iterable, List, MutableMapping, Optional, Tuple
 from urllib.parse import urlencode
 
@@ -42,7 +43,7 @@ from zerver.lib.response import (
 from zerver.lib.subdomains import get_subdomain
 from zerver.lib.user_agent import parse_user_agent
 from zerver.lib.utils import statsd
-from zerver.models import Realm, SCIMClient, flush_per_request_caches, get_realm
+from zerver.models import Realm, flush_per_request_caches, get_realm
 
 ParamT = ParamSpec("ParamT")
 logger = logging.getLogger("zulip.requests")
@@ -696,6 +697,31 @@ class ZulipCommonMiddleware(CommonMiddleware):
         return super().should_redirect_with_slash(request)
 
 
+@dataclass
+class SCIMClient:
+    realm: Realm
+    name: str
+
+    def __str__(self) -> str:
+        return f"<SCIMClient {self.name} for realm {self.realm_id}>"
+
+    def format_requestor_for_logs(self) -> str:
+        return f"scim-client:{self.name}:realm:{self.realm_id}"
+
+    @property
+    def realm_id(self) -> int:
+        return self.realm.id
+
+    @property
+    def is_authenticated(self) -> bool:
+        """
+        The purpose of this is to make SCIMClient behave like a UserProfile
+        when an instance is assigned to request.user - we need it to pass
+        request.user.is_authenticated verifications.
+        """
+        return True
+
+
 def validate_scim_bearer_token(request: HttpRequest) -> Optional[SCIMClient]:
     """
     This function verifies the request is allowed to make SCIM requests on this subdomain,
@@ -725,7 +751,7 @@ def validate_scim_bearer_token(request: HttpRequest) -> Optional[SCIMClient]:
     # While API authentication code paths are sufficiently high
     # traffic that we prefer to use a cache, SCIM is much lower
     # traffic, and doing a database query is plenty fast.
-    return SCIMClient.objects.get(realm=request_notes.realm, name=scim_client_name)
+    return SCIMClient(realm=request_notes.realm, name=scim_client_name)
 
 
 class ZulipSCIMAuthCheckMiddleware(SCIMAuthCheckMiddleware):

zerver/migrations/0415_delete_scimclient.py

@@ -0,0 +1,16 @@
+# Generated by Django 4.0.7 on 2022-09-24 06:32
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("zerver", "0414_remove_userstatus_status"),
+    ]
+
+    operations = [
+        migrations.DeleteModel(
+            name="SCIMClient",
+        ),
+    ]

zerver/models.py

@@ -4793,26 +4793,3 @@ def flush_alert_word(*, instance: AlertWord, **kwargs: object) -> None:
 
 post_save.connect(flush_alert_word, sender=AlertWord)
 post_delete.connect(flush_alert_word, sender=AlertWord)
-
-
-class SCIMClient(models.Model):
-    realm: Realm = models.ForeignKey(Realm, on_delete=CASCADE)
-    name: str = models.TextField()
-
-    class Meta:
-        unique_together = ("realm", "name")
-
-    def __str__(self) -> str:
-        return f"<SCIMClient {self.name} for realm {self.realm_id}>"
-
-    def format_requestor_for_logs(self) -> str:
-        return f"scim-client:{self.name}:realm:{self.realm_id}"
-
-    @property
-    def is_authenticated(self) -> bool:
-        """
-        The purpose of this is to make SCIMClient behave like a UserProfile
-        when an instance is assigned to request.user - we need it to pass
-        request.user.is_authenticated verifications.
-        """
-        return True

zerver/tests/test_scim.py

@@ -8,7 +8,8 @@ from django.conf import settings
 
 from zerver.actions.user_settings import do_change_full_name
 from zerver.lib.test_classes import ZulipTestCase
-from zerver.models import SCIMClient, UserProfile, get_realm
+from zerver.middleware import SCIMClient
+from zerver.models import UserProfile, get_realm
 
 if TYPE_CHECKING:
     from django.test.client import _MonkeyPatchedWSGIResponse as TestHttpResponse
@@ -22,7 +23,7 @@ class SCIMTestCase(ZulipTestCase):
     def setUp(self) -> None:
         super().setUp()
         self.realm = get_realm("zulip")
-        self.scim_client = SCIMClient.objects.create(
+        self.scim_client = SCIMClient(
             realm=self.realm, name=settings.SCIM_CONFIG["zulip"]["scim_client_name"]
         )