email: Set email based on realm email_address_visibility.

This causes changing the email_address_visibility field to actually
modify what user_profile.email values are generated for users, both on
user creation and afterwards as email addresses are edited.

The overall feature isn't yet complete, but this brings us pretty close.
This commit is contained in:
Tim Abbott
2018-12-06 14:17:46 -08:00
parent fd3be00fbd
commit 47c85fa02e
7 changed files with 119 additions and 12 deletions

View File

@@ -126,7 +126,7 @@ from zerver.lib.bulk_create import bulk_create_users
from zerver.lib.timestamp import timestamp_to_datetime, datetime_to_timestamp
from zerver.lib.queue import queue_json_publish
from zerver.lib.utils import generate_api_key
from zerver.lib.create_user import create_user
from zerver.lib.create_user import create_user, get_display_email_address
from zerver.lib import bugdown
from zerver.lib.cache import cache_with_key, cache_set, \
user_profile_by_email_cache_key, \
@@ -565,6 +565,20 @@ def do_set_realm_property(realm: Realm, name: str, value: Any) -> None:
)
send_event(realm, event, active_user_ids(realm.id))
if name == "email_address_visibility":
for user_profile in UserProfile.objects.filter(realm=realm, is_bot=False):
# TODO: This does linear queries in the number of users
# and thus is potentially very slow. Probably not super
# important since this is a feature few folks will toggle,
# but as a policy matter, we don't do linear queries
# ~anywhere in Zulip.
old_email = user_profile.email
user_profile.email = get_display_email_address(user_profile, realm)
user_profile.save(update_fields=["email"])
# TODO: Design a bulk event for this or force-reload all clients
if user_profile.email != old_email:
send_user_email_update_event(user_profile)
def do_set_realm_authentication_methods(realm: Realm,
authentication_methods: Dict[str, bool]) -> None:
@@ -788,10 +802,12 @@ def send_user_email_update_event(user_profile: UserProfile) -> None:
def do_change_user_delivery_email(user_profile: UserProfile, new_email: str) -> None:
delete_user_profile_caches([user_profile])
user_profile.delivery_email = new_email
if user_profile.realm.email_address_visibility == Realm.EMAIL_ADDRESS_VISIBILITY_EVERYONE:
user_profile.email = new_email
user_profile.delivery_email = new_email
user_profile.save(update_fields=["email", "delivery_email"])
else:
user_profile.save(update_fields=["delivery_email"])
# We notify just the target user (and eventually org admins) about
# their new delivery email, since that field is private.

View File

@@ -20,6 +20,10 @@ def bulk_create_users(realm: Realm,
realm=realm).values_list('email', flat=True))
users = sorted([user_raw for user_raw in users_raw if user_raw[0] not in existing_users])
# If we have a different email_address_visibility mode, the code
# below doesn't have the logic to set user_profile.email properly.
assert realm.email_address_visibility == Realm.EMAIL_ADDRESS_VISIBILITY_EVERYONE
# Now create user_profiles
profiles_to_create = [] # type: List[UserProfile]
for (email, full_name, short_name, active) in users:

View File

@@ -31,6 +31,12 @@ def copy_user_settings(source_profile: UserProfile, target_profile: UserProfile)
copy_hotpots(source_profile, target_profile)
def get_display_email_address(user_profile: UserProfile, realm: Realm) -> str:
if realm.email_address_visibility != Realm.EMAIL_ADDRESS_VISIBILITY_EVERYONE:
# TODO: realm.host isn't always a valid option here.
return "user%s@%s" % (user_profile.id, realm.host.split(':')[0])
return user_profile.delivery_email
# create_user_profile is based on Django's User.objects.create_user,
# except that we don't save to the database so it can used in
# bulk_creates
@@ -48,7 +54,7 @@ def create_user_profile(realm: Realm, email: str, password: Optional[str],
now = timezone_now()
email = UserManager.normalize_email(email)
user_profile = UserProfile(email=email, is_staff=False, is_active=active,
user_profile = UserProfile(is_staff=False, is_active=active,
full_name=full_name, short_name=short_name,
last_login=now, date_joined=now, realm=realm,
pointer=-1, is_bot=bool(bot_type), bot_type=bot_type,
@@ -63,9 +69,10 @@ def create_user_profile(realm: Realm, email: str, password: Optional[str],
if bot_type or not active:
password = None
if realm.email_address_visibility == Realm.EMAIL_ADDRESS_VISIBILITY_EVERYONE:
# If emails are visible to everyone, we can set this here and save a DB query
user_profile.email = get_display_email_address(user_profile, realm)
user_profile.set_password(password)
user_profile.api_key = generate_api_key()
return user_profile
@@ -107,6 +114,13 @@ def create_user(email: str, password: Optional[str], realm: Realm,
else:
user_profile.save()
if realm.email_address_visibility != Realm.EMAIL_ADDRESS_VISIBILITY_EVERYONE:
# With restricted access to email addresses, we can't generate
# the fake email addresses we use for display purposes without
# a User ID, which isn't generated until the .save() above.
user_profile.email = get_display_email_address(user_profile, realm)
user_profile.save(update_fields=['email'])
recipient = Recipient.objects.create(type_id=user_profile.id,
type=Recipient.PERSONAL)
Subscription.objects.create(user_profile=user_profile, recipient=recipient)

View File

@@ -12,7 +12,8 @@ from zerver.lib.actions import do_start_email_change_process, do_set_realm_prope
from zerver.lib.test_classes import (
ZulipTestCase,
)
from zerver.models import get_user_by_delivery_email, EmailChangeStatus, get_realm
from zerver.models import get_user_by_delivery_email, EmailChangeStatus, get_realm, \
Realm, UserProfile, get_user, get_user_profile_by_id
class EmailChangeTestCase(ZulipTestCase):
@@ -186,3 +187,37 @@ class EmailChangeTestCase(ZulipTestCase):
result = self.client_patch(url, data)
self.assertEqual('success', result.json()['result'])
self.assertEqual('', result.json()['msg'])
def test_change_delivery_email_end_to_end_with_admins_visibility(self) -> None:
user_profile = self.example_user('hamlet')
do_set_realm_property(user_profile.realm, 'email_address_visibility',
Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS)
old_email = user_profile.email
new_email = 'hamlet-new@zulip.com'
self.login(self.example_email('hamlet'))
obj = EmailChangeStatus.objects.create(new_email=new_email,
old_email=old_email,
user_profile=user_profile,
realm=user_profile.realm)
key = generate_key()
Confirmation.objects.create(content_object=obj,
date_sent=now(),
confirmation_key=key,
type=Confirmation.EMAIL_CHANGE)
url = confirmation_url(key, user_profile.realm.host, Confirmation.EMAIL_CHANGE)
response = self.client_get(url)
self.assertEqual(response.status_code, 200)
self.assert_in_success_response(["This confirms that the email address for your Zulip"],
response)
user_profile = get_user_profile_by_id(user_profile.id)
self.assertEqual(user_profile.delivery_email, new_email)
self.assertEqual(user_profile.email, "user4@zulip.testserver")
obj.refresh_from_db()
self.assertEqual(obj.status, 1)
with self.assertRaises(UserProfile.DoesNotExist):
get_user(old_email, user_profile.realm)
with self.assertRaises(UserProfile.DoesNotExist):
get_user_by_delivery_email(old_email, user_profile.realm)
self.assertEqual(get_user_by_delivery_email(new_email, user_profile.realm), user_profile)

View File

@@ -16,7 +16,7 @@ from zerver.models import (
get_client, get_realm, get_stream_recipient, get_stream,
Message, RealmDomain, Recipient, UserMessage, UserPresence, UserProfile,
Realm, Subscription, Stream, flush_per_request_caches, UserGroup, Service,
Attachment, PreregistrationUser,
Attachment, PreregistrationUser, get_user_by_delivery_email
)
from zerver.lib.actions import (
@@ -1183,6 +1183,36 @@ class EventsRegisterTest(ZulipTestCase):
self.assert_length(events, 1)
error = realm_user_add_checker('events[0]', events[0])
self.assert_on_error(error)
new_user_profile = get_user_by_delivery_email("test1@zulip.com", self.user_profile.realm)
self.assertEqual(new_user_profile.email, "test1@zulip.com")
def test_register_events_email_address_visibility(self) -> None:
realm_user_add_checker = self.check_events_dict([
('type', equals('realm_user')),
('op', equals('add')),
('person', check_dict_only([
('user_id', check_int),
('email', check_string),
('avatar_url', check_none_or(check_string)),
('full_name', check_string),
('is_admin', check_bool),
('is_bot', check_bool),
('is_guest', check_bool),
('profile_data', check_dict_only([])),
('timezone', check_string),
('date_joined', check_string),
])),
])
do_set_realm_property(self.user_profile.realm, "email_address_visibility",
Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS)
events = self.do_test(lambda: self.register("test1@zulip.com", "test1"))
self.assert_length(events, 1)
error = realm_user_add_checker('events[0]', events[0])
self.assert_on_error(error)
new_user_profile = get_user_by_delivery_email("test1@zulip.com", self.user_profile.realm)
self.assertEqual(new_user_profile.email, "user%s@zulip.testserver" % (new_user_profile.id))
def test_alert_words_events(self) -> None:
alert_words_checker = self.check_events_dict([
@@ -1449,6 +1479,10 @@ class EventsRegisterTest(ZulipTestCase):
])
do_set_realm_property(self.user_profile.realm, "email_address_visibility",
Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS)
# Important: We need to refresh from the database here so that
# we don't have a stale UserProfile object with an old value
# for email being passed into this next function.
self.user_profile.refresh_from_db()
action = lambda: do_change_user_delivery_email(self.user_profile, 'newhamlet@zulip.com')
events = self.do_test(action, num_events=1)
error = schema_checker('events[0]', events[0])

View File

@@ -27,7 +27,8 @@ from zerver.lib.test_classes import ZulipTestCase
from zerver.lib.test_helpers import tornado_redirected_to_list
from zerver.lib.test_runner import slow
from zerver.models import get_realm, Realm, UserProfile, ScheduledEmail, get_stream, \
CustomProfileField, Message, UserMessage, Attachment, get_user_profile_by_email
CustomProfileField, Message, UserMessage, Attachment, get_user_profile_by_email, \
get_user_profile_by_id
class RealmTest(ZulipTestCase):
def assert_user_profile_cache_gets_new_name(self, user_profile: UserProfile,
@@ -364,8 +365,8 @@ class RealmTest(ZulipTestCase):
def test_change_email_address_visibility(self) -> None:
# We need an admin user.
email = 'iago@zulip.com'
self.login(email)
user_profile = self.example_user("iago")
self.login(user_profile.email)
invalid_value = 4
req = dict(email_address_visibility = ujson.dumps(invalid_value))
result = self.client_patch('/json/realm', req)
@@ -379,6 +380,9 @@ class RealmTest(ZulipTestCase):
realm = get_realm("zulip")
self.assertEqual(realm.email_address_visibility, Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS)
edited_user_profile = get_user_profile_by_id(user_profile.id)
self.assertEqual(edited_user_profile.email, "user%s@zulip.testserver" % (edited_user_profile.id,))
def test_change_video_chat_provider(self) -> None:
self.assertEqual(get_realm('zulip').video_chat_provider, "Jitsi")
email = self.example_email("iago")

View File

@@ -296,7 +296,7 @@ def accounts_register(request: HttpRequest) -> HttpResponse:
# This dummy_backend check below confirms the user is
# authenticating to the correct subdomain.
auth_result = authenticate(username=user_profile.email,
auth_result = authenticate(username=user_profile.delivery_email,
realm=realm,
return_data=return_data,
use_dummy_backend=True)