From afbd3b2fc4834a0f03236b550892e0f1d96b54c3 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 28 May 2015 18:05:00 +0100 Subject: SYN-395: Fix CAPTCHA, don't double decode json --- synapse/handlers/auth.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'synapse/handlers') diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 4e2e50345e..0cc28248a9 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -187,7 +187,7 @@ class AuthHandler(BaseHandler): # each request try: client = SimpleHttpClient(self.hs) - data = yield client.post_urlencoded_get_json( + resp_body = yield client.post_urlencoded_get_json( "https://www.google.com/recaptcha/api/siteverify", args={ 'secret': self.hs.config.recaptcha_private_key, @@ -198,7 +198,8 @@ class AuthHandler(BaseHandler): except PartialDownloadError as pde: # Twisted is silly data = pde.response - resp_body = simplejson.loads(data) + resp_body = simplejson.loads(data) + if 'success' in resp_body and resp_body['success']: defer.returnValue(True) raise LoginError(401, "", errcode=Codes.UNAUTHORIZED) -- cgit 1.5.1 From d94590ed4855691e54655396e45e3e275f9c1860 Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Fri, 29 May 2015 12:11:40 +0100 Subject: Add config for setting the recaptcha verify api endpoint, so we can test it in sytest --- synapse/config/captcha.py | 4 ++++ synapse/handlers/auth.py | 6 +++--- 2 files changed, 7 insertions(+), 3 deletions(-) (limited to 'synapse/handlers') diff --git a/synapse/config/captcha.py b/synapse/config/captcha.py index d8fe577e34..ba221121cb 100644 --- a/synapse/config/captcha.py +++ b/synapse/config/captcha.py @@ -26,6 +26,7 @@ class CaptchaConfig(Config): config["captcha_ip_origin_is_x_forwarded"] ) self.captcha_bypass_secret = config.get("captcha_bypass_secret") + self.recaptcha_siteverify_api = config["recaptcha_siteverify_api"] def default_config(self, config_dir_path, server_name): return """\ @@ -48,4 +49,7 @@ class CaptchaConfig(Config): # A secret key used to bypass the captcha test entirely. #captcha_bypass_secret: "YOUR_SECRET_HERE" + + # The API endpoint to use for verifying m.login.recaptcha responses. + recaptcha_siteverify_api: "https://www.google.com/recaptcha/api/siteverify" """ diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 4e2e50345e..4b442a8358 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -187,8 +187,8 @@ class AuthHandler(BaseHandler): # each request try: client = SimpleHttpClient(self.hs) - data = yield client.post_urlencoded_get_json( - "https://www.google.com/recaptcha/api/siteverify", + resp_body = yield client.post_urlencoded_get_json( + self.hs.config.recaptcha_siteverify_api, args={ 'secret': self.hs.config.recaptcha_private_key, 'response': user_response, @@ -198,7 +198,7 @@ class AuthHandler(BaseHandler): except PartialDownloadError as pde: # Twisted is silly data = pde.response - resp_body = simplejson.loads(data) + resp_body = simplejson.loads(data) if 'success' in resp_body and resp_body['success']: defer.returnValue(True) raise LoginError(401, "", errcode=Codes.UNAUTHORIZED) -- cgit 1.5.1 From 4bbfbf898ecfe9bd77b02a7e014d94163d1fd534 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 1 Jun 2015 17:02:23 +0100 Subject: Correctly pass in auth_events --- synapse/handlers/federation.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'synapse/handlers') diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 46ce3699d7..caf6a1f8eb 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -262,7 +262,13 @@ class FederationHandler(BaseHandler): yield defer.gatherResults( [ - self._handle_new_event(dest, a) + self._handle_new_event( + dest, a, + auth_events={ + (e.type, e.state_key): e for e in auth_events + if e.event_id in [a_id for a_id, _ in a.auth_events] + } + ) for a in auth_events.values() ], consumeErrors=True, @@ -274,6 +280,10 @@ class FederationHandler(BaseHandler): dest, event_map[e_id], state=events_to_state[e_id], backfilled=True, + auth_events={ + (e.type, e.state_key): e for e in auth_events + if e.event_id in [a_id for a_id, _ in a.auth_events] + } ) for e_id in events_to_state ], -- cgit 1.5.1 From 3f04a08a0c922962e3a5439b1740424202dc4616 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 2 Jun 2015 10:11:32 +0100 Subject: Don't process events we've already processed. Remember to process state events --- synapse/handlers/federation.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'synapse/handlers') diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index caf6a1f8eb..acbb53d6c5 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -250,6 +250,7 @@ class FederationHandler(BaseHandler): # For each edge get the current state. auth_events = {} + state_events = {} events_to_state = {} for e_id in edges: state, auth = yield self.replication_layer.get_state_for_room( @@ -258,8 +259,13 @@ class FederationHandler(BaseHandler): event_id=e_id ) auth_events.update({a.event_id: a for a in auth}) + state_events.update({s.event_id: s for s in state}) events_to_state[e_id] = state + seen_events = yield self.store.have_events( + set(auth_events.keys()) | set(state_events.keys()) + ) + yield defer.gatherResults( [ self._handle_new_event( @@ -270,6 +276,22 @@ class FederationHandler(BaseHandler): } ) for a in auth_events.values() + if a.event_id not in seen_events + ], + consumeErrors=True, + ).addErrback(unwrapFirstError) + + yield defer.gatherResults( + [ + self._handle_new_event( + dest, s, + auth_events={ + (e.type, e.state_key): e for e in auth_events + if e.event_id in [a_id for a_id, _ in s.auth_events] + } + ) + for s in state_events.values() + if s.event_id not in seen_events ], consumeErrors=True, ).addErrback(unwrapFirstError) -- cgit 1.5.1 From fde0da6f19aeb6dee26fc0b89fcee9d27a50b8f4 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 2 Jun 2015 10:19:38 +0100 Subject: Correctly look up auth_events --- synapse/handlers/federation.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) (limited to 'synapse/handlers') diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index acbb53d6c5..5cd853d85e 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -271,9 +271,10 @@ class FederationHandler(BaseHandler): self._handle_new_event( dest, a, auth_events={ - (e.type, e.state_key): e for e in auth_events - if e.event_id in [a_id for a_id, _ in a.auth_events] - } + (auth_events[a_id].type, auth_events[a_id].state_key): + auth_events[a_id] + for a_id, _ in a.auth_events + }, ) for a in auth_events.values() if a.event_id not in seen_events @@ -286,9 +287,10 @@ class FederationHandler(BaseHandler): self._handle_new_event( dest, s, auth_events={ - (e.type, e.state_key): e for e in auth_events - if e.event_id in [a_id for a_id, _ in s.auth_events] - } + (auth_events[a_id].type, auth_events[a_id].state_key): + auth_events[a_id] + for a_id, _ in s.auth_events + }, ) for s in state_events.values() if s.event_id not in seen_events @@ -303,9 +305,10 @@ class FederationHandler(BaseHandler): state=events_to_state[e_id], backfilled=True, auth_events={ - (e.type, e.state_key): e for e in auth_events - if e.event_id in [a_id for a_id, _ in a.auth_events] - } + (auth_events[a_id].type, auth_events[a_id].state_key): + auth_events[a_id] + for a_id, _ in event_map[e_id].auth_events + }, ) for e_id in events_to_state ], -- cgit 1.5.1 From e552b78d50a2f162a536057af48c7815fb181c30 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 2 Jun 2015 10:28:14 +0100 Subject: Add some logging --- synapse/handlers/federation.py | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'synapse/handlers') diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 5cd853d85e..ced6fab16f 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -247,6 +247,11 @@ class FederationHandler(BaseHandler): if set(e_id for e_id, _ in ev.prev_events) - event_ids ] + logger.info( + "backfill: Got %d events with %d edges", + len(events), len(edges), + ) + # For each edge get the current state. auth_events = {} -- cgit 1.5.1 From 02410e92395f53b88017bf93a82be41afe1a839e Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 2 Jun 2015 10:58:35 +0100 Subject: Handle the fact we might be missing auth events --- synapse/handlers/federation.py | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) (limited to 'synapse/handlers') diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index ced6fab16f..b333175349 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -264,6 +264,7 @@ class FederationHandler(BaseHandler): event_id=e_id ) auth_events.update({a.event_id: a for a in auth}) + auth_events.update({s.event_id: s for s in state}) state_events.update({s.event_id: s for s in state}) events_to_state[e_id] = state @@ -271,34 +272,37 @@ class FederationHandler(BaseHandler): set(auth_events.keys()) | set(state_events.keys()) ) - yield defer.gatherResults( + all_events = events + state_events.values() + auth_events.values() + required_auth = set( + a_id for event in all_events for a_id, _ in event.auth_events + ) + + missing_auth = required_auth - set(auth_events) + results = yield defer.gatherResults( [ - self._handle_new_event( - dest, a, - auth_events={ - (auth_events[a_id].type, auth_events[a_id].state_key): - auth_events[a_id] - for a_id, _ in a.auth_events - }, + self.replication_layer.get_pdu( + [dest], + event_id, + outlier=True, ) - for a in auth_events.values() - if a.event_id not in seen_events + for event_id in missing_auth ], - consumeErrors=True, + consumeErrors=True ).addErrback(unwrapFirstError) + auth_events.update({a.event_id: a for a in results}) yield defer.gatherResults( [ self._handle_new_event( - dest, s, + dest, a, auth_events={ (auth_events[a_id].type, auth_events[a_id].state_key): auth_events[a_id] - for a_id, _ in s.auth_events + for a_id, _ in a.auth_events }, ) - for s in state_events.values() - if s.event_id not in seen_events + for a in auth_events.values() + if a.event_id not in seen_events ], consumeErrors=True, ).addErrback(unwrapFirstError) -- cgit 1.5.1 From 09e23334decf6e01533d2c15f49f418e9e8df6e5 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 2 Jun 2015 11:00:37 +0100 Subject: Add a timeout --- synapse/handlers/federation.py | 1 + 1 file changed, 1 insertion(+) (limited to 'synapse/handlers') diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index b333175349..e8e3173ca2 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -284,6 +284,7 @@ class FederationHandler(BaseHandler): [dest], event_id, outlier=True, + timeout=10000, ) for event_id in missing_auth ], -- cgit 1.5.1 From 1c3d844e7314dd5c1722ed77daf4bad8a056217d Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 3 Jun 2015 16:30:01 +0100 Subject: Don't needlessly compute context --- synapse/handlers/federation.py | 6 ++++-- synapse/state.py | 15 +++++++++++++-- tests/handlers/test_federation.py | 4 ++-- 3 files changed, 19 insertions(+), 6 deletions(-) (limited to 'synapse/handlers') diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 46ce3699d7..5503d9ae86 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -900,8 +900,10 @@ class FederationHandler(BaseHandler): event.event_id, event.signatures, ) + outlier = event.internal_metadata.is_outlier() + context = yield self.state_handler.compute_event_context( - event, old_state=state + event, old_state=state, outlier=outlier, ) if not auth_events: @@ -912,7 +914,7 @@ class FederationHandler(BaseHandler): event.event_id, auth_events, ) - is_new_state = not event.internal_metadata.is_outlier() + is_new_state = not outlier # This is a hack to fix some old rooms where the initial join event # didn't reference the create event in its auth events. diff --git a/synapse/state.py b/synapse/state.py index 9dddb77d5b..c1ce46d1b2 100644 --- a/synapse/state.py +++ b/synapse/state.py @@ -106,7 +106,7 @@ class StateHandler(object): defer.returnValue(state) @defer.inlineCallbacks - def compute_event_context(self, event, old_state=None): + def compute_event_context(self, event, old_state=None, outlier=False): """ Fills out the context with the `current state` of the graph. The `current state` here is defined to be the state of the event graph just before the event - i.e. it never includes `event` @@ -119,9 +119,20 @@ class StateHandler(object): Returns: an EventContext """ + yield run_on_reactor() + context = EventContext() - yield run_on_reactor() + if outlier: + if old_state: + context.current_state = { + (s.type, s.state_key): s for s in old_state + } + else: + context.current_state = {} + context.prev_state_events = [] + context.state_group = None + defer.returnValue(context) if old_state: context.current_state = { diff --git a/tests/handlers/test_federation.py b/tests/handlers/test_federation.py index f3821242bc..d392c23015 100644 --- a/tests/handlers/test_federation.py +++ b/tests/handlers/test_federation.py @@ -100,7 +100,7 @@ class FederationTestCase(unittest.TestCase): return defer.succeed({}) self.datastore.have_events.side_effect = have_events - def annotate(ev, old_state=None): + def annotate(ev, old_state=None, outlier=False): context = Mock() context.current_state = {} context.auth_events = {} @@ -120,7 +120,7 @@ class FederationTestCase(unittest.TestCase): ) self.state_handler.compute_event_context.assert_called_once_with( - ANY, old_state=None, + ANY, old_state=None, outlier=False ) self.auth.check.assert_called_once_with(ANY, auth_events={}) -- cgit 1.5.1 From 55bf90b9e458c6adbbd0e82403f9da06da95a812 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 3 Jun 2015 16:06:39 +0100 Subject: Don't needlessly compute prev_state --- synapse/handlers/_base.py | 4 +++- synapse/state.py | 4 ---- tests/handlers/test_room.py | 2 ++ 3 files changed, 5 insertions(+), 5 deletions(-) (limited to 'synapse/handlers') diff --git a/synapse/handlers/_base.py b/synapse/handlers/_base.py index 833ff41377..d6c064b398 100644 --- a/synapse/handlers/_base.py +++ b/synapse/handlers/_base.py @@ -78,7 +78,9 @@ class BaseHandler(object): context = yield state_handler.compute_event_context(builder) if builder.is_state(): - builder.prev_state = context.prev_state_events + builder.prev_state = yield self.store.add_event_hashes( + context.prev_state_events + ) yield self.auth.add_auth_events(builder, context) diff --git a/synapse/state.py b/synapse/state.py index 9dddb77d5b..d4d8930001 100644 --- a/synapse/state.py +++ b/synapse/state.py @@ -155,10 +155,6 @@ class StateHandler(object): context.current_state = curr_state context.state_group = group if not event.is_state() else None - prev_state = yield self.store.add_event_hashes( - prev_state - ) - if event.is_state(): key = (event.type, event.state_key) if key in context.current_state: diff --git a/tests/handlers/test_room.py b/tests/handlers/test_room.py index a2d7635995..2a7553f982 100644 --- a/tests/handlers/test_room.py +++ b/tests/handlers/test_room.py @@ -42,6 +42,7 @@ class RoomMemberHandlerTestCase(unittest.TestCase): "get_room", "store_room", "get_latest_events_in_room", + "add_event_hashes", ]), resource_for_federation=NonCallableMock(), http_client=NonCallableMock(spec_set=[]), @@ -88,6 +89,7 @@ class RoomMemberHandlerTestCase(unittest.TestCase): self.ratelimiter.send_message.return_value = (True, 0) self.datastore.persist_event.return_value = (1,1) + self.datastore.add_event_hashes.return_value = [] @defer.inlineCallbacks def test_invite(self): -- cgit 1.5.1