Use user_profile, not email, in check_message().

We are trying to convert emails to user_profiles earlier in
the codepath.  This may cause subtle changes in which errors
appear, but it's probably generally good to report on bad
addressees sooner than later.
This commit is contained in:
Steve Howell
2017-08-17 23:01:22 -04:00
committed by Tim Abbott
parent c56a631b65
commit c61c0f3edc
2 changed files with 24 additions and 12 deletions

View File

@@ -1374,16 +1374,16 @@ def check_message(sender, client, addressee,
raise JsonableError(_("Not authorized to send to stream '%s'") % (stream.name,)) raise JsonableError(_("Not authorized to send to stream '%s'") % (stream.name,))
elif addressee.is_private(): elif addressee.is_private():
emails = addressee.emails() user_profiles = addressee.user_profiles()
if emails is None or len(emails) == 0: if user_profiles is None or len(user_profiles) == 0:
raise JsonableError(_("Message must have recipients")) raise JsonableError(_("Message must have recipients"))
mirror_message = client and client.name in ["zephyr_mirror", "irc_mirror", "jabber_mirror", "JabberMirror"] mirror_message = client and client.name in ["zephyr_mirror", "irc_mirror", "jabber_mirror", "JabberMirror"]
not_forged_mirror_message = mirror_message and not forged not_forged_mirror_message = mirror_message and not forged
try: try:
recipient = recipient_for_emails(emails, not_forged_mirror_message, recipient = recipient_for_user_profiles(user_profiles, not_forged_mirror_message,
forwarder_user_profile, sender) forwarder_user_profile, sender)
except ValidationError as e: except ValidationError as e:
assert isinstance(e.messages[0], six.string_types) assert isinstance(e.messages[0], six.string_types)
raise JsonableError(e.messages[0]) raise JsonableError(e.messages[0])

View File

@@ -5,11 +5,13 @@ from typing import Iterable, List, Optional, Sequence, Text
from django.core.exceptions import ValidationError from django.core.exceptions import ValidationError
from django.utils.translation import ugettext as _ from django.utils.translation import ugettext as _
from zerver.lib.exceptions import JsonableError
from zerver.lib.request import JsonableError from zerver.lib.request import JsonableError
from zerver.models import ( from zerver.models import (
UserProfile, UserProfile,
get_user_profile_by_email, get_user_profile_by_email,
) )
import six
def user_profiles_from_unvalidated_emails(emails): def user_profiles_from_unvalidated_emails(emails):
# type: (Iterable[Text]) -> List[UserProfile] # type: (Iterable[Text]) -> List[UserProfile]
@@ -22,6 +24,14 @@ def user_profiles_from_unvalidated_emails(emails):
user_profiles.append(user_profile) user_profiles.append(user_profile)
return user_profiles return user_profiles
def get_user_profiles(emails):
# type: (Iterable[Text]) -> List[UserProfile]
try:
return user_profiles_from_unvalidated_emails(emails)
except ValidationError as e:
assert isinstance(e.messages[0], six.string_types)
raise JsonableError(e.messages[0])
class Addressee(object): class Addressee(object):
# This is really just a holder for vars that tended to be passed # This is really just a holder for vars that tended to be passed
# around in a non-type-safe way before this class was introduced. # around in a non-type-safe way before this class was introduced.
@@ -34,11 +44,11 @@ class Addressee(object):
# in memory. # in memory.
# #
# This should be treated as an immutable class. # This should be treated as an immutable class.
def __init__(self, msg_type, emails=None, stream_name=None, topic=None): def __init__(self, msg_type, user_profiles=None, stream_name=None, topic=None):
# type: (str, Optional[Sequence[Text]], Optional[Text], Text) -> None # type: (str, Optional[Sequence[UserProfile]], Optional[Text], Text) -> None
assert(msg_type in ['stream', 'private']) assert(msg_type in ['stream', 'private'])
self._msg_type = msg_type self._msg_type = msg_type
self._emails = emails self._user_profiles = user_profiles
self._stream_name = stream_name self._stream_name = stream_name
self._topic = topic self._topic = topic
@@ -54,10 +64,10 @@ class Addressee(object):
# type: () -> bool # type: () -> bool
return self._msg_type == 'private' return self._msg_type == 'private'
def emails(self): def user_profiles(self):
# type: () -> Sequence[Text] # type: () -> List[UserProfile]
assert(self.is_private()) assert(self.is_private())
return self._emails return self._user_profiles # type: ignore # assertion protects us
def stream_name(self): def stream_name(self):
# type: () -> Text # type: () -> Text
@@ -107,15 +117,17 @@ class Addressee(object):
@staticmethod @staticmethod
def for_private(emails): def for_private(emails):
# type: (Sequence[Text]) -> Addressee # type: (Sequence[Text]) -> Addressee
user_profiles = get_user_profiles(emails)
return Addressee( return Addressee(
msg_type='private', msg_type='private',
emails=emails, user_profiles=user_profiles,
) )
@staticmethod @staticmethod
def for_email(email): def for_email(email):
# type: (Text) -> Addressee # type: (Text) -> Addressee
user_profiles = get_user_profiles([email])
return Addressee( return Addressee(
msg_type='private', msg_type='private',
emails=[email], user_profiles=user_profiles,
) )