From 6a4b650d8ad3e6c095020cac3861e430d643d53d Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Wed, 26 Aug 2015 13:22:23 +0100 Subject: Attempt to validate macaroons A couple of weird caveats: * If we can't validate your macaroon, we fall back to checking that your access token is in the DB, and ignoring the failure * Even if we can validate your macaroon, we still have to hit the DB to get the access token ID, which we pretend is a device ID all over the codebase. This mostly adds the interesting code, and points out the two pieces we need to delete (and necessary conditions) in order to fix the above caveats. --- tests/rest/client/v1/test_presence.py | 8 ++++---- tests/rest/client/v1/test_rooms.py | 28 ++++++++++++++-------------- tests/rest/client/v1/test_typing.py | 4 ++-- tests/rest/client/v1/utils.py | 3 --- tests/rest/client/v2_alpha/__init__.py | 4 ++-- 5 files changed, 22 insertions(+), 25 deletions(-) (limited to 'tests/rest') diff --git a/tests/rest/client/v1/test_presence.py b/tests/rest/client/v1/test_presence.py index 91547bdd06..d8d1416f59 100644 --- a/tests/rest/client/v1/test_presence.py +++ b/tests/rest/client/v1/test_presence.py @@ -72,11 +72,11 @@ class PresenceStateTestCase(unittest.TestCase): def _get_user_by_access_token(token=None): return { - "user": UserID.from_string(myid), + "user_id": UserID.from_string(myid), "token_id": 1, } - hs.get_v1auth().get_user_by_access_token = _get_user_by_access_token + hs.get_v1auth()._get_user_by_access_token = _get_user_by_access_token room_member_handler = hs.handlers.room_member_handler = Mock( spec=[ @@ -159,7 +159,7 @@ class PresenceListTestCase(unittest.TestCase): def _get_user_by_access_token(token=None): return { - "user": UserID.from_string(myid), + "user_id": UserID.from_string(myid), "token_id": 1, } @@ -169,7 +169,7 @@ class PresenceListTestCase(unittest.TestCase): ] ) - hs.get_v1auth().get_user_by_access_token = _get_user_by_access_token + hs.get_v1auth()._get_user_by_access_token = _get_user_by_access_token presence.register_servlets(hs, self.mock_resource) diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py index 34ab47d02e..be1d52f720 100644 --- a/tests/rest/client/v1/test_rooms.py +++ b/tests/rest/client/v1/test_rooms.py @@ -56,10 +56,10 @@ class RoomPermissionsTestCase(RestTestCase): def _get_user_by_access_token(token=None): return { - "user": UserID.from_string(self.auth_user_id), + "user_id": UserID.from_string(self.auth_user_id), "token_id": 1, } - hs.get_v1auth().get_user_by_access_token = _get_user_by_access_token + hs.get_v1auth()._get_user_by_access_token = _get_user_by_access_token def _insert_client_ip(*args, **kwargs): return defer.succeed(None) @@ -441,10 +441,10 @@ class RoomsMemberListTestCase(RestTestCase): def _get_user_by_access_token(token=None): return { - "user": UserID.from_string(self.auth_user_id), + "user_id": UserID.from_string(self.auth_user_id), "token_id": 1, } - hs.get_v1auth().get_user_by_access_token = _get_user_by_access_token + hs.get_v1auth()._get_user_by_access_token = _get_user_by_access_token def _insert_client_ip(*args, **kwargs): return defer.succeed(None) @@ -519,10 +519,10 @@ class RoomsCreateTestCase(RestTestCase): def _get_user_by_access_token(token=None): return { - "user": UserID.from_string(self.auth_user_id), + "user_id": UserID.from_string(self.auth_user_id), "token_id": 1, } - hs.get_v1auth().get_user_by_access_token = _get_user_by_access_token + hs.get_v1auth()._get_user_by_access_token = _get_user_by_access_token def _insert_client_ip(*args, **kwargs): return defer.succeed(None) @@ -610,11 +610,11 @@ class RoomTopicTestCase(RestTestCase): def _get_user_by_access_token(token=None): return { - "user": UserID.from_string(self.auth_user_id), + "user_id": UserID.from_string(self.auth_user_id), "token_id": 1, } - hs.get_v1auth().get_user_by_access_token = _get_user_by_access_token + hs.get_v1auth()._get_user_by_access_token = _get_user_by_access_token def _insert_client_ip(*args, **kwargs): return defer.succeed(None) @@ -715,10 +715,10 @@ class RoomMemberStateTestCase(RestTestCase): def _get_user_by_access_token(token=None): return { - "user": UserID.from_string(self.auth_user_id), + "user_id": UserID.from_string(self.auth_user_id), "token_id": 1, } - hs.get_v1auth().get_user_by_access_token = _get_user_by_access_token + hs.get_v1auth()._get_user_by_access_token = _get_user_by_access_token def _insert_client_ip(*args, **kwargs): return defer.succeed(None) @@ -840,10 +840,10 @@ class RoomMessagesTestCase(RestTestCase): def _get_user_by_access_token(token=None): return { - "user": UserID.from_string(self.auth_user_id), + "user_id": UserID.from_string(self.auth_user_id), "token_id": 1, } - hs.get_v1auth().get_user_by_access_token = _get_user_by_access_token + hs.get_v1auth()._get_user_by_access_token = _get_user_by_access_token def _insert_client_ip(*args, **kwargs): return defer.succeed(None) @@ -935,10 +935,10 @@ class RoomInitialSyncTestCase(RestTestCase): def _get_user_by_access_token(token=None): return { - "user": UserID.from_string(self.auth_user_id), + "user_id": UserID.from_string(self.auth_user_id), "token_id": 1, } - hs.get_v1auth().get_user_by_access_token = _get_user_by_access_token + hs.get_v1auth()._get_user_by_access_token = _get_user_by_access_token def _insert_client_ip(*args, **kwargs): return defer.succeed(None) diff --git a/tests/rest/client/v1/test_typing.py b/tests/rest/client/v1/test_typing.py index 1c4519406d..da6fc975f7 100644 --- a/tests/rest/client/v1/test_typing.py +++ b/tests/rest/client/v1/test_typing.py @@ -63,11 +63,11 @@ class RoomTypingTestCase(RestTestCase): def _get_user_by_access_token(token=None): return { - "user": UserID.from_string(self.auth_user_id), + "user_id": UserID.from_string(self.auth_user_id), "token_id": 1, } - hs.get_v1auth().get_user_by_access_token = _get_user_by_access_token + hs.get_v1auth()._get_user_by_access_token = _get_user_by_access_token def _insert_client_ip(*args, **kwargs): return defer.succeed(None) diff --git a/tests/rest/client/v1/utils.py b/tests/rest/client/v1/utils.py index c472d53043..85096a0326 100644 --- a/tests/rest/client/v1/utils.py +++ b/tests/rest/client/v1/utils.py @@ -37,9 +37,6 @@ class RestTestCase(unittest.TestCase): self.mock_resource = None self.auth_user_id = None - def mock_get_user_by_access_token(self, token=None): - return self.auth_user_id - @defer.inlineCallbacks def create_room_as(self, room_creator, is_public=True, tok=None): temp_id = self.auth_user_id diff --git a/tests/rest/client/v2_alpha/__init__.py b/tests/rest/client/v2_alpha/__init__.py index ef972a53aa..7d0f77a3ee 100644 --- a/tests/rest/client/v2_alpha/__init__.py +++ b/tests/rest/client/v2_alpha/__init__.py @@ -45,10 +45,10 @@ class V2AlphaRestTestCase(unittest.TestCase): def _get_user_by_access_token(token=None): return { - "user": UserID.from_string(self.USER_ID), + "user_id": UserID.from_string(self.USER_ID), "token_id": 1, } - hs.get_auth().get_user_by_access_token = _get_user_by_access_token + hs.get_auth()._get_user_by_access_token = _get_user_by_access_token for r in self.TO_REGISTER: r.register_servlets(hs, self.mock_resource) -- cgit 1.5.1 From e255c2c32ff85db03abbf2dac184b2949f481cfb Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Tue, 1 Sep 2015 12:41:16 +0100 Subject: s/user_id/user/g for consistency --- synapse/api/auth.py | 20 ++++++++++---------- tests/api/test_auth.py | 8 ++++---- tests/rest/client/v1/test_presence.py | 4 ++-- tests/rest/client/v1/test_rooms.py | 14 +++++++------- tests/rest/client/v1/test_typing.py | 2 +- tests/rest/client/v2_alpha/__init__.py | 2 +- 6 files changed, 25 insertions(+), 25 deletions(-) (limited to 'tests/rest') diff --git a/synapse/api/auth.py b/synapse/api/auth.py index f8ea1e2c69..0a77a76cb8 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -367,7 +367,7 @@ class Auth(object): pass # normal users won't have the user_id query parameter set. user_info = yield self._get_user_by_access_token(access_token) - user_id = user_info["user_id"] + user = user_info["user"] token_id = user_info["token_id"] ip_addr = self.hs.get_ip_from_request(request) @@ -375,17 +375,17 @@ class Auth(object): "User-Agent", default=[""] )[0] - if user_id and access_token and ip_addr: + if user and access_token and ip_addr: self.store.insert_client_ip( - user=user_id, + user=user, access_token=access_token, ip=ip_addr, user_agent=user_agent ) - request.authenticated_entity = user_id.to_string() + request.authenticated_entity = user.to_string() - defer.returnValue((user_id, token_id,)) + defer.returnValue((user, token_id,)) except KeyError: raise AuthError( self.TOKEN_NOT_FOUND_HTTP_STATUS, "Missing access token.", @@ -420,18 +420,18 @@ class Auth(object): user_prefix = "user_id = " for caveat in macaroon.caveats: if caveat.caveat_id.startswith(user_prefix): - user_id = UserID.from_string(caveat.caveat_id[len(user_prefix):]) + user = UserID.from_string(caveat.caveat_id[len(user_prefix):]) # This codepath exists so that we can actually return a # token ID, because we use token IDs in place of device # identifiers throughout the codebase. # TODO(daniel): Remove this fallback when device IDs are # properly implemented. ret = yield self._look_up_user_by_access_token(macaroon_str) - if ret["user_id"] != user_id: + if ret["user"] != user: logger.error( "Macaroon user (%s) != DB user (%s)", - user_id, - ret["user_id"] + user, + ret["user"] ) raise AuthError( self.TOKEN_NOT_FOUND_HTTP_STATUS, @@ -490,7 +490,7 @@ class Auth(object): errcode=Codes.UNKNOWN_TOKEN ) user_info = { - "user_id": UserID.from_string(ret.get("name")), + "user": UserID.from_string(ret.get("name")), "token_id": ret.get("token_id", None), } defer.returnValue(user_info) diff --git a/tests/api/test_auth.py b/tests/api/test_auth.py index 2e2d0c428a..c96273480d 100644 --- a/tests/api/test_auth.py +++ b/tests/api/test_auth.py @@ -146,17 +146,17 @@ class AuthTestCase(unittest.TestCase): return_value={"name": "@baldrick:matrix.org"} ) - user = "@baldrick:matrix.org" + user_id = "@baldrick:matrix.org" macaroon = pymacaroons.Macaroon( location=self.hs.config.server_name, identifier="key", key=self.hs.config.macaroon_secret_key) macaroon.add_first_party_caveat("gen = 1") macaroon.add_first_party_caveat("type = access") - macaroon.add_first_party_caveat("user_id = %s" % (user,)) + macaroon.add_first_party_caveat("user_id = %s" % (user_id,)) user_info = yield self.auth._get_user_from_macaroon(macaroon.serialize()) - user_id = user_info["user_id"] - self.assertEqual(UserID.from_string(user), user_id) + user = user_info["user"] + self.assertEqual(UserID.from_string(user_id), user) @defer.inlineCallbacks def test_get_user_from_macaroon_user_db_mismatch(self): diff --git a/tests/rest/client/v1/test_presence.py b/tests/rest/client/v1/test_presence.py index d8d1416f59..2ee3da0b34 100644 --- a/tests/rest/client/v1/test_presence.py +++ b/tests/rest/client/v1/test_presence.py @@ -72,7 +72,7 @@ class PresenceStateTestCase(unittest.TestCase): def _get_user_by_access_token(token=None): return { - "user_id": UserID.from_string(myid), + "user": UserID.from_string(myid), "token_id": 1, } @@ -159,7 +159,7 @@ class PresenceListTestCase(unittest.TestCase): def _get_user_by_access_token(token=None): return { - "user_id": UserID.from_string(myid), + "user": UserID.from_string(myid), "token_id": 1, } diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py index be1d52f720..9fb2bfb315 100644 --- a/tests/rest/client/v1/test_rooms.py +++ b/tests/rest/client/v1/test_rooms.py @@ -56,7 +56,7 @@ class RoomPermissionsTestCase(RestTestCase): def _get_user_by_access_token(token=None): return { - "user_id": UserID.from_string(self.auth_user_id), + "user": UserID.from_string(self.auth_user_id), "token_id": 1, } hs.get_v1auth()._get_user_by_access_token = _get_user_by_access_token @@ -441,7 +441,7 @@ class RoomsMemberListTestCase(RestTestCase): def _get_user_by_access_token(token=None): return { - "user_id": UserID.from_string(self.auth_user_id), + "user": UserID.from_string(self.auth_user_id), "token_id": 1, } hs.get_v1auth()._get_user_by_access_token = _get_user_by_access_token @@ -519,7 +519,7 @@ class RoomsCreateTestCase(RestTestCase): def _get_user_by_access_token(token=None): return { - "user_id": UserID.from_string(self.auth_user_id), + "user": UserID.from_string(self.auth_user_id), "token_id": 1, } hs.get_v1auth()._get_user_by_access_token = _get_user_by_access_token @@ -610,7 +610,7 @@ class RoomTopicTestCase(RestTestCase): def _get_user_by_access_token(token=None): return { - "user_id": UserID.from_string(self.auth_user_id), + "user": UserID.from_string(self.auth_user_id), "token_id": 1, } @@ -715,7 +715,7 @@ class RoomMemberStateTestCase(RestTestCase): def _get_user_by_access_token(token=None): return { - "user_id": UserID.from_string(self.auth_user_id), + "user": UserID.from_string(self.auth_user_id), "token_id": 1, } hs.get_v1auth()._get_user_by_access_token = _get_user_by_access_token @@ -840,7 +840,7 @@ class RoomMessagesTestCase(RestTestCase): def _get_user_by_access_token(token=None): return { - "user_id": UserID.from_string(self.auth_user_id), + "user": UserID.from_string(self.auth_user_id), "token_id": 1, } hs.get_v1auth()._get_user_by_access_token = _get_user_by_access_token @@ -935,7 +935,7 @@ class RoomInitialSyncTestCase(RestTestCase): def _get_user_by_access_token(token=None): return { - "user_id": UserID.from_string(self.auth_user_id), + "user": UserID.from_string(self.auth_user_id), "token_id": 1, } hs.get_v1auth()._get_user_by_access_token = _get_user_by_access_token diff --git a/tests/rest/client/v1/test_typing.py b/tests/rest/client/v1/test_typing.py index da6fc975f7..6395ce79db 100644 --- a/tests/rest/client/v1/test_typing.py +++ b/tests/rest/client/v1/test_typing.py @@ -63,7 +63,7 @@ class RoomTypingTestCase(RestTestCase): def _get_user_by_access_token(token=None): return { - "user_id": UserID.from_string(self.auth_user_id), + "user": UserID.from_string(self.auth_user_id), "token_id": 1, } diff --git a/tests/rest/client/v2_alpha/__init__.py b/tests/rest/client/v2_alpha/__init__.py index 7d0f77a3ee..f45570a1c0 100644 --- a/tests/rest/client/v2_alpha/__init__.py +++ b/tests/rest/client/v2_alpha/__init__.py @@ -45,7 +45,7 @@ class V2AlphaRestTestCase(unittest.TestCase): def _get_user_by_access_token(token=None): return { - "user_id": UserID.from_string(self.USER_ID), + "user": UserID.from_string(self.USER_ID), "token_id": 1, } hs.get_auth()._get_user_by_access_token = _get_user_by_access_token -- cgit 1.5.1 From bc8b25eb56bf4fcec3546c2ea28741189a519da5 Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Wed, 9 Sep 2015 15:42:16 +0100 Subject: Allow users that have left the room to view the member list from the point they left --- synapse/handlers/room.py | 36 ------------------------------------ synapse/rest/client/v1/room.py | 18 +++++++++++++----- tests/rest/client/v1/test_rooms.py | 4 ++-- 3 files changed, 15 insertions(+), 43 deletions(-) (limited to 'tests/rest') diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index c5d1001b50..0ff816d53e 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -25,7 +25,6 @@ from synapse.api.constants import ( from synapse.api.errors import StoreError, SynapseError from synapse.util import stringutils, unwrapFirstError from synapse.util.async import run_on_reactor -from synapse.events.utils import serialize_event from collections import OrderedDict import logging @@ -342,41 +341,6 @@ class RoomMemberHandler(BaseHandler): if remotedomains is not None: remotedomains.add(member.domain) - @defer.inlineCallbacks - def get_room_members_as_pagination_chunk(self, room_id=None, user_id=None, - limit=0, start_tok=None, - end_tok=None): - """Retrieve a list of room members in the room. - - Args: - room_id (str): The room to get the member list for. - user_id (str): The ID of the user making the request. - limit (int): The max number of members to return. - start_tok (str): Optional. The start token if known. - end_tok (str): Optional. The end token if known. - Returns: - dict: A Pagination streamable dict. - Raises: - SynapseError if something goes wrong. - """ - yield self.auth.check_joined_room(room_id, user_id) - - member_list = yield self.store.get_room_members(room_id=room_id) - time_now = self.clock.time_msec() - event_list = [ - serialize_event(entry, time_now) - for entry in member_list - ] - chunk_data = { - "start": "START", # FIXME (erikj): START is no longer valid - "end": "END", - "chunk": event_list - } - # TODO honor Pagination stream params - # TODO snapshot this list to return on subsequent requests when - # paginating - defer.returnValue(chunk_data) - @defer.inlineCallbacks def change_membership(self, event, context, do_auth=True): """ Change the membership status of a user in a room. diff --git a/synapse/rest/client/v1/room.py b/synapse/rest/client/v1/room.py index c9c27dd5a0..f4558b95a7 100644 --- a/synapse/rest/client/v1/room.py +++ b/synapse/rest/client/v1/room.py @@ -290,12 +290,18 @@ class RoomMemberListRestServlet(ClientV1RestServlet): def on_GET(self, request, room_id): # TODO support Pagination stream API (limit/tokens) user, _ = yield self.auth.get_user_by_req(request) - handler = self.handlers.room_member_handler - members = yield handler.get_room_members_as_pagination_chunk( + handler = self.handlers.message_handler + events = yield handler.get_state_events( room_id=room_id, - user_id=user.to_string()) + user_id=user.to_string(), + ) + + chunk = [] - for event in members["chunk"]: + for event in events: + if event["type"] != EventTypes.Member: + continue + chunk.append(event) # FIXME: should probably be state_key here, not user_id target_user = UserID.from_string(event["user_id"]) # Presence is an optional cache; don't fail if we can't fetch it @@ -308,7 +314,9 @@ class RoomMemberListRestServlet(ClientV1RestServlet): except: pass - defer.returnValue((200, members)) + defer.returnValue((200, { + "chunk": chunk + })) # TODO: Needs unit testing diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py index 34ab47d02e..d50cfe4298 100644 --- a/tests/rest/client/v1/test_rooms.py +++ b/tests/rest/client/v1/test_rooms.py @@ -492,9 +492,9 @@ class RoomsMemberListTestCase(RestTestCase): self.assertEquals(200, code, msg=str(response)) yield self.leave(room=room_id, user=self.user_id) - # can no longer see list, you've left. + # can see old list once left (code, response) = yield self.mock_resource.trigger_get(room_path) - self.assertEquals(403, code, msg=str(response)) + self.assertEquals(200, code, msg=str(response)) class RoomsCreateTestCase(RestTestCase): -- cgit 1.5.1 From e2054ce21a04f3d741293f50b283c01bbe2b0591 Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Thu, 10 Sep 2015 15:06:47 +0100 Subject: Allow users to GET individual state events for rooms that they have left --- synapse/handlers/message.py | 20 +++++++++++++------- tests/rest/client/v1/test_rooms.py | 10 +++++----- 2 files changed, 18 insertions(+), 12 deletions(-) (limited to 'tests/rest') diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index db89491b46..5d18aaacf0 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -16,7 +16,7 @@ from twisted.internet import defer from synapse.api.constants import EventTypes, Membership -from synapse.api.errors import RoomError, SynapseError +from synapse.api.errors import SynapseError from synapse.streams.config import PaginationConfig from synapse.events.utils import serialize_event from synapse.events.validator import EventValidator @@ -277,13 +277,19 @@ class MessageHandler(BaseHandler): Raises: SynapseError if something went wrong. """ - have_joined = yield self.auth.check_joined_room(room_id, user_id) - if not have_joined: - raise RoomError(403, "User not in room.") + member_event = yield self.auth.check_user_was_in_room(room_id, user_id) + + if member_event.membership == Membership.JOIN: + data = yield self.state_handler.get_current_state( + room_id, event_type, state_key + ) + elif member_event.membership == Membership.LEAVE: + key = (event_type, state_key) + room_state = yield self.store.get_state_for_events( + room_id, [member_event.event_id], [key] + ) + data = room_state[member_event.event_id].get(key) - data = yield self.state_handler.get_current_state( - room_id, event_type, state_key - ) defer.returnValue(data) @defer.inlineCallbacks diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py index d50cfe4298..ed0ac8d5c8 100644 --- a/tests/rest/client/v1/test_rooms.py +++ b/tests/rest/client/v1/test_rooms.py @@ -239,7 +239,7 @@ class RoomPermissionsTestCase(RestTestCase): "PUT", topic_path, topic_content) self.assertEquals(403, code, msg=str(response)) (code, response) = yield self.mock_resource.trigger_get(topic_path) - self.assertEquals(403, code, msg=str(response)) + self.assertEquals(200, code, msg=str(response)) # get topic in PUBLIC room, not joined, expect 403 (code, response) = yield self.mock_resource.trigger_get( @@ -301,11 +301,11 @@ class RoomPermissionsTestCase(RestTestCase): room=room, expect_code=200) # get membership of self, get membership of other, private room + left - # expect all 403s + # expect all 200s yield self.leave(room=room, user=self.user_id) yield self._test_get_membership( members=[self.user_id, self.rmcreator_id], - room=room, expect_code=403) + room=room, expect_code=200) @defer.inlineCallbacks def test_membership_public_room_perms(self): @@ -326,11 +326,11 @@ class RoomPermissionsTestCase(RestTestCase): room=room, expect_code=200) # get membership of self, get membership of other, public room + left - # expect 403. + # expect 200. yield self.leave(room=room, user=self.user_id) yield self._test_get_membership( members=[self.user_id, self.rmcreator_id], - room=room, expect_code=403) + room=room, expect_code=200) @defer.inlineCallbacks def test_invited_permissions(self): -- cgit 1.5.1