Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

users: Update presence and user status code to support restricted users. 

The presence and user status update events are only sent to accessible
users, i.e. guests do not receive presence and user status updates for
users they cannot access.
This commit is contained in:
Sahil Batra 2023-10-17 16:26:39 +05:30 committed by Tim Abbott
parent 650e55fef8
commit dbcc9ea826
13 changed files with 236 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
api_docs/changelog.md
View File

@ -25,6 +25,19 @@ format used by the Zulip server that they are interacting with.
* [`GET /events`](/api/get-events): `realm_user` events with `op: "update"`
are now only sent to users who can access the modified user.
* [`GET /events`](/api/get-events): `presence` events are now only sent to
users who can access the user who comes back online if the
`CAN_ACCESS_ALL_USERS_GROUP_LIMITS_PRESENCE` server setting is set
to `true`.
* [`GET /events`](/api/get-events): `user_status` events are now only
sent to users who can access the modified user.
* [`GET /realm/presence`](/api/get-presence): The endpoint now returns
presence information of accessible users only if the
`CAN_ACCESS_ALL_USERS_GROUP_LIMITS_PRESENCE` server setting is set
to `true`.
**Feature level 227**
* [`PATCH /realm/user_settings_defaults`](/api/update-realm-user-settings-defaults),

5
zerver/actions/presence.py
View File

@ -11,6 +11,7 @@ from zerver.lib.presence import (
)
from zerver.lib.queue import queue_json_publish
from zerver.lib.timestamp import datetime_to_timestamp
from zerver.lib.users import get_user_ids_who_can_access_user
from zerver.models import Client, UserPresence, UserProfile, active_user_ids, get_client
from zerver.tornado.django_api import send_event
@ -27,7 +28,11 @@ def send_presence_changed(
#
# See https://zulip.readthedocs.io/en/latest/subsystems/presence.html for
# internals documentation on presence.
if settings.CAN_ACCESS_ALL_USERS_GROUP_LIMITS_PRESENCE:
user_ids = get_user_ids_who_can_access_user(user_profile)
else:
user_ids = active_user_ids(user_profile.realm_id)
if (
len(user_ids) > settings.USER_LIMIT_FOR_SENDING_PRESENCE_UPDATE_EVENTS
and not force_send_update

5
zerver/actions/user_status.py
View File

@ -2,7 +2,8 @@ from typing import Optional
from zerver.actions.user_settings import do_change_user_setting
from zerver.lib.user_status import update_user_status
from zerver.models import UserProfile, active_user_ids
from zerver.lib.users import get_user_ids_who_can_access_user
from zerver.models import UserProfile
from zerver.tornado.django_api import send_event
@ -50,4 +51,4 @@ def do_update_user_status(
event["emoji_name"] = emoji_name
event["emoji_code"] = emoji_code
event["reaction_type"] = reaction_type
send_event(realm, event, active_user_ids(realm.id))
send_event(realm, event, get_user_ids_who_can_access_user(user_profile))

8
zerver/lib/events.py
View File

@ -231,7 +231,9 @@ def fetch_initial_state_data(
if want("presence"):
state["presences"] = (
{} if user_profile is None else get_presences_for_realm(realm, slim_presence)
{}
if user_profile is None
else get_presences_for_realm(realm, slim_presence, user_profile)
)
# Send server_timestamp, to match the format of `GET /presence` requests.
state["server_timestamp"] = time.time()
@ -652,7 +654,9 @@ def fetch_initial_state_data(
if want("user_status"):
# We require creating an account to access statuses.
state["user_status"] = (
{} if user_profile is None else get_user_status_dict(realm_id=realm.id)
{}
if user_profile is None
else get_user_status_dict(realm=realm, user_profile=user_profile)
)
if want("user_topic"):

26
zerver/lib/presence.py
View File

@ -7,6 +7,7 @@ from django.conf import settings
from django.utils.timezone import now as timezone_now
from zerver.lib.timestamp import datetime_to_timestamp
from zerver.lib.users import check_user_can_access_all_users, get_accessible_user_ids
from zerver.models import PushDeviceToken, Realm, UserPresence, UserProfile, query_for_ids
@ -151,15 +152,25 @@ def get_presence_for_user(
def get_presence_dict_by_realm(
realm_id: int, slim_presence: bool = False
realm: Realm, slim_presence: bool = False, requesting_user_profile: Optional[UserProfile] = None
) -> Dict[str, Dict[str, Any]]:
two_weeks_ago = timezone_now() - datetime.timedelta(weeks=2)
query = UserPresence.objects.filter(
realm_id=realm_id,
realm_id=realm.id,
last_connected_time__gte=two_weeks_ago,
user_profile__is_active=True,
user_profile__is_bot=False,
).values(
)
if settings.CAN_ACCESS_ALL_USERS_GROUP_LIMITS_PRESENCE and not check_user_can_access_all_users(
requesting_user_profile
):
assert requesting_user_profile is not None
accessible_user_ids = get_accessible_user_ids(realm, requesting_user_profile)
query = query.filter(user_profile_id__in=accessible_user_ids)
presence_rows = list(
query.values(
"last_active_time",
"last_connected_time",
"user_profile__email",
@ -167,8 +178,7 @@ def get_presence_dict_by_realm(
"user_profile__enable_offline_push_notifications",
"user_profile__date_joined",
)
presence_rows = list(query)
)
mobile_query = PushDeviceToken.objects.distinct("user_id").values_list(
"user_id",
@ -196,13 +206,13 @@ def get_presence_dict_by_realm(
def get_presences_for_realm(
realm: Realm, slim_presence: bool
realm: Realm, slim_presence: bool, requesting_user_profile: UserProfile
) -> Dict[str, Dict[str, Dict[str, Any]]]:
if realm.presence_disabled:
# Return an empty dict if presence is disabled in this realm
return defaultdict(dict)
return get_presence_dict_by_realm(realm.id, slim_presence)
return get_presence_dict_by_realm(realm, slim_presence, requesting_user_profile)
def get_presence_response(
@ -210,5 +220,5 @@ def get_presence_response(
) -> Dict[str, Any]:
realm = requesting_user_profile.realm
server_timestamp = time.time()
presences = get_presences_for_realm(realm, slim_presence)
presences = get_presences_for_realm(realm, slim_presence, requesting_user_profile)
return dict(presences=presences, server_timestamp=server_timestamp)

21
zerver/lib/user_status.py
View File

@ -3,7 +3,8 @@ from typing import Dict, Optional, TypedDict
from django.db.models import Q
from django.utils.timezone import now as timezone_now
from zerver.models import UserStatus
from zerver.lib.users import check_user_can_access_all_users, get_accessible_user_ids
from zerver.models import Realm, UserProfile, UserStatus
class UserInfoDict(TypedDict, total=False):
@ -48,20 +49,23 @@ def format_user_status(row: RawUserInfoDict) -> UserInfoDict:
return dct
def get_user_status_dict(realm_id: int) -> Dict[str, UserInfoDict]:
rows = (
UserStatus.objects.filter(
user_profile__realm_id=realm_id,
def get_user_status_dict(realm: Realm, user_profile: UserProfile) -> Dict[str, UserInfoDict]:
query = UserStatus.objects.filter(
user_profile__realm_id=realm.id,
user_profile__is_active=True,
)
.exclude(
).exclude(
Q(user_profile__presence_enabled=True)
& Q(status_text="")
& Q(emoji_name="")
& Q(emoji_code="")
& Q(reaction_type=UserStatus.UNICODE_EMOJI),
)
.values(
if not check_user_can_access_all_users(user_profile):
accessible_user_ids = get_accessible_user_ids(realm, user_profile)
query = query.filter(user_profile_id__in=accessible_user_ids)
rows = query.values(
"user_profile_id",
"user_profile__presence_enabled",
"status_text",
@ -69,7 +73,6 @@ def get_user_status_dict(realm_id: int) -> Dict[str, UserInfoDict]:
"emoji_code",
"reaction_type",
)
)
user_dict: Dict[str, UserInfoDict] = {}
for row in rows:

3
zerver/lib/users.py
View File

@ -591,7 +591,8 @@ def check_can_access_user(
def get_user_ids_who_can_access_user(target_user: UserProfile) -> List[int]:
# We assume that caller only needs active users here, since
# this function is used to get users to send events.
# this function is used to get users to send events and to
# send presence update.
realm = target_user.realm
if not user_access_restricted_in_realm(target_user):
return active_user_ids(realm.id)

18
zerver/openapi/zulip.yaml
View File

@ -1133,6 +1133,13 @@ paths:
confusing users when someone comes online and immediately sends
a message (one wouldn't want them to still appear offline at
that point!).
If the `CAN_ACCESS_ALL_USERS_GROUP_LIMITS_PRESENCE` server-level
setting is set to `true`, then the event is only sent to users
who can access the user who came back online.
**Changes**: Prior to Zulip 8.0 (feature level 228), this event
was sent to all users in the organization.
properties:
id:
$ref: "#/components/schemas/EventIdSchema"
@ -1702,8 +1709,11 @@ paths:
- type: object
additionalProperties: false
description: |
Event sent to all users in a Zulip organization when the
status of a user changes.
Event sent to all users who can access the modified
user when the status of a user changes.
**Changes**: Prior to Zulip 8.0 (feature level 228),
this event was sent to all users in the organization.
properties:
id:
$ref: "#/components/schemas/EventIdSchema"
@ -9761,6 +9771,10 @@ paths:
description: |
Get the presence information of all the users in an organization.
If the `CAN_ACCESS_ALL_USERS_GROUP_LIMITS_PRESENCE` server-level
setting is set to `true`, presence information of only accessible
users are returned.
See [Zulip's developer documentation][subsystems-presence]
for details on the data model for presence in Zulip.

49
zerver/tests/test_events.py
View File

@ -1645,6 +1645,55 @@ class NormalActionsTest(BaseAction):
check_user_status("events[0]", events[0], {"status_text"})
self.set_up_db_for_testing_user_access()
cordelia = self.example_user("cordelia")
self.user_profile = self.example_user("polonius")
# Set the date_joined for cordelia here like we did at
# the start of this test.
cordelia.date_joined = timezone_now() - datetime.timedelta(days=15)
cordelia.save()
away_val = False
with self.settings(CAN_ACCESS_ALL_USERS_GROUP_LIMITS_PRESENCE=True):
events = self.verify_action(
lambda: do_update_user_status(
user_profile=cordelia,
away=away_val,
status_text="out to lunch",
emoji_name="car",
emoji_code="1f697",
reaction_type=UserStatus.UNICODE_EMOJI,
client_id=client.id,
),
num_events=0,
state_change_expected=False,
)
away_val = True
events = self.verify_action(
lambda: do_update_user_status(
user_profile=cordelia,
away=away_val,
status_text="at the beach",
emoji_name=None,
emoji_code=None,
reaction_type=None,
client_id=client.id,
),
num_events=1,
state_change_expected=True,
)
check_presence(
"events[0]",
events[0],
has_email=True,
# We no longer store information about the client and we simply
# set the field to 'website' for backwards compatibility.
presence_key="website",
status="idle",
)
def test_user_group_events(self) -> None:
othello = self.example_user("othello")
events = self.verify_action(

60
zerver/tests/test_presence.py
View File

@ -37,7 +37,7 @@ class UserPresenceModelTests(ZulipTestCase):
user_profile = self.example_user("hamlet")
email = user_profile.email
presence_dct = get_presence_dict_by_realm(user_profile.realm_id)
presence_dct = get_presence_dict_by_realm(user_profile.realm)
self.assert_length(presence_dct, 0)
self.login_user(user_profile)
@ -45,12 +45,12 @@ class UserPresenceModelTests(ZulipTestCase):
self.assert_json_success(result)
slim_presence = False
presence_dct = get_presence_dict_by_realm(user_profile.realm_id, slim_presence)
presence_dct = get_presence_dict_by_realm(user_profile.realm, slim_presence)
self.assert_length(presence_dct, 1)
self.assertEqual(presence_dct[email]["website"]["status"], "active")
slim_presence = True
presence_dct = get_presence_dict_by_realm(user_profile.realm_id, slim_presence)
presence_dct = get_presence_dict_by_realm(user_profile.realm, slim_presence)
self.assert_length(presence_dct, 1)
info = presence_dct[str(user_profile.id)]
self.assertEqual(set(info.keys()), {"active_timestamp", "idle_timestamp"})
@ -64,19 +64,19 @@ class UserPresenceModelTests(ZulipTestCase):
# Simulate the presence being a week old first. Nothing should change.
back_date(num_weeks=1)
presence_dct = get_presence_dict_by_realm(user_profile.realm_id)
presence_dct = get_presence_dict_by_realm(user_profile.realm)
self.assert_length(presence_dct, 1)
# If the UserPresence row is three weeks old, we ignore it.
back_date(num_weeks=3)
presence_dct = get_presence_dict_by_realm(user_profile.realm_id)
presence_dct = get_presence_dict_by_realm(user_profile.realm)
self.assert_length(presence_dct, 0)
# If the values are set to "never", ignore it just like for sufficiently old presence rows.
UserPresence.objects.filter(id=user_profile.id).update(
last_active_time=None, last_connected_time=None
)
presence_dct = get_presence_dict_by_realm(user_profile.realm_id)
presence_dct = get_presence_dict_by_realm(user_profile.realm)
self.assert_length(presence_dct, 0)
def test_pushable_always_false(self) -> None:
@ -92,7 +92,7 @@ class UserPresenceModelTests(ZulipTestCase):
self.assert_json_success(result)
def pushable() -> bool:
presence_dct = get_presence_dict_by_realm(user_profile.realm_id)
presence_dct = get_presence_dict_by_realm(user_profile.realm)
self.assert_length(presence_dct, 1)
return presence_dct[email]["website"]["pushable"]
@ -491,6 +491,17 @@ class SingleUserPresenceTests(ZulipTestCase):
result = self.client_get(f"/json/users/{othello.id}/presence", subdomain="zephyr")
self.assert_json_error(result, "No such user")
self.set_up_db_for_testing_user_access()
self.login("polonius")
with self.settings(CAN_ACCESS_ALL_USERS_GROUP_LIMITS_PRESENCE=True):
result = self.client_get(f"/json/users/{othello.id}/presence")
self.assert_json_error(result, "Insufficient permission")
result = self.client_get(f"/json/users/{othello.id}/presence")
result_dict = self.assert_json_success(result)
self.assertEqual(set(result_dict["presence"].keys()), {"website", "aggregated"})
self.assertEqual(set(result_dict["presence"]["website"].keys()), {"status", "timestamp"})
# Then, we check everything works
self.login("hamlet")
result = self.client_get("/json/users/othello@zulip.com/presence")
@ -658,6 +669,7 @@ class GetRealmStatusesTest(ZulipTestCase):
# Set up the test by simulating users reporting their presence data.
othello = self.example_user("othello")
hamlet = self.example_user("hamlet")
self.example_user("cordelia")
result = self.api_post(
othello,
@ -689,6 +701,40 @@ class GetRealmStatusesTest(ZulipTestCase):
json = self.assert_json_success(result)
self.assertEqual(set(json["presences"].keys()), {hamlet.email, othello.email})
# Check that polonius cannot fetch presence data for inaccessible user Othello
# if CAN_ACCESS_ALL_USERS_GROUP_LIMITS_PRESENCE is set to True.
self.set_up_db_for_testing_user_access()
polonius = self.example_user("polonius")
with self.settings(CAN_ACCESS_ALL_USERS_GROUP_LIMITS_PRESENCE=True):
result = self.api_get(polonius, "/api/v1/realm/presence")
json = self.assert_json_success(result)
self.assertEqual(set(json["presences"].keys()), {hamlet.email})
result = self.api_get(polonius, "/api/v1/realm/presence")
json = self.assert_json_success(result)
self.assertEqual(set(json["presences"].keys()), {hamlet.email, othello.email})
with self.settings(CAN_ACCESS_ALL_USERS_GROUP_LIMITS_PRESENCE=True):
result = self.api_post(
polonius,
"/api/v1/users/me/presence",
dict(status="idle"),
HTTP_USER_AGENT="ZulipDesktop/1.0",
)
json = self.assert_json_success(result)
self.assertEqual(set(json["presences"].keys()), {hamlet.email, polonius.email})
result = self.api_post(
polonius,
"/api/v1/users/me/presence",
dict(status="idle"),
HTTP_USER_AGENT="ZulipDesktop/1.0",
)
json = self.assert_json_success(result)
self.assertEqual(
set(json["presences"].keys()), {hamlet.email, polonius.email, othello.email}
)
def test_presence_disabled(self) -> None:
# Disable presence status and test whether the presence
# is reported or not.

32
zerver/tests/test_user_status.py
View File

@ -1,4 +1,4 @@
from typing import Any, Dict
from typing import Any, Dict, Optional
import orjson
@ -7,8 +7,10 @@ from zerver.lib.user_status import UserInfoDict, get_user_status_dict, update_us
from zerver.models import UserProfile, UserStatus, get_client
def user_status_info(user: UserProfile) -> UserInfoDict:
user_dict = get_user_status_dict(user.realm_id)
def user_status_info(user: UserProfile, acting_user: Optional[UserProfile] = None) -> UserInfoDict:
if acting_user is None:
acting_user = user
user_dict = get_user_status_dict(user.realm, acting_user)
return user_dict.get(str(user.id), {})
@ -115,6 +117,26 @@ class UserStatusTest(ZulipTestCase):
dict(status_text="in a meeting"),
)
# Test user status for inaccessible users.
self.set_up_db_for_testing_user_access()
cordelia = self.example_user("cordelia")
update_user_status(
user_profile_id=cordelia.id,
status_text="on vacation",
emoji_name=None,
emoji_code=None,
reaction_type=None,
client_id=client2.id,
)
self.assertEqual(
user_status_info(hamlet, self.example_user("polonius")),
dict(status_text="in a meeting"),
)
self.assertEqual(
user_status_info(cordelia, self.example_user("polonius")),
{},
)
def update_status_and_assert_event(
self, payload: Dict[str, Any], expected_event: Dict[str, Any], num_events: int = 1
) -> None:
@ -125,7 +147,7 @@ class UserStatusTest(ZulipTestCase):
def test_endpoints(self) -> None:
hamlet = self.example_user("hamlet")
realm_id = hamlet.realm_id
realm = hamlet.realm
self.login_user(hamlet)
@ -263,7 +285,7 @@ class UserStatusTest(ZulipTestCase):
expected_event=dict(type="user_status", user_id=hamlet.id, status_text=""),
)
self.assertEqual(
get_user_status_dict(realm_id=realm_id),
get_user_status_dict(realm=realm, user_profile=hamlet),
{},
)

6
zerver/views/presence.py
View File

@ -18,6 +18,7 @@ from zerver.lib.request import RequestNotes
from zerver.lib.response import json_success
from zerver.lib.timestamp import datetime_to_timestamp
from zerver.lib.typed_endpoint import ApiParamConfig, typed_endpoint
from zerver.lib.users import check_can_access_user
from zerver.models import (
UserActivity,
UserPresence,
@ -48,6 +49,11 @@ def get_presence_backend(
if target.is_bot:
raise JsonableError(_("Presence is not supported for bot users."))
if settings.CAN_ACCESS_ALL_USERS_GROUP_LIMITS_PRESENCE and not check_can_access_user(
target, user_profile
):
raise JsonableError(_("Insufficient permission"))
presence_dict = get_presence_for_user(target.id)
if len(presence_dict) == 0:
raise JsonableError(

7
zproject/default_settings.py
View File

@ -601,3 +601,10 @@ TYPING_STARTED_WAIT_PERIOD_MILLISECONDS = 10000
# notifications enabled. Default is set to avoid excessive Tornado
# load in large organizations.
MAX_STREAM_SIZE_FOR_TYPING_NOTIFICATIONS = 100
# Limiting guest access to other users via the
# can_access_all_users_group setting makes presence queries much more
# expensive. This can be a significant performance problem for
# installations with thousands of users with many guests limited in
# this way, pending further optimization of the relevant code paths.
CAN_ACCESS_ALL_USERS_GROUP_LIMITS_PRESENCE = False