Enable login_via_existing_session by default

changelog.d/15719.misc

@@ -0,0 +1 @@
+Enabled login_via_existing_session by default.

docs/usage/configuration/config_documentation.md

@@ -2598,14 +2598,11 @@ ui_auth:
 Matrix supports the ability of an existing session to mint a login token for
 another client.
 
-Synapse disables this by default as it has security ramifications -- a malicious
-client could use the mechanism to spawn more than one session.
-
 The duration of time the generated token is valid for can be configured with the
 `token_timeout` sub-option.
 
-User-interactive authentication is required when this is enabled unless the
-`require_ui_auth` sub-option is set to `False`.
+To protect against malicious clients abusing this capability, user-interactive authentication
+is required unless the `require_ui_auth` sub-option is set to `False`.
 
 Example configuration:
 ```yaml

synapse/config/auth.py

@@ -63,7 +63,7 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
 
         # Logging in with an existing session.
         login_via_existing = config.get("login_via_existing_session", {})
-        self.login_via_existing_enabled = login_via_existing.get("enabled", False)
+        self.login_via_existing_enabled = login_via_existing.get("enabled", True)
         self.login_via_existing_require_ui_auth = login_via_existing.get(
             "require_ui_auth", True
         )

tests/config/test_oauth_delegation.py

@@ -61,6 +61,9 @@ def setUp(self) -> None:
             **default_config("test"),
             "public_baseurl": BASE_URL,
             "enable_registration": False,
+            "login_via_existing_session": {
+                "enabled": False,
+            },
             "experimental_features": {
                 "msc3861": {
                     "enabled": True,

tests/handlers/test_oauth_delegation.py

@@ -115,6 +115,9 @@ def default_config(self) -> Dict[str, Any]:
         config = super().default_config()
         config["public_baseurl"] = BASE_URL
         config["disable_registration"] = True
+        config["login_via_existing_session"] = {
+            "enabled": False,
+        }
         config["experimental_features"] = {
             "msc3861": {
                 "enabled": True,

tests/handlers/test_password_providers.py

@@ -36,15 +36,20 @@
 from tests.unittest import override_config
 
 # Login flows we expect to appear in the list after the normal ones.
-ADDITIONAL_LOGIN_FLOWS = [
+ADDITIONAL_LOGIN_FLOWS: List[Dict] = [
     {"type": "m.login.application_service"},
+    {"type": "m.login.token", "get_login_token": True},
 ]
 
 # a mock instance which the dummy auth providers delegate to, so we can see what's going
 # on
 mock_password_provider = Mock()
 
 
+def sort_flows(flows: List[Dict[str, Any]]) -> List[Dict[str, Any]]:
+    return sorted(flows, key=lambda f: f["type"])
+
+
 class LegacyPasswordOnlyAuthProvider:
     """A legacy password_provider which only implements `check_password`."""
 
@@ -184,7 +189,10 @@ def test_password_only_auth_progiver_login_legacy(self) -> None:
     def password_only_auth_provider_login_test_body(self) -> None:
         # login flows should only have m.login.password
         flows = self._get_login_flows()
-        self.assertEqual(flows, [{"type": "m.login.password"}] + ADDITIONAL_LOGIN_FLOWS)
+        self.assertEqual(
+            sort_flows(flows),
+            sort_flows([{"type": "m.login.password"}] + ADDITIONAL_LOGIN_FLOWS),
+        )
 
         # check_password must return an awaitable
         mock_password_provider.check_password.return_value = make_awaitable(True)
@@ -365,7 +373,7 @@ def password_auth_disabled_test_body(self) -> None:
         """password auth doesn't work if it's disabled across the board"""
         # login flows should be empty
         flows = self._get_login_flows()
-        self.assertEqual(flows, ADDITIONAL_LOGIN_FLOWS)
+        self.assertEqual(sort_flows(flows), sort_flows(ADDITIONAL_LOGIN_FLOWS))
 
         # login shouldn't work and should be rejected with a 400 ("unknown login type")
         channel = self._send_password_login("u", "p")
@@ -386,9 +394,11 @@ def custom_auth_provider_login_test_body(self) -> None:
         # (password must come first, because reasons)
         flows = self._get_login_flows()
         self.assertEqual(
-            flows,
-            [{"type": "m.login.password"}, {"type": "test.login_type"}]
-            + ADDITIONAL_LOGIN_FLOWS,
+            sort_flows(flows),
+            sort_flows(
+                [{"type": "m.login.password"}, {"type": "test.login_type"}]
+                + ADDITIONAL_LOGIN_FLOWS
+            ),
         )
 
         # login with missing param should be rejected
@@ -519,7 +529,10 @@ def custom_auth_password_disabled_test_body(self) -> None:
         self.register_user("localuser", "localpass")
 
         flows = self._get_login_flows()
-        self.assertEqual(flows, [{"type": "test.login_type"}] + ADDITIONAL_LOGIN_FLOWS)
+        self.assertEqual(
+            sort_flows(flows),
+            sort_flows([{"type": "test.login_type"}] + ADDITIONAL_LOGIN_FLOWS),
+        )
 
         # login shouldn't work and should be rejected with a 400 ("unknown login type")
         channel = self._send_password_login("localuser", "localpass")
@@ -554,7 +567,10 @@ def custom_auth_password_disabled_localdb_enabled_test_body(self) -> None:
         self.register_user("localuser", "localpass")
 
         flows = self._get_login_flows()
-        self.assertEqual(flows, [{"type": "test.login_type"}] + ADDITIONAL_LOGIN_FLOWS)
+        self.assertEqual(
+            sort_flows(flows),
+            sort_flows([{"type": "test.login_type"}] + ADDITIONAL_LOGIN_FLOWS),
+        )
 
         # login shouldn't work and should be rejected with a 400 ("unknown login type")
         channel = self._send_password_login("localuser", "localpass")
@@ -585,7 +601,10 @@ def password_custom_auth_password_disabled_login_test_body(self) -> None:
         self.register_user("localuser", "localpass")
 
         flows = self._get_login_flows()
-        self.assertEqual(flows, [{"type": "test.login_type"}] + ADDITIONAL_LOGIN_FLOWS)
+        self.assertEqual(
+            sort_flows(flows),
+            sort_flows([{"type": "test.login_type"}] + ADDITIONAL_LOGIN_FLOWS),
+        )
 
         # login shouldn't work and should be rejected with a 400 ("unknown login type")
         channel = self._send_password_login("localuser", "localpass")
@@ -690,7 +709,10 @@ def custom_auth_no_local_user_fallback_test_body(self) -> None:
         self.register_user("localuser", "localpass")
 
         flows = self._get_login_flows()
-        self.assertEqual(flows, [{"type": "test.login_type"}] + ADDITIONAL_LOGIN_FLOWS)
+        self.assertEqual(
+            sort_flows(flows),
+            sort_flows([{"type": "test.login_type"}] + ADDITIONAL_LOGIN_FLOWS),
+        )
 
         # password login shouldn't work and should be rejected with a 400
         # ("unknown login type")
@@ -928,7 +950,7 @@ def _do_uia_assert_mock_not_called(self, username: str, m: Mock) -> JsonDict:
         self.assertEqual(channel.code, HTTPStatus.OK, channel.json_body)
         return channel.json_body
 
-    def _get_login_flows(self) -> JsonDict:
+    def _get_login_flows(self) -> List[JsonDict]:
         channel = self.make_request("GET", "/_matrix/client/r0/login")
         self.assertEqual(channel.code, HTTPStatus.OK, channel.result)
         return channel.json_body["flows"]

tests/rest/admin/test_jwks.py

@@ -45,6 +45,7 @@ def test_empty_jwks(self) -> None:
     @override_config(
         {
             "disable_registration": True,
+            "login_via_existing_session": {"enabled": False},
             "experimental_features": {
                 "msc3861": {
                     "enabled": True,
@@ -65,6 +66,7 @@ def test_empty_jwks_for_msc3861_client_secret_post(self) -> None:
     @override_config(
         {
             "disable_registration": True,
+            "login_via_existing_session": {"enabled": False},
             "experimental_features": {
                 "msc3861": {
                     "enabled": True,

tests/rest/client/test_capabilities.py

@@ -187,6 +187,7 @@ def test_get_does_include_msc3244_fields_when_enabled(self) -> None:
             for room_version in details["support"]:
                 self.assertTrue(room_version in KNOWN_ROOM_VERSIONS, str(room_version))
 
+    @override_config({"login_via_existing_session": {"enabled": False}})
     def test_get_get_token_login_fields_when_disabled(self) -> None:
         """By default login via an existing session is disabled."""
         access_token = self.get_success(
@@ -201,7 +202,6 @@ def test_get_get_token_login_fields_when_disabled(self) -> None:
         self.assertEqual(channel.code, HTTPStatus.OK)
         self.assertFalse(capabilities["m.get_login_token"]["enabled"])
 
-    @override_config({"login_via_existing_session": {"enabled": True}})
     def test_get_get_token_login_fields_when_enabled(self) -> None:
         access_token = self.get_success(
             self.auth_handler.create_access_token_for_user_id(

tests/rest/client/test_login.py

@@ -446,6 +446,7 @@ def test_require_approval(self) -> None:
             ApprovalNoticeMedium.NONE, channel.json_body["approval_notice_medium"]
         )
 
+    @override_config({"login_via_existing_session": {"enabled": False}})
     def test_get_login_flows_with_login_via_existing_disabled(self) -> None:
         """GET /login should return m.login.token without get_login_token"""
         channel = self.make_request("GET", "/_matrix/client/r0/login")
@@ -454,7 +455,6 @@ def test_get_login_flows_with_login_via_existing_disabled(self) -> None:
         flows = {flow["type"]: flow for flow in channel.json_body["flows"]}
         self.assertNotIn("m.login.token", flows)
 
-    @override_config({"login_via_existing_session": {"enabled": True}})
     def test_get_login_flows_with_login_via_existing_enabled(self) -> None:
         """GET /login should return m.login.token with get_login_token true"""
         channel = self.make_request("GET", "/_matrix/client/r0/login")

tests/rest/client/test_login_token_request.py

@@ -46,6 +46,7 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
         self.user = "user123"
         self.password = "password"
 
+    @override_config({"login_via_existing_session": {"enabled": False}})
     def test_disabled(self) -> None:
         channel = self.make_request("POST", GET_TOKEN_ENDPOINT, {}, access_token=None)
         self.assertEqual(channel.code, 404)
@@ -56,12 +57,10 @@ def test_disabled(self) -> None:
         channel = self.make_request("POST", GET_TOKEN_ENDPOINT, {}, access_token=token)
         self.assertEqual(channel.code, 404)
 
-    @override_config({"login_via_existing_session": {"enabled": True}})
     def test_require_auth(self) -> None:
         channel = self.make_request("POST", GET_TOKEN_ENDPOINT, {}, access_token=None)
         self.assertEqual(channel.code, 401)
 
-    @override_config({"login_via_existing_session": {"enabled": True}})
     def test_uia_on(self) -> None:
         user_id = self.register_user(self.user, self.password)
         token = self.login(self.user, self.password)
@@ -95,9 +94,7 @@ def test_uia_on(self) -> None:
         self.assertEqual(channel.code, 200, channel.result)
         self.assertEqual(channel.json_body["user_id"], user_id)
 
-    @override_config(
-        {"login_via_existing_session": {"enabled": True, "require_ui_auth": False}}
-    )
+    @override_config({"login_via_existing_session": {"require_ui_auth": False}})
     def test_uia_off(self) -> None:
         user_id = self.register_user(self.user, self.password)
         token = self.login(self.user, self.password)
@@ -119,7 +116,6 @@ def test_uia_off(self) -> None:
     @override_config(
         {
             "login_via_existing_session": {
-                "enabled": True,
                 "require_ui_auth": False,
                 "token_timeout": "15s",
             }
@@ -136,7 +132,6 @@ def test_expires_in(self) -> None:
     @override_config(
         {
             "login_via_existing_session": {
-                "enabled": True,
                 "require_ui_auth": False,
                 "token_timeout": "15s",
             }

tests/rest/test_well_known.py

@@ -119,6 +119,7 @@ def test_server_well_known_disabled(self) -> None:
                 },
             },
             "disable_registration": True,
+            "login_via_existing_session": {"enabled": False},
         }
     )
     def test_client_well_known_msc3861_oauth_delegation(self) -> None: