org_settings: Display delivery_email to admins always is admin UI.

Mostly rewritten by Tim Abbott to ensure it correctly implements the
desired security model.

Administrators should have access to users' real email address so that
they can contact users out-of-band.
This commit is contained in:
Joshua Pan 2018-08-08 14:21:14 -07:00 committed by Tim Abbott
parent 07856ad648
commit c28c301506
4 changed files with 46 additions and 14 deletions

View File

@ -176,9 +176,14 @@ function populate_users(realm_people_data) {
filter: {
element: $users_table.closest(".settings-section").find(".search"),
callback: function (item, value) {
var email = item.email;
if (page_params.is_admin) {
email = item.delivery_email;
}
return (
item.full_name.toLowerCase().indexOf(value) >= 0 ||
item.email.toLowerCase().indexOf(value) >= 0
email.toLowerCase().indexOf(value) >= 0
);
},
onupdate: reset_scrollbar($users_table),
@ -194,9 +199,14 @@ function populate_users(realm_people_data) {
filter: {
element: $deactivated_users_table.closest(".settings-section").find(".search"),
callback: function (item, value) {
var email = item.email;
if (page_params.is_admin) {
email = item.delivery_email;
}
return (
item.full_name.toLowerCase().indexOf(value) >= 0 ||
item.email.toLowerCase().indexOf(value) >= 0
email.toLowerCase().indexOf(value) >= 0
);
},
onupdate: reset_scrollbar($deactivated_users_table),

View File

@ -4,7 +4,11 @@
<span class="user_name">{{full_name}}</span>
</td>
<td>
{{#if ../can_modify}}
<span class="email">{{delivery_email}}</span>
{{else}}
<span class="email">{{email}}</span>
{{/if}}
</td>
{{#unless is_bot}}
<td>
@ -21,7 +25,11 @@
{{/unless}}
{{#if is_bot}}
<td>
{{#if ../can_modify}}
<span class="owner">{{bot_owner_delivery_email}}</span>
{{else}}
<span class="owner">{{bot_owner}}</span>
{{/if}}
</td>
<td>
<span class="bot type">{{bot_type}}</span>

View File

@ -161,6 +161,7 @@ class PermissionTest(ZulipTestCase):
hamlet = find_dict(members, 'user_id', user.id)
self.assertEqual(hamlet['email'], user.email)
self.assertIsNone(hamlet['avatar_url'])
self.assertNotIn('delivery_email', hamlet)
# Also verify the /events code path. This is a bit hacky, but
# we need to verify client_gravatar is not being overridden.
@ -183,6 +184,7 @@ class PermissionTest(ZulipTestCase):
hamlet = find_dict(members, 'user_id', user.id)
self.assertEqual(hamlet['email'], "user%s@zulip.testserver" % (user.id,))
self.assertEqual(hamlet['avatar_url'], "https://secure.gravatar.com/avatar/5106fb7f928d89e2f2ddb3c4c42d6949?d=identicon&version=1")
self.assertNotIn('delivery_email', hamlet)
# Also verify the /events code path. This is a bit hacky, but
# basically we want to verify client_gravatar is being
@ -195,9 +197,10 @@ class PermissionTest(ZulipTestCase):
self.assertEqual(mock_request_event_queue.call_args_list[0][0][3],
False)
# It's still turned off for admins. In theory, it doesn't
# need to be, but client-side changes would be required in
# apps like the mobile apps.
# client_gravatar is still turned off for admins. In theory,
# it doesn't need to be, but client-side changes would be
# required in apps like the mobile apps.
# delivery_email is sent for admins.
admin.refresh_from_db()
self.login(admin.delivery_email)
result = self.client_get('/json/users?client_gravatar=true')
@ -206,6 +209,7 @@ class PermissionTest(ZulipTestCase):
hamlet = find_dict(members, 'user_id', user.id)
self.assertEqual(hamlet['email'], "user%s@zulip.testserver" % (user.id,))
self.assertEqual(hamlet['avatar_url'], "https://secure.gravatar.com/avatar/5106fb7f928d89e2f2ddb3c4c42d6949?d=identicon&version=1")
self.assertEqual(hamlet['delivery_email'], self.example_email("hamlet"))
def test_user_cannot_promote_to_admin(self) -> None:
self.login(self.example_email("hamlet"))

View File

@ -400,14 +400,7 @@ def get_members_backend(request: HttpRequest, user_profile: UserProfile,
realm = user_profile.realm
if realm.email_address_visibility == Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS:
# If email addresses are only available to administrators,
# clients cannot compute gravatars, so we force-set it to false.
client_gravatar = False
query = UserProfile.objects.filter(
realm_id=realm.id
).values(
fields = [
'id',
'email',
'realm_id',
@ -421,7 +414,19 @@ def get_members_backend(request: HttpRequest, user_profile: UserProfile,
'avatar_version',
'bot_owner__email',
'timezone',
)
]
if realm.email_address_visibility == Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS:
# If email addresses are only available to administrators,
# clients cannot compute gravatars, so we force-set it to false.
client_gravatar = False
include_delivery_email = user_profile.is_realm_admin
if include_delivery_email:
fields.append("delivery_email")
fields.append("bot_owner__delivery_email")
query = UserProfile.objects.filter(
realm_id=realm.id
).values(*fields)
def get_member(row: Dict[str, Any]) -> Dict[str, Any]:
email = row['email']
@ -452,6 +457,11 @@ def get_members_backend(request: HttpRequest, user_profile: UserProfile,
if row['bot_owner__email']:
result['bot_owner'] = row['bot_owner__email']
if include_delivery_email:
result['delivery_email'] = row['delivery_email']
if row['bot_owner__delivery_email']:
result['bot_owner_delivery_email'] = row['bot_owner__delivery_email']
return result
members = [get_member(row) for row in query]