From 62073992c523cc9b40c342c8443966cda7d8b5a4 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 14 Oct 2016 13:56:53 +0100 Subject: Make password reset email field case insensitive --- synapse/storage/registration.py | 30 ++++++++++++++-------- .../36/user_threepids_medium_address_index.sql | 16 ++++++++++++ 2 files changed, 36 insertions(+), 10 deletions(-) create mode 100644 synapse/storage/schema/delta/36/user_threepids_medium_address_index.sql (limited to 'synapse/storage') diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py index e404fa72de..a6aa64f9fb 100644 --- a/synapse/storage/registration.py +++ b/synapse/storage/registration.py @@ -458,17 +458,27 @@ class RegistrationStore(background_updates.BackgroundUpdateStore): @defer.inlineCallbacks def get_user_id_by_threepid(self, medium, address): - ret = yield self._simple_select_one( - "user_threepids", - { - "medium": medium, - "address": address - }, - ['user_id'], True, 'get_user_id_by_threepid' + def f(txn): + sql = ( + "SELECT user_id" + " FROM user_threepids" + " WHERE medium = ? AND LOWER(address) = LOWER(?)" + ) + txn.execute(sql, (medium, address)) + row = txn.fetchone() + if not row: + return None + if txn.rowcount > 1: + raise StoreError(500, "More than one row matched") + return { + "user_id": row[0] + } + + res = yield self.runInteraction( + "get_user_id_by_threepid", f ) - if ret: - defer.returnValue(ret['user_id']) - defer.returnValue(None) + + defer.returnValue(res) def user_delete_threepids(self, user_id): return self._simple_delete( diff --git a/synapse/storage/schema/delta/36/user_threepids_medium_address_index.sql b/synapse/storage/schema/delta/36/user_threepids_medium_address_index.sql new file mode 100644 index 0000000000..702a872784 --- /dev/null +++ b/synapse/storage/schema/delta/36/user_threepids_medium_address_index.sql @@ -0,0 +1,16 @@ +/* Copyright 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +CREATE INDEX user_threepids_medium_address on user_threepids (medium, LOWER(address)); -- cgit 1.5.1 From 29c592202136a3bdb04f78a49d02b7b53893a973 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 14 Oct 2016 16:20:24 +0100 Subject: Revert part of 6207399 older sqlite doesn't support indexes on expressions, lets just store things lowercase in the db --- synapse/storage/registration.py | 30 ++++++++++-------------------- 1 file changed, 10 insertions(+), 20 deletions(-) (limited to 'synapse/storage') diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py index a6aa64f9fb..e404fa72de 100644 --- a/synapse/storage/registration.py +++ b/synapse/storage/registration.py @@ -458,27 +458,17 @@ class RegistrationStore(background_updates.BackgroundUpdateStore): @defer.inlineCallbacks def get_user_id_by_threepid(self, medium, address): - def f(txn): - sql = ( - "SELECT user_id" - " FROM user_threepids" - " WHERE medium = ? AND LOWER(address) = LOWER(?)" - ) - txn.execute(sql, (medium, address)) - row = txn.fetchone() - if not row: - return None - if txn.rowcount > 1: - raise StoreError(500, "More than one row matched") - return { - "user_id": row[0] - } - - res = yield self.runInteraction( - "get_user_id_by_threepid", f + ret = yield self._simple_select_one( + "user_threepids", + { + "medium": medium, + "address": address + }, + ['user_id'], True, 'get_user_id_by_threepid' ) - - defer.returnValue(res) + if ret: + defer.returnValue(ret['user_id']) + defer.returnValue(None) def user_delete_threepids(self, user_id): return self._simple_delete( -- cgit 1.5.1 From e8b1d2a45200d88a576360a83e3dff1aac4ad679 Mon Sep 17 00:00:00 2001 From: pik Date: Tue, 18 Oct 2016 12:17:38 -0500 Subject: Refactor test_filter to use real DataStore * add tests for filter api errors --- synapse/rest/client/v2_alpha/filter.py | 4 +- synapse/storage/_base.py | 1 - tests/rest/client/v2_alpha/test_filter.py | 124 +++++++++++++++++++----------- 3 files changed, 83 insertions(+), 46 deletions(-) (limited to 'synapse/storage') diff --git a/synapse/rest/client/v2_alpha/filter.py b/synapse/rest/client/v2_alpha/filter.py index f7758fc68c..b4084fec62 100644 --- a/synapse/rest/client/v2_alpha/filter.py +++ b/synapse/rest/client/v2_alpha/filter.py @@ -74,6 +74,7 @@ class CreateFilterRestServlet(RestServlet): @defer.inlineCallbacks def on_POST(self, request, user_id): + target_user = UserID.from_string(user_id) requester = yield self.auth.get_user_by_req(request) @@ -81,10 +82,9 @@ class CreateFilterRestServlet(RestServlet): raise AuthError(403, "Cannot create filters for other users") if not self.hs.is_mine(target_user): - raise SynapseError(400, "Can only create filters for local users") + raise AuthError(403, "Can only create filters for local users") content = parse_json_object_from_request(request) - filter_id = yield self.filtering.add_user_filter( user_localpart=target_user.localpart, user_filter=content, diff --git a/synapse/storage/_base.py b/synapse/storage/_base.py index 49fa8614f2..d828d6ee1d 100644 --- a/synapse/storage/_base.py +++ b/synapse/storage/_base.py @@ -85,7 +85,6 @@ class LoggingTransaction(object): sql_logger.debug("[SQL] {%s} %s", self.name, sql) sql = self.database_engine.convert_param_style(sql) - if args: try: sql_logger.debug( diff --git a/tests/rest/client/v2_alpha/test_filter.py b/tests/rest/client/v2_alpha/test_filter.py index 47ca5e8c8a..3d27d03cbf 100644 --- a/tests/rest/client/v2_alpha/test_filter.py +++ b/tests/rest/client/v2_alpha/test_filter.py @@ -15,87 +15,125 @@ from twisted.internet import defer -from . import V2AlphaRestTestCase +from tests import unittest from synapse.rest.client.v2_alpha import filter -from synapse.api.errors import StoreError, Codes +from synapse.api.errors import Codes +import synapse.types + +from synapse.types import UserID + +from ....utils import MockHttpResource, setup_test_homeserver + +PATH_PREFIX = "/_matrix/client/v2_alpha" + + +class FilterTestCase(unittest.TestCase): -class FilterTestCase(V2AlphaRestTestCase): USER_ID = "@apple:test" + EXAMPLE_FILTER = {"type": ["m.*"]} + EXAMPLE_FILTER_JSON = '{"type": ["m.*"]}' TO_REGISTER = [filter] - def make_datastore_mock(self): - datastore = super(FilterTestCase, self).make_datastore_mock() + @defer.inlineCallbacks + def setUp(self): + self.mock_resource = MockHttpResource(prefix=PATH_PREFIX) + + self.hs = yield setup_test_homeserver( + http_client=None, + resource_for_client=self.mock_resource, + resource_for_federation=self.mock_resource, + ) + + self.auth = self.hs.get_auth() - self._user_filters = {} + def get_user_by_access_token(token=None, allow_guest=False): + return { + "user": UserID.from_string(self.USER_ID), + "token_id": 1, + "is_guest": False, + } - def add_user_filter(user_localpart, definition): - filters = self._user_filters.setdefault(user_localpart, []) - filter_id = len(filters) - filters.append(definition) - return defer.succeed(filter_id) - datastore.add_user_filter = add_user_filter + def get_user_by_req(request, allow_guest=False, rights="access"): + return synapse.types.create_requester( + UserID.from_string(self.USER_ID), 1, False, None) - def get_user_filter(user_localpart, filter_id): - if user_localpart not in self._user_filters: - raise StoreError(404, "No user") - filters = self._user_filters[user_localpart] - if filter_id >= len(filters): - raise StoreError(404, "No filter") - return defer.succeed(filters[filter_id]) - datastore.get_user_filter = get_user_filter + self.auth.get_user_by_access_token = get_user_by_access_token + self.auth.get_user_by_req = get_user_by_req - return datastore + self.store = self.hs.get_datastore() + self.filtering = self.hs.get_filtering() + + for r in self.TO_REGISTER: + r.register_servlets(self.hs, self.mock_resource) @defer.inlineCallbacks def test_add_filter(self): (code, response) = yield self.mock_resource.trigger( - "POST", "/user/%s/filter" % (self.USER_ID), '{"type": ["m.*"]}' + "POST", "/user/%s/filter" % (self.USER_ID), self.EXAMPLE_FILTER_JSON ) self.assertEquals(200, code) self.assertEquals({"filter_id": "0"}, response) + filter = yield self.store.get_user_filter( + user_localpart='apple', + filter_id=0, + ) + self.assertEquals(filter, self.EXAMPLE_FILTER) + + @defer.inlineCallbacks + def test_add_filter_for_other_user(self): + (code, response) = yield self.mock_resource.trigger( + "POST", "/user/%s/filter" % ('@watermelon:test'), self.EXAMPLE_FILTER_JSON + ) + self.assertEquals(403, code) + self.assertEquals(response['errcode'], Codes.FORBIDDEN) - self.assertIn("apple", self._user_filters) - self.assertEquals(len(self._user_filters["apple"]), 1) - self.assertEquals({"type": ["m.*"]}, self._user_filters["apple"][0]) + @defer.inlineCallbacks + def test_add_filter_non_local_user(self): + _is_mine = self.hs.is_mine + self.hs.is_mine = lambda target_user: False + (code, response) = yield self.mock_resource.trigger( + "POST", "/user/%s/filter" % (self.USER_ID), self.EXAMPLE_FILTER_JSON + ) + self.hs.is_mine = _is_mine + self.assertEquals(403, code) + self.assertEquals(response['errcode'], Codes.FORBIDDEN) @defer.inlineCallbacks def test_get_filter(self): - self._user_filters["apple"] = [ - {"type": ["m.*"]} - ] - + filter_id = yield self.filtering.add_user_filter( + user_localpart='apple', + user_filter=self.EXAMPLE_FILTER + ) (code, response) = yield self.mock_resource.trigger_get( - "/user/%s/filter/0" % (self.USER_ID) + "/user/%s/filter/%s" % (self.USER_ID, filter_id) ) self.assertEquals(200, code) - self.assertEquals({"type": ["m.*"]}, response) + self.assertEquals(self.EXAMPLE_FILTER, response) @defer.inlineCallbacks - def test_get_filter_no_id(self): - self._user_filters["apple"] = [ - {"type": ["m.*"]} - ] - + def test_get_filter_non_existant(self): (code, response) = yield self.mock_resource.trigger_get( - "/user/%s/filter/2" % (self.USER_ID) + "/user/%s/filter/12382148321" % (self.USER_ID) ) self.assertEquals(400, code) + self.assertEquals(response['errcode'], Codes.NOT_FOUND) + # Currently invalid params do not have an appropriate errcode + # in errors.py @defer.inlineCallbacks - def test_get_filter_no_user(self): + def test_get_filter_invalid_id(self): (code, response) = yield self.mock_resource.trigger_get( - "/user/%s/filter/0" % (self.USER_ID) + "/user/%s/filter/foobar" % (self.USER_ID) ) self.assertEquals(400, code) - self.assertEquals(response['errcode'], Codes.FORBIDDEN) + # No ID also returns an invalid_id error @defer.inlineCallbacks - def test_get_filter_missing_id(self): + def test_get_filter_no_id(self): (code, response) = yield self.mock_resource.trigger_get( - "/user/%s/filter/0" % (self.USER_ID) + "/user/%s/filter/" % (self.USER_ID) ) self.assertEquals(400, code) - self.assertEquals(response['errcode'], Codes.NOT_FOUND) -- cgit 1.5.1 From df2a616c7b028a6eb8b50c57e7e73847287a6feb Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 19 Oct 2016 11:13:55 +0100 Subject: Convert emails to lowercase when storing And db migration sql to convert existing addresses. --- synapse/handlers/auth.py | 12 +++++++++++ synapse/storage/schema/delta/36/user_threepids.sql | 23 ++++++++++++++++++++++ .../36/user_threepids_medium_address_index.sql | 16 --------------- 3 files changed, 35 insertions(+), 16 deletions(-) create mode 100644 synapse/storage/schema/delta/36/user_threepids.sql delete mode 100644 synapse/storage/schema/delta/36/user_threepids_medium_address_index.sql (limited to 'synapse/storage') diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index dc0fe60e1b..3635521230 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -611,6 +611,18 @@ class AuthHandler(BaseHandler): @defer.inlineCallbacks def add_threepid(self, user_id, medium, address, validated_at): + # 'Canonicalise' email addresses down to lower case. + # We've now moving towards the Home Server being the entity that + # is responsible for validating threepids used for resetting passwords + # on accounts, so in future Synapse will gain knowledge of specific + # types (mediums) of threepid. For now, we still use the existing + # infrastructure, but this is the start of synapse gaining knowledge + # of specific types of threepid (and fixes the fact that checking + # for the presenc eof an email address during password reset was + # case sensitive). + if medium == 'email': + address = address.lower() + yield self.store.user_add_threepid( user_id, medium, address, validated_at, self.hs.get_clock().time_msec() diff --git a/synapse/storage/schema/delta/36/user_threepids.sql b/synapse/storage/schema/delta/36/user_threepids.sql new file mode 100644 index 0000000000..ef8813e72a --- /dev/null +++ b/synapse/storage/schema/delta/36/user_threepids.sql @@ -0,0 +1,23 @@ +/* Copyright 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/* + * Update any email addresses that were stored with mixed case into all + * lowercase + */ +UPDATE user_threepids SET address = LOWER(address) where medium = 'email'; + +/* Add an index for the select we do on passwored reset */ +CREATE INDEX user_threepids_medium_address on user_threepids (medium, address); diff --git a/synapse/storage/schema/delta/36/user_threepids_medium_address_index.sql b/synapse/storage/schema/delta/36/user_threepids_medium_address_index.sql deleted file mode 100644 index 702a872784..0000000000 --- a/synapse/storage/schema/delta/36/user_threepids_medium_address_index.sql +++ /dev/null @@ -1,16 +0,0 @@ -/* Copyright 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. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -CREATE INDEX user_threepids_medium_address on user_threepids (medium, LOWER(address)); -- cgit 1.5.1 From 0108ed8ae6430b551a6d8e9f05820c615631a84b Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 19 Oct 2016 11:40:35 +0100 Subject: Latest delta is now 37 --- synapse/storage/schema/delta/36/user_threepids.sql | 23 ---------------------- synapse/storage/schema/delta/37/user_threepids.sql | 23 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 23 deletions(-) delete mode 100644 synapse/storage/schema/delta/36/user_threepids.sql create mode 100644 synapse/storage/schema/delta/37/user_threepids.sql (limited to 'synapse/storage') diff --git a/synapse/storage/schema/delta/36/user_threepids.sql b/synapse/storage/schema/delta/36/user_threepids.sql deleted file mode 100644 index ef8813e72a..0000000000 --- a/synapse/storage/schema/delta/36/user_threepids.sql +++ /dev/null @@ -1,23 +0,0 @@ -/* Copyright 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. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -/* - * Update any email addresses that were stored with mixed case into all - * lowercase - */ -UPDATE user_threepids SET address = LOWER(address) where medium = 'email'; - -/* Add an index for the select we do on passwored reset */ -CREATE INDEX user_threepids_medium_address on user_threepids (medium, address); diff --git a/synapse/storage/schema/delta/37/user_threepids.sql b/synapse/storage/schema/delta/37/user_threepids.sql new file mode 100644 index 0000000000..ef8813e72a --- /dev/null +++ b/synapse/storage/schema/delta/37/user_threepids.sql @@ -0,0 +1,23 @@ +/* Copyright 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/* + * Update any email addresses that were stored with mixed case into all + * lowercase + */ +UPDATE user_threepids SET address = LOWER(address) where medium = 'email'; + +/* Add an index for the select we do on passwored reset */ +CREATE INDEX user_threepids_medium_address on user_threepids (medium, address); -- cgit 1.5.1 From d04e2ff3a43cca3f7d393a4770f022c7bf1a372c Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 24 Oct 2016 13:35:51 +0100 Subject: Fix incredubly slow back pagination query If a client didn't specify a from token when paginating backwards synapse would attempt to query the (global) maximum topological token. This a) doesn't make much sense since they're room specific and b) there are no indices that lets postgres do this efficiently. --- synapse/handlers/message.py | 4 ++-- synapse/handlers/room.py | 7 +++++-- synapse/storage/stream.py | 19 +++++++++++++------ synapse/streams/events.py | 30 ++++++++++++++++++++++++++++-- 4 files changed, 48 insertions(+), 12 deletions(-) (limited to 'synapse/storage') diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 59eb26beaf..abfa8c65a4 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -82,8 +82,8 @@ class MessageHandler(BaseHandler): room_token = pagin_config.from_token.room_key else: pagin_config.from_token = ( - yield self.hs.get_event_sources().get_current_token( - direction='b' + yield self.hs.get_event_sources().get_current_token_for_room( + room_id=room_id ) ) room_token = pagin_config.from_token.room_key diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index a7f533f7be..59e4d1cd15 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -475,8 +475,11 @@ class RoomEventSource(object): defer.returnValue((events, end_key)) - def get_current_key(self, direction='f'): - return self.store.get_room_events_max_id(direction) + def get_current_key(self): + return self.store.get_room_events_max_id() + + def get_current_key_for_room(self, room_id): + return self.store.get_room_events_max_id(room_id) @defer.inlineCallbacks def get_pagination_rows(self, user, config, key): diff --git a/synapse/storage/stream.py b/synapse/storage/stream.py index 07ea969d4d..888b1cb35d 100644 --- a/synapse/storage/stream.py +++ b/synapse/storage/stream.py @@ -521,13 +521,20 @@ class StreamStore(SQLBaseStore): ) @defer.inlineCallbacks - def get_room_events_max_id(self, direction='f'): + def get_room_events_max_id(self, room_id=None): + """Returns the current token for rooms stream. + + By default, it returns the current global stream token. Specifying a + `room_id` causes it to return the current room specific topological + token. + """ token = yield self._stream_id_gen.get_current_token() - if direction != 'b': + if room_id is None: defer.returnValue("s%d" % (token,)) else: topo = yield self.runInteraction( - "_get_max_topological_txn", self._get_max_topological_txn + "_get_max_topological_txn", self._get_max_topological_txn, + room_id, ) defer.returnValue("t%d-%d" % (topo, token)) @@ -579,11 +586,11 @@ class StreamStore(SQLBaseStore): lambda r: r[0][0] if r else 0 ) - def _get_max_topological_txn(self, txn): + def _get_max_topological_txn(self, txn, room_id): txn.execute( "SELECT MAX(topological_ordering) FROM events" - " WHERE outlier = ?", - (False,) + " WHERE room_id = ?", + (room_id,) ) rows = txn.fetchall() diff --git a/synapse/streams/events.py b/synapse/streams/events.py index 6bf21d6f5e..4018dbde56 100644 --- a/synapse/streams/events.py +++ b/synapse/streams/events.py @@ -41,13 +41,39 @@ class EventSources(object): self.store = hs.get_datastore() @defer.inlineCallbacks - def get_current_token(self, direction='f'): + def get_current_token(self): push_rules_key, _ = self.store.get_push_rules_stream_token() to_device_key = self.store.get_to_device_stream_token() token = StreamToken( room_key=( - yield self.sources["room"].get_current_key(direction) + yield self.sources["room"].get_current_key() + ), + presence_key=( + yield self.sources["presence"].get_current_key() + ), + typing_key=( + yield self.sources["typing"].get_current_key() + ), + receipt_key=( + yield self.sources["receipt"].get_current_key() + ), + account_data_key=( + yield self.sources["account_data"].get_current_key() + ), + push_rules_key=push_rules_key, + to_device_key=to_device_key, + ) + defer.returnValue(token) + + @defer.inlineCallbacks + def get_current_token_for_room(self, room_id): + push_rules_key, _ = self.store.get_push_rules_stream_token() + to_device_key = self.store.get_to_device_stream_token() + + token = StreamToken( + room_key=( + yield self.sources["room"].get_current_key() ), presence_key=( yield self.sources["presence"].get_current_key() -- cgit 1.5.1