mirror of
				https://github.com/zulip/zulip.git
				synced 2025-11-04 05:53:43 +00:00 
			
		
		
		
	Improve api_fetch_api_key error messages.
Previously, api_fetch_api_key would not give clear error messages if password auth was disabled or the user's realm had been deactivated; additionally, the account disabled error stopped triggering when we moved the active account check into the auth decorators.
This commit is contained in:
		@@ -7,6 +7,7 @@ import mock
 | 
			
		||||
 | 
			
		||||
from zerver.lib.actions import do_deactivate_realm, do_deactivate_user, \
 | 
			
		||||
    do_reactivate_realm, do_reactivate_user
 | 
			
		||||
from zerver.lib.initial_password import initial_password
 | 
			
		||||
from zerver.lib.test_helpers import (
 | 
			
		||||
    AuthedTestCase,
 | 
			
		||||
)
 | 
			
		||||
@@ -139,3 +140,41 @@ class AuthBackendTest(TestCase):
 | 
			
		||||
        with self.settings(SSO_APPEND_DOMAIN='zulip.com'):
 | 
			
		||||
            self.verify_backend(ZulipRemoteUserBackend(),
 | 
			
		||||
                                email_to_username=email_to_username)
 | 
			
		||||
 | 
			
		||||
class FetchAPIKeyTest(AuthedTestCase):
 | 
			
		||||
    def setUp(self):
 | 
			
		||||
        self.email = "hamlet@zulip.com"
 | 
			
		||||
        self.user_profile = get_user_profile_by_email(self.email)
 | 
			
		||||
 | 
			
		||||
    def test_success(self):
 | 
			
		||||
        result = self.client.post("/api/v1/fetch_api_key",
 | 
			
		||||
                                  dict(username=self.email,
 | 
			
		||||
                                       password=initial_password(self.email)))
 | 
			
		||||
        self.assert_json_success(result)
 | 
			
		||||
 | 
			
		||||
    def test_wrong_password(self):
 | 
			
		||||
        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)
 | 
			
		||||
 | 
			
		||||
    def test_password_auth_disabled(self):
 | 
			
		||||
        with mock.patch('zproject.backends.password_auth_enabled', return_value=False):
 | 
			
		||||
            result = self.client.post("/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)
 | 
			
		||||
 | 
			
		||||
    def test_inactive_user(self):
 | 
			
		||||
        do_deactivate_user(self.user_profile)
 | 
			
		||||
        result = self.client.post("/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)
 | 
			
		||||
 | 
			
		||||
    def test_deactivated_realm(self):
 | 
			
		||||
        do_deactivate_realm(self.user_profile.realm)
 | 
			
		||||
        result = self.client.post("/api/v1/fetch_api_key",
 | 
			
		||||
                                  dict(username=self.email,
 | 
			
		||||
                                       password=initial_password(self.email)))
 | 
			
		||||
        self.assert_json_error_contains(result, "Your realm has been deactivated", 403)
 | 
			
		||||
 
 | 
			
		||||
@@ -1078,14 +1078,18 @@ def api_fetch_api_key(request, username=REQ, password=REQ):
 | 
			
		||||
    if username == "google-oauth2-token":
 | 
			
		||||
        user_profile = authenticate(google_oauth2_token=password, return_data=return_data)
 | 
			
		||||
    else:
 | 
			
		||||
        user_profile = authenticate(username=username, password=password)
 | 
			
		||||
        user_profile = authenticate(username=username, password=password, return_data=return_data)
 | 
			
		||||
    if return_data.get("inactive_user") == True:
 | 
			
		||||
        return json_error("Your account has been disabled.", data={"reason": "user disable"}, status=403)
 | 
			
		||||
    if return_data.get("inactive_realm") == True:
 | 
			
		||||
        return json_error("Your realm has been deactivated.", data={"reason": "realm deactivated"}, status=403)
 | 
			
		||||
    if return_data.get("password_auth_disabled") == True:
 | 
			
		||||
        return json_error("Password auth is disabled in your team.", data={"reason": "password auth disabled"}, status=403)
 | 
			
		||||
    if user_profile is None:
 | 
			
		||||
        if return_data.get("valid_attestation") == True:
 | 
			
		||||
            # We can leak that the user is unregistered iff they present a valid authentication string for the user.
 | 
			
		||||
            return json_error("This user is not registered; do so from a browser.", data={"reason": "unregistered"}, status=403)
 | 
			
		||||
        return json_error("Your username or password is incorrect.", data={"reason": "incorrect_creds"}, status=403)
 | 
			
		||||
    if not user_profile.is_active:
 | 
			
		||||
        return json_error("Your account has been disabled.", data={"reason": "disabled"}, status=403)
 | 
			
		||||
    return json_success({"api_key": user_profile.api_key, "email": user_profile.email})
 | 
			
		||||
 | 
			
		||||
@authenticated_json_post_view
 | 
			
		||||
 
 | 
			
		||||
@@ -40,12 +40,18 @@ def google_auth_enabled():
 | 
			
		||||
            return True
 | 
			
		||||
    return False
 | 
			
		||||
 | 
			
		||||
def common_get_active_user_by_email(email):
 | 
			
		||||
def common_get_active_user_by_email(email, return_data=None):
 | 
			
		||||
    try:
 | 
			
		||||
        user_profile = get_user_profile_by_email(email)
 | 
			
		||||
    except UserProfile.DoesNotExist:
 | 
			
		||||
        return None
 | 
			
		||||
    if not user_profile.is_active or user_profile.realm.deactivated:
 | 
			
		||||
    if not user_profile.is_active:
 | 
			
		||||
        if return_data is not None:
 | 
			
		||||
            return_data['inactive_user'] = True
 | 
			
		||||
        return None
 | 
			
		||||
    if user_profile.realm.deactivated:
 | 
			
		||||
        if return_data is not None:
 | 
			
		||||
            return_data['inactive_realm'] = True
 | 
			
		||||
        return None
 | 
			
		||||
    return user_profile
 | 
			
		||||
 | 
			
		||||
@@ -74,7 +80,7 @@ class EmailAuthBackend(ZulipAuthMixin):
 | 
			
		||||
    a username/password pair.
 | 
			
		||||
    """
 | 
			
		||||
 | 
			
		||||
    def authenticate(self, username=None, password=None):
 | 
			
		||||
    def authenticate(self, username=None, password=None, return_data=None):
 | 
			
		||||
        """ Authenticate a user based on email address as the user name. """
 | 
			
		||||
        if username is None or password is None:
 | 
			
		||||
            # Return immediately.  Otherwise we will look for a SQL row with
 | 
			
		||||
@@ -82,10 +88,12 @@ class EmailAuthBackend(ZulipAuthMixin):
 | 
			
		||||
            # exposure.
 | 
			
		||||
            return None
 | 
			
		||||
 | 
			
		||||
        user_profile = common_get_active_user_by_email(username)
 | 
			
		||||
        user_profile = common_get_active_user_by_email(username, return_data=return_data)
 | 
			
		||||
        if user_profile is None:
 | 
			
		||||
            return None
 | 
			
		||||
        if not password_auth_enabled(user_profile.realm):
 | 
			
		||||
            if return_data is not None:
 | 
			
		||||
                return_data['password_auth_disabled'] = True
 | 
			
		||||
            return None
 | 
			
		||||
        if user_profile.check_password(password):
 | 
			
		||||
            return user_profile
 | 
			
		||||
@@ -112,7 +120,11 @@ class GoogleMobileOauth2Backend(ZulipAuthMixin):
 | 
			
		||||
            except UserProfile.DoesNotExist:
 | 
			
		||||
                return_data["valid_attestation"] = True
 | 
			
		||||
                return None
 | 
			
		||||
            if not user_profile.is_active or user_profile.realm.deactivated:
 | 
			
		||||
            if not user_profile.is_active:
 | 
			
		||||
                return_data["inactive_user"] = True
 | 
			
		||||
                return None
 | 
			
		||||
            if user_profile.realm.deactivated:
 | 
			
		||||
                return_data["inactive_realm"] = True
 | 
			
		||||
                return None
 | 
			
		||||
            return user_profile
 | 
			
		||||
        else:
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user