Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Allow guest access to /sync #455

Merged
merged 9 commits into from
Dec 22, 2015
16 changes: 16 additions & 0 deletions synapse/api/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,22 @@ def __init__(self, *args, **kwargs):
super(AuthError, self).__init__(*args, **kwargs)


class GuestAccessError(AuthError):
"""An error raised when a there is a problem with a guest user accessing
a room"""

def __init__(self, rooms, *args, **kwargs):
self.rooms = rooms
super(GuestAccessError, self).__init__(*args, **kwargs)

def error_dict(self):
return cs_error(
self.msg,
self.errcode,
rooms=self.rooms,
)


class EventSizeError(SynapseError):
"""An error raised when an event is too big."""

Expand Down
12 changes: 12 additions & 0 deletions synapse/api/filtering.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,9 @@ def __init__(self, filter_json):
"include_leave", False
)

def list_rooms(self):
return self.room_filter.list_rooms()

def timeline_limit(self):
return self.room_timeline_filter.limit()

Expand Down Expand Up @@ -181,6 +184,15 @@ class Filter(object):
def __init__(self, filter_json):
self.filter_json = filter_json

def list_rooms(self):
"""The list of room_id strings this filter restricts the output to
or None if the this filter doesn't list the room ids.
"""
if "rooms" in self.filter_json:
return list(set(self.filter_json["rooms"]))
else:
return None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't do None checks in a ton of places in this PR which call list_rooms(). Either do the null-guards or make non-existence return an empty list, else this is too error-prone IMO.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We check that the list is non-None before calling into these handlers. I'd rather have a hard error here than a default.


def check(self, event):
"""Checks whether the filter matches the given event.

Expand Down
156 changes: 107 additions & 49 deletions synapse/handlers/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@

from ._base import BaseHandler

from synapse.streams.config import PaginationConfig
from synapse.api.constants import Membership, EventTypes
from synapse.api.errors import GuestAccessError
from synapse.util import unwrapFirstError

from twisted.internet import defer
Expand All @@ -29,6 +29,7 @@

SyncConfig = collections.namedtuple("SyncConfig", [
"user",
"is_guest",
"filter",
])

Expand Down Expand Up @@ -117,6 +118,8 @@ def __nonzero__(self):
self.presence or self.joined or self.invited
)

GuestRoom = collections.namedtuple("GuestRoom", ("room_id", "membership"))


class SyncHandler(BaseHandler):

Expand All @@ -135,6 +138,18 @@ def wait_for_sync_for_user(self, sync_config, since_token=None, timeout=0,
A Deferred SyncResult.
"""

if sync_config.is_guest:
bad_rooms = []
for room_id in sync_config.filter.list_rooms():
world_readable = yield self._is_world_readable(room_id)
if not world_readable:
bad_rooms.append(room_id)

if bad_rooms:
raise GuestAccessError(
bad_rooms, 403, "Guest access not allowed"
)

if timeout == 0 or since_token is None or full_state:
# we are going to return immediately, so don't bother calling
# notifier.wait_for_events.
Expand All @@ -151,6 +166,17 @@ def current_sync_callback(before_token, after_token):
)
defer.returnValue(result)

@defer.inlineCallbacks
def _is_world_readable(self, room_id):
state = yield self.hs.get_state_handler().get_current_state(
room_id,
EventTypes.RoomHistoryVisibility
)
if state and "history_visibility" in state.content:
defer.returnValue(state.content["history_visibility"] == "world_readable")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

http://matrix.org/docs/spec/r0.0.0/client_server.html#redactions makes no mention of history_visibility being a preserved key in the event of a redacted m.room.history_visibility event. This will throw if the event is redacted.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this will throw because we check if the history_visibility is in the content before accessing it.
This the same check as used for /events FWIW

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad; completely didn't see that you are indeed checking. Sorry!

else:
defer.returnValue(False)

def current_sync_for_user(self, sync_config, since_token=None,
full_state=False):
"""Get the sync for client needed to match what the server has now.
Expand All @@ -174,37 +200,52 @@ def full_state_sync(self, sync_config, timeline_since_token):
"""
now_token = yield self.event_sources.get_current_token()

now_token, ephemeral_by_room = yield self.ephemeral_by_room(
sync_config, now_token
)
if sync_config.is_guest:
room_list = [
GuestRoom(room_id, Membership.JOIN)
for room_id in sync_config.filter.list_rooms()
]

presence_stream = self.event_sources.sources["presence"]
# TODO (mjark): This looks wrong, shouldn't we be getting the presence
# UP to the present rather than after the present?
pagination_config = PaginationConfig(from_token=now_token)
presence, _ = yield presence_stream.get_pagination_rows(
user=sync_config.user,
pagination_config=pagination_config.get_source_config("presence"),
key=None
)
account_data = {}
account_data_by_room = {}
tags_by_room = {}

membership_list = (Membership.INVITE, Membership.JOIN)
if sync_config.filter.include_leave:
membership_list += (Membership.LEAVE, Membership.BAN)
else:
membership_list = (Membership.INVITE, Membership.JOIN)
if sync_config.filter.include_leave:
membership_list += (Membership.LEAVE, Membership.BAN)

room_list = yield self.store.get_rooms_for_user_where_membership_is(
user_id=sync_config.user.to_string(),
membership_list=membership_list
)
room_list = yield self.store.get_rooms_for_user_where_membership_is(
user_id=sync_config.user.to_string(),
membership_list=membership_list
)

account_data, account_data_by_room = (
yield self.store.get_account_data_for_user(
sync_config.user.to_string()
)
)

account_data, account_data_by_room = (
yield self.store.get_account_data_for_user(
tags_by_room = yield self.store.get_tags_for_user(
sync_config.user.to_string()
)

presence_stream = self.event_sources.sources["presence"]

joined_room_ids = [
room.room_id for room in room_list
if room.membership == Membership.JOIN
]

presence, _ = yield presence_stream.get_new_events(
from_key=0,
user=sync_config.user,
room_ids=joined_room_ids,
is_guest=sync_config.is_guest,
)

tags_by_room = yield self.store.get_tags_for_user(
sync_config.user.to_string()
now_token, ephemeral_by_room = yield self.ephemeral_by_room(
sync_config, now_token, joined_room_ids
)

joined = []
Expand Down Expand Up @@ -315,11 +356,13 @@ def account_data_for_room(self, room_id, tags_by_room, account_data_by_room):
return account_data_events

@defer.inlineCallbacks
def ephemeral_by_room(self, sync_config, now_token, since_token=None):
def ephemeral_by_room(self, sync_config, now_token, room_ids,
since_token=None):
"""Get the ephemeral events for each room the user is in
Args:
sync_config (SyncConfig): The flags, filters and user for the sync.
now_token (StreamToken): Where the server is currently up to.
room_ids (list): List of room id strings to get data for.
since_token (StreamToken): Where the server was when the client
last synced.
Returns:
Expand All @@ -330,9 +373,6 @@ def ephemeral_by_room(self, sync_config, now_token, since_token=None):

typing_key = since_token.typing_key if since_token else "0"

rooms = yield self.store.get_rooms_for_user(sync_config.user.to_string())
room_ids = [room.room_id for room in rooms]

typing_source = self.event_sources.sources["typing"]
typing, typing_key = yield typing_source.get_new_events(
user=sync_config.user,
Expand Down Expand Up @@ -410,24 +450,49 @@ def incremental_sync_with_gap(self, sync_config, since_token):
"""
now_token = yield self.event_sources.get_current_token()

rooms = yield self.store.get_rooms_for_user(sync_config.user.to_string())
room_ids = [room.room_id for room in rooms]
if sync_config.is_guest:
room_ids = sync_config.filter.list_rooms()

tags_by_room = {}
account_data = {}
account_data_by_room = {}

else:
rooms = yield self.store.get_rooms_for_user(
sync_config.user.to_string()
)
room_ids = [room.room_id for room in rooms]

now_token, ephemeral_by_room = yield self.ephemeral_by_room(
sync_config, now_token, since_token
)

tags_by_room = yield self.store.get_updated_tags(
sync_config.user.to_string(),
since_token.account_data_key,
)

account_data, account_data_by_room = (
yield self.store.get_updated_account_data_for_user(
sync_config.user.to_string(),
since_token.account_data_key,
)
)

now_token, ephemeral_by_room = yield self.ephemeral_by_room(
sync_config, now_token, room_ids, since_token
)

presence_source = self.event_sources.sources["presence"]
presence, presence_key = yield presence_source.get_new_events(
user=sync_config.user,
from_key=since_token.presence_key,
limit=sync_config.filter.presence_limit(),
room_ids=room_ids,
# /sync doesn't support guest access, they can't get to this point in code
is_guest=False,
is_guest=sync_config.is_guest,
)
now_token = now_token.copy_and_replace("presence_key", presence_key)

now_token, ephemeral_by_room = yield self.ephemeral_by_room(
sync_config, now_token, since_token
)

rm_handler = self.hs.get_handlers().room_member_handler
app_service = yield self.store.get_app_service_by_user_id(
sync_config.user.to_string()
Expand All @@ -447,18 +512,8 @@ def incremental_sync_with_gap(self, sync_config, since_token):
from_key=since_token.room_key,
to_key=now_token.room_key,
limit=timeline_limit + 1,
)

tags_by_room = yield self.store.get_updated_tags(
sync_config.user.to_string(),
since_token.account_data_key,
)

account_data, account_data_by_room = (
yield self.store.get_updated_account_data_for_user(
sync_config.user.to_string(),
since_token.account_data_key,
)
room_ids=room_ids if sync_config.is_guest else (),
is_guest=sync_config.is_guest,
)

joined = []
Expand Down Expand Up @@ -590,7 +645,10 @@ def load_filtered_recents(self, room_id, sync_config, now_token,
end_key = "s" + room_key.split('-')[-1]
loaded_recents = sync_config.filter.filter_room_timeline(events)
loaded_recents = yield self._filter_events_for_client(
sync_config.user.to_string(), loaded_recents,
sync_config.user.to_string(),
loaded_recents,
is_guest=sync_config.is_guest,
require_all_visible_for_guests=False
)
loaded_recents.extend(recents)
recents = loaded_recents
Expand Down
10 changes: 9 additions & 1 deletion synapse/rest/client/v2_alpha/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ def __init__(self, hs):

@defer.inlineCallbacks
def on_GET(self, request):
user, token_id, _ = yield self.auth.get_user_by_req(request)
user, token_id, is_guest = yield self.auth.get_user_by_req(
request, allow_guest=True
)

timeout = parse_integer(request, "timeout", default=0)
since = parse_string(request, "since")
Expand Down Expand Up @@ -118,8 +120,14 @@ def on_GET(self, request):
except:
filter = FilterCollection({})

if is_guest and filter.list_rooms() is None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like filter.list_rooms() will return None iff there is no rooms key but there is no 0-length array check. We should probably guard against { "rooms": [] } as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it matter? I'm not sure if passing an empty list of rooms is invalid or not here.

raise SynapseError(
400, "Guest users must provide a list of rooms in the filter"
)

sync_config = SyncConfig(
user=user,
is_guest=is_guest,
filter=filter,
)

Expand Down