Comment cleanups, log on KeyError during login

Author: anoadragon453

synapse/handlers/auth.py

@@ -396,7 +396,7 @@ async def check_auth(
             # otherwise use whatever was last provided.
             #
             # This was designed to allow the client to omit the parameters
-            # and just supply the session in subsequent calls so it split
+            # and just supply the session in subsequent calls. So it splits
             # auth between devices by just sharing the session, (eg. so you
             # could continue registration from your phone having clicked the
             # email auth link on there). It's probably too open to abuse
@@ -876,7 +876,8 @@ async def validate_login(
         m.login.password auth types.
 
         Args:
-            username: username supplied by the user
+            username: a localpart or fully qualified user ID - what is provided by the
+                client
             login_submission: the whole of the login submission
                 (including 'type' and other relevant fields)
         Returns:
@@ -888,10 +889,10 @@ async def validate_login(
             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()
+        # We need a fully qualified User ID for some method calls here
+        qualified_user_id = username
+        if not qualified_user_id.startswith("@"):
+            qualified_user_id = UserID(qualified_user_id, self.hs.hostname).to_string()
 
         login_type = login_submission.get("type")
         known_login_type = False

synapse/rest/client/v1/login.py

@@ -111,7 +111,8 @@ async def on_POST(self, request):
                 result = await self.do_token_login(login_submission)
             else:
                 result = await self._do_other_login(login_submission)
-        except KeyError:
+        except KeyError as e:
+            logger.debug("KeyError during login: %s", e)
             raise SynapseError(400, "Missing JSON keys.")
 
         well_known_data = self._well_known_builder.get_well_known()
@@ -181,8 +182,8 @@ async def _do_other_login(self, 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.
+            # exception and masking the LoginError. This just records the attempt.
+            # The actual rate-limiting happens above
             self._failed_attempts_ratelimiter.can_do_action(username.lower())
             raise
 
@@ -195,10 +196,10 @@ async def _complete_login(
         self, user_id, login_submission, callback=None, create_non_existent_users=False
     ):
         """Called when we've successfully authed the user and now need to
-        actually login them in (e.g. create devices). This gets called on
-        all succesful logins.
+        actually log them in (e.g. create devices). This gets called on
+        all successful logins.
 
-        Applies the ratelimiting for succesful login attempts against an
+        Applies the ratelimiting for successful login attempts against an
         account.
 
         Args: