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

Commit 1af6657

Browse files
committed
Add a configuration option to keep UI auth sessions open for a period of time.
1 parent 736e1df commit 1af6657

File tree

7 files changed

+131
-11
lines changed

7 files changed

+131
-11
lines changed

changelog.d/8970.feature

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Allow re-using an user-interactive authentication session for a period of time.

docs/sample_config.yaml

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2068,6 +2068,21 @@ password_config:
20682068
#
20692069
#require_uppercase: true
20702070

2071+
ui_auth:
2072+
# The number of milliseconds to allow a user-interactive authentication
2073+
# session to be active.
2074+
#
2075+
# This defaults to 0, meaning the user is queried for their credentials
2076+
# before every action, but this can be overridden to alow a single
2077+
# validation to be re-used. This weakens the protections afforded by
2078+
# the user-interactive authentication process, by allowing for multiple
2079+
# (and potentially different) operations to use the same validation session.
2080+
#
2081+
# Uncomment below to allow for credential validation to last for 15
2082+
# seconds.
2083+
#
2084+
#session_timeout: 15000
2085+
20712086

20722087
# Configuration for sending emails from Synapse.
20732088
#

synapse/config/auth.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ def read_config(self, config, **kwargs):
3636
self.password_policy = password_config.get("policy") or {}
3737
self.password_policy_enabled = self.password_policy.get("enabled", False)
3838

39+
# User-interactive authentication
40+
ui_auth = config.get("ui_auth") or {}
41+
self.ui_auth_session_timeout = ui_auth.get("session_timeout", 0)
42+
3943
def generate_config_section(self, config_dir_path, server_name, **kwargs):
4044
return """\
4145
password_config:
@@ -88,4 +92,19 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs):
8892
# Defaults to 'false'.
8993
#
9094
#require_uppercase: true
95+
96+
ui_auth:
97+
# The number of milliseconds to allow a user-interactive authentication
98+
# session to be active.
99+
#
100+
# This defaults to 0, meaning the user is queried for their credentials
101+
# before every action, but this can be overridden to alow a single
102+
# validation to be re-used. This weakens the protections afforded by
103+
# the user-interactive authentication process, by allowing for multiple
104+
# (and potentially different) operations to use the same validation session.
105+
#
106+
# Uncomment below to allow for credential validation to last for 15
107+
# seconds.
108+
#
109+
#session_timeout: 15000
91110
"""

synapse/handlers/auth.py

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,9 @@ def __init__(self, hs: "HomeServer"):
226226
burst_count=self.hs.config.rc_login_failed_attempts.burst_count,
227227
)
228228

229+
# The number of seconds to keep a UI auth session active.
230+
self._ui_auth_session_timeout = hs.config.ui_auth_session_timeout
231+
229232
# Ratelimitier for failed /login attempts
230233
self._failed_login_attempts_ratelimiter = Ratelimiter(
231234
clock=hs.get_clock(),
@@ -283,7 +286,7 @@ async def validate_user_via_ui_auth(
283286
request_body: Dict[str, Any],
284287
clientip: str,
285288
description: str,
286-
) -> Tuple[dict, str]:
289+
) -> Tuple[dict, Optional[str]]:
287290
"""
288291
Checks that the user is who they claim to be, via a UI auth.
289292
@@ -310,7 +313,8 @@ async def validate_user_via_ui_auth(
310313
have been given only in a previous call).
311314
312315
'session_id' is the ID of this session, either passed in by the
313-
client or assigned by this call
316+
client or assigned by this call. This is None if UI auth was
317+
skipped (by re-using a previous validation).
314318
315319
Raises:
316320
InteractiveAuthIncompleteError if the client has not yet completed
@@ -324,6 +328,19 @@ async def validate_user_via_ui_auth(
324328
325329
"""
326330

331+
if self._ui_auth_session_timeout:
332+
last_validated = await self.store.get_access_token_last_validated(
333+
requester.access_token_id
334+
)
335+
if (
336+
self.clock.time_msec() - last_validated
337+
< self._ui_auth_session_timeout
338+
):
339+
# Return the input parameters, minus the auth key, which matches
340+
# the logic in check_ui_auth.
341+
request_body.pop("auth", None)
342+
return request_body, None
343+
327344
user_id = requester.user.to_string()
328345

329346
# Check if we should be ratelimited due to too many previous failed attempts
@@ -455,13 +472,10 @@ async def check_ui_auth(
455472
all the stages in any of the permitted flows.
456473
"""
457474

458-
authdict = None
459475
sid = None # type: Optional[str]
460-
if clientdict and "auth" in clientdict:
461-
authdict = clientdict["auth"]
462-
del clientdict["auth"]
463-
if "session" in authdict:
464-
sid = authdict["session"]
476+
authdict = clientdict.pop("auth", {})
477+
if "session" in authdict:
478+
sid = authdict["session"]
465479

466480
# Convert the URI and method to strings.
467481
uri = request.uri.decode("utf-8")

synapse/rest/client/v2_alpha/account.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -254,14 +254,18 @@ async def on_POST(self, request):
254254
logger.error("Auth succeeded but no known type! %r", result.keys())
255255
raise SynapseError(500, "", Codes.UNKNOWN)
256256

257-
# If we have a password in this request, prefer it. Otherwise, there
258-
# must be a password hash from an earlier request.
257+
# If we have a password in this request, prefer it. Otherwise, use the
258+
# password hash from an earlier request.
259259
if new_password:
260260
password_hash = await self.auth_handler.hash(new_password)
261-
else:
261+
elif session_id is not None:
262262
password_hash = await self.auth_handler.get_session_data(
263263
session_id, "password_hash", None
264264
)
265+
else:
266+
# UI validation was skipped, but the request did not include a new
267+
# password.
268+
password_hash = None
265269
if not password_hash:
266270
raise SynapseError(400, "Missing params: password", Codes.MISSING_PARAM)
267271

synapse/storage/databases/main/registration.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -943,6 +943,25 @@ async def del_user_pending_deactivation(self, user_id: str) -> None:
943943
desc="del_user_pending_deactivation",
944944
)
945945

946+
async def get_access_token_last_validated(self, token_id: int) -> int:
947+
"""Retrieves the time (in milliseconds) of the last validation of an access token.
948+
949+
Args:
950+
token_id: The ID of the access token to update.
951+
Raises:
952+
StoreError if the access token was not found.
953+
954+
Returns:
955+
The last validation time.
956+
"""
957+
result = await self.db_pool.simple_select_one_onecol(
958+
"access_tokens", {"id": token_id}, "last_validated"
959+
)
960+
961+
# If this token has not been validated (since starting to track this),
962+
# return 0 instead of None.
963+
return result or 0
964+
946965
async def update_access_token_last_validated(self, token_id: int) -> None:
947966
"""Updates the last time an access token was validated.
948967

tests/rest/client/v2_alpha/test_auth.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,8 @@ def test_cannot_change_uri(self):
318318

319319
# Make another request providing the UI auth flow, but try to delete the
320320
# second device. This results in an error.
321+
#
322+
# This makes use of the fact that the device ID is embedded into the URL.
321323
self.delete_device(
322324
self.user_tok,
323325
"dev2",
@@ -332,6 +334,52 @@ def test_cannot_change_uri(self):
332334
},
333335
)
334336

337+
@unittest.override_config({"ui_auth": {"session_timeout": 5 * 1000}})
338+
def test_can_reuse_session(self):
339+
"""
340+
The session can be reused if configured.
341+
342+
Compare to test_cannot_change_uri.
343+
"""
344+
# Create a second and third login.
345+
self.login("test", self.user_pass, "dev2")
346+
self.login("test", self.user_pass, "dev3")
347+
348+
# Attempt to delete a device. This works since the user just logged in.
349+
self.delete_device(self.user_tok, "dev2", 200)
350+
351+
# Move the clock forward past the validation timeout.
352+
self.reactor.advance(6)
353+
354+
# Deleting another devices throws the user into UI auth.
355+
channel = self.delete_device(self.user_tok, "dev3", 401)
356+
357+
# Grab the session
358+
session = channel.json_body["session"]
359+
# Ensure that flows are what is expected.
360+
self.assertIn({"stages": ["m.login.password"]}, channel.json_body["flows"])
361+
362+
# Make another request providing the UI auth flow.
363+
self.delete_device(
364+
self.user_tok,
365+
"dev3",
366+
200,
367+
{
368+
"auth": {
369+
"type": "m.login.password",
370+
"identifier": {"type": "m.id.user", "user": self.user},
371+
"password": self.user_pass,
372+
"session": session,
373+
},
374+
},
375+
)
376+
377+
# Make another request, but try to delete the first device. This works
378+
# due to re-using the previous session.
379+
#
380+
# Note that *no auth* information is provided, not even a session iD!
381+
self.delete_device(self.user_tok, self.device_id, 200)
382+
335383
def test_does_not_offer_password_for_sso_user(self):
336384
login_resp = self.helper.login_via_oidc("username")
337385
user_tok = login_resp["access_token"]

0 commit comments

Comments
 (0)