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

Commit

Permalink
Skip, rather than erroring, invalid guest requests
Browse files Browse the repository at this point in the history
Erroring causes problems when people make illegal requests, because they
don't know what limit parameter they should pass.

This is definitely buggy. It leaks message counts for rooms people don't
have permission to see, via tokens. But apparently we already
consciously decided to allow that as a team, so this preserves that
behaviour.
  • Loading branch information
illicitonion authored and richvdh committed Jan 13, 2016
1 parent c110eb9 commit 93afb40
Show file tree
Hide file tree
Showing 5 changed files with 5 additions and 21 deletions.
16 changes: 2 additions & 14 deletions synapse/handlers/_base.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# -*- coding: utf-8 -*-
# Copyright 2014, 2015 OpenMarket Ltd
# Copyright 2014 - 2016 OpenMarket Ltd
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -52,8 +52,7 @@ def __init__(self, hs):
self.event_builder_factory = hs.get_event_builder_factory()

@defer.inlineCallbacks
def _filter_events_for_client(self, user_id, events, is_guest=False,
require_all_visible_for_guests=True):
def _filter_events_for_client(self, user_id, events, is_guest=False):
# Assumes that user has at some point joined the room if not is_guest.

def allowed(event, membership, visibility):
Expand Down Expand Up @@ -114,17 +113,6 @@ def allowed(event, membership, visibility):
if should_include:
events_to_return.append(event)

if (require_all_visible_for_guests
and is_guest
and len(events_to_return) < len(events)):
# This indicates that some events in the requested range were not
# visible to guest users. To be safe, we reject the entire request,
# so that we don't have to worry about interpreting visibility
# boundaries.
raise AuthError(403, "User %s does not have permission" % (
user_id
))

defer.returnValue(events_to_return)

def ratelimit(self, user_id):
Expand Down
4 changes: 2 additions & 2 deletions synapse/handlers/message.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# -*- coding: utf-8 -*-
# Copyright 2014, 2015 OpenMarket Ltd
# Copyright 2014 - 2016 OpenMarket Ltd
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -685,7 +685,7 @@ def get_receipts():
).addErrback(unwrapFirstError)

messages = yield self._filter_events_for_client(
user_id, messages, is_guest=is_guest, require_all_visible_for_guests=False
user_id, messages, is_guest=is_guest,
)

start_token = now_token.copy_and_replace("room_key", token[0])
Expand Down
2 changes: 0 additions & 2 deletions synapse/handlers/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -879,14 +879,12 @@ def get_event_context(self, user, room_id, event_id, limit, is_guest):
user.to_string(),
results["events_before"],
is_guest=is_guest,
require_all_visible_for_guests=False
)

results["events_after"] = yield self._filter_events_for_client(
user.to_string(),
results["events_after"],
is_guest=is_guest,
require_all_visible_for_guests=False
)

if results["events_after"]:
Expand Down
1 change: 0 additions & 1 deletion synapse/handlers/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,6 @@ def load_filtered_recents(self, room_id, sync_config, now_token,
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
3 changes: 1 addition & 2 deletions synapse/notifier.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# -*- coding: utf-8 -*-
# Copyright 2014, 2015 OpenMarket Ltd
# Copyright 2014 - 2016 OpenMarket Ltd
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -386,7 +386,6 @@ def check_for_updates(before_token, after_token):
user.to_string(),
new_events,
is_guest=is_guest,
require_all_visible_for_guests=False
)

events.extend(new_events)
Expand Down

0 comments on commit 93afb40

Please sign in to comment.