From aa3c9c7bd0736bca1b3626c87535192b89431583 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 21 Aug 2015 10:57:47 +0100 Subject: Don't allow people to register user ids which only differ by case to an existing one --- synapse/handlers/register.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'synapse/handlers') diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py index 39392d9fdd..86390a3671 100644 --- a/synapse/handlers/register.py +++ b/synapse/handlers/register.py @@ -57,8 +57,8 @@ class RegistrationHandler(BaseHandler): yield self.check_user_id_is_valid(user_id) - u = yield self.store.get_user_by_id(user_id) - if u: + users = yield self.store.get_users_by_id_case_insensitive(user_id) + if users: raise SynapseError( 400, "User ID already taken.", -- cgit 1.5.1 From 42f12ad92f5bc372569f15ffc81e9cf8146d2ac6 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 21 Aug 2015 11:34:43 +0100 Subject: When logging in fetch user by user_id case insensitively, *unless* there are multiple case insensitive matches, in which case require the exact user_id --- synapse/handlers/auth.py | 31 +++++++++++++++++++++++-------- synapse/rest/client/v1/login.py | 5 +++-- synapse/storage/registration.py | 7 +++++-- 3 files changed, 31 insertions(+), 12 deletions(-) (limited to 'synapse/handlers') diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index ff2c66f442..058a0f416d 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -162,7 +162,8 @@ class AuthHandler(BaseHandler): if not user_id.startswith('@'): user_id = UserID.create(user_id, self.hs.hostname).to_string() - yield self._check_password(user_id, password) + user_id, password_hash = yield self._find_user_id_and_pwd_hash(user_id) + self._check_password(user_id, password, password_hash) defer.returnValue(user_id) @defer.inlineCallbacks @@ -283,23 +284,37 @@ class AuthHandler(BaseHandler): StoreError if there was a problem storing the token. LoginError if there was an authentication problem. """ - yield self._check_password(user_id, password) + user_id, password_hash = yield self._find_user_id_and_pwd_hash(user_id) + self._check_password(user_id, password, password_hash) reg_handler = self.hs.get_handlers().registration_handler access_token = reg_handler.generate_token(user_id) logger.info("Logging in user %s", user_id) yield self.store.add_access_token_to_user(user_id, access_token) - defer.returnValue(access_token) + defer.returnValue((user_id, access_token)) @defer.inlineCallbacks - def _check_password(self, user_id, password): - """Checks that user_id has passed password, raises LoginError if not.""" - user_info = yield self.store.get_user_by_id(user_id=user_id) - if not user_info: + def _find_user_id_and_pwd_hash(self, user_id): + user_infos = yield self.store.get_users_by_id_case_insensitive(user_id) + if not user_infos: logger.warn("Attempted to login as %s but they do not exist", user_id) raise LoginError(403, "", errcode=Codes.FORBIDDEN) - stored_hash = user_info["password_hash"] + if len(user_infos) > 1: + if user_id not in user_infos: + logger.warn( + "Attempted to login as %s but it matches more than one user " + "inexactly: %r", + user_id, user_infos.keys() + ) + raise LoginError(403, "", errcode=Codes.FORBIDDEN) + + defer.returnValue((user_id, user_infos[user_id])) + else: + defer.returnValue(user_infos.popitem()) + + def _check_password(self, user_id, password, stored_hash): + """Checks that user_id has passed password, raises LoginError if not.""" if not bcrypt.checkpw(password, stored_hash): logger.warn("Failed password login for user %s", user_id) raise LoginError(403, "", errcode=Codes.FORBIDDEN) diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index 0d5eafd0fa..2444f27366 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -83,9 +83,10 @@ class LoginRestServlet(ClientV1RestServlet): if not user_id.startswith('@'): user_id = UserID.create( - user_id, self.hs.hostname).to_string() + user_id, self.hs.hostname + ).to_string() - token = yield self.handlers.auth_handler.login_with_password( + user_id, token = yield self.handlers.auth_handler.login_with_password( user_id=user_id, password=login_submission["password"]) diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py index 25adecaf6d..586628579d 100644 --- a/synapse/storage/registration.py +++ b/synapse/storage/registration.py @@ -99,13 +99,16 @@ class RegistrationStore(SQLBaseStore): ) def get_users_by_id_case_insensitive(self, user_id): + """Gets users that match user_id case insensitively. + Returns a mapping of user_id -> password_hash. + """ def f(txn): sql = ( "SELECT name, password_hash FROM users" - " WHERE name = lower(?)" + " WHERE lower(name) = lower(?)" ) txn.execute(sql, (user_id,)) - return self.cursor_to_dict(txn) + return dict(txn.fetchall()) return self.runInteraction("get_users_by_id_case_insensitive", f) -- cgit 1.5.1 From fd5ad0f00ec963e9722d9f5bbe526dc84038e408 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 21 Aug 2015 11:45:43 +0100 Subject: Doc string --- synapse/handlers/auth.py | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'synapse/handlers') diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 058a0f416d..602c5bcd89 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -295,6 +295,12 @@ class AuthHandler(BaseHandler): @defer.inlineCallbacks def _find_user_id_and_pwd_hash(self, user_id): + """Checks to see if a user with the given id exists. Will check case + insensitively, but will throw if there are multiple inexact matches. + + Returns: + tuple: A 2-tuple of `(canonical_user_id, password_hash)` + """ user_infos = yield self.store.get_users_by_id_case_insensitive(user_id) if not user_infos: logger.warn("Attempted to login as %s but they do not exist", user_id) -- cgit 1.5.1 From f8f3d72e2b6e5b38bb6ab8e057ede454d77d114f Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 24 Aug 2015 16:19:43 +0100 Subject: Don't make pushers handle presence/typing events --- synapse/handlers/events.py | 10 ++++++++-- synapse/notifier.py | 7 ++++++- synapse/push/__init__.py | 8 +++++--- 3 files changed, 19 insertions(+), 6 deletions(-) (limited to 'synapse/handlers') diff --git a/synapse/handlers/events.py b/synapse/handlers/events.py index f9ca2f8634..891502c04f 100644 --- a/synapse/handlers/events.py +++ b/synapse/handlers/events.py @@ -49,7 +49,12 @@ class EventStreamHandler(BaseHandler): @defer.inlineCallbacks @log_function def get_stream(self, auth_user_id, pagin_config, timeout=0, - as_client_event=True, affect_presence=True): + as_client_event=True, affect_presence=True, + only_room_events=False): + """Fetches the events stream for a given user. + + If `only_room_events` is `True` only room events will be returned. + """ auth_user = UserID.from_string(auth_user_id) try: @@ -89,7 +94,8 @@ class EventStreamHandler(BaseHandler): timeout = random.randint(int(timeout*0.9), int(timeout*1.1)) events, tokens = yield self.notifier.get_events_for( - auth_user, room_ids, pagin_config, timeout + auth_user, room_ids, pagin_config, timeout, + only_room_events=only_room_events ) time_now = self.clock.time_msec() diff --git a/synapse/notifier.py b/synapse/notifier.py index dbd8efe9fb..f998fc83bf 100644 --- a/synapse/notifier.py +++ b/synapse/notifier.py @@ -328,10 +328,13 @@ class Notifier(object): defer.returnValue(result) @defer.inlineCallbacks - def get_events_for(self, user, rooms, pagination_config, timeout): + def get_events_for(self, user, rooms, pagination_config, timeout, + only_room_events=False): """ For the given user and rooms, return any new events for them. If there are no new events wait for up to `timeout` milliseconds for any new events to happen before returning. + + If `only_room_events` is `True` only room events will be returned. """ from_token = pagination_config.from_token if not from_token: @@ -352,6 +355,8 @@ class Notifier(object): after_id = getattr(after_token, keyname) if before_id == after_id: continue + if only_room_events and name != "room": + continue new_events, new_key = yield source.get_new_events_for_user( user, getattr(from_token, keyname), limit, ) diff --git a/synapse/push/__init__.py b/synapse/push/__init__.py index 13002e0db4..f1952b5a0f 100644 --- a/synapse/push/__init__.py +++ b/synapse/push/__init__.py @@ -249,7 +249,9 @@ class Pusher(object): # we fail to dispatch the push) config = PaginationConfig(from_token=None, limit='1') chunk = yield self.evStreamHandler.get_stream( - self.user_name, config, timeout=0) + self.user_name, config, timeout=0, affect_presence=False, + only_room_events=True + ) self.last_token = chunk['end'] self.store.update_pusher_last_token( self.app_id, self.pushkey, self.user_name, self.last_token @@ -280,8 +282,8 @@ class Pusher(object): config = PaginationConfig(from_token=from_tok, limit='1') timeout = (300 + random.randint(-60, 60)) * 1000 chunk = yield self.evStreamHandler.get_stream( - self.user_name, config, - timeout=timeout, affect_presence=False + self.user_name, config, timeout=timeout, affect_presence=False, + only_room_events=True ) # limiting to 1 may get 1 event plus 1 presence event, so -- cgit 1.5.1 From 51c53369a318262ecc3adc3777be8b838509d19d Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 24 Aug 2015 16:38:20 +0100 Subject: Do auth checks *before* persisting the event --- synapse/handlers/_base.py | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) (limited to 'synapse/handlers') diff --git a/synapse/handlers/_base.py b/synapse/handlers/_base.py index e91f1129db..cb992143f5 100644 --- a/synapse/handlers/_base.py +++ b/synapse/handlers/_base.py @@ -107,6 +107,22 @@ class BaseHandler(object): if not suppress_auth: self.auth.check(event, auth_events=context.current_state) + if event.type == EventTypes.CanonicalAlias: + # Check the alias is acually valid (at this time at least) + room_alias_str = event.content.get("alias", None) + if room_alias_str: + room_alias = RoomAlias.from_string(room_alias_str) + directory_handler = self.hs.get_handlers().directory_handler + mapping = yield directory_handler.get_association(room_alias) + + if mapping["room_id"] != event.room_id: + raise SynapseError( + 400, + "Room alias %s does not point to the room" % ( + room_alias_str, + ) + ) + (event_stream_id, max_stream_id) = yield self.store.persist_event( event, context=context ) @@ -130,22 +146,6 @@ class BaseHandler(object): returned_invite.signatures ) - if event.type == EventTypes.CanonicalAlias: - # Check the alias is acually valid (at this time at least) - room_alias_str = event.content.get("alias", None) - if room_alias_str: - room_alias = RoomAlias.from_string(room_alias_str) - directory_handler = self.hs.get_handlers().directory_handler - mapping = yield directory_handler.get_association(room_alias) - - if mapping["room_id"] != event.room_id: - raise SynapseError( - 400, - "Room alias %s does not point to the room" % ( - room_alias_str, - ) - ) - destinations = set(extra_destinations) for k, s in context.current_state.items(): try: -- cgit 1.5.1 From f4d552589e3cb815144dea646140db66d845a237 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 26 Aug 2015 10:51:08 +0100 Subject: Don't loop over all rooms ever in typing.get_new_events_for_user --- synapse/handlers/typing.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'synapse/handlers') diff --git a/synapse/handlers/typing.py b/synapse/handlers/typing.py index 026bd2b9d4..1ed220d871 100644 --- a/synapse/handlers/typing.py +++ b/synapse/handlers/typing.py @@ -260,8 +260,8 @@ class TypingNotificationEventSource(object): ) events = [] - for room_id in handler._room_serials: - if room_id not in joined_room_ids: + for room_id in joined_room_ids: + if room_id not in handler._room_serials: continue if handler._room_serials[room_id] <= from_key: continue -- cgit 1.5.1 From da51acf0e752badb73c036f4b1cf0ec943b6dcb1 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 26 Aug 2015 11:08:23 +0100 Subject: Remove needless existence checks --- synapse/handlers/typing.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) (limited to 'synapse/handlers') diff --git a/synapse/handlers/typing.py b/synapse/handlers/typing.py index 1ed220d871..d7096aab8c 100644 --- a/synapse/handlers/typing.py +++ b/synapse/handlers/typing.py @@ -204,15 +204,11 @@ class TypingNotificationHandler(BaseHandler): ) def _push_update_local(self, room_id, user, typing): - if room_id not in self._room_serials: - self._room_serials[room_id] = 0 - self._room_typing[room_id] = set() - - room_set = self._room_typing[room_id] + room_set = self._room_typing.setdefault(room_id, set()) if typing: room_set.add(user) - elif user in room_set: - room_set.remove(user) + else: + room_set.discard(user) self._latest_room_serial += 1 self._room_serials[room_id] = self._latest_room_serial -- cgit 1.5.1