refactor: Add get_stream_by_narrow_operand_access_unchecked.

This let's us clean up the linter that excludes the use of get_stream
and by adding the access_unchecked in the name we make it clear that
it should be used with caution.

Refactoring idea by Tim Abbott.

tools/linter_lib/custom_check.py

@@ -395,10 +395,6 @@ python_rules = RuleList(
              # how most instances are written, but better to exclude something than nothing
              ('zerver/lib/actions.py', 'stream = get_stream(stream_name, realm)'),
              ('zerver/lib/actions.py', 'get_stream(admin_realm_signup_notifications_stream, admin_realm)'),
-             # Here we need get_stream to access streams you've since unsubscribed from.
-             ('zerver/views/messages.py', 'stream = get_stream(operand, self.user_profile.realm)'),
-             # Use stream_id to exclude mutes.
-             ('zerver/views/messages.py', 'stream_id = get_stream(stream_name, user_profile.realm).id'),
          ]),
          'description': 'Please use access_stream_by_*() to fetch Stream objects',
          },

zerver/lib/streams.py

@@ -1,4 +1,4 @@
-from typing import Any, Iterable, List, Mapping, Set, Tuple, Optional
+from typing import Any, Iterable, List, Mapping, Set, Tuple, Optional, Union
 
 from django.utils.translation import ugettext as _
 
@@ -6,7 +6,7 @@ from zerver.lib.actions import check_stream_name, create_streams_if_needed
 from zerver.lib.request import JsonableError
 from zerver.models import UserProfile, Stream, Subscription, \
     Realm, Recipient, bulk_get_recipients, get_stream_recipient, get_stream, \
-    bulk_get_streams, get_realm_stream, DefaultStreamGroup
+    bulk_get_streams, get_realm_stream, DefaultStreamGroup, get_stream_by_id_in_realm
 
 def check_for_exactly_one_stream_arg(stream_id: Optional[int], stream: Optional[str]) -> None:
     if stream_id is None and stream is None:
@@ -282,3 +282,12 @@ def access_default_stream_group_by_id(realm: Realm, group_id: int) -> DefaultStr
         return DefaultStreamGroup.objects.get(realm=realm, id=group_id)
     except DefaultStreamGroup.DoesNotExist:
         raise JsonableError(_("Default stream group with id '%s' does not exist.") % (group_id,))
+
+def get_stream_by_narrow_operand_access_unchecked(operand: Union[str, int], realm: Realm) -> Stream:
+    """This is required over access_stream_* in certain cases where
+    we need the stream data only to prepare a response that user can access
+    and not send it out to unauthorized recipients.
+    """
+    if isinstance(operand, str):
+        return get_stream(operand, realm)
+    return get_stream_by_id_in_realm(operand, realm)

zerver/views/messages.py

@@ -30,7 +30,8 @@ from zerver.lib.message import (
 )
 from zerver.lib.response import json_success, json_error
 from zerver.lib.sqlalchemy_utils import get_sqlalchemy_connection
-from zerver.lib.streams import access_stream_by_id, can_access_stream_history_by_name
+from zerver.lib.streams import access_stream_by_id, can_access_stream_history_by_name, \
+    get_stream_by_narrow_operand_access_unchecked
 from zerver.lib.timestamp import datetime_to_timestamp, convert_to_UTC
 from zerver.lib.timezone import get_timezone
 from zerver.lib.topic import (
@@ -50,8 +51,8 @@ from zerver.lib.validator import \
 from zerver.lib.zephyr import compute_mit_user_fullname
 from zerver.models import Message, UserProfile, Stream, Subscription, Client,\
     Realm, RealmDomain, Recipient, UserMessage, bulk_get_recipients, get_personal_recipient, \
-    get_stream, email_to_domain, get_realm, get_active_streams, \
-    get_user_including_cross_realm, get_user_by_id_in_realm_including_cross_realm, get_stream_recipient
+    email_to_domain, get_realm, get_active_streams, get_user_including_cross_realm, \
+    get_user_by_id_in_realm_including_cross_realm, get_stream_recipient
 
 from sqlalchemy import func
 from sqlalchemy.sql import select, join, column, literal_column, literal, and_, \
@@ -207,8 +208,8 @@ class NarrowBuilder:
         try:
             # Because you can see your own message history for
             # private streams you are no longer subscribed to, we
-            # need get_stream, not access_stream, here.
-            stream = get_stream(operand, self.user_profile.realm)
+            # need get_stream_by_narrow_operand_access_unchecked here.
+            stream = get_stream_by_narrow_operand_access_unchecked(operand, self.user_profile.realm)
         except Stream.DoesNotExist:
             raise BadNarrowOperator('unknown stream ' + operand)
 
@@ -593,11 +594,11 @@ def exclude_muting_conditions(user_profile: UserProfile,
     stream_id = None
     if stream_name is not None:
         try:
-            # Note that this code works around a lint rule that
-            # says we should use access_stream_by_name to get the
-            # stream.  It is okay here, because we are only using
-            # the stream id to exclude data, not to include results.
-            stream_id = get_stream(stream_name, user_profile.realm).id
+            # Note: It is okay here to not check access to stream
+            # because we are only using the stream id to exclude data,
+            # not to include results.
+            stream = get_stream_by_narrow_operand_access_unchecked(stream_name, user_profile.realm)
+            stream_id = stream.id
         except Stream.DoesNotExist:
             pass