Allow for identifier dicts in User Interactive Auth dicts

synapse/handlers/auth.py

@@ -603,6 +603,87 @@ async def _check_auth_dict(
         (canonical_id, callback) = await self.validate_login(user_id, authdict)
         return canonical_id
 
+    async def username_from_identifier(
+        self, identifier: Dict[str, str], password: Optional[str] = None
+    ) -> Optional[str]:
+        """Given a dictionary containing an identifier from a client, extract the
+        possibly unqualified username of the user that it identifies. Does *not*
+        guarantee that the user exists.
+        If this identifier dict contains a threepid, we attempt to ask password
+        auth providers about it or, failing that, look up an associated user in
+        the database.
+        Args:
+            identifier: The identifier dictionary provided by the client
+            password: The user provided password if one exists. Used for asking
+                password auth providers for usernames from 3pid+password combos.
+        Returns:
+            A username if one was found, or None otherwise
+        Raises:
+            SynapseError: If the identifier dict is invalid
+        """
+
+        # Convert phone type identifiers to generic threepid identifiers, which
+        # will be handled in the next step
+        if identifier["type"] == "m.id.phone":
+            login_id_phone_to_thirdparty(identifier)
+
+        # Convert a threepid identifier to an user identifier
+        if identifier["type"] == "m.id.thirdparty":
+            address = identifier.get("address")
+            medium = identifier.get("medium")
+
+            if not medium or not address:
+                # An error would've already been raised in
+                # `login_id_thirdparty_from_phone` if the original submission
+                # was a phone identifier
+                raise SynapseError(
+                    400, "Invalid thirdparty identifier", errcode=Codes.INVALID_PARAM,
+                )
+
+            if medium == "email":
+                # For emails, transform the address to lowercase.
+                # We store all email addresses as lowercase in the DB.
+                # (See add_threepid in synapse/handlers/auth.py)
+                address = address.lower()
+
+            # Check for auth providers that support 3pid login types
+            if password is not None:
+                canonical_user_id, _ = await self.check_password_provider_3pid(
+                    medium, address, password,
+                )
+                if canonical_user_id:
+                    # Authentication through password provider and 3pid succeeded
+                    return canonical_user_id
+
+            # Check local store
+            user_id = await self.hs.get_datastore().get_user_id_by_threepid(
+                medium, address
+            )
+            if not user_id:
+                # We were unable to find a user_id that belonged to the threepid returned
+                # by the password auth provider
+                return None
+
+            identifier = {"type": "m.id.user", "user": user_id}
+
+        # By this point, the identifier should be a `m.id.user`: if it's anything
+        # else, we haven't understood it.
+        if identifier["type"] != "m.id.user":
+            raise SynapseError(
+                400, "Unknown login identifier type", errcode=Codes.INVALID_PARAM,
+            )
+
+        # User identifiers have a "user" key
+        user = identifier.get("user")
+        if user is None:
+            raise SynapseError(
+                400,
+                "User identifier is missing 'user' key",
+                errcode=Codes.INVALID_PARAM,
+            )
+
+        return user
+
     def _get_params_recaptcha(self) -> dict:
         return {"public_key": self.hs.config.recaptcha_public_key}
 

synapse/rest/client/v1/login.py

@@ -17,10 +17,7 @@
 
 from synapse.api.errors import Codes, LoginError, SynapseError
 from synapse.api.ratelimiting import Ratelimiter
-from synapse.handlers.auth import (
-    client_dict_convert_legacy_fields_to_identifier,
-    login_id_phone_to_thirdparty,
-)
+from synapse.handlers.auth import client_dict_convert_legacy_fields_to_identifier
 from synapse.http.server import finish_request
 from synapse.http.servlet import (
     RestServlet,
@@ -144,99 +141,49 @@ async def _do_other_login(self, login_submission):
         # Convert deprecated authdict formats to the current scheme
         client_dict_convert_legacy_fields_to_identifier(login_submission)
 
-        if "identifier" not in login_submission:
-            raise SynapseError(400, "Missing param: identifier")
-
-        identifier = login_submission["identifier"]
-        if "type" not in identifier:
-            raise SynapseError(400, "Login identifier has no type")
-
-        # 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")
-
-            if medium == "email":
-                # For emails, transform the address to lowercase.
-                # We store all email addreses as lowercase in the DB.
-                # (See add_threepid in synapse/handlers/auth.py)
-                address = address.lower()
-
-            # 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"]
+        # Check whether this attempt uses a threepid, if so, check if our failed attempt
+        # ratelimiter allows another attempt at this time
+        medium, address = (
+            login_submission.get("medium"),
+            login_submission.get("address"),
+        )
+        if medium and address:
+            self._failed_attempts_ratelimiter.ratelimit(
+                (medium.lower(), address.lower()), update=False
             )
-            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
+        # Extract a localpart or user ID from the values in the identifier
+        username = await self.auth_handler.username_from_identifier(
+            login_submission["identifier"], login_submission.get("password")
+        )
 
-            # 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
+        if not username:
+            if medium and address:
+                # The user attempted to login via threepid and failed
+                # Record this failed attempt
+                self._failed_attempts_ratelimiter.can_do_action(
+                    (medium.lower(), address.lower())
                 )
-                # 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.
-        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("@"):
-            qualified_user_id = identifier["user"]
-        else:
-            qualified_user_id = UserID(identifier["user"], self.hs.hostname).to_string()
-
-        # Check if we've hit the failed ratelimit (but don't update it)
-        self._failed_attempts_ratelimiter.ratelimit(
-            qualified_user_id.lower(), update=False
-        )
+
+                raise LoginError(403, "Unauthorized threepid", errcode=Codes.FORBIDDEN)
+
+            # The login failed for another reason
+            raise LoginError(403, "Invalid login", errcode=Codes.FORBIDDEN)
+
+        # We were able to extract a username successfully
+        # Check if we've hit the failed ratelimit for this user ID
+        self._failed_attempts_ratelimiter.ratelimit(username.lower(), update=False)
 
         try:
             canonical_user_id, callback = await self.auth_handler.validate_login(
-                identifier["user"], login_submission
+                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.
-            self._failed_attempts_ratelimiter.can_do_action(qualified_user_id.lower())
+            self._failed_attempts_ratelimiter.can_do_action(username.lower())
             raise
 
         result = await self._complete_login(