Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Support "identifier" dicts in UIA (#8848)
Browse files Browse the repository at this point in the history 
The spec requires synapse to support `identifier` dicts for `m.login.password`
user-interactive auth, which it did not (instead, it required an undocumented
`user` parameter.)

To fix this properly, we need to pull the code that interprets `identifier`
into `AuthHandler.validate_login` so that it can be called from the UIA code.

Fixes #5665.
  • Loading branch information
@richvdh
richvdh authored Dec 1, 2020
1 parent 9edff90 commit 4d94965
Show file tree
Hide file tree
Showing 5 changed files with 190 additions and 147 deletions.
1 change: 1 addition & 0 deletions changelog.d/8848.bugfix
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a long-standing bug which caused Synapse to require unspecified parameters during user-interactive authentication.
185 changes: 161 additions & 24 deletions synapse/handlers/auth.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,13 @@ def __init__(self, hs: "HomeServer"):
burst_count=self.hs.config.rc_login_failed_attempts.burst_count,
)

# Ratelimitier for failed /login attempts
self._failed_login_attempts_ratelimiter = Ratelimiter(
clock=hs.get_clock(),
rate_hz=self.hs.config.rc_login_failed_attempts.per_second,
burst_count=self.hs.config.rc_login_failed_attempts.burst_count,
)

self._clock = self.hs.get_clock()

# Expire old UI auth sessions after a period of time.
Expand Down Expand Up @@ -650,14 +657,8 @@ async def _check_auth_dict(
res = await checker.check_auth(authdict, clientip=clientip)
return res

# build a v1-login-style dict out of the authdict and fall back to the
# v1 code
user_id = authdict.get("user")

if user_id is None:
raise SynapseError(400, "", Codes.MISSING_PARAM)

(canonical_id, callback) = await self.validate_login(user_id, authdict)
# fall back to the v1 login flow
canonical_id, _ = await self.validate_login(authdict)
return canonical_id

def _get_params_recaptcha(self) -> dict:
Expand Down Expand Up @@ -832,17 +833,17 @@ def get_supported_login_types(self) -> Iterable[str]:
return self._supported_login_types

async def validate_login(
self, username: str, login_submission: Dict[str, Any]
self, login_submission: Dict[str, Any], ratelimit: bool = False,
) -> Tuple[str, Optional[Callable[[Dict[str, str]], None]]]:
"""Authenticates the user for the /login API
Also used by the user-interactive auth flow to validate
m.login.password auth types.
Also used by the user-interactive auth flow to validate auth types which don't
have an explicit UIA handler, including m.password.auth.
Args:
username: username supplied by the user
login_submission: the whole of the login submission
(including 'type' and other relevant fields)
ratelimit: whether to apply the failed_login_attempt ratelimiter
Returns:
A tuple of the canonical user id, and optional callback
to be called once the access token and device id are issued
Expand All @@ -851,29 +852,161 @@ async def validate_login(
SynapseError if there was a problem with the request
LoginError if there was an authentication problem.
"""

if username.startswith("@"):
qualified_user_id = username
else:
qualified_user_id = UserID(username, self.hs.hostname).to_string()

login_type = login_submission.get("type")
known_login_type = False

# ideally, we wouldn't be checking the identifier unless we know we have a login
# method which uses it (https://github.com/matrix-org/synapse/issues/8836)
#
# But the auth providers' check_auth interface requires a username, so in
# practice we can only support login methods which we can map to a username
# anyway.

# special case to check for "password" for the check_password interface
# for the auth providers
password = login_submission.get("password")

if login_type == LoginType.PASSWORD:
if not self._password_enabled:
raise SynapseError(400, "Password login has been disabled.")
if not password:
raise SynapseError(400, "Missing parameter: password")
if not isinstance(password, str):
raise SynapseError(400, "Bad parameter: password", Codes.INVALID_PARAM)

# map old-school login fields into new-school "identifier" fields.
identifier_dict = convert_client_dict_legacy_fields_to_identifier(
login_submission
)

# convert phone type identifiers to generic threepids
if identifier_dict["type"] == "m.id.phone":
identifier_dict = login_id_phone_to_thirdparty(identifier_dict)

# convert threepid identifiers to user IDs
if identifier_dict["type"] == "m.id.thirdparty":
address = identifier_dict.get("address")
medium = identifier_dict.get("medium")

if medium is None or address is None:
raise SynapseError(400, "Invalid thirdparty identifier")

# For emails, canonicalise the address.
# We store all email addresses canonicalised in the DB.
# (See add_threepid in synapse/handlers/auth.py)
if medium == "email":
try:
address = canonicalise_email(address)
except ValueError as e:
raise SynapseError(400, str(e))

# We also apply account rate limiting using the 3PID as a key, as
# otherwise using 3PID bypasses the ratelimiting based on user ID.
if ratelimit:
self._failed_login_attempts_ratelimiter.ratelimit(
(medium, address), update=False
)

# Check for login providers that support 3pid login types
if login_type == LoginType.PASSWORD:
# we've already checked that there is a (valid) password field
assert isinstance(password, str)
(
canonical_user_id,
callback_3pid,
) = await self.check_password_provider_3pid(medium, address, password)
if canonical_user_id:
# Authentication through password provider and 3pid succeeded
return canonical_user_id, callback_3pid

# No password providers were able to handle this 3pid
# Check local store
user_id = await self.hs.get_datastore().get_user_id_by_threepid(
medium, address
)
if not user_id:
logger.warning(
"unknown 3pid identifier medium %s, address %r", medium, address
)
# We mark that we've failed to log in here, as
# `check_password_provider_3pid` might have returned `None` due
# to an incorrect password, rather than the account not
# existing.
#
# If it returned None but the 3PID was bound then we won't hit
# this code path, which is fine as then the per-user ratelimit
# will kick in below.
if ratelimit:
self._failed_login_attempts_ratelimiter.can_do_action(
(medium, address)
)
raise LoginError(403, "", errcode=Codes.FORBIDDEN)

identifier_dict = {"type": "m.id.user", "user": user_id}

# by this point, the identifier should be an m.id.user: if it's anything
# else, we haven't understood it.
if identifier_dict["type"] != "m.id.user":
raise SynapseError(400, "Unknown login identifier type")

username = identifier_dict.get("user")
if not username:
raise SynapseError(400, "User identifier is missing 'user' key")

if username.startswith("@"):
qualified_user_id = username
else:
qualified_user_id = UserID(username, self.hs.hostname).to_string()

# Check if we've hit the failed ratelimit (but don't update it)
if ratelimit:
self._failed_login_attempts_ratelimiter.ratelimit(
qualified_user_id.lower(), update=False
)

try:
return await self._validate_userid_login(username, login_submission)
except LoginError:
# The user has failed to log in, so we need to update the rate
# limiter. Using `can_do_action` avoids us raising a ratelimit
# exception and masking the LoginError. The actual ratelimiting
# should have happened above.
if ratelimit:
self._failed_login_attempts_ratelimiter.can_do_action(
qualified_user_id.lower()
)
raise

async def _validate_userid_login(
self, username: str, login_submission: Dict[str, Any],
) -> Tuple[str, Optional[Callable[[Dict[str, str]], None]]]:
"""Helper for validate_login
Handles login, once we've mapped 3pids onto userids
Args:
username: the username, from the identifier dict
login_submission: the whole of the login submission
(including 'type' and other relevant fields)
Returns:
A tuple of the canonical user id, and optional callback
to be called once the access token and device id are issued
Raises:
StoreError if there was a problem accessing the database
SynapseError if there was a problem with the request
LoginError if there was an authentication problem.
"""
if username.startswith("@"):
qualified_user_id = username
else:
qualified_user_id = UserID(username, self.hs.hostname).to_string()

login_type = login_submission.get("type")
known_login_type = False

for provider in self.password_providers:
if hasattr(provider, "check_password") and login_type == LoginType.PASSWORD:
known_login_type = True
is_valid = await provider.check_password(qualified_user_id, password)
# we've already checked that there is a (valid) password field
is_valid = await provider.check_password(
qualified_user_id, login_submission["password"]
)
if is_valid:
return qualified_user_id, None

Expand Down Expand Up @@ -914,8 +1047,12 @@ async def validate_login(
if login_type == LoginType.PASSWORD and self.hs.config.password_localdb_enabled:
known_login_type = True

# we've already checked that there is a (valid) password field
password = login_submission["password"]
assert isinstance(password, str)

canonical_user_id = await self._check_local_password(
qualified_user_id, password # type: ignore
qualified_user_id, password
)

if canonical_user_id:
Expand Down
107 changes: 2 additions & 105 deletions synapse/rest/client/v1/login.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,6 @@
from synapse.api.errors import Codes, LoginError, SynapseError
from synapse.api.ratelimiting import Ratelimiter
from synapse.appservice import ApplicationService
from synapse.handlers.auth import (
convert_client_dict_legacy_fields_to_identifier,
login_id_phone_to_thirdparty,
)
from synapse.http.server import finish_request
from synapse.http.servlet import (
RestServlet,
Expand All @@ -33,7 +29,6 @@
from synapse.rest.client.v2_alpha._base import client_patterns
from synapse.rest.well_known import WellKnownBuilder
from synapse.types import JsonDict, UserID
from synapse.util.threepids import canonicalise_email

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -78,11 +73,6 @@ def __init__(self, hs):
rate_hz=self.hs.config.rc_login_account.per_second,
burst_count=self.hs.config.rc_login_account.burst_count,
)
self._failed_attempts_ratelimiter = Ratelimiter(
clock=hs.get_clock(),
rate_hz=self.hs.config.rc_login_failed_attempts.per_second,
burst_count=self.hs.config.rc_login_failed_attempts.burst_count,
)

def on_GET(self, request: SynapseRequest):
flows = []
Expand Down Expand Up @@ -140,17 +130,6 @@ async def on_POST(self, request: SynapseRequest):
result["well_known"] = well_known_data
return 200, result

def _get_qualified_user_id(self, identifier):
if identifier["type"] != "m.id.user":
raise SynapseError(400, "Unknown login identifier type")
if "user" not in identifier:
raise SynapseError(400, "User identifier is missing 'user' key")

if identifier["user"].startswith("@"):
return identifier["user"]
else:
return UserID(identifier["user"], self.hs.hostname).to_string()

async def _do_appservice_login(
self, login_submission: JsonDict, appservice: ApplicationService
):
Expand Down Expand Up @@ -201,91 +180,9 @@ async def _do_other_login(self, login_submission: JsonDict) -> Dict[str, str]:
login_submission.get("address"),
login_submission.get("user"),
)
identifier = convert_client_dict_legacy_fields_to_identifier(login_submission)

# convert phone type identifiers to generic threepids
if identifier["type"] == "m.id.phone":
identifier = login_id_phone_to_thirdparty(identifier)

# convert threepid identifiers to user IDs
if identifier["type"] == "m.id.thirdparty":
address = identifier.get("address")
medium = identifier.get("medium")

if medium is None or address is None:
raise SynapseError(400, "Invalid thirdparty identifier")

# For emails, canonicalise the address.
# We store all email addresses canonicalised in the DB.
# (See add_threepid in synapse/handlers/auth.py)
if medium == "email":
try:
address = canonicalise_email(address)
except ValueError as e:
raise SynapseError(400, str(e))

# We also apply account rate limiting using the 3PID as a key, as
# otherwise using 3PID bypasses the ratelimiting based on user ID.
self._failed_attempts_ratelimiter.ratelimit((medium, address), update=False)

# Check for login providers that support 3pid login types
(
canonical_user_id,
callback_3pid,
) = await self.auth_handler.check_password_provider_3pid(
medium, address, login_submission["password"]
)
if canonical_user_id:
# Authentication through password provider and 3pid succeeded

result = await self._complete_login(
canonical_user_id, login_submission, callback_3pid
)
return result

# No password providers were able to handle this 3pid
# Check local store
user_id = await self.hs.get_datastore().get_user_id_by_threepid(
medium, address
)
if not user_id:
logger.warning(
"unknown 3pid identifier medium %s, address %r", medium, address
)
# We mark that we've failed to log in here, as
# `check_password_provider_3pid` might have returned `None` due
# to an incorrect password, rather than the account not
# existing.
#
# If it returned None but the 3PID was bound then we won't hit
# this code path, which is fine as then the per-user ratelimit
# will kick in below.
self._failed_attempts_ratelimiter.can_do_action((medium, address))
raise LoginError(403, "", errcode=Codes.FORBIDDEN)

identifier = {"type": "m.id.user", "user": user_id}

# by this point, the identifier should be an m.id.user: if it's anything
# else, we haven't understood it.
qualified_user_id = self._get_qualified_user_id(identifier)

# Check if we've hit the failed ratelimit (but don't update it)
self._failed_attempts_ratelimiter.ratelimit(
qualified_user_id.lower(), update=False
canonical_user_id, callback = await self.auth_handler.validate_login(
login_submission, ratelimit=True
)

try:
canonical_user_id, callback = await self.auth_handler.validate_login(
identifier["user"], login_submission
)
except LoginError:
# The user has failed to log in, so we need to update the rate
# limiter. Using `can_do_action` avoids us raising a ratelimit
# exception and masking the LoginError. The actual ratelimiting
# should have happened above.
self._failed_attempts_ratelimiter.can_do_action(qualified_user_id.lower())
raise

result = await self._complete_login(
canonical_user_id, login_submission, callback
)
Expand Down
Loading

0 comments on commit 4d94965

Please sign in to comment.