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.pull/492/head
parent
c110eb92f8
commit
93afb40cd4
|
@ -1,5 +1,5 @@
|
||||||
# -*- coding: utf-8 -*-
|
# -*- coding: utf-8 -*-
|
||||||
# Copyright 2014, 2015 OpenMarket Ltd
|
# Copyright 2014 - 2016 OpenMarket Ltd
|
||||||
#
|
#
|
||||||
# Licensed under the Apache License, Version 2.0 (the "License");
|
# Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
# you may not use this file except in compliance with 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()
|
self.event_builder_factory = hs.get_event_builder_factory()
|
||||||
|
|
||||||
@defer.inlineCallbacks
|
@defer.inlineCallbacks
|
||||||
def _filter_events_for_client(self, user_id, events, is_guest=False,
|
def _filter_events_for_client(self, user_id, events, is_guest=False):
|
||||||
require_all_visible_for_guests=True):
|
|
||||||
# Assumes that user has at some point joined the room if not is_guest.
|
# Assumes that user has at some point joined the room if not is_guest.
|
||||||
|
|
||||||
def allowed(event, membership, visibility):
|
def allowed(event, membership, visibility):
|
||||||
|
@ -114,17 +113,6 @@ class BaseHandler(object):
|
||||||
if should_include:
|
if should_include:
|
||||||
events_to_return.append(event)
|
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)
|
defer.returnValue(events_to_return)
|
||||||
|
|
||||||
def ratelimit(self, user_id):
|
def ratelimit(self, user_id):
|
||||||
|
|
|
@ -1,5 +1,5 @@
|
||||||
# -*- coding: utf-8 -*-
|
# -*- coding: utf-8 -*-
|
||||||
# Copyright 2014, 2015 OpenMarket Ltd
|
# Copyright 2014 - 2016 OpenMarket Ltd
|
||||||
#
|
#
|
||||||
# Licensed under the Apache License, Version 2.0 (the "License");
|
# Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
# you may not use this file except in compliance with the License.
|
# you may not use this file except in compliance with the License.
|
||||||
|
@ -685,7 +685,7 @@ class MessageHandler(BaseHandler):
|
||||||
).addErrback(unwrapFirstError)
|
).addErrback(unwrapFirstError)
|
||||||
|
|
||||||
messages = yield self._filter_events_for_client(
|
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])
|
start_token = now_token.copy_and_replace("room_key", token[0])
|
||||||
|
|
|
@ -879,14 +879,12 @@ class RoomContextHandler(BaseHandler):
|
||||||
user.to_string(),
|
user.to_string(),
|
||||||
results["events_before"],
|
results["events_before"],
|
||||||
is_guest=is_guest,
|
is_guest=is_guest,
|
||||||
require_all_visible_for_guests=False
|
|
||||||
)
|
)
|
||||||
|
|
||||||
results["events_after"] = yield self._filter_events_for_client(
|
results["events_after"] = yield self._filter_events_for_client(
|
||||||
user.to_string(),
|
user.to_string(),
|
||||||
results["events_after"],
|
results["events_after"],
|
||||||
is_guest=is_guest,
|
is_guest=is_guest,
|
||||||
require_all_visible_for_guests=False
|
|
||||||
)
|
)
|
||||||
|
|
||||||
if results["events_after"]:
|
if results["events_after"]:
|
||||||
|
|
|
@ -648,7 +648,6 @@ class SyncHandler(BaseHandler):
|
||||||
sync_config.user.to_string(),
|
sync_config.user.to_string(),
|
||||||
loaded_recents,
|
loaded_recents,
|
||||||
is_guest=sync_config.is_guest,
|
is_guest=sync_config.is_guest,
|
||||||
require_all_visible_for_guests=False
|
|
||||||
)
|
)
|
||||||
loaded_recents.extend(recents)
|
loaded_recents.extend(recents)
|
||||||
recents = loaded_recents
|
recents = loaded_recents
|
||||||
|
|
|
@ -1,5 +1,5 @@
|
||||||
# -*- coding: utf-8 -*-
|
# -*- coding: utf-8 -*-
|
||||||
# Copyright 2014, 2015 OpenMarket Ltd
|
# Copyright 2014 - 2016 OpenMarket Ltd
|
||||||
#
|
#
|
||||||
# Licensed under the Apache License, Version 2.0 (the "License");
|
# Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
# you may not use this file except in compliance with the License.
|
# you may not use this file except in compliance with the License.
|
||||||
|
@ -386,7 +386,6 @@ class Notifier(object):
|
||||||
user.to_string(),
|
user.to_string(),
|
||||||
new_events,
|
new_events,
|
||||||
is_guest=is_guest,
|
is_guest=is_guest,
|
||||||
require_all_visible_for_guests=False
|
|
||||||
)
|
)
|
||||||
|
|
||||||
events.extend(new_events)
|
events.extend(new_events)
|
||||||
|
|
Loading…
Reference in New Issue