validate_user_access_to_subscribers: Avoid unchecked cast.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
This commit is contained in:
Anders Kaseorg 2020-06-22 15:33:46 -07:00 committed by Tim Abbott
parent 5ac411aeaa
commit 5733a1bcd4
2 changed files with 15 additions and 11 deletions

View File

@ -20,7 +20,6 @@ from typing import (
Set, Set,
Tuple, Tuple,
Union, Union,
cast,
) )
import django.db.utils import django.db.utils
@ -2524,11 +2523,13 @@ def validate_user_access_to_subscribers(user_profile: Optional[UserProfile],
"invite_only": stream.invite_only}, "invite_only": stream.invite_only},
# We use a lambda here so that we only compute whether the # We use a lambda here so that we only compute whether the
# user is subscribed if we have to # user is subscribed if we have to
lambda: subscribed_to_stream(cast(UserProfile, user_profile), stream.id)) lambda user_profile: subscribed_to_stream(user_profile, stream.id))
def validate_user_access_to_subscribers_helper(user_profile: Optional[UserProfile], def validate_user_access_to_subscribers_helper(
stream_dict: Mapping[str, Any], user_profile: Optional[UserProfile],
check_user_subscribed: Callable[[], bool]) -> None: stream_dict: Mapping[str, Any],
check_user_subscribed: Callable[[UserProfile], bool],
) -> None:
"""Helper for validate_user_access_to_subscribers that doesn't require """Helper for validate_user_access_to_subscribers that doesn't require
a full stream object. This function is a bit hard to read, a full stream object. This function is a bit hard to read,
because it is carefully optimized for performance in the two code because it is carefully optimized for performance in the two code
@ -2556,7 +2557,7 @@ def validate_user_access_to_subscribers_helper(user_profile: Optional[UserProfil
# Guest users can access subscribed public stream's subscribers # Guest users can access subscribed public stream's subscribers
if user_profile.is_guest: if user_profile.is_guest:
if check_user_subscribed(): if check_user_subscribed(user_profile):
return return
# We could put an AssertionError here; in that we don't have # We could put an AssertionError here; in that we don't have
# any code paths that would allow a guest user to access other # any code paths that would allow a guest user to access other
@ -2569,7 +2570,7 @@ def validate_user_access_to_subscribers_helper(user_profile: Optional[UserProfil
if user_profile.is_realm_admin: if user_profile.is_realm_admin:
return return
if (stream_dict["invite_only"] and not check_user_subscribed()): if (stream_dict["invite_only"] and not check_user_subscribed(user_profile)):
raise JsonableError(_("Unable to retrieve subscribers for private stream")) raise JsonableError(_("Unable to retrieve subscribers for private stream"))
def bulk_get_subscriber_user_ids(stream_dicts: Iterable[Mapping[str, Any]], def bulk_get_subscriber_user_ids(stream_dicts: Iterable[Mapping[str, Any]],
@ -2582,8 +2583,11 @@ def bulk_get_subscriber_user_ids(stream_dicts: Iterable[Mapping[str, Any]],
stream_recipient.populate_with(stream_id=stream_dict["id"], stream_recipient.populate_with(stream_id=stream_dict["id"],
recipient_id=stream_dict["recipient_id"]) recipient_id=stream_dict["recipient_id"])
try: try:
validate_user_access_to_subscribers_helper(user_profile, stream_dict, validate_user_access_to_subscribers_helper(
lambda: sub_dict[stream_dict["id"]]) user_profile,
stream_dict,
lambda user_profile: sub_dict[stream_dict["id"]],
)
except JsonableError: except JsonableError:
continue continue
target_stream_dicts.append(stream_dict) target_stream_dicts.append(stream_dict)

View File

@ -3417,11 +3417,11 @@ class SubscriptionAPITest(ZulipTestCase):
# This should result in missing user # This should result in missing user
with self.assertRaises(ValidationError): with self.assertRaises(ValidationError):
validate_user_access_to_subscribers_helper(None, stream_dict, lambda: True) validate_user_access_to_subscribers_helper(None, stream_dict, lambda user_profile: True)
# This should result in user not in realm # This should result in user not in realm
with self.assertRaises(ValidationError): with self.assertRaises(ValidationError):
validate_user_access_to_subscribers_helper(user_profile, stream_dict, lambda: True) validate_user_access_to_subscribers_helper(user_profile, stream_dict, lambda user_profile: True)
def test_subscriptions_query_count(self) -> None: def test_subscriptions_query_count(self) -> None:
""" """