auth: Use standard classes for API key fetch exceptions.

This lets us reuse a bunch of code and error handling logic, resulting
in a more sensible and consistent API.

zerver/lib/exceptions.py

@@ -53,6 +53,9 @@ class ErrorCode(AbstractEnum):
     RATE_LIMIT_HIT = ()
     USER_DEACTIVATED = ()
     REALM_DEACTIVATED = ()
+    PASSWORD_AUTH_DISABLED = ()
+    PASSWORD_RESET_REQUIRED = ()
+    AUTHENTICATION_FAILED = ()
 
 
 class JsonableError(Exception):
@@ -274,6 +277,20 @@ class StreamAdministratorRequired(JsonableError):
         return _("Must be an organization or stream administrator")
 
 
+class AuthenticationFailedError(JsonableError):
+    # Generic class for authentication failures
+    code: ErrorCode = ErrorCode.AUTHENTICATION_FAILED
+    # Bug: Shouldn't this be 401?
+    http_status_code = 403
+
+    def __init__(self) -> None:
+        pass
+
+    @staticmethod
+    def msg_format() -> str:
+        return _("Your username or password is incorrect")
+
+
 class UserDeactivatedError(JsonableError):
     code: ErrorCode = ErrorCode.USER_DEACTIVATED
     http_status_code = 403
@@ -298,6 +315,30 @@ class RealmDeactivatedError(JsonableError):
         return _("This organization has been deactivated")
 
 
+class PasswordAuthDisabledError(JsonableError):
+    code: ErrorCode = ErrorCode.PASSWORD_AUTH_DISABLED
+    http_status_code = 403
+
+    def __init__(self) -> None:
+        pass
+
+    @staticmethod
+    def msg_format() -> str:
+        return _("Password authentication is disabled in this organization")
+
+
+class PasswordResetRequiredError(JsonableError):
+    code: ErrorCode = ErrorCode.PASSWORD_RESET_REQUIRED
+    http_status_code = 403
+
+    def __init__(self) -> None:
+        pass
+
+    @staticmethod
+    def msg_format() -> str:
+        return _("Your password has been disabled and needs to be reset")
+
+
 class MarkdownRenderingException(Exception):
     pass
 

zerver/tests/test_auth_backends.py

@@ -4030,7 +4030,7 @@ class FetchAPIKeyTest(ZulipTestCase):
         result = self.client_post(
             "/api/v1/fetch_api_key", dict(username=self.email, password="wrong")
         )
-        self.assert_json_error(result, "Your username or password is incorrect.", 403)
+        self.assert_json_error(result, "Your username or password is incorrect", 403)
 
     def test_invalid_subdomain(self) -> None:
         with mock.patch("zerver.views.auth.get_realm_from_request", return_value=None):
@@ -4038,7 +4038,7 @@ class FetchAPIKeyTest(ZulipTestCase):
                 "/api/v1/fetch_api_key",
                 dict(username="hamlet", password=initial_password(self.email)),
             )
-        self.assert_json_error(result, "Invalid subdomain", 400)
+        self.assert_json_error(result, "Invalid subdomain", 404)
 
     def test_password_auth_disabled(self) -> None:
         with mock.patch("zproject.backends.password_auth_enabled", return_value=False):
@@ -4046,7 +4046,9 @@ class FetchAPIKeyTest(ZulipTestCase):
                 "/api/v1/fetch_api_key",
                 dict(username=self.email, password=initial_password(self.email)),
             )
-            self.assert_json_error_contains(result, "Password auth is disabled", 403)
+            self.assert_json_error_contains(
+                result, "Password authentication is disabled in this organization", 403
+            )
 
     @override_settings(AUTHENTICATION_BACKENDS=("zproject.backends.ZulipLDAPAuthBackend",))
     def test_ldap_auth_email_auth_disabled_success(self) -> None:
@@ -4072,14 +4074,14 @@ class FetchAPIKeyTest(ZulipTestCase):
             "/api/v1/fetch_api_key",
             dict(username="hamlet", password=self.ldap_password("hamlet")),
         )
-        self.assert_json_error(result, "Your username or password is incorrect.", 403)
+        self.assert_json_error(result, "Your username or password is incorrect", 403)
 
         self.change_ldap_user_attr("hamlet", "department", "testWrongRealm")
         result = self.client_post(
             "/api/v1/fetch_api_key",
             dict(username="hamlet", password=self.ldap_password("hamlet")),
         )
-        self.assert_json_error(result, "Your username or password is incorrect.", 403)
+        self.assert_json_error(result, "Your username or password is incorrect", 403)
 
         self.change_ldap_user_attr("hamlet", "department", "zulip")
         result = self.client_post(
@@ -4105,7 +4107,7 @@ class FetchAPIKeyTest(ZulipTestCase):
             "/api/v1/fetch_api_key",
             dict(username="hamlet", password=self.ldap_password("hamlet")),
         )
-        self.assert_json_error(result, "Your username or password is incorrect.", 403)
+        self.assert_json_error(result, "Your username or password is incorrect", 403)
 
         self.change_ldap_user_attr("hamlet", "test2", "testing")
         # Check with only one set
@@ -4113,7 +4115,7 @@ class FetchAPIKeyTest(ZulipTestCase):
             "/api/v1/fetch_api_key",
             dict(username="hamlet", password=self.ldap_password("hamlet")),
         )
-        self.assert_json_error(result, "Your username or password is incorrect.", 403)
+        self.assert_json_error(result, "Your username or password is incorrect", 403)
 
         self.change_ldap_user_attr("hamlet", "test1", "test")
         # Setting org_membership to not cause django_ldap_auth to warn, when synchronising
@@ -4148,7 +4150,7 @@ class FetchAPIKeyTest(ZulipTestCase):
             "/api/v1/fetch_api_key",
             dict(username="hamlet", password=self.ldap_password("hamlet")),
         )
-        self.assert_json_error(result, "Your username or password is incorrect.", 403)
+        self.assert_json_error(result, "Your username or password is incorrect", 403)
 
         # Override access with `org_membership`
         self.change_ldap_user_attr("hamlet", "department", "zulip")
@@ -4167,7 +4169,7 @@ class FetchAPIKeyTest(ZulipTestCase):
                 "/api/v1/fetch_api_key",
                 dict(username="hamlet", password=self.ldap_password("hamlet")),
             )
-            self.assert_json_error(result, "Your username or password is incorrect.", 403)
+            self.assert_json_error(result, "Your username or password is incorrect", 403)
 
     def test_inactive_user(self) -> None:
         do_deactivate_user(self.user_profile, acting_user=None)
@@ -4175,7 +4177,7 @@ class FetchAPIKeyTest(ZulipTestCase):
             "/api/v1/fetch_api_key",
             dict(username=self.email, password=initial_password(self.email)),
         )
-        self.assert_json_error_contains(result, "Your account has been disabled", 403)
+        self.assert_json_error_contains(result, "Account is deactivated", 403)
 
     def test_deactivated_realm(self) -> None:
         do_deactivate_realm(self.user_profile.realm, acting_user=None)
@@ -4204,7 +4206,9 @@ class FetchAPIKeyTest(ZulipTestCase):
                 "/api/v1/fetch_api_key",
                 dict(username=self.email, password=password),
             )
-            self.assert_json_error(result, "You need to reset your password.", 403)
+            self.assert_json_error(
+                result, "Your password has been disabled and needs to be reset", 403
+            )
 
 
 class DevFetchAPIKeyTest(ZulipTestCase):
@@ -4229,12 +4233,12 @@ class DevFetchAPIKeyTest(ZulipTestCase):
     def test_unregistered_user(self) -> None:
         email = "foo@zulip.com"
         result = self.client_post("/api/v1/dev_fetch_api_key", dict(username=email))
-        self.assert_json_error_contains(result, "This user is not registered.", 403)
+        self.assert_json_error_contains(result, "Your username or password is incorrect", 403)
 
     def test_inactive_user(self) -> None:
         do_deactivate_user(self.user_profile, acting_user=None)
         result = self.client_post("/api/v1/dev_fetch_api_key", dict(username=self.email))
-        self.assert_json_error_contains(result, "Your account has been disabled", 403)
+        self.assert_json_error_contains(result, "Account is deactivated", 403)
 
     def test_deactivated_realm(self) -> None:
         do_deactivate_realm(self.user_profile.realm, acting_user=None)
@@ -4254,7 +4258,7 @@ class DevFetchAPIKeyTest(ZulipTestCase):
                 "/api/v1/dev_fetch_api_key",
                 dict(username=self.email, password=initial_password(self.email)),
             )
-            self.assert_json_error_contains(result, "Invalid subdomain", 400)
+            self.assert_json_error_contains(result, "Invalid subdomain", 404)
 
 
 class DevGetEmailsTest(ZulipTestCase):

zerver/views/auth.py

@@ -41,12 +41,20 @@ from zerver.forms import (
     OurAuthenticationForm,
     ZulipPasswordResetForm,
 )
+from zerver.lib.exceptions import (
+    AuthenticationFailedError,
+    InvalidSubdomainError,
+    PasswordAuthDisabledError,
+    PasswordResetRequiredError,
+    RealmDeactivatedError,
+    UserDeactivatedError,
+)
 from zerver.lib.mobile_auth_otp import otp_encrypt_api_key
 from zerver.lib.push_notifications import push_notifications_enabled
 from zerver.lib.pysa import mark_sanitized
 from zerver.lib.realm_icon import realm_icon_url
 from zerver.lib.request import REQ, JsonableError, has_request_variables
-from zerver.lib.response import json_error, json_success
+from zerver.lib.response import json_success
 from zerver.lib.sessions import set_expirable_session_var
 from zerver.lib.subdomains import get_subdomain, is_subdomain_root_or_alias
 from zerver.lib.types import ViewFuncT
@@ -822,7 +830,7 @@ def api_fetch_api_key(
 
     realm = get_realm_from_request(request)
     if realm is None:
-        raise JsonableError(_("Invalid subdomain"))
+        raise InvalidSubdomainError()
 
     if not ldap_auth_enabled(realm=realm):
         # In case we don't authenticate against LDAP, check for a valid
@@ -832,33 +840,15 @@ def api_fetch_api_key(
         request=request, username=username, password=password, realm=realm, return_data=return_data
     )
     if return_data.get("inactive_user"):
-        return json_error(
+        raise UserDeactivatedError()
-            _("Your account has been disabled."), data={"reason": "user disable"}, status=403
-        )
     if return_data.get("inactive_realm"):
-        return json_error(
+        raise RealmDeactivatedError()
-            _("This organization has been deactivated."),
-            data={"reason": "realm deactivated"},
-            status=403,
-        )
     if return_data.get("password_auth_disabled"):
-        return json_error(
+        raise PasswordAuthDisabledError()
-            _("Password auth is disabled in your team."),
-            data={"reason": "password auth disabled"},
-            status=403,
-        )
     if return_data.get("password_reset_needed"):
-        return json_error(
+        raise PasswordResetRequiredError()
-            _("You need to reset your password."),
-            data={"reason": "password reset needed"},
-            status=403,
-        )
     if user_profile is None:
-        return json_error(
+        raise AuthenticationFailedError()
-            _("Your username or password is incorrect."),
-            data={"reason": "incorrect_creds"},
-            status=403,
-        )
 
     # Maybe sending 'user_logged_in' signal is the better approach:
     #   user_logged_in.send(sender=user_profile.__class__, request=request, user=user_profile)

zerver/views/development/dev_login.py

@@ -8,8 +8,14 @@ from django.views.decorators.csrf import csrf_exempt
 
 from zerver.context_processors import get_realm_from_request
 from zerver.decorator import do_login, require_post
+from zerver.lib.exceptions import (
+    AuthenticationFailedError,
+    InvalidSubdomainError,
+    RealmDeactivatedError,
+    UserDeactivatedError,
+)
 from zerver.lib.request import REQ, JsonableError, has_request_variables
-from zerver.lib.response import json_error, json_success
+from zerver.lib.response import json_success
 from zerver.lib.subdomains import get_subdomain
 from zerver.lib.users import get_api_key
 from zerver.lib.validator import validate_login_email
@@ -103,23 +109,22 @@ def api_dev_fetch_api_key(request: HttpRequest, username: str = REQ()) -> HttpRe
     validate_login_email(username)
     realm = get_realm_from_request(request)
     if realm is None:
-        raise JsonableError(_("Invalid subdomain"))
+        raise InvalidSubdomainError()
     return_data: Dict[str, bool] = {}
     user_profile = authenticate(dev_auth_username=username, realm=realm, return_data=return_data)
     if return_data.get("inactive_realm"):
-        return json_error(
+        raise RealmDeactivatedError()
-            _("This organization has been deactivated."),
-            data={"reason": "realm deactivated"},
-            status=403,
-        )
     if return_data.get("inactive_user"):
-        return json_error(
+        raise UserDeactivatedError()
-            _("Your account has been disabled."), data={"reason": "user disable"}, status=403
+    if return_data.get("invalid_subdomain"):
-        )
+        raise InvalidSubdomainError()
     if user_profile is None:
-        return json_error(
+        # Since we're not actually checking passwords, this condition
-            _("This user is not registered."), data={"reason": "unregistered"}, status=403
+        # is when one's attempting to send an email address that
-        )
+        # doesn't have an account, i.e. it's definitely invalid username.
+        raise AuthenticationFailedError()
+    assert user_profile is not None
+
     do_login(request, user_profile)
     api_key = get_api_key(user_profile)
     return json_success({"api_key": api_key, "email": user_profile.delivery_email})