summary refs log tree commit diff
diff options
context:
space:
mode:
authorNeil Johnson <neil@matrix.org>2019-03-27 09:18:35 +0000
committerNeil Johnson <neil@matrix.org>2019-03-27 09:18:35 +0000
commit1048e2ca6a815507b3d80b432bfbc551c0e6e779 (patch)
tree9422ff807b52e0a7e22a607769cdfa962408dda8
parentMerge branch 'develop' into matrix-org-hotfixes (diff)
parentSupport 3PID login in password providers (#4931) (diff)
downloadsynapse-1048e2ca6a815507b3d80b432bfbc551c0e6e779.tar.xz
Merge branch 'develop' of github.com:matrix-org/synapse into matrix-org-hotfixes
-rw-r--r--changelog.d/4793.feature1
-rw-r--r--changelog.d/4931.feature1
-rw-r--r--changelog.d/4944.feature1
-rw-r--r--docs/password_auth_providers.rst14
-rw-r--r--synapse/api/auth.py22
-rw-r--r--synapse/federation/transport/client.py2
-rw-r--r--synapse/federation/transport/server.py14
-rw-r--r--synapse/handlers/auth.py39
-rw-r--r--synapse/handlers/profile.py10
-rw-r--r--synapse/handlers/register.py10
-rw-r--r--synapse/http/matrixfederationclient.py1
-rw-r--r--synapse/module_api/__init__.py18
-rw-r--r--synapse/rest/client/v1/login.py49
-rw-r--r--synapse/storage/user_directory.py28
-rw-r--r--tests/handlers/test_typing.py6
15 files changed, 171 insertions, 45 deletions
diff --git a/changelog.d/4793.feature b/changelog.d/4793.feature
new file mode 100644

index 0000000000..90dba7d122 --- /dev/null +++ b/changelog.d/4793.feature
@@ -0,0 +1 @@ +Synapse is now permissive about trailing slashes on some of its federation endpoints, allowing zero or more to be present. \ No newline at end of file diff --git a/changelog.d/4931.feature b/changelog.d/4931.feature new file mode 100644
index 0000000000..5d34d16800 --- /dev/null +++ b/changelog.d/4931.feature
@@ -0,0 +1 @@ +Add ability for password providers to login/register a user via 3PID (email, phone). \ No newline at end of file diff --git a/changelog.d/4944.feature b/changelog.d/4944.feature new file mode 100644
index 0000000000..8f792b8890 --- /dev/null +++ b/changelog.d/4944.feature
@@ -0,0 +1 @@ +The user directory has been rewritten to make it faster, with less chance of falling behind on a large server. diff --git a/docs/password_auth_providers.rst b/docs/password_auth_providers.rst
index d8a7b61cdc..6149ba7458 100644 --- a/docs/password_auth_providers.rst +++ b/docs/password_auth_providers.rst
@@ -75,6 +75,20 @@ Password auth provider classes may optionally provide the following methods. result from the ``/login`` call (including ``access_token``, ``device_id``, etc.) +``someprovider.check_3pid_auth``\(*medium*, *address*, *password*) + + This method, if implemented, is called when a user attempts to register or + log in with a third party identifier, such as email. It is passed the + medium (ex. "email"), an address (ex. "jdoe@example.com") and the user's + password. + + The method should return a Twisted ``Deferred`` object, which resolves to + a ``str`` containing the user's (canonical) User ID if authentication was + successful, and ``None`` if not. + + As with ``check_auth``, the ``Deferred`` may alternatively resolve to a + ``(user_id, callback)`` tuple. + ``someprovider.check_password``\(*user_id*, *password*) This method provides a simpler interface than ``get_supported_login_types`` diff --git a/synapse/api/auth.py b/synapse/api/auth.py
index ee646a97e8..e8112d5f05 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py
@@ -621,13 +621,13 @@ class Auth(object): Returns: True if the the sender is allowed to redact the target event if the - target event was created by them. + target event was created by them. False if the sender is allowed to redact the target event with no - further checks. + further checks. Raises: AuthError if the event sender is definitely not allowed to redact - the target event. + the target event. """ return event_auth.check_redaction(room_version, event, auth_events) @@ -743,9 +743,9 @@ class Auth(object): Returns: Deferred[tuple[str, str|None]]: Resolves to the current membership of - the user in the room and the membership event ID of the user. If - the user is not in the room and never has been, then - `(Membership.JOIN, None)` is returned. + the user in the room and the membership event ID of the user. If + the user is not in the room and never has been, then + `(Membership.JOIN, None)` is returned. """ try: @@ -777,13 +777,13 @@ class Auth(object): Args: user_id(str|None): If present, checks for presence against existing - MAU cohort + MAU cohort threepid(dict|None): If present, checks for presence against configured - reserved threepid. Used in cases where the user is trying register - with a MAU blocked server, normally they would be rejected but their - threepid is on the reserved list. user_id and - threepid should never be set at the same time. + reserved threepid. Used in cases where the user is trying register + with a MAU blocked server, normally they would be rejected but their + threepid is on the reserved list. user_id and + threepid should never be set at the same time. """ # Never fail an auth check for the server notices users or support user diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py
index 0cdb31178f..e424c40fdf 100644 --- a/synapse/federation/transport/client.py +++ b/synapse/federation/transport/client.py
@@ -173,7 +173,7 @@ class TransportLayerClient(object): # generated by the json_data_callback. json_data = transaction.get_dict() - path = _create_v1_path("/send/%s/", transaction.transaction_id) + path = _create_v1_path("/send/%s", transaction.transaction_id) response = yield self.client.put_json( transaction.destination, diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py
index 96d680a5ad..efb6bdca48 100644 --- a/synapse/federation/transport/server.py +++ b/synapse/federation/transport/server.py
@@ -312,7 +312,7 @@ class BaseFederationServlet(object): class FederationSendServlet(BaseFederationServlet): - PATH = "/send/(?P<transaction_id>[^/]*)/" + PATH = "/send/(?P<transaction_id>[^/]*)/?" def __init__(self, handler, server_name, **kwargs): super(FederationSendServlet, self).__init__( @@ -378,7 +378,7 @@ class FederationSendServlet(BaseFederationServlet): class FederationEventServlet(BaseFederationServlet): - PATH = "/event/(?P<event_id>[^/]*)/" + PATH = "/event/(?P<event_id>[^/]*)/?" # This is when someone asks for a data item for a given server data_id pair. def on_GET(self, origin, content, query, event_id): @@ -386,7 +386,7 @@ class FederationEventServlet(BaseFederationServlet): class FederationStateServlet(BaseFederationServlet): - PATH = "/state/(?P<context>[^/]*)/" + PATH = "/state/(?P<context>[^/]*)/?" # This is when someone asks for all data for a given context. def on_GET(self, origin, content, query, context): @@ -398,7 +398,7 @@ class FederationStateServlet(BaseFederationServlet): class FederationStateIdsServlet(BaseFederationServlet): - PATH = "/state_ids/(?P<room_id>[^/]*)/" + PATH = "/state_ids/(?P<room_id>[^/]*)/?" def on_GET(self, origin, content, query, room_id): return self.handler.on_state_ids_request( @@ -409,7 +409,7 @@ class FederationStateIdsServlet(BaseFederationServlet): class FederationBackfillServlet(BaseFederationServlet): - PATH = "/backfill/(?P<context>[^/]*)/" + PATH = "/backfill/(?P<context>[^/]*)/?" def on_GET(self, origin, content, query, context): versions = [x.decode('ascii') for x in query[b"v"]] @@ -1080,7 +1080,7 @@ class FederationGroupsCategoriesServlet(BaseFederationServlet): """Get all categories for a group """ PATH = ( - "/groups/(?P<group_id>[^/]*)/categories/" + "/groups/(?P<group_id>[^/]*)/categories/?" ) @defer.inlineCallbacks @@ -1150,7 +1150,7 @@ class FederationGroupsRolesServlet(BaseFederationServlet): """Get roles in a group """ PATH = ( - "/groups/(?P<group_id>[^/]*)/roles/" + "/groups/(?P<group_id>[^/]*)/roles/?" ) @defer.inlineCallbacks diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index caad9ae2dd..4544de821d 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py
@@ -746,6 +746,42 @@ class AuthHandler(BaseHandler): ) @defer.inlineCallbacks + def check_password_provider_3pid(self, medium, address, password): + """Check if a password provider is able to validate a thirdparty login + + Args: + medium (str): The medium of the 3pid (ex. email). + address (str): The address of the 3pid (ex. jdoe@example.com). + password (str): The password of the user. + + Returns: + Deferred[(str|None, func|None)]: A tuple of `(user_id, + callback)`. If authentication is successful, `user_id` is a `str` + containing the authenticated, canonical user ID. `callback` is + then either a function to be later run after the server has + completed login/registration, or `None`. If authentication was + unsuccessful, `user_id` and `callback` are both `None`. + """ + for provider in self.password_providers: + if hasattr(provider, "check_3pid_auth"): + # This function is able to return a deferred that either + # resolves None, meaning authentication failure, or upon + # success, to a str (which is the user_id) or a tuple of + # (user_id, callback_func), where callback_func should be run + # after we've finished everything else + result = yield provider.check_3pid_auth( + medium, address, password, + ) + if result: + # Check if the return value is a str or a tuple + if isinstance(result, str): + # If it's a str, set callback function to None + result = (result, None) + defer.returnValue(result) + + defer.returnValue((None, None)) + + @defer.inlineCallbacks def _check_local_password(self, user_id, password): """Authenticate a user against the local password database. @@ -756,7 +792,8 @@ class AuthHandler(BaseHandler): user_id (unicode): complete @user:id password (unicode): the provided password Returns: - (unicode) the canonical_user_id, or None if unknown user / bad password + Deferred[unicode] the canonical_user_id, or Deferred[None] if + unknown user/bad password Raises: LimitExceededError if the ratelimiter's login requests count for this diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py
index 1dfbde84fd..a65c98ff5c 100644 --- a/synapse/handlers/profile.py +++ b/synapse/handlers/profile.py
@@ -147,8 +147,14 @@ class BaseProfileHandler(BaseHandler): @defer.inlineCallbacks def set_displayname(self, target_user, requester, new_displayname, by_admin=False): - """target_user is the user whose displayname is to be changed; - auth_user is the user attempting to make this change.""" + """Set the displayname of a user + + Args: + target_user (UserID): the user whose displayname is to be changed. + requester (Requester): The user attempting to make this change. + new_displayname (str): The displayname to give this user. + by_admin (bool): Whether this change was made by an administrator. + """ if not self.hs.is_mine(target_user): raise SynapseError(400, "User is not hosted on this Home Server") diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py
index 68f73d3793..58940e0320 100644 --- a/synapse/handlers/register.py +++ b/synapse/handlers/register.py
@@ -171,7 +171,7 @@ class RegistrationHandler(BaseHandler): api.constants.UserTypes, or None for a normal user. default_display_name (unicode|None): if set, the new user's displayname will be set to this. Defaults to 'localpart'. - address (str|None): the IP address used to perform the regitration. + address (str|None): the IP address used to perform the registration. Returns: A tuple of (user_id, access_token). Raises: @@ -623,7 +623,7 @@ class RegistrationHandler(BaseHandler): admin (boolean): is an admin user? user_type (str|None): type of user. One of the values from api.constants.UserTypes, or None for a normal user. - address (str|None): the IP address used to perform the regitration. + address (str|None): the IP address used to perform the registration. Returns: Deferred @@ -721,9 +721,9 @@ class RegistrationHandler(BaseHandler): access_token (str|None): The access token of the newly logged in device, or None if `inhibit_login` enabled. bind_email (bool): Whether to bind the email with the identity - server + server. bind_msisdn (bool): Whether to bind the msisdn with the identity - server + server. """ if self.hs.config.worker_app: yield self._post_registration_client( @@ -765,7 +765,7 @@ class RegistrationHandler(BaseHandler): """A user consented to the terms on registration Args: - user_id (str): The user ID that consented + user_id (str): The user ID that consented. consent_version (str): version of the policy the user has consented to. """ diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index 8e855d13d6..ff63d0b2a8 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py
@@ -231,6 +231,7 @@ class MatrixFederationHttpClient(object): # Retry with a trailing slash if we received a 400 with # 'M_UNRECOGNIZED' which some endpoints can return when omitting a # trailing slash on Synapse <= v0.99.3. + logger.info("Retrying request with trailing slash") request.path += "/" response = yield self._send_request( diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py
index fc9a20ff59..235ce8334e 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py
@@ -73,14 +73,26 @@ class ModuleApi(object): """ return self._auth_handler.check_user_exists(user_id) - def register(self, localpart): - """Registers a new user with given localpart + @defer.inlineCallbacks + def register(self, localpart, displayname=None): + """Registers a new user with given localpart and optional + displayname. + + Args: + localpart (str): The localpart of the new user. + displayname (str|None): The displayname of the new user. If None, + the user's displayname will default to `localpart`. Returns: Deferred: a 2-tuple of (user_id, access_token) """ + # Register the user reg = self.hs.get_registration_handler() - return reg.register(localpart=localpart) + user_id, access_token = yield reg.register( + localpart=localpart, default_display_name=displayname, + ) + + defer.returnValue((user_id, access_token)) @defer.inlineCallbacks def invalidate_access_token(self, access_token): diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py
index 8d56effbb8..5180e9eaf1 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py
@@ -201,6 +201,24 @@ class LoginRestServlet(ClientV1RestServlet): # We store all email addreses as lowercase in the DB. # (See add_threepid in synapse/handlers/auth.py) address = address.lower() + + # Check for login providers that support 3pid login types + canonical_user_id, callback_3pid = ( + yield self.auth_handler.check_password_provider_3pid( + medium, + address, + login_submission["password"], + ) + ) + if canonical_user_id: + # Authentication through password provider and 3pid succeeded + result = yield self._register_device_with_callback( + canonical_user_id, login_submission, callback_3pid, + ) + defer.returnValue(result) + + # No password providers were able to handle this 3pid + # Check local store user_id = yield self.hs.get_datastore().get_user_id_by_threepid( medium, address, ) @@ -223,20 +241,43 @@ class LoginRestServlet(ClientV1RestServlet): if "user" not in identifier: raise SynapseError(400, "User identifier is missing 'user' key") - auth_handler = self.auth_handler - canonical_user_id, callback = yield auth_handler.validate_login( + canonical_user_id, callback = yield self.auth_handler.validate_login( identifier["user"], login_submission, ) + result = yield self._register_device_with_callback( + canonical_user_id, login_submission, callback, + ) + defer.returnValue(result) + + @defer.inlineCallbacks + def _register_device_with_callback( + self, + user_id, + login_submission, + callback=None, + ): + """ Registers a device with a given user_id. Optionally run a callback + function after registration has completed. + + Args: + user_id (str): ID of the user to register. + login_submission (dict): Dictionary of login information. + callback (func|None): Callback function to run after registration. + + Returns: + result (Dict[str,str]): Dictionary of account information after + successful registration. + """ device_id = login_submission.get("device_id") initial_display_name = login_submission.get("initial_device_display_name") device_id, access_token = yield self.registration_handler.register_device( - canonical_user_id, device_id, initial_display_name, + user_id, device_id, initial_display_name, ) result = { - "user_id": canonical_user_id, + "user_id": user_id, "access_token": access_token, "home_server": self.hs.hostname, "device_id": device_id, diff --git a/synapse/storage/user_directory.py b/synapse/storage/user_directory.py
index 65bdb1b4a5..4d60a5726f 100644 --- a/synapse/storage/user_directory.py +++ b/synapse/storage/user_directory.py
@@ -135,7 +135,12 @@ class UserDirectoryStore(StateDeltasStore, BackgroundUpdateStore): @defer.inlineCallbacks def _populate_user_directory_process_rooms(self, progress, batch_size): - + """ + Args: + progress (dict) + batch_size (int): Maximum number of state events to process + per cycle. + """ state = self.hs.get_state_handler() # If we don't have progress filed, delete everything. @@ -143,13 +148,14 @@ class UserDirectoryStore(StateDeltasStore, BackgroundUpdateStore): yield self.delete_all_from_user_dir() def _get_next_batch(txn): + # Only fetch 250 rooms, so we don't fetch too many at once, even + # if those 250 rooms have less than batch_size state events. sql = """ - SELECT room_id FROM %s + SELECT room_id, events FROM %s ORDER BY events DESC - LIMIT %s + LIMIT 250 """ % ( TEMP_TABLE + "_rooms", - str(batch_size), ) txn.execute(sql) rooms_to_work_on = txn.fetchall() @@ -157,8 +163,6 @@ class UserDirectoryStore(StateDeltasStore, BackgroundUpdateStore): if not rooms_to_work_on: return None - rooms_to_work_on = [x[0] for x in rooms_to_work_on] - # Get how many are left to process, so we can give status on how # far we are in processing txn.execute("SELECT COUNT(*) FROM " + TEMP_TABLE + "_rooms") @@ -180,7 +184,9 @@ class UserDirectoryStore(StateDeltasStore, BackgroundUpdateStore): % (len(rooms_to_work_on), progress["remaining"]) ) - for room_id in rooms_to_work_on: + processed_event_count = 0 + + for room_id, event_count in rooms_to_work_on: is_in_room = yield self.is_host_joined(room_id, self.server_name) if is_in_room: @@ -247,7 +253,13 @@ class UserDirectoryStore(StateDeltasStore, BackgroundUpdateStore): progress, ) - defer.returnValue(len(rooms_to_work_on)) + processed_event_count += event_count + + if processed_event_count > batch_size: + # Don't process any more rooms, we've hit our batch size. + defer.returnValue(processed_event_count) + + defer.returnValue(processed_event_count) @defer.inlineCallbacks def _populate_user_directory_process_users(self, progress, batch_size): diff --git a/tests/handlers/test_typing.py b/tests/handlers/test_typing.py
index 7decb22933..6460cbc708 100644 --- a/tests/handlers/test_typing.py +++ b/tests/handlers/test_typing.py
@@ -180,7 +180,7 @@ class TypingNotificationsTestCase(unittest.HomeserverTestCase): put_json = self.hs.get_http_client().put_json put_json.assert_called_once_with( "farm", - path="/_matrix/federation/v1/send/1000000/", + path="/_matrix/federation/v1/send/1000000", data=_expect_edu_transaction( "m.typing", content={ @@ -202,7 +202,7 @@ class TypingNotificationsTestCase(unittest.HomeserverTestCase): (request, channel) = self.make_request( "PUT", - "/_matrix/federation/v1/send/1000000/", + "/_matrix/federation/v1/send/1000000", _make_edu_transaction_json( "m.typing", content={ @@ -258,7 +258,7 @@ class TypingNotificationsTestCase(unittest.HomeserverTestCase): put_json = self.hs.get_http_client().put_json put_json.assert_called_once_with( "farm", - path="/_matrix/federation/v1/send/1000000/", + path="/_matrix/federation/v1/send/1000000", data=_expect_edu_transaction( "m.typing", content={