diff --git a/zerver/lib/exceptions.py b/zerver/lib/exceptions.py index 5ea75cccd6..6490267a9f 100644 --- a/zerver/lib/exceptions.py +++ b/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 diff --git a/zerver/tests/test_auth_backends.py b/zerver/tests/test_auth_backends.py index 4fa6097af6..ed04736446 100644 --- a/zerver/tests/test_auth_backends.py +++ b/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): diff --git a/zerver/views/auth.py b/zerver/views/auth.py index 950774b4cb..df44d91796 100644 --- a/zerver/views/auth.py +++ b/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( - _("Your account has been disabled."), data={"reason": "user disable"}, status=403 - ) + raise UserDeactivatedError() if return_data.get("inactive_realm"): - return json_error( - _("This organization has been deactivated."), - data={"reason": "realm deactivated"}, - status=403, - ) + raise RealmDeactivatedError() if return_data.get("password_auth_disabled"): - return json_error( - _("Password auth is disabled in your team."), - data={"reason": "password auth disabled"}, - status=403, - ) + raise PasswordAuthDisabledError() if return_data.get("password_reset_needed"): - return json_error( - _("You need to reset your password."), - data={"reason": "password reset needed"}, - status=403, - ) + raise PasswordResetRequiredError() if user_profile is None: - return json_error( - _("Your username or password is incorrect."), - data={"reason": "incorrect_creds"}, - status=403, - ) + raise AuthenticationFailedError() # Maybe sending 'user_logged_in' signal is the better approach: # user_logged_in.send(sender=user_profile.__class__, request=request, user=user_profile) diff --git a/zerver/views/development/dev_login.py b/zerver/views/development/dev_login.py index 0cec6a1517..e5557d1cc8 100644 --- a/zerver/views/development/dev_login.py +++ b/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( - _("This organization has been deactivated."), - data={"reason": "realm deactivated"}, - status=403, - ) + raise RealmDeactivatedError() if return_data.get("inactive_user"): - return json_error( - _("Your account has been disabled."), data={"reason": "user disable"}, status=403 - ) + raise UserDeactivatedError() + if return_data.get("invalid_subdomain"): + raise InvalidSubdomainError() if user_profile is None: - return json_error( - _("This user is not registered."), data={"reason": "unregistered"}, status=403 - ) + # Since we're not actually checking passwords, this condition + # 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})