Always require users to re-authenticate for dangerous operations. (#1…

…0184)

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.

changelog.d/10184.bugfix

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

docs/sample_config.yaml

@@ -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.
     #

synapse/config/auth.py

@@ -103,6 +103,10 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs):
             # 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.
             #

synapse/handlers/auth.py

@@ -302,6 +302,7 @@ async def validate_user_via_ui_auth(
         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 @@ async def validate_user_via_ui_auth(
             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 @@ async def validate_user_via_ui_auth(
         """
         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
             )

synapse/rest/client/v2_alpha/devices.py

@@ -86,6 +86,9 @@ async def on_POST(self, request):
             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 @@ async def on_DELETE(self, request, device_id):
             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)

synapse/rest/client/v2_alpha/keys.py

@@ -277,6 +277,9 @@ async def on_POST(self, request):
             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)