From 2ef6de928d9f0d2761e0664d374b2bfe13682061 Mon Sep 17 00:00:00 2001
From: Daniel Wagner-Hall <daniel@matrix.org>
Date: Tue, 5 Jan 2016 18:12:37 +0000
Subject: [PATCH] Skip, rather than erroring, invalid guest requests

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.
---
 synapse/handlers/_base.py   | 16 ++--------------
 synapse/handlers/message.py |  4 ++--
 synapse/handlers/room.py    |  2 --
 synapse/handlers/sync.py    |  1 -
 synapse/notifier.py         |  3 +--
 5 files changed, 5 insertions(+), 21 deletions(-)

diff --git a/synapse/handlers/_base.py b/synapse/handlers/_base.py
index 5fd20285d..b474042e8 100644
--- a/synapse/handlers/_base.py
+++ b/synapse/handlers/_base.py
@@ -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.
@@ -52,8 +52,7 @@ class BaseHandler(object):
         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):
@@ -114,17 +113,6 @@ class BaseHandler(object):
             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):
diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py
index a1bed9b0d..5805190ce 100644
--- a/synapse/handlers/message.py
+++ b/synapse/handlers/message.py
@@ -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.
@@ -685,7 +685,7 @@ class MessageHandler(BaseHandler):
         ).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])
diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index 6186c37c7..48a07e4e3 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -895,14 +895,12 @@ class RoomContextHandler(BaseHandler):
             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"]:
diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py
index 41a42418a..b63a27b38 100644
--- a/synapse/handlers/sync.py
+++ b/synapse/handlers/sync.py
@@ -648,7 +648,6 @@ class SyncHandler(BaseHandler):
                 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
diff --git a/synapse/notifier.py b/synapse/notifier.py
index fd5257832..0a5653b8d 100644
--- a/synapse/notifier.py
+++ b/synapse/notifier.py
@@ -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.
@@ -386,7 +386,6 @@ class Notifier(object):
                         user.to_string(),
                         new_events,
                         is_guest=is_guest,
-                        require_all_visible_for_guests=False
                     )
 
                 events.extend(new_events)