From e16de8d9e793d3d187c280453f9c3067f84c7c33 Mon Sep 17 00:00:00 2001
From: Zixuan James Li <p359101898@gmail.com>
Date: Wed, 28 Sep 2022 20:48:41 -0400
Subject: [PATCH] scim: Further slim down SCIMClient removing unused
 attributes.

This removes everything from SCIMClient except the "is_authenticated`
method. Previously, "realm" and "name" were only needed for logging
purposes. It is the best to keep SCIMClient as minimal as possible, as
it is only intended to be used for authenticating requests to SCIM
views.

This change also gurantees that the "LogRequests" middleware will not
rely on the type unsafe access of the format_requestor_for_logs method
on SCIMClient.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
---
 zerver/middleware.py      | 22 +++++-----------------
 zerver/tests/test_scim.py | 10 ++++++++++
 2 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/zerver/middleware.py b/zerver/middleware.py
index db72b3d1a9..4bc2012844 100644
--- a/zerver/middleware.py
+++ b/zerver/middleware.py
@@ -2,7 +2,6 @@ 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
 
@@ -697,21 +696,7 @@ 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:
         """
@@ -746,12 +731,15 @@ def validate_scim_bearer_token(request: HttpRequest) -> Optional[SCIMClient]:
         return None
 
     request_notes = RequestNotes.get_notes(request)
-    assert request_notes.realm
+    assert request_notes.realm is not None
+    request_notes.requestor_for_logs = (
+        f"scim-client:{scim_client_name}:realm:{request_notes.realm.id}"
+    )
 
     # 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(realm=request_notes.realm, name=scim_client_name)
+    return SCIMClient()
 
 
 class ZulipSCIMAuthCheckMiddleware(SCIMAuthCheckMiddleware):
diff --git a/zerver/tests/test_scim.py b/zerver/tests/test_scim.py
index 502a9f7e2c..a45f24c444 100644
--- a/zerver/tests/test_scim.py
+++ b/zerver/tests/test_scim.py
@@ -680,6 +680,16 @@ class TestSCIMUser(SCIMTestCase):
                 m.output, [f"ERROR:django.request:Not Implemented: /scim/v2/Users/{hamlet.id}"]
             )
 
+    def test_scim_client_requestor_for_logs(self) -> None:
+        hamlet = self.example_user("hamlet")
+        with self.assertLogs("zulip.requests", level="INFO") as m:
+            result = self.client_get(f"/scim/v2/Users/{hamlet.id}", {}, **self.scim_headers())
+        self.assertIn(
+            f"scim-client:{settings.SCIM_CONFIG['zulip']['scim_client_name']}:realm:{hamlet.realm.id}",
+            m.output[0],
+        )
+        self.assertEqual(result.status_code, 200)
+
 
 class TestSCIMGroup(SCIMTestCase):
     """