Forbid locked users from using POST /login (#18594)

Discussed in the [Synapse Dev
room](https://matrix.to/#/!vcyiEtMVHIhWXcJAfl:sw1v.org/$K4UojQtvaSpxSe35TWFXtKWGoAuHwHFcKo8qn2lwxSs?via=matrix.org&via=element.io&via=envs.net)

### Pull Request Checklist

<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->

* [x] Pull request is based on the develop branch
* [x] Pull request includes a [changelog
file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog).
The entry should:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
  - Use markdown where necessary, mostly for `code blocks`.
  - End with either a period (.) or an exclamation mark (!).
  - Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [x] [Code
style](https://element-hq.github.io/synapse/latest/code_style.html) is
correct (run the
[linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))

changelog.d/18594.bugfix

@@ -0,0 +1 @@
+Respond with 401 & `M_USER_LOCKED` when a locked user calls `POST /login`, as per the spec.

synapse/api/auth/internal.py

@@ -29,6 +29,7 @@ from synapse.api.errors import (
     InvalidClientTokenError,
     MissingClientTokenError,
     UnrecognizedRequestError,
+    UserLockedError,
 )
 from synapse.http.site import SynapseRequest
 from synapse.logging.opentracing import active_span, force_tracing, start_active_span
@@ -162,12 +163,7 @@ class InternalAuth(BaseAuth):
                 if not allow_locked and await self.store.get_user_locked_status(
                     requester.user.to_string()
                 ):
-                    raise AuthError(
-                        401,
-                        "User account has been locked",
-                        errcode=Codes.USER_LOCKED,
-                        additional_fields={"soft_logout": True},
-                    )
+                    raise UserLockedError()
 
                 # Deny the request if the user account has expired.
                 # This check is only done for regular users, not appservice ones.

synapse/api/errors.py

@@ -306,6 +306,20 @@ class UserDeactivatedError(SynapseError):
         )
 
 
+class UserLockedError(SynapseError):
+    """The error returned to the client when the user attempted to access an
+    authenticated endpoint, but the account has been locked.
+    """
+
+    def __init__(self) -> None:
+        super().__init__(
+            code=HTTPStatus.UNAUTHORIZED,
+            msg="User account has been locked",
+            errcode=Codes.USER_LOCKED,
+            additional_fields={"soft_logout": True},
+        )
+
+
 class FederationDeniedError(SynapseError):
     """An error raised when the server tries to federate with a server which
     is not on its federation whitelist.

synapse/rest/client/login.py

@@ -42,6 +42,7 @@ from synapse.api.errors import (
     NotApprovedError,
     SynapseError,
     UserDeactivatedError,
+    UserLockedError,
 )
 from synapse.api.ratelimiting import Ratelimiter
 from synapse.api.urls import CLIENT_API_PREFIX
@@ -313,7 +314,7 @@ class LoginRestServlet(RestServlet):
             should_issue_refresh_token=should_issue_refresh_token,
             # The user represented by an appservice's configured sender_localpart
             # is not actually created in Synapse.
-            should_check_deactivated=qualified_user_id != appservice.sender,
+            should_check_deactivated_or_locked=qualified_user_id != appservice.sender,
             request_info=request_info,
         )
 
@@ -367,7 +368,7 @@ class LoginRestServlet(RestServlet):
         auth_provider_id: Optional[str] = None,
         should_issue_refresh_token: bool = False,
         auth_provider_session_id: Optional[str] = None,
-        should_check_deactivated: bool = True,
+        should_check_deactivated_or_locked: bool = True,
         *,
         request_info: RequestInfo,
     ) -> LoginResponse:
@@ -389,8 +390,8 @@ class LoginRestServlet(RestServlet):
             should_issue_refresh_token: True if this login should issue
                 a refresh token alongside the access token.
             auth_provider_session_id: The session ID got during login from the SSO IdP.
-            should_check_deactivated: True if the user should be checked for
-                deactivation status before logging in.
+            should_check_deactivated_or_locked: True if the user should be checked for
+                deactivation or locked status before logging in.
 
                 This exists purely for appservice's configured sender_localpart
                 which doesn't have an associated user in the database.
@@ -415,11 +416,14 @@ class LoginRestServlet(RestServlet):
                 )
             user_id = canonical_uid
 
-        # If the account has been deactivated, do not proceed with the login.
-        if should_check_deactivated:
+        # If the account has been deactivated or locked, do not proceed with the login.
+        if should_check_deactivated_or_locked:
             deactivated = await self._main_store.get_user_deactivated_status(user_id)
             if deactivated:
                 raise UserDeactivatedError("This account has been deactivated")
+            locked = await self._main_store.get_user_locked_status(user_id)
+            if locked:
+                raise UserLockedError()
 
         device_id = login_submission.get("device_id")
 

tests/rest/admin/test_user.py

@@ -2846,6 +2846,16 @@ class UserRestTestCase(unittest.HomeserverTestCase):
         self.assertEqual(Codes.USER_LOCKED, channel.json_body["errcode"])
         self.assertTrue(channel.json_body["soft_logout"])
 
+        # User is not authorized to log in anymore
+        channel = self.make_request(
+            "POST",
+            "/_matrix/client/r0/login",
+            {"type": "m.login.password", "user": "user", "password": "pass"},
+        )
+        self.assertEqual(401, channel.code, msg=channel.json_body)
+        self.assertEqual(Codes.USER_LOCKED, channel.json_body["errcode"])
+        self.assertTrue(channel.json_body["soft_logout"])
+
     @override_config({"user_directory": {"enabled": True, "search_all_users": True}})
     def test_locked_user_not_in_user_dir(self) -> None:
         # User is available in the user dir