mirror of
https://github.com/element-hq/synapse.git
synced 2024-12-18 08:54:54 +03:00
Add an option allowing users to use their password to reauthenticate even though password authentication is disabled. (#12883)
This commit is contained in:
parent
317248d42c
commit
7b88f5a107
6 changed files with 83 additions and 12 deletions
1
changelog.d/12883.feature
Normal file
1
changelog.d/12883.feature
Normal file
|
@ -0,0 +1 @@
|
||||||
|
Add an option allowing users to use their password to reauthenticate for privileged actions even though password login is disabled.
|
|
@ -2216,7 +2216,9 @@ sso:
|
||||||
|
|
||||||
|
|
||||||
password_config:
|
password_config:
|
||||||
# Uncomment to disable password login
|
# Uncomment to disable password login.
|
||||||
|
# Set to `only_for_reauth` to permit reauthentication for users that
|
||||||
|
# have passwords and are already logged in.
|
||||||
#
|
#
|
||||||
#enabled: false
|
#enabled: false
|
||||||
|
|
||||||
|
|
|
@ -2930,6 +2930,9 @@ Use this setting to enable password-based logins.
|
||||||
|
|
||||||
This setting has the following sub-options:
|
This setting has the following sub-options:
|
||||||
* `enabled`: Defaults to true.
|
* `enabled`: Defaults to true.
|
||||||
|
Set to false to disable password authentication.
|
||||||
|
Set to `only_for_reauth` to allow users with existing passwords to use them
|
||||||
|
to log in and reauthenticate, whilst preventing new users from setting passwords.
|
||||||
* `localdb_enabled`: Set to false to disable authentication against the local password
|
* `localdb_enabled`: Set to false to disable authentication against the local password
|
||||||
database. This is ignored if `enabled` is false, and is only useful
|
database. This is ignored if `enabled` is false, and is only useful
|
||||||
if you have other `password_providers`. Defaults to true.
|
if you have other `password_providers`. Defaults to true.
|
||||||
|
|
|
@ -29,7 +29,18 @@ class AuthConfig(Config):
|
||||||
if password_config is None:
|
if password_config is None:
|
||||||
password_config = {}
|
password_config = {}
|
||||||
|
|
||||||
self.password_enabled = password_config.get("enabled", True)
|
passwords_enabled = password_config.get("enabled", True)
|
||||||
|
# 'only_for_reauth' allows users who have previously set a password to use it,
|
||||||
|
# even though passwords would otherwise be disabled.
|
||||||
|
passwords_for_reauth_only = passwords_enabled == "only_for_reauth"
|
||||||
|
|
||||||
|
self.password_enabled_for_login = (
|
||||||
|
passwords_enabled and not passwords_for_reauth_only
|
||||||
|
)
|
||||||
|
self.password_enabled_for_reauth = (
|
||||||
|
passwords_for_reauth_only or passwords_enabled
|
||||||
|
)
|
||||||
|
|
||||||
self.password_localdb_enabled = password_config.get("localdb_enabled", True)
|
self.password_localdb_enabled = password_config.get("localdb_enabled", True)
|
||||||
self.password_pepper = password_config.get("pepper", "")
|
self.password_pepper = password_config.get("pepper", "")
|
||||||
|
|
||||||
|
@ -46,7 +57,9 @@ class AuthConfig(Config):
|
||||||
def generate_config_section(self, **kwargs: Any) -> str:
|
def generate_config_section(self, **kwargs: Any) -> str:
|
||||||
return """\
|
return """\
|
||||||
password_config:
|
password_config:
|
||||||
# Uncomment to disable password login
|
# Uncomment to disable password login.
|
||||||
|
# Set to `only_for_reauth` to permit reauthentication for users that
|
||||||
|
# have passwords and are already logged in.
|
||||||
#
|
#
|
||||||
#enabled: false
|
#enabled: false
|
||||||
|
|
||||||
|
|
|
@ -210,7 +210,8 @@ class AuthHandler:
|
||||||
|
|
||||||
self.hs = hs # FIXME better possibility to access registrationHandler later?
|
self.hs = hs # FIXME better possibility to access registrationHandler later?
|
||||||
self.macaroon_gen = hs.get_macaroon_generator()
|
self.macaroon_gen = hs.get_macaroon_generator()
|
||||||
self._password_enabled = hs.config.auth.password_enabled
|
self._password_enabled_for_login = hs.config.auth.password_enabled_for_login
|
||||||
|
self._password_enabled_for_reauth = hs.config.auth.password_enabled_for_reauth
|
||||||
self._password_localdb_enabled = hs.config.auth.password_localdb_enabled
|
self._password_localdb_enabled = hs.config.auth.password_localdb_enabled
|
||||||
self._third_party_rules = hs.get_third_party_event_rules()
|
self._third_party_rules = hs.get_third_party_event_rules()
|
||||||
|
|
||||||
|
@ -387,13 +388,13 @@ class AuthHandler:
|
||||||
return params, session_id
|
return params, session_id
|
||||||
|
|
||||||
async def _get_available_ui_auth_types(self, user: UserID) -> Iterable[str]:
|
async def _get_available_ui_auth_types(self, user: UserID) -> Iterable[str]:
|
||||||
"""Get a list of the authentication types this user can use"""
|
"""Get a list of the user-interactive authentication types this user can use."""
|
||||||
|
|
||||||
ui_auth_types = set()
|
ui_auth_types = set()
|
||||||
|
|
||||||
# if the HS supports password auth, and the user has a non-null password, we
|
# if the HS supports password auth, and the user has a non-null password, we
|
||||||
# support password auth
|
# support password auth
|
||||||
if self._password_localdb_enabled and self._password_enabled:
|
if self._password_localdb_enabled and self._password_enabled_for_reauth:
|
||||||
lookupres = await self._find_user_id_and_pwd_hash(user.to_string())
|
lookupres = await self._find_user_id_and_pwd_hash(user.to_string())
|
||||||
if lookupres:
|
if lookupres:
|
||||||
_, password_hash = lookupres
|
_, password_hash = lookupres
|
||||||
|
@ -402,7 +403,7 @@ class AuthHandler:
|
||||||
|
|
||||||
# also allow auth from password providers
|
# also allow auth from password providers
|
||||||
for t in self.password_auth_provider.get_supported_login_types().keys():
|
for t in self.password_auth_provider.get_supported_login_types().keys():
|
||||||
if t == LoginType.PASSWORD and not self._password_enabled:
|
if t == LoginType.PASSWORD and not self._password_enabled_for_reauth:
|
||||||
continue
|
continue
|
||||||
ui_auth_types.add(t)
|
ui_auth_types.add(t)
|
||||||
|
|
||||||
|
@ -710,7 +711,7 @@ class AuthHandler:
|
||||||
return res
|
return res
|
||||||
|
|
||||||
# fall back to the v1 login flow
|
# fall back to the v1 login flow
|
||||||
canonical_id, _ = await self.validate_login(authdict)
|
canonical_id, _ = await self.validate_login(authdict, is_reauth=True)
|
||||||
return canonical_id
|
return canonical_id
|
||||||
|
|
||||||
def _get_params_recaptcha(self) -> dict:
|
def _get_params_recaptcha(self) -> dict:
|
||||||
|
@ -1064,7 +1065,7 @@ class AuthHandler:
|
||||||
Returns:
|
Returns:
|
||||||
Whether users on this server are allowed to change or set a password
|
Whether users on this server are allowed to change or set a password
|
||||||
"""
|
"""
|
||||||
return self._password_enabled and self._password_localdb_enabled
|
return self._password_enabled_for_login and self._password_localdb_enabled
|
||||||
|
|
||||||
def get_supported_login_types(self) -> Iterable[str]:
|
def get_supported_login_types(self) -> Iterable[str]:
|
||||||
"""Get a the login types supported for the /login API
|
"""Get a the login types supported for the /login API
|
||||||
|
@ -1089,9 +1090,9 @@ class AuthHandler:
|
||||||
# that comes first, where it's present.
|
# that comes first, where it's present.
|
||||||
if LoginType.PASSWORD in types:
|
if LoginType.PASSWORD in types:
|
||||||
types.remove(LoginType.PASSWORD)
|
types.remove(LoginType.PASSWORD)
|
||||||
if self._password_enabled:
|
if self._password_enabled_for_login:
|
||||||
types.insert(0, LoginType.PASSWORD)
|
types.insert(0, LoginType.PASSWORD)
|
||||||
elif self._password_localdb_enabled and self._password_enabled:
|
elif self._password_localdb_enabled and self._password_enabled_for_login:
|
||||||
types.insert(0, LoginType.PASSWORD)
|
types.insert(0, LoginType.PASSWORD)
|
||||||
|
|
||||||
return types
|
return types
|
||||||
|
@ -1100,6 +1101,7 @@ class AuthHandler:
|
||||||
self,
|
self,
|
||||||
login_submission: Dict[str, Any],
|
login_submission: Dict[str, Any],
|
||||||
ratelimit: bool = False,
|
ratelimit: bool = False,
|
||||||
|
is_reauth: bool = False,
|
||||||
) -> Tuple[str, Optional[Callable[["LoginResponse"], Awaitable[None]]]]:
|
) -> Tuple[str, Optional[Callable[["LoginResponse"], Awaitable[None]]]]:
|
||||||
"""Authenticates the user for the /login API
|
"""Authenticates the user for the /login API
|
||||||
|
|
||||||
|
@ -1110,6 +1112,9 @@ class AuthHandler:
|
||||||
login_submission: the whole of the login submission
|
login_submission: the whole of the login submission
|
||||||
(including 'type' and other relevant fields)
|
(including 'type' and other relevant fields)
|
||||||
ratelimit: whether to apply the failed_login_attempt ratelimiter
|
ratelimit: whether to apply the failed_login_attempt ratelimiter
|
||||||
|
is_reauth: whether this is part of a User-Interactive Authorisation
|
||||||
|
flow to reauthenticate for a privileged action (rather than a
|
||||||
|
new login)
|
||||||
Returns:
|
Returns:
|
||||||
A tuple of the canonical user id, and optional callback
|
A tuple of the canonical user id, and optional callback
|
||||||
to be called once the access token and device id are issued
|
to be called once the access token and device id are issued
|
||||||
|
@ -1132,8 +1137,14 @@ class AuthHandler:
|
||||||
# special case to check for "password" for the check_password interface
|
# special case to check for "password" for the check_password interface
|
||||||
# for the auth providers
|
# for the auth providers
|
||||||
password = login_submission.get("password")
|
password = login_submission.get("password")
|
||||||
|
|
||||||
if login_type == LoginType.PASSWORD:
|
if login_type == LoginType.PASSWORD:
|
||||||
if not self._password_enabled:
|
if is_reauth:
|
||||||
|
passwords_allowed_here = self._password_enabled_for_reauth
|
||||||
|
else:
|
||||||
|
passwords_allowed_here = self._password_enabled_for_login
|
||||||
|
|
||||||
|
if not passwords_allowed_here:
|
||||||
raise SynapseError(400, "Password login has been disabled.")
|
raise SynapseError(400, "Password login has been disabled.")
|
||||||
if not isinstance(password, str):
|
if not isinstance(password, str):
|
||||||
raise SynapseError(400, "Bad parameter: password", Codes.INVALID_PARAM)
|
raise SynapseError(400, "Bad parameter: password", Codes.INVALID_PARAM)
|
||||||
|
|
|
@ -195,8 +195,17 @@ class UIAuthTests(unittest.HomeserverTestCase):
|
||||||
self.user_pass = "pass"
|
self.user_pass = "pass"
|
||||||
self.user = self.register_user("test", self.user_pass)
|
self.user = self.register_user("test", self.user_pass)
|
||||||
self.device_id = "dev1"
|
self.device_id = "dev1"
|
||||||
|
|
||||||
|
# Force-enable password login for just long enough to log in.
|
||||||
|
auth_handler = self.hs.get_auth_handler()
|
||||||
|
allow_auth_for_login = auth_handler._password_enabled_for_login
|
||||||
|
auth_handler._password_enabled_for_login = True
|
||||||
|
|
||||||
self.user_tok = self.login("test", self.user_pass, self.device_id)
|
self.user_tok = self.login("test", self.user_pass, self.device_id)
|
||||||
|
|
||||||
|
# Restore password login to however it was.
|
||||||
|
auth_handler._password_enabled_for_login = allow_auth_for_login
|
||||||
|
|
||||||
def delete_device(
|
def delete_device(
|
||||||
self,
|
self,
|
||||||
access_token: str,
|
access_token: str,
|
||||||
|
@ -263,6 +272,38 @@ class UIAuthTests(unittest.HomeserverTestCase):
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
|
|
||||||
|
@override_config({"password_config": {"enabled": "only_for_reauth"}})
|
||||||
|
def test_ui_auth_with_passwords_for_reauth_only(self) -> None:
|
||||||
|
"""
|
||||||
|
Test user interactive authentication outside of registration.
|
||||||
|
"""
|
||||||
|
|
||||||
|
# Attempt to delete this device.
|
||||||
|
# Returns a 401 as per the spec
|
||||||
|
channel = self.delete_device(
|
||||||
|
self.user_tok, self.device_id, HTTPStatus.UNAUTHORIZED
|
||||||
|
)
|
||||||
|
|
||||||
|
# Grab the session
|
||||||
|
session = channel.json_body["session"]
|
||||||
|
# Ensure that flows are what is expected.
|
||||||
|
self.assertIn({"stages": ["m.login.password"]}, channel.json_body["flows"])
|
||||||
|
|
||||||
|
# Make another request providing the UI auth flow.
|
||||||
|
self.delete_device(
|
||||||
|
self.user_tok,
|
||||||
|
self.device_id,
|
||||||
|
HTTPStatus.OK,
|
||||||
|
{
|
||||||
|
"auth": {
|
||||||
|
"type": "m.login.password",
|
||||||
|
"identifier": {"type": "m.id.user", "user": self.user},
|
||||||
|
"password": self.user_pass,
|
||||||
|
"session": session,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
def test_grandfathered_identifier(self) -> None:
|
def test_grandfathered_identifier(self) -> None:
|
||||||
"""Check behaviour without "identifier" dict
|
"""Check behaviour without "identifier" dict
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue