From 0841865aefd4729ef3166cac38b7b7e5be9e7781 Mon Sep 17 00:00:00 2001
From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com>
Date: Fri, 6 Aug 2021 10:08:45 +0100
Subject: [PATCH 01/23] Moved to using new callback registering style
You now register logintype->callback rather than
having check_auth and check_password as the two
only possible functions.
modules now must provide the login types they
support when they register, instead of providing
a function get_supported_login_types.
The auth handler is initialised before the modules
are loaded and so supported login types must be
worked out later
A load_legacy method wraps old-style modules so
that they work with the new system
---
synapse/app/_base.py | 3 +
synapse/handlers/auth.py | 392 ++++++++++++++++++++++-----------
synapse/module_api/__init__.py | 5 +
synapse/server.py | 6 +-
4 files changed, 275 insertions(+), 131 deletions(-)
diff --git a/synapse/app/_base.py b/synapse/app/_base.py
index 50a02f51f54a..375b901221b7 100644
--- a/synapse/app/_base.py
+++ b/synapse/app/_base.py
@@ -39,6 +39,7 @@
from synapse.crypto import context_factory
from synapse.events.spamcheck import load_legacy_spam_checkers
from synapse.events.third_party_rules import load_legacy_third_party_event_rules
+from synapse.handlers.auth import load_legacy_password_auth_provider
from synapse.logging.context import PreserveLoggingContext
from synapse.metrics.background_process_metrics import wrap_as_background_process
from synapse.metrics.jemalloc import setup_jemalloc_stats
@@ -370,6 +371,8 @@ def run_sighup(*args, **kwargs):
load_legacy_spam_checkers(hs)
load_legacy_third_party_event_rules(hs)
+ for module, config in hs.config.password_providers:
+ load_legacy_password_auth_provider(module=module, config=config, api=module_api)
# If we've configured an expiry time for caches, start the background job now.
setup_expire_lru_cache_entries(hs)
diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index 22a855224188..fe77b0718187 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -207,38 +207,13 @@ def __init__(self, hs: "HomeServer"):
# better way to break the loop
account_handler = ModuleApi(hs, self)
- self.password_providers = [
- PasswordProvider.load(module, config, account_handler)
- for module, config in hs.config.password_providers
- ]
-
- logger.info("Extra password_providers: %s", self.password_providers)
+ self.password_auth_provider = hs.get_password_auth_provider()
self.hs = hs # FIXME better possibility to access registrationHandler later?
self.macaroon_gen = hs.get_macaroon_generator()
self._password_enabled = hs.config.password_enabled
self._password_localdb_enabled = hs.config.password_localdb_enabled
- # start out by assuming PASSWORD is enabled; we will remove it later if not.
- login_types = set()
- if self._password_localdb_enabled:
- login_types.add(LoginType.PASSWORD)
-
- for provider in self.password_providers:
- login_types.update(provider.get_supported_login_types().keys())
-
- if not self._password_enabled:
- login_types.discard(LoginType.PASSWORD)
-
- # Some clients just pick the first type in the list. In this case, we want
- # them to use PASSWORD (rather than token or whatever), so we want to make sure
- # that comes first, where it's present.
- self._supported_login_types = []
- if LoginType.PASSWORD in login_types:
- self._supported_login_types.append(LoginType.PASSWORD)
- login_types.remove(LoginType.PASSWORD)
- self._supported_login_types.extend(login_types)
-
# Ratelimiter for failed auth during UIA. Uses same ratelimit config
# as per `rc_login.failed_attempts`.
self._failed_uia_attempts_ratelimiter = Ratelimiter(
@@ -424,11 +399,10 @@ async def _get_available_ui_auth_types(self, user: UserID) -> Iterable[str]:
ui_auth_types.add(LoginType.PASSWORD)
# also allow auth from password providers
- for provider in self.password_providers:
- for t in provider.get_supported_login_types().keys():
- if t == LoginType.PASSWORD and not self._password_enabled:
- continue
- ui_auth_types.add(t)
+ for t in self.password_auth_provider.get_supported_login_types().keys():
+ if t == LoginType.PASSWORD and not self._password_enabled:
+ continue
+ ui_auth_types.add(t)
# if sso is enabled, allow the user to log in via SSO iff they have a mapping
# from sso to mxid.
@@ -1030,7 +1004,25 @@ def get_supported_login_types(self) -> Iterable[str]:
Returns:
login types
"""
- return self._supported_login_types
+ # Load any login types registered by modules
+ # This is stored in the password_auth_provider so this doesn't trigger
+ # any callbacks
+ types = list(self.password_auth_provider.get_supported_login_types().keys())
+
+ # This list should include PASSWORD if (either _password_localdb_enabled is
+ # true or if one of the modules registered it) AND _password_enabled is true
+ # Also:
+ # Some clients just pick the first type in the list. In this case, we want
+ # them to use PASSWORD (rather than token or whatever), so we want to make sure
+ # that comes first, where it's present.
+ if LoginType.PASSWORD in types:
+ types.remove(LoginType.PASSWORD)
+ if self._password_enabled:
+ types.insert(0, LoginType.PASSWORD)
+ elif self._password_localdb_enabled and self._password_enabled:
+ types.insert(0, LoginType.PASSWORD)
+
+ return types
async def validate_login(
self,
@@ -1209,15 +1201,19 @@ async def _validate_userid_login(
known_login_type = False
- for provider in self.password_providers:
- supported_login_types = provider.get_supported_login_types()
- if login_type not in supported_login_types:
- # this password provider doesn't understand this login type
- continue
-
+ # get all of the login_types registered by modules
+ supported_login_types = self.password_auth_provider.get_supported_login_types()
+ # check if the login type being used is supported by a module
+ # note that if LoginType.PASSWORD has been disabled then login_type has already been
+ # checked for that by this point
+ if login_type in supported_login_types:
+ # Make a note that this login type is supported by the server
known_login_type = True
+ # Get all the fields expected for this login types
login_fields = supported_login_types[login_type]
+ # go through the login submission and keep track of which required fields are
+ # provided/not provided
missing_fields = []
login_dict = {}
for f in login_fields:
@@ -1225,6 +1221,7 @@ async def _validate_userid_login(
missing_fields.append(f)
else:
login_dict[f] = login_submission[f]
+ # raise an error if any of the expected fields for that login type weren't provided
if missing_fields:
raise SynapseError(
400,
@@ -1232,10 +1229,15 @@ async def _validate_userid_login(
% (login_type, missing_fields),
)
- result = await provider.check_auth(username, login_type, login_dict)
+ # call all of the check_auth hooks for that login_type
+ # it will return a result once the first success is found (or None otherwise)
+ result = await self.password_auth_provider.check_auth(
+ username, login_type, login_dict
+ )
if result:
return result
+ # if no module managed to authenticate the user, then fallback to built in password based auth
if login_type == LoginType.PASSWORD and self._password_localdb_enabled:
known_login_type = True
@@ -1274,11 +1276,16 @@ async def check_password_provider_3pid(
completed login/registration, or `None`. If authentication was
unsuccessful, `user_id` and `callback` are both `None`.
"""
- for provider in self.password_providers:
- result = await provider.check_3pid_auth(medium, address, password)
- if result:
- return result
+ # call all of the check_3pid_auth callbacks
+ # Result will be from the first callback that returns something other than None
+ # If all the callbacks return None, then result is also set to None
+ result = await self.password_auth_provider.check_3pid_auth(
+ medium, address, password
+ )
+ if result:
+ return result
+ # if result is None then return (None, None)
return None, None
async def _check_local_password(self, user_id: str, password: str) -> Optional[str]:
@@ -1357,13 +1364,12 @@ async def delete_access_token(self, access_token: str):
user_info = await self.auth.get_user_by_access_token(access_token)
await self.store.delete_access_token(access_token)
- # see if any of our auth providers want to know about this
- for provider in self.password_providers:
- await provider.on_logged_out(
- user_id=user_info.user_id,
- device_id=user_info.device_id,
- access_token=access_token,
- )
+ # see if any modules want to know about this
+ await self.password_auth_provider.on_logged_out(
+ user_id=user_info.user_id,
+ device_id=user_info.device_id,
+ access_token=access_token,
+ )
# delete pushers associated with this access token
if user_info.token_id is not None:
@@ -1390,12 +1396,11 @@ async def delete_access_tokens_for_user(
user_id, except_token_id=except_token_id, device_id=device_id
)
- # see if any of our auth providers want to know about this
- for provider in self.password_providers:
- for token, _, device_id in tokens_and_devices:
- await provider.on_logged_out(
- user_id=user_id, device_id=device_id, access_token=token
- )
+ # see if any modules want to know about this
+ for token, _, device_id in tokens_and_devices:
+ await self.password_auth_provider.on_logged_out(
+ user_id=user_id, device_id=device_id, access_token=token
+ )
# delete pushers associated with the access tokens
await self.hs.get_pusherpool().remove_pushers_by_access_token(
@@ -1727,7 +1732,6 @@ def add_query_param_to_url(url: str, param_name: str, param: Any):
@attr.s(slots=True)
class MacaroonGenerator:
-
hs = attr.ib()
def generate_guest_access_token(self, user_id: str) -> str:
@@ -1800,38 +1804,184 @@ def _generate_base_macaroon(self, user_id: str) -> pymacaroons.Macaroon:
return macaroon
-class PasswordProvider:
- """Wrapper for a password auth provider module
+def load_legacy_password_auth_provider(module, config, api: ModuleApi):
+ try:
+ provider = module(config=config, account_handler=api)
+ except Exception as e:
+ logger.error("Error while initializing %r: %s", module, e)
+ raise
- This class abstracts out all of the backwards-compatibility hacks for
- password providers, to provide a consistent interface.
- """
+ # The known hooks. If a module implements a method who's name appears in this set
+ # we'll want to register it
+ password_auth_provider_methods = {
+ "check_3pid_auth",
+ "on_logged_out",
+ }
- @classmethod
- def load(cls, module, config, module_api: ModuleApi) -> "PasswordProvider":
- try:
- pp = module(config=config, account_handler=module_api)
- except Exception as e:
- logger.error("Error while initializing %r: %s", module, e)
- raise
- return cls(pp, module_api)
+ # All methods that the module provides should be async, but this wasn't enforced
+ # in the old module system, so we wrap them if needed
+ def async_wrapper(f: Optional[Callable]) -> Optional[Callable[..., Awaitable]]:
+ # f might be None if the callback isn't implemented by the module. In this
+ # case we don't want to register a callback at all so we return None.
+ if f is None:
+ return None
+
+ # We need to wrap check_password because its old form would return a boolean
+ # but we now want it to behave just like check_auth() and return the matrix id of
+ # the user if authentication succeeded or None otherwise
+ if f.__name__ == "check_password":
- def __init__(self, pp, module_api: ModuleApi):
- self._pp = pp
- self._module_api = module_api
+ async def wrapped_check_password(
+ username: str, login_type: str, login_dict: JsonDict
+ ) -> Optional[str]:
+ # We've already made sure f is not None above, but mypy doesn't do well
+ # across function boundaries so we need to tell it f is definitely not
+ # None.
+ assert f is not None
+
+ matrix_user_id = api.get_qualified_user_id(username)
+ password = login_dict["password"]
+
+ is_valid = await f(matrix_user_id, password)
+
+ if is_valid:
+ return matrix_user_id
- self._supported_login_types = {}
+ return None
- # grandfather in check_password support
- if hasattr(self._pp, "check_password"):
- self._supported_login_types[LoginType.PASSWORD] = ("password",)
+ return wrapped_check_password
- g = getattr(self._pp, "get_supported_login_types", None)
- if g:
- self._supported_login_types.update(g())
+ def run(*args, **kwargs):
+ # mypy doesn't do well across function boundaries so we need to tell it
+ # f is definitely not None.
+ assert f is not None
- def __str__(self):
- return str(self._pp)
+ return maybe_awaitable(f(*args, **kwargs))
+
+ return run
+
+ # populate hooks with the implemented methods, wrapped with async_wrapper
+ hooks = {
+ hook: async_wrapper(getattr(provider, hook, None))
+ for hook in password_auth_provider_methods
+ }
+
+ supported_login_types = {}
+ # call get_supported_login_types and add that to the dict
+ g = getattr(provider, "get_supported_login_types", None)
+ if g is not None:
+ # Note the old module style also called get_supported_login_types at loading time
+ # and it is synchronous
+ supported_login_types.update(g())
+
+ auth_checkers = {}
+ # Legacy modules have a check_auth method which expects to be called with one of
+ # the keys returned by get_supported_login_types. New style modules register a
+ # dictionary of login_type->check_auth_method mappings
+ check_auth = async_wrapper(getattr(provider, "check_auth", None))
+ if check_auth is not None:
+ for login_type in supported_login_types:
+ auth_checkers[login_type] = check_auth
+
+ # if it has a "check_password" method then it should handle all auth checks
+ # with login type of LoginType.PASSWORD
+ check_password = async_wrapper(getattr(provider, "check_password", None))
+ if check_password is not None:
+ supported_login_types[LoginType.PASSWORD] = ("password",)
+ auth_checkers[LoginType.PASSWORD] = check_password
+
+ api.register_password_auth_provider_callbacks(
+ hooks, supported_login_types=supported_login_types, auth_checkers=auth_checkers
+ )
+
+
+CHECK_3PID_AUTH = Callable[
+ [str, str, str], Awaitable[Optional[Union[str, Tuple[str, Optional[Callable]]]]]
+]
+ON_LOGGED_OUT = Callable[[str, Optional[str], str], Awaitable]
+CHECK_AUTH = Callable[
+ [str, str, JsonDict],
+ Awaitable[Optional[Union[str, Tuple[str, Optional[Callable]]]]],
+]
+
+
+class PasswordAuthProvider:
+ """
+ A class that the AuthHandler calls when doing authenticating users
+ It allows modules to provide alternative methods for authentication
+ """
+
+ def __init__(self):
+ # lists of callbacks
+ self.check_3pid_auth_callbacks: List[CHECK_3PID_AUTH] = []
+ self.on_logged_out_callbacks: List[ON_LOGGED_OUT] = []
+
+ # Mapping from login type to login parameters
+ self._supported_login_types: Dict[str, Iterable[str]] = {}
+
+ # Mapping from login type to auth checker callbacks
+ self.auth_checker_callbacks: Dict[str, List[CHECK_AUTH]] = {}
+
+ def register_password_auth_provider_callbacks(
+ self,
+ check_3pid_auth: Optional[CHECK_3PID_AUTH] = None,
+ on_logged_out: Optional[ON_LOGGED_OUT] = None,
+ supported_login_types: Optional[Dict[str, Iterable[str]]] = None,
+ auth_checkers: Optional[Dict[str, CHECK_AUTH]] = None,
+ ):
+ # Register check_3pid_auth callback
+ if check_3pid_auth is not None:
+ self.check_3pid_auth_callbacks.append(check_3pid_auth)
+
+ # register on_logged_out callback
+ if on_logged_out is not None:
+ self.on_logged_out_callbacks.append(on_logged_out)
+
+ # register a new supported login_type
+ if supported_login_types is not None:
+ # Iterate through all of the types being registered
+ for login_type, fields in supported_login_types.items():
+ # If a module is supporting a login_type it must also provide a
+ # callback for authenticating using it
+ if auth_checkers is None or auth_checkers.get(login_type) is None:
+ raise Exception(
+ "A module tried to register support for login type: %s without providing"
+ "an authentication method for it" % login_type
+ )
+ # 2 modules supporting the same login type must expect the same fields
+ # e.g. 1 can't expect "pass" if the other expects "password"
+ # so throw an exception if that happens
+ if login_type not in self._supported_login_types.get(login_type, []):
+ self._supported_login_types[login_type] = fields
+ else:
+ fields_currently_supported = self._supported_login_types.get(
+ login_type
+ )
+ if fields_currently_supported != fields:
+ raise Exception(
+ "A module tried to register support for login type: %s with parameters %s"
+ "but another module had already registered support for that type with parameters %s"
+ % (login_type, fields, fields_currently_supported)
+ )
+
+ # register an authentication callback for specific login types
+ if auth_checkers is not None:
+ # Iterate through all of the callbacks being registered
+ for login_type, callback in auth_checkers.items():
+ # If a module is providing a callback for a login type, then it must also register it as
+ # a supported login_type (in order to provide information on what fields it accepts)
+ if (
+ supported_login_types is None
+ or supported_login_types.get(login_type) is None
+ ):
+ raise Exception(
+ "A module tried to register an authentication method for login type: %s"
+ "without listing it as a supported login type" % login_type
+ )
+ # Add the new method to the list of auth_checker_callbacks
+ callback_list = self.auth_checker_callbacks.get(login_type, [])
+ callback_list.append(callback)
+ self.auth_checker_callbacks[login_type] = callback_list
def get_supported_login_types(self) -> Mapping[str, Iterable[str]]:
"""Get the login types supported by this password provider
@@ -1839,10 +1989,8 @@ def get_supported_login_types(self) -> Mapping[str, Iterable[str]]:
Returns a map from a login type identifier (such as m.login.password) to an
iterable giving the fields which must be provided by the user in the submission
to the /login API.
-
- This wrapper adds m.login.password to the list if the underlying password
- provider supports the check_password() api.
"""
+
return self._supported_login_types
async def check_auth(
@@ -1850,9 +1998,6 @@ async def check_auth(
) -> Optional[Tuple[str, Optional[Callable]]]:
"""Check if the user has presented valid login credentials
- This wrapper also calls check_password() if the underlying password provider
- supports the check_password() api and the login type is m.login.password.
-
Args:
username: user id presented by the client. Either an MXID or an unqualified
username.
@@ -1866,63 +2011,50 @@ async def check_auth(
user, and `callback` is an optional callback which will be called with the
result from the /login call (including access_token, device_id, etc.)
"""
- # first grandfather in a call to check_password
- if login_type == LoginType.PASSWORD:
- g = getattr(self._pp, "check_password", None)
- if g:
- qualified_user_id = self._module_api.get_qualified_user_id(username)
- is_valid = await self._pp.check_password(
- qualified_user_id, login_dict["password"]
- )
- if is_valid:
- return qualified_user_id, None
- g = getattr(self._pp, "check_auth", None)
- if not g:
- return None
- result = await g(username, login_type, login_dict)
-
- # 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
- return result, None
+ # Go through all callbacks for the login type until one returns with a value
+ # other than None (i.e. until a callback returns a success)
+ for callback in self.auth_checker_callbacks[login_type]:
+ result = await callback(username, login_type, login_dict)
+ if result is not None:
+ # Check whether result is str or tuple
+ if isinstance(result, str):
+ # If it's a str, set callback function to None
+ return result, None
+ # otherwise it's a (str, callback) tuple so return as is
+ return result
- return result
+ # If this point has been reached then none of the callbacks successfully authenticated
+ # the user so return None
+ return None
async def check_3pid_auth(
self, medium: str, address: str, password: str
) -> Optional[Tuple[str, Optional[Callable]]]:
- g = getattr(self._pp, "check_3pid_auth", None)
- if not g:
- return None
-
# 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 = await g(medium, address, password)
- # 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
- return result, None
+ for callback in self.check_3pid_auth_callbacks:
+ result = await callback(medium, address, password)
+ if result is not None:
+ # Check whether result is str or tuple
+ if isinstance(result, str):
+ # If it's a str, set callback function to None
+ return result, None
+ # otherwise it's a (str, callback) tuple so return as is
+ return result
- return result
+ # If this point has been reached then none of the callbacks successfully authenticated
+ # the user so return None
+ return None
async def on_logged_out(
self, user_id: str, device_id: Optional[str], access_token: str
) -> None:
- g = getattr(self._pp, "on_logged_out", None)
- if not g:
- return
- # This might return an awaitable, if it does block the log out
- # until it completes.
- await maybe_awaitable(
- g(
- user_id=user_id,
- device_id=device_id,
- access_token=access_token,
- )
- )
+ # call all of the on_logged_out callbacks
+ for callback in self.on_logged_out_callbacks:
+ callback(user_id, device_id, access_token)
diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py
index 1cc13fc97b22..fdaca9e49da5 100644
--- a/synapse/module_api/__init__.py
+++ b/synapse/module_api/__init__.py
@@ -111,6 +111,7 @@ def __init__(self, hs: "HomeServer", auth_handler):
self._spam_checker = hs.get_spam_checker()
self._account_validity_handler = hs.get_account_validity_handler()
self._third_party_event_rules = hs.get_third_party_event_rules()
+ self._password_auth_provider = hs.get_password_auth_provider()
#################################################################################
# The following methods should only be called during the module's initialisation.
@@ -130,6 +131,10 @@ def register_third_party_rules_callbacks(self):
"""Registers callbacks for third party event rules capabilities."""
return self._third_party_event_rules.register_third_party_rules_callbacks
+ @property
+ def register_password_auth_provider_callbacks(self):
+ return self._password_auth_provider.register_password_auth_provider_callbacks
+
def register_web_resource(self, path: str, resource: IResource):
"""Registers a web resource to be served at the given path.
diff --git a/synapse/server.py b/synapse/server.py
index 6c867f0f479e..e825f47b4fbf 100644
--- a/synapse/server.py
+++ b/synapse/server.py
@@ -65,7 +65,7 @@
from synapse.handlers.account_validity import AccountValidityHandler
from synapse.handlers.admin import AdminHandler
from synapse.handlers.appservice import ApplicationServicesHandler
-from synapse.handlers.auth import AuthHandler, MacaroonGenerator
+from synapse.handlers.auth import AuthHandler, MacaroonGenerator, PasswordAuthProvider
from synapse.handlers.cas import CasHandler
from synapse.handlers.deactivate_account import DeactivateAccountHandler
from synapse.handlers.device import DeviceHandler, DeviceWorkerHandler
@@ -677,6 +677,10 @@ def get_spam_checker(self) -> SpamChecker:
def get_third_party_event_rules(self) -> ThirdPartyEventRules:
return ThirdPartyEventRules(self)
+ @cache_in_self
+ def get_password_auth_provider(self) -> PasswordAuthProvider:
+ return PasswordAuthProvider()
+
@cache_in_self
def get_room_member_handler(self) -> RoomMemberHandler:
if self.config.worker_app:
From 58d87e9b3cea8c1ba836eb98dd8fd22a5d7a7cb3 Mon Sep 17 00:00:00 2001
From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com>
Date: Fri, 6 Aug 2021 10:09:40 +0100
Subject: [PATCH 02/23] Added tests for new and legacy modules
---
tests/handlers/test_password_providers.py | 217 +++++++++++++++++++---
1 file changed, 196 insertions(+), 21 deletions(-)
diff --git a/tests/handlers/test_password_providers.py b/tests/handlers/test_password_providers.py
index 32651db09669..cced6b0ec728 100644
--- a/tests/handlers/test_password_providers.py
+++ b/tests/handlers/test_password_providers.py
@@ -20,6 +20,8 @@
from twisted.internet import defer
import synapse
+from synapse.handlers.auth import load_legacy_password_auth_provider
+from synapse.module_api import ModuleApi
from synapse.rest.client.v1 import login
from synapse.rest.client.v2_alpha import devices
from synapse.types import JsonDict
@@ -37,8 +39,8 @@
mock_password_provider = Mock()
-class PasswordOnlyAuthProvider:
- """A password_provider which only implements `check_password`."""
+class LegacyPasswordOnlyAuthProvider:
+ """A legacy password_provider which only implements `check_password`."""
@staticmethod
def parse_config(self):
@@ -51,8 +53,8 @@ def check_password(self, *args):
return mock_password_provider.check_password(*args)
-class CustomAuthProvider:
- """A password_provider which implements a custom login type."""
+class LegacyCustomAuthProvider:
+ """A legacy password_provider which implements a custom login type."""
@staticmethod
def parse_config(self):
@@ -68,7 +70,24 @@ def check_auth(self, *args):
return mock_password_provider.check_auth(*args)
-class PasswordCustomAuthProvider:
+class CustomAuthProvider:
+ """A module which registers password_auth_provider callbacks for a custom login type."""
+
+ @staticmethod
+ def parse_config(self):
+ pass
+
+ def __init__(self, config, api: ModuleApi):
+ api.register_password_auth_provider_callbacks(
+ supported_login_types={"test.login_type": ["test_field"]},
+ auth_checkers={"test.login_type": self.check_auth},
+ )
+
+ def check_auth(self, *args):
+ return mock_password_provider.check_auth(*args)
+
+
+class LegacyPasswordCustomAuthProvider:
"""A password_provider which implements password login via `check_auth`, as well
as a custom type."""
@@ -86,8 +105,36 @@ def check_auth(self, *args):
return mock_password_provider.check_auth(*args)
-def providers_config(*providers: Type[Any]) -> dict:
- """Returns a config dict that will enable the given password auth providers"""
+class PasswordCustomAuthProvider:
+ """A module which registers password_auth_provider callbacks for a custom login type.
+ as well as a password login"""
+
+ @staticmethod
+ def parse_config(self):
+ pass
+
+ def __init__(self, config, api: ModuleApi):
+ api.register_password_auth_provider_callbacks(
+ supported_login_types={
+ "test.login_type": ["test_field"],
+ "m.login.password": ["password"],
+ },
+ auth_checkers={
+ "test.login_type": self.check_auth,
+ "m.login.password": self.check_auth,
+ },
+ )
+ pass
+
+ def check_auth(self, *args):
+ return mock_password_provider.check_auth(*args)
+
+ def check_pass(self, *args):
+ return mock_password_provider.check_password(*args)
+
+
+def legacy_providers_config(*providers: Type[Any]) -> dict:
+ """Returns a config dict that will enable the given legacy password auth providers"""
return {
"password_providers": [
{"module": "%s.%s" % (__name__, provider.__qualname__), "config": {}}
@@ -96,6 +143,16 @@ def providers_config(*providers: Type[Any]) -> dict:
}
+def providers_config(*providers: Type[Any]) -> dict:
+ """Returns a config dict that will enable the given modules"""
+ return {
+ "modules": [
+ {"module": "%s.%s" % (__name__, provider.__qualname__), "config": {}}
+ for provider in providers
+ ]
+ }
+
+
class PasswordAuthProviderTests(unittest.HomeserverTestCase):
servlets = [
synapse.rest.admin.register_servlets,
@@ -108,8 +165,24 @@ def setUp(self):
mock_password_provider.reset_mock()
super().setUp()
- @override_config(providers_config(PasswordOnlyAuthProvider))
- def test_password_only_auth_provider_login(self):
+ def make_homeserver(self, reactor, clock):
+ hs = self.setup_test_homeserver()
+ # Load the modules into the homeserver
+ module_api = hs.get_module_api()
+ for module, config in hs.config.modules.loaded_modules:
+ module(config=config, api=module_api)
+ for module, config in hs.config.password_providers:
+ load_legacy_password_auth_provider(
+ module=module, config=config, api=module_api
+ )
+
+ return hs
+
+ @override_config(legacy_providers_config(LegacyPasswordOnlyAuthProvider))
+ def test_password_only_auth_progiver_login_legacy(self):
+ self.password_only_auth_provider_login_test_body()
+
+ def password_only_auth_provider_login_test_body(self):
# login flows should only have m.login.password
flows = self._get_login_flows()
self.assertEqual(flows, [{"type": "m.login.password"}] + ADDITIONAL_LOGIN_FLOWS)
@@ -139,8 +212,11 @@ def test_password_only_auth_provider_login(self):
"@ USER🙂NAME :test", " pASS😢word "
)
- @override_config(providers_config(PasswordOnlyAuthProvider))
- def test_password_only_auth_provider_ui_auth(self):
+ @override_config(legacy_providers_config(LegacyPasswordOnlyAuthProvider))
+ def test_password_only_auth_provider_ui_auth_legacy(self):
+ self.password_only_auth_provider_ui_auth_test_body()
+
+ def password_only_auth_provider_ui_auth_test_body(self):
"""UI Auth should delegate correctly to the password provider"""
# create the user, otherwise access doesn't work
@@ -173,8 +249,11 @@ def test_password_only_auth_provider_ui_auth(self):
self.assertEqual(channel.code, 200)
mock_password_provider.check_password.assert_called_once_with("@u:test", "p")
- @override_config(providers_config(PasswordOnlyAuthProvider))
- def test_local_user_fallback_login(self):
+ @override_config(legacy_providers_config(LegacyPasswordOnlyAuthProvider))
+ def test_local_user_fallback_login_legacy(self):
+ self.local_user_fallback_login_test_body()
+
+ def local_user_fallback_login_test_body(self):
"""rejected login should fall back to local db"""
self.register_user("localuser", "localpass")
@@ -187,8 +266,11 @@ def test_local_user_fallback_login(self):
self.assertEqual(channel.code, 200, channel.result)
self.assertEqual("@localuser:test", channel.json_body["user_id"])
- @override_config(providers_config(PasswordOnlyAuthProvider))
- def test_local_user_fallback_ui_auth(self):
+ @override_config(legacy_providers_config(LegacyPasswordOnlyAuthProvider))
+ def test_local_user_fallback_ui_auth_legacy(self):
+ self.local_user_fallback_ui_auth_test_body()
+
+ def local_user_fallback_ui_auth_test_body(self):
"""rejected login should fall back to local db"""
self.register_user("localuser", "localpass")
@@ -224,11 +306,14 @@ def test_local_user_fallback_ui_auth(self):
@override_config(
{
- **providers_config(PasswordOnlyAuthProvider),
+ **legacy_providers_config(LegacyPasswordOnlyAuthProvider),
"password_config": {"localdb_enabled": False},
}
)
- def test_no_local_user_fallback_login(self):
+ def test_no_local_user_fallback_login_legacy(self):
+ self.no_local_user_fallback_login_test_body()
+
+ def no_local_user_fallback_login_test_body(self):
"""localdb_enabled can block login with the local password"""
self.register_user("localuser", "localpass")
@@ -243,11 +328,14 @@ def test_no_local_user_fallback_login(self):
@override_config(
{
- **providers_config(PasswordOnlyAuthProvider),
+ **legacy_providers_config(LegacyPasswordOnlyAuthProvider),
"password_config": {"localdb_enabled": False},
}
)
- def test_no_local_user_fallback_ui_auth(self):
+ def test_no_local_user_fallback_ui_auth_legacy(self):
+ self.no_local_user_fallback_ui_auth_test_body()
+
+ def no_local_user_fallback_ui_auth_test_body(self):
"""localdb_enabled can block ui auth with the local password"""
self.register_user("localuser", "localpass")
@@ -281,11 +369,14 @@ def test_no_local_user_fallback_ui_auth(self):
@override_config(
{
- **providers_config(PasswordOnlyAuthProvider),
+ **legacy_providers_config(LegacyPasswordOnlyAuthProvider),
"password_config": {"enabled": False},
}
)
- def test_password_auth_disabled(self):
+ def test_password_auth_disabled_legacy(self):
+ self.password_auth_disabled_test_body()
+
+ def password_auth_disabled_test_body(self):
"""password auth doesn't work if it's disabled across the board"""
# login flows should be empty
flows = self._get_login_flows()
@@ -296,8 +387,15 @@ def test_password_auth_disabled(self):
self.assertEqual(channel.code, 400, channel.result)
mock_password_provider.check_password.assert_not_called()
+ @override_config(legacy_providers_config(LegacyCustomAuthProvider))
+ def test_custom_auth_provider_login_legacy(self):
+ self.custom_auth_provider_login_test_body()
+
@override_config(providers_config(CustomAuthProvider))
def test_custom_auth_provider_login(self):
+ self.custom_auth_provider_login_test_body()
+
+ def custom_auth_provider_login_test_body(self):
# login flows should have the custom flow and m.login.password, since we
# haven't disabled local password lookup.
# (password must come first, because reasons)
@@ -335,8 +433,15 @@ def test_custom_auth_provider_login(self):
" USER🙂NAME ", "test.login_type", {"test_field": " abc "}
)
+ @override_config(legacy_providers_config(LegacyCustomAuthProvider))
+ def test_custom_auth_provider_ui_auth_legacy(self):
+ self.custom_auth_provider_ui_auth_test_body()
+
@override_config(providers_config(CustomAuthProvider))
def test_custom_auth_provider_ui_auth(self):
+ self.custom_auth_provider_ui_auth_test_body()
+
+ def custom_auth_provider_ui_auth_test_body(self):
# register the user and log in twice, to get two devices
self.register_user("localuser", "localpass")
tok1 = self.login("localuser", "localpass")
@@ -388,8 +493,15 @@ def test_custom_auth_provider_ui_auth(self):
"localuser", "test.login_type", {"test_field": "foo"}
)
+ @override_config(legacy_providers_config(LegacyCustomAuthProvider))
+ def test_custom_auth_provider_callback_legacy(self):
+ self.custom_auth_provider_callback_test_body()
+
@override_config(providers_config(CustomAuthProvider))
def test_custom_auth_provider_callback(self):
+ self.custom_auth_provider_callback_test_body()
+
+ def custom_auth_provider_callback_test_body(self):
callback = Mock(return_value=defer.succeed(None))
mock_password_provider.check_auth.return_value = defer.succeed(
@@ -411,10 +523,22 @@ def test_custom_auth_provider_callback(self):
for p in ["user_id", "access_token", "device_id", "home_server"]:
self.assertIn(p, call_args[0])
+ @override_config(
+ {
+ **legacy_providers_config(LegacyCustomAuthProvider),
+ "password_config": {"enabled": False},
+ }
+ )
+ def test_custom_auth_password_disabled_legacy(self):
+ self.custom_auth_password_disabled_test_body()
+
@override_config(
{**providers_config(CustomAuthProvider), "password_config": {"enabled": False}}
)
def test_custom_auth_password_disabled(self):
+ self.custom_auth_password_disabled_test_body()
+
+ def custom_auth_password_disabled_test_body(self):
"""Test login with a custom auth provider where password login is disabled"""
self.register_user("localuser", "localpass")
@@ -426,6 +550,15 @@ def test_custom_auth_password_disabled(self):
self.assertEqual(channel.code, 400, channel.result)
mock_password_provider.check_auth.assert_not_called()
+ @override_config(
+ {
+ **legacy_providers_config(LegacyCustomAuthProvider),
+ "password_config": {"enabled": False, "localdb_enabled": False},
+ }
+ )
+ def test_custom_auth_password_disabled_localdb_enabled_legacy(self):
+ self.custom_auth_password_disabled_localdb_enabled_test_body()
+
@override_config(
{
**providers_config(CustomAuthProvider),
@@ -433,6 +566,9 @@ def test_custom_auth_password_disabled(self):
}
)
def test_custom_auth_password_disabled_localdb_enabled(self):
+ self.custom_auth_password_disabled_localdb_enabled_test_body()
+
+ def custom_auth_password_disabled_localdb_enabled_test_body(self):
"""Check the localdb_enabled == enabled == False
Regression test for https://github.com/matrix-org/synapse/issues/8914: check
@@ -449,6 +585,15 @@ def test_custom_auth_password_disabled_localdb_enabled(self):
self.assertEqual(channel.code, 400, channel.result)
mock_password_provider.check_auth.assert_not_called()
+ @override_config(
+ {
+ **legacy_providers_config(LegacyPasswordCustomAuthProvider),
+ "password_config": {"enabled": False},
+ }
+ )
+ def test_password_custom_auth_password_disabled_login_legacy(self):
+ self.password_custom_auth_password_disabled_login_test_body()
+
@override_config(
{
**providers_config(PasswordCustomAuthProvider),
@@ -456,6 +601,9 @@ def test_custom_auth_password_disabled_localdb_enabled(self):
}
)
def test_password_custom_auth_password_disabled_login(self):
+ self.password_custom_auth_password_disabled_login_test_body()
+
+ def password_custom_auth_password_disabled_login_test_body(self):
"""log in with a custom auth provider which implements password, but password
login is disabled"""
self.register_user("localuser", "localpass")
@@ -467,6 +615,16 @@ def test_password_custom_auth_password_disabled_login(self):
channel = self._send_password_login("localuser", "localpass")
self.assertEqual(channel.code, 400, channel.result)
mock_password_provider.check_auth.assert_not_called()
+ mock_password_provider.check_password.assert_not_called()
+
+ @override_config(
+ {
+ **legacy_providers_config(LegacyPasswordCustomAuthProvider),
+ "password_config": {"enabled": False},
+ }
+ )
+ def test_password_custom_auth_password_disabled_ui_auth_legacy(self):
+ self.password_custom_auth_password_disabled_ui_auth_test_body()
@override_config(
{
@@ -475,6 +633,9 @@ def test_password_custom_auth_password_disabled_login(self):
}
)
def test_password_custom_auth_password_disabled_ui_auth(self):
+ self.password_custom_auth_password_disabled_ui_auth_test_body()
+
+ def password_custom_auth_password_disabled_ui_auth_test_body(self):
"""UI Auth with a custom auth provider which implements password, but password
login is disabled"""
# register the user and log in twice via the test login type to get two devices,
@@ -517,6 +678,7 @@ def test_password_custom_auth_password_disabled_ui_auth(self):
"Password login has been disabled.", channel.json_body["error"]
)
mock_password_provider.check_auth.assert_not_called()
+ mock_password_provider.check_password.assert_not_called()
mock_password_provider.reset_mock()
# successful auth
@@ -527,6 +689,16 @@ def test_password_custom_auth_password_disabled_ui_auth(self):
mock_password_provider.check_auth.assert_called_once_with(
"localuser", "test.login_type", {"test_field": "x"}
)
+ mock_password_provider.check_password.assert_not_called()
+
+ @override_config(
+ {
+ **legacy_providers_config(LegacyCustomAuthProvider),
+ "password_config": {"localdb_enabled": False},
+ }
+ )
+ def test_custom_auth_no_local_user_fallback_legacy(self):
+ self.custom_auth_no_local_user_fallback_test_body()
@override_config(
{
@@ -535,6 +707,9 @@ def test_password_custom_auth_password_disabled_ui_auth(self):
}
)
def test_custom_auth_no_local_user_fallback(self):
+ self.custom_auth_no_local_user_fallback_test_body()
+
+ def custom_auth_no_local_user_fallback_test_body(self):
"""Test login with a custom auth provider where the local db is disabled"""
self.register_user("localuser", "localpass")
From accef110e7986ee771dbf7f0afe62f7bf4e3e5a6 Mon Sep 17 00:00:00 2001
From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com>
Date: Fri, 6 Aug 2021 10:11:05 +0100
Subject: [PATCH 03/23] Removed the old module style from sample_config
---
docs/sample_config.yaml | 28 ------------
synapse/config/password_auth_providers.py | 53 ++++++++++-------------
synapse/storage/prepare_database.py | 2 +
3 files changed, 25 insertions(+), 58 deletions(-)
diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml
index aeebcaf45ff0..fc59478beeb4 100644
--- a/docs/sample_config.yaml
+++ b/docs/sample_config.yaml
@@ -2407,34 +2407,6 @@ email:
#email_validation: "[%(server_name)s] Validate your email"
-# Password providers allow homeserver administrators to integrate
-# their Synapse installation with existing authentication methods
-# ex. LDAP, external tokens, etc.
-#
-# For more information and known implementations, please see
-# https://matrix-org.github.io/synapse/latest/password_auth_providers.html
-#
-# Note: instances wishing to use SAML or CAS authentication should
-# instead use the `saml2_config` or `cas_config` options,
-# respectively.
-#
-password_providers:
-# # Example config for an LDAP auth provider
-# - module: "ldap_auth_provider.LdapAuthProvider"
-# config:
-# enabled: true
-# uri: "ldap://ldap.example.com:389"
-# start_tls: true
-# base: "ou=users,dc=example,dc=com"
-# attributes:
-# uid: "cn"
-# mail: "email"
-# name: "givenName"
-# #bind_dn:
-# #bind_password:
-# #filter: "(objectClass=posixAccount)"
-
-
## Push ##
diff --git a/synapse/config/password_auth_providers.py b/synapse/config/password_auth_providers.py
index 0f5b2b397754..16ad3cd4b66e 100644
--- a/synapse/config/password_auth_providers.py
+++ b/synapse/config/password_auth_providers.py
@@ -25,6 +25,29 @@ class PasswordAuthProviderConfig(Config):
section = "authproviders"
def read_config(self, config, **kwargs):
+ """Parses the old account validity config. The config format looks like this:
+
+ password_providers:
+ # Example config for an LDAP auth provider
+ - module: "ldap_auth_provider.LdapAuthProvider"
+ config:
+ enabled: true
+ uri: "ldap://ldap.example.com:389"
+ start_tls: true
+ base: "ou=users,dc=example,dc=com"
+ attributes:
+ uid: "cn"
+ mail: "email"
+ name: "givenName"
+ #bind_dn:
+ #bind_password:
+ #filter: "(objectClass=posixAccount)"
+
+ We expect admins to use modules for this feature (which is why it doesn't appear
+ in the sample config file), but we want to keep support for it around for a bit
+ for backwards compatibility.
+ """
+
self.password_providers: List[Any] = []
providers = []
@@ -49,33 +72,3 @@ def read_config(self, config, **kwargs):
)
self.password_providers.append((provider_class, provider_config))
-
- def generate_config_section(self, **kwargs):
- return """\
- # Password providers allow homeserver administrators to integrate
- # their Synapse installation with existing authentication methods
- # ex. LDAP, external tokens, etc.
- #
- # For more information and known implementations, please see
- # https://matrix-org.github.io/synapse/latest/password_auth_providers.html
- #
- # Note: instances wishing to use SAML or CAS authentication should
- # instead use the `saml2_config` or `cas_config` options,
- # respectively.
- #
- password_providers:
- # # Example config for an LDAP auth provider
- # - module: "ldap_auth_provider.LdapAuthProvider"
- # config:
- # enabled: true
- # uri: "ldap://ldap.example.com:389"
- # start_tls: true
- # base: "ou=users,dc=example,dc=com"
- # attributes:
- # uid: "cn"
- # mail: "email"
- # name: "givenName"
- # #bind_dn:
- # #bind_password:
- # #filter: "(objectClass=posixAccount)"
- """
diff --git a/synapse/storage/prepare_database.py b/synapse/storage/prepare_database.py
index 61392b9639c3..d76b55150664 100644
--- a/synapse/storage/prepare_database.py
+++ b/synapse/storage/prepare_database.py
@@ -545,6 +545,8 @@ def _apply_module_schemas(
database_engine:
config: application config
"""
+ # This is the old way for password_auth_provider modules to make changes
+ # to the database. This should instead be done using the module API
for (mod, _config) in config.password_providers:
if not hasattr(mod, "get_db_schema_files"):
continue
From d84e47cbff9bbadae3a531aab600fe00f164e4e1 Mon Sep 17 00:00:00 2001
From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com>
Date: Fri, 6 Aug 2021 11:22:11 +0100
Subject: [PATCH 04/23] Updated docs with information on new callbacks
---
docs/modules.md | 95 +++++++++++++++++++++++++++++++++
docs/password_auth_providers.md | 6 +++
2 files changed, 101 insertions(+)
diff --git a/docs/modules.md b/docs/modules.md
index 9a430390a4cb..237ddced42de 100644
--- a/docs/modules.md
+++ b/docs/modules.md
@@ -282,6 +282,101 @@ the request is a server admin.
Modules can modify the `request_content` (by e.g. adding events to its `initial_state`),
or deny the room's creation by raising a `module_api.errors.SynapseError`.
+#### Password auth provider callbacks
+
+Password auth providers offer a way for server administrators to integrate
+their Synapse installation with an existing authentication system. The callbacks can be
+registered by using the Module API's `register_password_auth_provider_callbacks` method.
+
+To register authentication checkers, the module should register both of:
+
+- `supported_login_types: Dict[str, Iterable[str]]`
+
+A dict mapping from a login type identifier (such as `m.login.password`) to an
+iterable giving the fields which must be provided by the user in the submission
+to the `/login` API.
+
+For example, if a module wants to implement a custom login type of
+`com.example.custom_login`, where the client is expected to pass the fields `secret1`
+and `secret2` and an alternative password authenticator for `m.login.password`, then
+it should register the following dict:
+
+```python
+{
+ "com.example.custom_login": ("secret1", "secret2"),
+ "m.login.password": ("password",)
+}
+```
+
+- `auth_checkers: Dict[str, CHECK_AUTH]`
+
+A dict mapping from a login type identifier (such as `m.login.password`) to an authentication
+checking method of the following form:
+
+```python
+async def check_auth(
+ username: str,
+ login_type: str,
+ login_dict: JsonDict
+) -> Optional[Union[str, Tuple[str, Optional[Callable]]]]
+```
+
+It is passed the (possibly unqualified) user field provided by the client,
+the login type, and a dictionary of login secrets passed by the client.
+
+If authentication was successful, then it should return either a str containing
+the user's (canonical) User id or a (user_id, callback) tuple, where the callback is
+called with the result from the `/login` call (see the
+spec
+for details).
+
+So continuing the same example, if the module has two authentication checkers, one for
+`com.example.custom_login` called `self.custom_check` and one for `m.login.password` called
+`self.password_check` then it should register the following dict:
+
+```python
+{
+ "com.example.custom_login": self.custom_check,
+ "m.login.password": self.password_check,
+}
+```
+
+Additionally, the following callbacks are available:
+
+```python
+async def check_3pid_auth(
+ medium: str,
+ address: str,
+ password: str,
+) -> Optional[Union[str, Tuple[str, Optional[Callable]]]]
+```
+
+This is called when a user attempts to register or log in with a third party identifier,
+such as email. It is passed the medium (eg. "email"), an address (eg. "jdoe@example.com")
+and the user's password.
+
+The method should return None if the authentication is unsuccessful.
+
+If authentication was successful, then it should return either a str containing
+the user's (canonical) User id or a (user_id, callback) tuple, where the callback is
+called with the result from the `/login` call (see the
+spec
+for details).
+
+```python
+async def on_logged_out(
+ user_id: str,
+ device_id: str,
+ access_token: str,
+) -> None
+```
+This is called when a user logs out. It is passed the qualified user ID, the ID of the
+deactivated device (if any: access tokens are occasionally created without an associated
+device ID), and the (now deactivated) access token.
+
+The callback must be asynchronous but the logout request will wait for the callback
+to complete.
+
### Porting an existing module that uses the old interface
diff --git a/docs/password_auth_providers.md b/docs/password_auth_providers.md
index d2cdb9b2f4a3..d7beacfff3e9 100644
--- a/docs/password_auth_providers.md
+++ b/docs/password_auth_providers.md
@@ -1,3 +1,9 @@
+
+This page of the Synapse documentation is now deprecated. For up to date
+documentation on setting up or writing a password auth provider module, please see
+this page.
+
+
# Password auth provider modules
Password auth providers offer a way for server administrators to
From b19864790773b5de49ea0bdd830be59ecc423826 Mon Sep 17 00:00:00 2001
From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com>
Date: Fri, 6 Aug 2021 11:46:08 +0100
Subject: [PATCH 05/23] Added porting instructions to the docs
---
docs/modules.md | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/docs/modules.md b/docs/modules.md
index 237ddced42de..7a5dd1615be4 100644
--- a/docs/modules.md
+++ b/docs/modules.md
@@ -304,7 +304,7 @@ it should register the following dict:
```python
{
"com.example.custom_login": ("secret1", "secret2"),
- "m.login.password": ("password",)
+ "m.login.password": ("password",),
}
```
@@ -396,6 +396,22 @@ The module's author should also update any example in the module's configuration
use the new `modules` section in Synapse's configuration file (see [this section](#using-modules)
for more info).
+#### Porting password auth provider modules
+
+There is no longer a `get_db_schema_files` callback provided, any changes to the database
+should now be made by the module using the module API class.
+
+To port a module that has a `check_password` method:
+- Register `"m.login.password": ("password",)` as a supported login type
+- Register `"m.login.password": self.check_password` as an auth checker
+- Change the arguments of `check_password` to
+ `(username: str, login_type: str, login_dict: JsonDict)`
+- Add the following lines to the top of the `check_password` method:
+ - `user_id = self.module_api.get_qualified_user_id(username)`
+ - `password = login_dict["password"]`
+- Alter `check_password` so that it returns `None` on an authentication failure, and `user_id`
+ on a success
+
### Example
The example below is a module that implements the spam checker callback
From d7d89ce046ec28345de7f30f2a98d141e9bbef54 Mon Sep 17 00:00:00 2001
From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com>
Date: Fri, 6 Aug 2021 12:27:45 +0100
Subject: [PATCH 06/23] added changelog
---
changelog.d/10548.feature | 1 +
1 file changed, 1 insertion(+)
create mode 100644 changelog.d/10548.feature
diff --git a/changelog.d/10548.feature b/changelog.d/10548.feature
new file mode 100644
index 000000000000..263a811faf16
--- /dev/null
+++ b/changelog.d/10548.feature
@@ -0,0 +1 @@
+Port the Password Auth Providers module interface to the new generic interface.
\ No newline at end of file
From 1f87fbd473258e55829811db4cbee21a2910eeb8 Mon Sep 17 00:00:00 2001
From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com>
Date: Fri, 6 Aug 2021 15:41:25 +0100
Subject: [PATCH 07/23] removed now unused ModuleApi object in auth handler
---
synapse/handlers/auth.py | 8 --------
1 file changed, 8 deletions(-)
diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index fe77b0718187..9c75ac34c99f 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -199,14 +199,6 @@ def __init__(self, hs: "HomeServer"):
self.bcrypt_rounds = hs.config.bcrypt_rounds
- # we can't use hs.get_module_api() here, because to do so will create an
- # import loop.
- #
- # TODO: refactor this class to separate the lower-level stuff that
- # ModuleApi can use from the higher-level stuff that uses ModuleApi, as
- # better way to break the loop
- account_handler = ModuleApi(hs, self)
-
self.password_auth_provider = hs.get_password_auth_provider()
self.hs = hs # FIXME better possibility to access registrationHandler later?
From 6ad6ccdea45a2aa9f4dc3aff83ba62e899851d24 Mon Sep 17 00:00:00 2001
From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com>
Date: Wed, 11 Aug 2021 11:02:32 +0100
Subject: [PATCH 08/23] Added error handling when calling callbacks
Also renamed the callback types to the form
of XYZ_CALLBACK
---
docs/modules.md | 5 +-
synapse/handlers/auth.py | 118 +++++++++++++++++++++++++++++++++------
2 files changed, 104 insertions(+), 19 deletions(-)
diff --git a/docs/modules.md b/docs/modules.md
index 7a5dd1615be4..2c796f92ca68 100644
--- a/docs/modules.md
+++ b/docs/modules.md
@@ -308,7 +308,7 @@ it should register the following dict:
}
```
-- `auth_checkers: Dict[str, CHECK_AUTH]`
+- `auth_checkers: Dict[str, CHECK_AUTH_CALLBACK]`
A dict mapping from a login type identifier (such as `m.login.password`) to an authentication
checking method of the following form:
@@ -331,7 +331,7 @@ called with the result from the `/login` call (see the
for details).
So continuing the same example, if the module has two authentication checkers, one for
-`com.example.custom_login` called `self.custom_check` and one for `m.login.password` called
+`com.example.custom_login` named `self.custom_check` and one for `m.login.password` named
`self.password_check` then it should register the following dict:
```python
@@ -404,6 +404,7 @@ should now be made by the module using the module API class.
To port a module that has a `check_password` method:
- Register `"m.login.password": ("password",)` as a supported login type
- Register `"m.login.password": self.check_password` as an auth checker
+- Set `self.module_api` to point to the `ModuleApi` object given in `__init__`
- Change the arguments of `check_password` to
`(username: str, login_type: str, login_dict: JsonDict)`
- Add the following lines to the top of the `check_password` method:
diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index 9c75ac34c99f..25183dda020a 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -1887,11 +1887,11 @@ def run(*args, **kwargs):
)
-CHECK_3PID_AUTH = Callable[
+CHECK_3PID_AUTH_CALLBACK = Callable[
[str, str, str], Awaitable[Optional[Union[str, Tuple[str, Optional[Callable]]]]]
]
-ON_LOGGED_OUT = Callable[[str, Optional[str], str], Awaitable]
-CHECK_AUTH = Callable[
+ON_LOGGED_OUT_CALLBACK = Callable[[str, Optional[str], str], Awaitable]
+CHECK_AUTH_CALLBACK = Callable[
[str, str, JsonDict],
Awaitable[Optional[Union[str, Tuple[str, Optional[Callable]]]]],
]
@@ -1905,21 +1905,21 @@ class PasswordAuthProvider:
def __init__(self):
# lists of callbacks
- self.check_3pid_auth_callbacks: List[CHECK_3PID_AUTH] = []
- self.on_logged_out_callbacks: List[ON_LOGGED_OUT] = []
+ self.check_3pid_auth_callbacks: List[CHECK_3PID_AUTH_CALLBACK] = []
+ self.on_logged_out_callbacks: List[ON_LOGGED_OUT_CALLBACK] = []
# Mapping from login type to login parameters
self._supported_login_types: Dict[str, Iterable[str]] = {}
# Mapping from login type to auth checker callbacks
- self.auth_checker_callbacks: Dict[str, List[CHECK_AUTH]] = {}
+ self.auth_checker_callbacks: Dict[str, List[CHECK_AUTH_CALLBACK]] = {}
def register_password_auth_provider_callbacks(
self,
- check_3pid_auth: Optional[CHECK_3PID_AUTH] = None,
- on_logged_out: Optional[ON_LOGGED_OUT] = None,
+ check_3pid_auth: Optional[CHECK_3PID_AUTH_CALLBACK] = None,
+ on_logged_out: Optional[ON_LOGGED_OUT_CALLBACK] = None,
supported_login_types: Optional[Dict[str, Iterable[str]]] = None,
- auth_checkers: Optional[Dict[str, CHECK_AUTH]] = None,
+ auth_checkers: Optional[Dict[str, CHECK_AUTH_CALLBACK]] = None,
):
# Register check_3pid_auth callback
if check_3pid_auth is not None:
@@ -1936,7 +1936,7 @@ def register_password_auth_provider_callbacks(
# If a module is supporting a login_type it must also provide a
# callback for authenticating using it
if auth_checkers is None or auth_checkers.get(login_type) is None:
- raise Exception(
+ raise RuntimeError(
"A module tried to register support for login type: %s without providing"
"an authentication method for it" % login_type
)
@@ -1950,7 +1950,7 @@ def register_password_auth_provider_callbacks(
login_type
)
if fields_currently_supported != fields:
- raise Exception(
+ raise RuntimeError(
"A module tried to register support for login type: %s with parameters %s"
"but another module had already registered support for that type with parameters %s"
% (login_type, fields, fields_currently_supported)
@@ -1966,7 +1966,7 @@ def register_password_auth_provider_callbacks(
supported_login_types is None
or supported_login_types.get(login_type) is None
):
- raise Exception(
+ raise RuntimeError(
"A module tried to register an authentication method for login type: %s"
"without listing it as a supported login type" % login_type
)
@@ -2007,13 +2007,53 @@ async def check_auth(
# Go through all callbacks for the login type until one returns with a value
# other than None (i.e. until a callback returns a success)
for callback in self.auth_checker_callbacks[login_type]:
- result = await callback(username, login_type, login_dict)
+ try:
+ result = await callback(username, login_type, login_dict)
+ except Exception as e:
+ logger.warning("Failed to run module API callback %s: %s", callback, e)
+ continue
+
if result is not None:
# Check whether result is str or tuple
if isinstance(result, str):
# If it's a str, set callback function to None
return result, None
- # otherwise it's a (str, callback) tuple so return as is
+
+ # if it's not a str then it should be a tuple
+ if not isinstance(result, Tuple):
+ logger.warning(
+ "Wrong type returned by module API callback %s: %s, expected"
+ " Optional[Union[str, Tuple[str, Optional[Callable]]]]",
+ callback,
+ result,
+ )
+ continue
+
+ # pull out the two parts of the tuple so we can do type checking
+ str_result, callback_result = result
+
+ # the 1st item in the tuple should be a str
+ if not isinstance(str_result, str):
+ logger.warning(
+ "Wrong type returned by module API callback %s: %s, expected"
+ " Optional[Union[str, Tuple[str, Optional[Callable]]]]",
+ callback,
+ result,
+ )
+ continue
+
+ # the second should be Optional[Callable]
+ if callback_result is not None:
+ if not isinstance(callback_result, Callable):
+ logger.warning(
+ "Wrong type returned by module API callback %s: %s, expected"
+ " Optional[Union[str, Tuple[str, Optional[Callable]]]]",
+ callback,
+ result,
+ )
+ continue
+
+ # The result is a (str, Optional[callback]) tuple so return the successful result
return result
# If this point has been reached then none of the callbacks successfully authenticated
@@ -2030,13 +2070,53 @@ async def check_3pid_auth(
# after we've finished everything else
for callback in self.check_3pid_auth_callbacks:
- result = await callback(medium, address, password)
+ try:
+ result = await callback(medium, address, password)
+ except Exception as e:
+ logger.warning("Failed to run module API callback %s: %s", callback, e)
+ continue
+
if result is not None:
# Check whether result is str or tuple
if isinstance(result, str):
# If it's a str, set callback function to None
return result, None
- # otherwise it's a (str, callback) tuple so return as is
+
+ # if it's not a str then it should be a tuple
+ if not isinstance(result, Tuple):
+ logger.warning(
+ "Wrong type returned by module API callback %s: %s, expected"
+ " Optional[Union[str, Tuple[str, Optional[Callable]]]]",
+ callback,
+ result,
+ )
+ continue
+
+ # pull out the two parts of the tuple so we can do type checking
+ str_result, callback_result = result
+
+ # the 1st item in the tuple should be a str
+ if not isinstance(str_result, str):
+ logger.warning(
+ "Wrong type returned by module API callback %s: %s, expected"
+ " Optional[Union[str, Tuple[str, Optional[Callable]]]]",
+ callback,
+ result,
+ )
+ continue
+
+ # the second should be Optional[Callable]
+ if callback_result is not None:
+ if not isinstance(callback_result, Callable):
+ logger.warning(
+ "Wrong type returned by module API callback %s: %s, expected"
+ " Optional[Union[str, Tuple[str, Optional[Callable]]]]",
+ callback,
+ result,
+ )
+ continue
+
+ # The result is a (str, Optional[callback]) tuple so return the successful result
return result
# If this point has been reached then none of the callbacks successfully authenticated
@@ -2049,4 +2129,8 @@ async def on_logged_out(
# call all of the on_logged_out callbacks
for callback in self.on_logged_out_callbacks:
- callback(user_id, device_id, access_token)
+ try:
+ callback(user_id, device_id, access_token)
+ except Exception as e:
+ logger.warning("Failed to run module API callback %s: %s", callback, e)
+ continue
From efeb758c6b5c9d22905df7482ad22e33ab718ada Mon Sep 17 00:00:00 2001
From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com>
Date: Wed, 11 Aug 2021 12:35:34 +0100
Subject: [PATCH 09/23] auth checkers now MUST return tuple
(could previously return either a string or a tuple)
---
docs/modules.md | 20 ++---
synapse/handlers/auth.py | 90 ++++++++++++++++-------
tests/handlers/test_password_providers.py | 14 ++--
3 files changed, 83 insertions(+), 41 deletions(-)
diff --git a/docs/modules.md b/docs/modules.md
index 2c796f92ca68..f5b60d5d0e61 100644
--- a/docs/modules.md
+++ b/docs/modules.md
@@ -318,17 +318,17 @@ async def check_auth(
username: str,
login_type: str,
login_dict: JsonDict
-) -> Optional[Union[str, Tuple[str, Optional[Callable]]]]
+) -> Optional[Tuple[str, Optional[Callable]]]
```
It is passed the (possibly unqualified) user field provided by the client,
the login type, and a dictionary of login secrets passed by the client.
-If authentication was successful, then it should return either a str containing
-the user's (canonical) User id or a (user_id, callback) tuple, where the callback is
-called with the result from the `/login` call (see the
+On a failure it should return None. On a success it should return a (user_id, callback) tuple
+where user_id is a str containing the user's (canonical) matrix id and the callback is a method
+to be called with the result from the `/login` call (see the
spec
-for details).
+for details). This callback can be None.
So continuing the same example, if the module has two authentication checkers, one for
`com.example.custom_login` named `self.custom_check` and one for `m.login.password` named
@@ -348,7 +348,7 @@ async def check_3pid_auth(
medium: str,
address: str,
password: str,
-) -> Optional[Union[str, Tuple[str, Optional[Callable]]]]
+) -> Optional[Tuple[str, Optional[Callable]]]
```
This is called when a user attempts to register or log in with a third party identifier,
@@ -357,11 +357,11 @@ and the user's password.
The method should return None if the authentication is unsuccessful.
-If authentication was successful, then it should return either a str containing
-the user's (canonical) User id or a (user_id, callback) tuple, where the callback is
-called with the result from the `/login` call (see the
+On a failure it should return None. On a success it should return a (user_id, callback) tuple
+where user_id is a str containing the user's (canonical) matrix id and the callback is a method
+to be called with the result from the `/login` call (see the
spec
-for details).
+for details). This callback can be None.
```python
async def on_logged_out(
diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index 25183dda020a..3c239a2c3100 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -1825,7 +1825,7 @@ def async_wrapper(f: Optional[Callable]) -> Optional[Callable[..., Awaitable]]:
async def wrapped_check_password(
username: str, login_type: str, login_dict: JsonDict
- ) -> Optional[str]:
+ ) -> Optional[Tuple[str, Optional[Callable]]]:
# We've already made sure f is not None above, but mypy doesn't do well
# across function boundaries so we need to tell it f is definitely not
# None.
@@ -1837,12 +1837,54 @@ async def wrapped_check_password(
is_valid = await f(matrix_user_id, password)
if is_valid:
- return matrix_user_id
+ return matrix_user_id, None
return None
return wrapped_check_password
+ # We need to wrap check_auth as in the old form it could return
+ # just a str, but now it must return Optional[Tuple[str, Optional[Callable]]
+ if f.__name__ == "check_auth":
+
+ async def wrapped_check_auth(
+ username: str, login_type: str, login_dict: JsonDict
+ ) -> Optional[Tuple[str, Optional[Callable]]]:
+ # We've already made sure f is not None above, but mypy doesn't do well
+ # across function boundaries so we need to tell it f is definitely not
+ # None.
+ assert f is not None
+
+ result = await f(username, login_type, login_dict)
+
+ if isinstance(result, str):
+ return result, None
+
+ return result
+
+ return wrapped_check_auth
+
+ # We need to wrap check_3pid_auth as in the old form it could return
+ # just a str, but now it must return Optional[Tuple[str, Optional[Callable]]
+ if f.__name__ == "check_3pid_auth":
+
+ async def wrapped_check_3pid_auth(
+ medium: str, address: str, password: str
+ ) -> Optional[Tuple[str, Optional[Callable]]]:
+ # We've already made sure f is not None above, but mypy doesn't do well
+ # across function boundaries so we need to tell it f is definitely not
+ # None.
+ assert f is not None
+
+ result = await f(medium, address, password)
+
+ if isinstance(result, str):
+ return result, None
+
+ return result
+
+ return wrapped_check_3pid_auth
+
def run(*args, **kwargs):
# mypy doesn't do well across function boundaries so we need to tell it
# f is definitely not None.
@@ -1888,12 +1930,12 @@ def run(*args, **kwargs):
CHECK_3PID_AUTH_CALLBACK = Callable[
- [str, str, str], Awaitable[Optional[Union[str, Tuple[str, Optional[Callable]]]]]
+ [str, str, str], Awaitable[Optional[Tuple[str, Optional[Callable]]]]
]
ON_LOGGED_OUT_CALLBACK = Callable[[str, Optional[str], str], Awaitable]
CHECK_AUTH_CALLBACK = Callable[
[str, str, JsonDict],
- Awaitable[Optional[Union[str, Tuple[str, Optional[Callable]]]]],
+ Awaitable[Optional[Tuple[str, Optional[Callable]]]],
]
@@ -2014,16 +2056,14 @@ async def check_auth(
continue
if result is not None:
- # Check whether result is str or tuple
- if isinstance(result, str):
- # If it's a str, set callback function to None
- return result, None
+ # Check that the callback returned a Tuple[str, Optional[Callable]]
- # if it's not a str then it should be a tuple
- if not isinstance(result, Tuple):
+ # "type: ignore" is used on the isinstance checks because mypy thinks
+ # result is always the right type, but as it is 3rd party code it might not be
+ if not isinstance(result, Tuple) or len(result) != 2: # type: ignore
logger.warning(
"Wrong type returned by module API callback %s: %s, expected"
- " Optional[Union[str, Tuple[str, Optional[Callable]]]]",
+ " Optional[Tuple[str, Optional[Callable]]]",
callback,
result,
)
@@ -2034,9 +2074,9 @@ async def check_auth(
# the 1st item in the tuple should be a str
if not isinstance(str_result, str):
- logger.warning(
+ logger.warning( # type: ignore[unreachable]
"Wrong type returned by module API callback %s: %s, expected"
- " Optional[Union[str, Tuple[str, Optional[Callable]]]]",
+ " Optional[Tuple[str, Optional[Callable]]]",
callback,
result,
)
@@ -2044,10 +2084,10 @@ async def check_auth(
# the second should be Optional[Callable]
if callback_result is not None:
- if not isinstance(callback_result, Callable):
+ if not isinstance(callback_result, Callable): # type: ignore
logger.warning(
"Wrong type returned by module API callback %s: %s, expected"
- " Optional[Union[str, Tuple[str, Optional[Callable]]]]",
+ " Optional[Tuple[str, Optional[Callable]]]",
callback,
result,
)
@@ -2077,16 +2117,14 @@ async def check_3pid_auth(
continue
if result is not None:
- # Check whether result is str or tuple
- if isinstance(result, str):
- # If it's a str, set callback function to None
- return result, None
+ # Check that the callback returned a Tuple[str, Optional[Callable]]
- # if it's not a str then it should be a tuple
- if not isinstance(result, Tuple):
+ # "type: ignore" is used on the isinstance checks because mypy thinks
+ # result is always the right type, but as it is 3rd party code it might not be
+ if not isinstance(result, Tuple) or len(result) != 2: # type: ignore
logger.warning(
"Wrong type returned by module API callback %s: %s, expected"
- " Optional[Union[str, Tuple[str, Optional[Callable]]]]",
+ " Optional[Tuple[str, Optional[Callable]]]",
callback,
result,
)
@@ -2097,9 +2135,9 @@ async def check_3pid_auth(
# the 1st item in the tuple should be a str
if not isinstance(str_result, str):
- logger.warning(
+ logger.warning( # type: ignore[unreachable]
"Wrong type returned by module API callback %s: %s, expected"
- " Optional[Union[str, Tuple[str, Optional[Callable]]]]",
+ " Optional[Tuple[str, Optional[Callable]]]",
callback,
result,
)
@@ -2107,10 +2145,10 @@ async def check_3pid_auth(
# the second should be Optional[Callable]
if callback_result is not None:
- if not isinstance(callback_result, Callable):
+ if not isinstance(callback_result, Callable): # type: ignore
logger.warning(
"Wrong type returned by module API callback %s: %s, expected"
- " Optional[Union[str, Tuple[str, Optional[Callable]]]]",
+ " Optional[Tuple[str, Optional[Callable]]]",
callback,
result,
)
diff --git a/tests/handlers/test_password_providers.py b/tests/handlers/test_password_providers.py
index cced6b0ec728..4a999f2f3502 100644
--- a/tests/handlers/test_password_providers.py
+++ b/tests/handlers/test_password_providers.py
@@ -411,7 +411,9 @@ def custom_auth_provider_login_test_body(self):
self.assertEqual(channel.code, 400, channel.result)
mock_password_provider.check_auth.assert_not_called()
- mock_password_provider.check_auth.return_value = defer.succeed("@user:bz")
+ mock_password_provider.check_auth.return_value = defer.succeed(
+ ("@user:bz", None)
+ )
channel = self._send_login("test.login_type", "u", test_field="y")
self.assertEqual(channel.code, 200, channel.result)
self.assertEqual("@user:bz", channel.json_body["user_id"])
@@ -424,7 +426,7 @@ def custom_auth_provider_login_test_body(self):
# in these cases, but at least we can guard against the API changing
# unexpectedly
mock_password_provider.check_auth.return_value = defer.succeed(
- "@ MALFORMED! :bz"
+ ("@ MALFORMED! :bz", None)
)
channel = self._send_login("test.login_type", " USER🙂NAME ", test_field=" abc ")
self.assertEqual(channel.code, 200, channel.result)
@@ -473,7 +475,9 @@ def custom_auth_provider_ui_auth_test_body(self):
mock_password_provider.reset_mock()
# right params, but authing as the wrong user
- mock_password_provider.check_auth.return_value = defer.succeed("@user:bz")
+ mock_password_provider.check_auth.return_value = defer.succeed(
+ ("@user:bz", None)
+ )
body["auth"]["test_field"] = "foo"
channel = self._delete_device(tok1, "dev2", body)
self.assertEqual(channel.code, 403)
@@ -485,7 +489,7 @@ def custom_auth_provider_ui_auth_test_body(self):
# and finally, succeed
mock_password_provider.check_auth.return_value = defer.succeed(
- "@localuser:test"
+ ("@localuser:test", None)
)
channel = self._delete_device(tok1, "dev2", body)
self.assertEqual(channel.code, 200)
@@ -641,7 +645,7 @@ def password_custom_auth_password_disabled_ui_auth_test_body(self):
# register the user and log in twice via the test login type to get two devices,
self.register_user("localuser", "localpass")
mock_password_provider.check_auth.return_value = defer.succeed(
- "@localuser:test"
+ ("@localuser:test", None)
)
channel = self._send_login("test.login_type", "localuser", test_field="")
self.assertEqual(channel.code, 200, channel.result)
From e6bfd491ff4e55faf89bae43a82097aa11ebe390 Mon Sep 17 00:00:00 2001
From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com>
Date: Thu, 19 Aug 2021 09:23:56 +0100
Subject: [PATCH 10/23] Removed duplicated statement in the docs
---
docs/modules.md | 2 --
1 file changed, 2 deletions(-)
diff --git a/docs/modules.md b/docs/modules.md
index f5b60d5d0e61..f1f3e8efa109 100644
--- a/docs/modules.md
+++ b/docs/modules.md
@@ -355,8 +355,6 @@ This is called when a user attempts to register or log in with a third party id
such as email. It is passed the medium (eg. "email"), an address (eg. "jdoe@example.com")
and the user's password.
-The method should return None if the authentication is unsuccessful.
-
On a failure it should return None. On a success it should return a (user_id, callback) tuple
where user_id is a str containing the user's (canonical) matrix id and the callback is a method
to be called with the result from the `/login` call (see the
From 3083565ddb522a03970ab602b2685f5f246ed2cd Mon Sep 17 00:00:00 2001
From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com>
Date: Mon, 23 Aug 2021 10:25:20 +0100
Subject: [PATCH 11/23] Applied suggestions from code review:
Reworded some stuff
Added a docstring
Added a newline in docs
Added some spaces so string concatinate as expected
Made mypy '# type: ignore' comments more selective
---
docs/modules.md | 3 ++-
synapse/config/password_auth_providers.py | 2 +-
synapse/handlers/auth.py | 16 ++++++++--------
synapse/module_api/__init__.py | 1 +
4 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/docs/modules.md b/docs/modules.md
index 662a24503aa9..a0d5763bf0f3 100644
--- a/docs/modules.md
+++ b/docs/modules.md
@@ -328,10 +328,11 @@ For example, if the user `@alice:example.org` is passed to this method, and the
`{"@bob:example.com", "@charlie:somewhere.org"}` is returned, this signifies that Alice
should receive presence updates sent by Bob and Charlie, regardless of whether these users
share a room.
+
#### Password auth provider callbacks
Password auth providers offer a way for server administrators to integrate
-their Synapse installation with an existing authentication system. The callbacks can be
+their Synapse installation with an external authentication system. The callbacks can be
registered by using the Module API's `register_password_auth_provider_callbacks` method.
To register authentication checkers, the module should register both of:
diff --git a/synapse/config/password_auth_providers.py b/synapse/config/password_auth_providers.py
index 16ad3cd4b66e..e621a2d8101d 100644
--- a/synapse/config/password_auth_providers.py
+++ b/synapse/config/password_auth_providers.py
@@ -25,7 +25,7 @@ class PasswordAuthProviderConfig(Config):
section = "authproviders"
def read_config(self, config, **kwargs):
- """Parses the old account validity config. The config format looks like this:
+ """Parses the old password auth providers config. The config format looks like this:
password_providers:
# Example config for an LDAP auth provider
diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index b8da2cb4ba7f..6e3c061c721a 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -1946,7 +1946,7 @@ def run(*args, **kwargs):
class PasswordAuthProvider:
"""
- A class that the AuthHandler calls when doing authenticating users
+ A class that the AuthHandler calls when authenticating users
It allows modules to provide alternative methods for authentication
"""
@@ -1985,7 +1985,7 @@ def register_password_auth_provider_callbacks(
if auth_checkers is None or auth_checkers.get(login_type) is None:
raise RuntimeError(
"A module tried to register support for login type: %s without providing"
- "an authentication method for it" % login_type
+ " an authentication method for it" % login_type
)
# 2 modules supporting the same login type must expect the same fields
# e.g. 1 can't expect "pass" if the other expects "password"
@@ -1999,7 +1999,7 @@ def register_password_auth_provider_callbacks(
if fields_currently_supported != fields:
raise RuntimeError(
"A module tried to register support for login type: %s with parameters %s"
- "but another module had already registered support for that type with parameters %s"
+ " but another module had already registered support for that type with parameters %s"
% (login_type, fields, fields_currently_supported)
)
@@ -2015,7 +2015,7 @@ def register_password_auth_provider_callbacks(
):
raise RuntimeError(
"A module tried to register an authentication method for login type: %s"
- "without listing it as a supported login type" % login_type
+ " without listing it as a supported login type" % login_type
)
# Add the new method to the list of auth_checker_callbacks
callback_list = self.auth_checker_callbacks.get(login_type, [])
@@ -2065,7 +2065,7 @@ async def check_auth(
# "type: ignore" is used on the isinstance checks because mypy thinks
# result is always the right type, but as it is 3rd party code it might not be
- if not isinstance(result, Tuple) or len(result) != 2: # type: ignore
+ if not isinstance(result, Tuple) or len(result) != 2: # type: ignore[arg-type]
logger.warning(
"Wrong type returned by module API callback %s: %s, expected"
" Optional[Tuple[str, Optional[Callable]]]",
@@ -2089,7 +2089,7 @@ async def check_auth(
# the second should be Optional[Callable]
if callback_result is not None:
- if not isinstance(callback_result, Callable): # type: ignore
+ if not isinstance(callback_result, Callable): # type: ignore[arg-type]
logger.warning(
"Wrong type returned by module API callback %s: %s, expected"
" Optional[Tuple[str, Optional[Callable]]]",
@@ -2126,7 +2126,7 @@ async def check_3pid_auth(
# "type: ignore" is used on the isinstance checks because mypy thinks
# result is always the right type, but as it is 3rd party code it might not be
- if not isinstance(result, Tuple) or len(result) != 2: # type: ignore
+ if not isinstance(result, Tuple) or len(result) != 2: # type: ignore[arg-type]
logger.warning(
"Wrong type returned by module API callback %s: %s, expected"
" Optional[Tuple[str, Optional[Callable]]]",
@@ -2150,7 +2150,7 @@ async def check_3pid_auth(
# the second should be Optional[Callable]
if callback_result is not None:
- if not isinstance(callback_result, Callable): # type: ignore
+ if not isinstance(callback_result, Callable): # type: ignore[arg-type]
logger.warning(
"Wrong type returned by module API callback %s: %s, expected"
" Optional[Tuple[str, Optional[Callable]]]",
diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py
index 4843f6b8fed3..4648faedde93 100644
--- a/synapse/module_api/__init__.py
+++ b/synapse/module_api/__init__.py
@@ -144,6 +144,7 @@ def register_presence_router_callbacks(self):
@property
def register_password_auth_provider_callbacks(self):
+ """Registers callbacks for password auth provider capabilities."""
return self._password_auth_provider.register_password_auth_provider_callbacks
def register_web_resource(self, path: str, resource: IResource):
From 4f18f1bafdc8d8e0786d4e9c41bc556786fa30c0 Mon Sep 17 00:00:00 2001
From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com>
Date: Mon, 23 Aug 2021 12:35:31 +0100
Subject: [PATCH 12/23] Register login_type, fields, auth_checker all in one
chunk
---
docs/modules.md | 63 +++++++++++------------
synapse/handlers/auth.py | 57 +++++++++-----------
tests/handlers/test_password_providers.py | 11 ++--
3 files changed, 57 insertions(+), 74 deletions(-)
diff --git a/docs/modules.md b/docs/modules.md
index a0d5763bf0f3..d13c9f74e215 100644
--- a/docs/modules.md
+++ b/docs/modules.md
@@ -335,30 +335,37 @@ Password auth providers offer a way for server administrators to integrate
their Synapse installation with an external authentication system. The callbacks can be
registered by using the Module API's `register_password_auth_provider_callbacks` method.
-To register authentication checkers, the module should register both of:
+To register authentication checkers, the module should register:
-- `supported_login_types: Dict[str, Iterable[str]]`
+```
+ auth_checkers: Dict[Tuple[str,Tuple], CHECK_AUTH_CALLBACK]
+```
+
+A dict mapping from tuples of a login type identifier (such as `m.login.password`) and a
+tuple of field names (such as `("password", "secret_thing")`) to authentication checking
+methods.
-A dict mapping from a login type identifier (such as `m.login.password`) to an
-iterable giving the fields which must be provided by the user in the submission
+The login type and field names are the things that are provided by the user in their submission
to the `/login` API.
-For example, if a module wants to implement a custom login type of
-`com.example.custom_login`, where the client is expected to pass the fields `secret1`
-and `secret2` and an alternative password authenticator for `m.login.password`, then
-it should register the following dict:
+For example, if a module implements authentication checkers for two different login types:
+- `com.example.custom_login`
+ - Expects `secret1` and `secret2` fields to be sent to `/login`
+ - Is checked by the method: `self.custom_login_check`
+- `m.login.password`
+ - Expects a `password` field to be sent to `/login`
+ - Is checked by the method: `self.password_checker`
+
+Then it should register the following dict:
```python
-{
- "com.example.custom_login": ("secret1", "secret2"),
- "m.login.password": ("password",),
+{
+ ("com.example.custom_login", ("secret1", "secret2")): self.check_custom_login,
+ ("m.login.password", ("password",)): self.password_checker,
}
```
-- `auth_checkers: Dict[str, CHECK_AUTH_CALLBACK]`
-
-A dict mapping from a login type identifier (such as `m.login.password`) to an authentication
-checking method of the following form:
+An authentication checking method should be of the following form:
```python
async def check_auth(
@@ -368,27 +375,16 @@ async def check_auth(
) -> Optional[Tuple[str, Optional[Callable]]]
```
-It is passed the (possibly unqualified) user field provided by the client,
+It is passed the user field provided by the client (which might not be in `@username:server` form),
the login type, and a dictionary of login secrets passed by the client.
On a failure it should return None. On a success it should return a (user_id, callback) tuple
-where user_id is a str containing the user's (canonical) matrix id and the callback is a method
-to be called with the result from the `/login` call (see the
+where user_id is a str containing the user's matrix id (i.e. in `@username:server` form) and the
+callback is a method to be called with the result from the `/login` call (see the
spec
for details). This callback can be None.
-So continuing the same example, if the module has two authentication checkers, one for
-`com.example.custom_login` named `self.custom_check` and one for `m.login.password` named
-`self.password_check` then it should register the following dict:
-
-```python
-{
- "com.example.custom_login": self.custom_check,
- "m.login.password": self.password_check,
-}
-```
-
-Additionally, the following callbacks are available:
+Aditionally the module can register:
```python
async def check_3pid_auth(
@@ -403,8 +399,8 @@ such as email. It is passed the medium (eg. "email"), an address (eg. "jdoe@exam
and the user's password.
On a failure it should return None. On a success it should return a (user_id, callback) tuple
-where user_id is a str containing the user's (canonical) matrix id and the callback is a method
-to be called with the result from the `/login` call (see the
+where user_id is a str containing the user's matrix id (i.e. in `@username:server` form) and the
+callback is a method to be called with the result from the `/login` call (see the
spec
for details). This callback can be None.
@@ -447,8 +443,7 @@ There is no longer a `get_db_schema_files` callback provided, any changes to the
should now be made by the module using the module API class.
To port a module that has a `check_password` method:
-- Register `"m.login.password": ("password",)` as a supported login type
-- Register `"m.login.password": self.check_password` as an auth checker
+- Register `("m.login.password", ("password",): self.check_password` as an auth checker
- Set `self.module_api` to point to the `ModuleApi` object given in `__init__`
- Change the arguments of `check_password` to
`(username: str, login_type: str, login_dict: JsonDict)`
diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index 6e3c061c721a..7450989bccca 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -1919,19 +1919,18 @@ def run(*args, **kwargs):
# dictionary of login_type->check_auth_method mappings
check_auth = async_wrapper(getattr(provider, "check_auth", None))
if check_auth is not None:
- for login_type in supported_login_types:
- auth_checkers[login_type] = check_auth
+ for login_type, fields in supported_login_types.items():
+ # need tuple(fields) since fields can be any Iterable type (so may not be hashable)
+ auth_checkers[(login_type, tuple(fields))] = check_auth
# if it has a "check_password" method then it should handle all auth checks
# with login type of LoginType.PASSWORD
check_password = async_wrapper(getattr(provider, "check_password", None))
if check_password is not None:
- supported_login_types[LoginType.PASSWORD] = ("password",)
- auth_checkers[LoginType.PASSWORD] = check_password
+ # need to use a tuple here for ("password",) not a list since lists aren't hashable
+ auth_checkers[(LoginType.PASSWORD, ("password",))] = check_password
- api.register_password_auth_provider_callbacks(
- hooks, supported_login_types=supported_login_types, auth_checkers=auth_checkers
- )
+ api.register_password_auth_provider_callbacks(hooks, auth_checkers=auth_checkers)
CHECK_3PID_AUTH_CALLBACK = Callable[
@@ -1965,8 +1964,7 @@ def register_password_auth_provider_callbacks(
self,
check_3pid_auth: Optional[CHECK_3PID_AUTH_CALLBACK] = None,
on_logged_out: Optional[ON_LOGGED_OUT_CALLBACK] = None,
- supported_login_types: Optional[Dict[str, Iterable[str]]] = None,
- auth_checkers: Optional[Dict[str, CHECK_AUTH_CALLBACK]] = None,
+ auth_checkers: Optional[Dict[Tuple[str, Tuple], CHECK_AUTH_CALLBACK]] = None,
):
# Register check_3pid_auth callback
if check_3pid_auth is not None:
@@ -1976,17 +1974,23 @@ def register_password_auth_provider_callbacks(
if on_logged_out is not None:
self.on_logged_out_callbacks.append(on_logged_out)
- # register a new supported login_type
- if supported_login_types is not None:
+ if auth_checkers is not None:
+ supported_login_types = auth_checkers.keys()
+ # register a new supported login_type
# Iterate through all of the types being registered
- for login_type, fields in supported_login_types.items():
- # If a module is supporting a login_type it must also provide a
- # callback for authenticating using it
- if auth_checkers is None or auth_checkers.get(login_type) is None:
- raise RuntimeError(
- "A module tried to register support for login type: %s without providing"
- " an authentication method for it" % login_type
- )
+ for login_type, fields in supported_login_types:
+ # Note: fields may be empty here. This would allow a modules auth checker to
+ # be called with just 'login_type' and no password or other secrets
+
+ # Need to check that all the field names are strings or will get nasty error later
+ for f in fields:
+ if not isinstance(f, str):
+ raise RuntimeError(
+ "A module tried to register support for login type: %s with parameters %s"
+ " but all parameter names must be strings"
+ % (login_type, fields)
+ )
+
# 2 modules supporting the same login type must expect the same fields
# e.g. 1 can't expect "pass" if the other expects "password"
# so throw an exception if that happens
@@ -2003,20 +2007,9 @@ def register_password_auth_provider_callbacks(
% (login_type, fields, fields_currently_supported)
)
- # register an authentication callback for specific login types
- if auth_checkers is not None:
+ # register an authentication callback for specific login types
# Iterate through all of the callbacks being registered
- for login_type, callback in auth_checkers.items():
- # If a module is providing a callback for a login type, then it must also register it as
- # a supported login_type (in order to provide information on what fields it accepts)
- if (
- supported_login_types is None
- or supported_login_types.get(login_type) is None
- ):
- raise RuntimeError(
- "A module tried to register an authentication method for login type: %s"
- " without listing it as a supported login type" % login_type
- )
+ for (login_type, _), callback in auth_checkers.items():
# Add the new method to the list of auth_checker_callbacks
callback_list = self.auth_checker_callbacks.get(login_type, [])
callback_list.append(callback)
diff --git a/tests/handlers/test_password_providers.py b/tests/handlers/test_password_providers.py
index 91393b1c825f..4bb4ec819968 100644
--- a/tests/handlers/test_password_providers.py
+++ b/tests/handlers/test_password_providers.py
@@ -78,8 +78,7 @@ def parse_config(self):
def __init__(self, config, api: ModuleApi):
api.register_password_auth_provider_callbacks(
- supported_login_types={"test.login_type": ["test_field"]},
- auth_checkers={"test.login_type": self.check_auth},
+ auth_checkers={("test.login_type", ("test_field",)): self.check_auth},
)
def check_auth(self, *args):
@@ -114,13 +113,9 @@ def parse_config(self):
def __init__(self, config, api: ModuleApi):
api.register_password_auth_provider_callbacks(
- supported_login_types={
- "test.login_type": ["test_field"],
- "m.login.password": ["password"],
- },
auth_checkers={
- "test.login_type": self.check_auth,
- "m.login.password": self.check_auth,
+ ("test.login_type", ("test_field",)): self.check_auth,
+ ("m.login.password", ("password",)): self.check_auth,
},
)
pass
From f4a8484c39b57001628b0e8d1497e88758523cab Mon Sep 17 00:00:00 2001
From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com>
Date: Wed, 25 Aug 2021 17:15:47 +0100
Subject: [PATCH 13/23] Applied suggestions from code review
---
docs/modules.md | 55 +++++++++++----------
synapse/app/_base.py | 5 +-
synapse/handlers/auth.py | 58 +++++++++++++----------
synapse/module_api/__init__.py | 2 +
tests/handlers/test_password_providers.py | 7 +--
5 files changed, 69 insertions(+), 58 deletions(-)
diff --git a/docs/modules.md b/docs/modules.md
index d13c9f74e215..41b686f32e49 100644
--- a/docs/modules.md
+++ b/docs/modules.md
@@ -343,10 +343,10 @@ To register authentication checkers, the module should register:
A dict mapping from tuples of a login type identifier (such as `m.login.password`) and a
tuple of field names (such as `("password", "secret_thing")`) to authentication checking
-methods.
+callbacks.
-The login type and field names are the things that are provided by the user in their submission
-to the `/login` API.
+The login type and field names are the things that should be provided by the user in their
+request to the `/login` API.
For example, if a module implements authentication checkers for two different login types:
- `com.example.custom_login`
@@ -369,20 +369,23 @@ An authentication checking method should be of the following form:
```python
async def check_auth(
- username: str,
- login_type: str,
- login_dict: JsonDict
-) -> Optional[Tuple[str, Optional[Callable]]]
+ username: str,
+ login_type: str,
+ login_dict: JsonDict
+) -> Optional[
+ Tuple[
+ str,
+ Optional[Callable[["synapse.module_api.LoginResponse"], Awaitable[None]]]
+ ]
+]
```
It is passed the user field provided by the client (which might not be in `@username:server` form),
the login type, and a dictionary of login secrets passed by the client.
-On a failure it should return None. On a success it should return a (user_id, callback) tuple
-where user_id is a str containing the user's matrix id (i.e. in `@username:server` form) and the
-callback is a method to be called with the result from the `/login` call (see the
-spec
-for details). This callback can be None.
+If the authentication is successful, the module must return the user's Matrix ID (e.g. @alice:example.com) and optionally a callback to be called with the response to the `/login` request. If the module doesn't wish to return a callback, it must return None instead.
+
+If the authentication is unsuccessful, the module must return None.
Aditionally the module can register:
@@ -391,33 +394,33 @@ async def check_3pid_auth(
medium: str,
address: str,
password: str,
-) -> Optional[Tuple[str, Optional[Callable]]]
+) -> Optional[
+ Tuple[
+ str,
+ Optional[Callable[["synapse.module_api.LoginResponse"], Awaitable[None]]]
+ ]
+]
```
-This is called when a user attempts to register or log in with a third party identifier,
+Called when a user attempts to register or log in with a third party identifier,
such as email. It is passed the medium (eg. "email"), an address (eg. "jdoe@example.com")
and the user's password.
-On a failure it should return None. On a success it should return a (user_id, callback) tuple
-where user_id is a str containing the user's matrix id (i.e. in `@username:server` form) and the
-callback is a method to be called with the result from the `/login` call (see the
-spec
-for details). This callback can be None.
+If the authentication is successful, the module must return the user's Matrix ID (e.g. @alice:example.com) and optionally a callback to be called with the response to the `/login` request. If the module doesn't wish to return a callback, it must return None instead.
+
+If the authentication is unsuccessful, the module must return None.
```python
async def on_logged_out(
- user_id: str,
- device_id: str,
- access_token: str,
+ user_id: str,
+ device_id: Optional[str],
+ access_token: str
) -> None
```
-This is called when a user logs out. It is passed the qualified user ID, the ID of the
+Called during a logout request for a user. It is passed the qualified user ID, the ID of the
deactivated device (if any: access tokens are occasionally created without an associated
device ID), and the (now deactivated) access token.
-The callback must be asynchronous but the logout request will wait for the callback
-to complete.
-
### Porting an existing module that uses the old interface
diff --git a/synapse/app/_base.py b/synapse/app/_base.py
index 8d86716c0c7c..b010b03e0436 100644
--- a/synapse/app/_base.py
+++ b/synapse/app/_base.py
@@ -40,7 +40,7 @@
from synapse.events.presence_router import load_legacy_presence_router
from synapse.events.spamcheck import load_legacy_spam_checkers
from synapse.events.third_party_rules import load_legacy_third_party_event_rules
-from synapse.handlers.auth import load_legacy_password_auth_provider
+from synapse.handlers.auth import load_legacy_password_auth_providers
from synapse.logging.context import PreserveLoggingContext
from synapse.metrics.background_process_metrics import wrap_as_background_process
from synapse.metrics.jemalloc import setup_jemalloc_stats
@@ -373,8 +373,7 @@ def run_sighup(*args, **kwargs):
load_legacy_spam_checkers(hs)
load_legacy_third_party_event_rules(hs)
load_legacy_presence_router(hs)
- for module, config in hs.config.password_providers:
- load_legacy_password_auth_provider(module=module, config=config, api=module_api)
+ load_legacy_password_auth_providers(hs)
# If we've configured an expiry time for caches, start the background job now.
setup_expire_lru_cache_entries(hs)
diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index 7450989bccca..8893f05de3bb 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -1198,11 +1198,12 @@ async def _validate_userid_login(
known_login_type = False
- # get all of the login_types registered by modules
+ # Check if login_type matches a type registered by one of the modules
+ # We don't need to remove LoginType.PASSWORD from the list if password login is
+ # disabled, since if that were the case then by this point we know that the
+ # login_type is not LoginType.PASSWORD
supported_login_types = self.password_auth_provider.get_supported_login_types()
# check if the login type being used is supported by a module
- # note that if LoginType.PASSWORD has been disabled then login_type has already been
- # checked for that by this point
if login_type in supported_login_types:
# Make a note that this login type is supported by the server
known_login_type = True
@@ -1801,7 +1802,15 @@ def _generate_base_macaroon(self, user_id: str) -> pymacaroons.Macaroon:
return macaroon
-def load_legacy_password_auth_provider(module, config, api: ModuleApi):
+def load_legacy_password_auth_providers(hs: "HomeServer"):
+ module_api = hs.get_module_api()
+ for module, config in hs.config.password_providers:
+ load_single_legacy_password_auth_provider(
+ module=module, config=config, api=module_api
+ )
+
+
+def load_single_legacy_password_auth_provider(module, config, api: ModuleApi):
try:
provider = module(config=config, account_handler=api)
except Exception as e:
@@ -1934,12 +1943,17 @@ def run(*args, **kwargs):
CHECK_3PID_AUTH_CALLBACK = Callable[
- [str, str, str], Awaitable[Optional[Tuple[str, Optional[Callable]]]]
+ [str, str, str],
+ Awaitable[
+ Optional[Tuple[str, Optional[Callable[["LoginResponse"], Awaitable[None]]]]]
+ ],
]
ON_LOGGED_OUT_CALLBACK = Callable[[str, Optional[str], str], Awaitable]
CHECK_AUTH_CALLBACK = Callable[
[str, str, JsonDict],
- Awaitable[Optional[Tuple[str, Optional[Callable]]]],
+ Awaitable[
+ Optional[Tuple[str, Optional[Callable[["LoginResponse"], Awaitable[None]]]]]
+ ],
]
@@ -1975,10 +1989,9 @@ def register_password_auth_provider_callbacks(
self.on_logged_out_callbacks.append(on_logged_out)
if auth_checkers is not None:
- supported_login_types = auth_checkers.keys()
# register a new supported login_type
# Iterate through all of the types being registered
- for login_type, fields in supported_login_types:
+ for (login_type, fields), callback in auth_checkers.items():
# Note: fields may be empty here. This would allow a modules auth checker to
# be called with just 'login_type' and no password or other secrets
@@ -2007,10 +2020,7 @@ def register_password_auth_provider_callbacks(
% (login_type, fields, fields_currently_supported)
)
- # register an authentication callback for specific login types
- # Iterate through all of the callbacks being registered
- for (login_type, _), callback in auth_checkers.items():
- # Add the new method to the list of auth_checker_callbacks
+ # Add the new method to the list of auth_checker_callbacks for this login type
callback_list = self.auth_checker_callbacks.get(login_type, [])
callback_list.append(callback)
self.auth_checker_callbacks[login_type] = callback_list
@@ -2027,7 +2037,7 @@ def get_supported_login_types(self) -> Mapping[str, Iterable[str]]:
async def check_auth(
self, username: str, login_type: str, login_dict: JsonDict
- ) -> Optional[Tuple[str, Optional[Callable]]]:
+ ) -> Optional[Tuple[str, Optional[Callable[["LoginResponse"], Awaitable[None]]]]]:
"""Check if the user has presented valid login credentials
Args:
@@ -2055,10 +2065,10 @@ async def check_auth(
if result is not None:
# Check that the callback returned a Tuple[str, Optional[Callable]]
-
- # "type: ignore" is used on the isinstance checks because mypy thinks
+ # "type: ignore[unreachable]" is used after some isinstance checks because mypy thinks
# result is always the right type, but as it is 3rd party code it might not be
- if not isinstance(result, Tuple) or len(result) != 2: # type: ignore[arg-type]
+
+ if not isinstance(result, tuple) or len(result) != 2:
logger.warning(
"Wrong type returned by module API callback %s: %s, expected"
" Optional[Tuple[str, Optional[Callable]]]",
@@ -2082,8 +2092,8 @@ async def check_auth(
# the second should be Optional[Callable]
if callback_result is not None:
- if not isinstance(callback_result, Callable): # type: ignore[arg-type]
- logger.warning(
+ if not callable(callback_result):
+ logger.warning( # type: ignore[unreachable]
"Wrong type returned by module API callback %s: %s, expected"
" Optional[Tuple[str, Optional[Callable]]]",
callback,
@@ -2100,7 +2110,7 @@ async def check_auth(
async def check_3pid_auth(
self, medium: str, address: str, password: str
- ) -> Optional[Tuple[str, Optional[Callable]]]:
+ ) -> Optional[Tuple[str, Optional[Callable[["LoginResponse"], Awaitable[None]]]]]:
# 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
@@ -2116,10 +2126,10 @@ async def check_3pid_auth(
if result is not None:
# Check that the callback returned a Tuple[str, Optional[Callable]]
-
- # "type: ignore" is used on the isinstance checks because mypy thinks
+ # "type: ignore[unreachable]" is used after some isinstance checks because mypy thinks
# result is always the right type, but as it is 3rd party code it might not be
- if not isinstance(result, Tuple) or len(result) != 2: # type: ignore[arg-type]
+
+ if not isinstance(result, tuple) or len(result) != 2:
logger.warning(
"Wrong type returned by module API callback %s: %s, expected"
" Optional[Tuple[str, Optional[Callable]]]",
@@ -2143,8 +2153,8 @@ async def check_3pid_auth(
# the second should be Optional[Callable]
if callback_result is not None:
- if not isinstance(callback_result, Callable): # type: ignore[arg-type]
- logger.warning(
+ if not callable(callback_result):
+ logger.warning( # type: ignore[unreachable]
"Wrong type returned by module API callback %s: %s, expected"
" Optional[Tuple[str, Optional[Callable]]]",
callback,
diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py
index 4648faedde93..13eb328e2dc6 100644
--- a/synapse/module_api/__init__.py
+++ b/synapse/module_api/__init__.py
@@ -43,6 +43,7 @@
from synapse.http.site import SynapseRequest
from synapse.logging.context import make_deferred_yieldable, run_in_background
from synapse.metrics.background_process_metrics import run_as_background_process
+from synapse.rest.client.login import LoginResponse
from synapse.storage.database import DatabasePool, LoggingTransaction
from synapse.storage.databases.main.roommember import ProfileInfo
from synapse.storage.state import StateFilter
@@ -74,6 +75,7 @@
"DirectServeJsonResource",
"ModuleApi",
"PRESENCE_ALL_USERS",
+ "LoginResponse",
]
logger = logging.getLogger(__name__)
diff --git a/tests/handlers/test_password_providers.py b/tests/handlers/test_password_providers.py
index 4bb4ec819968..7dd4a5a36764 100644
--- a/tests/handlers/test_password_providers.py
+++ b/tests/handlers/test_password_providers.py
@@ -20,7 +20,7 @@
from twisted.internet import defer
import synapse
-from synapse.handlers.auth import load_legacy_password_auth_provider
+from synapse.handlers.auth import load_legacy_password_auth_providers
from synapse.module_api import ModuleApi
from synapse.rest.client import devices, login
from synapse.types import JsonDict
@@ -165,10 +165,7 @@ def make_homeserver(self, reactor, clock):
module_api = hs.get_module_api()
for module, config in hs.config.modules.loaded_modules:
module(config=config, api=module_api)
- for module, config in hs.config.password_providers:
- load_legacy_password_auth_provider(
- module=module, config=config, api=module_api
- )
+ load_legacy_password_auth_providers(hs)
return hs
From 3c9b6c25d57f949ad942f160d869b0574a974cdf Mon Sep 17 00:00:00 2001
From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com>
Date: Thu, 9 Sep 2021 16:10:14 +0100
Subject: [PATCH 14/23] Reformat password_aut_providers docs and fix some small
issues
---
.../password_auth_provider_callbacks.md | 86 +++++++++++++++----
docs/modules/porting_legacy_module.md | 3 +
synapse/module_api/__init__.py | 1 +
3 files changed, 71 insertions(+), 19 deletions(-)
diff --git a/docs/modules/password_auth_provider_callbacks.md b/docs/modules/password_auth_provider_callbacks.md
index c508dda67923..ccff7371cf14 100644
--- a/docs/modules/password_auth_provider_callbacks.md
+++ b/docs/modules/password_auth_provider_callbacks.md
@@ -1,10 +1,12 @@
-#### Password auth provider callbacks
+# Password auth provider callbacks
Password auth providers offer a way for server administrators to integrate
their Synapse installation with an external authentication system. The callbacks can be
registered by using the Module API's `register_password_auth_provider_callbacks` method.
-To register authentication checkers, the module should register:
+## Callbacks
+
+### `auth_checkers`
```
auth_checkers: Dict[Tuple[str,Tuple], CHECK_AUTH_CALLBACK]
@@ -40,7 +42,7 @@ An authentication checking method should be of the following form:
async def check_auth(
username: str,
login_type: str,
- login_dict: JsonDict
+ login_dict: "synapse.module_api.JsonDict",
) -> Optional[
Tuple[
str,
@@ -56,7 +58,7 @@ If the authentication is successful, the module must return the user's Matrix ID
If the authentication is unsuccessful, the module must return None.
-Aditionally the module can register:
+### `check_3pid_auth`
```python
async def check_3pid_auth(
@@ -79,6 +81,8 @@ If the authentication is successful, the module must return the user's Matrix ID
If the authentication is unsuccessful, the module must return None.
+### `on_logged_out`
+
```python
async def on_logged_out(
user_id: str,
@@ -90,18 +94,62 @@ Called during a logout request for a user. It is passed the qualified user ID, t
deactivated device (if any: access tokens are occasionally created without an associated
device ID), and the (now deactivated) access token.
-#### Porting password auth provider modules
-
-There is no longer a `get_db_schema_files` callback provided, any changes to the database
-should now be made by the module using the module API class.
-
-To port a module that has a `check_password` method:
-- Register `("m.login.password", ("password",): self.check_password` as an auth checker
-- Set `self.module_api` to point to the `ModuleApi` object given in `__init__`
-- Change the arguments of `check_password` to
- `(username: str, login_type: str, login_dict: JsonDict)`
-- Add the following lines to the top of the `check_password` method:
- - `user_id = self.module_api.get_qualified_user_id(username)`
- - `password = login_dict["password"]`
-- Alter `check_password` so that it returns `None` on an authentication failure, and `user_id`
- on a success
+## Example
+```python
+from typing import Awaitable, Callable, Optional, Tuple
+
+import synapse
+from synapse import module_api
+
+
+class MyAuthProvider:
+ def __init__(self, config: dict, api: module_api):
+
+ self.api = api
+
+ self.credentials = {
+ "bob": "building",
+ "@scoop:matrix.org": "digging",
+ }
+
+ api.register_password_auth_provider_callbacks(
+ auth_checkers={
+ ("my.login_type", ("my_field",)): self.check_my_login,
+ ("m.login.password", ("password",)): self.check_pass,
+ },
+ )
+
+ async def check_my_login(
+ self,
+ username: str,
+ login_type: str,
+ login_dict: "synapse.module_api.JsonDict",
+ ) -> Optional[
+ Tuple[
+ str,
+ Optional[Callable[["synapse.module_api.LoginResponse"], Awaitable[None]]],
+ ]
+ ]:
+ if login_type != "my.login_type":
+ return None
+
+ if self.credentials.get(username) == login_dict.get("my_field"):
+ return self.api.get_qualified_user_id(username)
+
+ async def check_pass(
+ self,
+ username: str,
+ login_type: str,
+ login_dict: "synapse.module_api.JsonDict",
+ ) -> Optional[
+ Tuple[
+ str,
+ Optional[Callable[["synapse.module_api.LoginResponse"], Awaitable[None]]],
+ ]
+ ]:
+ if login_type != "m.login.password":
+ return None
+
+ if self.credentials.get(username) == login_dict.get("password"):
+ return self.api.get_qualified_user_id(username)
+```
diff --git a/docs/modules/porting_legacy_module.md b/docs/modules/porting_legacy_module.md
index a7a251e53580..89084eb7b32b 100644
--- a/docs/modules/porting_legacy_module.md
+++ b/docs/modules/porting_legacy_module.md
@@ -12,6 +12,9 @@ should register this resource in its `__init__` method using the `register_web_r
method from the `ModuleApi` class (see [this section](writing_a_module.html#registering-a-web-resource) for
more info).
+There is no longer a `get_db_schema_files` callback provided for password auth provider modules. Any
+changes to the database should now be made by the module using the module API class.
+
The module's author should also update any example in the module's configuration to only
use the new `modules` section in Synapse's configuration file (see [this section](index.html#using-modules)
for more info).
diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py
index 13eb328e2dc6..be6eef5ad900 100644
--- a/synapse/module_api/__init__.py
+++ b/synapse/module_api/__init__.py
@@ -76,6 +76,7 @@
"ModuleApi",
"PRESENCE_ALL_USERS",
"LoginResponse",
+ "JsonDict"
]
logger = logging.getLogger(__name__)
From e889594966a20fafb693ebfacdef210b4671068a Mon Sep 17 00:00:00 2001
From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com>
Date: Thu, 9 Sep 2021 16:13:35 +0100
Subject: [PATCH 15/23] Run linters
---
synapse/module_api/__init__.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py
index be6eef5ad900..317191c64763 100644
--- a/synapse/module_api/__init__.py
+++ b/synapse/module_api/__init__.py
@@ -76,7 +76,7 @@
"ModuleApi",
"PRESENCE_ALL_USERS",
"LoginResponse",
- "JsonDict"
+ "JsonDict",
]
logger = logging.getLogger(__name__)
From 4ad1bd1347c2a00c970947912b8731c316468a88 Mon Sep 17 00:00:00 2001
From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com>
Date: Mon, 20 Sep 2021 14:55:16 +0100
Subject: [PATCH 16/23] Apply suggestion from code review
---
.../password_auth_provider_callbacks.md | 48 ++++++++-----------
synapse/handlers/auth.py | 6 +--
2 files changed, 23 insertions(+), 31 deletions(-)
diff --git a/docs/modules/password_auth_provider_callbacks.md b/docs/modules/password_auth_provider_callbacks.md
index ccff7371cf14..8978e137e1b3 100644
--- a/docs/modules/password_auth_provider_callbacks.md
+++ b/docs/modules/password_auth_provider_callbacks.md
@@ -9,34 +9,12 @@ registered by using the Module API's `register_password_auth_provider_callbacks`
### `auth_checkers`
```
- auth_checkers: Dict[Tuple[str,Tuple], CHECK_AUTH_CALLBACK]
+ auth_checkers: Dict[Tuple[str,Tuple], Callable]
```
A dict mapping from tuples of a login type identifier (such as `m.login.password`) and a
tuple of field names (such as `("password", "secret_thing")`) to authentication checking
-callbacks.
-
-The login type and field names are the things that should be provided by the user in their
-request to the `/login` API.
-
-For example, if a module implements authentication checkers for two different login types:
-- `com.example.custom_login`
- - Expects `secret1` and `secret2` fields to be sent to `/login`
- - Is checked by the method: `self.custom_login_check`
-- `m.login.password`
- - Expects a `password` field to be sent to `/login`
- - Is checked by the method: `self.password_checker`
-
-Then it should register the following dict:
-
-```python
-{
- ("com.example.custom_login", ("secret1", "secret2")): self.check_custom_login,
- ("m.login.password", ("password",)): self.password_checker,
-}
-```
-
-An authentication checking method should be of the following form:
+callbacks, which should be of the following form:
```python
async def check_auth(
@@ -51,10 +29,15 @@ async def check_auth(
]
```
+The login type and field names are the things that should be provided by the user in their
+request to the `/login` API.
+
It is passed the user field provided by the client (which might not be in `@username:server` form),
the login type, and a dictionary of login secrets passed by the client.
-If the authentication is successful, the module must return the user's Matrix ID (e.g. @alice:example.com) and optionally a callback to be called with the response to the `/login` request. If the module doesn't wish to return a callback, it must return None instead.
+If the authentication is successful, the module must return the user's Matrix ID (e.g.
+`@alice:example.com`) and optionally a callback to be called with the response to the `/login` request.
+If the module doesn't wish to return a callback, it must return None instead.
If the authentication is unsuccessful, the module must return None.
@@ -74,10 +57,12 @@ async def check_3pid_auth(
```
Called when a user attempts to register or log in with a third party identifier,
-such as email. It is passed the medium (eg. "email"), an address (eg. "jdoe@example.com")
+such as email. It is passed the medium (eg. `"email"`), an address (eg. `"jdoe@example.com"`)
and the user's password.
-If the authentication is successful, the module must return the user's Matrix ID (e.g. @alice:example.com) and optionally a callback to be called with the response to the `/login` request. If the module doesn't wish to return a callback, it must return None instead.
+If the authentication is successful, the module must return the user's Matrix ID (e.g.
+`"@alice:example.com"`) and optionally a callback to be called with the response to the `/login` request.
+If the module doesn't wish to return a callback, it must return None instead.
If the authentication is unsuccessful, the module must return None.
@@ -95,6 +80,15 @@ deactivated device (if any: access tokens are occasionally created without an as
device ID), and the (now deactivated) access token.
## Example
+
+The following module implements authentication checkers for two different login types:
+- `my.login.type`
+ - Expects a `my_field` field to be sent to `/login`
+ - Is checked by the method: `self.check_my_login`
+- `m.login.password`
+ - Expects a `password` field to be sent to `/login`
+ - Is checked by the method: `self.check_pass`
+
```python
from typing import Awaitable, Callable, Optional, Tuple
diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index 888ca1ad10a8..966ec89ba143 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -1999,7 +1999,7 @@ def register_password_auth_provider_callbacks(
# Note: fields may be empty here. This would allow a modules auth checker to
# be called with just 'login_type' and no password or other secrets
- # Need to check that all the field names are strings or will get nasty error later
+ # Need to check that all the field names are strings or may get nasty errors later
for f in fields:
if not isinstance(f, str):
raise RuntimeError(
@@ -2025,9 +2025,7 @@ def register_password_auth_provider_callbacks(
)
# Add the new method to the list of auth_checker_callbacks for this login type
- callback_list = self.auth_checker_callbacks.get(login_type, [])
- callback_list.append(callback)
- self.auth_checker_callbacks[login_type] = callback_list
+ self.auth_checker_callbacks.setdefault(login_type, []).append(callback)
def get_supported_login_types(self) -> Mapping[str, Iterable[str]]:
"""Get the login types supported by this password provider
From 55eede59f7d35c2b638f496be918956c7661c4ef Mon Sep 17 00:00:00 2001
From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com>
Date: Mon, 20 Sep 2021 15:03:32 +0100
Subject: [PATCH 17/23] Run linters
---
synapse/handlers/auth.py | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index fd7a819f886a..5a05d8140b08 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -1815,7 +1815,9 @@ def load_legacy_password_auth_providers(hs: "HomeServer"):
)
-def load_single_legacy_password_auth_provider(module: Type, config: JsonDict, api: ModuleApi):
+def load_single_legacy_password_auth_provider(
+ module: Type, config: JsonDict, api: ModuleApi
+):
try:
provider = module(config=config, account_handler=api)
except Exception as e:
From 01c65d79a6b97a438f4a61c4e535e29441c9b6d6 Mon Sep 17 00:00:00 2001
From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com>
Date: Mon, 20 Sep 2021 15:07:06 +0100
Subject: [PATCH 18/23] Add link to spec
---
docs/modules/password_auth_provider_callbacks.md | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/docs/modules/password_auth_provider_callbacks.md b/docs/modules/password_auth_provider_callbacks.md
index 8978e137e1b3..8a0b758e7a4f 100644
--- a/docs/modules/password_auth_provider_callbacks.md
+++ b/docs/modules/password_auth_provider_callbacks.md
@@ -30,7 +30,8 @@ async def check_auth(
```
The login type and field names are the things that should be provided by the user in their
-request to the `/login` API.
+request to the `/login` API. [The spec](https://matrix.org/docs/spec/client_server/latest#user-interactive-authentication-api)
+defines some types, however user defined ones are also allowed.
It is passed the user field provided by the client (which might not be in `@username:server` form),
the login type, and a dictionary of login secrets passed by the client.
From 6fe825f30e7f599f356d5b79ef6eb0e1bdbfbb57 Mon Sep 17 00:00:00 2001
From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com>
Date: Mon, 20 Sep 2021 15:12:36 +0100
Subject: [PATCH 19/23] Tweaks to documentation
---
docs/modules/password_auth_provider_callbacks.md | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/docs/modules/password_auth_provider_callbacks.md b/docs/modules/password_auth_provider_callbacks.md
index 8a0b758e7a4f..df0938b51411 100644
--- a/docs/modules/password_auth_provider_callbacks.md
+++ b/docs/modules/password_auth_provider_callbacks.md
@@ -30,7 +30,7 @@ async def check_auth(
```
The login type and field names are the things that should be provided by the user in their
-request to the `/login` API. [The spec](https://matrix.org/docs/spec/client_server/latest#user-interactive-authentication-api)
+request to the `/login` API. [The spec](https://matrix.org/docs/spec/client_server/latest#authentication-types)
defines some types, however user defined ones are also allowed.
It is passed the user field provided by the client (which might not be in `@username:server` form),
@@ -82,14 +82,15 @@ device ID), and the (now deactivated) access token.
## Example
-The following module implements authentication checkers for two different login types:
+The example module below implements authentication checkers for two different login types:
- `my.login.type`
- Expects a `my_field` field to be sent to `/login`
- Is checked by the method: `self.check_my_login`
-- `m.login.password`
+- `m.login.password` (defined in [the spec](https://matrix.org/docs/spec/client_server/latest#password-based))
- Expects a `password` field to be sent to `/login`
- Is checked by the method: `self.check_pass`
+
```python
from typing import Awaitable, Callable, Optional, Tuple
From 86e9607d14a2b9a3add3fe50a12e7bd5ac765890 Mon Sep 17 00:00:00 2001
From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com>
Date: Mon, 20 Sep 2021 15:18:42 +0100
Subject: [PATCH 20/23] Add typehints to auth.py
---
synapse/handlers/auth.py | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index 5a05d8140b08..bed571a8cf1a 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -1807,7 +1807,7 @@ def _generate_base_macaroon(self, user_id: str) -> pymacaroons.Macaroon:
return macaroon
-def load_legacy_password_auth_providers(hs: "HomeServer"):
+def load_legacy_password_auth_providers(hs: "HomeServer") -> None:
module_api = hs.get_module_api()
for module, config in hs.config.password_providers:
load_single_legacy_password_auth_provider(
@@ -1817,7 +1817,7 @@ def load_legacy_password_auth_providers(hs: "HomeServer"):
def load_single_legacy_password_auth_provider(
module: Type, config: JsonDict, api: ModuleApi
-):
+) -> None:
try:
provider = module(config=config, account_handler=api)
except Exception as e:
@@ -1906,7 +1906,7 @@ async def wrapped_check_3pid_auth(
return wrapped_check_3pid_auth
- def run(*args, **kwargs):
+ def run(*args: Tuple, **kwargs: Dict) -> Awaitable:
# mypy doesn't do well across function boundaries so we need to tell it
# f is definitely not None.
assert f is not None
@@ -1970,7 +1970,7 @@ class PasswordAuthProvider:
It allows modules to provide alternative methods for authentication
"""
- def __init__(self):
+ def __init__(self) -> None:
# lists of callbacks
self.check_3pid_auth_callbacks: List[CHECK_3PID_AUTH_CALLBACK] = []
self.on_logged_out_callbacks: List[ON_LOGGED_OUT_CALLBACK] = []
@@ -1986,7 +1986,7 @@ def register_password_auth_provider_callbacks(
check_3pid_auth: Optional[CHECK_3PID_AUTH_CALLBACK] = None,
on_logged_out: Optional[ON_LOGGED_OUT_CALLBACK] = None,
auth_checkers: Optional[Dict[Tuple[str, Tuple], CHECK_AUTH_CALLBACK]] = None,
- ):
+ ) -> None:
# Register check_3pid_auth callback
if check_3pid_auth is not None:
self.check_3pid_auth_callbacks.append(check_3pid_auth)
From 5e37f11d314b48695f28200660a475f63a1bc1fe Mon Sep 17 00:00:00 2001
From: Brendan Abolivier
Date: Wed, 13 Oct 2021 12:47:31 +0200
Subject: [PATCH 21/23] Apply suggestions from code review
---
docs/modules/password_auth_provider_callbacks.md | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/docs/modules/password_auth_provider_callbacks.md b/docs/modules/password_auth_provider_callbacks.md
index df0938b51411..fdf2e2640e86 100644
--- a/docs/modules/password_auth_provider_callbacks.md
+++ b/docs/modules/password_auth_provider_callbacks.md
@@ -29,18 +29,18 @@ async def check_auth(
]
```
-The login type and field names are the things that should be provided by the user in their
+The login type and field names should be provided by the user in the
request to the `/login` API. [The spec](https://matrix.org/docs/spec/client_server/latest#authentication-types)
defines some types, however user defined ones are also allowed.
-It is passed the user field provided by the client (which might not be in `@username:server` form),
+The callback is passed the `user` field provided by the client (which might not be in `@username:server` form),
the login type, and a dictionary of login secrets passed by the client.
If the authentication is successful, the module must return the user's Matrix ID (e.g.
`@alice:example.com`) and optionally a callback to be called with the response to the `/login` request.
-If the module doesn't wish to return a callback, it must return None instead.
+If the module doesn't wish to return a callback, it must return `None` instead.
-If the authentication is unsuccessful, the module must return None.
+If the authentication is unsuccessful, the module must return `None`.
### `check_3pid_auth`
From 1cd658a04831f93cb07703183c6004adc72a7ac7 Mon Sep 17 00:00:00 2001
From: Brendan Abolivier
Date: Wed, 13 Oct 2021 11:50:30 +0100
Subject: [PATCH 22/23] Incorporate review comment + cleanup
The documentation mentions the username parameter of auth checkers as
`user` even though the code doesn't, in order to make it clear this is
the same value clients include in login requests as `user`.
---
docs/modules/password_auth_provider_callbacks.md | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/docs/modules/password_auth_provider_callbacks.md b/docs/modules/password_auth_provider_callbacks.md
index fdf2e2640e86..34d1af3c4e9d 100644
--- a/docs/modules/password_auth_provider_callbacks.md
+++ b/docs/modules/password_auth_provider_callbacks.md
@@ -18,7 +18,7 @@ callbacks, which should be of the following form:
```python
async def check_auth(
- username: str,
+ user: str,
login_type: str,
login_dict: "synapse.module_api.JsonDict",
) -> Optional[
@@ -30,15 +30,17 @@ async def check_auth(
```
The login type and field names should be provided by the user in the
-request to the `/login` API. [The spec](https://matrix.org/docs/spec/client_server/latest#authentication-types)
+request to the `/login` API. [The Matrix specification](https://matrix.org/docs/spec/client_server/latest#authentication-types)
defines some types, however user defined ones are also allowed.
-The callback is passed the `user` field provided by the client (which might not be in `@username:server` form),
-the login type, and a dictionary of login secrets passed by the client.
+The callback is passed the `user` field provided by the client (which might not be in
+`@username:server` form), the login type, and a dictionary of login secrets passed by
+the client.
If the authentication is successful, the module must return the user's Matrix ID (e.g.
-`@alice:example.com`) and optionally a callback to be called with the response to the `/login` request.
-If the module doesn't wish to return a callback, it must return `None` instead.
+`@alice:example.com`) and optionally a callback to be called with the response to the
+`/login` request. If the module doesn't wish to return a callback, it must return `None`
+instead.
If the authentication is unsuccessful, the module must return `None`.
From 73ff756761c90250b8c591c4a232988d0a174fb1 Mon Sep 17 00:00:00 2001
From: Brendan Abolivier
Date: Wed, 13 Oct 2021 11:56:16 +0100
Subject: [PATCH 23/23] Standardise example values
---
docs/modules/password_auth_provider_callbacks.md | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/docs/modules/password_auth_provider_callbacks.md b/docs/modules/password_auth_provider_callbacks.md
index 34d1af3c4e9d..36417dd39e20 100644
--- a/docs/modules/password_auth_provider_callbacks.md
+++ b/docs/modules/password_auth_provider_callbacks.md
@@ -60,11 +60,11 @@ async def check_3pid_auth(
```
Called when a user attempts to register or log in with a third party identifier,
-such as email. It is passed the medium (eg. `"email"`), an address (eg. `"jdoe@example.com"`)
+such as email. It is passed the medium (eg. `email`), an address (eg. `jdoe@example.com`)
and the user's password.
If the authentication is successful, the module must return the user's Matrix ID (e.g.
-`"@alice:example.com"`) and optionally a callback to be called with the response to the `/login` request.
+`@alice:example.com`) and optionally a callback to be called with the response to the `/login` request.
If the module doesn't wish to return a callback, it must return None instead.
If the authentication is unsuccessful, the module must return None.