Always require users to re-authenticate for dangerous operations. (#10184)

Dangerous actions means deactivating an account, modifying an account
password, or adding a 3PID.

Other actions (deleting devices, uploading keys) can re-use the same UI
auth session if ui_auth.session_timeout is configured.
This commit is contained in:
Patrick Cloke 2021-06-16 11:07:28 -04:00 committed by GitHub
parent b8b282aa32
commit 76f9c701c3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 24 additions and 1 deletions

1
changelog.d/10184.bugfix Normal file
View file

@ -0,0 +1 @@
Always require users to re-authenticate for dangerous operations: deactivating an account, modifying an account password, and adding 3PIDs.

View file

@ -2318,6 +2318,10 @@ ui_auth:
# the user-interactive authentication process, by allowing for multiple
# (and potentially different) operations to use the same validation session.
#
# This is ignored for potentially "dangerous" operations (including
# deactivating an account, modifying an account password, and
# adding a 3PID).
#
# Uncomment below to allow for credential validation to last for 15
# seconds.
#

View file

@ -103,6 +103,10 @@ class AuthConfig(Config):
# the user-interactive authentication process, by allowing for multiple
# (and potentially different) operations to use the same validation session.
#
# This is ignored for potentially "dangerous" operations (including
# deactivating an account, modifying an account password, and
# adding a 3PID).
#
# Uncomment below to allow for credential validation to last for 15
# seconds.
#

View file

@ -302,6 +302,7 @@ class AuthHandler(BaseHandler):
request: SynapseRequest,
request_body: Dict[str, Any],
description: str,
can_skip_ui_auth: bool = False,
) -> Tuple[dict, Optional[str]]:
"""
Checks that the user is who they claim to be, via a UI auth.
@ -320,6 +321,10 @@ class AuthHandler(BaseHandler):
description: A human readable string to be displayed to the user that
describes the operation happening on their account.
can_skip_ui_auth: True if the UI auth session timeout applies this
action. Should be set to False for any "dangerous"
actions (e.g. deactivating an account).
Returns:
A tuple of (params, session_id).
@ -343,7 +348,7 @@ class AuthHandler(BaseHandler):
"""
if not requester.access_token_id:
raise ValueError("Cannot validate a user without an access token")
if self._ui_auth_session_timeout:
if can_skip_ui_auth and self._ui_auth_session_timeout:
last_validated = await self.store.get_access_token_last_validated(
requester.access_token_id
)

View file

@ -86,6 +86,9 @@ class DeleteDevicesRestServlet(RestServlet):
request,
body,
"remove device(s) from your account",
# Users might call this multiple times in a row while cleaning up
# devices, allow a single UI auth session to be re-used.
can_skip_ui_auth=True,
)
await self.device_handler.delete_devices(
@ -135,6 +138,9 @@ class DeviceRestServlet(RestServlet):
request,
body,
"remove a device from your account",
# Users might call this multiple times in a row while cleaning up
# devices, allow a single UI auth session to be re-used.
can_skip_ui_auth=True,
)
await self.device_handler.delete_device(requester.user.to_string(), device_id)

View file

@ -277,6 +277,9 @@ class SigningKeyUploadServlet(RestServlet):
request,
body,
"add a device signing key to your account",
# Allow skipping of UI auth since this is frequently called directly
# after login and it is silly to ask users to re-auth immediately.
can_skip_ui_auth=True,
)
result = await self.e2e_keys_handler.upload_signing_keys_for_user(user_id, body)