users: Remove weird get_all_api_keys helper.

This implied by its name that users could have more than one key.
They cannot, currently; make the code clearer by switching to the
explicit column access.
This commit is contained in:
Alex Vandiver
2025-02-11 21:42:38 +00:00
committed by Tim Abbott
parent c29e11de93
commit 8804c1afaa
5 changed files with 38 additions and 38 deletions

View File

@@ -511,14 +511,13 @@ def bot_dicts_in_realm_cache_key(realm_id: int) -> str:
def delete_user_profile_caches(user_profiles: Iterable["UserProfile"], realm_id: int) -> None: def delete_user_profile_caches(user_profiles: Iterable["UserProfile"], realm_id: int) -> None:
# Imported here to avoid cyclic dependency. # Imported here to avoid cyclic dependency.
from zerver.lib.users import get_all_api_keys
from zerver.models.users import is_cross_realm_bot_email from zerver.models.users import is_cross_realm_bot_email
keys = [] keys = []
for user_profile in user_profiles: for user_profile in user_profiles:
keys.append(user_profile_by_id_cache_key(user_profile.id)) keys.append(user_profile_by_id_cache_key(user_profile.id))
keys.append(user_profile_narrow_by_id_cache_key(user_profile.id)) keys.append(user_profile_narrow_by_id_cache_key(user_profile.id))
keys += map(user_profile_by_api_key_cache_key, get_all_api_keys(user_profile)) keys.append(user_profile_by_api_key_cache_key(user_profile.api_key))
keys.append(user_profile_cache_key_id(user_profile.email, realm_id)) keys.append(user_profile_cache_key_id(user_profile.email, realm_id))
keys.append(user_profile_delivery_email_cache_key(user_profile.delivery_email, realm_id)) keys.append(user_profile_delivery_email_cache_key(user_profile.delivery_email, realm_id))
if user_profile.is_bot and is_cross_realm_bot_email(user_profile.email): if user_profile.is_bot and is_cross_realm_bot_email(user_profile.email):

View File

@@ -23,7 +23,6 @@ from zerver.lib.cache import (
) )
from zerver.lib.safe_session_cached_db import SessionStore from zerver.lib.safe_session_cached_db import SessionStore
from zerver.lib.sessions import session_engine from zerver.lib.sessions import session_engine
from zerver.lib.users import get_all_api_keys
from zerver.models import Client, UserProfile from zerver.models import Client, UserProfile
from zerver.models.clients import get_client_cache_key from zerver.models.clients import get_client_cache_key
from zerver.models.users import base_get_user_queryset from zerver.models.users import base_get_user_queryset
@@ -36,8 +35,9 @@ def get_users() -> QuerySet[UserProfile]:
def user_cache_items( def user_cache_items(
items_for_remote_cache: dict[str, tuple[UserProfile]], user_profile: UserProfile items_for_remote_cache: dict[str, tuple[UserProfile]], user_profile: UserProfile
) -> None: ) -> None:
for api_key in get_all_api_keys(user_profile): items_for_remote_cache[user_profile_by_api_key_cache_key(user_profile.api_key)] = (
items_for_remote_cache[user_profile_by_api_key_cache_key(api_key)] = (user_profile,) user_profile,
)
items_for_remote_cache[user_profile_cache_key_id(user_profile.email, user_profile.realm_id)] = ( items_for_remote_cache[user_profile_cache_key_id(user_profile.email, user_profile.realm_id)] = (
user_profile, user_profile,
) )

View File

@@ -501,11 +501,6 @@ def get_api_key(user_profile: UserProfile) -> str:
return user_profile.api_key return user_profile.api_key
def get_all_api_keys(user_profile: UserProfile) -> list[str]:
# Users can only have one API key for now
return [user_profile.api_key]
def validate_user_custom_profile_field( def validate_user_custom_profile_field(
realm_id: int, field: CustomProfileField, value: ProfileDataElementValue realm_id: int, field: CustomProfileField, value: ProfileDataElementValue
) -> ProfileDataElementValue: ) -> ProfileDataElementValue:

View File

@@ -83,7 +83,7 @@ from zerver.lib.test_helpers import (
from zerver.lib.thumbnail import DEFAULT_AVATAR_SIZE, MEDIUM_AVATAR_SIZE, resize_avatar from zerver.lib.thumbnail import DEFAULT_AVATAR_SIZE, MEDIUM_AVATAR_SIZE, resize_avatar
from zerver.lib.types import Validator from zerver.lib.types import Validator
from zerver.lib.user_groups import is_user_in_group from zerver.lib.user_groups import is_user_in_group
from zerver.lib.users import get_all_api_keys, get_api_key, get_users_for_api from zerver.lib.users import get_api_key, get_users_for_api
from zerver.lib.utils import assert_is_not_none from zerver.lib.utils import assert_is_not_none
from zerver.lib.validator import ( from zerver.lib.validator import (
check_bool, check_bool,
@@ -1289,8 +1289,10 @@ class SocialAuthBase(DesktopFlowTestingLib, ZulipTestCase, ABC):
self.assertEqual(query_params["user_id"], [str(hamlet.id)]) self.assertEqual(query_params["user_id"], [str(hamlet.id)])
encrypted_api_key = query_params["otp_encrypted_api_key"][0] encrypted_api_key = query_params["otp_encrypted_api_key"][0]
hamlet_api_keys = get_all_api_keys(self.example_user("hamlet")) self.assertEqual(
self.assertIn(otp_decrypt_api_key(encrypted_api_key, mobile_flow_otp), hamlet_api_keys) otp_decrypt_api_key(encrypted_api_key, mobile_flow_otp),
self.example_user("hamlet").api_key,
)
self.assert_length(mail.outbox, 1) self.assert_length(mail.outbox, 1)
self.assertIn("Zulip on Android", mail.outbox[0].body) self.assertIn("Zulip on Android", mail.outbox[0].body)
@@ -1452,8 +1454,10 @@ class SocialAuthBase(DesktopFlowTestingLib, ZulipTestCase, ABC):
self.assertEqual(query_params["realm"], ["http://zulip.testserver"]) self.assertEqual(query_params["realm"], ["http://zulip.testserver"])
self.assertEqual(query_params["email"], [email]) self.assertEqual(query_params["email"], [email])
encrypted_api_key = query_params["otp_encrypted_api_key"][0] encrypted_api_key = query_params["otp_encrypted_api_key"][0]
user_api_keys = get_all_api_keys(get_user_by_delivery_email(email, realm)) self.assertIn(
self.assertIn(otp_decrypt_api_key(encrypted_api_key, mobile_flow_otp), user_api_keys) otp_decrypt_api_key(encrypted_api_key, mobile_flow_otp),
get_user_by_delivery_email(email, realm).api_key,
)
return return
elif desktop_flow_otp: elif desktop_flow_otp:
self.verify_desktop_flow_end_page(result, email, desktop_flow_otp) self.verify_desktop_flow_end_page(result, email, desktop_flow_otp)
@@ -4660,8 +4664,10 @@ class GoogleAuthBackendTest(SocialAuthBase):
self.assertEqual(query_params["realm"], ["http://zulip-mobile.testserver"]) self.assertEqual(query_params["realm"], ["http://zulip-mobile.testserver"])
self.assertEqual(query_params["email"], [self.example_email("hamlet")]) self.assertEqual(query_params["email"], [self.example_email("hamlet")])
encrypted_api_key = query_params["otp_encrypted_api_key"][0] encrypted_api_key = query_params["otp_encrypted_api_key"][0]
hamlet_api_keys = get_all_api_keys(self.example_user("hamlet")) self.assertEqual(
self.assertIn(otp_decrypt_api_key(encrypted_api_key, mobile_flow_otp), hamlet_api_keys) otp_decrypt_api_key(encrypted_api_key, mobile_flow_otp),
self.example_user("hamlet").api_key,
)
def test_social_auth_mobile_success_legacy_url(self) -> None: def test_social_auth_mobile_success_legacy_url(self) -> None:
mobile_flow_otp = "1234abcd" * 8 mobile_flow_otp = "1234abcd" * 8
@@ -4705,8 +4711,10 @@ class GoogleAuthBackendTest(SocialAuthBase):
self.assertEqual(query_params["realm"], ["http://zulip.testserver"]) self.assertEqual(query_params["realm"], ["http://zulip.testserver"])
self.assertEqual(query_params["email"], [self.example_email("hamlet")]) self.assertEqual(query_params["email"], [self.example_email("hamlet")])
encrypted_api_key = query_params["otp_encrypted_api_key"][0] encrypted_api_key = query_params["otp_encrypted_api_key"][0]
hamlet_api_keys = get_all_api_keys(self.example_user("hamlet")) self.assertIn(
self.assertIn(otp_decrypt_api_key(encrypted_api_key, mobile_flow_otp), hamlet_api_keys) otp_decrypt_api_key(encrypted_api_key, mobile_flow_otp),
self.example_user("hamlet").api_key,
)
self.assert_length(mail.outbox, 1) self.assert_length(mail.outbox, 1)
self.assertIn("Zulip on Android", mail.outbox[0].body) self.assertIn("Zulip on Android", mail.outbox[0].body)
@@ -5244,8 +5252,7 @@ class DevFetchAPIKeyTest(ZulipTestCase):
data = self.assert_json_success(result) data = self.assert_json_success(result)
self.assertEqual(data["email"], self.email) self.assertEqual(data["email"], self.email)
self.assertEqual(data["user_id"], self.user_profile.id) self.assertEqual(data["user_id"], self.user_profile.id)
user_api_keys = get_all_api_keys(self.user_profile) self.assertEqual(data["api_key"], self.user_profile.api_key)
self.assertIn(data["api_key"], user_api_keys)
def test_invalid_email(self) -> None: def test_invalid_email(self) -> None:
email = "hamlet" email = "hamlet"
@@ -5936,8 +5943,10 @@ class TestZulipRemoteUserBackend(DesktopFlowTestingLib, ZulipTestCase):
self.assertEqual(query_params["realm"], ["http://zulip.testserver"]) self.assertEqual(query_params["realm"], ["http://zulip.testserver"])
self.assertEqual(query_params["email"], [self.example_email("hamlet")]) self.assertEqual(query_params["email"], [self.example_email("hamlet")])
encrypted_api_key = query_params["otp_encrypted_api_key"][0] encrypted_api_key = query_params["otp_encrypted_api_key"][0]
hamlet_api_keys = get_all_api_keys(self.example_user("hamlet")) self.assertIn(
self.assertIn(otp_decrypt_api_key(encrypted_api_key, mobile_flow_otp), hamlet_api_keys) otp_decrypt_api_key(encrypted_api_key, mobile_flow_otp),
self.example_user("hamlet").api_key,
)
self.assert_length(mail.outbox, 1) self.assert_length(mail.outbox, 1)
self.assertIn("Zulip on Android", mail.outbox[0].body) self.assertIn("Zulip on Android", mail.outbox[0].body)
@@ -5990,8 +5999,10 @@ class TestZulipRemoteUserBackend(DesktopFlowTestingLib, ZulipTestCase):
self.assertEqual(query_params["realm"], ["http://zulip.testserver"]) self.assertEqual(query_params["realm"], ["http://zulip.testserver"])
self.assertEqual(query_params["email"], [self.example_email("hamlet")]) self.assertEqual(query_params["email"], [self.example_email("hamlet")])
encrypted_api_key = query_params["otp_encrypted_api_key"][0] encrypted_api_key = query_params["otp_encrypted_api_key"][0]
hamlet_api_keys = get_all_api_keys(self.example_user("hamlet")) self.assertIn(
self.assertIn(otp_decrypt_api_key(encrypted_api_key, mobile_flow_otp), hamlet_api_keys) otp_decrypt_api_key(encrypted_api_key, mobile_flow_otp),
self.example_user("hamlet").api_key,
)
self.assert_length(mail.outbox, 1) self.assert_length(mail.outbox, 1)
self.assertIn("Zulip on Android", mail.outbox[0].body) self.assertIn("Zulip on Android", mail.outbox[0].body)

View File

@@ -10,7 +10,6 @@ from django.test import override_settings
from zerver.lib.initial_password import initial_password from zerver.lib.initial_password import initial_password
from zerver.lib.test_classes import ZulipTestCase from zerver.lib.test_classes import ZulipTestCase
from zerver.lib.test_helpers import get_test_image_file, ratelimit_rule from zerver.lib.test_helpers import get_test_image_file, ratelimit_rule
from zerver.lib.users import get_all_api_keys
from zerver.models import Draft, ScheduledMessageNotificationEmail, UserProfile from zerver.models import Draft, ScheduledMessageNotificationEmail, UserProfile
from zerver.models.scheduled_jobs import NotificationTriggers from zerver.models.scheduled_jobs import NotificationTriggers
from zerver.models.users import get_user_profile_by_api_key from zerver.models.users import get_user_profile_by_api_key
@@ -683,11 +682,9 @@ class UserChangesTest(ZulipTestCase):
email = user.email email = user.email
self.login_user(user) self.login_user(user)
old_api_keys = get_all_api_keys(user) # Ensure the old API key is in the authentication cache, so
# Ensure the old API keys are in the authentication cache, so
# that the below logic can test whether we have a cache-flushing bug. # that the below logic can test whether we have a cache-flushing bug.
for api_key in old_api_keys: self.assertEqual(get_user_profile_by_api_key(user.api_key).email, email)
self.assertEqual(get_user_profile_by_api_key(api_key).email, email)
# First verify this endpoint is not registered in the /json/... path # First verify this endpoint is not registered in the /json/... path
# to prevent access with only a session. # to prevent access with only a session.
@@ -699,19 +696,17 @@ class UserChangesTest(ZulipTestCase):
result = self.client_post("/api/v1/users/me/api_key/regenerate") result = self.client_post("/api/v1/users/me/api_key/regenerate")
self.assertEqual(result.status_code, 401) self.assertEqual(result.status_code, 401)
old_api_key = user.api_key
result = self.api_post(user, "/api/v1/users/me/api_key/regenerate") result = self.api_post(user, "/api/v1/users/me/api_key/regenerate")
new_api_key = self.assert_json_success(result)["api_key"] new_api_key = self.assert_json_success(result)["api_key"]
self.assertNotIn(new_api_key, old_api_keys) self.assertNotEqual(new_api_key, old_api_key)
user = self.example_user("hamlet") user = self.example_user("hamlet")
current_api_keys = get_all_api_keys(user) self.assertEqual(new_api_key, user.api_key)
self.assertIn(new_api_key, current_api_keys)
for api_key in old_api_keys: with self.assertRaises(UserProfile.DoesNotExist):
with self.assertRaises(UserProfile.DoesNotExist): get_user_profile_by_api_key(old_api_key)
get_user_profile_by_api_key(api_key)
for api_key in current_api_keys: self.assertEqual(get_user_profile_by_api_key(user.api_key).email, email)
self.assertEqual(get_user_profile_by_api_key(api_key).email, email)
class UserDraftSettingsTests(ZulipTestCase): class UserDraftSettingsTests(ZulipTestCase):