email_validation/tests: Return anonymous email when inviting existing…

… users. Previously during an error with invite_users_backend codepath, we returned 3 error items - invited email, error message, is_deactivated. However, for organizations which haven't enabled EMAIL_ADDRESS_VISIBILITY_EVERYONE the actual user email stored in the frontend is anonymised and different from the invited email. To fix this, we return back the anonymous emails for existing accounts along with the other parameters. This results in 4 return values -> - email (invited email) - error_msg - is_deactivated - maybe_anonymous_email Incase EMAIL_ADDRESS_VISIBILITY_EVERYONE is enabled for the realm, maybe_anonymous_email would be same as the invited email. This is also the case for cross_realm_bots. For invalid emails, we just return back no strings. Tests amended.
zulip · sumanthvrao · Aug 4, 2020 · Aug 5, 2020 · Aug 5, 2020 · Aug 5, 2020
commit e6ba3946f25bb838ff699ee987fa3f673ac22e76
diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py
@@ -5149,10 +5149,10 @@ class InvitationError(JsonableError):
     code = ErrorCode.INVITATION_FAILED
     data_fields = ['errors', 'sent_invitations']
 
-    def __init__(self, msg: str, errors: List[Tuple[str, str, bool]],
+    def __init__(self, msg: str, errors: List[Tuple[str, str, bool, str]],
                  sent_invitations: bool) -> None:
         self._msg: str = msg
-        self.errors: List[Tuple[str, str, bool]] = errors
+        self.errors: List[Tuple[str, str, bool, str]] = errors
         self.sent_invitations: bool = sent_invitations
 
 def estimate_recent_invites(realms: Iterable[Realm], *, days: int) -> int:
@@ -5220,7 +5220,7 @@ def do_invite_users(user_profile: UserProfile,
                 [], sent_invitations=False)
 
     good_emails: Set[str] = set()
-    errors: List[Tuple[str, str, bool]] = []
+    errors: List[Tuple[str, str, bool, str]] = []
     validate_email_allowed_in_realm = get_realm_email_validator(user_profile.realm)
     for email in invitee_emails:
         if email == '':
@@ -5231,7 +5231,7 @@ def do_invite_users(user_profile: UserProfile,
         )
 
         if email_error:
-            errors.append((email, email_error, False))
+            errors.append((email, email_error, False, ''))
         else:
             good_emails.add(email)
 
@@ -5242,10 +5242,10 @@ def do_invite_users(user_profile: UserProfile,
     '''
     error_dict = get_existing_user_errors(user_profile.realm, good_emails)
 
-    skipped: List[Tuple[str, str, bool]] = []
+    skipped: List[Tuple[str, str, bool, str]] = []
     for email in error_dict:
-        msg, deactivated = error_dict[email]
-        skipped.append((email, msg, deactivated))
+        msg, deactivated, maybe_anonymous_email = error_dict[email]
+        skipped.append((email, msg, deactivated, maybe_anonymous_email))
         good_emails.remove(email)
 
     validated_emails = list(good_emails)

diff --git a/zerver/lib/email_validation.py b/zerver/lib/email_validation.py
@@ -119,7 +119,7 @@ def get_existing_user_errors(
     target_realm: Realm,
     emails: Set[str],
     verbose: bool=False,
-) -> Dict[str, Tuple[str, bool]]:
+) -> Dict[str, Tuple[str, bool, str]]:
     '''
     We use this function even for a list of one emails.
 
@@ -128,7 +128,7 @@ def get_existing_user_errors(
     to cross-realm bots and mirror dummies too.
     '''
 
-    errors: Dict[str, Tuple[str, bool]] = {}
+    errors: Dict[str, Tuple[str, bool, str]] = {}
 
     users = get_users_by_delivery_email(emails, target_realm).only(
         'delivery_email',
@@ -153,7 +153,7 @@ def process_email(email: str) -> None:
             else:
                 msg = _('Reserved for system bots.')
             deactivated = False
-            errors[email] = (msg, deactivated)
+            errors[email] = (msg, deactivated, email)
             return
 
         existing_user_profile = user_dict.get(email.lower())
@@ -180,7 +180,12 @@ def process_email(email: str) -> None:
         else:
             msg = _("Account has been deactivated.")
 
-        errors[email] = (msg, deactivated)
+        '''
+        The existing_user_profile.email sends back the anonymous emails (different from
+        delivery_email depending on EMAIL_ADDRESS_VISIBILITY_EVERYONE) so that the clients
+        can continue to subscribe these existing users if they wish to.
+        '''
+        errors[email] = (msg, deactivated, existing_user_profile.email)
 
     for email in emails:
         process_email(email)
@@ -204,5 +209,5 @@ def validate_email_not_already_in_realm(target_realm: Realm,
     # Loop through errors, the only key should be our email.
     for key, error_info in error_dict.items():
         assert key == email
-        msg, deactivated = error_info
+        msg, deactivated, _ = error_info
         raise ValidationError(msg)
diff --git a/zerver/tests/test_auth_backends.py b/zerver/tests/test_auth_backends.py
@@ -4998,18 +4998,21 @@ def test_validate_email(self) -> None:
         self.assertIn('containing + are not allowed', error)
 
         cordelia_email = cordelia.delivery_email
+        cordelia_anonymous_email = cordelia.email
         errors = get_existing_user_errors(realm, {cordelia_email})
-        error, is_deactivated = errors[cordelia_email]
+        error, is_deactivated, maybe_anonymous_email = errors[cordelia_email]
         self.assertEqual(False, is_deactivated)
         self.assertEqual(error, 'Already has an account.')
+        self.assertEqual(maybe_anonymous_email, cordelia_anonymous_email)
 
         cordelia.is_active = False
         cordelia.save()
 
         errors = get_existing_user_errors(realm, {cordelia_email})
-        error, is_deactivated = errors[cordelia_email]
+        error, is_deactivated, maybe_anonymous_email = errors[cordelia_email]
         self.assertEqual(True, is_deactivated)
         self.assertEqual(error, 'Account has been deactivated.')
+        self.assertEqual(maybe_anonymous_email, cordelia_anonymous_email)
 
         errors = get_existing_user_errors(realm, {'fred-is-fine@zulip.com'})
         self.assertEqual(errors, {})