Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

Commit 7b88f5a

Browse files
authored
Add an option allowing users to use their password to reauthenticate even though password authentication is disabled. (#12883)
1 parent 317248d commit 7b88f5a

6 files changed

Lines changed: 83 additions & 12 deletions

File tree

changelog.d/12883.feature

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Add an option allowing users to use their password to reauthenticate for privileged actions even though password login is disabled.

docs/sample_config.yaml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2216,7 +2216,9 @@ sso:
22162216

22172217

22182218
password_config:
2219-
# Uncomment to disable password login
2219+
# Uncomment to disable password login.
2220+
# Set to `only_for_reauth` to permit reauthentication for users that
2221+
# have passwords and are already logged in.
22202222
#
22212223
#enabled: false
22222224

docs/usage/configuration/config_documentation.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2930,6 +2930,9 @@ Use this setting to enable password-based logins.
29302930

29312931
This setting has the following sub-options:
29322932
* `enabled`: Defaults to true.
2933+
Set to false to disable password authentication.
2934+
Set to `only_for_reauth` to allow users with existing passwords to use them
2935+
to log in and reauthenticate, whilst preventing new users from setting passwords.
29332936
* `localdb_enabled`: Set to false to disable authentication against the local password
29342937
database. This is ignored if `enabled` is false, and is only useful
29352938
if you have other `password_providers`. Defaults to true.

synapse/config/auth.py

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,18 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
2929
if password_config is None:
3030
password_config = {}
3131

32-
self.password_enabled = password_config.get("enabled", True)
32+
passwords_enabled = password_config.get("enabled", True)
33+
# 'only_for_reauth' allows users who have previously set a password to use it,
34+
# even though passwords would otherwise be disabled.
35+
passwords_for_reauth_only = passwords_enabled == "only_for_reauth"
36+
37+
self.password_enabled_for_login = (
38+
passwords_enabled and not passwords_for_reauth_only
39+
)
40+
self.password_enabled_for_reauth = (
41+
passwords_for_reauth_only or passwords_enabled
42+
)
43+
3344
self.password_localdb_enabled = password_config.get("localdb_enabled", True)
3445
self.password_pepper = password_config.get("pepper", "")
3546

@@ -46,7 +57,9 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
4657
def generate_config_section(self, **kwargs: Any) -> str:
4758
return """\
4859
password_config:
49-
# Uncomment to disable password login
60+
# Uncomment to disable password login.
61+
# Set to `only_for_reauth` to permit reauthentication for users that
62+
# have passwords and are already logged in.
5063
#
5164
#enabled: false
5265

synapse/handlers/auth.py

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,8 @@ def __init__(self, hs: "HomeServer"):
210210

211211
self.hs = hs # FIXME better possibility to access registrationHandler later?
212212
self.macaroon_gen = hs.get_macaroon_generator()
213-
self._password_enabled = hs.config.auth.password_enabled
213+
self._password_enabled_for_login = hs.config.auth.password_enabled_for_login
214+
self._password_enabled_for_reauth = hs.config.auth.password_enabled_for_reauth
214215
self._password_localdb_enabled = hs.config.auth.password_localdb_enabled
215216
self._third_party_rules = hs.get_third_party_event_rules()
216217

@@ -387,13 +388,13 @@ def get_new_session_data() -> JsonDict:
387388
return params, session_id
388389

389390
async def _get_available_ui_auth_types(self, user: UserID) -> Iterable[str]:
390-
"""Get a list of the authentication types this user can use"""
391+
"""Get a list of the user-interactive authentication types this user can use."""
391392

392393
ui_auth_types = set()
393394

394395
# if the HS supports password auth, and the user has a non-null password, we
395396
# support password auth
396-
if self._password_localdb_enabled and self._password_enabled:
397+
if self._password_localdb_enabled and self._password_enabled_for_reauth:
397398
lookupres = await self._find_user_id_and_pwd_hash(user.to_string())
398399
if lookupres:
399400
_, password_hash = lookupres
@@ -402,7 +403,7 @@ async def _get_available_ui_auth_types(self, user: UserID) -> Iterable[str]:
402403

403404
# also allow auth from password providers
404405
for t in self.password_auth_provider.get_supported_login_types().keys():
405-
if t == LoginType.PASSWORD and not self._password_enabled:
406+
if t == LoginType.PASSWORD and not self._password_enabled_for_reauth:
406407
continue
407408
ui_auth_types.add(t)
408409

@@ -710,7 +711,7 @@ async def _check_auth_dict(
710711
return res
711712

712713
# fall back to the v1 login flow
713-
canonical_id, _ = await self.validate_login(authdict)
714+
canonical_id, _ = await self.validate_login(authdict, is_reauth=True)
714715
return canonical_id
715716

716717
def _get_params_recaptcha(self) -> dict:
@@ -1064,7 +1065,7 @@ def can_change_password(self) -> bool:
10641065
Returns:
10651066
Whether users on this server are allowed to change or set a password
10661067
"""
1067-
return self._password_enabled and self._password_localdb_enabled
1068+
return self._password_enabled_for_login and self._password_localdb_enabled
10681069

10691070
def get_supported_login_types(self) -> Iterable[str]:
10701071
"""Get a the login types supported for the /login API
@@ -1089,9 +1090,9 @@ def get_supported_login_types(self) -> Iterable[str]:
10891090
# that comes first, where it's present.
10901091
if LoginType.PASSWORD in types:
10911092
types.remove(LoginType.PASSWORD)
1092-
if self._password_enabled:
1093+
if self._password_enabled_for_login:
10931094
types.insert(0, LoginType.PASSWORD)
1094-
elif self._password_localdb_enabled and self._password_enabled:
1095+
elif self._password_localdb_enabled and self._password_enabled_for_login:
10951096
types.insert(0, LoginType.PASSWORD)
10961097

10971098
return types
@@ -1100,6 +1101,7 @@ async def validate_login(
11001101
self,
11011102
login_submission: Dict[str, Any],
11021103
ratelimit: bool = False,
1104+
is_reauth: bool = False,
11031105
) -> Tuple[str, Optional[Callable[["LoginResponse"], Awaitable[None]]]]:
11041106
"""Authenticates the user for the /login API
11051107
@@ -1110,6 +1112,9 @@ async def validate_login(
11101112
login_submission: the whole of the login submission
11111113
(including 'type' and other relevant fields)
11121114
ratelimit: whether to apply the failed_login_attempt ratelimiter
1115+
is_reauth: whether this is part of a User-Interactive Authorisation
1116+
flow to reauthenticate for a privileged action (rather than a
1117+
new login)
11131118
Returns:
11141119
A tuple of the canonical user id, and optional callback
11151120
to be called once the access token and device id are issued
@@ -1132,8 +1137,14 @@ async def validate_login(
11321137
# special case to check for "password" for the check_password interface
11331138
# for the auth providers
11341139
password = login_submission.get("password")
1140+
11351141
if login_type == LoginType.PASSWORD:
1136-
if not self._password_enabled:
1142+
if is_reauth:
1143+
passwords_allowed_here = self._password_enabled_for_reauth
1144+
else:
1145+
passwords_allowed_here = self._password_enabled_for_login
1146+
1147+
if not passwords_allowed_here:
11371148
raise SynapseError(400, "Password login has been disabled.")
11381149
if not isinstance(password, str):
11391150
raise SynapseError(400, "Bad parameter: password", Codes.INVALID_PARAM)

tests/rest/client/test_auth.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,8 +195,17 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
195195
self.user_pass = "pass"
196196
self.user = self.register_user("test", self.user_pass)
197197
self.device_id = "dev1"
198+
199+
# Force-enable password login for just long enough to log in.
200+
auth_handler = self.hs.get_auth_handler()
201+
allow_auth_for_login = auth_handler._password_enabled_for_login
202+
auth_handler._password_enabled_for_login = True
203+
198204
self.user_tok = self.login("test", self.user_pass, self.device_id)
199205

206+
# Restore password login to however it was.
207+
auth_handler._password_enabled_for_login = allow_auth_for_login
208+
200209
def delete_device(
201210
self,
202211
access_token: str,
@@ -263,6 +272,38 @@ def test_ui_auth(self) -> None:
263272
},
264273
)
265274

275+
@override_config({"password_config": {"enabled": "only_for_reauth"}})
276+
def test_ui_auth_with_passwords_for_reauth_only(self) -> None:
277+
"""
278+
Test user interactive authentication outside of registration.
279+
"""
280+
281+
# Attempt to delete this device.
282+
# Returns a 401 as per the spec
283+
channel = self.delete_device(
284+
self.user_tok, self.device_id, HTTPStatus.UNAUTHORIZED
285+
)
286+
287+
# Grab the session
288+
session = channel.json_body["session"]
289+
# Ensure that flows are what is expected.
290+
self.assertIn({"stages": ["m.login.password"]}, channel.json_body["flows"])
291+
292+
# Make another request providing the UI auth flow.
293+
self.delete_device(
294+
self.user_tok,
295+
self.device_id,
296+
HTTPStatus.OK,
297+
{
298+
"auth": {
299+
"type": "m.login.password",
300+
"identifier": {"type": "m.id.user", "user": self.user},
301+
"password": self.user_pass,
302+
"session": session,
303+
},
304+
},
305+
)
306+
266307
def test_grandfathered_identifier(self) -> None:
267308
"""Check behaviour without "identifier" dict
268309

0 commit comments

Comments
 (0)