Simplify/fix cross-realm validation in recipient_for_emails().

We now simply exclude all cross-realm bots from the set of emails
under consideration, and then if the remaining emails are all in
the same realm, we're good.

This fix changes two behaviors:
* You can no longer send a PM to an ordinary user in another realm
  by piggy-backing a cross-realm bot on to the message.  (This was
  basically a bug, but it would never manifest under current
  configurations.)
* You will be able to send PMs to multiple cross-realm bots at once.
  (This was an arbitrary restriction.  We don't really care about this
  scenario much yet, and it fell out of the new implementation.)
This commit is contained in:
Steve Howell
2016-11-01 16:51:11 -07:00
committed by Tim Abbott
parent 497b8e8bc4
commit 6659664e59
2 changed files with 21 additions and 25 deletions

View File

@@ -909,10 +909,13 @@ def recipient_for_emails(emails, not_forged_mirror_message,
user_profile, sender):
# type: (Iterable[text_type], bool, UserProfile, UserProfile) -> Recipient
recipient_profile_ids = set()
normalized_emails = set()
# We exempt cross-realm bots from the check that all the recipients
# are in the same domain.
realm_domains = set()
normalized_emails.add(sender.email)
realm_domains.add(sender.realm.domain)
exempt_emails = get_cross_realm_users()
if sender.email not in exempt_emails:
realm_domains.add(sender.realm.domain)
for email in emails:
try:
@@ -923,21 +926,13 @@ def recipient_for_emails(emails, not_forged_mirror_message,
user_profile.realm.deactivated:
raise ValidationError(_("'%s' is no longer using Zulip.") % (email,))
recipient_profile_ids.add(user_profile.id)
normalized_emails.add(user_profile.email)
realm_domains.add(user_profile.realm.domain)
if email not in exempt_emails:
realm_domains.add(user_profile.realm.domain)
if not_forged_mirror_message and user_profile.id not in recipient_profile_ids:
raise ValidationError(_("User not authorized for this query"))
# Prevent cross realm private messages unless it is between only two realms
# and one of users is a cross-realm user
if len(realm_domains) == 2:
# get_cross_realm_users does database queries; We assume that
# cross-realm PMs with the "admin realm" are rare, and
# therefore can be slower
if not (normalized_emails & get_cross_realm_users()):
raise ValidationError(_("You can't send private messages outside of your organization."))
if len(realm_domains) > 2:
if len(realm_domains) > 1:
raise ValidationError(_("You can't send private messages outside of your organization."))
# If the private message is just between the sender and

View File

@@ -112,7 +112,7 @@ class TestCrossRealmPMs(ZulipTestCase):
user1 = self.create_user(user1_email)
user1a = self.create_user(user1a_email)
user2 = self.create_user(user2_email)
user3 = self.create_user(user3_email)
self.create_user(user3_email)
feedback_bot = self.create_user(feedback_email)
support_bot = self.create_user(support_email)
@@ -138,19 +138,20 @@ class TestCrossRealmPMs(ZulipTestCase):
self.send_message(user1_email, [support_email], Recipient.PERSONAL)
assert_message_received(support_bot, user1)
# We have a loophole where I can send PMs to other users as long
# as I copy a cross-realm bot from the same realm. In practice this
# not a bug, since our only cross-realm bots are on the zulip.com
# realm.
self.send_message(user1_email, [user3_email, support_email], Recipient.PERSONAL)
assert_message_received(user3, user1)
# Allow sending PMs to two different cross-realm bots simultaneously.
# (We don't particularly need this feature, but since users can
# already individually send PMs to cross-realm bots, we shouldn't
# prevent them from sending multiple bots at once. We may revisit
# this if it's a nuisance for huddles.)
self.send_message(user1_email, [feedback_email, support_email],
Recipient.PERSONAL)
assert_message_received(feedback_bot, user1)
assert_message_received(support_bot, user1)
# Users can't email two cross-realm bots at once. (This is just
# an anomaly of the current implementation.)
# Prevent old loophole where I could send PMs to other users as long
# as I copied a cross-realm bot from the same realm.
with assert_disallowed():
self.send_message(user1_email, [feedback_email, support_email],
Recipient.PERSONAL)
self.send_message(user1_email, [user3_email, support_email], Recipient.PERSONAL)
# Users on three different realms can't PM each other,
# even if one of the users is a cross-realm bot.