mirror of
https://git.anonymousland.org/anonymousland/synapse-product.git
synced 2024-10-01 08:25:44 -04:00
Don't offer password login when it is disabled (#8835)
Fix a minor bug where we would offer "m.login.password" login if a custom auth provider supported it, even if password login was disabled.
This commit is contained in:
parent
ddc4343683
commit
89f7930730
1
changelog.d/8835.bugfix
Normal file
1
changelog.d/8835.bugfix
Normal file
@ -0,0 +1 @@
|
|||||||
|
Fix minor long-standing bug in login, where we would offer the `password` login type if a custom auth provider supported it, even if password login was disabled.
|
@ -205,15 +205,23 @@ class AuthHandler(BaseHandler):
|
|||||||
# type in the list. (NB that the spec doesn't require us to do so and
|
# 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
|
# clients which favour types that they don't understand over those that
|
||||||
# they do are technically broken)
|
# they do are technically broken)
|
||||||
|
|
||||||
|
# start out by assuming PASSWORD is enabled; we will remove it later if not.
|
||||||
login_types = []
|
login_types = []
|
||||||
if self._password_enabled:
|
if hs.config.password_localdb_enabled:
|
||||||
login_types.append(LoginType.PASSWORD)
|
login_types.append(LoginType.PASSWORD)
|
||||||
|
|
||||||
for provider in self.password_providers:
|
for provider in self.password_providers:
|
||||||
if hasattr(provider, "get_supported_login_types"):
|
if hasattr(provider, "get_supported_login_types"):
|
||||||
for t in provider.get_supported_login_types().keys():
|
for t in provider.get_supported_login_types().keys():
|
||||||
if t not in login_types:
|
if t not in login_types:
|
||||||
login_types.append(t)
|
login_types.append(t)
|
||||||
|
|
||||||
|
if not self._password_enabled:
|
||||||
|
login_types.remove(LoginType.PASSWORD)
|
||||||
|
|
||||||
self._supported_login_types = login_types
|
self._supported_login_types = login_types
|
||||||
|
|
||||||
# Login types and UI Auth types have a heavy overlap, but are not
|
# Login types and UI Auth types have a heavy overlap, but are not
|
||||||
# necessarily identical. Login types have SSO (and other login types)
|
# necessarily identical. Login types have SSO (and other login types)
|
||||||
# added in the rest layer, see synapse.rest.client.v1.login.LoginRestServerlet.on_GET.
|
# added in the rest layer, see synapse.rest.client.v1.login.LoginRestServerlet.on_GET.
|
||||||
|
@ -70,6 +70,24 @@ class CustomAuthProvider:
|
|||||||
return mock_password_provider.check_auth(*args)
|
return mock_password_provider.check_auth(*args)
|
||||||
|
|
||||||
|
|
||||||
|
class PasswordCustomAuthProvider:
|
||||||
|
"""A password_provider which implements password login via `check_auth`, as well
|
||||||
|
as a custom type."""
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def parse_config(self):
|
||||||
|
pass
|
||||||
|
|
||||||
|
def __init__(self, config, account_handler):
|
||||||
|
pass
|
||||||
|
|
||||||
|
def get_supported_login_types(self):
|
||||||
|
return {"m.login.password": ["password"], "test.login_type": ["test_field"]}
|
||||||
|
|
||||||
|
def check_auth(self, *args):
|
||||||
|
return mock_password_provider.check_auth(*args)
|
||||||
|
|
||||||
|
|
||||||
def providers_config(*providers: Type[Any]) -> dict:
|
def providers_config(*providers: Type[Any]) -> dict:
|
||||||
"""Returns a config dict that will enable the given password auth providers"""
|
"""Returns a config dict that will enable the given password auth providers"""
|
||||||
return {
|
return {
|
||||||
@ -246,7 +264,11 @@ class PasswordAuthProviderTests(unittest.HomeserverTestCase):
|
|||||||
mock_password_provider.check_password.reset_mock()
|
mock_password_provider.check_password.reset_mock()
|
||||||
|
|
||||||
# first delete should give a 401
|
# first delete should give a 401
|
||||||
session = self._start_delete_device_session(tok1, "dev2")
|
channel = self._delete_device(tok1, "dev2")
|
||||||
|
self.assertEqual(channel.code, 401)
|
||||||
|
# there are no valid flows here!
|
||||||
|
self.assertEqual(channel.json_body["flows"], [])
|
||||||
|
session = channel.json_body["session"]
|
||||||
mock_password_provider.check_password.assert_not_called()
|
mock_password_provider.check_password.assert_not_called()
|
||||||
|
|
||||||
# now try deleting with the local password
|
# now try deleting with the local password
|
||||||
@ -410,6 +432,88 @@ class PasswordAuthProviderTests(unittest.HomeserverTestCase):
|
|||||||
self.assertEqual(channel.code, 400, channel.result)
|
self.assertEqual(channel.code, 400, channel.result)
|
||||||
mock_password_provider.check_auth.assert_not_called()
|
mock_password_provider.check_auth.assert_not_called()
|
||||||
|
|
||||||
|
@override_config(
|
||||||
|
{
|
||||||
|
**providers_config(PasswordCustomAuthProvider),
|
||||||
|
"password_config": {"enabled": False},
|
||||||
|
}
|
||||||
|
)
|
||||||
|
def test_password_custom_auth_password_disabled_login(self):
|
||||||
|
"""log in with a custom auth provider which implements password, but password
|
||||||
|
login is disabled"""
|
||||||
|
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},
|
||||||
|
}
|
||||||
|
)
|
||||||
|
def test_password_custom_auth_password_disabled_ui_auth(self):
|
||||||
|
"""UI Auth with a custom auth provider which implements password, but password
|
||||||
|
login is disabled"""
|
||||||
|
# register the user and log in twice via the test login type to get two devices,
|
||||||
|
self.register_user("localuser", "localpass")
|
||||||
|
mock_password_provider.check_auth.return_value = defer.succeed(
|
||||||
|
"@localuser:test"
|
||||||
|
)
|
||||||
|
channel = self._send_login("test.login_type", "localuser", test_field="")
|
||||||
|
self.assertEqual(channel.code, 200, channel.result)
|
||||||
|
tok1 = channel.json_body["access_token"]
|
||||||
|
|
||||||
|
channel = self._send_login(
|
||||||
|
"test.login_type", "localuser", test_field="", device_id="dev2"
|
||||||
|
)
|
||||||
|
self.assertEqual(channel.code, 200, channel.result)
|
||||||
|
|
||||||
|
# make the initial request which returns a 401
|
||||||
|
channel = self._delete_device(tok1, "dev2")
|
||||||
|
self.assertEqual(channel.code, 401)
|
||||||
|
# Ensure that flows are what is expected. In particular, "password" should *not*
|
||||||
|
# be present.
|
||||||
|
self.assertIn({"stages": ["test.login_type"]}, channel.json_body["flows"])
|
||||||
|
session = channel.json_body["session"]
|
||||||
|
|
||||||
|
mock_password_provider.reset_mock()
|
||||||
|
|
||||||
|
# check that auth with password is rejected
|
||||||
|
body = {
|
||||||
|
"auth": {
|
||||||
|
"type": "m.login.password",
|
||||||
|
"identifier": {"type": "m.id.user", "user": "localuser"},
|
||||||
|
# FIXME "identifier" is ignored
|
||||||
|
# https://github.com/matrix-org/synapse/issues/5665
|
||||||
|
"user": "localuser",
|
||||||
|
"password": "localpass",
|
||||||
|
"session": session,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
channel = self._delete_device(tok1, "dev2", body)
|
||||||
|
self.assertEqual(channel.code, 400)
|
||||||
|
self.assertEqual(
|
||||||
|
"Password login has been disabled.", channel.json_body["error"]
|
||||||
|
)
|
||||||
|
mock_password_provider.check_auth.assert_not_called()
|
||||||
|
mock_password_provider.reset_mock()
|
||||||
|
|
||||||
|
# successful auth
|
||||||
|
body["auth"]["type"] = "test.login_type"
|
||||||
|
body["auth"]["test_field"] = "x"
|
||||||
|
channel = self._delete_device(tok1, "dev2", body)
|
||||||
|
self.assertEqual(channel.code, 200)
|
||||||
|
mock_password_provider.check_auth.assert_called_once_with(
|
||||||
|
"localuser", "test.login_type", {"test_field": "x"}
|
||||||
|
)
|
||||||
|
|
||||||
@override_config(
|
@override_config(
|
||||||
{
|
{
|
||||||
**providers_config(CustomAuthProvider),
|
**providers_config(CustomAuthProvider),
|
||||||
@ -428,8 +532,6 @@ class PasswordAuthProviderTests(unittest.HomeserverTestCase):
|
|||||||
channel = self._send_password_login("localuser", "localpass")
|
channel = self._send_password_login("localuser", "localpass")
|
||||||
self.assertEqual(channel.code, 400, channel.result)
|
self.assertEqual(channel.code, 400, channel.result)
|
||||||
|
|
||||||
test_custom_auth_no_local_user_fallback.skip = "currently broken"
|
|
||||||
|
|
||||||
def _get_login_flows(self) -> JsonDict:
|
def _get_login_flows(self) -> JsonDict:
|
||||||
_, channel = self.make_request("GET", "/_matrix/client/r0/login")
|
_, channel = self.make_request("GET", "/_matrix/client/r0/login")
|
||||||
self.assertEqual(channel.code, 200, channel.result)
|
self.assertEqual(channel.code, 200, channel.result)
|
||||||
|
Loading…
Reference in New Issue
Block a user