diff options
| -rw-r--r-- | changelog.d/8937.bugfix | 1 | ||||
| -rw-r--r-- | synapse/handlers/auth.py | 26 | ||||
| -rw-r--r-- | tests/handlers/test_password_providers.py | 23 |
3 files changed, 36 insertions, 14 deletions
diff --git a/changelog.d/8937.bugfix b/changelog.d/8937.bugfix new file mode 100644 index 0000000000..01e1848448 --- /dev/null +++ b/changelog.d/8937.bugfix @@ -0,0 +1 @@ +Fix bug introduced in Synapse v1.24.0 which would cause an exception on startup if both `enabled` and `localdb_enabled` were set to `False` in the `password_config` setting of the configuration file. diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 8deec4cd0c..21e568f226 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -198,27 +198,25 @@ class AuthHandler(BaseHandler): self._password_enabled = hs.config.password_enabled self._password_localdb_enabled = hs.config.password_localdb_enabled - # we keep this as a list despite the O(N^2) implication so that we can - # keep PASSWORD first and avoid confusing clients which pick the first - # type in the list. (NB that the spec doesn't require us to do so and - # clients which favour types that they don't understand over those that - # they do are technically broken) - # start out by assuming PASSWORD is enabled; we will remove it later if not. - login_types = [] + login_types = set() if self._password_localdb_enabled: - login_types.append(LoginType.PASSWORD) + login_types.add(LoginType.PASSWORD) for provider in self.password_providers: - if hasattr(provider, "get_supported_login_types"): - for t in provider.get_supported_login_types().keys(): - if t not in login_types: - login_types.append(t) + login_types.update(provider.get_supported_login_types().keys()) if not self._password_enabled: + login_types.discard(LoginType.PASSWORD) + + # Some clients just pick the first type in the list. In this case, we want + # them to use PASSWORD (rather than token or whatever), so we want to make sure + # that comes first, where it's present. + self._supported_login_types = [] + if LoginType.PASSWORD in login_types: + self._supported_login_types.append(LoginType.PASSWORD) login_types.remove(LoginType.PASSWORD) - - self._supported_login_types = login_types + self._supported_login_types.extend(login_types) # Ratelimiter for failed auth during UIA. Uses same ratelimit config # as per `rc_login.failed_attempts`. diff --git a/tests/handlers/test_password_providers.py b/tests/handlers/test_password_providers.py index ceaf0902d2..8d50265145 100644 --- a/tests/handlers/test_password_providers.py +++ b/tests/handlers/test_password_providers.py @@ -432,6 +432,29 @@ class PasswordAuthProviderTests(unittest.HomeserverTestCase): @override_config( { + **providers_config(CustomAuthProvider), + "password_config": {"enabled": False, "localdb_enabled": False}, + } + ) + def test_custom_auth_password_disabled_localdb_enabled(self): + """Check the localdb_enabled == enabled == False + + Regression test for https://github.com/matrix-org/synapse/issues/8914: check + that setting *both* `localdb_enabled` *and* `password: enabled` to False doesn't + cause an exception. + """ + self.register_user("localuser", "localpass") + + flows = self._get_login_flows() + self.assertEqual(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") + self.assertEqual(channel.code, 400, channel.result) + mock_password_provider.check_auth.assert_not_called() + + @override_config( + { **providers_config(PasswordCustomAuthProvider), "password_config": {"enabled": False}, } |